Repository navigation
Merge Aurora Mesa aurora-wip: G17P submit, refit and leaf-pairing work - #19
Conversation
Remove constant branches before coalescing sample-shaded epilog coverage updates. This keeps the coverage transaction ahead of colour accesses and avoids reacquiring coverage while colour classes are held. The folding runs for M5 coverage keys only, so G17P and G15 keep their epilog shape.
Reuse the forced-early API mask combination for M5 per-pixel fragments. Keep the M5 sample-shaded path unchanged until its separate unroll path is qualified with a dynamic API mask.
Snapshot of the previously uncommitted native-write-class changes that form the base of the qualified M5 driver build (libvulkan SHA-256 ae087e88c5609d25f063e8f241f5138d3129930a465a4bddaf8c36d3d20e50b3). Signed-off-by: Diljit Singh <hey@djdev.me>
Robustness vertex shaders that loop over several texture operations consumed each result right after issuing it, so every fetch completed alone. With ~2000 iterations of 12 serialized round trips the vertex stage outlasted the firmware TA progress watchdog and the device was lost. The native compiler issues all fetches of the loop body first and consumes them afterwards. Add agx_nir_opt_sink_loop_sample_uses, which sinks sample-dependent ALU to the latest top-level loop-body block dominating its uses without moving the texture instructions, and let the M5 ILP sample schedule handle in-loop blocks with two or more samples. Only M5 (AGX3) vertex, geometry and tessellation stages are affected; fragment and compute binaries are unchanged. On M5 the five previously faulting robustness2 sampled_image 3D vertex cases pass 3/3, and a 92,450-case CTS regression passes except one pre-existing concurrency-only DeviceLost. Signed-off-by: Diljit Singh <hey@djdev.me>
Implicit image planes were packed at 128-byte alignment. A sparse binding image with several small planes (for example 3-plane 4:4:4 YCbCr at 11x137) then placed its second plane mid-page, so the opaque bind for that plane produced a VM_BIND with a 4 KiB memory offset and a non-page range, which the kernel rejects, and vkQueueBindSparse reported device loss. Start every implicit plane of a sparse binding image at the 64 KiB sparse alignment the driver already reports, so each plane's bind range and memory offset are page aligned. On M5, the 48 failing image_sparse_binding.multiple_* multi-planar 4:4:4 cases now pass; the whole sparse_resources suite has no failures. Signed-off-by: Diljit Singh <hey@djdev.me>
… query arrays The M5 native ray-query lowering refused any shader with more live queries than the witnessed native allocation indices (8) and any candidate-API query array. rayqueryterminate in any-hit and intersection shaders has eight application queries live next to the pipeline's traversal query, and ray_query.misc.dynamic_indexing selects one of 48 queries per thread, so both failed at pipeline creation with "unsupported query shape". Number queries logically and map each logical query to a native slot modulo the slot count. A slot switch between logical queries is the existing owner-mismatch restart/replay; a resident-logical-query variable forces that restart as well, so two logical queries sharing a slot never read each other's payload. Query arrays larger than the slot count keep their array form: every element gets its own state, a dynamically selected element enters its slot through a select over the reachable slots, and its state is staged through per-operation shadows that are resolved to selects after lowering, so no query state reaches thread-private memory beyond ordinary spills. Arrays up to the slot count keep the qualified split into separate queries. Single-query shaders compile byte-identically; multi-query shaders gain one compare per proceed.
A storage image descriptor carries a texture for imageLoad/imageSize and a PBE for stores. On M5 the native PBE path only covers single-sampled views, and the texture half was kept for views without a PBE only on G17P. Every multisampled (and linear) M5 storage view therefore got a null texture as well, so imageLoad returned zero for every sample: synchronization2.layout_transition.compute_transition_storage loaded a cleared 4x rgba8 storage image and read all zeros. Keep the readable texture on both AGX3 descriptor generations; stores to such views stay null as before. Advertising single-sampled storage images instead would turn the failure into NotSupported, and on M5 it also turned about a third of image.mutable passes into NotSupported. On M5, compute_transition_storage passes and a 15,517-case image load/store, mutable, reinterpret, atomic, format-property and robustness sample has no failures. Signed-off-by: Diljit Singh <hey@djdev.me>
The robust texel-fetch bounds check compared every coordinate component with the texture size at the fetched level. A component that is the constant zero lies inside every valid level and layer, so only the level check can fail for it, and a null descriptor already swizzles every channel to zero. Skip those components. A fetch of texel 0 at a varying level, as in the robustness2 vertex loops' texelFetch(t, ivec3(0), c), then needs neither the size query at that level nor its compares. The skip applies to Gen10 (M5) descriptors only; Gen9 keeps the whole-vector check. Signed-off-by: DjDeveloperr <hey@djdev.me>
Expose only the merge of adjacent if-statements with the same condition, like nir_opt_if_empty_then exposes only the empty-then inversion. A late code-motion pass that empties the block between two such ifs can then merge them without the rest of nir_opt_if reshaping its control flow. Keep absorbing following ifs into the same one, so a run of several same-condition ifs becomes one in a single call. Signed-off-by: DjDeveloperr <hey@djdev.me>
The M5 backend retires every outstanding scoreboard slot before a control-flow instruction. In the robustness2 sampled_image vertex loops each axis has a conditional texel fetch, `if (c != 0) texelFetch(t, ivec3(0), c)`, between its unconditional samples, so even with every consumer sunk to the end of the body the eleven or twelve samples of an iteration completed in seven or eight serial waits. With ~2100 iterations the vertex stage still ran close to the TA watchdog and was lost when three CTS processes shared the GPU. Add agx_nir_opt_hoist_loop_samples: a sample in a later top-level block of a loop body moves, with the pure computation of its sources, to the end of the earliest top-level block before it from which only conditional blocks without jumps, loops or side effects separate it. It runs exactly as often as before and no sample is speculated. Only the samples whose consumers agx_nir_opt_sink_loop_sample_uses sinks move, so the live results keep that pass's bound. The conditional blocks left adjacent with the same condition are merged with nir_opt_if_merge, after which their identical fetches CSE. In the r32i 3D vertex loop the unconditional samples now issue back to back and the three conditional fetches become one: 12 samples and 8 serial waits per iteration become 10 samples and 3 waits, and the loop shrinks from 413 to 328 instructions. Only geometry-side M5 stages are affected, as for the sink pass. The loop-sample unit test now builds valid NIR (phi sources added after insertion never joined their definitions' use lists, and if-phi constants sat ahead of the phis), so it also passes with assertions enabled. Signed-off-by: DjDeveloperr <hey@djdev.me>
Sort-by-rank only distinguished constants, convergent and divergent sources (the TODO for loop-invariant code motion). A chain inside a loop whose sources are all convergent, such as an accumulation of texture results with uniform coordinates plus per-iteration constants, therefore had a single rank and was left alone once it was too long for the CSE heuristic, so each iteration added every constant separately. Add nir_reassociate_loop_invariant. With it, each class is split by invariance in the innermost loop around the chain: a source defined outside the loop, or computed inside it without side effects from such values, ranks below the sources varying with the loop. The invariant part of the chain is then computed first and can leave the loop through nir_opt_preamble or loop-invariant code motion. Invariance is a bounded, memoized walk over ALU, texture and reorderable intrinsic sources; it only steers the order, so a conservative answer only costs optimization. The ranks of a chain are now computed once and sorted together with its sources by the same comparison as before. nir_opt_reassociate_loop keeps its second, scalar-only round when callers pass extra options. Without the new option the chains are reassociated exactly as before. Signed-off-by: DjDeveloperr <hey@djdev.me>
The robustness2 null-descriptor vertex loops add the same three size queries per axis to every component of their accumulator in each of the ~2100 iterations. All terms of those chains are convergent, so reassociation gave them one rank and left the chains alone, and each iteration spent 36 adds on values that never change. Reassociate with nir_reassociate_loop_invariant on M5: the invariant terms are summed first, so preamble formation computes their sum once and each iteration adds it once per component. The r32i 2D-array null-descriptor loop shrinks from 305 to 275 instructions. Signed-off-by: DjDeveloperr <hey@djdev.me>
A value added twice to an accumulator, x + (y + y) after reassociation groups a chain's duplicated sources or (x + y) + y where it does not, costs two adds. The robustness2 vertex loops add each texture() result twice, because texture() and textureLod(.., 0) CSE to one sample, once per component and axis. Add agx_nir_fuse_doubled_add, run with the late fusions on M5, which rewrites both forms of a 32-bit doubled add to x + (y << 1), one iadd with the source shift. The r32i 3D vertex loop loses another 12 instructions (328 to 316), the r32i 2D-array null-descriptor loop another 12 (275 to 263). Signed-off-by: DjDeveloperr <hey@djdev.me>
…ure queue hk_host_as_init() creates its queue as index 0xffff of a create info whose pQueuePriorities points at one float, and hk_queue_init() reads pQueuePriorities[index_in_family] when no global priority is chained. The read lands 256 KiB past the float; on the Neo it hits unmapped stack and every device that enables accelerationStructureHostCommands crashes at vkCreateDevice (dEQP's default device does). Chain VK_QUEUE_GLOBAL_PRIORITY_MEDIUM_KHR, the class the intended 1.0 priority maps to, so no priority array is indexed. Signed-off-by: Diljit Singh <hey@djdev.me>
HK_FRAME_ANATOMY=<path> logs, for every Nth vkQueueSubmit, each hardware command the submit carries (render geometry, attachment load/store and background/end-of-tile programs, draw counts, barriers handed to the kernel) and optional per-command GPU timestamps read back lazily at the next sampled submit. Unset, the profiler is compiled in but inert: every hook is a single NULL test and no hardware or kernel structure changes.
Two consecutive render pass instances on the same attachments, where the second loads what the first stored and only state binding and framebuffer-local barriers are recorded between them, are one tile pass to the hardware. The second instance's draws now continue in the first instance's render stream, which drops the store and reload of every attachment and one command boundary. The ended instance records where its draws ended and the tracked state; the next instance continues only when its geometry and every attachment match, it loads every attachment, and nothing was recorded or ordered in between except barriers whose second scope is the fragment-test and colour-output stages with attachment accesses only, or whose first scope does not include graphics work. Those are applied to the merged stream as a newly created render stream would take them. A new sync epoch counter, bumped by every hk_sync mutator, lets the candidate detect any intervening dependency change. The frame anatomy profiler gains opt-in decision notes. aurora-wip already continues render pass instances on native AGX3 (hk: continue render pass instances on the same attachments on native AGX3, and the later ordered-dependency form). This enables that path on M5 and adds its frame-anatomy notes.
Records after the first of one indexed-indirect multi-draw that read draw parameters republish the vertex stage program per record. No API call can intervene between records, so every launch range the previous record derived from the root, descriptor sets and their buffers is unchanged; only the draw-parameter and draw-index addresses follow the record. Publish from the previous record's ranges with those substituted instead of replaying the launch, and fall back to the general path on any mismatch. The emitted loader bytes are identical. Also evaluate the prepared bind-tail match only on the chip whose bind tail consumes it, as the slow dynamic-state path already does. aurora-wip already republishes multi-draw vertex programs on native AGX3 (hk: republish multi-draw vertex programs from the previous record on native AGX3); this enables it on M5.
A dependency between two render pass instances on the same attachments that is not framebuffer-local, up to ALL_COMMANDS -> ALL_COMMANDS with every memory access as Minecraft records between its sky and main passes, previously ended the continuation candidacy. Accept it when one tile pass is memory-equivalent to the two instances by construction: - the first instance's stream wrote no memory but its attachments, so the second instance's vertex work, which the tile pass runs before the first instance's fragment work, reads what it would have read after it; - the second instance writes no memory but its attachments either: a draw whose bound programs write external memory, transform feedback, software or mesh stages and adjacency end the stream first, and the draw continues in a new stream ordered after the whole merged pass; - no shader can read the attachments' memory, which the merged pass writes only at its end: the layouts match, no attachment is in a feedback loop or can alias another resource, no descriptor set is bound for graphics, and no graphics push descriptor recorded so far in the command buffer names an attachment image. Later set binds and pushes that name one end the stream before the next draw. Layout transitions and queue family transfers between the instances still end the candidacy. Queries, timestamps, conditional rendering and transform feedback commands inside an ordered continuation end it at the command. A pre-graphics stream created for a continued draw (index bounds passes) sits before the merged render; it no longer waits on that render, which holds only work it cannot depend on. aurora-wip already continues render passes across ordered dependencies on native AGX3 (hk: continue render passes across ordered dependencies on native AGX3); this enables its push-descriptor bookkeeping on M5 too.
On M5 a fragment shader's main part picks between a testing and a killing coverage update with a uniform branch on the draw-time no_epilog_discard selector, so that an epilog that discards (alpha to coverage, depth/stencil export) can own the tests instead. With alpha to coverage statically disabled in the pipeline and no depth/stencil export, no epilog of the shader discards, and the selector is all ones for every draw that can use the shader. Record that in the fragment key and fold the selector at compile time. The main part then ends in the single unconditional testing coverage update the native compiler emits, instead of the uniform branch and its second arm. Pipelines with dynamic alpha to coverage, libraries without multisample state and shader objects keep the selector. Minecraft's rain fragment shader (particle, translucent blend, discard, reverse-Z test without depth writes) shrinks from 74 to 62 main-part instructions and from 99 to 85 linked, against 71 from the native compiler for the same shader and blend state. aurora-wip already folds the selector on native AGX3 (hk: fold the depth/stencil test selector under static state on AGX3), where G17P also requires the static no-early-reject proof. This enables the fold on M5 through the same key bit, without that proof: M5 has no read-only early reject.
The precise depth/stencil store (stores follow storeOp/resolve in the stream that ends the pass) and the elision of a depth/stencil attachment that no draw touches were limited to G17P. Every other AGX3 part fell back to hk_pack_zls_control()'s forced store, so M5 wrote the whole depth plane at the end of every render pass even for storeOp DONT_CARE, and loaded and stored untouched depth in full-screen colour-only passes. Both decisions depend only on Honeykrisp's control-stream split and the kernel's partial-render ZLS word, not on the encoding, so gate them on the AGX3 ISA family (G17P, M5 and the other parts on the AGX3B encoding) instead of G17P. On M5, a 23,232-case depth/stencil load/store sample (renderpass1/2, dynamic rendering, pipeline depth/stencil, fragment operations, depth copies) passes with no failures or GPU faults. aurora-wip already stores and elides precisely on every native AGX3 part (hk: store and elide depth/stencil precisely on every native AGX3 part); this adds M5. Signed-off-by: Diljit Singh <hey@djdev.me>
The render pass continuation (same-attachment and ordered), the multi-draw vertex program republication and the static depth/stencil test selector fold were gated to M5. None of them depends on the HAL300 command format: continuation keeps an ordinary render stream open and uses the common sync tracker, the republication goes through the shared AGX3 graphics loader, and the selector exists in every main part that may discard. Gate them on the AGX3 instruction set instead. G17P can move a discarding shader's read-only depth test into the prolog at draw time, with the main part's selector off. Fold the selector there only when the pipeline's static state rules that out (multisampled, depth test off, depth writes on, compare ALWAYS or depth bounds on), and never take the early reject for a folded shader. aurora-wip already runs both on native AGX3 and the preceding commits add M5, so only the comments that still named M5 change here. Signed-off-by: Diljit Singh <hey@djdev.me>
With the native acceleration-structure profile, every M5 device read the query traversal assets named by ASAHI_M5_RQ_CAPTURE_DIR at creation (an 8 MiB code buffer at a fixed VA, an 8 MiB host image and the intersection table), and failed with VK_ERROR_INITIALIZATION_FAILED if any file was missing. Steam's container runtime replaces /usr, so inside it the directory does not exist and every game's vkCreateDevice failed (DXVK, zink), although none of them uses ray queries. Load the assets only when the device enables rayQuery or rayTracingPipeline, and advertise VK_KHR_ray_query only when the files are readable in the process, so a process that cannot see them gets no ray queries instead of no device. Devices without ray queries also stop paying for the buffers and the fixed VA range. Signed-off-by: Diljit Singh <hey@djdev.me>
When the AGX3 encoder refused an instruction, the compile failed with "AGX3 binary packer refused an instruction" and only AGX_MESA_DEBUG=shaders, with a full dump of every shader, said which. That is useless for finding failures in a running game. Keep the first refusal of each pack (opcode, encoder line, gate) without any debug flag, put it into the compile refusal, and print that refusal in hk's "constant program failed to compile" fallback warnings. Signed-off-by: Diljit Singh <hey@djdev.me>
The f32mul forms that encode persp_mul carry scoreboard waits only for slots 1..6, but AGX3C allocates eight slots. A persp_mul that had to wait for an asynchronous result on slot 7 or 8 therefore failed the whole compile, and the fragment shader could not be created: Skyrim SE loses 62 of its 7615 fragment shaders this way (VK_ERROR_UNKNOWN from pipeline creation, so the draws are dropped). When the form cannot carry the wait, retire it first on an identity of the destination, as already done for other producers without a suitable wait field, and encode the multiply without a wait. Signed-off-by: Diljit Singh <hey@djdev.me>
A cube sample whose texture and sampler operands are uniforms or immediates groups CEM's two face finalizers with its own result slot, and the finalizer form's slot field only names slots 1..7. On AGX3C the round-robin allocator also hands out slot 8, and such a sample made the whole shader unencodable. Every fragment shader whose constant program hoists cube descriptors then lost the constant program (505 of Skyrim SE's 7615 fragment shaders, "G17P fragment constant program failed to compile; compiling without it"). Let the slot allocator take a per-instruction slot ceiling and give these samples 1..7. Register descriptors stage on slot 6 and are unaffected. Signed-off-by: Diljit Singh <hey@djdev.me>
AGX3C encodes rcp and rsqrt without a saturating form, so a saturate folded into one by the fmov propagation made the shader unencodable: 6 of Skyrim SE's fragment shaders clamp a reciprocal and could not be created at all. Keep the separate saturating move for those two operations on AGX3C. Signed-off-by: Diljit Singh <hey@djdev.me>
A new dynamic offset (vkCmdBindDescriptorSets per draw, as D3D translation layers and zink issue) republishes the root. On M5 that also republished the fragment program, whose uniform ranges address the root, and with it the whole HAL300 graphics-state block: about 250 ns of GPU time per draw. Record which root bytes each shader can read, by classifying its constant-offset root loads against the spans a draw may republish without the fragment stage seeing them: the dynamic buffer table and the vertex input tables. Any other root access counts as reading everything. When the new root differs from the fragment program's only in spans the fragment shader does not read, the program keeps the root it was published with: that root stays valid for the command buffer and holds the same bytes wherever the shader reads, so the published program and the PPP state stay as they are. Twin benchmark, 1210 direct draws with a new dynamic uniform buffer offset per draw: pass span 1360 us -> 1108 us, matching the same draws with fixed bindings (1067 us), identical image. Signed-off-by: Diljit Singh <hey@djdev.me>
The M5 task payload lowering refused every shader with a payload atomic, so pipeline creation failed for all 20 glsl.atomic_operations.*_task_payload cases once task shaders stopped being NotSupported. The payload is ordinary memory at the native payload address, so its atomics are global atomics there; the generic lowering then handles them like any other, including the float operations. Signed-off-by: Diljit Singh <hey@djdev.me>
|
Updates since the aurora-wip #84-#90 sync:
Mesa CI on 14d5e38 passed (source, x86_64 and aarch64 builds): https://github.com/omacom/mesa/actions/runs/40755713326 |
#229 gained 6c66b41a ("drm/asahi: restore the DRM driver's module owner"),
so its head moved from 1032928116c1. Re-pin the kernel contract to the
new head. The UAPI header is the same in both, so the ABI check is
unchanged. This branch still merges after #229.
Signed-off-by: DjDeveloperr <hey@djdev.me>
|
Re-pinned the kernel contract to omacom/linux-aurora#229's new head 6c66b41a (ebc78bf), after #229 gained the restored DRM module owner. The UAPI header is unchanged, and this still merges after #229. |
chriskearney
left a comment
chriskearney
left a comment
There was a problem hiding this comment.
Source review of ebc78bf786f5e4a61a3c4e89c5b645c8b05e6990: requesting changes for the G15 occlusion-query availability path below. This is a source-level correctness finding; a focused M3 Pro hardware reproducer is still pending. Native build tests passed 122/122, and a compiled UAPI-layout comparison against the tested #229/#244 kernel found no incompatible common fields. Those checks do not validate GPU query completion ordering. Four-board benchmark results will follow separately once the test and restoration checks are complete.
| /* A retirement timestamp (any value but the queued store's 1) was | ||
| * written after the end-of-render maintenance: no submission wait. */ | ||
| if (available && | ||
| !(pool->avail_shift && hk_query_available_value(pool, query) > 1)) { | ||
| VkResult done = hk_oq_submission_done( |
There was a problem hiding this comment.
[P1] Keep the submission-completion check for G15 stamped occlusion queries
The > 1 shortcut treats every render timestamp as post-maintenance, but that holds for the G17 retirement implementation, not the G15 firmware timestamps. The stamped path is enabled on G15S/M3 Pro. hk_try_stamp_available() assigns consecutive queries to frag.end, vtx.end, frag.start, then vtx.start. The G15 kernel resolves all four destinations and places the vertex timestamps in the TA work item. A vertex timestamp can therefore make a query available before its fragment counts reach memory. Skipping hk_oq_submission_done() here allows vkGetQueryPoolResults(..., WAIT_BIT) to return success with zero or incomplete counts.
A minimal trigger is two queries in one render pass, followed by reading query 1 first, before waiting for the submission fence. One query plus an in-pass API timestamp occupying frag.end also reaches the early slot. The existing end-of-render maintenance explanation above this function describes why the submission wait is necessary. A batched read beginning with query 0 can mask this ordering issue.
Please retain the submission wait on G15 or restrict this shortcut to a kernel/chip path with a proven post-maintenance timestamp contract. G17's retirement check and destination publication provide that ordering; this finding does not apply to that path or to M1's unstamped path. Add a G15 regression that reads the later query first and compares against the completed-fence result. No hardware reproduction is claimed yet.
There was a problem hiding this comment.
Right, thanks for the hardware confirmation. Fixed in 1ccff51 and 2b1bc1e (aurora-silicon#94). hk_oq_render_stamps_supported() now gates the stamped pool. It returns true only for chips whose kernel writes the slots at render retirement: G17P, and M5, which runs the same G17 retirement path. G15 (and G16G) no longer bind the timestamp buffer, so every query end keeps the queued availability store, and readback keeps hk_oq_submission_done(), as in your 7f0206e control arm. test-hk-query-oq checks the chip decision. A G15 regression that reads the later query first needs M3 hardware, so it would come from your probe rather than from CI.
maralcbr
left a comment
maralcbr
left a comment
There was a problem hiding this comment.
@DjDeveloperr thanks. The write-up made a 371-commit sync reviewable. I read it by area, with the shipped chips as the lens: M1/M2 (G13/G14), M3 (G15G/S/C) and Neo (G17P). I didn't read every M5 line. One blocker, on M3 only.
Blocker: M3 treats stage timestamps as render retirement
Occlusion availability carried by renders (#62) is on for every native AGX3 chip, not only G17P:
stamped = occlusion && agx_native_graphics()(hk_query_pool.c:575), andagx_native_graphics()is true on G15G/S/C.hk_defer_oq_available()has no chip check (hk_query_oq.c:22-41). On M3 the forward resolver runs (hk_query_oq.c:266-274). When it returnsHK_OQ_STAMPED, it drops the post stream that held the queued availability store (hk_query_oq.c:152-153).- Slots are handed out in the order
{3, 1, 2, 0}(hk_query_oq.c:109), andhk_queue.c:332-341maps them to vertex start, vertex end, fragment start and fragment end. hk_GetQueryPoolResults()takes any availability value above 1 as a retirement timestamp and skipshk_oq_submission_done()(hk_query_pool.c:1230-1237).
The M3 kernel doesn't write retirement times. On G15 the firmware writes each of the four values straight from the stage's start and end records (g15/timestamp.rs, header comment; the TA work's timestamp tail at g15/encode.rs:679-687). So the vertex-end stamp lands when tiling finishes, before the fragment work that writes the occlusion counts. Fragment start lands earlier still.
Failure scenario: two occlusion queries end in the same render on an M3. The second one gets slot 1 (vertex end). Its availability turns nonzero once tiling finishes, and vkGetQueryPoolResults returns its count as available, before the fragment work has written it.
The G17 kernel keeps timestamp destinations until retirement (g17/completion.rs), which fits the assumption, and the A18 Pro CTS was clean. So the simplest fix is to stamp only on G17P: stamped = occlusion && dev->chip == AGX_CHIP_G17P. M3 then keeps the queued store, as on the base. If you'd rather keep it on M3, it needs fragment end only, plus a completion check on readback. I found this in the code and haven't reproduced it on hardware.
What I checked
-
Merges. I reverse-applied omacom's side (4911479..7f0206e, #17 included) to the head tree. 76 of the 83 source files apply as-is. The other seven keep omacom's behaviour:
hk_physical_device.c: the G15Snative_agx3gate, with M5 OR'd in (:105-108);asahi_clc.c:g15gnext tog17g;agx_device.c: the T8122 USC/VA reservation andagx_va_user_limit();hk_g17p_compression.h: omacom's pNext walk;hk_cmd_draw.c: the variables 87ba417 brings back;- the two compiler tests:
test-native-form-regressions.cppkeeps Aurora's image-store test, as your description says, andtest-preamble-modifiers.cppdiffers only by the new argument.
-
UAPI. I compiled both headers (this one, and the kernel's at
integration/aurora, which now has #229) and compared every shared struct, member and constant. They match:params_globalis 616 bytes,gpu_hal_generationis at 596, and the limits tail is at 600/608;CUMULATIVE_SIGNALSis bit 29, and the ioctl numbers agree;- the kernel's
get_paramscopiesmin(its size, requested), so a pre-#229 kernel leaves the tail zero, and the M3 runtime's 600-byte minimum passes.
A matching layout says nothing about timestamp semantics, hence the blocker.
-
Shared Mesa code.
- The NIR additions are opt-in.
nir_opt_if_mergeis a new entry point, andnir_reassociate_loop_invariantis a new flag; without the flag,rank()is unchanged. vk_queue.cchanges every driver on the common runtime, but our package builds only hk (-Dvulkan-drivers=asahi). The dropped-signal path records the loss first and doesn't signal twice.- The common BVH leaf changes don't reach M1–M3. Acceleration structures are on only for G17P, G16S and the M5 bring-up profile (
hk_physical_device.c:1923-1951), andray_query = false(:117).
- The NIR additions are opt-in.
-
M5 and M4 Pro stay off. M5 needs
ASAHI_M5_BRINGUP=1, andagx_params_driver_supported()still refuses T6040. -
CI. The Mesa CI run for this head built PR merge commit bf87c39 on aarch64 and x86_64. All 156 configured meson tests passed on both, including
agx_tests,hk_query_oq,vk_queue_dropped_signaland the NIR tests. The kernel ABI check against 6c66b41a matched.
Changes that reach shipped chips, with no hardware run cited for them
These are edge checks after merge, not blockers.
- Occlusion counters taken on first use, every chip (c46b868).
- Counters now come from the hardware heap when a command first uses a query:
hk_query_pool.c:803-823for the allocation and its compare-and-swap,:865-869for the begin/end path,:1321-1328for copies recorded before their producer. - Running out of counters is now a command-buffer error rather than a pool-creation failure.
- The code holds up, including a reset recorded before the counter exists; on AGX2 the libagx reset handles the unassigned value.
- The commit's 450 CTS passes don't name a chip.
- Counters now come from the hardware heap when a command first uses a query:
- Uniform-address atomics, M3 and Neo (
agx_compile.c:13063-13073). The base keptnir_opt_uniform_atomicsoff on all AGX3 (uniform_atomics = !is_agx3). It's now on for AGX3B in the compute, vertex, tessellation-control and fragment stages. That includes G15, despite theis_g17pname (:12992, which checks for AGX3B). Renaming it tois_agx3bwould help. - Counter-Strike 2 option, M1/M2 and Neo (6f3da56,
00-hk-defaults.conf:36-46,hk_image.c:488-500).application_name_match="csgo"turns onhk_mutable_format_srgb_pair. That makes CS2's otherwise-eligible mutable images compressible when they have no format list and have an sRGB/UNORM twin. Before, they never were.- M3 isn't affected, since
allow_agx3doesn't admit G15 (hk_image.c:435-439). - If CS2 ever creates a view through another channel layout, it reads compressed data through the wrong layout; the check only logs once (
hk_image_view.c:911-925). - It's an app-specific default, so I'm flagging it for the maintainers. Which chips was it run on?
- M1 Pro/Max: one command per submission (
hk_queue.c:428-438). G13X now submits each control stream in its own ioctl.
Non-blocking
- UAPI bits the kernel hasn't reserved. This header defines feature bits 17–24, 26 and 28, command types 5–7, and
BIND_SPARSE_TILE. linux-aurora defines none of them. Every use is gated on its feature bit, so nothing runs today. But if the kernel later gives one of those bits to something else, Mesa enables an M5 path it doesn't have. Please reserve them in the kernel header, in a small linux-aurora PR. - High-priority GL contexts (
agx_pipe.c:1951-1964). High and realtime contexts now ask for HIGH and fall back to MEDIUM. On M1–M3 that costs one failed ioctl per such context. Fine as is. - Unreferenced test file.
src/asahi/compiler/test-agx3-m5-instructions.cisn't built; meson usescompiler/test/test-agx3-m5-instructions.c(src/asahi/lib/meson.build:325). - M5 pNext walks. Two M5 loops in
hk_g17p_compression.h(381 and 497) still use thememcpy-then-cast walk that omacom replaced at 153. M5 only.
Testing
On edge, after the fix and the merge:
- occlusion queries on M1 or M2, on M3, and on an M1 Pro/Max (DXVK, or the CTS
query_poolocclusion group); - an atomics check on M3 (compute and fragment, including returned values);
- a GL/Vulkan desktop smoke on M1/M2/M3 for the shared register-allocator retry;
- CS2 on M1/M2.
|
Hardware confirmation of the G15 query issue at ebc78bf, on M3 Pro / G15S B1, using the fixed #235+#229+#244 kernel ( The probe uses PRECISE occlusion queries around 1024×1024 fullscreen draws, one render pass, host reset, no subsequent command touching the pool, and single-query
Every incorrect early result was 0, with Both reported candidate variants completed with the actual pinned staged Bounds: the full four-variant candidate probe did not complete. Its later one-query-plus-timestamp variant ended too quickly for the mapping witness and was rejected; that output is not counted, and the candidate copy-results control was not reached. The existing-Mesa four-variant control completed cleanly. The two basic occlusion CTS cases passed on both Mesa arms, illustrating why those tests alone did not catch this ordering bug. Benchmark results and restoration checks are being collected separately. The G15 changes request remains. |
|
Four-board benchmark and validation report for Mesa #19 ebc78bf. Approval remains blocked by the hardware-confirmed G15 occlusion-query bug (reproducer results). Each board used the same fixed DJ kernel in A and B: source union Seven workloads, one warm-up and three measured runs each, plus clean and deliberately failing compute controls: 30 attributed benchmark runs per arm. All eight selected A/B arms completed their benchmark gates (240 runs including controls/warm-ups), with native driver mapping, result checks, fault-cursor checks and process cleanup. The runner used a private user HOME/cache and the same pinned Vulkan helper in both arms. Observed median change in submit-and-wait wall time, B versus A. Positive means slower. These are descriptive n=3 samples, not a performance signoff. CPU affinity/frequency were not fixed.
GPU timestamp timing changes for the same runs
The CPU-sensitive numbers varied between jobs. For example, M1 batched-submit was −3.3% in the first #19 arm but +15.2% in the later arm, while GPU time stayed about −0.1%. M3 single-submit control medians ranged from 17.90 to 23.59 ms across A runs; #19 was 17.49 ms. The −25.9% cell above uses the immediately preceding A control and must not be read as an established speedup. Raw min/median/max and CPU/GPU metrics are retained; none of these timings overrides the correctness blocker. Validation bounds:
Selected benchmark jobs:
All four machines returned to their original kernels. Installed packages, boot.bin, original UKIs and original kernel modules were verified; temporary staging and permissions were removed and leases released. M1 Air’s unrelated 03:00 hourly snapshot/menu addition was verified and preserved. This was staged diagnostic coverage, not installed-desktop qualification, full CTS, or RC approval. |
Native AGX3 occlusion pools let a render carry a query's availability in one of its four retirement timestamp slots, and a CPU read that finds such a timestamp skips the submission wait. That assumes the kernel writes every slot when it retires the render, after the end-of-render maintenance that makes the counts visible. The G17 kernel does. On G15 (M3) the firmware writes each slot from its stage's own start or end record: the vertex end lands when tiling finishes, before the fragment work writes the counts. With two queries in one render, the second one rides on the vertex-end slot, and vkGetQueryPoolResults, even with WAIT_BIT, returns its count as available before the fragment work wrote it. On an M3 Pro, reading the later query first before the fence returned 0 in 48 of 48 reads, against the correct count after the fence. Add hk_oq_render_stamps_supported(), true only for G17P, where the path was measured, and gate the stamped pool on it. Every other chip keeps the queued availability store and the submission wait on readback, as before the stamped path. Other chips whose kernel writes the slots at retirement can be added once qualified. test-hk-query-oq checks the chip decision. (cherry picked from commit c68abcd)
M5 (G17G/G17S) runs the same G17 kernel runtime as G17P for render retirement. A render completes only once its completion stamps cover it, after the end-of-render maintenance (g17/queue/render/retirement.rs, complete_poll()). The host then writes the four stage timestamps into the requested slots before signalling the fence (g17/completion.rs, Output::publish()). Only the render publication and its notification differ on M5. A stamp in a query's availability slot therefore means the counts are visible there too, so include G17G and G17S in hk_oq_render_stamps_supported(). G15 and G16G keep the queued store. (cherry picked from commit 9876607)
|
@maralcbr @chriskearney thanks both. chriskearney, thanks especially for the M3 Pro reproducer. Blocker (G15 occlusion availability): fixed in 1ccff51 and 2b1bc1e, first merged into aurora-wip as aurora-silicon#94.
CS2 option ( Other points:
Mesa CI on 2b1bc1e is running: https://github.com/omacom/mesa/actions/runs/40814062291 |
|
UAPI reservation: omacom/linux-aurora#273 reserves feature bits 17–24, 26 and 28, command types 5–7 and |
src/asahi/compiler/test-agx3-m5-instructions.c is an early copy of compiler/test/test-agx3-m5-instructions.c. Meson only builds the one under test/ (src/asahi/lib/meson.build), and nothing else refers to the stray copy. (cherry picked from commit 390bcee)
The M5 colour, G17S format-list and M5 depth eligibility checks in hk_g17p_compression.h copied a whole VkBaseInStructure per chain link and read VkExternalMemoryImageCreateInfo::handleTypes through a cast of the chain pointer. Read their chains as the G17P walk on this branch already does: copy each sType and the next pointer, and copy the external-memory info into a local. The file now matches aurora-wip. hk_g17p_compression_test: 4069 checks pass. (cherry picked from aurora-silicon/mesa commits ea89aed and 3142c63; the G17P walk already had this form here)
|
Cleanups from the review are in, both first merged into aurora-wip (aurora-silicon#95, #96):
Mesa CI on 463b1a7 passed: https://github.com/omacom/mesa/actions/runs/40815496345 |
There was a problem hiding this comment.
The G15 occlusion-query blocker is fixed. Hardware-tested head: 2b1bc1e818cf08e091f3d73204494a86759b3e46; approval/source-reviewed head: 463b1a71d47eb83554b3e0efa178a764af6e6479. I reviewed the four-file fix from ebc78bf7 and reran the later-query-first reproducer on the same M3 Pro (G15S/J516S). The two later cleanup commits are accounted for separately below.
| Mesa arm | Qualified reads | Incorrect pre-fence-wait results | Incorrect after-fence results | Reads returning while the fence was unsignalled |
|---|---|---|---|---|
Previous #19 ebc78bf7 (earlier hardware run) |
48 | 48 (all zero) | 0 | 48 |
Fresh installed control 7f0206e79f9 |
48 | 0 | 0 | 0 |
Fixed #19 2b1bc1e8, isolated staged driver |
48 | 0 | 0 | 0 |
Both fresh arms used kernel 7.2.11-9-3504.2-ARCH (source union 90e5e49d), the same probe binary 31b073df / source manifest 6134f7af, 1024×1024 PRECISE queries, 2048 fragment iterations, and eight rounds each of the two-query and four-query variants. Query 1 is read first with single-index WAIT_BIT, before any explicit fence wait. Every value was 1,048,576 in both fresh arms. The runner change only selects those two variants and verifies their 48-read total; the one-query/timestamp and GPU-copy variants were not rerun. This is targeted regression evidence, not full Vulkan conformance.
The fixed driver mapping was witnessed at the job's staged path and matched the production archive member ac2fdacd; the installed driver was not mapped in that arm. Job/input/boot/kernel/library identities, per-run fault cursors, process-group drain and temporary-directory cleanup passed. No new kernel fault appeared during either focused probe. Control job: j1028-205708-5268; fixed-head job: j1028-210158-b1f1. Production build: b1028-205249-676f.
Source reasoning agrees with the hardware result: hk_oq_render_stamps_supported() admits G17P and M5's G17 chips only. G15 pools retain avail_shift == 0 and no stamped pool handle, so query completion uses the queued availability store and CPU readback keeps the submission-completion wait. The new chip tests cover G15/G16 negatives and G17P/G17G/G17S positives. A separate same-head native M5 validation build actually ran 122/122 tests, including 55/55 hk_query_oq checks; the production builder skips check(), so the test result is not inferred from package-build success. CI for the approval head passed the source, x86_64 and aarch64 jobs.
Validation limits: the existing-Mesa GL/Vulkan pixel control passed. The staged arm's managed provider checker still rejected DRM character-device maps; its isolated corrected extra checker could not obtain its required process-image witness at the completion boundary. Those failures remain recorded, and the overall staged job is not presented as a pass or as staged GL/installed-desktop qualification. They do not invalidate the separately attributed Vulkan reproducer. The stock-Mesa system-check control passed. On the fixed-head boot, OMT flagged hardware.probe-errors and input.ambient-light: AOP firmware had crashed during startup, before the staged provider tests. Those failures are retained separately; no Mesa cause is established by this run.
The M3 Pro returned to its original 7.2.11-9-1-ARCH boot with AOP healthy and both ambient-light IIO devices present. Original boot.bin, UKIs, modules, configs, package versions and known-good records matched the saved audit. The original agent and parent permissions were restored, audio mute/volume matched exactly, staging was removed and this campaign’s lease released.
Successor 463b1a71 adds only two changes after the hardware-tested head: deletion of an unbuilt duplicate M5 instruction-test source (the registered copy is unchanged), and pNext copying in three M5 compression helpers. All three helpers reject non-M5 chips before walking the chain; the OQ files are byte-identical. The existing compression policy host suite passed 4,069 checks with both old and new headers. Correction after function-targeted mutation checks: all three chip-gate removals are caught. HAL200/G17P chain truncation is caught, while M5 colour, MSAA and depth later-link truncations are not caught at this head. The initial occurrence-based mutant labels incorrectly called the HAL200 walk the M5 colour walk; the corrected results are recorded in the later approval review. This successor review is source/host evidence, not another hardware run.
Approving this reviewed head: the blocking correctness regression is closed by the source fix and focused M3 hardware evidence. The earlier four-board benchmark report remains descriptive coverage of ebc78bf7; its small-sample timings are not a performance signoff, and no new Neo/M5 hardware result is claimed here.
The M5 depth eligibility test only put external memory as the first chain link, so a walk that stopped after one link still passed. Check a format list followed by external memory, and by the WSI marker, as refusals, and the format list alone as eligible. hk_g17p_compression_test: 4078 checks pass; the same test fails when the depth walk stops after its first link. (cherry picked from commit 4476f9c)
|
@chriskearney thanks for the re-run and the review. On the test gap you found, acb855f (aurora-silicon#97) puts external memory and the WSI marker behind a format list in the M5 depth chain test. The suite is now 4078 checks, and it fails when the depth walk stops after its first link. Test-only. Mesa CI on acb855f passed: https://github.com/omacom/mesa/actions/runs/40816598657 |
chriskearney
left a comment
chriskearney
left a comment
There was a problem hiding this comment.
Approval refreshed at acb855f. It adds only the later-link M5 depth-chain regression tests; all production code, including the fixed G15 OQ path, is byte-identical to the previously reviewed head.
The existing host compression test passes 4,078 checks. An accurately targeted depth-walk truncation passes the old test and fails the new test at the later external-memory link, confirming the added regression coverage. Current source/x86_64/aarch64 CI is green.
Correction to the mutation labels in my earlier review: the initial script selected repeated statements by occurrence and labelled the HAL200/G17P walk as M5 colour. A function-targeted rerun shows HAL200 truncation is caught, and the new commit now catches M5 depth truncation. M5 colour and MSAA later-link truncation remain uncovered; all three chip-gate removals are caught. The earlier review text has been corrected. These are coverage limits, not observed failures in the actual refactor.
The M3 hardware evidence and its limits still apply: exact tested driver head 2b1bc1e8 returned 48/48 correct query values, versus 48 early zeros before the fix, on the same kernel and probe. No hardware run is claimed for the later M5-only cleanup or this test-only commit. The broader provider-check and AOP/OMT failures remain recorded separately; the M3 originals were restored and verified. No merge performed.
maralcbr
left a comment
maralcbr
left a comment
There was a problem hiding this comment.
Approving at acb855f. The M3 blocker is fixed: hk_oq_render_stamps_supported() limits stamped pools to G17P and M5. G15 and G16G pools get no timestamp binding and avail_shift == 0, so they keep the queued availability store, and readback keeps hk_oq_submission_done() (hk_query_pool.c:576-578, 647, 969-976, 1235-1237). GPU copies pass the real shift. Chris's M3 Pro rerun on 2b1bc1e read 48/48 correct values, against 48 early zeros before the fix. The later commits only touch M5 or tests, and CI on this head passed.
Thanks for the cleanups and omacom/linux-aurora#273.
Still open after merge:
- CS2 option: validated on M5 Pro only. Leaving it as is for now; the CS2-on-M1/M2 edge check decides whether it gets scoped to M5.
- Edge checks from my earlier review: occlusion queries on M1/M2 and M1 Pro/Max, atomics on M3, and a desktop smoke test.
This mirrors the Aurora Mesa work merged into aurora-silicon/mesa
aurora-wipsince the two trees last shared history (aurora-silicon#64, 2026-10-24). aurora-wip was 40 commits ahead, so this is one merge commit.What it brings in:
hk_query_oq.cwith no functional change (the moved functions are byte-identical), plustests/test-hk-query-oq.c: 36 checks of the defer rules and the KEEP, STAMPED and MOVABLE outcomes, including a later reset and occupied timestamp slots. These are the tests Greptile asked for below.hk_query_oqnow has 48 checks.src/vulkan/runtime/bvh/hploc_internal.comp, added in a second merge commit). The A18 Pro RT CTS was clean.DRM_ASAHI_FEATURE_CUMULATIVE_SIGNALSat bit 29 andDRM_ASAHI_SYNC_TIMELINE_SYNCOBJ_CUMULATIVE. These are additions only. omacom's kernel integration/aurora doesn't define bit 29, so Mesa keeps its current path there. drm/asahi: G17 (Neo, T8140) driver update for 7.2.11 linux-aurora#229 adds the feature at bit 29.Native acceleration structures are used only on G17P (
hk_g17p_rt_enabledchecksAGX_CHIP_G17P), so the RT changes don't affect other chips.Conflicts, both resolved without losing behaviour:
src/asahi/vulkan/hk_g17p_compression.h: both sides fixed the pNext walk the same way. omacom's form is kept; it also copies the external-memory struct withmemcpy.src/asahi/compiler/test/test-native-form-regressions.cpp: both sides rewrote the image-store data-type test so it no longer depends on identical chip schedules. Aurora's version is kept, because the merged test body around it is Aurora's. Its references keep float and integer separate and check that the two encodings differ.Testing on an aarch64 host:
asahi_clc, which includes the g15g target.agx_tests: 338 tests, 337 pass, 1 skipped; all 53NativeFormstests pass.Update 10-28, fourth merge commit: aurora-wip up to 11005a3 (aurora-silicon/mesa #84, #86–#90).
Conflicts, all resolved without losing behaviour:
src/asahi/clc/asahi_clc.c: the libagx target list keeps omacom'sg15g(T8122) and adds aurora'sg17g.target_chip()already maps both.src/asahi/lib/agx_device.c: keeps omacom'sagx_va_user_limit()(the T8122 runtime reservation) and adds aurora's M5 address-window layout ahead of it.src/asahi/vulkan/hk_g17p_compression.h: takes aurora's comment and its depth/stencil restriction. The pNext walk below it is the same on both sides.The merged tree builds on an aarch64 host with the in-tree
asahi_clc, both libagx targets included. On A18 Pro it passes a sanity run with no GPU errors: deqp-vk api.smoke 6/6 and the gpubench light rows (render, compute, cross-queue timeline).