Author: Thomas Renninger <[email protected]>
userspace governor need not to hold it's own cpufreq_policy,
better make use of the global core policy.
Also fixes a bug in case of frequency changes via _PPC.
Old min/max values have wrongly been passed to __cpufreq_driver_target()
(kind of buffered) and when max freq was available again, only the old
max(normally lowest freq) was still active.
cpufreq_userspace.c | 36 +++++++++++++++++-------------------
1 files changed, 17 insertions(+), 19 deletions(-)
Index: linux-2.6.15/drivers/cpufreq/cpufreq_userspace.c
===================================================================
--- linux-2.6.15.orig/drivers/cpufreq/cpufreq_userspace.c 2006-01-03 04:21:10.000000000 +0100
+++ linux-2.6.15/drivers/cpufreq/cpufreq_userspace.c 2006-01-20 11:41:27.000000000 +0100
@@ -33,7 +33,6 @@
static unsigned int cpu_cur_freq[NR_CPUS]; /* current CPU freq */
static unsigned int cpu_set_freq[NR_CPUS]; /* CPU freq desired by userspace */
static unsigned int cpu_is_managed[NR_CPUS];
-static struct cpufreq_policy current_policy[NR_CPUS];
static DECLARE_MUTEX (userspace_sem);
@@ -64,22 +63,22 @@
*
* Sets the CPU frequency to freq.
*/
-static int cpufreq_set(unsigned int freq, unsigned int cpu)
+static int cpufreq_set(unsigned int freq, struct cpufreq_policy *policy)
{
int ret = -EINVAL;
- dprintk("cpufreq_set for cpu %u, freq %u kHz\n", cpu, freq);
+ dprintk("cpufreq_set for cpu %u, freq %u kHz\n", policy->cpu, freq);
down(&userspace_sem);
- if (!cpu_is_managed[cpu])
+ if (!cpu_is_managed[policy->cpu])
goto err;
- cpu_set_freq[cpu] = freq;
+ cpu_set_freq[policy->cpu] = freq;
- if (freq < cpu_min_freq[cpu])
- freq = cpu_min_freq[cpu];
- if (freq > cpu_max_freq[cpu])
- freq = cpu_max_freq[cpu];
+ if (freq < cpu_min_freq[policy->cpu])
+ freq = cpu_min_freq[policy->cpu];
+ if (freq > cpu_max_freq[policy->cpu])
+ freq = cpu_max_freq[policy->cpu];
/*
* We're safe from concurrent calls to ->target() here
@@ -88,8 +87,7 @@
* A: cpufreq_set (lock userspace_sem) -> cpufreq_driver_target(lock policy->lock)
* B: cpufreq_set_policy(lock policy->lock) -> __cpufreq_governor -> cpufreq_governor_userspace (lock userspace_sem)
*/
- ret = __cpufreq_driver_target(¤t_policy[cpu], freq,
- CPUFREQ_RELATION_L);
+ ret = __cpufreq_driver_target(policy, freq, CPUFREQ_RELATION_L);
err:
up(&userspace_sem);
@@ -113,7 +111,7 @@
if (ret != 1)
return -EINVAL;
- cpufreq_set(freq, policy->cpu);
+ cpufreq_set(freq, policy);
return count;
}
@@ -141,7 +139,6 @@
cpu_cur_freq[cpu] = policy->cur;
cpu_set_freq[cpu] = policy->cur;
sysfs_create_file (&policy->kobj, &freq_attr_scaling_setspeed.attr);
- memcpy (¤t_policy[cpu], policy, sizeof(struct cpufreq_policy));
dprintk("managing cpu %u started (%u - %u kHz, currently %u kHz)\n", cpu, cpu_min_freq[cpu], cpu_max_freq[cpu], cpu_cur_freq[cpu]);
up(&userspace_sem);
break;
@@ -161,16 +158,17 @@
cpu_max_freq[cpu] = policy->max;
dprintk("limit event for cpu %u: %u - %u kHz, currently %u kHz, last set to %u kHz\n", cpu, cpu_min_freq[cpu], cpu_max_freq[cpu], cpu_cur_freq[cpu], cpu_set_freq[cpu]);
if (policy->max < cpu_set_freq[cpu]) {
- __cpufreq_driver_target(¤t_policy[cpu], policy->max,
- CPUFREQ_RELATION_H);
+ if (!__cpufreq_driver_target(policy, policy->max,
+ CPUFREQ_RELATION_H))
+ cpu_cur_freq[cpu] = policy->max;
} else if (policy->min > cpu_set_freq[cpu]) {
- __cpufreq_driver_target(¤t_policy[cpu], policy->min,
- CPUFREQ_RELATION_L);
+ if (!__cpufreq_driver_target(policy, policy->min,
+ CPUFREQ_RELATION_L))
+ cpu_cur_freq[cpu] = policy->min;
} else {
- __cpufreq_driver_target(¤t_policy[cpu], cpu_set_freq[cpu],
+ __cpufreq_driver_target(policy, cpu_set_freq[cpu],
CPUFREQ_RELATION_L);
}
- memcpy (¤t_policy[cpu], policy, sizeof(struct cpufreq_policy));
up(&userspace_sem);
break;
}
On Tue, 24 Jan 2006, Thomas Renninger wrote:
> and the second one fixing the usergovernor for these machines...
>
The indentation looks bad/wrong in both patches... but I can't
quote it since they were attachments.
--
~Randy
Thanks for identifying the issues and sendint these patches Thomas.
Patch 1 looks clean. New lines seem to contain spaces instead of tabs.
The same issue is there in patch 2 as well. Can you resent it with
indentation fixed.
Patch 2 I am concenred with following hunk.
@@ -161,16 +158,17 @@
cpu_max_freq[cpu] = policy->max;
dprintk("limit event for cpu %u: %u - %u kHz, currently
%u kHz, last set to %u kHz\n", cpu, cpu_min_freq[cpu],
cpu_max_freq[cpu], cpu_cur_freq[cpu], cpu_set_freq[cpu]);
if (policy->max < cpu_set_freq[cpu]) {
- __cpufreq_driver_target(¤t_policy[cpu],
policy->max,
- CPUFREQ_RELATION_H);
+ if (!__cpufreq_driver_target(policy, policy->max,
+ CPUFREQ_RELATION_H))
+ cpu_cur_freq[cpu] = policy->max;
Should this me cpu_cur_freq[cpu] = policy->cur instead. As the max
setting may not be supported by the driver, it might have set some
closer available freq
Same comment for below two driver target calls as well.
} else if (policy->min > cpu_set_freq[cpu]) {
- __cpufreq_driver_target(¤t_policy[cpu],
policy->min,
- CPUFREQ_RELATION_L);
+ if (!__cpufreq_driver_target(policy, policy->min,
+ CPUFREQ_RELATION_L))
+ cpu_cur_freq[cpu] = policy->min;
} else {
- __cpufreq_driver_target(¤t_policy[cpu],
cpu_set_freq[cpu],
+ __cpufreq_driver_target(policy, cpu_set_freq[cpu],
CPUFREQ_RELATION_L);
}
Thx,
Venki
>-----Original Message-----
>From: Thomas Renninger [mailto:[email protected]]
>Sent: Tuesday, January 24, 2006 8:18 AM
>To: cpufreq@http://www.linux.org.uk
>Cc: Pallipadi, Venkatesh; Dominik Brodowski; Kernel Mailing List
>Subject: [PATCH 2/2] _PPC frequency change issues
>
>and the second one fixing the usergovernor for these machines...
>
On Tuesday 24 January 2006 18:06, Pallipadi, Venkatesh wrote:
> Thanks for identifying the issues and sendint these patches Thomas.
>
> Patch 1 looks clean. New lines seem to contain spaces instead of tabs.
> The same issue is there in patch 2 as well. Can you resent it with
> indentation fixed.
>
> Patch 2 I am concenred with following hunk.
>
> @@ -161,16 +158,17 @@
> cpu_max_freq[cpu] = policy->max;
> dprintk("limit event for cpu %u: %u - %u kHz, currently
> %u kHz, last set to %u kHz\n", cpu, cpu_min_freq[cpu],
> cpu_max_freq[cpu], cpu_cur_freq[cpu], cpu_set_freq[cpu]);
> if (policy->max < cpu_set_freq[cpu]) {
> - __cpufreq_driver_target(¤t_policy[cpu],
> policy->max,
> - CPUFREQ_RELATION_H);
> + if (!__cpufreq_driver_target(policy, policy->max,
> + CPUFREQ_RELATION_H))
> + cpu_cur_freq[cpu] = policy->max;
>
> Should this me cpu_cur_freq[cpu] = policy->cur instead. As the max
> setting may not be supported by the driver, it might have set some
> closer available freq
>
> Same comment for below two driver target calls as well.
Ok, I all (cpu_max/min/cur_freq) assigned it after setting, this should be OK?
Seems as if I had some wrong indentation offset set and had not checked
the patch output itself, sorry about that.
Tell me if you still see any problems ...
Author: Thomas Renninger <[email protected]>
userspace governor need not to hold it's own cpufreq_policy,
better make use of the global core policy.
Also fixes a bug in case of frequency changes via _PPC.
Old min/max values have wrongly been passed to __cpufreq_driver_target()
(kind of buffered) and when max freq was available again, only the old
max(normally lowest freq) was still active.
cpufreq_userspace.c | 52
+++++++++++++++++++++++++++-------------------------
1 files changed, 27 insertions(+), 25 deletions(-)
Index: linux-2.6.15/drivers/cpufreq/cpufreq_userspace.c
===================================================================
--- linux-2.6.15.orig/drivers/cpufreq/cpufreq_userspace.c
+++ linux-2.6.15/drivers/cpufreq/cpufreq_userspace.c
@@ -33,7 +33,6 @@ static unsigned int cpu_min_freq[NR_CPUS
static unsigned int cpu_cur_freq[NR_CPUS]; /* current CPU freq */
static unsigned int cpu_set_freq[NR_CPUS]; /* CPU freq desired by userspace
*/
static unsigned int cpu_is_managed[NR_CPUS];
-static struct cpufreq_policy current_policy[NR_CPUS];
static DECLARE_MUTEX (userspace_sem);
@@ -64,22 +63,22 @@ static struct notifier_block userspace_c
*
* Sets the CPU frequency to freq.
*/
-static int cpufreq_set(unsigned int freq, unsigned int cpu)
+static int cpufreq_set(unsigned int freq, struct cpufreq_policy *policy)
{
int ret = -EINVAL;
- dprintk("cpufreq_set for cpu %u, freq %u kHz\n", cpu, freq);
+ dprintk("cpufreq_set for cpu %u, freq %u kHz\n", policy->cpu, freq);
down(&userspace_sem);
- if (!cpu_is_managed[cpu])
+ if (!cpu_is_managed[policy->cpu])
goto err;
- cpu_set_freq[cpu] = freq;
+ cpu_set_freq[policy->cpu] = freq;
- if (freq < cpu_min_freq[cpu])
- freq = cpu_min_freq[cpu];
- if (freq > cpu_max_freq[cpu])
- freq = cpu_max_freq[cpu];
+ if (freq < cpu_min_freq[policy->cpu])
+ freq = cpu_min_freq[policy->cpu];
+ if (freq > cpu_max_freq[policy->cpu])
+ freq = cpu_max_freq[policy->cpu];
/*
* We're safe from concurrent calls to ->target() here
@@ -88,8 +87,7 @@ static int cpufreq_set(unsigned int freq
* A: cpufreq_set (lock userspace_sem) -> cpufreq_driver_target(lock
policy->lock)
* B: cpufreq_set_policy(lock policy->lock) -> __cpufreq_governor ->
cpufreq_governor_userspace (lock userspace_sem)
*/
- ret = __cpufreq_driver_target(¤t_policy[cpu], freq,
- CPUFREQ_RELATION_L);
+ ret = __cpufreq_driver_target(policy, freq, CPUFREQ_RELATION_L);
err:
up(&userspace_sem);
@@ -113,7 +111,7 @@ store_speed (struct cpufreq_policy *poli
if (ret != 1)
return -EINVAL;
- cpufreq_set(freq, policy->cpu);
+ cpufreq_set(freq, policy);
return count;
}
@@ -141,7 +139,6 @@ static int cpufreq_governor_userspace(st
cpu_cur_freq[cpu] = policy->cur;
cpu_set_freq[cpu] = policy->cur;
sysfs_create_file (&policy->kobj, &freq_attr_scaling_setspeed.attr);
- memcpy (¤t_policy[cpu], policy, sizeof(struct cpufreq_policy));
dprintk("managing cpu %u started (%u - %u kHz, currently %u kHz)\n", cpu,
cpu_min_freq[cpu], cpu_max_freq[cpu], cpu_cur_freq[cpu]);
up(&userspace_sem);
break;
@@ -157,20 +154,25 @@ static int cpufreq_governor_userspace(st
break;
case CPUFREQ_GOV_LIMITS:
down(&userspace_sem);
- cpu_min_freq[cpu] = policy->min;
- cpu_max_freq[cpu] = policy->max;
- dprintk("limit event for cpu %u: %u - %u kHz, currently %u kHz, last set to
%u kHz\n", cpu, cpu_min_freq[cpu], cpu_max_freq[cpu], cpu_cur_freq[cpu],
cpu_set_freq[cpu]);
+ dprintk("limit event for cpu %u: %u - %u kHz,"
+ "currently %u kHz, last set to %u kHz\n",
+ cpu, policy->min, policy->max,
+ cpu_cur_freq[cpu], cpu_set_freq[cpu]);
if (policy->max < cpu_set_freq[cpu]) {
- __cpufreq_driver_target(¤t_policy[cpu], policy->max,
- CPUFREQ_RELATION_H);
- } else if (policy->min > cpu_set_freq[cpu]) {
- __cpufreq_driver_target(¤t_policy[cpu], policy->min,
- CPUFREQ_RELATION_L);
- } else {
- __cpufreq_driver_target(¤t_policy[cpu], cpu_set_freq[cpu],
- CPUFREQ_RELATION_L);
+ __cpufreq_driver_target(policy, policy->max,
+ CPUFREQ_RELATION_H);
+ }
+ else if (policy->min > cpu_set_freq[cpu]) {
+ __cpufreq_driver_target(policy, policy->min,
+ CPUFREQ_RELATION_L);
+ }
+ else {
+ __cpufreq_driver_target(policy, cpu_set_freq[cpu],
+ CPUFREQ_RELATION_L);
}
- memcpy (¤t_policy[cpu], policy, sizeof(struct cpufreq_policy));
+ cpu_min_freq[cpu] = policy->min;
+ cpu_max_freq[cpu] = policy->max;
+ cpu_cur_freq[cpu] = policy->cur;
up(&userspace_sem);
break;
}