comp: Factor out some shared functionality.

Simplifies error handling as well.

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;
+	VkImageUsageFlags usage_flags = 0;
-	VkDeviceMemory memory;
-	VkImage image;
-	VkResult ret;
-
-	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;
+	ret = vk_alloc_and_bind_image_memory(vk, image, SIZE_MAX, NULL, out_mem,
-	vk->vkGetImageMemoryRequirements(vk->device, image,
+	                                     NULL);
-	                                 &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);
 	if (ret != VK_SUCCESS) {
-		VK_ERROR(vk, "vkAllocateMemory: %s", vk_result_string(ret));
+		// Clean up image
-		goto err_image;
+		vk->vkDestroyImage(vk->device, image, NULL);
-	}
+		return ret;
-
-	// 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;
 	}
 
 	*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;
+	VkImage image = VK_NULL_HANDLE;
-	uint32_t memory_type_index = UINT32_MAX;
-	VkImage image = NULL;
 	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,
 	};
-
+	VkMemoryDedicatedAllocateInfoKHR dedicated_memory_info = {
-	VkMemoryAllocateInfo alloc_info = {
+	    .sType = VK_STRUCTURE_TYPE_MEMORY_DEDICATED_ALLOCATE_INFO_KHR,
-	    .sType = VK_STRUCTURE_TYPE_MEMORY_ALLOCATE_INFO,
 	    .pNext = &import_memory_info,
-	    .allocationSize = memory_requirements.size,
+	    .image = image,
-	    .memoryTypeIndex = memory_type_index,
+	    .buffer = VK_NULL_HANDLE,
 	};
-
+	ret = vk_alloc_and_bind_image_memory(
-	ret =
+	    vk, image, image_fd->size, &dedicated_memory_info, out_mem, NULL);
-	    vk->vkAllocateMemory(vk->device, &alloc_info, NULL, &device_memory);
 	if (ret != VK_SUCCESS) {
-		VK_ERROR(vk, "vkAllocateMemory: %s", vk_result_string(ret));
+		vk->vkDestroyImage(vk->device, image, NULL);
-		goto err_image;
+		return ret;
-	}
-
-	// 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;
 	}
 
 	*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;

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
  */

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;
+	VkDeviceMemory device_memory = VK_NULL_HANDLE;
-	uint32_t memory_type_index = UINT32_MAX;
+	VkImage image = VK_NULL_HANDLE;
-	VkImage image = NULL;
+	VkResult ret = VK_SUCCESS;
-	VkResult ret;
 	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;
+		// Nothing to cleanup
-	}
+		return ret;
-
-
-	/*
-	 * 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;
 	}
 
 	/*
-	 * 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 = {
+	ret = vk_alloc_and_bind_image_memory(
-	    .sType = VK_STRUCTURE_TYPE_MEMORY_ALLOCATE_INFO,
+	    &c->vk, image, SIZE_MAX, &export_alloc_info, &device_memory, &size);
-	    .pNext = &export_alloc_info,
-	    .allocationSize = size,
-	    .memoryTypeIndex = memory_type_index,
-	};
-
-	ret = c->vk.vkAllocateMemory(c->vk.device, &alloc_info, NULL,
-	                             &device_memory);
 	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.
 	 */
-
+	ret = get_device_memory_fd(c, device_memory, &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);
 	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;
 }