On Thursday, May 12, 2016 8:50:33 AM PDT Topi Pohjolainen wrote: > This hasn't been visible before. It showed up with lossless > compression with: > > dEQP-GLES3.functional.fbo.color.repeated_clear.sample.tex2d.rgb8 > > Current fast clear logic kicks color resolves even for gpu sampling. > In the test case this results into trashing of the fast color clear > state between two subsequent clears, and therefore each clear is > performed correctly. > With lossless compression the resolves are unnecessary and therefore > the clear state indicates that the buffer is already cleared. Without > considering if the previous color value was the same as the new, > clears that need to be performed are skipped and the buffer ends up > holding old pixel values. > > Signed-off-by: Topi Pohjolainen <[email protected]> > CC: Kenneth Graunke <[email protected]> > CC: Ben Widawsky <[email protected]> > --- > src/mesa/drivers/dri/i965/brw_blorp_clear.cpp | 6 ++++-- > src/mesa/drivers/dri/i965/brw_meta_fast_clear.c | 13 ++++++++++++- > src/mesa/drivers/dri/i965/brw_meta_util.h | 2 +- > 3 files changed, 17 insertions(+), 4 deletions(-) > > diff --git a/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp b/src/mesa/ drivers/dri/i965/brw_blorp_clear.cpp > index ed537ba..2cde347 100644 > --- a/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp > +++ b/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp > @@ -301,12 +301,14 @@ do_single_blorp_clear(struct brw_context *brw, struct gl_framebuffer *fb, > * programmed in SURFACE_STATE by later rendering and resolve > * operations. > */ > - brw_meta_set_fast_clear_color(brw, irb->mt, &ctx->Color.ClearColor); > + const bool color_updated = brw_meta_set_fast_clear_color( > + brw, irb->mt, &ctx->Color.ClearColor); > > /* If the buffer is already in INTEL_FAST_CLEAR_STATE_CLEAR, the clear > * is redundant and can be skipped. > */ > - if (irb->mt->fast_clear_state == INTEL_FAST_CLEAR_STATE_CLEAR) > + if (!color_updated && > + irb->mt->fast_clear_state == INTEL_FAST_CLEAR_STATE_CLEAR) > return true; > > /* If the MCS buffer hasn't been allocated yet, we need to allocate > diff --git a/src/mesa/drivers/dri/i965/brw_meta_fast_clear.c b/src/mesa/ drivers/dri/i965/brw_meta_fast_clear.c > index 76988bf..d98f870 100644 > --- a/src/mesa/drivers/dri/i965/brw_meta_fast_clear.c > +++ b/src/mesa/drivers/dri/i965/brw_meta_fast_clear.c > @@ -421,8 +421,10 @@ brw_is_color_fast_clear_compatible(struct brw_context *brw, > /** > * Convert the given color to a bitfield suitable for ORing into DWORD 7 of > * SURFACE_STATE (DWORD 12-15 on SKL+). > + * > + * Returned boolean tells if the given color differs from the stored. > */ > -void > +bool > brw_meta_set_fast_clear_color(struct brw_context *brw, > struct intel_mipmap_tree *mt, > const union gl_color_union *color) > @@ -466,9 +468,14 @@ brw_meta_set_fast_clear_color(struct brw_context *brw, > } > } > > + bool updated; > if (brw->gen >= 9) { > + updated = memcmp(&mt->gen9_fast_clear_color, &override_color, > + sizeof(mt->gen9_fast_clear_color));
I think this would be easier to read with an explicit != 0 at the end.
> mt->gen9_fast_clear_color = override_color;
> } else {
> + const uint32_t old_color_value = mt->fast_clear_color_value;
> +
> mt->fast_clear_color_value = 0;
> for (int i = 0; i < 4; i++) {
> /* Testing for non-0 works for integer and float colors */
> @@ -477,7 +484,11 @@ brw_meta_set_fast_clear_color(struct brw_context *brw,
> 1 << (GEN7_SURFACE_CLEAR_COLOR_SHIFT + (3 - i));
> }
> }
> +
> + updated = (old_color_value == mt->fast_clear_color_value);
Shouldn't this be !=?
With that fixed,
Reviewed-by: Kenneth Graunke <[email protected]>
> }
> +
> + return updated;
> }
>
> static const uint32_t fast_clear_color[4] = { ~0, ~0, ~0, ~0 };
> diff --git a/src/mesa/drivers/dri/i965/brw_meta_util.h b/src/mesa/drivers/
dri/i965/brw_meta_util.h
> index 550a46a..ac051e2 100644
> --- a/src/mesa/drivers/dri/i965/brw_meta_util.h
> +++ b/src/mesa/drivers/dri/i965/brw_meta_util.h
> @@ -60,7 +60,7 @@ brw_meta_get_buffer_rect(const struct gl_framebuffer *fb,
> unsigned *x0, unsigned *y0,
> unsigned *x1, unsigned *y1);
>
> -void
> +bool
> brw_meta_set_fast_clear_color(struct brw_context *brw,
> struct intel_mipmap_tree *mt,
> const union gl_color_union *color);
>
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
