[AMD Official Use Only]
> -----Original Message-----
> From: Lazar, Lijo <Lijo.Lazar@xxxxxxx>
> Sent: Thursday, August 12, 2021 5:49 PM
> To: Quan, Evan <Evan.Quan@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> Cc: nils.wallmenius@xxxxxxxxx; Chen, Guchun <Guchun.Chen@xxxxxxx>
> Subject: Re: [PATCH V3 2/7] drm/amd/pm: record the RPM and PWM based
> fan speed settings
>
>
>
> On 8/12/2021 3:03 PM, Evan Quan wrote:
> > As the relationship "PWM = RPM / smu->fan_max_rpm" between fan
> speed
> > PWM and RPM is not true for SMU11 ASICs. So, both the RPM and PWM
> > settings need to be saved.
> >
> > Change-Id: I318c134d442273d518b805339cdf383e151b935d
> > Signed-off-by: Evan Quan <evan.quan@xxxxxxx>
> > --
> > v1->v2:
> > - coding style and logging prints optimization (Guchun)
> > - reuse existing flags (Lijo)
> > v2->v3:
> > - disallow fan speed setting via PWM and RPM at the same time
> > (Lijo)
> > ---
> > drivers/gpu/drm/amd/pm/amdgpu_pm.c | 3 +++
> > drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h | 3 +++
> > drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 30
> ++++++++++++++++++-----
> > 3 files changed, 30 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> > b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> > index 04c7d82f8b89..112ee5f5d855 100644
> > --- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> > +++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> > @@ -3174,6 +3174,9 @@ static ssize_t
> amdgpu_hwmon_show_mclk_label(struct device *dev,
> > *
> > * - fan[1-\*]_enable: Enable or disable the sensors.1: Enable 0: Disable
> > *
> > + * NOTE: DO NOT set the fan speed via "pwm1" and "fan[1-\*]_target"
> interfaces at the same time.
> > + * That will get the former one overridden.
> > + *
> > * hwmon interfaces for GPU clocks:
> > *
> > * - freq1_input: the gfx/compute clock in hertz diff --git
> > a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
> > b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
> > index 183654f8b564..29934a5af44e 100644
> > --- a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
> > +++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
> > @@ -34,6 +34,8 @@
> > #define SMU_FW_NAME_LEN 0x24
> >
> > #define SMU_DPM_USER_PROFILE_RESTORE (1 << 0)
> > +#define SMU_CUSTOM_FAN_SPEED_RPM (1 << 1)
> > +#define SMU_CUSTOM_FAN_SPEED_PWM (1 << 2)
> >
> > // Power Throttlers
> > #define SMU_THROTTLER_PPT0_BIT 0
> > @@ -230,6 +232,7 @@ struct smu_user_dpm_profile {
> > uint32_t fan_mode;
> > uint32_t power_limit;
> > uint32_t fan_speed_percent;
> > + uint32_t fan_speed_rpm;
> > uint32_t flags;
> > uint32_t user_od;
> >
> > diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> > b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> > index e33e67310030..39390bbb79e8 100644
> > --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> > +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> > @@ -413,7 +413,13 @@ static void smu_restore_dpm_user_profile(struct
> smu_context *smu)
> > if (!ret && smu->user_dpm_profile.fan_speed_percent) {
> > ret = smu_set_fan_speed_percent(smu, smu-
> >user_dpm_profile.fan_speed_percent);
> > if (ret)
> > - dev_err(smu->adev->dev, "Failed to set
> manual fan speed\n");
> > + dev_err(smu->adev->dev, "Failed to set
> manual fan speed in percent\n");
> > + }
> > +
> > + if (!ret && smu->user_dpm_profile.fan_speed_rpm) {
> > + ret = smu_set_fan_speed_rpm(smu, smu-
> >user_dpm_profile.fan_speed_rpm);
> > + if (ret)
> > + dev_err(smu->adev->dev, "Failed to set
> manual fan speed in
> > +rpm\n");
> > }
> > }
> >
> > @@ -2179,7 +2185,6 @@ static int smu_set_gfx_cgpg(struct smu_context
> *smu, bool enabled)
> > static int smu_set_fan_speed_rpm(void *handle, uint32_t speed)
> > {
> > struct smu_context *smu = handle;
> > - u32 percent;
> > int ret = 0;
> >
> > if (!smu->pm_enabled || !smu->adev->pm.dpm_enabled) @@ -
> 2190,8
> > +2195,12 @@ static int smu_set_fan_speed_rpm(void *handle, uint32_t
> speed)
> > if (smu->ppt_funcs->set_fan_speed_rpm) {
> > ret = smu->ppt_funcs->set_fan_speed_rpm(smu, speed);
> > if (!ret && smu->user_dpm_profile.flags &
> > SMU_DPM_USER_PROFILE_RESTORE) {
>
> Sorry, another miss in review :) The above line should probably be - if (!ret
> && !(smu->user_dpm_profile.flags & SMU_DPM_USER_PROFILE_RESTORE))
> {
>
> Fix it while submitting. Apart from that, the series is
>
> Reviewed-by: Lijo Lazar <lijo.lazar@xxxxxxx>
[Quan, Evan] Thanks! Will update this on submitting.
BR
Evan
>
> Thanks,
> Lijo
>
> > - percent = speed * 100 / smu->fan_max_rpm;
> > - smu->user_dpm_profile.fan_speed_percent =
> percent;
> > + smu->user_dpm_profile.flags |=
> SMU_CUSTOM_FAN_SPEED_RPM;
> > + smu->user_dpm_profile.fan_speed_rpm = speed;
> > +
> > + /* Override custom PWM setting as they cannot co-
> exist */
> > + smu->user_dpm_profile.flags &=
> ~SMU_CUSTOM_FAN_SPEED_PWM;
> > + smu->user_dpm_profile.fan_speed_percent = 0;
> > }
> > }
> >
> > @@ -2552,8 +2561,11 @@ static int smu_set_fan_control_mode(struct
> > smu_context *smu, int value)
> >
> > /* reset user dpm fan speed */
> > if (!ret && value != AMD_FAN_CTRL_MANUAL &&
> > - !(smu->user_dpm_profile.flags &
> SMU_DPM_USER_PROFILE_RESTORE))
> > + !(smu->user_dpm_profile.flags &
> SMU_DPM_USER_PROFILE_RESTORE)) {
> > smu->user_dpm_profile.fan_speed_percent = 0;
> > + smu->user_dpm_profile.fan_speed_rpm = 0;
> > + smu->user_dpm_profile.flags &=
> ~(SMU_CUSTOM_FAN_SPEED_RPM | SMU_CUSTOM_FAN_SPEED_PWM);
> > + }
> >
> > return ret;
> > }
> > @@ -2604,8 +2616,14 @@ static int smu_set_fan_speed_percent(void
> *handle, u32 speed)
> > if (speed > 100)
> > speed = 100;
> > ret = smu->ppt_funcs->set_fan_speed_percent(smu,
> speed);
> > - if (!ret && !(smu->user_dpm_profile.flags &
> SMU_DPM_USER_PROFILE_RESTORE))
> > + if (!ret && !(smu->user_dpm_profile.flags &
> SMU_DPM_USER_PROFILE_RESTORE)) {
> > + smu->user_dpm_profile.flags |=
> SMU_CUSTOM_FAN_SPEED_PWM;
> > smu->user_dpm_profile.fan_speed_percent =
> speed;
> > +
> > + /* Override custom RPM setting as they cannot co-
> exist */
> > + smu->user_dpm_profile.flags &=
> ~SMU_CUSTOM_FAN_SPEED_RPM;
> > + smu->user_dpm_profile.fan_speed_rpm = 0;
> > + }
> > }
> >
> > mutex_unlock(&smu->mutex);
> >
