LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Pierre Ossman <drzeus-list@drzeus.cx>
To: Len Brown <lenb@kernel.org>,
	"Pallipadi, Venkatesh" <venkatesh.pallipadi@intel.com>
Cc: linux-pm@lists.linux-foundation.org, Pavel Machek <pavel@ucw.cz>,
	LKML <linux-kernel@vger.kernel.org>,
	Adam Belay <abelay@novell.com>, Andi Kleen <andi@firstfloor.org>,
	Lee Revell <rlrevell@joe-job.com>
Subject: Re: [linux-pm] [PATCH] cpuidle: avoid singing capacitors
Date: Fri, 14 Mar 2008 20:40:41 +0100	[thread overview]
Message-ID: <20080314204041.6e376568@mjolnir.drzeus.cx> (raw)
In-Reply-To: <20080313173437.5f00f70e@mjolnir.drzeus.cx>

On Thu, 13 Mar 2008 17:34:37 +0100
Pierre Ossman <drzeus-list@drzeus.cx> wrote:

> On Wed, 12 Mar 2008 15:11:17 -0400
> Len Brown <lenb@kernel.org> wrote:
> 
> > 
> > You'll see "desc" change if ACPI pulls a _CST change on you.
> > 
> 
> It does not. But my C3 desc looks like this:
> 
> state3/desc:ACPI FFH INTEL MWAIT 0x50
> 

I must have done something wrong. I now see a switch between C6 and C3
when I play with the AC cord.

On that theme, I've tested fiddling with the real C-states. I've added
a new max_hwcstate that makes ACPI downgrade MWAIT hints. I also made
some odd discoveries:

C3: More or less completely silent (I haven't tested it in a really
quiet environment yet).
C4: Constant noise
C5: Constant noise
C6: Intermittent noise

When I say constant, I mean that the noise is not generated as a result
of switching between modes (to any extent I can see at least). The
average time spent in C3 (as reported by Powertop) is over 200 ms. So
that would give a frequency of around 5 Hz, not a consistent tone of
several kHz.

Here's said patch. Please comment as I hope this can be merged:

diff --git a/arch/x86/kernel/acpi/cstate.c b/arch/x86/kernel/acpi/cstate.c
index 8ca3557..389ea8b 100644
--- a/arch/x86/kernel/acpi/cstate.c
+++ b/arch/x86/kernel/acpi/cstate.c
@@ -47,6 +47,9 @@ EXPORT_SYMBOL(acpi_processor_power_init_bm_check);
 
 /* The code below handles cstate entry with monitor-mwait pair on Intel*/
 
