Re: [PATCH v4 1/2] drm/amd: Unwind for failed device suspend

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

On 10/29/2025 4:19 PM, Alex Deucher wrote:
> On Thu, Oct 23, 2025 at 12:53 PM Mario Limonciello
> <mario.limonciello@amd.com> wrote:
>>
>> 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 <mario.limonciello@amd.com>
> 
> Patch is:
> Acked-by: Alex Deucher <alexander.deucher@amd.com>

I made some minor changes to this patch from testing feedback in a v5. 
It's now patch 4/5 with a few other things I found in 2-3 and part of 
another of your patch as 1.

Could you check that one?

https://lore.kernel.org/amd-gfx/20251026042942.549389-1-superm1@kernel.org/
> 
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 80 +++++++++++++++++++---
>>   1 file changed, 72 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> index 3ffb9bb1ec0b..645b15aa34f1 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -5231,7 +5231,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;
>> @@ -5247,8 +5247,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);
>> @@ -5259,16 +5260,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);
>>
>> @@ -5276,16 +5277,79 @@ 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);
>>
>>          r = amdgpu_dpm_notify_rlc_state(adev, false);
>>          if (r)
>> -               return r;
>> +               goto unwind_ip_phase2;
>>
>>          return 0;
>> +
>> +unwind_ip_phase2:
>> +       /* suspend phase 2 = resume phase 2 + resume phase 1 */
>> +       rec = amdgpu_device_ip_resume_phase2(adev);
>> +       if (rec) {
>> +               dev_warn(adev->dev, "failed to re-initialize IPs phase2: %d\n", rec);
>> +               return r;
>> +       }
>> +       rec = amdgpu_device_fw_loading(adev);
>> +       if (rec) {
>> +               dev_warn(adev->dev, "failed to reload firmwares: %d\n", rec);
>> +               return r;
>> +       }
>> +       rec = amdgpu_device_ip_resume_phase1(adev);
>> +       if (rec) {
>> +               dev_warn(adev->dev, "failed to re-initialize IPs phase1: %d\n", rec);
>> +               return r;
>> +       }
>> +
>> +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;
>> +       }
>> +
>> +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
>>