The previous patch had a typo, so here's a fixed one. This patch stores
the chassis type of the hardware, making it easier to determine whether
a piece of hardware from a manufacturer is a laptop. This information is
not 100% reliable - some vendors insert bogus values, but most of the
big ones are fairly accurate.
Signed-Off-By: Matthew Garrett <[email protected]>
diff --git a/arch/i386/kernel/dmi_scan.c b/arch/i386/kernel/dmi_scan.c
index 58516e2..e555588 100644
--- a/arch/i386/kernel/dmi_scan.c
+++ b/arch/i386/kernel/dmi_scan.c
@@ -30,6 +30,50 @@ static char * __init dmi_string(struct d
return str;
}
+static char * __init dmi_chassis_type(struct dmi_header *dm, u8 s)
+{
+ char *str = "";
+ static const char *type[]={
+ "Other", /* 0x01 */
+ "Unknown",
+ "Desktop",
+ "Low Profile Desktop",
+ "Pizza Box",
+ "Mini Tower",
+ "Tower",
+ "Portable",
+ "Laptop",
+ "Notebook",
+ "Hand Held",
+ "Docking Station",
+ "All In One",
+ "Sub Notebook",
+ "Space-saving",
+ "Lunch Box",
+ "Main Server Chassis", /* master.mif says System */
+ "Expansion Chassis",
+ "Sub Chassis",
+ "Bus Expansion Chassis",
+ "Peripheral Chassis",
+ "RAID Chassis",
+ "Rack Mount Chassis",
+ "Sealed-case PC",
+ "Multi-system" /* 0x19 */
+ };
+
+ if(s>=0x01 && s<=0x19) {
+ str = alloc_bootmem(strlen(type[code-0x01]));
+ if (str != NULL)
+ strcpy(str, type[code-0x01]);
+ else
+ printk(KERN_ERR "dmi_chassis_type: out of memory.\n");
+
+ return str;
+ }
+
+ return NULL;
+}
+
/*
* We have to be cautious here. We have seen BIOSes with DMI pointers
* pointing to completely the wrong place for example
@@ -93,7 +137,11 @@ static void __init dmi_save_ident(struct
if (dmi_ident[slot])
return;
- p = dmi_string(dm, d[string]);
+ if (slot == DMI_CHASSIS_TYPE)
+ p = dmi_chassis_type(string & 0x7f);
+ else
+ p = dmi_string(dm, d[string]);
+
if (p == NULL)
return;
@@ -176,6 +224,11 @@ static void __init dmi_decode(struct dmi
dmi_save_ident(dm, DMI_BOARD_NAME, 5);
dmi_save_ident(dm, DMI_BOARD_VERSION, 6);
break;
+ case 3: /* Chassis Information */
+ dmi_save_ident(dm, DMI_CHASSIS_VENDOR, 4);
+ dmi_save_ident(dm, DMI_CHASSIS_TYPE, 5);
+ break;
+
case 10: /* Onboard Devices Information */
dmi_save_devices(dm);
break;
diff --git a/include/linux/dmi.h b/include/linux/dmi.h
index 05f4132..5f425a7 100644
--- a/include/linux/dmi.h
+++ b/include/linux/dmi.h
@@ -15,6 +15,8 @@ enum dmi_field {
DMI_BOARD_VENDOR,
DMI_BOARD_NAME,
DMI_BOARD_VERSION,
+ DMI_CHASSIS_VENDOR,
+ DMI_CHASSIS_TYPE,
DMI_STRING_MAX,
};
--
Matthew Garrett | [email protected]
This patch hooks into the generic backlight framework and allows the
brightness of HP laptop displays to be read. The AC and DC values are
separate, but the framework currently provides no mechanism for them to
be provided separately and there's no straightforward way for the driver
to know if the system is on battery or not. As a result, I've put the AC
brightness in the top 16 bits of the returned value, with the DC
brightness in the bottom 16.
This patch requires my earlier patch to allow checking against the DMI
chassis type.
Signed-Off-By: Matthew Garrett <[email protected]>
diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index 996d543..e4f84eb 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -50,3 +50,10 @@ config BACKLIGHT_CORGI
If you have a Sharp Zaurus SL-C7xx, say y to enable the
backlight driver.
+config BACKLIGHT_HP
+ tristate "HP Laptop Backlight Driver"
+ depends on BACKLIGHT_DEVICE && X86
+ default n
+ help
+ Allows userspace applications to read the current screen brightness
+ on HP laptops
\ No newline at end of file
diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
index 4af321f..93ac108 100644
--- a/drivers/video/backlight/Makefile
+++ b/drivers/video/backlight/Makefile
@@ -4,3 +4,4 @@ obj-$(CONFIG_LCD_CLASS_DEVICE) += lc
obj-$(CONFIG_BACKLIGHT_CLASS_DEVICE) += backlight.o
obj-$(CONFIG_BACKLIGHT_CORGI) += corgi_bl.o
obj-$(CONFIG_SHARP_LOCOMO) += locomolcd.o
+obj-$(CONFIG_BACKLIGHT_HP) += hp_bl.o
\ No newline at end of file
diff --git a/drivers/video/backlight/hp_bl.c b/drivers/video/backlight/hp_bl.c
new file mode 100644
index 0000000..945c242
--- /dev/null
+++ b/drivers/video/backlight/hp_bl.c
@@ -0,0 +1,98 @@
+/*
+ * Backlight Driver for HP laptops
+ *
+ * Copyright (c) 2006 Matthew Garrett
+ *
+ * Based on corgi_bl.c, Copyright (c) 2004-2005 Richard Purdie
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/spinlock.h>
+#include <linux/fb.h>
+#include <linux/backlight.h>
+#include <linux/dmi.h>
+
+static struct backlight_properties hpbl_data;
+
+static spinlock_t bl_lock = SPIN_LOCK_UNLOCKED;
+
+static struct dmi_system_id __initdata hplcd_device_table[] = {
+ {
+ .ident = "HP",
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Hewlett-Packard"),
+ DMI_MATCH(DMI_CHASSIS_TYPE, "Notebook"),
+ },
+ },
+ { }
+};
+
+
+static int hpbl_get_intensity(struct backlight_device *bd)
+{
+ /* The backlight interface doesn't give us a means of providing
+ more than one brightness value, so we put the AC value in the
+ top bits of the brightness and the DC value in the bottom bits */
+
+ int value;
+ int combined;
+
+ spin_lock(&bl_lock);
+
+ outb(0x97, 0x72);
+ value = inb(0x73);
+
+ value &= 0x1f; // Brightness is in the lower 5 bits
+ combined = value << 16;
+
+ outb(0x99, 0x72);
+ value = inb(0x73);
+
+ spin_unlock(&bl_lock);
+
+ value &= 0x1f; // Brightness is in the lower 5 bits
+ combined |= value;
+
+ return combined;
+}
+
+static struct backlight_properties hpbl_data = {
+ .owner = THIS_MODULE,
+ .get_brightness = hpbl_get_intensity,
+ .max_brightness = 10,
+};
+
+static struct backlight_device *hp_backlight_device;
+
+static int __init hpbl_init(void)
+{
+ if (!dmi_check_system(hplcd_device_table))
+ return -ENODEV;
+
+ hp_backlight_device = backlight_device_register ("hp-bl",
+ NULL, &hpbl_data);
+ if (IS_ERR (hp_backlight_device))
+ return PTR_ERR (hp_backlight_device);
+
+ return 0;
+}
+
+static void __exit hpbl_exit(void)
+{
+ backlight_device_unregister(hp_backlight_device);
+}
+
+module_init(hpbl_init);
+module_exit(hpbl_exit);
+
+MODULE_AUTHOR("Matthew Garrett <[email protected]>");
+MODULE_DESCRIPTION("HP Backlight Driver");
+MODULE_LICENSE("GPL");
--
Matthew Garrett | [email protected]
This patch hooks into the generic backlight framework and allows the
brightness of Dell laptop displays to be read. The AC and DC values are
separate, but the framework currently provides no mechanism for them to
be provided separately and there's no straightforward way for the driver
to know if the system is on battery or not. As a result, I've put the AC
brightness in the top 16 bits of the returned value, with the DC
brightness in the bottom 16.
This patch requires my earlier patch to allow checking against the DMI
chassis type and should be applied after the HP backlight patch.
Signed-Off-By: Matthew Garrett <[email protected]>
diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index 996d543..e4f84eb 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -50,3 +50,10 @@ config BACKLIGHT_CORGI
If you have a Sharp Zaurus SL-C7xx, say y to enable the
backlight driver.
+config BACKLIGHT_HP
+ tristate "HP Laptop Backlight Driver"
+ depends on BACKLIGHT_DEVICE && X86
+ default n
+ help
+ Allows userspace applications to read the current screen brightness
+ on HP laptops
\ No newline at end of file
diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
index 4af321f..93ac108 100644
--- a/drivers/video/backlight/Makefile
+++ b/drivers/video/backlight/Makefile
@@ -4,3 +4,4 @@ obj-$(CONFIG_LCD_CLASS_DEVICE) += lc
obj-$(CONFIG_BACKLIGHT_CLASS_DEVICE) += backlight.o
obj-$(CONFIG_BACKLIGHT_CORGI) += corgi_bl.o
obj-$(CONFIG_SHARP_LOCOMO) += locomolcd.o
+obj-$(CONFIG_BACKLIGHT_HP) += hp_bl.o
\ No newline at end of file
diff --git a/drivers/video/backlight/hp_bl.c b/drivers/video/backlight/hp_bl.c
new file mode 100644
index 0000000..945c242
--- /dev/null
+++ b/drivers/video/backlight/hp_bl.c
@@ -0,0 +1,98 @@
+/*
+ * Backlight Driver for HP laptops
+ *
+ * Copyright (c) 2006 Matthew Garrett
+ *
+ * Based on corgi_bl.c, Copyright (c) 2004-2005 Richard Purdie
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/spinlock.h>
+#include <linux/fb.h>
+#include <linux/backlight.h>
+#include <linux/dmi.h>
+
+static struct backlight_properties hpbl_data;
+
+static spinlock_t bl_lock = SPIN_LOCK_UNLOCKED;
+
+static struct dmi_system_id __initdata hplcd_device_table[] = {
+ {
+ .ident = "HP",
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Hewlett-Packard"),
+ DMI_MATCH(DMI_CHASSIS_TYPE, "Notebook"),
+ },
+ },
+ { }
+};
+
+
+static int hpbl_get_intensity(struct backlight_device *bd)
+{
+ /* The backlight interface doesn't give us a means of providing
+ more than one brightness value, so we put the AC value in the
+ top bits of the brightness and the DC value in the bottom bits */
+
+ int value;
+ int combined;
+
+ spin_lock(&bl_lock);
+
+ outb(0x97, 0x72);
+ value = inb(0x73);
+
+ value &= 0x1f; // Brightness is in the lower 5 bits
+ combined = value << 16;
+
+ outb(0x99, 0x72);
+ value = inb(0x73);
+
+ spin_unlock(&bl_lock);
+
+ value &= 0x1f; // Brightness is in the lower 5 bits
+ combined |= value;
+
+ return combined;
+}
+
+static struct backlight_properties hpbl_data = {
+ .owner = THIS_MODULE,
+ .get_brightness = hpbl_get_intensity,
+ .max_brightness = 10,
+};
+
+static struct backlight_device *hp_backlight_device;
+
+static int __init hpbl_init(void)
+{
+ if (!dmi_check_system(hplcd_device_table))
+ return -ENODEV;
+
+ hp_backlight_device = backlight_device_register ("hp-bl",
+ NULL, &hpbl_data);
+ if (IS_ERR (hp_backlight_device))
+ return PTR_ERR (hp_backlight_device);
+
+ return 0;
+}
+
+static void __exit hpbl_exit(void)
+{
+ backlight_device_unregister(hp_backlight_device);
+}
+
+module_init(hpbl_init);
+module_exit(hpbl_exit);
+
+MODULE_AUTHOR("Matthew Garrett <[email protected]>");
+MODULE_DESCRIPTION("HP Backlight Driver");
+MODULE_LICENSE("GPL");
--
Matthew Garrett | [email protected]
On Mon, 2006-02-06 19:18:10 +0000, Matthew Garrett <[email protected]> wrote:
> diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
> index 996d543..e4f84eb 100644
> --- a/drivers/video/backlight/Kconfig
> +++ b/drivers/video/backlight/Kconfig
[...]
> \ No newline at end of file
> diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
Please don't do that, also for all the other files.
MfG, JBG
--
Jan-Benedict Glaw [email protected] . +49-172-7608481 _ O _
"Eine Freie Meinung in einem Freien Kopf | Gegen Zensur | Gegen Krieg _ _ O
für einen Freien Staat voll Freier Bürger" | im Internet! | im Irak! O O O
ret = do_actions((curr | FREE_SPEECH) & ~(NEW_COPYRIGHT_LAW | DRM | TCPA));
Resend: This time I've actually included the correct patch.
This patch hooks into the generic backlight framework and allows the
brightness of Dell laptop displays to be read. The AC and DC values are
separate, but the framework currently provides no mechanism for them to
be provided separately and there's no straightforward way for the driver
to know if the system is on battery or not. As a result, I've put the AC
brightness in the top 16 bits of the returned value, with the DC
brightness in the bottom 16.
This patch requires my earlier patch to allow checking against the DMI
chassis type and should be applied after the HP backlight patch.
Signed-Off-By: Matthew Garrett <[email protected]>
diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index e4f84eb..0f83183 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -56,4 +56,12 @@ config BACKLIGHT_HP
default n
help
Allows userspace applications to read the current screen brightness
- on HP laptops
\ No newline at end of file
+ on HP laptops
+
+config BACKLIGHT_DELL
+ tristate "Dell Laptop Backlight Driver"
+ depends on BACKLIGHT_DEVICE && X86
+ default n
+ help
+ Allows userspace applications to read the current screen brightness
+ on Dell laptops
\ No newline at end of file
diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
index 93ac108..f337f01 100644
--- a/drivers/video/backlight/Makefile
+++ b/drivers/video/backlight/Makefile
@@ -4,4 +4,5 @@ obj-$(CONFIG_LCD_CLASS_DEVICE) += lc
obj-$(CONFIG_BACKLIGHT_CLASS_DEVICE) += backlight.o
obj-$(CONFIG_BACKLIGHT_CORGI) += corgi_bl.o
obj-$(CONFIG_SHARP_LOCOMO) += locomolcd.o
-obj-$(CONFIG_BACKLIGHT_HP) += hp_bl.o
\ No newline at end of file
+obj-$(CONFIG_BACKLIGHT_HP) += hp_bl.o
+obj-$(CONFIG_BACKLIGHT_DELL) += dell_bl.o
\ No newline at end of file
diff --git a/drivers/video/backlight/dell_bl.c b/drivers/video/backlight/dell_bl.c
new file mode 100644
index 0000000..a97a4b8
--- /dev/null
+++ b/drivers/video/backlight/dell_bl.c
@@ -0,0 +1,92 @@
+/*
+ * Backlight Driver for Dell laptops
+ *
+ * Copyright (c) 2006 Matthew Garrett
+ *
+ * Based on corgi_bl.c, Copyright (c) 2004-2005 Richard Purdie
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/fb.h>
+#include <linux/backlight.h>
+#include <linux/dmi.h>
+#include <linux/nvram.h>
+
+static struct backlight_properties dellbl_data;
+
+static struct dmi_system_id __initdata delllcd_device_table[] = {
+ {
+ .ident = "Dell",
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Dell Inc."),
+ DMI_MATCH(DMI_CHASSIS_TYPE, "Portable"),
+ },
+ },
+ { }
+};
+
+
+static int dellbl_get_intensity(struct backlight_device *bd)
+{
+ /* The backlight interface doesn't give us a means of providing
+ more than one brightness value, so we put the AC value in the
+ top bits of the brightness and the DC value in the bottom bits */
+
+ int value;
+ int combined;
+
+ value = nvram_read_byte(0x53);
+
+ value &= 0xf0; // Brightness is in the upper 4 bits
+ combined = value << 12;
+
+ value = nvram_read_byte(0x16);
+
+ outb(0x99, 0x72);
+ value = inb(0x73);
+
+ value &= 0x0f; // Brightness is in the lower 4 bits
+ combined |= value;
+
+ return combined;
+}
+
+static struct backlight_properties dellbl_data = {
+ .owner = THIS_MODULE,
+ .get_brightness = dellbl_get_intensity,
+ .max_brightness = 0xe,
+};
+
+static struct backlight_device *dell_backlight_device;
+
+static int __init dellbl_init(void)
+{
+ if (!dmi_check_system(delllcd_device_table))
+ return -ENODEV;
+
+ dell_backlight_device = backlight_device_register ("dell-bl",
+ NULL, &dellbl_data);
+ if (IS_ERR (dell_backlight_device))
+ return PTR_ERR (dell_backlight_device);
+
+ return 0;
+}
+
+static void __exit dellbl_exit(void)
+{
+ backlight_device_unregister(dell_backlight_device);
+}
+
+module_init(hpbl_init);
+module_exit(hpbl_exit);
+
+MODULE_AUTHOR("Matthew Garrett <[email protected]>");
+MODULE_DESCRIPTION("Dell Backlight Driver");
+MODULE_LICENSE("GPL");
--
Matthew Garrett | [email protected]
On Monday 06 February 2006 19:37, Matthew Garrett wrote:
> +static void __exit dellbl_exit(void)
> +{
> +???????backlight_device_unregister(dell_backlight_device);
> +}
> +
> +module_init(hpbl_init);
> +module_exit(hpbl_exit);
This is not gonna work - dellbl_* vs hpbl_*
--
Dmitry
Matthew, Andrew,
(sorry, I'm not subscribed to l-k, so I'm just starting a new thread.)
I would _strongly_ suggest that this patch _not_ go in. This driver
uses hardcoded values that are subject to change without notice. It is
perfectly legitimate for future versions of Dell BIOS to interpret pokes
to cmos location 0x99 as the command to format your hard drive.
The proper way to do this is using libsmbios. The project page is at
http://linux.dell.com/libsmbios/main. Using libsmbios, plus the
already-included dcdbas kernel driver, you can correctly do brightness
control. If you would like to write a proper brightness control, it can
be done entirely in user space, and I could help you.
There are specific smbios structures, proprietary to Dell, that are
documented in libsmbios. These structures, properly decoded, tell the
proper port to use to control this. This is guaranteed to work across
BIOS versions and not to format your hard drive. :-)
Libsmbios is 100% open source (dual GPL/OSL license).
--
Michael Brown
Libsmbios maintainer
On Mon, Feb 06, 2006 at 09:43:16PM -0600, Michael E Brown wrote:
> I would _strongly_ suggest that this patch _not_ go in. This driver
> uses hardcoded values that are subject to change without notice. It is
> perfectly legitimate for future versions of Dell BIOS to interpret pokes
> to cmos location 0x99 as the command to format your hard drive.
I managed to send the wrong patch - the Dell one only reads from nvram.
If nvram reads are likely to reformat your hard drive, I think Dell
needs to reconsider its BIOS design :)
More seriously, a quick scan of libsmbios hasn't revealed any method for
obtaining the screen brightness. It's possible that I'm blind (I'm
certainly slightly drunk), but can you give a pointer to the correct
mechanism for making this call?
Thanks,
--
Matthew Garrett | [email protected]
On Mon, Feb 06, 2006 at 10:37:55PM -0500, Dmitry Torokhov wrote:
> This is not gonna work - dellbl_* vs hpbl_*
Good point. Third time's the charm?
--
Matthew Garrett | [email protected]
On Tue, 2006-02-07 at 12:32 +0000, Matthew Garrett wrote:
+ /* The backlight interface doesn't give us a means of providing
+ more than one brightness value, so we put the AC value in the
+ top bits of the brightness and the DC value in the bottom bits */
This is total abuse of the backlight class. The idea is that
cat /sys/class/backlight/ccc/brightness returns the *current* backlight
brightness. On AC power it should return the AC brightness value and on
DC power return the DC value.
If you want to know the DC and AC values as stored in the BIOS they
should be device specific attributes, not generic class ones.
I have a patch in the pipeline to change the backlight class slightly to
provide both the user requested brightness and the current brightness as
the two can differ (the Zaurus handhelds limit the backlight intensity
on low power).
Richard
On Tue, Feb 07, 2006 at 01:06:45PM +0000, Richard Purdie wrote:
> This is total abuse of the backlight class. The idea is that
> cat /sys/class/backlight/ccc/brightness returns the *current* backlight
> brightness. On AC power it should return the AC brightness value and on
> DC power return the DC value.
Unfortunately, the hardware doesn't seem to give us that option. There's
no obvious way of determining whether we're on AC or DC from the kernel
without doing very nasty things with ACPI and APM (userspace can work it
out fairly easily).
Would you be open to adding generic support for displaying separate AC
and DC brightnesses? Making it driver specific leaves the potential for
inconsistencies.
--
Matthew Garrett | [email protected]
On Tue, 2006-02-07 at 13:23 +0000, Matthew Garrett wrote:
> On Tue, Feb 07, 2006 at 01:06:45PM +0000, Richard Purdie wrote:
>
> > This is total abuse of the backlight class. The idea is that
> > cat /sys/class/backlight/ccc/brightness returns the *current* backlight
> > brightness. On AC power it should return the AC brightness value and on
> > DC power return the DC value.
>
> Unfortunately, the hardware doesn't seem to give us that option. There's
> no obvious way of determining whether we're on AC or DC from the kernel
> without doing very nasty things with ACPI and APM (userspace can work it
> out fairly easily).
>
> Would you be open to adding generic support for displaying separate AC
> and DC brightnesses? Making it driver specific leaves the potential for
> inconsistencies.
The problem is that AC or DC is not a generic property of backlights but
specific to your problematic hardware case. You're going to have a to
find a way to tell if its running on AC or not to report the right value
in the manner the class requires.
I can't see how you plan to add "generic support for displaying separate
AC and DC brightnesses" without destroying the point of the class (which
for the brightness parameter is to display the current backlight
brightness).
Richard
On Tue, Feb 07, 2006 at 01:37:06PM +0000, Richard Purdie wrote:
> On Tue, 2006-02-07 at 13:23 +0000, Matthew Garrett wrote:
> > Would you be open to adding generic support for displaying separate AC
> > and DC brightnesses? Making it driver specific leaves the potential for
> > inconsistencies.
>
> The problem is that AC or DC is not a generic property of backlights but
> specific to your problematic hardware case. You're going to have a to
> find a way to tell if its running on AC or not to report the right value
> in the manner the class requires.
Cases rather than case, sadly. Determining whether the hardware is on AC
or not tends to be much more awkward than you'd think. On HPs, it seems
to be done by making a specific call to the embedded controller. This is
very model specific, whereas the brightness values aren't. It's also
likely to go horribly wrong if the hardware is trying to access the
embedded controller at the same time. Realistically, it's impossible
without making the driver depend on ACPI and exporting acpi_ac_get_state
from the ACPI layer, which would be a shame since the rest of the
functionality isn't ACPI dependent at all.
> I can't see how you plan to add "generic support for displaying separate
> AC and DC brightnesses" without destroying the point of the class (which
> for the brightness parameter is to display the current backlight
> brightness).
I think providing a consistent interface for what information we can
provide is worthwhile.
--
Matthew Garrett | [email protected]
On Tue, 2006-02-07 at 13:55 +0000, Matthew Garrett wrote:
> On Tue, Feb 07, 2006 at 01:37:06PM +0000, Richard Purdie wrote:
> > On Tue, 2006-02-07 at 13:23 +0000, Matthew Garrett wrote:
> > > Would you be open to adding generic support for displaying separate AC
> > > and DC brightnesses? Making it driver specific leaves the potential for
> > > inconsistencies.
> >
> > The problem is that AC or DC is not a generic property of backlights but
> > specific to your problematic hardware case. You're going to have a to
> > find a way to tell if its running on AC or not to report the right value
> > in the manner the class requires.
>
> Realistically, it's impossible
> without making the driver depend on ACPI and exporting acpi_ac_get_state
> from the ACPI layer, which would be a shame since the rest of the
> functionality isn't ACPI dependent at all.
If that's what's needed to give the correct behaviour, so be it. In an
ideal world, we'd not need the dependency but it sounds unavoidable.
You can actually have a soft dependency where it could use the DC value
unless ACPI is present in which case it would be more intelligent. The
corgi_bl driver does this for the backlight limiting on low power.
Richard
You can get and set laptop brighness on Dell with the proper SMI call.
To do the proper SMI call requires parsing SMBIOS structure 0xDA, a
vendor-proprietary structure, and getting the SMI index and io port and
magic values. Then, you need to know how to setup the registers and
input/output buffers for the call. All of this is already present in
libsmbios.
Reading nvram is not a valid way to get brighness unless you also do
similar work (parse specific vendor-proprietary SMBIOS structures) to
ensure that you are reading the correct location. This location is
subject to change from BIOS to BIOS and machine to machine. The fact
that you may have observed it in the same location on a few laptops does
not change this fact.
In fact, I have the same objection to the I8K driver in the kernel. It
has hardcoded SMI calls, that are subject to change. There is a proper
way to get the correct IO ports to make this safe, but it is not
currently being done.
--
Michael
-----Original Message-----
From: Matthew Garrett [mailto:[email protected]]
Sent: Monday, February 06, 2006 10:10 PM
To: Brown, Michael E
Cc: Andrew Morton; Domsch, Matt; [email protected]
Subject: Re: [PATCH] [RESEND] Add Dell laptop backlight brightness
display
On Mon, Feb 06, 2006 at 09:43:16PM -0600, Michael E Brown wrote:
> I would _strongly_ suggest that this patch _not_ go in. This
driver
> uses hardcoded values that are subject to change without notice. It is
> perfectly legitimate for future versions of Dell BIOS to interpret
> pokes to cmos location 0x99 as the command to format your hard drive.
I managed to send the wrong patch - the Dell one only reads from nvram.
If nvram reads are likely to reformat your hard drive, I think Dell
needs to reconsider its BIOS design :)
More seriously, a quick scan of libsmbios hasn't revealed any method for
obtaining the screen brightness. It's possible that I'm blind (I'm
certainly slightly drunk), but can you give a pointer to the correct
mechanism for making this call?
Thanks,
--
Matthew Garrett | [email protected]
On Tue, Feb 07, 2006 at 10:34:31AM -0600, [email protected] wrote:
> You can get and set laptop brighness on Dell with the proper SMI call.
Oh, heavens. Could you people (and here I include pretty much everyone
who manufactures laptops) please (please!) just implement the ACPI video
extension? We're going to end up having to ship a 200K library for
each and every laptop manufacturer who's decided to implement basic
functionality in a proprietary manner, and it's going to make me cry.
(Which SMI do I need for brightness control? The libsmbios docs seem to
be remarkably quiet on what functionality is actually available, and I'm
not keen on calling things randomly :))
--
Matthew Garrett | [email protected]
Yes, the list of things to do is called the token table. I am working on
getting permission to release it. As it is, I am releasing tokens as
people ask for them, on a case by case basis.
I, personally, do not build or design our laptops, nor do I have any
sway with those that do. Crying about it isn't going to help, sorry.
If you would care to move the discussion to libsmbios-devel, I would be
more than happy to help you get the token you need and write a
util/lib/whatever to do what you need.
--
Michael
-----Original Message-----
From: Matthew Garrett [mailto:[email protected]]
Sent: Tuesday, February 07, 2006 11:20 AM
To: Brown, Michael E
Cc: [email protected]; Domsch, Matt; [email protected]
Subject: Re: [PATCH] [RESEND] Add Dell laptop backlight brightness
display
On Tue, Feb 07, 2006 at 10:34:31AM -0600, [email protected]
wrote:
> You can get and set laptop brighness on Dell with the proper SMI call.
Oh, heavens. Could you people (and here I include pretty much everyone
who manufactures laptops) please (please!) just implement the ACPI video
extension? We're going to end up having to ship a 200K library for each
and every laptop manufacturer who's decided to implement basic
functionality in a proprietary manner, and it's going to make me cry.
(Which SMI do I need for brightness control? The libsmbios docs seem to
be remarkably quiet on what functionality is actually available, and I'm
not keen on calling things randomly :))
--
Matthew Garrett | [email protected]
Hi!
On ?t 07-02-06 13:55:02, Matthew Garrett wrote:
> On Tue, Feb 07, 2006 at 01:37:06PM +0000, Richard Purdie wrote:
> > On Tue, 2006-02-07 at 13:23 +0000, Matthew Garrett wrote:
> > > Would you be open to adding generic support for displaying separate AC
> > > and DC brightnesses? Making it driver specific leaves the potential for
> > > inconsistencies.
> >
> > The problem is that AC or DC is not a generic property of backlights but
> > specific to your problematic hardware case. You're going to have a to
> > find a way to tell if its running on AC or not to report the right value
> > in the manner the class requires.
>
> Cases rather than case, sadly. Determining whether the hardware is on AC
> or not tends to be much more awkward than you'd think. On HPs, it seems
> to be done by making a specific call to the embedded controller. This is
> very model specific, whereas the brightness values aren't. It's also
> likely to go horribly wrong if the hardware is trying to access the
> embedded controller at the same time. Realistically, it's impossible
> without making the driver depend on ACPI and exporting acpi_ac_get_state
> from the ACPI layer, which would be a shame since the rest of the
> functionality isn't ACPI dependent at all.
Depending on acpi does not seem that bad. And exporting
acpi_ac_get_state is probably good idea for other reasons, too. (Look
how powernow-k8 does it; it needs pretty reliable AC/DC info.)
Pavel
--
Web maintainer for suspend.sf.net (http://www.sf.net/projects/suspend) wanted...
Hi!
> You can get and set laptop brighness on Dell with the proper SMI call.
>
> To do the proper SMI call requires parsing SMBIOS structure 0xDA, a
> vendor-proprietary structure, and getting the SMI index and io port and
> magic values. Then, you need to know how to setup the registers and
> input/output buffers for the call. All of this is already present in
> libsmbios.
Perhaps authors of libsmbios could help here?
> Reading nvram is not a valid way to get brighness unless you also do
> similar work (parse specific vendor-proprietary SMBIOS structures) to
> ensure that you are reading the correct location. This location is
> subject to change from BIOS to BIOS and machine to machine. The fact
> that you may have observed it in the same location on a few laptops does
> not change this fact.
Well, folks reverse-engineering your machines had no idea until now...
> In fact, I have the same objection to the I8K driver in the kernel. It
> has hardcoded SMI calls, that are subject to change. There is a proper
> way to get the correct IO ports to make this safe, but it is not
> currently being done.
Could you or someone at Dell submit patches to correct this?
--
64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms
Pavel,
Matthew has shown up on the libsmbios-devel mailing list. I sent
all the
info needed to do a test of Dell LCD brightness control. The main thing
left
would be to make one utility out of the current separate, unsupported,
test
utils.
As for fixing i8k, I don't have the slightest clue where to
begin. You
either have to split initialization with userspace to parse and send in
the
correct io/magic ports to do SMI, or you have to put Dell-specific SMI
token
parsing in the kernel.
If somebody wants to discuss the design, I can definetly
discuss. I
even have a _very_ rough mockup of userspace code to do this. Did not
take
it further because I don't know enough about lmsensors or how to fix
i8k.
--
Michael
PS> sorry for top-posting, my non-broken email client at home for some
reason
could not see your msg, so I could not reply from there.
-----Original Message-----
From: Pavel Machek [mailto:[email protected]]
Sent: Sunday, February 12, 2006 11:26 AM
To: Brown, Michael E
Cc: [email protected]; [email protected]; Domsch, Matt;
[email protected]
Subject: Re: [PATCH] [RESEND] Add Dell laptop backlight brightness
display
Hi!
> You can get and set laptop brighness on Dell with the proper SMI call.
>
> To do the proper SMI call requires parsing SMBIOS structure 0xDA, a
> vendor-proprietary structure, and getting the SMI index and io port
> and magic values. Then, you need to know how to setup the registers
> and input/output buffers for the call. All of this is already present
> in libsmbios.
Perhaps authors of libsmbios could help here?
> Reading nvram is not a valid way to get brighness unless you also do
> similar work (parse specific vendor-proprietary SMBIOS structures) to
> ensure that you are reading the correct location. This location is
> subject to change from BIOS to BIOS and machine to machine. The fact
> that you may have observed it in the same location on a few laptops
> does not change this fact.
Well, folks reverse-engineering your machines had no idea until now...
> In fact, I have the same objection to the I8K driver in the kernel. It
> has hardcoded SMI calls, that are subject to change. There is a proper
> way to get the correct IO ports to make this safe, but it is not
> currently being done.
Could you or someone at Dell submit patches to correct this?
--
64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms
Hi!
> Matthew has shown up on the libsmbios-devel mailing list. I sent
> all the
> info needed to do a test of Dell LCD brightness control. The main thing
> left
> would be to make one utility out of the current separate, unsupported,
> test
> utils.
>
> As for fixing i8k, I don't have the slightest clue where to
> begin. You
> either have to split initialization with userspace to parse and send in
> the
> correct io/magic ports to do SMI, or you have to put Dell-specific SMI
> token
> parsing in the kernel.
What is wrong with Dell-specific SMI parsing in kernel? Is it _that_
much code?
--
Web maintainer for suspend.sf.net (http://www.sf.net/projects/suspend) wanted...
On Mon, 2006-02-20 at 17:53 +0100, Pavel Machek wrote:
> Hi!
>
> > Matthew has shown up on the libsmbios-devel mailing list. I sent
> > all the
> > info needed to do a test of Dell LCD brightness control. The main thing
> > left
> > would be to make one utility out of the current separate, unsupported,
> > test
> > utils.
> >
> > As for fixing i8k, I don't have the slightest clue where to
> > begin. You
> > either have to split initialization with userspace to parse and send in
> > the
> > correct io/magic ports to do SMI, or you have to put Dell-specific SMI
> > token
> > parsing in the kernel.
>
> What is wrong with Dell-specific SMI parsing in kernel? Is it _that_
> much code?
>
Pavel,
Sorry for the late response.
No, it isn't that much code. You are welcome to rip the parsing from
libsmbios. There are about three or so C structs necessary to parse it.
The total code is probably about 30-40 lines. (see
libraries/token/TokenDA.cpp, and libraries/common/TokenLowLevel.h).
My only point is that _everything_ that this module does can now be
completely and fully implemented in userspace using dcdbas and
libsmbios. I'm not against fixing the current i8k code, though. I don't
have the time to submit a patch myself, but would be happy to help any
interested volunteers.
--
Michael