[Mesa-dev] [PATCH v3 02/14] i965/miptree: Allocate HiZ up-front

[Jason Ekstrand]

Topi, Ping?  I'd like to land as much of the prep-work as possible.

On Mon, Jul 17, 2017 at 10:33 AM, Pohjolainen, Topi <
topi.pohjolainen at gmail.com> wrote:

> On Mon, Jul 17, 2017 at 08:10:01AM -0700, Jason Ekstrand wrote:
> > On Mon, Jul 17, 2017 at 5:50 AM, Pohjolainen, Topi <
> > topi.pohjolainen at gmail.com> wrote:
> >
> > > On Mon, Jul 17, 2017 at 03:29:32PM +0300, Pohjolainen, Topi wrote:
> > > > On Fri, Jul 14, 2017 at 03:22:51PM -0700, Chad Versace wrote:
> > > > > On Wed 12 Jul 2017, Jason Ekstrand wrote:
> > > > > > HiZ, like MCS and CCS_E, can compress more than just clear
> colors so
> > > we
> > > > > > want it turned on whenever the miptree is being used as a depth
> > > > > > attachment.  It's theoretically possible for someone to create a
> > > depth
> > > > > > texture, upload data with glTexSubImage2D, and texture from it
> > > without
> > > > > > ever binding it as a depth target.  If this happens, we would
> end up
> > > > > > wasting a bit of space by allocating a HiZ surface we never use.
> > > > > > However, this is rather unlikely out side of test cases, so we're
> > > better
> > > > > > off just allocating it up-front.
> > > > > > ---
> > > > >
> > > > > I've read of rendering techniques that use high-quality,
> pre-rendered
> > > > > cascading shadow maps for scenes with fixed lights. And that would
> > > > > trigger the glTexSubImage2D case. But, even in that case, so little
> > > > > memory is wasted that I don't care.
> > > > >
> > > > > Reviewed-by: Chad Versace <chadversary at chromium.org>
> > > >
> > > > I'm now looking at this more after rebasing my isl patches on top. I
> > > think
> > > > this shows a bug we have in intel_miptree_choose_aux_usage() -
> there we
> > > > consider only if format and platform support hiz, but ignore tiling,
> > > ending
> > > > up setting HIZ usage even for linear depth.
> > > >
> > > > Combined with this patch we start to allocate hiz-buffers for linear
> > > depth
> > > > buffers used in read_pixels(). Backtrace with my isl support (which
> > > should be
> > > > more or less equivalent from this decision making point of you). By
> > > chance ISL
> > > > complains unlike i965 (here it actually complains about
> ISL_SURF_DIM_1D
> > > but
> > > > tiling is incompatible as well):
> > >
> >
> > Ugh... I didn't think linear depth was ever supported.
>
> Not for rendering but just for reading the pixels.
>
> >
> >
> > > > #4  0x00007ffff1d78e6e in isl_calc_phys_level0_extent_sa
> > > (dev=0x7ffff7fa1208,
> > > >     phys_level0_sa=<synthetic pointer>, msaa_layout=<optimized out>,
> > > >     tiling=ISL_TILING_HIZ, dim_layout=ISL_DIM_LAYOUT_GEN4_2D,
> > > >     info=0x7fffffffd6f0) at isl/isl.c:627
> > > > #5  isl_surf_init_s (dev=dev at entry=0x7ffff7fa1208,
> > > >     surf=surf at entry=0x7fffffffd740, info=info at entry=0x7fffffffd6f0)
> > > >     at isl/isl.c:1474
> > > > #6  0x00007ffff1d78fc9 in isl_surf_get_hiz_surf (dev=dev at entry
> > > =0x7ffff7fa1208,
> > > >     surf=surf at entry=0x555555b024d0, hiz_surf=hiz_surf at entry=
> > > 0x7fffffffd740)
> > > >     at isl/isl.c:1659
> > > > #7  0x00007ffff1cb4c4a in intel_miptree_alloc_hiz (
> > > >     brw=brw at entry=0x7ffff7f7c040, mt=mt at entry=0x555555b024d0)
> > > >     at intel_mipmap_tree.c:2139
> > > > #8  0x00007ffff1cb8017 in intel_miptree_alloc_aux (
> > > >     brw=brw at entry=0x7ffff7f7c040, mt=0x555555b024d0)
> > > >     at intel_mipmap_tree.c:2178
> > > > #9  0x00007ffff1cb8362 in intel_miptree_create (brw=brw at entry
> > > =0x7ffff7f7c040,
> > > >     target=target at entry=3553,
> > > >     format=format at entry=MESA_FORMAT_Z24_UNORM_X8_UINT,
> > > >     first_level=first_level at entry=0, last_level=last_level at entry=0,
> > > >     width0=<optimized out>, height0=60, depth0=1, num_samples=0,
> > > >     layout_flags=64) at intel_mipmap_tree.c:971
> > > > #10 0x00007ffff1cb8d14 in intel_miptree_map_blit (slice=0, level=0,
> > > >     map=0x555555a7c710, mt=0x555555b1b7d0, brw=0x7ffff7f7c040)
> > > >     at intel_mipmap_tree.c:3247
> > > > #11 intel_miptree_map (brw=brw at entry=0x7ffff7f7c040,
> mt=0x555555b1b7d0,
> > > >     level=0, slice=0, x=x at entry=60, y=y at entry=100, w=60, h=60,
> mode=1,
> > > >     out_ptr=0x7fffffffdaa8, out_stride=0x7fffffffdab0)
> > > >     at intel_mipmap_tree.c:3817
> > > > #12 0x00007ffff1cb252f in intel_map_renderbuffer (ctx=0x7ffff7f7c040,
> > > >     rb=0x5555558d2f60, x=60, y=100, w=<optimized out>, h=<optimized
> out>,
> > > >     mode=1, out_map=0x7fffffffdb98, out_stride=0x7fffffffdb90)
> > > >     at intel_fbo.c:169
> > > > #13 0x00007ffff19eef3b in read_depth_pixels (packing=0x7fffffffdce0,
> > >
> > > This should be sufficient (at least works for the case I was
> debugging):
> > >
> > > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > > b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > > index ecd57cbf30..5eb778541f 100644
> > > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > > @@ -219,6 +219,9 @@ intel_miptree_supports_hiz(struct brw_context
> *brw,
> > >     if (!brw->has_hiz)
> > >        return false;
> > >
> > > +   if (mt->tiling != I915_TILING_Y)
> > > +      return false;
> > >
> >
> > This doesn't quite work.  I sent out a patch to go before this one in the
> > series that should solve the problem.
>
> Ok. I'll take a look.
>
> >
> >
> > > +
> > >     switch (mt->format) {
> > >     case MESA_FORMAT_Z_FLOAT32:
> > >     case MESA_FORMAT_Z32_FLOAT_S8X24_UINT:
> > >
> > >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170717/737d255f/attachment.html>