Repository navigation
drm/asahi: g17: Name the cause and the clients of every M5 GPU recovery - #1242
rolotrealanis98 wants to merge 2 commits into
Conversation
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
b1615e3 to
0dd0833
Compare
RyanTheTide
left a comment
There was a problem hiding this comment.
- This branch carries all of #1173, so it needs that PR's changes first (rebase onto the current
funkzy/t6050-gpu-on-integration, and the removal of the kernel-extension and firmware code references ing17/fw/m5/initdata.rs). After that, push only c8c0484 on top of the updated #1173 branch. - c8c0484 conflicts with the current base in
drivers/gpu/drm/asahi/g17/recovery.rs, against 4d794a4 ("Fix M5 GPU recovery failing or completing the wrong job"), 857d37d ("Fix the M5 recovery log claiming listed jobs fail") and a12682e ("keep the firmware's blame for a GPU MMU fault"). Please resolve it on the rebase. macos_restart_reason()inrecovery/attribution.rsis a mapping taken from a function in the macOS kernel extension (processFirmwareInitiatedRecovery). Please remove it and the "macOS restart reason" field from the log. The firmware reason andcause()are enough for the log.- The failing
sourcejob (test_aurora_sep_sums_match_tree) comes from the old base, not from this diff. It should clear once the branch is rebased.
c8c0484 to
e2192e0
Compare
|
Updated (e2192e0, one commit on the new #1173 head d8ca53c).
Of the recovery record, only the reason word has been read on hardware (5, three times); the comments mark the other words as unconfirmed. Builds with W=1 and CLIPPY=1 with the base's warning set; also compiled with |
490c980 to
cad9793
Compare
|
Updated (9d1b9f2, cad9793): the two commits are now directly on the current The base's own per-build layout ( Still true: of the recovery record only the reason word has been read on hardware (5, three times, J716s); the other words are marked unconfirmed, and nothing has been read on a J813. Build: |
…the pause timeout parameter, recovery cause logging (J716s daily) The 26B123 and 26C5035 G17S builds with their InitData layouts in the per-build graph (status config and power policy maps, the 27.3 HW data bands), the J716S board selected by the machine compatible, aurora.m5_pause_timeout_us, and the recovery cause log of aurora-silicon#1242. The 26A428 graph is asserted unchanged. Private to the J716s daily: the layout values are not from traces. Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BV6NpkesyT31Ta49VjQELY
RyanTheTide
left a comment
There was a problem hiding this comment.
@rolotrealanis98 thanks for moving this off #1173. Two asks:
- The PR description still has the firmware TEXT address range, the "DSF Pause Timeout" name, the restart-reason mapping and a log sample with "macOS restart reason". The code no longer has them; please update the description to match the commits.
- Please drop m5_recovery_reason_blame. The record is useful as a log line now; the blame path can come with the evidence that the M5 reasons mean what G17P's do.
After that we will build it and run a recovery on our A18 Pro, since report_clients() runs on every G17 recovery.
On M5 every firmware recovery logs "reason None slot_mask ... sampled 0 known_keys 0": the driver reads only the slot mask of the firmware's recovery record, so a recovery never says why the firmware stopped or whose work was running. With several GPU clients that leaves nothing to act on. The rest of that record is in the primary status B: the reason at +0x4aac, the number of busy slots walked at +0x4aa0, a word describing the slot the record names at +0x4aa8 and a u64 detail at +0x4ab0 (a page fault leaves its latched MMU fault status there). Only the reason has been read from a running firmware so far: on a J716s with the 27.3 beta 1 firmware, all three recoveries of a ten-minute mixed client load carried reason 5, a scheduling pause that did not complete in time, and the firmware blamed no queue. The other words, and the meaning of reasons 3 (page fault) and 6 (forced idle after a sleep request), are not yet confirmed from captured InitData. Read the record with the slot mask and log it next to the snapshot, with the reason's meaning and the queue the firmware's restart event blames. After the handshake, name the client (process name and tgid) and the retained command count of every queue the recovery table lists and of the blamed queue. The record changes no blame: attribution on M5 keeps its unknown reason. Letting the firmware reason steer blame, as on G17P, waits for evidence that the M5 reasons mean what G17P's do. Signed-off-by: Rolando Trevino <rolotrealanis@gmail.com> Assisted-by: LLM Claude
On a J716s, M5 GPU clients trigger firmware recoveries with no MMU fault. Under a ten-minute mixed client load with the HW data word at +0x271c at its encoded value, 150 (the inherited fixed word of the 26A428 HW data, moved by the M5 insert), the GPU recovered three times, and all three recoveries carried firmware reason 5, a scheduling pause that timed out. With 2000 written to the word, the same load produced no recovery in ten minutes. One run each, so the word is shown to bound the pause but 2000 is not shown to be a better default. Name the word (M5_HW_PAUSE_TIMEOUT_US), describe it by that measurement, and hold the offset to the encoded fixed word with a compile-time check. Nothing else changes: the board value stays 150. Signed-off-by: Rolando Trevino <rolotrealanis@gmail.com> Assisted-by: LLM Claude
cad9793 to
4805b18
Compare
|
Updated (27f397b, 4805b18), still directly on
The second commit is unchanged. This revision has not been rebuilt yet (the change only removes the parameter and its branch and rewords two comments) and has not run on hardware. Thanks for running it on the A18 Pro. |
What this is
Two commits directly on
integration/aurora(base12f3c1bf4291); no dependency on #1173. Diagnostics only: every M5 GPU recovery now logs why the firmware stopped and whose work was running. Blame is unchanged.On M5 the recovery log said
reason None slot_mask ... sampled 0 known_keys 0for every recovery: the driver read only the slot mask of the firmware's recovery record. The rest of that record is in the primary status B, next to the slot mask: the reason at +0x4aac, the number of busy slots walked at +0x4aa0, a word describing the slot the record names at +0x4aa8, and a u64 detail at +0x4ab0 (a page fault leaves its latched MMU fault status there). Reasons named in the log: 3 a GPU page fault (as on G17P), 5 a scheduling pause that did not complete in time, 6 a forced idle that timed out after a sleep request. Only the reason word has been read on hardware (5, three times); the other words, and reasons 3 and 6 on M5, are marked unconfirmed in the code.The first commit reads the record with the slot mask, logs it next to the snapshot with the reason's meaning (
cause()), and after the handshake names the client (comm[tgid]) and the retained command count of every queue the recovery table lists and of the blamed queue:The record does not steer blame: M5 attribution keeps its unknown reason. Letting the reason steer blame, as on G17P, can follow once there is evidence that the M5 reasons mean what G17P's do.
The second commit names the HW data word that bounds a scheduling pause (
M5_HW_PAUSE_TIMEOUT_US, the M5 Pro's offset 0x271c) and holds its offset to the encoded fixed word with a compile-time check. The board value stays 150.Why
Without a reason there was nothing to act on when ordinary GPU clients (a Qt Quick bar, Chromium, a compositor) faulted the M5 Pro (J716s) several times an hour. Tracker: rolotrealanis98/m4-m5-linux#36
Hardware result (J716s, G17S, firmware 27.3 beta 1)
From an earlier revision of the first commit (same record read and log, on the #1173-based J716s tree), 10 min of multi-client load (RESULTS.md 2026-11-08 12:08 UTC):
integration/aurora, without drm/asahi: M5: InitData for the macOS 27.2 and 27.3 firmware (G17S bring-up on J716s) #1173) has not run on hardware, and nothing has run on a J813.Checks
drivers/gpu/drm/asahianddrivers/gpu/drm/applewithW=1 CLIPPY=1at the base and at both commits, 151 warnings each, none new; compiled withCONFIG_KUNIT=y, suites not run.m5_recovery_reason_blameparameter and its branch infault_sources(), and rewords two comments; not yet rebuilt.m5_record_names_the_cause_without_steering_blameis inattribution.rs; not run.Risks
pr_warnlines more per recovery, plus one per listed queue.report_clients()runs on every G17 recovery, not only M5.