<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Jun 1, 2017 at 3:17 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 Tue, May 30, 2017 at 05:55:20PM -0700, Jason Ekstrand wrote: > --- > src/intel/blorp/blorp_genX_exec.h | 32 ++++++++++++- > src/intel/isl/isl_emit_depth_stencil.c | 1 + > src/mesa/drivers/dri/i965/brw_blorp.c | 61 ------------------------ > src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 3 +- > src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 17 ++++--- > src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 2 +- > 6 files changed, 42 insertions(+), 74 deletions(-) > > diff --git a/src/intel/blorp/blorp_genX_exec.h b/src/intel/blorp/blorp_genX_exec.h > index 8b9b8d2..59c1d36 100644 > --- a/src/intel/blorp/blorp_genX_exec.h > +++ b/src/intel/blorp/blorp_genX_exec.h > @@ -1384,9 +1384,23 @@ blorp_emit_depth_stencil_config(struct blorp_batch *batch, > if (info.hiz_usage == ISL_AUX_USAGE_HIZ) { > info.hiz_surf = &params->depth.aux_surf; > > + struct blorp_address hiz_address = params->depth.aux_addr; > +#if GEN_GEN == 6 > + /* Sandy bridge hardware does not technically support mipmapped HiZ. > + * However, we have a special layout that allows us to make it work > + * anyway by manually offsetting to the specified miplevel. > + */ > + assert(info.hiz_surf->dim_layout == ISL_DIM_LAYOUT_GEN6_STENCIL_HIZ); > + uint32_t offset_B; > + isl_surf_get_image_offset_B_tile_sa(info.hiz_surf, > + info.view->base_level, 0, 0, > + &offset_B, NULL, NULL); > + hiz_address.offset += offset_B; > +#endif > + > info.hiz_address = > blorp_emit_reloc(batch, dw + isl_dev->ds.hiz_offset / 4, > - params->depth.aux_addr, 0); > + hiz_address, 0); > > info.depth_clear_value = params->depth.clear_color.u32[0]; > } > @@ -1395,9 +1409,23 @@ blorp_emit_depth_stencil_config(struct blorp_batch *batch, > if (params->stencil.enabled) { > info.stencil_surf = &params->stencil.surf; > > + struct blorp_address stencil_address = params->stencil.addr; > +#if GEN_GEN == 6 > + /* Sandy bridge hardware does not technically support mipmapped stencil. > + * However, we have a special layout that allows us to make it work > + * anyway by manually offsetting to the specified miplevel. > + */ > + assert(info.stencil_surf->dim_layout == ISL_DIM_LAYOUT_GEN6_STENCIL_HIZ); > + uint32_t offset_B; > + isl_surf_get_image_offset_B_tile_sa(info.stencil_surf, > + info.view->base_level, 0, 0, > + &offset_B, NULL, NULL); > + stencil_address.offset += offset_B; > +#endif > + > info.stencil_address = > blorp_emit_reloc(batch, dw + isl_dev->ds.stencil_offset / 4, > - params->stencil.addr, 0); > + stencil_address, 0); > } > > isl_emit_depth_stencil_hiz_s(isl_dev, dw, &info); > diff --git a/src/intel/isl/isl_emit_depth_stencil.c b/src/intel/isl/isl_emit_depth_stencil.c > index 41a01be..04bed07 100644 > --- a/src/intel/isl/isl_emit_depth_stencil.c > +++ b/src/intel/isl/isl_emit_depth_stencil.c > @@ -158,6 +158,7 @@ isl_genX(emit_depth_stencil_hiz_s)(const struct isl_device *dev, void *batch, > hiz.SurfaceBaseAddress = info->hiz_address; > hiz.HierarchicalDepthBufferMOCS = info->mocs; > hiz.SurfacePitch = info->hiz_surf->row_pitch - 1; > + </div></div>Leftover? <div><div class="h5"></div></div></blockquote><div> </div><div>Yup. Fixed. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5"> > #if GEN_GEN >= 8 > /* From the SKL PRM Vol2a: > * > diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c b/src/mesa/drivers/dri/i965/brw_blorp.c > index 61c6bda..28be620 100644 > --- a/src/mesa/drivers/dri/i965/brw_blorp.c > +++ b/src/mesa/drivers/dri/i965/brw_blorp.c > @@ -118,36 +118,6 @@ brw_blorp_init(struct brw_context *brw) > } > > static void > -apply_gen6_stencil_hiz_offset(struct isl_surf *surf, > - struct intel_mipmap_tree *mt, > - uint32_t lod, > - uint32_t *offset) > -{ > - assert(mt->array_layout == GEN6_HIZ_STENCIL); > - > - if (mt->format == MESA_FORMAT_S_UINT8) { > - /* Note: we can't compute the stencil offset using > - * intel_miptree_get_aligned_offset(), because the miptree > - * claims that the region is untiled even though it's W tiled. > - */ > - *offset = mt->level[lod].level_y * mt->pitch + > - mt->level[lod].level_x * 64; > - } else { > - *offset = intel_miptree_get_aligned_offset(mt, > - mt->level[lod].level_x, > - mt->level[lod].level_y); > - } > - > - surf->logical_level0_px.width = minify(surf->logical_level0_px.width, lod); > - surf->logical_level0_px.height = minify(surf->logical_level0_px.height, lod); > - surf->phys_level0_sa.width = minify(surf->phys_level0_sa.width, lod); > - surf->phys_level0_sa.height = minify(surf->phys_level0_sa.height, lod); > - surf->levels = 1; > - surf->array_pitch_el_rows = > - ALIGN(surf->phys_level0_sa.height, surf->image_alignment_el.height); > -} > - > -static void > blorp_surf_for_miptree(struct brw_context *brw, > struct blorp_surf *surf, > struct intel_mipmap_tree *mt, > @@ -181,24 +151,6 @@ blorp_surf_for_miptree(struct brw_context *brw, > .write_domain = is_render_target ? I915_GEM_DOMAIN_RENDER : 0, > }; > > - if (brw->gen == 6 && mt->format == MESA_FORMAT_S_UINT8 && > - mt->array_layout == GEN6_HIZ_STENCIL) { > - /* Sandy bridge stencil and HiZ use this GEN6_HIZ_STENCIL hack in > - * order to allow for layered rendering. The hack makes each LOD of the > - * stencil or HiZ buffer a single tightly packed array surface at some > - * offset into the surface. Since ISL doesn't know how to deal with the > - * crazy GEN6_HIZ_STENCIL layout and since we have to do a manual > - * offset of it anyway, we might as well do the offset here and keep the > - * hacks inside the i965 driver. > - * > - * See also gen6_depth_stencil_state.c > - */ > - uint32_t offset; > - apply_gen6_stencil_hiz_offset(&tmp_surfs[0], mt, *level, &offset); > - surf->addr.offset += offset; > - *level = 0; > - } > - > struct isl_surf *aux_surf = &tmp_surfs[1]; > intel_miptree_get_aux_isl_surf(brw, mt, aux_surf, &surf->aux_usage); > > @@ -258,19 +210,6 @@ blorp_surf_for_miptree(struct brw_context *brw, > > surf->aux_addr.buffer = mt->hiz_buf-><a href="http://aux_base.bo" rel="noreferrer" target="_blank">aux_base.bo</a>; > surf->aux_addr.offset = mt->hiz_buf->aux_base.offset; > - > - struct intel_mipmap_tree *hiz_mt = mt->hiz_buf->mt; > - if (hiz_mt) { > - assert(brw->gen == 6 && hiz_mt->array_layout == GEN6_HIZ_STENCIL); > - > - /* gen6 requires the HiZ buffer to be manually offset to the > - * right location. We could fixup the surf but it doesn't > - * matter since most of those fields don't matter. > - */ > - apply_gen6_stencil_hiz_offset(aux_surf, hiz_mt, *level, > - &surf->aux_addr.offset); > - assert(hiz_mt->pitch == aux_surf->row_pitch); > - } > } > } else { > surf->aux_addr = (struct blorp_address) { > diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c > index e019adc..ddfb4f9 100644 > --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c > +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c > @@ -89,7 +89,8 @@ brw_emit_surface_state(struct brw_context *brw, > surf.dim = get_isl_surf_dim(target); > > const enum isl_dim_layout dim_layout = > - get_isl_dim_layout(&brw->screen->devinfo, mt->tiling, target); > + get_isl_dim_layout(&brw->screen->devinfo, mt->tiling, target, > + mt->array_layout); > > if (surf.dim_layout != dim_layout) { > /* The layout of the specified texture target is not compatible with the > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c > index c0eaab0..55b760b 100644 > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c > @@ -1813,13 +1813,7 @@ intel_hiz_miptree_buf_create(struct brw_context *brw, > buf-><a href="http://aux_base.bo" rel="noreferrer" target="_blank">aux_base.bo</a> = buf->mt->bo; > buf->aux_base.size = buf->mt->total_height * buf->mt->pitch; > buf->aux_base.pitch = buf->mt->pitch; > - > - /* On gen6 hiz is unconditionally laid out packing all slices > - * at each level-of-detail (LOD). This means there is no valid qpitch > - * setting. In fact, this is ignored when hardware is setup - there is no > - * hardware qpitch setting of hiz on gen6. > - */ > - buf->aux_base.qpitch = 0; > + buf->aux_base.qpitch = buf->mt->qpitch * 2; </div></div>Is this really needed? I also didn't get the multiply by two. <div><div class="h5"></div></div></blockquote><div> </div><div>I did this entirely for sanity-checking purposes. Checking the qpitch is a quick and easy sanity check that ISL and brw_tex_layout calculated the same thing. It's not necessary (so long as we remove an assert from ISL) but I like the safety. You'll be deleting it soon enough anyway. :-) </div><div>Why multiply by 2? because brw_tex_layout thinks it's Y-tiled while ISL has a concept of HiZ-tiled. In order to get the right calculation in brw_tex_layout, we use a scale-down factor of 16x2 (a HiZ block is actually 8x4, see also Y vs. HiZ tiling). Since all other qpitch values in intel_mipmap_tree are in terms of samples (not surface elements), we need to undo the scale-down. Make sense? It's really easy to get lost in all the calculations. Just take comfort in the fact that this code will soon be gone. :-) </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5"> > > return buf; > } > @@ -3133,8 +3127,11 @@ get_isl_surf_dim(GLenum target) > > enum isl_dim_layout > get_isl_dim_layout(const struct gen_device_info *devinfo, uint32_t tiling, > - GLenum target) > + GLenum target, enum miptree_array_layout array_layout) > { > + if (array_layout == GEN6_HIZ_STENCIL) > + return ISL_DIM_LAYOUT_GEN6_STENCIL_HIZ; > + > switch (target) { > case GL_TEXTURE_1D: > case GL_TEXTURE_1D_ARRAY: > @@ -3188,7 +3185,8 @@ intel_miptree_get_isl_surf(struct brw_context *brw, > { > surf->dim = get_isl_surf_dim(mt->target); > surf->dim_layout = get_isl_dim_layout(&brw->screen->devinfo, > - mt->tiling, mt->target); > + mt->tiling, mt->target, > + mt->array_layout); > > if (mt->num_samples > 1) { > switch (mt->msaa_layout) { > @@ -3267,6 +3265,7 @@ intel_miptree_get_isl_surf(struct brw_context *brw, > switch (surf->dim_layout) { > case ISL_DIM_LAYOUT_GEN4_2D: > case ISL_DIM_LAYOUT_GEN4_3D: > + case ISL_DIM_LAYOUT_GEN6_STENCIL_HIZ: > if (brw->gen >= 9) { > surf->array_pitch_el_rows = mt->qpitch; > } else { > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h > index be460f3..6096cab 100644 > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h > @@ -804,7 +804,7 @@ get_isl_surf_dim(GLenum target); > > enum isl_dim_layout > get_isl_dim_layout(const struct gen_device_info *devinfo, uint32_t tiling, > - GLenum target); > + GLenum target, enum miptree_array_layout array_layout); > > enum isl_tiling > intel_miptree_get_isl_tiling(const struct intel_mipmap_tree *mt); > -- > 2.5.0.400.gff86faf > </div></div>> _______________________________________________ > mesa-dev mailing list > <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> > <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div> </div></div>