From ec73076e07640fc0752d9ecc66c5934726db2d7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michel=20D=C3=A4nzer?= Date: Fri, 3 Jan 2025 18:46:43 +0100 Subject: [PATCH] monitor-manager/native: Remove _read_current_state specialization It forced the power save mode to META_POWER_SAVE_ON when reading the current KMS state. This was problematic when a hotplug event is emitted while the session is locked, which can be triggered by monitors polling all their inputs for a signal: There's no mechanism to restore the previous power save mode in this case, so the monitors would fail to actually enter power saving mode but stayed on with a blank screen. This was at least one cause of the symptoms described in https://gitlab.freedesktop.org/drm/amd/-/issues/662 . Moreover, I suspect it hasn't had any effect for the actual reading of KMS state since 5f6aee341959 ("kms/update: Make power saving an update wide change"), as changing the power save mode to META_POWER_SAVE_ON no longer results in any immediate KMS state change, it's now only taken into account for the next mode set. It's not clear what the intended effect was in the first place, it was originally added as part of 65db8efbe8b1 ("MonitorManager: add a KMS backend") without rationale. It might have been cargo-culted from somewhere else. It shouldn't be necessary from a KMS API PoV though. Also adjust the KMS hotplug test to assert that a hotplug event doesn't implicitly change the power save mode. v2: (Sebastian Wick) * Fix shortlog of commit which added meta_monitor_manager_native_read_current_state. v3: * Adjust KMS hotplug test for the change. Part-of: --- .../native/meta-monitor-manager-native.c | 23 ------------------- src/tests/native-kms-hotplug.c | 10 ++++---- 2 files changed, 6 insertions(+), 27 deletions(-) diff --git a/src/backends/native/meta-monitor-manager-native.c b/src/backends/native/meta-monitor-manager-native.c index 0b1d28335..b6c6ce420 100644 --- a/src/backends/native/meta-monitor-manager-native.c +++ b/src/backends/native/meta-monitor-manager-native.c @@ -99,27 +99,6 @@ meta_monitor_manager_native_read_edid (MetaMonitorManager *manager, return meta_output_native_read_edid (META_OUTPUT_NATIVE (output)); } -static void -meta_monitor_manager_native_read_current_state (MetaMonitorManager *manager) -{ - MetaMonitorManagerClass *parent_class = - META_MONITOR_MANAGER_CLASS (meta_monitor_manager_native_parent_class); - MetaPowerSave power_save_mode; - - power_save_mode = meta_monitor_manager_get_power_save_mode (manager); - if (power_save_mode != META_POWER_SAVE_ON) - { - MetaPowerSaveChangeReason reason; - - reason = META_POWER_SAVE_CHANGE_REASON_HOTPLUG; - meta_monitor_manager_power_save_mode_changed (manager, - META_POWER_SAVE_ON, - reason); - } - - parent_class->read_current_state (manager); -} - static void meta_monitor_manager_native_set_power_save_mode (MetaMonitorManager *manager, MetaPowerSave mode) @@ -712,8 +691,6 @@ meta_monitor_manager_native_class_init (MetaMonitorManagerNativeClass *klass) manager_class->read_edid = meta_monitor_manager_native_read_edid; - manager_class->read_current_state = - meta_monitor_manager_native_read_current_state; manager_class->ensure_initial_config = meta_monitor_manager_native_ensure_initial_config; manager_class->apply_monitors_config = diff --git a/src/tests/native-kms-hotplug.c b/src/tests/native-kms-hotplug.c index db97a6f72..193166e26 100644 --- a/src/tests/native-kms-hotplug.c +++ b/src/tests/native-kms-hotplug.c @@ -385,7 +385,7 @@ emulate_hotplug (void) } static void -meta_test_power_save_implicit_on (void) +meta_test_power_save_no_implicit_on (void) { MetaBackend *backend = meta_context_get_backend (test_context); MetaMonitorManager *monitor_manager = @@ -423,9 +423,11 @@ meta_test_power_save_implicit_on (void) disconnect_connector_filter, NULL); emulate_hotplug (); - while (!power_save_mode_changed || !monitors_changed) + while (!monitors_changed) g_main_context_iteration (NULL, TRUE); + g_assert_false (power_save_mode_changed); + g_signal_handler_disconnect (monitor_manager, power_save_handler_id); g_signal_handler_disconnect (monitor_manager, monitors_changed_handler_id); @@ -445,8 +447,8 @@ init_tests (void) meta_test_disconnect_connect); g_test_add_func ("/hotplug/switch-config", meta_test_switch_config); - g_test_add_func ("/hotplug/power-save-implicit-off", - meta_test_power_save_implicit_on); + g_test_add_func ("/hotplug/power-save-no-implicit-on", + meta_test_power_save_no_implicit_on); } int