| Message ID | 5722513B.4080801@nvidia.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
On 28-04-16, 11:06, Sai Gurrappadi wrote: > Resending without the copyright headed appended..sorry! > > -Sai > > ======= :) For your information, this isn't the right place to add text which shouldn't be committed. > > Currently, the userspace governor only updates frequency on GOV_LIMITS > if policy->cur falls outside policy->{min/max}. However, it is also > necessary to update current frequency on GOV_LIMITS to match the user > requested value if it can be achieved within the new policy->{max/min}. > > This was previously the behaviour in the governor until commit d1922f0 > ("cpufreq: Simplify userspace governor") which incorrectly assumed that > policy->cur == user requested frequency via scaling_setspeed. This won't > be true if the user requested frequency falls outside policy->{min/max}. > Ex: a temporary thermal cap throttled the user requested frequency. > > Fix this by doing a partial revert of commit d1922f0 to bring back the > per-cpu cpu_set_freq variable that stores the user requested frequency. > The governor will then try to achieve this request on every GOV_LIMITS. > > Signed-off-by: Sai Gurrappadi <sgurrappadi@nvidia.com> > --- Rather do it here and git am will automatically drop that while applying the patch. > drivers/cpufreq/cpufreq_userspace.c | 20 ++++++++++++++++---- > 1 file changed, 16 insertions(+), 4 deletions(-) > > diff --git a/drivers/cpufreq/cpufreq_userspace.c > b/drivers/cpufreq/cpufreq_userspace.c > index 4d16f45..ae03ba7 100644 > --- a/drivers/cpufreq/cpufreq_userspace.c > +++ b/drivers/cpufreq/cpufreq_userspace.c > @@ -18,6 +18,8 @@ > #include <linux/module.h> > #include <linux/mutex.h> > > +static DEFINE_PER_CPU(unsigned int, cpu_set_freq); /* CPU freq desired by > + userspace */ > static DEFINE_PER_CPU(unsigned int, cpu_is_managed); > static DEFINE_MUTEX(userspace_mutex); > > @@ -38,6 +40,8 @@ static int cpufreq_set(struct cpufreq_policy *policy, > unsigned int freq) > if (!per_cpu(cpu_is_managed, policy->cpu)) > goto err; > > + per_cpu(cpu_set_freq, policy->cpu) = freq; > + > ret = __cpufreq_driver_target(policy, freq, CPUFREQ_RELATION_L); > err: > mutex_unlock(&userspace_mutex); > @@ -62,6 +66,7 @@ static int cpufreq_governor_userspace(struct cpufreq_policy > *policy, > > mutex_lock(&userspace_mutex); > per_cpu(cpu_is_managed, cpu) = 1; > + per_cpu(cpu_set_freq, cpu) = policy->cur; > mutex_unlock(&userspace_mutex); > break; > case CPUFREQ_GOV_STOP: > @@ -69,20 +74,27 @@ static int cpufreq_governor_userspace(struct > cpufreq_policy *policy, > > mutex_lock(&userspace_mutex); > per_cpu(cpu_is_managed, cpu) = 0; > + per_cpu(cpu_set_freq, cpu) = 0; > mutex_unlock(&userspace_mutex); > break; > case CPUFREQ_GOV_LIMITS: > mutex_lock(&userspace_mutex); > - pr_debug("limit event for cpu %u: %u - %u kHz, currently %u kHz\n", > + pr_debug("limit event for cpu %u: %u - %u kHz, " > + "currently %u kHz, last set to %u kHz\n", > cpu, policy->min, policy->max, > - policy->cur); > + policy->cur, per_cpu(cpu_set_freq, cpu)); > > - if (policy->max < policy->cur) > + if (policy->max < per_cpu(cpu_set_freq, cpu)) { > __cpufreq_driver_target(policy, policy->max, > CPUFREQ_RELATION_H); > - else if (policy->min > policy->cur) > + } else if (policy->min > per_cpu(cpu_set_freq, cpu)) { > __cpufreq_driver_target(policy, policy->min, > CPUFREQ_RELATION_L); > + } else { > + __cpufreq_driver_target(policy, > + per_cpu(cpu_set_freq, cpu), > + CPUFREQ_RELATION_L); > + } > mutex_unlock(&userspace_mutex); > break; > } > -- 2.1.4
On 04/28/2016 08:49 PM, Viresh Kumar wrote: > On 28-04-16, 11:06, Sai Gurrappadi wrote: >> Resending without the copyright headed appended..sorry! >> >> -Sai >> >> ======= > > :) > > For your information, this isn't the right place to add text which shouldn't be > committed. [..] >> >> Signed-off-by: Sai Gurrappadi <sgurrappadi@nvidia.com> >> --- > > Rather do it here and git am will automatically drop that while applying the > patch. Good to know. Thanks! -Sai > >> drivers/cpufreq/cpufreq_userspace.c | 20 ++++++++++++++++---- -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
=======
Currently, the userspace governor only updates frequency on GOV_LIMITS
if policy->cur falls outside policy->{min/max}. However, it is also
necessary to update current frequency on GOV_LIMITS to match the user
requested value if it can be achieved within the new policy->{max/min}.
This was previously the behaviour in the governor until commit d1922f0
("cpufreq: Simplify userspace governor") which incorrectly assumed that
policy->cur == user requested frequency via scaling_setspeed. This won't
be true if the user requested frequency falls outside policy->{min/max}.
Ex: a temporary thermal cap throttled the user requested frequency.
Fix this by doing a partial revert of commit d1922f0 to bring back the
per-cpu cpu_set_freq variable that stores the user requested frequency.
The governor will then try to achieve this request on every GOV_LIMITS.
Signed-off-by: Sai Gurrappadi <sgurrappadi@nvidia.com>
---
drivers/cpufreq/cpufreq_userspace.c | 20 ++++++++++++++++----
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/drivers/cpufreq/cpufreq_userspace.c
b/drivers/cpufreq/cpufreq_userspace.c
index 4d16f45..ae03ba7 100644
--- a/drivers/cpufreq/cpufreq_userspace.c
+++ b/drivers/cpufreq/cpufreq_userspace.c
@@ -18,6 +18,8 @@
#include <linux/module.h>
#include <linux/mutex.h>
+static DEFINE_PER_CPU(unsigned int, cpu_set_freq); /* CPU freq desired by
+ userspace */
static DEFINE_PER_CPU(unsigned int, cpu_is_managed);
static DEFINE_MUTEX(userspace_mutex);
@@ -38,6 +40,8 @@ static int cpufreq_set(struct cpufreq_policy *policy,
unsigned int freq)
if (!per_cpu(cpu_is_managed, policy->cpu))
goto err;
+ per_cpu(cpu_set_freq, policy->cpu) = freq;
+
ret = __cpufreq_driver_target(policy, freq, CPUFREQ_RELATION_L);
err:
Resending without the copyright headed appended..sorry! -Sai mutex_unlock(&userspace_mutex); @@ -62,6 +66,7 @@ static int cpufreq_governor_userspace(struct cpufreq_policy *policy, mutex_lock(&userspace_mutex); per_cpu(cpu_is_managed, cpu) = 1; + per_cpu(cpu_set_freq, cpu) = policy->cur; mutex_unlock(&userspace_mutex); break; case CPUFREQ_GOV_STOP: @@ -69,20 +74,27 @@ static int cpufreq_governor_userspace(struct cpufreq_policy *policy, mutex_lock(&userspace_mutex); per_cpu(cpu_is_managed, cpu) = 0; + per_cpu(cpu_set_freq, cpu) = 0; mutex_unlock(&userspace_mutex); break; case CPUFREQ_GOV_LIMITS: mutex_lock(&userspace_mutex); - pr_debug("limit event for cpu %u: %u - %u kHz, currently %u kHz\n", + pr_debug("limit event for cpu %u: %u - %u kHz, " + "currently %u kHz, last set to %u kHz\n", cpu, policy->min, policy->max, - policy->cur); + policy->cur, per_cpu(cpu_set_freq, cpu)); - if (policy->max < policy->cur) + if (policy->max < per_cpu(cpu_set_freq, cpu)) { __cpufreq_driver_target(policy, policy->max, CPUFREQ_RELATION_H); - else if (policy->min > policy->cur) + } else if (policy->min > per_cpu(cpu_set_freq, cpu)) { __cpufreq_driver_target(policy, policy->min, CPUFREQ_RELATION_L); + } else { + __cpufreq_driver_target(policy, + per_cpu(cpu_set_freq, cpu), + CPUFREQ_RELATION_L); + } mutex_unlock(&userspace_mutex); break; } -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html