Adding more people to CC. On Tue, 10 Jul 2018 17:53:15 +0200 Emilio Pozuelo Monfort <[email protected]> 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 <[email protected]> > --- > > Hi, > > This simplifies the code a lot, using gbm_bo as Emil suggested. Some problems > I still see: > > - NV12 doesn't work, it seems that > backends/dri/gbm_dri.c:gbm_format_to_dri_format() > doesn't support it. I think NV12 and other less common formats, should someone add them in this program, should not be lost. That may be part of the reason GBM wasn't used: the need to test YUV formats, and non-GPU-renderable formats in general. > > - We are still linking to libdrm, that's just to get the DRM_FORMAT_* > definitions. > I suppose we could switch those to GBM_FORMAT_* instead. We have precedent in the build system of using libdrm only for headers but not link to it, specifically to get the pixel format codes. Just leave out the $(LIBDRM_LIBS) or such. > > - Not sure if I need to pass any flags to gbm_bo_create or gbm_bo_map. > > This works fine for me with XRGB8888. > > Thoughts? Comments? > > Thanks, > Emilio Thanks, pq > > clients/simple-dmabuf-drm.c | 320 +++--------------------------------- > configure.ac | 2 +- > 2 files changed, 27 insertions(+), 295 deletions(-) > > diff --git a/clients/simple-dmabuf-drm.c b/clients/simple-dmabuf-drm.c > index fcab30e5..cdee27b3 100644 > --- a/clients/simple-dmabuf-drm.c > +++ b/clients/simple-dmabuf-drm.c > @@ -41,18 +41,7 @@ > #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> > @@ -66,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; > @@ -88,46 +73,22 @@ struct display { > int req_dmabuf_modifiers; > }; > > -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; > - > -#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 */ > + struct gbm_device *dev; > + struct gbm_bo *bo; > > uint32_t gem_handle; > int dmabuf_fd; > - uint8_t *mmap; > + void *mmap; > > int width; > int height; > int bpp; > - unsigned long stride; > + uint32_t stride; > int format; > }; > > @@ -163,170 +124,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) > { > @@ -373,83 +170,22 @@ fill_content(struct buffer *my_buf, uint64_t modifier) > } > > static void > -drm_device_destroy(struct buffer *buf) > +gbm_shutdown(struct buffer *buf) > { > - buf->dev->device_destroy(buf); > + gbm_device_destroy(buf->dev); > close(buf->drm_fd); > } > > -static int > -drm_device_init(struct buffer *buf) > -{ > - struct drm_device *dev = calloc(1, sizeof(struct drm_device)); > - > - drmVersionPtr version = drmGetVersion(buf->drm_fd); > - > - 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; > -} > - > -static int > -drm_connect(struct buffer *my_buf) > +static struct gbm_device * > +gbm_connect(struct buffer *buffer) > { > /* This won't work with card0 as we need to be authenticated; instead, > * boot with drm.rnodes=1 and use that. */ > - my_buf->drm_fd = open("/dev/dri/renderD128", O_RDWR); > - if (my_buf->drm_fd < 0) > + buffer->drm_fd = open("/dev/dri/renderD128", O_RDWR); > + if (buffer->drm_fd < 0) > return 0; > > - return drm_device_init(my_buf); > -} > - > -static void > -drm_shutdown(struct buffer *my_buf) > -{ > - drm_device_destroy(my_buf); > + return gbm_create_device(buffer->drm_fd); > } > > > @@ -491,44 +227,42 @@ create_dmabuf_buffer(struct display *display, struct > buffer *buffer, > struct zwp_linux_buffer_params_v1 *params; > uint64_t modifier = 0; > uint32_t flags = 0; > - struct drm_device *drm_dev; > > - if (!drm_connect(buffer)) { > - fprintf(stderr, "drm_connect failed\n"); > + if ((buffer->dev = gbm_connect(buffer)) == NULL) { > + fprintf(stderr, "gbm_device_connect failed\n"); > goto error; > } > - drm_dev = buffer->dev; > > buffer->width = width; > switch (format) { > case DRM_FORMAT_NV12: > + format = GBM_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: > + format = GBM_FORMAT_XRGB8888; > buffer->height = height; > buffer->bpp = 32; > } > buffer->format = format; > > - if (!drm_dev->alloc_bo(buffer)) { > - fprintf(stderr, "alloc_bo failed\n"); > + if ((buffer->bo = gbm_bo_create(buffer->dev, width, height, format, 0 > /* usage */)) == NULL) { > + fprintf(stderr, "bo_create failed\n"); > goto error1; > } > > - if (!drm_dev->map_bo(buffer)) { > + if ((buffer->mmap = gbm_bo_map(buffer->bo, 0 /* x */, 0 /* y */, width, > height, > + 0 /* flags */, &buffer->stride, > &buffer->mmap)) == NULL) { > fprintf(stderr, "map_bo failed\n"); > goto error2; > } > fill_content(buffer, modifier); > - drm_dev->unmap_bo(buffer); > + gbm_bo_unmap(buffer->bo, buffer->mmap); > > - if (drm_dev->export_bo_to_prime(buffer) != 0) { > - fprintf(stderr, "gem_export_to_prime failed\n"); > - goto error2; > - } > + buffer->dmabuf_fd = gbm_bo_get_fd(buffer->bo); > if (buffer->dmabuf_fd < 0) { > fprintf(stderr, "error: dmabuf_fd < 0\n"); > goto error2; > @@ -582,9 +316,9 @@ create_dmabuf_buffer(struct display *display, struct > buffer *buffer, > return 0; > > error2: > - drm_dev->free_bo(buffer); > + gbm_bo_destroy(buffer->bo); > error1: > - drm_shutdown(buffer); > + gbm_shutdown(buffer); > error: > return -1; > } > @@ -687,7 +421,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) > @@ -698,10 +431,9 @@ destroy_window(struct window *window) > continue; > > wl_buffer_destroy(window->buffers[i].buffer); > - dev = window->buffers[i].dev; > - dev->free_bo(&window->buffers[i]); > + gbm_bo_destroy(window->buffers[i].bo); > close(window->buffers[i].dmabuf_fd); > - drm_shutdown(&window->buffers[i]); > + gbm_shutdown(&window->buffers[i]); > } > > if (window->xdg_toplevel) > diff --git a/configure.ac b/configure.ac > index 39168e20..2169a8d3 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -391,7 +391,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 egl], > [have_simple_dmabuf_libs=yes], > + PKG_CHECK_MODULES(SIMPLE_DMABUF_DRM_CLIENT, [wayland-client libdrm egl > gbm], [have_simple_dmabuf_libs=yes], > [have_simple_dmabuf_libs=no]) > > PKG_CHECK_MODULES(LIBDRM_PLATFORM_FREEDRENO, [libdrm_freedreno],
pgpDTQ6Vsr4r8.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
