Module: Mesa
Branch: main
Commit: eb0419a1aa1f23dec6bcc908d60d949458067622
URL:    
http://cgit.freedesktop.org/mesa/mesa/commit/?id=eb0419a1aa1f23dec6bcc908d60d949458067622

Author: Tatsuyuki Ishi <ishitatsuy...@gmail.com>
Date:   Sat Nov 25 20:37:03 2023 +0900

radv: Remove aspect mask "expansion" for copy_image.

The Vulkan spec says multi-planar images can only be copied on a
per-plane basis. The COLOR_BIT to "all planes" expansion applies to
image memory barriers which is completely unrelated.

Remove the expansion logic to simplify the code. Add assertions to
clearly describe the invariant.

Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/26364>

---

 src/amd/vulkan/meta/radv_meta_copy.c | 201 +++++++++++++++++------------------
 1 file changed, 96 insertions(+), 105 deletions(-)

diff --git a/src/amd/vulkan/meta/radv_meta_copy.c 
b/src/amd/vulkan/meta/radv_meta_copy.c
index ad4c330bbce..b18851be32e 100644
--- a/src/amd/vulkan/meta/radv_meta_copy.c
+++ b/src/amd/vulkan/meta/radv_meta_copy.c
@@ -402,10 +402,18 @@ copy_image(struct radv_cmd_buffer *cmd_buffer, struct 
radv_image *src_image, VkI
 
    /* From the Vulkan 1.0 spec:
     *
-    *    vkCmdCopyImage can be used to copy image data between multisample
-    *    images, but both images must have the same number of samples.
+    *    vkCmdCopyImage can be used to copy image data between multisample 
images, but both images must have the same
+    *    number of samples.
     */
    assert(src_image->vk.samples == dst_image->vk.samples);
+   /* From the Vulkan 1.3 spec:
+    *
+    *    Multi-planar images can only be copied on a per-plane basis, and the 
subresources used in each region when
+    *    copying to or from such images must specify only one plane, though 
different regions can specify different
+    *    planes.
+    */
+   assert(src_image->plane_count == 1 || 
util_is_power_of_two_nonzero(region->srcSubresource.aspectMask));
+   assert(dst_image->plane_count == 1 || 
util_is_power_of_two_nonzero(region->dstSubresource.aspectMask));
 
    cs = cmd_buffer->qf == RADV_QUEUE_COMPUTE || 
!radv_image_is_renderable(cmd_buffer->device, dst_image);
 
@@ -446,120 +454,103 @@ copy_image(struct radv_cmd_buffer *cmd_buffer, struct 
radv_image *src_image, VkI
       }
    }
 
