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