From 432964005beca2a18d202995b40f897df34f8dc1 Mon Sep 17 00:00:00 2001 From: Erik Faye-Lund Date: Tue, 3 Aug 2021 17:08:44 +0200 Subject: [PATCH] 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 Reviewed-by: Bill Kristiansen Part-of: (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,