From 313860e2fab15249337f70d0c298dd02b903c7b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michel=20D=C3=A4nzer?= Date: Thu, 9 Jan 2025 17:20:11 +0100 Subject: [PATCH] kms: Wait for 2 seconds before handling udev hotplug events More precisely, wait until no further udev hotplug events have arrived in 2 seconds. Keep a list of unique UpdateStatesData structs which have received hotplug events, and call handle_hotplug_event for all of them once the timeout expires. This avoids problems due to some monitors generating hotplug events when power saving is enabled on locking the session, and then temporarily appearing disconnected. v2: * Make meta_test_disconnect_connect wait and run main context dispatch before checking the number of logical monitors. v3: * Call g_source_unref immediately after g_source_attach, simplifies source cleanup. (Sebastian Wick) * Free hotplug devices list in meta_kms_finalize. (Sebastian Wick) * Add KMS debug logging in on_udev_hotplug & hotplug_timeout. * Bump timeout to 2 seconds. With my affected monitor, the second udev hotplug event normally arrives almost a second after the first one, occasionally more than 1.5 seconds though. * Use UpdateStatesData with custom compare function for hotplug_devices list data, since the GUdevDevice / device path string pointer values are always different. * Don't wait for timeout if meta_is_udev_test_device returns TRUE, to hopefully fix VKMS CI tests. Drop meta_test_disconnect_connect changes again. v4: * Use hash table instead of list for hotplug_events set. (Sebastian Wick) * Add comment describing the keys & values in the hash table. (Sebastian Wick) * Add KMS debug logging in handle_hotplug_event as well. * Dropped Closes:, this might not suffice to fully address https://gitlab.gnome.org/GNOME/mutter/-/issues/3831 after all. v5: * Simplify hash table annotation comment. (Sebastian Wick) v6: * Rename hotplug_timeout local string pointer to hotplug_event. * Drop g_clear_pointer in favour of g_autofree in on_udev_hotplug. (Sebastian Wick) Part-of: --- src/backends/native/meta-kms.c | 86 ++++++++++++++++++++++++++++++++-- 1 file changed, 81 insertions(+), 5 deletions(-) diff --git a/src/backends/native/meta-kms.c b/src/backends/native/meta-kms.c index 042bf47d1..58c129cc2 100644 --- a/src/backends/native/meta-kms.c +++ b/src/backends/native/meta-kms.c @@ -52,6 +52,10 @@ struct _MetaKms gulong lease_handler_id; gulong removed_handler_id; + GSource *hotplug_timeout; + /* Set: Pointer to "::" string */ + GHashTable *hotplug_events; + GList *devices; int kernel_thread_inhibit_count; @@ -252,10 +256,14 @@ meta_kms_update_states_sync (MetaKms *kms) static void handle_hotplug_event (MetaKms *kms, char *hotplug_event, - MetaKmsResourceChanges changes) + MetaKmsResourceChanges changes, + const char *caller) { changes |= update_states_sync (kms, hotplug_event); + meta_topic (META_DEBUG_KMS, "%s -> %s for '%s', changes=0x%x", + caller, __func__, hotplug_event, changes); + if (changes != META_KMS_RESOURCE_CHANGE_NONE) meta_kms_emit_resources_changed (kms, changes); } @@ -274,11 +282,56 @@ resume_in_impl (MetaThreadImpl *thread_impl, void meta_kms_resume (MetaKms *kms) { - handle_hotplug_event (kms, NULL, META_KMS_RESOURCE_CHANGE_FULL); + handle_hotplug_event (kms, NULL, META_KMS_RESOURCE_CHANGE_FULL, __func__); meta_kms_run_impl_task_sync (kms, resume_in_impl, NULL, NULL); } +static gboolean +hotplug_timeout (gpointer user_data) +{ + MetaKms *kms = user_data; + GHashTableIter iter; + char *hotplug_event; + + if (meta_is_topic_enabled (META_DEBUG_KMS)) + { + int64_t dispatch_time = g_source_get_time (kms->hotplug_timeout); + int64_t ready_time = g_source_get_ready_time (kms->hotplug_timeout); + + meta_topic (META_DEBUG_KMS, + "%s: %" G_GINT64_FORMAT " (dispatch time)" + " - %" G_GINT64_FORMAT " (ready time)" + " = %" G_GINT64_FORMAT "µs", + __func__, dispatch_time, ready_time, + dispatch_time - ready_time); + } + + g_hash_table_iter_init (&iter, kms->hotplug_events); + while (g_hash_table_iter_next (&iter, (gpointer *) &hotplug_event, NULL)) + { + handle_hotplug_event (kms, hotplug_event, META_KMS_RESOURCE_CHANGE_NONE, + __func__); + g_hash_table_iter_remove (&iter); + } + + kms->hotplug_timeout = NULL; + return G_SOURCE_REMOVE; +} + +static void +ensure_hotplug_timeout_source (MetaKms *kms) +{ + if (kms->hotplug_timeout) + return; + + kms->hotplug_timeout = g_timeout_source_new_seconds (2); + g_source_set_callback (kms->hotplug_timeout, hotplug_timeout, kms, NULL); + g_source_set_name (kms->hotplug_timeout, "[mutter] MetaKms hotplug timeout"); + g_source_attach (kms->hotplug_timeout, NULL); + g_source_unref (kms->hotplug_timeout); +} + static char * hotplug_event_from_udev_device (GUdevDevice *udev_device) { @@ -286,7 +339,7 @@ hotplug_event_from_udev_device (GUdevDevice *udev_device) uint32_t crtc_id, connector_id; if (!udev_device) - return NULL; + return g_strdup (""); device_path = g_udev_device_get_device_file (udev_device); crtc_id = @@ -303,10 +356,27 @@ on_udev_hotplug (MetaUdev *udev, GUdevDevice *udev_device, MetaKms *kms) { + int64_t now = g_get_monotonic_time (); g_autofree char *hotplug_event = NULL; + meta_topic (META_DEBUG_KMS, + "%s called at %" G_GINT64_FORMAT, + __func__, now); + hotplug_event = hotplug_event_from_udev_device (udev_device); - handle_hotplug_event (kms, hotplug_event, META_KMS_RESOURCE_CHANGE_NONE); + + if (meta_is_udev_test_device (udev_device)) + { + handle_hotplug_event (kms, hotplug_event, + META_KMS_RESOURCE_CHANGE_NONE, __func__); + return; + } + + ensure_hotplug_timeout_source (kms); + g_source_set_ready_time (kms->hotplug_timeout, now + 2 * G_USEC_PER_SEC); + + g_hash_table_insert (kms->hotplug_events, g_steal_pointer (&hotplug_event), + NULL); } static void @@ -314,7 +384,7 @@ on_udev_device_removed (MetaUdev *udev, GUdevDevice *device, MetaKms *kms) { - handle_hotplug_event (kms, NULL, META_KMS_RESOURCE_CHANGE_NONE); + handle_hotplug_event (kms, NULL, META_KMS_RESOURCE_CHANGE_NONE, __func__); } static void @@ -490,6 +560,9 @@ meta_kms_finalize (GObject *object) g_list_free_full (kms->devices, g_object_unref); + g_clear_pointer (&kms->hotplug_timeout, g_source_destroy); + g_hash_table_destroy (kms->hotplug_events); + g_clear_signal_handler (&kms->hotplug_handler_id, udev); g_clear_signal_handler (&kms->lease_handler_id, udev); g_clear_signal_handler (&kms->removed_handler_id, udev); @@ -501,6 +574,9 @@ static void meta_kms_init (MetaKms *kms) { kms->cursor_manager = meta_kms_cursor_manager_new (kms); + + kms->hotplug_events = + g_hash_table_new_full (g_str_hash, g_str_equal, g_free, NULL); } static void