[PATCH] platform/x86: Add driver for Apple gmux device

[PATCH] platform/x86: Add driver for Apple gmux device

[Seth Forshee]

Apple laptops with hybrid graphics have a device named gmux that
controls the muxing of the LVDS panel between the GPUs as well as screen
brightness. This driver adds support for the gmux device. Only backlight
control is supported initially.

Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
 drivers/platform/x86/Kconfig      |   10 ++
 drivers/platform/x86/Makefile     |    1 +
 drivers/platform/x86/apple-gmux.c |  234 +++++++++++++++++++++++++++++++++++++
 3 files changed, 245 insertions(+), 0 deletions(-)
 create mode 100644 drivers/platform/x86/apple-gmux.c

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index f995e6e..a2a9ede 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -779,4 +779,14 @@ config SAMSUNG_Q10
 	  This driver provides support for backlight control on Samsung Q10
 	  and related laptops, including Dell Latitude X200.
 
+config APPLE_GMUX
+	tristate "Apple Gmux Driver"
+	depends on PNP
+	select BACKLIGHT_CLASS_DEVICE
+	---help---
+	  This driver provides support for the gmux device found on many
+	  Apple laptops, which controls the display mux for the hybrid
+	  graphics as well as the backlight. Currently only backlight
+	  control is supported by the driver.
+
 endif # X86_PLATFORM_DEVICES
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 293a320..a49c891 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -45,3 +45,4 @@ obj-$(CONFIG_MXM_WMI)		+= mxm-wmi.o
 obj-$(CONFIG_INTEL_MID_POWER_BUTTON)	+= intel_mid_powerbtn.o
 obj-$(CONFIG_INTEL_OAKTRAIL)	+= intel_oaktrail.o
 obj-$(CONFIG_SAMSUNG_Q10)	+= samsung-q10.o
+obj-$(CONFIG_APPLE_GMUX)	+= apple-gmux.o
diff --git a/drivers/platform/x86/apple-gmux.c b/drivers/platform/x86/apple-gmux.c
new file mode 100644
index 0000000..492abee
--- /dev/null
+++ b/drivers/platform/x86/apple-gmux.c
@@ -0,0 +1,234 @@
+/*
+ *  Gmux driver for Apple laptops
+ *
+ *  Copyright (C) Canonical Ltd. <seth.forshee@canonical.com>
+ *
+ *  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.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/backlight.h>
+#include <linux/acpi.h>
+#include <linux/pnp.h>
+
+struct apple_gmux_data {
+	unsigned long iostart;
+	unsigned long iolen;
+
+	int max_brightness;
+	struct backlight_device *bdev;
+};
+
+/*
+ * gmux port offsets. Many of these are not yet used, but may be in the
+ * future, and it's useful to have them documented here anyhow.
+ */
+#define GMUX_PORT_VERSION_MAJOR		0x04
+#define GMUX_PORT_VERSION_MINOR		0x05
+#define GMUX_PORT_VERSION_RELEASE	0x06
+#define GMUX_PORT_SWITCH_DISPLAY	0x10
+#define GMUX_PORT_SWITCH_GET_DISPLAY	0x11
+#define GMUX_PORT_INTERRUPT_ENABLE	0x14
+#define GMUX_PORT_INTERRUPT_STATUS	0x16
+#define GMUX_PORT_SWITCH_DDC		0x28
+#define GMUX_PORT_SWITCH_EXTERNAL	0x40
+#define GMUX_PORT_SWITCH_GET_EXTERNAL	0x41
+#define GMUX_PORT_DISCRETE_POWER	0x50
+#define GMUX_PORT_MAX_BRIGHTNESS	0x70
+#define GMUX_PORT_BRIGHTNESS		0x74
+
+#define GMUX_MIN_IO_LEN			(GMUX_PORT_BRIGHTNESS + 4)
+
+#define GMUX_INTERRUPT_ENABLE		0xff
+#define GMUX_INTERRUPT_DISABLE		0x00
+
+#define GMUX_INTERRUPT_STATUS_ACTIVE	0
+#define GMUX_INTERRUPT_STATUS_DISPLAY	(1 << 0)
+#define GMUX_INTERRUPT_STATUS_POWER	(1 << 2)
+#define GMUX_INTERRUPT_STATUS_HOTPLUG	(1 << 3)
+
+#define GMUX_BRIGHTNESS_MASK		0x00ffffff
+#define GMUX_MAX_BRIGHTNESS		GMUX_BRIGHTNESS_MASK
+
+static inline u8 gmux_read8(struct apple_gmux_data *gmux_data, int port)
+{
+	return inb(gmux_data->iostart + port);
+}
+
+static inline void gmux_write8(struct apple_gmux_data *gmux_data, int port,
+			       u8 val)
+{
+	outb(val, gmux_data->iostart + port);
+}
+
+static inline u32 gmux_read32(struct apple_gmux_data *gmux_data, int port)
+{
+	return inl(gmux_data->iostart + port);
+}
+
+static inline void gmux_write32(struct apple_gmux_data *gmux_data, int port,
+				u32 val)
+{
+	outl(val, gmux_data->iostart + port);
+}
+
+static int gmux_get_brightness(struct backlight_device *bd)
+{
+	struct apple_gmux_data *gmux_data = bl_get_data(bd);
+	return (int)gmux_read32(gmux_data, GMUX_PORT_BRIGHTNESS);
+}
+
+static int gmux_update_status(struct backlight_device *bd)
+{
+	struct apple_gmux_data *gmux_data = bl_get_data(bd);
+	u32 brightness = bd->props.brightness;
+
+	/*
+	 * Older gmux versions require writing out lower bytes first then
+	 * setting the upper byte to 0 to flush the values. Newer versions
+	 * accept a single u32 write, but the old method also work, so we
+	 * just use the old method for all gmux versions.
+	 */
+	gmux_write8(gmux_data, GMUX_PORT_BRIGHTNESS, brightness);
+	gmux_write8(gmux_data, GMUX_PORT_BRIGHTNESS + 1, brightness >> 8);
+	gmux_write8(gmux_data, GMUX_PORT_BRIGHTNESS + 2, brightness >> 16);
+	gmux_write8(gmux_data, GMUX_PORT_BRIGHTNESS + 3, 0);
+
+	return 0;
+}
+
+static const struct backlight_ops gmux_bl_ops = {
+	.get_brightness = gmux_get_brightness,
+	.update_status = gmux_update_status,
+};
+
+static int __devinit gmux_probe(struct pnp_dev *pnp,
+				const struct pnp_device_id *id)
+{
+	struct apple_gmux_data *gmux_data;
+	struct resource *res;
+	struct backlight_properties props;
+	struct backlight_device *bdev;
+	u8 ver_major, ver_minor, ver_release;
+	int ret = -ENXIO;
+
+	gmux_data = kzalloc(sizeof(*gmux_data), GFP_KERNEL);
+	if (!gmux_data)
+		return -ENOMEM;
+	pnp_set_drvdata(pnp, gmux_data);
+
+	res = pnp_get_resource(pnp, IORESOURCE_IO, 0);
+	if (!res) {
+		pr_err("Failed to find gmux I/O resource\n");
+		goto err_free;
+	}
+
+	gmux_data->iostart = res->start;
+	gmux_data->iolen = res->end - res->start;
+
+	if (gmux_data->iolen < GMUX_MIN_IO_LEN) {
+		pr_err("gmux I/O region too small (%lu < %u)\n",
+		       gmux_data->iolen, GMUX_MIN_IO_LEN);
+		goto err_free;
+	}
+
+	if (!request_region(gmux_data->iostart, gmux_data->iolen,
+			    "Apple gmux")) {
+		pr_err("gmux I/O already in use\n");
+		goto err_free;
+	}
+
+	/*
+	 * On some machines the gmux is in ACPI even thought the machine
+	 * doesn't really have a gmux. Check for invalid version information
+	 * to detect this.
+	 */
+	ver_major = gmux_read8(gmux_data, GMUX_PORT_VERSION_MAJOR);
+	ver_minor = gmux_read8(gmux_data, GMUX_PORT_VERSION_MINOR);
+	ver_release = gmux_read8(gmux_data, GMUX_PORT_VERSION_RELEASE);
+	if (ver_major == 0xff && ver_minor == 0xff && ver_release == 0xff) {
+		pr_info("gmux device not present\n");
+		ret = -ENODEV;
+		goto err_release;
+	}
+
+	pr_info("Found gmux version %d.%d.%d\n", ver_major, ver_minor,
+		ver_release);
+
+	memset(&props, 0, sizeof(props));
+	props.type = BACKLIGHT_PLATFORM;
+	props.max_brightness = gmux_read32(gmux_data, GMUX_PORT_MAX_BRIGHTNESS);
+
+	/*
+	 * Currently it's assumed that the maximum brightness is less than
+	 * 2^24 for compatibility with old gmux versions. Cap the max
+	 * brightness at this value, but print a warning if the hardware
+	 * reports something higher so that it can be fixed.
+	 */
+	if (WARN_ON(props.max_brightness > GMUX_MAX_BRIGHTNESS))
+		props.max_brightness = GMUX_MAX_BRIGHTNESS;
+
+	bdev = backlight_device_register("gmux_backlight", &pnp->dev,
+					 gmux_data, &gmux_bl_ops, &props);
+	if (IS_ERR(bdev)) {
+		ret = PTR_ERR(bdev);
+		goto err_release;
+	}
+
+	gmux_data->bdev = bdev;
+	bdev->props.brightness = gmux_read32(gmux_data, GMUX_PORT_BRIGHTNESS);
+	backlight_update_status(bdev);
+
+	return 0;
+
+err_release:
+	release_region(gmux_data->iostart, gmux_data->iolen);
+err_free:
+	kfree(gmux_data);
+	return ret;
+}
+
+static void __devexit gmux_remove(struct pnp_dev *pnp)
+{
+	struct apple_gmux_data *gmux_data = pnp_get_drvdata(pnp);
+
+	backlight_device_unregister(gmux_data->bdev);
+	release_region(gmux_data->iostart, gmux_data->iolen);
+	kfree(gmux_data);
+}
+
+static const struct pnp_device_id gmux_device_ids[] = {
+	{"APP000B", 0},
+	{"", 0}
+};
+
+static struct pnp_driver gmux_pnp_driver = {
+	.name		= "apple-gmux",
+	.probe		= gmux_probe,
+	.remove		= __devexit_p(gmux_remove),
+	.id_table	= gmux_device_ids,
+};
+
+static int __init apple_gmux_init(void)
+{
+	return pnp_register_driver(&gmux_pnp_driver);
+}
+
+static void __exit apple_gmux_exit(void)
+{
+	pnp_unregister_driver(&gmux_pnp_driver);
+}
+
+module_init(apple_gmux_init);
+module_exit(apple_gmux_exit);
+
+MODULE_AUTHOR("Seth Forshee <seth.forshee@canonical.com>");
+MODULE_DESCRIPTION("Apple Gmux Driver");
+MODULE_LICENSE("GPL");
+MODULE_DEVICE_TABLE(pnp, gmux_device_ids);
-- 
1.7.9

