[PATCH] Remove p4_clockmode driver

[PATCH] Remove p4_clockmode driver

[Thomas Renninger]

Remove p4_clockmode driver

The driver is doing throttling which is supposed to be done through another
ACPI interface. If both interfaces are used, the machine may get very slow.
Remove this driver which should never be used.

Signed-off-by: Thomas Renninger <trenn@suse.de>


---
 arch/x86/kernel/cpu/cpufreq/Kconfig       |   20 -
 arch/x86/kernel/cpu/cpufreq/Makefile      |    1 
 arch/x86/kernel/cpu/cpufreq/p4-clockmod.c |  315 ------------------------------
 3 files changed, 336 deletions(-)

Index: linux-acpi-2.6_video_native_vs_vendor/arch/x86/kernel/cpu/cpufreq/Kconfig
===================================================================
--- linux-acpi-2.6_video_native_vs_vendor.orig/arch/x86/kernel/cpu/cpufreq/Kconfig
+++ linux-acpi-2.6_video_native_vs_vendor/arch/x86/kernel/cpu/cpufreq/Kconfig
@@ -179,26 +179,6 @@ config X86_SPEEDSTEP_SMI
 
 	  If in doubt, say N.
 
-config X86_P4_CLOCKMOD
-	tristate "Intel Pentium 4 clock modulation"
-	select CPU_FREQ_TABLE
-	help
-	  This adds the CPUFreq driver for Intel Pentium 4 / XEON
-	  processors.  When enabled it will lower CPU temperature by skipping
-	  clocks.
-
-	  This driver should be only used in exceptional
-	  circumstances when very low power is needed because it causes severe
-	  slowdowns and noticeable latencies.  Normally Speedstep should be used
-	  instead.
-
-	  To compile this driver as a module, choose M here: the
-	  module will be called p4-clockmod.
-
-	  For details, take a look at <file:Documentation/cpu-freq/>.
-
-	  Unless you are absolutely sure say N.
-
 config X86_CPUFREQ_NFORCE2
 	tristate "nVidia nForce2 FSB changing"
 	depends on X86_32 && EXPERIMENTAL
Index: linux-acpi-2.6_video_native_vs_vendor/arch/x86/kernel/cpu/cpufreq/Makefile
===================================================================
--- linux-acpi-2.6_video_native_vs_vendor.orig/arch/x86/kernel/cpu/cpufreq/Makefile
+++ linux-acpi-2.6_video_native_vs_vendor/arch/x86/kernel/cpu/cpufreq/Makefile
@@ -12,5 +12,4 @@ obj-$(CONFIG_X86_SPEEDSTEP_LIB)		+= spee
 obj-$(CONFIG_X86_SPEEDSTEP_SMI)		+= speedstep-smi.o
 obj-$(CONFIG_X86_ACPI_CPUFREQ)		+= acpi-cpufreq.o
 obj-$(CONFIG_X86_SPEEDSTEP_CENTRINO)	+= speedstep-centrino.o
-obj-$(CONFIG_X86_P4_CLOCKMOD)		+= p4-clockmod.o
 obj-$(CONFIG_X86_CPUFREQ_NFORCE2)	+= cpufreq-nforce2.o
