[Mesa-dev] [v3 4/6] i965/rbc: Consult rb settings for texture surface setup
Jason Ekstrand
jason at jlekstrand.net
Thu Sep 8 15:49:56 UTC 2016
On Sep 7, 2016 9:30 PM, "Pohjolainen, Topi" <topi.pohjolainen at gmail.com>
wrote:
>
> 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.
Let me see if I understand this correctly: this handles the case where the
miptree is bound as both a render target and a texture and there is nothing
about the texture view scenario causing it to skip compression and, for
some reason, the surface starts of the draw call fully resolved. If *any*
drawing has been done without a resolve, we would be in the UNRESOLVED
state and this wouldn't be needed. That's terribly specific but also a
valid use case.
We could potentially handle this by setting the render target surfaces to
UNRESOLVED a bit earlier in the pipeline setup code. I believe we flag
things as unresolved rather late; in particular, after we've done our
texture state setup. The other option is to do exactly what you did and add
a comment. That's probably the best option. With a good comment added,
Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>
--Jason
> >
> > > + }
> > > +
> > > + 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160908/3e1883aa/attachment.html>
More information about the mesa-dev
mailing list