Re: [Mesa-dev] [PATCH v2 14/32] vulkan/wsi: Do image creation in common code

[Chad Versace]

On Tue 28 Nov 2017, Jason Ekstrand wrote:
> This uses the mock extension created in a previous commit to tell the
> driver that the image it's just been asked to create is, in fact, a
> window system image with whatever assumptions that implies.  There was a
> lot of redundant code between the two drivers to do basically exactly
> the same thing.
> ---
>  src/amd/vulkan/radv_wsi.c   | 124 
> +---
>  src/intel/vulkan/anv_wsi.c  | 122 +--
>  src/vulkan/wsi/wsi_common.c | 117 +-
>  src/vulkan/wsi/wsi_common.h |  28 +---
>  src/vulkan/wsi/wsi_common_private.h |  25 +++-
>  src/vulkan/wsi/wsi_common_wayland.c |  13 +---
>  src/vulkan/wsi/wsi_common_x11.c |  20 +-
>  7 files changed, 146 insertions(+), 303 deletions(-)

This patch is
Reviewed-by: Chad Versace 

This series is a good improvement over the old code :)

But I found some nits:

- The Vulkan spec, iirc, requires that vkCreateFoo not modify its
  output parameter on failure. The memset in this patch breaks that
  requirement. I'm sure we're breaking that elsewhere too. We should
  probably do a cleanup one day to fix that.

- wsi_destroy_image neither assets that image->fd == -1 nor closes
  it. I believe there is no fd leakage because anv_wsi* does the
  right thing. But that's a small cleanup we could do later.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH v2 14/32] vulkan/wsi: Do image creation in common code

[Jason Ekstrand]

This uses the mock extension created in a previous commit to tell the
driver that the image it's just been asked to create is, in fact, a
window system image with whatever assumptions that implies.  There was a
lot of redundant code between the two drivers to do basically exactly
the same thing.
---
 src/amd/vulkan/radv_wsi.c   | 124 +---
 src/intel/vulkan/anv_wsi.c  | 122 +--
 src/vulkan/wsi/wsi_common.c | 117 +-
 src/vulkan/wsi/wsi_common.h |  28 +---
 src/vulkan/wsi/wsi_common_private.h |  25 +++-
 src/vulkan/wsi/wsi_common_wayland.c |  13 +---
 src/vulkan/wsi/wsi_common_x11.c |  20 +-
 7 files changed, 146 insertions(+), 303 deletions(-)

diff --git a/src/amd/vulkan/radv_wsi.c b/src/amd/vulkan/radv_wsi.c
index 589eb5c..b576514 100644
--- a/src/amd/vulkan/radv_wsi.c
+++ b/src/amd/vulkan/radv_wsi.c
@@ -157,128 +157,6 @@ VkResult radv_GetPhysicalDeviceSurfacePresentModesKHR(
pPresentModes);
 }
 
-static VkResult
-radv_wsi_image_create(VkDevice device_h,
- const VkSwapchainCreateInfoKHR *pCreateInfo,
- const VkAllocationCallbacks* pAllocator,
- struct wsi_image *wsi_image)
-{
-   VkResult result = VK_SUCCESS;
-   struct radeon_surf *surface;
-   VkImage image_h;
-   struct radv_image *image;
-   int fd;
-   RADV_FROM_HANDLE(radv_device, device, device_h);
-
-   result = radv_image_create(device_h,
-  &(struct radv_image_create_info) {
-  .vk_info =
-  &(VkImageCreateInfo) {
-  .sType = 
VK_STRUCTURE_TYPE_IMAGE_CREATE_INFO,
-  .imageType = 
VK_IMAGE_TYPE_2D,
-  .format = 
pCreateInfo->imageFormat,
-  .extent = {
-  .width = 
pCreateInfo->imageExtent.width,
-  .height = 
pCreateInfo->imageExtent.height,
-  .depth = 1
-  },
-  .mipLevels = 1,
-  .arrayLayers = 1,
-  .samples = 1,
-  /* FIXME: Need a way to use 
X tiling to allow scanout */
-  .tiling = 
VK_IMAGE_TILING_OPTIMAL,
-  .usage = 
VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT,
-  .flags = 0,
-  },
-  .scanout = true},
-  NULL,
-  _h);
-   if (result != VK_SUCCESS)
-   return result;
-
-   image = radv_image_from_handle(image_h);
-
-   VkDeviceMemory memory_h;
-
-   const VkMemoryDedicatedAllocateInfoKHR ded_alloc = {
-   .sType = VK_STRUCTURE_TYPE_MEMORY_DEDICATED_ALLOCATE_INFO_KHR,
-   .pNext = NULL,
-   .buffer = VK_NULL_HANDLE,
-   .image = image_h
-   };
-
-   /* Find the first VRAM memory type, or GART for PRIME images. */
-   int memory_type_index = -1;
-   for (int i = 0; i < 
device->physical_device->memory_properties.memoryTypeCount; ++i) {
-   bool is_local = 
!!(device->physical_device->memory_properties.memoryTypes[i].propertyFlags & 
VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT);
-   if (is_local) {
-   memory_type_index = i;
-   break;
-   }
-   }
-
-   /* fallback */
-   if (memory_type_index == -1)
-   memory_type_index = 0;
-
-   result = radv_alloc_memory(device_h,
-&(VkMemoryAllocateInfo) {
-.sType = 
VK_STRUCTURE_TYPE_MEMORY_ALLOCATE_INFO,
-.pNext = _alloc,
-.allocationSize = image->size,
-.memoryTypeIndex = 
memory_type_index,
-},
-NULL /* XXX: pAllocator */,
-RADV_MEM_IMPLICIT_SYNC,
-_h);
-   if (result != VK_SUCCESS)
-   goto fail_create_image;
-
-   radv_BindImageMemory(device_h, image_h, memory_h, 0);
-
-