[Public]
>-----Original Message-----
>From: Lazar, Lijo <Lijo.Lazar@xxxxxxx>
>Sent: Monday, December 6, 2021 11:41 AM
>To: Yu, Lang <Lang.Yu@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
>Cc: Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Huang, Ray
><Ray.Huang@xxxxxxx>
>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@xxxxxxx>
>>> Sent: Friday, December 3, 2021 5:52 PM
>>> To: Yu, Lang <Lang.Yu@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
>>> Cc: Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Huang, Ray
>>> <Ray.Huang@xxxxxxx>
>>> 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@xxxxxxx>
>>>> ---
>>>> 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 false.
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);
>>>>