[PATCH v2] platform/x86: Add driver for Apple gmux device

[Seth Forshee]

Apple laptops with hybrid graphics have a device named gmux that
controls the muxing of the LVDS panel between the GPUs as well as screen
brightness. This driver adds support for the gmux device. Only backlight
control is supported initially.

Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---

This contains some minor updates to the previous submission. Changes in
v2:

 - Removed unused max_brightness field from struct apple_gmux_data
 - Removed unused gmux_write32() function
 - Mask off unused bits when reading brightness
 - Use gmux_get_brightness() when initializing backlight device
   brightness field
 - Fix some bad grammar in one of the comments.

 drivers/platform/x86/Kconfig      |   10 ++
 drivers/platform/x86/Makefile     |    1 +
 drivers/platform/x86/apple-gmux.c |  228 +++++++++++++++++++++++++++++++++++++
 3 files changed, 239 insertions(+), 0 deletions(-)
 create mode 100644 drivers/platform/x86/apple-gmux.c

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index f995e6e..a2a9ede 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -779,4 +779,14 @@ config SAMSUNG_Q10
 	  This driver provides support for backlight control on Samsung Q10
 	  and related laptops, including Dell Latitude X200.
 
+config APPLE_GMUX
+	tristate "Apple Gmux Driver"
+	depends on PNP
+	select BACKLIGHT_CLASS_DEVICE
+	---help---
+	  This driver provides support for the gmux device found on many
+	  Apple laptops, which controls the display mux for the hybrid
+	  graphics as well as the backlight. Currently only backlight
+	  control is supported by the driver.
+
 endif # X86_PLATFORM_DEVICES
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 293a320..a49c891 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -45,3 +45,4 @@ obj-$(CONFIG_MXM_WMI)		+= mxm-wmi.o
 obj-$(CONFIG_INTEL_MID_POWER_BUTTON)	+= intel_mid_powerbtn.o
 obj-$(CONFIG_INTEL_OAKTRAIL)	+= intel_oaktrail.o
 obj-$(CONFIG_SAMSUNG_Q10)	+= samsung-q10.o
+obj-$(CONFIG_APPLE_GMUX)	+= apple-gmux.o
diff --git a/drivers/platform/x86/apple-gmux.c b/drivers/platform/x86/apple-gmux.c
new file mode 100644
index 0000000..efc1fa9
--- /dev/null
+++ b/drivers/platform/x86/apple-gmux.c
@@ -0,0 +1,228 @@
+/*
+ *  Gmux driver for Apple laptops
+ *
+ *  Copyright (C) Canonical Ltd. <seth.forshee@canonical.com>
+ *
+ *  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.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/backlight.h>
+#include <linux/acpi.h>
+#include <linux/pnp.h>
+
+struct apple_gmux_data {
+	unsigned long iostart;
+	unsigned long iolen;
+
+	struct backlight_device *bdev;
+};
+
+/*
+ * gmux port offsets. Many of these are not yet used, but may be in the
+ * future, and it's useful to have them documented here anyhow.
+ */
+#define GMUX_PORT_VERSION_MAJOR		0x04
+#define GMUX_PORT_VERSION_MINOR		0x05
+#define GMUX_PORT_VERSION_RELEASE	0x06
+#define GMUX_PORT_SWITCH_DISPLAY	0x10
+#define GMUX_PORT_SWITCH_GET_DISPLAY	0x11
+#define GMUX_PORT_INTERRUPT_ENABLE	0x14
+#define GMUX_PORT_INTERRUPT_STATUS	0x16
+#define GMUX_PORT_SWITCH_DDC		0x28
+#define GMUX_PORT_SWITCH_EXTERNAL	0x40
+#define GMUX_PORT_SWITCH_GET_EXTERNAL	0x41
+#define GMUX_PORT_DISCRETE_POWER	0x50
+#define GMUX_PORT_MAX_BRIGHTNESS	0x70
+#define GMUX_PORT_BRIGHTNESS		0x74
+
+#define GMUX_MIN_IO_LEN			(GMUX_PORT_BRIGHTNESS + 4)
+
+#define GMUX_INTERRUPT_ENABLE		0xff
+#define GMUX_INTERRUPT_DISABLE		0x00
+
+#define GMUX_INTERRUPT_STATUS_ACTIVE	0
+#define GMUX_INTERRUPT_STATUS_DISPLAY	(1 << 0)
+#define GMUX_INTERRUPT_STATUS_POWER	(1 << 2)
+#define GMUX_INTERRUPT_STATUS_HOTPLUG	(1 << 3)
+
+#define GMUX_BRIGHTNESS_MASK		0x00ffffff
+#define GMUX_MAX_BRIGHTNESS		GMUX_BRIGHTNESS_MASK
+
+static inline u8 gmux_read8(struct apple_gmux_data *gmux_data, int port)
+{
+	return inb(gmux_data->iostart + port);
+}
+
+static inline void gmux_write8(struct apple_gmux_data *gmux_data, int port,
+			       u8 val)
+{
+	outb(val, gmux_data->iostart + port);
+}
+
+static inline u32 gmux_read32(struct apple_gmux_data *gmux_data, int port)
+{
+	return inl(gmux_data->iostart + port);
+}
+
+static int gmux_get_brightness(struct backlight_device *bd)
+{
+	struct apple_gmux_data *gmux_data = bl_get_data(bd);
+	return gmux_read32(gmux_data, GMUX_PORT_BRIGHTNESS) &
+	       GMUX_BRIGHTNESS_MASK;
+}
+
+static int gmux_update_status(struct backlight_device *bd)
+{
+	struct apple_gmux_data *gmux_data = bl_get_data(bd);
+	u32 brightness = bd->props.brightness;
+
+	/*
+	 * Older gmux versions require writing out lower bytes first then
+	 * setting the upper byte to 0 to flush the values. Newer versions
+	 * accept a single u32 write, but the old method also works, so we
+	 * just use the old method for all gmux versions.
+	 */
+	gmux_write8(gmux_data, GMUX_PORT_BRIGHTNESS, brightness);
+	gmux_write8(gmux_data, GMUX_PORT_BRIGHTNESS + 1, brightness >> 8);
+	gmux_write8(gmux_data, GMUX_PORT_BRIGHTNESS + 2, brightness >> 16);
+	gmux_write8(gmux_data, GMUX_PORT_BRIGHTNESS + 3, 0);
+
+	return 0;
+}
+
+static const struct backlight_ops gmux_bl_ops = {
+	.get_brightness = gmux_get_brightness,
+	.update_status = gmux_update_status,
+};
+
+static int __devinit gmux_probe(struct pnp_dev *pnp,
+				const struct pnp_device_id *id)
+{
+	struct apple_gmux_data *gmux_data;
+	struct resource *res;
+	struct backlight_properties props;
+	struct backlight_device *bdev;
+	u8 ver_major, ver_minor, ver_release;
+	int ret = -ENXIO;
+
+	gmux_data = kzalloc(sizeof(*gmux_data), GFP_KERNEL);
+	if (!gmux_data)
+		return -ENOMEM;
+	pnp_set_drvdata(pnp, gmux_data);
+
+	res = pnp_get_resource(pnp, IORESOURCE_IO, 0);
+	if (!res) {
+		pr_err("Failed to find gmux I/O resource\n");
+		goto err_free;
+	}
+
+	gmux_data->iostart = res->start;
+	gmux_data->iolen = res->end - res->start;
+
+	if (gmux_data->iolen < GMUX_MIN_IO_LEN) {
+		pr_err("gmux I/O region too small (%lu < %u)\n",
+		       gmux_data->iolen, GMUX_MIN_IO_LEN);
+		goto err_free;
+	}
+
+	if (!request_region(gmux_data->iostart, gmux_data->iolen,
+			    "Apple gmux")) {
+		pr_err("gmux I/O already in use\n");
+		goto err_free;
+	}
+
+	/*
+	 * On some machines the gmux is in ACPI even thought the machine
+	 * doesn't really have a gmux. Check for invalid version information
+	 * to detect this.
+	 */
+	ver_major = gmux_read8(gmux_data, GMUX_PORT_VERSION_MAJOR);
+	ver_minor = gmux_read8(gmux_data, GMUX_PORT_VERSION_MINOR);
+	ver_release = gmux_read8(gmux_data, GMUX_PORT_VERSION_RELEASE);
+	if (ver_major == 0xff && ver_minor == 0xff && ver_release == 0xff) {
+		pr_info("gmux device not present\n");
+		ret = -ENODEV;
+		goto err_release;
+	}
+
+	pr_info("Found gmux version %d.%d.%d\n", ver_major, ver_minor,
+		ver_release);
+
+	memset(&props, 0, sizeof(props));
+	props.type = BACKLIGHT_PLATFORM;
+	props.max_brightness = gmux_read32(gmux_data, GMUX_PORT_MAX_BRIGHTNESS);
+
+	/*
+	 * Currently it's assumed that the maximum brightness is less than
+	 * 2^24 for compatibility with old gmux versions. Cap the max
+	 * brightness at this value, but print a warning if the hardware
+	 * reports something higher so that it can be fixed.
+	 */
+	if (WARN_ON(props.max_brightness > GMUX_MAX_BRIGHTNESS))
+		props.max_brightness = GMUX_MAX_BRIGHTNESS;
+
+	bdev = backlight_device_register("gmux_backlight", &pnp->dev,
+					 gmux_data, &gmux_bl_ops, &props);
+	if (IS_ERR(bdev)) {
+		ret = PTR_ERR(bdev);
+		goto err_release;
+	}
+
+	gmux_data->bdev = bdev;
+	bdev->props.brightness = gmux_get_brightness(bdev);
+	backlight_update_status(bdev);
+
+	return 0;
+
+err_release:
+	release_region(gmux_data->iostart, gmux_data->iolen);
+err_free:
+	kfree(gmux_data);
+	return ret;
+}
+
+static void __devexit gmux_remove(struct pnp_dev *pnp)
+{
+	struct apple_gmux_data *gmux_data = pnp_get_drvdata(pnp);
+
+	backlight_device_unregister(gmux_data->bdev);
+	release_region(gmux_data->iostart, gmux_data->iolen);
+	kfree(gmux_data);
+}
+
+static const struct pnp_device_id gmux_device_ids[] = {
+	{"APP000B", 0},
+	{"", 0}
+};
+
+static struct pnp_driver gmux_pnp_driver = {
+	.name		= "apple-gmux",
+	.probe		= gmux_probe,
+	.remove		= __devexit_p(gmux_remove),
+	.id_table	= gmux_device_ids,
+};
+
+static int __init apple_gmux_init(void)
+{
+	return pnp_register_driver(&gmux_pnp_driver);
+}
+
+static void __exit apple_gmux_exit(void)
+{
+	pnp_unregister_driver(&gmux_pnp_driver);
+}
+
+module_init(apple_gmux_init);
+module_exit(apple_gmux_exit);
+
+MODULE_AUTHOR("Seth Forshee <seth.forshee@canonical.com>");
+MODULE_DESCRIPTION("Apple Gmux Driver");
+MODULE_LICENSE("GPL");
+MODULE_DEVICE_TABLE(pnp, gmux_device_ids);
-- 
1.7.9

