[gdm/wip/re-read-config: 1/4] local-display-factory: Stall startup until main graphics card is ready

[Ray Strode <halfline src gnome org>]

commit 9413476ae722c7566b106f66161859520352a703
Author: Ray Strode <rstrode redhat com>
Date:   Tue Mar 1 13:25:02 2022 -0500

    local-display-factory: Stall startup until main graphics card is ready
    
    At the moment, GDM waits until systemd says the system supports
    graphics (via the CanGraphical logind property).
    
    Unfortunately, this property isn't really what we need, since it flips
    to true when *any* graphics are available, not when the main graphics
    for the system are ready.
    
    This is a problem on hybrid graphics systems, if one card is slower to
    load than another. In particular, the vendor nvidia driver can be slow
    to load because it has multiple kernel modules it loads in series.
    
    Indeed on fast systems, that use the vendor nvidia driver, it's not
    unusual for boot to get to a point where all of userspace up to and
    including GDM is executed before the graphics are ready to go.
    
    This commit tries to mitigate the situation by adding an additional,
    check aside from CanGraphical to test if the system is ready.
    
    This check waits for the graphics card associated with boot to be fully
    up and running before proceeding to start a login screen.
    
    Closes: https://gitlab.gnome.org/GNOME/gdm/-/issues/763

 daemon/gdm-local-display-factory.c | 164 ++++++++++++++++++++++++++++++++++---
 daemon/meson.build                 |   4 +
 meson.build                        |   2 +
 3 files changed, 158 insertions(+), 12 deletions(-)
---
diff --git a/daemon/gdm-local-display-factory.c b/daemon/gdm-local-display-factory.c
index 1b43d0c17..9150548f1 100644
--- a/daemon/gdm-local-display-factory.c
+++ b/daemon/gdm-local-display-factory.c
@@ -28,6 +28,10 @@
 #include <glib-object.h>
 #include <gio/gio.h>
 
+#ifdef HAVE_UDEV
+#include <gudev/gudev.h>
+#endif
+
 #include <systemd/sd-login.h>
 
 #include "gdm-common.h"
