Re: [Mesa-dev] [PATCH v2 31/32] vulkan/wsi: Initialize individual WSI interfaces in wsi_device_init

2017-12-02 Thread Chad Versace 
On Tue 28 Nov 2017, Jason Ekstrand wrote:
> Now that we have anv_device_init/finish functions, there's no reason to
> have the individual driver do any more work than that.

You mean "wsi_device_init/finish" functions.

> ---
>  src/amd/vulkan/radv_wsi.c   | 36 ++--
>  src/intel/vulkan/anv_wsi.c  | 36 ++--
>  src/vulkan/wsi/wsi_common.c | 37 
> +++--
>  src/vulkan/wsi/wsi_common.h | 19 +++
>  src/vulkan/wsi/wsi_common_private.h | 10 ++
>  5 files changed, 64 insertions(+), 74 deletions(-)


> -void
> +VkResult
>  wsi_device_init(struct wsi_device *wsi,
>  VkPhysicalDevice pdevice,
> -WSI_FN_GetPhysicalDeviceProcAddr proc_addr)
> +WSI_FN_GetPhysicalDeviceProcAddr proc_addr,
> +const VkAllocationCallbacks *alloc)
>  {
> +   VkResult result;
> +
> memset(wsi, 0, sizeof(*wsi));
>  
>  #define WSI_GET_CB(func) \
> @@ -69,6 +72,36 @@ wsi_device_init(struct wsi_device *wsi,
> WSI_GET_CB(QueueSubmit);
> WSI_GET_CB(WaitForFences);
>  #undef WSI_GET_CB
> +
> +#ifdef VK_USE_PLATFORM_XCB_KHR
> +   result = wsi_x11_init_wsi(wsi, alloc);
> +   if (result != VK_SUCCESS)
> +  return result;
> +#endif
> +
> +#ifdef VK_USE_PLATFORM_WAYLAND_KHR
> +   result = wsi_wl_init_wsi(wsi, alloc, pdevice);
> +   if (result != VK_SUCCESS) {
> +#ifdef VK_USE_PLATFORM_XCB_KHR
> +  wsi_x11_finish_wsi(wsi, alloc);
> +#endif
> +  return result;
> +   }
> +#endif

This looks like a bug. wsi_device_init() fails if either of x11 or
wayland initialization fails, even if the user requested only one of
VK_KHR_{xcb,xlib,wayland}_surface.

I filed https://bugs.freedesktop.org/show_bug.cgi?id=104039 .

But this patch is just a refactor patch, and it preserves the existing
(and possibly buggy) behavior. With the commit message fixed,
Reviewed-by: Chad Versace 


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH v2 31/32] vulkan/wsi: Initialize individual WSI interfaces in wsi_device_init

2017-11-28 Thread Jason Ekstrand 
Now that we have anv_device_init/finish functions, there's no reason to
have the individual driver do any more work than that.
---
 src/amd/vulkan/radv_wsi.c   | 36 ++--
 src/intel/vulkan/anv_wsi.c  | 36 ++--
 src/vulkan/wsi/wsi_common.c | 37 +++--
 src/vulkan/wsi/wsi_common.h | 19 +++
 src/vulkan/wsi/wsi_common_private.h | 10 ++
 5 files changed, 64 insertions(+), 74 deletions(-)

diff --git a/src/amd/vulkan/radv_wsi.c b/src/amd/vulkan/radv_wsi.c
index aa06944..cb61eb6 100644
--- a/src/amd/vulkan/radv_wsi.c
+++ b/src/amd/vulkan/radv_wsi.c
@@ -38,41 +38,17 @@ radv_wsi_proc_addr(VkPhysicalDevice physicalDevice, const 
char *pName)
 VkResult
 radv_init_wsi(struct radv_physical_device *physical_device)
 {
-   VkResult result;
-
-   wsi_device_init(_device->wsi_device,
-   radv_physical_device_to_handle(physical_device),
-   radv_wsi_proc_addr);
-
-#ifdef VK_USE_PLATFORM_XCB_KHR
-   result = wsi_x11_init_wsi(_device->wsi_device, 
_device->instance->alloc);
-   if (result != VK_SUCCESS)
-   return result;
-#endif
-
-#ifdef VK_USE_PLATFORM_WAYLAND_KHR
-   result = wsi_wl_init_wsi(_device->wsi_device, 
_device->instance->alloc,
-
radv_physical_device_to_handle(physical_device));
-   if (result != VK_SUCCESS) {
-#ifdef VK_USE_PLATFORM_XCB_KHR
-   wsi_x11_finish_wsi(_device->wsi_device, 
_device->instance->alloc);
-#endif
-   return result;
-   }
-#endif
-
-   return VK_SUCCESS;
+   return wsi_device_init(_device->wsi_device,
+  radv_physical_device_to_handle(physical_device),
+  radv_wsi_proc_addr,
+  _device->instance->alloc);
 }
 
 void
 radv_finish_wsi(struct radv_physical_device *physical_device)
 {
-#ifdef VK_USE_PLATFORM_WAYLAND_KHR
-   wsi_wl_finish_wsi(_device->wsi_device, 
_device->instance->alloc);
-#endif
-#ifdef VK_USE_PLATFORM_XCB_KHR
-   wsi_x11_finish_wsi(_device->wsi_device, 
_device->instance->alloc);
-#endif
+   wsi_device_finish(_device->wsi_device,
+ _device->instance->alloc);
 }
 
 void radv_DestroySurfaceKHR(
diff --git a/src/intel/vulkan/anv_wsi.c b/src/intel/vulkan/anv_wsi.c
index add983f..6082c3d 100644
--- a/src/intel/vulkan/anv_wsi.c
+++ b/src/intel/vulkan/anv_wsi.c
@@ -36,41 +36,17 @@ anv_wsi_proc_addr(VkPhysicalDevice physicalDevice, const 
char *pName)
 VkResult
 anv_init_wsi(struct anv_physical_device *physical_device)
 {
-   VkResult result;
-
-   wsi_device_init(_device->wsi_device,
-   anv_physical_device_to_handle(physical_device),
-   anv_wsi_proc_addr);
-
-#ifdef VK_USE_PLATFORM_XCB_KHR
-   result = wsi_x11_init_wsi(_device->wsi_device, 
_device->instance->alloc);
-   if (result != VK_SUCCESS)
-  return result;
-#endif
-
-#ifdef VK_USE_PLATFORM_WAYLAND_KHR
-   result = wsi_wl_init_wsi(_device->wsi_device, 
_device->instance->alloc,
-anv_physical_device_to_handle(physical_device));
-   if (result != VK_SUCCESS) {
-#ifdef VK_USE_PLATFORM_XCB_KHR
-  wsi_x11_finish_wsi(_device->wsi_device, 
_device->instance->alloc);
-#endif
-  return result;
-   }
-#endif
-
-   return VK_SUCCESS;
+   return wsi_device_init(_device->wsi_device,
+  anv_physical_device_to_handle(physical_device),
+  anv_wsi_proc_addr,
+  _device->instance->alloc);
 }
 
 void
 anv_finish_wsi(struct anv_physical_device *physical_device)
 {
-#ifdef VK_USE_PLATFORM_WAYLAND_KHR
-   wsi_wl_finish_wsi(_device->wsi_device, 
_device->instance->alloc);
-#endif
-#ifdef VK_USE_PLATFORM_XCB_KHR
-   wsi_x11_finish_wsi(_device->wsi_device, 
_device->instance->alloc);
-#endif
+   wsi_device_finish(_device->wsi_device,
+ _device->instance->alloc);
 }
 
 void anv_DestroySurfaceKHR(
diff --git a/src/vulkan/wsi/wsi_common.c b/src/vulkan/wsi/wsi_common.c
index 9a5c783..2de5f15 100644
--- a/src/vulkan/wsi/wsi_common.c
+++ b/src/vulkan/wsi/wsi_common.c
@@ -25,11 +25,14 @@
 #include "util/macros.h"
 #include "vk_util.h"
 
-void
+VkResult
 wsi_device_init(struct wsi_device *wsi,
 VkPhysicalDevice pdevice,
-WSI_FN_GetPhysicalDeviceProcAddr proc_addr)
+WSI_FN_GetPhysicalDeviceProcAddr proc_addr,
+const VkAllocationCallbacks *alloc)
 {
+   VkResult result;
+
memset(wsi, 0, sizeof(*wsi));
 
 #define WSI_GET_CB(func) \
@@ -69,6 +72,36 @@ wsi_device_init(struct wsi_device *wsi,
WSI_GET_CB(QueueSubmit);
WSI_GET_CB(WaitForFences);
 #undef WSI_GET_CB
+
+#ifdef VK_USE_PLATFORM_XCB_KHR
+   result = wsi_x11_init_wsi(wsi, alloc);
+   if (result != VK_SUCCESS)
+  return result;
+#endif
+
+#ifdef