diff --git a/.pick_status.json b/.pick_status.json index d89eaf76568..e852e5d1be9 100644 --- a/.pick_status.json +++ b/.pick_status.json @@ -314,7 +314,7 @@ "description": "intel: fix null render target setup logic", "nominated": true, "nomination_type": 2, - "resolution": 0, + "resolution": 1, "main_sha": null, "because_sha": "98cdb9349a7fa181c3895655d217589f909a7beb", "notes": null diff --git a/src/intel/compiler/brw_nir.h b/src/intel/compiler/brw_nir.h index 95acf75930f..d520b14a7e7 100644 --- a/src/intel/compiler/brw_nir.h +++ b/src/intel/compiler/brw_nir.h @@ -154,6 +154,13 @@ brw_nir_fs_needs_null_rt(const struct intel_device_info *devinfo, if (devinfo->ver < 11) return true; + /* Depth/Stencil needs a valid render target even if there is no color + * output. + */ + if (nir->info.outputs_written & (BITFIELD_BIT(FRAG_RESULT_DEPTH) | + BITFIELD_BIT(FRAG_RESULT_STENCIL))) + return true; + uint64_t relevant_outputs = 0; if (multisample_fbo) relevant_outputs |= BITFIELD64_BIT(FRAG_RESULT_SAMPLE_MASK); diff --git a/src/intel/vulkan/anv_pipeline.c b/src/intel/vulkan/anv_pipeline.c index 78204082d72..aa3a09a9411 100644 --- a/src/intel/vulkan/anv_pipeline.c +++ b/src/intel/vulkan/anv_pipeline.c @@ -456,15 +456,6 @@ rp_color_mask(const struct vk_render_pass_state *rp) color_mask |= BITFIELD_BIT(i); } - /* If there is depth/stencil attachment, even if the fragment shader - * doesn't write the depth/stencil output, we need a valid render target so - * that the compiler doesn't use the null-rt which would cull the - * depth/stencil output. - */ - if (rp->depth_attachment_format != VK_FORMAT_UNDEFINED || - rp->stencil_attachment_format != VK_FORMAT_UNDEFINED) - color_mask |= 1; - return color_mask; } @@ -1481,23 +1472,13 @@ anv_pipeline_link_fs(const struct brw_compiler *compiler, const struct vk_render_pass_state *rp) { /* Initially the valid outputs value is set to all possible render targets - * valid (see populate_wm_prog_key()), before we look at the shader - * variables. Here we look at the output variables of the shader an compute - * a correct number of render target outputs. + * valid (see populate_wm_prog_key()), because we're not looking at the + * shader code yet. Here we look at the output written to get a correct + * number of render target outputs. */ - stage->key.wm.color_outputs_valid = 0; - nir_foreach_shader_out_variable_safe(var, stage->nir) { - if (var->data.location < FRAG_RESULT_DATA0) - continue; - - const unsigned rt = var->data.location - FRAG_RESULT_DATA0; - const unsigned array_len = - glsl_type_is_array(var->type) ? glsl_get_length(var->type) : 1; - assert(rt + array_len <= MAX_RTS); - - stage->key.wm.color_outputs_valid |= BITFIELD_RANGE(rt, array_len); - } - stage->key.wm.color_outputs_valid &= rp_color_mask(rp); + const uint64_t rt_mask = + stage->nir->info.outputs_written >> FRAG_RESULT_DATA0; + stage->key.wm.color_outputs_valid = rt_mask & rp_color_mask(rp); stage->key.wm.nr_color_regions = util_last_bit(stage->key.wm.color_outputs_valid); @@ -1527,6 +1508,9 @@ anv_pipeline_link_fs(const struct brw_compiler *compiler, compiler->devinfo, stage->nir, stage->key.wm.multisample_fbo != INTEL_NEVER, stage->key.wm.alpha_to_coverage != INTEL_NEVER)) { + /* Ensure the shader doesn't discard the writes */ + stage->key.wm.color_outputs_valid = 0x1; + stage->key.wm.nr_color_regions = 1; /* Setup a null render target */ rt_bindings[0] = (struct anv_pipeline_binding) { .set = ANV_DESCRIPTOR_SET_COLOR_ATTACHMENTS, diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h index 15f7ad94293..95223d01bdd 100644 --- a/src/intel/vulkan/anv_private.h +++ b/src/intel/vulkan/anv_private.h @@ -270,7 +270,14 @@ struct intel_perf_query_result; #define ANV_TRTT_L1_NULL_TILE_VAL 0 #define ANV_TRTT_L1_INVALID_TILE_VAL 1 +/* The binding table entry id disabled, the shader can write to it and the + * driver should use a null surface state so that writes are discarded. + */ #define ANV_COLOR_OUTPUT_DISABLED (0xff) +/* The binding table entry id unused, the shader does not write to it and the + * driver can leave whatever surface state was used before. Transitioning + * to/from this entry does not require render target cache flush. + */ #define ANV_COLOR_OUTPUT_UNUSED (0xfe) static inline uint32_t