[PATCH v5 0/5] Unwind failed suspend

[PATCH v5 0/5] Unwind failed suspend

[Mario Limonciello (AMD)]

If a suspend fails the PM core doesn't clean it up, the device
is just left in a bad state.  If this happens during memory pressure
it could be a hung system from just trying to suspend.

For all phases of suspend that return an error code, add an unwind
flow that will (try to) resume exactly the parts that have failed.

If this fails, then reset the GPU during complete() callback.

v5:
 * Take RLC patch from Alex's Van Gogh series, slight modifications
 * Unwind in middle of IP suspend too
 * Fix missing call to fix console
 * Cover issues with DPM_FLAG_SMART_SUSPEND

Alex Deucher (1):
  drm/amdgpu: Drop PMFW RLC notifier from amdgpu_device_suspend()

Mario Limonciello (AMD) (3):
  drm/amd: Reset the GPU if pmops failed
  drm/amd: Add an unwind for failures in
    amdgpu_device_ip_suspend_phase1()
  drm/amd: Add an unwind for failures in
    amdgpu_device_ip_suspend_phase2()
  drm/amd: Unwind for failed device suspend

 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 121 ++++++++++++++++++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c    |  11 ++
 drivers/gpu/drm/amd/pm/amdgpu_dpm.c        |  18 ---
 drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h    |   2 -
 4 files changed, 117 insertions(+), 35 deletions(-)

-- 
2.51.1

[PATCH v5 1/5] drm/amdgpu: Drop PMFW RLC notifier from amdgpu_device_suspend()

[Mario Limonciello (AMD)]

From: Alex Deucher <alexander.deucher@amd.com>

For S3 on vangogh, PMFW needs to be notified before the
driver powers down RLC.  This already happens in smu_disable_dpms()
so drop the superfluous call in amdgpu_device_suspend().

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Co-developed-by: Mario Limonciello (AMD) <superm1@kernel.org>
Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
---
Cc: lkml@antheas.dev
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  4 ----
 drivers/gpu/drm/amd/pm/amdgpu_dpm.c        | 18 ------------------
 drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h    |  2 --
 3 files changed, 24 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index b8d91247f51a..f6850b86e96f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -5280,10 +5280,6 @@ int amdgpu_device_suspend(struct drm_device *dev, bool notify_clients)
 	if (amdgpu_sriov_vf(adev))
 		amdgpu_virt_release_full_gpu(adev, false);
 
-	r = amdgpu_dpm_notify_rlc_state(adev, false);
-	if (r)
-		return r;
-
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/amd/pm/amdgpu_dpm.c b/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
index 5d08dc3b7110..5c4d0eb198c4 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
@@ -195,24 +195,6 @@ int amdgpu_dpm_set_mp1_state(struct amdgpu_device *adev,
 	return ret;
 }
 
