Hello Bryce, I think my comments on previous patch apply here, while you did some useful update here.
Best regards. Le 30/04/2016 00:40, Bryce Harrington a écrit : > The drm backend was copying most everything out of the config object > already, but now also copy the use_current_mode parameter and the > config_output function pointer, so that there are no remaining > references to the config object passed into backend_init(). > > By decoupling the config struct to the backend, if in the future if the > structure definition changes in non-backwards compatible ways, then any > version compatibility adaptation will be limited to just the > backend_init() routine. > > With the use_current_mode moved into the main config class, the > drm_config wrapper is redundant. Dropping it helps make the drm backend > config initialization more consistent with the other backends. > > Also, enforce destruction of the backend config object after > initialization. Since the backend config struct versioning implies that > there we expect potential future descrepancy between main's definition > of the config object and the backend's, don't allow the backend to hang > onto the config object outside the initialization scope. > > Signed-off-by: Bryce Harrington <[email protected]> > Reviewed-by: Pekka Paalanen <[email protected]> > > --- > v6: > - Drop use of drm_config config wrapper > v7: > - Update to master > - Put backend configs on stack instead of zalloc() > - Enforce destruction of backend config object > (Squashed patch as requested by pq) > > src/compositor-drm.c | 24 +++++++++++++++--------- > src/compositor-drm.h | 3 ++- > src/main.c | 46 ++++++++++++++++------------------------------ > 3 files changed, 33 insertions(+), 40 deletions(-) > > diff --git a/src/compositor-drm.c b/src/compositor-drm.c > index 2384fd2..6ef706a 100644 > --- a/src/compositor-drm.c > +++ b/src/compositor-drm.c > @@ -121,7 +121,17 @@ struct drm_backend { > int32_t cursor_width; > int32_t cursor_height; > > - struct weston_drm_backend_config *config; > + /** 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, > + bool use_current_mode, > + const char *name, > + struct weston_drm_backend_output_config > *output_config); > + bool use_current_mode; > }; > > struct drm_mode { > @@ -2311,8 +2321,8 @@ create_output_for_connector(struct drm_backend *b, > output->base.serial_number = "unknown"; > wl_list_init(&output->base.mode_list); > > - mode = b->config->configure_output(b->compositor, b->config, > - output->base.name, &config); > + mode = b->configure_output(b->compositor, b->use_current_mode, > + output->base.name, &config); > if (parse_gbm_format(config.gbm_format, b->gbm_format, > &output->gbm_format) == -1) > output->gbm_format = b->gbm_format; > > @@ -2699,11 +2709,6 @@ drm_destroy(struct weston_compositor *ec) > weston_launcher_destroy(ec->launcher); > > close(b->drm.fd); > - > - free(b->config->gbm_format); > - free(b->config->seat_id); > - free(b->config); > - > free(b); > } > > @@ -3054,7 +3059,8 @@ drm_backend_create(struct weston_compositor *compositor, > b->sprites_are_broken = 1; > b->compositor = compositor; > b->use_pixman = config->use_pixman; > - b->config = config; > + b->configure_output = config->configure_output; > + b->use_current_mode = config->use_current_mode; > > if (parse_gbm_format(config->gbm_format, GBM_FORMAT_XRGB8888, > &b->gbm_format) < 0) > goto err_compositor; > diff --git a/src/compositor-drm.h b/src/compositor-drm.h > index fdf5154..3b2dc17 100644 > --- a/src/compositor-drm.h > +++ b/src/compositor-drm.h > @@ -114,9 +114,10 @@ struct weston_drm_backend_config { > */ > enum weston_drm_backend_output_mode > (*configure_output)(struct weston_compositor *compositor, > - struct weston_drm_backend_config > *backend_config, > + bool use_current_mode, > const char *name, > struct weston_drm_backend_output_config > *output_config); > + bool use_current_mode; > }; > > #ifdef __cplusplus > diff --git a/src/main.c b/src/main.c > index 02b3278..745d527 100644 > --- a/src/main.c > +++ b/src/main.c > @@ -688,18 +688,12 @@ 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, > + bool use_current_mode, > 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; > @@ -714,7 +708,7 @@ drm_configure_output(struct weston_compositor *c, > return WESTON_DRM_BACKEND_OUTPUT_OFF; > } > > - if (drm_config->use_current_mode || strcmp(s, "current") == 0) { > + if (use_current_mode || strcmp(s, "current") == 0) { > mode = WESTON_DRM_BACKEND_OUTPUT_CURRENT; > } else if (strcmp(s, "preferred") != 0) { > config->modeline = s; > @@ -740,41 +734,33 @@ 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_drm_backend_config config = {{ 0, }}; > struct weston_config_section *section; > int ret = 0; > > - config = zalloc(sizeof (struct drm_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 }, > + { WESTON_OPTION_INTEGER, "connector", 0, &config.connector }, > + { WESTON_OPTION_STRING, "seat", 0, &config.seat_id }, > + { WESTON_OPTION_INTEGER, "tty", 0, &config.tty }, > + { WESTON_OPTION_BOOLEAN, "current-mode", 0, > &config.use_current_mode }, > + { WESTON_OPTION_BOOLEAN, "use-pixman", 0, &config.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, > + "gbm-format", &config.gbm_format, > NULL); > > - config->base.base.struct_version = WESTON_DRM_BACKEND_CONFIG_VERSION; > - config->base.base.struct_size = sizeof(struct > weston_drm_backend_config); > - config->base.configure_output = drm_configure_output; > + config.base.struct_version = WESTON_DRM_BACKEND_CONFIG_VERSION; > + config.base.struct_size = sizeof(struct weston_drm_backend_config); > + config.configure_output = drm_configure_output; > > - if (load_backend_new(c, backend, > - (struct weston_backend_config *)(&config->base)) < > 0) { > - ret = -1; > - free(config->base.gbm_format); > - free(config->base.seat_id); > - free(config); > - } > + ret = load_backend_new(c, backend, &config.base); > + > + free(config.gbm_format); > + free(config.seat_id); > > return ret; > } > _______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
