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@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/wayland-devel _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel