[PATCH weston 04/14 v3] weston: Port DRM backend to new output handling API
Pekka Paalanen
ppaalanen at gmail.com
Tue Sep 13 10:49:49 UTC 2016
On Thu, 18 Aug 2016 18:42:32 +0200
Armin Krezović <krezovic.armin at gmail.com> wrote:
> This is a complete port of the DRM backend that uses
> recently added output handling API for output
> configuration.
>
> Output can be configured at runtime by passing the
> necessary configuration parameters, which can be
> filled in manually or obtained from the configuration
> file using previously added functionality. It is
> required that the scale and transform values are set
> using the previously added functionality.
>
> After everything has been set, output needs to be
> enabled manually using weston_output_enable().
>
> v2:
>
> - Added missing drmModeFreeCrtc() to drm_output_enable()
> cleanup list in case of failure.
> - Split drm_backend_disable() into drm_backend_deinit()
> to accomodate for changes in the first patch in the
> series. Moved restoring original crtc to
> drm_output_destroy().
>
> v3:
>
> - Moved origcrtc allocation to drm_output_set_mode().
> - Swapped connector_get_current_mode() and
> drm_output_add_mode() calls in drm_output_set_mode()
> to match current weston.
> - Moved crtc_allocator and connector_allocator update
> from drm_output_enable() to create_output_for_connector()
> to avoid problems when more than one monitor is connected
> at startup and crtc allocator wasn't updated before
> create_output_for_connector() was called second time,
> resulting in one screen being turned off.
> - Moved crtc_allocator and connector_allocator update from
> drm_output_deinit() to drm_output_destroy(), as it
> should not be called on drm_output_disable().
> - Use weston_compositor_add_pending_output().
> - Bump weston_drm_backend_config version to 2.
>
> Signed-off-by: Armin Krezović <krezovic.armin at gmail.com>
> ---
> compositor/main.c | 104 ++++++-----
> libweston/compositor-drm.c | 434 ++++++++++++++++++++++++++-------------------
> libweston/compositor-drm.h | 50 +++---
> 3 files changed, 343 insertions(+), 245 deletions(-)
Hi Armin,
all in all, this patch looks very good. There are a few minor bugs to
be squashed, and I did list some future development ideas you are free
to ignore.
Please wait for reviews for the whole series before re-sending.
Details inlined below.
>
> diff --git a/compositor/main.c b/compositor/main.c
> index 0cc11a5..38df77f 100644
> --- a/compositor/main.c
> +++ b/compositor/main.c
> @@ -1087,48 +1087,6 @@ wet_configure_windowed_output_from_config(struct weston_output *output,
> return 0;
> }
>
> -static enum weston_drm_backend_output_mode
> -drm_configure_output(struct weston_compositor *c,
> - bool use_current_mode,
> - const char *name,
> - struct weston_drm_backend_output_config *config)
> -{
> - struct weston_config *wc = wet_get_config(c);
> - struct weston_config_section *section;
> - char *s;
> - int scale;
> - enum weston_drm_backend_output_mode mode =
> - WESTON_DRM_BACKEND_OUTPUT_PREFERRED;
> -
> - section = weston_config_get_section(wc, "output", "name", name);
> - weston_config_section_get_string(section, "mode", &s, "preferred");
> - if (strcmp(s, "off") == 0) {
> - free(s);
> - return WESTON_DRM_BACKEND_OUTPUT_OFF;
> - }
> -
> - if (use_current_mode || strcmp(s, "current") == 0) {
> - mode = WESTON_DRM_BACKEND_OUTPUT_CURRENT;
> - } else if (strcmp(s, "preferred") != 0) {
> - config->modeline = s;
> - s = NULL;
> - }
> - free(s);
> -
> - weston_config_section_get_int(section, "scale", &scale, 1);
> - config->base.scale = scale >= 1 ? scale : 1;
> - weston_config_section_get_string(section, "transform", &s, "normal");
> - if (weston_parse_transform(s, &config->base.transform) < 0)
> - weston_log("Invalid transform \"%s\" for output %s\n",
> - s, name);
> - free(s);
> -
> - weston_config_section_get_string(section,
> - "gbm-format", &config->gbm_format, NULL);
> - weston_config_section_get_string(section, "seat", &config->seat, "");
> - return mode;
> -}
> -
> static void
> configure_input_device(struct weston_compositor *compositor,
> struct libinput_device *device)
> @@ -1153,6 +1111,65 @@ configure_input_device(struct weston_compositor *compositor,
> }
> }
>
> +static void
> +drm_backend_output_configure(struct wl_listener *listener, void *data)
> +{
> + struct weston_output *output = data;
> + struct weston_config *wc = wet_get_config(output->compositor);
> + struct weston_config_section *section;
> + const struct weston_drm_output_api *api = weston_drm_output_get_api(output->compositor);
> + enum weston_drm_backend_output_mode mode =
> + WESTON_DRM_BACKEND_OUTPUT_PREFERRED;
> +
> + char *s;
> + char *modeline = NULL;
> + char *gbm_format = NULL;
> + char *seat = NULL;
> +
> + if (!api) {
> + weston_log("Cannot use weston_drm_output_api.\n");
> + return;
> + }
> +
> + section = weston_config_get_section(wc, "output", "name", output->name);
> + weston_config_section_get_string(section, "mode", &s, "preferred");
> +
> + if (strcmp(s, "off") == 0) {
> + weston_output_disable(output);
> + free(s);
> + return;
> + } else if (strcmp(s, "current") == 0) {
> + mode = WESTON_DRM_BACKEND_OUTPUT_CURRENT;
> + } else if (strcmp(s, "preferred") != 0) {
> + modeline = s;
> + s = NULL;
> + }
> + free(s);
> +
> + if (api->set_mode(output, mode, modeline) < 0) {
> + weston_log("Cannot configure an output using weston_drm_output_api.\n");
> + free(modeline);
> + return;
> + }
> + free(modeline);
> +
> + wet_output_set_scale(output, section, 1, 0);
> + wet_output_set_transform(output, section, WL_OUTPUT_TRANSFORM_NORMAL, UINT32_MAX);
> +
> + weston_config_section_get_string(section,
> + "gbm-format", &gbm_format, NULL);
> +
> + api->set_gbm_format(output, gbm_format);
> + free(gbm_format);
> +
> + weston_config_section_get_string(section, "seat", &seat, "");
> +
> + api->set_seat(output, seat);
> + free(seat);
> +
> + weston_output_enable(output);
> +}
> +
> static int
> load_drm_backend(struct weston_compositor *c,
> int *argc, char **argv, struct weston_config *wc)
> @@ -1178,12 +1195,13 @@ load_drm_backend(struct weston_compositor *c,
>
> config.base.struct_version = WESTON_DRM_BACKEND_CONFIG_VERSION;
> config.base.struct_size = sizeof(struct weston_drm_backend_config);
> - config.configure_output = drm_configure_output;
> config.configure_device = configure_input_device;
>
> ret = weston_compositor_load_backend(c, WESTON_BACKEND_DRM,
> &config.base);
>
> + wet_set_pending_output_handler(c, drm_backend_output_configure);
> +
> free(config.gbm_format);
> free(config.seat_id);
>
> diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
> index 8319d7c..5b23ea1 100644
> --- a/libweston/compositor-drm.c
> +++ b/libweston/compositor-drm.c
> @@ -122,16 +122,6 @@ struct drm_backend {
> int32_t cursor_width;
> int32_t cursor_height;
>
> - /** Callback used to configure the outputs.
> - *
> - * This function will be called by the backend when a new DRM
> - * output needs to be configured.
> - */
> - enum weston_drm_backend_output_mode
> - (*configure_output)(struct weston_compositor *compositor,
> - bool use_current_mode,
> - const char *name,
> - struct weston_drm_backend_output_config *output_config);
> bool use_current_mode;
> };
>
> @@ -161,7 +151,8 @@ struct drm_edid {
> };
>
> struct drm_output {
> - struct weston_output base;
> + struct weston_output base;
> + drmModeConnector *connector;
>
> uint32_t crtc_id;
> int pipe;
> @@ -176,6 +167,7 @@ struct drm_output {
> int vblank_pending;
> int page_flip_pending;
> int destroy_pending;
> + int disable_pending;
>
> struct gbm_surface *gbm_surface;
> struct gbm_bo *gbm_cursor_bo[2];
> @@ -681,7 +673,7 @@ drm_output_repaint(struct weston_output *output_base,
> struct drm_mode *mode;
> int ret = 0;
>
> - if (output->destroy_pending)
> + if (output->disable_pending || output->destroy_pending)
> return -1;
>
> if (!output->next)
> @@ -787,7 +779,7 @@ drm_output_start_repaint_loop(struct weston_output *output_base)
> .request.signal = 0,
> };
>
> - if (output->destroy_pending)
> + if (output->disable_pending || output->destroy_pending)
> return;
>
> if (!output->current) {
> @@ -877,7 +869,7 @@ vblank_handler(int fd, unsigned int frame, unsigned int sec, unsigned int usec,
> }
>
> static void
> -drm_output_destroy(struct weston_output *output_base);
> +drm_output_destroy(struct weston_output *base);
>
> static void
> page_flip_handler(int fd, unsigned int frame,
> @@ -904,6 +896,8 @@ page_flip_handler(int fd, unsigned int frame,
>
> if (output->destroy_pending)
> drm_output_destroy(&output->base);
> + else if (output->disable_pending)
> + weston_output_disable(&output->base);
> else if (!output->vblank_pending) {
> ts.tv_sec = sec;
> ts.tv_nsec = usec * 1000;
> @@ -1344,51 +1338,6 @@ drm_assign_planes(struct weston_output *output_base)
> static void
> drm_output_fini_pixman(struct drm_output *output);
>
> -static void
> -drm_output_destroy(struct weston_output *output_base)
> -{
> - struct drm_output *output = to_drm_output(output_base);
> - struct drm_backend *b = to_drm_backend(output->base.compositor);
> - drmModeCrtcPtr origcrtc = output->original_crtc;
> -
> - if (output->page_flip_pending) {
> - output->destroy_pending = 1;
> - weston_log("destroy output while page flip pending\n");
> - return;
> - }
> -
> - if (output->backlight)
> - backlight_destroy(output->backlight);
> -
> - drmModeFreeProperty(output->dpms_prop);
> -
> - /* Turn off hardware cursor */
> - drmModeSetCursor(b->drm.fd, output->crtc_id, 0, 0, 0);
> -
> - /* Restore original CRTC state */
> - drmModeSetCrtc(b->drm.fd, origcrtc->crtc_id, origcrtc->buffer_id,
> - origcrtc->x, origcrtc->y,
> - &output->connector_id, 1, &origcrtc->mode);
> - drmModeFreeCrtc(origcrtc);
> -
> - b->crtc_allocator &= ~(1 << output->crtc_id);
> - b->connector_allocator &= ~(1 << output->connector_id);
> -
> - if (b->use_pixman) {
> - drm_output_fini_pixman(output);
> - } else {
> - gl_renderer->output_destroy(output_base);
> - gbm_surface_destroy(output->gbm_surface);
> - }
> -
> - weston_plane_release(&output->fb_plane);
> - weston_plane_release(&output->cursor_plane);
> -
> - weston_output_destroy(&output->base);
> -
> - free(output);
> -}
> -
> /**
> * Find the closest-matching mode for a given target
> *
> @@ -2239,7 +2188,7 @@ static struct drm_mode *
> drm_output_choose_initial_mode(struct drm_backend *backend,
> struct drm_output *output,
> enum weston_drm_backend_output_mode mode,
> - struct weston_drm_backend_output_config *config,
> + const char *modeline,
> const drmModeModeInfo *current_mode)
> {
> struct drm_mode *preferred = NULL;
> @@ -2247,21 +2196,21 @@ drm_output_choose_initial_mode(struct drm_backend *backend,
> struct drm_mode *configured = NULL;
> struct drm_mode *best = NULL;
> struct drm_mode *drm_mode;
> - drmModeModeInfo modeline;
> + drmModeModeInfo drm_modeline;
> int32_t width = 0;
> int32_t height = 0;
>
> - if (mode == WESTON_DRM_BACKEND_OUTPUT_PREFERRED && config->modeline) {
> - if (sscanf(config->modeline, "%dx%d", &width, &height) != 2) {
> + if (mode == WESTON_DRM_BACKEND_OUTPUT_PREFERRED && modeline) {
> + if (sscanf(modeline, "%dx%d", &width, &height) != 2) {
> width = -1;
>
> - if (parse_modeline(config->modeline, &modeline) == 0) {
> - configured = drm_output_add_mode(output, &modeline);
> + if (parse_modeline(modeline, &drm_modeline) == 0) {
> + configured = drm_output_add_mode(output, &drm_modeline);
> if (!configured)
> return NULL;
> } else {
> weston_log("Invalid modeline \"%s\" for output %s\n",
> - config->modeline, output->base.name);
> + modeline, output->base.name);
> }
> }
> }
> @@ -2330,109 +2279,103 @@ connector_get_current_mode(drmModeConnector *connector, int drm_fd,
> return 0;
> }
>
> -/**
> - * Create and configure a Weston output structure
> - *
> - * Given a DRM connector, create a matching drm_output structure and add it
> - * to Weston's output list.
> - *
> - * @param b Weston backend structure structure
> - * @param resources DRM resources for this device
> - * @param connector DRM connector to use for this new output
> - * @param x Horizontal offset to use into global co-ordinate space
> - * @param y Vertical offset to use into global co-ordinate space
> - * @param drm_device udev device pointer
> - * @returns 0 on success, or -1 on failure
> - */
> static int
> -create_output_for_connector(struct drm_backend *b,
> - drmModeRes *resources,
> - drmModeConnector *connector,
> - int x, int y, struct udev_device *drm_device)
> +drm_output_set_mode(struct weston_output *base,
> + enum weston_drm_backend_output_mode mode,
> + const char *modeline)
> {
> - struct drm_output *output;
> - struct drm_mode *drm_mode, *next, *current;
> - struct weston_mode *m;
> + struct drm_output *output = to_drm_output(base);
> + struct drm_backend *b = to_drm_backend(base->compositor);
>
> + struct drm_mode *drm_mode, *next, *current;
> drmModeModeInfo crtc_mode;
> int i;
> - enum weston_drm_backend_output_mode mode;
> - struct weston_drm_backend_output_config config = {{ 0 }};
> -
> - i = find_crtc_for_connector(b, resources, connector);
> - if (i < 0) {
> - weston_log("No usable crtc/encoder pair for connector.\n");
> - return -1;
> - }
> -
> - output = zalloc(sizeof *output);
> - if (output == NULL)
> - return -1;
>
> - output->base.subpixel = drm_subpixel_to_wayland(connector->subpixel);
> - output->base.name = make_connector_name(connector);
> output->base.make = "unknown";
> output->base.model = "unknown";
> output->base.serial_number = "unknown";
> wl_list_init(&output->base.mode_list);
>
> - mode = b->configure_output(b->compositor, b->use_current_mode,
> - output->base.name, &config);
> - if (parse_gbm_format(config.gbm_format, b->gbm_format, &output->gbm_format) == -1)
> - output->gbm_format = b->gbm_format;
> -
> - setup_output_seat_constraint(b, &output->base,
> - config.seat ? config.seat : "");
> - free(config.seat);
> -
> - output->crtc_id = resources->crtcs[i];
> - output->pipe = i;
> - b->crtc_allocator |= (1 << output->crtc_id);
> - output->connector_id = connector->connector_id;
> - b->connector_allocator |= (1 << output->connector_id);
> -
> output->original_crtc = drmModeGetCrtc(b->drm.fd, output->crtc_id);
> - output->dpms_prop = drm_get_prop(b->drm.fd, connector, "DPMS");
>
> - if (connector_get_current_mode(connector, b->drm.fd, &crtc_mode) < 0)
> + if (connector_get_current_mode(output->connector, b->drm.fd, &crtc_mode) < 0)
> goto err_free;
>
> - for (i = 0; i < connector->count_modes; i++) {
> - drm_mode = drm_output_add_mode(output, &connector->modes[i]);
> + for (i = 0; i < output->connector->count_modes; i++) {
> + drm_mode = drm_output_add_mode(output, &output->connector->modes[i]);
> if (!drm_mode)
> goto err_free;
> }
>
> - if (mode == WESTON_DRM_BACKEND_OUTPUT_OFF) {
> - weston_log("Disabling output %s\n", output->base.name);
> - drmModeSetCrtc(b->drm.fd, output->crtc_id,
> - 0, 0, 0, 0, 0, NULL);
> - goto err_free;
> - }
> -
> - current = drm_output_choose_initial_mode(b, output, mode, &config,
> - &crtc_mode);
> + current = drm_output_choose_initial_mode(b, output, mode, modeline, &crtc_mode);
> if (!current)
> goto err_free;
> +
> output->base.current_mode = ¤t->base;
> output->base.current_mode->flags |= WL_OUTPUT_MODE_CURRENT;
Here is an idea for optional follow-up work, not to be done in this
patch: construct the mode list already in
create_output_for_connector(). This way the compositor can inspect the
possible video modes before it picks a video mode. That would actually
be a beginning for redesigning how the the video mode setting API for
DRM works. Obviously, that's for another time.
>
> - weston_output_init(&output->base, b->compositor, x, y,
> - connector->mmWidth, connector->mmHeight,
> - config.base.transform, config.base.scale);
> + /* Set native_ fields, so weston_output_mode_switch_to_native() works */
> + output->base.native_mode = output->base.current_mode;
> + output->base.native_scale = output->base.current_scale;
> +
> + output->base.mm_width = output->connector->mmWidth;
> + output->base.mm_height = output->connector->mmHeight;
> +
> + return 0;
> +
> +err_free:
> + drmModeFreeCrtc(output->original_crtc);
original_crtc needs to be reset to NULL, otherwise drm_output_destroy()
uses it.
> +
> + wl_list_for_each_safe(drm_mode, next, &output->base.mode_list,
> + base.link) {
> + wl_list_remove(&drm_mode->base.link);
> + free(drm_mode);
> + }
> +
> + return -1;
> +}
> +
> +static void
> +drm_output_set_gbm_format(struct weston_output *base,
> + const char *gbm_format)
> +{
> + struct drm_output *output = to_drm_output(base);
> + struct drm_backend *b = to_drm_backend(base->compositor);
> +
> + if (parse_gbm_format(gbm_format, b->gbm_format, &output->gbm_format) == -1)
> + output->gbm_format = b->gbm_format;
> +}
> +
> +static void
> +drm_output_set_seat(struct weston_output *base,
> + const char *seat)
> +{
> + struct drm_output *output = to_drm_output(base);
> + struct drm_backend *b = to_drm_backend(base->compositor);
> +
> + setup_output_seat_constraint(b, &output->base,
> + seat ? seat : "");
> +}
> +
> +static int
> +drm_output_enable(struct weston_output *base)
> +{
> + struct drm_output *output = to_drm_output(base);
> + struct drm_backend *b = to_drm_backend(base->compositor);
> + struct weston_mode *m;
> +
> + output->dpms_prop = drm_get_prop(b->drm.fd, output->connector, "DPMS");
>
> if (b->use_pixman) {
> if (drm_output_init_pixman(output, b) < 0) {
> weston_log("Failed to init output pixman state\n");
> - goto err_output;
> + goto err_free;
> }
> } else if (drm_output_init_egl(output, b) < 0) {
> weston_log("Failed to init output gl state\n");
> - goto err_output;
> + goto err_free;
> }
>
> - output->backlight = backlight_init(drm_device,
> - connector->connector_type);
> if (output->backlight) {
Any reason you left the backlight_init() call in
create_output_for_connector()? I think it would be more logical here,
being called only when enabled. It shouldn't matter in practise though.
> weston_log("Initialized backlight, device %s\n",
> output->backlight->path);
> @@ -2442,15 +2385,8 @@ create_output_for_connector(struct drm_backend *b,
> weston_log("Failed to initialize backlight\n");
> }
>
> - weston_compositor_add_output(b->compositor, &output->base);
> -
> - find_and_parse_output_edid(b, output, connector);
> - if (connector->connector_type == DRM_MODE_CONNECTOR_LVDS)
> - output->base.connection_internal = 1;
> -
> output->base.start_repaint_loop = drm_output_start_repaint_loop;
> output->base.repaint = drm_output_repaint;
> - output->base.destroy = drm_output_destroy;
> output->base.assign_planes = drm_assign_planes;
> output->base.set_dpms = drm_set_dpms;
> output->base.switch_mode = drm_output_switch_mode;
> @@ -2458,6 +2394,12 @@ create_output_for_connector(struct drm_backend *b,
> output->base.gamma_size = output->original_crtc->gamma_size;
> output->base.set_gamma = drm_output_set_gamma;
>
> + output->base.subpixel = drm_subpixel_to_wayland(output->connector->subpixel);
> +
> + find_and_parse_output_edid(b, output, output->connector);
> + if (output->connector->connector_type == DRM_MODE_CONNECTOR_LVDS)
> + output->base.connection_internal = 1;
> +
> weston_plane_init(&output->cursor_plane, b->compositor,
> INT32_MIN, INT32_MIN);
> weston_plane_init(&output->fb_plane, b->compositor, 0, 0);
> @@ -2475,31 +2417,155 @@ create_output_for_connector(struct drm_backend *b,
> ", preferred" : "",
> m->flags & WL_OUTPUT_MODE_CURRENT ?
> ", current" : "",
> - connector->count_modes == 0 ?
> + output->connector->count_modes == 0 ?
> ", built-in" : "");
>
> - /* Set native_ fields, so weston_output_mode_switch_to_native() works */
> - output->base.native_mode = output->base.current_mode;
> - output->base.native_scale = output->base.current_scale;
> -
> return 0;
>
> -err_output:
> - weston_output_destroy(&output->base);
> err_free:
> - wl_list_for_each_safe(drm_mode, next, &output->base.mode_list,
> - base.link) {
> - wl_list_remove(&drm_mode->base.link);
> - free(drm_mode);
> + drmModeFreeProperty(output->dpms_prop);
> +
> + return -1;
> +}
> +
> +static void
> +drm_output_deinit(struct weston_output *base)
> +{
> + struct drm_output *output = to_drm_output(base);
> + struct drm_backend *b = to_drm_backend(base->compositor);
> +
> + if (b->use_pixman) {
> + drm_output_fini_pixman(output);
> + } else {
> + gl_renderer->output_destroy(&output->base);
> + gbm_surface_destroy(output->gbm_surface);
> }
>
> - drmModeFreeCrtc(output->original_crtc);
> + weston_plane_release(&output->fb_plane);
> + weston_plane_release(&output->cursor_plane);
> +
> + drmModeFreeProperty(output->dpms_prop);
> +
> + /* Turn off hardware cursor */
> + drmModeSetCursor(b->drm.fd, output->crtc_id, 0, 0, 0);
> +}
> +
> +static void
> +drm_output_destroy(struct weston_output *base)
> +{
> + struct drm_output *output = to_drm_output(base);
> + struct drm_backend *b = to_drm_backend(base->compositor);
> + drmModeCrtcPtr origcrtc = output->original_crtc;
> +
> + if (output->page_flip_pending) {
> + output->destroy_pending = 1;
> + weston_log("destroy output while page flip pending\n");
> + return;
> + }
> +
> + if (output->base.enabled)
> + drm_output_deinit(&output->base);
> +
> + if (origcrtc) {
> + /* Restore original CRTC state */
> + drmModeSetCrtc(b->drm.fd, origcrtc->crtc_id, origcrtc->buffer_id,
> + origcrtc->x, origcrtc->y,
> + &output->connector_id, 1, &origcrtc->mode);
> + drmModeFreeCrtc(origcrtc);
> + }
> +
> + weston_output_destroy(&output->base);
> +
> + drmModeFreeConnector(output->connector);
> +
> + if (output->backlight)
> + backlight_destroy(output->backlight);
I think nothing is releasing the mode list constructed in
drm_output_set_mode(), but that was the case also before your patches,
so it's not for this patch to fix. It could be a follow-up.
> +
> b->crtc_allocator &= ~(1 << output->crtc_id);
> b->connector_allocator &= ~(1 << output->connector_id);
> +
> free(output);
> - free(config.modeline);
> +}
>
> - return -1;
> +static int
> +drm_output_disable(struct weston_output *base)
> +{
> + struct drm_output *output = to_drm_output(base);
> + struct drm_backend *b = to_drm_backend(base->compositor);
> +
> + if (output->page_flip_pending) {
> + output->disable_pending = 1;
> + weston_log("disable output while page flip pending\n");
Disable while page flip pending shouldn't be a problem, because the
compositor continues to run, and when the page flip event arrives, the
handler there will finish the disabling. So I think this message is not
needed.
Destroy while page flip pending is a different matter, since it can
happen during shutdown.
> + return -1;
> + }
> +
> + if (output->base.enabled)
> + drm_output_deinit(&output->base);
> +
> + output->disable_pending = 0;
> +
> + weston_log("Disabling output %s\n", output->base.name);
> + drmModeSetCrtc(b->drm.fd, output->crtc_id,
> + 0, 0, 0, 0, 0, NULL);
I was a bit surprised by these two statements here, but then I saw that
the page flip handler calls weston_output_disable() again, so also the
page flip path will end up here eventually. All good.
> +
> + return 0;
> +}
> +
> +/**
> + * Create and configure a Weston output structure
> + *
> + * Given a DRM connector, create a matching drm_output structure and add it
> + * to Weston's output list.
> + *
> + * @param b Weston backend structure structure
> + * @param resources DRM resources for this device
> + * @param connector DRM connector to use for this new output
Would be worth documenting that this function takes the ownership of
'connector' if it succeeds. It is fairly rare to do that.
> + * @param drm_device udev device pointer
> + * @returns 0 on success, or -1 on failure
> + */
> +static int
> +create_output_for_connector(struct drm_backend *b,
> + drmModeRes *resources,
> + drmModeConnector *connector,
> + struct udev_device *drm_device)
> +{
> + struct drm_output *output;
> + int i;
> +
> + i = find_crtc_for_connector(b, resources, connector);
> + if (i < 0) {
> + weston_log("No usable crtc/encoder pair for connector.\n");
> + return -1;
> + }
> +
> + output = zalloc(sizeof *output);
> + if (output == NULL)
> + return -1;
> +
> + output->connector = connector;
> + output->crtc_id = resources->crtcs[i];
> + output->pipe = i;
> + output->connector_id = connector->connector_id;
> +
> + output->backlight = backlight_init(drm_device,
> + connector->connector_type);
> +
> + output->base.enable = drm_output_enable;
> + output->base.destroy = drm_output_destroy;
> + output->base.disable = drm_output_disable;
> + output->base.name = make_connector_name(connector);
> +
> + output->destroy_pending = 0;
> + output->disable_pending = 0;
> + output->original_crtc = NULL;
> +
> + b->crtc_allocator |= (1 << output->crtc_id);
> + b->connector_allocator |= (1 << output->connector_id);
> +
> + weston_output_init_pending(&output->base, b->compositor);
> + weston_compositor_add_pending_output(&output->base, b->compositor);
> +
> + return 0;
> }
>
> static void
> @@ -2578,7 +2644,6 @@ create_outputs(struct drm_backend *b, uint32_t option_connector,
> drmModeConnector *connector;
> drmModeRes *resources;
> int i;
> - int x = 0, y = 0;
>
> resources = drmModeGetResources(b->drm.fd);
> if (!resources) {
> @@ -2610,21 +2675,15 @@ create_outputs(struct drm_backend *b, uint32_t option_connector,
> (option_connector == 0 ||
> connector->connector_id == option_connector)) {
> if (create_output_for_connector(b, resources,
> - connector, x, y,
> - drm_device) < 0) {
> + connector, drm_device) < 0) {
> drmModeFreeConnector(connector);
> continue;
> }
> -
> - x += container_of(b->compositor->output_list.prev,
> - struct weston_output,
> - link)->width;
> }
> -
> - drmModeFreeConnector(connector);
The Free should be moved into an else-branch instead of removed,
shouldn't it?
> }
>
> - if (wl_list_empty(&b->compositor->output_list))
> + if (wl_list_empty(&b->compositor->output_list) &&
> + wl_list_empty(&b->compositor->pending_output_list))
> weston_log("No currently active connector found.\n");
>
> drmModeFreeResources(resources);
> @@ -2638,7 +2697,6 @@ update_outputs(struct drm_backend *b, struct udev_device *drm_device)
> drmModeConnector *connector;
> drmModeRes *resources;
> struct drm_output *output, *next;
> - int x = 0, y = 0;
> uint32_t connected = 0, disconnects = 0;
> int i;
>
> @@ -2664,23 +2722,11 @@ update_outputs(struct drm_backend *b, struct udev_device *drm_device)
> connected |= (1 << connector_id);
>
> if (!(b->connector_allocator & (1 << connector_id))) {
> - struct weston_output *last =
> - container_of(b->compositor->output_list.prev,
> - struct weston_output, link);
> -
> - /* XXX: not yet needed, we die with 0 outputs */
> - if (!wl_list_empty(&b->compositor->output_list))
> - x = last->x + last->width;
> - else
> - x = 0;
> - y = 0;
> create_output_for_connector(b, resources,
> - connector, x, y,
> - drm_device);
> + connector, drm_device);
> weston_log("connector %d connected\n", connector_id);
>
> }
> - drmModeFreeConnector(connector);
drmModeFreeConnector() should stay but in a new else-branch, right?
> }
> drmModeFreeResources(resources);
>
> @@ -2695,6 +2741,16 @@ update_outputs(struct drm_backend *b, struct udev_device *drm_device)
> drm_output_destroy(&output->base);
> }
> }
> +
> + wl_list_for_each_safe(output, next, &b->compositor->pending_output_list,
> + base.link) {
> + if (disconnects & (1 << output->connector_id)) {
> + disconnects &= ~(1 << output->connector_id);
> + weston_log("connector %d disconnected\n",
> + output->connector_id);
> + drm_output_destroy(&output->base);
> + }
> + }
> }
> }
>
> @@ -3078,6 +3134,12 @@ renderer_switch_binding(struct weston_keyboard *keyboard, uint32_t time,
> switch_to_gl_renderer(b);
> }
>
> +static const struct weston_drm_output_api api = {
> + drm_output_set_mode,
> + drm_output_set_gbm_format,
> + drm_output_set_seat,
> +};
> +
> static struct drm_backend *
> drm_backend_create(struct weston_compositor *compositor,
> struct weston_drm_backend_config *config)
> @@ -3087,6 +3149,7 @@ drm_backend_create(struct weston_compositor *compositor,
> struct wl_event_loop *loop;
> const char *path;
> const char *seat_id = default_seat;
> + int ret;
>
> weston_log("initializing drm backend\n");
>
> @@ -3107,7 +3170,6 @@ drm_backend_create(struct weston_compositor *compositor,
> b->sprites_are_broken = 1;
> b->compositor = compositor;
> b->use_pixman = config->use_pixman;
> - b->configure_output = config->configure_output;
> b->use_current_mode = config->use_current_mode;
>
> if (parse_gbm_format(config->gbm_format, GBM_FORMAT_XRGB8888, &b->gbm_format) < 0)
> @@ -3230,6 +3292,14 @@ drm_backend_create(struct weston_compositor *compositor,
>
> compositor->backend = &b->base;
>
> + ret = weston_plugin_api_register(compositor, WESTON_DRM_OUTPUT_API_NAME,
> + &api, sizeof(api));
> +
> + if (ret < 0) {
> + weston_log("Failed to register output API.\n");
> + goto err_udev_monitor;
> + }
> +
> return b;
>
> err_udev_monitor:
> diff --git a/libweston/compositor-drm.h b/libweston/compositor-drm.h
> index 1266031..8f89a2b 100644
> --- a/libweston/compositor-drm.h
> +++ b/libweston/compositor-drm.h
> @@ -29,12 +29,13 @@
> #define WESTON_COMPOSITOR_DRM_H
>
> #include "compositor.h"
> +#include "plugin-registry.h"
>
> #ifdef __cplusplus
> extern "C" {
> #endif
>
> -#define WESTON_DRM_BACKEND_CONFIG_VERSION 1
> +#define WESTON_DRM_BACKEND_CONFIG_VERSION 2
>
> struct libinput_device;
>
> @@ -51,8 +52,17 @@ enum weston_drm_backend_output_mode {
> WESTON_DRM_BACKEND_OUTPUT_PREFERRED,
> };
>
> -struct weston_drm_backend_output_config {
> - struct weston_backend_output_config base;
> +#define WESTON_DRM_OUTPUT_API_NAME "weston_drm_output_api_v1"
> +
> +struct weston_drm_output_api {
> + /** The mode to be used by the output. Refer to the documentation
> + * of WESTON_DRM_BACKEND_OUTPUT_PREFERRED for details.
> + *
> + * Returns 0 on success, -1 on failure.
> + */
> + int (*set_mode)(struct weston_output *output,
> + enum weston_drm_backend_output_mode mode,
> + const char *modeline);
>
> /** The pixel format to be used by the output. Valid values are:
> * - NULL - The format set at backend creation time will be used;
> @@ -60,15 +70,26 @@ struct weston_drm_backend_output_config {
> * - "rgb565"
> * - "xrgb2101010"
> */
> - char *gbm_format;
> + void (*set_gbm_format)(struct weston_output *output,
> + const char *gbm_format);
> +
> /** The seat to be used by the output. Set to NULL to use the
> - * default seat. */
> - char *seat;
> - /** The modeline to be used by the output. Refer to the documentation
> - * of WESTON_DRM_BACKEND_OUTPUT_PREFERRED for details. */
> - char *modeline;
> + * default seat.
> + */
> + void (*set_seat)(struct weston_output *output,
> + const char *seat);
> };
>
> +static inline const struct weston_drm_output_api *
> +weston_drm_output_get_api(struct weston_compositor *compositor)
> +{
> + const void *api;
> + api = weston_plugin_api_get(compositor, WESTON_DRM_OUTPUT_API_NAME,
> + sizeof(struct weston_drm_output_api));
> +
> + return (const struct weston_drm_output_api *)api;
> +}
> +
> /** The backend configuration struct.
> *
> * weston_drm_backend_config contains the configuration used by a DRM
> @@ -109,17 +130,6 @@ struct weston_drm_backend_config {
> */
> char *gbm_format;
>
> - /** Callback used to configure the outputs.
> - *
> - * This function will be called by the backend when a new DRM
> - * output needs to be configured.
> - */
> - enum weston_drm_backend_output_mode
> - (*configure_output)(struct weston_compositor *compositor,
> - bool use_current_mode,
> - const char *name,
> - struct weston_drm_backend_output_config *output_config);
> -
> /** Callback used to configure input devices.
> *
> * This function will be called by the backend when a new input device
Nice.
Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20160913/1a272001/attachment-0001.sig>
More information about the wayland-devel
mailing list