Re: [PATCH V5 2/2] cpufreq: intel_pstate: Implement QoS supported freq constraints

2019-08-26 Thread Rafael J. Wysocki
On Friday, August 9, 2019 4:22:49 AM CEST Viresh Kumar wrote:
> Intel pstate driver exposes min_perf_pct and max_perf_pct sysfs files,
> which can be used to force a limit on the min/max P state of the driver.
> Though these files eventually control the min/max frequencies that the
> CPUs will run at, they don't make a change to policy->min/max values.
> 
> When the values of these files are changed (in passive mode of the
> driver), it leads to calling ->limits() callback of the cpufreq
> governors, like schedutil. On a call to it the governors shall
> forcefully update the frequency to come within the limits. Since the
> limits, i.e.  policy->min/max, aren't updated by the driver, the
> governors fails to get the target freq within limit and sometimes aborts
> the update believing that the frequency is already set to the target
> value.
> 
> This patch implements the QoS supported frequency constraints to update
> policy->min/max values whenever min_perf_pct or max_perf_pct files are
> updated. This is only done for the passive mode as of now, as the driver
> is already working fine in active mode.
> 
> Fixes: ecd288429126 ("cpufreq: schedutil: Don't set next_freq to UINT_MAX")
> Reported-by: Doug Smythies 
> Signed-off-by: Viresh Kumar 
> ---
> V4->V5:
> - dev_pm_qos_update_request() can return 1 in case of success, handle
>   that.
> 
>  drivers/cpufreq/intel_pstate.c | 120 +++--
>  1 file changed, 116 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index cc27d4c59dca..32f27563613b 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -24,6 +24,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  #include 
> @@ -1085,6 +1086,47 @@ static ssize_t store_no_turbo(struct kobject *a, 
> struct kobj_attribute *b,
>   return count;
>  }
>  
> +static struct cpufreq_driver intel_pstate;
> +
> +static void update_qos_request(enum dev_pm_qos_req_type type)
> +{
> + int max_state, turbo_max, freq, i, perf_pct;
> + struct dev_pm_qos_request *req;
> + struct cpufreq_policy *policy;
> +
> + for_each_possible_cpu(i) {
> + struct cpudata *cpu = all_cpu_data[i];
> +
> + policy = cpufreq_cpu_get(i);
> + if (!policy)
> + continue;
> +
> + req = policy->driver_data;
> + cpufreq_cpu_put(policy);
> +
> + if (!req)
> + continue;
> +
> + if (hwp_active)
> + intel_pstate_get_hwp_max(i, _max, _state);
> + else
> + turbo_max = cpu->pstate.turbo_pstate;
> +
> + if (type == DEV_PM_QOS_MIN_FREQUENCY) {
> + perf_pct = global.min_perf_pct;
> + } else {
> + req++;
> + perf_pct = global.max_perf_pct;
> + }
> +
> + freq = DIV_ROUND_UP(turbo_max * perf_pct, 100);
> + freq *= cpu->pstate.scaling;
> +
> + if (dev_pm_qos_update_request(req, freq) < 0)
> + pr_warn("Failed to update freq constraint: CPU%d\n", i);
> + }
> +}
> +
>  static ssize_t store_max_perf_pct(struct kobject *a, struct kobj_attribute 
> *b,
> const char *buf, size_t count)
>  {
> @@ -1108,7 +1150,10 @@ static ssize_t store_max_perf_pct(struct kobject *a, 
> struct kobj_attribute *b,
>  
>   mutex_unlock(_pstate_limits_lock);
>  
> - intel_pstate_update_policies();
> + if (intel_pstate_driver == _pstate)
> + intel_pstate_update_policies();
> + else
> + update_qos_request(DEV_PM_QOS_MAX_FREQUENCY);
>  
>   mutex_unlock(_pstate_driver_lock);
>  
> @@ -1139,7 +1184,10 @@ static ssize_t store_min_perf_pct(struct kobject *a, 
> struct kobj_attribute *b,
>  
>   mutex_unlock(_pstate_limits_lock);
>  
> - intel_pstate_update_policies();
> + if (intel_pstate_driver == _pstate)
> + intel_pstate_update_policies();
> + else
> + update_qos_request(DEV_PM_QOS_MIN_FREQUENCY);
>  
>   mutex_unlock(_pstate_driver_lock);
>  
> @@ -2332,8 +2380,16 @@ static unsigned int intel_cpufreq_fast_switch(struct 
> cpufreq_policy *policy,
>  
>  static int intel_cpufreq_cpu_init(struct cpufreq_policy *policy)
>  {
> - int ret = __intel_pstate_cpu_init(policy);
> + int max_state, turbo_max, min_freq, max_freq, ret;
> + struct dev_pm_qos_request *req;
> + struct cpudata *cpu;
> + struct device *dev;
> +
> + dev = get_cpu_device(policy->cpu);
> + if (!dev)
> + return -ENODEV;
>  
> + ret = __intel_pstate_cpu_init(policy);
>   if (ret)
>   return ret;
>  
> @@ -2342,7 +2398,63 @@ static int intel_cpufreq_cpu_init(struct 
> cpufreq_policy *policy)
>   /* This reflects the intel_pstate_get_cpu_pstates() setting. */
>   

RE: [PATCH V5 2/2] cpufreq: intel_pstate: Implement QoS supported freq constraints

2019-08-08 Thread Doug Smythies
On 2019.08.08 19:23 Viresh Kumar wrote:
> ---
> V4->V5:
> - dev_pm_qos_update_request() can return 1 in case of success, handle
>   that.

O.K. thanks,
That fixes the "Fail" messages I was getting with V4.

... Doug