-   VkImageAspectFlags src_aspects[3] = {region->srcSubresource.aspectMask};
-   VkImageAspectFlags dst_aspects[3] = {region->dstSubresource.aspectMask};
-   unsigned aspect_count = 1;
-
-   if (region->srcSubresource.aspectMask == VK_IMAGE_ASPECT_COLOR_BIT && 
src_image->plane_count > 1) {
-      static const VkImageAspectFlags all_planes[3] = 
{VK_IMAGE_ASPECT_PLANE_0_BIT, VK_IMAGE_ASPECT_PLANE_1_BIT,
-                                                       
VK_IMAGE_ASPECT_PLANE_2_BIT};
-
-      aspect_count = src_image->plane_count;
-      for (unsigned i = 0; i < aspect_count; i++) {
-         src_aspects[i] = all_planes[i];
-         dst_aspects[i] = all_planes[i];
-      }
+   /* Create blit surfaces */
+   struct radv_meta_blit2d_surf b_src = blit_surf_for_image_level_layer(
+      src_image, src_image_layout, &region->srcSubresource, 
region->srcSubresource.aspectMask);
+
+   struct radv_meta_blit2d_surf b_dst = blit_surf_for_image_level_layer(
+      dst_image, dst_image_layout, &region->dstSubresource, 
region->dstSubresource.aspectMask);
+
+   uint32_t dst_queue_mask = radv_image_queue_family_mask(dst_image, 
cmd_buffer->qf, cmd_buffer->qf);
+   bool dst_compressed = radv_layout_dcc_compressed(cmd_buffer->device, 
dst_image, region->dstSubresource.mipLevel,
+                                                    dst_image_layout, 
dst_queue_mask);
+   uint32_t src_queue_mask = radv_image_queue_family_mask(src_image, 
cmd_buffer->qf, cmd_buffer->qf);
+   bool src_compressed = radv_layout_dcc_compressed(cmd_buffer->device, 
src_image, region->srcSubresource.mipLevel,
+                                                    src_image_layout, 
src_queue_mask);
+   bool need_dcc_sign_reinterpret = false;
+
+   if (!src_compressed || 
(radv_dcc_formats_compatible(cmd_buffer->device->physical_device->rad_info.gfx_level,
+                                                       b_src.format, 
b_dst.format, &need_dcc_sign_reinterpret) &&
+                           !need_dcc_sign_reinterpret)) {
+      b_src.format = b_dst.format;
+   } else if (!dst_compressed) {
+      b_dst.format = b_src.format;
+   } else {
+      radv_describe_barrier_start(cmd_buffer, RGP_BARRIER_UNKNOWN_REASON);
+
+      radv_decompress_dcc(cmd_buffer, dst_image,
+                          &(VkImageSubresourceRange){
+                             .aspectMask = region->dstSubresource.aspectMask,
+                             .baseMipLevel = region->dstSubresource.mipLevel,
+                             .levelCount = 1,
+                             .baseArrayLayer = 
region->dstSubresource.baseArrayLayer,
+                             .layerCount = 
vk_image_subresource_layer_count(&dst_image->vk, &region->dstSubresource),
+                          });
+      b_dst.format = b_src.format;
+      b_dst.disable_compression = true;
+
+      radv_describe_barrier_end(cmd_buffer);
    }
 
-   for (unsigned a = 0; a < aspect_count; ++a) {
-      /* Create blit surfaces */
-      struct radv_meta_blit2d_surf b_src =
-         blit_surf_for_image_level_layer(src_image, src_image_layout, 
&region->srcSubresource, src_aspects[a]);
-
-      struct radv_meta_blit2d_surf b_dst =
-         blit_surf_for_image_level_layer(dst_image, dst_image_layout, 
&region->dstSubresource, dst_aspects[a]);
-
-      uint32_t dst_queue_mask = radv_image_queue_family_mask(dst_image, 
cmd_buffer->qf, cmd_buffer->qf);
-      bool dst_compressed = radv_layout_dcc_compressed(cmd_buffer->device, 
dst_image, region->dstSubresource.mipLevel,
-                                                       dst_image_layout, 
dst_queue_mask);
-      uint32_t src_queue_mask = radv_image_queue_family_mask(src_image, 
cmd_buffer->qf, cmd_buffer->qf);
-      bool src_compressed = radv_layout_dcc_compressed(cmd_buffer->device, 
src_image, region->srcSubresource.mipLevel,
-                                                       src_image_layout, 
src_queue_mask);
-      bool need_dcc_sign_reinterpret = false;
-
-      if (!src_compressed || 
(radv_dcc_formats_compatible(cmd_buffer->device->physical_device->rad_info.gfx_level,
-                                                          b_src.format, 
b_dst.format, &need_dcc_sign_reinterpret) &&
-                              !need_dcc_sign_reinterpret)) {
-         b_src.format = b_dst.format;
-      } else if (!dst_compressed) {
-         b_dst.format = b_src.format;
-      } else {
-         radv_describe_barrier_start(cmd_buffer, RGP_BARRIER_UNKNOWN_REASON);
-
-         radv_decompress_dcc(cmd_buffer, dst_image,
-                             &(VkImageSubresourceRange){
-                                .aspectMask = dst_aspects[a],
-                                .baseMipLevel = 
region->dstSubresource.mipLevel,
-                                .levelCount = 1,
-                                .baseArrayLayer = 
region->dstSubresource.baseArrayLayer,
-                                .layerCount = 
vk_image_subresource_layer_count(&dst_image->vk, &region->dstSubresource),
-                             });
-         b_dst.format = b_src.format;
-         b_dst.disable_compression = true;
-
-         radv_describe_barrier_end(cmd_buffer);
-      }
-
-      /**
-       * From the Vulkan 1.0.6 spec: 18.4 Copying Data Between Buffers and 
Images
-       *    imageExtent is the size in texels of the image to copy in width, 
height
-       *    and depth. 1D images use only x and width. 2D images use x, y, 
width
-       *    and height. 3D images use x, y, z, width, height and depth.
-       *
-       * Also, convert the offsets and extent from units of texels to units of
-       * blocks - which is the highest resolution accessible in this command.
-       */
-      const VkOffset3D dst_offset_el = 
vk_image_offset_to_elements(&dst_image->vk, region->dstOffset);
-      const VkOffset3D src_offset_el = 
vk_image_offset_to_elements(&src_image->vk, region->srcOffset);
-
-      /*
-       * From Vulkan 1.0.68, "Copying Data Between Images":
-       *    "When copying between compressed and uncompressed formats
-       *     the extent members represent the texel dimensions of the
-       *     source image and not the destination."
-       * However, we must use the destination image type to avoid
-       * clamping depth when copying multiple layers of a 2D image to
-       * a 3D image.
-       */
-      const VkExtent3D img_extent_el = 
vk_image_extent_to_elements(&src_image->vk, region->extent);
+   /**
+    * From the Vulkan 1.0.6 spec: 18.4 Copying Data Between Buffers and Images
+    *    imageExtent is the size in texels of the image to copy in width, 
height
+    *    and depth. 1D images use only x and width. 2D images use x, y, width
+    *    and height. 3D images use x, y, z, width, height and depth.
+    *
+    * Also, convert the offsets and extent from units of texels to units of
+    * blocks - which is the highest resolution accessible in this command.
+    */
+   const VkOffset3D dst_offset_el = 
vk_image_offset_to_elements(&dst_image->vk, region->dstOffset);
+   const VkOffset3D src_offset_el = 
vk_image_offset_to_elements(&src_image->vk, region->srcOffset);
+
+   /*
+    * From Vulkan 1.0.68, "Copying Data Between Images":
+    *    "When copying between compressed and uncompressed formats
+    *     the extent members represent the texel dimensions of the
+    *     source image and not the destination."
+    * However, we must use the destination image type to avoid
+    * clamping depth when copying multiple layers of a 2D image to
+    * a 3D image.
+    */
+   const VkExtent3D img_extent_el = 
vk_image_extent_to_elements(&src_image->vk, region->extent);
 
-      /* Start creating blit rect */
-      struct radv_meta_blit2d_rect rect = {
-         .width = img_extent_el.width,
-         .height = img_extent_el.height,
-      };
+   /* Start creating blit rect */
+   struct radv_meta_blit2d_rect rect = {
+      .width = img_extent_el.width,
+      .height = img_extent_el.height,
+   };
 
-      unsigned num_slices = vk_image_subresource_layer_count(&src_image->vk, 
&region->srcSubresource);
+   unsigned num_slices = vk_image_subresource_layer_count(&src_image->vk, 
&region->srcSubresource);
 
-      if (src_image->vk.image_type == VK_IMAGE_TYPE_3D) {
-         b_src.layer = src_offset_el.z;
-         num_slices = img_extent_el.depth;
-      }
+   if (src_image->vk.image_type == VK_IMAGE_TYPE_3D) {
+      b_src.layer = src_offset_el.z;
+      num_slices = img_extent_el.depth;
+   }
 
-      if (dst_image->vk.image_type == VK_IMAGE_TYPE_3D)
-         b_dst.layer = dst_offset_el.z;
+   if (dst_image->vk.image_type == VK_IMAGE_TYPE_3D)
+      b_dst.layer = dst_offset_el.z;
 
-      for (unsigned slice = 0; slice < num_slices; slice++) {
-         /* Finish creating blit rect */
-         rect.dst_x = dst_offset_el.x;
-         rect.dst_y = dst_offset_el.y;
-         rect.src_x = src_offset_el.x;
-         rect.src_y = src_offset_el.y;
+   for (unsigned slice = 0; slice < num_slices; slice++) {
+      /* Finish creating blit rect */
+      rect.dst_x = dst_offset_el.x;
+      rect.dst_y = dst_offset_el.y;
+      rect.src_x = src_offset_el.x;
+      rect.src_y = src_offset_el.y;
 
-         /* Perform Blit */
-         if (cs) {
-            radv_meta_image_to_image_cs(cmd_buffer, &b_src, &b_dst, 1, &rect);
+      /* Perform Blit */
+      if (cs) {
+         radv_meta_image_to_image_cs(cmd_buffer, &b_src, &b_dst, 1, &rect);
+      } else {
+         if (radv_can_use_fmask_copy(cmd_buffer, b_src.image, b_dst.image, 1, 
&rect)) {
+            radv_fmask_copy(cmd_buffer, &b_src, &b_dst);
          } else {
-            if (radv_can_use_fmask_copy(cmd_buffer, b_src.image, b_dst.image, 
1, &rect)) {
-               radv_fmask_copy(cmd_buffer, &b_src, &b_dst);
-            } else {
-               radv_meta_blit2d(cmd_buffer, &b_src, NULL, &b_dst, 1, &rect);
-            }
+            radv_meta_blit2d(cmd_buffer, &b_src, NULL, &b_dst, 1, &rect);
          }
-
-         b_src.layer++;
-         b_dst.layer++;
       }
+
+      b_src.layer++;
+      b_dst.layer++;
    }
 
    if (cs) {

Reply via email to