[Mesa-dev] [v2 02/17] i965/blorp: Skip redundant re-fast clear for non-compressed
Jason Ekstrand
jason at jlekstrand.net
Thu Nov 24 17:23:31 UTC 2016
On Nov 24, 2016 8:42 AM, "Ben Widawsky" <benjamin.widawsky at intel.com> wrote:
>
> 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 mean? Of i recall correctly, we noticed zero speedups in any
apps when we enabled non-0/1 fast clears in the first place. Also, I
wasn't trying to say we shouldn't enable them or shouldn't get them
working; my understanding is that they're enabled and broken today. I was
saying that we shouldn't block landing this on coming up with a partial
resolve story to fix non-0/1 clears with compression because that's really
an unrelated task. We do need better resolves and hopefully we'll get that
done soon but poor Topi has been sitting on this 3‰ perf improvement and
trying to land it fitter months now.
Sorry of that got a bit ranty. I hope it makes more sense.
>>
>>> 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20161124/acb8a5d7/attachment-0001.html>
More information about the mesa-dev
mailing list