Re: [PATCH] drm/amdgpu: getting fan speed pwm for vega10 properly

[Alex Deucher]

On Mon, Sep 5, 2022 at 3:04 AM Yury Zhuravlev <stalk...@gmail.com> wrote:
>
> Hello,
>
> During the setup, the fan manager https://github.com/markusressel/fan2go I 
> found that my Vega56 was not working correctly. This fan manager expects what 
> read PWM value should be the same as you wrote before, but it's not the case. 
> PWM value was volatile, and what is more critical, if I wrote 200, after 
> reading I saw ~70-100, which is very confusing.
> After that, I started reading the amdgpu driver, and how fan speed works, and 
> I found what PWM value was calculated from RPM speed and not correct for my 
> case (different BIOS or fan configuration?).
> Because it looked wrong, I started looking into different implementations and 
> found that Vega20 used mmCG_FDO_CTRL1 and mmCG_THERMAL_STATUS registers to 
> calculate the PWM value.
> I also checked how we set PWM for Vega10 and found the same registers. After 
> that, I copy-pasted the function from Vega20 to Vega10, and it started 
> working much better. It still has some fluctuation, but as I understand, this 
> behavior is expected.
>
> I have no in-depth information about amdgpu, and the original function may 
> have been for some reason (maybe for some broken BIOS?), but I suppose 
> somebody forgot to backport this code after prototype implementation.
>
> It would be my first patch here. Sorry if I skipped some procedures, will be 
> appreciated it if you help me.

Please send this as a proper patch with your Signed-off-by using git-send-email.

Alex

>
> Regards,
>
> ---
> diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_thermal.c 
> b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_thermal.c
> index dad3e3741a4e..190af79f3236 100644
> --- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_thermal.c
> +++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_thermal.c
> @@ -67,22 +67,21 @@ int vega10_fan_ctrl_get_fan_speed_info(struct pp_hwmgr 
> *hwmgr,
>  int vega10_fan_ctrl_get_fan_speed_pwm(struct pp_hwmgr *hwmgr,
>                 uint32_t *speed)
>  {
> -       uint32_t current_rpm;
> -       uint32_t percent = 0;
> -
> -       if (hwmgr->thermal_controller.fanInfo.bNoFan)
> -               return 0;
> +       struct amdgpu_device *adev = hwmgr->adev;
> +       uint32_t duty100, duty;
> +       uint64_t tmp64;
>
> -       if (vega10_get_current_rpm(hwmgr, &current_rpm))
> -               return -1;
> +       duty100 = REG_GET_FIELD(RREG32_SOC15(THM, 0, mmCG_FDO_CTRL1),
> +                               CG_FDO_CTRL1, FMAX_DUTY100);
> +       duty = REG_GET_FIELD(RREG32_SOC15(THM, 0, mmCG_THERMAL_STATUS),
> +                               CG_THERMAL_STATUS, FDO_PWM_DUTY);
>
> -       if (hwmgr->thermal_controller.
> -                       advanceFanControlParameters.usMaxFanRPM != 0)
> -               percent = current_rpm * 255 /
> -                       hwmgr->thermal_controller.
> -                       advanceFanControlParameters.usMaxFanRPM;
> +       if (!duty100)
> +               return -EINVAL;
>
> -       *speed = MIN(percent, 255);
> +       tmp64 = (uint64_t)duty * 255;
> +       do_div(tmp64, duty100);
> +       *speed = MIN((uint32_t)tmp64, 255);
>
>         return 0;
>  }
> --