[RFC] [PATCH weston] Allow output configuration from config file.
Kristian Høgsberg
hoegsberg at gmail.com
Mon Jul 30 17:44:19 PDT 2012
On Mon, Jul 30, 2012 at 05:55:12PM -0600, Scott Moreau wrote:
> Parse the config file for [output] sections and check for 'name'
> and 'mode' keys. The key strings are compared to what is reported
> by weston log. The 'mode' key string can be one of the following:
>
> 1) WIDTHxHEIGHT - One that is reported by weston log, e.g. 1280x1024
> 2) off - Disables the output
> 3) native - Uses the native mode of the output
> 4) current - Uses the mode currently driving the crtc
This looks good, I'm tempted to just apply it as is, but I just have a
couple of nitpicks below. We should stick to "preferred" and not call
it "native", both KMS and xrandr uses "preferred".
> ---
> src/compositor-drm.c | 98 +++++++++++++++++++++++++++++++++++++++++++++++++++-
> weston.ini | 11 +++---
> 2 files changed, 103 insertions(+), 6 deletions(-)
>
> diff --git a/src/compositor-drm.c b/src/compositor-drm.c
> index cd0395e..da75937 100644
> --- a/src/compositor-drm.c
> +++ b/src/compositor-drm.c
> @@ -44,6 +44,16 @@
> #include "log.h"
>
> static int option_current_mode = 0;
> +static char *output_name;
> +static char *output_mode;
> +static struct wl_list configured_output_list;
> +
> +struct drm_configured_output {
> + char *name;
> + char *mode;
> + int32_t width, height;
> + struct wl_list link;
> +};
>
>
> enum {
> WESTON_PLANE_DRM_CURSOR = 0x100,
> @@ -1296,7 +1306,8 @@ create_output_for_connector(struct drm_compositor *ec,
> {
> struct drm_output *output;
> struct drm_mode *drm_mode, *next;
> - struct weston_mode *m, *preferred, *current;
> + struct weston_mode *m, *preferred, *current, *configured;
> + struct drm_configured_output *o = NULL, *temp;
> drmModeEncoder *encoder;
> drmModeModeInfo crtc_mode;
> drmModeCrtc *crtc;
> @@ -1356,13 +1367,40 @@ create_output_for_connector(struct drm_compositor *ec,
>
> preferred = NULL;
> current = NULL;
> + configured = NULL;
> +
> + wl_list_for_each(temp, &configured_output_list, link) {
> + if (strcmp(temp->name, output->name) == 0) {
> + weston_log("Matched output from config file: %s\n", temp->name);
> + o = temp;
> + break;
> + }
> + }
> +
> + if (o && strcmp("off", o->mode) == 0) {
> + weston_log("%s mode \"%s\" in config\n", o->name, o->mode);
I think just the weston_log() above when we find o is enough, just log
both o->mode and o->name there,
> + weston_log("Disabling output %s\n", o->name);
> +
> + drmModeSetCrtc(ec->drm.fd, output->crtc_id,
> + 0, 0, 0, 0, 0, NULL);
> + goto err_free;
> + }
> +
> wl_list_for_each(drm_mode, &output->base.mode_list, base.link) {
> + if (o && o->width == drm_mode->base.width &&
> + o->height == drm_mode->base.height) {
> + weston_log("Using mode %s for output %s\n", o->mode, o->name);
and then this one shouldn't be necessary either
> + configured = &drm_mode->base;
> + }
> if (!memcmp(&crtc_mode, &drm_mode->mode_info, sizeof crtc_mode))
> current = &drm_mode->base;
> if (drm_mode->base.flags & WL_OUTPUT_MODE_PREFERRED)
> preferred = &drm_mode->base;
> }
>
> + if (o && configured == NULL)
> + weston_log("Using %s mode for output %s\n", o->mode, o->name);
> +
nor this one.
> if (current == NULL && crtc_mode.clock != 0) {
> ret = drm_output_add_mode(output, &crtc_mode);
> if (ret)
> @@ -1371,8 +1409,16 @@ create_output_for_connector(struct drm_compositor *ec,
> struct weston_mode, link);
> }
>
> + if (configured == NULL && (o && strcmp("current", o->mode) == 0))
> + configured = current;
No need to check configure here, if it's not NULL, it's because we
matched a mode above. Instead of this and the "native" check below,
I'd just set configured to current if o->config is
OUTPUT_CONFIG_CURRENT (see below) else if OUTPUT_CONFIG_PREFERRED, set
it to preferred...
> + if (preferred == NULL)
> + preferred = current;
(I got rid of these two lines in previous commit, they're not necessary now)
> if (option_current_mode && current)
> output->base.current = current;
> + else if (configured && (o && strcmp("native", o->mode) != 0))
> + output->base.current = configured;
and then here we can just say
else if (configured)
output->base.current = configured;
...
> else if (preferred)
> output->base.current = preferred;
> else if (current)
> @@ -1712,9 +1758,12 @@ drm_destroy(struct weston_compositor *ec)
> {
> struct drm_compositor *d = (struct drm_compositor *) ec;
> struct weston_seat *seat, *next;
> + struct drm_configured_output *o, *n;
>
> wl_list_for_each_safe(seat, next, &ec->seat_list, link)
> evdev_input_destroy(seat);
> + wl_list_for_each_safe(o, n, &configured_output_list, link)
> + free(o);
>
> wl_event_source_remove(d->udev_drm_source);
> wl_event_source_remove(d->drm_source);
> @@ -1982,6 +2031,38 @@ err_base:
> return NULL;
> }
>
> +static void
> +output_section_done(void *data)
> +{
> + struct drm_configured_output *output;
> + int valid_mode_key = 0;
> +
> + output = malloc(sizeof *output);
> +
> + if (!output)
> + return;
> +
> + if (sscanf(output_mode, "%dx%d", &output->width, &output->height) == 2)
> + valid_mode_key = 1;
> + else
> + output->width = output->height = 0;
> +
> + if (strcmp(output_mode, "off") == 0 ||
> + strcmp(output_mode, "native") == 0 ||
> + strcmp(output_mode, "current") == 0) {
> + valid_mode_key = 1;
> + }
> +
> + if (valid_mode_key) {
> + output->name = output_name;
> + output->mode = output_mode;
> +
> + wl_list_insert(&configured_output_list, &output->link);
> + } else
> + weston_log("Ignoring malformed mode \"%s\" for output %s\n",
> + output_mode, output_name);
Can we introduce an output_config enum with the values
OUTPUT_CONFIG_CURRENT, OUTPUT_CONFIG_PREFERRED, OUTPUT_CONFIG_OFF and
OUTPUT_CONFIG_MODE and do the strcmp when we parse the config section
and just store the mode in drm_configured_output?
output->config = OUTPUT_CONFIG_INVALID;
if (sscanf(output_mode, "%dx%d", &output->width, &output->height) == 2)
output->config = OUTPUT_CONFIG_MODE;
else if (strcmp(output_mode, "off") == 0)
output->config = OUTPUT_CONFIG_OFF;
etc
and then
if (output->config != OUTPUT_CONFIG_INVALID)
wl_list_insert(&configured_output_list, &output->link);
else {
free(output);
weston_log();
}
but keep output->mode so we can use it in the log.
> +}
> +
> WL_EXPORT struct weston_compositor *
> backend_init(struct wl_display *display, int argc, char *argv[],
> const char *config_file)
> @@ -1998,6 +2079,21 @@ backend_init(struct wl_display *display, int argc, char *argv[],
>
> parse_options(drm_options, ARRAY_LENGTH(drm_options), argc, argv);
>
> + wl_list_init(&configured_output_list);
> +
> + const struct config_key drm_config_keys[] = {
> + { "name", CONFIG_KEY_STRING, &output_name },
> + { "mode", CONFIG_KEY_STRING, &output_mode },
> + };
> +
> + const struct config_section config_section[] = {
> + { "output", drm_config_keys,
> + ARRAY_LENGTH(drm_config_keys), output_section_done },
> + };
> +
> + parse_config_file(config_file, config_section,
> + ARRAY_LENGTH(config_section), NULL);
> +
> return drm_compositor_create(display, connector, seat, tty, argc, argv,
> config_file);
> }
> diff --git a/weston.ini b/weston.ini
> index f736c8a..375f4ee 100644
> --- a/weston.ini
> +++ b/weston.ini
> @@ -34,9 +34,10 @@ path=./clients/flower
> path=/usr/libexec/weston-screensaver
> duration=600
>
> -[output]
> -name=LVDS1
> -mode=off
> +#[output]
> +#name=LVDS1
> +#mode=off
>
> -mode=1366x768
> -modeline=36.25 912 936 1024 1136 512 515 525 533 -hsync +vsync
> +#[output]
> +#name=VGA1
> +#mode=1280x1024
> --
> 1.7.11.2
>
> _______________________________________________
> 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