[PATCH 2/2] compositor-drm: add sprite support
Kristian Høgsberg
krh at bitplanet.net
Fri Jan 27 14:24:04 PST 2012
On Fri, Jan 27, 2012 at 4:41 PM, Jesse Barnes <jbarnes at virtuousgeek.org> wrote:
> Add support for assigning surfaces to overlay sprites using the new
> assign_planes hook.
Yeah, looks good overall, comments below.
> NOTE: this patch fails to handle fbs correctly; a live fb/surface pair
> may be removed while still active!
> ---
> configure.ac | 2 +-
> src/compositor-drm.c | 413 +++++++++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 413 insertions(+), 2 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index 3522bee..3ab6f50 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -73,7 +73,7 @@ AC_ARG_ENABLE(drm-compositor, [ --enable-drm-compositor],,
> AM_CONDITIONAL(ENABLE_DRM_COMPOSITOR, test x$enable_drm_compositor == xyes)
> if test x$enable_drm_compositor == xyes; then
> AC_DEFINE([BUILD_DRM_COMPOSITOR], [1], [Build the DRM compositor])
> - PKG_CHECK_MODULES(DRM_COMPOSITOR, [libudev >= 136 libdrm >= 2.4.23 gbm])
> + PKG_CHECK_MODULES(DRM_COMPOSITOR, [libudev >= 136 libdrm >= 2.4.30 gbm])
> fi
>
>
> diff --git a/src/compositor-drm.c b/src/compositor-drm.c
> index 0c1add5..eedf5b7 100644
> --- a/src/compositor-drm.c
> +++ b/src/compositor-drm.c
> @@ -23,6 +23,7 @@
>
> #define _GNU_SOURCE
>
> +#include <errno.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> @@ -31,6 +32,7 @@
>
> #include <xf86drm.h>
> #include <xf86drmMode.h>
> +#include <drm_fourcc.h>
>
> #include <gbm.h>
>
> @@ -50,10 +52,14 @@ struct drm_compositor {
> int fd;
> } drm;
> struct gbm_device *gbm;
> + uint32_t *crtcs;
> + int num_crtcs;
> uint32_t crtc_allocator;
> uint32_t connector_allocator;
> struct tty *tty;
>
> + struct wl_list sprite_list;
> +
> uint32_t prev_state;
> };
>
> @@ -82,6 +88,72 @@ struct drm_output {
> struct wl_listener pending_scanout_buffer_destroy_listener;
> };
>
> +/*
> + * An output has a primary display plane plus zero or more sprites for
> + * blending display contents.
> + */
> +struct drm_sprite {
> + struct wl_list link;
> +
> + uint32_t fb_id;
> + uint32_t pending_fb_id;
> + struct weston_surface *surface;
> + struct weston_surface *pending_surface;
> +
> + struct wl_listener destroy_listener;
> + struct wl_listener pending_destroy_listener;
> +
> + uint32_t possible_crtcs;
> + uint32_t plane_id;
> + uint32_t count_formats;
> +
> + int32_t src_x, src_y;
> + uint32_t src_w, src_h;
> + uint32_t dest_x, dest_y;
> + uint32_t dest_w, dest_h;
> +
> + uint32_t formats[];
> +};
> +
> +static int
> +surface_is_primary(struct weston_compositor *ec, struct weston_surface *es)
> +{
> + struct weston_surface *primary;
> +
> + primary = container_of(ec->surface_list.next, struct weston_surface,
> + link);
> + if (es == primary)
> + return -1;
> + return 0;
> +}
A scanout surface isn't necessarily the top of the stack. Each output
could have a surface and only one can be first in the list. I think
we may need backend privates where we can store state such as "this
surface is the scanout for that output/sprite" etc.
> +
> +static int
> +surface_is_cursor(struct weston_compositor *ec, struct weston_surface *es)
> +{
> + if (!es->buffer)
> + return -1;
> + return 0;
> +}
Not sure what this check does, cursors are surfaces just like anything
else. And we should be able to use a sprite as a hw cursor in case we
have multiple pointers.
> +static int
> +drm_sprite_crtc_supported(struct weston_output *output_base, uint32_t supported)
> +{
> + struct weston_compositor *ec = output_base->compositor;
> + struct drm_compositor *c =(struct drm_compositor *) ec;
> + struct drm_output *output = (struct drm_output *) output_base;
> + int crtc;
> +
> + for (crtc = 0; crtc < c->num_crtcs; crtc++) {
> + if (c->crtcs[crtc] != output->crtc_id)
> + continue;
> +
> + if (supported & (1 << crtc))
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> static int
> drm_output_prepare_scanout_surface(struct drm_output *output)
> {
> @@ -147,7 +219,10 @@ drm_output_repaint(struct weston_output *output_base)
> struct drm_compositor *compositor =
> (struct drm_compositor *) output->base.compositor;
> struct weston_surface *surface;
> + struct drm_sprite *s;
> uint32_t fb_id = 0;
> + int ret = 0;
> + int queued_sprite = 0;
>
> glFramebufferRenderbuffer(GL_FRAMEBUFFER,
> GL_COLOR_ATTACHMENT0,
> @@ -179,10 +254,81 @@ drm_output_repaint(struct weston_output *output_base)
> return;
> }
>
> + /*
> + * Now, update all the sprite surfaces
> + */
> + wl_list_for_each(s, &compositor->sprite_list, link) {
> + uint32_t flags = 0;
> +
> + if (!drm_sprite_crtc_supported(output_base, s->possible_crtcs))
> + continue;
> +
> + ret = drmModeSetPlane(compositor->drm.fd, s->plane_id,
> + output->crtc_id, s->pending_fb_id, flags,
> + s->dest_x, s->dest_y,
> + s->dest_w, s->dest_h,
> + s->src_x, s->src_y,
> + s->src_w, s->src_h);
> + if (ret)
> + fprintf(stderr, "setplane failed: %d: %s\n",
> + ret, strerror(errno));
> + queued_sprite = 1;
> + }
The plane flip only happens on the next vblank, right?
> +
> + /*
> + * Request a vblank event for the next seqno so we can free up
> + * the sprite surfaces.
> + */
> + if (queued_sprite) {
> + drmVBlank vbl = {
> + .request.type = DRM_VBLANK_RELATIVE | DRM_VBLANK_EVENT,
> + .request.sequence = 1,
> + };
> +
> + vbl.request.signal = (unsigned long)output;
> + ret = drmWaitVBlank(compositor->drm.fd, &vbl);
> + if (ret) {
> + fprintf(stderr, "vblank event request failed: %d: %s\n",
> + ret, strerror(errno));
> + }
> + }
> +
> return;
> }
>
> static void
> +vblank_handler(int fd, unsigned int frame, unsigned int sec, unsigned int usec,
> + void *data)
> +{
> + struct drm_output *output = (struct drm_output *) data;
> + struct drm_compositor *c =
> + (struct drm_compositor *) output->base.compositor;
> + struct drm_sprite *s;
> +
> + /* Free the FBs for the sprites */
> + wl_list_for_each(s, &c->sprite_list, link) {
> + if (s->surface) {
> + weston_buffer_post_release(s->surface->buffer);
> + wl_list_remove(&s->destroy_listener.link);
> + s->surface = NULL;
> + /* FIXME: don't destroy active fb */
> + drmModeRmFB(c->drm.fd, s->fb_id);
> + s->fb_id = 0;
> + }
> +
> + if (s->pending_surface) {
> + wl_list_remove(&s->pending_destroy_listener.link);
> + wl_list_insert(s->pending_surface->buffer->resource.destroy_listener_list.prev,
> + &s->destroy_listener.link);
> + s->surface = s->pending_surface;
> + s->pending_surface = NULL;
> + s->fb_id = s->pending_fb_id;
> + s->pending_fb_id = 0;
> + }
> + }
> +}
> +
> +static void
> page_flip_handler(int fd, unsigned int frame,
> unsigned int sec, unsigned int usec, void *data)
> {
> @@ -214,6 +360,185 @@ page_flip_handler(int fd, unsigned int frame,
> }
>
> static int
> +drm_surface_format_supported(struct weston_surface *es)
> +{
> + int ret = 0;
> + enum wl_shm_format format;
> +
> + format = wl_shm_buffer_get_format(es->buffer);
We can't do this, it only work if we know it's a shm buffer, and they
dont work with kms. We have create the gbm bo from the EGLImage and
then ask gbm for the format. And we dont have that gbm getter...I
wonder if we can add it for 8.0...
> + switch (format) {
> + case WL_SHM_FORMAT_ARGB8888:
> + default:
> +#if 1 //def DEBUG_PLANES
> + /*
> + * Enable this code to test non-overlapping surfaces even
> + * if they need alpha blending.
> + */
> + if (es->overlapped)
> + ret = 0;
> + else
> + ret = 1;
> +#endif
> + break;
> + case WL_SHM_FORMAT_XRGB8888:
> + ret = 1;
> + break;
> + }
> +
> + return ret;
> +}
> +
> +static void
> +drm_disable_unused_sprites(struct weston_output *output_base)
> +{
> + struct weston_compositor *ec = output_base->compositor;
> + struct drm_compositor *c =(struct drm_compositor *) ec;
> + struct drm_output *output = (struct drm_output *) output_base;
> + struct drm_sprite *s;
> + int ret;
> +
> + wl_list_for_each(s, &c->sprite_list, link) {
> + if (s->pending_fb_id)
> + continue;
Doesn't this turn off sprites that aren't pending an update but just
displaying a static buffer? Or I guess you set the sprite in each
frame so even for static sprites, there's always a pending_fb_id?
> + ret = drmModeSetPlane(c->drm.fd, s->plane_id,
> + output->crtc_id, 0, 0,
> + 0, 0, 0, 0, 0, 0, 0, 0);
> + if (ret)
> + fprintf(stderr,
> + "failed to disable plane: %d: %s\n",
> + ret, strerror(errno));
> + drmModeRmFB(c->drm.fd, s->fb_id);
> + s->surface = NULL;
> + s->pending_surface = NULL;
> + s->fb_id = 0;
> + s->pending_fb_id = 0;
> + }
> +}
> +
> +/*
> + * Must damage the surface if it was on a sprite before
> + */
> +static int
> +drm_output_prepare_overlay_surface(struct weston_output *output_base,
> + struct weston_surface *es)
> +{
> + struct weston_compositor *ec = output_base->compositor;
> + struct drm_compositor *c =(struct drm_compositor *) ec;
> + struct drm_sprite *s;
> + int found = 0;
> + EGLint handle, stride;
> + struct gbm_bo *bo;
> + uint32_t fb_id = 0;
> + uint32_t handles[4], pitches[4], offsets[4];
> + int ret = 0;
> +
> + if (surface_is_primary(ec, es))
> + return -1;
> +
> + if (surface_is_cursor(ec, es))
> + return -1;
> +
> + if (!drm_surface_format_supported(es))
> + return -1;
> +
> + wl_list_for_each(s, &c->sprite_list, link) {
> + if (!drm_sprite_crtc_supported(output_base, s->possible_crtcs))
> + continue;
> +
> + if (!s->pending_fb_id) {
> + found = 1;
> + break;
> + }
> + }
> +
> + /* No sprites available */
> + if (!found)
> + return -1;
> +
> + bo = gbm_bo_create_from_egl_image(c->gbm, c->base.display, es->image,
> + es->width, es->height,
> + GBM_BO_USE_SCANOUT);
> + handle = gbm_bo_get_handle(bo).s32;
> + stride = gbm_bo_get_pitch(bo);
> +
> + gbm_bo_destroy(bo);
> +
> + if (!handle)
> + return -1;
> +
> + handles[0] = handle;
> + pitches[0] = stride;
> + offsets[0] = 0;
> +
> + ret = drmModeAddFB2(c->drm.fd, es->width, es->height,
> + DRM_FORMAT_XRGB8888, handles, pitches, offsets,
> + &fb_id, 0);
> + if (ret) {
> + fprintf(stderr, "addfb2 failed: %d\n", ret);
> + return -1;
> + }
> +
> + if (s->surface && s->surface != es) {
> + struct weston_surface *old_surf = s->surface;
> + pixman_region32_init_rect(&old_surf->damage,
> + old_surf->x, old_surf->y,
> + old_surf->width, old_surf->height);
> + }
> +
> + s->pending_fb_id = fb_id;
> + s->pending_surface = es;
> + es->buffer->busy_count++;
> +
> + s->dest_x = es->x;
> + s->dest_y = es->y;
> + s->dest_w = es->width;
> + s->dest_h = es->height;
> + s->src_x = 0;
> + s->src_y = 0;
> + s->src_w = es->width;
> + s->src_h = es->height;
> +
> + wl_list_insert(es->buffer->resource.destroy_listener_list.prev,
> + &s->pending_destroy_listener.link);
> + return 0;
> +}
> +
> +static void
> +drm_assign_planes(struct weston_output *output)
> +{
> + struct weston_compositor *ec = output->compositor;
> + struct weston_surface *es;
> +
> + /*
> + * Find a surface for each sprite in the output using some heuristics:
> + * 1) size
> + * 2) frequency of update
> + * 3) opacity (though some hw might support alpha blending)
> + * 4) clipping (this can be fixed with color keys)
> + *
> + * The idea is to save on blitting since this should save power.
> + * If we can get a large video surface on the sprite for example,
> + * the main display surface may not need to update at all, and
> + * the client buffer can be used directly for the sprite surface
> + * as we do for flipping full screen surfaces.
> + */
> + wl_list_for_each_reverse(es, &ec->surface_list, link) {
> + /* FIXME: if an upper surface is on a hw plane, we don't
> + * need to track its overlap.
> + * FIXME: try to assign hw cursors here too, they're just
> + * special overlays
> + */
> + if (!drm_output_prepare_overlay_surface(output, es)) {
> + pixman_region32_fini(&es->damage);
> + pixman_region32_init(&es->damage);
> + }
> + }
> +
> + drm_disable_unused_sprites(output);
> +}
> +
> +static int
> drm_output_set_cursor(struct weston_output *output_base,
> struct weston_input_device *eid)
> {
> @@ -336,6 +661,7 @@ on_drm_input(int fd, uint32_t mask, void *data)
> memset(&evctx, 0, sizeof evctx);
> evctx.version = DRM_EVENT_CONTEXT_VERSION;
> evctx.page_flip_handler = page_flip_handler;
> + evctx.vblank_handler = vblank_handler;
> drmHandleEvent(fd, &evctx);
>
> return 1;
> @@ -480,6 +806,30 @@ output_handle_pending_scanout_buffer_destroy(struct wl_listener *listener,
> weston_compositor_schedule_repaint(output->base.compositor);
> }
>
> +static void
> +sprite_handle_buffer_destroy(struct wl_listener *listener,
> + struct wl_resource *resource,
> + uint32_t time)
> +{
> + struct drm_sprite *sprite =
> + container_of(listener, struct drm_sprite,
> + destroy_listener);
> +
> + sprite->surface = NULL;
> +}
> +
> +static void
> +sprite_handle_pending_buffer_destroy(struct wl_listener *listener,
> + struct wl_resource *resource,
> + uint32_t time)
> +{
> + struct drm_sprite *sprite =
> + container_of(listener, struct drm_sprite,
> + pending_destroy_listener);
> +
> + sprite->pending_surface = NULL;
> +}
> +
> static int
> create_output_for_connector(struct drm_compositor *ec,
> drmModeRes *resources,
> @@ -613,7 +963,7 @@ create_output_for_connector(struct drm_compositor *ec,
> output->base.repaint = drm_output_repaint;
> output->base.set_hardware_cursor = drm_output_set_cursor;
> output->base.destroy = drm_output_destroy;
> - output->base.assign_planes = NULL;
> + output->base.assign_planes = drm_assign_planes;
>
> return 0;
>
> @@ -649,6 +999,57 @@ err_free:
> return -1;
> }
>
> +static void
> +create_sprites(struct drm_compositor *ec)
> +{
> + struct drm_sprite *sprite;
> + drmModePlaneRes *plane_res;
> + drmModePlane *plane;
> + int i;
> +
> + plane_res = drmModeGetPlaneResources(ec->drm.fd);
> + if (!plane_res) {
> + fprintf(stderr, "failed to get plane resources: %s\n",
> + strerror(errno));
> + return;
> + }
> +
> + for (i = 0; i < plane_res->count_planes; i++) {
> + plane = drmModeGetPlane(ec->drm.fd, plane_res->planes[i]);
> + if (!plane)
> + continue;
> + sprite = malloc(sizeof(*sprite) + ((sizeof(uint32_t)) *
> + plane->count_formats));
> + if (!sprite) {
> + fprintf(stderr, "%s: out of memory\n",
> + __func__);
> + free(plane);
> + continue;
> + }
> +
> + memset(sprite, 0, sizeof *sprite);
> +
> + sprite->possible_crtcs = plane->possible_crtcs;
> + sprite->plane_id = plane->plane_id;
> + sprite->surface = NULL;
> + sprite->pending_surface = NULL;
> + sprite->fb_id = 0;
> + sprite->pending_fb_id = 0;
> + sprite->destroy_listener.func = sprite_handle_buffer_destroy;
> + sprite->pending_destroy_listener.func =
> + sprite_handle_pending_buffer_destroy;
> + memcpy(sprite->formats, plane->formats,
> + plane->count_formats);
> + drmModeFreePlane(plane);
> +
> + wl_list_insert(&ec->sprite_list, &sprite->link);
> + }
> +
> + free(plane_res->planes);
> + free(plane_res);
> +}
> +
> +
> static int
> create_outputs(struct drm_compositor *ec, int option_connector)
> {
> @@ -663,6 +1064,13 @@ create_outputs(struct drm_compositor *ec, int option_connector)
> return -1;
> }
>
> + ec->crtcs = calloc(resources->count_crtcs, sizeof(uint32_t));
> + if (!ec->crtcs)
> + return -1;
> +
> + ec->num_crtcs = resources->count_crtcs;
> + memcpy(ec->crtcs, resources->crtcs, sizeof(uint32_t) * ec->num_crtcs);
> +
> for (i = 0; i < resources->count_connectors; i++) {
> connector = drmModeGetConnector(ec->drm.fd,
> resources->connectors[i]);
> @@ -994,6 +1402,9 @@ drm_compositor_create(struct wl_display *display,
> if (weston_compositor_init(&ec->base, display) < 0)
> return NULL;
>
> + wl_list_init(&ec->sprite_list);
> + create_sprites(ec);
> +
> if (create_outputs(ec, connector) < 0) {
> fprintf(stderr, "failed to create output for %s\n", path);
> return NULL;
> --
> 1.7.4.1
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
More information about the wayland-devel
mailing list