[Mesa-dev] [PATCH 2/2] intel/isl/gen4: Make depth/stencil buffers Y-Tiled

[Nanley Chery]

On Tue, Jul 17, 2018 at 08:19:30AM -0700, Kenneth Graunke wrote:
> On Monday, July 16, 2018 4:57:40 PM PDT Nanley Chery wrote:
> > Rendering to a linear depth buffer on gen4 is causing a GPU hang in the
> > CI system. Until a better explanation is found, assume that errata is
> > applicable to all gen4 platforms.
> > 
> > Fixes fbe01625f6bf2cef6742e1ff0d3d44a2afec003e
> > ("i965/miptree: Share tiling_flags in miptree_create").
> > 
> > Reported-by: Mark Janes <mark.a.janes at intel.com>
> > ---
> >  src/intel/isl/isl_gen4.c | 9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/intel/isl/isl_gen4.c b/src/intel/isl/isl_gen4.c
> > index 14706c895a5..a212d0ee0af 100644
> > --- a/src/intel/isl/isl_gen4.c
> > +++ b/src/intel/isl/isl_gen4.c
> > @@ -51,8 +51,15 @@ isl_gen4_filter_tiling(const struct isl_device *dev,
> >        /* From the g35 PRM Vol. 2, 3DSTATE_DEPTH_BUFFER::Tile Walk:
> >         *
> >         *    "The Depth Buffer, if tiled, must use Y-Major tiling"
> > +       *
> > +       *    Errata   Description    Project
> > +       *    BWT014   The Depth Buffer Must be Tiled, it cannot be linear. This
> > +       *    field must be set to 1 on DevBW-A.  [DevBW -A,B]
> > +       *
> > +       * In testing, the linear configuration doesn't seem to work on gen4.
> >         */
> > -      *flags &= (ISL_TILING_LINEAR_BIT | ISL_TILING_Y0_BIT);
> > +      *flags &= (ISL_DEV_GEN(dev) == 4 && !ISL_DEV_IS_G4X(dev)) ?
> > +                ISL_TILING_Y0_BIT : (ISL_TILING_Y0_BIT | ISL_TILING_LINEAR_BIT);
> >     }
> >  
> >     if (info->usage & (ISL_SURF_USAGE_DISPLAY_ROTATE_90_BIT |
> > 
> 
> Wow, I had no idea we were actually using linear depth buffers.

Neither did I until Mark let me know that I regressed some tests after
landing commit fbe01625f6bf2cef6742e1ff0d3d44a2afec003e. Whoops!

I've included the Bugzilla tag locally.
https://bugs.freedesktop.org/show_bug.cgi?id=107248

> Is there any compelling reason to use them?  I can't think of one.
> 

Yes. They give us the best performance and memory usage for 1D textures
(see isl_surf_choose_tiling()).

> Y-tiling should offer better performance.  PBO upload needs to be
> done with a R32_UINT format anyway, as R24_X8 isn't renderable.
> 

I don't understand how the PBO format comes into play here. Care to
elaborate?

> I'm pretty sure that we used to always use Y-tiling in the old
> miptree code...maybe this regressed when we switched to ISL?
> 

Yeah, we did always use Y-tiling. I caused the regression when I pushed
that commit.

> The hardware timeline looks like:
> 
>    BROKEN     /-------------- Works??? -----------\    DISALLOWED
> [Broadwater, Crestline, Eaglelake, Cantiga, Ironlake, Sandybridge, ...]
> 
> Given that it was broken and ultimately became disallowed, I'm a bit
> skeptical whether it really works, or is useful, in the meantime.
> 

It seems to work. We're getting testing from these piglit tests:
* piglit.spec.arb_shader_texture_lod.execution.tex-miplevel-selection *projgradarb 1dshadow
* piglit.spec.arb_shader_texture_lod.execution.tex-miplevel-selection *gradarb 1dshadow
* piglit.spec.arb_shader_texture_lod.execution.tex-miplevel-selection *lod 1dshadow
* piglit.spec.!opengl 1_1.copyteximage 1d
* piglit.spec.ext_texture_array.copyteximage 1d_array
* piglit.spec.arb_shader_texture_lod.execution.tex-miplevel-selection *projlod 1dshadow

On ILK and g45 they go from fail->pass with the last patch. But g965
needed the linear option removed completely.

> Personally, I'd be inclined to simply make this
> 
>    *flags &= ISL_TILING_Y0_BIT;
> 
> which I suppose gets into philosophy about whether ISL should represent
> exactly what the hardware can do, or instead do what we want...but...we
> ought to disallow linear somehow.

Yeah, I think ISL mostly tries to represent what the HW can do. Allowing
linear in i965 seems painless thus far, so I thought I might as well
give it a try.

> 
> Good find with the errata!  I'm glad to see it cited here.

Thanks.