Re: [PATCH v2 3/4] cpufreq: Add strict_target to struct cpufreq_policy

[Viresh Kumar <viresh.kumar@linaro.org>]

On 09-11-20, 17:53, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Add a new field to be set when the CPUFREQ_GOV_FLAG_STRICT_TARGET
> flag is set for the current governor to struct cpufreq_policy, so
> that the drivers needing to check CPUFREQ_GOV_FLAG_STRICT_TARGET do
> not have to access the governor object during every frequency
> transition.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  drivers/cpufreq/cpufreq.c |    2 ++
>  include/linux/cpufreq.h   |    6 ++++++
>  2 files changed, 8 insertions(+)
> 
> Index: linux-pm/drivers/cpufreq/cpufreq.c
> ===================================================================
> --- linux-pm.orig/drivers/cpufreq/cpufreq.c
> +++ linux-pm/drivers/cpufreq/cpufreq.c
> @@ -2280,6 +2280,8 @@ static int cpufreq_init_governor(struct
>  		}
>  	}
>  
> +	policy->strict_target = !!(policy->governor->flags & CPUFREQ_GOV_FLAG_STRICT_TARGET);
> +
>  	return 0;
>  }
>  
> Index: linux-pm/include/linux/cpufreq.h
> ===================================================================
> --- linux-pm.orig/include/linux/cpufreq.h
> +++ linux-pm/include/linux/cpufreq.h
> @@ -109,6 +109,12 @@ struct cpufreq_policy {
>  	bool			fast_switch_enabled;
>  
>  	/*
> +	 * Set if the CPUFREQ_GOV_FLAG_STRICT_TARGET flag is set for the
> +	 * current governor.
> +	 */
> +	bool			strict_target;
> +
> +	/*
>  	 * Preferred average time interval between consecutive invocations of
>  	 * the driver to set the frequency for this policy.  To be set by the
>  	 * scaling driver (0, which is the default, means no preference).

I was kind of hoping to avoid adding a field here when I proposed updating the
gov structure. I do understand the performance related penalty of accessing the
gov structure for fast switch case though and so wonder if we really need this,
then should we avoid changing the gov structure at all ? I mean there is only
one user of that field now, do we really need a flag for it ? We can just do the
string comparison here with powersave and performance to set strict_target.

Whatever you feel is better though.

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh