panvk: Add planar Z24S8 support

It allows us to use AFBC even if separateDepthStencilLayouts=true.
Unfortunately, AFBC(S8) is only supported on v9+, so we keep using
interleaved Z24S8 on earlier gens.

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
Reviewed-by: Eric R. Smith <eric.smith@collabora.com>
Reviewed-by: Christoph Pillmayer <christoph.pillmayer@arm.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/37158>

src/panfrost/vulkan/panvk_host_copy.c

@@ -92,9 +92,10 @@ panvk_copy_image_to_from_memory(struct image_params img,
 
    /* D24S8 is a special case because the aspects are interleaved in a single
     * plane */
-   VkFormat vkfmt = img.img->vk.format == VK_FORMAT_D24_UNORM_S8_UINT ?
+   VkFormat vkfmt = panvk_image_is_interleaved_depth_stencil(img.img)
-      img.img->vk.format :
+                       ? img.img->vk.format
-      vk_format_get_aspect_format(img.img->vk.format, img.subres.aspectMask);
+                       : vk_format_get_aspect_format(img.img->vk.format,
+                                                     img.subres.aspectMask);
    enum pipe_format pfmt = vk_format_to_pipe_format(vkfmt);
    const struct util_format_description *fmt = util_format_description(pfmt);
 

src/panfrost/vulkan/panvk_image.c

@@ -131,6 +131,17 @@ get_iusage(struct panvk_image *image, const VkImageCreateInfo *create_info)
 static unsigned
 get_plane_count(struct panvk_image *image)
 {
+   bool combined_ds = vk_format_aspects(image->vk.format) ==
+                      (VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT);
+
+   /* Only depth+stencil images can be made multiplanar behind the scene. */
+   if (!combined_ds)
+      return vk_format_get_plane_count(image->vk.format);
+
+   struct panvk_physical_device *phys_dev =
+      to_panvk_physical_device(image->vk.base.device->physical);
+   unsigned arch = pan_arch(phys_dev->kmod.props.gpu_id);
+
    /* Z32_S8X24 is not supported on v9+, and we don't want to use it
     * on v7- anyway, because it's less efficient than the multiplanar
     * alternative.
@@ -138,7 +149,49 @@ get_plane_count(struct panvk_image *image)
    if (image->vk.format == VK_FORMAT_D32_SFLOAT_S8_UINT)
       return 2;
 
-   return vk_format_get_plane_count(image->vk.format);
+   assert(image->vk.format == VK_FORMAT_D24_UNORM_S8_UINT);
+
+   /* We can do AFBC(S8) on Valhall and we're thus better off using planar
+    * Z24+S8 so we can use AFBC when separateDepthStencilLayouts=true.
+    */
+   return arch >= 9 ? 2 : 1;
+}
+
+static enum pipe_format
+select_depth_plane_pfmt(struct panvk_image *image)
+{
+   switch (image->vk.format) {
+   case VK_FORMAT_D24_UNORM_S8_UINT:
+      return PIPE_FORMAT_Z24_UNORM_PACKED;
+   case VK_FORMAT_D32_SFLOAT_S8_UINT:
+      return PIPE_FORMAT_Z32_FLOAT;
+   default:
+      UNREACHABLE("Invalid depth+stencil format");
+   }
+}
+
+static enum pipe_format
+select_stencil_plane_pfmt(struct panvk_image *image)
+{
+   switch (image->vk.format) {
+   case VK_FORMAT_D24_UNORM_S8_UINT:
+   case VK_FORMAT_D32_SFLOAT_S8_UINT:
+      return PIPE_FORMAT_S8_UINT;
+   default:
+      UNREACHABLE("Invalid depth+stencil format");
+   }
+}
+
+static enum pipe_format
+select_plane_pfmt(struct panvk_image *image, unsigned plane)
+{
+   if (panvk_image_is_planar_depth_stencil(image)) {
+      return plane > 0 ? select_stencil_plane_pfmt(image)
+                       : select_depth_plane_pfmt(image);
+   }
+
+   VkFormat plane_format = vk_format_get_plane_format(image->vk.format, plane);
+   return vk_format_to_pipe_format(plane_format);
 }
 
 static bool
@@ -184,14 +237,11 @@ panvk_image_can_use_mod(struct panvk_image *image,
          return false;
 
       /* We can't have separate depth/stencil layout transitions with
-       * interleaved Z24S8, so make sure we always disallow AFBC on Z24S8 until
+       * interleaved ZS, so make sure we disallow AFBC on ZS unless
-       * we've extended the image logic to support planar Z24+S8. Note that
+       * it's using a planar layout.
-       * AFBC(S8) is not supported on Bifrost, so we want to keep support for
-       * interleaved Z24S8 to at least have AFBC(Z24S8) when
-       * separateDepthStencilLayouts=false.
        */
       if (image->vk.base.device->enabled_features.separateDepthStencilLayouts &&
-          image->vk.format == VK_FORMAT_D24_UNORM_S8_UINT)
+          panvk_image_is_interleaved_depth_stencil(image))
          return false;
    }
 
