[PATCH weston v6 08/12] drm: Don't hang onto the backend config object post-backend_init
Pekka Paalanen
ppaalanen at gmail.com
Mon Apr 18 14:42:26 UTC 2016
On Sun, 17 Apr 2016 17:18:17 +0300
Giulio Camuffo <giuliocamuffo at gmail.com> wrote:
> Ah, so this fixes the problem with patch 5. I think it would make
> sense to squash them together.
> One comment below.
>
> 2016-04-16 6:28 GMT+03:00 Bryce Harrington <bryce at osg.samsung.com>:
> > 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.
> >
> > 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
> >
> > Signed-off-by: Bryce Harrington <bryce at osg.samsung.com>
> > ---
> > src/compositor-drm.c | 24 +++++++++++++++---------
> > src/compositor-drm.h | 3 ++-
> > src/main.c | 47 +++++++++++++++++++----------------------------
> > 3 files changed, 36 insertions(+), 38 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 feb967d..21b7f5c 100644
> > --- a/src/main.c
> > +++ b/src/main.c
> > @@ -674,19 +674,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;
> > @@ -701,7 +694,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;
> > @@ -727,41 +720,39 @@ 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;
>
> Any reason to not just allocate it on the stack?
I agree on both stack and squash, would be nice.
Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20160418/32ffbf8b/attachment.sig>
More information about the wayland-devel
mailing list