Repository navigation
Fix M3 Pro GPU startup - #202
Conversation
[Medium impact] The PR appears safe to merge; no actionable defects were found. SummaryThis PR fixes two barriers to M3 Pro GPU startup:
It also expands UAT regression coverage and adds checks for both PMP profiles on J514S and J516S. No actionable issues were found. Tests were inspected, not executed. Reviews (2) · Last reviewed commit: "drm/asahi: g15: match UAT reservations w..." · Reviewed by Greptile |
chriskearney
left a comment
There was a problem hiding this comment.
Thanks, Eryk. Reviewed 91c22e90236e against b6f070c91193; I didn’t find a source blocker.
The reservation lookup handles unit addresses without weakening the owner checks: duplicate names still refuse, including a disabled duplicate, and the exact range, availability and reusable checks remain. The child and parent references balance. Guarding all four PMP nodes also matches the existing alias and consumer guards.
Independent checks:
- G15 host suite: 74 Rust tests passed, 2 ignored; the C memory, UAT-owner and PMP-bridge checks passed.
- Your new UAT fixture fails against the base lookup. Seven deliberate faults in duplicate/name matching, reference handling and range checks all built and failed at runtime; controls passed before and after, with ASan/UBSan enabled.
- J514S/J516S DTs built with and without
APPLE_USE_PMP, using this tree’s dtc. Default builds omit all four PMP nodes; static builds retain them and are byte-identical to the base. J313, J613 and J516C comparison DTBs also remain byte-identical.
One non-blocking test suggestion: keep those default/static PMP node-presence assertions in CI. A successful DT compile alone would not catch the original runtime overlay collision.
Both commits still need your Signed-off-by trailer to clear ci / commits; compile, device-tree and both host-test jobs have now passed. I haven’t boot-tested this PR or run a full local kernel build. Review assisted by Codex.
91c22e9 to
a565165
Compare
The static PMP nodes block the display gate's overlay and leave G15 deferred on M3 Pro. Keeping either the static DART or mailbox would still conflict with the overlay. Guard all four nodes with APPLE_USE_PMP, matching the existing alias and consumer guards. Check the default and static PMP nodes in CI. Assisted-by: Codex:GPT-6 Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Eryk Wieliczko <eryk@wieliczko.ninja>
The DT uses names such as uat-ttbs@10, but of_find_node_by_path() requires an exact match and misses them. Use of_node_name_eq() on /reserved-memory children. Require one matching owner and keep the existing range and property checks. Extend the host test to cover unit addresses and duplicate owners. Assisted-by: Codex:GPT-6 Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Eryk Wieliczko <eryk@wieliczko.ninja>
a565165 to
8b0cbd5
Compare
|
Done. |
maralcbr
left a comment
There was a problem hiding this comment.
Thanks @erykwieliczko, both fixes are right and small. Looks right to me, no blockers.
- Static PMP nodes. On
b6f070c91193the four nodes att6030.dtsi:839-986are always present. The gate refuses on any node with the PMP or report compatible, disabled or not (t6030-display-gate.c:692-698). So every J516S, and every J514S whose boot loader passes PMP values, hit "already exists" and stayed on the boot framebuffer. Every reference to the guarded labels was already underAPPLE_USE_PMP(alias:22-24, AVD:731, ANS mailbox, SART and NVMe:1399,:1409,:1428). Neither board defines it, and the overlays carry their own four nodes, so nothing dangles and nothing collides. T8122 and T6031 reuse their static PMP and are untouched. - UAT owner lookup.
of_find_node_by_path()compares the full node name, so it could never matchuat-ttbs@10(t6030-gpu.dtsi:47-66).of_node_name_eq()with the unique-owner count is the right fix. The references balance, a missing/reserved-memoryis safe, and the range, availability,reusableand single-regchecks are unchanged. m1n1 fills the existing placeholders in place, so refusing duplicates won't reject real hardware.
What I ran: the G15 host suite passes locally (74 passed, 2 ignored, plus the C checks). The UAT fixture passes under ASan/UBSan and aborts at uat-owner.c:119 against the old lookup. t6030-pmp-dt-tests.py with the tree's own dtc passes all four profiles, and on the base tree it fails on the J514S default profile. check-commits.py shows both commits signed off. CI is green at 8b0cbd5, and the host tests (tools) log shows the new suite's setup and all four profiles passing. Since @chriskearney's review at 91c22e90236e the production code is unchanged: the sign-offs and his suggested DT assertions are in.
Optional, fine as a follow-up:
- Assert on what the gate checks.
t6030-pmp-dt-tests.py:12-17matches direct/socchildren by name. The gate searches the whole tree by compatible (apple,t6000-pmp-v2andapple,t6030-pmp-v2-report,t6030-display-gate.c:693-695). A static node added later under another name, for example the overlay's ownpmp-report@/mailbox@spellings, would pass the test and break the gate again. Could the default profile also assert that no node carries those two compatibles?
For release (not the merge): boot-critical for every M3 Pro, since default kernels now take the native display handoff and start the PMP. Before release I'd want cold boots of the final artifact on a J514S with loader PMP values and on a J516S, each with the native display up, a render node, and a GL/Vulkan readback. I'd also want one boot with apple_t6030_display.enable=0 to confirm the framebuffer fallback without the static nodes. Your readbacks were on be6381f9d2a5, so a run at the final head would close that. The merge queue's full run (KUnit, package, boot) still applies as usual.
|
|
||
| ROOT = Path(__file__).resolve().parents[2] | ||
| BOARDS = ("t6030-j514s", "t6030-j516s") | ||
| PMP_NODES = { |
There was a problem hiding this comment.
Optional: the gate refuses on the apple,t6000-pmp-v2 and apple,t6030-pmp-v2-report compatibles anywhere in the tree (t6030-display-gate.c:693-695), not on these names. Asserting that neither compatible is present in the default profile would catch a renamed static node too.
There was a problem hiding this comment.
Boot-tested 8b0cbd58bf95 on J516S/T6030 with the matching J516S DTB and unchanged loader executable.
Full Image/modules/DTBs build passed. The kernel booted in 30 seconds and native G15 started with the unit-addressed UAT reservations, exercising the new lookup. OpenGL and Vulkan pixel readbacks passed with zero wrong pixels. No kernel WARN/Oops/SError, DART faults, GPU faults, recoveries or hang kills were observed during the scoped checks.
The updated default/static PMP regression test passes too. No kernel/GPU blocker found in these two fixes. This was one boot and scoped graphics validation, rather than full hardware qualification. Review/testing assisted by Codex.
chriskearney
left a comment
There was a problem hiding this comment.
Reviewed 8b0cbd5. Full kernel build, host regression checks and a J516S boot with the matching DTB passed the scoped startup/graphics checks. The unit-addressed UAT reservations were exercised on hardware; GL/Vulkan readbacks passed with no GPU/DART faults observed. No blockers found in these two fixes. Review assisted by Codex.
|
Tested on the lab laptops as part of a combined hand build (also carrying 186, 183, 179). Posted here for 202. What was built
Boot regression check (the lab fleet has no T6030/T6031, so the boots prove the g15 change does not break the M1-family/M2-family build): Both candidates boot one-shot with 0 failed units, smoke suites equal to the machines' own stock baselines, no new dmesg error class. The g15 driver code is not used on T8103/T6021, so these boots say nothing about M3 Pro GPU startup behaviour - that path is exactly what was NOT tested (no hardware). Regressions: none found. The 16-inch M1 Max run is pending (its lab window was taken by other tests); I will add one line to this comment thread when it ran. |
|
Hand build of the integration/aurora tip 7b96998 (contains the merges of this PR and #199, #202, #203, #204). Package release 7.2.11-tip-ARCH+, package sha256 52830bb4538e8e8f5344eacff87fbdb9ab26cc553540c36e993ba4e097561c23. Tested so far (one-shot boot only, stock untouched):
CI on this PR: 9 pass, 4 skipped (attest, KUnit, boot, package). Regressions: none found. dmesg error classes on the candidate boot match the stock baseline (known flakes only: dp-xbar -517 deferrals, brcmfmac join_pref/assoc noise, Bluetooth codec-capabilities -110). m-test check list identical to stock except two tool-level deltas, neither a functional failure: (1) hardware.drivers counts Not tested: M3 paths (this PR's T8122-without-G15 build path is M3-family; the M3 Pro GPU startup in #202 and the J615 camera path in #203 compiled and booted in this build on M1/M2 but their M3-only code paths were not exercised — no M3 machine in this lab). |
The display gate refuses any existing PMP or report compatible anywhere in the tree, including disabled nodes. The DT control only checks four names under /soc, so renamed or nested nodes can pass it and still block the runtime PMP overlay. Walk the whole default-profile tree and reject either compatible in any entry of each compatible string list. Keep the existing static-profile node and alias checks. Compare case-insensitively, as the OF lookup does. Link: omacom#202 (comment) Signed-off-by: Chris Kearney <303316+chriskearney@users.noreply.github.com> Generated-by: LLM
<ai>
On my J514S M3 Pro,
integration/auroraatb6f070c91193boots without a GPU render node. The display gate logs:This fixes two startup issues:
APPLE_USE_PMP, matching the existing alias and consumer guards. They otherwise conflict with the display gate's runtime overlay.uat-ttbs@10withof_node_name_eq(). The exact-path lookup misses them. Keep the unique-owner and range checks, and add regression coverage.The UAT test passes with the fix and fails against the old lookup. CI also checks the PMP nodes and alias in J514S/J516S DTBs built with and without
APPLE_USE_PMP. The surrounding G15 and PMP host suites also pass.The same production fixes passed native GL/Vulkan readbacks and three PB reclaim cycles on
be6381f9d2a5, using native G15 Mesa132fff8452a4.</ai>