Re: [PATCH] PM: Use hibernate flows for system power off

[Denis Benato <benato.denis96@gmail.com>]

> From: Mario Limonciello <mario.limonciello@amd.com>
>
> When the system is powered off the kernel will call device_shutdown()
> which will issue callbacks into PCI core to wake up a device and call
> it's shutdown() callback.  This will leave devices in ACPI D0 which can
> cause some devices to misbehave with spurious wakeups and also leave some
> devices on which will consume power needlessly.
>
> The issue won't happen if the device is in D3 before system shutdown, so
> putting device to low power state before shutdown solves the issue.
>
> ACPI Spec 6.5, "7.4.2.5 System \_S4 State" says "Devices states are
> compatible with the current Power Resource states. In other words, all
> devices are in the D3 state when the system state is S4."
>
> The following "7.4.2.6 System \_S5 State (Soft Off)" states "The S5
> state is similar to the S4 state except that OSPM does not save any
> context." so it's safe to assume devices should be at D3 for S5.
>
> To accomplish this, modify the PM core to call all the device hibernate
> callbacks when turning off the system. To avoid issues when the kernel
> is compiled without hibernate support introduce a new internal PM message
> type "SHUTDOWN" which points to all the same callbacks as hibernate would.
>
> Cc: AceLan Kao <acelan.kao@canonical.com>
> Cc: Kai-Heng Feng <kaihengf@nvidia.com>
> Cc: Mark Pearson <mpearson-lenovo@squebb.ca>
> Cc: Denis Benato <benato.denis96@gmail.com>
> Cc: Merthan Karakaş <m3rthn.k@gmail.com>
> Link: https://lore.kernel.org/linux-pci/20231213182656.6165-1-mario.limonciello@amd.com/
> Link: https://lore.kernel.org/linux-pci/20250506041934.1409302-1-superm1@kernel.org/
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
Hello,

As for the previous version this patch makes my laptop shutdown cleanly very quickly
and I could not link to any regressions or change in behavior while laptop is on.

Tested-by: Denis Benato <benato.denis96@gmail.com>
> ---
> This is the spiritual successor to "PCI/PM: Put devices to low power
> state on shutdown" as well as "Improvements to system power consumption
> at S5"
> ---
>  drivers/base/power/main.c | 6 ++++++
>  include/linux/pm.h        | 5 +++++
>  kernel/reboot.c           | 6 ++++++
>  3 files changed, 17 insertions(+)
>
> diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
> index 29561f7346d93..58adedc4dab23 100644
> --- a/drivers/base/power/main.c
> +++ b/drivers/base/power/main.c
> @@ -363,6 +363,8 @@ static pm_callback_t pm_op(const struct dev_pm_ops *ops, pm_message_t state)
>  	case PM_EVENT_RESTORE:
>  		return ops->restore;
>  #endif /* CONFIG_HIBERNATE_CALLBACKS */
> +	case PM_EVENT_SHUTDOWN:
> +		return ops->poweroff;
>  	}
>  
>  	return NULL;
> @@ -397,6 +399,8 @@ static pm_callback_t pm_late_early_op(const struct dev_pm_ops *ops,
>  	case PM_EVENT_RESTORE:
>  		return ops->restore_early;
>  #endif /* CONFIG_HIBERNATE_CALLBACKS */
> +	case PM_EVENT_SHUTDOWN:
> +		return ops->poweroff_late;
>  	}
>  
>  	return NULL;
> @@ -431,6 +435,8 @@ static pm_callback_t pm_noirq_op(const struct dev_pm_ops *ops, pm_message_t stat
>  	case PM_EVENT_RESTORE:
>  		return ops->restore_noirq;
>  #endif /* CONFIG_HIBERNATE_CALLBACKS */
> +	case PM_EVENT_SHUTDOWN:
> +		return ops->poweroff_noirq;
>  	}
>  
>  	return NULL;
> diff --git a/include/linux/pm.h b/include/linux/pm.h
> index f0bd8fbae4f2c..536defa771716 100644
> --- a/include/linux/pm.h
> +++ b/include/linux/pm.h
> @@ -490,6 +490,9 @@ const struct dev_pm_ops name = { \
>   * HIBERNATE	Hibernation image has been saved, call ->prepare() and
>   *		->poweroff() for all devices.
>   *
> + * SHUTDOWN	System is going to shut down, call ->prepare() and ->poweroff()
> + * 		for all devices.
> + *
>   * QUIESCE	Contents of main memory are going to be restored from a (loaded)
>   *		hibernation image, call ->prepare() and ->freeze() for all
>   *		devices.
> @@ -536,6 +539,7 @@ const struct dev_pm_ops name = { \
>  #define PM_EVENT_USER		0x0100
>  #define PM_EVENT_REMOTE		0x0200
>  #define PM_EVENT_AUTO		0x0400
> +#define PM_EVENT_SHUTDOWN	0x0800
>  
>  #define PM_EVENT_SLEEP		(PM_EVENT_SUSPEND | PM_EVENT_HIBERNATE)
>  #define PM_EVENT_USER_SUSPEND	(PM_EVENT_USER | PM_EVENT_SUSPEND)
> @@ -550,6 +554,7 @@ const struct dev_pm_ops name = { \
>  #define PMSG_QUIESCE	((struct pm_message){ .event = PM_EVENT_QUIESCE, })
>  #define PMSG_SUSPEND	((struct pm_message){ .event = PM_EVENT_SUSPEND, })
>  #define PMSG_HIBERNATE	((struct pm_message){ .event = PM_EVENT_HIBERNATE, })
> +#define PMSG_SHUTDOWN	((struct pm_message){ .event = PM_EVENT_SHUTDOWN, })
>  #define PMSG_RESUME	((struct pm_message){ .event = PM_EVENT_RESUME, })
>  #define PMSG_THAW	((struct pm_message){ .event = PM_EVENT_THAW, })
>  #define PMSG_RESTORE	((struct pm_message){ .event = PM_EVENT_RESTORE, })
> diff --git a/kernel/reboot.c b/kernel/reboot.c
> index ec087827c85cd..083c143f99e40 100644
> --- a/kernel/reboot.c
> +++ b/kernel/reboot.c
> @@ -13,6 +13,7 @@
>  #include <linux/kexec.h>
>  #include <linux/kmod.h>
>  #include <linux/kmsg_dump.h>
> +#include <linux/pm.h>
>  #include <linux/reboot.h>
>  #include <linux/suspend.h>
>  #include <linux/syscalls.h>
> @@ -305,7 +306,12 @@ static void kernel_shutdown_prepare(enum system_states state)
>  		(state == SYSTEM_HALT) ? SYS_HALT : SYS_POWER_OFF, NULL);
>  	system_state = state;
>  	usermodehelper_disable();
> +#ifdef CONFIG_PM_SLEEP
> +	dpm_suspend_start(PMSG_SHUTDOWN);
> +	dpm_suspend_end(PMSG_SHUTDOWN);
> +#else
>  	device_shutdown();
> +#endif
>  }
>  /**
>   *	kernel_halt - halt the system