[PATCH v4 3/3] compositor-drm: allow to be a wl_dmabuf server
Thomas Hellstrom
thellstrom at vmware.com
Tue Jan 7 10:37:11 PST 2014
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.
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
More information about the wayland-devel
mailing list