[Mesa-dev] [PATCH v3 14/19] i965: Change mipmap array_spacing_lod0 to array_layout (enum)
Kenneth Graunke
kenneth at whitecape.org
Tue Aug 12 14:20:27 PDT 2014
On Tuesday, August 12, 2014 02:06:31 PM Kenneth Graunke wrote:
> On Friday, August 01, 2014 12:53:44 AM Jordan Justen wrote:
> > We will want to setup gen6 separate stencil and hiz miptrees in a
> > layout that is similar to array_spacing_lod0. This is needed because
> > gen6 hiz and stencil only support a single mip-level.
> >
> > In both use cases (gen7+ LOD0 spacing & gen6 separate stencil/hiz),
> > the array slices will be packed at each LOD without reserving extra
> > space for LODs within each array slice.
> >
> > So, we generalize the name of this field and add comments to indicate
> > the old and new uses.
> >
> > Motivation for the gen6 change comes from the PRM:
> >
> > PRM Volume 1, Part 1, 7.18.3.7.2 For separate stencil buffer [DevILK]
> > to [DevSNB]:
> > "The separate stencil buffer does not support mip mapping, thus the
> > storage for LODs other than LOD 0 is not needed."
> >
> > PRM Volume 2, Part 1, 7.5.3 Hierarchical Depth Buffer
> > "[DevSNB]: The hierarchical depth buffer does not support the LOD
> > field, it is assumed by hardware to be zero. A separate
> > hierarachical depth buffer is required for each LOD used, and the
> > corresponding buffer’s state delivered to hardware each time a new
> > depth buffer state with modified LOD is delivered."
> >
> > v2:
> > * Rename array_spacing_lod0 to non_mip_arrays
> > v3:
> > * Instead, replace array_spacing_lod0 with array_layout enum
> >
> > Signed-off-by: Jordan Justen <jordan.l.justen at intel.com>
> > Reviewed-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
> > ---
> > src/mesa/drivers/dri/i965/brw_blorp.cpp | 2 +-
> > src/mesa/drivers/dri/i965/brw_blorp.h | 10 ++++++---
> > src/mesa/drivers/dri/i965/brw_tex_layout.c | 2 +-
> > src/mesa/drivers/dri/i965/gen7_blorp.cpp | 2 +-
> > src/mesa/drivers/dri/i965/gen7_wm_surface_state.c | 6 ++---
> > src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 9 ++++----
> > src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 27 ++++++++++++++++++-----
> > 7 files changed, 39 insertions(+), 19 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_blorp.cpp b/src/mesa/drivers/dri/i965/brw_blorp.cpp
> > index b57721c..6b161c9 100644
> > --- a/src/mesa/drivers/dri/i965/brw_blorp.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_blorp.cpp
> > @@ -82,7 +82,7 @@ brw_blorp_surface_info::set(struct brw_context *brw,
> > {
> > brw_blorp_mip_info::set(mt, level, layer);
> > this->num_samples = mt->num_samples;
> > - this->array_spacing_lod0 = mt->array_spacing_lod0;
> > + this->array_layout = mt->array_layout;
> > this->map_stencil_as_y_tiled = false;
> > this->msaa_layout = mt->msaa_layout;
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_blorp.h b/src/mesa/drivers/dri/i965/brw_blorp.h
> > index 683f09e..a301724 100644
> > --- a/src/mesa/drivers/dri/i965/brw_blorp.h
> > +++ b/src/mesa/drivers/dri/i965/brw_blorp.h
> > @@ -150,10 +150,14 @@ public:
> >
> > unsigned num_samples;
> >
> > - /* Setting this flag indicates that the surface should be set up in
> > - * ARYSPC_LOD0 mode. Ignored prior to Gen7.
> > + /**
> > + * Indicates if we use the standard miptree layout (ALL_LOD_IN_EACH_SLICE),
> > + * or if we tightly pack array slices at each LOD (ALL_SLICES_AT_EACH_LOD).
> > + *
> > + * If ALL_SLICES_AT_EACH_LOD is set, then ARYSPC_LOD0 can be used. Ignored
> > + * prior to Gen7.
> > */
> > - bool array_spacing_lod0;
> > + enum miptree_array_layout array_layout;
> >
> > /**
> > * Format that should be used when setting up the surface state for this
> > diff --git a/src/mesa/drivers/dri/i965/brw_tex_layout.c b/src/mesa/drivers/dri/i965/brw_tex_layout.c
> > index 76044b2..1ed62a6 100644
> > --- a/src/mesa/drivers/dri/i965/brw_tex_layout.c
> > +++ b/src/mesa/drivers/dri/i965/brw_tex_layout.c
> > @@ -241,7 +241,7 @@ brw_miptree_layout_texture_array(struct brw_context *brw,
> >
> > h0 = ALIGN(mt->physical_height0, mt->align_h);
> > h1 = ALIGN(minify(mt->physical_height0, 1), mt->align_h);
> > - if (mt->array_spacing_lod0)
> > + if (mt->array_layout == ALL_SLICES_AT_EACH_LOD)
> > mt->qpitch = h0;
> > else
> > mt->qpitch = (h0 + h1 + (brw->gen >= 7 ? 12 : 11) * mt->align_h);
> > diff --git a/src/mesa/drivers/dri/i965/gen7_blorp.cpp b/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> > index 0ad570b..fee25f7 100644
> > --- a/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> > +++ b/src/mesa/drivers/dri/i965/gen7_blorp.cpp
> > @@ -169,7 +169,7 @@ gen7_blorp_emit_surface_state(struct brw_context *brw,
> > if (surface->mt->align_w == 8)
> > surf[0] |= GEN7_SURFACE_HALIGN_8;
> >
> > - if (surface->array_spacing_lod0)
> > + if (surface->array_layout == ALL_SLICES_AT_EACH_LOD)
> > surf[0] |= GEN7_SURFACE_ARYSPC_LOD0;
> > else
> > surf[0] |= GEN7_SURFACE_ARYSPC_FULL;
> > diff --git a/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c b/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
> > index 01120af..a11106f 100644
> > --- a/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
> > +++ b/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
> > @@ -315,7 +315,7 @@ gen7_update_texture_surface(struct gl_context *ctx,
> > uint32_t effective_depth = (tObj->Immutable && tObj->Target != GL_TEXTURE_3D)
> > ? tObj->NumLayers : mt->logical_depth0;
> >
> > - if (mt->array_spacing_lod0)
> > + if (mt->array_layout == ALL_SLICES_AT_EACH_LOD)
> > surf[0] |= GEN7_SURFACE_ARYSPC_LOD0;
> >
> > surf[1] = mt->bo->offset64 + mt->offset; /* reloc */
> > @@ -508,8 +508,8 @@ gen7_update_renderbuffer_surface(struct brw_context *brw,
> >
> > surf[0] = surftype << BRW_SURFACE_TYPE_SHIFT |
> > format << BRW_SURFACE_FORMAT_SHIFT |
> > - (irb->mt->array_spacing_lod0 ? GEN7_SURFACE_ARYSPC_LOD0
> > - : GEN7_SURFACE_ARYSPC_FULL) |
> > + (irb->mt->array_layout == ALL_SLICES_AT_EACH_LOD ?
> > + GEN7_SURFACE_ARYSPC_LOD0 : GEN7_SURFACE_ARYSPC_FULL) |
> > gen7_surface_tiling_mode(mt->tiling);
> >
> > if (irb->mt->align_h == 4)
> > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > index b36ffc7..6467177 100644
> > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > @@ -330,17 +330,18 @@ intel_miptree_create_layout(struct brw_context *brw,
> > }
> > }
> >
> > - /* array_spacing_lod0 is only used for non-IMS MSAA surfaces. TODO: can we
> > - * use it elsewhere?
> > + /* Set array_layout to ALL_SLICES_AT_EACH_LOD when gen7+ array_spacing_lod0
> > + * can be used. array_spacing_lod0 is only used for non-IMS MSAA surfaces.
> > + * TODO: can we use it elsewhere?
> > */
> > switch (mt->msaa_layout) {
> > case INTEL_MSAA_LAYOUT_NONE:
> > case INTEL_MSAA_LAYOUT_IMS:
> > - mt->array_spacing_lod0 = false;
> > + mt->array_layout = ALL_LOD_IN_EACH_SLICE;
> > break;
> > case INTEL_MSAA_LAYOUT_UMS:
> > case INTEL_MSAA_LAYOUT_CMS:
> > - mt->array_spacing_lod0 = true;
> > + mt->array_layout = ALL_SLICES_AT_EACH_LOD;
> > break;
> > }
> >
> > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> > index bb04084..be1bcb8 100644
> > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> > @@ -255,6 +255,24 @@ enum intel_fast_clear_state
> > INTEL_FAST_CLEAR_STATE_CLEAR,
> > };
> >
> > +enum miptree_array_layout {
> > + /* Each array slice contains all miplevels packed together.
> > + *
> > + * Gen hardware usually wants multilevel miptrees configured this way.
> > + */
> > + ALL_LOD_IN_EACH_SLICE,
> > +
> > + /* Each LOD contains all slices of that LOD packed together.
> > + *
> > + * In some situations, Gen7+ hardware can use the array_spacing_lod0
> > + * feature to save space when the surface only contains LOD 0.
> > + *
> > + * Gen6 uses this for separate stencil and hiz since gen6 does not support
> > + * multiple LODs for separate stencil and hiz.
> > + */
> > + ALL_SLICES_AT_EACH_LOD,
>
> Sorry for missing the original discussion, but I really don't like these names. array_spacing_lod0 made sense to me, but "all slices at each LOD" sounds like a layout:
>
> [lod 0]:
> [slice 0]
> [slice 1]
> [slice 2]
> [slice 3]
> [lod 1:]
> [slice 0]
> [slice 1]
> [slice 2]
> [slice 3]
>
> which definitely doesn't exist.
Oh, except that's exactly what you end up doing in the subsequent patches...so that you can use a single BO instead of storing separate buffers for each miplevel.
Interesting...
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140812/607be362/attachment.sig>
More information about the mesa-dev
mailing list