Re: [Mesa-dev] [PATCH 2/4] vulkan/wsi/wayland: Stop caching Wayland displays

[Jason Ekstrand]

On Wed, Sep 27, 2017 at 1:52 AM, Daniel Stone  wrote:

> Hi,
>
> On 26 September 2017 at 23:55, Jason Ekstrand 
> wrote:
> > @@ -833,24 +816,19 @@ wsi_wl_surface_create_swapchain(VkIcdSurfaceBase
> *icd_surface,
> > chain->vk_format = pCreateInfo->imageFormat;
> > chain->drm_format = wl_drm_format_for_vk_format(chain->vk_format,
> alpha);
> >
> > -   chain->display = wsi_wl_get_display(wsi_device, surface->display);
> > +   chain->display = wsi_wl_display_create(wsi, surface->display, false);
>
> Please leave this still getting the formats. In the modifier-support
> patches, we need the display to have stored the set of acceptable
> per-format modifiers, so we'd need to flip the false to true here.
>

Sure, I can do that.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/4] vulkan/wsi/wayland: Stop caching Wayland displays

[Daniel Stone]

Hi,

On 26 September 2017 at 23:55, Jason Ekstrand  wrote:
> @@ -833,24 +816,19 @@ wsi_wl_surface_create_swapchain(VkIcdSurfaceBase 
> *icd_surface,
> chain->vk_format = pCreateInfo->imageFormat;
> chain->drm_format = wl_drm_format_for_vk_format(chain->vk_format, alpha);
>
> -   chain->display = wsi_wl_get_display(wsi_device, surface->display);
> +   chain->display = wsi_wl_display_create(wsi, surface->display, false);

Please leave this still getting the formats. In the modifier-support
patches, we need the display to have stored the set of acceptable
per-format modifiers, so we'd need to flip the false to true here.

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

[Mesa-dev] [PATCH 2/4] vulkan/wsi/wayland: Stop caching Wayland displays

[Jason Ekstrand]

We originally implemented caching to avoid unneeded round-trips to the
compositor when querying surface capabilities etc. to set up the
swapchain.  Unfortunately, this doesn't work if vkDestroyInstance is
called after the Wayland connection has been dropped.  In this case, we
end up trying to clean up already destroyed wl_proxy objects which leads
to crashes.  In particular most of dEQP-VK.wsi.wayland is crashing
thanks to this problem.

This commit gets rid of the cache and simply embeds the wsi_wl_display
struct in the swapchain.  While we're at it, we can get rid of the
wl_event_queue that we were storing in the swapchain because we can just
use the one in the embedded wsi_wl_display.

Bugzilla: https://bugs.freedesktop.org/102578
Cc: mesa-sta...@lists.freedesktop.org
---
 src/vulkan/wsi/wsi_common_wayland.c | 164 
 1 file changed, 56 insertions(+), 108 deletions(-)

diff --git a/src/vulkan/wsi/wsi_common_wayland.c 
b/src/vulkan/wsi/wsi_common_wayland.c
index 62fc97c..c5a72d6 100644
--- a/src/vulkan/wsi/wsi_common_wayland.c
+++ b/src/vulkan/wsi/wsi_common_wayland.c
@@ -66,10 +66,6 @@ struct wsi_wayland {
const VkAllocationCallbacks *alloc;
VkPhysicalDevice physical_device;
 
-   pthread_mutex_t  mutex;
-   /* Hash table of wl_display -> wsi_wl_display mappings */
-   struct hash_table *  displays;
-
const struct wsi_callbacks *cbs;
 };
 
@@ -148,6 +144,8 @@ static void
 drm_handle_format(void *data, struct wl_drm *drm, uint32_t wl_format)
 {
struct wsi_wl_display *display = data;
+   if (display->formats.element_size == 0)
+  return;
 
switch (wl_format) {
 #if 0
@@ -263,15 +261,18 @@ wsi_wl_display_finish(struct wsi_wl_display *display)
 static int
 wsi_wl_display_init(struct wsi_wayland *wsi_wl,
 struct wsi_wl_display *display,
-struct wl_display *wl_display)
+struct wl_display *wl_display,
+bool get_format_list)
 {
memset(display, 0, sizeof(*display));
 
display->wsi_wl = wsi_wl;
display->wl_display = wl_display;
 
-   if (!u_vector_init(>formats, sizeof(VkFormat), 8))
-  goto fail;
+   if (get_format_list) {
+  if (!u_vector_init(>formats, sizeof(VkFormat), 8))
+ goto fail;
+   }
 
display->queue = wl_display_create_queue(wl_display);
if (!display->queue)
@@ -319,7 +320,8 @@ fail:
 }
 
 static struct wsi_wl_display *
-wsi_wl_display_create(struct wsi_wayland *wsi, struct wl_display *wl_display)
+wsi_wl_display_create(struct wsi_wayland *wsi, struct wl_display *wl_display,
+  bool get_format_list)
 {
struct wsi_wl_display *display =
   vk_alloc(wsi->alloc, sizeof(*display), 8,
@@ -327,7 +329,7 @@ wsi_wl_display_create(struct wsi_wayland *wsi, struct 
wl_display *wl_display)
if (!display)
   return NULL;
 
-   if (wsi_wl_display_init(wsi, display, wl_display)) {
+   if (wsi_wl_display_init(wsi, display, wl_display, get_format_list)) {
   vk_free(wsi->alloc, display);
   return NULL;
}
@@ -336,54 +338,25 @@ wsi_wl_display_create(struct wsi_wayland *wsi, struct 
wl_display *wl_display)
 }
 
 static void
-wsi_wl_display_destroy(struct wsi_wayland *wsi, struct wsi_wl_display *display)
+wsi_wl_display_destroy(struct wsi_wl_display *display)
 {
+   struct wsi_wayland *wsi = display->wsi_wl;
wsi_wl_display_finish(display);
vk_free(wsi->alloc, display);
 }
 
-static struct wsi_wl_display *
-wsi_wl_get_display(struct wsi_device *wsi_device,
-   struct wl_display *wl_display)
+VkBool32
+wsi_wl_get_presentation_support(struct wsi_device *wsi_device,
+   struct wl_display *wl_display)
 {
struct wsi_wayland *wsi =
   (struct wsi_wayland *)wsi_device->wsi[VK_ICD_WSI_PLATFORM_WAYLAND];
 
-   pthread_mutex_lock(>mutex);
-
-   struct hash_entry *entry = _mesa_hash_table_search(wsi->displays,
-  wl_display);
-   if (!entry) {
-  /* We're about to make a bunch of blocking calls.  Let's drop the
-   * mutex for now so we don't block up too badly.
-   */
-  pthread_mutex_unlock(>mutex);
-
-  struct wsi_wl_display *display = wsi_wl_display_create(wsi, wl_display);
-  if (!display)
- return NULL;
-
-  pthread_mutex_lock(>mutex);
-
-  entry = _mesa_hash_table_search(wsi->displays, wl_display);
-  if (entry) {
- /* Oops, someone raced us to it */
- wsi_wl_display_destroy(wsi, display);
-  } else {
- entry = _mesa_hash_table_insert(wsi->displays, wl_display, display);
-  }
-   }
-
-   pthread_mutex_unlock(>mutex);
-
-   return entry->data;
-}
+   struct wsi_wl_display display;
+   int ret = wsi_wl_display_init(wsi, , wl_display, false);
+   wsi_wl_display_finish();
 
-VkBool32
-wsi_wl_get_presentation_support(struct wsi_device *wsi_device,
-