powerd doesn't decrease CPU frequency in some cases
Takeharu KATO
takeharu1219 at ybb.ne.jp
Fri Jan 4 19:22:07 PST 2008
Hi,
Sorry, I sent wrong patch, please apply this instead.
Takeharu KATO wrote:
> Hi,
>
> I met same problem on my Panasonic CF-R7 note book PC.
> As far as I investigate, cpufreq(est driver) does not check setting
> values for PERFCTL register which are reported from acpi_perf driver.
>
> According to comments in sys/i386/cpufreq/est.c,
> the auther of the driver regards this as TODO thing
> (see following lines.).
>
> -- sys/i386/cpufreq/est.c
> for (i = 0; i < count; i++) {
> /*
> * TODO: Figure out validity checks for id16. Linux checks
> * that the control and status values match.
> */
> --
>
> I wrote the patch to check id16 values as comments says.
> As far as I test the patch, the patch fixes the problem.
>
> Would you try this patch?
>
> P.S. I've send-pr this problem with this patch in this
> morning(kern/119350).
>
> Andrey wrote:
>> Good time of the day.
>>
>> I've noticed that powerd isn't able to decrease CPU frequency on my
>> laptop (HP Compaq 6710b) as soon as frequency gets highest level.
>>
>> I've pottered a bit in the sources and it seems found the root of the
>> issue.
>> So those who are interested in the subject let consider it.
>>
>> For instance my system reports the following frequency levels:
>>
>> [silent at beastie][/home/silent]sysctl dev.cpu.0.freq_levels
>> dev.cpu.0.freq_levels: 2001/35000 2000/35000 1750/30625 1600/25000
>> 1400/21875 1200/16000 1050/14000 900/12000 800/14000 700/12250
>> 600/10500 500/8750 400/7000 300/5250
>>
>> If I try to adjust current frequency to 2000 MHz then I'll get:
>> [silent at beastie][/home/silent]sudo sysctl dev.cpu.0.freq=2000
>> dev.cpu.0.freq: 300 -> 2001
>> Let check:
>> [silent at beastie][/home/silent]sysctl dev.cpu.0.freq
>> dev.cpu.0.freq: 2001
>>
>> Thus, as you can see, I have level "2000" which system reports me but
>> actually I can't to adjust those one exactly because it silently
>> becomes "2001"
>>
>> Well... If I'm not mistaken powerd calculates the current "CPU idle
>> mark" and if it is more then adopted value (by default 90%) then it
>> shifts CPU frequency value 1 step down. In my case powerd sticks at
>> "2001". It is obvious because when powerd decreases CPU frequency from
>> the highest frequency level we'll get the following scenario:
>>
>> +-----------------------+
>> | dev.cpu.0.freq=2001 +--<-+
>> +-----------+-----------+ |
>> | |
>> Y |
>> +-----------+-----------+ |
>> | "CPU idle" > 90% | |
>> +-----------+-----------+ |
>> | |
>> Y ^
>> +-----------+-----------+ ^
>> | powerd shifts freq. | ^
>> | 1 step down: | |
>> | "2001" -> "2000" | |
>> +-----------+-----------+ |
>> | |
>> Y |
>> +-----------+-----------+ |
>> | actually we have here | |
>> | dev.cpu.0.freq == 2001+-->-+
>> +-----------------------+
>>
>>
>> According to the things mentioned above I've came to conclusion that
>> in my case it is not a good idea to rely on frequency levels reported
>> by the system. (Also I saw many sysctl mibs (dev.cpu.0.freq) of many
>> other people. And there were "strange" frequency levels like "2000"
>> and "2001". Of course I can't state that their systems' behaviors fit
>> my case. But still...)
>>
>> So the simple way out I see is to teach powerd recognize "fake"
>> frequency levels. Here I suggest a very simple workaround (and may be
>> quite ugly... sorry I'm not sure if it is my cup of tee) which allows
>> me to overcome the issue. And I hope it can be useful for smb. else.
>>
>> Also I'd like to hear opinions of others. May be there exists another
>> and simpler way to overcome an issue or even I've missed something or
>> not aware of something.
>>
>>
>> Thank you.
>>
>>
>> --
>> Sincerely,
>> Andrey Kosachenko
>>
>>
>> ------------------------------------------------------------------------
>>
>> _______________________________________________
>> freebsd-acpi at freebsd.org mailing list
>> http://lists.freebsd.org/mailman/listinfo/freebsd-acpi
>> To unsubscribe, send any mail to "freebsd-acpi-unsubscribe at freebsd.org"
>
>
> ------------------------------------------------------------------------
>
> _______________________________________________
> freebsd-acpi at freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-acpi
> To unsubscribe, send any mail to "freebsd-acpi-unsubscribe at freebsd.org"
-------------- next part --------------
--- sys.orig/i386/cpufreq/est.c 2006-05-11 17:35:44.000000000 +0000
+++ sys/i386/cpufreq/est.c 2008-01-05 12:08:41.359933175 +0000
@@ -902,6 +902,8 @@
static int est_set(device_t dev, const struct cf_setting *set);
static int est_get(device_t dev, struct cf_setting *set);
static int est_type(device_t dev, int *type);
+static int est_set_id16(uint16_t id16, int need_check);
+static void est_get_id16(uint16_t *id16_p);
static device_method_t est_methods[] = {
/* Device interface */
@@ -1068,7 +1070,6 @@
return (0);
}
-
static int
est_acpi_info(device_t dev, freq_info **freqs)
{
@@ -1076,7 +1077,8 @@
struct cf_setting *sets;
freq_info *table;
device_t perf_dev;
- int count, error, i;
+ int count, error, i, idx;
+ uint16_t saved_id16;
perf_dev = device_find_child(device_get_parent(dev), "acpi_perf", -1);
if (perf_dev == NULL || !device_is_attached(perf_dev))
@@ -1098,19 +1100,38 @@
error = ENOMEM;
goto out;
}
- for (i = 0; i < count; i++) {
+ for (i = 0, idx = 0; i < count; i++) {
/*
- * TODO: Figure out validity checks for id16. Linux checks
- * that the control and status values match.
+ * Confirm id16 value is correct.
*/
- table[i].freq = sets[i].freq;
- table[i].volts = sets[i].volts;
- table[i].id16 = sets[i].spec[0];
- table[i].power = sets[i].power;
+ if (sets[i].freq > 0) {
+
+ /* save current value */
+ est_get_id16(&saved_id16);
+
+ /*
+ * Try to set specified value
+ */
+ error = est_set_id16(sets[i].spec[0], 1);
+ if (error != 0) {
+ if (bootverbose)
+ printf("Invalid freq %u, ignored.\n",
+ sets[i].freq);
+ } else {
+ table[idx].freq = sets[i].freq;
+ table[idx].volts = sets[i].volts;
+ table[idx].id16 = sets[i].spec[0];
+ table[idx].power = sets[i].power;
+ ++idx;
+ }
+
+ /* restore saved setting */
+ est_set_id16(sets[i].spec[0], 0);
+ }
}
/* Mark end of table with a terminator. */
- bzero(&table[i], sizeof(freq_info));
+ bzero(&table[idx], sizeof(freq_info));
sc->acpi_settings = TRUE;
*freqs = table;
@@ -1148,6 +1169,41 @@
*freqs = p->freqtab;
return (0);
}
+static void
+est_get_id16(uint16_t *id16_p) {
+ *id16_p = rdmsr(MSR_PERF_STATUS) & 0xffff;
+}
+
+static int
+est_set_id16(uint16_t id16, int need_check) {
+ uint64_t msr;
+ uint16_t new_id16;
+ int rc = 0;
+
+ /*
+ * Try to set freq.
+ */
+
+ /* Read the current register, mask out the old, set the new id. */
+ msr = rdmsr(MSR_PERF_CTL);
+ msr = (msr & ~0xffff) | id16;
+ wrmsr(MSR_PERF_CTL, msr);
+
+ /* Wait a short while for the new setting. XXX Is this necessary? */
+ DELAY(EST_TRANS_LAT);
+
+ if (need_check) {
+ est_get_id16(&new_id16);
+ if (new_id16 != id16) {
+ if (bootverbose)
+ printf("Invalid id16 (set, cur) = (%u, %u)\n",
+ id16, new_id16);
+ rc = ENXIO; /* Can not set this value */
+ }
+ }
+
+ return (rc);
+}
static freq_info *
est_get_current(freq_info *freq_list)
@@ -1162,7 +1218,7 @@
* we get a temporary invalid result.
*/
for (i = 0; i < 5; i++) {
- id16 = rdmsr(MSR_PERF_STATUS) & 0xffff;
+ est_get_id16(&id16);
for (f = freq_list; f->id16 != 0; f++) {
if (f->id16 == id16)
return (f);
@@ -1201,7 +1257,6 @@
{
struct est_softc *sc;
freq_info *f;
- uint64_t msr;
/* Find the setting matching the requested one. */
sc = device_get_softc(dev);
@@ -1213,9 +1268,7 @@
return (EINVAL);
/* Read the current register, mask out the old, set the new id. */
- msr = rdmsr(MSR_PERF_CTL);
- msr = (msr & ~0xffff) | f->id16;
- wrmsr(MSR_PERF_CTL, msr);
+ est_set_id16(f->id16, 0);
/* Wait a short while for the new setting. XXX Is this necessary? */
DELAY(EST_TRANS_LAT);
More information about the freebsd-acpi
mailing list