Index: linux-acpi-2.6_video_native_vs_vendor/arch/x86/kernel/cpu/cpufreq/p4-clockmod.c
===================================================================
--- linux-acpi-2.6_video_native_vs_vendor.orig/arch/x86/kernel/cpu/cpufreq/p4-clockmod.c
+++ /dev/null
@@ -1,315 +0,0 @@
-/*
- *	Pentium 4/Xeon CPU on demand clock modulation/speed scaling
- *	(C) 2002 - 2003 Dominik Brodowski <linux@brodo.de>
- *	(C) 2002 Zwane Mwaikambo <zwane@commfireservices.com>
- *	(C) 2002 Arjan van de Ven <arjanv@redhat.com>
- *	(C) 2002 Tora T. Engstad
- *	All Rights Reserved
- *
- *	This program is free software; you can redistribute it and/or
- *      modify it under the terms of the GNU General Public License
- *      as published by the Free Software Foundation; either version
- *      2 of the License, or (at your option) any later version.
- *
- *      The author(s) of this software shall not be held liable for damages
- *      of any nature resulting due to the use of this software. This
- *      software is provided AS-IS with no warranties.
- *
- *	Date		Errata			Description
- *	20020525	N44, O17	12.5% or 25% DC causes lockup
- *
- */
-
-#include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/init.h>
-#include <linux/smp.h>
-#include <linux/cpufreq.h>
-#include <linux/slab.h>
-#include <linux/cpumask.h>
-
-#include <asm/processor.h>
-#include <asm/msr.h>
-#include <asm/timex.h>
-
-#include "speedstep-lib.h"
-
-#define PFX	"p4-clockmod: "
-#define dprintk(msg...) cpufreq_debug_printk(CPUFREQ_DEBUG_DRIVER, "p4-clockmod", msg)
-
-/*
- * Duty Cycle (3bits), note DC_DISABLE is not specified in
- * intel docs i just use it to mean disable
- */
-enum {
-	DC_RESV, DC_DFLT, DC_25PT, DC_38PT, DC_50PT,
-	DC_64PT, DC_75PT, DC_88PT, DC_DISABLE
-};
-
-#define DC_ENTRIES	8
-
-
-static int has_N44_O17_errata[NR_CPUS];
-static unsigned int stock_freq;
-static struct cpufreq_driver p4clockmod_driver;
-static unsigned int cpufreq_p4_get(unsigned int cpu);
-
-static int cpufreq_p4_setdc(unsigned int cpu, unsigned int newstate)
-{
-	u32 l, h;
-
-	if (!cpu_online(cpu) || (newstate > DC_DISABLE) || (newstate == DC_RESV))
-		return -EINVAL;
-
-	rdmsr_on_cpu(cpu, MSR_IA32_THERM_STATUS, &l, &h);
-
-	if (l & 0x01)
-		dprintk("CPU#%d currently thermal throttled\n", cpu);
-
-	if (has_N44_O17_errata[cpu] && (newstate == DC_25PT || newstate == DC_DFLT))
-		newstate = DC_38PT;
-
-	rdmsr_on_cpu(cpu, MSR_IA32_THERM_CONTROL, &l, &h);
-	if (newstate == DC_DISABLE) {
-		dprintk("CPU#%d disabling modulation\n", cpu);
-		wrmsr_on_cpu(cpu, MSR_IA32_THERM_CONTROL, l & ~(1<<4), h);
-	} else {
-		dprintk("CPU#%d setting duty cycle to %d%%\n",
-			cpu, ((125 * newstate) / 10));
-		/* bits 63 - 5	: reserved
-		 * bit  4	: enable/disable
-		 * bits 3-1	: duty cycle
-		 * bit  0	: reserved
-		 */
-		l = (l & ~14);
-		l = l | (1<<4) | ((newstate & 0x7)<<1);
-		wrmsr_on_cpu(cpu, MSR_IA32_THERM_CONTROL, l, h);
-	}
-
-	return 0;
-}
-
-
-static struct cpufreq_frequency_table p4clockmod_table[] = {
-	{DC_RESV, CPUFREQ_ENTRY_INVALID},
-	{DC_DFLT, 0},
-	{DC_25PT, 0},
-	{DC_38PT, 0},
-	{DC_50PT, 0},
-	{DC_64PT, 0},
-	{DC_75PT, 0},
-	{DC_88PT, 0},
-	{DC_DISABLE, 0},
-	{DC_RESV, CPUFREQ_TABLE_END},
-};
-
-
-static int cpufreq_p4_target(struct cpufreq_policy *policy,
-			     unsigned int target_freq,
-			     unsigned int relation)
-{
-	unsigned int    newstate = DC_RESV;
-	struct cpufreq_freqs freqs;
-	int i;
-
-	if (cpufreq_frequency_table_target(policy, &p4clockmod_table[0], target_freq, relation, &newstate))
-		return -EINVAL;
-
-	freqs.old = cpufreq_p4_get(policy->cpu);
-	freqs.new = stock_freq * p4clockmod_table[newstate].index / 8;
-
-	if (freqs.new == freqs.old)
-		return 0;
-
-	/* notifiers */
-	for_each_cpu_mask(i, policy->cpus) {
-		freqs.cpu = i;
-		cpufreq_notify_transition(&freqs, CPUFREQ_PRECHANGE);
-	}
-
-	/* run on each logical CPU, see section 13.15.3 of IA32 Intel Architecture Software
-	 * Developer's Manual, Volume 3
-	 */
-	for_each_cpu_mask(i, policy->cpus)
-		cpufreq_p4_setdc(i, p4clockmod_table[newstate].index);
-
-	/* notifiers */
-	for_each_cpu_mask(i, policy->cpus) {
-		freqs.cpu = i;
-		cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
-	}
-
-	return 0;
-}
-
-
-static int cpufreq_p4_verify(struct cpufreq_policy *policy)
-{
-	return cpufreq_frequency_table_verify(policy, &p4clockmod_table[0]);
-}
-
-
-static unsigned int cpufreq_p4_get_frequency(struct cpuinfo_x86 *c)
-{
-	if (c->x86 == 0x06) {
-		if (cpu_has(c, X86_FEATURE_EST))
-			printk(KERN_WARNING PFX "Warning: EST-capable CPU detected. "
-			       "The acpi-cpufreq module offers voltage scaling"
-			       " in addition of frequency scaling. You should use "
-			       "that instead of p4-clockmod, if possible.\n");
-		switch (c->x86_model) {
-		case 0x0E: /* Core */
-		case 0x0F: /* Core Duo */
-			p4clockmod_driver.flags |= CPUFREQ_CONST_LOOPS;
-			return speedstep_get_processor_frequency(SPEEDSTEP_PROCESSOR_PCORE);
-		case 0x0D: /* Pentium M (Dothan) */
-			p4clockmod_driver.flags |= CPUFREQ_CONST_LOOPS;
-			/* fall through */
-		case 0x09: /* Pentium M (Banias) */
-			return speedstep_get_processor_frequency(SPEEDSTEP_PROCESSOR_PM);
-		}
-	}
-
-	if (c->x86 != 0xF) {
-		printk(KERN_WARNING PFX "Unknown p4-clockmod-capable CPU. Please send an e-mail to <cpufreq@lists.linux.org.uk>\n");
-		return 0;
-	}
-
-	/* on P-4s, the TSC runs with constant frequency independent whether
-	 * throttling is active or not. */
-	p4clockmod_driver.flags |= CPUFREQ_CONST_LOOPS;
-
-	if (speedstep_detect_processor() == SPEEDSTEP_PROCESSOR_P4M) {
-		printk(KERN_WARNING PFX "Warning: Pentium 4-M detected. "
-		       "The speedstep-ich or acpi cpufreq modules offer "
-		       "voltage scaling in addition of frequency scaling. "
-		       "You should use either one instead of p4-clockmod, "
-		       "if possible.\n");
-		return speedstep_get_processor_frequency(SPEEDSTEP_PROCESSOR_P4M);
-	}
-
-	return speedstep_get_processor_frequency(SPEEDSTEP_PROCESSOR_P4D);
-}
-
-
-
-static int cpufreq_p4_cpu_init(struct cpufreq_policy *policy)
-{
-	struct cpuinfo_x86 *c = &cpu_data(policy->cpu);
-	int cpuid = 0;
-	unsigned int i;
-
-#ifdef CONFIG_SMP
-	policy->cpus = per_cpu(cpu_sibling_map, policy->cpu);
-#endif
-
-	/* Errata workaround */
-	cpuid = (c->x86 << 8) | (c->x86_model << 4) | c->x86_mask;
-	switch (cpuid) {
-	case 0x0f07:
-	case 0x0f0a:
-	case 0x0f11:
-	case 0x0f12:
-		has_N44_O17_errata[policy->cpu] = 1;
-		dprintk("has errata -- disabling low frequencies\n");
-	}
-
-	/* get max frequency */
-	stock_freq = cpufreq_p4_get_frequency(c);
-	if (!stock_freq)
-		return -EINVAL;
-
-	/* table init */
-	for (i=1; (p4clockmod_table[i].frequency != CPUFREQ_TABLE_END); i++) {
-		if ((i<2) && (has_N44_O17_errata[policy->cpu]))
-			p4clockmod_table[i].frequency = CPUFREQ_ENTRY_INVALID;
-		else
-			p4clockmod_table[i].frequency = (stock_freq * i)/8;
-	}
-	cpufreq_frequency_table_get_attr(p4clockmod_table, policy->cpu);
-
-	/* cpuinfo and default policy values */
-	policy->cpuinfo.transition_latency = 1000000; /* assumed */
-	policy->cur = stock_freq;
-
-	return cpufreq_frequency_table_cpuinfo(policy, &p4clockmod_table[0]);
-}
-
-
-static int cpufreq_p4_cpu_exit(struct cpufreq_policy *policy)
-{
-	cpufreq_frequency_table_put_attr(policy->cpu);
-	return 0;
-}
-
-static unsigned int cpufreq_p4_get(unsigned int cpu)
-{
-	u32 l, h;
-
-	rdmsr_on_cpu(cpu, MSR_IA32_THERM_CONTROL, &l, &h);
-
-	if (l & 0x10) {
-		l = l >> 1;
-		l &= 0x7;
-	} else
-		l = DC_DISABLE;
-
-	if (l != DC_DISABLE)
-		return (stock_freq * l / 8);
-
-	return stock_freq;
-}
-
-static struct freq_attr* p4clockmod_attr[] = {
-	&cpufreq_freq_attr_scaling_available_freqs,
-	NULL,
-};
-
-static struct cpufreq_driver p4clockmod_driver = {
-	.verify		= cpufreq_p4_verify,
-	.target		= cpufreq_p4_target,
-	.init		= cpufreq_p4_cpu_init,
-	.exit		= cpufreq_p4_cpu_exit,
-	.get		= cpufreq_p4_get,
-	.name		= "p4-clockmod",
-	.owner		= THIS_MODULE,
-	.attr		= p4clockmod_attr,
-};
-
-
-static int __init cpufreq_p4_init(void)
-{
-	struct cpuinfo_x86 *c = &cpu_data(0);
-	int ret;
-
-	/*
-	 * THERM_CONTROL is architectural for IA32 now, so
-	 * we can rely on the capability checks
-	 */
-	if (c->x86_vendor != X86_VENDOR_INTEL)
-		return -ENODEV;
-
-	if (!test_cpu_cap(c, X86_FEATURE_ACPI) ||
-				!test_cpu_cap(c, X86_FEATURE_ACC))
-		return -ENODEV;
-
-	ret = cpufreq_register_driver(&p4clockmod_driver);
-	if (!ret)
-		printk(KERN_INFO PFX "P4/Xeon(TM) CPU On-Demand Clock Modulation available\n");
-
-	return (ret);
-}
-
-
-static void __exit cpufreq_p4_exit(void)
-{
-	cpufreq_unregister_driver(&p4clockmod_driver);
-}
-
-
-MODULE_AUTHOR ("Zwane Mwaikambo <zwane@commfireservices.com>");
-MODULE_DESCRIPTION ("cpufreq driver for Pentium(TM) 4/Xeon(TM)");
-MODULE_LICENSE ("GPL");
-
-late_initcall(cpufreq_p4_init);
-module_exit(cpufreq_p4_exit);

