[Mesa-dev] [PATCH] i965/blorp: Do not skip fast color clear with new color

[Topi Pohjolainen]

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 <topi.pohjolainen at intel.com>
CC: Kenneth Graunke <kenneth at whitecape.org>
CC: Ben Widawsky <ben at bwidawsk.net>
---
 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));
       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);
    }
+
+   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);
-- 
2.5.5