>+static void
>+probe_udev_for_graphics (state_t *state)
>+{
>+ struct udev_enumerate *udev_enum;
>+ struct udev_list_entry *entries, *entry;
>+
>+ assert (state != NULL);
>+
>+ udev_enum = udev_enumerate_new (state->udev);
>+ udev_enumerate_add_match_subsystem (udev_enum, "graphics");
>So this makes the assumption that the fb device will always be set up
>after the drm device is.
>I think that's a valid assumption in practice, but there should
>probably be a comment to that effect.
I don't think that assumption is being made. For context, the full list of
udev calls there is:
+ udev_enum = udev_enumerate_new (state->udev);
+ udev_enumerate_add_match_subsystem (udev_enum, "graphics");
+ udev_enumerate_add_match_tag(udev_enum, "seat");
+ udev_enumerate_add_match_is_initialized(udev_enum);
+ udev_enumerate_scan_devices (udev_enum);
+
+ entries = udev_enumerate_get_list_entry (udev_enum);
Unless I'm mistaken about how to drive the udev API, that should just
simply gather a list of all graphics devices with the "seat" tag that have
been fully initialized out of the list of all the devices that udev has
detected at that point; the ordering with framebuffer devices shouldn't
come into play.
>Alternatively, it could look for events in the drm subsystem, but then
>care would need to be taken to ensure we aren't acting on events "too
>soon" while things are partially initialized.
This is what's happening with setup_and_watch_udev(). In that function, I
watch the "graphics" subsystem with a udev_monitor and tell the plymouth
event loop to handle events on that udev_monitor with
on_udev_graphics_event(). As far as I remember, udev events don't fire
until the device is fully initialized.
Thanks,
Kevin
On Thu, Feb 28, 2013 at 10:31 PM, Ray Strode <halfline at gmail.com> wrote:
> Hey,
>
> On Mon, Feb 25, 2013 at 11:39 PM, Kevin Murphy <kemurphy.cmu at gmail.com>
> wrote:
> > Hello everyone,
> >
> > At some point last year, I started on adding udev support to plymouth. I
> > eventually ran out of time to work on it, but the attached patch is as
> far
> > as I got (I've rebased it to the current master as of 5 days ago). It
> looks
> > like I'll be able to work on this a bit more in my spare time again, so
> I'd
> > appreciate any comments you may have. The next step I was going to take
> was
> > to try to avoid loading and unloading the renderer each time we show and
> > hide the splash screen. Ray mentioned that one way to do this might
> involve
> > ply_renderer_close_input_source. There is a relevant conversation log
> from
> > IRC at http://people.freedesktop.org/~halfline/renderer-discussion.txtfrom
> > nearly a year ago, as well.
> Thanks for looking into this! It's sorely need feature. I'll give your
> patches a try soon, but wanted to give a few drive by comments below
> from doing a quick read though:
>
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -20,6 +20,7 @@ plymouthd_CFLAGS = $(PLYMOUTH_CFLAGS)
> \
> -DPLYMOUTH_POLICY_DIR=\"$(PLYMOUTH_POLICY_DIR)/\"
> \
> -DPLYMOUTH_RUNTIME_DIR=\"$(PLYMOUTH_RUNTIME_DIR)\"
> \
> -DPLYMOUTH_CONF_DIR=\"$(PLYMOUTH_CONF_DIR)/\"
> +plymouthd_LDFLAGS = -ludev
> Should use PKG_CHECK_MODULES(... libudev ...) in configure.ac
>
> --- a/src/main.c
> +++ b/src/main.c
> typedef struct
> {
> + const char *fb;
> + const char *card;
> + ply_renderer_t *renderer;
> +} ply_pixel_device_t;
>
> It seems like there may be some conceptual overlap between what a
> pixel device is and what a renderer is. Maybe they should be
> consolidated? if there's a compelling reason they shouldn't be so be
> it, though.
>
> The other thing is main.c is already pretty fat. It would be good if
> we could farm out logic to other source files.
> @@ -455,6 +478,336 @@ show_default_splash (state_t *state)
>
> +static void
> +probe_udev_for_graphics (state_t *state)
> +{
> + struct udev_enumerate *udev_enum;
> + struct udev_list_entry *entries, *entry;
> +
> + assert (state != NULL);
> +
> + udev_enum = udev_enumerate_new (state->udev);
> + udev_enumerate_add_match_subsystem (udev_enum, "graphics");
> So this makes the assumption that the fb device will always be set up
> after the drm device is.
> I think that's a valid assumption in practice, but there should
> probably be a comment to that effect.
>
> Alternatively, it could look for events in the drm subsystem, but then
> care would need to be taken to ensure we aren't acting on events "too
> soon" while things are partially initialized.
>
> Will try this soon!
>
> --Ray
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/plymouth/attachments/20130228/89dce4db/attachment-0001.html>