tu: Follow pipeline compatibility rules for dynamic descriptors

When we bind a descriptor set with dynamic descriptors, we can't ignore
dynamic descriptors in previously-bound higher descriptor sets. For
example, assume we have descriptor sets A and B, each of which has one
dynamic storage buffer, and we do:

CmdBindDescriptorSets(firstSet=1, descriptorSetCount=1, A)
CmdBindDescriptorSets(firstSet=0, descriptorSetCount=1, B)

and in the first CmdBindDescriptorSets the pipeline layout includes a
descriptor set layout compatible with B in set 0. Then, following
"Pipeline Layout Compatibility," set 0 is disturbed:

   When binding a descriptor set to set number N, a previously bound
   descriptor set bound with lower index M than N is disturbed if the
   pipeline layouts for set M and N are not compatible for set M.
   Otherwise, the bound descriptor set in M is not disturbed

When it's disturbed, it's effectively turned into a set with 1 undefined
dynamic storage buffer:

   When a descriptor set is disturbed by binding descriptor sets, the
   disturbed set is considered to contain undefined descriptors bound
   with the same pipeline layout as the disturbing descriptor set.

This disturbed set is compatible with B, so in the second
CmdBindDescriptorSets this clause doesn't apply:

   If, additionally, the previously bound descriptor set for set N was
   bound using a pipeline layout not compatible for set N, then all
   bindings in sets numbered greater than N are disturbed.

and A remains valid to access. The code before 88db7364 worked only if
the pipeline layout when binding B contained a descriptor layout
compatible with A in set 1, because it used the pipeline layout's total
size when allocating the internal dynamic descriptors array, but that
isn't actually a requirement, so the previous code was already broken.
After 88db7364 we only allocate as much space as required by the current
descriptors being bound, because I misread the rules here, which made it
more broken and broke 3DMark Wildlife Extreme that does something like
this.

In order to properly fix this we need to keep track of the maximum ever
seen dynamic descriptor size, similar to what we already do for
descriptor sets, and use that. We have no idea what needs to be
preserved when binding a descriptor set with dynamic descriptors, so we
have to be conservative.

Fixes: 88db7364 ("tu: Rework dynamic offset handling")
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/27750>
This commit is contained in:
Connor Abbott
2024-02-22 13:58:11 +00:00
committed by Marge Bot
parent 8c86690072
commit db0291c235
2 changed files with 14 additions and 10 deletions
+13 -9
View File
@@ -2151,7 +2151,7 @@ tu_reset_cmd_buffer(struct vk_command_buffer *vk_cmd_buffer,
memset(&cmd_buffer->descriptors[i].push_set, 0, sizeof(cmd_buffer->descriptors[i].push_set));
cmd_buffer->descriptors[i].push_set.base.type = VK_OBJECT_TYPE_DESCRIPTOR_SET;
cmd_buffer->descriptors[i].max_sets_bound = 0;
cmd_buffer->descriptors[i].dynamic_bound = 0;
cmd_buffer->descriptors[i].max_dynamic_offset_size = 0;
}
u_trace_fini(&cmd_buffer->trace);
@@ -2441,12 +2441,12 @@ tu6_emit_descriptor_sets(struct tu_cmd_buffer *cmd,
cmd->state.desc_sets =
tu_cs_draw_state(&cmd->sub_cs, &state_cs,
4 + 4 * descriptors_state->max_sets_bound +
(descriptors_state->dynamic_bound ? 6 : 0));
(descriptors_state->max_dynamic_offset_size ? 6 : 0));
} else {
cmd->state.desc_sets =
tu_cs_draw_state(&cmd->sub_cs, &state_cs,
3 + 2 * descriptors_state->max_sets_bound +
(descriptors_state->dynamic_bound ? 3 : 0));
(descriptors_state->max_dynamic_offset_size ? 3 : 0));
}
cs = &state_cs;
} else {
@@ -2466,7 +2466,7 @@ tu6_emit_descriptor_sets(struct tu_cmd_buffer *cmd,
}
/* Dynamic descriptors get the reserved descriptor set. */
if (descriptors_state->dynamic_bound) {
if (descriptors_state->max_dynamic_offset_size) {
int reserved_set_idx = cmd->device->physical_device->reserved_set_idx;
assert(reserved_set_idx >= 0); /* reserved set must be bound */
@@ -2617,22 +2617,26 @@ tu_CmdBindDescriptorSets(VkCommandBuffer commandBuffer,
assert(dyn_idx == dynamicOffsetCount);
if (dynamic_offset_offset) {
descriptors_state->max_dynamic_offset_size =
MAX2(descriptors_state->max_dynamic_offset_size, dynamic_offset_offset);
/* allocate and fill out dynamic descriptor set */
struct tu_cs_memory dynamic_desc_set;
int reserved_set_idx = cmd->device->physical_device->reserved_set_idx;
VkResult result = tu_cs_alloc(&cmd->sub_cs,
dynamic_offset_offset / (4 * A6XX_TEX_CONST_DWORDS),
A6XX_TEX_CONST_DWORDS, &dynamic_desc_set);
VkResult result =
tu_cs_alloc(&cmd->sub_cs,
descriptors_state->max_dynamic_offset_size /
(4 * A6XX_TEX_CONST_DWORDS),
A6XX_TEX_CONST_DWORDS, &dynamic_desc_set);
if (result != VK_SUCCESS) {
vk_command_buffer_set_error(&cmd->vk, result);
return;
}
memcpy(dynamic_desc_set.map, descriptors_state->dynamic_descriptors,
dynamic_offset_offset);
descriptors_state->max_dynamic_offset_size);
assert(reserved_set_idx >= 0); /* reserved set must be bound */
descriptors_state->set_iova[reserved_set_idx] = dynamic_desc_set.iova | BINDLESS_DESCRIPTOR_64B;
descriptors_state->dynamic_bound = true;
}
tu_dirty_desc_sets(cmd, pipelineBindPoint);
+1 -1
View File
@@ -54,7 +54,7 @@ struct tu_descriptor_state
uint32_t dynamic_descriptors[MAX_DYNAMIC_BUFFERS_SIZE];
uint64_t set_iova[MAX_SETS];
uint32_t max_sets_bound;
bool dynamic_bound;
uint32_t max_dynamic_offset_size;
};
enum tu_cmd_dirty_bits