RE: [PATCH] [RESEND] Add Dell laptop backlight brightness display

RE: [PATCH] [RESEND] Add Dell laptop backlight brightness display

[Michael E Brown]

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

Re: [PATCH] [RESEND] Add Dell laptop backlight brightness display

[Matthew Garrett]

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 | mjg59@srcf.ucam.org

RE: [PATCH] [RESEND] Add Dell laptop backlight brightness display

[Michael_E_Brown]

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:pavel@ucw.cz] 
Sent: Sunday, February 12, 2006 11:26 AM
To: Brown, Michael E
Cc: mjg59@srcf.ucam.org; akpm@osdl.org; Domsch, Matt;
linux-kernel@vger.kernel.org
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

Re: [PATCH] [RESEND] Add Dell laptop backlight brightness display

[Pavel Machek]

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 (www.sf.net/projects/suspend) wanted...

Re: [PATCH] [RESEND] Add Dell laptop backlight brightness display

[Michael E Brown]

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

RE: [PATCH] [RESEND] Add Dell laptop backlight brightness display

[Michael_E_Brown]

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:mjg59@srcf.ucam.org] 
Sent: Tuesday, February 07, 2006 11:20 AM
To: Brown, Michael E
Cc: akpm@osdl.org; Domsch, Matt; linux-kernel@vger.kernel.org
Subject: Re: [PATCH] [RESEND] Add Dell laptop backlight brightness
display

On Tue, Feb 07, 2006 at 10:34:31AM -0600, Michael_E_Brown@Dell.com
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 | mjg59@srcf.ucam.org

RE: [PATCH] [RESEND] Add Dell laptop backlight brightness display

[Michael_E_Brown]

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:mjg59@srcf.ucam.org] 
Sent: Monday, February 06, 2006 10:10 PM
To: Brown, Michael E
Cc: Andrew Morton; Domsch, Matt; linux-kernel@vger.kernel.org
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 | mjg59@srcf.ucam.org

Re: [PATCH] [RESEND] Add Dell laptop backlight brightness display

[Matthew Garrett]

On Tue, Feb 07, 2006 at 10:34:31AM -0600, Michael_E_Brown@Dell.com 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 | mjg59@srcf.ucam.org

Re: [PATCH] [RESEND] Add Dell laptop backlight brightness display

[Pavel Machek]

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         

[PATCH] Make DMI code store chassis type

[Matthew Garrett]

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 <mjg59@srcf.ucam.org>

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 | mjg59@srcf.ucam.org

[PATCH] Add HP laptop backlight brightness display

[Matthew Garrett]

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 <mjg59@srcf.ucam.org>

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 <mjg59@srcf.ucam.org>");
+MODULE_DESCRIPTION("HP Backlight Driver");
+MODULE_LICENSE("GPL");


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

[PATCH] Add Dell laptop backlight brightness display

[Matthew Garrett]

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 <mjg59@srcf.ucam.org>

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 <mjg59@srcf.ucam.org>");
+MODULE_DESCRIPTION("HP Backlight Driver");
+MODULE_LICENSE("GPL");

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

[PATCH] [RESEND] Add Dell laptop backlight brightness display

[Matthew Garrett]

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 <mjg59@srcf.ucam.org>

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 <mjg59@srcf.ucam.org>");
+MODULE_DESCRIPTION("Dell Backlight Driver");
+MODULE_LICENSE("GPL");

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

Re: [PATCH] [RESEND] Add Dell laptop backlight brightness display

[Dmitry Torokhov]

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

Re: [PATCH] [RESEND] Add Dell laptop backlight brightness display

[Matthew Garrett]

[-- Attachment #1: Type: text/plain, Size: 192 bytes --]

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 | mjg59@srcf.ucam.org

[-- Attachment #2: dell.diff --]
[-- Type: text/plain, Size: 3598 bytes --]

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(dellbl_init);
+module_exit(dellbl_exit);
+
+MODULE_AUTHOR("Matthew Garrett <mjg59@srcf.ucam.org>");
+MODULE_DESCRIPTION("Dell Backlight Driver");
+MODULE_LICENSE("GPL");

Re: [PATCH] [RESEND] Add Dell laptop backlight brightness display

[Richard Purdie]

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

Re: [PATCH] [RESEND] Add Dell laptop backlight brightness display

[Matthew Garrett]

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 | mjg59@srcf.ucam.org

Re: [PATCH] [RESEND] Add Dell laptop backlight brightness display

[Richard Purdie]

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

Re: [PATCH] [RESEND] Add Dell laptop backlight brightness display

[Matthew Garrett]

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 | mjg59@srcf.ucam.org

Re: [PATCH] [RESEND] Add Dell laptop backlight brightness display

[Richard Purdie]

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

Re: [PATCH] [RESEND] Add Dell laptop backlight brightness display

[Pavel Machek]

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 (www.sf.net/projects/suspend) wanted...