Re: [RFC weston 4/5] drm: Move backend to libweston
Benoit Gschwind
gschwind at gnu-log.net
Fri Feb 12 17:45:08 UTC 2016
Hello,
Le 09/02/2016 16:14, Quentin Glidic a écrit :
> From: Quentin Glidic <sardemff7+git at sardemff7.net>
>
> Signed-off-by: Quentin Glidic <sardemff7+git at sardemff7.net>
> ---
> Makefile.am | 10 ++---
> {src => lib}/compositor-drm.c | 87 +++++++++++++++++++------------------------
> {src => lib}/libbacklight.c | 0
> {src => lib}/libbacklight.h | 0
> lib/libweston.c | 23 ++++++++++++
> lib/libweston.h | 1 +
> src/main.c | 3 ++
> 7 files changed, 71 insertions(+), 53 deletions(-)
> rename {src => lib}/compositor-drm.c (97%)
> rename {src => lib}/libbacklight.c (100%)
> rename {src => lib}/libbacklight.h (100%)
>
> diff --git a/Makefile.am b/Makefile.am
> index 36078ba..68997d8 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -287,12 +287,12 @@ drm_backend_la_CFLAGS = \
> $(DRM_COMPOSITOR_CFLAGS) \
> $(AM_CFLAGS)
> drm_backend_la_SOURCES = \
> - src/compositor-drm.c \
> + lib/compositor-drm.c \
> $(INPUT_BACKEND_SOURCES) \
> shared/helpers.h \
> shared/timespec-util.h \
> - src/libbacklight.c \
> - src/libbacklight.h
> + lib/libbacklight.c \
> + lib/libbacklight.h
>
> if ENABLE_VAAPI_RECORDER
> drm_backend_la_SOURCES += src/vaapi-recorder.c src/vaapi-recorder.h
> @@ -1325,8 +1325,8 @@ if BUILD_SETBACKLIGHT
> noinst_PROGRAMS += setbacklight
> setbacklight_SOURCES = \
> tests/setbacklight.c \
> - src/libbacklight.c \
> - src/libbacklight.h
> + lib/libbacklight.c \
> + lib/libbacklight.h
> setbacklight_CFLAGS = $(AM_CFLAGS) $(SETBACKLIGHT_CFLAGS)
> setbacklight_LDADD = $(SETBACKLIGHT_LIBS)
> endif
> diff --git a/src/compositor-drm.c b/lib/compositor-drm.c
> similarity index 97%
> rename from src/compositor-drm.c
> rename to lib/compositor-drm.c
> index 04de95e..5d3cd0c 100644
> --- a/src/compositor-drm.c
> +++ b/lib/compositor-drm.c
> @@ -46,7 +46,7 @@
> #include <gbm.h>
> #include <libudev.h>
>
> -#include "libweston.h"
> +#include "libweston-internal.h"
> #include "shared/helpers.h"
> #include "shared/timespec-util.h"
> #include "libbacklight.h"
> @@ -88,6 +88,7 @@ enum output_config {
>
> struct drm_backend {
> struct weston_backend base;
> + struct libweston_context *context;
> struct weston_compositor *compositor;
>
> struct udev *udev;
> @@ -2144,15 +2145,15 @@ 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)
> +get_gbm_format_from_config(struct libweston_context *context,
> + const char *section,
> + uint32_t default_value,
> + uint32_t *format)
> {
> char *s;
> int ret = 0;
>
> - weston_config_section_get_string(section,
> - "gbm-format", &s, NULL);
> + s = context->backend_config.string_getter(section, "gbm-format", NULL, context->backend_config.user_data);
>
> if (s == NULL)
> *format = default_value;
> @@ -2296,7 +2297,7 @@ create_output_for_connector(struct drm_backend *b,
> struct drm_output *output;
> struct drm_mode *drm_mode, *next, *current;
> struct weston_mode *m;
[REF#1]
> - struct weston_config_section *section;
> + char section[39];
> drmModeModeInfo crtc_mode, modeline;
> int i, width, height, scale;
> char *s;
> @@ -2320,9 +2321,9 @@ create_output_for_connector(struct drm_backend *b,
> output->base.serial_number = "unknown";
> wl_list_init(&output->base.mode_list);
>
The _following changes_ seems wrong, in previous version, the function
weston_config_get_section will try to find a section with a 'name' key
and where the value of this 'name' equals to output->base.name. You
replaced it with b->context->backend_config.string_getter, where first
section is undefined see [REF#1]:
- will probably always fallback to default values when not segfault,
- even if section were fine, this approach does not handle severals
outputs.
This case show that this approach is not very good, In that case the
developer must give a list of unbound outputs setup and key/value is not
well suited for that case. (there is some workaround, like using key
pattern).
This case also leave me to add that section/key/value is not a good
choice and you should stick to key/value.
I still prefer the opaque configuration structure with function to set
the structure content.
Best regards.
> - section = weston_config_get_section(b->compositor->config, "output", "name",
> - output->base.name);
> - weston_config_section_get_string(section, "mode", &s, "preferred");
> + snprintf(section, sizeof section, "output %s", output->base.name);
> +
> + s = b->context->backend_config.string_getter(section, "mode", "preferred", b->context->backend_config.user_data);
> if (strcmp(s, "off") == 0)
> config = OUTPUT_CONFIG_OFF;
> else if (strcmp(s, "preferred") == 0)
> @@ -2340,20 +2341,21 @@ create_output_for_connector(struct drm_backend *b,
> }
> free(s);
>
> - weston_config_section_get_int(section, "scale", &scale, 1);
> - weston_config_section_get_string(section, "transform", &s, "normal");
> + scale = b->context->backend_config.int_getter(section, "scale", 1, b->context->backend_config.user_data);
> + s = b->context->backend_config.string_getter(section, "transform", "normal", b->context->backend_config.user_data);
> 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->format,
> - &output->format) == -1)
> + if (get_gbm_format_from_config(b->context,
> + section,
> + b->format,
> + &output->format) == -1)
> output->format = b->format;
>
> - weston_config_section_get_string(section, "seat", &s, "");
> + s = b->context->backend_config.string_getter(section, "seat", "", b->context->backend_config.user_data);
> setup_output_seat_constraint(b, &output->base, s);
> free(s);
>
> @@ -3058,16 +3060,16 @@ 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)
> +drm_backend_create(struct libweston_context *context)
> {
> + struct weston_compositor *compositor = context->compositor;
> struct drm_backend *b;
> - struct weston_config_section *section;
> struct udev_device *drm_device;
> struct wl_event_loop *loop;
> const char *path;
> + int tty;
> + uint32_t connector;
> + const char *seat_id = default_seat;
>
> weston_log("initializing drm backend\n");
>
> @@ -3075,6 +3077,8 @@ drm_backend_create(struct weston_compositor *compositor,
> if (b == NULL)
> return NULL;
>
> + b->context = context;
> +
> /*
> * KMS support for hardware planes cannot properly synchronize
> * without nuclear page flip. Without nuclear/atomic, hw plane
> @@ -3088,17 +3092,19 @@ drm_backend_create(struct weston_compositor *compositor,
> b->sprites_are_broken = 1;
> b->compositor = compositor;
>
> - section = weston_config_get_section(config, "core", NULL, NULL);
> - if (get_gbm_format_from_section(section,
> - GBM_FORMAT_XRGB8888,
> - &b->format) == -1)
> + if (get_gbm_format_from_config(context,
> + "core",
> + GBM_FORMAT_XRGB8888,
> + &b->format) == -1)
> goto err_base;
>
> - b->use_pixman = param->use_pixman;
> + b->use_pixman = context->backend_config.bool_getter(NULL, "use-pixman", false, context->backend_config.user_data);
> + tty = context->backend_config.int_getter(NULL, "tty", 0, context->backend_config.user_data);
> + connector = context->backend_config.int_getter(NULL, "connector", 0, context->backend_config.user_data);
>
> /* 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, tty,
> + seat_id, true);
> if (compositor->launcher == NULL) {
> weston_log("fatal: drm backend should be run "
> "using weston-launch binary or as root\n");
> @@ -3114,7 +3120,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;
> @@ -3149,12 +3155,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, connector, drm_device) < 0) {
> weston_log("failed to create output for %s\n", path);
> goto err_udev_input;
> }
> @@ -3235,26 +3241,11 @@ err_base:
> }
>
> WL_EXPORT int
> -backend_init(struct weston_compositor *compositor, int *argc, char *argv[],
> - struct weston_config *config,
> - struct weston_backend_config *config_base)
> +libweston_backend_init(struct libweston_context *context)
> {
> 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 },
> - };
> -
> - param.seat_id = default_seat;
> -
> - parse_options(drm_options, ARRAY_LENGTH(drm_options), argc, argv);
>
> - b = drm_backend_create(compositor, ¶m, argc, argv, config);
> + b = drm_backend_create(context);
> if (b == NULL)
> return -1;
> return 0;
> diff --git a/src/libbacklight.c b/lib/libbacklight.c
> similarity index 100%
> rename from src/libbacklight.c
> rename to lib/libbacklight.c
> diff --git a/src/libbacklight.h b/lib/libbacklight.h
> similarity index 100%
> rename from src/libbacklight.h
> rename to lib/libbacklight.h
> diff --git a/lib/libweston.c b/lib/libweston.c
> index da21f8e..dd77f6b 100644
> --- a/lib/libweston.c
> +++ b/lib/libweston.c
> @@ -69,8 +69,31 @@ libweston_load_module(const char *name, const char *entrypoint)
> return init;
> }
>
> +static enum libweston_backend
> +load_backend(struct libweston_context *context, const char *backend)
> +{
> + int (*backend_init)(struct libweston_context *context);
> +
> + backend_init = libweston_load_module(backend, "libweston_backend_init");
> + if (!backend_init)
> + return -1;
> +
> + return backend_init(context);
> +
> +}
> +
> WL_EXPORT int
> libweston_load_backend(struct libweston_context *context, enum libweston_backend preffered)
> {
> + if (preffered == LIBWESTON_BACKEND_NONE) {
> + preffered = LIBWESTON_BACKEND_DRM;
> + }
> +
> + switch (preffered) {
> + case LIBWESTON_BACKEND_DRM:
> + return load_backend(context, "drm-backend.so");
> + case LIBWESTON_BACKEND_NONE:
> + break;
> + }
> return -1;
> }
> diff --git a/lib/libweston.h b/lib/libweston.h
> index 7a995b8..3950316 100644
> --- a/lib/libweston.h
> +++ b/lib/libweston.h
> @@ -14,6 +14,7 @@ void *libweston_load_module(const char *name, const char *entrypoint);
>
> enum libweston_backend {
> LIBWESTON_BACKEND_NONE = 0,
> + LIBWESTON_BACKEND_DRM,
> };
> int libweston_load_backend(struct libweston_context *context, enum libweston_backend preffered);
>
> diff --git a/src/main.c b/src/main.c
> index 3e52168..c03835a 100644
> --- a/src/main.c
> +++ b/src/main.c
> @@ -660,6 +660,9 @@ load_backend(struct weston_compositor *compositor, const char *backend,
> {
> enum libweston_backend libweston_backend = LIBWESTON_BACKEND_NONE;
>
> + if (strstr(backend, "drm"))
> + libweston_backend = LIBWESTON_BACKEND_DRM;
> +
> if (libweston_load_backend(compositor->libweston, libweston_backend) == 0)
> return 0;
>
>
More information about the wayland-devel
mailing list