Preliminary udev support

Ray Strode halfline at gmail.com
Thu Feb 28 19:31:14 PST 2013


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.txt from
> 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


More information about the plymouth mailing list