tu: Fix context faults loading unused descriptor sets
authorConnor Abbott <cwabbott0@gmail.com>
Tue, 9 Jun 2020 12:40:58 +0000 (14:40 +0200)
committerMarge Bot <eric+marge@anholt.net>
Tue, 9 Jun 2020 15:35:29 +0000 (15:35 +0000)
The app is allowed to never bind descriptor sets that are statically
unused by the pipeline, which would've caused a context fault since
CP_LOAD_STATE6 would try to load the descriptors that don't exist. Fix
this by not preloading descriptors from unused descriptor sets. We could
do more fine-grained accounting of which descriptors are used, but this
is enough to fix the problem.

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

src/freedreno/vulkan/tu_pipeline.c
src/freedreno/vulkan/tu_private.h
src/freedreno/vulkan/tu_shader.c

index 9ddcf61e2318f127d56e75aa15de953607582cc6..084ce412466abd0fe1dc4d72bc7247346ca83314 100644 (file)
@@ -185,6 +185,25 @@ tu6_emit_load_state(struct tu_pipeline *pipeline, bool compute)
 
    struct tu_pipeline_layout *layout = pipeline->layout;
    for (unsigned i = 0; i < layout->num_sets; i++) {
+      /* From 13.2.7. Descriptor Set Binding:
+       *
+       *    A compatible descriptor set must be bound for all set numbers that
+       *    any shaders in a pipeline access, at the time that a draw or
+       *    dispatch command is recorded to execute using that pipeline.
+       *    However, if none of the shaders in a pipeline statically use any
+       *    bindings with a particular set number, then no descriptor set need
+       *    be bound for that set number, even if the pipeline layout includes
+       *    a non-trivial descriptor set layout for that set number.
+       *
+       * This means that descriptor sets unused by the pipeline may have a
+       * garbage or 0 BINDLESS_BASE register, which will cause context faults
+       * when prefetching descriptors from these sets. Skip prefetching for
+       * descriptors from them to avoid this. This is also an optimization,
+       * since these prefetches would be useless.
+       */
+      if (!(pipeline->active_desc_sets & (1u << i)))
+         continue;
+
       struct tu_descriptor_set_layout *set_layout = layout->set[i].layout;
       for (unsigned j = 0; j < set_layout->binding_count; j++) {
          struct tu_descriptor_set_binding_layout *binding = &set_layout->binding[j];
@@ -2273,6 +2292,7 @@ tu_pipeline_builder_parse_shader_stages(struct tu_pipeline_builder *builder,
    }
    pipeline->active_stages = stages;
 
+   uint32_t desc_sets = 0;
    for (unsigned i = 0; i < MESA_SHADER_STAGES; i++) {
       if (!builder->shaders[i])
          continue;
@@ -2280,7 +2300,9 @@ tu_pipeline_builder_parse_shader_stages(struct tu_pipeline_builder *builder,
       tu_pipeline_set_linkage(&pipeline->program.link[i],
                               builder->shaders[i],
                               &builder->shaders[i]->variants[0]);
+      desc_sets |= builder->shaders[i]->active_desc_sets;
    }
+   pipeline->active_desc_sets = desc_sets;
 
    if (builder->shaders[MESA_SHADER_FRAGMENT]) {
       memcpy(pipeline->program.input_attachment_idx,
index da89067448548e7182197ed648a3a098ccd05740..768034236291029e292ec9be9b6225a7a36939fa 100644 (file)
@@ -1289,6 +1289,7 @@ struct tu_shader
 
    struct tu_push_constant_range push_consts;
    unsigned attachment_idx[MAX_RTS];
+   uint8_t active_desc_sets;
 
    /* This may be true for vertex shaders.  When true, variants[1] is the
     * binning variant and binning_binary is non-NULL.
@@ -1345,6 +1346,7 @@ struct tu_pipeline
 
    bool need_indirect_descriptor_sets;
    VkShaderStageFlags active_stages;
+   uint32_t active_desc_sets;
 
    struct tu_streamout_state streamout;
 
index 29ac80bc48db4ebb0087bbb162b87b037c4d2573..d26154a3b02579d1a41de194b946cf9d4b94e098 100644 (file)
@@ -135,6 +135,8 @@ lower_vulkan_resource_index(nir_builder *b, nir_intrinsic_instr *instr,
       &set_layout->binding[binding];
    uint32_t base;
 
+   shader->active_desc_sets |= 1u << set;
+
    switch (binding_layout->type) {
    case VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC:
    case VK_DESCRIPTOR_TYPE_STORAGE_BUFFER_DYNAMIC:
@@ -175,6 +177,8 @@ build_bindless(nir_builder *b, nir_deref_instr *deref, bool is_sampler,
    const struct tu_descriptor_set_binding_layout *bind_layout =
       &layout->set[set].layout->binding[binding];
 
+   shader->active_desc_sets |= 1u << set;
+
    nir_ssa_def *desc_offset;
    unsigned descriptor_stride;
    if (bind_layout->type == VK_DESCRIPTOR_TYPE_INPUT_ATTACHMENT) {