hk: give G17P host-read and aliased-memory flushes memory scope

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;
 

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);
    }
 

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);
 

src/asahi/vulkan/hk_cmd_buffer.c

@@ -1149,20 +1149,27 @@ 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);
 
    /* 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;

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,

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,