[PATCH 2/5] drm: port the drm backend to the new init api

[Pekka Paalanen]

On Tue, 12 Apr 2016 21:34:28 -0700
Bryce Harrington <bryce at osg.samsung.com> wrote:

> On Wed, Apr 06, 2016 at 11:37:57AM +0300, Pekka Paalanen wrote:
> > On Wed,  9 Mar 2016 16:49:29 -0800
> > Bryce Harrington <bryce at osg.samsung.com> wrote:
> >   
> > > From: Giulio Camuffo <giuliocamuffo at gmail.com>
> > > 
> > > Signed-off-by: Bryce Harrington <bryce at osg.samsung.com>
> > > Reviewed-by: Quentin Glidic <sardemff7+git at sardemff7.net>
> > > Acked-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> > > ---
> > > v4: Update to current trunk
> > >     - Add missing param doc for mode in drm_output_choose_initial_mode
> > >     - Rebase to account for code changes by 91880f1e to make vt
> > >       switching configurable.
> > > 
> > >  Makefile.am          |   3 +
> > >  src/compositor-drm.c | 196 ++++++++++++++++++---------------------------------
> > >  src/compositor.h     |   2 -
> > >  src/main.c           |  94 +++++++++++++++++++++++-
> > >  4 files changed, 165 insertions(+), 130 deletions(-)  
> > 
> > Hi Giulio and Bryce,
> > 
> > I'm sorry it has taken so long for me to come back to this.

> > > +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_config_section *section;
> > > +	int ret = 0;
> > > +
> > > +	config = zalloc(sizeof *config);
> > > +	if (!config)
> > > +		return -1;  
> > 
> > This function will be affected by the struct versioning too.  
> 
> The subsequent patch adds the versioning, although later in the routine
> than here.
> 
> > > +	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 },
> > > +	};  
> > 
> > Mixed declarations and code.  
> 
> Hmm, I'm not sure best how to address this.  Options is being
> initialized with pointers that are created by the zalloc, so I don't
> think I can just separate the declaration from the initialization.  Does
> options need to be changed to be dynamically allocated as well?

Hi,

I suppose the simplest solution is to make another function, define
'options[]' in it, and get 'config' as an argument. Then it can call
the parser and return. Something like ...fill_from_command_line(struct
drm_config *config, argc, argv...).

It's fine to keep your changes as separate patches as long as every
patch is bisectable.

I'm going through the new series right now, and noticed these emails
from you just now.


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/20160413/4e517251/attachment-0001.sig>