[Mesa-dev] [PATCH 22/22] i965: Represent depth surfaces with isl

Pohjolainen, Topi topi.pohjolainen at gmail.com
Thu Jul 20 04:52:36 UTC 2017


On Tue, Jul 18, 2017 at 02:42:21PM -0700, Jason Ekstrand wrote:
> On Tue, Jul 18, 2017 at 1:46 AM, Topi Pohjolainen <
> topi.pohjolainen at gmail.com> wrote:
> 
> > Signed-off-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
> > ---
> >  src/mesa/drivers/dri/i965/brw_clear.c         |   5 +-
> >  src/mesa/drivers/dri/i965/gen8_depth_state.c  |   3 +-
> >  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 136
> > +++++++++++++++++---------
> >  3 files changed, 97 insertions(+), 47 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_clear.c
> > b/src/mesa/drivers/dri/i965/brw_clear.c
> > index 7fbaa3a47d..c310d2547a 100644
> > --- a/src/mesa/drivers/dri/i965/brw_clear.c
> > +++ b/src/mesa/drivers/dri/i965/brw_clear.c
> > @@ -121,7 +121,8 @@ brw_fast_clear_depth(struct gl_context *ctx)
> >     if ((ctx->Scissor.EnableFlags & 1) && !noop_scissor(fb)) {
> >        perf_debug("Failed to fast clear %dx%d depth because of scissors.  "
> >                   "Possible 5%% performance win if avoided.\n",
> > -                 mt->logical_width0, mt->logical_height0);
> > +                 mt->surf.logical_level0_px.width,
> > +                 mt->surf.logical_level0_px.height);
> >        return false;
> >     }
> >
> > @@ -149,7 +150,7 @@ brw_fast_clear_depth(struct gl_context *ctx)
> >         *        optimization must be disabled.
> >         */
> >        if (brw->gen == 6 &&
> > -          (minify(mt->physical_width0,
> > +          (minify(mt->surf.phys_level0_sa.width,
> >                    depth_irb->mt_level - mt->first_level) % 16) != 0)
> >          return false;
> >        break;
> > diff --git a/src/mesa/drivers/dri/i965/gen8_depth_state.c
> > b/src/mesa/drivers/dri/i965/gen8_depth_state.c
> > index c934d0d21a..5cee93ade0 100644
> > --- a/src/mesa/drivers/dri/i965/gen8_depth_state.c
> > +++ b/src/mesa/drivers/dri/i965/gen8_depth_state.c
> > @@ -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_mipmap_tree.c
> > b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > index 702dcd8635..ea8b2662fd 100644
> > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > @@ -520,43 +520,7 @@ intel_miptree_create_layout(struct brw_context *brw,
> >     mt->physical_height0 = height0;
> >     mt->physical_depth0 = depth0;
> >
> > -   if (needs_separate_stencil(brw, mt, format, layout_flags)) {
> > -      uint32_t stencil_flags = MIPTREE_LAYOUT_ACCELERATED_UPLOAD;
> > -      if (brw->gen == 6) {
> > -         stencil_flags |= 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_separate_stencil(brw, mt, format, layout_flags));
> >
> >     /*
> >      * Obey HALIGN_16 constraints for Gen8 and Gen9 buffers which are
> > @@ -829,6 +793,40 @@ fail:
> >     return NULL;
> >  }
> >
> > +static bool
> > +separate_stencil_surface(struct brw_context *brw,
> > +                         struct intel_mipmap_tree *mt)
> >
> 
> make_separate_stencil_surface?  Also, it seems perfectly reasonable for
> this to return the miptree rather than a bool.
> 
> 
> > +{
> > +   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_BIT,
> > +                                 ISL_SURF_USAGE_STENCIL_BIT |
> > +                                 ISL_SURF_USAGE_TEXTURE_BIT,
> > +                                 BO_ALLOC_FOR_RENDER, 0, 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,
> > @@ -849,6 +847,34 @@ miptree_create(struct brw_context *brw,
> >                            ISL_SURF_USAGE_TEXTURE_BIT,
> >                            BO_ALLOC_FOR_RENDER, 0, 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)) {
> > +      /* 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_BIT,
> > +         ISL_SURF_USAGE_DEPTH_BIT | ISL_SURF_USAGE_TEXTURE_BIT,
> > +         BO_ALLOC_FOR_RENDER, 0, NULL);
> >
> 
> One argument per line please.  This is unreadable.

And I missed this, I'll revise locally.

> 
> 
> > +
> > +      if (needs_separate_stencil(brw, mt, format, layout_flags) &&
> > +          !separate_stencil_surface(brw, mt)) {
> > +         intel_miptree_release(&mt);
> > +         return NULL;
> > +      }
> > +
> > +      if (!(layout_flags & MIPTREE_LAYOUT_DISABLE_AUX))
> > +         intel_miptree_choose_aux_usage(brw, mt);
> > +
> > +      return mt;
> > +   }
> > +
> >     struct intel_mipmap_tree *mt;
> >     mesa_format tex_format = format;
> >     mesa_format etc_format = MESA_FORMAT_NONE;
> > @@ -970,8 +996,31 @@ intel_miptree_create_for_bo(struct brw_context *brw,
> >     struct intel_mipmap_tree *mt;
> >     uint32_t tiling, swizzle;
> >     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)) {
> > +      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_BIT,
> > +                        ISL_SURF_USAGE_DEPTH_BIT |
> > ISL_SURF_USAGE_TEXTURE_BIT,
> > +                        BO_ALLOC_FOR_RENDER, pitch, bo);
> >
> > -   if (format == MESA_FORMAT_S_UINT8) {
> > +      if (needs_separate_stencil(brw, mt, format, layout_flags) &&
> > +          !separate_stencil_surface(brw, mt)) {
> >
> 
> If someone calls create_for_bo and wants separate stencil, that seems bad
> to me.
> 
> 
> > +         intel_miptree_release(&mt);
> > +         return NULL;
> > +      }
> > +
> > +      brw_bo_reference(bo);
> > +
> > +      if (!(layout_flags & MIPTREE_LAYOUT_DISABLE_AUX))
> > +         intel_miptree_choose_aux_usage(brw, mt);
> > +
> > +      return mt;
> > +   } else if (format == MESA_FORMAT_S_UINT8) {
> >        mt = make_surface(brw, target, MESA_FORMAT_S_UINT8,
> >                          0, 0, width, height, depth, 1,
> >                          ISL_TILING_W_BIT,
> > @@ -1966,10 +2015,11 @@ intel_miptree_level_enable_hiz(struct brw_context
> > *brw,
> >                                 uint32_t level)
> >  {
> >     assert(mt->hiz_buf);
> > +   assert(mt->surf.size > 0);
> >
> >     if (brw->gen >= 8 || brw->is_haswell) {
> > -      uint32_t width = minify(mt->physical_width0, level);
> > -      uint32_t height = minify(mt->physical_height0, level);
> > +      uint32_t width = minify(mt->surf.phys_level0_sa.width, level);
> > +      uint32_t height = minify(mt->surf.phys_level0_sa.height, level);
> >
> >        /* Disable HiZ for LOD > 0 unless the width is 8 aligned
> >         * and the height is 4 aligned. This allows our HiZ support
> > @@ -2000,12 +2050,10 @@ 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);
> >     MAYBE_UNUSED bool ok =
> > -      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(ok);
> >
> >     const uint32_t alloc_flags = BO_ALLOC_FOR_RENDER;
> > @@ -2094,7 +2142,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;
> >     }
> > --
> > 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