Merge branch 'wip/re-read-config' into 'main'

Try to address nvidia race on hybrid graphics setups

Closes #763

See merge request GNOME/gdm!173

13 files changed, 192 insertions, 67 deletions

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 1ea365a9..cba32d82 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -21,6 +21,7 @@ build-fedora:
         libXdmcp-devel
         libattr-devel
         libcanberra-devel
+        libgudev-devel
         libdmx-devel
         libselinux-devel
         libtool
diff --git a/common/gdm-settings-direct.c b/common/gdm-settings-direct.c
index ddb31908..5fbe0326 100644
--- a/common/gdm-settings-direct.c
+++ b/common/gdm-settings-direct.c
@@ -252,6 +252,15 @@ gdm_settings_direct_init (GdmSettings *settings,
 }
 
 void
+gdm_settings_direct_reload (void)
+{
+    if (!settings_object)
+            return;
+
+    gdm_settings_reload (settings_object);
+}
+
+void
 gdm_settings_direct_shutdown (void)
 {
 
diff --git a/common/gdm-settings-direct.h b/common/gdm-settings-direct.h
index 156489cd..6754955f 100644
--- a/common/gdm-settings-direct.h
+++ b/common/gdm-settings-direct.h
@@ -30,6 +30,8 @@ G_BEGIN_DECLS
 gboolean              gdm_settings_direct_init                       (GdmSettings       *settings,
                                                                       const char        *schemas_file,
                                                                       const char        *root);
+
+void                  gdm_settings_direct_reload                     (void);
 void                  gdm_settings_direct_shutdown                   (void);
 
 gboolean              gdm_settings_direct_get                        (const char        *key,
diff --git a/common/gdm-settings.c b/common/gdm-settings.c
index e6f46ec3..96c2f8d3 100644
--- a/common/gdm-settings.c
+++ b/common/gdm-settings.c
@@ -184,12 +184,16 @@ backend_value_changed (GdmSettingsBackend *backend,
         g_signal_emit (settings, signals [VALUE_CHANGED], 0, key, old_value, new_value);
 }
 
-static void
-gdm_settings_init (GdmSettings *settings)
+void
+gdm_settings_reload (GdmSettings *settings)
 {
         GList *l;
         GdmSettingsBackend *backend;
 
+        g_list_foreach (settings->backends, (GFunc) g_object_unref, NULL);
+        g_list_free (settings->backends);
+        settings->backends = NULL;
+
         backend = gdm_settings_desktop_backend_new (GDM_CUSTOM_CONF);
         if (backend)
                 settings->backends = g_list_prepend (NULL, backend);
@@ -209,6 +213,12 @@ gdm_settings_init (GdmSettings *settings)
 }
 
 static void
+gdm_settings_init (GdmSettings *settings)
+{
+        gdm_settings_reload (settings);
+}
+
+static void
 gdm_settings_finalize (GObject *object)
 {
         GdmSettings *settings;
diff --git a/common/gdm-settings.h b/common/gdm-settings.h
index 786868a9..07b64785 100644
--- a/common/gdm-settings.h
+++ b/common/gdm-settings.h
@@ -40,6 +40,7 @@ typedef enum
 GQuark              gdm_settings_error_quark                    (void);
 
 GdmSettings *       gdm_settings_new                            (void);
+void                gdm_settings_reload                         (GdmSettings *settings);
 
 /* exported */
 
diff --git a/daemon/gdm-local-display-factory.c b/daemon/gdm-local-display-factory.c
index 1b43d0c1..7de7b99c 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
@@ -949,10 +1051,12 @@ on_seat_properties_changed (GDBusConnection *connection,
         if (ret < 0)
                 return;
 
-        if (ret != 0)
+        if (ret != 0) {
+                gdm_settings_direct_reload ();
                 ensure_display_for_seat (GDM_LOCAL_DISPLAY_FACTORY (user_data), seat);
-        else
+        } else {
                 delete_display (GDM_LOCAL_DISPLAY_FACTORY (user_data), seat);
+        }
 }
 
 static gboolean
@@ -1166,10 +1270,36 @@ 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;
+
+        gdm_settings_direct_reload ();
+        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 +1331,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 +1356,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/main.c b/daemon/main.c
index 1b893fe0..344d1b74 100644
--- a/daemon/main.c
+++ b/daemon/main.c
@@ -267,16 +267,8 @@ static gboolean
 on_sighup_cb (gpointer user_data)
 {
         g_debug ("Got HUP signal");
-        /* Reread config stuff like system config files, VPN service
-         * files, etc
-         */
-        g_object_unref (settings);
-        settings = gdm_settings_new ();
-        if (settings != NULL) {
-                if (! gdm_settings_direct_init (settings, DATADIR "/gdm/gdm.schemas", "/")) {
-                        g_warning ("Unable to initialize settings");
-                }
-        }
+
+        gdm_settings_reload (settings);
 
         return TRUE;
 }
diff --git a/daemon/meson.build b/daemon/meson.build
index 2e61b644..41f30abe 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/data/61-gdm.rules.in b/data/61-gdm.rules.in
index d4f094ea..e9c72697 100644
--- a/data/61-gdm.rules.in
+++ b/data/61-gdm.rules.in
@@ -9,15 +9,6 @@ DRIVERS=="simple-framebuffer", GOTO="gdm_nomodeset_end"
 IMPORT{cmdline}="nomodeset", GOTO="gdm_disable_wayland"
 LABEL="gdm_nomodeset_end"
 
-# The vendor nvidia driver has mutiple modules that need to be loaded before GDM can make an
-# informed choice on which way to proceed, so force GDM to wait until NVidia's modules are
-# loaded before starting up.
-KERNEL!="nvidia", GOTO="gdm_nvidia_end"
-SUBSYSTEM!="module", GOTO="gdm_nvidia_end"
-ACTION!="add", GOTO="gdm_nvidia_end"
-RUN+="@bindir@/touch /run/udev/gdm-waiting-on-udev"
-LABEL="gdm_nvidia_end"
-
 # Disable wayland when nvidia modeset is disabled or when drivers are a lower
 # version than 470,
 # For versions above 470 but lower than 510 prefer Xorg,
@@ -30,7 +21,7 @@ ATTR{parameters/modeset}!="Y", GOTO="gdm_disable_wayland"
 # disable wayland for nvidia drivers versions lower than 470
 ATTR{version}=="4[0-6][0-9].*|[0-3][0-9][0-9].*|[0-9][0-9].*|[0-9].*", GOTO="gdm_disable_wayland"
 # For nvidia drivers versions Above 510, keep Wayland by default
-ATTR{version}=="[5-9][1-9][0-9].*", GOTO="gdm_stop_waiting_on_udev"
+ATTR{version}=="[5-9][1-9][0-9].*", GOTO="gdm_end"
 # For nvidia drivers versions 470-495, prefer Xorg by default
 GOTO="gdm_prefer_xorg"
 LABEL="gdm_nvidia_drm_end"
@@ -39,14 +30,10 @@ GOTO="gdm_end"
 
 LABEL="gdm_prefer_xorg"
 RUN+="@libexecdir@/gdm-runtime-config set daemon PreferredDisplayServer xorg"
-GOTO="gdm_stop_waiting_on_udev"
+GOTO="gdm_end"
 
 LABEL="gdm_disable_wayland"
 RUN+="@libexecdir@/gdm-runtime-config set daemon WaylandEnable false"
-GOTO="gdm_stop_waiting_on_udev"
-
-LABEL="gdm_stop_waiting_on_udev"
-RUN+="@bindir@/rm -f /run/udev/gdm-waiting-on-udev"
 GOTO="gdm_end"
 
 LABEL="gdm_end"
diff --git a/data/gdm-waiting-on-udev.path.in b/data/gdm-waiting-on-udev.path.in
deleted file mode 100644
index ceb03704..00000000
--- a/data/gdm-waiting-on-udev.path.in
+++ /dev/null
@@ -1,12 +0,0 @@
-[Unit]
-Description=Synchronize GNOME Display Manager with udev
-ConditionPathExists=/run/udev/gdm-waiting-on-udev
-
-[Path]
-PathChanged=/run/udev/gdm-waiting-on-udev
-Unit=gdm.service
-
-[Install]
-WantedBy=graphical.target
-
-
diff --git a/data/gdm.service.in b/data/gdm.service.in
index effdf56a..17e8a8de 100644
--- a/data/gdm.service.in
+++ b/data/gdm.service.in
@@ -19,11 +19,6 @@ After=rc-local.service plymouth-start.service systemd-user-sessions.service
 # for any reason, make sure plymouth still stops
 OnFailure=plymouth-quit.service
 
-# If our udev rule is still trying to figure out the lay of the land
-# then block for a bit. gdm-waiting-on-udev.path will start us later
-# when appropriate.
-ConditionPathExists=!/run/udev/gdm-waiting-on-udev
-
 [Service]
 ExecStart=${sbindir}/gdm
 KillMode=mixed
diff --git a/data/meson.build b/data/meson.build
index 8764abfe..2dec4c23 100644
--- a/data/meson.build
+++ b/data/meson.build
@@ -149,7 +149,6 @@ gdm_rules = configure_file(
   output: '@BASENAME@',
   configuration: {
     'libexecdir': gdm_prefix / get_option('libexecdir'),
-    'bindir': gdm_prefix / get_option('bindir'),
   },
   install_dir: udev_dir,
 )
@@ -186,14 +185,6 @@ configure_file(
   format: 'cmake'
 )
 
-configure_file(
-  input: 'gdm-waiting-on-udev.path.in',
-  output: '@BASENAME@',
-  configuration: service_config,
-  install_dir: systemd_systemunitdir,
-  format: 'cmake'
-)
-
 gdm_gnome_session_wanted_targets = []
 foreach component: gdm_gnome_user_session_wanted_components
   gdm_gnome_session_wanted_targets += 'Wants=@0@.target'.format(component)
diff --git a/meson.build b/meson.build
index 3be48680..1caebc6c 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)