[Mesa-dev] [PATCH 19/30] i965: Move color rendering to the new resolve functions

[Pohjolainen, Topi]

On Tue, May 30, 2017 at 05:59:51PM -0700, Jason Ekstrand wrote:
> On Fri, May 26, 2017 at 4:30 PM, Jason Ekstrand <jason at jlekstrand.net>
> wrote:
> 
> > ---
> >  src/mesa/drivers/dri/i965/brw_context.c       | 35 +++----------------
> >  src/mesa/drivers/dri/i965/brw_draw.c          |  4 +--
> >  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 48
> > +++++++++++++++++++++++++++
> >  src/mesa/drivers/dri/i965/intel_mipmap_tree.h |  9 +++++
> >  4 files changed, 63 insertions(+), 33 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_context.c
> > b/src/mesa/drivers/dri/i965/brw_context.c
> > index 07ddaf0..48e8b6c 100644
> > --- a/src/mesa/drivers/dri/i965/brw_context.c
> > +++ b/src/mesa/drivers/dri/i965/brw_context.c
> > @@ -301,38 +301,11 @@ intel_update_state(struct gl_context * ctx, GLuint
> > new_state)
> >        if (irb == NULL || irb->mt == NULL)
> >           continue;
> >
> > -      struct intel_mipmap_tree *mt = irb->mt;
> > +      intel_miptree_prepare_render(brw, irb->mt, irb->mt_level,
> > +                                   irb->mt_layer, irb->layer_count,
> > +                                   ctx->Color.sRGBEnabled);
> >
> > -      /* If FRAMEBUFFER_SRGB is used on Gen9+ then we need to resolve any
> > of
> > -       * the single-sampled color renderbuffers because the CCS buffer
> > isn't
> > -       * supported for SRGB formats. This only matters if
> > FRAMEBUFFER_SRGB is
> > -       * enabled because otherwise the surface state will be programmed
> > with
> > -       * the linear equivalent format anyway.
> > -       */
> > -      if (brw->gen >= 9 && ctx->Color.sRGBEnabled && mt->num_samples <= 1
> > &&
> > -          _mesa_get_srgb_format_linear(mt->format) != mt->format) {
> > -
> > -         /* Lossless compression is not supported for SRGB formats, it
> > -          * should be impossible to get here with such surfaces.
> > -          */
> > -         assert(!intel_miptree_is_lossless_compressed(brw, mt));
> > -         intel_miptree_all_slices_resolve_color(brw, mt, 0);
> > -         brw_render_cache_set_check_flush(brw, mt->bo);
> > -      }
> > -
> > -      /* For layered rendering non-compressed fast cleared buffers need
> > to be
> > -       * resolved. Surface state can carry only one fast color clear value
> > -       * while each layer may have its own fast clear color value. For
> > -       * compressed buffers color value is available in the color buffer.
> > -       */
> > -      if (irb->layer_count > 1 &&
> > -          !(irb->mt->aux_disable & INTEL_AUX_DISABLE_CCS) &&
> > -          !intel_miptree_is_lossless_compressed(brw, mt)) {
> > -         assert(brw->gen >= 8);
> > -
> > -         intel_miptree_resolve_color(brw, mt, irb->mt_level, 1,
> > -                                     irb->mt_layer, irb->layer_count, 0);
> > -      }
> > +      brw_render_cache_set_check_flush(brw, irb->mt->bo);
> >
> 
> This render_cache_set_check_flush is unneeded and is actually the cause of
> most of the performance regressions in this series.  Making it
> unconditional meant we flushed the render cache on every draw call.  It's a
> bit surprising that doing so didn't hurt things any worse than it did.  It
> was originally put in to satisfy the requirements about flushing around
> resolves.  Now that we do that directly in brw_blorp_resolve_color, we
> don't need it at all much less unconditionally.  I've removed this line
> locally.

Makes sense, I remember fighting against unconditional flushing as well. This
series makes a big difference in how easy is to keep track of aux state and
flushing more precisely.