-int amdgpu_dpm_notify_rlc_state(struct amdgpu_device *adev, bool en)
-{
-	int ret = 0;
-	const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
-
-	if (pp_funcs && pp_funcs->notify_rlc_state) {
-		mutex_lock(&adev->pm.mutex);
-
-		ret = pp_funcs->notify_rlc_state(
-				adev->powerplay.pp_handle,
-				en);
-
-		mutex_unlock(&adev->pm.mutex);
-	}
-
-	return ret;
-}
-
 int amdgpu_dpm_is_baco_supported(struct amdgpu_device *adev)
 {
 	const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
index 3bce74f8bb0a..c7ea29385682 100644
--- a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
+++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
@@ -424,8 +424,6 @@ int amdgpu_dpm_mode1_reset(struct amdgpu_device *adev);
 int amdgpu_dpm_set_mp1_state(struct amdgpu_device *adev,
 			     enum pp_mp1_state mp1_state);
 
-int amdgpu_dpm_notify_rlc_state(struct amdgpu_device *adev, bool en);
-
 int amdgpu_dpm_set_gfx_power_up_by_imu(struct amdgpu_device *adev);
 
 int amdgpu_dpm_baco_exit(struct amdgpu_device *adev);
-- 
2.51.1

Re: [PATCH v5 1/5] drm/amdgpu: Drop PMFW RLC notifier from amdgpu_device_suspend()

[Antheas Kapenekakis]

On Sun, 26 Oct 2025 at 05:30, Mario Limonciello (AMD)
<superm1@kernel.org> wrote:
>
> From: Alex Deucher <alexander.deucher@amd.com>
>
> For S3 on vangogh, PMFW needs to be notified before the
> driver powers down RLC.  This already happens in smu_disable_dpms()
> so drop the superfluous call in amdgpu_device_suspend().
>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> Co-developed-by: Mario Limonciello (AMD) <superm1@kernel.org>
> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>

Just for this patch:

Tested-by: Antheas Kapenekakis <lkml@antheas.dev>

If the subject is refactored to take into account that it fixes sleep add:
#Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/4659
#Reported-by: Antheas Kapenekakis <lkml@antheas.dev>

Tested on a Steam Deck OLED and Xbox Ally.

@Mario: For my series, can you have a look at the first two patches
and if they are ok push forward with merging? Also, reminder for the
Legion Go 2 quirk.

Best,
Antheas

> ---
> Cc: lkml@antheas.dev
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  4 ----
>  drivers/gpu/drm/amd/pm/amdgpu_dpm.c        | 18 ------------------
>  drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h    |  2 --
>  3 files changed, 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index b8d91247f51a..f6850b86e96f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -5280,10 +5280,6 @@ int amdgpu_device_suspend(struct drm_device *dev, bool notify_clients)
>         if (amdgpu_sriov_vf(adev))
>                 amdgpu_virt_release_full_gpu(adev, false);
>
> -       r = amdgpu_dpm_notify_rlc_state(adev, false);
> -       if (r)
> -               return r;
> -
>         return 0;
>  }
>
> diff --git a/drivers/gpu/drm/amd/pm/amdgpu_dpm.c b/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
> index 5d08dc3b7110..5c4d0eb198c4 100644
> --- a/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
> +++ b/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
> @@ -195,24 +195,6 @@ int amdgpu_dpm_set_mp1_state(struct amdgpu_device *adev,
>         return ret;
>  }
>
> -int amdgpu_dpm_notify_rlc_state(struct amdgpu_device *adev, bool en)
> -{
> -       int ret = 0;
> -       const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
> -
> -       if (pp_funcs && pp_funcs->notify_rlc_state) {
> -               mutex_lock(&adev->pm.mutex);
> -
> -               ret = pp_funcs->notify_rlc_state(
> -                               adev->powerplay.pp_handle,
> -                               en);
> -
> -               mutex_unlock(&adev->pm.mutex);
> -       }
> -
> -       return ret;
> -}
> -
>  int amdgpu_dpm_is_baco_supported(struct amdgpu_device *adev)
>  {
>         const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
> diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
> index 3bce74f8bb0a..c7ea29385682 100644
> --- a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
> +++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
> @@ -424,8 +424,6 @@ int amdgpu_dpm_mode1_reset(struct amdgpu_device *adev);
>  int amdgpu_dpm_set_mp1_state(struct amdgpu_device *adev,
>                              enum pp_mp1_state mp1_state);
>
> -int amdgpu_dpm_notify_rlc_state(struct amdgpu_device *adev, bool en);
> -
>  int amdgpu_dpm_set_gfx_power_up_by_imu(struct amdgpu_device *adev);
>
>  int amdgpu_dpm_baco_exit(struct amdgpu_device *adev);
> --
> 2.51.1
>
>

Re: [PATCH v5 1/5] drm/amdgpu: Drop PMFW RLC notifier from amdgpu_device_suspend()

[Mario Limonciello (AMD) (kernel.org)]

On 10/31/2025 7:01 AM, Antheas Kapenekakis wrote:
> On Sun, 26 Oct 2025 at 05:30, Mario Limonciello (AMD)
> <superm1@kernel.org> wrote:
>>
>> From: Alex Deucher <alexander.deucher@amd.com>
>>
>> For S3 on vangogh, PMFW needs to be notified before the
>> driver powers down RLC.  This already happens in smu_disable_dpms()
>> so drop the superfluous call in amdgpu_device_suspend().
>>
>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>> Co-developed-by: Mario Limonciello (AMD) <superm1@kernel.org>
>> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
> 
> Just for this patch:
> 
> Tested-by: Antheas Kapenekakis <lkml@antheas.dev>
> 
> If the subject is refactored to take into account that it fixes sleep add:
> #Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/4659
> #Reported-by: Antheas Kapenekakis <lkml@antheas.dev>
> 
> Tested on a Steam Deck OLED and Xbox Ally.

Thanks for checking.  This series is already merged so too late for 
tags.  But I'll add your tags to the other patch.

> 
> @Mario: For my series, can you have a look at the first two patches
> and if they are ok push forward with merging? Also, reminder for the
> Legion Go 2 quirk.

I'll look at the two platform-x86 ones again and leave comments if 
necessary.

> 
> Best,
> Antheas
> 
>> ---
>> Cc: lkml@antheas.dev
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  4 ----
>>   drivers/gpu/drm/amd/pm/amdgpu_dpm.c        | 18 ------------------
>>   drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h    |  2 --
>>   3 files changed, 24 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> index b8d91247f51a..f6850b86e96f 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -5280,10 +5280,6 @@ int amdgpu_device_suspend(struct drm_device *dev, bool notify_clients)
>>          if (amdgpu_sriov_vf(adev))
>>                  amdgpu_virt_release_full_gpu(adev, false);
>>
>> -       r = amdgpu_dpm_notify_rlc_state(adev, false);
>> -       if (r)
>> -               return r;
>> -
>>          return 0;
>>   }
>>
>> diff --git a/drivers/gpu/drm/amd/pm/amdgpu_dpm.c b/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
>> index 5d08dc3b7110..5c4d0eb198c4 100644
>> --- a/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
>> +++ b/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
>> @@ -195,24 +195,6 @@ int amdgpu_dpm_set_mp1_state(struct amdgpu_device *adev,
>>          return ret;
>>   }
>>
>> -int amdgpu_dpm_notify_rlc_state(struct amdgpu_device *adev, bool en)
>> -{
>> -       int ret = 0;
>> -       const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
>> -
>> -       if (pp_funcs && pp_funcs->notify_rlc_state) {
>> -               mutex_lock(&adev->pm.mutex);
>> -
>> -               ret = pp_funcs->notify_rlc_state(
>> -                               adev->powerplay.pp_handle,
>> -                               en);
>> -
>> -               mutex_unlock(&adev->pm.mutex);
>> -       }
>> -
>> -       return ret;
>> -}
>> -
>>   int amdgpu_dpm_is_baco_supported(struct amdgpu_device *adev)
>>   {
>>          const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs;
>> diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
>> index 3bce74f8bb0a..c7ea29385682 100644
>> --- a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
>> +++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
>> @@ -424,8 +424,6 @@ int amdgpu_dpm_mode1_reset(struct amdgpu_device *adev);
>>   int amdgpu_dpm_set_mp1_state(struct amdgpu_device *adev,
>>                               enum pp_mp1_state mp1_state);
>>
>> -int amdgpu_dpm_notify_rlc_state(struct amdgpu_device *adev, bool en);
>> -
>>   int amdgpu_dpm_set_gfx_power_up_by_imu(struct amdgpu_device *adev);
>>
>>   int amdgpu_dpm_baco_exit(struct amdgpu_device *adev);
>> --
>> 2.51.1
>>
>>
> 

[PATCH v5 2/5] drm/amd: Add an unwind for failures in amdgpu_device_ip_suspend_phase1()

[Mario Limonciello (AMD)]

If any hardware IPs involved with the first phase of suspend fail, unwind
all steps to restore back to original state.

Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index f6850b86e96f..b9ea91b2c92f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -178,6 +178,7 @@ struct amdgpu_init_level amdgpu_init_minimal_xgmi = {
 		BIT(AMD_IP_BLOCK_TYPE_COMMON) | BIT(AMD_IP_BLOCK_TYPE_IH) |
 		BIT(AMD_IP_BLOCK_TYPE_PSP)
 };
+static int amdgpu_device_ip_resume_phase3(struct amdgpu_device *adev);
 
 static void amdgpu_device_load_switch_state(struct amdgpu_device *adev);
 
@@ -3784,7 +3785,7 @@ static void amdgpu_device_delay_enable_gfx_off(struct work_struct *work)
  */
 static int amdgpu_device_ip_suspend_phase1(struct amdgpu_device *adev)
 {
-	int i, r;
+	int i, r, rec;
 
 	amdgpu_device_set_pg_state(adev, AMD_PG_STATE_UNGATE);
 	amdgpu_device_set_cg_state(adev, AMD_CG_STATE_UNGATE);
@@ -3807,10 +3808,23 @@ static int amdgpu_device_ip_suspend_phase1(struct amdgpu_device *adev)
 
 		r = amdgpu_ip_block_suspend(&adev->ip_blocks[i]);
 		if (r)
-			return r;
+			goto unwind;
 	}
 
 	return 0;
+unwind:
+	rec = amdgpu_device_ip_resume_phase3(adev);
+	if (rec)
+		dev_err(adev->dev,
+			"amdgpu_device_ip_resume_phase3 failed during unwind: %d\n",
+			rec);
+
+	amdgpu_dpm_set_df_cstate(adev, DF_CSTATE_ALLOW);
+
+	amdgpu_device_set_pg_state(adev, AMD_PG_STATE_GATE);
+	amdgpu_device_set_cg_state(adev, AMD_CG_STATE_GATE);
+
+	return r;
 }
 
 /**
-- 
2.51.1

Re: [PATCH v5 2/5] drm/amd: Add an unwind for failures in amdgpu_device_ip_suspend_phase1()

[Alex Deucher]

Patches 2-4 are:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

On Sun, Oct 26, 2025 at 12:36 AM Mario Limonciello (AMD)
<superm1@kernel.org> wrote:
>
> If any hardware IPs involved with the first phase of suspend fail, unwind
> all steps to restore back to original state.
>
> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index f6850b86e96f..b9ea91b2c92f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -178,6 +178,7 @@ struct amdgpu_init_level amdgpu_init_minimal_xgmi = {
>                 BIT(AMD_IP_BLOCK_TYPE_COMMON) | BIT(AMD_IP_BLOCK_TYPE_IH) |
>                 BIT(AMD_IP_BLOCK_TYPE_PSP)
>  };
> +static int amdgpu_device_ip_resume_phase3(struct amdgpu_device *adev);
>
>  static void amdgpu_device_load_switch_state(struct amdgpu_device *adev);
>
> @@ -3784,7 +3785,7 @@ static void amdgpu_device_delay_enable_gfx_off(struct work_struct *work)
>   */
>  static int amdgpu_device_ip_suspend_phase1(struct amdgpu_device *adev)
>  {
> -       int i, r;
> +       int i, r, rec;
>
>         amdgpu_device_set_pg_state(adev, AMD_PG_STATE_UNGATE);
>         amdgpu_device_set_cg_state(adev, AMD_CG_STATE_UNGATE);
> @@ -3807,10 +3808,23 @@ static int amdgpu_device_ip_suspend_phase1(struct amdgpu_device *adev)
>
>                 r = amdgpu_ip_block_suspend(&adev->ip_blocks[i]);
>                 if (r)
> -                       return r;
> +                       goto unwind;
>         }
>
>         return 0;
> +unwind:
> +       rec = amdgpu_device_ip_resume_phase3(adev);
> +       if (rec)
> +               dev_err(adev->dev,
> +                       "amdgpu_device_ip_resume_phase3 failed during unwind: %d\n",
> +                       rec);
> +
> +       amdgpu_dpm_set_df_cstate(adev, DF_CSTATE_ALLOW);
> +
> +       amdgpu_device_set_pg_state(adev, AMD_PG_STATE_GATE);
> +       amdgpu_device_set_cg_state(adev, AMD_CG_STATE_GATE);
> +
> +       return r;
>  }
>
>  /**
> --
> 2.51.1
>

Re: [PATCH v5 2/5] drm/amd: Add an unwind for failures in amdgpu_device_ip_suspend_phase1()

[Mario Limonciello (AMD) (kernel.org)]

On 10/30/2025 10:14 AM, Alex Deucher wrote:
> Patches 2-4 are:
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

Thanks!

How about patch 1?  Patch 4 builds on it, so if that doesn't go in there 
is another unwind step needed.

> 
> On Sun, Oct 26, 2025 at 12:36 AM Mario Limonciello (AMD)
> <superm1@kernel.org> wrote:
>>
>> If any hardware IPs involved with the first phase of suspend fail, unwind
>> all steps to restore back to original state.
>>
>> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 18 ++++++++++++++++--
>>   1 file changed, 16 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> index f6850b86e96f..b9ea91b2c92f 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -178,6 +178,7 @@ struct amdgpu_init_level amdgpu_init_minimal_xgmi = {
>>                  BIT(AMD_IP_BLOCK_TYPE_COMMON) | BIT(AMD_IP_BLOCK_TYPE_IH) |
>>                  BIT(AMD_IP_BLOCK_TYPE_PSP)
>>   };
>> +static int amdgpu_device_ip_resume_phase3(struct amdgpu_device *adev);
>>
>>   static void amdgpu_device_load_switch_state(struct amdgpu_device *adev);
>>
>> @@ -3784,7 +3785,7 @@ static void amdgpu_device_delay_enable_gfx_off(struct work_struct *work)
>>    */
>>   static int amdgpu_device_ip_suspend_phase1(struct amdgpu_device *adev)
>>   {
>> -       int i, r;
>> +       int i, r, rec;
>>
>>          amdgpu_device_set_pg_state(adev, AMD_PG_STATE_UNGATE);
>>          amdgpu_device_set_cg_state(adev, AMD_CG_STATE_UNGATE);
>> @@ -3807,10 +3808,23 @@ static int amdgpu_device_ip_suspend_phase1(struct amdgpu_device *adev)
>>
>>                  r = amdgpu_ip_block_suspend(&adev->ip_blocks[i]);
>>                  if (r)
>> -                       return r;
>> +                       goto unwind;
>>          }
>>
>>          return 0;
>> +unwind:
>> +       rec = amdgpu_device_ip_resume_phase3(adev);
>> +       if (rec)
>> +               dev_err(adev->dev,
>> +                       "amdgpu_device_ip_resume_phase3 failed during unwind: %d\n",
>> +                       rec);
>> +
>> +       amdgpu_dpm_set_df_cstate(adev, DF_CSTATE_ALLOW);
>> +
>> +       amdgpu_device_set_pg_state(adev, AMD_PG_STATE_GATE);
>> +       amdgpu_device_set_cg_state(adev, AMD_CG_STATE_GATE);
>> +
>> +       return r;
>>   }
>>
>>   /**
>> --
>> 2.51.1
>>

Re: [PATCH v5 2/5] drm/amd: Add an unwind for failures in amdgpu_device_ip_suspend_phase1()

[Alex Deucher]

On Thu, Oct 30, 2025 at 11:16 AM Mario Limonciello (AMD) (kernel.org)
<superm1@kernel.org> wrote:
>
>
>
> On 10/30/2025 10:14 AM, Alex Deucher wrote:
> > Patches 2-4 are:
> > Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
>
> Thanks!
>
> How about patch 1?  Patch 4 builds on it, so if that doesn't go in there
> is another unwind step needed.

Oh, yeah, feel free to add my RB on that one as well, I guess it's not
quite the same as the one I sent out originally.

Alex

>
> >
> > On Sun, Oct 26, 2025 at 12:36 AM Mario Limonciello (AMD)
> > <superm1@kernel.org> wrote:
> >>
> >> If any hardware IPs involved with the first phase of suspend fail, unwind
> >> all steps to restore back to original state.
> >>
> >> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
> >> ---
> >>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 18 ++++++++++++++++--
> >>   1 file changed, 16 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >> index f6850b86e96f..b9ea91b2c92f 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >> @@ -178,6 +178,7 @@ struct amdgpu_init_level amdgpu_init_minimal_xgmi = {
> >>                  BIT(AMD_IP_BLOCK_TYPE_COMMON) | BIT(AMD_IP_BLOCK_TYPE_IH) |
> >>                  BIT(AMD_IP_BLOCK_TYPE_PSP)
> >>   };
> >> +static int amdgpu_device_ip_resume_phase3(struct amdgpu_device *adev);
> >>
> >>   static void amdgpu_device_load_switch_state(struct amdgpu_device *adev);
> >>
> >> @@ -3784,7 +3785,7 @@ static void amdgpu_device_delay_enable_gfx_off(struct work_struct *work)
> >>    */
> >>   static int amdgpu_device_ip_suspend_phase1(struct amdgpu_device *adev)
> >>   {
> >> -       int i, r;
> >> +       int i, r, rec;
> >>
> >>          amdgpu_device_set_pg_state(adev, AMD_PG_STATE_UNGATE);
> >>          amdgpu_device_set_cg_state(adev, AMD_CG_STATE_UNGATE);
> >> @@ -3807,10 +3808,23 @@ static int amdgpu_device_ip_suspend_phase1(struct amdgpu_device *adev)
> >>
> >>                  r = amdgpu_ip_block_suspend(&adev->ip_blocks[i]);
> >>                  if (r)
> >> -                       return r;
> >> +                       goto unwind;
> >>          }
> >>
> >>          return 0;
> >> +unwind:
> >> +       rec = amdgpu_device_ip_resume_phase3(adev);
> >> +       if (rec)
> >> +               dev_err(adev->dev,
> >> +                       "amdgpu_device_ip_resume_phase3 failed during unwind: %d\n",
> >> +                       rec);
> >> +
> >> +       amdgpu_dpm_set_df_cstate(adev, DF_CSTATE_ALLOW);
> >> +
> >> +       amdgpu_device_set_pg_state(adev, AMD_PG_STATE_GATE);
> >> +       amdgpu_device_set_cg_state(adev, AMD_CG_STATE_GATE);
> >> +
> >> +       return r;
> >>   }
> >>
> >>   /**
> >> --
> >> 2.51.1
> >>
>

Re: [PATCH v5 2/5] drm/amd: Add an unwind for failures in amdgpu_device_ip_suspend_phase1()

[Mario Limonciello (AMD) (kernel.org)]

On 10/30/2025 10:18 AM, Alex Deucher wrote:
> On Thu, Oct 30, 2025 at 11:16 AM Mario Limonciello (AMD) (kernel.org)
> <superm1@kernel.org> wrote:
>>
>>
>>
>> On 10/30/2025 10:14 AM, Alex Deucher wrote:
>>> Patches 2-4 are:
>>> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
>>
>> Thanks!
>>
>> How about patch 1?  Patch 4 builds on it, so if that doesn't go in there
>> is another unwind step needed.
> 
> Oh, yeah, feel free to add my RB on that one as well, I guess it's not
> quite the same as the one I sent out originally.

OK Thanks.  Will queue up 1-4 and will drop #5 based on your comments 
from v4.

> 
> Alex
> 
>>
>>>
>>> On Sun, Oct 26, 2025 at 12:36 AM Mario Limonciello (AMD)
>>> <superm1@kernel.org> wrote:
>>>>
>>>> If any hardware IPs involved with the first phase of suspend fail, unwind
>>>> all steps to restore back to original state.
>>>>
>>>> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
>>>> ---
>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 18 ++++++++++++++++--
>>>>    1 file changed, 16 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>> index f6850b86e96f..b9ea91b2c92f 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>> @@ -178,6 +178,7 @@ struct amdgpu_init_level amdgpu_init_minimal_xgmi = {
>>>>                   BIT(AMD_IP_BLOCK_TYPE_COMMON) | BIT(AMD_IP_BLOCK_TYPE_IH) |
>>>>                   BIT(AMD_IP_BLOCK_TYPE_PSP)
>>>>    };
>>>> +static int amdgpu_device_ip_resume_phase3(struct amdgpu_device *adev);
>>>>
>>>>    static void amdgpu_device_load_switch_state(struct amdgpu_device *adev);
>>>>
>>>> @@ -3784,7 +3785,7 @@ static void amdgpu_device_delay_enable_gfx_off(struct work_struct *work)
>>>>     */
>>>>    static int amdgpu_device_ip_suspend_phase1(struct amdgpu_device *adev)
>>>>    {
>>>> -       int i, r;
>>>> +       int i, r, rec;
>>>>
>>>>           amdgpu_device_set_pg_state(adev, AMD_PG_STATE_UNGATE);
>>>>           amdgpu_device_set_cg_state(adev, AMD_CG_STATE_UNGATE);
>>>> @@ -3807,10 +3808,23 @@ static int amdgpu_device_ip_suspend_phase1(struct amdgpu_device *adev)
>>>>
>>>>                   r = amdgpu_ip_block_suspend(&adev->ip_blocks[i]);
>>>>                   if (r)
>>>> -                       return r;
>>>> +                       goto unwind;
>>>>           }
>>>>
>>>>           return 0;
>>>> +unwind:
>>>> +       rec = amdgpu_device_ip_resume_phase3(adev);
>>>> +       if (rec)
>>>> +               dev_err(adev->dev,
>>>> +                       "amdgpu_device_ip_resume_phase3 failed during unwind: %d\n",
>>>> +                       rec);
>>>> +
>>>> +       amdgpu_dpm_set_df_cstate(adev, DF_CSTATE_ALLOW);
>>>> +
>>>> +       amdgpu_device_set_pg_state(adev, AMD_PG_STATE_GATE);
>>>> +       amdgpu_device_set_cg_state(adev, AMD_CG_STATE_GATE);
>>>> +
>>>> +       return r;
>>>>    }
>>>>
>>>>    /**
>>>> --
>>>> 2.51.1
>>>>
>>

[PATCH v5 3/5] drm/amd: Add an unwind for failures in amdgpu_device_ip_suspend_phase2()

[Mario Limonciello (AMD)]

If any hardware IPs involved with the second phase of suspend fail, unwind
all steps to restore back to original state.

Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 36 ++++++++++++++++++++--
 1 file changed, 33 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index b9ea91b2c92f..5945f441d01e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -178,6 +178,9 @@ struct amdgpu_init_level amdgpu_init_minimal_xgmi = {
 		BIT(AMD_IP_BLOCK_TYPE_COMMON) | BIT(AMD_IP_BLOCK_TYPE_IH) |
 		BIT(AMD_IP_BLOCK_TYPE_PSP)
 };
+
+static int amdgpu_device_ip_resume_phase1(struct amdgpu_device *adev);
+static int amdgpu_device_ip_resume_phase2(struct amdgpu_device *adev);
 static int amdgpu_device_ip_resume_phase3(struct amdgpu_device *adev);
 
 static void amdgpu_device_load_switch_state(struct amdgpu_device *adev);
@@ -3840,7 +3843,7 @@ static int amdgpu_device_ip_suspend_phase1(struct amdgpu_device *adev)
  */
 static int amdgpu_device_ip_suspend_phase2(struct amdgpu_device *adev)
 {
-	int i, r;
+	int i, r, rec;
 
 	if (adev->in_s0ix)
 		amdgpu_dpm_gfx_state_change(adev, sGpuChangeState_D3Entry);
@@ -3903,7 +3906,7 @@ static int amdgpu_device_ip_suspend_phase2(struct amdgpu_device *adev)
 
 		r = amdgpu_ip_block_suspend(&adev->ip_blocks[i]);
 		if (r)
-			return r;
+			goto unwind;
 
 		/* handle putting the SMC in the appropriate state */
 		if (!amdgpu_sriov_vf(adev)) {
@@ -3913,13 +3916,40 @@ static int amdgpu_device_ip_suspend_phase2(struct amdgpu_device *adev)
 					dev_err(adev->dev,
 						"SMC failed to set mp1 state %d, %d\n",
 						adev->mp1_state, r);
-					return r;
+					goto unwind;
 				}
 			}
 		}
 	}
 
 	return 0;
+unwind:
+	/* suspend phase 2 = resume phase 1 + resume phase 2 */
+	rec = amdgpu_device_ip_resume_phase1(adev);
+	if (rec) {
+		dev_err(adev->dev,
+			"amdgpu_device_ip_resume_phase1 failed during unwind: %d\n",
+			rec);
+		return r;
+	}
+
+	rec = amdgpu_device_fw_loading(adev);
+	if (rec) {
+		dev_err(adev->dev,
+			"amdgpu_device_fw_loading failed during unwind: %d\n",
+			rec);
+		return r;
+	}
+
+	rec = amdgpu_device_ip_resume_phase2(adev);
+	if (rec) {
+		dev_err(adev->dev,
+			"amdgpu_device_ip_resume_phase2 failed during unwind: %d\n",
+			rec);
+		return r;
+	}
+
+	return r;
 }
 
 /**
-- 
2.51.1

[PATCH v5 4/5] drm/amd: Unwind for failed device suspend

[Mario Limonciello (AMD)]

If device suspend has failed, add a recovery flow that will attempt
to unwind the suspend and get things back up and running.

Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/4627
Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
---
v5:
 * Add missing drm client notification (should fix console resume)
 * Add missing RAS notification
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 65 +++++++++++++++++++---
 1 file changed, 58 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 5945f441d01e..d8a2dcf6100a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -5274,7 +5274,7 @@ void amdgpu_device_complete(struct drm_device *dev)
 int amdgpu_device_suspend(struct drm_device *dev, bool notify_clients)
 {
 	struct amdgpu_device *adev = drm_to_adev(dev);
-	int r = 0;
+	int r, rec;
 
 	if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
 		return 0;
@@ -5290,8 +5290,9 @@ int amdgpu_device_suspend(struct drm_device *dev, bool notify_clients)
 			return r;
 	}
 
-	if (amdgpu_acpi_smart_shift_update(adev, AMDGPU_SS_DEV_D3))
-		dev_warn(adev->dev, "smart shift update failed\n");
+	r = amdgpu_acpi_smart_shift_update(adev, AMDGPU_SS_DEV_D3);
+	if (r)
+		goto unwind_sriov;
 
 	if (notify_clients)
 		drm_client_dev_suspend(adev_to_drm(adev), false);
@@ -5302,16 +5303,16 @@ int amdgpu_device_suspend(struct drm_device *dev, bool notify_clients)
 
 	r = amdgpu_device_ip_suspend_phase1(adev);
 	if (r)
-		return r;
+		goto unwind_smartshift;
 
 	amdgpu_amdkfd_suspend(adev, !amdgpu_sriov_vf(adev) && !adev->in_runpm);
 	r = amdgpu_userq_suspend(adev);
 	if (r)
-		return r;
+		goto unwind_ip_phase1;
 
 	r = amdgpu_device_evict_resources(adev);
 	if (r)
-		return r;
+		goto unwind_userq;
 
 	amdgpu_ttm_set_buffer_funcs_status(adev, false);
 
@@ -5319,12 +5320,62 @@ int amdgpu_device_suspend(struct drm_device *dev, bool notify_clients)
 
 	r = amdgpu_device_ip_suspend_phase2(adev);
 	if (r)
-		return r;
+		goto unwind_evict;
 
 	if (amdgpu_sriov_vf(adev))
 		amdgpu_virt_release_full_gpu(adev, false);
 
 	return 0;
+
+unwind_evict:
+	if (adev->mman.buffer_funcs_ring->sched.ready)
+		amdgpu_ttm_set_buffer_funcs_status(adev, true);
+	amdgpu_fence_driver_hw_init(adev);
+
+unwind_userq:
+	rec = amdgpu_userq_resume(adev);
+	if (rec) {
+		dev_warn(adev->dev, "failed to re-initialize user queues: %d\n", rec);
+		return r;
+	}
+	rec = amdgpu_amdkfd_resume(adev, !amdgpu_sriov_vf(adev) && !adev->in_runpm);
+	if (rec) {
+		dev_warn(adev->dev, "failed to re-initialize kfd: %d\n", rec);
+		return r;
+	}
+
+unwind_ip_phase1:
+	/* suspend phase 1 = resume phase 3 */
+	rec = amdgpu_device_ip_resume_phase3(adev);
+	if (rec) {
+		dev_warn(adev->dev, "failed to re-initialize IPs phase1: %d\n", rec);
+		return r;
+	}
+
+unwind_smartshift:
+	rec = amdgpu_acpi_smart_shift_update(adev, AMDGPU_SS_DEV_D0);
+	if (rec) {
+		dev_warn(adev->dev, "failed to re-update smart shift: %d\n", rec);
+		return r;
+	}
+
+	if (notify_clients)
+		drm_client_dev_resume(adev_to_drm(adev), false);
+
+	amdgpu_ras_resume(adev);
+
+unwind_sriov:
+	if (amdgpu_sriov_vf(adev)) {
+		rec = amdgpu_virt_request_full_gpu(adev, true);
+		if (rec) {
+			dev_warn(adev->dev, "failed to reinitialize sriov: %d\n", rec);
+			return r;
+		}
+	}
+
+	adev->in_suspend = adev->in_s0ix = adev->in_s3 = false;
+
+	return r;
 }
 
 static inline int amdgpu_virt_resume(struct amdgpu_device *adev)
