Re: [Mesa-dev] [PATCH 3/3] i965/tex_image: Avoid the ASTC LDR workaround on gen9lp

2018-03-07 Thread Eero Tamminen 

Hi,

This drops the internal benchmark startup time from 18 to ~1 seconds 
(for warm startup) on the gen9lp platform where I tested it.


Tested-by: Eero tammi...@intel.com

- Eero

On 06.03.2018 00:07, Nanley Chery wrote:

Both the internal documentation and the results of testing this in the
CI suggest that this is unnecessary. Add the fixes tag because this
reduces an internal benchmark's startup time significantly. Eero
reported that a less optimal version of this patch reduced the startup
time of the benchmark by about 14 seconds.

Fixes: 710b1d2e665 "i965/tex_image: Flush certain subnormal ASTC channel values"
Cc: Eero Tamminen 
Cc: Scott D Phillips 
---
  src/mesa/drivers/dri/i965/intel_tex_image.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/mesa/drivers/dri/i965/intel_tex_image.c 
b/src/mesa/drivers/dri/i965/intel_tex_image.c
index e25bc9a0c08..a0408b304d5 100644
--- a/src/mesa/drivers/dri/i965/intel_tex_image.c
+++ b/src/mesa/drivers/dri/i965/intel_tex_image.c
@@ -927,7 +927,7 @@ intelCompressedTexSubImage(struct gl_context *ctx, GLuint 
dims,
  !_mesa_is_srgb_format(gl_format);
 struct brw_context *brw = (struct brw_context*) ctx;
 const struct gen_device_info *devinfo = >screen->devinfo;
-   if (devinfo->gen == 9 && is_linear_astc)
+   if (devinfo->gen == 9 && !gen_device_info_is_9lp(devinfo) && is_linear_astc)
flush_astc_denorms(ctx, dims, texImage,
   xoffset, yoffset, zoffset,
   width, height, depth);



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

Re: [Mesa-dev] [PATCH 3/3] i965/tex_image: Avoid the ASTC LDR workaround on gen9lp

2018-03-06 Thread Nanley Chery 
On Tue, Mar 06, 2018 at 12:22:40AM -0800, Kenneth Graunke wrote:
> On Monday, March 5, 2018 2:07:55 PM PST Nanley Chery wrote:
> > Both the internal documentation and the results of testing this in the
> > CI suggest that this is unnecessary. Add the fixes tag because this
> > reduces an internal benchmark's startup time significantly. Eero
> > reported that a less optimal version of this patch reduced the startup
> > time of the benchmark by about 14 seconds.
> > 
> > Fixes: 710b1d2e665 "i965/tex_image: Flush certain subnormal ASTC channel 
> > values"
> > Cc: Eero Tamminen 
> > Cc: Scott D Phillips 
> > ---
> >  src/mesa/drivers/dri/i965/intel_tex_image.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/src/mesa/drivers/dri/i965/intel_tex_image.c 
> > b/src/mesa/drivers/dri/i965/intel_tex_image.c
> > index e25bc9a0c08..a0408b304d5 100644
> > --- a/src/mesa/drivers/dri/i965/intel_tex_image.c
> > +++ b/src/mesa/drivers/dri/i965/intel_tex_image.c
> > @@ -927,7 +927,7 @@ intelCompressedTexSubImage(struct gl_context *ctx, 
> > GLuint dims,
> >  !_mesa_is_srgb_format(gl_format);
> > struct brw_context *brw = (struct brw_context*) ctx;
> > const struct gen_device_info *devinfo = >screen->devinfo;
> > -   if (devinfo->gen == 9 && is_linear_astc)
> > +   if (devinfo->gen == 9 && !gen_device_info_is_9lp(devinfo) && 
> > is_linear_astc)
> >flush_astc_denorms(ctx, dims, texImage,
> >   xoffset, yoffset, zoffset,
> >   width, height, depth);
> > 
> 
> I don't know how to review this, really, but it sounds very plausible
> that they would have fixed that bug on gen9lp.  If it doesn't regress
> any tests, then:
> 
> Acked-by: Kenneth Graunke 
> 

Thanks! 

> This begs the question, though - is it just Skylake that needs the
> denorm flushing hack?  What about Kabylake or Coffeelake?  
> 

The results of the Jenkins jobs, nchery/281 and mesa_custom/3672, show
that all big-core gen9 platforms need it. 

-Nanley

> --Ken


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

Re: [Mesa-dev] [PATCH 3/3] i965/tex_image: Avoid the ASTC LDR workaround on gen9lp

2018-03-06 Thread Kenneth Graunke 
On Monday, March 5, 2018 2:07:55 PM PST Nanley Chery wrote:
> Both the internal documentation and the results of testing this in the
> CI suggest that this is unnecessary. Add the fixes tag because this
> reduces an internal benchmark's startup time significantly. Eero
> reported that a less optimal version of this patch reduced the startup
> time of the benchmark by about 14 seconds.
> 
> Fixes: 710b1d2e665 "i965/tex_image: Flush certain subnormal ASTC channel 
> values"
> Cc: Eero Tamminen 
> Cc: Scott D Phillips 
> ---
>  src/mesa/drivers/dri/i965/intel_tex_image.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/intel_tex_image.c 
> b/src/mesa/drivers/dri/i965/intel_tex_image.c
> index e25bc9a0c08..a0408b304d5 100644
> --- a/src/mesa/drivers/dri/i965/intel_tex_image.c
> +++ b/src/mesa/drivers/dri/i965/intel_tex_image.c
> @@ -927,7 +927,7 @@ intelCompressedTexSubImage(struct gl_context *ctx, GLuint 
> dims,
>  !_mesa_is_srgb_format(gl_format);
> struct brw_context *brw = (struct brw_context*) ctx;
> const struct gen_device_info *devinfo = >screen->devinfo;
> -   if (devinfo->gen == 9 && is_linear_astc)
> +   if (devinfo->gen == 9 && !gen_device_info_is_9lp(devinfo) && 
> is_linear_astc)
>flush_astc_denorms(ctx, dims, texImage,
>   xoffset, yoffset, zoffset,
>   width, height, depth);
> 

I don't know how to review this, really, but it sounds very plausible
that they would have fixed that bug on gen9lp.  If it doesn't regress
any tests, then:

Acked-by: Kenneth Graunke 

This begs the question, though - is it just Skylake that needs the
denorm flushing hack?  What about Kabylake or Coffeelake?  

--Ken


signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 3/3] i965/tex_image: Avoid the ASTC LDR workaround on gen9lp

2018-03-05 Thread Nanley Chery 
Both the internal documentation and the results of testing this in the
CI suggest that this is unnecessary. Add the fixes tag because this
reduces an internal benchmark's startup time significantly. Eero
reported that a less optimal version of this patch reduced the startup
time of the benchmark by about 14 seconds.

Fixes: 710b1d2e665 "i965/tex_image: Flush certain subnormal ASTC channel values"
Cc: Eero Tamminen 
Cc: Scott D Phillips 
---
 src/mesa/drivers/dri/i965/intel_tex_image.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/mesa/drivers/dri/i965/intel_tex_image.c 
b/src/mesa/drivers/dri/i965/intel_tex_image.c
index e25bc9a0c08..a0408b304d5 100644
--- a/src/mesa/drivers/dri/i965/intel_tex_image.c
+++ b/src/mesa/drivers/dri/i965/intel_tex_image.c
@@ -927,7 +927,7 @@ intelCompressedTexSubImage(struct gl_context *ctx, GLuint 
dims,
 !_mesa_is_srgb_format(gl_format);
struct brw_context *brw = (struct brw_context*) ctx;
const struct gen_device_info *devinfo = >screen->devinfo;
-   if (devinfo->gen == 9 && is_linear_astc)
+   if (devinfo->gen == 9 && !gen_device_info_is_9lp(devinfo) && is_linear_astc)
   flush_astc_denorms(ctx, dims, texImage,
  xoffset, yoffset, zoffset,
  width, height, depth);
-- 
2.16.1

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