From owner-freebsd-acpi@FreeBSD.ORG Sat Jan 5 03:22:06 2008 Return-Path: Delivered-To: freebsd-acpi@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id EB15416A46D for ; Sat, 5 Jan 2008 03:22:06 +0000 (UTC) (envelope-from takeharu1219@ybb.ne.jp) Received: from ybbsmtp08.mail.ogk.yahoo.co.jp (ybbsmtp08.mail.ogk.yahoo.co.jp [124.83.153.128]) by mx1.freebsd.org (Postfix) with SMTP id 8C1F913C44B for ; Sat, 5 Jan 2008 03:22:06 +0000 (UTC) (envelope-from takeharu1219@ybb.ne.jp) Received: (qmail 25198 invoked by alias); 5 Jan 2008 03:22:05 -0000 DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=ybb20050223; d=ybb.ne.jp; h=Received:X-Apparently-From:Message-ID:Date:From:User-Agent:MIME-Version:To:CC:Subject:References:In-Reply-To:Content-Type; b=QIgzbM9fN+7BRGnCXqnqc+XEk/GbMVq7MCHUFRE1AugQGgwsWQgdIagN8uRV1KOaQKFslCoMLnuzWnRm52OE/4BgyMGFkqapQJsLORCNavwe5OkjFNazjGuZ85cl9vtA ; Received: from unknown (HELO ?127.0.0.1?) (takeharu1219@219.35.170.86 with plain) by ybbsmtp08.mail.ogk.yahoo.co.jp with SMTP; 5 Jan 2008 03:22:05 -0000 X-Apparently-From: Message-ID: <477EF7BD.8040800@ybb.ne.jp> Date: Sat, 05 Jan 2008 12:21:33 +0900 From: Takeharu KATO User-Agent: Thunderbird 1.5.0.9 (Windows/20061207) MIME-Version: 1.0 To: Takeharu KATO References: <476E8674.5000303@gmail.com> <477EF09E.60606@ybb.ne.jp> In-Reply-To: <477EF09E.60606@ybb.ne.jp> Content-Type: multipart/mixed; boundary="------------050906000808070009080005" Cc: freebsd-acpi@FreeBSD.org Subject: Re: powerd doesn't decrease CPU frequency in some cases X-BeenThere: freebsd-acpi@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: ACPI and power management development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 05 Jan 2008 03:22:07 -0000 This is a multi-part message in MIME format. --------------050906000808070009080005 Content-Type: text/plain; charset=windows-1251; format=flowed Content-Transfer-Encoding: 7bit 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@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@beastie][/home/silent]sudo sysctl dev.cpu.0.freq=2000 >> dev.cpu.0.freq: 300 -> 2001 >> Let check: >> [silent@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@freebsd.org mailing list >> http://lists.freebsd.org/mailman/listinfo/freebsd-acpi >> To unsubscribe, send any mail to "freebsd-acpi-unsubscribe@freebsd.org" > > > ------------------------------------------------------------------------ > > _______________________________________________ > freebsd-acpi@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-acpi > To unsubscribe, send any mail to "freebsd-acpi-unsubscribe@freebsd.org" --------------050906000808070009080005 Content-Type: text/plain; name="est-fix-v2.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="est-fix-v2.patch" --- 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); --------------050906000808070009080005--