[PATCH 1/3] compositor: add an plane assignment hook
Kristian Høgsberg
krh at bitplanet.net
Wed Jan 25 18:35:57 PST 2012
On Wed, Jan 25, 2012 at 5:00 PM, Jesse Barnes <jbarnes at virtuousgeek.org> wrote:
> Each output may have ways of optimizing surface drawing (e.g. by using
> sprites), so push the handling of surface assignment to display planes
> into the output structure, providing the existing surface setup function
> as a helper.
> ---
> configure.ac | 2 +-
> src/compositor-drm.c | 194 ++++++++++++++++++++++++++++++++++++++++++++-
> src/compositor-openwfd.c | 1 +
> src/compositor-wayland.c | 9 --
> src/compositor-x11.c | 9 --
> src/compositor.c | 54 ++++++-------
> src/compositor.h | 8 +-
> 7 files changed, 220 insertions(+), 57 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 1f30e7b..1d8da77 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>
>
> @@ -51,9 +53,12 @@ struct drm_compositor {
> } drm;
> struct gbm_device *gbm;
> uint32_t crtc_allocator;
> + uint32_t sprite_allocator;
> uint32_t connector_allocator;
> struct tty *tty;
>
> + struct wl_list sprite_list;
> +
> uint32_t prev_state;
> };
>
> @@ -74,10 +79,33 @@ struct drm_output {
> struct gbm_bo *bo[2];
> uint32_t current;
>
> + struct wl_list sprite_list;
> +
> uint32_t fs_surf_fb_id;
> uint32_t pending_fs_surf_fb_id;
> };
>
> +/*
> + * An output has a primary display plane plus zero or more sprites for
> + * blending display contents.
> + */
> +struct drm_sprite {
> + struct wl_list link;
> + struct wl_list output_link;
> +
> + uint32_t possible_crtcs;
> + uint32_t plane_id;
> + uint32_t fb_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
> drm_output_prepare_render(struct weston_output *output_base)
> {
> @@ -191,6 +219,106 @@ drm_output_prepare_scanout_surface(struct weston_output *output_base,
> }
>
> static int
> +drm_assign_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_output *output = (struct drm_output *) output_base;
> + 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;
> +
> + wl_list_for_each(s, &output->sprite_list, output_link) {
> + if (!s->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;
> + }
> +
> + ret = drmModeSetPlane(c->drm.fd, s->plane_id, output->crtc_id, fb_id, 0,
> + es->x, es->y, es->width, es->height, 0, 0,
> + es->width, es->height);
> + if (ret) {
> + fprintf(stderr, "setplane failed %d: %s\n", ret,
> + strerror(errno));
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +static int
> +drm_assign_planes(struct weston_output *output)
> +{
> + struct weston_compositor *ec = output->compositor;
> + struct weston_surface *es, *scanout = NULL;
> +
> + es = container_of(ec->surface_list.next, struct weston_surface, link);
> +
> + if (es->visual == WESTON_RGB_VISUAL) {
> + if (!drm_output_prepare_scanout_surface(output, es)) {
> + weston_queue_scanout_surface(output, es);
> + scanout = 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) {
> + if (es == scanout)
> + continue;
> +
> + if (!drm_assign_overlay_surface(output, es))
> + pixman_region32_init(&es->damage);
This leaks the old region, we need pixman_region32_clear().
> + }
> +
> + return 0;
> +}
> +
> +static int
> drm_output_set_cursor(struct weston_output *output_base,
> struct weston_input_device *eid)
> {
> @@ -428,6 +556,50 @@ drm_subpixel_to_wayland(int drm_value)
> }
> }
>
> +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;
> + memcpy(sprite->formats, plane->formats,
> + plane->count_formats);
> + drmModeFreePlane(plane);
> +
> + ec->sprite_allocator = (1 << sprite->plane_id);
> + wl_list_insert(&ec->sprite_list, &sprite->link);
> + }
> +
> + free(plane_res->planes);
> + free(plane_res);
> +}
> +
> static int
> create_output_for_connector(struct drm_compositor *ec,
> drmModeRes *resources,
> @@ -437,6 +609,7 @@ create_output_for_connector(struct drm_compositor *ec,
> struct drm_output *output;
> struct drm_mode *drm_mode, *next;
> drmModeEncoder *encoder;
> + struct drm_sprite *sprite;
> int i, ret;
> unsigned handle, stride;
>
> @@ -470,6 +643,7 @@ create_output_for_connector(struct drm_compositor *ec,
> output->base.make = "unknown";
> output->base.model = "unknown";
> wl_list_init(&output->base.mode_list);
> + wl_list_init(&output->sprite_list);
>
> output->crtc_id = resources->crtcs[i];
> ec->crtc_allocator |= (1 << output->crtc_id);
> @@ -479,6 +653,16 @@ create_output_for_connector(struct drm_compositor *ec,
> output->original_crtc = drmModeGetCrtc(ec->drm.fd, output->crtc_id);
> drmModeFreeEncoder(encoder);
>
> + wl_list_for_each(sprite, &ec->sprite_list, link) {
> + for (i = 0; i < resources->count_crtcs; i++)
> + if (sprite->possible_crtcs & (1 << i)) {
Shouldn't this just be
if (sprite->possible_crtcs & (1 << output->crtc_id)) instead of the for loop?
> + wl_list_insert(&output->sprite_list,
> + &sprite->output_link);
> + fprintf(stderr, "found sprite %d\n",
> + sprite->plane_id);
> + }
> + }
This doesn't work for sprites that can be used on multiple outputs,
there's only one output_link. I think we just have to keep the sprite
list in the compositor struct and loop through all of them in
assign_overlays, looking for one that available and works with the
output.
> for (i = 0; i < connector->count_modes; i++) {
> ret = drm_output_add_mode(output, &connector->modes[i]);
> if (ret)
> @@ -553,10 +737,9 @@ create_output_for_connector(struct drm_compositor *ec,
> wl_list_insert(ec->base.output_list.prev, &output->base.link);
>
> output->pending_fs_surf_fb_id = 0;
> + output->base.assign_planes = drm_assign_planes;
> output->base.prepare_render = drm_output_prepare_render;
> output->base.present = drm_output_present;
> - output->base.prepare_scanout_surface =
> - drm_output_prepare_scanout_surface;
> output->base.set_hardware_cursor = drm_output_set_cursor;
> output->base.destroy = drm_output_destroy;
>
> @@ -603,10 +786,8 @@ create_outputs(struct drm_compositor *ec, int option_connector)
> int x = 0, y = 0;
>
> resources = drmModeGetResources(ec->drm.fd);
> - if (!resources) {
> - fprintf(stderr, "drmModeGetResources failed\n");
> + if (!resources)
> return -1;
> - }
>
> for (i = 0; i < resources->count_connectors; i++) {
> connector = drmModeGetConnector(ec->drm.fd,
> @@ -939,6 +1120,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;
> diff --git a/src/compositor-openwfd.c b/src/compositor-openwfd.c
> index 2299d99..2e5fe8f 100644
> --- a/src/compositor-openwfd.c
> +++ b/src/compositor-openwfd.c
> @@ -406,6 +406,7 @@ create_output_for_port(struct wfd_compositor *ec,
>
> wfdDeviceCommit(ec->dev, WFD_COMMIT_ENTIRE_DEVICE, WFD_INVALID_HANDLE);
>
> + output->base.repaint = weston_output_repaint;
I can see you don't compile the openwf compositor either :-)
> output->base.prepare_render = wfd_output_prepare_render;
> output->base.present = wfd_output_present;
> output->base.prepare_scanout_surface =
> diff --git a/src/compositor-wayland.c b/src/compositor-wayland.c
> index 4fa9df1..a3f2b1e 100644
> --- a/src/compositor-wayland.c
> +++ b/src/compositor-wayland.c
> @@ -207,13 +207,6 @@ wayland_output_present(struct weston_output *output_base)
> }
>
> static int
> -wayland_output_prepare_scanout_surface(struct weston_output *output_base,
> - struct weston_surface *es)
> -{
> - return -1;
> -}
> -
> -static int
> wayland_output_set_cursor(struct weston_output *output_base,
> struct weston_input_device *input)
> {
> @@ -292,8 +285,6 @@ wayland_compositor_create_output(struct wayland_compositor *c,
>
> output->base.prepare_render = wayland_output_prepare_render;
> output->base.present = wayland_output_present;
> - output->base.prepare_scanout_surface =
> - wayland_output_prepare_scanout_surface;
> output->base.set_hardware_cursor = wayland_output_set_cursor;
> output->base.destroy = wayland_output_destroy;
>
> diff --git a/src/compositor-x11.c b/src/compositor-x11.c
> index 09213f7..cd10968 100644
> --- a/src/compositor-x11.c
> +++ b/src/compositor-x11.c
> @@ -231,13 +231,6 @@ x11_output_present(struct weston_output *output_base)
> }
>
> static int
> -x11_output_prepare_scanout_surface(struct weston_output *output_base,
> - struct weston_surface *es)
> -{
> - return -1;
> -}
> -
> -static int
> x11_output_set_cursor(struct weston_output *output_base,
> struct weston_input_device *input)
> {
> @@ -463,8 +456,6 @@ x11_compositor_create_output(struct x11_compositor *c, int x, int y,
>
> output->base.prepare_render = x11_output_prepare_render;
> output->base.present = x11_output_present;
> - output->base.prepare_scanout_surface =
> - x11_output_prepare_scanout_surface;
> output->base.set_hardware_cursor = x11_output_set_cursor;
> output->base.destroy = x11_output_destroy;
>
> diff --git a/src/compositor.c b/src/compositor.c
> index b4c766c..f328f55 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -762,13 +762,10 @@ out:
> pixman_region32_fini(&cursor_region);
> }
>
> -static int
> -setup_scanout_surface(struct weston_output *output, struct weston_surface *es)
> +WL_EXPORT int
> +weston_queue_scanout_surface(struct weston_output *output,
> + struct weston_surface *es)
> {
> - if (es->visual != WESTON_RGB_VISUAL ||
> - output->prepare_scanout_surface(output, es) != 0)
> - return -1;
> -
> /* assert output->pending_scanout_buffer == NULL */
> output->pending_scanout_buffer = es->buffer;
> output->pending_scanout_buffer->busy_count++;
> @@ -779,7 +776,7 @@ setup_scanout_surface(struct weston_output *output, struct weston_surface *es)
> return 0;
> }
>
> -static void
> +WL_EXPORT void
> weston_output_repaint(struct weston_output *output)
> {
> struct weston_compositor *ec = output->compositor;
> @@ -818,30 +815,27 @@ weston_output_repaint(struct weston_output *output)
>
> es = container_of(ec->surface_list.next, struct weston_surface, link);
>
> - if (setup_scanout_surface(output, es) == 0)
> - /* We're drawing nothing, just let the damage accumulate */
> - return;
> -
> - if (es->fullscreen_output == output) {
> - if (es->width < output->current->width ||
> - es->height < output->current->height)
> - glClear(GL_COLOR_BUFFER_BIT);
> - weston_surface_draw(es, output, &total_damage);
> - } else {
> - wl_list_for_each(es, &ec->surface_list, link) {
> - pixman_region32_copy(&es->damage, &total_damage);
> - pixman_region32_subtract(&total_damage, &total_damage, &es->opaque);
> - }
> + wl_list_for_each(es, &ec->surface_list, link) {
> + pixman_region32_copy(&es->damage, &total_damage);
> + pixman_region32_subtract(&total_damage, &total_damage,
> + &es->opaque);
> + }
>
> - wl_list_for_each_reverse(es, &ec->surface_list, link) {
> - pixman_region32_init(&repaint);
> - pixman_region32_intersect(&repaint, &output->region,
> - &es->damage);
> - weston_surface_draw(es, output, &repaint);
> - pixman_region32_subtract(&es->damage,
> - &es->damage, &output->region);
> - pixman_region32_fini(&repaint);
> - }
> + if (output->assign_planes)
> + /* This will queue flips for the fbs and sprites where
> + * applicable and clear the damage for those surfaces.
> + * The repaint loop below will repaint everything
> + * else. */
> + output->assign_planes(output);
> +
> + wl_list_for_each_reverse(es, &ec->surface_list, link) {
> + pixman_region32_init(&repaint);
> + pixman_region32_intersect(&repaint, &output->region,
> + &es->damage);
> + weston_surface_draw(es, output, &repaint);
> + pixman_region32_subtract(&es->damage,
> + &es->damage, &output->region);
> + pixman_region32_fini(&repaint);
> }
>
> if (ec->fade.spring.current > 0.001)
> diff --git a/src/compositor.h b/src/compositor.h
> index d21e285..031d094 100644
> --- a/src/compositor.h
> +++ b/src/compositor.h
> @@ -89,8 +89,7 @@ struct weston_output {
>
> int (*prepare_render)(struct weston_output *output);
> int (*present)(struct weston_output *output);
> - int (*prepare_scanout_surface)(struct weston_output *output,
> - struct weston_surface *es);
> + int (*assign_planes)(struct weston_output *output);
> int (*set_hardware_cursor)(struct weston_output *output,
> struct weston_input_device *input);
> void (*destroy)(struct weston_output *output);
> @@ -299,6 +298,8 @@ notify_touch(struct wl_input_device *device, uint32_t time, int touch_id,
> int x, int y, int touch_type);
>
> void
> +weston_output_repaint(struct weston_output *output);
> +void
> weston_output_finish_frame(struct weston_output *output, int msecs);
> void
> weston_output_damage(struct weston_output *output);
> @@ -447,5 +448,6 @@ typedef void (*weston_zoom_done_func_t)(struct weston_zoom *zoom, void *data);
> struct weston_zoom *
> weston_zoom_run(struct weston_surface *surface, GLfloat start, GLfloat stop,
> weston_zoom_done_func_t done, void *data);
> -
> +int weston_queue_scanout_surface(struct weston_output *output,
> + struct weston_surface *es);
> #endif
> --
> 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