backend/drm: introduce wlr_drm_bo_handle_table

Using GBM to import DRM dumb buffers tends to not work well. By
using GBM we're calling some driver-specific functions in Mesa.
These functions check whether Mesa can work with the buffer.
Sometimes Mesa has requirements which differ from DRM dumb buffers
and the GBM import will fail (e.g. on amdgpu).

Instead, drop GBM and use drmPrimeFDToHandle directly. But there's
a twist: BO handles are not ref'counted by the kernel and need to
be ref'counted in user-space [1]. libdrm usually performs this
bookkeeping and is used under-the-hood by Mesa.

We can't re-use libdrm for this task without using driver-specific
APIs. So let's just re-implement the ref'counting logic in wlroots.
The wlroots implementation is inspired from amdgpu's in libdrm [2].

Closes: https://github.com/swaywm/wlroots/issues/2916

[1]: https://gitlab.freedesktop.org/mesa/drm/-/merge_requests/110
[2]: 1a4c0ec9ae/amdgpu/handle_table.c

backend/drm/atomic.c

@@ -1,4 +1,3 @@
-#include <gbm.h>
 #include <stdlib.h>
 #include <wlr/util/log.h>
 #include <xf86drm.h>
@@ -144,8 +143,8 @@ static void set_plane_props(struct atomic *atom, struct wlr_drm_backend *drm,
 		goto error;
 	}
 
-	uint32_t width = gbm_bo_get_width(fb->bo);
-	uint32_t height = gbm_bo_get_height(fb->bo);
+	uint32_t width = fb->wlr_buf->width;
+	uint32_t height = fb->wlr_buf->height;
 
 	// The src_* properties are in 16.16 fixed point
 	atomic_add(atom, id, props->src_x, 0);

backend/drm/backend.c

