From 15642a52ce216a0043eb88447d65d33f8516efd5 Mon Sep 17 00:00:00 2001 From: Chad Versace Date: Sun, 9 Aug 2020 13:46:34 -0700 Subject: [PATCH] anv/image: Further split add_*_surface funcs (v2) Months ago, make_surface() added *all* surfaces required for the given aspect. It was a monster monolithic function, and difficult to reason about its correctness. In commit c652ff8c (2020-03-06), I split the code for aux surfaces into its own function, add_aux_surface_if_supported(). This patch continues the splitting, therefore making bugs easier to identify. Code changes: - Move the code that adds the shadow surface from make_surface() to a new function add_shadow_surface(), called from add_all_surfaces(). - Move the call to add_aux_surface_if_supported() from make_surface() to add_all_surfaces(). - To preserve correctness of the assertions on image layout in make_surface(), move them to the loop in add_all_surfaces() after all the aspect's surfaces have been added. - Rename make_surface() to add_primary_surface() because now that's what it does. Pure refactor, no intended change in behavior. v2: - Rebase onto "anv: Fix isl_surf_usage_flags for stencil images". - Sanitize the image's extent earlier. Reviewed-by: Lionel Landwerlin (v2) Reviewed-by: Jason Ekstrand (v2) --- src/intel/vulkan/anv_image.c | 167 ++++++++++++++++++++--------------- 1 file changed, 97 insertions(+), 70 deletions(-) diff --git a/src/intel/vulkan/anv_image.c b/src/intel/vulkan/anv_image.c index b68028072d2..f17ae753c3b 100644 --- a/src/intel/vulkan/anv_image.c +++ b/src/intel/vulkan/anv_image.c @@ -36,6 +36,13 @@ #include "vk_format_info.h" +static const enum isl_surf_dim +vk_to_isl_surf_dim[] = { + [VK_IMAGE_TYPE_1D] = ISL_SURF_DIM_1D, + [VK_IMAGE_TYPE_2D] = ISL_SURF_DIM_2D, + [VK_IMAGE_TYPE_3D] = ISL_SURF_DIM_3D, +}; + static isl_surf_usage_flags_t choose_isl_surf_usage(VkImageCreateFlags vk_create_flags, VkImageUsageFlags vk_usage, @@ -538,51 +545,58 @@ add_aux_surface_if_supported(struct anv_device *device, return VK_SUCCESS; } +static VkResult +add_shadow_surface(struct anv_device *device, + struct anv_image *image, + uint32_t plane, + struct anv_format_plane plane_format, + uint32_t stride, + VkImageUsageFlags vk_plane_usage) +{ + bool ok; + + ok = isl_surf_init(&device->isl_dev, + &image->planes[plane].shadow_surface.isl, + .dim = vk_to_isl_surf_dim[image->type], + .format = plane_format.isl_format, + .width = image->extent.width, + .height = image->extent.height, + .depth = image->extent.depth, + .levels = image->levels, + .array_len = image->array_size, + .samples = image->samples, + .min_alignment_B = 0, + .row_pitch_B = stride, + .usage = ISL_SURF_USAGE_TEXTURE_BIT | + (vk_plane_usage & ISL_SURF_USAGE_CUBE_BIT), + .tiling_flags = ISL_TILING_ANY_MASK); + + /* isl_surf_init() will fail only if provided invalid input. Invalid input + * here is illegal in Vulkan. + */ + assert(ok); + + add_surface(image, &image->planes[plane].shadow_surface, plane); + return VK_SUCCESS; +} + /** * Initialize the anv_image::*_surface selected by \a aspect. Then update the * image's memory requirements (that is, the image's size and alignment). */ static VkResult -make_surface(struct anv_device *device, - struct anv_image *image, - const VkImageFormatListCreateInfoKHR *fmt_list, - uint32_t stride, - isl_tiling_flags_t tiling_flags, - isl_surf_usage_flags_t isl_extra_usage_flags, - VkImageAspectFlagBits aspect) +add_primary_surface(struct anv_device *device, + struct anv_image *image, + uint32_t plane, + struct anv_format_plane plane_format, + uint32_t stride, + isl_tiling_flags_t isl_tiling_flags, + isl_surf_usage_flags_t isl_usage) { - VkResult result; bool ok; - static const enum isl_surf_dim vk_to_isl_surf_dim[] = { - [VK_IMAGE_TYPE_1D] = ISL_SURF_DIM_1D, - [VK_IMAGE_TYPE_2D] = ISL_SURF_DIM_2D, - [VK_IMAGE_TYPE_3D] = ISL_SURF_DIM_3D, - }; - - image->extent = anv_sanitize_image_extent(image->type, image->extent); - - const unsigned plane = anv_image_aspect_to_plane(image->aspects, aspect); - const struct anv_format_plane plane_format = - anv_get_format_plane(&device->info, image->vk_format, aspect, image->tiling); struct anv_surface *anv_surf = &image->planes[plane].surface; - VkImageUsageFlags plane_vk_usage = - aspect == VK_IMAGE_ASPECT_STENCIL_BIT ? - image->stencil_usage : image->usage; - - const isl_surf_usage_flags_t usage = - choose_isl_surf_usage(image->create_flags, plane_vk_usage, - isl_extra_usage_flags, aspect); - - bool needs_shadow = - anv_image_plane_needs_shadow_surface(&device->info, - plane_format, - image->tiling, - plane_vk_usage, - image->create_flags, - &tiling_flags); - ok = isl_surf_init(&device->isl_dev, &anv_surf->isl, .dim = vk_to_isl_surf_dim[image->type], .format = plane_format.isl_format, @@ -594,8 +608,8 @@ make_surface(struct anv_device *device, .samples = image->samples, .min_alignment_B = 0, .row_pitch_B = stride, - .usage = usage, - .tiling_flags = tiling_flags); + .usage = isl_usage, + .tiling_flags = isl_tiling_flags); if (!ok) return VK_ERROR_OUT_OF_DEVICE_MEMORY; @@ -604,35 +618,12 @@ make_surface(struct anv_device *device, add_surface(image, anv_surf, plane); - if (needs_shadow) { - ok = isl_surf_init(&device->isl_dev, &image->planes[plane].shadow_surface.isl, - .dim = vk_to_isl_surf_dim[image->type], - .format = plane_format.isl_format, - .width = image->extent.width, - .height = image->extent.height, - .depth = image->extent.depth, - .levels = image->levels, - .array_len = image->array_size, - .samples = image->samples, - .min_alignment_B = 0, - .row_pitch_B = stride, - .usage = ISL_SURF_USAGE_TEXTURE_BIT | - (usage & ISL_SURF_USAGE_CUBE_BIT), - .tiling_flags = ISL_TILING_ANY_MASK); - - /* isl_surf_init() will fail only if provided invalid input. Invalid input - * is illegal in Vulkan. - */ - assert(ok); - - add_surface(image, &image->planes[plane].shadow_surface, plane); - } - - result = add_aux_surface_if_supported(device, image, plane, plane_format, - fmt_list, isl_extra_usage_flags); - if (result != VK_SUCCESS) - return result; + return VK_SUCCESS; +} +static void +check_surfaces(const struct anv_image *image, uint32_t plane) +{ assert((image->planes[plane].offset + image->planes[plane].size) == image->size); /* Upper bound of the last surface should be smaller than the plane's @@ -651,8 +642,6 @@ make_surface(struct anv_device *device, assert(image->planes[plane].fast_clear_state_offset < (image->planes[plane].offset + image->planes[plane].size)); } - - return VK_SUCCESS; } static VkResult @@ -663,15 +652,52 @@ add_all_surfaces(struct anv_device *device, isl_tiling_flags_t isl_tiling_flags, isl_surf_usage_flags_t isl_extra_usage_flags) { + const struct gen_device_info *devinfo = &device->info; VkResult result; uint32_t b; for_each_bit(b, image->aspects) { VkImageAspectFlagBits aspect = 1 << b; - result = make_surface(device, image, format_list_info, stride, - isl_tiling_flags, isl_extra_usage_flags, aspect); + uint32_t plane = anv_image_aspect_to_plane(image->aspects, aspect); + const struct anv_format_plane plane_format = + anv_get_format_plane(devinfo, image->vk_format, aspect, image->tiling); + + VkImageUsageFlags vk_usage = image->usage; + if (aspect == VK_IMAGE_ASPECT_STENCIL_BIT) + vk_usage = image->stencil_usage; + + isl_surf_usage_flags_t isl_usage = + choose_isl_surf_usage(image->create_flags, vk_usage, + isl_extra_usage_flags, aspect); + + /* Must call this before adding any surfaces because it may modify + * isl_tiling_flags. + */ + bool needs_shadow = + anv_image_plane_needs_shadow_surface(devinfo, plane_format, + image->tiling, vk_usage, + image->create_flags, + &isl_tiling_flags); + + result = add_primary_surface(device, image, plane, plane_format, stride, + isl_tiling_flags, isl_usage); if (result != VK_SUCCESS) return result; + + if (needs_shadow) { + result = add_shadow_surface(device, image, plane, plane_format, stride, + vk_usage); + if (result != VK_SUCCESS) + return result; + } + + result = add_aux_surface_if_supported(device, image, plane, plane_format, + format_list_info, + isl_extra_usage_flags); + if (result != VK_SUCCESS) + return result; + + check_surfaces(image, plane); } return VK_SUCCESS; @@ -739,7 +765,8 @@ anv_image_create(VkDevice _device, vk_object_base_init(&device->vk, &image->base, VK_OBJECT_TYPE_IMAGE); image->type = pCreateInfo->imageType; - image->extent = pCreateInfo->extent; + image->extent = anv_sanitize_image_extent(pCreateInfo->imageType, + pCreateInfo->extent); image->vk_format = pCreateInfo->format; image->format = anv_get_format(pCreateInfo->format); image->aspects = vk_format_aspects(image->vk_format);