[Mesa-dev] [PATCH v2] i965/skl: Don't use ALL_SLICES_AT_EACH_LOD

[Neil Roberts]

The render surface state command for Skylake doesn't have the surface
array spacing bit so it's not possible to select this layout. I think
it was only used in order to make it pick a tightly-packed qpitch
value that doesn't include space for the mipmaps. However this won't
be necessary after the next patch because it will automatically pick a
packed qpitch value whenever first_level==last_level. It is better to
remove this layout entirely on Gen8+ because although it can
effectively be implemented with a small qpitch value when there are no
mipmaps it isn't possible to support the case where there are mipmaps
because in that case the layout is very different.

It could be good to make a similar change for Gen8 if we also change
the layouting code to pick the qpitch value in a similar way.

v2: Make the commit message and comments more convincing
---

Here is a v2 of the patch which just changes the commit message and
comment now that I understand it a bit better. I'd like to move this
to before the qpitch patch because then the bit about causing a kernel
panic doesn't apply.

I'm also going to send a v3 of the qpitch patch which adds the assert
about never using ALL_SLICES_AT_EACH_LOD.

I've pushed all of my Skylake patches to a branch here to make it
easier to track them:

 https://github.com/bpeel/mesa/tree/wip/skylake

 src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 30 ++++++++++++++++++---------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
index eb226d5..ce777f1 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -371,19 +371,29 @@ intel_miptree_create_layout(struct brw_context *brw,
       }
    }
 
-   /* 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.
+   /* Set array_layout to ALL_SLICES_AT_EACH_LOD when array_spacing_lod0 can
+    * be used. array_spacing_lod0 is only used for non-IMS MSAA surfaces on
+    * Gen 7 and 8. On Gen 8 and 9 this layout is not available but it is still
+    * used on Gen8 to make it pick a qpitch value which doesn't include space
+    * for the mipmaps. On Gen9 this is not necessary because it will
+    * automatically pick a packed qpitch value whenever mt->first_level ==
+    * mt->last_level.
     * TODO: can we use it elsewhere?
+    * TODO: also disable this on Gen8 and pick the qpitch value like Gen9
     */
-   switch (mt->msaa_layout) {
-   case INTEL_MSAA_LAYOUT_NONE:
-   case INTEL_MSAA_LAYOUT_IMS:
+   if (brw->gen >= 9) {
       mt->array_layout = ALL_LOD_IN_EACH_SLICE;
-      break;
-   case INTEL_MSAA_LAYOUT_UMS:
-   case INTEL_MSAA_LAYOUT_CMS:
-      mt->array_layout = ALL_SLICES_AT_EACH_LOD;
-      break;
+   } else {
+      switch (mt->msaa_layout) {
+      case INTEL_MSAA_LAYOUT_NONE:
+      case INTEL_MSAA_LAYOUT_IMS:
+         mt->array_layout = ALL_LOD_IN_EACH_SLICE;
+         break;
+      case INTEL_MSAA_LAYOUT_UMS:
+      case INTEL_MSAA_LAYOUT_CMS:
+         mt->array_layout = ALL_SLICES_AT_EACH_LOD;
+         break;
+      }
    }
 
    if (target == GL_TEXTURE_CUBE_MAP) {
-- 
1.9.3