Re: [Mesa-dev] [PATCH] i965/cnl: Avoid fast-clearing sRGB render buffers

[Nanley Chery]

On Fri, Dec 01, 2017 at 04:42:22PM -0800, Jason Ekstrand wrote:
> On Fri, Dec 1, 2017 at 2:44 PM, Nanley Chery  wrote:
> 
> > Gen10 doesn't automatically decode the clear color of sRGB buffers. To
> > get correct rendering, avoid fast-clearing such buffers for now.
> >
> > The driver now passes the following piglit tests:
> > * spec@arb_framebuffer_srgb@msaa-fast-clear
> > * spec@ext_texture_srgb@multisample-fast-clear gl_ext_texture_srgb
> >
> > Suggested-by: Kenneth Graunke 
> > Suggested-by: Jason Ekstrand 
> > Signed-off-by: Nanley Chery 
> > ---
> >
> > This patch is currently going through the jenkins pipeline.
> >
> >  src/mesa/drivers/dri/i965/brw_meta_util.c | 14 --
> >  1 file changed, 12 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_meta_util.c
> > b/src/mesa/drivers/dri/i965/brw_meta_util.c
> > index ba92168934..54dc6a5ff9 100644
> > --- a/src/mesa/drivers/dri/i965/brw_meta_util.c
> > +++ b/src/mesa/drivers/dri/i965/brw_meta_util.c
> > @@ -298,13 +298,23 @@ brw_is_color_fast_clear_compatible(struct
> > brw_context *brw,
> >  * resolved in intel_update_state. In that case it's pointless to do a
> >  * fast clear because it's very likely to be immediately resolved.
> >  */
> > +   const bool srgb_rb = _mesa_get_srgb_format_linear(mt->format) !=
> > mt->format;
> > if (devinfo->gen >= 9 &&
> > mt->surf.samples == 1 &&
> > -   ctx->Color.sRGBEnabled &&
> > -   _mesa_get_srgb_format_linear(mt->format) != mt->format)
> > +   ctx->Color.sRGBEnabled && srgb_rb)
> >return false;
> >
> > +  /* Gen10 doesn't automatically decode the clear color of sRGB buffers.
> > Since
> > +   * we currently don't perform this decode in software, avoid a
> > fast-clear
> > +   * altogether. TODO: Do this in software.
> > +   */
> > const mesa_format format = _mesa_get_render_format(ctx, mt->format);
> > +   if (devinfo->gen >= 10 && srgb_rb) {
> >
> 
> We could be a bit more precise and only disable it for non-0/1.  Either way,
> 
> Reviewed-by: Jason Ekstrand 
> 
> 


Thanks! I'm gonna push this and hopefully we'll get around to using the
packed clear method to solve all the corner cases.

> > +  perf_debug("sRGB fast clear not enabled for (%s)",
> > + _mesa_get_format_name(format));
> >
> 
> Thanks for leaving a perf_debug!
> 
> 

Np, I was inspired by the one nearby :)

-Nanley

