All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dominik Brodowski <linux@brodo.de>
To: Dave Jones <davej@redhat.com>
Cc: cpufreq@www.linux.org.uk
Subject: Re: [PATCH 7/8] latency must be in _nano_seconds
Date: Thu, 20 Nov 2003 19:31:33 +0100	[thread overview]
Message-ID: <20031120183133.GA4425@brodo.de> (raw)
In-Reply-To: <20031119190725.GA5653@redhat.com>

On Wed, Nov 19, 2003 at 07:07:25PM +0000, Dave Jones wrote:
> On Wed, Nov 19, 2003 at 07:33:30PM +0100, Dominik Brodowski wrote:
> 
>  >  	return cpufreq_frequency_table_cpuinfo(policy, &p4clockmod_table[0]);
>  > diff -ruN linux-original/arch/i386/kernel/cpu/cpufreq/powernow-k7.c linux/arch/i386/kernel/cpu/cpufreq/powernow-k7.c
>  > --- linux-original/arch/i386/kernel/cpu/cpufreq/powernow-k7.c	2003-11-19 17:06:10.571868800 +0100
>  > +++ linux/arch/i386/kernel/cpu/cpufreq/powernow-k7.c	2003-11-19 18:48:10.150551432 +0100
>  > @@ -386,7 +386,7 @@
>  >  				minimum_speed, maximum_speed);
>  >  
>  >  	policy->governor = CPUFREQ_DEFAULT_GOVERNOR;
>  > -	policy->cpuinfo.transition_latency = latency;
>  > +	policy->cpuinfo.transition_latency = latency * 20;
>  >  	policy->cur = maximum_speed;
> 
> Can we at least get a comment explaining where '20' comes from
> when we add magic numbers like this? I know, and you know, but
> it's a little non-obvious, and in six months time, I'll probably
> have forgotten too 8)

Comment added.
 
>  > diff -ruN linux-original/arch/i386/kernel/cpu/cpufreq/powernow-k8.c linux/arch/i386/kernel/cpu/cpufreq/powernow-k8.c
>  > --- linux-original/arch/i386/kernel/cpu/cpufreq/powernow-k8.c	2003-11-19 17:07:58.537455536 +0100
>  > +++ linux/arch/i386/kernel/cpu/cpufreq/powernow-k8.c	2003-11-19 18:46:57.031667192 +0100
>  > @@ -959,8 +959,8 @@
>  >  	pol->governor = CPUFREQ_DEFAULT_GOVERNOR;
>  >  
>  >  	/* Take a crude guess here. */
>  > -	pol->cpuinfo.transition_latency = ((rvo + 8) * vstable * VST_UNITS_20US)
>  > -	    + (3 * (1 << irt) * 10);
>  > +	pol->cpuinfo.transition_latency = (((rvo + 8) * vstable * VST_UNITS_20US)
>  > +	    + (3 * (1 << irt) * 10)) * 1000;
> 
> Ditto. This one moreso, as its quite complex to begin with.

What Paul Devriendt did here I do not know, but I added a comment for the
factor "1000"...

	Dominik

Even though the core stated that cpuinfo.transition_latency needs to be
in 10^(-9) s, hardly any driver set it to nanoseconds but to microseconds.
So, fix up the drivers.

 arch/arm/mach-integrator/cpu.c             |    2 +-
 arch/i386/kernel/cpu/cpufreq/acpi.c        |    4 ++--
 arch/i386/kernel/cpu/cpufreq/p4-clockmod.c |    2 +-
 arch/i386/kernel/cpu/cpufreq/powernow-k7.c |    6 +++++-
 arch/i386/kernel/cpu/cpufreq/powernow-k8.c |    7 ++++---
 include/linux/cpufreq.h                    |    2 +-
 6 files changed, 14 insertions(+), 9 deletions(-)

diff -ruN linux-original/arch/arm/mach-integrator/cpu.c linux/arch/arm/mach-integrator/cpu.c
--- linux-original/arch/arm/mach-integrator/cpu.c	2003-11-20 19:17:28.489660360 +0100
+++ linux/arch/arm/mach-integrator/cpu.c	2003-11-20 19:17:34.311775264 +0100
@@ -172,7 +172,7 @@
 	policy->governor = CPUFREQ_DEFAULT_GOVERNOR;
 	policy->cpuinfo.max_freq = 160000;
 	policy->cpuinfo.min_freq = 12000;
-	policy->cpuinfo.transition_latency = 1000; /* 1 ms, assumed */
+	policy->cpuinfo.transition_latency = 1000000; /* 1 ms, assumed */
 	policy->cur = policy->min = policy->max =
 		icst525_khz(&cclk_params, vco); /* current freq */
 
