Mesa (staging/21.2): d3d12: split up root parameter update and set

[GitLab Mirror]

Module: Mesa
Branch: staging/21.2
Commit: 432964005beca2a18d202995b40f897df34f8dc1
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=432964005beca2a18d202995b40f897df34f8dc1

Author: Erik Faye-Lund <erik.faye-lund at collabora.com>
Date:   Tue Aug  3 17:08:44 2021 +0200

d3d12: split up root parameter update and set

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 at microsoft.com>
Reviewed-by: Bill Kristiansen <billkris at microsoft.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12187>
(cherry picked from commit cd79351f02af898b0b01997f0b8b77007ed88135)

---

 .pick_status.json                        |  2 +-
 src/gallium/drivers/d3d12/d3d12_draw.cpp | 42 +++++++++++++++++++++++---------
 2 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/.pick_status.json b/.pick_status.json
index ba540c59be3..7b8c3796ee2 100644
--- a/.pick_status.json
+++ b/.pick_status.json
@@ -67,7 +67,7 @@
         "description": "d3d12: split up root parameter update and set",
         "nominated": true,
         "nomination_type": 1,
-        "resolution": 0,
+        "resolution": 1,
         "main_sha": null,
         "because_sha": "12082905582fe291da939f024661f2c505111dc2"
     },
diff --git a/src/gallium/drivers/d3d12/d3d12_draw.cpp b/src/gallium/drivers/d3d12/d3d12_draw.cpp
index c4efa7bd87e..1a2f0b98c44 100644
--- a/src/gallium/drivers/d3d12/d3d12_draw.cpp
+++ b/src/gallium/drivers/d3d12/d3d12_draw.cpp
@@ -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,