[PATCH] drm/amd/pm: skip the RLC stop when S0i3 suspend for SMU v13.0.4/11

[Mario Limonciello]

On 7/27/23 21:21, Huang, Tim wrote:
> [AMD Official Use Only - General]
> 
> 
> 
> *From:* Limonciello, Mario <Mario.Limonciello at amd.com>
> *Sent:* Friday, July 28, 2023 9:14 AM
> *To:* Huang, Tim <Tim.Huang at amd.com>; amd-gfx at lists.freedesktop.org 
> <amd-gfx at lists.freedesktop.org>
> *Cc:* Deucher, Alexander <Alexander.Deucher at amd.com>; Zhang, Yifan 
> <Yifan1.Zhang at amd.com>
> *Subject:* Re: [PATCH] drm/amd/pm: skip the RLC stop when S0i3 suspend 
> for SMU v13.0.4/11
> On 7/27/23 20:05, Tim Huang wrote:
>> From: Tim Huang <Tim.Huang at amd.com>
>>
>> For SMU v13.0.4/11, driver does not need to stop RLC for S0i3,
>> the firmwares will handle that properly.
>> Conceptually I'm aligned to this.
>> But, just to confirm, have you already run some testing with this
>> with current GPU F/W, BIOS and either 6.1.y, 6.4.y or ASDN?
> 
>> I checked with this on my side and saw success but I'm fearful
>> that it introduces some of the fence expiration problems we
>> had in the past and I'm just not seeing them for some reason.
> 
> Yes, verified based on current BIOS and latest drm-next kernel and the 
> working FWs as the latest FWs
> may have some new issues.
> For this patch, it is asked by the FW guys, driver should not touch 
> RLC_CNTL in S0i3, let RLC FW to do that. If driver programs RLC_CNTL to 
> halt RLC, RLC cannot go GFXOFF exit sequence.
> 
> The fence expiration should be still there by the delayed GFXOFF , but 
> this patch should be useful to help debug it as it avoids the system 
> hang when the issue happens.

So in that case you think that when the driver programs RLC_CNTL  but 
GFXOFF was delayed sequence was going out of order and it triggered 
system hang.  But now with this patch it will not hang but fences expire.

It makes sense to me.  This patch shouldn't be any "more" harmful then.

Reviewed-by: Mario Limonciello <mario.limonciello at amd.com>

For the delayed GFXOFF issue maybe we should revisit my previous idea 
for flushing GFXOFF requests.  IIRC the most recent version was:

https://patchwork.freedesktop.org/patch/537888/?series=117965&rev=1

> 
> Best Regards,
> Tim
> 
>>
>> Signed-off-by: Tim Huang <Tim.Huang at amd.com>
>> ---
>>   drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>> index ce41a8309582..222af2fae745 100644
>> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>> @@ -1581,9 +1581,9 @@ static int smu_disable_dpms(struct smu_context *smu)
>>   
>>        /*
>>         * For SMU 13.0.4/11, PMFW will handle the features disablement properly
>> -      * for gpu reset case. Driver involvement is unnecessary.
>> +      * for gpu reset and S0i3 cases. Driver involvement is unnecessary.
>>         */
>> -     if (amdgpu_in_reset(adev)) {
>> +     if (amdgpu_in_reset(adev) || adev->in_s0ix) {
>>                switch (adev->ip_versions[MP1_HWIP][0]) {
>>                case IP_VERSION(13, 0, 4):
>>                case IP_VERSION(13, 0, 11):
>