diff -ruN linux-original/arch/i386/kernel/cpu/cpufreq/acpi.c linux/arch/i386/kernel/cpu/cpufreq/acpi.c
--- linux-original/arch/i386/kernel/cpu/cpufreq/acpi.c	2003-11-20 19:17:28.510657168 +0100
+++ linux/arch/i386/kernel/cpu/cpufreq/acpi.c	2003-11-20 19:17:34.312775112 +0100
@@ -578,8 +578,8 @@
 	/* detect transition latency */
 	policy->cpuinfo.transition_latency = 0;
 	for (i=0;i<perf->state_count;i++) {
-		if (perf->states[i].transition_latency > policy->cpuinfo.transition_latency)
-			policy->cpuinfo.transition_latency = perf->states[i].transition_latency;
+		if ((perf->states[i].transition_latency * 1000) > policy->cpuinfo.transition_latency)
+			policy->cpuinfo.transition_latency = perf->states[i].transition_latency * 1000;
 	}
 	policy->governor = CPUFREQ_DEFAULT_GOVERNOR;
 	policy->cur = perf->states[pr->limit.state.px].core_frequency * 1000;
diff -ruN linux-original/arch/i386/kernel/cpu/cpufreq/p4-clockmod.c linux/arch/i386/kernel/cpu/cpufreq/p4-clockmod.c
--- linux-original/arch/i386/kernel/cpu/cpufreq/p4-clockmod.c	2003-11-20 19:17:28.540652608 +0100
+++ linux/arch/i386/kernel/cpu/cpufreq/p4-clockmod.c	2003-11-20 19:17:34.312775112 +0100
@@ -255,7 +255,7 @@
 	
 	/* cpuinfo and default policy values */
 	policy->governor = CPUFREQ_DEFAULT_GOVERNOR;
-	policy->cpuinfo.transition_latency = 1000;
+	policy->cpuinfo.transition_latency = 1000000; /* assumed */
 	policy->cur = stock_freq;
 
 	return cpufreq_frequency_table_cpuinfo(policy, &p4clockmod_table[0]);
diff -ruN linux-original/arch/i386/kernel/cpu/cpufreq/powernow-k7.c linux/arch/i386/kernel/cpu/cpufreq/powernow-k7.c
--- linux-original/arch/i386/kernel/cpu/cpufreq/powernow-k7.c	2003-11-20 19:17:28.541652456 +0100
+++ linux/arch/i386/kernel/cpu/cpufreq/powernow-k7.c	2003-11-20 19:20:35.230271472 +0100
@@ -386,7 +386,11 @@
 				minimum_speed, maximum_speed);
 
 	policy->governor = CPUFREQ_DEFAULT_GOVERNOR;
-	policy->cpuinfo.transition_latency = latency;
+
+	/* latency is in 10 ns (look for SGTC above) for each VID
+	 * and FID transition, so multiply that value with 20 */
+	policy->cpuinfo.transition_latency = latency * 20;
+
 	policy->cur = maximum_speed;
 
 	return cpufreq_frequency_table_cpuinfo(policy, powernow_table);
diff -ruN linux-original/arch/i386/kernel/cpu/cpufreq/powernow-k8.c linux/arch/i386/kernel/cpu/cpufreq/powernow-k8.c
--- linux-original/arch/i386/kernel/cpu/cpufreq/powernow-k8.c	2003-11-20 19:17:28.591644856 +0100
+++ linux/arch/i386/kernel/cpu/cpufreq/powernow-k8.c	2003-11-20 19:18:45.550945264 +0100
@@ -958,9 +958,10 @@
 
 	pol->governor = CPUFREQ_DEFAULT_GOVERNOR;
 
-	/* Take a crude guess here. */
-	pol->cpuinfo.transition_latency = ((rvo + 8) * vstable * VST_UNITS_20US)
-	    + (3 * (1 << irt) * 10);
+	/* Take a crude guess here. 
+	 * That guess was in microseconds, so multply with 1000 */
+	pol->cpuinfo.transition_latency = (((rvo + 8) * vstable * VST_UNITS_20US)
+	    + (3 * (1 << irt) * 10)) * 1000;
 
 	if (query_current_values_with_pending_wait())
 		return -EIO;
diff -ruN linux-original/include/linux/cpufreq.h linux/include/linux/cpufreq.h
--- linux-original/include/linux/cpufreq.h	2003-11-20 19:17:28.695629048 +0100
+++ linux/include/linux/cpufreq.h	2003-11-20 19:17:34.314774808 +0100
@@ -57,7 +57,7 @@
 struct cpufreq_cpuinfo {
 	unsigned int		max_freq;
 	unsigned int		min_freq;
-	unsigned int		transition_latency; /* in 10^(-9) s */
+	unsigned int		transition_latency; /* in 10^(-9) s = nanoseconds */
 };
 
 struct cpufreq_real_policy {

      reply	other threads:[~2003-11-20 18:31 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2003-11-19 18:33 [PATCH 7/8] latency must be in _nano_seconds Dominik Brodowski
2003-11-19 19:07 ` Dave Jones
2003-11-20 18:31   ` Dominik Brodowski [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20031120183133.GA4425@brodo.de \
    --to=linux@brodo.de \
    --cc=cpufreq@www.linux.org.uk \
    --cc=davej@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.