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.jus...@intel.com>
> Reviewed-by: Topi Pohjolainen <topi.pohjolai...@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.
How about just renaming "bool array_spacing_lod0" to "bool has_mipmap_levels"?
The field's comment states "if the surface only contains LOD 0". It only
matters if you have multiple array slices, but is theoretically independent.
Or, just leaving it as is...
> +};
> +
> struct intel_mipmap_tree
> {
> /** Buffer object containing the pixel data. */
> @@ -322,13 +340,10 @@ struct intel_mipmap_tree
> uint32_t logical_width0, logical_height0, logical_depth0;
>
> /**
> - * For 1D array, 2D array, cube, and 2D multisampled surfaces on Gen7:
> true
> - * if the surface only contains LOD 0, and hence no space is for LOD's
> - * other than 0 in between array slices.
> - *
> - * Corresponds to the surface_array_spacing bit in gen7_surface_state.
> + * 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).
> */
> - bool array_spacing_lod0;
> + enum miptree_array_layout array_layout;
>
> /**
> * The distance in rows between array slices in an uncompressed surface.
>
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
