[Mesa-dev] [PATCH 2/2] i965: Disable auxiliary buffers when there are self-dependencies.

[Jason Ekstrand]

On October 5, 2017 10:02:58 PM Kenneth Graunke <kenneth at whitecape.org> wrote:

> Jason and I investigated several OpenGL CTS failures where the tests
> bind the same texture for rendering and texturing, at the same time.
> This has defined results as long as the reads happen before writes,
> or the regions are non-overlapping.  Normally, this just works out.
>
> However, CCS can cause problems.  If the shader is reading one set of
> pixels, and writing to different pixels that are adjacent, they may end
> up being covered by the same CCS block.  So rendering may be writing a
> CCS block, while the sampler is trying to read it.  Corruption ensues.
>
> Disabling CCS is unfortunate, but safe.
>
> Fixes several KHR-GL45.texture_barrier.* subtests.
>
> Cc: nanleychery at gmail.com
> Cc: jason at jlekstrand.net
> ---
>  src/mesa/drivers/dri/i965/brw_draw.c          |  8 +++++++-
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 13 +++++++------
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.h |  2 +-
>  3 files changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_draw.c 
> b/src/mesa/drivers/dri/i965/brw_draw.c
> index cab3758d7b5..c13fa8c367a 100644
> --- a/src/mesa/drivers/dri/i965/brw_draw.c
> +++ b/src/mesa/drivers/dri/i965/brw_draw.c
> @@ -383,7 +383,13 @@ brw_predraw_resolve_inputs(struct brw_context *brw)
>           translate_tex_format(brw, tex_obj->_Format, sampler->sRGBDecode);
>        int drawbuf_idx = get_drawbuffer_index(brw, tex_obj->mt->bo);
>
> -      bool aux_supported;
> +      /* Disable auxiliary buffers if there's a self-dependency, where
> +       * we're both texturing from and rendering to the same buffer.
> +       * It's not necessarily safe - concurrent reads and writes to the
> +       * CCS buffer can result in incorrect pixels.
> +       */
> +      bool aux_supported = drawbuf_idx == -1;
> +
>        intel_miptree_prepare_texture(brw, tex_obj->mt, view_format,
>                                      &aux_supported);
>
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c 
> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> index 5b7cde82f65..29b93dd656c 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -2651,9 +2651,10 @@ intel_miptree_prepare_texture_slices(struct 
> brw_context *brw,
>                                       enum isl_format view_format,
>                                       uint32_t start_level, uint32_t num_levels,
>                                       uint32_t start_layer, uint32_t num_layers,
> -                                     bool *aux_supported_out)
> +                                     bool *aux_supported)

Now that this is an input parameter, there's no reason for it to be a pointer.

>  {
> -   enum isl_aux_usage aux_usage =
> +   enum isl_aux_usage aux_usage = aux_supported && !*aux_supported ?
> +      ISL_AUX_USAGE_NONE :
>        intel_miptree_texture_aux_usage(brw, mt, view_format);
>     bool clear_supported = aux_usage != ISL_AUX_USAGE_NONE;
>
> @@ -2667,20 +2668,20 @@ intel_miptree_prepare_texture_slices(struct 
> brw_context *brw,
>     intel_miptree_prepare_access(brw, mt, start_level, num_levels,
>                                  start_layer, num_layers,
>                                  aux_usage, clear_supported);
> -   if (aux_supported_out)
> -      *aux_supported_out = aux_usage != ISL_AUX_USAGE_NONE;
> +   if (aux_supported)
> +      *aux_supported = aux_usage != ISL_AUX_USAGE_NONE;
>  }
>
>  void
>  intel_miptree_prepare_texture(struct brw_context *brw,
>                                struct intel_mipmap_tree *mt,
>                                enum isl_format view_format,
> -                              bool *aux_supported_out)
> +                              bool *aux_supported)
>  {
>     intel_miptree_prepare_texture_slices(brw, mt, view_format,
>                                          0, INTEL_REMAINING_LEVELS,
>                                          0, INTEL_REMAINING_LAYERS,
> -                                        aux_supported_out);
> +                                        aux_supported);
>  }
>
>  void
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h 
> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> index 2fce28c524b..0da7fafe601 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> @@ -640,7 +640,7 @@ void
>  intel_miptree_prepare_texture(struct brw_context *brw,
>                                struct intel_mipmap_tree *mt,
>                                enum isl_format view_format,
> -                              bool *aux_supported_out);
> +                              bool *aux_supported);
>  void
>  intel_miptree_prepare_image(struct brw_context *brw,
>                              struct intel_mipmap_tree *mt);
> --
> 2.14.2
>