On Tuesday, September 26, 2017 10:00:47 AM EDT Pekka Paalanen wrote:
> On Wed, 6 Sep 2017 08:17:22 -0400
> nerdopolis <bluescreen_aven...@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
Thanks for the review.
I care about the fbdev backend because of the support for DisplayLink devices.
I don't think I can am good enough where I can fix *those* problems with Kernel
Mode setting... ...and as far as multi seat goes, USB devices are probably the
most suitable for supporting multiseat setups (as opposed to multi PCI cards)
_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
