[PATCH v4 3/3] compositor-drm: allow to be a wl_dmabuf server
Daniel Vetter
daniel at ffwll.ch
Wed Jan 8 01:23:55 PST 2014
On Tue, Jan 07, 2014 at 07:37:11PM +0100, Thomas Hellstrom wrote:
>
> I think dma-buf mmap() used without care like this should be avoided at
> all cost,
> because it will make people happily start using it without thinking
> about non-coherent
> architectures where mmap would be painfully inefficient: This is because
> when the accelerator
> uses the dma-buf it has no idea what part of it was dirtied and may need
> to flush a much larger
> region than was actually touched by the CPU. Think of pixman writing a
> single char to a 1280x1024
> canvas, which is then used by the compositor.
> A smart driver non-coherent would need to flush all pages touched. A
> not-so-smart driver would
> need to flush the whole buffer. For a single character.
>
> Also even if a coherent driver implements mmap(),
> the underlying dma-buf may reside in write-combined memory and using it
> for compositing with
> pixman may actually be much slower than shmem, even with the extra shmem
> copy.
>
> Conclusion: Avoid using dma-buf mmap() outside of drivers that know
> exactly what they are
> doing, and avoid it at all cost.
>
> In particular, don't build it into generic code like this.
Fully agreed. Furthermore there's not even a generic way to allocate
dma-bufs (see my other mail) so without a real userspace gfx driver
compononent on each side (compositor+client), which then also might need
to share other metadata about a buffer I don't really see how this is
useful. What's your intended use-case here?
-Daniel
>
> Thanks,
> Thomas
>
>
> On 01/07/2014 06:41 PM, benjamin.gaignard at linaro.org wrote:
> > From: Benjamin Gaignard <benjamin.gaignard at linaro.org>
> >
> > Make drm compositor aware of the wl_dmabuf protocol if pixman is used.
> > Add functions to have a wl_dmabuf server inside drm compositor.
> > Change pixman to let it know how use a wl_dmabuf buffer.
> >
> > Signed-off-by: Benjamin Gaignard <benjamin.gaignard at linaro.org>
> > ---
> > src/compositor-drm.c | 83 ++++++++++++++++++++++++++++++++++++++++---
> > src/compositor.c | 4 ++-
> > src/compositor.h | 2 ++
> > src/pixman-renderer.c | 93 ++++++++++++++++++++++++++++++++++++-------------
> > 4 files changed, 152 insertions(+), 30 deletions(-)
> >
> > diff --git a/src/compositor-drm.c b/src/compositor-drm.c
> > index 4f015d1..669ee45 100644
> > --- a/src/compositor-drm.c
> > +++ b/src/compositor-drm.c
> > @@ -49,6 +49,8 @@
> > #include "udev-seat.h"
> > #include "launcher-util.h"
> > #include "vaapi-recorder.h"
> > +#include "wayland-dmabuf.h"
> > +#include "wayland-dmabuf-server-protocol.h"
> >
> > #ifndef DRM_CAP_TIMESTAMP_MONOTONIC
> > #define DRM_CAP_TIMESTAMP_MONOTONIC 0x6
> > @@ -826,7 +828,8 @@ drm_output_prepare_overlay_surface(struct weston_output *output_base,
> > if (es->alpha != 1.0f)
> > return NULL;
> >
> > - if (wl_shm_buffer_get(es->buffer_ref.buffer->resource))
> > + if (wl_shm_buffer_get(es->buffer_ref.buffer->resource)
> > + || wl_dmabuf_buffer_get(es->buffer_ref.buffer->resource))
> > return NULL;
> >
> > if (!drm_surface_transform_supported(es))
> > @@ -951,7 +954,8 @@ drm_output_prepare_cursor_surface(struct weston_output *output_base,
> > if (c->cursors_are_broken)
> > return NULL;
> > if (es->buffer_ref.buffer == NULL ||
> > - !wl_shm_buffer_get(es->buffer_ref.buffer->resource) ||
> > + (!wl_shm_buffer_get(es->buffer_ref.buffer->resource) &&
> > + !wl_dmabuf_buffer_get(es->buffer_ref.buffer->resource))||
> > es->geometry.width > 64 || es->geometry.height > 64)
> > return NULL;
> >
> > @@ -985,12 +989,25 @@ drm_output_set_cursor(struct drm_output *output)
> > output->current_cursor ^= 1;
> > bo = output->cursor_bo[output->current_cursor];
> > memset(buf, 0, sizeof buf);
> > - stride = wl_shm_buffer_get_stride(es->buffer_ref.buffer->shm_buffer);
> > - s = wl_shm_buffer_get_data(es->buffer_ref.buffer->shm_buffer);
> > + if (wl_shm_buffer_get(es->buffer_ref.buffer->resource)) {
> > + stride = wl_shm_buffer_get_stride(es->buffer_ref.buffer->shm_buffer);
> > + s = wl_shm_buffer_get_data(es->buffer_ref.buffer->shm_buffer);
> > + }
> > + if (wl_dmabuf_buffer_get(es->buffer_ref.buffer->resource)) {
> > + stride = wl_dmabuf_buffer_get_stride(es->buffer_ref.buffer->dmabuf_buffer);
> > + s = wl_dmabuf_buffer_get_data(es->buffer_ref.buffer->dmabuf_buffer);
> > + if (!s) {
> > + weston_log("failed to get data from dmabuf buffer");
> > + return;
> > + }
> > + }
> > for (i = 0; i < es->geometry.height; i++)
> > memcpy(buf + i * 64, s + i * stride,
> > es->geometry.width * 4);
> >
> > + if (wl_dmabuf_buffer_get(es->buffer_ref.buffer->resource))
> > + wl_dmabuf_buffer_put_data(es->buffer_ref.buffer->dmabuf_buffer);
> > +
> > if (gbm_bo_write(bo, buf, sizeof buf) < 0)
> > weston_log("failed update cursor: %m\n");
> >
> > @@ -1044,7 +1061,8 @@ drm_assign_planes(struct weston_output *output)
> > * non-shm, or small enough to be a cursor
> > */
> > if ((es->buffer_ref.buffer &&
> > - !wl_shm_buffer_get(es->buffer_ref.buffer->resource)) ||
> > + (!wl_shm_buffer_get(es->buffer_ref.buffer->resource))
> > + && !wl_dmabuf_buffer_get(es->buffer_ref.buffer->resource)) ||
> > (es->geometry.width <= 64 && es->geometry.height <= 64))
> > es->keep_buffer = 1;
> > else
> > @@ -1268,6 +1286,57 @@ init_drm(struct drm_compositor *ec, struct udev_device *device)
> > return 0;
> > }
> >
> > +static void dmabuf_send_supported_formats(void *user_data, struct wl_resource *resource)
> > +{
> > + struct drm_compositor *ec = user_data;
> > + drmModePlaneRes *plane_resources;
> > + unsigned int i, j;
> > +
> > + weston_log("send supported formats\n");
> > +
> > + plane_resources = drmModeGetPlaneResources(ec->drm.fd);
> > + if (!plane_resources)
> > + return;
> > +
> > + for (i = 0; i < plane_resources->count_planes; i++) {
> > + drmModePlane *ovr = drmModeGetPlane(ec->drm.fd, plane_resources->planes[i]);
> > +
> > + for (j = 0 ; j < ovr->count_formats; j++) {
> > + weston_log("server support format %4.4s\n", (char *)&ovr->formats[j]);
> > + wl_dmabuf_send_format(resource, ovr->formats[j]);
> > + }
> > + drmModeFreePlane(ovr);
> > + }
> > +
> > + drmModeFreePlaneResources(plane_resources);
> > +}
> > +
> > +static void dmabuf_send_device_name(void *user_data, struct wl_resource *resource)
> > +{
> > + struct drm_compositor *ec = user_data;
> > + weston_log("send device name %s\n", ec->drm.filename);
> > +
> > + wl_dmabuf_send_device(resource, ec->drm.filename);
> > +}
> > +
> > +static void dmabuf_send_server_info(void *user_data, struct wl_resource *resource)
> > +{
> > + dmabuf_send_supported_formats(user_data, resource);
> > + dmabuf_send_device_name(user_data, resource);
> > +}
> > +
> > +static struct wl_dmabuf_callbacks wl_dmabuf_callbacks = {
> > + dmabuf_send_server_info
> > +};
> > +
> > +static int
> > +init_dmabuf_protocol(struct drm_compositor *ec)
> > +{
> > + wl_dmabuf_init(ec->base.wl_display,
> > + &wl_dmabuf_callbacks, ec, 0);
> > + return 0;
> > +}
> > +
> > static int
> > init_egl(struct drm_compositor *ec)
> > {
> > @@ -1955,6 +2024,10 @@ create_output_for_connector(struct drm_compositor *ec,
> > weston_log("Failed to init output pixman state\n");
> > goto err_output;
> > }
> > + if (init_dmabuf_protocol(ec) < 0) {
> > + weston_log("Failed to init wl_dmabuf server\n");
> > + goto err_output;
> > + }
> > } else if (drm_output_init_egl(output, ec) < 0) {
> > weston_log("Failed to init output gl state\n");
> > goto err_output;
> > diff --git a/src/compositor.c b/src/compositor.c
> > index 4cb44e5..037b695 100644
> > --- a/src/compositor.c
> > +++ b/src/compositor.c
> > @@ -1257,7 +1257,9 @@ surface_accumulate_damage(struct weston_surface *surface,
> > pixman_region32_t *opaque)
> > {
> > if (surface->buffer_ref.buffer &&
> > - wl_shm_buffer_get(surface->buffer_ref.buffer->resource))
> > + (wl_shm_buffer_get(surface->buffer_ref.buffer->resource)
> > + || wl_dmabuf_buffer_get(surface->buffer_ref.buffer->resource))
> > + )
> > surface->compositor->renderer->flush_damage(surface);
> >
> > if (surface->transform.enabled) {
> > diff --git a/src/compositor.h b/src/compositor.h
> > index 0dcb604..537a9b7 100644
> > --- a/src/compositor.h
> > +++ b/src/compositor.h
> > @@ -33,6 +33,7 @@ extern "C" {
> >
> > #define WL_HIDE_DEPRECATED
> > #include <wayland-server.h>
> > +#include <wayland-dmabuf.h>
> >
> > #include "version.h"
> > #include "matrix.h"
> > @@ -626,6 +627,7 @@ struct weston_buffer {
> >
> > union {
> > struct wl_shm_buffer *shm_buffer;
> > + struct wl_dmabuf_buffer *dmabuf_buffer;
> > void *legacy_buffer;
> > };
> > int32_t width, height;
> > diff --git a/src/pixman-renderer.c b/src/pixman-renderer.c
> > index 987c539..0a88c58 100644
> > --- a/src/pixman-renderer.c
> > +++ b/src/pixman-renderer.c
> > @@ -528,11 +528,19 @@ pixman_renderer_flush_damage(struct weston_surface *surface)
> > /* No-op for pixman renderer */
> > }
> >
> > +static void wl_dmabuf_pixman_destroy (pixman_image_t *image, void *data)
> > +{
> > + struct wl_dmabuf_buffer *dmabuf_buffer = data;
> > + wl_dmabuf_buffer_put_data(dmabuf_buffer);
> > +}
> > +
> > static void
> > pixman_renderer_attach(struct weston_surface *es, struct weston_buffer *buffer)
> > {
> > struct pixman_surface_state *ps = get_surface_state(es);
> > struct wl_shm_buffer *shm_buffer;
> > + struct wl_dmabuf_buffer *dmabuf_buffer;
> > +
> > pixman_format_code_t pixman_format;
> >
> > weston_buffer_reference(&ps->buffer_ref, buffer);
> > @@ -544,40 +552,77 @@ pixman_renderer_attach(struct weston_surface *es, struct weston_buffer *buffer)
> >
> > if (!buffer)
> > return;
> > -
> > +
> > shm_buffer = wl_shm_buffer_get(buffer->resource);
> > + dmabuf_buffer = wl_dmabuf_buffer_get(buffer->resource);
> >
> > - if (! shm_buffer) {
> > - weston_log("Pixman renderer supports only SHM buffers\n");
> > + if (!shm_buffer && !dmabuf_buffer) {
> > + weston_log("Pixman renderer supports only SHM and DMABUF buffers\n");
> > weston_buffer_reference(&ps->buffer_ref, NULL);
> > return;
> > }
> >
> > - switch (wl_shm_buffer_get_format(shm_buffer)) {
> > - case WL_SHM_FORMAT_XRGB8888:
> > - pixman_format = PIXMAN_x8r8g8b8;
> > - break;
> > - case WL_SHM_FORMAT_ARGB8888:
> > - pixman_format = PIXMAN_a8r8g8b8;
> > - break;
> > - case WL_SHM_FORMAT_RGB565:
> > - pixman_format = PIXMAN_r5g6b5;
> > + if (shm_buffer) {
> > + switch (wl_shm_buffer_get_format(shm_buffer)) {
> > + case WL_SHM_FORMAT_XRGB8888:
> > + pixman_format = PIXMAN_x8r8g8b8;
> > + break;
> > + case WL_SHM_FORMAT_ARGB8888:
> > + pixman_format = PIXMAN_a8r8g8b8;
> > + break;
> > + case WL_SHM_FORMAT_RGB565:
> > + pixman_format = PIXMAN_r5g6b5;
> > + break;
> > + default:
> > + weston_log("Unsupported SHM buffer format\n");
> > + weston_buffer_reference(&ps->buffer_ref, NULL);
> > + return;
> > break;
> > - default:
> > - weston_log("Unsupported SHM buffer format\n");
> > - weston_buffer_reference(&ps->buffer_ref, NULL);
> > - return;
> > - break;
> > + }
> > +
> > + buffer->shm_buffer = shm_buffer;
> > + buffer->width = wl_shm_buffer_get_width(shm_buffer);
> > + buffer->height = wl_shm_buffer_get_height(shm_buffer);
> > +
> > + ps->image = pixman_image_create_bits(pixman_format,
> > + buffer->width, buffer->height,
> > + wl_shm_buffer_get_data(shm_buffer),
> > + wl_shm_buffer_get_stride(shm_buffer));
> > }
> >
> > - buffer->shm_buffer = shm_buffer;
> > - buffer->width = wl_shm_buffer_get_width(shm_buffer);
> > - buffer->height = wl_shm_buffer_get_height(shm_buffer);
> > + if (dmabuf_buffer) {
> > + void *data;
> > + switch (wl_dmabuf_buffer_get_format(dmabuf_buffer)) {
> > + case WL_DMABUF_FORMAT_XRGB8888:
> > + pixman_format = PIXMAN_x8r8g8b8;
> > + break;
> > + case WL_DMABUF_FORMAT_ARGB8888:
> > + pixman_format = PIXMAN_a8r8g8b8;
> > + break;
> > + case WL_DMABUF_FORMAT_RGB565:
> > + pixman_format = PIXMAN_r5g6b5;
> > + break;
> > + default:
> > + weston_log("Unsupported DMABUF buffer format\n");
> > + weston_buffer_reference(&ps->buffer_ref, NULL);
> > + return;
> > + break;
> > + }
> >
> > - ps->image = pixman_image_create_bits(pixman_format,
> > - buffer->width, buffer->height,
> > - wl_shm_buffer_get_data(shm_buffer),
> > - wl_shm_buffer_get_stride(shm_buffer));
> > + buffer->dmabuf_buffer = dmabuf_buffer;
> > + buffer->width = wl_dmabuf_buffer_get_width(dmabuf_buffer);
> > + buffer->height = wl_dmabuf_buffer_get_height(dmabuf_buffer);
> > +
> > + data = wl_dmabuf_buffer_get_data(dmabuf_buffer);
> > + if (data) {
> > + ps->image = pixman_image_create_bits(pixman_format,
> > + buffer->width, buffer->height, data,
> > + wl_dmabuf_buffer_get_stride(dmabuf_buffer));
> > +
> > + pixman_image_set_destroy_function(ps->image, wl_dmabuf_pixman_destroy, dmabuf_buffer);
> > + } else
> > + weston_log("failed to get data from dmabuf buffer");
> > + }
> > }
> >
> > static int
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
More information about the wayland-devel
mailing list