From d9d8fc1ab9b77d5696f8ed79de0568bf25d31500 Mon Sep 17 00:00:00 2001 From: Simon Ser Date: Wed, 1 Sep 2021 19:05:18 +0200 Subject: [PATCH] render/allocator: re-open GBM FD Using the same DRM file description for the DRM backend and for the GBM allocator will result in GEM handle ref'counting issues [1]. Re-open the DRM FD to fix these issues. [1]: https://gitlab.freedesktop.org/mesa/drm/-/merge_requests/110 --- render/allocator/allocator.c | 41 ++++++++++++++++++++++++++++++++---- render/allocator/drm_dumb.c | 25 ++++------------------ render/allocator/gbm.c | 8 +------ 3 files changed, 42 insertions(+), 32 deletions(-) diff --git a/render/allocator/allocator.c b/render/allocator/allocator.c index 8664202f..affc928e 100644 --- a/render/allocator/allocator.c +++ b/render/allocator/allocator.c @@ -1,5 +1,8 @@ +#define _POSIX_C_SOURCE 200809L #include +#include #include +#include #include #include #include "backend/backend.h" @@ -17,6 +20,26 @@ void wlr_allocator_init(struct wlr_allocator *alloc, wl_signal_init(&alloc->events.destroy); } +/* Re-open the DRM node to avoid GEM handle ref'counting issues. See: + * https://gitlab.freedesktop.org/mesa/drm/-/merge_requests/110 + * TODO: don't assume we have the permission to just open the DRM node, + * find another way to re-open it. + */ +static int reopen_drm_node(int drm_fd) { + char *name = drmGetDeviceNameFromFd2(drm_fd); + if (name == NULL) { + wlr_log(WLR_ERROR, "drmGetDeviceNameFromFd2 failed"); + return -1; + } + + int new_fd = open(name, O_RDWR | O_CLOEXEC); + if (new_fd < 0) { + wlr_log_errno(WLR_ERROR, "Failed to open DRM node '%s'", name); + } + free(name); + return new_fd; +} + struct wlr_allocator *allocator_autocreate_with_drm_fd( struct wlr_backend *backend, struct wlr_renderer *renderer, int drm_fd) { @@ -26,11 +49,16 @@ struct wlr_allocator *allocator_autocreate_with_drm_fd( struct wlr_allocator *alloc = NULL; uint32_t gbm_caps = WLR_BUFFER_CAP_DMABUF; if ((backend_caps & gbm_caps) && (renderer_caps & gbm_caps) - && drm_fd != -1) { + && drm_fd >= 0) { wlr_log(WLR_DEBUG, "Trying to create gbm allocator"); - if ((alloc = wlr_gbm_allocator_create(drm_fd)) != NULL) { + int gbm_fd = reopen_drm_node(drm_fd); + if (gbm_fd < 0) { + return NULL; + } + if ((alloc = wlr_gbm_allocator_create(gbm_fd)) != NULL) { return alloc; } + close(gbm_fd); wlr_log(WLR_DEBUG, "Failed to create gbm allocator"); } @@ -45,11 +73,16 @@ struct wlr_allocator *allocator_autocreate_with_drm_fd( uint32_t drm_caps = WLR_BUFFER_CAP_DMABUF | WLR_BUFFER_CAP_DATA_PTR; if ((backend_caps & drm_caps) && (renderer_caps & drm_caps) - && drm_fd != -1) { + && drm_fd >= 0 && drmIsMaster(drm_fd)) { wlr_log(WLR_DEBUG, "Trying to create drm dumb allocator"); - if ((alloc = wlr_drm_dumb_allocator_create(drm_fd)) != NULL) { + int dumb_fd = reopen_drm_node(drm_fd); + if (dumb_fd < 0) { + return NULL; + } + if ((alloc = wlr_drm_dumb_allocator_create(dumb_fd)) != NULL) { return alloc; } + close(dumb_fd); wlr_log(WLR_DEBUG, "Failed to create drm dumb allocator"); } diff --git a/render/allocator/drm_dumb.c b/render/allocator/drm_dumb.c index a47df624..ea982a03 100644 --- a/render/allocator/drm_dumb.c +++ b/render/allocator/drm_dumb.c @@ -199,41 +199,25 @@ static const struct wlr_allocator_interface allocator_impl = { .destroy = allocator_destroy, }; -struct wlr_allocator *wlr_drm_dumb_allocator_create(int fd) { - if (!drmIsMaster(fd)) { - wlr_log(WLR_ERROR, "Cannot use DRM dumb buffers with non-master DRM FD"); - return NULL; - } - - /* Re-open the DRM node to avoid GEM handle ref'counting issues. See: - * https://gitlab.freedesktop.org/mesa/drm/-/merge_requests/110 - * TODO: don't assume we have the permission to just open the DRM node, - * find another way to re-open it. - */ - char *path = drmGetDeviceNameFromFd2(fd); - int drm_fd = open(path, O_RDWR | O_CLOEXEC); - if (drm_fd < 0) { - wlr_log_errno(WLR_ERROR, "Failed to open DRM node %s", path); - free(path); +struct wlr_allocator *wlr_drm_dumb_allocator_create(int drm_fd) { + if (drmGetNodeTypeFromFd(drm_fd) != DRM_NODE_PRIMARY) { + wlr_log(WLR_ERROR, "Cannot use DRM dumb buffers with non-primary DRM FD"); return NULL; } uint64_t has_dumb = 0; if (drmGetCap(drm_fd, DRM_CAP_DUMB_BUFFER, &has_dumb) < 0) { wlr_log(WLR_ERROR, "Failed to get DRM capabilities"); - free(path); return NULL; } if (has_dumb == 0) { wlr_log(WLR_ERROR, "DRM dumb buffers not supported"); - free(path); return NULL; } struct wlr_drm_dumb_allocator *alloc = calloc(1, sizeof(*alloc)); if (alloc == NULL) { - free(path); return NULL; } wlr_allocator_init(&alloc->base, &allocator_impl, @@ -242,7 +226,6 @@ struct wlr_allocator *wlr_drm_dumb_allocator_create(int fd) { alloc->drm_fd = drm_fd; wl_list_init(&alloc->buffers); - wlr_log(WLR_DEBUG, "Created DRM dumb allocator with node %s", path); - free(path); + wlr_log(WLR_DEBUG, "Created DRM dumb allocator"); return &alloc->base; } diff --git a/render/allocator/gbm.c b/render/allocator/gbm.c index ace7ca68..0092afd6 100644 --- a/render/allocator/gbm.c +++ b/render/allocator/gbm.c @@ -163,13 +163,7 @@ static struct wlr_gbm_allocator *get_gbm_alloc_from_alloc( return (struct wlr_gbm_allocator *)alloc; } -struct wlr_allocator *wlr_gbm_allocator_create(int drm_fd) { - int fd = fcntl(drm_fd, F_DUPFD_CLOEXEC, 0); - if (fd < 0) { - wlr_log(WLR_ERROR, "fcntl(F_DUPFD_CLOEXEC) failed"); - return NULL; - } - +struct wlr_allocator *wlr_gbm_allocator_create(int fd) { uint64_t cap; if (drmGetCap(fd, DRM_CAP_PRIME, &cap) || !(cap & DRM_PRIME_CAP_EXPORT)) {