Repository navigation
soc: apple: vote GPU activity to the PMP so memory leaves idle DVFS on t600x - #334
joshuaswarren wants to merge 19 commits into
Conversation
…n t600x On the M1 Max (t6001) the PMP keeps memory at its idle floor for a busy GPU until the AGX power request, bit 9 of the PMP report's PS-REQ word, is set. Nothing sets it on Linux, so GPU reads top out at 241 GB/s. The same chip reads 361 GB/s on macOS. Report GPU activity to the PMP. The GPU driver calls apple_pmp_agx_activity_begin() when a submission starts and apple_pmp_agx_activity_end() when it finishes. pmp-report sets bit 9 when the first operation starts and clears it 100 ms after the last one ends. It also clears it at once when the GPU driver unbinds or the report goes away, so the request is never left set past the GPU's power-domain release. The register sequence is the one apple_pmp_report_set_state() already uses for report entries: read 0xf80, write 0x107c0, poll 0x1000, status 0x10, 50 ms acknowledge. A set that is not acknowledged is cleared again and not retried for a second. The vote is off by default. pmp_report.agx_vote=1 turns it on, and pmp_report.agx_vote_idle_ms (default 100) sets the hold. With it off nothing is written. It applies to apple,t6001 only; t6000 is untested. The decisions (set, clear, hold, retry, wrap of the 32-bit millisecond clock) are plain functions in pmp-agx-vote.h with no kernel dependency. Setting the bit by hand with a test module on a 16-inch M1 Max moved a GPU read microbenchmark from 241 to 368 GB/s. This patch has not yet been run on hardware. Signed-off-by: Joshua Warren <816217+joshuaswarren@users.noreply.github.com>
…one power request bit The PS-REQ word of the PMP report is shared by every client of the PMP: the report entries (display, ISP, video decode, the AFNC IOAs), the T6001 AGX vote (bit 9) and, out of tree, the ANE clock level of omarchy-ane (bit 10). The report entries and the AGX vote read-modify-write the word under rep->lock. A module outside this driver could only map the window and read-modify-write it without that lock, so a concurrent write of another bit could be lost. apple_pmp_report_set_request(report_np, id, enable, no_ack) is the one writer a client outside this file may use. It calls the same apple_pmp_report_set_state() the entries use, so it takes the same lock and does the same acknowledge poll (at most 50 ms). The caller owns the bit it asks for. Signed-off-by: Joshua Warren <816217+joshuaswarren@users.noreply.github.com>
|
GREEN on f7ad8cf Source review of the combined head: 689f2f9 (the AGX vote port) plus f7ad8cf (export of Reviewer identity: routed by Main to Sonnet (Claude); the model id is unverifiable from inside the session and Main's routing notice is the source. The implementer is a different party; implementer model unverified. I did not edit the PR and I did not build it; the compile check on the build host is a separate step. Main's three questions, answered from the code at this head:
Also checked:
Notes, none blocking: Not covered: no build, no boot, no hardware run from me. Measured elsewhere on the same hardware: setting bit 9 raised the Linux read roof from 241 to 368 GB/s and 8192-context prefill on the 2B by 6.5%; the decode result with the vote is pending. |
…ment clearing apple_pmp_report_set_request() lets a client outside pmp-report update one bit of the PS-REQ word. On the T6001 bit 9 is the AGX request, which the GPU vote owns: a second writer could clear it while the GPU works, or set it and leave it set. Refuse it from the export with -EBUSY on a t600x report. The vote itself writes through apple_pmp_report_set_state() and is not affected. Document what a caller must do after -ETIMEDOUT: the request was written but not acknowledged and stays in the word, so the caller clears it with @enable false and @no_ack true before giving up. Signed-off-by: Joshua Warren <816217+joshuaswarren@users.noreply.github.com>
|
GREEN on e033631 Source re-review of the combined head, one commit (e033631, "keep the AGX bit out of the export, document clearing") over f7ad8cf, which I reviewed and approved earlier. It closes my notes N1 and N2 from that review. Reviewer identity: routed by Main to Sonnet (Claude); the model id is unverifiable from inside the session and Main's routing notice is the source. The implementer is a different party; implementer model unverified. I did not edit the PR and I did not build it; the compile check is a separate step. Delta checked at the exact head (fetched
Observation, no action: the guard keys on the t600x offsets, so it also refuses id 9 on a T6000 report; the comment names the T6001. The refusal is conservative and the T6000 AGX bit would be the same index, so I see no harm. Not covered: no build, boot or hardware run from me. The lab-tree 0003 that Main mentions (main ea079b221) is a separate artifact and is not part of this review. |
…st operation A 9B model at 8192 tokens of context had one gap of up to 111.5 ms between two decoded tokens in every process, once per run, with the GPU idle for a good part of it. The 100 ms default hold dropped the request in that gap, which costs one slow token while the PMP raises memory again. The longest gap in any other configuration measured was 64 ms. Raise the default to 250 ms, about 2.2 times the longest gap seen. The cost is a few hundred milliseconds of the higher memory state after the last job. pmp_report.agx_vote_idle_ms still overrides it at run time. The default is one constant, AGX_VOTE_IDLE_MS_DEFAULT, used for both the parameter and the initial state. The userspace test has a case that rides through a 112 ms gap with the default and still drops the request after a long idle. Signed-off-by: Joshua Warren <816217+joshuaswarren@users.noreply.github.com>
|
GREEN on 9f2a319 Source re-review of the combined head: one commit (9f2a319, "hold the AGX request 250 ms after the last operation") over e033631, which I approved earlier. This is the change I suggested after the 9B result (hold of 100 ms is below the largest token interval seen). Reviewer identity: routed by Main to Sonnet (Claude); the model id is unverifiable from inside the session and Main's routing notice is the source. The implementer is a different party; implementer model unverified. I did not edit the PR and I did not build it; the compile check is a separate step. Delta checked at the exact head (fetched
Note, no action needed for the GREEN: the new comment says "w7X measured idle gaps of up to 111.5 ms inside one 9B generation". What I measured is the largest token interval (111.5 ms at 9B, context 8192, once per process); in the one case I inspected in the 20 Hz firmware stats the GPU was idle for at least 50 ms of that interval. "Token intervals of up to 111.5 ms" is the accurate wording. The 250 ms figure (about 2.2 times that interval) stands either way. Not covered: no build, boot or hardware run from me. The decode A/B through this parameter on the lab3 kernel is held for the one-shot slot; its rows write 250 to the parameter at run time, which is this default. |
scottjones
left a comment
There was a problem hiding this comment.
We reviewed 9f2a319 against 2a76ae3. The vote logic is small and easy to follow, and the begin/OpGuard pairing in start_op() holds on every path, including a failed kick. Findings 1 and 2 are the blockers. Finding 3 is a should-fix, and finding 4 is a question.
1. pmp_report.agx_vote=1 on the kernel command line panics at boot. drivers/pmdomain/apple/pmp-report.c:1340
APPLE_PMP_REPORT is a bool, so this setter runs from parse_args() at init/main.c:1016. workqueue_init_early() only runs at init/main.c:1070, so system_wq is still NULL. kick(0) goes mod_delayed_work_on() -> __queue_delayed_work() (delay 0) -> __queue_work(), which reads wq->flags at kernel/workqueue.c:2286. Nothing checks for a NULL wq on the way. This runs in start_kernel on the boot CPU, so the fault kills the idle task and the machine never boots, probably before the console is up. The A/B runs flipped the param at run time, so this path was never hit. The PR description says "pmp_report.agx_vote=1 turns it on", and putting that on the command line is how most people will do it. Since begin() and the work fn already read agx_vote themselves, skipping the kick before the workqueues exist is enough (for example if (system_wq), or a check on system_state).
Related, minor: system_wq is __WQ_DEPRECATED in this tree (kernel/workqueue.c:8088), so the first kick also prints the "enqueued on deprecated workqueue" warning. system_dfl_wq avoids it. pmp-report.c:1332
2. A begin during the clear write leaves the GPU running with no vote and nothing scheduled. pmp-report.c:1416, drivers/pmdomain/apple/pmp-agx-vote.h:86
The work fn picks CLEAR under the spinlock, drops it, then does the write and the ack poll (100 us to 50 ms). If an operation begins in that window, agx_vote_begin() sees voted still true and does not kick. agx_vote_applied() then sets voted = false, and agx_vote_next() has no case for "users > 0, not voted, no failures", so it returns 0 and no work is queued. Bit 9 stays clear until some later begin happens. The likeliest time is when work resumes after more than 250 ms idle, such as a new prompt after a pause. In a decode stream that costs one operation. A long single submission runs at the idle floor for its whole length. We reproduced it with the header alone: decide (CLEAR), begin, applied, next gives users=1 voted=0 next=0, with decide() now returning SET. Re-running decide() after applied(), and kicking 0 when it is not NONE, closes it. The userspace test should get this case too.
3. release() does not stop a later begin from setting the bit again. drivers/gpu/drm/asahi/driver.rs:236, pmp-report.c:1462
unbind() calls release() first, but the DRM registration (_reg) is part of the driver data and is only dropped after unbind() returns (driver.rs:51). A submission in that window calls begin(), sees !voted, kicks the work, and the work sets bit 9 again. The bit can then stay set past the GPU's power-domain release, which the commit message says cannot happen. The report side has the same shape: pmp-report.c:187 clears the vote before list_del_rcu() at :191, so work that a begin queues between the two can set the bit on a report that is going away. A "stopped" flag under the vote spinlock, checked in begin(), would fix both. Clearing it needs a new call at GPU probe, so a rebind can vote again. Moving release() later does not help. drm_dev_unplug() only runs in Registration::drop after unbind returns (rust/kernel/drm/driver.rs:242). Even then, drm_ioctl() checks for unplug only on entry (drivers/gpu/drm/drm_ioctl.c:810), and the asahi submit path has no drm_dev_enter() section, so an ioctl already past that check can still reach start_op(). We think this one is rare, because it needs a GPU unbind or report unbind with work in flight.
4. Question: what should happen to bit 9 over system sleep? pmp-report.c:1593
The report entries already withdraw requests in suspend_noirq. The comment there says a held external display request cost about 0.25 W in s2idle on J514S, which is this SoC family. We don't know what holding the AGX bit costs. The AGX vote has no equivalent. If suspend starts within the 250 ms hold, or with an operation in flight (asahi has no sleep hooks), bit 9 can be held through s2idle. If the PMP drops the word over sleep, voted stays true after resume and the first busy period gets no SET. We have not measured either case. A clear in suspend_noirq (or a PM notifier) and a re-decide on resume look cheap. If you know the bit is harmless in s2idle, a comment saying so would be enough.
Small things
- The Signed-off-by on 9f2a319 says
joshuawarren, while the author isjoshuaswarren. checkpatch flags the mismatch. - The vote test lives outside this tree, so nothing here fails if the logic regresses. The KUnit follow-up would cover that.
What we checked: we built pmp-report.o with W=1 on aarch64 (clean), and the four exports are present and GPL. We confirmed the boot order in init/main.c and the NULL deref path in kernel/workqueue.c. We reproduced finding 2 against pmp-agx-vote.h under ASan/UBSan, and ran checkpatch. We did not build the Rust side, boot anything, or run on hardware.
… it across unbind and sleep Review of the AGX vote found four problems. 1. agx_vote=1 on the kernel command line runs the parameter setter from parse_args(), before workqueue_init_early() has created the workqueues, and the setter queued work on a NULL workqueue. kick() now returns while system_dfl_wq does not exist; begin() and the work function read the parameter themselves, so the first GPU operation votes. It also uses system_dfl_wq instead of the deprecated system_wq. 2. An operation that began while the clear was being written saw voted still true and did not kick; after the clear nothing scheduled a set, so the GPU ran at the idle floor until the next begin. agx_vote_next() now reports a set that is owed (1 ms) when users > 0, the request is not voted and no set has failed. 3. release() did not stop a later begin() from setting the bit again, and the report's removal had the same shape. A stopped flag, set under the vote lock at release and at report removal, is checked by begin(), decide() and next(); it is cleared by a new apple_pmp_agx_activity_resume() called at GPU probe and at report probe. 4. The request is withdrawn on PM_SUSPEND_PREPARE and PM_HIBERNATION_PREPARE and decided again on PM_POST_SUSPEND and PM_POST_HIBERNATION: holding the bit through s2idle may cost power (a held display request cost about 0.25 W on J514S), and the PMP may drop the word across sleep, which would leave voted true with nothing on the wire. Each case has a test in tools/ptd-vote of the research repository that carries the policy header (cases 14 to 17 of the vote simulator, and a source check for the boot-time kick); they were written to fail first. Signed-off-by: Joshua Warren <816217+joshuaswarren@users.noreply.github.com>
|
GREEN on 778b924 Source re-review of one commit (778b924, "close the races in the AGX vote and stop it across unbind and sleep") over 9f2a319, which I approved earlier. The delta is four files (driver.rs +3, pmp-agx-vote.h +28 -5, pmp-report.c +71 -1, pmp-report.h +5), read in full, plus the vote code around it at the new head. Not built or booted by me: the build result is the author's statement; my checks are reading, the simulator and the source check below. Reviewer identity: routed by Main to Sonnet (Claude); the model id is unverifiable from inside the session and Main's routing notice is the source. The implementer's model is unverified. The four findings, each against the code:
The check you asked for, that I did not weaken the earlier behaviour (with the vote on, decode must still set once and hold): by reading, the new conditions only add false/NONE when stopped is set, and the only new non-zero timer is the owed set (users, not voted, no failures), which never occurs while voted is true. With voted true and bursts every ~8 ms the path is the old one: end() queues the idle timer, begin() does not kick, the work function finds users or an unexpired hold and decides NONE. Cases 1 to 13 of the simulator (including case 13, the 111.5 ms gap with one vote and no clear) pass unchanged against the new header. I did not run decode on hardware for this commit; the kernel with this commit has not been built into a lab image that I have seen, so the +25 to +29 percent decode result belongs to the earlier commits. Tests, run by me: the header copy in the research repo at fec36d0f1 is byte-identical to the PR's pmp-agx-vote.h (sha256 b6788472ea0d7eac...). The simulator (cases 1 to 17) prints AGX-VOTE-SELFCHECK-OK with -fsanitize=address,undefined (ASan needs setarch -R on this host; with ASLR on it dies at startup, not a test failure) and with UBSan alone. Mutation of the header, ten mutants: killed seven (decide ignoring stopped, next ignoring stopped, no owed set, release not setting stopped, resume not clearing it, resume never asking to kick, resume kicking when idle). Three survive, none blocking: Not blocking, for the record:
|
|
Thank you for the review. All four points were real. The fix is one commit, Point 1, the boot-time panic: Point 2, a begin during the clear write: Point 3, release and report removal: A Point 4, system sleep: A PM notifier withdraws the request on Signed-off-by: The wrong address is on Checked: |
778b924 to
6ece082
Compare
|
Update: I fixed the Signed-off-by address on |
…d report probe now uses The report probe tells the AGX vote that a T6001 report came back (apple_pmp_agx_activity_resume) and compares rep->offsets with the T6001 table. The harness extracts the probe verbatim and compiles it standalone, so production.h failed with an undeclared apple_pmp_offsets_t600x and an implicit declaration of apple_pmp_agx_activity_resume. An empty T6001 table that never matches and an empty resume function are enough: the harness models the T6030 graph. All 21 graph cases pass, and the supplier harness passes (ASan needs ASLR off on this host: setarch -R). Signed-off-by: Joshua Warren <816217+joshuaswarren@users.noreply.github.com>
|
Head is now The The fix is four lines in Run locally: the graph harness passes 21 of 21 cases, and the mutation controls still abort. The supplier harness passes. The four findings from the review are fixed at this head. A re-check is welcome. |
|
GREEN at e8ddbd9 Reviewer identity: routed by Main to Sonnet (Claude); the model id is unverifiable from inside the session and Main's routing notice is the source. The implementer's model is unverified. PR head checked after the push: e8ddbd9 has tree c80c04833abbe7d179acb3afaf943697e6bd5430 (the tree I reviewed from w72's branch) and parent 6ece082, which has the same tree (06cb58b9d15f...) as 778b924, the head I approved earlier. The only change since that approval is the 4 lines in tools/asahi/t6030-pmp-graph-tests.c. My earlier remark about pushing on top of 778b924 does not apply: the PR chain had already been re-signed to 6ece082 with identical content, and this commit is a fast-forward of it. Re-run by me from Follow-up, not a blocker: make the empty resume stub count its calls and assert 0 at the end of the 21 cases, so the harness can notice if the T6030 probe starts calling the vote's resume. |
scottjones
left a comment
There was a problem hiding this comment.
We re-reviewed at 6ece082 and checked e8ddbd9 on top, which only adds the 4 lines in tools/asahi/t6030-pmp-graph-tests.c. git diff 778b924bc058 6ece0825c8eb is empty, and 9f2a319 and 5b54de3 have the same tree, so the force-push only changed the Signed-off-by. All five commits now sign off as joshuaswarren. Thanks for the quick turnaround. All four points are fixed. The one thing we'd still like before this merges is the vote test in the tree (see Tests).
1. Boot panic with agx_vote=1 on the command line. Fixed. The setter runs from parse_args() on the boot CPU, before anything else can run. system_dfl_wq is written once, in workqueue_init_early(), so the NULL check in kick() can't race. The value still applies later: begin() reads agx_vote itself and kicks on the first GPU operation once the workqueue exists. The deprecated system_wq is gone too.
2. Lost SET when begin() races the CLEAR write. Fixed. After the clear, agx_vote_next() returns 1 and the work runs a SET. We checked that it can't spin. Every state where next() returns 1 through the new branch is one where decide() returns SET. After a failed set, next() returns the retry delay, not 1. We checked this by hand and with a sweep over all the flag combinations.
3. Teardown race. Fixed. release() and report removal set stopped under the spinlock before cancel_delayed_work_sync(). The mutex is not held across the cancel, so it can't deadlock with the work fn. A begin() after the stop still counts a user and still ends cleanly, so the count stays balanced and nothing leaks. It just doesn't vote. An end() that queues the idle timer between the cancel and the stop finds stopped in the work fn and writes nothing. Rebind works. The GPU probe and the report probe both call resume(), and resume() kicks when work is already in flight. The report probe calls it after list_add_tail_rcu(), so that kick can find the report.
One thing we agree with from your second reviewer, not a blocker: one stopped flag stands for unbind, report removal and sleep. A suspend and resume after a GPU unbind clears it, and so does a report rebind. Only an operation still in flight from before the unbind could then vote. The vote drops 250 ms after that operation ends, so if it never ends, the bit stays set until it does. That costs power, not correctness. Separately, the module exit note in that review doesn't apply. APPLE_PMP_REPORT is a bool, so apple_pmp_report_exit() never runs.
4. Sleep. Fixed. The PM notifier clears on PM_SUSPEND_PREPARE and PM_HIBERNATION_PREPARE and decides again on PM_POST_SUSPEND and PM_POST_HIBERNATION, all in process context. A work item already running when suspend starts is waited out, and its set is cleared by the stop().
Tests. tools/ptd-vote isn't in a repo we can reach, so we couldn't run cases 14 to 17 or check that they fail. We wrote our own harness against the header. Removing the owed-set branch from next() fails it, and so does dropping stopped from begin() or decide(), dropping !failures from the new branch, or making resume() leave stopped set. Your mutation run says the begin() and !failures mutants survive in your suite. Asserting !agx_vote_begin() after a refuse, and next == retry_at - now after a failed set, would kill both. Nothing in this tree fails yet if the policy regresses, and that's the reason we're requesting changes. The simulator is already plain C against pmp-agx-vote.h with no kernel dependency, so it fits the pattern tools/asahi already uses: a .c harness with a Python wrapper, registered in tools/aurora-ci/host-tests.json like t6030-pmp-graph-tests, so the host tests (tools) job runs it on every PR. Please add it to this PR, including the 17 cases and the two asserts above. KUnit can still come later.
What we ran: pmp-report.o with W=1 on aarch64 (clang, LLVM=1), clean. drivers/gpu/drm/asahi/ with Rust 1.85, which builds; its four dead-code warnings are in code this PR doesn't touch. checkpatch shows only long lines. We did not boot anything, suspend, unbind, or run on hardware.
|
For reproducing the benchmark, could you link the harness/commands and pin the tested kernel commit and config? The description identifies a 7.1.12 lab-kernel form, while current head Scott’s request to put the vote-policy suite and its regression assertions into this tree’s host CI is still outstanding; we’ll check the successor when that lands. |
chriskearney
left a comment
There was a problem hiding this comment.
At e8ddbd9fff0252ef8aeb95a3463cf1b8b1ce24c1, two nonblocking retry-policy issues reproduce against the exact pmp-agx-vote.h (SHA256 b6788472ea0d7eac557faf0850a7b699fdf30ab2fb83cb5773cc9a494e32519d):
-
Backoff limits writes, but not immediate reconciliation requests. After a failed SET, every
agx_vote_begin()still returns true whilevotedis false.agx_vote_decide()correctly returns NONE beforeretry_at, but the glue keeps asking the workqueue to run immediately. In a sequential host model, 99 operations inside the backoff requested 99 immediate reconciliations and made zero SET attempts. Kernel workqueue coalescing can reduce the number of actual executions. This also occurs when opt-in is enabled on a non-T6001 machine and writes return-ENODEV. -
An idle interval beyond half the 32-bit clock range can revive an expired backoff.
failuresandretry_atsurvive the last end indefinitely. After one failed SET, ending the work and starting again atretry_at + 0x80000005Ugives NONE andagx_vote_next() == 2147483643ms, although the one-second retry delay expired long ago. The control atretry_at + 1000correctly gives SET. A bounded expiry of stored retry state, or an appropriately sourced wider clock, would avoid this. Please include a regression case when addressing it.
These are opt-in performance limitations, not new hardware-fault findings. Independent host policy tests pass under ASan+UBSan and UBSan; these are local checks, not tests registered in this PR. Scott’s request to land the vote suite and regression assertions in the repository’s host CI remains the merge blocker. The full package job is still running, and we have not booted this revision or measured its performance.
There was a problem hiding this comment.
Blocking finding at e8ddbd9fff0252ef8aeb95a3463cf1b8b1ce24c1: the final operation can end during a SET write and leave the AGX vote set with no clear scheduled.
In apple_pmp_agx_vote_fn(), the spinlock is dropped between agx_vote_decide() and agx_vote_applied() while the SET and acknowledgement happen. If the last end() runs in that window, it records idle_at but sees voted == false and requests no timer. The worker then marks the successful SET as voted. If the idle deadline has already elapsed, agx_vote_next() returns 0, which this caller interprets as no work to schedule. agx_vote_decide() would now return CLEAR, but nothing invokes it.
Minimal sequence against the exact policy header, following the actual work function’s ordering:
struct agx_vote v = { .enabled = true, .idle_ms = 0 };
agx_vote_begin(&v);
enum agx_vote_action act = agx_vote_decide(&v, 5000); /* SET */
agx_ms_t timer = agx_vote_end(&v, 5001); /* 0: not yet voted */
agx_vote_applied(&v, act, 0, 5000); /* SET acknowledged */
/* At 5001: users=0, voted=true, timer=0, next=0, decide=CLEAR. */This reproduces with the accepted runtime parameter agx_vote_idle_ms=0, or a 10 ms hold and an 11 ms write/worker interval. With the default 250 ms hold, the model’s 50 ms acknowledgement control correctly schedules a timer; a worker delayed until 251 ms reproduces the problem, including across clock wrap. That longer delay is a scheduling model, not a measured hardware acknowledgement time.
Please schedule another reconciliation when a CLEAR is already owed after the write, and add the end-during-SET interleaving to the in-tree suite. In a scratch control, returning 1 rather than 0 for an already-reached next deadline removes all four stuck cases and preserves the unit cases; it still needs normal review and CI.
Validation: independent host interleaving model plus deterministic header-only reproducer; I reran the reproducer and its control and checked the actual C work/end paths. No MMIO, boot, suspend or hardware test was performed. The two retry-policy notes in my earlier review remain nonblocking; this owed-CLEAR race and the missing in-tree regression coverage need resolving before merge.
Follow-up validation: the same failure also reproduces through the work/begin/end functions extracted from this exact PR, with register access and scheduling stubbed on the host. The locally corrected header clears those cases and retains the begin-during-CLEAR control. CI run 41648287024 has now passed the full package build and QEMU boot smoke check; neither exercises the Apple PMP hardware. These green checks do not resolve the reproduced race.
…ontrols The vote policy is a header of static inline transition functions, so the suite compiles pmp-agx-vote-tests.c against a copy of it and drives the same begin, decide, next and applied calls the kernel glue makes, with a fake clock, a fake PMP bit and a fake timer. 18 cases: the parameter off, the set and idle drop, decode-like bursts, a drop cancelled by new work, the failed set backoff, unbalanced ends, the parameter switched off mid-flight, the zero idle tick, millisecond clock wraps, release and refuse, resume with and without work in flight, sleep and resume, and the two asserts from review (the begin inside a clear write must be re-set, and begin must refuse while stopped). --mutation and --mutations remove one guard each (the owed-set branch, the !failures in it, and the stopped check in begin and decide) and require the suite to fail. The two mutants that survived before the races fix (begin not refusing while stopped, and the retry deadline indistinguishable from the 1 ms owed-set kick) are the stopped-in-begin and owed-set-branch controls: at 5b54de3 (the pre-fix header) both survive because the guarded lines do not exist; at this head each is caught at the case that pins it. Registered in tools/aurora-ci/host-tests.json (base, --mutations, four single-mutant entries, the .c as a helper). Proof: plain run passes (18 cases, AGX-VOTE-SELFCHECK-OK); --mutations exits 0 with all four caught; --ref 5b54de3 --mutation stopped-in-begin and --mutation owed-set-branch print SURVIVED there. Signed-off-by: Joshua Warren <816217+joshuaswarren@users.noreply.github.com>
…he mutation modes Review of 1142df9 by w7X, two non-blocking items folded in before push. Case 19 pins the signed 32-bit compare in agx_vote_reached: with idle_at 0xFFFFFFF0, idle_ms 100 and voted, decide at 0xFFFFFFF8 must be NONE and next must return the 92 ms to the wrapped deadline, and a retry_at that has wrapped must not read as reached either. The wrap-reached mutant (now >= t) survives the old cases because they jump straight to the wrapped deadline; it dies at line 306. Registered as a fifth mutation entry. The mutation modes now assert the unmutated suite passed first, so a base failure cannot make every mutant read as caught. The negative control (--ref) is exempt: the pre-fix header cannot compile this suite, which is the point. The suite runs under setarch -R when available; ASan startup crashes under ASLR on a loaded host otherwise. Proof: plain run passes; --mutations exits 0 with all five mutants caught; wrap-reached CAUGHT at line 306; at 5b54de3 the owed-set-branch and stopped-in-begin mutants still print SURVIVED. Signed-off-by: Joshua Warren <816217+joshuaswarren@users.noreply.github.com>
|
The vote simulator is in the tree. Head is
The mutant matrix:
The two mutants that survived before the review fixes were begin without the stopped refusal, and the retry deadline indistinguishable from the 1 ms kick. Against the header before the fix commit, both survive: the guarded lines do not exist there, so there is nothing to remove. Two review rounds on this suite are folded in: the clock-wrap case and the wrap mutant came from the first, the base-pass assert and a |
…ff kick storm and the revived retry Three cases for chriskearney's findings on AsahiLinux#334, written first and red at this commit: - Case 20: an end lands between decide (SET) and applied. The end sees voted false, arms nothing, and applied then sets the vote with no user and an already-reached idle deadline. next() must return 1 there, not 0: 0 is the suite's "nothing pending", so the caller would never run the reconcile that clears the vote. - Case 21: an idle gap beyond half the 32-bit millisecond range makes the wrapped arithmetic read an expired retry deadline as future, which would keep the vote off for days. A retry deadline more than one backoff window in the future cannot be real and counts as reached. - Case 22: during backoff, begin must not ask for a reconciliation decide would refuse; the retry timer already drives the set. Three mutants pin them (set-write-ends, retry-revival, begin-backoff-kick), and stopped-in-begin now matches the begin line the fix will produce. Red here: case 20 fails at `agx_vote_next(&v, 5001) == 1`. Signed-off-by: Joshua Warren <816217+joshuaswarren@users.noreply.github.com>
…try deadline and quiet the backoff kicks Three findings from chriskearney's review of AsahiLinux#334; the suite cases landed first (825e9ac, red) and this is the smallest change that turns them green. - end() during the SET write (between decide and applied) sees voted false and arms nothing; applied then leaves the vote set with no user and an already-reached idle deadline, and next() answered 0, which the caller reads as nothing pending. An already-reached deadline now returns 1, so the caller runs the reconcile that clears the vote. It settles one step later: decide CLEAR, applied, next 0. - A retry deadline more than one backoff window in the future cannot be real (an idle gap beyond half the 32-bit millisecond range reads an expired deadline as future through the wrapped compare), so the new agx_vote_retry_reached() treats it as expired instead of keeping the vote off for days. - begin() no longer asks for a reconciliation while a failed set is in backoff: decide would return NONE, and the retry timer already drives the set. One kick per operation during the window was pure waste. The mutants in tools/asahi/pmp-agx-vote-tests.py pin each: set-write-ends dies at the race assert, retry-revival at the bound assert, begin-backoff-kick at the kick-count assert, and the five earlier mutants still die. Plain suite: 22 cases, selfcheck ok. Signed-off-by: Joshua Warren <816217+joshuaswarren@users.noreply.github.com>
…ot strand or spam the vote w7X's review of the first cut (a7330a5) found a blocking regression and two correct behaviours to keep. The gate on begin had to know the time, so agx_vote_begin() now takes now (the caller reads it under its lock; the glue passes apple_pmp_agx_now_ms() beside its existing end and next reads): - begin returns false during a failed-set backoff before the deadline (decide would return NONE; the retry timer drives the set), and true again once the deadline has passed. The first cut's unconditional !v->failures stranded the vote after one failed set whose retry timer fired idle: failures is only cleared by a successful SET, so every later begin refused forever. Case 23 is that sequence (failed set, end before the retry, timer fires idle, a later begin must kick and the set must land); the begin-ignores-retry mutant is exactly the first cut's expression and dies at it (line 363). - The other direction is pinned too: begin-backoff-kick (kick during the window regardless of the deadline) dies at the kick-storm case (line 357), so the gate cannot silently decay into "always kick" again. Case 23 exists because the simulator cases kept the operation alive through the backoff; this one ends it first. The begin signature change touches the suite's helper, four header-only call sites, and the one glue call. Proof: plain suite 23 cases selfcheck ok; --mutations catches all nine mutants (owed-set-branch, owed-failures, stopped-in-begin, stopped-in-decide, wrap-reached, set-write-ends, begin-backoff-kick, begin-ignores-retry, retry-revival); check-kick-guard still passes on pmp-report.c; the tree runner passes the registered vote entries. Signed-off-by: Joshua Warren <816217+joshuaswarren@users.noreply.github.com>
w7X's re-review nit: next() takes the first match, so the second entry could never be selected. Nine distinct mutants; --mutations still passes. Signed-off-by: Joshua Warren <816217+joshuaswarren@users.noreply.github.com>
|
All three findings from this review are addressed, with the suite cases landed first. Head is
One more round happened on the first cut of the begin gate: an unconditional On the benchmark: the decode figures in the description (+26.7% and +29.4% on the 9B 4-bit model, the +25 to 29% range) came from the earlier lab-kernel form of this branch, not from the current source. Machine and stack: a 16-inch M1 Max, lab kernel |
scottjones
left a comment
There was a problem hiding this comment.
We re-reviewed at efff25c. Thanks for the quick turnaround on both the suite and Chris's findings.
Tests. The test request is satisfied. tools/asahi/pmp-agx-vote-tests.c and .py are in the tree, and host-tests.json registers the base run, the --mutations self-check and each of the nine mutants in the tools group. From a fresh worktree, python3 tools/asahi/pmp-agx-vote-tests.py --output <dir> passes, --mutations catches all nine, and host-tests.py runs all 11 entries as passing. The host tests (tools) job at this head ran all 11 and passed. It includes the 17 earlier cases and the two asserts we asked for (lines 295 and 302).
We also reverted the fixes by hand instead of trusting the mutant list. Putting back the e8ddbd9 header (with only the begin() signature adapted) fails case 20. Reverting just the ? 1 in next() fails case 20. Reverting just the backoff gate in begin() fails case 22. Dropping the wrap clause in retry_reached() fails case 21, and so does dropping the ? now : v->retry_at in next(). Our four earlier points are still covered: the owed set after a CLEAR (case 14), stopped in begin() and decide() (cases 15 and 18), and sleep (case 16). Making resume() leave stopped set fails case 15. The boot kick guard and the PM notifier live in pmp-report.c, outside what the suite tests, and neither changed since e8ddbd9.
Chris's three findings. All fixed.
- End during the SET write. end() sees voted false and arms nothing, applied() then sets voted, and next() now returns 1 for the reached idle deadline, so the work runs the CLEAR. Correct.
- Kick storm in backoff. begin() now returns false before the retry deadline, so a busy GPU no longer queues a NONE reconcile per operation.
- Revived retry deadline. retry_reached() counts a deadline more than one backoff window ahead as expired, which covers the wrap after a long idle gap.
next() returning 1 can't spin. decide() and next() use the same predicates, so on the NONE path, where next() gets the same now, next() never returns 1. On the write path, a 1 always means the next run takes an action. A failing SET re-arms at about 1000 ms. The ack poll is capped at 50 ms, so the retry is never back to back. begin() has one kernel caller, apple_pmp_agx_activity_begin(), which reads jiffies_to_msecs(jiffies) under the vote spinlock. That is the same clock applied() uses, and it can't be earlier than the now retry_at was computed from.
We walked both directions of each window. Begin during SET: begin kicks, or the backoff re-arms. End during SET: fixed as above. Begin during CLEAR: next() returns 1 and the SET runs. End during CLEAR: that only happens for a disabled or stopped clear, where end's extra kick finds nothing to do. Resume during CLEAR: the same path as begin, which next() covers. We found nothing left open in these windows.
One small gap, not a blocker. The new backoff gate in begin() relies on the retry timer staying armed. If the GPU goes idle during a backoff and something else consumes that timer, the next begin() inside the window doesn't kick, and nothing is scheduled until a begin() after retry_at. Two things can consume it: a write to the agx_vote parameter (mod_delayed_work(0) replaces the timer, and the reconcile sees no users and re-arms nothing), or a release and resume, from suspend, GPU unbind and rebind, or report removal and reprobe (cancel_delayed_work_sync() drops it, and resume() won't kick with no users). We reproduced it on the header: fail a set at 0, end at 100, run the reconcile at 200, then begin at 300. begin returns false, decide would return SET at 1500, and nothing is armed. It recovers at the next begin after the deadline, so a steady GPU loses at most about a second. A single long job started in the window runs without the vote. A fix would be for next() to keep re-arming the retry deadline while idle in backoff, returning 0 once it passes so begin()'s gate takes over (returning 1 there would requeue every 1 ms while idle), and for resume() to kick when failures is set. Fine as a follow-up.
What we ran: the suite, the mutants and the hand reverts above. pmp-report.o with W=1 on aarch64 (clang 23, LLVM=1) builds clean. drivers/gpu/drm/asahi/ builds with Rust 1.85. Its dead-code warnings are in code this PR doesn't touch, and no Rust file changed since e8ddbd9. checkpatch on the six new commits shows long lines, block comment style and missing blank lines in the test file, nothing functional. We did not boot, suspend, unbind or run anything on hardware.
This looks right to us. Could it also cover t6000 (M1 Pro)? If AGX is soc-device record 9 there too, Marcelo has an M1 Pro that could test it, which would give us a second chip before it lands. Since this is new power behaviour that hasn't run on hardware yet, we'd like to approve after 4.5 ships.
There was a problem hiding this comment.
Re-review at efff25c15c4f1abb1fb141d87ecfe07432bdb3c4: the original owed-CLEAR blocker is resolved, and the in-tree coverage request is resolved. The unchanged extracted-C reproducer now clears all five previously stuck cases; the new suite catches the old header at case 20. The registered suite and all nine mutants pass locally and in CI.
There is a new blocking regression in the quiet-begin gate: a job starting during backoff can have no timer to retry its vote. The gate assumes that a retry timer already exists, but that is not guaranteed. Reproduced ordering through this revision’s actual work/begin/end functions, with register access and scheduling stubbed on the host:
- The worker decides SET with one operation active.
- The last operation ends during the acknowledgement poll. It sees
voted == falseand queues no timer. - The SET times out; the glue clears the request.
applied()records a failure andretry_at, butnext()sees no users and no vote and returns 0. Nothing is pending. - A new sole operation begins before
retry_at. The newagx_vote_begin(v, now)returns false, so the glue queues nothing. - At the retry deadline and ten seconds into that operation, the host harness still has
users=1, voted=0, pending=0, althoughdecide()returns SET. No caller runs it.
This reproduces with the new operation starting either 500 ms or 1 ms before the deadline, and across clock wrap. The same controls recover at e8ddbd9f: begin kicks the worker, which sees backoff and arms the remaining delay. At the new head, starting exactly at the deadline or retaining an active operation through the first failure works. A later begin after the deadline also recovers. Case 23 covers the later begin, not the missing-timer variant. Idle release/resume can likewise cancel the existing retry timer while retaining the failure state.
Please ensure a begin during backoff arranges the future retry when no timer exists, and add this end-during-failed-SET / begin-before-deadline regression case, including timer cancellation on idle release/resume. The effect is a lost performance vote for the active job. This is a host fault-injection result, not an observed hardware failure: the PMP acknowledgement is made to exceed its 50 ms poll limit for the first SET and succeeds thereafter. Two independent harnesses reproduce the sequence; I reran both and checked the production control flow.
A separate nonblocking boundary remains in agx_vote_retry_reached(): at exactly now = retry_at + 0x80000000U, both signed differences are INT_MIN, so it reports not reached and can return a 0x80000000 delay. The adjacent millisecond controls expire correctly. Comparing the future distance as unsigned would avoid that exact half-range hole; it should have its own boundary assertion.
The original change-request reasons are closed. Changes are still requested for the new missing-retry-timer regression. Full build and QEMU CI are green; no current-head Apple hardware boot or benchmark was performed by us.
Host-only fix control: the idle-rearm plus resume approach described in Scott’s review closes the failed-SET, idle release/resume and idle parameter-write timer-loss cases in our harnesses. It preserves the existing 23-case suite, all nine mutation checks and the old idle-clear controls. Both the idle-backoff rearm and the resume kick are needed; the resume kick alone still leaves reconciliation with no timer to arm. This is validation of a local candidate, not qualification of a pushed fix or hardware. The new in-tree regression should include end during a failed SET, a begin inside the remaining backoff, and cancellation/replacement of the idle retry timer.
…ommit) chriskearney's second blocker at efff25c: the last operation ends during a SET ack that then times out. applied records the failure with no user and no timer (next returns 0 on that path), and a begin before the retry deadline must arm the remaining delay instead of reporting no kick, or the retry never runs. The case also walks the release/resume cycle: an idle release does not write a clear, an idle resume does not kick, and a begin past the deadline still kicks. Red here: the begin result is 0 where the remaining 998 ms delay is required (FAIL line 397). Signed-off-by: Joshua Warren <816217+joshuaswarren@users.noreply.github.com>
…e path could not chriskearney's second blocker at efff25c: when the last operation ends during a SET ack that then times out, applied records the failure with users already 0, so next() returns 0 on that path and no timer is armed. A begin before the retry deadline reported "no kick" (the clock gate from 2ec389c), so nothing ever ran the retry: the vote stayed off until some unrelated reconcile. agx_vote_begin() now returns the same delay contract as agx_vote_end(): 1 run the reconcile now, N arm it N ms out, 0 nothing pending. A begin inside a failed-set backoff arms the remaining delay; past the deadline it returns 1 as before; a voted overlap, stopped or disabled still return 0. The glue passes the delay to the kicker instead of a boolean, and an idle release/resume cycle does not clear the retry state, so the next begin still arms it. Case 24 (fb8cba3, red at line 397) walks the whole sequence including the release/resume variant. Controls re-pointed at the new return: begin-no-retry- arm (returns 0 where the delay belongs) and begin-backoff-kick (returns 1, the kick storm) both die; begin-ignores-retry is gone because the delay branch subsumes it. Proof: plain suite 24 cases selfcheck ok; --mutations catches all nine mutants; the tree runner passes the registered entries. Signed-off-by: Joshua Warren <816217+joshuaswarren@users.noreply.github.com>
The residual w7X flagged on the delay contract: the end lands inside a failed SET write and no new operation starts. The write path arms nothing (users is already 0 when applied runs), so failures lingers with no timer, and a begin before the deadline reports "no kick". Case 25 walks it: the retry must be armed from the failure state itself, and when the retry fires with nobody waiting the backoff must be cleared once instead of re-arming forever. The header-alone asserts pin next(): 500 at 500 ms before the deadline, 1 at it. Red here: FAIL line 424, s.armed is false after the race. Signed-off-by: Joshua Warren <816217+joshuaswarren@users.noreply.github.com>
…the operation The residual from the delay-contract review: when the last operation ends inside a failed SET write, applied records the failure with users already 0, and next() returned 0 on that path, so no timer existed and a begin before the deadline reported "no kick" (2ec389c's clock gate) - the retry never ran. next() now arms the retry from the failure state itself (users 0, not voted, failures set): the remaining delay, or 1 when the deadline is already reached. That alone would re-arm forever once the retry fired idle, so the work function also clears a spent backoff: a reconcile that finds nobody waiting and an expired deadline (decide returned NONE) sets failures to 0, and the state settles. A later operation then starts a fresh attempt, exactly like the first. Case 25 (4574c14, red at s.armed) walks the sequence through the release/resume cycle; the retry-dropped-when-idle mutant removes the new branch and dies there. The sim's begin helper now arms the returned delay (delay 1 is "due now" within a tick), case 23's spent-backoff assert and case 24's next() assert updated to the new contract. Plain suite 25 cases, ten mutants, all caught; the tree runner passes the registered entries. Signed-off-by: Joshua Warren <816217+joshuaswarren@users.noreply.github.com>
|
Both rounds of your review are addressed. Head is The in-tree suite you asked Scott for is in ( Your findings, each with its case:
One intermediate gate on begin was itself reverted. Review found it stranded the vote; the history shows both rounds. On the benchmark: the +25 to 29% decode figures in the description came from the earlier lab-kernel form of this branch. The current source, including your two races and the clock fix, has not been re-measured. It is being built into the next lab kernel, and the same A/B re-runs on it. Those numbers will replace the description's. The earlier stack, for reproducing: a 16-inch M1 Max; lab kernel |
Treat retry deadlines beyond the backoff window as expired using unsigned distance. Cover the exact half-range boundary and keep the mutation control aligned. Signed-off-by: Joshua Warren <816217+joshuaswarren@users.noreply.github.com>
|
Note 🤖 GPT-6.1 Terra responding on behalf of Joshua Warren The retry rearm change is on the branch. Commit |
|
[Joshua's agent] Chris, this answers your 14:31Z request: the pinned kernel and config, the harness, and the commands. The next comments hold the harness files (2 and 3) and the config (4 and 5). Two corrections to the PR come first. Corrections
Pinned kernel (both data sets below ran on this image)
The patch ids are the first 8 or 12 hex digits of each patch file's sha256. The patch bodies are not inline. Say so if you want them attached. The config has Pinned user space. mlx-omarchy wheel v0.7.36 ( Data set A: the numbers in the PR description. Hold 250 ms (set at run time). Script
Only 2 T processes per decode row and 3 C processes. The 9B pair in the description is 33.81 to 42.82 (+26.7%) at 512 and 29.01 to 37.55 (+29.4%) at 8192, from the same script. Data set B: a later matrix on the same image at the 100 ms compiled default. Script
The 9B 8192 decode has a 111.5 ms token interval, longer than a 100 ms hold. The 9B 8192 vote-on number can be too low for that reason. Commands. # preflight (both data sets)
test "$(uname -r)" = "7.1.12-2-20261103.2-lab-ARCH"
test "$(cat /sys/module/pmp_report/parameters/agx_vote)" = N
cat /sys/module/pmp_report/parameters/agx_vote_idle_ms # 100 on this image
# data set A: hold 250 ms for the run, then restore
echo 250 | sudo tee /sys/module/pmp_report/parameters/agx_vote_idle_ms
python3 decode_b.py --model 2b --out-dir results/decode-b-2b # then --model 9b
echo 100 | sudo tee /sys/module/pmp_report/parameters/agx_vote_idle_ms
# data set B, one cell (2B, 512): C T C T C
for s in "C1 0" "T1 1" "C2 0" "T2 1" "C3 0"; do set -- $s; l=$1; v=$2
python3 agx_vote_wrap.py --vote $v -- \
env HF_HUB_OFFLINE=1 TRANSFORMERS_OFFLINE=1 \
venv/bin/python matrix_cell.py cell --python venv/bin/python \
--model models/2b --size 512 --decode-tokens 64 --processes 1 \
--out-dir results/2b-512-$l --deadline 360
doneBoth wrappers need Not covered. No t6000 run (Scott's question stands). No run at the current PR head. The read-roof and power columns in data set A need |
|
[Joshua's agent] Part 2 of 5: the harness for data set A (
|
|
[Joshua's agent] Part 3 of 5: the harness for data set B. The wrapper is the full file.
|
|
[Joshua's agent] Part 4 of 5: kernel config of base64 -d cfg.b64 | gunzip > config
sha256sum config # 443233d9b3d34544afc2b119809dc0a44c3c24c78f7e36f7f7b174b50c239fd5base64, lines 1 to 473 of 947 |
|
[Joshua's agent] Part 5 of 5: kernel config of base64 -d cfg.b64 | gunzip > config
sha256sum config # 443233d9b3d34544afc2b119809dc0a44c3c24c78f7e36f7f7b174b50c239fd5base64, lines 474 to 947 of 947 |
|
[Joshua's agent] Chris, the changes you requested at I re-ran the current-head host suite with The current PR CI checks are green. No new Apple hardware run was performed for these checks. Could you re-review the current head when you have a chance? |
|
Fixed at Your repro, walked against the pushed header. I cannot reproduce the stuck state at the new head. With Your conclusion holds, and the fix takes it. The begin-during-backoff delay return was the only way Red-first regression, in the tree. Case 26 (with the pre-fix header checked out at the parent commit, the case fails the same way your harness does: The half-range boundary fix ( State of the evidence: host fault injection and the header walk only, on the same terms as your re-review - no Apple hardware was rebooted or benchmarked at this head by me either. Full build and QEMU CI are on this push (same tree as |
chriskearney's re-review at c3487a5 reproduced a stranded retry: with the timer already armed by the write path, a begin before the deadline returned the remaining delay, and a caller that queue_works the delay pushes the deadline out. mod_delayed_work only pulls a deadline earlier, so the pushed timer would fire after retry_at and reconcile a decide that returns NONE. begin() now returns the two-value contract only: 1 when the deadline has passed, 0 otherwise. The retry arming belongs to next() alone, which already arms from the failure state (85a68bf) and is monotone under the same queueing rule. A begin during the window cannot strand the vote and cannot delay it. Case 26 walks the repro end to end (end during a failed SET ack, begin before the deadline, vote lands inside the new operation) and pins begin at the deadline edge and 1 ms before it. Case 22 gains the monotone-deadline assert. The begin-no-retry-arm control now reads the real return, and the new begin-drops-users mutant (drop the users++ behind the stopped check) dies at the base case. The tree runner registers the mutant entry. Proof: plain suite 26 cases selfcheck ok; --mutations catches all ten mutants; host-tests.py runs the vote entries green (base, --mutations and the new mutant entry). The whole tools group was not run here: it exceeds the 20 minute budget on this host and is CI's job. Signed-off-by: Joshua Warren <816217+joshuaswarren@users.noreply.github.com>
3d114c7 to
bfb4186
Compare
|
Correction to the head: the same fix is now |
The device trees job on bfb4186 died in "Install dtschema and yamllint": apt-get update stalled on the azure mirror for about 45 minutes and the job was cancelled at its timeout, before any dts check ran. That is a runner stall, not a code failure, and rerunning the failed job needs admin rights on omacom, so this empty commit retriggers the run. No code change. Signed-off-by: Joshua Warren <816217+joshuaswarren@users.noreply.github.com>
ed4a653 to
b36d019
Compare
|
CI state at |
GPU reads on the M1 Max (t6001) top out at 241 GB/s on Linux. The same chip reads 361 GB/s on macOS. One bit in the PMP report closes that gap: with it set, the same read microbenchmark reaches 368 GB/s, above the macOS figure.
The bit. AGX is soc-device record 9 on t6001. Bit 9 of the report's PS-REQ word is the AGX power request. With it clear, the PMP ignores the GPU's bandwidth demand and keeps memory at its idle floor. This is the t6030 mechanism from
9bb74767dcd4, on the t600x report layout.What was measured. A test module sets and clears bit 9 by hand. It uses the same register sequence as
apple_pmp_report_set_state(): read+0xf80, write+0x107c0, acknowledge+0x1000, status+0x10. On a 16-inch M1 Max it moved a GPU read microbenchmark from 241 to 368 GB/s. The same microbenchmark reads 361 GB/s on macOS. A dependent FMA kernel runs at 4.22 TFLOPS on Linux and 4.28 on macOS. The shader clock is not what differs. The gap is memory.Decode effect, measured with this patch. A 16-inch M1 Max booted the same code in its lab-kernel form (the 7.1.12 tree this branch was first written against), with
pmp_report.agx_vote0 against 1 and a 250 ms hold set at run time (the image's compiled default was 100 ms). Each arm is a fresh process, alternating off and on. A 2B 4-bit model decodes 93.88 to 117.54 tok/s (+25.2%) at 512 tokens of context and 81.94 to 105.22 tok/s (+28.4%) at 8192. The 8192-token prefill takes 6.30 s instead of 6.78 s (7.5% faster). The read microbenchmark goes from 242 to 249 GB/s up to 368 GB/s. The spread inside an arm is under 1% for decode. Holding bit 9 by hand for the whole process gave the same decode rates (117.59 and 105.21 tok/s), so the vote and its hold cost nothing in 2B decode. The longest gap between two tokens was 65.8 ms.The same A/B on a 9B 4-bit model: decode 33.81 to 42.82 tok/s (+26.7%) at 512 tokens and 29.01 to 37.55 tok/s (+29.4%) at 8192. The 8192-token prefill takes 26.5 s instead of 29.5 s (10.3% faster), and the read microbenchmark reaches 366 GB/s. The vote again matches the hand-held bit (42.82 against 42.92 and 37.55 against 37.61 tok/s). The longest gap between two tokens was 118 ms, once per run at a fixed token index, and it also appears in the runs with the vote off. A 100 ms hold would have dropped the request inside that gap. The 250 ms default covers it, and no run with the vote on shows a longer token interval.
Writes too. The vote lifts the write roof as well. On the same machine and kernel, a write bandwidth microbenchmark reads 193.1 GB/s with the vote off (two cells). With the vote on it reads 343.2 GB/s, which is 77.7 percent more. macOS writes 384.9 GB/s on that chip, so the remaining gap is 12 percent, not a factor of two. The read roof measured again in the same session was 247.8 GB/s off and 369.2 on, against 362.0 on macOS.
Design. The GPU driver reports operations in flight. It calls
apple_pmp_agx_activity_begin()when a submission starts andapple_pmp_agx_activity_end()when it finishes.pmp-reportsets bit 9 when the first operation starts. It clears the bit 250 ms after the last one ends. A decode loop with a submission every 10 ms keeps one vote. The bit is also cleared at once at GPU unbind and when the report goes away. The request is never left set past the GPU's power-domain release, which matches the t6030 fix. The first operation comes after firmware start. The t6030 note says an earlier report is acknowledged without scaling.pmp_report.agx_vote=1turns it on.pmp_report.agx_vote_idle_ms(default 250, writable at run time) sets the hold. With it off nothing is written, so one boot can run an A/B.apple,t6001only. t6000 is untested.unsafecall that takes no arguments, instart_op(), inend_op()and inunbind. The begin call sits before the guard exists, so a failed kick still pairs with the guard's drop.pmp-agx-vote.hwith no kernel dependency. A userspace test runs thirteen cases with a fake clock and a fake PMP bit under ASan and UBSan. One case is the 2^32 ms wrap ofjiffies_to_msecs(). A reviewer showed that a 64-bit compare never decided the idle clear across that wrap, so the clock is 32-bit.The branch has two more commits on top of the vote.
apple_pmp_report_set_request()is a locked, acknowledged update of one PS-REQ bit for clients outside this driver, such as the ANE driver, so they stop writing the word themselves. The next commit makes it refuse the AGX bit on t600x with-EBUSY, so the vote stays the only writer of bit 9, and documents that a caller clears its request after-ETIMEDOUT.Checked so far:
integration/aurorawith the lab kernel config, clang 22 and rustc 1.93.1, at each commit.pmp-report.odefinesapple_pmp_agx_activity_begin,_end,_releaseandapple_pmp_report_set_request. The GPU crate references the three activity calls, and the generated Rust bindings declare them. No warning comes from the new code.Not covered: