From 0e1886efb9e151c8ffd5cb89f54c966a54a4fe74 Mon Sep 17 00:00:00 2001 From: Alex Smith Date: Wed, 12 Jul 2017 10:29:52 +0100 Subject: [PATCH] radv: Fix descriptors for cube images with VK_IMAGE_USAGE_STORAGE_BIT If a cube image has VK_IMAGE_USAGE_STORAGE_BIT set, the type in an image view's descriptor was set to a 2D array (and a few other fields adjusted accordingly). This is correct when the image view is actually bound as a storage image, but not when bound as a sampled image. In that case the type should be set as a cube. Fix by generating 2 sets of descriptors at view creation time for both storage and non-storage usage, and then choose between them based on descriptor type when writing descriptor sets. v2: Generate storage descriptors for images with TRANSFER_DST, since those may be used as storage images internally. Signed-off-by: Alex Smith Reviewed-by: Bas Nieuwenhuizen --- src/amd/vulkan/radv_descriptor_set.c | 18 +++++-- src/amd/vulkan/radv_image.c | 79 +++++++++++++++++++--------- src/amd/vulkan/radv_private.h | 6 +++ 3 files changed, 74 insertions(+), 29 deletions(-) diff --git a/src/amd/vulkan/radv_descriptor_set.c b/src/amd/vulkan/radv_descriptor_set.c index b482843539d..7cee9d4dedd 100644 --- a/src/amd/vulkan/radv_descriptor_set.c +++ b/src/amd/vulkan/radv_descriptor_set.c @@ -602,11 +602,18 @@ write_image_descriptor(struct radv_device *device, struct radv_cmd_buffer *cmd_buffer, unsigned *dst, struct radeon_winsys_bo **buffer_list, + VkDescriptorType descriptor_type, const VkDescriptorImageInfo *image_info) { RADV_FROM_HANDLE(radv_image_view, iview, image_info->imageView); - memcpy(dst, iview->descriptor, 8 * 4); - memcpy(dst + 8, iview->fmask_descriptor, 8 * 4); + + if (descriptor_type == VK_DESCRIPTOR_TYPE_STORAGE_IMAGE) { + memcpy(dst, iview->storage_descriptor, 8 * 4); + memcpy(dst + 8, iview->storage_fmask_descriptor, 8 * 4); + } else { + memcpy(dst, iview->descriptor, 8 * 4); + memcpy(dst + 8, iview->fmask_descriptor, 8 * 4); + } if (cmd_buffer) device->ws->cs_add_buffer(cmd_buffer->cs, iview->bo, 7); @@ -619,12 +626,13 @@ write_combined_image_sampler_descriptor(struct radv_device *device, struct radv_cmd_buffer *cmd_buffer, unsigned *dst, struct radeon_winsys_bo **buffer_list, + VkDescriptorType descriptor_type, const VkDescriptorImageInfo *image_info, bool has_sampler) { RADV_FROM_HANDLE(radv_sampler, sampler, image_info->sampler); - write_image_descriptor(device, cmd_buffer, dst, buffer_list, image_info); + write_image_descriptor(device, cmd_buffer, dst, buffer_list, descriptor_type, image_info); /* copy over sampler state */ if (has_sampler) memcpy(dst + 16, sampler->state, 16); @@ -695,10 +703,12 @@ void radv_update_descriptor_sets( case VK_DESCRIPTOR_TYPE_STORAGE_IMAGE: case VK_DESCRIPTOR_TYPE_INPUT_ATTACHMENT: write_image_descriptor(device, cmd_buffer, ptr, buffer_list, + writeset->descriptorType, writeset->pImageInfo + j); break; case VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER: write_combined_image_sampler_descriptor(device, cmd_buffer, ptr, buffer_list, + writeset->descriptorType, writeset->pImageInfo + j, !binding_layout->immutable_samplers_offset); if (copy_immutable_samplers) { @@ -865,10 +875,12 @@ void radv_update_descriptor_set_with_template(struct radv_device *device, case VK_DESCRIPTOR_TYPE_STORAGE_IMAGE: case VK_DESCRIPTOR_TYPE_INPUT_ATTACHMENT: write_image_descriptor(device, cmd_buffer, pDst, buffer_list, + templ->entry[i].descriptor_type, (struct VkDescriptorImageInfo *) pSrc); break; case VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER: write_combined_image_sampler_descriptor(device, cmd_buffer, pDst, buffer_list, + templ->entry[i].descriptor_type, (struct VkDescriptorImageInfo *) pSrc, templ->entry[i].has_sampler); if (templ->entry[i].immutable_samplers) diff --git a/src/amd/vulkan/radv_image.c b/src/amd/vulkan/radv_image.c index 115e5a5cc31..245bca31617 100644 --- a/src/amd/vulkan/radv_image.c +++ b/src/amd/vulkan/radv_image.c @@ -325,7 +325,7 @@ static unsigned gfx9_border_color_swizzle(const unsigned char swizzle[4]) static void si_make_texture_descriptor(struct radv_device *device, struct radv_image *image, - bool sampler, + bool is_storage_image, VkImageViewType view_type, VkFormat vk_format, const VkComponentMapping *mapping, @@ -362,7 +362,7 @@ si_make_texture_descriptor(struct radv_device *device, } type = radv_tex_dim(image->type, view_type, image->info.array_size, image->info.samples, - (image->usage & VK_IMAGE_USAGE_STORAGE_BIT)); + is_storage_image); if (type == V_008F1C_SQ_RSRC_IMG_1D_ARRAY) { height = 1; depth = image->info.array_size; @@ -526,7 +526,7 @@ radv_query_opaque_metadata(struct radv_device *device, md->metadata[1] = si_get_bo_metadata_word1(device); - si_make_texture_descriptor(device, image, true, + si_make_texture_descriptor(device, image, false, (VkImageViewType)image->type, image->vk_format, &fixedmapping, 0, image->info.levels - 1, 0, image->info.array_size, @@ -834,6 +834,50 @@ radv_image_create(VkDevice _device, return VK_SUCCESS; } +static void +radv_image_view_make_descriptor(struct radv_image_view *iview, + struct radv_device *device, + const VkImageViewCreateInfo* pCreateInfo, + bool is_storage_image) +{ + RADV_FROM_HANDLE(radv_image, image, pCreateInfo->image); + const VkImageSubresourceRange *range = &pCreateInfo->subresourceRange; + bool is_stencil = iview->aspect_mask == VK_IMAGE_ASPECT_STENCIL_BIT; + uint32_t blk_w; + uint32_t *descriptor; + uint32_t *fmask_descriptor; + + if (is_storage_image) { + descriptor = iview->storage_descriptor; + fmask_descriptor = iview->storage_fmask_descriptor; + } else { + descriptor = iview->descriptor; + fmask_descriptor = iview->fmask_descriptor; + } + + assert(image->surface.blk_w % vk_format_get_blockwidth(image->vk_format) == 0); + blk_w = image->surface.blk_w / vk_format_get_blockwidth(image->vk_format) * vk_format_get_blockwidth(iview->vk_format); + + si_make_texture_descriptor(device, image, is_storage_image, + iview->type, + iview->vk_format, + &pCreateInfo->components, + 0, radv_get_levelCount(image, range) - 1, + range->baseArrayLayer, + range->baseArrayLayer + radv_get_layerCount(image, range) - 1, + iview->extent.width, + iview->extent.height, + iview->extent.depth, + descriptor, + fmask_descriptor); + si_set_mutable_tex_desc_fields(device, image, + is_stencil ? &image->surface.u.legacy.stencil_level[range->baseMipLevel] + : &image->surface.u.legacy.level[range->baseMipLevel], + range->baseMipLevel, + range->baseMipLevel, + blk_w, is_stencil, descriptor); +} + void radv_image_view_init(struct radv_image_view *iview, struct radv_device *device, @@ -841,8 +885,7 @@ radv_image_view_init(struct radv_image_view *iview, { RADV_FROM_HANDLE(radv_image, image, pCreateInfo->image); const VkImageSubresourceRange *range = &pCreateInfo->subresourceRange; - uint32_t blk_w; - bool is_stencil = false; + switch (image->type) { case VK_IMAGE_TYPE_1D: case VK_IMAGE_TYPE_2D: @@ -862,7 +905,6 @@ radv_image_view_init(struct radv_image_view *iview, iview->aspect_mask = pCreateInfo->subresourceRange.aspectMask; if (iview->aspect_mask == VK_IMAGE_ASPECT_STENCIL_BIT) { - is_stencil = true; iview->vk_format = vk_format_stencil_only(iview->vk_format); } else if (iview->aspect_mask == VK_IMAGE_ASPECT_DEPTH_BIT) { iview->vk_format = vk_format_depth_only(iview->vk_format); @@ -879,30 +921,15 @@ radv_image_view_init(struct radv_image_view *iview, iview->extent.height = round_up_u32(iview->extent.height * vk_format_get_blockheight(iview->vk_format), vk_format_get_blockheight(image->vk_format)); - assert(image->surface.blk_w % vk_format_get_blockwidth(image->vk_format) == 0); - blk_w = image->surface.blk_w / vk_format_get_blockwidth(image->vk_format) * vk_format_get_blockwidth(iview->vk_format); iview->base_layer = range->baseArrayLayer; iview->layer_count = radv_get_layerCount(image, range); iview->base_mip = range->baseMipLevel; - si_make_texture_descriptor(device, image, false, - iview->type, - iview->vk_format, - &pCreateInfo->components, - 0, radv_get_levelCount(image, range) - 1, - range->baseArrayLayer, - range->baseArrayLayer + radv_get_layerCount(image, range) - 1, - iview->extent.width, - iview->extent.height, - iview->extent.depth, - iview->descriptor, - iview->fmask_descriptor); - si_set_mutable_tex_desc_fields(device, image, - is_stencil ? &image->surface.u.legacy.stencil_level[range->baseMipLevel] - : &image->surface.u.legacy.level[range->baseMipLevel], - range->baseMipLevel, - range->baseMipLevel, - blk_w, is_stencil, iview->descriptor); + radv_image_view_make_descriptor(iview, device, pCreateInfo, false); + + /* For transfers we may use the image as a storage image. */ + if (image->usage & (VK_IMAGE_USAGE_STORAGE_BIT | VK_IMAGE_USAGE_TRANSFER_DST_BIT)) + radv_image_view_make_descriptor(iview, device, pCreateInfo, true); } bool radv_layout_has_htile(const struct radv_image *image, diff --git a/src/amd/vulkan/radv_private.h b/src/amd/vulkan/radv_private.h index a1674098845..a8bc5c96586 100644 --- a/src/amd/vulkan/radv_private.h +++ b/src/amd/vulkan/radv_private.h @@ -1276,6 +1276,12 @@ struct radv_image_view { uint32_t descriptor[8]; uint32_t fmask_descriptor[8]; + + /* Descriptor for use as a storage image as opposed to a sampled image. + * This has a few differences for cube maps (e.g. type). + */ + uint32_t storage_descriptor[8]; + uint32_t storage_fmask_descriptor[8]; }; struct radv_image_create_info { -- 2.30.2