2008-06-07 02:54:50

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH] Add backlight driver for Nvidia-based Apple Macbook Pros

Nvidia-based Apple Macbook Pros don't appear to handle backlight control
through the graphics card registers or ACPI, but instead trigger changes
via SMI calls. This driver registers a generic backlight device that
lets existing userspace deal with it. Code derived from Julien Blache's
Pommed application.

Signed-off-by: Matthew Garrett <[email protected]>

---

Lennart - as far as I could tell from your description, the DMI
autoloading code for this should be correct. However, it's not working
for me on F9. Have I missed something, or does it just not work there?

diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index dcd8073..56f4572 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -112,3 +112,11 @@ config BACKLIGHT_CARILLO_RANCH
help
If you have a Intel LE80578 (Carillo Ranch) say Y to enable the
backlight driver.
+
+config BACKLIGHT_MBP_NVIDIA
+ tristate "Macbook Pro Nvidia Backlight Driver"
+ depends on BACKLIGHT_CLASS_DEVICE && X86
+ default n
+ help
+ If you have an Apple Macbook Pro with Nvidia graphics hardware say Y
+ to enable a driver for its backlight
diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
index 33f6c7c..5b91515 100644
--- a/drivers/video/backlight/Makefile
+++ b/drivers/video/backlight/Makefile
@@ -10,3 +10,4 @@ obj-$(CONFIG_BACKLIGHT_LOCOMO) += locomolcd.o
obj-$(CONFIG_BACKLIGHT_OMAP1) += omap1_bl.o
obj-$(CONFIG_BACKLIGHT_PROGEAR) += progear_bl.o
obj-$(CONFIG_BACKLIGHT_CARILLO_RANCH) += cr_bllcd.o
+obj-$(CONFIG_BACKLIGHT_MBP_NVIDIA) += mbp_nvidia_bl.o
diff --git a/drivers/video/backlight/mbp_nvidia_bl.c b/drivers/video/backlight/mbp_nvidia_bl.c
new file mode 100644
index 0000000..c622d22
--- /dev/null
+++ b/drivers/video/backlight/mbp_nvidia_bl.c
@@ -0,0 +1,114 @@
+/*
+ * Backlight Driver for Nvidia 8600 in Macbook Pro
+ *
+ * Copyright (c) Red Hat <[email protected]>
+ * Based on code from Pommed:
+ * Copyright (C) 2006 Nicolas Boichat <nicolas @boichat.ch>
+ * Copyright (C) 2006 Felipe Alfaro Solana <felipe_alfaro @linuxmail.org>
+ * Copyright (C) 2007 Julien BLACHE <[email protected]>
+ *
+ * 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.
+ *
+ * This driver triggers SMIs which cause the firmware to change the
+ * backlight brightness. This is icky in many ways, but it's impractical to
+ * get at the firmware code in order to figure out what it's actually doing.
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/backlight.h>
+#include <linux/err.h>
+#include <linux/dmi.h>
+#include <linux/io.h>
+
+struct backlight_device *mbp_backlight_device;
+
+static struct dmi_system_id __initdata mbp_device_table[] = {
+ {
+ .ident = "3,1",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro3,1"),
+ },
+ },
+ {
+ .ident = "3,2",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro3,2"),
+ },
+ },
+ {
+ .ident = "4,1",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro4,1"),
+ },
+ },
+ { }
+};
+
+static int mbp_send_intensity(struct backlight_device *bd)
+{
+ int intensity = bd->props.brightness;
+
+ outb(0x04 | (intensity << 4), 0xb3);
+ outb(0xbf, 0xb2);
+
+ return 0;
+}
+
+static int mbp_get_intensity(struct backlight_device *bd)
+{
+ outb(0x03, 0xb3);
+ outb(0xbf, 0xb2);
+ return inb(0xb3) >> 4;
+}
+
+static struct backlight_ops mbp_ops = {
+ .get_brightness = mbp_get_intensity,
+ .update_status = mbp_send_intensity,
+};
+
+static int __init mbp_init(void)
+{
+ if (!dmi_check_system(mbp_device_table))
+ return -ENODEV;
+
+ if (!request_region(0xb2, 2, "Macbook Pro backlight"))
+ return -ENXIO;
+
+ mbp_backlight_device = backlight_device_register("mbp_backlight",
+ NULL, NULL,
+ &mbp_ops);
+ if (IS_ERR(mbp_backlight_device))
+ return PTR_ERR(mbp_backlight_device);
+
+ mbp_backlight_device->props.max_brightness = 15;
+ mbp_backlight_device->props.brightness =
+ mbp_get_intensity(mbp_backlight_device);
+ backlight_update_status(mbp_backlight_device);
+
+ return 0;
+}
+
+static void __exit mbp_exit(void)
+{
+ backlight_device_unregister(mbp_backlight_device);
+
+ release_region(0xb2, 2);
+}
+
+module_init(mbp_init);
+module_exit(mbp_exit);
+
+MODULE_AUTHOR("Matthew Garrett <[email protected]>");
+MODULE_DESCRIPTION("Nvidia-based Macbook Pro Backlight Driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("dmi:*:svnApple Inc.:pnMacBookPro3,1:*");
+MODULE_ALIAS("dmi:*:svnApple Inc.:pnMacBookPro3,2:*");
+MODULE_ALIAS("dmi:*:svnApple Inc.:pnMacBookPro4,1:*");

--
Matthew Garrett | [email protected]


2008-06-07 05:09:07

by Andrey Panin

[permalink] [raw]
Subject: Re: [PATCH] Add backlight driver for Nvidia-based Apple Macbook Pros

On 159, 06 07, 2008 at 03:54:36 +0100, Matthew Garrett wrote:
> Nvidia-based Apple Macbook Pros don't appear to handle backlight control
> through the graphics card registers or ACPI, but instead trigger changes
> via SMI calls. This driver registers a generic backlight device that
> lets existing userspace deal with it. Code derived from Julien Blache's
> Pommed application.
>
> Signed-off-by: Matthew Garrett <[email protected]>
>
> ---
>
> Lennart - as far as I could tell from your description, the DMI
> autoloading code for this should be correct. However, it's not working
> for me on F9. Have I missed something, or does it just not work there?

BTW do we have any documentation to help ordinary human understand and generate
these aliases ?

> diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
> index dcd8073..56f4572 100644
> --- a/drivers/video/backlight/Kconfig
> +++ b/drivers/video/backlight/Kconfig
> @@ -112,3 +112,11 @@ config BACKLIGHT_CARILLO_RANCH
> help
> If you have a Intel LE80578 (Carillo Ranch) say Y to enable the
> backlight driver.
> +
> +config BACKLIGHT_MBP_NVIDIA
> + tristate "Macbook Pro Nvidia Backlight Driver"
> + depends on BACKLIGHT_CLASS_DEVICE && X86
> + default n
> + help
> + If you have an Apple Macbook Pro with Nvidia graphics hardware say Y
> + to enable a driver for its backlight
> diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
> index 33f6c7c..5b91515 100644
> --- a/drivers/video/backlight/Makefile
> +++ b/drivers/video/backlight/Makefile
> @@ -10,3 +10,4 @@ obj-$(CONFIG_BACKLIGHT_LOCOMO) += locomolcd.o
> obj-$(CONFIG_BACKLIGHT_OMAP1) += omap1_bl.o
> obj-$(CONFIG_BACKLIGHT_PROGEAR) += progear_bl.o
> obj-$(CONFIG_BACKLIGHT_CARILLO_RANCH) += cr_bllcd.o
> +obj-$(CONFIG_BACKLIGHT_MBP_NVIDIA) += mbp_nvidia_bl.o
> diff --git a/drivers/video/backlight/mbp_nvidia_bl.c b/drivers/video/backlight/mbp_nvidia_bl.c
> new file mode 100644
> index 0000000..c622d22
> --- /dev/null
> +++ b/drivers/video/backlight/mbp_nvidia_bl.c
> @@ -0,0 +1,114 @@
> +/*
> + * Backlight Driver for Nvidia 8600 in Macbook Pro
> + *
> + * Copyright (c) Red Hat <[email protected]>
> + * Based on code from Pommed:
> + * Copyright (C) 2006 Nicolas Boichat <nicolas @boichat.ch>
> + * Copyright (C) 2006 Felipe Alfaro Solana <felipe_alfaro @linuxmail.org>
> + * Copyright (C) 2007 Julien BLACHE <[email protected]>
> + *
> + * 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.
> + *
> + * This driver triggers SMIs which cause the firmware to change the
> + * backlight brightness. This is icky in many ways, but it's impractical to
> + * get at the firmware code in order to figure out what it's actually doing.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/platform_device.h>
> +#include <linux/backlight.h>
> +#include <linux/err.h>
> +#include <linux/dmi.h>
> +#include <linux/io.h>
> +
> +struct backlight_device *mbp_backlight_device;

Missing static ?

> +
> +static struct dmi_system_id __initdata mbp_device_table[] = {
> + {
> + .ident = "3,1",
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "Apple Inc."),
> + DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro3,1"),
> + },
> + },
> + {
> + .ident = "3,2",
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "Apple Inc."),
> + DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro3,2"),
> + },
> + },
> + {
> + .ident = "4,1",
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "Apple Inc."),
> + DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro4,1"),
> + },
> + },
> + { }
> +};
> +
> +static int mbp_send_intensity(struct backlight_device *bd)
> +{
> + int intensity = bd->props.brightness;
> +
> + outb(0x04 | (intensity << 4), 0xb3);
> + outb(0xbf, 0xb2);
> +
> + return 0;
> +}
> +
> +static int mbp_get_intensity(struct backlight_device *bd)
> +{
> + outb(0x03, 0xb3);
> + outb(0xbf, 0xb2);
> + return inb(0xb3) >> 4;
> +}
> +
> +static struct backlight_ops mbp_ops = {
> + .get_brightness = mbp_get_intensity,
> + .update_status = mbp_send_intensity,
> +};
> +
> +static int __init mbp_init(void)
> +{
> + if (!dmi_check_system(mbp_device_table))
> + return -ENODEV;
> +
> + if (!request_region(0xb2, 2, "Macbook Pro backlight"))
> + return -ENXIO;
> +
> + mbp_backlight_device = backlight_device_register("mbp_backlight",
> + NULL, NULL,
> + &mbp_ops);
> + if (IS_ERR(mbp_backlight_device))
> + return PTR_ERR(mbp_backlight_device);

You leak ioport region here.

> +
> + mbp_backlight_device->props.max_brightness = 15;
> + mbp_backlight_device->props.brightness =
> + mbp_get_intensity(mbp_backlight_device);
> + backlight_update_status(mbp_backlight_device);
> +
> + return 0;
> +}
> +
> +static void __exit mbp_exit(void)
> +{
> + backlight_device_unregister(mbp_backlight_device);
> +
> + release_region(0xb2, 2);
> +}
> +
> +module_init(mbp_init);
> +module_exit(mbp_exit);
> +
> +MODULE_AUTHOR("Matthew Garrett <[email protected]>");
> +MODULE_DESCRIPTION("Nvidia-based Macbook Pro Backlight Driver");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("dmi:*:svnApple Inc.:pnMacBookPro3,1:*");
> +MODULE_ALIAS("dmi:*:svnApple Inc.:pnMacBookPro3,2:*");
> +MODULE_ALIAS("dmi:*:svnApple Inc.:pnMacBookPro4,1:*");

--
Andrey Panin | Linux and UNIX system administrator
[email protected] | PGP key: wwwkeys.pgp.net


Attachments:
(No filename) (5.40 kB)
signature.asc (189.00 B)
Digital signature
Download all attachments

2008-06-07 06:08:17

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] Add backlight driver for Nvidia-based Apple Macbook Pros

On Sat, 7 Jun 2008 03:54:36 +0100 Matthew Garrett <[email protected]> wrote:

> Nvidia-based Apple Macbook Pros don't appear to handle backlight control
> through the graphics card registers or ACPI, but instead trigger changes
> via SMI calls. This driver registers a generic backlight device that
> lets existing userspace deal with it. Code derived from Julien Blache's
> Pommed application.
>
> Signed-off-by: Matthew Garrett <[email protected]>
>
> ...
>
> --- /dev/null
> +++ b/drivers/video/backlight/mbp_nvidia_bl.c
> @@ -0,0 +1,114 @@
> +/*
> + * Backlight Driver for Nvidia 8600 in Macbook Pro
> + *
> + * Copyright (c) Red Hat <[email protected]>
> + * Based on code from Pommed:
> + * Copyright (C) 2006 Nicolas Boichat <nicolas @boichat.ch>
> + * Copyright (C) 2006 Felipe Alfaro Solana <felipe_alfaro @linuxmail.org>
> + * Copyright (C) 2007 Julien BLACHE <[email protected]>

Did this patch have appropriate attribution and signoffs?

> + * 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.
> + *
> + * This driver triggers SMIs which cause the firmware to change the
> + * backlight brightness. This is icky in many ways, but it's impractical to
> + * get at the firmware code in order to figure out what it's actually doing.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/platform_device.h>
> +#include <linux/backlight.h>
> +#include <linux/err.h>
> +#include <linux/dmi.h>
> +#include <linux/io.h>
> +
> +struct backlight_device *mbp_backlight_device;
> +
> +static struct dmi_system_id __initdata mbp_device_table[] = {
> + {
> + .ident = "3,1",
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "Apple Inc."),
> + DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro3,1"),
> + },
> + },
> + {
> + .ident = "3,2",
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "Apple Inc."),
> + DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro3,2"),
> + },
> + },
> + {
> + .ident = "4,1",
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "Apple Inc."),
> + DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro4,1"),
> + },
> + },
> + { }
> +};
> +
> +static int mbp_send_intensity(struct backlight_device *bd)
> +{
> + int intensity = bd->props.brightness;
> +
> + outb(0x04 | (intensity << 4), 0xb3);
> + outb(0xbf, 0xb2);
> +
> + return 0;
> +}
> +
> +static int mbp_get_intensity(struct backlight_device *bd)
> +{
> + outb(0x03, 0xb3);
> + outb(0xbf, 0xb2);
> + return inb(0xb3) >> 4;
> +}
> +
> +static struct backlight_ops mbp_ops = {
> + .get_brightness = mbp_get_intensity,
> + .update_status = mbp_send_intensity,
> +};
> +
> +static int __init mbp_init(void)
> +{
> + if (!dmi_check_system(mbp_device_table))
> + return -ENODEV;
> +
> + if (!request_region(0xb2, 2, "Macbook Pro backlight"))
> + return -ENXIO;
> +
> + mbp_backlight_device = backlight_device_register("mbp_backlight",
> + NULL, NULL,
> + &mbp_ops);
> + if (IS_ERR(mbp_backlight_device))
> + return PTR_ERR(mbp_backlight_device);

Missing release_region()?

> + mbp_backlight_device->props.max_brightness = 15;
> + mbp_backlight_device->props.brightness =
> + mbp_get_intensity(mbp_backlight_device);
> + backlight_update_status(mbp_backlight_device);
> +
> + return 0;
> +}
> +
> +static void __exit mbp_exit(void)
> +{
> + backlight_device_unregister(mbp_backlight_device);
> +
> + release_region(0xb2, 2);
> +}
> +
> +module_init(mbp_init);
> +module_exit(mbp_exit);
> +
> +MODULE_AUTHOR("Matthew Garrett <[email protected]>");
> +MODULE_DESCRIPTION("Nvidia-based Macbook Pro Backlight Driver");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("dmi:*:svnApple Inc.:pnMacBookPro3,1:*");
> +MODULE_ALIAS("dmi:*:svnApple Inc.:pnMacBookPro3,2:*");
> +MODULE_ALIAS("dmi:*:svnApple Inc.:pnMacBookPro4,1:*");

2008-06-07 09:58:51

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH] Add backlight driver for Nvidia-based Apple Macbook Pros

On Fri, Jun 06, 2008 at 11:07:45PM -0700, Andrew Morton wrote:
> On Sat, 7 Jun 2008 03:54:36 +0100 Matthew Garrett <[email protected]> wrote:
> > + * Copyright (C) 2006 Nicolas Boichat <nicolas @boichat.ch>
> > + * Copyright (C) 2006 Felipe Alfaro Solana <felipe_alfaro @linuxmail.org>
> > + * Copyright (C) 2007 Julien BLACHE <[email protected]>
>
> Did this patch have appropriate attribution and signoffs?

Pommed is GPLv2.

> Missing release_region()?

Yup.

--
Matthew Garrett | [email protected]

2008-06-07 10:01:29

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH v2] Add backlight driver for Nvidia-based Apple Macbook Pros

Nvidia-based Apple Macbook Pros don't appear to handle backlight control
through the graphics card registers or ACPI, but instead trigger changes
via SMI calls. This driver registers a generic backlight device that
lets existing userspace deal with it. Code derived from Julien Blache's
Pommed application.

Signed-off-by: Matthew Garrett <[email protected]>

---

Added a missing static and a release_region() in the error path pointed
out by Andrey Panin and Andrew Morton

diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index dcd8073..56f4572 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -112,3 +112,11 @@ config BACKLIGHT_CARILLO_RANCH
help
If you have a Intel LE80578 (Carillo Ranch) say Y to enable the
backlight driver.
+
+config BACKLIGHT_MBP_NVIDIA
+ tristate "Macbook Pro Nvidia Backlight Driver"
+ depends on BACKLIGHT_CLASS_DEVICE && X86
+ default n
+ help
+ If you have an Apple Macbook Pro with Nvidia graphics hardware say Y
+ to enable a driver for its backlight
diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
index 33f6c7c..5b91515 100644
--- a/drivers/video/backlight/Makefile
+++ b/drivers/video/backlight/Makefile
@@ -10,3 +10,4 @@ obj-$(CONFIG_BACKLIGHT_LOCOMO) += locomolcd.o
obj-$(CONFIG_BACKLIGHT_OMAP1) += omap1_bl.o
obj-$(CONFIG_BACKLIGHT_PROGEAR) += progear_bl.o
obj-$(CONFIG_BACKLIGHT_CARILLO_RANCH) += cr_bllcd.o
+obj-$(CONFIG_BACKLIGHT_MBP_NVIDIA) += mbp_nvidia_bl.o
diff --git a/drivers/video/backlight/mbp_nvidia_bl.c b/drivers/video/backlight/mbp_nvidia_bl.c
new file mode 100644
index 0000000..ebe6be5
--- /dev/null
+++ b/drivers/video/backlight/mbp_nvidia_bl.c
@@ -0,0 +1,116 @@
+/*
+ * Backlight Driver for Nvidia 8600 in Macbook Pro
+ *
+ * Copyright (c) Red Hat <[email protected]>
+ * Based on code from Pommed:
+ * Copyright (C) 2006 Nicolas Boichat <nicolas @boichat.ch>
+ * Copyright (C) 2006 Felipe Alfaro Solana <felipe_alfaro @linuxmail.org>
+ * Copyright (C) 2007 Julien BLACHE <[email protected]>
+ *
+ * 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.
+ *
+ * This driver triggers SMIs which cause the firmware to change the
+ * backlight brightness. This is icky in many ways, but it's impractical to
+ * get at the firmware code in order to figure out what it's actually doing.
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/backlight.h>
+#include <linux/err.h>
+#include <linux/dmi.h>
+#include <linux/io.h>
+
+static struct backlight_device *mbp_backlight_device;
+
+static struct dmi_system_id __initdata mbp_device_table[] = {
+ {
+ .ident = "3,1",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro3,1"),
+ },
+ },
+ {
+ .ident = "3,2",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro3,2"),
+ },
+ },
+ {
+ .ident = "4,1",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro4,1"),
+ },
+ },
+ { }
+};
+
+static int mbp_send_intensity(struct backlight_device *bd)
+{
+ int intensity = bd->props.brightness;
+
+ outb(0x04 | (intensity << 4), 0xb3);
+ outb(0xbf, 0xb2);
+
+ return 0;
+}
+
+static int mbp_get_intensity(struct backlight_device *bd)
+{
+ outb(0x03, 0xb3);
+ outb(0xbf, 0xb2);
+ return inb(0xb3) >> 4;
+}
+
+static struct backlight_ops mbp_ops = {
+ .get_brightness = mbp_get_intensity,
+ .update_status = mbp_send_intensity,
+};
+
+static int __init mbp_init(void)
+{
+ if (!dmi_check_system(mbp_device_table))
+ return -ENODEV;
+
+ if (!request_region(0xb2, 2, "Macbook Pro backlight"))
+ return -ENXIO;
+
+ mbp_backlight_device = backlight_device_register("mbp_backlight",
+ NULL, NULL,
+ &mbp_ops);
+ if (IS_ERR(mbp_backlight_device)) {
+ release_region(0xb2, 2);
+ return PTR_ERR(mbp_backlight_device);
+ }
+
+ mbp_backlight_device->props.max_brightness = 15;
+ mbp_backlight_device->props.brightness =
+ mbp_get_intensity(mbp_backlight_device);
+ backlight_update_status(mbp_backlight_device);
+
+ return 0;
+}
+
+static void __exit mbp_exit(void)
+{
+ backlight_device_unregister(mbp_backlight_device);
+
+ release_region(0xb2, 2);
+}
+
+module_init(mbp_init);
+module_exit(mbp_exit);
+
+MODULE_AUTHOR("Matthew Garrett <[email protected]>");
+MODULE_DESCRIPTION("Nvidia-based Macbook Pro Backlight Driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("svnApple Inc.:pnMacBookPro3,1");
+MODULE_ALIAS("svnApple Inc.:pnMacBookPro3,2");
+MODULE_ALIAS("svnApple Inc.:pnMacBookPro4,1");


--
Matthew Garrett | [email protected]

2008-06-07 10:05:34

by Pekka Enberg

[permalink] [raw]
Subject: Re: [PATCH] Add backlight driver for Nvidia-based Apple Macbook Pros

Hi Matthew,

On Fri, Jun 06, 2008 at 11:07:45PM -0700, Andrew Morton wrote:
>> Did this patch have appropriate attribution and signoffs?

On Sat, Jun 7, 2008 at 12:58 PM, Matthew Garrett <[email protected]> wrote:
> Pommed is GPLv2.

Andrew has required signoffs from all copyright holders of a patch in
the past regardless of that.

2008-06-07 10:14:46

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH] Add backlight driver for Nvidia-based Apple Macbook Pros

On Sat, Jun 07, 2008 at 01:05:23PM +0300, Pekka Enberg wrote:
> Hi Matthew,
>
> On Fri, Jun 06, 2008 at 11:07:45PM -0700, Andrew Morton wrote:
> >> Did this patch have appropriate attribution and signoffs?
>
> On Sat, Jun 7, 2008 at 12:58 PM, Matthew Garrett <[email protected]> wrote:
> > Pommed is GPLv2.
>
> Andrew has required signoffs from all copyright holders of a patch in
> the past regardless of that.

SubmittingPatches says (in part):

(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source
license and I have the right under that license to submit that
work with modifications, whether created in whole or in part
by me, under the same open source license (unless I am
permitted to submit under a different license), as indicated
in the file; or

If that's not sufficient, then the documentation needs fixing. It's also
not current practice, as far as I can tell - did ath5k have sign-offs
from the BSD people?

--
Matthew Garrett | [email protected]

2008-06-07 10:50:40

by Pekka Enberg

[permalink] [raw]
Subject: Re: [PATCH] Add backlight driver for Nvidia-based Apple Macbook Pros

On Sat, Jun 7, 2008 at 1:14 PM, Matthew Garrett <[email protected]> wrote:
> If that's not sufficient, then the documentation needs fixing. It's also
> not current practice, as far as I can tell - did ath5k have sign-offs
> from the BSD people?

I don't know about ath5k. But when we submitted the IP1000 driver, we
had to contact the copyright holder (a company) and get proper
signoffs before Andrew acked the merge.

2008-06-07 14:55:13

by Julien BLACHE

[permalink] [raw]
Subject: Re: [PATCH v2] Add backlight driver for Nvidia-based Apple Macbook Pros

Matthew Garrett <[email protected]> wrote:

Hi,

> Signed-off-by: Matthew Garrett <[email protected]>

If you need it, here you have it:

Signed-off-by: Julien Blache <[email protected]>

Minor nitpicks follow:

> +config BACKLIGHT_MBP_NVIDIA
> + tristate "Macbook Pro Nvidia Backlight Driver"
^^^^^^^
Proper capitalization is "MacBook", also to be consistent with other
help texts in the kernel (I hope).

> + * 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.

This particular file in pommed is GPL v2 or later; it's external
code I included in pommed and the original license was GPL v2 or
later.

JB.

--
Julien BLACHE <http://www.jblache.org>
<[email protected]> GPG KeyID 0xF5D65169

2008-06-07 19:00:29

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] Add backlight driver for Nvidia-based Apple Macbook Pros

On Sat, 7 Jun 2008 13:05:23 +0300 "Pekka Enberg" <[email protected]> wrote:

> Hi Matthew,
>
> On Fri, Jun 06, 2008 at 11:07:45PM -0700, Andrew Morton wrote:
> >> Did this patch have appropriate attribution and signoffs?
>
> On Sat, Jun 7, 2008 at 12:58 PM, Matthew Garrett <[email protected]> wrote:
> > Pommed is GPLv2.
>
> Andrew has required signoffs from all copyright holders of a patch in
> the past regardless of that.

More like "asked for". It's best to gather those signoffs if the
people are around.

Getting the From: correct is (IMO) more important.

2008-06-08 18:39:39

by Lennart Poettering

[permalink] [raw]
Subject: Re: [PATCH] Add backlight driver for Nvidia-based Apple Macbook Pros

On Sat, 07.06.08 03:54, Matthew Garrett ([email protected]) wrote:

> Lennart - as far as I could tell from your description, the DMI
> autoloading code for this should be correct. However, it's not working
> for me on F9. Have I missed something, or does it just not work there?

It should be pretty easy to figure out the right string for
DMI autoloading. Just do a cat /sys/class/dmi/id/modalias and replace the
part of the string you consider not very useful for identifying the
machine with (dates, version numbers, empty strings, 0123456
rubbish data, ...) with an asterisk.

Looking at those strings included in your patch they look pretty much
correct, but since I don't have access to a Mac I cannot really check
this.

It's udev's job to match the modalias strings of your driver module
with the modalias string from /sys/class/dmi/id/modalias. You can
retrigger this matching by doing something like "echo 1 >
/sys/class/dmi/id/uevent" or suchlike, but I don't remember the
details.

Lennart

--
Lennart Poettering Red Hat, Inc.
lennart [at] poettering [dot] net ICQ# 11060553
http://0pointer.net/lennart/ GnuPG 0x1A015CC4

2008-06-09 00:03:50

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH] Add backlight driver for Nvidia-based Apple Macbook Pros

On Sun, Jun 08, 2008 at 08:32:55PM +0200, Lennart Poettering wrote:

> It should be pretty easy to figure out the right string for
> DMI autoloading. Just do a cat /sys/class/dmi/id/modalias and replace the
> part of the string you consider not very useful for identifying the
> machine with (dates, version numbers, empty strings, 0123456
> rubbish data, ...) with an asterisk.

Ah! Spaces are removed in the sysfs interface. That would explain the
problem. I'll send a new version.

--
Matthew Garrett | [email protected]

2008-06-09 00:06:14

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH v2] Add backlight driver for Nvidia-based Apple Macbook Pros

Nvidia-based Apple Macbook Pros don't appear to handle backlight control
through the graphics card registers or ACPI, but instead trigger changes
via SMI calls. This driver registers a generic backlight device that
lets existing userspace deal with it. Code derived from Julien Blache's
Pommed application.

Signed-off-by: Matthew Garrett <[email protected]>

---

Fixed the DMI strings and changed the capitalisation of "MacBook" in the
Kconfig text.

diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index dcd8073..56f4572 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -112,3 +112,11 @@ config BACKLIGHT_CARILLO_RANCH
help
If you have a Intel LE80578 (Carillo Ranch) say Y to enable the
backlight driver.
+
+config BACKLIGHT_MBP_NVIDIA
+ tristate "MacBook Pro Nvidia Backlight Driver"
+ depends on BACKLIGHT_CLASS_DEVICE && X86
+ default n
+ help
+ If you have an Apple Macbook Pro with Nvidia graphics hardware say Y
+ to enable a driver for its backlight
diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
index 33f6c7c..5b91515 100644
--- a/drivers/video/backlight/Makefile
+++ b/drivers/video/backlight/Makefile
@@ -10,3 +10,4 @@ obj-$(CONFIG_BACKLIGHT_LOCOMO) += locomolcd.o
obj-$(CONFIG_BACKLIGHT_OMAP1) += omap1_bl.o
obj-$(CONFIG_BACKLIGHT_PROGEAR) += progear_bl.o
obj-$(CONFIG_BACKLIGHT_CARILLO_RANCH) += cr_bllcd.o
+obj-$(CONFIG_BACKLIGHT_MBP_NVIDIA) += mbp_nvidia_bl.o
diff --git a/drivers/video/backlight/mbp_nvidia_bl.c b/drivers/video/backlight/mbp_nvidia_bl.c
new file mode 100644
index 0000000..ebe6be5
--- /dev/null
+++ b/drivers/video/backlight/mbp_nvidia_bl.c
@@ -0,0 +1,116 @@
+/*
+ * Backlight Driver for Nvidia 8600 in Macbook Pro
+ *
+ * Copyright (c) Red Hat <[email protected]>
+ * Based on code from Pommed:
+ * Copyright (C) 2006 Nicolas Boichat <nicolas @boichat.ch>
+ * Copyright (C) 2006 Felipe Alfaro Solana <felipe_alfaro @linuxmail.org>
+ * Copyright (C) 2007 Julien BLACHE <[email protected]>
+ *
+ * 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.
+ *
+ * This driver triggers SMIs which cause the firmware to change the
+ * backlight brightness. This is icky in many ways, but it's impractical to
+ * get at the firmware code in order to figure out what it's actually doing.
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/backlight.h>
+#include <linux/err.h>
+#include <linux/dmi.h>
+#include <linux/io.h>
+
+static struct backlight_device *mbp_backlight_device;
+
+static struct dmi_system_id __initdata mbp_device_table[] = {
+ {
+ .ident = "3,1",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro3,1"),
+ },
+ },
+ {
+ .ident = "3,2",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro3,2"),
+ },
+ },
+ {
+ .ident = "4,1",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Apple Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro4,1"),
+ },
+ },
+ { }
+};
+
+static int mbp_send_intensity(struct backlight_device *bd)
+{
+ int intensity = bd->props.brightness;
+
+ outb(0x04 | (intensity << 4), 0xb3);
+ outb(0xbf, 0xb2);
+
+ return 0;
+}
+
+static int mbp_get_intensity(struct backlight_device *bd)
+{
+ outb(0x03, 0xb3);
+ outb(0xbf, 0xb2);
+ return inb(0xb3) >> 4;
+}
+
+static struct backlight_ops mbp_ops = {
+ .get_brightness = mbp_get_intensity,
+ .update_status = mbp_send_intensity,
+};
+
+static int __init mbp_init(void)
+{
+ if (!dmi_check_system(mbp_device_table))
+ return -ENODEV;
+
+ if (!request_region(0xb2, 2, "Macbook Pro backlight"))
+ return -ENXIO;
+
+ mbp_backlight_device = backlight_device_register("mbp_backlight",
+ NULL, NULL,
+ &mbp_ops);
+ if (IS_ERR(mbp_backlight_device)) {
+ release_region(0xb2, 2);
+ return PTR_ERR(mbp_backlight_device);
+ }
+
+ mbp_backlight_device->props.max_brightness = 15;
+ mbp_backlight_device->props.brightness =
+ mbp_get_intensity(mbp_backlight_device);
+ backlight_update_status(mbp_backlight_device);
+
+ return 0;
+}
+
+static void __exit mbp_exit(void)
+{
+ backlight_device_unregister(mbp_backlight_device);
+
+ release_region(0xb2, 2);
+}
+
+module_init(mbp_init);
+module_exit(mbp_exit);
+
+MODULE_AUTHOR("Matthew Garrett <[email protected]>");
+MODULE_DESCRIPTION("Nvidia-based Macbook Pro Backlight Driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("svnAppleInc.:pnMacBookPro3,1");
+MODULE_ALIAS("svnAppleInc.:pnMacBookPro3,2");
+MODULE_ALIAS("svnAppleInc.:pnMacBookPro4,1");

--
Matthew Garrett | [email protected]

2008-06-09 21:24:13

by Richard Purdie

[permalink] [raw]
Subject: Re: [PATCH v2] Add backlight driver for Nvidia-based Apple Macbook Pros

On Mon, 2008-06-09 at 01:05 +0100, Matthew Garrett wrote:
> Nvidia-based Apple Macbook Pros don't appear to handle backlight control
> through the graphics card registers or ACPI, but instead trigger changes
> via SMI calls. This driver registers a generic backlight device that
> lets existing userspace deal with it. Code derived from Julien Blache's
> Pommed application.
>
> Signed-off-by: Matthew Garrett <[email protected]>

Queued in the backlight tree, thanks.

Richard