Bug 144232

Summary: [cpufreq] [patch] Add debug.cpufreq.highest to cpufreq
Product: Base System Reporter: Boris Kochergin <spawk>
Component: kernAssignee: freebsd-acpi (Nobody) <acpi>
Status: Closed FIXED    
Severity: Affects Only Me    
Priority: Normal    
Version: 8.0-RELEASE   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
file.diff none

Description Boris Kochergin 2010-02-23 14:40:00 UTC
I request that support for the debug.cpufreq.highest sysctl, which places an upper bound on the frequencies returned to users, be added to cpufreq. I am currently using it in conjunction with the dev.acpi_ibm.0.fan_level sysctl from the acpi_ibm module set to 0 to keep a laptop both cool and quiet while it has a broken fan. Others have also found uses for the functionality (http://forums.freebsd.org/showthread.php?t=172).

Fix: Patch attached with submission follows:
Comment 1 Bruce Cran freebsd_committer freebsd_triage 2010-03-25 08:29:06 UTC
Responsible Changed
From-To: freebsd-bugs->freebsd-acpi

Over to maintainer(s).
Comment 2 Dan Lukes 2010-03-25 10:04:15 UTC
It sound like improper place for implementation of such logic.

Cpufreq is hardware driver - it allow others to control CPU speeds. It 
do no own decisions nor should do (imho). When it should not do 
decisions, then it's not appropriate place to store variables that exist 
for the purpose of such decision process only.

cpufreq consumers (like powerd or acpi_thermal) are there for decision 
making so such logic and configuration variables should be there.

The debug.cpufreq.lowest is here because some reported levels are not 
usable in the real, not because someone decided he don't want to use it.

You may be interested that requested feature is implemented as part of 
acpi_thermal. From man acpi_thermal:

hw.acpi.thermal.tz%d.passive_cooling
     If set to 1, passive cooling is enabled.  It does cooling without
     fans using cpufreq(4) as the mechanism for controlling CPU speed.
     Default is enabled for tz0 where it is available.

It require support from ACPI on your notebook which may or may not be 
present. If such support is not present, so acpi_thermal can't help you, 
then another "frequency decision" utility - e.g. - powerd - is 
candidate-place to implement requested logic. No logic should belong to 
cpufreq device driver itself, so no tunables for them there.

I noticed the argument "maximum on AC is another than maximum on 
battery", but power state is available to powerd, so the logic we are 
speaking about can count the power state as well. The only question is - 
how to tell to powerd what we want from it exactly.

						Dan
Comment 3 Nate Lawson 2010-03-25 16:06:04 UTC
Dan Lukes wrote:
 >  It sound like improper place for implementation of such logic.
>  
>  Cpufreq is hardware driver - it allow others to control CPU speeds. It 
>  do no own decisions nor should do (imho). When it should not do 
>  decisions, then it's not appropriate place to store variables that exist 
>  for the purpose of such decision process only.
>  
>  cpufreq consumers (like powerd or acpi_thermal) are there for decision 
>  making so such logic and configuration variables should be there.
>  
>  The debug.cpufreq.lowest is here because some reported levels are not 
>  usable in the real, not because someone decided he don't want to use it.

Exactly right. The "lowest" sysctl was there to prevent use of modes
that users said froze their laptop. It is not for scheduling/general
policy decisions. There is no reason for "highest" as this is a
scheduling decision. Such logic should be in powerd and such control
programs.

-- 
Nate
Comment 4 Nate Lawson 2010-03-25 18:08:52 UTC
Boris Kochergin wrote:
> 
> OK. I also have code to implement -m and -M (minimum and maximum
> frequency, respectively) options in powerd:
> 
> http://acm.poly.edu/~spawk/powerd/
> 
> It's for 7.0-RELEASE, so I will see if it needs to be brought up to date
> and will file a PR.

Seems reasonable. Needs a little whitespace cleanup.

-- 
Nate
Comment 5 Boris Kochergin 2010-03-26 17:02:33 UTC
This PR can be closed. A new one with modernized patches to add support 
for this in powerd is at bin/145063.

-Boris
Comment 6 Joerg Wunsch freebsd_committer freebsd_triage 2010-03-26 20:03:30 UTC
State Changed
From-To: open->closed

Closed upon Boris' request.