diff mbox series

[-next] cpufreq: userspace: Keep the current frequency when set userspace policy

Message ID 20231025080910.3245690-1-zengheng4@huawei.com
State New
Headers
Series [-next] cpufreq: userspace: Keep the current frequency when set userspace policy |

Commit Message

Zeng Heng Oct. 25, 2023, 8:09 a.m. UTC 
  When switching to the userspace policy, if the current frequency is within
the range of policy's min and max values, the current frequency value
should be remained. The .limit() function is called when changing governor
or updating governor limits, so in both cases, there is no need to update
frequency if the current frequency does not exceed the threshold.

Additionally, when changing to userspace governor, the default value of
set_speed is set by reading the current frequency of the CPU, but there
is inevitable error between the frequency coming from .get_rate() interface
and the actual working frequency. Consequently, when switching to userspace
policy, keeping the current frequency can avoid unexpected changes.

Signed-off-by: Zeng Heng <zengheng4@huawei.com>
---
 drivers/cpufreq/cpufreq_userspace.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

--
2.25.1

Comments

Viresh Kumar Oct. 25, 2023, 11:18 a.m. UTC | #1 
On 25-10-23, 16:09, Zeng Heng wrote:
> When switching to the userspace policy, if the current frequency is within
> the range of policy's min and max values, the current frequency value
> should be remained. The .limit() function is called when changing governor
> or updating governor limits, so in both cases, there is no need to update
> frequency if the current frequency does not exceed the threshold.
> 
> Additionally, when changing to userspace governor, the default value of
> set_speed is set by reading the current frequency of the CPU, but there
> is inevitable error between the frequency coming from .get_rate() interface
> and the actual working frequency. Consequently, when switching to userspace
> policy, keeping the current frequency can avoid unexpected changes.
> 
> Signed-off-by: Zeng Heng <zengheng4@huawei.com>
> ---
>  drivers/cpufreq/cpufreq_userspace.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/cpufreq/cpufreq_userspace.c b/drivers/cpufreq/cpufreq_userspace.c
> index 2c42fee76daa..fe55a7bb663c 100644
> --- a/drivers/cpufreq/cpufreq_userspace.c
> +++ b/drivers/cpufreq/cpufreq_userspace.c
> @@ -117,9 +117,7 @@ static void cpufreq_userspace_policy_limits(struct cpufreq_policy *policy)
>  	else if (policy->min > userspace->setspeed)
>  		__cpufreq_driver_target(policy, policy->min,
>  					CPUFREQ_RELATION_L);
> -	else
> -		__cpufreq_driver_target(policy, userspace->setspeed,
> -					CPUFREQ_RELATION_L);
> +	/* Otherwise, keep the current frequency. */
> 
>  	mutex_unlock(&userspace->mutex);
>  }

Here is some reasoning why it should be done the way it is:

commit e43e94c1eda7 ("cpufreq: Fix GOV_LIMITS handling for the userspace governor")
Zeng Heng Oct. 26, 2023, 2:36 a.m. UTC | #2 
在 2023/10/25 19:18, Viresh Kumar 写道:
> On 25-10-23, 16:09, Zeng Heng wrote:
>> When switching to the userspace policy, if the current frequency is within
>> the range of policy's min and max values, the current frequency value
>> should be remained. The .limit() function is called when changing governor
>> or updating governor limits, so in both cases, there is no need to update
>> frequency if the current frequency does not exceed the threshold.
>>
>> Additionally, when changing to userspace governor, the default value of
>> set_speed is set by reading the current frequency of the CPU, but there
>> is inevitable error between the frequency coming from .get_rate() interface
>> and the actual working frequency. Consequently, when switching to userspace
>> policy, keeping the current frequency can avoid unexpected changes.
>>
>> Signed-off-by: Zeng Heng <zengheng4@huawei.com>
>> ---
>>   drivers/cpufreq/cpufreq_userspace.c | 4 +---
>>   1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/drivers/cpufreq/cpufreq_userspace.c b/drivers/cpufreq/cpufreq_userspace.c
>> index 2c42fee76daa..fe55a7bb663c 100644
>> --- a/drivers/cpufreq/cpufreq_userspace.c
>> +++ b/drivers/cpufreq/cpufreq_userspace.c
>> @@ -117,9 +117,7 @@ static void cpufreq_userspace_policy_limits(struct cpufreq_policy *policy)
>>   	else if (policy->min > userspace->setspeed)
>>   		__cpufreq_driver_target(policy, policy->min,
>>   					CPUFREQ_RELATION_L);
>> -	else
>> -		__cpufreq_driver_target(policy, userspace->setspeed,
>> -					CPUFREQ_RELATION_L);
>> +	/* Otherwise, keep the current frequency. */
>>
>>   	mutex_unlock(&userspace->mutex);
>>   }
> Here is some reasoning why it should be done the way it is:
>
> commit e43e94c1eda7 ("cpufreq: Fix GOV_LIMITS handling for the userspace governor")


Get it, thanks for the response.

Zeng Heng
diff mbox series

Patch

diff --git a/drivers/cpufreq/cpufreq_userspace.c b/drivers/cpufreq/cpufreq_userspace.c
index 2c42fee76daa..fe55a7bb663c 100644
--- a/drivers/cpufreq/cpufreq_userspace.c
+++ b/drivers/cpufreq/cpufreq_userspace.c
@@ -117,9 +117,7 @@  static void cpufreq_userspace_policy_limits(struct cpufreq_policy *policy)
 	else if (policy->min > userspace->setspeed)
 		__cpufreq_driver_target(policy, policy->min,
 					CPUFREQ_RELATION_L);
-	else
-		__cpufreq_driver_target(policy, userspace->setspeed,
-					CPUFREQ_RELATION_L);
+	/* Otherwise, keep the current frequency. */

 	mutex_unlock(&userspace->mutex);
 }