[PATCH] drm/amdgpu: disable runtime pm on several sienna cichlid cards(v2)

[Chen, Guchun]

I have submitted this patch to amd-staging-drm-next branch. Shall I need to improve it?

Regards,
Guchun

-----Original Message-----
From: Lazar, Lijo <Lijo.Lazar at amd.com> 
Sent: Thursday, April 28, 2022 12:18 AM
To: Alex Deucher <alexdeucher at gmail.com>; Quan, Evan <Evan.Quan at amd.com>
Cc: Chen, Guchun <Guchun.Chen at amd.com>; amd-gfx at lists.freedesktop.org; Koenig, Christian <Christian.Koenig at amd.com>; Zhang, Hawking <Hawking.Zhang at amd.com>; Deucher, Alexander <Alexander.Deucher at amd.com>
Subject: Re: [PATCH] drm/amdgpu: disable runtime pm on several sienna cichlid cards(v2)



On 4/27/2022 9:40 PM, Alex Deucher wrote:
> On Wed, Apr 27, 2022 at 7:42 AM Quan, Evan <Evan.Quan at amd.com> wrote:
>>
>> [AMD Official Use Only - General]
>>
>>
>>
>>> -----Original Message-----
>>> From: Chen, Guchun <Guchun.Chen at amd.com>
>>> Sent: Wednesday, April 27, 2022 6:27 PM
>>> To: amd-gfx at lists.freedesktop.org; Koenig, Christian 
>>> <Christian.Koenig at amd.com>; Zhang, Hawking <Hawking.Zhang at amd.com>; 
>>> Deucher, Alexander <Alexander.Deucher at amd.com>; Lazar, Lijo 
>>> <Lijo.Lazar at amd.com>; Quan, Evan <Evan.Quan at amd.com>
>>> Cc: Chen, Guchun <Guchun.Chen at amd.com>
>>> Subject: [PATCH] drm/amdgpu: disable runtime pm on several sienna 
>>> cichlid
>>> cards(v2)
>>>
>>> Disable runtime power management on several sienna cichlid cards, 
>>> otherwise SMU will possibly fail to be resumed from runtime suspend. 
>>> Will drop this after a clean solution between kernel driver and SMU 
>>> FW is available.
>>>
>>> amdgpu 0000:63:00.0: amdgpu: GECC is enabled amdgpu 0000:63:00.0: 
>>> amdgpu: SECUREDISPLAY: securedisplay ta ucode is not available 
>>> amdgpu 0000:63:00.0: amdgpu: SMU is resuming...
>>> amdgpu 0000:63:00.0: amdgpu: SMU: I'm not done with your command:
>>> SMN_C2PMSG_66:0x0000000E SMN_C2PMSG_82:0x00000080 amdgpu 
>>> 0000:63:00.0: amdgpu: Failed to SetDriverDramAddr!
>>> amdgpu 0000:63:00.0: amdgpu: Failed to setup smc hw!
>>> [drm:amdgpu_device_ip_resume_phase2 [amdgpu]] *ERROR* resume of IP 
>>> block <smu> failed -62 amdgpu 0000:63:00.0: amdgpu: 
>>> amdgpu_device_ip_resume failed (-62)
>>>
>>> v2: seperate to a function.
>>>
>>> Signed-off-by: Guchun Chen <guchun.chen at amd.com>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 14 ++++++++++++++
>>>   1 file changed, 14 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>> index 262938f0dfdb..1bf8ff71b6b5 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>> @@ -43,6 +43,17 @@
>>>   #include "amdgpu_display.h"
>>>   #include "amdgpu_ras.h"
>>>
>>> +static void amdgpu_runtime_pm_quirk(struct amdgpu_device *adev) {
>>> +     /*
>>> +      * Add below quirk on several sienna_cichlid cards to disable
>>> +      * runtime pm to fix EMI failures.
>>> +      */
>>> +     if (((adev->pdev->device == 0x73A1) && (adev->pdev->revision 
>>> +==
>>> 0x00)) ||
>>> +         ((adev->pdev->device == 0x73BF) && (adev->pdev->revision 
>>> + ==
>>> 0xCF)))
>>> +             adev->runpm = false;
>>> +}
>>> +
>> [Quan, Evan] Better to move this to 
>> sienna_cichlid_check_bxco_support(). As long as smu_baco->platform_support is set as false there, the runpm will be disabled accordingly I think.
> 
> Good point.  We probably want to disable BACO entirely or enable the 
> workaround because BACO could also be used for a GPU reset in some 
> cases.  Then only disable runtime pm if BACO will be used.  BOCO 
> should still work fine for example.
> 

As far as I remember, the WA proposal still wants to retain BACO as a reset method.

Thanks,
Lijo

> Alex
> 
>>
>> Either way the patch is reviewed-by: Evan Quan <evan.quan at amd.com>
>>
>> BR
>> Evan
>>>   void amdgpu_unregister_gpu_instance(struct amdgpu_device *adev)
>>>   {
>>>        struct amdgpu_gpu_instance *gpu_instance; @@ -180,6 +191,9 @@ 
>>> int amdgpu_driver_load_kms(struct amdgpu_device *adev, unsigned long 
>>> flags)
>>>                 */
>>>                if (adev->is_fw_fb)
>>>                        adev->runpm = false;
>>> +
>>> +             amdgpu_runtime_pm_quirk(adev);
>>> +
>>>                if (adev->runpm)
>>>                        dev_info(adev->dev, "Using BACO for runtime 
>>> pm\n");
>>>        }
>>> --
>>> 2.17.1