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

[Alex Deucher <alexdeucher@gmail.com>]

On Tue, Sep 28, 2021 at 2:29 AM Christian König
<ckoenig.leichtzumerken@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.

Alex


> Christian.
>
> >
> > Signed-off-by: huangyizhi <huangyizhi@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;
> >   }
> >
>