-- 
2.51.1

[PATCH v5 5/5] drm/amd: Reset the GPU if pmops failed

[Mario Limonciello (AMD)]

If the GPU fails to suspend the return code is passed up to the caller
but it's left in an inconsistent state.  This could lead to hangs
if userspace tries to access it.

The last stage of all pmpops calls (success or fail) is the complete()
callback.  If by the time the PM core reaches this state the GPU is still
in suspend something went really wrong, so reset it.

Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
---
v5:
 * Handle case of DPM_FLAG_SMART_SUSPEND (Lijo)
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index cee90f9e58a9..4d437e31d1bd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -2590,6 +2590,17 @@ static int amdgpu_pmops_prepare(struct device *dev)
 
 static void amdgpu_pmops_complete(struct device *dev)
 {
+	struct drm_device *drm_dev = dev_get_drvdata(dev);
+	struct amdgpu_device *adev = drm_to_adev(drm_dev);
+
+	/* sequence failed, use a big 🔨 try to cleanup */
+	if (adev->in_suspend && !pm_runtime_suspended(dev)) {
+		adev->in_suspend = adev->in_s0ix = adev->in_s3 = false;
+		dev_crit(adev->dev, "pmpops sequence failed, resetting\n");
+		amdgpu_asic_reset(adev);
+		return;
+	}
+
 	amdgpu_device_complete(dev_get_drvdata(dev));
 }
 
-- 
2.51.1