From dbf0bdd01424ed7f0010e489f227e53d8815045a Mon Sep 17 00:00:00 2001 From: Ryan Pavlik Date: Mon, 25 Mar 2019 10:44:01 -0500 Subject: [PATCH] comp: Factor out some shared functionality. Simplifies error handling as well. --- src/xrt/compositor/common/comp_vk.c | 221 +++++++++++------------ src/xrt/compositor/common/comp_vk.h | 40 ++++ src/xrt/compositor/main/comp_swapchain.c | 90 ++++----- 3 files changed, 176 insertions(+), 175 deletions(-) diff --git a/src/xrt/compositor/common/comp_vk.c b/src/xrt/compositor/common/comp_vk.c index 61e04873d..a9d2c35f0 100644 --- a/src/xrt/compositor/common/comp_vk.c +++ b/src/xrt/compositor/common/comp_vk.c @@ -146,6 +146,65 @@ vk_get_memory_type(struct vk_bundle *vk, return false; } +VkResult +vk_alloc_and_bind_image_memory(struct vk_bundle *vk, + VkImage image, + size_t max_size, + const void *pNext_for_allocate, + VkDeviceMemory *out_mem, + VkDeviceSize *out_size) +{ + VkMemoryRequirements memory_requirements; + vk->vkGetImageMemoryRequirements(vk->device, image, + &memory_requirements); + if (memory_requirements.size > max_size) { + VK_ERROR(vk, + "client_vk_swapchain - Got too little memory " + "%u vs %u\n", + (uint32_t)memory_requirements.size, + (uint32_t)max_size); + return VK_ERROR_OUT_OF_DEVICE_MEMORY; + } + if (out_size != NULL) { + *out_size = memory_requirements.size; + } + + uint32_t memory_type_index = UINT32_MAX; + if (!vk_get_memory_type(vk, memory_requirements.memoryTypeBits, + VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT, + &memory_type_index)) { + VK_ERROR(c, "vk_get_memory_type failed!"); + return VK_ERROR_OUT_OF_DEVICE_MEMORY; + } + + VkMemoryAllocateInfo alloc_info = { + .sType = VK_STRUCTURE_TYPE_MEMORY_ALLOCATE_INFO, + .pNext = pNext_for_allocate, + .allocationSize = memory_requirements.size, + .memoryTypeIndex = memory_type_index, + }; + + VkDeviceMemory device_memory = VK_NULL_HANDLE; + VkResult ret = + vk->vkAllocateMemory(vk->device, &alloc_info, NULL, &device_memory); + if (ret != VK_SUCCESS) { + VK_ERROR(vk, "vkAllocateMemory: %s", vk_result_string(ret)); + return ret; + } + + // Bind the memory to the image. + ret = vk->vkBindImageMemory(vk->device, image, device_memory, 0); + if (ret != VK_SUCCESS) { + // Clean up memory + vk->vkFreeMemory(vk->device, device_memory, NULL); + VK_ERROR(vk, "vkBindImageMemory: %s", vk_result_string(ret)); + return ret; + } + + *out_mem = device_memory; + return ret; +} + VkResult vk_create_image_simple(struct vk_bundle *vk, uint32_t width, @@ -154,12 +213,7 @@ vk_create_image_simple(struct vk_bundle *vk, VkDeviceMemory *out_mem, VkImage *out_image) { - VkImageUsageFlags usage_flags; - VkDeviceMemory memory; - VkImage image; - VkResult ret; - - usage_flags = 0; + VkImageUsageFlags usage_flags = 0; usage_flags |= VK_IMAGE_USAGE_SAMPLED_BIT; usage_flags |= VK_IMAGE_USAGE_TRANSFER_DST_BIT; @@ -186,51 +240,23 @@ vk_create_image_simple(struct vk_bundle *vk, .initialLayout = VK_IMAGE_LAYOUT_UNDEFINED, }; - ret = vk->vkCreateImage(vk->device, &image_info, NULL, &image); + VkImage image; + VkResult ret = vk->vkCreateImage(vk->device, &image_info, NULL, &image); if (ret != VK_SUCCESS) { VK_ERROR(vk, "vkCreateImage: %s", vk_result_string(ret)); - goto err; + // Nothing to cleanup + return ret; } - VkMemoryRequirements memory_requirements; - vk->vkGetImageMemoryRequirements(vk->device, image, - &memory_requirements); - - uint32_t memory_type_index; - vk_get_memory_type(vk, memory_requirements.memoryTypeBits, - VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT, - &memory_type_index); - - VkMemoryAllocateInfo alloc_info = { - .sType = VK_STRUCTURE_TYPE_MEMORY_ALLOCATE_INFO, - .pNext = NULL, - .allocationSize = memory_requirements.size, - .memoryTypeIndex = memory_type_index, - }; - - ret = vk->vkAllocateMemory(vk->device, &alloc_info, NULL, &memory); + ret = vk_alloc_and_bind_image_memory(vk, image, SIZE_MAX, NULL, out_mem, + NULL); if (ret != VK_SUCCESS) { - VK_ERROR(vk, "vkAllocateMemory: %s", vk_result_string(ret)); - goto err_image; - } - - // Bind the memory to the image. - ret = vk->vkBindImageMemory(vk->device, image, memory, 0); - if (ret != VK_SUCCESS) { - VK_ERROR(vk, "vkBindImageMemory: %s", vk_result_string(ret)); - goto err_mem; + // Clean up image + vk->vkDestroyImage(vk->device, image, NULL); + return ret; } *out_image = image; - *out_mem = memory; - - return ret; - -err_mem: - vk->vkFreeMemory(vk->device, memory, NULL); -err_image: - vk->vkDestroyImage(vk->device, image, NULL); -err: return ret; } @@ -246,16 +272,13 @@ vk_create_image_from_fd(struct vk_bundle *vk, VkImage *out_image, VkDeviceMemory *out_mem) { - VkMemoryRequirements memory_requirements; VkImageUsageFlags image_usage = (VkImageUsageFlags)0; - VkDeviceMemory device_memory = NULL; - uint32_t memory_type_index = UINT32_MAX; - VkImage image = NULL; + VkImage image = VK_NULL_HANDLE; VkResult ret = VK_SUCCESS; VkExternalMemoryImageCreateInfoKHR external_memory_image_create_info = { .sType = VK_STRUCTURE_TYPE_EXTERNAL_MEMORY_IMAGE_CREATE_INFO_KHR, - .pNext = 0, + .pNext = NULL, .handleTypes = VK_EXTERNAL_MEMORY_HANDLE_TYPE_OPAQUE_FD_BIT_KHR, }; @@ -299,72 +322,30 @@ vk_create_image_from_fd(struct vk_bundle *vk, ret = vk->vkCreateImage(vk->device, &info, NULL, &image); if (ret != VK_SUCCESS) { VK_ERROR(vk, "vkCreateImage: %s", vk_result_string(ret)); - goto err; + // Nothing to cleanup + return ret; } - vk->vkGetImageMemoryRequirements(vk->device, image, - &memory_requirements); - if (memory_requirements.size > image_fd->size) { - VK_ERROR(vk, - "client_vk_swapchain - Got too little memory " - "%u vs %u\n", - (uint32_t)memory_requirements.size, - (uint32_t)image_fd->size); - } - - if (!vk_get_memory_type(vk, memory_requirements.memoryTypeBits, - VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT, - &memory_type_index)) { - VK_ERROR(c, "vk_get_memory_type failed!"); - ret = VK_ERROR_OUT_OF_DEVICE_MEMORY; - goto err_image; - } - - VkMemoryDedicatedAllocateInfoKHR dedicated_memory_info = { - .sType = VK_STRUCTURE_TYPE_MEMORY_DEDICATED_ALLOCATE_INFO_KHR, - .pNext = NULL, - .image = image, - .buffer = VK_NULL_HANDLE, - }; - VkImportMemoryFdInfoKHR import_memory_info = { .sType = VK_STRUCTURE_TYPE_IMPORT_MEMORY_FD_INFO_KHR, - .pNext = &dedicated_memory_info, + .pNext = NULL, .handleType = VK_EXTERNAL_MEMORY_HANDLE_TYPE_OPAQUE_FD_BIT_KHR, .fd = image_fd->fd, }; - - VkMemoryAllocateInfo alloc_info = { - .sType = VK_STRUCTURE_TYPE_MEMORY_ALLOCATE_INFO, + VkMemoryDedicatedAllocateInfoKHR dedicated_memory_info = { + .sType = VK_STRUCTURE_TYPE_MEMORY_DEDICATED_ALLOCATE_INFO_KHR, .pNext = &import_memory_info, - .allocationSize = memory_requirements.size, - .memoryTypeIndex = memory_type_index, + .image = image, + .buffer = VK_NULL_HANDLE, }; - - ret = - vk->vkAllocateMemory(vk->device, &alloc_info, NULL, &device_memory); + ret = vk_alloc_and_bind_image_memory( + vk, image, image_fd->size, &dedicated_memory_info, out_mem, NULL); if (ret != VK_SUCCESS) { - VK_ERROR(vk, "vkAllocateMemory: %s", vk_result_string(ret)); - goto err_image; - } - - // Bind the memory to the image. - ret = vk->vkBindImageMemory(vk->device, image, device_memory, 0); - if (ret != VK_SUCCESS) { - VK_ERROR(vk, "vkBindImageMemory: %s", vk_result_string(ret)); - goto err_mem; + vk->vkDestroyImage(vk->device, image, NULL); + return ret; } *out_image = image; - *out_mem = device_memory; - - return ret; - -err_mem: - vk->vkFreeMemory(vk->device, device_memory, NULL); -err_image: - vk->vkDestroyImage(vk->device, image, NULL); -err: return ret; } @@ -471,7 +452,8 @@ vk_init_cmd_buffer(struct vk_bundle *vk, VkCommandBuffer *out_cmd_buffer) if (ret != VK_SUCCESS) { VK_ERROR(vk, "vkAllocateCommandBuffers: %s", vk_result_string(ret)); - goto err; + // Nothing to cleanup + return ret; } // Start the command buffer as well. @@ -495,7 +477,6 @@ vk_init_cmd_buffer(struct vk_bundle *vk, VkCommandBuffer *out_cmd_buffer) err_buffer: vk->vkFreeCommandBuffers(vk->device, vk->cmd_pool, 1, &cmd_buffer); -err: return ret; } @@ -535,6 +516,22 @@ vk_submit_cmd_buffer(struct vk_bundle *vk, VkCommandBuffer cmd_buffer) VkResult ret = VK_SUCCESS; VkQueue queue; VkFence fence; + VkFenceCreateInfo fence_info = { + .sType = VK_STRUCTURE_TYPE_FENCE_CREATE_INFO, + .pNext = NULL, + .flags = 0, + }; + VkSubmitInfo submitInfo = { + .sType = VK_STRUCTURE_TYPE_SUBMIT_INFO, + .pNext = NULL, + .waitSemaphoreCount = 0, + .pWaitSemaphores = NULL, + .pWaitDstStageMask = NULL, + .commandBufferCount = 1, + .pCommandBuffers = &cmd_buffer, + .signalSemaphoreCount = 0, + .pSignalSemaphores = NULL, + }; // Finish the command buffer first. ret = vk->vkEndCommandBuffer(cmd_buffer); @@ -547,11 +544,6 @@ vk_submit_cmd_buffer(struct vk_bundle *vk, VkCommandBuffer cmd_buffer) vk->vkGetDeviceQueue(vk->device, vk->queue_family_index, 0, &queue); // Create the fence. - VkFenceCreateInfo fence_info = { - .sType = VK_STRUCTURE_TYPE_FENCE_CREATE_INFO, - .pNext = NULL, - .flags = 0, - }; ret = vk->vkCreateFence(vk->device, &fence_info, NULL, &fence); if (ret != VK_SUCCESS) { VK_ERROR(vk, "vkCreateFence: %s", vk_result_string(ret)); @@ -559,17 +551,6 @@ vk_submit_cmd_buffer(struct vk_bundle *vk, VkCommandBuffer cmd_buffer) } // Do the actual submitting. - VkSubmitInfo submitInfo = { - .sType = VK_STRUCTURE_TYPE_SUBMIT_INFO, - .pNext = NULL, - .waitSemaphoreCount = 0, - .pWaitSemaphores = NULL, - .pWaitDstStageMask = NULL, - .commandBufferCount = 1, - .pCommandBuffers = &cmd_buffer, - .signalSemaphoreCount = 0, - .pSignalSemaphores = NULL, - }; ret = vk->vkQueueSubmit(queue, 1, &submitInfo, fence); if (ret != VK_SUCCESS) { @@ -907,6 +888,7 @@ vk_find_graphics_queue(struct vk_bundle *vk, uint32_t *out_graphics_queue) { /* Find the first graphics queue */ uint32_t num_queues = 0; + uint32_t i = 0; vk->vkGetPhysicalDeviceQueueFamilyProperties(vk->physical_device, &num_queues, NULL); @@ -921,7 +903,6 @@ vk_find_graphics_queue(struct vk_bundle *vk, uint32_t *out_graphics_queue) goto err_free; } - uint32_t i = 0; for (i = 0; i < num_queues; i++) { if (queue_family_props[i].queueFlags & VK_QUEUE_GRAPHICS_BIT) { break; diff --git a/src/xrt/compositor/common/comp_vk.h b/src/xrt/compositor/common/comp_vk.h index 1c948246a..2b462d29c 100644 --- a/src/xrt/compositor/common/comp_vk.h +++ b/src/xrt/compositor/common/comp_vk.h @@ -276,6 +276,46 @@ vk_get_memory_type(struct vk_bundle *vk, VkMemoryPropertyFlags memory_props, uint32_t *out_type_id); +/*! + * Allocate memory for an image and bind it to that image. + * + * Handles the following steps: + * + * - calling vkGetImageMemoryRequirements + * - comparing against the max_size + * - getting the memory type (as dictated by the VkMemoryRequirements and + * VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT) + * - calling vkAllocateMemory + * - calling vkBindImageMemory + * - calling vkDestroyMemory in case of an error. + * + * If this fails, it cleans up the VkDeviceMemory. + * + * @param vk Vulkan bundle + * @param image The VkImage to allocate for and bind. + * @param max_size The maximum value you'll allow for + * VkMemoryRequirements::size. Pass SIZE_MAX if you will accept any size + * that works. + * @param pNext_for_allocate (Optional) a pointer to use in the pNext chain of + * VkMemoryAllocateInfo. + * @param out_mem Output parameter: will be set to the allocated memory if + * everything succeeds. Not modified if there is an error. + * @param out_size (Optional) pointer to receive the value of + * VkMemoryRequirements::size. + * + * If this fails, you may want to destroy your VkImage as well, since this + * routine is usually used in combination with vkCreateImage. + * + * @ingroup comp_common + */ +VkResult +vk_alloc_and_bind_image_memory(struct vk_bundle *vk, + VkImage image, + size_t max_size, + const void *pNext_for_allocate, + VkDeviceMemory *out_mem, + VkDeviceSize *out_size); + /*! * @ingroup comp_common */ diff --git a/src/xrt/compositor/main/comp_swapchain.c b/src/xrt/compositor/main/comp_swapchain.c index 2ccbbb600..5b5368e39 100644 --- a/src/xrt/compositor/main/comp_swapchain.c +++ b/src/xrt/compositor/main/comp_swapchain.c @@ -61,6 +61,30 @@ swapchain_release_image(struct xrt_swapchain *xsc, uint32_t index) return true; } +static VkResult +get_device_memory_fd(struct comp_compositor *c, + VkDeviceMemory device_memory, + int *out_fd) +{ + + // vkGetMemoryFdKHR parameter + VkMemoryGetFdInfoKHR fd_info = { + .sType = VK_STRUCTURE_TYPE_MEMORY_GET_FD_INFO_KHR, + .pNext = NULL, + .memory = device_memory, + .handleType = VK_EXTERNAL_MEMORY_HANDLE_TYPE_OPAQUE_FD_BIT_KHR, + }; + int fd; + VkResult ret = c->vk.vkGetMemoryFdKHR(c->vk.device, &fd_info, &fd); + if (ret != VK_SUCCESS) { + COMP_ERROR(c, "->image - vkGetMemoryFdKHR: %s", + vk_result_string(ret)); + return VK_ERROR_FEATURE_NOT_PRESENT; + } + *out_fd = fd; + return ret; +} + static VkResult create_image_fd(struct comp_compositor *c, enum xrt_swapchain_usage_bits swapchain_usage, @@ -73,12 +97,10 @@ create_image_fd(struct comp_compositor *c, VkDeviceMemory *out_mem, struct xrt_image_fd *out_image_fd) { - VkMemoryRequirements memory_requirements; VkImageUsageFlags image_usage = (VkImageUsageFlags)0; - VkDeviceMemory device_memory = NULL; - uint32_t memory_type_index = UINT32_MAX; - VkImage image = NULL; - VkResult ret; + VkDeviceMemory device_memory = VK_NULL_HANDLE; + VkImage image = VK_NULL_HANDLE; + VkResult ret = VK_SUCCESS; size_t size; int fd; @@ -136,30 +158,14 @@ create_image_fd(struct comp_compositor *c, if (ret != VK_SUCCESS) { COMP_ERROR(c, "->image - vkCreateImage: %s", vk_result_string(ret)); - goto err; - } - - - /* - * Get the size of the buffer. - */ - - c->vk.vkGetImageMemoryRequirements(c->vk.device, image, - &memory_requirements); - size = memory_requirements.size; - - if (!vk_get_memory_type(&c->vk, memory_requirements.memoryTypeBits, - VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT, - &memory_type_index)) { - COMP_ERROR(c, "->image - _get_memory_type!"); - ret = VK_ERROR_OUT_OF_DEVICE_MEMORY; - goto err_image; + // Nothing to cleanup + return ret; } /* - * Create the memory. + * Create and bind the memory. */ - + // vkAllocateMemory parameters VkMemoryDedicatedAllocateInfoKHR dedicated_memory_info = { .sType = VK_STRUCTURE_TYPE_MEMORY_DEDICATED_ALLOCATE_INFO_KHR, .pNext = NULL, @@ -173,48 +179,23 @@ create_image_fd(struct comp_compositor *c, .handleTypes = VK_EXTERNAL_MEMORY_HANDLE_TYPE_OPAQUE_FD_BIT_KHR, }; - VkMemoryAllocateInfo alloc_info = { - .sType = VK_STRUCTURE_TYPE_MEMORY_ALLOCATE_INFO, - .pNext = &export_alloc_info, - .allocationSize = size, - .memoryTypeIndex = memory_type_index, - }; - - ret = c->vk.vkAllocateMemory(c->vk.device, &alloc_info, NULL, - &device_memory); + ret = vk_alloc_and_bind_image_memory( + &c->vk, image, SIZE_MAX, &export_alloc_info, &device_memory, &size); if (ret != VK_SUCCESS) { COMP_ERROR(c, "->image - vkAllocateMemory: %s", vk_result_string(ret)); goto err_image; } - ret = c->vk.vkBindImageMemory(c->vk.device, image, device_memory, 0); - if (ret != VK_SUCCESS) { - COMP_ERROR(c, "->image - vkBindImageMemory: %s", - vk_result_string(ret)); - goto err_mem; - } - - /* * Get the fd. */ - - VkMemoryGetFdInfoKHR fd_info = { - .sType = VK_STRUCTURE_TYPE_MEMORY_GET_FD_INFO_KHR, - .pNext = NULL, - .memory = device_memory, - .handleType = VK_EXTERNAL_MEMORY_HANDLE_TYPE_OPAQUE_FD_BIT_KHR, - }; - - ret = c->vk.vkGetMemoryFdKHR(c->vk.device, &fd_info, &fd); + ret = get_device_memory_fd(c, device_memory, &fd); if (ret != VK_SUCCESS) { - COMP_ERROR(c, "->image - vkGetMemoryFdKHR: %s", - vk_result_string(ret)); - ret = VK_ERROR_FEATURE_NOT_PRESENT; goto err_mem; } + *out_image = image; *out_mem = device_memory; out_image_fd->fd = fd; @@ -226,7 +207,6 @@ err_mem: c->vk.vkFreeMemory(c->vk.device, device_memory, NULL); err_image: c->vk.vkDestroyImage(c->vk.device, image, NULL); -err: return ret; }