[PATCH 5/6] libweston: fbdev: Attempt to detect the first framebuffer device in the seat. instead of defaulting to /dev/fb0

[Pekka Paalanen]

On Wed,  6 Sep 2017 08:17:22 -0400
nerdopolis <bluescreen_avenger at verizon.net> wrote:

> ---
>  libweston/compositor-fbdev.c | 35 +++++++++++++++++++++++++++++++++--
>  1 file changed, 33 insertions(+), 2 deletions(-)
> 
> diff --git a/libweston/compositor-fbdev.c b/libweston/compositor-fbdev.c
> index a9cc08be..99362b8a 100644
> --- a/libweston/compositor-fbdev.c
> +++ b/libweston/compositor-fbdev.c
> @@ -732,6 +732,12 @@ fbdev_backend_create(struct weston_compositor *compositor,
>  	const char *seat_id = default_seat;
>  	const char *session_seat;
>  
> +	struct udev_enumerate *e;
> +	struct udev_list_entry *entry;
> +	const char *path, *device_seat;
> +        char *fb_device;

Use tabs.

> +	struct udev_device *device;
> +
>  	session_seat=getenv("XDG_SEAT");
>  	if (session_seat)
>  		seat_id=session_seat;
> @@ -755,6 +761,33 @@ fbdev_backend_create(struct weston_compositor *compositor,
>  		goto out_compositor;
>  	}
>  
> +	e = udev_enumerate_new(backend->udev);
> +	udev_enumerate_add_match_subsystem(e, "graphics");
> +	udev_enumerate_add_match_sysname(e, "fb[0-9]*");
> +
> +	udev_enumerate_scan_devices(e);
> +	fb_device = NULL;
> +	udev_list_entry_foreach(entry, udev_enumerate_get_list_entry(e)) {
> +
> +		path = udev_list_entry_get_name(entry);
> +		device = udev_device_new_from_syspath(backend->udev, path);
> +		if (!device)
> +			continue;
> +		device_seat = udev_device_get_property_value(device, "ID_SEAT");
> +		if (!device_seat)
> +			device_seat = default_seat;
> +		if (!strcmp(device_seat, seat_id)) {
> +			fb_device = udev_device_get_devnode(device);
> +			if (fb_device && !param->device)
> +				param->device = strdup(fb_device);
> +			udev_device_unref(device);
> +			udev_enumerate_unref(e);
> +			break;
> +		}
> +	}

If you put all the above in a new function, you could skip it all when
param->device is already set. Just a thought.

> +	if (!param->device)
> +		param->device=strdup("/dev/fb0");
> +
>  	/* Set up the TTY. */
>  	backend->session_listener.notify = session_notify;
>  	wl_signal_add(&compositor->session_signal,
> @@ -802,8 +835,6 @@ out_compositor:
>  static void
>  config_init_to_defaults(struct weston_fbdev_backend_config *config)
>  {
> -	/* TODO: Ideally, available frame buffers should be enumerated using
> -	 * udev, rather than passing a device node in as a parameter. */
>  	config->tty = 0; /* default to current tty */
>  	config->device = "/dev/fb0"; /* default frame buffer */
>  	config->seat_id = "seat0"; /* default seat*/

I see the following patch removes the default setting of device from
main.c, but it should removed from here as well, for the same reasons
as in patch 2 for seat_id.

This patch looks ok, but is again missing the rationale on why we want
this.

I understand this is part of the whole make seats work with fbdev
backend, but why care so much about the fbdev backend?


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20170926/f5199672/attachment-0001.sig>