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

["Lazar, Lijo" <lijo.lazar@amd.com>]

On 8/12/2021 1:49 PM, Quan, Evan wrote:
> [AMD Official Use Only]
> 
> 
> 
>> -----Original Message-----
>> From: Lazar, Lijo <Lijo.Lazar@amd.com>
>> Sent: Thursday, August 12, 2021 3:38 PM
>> To: Quan, Evan <Evan.Quan@amd.com>; amd-gfx@lists.freedesktop.org
>> Cc: nils.wallmenius@gmail.com; Chen, Guchun <Guchun.Chen@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@amd.com>
>>>> Sent: Thursday, August 12, 2021 2:16 PM
>>>> To: Quan, Evan <Evan.Quan@amd.com>; amd-gfx@lists.freedesktop.org
>>>> Cc: nils.wallmenius@gmail.com; Chen, Guchun <Guchun.Chen@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@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.
> 
Yes, that is the right place.

Thanks,
Lijo

> 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,
>>>>>