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

[Nanley Chery]

On Wed, Apr 25, 2018 at 11:43:23AM -0700, Rafael Antognolli wrote:
> 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>
> 

Thanks!

> > > > 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