[PATCH 0/2] Fixups for cancelled hibernate

[PATCH 0/2] Fixups for cancelled hibernate

[Mario Limonciello]

Muhammad Usama Anjun's recent series for being able to cancel
the hibernate sequence [1] exposes a bug with amdgpu handling for
skipping the thaw step.

Because the thaw step is skipped in most cases, cancelling the
hibernate means that the device is left in an inconsistent
state.

Add support to the PM core to let a driver track it's frozen
state when an error code is returned during thaw(). This will
ensure that the poweroff() and restore() callbacks get run.

changes since RFC:
 * Drop first patch
 * Introduce is_frozen

Cc: Muhammad Usama Anjum <usama.anjum@collabora.com>

Mario Limonciello (AMD) (2):
  PM: Allow device drivers to manage the frozen state of a device
  drm/amd: Manage frozen state internally

 Documentation/driver-api/pm/devices.rst    |  8 ++++++++
 drivers/base/power/main.c                  |  5 +++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 10 ++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c    |  2 +-
 include/linux/pm.h                         |  3 +++
 5 files changed, 27 insertions(+), 1 deletion(-)

-- 
2.51.0

[PATCH 1/2] PM: Allow device drivers to manage the frozen state of a device

[Mario Limonciello]

From: "Mario Limonciello (AMD)" <superm1@kernel.org>

During a normal successful hibernate sequence devices will go through
the freeze() callbacks create an image, go through the thaw() callbacks,
and poweroff() callbacks.

During a successful hibernate sequence some device drivers may want to
skip the thaw() callbacks.  This confuses the PM core though because it
thinks the device is no longer suspended.

To accommodate drivers that want to do this, introduce a new is_frozen
bit that the driver can set and manage.  From the driver perspective
any thaw() or restore() callbacks that are being skipped should set
is_frozen and return an error code.  The PM core will then put the
device back into the list of devices to resume for any aborted hibernate.

Cc: Muhammad Usama Anjum <usama.anjum@collabora.com>
Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
---
 Documentation/driver-api/pm/devices.rst | 8 ++++++++
 drivers/base/power/main.c               | 5 +++++
 include/linux/pm.h                      | 3 +++
 3 files changed, 16 insertions(+)

diff --git a/Documentation/driver-api/pm/devices.rst b/Documentation/driver-api/pm/devices.rst
index 36d5c9c9fd11..55c633727108 100644
--- a/Documentation/driver-api/pm/devices.rst
+++ b/Documentation/driver-api/pm/devices.rst
@@ -578,6 +578,14 @@ should already have been stored during the ``freeze``, ``freeze_late`` or
 the entire system, so it is not necessary for the callback to put the device in
 a low-power state.
 
+Skipping thaw phase
+-------------------
+In some rare situations, it may be desirable to skip the thaw phases
+(``thaw_noirq``, ``thaw_early``, ``thaw``) of a device entirely.  This can be
+achieved by a device driver returning an error code from any of it's thaw
+callbacks but also setting dev->power.is_frozen to true.  This indicates to the
+PM core that the device is still in the frozen state.  The PM core will consider
+this when resuming the device in later phases such as `restore` or `poweroff`.
 
 Leaving Hibernation
 -------------------
diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index e83503bdc1fd..451d54486645 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -1100,6 +1100,11 @@ static void device_resume(struct device *dev, pm_message_t state, bool async)
 
  End:
 	error = dpm_run_callback(callback, dev, state, info);
+	/* device manages frozen state */
+	if (error && dev->power.is_frozen) {
+		dev->power.is_suspended = true;
+		error = 0;
+	}
 
 	device_unlock(dev);
 	dpm_watchdog_clear(&wd);
