[Mesa-dev] [PATCH v2 3/5] i965/clear: Remove an early return in fast_clear_depth
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.
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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 3/5] i965/clear: Remove an early return in fast_clear_depth
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?
> 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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 3/5] i965/clear: Remove an early return in fast_clear_depth
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.
-Nanley
> > 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@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 3/5] i965/clear: Remove an early return in fast_clear_depth
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
> > > 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@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 3/5] i965/clear: Remove an early return in fast_clear_depth
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
>
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@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
