Generic in-kernel AC status
Subsystems (4)
RFC PATCH 1/3 Generic in-kernel AC status
The included patch adds support for power management methods to register callbacks in order to allow drivers to check if the system is on AC or not. Following patches add support to ACPI and APM. Feedback welcome.
include/linux/pm.h +2 -0
@@ -129,6 +129,8 @@ struct pm_ops {extern void pm_set_ops (struct pm_ops *);extern struct pm_ops *pm_ops; +extern void pm_set_ac_status (int (*ac_status_function)(void )); +extern int pm_get_ac_status (void );extern int pm_suspend (suspend_state_t state);
kernel/power/main.c +34 -0
@@ -25,6 +25,7 @@DECLARE_MUTEX (pm_sem);struct pm_ops *pm_ops =NULL ; +int (*get_ac_status)(void );suspend_disk_method_t pm_disk_mode =PM_DISK_SHUTDOWN ;/**
@@ -39,6 +40,39 @@ void pm_set_ops(struct pm_ops * ops)up (&pm_sem); } +/** +* pm_set_ac_status - Set the ac status callback +* @ops: Pointer to function +*/ + +void pm_set_ac_status (int (*ac_status_function)(void )) +{ +down (&pm_sem); + get_ac_status = ac_status_function; +up (&pm_sem); +} + +EXPORT_SYMBOL (pm_set_ac_status); + +/** +* pm_get_ac_status - return true if the system is on AC +*/ + +int pm_get_ac_status (void ) +{ +int status; + +down (&pm_sem); +if (get_ac_status) + status =get_ac_status (); +else + status =0 ; +up (&pm_sem); + +return status; +} + +EXPORT_SYMBOL (pm_get_ac_status);/** * suspend_prepare - Do prep work before entering low-power state.
Signature · 1 line
Matthew Garrett | [email protected]
RFC PATCH 2/3 ACPI support for generic in-kernel AC status
The included patch adds support for the generic in-kernel AC status code. Each AC adapter device is added to a list - when queried, if at least one claims to be online then we assume that the system is on AC.
drivers/acpi/ac.c +27 -0
@@ -29,6 +29,7 @@#include <linux/types.h> #include <linux/proc_fs.h> #include <linux/seq_file.h> +#include <linux/pm.h> #include <acpi/acpi_bus.h> #include <acpi/acpi_drivers.h>
@@ -67,8 +68,11 @@ static struct acpi_driver acpi_ac_driverstruct acpi_ac {acpi_handle handle;unsigned long state; +struct list_head entry; }; +static struct list_head ac_adapter_list; +static struct file_operations acpi_ac_fops = { .open = acpi_ac_open_fs, .read = seq_read,
@@ -255,6 +259,8 @@ static int acpi_ac_add(struct acpi_devic goto end; } +list_add (&ac->entry, &ac_adapter_list); + printk(KERN_INFO PREFIX "%s [%s] (%s)\n",acpi_device_name (device),acpi_device_bid (device), ac->state ? "on-line" : "off-line");
@@ -288,17 +294,34 @@ static int acpi_ac_remove(struct acpi_deacpi_ac_remove_fs (device); +list_del (&ac->entry); +kfree (ac);return_VALUE (0 ); } +static int acpi_ac_online_status (void ) +{ +struct acpi_ac *adapter; + +list_for_each_entry (adapter, &ac_adapter_list, entry) { +acpi_ac_get_state (adapter); +if (adapter->state) +return 1 ; + } + +return 0 ; +} +static int __init acpi_ac_init (void ) {int result =0 ;ACPI_FUNCTION_TRACE ("acpi_ac_init" ); +INIT_LIST_HEAD (&ac_adapter_list); + acpi_ac_dir =proc_mkdir (ACPI_AC_CLASS , acpi_root_dir);if (!acpi_ac_dir)return_VALUE (-ENODEV );
@@ -310,6 +333,8 @@ static int __init acpi_ac_init(void)return_VALUE (-ENODEV ); } +pm_set_ac_status (acpi_ac_online_status); +return_VALUE (0 ); }
@@ -317,6 +342,8 @@ static void __exit acpi_ac_exit(void) {ACPI_FUNCTION_TRACE ("acpi_ac_exit" ); +pm_set_ac_status (NULL ); +acpi_bus_unregister_driver (&acpi_ac_driver);remove_proc_entry (ACPI_AC_CLASS , acpi_root_dir);
Signature · 1 line
Matthew Garrett | [email protected]
Whoops - missing <linux/module.h>. Here's the fixed version.
include/linux/pm.h +2 -0
@@ -129,6 +129,8 @@ struct pm_ops {extern void pm_set_ops (struct pm_ops *);extern struct pm_ops *pm_ops; +extern void pm_set_ac_status (int (*ac_status_function)(void )); +extern int pm_get_ac_status (void );extern int pm_suspend (suspend_state_t state);
kernel/power/main.c +35 -1
@@ -15,7 +15,7 @@#include <linux/errno.h> #include <linux/init.h> #include <linux/pm.h> - +#include <linux/module.h> #include "power.h"
@@ -25,6 +25,7 @@DECLARE_MUTEX (pm_sem);struct pm_ops *pm_ops =NULL ; +int (*get_ac_status)(void );suspend_disk_method_t pm_disk_mode =PM_DISK_SHUTDOWN ;/**
@@ -39,6 +40,39 @@ void pm_set_ops(struct pm_ops * ops)up (&pm_sem); } +/** +* pm_set_ac_status - Set the ac status callback +* @ops: Pointer to function +*/ + +void pm_set_ac_status (int (*ac_status_function)(void )) +{ +down (&pm_sem); + get_ac_status = ac_status_function; +up (&pm_sem); +} + +EXPORT_SYMBOL (pm_set_ac_status); + +/** +* pm_get_ac_status - return true if the system is on AC +*/ + +int pm_get_ac_status (void ) +{ +int status; + +down (&pm_sem); +if (get_ac_status) + status =get_ac_status (); +else + status =0 ; +up (&pm_sem); + +return status; +} + +EXPORT_SYMBOL (pm_get_ac_status);/** * suspend_prepare - Do prep work before entering low-power state.
Signature · 1 line
Matthew Garrett | [email protected]
RFC PATCH 3/3 APM support for generic in-kernel AC status
This adds APM support for the generic in-kernel AC status code.
arch/i386/kernel/apm.c +17 -0
@@ -1627,6 +1627,18 @@ static int do_open(struct inode * inode,return 0 ; } +static int apm_get_online_status (void ) +{ +unsigned short bx; +unsigned short cx; +unsigned short dx; + +if (apm_get_power_status (&bx, &cx, &dx)) +return 0 ; + +return ((bx >>8 ) &0xff ); +}; +static int apm_get_info (char *buf,char **start,off_t fpos,int length) {char * p;
@@ -2372,6 +2384,8 @@ static int __init apm_init(void)misc_register (&apm_device); +pm_set_ac_status (apm_get_online_status); +if (HZ !=100 ) idle_period = (idle_period *HZ ) /100 ;if (idle_threshold <100 ) {
@@ -2396,6 +2410,9 @@ static void __exit apm_exit(void)*/ cpu_idle_wait (); } + +pm_set_ac_status (NULL ); +if (((apm_info.bios.flags &APM_BIOS_DISENGAGED ) ==0 ) && (apm_info.connection_version >0x0100 )) { error =apm_engage_power_management (APM_DEVICE_ALL ,0 );
Signature · 1 line
Matthew Garrett | [email protected]
kernel/power/main.c void pm_set_ops(struct pm_ops * ops) · line 48
Quoted 6 lines · Matthew Garrett
kernel/power/main.c +6
-0+/** +* pm_set_ac_status - Set the ac status callback +* @ops: Pointer to function +*/ + +void pm_set_ac_status (int (*ac_status_function)(void ))
No extra line in there please.
And perhaps a bit more description of what this is used for?
kernel/power/main.c void pm_set_ops(struct pm_ops * ops) · line 50
Quoted 2 lines
kernel/power/main.c +2
-0+{ +down (&pm_sem);
Shouldn't this be a mutex?
kernel/power/main.c void pm_set_ops(struct pm_ops * ops) · line 55
Quoted 5 lines
kernel/power/main.c +5
-0+ get_ac_status = ac_status_function; +up (&pm_sem); +} + +EXPORT_SYMBOL (pm_set_ac_status);
No extra line between the function and the EXPORT_SYMBOL please.
Also, how about EXPORT_SYMBOL_GPL()?
And, who will be using this interface, and what for?
Quoted 5 lines
kernel/power/main.c +5
-0+ +/** +* pm_get_ac_status - return true if the system is on AC +*/ +
kernel/power/main.c void pm_set_ops(struct pm_ops * ops) · line 70
Quoted 10 lines
kernel/power/main.c +10
-0+int pm_get_ac_status (void ) +{ +int status; + +down (&pm_sem); +if (get_ac_status) + status =get_ac_status (); +else + status =0 ; +up (&pm_sem);
You can save 2 lines by setting status = 0 on the first line of this function :)
thanks,
greg k-h
Quoted 3 lines · Greg KH
Quoted 1 line
+void
pm_set_ac_status(int (*ac_status_function)(void))No extra line in there please.
That matches the style used in the rest of the file.
Quoted 1 line
And perhaps a bit more description of what this is used for?
Ok.
Quoted 4 lines
Review
Quoted 2 lines
(quoted diff) +2
-0+{ + down(&pm_sem);Shouldn't this be a mutex?
It is, isn't it? The name's somewhat misleading, but I was just using what already existed...
Quoted 7 lines
Review
Quoted 5 lines
(quoted diff) +5
-0+ get_ac_status = ac_status_function; + up(&pm_sem); +} + +EXPORT_SYMBOL(pm_set_ac_status);No extra line between the function and the
EXPORT_SYMBOLplease.
Sure.
Quoted 1 line
Also, how about
EXPORT_SYMBOL_GPL()?
If that's considered reasonable, sure.
Quoted 1 line
And, who will be using this interface, and what for?
The backlight interface only supports exporting and setting the current brightness. For various bits of hardware, the AC and DC brightnesses are stored separately. Drivers would need to know which brightness value to export to userspace. I have an HP backlight driver here which would benefit from this, and I'm looking at the same issue for a Panasonic one.
Here's an updated version, anyway.
include/linux/pm.h +2 -0
@@ -129,6 +129,8 @@ struct pm_ops {extern void pm_set_ops (struct pm_ops *);extern struct pm_ops *pm_ops; +extern void pm_set_ac_status (int (*ac_status_function)(void )); +extern int pm_get_ac_status (void );extern int pm_suspend (suspend_state_t state);
kernel/power/main.c +32 -1
@@ -15,7 +15,7 @@#include <linux/errno.h> #include <linux/init.h> #include <linux/pm.h> - +#include <linux/module.h> #include "power.h"
@@ -25,6 +25,7 @@DECLARE_MUTEX (pm_sem);struct pm_ops *pm_ops =NULL ; +int (*get_ac_status)(void );suspend_disk_method_t pm_disk_mode =PM_DISK_SHUTDOWN ;/**
@@ -39,6 +40,36 @@ void pm_set_ops(struct pm_ops * ops)up (&pm_sem); } +/** +* pm_set_ac_status - Set the ac status callback. Returns true if the +* system is on AC and has a registered callback. +* @ops: Pointer to function +*/ + +void pm_set_ac_status (int (*ac_status_function)(void )) +{ +down (&pm_sem); + get_ac_status = ac_status_function; +up (&pm_sem); +} +EXPORT_SYMBOL_GPL (pm_set_ac_status); + +/** +* pm_get_ac_status - return true if the system is on AC +*/ + +int pm_get_ac_status (void ) +{ +int status=0 ; + +down (&pm_sem); +if (get_ac_status) + status =get_ac_status (); +up (&pm_sem); + +return status; +} +EXPORT_SYMBOL_GPL (pm_get_ac_status);/** * suspend_prepare - Do prep work before entering low-power state.
Signature · 1 line
Matthew Garrett | [email protected]
Quoted 8 lines · Matthew Garrett
Quoted 4 lines · Greg KH
kernel/power/main.c void pm_set_ops(struct pm_ops * ops) · line 51
Quoted 2 lines
kernel/power/main.c +2
-0+{ +down (&pm_sem);Shouldn't this be a mutex?
It is, isn't it? The name's somewhat misleading, but I was just using what already existed...
Ah, ok, nevermind, I thought this was new.
Quoted 3 lines
+/**
+ *pm_set_ac_status- Set the ac status callback. Returns true if the
+ * system is on AC and has a registered callback.
kerneldoc will not work with this. It needs to be a one line, short, description. Put the full description below the function paramaters, it can be as many lines as you want there.
thanks,
greg k-h
Quoted 3 lines · Greg KH
kerneldoc will not work with this. It needs to be a one line, short, description. Put the full description below the function paramaters, it can be as many lines as you want there.
Ok, how's this one?
include/linux/pm.h +2 -0
@@ -129,6 +129,8 @@ struct pm_ops {extern void pm_set_ops (struct pm_ops *);extern struct pm_ops *pm_ops; +extern void pm_set_ac_status (int (*ac_status_function)(void )); +extern int pm_get_ac_status (void );extern int pm_suspend (suspend_state_t state);
kernel/power/main.c +37 -1
@@ -15,7 +15,7 @@#include <linux/errno.h> #include <linux/init.h> #include <linux/pm.h> - +#include <linux/module.h> #include "power.h"
@@ -25,6 +25,7 @@DECLARE_MUTEX (pm_sem);struct pm_ops *pm_ops =NULL ; +int (*get_ac_status)(void );suspend_disk_method_t pm_disk_mode =PM_DISK_SHUTDOWN ;/**
@@ -39,6 +40,41 @@ void pm_set_ops(struct pm_ops * ops)up (&pm_sem); } +/** +* pm_set_ac_status - Set the ac status callback. +* @ops: Pointer to function +* +* Provide a callback that returns true if the system is currently on +* AC power. This should be called by power management subsystems. +*/ + +void pm_set_ac_status (int (*ac_status_function)(void )) +{ +down (&pm_sem); + get_ac_status = ac_status_function; +up (&pm_sem); +} +EXPORT_SYMBOL_GPL (pm_set_ac_status); + +/** +* pm_get_ac_status - return true if the system is on AC +* +* Returns true if the system has a registered callback for determining +* AC state and is currently on AC power, false otherwise. +*/ + +int pm_get_ac_status (void ) +{ +int status=0 ; + +down (&pm_sem); +if (get_ac_status) + status =get_ac_status (); +up (&pm_sem); + +return status; +} +EXPORT_SYMBOL_GPL (pm_get_ac_status);/** * suspend_prepare - Do prep work before entering low-power state.
Signature · 1 line
Matthew Garrett | [email protected]
Quoted 7 lines · Matthew Garrett
+/**
+ *pm_set_ac_status- Set the ac status callback.
+ * @ops: Pointer to function
+ *
+ * Provide a callback that returns true if the system is currently on
+ * AC power. This should be called by power management subsystems.
+ */
Mix of tabs and spaces here. Just use 1 space after the "*" character.
thanks,
greg k-h
Quoted 6 lines · Matthew Garrett
The backlight interface only supports exporting and setting the current brightness. For various bits of hardware, the AC and DC brightnesses are stored separately. Drivers would need to know which brightness value to export to userspace. I have an HP backlight driver here which would benefit from this, and I'm looking at the same issue for a Panasonic one.
I don't know the backlight interface but extending it to export all available brightness values would seem more logical to me.
If I'd had a laptop I'd hate if I could only set the DC brightness if I plug out the AC power.
Gabor
Signature · 4 lines
---------------------------------------------------------
MTA SZTAKI Computer and Automation Research Institute
Hungarian Academy of Sciences
---------------------------------------------------------Quoted 9 lines · Gabor Gombas
Quoted 6 lines · Matthew Garrett
The backlight interface only supports exporting and setting the current brightness. For various bits of hardware, the AC and DC brightnesses are stored separately. Drivers would need to know which brightness value to export to userspace. I have an HP backlight driver here which would benefit from this, and I'm looking at the same issue for a Panasonic one.
Quoted 5 lines
I don't know the backlight interface but extending it to export all available brightness values would seem more logical to me.
If I'd had a laptop I'd hate if I could only set the DC brightness if I plug out the AC power.
Still "set current brightness" operation makes a lot of sense.
Pavel
Signature · 5 lines
Web maintainer for suspend.sf.net (www.sf.net/projects/suspend) wanted... - To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 1 line · Pavel Machek
Still "set current brightness" operation makes a lot of sense.
Yes, but userspace already knows if you are on AC power or not, therefore it can decide what "current" means. If this is the only reason then adding a new kernel infrastructure is overkill.
Also, doing things differently when on AC power smells like a policy decision, and AFAIK policy handling is not wanted in the kernel.
Gabor
Signature · 8 lines
---------------------------------------------------------
MTA SZTAKI Computer and Automation Research Institute
Hungarian Academy of Sciences,
Laboratory of Parallel and Distributed Systems
Address : H-1132 Budapest Victor Hugo u. 18-22. Hungary
Phone/Fax : +36 1 329-78-64 (secretary)
W3 : http://www.lpds.sztaki.hu
---------------------------------------------------------Quoted 2 lines · Gabor Gombas
Also, doing things differently when on AC power smells like a policy decision, and AFAIK policy handling is not wanted in the kernel.
Backlight drivers are supposed to return the current brightness. With some hardware, the only way to do that requires knowing whether the system is on AC or not.
Signature · 1 line
Matthew Garrett | [email protected]
Quoted 7 lines · Gabor Gombas
Quoted 1 line · Pavel Machek
Still "set current brightness" operation makes a lot of sense.
Yes, but userspace already knows if you are on AC power or not, therefore it can decide what "current" means. If this is the only reason then adding a new kernel infrastructure is overkill.
It is not.
powernow-k8 needs to know if ac is plugged, else it can overload battery.
backlight code needs it for get current brightness.
on Zauruses, battery charging is done in kernel (not in hardware). Battery charging obviously needs to know if ac is connected.
on Zauruses, IIRC backlight control code needs to know ac/dc, because voltage differs on some internal buses and backlight power needs to be programmed in different way.
Quoted 2 lines
Also, doing things differently when on AC power smells like a policy decision, and AFAIK policy handling is not wanted in the kernel.
It is not policy decision, it is protect-hardware-from-damage on embedded platorms/pn-k8.
Signature · 5 lines
Web maintainer for suspend.sf.net (www.sf.net/projects/suspend) wanted... - To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 5 lines · Pavel Machek
Quoted 3 lines
Yes, but userspace already knows if you are on AC power or not, therefore it can decide what "current" means. If this is the only reason then adding a new kernel infrastructure is overkill.
It is not.
Ok, fair enough.
Gabor
Signature · 8 lines
---------------------------------------------------------
MTA SZTAKI Computer and Automation Research Institute
Hungarian Academy of Sciences,
Laboratory of Parallel and Distributed Systems
Address : H-1132 Budapest Victor Hugo u. 18-22. Hungary
Phone/Fax : +36 1 329-78-64 (secretary)
W3 : http://www.lpds.sztaki.hu
---------------------------------------------------------Hi!
Quoted 1 line · Matthew Garrett
include/linux/pm.h +0
-0
...
include/linux/pm.h
Quoted 6 lines
include/linux/pm.h +6
-0+void pm_set_ac_status (int (*ac_status_function)(void )) +{ +down (&pm_sem); + get_ac_status = ac_status_function; +up (&pm_sem); +}
Why do you need a semaphore/mutex, when you only do one assignment, which is atomic by itself?
BYtE
Philipp
Signature · 3 lines
/ / (_)__ __ ____ __ Philipp Hahn / /__/ / _ \/ // /\ \/ / /____/_/_//_/\_,_/ /_/\_\ [email protected]
Quoted 5 lines · Philipp Matthias Hahn
Hi!
Quoted 1 line · Matthew Garrett
include/linux/pm.h +0
-0...
Quoted 9 lines
include/linux/pm.h
Quoted 6 lines
include/linux/pm.h +6
-0+void pm_set_ac_status (int (*ac_status_function)(void )) +{ +down (&pm_sem); + get_ac_status = ac_status_function; +up (&pm_sem); +}Why do you need a semaphore/mutex, when you only do one assignment, which is atomic by itself?
It is not.
Pavel
Signature · 1 line
Web maintainer for suspend.sf.net (www.sf.net/projects/suspend) wanted...
Quoted 3 lines · Matthew Garrett
The included patch adds support for power management methods to register callbacks in order to allow drivers to check if the system is on AC or not. Following patches add support to ACPI and APM. Feedback welcome.
Ok. Maybe i am not seeing the point. But why do we need this in the kernel? Can't we handles this easily in userspace?
Signature · 7 lines
Stefan Seyfried \ "I didn't want to write for pay. I QA / R&D Team Mobile Devices \ wanted to be paid for what I write." SUSE LINUX Products GmbH, Nürnberg \ -- Leonard Cohen - To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 7 lines · Stefan Seyfried
Quoted 3 lines · Matthew Garrett
The included patch adds support for power management methods to register callbacks in order to allow drivers to check if the system is on AC or not. Following patches add support to ACPI and APM. Feedback welcome.
Ok. Maybe i am not seeing the point. But why do we need this in the kernel? Can't we handles this easily in userspace?
Some kernel parts need to now: for example powernow-k8: some frequencies are not allowed when you are running off battery. [Just now it is solved by not allowing those frequencies at all unless ACPI is available; quite an ugly solution.]
Pavel
Signature · 5 lines
Web maintainer for suspend.sf.net (www.sf.net/projects/suspend) wanted... - To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 6 lines · Pavel Machek
Quoted 2 lines · Stefan Seyfried
Ok. Maybe i am not seeing the point. But why do we need this in the kernel? Can't we handles this easily in userspace?
Some kernel parts need to now: for example powernow-k8: some
we can tell them from userspace.
Quoted 3 lines
frequencies are not allowed when you are running off battery. [Just now it is solved by not allowing those frequencies at all unless ACPI is available; quite an ugly solution.]
And this is a reason to include it in the kernel?
Pavel? Is it you? What have they done to you? ;-)
I mean - we are moving suspend and everything to userspace, so i wonder why this has to be in kernel.
Signature · 7 lines
Stefan Seyfried \ "I didn't want to write for pay. I QA / R&D Team Mobile Devices \ wanted to be paid for what I write." SUSE LINUX Products GmbH, Nürnberg \ -- Leonard Cohen - To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 9 lines · Stefan Seyfried
Quoted 6 lines · Pavel Machek
Quoted 2 lines · Stefan Seyfried
Ok. Maybe i am not seeing the point. But why do we need this in the kernel? Can't we handles this easily in userspace?
Some kernel parts need to now: for example powernow-k8: some
we can tell them from userspace.
No, because battery will explode if you do not slow cpu down fast enough. And it would be extremely ugly.
Right now, interfaces we have are:
/proc/apm -- to get AC/DC info on APM
/proc/acpi/ac_adapter/*/state -- to get in on ACPI
...then we have...
in-kernel interface to get AC/DC info on APM, and in-kernel interface to get AC/DC info on ACPI (powernow-k8 needs it). Brightness needs to use those interfaces, too.
With your proposed solution, we'd have to add /proc/powernow_k8/you_are_on_ac and /proc/backlight/you_are_on_ac and ..., or something, along with daemon to poll /proc/apm and /proc/acpi/*/*/state (and whatever else embedded machines do), and feed it back to kernel.
That's clearly wrong thing to do. So Matthew patch is actually cleanup. It should allow us to kill special interfaces and have something like /sys/power/ac ... and not having to introduce special interfaces for pn-k8, backlight and more.
(More responses in separate thread.)
Quoted 6 lines
Quoted 3 lines
frequencies are not allowed when you are running off battery. [Just now it is solved by not allowing those frequencies at all unless ACPI is available; quite an ugly solution.]
And this is a reason to include it in the kernel?
Pavel? Is it you? What have they done to you? ;-)
Frequency scaling is currently done in kernel, and this is "battery blows up" issue.
Quoted 2 lines
I mean - we are moving suspend and everything to userspace, so i wonder why this has to be in kernel.
Unless you want LZF, encryption, and graphical progress bar, suspend goes to userspace. But AC/DC knowledge is actually neccessary for kernel. You can live without it on most PCs (except powernow-k8 machines and notebooks that have braindamaged), but embedded platforms definitely need it.
Pavel
Signature · 5 lines
Web maintainer for suspend.sf.net (www.sf.net/projects/suspend) wanted... - To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 9 lines
Quoted 6 lines
Quoted 4 lines
Quoted 2 lines
Ok. Maybe i am not seeing the point. But why do we need this in the kernel? Can't we handles this easily in userspace?
Some kernel parts need to now: for example powernow-k8: some
we can tell them from userspace.
No, because battery will explode if you do not slow cpu down fast enough.
How is that? APC UPS don't explode either if they are switching to battery.
Jan Engelhardt
Quoted 9 lines · Jan Engelhardt
Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
Quoted 1 line
Ok. Maybe i am not seeing the point. But why do we need this in the kernel? Can't we handles this easily in userspace?
Some kernel parts need to now: for example powernow-k8: some
we can tell them from userspace.
No, because battery will explode if you do not slow cpu down fast enough.
How is that? APC UPS don't explode either if they are switching to battery.
Yes, but APC UPS have big 3-inch fans blowing air over the batteries to keep them from overheating; laptops dont. Besides which, have you ever stuck your hand on a UPS while it's running a server on battery? Those things get damn hot even with the fans, not even considering the fact that a lead-acid battery is much more efficient about storing and using energy than a Lithium-ion battery.
Incidentally, my UPS thermometer tells me that the peak load on-battery temp is more than 120 F; would you _really_ want that sitting on your lap in addition to all the heat generated by CPU, ram, GPU, etc?
There are real applications out there that have these problems, in fact a lot of manufacturers have issued recalls because of exploding batteries, power bricks, etc, indicating that this is not a simple problem.
Cheers,
Kyle Moffett
--
Diplomacy involves walking softly and _carrying_ a big stick. Actually using the big stick means the diplomacy part failed.
-- Rob Landley
Quoted 11 lines · Pavel Machek
Quoted 4 lines · Stefan Seyfried
Quoted 1 line · Matthew Garrett
The included patch adds support for power management methods to register callbacks in order to allow drivers to check if the system is on AC or not. Following patches add support to ACPI and APM. Feedback welcome.
Ok. Maybe i am not seeing the point. But why do we need this in the kernel? Can't we handles this easily in userspace?
Some kernel parts need to now: for example powernow-k8: some frequencies are not allowed when you are running off battery. [Just now it is solved by not allowing those frequencies at all unless ACPI is available; quite an ugly solution.]
Allowed CPUfreqs are exported via _PPC.
This is why a lot hardware sends an ac_adapter and a processor event when (un)plugging ac adapter.
Limiting cpufreq already works nice that way.
AMD64 laptops are booting with lower freqs per default until they are pushed up, so there shouldn't be anything critical?
For the brightness part, I don't see any "laptop is going to explode" issue.
I always hated the brightness going down when I unplugged ac on M$ and had to push ten times the brightness "up button" before I could go on working...
Shouldn't it be:
ac Event ---> userspace <--- user config
|
| brightness <-------|
Whether the ac brightness will be set when going to ac, or whatever brightness, should be configurable in userspace IMO. This is a one liner in the acpid config?
However, I also like the general /sys/../brightness file very much!
Thomas
-
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 14 lines · Thomas Renninger
Quoted 5 lines · Pavel Machek
Quoted 4 lines · Stefan Seyfried
Quoted 3 lines · Matthew Garrett
The included patch adds support for power management methods to register callbacks in order to allow drivers to check if the system is on AC or not. Following patches add support to ACPI and APM. Feedback welcome.
Quoted 8 lines
Quoted 2 lines
Ok. Maybe i am not seeing the point. But why do we need this in the kernel? Can't we handles this easily in userspace?
Some kernel parts need to now: for example powernow-k8: some frequencies are not allowed when you are running off battery. [Just now it is solved by not allowing those frequencies at all unless ACPI is available; quite an ugly solution.]
Quoted 7 lines
Allowed CPUfreqs are exported via
_PPC.
This is why a lot hardware sends anac_adapterand a processor event when (un)plugging ac adapter.
Limiting cpufreq already works nice that way.AMD64 laptops are booting with lower freqs per default until they are pushed up, so there shouldn't be anything critical?
This is not true as far as my box is concerned (Asus L5D). It starts with the _highest_ clock available.
Quoted 3 lines
For the brightness part, I don't see any "laptop is going to explode" issue.
I always hated the brightness going down when I unplugged ac on M$
Currently I have the same problem on Linux, but I don't know the solution (yet). Any hints? :-)
Rafael
-
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 17 lines · Rafael J. Wysocki
Quoted 16 lines · Thomas Renninger
Quoted 10 lines · Pavel Machek
Quoted 4 lines · Stefan Seyfried
Quoted 1 line · Matthew Garrett
The included patch adds support for power management methods to register callbacks in order to allow drivers to check if the system is on AC or not. Following patches add support to ACPI and APM. Feedback welcome.
Ok. Maybe i am not seeing the point. But why do we need this in the kernel? Can't we handles this easily in userspace?
Some kernel parts need to now: for example powernow-k8: some frequencies are not allowed when you are running off battery. [Just now it is solved by not allowing those frequencies at all unless ACPI is available; quite an ugly solution.]
Allowed CPUfreqs are exported via
_PPC.
This is why a lot hardware sends anac_adapterand a processor event when (un)plugging ac adapter.
Limiting cpufreq already works nice that way.
Quoted 5 lines
Quoted 2 lines
AMD64 laptops are booting with lower freqs per default until they are pushed up, so there shouldn't be anything critical?
This is not true as far as my box is concerned (Asus L5D). It starts with the _highest_ clock available.
Hmm, but then there shouldn't be any critical overheat problems and if, the hardware has to switch off the machine hard. OS always could freeze, but the battery must not start to burn...
Quoted 7 lines
Quoted 4 lines
For the brightness part, I don't see any "laptop is going to explode" issue.
I always hated the brightness going down when I unplugged ac on M$Currently I have the same problem on Linux, but I don't know the solution (yet). Any hints? :-)
Hmm, this is probably done by ACPI in some ac connected function? Seems as some machines already adjust brightness in ACPI context to the ac/battery brightness value and some leave it to the OS to adjust. Override it in /sys/../brightness (as soon as it exists) or the current vendor specific solution file.
Connect the acpid to a battery ACPI event rule and let override it there.
IMO, the /sys/.../brightness patch should go in as soon as possible, I think all everybody agrees here?
Maybe I oversaw an issue, but I really don't see a reason for connecting the brightness to ac in kernel space.
Some weeks later someone likes to have a /sys/../brightness_ignore_ac switch ...
Thomas
-
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 2 lines · Thomas Renninger
Maybe I oversaw an issue, but I really don't see a reason for connecting the brightness to ac in kernel space.
The backlight class maintainer specified that /sys/../brightness should return the *current* brightness. In some cases that's impossible without knowing the ac status.
Signature · 1 line
Matthew Garrett | [email protected]
Quoted 6 lines · Matthew Garrett
Quoted 2 lines · Thomas Renninger
Maybe I oversaw an issue, but I really don't see a reason for connecting the brightness to ac in kernel space.
The backlight class maintainer specified that
/sys/../brightnessshould
Who's that? Is there more info/spec available?
Quoted 2 lines
return the *current* brightness. In some cases that's impossible without knowing the ac status.
Ok.
The ac/battery status is used to guess the current brightness? Please
do not set suggested values automatically, IMO this is not a nice feature... I am really looking forward to see a general /sys/../brightness.
Thanks a lot for working on this!
Just an idea for later:
Beside min/max there probably should be a /sys/../brightness_bat and /sys/../brightness_ac.
- If ac/battery brightness will not be set by kernel, userspace can write the right value to /sys/../brightness and the above two can be readonly.
- If ac/battery brightness will be set by kernel the above two should even be writeable so that people can avoid the need of pressing "brightness up" button without hacking the kernel.
I vote for letting userspace doing the stuff..., e.g. changing brightness behind the back of some userspace tool will result in userspace tools starting to poll.
Comments?
Thomas
Hi!
Quoted 8 lines
Quoted 3 lines
This is not true as far as my box is concerned
(Asus L5D). It starts with
the _highest_ clock available.Hmm, but then there shouldn't be any critical
overheat problems and if,
the hardware has to switch off the machine
hard. OS always could freeze,
but the battery must not start to burn...
I told that to hw designers... too late. Fortunately batteries usually only crash machine if you overload them.
Quoted 3 lines
IMO, the
/sys/.../brightnesspatch should go in
as soon as possible, I think
all everybody agrees here?
Yep.
Quoted 3 lines
Maybe I oversaw an issue, but I really don't
see a reason for connecting
the brightness to ac in kernel space.
We are not going to connect it. But to implement .../brightness, you need to know ac/battery on several "broken" notebooks.
Signature · 1 line
64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms
Hi!
Quoted 5 lines
Quoted 2 lines
AMD64 laptops are booting with lower freqs per default until they are pushed up, so there shouldn't be anything critical?
This is not true as far as my box is concerned (Asus L5D). It starts with the _highest_ clock available.
Not all laptops have underpowered batteries. And I have seen machine that booted fast with underpowered battery... not nice. Would crash in POST in 30% cases.
Quoted 7 lines
Quoted 4 lines
For the brightness part, I don't see any "laptop is going to explode" issue.
I always hated the brightness going down when I unplugged ac on M_Currently I have the same problem on Linux, but I don't know the solution (yet). Any hints? :-)
Work around acpi bios... not going to be nice.
Signature · 1 line
64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms