diff --git a/src/asahi/vulkan/hk_cmd_buffer.c b/src/asahi/vulkan/hk_cmd_buffer.c index 49772d7f0d66..2ad7cc1a9149 100644 --- a/src/asahi/vulkan/hk_cmd_buffer.c +++ b/src/asahi/vulkan/hk_cmd_buffer.c @@ -1154,6 +1154,9 @@ hk_CmdPipelineBarrier2(VkCommandBuffer commandBuffer, * scope (hk_cdm_alias_scope()). */ struct hk_cs *cs = cmd->current_cs.cs; const bool alias_scope = hk_cdm_alias_scope(dev); + const bool alias_dep = alias_scope && + (src_kinds & (HK_SYNC_COMPUTE | HK_SYNC_XFER)) && + dst_kinds; if (cs && (src_kinds & (HK_SYNC_COMPUTE | HK_SYNC_XFER)) && dst_kinds && hk_cdm_visibility_flush_needed(dev, cs, alias_scope)) { hk_ensure_cs_has_space(cmd, cs, AGX3_CDM_BARRIER_SIZE); @@ -1162,6 +1165,12 @@ hk_CmdPipelineBarrier2(VkCommandBuffer commandBuffer, hk_sync_barrier(cmd, src_kinds, dst_kinds); + /* The open stream may hold unrelated compute work after the source + * stream ended. Graphics must wait for its alias flush too, even though + * the source-stage dependency alone only reaches the earlier stream. */ + if (alias_dep && cs && cs->stats.cmds) + hk_sync_after_alias_flush(cmd, cs, dst_kinds); + /* 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 @@ -1169,9 +1178,20 @@ hk_CmdPipelineBarrier2(VkCommandBuffer commandBuffer, * 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)) { + * when render work shares the queue. + * + * With alias scope, a dependency recorded while no open compute stream + * holds work (the first barrier of a command buffer, or one after the + * stream ended) has its source work in an earlier stream or command + * buffer, which nothing above flushed with memory scope. Record the + * flush the same way, and order the destination after that stream: a + * reader in the same stream follows the record, a render waits for the + * stream. */ + const bool host_read = + (dev->dev.chip == AGX_CHIP_G17P || agx_chip_g17_m5(dev->dev.chip)) && + hk_dependency_has_host_read(pDependencyInfo); + const bool alias_opener = alias_dep && !(cs && cs->stats.cmds); + if (host_read || alias_opener) { struct hk_sync_dep dep[HK_SYNC_NUM_SQ]; hk_sync_src_dep(cmd, src_kinds, cmd->current_cs.cs, dep); @@ -1183,6 +1203,8 @@ hk_CmdPipelineBarrier2(VkCommandBuffer commandBuffer, hk_ensure_cs_has_space(cmd, flush, AGX3_CDM_BARRIER_SIZE); hk_cdm_visibility_flush(dev, flush, true); + if (alias_opener) + hk_sync_after_stream(cmd, flush, dst_kinds); } /* The dependency this barrier recorded is the one a continued instance diff --git a/src/asahi/vulkan/hk_device.h b/src/asahi/vulkan/hk_device.h index 725bd4ca8b27..e6c8d5b2babf 100644 --- a/src/asahi/vulkan/hk_device.h +++ b/src/asahi/vulkan/hk_device.h @@ -365,23 +365,22 @@ 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 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. 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 - * 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. */ + * command buffers. When no open compute stream holds work, including after + * a timestamp or query copy closed it partway through a command buffer, + * hk_CmdPipelineBarrier2() and hk_CmdSetEvent2() record the flush into a + * stream ordered after the source scope, as a host-read dependency does, + * and order the destination after that stream. When the open stream holds + * work unrelated to the source, the flush goes at its end and a graphics + * destination is ordered after that stream too (hk_sync_after_alias_flush()). + * That the hardware writes those earlier lines back from a flush in a later + * stream 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). + * 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) { diff --git a/src/asahi/vulkan/hk_event.c b/src/asahi/vulkan/hk_event.c index 56117f07a2d5..dd33530b07c4 100644 --- a/src/asahi/vulkan/hk_event.c +++ b/src/asahi/vulkan/hk_event.c @@ -98,15 +98,22 @@ hk_ResetEvent(VkDevice device, VkEvent _event) */ static void hk_event_write(struct hk_cmd_buffer *cmd, struct hk_event *event, - uint8_t src_kinds, uint32_t value) + uint8_t src_kinds, uint32_t value, bool memory_flush) { struct hk_sync_dep dep[HK_SYNC_NUM_SQ]; hk_sync_src_dep(cmd, src_kinds, cmd->current_cs.cs, dep); cmd->sync.kind_override = HK_SYNC_ALL; struct hk_cs *cs = hk_sync_get_cs_after(cmd, dep); - if (cs) + if (cs) { + /* Source work in an earlier stream or command buffer: the memory-scope + * flush goes ahead of the write, in this stream ordered after it. */ + if (memory_flush) { + hk_ensure_cs_has_space(cmd, cs, AGX3_CDM_BARRIER_SIZE); + hk_cdm_visibility_flush(hk_cmd_buffer_device(cmd), cs, true); + } hk_queue_write(cmd, event->bo->va->addr, value, false); + } cmd->sync.kind_override = 0; } @@ -130,13 +137,17 @@ hk_CmdSetEvent2(VkCommandBuffer commandBuffer, VkEvent _event, * 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)) { + const bool alias_dep = (src_kinds & (HK_SYNC_COMPUTE | HK_SYNC_XFER)) && + hk_cdm_alias_scope(dev); + if (cs && alias_dep && 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); + /* No open stream holds the source work (hk_CmdPipelineBarrier2()): the + * flush goes into the stream that carries the write. */ + hk_event_write(cmd, event, src_kinds, VK_EVENT_SET, + alias_dep && !(cs && cs->stats.cmds)); } VKAPI_ATTR void VKAPI_CALL @@ -148,7 +159,8 @@ hk_CmdResetEvent2(VkCommandBuffer commandBuffer, VkEvent _event, VK_FROM_HANDLE(hk_event, event, _event); perf_debug(cmd, "Reset event"); - hk_event_write(cmd, event, hk_sync_kinds_src(stageMask), VK_EVENT_RESET); + hk_event_write(cmd, event, hk_sync_kinds_src(stageMask), VK_EVENT_RESET, + false); } /* A wait polls the event word on the GPU, so a later host vkSetEvent is diff --git a/src/asahi/vulkan/hk_sync.c b/src/asahi/vulkan/hk_sync.c index 80dd841ba96c..c4b89329774b 100644 --- a/src/asahi/vulkan/hk_sync.c +++ b/src/asahi/vulkan/hk_sync.c @@ -694,6 +694,26 @@ hk_sync_after_stream(struct hk_cmd_buffer *cmd, struct hk_cs *cs, cmd->sync.epoch++; } +void +hk_sync_after_alias_flush(struct hk_cmd_buffer *cmd, struct hk_cs *cs, + uint8_t dst_kinds) +{ + assert(cs->type == HK_CS_CDM); + /* Compute and transfer readers follow the flush on the CDM subqueue. + * A source cut at this stream (or a later one) already orders graphics + * after it; preserve that requirement without adding another edge. */ + uint8_t after = 0; + u_foreach_bit(d, dst_kinds & (HK_SYNC_GFX | HK_SYNC_GFX_FRAG | + HK_SYNC_GFX_IMG)) { + struct hk_cs *cut = cmd->sync.req[d][HK_SYNC_CDM].cut; + if (!cut || cut->sync.seq < cs->sync.seq) + after |= BITFIELD_BIT(d); + } + + if (after) + hk_sync_after_stream(cmd, cs, after); +} + static uint8_t stream_kind(struct hk_cmd_buffer *cmd, struct hk_cs **slot, bool compute) { diff --git a/src/asahi/vulkan/hk_sync.h b/src/asahi/vulkan/hk_sync.h index a1d38e6bec03..d5efc3735b3e 100644 --- a/src/asahi/vulkan/hk_sync.h +++ b/src/asahi/vulkan/hk_sync.h @@ -405,6 +405,11 @@ void hk_sync_note_memory_write(struct hk_cmd_buffer *cmd, struct hk_cs *cs); void hk_sync_after_stream(struct hk_cmd_buffer *cmd, struct hk_cs *cs, uint8_t dst_kinds); +/* After recording a memory-scope alias flush, extend only graphics + * requirements that do not already include the stream carrying it. */ +void hk_sync_after_alias_flush(struct hk_cmd_buffer *cmd, struct hk_cs *cs, + uint8_t dst_kinds); + /* Hooks from the control-stream machinery. */ void hk_sync_stream_created(struct hk_cmd_buffer *cmd, struct hk_cs *cs, struct hk_cs **slot, bool compute); diff --git a/src/asahi/vulkan/tests/test-hk-sync-prior.c b/src/asahi/vulkan/tests/test-hk-sync-prior.c index fdc5e2c6e499..a9d255290947 100644 --- a/src/asahi/vulkan/tests/test-hk-sync-prior.c +++ b/src/asahi/vulkan/tests/test-hk-sync-prior.c @@ -412,6 +412,54 @@ xfer_write(struct hk_cmd_buffer *cmd, uint64_t lo, uint64_t hi, bool bda) return cs; } +/* A transfer ends in A; unrelated compute opens B, where the alias flush + * is recorded. Exercise the production ordering helper and resolver without + * a GPU payload. The ordinary source cut alone reaches only A. */ +static void +test_alias_flush_order(void) +{ + const uint8_t gfx_kinds[] = {HK_SYNC_GFX, HK_SYNC_GFX_FRAG, HK_SYNC_GFX_IMG}; + for (unsigned g = 0; g < ARRAY_SIZE(gfx_kinds); ++g) { + const uint8_t dst = gfx_kinds[g]; + struct hk_cmd_buffer *cmd = new_cmd(); + struct hk_cs *a = xfer_write(cmd, 0x10000, 0x20000, false); + struct hk_cs *b = add(cmd, true); + hk_sync_barrier(cmd, HK_SYNC_XFER, dst); + T("transfer source cut excludes unrelated flush stream", + cmd->sync.req[util_logbase2(dst)][HK_SYNC_CDM].cut == a); + hk_sync_after_alias_flush(cmd, b, dst); + T("alias flush extends graphics requirement to B", + cmd->sync.req[util_logbase2(dst)][HK_SYNC_CDM].cut == b); + hk_sync_begin_rendering(cmd, false); + struct hk_cs *render = add(cmd, false); + if (dst == HK_SYNC_GFX_IMG) + hk_sync_vertex_images(cmd, render); + hk_sync_end_rendering(cmd); + T("alias flush ordering resolves", submit(new_queue(true), cmd)); + T("render waits for B's alias flush, not only A", + (dst == HK_SYNC_GFX_IMG ? render->sync.barrier_ta[HK_SYNC_CDM] + : render->sync.barrier[HK_SYNC_CDM]) == + b->sync.index); + + cmd = new_cmd(); + b = xfer_write(cmd, 0x10000, 0x20000, false); + hk_sync_barrier(cmd, HK_SYNC_XFER, dst); + uint32_t epoch = cmd->sync.epoch; + hk_sync_after_alias_flush(cmd, b, dst); + T("flush in source stream adds no ordering", + cmd->sync.epoch == epoch && + cmd->sync.req[util_logbase2(dst)][HK_SYNC_CDM].cut == b); + } + + struct hk_cmd_buffer *cmd = new_cmd(); + xfer_write(cmd, 0x10000, 0x20000, false); + struct hk_cs *b = add(cmd, true); + hk_sync_barrier(cmd, HK_SYNC_XFER, HK_SYNC_COMPUTE | HK_SYNC_XFER); + uint32_t epoch = cmd->sync.epoch; + hk_sync_after_alias_flush(cmd, b, HK_SYNC_COMPUTE | HK_SYNC_XFER); + T("CDM destinations add no ordering", cmd->sync.epoch == epoch); +} + /* G17P transfer writes: only work that may access the written range is * ordered after the transfer, everything else keeps its overlap. */ static void @@ -1272,6 +1320,7 @@ main(void) test_precise_tile_evictions(); test_direct_single_footprints(); test_xfer_write_order(); + test_alias_flush_order(); test_barrier_chains_without_work(); test_multiview_layer_preservation(); test_compute_clear_layer_preservation();