dzn: Changes to descriptor set dirty flag handling
authorJesse Natalie <jenatali@microsoft.com>
Fri, 7 Apr 2023 23:44:53 +0000 (16:44 -0700)
committerMarge Bot <emma+marge@anholt.net>
Mon, 10 Apr 2023 18:43:12 +0000 (18:43 +0000)
The scenario of:
* App binds multiple descriptor sets
* App binds a pipeline that uses a subset of them
* App binds a pipeline that uses more of them

was broken. We were only copying the descriptors for the accessible
subset before, but then clearing all dirty bits, so simply changing
the pipeline wouldn't result in more descriptors being copied.

When running not-bindless, the right thing to do is to copy *all*
descriptors if we're copying any. When running bindless, each parameter
is set separately, and more importantly, *can't* be set on the command
list if the root signature can't access them.

Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/22371>

src/microsoft/vulkan/dzn_cmd_buffer.c
src/microsoft/vulkan/dzn_pipeline.c
src/microsoft/vulkan/dzn_private.h

index a87e3fa..425189c 100644 (file)
@@ -3097,7 +3097,16 @@ dzn_cmd_buffer_update_heaps(struct dzn_cmd_buffer *cmdbuf, uint32_t bindpoint)
    const struct dzn_pipeline *pipeline =
       cmdbuf->state.bindpoint[bindpoint].pipeline;
 
-   if (!(cmdbuf->state.bindpoint[bindpoint].dirty & DZN_CMD_BINDPOINT_DIRTY_HEAPS))
+   /* The set of dirty bits that are cleared by running this function. Notably, 
+    * for bindless, descriptor sets that are bound but unused by the currently
+    * set pipeline are not processed, meaning their dirty bits should persist 
+    * until such a point as a pipeline does use them. For not-bindless,
+    * all sets are processed. */
+   uint32_t dirty_bits_bindless =
+      (pipeline->dynamic_buffer_count ? DZN_CMD_BINDPOINT_DIRTY_DYNAMIC_BUFFERS : 0) |
+      (((DZN_CMD_BINDPOINT_DIRTY_DESC_SET0 << pipeline->set_count) - 1) & DZN_CMD_BINDPOINT_DIRTY_DESC_SETS);
+   uint32_t dirty_bits = (device->bindless ? dirty_bits_bindless : DZN_CMD_BINDPOINT_DIRTY_DESC_SETS | DZN_CMD_BINDPOINT_DIRTY_DYNAMIC_BUFFERS);
+   if (!(cmdbuf->state.bindpoint[bindpoint].dirty & dirty_bits))
       return;
 
    dzn_foreach_pool_type (type) {
@@ -3119,7 +3128,7 @@ dzn_cmd_buffer_update_heaps(struct dzn_cmd_buffer *cmdbuf, uint32_t bindpoint)
          new_heap_offsets[type] = dst_heap_offset;
          update_root_desc_table[type] = true;
 
-         for (uint32_t s = 0; s < cmdbuf->state.pipeline->root.sets_param_count; s++) {
+         for (uint32_t s = 0; s < MAX_SETS; s++) {
             const struct dzn_descriptor_set *set = desc_state->sets[s].set;
             if (!set) continue;
 
@@ -3190,7 +3199,7 @@ dzn_cmd_buffer_update_heaps(struct dzn_cmd_buffer *cmdbuf, uint32_t bindpoint)
    }
 
    if (device->bindless) {
-      for (uint32_t s = 0; s < pipeline->root.sets_param_count; ++s) {
+      for (uint32_t s = 0; s < pipeline->set_count; ++s) {
          const struct dzn_descriptor_set *set = desc_state->sets[s].set;
          if (!set || !set->pool->bindless.buf)
             continue;
@@ -3252,6 +3261,8 @@ dzn_cmd_buffer_update_heaps(struct dzn_cmd_buffer *cmdbuf, uint32_t bindpoint)
                                                                         gpuva);
       }
    }
+
+   cmdbuf->state.bindpoint[bindpoint].dirty &= ~dirty_bits;
 }
 
 static void
@@ -3272,6 +3283,8 @@ dzn_cmd_buffer_update_sysvals(struct dzn_cmd_buffer *cmdbuf, uint32_t bindpoint)
                                                     sizeof(cmdbuf->state.sysvals.compute) / 4,
                                                     &cmdbuf->state.sysvals.compute, 0);
    }
