Re: [PATCH v7 5/6] compositor-fbdev: detect the first fb device in the seat

2018-06-27 Thread Emil Velikov
On 27 June 2018 at 00:39, nerdopolis  wrote:
> This adds a function to detect the first framebuffer device in the
> current seat. Instead of hardcoding /dev/fb0, detect the device
> with udev, favoring the boot_vga device, and falling back to the
> first framebuffer device in the seat if there is none. This is very
> similar to what compositor-drm does to find display devices
> ---
>  libweston/compositor-fbdev.c | 83 ++--
>  1 file changed, 80 insertions(+), 3 deletions(-)
>
> diff --git a/libweston/compositor-fbdev.c b/libweston/compositor-fbdev.c
> index e3777495..616300dc 100644
> --- a/libweston/compositor-fbdev.c
> +++ b/libweston/compositor-fbdev.c
> @@ -777,6 +777,77 @@ session_notify(struct wl_listener *listener, void *data)
> }
>  }
>
> +static char *
> +find_framebuffer_device(struct fbdev_backend *b, const char *seat)
> +{
> +   struct udev_enumerate *e;
> +   struct udev_list_entry *entry;
> +   const char *path, *device_seat, *id, *fb_device_path;
> +   struct udev_device *device, *fb_device, *pci;
> +
> +   e = udev_enumerate_new(b->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)) {
> +   bool is_boot_vga = false;
> +
> +   path = udev_list_entry_get_name(entry);
> +   device = udev_device_new_from_syspath(b->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)) {
> +   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) {
> +   if (fb_device)
> +   udev_device_unref(fb_device);
> +   fb_device = device;
> +   break;
> +   }
> +
> +   /* 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 = device;
> +   }
> +
> +   udev_enumerate_unref(e);
> +
This function (barring the if bellow) seems mostly identical to the
one in compositor-drm.c
Might be a good idea to make it a helper vaguely alike

struct udev_device *
foo(struct backend *b, const char *seat, const char *subsys, const
char *sysname,
  int (*predicate)(struct backend b*, struct udev_device *d))

The callers will be as trivial as
foo(b, seat, "drm", "card[0-9]*", drm_device_is_kms);
foo(b, seat, "graphics", "fb[0-9]*", NULL);

You'd want to do this at some point, since close to nobody tests
fbdev. I think that people porting any drm fixes to fbdev is even less
;-)

> +   if (fb_device)
> +   {
> +   fb_device_path=strdup(udev_device_get_devnode(fb_device));
> +   udev_device_unref(fb_device);
> +   }
> +
Obviously this trivial hunk will have to stay in fbdev.

HTH
Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH v7 5/6] compositor-fbdev: detect the first fb device in the seat

2018-06-26 Thread nerdopolis
This adds a function to detect the first framebuffer device in the
current seat. Instead of hardcoding /dev/fb0, detect the device
with udev, favoring the boot_vga device, and falling back to the
first framebuffer device in the seat if there is none. This is very
similar to what compositor-drm does to find display devices
---
 libweston/compositor-fbdev.c | 83 ++--
 1 file changed, 80 insertions(+), 3 deletions(-)

diff --git a/libweston/compositor-fbdev.c b/libweston/compositor-fbdev.c
index e3777495..616300dc 100644
--- a/libweston/compositor-fbdev.c
+++ b/libweston/compositor-fbdev.c
@@ -777,6 +777,77 @@ session_notify(struct wl_listener *listener, void *data)
}
 }
 
+static char *
+find_framebuffer_device(struct fbdev_backend *b, const char *seat)
+{
+   struct udev_enumerate *e;
+   struct udev_list_entry *entry;
+   const char *path, *device_seat, *id, *fb_device_path;
+   struct udev_device *device, *fb_device, *pci;
+
+   e = udev_enumerate_new(b->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)) {
+   bool is_boot_vga = false;
+
+   path = udev_list_entry_get_name(entry);
+   device = udev_device_new_from_syspath(b->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)) {
+   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) {
+   if (fb_device)
+   udev_device_unref(fb_device);
+   fb_device = device;
+   break;
+   }
+
+   /* 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 = device;
+   }
+
+   udev_enumerate_unref(e);
+
+   if (fb_device)
+   {
+   fb_device_path=strdup(udev_device_get_devnode(fb_device));
+   udev_device_unref(fb_device);
+   }
+
+   return fb_device_path;
+}
+
 static struct fbdev_backend *
 fbdev_backend_create(struct weston_compositor *compositor,
  struct weston_fbdev_backend_config *param)
@@ -809,6 +880,11 @@ fbdev_backend_create(struct weston_compositor *compositor,
goto out_compositor;
}
 
+   if (!param->device)
+   param->device = find_framebuffer_device(backend, seat_id);
+   if (!param->device)
+   param->device = strdup("/dev/fb0");
+
/* Set up the TTY. */
backend->session_listener.notify = session_notify;
wl_signal_add(>session_signal,
@@ -835,12 +911,15 @@ fbdev_backend_create(struct weston_compositor *compositor,
if (!fbdev_head_create(backend, param->device))
goto out_launcher;
 
+   free(param->device);
+
udev_input_init(>input, compositor, backend->udev,
seat_id, param->configure_device);
 
return backend;
 
 out_launcher:
+   free(param->device);
weston_launcher_destroy(compositor->launcher);
 
 out_udev:
@@ -856,10 +935,8 @@ 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->device = NULL;
config->seat_id = NULL;
 }
 
-- 
2.17.1

___