[Mesa-dev] [PATCH 7/7] [v2] i965/gen9: Support fast clears for 32b float

[Neil Roberts]

Ben Widawsky <benjamin.widawsky at intel.com> writes:

> Two formats are explicitly disabled because they fail piglit tests,
> LUMINANCE16F and INTENSITY16F. There is some question about the
> validity of sampling from these surfaces for all gens, however, there
> seem to be no other failures, so I'd prefer to leave tackling that for
> a separate series.

Did you see my message in another thread pointing out that there are
other issues with the SKL fast clear? If you clear to white instead of
black then a bunch of other more normal formats fail such as GL_RGB.
This isn't related to this 32b float patch because obviously white
should fit the criteria of being just 0 and 1.

Perhaps the easiest way to demonstrate this bug is to tweak the formats
test so that it clears to white instead of black, like this:

diff --git a/tests/util/piglit-test-pattern.cpp b/tests/util/piglit-test-pattern.cpp
index 12d9918..bec25bc 100644
--- a/tests/util/piglit-test-pattern.cpp
+++ b/tests/util/piglit-test-pattern.cpp
@@ -664,7 +664,7 @@ ColorGradientSunburst::draw_with_scale_and_offset(const float (*proj)[4],
        }
        case GL_UNSIGNED_NORMALIZED:
        case GL_FLOAT: {
-               glClearColor(offset, offset, offset, offset);
+               glClearColor(1.0, 1.0, 1.0, 1.0);
                glClear(GL_COLOR_BUFFER_BIT);
                break;
        }

This passes on HSW but makes a bunch of formats fail on SKL.

It seems like we're missing something about how we're supposed to
program the fast colour state. For GL_RGB it seems to be expecting an
integer value in the range 0→255 instead of a float. Eg, if you program
the integer 128 you get back 0.5 when reading. However this isn't the
case for GL_RGBA which does seem to expect float values. I can't find
anything that would explain this in the specs. It makes no sense!

There is this bit in the spec which we are ignoring:

“If Number of Multisamples is not MULTISAMPLECOUNT_1, only 0/1 values
 allowed”

But that doesn't explain the problem because in this case we are
clearing to white.

http://lists.freedesktop.org/archives/mesa-dev/2015-November/099274.html

Regards,
- Neil

>
> v2: Just reject the two failing types.
>
> Cc: Neil Roberts <neil at linux.intel.com
> Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
> ---
>  src/mesa/drivers/dri/i965/brw_meta_fast_clear.c | 12 ++++++++++--
>  src/mesa/drivers/dri/i965/gen8_surface_state.c  |  8 --------
>  2 files changed, 10 insertions(+), 10 deletions(-)
>
> 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 67dd22d..a024d02 100644
> --- a/src/mesa/drivers/dri/i965/brw_meta_fast_clear.c
> +++ b/src/mesa/drivers/dri/i965/brw_meta_fast_clear.c
> @@ -360,10 +360,18 @@ is_color_fast_clear_compatible(struct brw_context *brw,
>     }
>  
>     for (int i = 0; i < 4; i++) {
> -      if (color->f[i] != 0.0f && color->f[i] != 1.0f &&
> -          _mesa_format_has_color_component(format, i)) {
> +      if (!_mesa_format_has_color_component(format, i)) {
> +         continue;
> +      }
> +
> +      if (brw->gen < 9 &&
> +          color->f[i] != 0.0f && color->f[i] != 1.0f) {
>           return false;
>        }
> +
> +      if (format == MESA_FORMAT_L_FLOAT16 ||
> +          format == MESA_FORMAT_I_FLOAT16)
> +         return false;
>     }
>     return true;
>  }
> diff --git a/src/mesa/drivers/dri/i965/gen8_surface_state.c b/src/mesa/drivers/dri/i965/gen8_surface_state.c
> index 8fe480c..2381c83 100644
> --- a/src/mesa/drivers/dri/i965/gen8_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/gen8_surface_state.c
> @@ -188,14 +188,6 @@ gen8_emit_fast_clear_color(struct brw_context *brw,
>                             uint32_t *surf)
>  {
>     if (brw->gen >= 9) {
> -#define check_fast_clear_val(x) \
> -      assert(mt->gen9_fast_clear_color.f[x] == 0.0 || \
> -             mt->gen9_fast_clear_color.f[x] == 1.0)
> -      check_fast_clear_val(0);
> -      check_fast_clear_val(1);
> -      check_fast_clear_val(2);
> -      check_fast_clear_val(3);
> -#undef check_fast_clear_val
>        surf[12] = mt->gen9_fast_clear_color.ui[0];
>        surf[13] = mt->gen9_fast_clear_color.ui[1];
>        surf[14] = mt->gen9_fast_clear_color.ui[2];
> -- 
> 2.6.2