[weston,2/2] simple-dmabuf-drm: use GBM generic calls

Submitted by Emilio Pozuelo Monfort on July 19, 2018, 2:45 p.m.

Details

Message ID 20180719144502.28665-2-emilio.pozuelo@collabora.co.uk
State New
Series "simple-dmabuf-drm: fix build with --disable-egl"
Headers show

Commit Message

Emilio Pozuelo Monfort July 19, 2018, 2:45 p.m.
No need to write libdrm driver specific code for each supported
driver, we can just let GBM call the right one for us now.

Signed-off-by: Emilio Pozuelo Monfort <emilio.pozuelo@collabora.co.uk>
---
Some improvements from Daniel (thanks!). I also added missing error messages,
formatting fixes and a logic error in your improvements that caused NV12 to
abort if create_bo was working fine.

No changes to XRGB, so that's likely to still fail for you. I suspect
a bug in mesa's i965 code, I can look at that separately.

Cheers,
Emilio

 clients/simple-dmabuf-drm.c | 444 ++++++++++--------------------------
 configure.ac                |   2 +-
 2 files changed, 122 insertions(+), 324 deletions(-)

Patch hide | download patch | download mbox

diff --git a/clients/simple-dmabuf-drm.c b/clients/simple-dmabuf-drm.c
index 5bc5323e..5cbd2bb1 100644
--- a/clients/simple-dmabuf-drm.c
+++ b/clients/simple-dmabuf-drm.c
@@ -41,21 +41,11 @@ 
 #include <getopt.h>
 #include <errno.h>
 
-#include <xf86drm.h>
-
-#ifdef HAVE_LIBDRM_INTEL
-#include <i915_drm.h>
-#include <intel_bufmgr.h>
-#endif
-#ifdef HAVE_LIBDRM_FREEDRENO
-#include <freedreno/freedreno_drmif.h>
-#endif
-#ifdef HAVE_LIBDRM_ETNAVIV
-#include <etnaviv_drmif.h>
-#endif
+#include <gbm.h>
 #include <drm_fourcc.h>
 
 #include <wayland-client.h>
+#include "shared/helpers.h"
 #include "shared/zalloc.h"
 #include "xdg-shell-unstable-v6-client-protocol.h"
 #include "fullscreen-shell-unstable-v1-client-protocol.h"
@@ -65,14 +55,10 @@ 
 #define DRM_FORMAT_MOD_LINEAR 0
 #endif
 
-struct buffer;
-
 /* Possible options that affect the displayed image */
 #define OPT_Y_INVERTED 1  /* contents has y axis inverted */
 #define OPT_IMMEDIATE  2  /* create wl_buffer immediately */
 