> > +  return false;
> > +   }
> > +
> > if (_mesa_is_format_integer_color(format)) {
> >if (devinfo->gen >= 8) {
> >   perf_debug("Integer fast clear not enabled for (%s)",
> > --
> > 2.15.1
> >
> > ___
> > 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

Re: [Mesa-dev] [PATCH] i965/cnl: Avoid fast-clearing sRGB render buffers

[Jason Ekstrand]

On Fri, Dec 1, 2017 at 2:44 PM, Nanley Chery  wrote:

> Gen10 doesn't automatically decode the clear color of sRGB buffers. To
> get correct rendering, avoid fast-clearing such buffers for now.
>
> The driver now passes the following piglit tests:
> * spec@arb_framebuffer_srgb@msaa-fast-clear
> * spec@ext_texture_srgb@multisample-fast-clear gl_ext_texture_srgb
>
> Suggested-by: Kenneth Graunke 
> Suggested-by: Jason Ekstrand 
> Signed-off-by: Nanley Chery 
> ---
>
> This patch is currently going through the jenkins pipeline.
>
>  src/mesa/drivers/dri/i965/brw_meta_util.c | 14 --
>  1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_meta_util.c
> b/src/mesa/drivers/dri/i965/brw_meta_util.c
> index ba92168934..54dc6a5ff9 100644
> --- a/src/mesa/drivers/dri/i965/brw_meta_util.c
> +++ b/src/mesa/drivers/dri/i965/brw_meta_util.c
> @@ -298,13 +298,23 @@ brw_is_color_fast_clear_compatible(struct
> brw_context *brw,
>  * resolved in intel_update_state. In that case it's pointless to do a
>  * fast clear because it's very likely to be immediately resolved.
>  */
> +   const bool srgb_rb = _mesa_get_srgb_format_linear(mt->format) !=
> mt->format;
> if (devinfo->gen >= 9 &&
> mt->surf.samples == 1 &&
> -   ctx->Color.sRGBEnabled &&
> -   _mesa_get_srgb_format_linear(mt->format) != mt->format)
> +   ctx->Color.sRGBEnabled && srgb_rb)
>return false;
>
> +  /* Gen10 doesn't automatically decode the clear color of sRGB buffers.
> Since
> +   * we currently don't perform this decode in software, avoid a
> fast-clear
> +   * altogether. TODO: Do this in software.
> +   */
> const mesa_format format = _mesa_get_render_format(ctx, mt->format);
> +   if (devinfo->gen >= 10 && srgb_rb) {
>

We could be a bit more precise and only disable it for non-0/1.  Either way,

Reviewed-by: Jason Ekstrand 


> +  perf_debug("sRGB fast clear not enabled for (%s)",
> + _mesa_get_format_name(format));
>

Thanks for leaving a perf_debug!


> +  return false;
> +   }
> +
> if (_mesa_is_format_integer_color(format)) {
>if (devinfo->gen >= 8) {
>   perf_debug("Integer fast clear not enabled for (%s)",
> --
> 2.15.1
>
> ___
> 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

Re: [Mesa-dev] [PATCH] i965/cnl: Avoid fast-clearing sRGB render buffers

[Nanley Chery]

On Fri, Dec 01, 2017 at 02:44:42PM -0800, Nanley Chery wrote:
> Gen10 doesn't automatically decode the clear color of sRGB buffers. To
> get correct rendering, avoid fast-clearing such buffers for now.
> 
> The driver now passes the following piglit tests:
> * spec@arb_framebuffer_srgb@msaa-fast-clear
> * spec@ext_texture_srgb@multisample-fast-clear gl_ext_texture_srgb
> 
> Suggested-by: Kenneth Graunke 
> Suggested-by: Jason Ekstrand 
> Signed-off-by: Nanley Chery 
> ---
> 
> This patch is currently going through the jenkins pipeline.
> 

The run came back green.

-Nanley

>  src/mesa/drivers/dri/i965/brw_meta_util.c | 14 --
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_meta_util.c 
> b/src/mesa/drivers/dri/i965/brw_meta_util.c
> index ba92168934..54dc6a5ff9 100644
> --- a/src/mesa/drivers/dri/i965/brw_meta_util.c
> +++ b/src/mesa/drivers/dri/i965/brw_meta_util.c
> @@ -298,13 +298,23 @@ brw_is_color_fast_clear_compatible(struct brw_context 
> *brw,
>  * resolved in intel_update_state. In that case it's pointless to do a
>  * fast clear because it's very likely to be immediately resolved.
>  */
> +   const bool srgb_rb = _mesa_get_srgb_format_linear(mt->format) != 
> mt->format;
> if (devinfo->gen >= 9 &&
> mt->surf.samples == 1 &&
> -   ctx->Color.sRGBEnabled &&
> -   _mesa_get_srgb_format_linear(mt->format) != mt->format)
> +   ctx->Color.sRGBEnabled && srgb_rb)
>return false;
>  
> +  /* Gen10 doesn't automatically decode the clear color of sRGB buffers. 
> Since
> +   * we currently don't perform this decode in software, avoid a fast-clear
> +   * altogether. TODO: Do this in software.
> +   */
> const mesa_format format = _mesa_get_render_format(ctx, mt->format);
> +   if (devinfo->gen >= 10 && srgb_rb) {
> +  perf_debug("sRGB fast clear not enabled for (%s)",
> + _mesa_get_format_name(format));
> +  return false;
> +   }
> +
> if (_mesa_is_format_integer_color(format)) {
>if (devinfo->gen >= 8) {
>   perf_debug("Integer fast clear not enabled for (%s)",
> -- 
> 2.15.1
> 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] i965/cnl: Avoid fast-clearing sRGB render buffers

[Nanley Chery]

Gen10 doesn't automatically decode the clear color of sRGB buffers. To
get correct rendering, avoid fast-clearing such buffers for now.

The driver now passes the following piglit tests:
* spec@arb_framebuffer_srgb@msaa-fast-clear
* spec@ext_texture_srgb@multisample-fast-clear gl_ext_texture_srgb

Suggested-by: Kenneth Graunke 
Suggested-by: Jason Ekstrand 
Signed-off-by: Nanley Chery 
---

This patch is currently going through the jenkins pipeline.

 src/mesa/drivers/dri/i965/brw_meta_util.c | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_meta_util.c 
b/src/mesa/drivers/dri/i965/brw_meta_util.c
index ba92168934..54dc6a5ff9 100644
--- a/src/mesa/drivers/dri/i965/brw_meta_util.c
+++ b/src/mesa/drivers/dri/i965/brw_meta_util.c
@@ -298,13 +298,23 @@ brw_is_color_fast_clear_compatible(struct brw_context 
*brw,
 * resolved in intel_update_state. In that case it's pointless to do a
 * fast clear because it's very likely to be immediately resolved.
 */
+   const bool srgb_rb = _mesa_get_srgb_format_linear(mt->format) != mt->format;
if (devinfo->gen >= 9 &&
mt->surf.samples == 1 &&
-   ctx->Color.sRGBEnabled &&
-   _mesa_get_srgb_format_linear(mt->format) != mt->format)
+   ctx->Color.sRGBEnabled && srgb_rb)
   return false;
 
+  /* Gen10 doesn't automatically decode the clear color of sRGB buffers. Since
+   * we currently don't perform this decode in software, avoid a fast-clear
+   * altogether. TODO: Do this in software.
+   */
const mesa_format format = _mesa_get_render_format(ctx, mt->format);
+   if (devinfo->gen >= 10 && srgb_rb) {
+  perf_debug("sRGB fast clear not enabled for (%s)",
+ _mesa_get_format_name(format));
+  return false;
+   }
+
if (_mesa_is_format_integer_color(format)) {
   if (devinfo->gen >= 8) {
  perf_debug("Integer fast clear not enabled for (%s)",
-- 
2.15.1

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev