[PATCH 2/2] drm/amdgpu: allow APU to send power gate message when dpm is disabled

[Yu, Lang]

[Public]

A typo.

>-----Original Message-----
>From: Yu, Lang
>Sent: Monday, December 6, 2021 2:47 PM
>To: Lazar, Lijo <Lijo.Lazar 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 2/2] drm/amdgpu: allow APU to send power gate message
>when dpm is disabled
>
>[Public]
>
>
>
>>-----Original Message-----
>>From: Lazar, Lijo <Lijo.Lazar at amd.com>
>>Sent: Monday, December 6, 2021 11:41 AM
>>To: Yu, Lang <Lang.Yu 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 2/2] drm/amdgpu: allow APU to send power gate
>>message when dpm is disabled
>>
>>
>>
>>On 12/6/2021 8:19 AM, Yu, Lang wrote:
>>> [Public]
>>>
>>>
>>>
>>>> -----Original Message-----
>>>> From: Lazar, Lijo <Lijo.Lazar at amd.com>
>>>> Sent: Friday, December 3, 2021 5:52 PM
>>>> To: Yu, Lang <Lang.Yu 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 2/2] drm/amdgpu: allow APU to send power gate
>>>> message when dpm is disabled
>>>>
>>>>
>>>>
>>>> On 12/3/2021 12:24 PM, Lang Yu wrote:
>>>>> The general hw fini sequence is SMU-> ... ->SDMA-> ...
>>>>> We need to send power gate message to power off SDMA(in SDMA
>>>>> hw_fini()) afer dpm is disabled(in SMU hw_fini()). Allow that for APU.
>>>>
>>>> This message is not right. In APUs there is no message provided by
>>>> FW to enable/disable DPM, it is done in BIOS. Rephrase to something
>>>> like after smu hw_fini is completed.
>>>
>>> It is power on/off SDMA message. Not enable/disable DPM.
>>>
>>Bad choice of word :) I didn't mean FW message, it was about this line
>>in "commit message" - "afer dpm is disabled".
>
>Ok. I got it.
>
>>
>>>>>
>>>>> Signed-off-by: Lang Yu <lang.yu at amd.com>
>>>>> ---
>>>>>    drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 2 +-
>>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>>>>> b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>>>>> index 2d718c30c8eb..285a237f3605 100644
>>>>> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>>>>> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>>>>> @@ -277,7 +277,7 @@ static int smu_dpm_set_power_gate(void *handle,
>>>>>    	struct smu_context *smu = handle;
>>>>>    	int ret = 0;
>>>>>
>>>>> -	if (!smu->pm_enabled || !smu->adev->pm.dpm_enabled) {
>>>>> +	if (!smu->pm_enabled || (!smu->is_apu &&
>>>>> +!smu->adev->pm.dpm_enabled)) {
>>>>
>>>>
>>>> This check was there before also, only the WARN is added. That means
>>>> it was skipping sending messages in APUs also and so far this was
>>>> working fine (until this gets noticed because of the warning).
>>>>
>>>> Now this would try to send the message to APU without any check.
>>>> That doesn't look good. Ideal way should be to fix the sequence.
>>>> Otherwise, suggest to do something like below as the last step of
>>>> smu hw cleanup rather than sending the message blindly.
>>>>
>>>> 	if (smu->is_apu)
>>>> 		smu->pm.dpm_enabled = smu_is_dpm_running(smu);
>>>
>>> smu_is_dpm_running(smu) will cause errors in suspend.
>>>
>>That is interesting. What is the error you get?
>
>[drm:amdgpu_dpm_enable_uvd [amdgpu]] *ERROR* Dpm enable uvd failed, ret =
>-95 That means EOPNOTSUPP.
>
>Actually, in resume process, but adev->in_suspend  is still true.
>For Renoir series APU, smu_is_dpm_running is hardcoded as following,
>
>static bool renoir_is_dpm_running(struct smu_context *smu) {
>	struct amdgpu_device *adev = smu->adev;
>
>	/*
>	 * Until now, the pmfw hasn't exported the interface of SMU
>	 * feature mask to APU SKU so just force on all the feature
>	 * at early initial stage.
>	 */
>	if (adev->in_suspend)
>		return false;
>	else
>		return true;
>
>}
>
>So we got such an error.
>
>Regards,
>Lang
>
>>Thanks,
>>Lijo
>>
>>> Here we just  send some IP power on/off messages.
>>> Is it necessary to enable DPM to send such messages?
>>>
>>> Regards,
>>> Lang
>>>
>>>> Thanks,
>>>> Lijo
>>>>
>>>>>    		dev_WARN(smu->adev->dev,
>>>>>    			 "SMU uninitialized but power %s requested for %u!\n",
>>>>>    			 gate ? "gate" : "ungate", block_type);
>>>>>