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

[Jason Ekstrand]

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.


> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20161123/94ad0acf/attachment.html>