[Mesa-dev] [PATCH 15/15] i965: Represent depth surfaces with isl
Pohjolainen, Topi
topi.pohjolainen at gmail.com
Fri Jun 16 05:52:45 UTC 2017
On Thu, Jun 15, 2017 at 04:41:14PM -0700, Jason Ekstrand wrote:
> On Tue, Jun 13, 2017 at 12:11 PM, Topi Pohjolainen <
> topi.pohjolainen at gmail.com> wrote:
>
> > Signed-off-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
> > ---
> > src/mesa/drivers/dri/i965/brw_misc_state.c | 9 +-
> > src/mesa/drivers/dri/i965/gen6_depth_state.c | 2 +-
> > src/mesa/drivers/dri/i965/gen7_misc_state.c | 2 +-
> > src/mesa/drivers/dri/i965/gen8_depth_state.c | 5 +-
> > src/mesa/drivers/dri/i965/intel_fbo.c | 4 +-
> > src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 140
> > +++++++++++++++++---------
> > 6 files changed, 102 insertions(+), 60 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_misc_state.c
> > b/src/mesa/drivers/dri/i965/brw_misc_state.c
> > index 3f3bd2535e..e744ed2482 100644
> > --- a/src/mesa/drivers/dri/i965/brw_misc_state.c
> > +++ b/src/mesa/drivers/dri/i965/brw_misc_state.c
> > @@ -143,8 +143,8 @@ rebase_depth_stencil(struct brw_context *brw, struct
> > intel_renderbuffer *irb,
> > struct gl_context *ctx = &brw->ctx;
> > uint32_t tile_mask_x = 0, tile_mask_y = 0;
> >
> > - intel_get_tile_masks(irb->mt->tiling, irb->mt->cpp,
> > - &tile_mask_x, &tile_mask_y);
> > + const unsigned cpp = isl_format_get_layout(irb->mt->surf.format)->bpb
> > / 8;
> > + intel_get_tile_masks(I915_TILING_Y, cpp, &tile_mask_x, &tile_mask_y);
> > assert(!intel_miptree_level_has_hiz(irb->mt, irb->mt_level));
> >
> > uint32_t tile_x = irb->draw_x & tile_mask_x;
> > @@ -313,8 +313,7 @@ brw_emit_depthbuffer(struct brw_context *brw)
> > /* Prior to Gen7, if using separate stencil, hiz must be enabled. */
> > assert(brw->gen >= 7 || !separate_stencil || hiz);
> >
> > - assert(brw->gen < 6 || depth_mt->tiling == I915_TILING_Y);
> > - assert(!hiz || depth_mt->tiling == I915_TILING_Y);
> > + assert(depth_mt->surf.tiling == ISL_TILING_Y0);
> >
>
> The top two hunks should probably go in the previous patch.
They can't, do they? They refer to the isl surface unconditionally and only
this patch switches it on (for depth).
>
>
> >
> > depthbuffer_format = brw_depthbuffer_format(brw);
> > depth_surface_type = BRW_SURFACE_2D;
> > @@ -387,7 +386,7 @@ brw_emit_depth_stencil_hiz(struct brw_context *brw,
> >
> > BEGIN_BATCH(len);
> > OUT_BATCH(_3DSTATE_DEPTH_BUFFER << 16 | (len - 2));
> > - OUT_BATCH((depth_mt ? depth_mt->pitch - 1 : 0) |
> > + OUT_BATCH((depth_mt ? depth_mt->surf.row_pitch - 1 : 0) |
> > (depthbuffer_format << 18) |
> > (BRW_TILEWALK_YMAJOR << 26) |
> > (1 << 27) |
> > diff --git a/src/mesa/drivers/dri/i965/gen6_depth_state.c
> > b/src/mesa/drivers/dri/i965/gen6_depth_state.c
> > index 35c4aa537c..3e3d2c629b 100644
> > --- a/src/mesa/drivers/dri/i965/gen6_depth_state.c
> > +++ b/src/mesa/drivers/dri/i965/gen6_depth_state.c
> > @@ -116,7 +116,7 @@ gen6_emit_depth_stencil_hiz(struct brw_context *brw,
> > OUT_BATCH(_3DSTATE_DEPTH_BUFFER << 16 | (7 - 2));
> >
> > /* 3DSTATE_DEPTH_BUFFER dw1 */
> > - OUT_BATCH((depth_mt ? depth_mt->pitch - 1 : 0) |
> > + OUT_BATCH((depth_mt ? depth_mt->surf.row_pitch - 1 : 0) |
> > (depthbuffer_format << 18) |
> > ((enable_hiz_ss ? 1 : 0) << 21) | /* separate stencil enable
> > */
> > ((enable_hiz_ss ? 1 : 0) << 22) | /* hiz enable */
> > diff --git a/src/mesa/drivers/dri/i965/gen7_misc_state.c
> > b/src/mesa/drivers/dri/i965/gen7_misc_state.c
> > index f272e05860..6b519527e9 100644
> > --- a/src/mesa/drivers/dri/i965/gen7_misc_state.c
> > +++ b/src/mesa/drivers/dri/i965/gen7_misc_state.c
> > @@ -109,7 +109,7 @@ gen7_emit_depth_stencil_hiz(struct brw_context *brw,
> > OUT_BATCH(GEN7_3DSTATE_DEPTH_BUFFER << 16 | (7 - 2));
> >
> > /* 3DSTATE_DEPTH_BUFFER dw1 */
> > - OUT_BATCH((depth_mt ? depth_mt->pitch - 1 : 0) |
> > + OUT_BATCH((depth_mt ? depth_mt->surf.row_pitch - 1 : 0) |
> > (depthbuffer_format << 18) |
> > ((hiz ? 1 : 0) << 22) |
> > ((stencil_mt != NULL && ctx->Stencil._WriteEnabled) << 27) |
> > diff --git a/src/mesa/drivers/dri/i965/gen8_depth_state.c
> > b/src/mesa/drivers/dri/i965/gen8_depth_state.c
> > index 7729fac091..d826654cc4 100644
> > --- a/src/mesa/drivers/dri/i965/gen8_depth_state.c
> > +++ b/src/mesa/drivers/dri/i965/gen8_depth_state.c
> > @@ -67,7 +67,7 @@ emit_depth_packets(struct brw_context *brw,
> > (stencil_mt != NULL && stencil_writable) << 27 |
> > (hiz ? 1 : 0) << 22 |
> > depthbuffer_format << 18 |
> > - (depth_mt ? depth_mt->pitch - 1 : 0));
> > + (depth_mt ? depth_mt->surf.row_pitch - 1 : 0));
> >
>
> Wow, this code is repeated too many times...
>
>
> > if (depth_mt) {
> > OUT_RELOC64(depth_mt->bo,
> > I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER, 0);
> > @@ -78,7 +78,8 @@ emit_depth_packets(struct brw_context *brw,
> > OUT_BATCH(((width - 1) << 4) | ((height - 1) << 18) | lod);
> > OUT_BATCH(((depth - 1) << 21) | (min_array_element << 10) | mocs_wb);
> > OUT_BATCH(0);
> > - OUT_BATCH(((depth - 1) << 21) | (depth_mt ? depth_mt->qpitch >> 2 :
> > 0));
> > + OUT_BATCH(((depth - 1) << 21) |
> > + (depth_mt ? depth_mt->surf.array_pitch_el_rows >> 2 : 0));
> > ADVANCE_BATCH();
> >
> > if (!hiz) {
> > diff --git a/src/mesa/drivers/dri/i965/intel_fbo.c
> > b/src/mesa/drivers/dri/i965/intel_fbo.c
> > index 04ca480dfa..2b19971996 100644
> > --- a/src/mesa/drivers/dri/i965/intel_fbo.c
> > +++ b/src/mesa/drivers/dri/i965/intel_fbo.c
> > @@ -989,7 +989,9 @@ intel_renderbuffer_move_to_temp(struct brw_context
> > *brw,
> > intel_image->base.Base.TexFormat,
> > 0, 0,
> > width, height, 1,
> > - irb->mt->num_samples,
> > + irb->mt->surf.size > 0 ?
> > + irb->mt->surf.samples :
> > + irb->mt->num_samples,
> > layout_flags);
> >
> > if (intel_miptree_wants_hiz_buffer(brw, new_mt)) {
> > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > index b58e9454d1..beac3085a2 100644
> > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > @@ -501,44 +501,7 @@ intel_miptree_create_layout(struct brw_context *brw,
> > mt->physical_height0 = height0;
> > mt->physical_depth0 = depth0;
> >
> > - if (needs_stencil(brw, mt, format, layout_flags)) {
> > - uint32_t stencil_flags = MIPTREE_LAYOUT_ACCELERATED_UPLOAD;
> > - if (brw->gen == 6) {
> > - stencil_flags |= MIPTREE_LAYOUT_GEN6_HIZ_STENCIL |
> > - MIPTREE_LAYOUT_TILING_ANY;
> > - }
> > -
> > - mt->stencil_mt = intel_miptree_create(brw,
> > - mt->target,
> > - MESA_FORMAT_S_UINT8,
> > - mt->first_level,
> > - mt->last_level,
> > - mt->logical_width0,
> > - mt->logical_height0,
> > - mt->logical_depth0,
> > - num_samples,
> > - stencil_flags);
> > -
> > - if (!mt->stencil_mt) {
> > - intel_miptree_release(&mt);
> > - return NULL;
> > - }
> > - mt->stencil_mt->r8stencil_needs_update = true;
> > -
> > - /* Fix up the Z miptree format for how we're splitting out separate
> > - * stencil. Gen7 expects there to be no stencil bits in its depth
> > buffer.
> > - */
> > - mt->format = intel_depth_format_for_depthstencil_format(mt->
> > format);
> > - mt->cpp = 4;
> > -
> > - if (format == mt->format) {
> > - _mesa_problem(NULL, "Unknown format %s in separate stencil mt\n",
> > - _mesa_get_format_name(mt->format));
> > - }
> > - }
> > -
> > - if (layout_flags & MIPTREE_LAYOUT_GEN6_HIZ_STENCIL)
> > - mt->array_layout = GEN6_HIZ_STENCIL;
> > + assert(!needs_stencil(brw, mt, format, layout_flags));
> >
> > /*
> > * Obey HALIGN_16 constraints for Gen8 and Gen9 buffers which are
> > @@ -748,6 +711,40 @@ fail:
> > return NULL;
> > }
> >
> > +static bool
> > +separate_stencil_surface(struct brw_context *brw,
> > + struct intel_mipmap_tree *mt)
> >
>
> Calling these _surface seems a bit odd since they do create something. Why
> so generic?
>
>
> > +{
> > + mt->stencil_mt = make_surface(brw, mt->target, MESA_FORMAT_S_UINT8,
> > + 0, mt->surf.levels - 1,
> > + mt->surf.logical_level0_px.width,
> > + mt->surf.logical_level0_px.height,
> > + mt->surf.dim == ISL_SURF_DIM_3D ?
> > + mt->surf.logical_level0_px.depth :
> > + mt->surf.logical_level0_px.array_len,
> > + mt->surf.samples, ISL_TILING_W,
> > + ISL_SURF_USAGE_STENCIL_BIT |
> > + ISL_SURF_USAGE_TEXTURE_BIT,
> > + BO_ALLOC_FOR_RENDER, NULL);
> > +
> > + if (!mt->stencil_mt)
> > + return false;
> > +
> > + mt->stencil_mt->r8stencil_needs_update = true;
> > +
> > + return true;
> > +}
> > +
> > +static bool
> > +force_linear_tiling(uint32_t layout_flags)
> > +{
> > + /* ANY includes NONE and Y bit. */
> > + if (layout_flags & MIPTREE_LAYOUT_TILING_Y)
> > + return false;
> > +
> > + return layout_flags & MIPTREE_LAYOUT_TILING_NONE;
> > +}
> > +
> > static struct intel_mipmap_tree *
> > miptree_create(struct brw_context *brw,
> > GLenum target,
> > @@ -767,6 +764,31 @@ miptree_create(struct brw_context *brw,
> > ISL_SURF_USAGE_TEXTURE_BIT,
> > BO_ALLOC_FOR_RENDER, NULL);
> >
> > + const GLenum base_format = _mesa_get_format_base_format(format);
> > + if ((base_format == GL_DEPTH_COMPONENT ||
> > + base_format == GL_DEPTH_STENCIL) &&
> > + !force_linear_tiling(layout_flags)) {
> >
>
> Previous patches deleted support for linear depth... Maybe we should assert
> here instead?
>
>
> > + /* Fix up the Z miptree format for how we're splitting out separate
> > + * stencil. Gen7 expects there to be no stencil bits in its depth
> > buffer.
> > + */
> > + const mesa_format depth_only_format =
> > + intel_depth_format_for_depthstencil_format(format);
> > + struct intel_mipmap_tree *mt = make_surface(
> > + brw, target, brw->gen >= 6 ? depth_only_format : format,
> > + first_level, last_level,
> > + width0, height0, depth0, num_samples, ISL_TILING_Y0,
> > + ISL_SURF_USAGE_DEPTH_BIT | ISL_SURF_USAGE_TEXTURE_BIT,
> > + BO_ALLOC_FOR_RENDER, NULL);
> > +
> > + if (needs_stencil(brw, mt, format, layout_flags) &&
> > + !separate_stencil_surface(brw, mt)) {
> > + intel_miptree_release(&mt);
> > + return NULL;
> > + }
> > +
> > + return mt;
> > + }
> > +
> > struct intel_mipmap_tree *mt;
> > mesa_format tex_format = format;
> > mesa_format etc_format = MESA_FORMAT_NONE;
> > @@ -906,8 +928,31 @@ intel_miptree_create_for_bo(struct brw_context *brw,
> > uint32_t layout_flags)
> >
>
> Does create_for_bo get called on depth surfaces? That's a bit horrifying...
>
>
> > {
> > struct intel_mipmap_tree *mt;
> > + const GLenum target = depth > 1 ? GL_TEXTURE_2D_ARRAY : GL_TEXTURE_2D;
> > + const GLenum base_format = _mesa_get_format_base_format(format);
> > +
> > + if ((base_format == GL_DEPTH_COMPONENT ||
> > + base_format == GL_DEPTH_STENCIL) &&
> > + !force_linear_tiling(layout_flags)) {
> > + const mesa_format depth_only_format =
> > + intel_depth_format_for_depthstencil_format(format);
> > + mt = make_surface(brw, target,
> > + brw->gen >= 6 ? depth_only_format : format,
> > + 0, 0, width, height, depth, 1, ISL_TILING_Y0,
> > + ISL_SURF_USAGE_DEPTH_BIT |
> > ISL_SURF_USAGE_TEXTURE_BIT,
> > + BO_ALLOC_FOR_RENDER, bo);
> > +
> > + if (needs_stencil(brw, mt, format, layout_flags) &&
> > + !separate_stencil_surface(brw, mt)) {
> > + intel_miptree_release(&mt);
> > + return NULL;
> > + }
> > +
> > + brw_bo_reference(bo);
> > + return mt;
> > + }
> > +
> > uint32_t tiling, swizzle;
> > - GLenum target;
> >
> > brw_bo_get_tiling(bo, &tiling, &swizzle);
> >
> > @@ -922,8 +967,6 @@ intel_miptree_create_for_bo(struct brw_context *brw,
> > */
> > assert(pitch >= 0);
> >
> > - target = depth > 1 ? GL_TEXTURE_2D_ARRAY : GL_TEXTURE_2D;
> > -
> > /* The BO already has a tiling format and we shouldn't confuse the
> > lower
> > * layers by making it try to find a tiling format again.
> > */
> > @@ -1869,18 +1912,15 @@ intel_miptree_alloc_hiz(struct brw_context *brw,
> > if (!aux_state)
> > return false;
> >
> > - struct isl_surf temp_main_surf;
> > struct isl_surf temp_hiz_surf;
> > -
> > - intel_miptree_get_isl_surf(brw, mt, &temp_main_surf);
> > - isl_surf_get_hiz_surf(&brw->isl_dev, &temp_main_surf, &temp_hiz_surf);
> > + isl_surf_get_hiz_surf(&brw->isl_dev, &mt->surf, &temp_hiz_surf);
> >
> > assert(temp_hiz_surf.size &&
> > (temp_hiz_surf.size % temp_hiz_surf.row_pitch == 0));
> >
> > const uint32_t alloc_flags = BO_ALLOC_FOR_RENDER;
> > mt->hiz_buf = intel_alloc_aux_buffer(brw, "hiz-miptree",
> > - &temp_main_surf, &temp_hiz_surf,
> > + &mt->surf, &temp_hiz_surf,
> > alloc_flags, mt);
> >
> > if (!mt->hiz_buf) {
> > @@ -1918,7 +1958,7 @@ intel_miptree_sample_with_hiz(struct brw_context
> > *brw,
> > * mipmap levels aren't available in the HiZ buffer. So we need all
> > levels
> > * of the texture to be HiZ enabled.
> > */
> > - for (unsigned level = mt->first_level; level <= mt->last_level;
> > ++level) {
> > + for (unsigned level = 0; level < mt->surf.levels; ++level) {
> > if (!intel_miptree_level_has_hiz(mt, level))
> > return false;
> > }
> > @@ -1935,7 +1975,7 @@ intel_miptree_sample_with_hiz(struct brw_context
> > *brw,
> > * There is no such blurb for 1D textures, but there is sufficient
> > evidence
> > * that this is broken on SKL+.
> > */
> > - return (mt->num_samples <= 1 &&
> > + return (mt->surf.samples <= 1 &&
> > mt->target != GL_TEXTURE_3D &&
> > mt->target != GL_TEXTURE_1D /* gen9+ restriction */);
> > }
> > @@ -3271,7 +3311,7 @@ intel_miptree_map_depthstencil(struct brw_context
> > *brw,
> > map_y + s_image_y,
> > brw->has_swizzling);
> > ptrdiff_t z_offset = ((map_y + z_image_y) *
> > - (z_mt->pitch / 4) +
> > + (z_mt->surf.row_pitch / 4) +
> > (map_x + z_image_x));
> > uint8_t s = s_map[s_offset];
> > uint32_t z = z_map[z_offset];
> > @@ -3337,7 +3377,7 @@ intel_miptree_unmap_depthstencil(struct brw_context
> > *brw,
> > y + s_image_y + map->y,
> > brw->has_swizzling);
> > ptrdiff_t z_offset = ((y + z_image_y + map->y) *
> > - (z_mt->pitch / 4) +
> > + (z_mt->surf.row_pitch / 4) +
> > (x + z_image_x + map->x));
> >
> > if (map_z32f_x24s8) {
> > --
> > 2.11.0
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >
More information about the mesa-dev
mailing list