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, ®ion->srcSubresource, region->srcSubresource.aspectMask); + + struct radv_meta_blit2d_surf b_dst = blit_surf_for_image_level_layer( + dst_image, dst_image_layout, ®ion->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, ®ion->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, ®ion->srcSubresource, src_aspects[a]); - - struct radv_meta_blit2d_surf b_dst = - blit_surf_for_image_level_layer(dst_image, dst_image_layout, ®ion->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, ®ion->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, ®ion->srcSubresource); + unsigned num_slices = vk_image_subresource_layer_count(&src_image->vk, ®ion->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) {