[PATCH][ACPI] AC/DC notifier

[PATCH][ACPI] AC/DC notifier

[Mark Langsdorf]

Add an ACPI event notifier for AC/DC connect/disconnect events.

Signed-off-by: Mark Langsdorf <mark.langsdorf@amd.com>

---
 drivers/acpi/ac.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)


diff --git a/drivers/acpi/ac.c b/drivers/acpi/ac.c
index 0df8fcb..656b903 100644
--- a/drivers/acpi/ac.c
+++ b/drivers/acpi/ac.c
@@ -243,6 +243,7 @@ static void acpi_ac_notify(struct acpi_device *device, u32 event)
 		acpi_bus_generate_netlink_event(device->pnp.device_class,
 						  dev_name(&device->dev), event,
 						  (u32) ac->state);
+		acpi_notifier_call_chain(device, event, (u32) ac->state);
 #ifdef CONFIG_ACPI_SYSFS_POWER
 		kobject_uevent(&ac->charger.dev->kobj, KOBJ_CHANGE);
 #endif
-- 
1.6.0.2

Re: [PATCH]AC/DC notifier

[Matthew Garrett]

On Tue, Aug 11, 2009 at 03:15:42PM -0500, Mark Langsdorf wrote:
> Add an ACPI event notifier for AC/DC connect/disconnect events.

Not that I inherently disagree with this, but what's it likely to be 
used for?

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH]AC/DC notifier

[Len Brown]

applied

thanks,
Len Brown, Intel Open Source Technology Center

Re: [PATCH]AC/DC notifier

[Matthew Garrett]

On Tue, Aug 11, 2009 at 08:51:49PM -0400, Tippett, Matthew wrote:

>    From a graphics perspective (your area of expertise), this will allow KMS
>    drivers to do some more intelligent actions based on the ac/dc state. 
>    Some examples of this could be improving the power consumption of the
>    graphics hardware through adapting clock memory/engine settings for
>    reduced power consumption, reducing refresh rate of the display to reduce
>    scanout memory access, adjusting backlight brightness, etc.

Right. As you say, my concern is that most of this should belong in 
userspace. Where we risk hardware damage there's an obvious argument for 
doing this in kernel, but we should ensure that that's limited to 
whatever coarse-grain handling is absolutely required rather than doing 
things like touching display brightness.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH][ACPI] AC/DC notifier

[Pavel Machek]

On Wed 2009-08-12 01:55:32, Matthew Garrett wrote:
> On Tue, Aug 11, 2009 at 08:51:49PM -0400, Tippett, Matthew wrote:
> 
> >    From a graphics perspective (your area of expertise), this will allow KMS
> >    drivers to do some more intelligent actions based on the ac/dc state. 
> >    Some examples of this could be improving the power consumption of the
> >    graphics hardware through adapting clock memory/engine settings for
> >    reduced power consumption, reducing refresh rate of the display to reduce
> >    scanout memory access, adjusting backlight brightness, etc.
> 
> Right. As you say, my concern is that most of this should belong in 
> userspace. Where we risk hardware damage there's an obvious argument for 
> doing this in kernel, but we should ensure that that's limited to 
> whatever coarse-grain handling is absolutely required rather than doing 
> things like touching display brightness.

Yep... Some may want to save power even when AC is online -- like when
running on UPS. Some may want max performmance even on battery. 
								Pavel 

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH]AC/DC notifier

[Willy Tarreau]

On Fri, Aug 14, 2009 at 06:32:33PM +0200, Pavel Machek wrote:
> On Wed 2009-08-12 01:55:32, Matthew Garrett wrote:
> > On Tue, Aug 11, 2009 at 08:51:49PM -0400, Tippett, Matthew wrote:
> > 
> > >    From a graphics perspective (your area of expertise), this will allow KMS
> > >    drivers to do some more intelligent actions based on the ac/dc state. 
> > >    Some examples of this could be improving the power consumption of the
> > >    graphics hardware through adapting clock memory/engine settings for
> > >    reduced power consumption, reducing refresh rate of the display to reduce
> > >    scanout memory access, adjusting backlight brightness, etc.
> > 
> > Right. As you say, my concern is that most of this should belong in 
> > userspace. Where we risk hardware damage there's an obvious argument for 
> > doing this in kernel, but we should ensure that that's limited to 
> > whatever coarse-grain handling is absolutely required rather than doing 
> > things like touching display brightness.
> 
> Yep... Some may want to save power even when AC is online -- like when
> running on UPS. Some may want max performmance even on battery. 

Wholeheartly agreed. IMHO, there's absolutely no relation between power
source and the expected performance. It's really frustrating when your
laptop becomes a snail on battery, as well as it's annoying to hear it
sound like a hairdryer when plugged to mains. This should only be the
user's choice. Mine automatically adjusts its frequency on demand,
regardless of the power source, which provides me with the best
experience. I think that all the tricks used to save power when running
on battery were invented by laptop makers to artificially show longer
lasting eventhough the machine sometimes becomes barely usable. For
instance, some of them dim the backlight so that you can't read anything
in full light, so you need a power prolongator to use them outside !

Also, with the new trend of laptops making use of huge power-hungry 3D
graphic chips which suck all the juice out of your battery in less than
two hours doing nothing, you'd better run at full speed when on battery
to save energy for CPU-bound tasks, because eventhough the CPU eats more
power, you significantly reduce the run time, thus the static consumption
(GPU, backlight, hard disk, ...).

Willy

Re: [PATCH]AC/DC notifier

[Tippett, Matthew]

(Resending as text-only - sorry)

Bringing this item back up again.

I am not suggesting that the application of any particular policy 
appears within the kernel or userspace or a secondary policy engine.   
In general I am also against codifying policy within drivers.

I am interested seeing the ACPI notifier mechanism expanded to allow 
AC/DC state changes propagate to other kernel drivers without requiring 
a userspace in between.

I can continue to come up with real scenarios that would possibly 
require kernel-to-kernel notification, but would rather focus this 
discussion of the pure technical issues associated with adding the 
notifier to the AC/DC ACPI subsystem. 

Remember it is a one line patch.

Regards,

Matthew

-------- Original Message  --------
Subject: Re: [PATCH][ACPI] AC/DC notifier
From: Willy Tarreau <w@1wt.eu>
To: Pavel Machek <pavel@ucw.cz>
Cc: "Matthew Garrett" <mjg59@srcf.ucam.org>, "Tippett, Matthew" 
<Matthew.Tippett@amd.com>, "Langsdorf, Mark" <mark.langsdorf@amd.com>, 
lenb@kernel.org, linux-acpi@vger.kernel.org, 
linux-kernel@vger.kernel.org, "Li, Samuel" <Samuel.Li@amd.com>
Date: 08/16/2009 03:40 AM
>
> On Fri, Aug 14, 2009 at 06:32:33PM +0200, Pavel Machek wrote:
> > On Wed 2009-08-12 01:55:32, Matthew Garrett wrote:
> > > On Tue, Aug 11, 2009 at 08:51:49PM -0400, Tippett, Matthew wrote:
> > >
> > > >    From a graphics perspective (your area of expertise), this 
> will allow KMS
> > > >    drivers to do some more intelligent actions based on the 
> ac/dc state.
> > > >    Some examples of this could be improving the power 
> consumption of the
> > > >    graphics hardware through adapting clock memory/engine 
> settings for
> > > >    reduced power consumption, reducing refresh rate of the 
> display to reduce
> > > >    scanout memory access, adjusting backlight brightness, etc.
> > >
> > > Right. As you say, my concern is that most of this should belong in
> > > userspace. Where we risk hardware damage there's an obvious 
> argument for
> > > doing this in kernel, but we should ensure that that's limited to
> > > whatever coarse-grain handling is absolutely required rather than 
> doing
> > > things like touching display brightness.
> >
> > Yep... Some may want to save power even when AC is online -- like when
> > running on UPS. Some may want max performmance even on battery.
>
> Wholeheartly agreed. IMHO, there's absolutely no relation between power
> source and the expected performance. It's really frustrating when your
> laptop becomes a snail on battery, as well as it's annoying to hear it
> sound like a hairdryer when plugged to mains. This should only be the
> user's choice. Mine automatically adjusts its frequency on demand,
> regardless of the power source, which provides me with the best
> experience. I think that all the tricks used to save power when running
> on battery were invented by laptop makers to artificially show longer
> lasting eventhough the machine sometimes becomes barely usable. For
> instance, some of them dim the backlight so that you can't read anything
> in full light, so you need a power prolongator to use them outside !
>
> Also, with the new trend of laptops making use of huge power-hungry 3D
> graphic chips which suck all the juice out of your battery in less than
> two hours doing nothing, you'd better run at full speed when on battery
> to save energy for CPU-bound tasks, because eventhough the CPU eats more
> power, you significantly reduce the run time, thus the static consumption
> (GPU, backlight, hard disk, ...).
>
> Willy
>
>

Re: [PATCH][ACPI] AC/DC notifier

[Pavel Machek]

On Tue 2009-10-06 10:53:22, Tippett, Matthew wrote:
> (Resending as text-only - sorry)
>
> Bringing this item back up again.
>
> I am not suggesting that the application of any particular policy  
> appears within the kernel or userspace or a secondary policy engine.    
> In general I am also against codifying policy within drivers.
>
> I am interested seeing the ACPI notifier mechanism expanded to allow  
> AC/DC state changes propagate to other kernel drivers without requiring  
> a userspace in between.
>
> I can continue to come up with real scenarios that would possibly  
> require kernel-to-kernel notification, but would rather focus this  
> discussion of the pure technical issues associated with adding the  
> notifier to the AC/DC ACPI subsystem. 

Please do. So far you did not show valid use for such notifier.

(Ok, I know of one. Old amd64 notebooks had cpufreq scaling enabled,
with battery unable to supply enough current to feed the CPU at
highest cpufreq setting. At that point, scaling cpufreq down at unplug
is correctness issue, and AC/DC notifier in kernel makes
sense.)

So... what do you want to use it for?
								Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH][ACPI] AC/DC notifier

[Dave Airlie]

On Wed, Oct 7, 2009 at 5:31 PM, Pavel Machek <pavel@ucw.cz> wrote:
> On Tue 2009-10-06 10:53:22, Tippett, Matthew wrote:
>> (Resending as text-only - sorry)
>>
>> Bringing this item back up again.
>>
>> I am not suggesting that the application of any particular policy
>> appears within the kernel or userspace or a secondary policy engine.
>> In general I am also against codifying policy within drivers.
>>
>> I am interested seeing the ACPI notifier mechanism expanded to allow
>> AC/DC state changes propagate to other kernel drivers without requiring
>> a userspace in between.
>>
>> I can continue to come up with real scenarios that would possibly
>> require kernel-to-kernel notification, but would rather focus this
>> discussion of the pure technical issues associated with adding the
>> notifier to the AC/DC ACPI subsystem.
>
> Please do. So far you did not show valid use for such notifier.
>
> (Ok, I know of one. Old amd64 notebooks had cpufreq scaling enabled,
> with battery unable to supply enough current to feed the CPU at
> highest cpufreq setting. At that point, scaling cpufreq down at unplug
> is correctness issue, and AC/DC notifier in kernel makes
> sense.)
>
> So... what do you want to use it for?

I'm not sure we have a open driver use case for this, if not I suggest
we remove it and only add it when a user is added to the kernel.

Dave.

Re: [PATCH]AC/DC notifier

[Matthew Garrett]

On Wed, Oct 07, 2009 at 06:16:29PM +1000, Dave Airlie wrote:

> I'm not sure we have a open driver use case for this, if not I suggest
> we remove it and only add it when a user is added to the kernel.

We'll need it once we add scaling support to radeon, so I don't see the 
harm with it being merged now.

-- 
Matthew Garrett | mjg59@srcf.ucam.org