d3d12: split up root parameter update and set
authorErik Faye-Lund <erik.faye-lund@collabora.com>
Tue, 3 Aug 2021 15:08:44 +0000 (17:08 +0200)
committerMarge Bot <eric+marge@anholt.net>
Tue, 3 Aug 2021 16:17:11 +0000 (16:17 +0000)
SRV descriptors can require state-transitions before it's legal to set
them on the command-list. We used to just set them right away, and get
away with is, because the validator didn't verify this because we used
to flag the parameters as volatile.

Now that we don't, we trigger validation errors when setting a root
parameter that needs a transition first.

So let's split up the logic a bit, so we can prepare the tables, then do
the transision, and finally set the tables. We do this for all tables
instead of just the SRVs, just because it makes the logic a bit easier to
follow. We leave root constants alone, because they will never require
this, and doing them late would just compilcate things.

Fixes: 12082905582 ("d3d12: Sets all SRV descriptors as data-static")
Reviewed-by: Jesse Natalie <jenatali@microsoft.com>
Reviewed-by: Bill Kristiansen <billkris@microsoft.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12187>

src/gallium/drivers/d3d12/d3d12_draw.cpp

index c4efa7b..1a2f0b9 100644 (file)
@@ -243,12 +243,17 @@ check_descriptors_left(struct d3d12_context *ctx)
    return true;
 }
 
-static void
-set_graphics_root_parameters(struct d3d12_context *ctx,
-                             const struct pipe_draw_info *dinfo,
-                             const struct pipe_draw_start_count_bias *draw)
+#define MAX_DESCRIPTOR_TABLES (D3D12_GFX_SHADER_STAGES * 3)
+
+static unsigned
+update_graphics_root_parameters(struct d3d12_context *ctx,
+                                const struct pipe_draw_info *dinfo,
+                                const struct pipe_draw_start_count_bias *draw,
+                                D3D12_GPU_DESCRIPTOR_HANDLE root_desc_tables[MAX_DESCRIPTOR_TABLES],
+                                int root_desc_indices[MAX_DESCRIPTOR_TABLES])
 {
    unsigned num_params = 0;
+   unsigned num_root_desciptors = 0;
 
    for (unsigned i = 0; i < D3D12_GFX_SHADER_STAGES; ++i) {
       if (!ctx->gfx_stages[i])
@@ -260,16 +265,25 @@ set_graphics_root_parameters(struct d3d12_context *ctx,
       assert(shader);
 
       if (shader->num_cb_bindings > 0) {
-         if (dirty & D3D12_SHADER_DIRTY_CONSTBUF)
-            ctx->cmdlist->SetGraphicsRootDescriptorTable(num_params, fill_cbv_descriptors(ctx, shader, i));
+         if (dirty & D3D12_SHADER_DIRTY_CONSTBUF) {
+            assert(num_root_desciptors < MAX_DESCRIPTOR_TABLES);
+            root_desc_tables[num_root_desciptors] = fill_cbv_descriptors(ctx, shader, i);
+            root_desc_indices[num_root_desciptors++] = num_params;
+         }
          num_params++;
       }
       if (shader->end_srv_binding > 0) {
-         if (dirty & D3D12_SHADER_DIRTY_SAMPLER_VIEWS)
-            ctx->cmdlist->SetGraphicsRootDescriptorTable(num_params, fill_srv_descriptors(ctx, shader, i));
+         if (dirty & D3D12_SHADER_DIRTY_SAMPLER_VIEWS) {
+            assert(num_root_desciptors < MAX_DESCRIPTOR_TABLES);
+            root_desc_tables[num_root_desciptors] = fill_srv_descriptors(ctx, shader, i);
+            root_desc_indices[num_root_desciptors++] = num_params;
+         }
          num_params++;
-         if (dirty & D3D12_SHADER_DIRTY_SAMPLERS)
-            ctx->cmdlist->SetGraphicsRootDescriptorTable(num_params, fill_sampler_descriptors(ctx, shader_sel, i));
+         if (dirty & D3D12_SHADER_DIRTY_SAMPLERS) {
+            assert(num_root_desciptors < MAX_DESCRIPTOR_TABLES);
+            root_desc_tables[num_root_desciptors] = fill_sampler_descriptors(ctx, shader_sel, i);
+            root_desc_indices[num_root_desciptors++] = num_params;
+         }
          num_params++;
       }
       /* TODO Don't always update state vars */
@@ -280,6 +294,7 @@ set_graphics_root_parameters(struct d3d12_context *ctx,
          num_params++;
       }
    }
+   return num_root_desciptors;
 }
 
 static bool
@@ -580,7 +595,9 @@ d3d12_draw_vbo(struct pipe_context *pctx,
       ctx->cmdlist->SetPipelineState(ctx->current_pso);
    }
 
-   set_graphics_root_parameters(ctx, dinfo, &draws[0]);
+   D3D12_GPU_DESCRIPTOR_HANDLE root_desc_tables[MAX_DESCRIPTOR_TABLES];
+   int root_desc_indices[MAX_DESCRIPTOR_TABLES];
+   unsigned num_root_desciptors = update_graphics_root_parameters(ctx, dinfo, &draws[0], root_desc_tables, root_desc_indices);
 
    bool need_zero_one_depth_range = d3d12_need_zero_one_depth_range(ctx);
    if (need_zero_one_depth_range != ctx->need_zero_one_depth_range) {
@@ -718,6 +735,9 @@ d3d12_draw_vbo(struct pipe_context *pctx,
 
    d3d12_apply_resource_states(ctx);
 
+   for (unsigned i = 0; i < num_root_desciptors; ++i)
+      ctx->cmdlist->SetGraphicsRootDescriptorTable(root_desc_indices[i], root_desc_tables[i]);
+
    if (dinfo->index_size > 0)
       ctx->cmdlist->DrawIndexedInstanced(draws[0].count, dinfo->instance_count,
                                          draws[0].start, draws[0].index_bias,