On Tue, Jul 22, 2014 at 11:14 AM, Pohjolainen, Topi
<topi.pohjolai...@intel.com> wrote:
> 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.jus...@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 don't want leave it as array_spacing_lod0, since that has a specific
hardware meaning, and we are extending the use of the field.
What about something like this?
enum miptree_array_layout {
/* Each array slice contains all miplevels packed together.
*
* Gen hardware usually wants miptrees
* configured this way.
*/
ALL_LOD_IN_EACH_SLICE,
/* Each LOD contains all slices of that LOD packed together.
*
* Multisampled surfaces use this for array_spacing_lod0.
*
* Gen6 uses this for separate stencil and hiz.
*/
ALL_SLICES_AT_EACH_LOD,
};
So, code might look like:
if (mt->array_layout == ALL_SLICES_AT_EACH_LOD)
-Jordan
> 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.pohjolai...@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@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
