From: Erico Nunes Date: Thu, 16 Nov 2023 16:00:16 +0000 (+0100) Subject: v3dv: Rework to remove drm authentication for wsi X-Git-Tag: upstream/23.3.3~189 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=16361af817e79e24147523e1481a36ccc8f8954a;p=platform%2Fupstream%2Fmesa.git v3dv: Rework to remove drm authentication for wsi For Wayland wsi allocations, v3dv used the wl_drm protocol, which is now being phased out in favor of dmabuf feedback. wl_drm is used to figure out the display device (in v3dv assumed to be vc4) and then to authenticate with the Wayland compositor in order to allocate scanout-able buffers (in this case, dumb buffers) directly at the display device. Recent commit 88c03ddd34 changed the behavior of the wsi code, and wl_drm is now passing the render device instead, which broke Wayland wsi. It turns out that the authentication code is not really needed and since we would like to remove wl_drm usage and the master device is assumed to be vc4 anyway, we can just remove some unneeded device-specific wsi code and get Vulkan Wayland wsi back to work. Fixes: 88c03ddd345 ("egl/drm: get compatible render-only device fd for kms-only device") Signed-off-by: Erico Nunes Reviewed-by: Iago Toral Quiroga Part-of: (cherry picked from commit 898700ca647b2de0eecff864b6b0a4cbeb935840) --- diff --git a/.pick_status.json b/.pick_status.json index 984fa21..b78e653 100644 --- a/.pick_status.json +++ b/.pick_status.json @@ -1594,7 +1594,7 @@ "description": "v3dv: Rework to remove drm authentication for wsi", "nominated": true, "nomination_type": 1, - "resolution": 0, + "resolution": 1, "main_sha": null, "because_sha": "88c03ddd345fe6b0cd16c11cb5c5309f8d7d16ff", "notes": null diff --git a/src/broadcom/vulkan/v3dv_device.c b/src/broadcom/vulkan/v3dv_device.c index 60c5512..027c35f 100644 --- a/src/broadcom/vulkan/v3dv_device.c +++ b/src/broadcom/vulkan/v3dv_device.c @@ -553,8 +553,6 @@ physical_device_finish(struct v3dv_physical_device *device) close(device->render_fd); if (device->display_fd >= 0) close(device->display_fd); - if (device->master_fd >= 0) - close(device->master_fd); free(device->name); @@ -636,273 +634,6 @@ compute_memory_budget(struct v3dv_physical_device *device) return MIN2(heap_size, heap_used + heap_available); } -#if !using_v3d_simulator -#ifdef VK_USE_PLATFORM_XCB_KHR -static int -create_display_fd_xcb(VkIcdSurfaceBase *surface) -{ - int fd = -1; - - xcb_connection_t *conn; - xcb_dri3_open_reply_t *reply = NULL; - if (surface) { - if (surface->platform == VK_ICD_WSI_PLATFORM_XLIB) - conn = XGetXCBConnection(((VkIcdSurfaceXlib *)surface)->dpy); - else - conn = ((VkIcdSurfaceXcb *)surface)->connection; - } else { - conn = xcb_connect(NULL, NULL); - } - - if (xcb_connection_has_error(conn)) - goto finish; - - const xcb_setup_t *setup = xcb_get_setup(conn); - xcb_screen_iterator_t iter = xcb_setup_roots_iterator(setup); - xcb_screen_t *screen = iter.data; - - xcb_dri3_open_cookie_t cookie; - cookie = xcb_dri3_open(conn, screen->root, None); - reply = xcb_dri3_open_reply(conn, cookie, NULL); - if (!reply) - goto finish; - - if (reply->nfd != 1) - goto finish; - - fd = xcb_dri3_open_reply_fds(conn, reply)[0]; - fcntl(fd, F_SETFD, fcntl(fd, F_GETFD) | FD_CLOEXEC); - -finish: - if (!surface) - xcb_disconnect(conn); - if (reply) - free(reply); - - return fd; -} -#endif - -#ifdef VK_USE_PLATFORM_WAYLAND_KHR -struct v3dv_wayland_info { - struct wl_drm *wl_drm; - int fd; - bool is_set; - bool authenticated; -}; - -static void -v3dv_drm_handle_device(void *data, struct wl_drm *drm, const char *device) -{ - struct v3dv_wayland_info *info = data; - info->fd = open(device, O_RDWR | O_CLOEXEC); - info->is_set = info->fd != -1; - if (!info->is_set) { - fprintf(stderr, "v3dv_drm_handle_device: could not open %s (%s)\n", - device, strerror(errno)); - return; - } - - drm_magic_t magic; - if (drmGetMagic(info->fd, &magic)) { - fprintf(stderr, "v3dv_drm_handle_device: drmGetMagic failed\n"); - close(info->fd); - info->fd = -1; - info->is_set = false; - return; - } - wl_drm_authenticate(info->wl_drm, magic); -} - -static void -v3dv_drm_handle_format(void *data, struct wl_drm *drm, uint32_t format) -{ -} - -static void -v3dv_drm_handle_authenticated(void *data, struct wl_drm *drm) -{ - struct v3dv_wayland_info *info = data; - info->authenticated = true; -} - -static void -v3dv_drm_handle_capabilities(void *data, struct wl_drm *drm, uint32_t value) -{ -} - -struct wl_drm_listener v3dv_drm_listener = { - .device = v3dv_drm_handle_device, - .format = v3dv_drm_handle_format, - .authenticated = v3dv_drm_handle_authenticated, - .capabilities = v3dv_drm_handle_capabilities -}; - -static void -v3dv_registry_global(void *data, - struct wl_registry *registry, - uint32_t name, - const char *interface, - uint32_t version) -{ - struct v3dv_wayland_info *info = data; - if (strcmp(interface, wl_drm_interface.name) == 0) { - info->wl_drm = wl_registry_bind(registry, name, &wl_drm_interface, - MIN2(version, 2)); - wl_drm_add_listener(info->wl_drm, &v3dv_drm_listener, data); - }; -} - -static void -v3dv_registry_global_remove_cb(void *data, - struct wl_registry *registry, - uint32_t name) -{ -} - -static int -create_display_fd_wayland(VkIcdSurfaceBase *surface) -{ - struct wl_display *display; - struct wl_registry *registry = NULL; - - struct v3dv_wayland_info info = { - .wl_drm = NULL, - .fd = -1, - .is_set = false, - .authenticated = false - }; - - if (surface) - display = ((VkIcdSurfaceWayland *) surface)->display; - else - display = wl_display_connect(NULL); - - if (!display) - return -1; - - registry = wl_display_get_registry(display); - if (!registry) { - if (!surface) - wl_display_disconnect(display); - return -1; - } - - static const struct wl_registry_listener registry_listener = { - v3dv_registry_global, - v3dv_registry_global_remove_cb - }; - wl_registry_add_listener(registry, ®istry_listener, &info); - - wl_display_roundtrip(display); /* For the registry advertisement */ - wl_display_roundtrip(display); /* For the DRM device event */ - wl_display_roundtrip(display); /* For the authentication event */ - - wl_drm_destroy(info.wl_drm); - wl_registry_destroy(registry); - - if (!surface) - wl_display_disconnect(display); - - if (!info.is_set) - return -1; - - if (!info.authenticated) - return -1; - - return info.fd; -} -#endif - -/* Acquire an authenticated display fd without a surface reference. This is the - * case where the application is making WSI allocations outside the Vulkan - * swapchain context (only Zink, for now). Since we lack information about the - * underlying surface we just try our best to figure out the correct display - * and platform to use. It should work in most cases. - */ -static void -acquire_display_device_no_surface(struct v3dv_physical_device *pdevice) -{ -#ifdef VK_USE_PLATFORM_WAYLAND_KHR - pdevice->display_fd = create_display_fd_wayland(NULL); -#endif - -#ifdef VK_USE_PLATFORM_XCB_KHR - if (pdevice->display_fd == -1) - pdevice->display_fd = create_display_fd_xcb(NULL); -#endif - -#ifdef VK_USE_PLATFORM_DISPLAY_KHR - if (pdevice->display_fd == - 1 && pdevice->master_fd >= 0) - pdevice->display_fd = dup(pdevice->master_fd); -#endif -} - -/* Acquire an authenticated display fd from the surface. This is the regular - * case where the application is using swapchains to create WSI allocations. - * In this case we use the surface information to figure out the correct - * display and platform combination. - */ -static void -acquire_display_device_surface(struct v3dv_physical_device *pdevice, - VkIcdSurfaceBase *surface) -{ - /* Mesa will set both of VK_USE_PLATFORM_{XCB,XLIB} when building with - * platform X11, so only check for XCB and rely on XCB to get an - * authenticated device also for Xlib. - */ -#ifdef VK_USE_PLATFORM_XCB_KHR - if (surface->platform == VK_ICD_WSI_PLATFORM_XCB || - surface->platform == VK_ICD_WSI_PLATFORM_XLIB) { - pdevice->display_fd = create_display_fd_xcb(surface); - } -#endif - -#ifdef VK_USE_PLATFORM_WAYLAND_KHR - if (surface->platform == VK_ICD_WSI_PLATFORM_WAYLAND) - pdevice->display_fd = create_display_fd_wayland(surface); -#endif - -#ifdef VK_USE_PLATFORM_DISPLAY_KHR - if (surface->platform == VK_ICD_WSI_PLATFORM_DISPLAY && - pdevice->master_fd >= 0) { - pdevice->display_fd = dup(pdevice->master_fd); - } -#endif -} -#endif /* !using_v3d_simulator */ - -/* Attempts to get an authenticated display fd from the display server that - * we can use to allocate BOs for presentable images. - */ -VkResult -v3dv_physical_device_acquire_display(struct v3dv_physical_device *pdevice, - VkIcdSurfaceBase *surface) -{ - VkResult result = VK_SUCCESS; - mtx_lock(&pdevice->mutex); - - if (pdevice->display_fd != -1) - goto done; - - /* When running on the simulator we do everything on a single render node so - * we don't need to get an authenticated display fd from the display server. - */ -#if !using_v3d_simulator - if (surface) - acquire_display_device_surface(pdevice, surface); - else - acquire_display_device_no_surface(pdevice); - - if (pdevice->display_fd == -1) - result = VK_ERROR_INITIALIZATION_FAILED; -#endif - -done: - mtx_unlock(&pdevice->mutex); - return result; -} - static bool v3d_has_feature(struct v3dv_physical_device *device, enum drm_v3d_param feature) { @@ -999,7 +730,7 @@ create_physical_device(struct v3dv_instance *instance, drmDevicePtr display_device) { VkResult result = VK_SUCCESS; - int32_t master_fd = -1; + int32_t display_fd = -1; int32_t render_fd = -1; struct v3dv_physical_device *device = @@ -1073,16 +804,19 @@ create_physical_device(struct v3dv_instance *instance, #endif if (instance->vk.enabled_extensions.KHR_display || + instance->vk.enabled_extensions.KHR_xcb_surface || + instance->vk.enabled_extensions.KHR_xlib_surface || + instance->vk.enabled_extensions.KHR_wayland_surface || instance->vk.enabled_extensions.EXT_acquire_drm_display) { #if !using_v3d_simulator /* Open the primary node on the vc4 display device */ assert(display_device); - master_fd = open(primary_path, O_RDWR | O_CLOEXEC); + display_fd = open(primary_path, O_RDWR | O_CLOEXEC); #else /* There is only one device with primary and render nodes. * Open its primary node. */ - master_fd = open(primary_path, O_RDWR | O_CLOEXEC); + display_fd = open(primary_path, O_RDWR | O_CLOEXEC); #endif } @@ -1091,8 +825,7 @@ create_physical_device(struct v3dv_instance *instance, #endif device->render_fd = render_fd; /* The v3d render node */ - device->display_fd = -1; /* Authenticated vc4 primary node */ - device->master_fd = master_fd; /* Master vc4 primary node */ + device->display_fd = display_fd; /* Master vc4 primary node */ if (!v3d_get_device_info(device->render_fd, &device->devinfo, &v3dv_ioctl)) { result = vk_errorf(instance, VK_ERROR_INITIALIZATION_FAILED, @@ -1196,8 +929,8 @@ fail: if (render_fd >= 0) close(render_fd); - if (master_fd >= 0) - close(master_fd); + if (display_fd >= 0) + close(display_fd); return result; } @@ -2253,18 +1986,8 @@ device_alloc_for_wsi(struct v3dv_device *device, #if using_v3d_simulator return device_alloc(device, mem, size); #else - /* If we are allocating for WSI we should have a swapchain and thus, - * we should've initialized the display device. However, Zink doesn't - * use swapchains, so in that case we can get here without acquiring the - * display device and we need to do it now. - */ VkResult result; struct v3dv_physical_device *pdevice = device->pdevice; - if (unlikely(pdevice->display_fd < 0)) { - result = v3dv_physical_device_acquire_display(pdevice, NULL); - if (result != VK_SUCCESS) - return result; - } assert(pdevice->display_fd != -1); mem->is_for_wsi = true; diff --git a/src/broadcom/vulkan/v3dv_private.h b/src/broadcom/vulkan/v3dv_private.h index 9c104b3..21934d8 100644 --- a/src/broadcom/vulkan/v3dv_private.h +++ b/src/broadcom/vulkan/v3dv_private.h @@ -138,7 +138,6 @@ struct v3dv_physical_device { char *name; int32_t render_fd; int32_t display_fd; - int32_t master_fd; /* We need these because it is not clear how to detect * valid devids in a portable way @@ -206,9 +205,6 @@ struct v3dv_physical_device { } caps; }; -VkResult v3dv_physical_device_acquire_display(struct v3dv_physical_device *pdevice, - VkIcdSurfaceBase *surface); - static inline struct v3dv_bo * v3dv_device_lookup_bo(struct v3dv_physical_device *device, uint32_t handle) { diff --git a/src/broadcom/vulkan/v3dv_wsi.c b/src/broadcom/vulkan/v3dv_wsi.c index 5efb1ea..404a64d 100644 --- a/src/broadcom/vulkan/v3dv_wsi.c +++ b/src/broadcom/vulkan/v3dv_wsi.c @@ -24,8 +24,6 @@ */ #include "v3dv_private.h" -#include "drm-uapi/drm_fourcc.h" -#include "wsi_common_entrypoints.h" #include "vk_util.h" #include "wsi_common.h" #include "wsi_common_drm.h" @@ -41,19 +39,7 @@ static bool v3dv_wsi_can_present_on_device(VkPhysicalDevice _pdevice, int fd) { V3DV_FROM_HANDLE(v3dv_physical_device, pdevice, _pdevice); - - /* There are some instances with direct display extensions where this may be - * called before we have ever tried to create a swapchain, and therefore, - * before we have ever tried to acquire the display device, in which case we - * have to do it now. - */ - if (unlikely(pdevice->display_fd < 0 && pdevice->master_fd >= 0)) { - VkResult result = - v3dv_physical_device_acquire_display(pdevice, NULL); - if (result != VK_SUCCESS) - return false; - } - + assert(pdevice->display_fd != -1); return wsi_common_drm_devices_equal(fd, pdevice->display_fd); } @@ -66,7 +52,7 @@ v3dv_wsi_init(struct v3dv_physical_device *physical_device) v3dv_physical_device_to_handle(physical_device), v3dv_wsi_proc_addr, &physical_device->vk.instance->alloc, - physical_device->master_fd, NULL, + physical_device->display_fd, NULL, &(struct wsi_device_options){.sw_device = false}); if (result != VK_SUCCESS) @@ -89,67 +75,6 @@ v3dv_wsi_finish(struct v3dv_physical_device *physical_device) &physical_device->vk.instance->alloc); } -static void -constraint_surface_capabilities(VkSurfaceCapabilitiesKHR *caps) -{ - /* Our display pipeline requires that images are linear, so we cannot - * ensure that our swapchain images can be sampled. If we are running under - * a compositor in windowed mode, the DRM modifier negotiation should - * probably end up selecting an UIF layout for the swapchain images but it - * may still choose linear and send images directly for scanout if the - * surface is in fullscreen mode for example. If we are not running under - * a compositor, then we would always need them to be linear anyway. - */ - caps->supportedUsageFlags &= ~VK_IMAGE_USAGE_SAMPLED_BIT; -} - -VKAPI_ATTR VkResult VKAPI_CALL -v3dv_GetPhysicalDeviceSurfaceCapabilitiesKHR( - VkPhysicalDevice physicalDevice, - VkSurfaceKHR surface, - VkSurfaceCapabilitiesKHR* pSurfaceCapabilities) -{ - VkResult result; - result = wsi_GetPhysicalDeviceSurfaceCapabilitiesKHR(physicalDevice, - surface, - pSurfaceCapabilities); - constraint_surface_capabilities(pSurfaceCapabilities); - return result; -} - -VKAPI_ATTR VkResult VKAPI_CALL -v3dv_GetPhysicalDeviceSurfaceCapabilities2KHR( - VkPhysicalDevice physicalDevice, - const VkPhysicalDeviceSurfaceInfo2KHR* pSurfaceInfo, - VkSurfaceCapabilities2KHR* pSurfaceCapabilities) -{ - VkResult result; - result = wsi_GetPhysicalDeviceSurfaceCapabilities2KHR(physicalDevice, - pSurfaceInfo, - pSurfaceCapabilities); - constraint_surface_capabilities(&pSurfaceCapabilities->surfaceCapabilities); - return result; -} - -VKAPI_ATTR VkResult VKAPI_CALL -v3dv_CreateSwapchainKHR( - VkDevice _device, - const VkSwapchainCreateInfoKHR* pCreateInfo, - const VkAllocationCallbacks* pAllocator, - VkSwapchainKHR* pSwapchain) -{ - V3DV_FROM_HANDLE(v3dv_device, device, _device); - struct v3dv_physical_device *pdevice = device->pdevice; - - ICD_FROM_HANDLE(VkIcdSurfaceBase, surface, pCreateInfo->surface); - VkResult result = - v3dv_physical_device_acquire_display(pdevice, surface); - if (result != VK_SUCCESS) - return result; - - return wsi_CreateSwapchainKHR(_device, pCreateInfo, pAllocator, pSwapchain); -} - struct v3dv_image * v3dv_wsi_get_image_from_swapchain(VkSwapchainKHR swapchain, uint32_t index) {