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

[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 <eero.t.tamminen at intel.com>
> > Cc: Scott D Phillips <scott.d.phillips at intel.com>
> > ---
> >  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 = &brw->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 <kenneth at whitecape.org>
> 

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