<div dir="ltr">Topi, Ping? I'd like to land as much of the prep-work as possible. </div><div class="gmail_extra"> <div class="gmail_quote">On Mon, Jul 17, 2017 at 10:33 AM, Pohjolainen, Topi <<a href="mailto:topi.pohjolainen@gmail.com" target="_blank">topi.pohjolainen@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Mon, Jul 17, 2017 at 08:10:01AM -0700, Jason Ekstrand wrote: > On Mon, Jul 17, 2017 at 5:50 AM, Pohjolainen, Topi < > <a href="mailto:topi.pohjolainen@gmail.com">topi.pohjolainen@gmail.com</a>> 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 <<a href="mailto:chadversary@chromium.org">chadversary@chromium.org</a>> > > > > > > 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. </div></div>Not for rendering but just for reading the pixels. <div><div class="h5"> > > > > > #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@entry=0x7ffff7fa1208, > > > surf=surf@entry=0x7fffffffd740, info=info@entry=0x7fffffffd6f0) > > > at isl/isl.c:1474 > > > #6 0x00007ffff1d78fc9 in isl_surf_get_hiz_surf (dev=dev@entry > > =0x7ffff7fa1208, > > > surf=surf@entry=0x555555b024d0, hiz_surf=hiz_surf@entry= > > 0x7fffffffd740) > > > at isl/isl.c:1659 > > > #7 0x00007ffff1cb4c4a in intel_miptree_alloc_hiz ( > > > brw=brw@entry=0x7ffff7f7c040, mt=mt@entry=0x555555b024d0) > > > at intel_mipmap_tree.c:2139 > > > #8 0x00007ffff1cb8017 in intel_miptree_alloc_aux ( > > > brw=brw@entry=0x7ffff7f7c040, mt=0x555555b024d0) > > > at intel_mipmap_tree.c:2178 > > > #9 0x00007ffff1cb8362 in intel_miptree_create (brw=brw@entry > > =0x7ffff7f7c040, > > > target=target@entry=3553, > > > format=format@entry=MESA_FORMAT_Z24_UNORM_X8_UINT, > > > first_level=first_level@entry=0, last_level=last_level@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@entry=0x7ffff7f7c040, mt=0x555555b1b7d0, > > > level=0, slice=0, x=x@entry=60, y=y@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. </div></div>Ok. I'll take a look. <div class="HOEnZb"><div class="h5"> > > > > + > > switch (mt->format) { > > case MESA_FORMAT_Z_FLOAT32: > > case MESA_FORMAT_Z32_FLOAT_S8X24_UINT: > > > > </div></div></blockquote></div> </div>