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

[Mario Limonciello <mario.limonciello@amd.com>]

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 ?