-#define ALIGN(v, a) ((v + a - 1) & ~(a - 1))
-
 struct display {
 	struct wl_display *display;
 	struct wl_registry *registry;
@@ -86,46 +72,24 @@  struct display {
 	int req_dmabuf_immediate;
 };
 
-struct drm_device {
-	int fd;
-	char *name;
-
-	int (*alloc_bo)(struct buffer *buf);
-	void (*free_bo)(struct buffer *buf);
-	int (*export_bo_to_prime)(struct buffer *buf);
-	int (*map_bo)(struct buffer *buf);
-	void (*unmap_bo)(struct buffer *buf);
-	void (*device_destroy)(struct buffer *buf);
-};
-
 struct buffer {
 	struct wl_buffer *buffer;
 	int busy;
 
-	struct drm_device *dev;
 	int drm_fd;
+	struct gbm_device *dev;
+	struct gbm_bo *bo;
+	struct gbm_bo *bo2;
 
-#ifdef HAVE_LIBDRM_INTEL
-	drm_intel_bufmgr *bufmgr;
-	drm_intel_bo *intel_bo;
-#endif /* HAVE_LIBDRM_INTEL */
-#if HAVE_LIBDRM_FREEDRENO
-	struct fd_device *fd_dev;
-	struct fd_bo *fd_bo;
-#endif /* HAVE_LIBDRM_FREEDRENO */
-#if HAVE_LIBDRM_ETNAVIV
-	struct etna_device *etna_dev;
-	struct etna_bo *etna_bo;
-#endif /* HAVE_LIBDRM_ETNAVIV */
-
-	uint32_t gem_handle;
 	int dmabuf_fd;
-	uint8_t *mmap;
+	int dmabuf_fd2;
+	void *mmap;
+	void *mmap2;
 
 	int width;
 	int height;
-	int bpp;
-	unsigned long stride;
+	uint32_t stride;
+	uint32_t stride2;
 	int format;
 };
 
@@ -161,170 +125,6 @@  static const struct wl_buffer_listener buffer_listener = {
 	buffer_release
 };
 
-
-#ifdef HAVE_LIBDRM_INTEL
-static int
-intel_alloc_bo(struct buffer *my_buf)
-{
-	/* XXX: try different tiling modes for testing FB modifiers. */
-	uint32_t tiling = I915_TILING_NONE;
-
-	assert(my_buf->bufmgr);
-
-	my_buf->intel_bo = drm_intel_bo_alloc_tiled(my_buf->bufmgr, "test",
-						    my_buf->width, my_buf->height,
-						    (my_buf->bpp / 8), &tiling,
-						    &my_buf->stride, 0);
-
-	printf("buffer allocated w %d, h %d, stride %lu, size %lu\n",
-	       my_buf->width, my_buf->height, my_buf->stride, my_buf->intel_bo->size);
-
-	if (!my_buf->intel_bo)
-		return 0;
-
-	if (tiling != I915_TILING_NONE)
-		return 0;
-
-	return 1;
-}
-
-static void
-intel_free_bo(struct buffer *my_buf)
-{
-	drm_intel_bo_unreference(my_buf->intel_bo);
-}
-
-static int
-intel_map_bo(struct buffer *my_buf)
-{
-	if (drm_intel_gem_bo_map_gtt(my_buf->intel_bo) != 0)
-		return 0;
-
-	my_buf->mmap = my_buf->intel_bo->virtual;
-
-	return 1;
-}
-
-static int
-intel_bo_export_to_prime(struct buffer *buffer)
-{
-	return drm_intel_bo_gem_export_to_prime(buffer->intel_bo, &buffer->dmabuf_fd);
-}
-
-static void
-intel_unmap_bo(struct buffer *my_buf)
-{
-	drm_intel_gem_bo_unmap_gtt(my_buf->intel_bo);
-}
-
-static void
-intel_device_destroy(struct buffer *my_buf)
-{
-	drm_intel_bufmgr_destroy(my_buf->bufmgr);
-}
-
-#endif /* HAVE_LIBDRM_INTEL */
-#ifdef HAVE_LIBDRM_FREEDRENO
-
-static int
-fd_alloc_bo(struct buffer *buf)
-{
-	int flags = DRM_FREEDRENO_GEM_CACHE_WCOMBINE;
-	int size;
-
-	buf->fd_dev = fd_device_new(buf->drm_fd);
-	buf->stride = ALIGN(buf->width, 32) * buf->bpp / 8;
-	size = buf->stride * buf->height;
-	buf->fd_dev = fd_device_new(buf->drm_fd);
-	buf->fd_bo = fd_bo_new(buf->fd_dev, size, flags);
-
-	if (!buf->fd_bo)
-		return 0;
-	return 1;
-}
-
-static void
-fd_free_bo(struct buffer *buf)
-{
-	fd_bo_del(buf->fd_bo);
-}
-
-static int
-fd_bo_export_to_prime(struct buffer *buf)
-{
-	buf->dmabuf_fd = fd_bo_dmabuf(buf->fd_bo);
-	return buf->dmabuf_fd < 0;
-}
-
-static int
-fd_map_bo(struct buffer *buf)
-{
-	buf->mmap = fd_bo_map(buf->fd_bo);
-	return buf->mmap != NULL;
-}
-
-static void
-fd_unmap_bo(struct buffer *buf)
-{
-}
-
-static void
-fd_device_destroy(struct buffer *buf)
-{
-	fd_device_del(buf->fd_dev);
-}
-#endif /* HAVE_LIBDRM_FREEDRENO */
-#ifdef HAVE_LIBDRM_ETNAVIV
-
-static int
-etna_alloc_bo(struct buffer *buf)
-{
-	int flags = DRM_ETNA_GEM_CACHE_WC;
-	int size;
-
-	buf->stride = ALIGN(buf->width, 32) * buf->bpp / 8;
-	size = 	buf->stride * buf->height;
-	buf->etna_dev = etna_device_new(buf->drm_fd);
-	buf->etna_bo = etna_bo_new(buf->etna_dev, size, flags);
-
-	return buf->etna_bo != NULL;
-}
-
-static void
-etna_free_bo(struct buffer *buf)
-{
-	etna_bo_del(buf->etna_bo);
-}
-
-static int
-etna_bo_export_to_prime(struct buffer *buf)
-{
-	buf->dmabuf_fd = etna_bo_dmabuf(buf->etna_bo);
-	return buf->dmabuf_fd < 0;
-}
-
-static int
-etna_map_bo(struct buffer *buf)
-{
-	buf->mmap = etna_bo_map(buf->etna_bo);
-	return buf->mmap != NULL;
-}
-
-static void
-etna_unmap_bo(struct buffer *buf)
-{
-	if (munmap(buf->mmap, buf->stride * buf->height) < 0)
-		fprintf(stderr, "Failed to unmap buffer: %s", strerror(errno));
-	buf->mmap = NULL;
-}
-
-static void
-etna_device_destroy(struct buffer *buf)
-{
-	etna_device_del(buf->etna_dev);
-}
-#endif /* HAVE_LIBDRM_ENTAVIV */
-
 static void
 fill_content(struct buffer *my_buf, uint64_t modifier)
 {
@@ -343,19 +143,18 @@  fill_content(struct buffer *my_buf, uint64_t modifier)
 		}
 		else if (modifier == DRM_FORMAT_MOD_LINEAR) {
 			uint8_t *pix8;
-			/* first plane: Y (2/3 of the buffer)	*/
-			for (y = 0; y < my_buf->height * 2/3; y++) {
+			/* first plane: Y */
+			for (y = 0; y < my_buf->height; y++) {
 				pix8 = my_buf->mmap + y * my_buf->stride;
 				for (x = 0; x < my_buf->width; x++)
 					*pix8++ = x % 256;
 			}
-			/* second plane (CbCr) is half the size of Y
-			   plane (last 1/3 of the buffer) */
-			for (y = my_buf->height * 2/3; y < my_buf->height; y++) {
-				pix8 = my_buf->mmap + y * my_buf->stride;
-				for (x = 0; x < my_buf->width; x+=2) {
-					*pix8++ = x % 256;
-					*pix8++ = y % 256;
+			/* second plane (CbCr) is half the size of Y */
+			for (y = 0; y < my_buf->height / 2; y++) {
+				pix8 = my_buf->mmap2 + y * my_buf->stride2;
+				for (x = 0; x < my_buf->width / 2; x++) {
+					*pix8++ = (x * 2) % 256;
+					*pix8++ = (y * 2) % 256;
 				}
 			}
 		}
@@ -371,69 +170,26 @@  fill_content(struct buffer *my_buf, uint64_t modifier)
 }
 
 static void
-drm_device_destroy(struct buffer *buf)
+buffer_free(struct buffer *buf)
 {
-	buf->dev->device_destroy(buf);
-	close(buf->drm_fd);
-}
+	if (buf->buffer)
+		wl_buffer_destroy(buf->buffer);
 
-static int
-drm_device_init(struct buffer *buf)
-{
-	struct drm_device *dev = calloc(1, sizeof(struct drm_device));
+	if (buf->bo)
+		gbm_bo_destroy(buf->bo);
+	if (buf->bo2)
+		gbm_bo_destroy(buf->bo2);
 
-	drmVersionPtr version = drmGetVersion(buf->drm_fd);
+	close(buf->dmabuf_fd);
+	close(buf->dmabuf_fd2);
 
-	dev->fd = buf->drm_fd;
-	dev->name = strdup(version->name);
-	if (0) {
-		/* nothing */
-	}
-#ifdef HAVE_LIBDRM_INTEL
-	else if (!strcmp(dev->name, "i915")) {
-		buf->bufmgr = drm_intel_bufmgr_gem_init(buf->drm_fd, 32);
-		if (!buf->bufmgr)
-			return 0;
-		dev->alloc_bo = intel_alloc_bo;
-		dev->free_bo = intel_free_bo;
-		dev->export_bo_to_prime = intel_bo_export_to_prime;
-		dev->map_bo = intel_map_bo;
-		dev->unmap_bo = intel_unmap_bo;
-		dev->device_destroy = intel_device_destroy;
-	}
-#endif
-#ifdef HAVE_LIBDRM_FREEDRENO
-	else if (!strcmp(dev->name, "msm")) {
-		dev->alloc_bo = fd_alloc_bo;
-		dev->free_bo = fd_free_bo;
-		dev->export_bo_to_prime = fd_bo_export_to_prime;
-		dev->map_bo = fd_map_bo;
-		dev->unmap_bo = fd_unmap_bo;
-		dev->device_destroy = fd_device_destroy;
-	}
-#endif
-#ifdef HAVE_LIBDRM_ETNAVIV
-	else if (!strcmp(dev->name, "etnaviv")) {
-		dev->alloc_bo = etna_alloc_bo;
-		dev->free_bo = etna_free_bo;
-		dev->export_bo_to_prime = etna_bo_export_to_prime;
-		dev->map_bo = etna_map_bo;
-		dev->unmap_bo = etna_unmap_bo;
-		dev->device_destroy = etna_device_destroy;
-	}
-#endif
-	else {
-		fprintf(stderr, "Error: drm device %s unsupported.\n",
-			dev->name);
-		free(dev);
-		return 0;
-	}
-	buf->dev = dev;
-	return 1;
+	if (buf->dev)
+		gbm_device_destroy(buf->dev);
+	close(buf->drm_fd);
 }
 
-static int
-drm_connect(struct buffer *my_buf)
+static struct gbm_device *
+gbm_connect(struct buffer *my_buf)
 {
 	/* This won't work with card0 as we need to be authenticated; instead,
 	 * boot with drm.rnodes=1 and use that. */
@@ -441,16 +197,9 @@  drm_connect(struct buffer *my_buf)
 	if (my_buf->drm_fd < 0)
 		return 0;
 
-	return drm_device_init(my_buf);
+	return my_buf->dev = gbm_create_device(my_buf->drm_fd);
 }
 
-static void
-drm_shutdown(struct buffer *my_buf)
-{
-	drm_device_destroy(my_buf);
-}
-
-
 static void
 create_succeeded(void *data,
 		 struct zwp_linux_buffer_params_v1 *params,
@@ -482,54 +231,111 @@  static const struct zwp_linux_buffer_params_v1_listener params_listener = {
 	create_failed
 };
 
+static struct gbm_bo *
+create_bo(struct buffer *buffer, int format, int subsamp,
+	  const uint64_t *modifiers, const unsigned int count)
+{
+	struct gbm_bo *bo;
+
+	if (!modifiers) {
+		bo = gbm_bo_create(buffer->dev,
+		                   buffer->width / subsamp, buffer->height / subsamp,
+				   format,
+				   0 /* usage */);
+	} else {
+		bo = gbm_bo_create_with_modifiers(buffer->dev,
+						  buffer->width / subsamp,
+						  buffer->height / subsamp,
+						  format,
+						  modifiers,
+						  count);
+	}
+
+	if (!bo)
+		fprintf(stderr, "bo_create failed\n");
+
+	return bo;
+}
+
 static int
 create_dmabuf_buffer(struct display *display, struct buffer *buffer,
 		     int width, int height, int format, uint32_t opts)
 {
 	struct zwp_linux_buffer_params_v1 *params;
-	uint64_t modifier = 0;
+	uint64_t modifier = 0, modifier2 = 0;
 	uint32_t flags = 0;
-	struct drm_device *drm_dev;
 
-	if (!drm_connect(buffer)) {
-		fprintf(stderr, "drm_connect failed\n");
+	if (!gbm_connect(buffer)) {
+		fprintf(stderr, "gbm_device_connect failed\n");
 		goto error;
 	}
-	drm_dev = buffer->dev;
 
 	buffer->width = width;
+	buffer->height = height;
+	buffer->format = format;
 	switch (format) {
 	case DRM_FORMAT_NV12:
-		/* adjust height for allocation of NV12 Y and UV planes */
-		buffer->height = height * 3 / 2;
-		buffer->bpp = 8;
 		modifier = display->nv12_modifier;
 		break;
-	default:
-		buffer->height = height;
-		buffer->bpp = 32;
 	}
-	buffer->format = format;
 
-	if (!drm_dev->alloc_bo(buffer)) {
-		fprintf(stderr, "alloc_bo failed\n");
-		goto error1;
+	if (format == DRM_FORMAT_XRGB8888) {
+		uint64_t modifiers[] = { DRM_FORMAT_MOD_LINEAR };
+
+		buffer->bo = create_bo(buffer, format, 1,
+				       modifiers, ARRAY_LENGTH(modifiers));
+		if (!buffer->bo) {
+			fprintf(stderr, "create_bo for XRGB8888 failed\n");
+			goto error;
+		}
+	} else {
+		uint64_t modifiers[] = { modifier };
+
+		buffer->bo = create_bo(buffer, GBM_FORMAT_R8, 1,
+				       modifiers, ARRAY_LENGTH(modifiers));
+		buffer->bo2 = create_bo(buffer, GBM_FORMAT_GR88, 2,
+					modifiers, ARRAY_LENGTH(modifiers));
+		if (!buffer->bo || !buffer->bo2) {
+			fprintf(stderr, "create_bo for NV12 failed\n");
+			goto error;
+		}
 	}
 
-	if (!drm_dev->map_bo(buffer)) {
+	buffer->mmap = gbm_bo_map(buffer->bo, 0 /* x */, 0 /* y */,
+				  width, height,
+				  GBM_BO_TRANSFER_WRITE,
+				  &buffer->stride, &buffer->mmap);
+	if (!buffer->mmap) {
 		fprintf(stderr, "map_bo failed\n");
-		goto error2;
+		goto error;
 	}
-	fill_content(buffer, modifier);
-	drm_dev->unmap_bo(buffer);
 
-	if (drm_dev->export_bo_to_prime(buffer) != 0) {
-		fprintf(stderr, "gem_export_to_prime failed\n");
-		goto error2;
+	if (format == DRM_FORMAT_NV12) {
+		buffer->mmap2 = gbm_bo_map(buffer->bo2, 0 /* x */, 0 /* y */,
+					   width / 2, height / 2,
+					   GBM_BO_TRANSFER_WRITE,
+					   &buffer->stride2, &buffer->mmap2);
+		if (!buffer->mmap2) {
+			fprintf(stderr, "map_bo failed\n");
+			goto error;
+		}
 	}
-	if (buffer->dmabuf_fd < 0) {
+
+	modifier = gbm_bo_get_modifier(buffer->bo);
+	fill_content(buffer, modifier);
+	gbm_bo_unmap(buffer->bo, buffer->mmap);
+	if (buffer->bo2)
+		gbm_bo_unmap(buffer->bo2, buffer->mmap2);
+
+	buffer->dmabuf_fd = gbm_bo_get_fd(buffer->bo);
+	if (buffer->bo2)
+		buffer->dmabuf_fd2 = gbm_bo_get_fd(buffer->bo2);
+	else
+		buffer->dmabuf_fd2 = -1;
+
+	if (buffer->dmabuf_fd < 0 || (format == DRM_FORMAT_NV12 && buffer->dmabuf_fd2 < 0)) {
 		fprintf(stderr, "error: dmabuf_fd < 0\n");
-		goto error2;
+		goto error;
 	}
 
 	/* We now have a dmabuf! For format XRGB8888, it should contain 2x2
@@ -554,12 +360,12 @@  create_dmabuf_buffer(struct display *display, struct buffer *buffer,
 	if (format == DRM_FORMAT_NV12) {
 		/* add the second plane params */
 		zwp_linux_buffer_params_v1_add(params,
-					       buffer->dmabuf_fd,
+					       buffer->dmabuf_fd2,
 					       1,
-					       buffer->width * buffer->height,
-					       buffer->stride,
-					       modifier >> 32,
-					       modifier & 0xffffffff);
+					       0,
+					       buffer->stride2,
+					       modifier2 >> 32,
+					       modifier2 & 0xffffffff);
 	}
 	zwp_linux_buffer_params_v1_add_listener(params, &params_listener, buffer);
 	if (display->req_dmabuf_immediate) {
@@ -579,11 +385,8 @@  create_dmabuf_buffer(struct display *display, struct buffer *buffer,
 
 	return 0;
 
-error2:
-	drm_dev->free_bo(buffer);
-error1:
-	drm_shutdown(buffer);
 error:
+	buffer_free(buffer);
 	return -1;
 }
 
@@ -685,7 +488,6 @@  create_window(struct display *display, int width, int height, int format,
 static void
 destroy_window(struct window *window)
 {
-	struct drm_device* dev;
 	int i;
 
 	if (window->callback)
@@ -695,11 +497,7 @@  destroy_window(struct window *window)
 		if (!window->buffers[i].buffer)
 			continue;
 
-		wl_buffer_destroy(window->buffers[i].buffer);
-		dev = window->buffers[i].dev;
-		dev->free_bo(&window->buffers[i]);
-		close(window->buffers[i].dmabuf_fd);
-		drm_shutdown(&window->buffers[i]);
+		buffer_free(&window->buffers[i]);
 	}
 
 	if (window->xdg_toplevel)
diff --git a/configure.ac b/configure.ac
index 093d6b54..8ff7d5bb 100644
--- a/configure.ac
+++ b/configure.ac
@@ -400,7 +400,7 @@  AC_ARG_ENABLE(simple-dmabuf-drm-client,
                              [do not build the simple dmabuf drm client]),,
               enable_simple_dmabuf_drm_client="auto")
 if ! test "x$enable_simple_dmabuf_drm_client" = "xno"; then
-  PKG_CHECK_MODULES(SIMPLE_DMABUF_DRM_CLIENT, [wayland-client libdrm], [have_simple_dmabuf_libs=yes],
+  PKG_CHECK_MODULES(SIMPLE_DMABUF_DRM_CLIENT, [wayland-client libdrm gbm >= 17.1], [have_simple_dmabuf_libs=yes],
 		    [have_simple_dmabuf_libs=no])
 
   PKG_CHECK_MODULES(LIBDRM_PLATFORM_FREEDRENO, [libdrm_freedreno],

Comments

Guido G√ľnther July 20, 2018, 7:21 p.m.
Hi,
On Thu, Jul 19, 2018 at 04:45:02PM +0200, Emilio Pozuelo Monfort wrote:
> No need to write libdrm driver specific code for each supported
> driver, we can just let GBM call the right one for us now.
> 
> Signed-off-by: Emilio Pozuelo Monfort <emilio.pozuelo@collabora.co.uk>
> ---
> Some improvements from Daniel (thanks!). I also added missing error messages,
> formatting fixes and a logic error in your improvements that caused NV12 to
> abort if create_bo was working fine.
> 
> No changes to XRGB, so that's likely to still fail for you. I suspect
> a bug in mesa's i965 code, I can look at that separately.
> 
> Cheers,
> Emilio
> 
>  clients/simple-dmabuf-drm.c | 444 ++++++++++--------------------------
>  configure.ac                |   2 +-
>  2 files changed, 122 insertions(+), 324 deletions(-)
> 
> diff --git a/clients/simple-dmabuf-drm.c b/clients/simple-dmabuf-drm.c
> index 5bc5323e..5cbd2bb1 100644
> --- a/clients/simple-dmabuf-drm.c
> +++ b/clients/simple-dmabuf-drm.c
> @@ -41,21 +41,11 @@
>  #include <getopt.h>
>  #include <errno.h>
>  
> -#include <xf86drm.h>
> -
> -#ifdef HAVE_LIBDRM_INTEL
> -#include <i915_drm.h>
> -#include <intel_bufmgr.h>
> -#endif
> -#ifdef HAVE_LIBDRM_FREEDRENO
> -#include <freedreno/freedreno_drmif.h>
> -#endif
> -#ifdef HAVE_LIBDRM_ETNAVIV
> -#include <etnaviv_drmif.h>
> -#endif
> +#include <gbm.h>
>  #include <drm_fourcc.h>
>  
>  #include <wayland-client.h>
> +#include "shared/helpers.h"
>  #include "shared/zalloc.h"
>  #include "xdg-shell-unstable-v6-client-protocol.h"
>  #include "fullscreen-shell-unstable-v1-client-protocol.h"
> @@ -65,14 +55,10 @@
>  #define DRM_FORMAT_MOD_LINEAR 0
>  #endif
>  
> -struct buffer;
> -
>  /* Possible options that affect the displayed image */
>  #define OPT_Y_INVERTED 1  /* contents has y axis inverted */
>  #define OPT_IMMEDIATE  2  /* create wl_buffer immediately */
>  
> -#define ALIGN(v, a) ((v + a - 1) & ~(a - 1))
> -
>  struct display {
>  	struct wl_display *display;
>  	struct wl_registry *registry;
> @@ -86,46 +72,24 @@ struct display {
>  	int req_dmabuf_immediate;
>  };
>  
> -struct drm_device {
> -	int fd;
> -	char *name;
> -
> -	int (*alloc_bo)(struct buffer *buf);
> -	void (*free_bo)(struct buffer *buf);
> -	int (*export_bo_to_prime)(struct buffer *buf);
> -	int (*map_bo)(struct buffer *buf);
> -	void (*unmap_bo)(struct buffer *buf);
> -	void (*device_destroy)(struct buffer *buf);
> -};
> -
>  struct buffer {
>  	struct wl_buffer *buffer;
>  	int busy;
>  
> -	struct drm_device *dev;
>  	int drm_fd;
> +	struct gbm_device *dev;
> +	struct gbm_bo *bo;
> +	struct gbm_bo *bo2;
>  
> -#ifdef HAVE_LIBDRM_INTEL
> -	drm_intel_bufmgr *bufmgr;
> -	drm_intel_bo *intel_bo;
> -#endif /* HAVE_LIBDRM_INTEL */
> -#if HAVE_LIBDRM_FREEDRENO
> -	struct fd_device *fd_dev;
> -	struct fd_bo *fd_bo;
> -#endif /* HAVE_LIBDRM_FREEDRENO */
> -#if HAVE_LIBDRM_ETNAVIV
> -	struct etna_device *etna_dev;
> -	struct etna_bo *etna_bo;
> -#endif /* HAVE_LIBDRM_ETNAVIV */
> -
> -	uint32_t gem_handle;
>  	int dmabuf_fd;
> -	uint8_t *mmap;
> +	int dmabuf_fd2;
> +	void *mmap;
> +	void *mmap2;
>  
>  	int width;
>  	int height;
> -	int bpp;
> -	unsigned long stride;
> +	uint32_t stride;
> +	uint32_t stride2;
>  	int format;
>  };
>  
> @@ -161,170 +125,6 @@ static const struct wl_buffer_listener buffer_listener = {
>  	buffer_release
>  };
>  
> -
> -#ifdef HAVE_LIBDRM_INTEL
> -static int
> -intel_alloc_bo(struct buffer *my_buf)
> -{
> -	/* XXX: try different tiling modes for testing FB modifiers. */
> -	uint32_t tiling = I915_TILING_NONE;
> -
> -	assert(my_buf->bufmgr);
> -
> -	my_buf->intel_bo = drm_intel_bo_alloc_tiled(my_buf->bufmgr, "test",
> -						    my_buf->width, my_buf->height,
> -						    (my_buf->bpp / 8), &tiling,
> -						    &my_buf->stride, 0);
> -
> -	printf("buffer allocated w %d, h %d, stride %lu, size %lu\n",
> -	       my_buf->width, my_buf->height, my_buf->stride, my_buf->intel_bo->size);
> -
> -	if (!my_buf->intel_bo)
> -		return 0;
> -
> -	if (tiling != I915_TILING_NONE)
> -		return 0;
> -
> -	return 1;
> -}
> -
> -static void
> -intel_free_bo(struct buffer *my_buf)
> -{
> -	drm_intel_bo_unreference(my_buf->intel_bo);
> -}
> -
> -static int
> -intel_map_bo(struct buffer *my_buf)
> -{
> -	if (drm_intel_gem_bo_map_gtt(my_buf->intel_bo) != 0)
> -		return 0;
> -
> -	my_buf->mmap = my_buf->intel_bo->virtual;
> -
> -	return 1;
> -}
> -
> -static int
> -intel_bo_export_to_prime(struct buffer *buffer)
> -{
> -	return drm_intel_bo_gem_export_to_prime(buffer->intel_bo, &buffer->dmabuf_fd);
> -}
> -
> -static void
> -intel_unmap_bo(struct buffer *my_buf)
> -{
> -	drm_intel_gem_bo_unmap_gtt(my_buf->intel_bo);
> -}
> -
> -static void
> -intel_device_destroy(struct buffer *my_buf)
> -{
> -	drm_intel_bufmgr_destroy(my_buf->bufmgr);
> -}
> -
> -#endif /* HAVE_LIBDRM_INTEL */
> -#ifdef HAVE_LIBDRM_FREEDRENO
> -
> -static int
> -fd_alloc_bo(struct buffer *buf)
> -{
> -	int flags = DRM_FREEDRENO_GEM_CACHE_WCOMBINE;
> -	int size;
> -
> -	buf->fd_dev = fd_device_new(buf->drm_fd);
> -	buf->stride = ALIGN(buf->width, 32) * buf->bpp / 8;
> -	size = buf->stride * buf->height;
> -	buf->fd_dev = fd_device_new(buf->drm_fd);
> -	buf->fd_bo = fd_bo_new(buf->fd_dev, size, flags);
> -
> -	if (!buf->fd_bo)
> -		return 0;
> -	return 1;
> -}
> -
> -static void
> -fd_free_bo(struct buffer *buf)
> -{
> -	fd_bo_del(buf->fd_bo);
> -}
> -
> -static int
> -fd_bo_export_to_prime(struct buffer *buf)
> -{
> -	buf->dmabuf_fd = fd_bo_dmabuf(buf->fd_bo);
> -	return buf->dmabuf_fd < 0;
> -}
> -
> -static int
> -fd_map_bo(struct buffer *buf)
> -{
> -	buf->mmap = fd_bo_map(buf->fd_bo);
> -	return buf->mmap != NULL;
> -}
> -
> -static void
> -fd_unmap_bo(struct buffer *buf)
> -{
> -}
> -
> -static void
> -fd_device_destroy(struct buffer *buf)
> -{
> -	fd_device_del(buf->fd_dev);
> -}
> -#endif /* HAVE_LIBDRM_FREEDRENO */
> -#ifdef HAVE_LIBDRM_ETNAVIV
> -
> -static int
> -etna_alloc_bo(struct buffer *buf)
> -{
> -	int flags = DRM_ETNA_GEM_CACHE_WC;
> -	int size;
> -
> -	buf->stride = ALIGN(buf->width, 32) * buf->bpp / 8;
> -	size = 	buf->stride * buf->height;
> -	buf->etna_dev = etna_device_new(buf->drm_fd);
> -	buf->etna_bo = etna_bo_new(buf->etna_dev, size, flags);
> -
> -	return buf->etna_bo != NULL;
> -}
> -
> -static void
> -etna_free_bo(struct buffer *buf)
> -{
> -	etna_bo_del(buf->etna_bo);
> -}
> -
> -static int
> -etna_bo_export_to_prime(struct buffer *buf)
> -{
> -	buf->dmabuf_fd = etna_bo_dmabuf(buf->etna_bo);
> -	return buf->dmabuf_fd < 0;
> -}
> -
> -static int
> -etna_map_bo(struct buffer *buf)
> -{
> -	buf->mmap = etna_bo_map(buf->etna_bo);
> -	return buf->mmap != NULL;
> -}
> -
> -static void
> -etna_unmap_bo(struct buffer *buf)
> -{
> -	if (munmap(buf->mmap, buf->stride * buf->height) < 0)
> -		fprintf(stderr, "Failed to unmap buffer: %s", strerror(errno));
> -	buf->mmap = NULL;
> -}
> -
> -static void
> -etna_device_destroy(struct buffer *buf)
> -{
> -	etna_device_del(buf->etna_dev);
> -}
> -#endif /* HAVE_LIBDRM_ENTAVIV */
> -
>  static void
>  fill_content(struct buffer *my_buf, uint64_t modifier)
>  {
> @@ -343,19 +143,18 @@ fill_content(struct buffer *my_buf, uint64_t modifier)
>  		}
>  		else if (modifier == DRM_FORMAT_MOD_LINEAR) {
>  			uint8_t *pix8;
> -			/* first plane: Y (2/3 of the buffer)	*/
> -			for (y = 0; y < my_buf->height * 2/3; y++) {
> +			/* first plane: Y */
> +			for (y = 0; y < my_buf->height; y++) {
>  				pix8 = my_buf->mmap + y * my_buf->stride;
>  				for (x = 0; x < my_buf->width; x++)
>  					*pix8++ = x % 256;
>  			}
> -			/* second plane (CbCr) is half the size of Y
> -			   plane (last 1/3 of the buffer) */
> -			for (y = my_buf->height * 2/3; y < my_buf->height; y++) {
> -				pix8 = my_buf->mmap + y * my_buf->stride;
> -				for (x = 0; x < my_buf->width; x+=2) {
> -					*pix8++ = x % 256;
> -					*pix8++ = y % 256;
> +			/* second plane (CbCr) is half the size of Y */
> +			for (y = 0; y < my_buf->height / 2; y++) {
> +				pix8 = my_buf->mmap2 + y * my_buf->stride2;
> +				for (x = 0; x < my_buf->width / 2; x++) {
> +					*pix8++ = (x * 2) % 256;
> +					*pix8++ = (y * 2) % 256;
>  				}
>  			}
>  		}
> @@ -371,69 +170,26 @@ fill_content(struct buffer *my_buf, uint64_t modifier)
>  }
>  
>  static void
> -drm_device_destroy(struct buffer *buf)
> +buffer_free(struct buffer *buf)
>  {
> -	buf->dev->device_destroy(buf);
> -	close(buf->drm_fd);
> -}
> +	if (buf->buffer)
> +		wl_buffer_destroy(buf->buffer);
>  
> -static int
> -drm_device_init(struct buffer *buf)
> -{
> -	struct drm_device *dev = calloc(1, sizeof(struct drm_device));
> +	if (buf->bo)
> +		gbm_bo_destroy(buf->bo);
> +	if (buf->bo2)
> +		gbm_bo_destroy(buf->bo2);
>  
> -	drmVersionPtr version = drmGetVersion(buf->drm_fd);
> +	close(buf->dmabuf_fd);
> +	close(buf->dmabuf_fd2);
>  
> -	dev->fd = buf->drm_fd;
> -	dev->name = strdup(version->name);
> -	if (0) {
> -		/* nothing */
> -	}
> -#ifdef HAVE_LIBDRM_INTEL
> -	else if (!strcmp(dev->name, "i915")) {
> -		buf->bufmgr = drm_intel_bufmgr_gem_init(buf->drm_fd, 32);
> -		if (!buf->bufmgr)
> -			return 0;
> -		dev->alloc_bo = intel_alloc_bo;
> -		dev->free_bo = intel_free_bo;
> -		dev->export_bo_to_prime = intel_bo_export_to_prime;
> -		dev->map_bo = intel_map_bo;
> -		dev->unmap_bo = intel_unmap_bo;
> -		dev->device_destroy = intel_device_destroy;
> -	}
> -#endif
> -#ifdef HAVE_LIBDRM_FREEDRENO
> -	else if (!strcmp(dev->name, "msm")) {
> -		dev->alloc_bo = fd_alloc_bo;
> -		dev->free_bo = fd_free_bo;
> -		dev->export_bo_to_prime = fd_bo_export_to_prime;
> -		dev->map_bo = fd_map_bo;
> -		dev->unmap_bo = fd_unmap_bo;
> -		dev->device_destroy = fd_device_destroy;
> -	}
> -#endif
> -#ifdef HAVE_LIBDRM_ETNAVIV
> -	else if (!strcmp(dev->name, "etnaviv")) {
> -		dev->alloc_bo = etna_alloc_bo;
> -		dev->free_bo = etna_free_bo;
> -		dev->export_bo_to_prime = etna_bo_export_to_prime;
> -		dev->map_bo = etna_map_bo;
> -		dev->unmap_bo = etna_unmap_bo;
> -		dev->device_destroy = etna_device_destroy;
> -	}
> -#endif
> -	else {
> -		fprintf(stderr, "Error: drm device %s unsupported.\n",
> -			dev->name);
> -		free(dev);
> -		return 0;
> -	}
> -	buf->dev = dev;
> -	return 1;
> +	if (buf->dev)
> +		gbm_device_destroy(buf->dev);
> +	close(buf->drm_fd);
>  }
>  
> -static int
> -drm_connect(struct buffer *my_buf)
> +static struct gbm_device *
> +gbm_connect(struct buffer *my_buf)
>  {
>  	/* This won't work with card0 as we need to be authenticated; instead,
>  	 * boot with drm.rnodes=1 and use that. */
> @@ -441,16 +197,9 @@ drm_connect(struct buffer *my_buf)
>  	if (my_buf->drm_fd < 0)
>  		return 0;
>  
> -	return drm_device_init(my_buf);
> +	return my_buf->dev = gbm_create_device(my_buf->drm_fd);
>  }
>  
> -static void
> -drm_shutdown(struct buffer *my_buf)
> -{
> -	drm_device_destroy(my_buf);
> -}
> -
> -
>  static void
>  create_succeeded(void *data,
>  		 struct zwp_linux_buffer_params_v1 *params,
> @@ -482,54 +231,111 @@ static const struct zwp_linux_buffer_params_v1_listener params_listener = {
>  	create_failed
>  };
>  
> +static struct gbm_bo *
> +create_bo(struct buffer *buffer, int format, int subsamp,
> +	  const uint64_t *modifiers, const unsigned int count)
> +{
> +	struct gbm_bo *bo;
> +
> +	if (!modifiers) {
> +		bo = gbm_bo_create(buffer->dev,
> +		                   buffer->width / subsamp, buffer->height / subsamp,
> +				   format,
> +				   0 /* usage */);
> +	} else {
> +		bo = gbm_bo_create_with_modifiers(buffer->dev,
> +						  buffer->width / subsamp,
> +						  buffer->height / subsamp,
> +						  format,
> +						  modifiers,
> +						  count);
> +	}
> +
> +	if (!bo)
> +		fprintf(stderr, "bo_create failed\n");
> +
> +	return bo;
> +}
> +
>  static int
>  create_dmabuf_buffer(struct display *display, struct buffer *buffer,
>  		     int width, int height, int format, uint32_t opts)
>  {
>  	struct zwp_linux_buffer_params_v1 *params;
> -	uint64_t modifier = 0;
> +	uint64_t modifier = 0, modifier2 = 0;
>  	uint32_t flags = 0;
> -	struct drm_device *drm_dev;
>  
> -	if (!drm_connect(buffer)) {
> -		fprintf(stderr, "drm_connect failed\n");
> +	if (!gbm_connect(buffer)) {
> +		fprintf(stderr, "gbm_device_connect failed\n");
>  		goto error;
>  	}
> -	drm_dev = buffer->dev;
>  
>  	buffer->width = width;
> +	buffer->height = height;
> +	buffer->format = format;
>  	switch (format) {
>  	case DRM_FORMAT_NV12:
> -		/* adjust height for allocation of NV12 Y and UV planes */
> -		buffer->height = height * 3 / 2;
> -		buffer->bpp = 8;
>  		modifier = display->nv12_modifier;
>  		break;
> -	default:
> -		buffer->height = height;
> -		buffer->bpp = 32;
>  	}
> -	buffer->format = format;
>  
> -	if (!drm_dev->alloc_bo(buffer)) {
> -		fprintf(stderr, "alloc_bo failed\n");
> -		goto error1;
> +	if (format == DRM_FORMAT_XRGB8888) {
> +		uint64_t modifiers[] = { DRM_FORMAT_MOD_LINEAR };
> +
> +		buffer->bo = create_bo(buffer, format, 1,
> +				       modifiers, ARRAY_LENGTH(modifiers));
> +		if (!buffer->bo) {
> +			fprintf(stderr, "create_bo for XRGB8888 failed\n");
> +			goto error;
> +		}
> +	} else {
> +		uint64_t modifiers[] = { modifier };
> +
> +		buffer->bo = create_bo(buffer, GBM_FORMAT_R8, 1,
> +				       modifiers, ARRAY_LENGTH(modifiers));
> +		buffer->bo2 = create_bo(buffer, GBM_FORMAT_GR88, 2,
> +					modifiers, ARRAY_LENGTH(modifiers));
> +		if (!buffer->bo || !buffer->bo2) {
> +			fprintf(stderr, "create_bo for NV12 failed\n");
> +			goto error;
> +		}
>  	}
>  
> -	if (!drm_dev->map_bo(buffer)) {
> +	buffer->mmap = gbm_bo_map(buffer->bo, 0 /* x */, 0 /* y */,
> +				  width, height,
> +				  GBM_BO_TRANSFER_WRITE,
> +				  &buffer->stride, &buffer->mmap);
> +	if (!buffer->mmap) {
>  		fprintf(stderr, "map_bo failed\n");
> -		goto error2;
> +		goto error;
>  	}
> -	fill_content(buffer, modifier);
> -	drm_dev->unmap_bo(buffer);
>  
> -	if (drm_dev->export_bo_to_prime(buffer) != 0) {
> -		fprintf(stderr, "gem_export_to_prime failed\n");
> -		goto error2;
> +	if (format == DRM_FORMAT_NV12) {
> +		buffer->mmap2 = gbm_bo_map(buffer->bo2, 0 /* x */, 0 /* y */,
> +					   width / 2, height / 2,

I might be wrong here but wouldn't you want half the pixels of the Y
plane here, not a quarter (2x2 subsampling for both U and V)?

> +					   GBM_BO_TRANSFER_WRITE,
> +					   &buffer->stride2, &buffer->mmap2);
> +		if (!buffer->mmap2) {
> +			fprintf(stderr, "map_bo failed\n");
> +			goto error;
> +		}
>  	}
> -	if (buffer->dmabuf_fd < 0) {
> +
> +	modifier = gbm_bo_get_modifier(buffer->bo);
> +	fill_content(buffer, modifier);
> +	gbm_bo_unmap(buffer->bo, buffer->mmap);
> +	if (buffer->bo2)
> +		gbm_bo_unmap(buffer->bo2, buffer->mmap2);
> +
> +	buffer->dmabuf_fd = gbm_bo_get_fd(buffer->bo);
> +	if (buffer->bo2)
> +		buffer->dmabuf_fd2 = gbm_bo_get_fd(buffer->bo2);
> +	else
> +		buffer->dmabuf_fd2 = -1;
> +
> +	if (buffer->dmabuf_fd < 0 || (format == DRM_FORMAT_NV12 && buffer->dmabuf_fd2 < 0)) {
>  		fprintf(stderr, "error: dmabuf_fd < 0\n");
> -		goto error2;
> +		goto error;
>  	}
>  
>  	/* We now have a dmabuf! For format XRGB8888, it should contain
>  	2x2


This code here looks like:

	/* We now have a dmabuf! For format XRGB8888, it should contain 2x2
	 * tiles (i.e. each tile is 256x256) of misc colours, and be mappable,
	 * either as ARGB8888, or XRGB8888. For format NV12, it should contain
	 * the Y and UV components, and needs to be re-adjusted for passing the
	 * correct height to the compositor.
	 */
      	buffer->height = height;

And I think the comment as well as the assignment are outdated now since
the readjustment is not needed anymore (already done at the beginning of
the function).
Cheers,
 -- Guido


> @@ -554,12 +360,12 @@ create_dmabuf_buffer(struct display *display, struct buffer *buffer,
>  	if (format == DRM_FORMAT_NV12) {
>  		/* add the second plane params */
>  		zwp_linux_buffer_params_v1_add(params,
> -					       buffer->dmabuf_fd,
> +					       buffer->dmabuf_fd2,
>  					       1,
> -					       buffer->width * buffer->height,
> -					       buffer->stride,
> -					       modifier >> 32,
> -					       modifier & 0xffffffff);
> +					       0,
> +					       buffer->stride2,
> +					       modifier2 >> 32,
> +					       modifier2 & 0xffffffff);
>  	}
>  	zwp_linux_buffer_params_v1_add_listener(params, &params_listener, buffer);
>  	if (display->req_dmabuf_immediate) {
> @@ -579,11 +385,8 @@ create_dmabuf_buffer(struct display *display, struct buffer *buffer,
>  
>  	return 0;
>  
> -error2:
> -	drm_dev->free_bo(buffer);
> -error1:
> -	drm_shutdown(buffer);
>  error:
> +	buffer_free(buffer);
>  	return -1;
>  }
>  
> @@ -685,7 +488,6 @@ create_window(struct display *display, int width, int height, int format,
>  static void
>  destroy_window(struct window *window)
>  {
> -	struct drm_device* dev;
>  	int i;
>  
>  	if (window->callback)
> @@ -695,11 +497,7 @@ destroy_window(struct window *window)
>  		if (!window->buffers[i].buffer)
>  			continue;
>  
> -		wl_buffer_destroy(window->buffers[i].buffer);
> -		dev = window->buffers[i].dev;
> -		dev->free_bo(&window->buffers[i]);
> -		close(window->buffers[i].dmabuf_fd);
> -		drm_shutdown(&window->buffers[i]);
> +		buffer_free(&window->buffers[i]);
>  	}
>  
>  	if (window->xdg_toplevel)
> diff --git a/configure.ac b/configure.ac
> index 093d6b54..8ff7d5bb 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -400,7 +400,7 @@ AC_ARG_ENABLE(simple-dmabuf-drm-client,
>                               [do not build the simple dmabuf drm client]),,
>                enable_simple_dmabuf_drm_client="auto")
>  if ! test "x$enable_simple_dmabuf_drm_client" = "xno"; then
> -  PKG_CHECK_MODULES(SIMPLE_DMABUF_DRM_CLIENT, [wayland-client libdrm], [have_simple_dmabuf_libs=yes],
> +  PKG_CHECK_MODULES(SIMPLE_DMABUF_DRM_CLIENT, [wayland-client libdrm gbm >= 17.1], [have_simple_dmabuf_libs=yes],
>  		    [have_simple_dmabuf_libs=no])
>  
>    PKG_CHECK_MODULES(LIBDRM_PLATFORM_FREEDRENO, [libdrm_freedreno],
> -- 
> 2.18.0
>