@@ -53,7 +53,7 @@ static void backend_destroy(struct wlr_backend *backend) {
 	wl_list_remove(&drm->dev_change.link);
 	wl_list_remove(&drm->dev_remove.link);
 
-	gbm_device_destroy(drm->gbm);
+	drm_bo_handle_table_finish(&drm->bo_handles);
 
 	if (drm->parent) {
 		finish_drm_renderer(&drm->mgpu_renderer);
@@ -224,12 +224,6 @@ struct wlr_backend *wlr_drm_backend_create(struct wl_display *display,
 		goto error_event;
 	}
 
-	drm->gbm = gbm_create_device(drm->fd);
-	if (!drm->gbm) {
-		wlr_log(WLR_ERROR, "Failed to create GBM device");
-		goto error_resources;
-	}
-
 	if (drm->parent) {
 		// Ensure we use the same renderer as the parent backend
 		drm->backend.renderer = wlr_backend_get_renderer(&drm->parent->backend);

backend/drm/bo_handle_table.c

@@ -0,0 +1,43 @@
+#include <assert.h>
+#include <stdint.h>
+#include <stdlib.h>
+#include <wlr/util/log.h>
+#include "backend/drm/bo_handle_table.h"
+
+static size_t align(size_t val, size_t align) {
+	size_t mask = align - 1;
+	return (val + mask) & ~mask;
+}
+
+void drm_bo_handle_table_finish(struct wlr_drm_bo_handle_table *table) {
+	free(table->nrefs);
+}
+
+bool drm_bo_handle_table_ref(struct wlr_drm_bo_handle_table *table,
+		uint32_t handle) {
+	assert(handle != 0);
+
+	if (handle > table->len) {
+		// Grow linearily, because we don't expect the number of BOs to explode
+		size_t len = align(handle + 1, 512);
+		size_t *nrefs = realloc(table->nrefs, len * sizeof(size_t));
+		if (nrefs == NULL) {
+			wlr_log_errno(WLR_ERROR, "realloc failed");
+			return false;
+		}
+		memset(&nrefs[table->len], 0, (len - table->len) * sizeof(size_t));
+		table->len = len;
+		table->nrefs = nrefs;
+	}
+
+	table->nrefs[handle]++;
+	return true;
+}
+
+size_t drm_bo_handle_table_unref(struct wlr_drm_bo_handle_table *table,
+		uint32_t handle) {
+	assert(handle < table->len);
+	assert(table->nrefs[handle] > 0);
+	table->nrefs[handle]--;
+	return table->nrefs[handle];
+}

backend/drm/drm.c

@@ -3,7 +3,6 @@
 #include <drm_fourcc.h>
 #include <drm_mode.h>
 #include <errno.h>
-#include <gbm.h>
 #include <inttypes.h>
 #include <stdint.h>
 #include <stdio.h>

backend/drm/legacy.c

@@ -1,5 +1,4 @@
 #include <assert.h>
-#include <gbm.h>
 #include <stdlib.h>
 #include <wlr/util/log.h>
 #include <xf86drm.h>
@@ -10,20 +9,23 @@
 
 static bool legacy_fb_props_match(struct wlr_drm_fb *fb1,
 		struct wlr_drm_fb *fb2) {
-	if (fb1->wlr_buf->width != fb2->wlr_buf->width ||
-			fb1->wlr_buf->height != fb2->wlr_buf->height ||
-			gbm_bo_get_format(fb1->bo) != gbm_bo_get_format(fb2->bo) ||
-			gbm_bo_get_modifier(fb1->bo) != gbm_bo_get_modifier(fb2->bo) ||
-			gbm_bo_get_plane_count(fb1->bo) != gbm_bo_get_plane_count(fb2->bo)) {
+	struct wlr_dmabuf_attributes dmabuf1 = {0}, dmabuf2 = {0};
+	if (!wlr_buffer_get_dmabuf(fb1->wlr_buf, &dmabuf1) ||
+			!wlr_buffer_get_dmabuf(fb2->wlr_buf, &dmabuf2)) {
 		return false;
 	}
 
-	for (int i = 0; i < gbm_bo_get_plane_count(fb1->bo); i++) {
-		if (gbm_bo_get_stride_for_plane(fb1->bo, i) !=
-				gbm_bo_get_stride_for_plane(fb2->bo, i)) {
-			return false;
-		}
-		if (gbm_bo_get_offset(fb1->bo, i) != gbm_bo_get_offset(fb2->bo, i)) {
+	if (dmabuf1.width != dmabuf2.width ||
+			dmabuf1.height != dmabuf2.height ||
+			dmabuf1.format != dmabuf2.format ||
+			dmabuf1.modifier != dmabuf2.modifier ||
+			dmabuf1.n_planes != dmabuf2.n_planes) {
+		return false;
+	}
+
+	for (int i = 0; i < dmabuf1.n_planes; i++) {
+		if (dmabuf1.stride[i] != dmabuf2.stride[i] ||
+				dmabuf1.offset[i] != dmabuf2.offset[i]) {
 			return false;
 		}
 	}
@@ -140,9 +142,9 @@ static bool legacy_crtc_commit(struct wlr_drm_connector *conn,
 			return false;
 		}
 
-		uint32_t cursor_handle = gbm_bo_get_handle(cursor_fb->bo).u32;
-		uint32_t cursor_width = gbm_bo_get_width(cursor_fb->bo);
-		uint32_t cursor_height = gbm_bo_get_height(cursor_fb->bo);
+		uint32_t cursor_handle = cursor_fb->handles[0];
+		uint32_t cursor_width = cursor_fb->wlr_buf->width;
+		uint32_t cursor_height = cursor_fb->wlr_buf->height;
 		if (drmModeSetCursor(drm->fd, crtc->id, cursor_handle,
 				cursor_width, cursor_height)) {
 			wlr_drm_conn_log_errno(conn, WLR_DEBUG, "drmModeSetCursor failed");

backend/drm/meson.build

@@ -1,6 +1,7 @@
 wlr_files += files(
 	'atomic.c',
 	'backend.c',
+	'bo_handle_table.c',
 	'cvt.c',
 	'drm.c',
 	'legacy.c',

backend/drm/renderer.c

@@ -2,7 +2,6 @@
 #include <assert.h>
 #include <drm_fourcc.h>
 #include <fcntl.h>
-#include <gbm.h>
 #include <stdbool.h>
 #include <stdlib.h>
 #include <string.h>
@@ -195,43 +194,6 @@ void drm_fb_clear(struct wlr_drm_fb **fb_ptr) {
 	*fb_ptr = NULL;
 }
 
-static struct gbm_bo *get_bo_for_dmabuf(struct gbm_device *gbm,
-		struct wlr_dmabuf_attributes *attribs) {
-	if (attribs->modifier != DRM_FORMAT_MOD_INVALID ||
-			attribs->n_planes > 1 || attribs->offset[0] != 0) {
-		struct gbm_import_fd_modifier_data data = {
-			.width = attribs->width,
-			.height = attribs->height,
-			.format = attribs->format,
-			.num_fds = attribs->n_planes,
-			.modifier = attribs->modifier,
-		};
-
-		if ((size_t)attribs->n_planes > sizeof(data.fds) / sizeof(data.fds[0])) {
-			return false;
-		}
-
-		for (size_t i = 0; i < (size_t)attribs->n_planes; ++i) {
-			data.fds[i] = attribs->fd[i];
-			data.strides[i] = attribs->stride[i];
-			data.offsets[i] = attribs->offset[i];
-		}
-
-		return gbm_bo_import(gbm, GBM_BO_IMPORT_FD_MODIFIER,
-			&data, GBM_BO_USE_SCANOUT);
-	} else {
-		struct gbm_import_fd_data data = {
-			.fd = attribs->fd[0],
-			.width = attribs->width,
-			.height = attribs->height,
-			.stride = attribs->stride[0],
-			.format = attribs->format,
-		};
-
-		return gbm_bo_import(gbm, GBM_BO_IMPORT_FD, &data, GBM_BO_USE_SCANOUT);
-	}
-}
-
 static void drm_fb_handle_destroy(struct wlr_addon *addon) {
 	struct wlr_drm_fb *fb = wl_container_of(addon, fb, addon);
 	drm_fb_destroy(fb);
@@ -242,6 +204,83 @@ static const struct wlr_addon_interface fb_addon_impl = {
 	.destroy = drm_fb_handle_destroy,
 };
 
+static uint32_t get_bo_handle_for_fd(struct wlr_drm_backend *drm,
+		int dmabuf_fd) {
+	uint32_t handle = 0;
+	int ret = drmPrimeFDToHandle(drm->fd, dmabuf_fd, &handle);
+	if (ret != 0) {
+		wlr_log_errno(WLR_DEBUG, "drmPrimeFDToHandle failed");
+		return 0;
+	}
+
+	if (!drm_bo_handle_table_ref(&drm->bo_handles, handle)) {
+		// If that failed, the handle wasn't ref'ed in the table previously,
+		// so safe to delete
+		struct drm_gem_close args = { .handle = handle };
+		drmIoctl(drm->fd, DRM_IOCTL_GEM_CLOSE, &args);
+		return 0;
+	}
+
+	return handle;
+}
+
+static void close_bo_handle(struct wlr_drm_backend *drm, uint32_t handle) {
+	if (handle == 0) {
+		return;
+	}
+
+	size_t nrefs = drm_bo_handle_table_unref(&drm->bo_handles, handle);
+	if (nrefs > 0) {
+		return;
+	}
+
+	struct drm_gem_close args = { .handle = handle };
+	if (drmIoctl(drm->fd, DRM_IOCTL_GEM_CLOSE, &args) != 0) {
+		wlr_log_errno(WLR_ERROR, "drmIoctl(GEM_CLOSE) failed");
+	}
+}
+
+static uint32_t get_fb_for_bo(struct wlr_drm_backend *drm,
+		struct wlr_dmabuf_attributes *dmabuf, uint32_t handles[static 4]) {
+	uint64_t modifiers[4] = {0};
+	for (int i = 0; i < dmabuf->n_planes; i++) {
+		// KMS requires all BO planes to have the same modifier
+		modifiers[i] = dmabuf->modifier;
+	}
+
+	uint32_t id = 0;
+	if (drm->addfb2_modifiers && dmabuf->modifier != DRM_FORMAT_MOD_INVALID) {
+		if (drmModeAddFB2WithModifiers(drm->fd, dmabuf->width, dmabuf->height,
+				dmabuf->format, handles, dmabuf->stride, dmabuf->offset,
+				modifiers, &id, DRM_MODE_FB_MODIFIERS) != 0) {
+			wlr_log_errno(WLR_DEBUG, "drmModeAddFB2WithModifiers failed");
+		}
+	} else {
+		int ret = drmModeAddFB2(drm->fd, dmabuf->width, dmabuf->height,
+			dmabuf->format, handles, dmabuf->stride, dmabuf->offset, &id, 0);
+		if (ret != 0 && dmabuf->format == DRM_FORMAT_ARGB8888 &&
+				dmabuf->n_planes == 1 && dmabuf->offset[0] == 0) {
+			// Some big-endian machines don't support drmModeAddFB2. Try a
+			// last-resort fallback for ARGB8888 buffers, like Xorg's
+			// modesetting driver does.
+			wlr_log(WLR_DEBUG, "drmModeAddFB2 failed (%s), falling back to "
+				"legacy drmModeAddFB", strerror(-ret));
+
+			uint32_t depth = 32;
+			uint32_t bpp = 32;
+			ret = drmModeAddFB(drm->fd, dmabuf->width, dmabuf->height, depth,
+				bpp, dmabuf->stride[0], handles[0], &id);
+			if (ret != 0) {
+				wlr_log_errno(WLR_DEBUG, "drmModeAddFB failed");
+			}
+		} else if (ret != 0) {
+			wlr_log_errno(WLR_DEBUG, "drmModeAddFB2 failed");
+		}
+	}
+
+	return id;
+}
+
 static struct wlr_drm_fb *drm_fb_create(struct wlr_drm_backend *drm,
 		struct wlr_buffer *buf, const struct wlr_drm_format_set *formats) {
 	struct wlr_drm_fb *fb = calloc(1, sizeof(*fb));
@@ -279,18 +318,20 @@ static struct wlr_drm_fb *drm_fb_create(struct wlr_drm_backend *drm,
 		}
 	}
 
-	fb->bo = get_bo_for_dmabuf(drm->gbm, &attribs);
-	if (!fb->bo) {
-		wlr_log(WLR_DEBUG, "Failed to import DMA-BUF in GBM");
-		goto error_get_dmabuf;
+	for (int i = 0; i < attribs.n_planes; ++i) {
+		fb->handles[i] = get_bo_handle_for_fd(drm, attribs.fd[i]);
+		if (fb->handles[i] == 0) {
+			goto error_bo_handle;
+		}
 	}
 
-	fb->id = get_fb_for_bo(fb->bo, drm->addfb2_modifiers);
+	fb->id = get_fb_for_bo(drm, &attribs, fb->handles);
 	if (!fb->id) {
-		wlr_log(WLR_DEBUG, "Failed to import GBM BO in KMS");
-		goto error_get_fb_for_bo;
+		wlr_log(WLR_DEBUG, "Failed to import BO in KMS");
+		goto error_bo_handle;
 	}
 
+	fb->backend = drm;
 	fb->wlr_buf = buf;
 
 	wlr_addon_init(&fb->addon, &buf->addons, drm, &fb_addon_impl);
@@ -298,23 +339,29 @@ static struct wlr_drm_fb *drm_fb_create(struct wlr_drm_backend *drm,
 
 	return fb;
 
-error_get_fb_for_bo:
-	gbm_bo_destroy(fb->bo);
+error_bo_handle:
+	for (int i = 0; i < attribs.n_planes; ++i) {
+		close_bo_handle(drm, fb->handles[i]);
+	}
 error_get_dmabuf:
 	free(fb);
 	return NULL;
 }
 
 void drm_fb_destroy(struct wlr_drm_fb *fb) {
+	struct wlr_drm_backend *drm = fb->backend;
+
 	wl_list_remove(&fb->link);
 	wlr_addon_finish(&fb->addon);
 
-	struct gbm_device *gbm = gbm_bo_get_device(fb->bo);
-	if (drmModeRmFB(gbm_device_get_fd(gbm), fb->id) != 0) {
+	if (drmModeRmFB(drm->fd, fb->id) != 0) {
 		wlr_log(WLR_ERROR, "drmModeRmFB failed");
 	}
 
-	gbm_bo_destroy(fb->bo);
+	for (size_t i = 0; i < sizeof(fb->handles) / sizeof(fb->handles[0]); ++i) {
+		close_bo_handle(drm, fb->handles[i]);
+	}
+
 	free(fb);
 }
 

backend/drm/util.c

@@ -2,7 +2,6 @@
 #include <drm_fourcc.h>
 #include <drm_mode.h>
 #include <drm.h>
-#include <gbm.h>
 #include <stdio.h>
 #include <string.h>
 #include <wlr/util/log.h>
@@ -175,56 +174,6 @@ const char *conn_get_name(uint32_t type_id) {
 	}
 }
 
-uint32_t get_fb_for_bo(struct gbm_bo *bo, bool with_modifiers) {
-	struct gbm_device *gbm = gbm_bo_get_device(bo);
-
-	int fd = gbm_device_get_fd(gbm);
-	uint32_t width = gbm_bo_get_width(bo);
-	uint32_t height = gbm_bo_get_height(bo);
-	uint32_t format = gbm_bo_get_format(bo);
-
-	uint32_t handles[4] = {0};
-	uint32_t strides[4] = {0};
-	uint32_t offsets[4] = {0};
-	uint64_t modifiers[4] = {0};
-	for (int i = 0; i < gbm_bo_get_plane_count(bo); i++) {
-		handles[i] = gbm_bo_get_handle_for_plane(bo, i).u32;
-		strides[i] = gbm_bo_get_stride_for_plane(bo, i);
-		offsets[i] = gbm_bo_get_offset(bo, i);
-		// KMS requires all BO planes to have the same modifier
-		modifiers[i] = gbm_bo_get_modifier(bo);
-	}
-
-	uint32_t id = 0;
-	if (with_modifiers && gbm_bo_get_modifier(bo) != DRM_FORMAT_MOD_INVALID) {
-		if (drmModeAddFB2WithModifiers(fd, width, height, format, handles,
-				strides, offsets, modifiers, &id, DRM_MODE_FB_MODIFIERS)) {
-			wlr_log_errno(WLR_ERROR, "Unable to add DRM framebuffer");
-		}
-	} else {
-		int ret = drmModeAddFB2(fd, width, height, format, handles, strides,
-			offsets, &id, 0);
-		if (ret != 0 && gbm_bo_get_format(bo) == GBM_FORMAT_ARGB8888 &&
-				gbm_bo_get_plane_count(bo) == 1) {
-			// Some big-endian machines don't support drmModeAddFB2. Try a
-			// last-resort fallback for ARGB8888 buffers, like Xorg's
-			// modesetting driver does.
-			wlr_log(WLR_DEBUG, "drmModeAddFB2 failed (%s), falling back to "
-				"legacy drmModeAddFB", strerror(-ret));
-
-			uint32_t depth = 32;
-			uint32_t bpp = gbm_bo_get_bpp(bo);
-			ret = drmModeAddFB(fd, width, height, depth, bpp, strides[0],
-				handles[0], &id);
-		}
-		if (ret != 0) {
-			wlr_log(WLR_ERROR, "Unable to add DRM framebuffer: %s", strerror(-ret));
-		}
-	}
-
-	return id;
-}
-
 static bool is_taken(size_t n, const uint32_t arr[static n], uint32_t key) {
 	for (size_t i = 0; i < n; ++i) {
 		if (arr[i] == key) {

include/backend/drm/bo_handle_table.h

@@ -0,0 +1,24 @@
+#ifndef BACKEND_DRM_BO_HANDLE_TABLE_H
+#define BACKEND_DRM_BO_HANDLE_TABLE_H
+
+/**
+ * Table performing reference counting for buffer object handles.
+ *
+ * The BO handles are allocated incrementally and are recycled by the kernel,
+ * so a simple array is used.
+ *
+ * This design is inspired from amdgpu's code in libdrm:
+ * https://gitlab.freedesktop.org/mesa/drm/-/blob/1a4c0ec9aea13211997f982715fe5ffcf19dd067/amdgpu/handle_table.c
+ */
+struct wlr_drm_bo_handle_table {
+	size_t *nrefs;
+	size_t len;
+};
+
+void drm_bo_handle_table_finish(struct wlr_drm_bo_handle_table *table);
+bool drm_bo_handle_table_ref(struct wlr_drm_bo_handle_table *table,
+	uint32_t handle);
+size_t drm_bo_handle_table_unref(struct wlr_drm_bo_handle_table *table,
+	uint32_t handle);
+
+#endif

include/backend/drm/drm.h

@@ -1,7 +1,6 @@
 #ifndef BACKEND_DRM_DRM_H
 #define BACKEND_DRM_DRM_H
 
-#include <gbm.h>
 #include <stdbool.h>
 #include <stddef.h>
 #include <stdint.h>
@@ -12,6 +11,7 @@
 #include <wlr/backend/session.h>
 #include <wlr/render/drm_format_set.h>
 #include <xf86drmMode.h>
+#include "backend/drm/bo_handle_table.h"
 #include "backend/drm/iface.h"
 #include "backend/drm/properties.h"
 #include "backend/drm/renderer.h"
@@ -62,7 +62,7 @@ struct wlr_drm_backend {
 	int fd;
 	char *name;
 	struct wlr_device *dev;
-	struct gbm_device *gbm;
+	struct wlr_drm_bo_handle_table bo_handles;
 
 	size_t num_crtcs;
 	struct wlr_drm_crtc *crtcs;

include/backend/drm/iface.h

@@ -1,7 +1,6 @@
 #ifndef BACKEND_DRM_IFACE_H
 #define BACKEND_DRM_IFACE_H
 
-#include <gbm.h>
 #include <stdbool.h>
 #include <stdint.h>
 #include <xf86drm.h>

include/backend/drm/renderer.h

@@ -1,7 +1,6 @@
 #ifndef BACKEND_DRM_RENDERER_H
 #define BACKEND_DRM_RENDERER_H
 
-#include <gbm.h>
 #include <stdbool.h>
 #include <stdint.h>
 #include <wlr/backend.h>
@@ -30,9 +29,10 @@ struct wlr_drm_surface {
 struct wlr_drm_fb {
 	struct wlr_buffer *wlr_buf;
 	struct wlr_addon addon;
+	struct wlr_drm_backend *backend;
 	struct wl_list link; // wlr_drm_backend.fbs
 
-	struct gbm_bo *bo;
+	uint32_t handles[WLR_DMABUF_MAX_PLANES];
 	uint32_t id;
 };
 

include/backend/drm/util.h

@@ -13,8 +13,6 @@ void parse_edid(struct wlr_output *restrict output, size_t len,
 	const uint8_t *data);
 // Returns the string representation of a DRM output type
 const char *conn_get_name(uint32_t type_id);
-// Returns the DRM framebuffer id for a gbm_bo
-uint32_t get_fb_for_bo(struct gbm_bo *bo, bool with_modifiers);
 
 // Part of match_obj
 enum {