Repository navigation
Conversation
aurora-wip no longer builds with -Dwerror=true and the tests enabled (GCC 16.1.1). None of the fixes changes behaviour: - agx3_mesh_core.c: EMIT() put "n += size" on the line of the return its if guards (-Wmisleading-indentation); give it its own line. - agx3_cmdstream.h: cast the narrowed shifts in the designated initializers, as the neighbouring fields already do (-Wnarrowing when the header is included from C++). - hk_cmd_draw.c: hk_m5_pass_merge_begin() set a slot name it never read and fetched a device it never used. - hk_acceleration_structure.c: discard the popped page explicitly. - hk_cpu_upload_bench.c: push[] is a byte array; store the byte that the 0x12345678 constant was truncated to anyway. - compiler and lib tests: brace the gtest assertions that sit alone under an if (-Wdangling-else from the macros' internal else). Signed-off-by: Jonathan Goyvaerts <jonathan.goyvaerts@gmail.com> Assisted-by: LLM (cherry picked from commit 141481c) Signed-off-by: Eryk Wieliczko <eryk@wieliczko.ninja>
Restore conditional rendering, unified image layouts, memory-budget reporting and the sixteen-set limit qualified on M3 Pro. G15S uses the native AGX3 path; other G15 parts retain their conservative profile pending separate hardware qualification. FMA/rounding gates are unchanged. Check extensions, feature bits and the set limit through the real drm-shim driver. The new check fails on the previous G15S profile. Fixes: a5f56d8 ("hk: preserve the G13-G15 capability profile and isolate G17P FMA") Generated-by: LLM Signed-off-by: Eryk Wieliczko <eryk@wieliczko.ninja>
[Medium impact] The PR appears safe to merge; no actionable defect was found in the changes. SummaryRestores three Vulkan extensions and sixteen descriptor sets on M3 Pro’s G15S profile while preserving the other profiles.
Reviews (2) · Last reviewed commit: "Record M3 Pro integration tests and sour..." · Reviewed by Greptile |
Pin the packaged and rebooted source, targeted CTS results and library hashes. Record the existing CTS fixes and remaining native-path candidates. Full CTS remains a separate hardware run. Generated-by: LLM Signed-off-by: Eryk Wieliczko <eryk@wieliczko.ninja>
19df9e2 to
7c9db52
Compare
chriskearney
left a comment
There was a problem hiding this comment.
Reviewed 7c9db5205daa5189cf6e767ccd95f54ff8ea8cf1 against 7c86130d26cecd50c7af1ac01ff3fa1705d58045; production code is eaf69705c33249bd5e4f4ac2211bdcc6fad94f63 with the final qualification documentation included.
The changed G15S profile predicate and explicit Clang narrowing/unused-value cleanup are bounded, and current CI 40313934825 is green, including aarch64 and x86_64 builds. Our independent combined-stack benchmarks are positive: M3 Pro J516S Quake 186.5 → 658.7 FPS, M1 MacBook Pro J293 506.9 → 507.4, and M1 Air J313 509.9 → 503.0. The full four-case table and methodology are in our omacom/linux-aurora#238 review. Kernel #233/#238 and Mesa changed together, so these are not Mesa-only gains.
I am withholding approval of the capability exposure pending functional qualification. The PR itself retains an unresolved first-attempt uaddcarry.uint_highp_geometry wrong result that passed retry, plus all 384 conditional-rendering draw cases and 60 clear-attachment cases skipped. A retry does not erase the original wrong result, and a full run that skips those paths cannot validate them. I am not attributing that failure to this PR; similar dropped draws are documented before it.
Our own focused CTS attempt does not close this gap: after an initial executable-permission failure, eight cases reported Pass but the mapped-driver attribution gate failed, leaving zero accepted attributed cases and 444 unrun. This is unqualified harness evidence, not a functional failure assigned to this Mesa change.
Before approval, the newly exposed G15S functionality needs actual, non-skipped functional coverage, and the recorded wrong-draw evidence needs resolution or a sufficiently bounded explanation. The existing successful descriptor-set readbacks are useful evidence and should remain separate from the skipped predicate/clear paths. No full-conformance or whole-system qualification is claimed from our native GL/VK and performance checks.
|
Clarification to my review: in the pinned CTS Those groups can therefore be blocked by a separate, unadvertised prerequisite. Their SKIPs are not evidence that core conditional rendering failed, and I am not requiring this PR to add nested-command-buffer support. The actual NotSupported messages are still needed before attributing the recorded runtime skips to that prerequisite. The existing non-nested |
chriskearney
left a comment
There was a problem hiding this comment.
Approved at 7c9db5205daa5189cf6e767ccd95f54ff8ea8cf1 following the targeted functional checks below. These close the coverage gap behind my earlier comment; the current aarch64/x86_64 CI is also green.
We independently checked the raw results on M3 Pro J516S with the combined kernel #233/#238 head 8824dd6a and this Mesa head, using pinned Vulkan CTS v1.4.6.1 (5c8aae22885448d70a2873e94a93b24b49505c32). All 978 exact, unique cases match their raw QPA/log records. Each of the ten lists has an observed, hash-matched native libvulkan_asahi.so mapping and the expected M3 Pro device, with no new GPU fault entries and all processes drained.
457 attributed Pass, 0 Fail, 0 Missing, 521 NotSupported. The passes include:
| Check | Pass |
|---|---|
| Non-nested conditional draw/clear | 108 |
| Conditional dispatch | 138 |
| Correct handling of commands unaffected by conditional rendering | 100 |
| Unified-image-layout GENERAL feedback-loop readbacks | 10 |
| Sixteen descriptor-set GPU readbacks | 64 |
| Capability/memory-budget queries | 7 |
| Other GENERAL-layout correctness checks | 22 |
| Pinned eight-case geometry regression group | 8 |
Device queries report conditionalRendering = 1, inheritedConditionalRendering = 1, unifiedImageLayouts = 1, and maxBoundDescriptorSets = 16. The unified-layout tests actually exercise the feature-specific GENERAL feedback-loop path; the ordinary GENERAL tests remain separate evidence. Memory-budget coverage is query sanity, not allocation-pressure testing.
The 521 skips have explicit reasons: 444 require the separate, unadvertised nested-command-buffer extension; 64 require an exclusive compute queue; six require ray-tracing pipelines; five require ASTC decode mode; two exceed maxColorAttachments. Those are not counted as passes or demands for additional features. This confirms the prerequisite explanation for the earlier 384/60 skipped groups.
All eight cases in our pinned superset of the previously reported geometry failure passed on their first attempt here. The author's original retry-passing wrong-draw evidence remains a known issue; this new sample does not erase it or establish its cause. No failed case was retried into a pass in this run.
This supports approval of this scoped G15S profile/build change. It is a targeted sample, not full Vulkan conformance, exhaustive stability/efficiency testing, or approval of kernel #233's still-failing host checks. The earlier combined-stack performance table and OMT/HID limitations remain as reported. The original M3 system was restored and independently verified after the test.
|
Landing-order clarification for my approval at The reviewed #233/#238 changes introduce no new userspace wire ABI, and Mesa17 leaves native-device ABI admission unchanged. I found no hard new-ABI requirement forcing the source PRs to merge in a particular order. However, that admission check does not establish that a kernel contains the stamp/ASID lifetime, locking and PMP fixes. An unchanged ABI is not proof that an untested pairing is safe. My release recommendation is to keep Mesa17 with the tested kernel cohort, after the kernel CI/review issues are resolved. The Mesa source can land first if package rollout remains held to that cohort. If it is to ship against an existing kernel instead, qualify that exact kernel/Mesa pair with the bounded native feature/output and return checks first. This is not a request for full CTS or an all-kernels matrix. The approval remains scoped to this Mesa change and the evidence above; it does not approve the outstanding kernel PRs or certify Mesa-only deployment on older kernels. |
|
@maralcbr @erykwieliczko the matching kernel carry linux-aurora#253 has now landed as Chris requested the normal merge. Our |
maralcbr
left a comment
There was a problem hiding this comment.
Approving at 7c9db520. Thanks @erykwieliczko.
- The profile change is G15S only:
hk_physical_device.c:92adds G15S to the conditional-rendering, unified-image-layouts and memory-budget gate, and:1069gives it 16 descriptor sets. The Air (G15G), Max (G15C), M1/M2 and Neo profiles are unchanged, and admission still limits G15S to T6030. - The Clang casts in
agx3_cmdstream.h:243,363,440,470keep the same values, since each shifted field already fits its width. Thehk_cmd_draw.c,hk_acceleration_structure.candagx3_mesh_core.cchanges only drop unused values and adjust whitespace. - Chris's attributed CTS on J516S (457 pass, 0 fail, with the new features actually exercised) and green CI cover it. The earlier retry-passing
uaddcarrywrong draw is still a known issue, and it predates this PR.
Merging the source now. The package rollout stays paired with the kernel carrying linux-aurora#253 (213f6272). This isn't qualified on older kernels.
Bring the aurora-wip sync up to integration/aurora 7f0206e (#17, M3 Pro integration validation). One conflict, in src/asahi/vulkan/hk_physical_device.c, where both sides widened the native_agx3 extension profile: integration/aurora restores the extended profile on G15S, and aurora-wip keeps it on M5. The two are independent, so the gate takes both, with the comments merged. Signed-off-by: DjDeveloperr <hey@djdev.me>
Also port Jonathan Goyvaerts's public Clang/warnings build fix, preserving
his authorship and source commit. Clang previously rejected four narrowing
conversions in the command-stream header when building the enabled tests.
The recent packed-GS-output and select-immediate fixes are already present
in this base as
d4b6840fandae035d59.Mesa integration base:
7c86130d26cecd50c7af1ac01ff3fa1705d58045.Built source:
eaf69705c33249bd5e4f4ac2211bdcc6fad94f63, packaged withassertions and rebooted on J514S. The final commit adds qualification docs.
139 host suites and the actual-driver shim regressions pass; the new G15S
check fails on the previous profile. The tracked hardware receipt records
the initial qualification on kernel
m3plasma-16575aa.The latest combined stack is now booted with kernel
7.2.11-m3plasma-8824dd6-ARCH: current integration937b727eplusomacom/linux-aurora#233 and
omacom/linux-aurora#238.
On that stack, focused CTS has 561 pass / 4 warnings / 5 skips; the broader
sample has 7,054 pass / 5,198 skips; capability checks 359 pass / 1,670 skips;
and the graphics/compute 16-set group 2,694 pass / 4,320 skips. GPU readbacks
at sets 0, 7, 8 and 15, all 50 GEM lifetime cases, ten Settings launches and
ten theme cycles pass, without GPU recoveries or KWin/Settings crashes.
The seven-case regression group has six passes and one wrong-result flake
that passed the automatic retry:
uaddcarry.uint_highp_geometry. Outputs remained at the clear colour. The earlier kernel also had oneretry-passing failure, in
uvec3_highp_geometry; similar dropped drawspredate this PR. The cause is unresolved; this is not a clean pass.
All 384 conditional-rendering draw cases and all 60 clear-attachment cases
skip. These paths remain unqualified; a full run that skips them will not
close that validation gap.
The owner is running full Vulkan 1.4 CTS on the new stack; qualification is
pending that result. Quake measurements overlapping CTS are excluded.