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.ja...@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. _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev