From 17fb3ad94f97a908b55f5a7a1f2130b90338bdbd Mon Sep 17 00:00:00 2001 From: Jason Ekstrand Date: Mon, 2 Mar 2020 17:38:16 -0600 Subject: [PATCH] vulkan/wsi: Only use a single fd per wsi_image The only thing this was helping was X11 where the protocol requires that we pass in an array of images. We can move all the dup() code to the X11 back-end and leave the others a bit cleaner. Reviewed-by: Lionel Landwerlin Reviewed-by: Simon Ser Reviewed-by: Adam Jackson Part-of: --- src/vulkan/wsi/wsi_common.c | 9 +++------ src/vulkan/wsi/wsi_common_display.c | 7 +++---- src/vulkan/wsi/wsi_common_drm.c | 22 ++++++---------------- src/vulkan/wsi/wsi_common_private.h | 2 +- src/vulkan/wsi/wsi_common_wayland.c | 6 +++--- src/vulkan/wsi/wsi_common_x11.c | 23 ++++++++++++++++++----- 6 files changed, 34 insertions(+), 35 deletions(-) diff --git a/src/vulkan/wsi/wsi_common.c b/src/vulkan/wsi/wsi_common.c index a2ba2021fb9..21106057e09 100644 --- a/src/vulkan/wsi/wsi_common.c +++ b/src/vulkan/wsi/wsi_common.c @@ -476,8 +476,7 @@ wsi_create_image(const struct wsi_swapchain *chain, VkResult result; memset(image, 0, sizeof(*image)); - for (int i = 0; i < ARRAY_SIZE(image->fds); i++) - image->fds[i] = -1; + image->dma_buf_fd = -1; result = wsi->CreateImage(chain->device, &info->create, &chain->alloc, &image->image); @@ -512,10 +511,8 @@ wsi_destroy_image(const struct wsi_swapchain *chain, { const struct wsi_device *wsi = chain->wsi; - for (int p = 0; p < image->num_planes; p++) { - if (image->fds[p] >= 0) - close(image->fds[p]); - } + if (image->dma_buf_fd >= 0) + close(image->dma_buf_fd); if (image->buffer.blit_cmd_buffers) { for (uint32_t i = 0; i < wsi->queue_family_count; i++) { diff --git a/src/vulkan/wsi/wsi_common_display.c b/src/vulkan/wsi/wsi_common_display.c index 410dc4220c2..22823c0aea6 100644 --- a/src/vulkan/wsi/wsi_common_display.c +++ b/src/vulkan/wsi/wsi_common_display.c @@ -1104,14 +1104,13 @@ wsi_display_image_init(VkDevice device_h, memset(image->buffer, 0, sizeof (image->buffer)); for (unsigned int i = 0; i < image->base.num_planes; i++) { - int ret = drmPrimeFDToHandle(wsi->fd, image->base.fds[i], + int ret = drmPrimeFDToHandle(wsi->fd, image->base.dma_buf_fd, &image->buffer[i]); - - close(image->base.fds[i]); - image->base.fds[i] = -1; if (ret < 0) goto fail_handle; } + close(image->base.dma_buf_fd); + image->base.dma_buf_fd = -1; image->chain = chain; image->state = WSI_IMAGE_IDLE; diff --git a/src/vulkan/wsi/wsi_common_drm.c b/src/vulkan/wsi/wsi_common_drm.c index 9b00ed2218a..2f82963841d 100644 --- a/src/vulkan/wsi/wsi_common_drm.c +++ b/src/vulkan/wsi/wsi_common_drm.c @@ -345,7 +345,6 @@ wsi_create_native_image_mem(const struct wsi_swapchain *chain, if (result != VK_SUCCESS) return result; - int fd = -1; if (!wsi->sw) { const VkMemoryGetFdInfoKHR memory_get_fd_info = { .sType = VK_STRUCTURE_TYPE_MEMORY_GET_FD_INFO_KHR, @@ -354,7 +353,8 @@ wsi_create_native_image_mem(const struct wsi_swapchain *chain, .handleType = VK_EXTERNAL_MEMORY_HANDLE_TYPE_DMA_BUF_BIT_EXT, }; - result = wsi->GetMemoryFdKHR(chain->device, &memory_get_fd_info, &fd); + result = wsi->GetMemoryFdKHR(chain->device, &memory_get_fd_info, + &image->dma_buf_fd); if (result != VK_SUCCESS) return result; } @@ -366,10 +366,9 @@ wsi_create_native_image_mem(const struct wsi_swapchain *chain, result = wsi->GetImageDrmFormatModifierPropertiesEXT(chain->device, image->image, &image_mod_props); - if (result != VK_SUCCESS) { - close(fd); + if (result != VK_SUCCESS) return result; - } + image->drm_modifier = image_mod_props.drmFormatModifier; assert(image->drm_modifier != DRM_FORMAT_MOD_INVALID); @@ -389,13 +388,6 @@ wsi_create_native_image_mem(const struct wsi_swapchain *chain, image->sizes[p] = image_layout.size; image->row_pitches[p] = image_layout.rowPitch; image->offsets[p] = image_layout.offset; - if (p == 0) { - image->fds[p] = fd; - } else { - image->fds[p] = os_dupfd_cloexec(fd); - if (image->fds[p] == -1) - return VK_ERROR_OUT_OF_HOST_MEMORY; - } } } else { const VkImageSubresource image_subresource = { @@ -412,7 +404,6 @@ wsi_create_native_image_mem(const struct wsi_swapchain *chain, image->sizes[0] = reqs.size; image->row_pitches[0] = image_layout.rowPitch; image->offsets[0] = 0; - image->fds[0] = fd; } return VK_SUCCESS; @@ -439,14 +430,13 @@ wsi_create_prime_image_mem(const struct wsi_swapchain *chain, .memory = image->buffer.memory, .handleType = VK_EXTERNAL_MEMORY_HANDLE_TYPE_DMA_BUF_BIT_EXT, }; - int fd; - result = wsi->GetMemoryFdKHR(chain->device, &linear_memory_get_fd_info, &fd); + result = wsi->GetMemoryFdKHR(chain->device, &linear_memory_get_fd_info, + &image->dma_buf_fd); if (result != VK_SUCCESS) return result; image->drm_modifier = info->prime_use_linear_modifier ? DRM_FORMAT_MOD_LINEAR : DRM_FORMAT_MOD_INVALID; - image->fds[0] = fd; return VK_SUCCESS; } diff --git a/src/vulkan/wsi/wsi_common_private.h b/src/vulkan/wsi/wsi_common_private.h index c6e575cd518..ae0b196b3aa 100644 --- a/src/vulkan/wsi/wsi_common_private.h +++ b/src/vulkan/wsi/wsi_common_private.h @@ -79,7 +79,7 @@ struct wsi_image { uint32_t sizes[4]; uint32_t offsets[4]; uint32_t row_pitches[4]; - int fds[4]; + int dma_buf_fd; }; struct wsi_swapchain { diff --git a/src/vulkan/wsi/wsi_common_wayland.c b/src/vulkan/wsi/wsi_common_wayland.c index ac2d918d127..06e3c395aa4 100644 --- a/src/vulkan/wsi/wsi_common_wayland.c +++ b/src/vulkan/wsi/wsi_common_wayland.c @@ -1160,15 +1160,15 @@ wsi_wl_image_init(struct wsi_wl_swapchain *chain, for (int i = 0; i < image->base.num_planes; i++) { zwp_linux_buffer_params_v1_add(params, - image->base.fds[i], + image->base.dma_buf_fd, i, image->base.offsets[i], image->base.row_pitches[i], image->base.drm_modifier >> 32, image->base.drm_modifier & 0xffffffff); - close(image->base.fds[i]); - image->base.fds[i] = -1; } + close(image->base.dma_buf_fd); + image->base.dma_buf_fd = -1; image->buffer = zwp_linux_buffer_params_v1_create_immed(params, diff --git a/src/vulkan/wsi/wsi_common_x11.c b/src/vulkan/wsi/wsi_common_x11.c index bdd869c8c2f..ebfa345351f 100644 --- a/src/vulkan/wsi/wsi_common_x11.c +++ b/src/vulkan/wsi/wsi_common_x11.c @@ -40,6 +40,7 @@ #include #include "drm-uapi/drm_fourcc.h" #include "util/hash_table.h" +#include "util/os_file.h" #include "util/os_time.h" #include "util/u_debug.h" #include "util/u_thread.h" @@ -1620,6 +1621,19 @@ x11_image_init(VkDevice device_h, struct x11_swapchain *chain, /* If the image has a modifier, we must have DRI3 v1.2. */ assert(chain->has_dri3_modifiers); + /* XCB requires an array of file descriptors but we only have one */ + int fds[4] = { -1, -1, -1, -1 }; + fds[0] = image->base.dma_buf_fd; + for (int i = 1; i < image->base.num_planes; i++) { + fds[i] = os_dupfd_cloexec(image->base.dma_buf_fd); + if (fds[i] == -1) { + for (int j = 1; j < i; j++) + close(fds[j]); + + return VK_ERROR_OUT_OF_HOST_MEMORY; + } + } + cookie = xcb_dri3_pixmap_from_buffers_checked(chain->conn, image->pixmap, @@ -1637,7 +1651,7 @@ x11_image_init(VkDevice device_h, struct x11_swapchain *chain, image->base.offsets[3], chain->depth, bpp, image->base.drm_modifier, - image->base.fds); + fds); } else #endif { @@ -1653,14 +1667,13 @@ x11_image_init(VkDevice device_h, struct x11_swapchain *chain, pCreateInfo->imageExtent.height, image->base.row_pitches[0], chain->depth, bpp, - image->base.fds[0]); + image->base.dma_buf_fd); } xcb_discard_reply(chain->conn, cookie.sequence); - /* XCB has now taken ownership of the FDs. */ - for (int i = 0; i < image->base.num_planes; i++) - image->base.fds[i] = -1; + /* XCB has now taken ownership of the FD. */ + image->base.dma_buf_fd = -1; out_fence: fence_fd = xshmfence_alloc_shm();