From 04bdbb71de4cfd31f3d6fef74c0e6754228fe941 Mon Sep 17 00:00:00 2001 From: Faith Ekstrand Date: Sat, 6 Jul 2024 11:21:01 -0500 Subject: [PATCH] nvk: Align sparse-bound images to the sparse binding size Instead of trusting in nil::Image::align_B, force it to the sparse binding size because we know we're going to try and sparse bind it. Otherwise, small sparse images could fail to bind at the bind step. Fixes: 7321d151a944 ("nvk: Add support for sparse images") Part-of: --- src/nouveau/vulkan/nvk_image.c | 95 ++++++++++++++-------- src/nouveau/vulkan/nvk_image.h | 4 + src/nouveau/vulkan/nvk_queue_drm_nouveau.c | 18 ++-- 3 files changed, 80 insertions(+), 37 deletions(-) diff --git a/src/nouveau/vulkan/nvk_image.c b/src/nouveau/vulkan/nvk_image.c index e7b9b894ee7..543004d18c9 100644 --- a/src/nouveau/vulkan/nvk_image.c +++ b/src/nouveau/vulkan/nvk_image.c @@ -804,22 +804,40 @@ nvk_image_init(struct nvk_device *dev, return VK_SUCCESS; } -static VkResult -nvk_image_plane_alloc_vma(struct nvk_device *dev, - struct nvk_image_plane *plane, - VkImageCreateFlags create_flags) +void +nvk_image_plane_size_align_B(const struct nvk_image *image, + const struct nvk_image_plane *plane, + uint64_t *size_B_out, uint64_t *align_B_out) { const bool sparse_bound = - create_flags & VK_IMAGE_CREATE_SPARSE_BINDING_BIT; + image->vk.create_flags & VK_IMAGE_CREATE_SPARSE_BINDING_BIT; + + assert(util_is_power_of_two_or_zero64(plane->nil.align_B)); + if (sparse_bound || plane->nil.pte_kind) { + *align_B_out = MAX2(plane->nil.align_B, NVK_SPARSE_BIND_ALIGN_B); + } else { + *align_B_out = plane->nil.align_B; + } + *size_B_out = align64(plane->nil.size_B, *align_B_out); +} + +static VkResult +nvk_image_plane_alloc_vma(struct nvk_device *dev, + const struct nvk_image *image, + struct nvk_image_plane *plane) +{ + const bool sparse_bound = + image->vk.create_flags & VK_IMAGE_CREATE_SPARSE_BINDING_BIT; const bool sparse_resident = - create_flags & VK_IMAGE_CREATE_SPARSE_RESIDENCY_BIT; + image->vk.create_flags & VK_IMAGE_CREATE_SPARSE_RESIDENCY_BIT; assert(sparse_bound || !sparse_resident); if (sparse_bound || plane->nil.pte_kind) { - plane->vma_size_B = plane->nil.size_B; + uint64_t vma_align_B; + nvk_image_plane_size_align_B(image, plane, &plane->vma_size_B, + &vma_align_B); plane->addr = nouveau_ws_alloc_vma(dev->ws_dev, 0, plane->vma_size_B, - plane->nil.align_B, - false, sparse_resident); + vma_align_B, false, sparse_resident); if (plane->addr == 0) { return vk_errorf(dev, VK_ERROR_OUT_OF_DEVICE_MEMORY, "Sparse VMA allocation failed"); @@ -829,7 +847,6 @@ nvk_image_plane_alloc_vma(struct nvk_device *dev, return VK_SUCCESS; } - static void nvk_image_plane_finish(struct nvk_device *dev, struct nvk_image_plane *plane, @@ -906,8 +923,7 @@ nvk_CreateImage(VkDevice _device, } for (uint8_t plane = 0; plane < image->plane_count; plane++) { - result = nvk_image_plane_alloc_vma(dev, &image->planes[plane], - image->vk.create_flags); + result = nvk_image_plane_alloc_vma(dev, image, &image->planes[plane]); if (result != VK_SUCCESS) { nvk_image_finish(dev, image, pAllocator); vk_free2(&dev->vk.alloc, pAllocator, image); @@ -916,8 +932,7 @@ nvk_CreateImage(VkDevice _device, } if (image->stencil_copy_temp.nil.size_B > 0) { - result = nvk_image_plane_alloc_vma(dev, &image->stencil_copy_temp, - image->vk.create_flags); + result = nvk_image_plane_alloc_vma(dev, image, &image->stencil_copy_temp); if (result != VK_SUCCESS) { nvk_image_finish(dev, image, pAllocator); vk_free2(&dev->vk.alloc, pAllocator, image); @@ -962,15 +977,17 @@ nvk_DestroyImage(VkDevice device, } static void -nvk_image_plane_add_req(struct nvk_image_plane *plane, +nvk_image_plane_add_req(const struct nvk_image *image, + const struct nvk_image_plane *plane, uint64_t *size_B, uint32_t *align_B) { assert(util_is_power_of_two_or_zero64(*align_B)); - assert(util_is_power_of_two_or_zero64(plane->nil.align_B)); + uint64_t plane_size_B, plane_align_B; + nvk_image_plane_size_align_B(image, plane, &plane_size_B, &plane_align_B); - *align_B = MAX2(*align_B, plane->nil.align_B); - *size_B = align64(*size_B, plane->nil.align_B); - *size_B += plane->nil.size_B; + *align_B = MAX2(*align_B, plane_align_B); + *size_B = align64(*size_B, plane_align_B); + *size_B += plane_size_B; } static void @@ -988,14 +1005,19 @@ nvk_get_image_memory_requirements(struct nvk_device *dev, uint32_t align_B = 0; if (image->disjoint) { uint8_t plane = nvk_image_memory_aspects_to_plane(image, aspects); - nvk_image_plane_add_req(&image->planes[plane], &size_B, &align_B); + nvk_image_plane_add_req(image, &image->planes[plane], + &size_B, &align_B); } else { - for (unsigned plane = 0; plane < image->plane_count; plane++) - nvk_image_plane_add_req(&image->planes[plane], &size_B, &align_B); + for (unsigned plane = 0; plane < image->plane_count; plane++) { + nvk_image_plane_add_req(image, &image->planes[plane], + &size_B, &align_B); + } } - if (image->stencil_copy_temp.nil.size_B > 0) - nvk_image_plane_add_req(&image->stencil_copy_temp, &size_B, &align_B); + if (image->stencil_copy_temp.nil.size_B > 0) { + nvk_image_plane_add_req(image, &image->stencil_copy_temp, + &size_B, &align_B); + } pMemoryRequirements->memoryRequirements.memoryTypeBits = memory_types; pMemoryRequirements->memoryRequirements.alignment = align_B; @@ -1181,8 +1203,10 @@ nvk_get_image_subresource_layout(UNUSED struct nvk_device *dev, uint64_t offset_B = 0; if (!image->disjoint) { uint32_t align_B = 0; - for (unsigned plane = 0; plane < p; plane++) - nvk_image_plane_add_req(&image->planes[plane], &offset_B, &align_B); + for (unsigned plane = 0; plane < p; plane++) { + nvk_image_plane_add_req(image, &image->planes[plane], + &offset_B, &align_B); + } } offset_B += nil_image_level_layer_offset_B(&plane->nil, isr->mipLevel, isr->arrayLayer); @@ -1228,11 +1252,14 @@ nvk_GetDeviceImageSubresourceLayoutKHR( static void nvk_image_plane_bind(struct nvk_device *dev, + struct nvk_image *image, struct nvk_image_plane *plane, struct nvk_device_memory *mem, uint64_t *offset_B) { - *offset_B = align64(*offset_B, (uint64_t)plane->nil.align_B); + uint64_t plane_size_B, plane_align_B; + nvk_image_plane_size_align_B(image, plane, &plane_size_B, &plane_align_B); + *offset_B = align64(*offset_B, plane_align_B); if (plane->vma_size_B) { nouveau_ws_bo_bind_vma(dev->ws_dev, @@ -1246,7 +1273,7 @@ nvk_image_plane_bind(struct nvk_device *dev, plane->addr = mem->bo->offset + *offset_B; } - *offset_B += plane->nil.size_B; + *offset_B += plane_size_B; } VKAPI_ATTR VkResult VKAPI_CALL @@ -1286,15 +1313,19 @@ nvk_BindImageMemory2(VkDevice device, const VkBindImagePlaneMemoryInfo *plane_info = vk_find_struct_const(pBindInfos[i].pNext, BIND_IMAGE_PLANE_MEMORY_INFO); uint8_t plane = nvk_image_memory_aspects_to_plane(image, plane_info->planeAspect); - nvk_image_plane_bind(dev, &image->planes[plane], mem, &offset_B); + nvk_image_plane_bind(dev, image, &image->planes[plane], + mem, &offset_B); } else { for (unsigned plane = 0; plane < image->plane_count; plane++) { - nvk_image_plane_bind(dev, &image->planes[plane], mem, &offset_B); + nvk_image_plane_bind(dev, image, &image->planes[plane], + mem, &offset_B); } } - if (image->stencil_copy_temp.nil.size_B > 0) - nvk_image_plane_bind(dev, &image->stencil_copy_temp, mem, &offset_B); + if (image->stencil_copy_temp.nil.size_B > 0) { + nvk_image_plane_bind(dev, image, &image->stencil_copy_temp, + mem, &offset_B); + } const VkBindMemoryStatusKHR *status = vk_find_struct_const(pBindInfos[i].pNext, BIND_MEMORY_STATUS_KHR); diff --git a/src/nouveau/vulkan/nvk_image.h b/src/nouveau/vulkan/nvk_image.h index d4cfc71898f..89c8004dc86 100644 --- a/src/nouveau/vulkan/nvk_image.h +++ b/src/nouveau/vulkan/nvk_image.h @@ -146,4 +146,8 @@ nvk_image_memory_aspects_to_plane(ASSERTED const struct nvk_image *image, } } +void nvk_image_plane_size_align_B(const struct nvk_image *image, + const struct nvk_image_plane *plane, + uint64_t *size_B_out, uint64_t *align_B_out); + #endif diff --git a/src/nouveau/vulkan/nvk_queue_drm_nouveau.c b/src/nouveau/vulkan/nvk_queue_drm_nouveau.c index a0087df592a..392d813ba69 100644 --- a/src/nouveau/vulkan/nvk_queue_drm_nouveau.c +++ b/src/nouveau/vulkan/nvk_queue_drm_nouveau.c @@ -310,12 +310,16 @@ next_opaque_bind_plane(const VkSparseMemoryBind *bind, static void push_add_image_plane_opaque_bind(struct push_builder *pb, + const struct nvk_image *image, const struct nvk_image_plane *plane, const VkSparseMemoryBind *bind, uint64_t *image_plane_offset_B) { + uint64_t plane_size_B, plane_align_B; + nvk_image_plane_size_align_B(image, plane, &plane_size_B, &plane_align_B); + uint64_t plane_offset_B, mem_offset_B, bind_size_B; - if (!next_opaque_bind_plane(bind, plane->nil.size_B, plane->nil.align_B, + if (!next_opaque_bind_plane(bind, plane_size_B, plane_align_B, &plane_offset_B, &mem_offset_B, &bind_size_B, image_plane_offset_B)) return; @@ -339,10 +343,14 @@ push_add_image_plane_opaque_bind(struct push_builder *pb, static void push_add_image_plane_mip_tail_bind(struct push_builder *pb, + const struct nvk_image *image, const struct nvk_image_plane *plane, const VkSparseMemoryBind *bind, uint64_t *image_plane_offset_B) { + uint64_t plane_size_B, plane_align_B; + nvk_image_plane_size_align_B(image, plane, &plane_size_B, &plane_align_B); + const uint64_t mip_tail_offset_B = nil_image_mip_tail_offset_B(&plane->nil); const uint64_t mip_tail_size_B = @@ -353,7 +361,7 @@ push_add_image_plane_mip_tail_bind(struct push_builder *pb, mip_tail_size_B * plane->nil.extent_px.array_len; uint64_t plane_offset_B, mem_offset_B, bind_size_B; - if (!next_opaque_bind_plane(bind, whole_mip_tail_size_B, plane->nil.align_B, + if (!next_opaque_bind_plane(bind, whole_mip_tail_size_B, plane_align_B, &plane_offset_B, &mem_offset_B, &bind_size_B, image_plane_offset_B)) return; @@ -410,15 +418,15 @@ push_add_image_opaque_bind(struct push_builder *pb, uint64_t image_plane_offset_B = 0; for (unsigned plane = 0; plane < image->plane_count; plane++) { if (bind->resourceOffset >= NVK_MIP_TAIL_START_OFFSET) { - push_add_image_plane_mip_tail_bind(pb, &image->planes[plane], + push_add_image_plane_mip_tail_bind(pb, image, &image->planes[plane], bind, &image_plane_offset_B); } else { - push_add_image_plane_opaque_bind(pb, &image->planes[plane], + push_add_image_plane_opaque_bind(pb, image, &image->planes[plane], bind, &image_plane_offset_B); } } if (image->stencil_copy_temp.nil.size_B > 0) { - push_add_image_plane_opaque_bind(pb, &image->stencil_copy_temp, + push_add_image_plane_opaque_bind(pb, image, &image->stencil_copy_temp, bind, &image_plane_offset_B); } }