[PATCH V2 6/7] drm/amd/pm: drop unnecessary manual mode check

[Quan, Evan]

[AMD Official Use Only]



> -----Original Message-----
> From: Lazar, Lijo <Lijo.Lazar at amd.com>
> Sent: Thursday, August 12, 2021 3:38 PM
> To: Quan, Evan <Evan.Quan at amd.com>; amd-gfx at lists.freedesktop.org
> Cc: nils.wallmenius at gmail.com; Chen, Guchun <Guchun.Chen at amd.com>
> 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 at amd.com>
> >> Sent: Thursday, August 12, 2021 2:16 PM
> >> To: Quan, Evan <Evan.Quan at amd.com>; amd-gfx at lists.freedesktop.org
> >> Cc: nils.wallmenius at gmail.com; Chen, Guchun <Guchun.Chen at amd.com>
> >> 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 at amd.com>
> >>> ---
> >>>    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,
> >>>