Re: [PATCH v1 2/4] acpi/x86: s2idle: handle screen off/on calls outside of suspend sequence

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



+dri-devel

For those joining late; this is the full series for context.

https://lore.kernel.org/linux-pm/20240919171952.403745-1-lkml@xxxxxxxxxxx/T/#maee308be5349d8df25c8ccf12144ea96bbd4cbbd

On 9/19/2024 12:19, Antheas Kapenekakis wrote:
Currently, the screen off/on calls are handled within the suspend
sequence, which is a deviation from Windows. This causes issues with
certain devices, such as the ROG Ally, which expects this call to be
executed with the kernel fully awake. The subsequent half-suspended
state makes the controller of the device to fail to suspend properly.

This patch calls the screen off/on callbacks before entering the suspend
sequence, which fixes this issue. In addition, it opens the possibility
of modelling a state such as "Screen Off" that mirrors Windows, as the
callbacks will be accessible and validated to work outside of the
suspend sequence.

Signed-off-by: Antheas Kapenekakis <lkml@xxxxxxxxxxx>
---
  kernel/power/suspend.c | 16 ++++++++++++++++
  1 file changed, 16 insertions(+)

diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
index 19734b297527..afa95271ef00 100644
--- a/kernel/power/suspend.c
+++ b/kernel/power/suspend.c
@@ -507,6 +507,19 @@ int suspend_devices_and_enter(suspend_state_t state)
pm_suspend_target_state = state; + /*
+	 * Linux does not have the concept of a "Screen Off" state, so call
+	 * the platform functions for screen off prior to beginning the suspend
+	 * sequence, mirroring Windows which calls them outside of it as well.
+	 *
+	 * If Linux ever gains a "Screen Off" state, the following callbacks can
+	 * be replaced with a call that checks if we are in "Screen Off", in which
+	 * case they will NOOP and if not call them as a fallback.
+	 */
+	error = platform_suspend_screen_off();

It's a bit muddy; but I wonder if calling drm_atomic_helper_disable_all() makes sense here.

+	if (error)
+		goto Screen_on;
+
  	if (state == PM_SUSPEND_TO_IDLE)
  		pm_set_suspend_no_platform();
@@ -540,6 +553,9 @@ int suspend_devices_and_enter(suspend_state_t state)
   Close:
  	platform_resume_end(state);
  	pm_suspend_target_state = PM_SUSPEND_ON;
+
+ Screen_on:
+	platform_suspend_screen_on();

The problem with my suggestion above is what would you put here for symmetry? drm_atomic_helper_resume() doesn't look right to me.

Maybe it's a no-op from DRM perspective and the drivers handle it.

  	return error;
Recover_platform:




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux