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

Pohjolainen, Topi topi.pohjolainen at gmail.com
Thu Sep 8 04:30:05 UTC 2016


On Wed, Sep 07, 2016 at 03:25:30PM -0700, Jason Ekstrand wrote:
>    On Sep 7, 2016 10:24 AM, "Topi Pohjolainen"
>    <[1]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 <[2]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.

Like the comment says, we can't skip setting aux buffer for sampling in case
the same surface is written as render target with aux enabled. Hardware
doesn't seem to like this. This is specifically for that purpose checking
the flag.

> 
>    > +   }
>    > +
>    > +   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
>    > [3]mesa-dev at lists.freedesktop.org
>    > [4]https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
> References
> 
>    1. mailto:topi.pohjolainen at gmail.com
>    2. mailto:topi.pohjolainen at intel.com
>    3. mailto:mesa-dev at lists.freedesktop.org
>    4. https://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list