@@ -220,26 +270,14 @@ panvk_image_can_use_mod(struct panvk_image *image,
    /* Defer the rest of the checks to the mod handler. */
    struct pan_image_props iprops = {
       .modifier = mod,
-      .format = vk_format_to_pipe_format(image->vk.format),
       .dim = panvk_image_type_to_mali_tex_dim(image->vk.image_type),
       .array_size = image->vk.array_layers,
       .nr_samples = image->vk.samples,
       .nr_slices = image->vk.mip_levels,
    };
-   const unsigned plane_count =
-      image->vk.format == VK_FORMAT_D32_SFLOAT_S8_UINT
-         ? 2
-         : vk_format_get_plane_count(image->vk.format);
 
-   for (uint8_t plane = 0; plane < plane_count; plane++) {
+   for (uint8_t plane = 0; plane < image->plane_count; plane++) {
-      VkFormat format;
+      iprops.format = select_plane_pfmt(image, plane);
-
-      if (image->vk.format == VK_FORMAT_D32_SFLOAT_S8_UINT)
-         format = plane == 0 ? VK_FORMAT_D32_SFLOAT : VK_FORMAT_S8_UINT;
-      else
-         format = vk_format_get_plane_format(image->vk.format, plane);
-
-      iprops.format = vk_format_to_pipe_format(format);
       iprops.extent_px = (struct pan_image_extent){
          .width = vk_format_get_plane_width(image->vk.format, plane,
                                             image->vk.extent.width),
@@ -332,7 +370,7 @@ static bool
 is_disjoint(const struct panvk_image *image)
 {
    assert((image->plane_count > 1 &&
-           image->vk.format != VK_FORMAT_D32_SFLOAT_S8_UINT) ||
+           !vk_format_is_depth_or_stencil(image->vk.format)) ||
           (image->vk.create_flags & VK_IMAGE_CREATE_ALIAS_BIT) ||
           !(image->vk.create_flags & VK_IMAGE_CREATE_DISJOINT_BIT));
    return image->vk.create_flags & VK_IMAGE_CREATE_DISJOINT_BIT;
@@ -364,27 +402,13 @@ panvk_image_init_layouts(struct panvk_image *image,
          pCreateInfo->pNext,
          IMAGE_DRM_FORMAT_MODIFIER_EXPLICIT_CREATE_INFO_EXT);
 
-   image->plane_count = vk_format_get_plane_count(pCreateInfo->format);
-
-   /* Z32_S8X24 is not supported on v9+, and we don't want to use it
-    * on v7- anyway, because it's less efficient than the multiplanar
-    * alternative.
-    */
-   if (image->vk.format == VK_FORMAT_D32_SFLOAT_S8_UINT)
-      image->plane_count = 2;
-
    const struct pan_mod_handler *mod_handler =
       pan_mod_get_handler(arch, image->vk.drm_format_mod);
    struct pan_image_layout_constraints plane_layout = {
       .offset_B = 0,
    };
    for (uint8_t plane = 0; plane < image->plane_count; plane++) {
-      VkFormat format;
+      enum pipe_format pfmt = select_plane_pfmt(image, plane);
-
-      if (image->vk.format == VK_FORMAT_D32_SFLOAT_S8_UINT)
-         format = plane == 0 ? VK_FORMAT_D32_SFLOAT : VK_FORMAT_S8_UINT;
-      else
-         format = vk_format_get_plane_format(image->vk.format, plane);
 
       if (explicit_info) {
          plane_layout = (struct pan_image_layout_constraints){
@@ -396,7 +420,7 @@ panvk_image_init_layouts(struct panvk_image *image,
       image->planes[plane].image = (struct pan_image){
          .props = {
             .modifier = image->vk.drm_format_mod,
-            .format = vk_format_to_pipe_format(format),
+            .format = pfmt,
             .dim = panvk_image_type_to_mali_tex_dim(image->vk.image_type),
             .extent_px = {
                .width = vk_format_get_plane_width(image->vk.format, plane,
@@ -621,7 +645,8 @@ get_image_subresource_layout(const struct panvk_image *image,
        * path because we need to interleave the depth/stencil components. For
        * the stencil aspect, the copied data only needs 1 byte/px instead of 4.
        */
-      if (image->vk.format == VK_FORMAT_D24_UNORM_S8_UINT) {
+      if (image->vk.format == VK_FORMAT_D24_UNORM_S8_UINT &&
+          image->plane_count == 1) {
          switch (subres->aspectMask) {
             case VK_IMAGE_ASPECT_DEPTH_BIT:
                memcpy_size->size = slice_layout->size_B;

src/panfrost/vulkan/panvk_image.h

@@ -6,6 +6,9 @@
 #ifndef PANVK_IMAGE_H
 #define PANVK_IMAGE_H
 
+#include "util/format/u_format.h"
+
+#include "vk_format.h"
 #include "vk_image.h"
 
 #include "pan_image.h"
@@ -64,6 +67,37 @@ panvk_plane_index(const struct panvk_image *image,
    }
 }
 
+static inline bool
+panvk_image_is_interleaved_depth_stencil(const struct panvk_image *image){
+   return image->plane_count == 1 &&
+          vk_format_aspects(image->vk.format) ==
+             (VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT);
+}
+
+static inline bool
+panvk_image_is_planar_depth_stencil(const struct panvk_image *image){
+   return image->plane_count > 1 &&
+          vk_format_aspects(image->vk.format) ==
+             (VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT);
+}
+
+static inline enum pipe_format
+panvk_image_depth_only_pfmt(const struct panvk_image *image)
+{
+   assert(vk_format_has_depth(image->vk.format));
+
+   return util_format_get_depth_only(image->planes[0].image.props.format);
+}
+
+static inline enum pipe_format
+panvk_image_stencil_only_pfmt(const struct panvk_image *image)
+{
+   assert(vk_format_has_stencil(image->vk.format));
+
+   return util_format_stencil_only(
+      image->planes[image->plane_count - 1].image.props.format);
+}
+
 VkResult panvk_image_init(struct panvk_image *image,
                           const VkImageCreateInfo *pCreateInfo);
 

src/panfrost/vulkan/panvk_meta.h

@@ -97,11 +97,21 @@ panvk_meta_copy_get_image_properties(struct panvk_image *img,
          props.stencil.component_mask = BITFIELD_MASK(1);
          break;
       case VK_FORMAT_D24_UNORM_S8_UINT:
-         props.depth.view_format =
+         if (panvk_image_is_planar_depth_stencil(img)) {
-            use_unorm ? VK_FORMAT_R8G8B8A8_UNORM : VK_FORMAT_R8G8B8A8_UINT;
+            props.depth.view_format =
-         props.depth.component_mask = BITFIELD_MASK(3);
+               use_unorm ? VK_FORMAT_R8G8B8_UNORM : VK_FORMAT_R8G8B8_UINT;
-         props.stencil.view_format = props.depth.view_format;
+            props.depth.component_mask = BITFIELD_MASK(3);
-         props.stencil.component_mask = BITFIELD_BIT(3);
+            props.stencil.view_format =
+               use_unorm ? VK_FORMAT_R8_UNORM : VK_FORMAT_R8_UINT;
+            props.stencil.component_mask = BITFIELD_BIT(0);
+         } else {
+            props.depth.view_format = use_unorm
+                                         ? VK_FORMAT_R8G8B8A8_UNORM
+                                         : VK_FORMAT_R8G8B8A8_UINT;
+            props.depth.component_mask = BITFIELD_MASK(3);
+            props.stencil.view_format = props.depth.view_format;
+            props.stencil.component_mask = BITFIELD_BIT(3);
+         }
          break;
       case VK_FORMAT_X8_D24_UNORM_PACK32:
          props.depth.view_format =
@@ -109,8 +119,9 @@ panvk_meta_copy_get_image_properties(struct panvk_image *img,
          props.depth.component_mask = BITFIELD_MASK(3);
          break;
       case VK_FORMAT_D32_SFLOAT_S8_UINT:
-         props.depth.view_format = use_unorm ? VK_FORMAT_R8G8B8A8_UNORM
+         assert(panvk_image_is_planar_depth_stencil(img));
-                                                    : VK_FORMAT_R8G8B8A8_UINT;
+         props.depth.view_format =
+            use_unorm ? VK_FORMAT_R8G8B8A8_UNORM : VK_FORMAT_R8G8B8A8_UINT;
          props.depth.component_mask = BITFIELD_MASK(4);
          props.stencil.view_format =
             use_unorm ? VK_FORMAT_R8_UNORM : VK_FORMAT_R8_UINT;

src/panfrost/vulkan/panvk_vX_cmd_draw.c

@@ -129,11 +129,9 @@ render_state_set_z_attachment(struct panvk_cmd_buffer *cmdbuf,
    state->render.zs_pview = iview->pview;
    fbinfo->zs.view.zs = &state->render.zs_pview;
 
-   /* D32_S8 is a multiplanar format, so we need to adjust the format of the
+   /* Fixup view format when the image is multiplanar. */
-    * depth-only view to match the one of the depth plane.
+   if (panvk_image_is_planar_depth_stencil(img))
-    */
+      state->render.zs_pview.format = panvk_image_depth_only_pfmt(img);
-   if (iview->pview.format == PIPE_FORMAT_Z32_FLOAT_S8X24_UINT)
-      state->render.zs_pview.format = PIPE_FORMAT_Z32_FLOAT;
 
    state->render.zs_pview.planes[0] = (struct pan_image_plane_ref){
       .image = &img->planes[0].image,
@@ -149,13 +147,12 @@ render_state_set_z_attachment(struct panvk_cmd_buffer *cmdbuf,
     * If we touch the depth component, we need to make sure the stencil
     * component is preserved, hence the preload, and the view format adjusment.
     */
-   if (img->vk.format == VK_FORMAT_D24_UNORM_S8_UINT) {
+   if (panvk_image_is_interleaved_depth_stencil(img)) {
       fbinfo->zs.preload.s = true;
       cmdbuf->state.gfx.render.zs_pview.format =
-         PIPE_FORMAT_Z24_UNORM_S8_UINT;
+         img->planes[0].image.props.format;
    } else {
-      state->render.zs_pview.format =
+      state->render.zs_pview.format = panvk_image_depth_only_pfmt(img);
-         vk_format_to_pipe_format(vk_format_depth_only(img->vk.format));
    }
 
    if (att->loadOp == VK_ATTACHMENT_LOAD_OP_CLEAR)
@@ -195,19 +192,15 @@ render_state_set_s_attachment(struct panvk_cmd_buffer *cmdbuf,
    state->render.s_pview = iview->pview;
    fbinfo->zs.view.s = &state->render.s_pview;
 
-   /* D32_S8 is a multiplanar format, so we need to adjust the format of the
+   if (panvk_image_is_planar_depth_stencil(img)) {
-    * stencil-only view to match the one of the stencil plane.
+      state->render.s_pview.format = panvk_image_stencil_only_pfmt(img);
-    */
-   state->render.s_pview.format = img->vk.format == VK_FORMAT_D24_UNORM_S8_UINT
-                                     ? PIPE_FORMAT_Z24_UNORM_S8_UINT
-                                     : PIPE_FORMAT_S8_UINT;
-   if (img->vk.format == VK_FORMAT_D32_SFLOAT_S8_UINT) {
       state->render.s_pview.planes[0] = (struct pan_image_plane_ref){0};
       state->render.s_pview.planes[1] = (struct pan_image_plane_ref){
          .image = &img->planes[1].image,
          .plane_idx = 0,
       };
    } else {
+      state->render.s_pview.format = panvk_image_stencil_only_pfmt(img);
       state->render.s_pview.planes[0] = (struct pan_image_plane_ref){
          .image = &img->planes[0].image,
          .plane_idx = 0,
@@ -224,7 +217,7 @@ render_state_set_s_attachment(struct panvk_cmd_buffer *cmdbuf,
     * and the format is D24S8, we can combine them in a single view
     * addressing both components.
     */
-   if (img->vk.format == VK_FORMAT_D24_UNORM_S8_UINT &&
+   if (state->render.s_pview.format == PIPE_FORMAT_X24S8_UINT &&
        state->render.z_attachment.iview &&
        state->render.z_attachment.iview->vk.image == iview->vk.image) {
       state->render.zs_pview.format = PIPE_FORMAT_Z24_UNORM_S8_UINT;
@@ -236,6 +229,7 @@ render_state_set_s_attachment(struct panvk_cmd_buffer *cmdbuf,
     * is not supported on the stencil-only slot on Bifrost.
     */
    } else if (img->vk.format == VK_FORMAT_D24_UNORM_S8_UINT &&
+              state->render.s_pview.format == PIPE_FORMAT_X24S8_UINT &&
               fbinfo->zs.view.zs == NULL) {
       fbinfo->zs.view.zs = &state->render.s_pview;
       state->render.s_pview.format = PIPE_FORMAT_Z24_UNORM_S8_UINT;

src/panfrost/vulkan/panvk_vX_image_view.c

@@ -83,12 +83,15 @@ prepare_tex_descs(struct panvk_image_view *view)
    struct panvk_image *image =
       container_of(view->vk.image, struct panvk_image, vk);
    struct panvk_device *dev = to_panvk_device(view->vk.base.device);
+   bool img_combined_ds =
+      vk_format_aspects(image->vk.format) ==
+      (VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT);
+   bool view_combined_ds = view->vk.aspects ==
+      (VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT);
    bool can_preload_other_aspect =
       (view->vk.usage & VK_IMAGE_USAGE_DEPTH_STENCIL_ATTACHMENT_BIT) &&
-      (image->vk.format == VK_FORMAT_D24_UNORM_S8_UINT ||
+      (img_combined_ds &&
-       (image->vk.format == VK_FORMAT_D32_SFLOAT_S8_UINT &&
+       (view_combined_ds || panvk_image_is_interleaved_depth_stencil(image)));
-        view->vk.aspects ==
-           (VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT)));
 
    if (util_format_is_depth_or_stencil(view->pview.format)) {
       /* Vulkan wants R001, where the depth/stencil is stored in the red
@@ -113,8 +116,12 @@ prepare_tex_descs(struct panvk_image_view *view)
    /* If the view contains both stencil and depth, we need to keep only the
     * depth. We'll create another texture with only the stencil.
     */
-   if (pview.format == PIPE_FORMAT_Z32_FLOAT_S8X24_UINT)
+   if (view->vk.aspects & VK_IMAGE_ASPECT_DEPTH_BIT) {
-      pview.format = PIPE_FORMAT_Z32_FLOAT;
+      /* View and image formats must match. */
+      assert(view->vk.format == vk_format_depth_only(image->vk.format) ||
+             view->vk.format == image->vk.format);
+      pview.format = panvk_image_depth_only_pfmt(image);
+   }
 
    uint32_t plane_count = vk_format_get_plane_count(view->vk.format);
    uint32_t tex_payload_size = GENX(pan_texture_estimate_payload_size)(&pview);
@@ -196,23 +203,13 @@ prepare_tex_descs(struct panvk_image_view *view)
    if (!can_preload_other_aspect)
       return VK_SUCCESS;
 
-   switch (pview.format) {
+   /* If the depth was present in the aspects mask, we've handled it already, so
-   case PIPE_FORMAT_Z24X8_UNORM:
+    * move on to the stencil. If it wasn't present, it's the stencil texture we
-   case PIPE_FORMAT_Z24_UNORM_S8_UINT:
+    * create first, and we need t handle the depth here.
-      pview.format = PIPE_FORMAT_X24S8_UINT;
+    */
-      break;
+   pview.format = (view->vk.aspects & VK_IMAGE_ASPECT_DEPTH_BIT)
-   case PIPE_FORMAT_X24S8_UINT:
+                     ? panvk_image_stencil_only_pfmt(image)
-      pview.format = PIPE_FORMAT_Z24X8_UNORM;
+                     : panvk_image_depth_only_pfmt(image);
-      break;
-   case PIPE_FORMAT_Z32_FLOAT:
-      pview.format = PIPE_FORMAT_S8_UINT;
-      break;
-   case PIPE_FORMAT_S8_UINT:
-      pview.format = PIPE_FORMAT_Z32_FLOAT;
-      break;
-   default:
-      assert(!"Invalid format");
-   }
 
    ptr.cpu += tex_payload_size;
    ptr.gpu += tex_payload_size;
@@ -346,7 +343,7 @@ panvk_per_arch(CreateImageView)(VkDevice _device,
 
    /* Depth/stencil are viewed as color for copies. */
    if (view->vk.aspects == VK_IMAGE_ASPECT_COLOR_BIT &&
-       image->vk.format == VK_FORMAT_D32_SFLOAT_S8_UINT &&
+       panvk_image_is_planar_depth_stencil(image) &&
        vk_format_get_blocksize(view->vk.view_format) == 1) {
       view->pview.planes[0] = (struct pan_image_plane_ref) {
          .image = &image->planes[1].image,
@@ -358,9 +355,10 @@ panvk_per_arch(CreateImageView)(VkDevice _device,
     * depth and stencil but the view only contains one of these components, so
     * we can ignore the component we don't use.
     */
-   if (view->vk.view_format == VK_FORMAT_S8_UINT &&
+   if (view->vk.aspects == VK_IMAGE_ASPECT_STENCIL_BIT)
-       image->vk.format == VK_FORMAT_D24_UNORM_S8_UINT)
+      view->pview.format = panvk_image_stencil_only_pfmt(image);
-      view->pview.format = PIPE_FORMAT_X24S8_UINT;
+   else if (view->vk.aspects == VK_IMAGE_ASPECT_DEPTH_BIT)
+      view->pview.format = panvk_image_depth_only_pfmt(image);
 
    /* Attachments need a texture for the FB preload logic. */
    VkImageUsageFlags tex_usage_mask =