Re: [PATCH] drm/amdgpu: keep mmhub clock gating being enabled during s2idle suspend

["Lazar, Lijo" <lijo.lazar@amd.com>]

On 4/27/2022 1:02 PM, Liang, Prike wrote:
> [AMD Official Use Only - General]
> 
>> -----Original Message-----
>> From: Lazar, Lijo <Lijo.Lazar@amd.com>
>> Sent: Wednesday, April 27, 2022 2:33 PM
>> To: Liang, Prike <Prike.Liang@amd.com>; amd-gfx@lists.freedesktop.org
>> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Huang, Ray
>> <Ray.Huang@amd.com>
>> Subject: Re: [PATCH] drm/amdgpu: keep mmhub clock gating being enabled
>> during s2idle suspend
>>
>>
>>
>> On 4/27/2022 9:44 AM, Liang, Prike wrote:
>>> [Public]
>>>
>>>> -----Original Message-----
>>>> From: Lazar, Lijo <Lijo.Lazar@amd.com>
>>>> Sent: Tuesday, April 26, 2022 7:19 PM
>>>> To: Liang, Prike <Prike.Liang@amd.com>; amd-gfx@lists.freedesktop.org
>>>> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Huang, Ray
>>>> <Ray.Huang@amd.com>
>>>> Subject: Re: [PATCH] drm/amdgpu: keep mmhub clock gating being
>>>> enabled during s2idle suspend
>>>>
>>>>
>>>>
>>>> On 4/25/2022 12:22 PM, Prike Liang wrote:
>>>>> Without MMHUB clock gating being enabled then MMHUB will not
>>>>> disconnect from DF and will result in DF C-state entry can't be
>>>>> accessed during S2idle suspend, and eventually s0ix entry will be blocked.
>>>>>
>>>>> Signed-off-by: Prike Liang <Prike.Liang@amd.com>
>>>>> ---
>>>>>     drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c | 10 ++++++++++
>>>>>     1 file changed, 10 insertions(+)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
>>>>> b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
>>>>> index a455e59f41f4..20946bc7fc93 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
>>>>> @@ -1151,6 +1151,16 @@ static int
>>>>> gmc_v10_0_set_clockgating_state(void
>>>> *handle,
>>>>>       int r;
>>>>>       struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>>>>>
>>>>> +   /*
>>>>> +    * The issue mmhub can't disconnect from DF with MMHUB clock
>>>> gating being disabled
>>>>> +    * is a new problem observed at DF 3.0.3, however with the same
>>>> suspend sequence not
>>>>> +    * seen any issue on the DF 3.0.2 series platform.
>>>>> +    */
>>>>> +   if (adev->in_s0ix && adev->ip_versions[DF_HWIP][0] >
>>>>> + IP_VERSION(3,
>>>> 0, 2)) {
>>>>> +           dev_dbg(adev->dev, "keep mmhub clock gating being
>>>> enabled for s0ix\n");
>>>>> +           return 0;
>>>>> +   }
>>>>> +
>>>>
>>>> This only ignores clock gating requests as long as s0ix flag is set.
>>>> As far as I see, s0ix flag is set to true even during resume and set
>>>> to false only after resume is completed. Is that intention and is
>>>> this tested to be working fine? I suggest to keep this specifically for
>> suspend calls.
>>>>
>>>> Thanks,
>>>> Lijo
>>>>
>>> It reasonable for also not reenable the clock gating on the s2ilde
>>> resume since clock gating not disabled on the s2idle suspend.
>>
>> Generally, the CG setting registers are not in always-on domain and the
>> register settings will be lost once it goes down. Not sure about the state of
>> this particular IP rail during S0i3 cycle.
>>
>> If the CG settings are driver-enabled, we reprogram CG settings during
>> resume - amdgpu_device_resume->amdgpu_device_ip_late_init ->
>> amdgpu_device_set_cg_state. This logic prevents this. Maybe, it works fine
>> during your testing because it's done by FW. If the settings are programmed
>> by FW components, usually reprogramming is taken care by FW.
>>
>> Thanks,
>> Lijo
>>
> In the S0i3 entry the gfx power rail will be turn off but MEM_S3 power rail is keeping on and involved device/IP context will be saved in the memory and then each context restored by PM firmware during S0i3 resume.

Thanks, that clarifies.

Thanks,
Lijo

> 
>> Have merged the fix for not blocking s0ix support for some
>>> upcoming asic and meanwhile still need dig into whether the DF C-state,
>> MMHUB DS or BIOS mmhub power gate request different on GC 10.3.7
>> introduce this issue and then make a generic solution for such this issue.
>>>
>>>>>       r = adev->mmhub.funcs->set_clockgating(adev, state);
>>>>>       if (r)
>>>>>               return r;
>>>>>