Re: [PATCH v2] platform/x86: Add driver for Apple gmux device

[Grant Likely]

On Wed, Feb 22, 2012 at 8:37 AM, Seth Forshee
<seth.forshee@canonical.com> wrote:
> Apple laptops with hybrid graphics have a device named gmux that
> controls the muxing of the LVDS panel between the GPUs as well as screen
> brightness. This driver adds support for the gmux device. Only backlight
> control is supported initially.
>
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>

Works for me.

Tested-by: Grant Likely <grant.likely@secretlab.ca>

Now I just need to figure out how to get the desktop backlight widget
to use gmux_backlight instead of acpi_video0...

g.

> ---
>
> This contains some minor updates to the previous submission. Changes in
> v2:
>
>  - Removed unused max_brightness field from struct apple_gmux_data
>  - Removed unused gmux_write32() function
>  - Mask off unused bits when reading brightness
>  - Use gmux_get_brightness() when initializing backlight device
>   brightness field
>  - Fix some bad grammar in one of the comments.
>
>  drivers/platform/x86/Kconfig      |   10 ++
>  drivers/platform/x86/Makefile     |    1 +
>  drivers/platform/x86/apple-gmux.c |  228 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 239 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/platform/x86/apple-gmux.c
>
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index f995e6e..a2a9ede 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -779,4 +779,14 @@ config SAMSUNG_Q10
>          This driver provides support for backlight control on Samsung Q10
>          and related laptops, including Dell Latitude X200.
>
> +config APPLE_GMUX
> +       tristate "Apple Gmux Driver"
> +       depends on PNP
> +       select BACKLIGHT_CLASS_DEVICE
> +       ---help---
> +         This driver provides support for the gmux device found on many
> +         Apple laptops, which controls the display mux for the hybrid
> +         graphics as well as the backlight. Currently only backlight
> +         control is supported by the driver.
> +
>  endif # X86_PLATFORM_DEVICES
> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
> index 293a320..a49c891 100644
> --- a/drivers/platform/x86/Makefile
> +++ b/drivers/platform/x86/Makefile
> @@ -45,3 +45,4 @@ obj-$(CONFIG_MXM_WMI)         += mxm-wmi.o
>  obj-$(CONFIG_INTEL_MID_POWER_BUTTON)   += intel_mid_powerbtn.o
>  obj-$(CONFIG_INTEL_OAKTRAIL)   += intel_oaktrail.o
>  obj-$(CONFIG_SAMSUNG_Q10)      += samsung-q10.o
> +obj-$(CONFIG_APPLE_GMUX)       += apple-gmux.o
> diff --git a/drivers/platform/x86/apple-gmux.c b/drivers/platform/x86/apple-gmux.c
> new file mode 100644
> index 0000000..efc1fa9
> --- /dev/null
> +++ b/drivers/platform/x86/apple-gmux.c
> @@ -0,0 +1,228 @@
> +/*
> + *  Gmux driver for Apple laptops
> + *
> + *  Copyright (C) Canonical Ltd. <seth.forshee@canonical.com>
> + *
> + *  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.
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/backlight.h>
> +#include <linux/acpi.h>
> +#include <linux/pnp.h>
> +
> +struct apple_gmux_data {
> +       unsigned long iostart;
> +       unsigned long iolen;
> +
> +       struct backlight_device *bdev;
> +};
> +
> +/*
> + * gmux port offsets. Many of these are not yet used, but may be in the
> + * future, and it's useful to have them documented here anyhow.
> + */
> +#define GMUX_PORT_VERSION_MAJOR                0x04
> +#define GMUX_PORT_VERSION_MINOR                0x05
> +#define GMUX_PORT_VERSION_RELEASE      0x06
> +#define GMUX_PORT_SWITCH_DISPLAY       0x10
> +#define GMUX_PORT_SWITCH_GET_DISPLAY   0x11
> +#define GMUX_PORT_INTERRUPT_ENABLE     0x14
> +#define GMUX_PORT_INTERRUPT_STATUS     0x16
> +#define GMUX_PORT_SWITCH_DDC           0x28
> +#define GMUX_PORT_SWITCH_EXTERNAL      0x40
> +#define GMUX_PORT_SWITCH_GET_EXTERNAL  0x41
> +#define GMUX_PORT_DISCRETE_POWER       0x50
> +#define GMUX_PORT_MAX_BRIGHTNESS       0x70
> +#define GMUX_PORT_BRIGHTNESS           0x74
> +
> +#define GMUX_MIN_IO_LEN                        (GMUX_PORT_BRIGHTNESS + 4)
> +
> +#define GMUX_INTERRUPT_ENABLE          0xff
> +#define GMUX_INTERRUPT_DISABLE         0x00
> +
> +#define GMUX_INTERRUPT_STATUS_ACTIVE   0
> +#define GMUX_INTERRUPT_STATUS_DISPLAY  (1 << 0)
> +#define GMUX_INTERRUPT_STATUS_POWER    (1 << 2)
> +#define GMUX_INTERRUPT_STATUS_HOTPLUG  (1 << 3)
> +
> +#define GMUX_BRIGHTNESS_MASK           0x00ffffff
> +#define GMUX_MAX_BRIGHTNESS            GMUX_BRIGHTNESS_MASK
> +
> +static inline u8 gmux_read8(struct apple_gmux_data *gmux_data, int port)
> +{
> +       return inb(gmux_data->iostart + port);
> +}
> +
> +static inline void gmux_write8(struct apple_gmux_data *gmux_data, int port,
> +                              u8 val)
> +{
> +       outb(val, gmux_data->iostart + port);
> +}
> +
> +static inline u32 gmux_read32(struct apple_gmux_data *gmux_data, int port)
> +{
> +       return inl(gmux_data->iostart + port);
> +}
> +
> +static int gmux_get_brightness(struct backlight_device *bd)
> +{
> +       struct apple_gmux_data *gmux_data = bl_get_data(bd);
> +       return gmux_read32(gmux_data, GMUX_PORT_BRIGHTNESS) &
> +              GMUX_BRIGHTNESS_MASK;
> +}
> +
> +static int gmux_update_status(struct backlight_device *bd)
> +{
> +       struct apple_gmux_data *gmux_data = bl_get_data(bd);
> +       u32 brightness = bd->props.brightness;
> +
> +       /*
> +        * Older gmux versions require writing out lower bytes first then
> +        * setting the upper byte to 0 to flush the values. Newer versions
> +        * accept a single u32 write, but the old method also works, so we
> +        * just use the old method for all gmux versions.
> +        */
> +       gmux_write8(gmux_data, GMUX_PORT_BRIGHTNESS, brightness);
> +       gmux_write8(gmux_data, GMUX_PORT_BRIGHTNESS + 1, brightness >> 8);
> +       gmux_write8(gmux_data, GMUX_PORT_BRIGHTNESS + 2, brightness >> 16);
> +       gmux_write8(gmux_data, GMUX_PORT_BRIGHTNESS + 3, 0);
> +
> +       return 0;
> +}
> +
> +static const struct backlight_ops gmux_bl_ops = {
> +       .get_brightness = gmux_get_brightness,
> +       .update_status = gmux_update_status,
> +};
> +
> +static int __devinit gmux_probe(struct pnp_dev *pnp,
> +                               const struct pnp_device_id *id)
> +{
> +       struct apple_gmux_data *gmux_data;
> +       struct resource *res;
> +       struct backlight_properties props;
> +       struct backlight_device *bdev;
> +       u8 ver_major, ver_minor, ver_release;
> +       int ret = -ENXIO;
> +
> +       gmux_data = kzalloc(sizeof(*gmux_data), GFP_KERNEL);
> +       if (!gmux_data)
> +               return -ENOMEM;
> +       pnp_set_drvdata(pnp, gmux_data);
> +
> +       res = pnp_get_resource(pnp, IORESOURCE_IO, 0);
> +       if (!res) {
> +               pr_err("Failed to find gmux I/O resource\n");
> +               goto err_free;
> +       }
> +
> +       gmux_data->iostart = res->start;
> +       gmux_data->iolen = res->end - res->start;
> +
> +       if (gmux_data->iolen < GMUX_MIN_IO_LEN) {
> +               pr_err("gmux I/O region too small (%lu < %u)\n",
> +                      gmux_data->iolen, GMUX_MIN_IO_LEN);
> +               goto err_free;
> +       }
> +
> +       if (!request_region(gmux_data->iostart, gmux_data->iolen,
> +                           "Apple gmux")) {
> +               pr_err("gmux I/O already in use\n");
> +               goto err_free;
> +       }
> +
> +       /*
> +        * On some machines the gmux is in ACPI even thought the machine
> +        * doesn't really have a gmux. Check for invalid version information
> +        * to detect this.
> +        */
> +       ver_major = gmux_read8(gmux_data, GMUX_PORT_VERSION_MAJOR);
> +       ver_minor = gmux_read8(gmux_data, GMUX_PORT_VERSION_MINOR);
> +       ver_release = gmux_read8(gmux_data, GMUX_PORT_VERSION_RELEASE);
> +       if (ver_major == 0xff && ver_minor == 0xff && ver_release == 0xff) {
> +               pr_info("gmux device not present\n");
> +               ret = -ENODEV;
> +               goto err_release;
> +       }
> +
> +       pr_info("Found gmux version %d.%d.%d\n", ver_major, ver_minor,
> +               ver_release);
> +
> +       memset(&props, 0, sizeof(props));
> +       props.type = BACKLIGHT_PLATFORM;
> +       props.max_brightness = gmux_read32(gmux_data, GMUX_PORT_MAX_BRIGHTNESS);
> +
> +       /*
> +        * Currently it's assumed that the maximum brightness is less than
> +        * 2^24 for compatibility with old gmux versions. Cap the max
> +        * brightness at this value, but print a warning if the hardware
> +        * reports something higher so that it can be fixed.
> +        */
> +       if (WARN_ON(props.max_brightness > GMUX_MAX_BRIGHTNESS))
> +               props.max_brightness = GMUX_MAX_BRIGHTNESS;
> +
> +       bdev = backlight_device_register("gmux_backlight", &pnp->dev,
> +                                        gmux_data, &gmux_bl_ops, &props);
> +       if (IS_ERR(bdev)) {
> +               ret = PTR_ERR(bdev);
> +               goto err_release;
> +       }
> +
> +       gmux_data->bdev = bdev;
> +       bdev->props.brightness = gmux_get_brightness(bdev);
> +       backlight_update_status(bdev);
> +
> +       return 0;
> +
> +err_release:
> +       release_region(gmux_data->iostart, gmux_data->iolen);
> +err_free:
> +       kfree(gmux_data);
> +       return ret;
> +}
> +
> +static void __devexit gmux_remove(struct pnp_dev *pnp)
> +{
> +       struct apple_gmux_data *gmux_data = pnp_get_drvdata(pnp);
> +
> +       backlight_device_unregister(gmux_data->bdev);
> +       release_region(gmux_data->iostart, gmux_data->iolen);
> +       kfree(gmux_data);
> +}
> +
> +static const struct pnp_device_id gmux_device_ids[] = {
> +       {"APP000B", 0},
> +       {"", 0}
> +};
> +
> +static struct pnp_driver gmux_pnp_driver = {
> +       .name           = "apple-gmux",
> +       .probe          = gmux_probe,
> +       .remove         = __devexit_p(gmux_remove),
> +       .id_table       = gmux_device_ids,
> +};
> +
> +static int __init apple_gmux_init(void)
> +{
> +       return pnp_register_driver(&gmux_pnp_driver);
> +}
> +
> +static void __exit apple_gmux_exit(void)
> +{
> +       pnp_unregister_driver(&gmux_pnp_driver);
> +}
> +
> +module_init(apple_gmux_init);
> +module_exit(apple_gmux_exit);
> +
> +MODULE_AUTHOR("Seth Forshee <seth.forshee@canonical.com>");
> +MODULE_DESCRIPTION("Apple Gmux Driver");
> +MODULE_LICENSE("GPL");
> +MODULE_DEVICE_TABLE(pnp, gmux_device_ids);
> --
> 1.7.9
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/



