On Thu, Jun 1, 2017 at 3:17 AM, Pohjolainen, Topi <
topi.pohjolai...@gmail.com> wrote:
> 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 = ¶ms->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 = ¶ms->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;
> > +
>
> Leftover?
>
Yup. Fixed.
> > #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->aux_base.bo;
> > 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->aux_base.bo = 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;
>
> Is this really needed? I also didn't get the multiply by two.
>
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. :-)
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. :-)
> >
> > 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
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
