diff mbox

[PATCHv2] omap2+: pm: cpufreq: Fix loops_per_jiffy calculation

Message ID 4E04DBF8.1050401@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Santosh Shilimkar June 24, 2011, 6:48 p.m. UTC
Russell,

On 6/24/2011 8:12 AM, Russell King - ARM Linux wrote:
> Right, thanks for the file.  Here's the patch.
>

[.....]

> Notice how we adjust _both_ the per-cpu loops_per_jiffy, and that we
> adjust them with reference to the initial values.
>
> If you adjust lpj with reference to the last, then you _will_ build up
> a progressively bigger and bigger error in the value over time.
>
Thanks Russell for the change. This change should fix the global
lpj for UP machine as well when build with SMP_ON_UP.

Can you have a look at below complete change which should
make the BOGOMIPS happy on all OMAP2PLUS machines. Generated
against Kevin's cpufreq branch.

url = 
git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-omap-pm.git 
pm-wip/cpufreq.

Just compile tested with UP and SMP OMAP builds. After your
review, I can give a test.

Regards
Santosh

 From 9a6154c0f68e39c4d1fbc4ef3fef5ce577ba87d4 Mon Sep 17 00:00:00 2001
From: Russell King <rmk+kernel@arm.linux.org.uk>
Date: Fri, 24 Jun 2011 10:51:17 -0700
Subject: [PATCH] OMAP2+: CPUfreq: update lpj with refernce value to 
avoid progressive error.

Adjust _both_ the per-cpu loops_per_jiffy and global lpj. Calibrate them
with with reference to the initial values to avoid a progressively
bigger and bigger error in the value over time.

While at this also re-use the notifiers for UP/SMP since on
UP machine or UP_ON_SMP policy->cpus mask would contain only
the one CPU.

Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
[santosh.shilimkar@ti.com: rebased against omap cpufreq upstream branch]
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
Cc: Kevin Hilman <khilman@ti.com>
---
  arch/arm/mach-omap2/omap2plus-cpufreq.c |   48 
+++++++++++++++++-------------
  1 files changed, 27 insertions(+), 21 deletions(-)

Comments

Sanjeev Premi June 25, 2011, 6:53 p.m. UTC | #1
> -----Original Message-----
> From: Shilimkar, Santosh 
> Sent: Saturday, June 25, 2011 12:18 AM
> To: Russell King - ARM Linux
> Cc: Premi, Sanjeev; linux-omap@vger.kernel.org; 
> linux-arm-kernel@lists.infradead.org; Hilman, Kevin
> Subject: Re: [PATCHv2] omap2+: pm: cpufreq: Fix 
> loops_per_jiffy calculation
> 
> Russell,
> 
> On 6/24/2011 8:12 AM, Russell King - ARM Linux wrote:
> > Right, thanks for the file.  Here's the patch.
> >
> 
> [.....]
> 
> > Notice how we adjust _both_ the per-cpu loops_per_jiffy, and that we
> > adjust them with reference to the initial values.
> >
> > If you adjust lpj with reference to the last, then you 
> _will_ build up
> > a progressively bigger and bigger error in the value over time.
> >
> Thanks Russell for the change. This change should fix the global
> lpj for UP machine as well when build with SMP_ON_UP.
> 
> Can you have a look at below complete change which should
> make the BOGOMIPS happy on all OMAP2PLUS machines. Generated
> against Kevin's cpufreq branch.
> 
> url = 
> git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-om
> ap-pm.git 
> pm-wip/cpufreq.
> 
> Just compile tested with UP and SMP OMAP builds. After your
> review, I can give a test.
> 
> Regards
> Santosh
> 
>  From 9a6154c0f68e39c4d1fbc4ef3fef5ce577ba87d4 Mon Sep 17 
> 00:00:00 2001
> From: Russell King <rmk+kernel@arm.linux.org.uk>
> Date: Fri, 24 Jun 2011 10:51:17 -0700
> Subject: [PATCH] OMAP2+: CPUfreq: update lpj with refernce value to 
> avoid progressive error.
> 
> Adjust _both_ the per-cpu loops_per_jiffy and global lpj. 
> Calibrate them
> with with reference to the initial values to avoid a progressively
> bigger and bigger error in the value over time.
> 
> While at this also re-use the notifiers for UP/SMP since on
> UP machine or UP_ON_SMP policy->cpus mask would contain only
> the one CPU.
> 
> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> [santosh.shilimkar@ti.com: rebased against omap cpufreq 
> upstream branch]
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> Cc: Kevin Hilman <khilman@ti.com>

[sp] I thought we were solving a problem - but this makes it
     look like race for addding sign-offs - which I am not
     interested in.

[snip]...[snip]
Russell King - ARM Linux June 25, 2011, 7:09 p.m. UTC | #2
On Sun, Jun 26, 2011 at 12:23:31AM +0530, Premi, Sanjeev wrote:
> [sp] I thought we were solving a problem - but this makes it
>      look like race for addding sign-offs - which I am not
>      interested in.

No, it's called packaging the patch up and getting it ready, putting it
out on the list for people to test and provide Tested-by's, acked-by's
etc.