-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: [PATCH v2] platform/x86: Add driver for Apple gmux device

[Seth Forshee]

On Wed, Feb 29, 2012 at 11:46:39AM -0600, Grant Likely wrote:
> On Wed, Feb 22, 2012 at 8:37 AM, Seth Forshee
> <seth.forshee@canonical.com> wrote:
> > Apple laptops with hybrid graphics have a device named gmux that
> > controls the muxing of the LVDS panel between the GPUs as well as screen
> > brightness. This driver adds support for the gmux device. Only backlight
> > control is supported initially.
> >
> > Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> 
> Works for me.
> 
> Tested-by: Grant Likely <grant.likely@secretlab.ca>
> 
> Now I just need to figure out how to get the desktop backlight widget
> to use gmux_backlight instead of acpi_video0...

The easy way is to pass acpi_backlight=vendor to the kernel, then you
won't have acpi_vidoe0.

Seth

Re: [PATCH v2] platform/x86: Add driver for Apple gmux device

[Grant Likely]

On Wed, Feb 29, 2012 at 11:53 AM, Seth Forshee
<seth.forshee@canonical.com> wrote:
> On Wed, Feb 29, 2012 at 11:46:39AM -0600, Grant Likely wrote:
>> On Wed, Feb 22, 2012 at 8:37 AM, Seth Forshee
>> <seth.forshee@canonical.com> wrote:
>> > Apple laptops with hybrid graphics have a device named gmux that
>> > controls the muxing of the LVDS panel between the GPUs as well as screen
>> > brightness. This driver adds support for the gmux device. Only backlight
>> > control is supported initially.
>> >
>> > Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
>>
>> Works for me.
>>
>> Tested-by: Grant Likely <grant.likely@secretlab.ca>
>>
>> Now I just need to figure out how to get the desktop backlight widget
>> to use gmux_backlight instead of acpi_video0...
>
> The easy way is to pass acpi_backlight=vendor to the kernel, then you
> won't have acpi_vidoe0.

That did it, thanks.  I'm assume something is in the works to set it
up automatically?

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: [PATCH v2] platform/x86: Add driver for Apple gmux device

[Seth Forshee]

On Wed, Feb 29, 2012 at 12:43:23PM -0600, Grant Likely wrote:
> On Wed, Feb 29, 2012 at 11:53 AM, Seth Forshee
> <seth.forshee@canonical.com> wrote:
> > On Wed, Feb 29, 2012 at 11:46:39AM -0600, Grant Likely wrote:
> >> On Wed, Feb 22, 2012 at 8:37 AM, Seth Forshee
> >> <seth.forshee@canonical.com> wrote:
> >> > Apple laptops with hybrid graphics have a device named gmux that
> >> > controls the muxing of the LVDS panel between the GPUs as well as screen
> >> > brightness. This driver adds support for the gmux device. Only backlight
> >> > control is supported initially.
> >> >
> >> > Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> >>
> >> Works for me.
> >>
> >> Tested-by: Grant Likely <grant.likely@secretlab.ca>
> >>
> >> Now I just need to figure out how to get the desktop backlight widget
> >> to use gmux_backlight instead of acpi_video0...
> >
> > The easy way is to pass acpi_backlight=vendor to the kernel, then you
> > won't have acpi_vidoe0.
> 
> That did it, thanks.  I'm assume something is in the works to set it
> up automatically?

