Re: [PATCH v2 1/5] acpi/x86: s2idle: add support for Display Off and Display On callbacks

Mario Limonciello
2024-09-23T15:57:41Z

On 9/22/2024 12:22, Antheas Kapenekakis wrote:
> The Display Off and Display On firmware notifications are meant to signify
> the system entering a state where the user is not actively interacting
> with it (i.e., in Windows this state is called "Screen Off" and the
> system enters it once it turns the screen off e.g., due to inactivity).
>
> Currently, these functions are called within the suspend sequence, which
> causes issues when these notifications interact with e.g., a USB device
> and makes them unable to be called as part of the screen turning off.
>
> This patch adds a set of callbacks to allow calling the Display On/Off
> notifications outside of the suspend/resume path.
>
> Co-developed-by: Mario Limonciello <mario.limonciello@amd.com>
> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>

Make sure you run your patches through checkpatch. I don't believe you
got these tags right.

> ---
> include/linux/suspend.h | 5 +++++
> kernel/power/suspend.c | 12 ++++++++++++
> 2 files changed, 17 insertions(+)
>
> diff --git a/include/linux/suspend.h b/include/linux/suspend.h
> index da6ebca3ff77..8f33249cc067 100644
> --- a/include/linux/suspend.h
> +++ b/include/linux/suspend.h
> @@ -132,6 +132,7 @@ struct platform_suspend_ops {
> };
>
> struct platform_s2idle_ops {
> + int (*display_off)(void);
> int (*begin)(void);
> int (*prepare)(void);
> int (*prepare_late)(void);
> @@ -140,6 +141,7 @@ struct platform_s2idle_ops {
> void (*restore_early)(void);
> void (*restore)(void);
> void (*end)(void);
> + int (*display_on)(void);
> };
>
> #ifdef CONFIG_SUSPEND
> @@ -160,6 +162,9 @@ extern unsigned int pm_suspend_global_flags;
> #define PM_SUSPEND_FLAG_FW_RESUME BIT(1)
> #define PM_SUSPEND_FLAG_NO_PLATFORM BIT(2)
>
> +int platform_suspend_display_off(void);
> +int platform_suspend_display_on(void);
> +
> static inline void pm_suspend_clear_flags(void)
> {
> pm_suspend_global_flags = 0;
> diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
> index 09f8397bae15..c527dc0ae5ae 100644
> --- a/kernel/power/suspend.c
> +++ b/kernel/power/suspend.c
> @@ -254,6 +254,18 @@ static bool sleep_state_supported(suspend_state_t state)
> (valid_state(state) && !cxl_mem_active());
> }
>
> +int platform_suspend_display_off(void)
> +{
> + return s2idle_ops && s2idle_ops->display_off ? s2idle_ops->display_off() : 0;
> +}
> +EXPORT_SYMBOL_GPL(platform_suspend_display_off);
> +
> +int platform_suspend_display_on(void)
> +{
> + return s2idle_ops && s2idle_ops->display_on ? s2idle_ops->display_on() : 0;
> +}
> +EXPORT_SYMBOL_GPL(platform_suspend_display_on);
> +
> static int platform_suspend_prepare(suspend_state_t state)
> {
> return state != PM_SUSPEND_TO_IDLE && suspend_ops->prepare ?