Re: [RFC weston 4/5] drm: Move backend to libweston
Benoit Gschwind
gschwind at gnu-log.net
Sat Feb 13 10:22:46 UTC 2016
Hi,
Le 12/02/2016 18:45, Benoit Gschwind a écrit :
> 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);
>>
Following a comment from Quentin Glidic on IRC, I read wrong the
snprintf (I readed printf ... a shame, my apology). Which make the patch
valid if output->base.name is not too long.
>
> 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.
But I keep the following:
>
> 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;
>>
>>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
More information about the wayland-devel
mailing list