[PATCH v5 7/7] libweston: fbdev: Follow the same logic as compositor-drm, and favor the boot_vga device

[Pekka Paalanen]

On Fri, 29 Dec 2017 13:31:53 -0500
nerdopolis <bluescreen_avenger at verizon.net> wrote:

> virtual framebuffer devices that are created by a modesetting driver have the same parent
> as the drm card devices.

Hi,

is this also true for legacy fbdev drivers that are not DRM drivers? I
suppose if not, they would just use the fallback path of not finding
any boot-vga and will work ok?

Only virtual fb devices? What about normal fb devices created by DRM
drivers?

The summary should be shorter, usually summaries should be much shorter
than a normal line length. In this case, "Follow the same logic as
compositor-drm, and " offers nothing more to identify this change. It
should be said in the commit message body instead.

> ---
>  libweston/compositor-fbdev.c | 40 ++++++++++++++++++++++++++++++++++------
>  1 file changed, 34 insertions(+), 6 deletions(-)
> 
> diff --git a/libweston/compositor-fbdev.c b/libweston/compositor-fbdev.c
> index 34e6c2f3..139d6624 100644
> --- a/libweston/compositor-fbdev.c
> +++ b/libweston/compositor-fbdev.c
> @@ -717,8 +717,8 @@ find_framebuffer_device(struct fbdev_backend *b, const char *seat)
>  	struct udev_enumerate *e;
>  	struct udev_list_entry *entry;
>  	const char *path, *device_seat;
> -	char *fb_device;
> -	struct udev_device *device;
> +	char *fb_device, *find_device, *id;
> +	struct udev_device *device, *pci;
>  
>  	e = udev_enumerate_new(b->udev);
>  	udev_enumerate_add_match_subsystem(e, "graphics");
> @@ -727,20 +727,48 @@ find_framebuffer_device(struct fbdev_backend *b, const char *seat)
>  	udev_enumerate_scan_devices(e);
>  	fb_device = NULL;
>  	udev_list_entry_foreach(entry, udev_enumerate_get_list_entry(e)) {
> +		bool is_boot_vga = false;
>  
>  		path = udev_list_entry_get_name(entry);
>  		device = udev_device_new_from_syspath(b->udev, path);
> +		find_device = udev_device_get_devnode(device);
>  		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)) {
> -			fb_device = udev_device_get_devnode(device);
> -			udev_enumerate_unref(e);
> +		if (strcmp(device_seat, seat)) {
> +			udev_device_unref(device);
> +			continue;
> +		}
> +
> +		pci = udev_device_get_parent_with_subsystem_devtype(device,
> +								"pci", NULL);
> +		if (pci) {
> +			id = udev_device_get_sysattr_value(pci, "boot_vga");
> +			if (id && !strcmp(id, "1"))
> +				is_boot_vga = true;
> +		}
> +
> +		/* If a framebuffer device was found, and this device isn't
> +		 * the boot-VGA device, don't use it. */
> +		if (!is_boot_vga && fb_device) {
> +			udev_device_unref(device);
> +			continue;
> +		}
> +
> +		/* There can only be one boot_vga device. Try to use it
> +		 * at all costs. */
> +		if (is_boot_vga) {
> +			fb_device = find_device;

Leaks 'device'. That leak fixed, maybe 'fb_device' needs to be strdup'd?

>  			break;
>  		}
> -		udev_device_unref(device);
> +
> +		/* Per the (!is_boot_vga && fb_device) test above, only
> +		 * trump existing saved devices with boot-VGA devices, so if
> +		 * the test ends up here, this must be the first device seen. */
> +		assert(!fb_device);
> +		fb_device = find_device;
>  	}
>  
>  	udev_enumerate_unref(e);


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/20180122/4b21f3da/attachment.sig>