From ac836598adce52e8bcd3b10f8ca2541ae964078a Mon Sep 17 00:00:00 2001 From: DjDeveloperr Date: Thu, 29 Oct 2026 01:02:24 -0400 Subject: [PATCH 1/6] asahi: ground the memory-scope CDM barrier on G17P The strong CDM barrier (flags 5) makes compute and transfer writes visible only to later reads through the same virtual address, because the G17P caches are tagged by virtual address. A read of the same memory through a second mapping in the same command returns stale memory. That second mapping can be a sparse binding next to a plain buffer over the same pages, or a sparse buffer's read-only shadow. Bit 4 of the barrier word adds memory scope. On a Neo (A18 Pro), one command buffer fills pages through a sparse buffer, records a transfer barrier, then copies the same pool pages through a plain buffer. Pages read back stale, by barrier word: - 0x60001560 (flags 5): 494 of 526 - 0x60001570 (bit 4): 0 of 526 - 0x6000157c (bits 4:2, the HAL300 memory record): 0 of 526 - 0x600215fe (the G15 wide-visibility word): 0 of 526 - 0x60001564 or 0x60001568 (bit 2 or bit 3 alone): 494 and 495 of 526 Bit 4 also fixes the reverse direction, shader stores, and reads of pages the GPU had already cached through the second mapping. Accept the HAL300 record (bits 4:2) on G17P as well, and name the bits. --- src/asahi/genxml/agx3_cmdstream.h | 58 ++++++++++++++++++---- src/asahi/genxml/tests/test-agx3-packing.c | 28 +++++++++-- 2 files changed, 70 insertions(+), 16 deletions(-) diff --git a/src/asahi/genxml/agx3_cmdstream.h b/src/asahi/genxml/agx3_cmdstream.h index f6e00946e455..d9bb0b7dd1e1 100644 --- a/src/asahi/genxml/agx3_cmdstream.h +++ b/src/asahi/genxml/agx3_cmdstream.h @@ -94,8 +94,10 @@ struct agx3_cdm_barrier_info { * 0x600215fe for a nonzero visibility: the ordinary word with 0x2009e * set. */ bool wide_visibility; - /* HAL300 memory visibility also requires the low cache-scope bits. - * The ordering-only record leaves these clear. */ + /* Memory-scope flush (AGX3_CDM_BARRIER_FLUSH_MEMORY): the strong flush + * (flags 5) alone makes writes visible only to later reads through the + * same virtual address. The ordering-only record leaves these bits clear. + * See agx3_cdm_barrier_flush_memory_grounded(). */ bool flush_memory; }; @@ -605,6 +607,39 @@ agx3_cdm_barrier_wide_visibility_grounded(enum agx_chip chip) #define AGX3_CDM_BARRIER_WIDE_VISIBILITY UINT32_C(0x0002009e) +/* Cache-scope bits 4:2 of the CDM barrier word, set only on a strong (flags 5) + * record. Bit 4 writes the GPU's cached data back to memory: the GPU caches + * are tagged by virtual address, so without it a write is visible only to + * later reads through the same mapping, and a read of the same memory + * through another mapping (a sparse binding and a plain buffer over the same + * pages, or a sparse buffer's read-only shadow) returns stale memory. The + * host reads memory, so host availability needs it too. Bits 3:2 are the + * lower cache-scope bits that both Apple-grounded forms set together with + * bit 4: HAL300's memory-visibility record (0x6000157c) and the G15/G16G + * wide-visibility word (AGX3_CDM_BARRIER_WIDE_VISIBILITY, bits 7, 4:1 and + * 17). On G17P, bit 4 alone was enough on hardware and bits 3:2 alone had no + * effect; the record keeps the grounded three-bit form. */ +#define AGX3_CDM_BARRIER_FLUSH_MEMORY UINT32_C(0x0000001c) + +/* Where the memory-scope flush is grounded: HAL300 (G17G/G17S/G17C) from + * Apple's emitter, and G17P (HAL200) from a hardware A/B in which only the + * memory-scope record made a transfer write visible through a second mapping + * within one command (and compute results visible to the host when render + * and compute shared a queue). */ +static inline bool +agx3_cdm_barrier_flush_memory_grounded(enum agx_chip chip) +{ + switch (chip) { + case AGX_CHIP_G17P: + case AGX_CHIP_G17G: + case AGX_CHIP_G17S: + case AGX_CHIP_G17C: + return true; + default: + return false; + } +} + static inline uint32_t agx3_cdm_barrier_word(uint8_t flags) { @@ -628,15 +663,15 @@ agx3_cdm_barrier_pack(const struct agx3_cdm_barrier_info *info, !agx3_cdm_barrier_wide_visibility_grounded(info->chip))) return false; - const bool gen7 = info->chip == AGX_CHIP_G17G || - info->chip == AGX_CHIP_G17S || info->chip == AGX_CHIP_G17C; - if (info->flush_memory && (!gen7 || info->flags != 5)) + if (info->flush_memory && + (!agx3_cdm_barrier_flush_memory_grounded(info->chip) || + info->flags != 5)) return false; agx3_cmdstream_store_le32( packed, agx3_cdm_barrier_word(info->flags) | (info->wide_visibility ? AGX3_CDM_BARRIER_WIDE_VISIBILITY : 0) | - (info->flush_memory ? 0x1c : 0)); + (info->flush_memory ? AGX3_CDM_BARRIER_FLUSH_MEMORY : 0)); return true; } @@ -662,13 +697,14 @@ agx3_cdm_barrier_unpack(enum agx_chip chip, const void *packed, word &= ~AGX3_CDM_BARRIER_WIDE_VISIBILITY; } - const bool flush_memory = (word & 0x1c) == 0x1c; - const bool gen7 = chip == AGX_CHIP_G17G || chip == AGX_CHIP_G17S || - chip == AGX_CHIP_G17C; - if ((word & 0x1c) && (!gen7 || !flush_memory)) + const bool flush_memory = (word & AGX3_CDM_BARRIER_FLUSH_MEMORY) == + AGX3_CDM_BARRIER_FLUSH_MEMORY; + if ((word & AGX3_CDM_BARRIER_FLUSH_MEMORY) && + (!agx3_cdm_barrier_flush_memory_grounded(chip) || !flush_memory)) return false; const uint32_t optional_mask = UINT32_C(0x00001c02) | - (flush_memory ? 0x1c : 0); + (flush_memory ? AGX3_CDM_BARRIER_FLUSH_MEMORY + : 0); if ((word & ~optional_mask) != UINT32_C(0x60000160)) return false; diff --git a/src/asahi/genxml/tests/test-agx3-packing.c b/src/asahi/genxml/tests/test-agx3-packing.c index 05acb2a88feb..6ced98d93e92 100644 --- a/src/asahi/genxml/tests/test-agx3-packing.c +++ b/src/asahi/genxml/tests/test-agx3-packing.c @@ -884,19 +884,37 @@ test_agx3_cdm_barrier(void) } } - /* Native HAL300 full-memory record. Ordering flags alone do not make - * writes visible through another virtual mapping of the same memory. */ - for (unsigned i = 0; i < ARRAY_SIZE(gen7); ++i) { + /* Memory-scope record on HAL300 and G17P. Ordering flags alone do not + * make writes visible through another virtual mapping of the same + * memory. */ + const enum agx_chip memory_scope[] = { + AGX_CHIP_G17G, AGX_CHIP_G17S, AGX_CHIP_G17C, AGX_CHIP_G17P, + }; + for (unsigned i = 0; i < ARRAY_SIZE(memory_scope); ++i) { struct agx3_cdm_barrier_info full = { - .chip = gen7[i], .flags = 5, .flush_memory = true, + .chip = memory_scope[i], .flags = 5, .flush_memory = true, }, decoded; uint8_t packed[AGX3_CDM_BARRIER_SIZE]; + CHECK_EQ(agx3_cdm_barrier_flush_memory_grounded(full.chip), true); CHECK_EQ(agx3_cdm_barrier_pack(&full, packed), true); CHECK_EQ(agx3_cmdstream_load_le32(packed), 0x6000157c); CHECK_EQ(agx3_cdm_barrier_unpack(full.chip, packed, sizeof(packed), &decoded), true); CHECK_EQ(decoded.flush_memory, true); CHECK_EQ(decoded.flags, 5); - full.chip = AGX_CHIP_G17P; + /* Only the strong record takes the memory scope. */ + full.flags = 0; + CHECK_EQ(agx3_cdm_barrier_pack(&full, packed), false); + /* A partial scope is not a grounded record. */ + agx3_cmdstream_store_le32(packed, 0x60001570); + CHECK_EQ(agx3_cdm_barrier_unpack(memory_scope[i], packed, sizeof(packed), &decoded), false); + } + const enum agx_chip no_memory_scope[] = {AGX_CHIP_G15S, AGX_CHIP_G16G}; + for (unsigned i = 0; i < ARRAY_SIZE(no_memory_scope); ++i) { + struct agx3_cdm_barrier_info full = { + .chip = no_memory_scope[i], .flags = 5, .flush_memory = true, + }; + uint8_t packed[AGX3_CDM_BARRIER_SIZE]; + CHECK_EQ(agx3_cdm_barrier_flush_memory_grounded(full.chip), false); CHECK_EQ(agx3_cdm_barrier_pack(&full, packed), false); } From 723037524c24b0a608d8b9d8be808be43d7b234d Mon Sep 17 00:00:00 2001 From: DjDeveloperr Date: Thu, 29 Oct 2026 01:02:43 -0400 Subject: [PATCH 2/6] hk: give G17P host-read and aliased-memory flushes memory scope On G17P a strong CDM flush leaves written data in the GPU caches, which are tagged by virtual address. Two cases depended on more than that. Host availability: a barrier to HOST_READ records hk's strong cache-maintenance flush, which never reached memory on G17P. The end of the command usually wrote the data back before completion was reported, but not when render work shared the queue. gpubench-stress queue.depth runs 32 compute and 32 render submissions on one queue. Compute results were read back stale 1-3 times per 4-second run, in every run. Tracepoints showed each stale submission had finished on the GPU 180-700 us before the host read it, and the kernel had retired it from its completion stamp 35-240 us before the read. The data reached memory 0.1-1.5 ms later. With the render ring on a second queue, or with a memory-scope host flush, three runs each saw no stale result in ~135000 completions. Aliased memory: sparse buffers and images, and capture/replay buffers, map their memory at an address of their own as well. In sparse.churn, a copy through a plain buffer over the pool read stale data for 494 of 526 pages written through the sparse buffer. That happens after a barrier in the same command buffer, whichever direction or kind of write is involved. Splitting the stream into two kernel commands of the same submission did not help. So: - hk_cdm_visibility_flush() records the strong flush with memory scope (AGX3_CDM_BARRIER_FLUSH_MEMORY) on request. It strengthens a trailing strong record in place, as a weak one already is. Inline stream concatenation keeps the scope of the record it folds into. - Host-read availability always asks for memory scope on G17P. - Compute and transfer dependencies ask for it only while the device holds a resource with its own GPU address (hk_cdm_alias_scope()). This covers pipeline barriers, the deferred-clear barrier, the transfer-before-graphics ordering, and vkCmdSetEvent2. Devices that never create such a resource record the same barriers as before. M5 already gives every strong flush memory scope. Other chips are unchanged. In a microbenchmark of 1 and 64 dispatch-plus-barrier pairs per command buffer, the memory-scope form cost nothing measurable: 106-109 us and 167-172 us per command buffer either way. --- src/asahi/vulkan/hk_buffer.c | 6 +++ src/asahi/vulkan/hk_cmd_buffer.c | 22 +++++++---- src/asahi/vulkan/hk_cmd_buffer.h | 36 +++++++++++++++++- src/asahi/vulkan/hk_cmd_dispatch.c | 61 ++++++++++++++++++++++++++---- src/asahi/vulkan/hk_cmd_draw.c | 5 ++- src/asahi/vulkan/hk_device.h | 25 ++++++++++++ src/asahi/vulkan/hk_event.c | 11 ++++++ src/asahi/vulkan/hk_image.c | 5 +++ 8 files changed, 152 insertions(+), 19 deletions(-) diff --git a/src/asahi/vulkan/hk_buffer.c b/src/asahi/vulkan/hk_buffer.c index 6db531756766..a477a3d7089d 100644 --- a/src/asahi/vulkan/hk_buffer.c +++ b/src/asahi/vulkan/hk_buffer.c @@ -315,6 +315,11 @@ hk_CreateBuffer(VkDevice device, const VkBufferCreateInfo *pCreateInfo, } } + /* Bound memory is reached through this buffer's own address as well as + * the memory's, or other resources' (hk_cdm_alias_scope()). */ + if (buffer->va) + p_atomic_inc(&dev->va_alias_resources); + *pBuffer = hk_buffer_to_handle(buffer); return VK_SUCCESS; @@ -336,6 +341,7 @@ hk_DestroyBuffer(VkDevice device, VkBuffer _buffer, if (buffer->va) { agx_va_free(&dev->dev, buffer->va, true); + p_atomic_dec(&dev->va_alias_resources); } hk_release_sparse_scratch(dev, buffer->sparse_scratch_B); diff --git a/src/asahi/vulkan/hk_cmd_buffer.c b/src/asahi/vulkan/hk_cmd_buffer.c index 40460151ddf9..49772d7f0d66 100644 --- a/src/asahi/vulkan/hk_cmd_buffer.c +++ b/src/asahi/vulkan/hk_cmd_buffer.c @@ -1149,12 +1149,15 @@ hk_CmdPipelineBarrier2(VkCommandBuffer commandBuffer, /* Compute work that stays in the open compute stream is ordered by the * stream itself; what it needs at a barrier is visibility. Every native * launch ends in a weak ordering barrier record, and the strong cache - * flush is that same record with Apple's flags, upgraded in place. */ + * flush is that same record with Apple's flags, upgraded in place. While + * memory can be reached through two addresses, the flush needs memory + * scope (hk_cdm_alias_scope()). */ struct hk_cs *cs = cmd->current_cs.cs; + const bool alias_scope = hk_cdm_alias_scope(dev); if (cs && (src_kinds & (HK_SYNC_COMPUTE | HK_SYNC_XFER)) && dst_kinds && - cs->stats.cmds > 0 && cs->current != cs->strong_flush_end) { + hk_cdm_visibility_flush_needed(dev, cs, alias_scope)) { hk_ensure_cs_has_space(cmd, cs, AGX3_CDM_BARRIER_SIZE); - hk_cdm_cache_flush(dev, cs); + hk_cdm_visibility_flush(dev, cs, alias_scope); } hk_sync_barrier(cmd, src_kinds, dst_kinds); @@ -1162,7 +1165,11 @@ hk_CmdPipelineBarrier2(VkCommandBuffer commandBuffer, /* A barrier-only command buffer otherwise records no GPU work, so it * cannot make a preceding fragment store visible to the host. Emit the * native strong CDM cache-maintenance record only when the API requests - * host-read availability, in a stream ordered after the source scope. */ + * host-read availability, in a stream ordered after the source scope. + * The host reads memory, so on G17P the record has memory scope: a strong + * flush alone leaves the data in the GPU caches, and the end of the + * command does not reliably write it back before completion is reported + * when render work shares the queue. */ if ((dev->dev.chip == AGX_CHIP_G17P || agx_chip_g17_m5(dev->dev.chip)) && hk_dependency_has_host_read(pDependencyInfo)) { struct hk_sync_dep dep[HK_SYNC_NUM_SQ]; @@ -1175,7 +1182,7 @@ hk_CmdPipelineBarrier2(VkCommandBuffer commandBuffer, return; hk_ensure_cs_has_space(cmd, flush, AGX3_CDM_BARRIER_SIZE); - hk_cdm_cache_flush(dev, flush); + hk_cdm_visibility_flush(dev, flush, true); } /* The dependency this barrier recorded is the one a continued instance @@ -4450,9 +4457,10 @@ hk_flush_deferred_clears_why(struct hk_cmd_buffer *cmd, const char *why) * order everything recorded from now on after them, as * hk_CmdPipelineBarrier2() does outside a render pass. */ struct hk_cs *cs = cmd->current_cs.cs; - if (cs && cs->stats.cmds > 0 && cs->current != cs->strong_flush_end) { + const bool alias_scope = hk_cdm_alias_scope(dev); + if (cs && hk_cdm_visibility_flush_needed(dev, cs, alias_scope)) { hk_ensure_cs_has_space(cmd, cs, AGX3_CDM_BARRIER_SIZE); - hk_cdm_cache_flush(dev, cs); + hk_cdm_visibility_flush(dev, cs, alias_scope); } hk_sync_barrier(cmd, HK_SYNC_XFER, HK_SYNC_ALL); cmd->state.deferred_clear_flushing = false; diff --git a/src/asahi/vulkan/hk_cmd_buffer.h b/src/asahi/vulkan/hk_cmd_buffer.h index d426bb29b20e..38941cb1eb5c 100644 --- a/src/asahi/vulkan/hk_cmd_buffer.h +++ b/src/asahi/vulkan/hk_cmd_buffer.h @@ -1079,6 +1079,11 @@ struct hk_cs { * emitting two adjacent barriers. */ void *weak_barrier_end; + /* AGX3 CDM: the value of current just after a strong flush that also has + * memory scope (AGX3_CDM_BARRIER_FLUSH_MEMORY). On M5 every strong flush + * has it; on G17P only the flushes hk_cdm_visibility_flush() asks for. */ + void *memory_flush_end; + /* AGX3 CDM: the value of current just after a standalone barrier that is * the stream's *first* record. Apple's emitter only opens a stream with a * barrier (the strong flush that makes a preceding render visible), so a @@ -1388,8 +1393,15 @@ hk_cs_merge_cdm(struct hk_cs *a, const struct hk_cs *b) const size_t b_skip = (b_opens_with_barrier && a_ends_with_barrier) ? AGX3_CDM_BARRIER_SIZE : 0; - if (b_skip) - memcpy(dst - AGX3_CDM_BARRIER_SIZE, b->start, AGX3_CDM_BARRIER_SIZE); + const bool a_memory = (uint8_t *)a->current == (uint8_t *)a->memory_flush_end; + if (b_skip) { + /* b's strong flush must not drop the memory scope a's trailing + * record carried for the work before it. */ + uint32_t word = agx3_cmdstream_load_le32(b->start); + if (a_memory) + word |= AGX3_CDM_BARRIER_FLUSH_MEMORY; + agx3_cmdstream_store_le32(dst - AGX3_CDM_BARRIER_SIZE, word); + } memcpy(dst, (const uint8_t *)b->start + b_skip, b_size - b_skip); a->current = dst + (b_size - b_skip); @@ -1408,6 +1420,12 @@ hk_cs_merge_cdm(struct hk_cs *a, const struct hk_cs *b) ? dst + ((uint8_t *)b->weak_barrier_end - (uint8_t *)b->start) - b_skip : NULL; + a->memory_flush_end = + b->memory_flush_end + ? dst + ((uint8_t *)b->memory_flush_end - (uint8_t *)b->start) - + b_skip + : (a_memory && b_skip && a->current == dst) ? dst + : NULL; a->app_dispatch_end = NULL; } else { void *link_end = @@ -1419,6 +1437,7 @@ hk_cs_merge_cdm(struct hk_cs *a, const struct hk_cs *b) a->stream_linked |= linked || b->stream_linked; a->strong_flush_end = b->strong_flush_end; a->weak_barrier_end = b->weak_barrier_end; + a->memory_flush_end = b->memory_flush_end; a->app_dispatch_end = NULL; } @@ -2530,6 +2549,19 @@ void hk_usc_upload_spilled_rt_descs(struct agx_usc_builder *b, void hk_cdm_cache_flush(struct hk_device *dev, struct hk_cs *cs); +/* As hk_cdm_cache_flush(), for a flush an application dependency asks for. + * With `memory`, the flush also has memory scope on G17P (M5 flushes always + * do): needed for host availability, and for device dependencies once the + * same memory may be reached through more than one virtual address + * (hk_cdm_alias_scope()). A trailing strong flush without memory scope is + * upgraded in place. The caller ensures space for one barrier record. */ +void hk_cdm_visibility_flush(struct hk_device *dev, struct hk_cs *cs, + bool memory); + +/* Whether hk_cdm_visibility_flush(dev, cs, memory) would record anything. */ +bool hk_cdm_visibility_flush_needed(const struct hk_device *dev, + const struct hk_cs *cs, bool memory); + void hk_dispatch_with_usc_launch(struct hk_device *dev, struct hk_cs *cs, struct agx_cdm_launch_word_0_packed launch, uint32_t usc, struct agx_grid grid, diff --git a/src/asahi/vulkan/hk_cmd_dispatch.c b/src/asahi/vulkan/hk_cmd_dispatch.c index 0d183179248a..e51e41fd222b 100644 --- a/src/asahi/vulkan/hk_cmd_dispatch.c +++ b/src/asahi/vulkan/hk_cmd_dispatch.c @@ -542,8 +542,30 @@ hk_cmd_resolve_compute_border(struct hk_cmd_buffer *cmd) } } -void -hk_cdm_cache_flush(struct hk_device *dev, struct hk_cs *cs) +/* Whether a strong flush recorded for `dev` with this request has memory + * scope: always on M5, on request on G17P. */ +static bool +hk_cdm_flush_has_memory_scope(const struct hk_device *dev, bool memory) +{ + return agx_chip_g17_m5(dev->dev.chip) || + (memory && dev->dev.chip == AGX_CHIP_G17P); +} + +bool +hk_cdm_visibility_flush_needed(const struct hk_device *dev, + const struct hk_cs *cs, bool memory) +{ + if (cs->stats.cmds == 0) + return false; + if (cs->current != cs->strong_flush_end) + return true; + /* M5's strong flushes already have memory scope. */ + return memory && dev->dev.chip == AGX_CHIP_G17P && + cs->current != cs->memory_flush_end; +} + +static void +hk_cdm_flush(struct hk_device *dev, struct hk_cs *cs, bool memory) { assert(cs->type == HK_CS_CDM); assert(cs->current + AGX_CDM_BARRIER_LENGTH < cs->end && @@ -557,10 +579,17 @@ hk_cdm_cache_flush(struct hk_device *dev, struct hk_cs *cs) case AGX_CS_GENERATION_G15: case AGX_CS_GENERATION_HAL200: case AGX_CS_GENERATION_HAL300: { + const bool memory_scope = hk_cdm_flush_has_memory_scope(dev, memory); + /* A strong flush already ends the stream: every launch has one in its * tail, and a second barrier with no launch between them is not a - * shape Apple's CDM emitter produces. */ - if (cs->current == cs->strong_flush_end) + * shape Apple's CDM emitter produces. On M5 that flush already has + * memory scope; on G17P a request for it strengthens the record in + * place below. */ + const bool upgrade_strong = cs->current == cs->strong_flush_end; + if (upgrade_strong && + (!memory_scope || agx_chip_g17_m5(dev->dev.chip) || + cs->current == cs->memory_flush_end)) return; /* Apple's CommandEncodingNext emitter uses flags=5 for its strong @@ -571,7 +600,7 @@ hk_cdm_cache_flush(struct hk_device *dev, struct hk_cs *cs) .chip = dev->dev.chip, .flags = 0x5, .wide_visibility = agx_chip_cdm_wide_visibility(dev->dev.chip), - .flush_memory = agx_chip_g17_m5(dev->dev.chip), + .flush_memory = memory_scope, }; uint8_t packed[AGX3_CDM_BARRIER_SIZE]; if (!agx3_cdm_barrier_pack(&info, packed)) { @@ -586,13 +615,16 @@ hk_cdm_cache_flush(struct hk_device *dev, struct hk_cs *cs) * When an API operation immediately needs a cache flush, strengthen * that record in place. Apple's emitter does not produce adjacent weak * and strong barriers, and retaining the weak record buys no ordering - * while making the control stream larger. */ - if (cs->current == cs->weak_barrier_end) { + * while making the control stream larger. A strong record that lacks + * the requested memory scope is strengthened the same way. */ + if (upgrade_strong || cs->current == cs->weak_barrier_end) { memcpy((uint8_t *)cs->current - sizeof(packed), packed, sizeof(packed)); cs->strong_flush_end = cs->current; cs->weak_barrier_end = NULL; - cs->stats.flushes++; + cs->memory_flush_end = memory_scope ? cs->current : NULL; + if (!upgrade_strong) + cs->stats.flushes++; return; } @@ -601,6 +633,7 @@ hk_cdm_cache_flush(struct hk_device *dev, struct hk_cs *cs) cs->current = (uint32_t *)((uint8_t *)cs->current + sizeof(packed)); cs->strong_flush_end = cs->current; cs->weak_barrier_end = NULL; + cs->memory_flush_end = memory_scope ? cs->current : NULL; /* A barrier that opens the stream is the post-graphics strong flush. * It is canonical here and only here; record it so a later inline * concatenation can fold it into the preceding stream's trailing @@ -622,6 +655,18 @@ hk_cdm_cache_flush(struct hk_device *dev, struct hk_cs *cs) cs->stats.flushes++; } +void +hk_cdm_cache_flush(struct hk_device *dev, struct hk_cs *cs) +{ + hk_cdm_flush(dev, cs, false); +} + +void +hk_cdm_visibility_flush(struct hk_device *dev, struct hk_cs *cs, bool memory) +{ + hk_cdm_flush(dev, cs, memory); +} + void hk_dispatch_with_usc_launch(struct hk_device *dev, struct hk_cs *cs, struct agx_cdm_launch_word_0_packed launch, diff --git a/src/asahi/vulkan/hk_cmd_draw.c b/src/asahi/vulkan/hk_cmd_draw.c index ac136901c8e9..81c7c77c5d5e 100644 --- a/src/asahi/vulkan/hk_cmd_draw.c +++ b/src/asahi/vulkan/hk_cmd_draw.c @@ -3558,11 +3558,12 @@ hk_xfer_order(struct hk_cmd_buffer *cmd, struct hk_cs *cs) /* Each launch ends in a weak ordering record; strengthen the last one * into a cache flush, as a barrier does (hk_CmdPipelineBarrier2()). */ + const bool alias_scope = hk_cdm_alias_scope(dev); for (unsigned i = 0; i < n; ++i) { struct hk_cs *w = flush[i]; - if (w->stats.cmds > 0 && w->current != w->strong_flush_end) { + if (hk_cdm_visibility_flush_needed(dev, w, alias_scope)) { hk_ensure_cs_has_space(cmd, w, AGX3_CDM_BARRIER_SIZE); - hk_cdm_cache_flush(dev, w); + hk_cdm_visibility_flush(dev, w, alias_scope); } } diff --git a/src/asahi/vulkan/hk_device.h b/src/asahi/vulkan/hk_device.h index fc4bfa1e5b45..255ce5d9f7ff 100644 --- a/src/asahi/vulkan/hk_device.h +++ b/src/asahi/vulkan/hk_device.h @@ -7,6 +7,7 @@ #pragma once +#include "util/u_atomic.h" #include "util/u_queue.h" #include "asahi/lib/agx_device.h" @@ -126,6 +127,10 @@ struct hk_device { struct agx_device dev; /* Scratch-filled sparse VA (M5: bounded by sparseAddressSpaceSize). */ uint64_t sparse_scratch_B; + /* Live buffers and image planes with a GPU virtual address of their own + * (sparse binding, capture/replay buffers), atomic. Their memory is also + * mapped at another address, see hk_cdm_alias_scope(). */ + uint32_t va_alias_resources; /* One bit per main-heap page: only unbound sparse buffer pages are set. */ struct agx_bo *physical_sparse_map; struct agxdecode_ctx *decode_ctx; @@ -347,6 +352,26 @@ enum hk_perftest { #define HK_PERF(dev, flag) unlikely((dev)->perftest &HK_PERF_##flag) +/* G17P GPU caches are tagged by virtual address: a strong CDM flush makes a + * write visible only to reads through the same mapping. While any resource + * maps memory at a second address, an application dependency between + * compute or transfer work in one stream needs the memory-scope flush + * (hk_cdm_visibility_flush()). Devices that never create such a resource + * keep the ordinary strong flush. + * + * The state is read when a barrier is recorded. A resource the command + * buffer references exists by then, and it counts from creation, so later + * sparse binds are covered; the flush writes back every dirty line, so it + * also covers writes recorded in earlier command buffers. Not covered: a + * resource created after recording and reached only through a device + * address the command buffer reads indirectly. */ +static inline bool +hk_cdm_alias_scope(const struct hk_device *dev) +{ + return dev->dev.chip == AGX_CHIP_G17P && + p_atomic_read(&dev->va_alias_resources) != 0; +} + static inline struct hk_physical_device * hk_device_physical(struct hk_device *dev) { diff --git a/src/asahi/vulkan/hk_event.c b/src/asahi/vulkan/hk_event.c index 72955c19c370..56117f07a2d5 100644 --- a/src/asahi/vulkan/hk_event.c +++ b/src/asahi/vulkan/hk_event.c @@ -125,6 +125,17 @@ hk_CmdSetEvent2(VkCommandBuffer commandBuffer, VkEvent _event, hk_sync_dependency_kinds(pDependencyInfo, hk_sync_image_class(cmd), &src_kinds, &dst_kinds); + /* The wait orders later work behind this stream's source work, but on + * G17P a read through another address of the same memory also needs + * the source writes flushed with memory scope (hk_cdm_alias_scope()). */ + struct hk_device *dev = hk_cmd_buffer_device(cmd); + struct hk_cs *cs = cmd->current_cs.cs; + if (cs && (src_kinds & (HK_SYNC_COMPUTE | HK_SYNC_XFER)) && + hk_cdm_alias_scope(dev) && hk_cdm_visibility_flush_needed(dev, cs, true)) { + hk_ensure_cs_has_space(cmd, cs, AGX3_CDM_BARRIER_SIZE); + hk_cdm_visibility_flush(dev, cs, true); + } + hk_event_write(cmd, event, src_kinds, VK_EVENT_SET); } diff --git a/src/asahi/vulkan/hk_image.c b/src/asahi/vulkan/hk_image.c index dedc3bba71a2..85cb97e1ab94 100644 --- a/src/asahi/vulkan/hk_image.c +++ b/src/asahi/vulkan/hk_image.c @@ -1701,6 +1701,9 @@ hk_image_plane_alloc_vma(struct hk_device *dev, struct hk_image_plane *plane, plane->addr = 0; return result; } + /* Bound memory is reached through this plane's own address as well + * (hk_cdm_alias_scope()); hk_image_plane_finish() drops the count. */ + p_atomic_inc(&dev->va_alias_resources); } if (sparse_resident) { @@ -1734,6 +1737,8 @@ hk_image_plane_finish(struct hk_device *dev, struct hk_image_plane *plane, { if (plane->va) { agx_va_free(&dev->dev, plane->va, true); + plane->va = NULL; + p_atomic_dec(&dev->va_alias_resources); } hk_release_sparse_scratch(dev, plane->sparse_scratch_B); plane->sparse_scratch_B = 0; From 18ff4cdc9a7edf0261842b520816ffd332e8850b Mon Sep 17 00:00:00 2001 From: DjDeveloperr Date: Thu, 29 Oct 2026 03:33:47 -0400 Subject: [PATCH 3/6] hk: test memory scope across CDM stream merges hk_cs_merge_cdm() folds the next stream's opening strong flush into a trailing barrier and must keep the memory scope that record carried, and keep memory_flush_end exact for later merges and cache flushes. The merge test only checked metadata and links, so dropping either would have passed it and brought back stale reads through another mapping. Cover a memory-scoped tail folded with a plain opening barrier, a barrier-only second stream, repeated merges, a memory-scoped opener folded onto a plain tail and an unfolded copy, checking each barrier word and memory_flush_end. With more than one call site the merge stays out of line, so the test stubs hk_drm_attachment_set_add() like the link emitter. The test fails when the fold drops the memory bit, and when a barrier-only fold loses the marker. asahi meson suite: 107 tests pass. (cherry picked from commit c22e446ed8653d9d605dfaf5c01695a4463df876) --- .../tests/test-hk-execution-allowance.c | 135 ++++++++++++++++++ 1 file changed, 135 insertions(+) diff --git a/src/asahi/vulkan/tests/test-hk-execution-allowance.c b/src/asahi/vulkan/tests/test-hk-execution-allowance.c index b784ba021465..c528f9e42413 100644 --- a/src/asahi/vulkan/tests/test-hk-execution-allowance.c +++ b/src/asahi/vulkan/tests/test-hk-execution-allowance.c @@ -20,6 +20,16 @@ hk_cmd_buffer_emit_cs_jump(struct hk_cmd_buffer *cmd, void *current, return (uint8_t *)current + AGX_CDM_STREAM_LINK_LENGTH; } +/* Merged streams here record no attachments; with more than one merge call + * the compiler keeps hk_cs_merge_cdm() out of line, with its attachment loop. */ +bool +hk_drm_attachment_set_add(struct hk_drm_attachment_set *set, uint64_t pointer, + uint64_t size) +{ + assert(!"no attachments are recorded"); + return false; +} + static void check_merge(bool first, bool second, bool linked) { @@ -40,6 +50,130 @@ check_merge(bool first, bool second, bool linked) assert(links == before + linked); } +/* Memory scope across hk_cs_merge_cdm(): a stream whose trailing strong flush + * has memory scope must keep it when the next stream's opening barrier is + * folded into that record, and memory_flush_end must stay exact so a later + * merge or cache flush sees the right tail. */ +#define LAUNCH_BYTES 8 +static const uint32_t plain_flush = UINT32_C(0x60001560); +static const uint32_t memory_flush = UINT32_C(0x60001560) | + AGX3_CDM_BARRIER_FLUSH_MEMORY; + +static uint32_t +word_at(const void *p) +{ + return agx3_cmdstream_load_le32((const uint8_t *)p); +} + +/* A stream holding a launch and then a strong flush, with or without memory + * scope, whose markers point past that flush. */ +static void +init_tail(struct hk_cs *cs, uint8_t *bytes, size_t size, bool memory) +{ + memset(bytes, 0xaa, LAUNCH_BYTES); + agx3_cmdstream_store_le32(bytes + LAUNCH_BYTES, + memory ? memory_flush : plain_flush); + *cs = (struct hk_cs){ + .type = HK_CS_CDM, .start = bytes, + .current = bytes + LAUNCH_BYTES + AGX3_CDM_BARRIER_SIZE, + .end = bytes + size, + }; + cs->strong_flush_end = cs->current; + cs->memory_flush_end = memory ? cs->current : NULL; +} + +/* A stream that opens with a strong flush and, when launch is set, has a + * launch after it. */ +static void +init_opener(struct hk_cs *cs, uint8_t *bytes, size_t size, bool memory, + bool launch) +{ + agx3_cmdstream_store_le32(bytes, memory ? memory_flush : plain_flush); + if (launch) + memset(bytes + AGX3_CDM_BARRIER_SIZE, 0xbb, LAUNCH_BYTES); + *cs = (struct hk_cs){ + .type = HK_CS_CDM, .start = bytes, + .current = bytes + AGX3_CDM_BARRIER_SIZE + (launch ? LAUNCH_BYTES : 0), + .end = bytes + size, + .leading_barrier_end = bytes + AGX3_CDM_BARRIER_SIZE, + }; + if (!launch) { + cs->strong_flush_end = cs->current; + cs->memory_flush_end = memory ? cs->current : NULL; + } +} + +static void +check_merge_memory_scope(void) +{ + uint8_t a_bytes[HK_CS_SPACE_PADDING + 256], b_bytes[64]; + struct hk_cs a, b; + uint8_t *tail = a_bytes + LAUNCH_BYTES; + + assert(agx3_cdm_barrier_word(5) == plain_flush); + + /* A memory-scoped tail folded with a plain opening barrier keeps its + * scope. b's launch then ends the stream, so no memory flush is last. */ + init_tail(&a, a_bytes, sizeof(a_bytes), true); + init_opener(&b, b_bytes, sizeof(b_bytes), false, true); + hk_cs_merge_cdm(&a, &b); + assert(word_at(tail) == memory_flush); + assert(a.current == tail + AGX3_CDM_BARRIER_SIZE + LAUNCH_BYTES); + assert(word_at(tail + AGX3_CDM_BARRIER_SIZE) == 0xbbbbbbbb); + assert(a.memory_flush_end == NULL); + + /* A barrier-only second stream folds away entirely: the scoped record + * stays last, and both markers still point just past it. */ + init_tail(&a, a_bytes, sizeof(a_bytes), true); + init_opener(&b, b_bytes, sizeof(b_bytes), false, false); + hk_cs_merge_cdm(&a, &b); + assert(word_at(tail) == memory_flush); + assert(a.current == tail + AGX3_CDM_BARRIER_SIZE); + assert(a.memory_flush_end == a.current); + assert(a.strong_flush_end == a.current); + + /* Repeated merges: more barrier-only streams keep the scoped record and + * its marker; a stream with a launch then clears the marker, and a later + * plain barrier is appended as its own record without memory scope. */ + for (unsigned i = 0; i < 3; ++i) { + init_opener(&b, b_bytes, sizeof(b_bytes), false, false); + hk_cs_merge_cdm(&a, &b); + assert(word_at(tail) == memory_flush); + assert(a.current == tail + AGX3_CDM_BARRIER_SIZE); + assert(a.memory_flush_end == a.current); + } + init_opener(&b, b_bytes, sizeof(b_bytes), false, true); + hk_cs_merge_cdm(&a, &b); + assert(word_at(tail) == memory_flush); + assert(a.memory_flush_end == NULL); + uint8_t *const after_launch = a.current; + init_opener(&b, b_bytes, sizeof(b_bytes), false, false); + hk_cs_merge_cdm(&a, &b); + assert(a.current == after_launch + AGX3_CDM_BARRIER_SIZE); + assert(word_at(after_launch) == plain_flush); + assert(a.memory_flush_end == NULL); + + /* A memory-scoped opener folded onto a plain tail brings its scope, and + * its marker maps onto the record that absorbed it. */ + init_tail(&a, a_bytes, sizeof(a_bytes), false); + init_opener(&b, b_bytes, sizeof(b_bytes), true, false); + hk_cs_merge_cdm(&a, &b); + assert(word_at(tail) == memory_flush); + assert(a.current == tail + AGX3_CDM_BARRIER_SIZE); + assert(a.memory_flush_end == a.current); + + /* Without a trailing barrier there is no fold: b is copied whole and its + * own scoped record and marker follow it. */ + init_tail(&a, a_bytes, sizeof(a_bytes), false); + a.current = a_bytes + LAUNCH_BYTES; + a.strong_flush_end = NULL; + init_opener(&b, b_bytes, sizeof(b_bytes), true, false); + hk_cs_merge_cdm(&a, &b); + assert(word_at(a_bytes + LAUNCH_BYTES) == memory_flush); + assert(a.current == a_bytes + LAUNCH_BYTES + AGX3_CDM_BARRIER_SIZE); + assert(a.memory_flush_end == a.current); +} + static void check_as_contracts(void) { @@ -98,5 +232,6 @@ int main(void) for (unsigned b = 0; b < 2; ++b) for (unsigned link = 0; link < 2; ++link) check_merge(a, b, link); + check_merge_memory_scope(); return 0; } From 7c8c738ca0ab3e12ca250a58cb665c137d09a2e6 Mon Sep 17 00:00:00 2001 From: DjDeveloperr Date: Sat, 31 Oct 2026 01:04:08 -0400 Subject: [PATCH 4/6] hk: say when the G17P alias-scope flush is recorded The comment on hk_cdm_alias_scope() said the memory-scope flush also covers writes recorded in earlier command buffers. It does when a later barrier records the flush: hk_CmdPipelineBarrier2() records it into the open compute stream, and only when that stream already holds work (hk_cdm_visibility_flush_needed() returns false at stats.cmds == 0). A barrier recorded before any work in a command buffer has no such stream and records no flush. Whether the hardware returns stale data then is untested; the comment now says so instead of claiming coverage. The "not covered" case also includes a descriptor set updated after bind to reach a resource created after recording (descriptorBinding*UpdateAfterBind is advertised). From the review of omacom/mesa#20. Co-Authored-By: Claude Opus 5.5 (cherry picked from commit 682223a8a9ae1e61ffff6f5d91c2f04845da66f4) --- src/asahi/vulkan/hk_device.h | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/src/asahi/vulkan/hk_device.h b/src/asahi/vulkan/hk_device.h index 255ce5d9f7ff..b9a184d9f30d 100644 --- a/src/asahi/vulkan/hk_device.h +++ b/src/asahi/vulkan/hk_device.h @@ -361,10 +361,19 @@ enum hk_perftest { * * The state is read when a barrier is recorded. A resource the command * buffer references exists by then, and it counts from creation, so later - * sparse binds are covered; the flush writes back every dirty line, so it - * also covers writes recorded in earlier command buffers. Not covered: a - * resource created after recording and reached only through a device - * address the command buffer reads indirectly. */ + * sparse binds are covered. The flush goes into the open compute stream, + * and only when that stream already holds work + * (hk_cdm_visibility_flush_needed()). It writes back every dirty line, so + * the writes it covers include those of earlier streams and earlier + * command buffers. A barrier recorded before any work in a command buffer + * has no such stream and records no flush: a write in an earlier command + * buffer, read through another address by the first work after that + * barrier, has no memory-scope flush between them. Whether the hardware + * returns stale data in that case has not been tested. Not covered: a + * resource created after the command buffer was recorded and reached + * through a path recording did not see, that is a device address the + * command buffer reads indirectly, or a descriptor set bound at recording + * and updated after it (descriptorBinding*UpdateAfterBind). */ static inline bool hk_cdm_alias_scope(const struct hk_device *dev) { From d9345510b19f4acfed3bea967a49f636f03d83b9 Mon Sep 17 00:00:00 2001 From: DjDeveloperr Date: Sat, 31 Oct 2026 12:29:12 -0400 Subject: [PATCH 5/6] hk: clarify G17P alias-scope gaps after stream closure Name barriers after a compute stream closes as well as command-buffer openers. Also document the untested graphics-only source case, which keeps the ordinary strong flush. No behaviour changes. (cherry picked from commit 1498a030ff6f750a8d56fbe907be5944f9f5721d) --- src/asahi/vulkan/hk_device.h | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/asahi/vulkan/hk_device.h b/src/asahi/vulkan/hk_device.h index b9a184d9f30d..25d7a2203a14 100644 --- a/src/asahi/vulkan/hk_device.h +++ b/src/asahi/vulkan/hk_device.h @@ -365,15 +365,20 @@ enum hk_perftest { * and only when that stream already holds work * (hk_cdm_visibility_flush_needed()). It writes back every dirty line, so * the writes it covers include those of earlier streams and earlier - * command buffers. A barrier recorded before any work in a command buffer - * has no such stream and records no flush: a write in an earlier command - * buffer, read through another address by the first work after that - * barrier, has no memory-scope flush between them. Whether the hardware + * command buffers. A barrier recorded with no open compute stream holding + * work records no flush. This includes the first barrier of a command + * buffer and barriers after the stream was closed, for example by a + * timestamp or query copy. Earlier writes read through another address + * after such a barrier have no memory-scope flush between them. Whether the hardware * returns stale data in that case has not been tested. Not covered: a * resource created after the command buffer was recorded and reached * through a path recording did not see, that is a device address the * command buffer reads indirectly, or a descriptor set bound at recording - * and updated after it (descriptorBinding*UpdateAfterBind). */ + * and updated after it (descriptorBinding*UpdateAfterBind). Graphics-only + * sources also do not request memory scope: attachment or shader writes + * through a sparse image followed by transfer or compute reads through a + * plain alias receive the ordinary post-graphics strong flush. That case + * has not been hardware-tested. */ static inline bool hk_cdm_alias_scope(const struct hk_device *dev) { From 75238e525bd7cd87b4fc50aebaea68a46aa5afec Mon Sep 17 00:00:00 2001 From: DjDeveloperr Date: Sat, 31 Oct 2026 12:49:34 -0400 Subject: [PATCH 6/6] hk: name the unrelated-stream gap in the G17P alias-scope comment When the open compute stream holds work unrelated to the source, the memory-scope flush goes at its end, but a graphics destination is still ordered only after the source's stream, so it can start before the flush. Say so, so the comment does not imply that every recorded alias flush orders its graphics reader. Also rewrap the one line past 80 columns. Comment only. Raised in review of omacom/mesa#20. Co-Authored-By: Claude Opus 5.5 (cherry picked from commit f1a1e8be52405ddc1ab93a5b2b393f37baa648a5) --- src/asahi/vulkan/hk_device.h | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/asahi/vulkan/hk_device.h b/src/asahi/vulkan/hk_device.h index 25d7a2203a14..725bd4ca8b27 100644 --- a/src/asahi/vulkan/hk_device.h +++ b/src/asahi/vulkan/hk_device.h @@ -369,8 +369,11 @@ enum hk_perftest { * work records no flush. This includes the first barrier of a command * buffer and barriers after the stream was closed, for example by a * timestamp or query copy. Earlier writes read through another address - * after such a barrier have no memory-scope flush between them. Whether the hardware - * returns stale data in that case has not been tested. Not covered: a + * after such a barrier have no memory-scope flush between them. Whether + * the hardware returns stale data in that case has not been tested. When + * the open stream holds work unrelated to the source, the flush goes at its + * end, but a graphics destination is still ordered only after the source's + * stream, so it can start before the flush. Not covered: a * resource created after the command buffer was recorded and reached * through a path recording did not see, that is a device address the * command buffer reads indirectly, or a descriptor set bound at recording