diff --git a/include/linux/pm.h b/include/linux/pm.h
index cc7b2dc28574..52ee38d72aa2 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -688,6 +688,9 @@ struct dev_pm_info {
 #else
 	bool			should_wakeup:1;
 #endif
+#ifdef CONFIG_HIBERNATE_CALLBACKS
+	bool			is_frozen:1;	/* Owned by the driver */
+#endif
 #ifdef CONFIG_PM
 	struct hrtimer		suspend_timer;
 	u64			timer_expires;
-- 
2.51.0

Re: [PATCH 1/2] PM: Allow device drivers to manage the frozen state of a device

[kernel test robot]

Hi Mario,

kernel test robot noticed the following build errors:

[auto build test ERROR on rafael-pm/linux-next]
[also build test ERROR on rafael-pm/bleeding-edge linus/master v6.18-rc2 next-20251023]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Mario-Limonciello/PM-Allow-device-drivers-to-manage-the-frozen-state-of-a-device/20251022-235319
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
patch link:    https://lore.kernel.org/r/20251022155114.48418-2-mario.limonciello%40amd.com
patch subject: [PATCH 1/2] PM: Allow device drivers to manage the frozen state of a device
config: riscv-randconfig-001-20251023 (https://download.01.org/0day-ci/archive/20251023/202510231647.os6nFPgQ-lkp@intel.com/config)
compiler: riscv32-linux-gcc (GCC) 8.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251023/202510231647.os6nFPgQ-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202510231647.os6nFPgQ-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/base/power/main.c: In function 'device_resume':
>> drivers/base/power/main.c:1114:25: error: 'struct dev_pm_info' has no member named 'is_frozen'
     if (error && dev->power.is_frozen) {
                            ^


vim +1114 drivers/base/power/main.c

  1026	
  1027	/**
  1028	 * device_resume - Execute "resume" callbacks for given device.
  1029	 * @dev: Device to handle.
  1030	 * @state: PM transition of the system being carried out.
  1031	 * @async: If true, the device is being resumed asynchronously.
  1032	 */
  1033	static void device_resume(struct device *dev, pm_message_t state, bool async)
  1034	{
  1035		pm_callback_t callback = NULL;
  1036		const char *info = NULL;
  1037		int error = 0;
  1038		DECLARE_DPM_WATCHDOG_ON_STACK(wd);
  1039	
  1040		TRACE_DEVICE(dev);
  1041		TRACE_RESUME(0);
  1042	
  1043		if (dev->power.syscore)
  1044			goto Complete;
  1045	
  1046		if (!dev->power.is_suspended)
  1047			goto Complete;
  1048	
  1049		dev->power.is_suspended = false;
  1050	
  1051		if (dev->power.direct_complete) {
  1052			/*
  1053			 * Allow new children to be added under the device after this
  1054			 * point if it has no PM callbacks.
  1055			 */
  1056			if (dev->power.no_pm_callbacks)
  1057				dev->power.is_prepared = false;
  1058	
  1059			/* Match the pm_runtime_disable() in device_suspend(). */
  1060			pm_runtime_enable(dev);
  1061			goto Complete;
  1062		}
  1063	
  1064		if (!dpm_wait_for_superior(dev, async))
  1065			goto Complete;
  1066	
  1067		dpm_watchdog_set(&wd, dev);
  1068		device_lock(dev);
  1069	
  1070		/*
  1071		 * This is a fib.  But we'll allow new children to be added below
  1072		 * a resumed device, even if the device hasn't been completed yet.
  1073		 */
  1074		dev->power.is_prepared = false;
  1075	
  1076		if (dev->pm_domain) {
  1077			info = "power domain ";
  1078			callback = pm_op(&dev->pm_domain->ops, state);
  1079			goto Driver;
  1080		}
  1081	
  1082		if (dev->type && dev->type->pm) {
  1083			info = "type ";
  1084			callback = pm_op(dev->type->pm, state);
  1085			goto Driver;
  1086		}
  1087	
  1088		if (dev->class && dev->class->pm) {
  1089			info = "class ";
  1090			callback = pm_op(dev->class->pm, state);
  1091			goto Driver;
  1092		}
  1093	
  1094		if (dev->bus) {
  1095			if (dev->bus->pm) {
  1096				info = "bus ";
  1097				callback = pm_op(dev->bus->pm, state);
  1098			} else if (dev->bus->resume) {
  1099				info = "legacy bus ";
  1100				callback = dev->bus->resume;
  1101				goto End;
  1102			}
  1103		}
  1104	
  1105	 Driver:
  1106		if (!callback && dev->driver && dev->driver->pm) {
  1107			info = "driver ";
  1108			callback = pm_op(dev->driver->pm, state);
  1109		}
  1110	
  1111	 End:
  1112		error = dpm_run_callback(callback, dev, state, info);
  1113		/* device manages frozen state */
> 1114		if (error && dev->power.is_frozen) {
  1115			dev->power.is_suspended = true;
  1116			error = 0;
  1117		}
  1118	
  1119		device_unlock(dev);
  1120		dpm_watchdog_clear(&wd);
  1121	
  1122	 Complete:
  1123		complete_all(&dev->power.completion);
  1124	
  1125		TRACE_RESUME(error);
  1126	
  1127		if (error) {
  1128			WRITE_ONCE(async_error, error);
  1129			dpm_save_failed_dev(dev_name(dev));
  1130			pm_dev_err(dev, state, async ? " async" : "", error);
  1131		}
  1132	
  1133		dpm_async_resume_subordinate(dev, async_resume);
  1134	}
  1135	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 1/2] PM: Allow device drivers to manage the frozen state of a device

[Muhammad Usama Anjum]

On 10/22/25 8:50 PM, Mario Limonciello wrote:
> From: "Mario Limonciello (AMD)" <superm1@kernel.org>
> 
> During a normal successful hibernate sequence devices will go through
> the freeze() callbacks create an image, go through the thaw() callbacks,
> and poweroff() callbacks.
> 
> During a successful hibernate sequence some device drivers may want to
> skip the thaw() callbacks.  This confuses the PM core though because it
> thinks the device is no longer suspended.
> 
> To accommodate drivers that want to do this, introduce a new is_frozen
> bit that the driver can set and manage.  From the driver perspective
> any thaw() or restore() callbacks that are being skipped should set
> is_frozen and return an error code.  The PM core will then put the
> device back into the list of devices to resume for any aborted hibernate.
> 
> Cc: Muhammad Usama Anjum <usama.anjum@collabora.com>
> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
Tested-by: Muhammad Usama Anjum <usama.anjum@collabora.com>

> ---
>  Documentation/driver-api/pm/devices.rst | 8 ++++++++
>  drivers/base/power/main.c               | 5 +++++
>  include/linux/pm.h                      | 3 +++
>  3 files changed, 16 insertions(+)
> 
> diff --git a/Documentation/driver-api/pm/devices.rst b/Documentation/driver-api/pm/devices.rst
> index 36d5c9c9fd11..55c633727108 100644
> --- a/Documentation/driver-api/pm/devices.rst
> +++ b/Documentation/driver-api/pm/devices.rst
> @@ -578,6 +578,14 @@ should already have been stored during the ``freeze``, ``freeze_late`` or
>  the entire system, so it is not necessary for the callback to put the device in
>  a low-power state.
>  
> +Skipping thaw phase
> +-------------------
> +In some rare situations, it may be desirable to skip the thaw phases
> +(``thaw_noirq``, ``thaw_early``, ``thaw``) of a device entirely.  This can be
> +achieved by a device driver returning an error code from any of it's thaw
> +callbacks but also setting dev->power.is_frozen to true.  This indicates to the
> +PM core that the device is still in the frozen state.  The PM core will consider
> +this when resuming the device in later phases such as `restore` or `poweroff`.
>  
>  Leaving Hibernation
>  -------------------
> diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
> index e83503bdc1fd..451d54486645 100644
> --- a/drivers/base/power/main.c
> +++ b/drivers/base/power/main.c
> @@ -1100,6 +1100,11 @@ static void device_resume(struct device *dev, pm_message_t state, bool async)
>  
>   End:
>  	error = dpm_run_callback(callback, dev, state, info);
> +	/* device manages frozen state */
> +	if (error && dev->power.is_frozen) {
> +		dev->power.is_suspended = true;
> +		error = 0;
> +	}
>  
>  	device_unlock(dev);
>  	dpm_watchdog_clear(&wd);
> diff --git a/include/linux/pm.h b/include/linux/pm.h
> index cc7b2dc28574..52ee38d72aa2 100644
> --- a/include/linux/pm.h
> +++ b/include/linux/pm.h
> @@ -688,6 +688,9 @@ struct dev_pm_info {
>  #else
>  	bool			should_wakeup:1;
>  #endif
> +#ifdef CONFIG_HIBERNATE_CALLBACKS
> +	bool			is_frozen:1;	/* Owned by the driver */
> +#endif
>  #ifdef CONFIG_PM
>  	struct hrtimer		suspend_timer;
>  	u64			timer_expires;


-- 
---
Thanks,
Usama

Re: [PATCH 1/2] PM: Allow device drivers to manage the frozen state of a device

[kernel test robot]

Hi Mario,

kernel test robot noticed the following build errors:

[auto build test ERROR on rafael-pm/linux-next]
[also build test ERROR on rafael-pm/bleeding-edge linus/master v6.18-rc2 next-20251023]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Mario-Limonciello/PM-Allow-device-drivers-to-manage-the-frozen-state-of-a-device/20251022-235319
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
patch link:    https://lore.kernel.org/r/20251022155114.48418-2-mario.limonciello%40amd.com
patch subject: [PATCH 1/2] PM: Allow device drivers to manage the frozen state of a device
config: x86_64-randconfig-003-20251023 (https://download.01.org/0day-ci/archive/20251023/202510231933.86hrXJ9i-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251023/202510231933.86hrXJ9i-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202510231933.86hrXJ9i-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/base/power/main.c:1114:26: error: no member named 'is_frozen' in 'struct dev_pm_info'
    1114 |         if (error && dev->power.is_frozen) {
         |                      ~~~~~~~~~~ ^
   1 error generated.


vim +1114 drivers/base/power/main.c

  1026	
  1027	/**
  1028	 * device_resume - Execute "resume" callbacks for given device.
  1029	 * @dev: Device to handle.
  1030	 * @state: PM transition of the system being carried out.
  1031	 * @async: If true, the device is being resumed asynchronously.
  1032	 */
  1033	static void device_resume(struct device *dev, pm_message_t state, bool async)
  1034	{
  1035		pm_callback_t callback = NULL;
  1036		const char *info = NULL;
  1037		int error = 0;
  1038		DECLARE_DPM_WATCHDOG_ON_STACK(wd);
  1039	
  1040		TRACE_DEVICE(dev);
  1041		TRACE_RESUME(0);
  1042	
  1043		if (dev->power.syscore)
  1044			goto Complete;
  1045	
  1046		if (!dev->power.is_suspended)
  1047			goto Complete;
  1048	
  1049		dev->power.is_suspended = false;
  1050	
  1051		if (dev->power.direct_complete) {
  1052			/*
  1053			 * Allow new children to be added under the device after this
  1054			 * point if it has no PM callbacks.
  1055			 */
  1056			if (dev->power.no_pm_callbacks)
  1057				dev->power.is_prepared = false;
  1058	
  1059			/* Match the pm_runtime_disable() in device_suspend(). */
  1060			pm_runtime_enable(dev);
  1061			goto Complete;
  1062		}
  1063	
  1064		if (!dpm_wait_for_superior(dev, async))
  1065			goto Complete;
  1066	
  1067		dpm_watchdog_set(&wd, dev);
  1068		device_lock(dev);
  1069	
  1070		/*
  1071		 * This is a fib.  But we'll allow new children to be added below
  1072		 * a resumed device, even if the device hasn't been completed yet.
  1073		 */
  1074		dev->power.is_prepared = false;
  1075	
  1076		if (dev->pm_domain) {
  1077			info = "power domain ";
  1078			callback = pm_op(&dev->pm_domain->ops, state);
  1079			goto Driver;
  1080		}
  1081	
  1082		if (dev->type && dev->type->pm) {
  1083			info = "type ";
  1084			callback = pm_op(dev->type->pm, state);
  1085			goto Driver;
  1086		}
  1087	
  1088		if (dev->class && dev->class->pm) {
  1089			info = "class ";
  1090			callback = pm_op(dev->class->pm, state);
  1091			goto Driver;
  1092		}
  1093	
  1094		if (dev->bus) {
  1095			if (dev->bus->pm) {
  1096				info = "bus ";
  1097				callback = pm_op(dev->bus->pm, state);
  1098			} else if (dev->bus->resume) {
  1099				info = "legacy bus ";
  1100				callback = dev->bus->resume;
  1101				goto End;
  1102			}
  1103		}
  1104	
  1105	 Driver:
  1106		if (!callback && dev->driver && dev->driver->pm) {
  1107			info = "driver ";
  1108			callback = pm_op(dev->driver->pm, state);
  1109		}
  1110	
  1111	 End:
  1112		error = dpm_run_callback(callback, dev, state, info);
  1113		/* device manages frozen state */
> 1114		if (error && dev->power.is_frozen) {
  1115			dev->power.is_suspended = true;
  1116			error = 0;
  1117		}
  1118	
  1119		device_unlock(dev);
  1120		dpm_watchdog_clear(&wd);
  1121	
  1122	 Complete:
  1123		complete_all(&dev->power.completion);
  1124	
  1125		TRACE_RESUME(error);
  1126	
  1127		if (error) {
  1128			WRITE_ONCE(async_error, error);
  1129			dpm_save_failed_dev(dev_name(dev));
  1130			pm_dev_err(dev, state, async ? " async" : "", error);
  1131		}
  1132	
  1133		dpm_async_resume_subordinate(dev, async_resume);
  1134	}
  1135	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 2/2] drm/amd: Manage frozen state internally

[Mario Limonciello]

On 10/22/25 10:50 AM, Mario Limonciello wrote:
> From: "Mario Limonciello (AMD)" <superm1@kernel.org>
> 
> [Why]
> On a normal hibernate sequence amdgpu will skip the thaw step due to
> commit 530694f54dd5e ("drm/amdgpu: do not resume device in thaw for
> normal hibernation").
> 
> If the hibernate sequence has been aborted however after this thawed
> step runs the PM core will think the device is suspended and will skip
> the restore() sequence for amdgpu.  This leads to accessing the device
> while in a low power state and will freeze the system.
> 
> [How]
> Set `dev->power.is_frozen` to indicate to the PM core that an error
> code will be returned for thaw() callback because driver managed the
> frozen state.  If the restore() callback is called by the PM core the
> driver will resume the device.
> 
> Cc: Muhammad Usama Anjum <usama.anjum@collabora.com>
> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
> ---

Explicitly add Rafael and linux-pm as kw failed to do so (sorry).

Here is the lore link for patch 2:

https://lore.kernel.org/amd-gfx/20251022155114.48418-3-mario.limonciello@amd.com/

>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 10 ++++++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c    |  2 +-
>   2 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 3d032c4e2dce..693347eb6861 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -5247,6 +5247,11 @@ int amdgpu_device_suspend(struct drm_device *dev, bool notify_clients)
>   	if (r)
>   		return r;
>   
> +#ifdef CONFIG_HIBERNATE_CALLBACKS
> +	if (adev->in_s4)
> +		dev->dev->power.is_frozen = 1;
> +#endif
> +
>   	return 0;
>   }
>   
> @@ -5385,6 +5390,11 @@ int amdgpu_device_resume(struct drm_device *dev, bool notify_clients)
>   	if (amdgpu_acpi_smart_shift_update(adev, AMDGPU_SS_DEV_D0))
>   		dev_warn(adev->dev, "smart shift update failed\n");
>   
> +#ifdef CONFIG_HIBERNATE_CALLBACKS
> +	if (adev->in_s4)
> +		dev->dev->power.is_frozen = 0;
> +#endif
> +
>   	return 0;
>   }
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 61268aa82df4..d40af069f24d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -2681,7 +2681,7 @@ static int amdgpu_pmops_thaw(struct device *dev)
>   
>   	/* do not resume device if it's normal hibernation */
>   	if (!pm_hibernate_is_recovering() && !pm_hibernation_mode_is_suspend())
> -		return 0;
> +		return -EBUSY;
>   
>   	return amdgpu_device_resume(drm_dev, true);
>   }