Not that I'm aware of. A number machines have this problem, that the
standard ACPI backlight interfaces are implemented but don't work. This
generally isn't detectable in software; with the Apples at least
everything looks like it's working except that the brightness doesn't
change (but not all Apple laptops are affected, so qurking based on
manufacturer wouldn't work). All we're left with is DMI quirking, which
isn't practical. Maybe we could add something so a platform driver can
tell acpi_video that it knows the ACPI backlight doesn't work, but I
think on some platforms that still is going to be based off of DMI
information.

What about userspace? I don't know about other desktops, but in the past
gnome-settings-daemon had a list to determine which backlight to prefer,
so it would have been possible to insert gmux_backlight in the list
above acpi_video0. It seems to have been changed now to use the
backlight type, preferring a firmware backlight to a platform one, which
would prefer acpi_video0 to gmux_backlight. I don't know know what the
solution is under this paradigm.

Seth

Re: [PATCH v2] platform/x86: Add driver for Apple gmux device

[Grant Likely]

On Wed, Feb 29, 2012 at 1:50 PM, Seth Forshee
<seth.forshee@canonical.com> wrote:
> On Wed, Feb 29, 2012 at 12:43:23PM -0600, Grant Likely wrote:
>> On Wed, Feb 29, 2012 at 11:53 AM, Seth Forshee
>> <seth.forshee@canonical.com> wrote:
>> > On Wed, Feb 29, 2012 at 11:46:39AM -0600, Grant Likely wrote:
>> >> On Wed, Feb 22, 2012 at 8:37 AM, Seth Forshee
>> >> <seth.forshee@canonical.com> wrote:
>> >> > Apple laptops with hybrid graphics have a device named gmux that
>> >> > controls the muxing of the LVDS panel between the GPUs as well as screen
>> >> > brightness. This driver adds support for the gmux device. Only backlight
>> >> > control is supported initially.
>> >> >
>> >> > Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
>> >>
>> >> Works for me.
>> >>
>> >> Tested-by: Grant Likely <grant.likely@secretlab.ca>
>> >>
>> >> Now I just need to figure out how to get the desktop backlight widget
>> >> to use gmux_backlight instead of acpi_video0...
>> >
>> > The easy way is to pass acpi_backlight=vendor to the kernel, then you
>> > won't have acpi_vidoe0.
>>
>> That did it, thanks.  I'm assume something is in the works to set it
>> up automatically?
>
> Not that I'm aware of. A number machines have this problem, that the
> standard ACPI backlight interfaces are implemented but don't work. This
> generally isn't detectable in software; with the Apples at least
> everything looks like it's working except that the brightness doesn't
> change (but not all Apple laptops are affected, so qurking based on
> manufacturer wouldn't work). All we're left with is DMI quirking, which
> isn't practical. Maybe we could add something so a platform driver can
> tell acpi_video that it knows the ACPI backlight doesn't work, but I
> think on some platforms that still is going to be based off of DMI
> information.

blacklisting based on specific product name (ie. MacBookPro8,*) or
machine model is probably the best.  It wouldn't be the first
blacklist in the linux kernel.

g.

Re: [PATCH v2] platform/x86: Add driver for Apple gmux device

[Seth Forshee]

On Wed, Feb 29, 2012 at 03:23:20PM -0600, Grant Likely wrote:
> On Wed, Feb 29, 2012 at 1:50 PM, Seth Forshee
> <seth.forshee@canonical.com> wrote:
> > On Wed, Feb 29, 2012 at 12:43:23PM -0600, Grant Likely wrote:
> >> On Wed, Feb 29, 2012 at 11:53 AM, Seth Forshee
> >> <seth.forshee@canonical.com> wrote:
> >> > On Wed, Feb 29, 2012 at 11:46:39AM -0600, Grant Likely wrote:
> >> >> On Wed, Feb 22, 2012 at 8:37 AM, Seth Forshee
> >> >> <seth.forshee@canonical.com> wrote:
> >> >> > Apple laptops with hybrid graphics have a device named gmux that
> >> >> > controls the muxing of the LVDS panel between the GPUs as well as screen
> >> >> > brightness. This driver adds support for the gmux device. Only backlight
> >> >> > control is supported initially.
> >> >> >
> >> >> > Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> >> >>
> >> >> Works for me.
> >> >>
> >> >> Tested-by: Grant Likely <grant.likely@secretlab.ca>
> >> >>
> >> >> Now I just need to figure out how to get the desktop backlight widget
> >> >> to use gmux_backlight instead of acpi_video0...
> >> >
> >> > The easy way is to pass acpi_backlight=vendor to the kernel, then you
> >> > won't have acpi_vidoe0.
> >>
> >> That did it, thanks.  I'm assume something is in the works to set it
> >> up automatically?
> >
> > Not that I'm aware of. A number machines have this problem, that the
> > standard ACPI backlight interfaces are implemented but don't work. This
> > generally isn't detectable in software; with the Apples at least
> > everything looks like it's working except that the brightness doesn't
> > change (but not all Apple laptops are affected, so qurking based on
> > manufacturer wouldn't work). All we're left with is DMI quirking, which
> > isn't practical. Maybe we could add something so a platform driver can
> > tell acpi_video that it knows the ACPI backlight doesn't work, but I
> > think on some platforms that still is going to be based off of DMI
> > information.
> 
> blacklisting based on specific product name (ie. MacBookPro8,*) or
> machine model is probably the best.  It wouldn't be the first
> blacklist in the linux kernel.

I think the blacklist would have to be against specific product names.
For example, the MacBook Pro 8,1 has a working acpi_video backlight and
no gmux_backlight, the 8,2 has both but only gmux_backlight works, and I
suspect the 8,3 is the same as the 8,2. We'd probably end up with an
entry in the blacklist for every single model whose acpi_video backlight
doesn't work, adding entries for each new generation of MacBooks.

And if we start blacklisting Macs we'd have start doing it for other
machines too, I guess. From what I've seen, open-ended blacklists like
this get nacked pretty consistently nowadays.

Seth

Re: [PATCH v2] platform/x86: Add driver for Apple gmux device

[Grant Likely]

On Wed, Feb 29, 2012 at 4:08 PM, Seth Forshee
<seth.forshee@canonical.com> wrote:
> On Wed, Feb 29, 2012 at 03:23:20PM -0600, Grant Likely wrote:
>> On Wed, Feb 29, 2012 at 1:50 PM, Seth Forshee
>> <seth.forshee@canonical.com> wrote:
>> > On Wed, Feb 29, 2012 at 12:43:23PM -0600, Grant Likely wrote:
>> >> On Wed, Feb 29, 2012 at 11:53 AM, Seth Forshee
>> >> <seth.forshee@canonical.com> wrote:
>> >> > On Wed, Feb 29, 2012 at 11:46:39AM -0600, Grant Likely wrote:
>> >> >> On Wed, Feb 22, 2012 at 8:37 AM, Seth Forshee
>> >> >> <seth.forshee@canonical.com> wrote:
>> >> >> > Apple laptops with hybrid graphics have a device named gmux that
>> >> >> > controls the muxing of the LVDS panel between the GPUs as well as screen
>> >> >> > brightness. This driver adds support for the gmux device. Only backlight
>> >> >> > control is supported initially.
>> >> >> >
>> >> >> > Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
>> >> >>
>> >> >> Works for me.
>> >> >>
>> >> >> Tested-by: Grant Likely <grant.likely@secretlab.ca>
>> >> >>
>> >> >> Now I just need to figure out how to get the desktop backlight widget
>> >> >> to use gmux_backlight instead of acpi_video0...
>> >> >
>> >> > The easy way is to pass acpi_backlight=vendor to the kernel, then you
>> >> > won't have acpi_vidoe0.
>> >>
>> >> That did it, thanks.  I'm assume something is in the works to set it
>> >> up automatically?
>> >
>> > Not that I'm aware of. A number machines have this problem, that the
>> > standard ACPI backlight interfaces are implemented but don't work. This
>> > generally isn't detectable in software; with the Apples at least
>> > everything looks like it's working except that the brightness doesn't
>> > change (but not all Apple laptops are affected, so qurking based on
>> > manufacturer wouldn't work). All we're left with is DMI quirking, which
>> > isn't practical. Maybe we could add something so a platform driver can
>> > tell acpi_video that it knows the ACPI backlight doesn't work, but I
>> > think on some platforms that still is going to be based off of DMI
>> > information.
>>
>> blacklisting based on specific product name (ie. MacBookPro8,*) or
>> machine model is probably the best.  It wouldn't be the first
>> blacklist in the linux kernel.
>
> I think the blacklist would have to be against specific product names.
> For example, the MacBook Pro 8,1 has a working acpi_video backlight and
> no gmux_backlight, the 8,2 has both but only gmux_backlight works, and I
> suspect the 8,3 is the same as the 8,2.

I have the 8,3, and my testing confirms that.

> We'd probably end up with an
> entry in the blacklist for every single model whose acpi_video backlight
> doesn't work, adding entries for each new generation of MacBooks.
>
> And if we start blacklisting Macs we'd have start doing it for other
> machines too, I guess. From what I've seen, open-ended blacklists like
> this get nacked pretty consistently nowadays.

An alternative would be to blacklist or disable acpi0_backlight when
the apple-gmux driver loads.  I don't know how acceptable that is, but
I also don't have much sympathy for nacking blacklists if there isn't
a viable alternative.

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: [PATCH v2] platform/x86: Add driver for Apple gmux device

[Seth Forshee]

On Wed, Feb 29, 2012 at 04:32:28PM -0600, Grant Likely wrote:
> On Wed, Feb 29, 2012 at 4:08 PM, Seth Forshee
> <seth.forshee@canonical.com> wrote:
> > On Wed, Feb 29, 2012 at 03:23:20PM -0600, Grant Likely wrote:
> >> On Wed, Feb 29, 2012 at 1:50 PM, Seth Forshee
> >> <seth.forshee@canonical.com> wrote:
> >> > On Wed, Feb 29, 2012 at 12:43:23PM -0600, Grant Likely wrote:
> >> >> On Wed, Feb 29, 2012 at 11:53 AM, Seth Forshee
> >> >> <seth.forshee@canonical.com> wrote:
> >> >> > On Wed, Feb 29, 2012 at 11:46:39AM -0600, Grant Likely wrote:
> >> >> >> On Wed, Feb 22, 2012 at 8:37 AM, Seth Forshee
> >> >> >> <seth.forshee@canonical.com> wrote:
> >> >> >> > Apple laptops with hybrid graphics have a device named gmux that
> >> >> >> > controls the muxing of the LVDS panel between the GPUs as well as screen
> >> >> >> > brightness. This driver adds support for the gmux device. Only backlight
> >> >> >> > control is supported initially.
> >> >> >> >
> >> >> >> > Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> >> >> >>
> >> >> >> Works for me.
> >> >> >>
> >> >> >> Tested-by: Grant Likely <grant.likely@secretlab.ca>
> >> >> >>
> >> >> >> Now I just need to figure out how to get the desktop backlight widget
> >> >> >> to use gmux_backlight instead of acpi_video0...
> >> >> >
> >> >> > The easy way is to pass acpi_backlight=vendor to the kernel, then you
> >> >> > won't have acpi_vidoe0.
> >> >>
> >> >> That did it, thanks.  I'm assume something is in the works to set it
> >> >> up automatically?
> >> >
> >> > Not that I'm aware of. A number machines have this problem, that the
> >> > standard ACPI backlight interfaces are implemented but don't work. This
> >> > generally isn't detectable in software; with the Apples at least
> >> > everything looks like it's working except that the brightness doesn't
> >> > change (but not all Apple laptops are affected, so qurking based on
> >> > manufacturer wouldn't work). All we're left with is DMI quirking, which
> >> > isn't practical. Maybe we could add something so a platform driver can
> >> > tell acpi_video that it knows the ACPI backlight doesn't work, but I
> >> > think on some platforms that still is going to be based off of DMI
> >> > information.
> >>
> >> blacklisting based on specific product name (ie. MacBookPro8,*) or
> >> machine model is probably the best.  It wouldn't be the first
> >> blacklist in the linux kernel.
> >
> > I think the blacklist would have to be against specific product names.
> > For example, the MacBook Pro 8,1 has a working acpi_video backlight and
> > no gmux_backlight, the 8,2 has both but only gmux_backlight works, and I
> > suspect the 8,3 is the same as the 8,2.
> 
> I have the 8,3, and my testing confirms that.
> 
> > We'd probably end up with an
> > entry in the blacklist for every single model whose acpi_video backlight
> > doesn't work, adding entries for each new generation of MacBooks.
> >
> > And if we start blacklisting Macs we'd have start doing it for other
> > machines too, I guess. From what I've seen, open-ended blacklists like
> > this get nacked pretty consistently nowadays.
> 
> An alternative would be to blacklist or disable acpi0_backlight when
> the apple-gmux driver loads.  I don't know how acceptable that is, but
> I also don't have much sympathy for nacking blacklists if there isn't
> a viable alternative.

Yes, that's one idea I was thinking about. For all the machines I've
been able to get tested, if the gmux is present then it can control the
backlight and acpi_video cannot, so that approach is reasonable for
Macs.

There are quite a few machines in this situation though, and whatever
solution is arrived at should be flexible enough to work beyond just
Macs. I'll try to find some time soon to explore this further and see if
I can come up with something.

Seth

Re: [PATCH v2] platform/x86: Add driver for Apple gmux device

[Corentin Chary]

On Wed, Feb 29, 2012 at 11:56 PM, Seth Forshee
<seth.forshee@canonical.com> wrote:
> On Wed, Feb 29, 2012 at 04:32:28PM -0600, Grant Likely wrote:
>> On Wed, Feb 29, 2012 at 4:08 PM, Seth Forshee
>> <seth.forshee@canonical.com> wrote:
>> > On Wed, Feb 29, 2012 at 03:23:20PM -0600, Grant Likely wrote:
>> >> On Wed, Feb 29, 2012 at 1:50 PM, Seth Forshee
>> >> <seth.forshee@canonical.com> wrote:
>> >> > On Wed, Feb 29, 2012 at 12:43:23PM -0600, Grant Likely wrote:
>> >> >> On Wed, Feb 29, 2012 at 11:53 AM, Seth Forshee
>> >> >> <seth.forshee@canonical.com> wrote:
>> >> >> > On Wed, Feb 29, 2012 at 11:46:39AM -0600, Grant Likely wrote:
>> >> >> >> On Wed, Feb 22, 2012 at 8:37 AM, Seth Forshee
>> >> >> >> <seth.forshee@canonical.com> wrote:
>> >> >> >> > Apple laptops with hybrid graphics have a device named gmux that
>> >> >> >> > controls the muxing of the LVDS panel between the GPUs as well as screen
>> >> >> >> > brightness. This driver adds support for the gmux device. Only backlight
>> >> >> >> > control is supported initially.
>> >> >> >> >
>> >> >> >> > Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
>> >> >> >>
>> >> >> >> Works for me.
>> >> >> >>
>> >> >> >> Tested-by: Grant Likely <grant.likely@secretlab.ca>
>> >> >> >>
>> >> >> >> Now I just need to figure out how to get the desktop backlight widget
>> >> >> >> to use gmux_backlight instead of acpi_video0...
>> >> >> >
>> >> >> > The easy way is to pass acpi_backlight=vendor to the kernel, then you
>> >> >> > won't have acpi_vidoe0.
>> >> >>
>> >> >> That did it, thanks.  I'm assume something is in the works to set it
>> >> >> up automatically?
>> >> >
>> >> > Not that I'm aware of. A number machines have this problem, that the
>> >> > standard ACPI backlight interfaces are implemented but don't work. This
>> >> > generally isn't detectable in software; with the Apples at least
>> >> > everything looks like it's working except that the brightness doesn't
>> >> > change (but not all Apple laptops are affected, so qurking based on
>> >> > manufacturer wouldn't work). All we're left with is DMI quirking, which
>> >> > isn't practical. Maybe we could add something so a platform driver can
>> >> > tell acpi_video that it knows the ACPI backlight doesn't work, but I
>> >> > think on some platforms that still is going to be based off of DMI
>> >> > information.
>> >>
>> >> blacklisting based on specific product name (ie. MacBookPro8,*) or
>> >> machine model is probably the best.  It wouldn't be the first
>> >> blacklist in the linux kernel.
>> >
>> > I think the blacklist would have to be against specific product names.
>> > For example, the MacBook Pro 8,1 has a working acpi_video backlight and
>> > no gmux_backlight, the 8,2 has both but only gmux_backlight works, and I
>> > suspect the 8,3 is the same as the 8,2.
>>
>> I have the 8,3, and my testing confirms that.
>>
>> > We'd probably end up with an
>> > entry in the blacklist for every single model whose acpi_video backlight
>> > doesn't work, adding entries for each new generation of MacBooks.
>> >
>> > And if we start blacklisting Macs we'd have start doing it for other
>> > machines too, I guess. From what I've seen, open-ended blacklists like
>> > this get nacked pretty consistently nowadays.
>>
>> An alternative would be to blacklist or disable acpi0_backlight when
>> the apple-gmux driver loads.  I don't know how acceptable that is, but
>> I also don't have much sympathy for nacking blacklists if there isn't
>> a viable alternative.
>
> Yes, that's one idea I was thinking about. For all the machines I've
> been able to get tested, if the gmux is present then it can control the
> backlight and acpi_video cannot, so that approach is reasonable for
> Macs.
>
> There are quite a few machines in this situation though, and whatever
> solution is arrived at should be flexible enough to work beyond just
> Macs. I'll try to find some time soon to explore this further and see if
> I can come up with something.

Old Samsung laptops have the same issue, and I ended up patching
drivers/acpi/video_detect.c (check "[PATCH] ACPI / Video: blacklist
some samsung laptops").
Doing it in the vendor module is complicated, since, it will be loaded
after the acpi video module most of the time, and that means addding
acpi_backlight, then removing it, which will probably confuse
usespace.
Patching drivers/acpi/video_detect.c seems safer.

-- 
Corentin Chary
http://xf.iksaif.net

Re: [PATCH v2] platform/x86: Add driver for Apple gmux device

[Seth Forshee]

On Thu, Mar 01, 2012 at 10:19:58AM +0100, Corentin Chary wrote:
> On Wed, Feb 29, 2012 at 11:56 PM, Seth Forshee
> <seth.forshee@canonical.com> wrote:
> > On Wed, Feb 29, 2012 at 04:32:28PM -0600, Grant Likely wrote:
> >> On Wed, Feb 29, 2012 at 4:08 PM, Seth Forshee
> >> <seth.forshee@canonical.com> wrote:
> >> > On Wed, Feb 29, 2012 at 03:23:20PM -0600, Grant Likely wrote:
> >> >> On Wed, Feb 29, 2012 at 1:50 PM, Seth Forshee
> >> >> <seth.forshee@canonical.com> wrote:
> >> >> > On Wed, Feb 29, 2012 at 12:43:23PM -0600, Grant Likely wrote:
> >> >> >> On Wed, Feb 29, 2012 at 11:53 AM, Seth Forshee
> >> >> >> <seth.forshee@canonical.com> wrote:
> >> >> >> > On Wed, Feb 29, 2012 at 11:46:39AM -0600, Grant Likely wrote:
> >> >> >> >> On Wed, Feb 22, 2012 at 8:37 AM, Seth Forshee
> >> >> >> >> <seth.forshee@canonical.com> wrote:
> >> >> >> >> > Apple laptops with hybrid graphics have a device named gmux that
> >> >> >> >> > controls the muxing of the LVDS panel between the GPUs as well as screen
> >> >> >> >> > brightness. This driver adds support for the gmux device. Only backlight
> >> >> >> >> > control is supported initially.
> >> >> >> >> >
> >> >> >> >> > Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> >> >> >> >>
> >> >> >> >> Works for me.
> >> >> >> >>
> >> >> >> >> Tested-by: Grant Likely <grant.likely@secretlab.ca>
> >> >> >> >>
> >> >> >> >> Now I just need to figure out how to get the desktop backlight widget
> >> >> >> >> to use gmux_backlight instead of acpi_video0...
> >> >> >> >
> >> >> >> > The easy way is to pass acpi_backlight=vendor to the kernel, then you
> >> >> >> > won't have acpi_vidoe0.
> >> >> >>
> >> >> >> That did it, thanks.  I'm assume something is in the works to set it
> >> >> >> up automatically?
> >> >> >
> >> >> > Not that I'm aware of. A number machines have this problem, that the
> >> >> > standard ACPI backlight interfaces are implemented but don't work. This
> >> >> > generally isn't detectable in software; with the Apples at least
> >> >> > everything looks like it's working except that the brightness doesn't
> >> >> > change (but not all Apple laptops are affected, so qurking based on
> >> >> > manufacturer wouldn't work). All we're left with is DMI quirking, which
> >> >> > isn't practical. Maybe we could add something so a platform driver can
> >> >> > tell acpi_video that it knows the ACPI backlight doesn't work, but I
> >> >> > think on some platforms that still is going to be based off of DMI
> >> >> > information.
> >> >>
> >> >> blacklisting based on specific product name (ie. MacBookPro8,*) or
> >> >> machine model is probably the best.  It wouldn't be the first
> >> >> blacklist in the linux kernel.
> >> >
> >> > I think the blacklist would have to be against specific product names.
> >> > For example, the MacBook Pro 8,1 has a working acpi_video backlight and
> >> > no gmux_backlight, the 8,2 has both but only gmux_backlight works, and I
> >> > suspect the 8,3 is the same as the 8,2.
> >>
> >> I have the 8,3, and my testing confirms that.
> >>
> >> > We'd probably end up with an
> >> > entry in the blacklist for every single model whose acpi_video backlight
> >> > doesn't work, adding entries for each new generation of MacBooks.
> >> >
> >> > And if we start blacklisting Macs we'd have start doing it for other
> >> > machines too, I guess. From what I've seen, open-ended blacklists like
> >> > this get nacked pretty consistently nowadays.
> >>
> >> An alternative would be to blacklist or disable acpi0_backlight when
> >> the apple-gmux driver loads.  I don't know how acceptable that is, but
> >> I also don't have much sympathy for nacking blacklists if there isn't
> >> a viable alternative.
> >
> > Yes, that's one idea I was thinking about. For all the machines I've
> > been able to get tested, if the gmux is present then it can control the
> > backlight and acpi_video cannot, so that approach is reasonable for
> > Macs.
> >
> > There are quite a few machines in this situation though, and whatever
> > solution is arrived at should be flexible enough to work beyond just
> > Macs. I'll try to find some time soon to explore this further and see if
> > I can come up with something.
> 
> Old Samsung laptops have the same issue, and I ended up patching
> drivers/acpi/video_detect.c (check "[PATCH] ACPI / Video: blacklist
> some samsung laptops").
> Doing it in the vendor module is complicated, since, it will be loaded
> after the acpi video module most of the time, and that means addding
> acpi_backlight, then removing it, which will probably confuse
> usespace.
> Patching drivers/acpi/video_detect.c seems safer.

I certainly agree that it's the easiest solution, so maybe it is worth
trying first and seeing what the response is. But last I saw the patch
you're referring to hadn't been merged or even commented on yet. And I
can't help but note that you included as justification for the patch
that the acpi_video backlight works on newer Samsungs so the list
wouldn't grow ;)

Beyond Samsung and Apple I know that there are also Toshibas that suffer
from this, and judging by the list from older versions of g-s-d [1] I'd
say there may well be others.

Seth

[1] http://git.gnome.org/browse/gnome-settings-daemon/tree/plugins/power/gsd-backlight-helper.c?id=GNOME_SETTINGS_DAEMON_3_2_2#n50

Re: [PATCH v2] platform/x86: Add driver for Apple gmux device

[Corentin Chary]

On Thu, Mar 1, 2012 at 3:53 PM, Seth Forshee <seth.forshee@canonical.com> wrote:
> On Thu, Mar 01, 2012 at 10:19:58AM +0100, Corentin Chary wrote:
>> On Wed, Feb 29, 2012 at 11:56 PM, Seth Forshee
>> <seth.forshee@canonical.com> wrote:
>> > On Wed, Feb 29, 2012 at 04:32:28PM -0600, Grant Likely wrote:
>> >> On Wed, Feb 29, 2012 at 4:08 PM, Seth Forshee
>> >> <seth.forshee@canonical.com> wrote:
>> >> > On Wed, Feb 29, 2012 at 03:23:20PM -0600, Grant Likely wrote:
>> >> >> On Wed, Feb 29, 2012 at 1:50 PM, Seth Forshee
>> >> >> <seth.forshee@canonical.com> wrote:
>> >> >> > On Wed, Feb 29, 2012 at 12:43:23PM -0600, Grant Likely wrote:
>> >> >> >> On Wed, Feb 29, 2012 at 11:53 AM, Seth Forshee
>> >> >> >> <seth.forshee@canonical.com> wrote:
>> >> >> >> > On Wed, Feb 29, 2012 at 11:46:39AM -0600, Grant Likely wrote:
>> >> >> >> >> On Wed, Feb 22, 2012 at 8:37 AM, Seth Forshee
>> >> >> >> >> <seth.forshee@canonical.com> wrote:
>> >> >> >> >> > Apple laptops with hybrid graphics have a device named gmux that
>> >> >> >> >> > controls the muxing of the LVDS panel between the GPUs as well as screen
>> >> >> >> >> > brightness. This driver adds support for the gmux device. Only backlight
>> >> >> >> >> > control is supported initially.
>> >> >> >> >> >
>> >> >> >> >> > Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
>> >> >> >> >>
>> >> >> >> >> Works for me.
>> >> >> >> >>
>> >> >> >> >> Tested-by: Grant Likely <grant.likely@secretlab.ca>
>> >> >> >> >>
>> >> >> >> >> Now I just need to figure out how to get the desktop backlight widget
>> >> >> >> >> to use gmux_backlight instead of acpi_video0...
>> >> >> >> >
>> >> >> >> > The easy way is to pass acpi_backlight=vendor to the kernel, then you
>> >> >> >> > won't have acpi_vidoe0.
>> >> >> >>
>> >> >> >> That did it, thanks.  I'm assume something is in the works to set it
>> >> >> >> up automatically?
>> >> >> >
>> >> >> > Not that I'm aware of. A number machines have this problem, that the
>> >> >> > standard ACPI backlight interfaces are implemented but don't work. This
>> >> >> > generally isn't detectable in software; with the Apples at least
>> >> >> > everything looks like it's working except that the brightness doesn't
>> >> >> > change (but not all Apple laptops are affected, so qurking based on
>> >> >> > manufacturer wouldn't work). All we're left with is DMI quirking, which
>> >> >> > isn't practical. Maybe we could add something so a platform driver can
>> >> >> > tell acpi_video that it knows the ACPI backlight doesn't work, but I
>> >> >> > think on some platforms that still is going to be based off of DMI
>> >> >> > information.
>> >> >>
>> >> >> blacklisting based on specific product name (ie. MacBookPro8,*) or
>> >> >> machine model is probably the best.  It wouldn't be the first
>> >> >> blacklist in the linux kernel.
>> >> >
>> >> > I think the blacklist would have to be against specific product names.
>> >> > For example, the MacBook Pro 8,1 has a working acpi_video backlight and
>> >> > no gmux_backlight, the 8,2 has both but only gmux_backlight works, and I
>> >> > suspect the 8,3 is the same as the 8,2.
>> >>
>> >> I have the 8,3, and my testing confirms that.
>> >>
>> >> > We'd probably end up with an
>> >> > entry in the blacklist for every single model whose acpi_video backlight
>> >> > doesn't work, adding entries for each new generation of MacBooks.
>> >> >
>> >> > And if we start blacklisting Macs we'd have start doing it for other
>> >> > machines too, I guess. From what I've seen, open-ended blacklists like
>> >> > this get nacked pretty consistently nowadays.
>> >>
>> >> An alternative would be to blacklist or disable acpi0_backlight when
>> >> the apple-gmux driver loads.  I don't know how acceptable that is, but
>> >> I also don't have much sympathy for nacking blacklists if there isn't
>> >> a viable alternative.
>> >
>> > Yes, that's one idea I was thinking about. For all the machines I've
>> > been able to get tested, if the gmux is present then it can control the
>> > backlight and acpi_video cannot, so that approach is reasonable for
>> > Macs.
>> >
>> > There are quite a few machines in this situation though, and whatever
>> > solution is arrived at should be flexible enough to work beyond just
>> > Macs. I'll try to find some time soon to explore this further and see if
>> > I can come up with something.
>>
>> Old Samsung laptops have the same issue, and I ended up patching
>> drivers/acpi/video_detect.c (check "[PATCH] ACPI / Video: blacklist
>> some samsung laptops").
>> Doing it in the vendor module is complicated, since, it will be loaded
>> after the acpi video module most of the time, and that means addding
>> acpi_backlight, then removing it, which will probably confuse
>> usespace.
>> Patching drivers/acpi/video_detect.c seems safer.
>
> I certainly agree that it's the easiest solution, so maybe it is worth
> trying first and seeing what the response is. But last I saw the patch
> you're referring to hadn't been merged or even commented on yet.

I'm still waiting for comments, but Matthew seems to be very busy
currently, so I'll probably repost it directly to linux-acpi in some
days.
Ccing Len and linux-acpi.

> And I can't help but note that you included as justification for the patch
> that the acpi_video backlight works on newer Samsungs so the list
> wouldn't grow ;)
>
> Beyond Samsung and Apple I know that there are also Toshibas that suffer
> from this, and judging by the list from older versions of g-s-d [1] I'd
> say there may well be others.
>
> Seth
>
> [1] http://git.gnome.org/browse/gnome-settings-daemon/tree/plugins/power/gsd-backlight-helper.c?id=GNOME_SETTINGS_DAEMON_3_2_2#n50
>

Anyway, like said before, there are already ton of blacklist in the
kernel, so why not ?

-- 
Corentin Chary
http://xf.iksaif.net

Re: [PATCH v2] platform/x86: Add driver for Apple gmux device

[Seth Forshee]

On Wed, Feb 22, 2012 at 08:37:37AM -0600, Seth Forshee wrote:
> Apple laptops with hybrid graphics have a device named gmux that
> controls the muxing of the LVDS panel between the GPUs as well as screen
> brightness. This driver adds support for the gmux device. Only backlight
> control is supported initially.
> 
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>

Ping. Is there anything that would prevent getting this into 3.4?

Seth

Re: [PATCH v2] platform/x86: Add driver for Apple gmux device

[Josh Boyer]

On Mon, Mar 5, 2012 at 5:06 PM, Seth Forshee <seth.forshee@canonical.com> wrote:
> On Wed, Feb 22, 2012 at 08:37:37AM -0600, Seth Forshee wrote:
>> Apple laptops with hybrid graphics have a device named gmux that
>> controls the muxing of the LVDS panel between the GPUs as well as screen
>> brightness. This driver adds support for the gmux device. Only backlight
>> control is supported initially.
>>
>> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
>
> Ping. Is there anything that would prevent getting this into 3.4?

I sent a list of patches that seemed ready for 3.4 to Matthew last week.  This
was on the list and he seemed to agree.  I would guess he just either needs
a minion, a clone, or a time machine.  Or maybe all three.

josh

Re: [PATCH v2] platform/x86: Add driver for Apple gmux device

[Seth Forshee]

On Mon, Mar 05, 2012 at 05:10:50PM -0500, Josh Boyer wrote:
> On Mon, Mar 5, 2012 at 5:06 PM, Seth Forshee <seth.forshee@canonical.com> wrote:
> > On Wed, Feb 22, 2012 at 08:37:37AM -0600, Seth Forshee wrote:
> >> Apple laptops with hybrid graphics have a device named gmux that
> >> controls the muxing of the LVDS panel between the GPUs as well as screen
> >> brightness. This driver adds support for the gmux device. Only backlight
> >> control is supported initially.
> >>
> >> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> >
> > Ping. Is there anything that would prevent getting this into 3.4?
> 
> I sent a list of patches that seemed ready for 3.4 to Matthew last week.  This
> was on the list and he seemed to agree.  I would guess he just either needs
> a minion, a clone, or a time machine.  Or maybe all three.

Huh, guess I missed that, and even now I can't seem to find it. Any
chance you could point me to the list?

Thanks for the heads-up!

Seth

Re: [PATCH v2] platform/x86: Add driver for Apple gmux device

[Josh Boyer]

On Mon, Mar 5, 2012 at 5:37 PM, Seth Forshee <seth.forshee@canonical.com> wrote:
> On Mon, Mar 05, 2012 at 05:10:50PM -0500, Josh Boyer wrote:
>> On Mon, Mar 5, 2012 at 5:06 PM, Seth Forshee <seth.forshee@canonical.com> wrote:
>> > On Wed, Feb 22, 2012 at 08:37:37AM -0600, Seth Forshee wrote:
>> >> Apple laptops with hybrid graphics have a device named gmux that
>> >> controls the muxing of the LVDS panel between the GPUs as well as screen
>> >> brightness. This driver adds support for the gmux device. Only backlight
>> >> control is supported initially.
>> >>
>> >> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
>> >
>> > Ping. Is there anything that would prevent getting this into 3.4?
>>
>> I sent a list of patches that seemed ready for 3.4 to Matthew last week.  This
>> was on the list and he seemed to agree.  I would guess he just either needs
>> a minion, a clone, or a time machine.  Or maybe all three.
>
> Huh, guess I missed that, and even now I can't seem to find it. Any
> chance you could point me to the list?

Oh, sorry.  It was a direct mail to Matthew, not on a list.  My mistake.

Basically, it consisted of this:

"OK, so digging through the platform list, here are the patches that seem
to be bugfixes that should probably go into 3.3:

http://article.gmane.org/gmane.linux.acpi.devel/51594
http://article.gmane.org/gmane.linux.drivers.platform.x86.devel/2908
http://article.gmane.org/gmane.linux.drivers.platform.x86.devel/2943
(replaces 3aa4634c0f23593c5be7e819e02dd6ace671820a in your
linux-next/for_linus branches)


For 3.4, these seem ready (in addition to what you have in linux-next):
http://article.gmane.org/gmane.linux.drivers.platform.x86.devel/2968
http://article.gmane.org/gmane.linux.drivers.platform.x86.devel/2888
http://article.gmane.org/gmane.linux.drivers.platform.x86.devel/2934
http://thread.gmane.org/gmane.linux.drivers.platform.x86.devel/2929
"

I might spend some time today creating a fixed up git tree with that set of
changes/patches and see what comes of it.

josh

Re: [PATCH v2] platform/x86: Add driver for Apple gmux device

[Matthew Garrett]

apple_bl probably needs some matching work to disable itself if there's 
a gmux present? Otherwise, this looks good.

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

Re: [PATCH v2] platform/x86: Add driver for Apple gmux device

[Seth Forshee]

On Mon, Mar 12, 2012 at 02:21:26PM +0000, Matthew Garrett wrote:
> apple_bl probably needs some matching work to disable itself if there's 
> a gmux present? Otherwise, this looks good.

How do you suggest doing this? The tricky point is that the gmux object
can be present in ACPI when the gmux hardware isn't really present
(which is what the version check really does, verifies the hardware is
actually there). So apple_bl can't just look for the ACPI object; it
needs to either communicate with apple-gmux or duplicate some of its
logic.

We also have the problem of gmux_backlight versus acpi_video. On most
machines with a gmux the acpi_video backlight interface is present but
just doesn't work. This problem isn't just limited to Apples. I'm of the
opinion that we need a more generalized solution for arbitrating between
the backlight interfaces present on a given machine, but I haven't had a
chance to really think about what that would look like.

Seth

Re: [PATCH v2] platform/x86: Add driver for Apple gmux device

[Matthew Garrett]

On Mon, Mar 12, 2012 at 09:57:01AM -0500, Seth Forshee wrote:
> On Mon, Mar 12, 2012 at 02:21:26PM +0000, Matthew Garrett wrote:
> > apple_bl probably needs some matching work to disable itself if there's 
> > a gmux present? Otherwise, this looks good.
> 
> How do you suggest doing this? The tricky point is that the gmux object
> can be present in ACPI when the gmux hardware isn't really present
> (which is what the version check really does, verifies the hardware is
> actually there). So apple_bl can't just look for the ACPI object; it
> needs to either communicate with apple-gmux or duplicate some of its
> logic.

The alternative is for apple_bl to export its unregister function and 
have gmux call that - downside is obviously that that results in gmux 
depending on apple_bl. Maybe some sort of notification list in the 
backlight core?

> We also have the problem of gmux_backlight versus acpi_video. On most
> machines with a gmux the acpi_video backlight interface is present but
> just doesn't work. This problem isn't just limited to Apples. I'm of the
> opinion that we need a more generalized solution for arbitrating between
> the backlight interfaces present on a given machine, but I haven't had a
> chance to really think about what that would look like.

The ACPI code appears to be trapping into system management mode, so 
figuring out what it's meant to do is going to be awkward. I think 
having a hook in the ACPI video driver to deregister it in cases where 
we know it doesn't work is legitimate, but since it presumably works 
under Windows it'd be nice to figure out what's broken about it.

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

Re: [PATCH v2] platform/x86: Add driver for Apple gmux device

[Seth Forshee]

On Mon, Mar 12, 2012 at 03:07:17PM +0000, Matthew Garrett wrote:
> > We also have the problem of gmux_backlight versus acpi_video. On most
> > machines with a gmux the acpi_video backlight interface is present but
> > just doesn't work. This problem isn't just limited to Apples. I'm of the
> > opinion that we need a more generalized solution for arbitrating between
> > the backlight interfaces present on a given machine, but I haven't had a
> > chance to really think about what that would look like.
> 
> The ACPI code appears to be trapping into system management mode, so 
> figuring out what it's meant to do is going to be awkward. I think 
> having a hook in the ACPI video driver to deregister it in cases where 
> we know it doesn't work is legitimate, but since it presumably works 
> under Windows it'd be nice to figure out what's broken about it.

Why do you presume it works under Windows? Boot camp installs a pile of
drivers for Windows, one of those could be a driver for the backlight. I
don't know either way. But it's pretty clear that MacOS is using the
gmux for the backlight when it is present from the reverse engineering
work done by others.

Seth

Re: [PATCH v2] platform/x86: Add driver for Apple gmux device

[Matthew Garrett]

On Mon, Mar 12, 2012 at 10:18:52AM -0500, Seth Forshee wrote:

> Why do you presume it works under Windows? Boot camp installs a pile of
> drivers for Windows, one of those could be a driver for the backlight. I
> don't know either way. But it's pretty clear that MacOS is using the
> gmux for the backlight when it is present from the reverse engineering
> work done by others.

The only reason to provide it at all is for Windows - it's never going 
to be used under OS X.

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

Re: [PATCH v2] platform/x86: Add driver for Apple gmux device

[Seth Forshee]

On Mon, Mar 12, 2012 at 03:22:24PM +0000, Matthew Garrett wrote:
> On Mon, Mar 12, 2012 at 10:18:52AM -0500, Seth Forshee wrote:
> 
> > Why do you presume it works under Windows? Boot camp installs a pile of
> > drivers for Windows, one of those could be a driver for the backlight. I
> > don't know either way. But it's pretty clear that MacOS is using the
> > gmux for the backlight when it is present from the reverse engineering
> > work done by others.
> 
> The only reason to provide it at all is for Windows - it's never going 
> to be used under OS X.

I'm not so sure. The MacBook Pro 8,1 and 8,2 seem to use the same BIOS
implementation -- the version numbers and DSDTs are identical.  The 8,1
has only Intel graphics, no gmux, and a working acpi_video0 backlight
under Linux. The 8,2 has hybrid ATI/Intel, a gmux w/ working backlight
control, and acpi_video0 does not work.

So while I'm not sure, I can't rule out that the acpi_video backlight
might be provided specifically for the 8,1 and that OS X is using it.
Unfortunately I don't have an 8,1 that I can play with to try and work
it out.

Seth

Re: [PATCH v2] platform/x86: Add driver for Apple gmux device

[Seth Forshee]

On Mon, Mar 12, 2012 at 03:07:17PM +0000, Matthew Garrett wrote:
> The alternative is for apple_bl to export its unregister function and 
> have gmux call that - downside is obviously that that results in gmux 
> depending on apple_bl. Maybe some sort of notification list in the 
> backlight core?

I've been think about this, and I'm not sure a notification in the
backlight core makes sense; it seems likely to be limited in use to only
this one use case. Other ideas might be to have an interface to disable
specific backlight devices, or a way to score backlights which leaves
only the "best" device of a given type enabled. Either of these might
affect userspace ABI however.

For now at least if something like this is needed maybe it makes the
most sense to have apple_bl export a function. What about something like
the patch at the end of this message (compile tested only)?

> > We also have the problem of gmux_backlight versus acpi_video. On most
> > machines with a gmux the acpi_video backlight interface is present but
> > just doesn't work. This problem isn't just limited to Apples. I'm of the
> > opinion that we need a more generalized solution for arbitrating between
> > the backlight interfaces present on a given machine, but I haven't had a
> > chance to really think about what that would look like.
> 
> The ACPI code appears to be trapping into system management mode, so 
> figuring out what it's meant to do is going to be awkward. I think 
> having a hook in the ACPI video driver to deregister it in cases where 
> we know it doesn't work is legitimate, but since it presumably works 
> under Windows it'd be nice to figure out what's broken about it.

I've been experimenting with a MBP 8,2. This actually has 2 ACPI
backlights, one associated with the radeon GPU (acpi_video0) and the
other with the Intel (acpi_video1).

Turns out that acpi_video0 does work under Windows, and it also works
under Linux depending on how the OS is booted. Doing a boot camp style
boot gives us working acpi_video0 and apple_bl backlights. acpi_video1
doesn't show up in a legacy boot situation, as evaluating _BCM gives an
error.

Under EFI boot, both acpi_video backlights show up but neither works.
The readback verify in apple_bl's probe fails, so that one doesn't show
up. Only gmux_backlight actually works.

Using rEFIt is an oddball case. A BIOS-compatible boot gives non-working
acpi_video and apple_bl backlights. I guess the Apple bootloader must
enable some extra legacy support that rEFIt does not, but I haven't been
able to find any way for us to enable it after Linux has started.

Seth


diff --git a/drivers/video/backlight/apple_bl.c b/drivers/video/backlight/apple_bl.c
index be98d15..9b16d58 100644
--- a/drivers/video/backlight/apple_bl.c
+++ b/drivers/video/backlight/apple_bl.c
@@ -27,6 +27,19 @@
 
 static struct backlight_device *apple_backlight_device;
 
+/*
+ * apple_bl may be disabled by other modules. We need to track the state
+ * of the device to know how to respond to various events.
+ */
+enum {
+	APPLE_BL_STATE_UNUSED,
+	APPLE_BL_STATE_IN_USE,
+	APPLE_BL_STATE_DISABLED,
+};
+
+static int apple_bl_state = APPLE_BL_STATE_UNUSED;
+DEFINE_MUTEX(apple_bl_mutex);
+
 struct hw_data {
 	/* I/O resource to allocate. */
 	unsigned long iostart;
@@ -139,17 +152,46 @@ static const struct hw_data nvidia_chipset_data = {
 	.set_brightness = nvidia_chipset_set_brightness,
 };
 
+static void apple_bl_unregister(void)
+{
+	backlight_device_unregister(apple_backlight_device);
+	release_region(hw_data->iostart, hw_data->iolen);
+	hw_data = NULL;
+}
+
+void apple_bl_disable(void)
+{
+	mutex_lock(&apple_bl_mutex);
+	if (apple_bl_state == APPLE_BL_STATE_IN_USE)
+		apple_bl_unregister();
+	apple_bl_state = APPLE_BL_STATE_DISABLED;
+	mutex_unlock(&apple_bl_mutex);
+}
+EXPORT_SYMBOL_GPL(apple_bl_disable);
+
 static int __devinit apple_bl_add(struct acpi_device *dev)
 {
 	struct backlight_properties props;
 	struct pci_dev *host;
 	int intensity;
+	int ret = -ENODEV;
+
+	mutex_lock(&apple_bl_mutex);
+
+	switch (apple_bl_state) {
+	case APPLE_BL_STATE_IN_USE:
+		ret = -EBUSY;
+		goto out;
+	case APPLE_BL_STATE_DISABLED:
+		printk(KERN_ERR DRIVER "apple_bl disabled, ignoring device\n");
+		goto out;
+	}
 
 	host = pci_get_bus_and_slot(0, 0);
 
 	if (!host) {
 		printk(KERN_ERR DRIVER "unable to find PCI host\n");
-		return -ENODEV;
+		goto out;
 	}
 
 	if (host->vendor == PCI_VENDOR_ID_INTEL)
@@ -161,7 +203,7 @@ static int __devinit apple_bl_add(struct acpi_device *dev)
 
 	if (!hw_data) {
 		printk(KERN_ERR DRIVER "unknown hardware\n");
-		return -ENODEV;
+		goto out;
 	}
 
 	/* Check that the hardware responds - this may not work under EFI */
@@ -171,14 +213,16 @@ static int __devinit apple_bl_add(struct acpi_device *dev)
 	if (!intensity) {
 		hw_data->set_brightness(1);
 		if (!hw_data->backlight_ops.get_brightness(NULL))
-			return -ENODEV;
+			goto out;
 
 		hw_data->set_brightness(0);
 	}
 
 	if (!request_region(hw_data->iostart, hw_data->iolen,
-			    "Apple backlight"))
-		return -ENXIO;
+			    "Apple backlight")) {
+		ret = -ENXIO;
+		goto out;
+	}
 
 	memset(&props, 0, sizeof(struct backlight_properties));
 	props.type = BACKLIGHT_PLATFORM;
@@ -188,22 +232,30 @@ static int __devinit apple_bl_add(struct acpi_device *dev)
 
 	if (IS_ERR(apple_backlight_device)) {
 		release_region(hw_data->iostart, hw_data->iolen);
-		return PTR_ERR(apple_backlight_device);
+		ret = PTR_ERR(apple_backlight_device);
+		goto out;
 	}
 
 	apple_backlight_device->props.brightness =
 		hw_data->backlight_ops.get_brightness(apple_backlight_device);
 	backlight_update_status(apple_backlight_device);
 
-	return 0;
+	apple_bl_state = APPLE_BL_STATE_IN_USE;
+	ret = 0;
+
+out:
+	mutex_unlock(&apple_bl_mutex);
+	return ret;
 }
 
 static int __devexit apple_bl_remove(struct acpi_device *dev, int type)
 {
-	backlight_device_unregister(apple_backlight_device);
-
-	release_region(hw_data->iostart, hw_data->iolen);
-	hw_data = NULL;
+	mutex_lock(&apple_bl_mutex);
+	if (apple_bl_state == APPLE_BL_STATE_IN_USE) {
+		apple_bl_unregister();
+		apple_bl_state = APPLE_BL_STATE_UNUSED;
+	}
+	mutex_unlock(&apple_bl_mutex);
 	return 0;
 }
 
diff --git a/include/linux/apple_bl.h b/include/linux/apple_bl.h
new file mode 100644
index 0000000..7a4b6bc
--- /dev/null
+++ b/include/linux/apple_bl.h
@@ -0,0 +1,10 @@
+/*
+ * apple_bl exported symbols
+ */
+
+#ifndef _LINUX_APPLE_BL_H
+#define _LINUX_APPLE_BL_H
+
+extern void apple_bl_disable(void);
+
+#endif

Re: [PATCH v2] platform/x86: Add driver for Apple gmux device

[Matthew Garrett]

On Thu, Mar 15, 2012 at 10:47:56AM -0500, Seth Forshee wrote:

> Turns out that acpi_video0 does work under Windows, and it also works
> under Linux depending on how the OS is booted. Doing a boot camp style
> boot gives us working acpi_video0 and apple_bl backlights. acpi_video1
> doesn't show up in a legacy boot situation, as evaluating _BCM gives an
> error.
> 
> Under EFI boot, both acpi_video backlights show up but neither works.
> The readback verify in apple_bl's probe fails, so that one doesn't show
> up. Only gmux_backlight actually works.

Ok, that's consistent. You can probably handle that case by just calling 
acpi_video_unregister() in gmux. Something like your patch looks fine 
for the apple_bl case.

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