[PATCH weston v7 3/3] drm: Don't hang onto the backend config object post-backend_init

Bryce Harrington bryce at osg.samsung.com
Fri Apr 29 22:40:35 UTC 2016


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 <bryce at osg.samsung.com>
Reviewed-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>

---
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;
 }
-- 
1.9.1



More information about the wayland-devel mailing list