[Mesa-dev] [PATCH v2 3/5] i965/clear: Remove an early return in fast_clear_depth

[Rafael Antognolli]

On Wed, Apr 25, 2018 at 11:40:15AM -0700, Nanley Chery wrote:
> On Wed, Apr 25, 2018 at 11:30:14AM -0700, Rafael Antognolli wrote:
> > On Tue, Apr 24, 2018 at 05:48:44PM -0700, Nanley Chery wrote:
> > > Reduce complexity and allow the next patch to delete some code. With
> > > this change, clear operations will still be skipped and setting the
> > > aux_state will cause no side-effects.
> > 
> > It's going to skip the fast clear, but if I understood correctly it will
> > call intel_miptree_set_aux_state(), which marks the BRW_NEW_AUX_STATE
> > and will make all the surface state to be reemited.
> > 
> > I'm not sure if there's something else that already triggers that, but
> > if that wasn't the case already, maybe we are going to be emitting a lot
> > more state now?
> > 
> 
> The surface state won't be re-emitted. intel_miptree_set_aux_state()
> will only mark BRW_NEW_AUX_STATE if the new aux state differs from the
> current one. In the case where we can skip the fast clear, the current
> and the new states will both equal ISL_AUX_STATE_CLEAR.

Ouch, you are right, I missed the big "if" there. In this case, this
patch is

Reviewed-by: Rafael Antognolli <rafael.antognolli at intel.com>

> > > Remove the associated comment which implies an early return.
> > > ---
> > >  src/mesa/drivers/dri/i965/brw_clear.c | 5 -----
> > >  1 file changed, 5 deletions(-)
> > > 
> > > diff --git a/src/mesa/drivers/dri/i965/brw_clear.c b/src/mesa/drivers/dri/i965/brw_clear.c
> > > index fdc31cd9b68..6521141d7f6 100644
> > > --- a/src/mesa/drivers/dri/i965/brw_clear.c
> > > +++ b/src/mesa/drivers/dri/i965/brw_clear.c
> > > @@ -230,10 +230,6 @@ brw_fast_clear_depth(struct gl_context *ctx)
> > >     }
> > >  
> > >     if (!need_clear) {
> > > -      /* If all of the layers we intend to clear are already in the clear
> > > -       * state then simply updating the miptree fast clear value is sufficient
> > > -       * to change their clear value.
> > > -       */
> > >        if (!same_clear_value) {
> > >           /* BLORP updates the indirect clear color buffer when performing a
> > >            * fast clear. Since we are skipping the fast clear here, we need to
> > > @@ -241,7 +237,6 @@ brw_fast_clear_depth(struct gl_context *ctx)
> > >            */
> > >           intel_miptree_update_indirect_color(brw, mt);
> > >        }
> > > -      return true;
> > >     }
> > >  
> > >     for (unsigned a = 0; a < num_layers; a++) {
> > > -- 
> > > 2.16.2
> > > 
> > > _______________________________________________
> > > mesa-dev mailing list
> > > mesa-dev at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev