From owner-freebsd-acpi@FreeBSD.ORG Fri Jul 8 12:20:50 2011 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 57D091065670 for ; Fri, 8 Jul 2011 12:20:50 +0000 (UTC) (envelope-from avg@FreeBSD.org) Received: from citadel.icyb.net.ua (citadel.icyb.net.ua [212.40.38.140]) by mx1.freebsd.org (Postfix) with ESMTP id 9E8588FC0A for ; Fri, 8 Jul 2011 12:20:49 +0000 (UTC) Received: from odyssey.starpoint.kiev.ua (alpha-e.starpoint.kiev.ua [212.40.38.101]) by citadel.icyb.net.ua (8.8.8p3/ICyb-2.3exp) with ESMTP id PAA11461; Fri, 08 Jul 2011 15:20:47 +0300 (EEST) (envelope-from avg@FreeBSD.org) Message-ID: <4E16F61E.80201@FreeBSD.org> Date: Fri, 08 Jul 2011 15:20:46 +0300 From: Andriy Gapon User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:5.0) Gecko/20110705 Thunderbird/5.0 MIME-Version: 1.0 To: Vitaly Magerya References: <4E05EB91.9090509@FreeBSD.org> <4E0862A0.7060405@FreeBSD.org> <4E09BADF.7050702@FreeBSD.org> <4E0A41C8.3000904@FreeBSD.org> <4E0CE158.6030804@FreeBSD.org> <4E0DB58F.4070906@FreeBSD.org> <4E130154.9080809@FreeBSD.org> <4E146FDB.2020602@FreeBSD.org> In-Reply-To: X-Enigmail-Version: 1.2pre Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Cc: freebsd-acpi@FreeBSD.org Subject: Re: (Missing) power states of an Atom N455-based netbook 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: Fri, 08 Jul 2011 12:20:50 -0000 on 06/07/2011 22:20 Vitaly Magerya said the following: > Actually, I have a simpler fix. We could allow setting hw.acpi.cx_lowest > to any value, including states that are not currently present. Then, > on updates to available Cx states, our ACPI code will automatically > set dev.cpu.N.cx_lowest to the closest valid value without the need > for a separate power_profile invocation. > > Here's the diff: > > --- acpi_cpu.c.orig 2011-07-05 19:50:31.000000000 +0000 > +++ acpi_cpu.c 2011-07-06 17:23:16.000000000 +0000 > @@ -1194,7 +1194,7 @@ > if (strlen(state) < 2 || toupper(state[0]) != 'C') > return (EINVAL); > val = (int) strtol(state + 1, NULL, 10) - 1; > - if (val < 0 || val > cpu_cx_count - 1) > + if (val < 0) > return (EINVAL); > cpu_cx_lowest = val; This change is a little bit more intrusive than I would like. There are some things about cpu_cx_lowest handling in the code that make me a bit unsure if this change is completely safe. I suspect that there could be problems on systems where number Cx states becomes smaller after some events (e.g. AC connection). I would prefer other developers to also comment on this. Maybe it's worth while opening a PR for this proposed change. > You can even simplify power_profile with this change: > > --- power_profile.orig 2011-07-06 18:39:27.000000000 +0000 > +++ power_profile 2011-07-06 18:40:20.000000000 +0000 > @@ -81,8 +81,7 @@ > # Set the various sysctls based on the profile's values. > node="hw.acpi.cpu.cx_lowest" > highest_value="C1" > -lowest_value="`(sysctl -n dev.cpu.0.cx_supported | \ > - awk '{ print "C" split($0, a) }' -) 2> /dev/null`" > +lowest_value="C99" > eval value=\$${profile}_cx_lowest > sysctl_set C99 looks too scary (and too familiar) :-) I think that C6 would be sufficient here. -- Andriy Gapon