[PATCH] drm/amdgpu: Maintain suspend-resume call consistency in gpu recovery

[Harry Wentland]

On 2018-02-01 03:02 PM, Alex Deucher wrote:
> On Thu, Feb 1, 2018 at 2:44 PM,  <mikita.lipski at amd.com> wrote:
>> From: Mikita Lipski <mikita.lipski at amd.com>
>>
>> Call dm_suspend function instead of drm_kms suspend function in
>> order to cache current state
>>
>> Call DM_resume first to restore dc hardware, then amdgpu_dm_display_resume
>> to restore dal's cached state.
> 
> We already call the IP block suspend and resume functions in
> amdgpu_device_ip_suspend() which is called by amdgpu_device_reset(),
> at least when we do a hard reset.  They will end up getting called

I missed that. I think we just need to drop the existing DC and drm_atomic_helper_* calls in amdgpu_device_gpu_recover and call amdgpu_dm_display_resume after amdgpu_device_ip_resume_phase2 in amdgpu_device_reset.

I really don't like the way we call amdgpu_dm_display_resume separately from the .resume hook now. We initially implemented it because in amdgpu_device_resume we need cursor pinning to happen before really resuming DC. Not sure if we can move the cursor pinning to before amdgpu_device_ip_resume in that function. If we can we can just move amdgpu_dm_display_resume into the proper .resume hook, and clean up both resume and gpu_recover code.

> twice in that case with this change.  Perhaps a better solution would
> be to only call the drm_atomic_helper_* functions when we have a hard
> reset rather than a soft one.

amdgpu_dm_display_resume will already call the drm_atomic_helper_ functions. We should really call them in gpu_recover, unless we need to re-apply the state on a soft reset, but then we probably want to block this call from a hard reset.

Harry

> 
> Alex
> 
> 
>>
>> Signed-off-by: Mikita Lipski <mikita.lipski at amd.com>
>> ---
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 10 ++++------
>>  1 file changed, 4 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> index 850453e..b55c929 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -2616,7 +2616,6 @@ static int amdgpu_device_reset_sriov(struct amdgpu_device *adev,
>>  int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
>>                               struct amdgpu_job *job, bool force)
>>  {
>> -       struct drm_atomic_state *state = NULL;
>>         uint64_t reset_flags = 0;
>>         int i, r, resched;
>>
>> @@ -2640,9 +2639,9 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
>>         /* block TTM */
>>         resched = ttm_bo_lock_delayed_workqueue(&adev->mman.bdev);
>>         /* store modesetting */
>> -       if (amdgpu_device_has_dc_support(adev))
>> -               state = drm_atomic_helper_suspend(adev->ddev);
>> -
>> +       if (amdgpu_device_has_dc_support(adev)){
>> +               adev->ip_blocks[5].version->funcs->suspend(adev);
>> +       }
>>         /* block scheduler */
>>         for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
>>                 struct amdgpu_ring *ring = adev->rings[i];
>> @@ -2727,8 +2726,7 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
>>         }
>>
>>         if (amdgpu_device_has_dc_support(adev)) {
>> -               if (drm_atomic_helper_resume(adev->ddev, state))
>> -                       dev_info(adev->dev, "drm resume failed:%d\n", r);
>> +               adev->ip_blocks[5].version->funcs->resume(adev);
>>                 amdgpu_dm_display_resume(adev);
>>         } else {
>>                 drm_helper_resume_force_mode(adev->ddev);
>> --
>> 2.7.4
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>