[PATCH] drm/amd/pm: Fix that RPM cannot be obtained for specific GPU

[Christian König]

Am 28.09.21 um 23:50 schrieb Alex Deucher:
> On Tue, Sep 28, 2021 at 2:29 AM Christian König
> <ckoenig.leichtzumerken at gmail.com> wrote:
>> Am 28.09.21 um 02:49 schrieb huangyizhi:
>>> The current mechanism for obtaining RPM is to read tach_period from
>>> the register, and then calculate the RPM together with the frequency.
>>> But we found that on specific GPUs, such as RX 550 and RX 560D,
>>> tach_period always reads as 0 and smu7_fan_ctrl_get_fan_speed_rpm
>>> will returns -EINVAL.
>>>
>>> To solve this problem, when reading tach_period as 0, we try
>>> to estimate the current RPM using the percentage of current pwm, the
>>> maximum and minimum RPM.
>> Well that is most likely a bad idea.
>>
>> When the fan speed is not available faking some value is certainly not
>> the right solution, especially when you don't know the topology of the
>> DC conversion driven by the PWM.
>>
> I think there is a flag in the vbios to determine whether a specific
> board supports rpm based fan control.  This used to be an AIB specific
> option.  If the flag is not set, the driver should not expose the rpm
> interface for fan control, only the PWM interface.  I think at some
> point rpm fan control became mandatory, but maybe it was still an
> option on polaris and we are missing a check for that flag.

Yeah, that sounds totally sane to me as well.

Let's ask for a volunteer for the job on Thursday if not somebody from 
the community speaks up.

Christian.

>
> Alex
>
>
>> Christian.
>>
>>> Signed-off-by: huangyizhi <huangyizhi at hnu.edu.cn>
>>> ---
>>>    .../drm/amd/pm/powerplay/hwmgr/smu7_thermal.c | 28 ++++++++++++++++---
>>>    1 file changed, 24 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_thermal.c b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_thermal.c
>>> index a6c3610db23e..307dd87d6882 100644
>>> --- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_thermal.c
>>> +++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_thermal.c
>>> @@ -81,6 +81,11 @@ int smu7_fan_ctrl_get_fan_speed_rpm(struct pp_hwmgr *hwmgr, uint32_t *speed)
>>>    {
>>>        uint32_t tach_period;
>>>        uint32_t crystal_clock_freq;
>>> +     uint32_t duty100;
>>> +     uint32_t duty;
>>> +     uint32_t speed_percent;
>>> +     uint64_t tmp64;
>>> +
>>>
>>>        if (hwmgr->thermal_controller.fanInfo.bNoFan ||
>>>            !hwmgr->thermal_controller.fanInfo.ucTachometerPulsesPerRevolution)
>>> @@ -89,13 +94,28 @@ int smu7_fan_ctrl_get_fan_speed_rpm(struct pp_hwmgr *hwmgr, uint32_t *speed)
>>>        tach_period = PHM_READ_VFPF_INDIRECT_FIELD(hwmgr->device, CGS_IND_REG__SMC,
>>>                        CG_TACH_STATUS, TACH_PERIOD);
>>>
>>> -     if (tach_period == 0)
>>> -             return -EINVAL;
>>> +     if (tach_period == 0) {
>>>
>>> -     crystal_clock_freq = amdgpu_asic_get_xclk((struct amdgpu_device *)hwmgr->adev);
>>> +             duty100 = PHM_READ_VFPF_INDIRECT_FIELD(hwmgr->device, CGS_IND_REG__SMC,
>>> +                             CG_FDO_CTRL1, FMAX_DUTY100);
>>> +             duty = PHM_READ_VFPF_INDIRECT_FIELD(hwmgr->device, CGS_IND_REG__SMC,
>>> +                             CG_THERMAL_STATUS, FDO_PWM_DUTY);
>>>
>>> -     *speed = 60 * crystal_clock_freq * 10000 / tach_period;
>>> +             if (duty100 == 0)
>>> +                     return -EINVAL;
>>>
>>> +             tmp64 = (uint64_t)duty * 100;
>>> +             do_div(tmp64, duty100);
>>> +             speed_percent = MIN((uint32_t)tmp64, 100);
>>> +
>>> +             *speed = speed_percent * (hwmgr->thermal_controller.fanInfo.ulMaxRPM
>>> +                     - hwmgr->thermal_controller.fanInfo.ulMinRPM) / 100;
>>> +     } else {
>>> +
>>> +             crystal_clock_freq = amdgpu_asic_get_xclk((struct amdgpu_device *)hwmgr->adev);
>>> +
>>> +             *speed = 60 * crystal_clock_freq * 10000 / tach_period;
>>> +     }
>>>        return 0;
>>>    }
>>>