@@ -52,7 +56,10 @@
 
 struct _GdmLocalDisplayFactory
 {
-        GdmDisplayFactory              parent;
+        GdmDisplayFactory  parent;
+#ifdef HAVE_UDEV
+        GUdevClient       *gudev_client;
+#endif
 
         GdmDBusLocalDisplayFactory *skeleton;
         GDBusConnection *connection;
@@ -65,9 +72,14 @@ struct _GdmLocalDisplayFactory
         guint            seat_removed_id;
         guint            seat_properties_changed_id;
 
+        gboolean         seat0_has_platform_graphics;
+        gboolean         seat0_has_boot_up_graphics;
+
         gboolean         seat0_graphics_check_timed_out;
         guint            seat0_graphics_check_timeout_id;
 
+        guint            uevent_handler_id;
+
 #if defined(ENABLE_USER_DISPLAY_SERVER)
         unsigned int     active_vt;
         guint            active_vt_watch_id;
@@ -622,6 +634,86 @@ lookup_prepared_display_by_seat_id (const char *id,
         return lookup_by_seat_id (id, display, user_data);
 }
 
+#ifdef HAVE_UDEV
+static gboolean
+udev_is_settled (GdmLocalDisplayFactory *factory)
+{
+        g_autoptr (GUdevEnumerator) enumerator = NULL;
+        GList *devices;
+        GList *node;
+
+        gboolean is_settled = FALSE;
+
+        if (factory->seat0_has_platform_graphics) {
+                g_debug ("GdmLocalDisplayFactory: udev settled, platform graphics enabled.");
+                return TRUE;
+        }
+
+        if (factory->seat0_has_boot_up_graphics) {
+                g_debug ("GdmLocalDisplayFactory: udev settled, boot up graphics available.");
+                return TRUE;
+        }
+
+        if (factory->seat0_graphics_check_timed_out) {
+                g_debug ("GdmLocalDisplayFactory: udev timed out, proceeding anyway.");
+                return TRUE;
+        }
+
+        g_debug ("GdmLocalDisplayFactory: Checking if udev has settled enough to support graphics.");
+
+        enumerator = g_udev_enumerator_new (factory->gudev_client);
+
+        g_udev_enumerator_add_match_name (enumerator, "card*");
+        g_udev_enumerator_add_match_tag (enumerator, "master-of-seat");
+        g_udev_enumerator_add_match_subsystem (enumerator, "drm");
+
+        devices = g_udev_enumerator_execute (enumerator);
+        if (!devices) {
+                g_debug ("GdmLocalDisplayFactory: udev has no candidate graphics devices available yet.");
+                return FALSE;
+        }
+
+        node = devices;
+        while (node != NULL) {
+                GUdevDevice *device = node->data;
+                GList *next_node = node->next;
+                g_autoptr (GUdevDevice) platform_device = NULL;
+                g_autoptr (GUdevDevice) pci_device = NULL;
+
+                platform_device = g_udev_device_get_parent_with_subsystem (device, "platform", NULL);
+
+                if (platform_device != NULL) {
+                        g_debug ("GdmLocalDisplayFactory: Found embedded platform graphics, proceeding.");
+                        factory->seat0_has_platform_graphics = TRUE;
+                        is_settled = TRUE;
+                        break;
+                }
+
+                pci_device = g_udev_device_get_parent_with_subsystem (device, "pci", NULL);
+
+                if (pci_device != NULL) {
+                        gboolean boot_vga;
+
+                        boot_vga = g_udev_device_get_sysfs_attr_as_int (pci_device, "boot_vga");
+
+                        if (boot_vga == 1) {
+                                 g_debug ("GdmLocalDisplayFactory: Found primary PCI graphics adapter, 
proceeding.");
+                                 factory->seat0_has_boot_up_graphics = TRUE;
+                                 is_settled = TRUE;
+                                 break;
+                        } else {
+                                 g_debug ("GdmLocalDisplayFactory: Found secondary PCI graphics adapter, not 
proceeding yet.");
+                        }
+                }
+                node = next_node;
+        }
+
+        g_debug ("GdmLocalDisplayFactory: udev has %ssettled enough for graphics.", is_settled? "" : "not ");
+        g_list_free_full (devices, g_object_unref);
+        return is_settled;
+}
+#endif
+
 static int
 on_seat0_graphics_check_timeout (gpointer user_data)
 {
@@ -653,6 +745,7 @@ ensure_display_for_seat (GdmLocalDisplayFactory *factory,
         gboolean wayland_enabled = FALSE, xorg_enabled = FALSE;
         g_autofree gchar *preferred_display_server = NULL;
         gboolean falling_back = FALSE;
+        gboolean waiting_on_udev = FALSE;
 
         gdm_settings_direct_get_boolean (GDM_KEY_WAYLAND_ENABLE, &wayland_enabled);
         gdm_settings_direct_get_boolean (GDM_KEY_XORG_ENABLE, &xorg_enabled);
@@ -664,19 +757,28 @@ ensure_display_for_seat (GdmLocalDisplayFactory *factory,
                return;
         }
 
-        ret = sd_seat_can_graphical (seat_id);
+#ifdef HAVE_UDEV
+        waiting_on_udev = !udev_is_settled (factory);
+#endif
 
-        if (ret < 0) {
-                g_critical ("Failed to query CanGraphical information for seat %s", seat_id);
-                return;
-        }
+        if (!waiting_on_udev) {
+                ret = sd_seat_can_graphical (seat_id);
 
-        if (ret == 0) {
-                g_debug ("GdmLocalDisplayFactory: System doesn't currently support graphics");
-                seat_supports_graphics = FALSE;
+                if (ret < 0) {
+                        g_critical ("Failed to query CanGraphical information for seat %s", seat_id);
+                        return;
+                }
+
+                if (ret == 0) {
+                        g_debug ("GdmLocalDisplayFactory: System doesn't currently support graphics");
+                        seat_supports_graphics = FALSE;
+                } else {
+                        g_debug ("GdmLocalDisplayFactory: System supports graphics");
+                        seat_supports_graphics = TRUE;
+                }
         } else {
-                g_debug ("GdmLocalDisplayFactory: System supports graphics");
-                seat_supports_graphics = TRUE;
+               g_debug ("GdmLocalDisplayFactory: udev is still settling, so not creating display yet");
+               seat_supports_graphics = FALSE;
         }
 
         if (g_strcmp0 (seat_id, "seat0") == 0) {
@@ -703,7 +805,7 @@ ensure_display_for_seat (GdmLocalDisplayFactory *factory,
 
         /* For seat0, we have a fallback logic to still try starting it after
          * SEAT0_GRAPHICS_CHECK_TIMEOUT seconds. i.e. we simply continue even if
-         * CanGraphical is unset.
+         * CanGraphical is unset or udev otherwise never finds a suitable graphics card.
          * This is ugly, but it means we'll come up eventually in some
          * scenarios where no master device is present.
          * Note that we'll force an X11 fallback even though there might be
@@ -1166,10 +1268,35 @@ on_vt_changed (GIOChannel    *source,
 }
 #endif
 
+#ifdef HAVE_UDEV
+static void
+on_uevent (GUdevClient *client,
+           const char  *action,
+           GUdevDevice *device,
+           GdmLocalDisplayFactory *factory)
+{
+        if (!g_udev_device_get_device_file (device))
+                return;
+
+        if (g_strcmp0 (action, "add") != 0 &&
+            g_strcmp0 (action, "change") != 0)
+                return;
+
+        if (!udev_is_settled (factory))
+                return;
+
+        g_signal_handler_disconnect (factory->gudev_client, factory->uevent_handler_id);
+        factory->uevent_handler_id = 0;
+
+        ensure_display_for_seat (factory, "seat0");
+}
+#endif
+
 static void
 gdm_local_display_factory_start_monitor (GdmLocalDisplayFactory *factory)
 {
         g_autoptr (GIOChannel) io_channel = NULL;
+        const char *subsystems[] = { "drm", NULL };
 
         factory->seat_new_id = g_dbus_connection_signal_subscribe (factory->connection,
                                                                          "org.freedesktop.login1",
@@ -1201,6 +1328,13 @@ gdm_local_display_factory_start_monitor (GdmLocalDisplayFactory *factory)
                                                                                   on_seat_properties_changed,
                                                                                   g_object_ref (factory),
                                                                                   g_object_unref);
+#ifdef HAVE_UDEV
+        factory->gudev_client = g_udev_client_new (subsystems);
+        factory->uevent_handler_id = g_signal_connect (factory->gudev_client,
+                                                       "uevent",
+                                                       G_CALLBACK (on_uevent),
+                                                       factory);
+#endif
 
 #if defined(ENABLE_USER_DISPLAY_SERVER)
         io_channel = g_io_channel_new_file ("/sys/class/tty/tty0/active", "r", NULL);
@@ -1219,6 +1353,12 @@ gdm_local_display_factory_start_monitor (GdmLocalDisplayFactory *factory)
 static void
 gdm_local_display_factory_stop_monitor (GdmLocalDisplayFactory *factory)
 {
+        if (factory->uevent_handler_id) {
+                g_signal_handler_disconnect (factory->gudev_client, factory->uevent_handler_id);
+                factory->uevent_handler_id = 0;
+        }
+        g_clear_object (&factory->gudev_client);
+
         if (factory->seat_new_id) {
                 g_dbus_connection_signal_unsubscribe (factory->connection,
                                                       factory->seat_new_id);
diff --git a/daemon/meson.build b/daemon/meson.build
index 2e61b6447..41f30abef 100644
--- a/daemon/meson.build
+++ b/daemon/meson.build
@@ -204,6 +204,10 @@ if xdmcp_dep.found()
   ]
 endif
 
+if gudev_dep.found()
+  gdm_daemon_deps += gudev_dep
+endif
+
 gdm_daemon = executable('gdm',
   [ gdm_daemon_sources, gdm_daemon_gen_sources ],
   dependencies: gdm_daemon_deps,
diff --git a/meson.build b/meson.build
index 3be48680f..1caebc6c2 100644
--- a/meson.build
+++ b/meson.build
@@ -38,6 +38,7 @@ config_h_dir = include_directories('.')
 
 # Dependencies
 udev_dep = dependency('udev')
+gudev_dep = dependency('gudev-1.0', version: '>= 232')
 
 glib_min_version = '2.56.0'
 
@@ -244,6 +245,7 @@ conf.set_quoted('SYSTEMD_X_SERVER', systemd_x_server)
 conf.set('WITH_PLYMOUTH', plymouth_dep.found())
 conf.set_quoted('X_SERVER', x_bin)
 conf.set_quoted('X_PATH', x_path)
+conf.set('HAVE_UDEV', gudev_dep.found())
 conf.set('HAVE_UT_UT_HOST', utmp_has_host_field)
 conf.set('HAVE_UT_UT_PID', utmp_has_pid_field)
 conf.set('HAVE_UT_UT_ID', utmp_has_id_field)