[Mesa-dev] [v3 4/6] i965/rbc: Consult rb settings for texture surface setup

[Jason Ekstrand]

On Sep 7, 2016 10:24 AM, "Topi Pohjolainen" <topi.pohjolainen at gmail.com>
wrote:
>
> Once mcs buffer gets allocated without delay for lossless
> compression (same as we do for msaa), one gets regression in:
>
> GL45-CTS.texture_barrier_ARB.same-texel-rw
>
> Setting the auxiliary surface for both sampling engine and data
> port seems to fix this. I haven't found any hardware documentation
> backing this though.
>
> v2 (Jason): Prepare also for the case where surface is sampled with
>             non-compressible format forcing also rendering without
>             compression.
> v3: Split asserts and decision making.
>
> Signed-off-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
> ---
>  src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 63
+++++++++++++++++++++---
>  1 file changed, 56 insertions(+), 7 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> index c1273c5..054c5c8 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> @@ -140,9 +140,7 @@ brw_emit_surface_state(struct brw_context *brw,
>     struct isl_surf *aux_surf = NULL, aux_surf_s;
>     uint64_t aux_offset = 0;
>     enum isl_aux_usage aux_usage = ISL_AUX_USAGE_NONE;
> -   if (mt->mcs_mt &&
> -       ((view.usage & ISL_SURF_USAGE_RENDER_TARGET_BIT) ||
> -        mt->fast_clear_state != INTEL_FAST_CLEAR_STATE_RESOLVED)) {
> +   if (mt->mcs_mt && !(flags & INTEL_AUX_BUFFER_DISABLED)) {
>        intel_miptree_get_aux_isl_surf(brw, mt, &aux_surf_s, &aux_usage);
>        aux_surf = &aux_surf_s;
>        assert(mt->mcs_mt->offset == 0);
> @@ -425,6 +423,54 @@ swizzle_to_scs(GLenum swizzle, bool
need_green_to_blue)
>     return (need_green_to_blue && scs == HSW_SCS_GREEN) ? HSW_SCS_BLUE :
scs;
>  }
>
> +static unsigned
> +brw_find_matching_rb(const struct gl_framebuffer *fb,
> +                     const struct intel_mipmap_tree *mt)
> +{
> +   for (unsigned i = 0; i < fb->_NumColorDrawBuffers; i++) {
> +      const struct intel_renderbuffer *irb =
> +         intel_renderbuffer(fb->_ColorDrawBuffers[i]);
> +
> +      if (irb->mt == mt)
> +         return i;
> +   }
> +
> +   return fb->_NumColorDrawBuffers;
> +}
> +
> +static bool
> +brw_disable_aux_surface(const struct brw_context *brw,
> +                        const struct intel_mipmap_tree *mt)
> +{
> +   /* Nothing to disable. */
> +   if (!mt->mcs_mt)
> +      return false;
> +
> +   /* There are special cases only for lossless compression. */
> +   if (!intel_miptree_is_lossless_compressed(brw, mt))
> +      return mt->fast_clear_state == INTEL_FAST_CLEAR_STATE_RESOLVED;
> +
> +   const struct gl_framebuffer *fb = brw->ctx.DrawBuffer;
> +   const unsigned rb_index = brw_find_matching_rb(fb, mt);
> +
> +   /* In practise it looks that setting the same lossless compressed
surface
> +    * to be sampled without auxiliary surface and to be written with
auxiliary
> +    * surface confuses the hardware. Therefore sampler engine must be
provided
> +    * with auxiliary buffer regardless of the fast clear state if the
same
> +    * surface is also going to be written during the same rendering pass
with
> +    * auxiliary buffer enabled.
> +    */
> +   if (rb_index < fb->_NumColorDrawBuffers) {
> +      if (brw->draw_aux_buffer_disabled[rb_index]) {
> +         assert(mt->fast_clear_state == INTEL_FAST_CLEAR_STATE_RESOLVED);
> +      }
> +
> +      return brw->draw_aux_buffer_disabled[rb_index];

Can we use "mt->fast_clear_state == INTEL_FAST_CLEAR_STATE_RESOLVED" here
as well?  For render targets we want to look at aux_buffer_disabled but for
textures (which is the only case that uses this function), fast_clear_state
should be sufficient.

> +   }
> +
> +   return mt->fast_clear_state == INTEL_FAST_CLEAR_STATE_RESOLVED;
> +}
> +
>  void
>  brw_update_texture_surface(struct gl_context *ctx,
>                             unsigned unit,
> @@ -542,7 +588,8 @@ brw_update_texture_surface(struct gl_context *ctx,
>            obj->Target == GL_TEXTURE_CUBE_MAP_ARRAY)
>           view.usage |= ISL_SURF_USAGE_CUBE_BIT;
>
> -      const int flags = 0;
> +      const int flags =
> +         brw_disable_aux_surface(brw, mt) ? INTEL_AUX_BUFFER_DISABLED :
0;
>        brw_emit_surface_state(brw, mt, flags, mt->target, view,
>                               surface_state_infos[brw->gen].tex_mocs,
>                               surf_offset, surf_index,
> @@ -1113,7 +1160,8 @@ update_renderbuffer_read_surfaces(struct
brw_context *brw)
>                 .usage = ISL_SURF_USAGE_TEXTURE_BIT,
>              };
>
> -            const int flags = 0;
> +            const int flags = brw->draw_aux_buffer_disabled[i] ?
> +                                 INTEL_AUX_BUFFER_DISABLED : 0;
>              brw_emit_surface_state(brw, irb->mt, flags, target, view,
>
surface_state_infos[brw->gen].tex_mocs,
>                                     surf_offset, surf_index,
> @@ -1672,8 +1720,9 @@ update_image_surface(struct brw_context *brw,
>              };
>
>              const int surf_index = surf_offset -
&brw->wm.base.surf_offset[0];
> -
> -            const int flags = 0;
> +            const int flags =
> +               mt->fast_clear_state == INTEL_FAST_CLEAR_STATE_RESOLVED ?
> +               INTEL_AUX_BUFFER_DISABLED : 0;

According to brw_context, images should *always* be resolved.  Can we make
this check an assert and always pass AUX_BUFFER_DISABLED?

>              brw_emit_surface_state(brw, mt, flags, mt->target, view,
>
surface_state_infos[brw->gen].tex_mocs,
>                                     surf_offset, surf_index,
> --
> 2.5.5
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160907/4f147ea2/attachment.html>