+static unsigned int max_hwcstate __read_mostly = -1;
+module_param(max_hwcstate, uint, 0644);
+
 struct cstate_entry {
 	struct {
 		unsigned int eax;
@@ -80,6 +83,7 @@ int acpi_processor_ffh_cstate_probe(unsigned int cpu,
 	unsigned int edx_part;
 	unsigned int cstate_type; /* C-state type and not ACPI C-state type */
 	unsigned int num_cstate_subtype;
+	unsigned int hint;
 
 	if (!cpu_cstate_entry || c->cpuid_level < CPUID_MWAIT_LEAF )
 		return -1;
@@ -100,16 +104,40 @@ int acpi_processor_ffh_cstate_probe(unsigned int cpu,
 	cpuid(CPUID_MWAIT_LEAF, &eax, &ebx, &ecx, &edx);
 
 	/* Check whether this particular cx_type (in CST) is supported or not */
-	cstate_type = (cx->address >> MWAIT_SUBSTATE_SIZE) + 1;
-	edx_part = edx >> (cstate_type * MWAIT_SUBSTATE_SIZE);
-	num_cstate_subtype = edx_part & MWAIT_SUBSTATE_MASK;
+	hint = cx->address;
+	for (;;) {
+		/* Compute main C-state */
+		cstate_type = (hint >> MWAIT_SUBSTATE_SIZE) + 1;
+
+		/* Determine number of sub-states for this C-state */
+		edx_part = edx >> (cstate_type * MWAIT_SUBSTATE_SIZE);
+		num_cstate_subtype = edx_part & MWAIT_SUBSTATE_MASK;
+
+		/* Check if it's within constraints, and supported */
+		if ((cstate_type > max_hwcstate) ||
+			(num_cstate_subtype <=
+				(hint & MWAIT_SUBSTATE_MASK))) {
+			/* Move down a C-state and drop sub-state */
+			cstate_type--;
+			hint = (cstate_type - 1) << MWAIT_SUBSTATE_SIZE;
+			/* Out of states, abort */
+			if (cstate_type == 0) {
+				retval = -1;
+				goto out;
+			}
+		} else {
+			break;
+		}
+	}
 
-	retval = 0;
-	if (num_cstate_subtype < (cx->address & MWAIT_SUBSTATE_MASK)) {
-		retval = -1;
-		goto out;
+	if (hint != cx->address) {
+		printk(KERN_DEBUG "ACPI: Downgrading hardware C%d to C%d\n",
+			(cx->address >> MWAIT_SUBSTATE_SIZE) + 1,
+			(hint >> MWAIT_SUBSTATE_SIZE) + 1);
 	}
 
+	retval = 0;
+
 	/* mwait ecx extensions INTERRUPT_BREAK should be supported for C2/C3 */
 	if (!(ecx & CPUID5_ECX_EXTENSIONS_SUPPORTED) ||
 	    !(ecx & CPUID5_ECX_INTERRUPT_BREAK)) {
@@ -119,15 +147,15 @@ int acpi_processor_ffh_cstate_probe(unsigned int cpu,
 	percpu_entry->states[cx->index].ecx = MWAIT_ECX_INTERRUPT_BREAK;
 
 	/* Use the hint in CST */
-	percpu_entry->states[cx->index].eax = cx->address;
+	percpu_entry->states[cx->index].eax = hint;
 
 	if (!mwait_supported[cstate_type]) {
 		mwait_supported[cstate_type] = 1;
 		printk(KERN_DEBUG "Monitor-Mwait will be used to enter C-%d "
 		       "state\n", cx->type);
 	}
-	snprintf(cx->desc, ACPI_CX_DESC_LEN, "ACPI FFH INTEL MWAIT 0x%x",
-		 cx->address);
+	snprintf(cx->desc, ACPI_CX_DESC_LEN, "ACPI FFH INTEL MWAIT 0x%x (0x%x)",
+		 hint, cx->address);
 
 out:
 	set_cpus_allowed(current, saved_mask);


Rgds
-- 
     -- Pierre Ossman

  Linux kernel, MMC maintainer        http://www.kernel.org
  PulseAudio, core developer          http://pulseaudio.org
  rdesktop, core developer          http://www.rdesktop.org

  parent reply	other threads:[~2008-03-14 19:42 UTC|newest]

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-02-29 18:38 [RFC][PATCH] " Pierre Ossman
2008-02-29 21:44 ` Lennart Sorensen
2008-03-01 12:31   ` Pierre Ossman
2008-03-01 13:40 ` Pierre Ossman
2008-03-02  2:27 ` Lee Revell
2008-03-02 14:17   ` Pierre Ossman
2008-03-03 12:36 ` Andi Kleen
2008-03-03 20:18 ` [PATCH] " Pierre Ossman
2008-03-03 20:46   ` Pavel Machek
2008-03-03 21:03     ` Pierre Ossman
2008-03-03 21:08       ` Pavel Machek
2008-03-03 21:14         ` Pallipadi, Venkatesh
2008-03-03 21:17           ` Pierre Ossman
2008-03-03 22:04             ` Pallipadi, Venkatesh
2008-03-03 23:05               ` [linux-pm] " Alan Stern
2008-03-03 23:10                 ` Andi Kleen
2008-03-04  4:00                   ` Dave Jones
2008-03-04  6:14                     ` Pierre Ossman
2008-03-04 17:19                       ` Pierre Ossman
2008-03-04 17:29                         ` Andi Kleen
2008-03-04 17:30                           ` Pierre Ossman
2008-03-04 17:43                             ` Andi Kleen
2008-03-04 18:04                               ` Pierre Ossman
2008-03-04 18:34                                 ` Andi Kleen
2008-03-05  6:04                                   ` Pierre Ossman
2008-03-05 15:48                                     ` Andi Kleen
2008-03-05 16:53                                       ` Pierre Ossman
2008-03-05 17:32                                         ` Andi Kleen
2008-03-04 19:01                         ` Pallipadi, Venkatesh
2008-03-05  6:02                           ` Pierre Ossman
2008-03-05  8:40                             ` Pierre Ossman
2008-03-05  9:03                               ` Pavel Machek
2008-03-05 13:42                                 ` Pierre Ossman
2008-03-05 13:47                                   ` Pavel Machek
2008-03-05 13:52                                     ` Pierre Ossman
2008-03-06  8:27                               ` Pierre Ossman
2008-03-09 14:16                                 ` Pierre Ossman
2008-03-09 18:19                                   ` Rafael J. Wysocki
2008-03-09 18:50                                   ` Alan Stern
2008-03-09 19:30                                   ` Henrique de Moraes Holschuh
2008-03-09 20:14                                     ` Pierre Ossman
2008-03-09 20:41                                       ` Henrique de Moraes Holschuh
2008-03-09 20:54                                         ` Henrique de Moraes Holschuh
2008-03-10 10:00                                   ` Pavel Machek
2008-03-10 12:49                                     ` Pierre Ossman
2008-03-10 13:04                                       ` Andi Kleen
2008-03-10 13:29                                         ` Pierre Ossman
2008-03-12 19:11                                       ` Len Brown
2008-03-13  8:10                                         ` Pavel Machek
2008-03-13 10:42                                           ` Andi Kleen
2008-03-14  4:13                                             ` Len Brown
2008-03-13 16:34                                         ` Pierre Ossman
2008-03-13 16:47                                           ` Pallipadi, Venkatesh
2008-03-13 17:44                                             ` Pierre Ossman
2008-03-13 17:49                                           ` Pierre Ossman
2008-03-14 19:40                                           ` Pierre Ossman [this message]
2008-03-14 21:15                                             ` Pallipadi, Venkatesh
2008-03-15  0:41                                               ` Pierre Ossman
2008-03-11  7:51                                   ` Pierre Ossman
2008-03-11 10:48                                     ` Andi Kleen
2008-03-11 15:20                                       ` Pierre Ossman
2008-03-11 17:31                                         ` Pierre Ossman
2008-03-12 19:17                                       ` Len Brown
2008-03-12 20:31                               ` Len Brown
2008-03-04  9:40                     ` Andi Kleen
2008-03-03 23:09               ` Andi Kleen

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=20080314204041.6e376568@mjolnir.drzeus.cx \
    --to=drzeus-list@drzeus.cx \
    --cc=abelay@novell.com \
    --cc=andi@firstfloor.org \
    --cc=lenb@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@lists.linux-foundation.org \
    --cc=pavel@ucw.cz \
    --cc=rlrevell@joe-job.com \
    --cc=venkatesh.pallipadi@intel.com \
    --subject='Re: [linux-pm] [PATCH] cpuidle: avoid singing capacitors' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).