[Mesa-dev] [PATCH 1/2] vulkan/wsi: Store the instance allocator in wsi_device

Jason Ekstrand jason at jlekstrand.net
Thu Oct 18 13:02:42 UTC 2018


On October 18, 2018 05:40:14 Eric Engestrom <eric.engestrom at intel.com> wrote:

> On Wednesday, 2018-10-17 15:14:03 -0500, Jason Ekstrand wrote:
>> We already have wsi_device and we know the instance allocator at
>> wsi_device_init time so there's no need to pass it into the physical
>> device queries.  This also fixes a memory allocation domain bug that can
>> occur if CreateSwapchain gets called prior to any queries (not likely)
>> in which case the cached connection gets allocated off the device
>> instead of the instance.
>> ---
>> src/amd/vulkan/radv_wsi.c           |  1 -
>> src/amd/vulkan/radv_wsi_x11.c       |  2 --
>> src/intel/vulkan/anv_wsi.c          |  1 -
>> src/intel/vulkan/anv_wsi_x11.c      |  2 --
>> src/vulkan/wsi/wsi_common.c         |  4 ++--
>> src/vulkan/wsi/wsi_common.h         |  4 +++-
>> src/vulkan/wsi/wsi_common_display.c |  1 -
>> src/vulkan/wsi/wsi_common_private.h |  1 -
>> src/vulkan/wsi/wsi_common_wayland.c |  1 -
>> src/vulkan/wsi/wsi_common_x11.c     | 25 +++++++++++--------------
>> src/vulkan/wsi/wsi_common_x11.h     |  1 -
>> 11 files changed, 16 insertions(+), 27 deletions(-)
>>
>> diff --git a/src/amd/vulkan/radv_wsi.c b/src/amd/vulkan/radv_wsi.c
>> index 6479bea070b..8b165ea3916 100644
>> --- a/src/amd/vulkan/radv_wsi.c
>> +++ b/src/amd/vulkan/radv_wsi.c
>> @@ -75,7 +75,6 @@ VkResult radv_GetPhysicalDeviceSurfaceSupportKHR(
>>    device->local_fd,
>>    queueFamilyIndex,
>>    surface,
>> -	     &device->instance->alloc,
>>    pSupported);
>> }
>>
>> diff --git a/src/amd/vulkan/radv_wsi_x11.c b/src/amd/vulkan/radv_wsi_x11.c
>> index c65ac938772..9ef02ccc435 100644
>> --- a/src/amd/vulkan/radv_wsi_x11.c
>> +++ b/src/amd/vulkan/radv_wsi_x11.c
>> @@ -44,7 +44,6 @@ VkBool32 radv_GetPhysicalDeviceXcbPresentationSupportKHR(
>>
>> return wsi_get_physical_device_xcb_presentation_support(
>> &device->wsi_device,
>> -      &device->instance->alloc,
>> queueFamilyIndex,
>> device->local_fd, true,
>> connection, visual_id);
>> @@ -60,7 +59,6 @@ VkBool32 radv_GetPhysicalDeviceXlibPresentationSupportKHR(
>>
>> return wsi_get_physical_device_xcb_presentation_support(
>> &device->wsi_device,
>> -      &device->instance->alloc,
>> queueFamilyIndex,
>> device->local_fd, true,
>> XGetXCBConnection(dpy), visualID);
>> diff --git a/src/intel/vulkan/anv_wsi.c b/src/intel/vulkan/anv_wsi.c
>> index 5ed1d711689..1c9a54804e8 100644
>> --- a/src/intel/vulkan/anv_wsi.c
>> +++ b/src/intel/vulkan/anv_wsi.c
>> @@ -92,7 +92,6 @@ VkResult anv_GetPhysicalDeviceSurfaceSupportKHR(
>>                                  device->local_fd,
>>                                  queueFamilyIndex,
>>                                  surface,
>> -                                         &device->instance->alloc,
>>                                  pSupported);
>> }
>>
>> diff --git a/src/intel/vulkan/anv_wsi_x11.c b/src/intel/vulkan/anv_wsi_x11.c
>> index 2feb5f13376..45c43f6f17f 100644
>> --- a/src/intel/vulkan/anv_wsi_x11.c
>> +++ b/src/intel/vulkan/anv_wsi_x11.c
>> @@ -40,7 +40,6 @@ VkBool32 anv_GetPhysicalDeviceXcbPresentationSupportKHR(
>>
>> return wsi_get_physical_device_xcb_presentation_support(
>> &device->wsi_device,
>> -      &device->instance->alloc,
>> queueFamilyIndex,
>> device->local_fd, false,
>> connection, visual_id);
>> @@ -56,7 +55,6 @@ VkBool32 anv_GetPhysicalDeviceXlibPresentationSupportKHR(
>>
>> return wsi_get_physical_device_xcb_presentation_support(
>> &device->wsi_device,
>> -      &device->instance->alloc,
>> queueFamilyIndex,
>> device->local_fd, false,
>> XGetXCBConnection(dpy), visualID);
>> diff --git a/src/vulkan/wsi/wsi_common.c b/src/vulkan/wsi/wsi_common.c
>> index 3416fef3076..1e3c4e0028b 100644
>> --- a/src/vulkan/wsi/wsi_common.c
>> +++ b/src/vulkan/wsi/wsi_common.c
>> @@ -39,6 +39,7 @@ wsi_device_init(struct wsi_device *wsi,
>>
>> memset(wsi, 0, sizeof(*wsi));
>>
>> +   wsi->instance_alloc = *alloc;
>> wsi->pdevice = pdevice;
>>
>> #define WSI_GET_CB(func) \
>> @@ -677,13 +678,12 @@ wsi_common_get_surface_support(struct wsi_device 
>> *wsi_device,
>>                        int local_fd,
>>                        uint32_t queueFamilyIndex,
>>                        VkSurfaceKHR _surface,
>> -                               const VkAllocationCallbacks *alloc,
>>                        VkBool32* pSupported)
>> {
>> ICD_FROM_HANDLE(VkIcdSurfaceBase, surface, _surface);
>> struct wsi_interface *iface = wsi_device->wsi[surface->platform];
>>
>> -   return iface->get_support(surface, wsi_device, alloc,
>> +   return iface->get_support(surface, wsi_device,
>>                      queueFamilyIndex, local_fd, pSupported);
>> }
>>
>> diff --git a/src/vulkan/wsi/wsi_common.h b/src/vulkan/wsi/wsi_common.h
>> index 14f65097bb3..424330de566 100644
>> --- a/src/vulkan/wsi/wsi_common.h
>> +++ b/src/vulkan/wsi/wsi_common.h
>> @@ -90,6 +90,9 @@ struct wsi_interface;
>> #define VK_ICD_WSI_PLATFORM_MAX (VK_ICD_WSI_PLATFORM_DISPLAY + 1)
>>
>> struct wsi_device {
>> +   /* Allocator for the instance */
>> +   VkAllocationCallbacks instance_alloc;
>> +
>> VkPhysicalDevice pdevice;
>> VkPhysicalDeviceMemoryProperties memory_props;
>> uint32_t queue_family_count;
>> @@ -166,7 +169,6 @@ wsi_common_get_surface_support(struct wsi_device 
>> *wsi_device,
>>                        int local_fd,
>>                        uint32_t queueFamilyIndex,
>>                        VkSurfaceKHR surface,
>> -                               const VkAllocationCallbacks *alloc,
>>                        VkBool32* pSupported);
>>
>> VkResult
>> diff --git a/src/vulkan/wsi/wsi_common_display.c 
>> b/src/vulkan/wsi/wsi_common_display.c
>> index 5cebde5b9bc..c004060a205 100644
>> --- a/src/vulkan/wsi/wsi_common_display.c
>> +++ b/src/vulkan/wsi/wsi_common_display.c
>> @@ -802,7 +802,6 @@ wsi_create_display_surface(VkInstance instance,
>> static VkResult
>> wsi_display_surface_get_support(VkIcdSurfaceBase *surface,
>>                         struct wsi_device *wsi_device,
>> -                                const VkAllocationCallbacks *allocator,
>>                         uint32_t queueFamilyIndex,
>>                         int local_fd,
>>                         VkBool32* pSupported)
>> diff --git a/src/vulkan/wsi/wsi_common_private.h 
>> b/src/vulkan/wsi/wsi_common_private.h
>> index ee7ae75b8f7..accc1170163 100644
>> --- a/src/vulkan/wsi/wsi_common_private.h
>> +++ b/src/vulkan/wsi/wsi_common_private.h
>> @@ -100,7 +100,6 @@ wsi_destroy_image(const struct wsi_swapchain *chain,
>> struct wsi_interface {
>> VkResult (*get_support)(VkIcdSurfaceBase *surface,
>>                    struct wsi_device *wsi_device,
>> -                           const VkAllocationCallbacks *alloc,
>>                    uint32_t queueFamilyIndex,
>>                    int local_fd,
>>                    VkBool32* pSupported);
>> diff --git a/src/vulkan/wsi/wsi_common_wayland.c 
>> b/src/vulkan/wsi/wsi_common_wayland.c
>> index 6b34e21bd98..67d5f8d4c80 100644
>> --- a/src/vulkan/wsi/wsi_common_wayland.c
>> +++ b/src/vulkan/wsi/wsi_common_wayland.c
>> @@ -464,7 +464,6 @@ wsi_wl_get_presentation_support(struct wsi_device 
>> *wsi_device,
>> static VkResult
>> wsi_wl_surface_get_support(VkIcdSurfaceBase *surface,
>>                    struct wsi_device *wsi_device,
>> -                           const VkAllocationCallbacks *alloc,
>>                    uint32_t queueFamilyIndex,
>>                    int local_fd,
>>                    VkBool32* pSupported)
>> diff --git a/src/vulkan/wsi/wsi_common_x11.c b/src/vulkan/wsi/wsi_common_x11.c
>> index 5f99b2ee473..75aa83bb7b4 100644
>> --- a/src/vulkan/wsi/wsi_common_x11.c
>> +++ b/src/vulkan/wsi/wsi_common_x11.c
>> @@ -125,7 +125,7 @@ wsi_x11_check_dri3_compatible(xcb_connection_t *conn, 
>> int local_fd)
>> }
>>
>> static struct wsi_x11_connection *
>> -wsi_x11_connection_create(const VkAllocationCallbacks *alloc,
>> +wsi_x11_connection_create(struct wsi_device *wsi_dev,
>
> Nit: I'd keep that `const`, it has no reason not to be

I guess. It also doesn't buy us anything.

>
>>                   xcb_connection_t *conn)
>> {
>> xcb_query_extension_cookie_t dri3_cookie, pres_cookie, amd_cookie, nv_cookie;
>> @@ -134,7 +134,7 @@ wsi_x11_connection_create(const VkAllocationCallbacks 
>> *alloc,
>> bool has_present_v1_2 = false;
>>
>> struct wsi_x11_connection *wsi_conn =
>> -      vk_alloc(alloc, sizeof(*wsi_conn), 8,
>> +      vk_alloc(&wsi_dev->instance_alloc, sizeof(*wsi_conn), 8,
>>         VK_SYSTEM_ALLOCATION_SCOPE_INSTANCE);
>> if (!wsi_conn)
>> return NULL;
>> @@ -163,7 +163,7 @@ wsi_x11_connection_create(const VkAllocationCallbacks 
>> *alloc,
>> free(pres_reply);
>> free(amd_reply);
>> free(nv_reply);
>> -      vk_free(alloc, wsi_conn);
>> +      vk_free(&wsi_dev->instance_alloc, wsi_conn);
>> return NULL;
>> }
>>
>> @@ -211,10 +211,10 @@ wsi_x11_connection_create(const VkAllocationCallbacks 
>> *alloc,
>> }
>>
>> static void
>> -wsi_x11_connection_destroy(const VkAllocationCallbacks *alloc,
>> +wsi_x11_connection_destroy(struct wsi_device *wsi_dev,
>
> Ditto: `const`
>
> Reviewed-by: Eric Engestrom <eric.engestrom at intel.com>
>
>>                    struct wsi_x11_connection *conn)
>> {
>> -   vk_free(alloc, conn);
>> +   vk_free(&wsi_dev->instance_alloc, conn);
>> }
>>
>> static bool
>> @@ -231,7 +231,6 @@ wsi_x11_check_for_dri3(struct wsi_x11_connection *wsi_conn)
>>
>> static struct wsi_x11_connection *
>> wsi_x11_get_connection(struct wsi_device *wsi_dev,
>> -	      const VkAllocationCallbacks *alloc,
>>                xcb_connection_t *conn)
>> {
>> struct wsi_x11 *wsi =
>> @@ -247,7 +246,7 @@ wsi_x11_get_connection(struct wsi_device *wsi_dev,
>> pthread_mutex_unlock(&wsi->mutex);
>>
>> struct wsi_x11_connection *wsi_conn =
>> -         wsi_x11_connection_create(alloc, conn);
>> +         wsi_x11_connection_create(wsi_dev, conn);
>> if (!wsi_conn)
>>  return NULL;
>>
>> @@ -256,7 +255,7 @@ wsi_x11_get_connection(struct wsi_device *wsi_dev,
>> entry = _mesa_hash_table_search(wsi->connections, conn);
>> if (entry) {
>>  /* Oops, someone raced us to it */
>> -         wsi_x11_connection_destroy(alloc, wsi_conn);
>> +         wsi_x11_connection_destroy(wsi_dev, wsi_conn);
>> } else {
>>  entry = _mesa_hash_table_insert(wsi->connections, conn, wsi_conn);
>> }
>> @@ -382,7 +381,6 @@ visual_has_alpha(xcb_visualtype_t *visual, unsigned depth)
>>
>> VkBool32 wsi_get_physical_device_xcb_presentation_support(
>> struct wsi_device *wsi_device,
>> -    VkAllocationCallbacks *alloc,
>> uint32_t                                    queueFamilyIndex,
>> int fd,
>> bool can_handle_different_gpu,
>> @@ -390,7 +388,7 @@ VkBool32 wsi_get_physical_device_xcb_presentation_support(
>> xcb_visualid_t                              visual_id)
>> {
>> struct wsi_x11_connection *wsi_conn =
>> -      wsi_x11_get_connection(wsi_device, alloc, connection);
>> +      wsi_x11_get_connection(wsi_device, connection);
>>
>> if (!wsi_conn)
>> return false;
>> @@ -433,7 +431,6 @@ x11_surface_get_window(VkIcdSurfaceBase *icd_surface)
>> static VkResult
>> x11_surface_get_support(VkIcdSurfaceBase *icd_surface,
>>                 struct wsi_device *wsi_device,
>> -                        const VkAllocationCallbacks *alloc,
>>                 uint32_t queueFamilyIndex,
>>                 int local_fd,
>>                 VkBool32* pSupported)
>> @@ -442,7 +439,7 @@ x11_surface_get_support(VkIcdSurfaceBase *icd_surface,
>> xcb_window_t window = x11_surface_get_window(icd_surface);
>>
>> struct wsi_x11_connection *wsi_conn =
>> -      wsi_x11_get_connection(wsi_device, alloc, conn);
>> +      wsi_x11_get_connection(wsi_device, conn);
>> if (!wsi_conn)
>> return VK_ERROR_OUT_OF_HOST_MEMORY;
>>
>> @@ -1279,7 +1276,7 @@ x11_surface_create_swapchain(VkIcdSurfaceBase 
>> *icd_surface,
>>
>> xcb_connection_t *conn = x11_surface_get_connection(icd_surface);
>> struct wsi_x11_connection *wsi_conn =
>> -      wsi_x11_get_connection(wsi_device, pAllocator, conn);
>> +      wsi_x11_get_connection(wsi_device, conn);
>> if (!wsi_conn)
>> return VK_ERROR_OUT_OF_HOST_MEMORY;
>>
>> @@ -1501,7 +1498,7 @@ wsi_x11_finish_wsi(struct wsi_device *wsi_device,
>> if (wsi) {
>> struct hash_entry *entry;
>> hash_table_foreach(wsi->connections, entry)
>> -         wsi_x11_connection_destroy(alloc, entry->data);
>> +         wsi_x11_connection_destroy(wsi_device, entry->data);
>>
>> _mesa_hash_table_destroy(wsi->connections, NULL);
>>
>> diff --git a/src/vulkan/wsi/wsi_common_x11.h b/src/vulkan/wsi/wsi_common_x11.h
>> index b33540856cb..e9b3ecfb2e0 100644
>> --- a/src/vulkan/wsi/wsi_common_x11.h
>> +++ b/src/vulkan/wsi/wsi_common_x11.h
>> @@ -27,7 +27,6 @@
>>
>> VkBool32 wsi_get_physical_device_xcb_presentation_support(
>> struct wsi_device *wsi_device,
>> -    VkAllocationCallbacks *alloc,
>> uint32_t                                    queueFamilyIndex,
>> int local_fd,
>> bool can_handle_different_gpu,
>> --
>> 2.19.1
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev





More information about the mesa-dev mailing list