[PATCH 2/2] fbdev-compositor: enumerate framebuffer device using udev

[Sjoerd Simons]

On Tue, 2015-07-07 at 07:21 -0500, Derek Foreman wrote:
> On 06/07/15 03:31 AM, Sjoerd Simons wrote:
> > On startup enumerate the available framebuffer devices using udev 
> > and
> > setup an output for each device, unless a specific device is passed 
> > with
> > --device in which case just create one for that device.
> > 
> > Signed-off-by: Sjoerd Simons <sjoerd.simons at collabora.co.uk>
> > ---
> >  src/compositor-fbdev.c | 80 
> > ++++++++++++++++++++++++++++++++++++++++----------
> >  1 file changed, 65 insertions(+), 15 deletions(-)
> > 
> > diff --git a/src/compositor-fbdev.c b/src/compositor-fbdev.c
> > index 75ad646..dc77014 100644
> > --- a/src/compositor-fbdev.c
> > +++ b/src/compositor-fbdev.c
> > > > @@ -450,9 +450,9 @@ fbdev_frame_buffer_destroy(struct fbdev_output 
> > +		output = fbdev_output_create (compositor, node, 
> > x);
> > +
> > +		if (output == NULL) {
> > +			ret = -1;
> > +			goto out;
> 
> Can't we just break; instead of goto here and lose the label?

In this case we can indeed as nothing happens after the loop. I just
used goto as a habit for bailing & cleaning up :)

No preference here, i'm happy to tweak that if people prefer just
breaking.

> > 
> > -	/* TODO: Ideally, available frame buffers should be 
> > enumerated using
> > -	 * udev, rather than passing a device node in as a 
> > parameter. */
> >  	struct fbdev_parameters param = {
> >  		.tty = 0, /* default to current tty */
> > -		.device = "/dev/fb0", /* default frame buffer */
> > +		.device = NULL, /* default frame buffer */
> 
> Mildly concerned about the change in default behavior.  Should we
> default to just fb0 and add a new --enumerate-all-the-things command
> line switch?

I'm happy to tune to whatever folks thinks is best, though i have a
small preference for the default to just be: Come up on all screens.
Would love to hear other opinions on that though and especially from
people for which this change would be a practical issue

-- 
Sjoerd Simons 
Collabora Ltd.