[Mesa-dev] [v2 02/17] i965/blorp: Skip redundant re-fast clear for non-compressed

[Ben Widawsky]

On 16-11-23 12:04:02, Jason Ekstrand wrote:
>On Wed, Nov 23, 2016 at 10:16 AM, Pohjolainen, Topi <
>topi.pohjolainen at gmail.com> wrote:
>
>> On Wed, Nov 23, 2016 at 09:05:39AM -0800, Jason Ekstrand wrote:
>> >    On Wed, Nov 23, 2016 at 1:16 AM, Topi Pohjolainen
>> >    <[1]topi.pohjolainen at gmail.com> wrote:
>> >
>> >      Originally re-clears where skipped but when lossless compression
>> >      was introduced the re-clears where errorneously enabled also for
>> >      non-compressed fast clears.
>> >      Signed-off-by: Topi Pohjolainen <[2]topi.pohjolainen at intel.com>
>> >      CC: Ben Widawsky <[3]benjamin.widawsky at intel.com>
>> >      CC: Kenneth Graunke <[4]kenneth at whitecape.org>
>> >      CC: Harri Syrja <[5]harri.syrja at intel.com>
>> >      Cc: Chad Versace <[6]chad at kiwitree.net>
>> >      ---
>> >       src/mesa/drivers/dri/i965/brw_blorp.c | 19 ++++++++++++++++---
>> >       1 file changed, 16 insertions(+), 3 deletions(-)
>> >      diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c
>> >      b/src/mesa/drivers/dri/i965/brw_blorp.c
>> >      index 556f2c0..9a849f5 100644
>> >      --- a/src/mesa/drivers/dri/i965/brw_blorp.c
>> >      +++ b/src/mesa/drivers/dri/i965/brw_blorp.c
>> >      @@ -825,10 +825,23 @@ do_single_blorp_clear(struct brw_context *brw,
>> >      struct gl_framebuffer *fb,
>> >                                           brw, &irb->mt->gen9_fast_clear_
>> >      color,
>> >                                           &override_color);
>> >      -      /* If the buffer is already in INTEL_FAST_CLEAR_STATE_CLEAR,
>> >      the clear
>> >      -       * is redundant and can be skipped.
>> >      +      /* If the buffer is already in INTEL_FAST_CLEAR_STATE_CLEAR,
>> >      and the
>> >      +       * buffer isn't compressed, the clear is redundant and can be
>> >      skipped.
>> >      +       *
>> >      +       * Without compression fast clear only operates on the mcs
>> >      buffer
>> >      +       * recording if color values are cleared. The hardware,
>> >      however,
>> >      +       * doesn't write the actual color value into the mcs or color
>> >      +       * buffer. Only by the time of render (inclucing color
>> >      resolve) does the
>> >      +       * hardware read the _current_ color value in the surface
>> >      state and
>> >      +       * write the actual pixel values in the color buffer
>> >      accordingly.
>> >      +       *
>> >      +       * This seems to be the reason why sampler engine cannot
>> >      handle
>> >      +       * non-compressed fast clear - it doesn't know how to read
>> >      the color
>> >      +       * value from the surface state. With compression the color
>> >      value is
>> >      +       * recorded in the color buffer (only not for every pixel)
>> >      and therefore
>> >      +       * it is available without consulting the surface state.
>> >
>> >    This doesn't jive with my understanding of fast clears on gen9.
>> >    Everything I've seen so far indicates that the clear color in the
>> >    surface state *does* matter.  Otherwise, why would it be there?  In
>> >    particular, my understanding of the 2-bit CCS values is that 0 means
>> >    resolved, 1 means compressed and 3 means clear where "clear" means "go
>> >    look at the clear color".  Have you done experiments that lead you to
>> >    some other conclusion?
>>
>> Right, you are correct. This is actually a patch from really early days
>> when I
>> didn't know any better. We might want to drop this for now, there is the
>> 0/1
>> color thing for sampler engine that we probably need to fix first anyway.
>>
>
>Let's drop it if we can.
>
>We should already have code for Broadwell and earlier that prevents
>fast-clears with non-0/1 clear colors.  We could just also do that on Sky
>Lake and deal with partial resolves later.  I doubt non-0/1 fast-clear is a
>big enough deal over color compression to slow down getting this landed.
>

Numbers

>
>> What do you think?
>>
>> >
>> >              */
>> >      -      if (!color_updated &&
>> >      +      if ((!color_updated || !is_lossless_compressed) &&
>> >                 irb->mt->fast_clear_state ==
>> >      INTEL_FAST_CLEAR_STATE_CLEAR)
>> >                return true;
>> >      --
>> >      2.5.5
>> >      _______________________________________________
>> >      mesa-dev mailing list
>> >      [7]mesa-dev at lists.freedesktop.org
>> >      [8]https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>> >
>> > References
>> >
>> >    1. mailto:topi.pohjolainen at gmail.com
>> >    2. mailto:topi.pohjolainen at intel.com
>> >    3. mailto:benjamin.widawsky at intel.com
>> >    4. mailto:kenneth at whitecape.org
>> >    5. mailto:harri.syrja at intel.com
>> >    6. mailto:chad at kiwitree.net
>> >    7. mailto:mesa-dev at lists.freedesktop.org
>> >    8. https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>

-- 
Ben Widawsky, Intel Open Source Technology Center