[AMD Official Use Only] > -----Original Message----- > From: Lazar, Lijo <Lijo.Lazar@xxxxxxx> > Sent: Thursday, August 12, 2021 3:38 PM > To: Quan, Evan <Evan.Quan@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx > Cc: nils.wallmenius@xxxxxxxxx; Chen, Guchun <Guchun.Chen@xxxxxxx> > Subject: Re: [PATCH V2 6/7] drm/amd/pm: drop unnecessary manual mode > check > > > > On 8/12/2021 12:16 PM, Quan, Evan wrote: > > [AMD Official Use Only] > > > > > > > >> -----Original Message----- > >> From: Lazar, Lijo <Lijo.Lazar@xxxxxxx> > >> Sent: Thursday, August 12, 2021 2:16 PM > >> To: Quan, Evan <Evan.Quan@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx > >> Cc: nils.wallmenius@xxxxxxxxx; Chen, Guchun <Guchun.Chen@xxxxxxx> > >> Subject: Re: [PATCH V2 6/7] drm/amd/pm: drop unnecessary manual > mode > >> check > >> > >> > >> > >> On 8/12/2021 9:31 AM, Evan Quan wrote: > >>> As the fan control was guarded under manual mode before fan speed > >>> RPM/PWM setting. Thus the extra check is totally redundant. > >>> > >>> Change-Id: Ia9d776141ec4aa39255accbf00d7e7ed81c8424d > >>> Signed-off-by: Evan Quan <evan.quan@xxxxxxx> > >>> --- > >>> drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c | 12 +---------- > - > >>> 1 file changed, 1 insertion(+), 11 deletions(-) > >>> > >>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c > >>> b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c > >>> index 9001952442ba..20ece0963f51 100644 > >>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c > >>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c > >>> @@ -1208,9 +1208,6 @@ smu_v11_0_set_fan_speed_pwm(struct > >> smu_context > >>> *smu, uint32_t speed) > >>> > >>> speed = MIN(speed, 255); > >>> > >>> - if (smu_v11_0_auto_fan_control(smu, 0)) > >>> - return -EINVAL; > >>> - > >> > >> There is a FAN_CONTROL_NONE mode where it is set to full speed. Is > >> that affected by this change? > > [Quan, Evan] This check was designed to block "auto" mode(like If it was > under auto mode, these manual settings will be not permitted). > > The FAN_CONTROL_NONE mode is not affected with and without this > check. > > > > To set FAN_CONTROL_NONE, basically also need to turn off auto mode and > manually set to 100% speed. If we take out turning off auto mode here, > probably that part needs to be handled elsewhere. [Quan, Evan] OK, I see. Will add that for AMD_FAN_CTRL_NONE in smu_v11_0_set_fan_control_mode(). That's the right place for mode switching. BR Evan > > Thanks, > Lijo > > > BR > > Evan > >> > >> Thanks, > >> Lijo > >> > >>> duty100 = REG_GET_FIELD(RREG32_SOC15(THM, 0, > >> mmCG_FDO_CTRL1), > >>> CG_FDO_CTRL1, FMAX_DUTY100); > >>> if (!duty100) > >>> @@ -1237,11 +1234,6 @@ int smu_v11_0_set_fan_speed_rpm(struct > >> smu_context *smu, > >>> */ > >>> uint32_t crystal_clock_freq = 2500; > >>> uint32_t tach_period; > >>> - int ret; > >>> - > >>> - ret = smu_v11_0_auto_fan_control(smu, 0); > >>> - if (ret) > >>> - return ret; > >>> > >>> /* > >>> * To prevent from possible overheat, some ASICs may have > >>> requirement @@ -1257,9 +1249,7 @@ int > >> smu_v11_0_set_fan_speed_rpm(struct smu_context *smu, > >>> CG_TACH_CTRL, TARGET_PERIOD, > >>> tach_period)); > >>> > >>> - ret = smu_v11_0_set_fan_static_mode(smu, > >> FDO_PWM_MODE_STATIC_RPM); > >>> - > >>> - return ret; > >>> + return smu_v11_0_set_fan_static_mode(smu, > >> FDO_PWM_MODE_STATIC_RPM); > >>> } > >>> > >>> int smu_v11_0_get_fan_speed_pwm(struct smu_context *smu, > >>>