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

[Lazar, Lijo]

On 4/27/2022 9:44 AM, Liang, Prike wrote:
> [Public]
> 
>> -----Original Message-----
>> From: Lazar, Lijo <Lijo.Lazar at amd.com>
>> Sent: Tuesday, April 26, 2022 7:19 PM
>> To: Liang, Prike <Prike.Liang at amd.com>; amd-gfx at lists.freedesktop.org
>> Cc: Deucher, Alexander <Alexander.Deucher at amd.com>; Huang, Ray
>> <Ray.Huang at 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 at 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

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;
>>>