+
+   cmdbuf->state.bindpoint[bindpoint].dirty &= ~DZN_CMD_BINDPOINT_DIRTY_SYSVALS;
 }
 
 static void
@@ -3548,7 +3561,7 @@ dzn_cmd_buffer_prepare_draw(struct dzn_cmd_buffer *cmdbuf, bool indexed)
    dzn_cmd_buffer_update_depth_bounds(cmdbuf);
 
    /* Reset the dirty states */
-   cmdbuf->state.bindpoint[VK_PIPELINE_BIND_POINT_GRAPHICS].dirty = 0;
+   cmdbuf->state.bindpoint[VK_PIPELINE_BIND_POINT_GRAPHICS].dirty &= DZN_CMD_BINDPOINT_DIRTY_HEAPS;
    cmdbuf->state.dirty = 0;
 }
 
@@ -3883,7 +3896,7 @@ dzn_cmd_buffer_prepare_dispatch(struct dzn_cmd_buffer *cmdbuf)
    dzn_cmd_buffer_update_push_constants(cmdbuf, VK_PIPELINE_BIND_POINT_COMPUTE);
 
    /* Reset the dirty states */
-   cmdbuf->state.bindpoint[VK_PIPELINE_BIND_POINT_COMPUTE].dirty = 0;
+   cmdbuf->state.bindpoint[VK_PIPELINE_BIND_POINT_COMPUTE].dirty &= DZN_CMD_BINDPOINT_DIRTY_HEAPS;
 }
 
 VKAPI_ATTR void VKAPI_CALL
index 0033e11..7e24f87 100644 (file)
@@ -1701,6 +1701,7 @@ dzn_pipeline_init(struct dzn_pipeline *pipeline,
 
    STATIC_ASSERT(sizeof(layout->sets) == sizeof(pipeline->sets));
    memcpy(pipeline->sets, layout->sets, sizeof(pipeline->sets));
+   pipeline->set_count = layout->set_count;
    pipeline->dynamic_buffer_count = layout->dynamic_buffer_count;
    vk_object_base_init(&device->vk, &pipeline->base, VK_OBJECT_TYPE_PIPELINE);
 
index 0386bbf..5b8a361 100644 (file)
@@ -359,9 +359,7 @@ enum dzn_cmd_bindpoint_dirty {
    DZN_CMD_BINDPOINT_DIRTY_DESC_SET5 = 1 << 8,
    DZN_CMD_BINDPOINT_DIRTY_DESC_SET6 = 1 << 9,
    DZN_CMD_BINDPOINT_DIRTY_DESC_SET7 = 1 << 10,
-   DZN_CMD_BINDPOINT_DIRTY_HEAPS =
-      DZN_CMD_BINDPOINT_DIRTY_DYNAMIC_BUFFERS |
-      DZN_CMD_BINDPOINT_DIRTY_SYSVALS |
+   DZN_CMD_BINDPOINT_DIRTY_DESC_SETS =
       DZN_CMD_BINDPOINT_DIRTY_DESC_SET0 |
       DZN_CMD_BINDPOINT_DIRTY_DESC_SET1 |
       DZN_CMD_BINDPOINT_DIRTY_DESC_SET2 |
@@ -370,6 +368,10 @@ enum dzn_cmd_bindpoint_dirty {
       DZN_CMD_BINDPOINT_DIRTY_DESC_SET5 |
       DZN_CMD_BINDPOINT_DIRTY_DESC_SET6 |
       DZN_CMD_BINDPOINT_DIRTY_DESC_SET7,
+   DZN_CMD_BINDPOINT_DIRTY_HEAPS =
+      DZN_CMD_BINDPOINT_DIRTY_DYNAMIC_BUFFERS |
+      DZN_CMD_BINDPOINT_DIRTY_SYSVALS |
+      DZN_CMD_BINDPOINT_DIRTY_DESC_SETS,
 };
 
 enum dzn_cmd_dirty {
@@ -920,6 +922,7 @@ struct dzn_pipeline {
       ID3D12RootSignature *sig;
    } root;
    struct dzn_pipeline_layout_set sets[MAX_SETS];
+   uint32_t set_count;
    uint32_t desc_count[NUM_POOL_TYPES];
    uint32_t dynamic_buffer_count;
    ID3D12PipelineState *state;