Skip to content

Restore M3 Pro Vulkan capabilities and fix the Clang test build - #17

Merged
maralcbr merged 3 commits into
omacom:integration/aurorafrom
erykwieliczko:fix/m3pro-integration-validation
Oct 28, 2026
Merged

maralcbr merged 3 commits into
omacom:integration/aurorafrom
erykwieliczko:fix/m3pro-integration-validation

Conversation

@erykwieliczko

@erykwieliczko erykwieliczko commented Oct 26, 2026 •

Copy link
Copy Markdown
Restore the G15S capability profile tested on M3 Pro: conditional rendering, unified image layouts, memory-budget reporting and 16 descriptor sets. The G13–G15 generation gate hid these on the native M3 Pro path. Other G15 profiles, FMA/rounding and device/kernel-ABI admission remain unchanged.

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 d4b6840f and ae035d59.

Mesa integration base: 7c86130d26cecd50c7af1ac01ff3fa1705d58045.
Built source: eaf69705c33249bd5e4f4ac2211bdcc6fad94f63, packaged with
assertions 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 integration 937b727e plus
omacom/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 one
retry-passing failure, in uvec3_highp_geometry; similar dropped draws
predate 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.

jgoyvaerts and others added 2 commits October 26, 2026 11:20
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>
@greptile-apps

greptile-apps Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge; no actionable defect was found in the changes.

Summary

Restores three Vulkan extensions and sixteen descriptor sets on M3 Pro’s G15S profile while preserving the other profiles.

  • Native G15S devices report three Vulkan extensions and 16 descriptor sets.
  • Clang builds the enabled tests without narrowing warnings.

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>
@erykwieliczko
erykwieliczko force-pushed the fix/m3pro-integration-validation branch from 19df9e2 to 7c9db52 Compare October 26, 2026 11:17

@chriskearney chriskearney left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@chriskearney

Copy link
Copy Markdown

Clarification to my review: in the pinned CTS 5c8aae22885448d70a2873e94a93b24b49505c32, both the draw constructor and clear constructor unconditionally call checkNestedRenderPassCapabilities(). That helper requires VK_EXT_nested_command_buffer and its nested-rendering features as well as the conditional-rendering checks.

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 draw_clear cases provide a better route for the core functional checks: ordinary clear requires conditional rendering, and ordinary draw adds only its applicable portability triangle-fan check. Optional maintenance5/device-address variants should be considered only when advertised. This narrows the missing validation to actual output checks for the supported functionality; it is not a request for every optional CTS path or a full conformance run.

@chriskearney chriskearney left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@chriskearney

Copy link
Copy Markdown

Landing-order clarification for my approval at 7c9db5205daa5189cf6e767ccd95f54ff8ea8cf1: the 457 attributed passes were obtained with kernel 8824dd6a (#233 + #238). Our three-machine benchmarks also changed kernel and Mesa together. These results do not qualify Mesa17 alone on an older kernel.

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.

@chriskearney

Copy link
Copy Markdown

@maralcbr @erykwieliczko the matching kernel carry linux-aurora#253 has now landed as 213f6272, with Marcelo's approval and green merge-queue CI. This Mesa head remains 7c9db520, with the existing targeted functional approval and passing aarch64/x86_64 CI; the base is unchanged.

Chris requested the normal merge. Our gh pr merge --merge --match-head-commit 7c9db520… attempt was denied because this account lacks MergePullRequest permission in this repository. Could a maintainer merge this exact approved head? Keep package rollout coordinated with the kernel carry, per the landing-order note; this source merge is not qualification of Mesa17 on an older kernel.

@maralcbr maralcbr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving at 7c9db520. Thanks @erykwieliczko.

  • The profile change is G15S only: hk_physical_device.c:92 adds G15S to the conditional-rendering, unified-image-layouts and memory-budget gate, and :1069 gives 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,470 keep the same values, since each shifted field already fits its width. The hk_cmd_draw.c, hk_acceleration_structure.c and agx3_mesh_core.c changes 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 uaddcarry wrong 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.

@maralcbr
maralcbr merged commit 7f0206e into omacom:integration/aurora Oct 28, 2026
7 checks passed
DjDeveloperr added a commit that referenced this pull request Oct 28, 2026
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants