[Mesa-dev] [PATCH] i965: do not fallback to linear tiling for depth/stencil surfaces

[Iago Toral]

On Fri, 2017-09-08 at 11:44 +0300, Pohjolainen, Topi wrote:
> On Fri, Sep 08, 2017 at 09:41:11AM +0200, Iago Toral wrote:
> > On Fri, 2017-09-08 at 00:34 -0700, Kenneth Graunke wrote:
> > > On Thursday, September 7, 2017 11:44:04 PM PDT Iago Toral Quiroga
> > > wrote:
> > > > It is not supported by the hardware and the driver assumes
> > > > W-tiling for stencil and Y-tiling for depth everywhere anyway.
> > > > 
> > > > This fixes a regression in a CTS test introduced with commit
> > > > 4ea63fab77f0 that started applying re-tiling for these surfaces
> > > > in certain scenarios.
> > > > 
> > > > Fixes:
> > > > KHR-GL45.direct_state_access.renderbuffers_storage
> > > > ---
> > > >  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 8 ++++++--
> > > >  1 file changed, 6 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > > > b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > > > index 79afdc5342..cfc1212353 100644
> > > > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > > > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > > > @@ -572,9 +572,13 @@ make_surface(struct brw_context *brw,
> > > > GLenum
> > > > target, mesa_format format,
> > > >  
> > > >     /* In case caller doesn't specifically request Y-tiling
> > > > (needed
> > > >      * unconditionally for depth), check for corner cases
> > > > needing
> > > > special
> > > > -    * treatment.
> > > > +    * treatment. Stencil and depth surfaces are always W  and
> > > > Y
> > > > tiled
> > > > +    * respectively, so we should not attempt to retile them to
> > > > different
> > > > +    * layouts.
> > > >      */
> > > > -   if (tiling_flags & ~ISL_TILING_Y0_BIT) {
> > > 
> > > Based on Topi's comment, it sounds like depth surfaces should be
> > > passing
> > > tiling_flags == ISL_TILING_Y0_BIT, so we shouldn't hit this for
> > > depth.
> > > 
> > > Separate stencil passes ISL_TILING_W_BIT, so I think you're right
> > > that
> > > it'll incorrectly hit this case.  Perhaps we need to just change
> > > it
> > > to:
> > > 
> > >    if (tiling_flags & ~(ISL_TILING_Y0_BIT | ISL_TILING_W_BIT))
> > > 
> > > ...?  Though, basing it on usage doesn't seem bad either...
> > 
> > Yeah, I noticed that the test for depth usage wasn't actually
> > required
> > after sending the patch.
> > 
> > Is there anything other than depth/stencil that can use Y/W tiling
> > and
> > could be retiled to linear or X? If that is the case then I think
> > we
> > want to check against usage, otherwise we could just check against
> > the
> > tiling flags.
> 
> Color renderbuffers can be X or Y (latter preferred).

Ok, in that case I think we should use the usage flags to decide if we
need to avoid retiling. I'll send a v2 removing the check for depth
usage as suggested by Ken, since that is redundant anyway.

Iago