cpufreq Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Renninger <trenn@suse.de>
To: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Dave Jones <davej@codemonkey.org.uk>,
	mzahor@dtech.sk, Dominik Brodowski <linux@dominikbrodowski.net>,
	cpufreq <cpufreq@lists.linux.org.uk>
Subject: Re: [PATCH] Remove p4_clockmode driver
Date: Fri, 23 May 2008 13:21:18 +0200	[thread overview]
Message-ID: <1211541678.29901.78.camel@queen.suse.de> (raw)
In-Reply-To: <20080522040118.GA28737@anvil.corenet.prv>

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

  reply	other threads:[~2008-05-23 11:21 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-05-03 19:47 [PATCH] Remove p4_clockmode driver Thomas Renninger
2008-05-11 16:19 ` Dave Jones
2008-05-15 10:55   ` Dominik Brodowski
2008-05-16 18:32   ` Pallipadi, Venkatesh
2008-05-22  4:01   ` Dmitry Torokhov
2008-05-23 11:21     ` Thomas Renninger [this message]
2008-05-23 12:55       ` Dmitry Torokhov
2008-05-23 15:19         ` Thomas Renninger
2008-05-23 15:54           ` Dmitry Torokhov
2008-05-23 16:16             ` Thomas Renninger
2008-05-23 17:40               ` Dmitry Torokhov
2008-05-24 11:03                 ` Dominik Brodowski
2008-05-25 15:39                   ` Thomas Renninger
2008-05-26 12:05                     ` Matthew Garrett
2008-05-26 12:56                       ` Thomas Renninger
2008-05-12 22:30 ` Cesar Eduardo Barros

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=1211541678.29901.78.camel@queen.suse.de \
    --to=trenn@suse.de \
    --cc=cpufreq@lists.linux.org.uk \
    --cc=davej@codemonkey.org.uk \
    --cc=dmitry.torokhov@gmail.com \
    --cc=linux@dominikbrodowski.net \
    --cc=mzahor@dtech.sk \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox