[PATCH v1 2/4] acpi/x86: s2idle: handle screen off/on calls outside of suspend sequence
Mario Limonciello
mario.limonciello at amd.com
Thu Sep 19 17:35:29 UTC 2024
+dri-devel
For those joining late; this is the full series for context.
https://lore.kernel.org/linux-pm/20240919171952.403745-1-lkml@antheas.dev/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 at antheas.dev>
> ---
> 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:
More information about the dri-devel
mailing list