Re: [PATCH] Remove p4_clockmode driver

[Dave Jones]

On Sat, May 03, 2008 at 09:47:10PM +0200, Thomas Renninger wrote:
 > Remove p4_clockmode driver
 >
 > The driver is doing throttling which is supposed to be done through another
 > ACPI interface. If both interfaces are used, the machine may get very slow.
 > Remove this driver which should never be used.
 >
 > Signed-off-by: Thomas Renninger <trenn@suse.de>

I agree this is long overdue being taken out back and shot.

Some thoughts though:

* Should we deprecate this for a release first ?
  I'm torn over this, because I think no matter how long we do this for,
  we're going to get people who claim to be surprised.
  Though given this driver is 99% useless, perhaps just ripping it out
  is for the best.

* Matthew Garrett mentioned something interesting this morning..

mjg59 | davej: On machines that don't support acpi throttling, I was under the impression that there was supposed to be a hook between the thermal code and cpufreq
mjg59 | Which would then let p4-clockmod be kicked even if it's below the threshold at which the CPU would throttle itself
davej | having that code expose T states rather than pretend to be P states is probably for the best.
mjg59 | Yeah, true

  So whilst killing this off is probably the right thing to do, there may be
  some value in parts of it living on in the ACPI code ?

I'll queue up this removal patch for linux-next, and we'll see if anyone screams.
(I doubt it, given the amount of testing that gets right now. It'll probably
not really get noticed until after it's in a Linus sanctioned release)

        Dave

--
http://www.codemonkey.org.uk

Re: [PATCH] Remove p4_clockmode driver

[Cesar Eduardo Barros]

Thomas Renninger escreveu:
> Remove p4_clockmode driver
> 
> The driver is doing throttling which is supposed to be done through another
> ACPI interface. If both interfaces are used, the machine may get very slow.
> Remove this driver which should never be used.

On the Dell Latitude D510 I use at work, acpi-cpufreq didn't load on the 
Ubuntu 7.10 kernel (2.6.22); of all the available cpufreq drivers, only 
p4-clockmod worked. I didn't try loading it on the Ubuntu 8.04 kernel 
(2.6.24) yet.

-- 
Cesar Eduardo Barros
cesarb@cesarb.net
cesar.barros@gmail.com

Re: [PATCH] Remove p4_clockmode driver

[Dominik Brodowski]

Dave,

On Sun, May 11, 2008 at 12:19:34PM -0400, Dave Jones wrote:
>   So whilst killing this off is probably the right thing to do, there may be
>   some value in parts of it living on in the ACPI code ?

if it indeed gets ripped out of the kernel code[*], a good place to keep it
is in cpufrequtils, where one working and one broken external debug module
are kept at the moment.

Best,
	Dominik


[*] even though it was the very first cpufreq driver which existed, as the
hardware I used back then had no proper frequency scaling...

RE: [PATCH] Remove p4_clockmode driver

[Pallipadi, Venkatesh]

 

>-----Original Message-----
>From: cpufreq-bounces@lists.linux.org.uk 
>[mailto:cpufreq-bounces@lists.linux.org.uk] On Behalf Of Dave Jones
>Sent: Sunday, May 11, 2008 9:20 AM
>To: Thomas Renninger
>Cc: cpufreq; mzahor@dtech.sk; Dominik Brodowski
>Subject: Re: [PATCH] Remove p4_clockmode driver
>
>On Sat, May 03, 2008 at 09:47:10PM +0200, Thomas Renninger wrote:
> > Remove p4_clockmode driver
> >
> > The driver is doing throttling which is supposed to be done 
>through another
> > ACPI interface. If both interfaces are used, the machine 
>may get very slow.
> > Remove this driver which should never be used.
> >
> > Signed-off-by: Thomas Renninger <trenn@suse.de>
>
>I agree this is long overdue being taken out back and shot.
>
>Some thoughts though:
>
>* Should we deprecate this for a release first ?
>  I'm torn over this, because I think no matter how long we do 
>this for,
>  we're going to get people who claim to be surprised.
>  Though given this driver is 99% useless, perhaps just ripping it out
>  is for the best.
>
>* Matthew Garrett mentioned something interesting this morning..
>
>mjg59 | davej: On machines that don't support acpi throttling, 
>I was under the impression that there was supposed to be a 
>hook between the thermal code and cpufreq
>mjg59 | Which would then let p4-clockmod be kicked even if 
>it's below the threshold at which the CPU would throttle itself
>davej | having that code expose T states rather than pretend 
>to be P states is probably for the best.
>mjg59 | Yeah, true
>
>  So whilst killing this off is probably the right thing to 
>do, there may be
>  some value in parts of it living on in the ACPI code ?

ACPI code already has support for T-states, which uses the same
mechanism as P4-clockmod
in response to thermal events (once the CPU is at lowest freq). However,
it needs the BIOS
to export T-state information. May be there is some value to add a
driver to do T-states even
without ACPI T-states and hook into thermal code.

Agree that sooner we get this out of cpufreq better it is. I don't think
we can explain it
any more that "P4-clockmod works does not mean it is saving any power".

Thanks,
Venki

Re: [PATCH] Remove p4_clockmode driver

[Dmitry Torokhov]

Hi Dave,

On Sun, May 11, 2008 at 12:19:34PM -0400, Dave Jones wrote:
> On Sat, May 03, 2008 at 09:47:10PM +0200, Thomas Renninger wrote:
>  > Remove p4_clockmode driver
>  >
>  > The driver is doing throttling which is supposed to be done through another
>  > ACPI interface. If both interfaces are used, the machine may get very slow.
>  > Remove this driver which should never be used.
>  >
>  > Signed-off-by: Thomas Renninger <trenn@suse.de>
> 
> I agree this is long overdue being taken out back and shot.
> 
> Some thoughts though:
> 
> * Should we deprecate this for a release first ?
>   I'm torn over this, because I think no matter how long we do this for,
>   we're going to get people who claim to be surprised.
>   Though given this driver is 99% useless, perhaps just ripping it out
>   is for the best.
> 
> * Matthew Garrett mentioned something interesting this morning..
> 
> mjg59 | davej: On machines that don't support acpi throttling, I was under the impression that there was supposed to be a hook between the thermal code and cpufreq
> mjg59 | Which would then let p4-clockmod be kicked even if it's below the threshold at which the CPU would throttle itself
> davej | having that code expose T states rather than pretend to be P states is probably for the best.
> mjg59 | Yeah, true
> 
>   So whilst killing this off is probably the right thing to do, there may be
>   some value in parts of it living on in the ACPI code ?
> 
> I'll queue up this removal patch for linux-next, and we'll see if anyone screams.
> (I doubt it, given the amount of testing that gets right now. It'll probably
> not really get noticed until after it's in a Linus sanctioned release)
> 

AAAAAAA!</scream>

I am using p4_clockmod in attempt to keep my old P4-based box a bit
cooler. It does not support anything besides C0 (no P-states, no
throttling) so acpi-based driver does not work.

-- 
Dmitry

Re: [PATCH] Remove p4_clockmode driver

[Thomas Renninger]

On Thu, 2008-05-22 at 00:01 -0400, Dmitry Torokhov wrote:
> Hi Dave,
> 
> On Sun, May 11, 2008 at 12:19:34PM -0400, Dave Jones wrote:
> > On Sat, May 03, 2008 at 09:47:10PM +0200, Thomas Renninger wrote:
> >  > Remove p4_clockmode driver
> >  >
> >  > The driver is doing throttling which is supposed to be done through another
> >  > ACPI interface. If both interfaces are used, the machine may get very slow.
> >  > Remove this driver which should never be used.
> >  >
> >  > Signed-off-by: Thomas Renninger <trenn@suse.de>
> > 
> > I agree this is long overdue being taken out back and shot.
> > 
> > Some thoughts though:
> > 
> > * Should we deprecate this for a release first ?
> >   I'm torn over this, because I think no matter how long we do this for,
> >   we're going to get people who claim to be surprised.
> >   Though given this driver is 99% useless, perhaps just ripping it out
> >   is for the best.
> > 
> > * Matthew Garrett mentioned something interesting this morning..
> > 
> > mjg59 | davej: On machines that don't support acpi throttling, I was under the impression that there was supposed to be a hook between the thermal code and cpufreq
> > mjg59 | Which would then let p4-clockmod be kicked even if it's below the threshold at which the CPU would throttle itself
> > davej | having that code expose T states rather than pretend to be P states is probably for the best.
> > mjg59 | Yeah, true
> > 
> >   So whilst killing this off is probably the right thing to do, there may be
> >   some value in parts of it living on in the ACPI code ?
> > 
> > I'll queue up this removal patch for linux-next, and we'll see if anyone screams.
> > (I doubt it, given the amount of testing that gets right now. It'll probably
> > not really get noticed until after it's in a Linus sanctioned release)
> > 
> 
> AAAAAAA!</scream>
> 
> I am using p4_clockmod in attempt to keep my old P4-based box a bit
> cooler. It does not support anything besides C0 (no P-states, no
> throttling) so acpi-based driver does not work.

P4 CPUs do waste power, it is not designed for power savings.
How much do you save with p4-clockmod?

But maybe a reimplementation as Matthew suggested it may really be worth
it. If it is, someone probably will do it.

The cpufreq interface is the wrong place for this driver.
If you have a machine that exposes throttling as expected and you make
use of it (and there are userspace tools doing this), you are busted.

Instead of screaming, better provide suggestions for a proper
integration for machines which are capable of throttling, but have a
broken BIOS which does not export it (maybe they are just doing this
because it's not worth it?).

AFAIK there are AMD drivers with static cpufreq tables which were never
accepted. People with a BIOS who's CPU is not detected and cpufreq info
is not exposed have bad luck and must either poke the vendor or
self-compile the out-of-tree static table solution.
I would 10 times more support this to be integrated because:
  - It fits the design and does not slow down other machines as a side
    effect
  - It really saves some power
I still think it's ok that it did not go mainline, for maintenance and
"it's the wrong way" reasons.

Please, if you have verified that it really saves power (I'd really like
to see some figures here, I never saw any for p4-clockmod) and it's
worth having it, help to get this implemented properly, but do not delay
a process which is overdue for a long time already.

    Thomas

Re: [PATCH] Remove p4_clockmode driver

[Dmitry Torokhov]

Hi Thomas,

On Fri, May 23, 2008 at 01:21:18PM +0200, Thomas Renninger wrote:
> On Thu, 2008-05-22 at 00:01 -0400, Dmitry Torokhov wrote:
> > Hi Dave,
> > 
> > On Sun, May 11, 2008 at 12:19:34PM -0400, Dave Jones wrote:
> > > On Sat, May 03, 2008 at 09:47:10PM +0200, Thomas Renninger wrote:
> > >  > Remove p4_clockmode driver
> > >  >
> > >  > The driver is doing throttling which is supposed to be done through another
> > >  > ACPI interface. If both interfaces are used, the machine may get very slow.
> > >  > Remove this driver which should never be used.
> > >  >
> > >  > Signed-off-by: Thomas Renninger <trenn@suse.de>
> > > 
> > > I agree this is long overdue being taken out back and shot.
> > > 
> > > Some thoughts though:
> > > 
> > > * Should we deprecate this for a release first ?
> > >   I'm torn over this, because I think no matter how long we do this for,
> > >   we're going to get people who claim to be surprised.
> > >   Though given this driver is 99% useless, perhaps just ripping it out
> > >   is for the best.
> > > 
> > > * Matthew Garrett mentioned something interesting this morning..
> > > 
> > > mjg59 | davej: On machines that don't support acpi throttling, I was under the impression that there was supposed to be a hook between the thermal code and cpufreq
> > > mjg59 | Which would then let p4-clockmod be kicked even if it's below the threshold at which the CPU would throttle itself
> > > davej | having that code expose T states rather than pretend to be P states is probably for the best.
> > > mjg59 | Yeah, true
> > > 
> > >   So whilst killing this off is probably the right thing to do, there may be
> > >   some value in parts of it living on in the ACPI code ?
> > > 
> > > I'll queue up this removal patch for linux-next, and we'll see if anyone screams.
> > > (I doubt it, given the amount of testing that gets right now. It'll probably
> > > not really get noticed until after it's in a Linus sanctioned release)
> > > 
> > 
> > AAAAAAA!</scream>
> > 
> > I am using p4_clockmod in attempt to keep my old P4-based box a bit
> > cooler. It does not support anything besides C0 (no P-states, no
> > throttling) so acpi-based driver does not work.
> 
> P4 CPUs do waste power, it is not designed for power savings.
> How much do you save with p4-clockmod?
> 

Zilch. As I said I am not really try to save power but I am trying to
keep my room a bit cooler when I leave the box idle.

> But maybe a reimplementation as Matthew suggested it may really be worth
> it. If it is, someone probably will do it.
> 
> The cpufreq interface is the wrong place for this driver.
> If you have a machine that exposes throttling as expected and you make
> use of it (and there are userspace tools doing this), you are busted.
>

And I don't care since:

	a) my box does not expose it and
	b) I might or might not be using such tools, depending on my
	   circumstances

 
> Instead of screaming, better provide suggestions for a proper
> integration for machines which are capable of throttling, but have a
> broken BIOS which does not export it (maybe they are just doing this
> because it's not worth it?).
> 
> AFAIK there are AMD drivers with static cpufreq tables which were never
> accepted. People with a BIOS who's CPU is not detected and cpufreq info
> is not exposed have bad luck and must either poke the vendor or
> self-compile the out-of-tree static table solution.
> I would 10 times more support this to be integrated because:
>   - It fits the design and does not slow down other machines as a side
>     effect
>   - It really saves some power
> I still think it's ok that it did not go mainline, for maintenance and
> "it's the wrong way" reasons.
> 

The difference here is that p4_clockmod is _in_ mainline so by simply
removing it you introduce regression. You can't just remove driver
without providing a similarly working replacement. If you are concerned
about unwanted iteractions with ACPI code you could either add a hook
disabling p4_clockmod when acpi throtting gets loaded, or put a big
bfat warning about p4_clockmod in dmesg and make it depend on
CONFIG_EXPERIMENTAL or CONFIG_EMBEDDED.

> Please, if you have verified that it really saves power (I'd really like
> to see some figures here, I never saw any for p4-clockmod) and it's
> worth having it, help to get this implemented properly, but do not delay
> a process which is overdue for a long time already.
> 

The process woudld be to implement the new way properly and then retire
the old driver. You are putting a carriage before the horse here.

-- 
Dmitry

Re: [PATCH] Remove p4_clockmode driver

[Thomas Renninger]

On Fri, 2008-05-23 at 08:55 -0400, Dmitry Torokhov wrote:
> Hi Thomas,
> 
> On Fri, May 23, 2008 at 01:21:18PM +0200, Thomas Renninger wrote:
> > On Thu, 2008-05-22 at 00:01 -0400, Dmitry Torokhov wrote:
> > > Hi Dave,
> > > 
> > > On Sun, May 11, 2008 at 12:19:34PM -0400, Dave Jones wrote:
> > > > On Sat, May 03, 2008 at 09:47:10PM +0200, Thomas Renninger wrote:
> > > >  > Remove p4_clockmode driver
> > > >  >
> > > >  > The driver is doing throttling which is supposed to be done through another
> > > >  > ACPI interface. If both interfaces are used, the machine may get very slow.
> > > >  > Remove this driver which should never be used.
> > > >  >
> > > >  > Signed-off-by: Thomas Renninger <trenn@suse.de>
> > > > 
> > > > I agree this is long overdue being taken out back and shot.
> > > > 
> > > > Some thoughts though:
> > > > 
> > > > * Should we deprecate this for a release first ?
> > > >   I'm torn over this, because I think no matter how long we do this for,
> > > >   we're going to get people who claim to be surprised.
> > > >   Though given this driver is 99% useless, perhaps just ripping it out
> > > >   is for the best.
> > > > 
> > > > * Matthew Garrett mentioned something interesting this morning..
> > > > 
> > > > mjg59 | davej: On machines that don't support acpi throttling, I was under the impression that there was supposed to be a hook between the thermal code and cpufreq
> > > > mjg59 | Which would then let p4-clockmod be kicked even if it's below the threshold at which the CPU would throttle itself
> > > > davej | having that code expose T states rather than pretend to be P states is probably for the best.
> > > > mjg59 | Yeah, true
> > > > 
> > > >   So whilst killing this off is probably the right thing to do, there may be
> > > >   some value in parts of it living on in the ACPI code ?
> > > > 
> > > > I'll queue up this removal patch for linux-next, and we'll see if anyone screams.
> > > > (I doubt it, given the amount of testing that gets right now. It'll probably
> > > > not really get noticed until after it's in a Linus sanctioned release)
> > > > 
> > > 
> > > AAAAAAA!</scream>
> > > 
> > > I am using p4_clockmod in attempt to keep my old P4-based box a bit
> > > cooler. It does not support anything besides C0 (no P-states, no
> > > throttling) so acpi-based driver does not work.
> > 
> > P4 CPUs do waste power, it is not designed for power savings.
> > How much do you save with p4-clockmod?
> > 
> 
> Zilch. As I said I am not really try to save power but I am trying to
> keep my room a bit cooler when I leave the box idle.
Having people believe they get a cooler room without saving power is a
nice feature. Removing it is not a regression. This is exactly what the
p4_clockmod driver suggests and what people were complaining "Windows is
running with 300 MHz on my P4", it's nice marketing, but not reality.
Intel told me they did some tests with SMP machines where throttling
actually increases the power consumption.

> > But maybe a reimplementation as Matthew suggested it may really be worth
> > it. If it is, someone probably will do it.
> > 
> > The cpufreq interface is the wrong place for this driver.
> > If you have a machine that exposes throttling as expected and you make
> > use of it (and there are userspace tools doing this), you are busted.
> >
> 
> And I don't care since:
> 
> 	a) my box does not expose it and
> 	b) I might or might not be using such tools, depending on my
> 	   circumstances
> 
>  
> > Instead of screaming, better provide suggestions for a proper
> > integration for machines which are capable of throttling, but have a
> > broken BIOS which does not export it (maybe they are just doing this
> > because it's not worth it?).
> > 
> > AFAIK there are AMD drivers with static cpufreq tables which were never
> > accepted. People with a BIOS who's CPU is not detected and cpufreq info
> > is not exposed have bad luck and must either poke the vendor or
> > self-compile the out-of-tree static table solution.
> > I would 10 times more support this to be integrated because:
> >   - It fits the design and does not slow down other machines as a side
> >     effect
> >   - It really saves some power
> > I still think it's ok that it did not go mainline, for maintenance and
> > "it's the wrong way" reasons.
> > 
> 
> The difference here is that p4_clockmod is _in_ mainline so by simply
> removing it you introduce regression.
Please prove, I do not see a regression. How much Watts does the machine
save power when it is idle using p4-clockmod?

> You can't just remove driver
> without providing a similarly working replacement. If you are concerned
> about unwanted iteractions with ACPI code you could either add a hook
> disabling p4_clockmod when acpi throtting gets loaded, or put a big
> bfat warning about p4_clockmod in dmesg and make it depend on
> CONFIG_EXPERIMENTAL or CONFIG_EMBEDDED.
> 
> > Please, if you have verified that it really saves power (I'd really like
> > to see some figures here, I never saw any for p4-clockmod) and it's
> > worth having it, help to get this implemented properly, but do not delay
> > a process which is overdue for a long time already.
> >
> The process woudld be to implement the new way properly and then retire
> the old driver. You are putting a carriage before the horse here.
This will never happen. Nobody cares about throttling because it doesn't
save you anything. So we have broken machines (through double
throttling) and a driver which is of no (at least not much) use.

This reminds me about the broken drivers/acpi/video.c state:
http://bugzilla.kernel.org/show_bug.cgi?id=9614
ThinkPads do use the wrong ACPI device (Nvidia instead Intel ACPI
graphics device) and therefore works by luck.
The correct fix, making a lot other models work is ignored because for
the Intel ACPI graphics device there is no ACPI (IGD) Linux driver.
There even is a workaround to get the ThinkPad working by using
thinkpad_acpi driver for brightness switching...
Now everything is broken and will stay like that, because the change
will introduce a regression for ThinkPads (cannot switch brightness via
video.c for Intel ACPI graphics device, poking with the wrong HW device
works somehow...).
What I mean..., the regression argument is a powerful one, misusing it
for unimportant, wrong designed or broken implementations (at least the
first two are valid here) can make things worse.

    Thomas

Re: [PATCH] Remove p4_clockmode driver

[Dmitry Torokhov]

On Fri, May 23, 2008 at 05:19:37PM +0200, Thomas Renninger wrote:
> On Fri, 2008-05-23 at 08:55 -0400, Dmitry Torokhov wrote:
> > Hi Thomas,
> > 
> > On Fri, May 23, 2008 at 01:21:18PM +0200, Thomas Renninger wrote:
> > > On Thu, 2008-05-22 at 00:01 -0400, Dmitry Torokhov wrote:
> > > > Hi Dave,
> > > > 
> > > > On Sun, May 11, 2008 at 12:19:34PM -0400, Dave Jones wrote:
> > > > > On Sat, May 03, 2008 at 09:47:10PM +0200, Thomas Renninger wrote:
> > > > >  > Remove p4_clockmode driver
> > > > >  >
> > > > >  > The driver is doing throttling which is supposed to be done through another
> > > > >  > ACPI interface. If both interfaces are used, the machine may get very slow.
> > > > >  > Remove this driver which should never be used.
> > > > >  >
> > > > >  > Signed-off-by: Thomas Renninger <trenn@suse.de>
> > > > > 
> > > > > I agree this is long overdue being taken out back and shot.
> > > > > 
> > > > > Some thoughts though:
> > > > > 
> > > > > * Should we deprecate this for a release first ?
> > > > >   I'm torn over this, because I think no matter how long we do this for,
> > > > >   we're going to get people who claim to be surprised.
> > > > >   Though given this driver is 99% useless, perhaps just ripping it out
> > > > >   is for the best.
> > > > > 
> > > > > * Matthew Garrett mentioned something interesting this morning..
> > > > > 
> > > > > mjg59 | davej: On machines that don't support acpi throttling, I was under the impression that there was supposed to be a hook between the thermal code and cpufreq
> > > > > mjg59 | Which would then let p4-clockmod be kicked even if it's below the threshold at which the CPU would throttle itself
> > > > > davej | having that code expose T states rather than pretend to be P states is probably for the best.
> > > > > mjg59 | Yeah, true
> > > > > 
> > > > >   So whilst killing this off is probably the right thing to do, there may be
> > > > >   some value in parts of it living on in the ACPI code ?
> > > > > 
> > > > > I'll queue up this removal patch for linux-next, and we'll see if anyone screams.
> > > > > (I doubt it, given the amount of testing that gets right now. It'll probably
> > > > > not really get noticed until after it's in a Linus sanctioned release)
> > > > > 
> > > > 
> > > > AAAAAAA!</scream>
> > > > 
> > > > I am using p4_clockmod in attempt to keep my old P4-based box a bit
> > > > cooler. It does not support anything besides C0 (no P-states, no
> > > > throttling) so acpi-based driver does not work.
> > > 
> > > P4 CPUs do waste power, it is not designed for power savings.
> > > How much do you save with p4-clockmod?
> > > 
> > 
> > Zilch. As I said I am not really try to save power but I am trying to
> > keep my room a bit cooler when I leave the box idle.
> Having people believe they get a cooler room without saving power is a
> nice feature. Removing it is not a regression. This is exactly what the
> p4_clockmod driver suggests and what people were complaining "Windows is
> running with 300 MHz on my P4", it's nice marketing, but not reality.
> Intel told me they did some tests with SMP machines where throttling
> actually increases the power consumption.

How about this:

The hardware does throttling in case of critical overheat therefore
throttling descreases the temperature. Thus one can use p4_clockmod
that does the throttling to make the box a bit cooler.

I don't care about overall power consumption. I thought I was clear in
the first letter and even more clear in the second.

> 
> > > But maybe a reimplementation as Matthew suggested it may really be worth
> > > it. If it is, someone probably will do it.
> > > 
> > > The cpufreq interface is the wrong place for this driver.
> > > If you have a machine that exposes throttling as expected and you make
> > > use of it (and there are userspace tools doing this), you are busted.
> > >
> > 
> > And I don't care since:
> > 
> > 	a) my box does not expose it and
> > 	b) I might or might not be using such tools, depending on my
> > 	   circumstances
> > 
> >  
> > > Instead of screaming, better provide suggestions for a proper
> > > integration for machines which are capable of throttling, but have a
> > > broken BIOS which does not export it (maybe they are just doing this
> > > because it's not worth it?).
> > > 
> > > AFAIK there are AMD drivers with static cpufreq tables which were never
> > > accepted. People with a BIOS who's CPU is not detected and cpufreq info
> > > is not exposed have bad luck and must either poke the vendor or
> > > self-compile the out-of-tree static table solution.
> > > I would 10 times more support this to be integrated because:
> > >   - It fits the design and does not slow down other machines as a side
> > >     effect
> > >   - It really saves some power
> > > I still think it's ok that it did not go mainline, for maintenance and
> > > "it's the wrong way" reasons.
> > > 
> > 
> > The difference here is that p4_clockmod is _in_ mainline so by simply
> > removing it you introduce regression.
> Please prove, I do not see a regression.

Yes it is. The thing is gone. I can't use it anymore.

> How much Watts does the machine
> save power when it is idle using p4-clockmod?

Don't care. Don't care. Do I need to repeat it? I don't care. All I
know is that it87 reports lower temperature when CPU is throttled.

> 
> > You can't just remove driver
> > without providing a similarly working replacement. If you are concerned
> > about unwanted iteractions with ACPI code you could either add a hook
> > disabling p4_clockmod when acpi throtting gets loaded, or put a big
> > bfat warning about p4_clockmod in dmesg and make it depend on
> > CONFIG_EXPERIMENTAL or CONFIG_EMBEDDED.
> > 
> > > Please, if you have verified that it really saves power (I'd really like
> > > to see some figures here, I never saw any for p4-clockmod) and it's
> > > worth having it, help to get this implemented properly, but do not delay
> > > a process which is overdue for a long time already.
> > >
> > The process woudld be to implement the new way properly and then retire
> > the old driver. You are putting a carriage before the horse here.
> This will never happen. Nobody cares about throttling because it doesn't
> save you anything. So we have broken machines (through double
> throttling) and a driver which is of no (at least not much) use.
> 
> This reminds me about the broken drivers/acpi/video.c state:
> http://bugzilla.kernel.org/show_bug.cgi?id=9614
> ThinkPads do use the wrong ACPI device (Nvidia instead Intel ACPI
> graphics device) and therefore works by luck.
> The correct fix, making a lot other models work is ignored because for
> the Intel ACPI graphics device there is no ACPI (IGD) Linux driver.
> There even is a workaround to get the ThinkPad working by using
> thinkpad_acpi driver for brightness switching...
> Now everything is broken and will stay like that, because the change
> will introduce a regression for ThinkPads (cannot switch brightness via
> video.c for Intel ACPI graphics device, poking with the wrong HW device
> works somehow...).
> What I mean..., the regression argument is a powerful one, misusing it
> for unimportant, wrong designed or broken implementations (at least the
> first two are valid here) can make things worse.
> 

You can call my hardware broken and the driver broken but the fact is
regression for me. And as long as you remove something that works well
for some people for the sake of non-existing yet clean implementaion
(which btw you aren't going to implement it appears) it is a
regression, plain and simple.

-- 
Dmitry

Re: [PATCH] Remove p4_clockmode driver

[Thomas Renninger]

...
> > How much Watts does the machine
> > save power when it is idle using p4-clockmod?
> 
> Don't care. Don't care. Do I need to repeat it? I don't care. All I
> know is that it87 reports lower temperature when CPU is throttled.
How much is it, constantly?

   Thomas

Re: [PATCH] Remove p4_clockmode driver

[Dmitry Torokhov]

On Fri, May 23, 2008 at 06:16:07PM +0200, Thomas Renninger wrote:
> ...
> > > How much Watts does the machine
> > > save power when it is idle using p4-clockmod?
> > 
> > Don't care. Don't care. Do I need to repeat it? I don't care. All I
> > know is that it87 reports lower temperature when CPU is throttled.
> How much is it, constantly?
> 

Hmm, it looks ike on ide it is about 78C throttled and 81 unthrottled
(I would not pay attention to absolute numbers, I never actually tried
to calibrate the readings). So apparently not a lot... For some reason
I remember the difference a bit bigger, maybe I was lookig under light
load?

-- 
Dmitry

Re: [PATCH] Remove p4_clockmode driver

[Dominik Brodowski]

Hi,

On Fri, May 23, 2008 at 01:40:22PM -0400, Dmitry Torokhov wrote:
> On Fri, May 23, 2008 at 06:16:07PM +0200, Thomas Renninger wrote:
> > ...
> > > > How much Watts does the machine
> > > > save power when it is idle using p4-clockmod?
> > > 
> > > Don't care. Don't care. Do I need to repeat it? I don't care. All I
> > > know is that it87 reports lower temperature when CPU is throttled.
> > How much is it, constantly?
> > 
> 
> Hmm, it looks ike on ide it is about 78C throttled and 81 unthrottled
> (I would not pay attention to absolute numbers, I never actually tried
> to calibrate the readings). So apparently not a lot... For some reason
> I remember the difference a bit bigger, maybe I was lookig under light
> load?

Probably you were looking under more or less heavy load, for then and only
then p4-clockmod makes a difference. (>12.5 load at least)

Thomas: the question whether it saves power when it is idle is not a good
criterion -- IIRC, some Intel SpeedStep-capable processors do enter the same
low voltage idle state, so on a 100% idle system acpi-cpufreq wouldn't make
a difference there either. Similarily, some cpufreq drivers which only do
frequency scaling but not frequency and voltage scaling will also only see
limited energy savings on 100% idle.

To summarize: If the CPU load is constantly x (0<x<1) within a timeframe,
the _CPU_ energy consumption of an ideal system is approximately

- if the CPU does not support frequency or voltage scaling:

	E_none	=  E_idle * (1-x) + E_max * x

- if the CPU supports frequency scaling (with linear energy savings):

	E_fs	=  E_idle + (E_max - E_idle) * x  = E_none

- if the CPU supports throttling:

	E_t	=  E_idle * (1-x) + E_max * x = E_none

- if the CPU supports frequency and voltage scaling (with energy savings to
  the power of two)

	E_fvs	=  E_idle + (E_max - E_idle) * x^2


This shows that:
1) _if_ idle states work properly, and
2) _if_ the system needs to finish tasks as soon as possible,
neither frequency scaling alone nor throttling will get you anything.

Now, let's test whether these two requirements are met:

ad 1) to the Intel folks out here: is C1 entered during throttling, or some
deeper sleep level? If it's something deeper, this requirement is not met on
my desktop P4 which only offers C1. 

ad 2) I can see of two aspects where this is invalid:
	- programs which will eat up as much CPU as possible e.g. for
	  even more frames per seconds, and
	- thermal side-effects: if the CPU energy consumption surpasses
	  certain thresholds, fans might be needed for cooling, or their
	  rotational speed increased, thereby also increasing the noise,
	  and not to forget the increased hardware wear by increased
	  temperatures...

However, this second requirement alone might be able to be met by software
alone, as long as the first requirement -- a proper idle state is entered --
is fulfilled:
(1) Currently, _all_ tasks preempt the idle task -- if there's only one task
running in nice level 19, that one gets 100% of the CPU, and the idle thread
gets none.
(2) One could now start a task with a very high priority, which schedules
its own wakeup, and then calls cpuidle_idle_call(). Caveat: this would not
work properly in a high-interrupt system.

Now, such a kernel module would do what -- it would _emulate_ frequency
scaling in a software level, while p4-clockmod _emulates_ frequency scaling
in hardware. Therefore, the correct place for p4-clockmod and such a generic
"I do not want to use my hardware 100%"-module is within the framework which
is made for the capabilities they emulate: cpufreq.

Best,
	Dominik

Re: [PATCH] Remove p4_clockmode driver

[Thomas Renninger]

On Sat, 2008-05-24 at 13:03 +0200, Dominik Brodowski wrote:
> Hi,
> 
> On Fri, May 23, 2008 at 01:40:22PM -0400, Dmitry Torokhov wrote:
> > On Fri, May 23, 2008 at 06:16:07PM +0200, Thomas Renninger wrote:
> > > ...
> > > > > How much Watts does the machine
> > > > > save power when it is idle using p4-clockmod?
> > > > 
> > > > Don't care. Don't care. Do I need to repeat it? I don't care. All I
> > > > know is that it87 reports lower temperature when CPU is throttled.
> > > How much is it, constantly?
> > > 
> > 
> > Hmm, it looks ike on ide it is about 78C throttled and 81 unthrottled
> > (I would not pay attention to absolute numbers, I never actually tried
> > to calibrate the readings). So apparently not a lot... For some reason
> > I remember the difference a bit bigger, maybe I was lookig under light
> > load?
> 
> Probably you were looking under more or less heavy load, for then and only
> then p4-clockmod makes a difference. (>12.5 load at least)
> 
> Thomas: the question whether it saves power when it is idle is not a good
> criterion -- IIRC, some Intel SpeedStep-capable processors do enter the same
> low voltage idle state, so on a 100% idle system acpi-cpufreq wouldn't make
> a difference there either. Similarily, some cpufreq drivers which only do
> frequency scaling but not frequency and voltage scaling will also only see
> limited energy savings on 100% idle.
> 
> To summarize: If the CPU load is constantly x (0<x<1) within a timeframe,
> the _CPU_ energy consumption of an ideal system is approximately
> 
> - if the CPU does not support frequency or voltage scaling:
> 
> 	E_none	=  E_idle * (1-x) + E_max * x
> 
> - if the CPU supports frequency scaling (with linear energy savings):
> 
> 	E_fs	=  E_idle + (E_max - E_idle) * x  = E_none
> 
> - if the CPU supports throttling:
> 
> 	E_t	=  E_idle * (1-x) + E_max * x = E_none
> 
> - if the CPU supports frequency and voltage scaling (with energy savings to
>   the power of two)
> 
> 	E_fvs	=  E_idle + (E_max - E_idle) * x^2
> 
> 
> This shows that:
> 1) _if_ idle states work properly, and
> 2) _if_ the system needs to finish tasks as soon as possible,
> neither frequency scaling alone nor throttling will get you anything.
> 
> Now, let's test whether these two requirements are met:
> 
> ad 1) to the Intel folks out here: is C1 entered during throttling, or some
> deeper sleep level? If it's something deeper, this requirement is not met on
> my desktop P4 which only offers C1. 
> 
> ad 2) I can see of two aspects where this is invalid:
> 	- programs which will eat up as much CPU as possible e.g. for
> 	  even more frames per seconds, and
> 	- thermal side-effects: if the CPU energy consumption surpasses
> 	  certain thresholds, fans might be needed for cooling, or their
> 	  rotational speed increased, thereby also increasing the noise,
> 	  and not to forget the increased hardware wear by increased
> 	  temperatures...
> 
> However, this second requirement alone might be able to be met by software
> alone, as long as the first requirement -- a proper idle state is entered --
> is fulfilled:
> (1) Currently, _all_ tasks preempt the idle task -- if there's only one task
> running in nice level 19, that one gets 100% of the CPU, and the idle thread
> gets none.
> (2) One could now start a task with a very high priority, which schedules
> its own wakeup, and then calls cpuidle_idle_call(). Caveat: this would not
> work properly in a high-interrupt system.
> 
> Now, such a kernel module would do what -- it would _emulate_ frequency
> scaling in a software level, while p4-clockmod _emulates_ frequency scaling
> in hardware. Therefore, the correct place for p4-clockmod and such a generic
> "I do not want to use my hardware 100%"-module is within the framework which
> is made for the capabilities they emulate: cpufreq.

Could you explain this in two easy sentences again, why this should not
use the throttling interface?
(former /proc/acpi/processor/*/throttling, now somewhere located in
sysfs. Also used for passive cooling if no cpufreq is available).
Throttling is what this driver is doing and with which it will interfere
and slow down the machine to be closed to unresponsive if both
(throttling and cpufreq interface) are using it?

Thanks,

   Thomas

Re: [PATCH] Remove p4_clockmode driver

[Matthew Garrett]

On Sun, May 25, 2008 at 05:39:31PM +0200, Thomas Renninger wrote:

> Could you explain this in two easy sentences again, why this should not
> use the throttling interface?

Because it involves rewriting code for purely aesthetic benefit. I agree 
that, in an ideal world, p4-clockmod would integrate into the throttling 
system. But unless someone wants to rewrite it, that doesn't look 
likely...

> (former /proc/acpi/processor/*/throttling, now somewhere located in
> sysfs. Also used for passive cooling if no cpufreq is available).
> Throttling is what this driver is doing and with which it will interfere
> and slow down the machine to be closed to unresponsive if both
> (throttling and cpufreq interface) are using it?

The ACPI throttling interface will only be used if the minimum cpufreq 
state has already been reached. Users really shouldn't play with the 
throttling interface themselves, given its lack of useful functionality.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] Remove p4_clockmode driver

[Thomas Renninger]

On Mon, 2008-05-26 at 13:05 +0100, Matthew Garrett wrote:
> On Sun, May 25, 2008 at 05:39:31PM +0200, Thomas Renninger wrote:
> 
> > Could you explain this in two easy sentences again, why this should not
> > use the throttling interface?
> 
> Because it involves rewriting code for purely aesthetic benefit. I agree 
> that, in an ideal world, p4-clockmod would integrate into the throttling 
> system. But unless someone wants to rewrite it, that doesn't look 
> likely...
Yes, I should add a p4-clockmod filter and move all upcoming issues on
the cpufreq list automatically to the trash...
> 
> > (former /proc/acpi/processor/*/throttling, now somewhere located in
> > sysfs. Also used for passive cooling if no cpufreq is available).
> > Throttling is what this driver is doing and with which it will interfere
> > and slow down the machine to be closed to unresponsive if both
> > (throttling and cpufreq interface) are using it?
> 
> The ACPI throttling interface will only be used if the minimum cpufreq 
> state has already been reached.
Yes and this is correct.
If there should really be a machine out there which can do real cpufreq
scaling and the p4-clockmod driver still works, real cpufreq scaling
should be preferred.
> Users really shouldn't play with the 
> throttling interface themselves, given its lack of useful functionality.
Right, this also applies for p4-clockmod. Therefore, if at all, this
should get moved to throttling. There it will be used for what it is
designed for: passive cooling to avoid critical temperatures. This is
done automatically through the trip points interface without the need of
user interaction (Yes, BIOSes do not define a passive trip point...,
this should hopefully be possible via userspace soon).

To be honest I do not care that much about removing this one. IMO things
should be cleaned up from time to time.
While ThinkPads, Acer, Dell seem to run quite hot currently with
powersaving capble CPUs, or cpufreq is broken or buggy on some, I am
tired discussing about p4-clockmod.
People waste time and file bugs on such unimportant things like
powersaving on a P4 and writing all this down and try to convince people
that p4-clockmod is not worth it is also not worth it. At least state
that this driver should not be used in distributions to not get
complains about this one any more or just remove it..., Dmitry can
compile it in for himself to save 2 degree and Pavel or whoever it was
with the broken battery... I mean does this still work, I expect the
battery is really broken now and you should be able to use this machine
on full performance again. As said p4-clockmod does only save if CPU is
utilized, but then you do not want throttling, only for critical
temperature reasons or broken batteries.

   Thomas