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

2016-11-27 Thread Ben Widawsky 

On 16-11-24 09:23:31, Jason Ekstrand wrote:

[snip]



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.



You made the statement that non 0/1 fast clears are not a big enough deal, and I
was saying I want numbers. We've had benchmarks in the past that we've cared
about which are a big deal.

I realize that we maybe aren't doing things correctly today, but we don't seem
to have failures AFAIK.

Anyway, I'm just asking us to be thorough.


[snip]

--
Ben Widawsky, Intel Open Source Technology Center
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2016-11-24 Thread Jason Ekstrand 
On Nov 24, 2016 8:42 AM, "Ben Widawsky"  wrote:
>
> On 16-11-23 12:04:02, Jason Ekstrand wrote:
>>
>> On Wed, Nov 23, 2016 at 10:16 AM, Pohjolainen, Topi <
>> topi.pohjolai...@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.pohjolai...@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.pohjolai...@intel.com>
>>> >  CC: Ben Widawsky <[3]benjamin.widaw...@intel.com>
>>> >  CC: Kenneth Graunke <[4]kenn...@whitecape.org>
>>> >  CC: Harri Syrja <[5]harri.sy...@intel.com>
>>> >  Cc: Chad Versace <[6]c...@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,
>mt->gen9_fast_clear_
>>> >  color,
>>> >   _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) &&
>>> >

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

2016-11-24 Thread Ben Widawsky 

On 16-11-23 12:04:02, Jason Ekstrand wrote:

On Wed, Nov 23, 2016 at 10:16 AM, Pohjolainen, Topi <
topi.pohjolai...@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.pohjolai...@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.pohjolai...@intel.com>
>  CC: Ben Widawsky <[3]benjamin.widaw...@intel.com>
>  CC: Kenneth Graunke <[4]kenn...@whitecape.org>
>  CC: Harri Syrja <[5]harri.sy...@intel.com>
>  Cc: Chad Versace <[6]c...@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, >mt->gen9_fast_clear_
>  color,
>   _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@lists.freedesktop.org
>  [8]https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
> References
>
>1. mailto:topi.pohjolai...@gmail.com
>2. mailto:topi.pohjolai...@intel.com
>3. mailto:benjamin.widaw...@intel.com
>4. mailto:kenn...@whitecape.org
>5. mailto:harri.sy...@intel.com
>6. mailto:c...@kiwitree.net
>7. mailto:mesa-dev@lists.freedesktop.org
>8. https://lists.freedesktop.org/mailman/listinfo/mesa-dev



--
Ben Widawsky, Intel Open Source Technology Center
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2016-11-23 Thread Jason Ekstrand 
On Wed, Nov 23, 2016 at 10:16 AM, Pohjolainen, Topi <
topi.pohjolai...@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.pohjolai...@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.pohjolai...@intel.com>
> >  CC: Ben Widawsky <[3]benjamin.widaw...@intel.com>
> >  CC: Kenneth Graunke <[4]kenn...@whitecape.org>
> >  CC: Harri Syrja <[5]harri.sy...@intel.com>
> >  Cc: Chad Versace <[6]c...@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, >mt->gen9_fast_clear_
> >  color,
> >   _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@lists.freedesktop.org
> >  [8]https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >
> > References
> >
> >1. mailto:topi.pohjolai...@gmail.com
> >2. mailto:topi.pohjolai...@intel.com
> >3. mailto:benjamin.widaw...@intel.com
> >4. mailto:kenn...@whitecape.org
> >5. mailto:harri.sy...@intel.com
> >6. mailto:c...@kiwitree.net
> >7. mailto:mesa-dev@lists.freedesktop.org
> >8. https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2016-11-23 Thread Pohjolainen, Topi 
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.pohjolai...@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.pohjolai...@intel.com>
>  CC: Ben Widawsky <[3]benjamin.widaw...@intel.com>
>  CC: Kenneth Graunke <[4]kenn...@whitecape.org>
>  CC: Harri Syrja <[5]harri.sy...@intel.com>
>  Cc: Chad Versace <[6]c...@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, >mt->gen9_fast_clear_
>  color,
>   _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.

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@lists.freedesktop.org
>  [8]https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
> References
> 
>1. mailto:topi.pohjolai...@gmail.com
>2. mailto:topi.pohjolai...@intel.com
>3. mailto:benjamin.widaw...@intel.com
>4. mailto:kenn...@whitecape.org
>5. mailto:harri.sy...@intel.com
>6. mailto:c...@kiwitree.net
>7. mailto:mesa-dev@lists.freedesktop.org
>8. https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2016-11-23 Thread Jason Ekstrand 
On Wed, Nov 23, 2016 at 1:16 AM, Topi Pohjolainen <
topi.pohjolai...@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 
> CC: Ben Widawsky 
> CC: Kenneth Graunke 
> CC: Harri Syrja 
> Cc: Chad Versace 
> ---
>  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, >mt->gen9_fast_clear_color,
>  _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?


> */
> -  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
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2016-11-23 Thread Topi Pohjolainen 
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 
CC: Ben Widawsky 
CC: Kenneth Graunke 
CC: Harri Syrja 
Cc: Chad Versace 
---
 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, >mt->gen9_fast_clear_color,
 _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.
*/
-  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
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev