All of lore.kernel.org
 help / color / mirror / Atom feed
From: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
To: davej@redhat.com
Cc: linux@dominikbrodowski.de, cpufreq@www.linux.org.uk
Subject: [PATCH] speedstep-centrino: Avoid returning zero freq on transient MSR values
Date: Thu, 9 Dec 2004 17:24:36 -0800	[thread overview]
Message-ID: <20041209172436.A3337@unix-os.sc.intel.com> (raw)




On some CPUs, we can see transient MSR values (which are not present in _PSS) in
IA32_PERF_STATUS MSR, while CPU is doing some automatic P-state transition 
(like TM2). 
Current code will return frequency as 0 in such cases. Fix it by retrying the 
get after a delay and use lowest possible frequency as the current frequency 
in worst case.

Thanks to Matt Domsch for identifying and root-causing this failure.

Signed-off-by: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>

--- linux-2.6.10-rc3/arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c.org	2004-12-07 18:55:57.000000000 -0800
+++ linux-2.6.10-rc3/arch/i386/kernel/cpu/cpufreq/speedstep-centrino.c	2004-12-09 16:18:30.000000000 -0800
@@ -22,6 +22,8 @@
 #include <linux/init.h>
 #include <linux/cpufreq.h>
 #include <linux/config.h>
+#include <linux/delay.h>
+#include <linux/compiler.h>
 
 #ifdef CONFIG_X86_SPEEDSTEP_CENTRINO_ACPI
 #include <linux/acpi.h>
@@ -277,7 +279,7 @@ static int centrino_verify_cpu_id(const 
 }
 
 /* To be called only after centrino_model is initialized */
-static unsigned extract_clock(unsigned msr, unsigned int cpu)
+static unsigned extract_clock(unsigned msr, unsigned int cpu, int failsafe)
 {
 	int i;
 
@@ -299,15 +301,19 @@ static unsigned extract_clock(unsigned m
 	msr &= 0xffff;
 	for (i=0;centrino_model[cpu]->op_points[i].frequency != CPUFREQ_TABLE_END; i++) {
 		if (msr == centrino_model[cpu]->op_points[i].index)
-		return centrino_model[cpu]->op_points[i].frequency;
+			return centrino_model[cpu]->op_points[i].frequency;
 	}
-	return 0;
+	if (failsafe)
+		return centrino_model[cpu]->op_points[i-1].frequency;
+	else
+		return 0;
 }
 
 /* Return the current CPU frequency in kHz */
 static unsigned int get_cur_freq(unsigned int cpu)
 {
 	unsigned l, h;
+	unsigned clock_freq;
 	cpumask_t saved_mask;
 
 	saved_mask = current->cpus_allowed;
@@ -316,8 +322,34 @@ static unsigned int get_cur_freq(unsigne
 		return 0;
 
 	rdmsr(MSR_IA32_PERF_STATUS, l, h);
+	clock_freq = extract_clock(l, cpu, 0);
+
+	if (unlikely(clock_freq == 0)) {
+		/*
+		 * On some CPUs, we can see transient MSR values (which are
+		 * not present in _PSS), while CPU is doing some automatic
+		 * P-state transition (like TM2). Allow CPU to stabilize at
+		 * some freq and retry.
+		 * If we continue to see transients for long time, just return
+		 * the lowest possible frequency as best guess.
+		 */
+		int retries = 0;
+#define MAX_EXTRACT_CLOCK_RETRIES	5
+		while (clock_freq == 0 && retries < MAX_EXTRACT_CLOCK_RETRIES) {
+			udelay(100);
+			retries++;
+			rdmsr(MSR_IA32_PERF_STATUS, l, h);
+			clock_freq = extract_clock(l, cpu, 0);
+		}
+
+		if (clock_freq == 0) {
+			rdmsr(MSR_IA32_PERF_STATUS, l, h);
+			clock_freq = extract_clock(l, cpu, 1);
+		}
+	}
+
 	set_cpus_allowed(current, saved_mask);
-	return extract_clock(l, cpu);
+	return clock_freq;
 }
 
 
@@ -426,10 +458,10 @@ static int centrino_cpu_init_acpi(struct
 			continue;
 		}
 		
-		if (extract_clock(centrino_model[cpu]->op_points[i].index, cpu) !=
+		if (extract_clock(centrino_model[cpu]->op_points[i].index, cpu, 0) !=
 		    (centrino_model[cpu]->op_points[i].frequency)) {
 			dprintk("Invalid encoded frequency (%u vs. %u)\n",
-				extract_clock(centrino_model[cpu]->op_points[i].index, cpu),
+				extract_clock(centrino_model[cpu]->op_points[i].index, cpu, 0),
 				centrino_model[cpu]->op_points[i].frequency);
 			result = -EINVAL;
 			goto err_kfree_all;
@@ -605,8 +637,8 @@ static int centrino_target (struct cpufr
 	}
 
 	freqs.cpu = cpu;
-	freqs.old = extract_clock(oldmsr, cpu);
-	freqs.new = extract_clock(msr, cpu);
+	freqs.old = extract_clock(oldmsr, cpu, 0);
+	freqs.new = extract_clock(msr, cpu, 0);
 
 	dprintk("target=%dkHz old=%d new=%d msr=%04x\n",
 		target_freq, freqs.old, freqs.new, msr);

             reply	other threads:[~2004-12-10  1:24 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2004-12-10  1:24 Venkatesh Pallipadi [this message]
2004-12-10  8:14 ` [PATCH] speedstep-centrino: Avoid returning zero freq on transient MSR values Dominik Brodowski
2004-12-10 10:11 ` Jeremy Fitzhardinge
2004-12-10 14:47   ` Dominik Brodowski
  -- strict thread matches above, loose matches on Subject: below --
2004-12-10 18:37 Pallipadi, Venkatesh
2004-12-10 21:16 ` Jeremy Fitzhardinge

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=20041209172436.A3337@unix-os.sc.intel.com \
    --to=venkatesh.pallipadi@intel.com \
    --cc=cpufreq@www.linux.org.uk \
    --cc=davej@redhat.com \
    --cc=linux@dominikbrodowski.de \
    /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.