Repository navigation
J713: G16 performance governor, and the PMP idle floor it needs - #455
erykwieliczko merged 2 commits into
Conversation
|
Hold for the performance evidence and the exact working T6040 source pin. The idle-power improvement reported here is useful, but the PR body needs a reproducible method and paired raw before/after runs with repeat count, duration, sample cadence and variance. The 620-643 offscreen build scores and 577 full score are not a paired run table; there is no frame-time or peak-temperature comparison. Keep this draft until those measurements are in the body. The chip scope at The PMP rail-validity change has no local chip check, but the entire driver's probe rejects machines other than For J616s, both A/B kernels must contain #453's same accepted firmware identity and the same working GPU bring-up commits. #453 changes exact firmware-identity admission, not the governor or backend routing. Compare the working baseline without either #455 commit against that identical source plus the PMP commit followed by the governor commit. Record idle GPU rail power, repeated glmark2 scores and frame pacing, and peak GPU/SoC temperature during the same fixed glmark2 loop, with identical display, power source, software, ambient/start temperature and thermal policy. Put the raw paired results and variance in this PR. A failed-probe baseline cannot establish a performance improvement. Resolve the routing mismatch first; unchanged g17 results would not exercise this new governor. The pure policy retains thermal/user ceilings after the S3 floor, including ceilings below S3. The added tests cover utilization, smoothing and demand thresholds; add coverage of production busy/drain accounting, recovery and the idle rearm path so those connections are checked too. The S3 energy argument is measured on J713 and cannot be assumed for G16S. A separate software-gate run for this head plus #453 is queued through the ordinary buildslot. The earlier J713 train passed both full WERROR builds and 275 KUnit tests, but that source does not contain #455 and is not evidence for this governor. I will update this same comment with the new run results. No J616s measurements were performed in this review. Merge only after the benchmark and routing gates, the shared-code regression boots required by the train, and the J616s A/B results. Keep #453 in both benchmark arms; within this PR, apply the PMP validity change before the governor. The existing J713 speaker-protection and suspend holds remain separate requirements. |
338a2ca to
1e25a18
Compare
|
Thanks. I've updated the body and pushed a new head.
|
The power work treats a reading of less than 0.1 W across the P-CPU, E-CPU and GPU rails as a firmware reset image: it reports -ENODATA, retains the minimum CPU and GPU ceilings and fails the hwmon rails with -EAGAIN. An idle J713 reads below that once its GPU idles at a low performance state (P-CPU 40-55 mW, E-CPU 9 mW, GPU 27 mW): the ceilings then sat at the minimum while idle, the CPUs ran at 900 MHz, and the GPU budget climbed back one state at a time once work arrived. A reset image reads zero on both CPU rails, which the running reader cannot. Reject only that. Signed-off-by: Erik Bourget <erik@ebourget.net>
The firmware runs at the state the three Globals selectors publish, busy or idle, and the host published the thermal ceiling: on the J713 the GPU sat at S9 (1470 MHz, 1000 mV) with nothing to draw. Its rail drew 1.06 W idle and 1.25 W for a 60 fps desktop, against 68-90 mW for the same desktop at S1-S3. Request a state from the GPU's busy share of each 20 ms window, between S3 and the thermal and user ceiling, which still bound it: the top state when a window was at least 70% busy, one step up at 40%, and down on a utilization averaged over about four windows, one step under 25% and to S3 under 10%. Busy time runs from publication to drain. An idle engine polls once more per window until it is back at S3. S3 draws no more than S1 on a desktop load and runs at 2.4 times its clock; S4 and up cost several times as much. The accounting is policy::Governor, which the host policy test runs directly: publication to drain, busy spans carried across windows, flights dropped by recovery without a drain, the idle re-arm back to S3, and thermal and user ceilings, including those below S3. Paired on the J713, on battery with the backlight at half (idle 5 x 60 s, three 60 Hz scenes 3 x 30 s each, the glmark2 suite 3 times): idle draw 7.32 -> 6.06 W, the GPU rail 1.09 W -> 27 mW. The 60 Hz build scene stays at S3 with no missed frame, and the glmark2 score is 574 -> 573. Signed-off-by: Erik Bourget <erik@ebourget.net> [eryk: don't re-arm the idle poll on a failed engine: its poll returns before service_power(), so the demand never decays, and an engine that failed while idle above S3 re-armed its poll every 20 ms until reboot once anything kicked it; the host test's call-site check follows] Signed-off-by: Eryk Wieliczko <eryk@wieliczko.ninja> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Merging with one fix. This comment is from Claude Opus 5.5, working for Eryk, not from Eryk himself. Rebased onto integration/aurora Changes
Testing
🤖 Written by Claude Opus 5.5 (in Claude Code) for Eryk. |
1e25a18 to
26fd983
Compare
The G16 firmware runs at the performance state the host publishes in the three Globals selectors, busy or idle. The host published the thermal ceiling, so on the J713 the GPU sat at S9 (1470 MHz, 1000 mV) with nothing to draw:
Idle with nothing redrawing, the rail still drew about 1.09 W at S9.
Scope: J713 / T8132 only
g16backend, which registers onlyapple,agx-t8132.apple,j713, and its rail reader selectsapple,t8132-smc.g17with its own power manager, so these commits do not run on it.g16backend later, the S3 floor and the utilization thresholds would need their own G16S measurements. The S3 energy argument below was measured on the J713 only.The PMP idle floor (patch 1)
apple-pmp-thermaltreats a reading of less than 0.1 W across the P-CPU, E-CPU and GPU rails as a firmware reset image. It then:-ENODATA;-EAGAIN.With the GPU idling properly, an idle J713 reads about 80 mW on those rails (P-CPU 40-55, E-CPU 9, GPU 27). As a result:
A reset image reads zero on both CPU rails, which the running reader can't, so that is now the only reading rejected. This patch has to land with or before the governor.
The governor (patch 2)
The host now requests a state from the GPU's busy share of each 20 ms window. Busy time runs from publication to drain. The thermal and user ceilings still bound the request, including ceilings below S3.
pollreturns before the governor, so the published state is left as it was (unchanged from before this series).The policy and its accounting are
policy::Governorinpower/policy.rs. The call sites are:engine.rs);poll.rs);power.rs);run_poll).The debugfs stats line gains
demandandutilization.Paired A/B on the J713
Arm B adds only the two commits of this PR to arm A, so these two commits are the only difference.
g16/andapple-pmp-thermal.cmatch this PR's head byte for byte.Both arms ran on the same machine:
Sampling:
asahi-g16/stats.LD_PRELOADshim timestamps the return of everyeglSwapBuffers(CLOCK_MONOTONIC). A missed frame at 60 Hz is an interval over 25 ms.Protocol:
build,shadowanddesktop:effect=blur, fullscreen with--swap-mode fifo. Each run follows a 30 s rest and a ≤ 53 °C gate, and the first 30 frames are skipped.Values are the mean ± sd of the per-run means, followed by the raw runs.
Idle desktop, nothing redrawing (5 × 60 s)
Vsync pacing, fullscreen fifo at 60 Hz (3 × 30 s per scene)
glmark2 default suite, fullscreen, unthrottled (3 runs per arm, 331 s each)
The published state during the loops was S9 in 994 of 994 samples for A and 993 of 994 for B.
How to read this
Idle: 1256 mW less battery draw (−17%), almost all of it GPU rail.
60 Hz pacing:
buildstays at S3, with the GPU rail down from 1277 to 106 mW.shadowandblurneed S9 in both arms.Unthrottled: the score is unchanged within the run-to-run spread. Two scenes are a little lower with B, and their runs do not overlap with A's:
ideas: 633-647 FPS vs 655-661 (−2.6%);build: about −1%.These are the burstiest scenes, which suggests brief step-downs inside the 20 ms windows that 1 s sampling does not resolve. All other scenes are within ±1%.
Temperatures: A loop run starts after a 120 s rest, once the die is at or below 53 °C. B idles cooler, so B's runs started about 5 °C lower. The lower B peaks are therefore not evidence of a cooler load. They show only that B is not hotter under the same load. Ambient was not measured, and the arms ran about 11 h apart.
Notes
ENODATAfrom two PMP temperature channels at similar counts: Ta000m 443 vs 446, Te000m 119 vs 111. This is unaffected by these commits.Testing
test-thermal-policy.py): 10 governor cases, run against the productionpolicy::Governor:checkpatch --strictare clean on both patches. The tree builds with no new warnings.