[PATCH weston v5 03/11] drm: port the drm backend to the new init api
Bryce Harrington
bryce at osg.samsung.com
Fri Apr 15 18:39:11 UTC 2016
On Fri, Apr 15, 2016 at 02:32:04PM +0200, Benoit Gschwind wrote:
> Hello Bryce and Giulio,
>
> Thanks for your contribution :), here is my comments.
>
> Le 13/04/2016 12:25, Bryce Harrington a écrit :
> > From: Giulio Camuffo <giuliocamuffo at gmail.com>
> >
> > Signed-off-by: Bryce Harrington <bryce at osg.samsung.com>
> > Reviewed-by: Quentin Glidic <sardemff7+git at sardemff7.net>
> > Acked-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> >
> > ---
> > v5:
> > - Update to reflect format rename to gdb_format
> > - Initialize width/height (suggested by pq)
> > - squash drm structure versioning (suggested by pq)
> >
> > Makefile.am | 3 +
> > src/compositor-drm.c | 197 ++++++++++++++++++++-------------------------------
> > src/compositor-drm.h | 111 +++++++++++++++++++++++++++++
> > src/compositor.h | 2 -
> > src/main.c | 96 ++++++++++++++++++++++++-
> > 5 files changed, 283 insertions(+), 126 deletions(-)
> > create mode 100644 src/compositor-drm.h
> >
> > diff --git a/Makefile.am b/Makefile.am
> > index d1644ac..f4cff4c 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -72,6 +72,7 @@ weston_SOURCES = \
> > src/log.c \
> > src/compositor.c \
> > src/compositor.h \
> > + src/compositor-drm.h \
> > src/input.c \
> > src/data-device.c \
> > src/screenshooter.c \
> > @@ -207,6 +208,7 @@ westonincludedir = $(includedir)/weston
> > westoninclude_HEADERS = \
> > src/version.h \
> > src/compositor.h \
> > + src/compositor-drm.h \
> > src/timeline-object.h \
> > shared/matrix.h \
> > shared/config-parser.h \
> > @@ -276,6 +278,7 @@ drm_backend_la_CFLAGS = \
> > $(AM_CFLAGS)
> > drm_backend_la_SOURCES = \
> > src/compositor-drm.c \
> > + src/compositor-drm.h \
> > $(INPUT_BACKEND_SOURCES) \
> > shared/helpers.h \
> > shared/timespec-util.h \
> > diff --git a/src/compositor-drm.c b/src/compositor-drm.c
> > index 119b6c5..508b7e8 100644
> > --- a/src/compositor-drm.c
> > +++ b/src/compositor-drm.c
> > @@ -50,6 +50,7 @@
> > #include "shared/timespec-util.h"
> > #include "libbacklight.h"
> > #include "compositor.h"
> > +#include "compositor-drm.h"
> > #include "gl-renderer.h"
> > #include "pixman-renderer.h"
> > #include "libinput-seat.h"
> > @@ -74,17 +75,6 @@
> > #define GBM_BO_USE_CURSOR GBM_BO_USE_CURSOR_64X64
> > #endif
> >
> > -static int option_current_mode = 0;
> > -
> > -enum output_config {
> > - OUTPUT_CONFIG_INVALID = 0,
> > - OUTPUT_CONFIG_OFF,
> > - OUTPUT_CONFIG_PREFERRED,
> > - OUTPUT_CONFIG_CURRENT,
> > - OUTPUT_CONFIG_MODE,
> > - OUTPUT_CONFIG_MODELINE
> > -};
> > -
> > struct drm_backend {
> > struct weston_backend base;
> > struct weston_compositor *compositor;
> > @@ -130,6 +120,8 @@ struct drm_backend {
> >
> > int32_t cursor_width;
> > int32_t cursor_height;
> > +
> > + struct weston_drm_backend_config *config;
> > };
> >
> > struct drm_mode {
> > @@ -220,13 +212,6 @@ struct drm_sprite {
> > uint32_t formats[];
> > };
> >
> > -struct drm_parameters {
> > - int connector;
> > - int tty;
> > - int use_pixman;
> > - const char *seat_id;
> > -};
> > -
> > static struct gl_renderer_interface *gl_renderer;
> >
> > static const char default_seat[] = "seat0";
> > @@ -2151,31 +2136,23 @@ setup_output_seat_constraint(struct drm_backend *b,
> > }
> >
> > static int
> > -get_gbm_format_from_section(struct weston_config_section *section,
> > - uint32_t default_value,
> > - uint32_t *format)
> > +parse_gbm_format(const char *s, uint32_t default_value, uint32_t *gbm_format)
> > {
> > - char *s;
> > int ret = 0;
> >
> > - weston_config_section_get_string(section,
> > - "gbm-format", &s, NULL);
> > -
> > if (s == NULL)
> > - *format = default_value;
> > + *gbm_format = default_value;
> > else if (strcmp(s, "xrgb8888") == 0)
> > - *format = GBM_FORMAT_XRGB8888;
> > + *gbm_format = GBM_FORMAT_XRGB8888;
> > else if (strcmp(s, "rgb565") == 0)
> > - *format = GBM_FORMAT_RGB565;
> > + *gbm_format = GBM_FORMAT_RGB565;
> > else if (strcmp(s, "xrgb2101010") == 0)
> > - *format = GBM_FORMAT_XRGB2101010;
> > + *gbm_format = GBM_FORMAT_XRGB2101010;
> > else {
> > weston_log("fatal: unrecognized pixel format: %s\n", s);
> > ret = -1;
> > }
> >
> > - free(s);
> > -
> > return ret;
> > }
> >
> > @@ -2194,21 +2171,38 @@ get_gbm_format_from_section(struct weston_config_section *section,
> > * @returns A mode from the output's mode list, or NULL if none available
> > */
> > static struct drm_mode *
> > -drm_output_choose_initial_mode(struct drm_output *output,
> > - enum output_config kind,
> > - int width, int height,
> > - const drmModeModeInfo *current_mode,
> > - const drmModeModeInfo *modeline)
> > +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 drmModeModeInfo *current_mode)
> > {
> > struct drm_mode *preferred = NULL;
> > struct drm_mode *current = NULL;
> > struct drm_mode *configured = NULL;
> > struct drm_mode *best = NULL;
> > struct drm_mode *drm_mode;
> > + drmModeModeInfo 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) {
> > + width = -1;
> > +
> > + if (parse_modeline(config->modeline, &modeline) == 0) {
> > + configured = drm_output_add_mode(output, &modeline);
> > + if (!configured)
> > + return NULL;
> > + } else {
> > + weston_log("Invalid modeline \"%s\" for output %s\n",
> > + config->modeline, output->base.name);
> > + }
> > + }
> > + }
> >
> > wl_list_for_each_reverse(drm_mode, &output->base.mode_list, base.link) {
> > - if (kind == OUTPUT_CONFIG_MODE &&
> > - width == drm_mode->base.width &&
> > + if (width == drm_mode->base.width &&
> > height == drm_mode->base.height)
> > configured = drm_mode;
> >
> > @@ -2222,24 +2216,15 @@ drm_output_choose_initial_mode(struct drm_output *output,
> > best = drm_mode;
> > }
> >
> > - if (kind == OUTPUT_CONFIG_MODELINE) {
> > - configured = drm_output_add_mode(output, modeline);
> > - if (!configured)
> > - return NULL;
> > - }
> > -
> > if (current == NULL && current_mode->clock != 0) {
> > current = drm_output_add_mode(output, current_mode);
> > if (!current)
> > return NULL;
> > }
> >
> > - if (kind == OUTPUT_CONFIG_CURRENT)
> > + if (mode == WESTON_DRM_BACKEND_OUTPUT_CURRENT)
> > configured = current;
> >
> > - if (option_current_mode && current)
> > - return current;
> > -
> > if (configured)
> > return configured;
> >
> > @@ -2303,12 +2288,11 @@ create_output_for_connector(struct drm_backend *b,
> > struct drm_output *output;
> > struct drm_mode *drm_mode, *next, *current;
> > struct weston_mode *m;
> > - struct weston_config_section *section;
> > - drmModeModeInfo crtc_mode, modeline;
> > - int i, width, height, scale;
> > - char *s;
> > - enum output_config config;
> > - uint32_t transform;
> > +
> > + 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) {
> > @@ -2327,42 +2311,14 @@ create_output_for_connector(struct drm_backend *b,
> > output->base.serial_number = "unknown";
> > wl_list_init(&output->base.mode_list);
> >
> > - section = weston_config_get_section(b->compositor->config, "output", "name",
> > - output->base.name);
> > - weston_config_section_get_string(section, "mode", &s, "preferred");
> > - if (strcmp(s, "off") == 0)
> > - config = OUTPUT_CONFIG_OFF;
> > - else if (strcmp(s, "preferred") == 0)
> > - config = OUTPUT_CONFIG_PREFERRED;
> > - else if (strcmp(s, "current") == 0)
> > - config = OUTPUT_CONFIG_CURRENT;
> > - else if (sscanf(s, "%dx%d", &width, &height) == 2)
> > - config = OUTPUT_CONFIG_MODE;
> > - else if (parse_modeline(s, &modeline) == 0)
> > - config = OUTPUT_CONFIG_MODELINE;
> > - else {
> > - weston_log("Invalid mode \"%s\" for output %s\n",
> > - s, output->base.name);
> > - config = OUTPUT_CONFIG_PREFERRED;
> > - }
> > - free(s);
> > -
> > - weston_config_section_get_int(section, "scale", &scale, 1);
> > - weston_config_section_get_string(section, "transform", &s, "normal");
> > - if (weston_parse_transform(s, &transform) < 0)
> > - weston_log("Invalid transform \"%s\" for output %s\n",
> > - s, output->base.name);
> > -
> > - free(s);
> > -
> > - if (get_gbm_format_from_section(section,
> > - b->gbm_format,
> > - &output->gbm_format) == -1)
> > + mode = b->config->configure_output(b->compositor, b->config,
> > + output->base.name, &config);
> > + if (parse_gbm_format(config.gbm_format, b->gbm_format, &output->gbm_format) == -1)
> > output->gbm_format = b->gbm_format;
> >
> > - weston_config_section_get_string(section, "seat", &s, "");
> > - setup_output_seat_constraint(b, &output->base, s);
> > - free(s);
> > + setup_output_seat_constraint(b, &output->base,
> > + config.seat ? config.seat : "");
> > + free(config.seat);
> >
> > output->crtc_id = resources->crtcs[i];
> > output->pipe = i;
> > @@ -2382,16 +2338,15 @@ create_output_for_connector(struct drm_backend *b,
> > goto err_free;
> > }
> >
> > - if (config == OUTPUT_CONFIG_OFF) {
> > + 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(output, config,
> > - width, height,
> > - &crtc_mode, &modeline);
> > + current = drm_output_choose_initial_mode(b, output, mode, &config,
> > + &crtc_mode);
> > if (!current)
> > goto err_free;
> > output->base.current_mode = ¤t->base;
> > @@ -2399,7 +2354,7 @@ create_output_for_connector(struct drm_backend *b,
> >
> > weston_output_init(&output->base, b->compositor, x, y,
> > connector->mmWidth, connector->mmHeight,
> > - transform, scale);
> > + config.base.transform, config.base.scale);
> >
> > if (b->use_pixman) {
> > if (drm_output_init_pixman(output, b) < 0) {
> > @@ -2477,6 +2432,7 @@ err_free:
> > b->crtc_allocator &= ~(1 << output->crtc_id);
> > b->connector_allocator &= ~(1 << output->connector_id);
> > free(output);
> > + free(config.modeline);
> >
> > return -1;
> > }
> > @@ -2744,6 +2700,10 @@ drm_destroy(struct weston_compositor *ec)
> >
> > close(b->drm.fd);
> >
> > + free(b->config->gbm_format);
> > + free(b->config->seat_id);
> > + free(b->config);
> > +
> > free(b);
> > }
> >
> > @@ -3067,15 +3027,13 @@ renderer_switch_binding(struct weston_keyboard *keyboard, uint32_t time,
> >
> > static struct drm_backend *
> > drm_backend_create(struct weston_compositor *compositor,
> > - struct drm_parameters *param,
> > - int *argc, char *argv[],
> > - struct weston_config *config)
> > + struct weston_drm_backend_config *config)
> > {
> > struct drm_backend *b;
> > - struct weston_config_section *section;
> > struct udev_device *drm_device;
> > struct wl_event_loop *loop;
> > const char *path;
> > + const char *seat_id = default_seat;
> >
> > weston_log("initializing drm backend\n");
> >
> > @@ -3095,18 +3053,18 @@ drm_backend_create(struct weston_compositor *compositor,
> > */
> > b->sprites_are_broken = 1;
> > b->compositor = compositor;
> > + b->use_pixman = config->use_pixman;
>
> Duplicate configuration variable, while config structure is kept during
> the backend life (line below), just keep the config->use_pixman.
A later patch in this series destroys config after backend_init is done,
so we do want to copy it into our local config object.
> > + b->config = config;
>
> When I did this with headless and x11 drm, a question raised: "Should I
> copy or should I take the ownership ?". Taking the ownership leave to
> the developer the possibility to change the configuration easily while
> running. Maybe some documentation may explicit that it's forbidden to
> change configuration after run is started.
Right, the "Enforce destruction of all backend config objects" patch
later in this series adds a doxygen code comment for load_backend_new()
to document that the backend is not to take ownership of the config
object.
I felt it is cleaner and perhaps safer for the backend to not reference
the config object after the backend_init() call. If in the future there
are major version changes (i.e. non-backwards compatible changes to the
structure definition), we may need to add messy compatibility
checks/corrections. By not hanging onto the config object at least we
can deal with that all in the backend_init()/drm_backend_create()
functions and keep the remainder of the backend code version-agnostic.
> >
> > - section = weston_config_get_section(config, "core", NULL, NULL);
> > - if (get_gbm_format_from_section(section,
> > - GBM_FORMAT_XRGB8888,
> > - &b->gbm_format) == -1)
> > - goto err_base;
> > + if (parse_gbm_format(config->gbm_format, GBM_FORMAT_XRGB8888, &b->gbm_format) < 0)
> > + goto err_compositor;
> >
> > - b->use_pixman = param->use_pixman;
> > + if (config->seat_id)
> > + seat_id = config->seat_id;
> >
> > /* Check if we run drm-backend using weston-launch */
> > - compositor->launcher = weston_launcher_connect(compositor, param->tty,
> > - param->seat_id, true);
> > + compositor->launcher = weston_launcher_connect(compositor, config->tty,
> > + seat_id, true);
> > if (compositor->launcher == NULL) {
> > weston_log("fatal: drm backend should be run "
> > "using weston-launch binary or as root\n");
> > @@ -3122,7 +3080,7 @@ drm_backend_create(struct weston_compositor *compositor,
> > b->session_listener.notify = session_notify;
> > wl_signal_add(&compositor->session_signal, &b->session_listener);
> >
> > - drm_device = find_primary_gpu(b, param->seat_id);
> > + drm_device = find_primary_gpu(b, seat_id);
> > if (drm_device == NULL) {
> > weston_log("no drm device found\n");
> > goto err_udev;
> > @@ -3148,6 +3106,7 @@ drm_backend_create(struct weston_compositor *compositor,
> >
> > b->base.destroy = drm_destroy;
> > b->base.restore = drm_restore;
> > + b->base.create_output = NULL;
>
> b->base.create_output variable above look obsolete. Should be removed.
Yeah you may be right. I need to look at this more, but on cursory
review I don't spot anything actually using it.
> > b->prev_state = WESTON_COMPOSITOR_ACTIVE;
> >
> > @@ -3157,12 +3116,12 @@ drm_backend_create(struct weston_compositor *compositor,
> > create_sprites(b);
> >
> > if (udev_input_init(&b->input,
> > - compositor, b->udev, param->seat_id) < 0) {
> > + compositor, b->udev, seat_id) < 0) {
> > weston_log("failed to create input devices\n");
> > goto err_sprite;
> > }
> >
> > - if (create_outputs(b, param->connector, drm_device) < 0) {
> > + if (create_outputs(b, config->connector, drm_device) < 0) {
> > weston_log("failed to create output for %s\n", path);
> > goto err_udev_input;
> > }
> > @@ -3237,32 +3196,26 @@ err_udev:
> > udev_unref(b->udev);
> > err_compositor:
> > weston_compositor_shutdown(compositor);
> > -err_base:
> > free(b);
> > return NULL;
> > }
> >
> > WL_EXPORT int
> > backend_init(struct weston_compositor *compositor, int *argc, char *argv[],
> > - struct weston_config *config,
> > + struct weston_config *wc,
> > struct weston_backend_config *config_base)
> > {
> > struct drm_backend *b;
> > - struct drm_parameters param = { 0, };
> > -
> > - const struct weston_option drm_options[] = {
> > - { WESTON_OPTION_INTEGER, "connector", 0, ¶m.connector },
> > - { WESTON_OPTION_STRING, "seat", 0, ¶m.seat_id },
> > - { WESTON_OPTION_INTEGER, "tty", 0, ¶m.tty },
> > - { WESTON_OPTION_BOOLEAN, "current-mode", 0, &option_current_mode },
> > - { WESTON_OPTION_BOOLEAN, "use-pixman", 0, ¶m.use_pixman },
> > - };
> > + struct weston_drm_backend_config *config;
> >
> > - param.seat_id = default_seat;
> > + if (config_base == NULL ||
> > + config_base->struct_version != 1 ||
> > + config_base->struct_size > sizeof(struct weston_drm_backend_config))
> > + return -1;
> >
> > - parse_options(drm_options, ARRAY_LENGTH(drm_options), argc, argv);
> > + config = (struct weston_drm_backend_config *)config_base;
> >
> > - b = drm_backend_create(compositor, ¶m, argc, argv, config);
> > + b = drm_backend_create(compositor, config);
> > if (b == NULL)
> > return -1;
> > return 0;
> > diff --git a/src/compositor-drm.h b/src/compositor-drm.h
> > new file mode 100644
> > index 0000000..423378a
> > --- /dev/null
> > +++ b/src/compositor-drm.h
> > @@ -0,0 +1,111 @@
> > +/*
> > + * Copyright © 2008-2011 Kristian Høgsberg
> > + * Copyright © 2011 Intel Corporation
> > + * Copyright © 2015 Giulio Camuffo
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining
> > + * a copy of this software and associated documentation files (the
> > + * "Software"), to deal in the Software without restriction, including
> > + * without limitation the rights to use, copy, modify, merge, publish,
> > + * distribute, sublicense, and/or sell copies of the Software, and to
> > + * permit persons to whom the Software is furnished to do so, subject to
> > + * the following conditions:
> > + *
> > + * The above copyright notice and this permission notice (including the
> > + * next paragraph) shall be included in all copies or substantial
> > + * portions of the Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> > + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> > + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
> > + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> > + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> > + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> > + * SOFTWARE.
> > + */
> > +
> > +#ifndef WESTON_COMPOSITOR_DRM_H
> > +#define WESTON_COMPOSITOR_DRM_H
> > +
> > +#ifdef __cplusplus
> > +extern "C" {
> > +#endif
> > +
> > +#include "compositor.h"
> > +
> > +enum weston_drm_backend_output_mode {
> > + /** The output is disabled */
> > + WESTON_DRM_BACKEND_OUTPUT_OFF,
> > + /** The output will use the current active mode */
> > + WESTON_DRM_BACKEND_OUTPUT_CURRENT,
> > + /** The output will use the preferred mode. A modeline can be provided
> > + * by setting weston_backend_output_config::modeline in the form of
> > + * "WIDTHxHEIGHT" or in the form of an explicit modeline calculated
> > + * using e.g. the cvt tool. If a valid modeline is supplied it will be
> > + * used, if invalid or NULL the preferred available mode will be used. */
> > + WESTON_DRM_BACKEND_OUTPUT_PREFERRED,
> > +};
> > +
> > +struct weston_drm_backend_output_config {
> > + struct weston_backend_output_config base;
> > +
> > + /** The pixel format to be used by the output. Valid values are:
> > + * - NULL - The format set at backend creation time will be used;
> > + * - "xrgb8888";
> > + * - "rgb565"
> > + * - "xrgb2101010"
> > + */
> > + char *gbm_format;
>
> Why do not pass gbm_format as enum?
It looks like it's because this gets filled out by a routine in main.c,
which doesn't know about the gbm enum definition.
However, perhaps that routine can be moved out of main.c. If it can,
then we may be able to at least track it as an integer rather than
string. I think we could only store it as an enum if this structure can
be made entirely private. (Or, obviously, we could make main.c aware of
the enum definition, but I'd kind of prefer keeping backend-specific
things encapsulated in the backend as much as possible.)
> > + /** 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;
>
> Maybe enum with drmModeModeInfo* is better API than free string API?
Again, since the struct is used in main.c it may not know about that
enum. But if the drm output config routine can be moved from main.c to
the backend, then that could be done.
> > +};
> > +
> > +/** The backend configuration struct.
> > + *
> > + * weston_drm_backend_config contains the configuration used by a DRM
> > + * backend. The backend will take ownership of the weston_backend_config
> > + * object passed to it on initialization and will free it on destruction. */
> > +struct weston_drm_backend_config {
> > + struct weston_backend_config base;
> > +
> > + /** The connector id of the output to be initialized. A value of 0 will
> > + * enable all available outputs. */
> > + int connector;
> > + /** The tty to be used. Set to 0 to use the current tty. */
> > + int tty;
> > + /** If true the pixman renderer will be used instead of the OpenGL ES
> > + * renderer. */
> > + bool use_pixman;
> > + /** The seat to be used for input and output. If NULL the default "seat0"
> > + * will be used.
> > + * The backend will take ownership of the seat_id pointer and will free
> > + * it on backend destruction. */
> > + char *seat_id;
> > + /** The pixel format of the framebuffer to be used. Valid values are:
> > + * - NULL - The default format ("xrgb8888") will be used;
> > + * - "xrgb8888";
> > + * - "rgb565"
> > + * - "xrgb2101010"
> > + * The backend will take ownership of the format pointer and will free
> > + * it on backend destruction. */
> > + char *gbm_format;
>
> Why do not pass gbm_format as enum?
As with the others, this is filled in by main.c which doesn't know about
the enum definitions.
However, unlike the other cases this structure has to remain visible in
main.c so I think this one has to be kept as a string. Fortunately,
this string only will need to be parsed one time during backend_init and
we should be able to consistently use the enum value subsequently to
that.
> > + /** 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,
> > + struct weston_drm_backend_config *backend_config,
> > + const char *name,
> > + struct weston_drm_backend_output_config *output_config);
> > +};
> > +
> > +#ifdef __cplusplus
> > +}
> > +#endif
> > +
> > +#endif
> > diff --git a/src/compositor.h b/src/compositor.h
> > index 5654b04..f93d76a 100644
> > --- a/src/compositor.h
> > +++ b/src/compositor.h
> > @@ -673,8 +673,6 @@ enum weston_capability {
> > */
> > struct weston_backend_output_config {
> > uint32_t transform;
> > - uint32_t width;
> > - uint32_t height;
> > uint32_t scale;
> > };
> >
> > diff --git a/src/main.c b/src/main.c
> > index 43de354..807bd4d 100644
> > --- a/src/main.c
> > +++ b/src/main.c
> > @@ -47,6 +47,8 @@
> > #include "git-version.h"
> > #include "version.h"
> >
> > +#include "compositor-drm.h"
> > +
> > static struct wl_list child_process_list;
> > static struct weston_compositor *segv_compositor;
> >
> > @@ -670,13 +672,103 @@ load_backend_new(struct weston_compositor *compositor, const char *backend,
> > return backend_init(compositor, NULL, NULL, NULL, config_base);
> > }
> >
> > +struct drm_config {
> > + struct weston_drm_backend_config base;
> > + bool use_current_mode;
> > +};
> > +
> > +static enum weston_drm_backend_output_mode
> > +drm_configure_output(struct weston_compositor *c,
> > + struct weston_drm_backend_config *backend_config,
> > + const char *name,
> > + struct weston_drm_backend_output_config *config)
> > +{
> > + struct drm_config *drm_config = (struct drm_config *)backend_config;
> > + struct weston_config *wc = weston_compositor_get_user_data(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 (drm_config->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 int
> > +load_drm_backend(struct weston_compositor *c, const char *backend,
> > + int *argc, char **argv, struct weston_config *wc)
> > +{
> > + struct drm_config *config;
> > + struct weston_config_section *section;
> > + int ret = 0;
> > +
> > + config = zalloc(sizeof *config);
> > + if (!config)
> > + return -1;
> > +
> > + const struct weston_option options[] = {
> > + { WESTON_OPTION_INTEGER, "connector", 0, &config->base.connector },
> > + { WESTON_OPTION_STRING, "seat", 0, &config->base.seat_id },
> > + { WESTON_OPTION_INTEGER, "tty", 0, &config->base.tty },
> > + { WESTON_OPTION_BOOLEAN, "current-mode", 0,
> > + &config->use_current_mode },
> > + { WESTON_OPTION_BOOLEAN, "use-pixman", 0, &config->base.use_pixman },
> > + };
> > +
> > + parse_options(options, ARRAY_LENGTH(options), argc, argv);
> > +
> > + section = weston_config_get_section(wc, "core", NULL, NULL);
> > + weston_config_section_get_string(section,
> > + "gbm-format", &config->base.gbm_format,
> > + NULL);
> > +
> > + config->base.base.struct_version = 1;
> > + config->base.base.struct_size = sizeof(struct weston_drm_backend_config);
> > + config->base.configure_output = drm_configure_output;
> > +
> > + if (load_backend_new(c, backend, &config->base.base) < 0) {
> > + ret = -1;
> > + free(config->base.gbm_format);
> > + free(config->base.seat_id);
> > + free(config);
> > + }
> > +
> > + return ret;
> > +}
> > +
> > static int
> > load_backend(struct weston_compositor *compositor, const char *backend,
> > int *argc, char **argv, struct weston_config *config)
> > {
> > -#if 0
> > if (strstr(backend, "drm-backend.so"))
> > return load_drm_backend(compositor, backend, argc, argv, config);
> > +#if 0
> > else if (strstr(backend, "wayland-backend.so"))
> > return load_wayland_backend(compositor, backend, argc, argv, config);
> > else if (strstr(backend, "x11-backend.so"))
> > @@ -785,7 +877,7 @@ int main(int argc, char *argv[])
> > backend = weston_choose_default_backend();
> > }
> >
> > - ec = weston_compositor_create(display, NULL);
> > + ec = weston_compositor_create(display, config);
> > if (ec == NULL) {
> > weston_log("fatal: failed to create compositor\n");
> > goto out;
> >
>
>
> I tested the patch above previous from the patch set and it work for me
> with simple configuration.
>
> I leave the community to choose which comments should be fixed.
>
> Tested-by: Benoit Gschwind <gschwind at gnu-log.net>
Thanks Benoit!
Bryce
More information about the wayland-devel
mailing list