Would you rather people sat on fixes doing nothing with them for a
month instead, watching broken -rc after broken -rc going by?
Sanjeev Premi June 27, 2011, 4:54 a.m. UTC | #3
> -----Original Message-----
> From: Russell King - ARM Linux [mailto:linux@arm.linux.org.uk] 
> Sent: Sunday, June 26, 2011 12:39 AM
> To: Premi, Sanjeev
> Cc: Shilimkar, Santosh; linux-omap@vger.kernel.org; 
> linux-arm-kernel@lists.infradead.org; Hilman, Kevin
> Subject: Re: [PATCHv2] omap2+: pm: cpufreq: Fix 
> loops_per_jiffy calculation
> 
> On Sun, Jun 26, 2011 at 12:23:31AM +0530, Premi, Sanjeev wrote:
> > [sp] I thought we were solving a problem - but this makes it
> >      look like race for addding sign-offs - which I am not
> >      interested in.
> 
> No, it's called packaging the patch up and getting it ready, 
> putting it
> out on the list for people to test and provide Tested-by's, acked-by's
> etc.

[sp] Agree.

> 
> Would you rather people sat on fixes doing nothing with them for a
> month instead, watching broken -rc after broken -rc going by?
>
[sp] The original patch was just few hours ago... not month(s).

~sanjeev
Russell King - ARM Linux June 27, 2011, 7:40 a.m. UTC | #4
On Mon, Jun 27, 2011 at 10:24:43AM +0530, Premi, Sanjeev wrote:
> > -----Original Message-----
> > From: Russell King - ARM Linux [mailto:linux@arm.linux.org.uk] 
> > Sent: Sunday, June 26, 2011 12:39 AM
> > To: Premi, Sanjeev
> > Cc: Shilimkar, Santosh; linux-omap@vger.kernel.org; 
> > linux-arm-kernel@lists.infradead.org; Hilman, Kevin
> > Subject: Re: [PATCHv2] omap2+: pm: cpufreq: Fix 
> > loops_per_jiffy calculation
> > 
> > On Sun, Jun 26, 2011 at 12:23:31AM +0530, Premi, Sanjeev wrote:
> > > [sp] I thought we were solving a problem - but this makes it
> > >      look like race for addding sign-offs - which I am not
> > >      interested in.
> > 
> > No, it's called packaging the patch up and getting it ready, 
> > putting it
> > out on the list for people to test and provide Tested-by's, acked-by's
> > etc.
> 
> [sp] Agree.
> 
> > 
> > Would you rather people sat on fixes doing nothing with them for a
> > month instead, watching broken -rc after broken -rc going by?
> >
> [sp] The original patch was just few hours ago... not month(s).

I fail to see what the problem is you're referring to.
diff mbox

Patch

diff --git a/arch/arm/mach-omap2/omap2plus-cpufreq.c 
b/arch/arm/mach-omap2/omap2plus-cpufreq.c
index 1f3b2e1..434698e 100644
--- a/arch/arm/mach-omap2/omap2plus-cpufreq.c
+++ b/arch/arm/mach-omap2/omap2plus-cpufreq.c
@@ -38,6 +38,16 @@ 

  #include <mach/hardware.h>

+#ifdef CONFIG_SMP
+struct lpj_info {
+	unsigned long	ref;
+	unsigned int	freq;
+};
+
+static DEFINE_PER_CPU(struct lpj_info, lpj_ref);
+static struct lpj_info global_lpj_ref;
+#endif
+
  static struct cpufreq_frequency_table *freq_table;
  static atomic_t freq_table_users = ATOMIC_INIT(0);
  static struct clk *mpu_clk;
@@ -96,11 +106,6 @@  static int omap_target(struct cpufreq_policy *policy,
  	if (freqs.old == freqs.new && policy->cur == freqs.new)
  		return ret;

-	if (!is_smp()) {
-		cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
-		goto set_freq;
-	}
-
  	/* notifiers */
  	for_each_cpu(i, policy->cpus) {
  		freqs.cpu = i;
@@ -114,19 +119,7 @@  set_freq:

  	ret = clk_set_rate(mpu_clk, freqs.new * 1000);

-	/*
-	 * Generic CPUFREQ driver jiffy update is under !SMP. So jiffies
-	 * won't get updated when UP machine cpufreq build with
-	 * CONFIG_SMP enabled. Below code is added only to manage that
-	 * scenario
-	 */
  	freqs.new = omap_getspeed(policy->cpu);
-	if (!is_smp()) {
-		loops_per_jiffy =
-			 cpufreq_scale(loops_per_jiffy, freqs.old, freqs.new);
-		cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
-		goto skip_lpj;
-	}

  #ifdef CONFIG_SMP
  	/*
@@ -134,10 +127,24 @@  set_freq:
  	 * cpufreq driver. So, update the per-CPU loops_per_jiffy value
  	 * on frequency transition. We need to update all dependent CPUs.
  	 */
-	for_each_cpu(i, policy->cpus)
+	for_each_cpu(i, policy->cpus) {
+		struct lpj_info *lpj = &per_cpu(lpj_ref, i);
+		if (!lpj->freq) {
+			lpj->ref = per_cpu(cpu_data, i).loops_per_jiffy;
+			lpj->freq = freqs.old;
+		}
+
  		per_cpu(cpu_data, i).loops_per_jiffy =
-			cpufreq_scale(per_cpu(cpu_data, i).loops_per_jiffy,
-					freqs.old, freqs.new);
+			cpufreq_scale(lpj->ref, lpj->freq, freqs.new);
+	}
+
+	/* And don't forget to adjust the global one */
+	if (!global_lpj_ref.freq) {
+		global_lpj_ref.ref = loops_per_jiffy;
+		global_lpj_ref.freq = freqs.old;
+	}
+	loops_per_jiffy = cpufreq_scale(global_lpj_ref.ref, global_lpj_ref.freq,
+					freqs.new);
  #endif

  	/* notifiers */
@@ -146,7 +153,6 @@  set_freq:
  		cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
  	}

-skip_lpj:
  	return ret;
  }