[PATCH] drm/amd/amdgpu: S3 resume fail

[PATCH] drm/amd/amdgpu: S3 resume fail

[jimqu]

Doing S3 stress test, sometimes, driver can not get console lock. So use
console_trylock() to instead it.

Change-Id: I50177f3d7dbb923cb4f602a11ea51c397b12b3e6
Signed-off-by: JimQu <Jim.Qu@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 087e520..bd5387d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1889,9 +1889,6 @@ int amdgpu_resume_kms(struct drm_device *dev, bool resume, bool fbcon)
 	if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
 		return 0;
 
-	if (fbcon) {
-		console_lock();
-	}
 	if (resume) {
 		pci_set_power_state(dev->pdev, PCI_D0);
 		pci_restore_state(dev->pdev);
@@ -1989,6 +1986,8 @@ int amdgpu_resume_kms(struct drm_device *dev, bool resume, bool fbcon)
 #endif
 
 	if (fbcon) {
+		if (!console_trylock())
+			return 0;
 		amdgpu_fbdev_set_suspend(adev, 0);
 		console_unlock();
 	}
-- 
1.9.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amd/amdgpu: S3 resume fail

[Michel Dänzer]

On 10/08/16 12:04 PM, jimqu wrote:
> Doing S3 stress test, sometimes, driver can not get console lock. So use
> console_trylock() to instead it.

This raises some questions:


> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 087e520..bd5387d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -1889,9 +1889,6 @@ int amdgpu_resume_kms(struct drm_device *dev, bool resume, bool fbcon)
>  	if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
>  		return 0;
>  
> -	if (fbcon) {
> -		console_lock();
> -	}

What else is holding console_lock at this time?


> @@ -1989,6 +1986,8 @@ int amdgpu_resume_kms(struct drm_device *dev, bool resume, bool fbcon)
>  #endif
>  
>  	if (fbcon) {
> +		if (!console_trylock())
> +			return 0;
>  		amdgpu_fbdev_set_suspend(adev, 0);
>  		console_unlock();
>  	}

Do virtual terminals controlled by fbcon actually work if we don't call
amdgpu_fbdev_set_suspend(adev, 0) here?


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amd/amdgpu: S3 resume fail

[Michel Dänzer]

On 10/08/16 12:24 PM, Michel Dänzer wrote:
> On 10/08/16 12:04 PM, jimqu wrote:
>> Doing S3 stress test, sometimes, driver can not get console lock. So use
>> console_trylock() to instead it.
> 
> This raises some questions:
> 
> 
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> index 087e520..bd5387d 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -1889,9 +1889,6 @@ int amdgpu_resume_kms(struct drm_device *dev, bool resume, bool fbcon)
>>  	if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
>>  		return 0;
>>  
>> -	if (fbcon) {
>> -		console_lock();
>> -	}
> 
> What else is holding console_lock at this time?

Maybe something calls console_lock but not console_unlock? E.g. I see
some error cases in amdgpu_resume_kms were this could happen.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

答复: [PATCH] drm/amd/amdgpu: S3 resume fail

[Qu, Jim]

HI Michel:

1. So far, I not sure which one causes the console_lock() fail, maybe I need enable kernel mutex debug feature to trace it.

2. Phenomenally, it can be restore to desktop normally. Do you know any methods to check it whether driver without calling amdgpu_fbdev_set_suspend(adev, 0)  has side effect or not?

Thanks
JimQu

________________________________________
发件人: Michel Dänzer <michel@daenzer.net>
发送时间: 2016年8月10日 11:28:39
收件人: Qu, Jim
抄送: amd-gfx@lists.freedesktop.org
主题: Re: [PATCH] drm/amd/amdgpu: S3 resume fail

On 10/08/16 12:24 PM, Michel Dänzer wrote:
> On 10/08/16 12:04 PM, jimqu wrote:
>> Doing S3 stress test, sometimes, driver can not get console lock. So use
>> console_trylock() to instead it.
>
> This raises some questions:
>
>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> index 087e520..bd5387d 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -1889,9 +1889,6 @@ int amdgpu_resume_kms(struct drm_device *dev, bool resume, bool fbcon)
>>      if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
>>              return 0;
>>
>> -    if (fbcon) {
>> -            console_lock();
>> -    }
>
> What else is holding console_lock at this time?

Maybe something calls console_lock but not console_unlock? E.g. I see
some error cases in amdgpu_resume_kms were this could happen.


--
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: 答复: [PATCH] drm/amd/amdgpu: S3 resume fail

[Michel Dänzer]

On 10/08/16 01:39 PM, Qu, Jim wrote:
> HI Michel:
> 
> 1. So far, I not sure which one causes the console_lock() fail, maybe I
> need enable kernel mutex debug feature to trace it.

Or, maybe you can just try fixing the cases in amdgpu_resume_kms where
console_unlock() isn't called, and see if that fixes the problem?


> 2. Phenomenally, it can be restore to desktop normally. Do you know
> any methods to check it whether driver without calling
> amdgpu_fbdev_set_suspend(adev, 0)  has side effect or not?

Does VT switching to console with Ctrl-Alt-Fx work? Does logging into
the console and interacting with the shell work?


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

答复: 答复: [PATCH] drm/amd/amdgpu: S3 resume fail

[Qu, Jim]

Hi Michel:

1. add console_unlock() in fail case
------issue also can be observed.
2. with console_trylock()
------switch VT is no problem.
------log into console and interact with shell are no problem.


Thanks
JimQu

________________________________________
发件人: Michel Dänzer <michel@daenzer.net>
发送时间: 2016年8月10日 14:54:48
收件人: Qu, Jim
抄送: amd-gfx@lists.freedesktop.org
主题: Re: 答复: [PATCH] drm/amd/amdgpu: S3 resume fail

On 10/08/16 01:39 PM, Qu, Jim wrote:
> HI Michel:
>
> 1. So far, I not sure which one causes the console_lock() fail, maybe I
> need enable kernel mutex debug feature to trace it.

Or, maybe you can just try fixing the cases in amdgpu_resume_kms where
console_unlock() isn't called, and see if that fixes the problem?


> 2. Phenomenally, it can be restore to desktop normally. Do you know
> any methods to check it whether driver without calling
> amdgpu_fbdev_set_suspend(adev, 0)  has side effect or not?

Does VT switching to console with Ctrl-Alt-Fx work? Does logging into
the console and interacting with the shell work?


--
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: 答复: 答复: [PATCH] drm/amd/amdgpu: S3 resume fail

[Christian König]

Michel is correct your patch will probably break resume in some cases.

Please enable lockdep debugging during your kernel compile and take a 
look what is deadlocking here and why instead of trying to band aid over 
the real problem.

Regards,
Christian.

Am 13.08.2016 um 08:56 schrieb Qu, Jim:
> Hi Michel:
>
> 1. add console_unlock() in fail case
> ------issue also can be observed.
> 2. with console_trylock()
> ------switch VT is no problem.
> ------log into console and interact with shell are no problem.
>
>
> Thanks
> JimQu
>
> ________________________________________
> 发件人: Michel Dänzer <michel@daenzer.net>
> 发送时间: 2016年8月10日 14:54:48
> 收件人: Qu, Jim
> 抄送: amd-gfx@lists.freedesktop.org
> 主题: Re: 答复: [PATCH] drm/amd/amdgpu: S3 resume fail
>
> On 10/08/16 01:39 PM, Qu, Jim wrote:
>> HI Michel:
>>
>> 1. So far, I not sure which one causes the console_lock() fail, maybe I
>> need enable kernel mutex debug feature to trace it.
> Or, maybe you can just try fixing the cases in amdgpu_resume_kms where
> console_unlock() isn't called, and see if that fixes the problem?
>
>
>> 2. Phenomenally, it can be restore to desktop normally. Do you know
>> any methods to check it whether driver without calling
>> amdgpu_fbdev_set_suspend(adev, 0)  has side effect or not?
> Does VT switching to console with Ctrl-Alt-Fx work? Does logging into
> the console and interacting with the shell work?
>
>
> --
> Earthling Michel Dänzer               |               http://www.amd.com
> Libre software enthusiast             |             Mesa and X developer
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: 答复: 答复: [PATCH] drm/amd/amdgpu: S3 resume fail

[Michel Dänzer]

On 13/08/16 03:56 PM, Qu, Jim wrote:
> Hi Michel:
> 
> 2. with console_trylock()
> ------switch VT is no problem.
> ------log into console and interact with shell are no problem.

Just to be clear, that's the case even if console_trylock() fails, and
amdgpu_fbdev_set_suspend(adev, 0) isn't called?


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

答复: 答复: 答复: [PATCH] drm/amd/amdgpu: S3 resume fail

[Qu, Jim]

Hi Christian:

OK, I will do it after solve VCE reset problem.

Thanks
JimQu

________________________________________
发件人: Christian König <deathsimple@vodafone.de>
发送时间: 2016年8月13日 16:51:49
收件人: Qu, Jim; Michel Dänzer
抄送: amd-gfx@lists.freedesktop.org
主题: Re: 答复: 答复: [PATCH] drm/amd/amdgpu: S3 resume fail

Michel is correct your patch will probably break resume in some cases.

Please enable lockdep debugging during your kernel compile and take a
look what is deadlocking here and why instead of trying to band aid over
the real problem.

Regards,
Christian.

Am 13.08.2016 um 08:56 schrieb Qu, Jim:
> Hi Michel:
>
> 1. add console_unlock() in fail case
> ------issue also can be observed.
> 2. with console_trylock()
> ------switch VT is no problem.
> ------log into console and interact with shell are no problem.
>
>
> Thanks
> JimQu
>
> ________________________________________
> 发件人: Michel Dänzer <michel@daenzer.net>
> 发送时间: 2016年8月10日 14:54:48
> 收件人: Qu, Jim
> 抄送: amd-gfx@lists.freedesktop.org
> 主题: Re: 答复: [PATCH] drm/amd/amdgpu: S3 resume fail
>
> On 10/08/16 01:39 PM, Qu, Jim wrote:
>> HI Michel:
>>
>> 1. So far, I not sure which one causes the console_lock() fail, maybe I
>> need enable kernel mutex debug feature to trace it.
> Or, maybe you can just try fixing the cases in amdgpu_resume_kms where
> console_unlock() isn't called, and see if that fixes the problem?
>
>
>> 2. Phenomenally, it can be restore to desktop normally. Do you know
>> any methods to check it whether driver without calling
>> amdgpu_fbdev_set_suspend(adev, 0)  has side effect or not?
> Does VT switching to console with Ctrl-Alt-Fx work? Does logging into
> the console and interacting with the shell work?
>
>
> --
> Earthling Michel Dänzer               |               http://www.amd.com
> Libre software enthusiast             |             Mesa and X developer
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

答复: 答复: 答复: [PATCH] drm/amd/amdgpu: S3 resume fail

[Qu, Jim]

Hi Michel:

I modify the code like as:

        if (fbcon) {
                return 0;
                if (!console_trylock())
                        return 0;
                amdgpu_fbdev_set_suspend(adev, 0);
                console_unlock();
        }



Thanks
JimQu

________________________________________
发件人: Michel Dänzer <michel@daenzer.net>
发送时间: 2016年8月15日 8:44:29
收件人: Qu, Jim
抄送: amd-gfx@lists.freedesktop.org
主题: Re: 答复: 答复: [PATCH] drm/amd/amdgpu: S3 resume fail

On 13/08/16 03:56 PM, Qu, Jim wrote:
> Hi Michel:
>
> 2. with console_trylock()
> ------switch VT is no problem.
> ------log into console and interact with shell are no problem.

Just to be clear, that's the case even if console_trylock() fails, and
amdgpu_fbdev_set_suspend(adev, 0) isn't called?


--
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx