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

[Pohjolainen, Topi]

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.pohjolai...@gmail.com> wrote:
> 
> > Signed-off-by: Topi Pohjolainen 
> > ---
> >  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();
> > -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.
> 
> 
> > +{
> > 

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

[Jason Ekstrand]

On Tue, Jul 18, 2017 at 1:46 AM, Topi Pohjolainen <
topi.pohjolai...@gmail.com> wrote:

> Signed-off-by: Topi Pohjolainen 
> ---
>  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();
> -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,
> +