[Mesa-dev] [PATCH 12/17] i965: Rename array_spacing_lod0 to non_mip_arrays

Pohjolainen, Topi topi.pohjolainen at intel.com
Tue Jul 22 11:14:56 PDT 2014


On Fri, Jul 18, 2014 at 02:16:47PM -0700, Jordan Justen wrote:
> Generalize the name array_spacing_lod0 to non_mip_arrays. Previously
> it was only used in certain cases where only a single mip-level was
> used.
> 
> For gen6 we will use non-mipmapped array spacing, but with multiple
> mip levels. This is needed because gen6 hiz and stencil only support a
> single mip-level.
> 
> 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."
> 
> Signed-off-by: Jordan Justen <jordan.l.justen at intel.com>

I know I complained about the name in the first place, and I'm not too happy
about this either. The structure is still a "mip array" as it supports
multiple layers for multiple levels (I think the original as array of
miptrees and this new layout as miptree of arrays). I don't have anything
better to suggest and hence I'm fine with this or even dropping this patch.

I had some minor comments for the rest of the series and something to be
clarified in patch 16 but 12-17 are:

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             | 2 +-
>  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     | 6 +++---
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.h     | 2 +-
>  7 files changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_blorp.cpp b/src/mesa/drivers/dri/i965/brw_blorp.cpp
> index b57721c..c5ed84a 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->non_mip_arrays = mt->non_mip_arrays;
>     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..0b360c5 100644
> --- a/src/mesa/drivers/dri/i965/brw_blorp.h
> +++ b/src/mesa/drivers/dri/i965/brw_blorp.h
> @@ -153,7 +153,7 @@ public:
>     /* Setting this flag indicates that the surface should be set up in
>      * ARYSPC_LOD0 mode.  Ignored prior to Gen7.
>      */
> -   bool array_spacing_lod0;
> +   bool non_mip_arrays;
>  
>     /**
>      * 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..9e2720b 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->non_mip_arrays)
>        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..c33cfeb 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->non_mip_arrays)
>        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..5d068d4 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->non_mip_arrays)
>        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->non_mip_arrays ? 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..cc3b705 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -330,17 +330,17 @@ intel_miptree_create_layout(struct brw_context *brw,
>        }
>     }
>  
> -   /* array_spacing_lod0 is only used for non-IMS MSAA surfaces.  TODO: can we
> +   /* non_mip_arrays 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->non_mip_arrays = false;
>        break;
>     case INTEL_MSAA_LAYOUT_UMS:
>     case INTEL_MSAA_LAYOUT_CMS:
> -      mt->array_spacing_lod0 = true;
> +      mt->non_mip_arrays = true;
>        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..2d902c3 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> @@ -328,7 +328,7 @@ struct intel_mipmap_tree
>      *
>      * Corresponds to the surface_array_spacing bit in gen7_surface_state.
>      */
> -   bool array_spacing_lod0;
> +   bool non_mip_arrays;
>  
>     /**
>      * The distance in rows between array slices in an uncompressed surface.
> -- 
> 2.0.1
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list