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

[Derek Foreman]

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)
>  static void fbdev_output_destroy(struct weston_output *base);
>  static void fbdev_output_disable(struct weston_output *base);
>  
> -static int
> +static struct fbdev_output *
>  fbdev_output_create(struct fbdev_compositor *compositor,
> -                    const char *device)
> +                    const char *device, int x)
>  {
>  	struct fbdev_output *output;
>  	struct weston_config_section *section;
> @@ -461,11 +461,11 @@ fbdev_output_create(struct fbdev_compositor *compositor,
>  	uint32_t config_transform;
>  	char *s;
>  
> -	weston_log("Creating fbdev output.\n");
> +	weston_log("Creating fbdev output for %s.\n", device);
>  
>  	output = zalloc(sizeof *output);
>  	if (output == NULL)
> -		return -1;
> +		return NULL;
>  
>  	output->compositor = compositor;
>  	output->device = device;
> @@ -502,7 +502,7 @@ fbdev_output_create(struct fbdev_compositor *compositor,
>  	output->base.subpixel = WL_OUTPUT_SUBPIXEL_UNKNOWN;
>  	output->base.make = "unknown";
>  	output->base.model = output->fb_info.id;
> -	output->base.name = strdup("fbdev");
> +	output->base.name = strdup(device);
>  
>  	section = weston_config_get_section(compositor->base.config,
>  					    "output", "name",
> @@ -514,7 +514,7 @@ fbdev_output_create(struct fbdev_compositor *compositor,
>  	free(s);
>  
>  	weston_output_init(&output->base, &compositor->base,
> -	                   0, 0, output->fb_info.width_mm,
> +	                   x, 0, output->fb_info.width_mm,
>  	                   output->fb_info.height_mm,
>  	                   config_transform,
>  			   1);
> @@ -544,7 +544,7 @@ fbdev_output_create(struct fbdev_compositor *compositor,
>  	weston_log_continue(STAMP_SPACE "guessing %d Hz and 96 dpi\n",
>  	                    output->mode.refresh / 1000);
>  
> -	return 0;
> +	return output;
>  
>  out_hw_surface:
>  	pixman_image_unref(output->hw_surface);
> @@ -554,7 +554,7 @@ out_hw_surface:
>  out_free:
>  	free(output);
>  
> -	return -1;
> +	return NULL;
>  }
>  
>  static void
> @@ -604,7 +604,7 @@ fbdev_output_reenable(struct fbdev_compositor *compositor,
>  {
>  	struct fbdev_output *output = to_fbdev_output(base);
>  	struct fbdev_screeninfo new_screen_info;
> -	int fb_fd;
> +	int fb_fd, x;
>  	const char *device;
>  
>  	weston_log("Re-enabling fbdev output.\n");
> @@ -633,8 +633,9 @@ fbdev_output_reenable(struct fbdev_compositor *compositor,
>  		 * the frame buffer X/Y resolution (such as the shadow buffer)
>  		 * are re-initialised. */
>  		device = output->device;
> +		x = pixman_region32_extents(&base->region)->x1;
>  		fbdev_output_destroy(base);
> -		fbdev_output_create(compositor, device);
> +		fbdev_output_create(compositor, device, x);
>  
>  		return 0;
>  	}
> @@ -746,6 +747,52 @@ switch_vt_binding(struct weston_seat *seat, uint32_t time, uint32_t key, void *d
>  	weston_launcher_activate_vt(compositor->launcher, key - KEY_F1 + 1);
>  }
>  
> +static int
> +fbdev_udev_create_outputs(struct fbdev_compositor *compositor)
> +{
> +	struct udev_enumerate *e;
> +	struct udev_list_entry *entry;
> +	int ret = -1;
> +	int x = 0;
> +
> +	e = udev_enumerate_new(compositor->udev);
> +	udev_enumerate_add_match_subsystem(e, "graphics");
> +	udev_enumerate_add_match_sysname(e, "fb[0-9]*");
> +
> +	udev_enumerate_scan_devices(e);
> +
> +	udev_list_entry_foreach (entry, udev_enumerate_get_list_entry(e)) {
> +		const char *path, *node;
> +		struct udev_device *device;
> +		struct fbdev_output *output;
> +
> +		path = udev_list_entry_get_name(entry);
> +		device = udev_device_new_from_syspath(compositor->udev, path);
> +		if (!device)
> +			continue;
> +
> +		node = udev_device_get_devnode(device);
> +
> +		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?

> +		}
> +
> +		/* Success if there is at least one output */
> +		ret = 0;
> +
> +		/* Offset for the next output */
> +		x = pixman_region32_extents(&output->base.region)->x2;
> +	}
> +
> +out:
> +	udev_enumerate_unref(e);
> +
> +	return ret;
> +}
> +
>  static struct weston_compositor *
>  fbdev_compositor_create(struct wl_display *display, int *argc, char *argv[],
>                          struct weston_config *config,
> @@ -819,8 +866,13 @@ fbdev_compositor_create(struct wl_display *display, int *argc, char *argv[],
>  		}
>  	}
>  
> -	if (fbdev_output_create(compositor, param->device) < 0)
> -		goto out_pixman;
> +	if (param->device != NULL) {
> +		if (fbdev_output_create(compositor, param->device, 0) == NULL)
> +			goto out_pixman;
> +	} else {
> +		if (fbdev_udev_create_outputs(compositor) < 0)
> +			goto out_pixman;
> +	}
>  
>  	udev_input_init(&compositor->input, &compositor->base, compositor->udev, seat_id);
>  
> @@ -848,11 +900,9 @@ WL_EXPORT struct weston_compositor *
>  backend_init(struct wl_display *display, int *argc, char *argv[],
>  	     struct weston_config *config)
>  {
> -	/* 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?

>  		.use_gl = 0,
>  	};
>  
>