[PATCH] apple-gmux: Restore switch registers on suspend/resume

[PATCH] apple-gmux: Restore switch registers on suspend/resume

[Arun Raghavan]

After suspend and resume, the values of these registers seem to change
from what they were at suspend time, potentially preventing the actual
output lines from being enabled post-resume. This saves relevant state
at suspend and restores it when resumed.

This is at least required on the MacBook Pro 8.2 when the Intel GPU is
manually selected in GRUB config before the kernel is loaded.

Signed-off-by: Arun Raghavan <arun.raghavan@collabora.co.uk>
---
 drivers/platform/x86/apple-gmux.c |   29 ++++++++++++++++++++++++++++-
 1 files changed, 28 insertions(+), 1 deletions(-)

diff --git a/drivers/platform/x86/apple-gmux.c b/drivers/platform/x86/apple-gmux.c
index 694a15a..5a5d005 100644
--- a/drivers/platform/x86/apple-gmux.c
+++ b/drivers/platform/x86/apple-gmux.c
@@ -26,6 +26,11 @@ struct apple_gmux_data {
 	unsigned long iolen;
 
 	struct backlight_device *bdev;
+
+	bool was_suspended;
+	u8 save_switch_ddc;
+	u8 save_switch_disp;
+	u8 save_switch_ext;
 };
 
 /*
@@ -87,8 +92,30 @@ static int gmux_update_status(struct backlight_device *bd)
 	struct apple_gmux_data *gmux_data = bl_get_data(bd);
 	u32 brightness = bd->props.brightness;
 
-	if (bd->props.state & BL_CORE_SUSPENDED)
+	if (bd->props.state & BL_CORE_SUSPENDED) {
+		/* Save mux settings for output lines since they get reset on
+		 * suspend */
+		gmux_data->was_suspended = TRUE;
+		gmux_data->save_switch_ddc = gmux_read8(gmux_data,
+				GMUX_PORT_SWITCH_DDC);
+		gmux_data->save_switch_disp = gmux_read8(gmux_data,
+				GMUX_PORT_SWITCH_DISPLAY);
+		gmux_data->save_switch_ext = gmux_read8(gmux_data,
+				GMUX_PORT_SWITCH_EXTERNAL);
+
 		return 0;
+	}
+
+	if (gmux_data->was_suspended) {
+		/* Restore pre-suspend values for output lines */
+		gmux_data->was_suspended = FALSE;
+		gmux_write8(gmux_data, GMUX_PORT_SWITCH_DDC,
+				gmux_data->save_switch_ddc);
+		gmux_write8(gmux_data, GMUX_PORT_SWITCH_DISPLAY,
+				gmux_data->save_switch_disp);
+		gmux_write8(gmux_data, GMUX_PORT_SWITCH_EXTERNAL,
+				gmux_data->save_switch_ext);
+	}
 
 	/*
 	 * Older gmux versions require writing out lower bytes first then
-- 
1.7.8.6

Re: [PATCH] apple-gmux: Restore switch registers on suspend/resume

[Seth Forshee]

On Tue, Jul 10, 2012 at 09:09:53AM +0530, Arun Raghavan wrote:
> After suspend and resume, the values of these registers seem to change
> from what they were at suspend time, potentially preventing the actual
> output lines from being enabled post-resume. This saves relevant state
> at suspend and restores it when resumed.
> 
> This is at least required on the MacBook Pro 8.2 when the Intel GPU is
> manually selected in GRUB config before the kernel is loaded.

I've got a couple of problems with this. The first is that the backlight
update_status callback isn't an appropriate place to do this. Adding
suspend/resume ops for the pnp_driver would be better.

But the more serious concern is whether or not its safe to simply
restore the register values. I know that the reverse engineering work
done on the gmux has revealed a particular sequence for selecting the
active GPU. A brute force restoration of the registers disregards this
sequencing, which doesn't seem like a good idea.

Seth

Re: [PATCH] apple-gmux: Restore switch registers on suspend/resume

[Matthew Garrett]

On Tue, Jul 10, 2012 at 09:09:53AM +0530, Arun Raghavan wrote:
> After suspend and resume, the values of these registers seem to change
> from what they were at suspend time, potentially preventing the actual
> output lines from being enabled post-resume. This saves relevant state
> at suspend and restores it when resumed.
> 
> This is at least required on the MacBook Pro 8.2 when the Intel GPU is
> manually selected in GRUB config before the kernel is loaded.

I'd really rather add proper switcheroo support. Andreas, I have a patch 
with your name on it that appears to work (attached) - any objections to 
getting this merged?

commit 225a5a4ba90e9cf3de836c936cd01e9828cd0765
Author: Matthew Garrett <mjg@redhat.com>
Date:   Thu Mar 22 09:28:27 2012 -0400

    gmux switcheroo

diff --git a/drivers/platform/x86/apple-gmux.c b/drivers/platform/x86/apple-gmux.c
index 694a15a..ef2c9ac 100644
--- a/drivers/platform/x86/apple-gmux.c
+++ b/drivers/platform/x86/apple-gmux.c
@@ -2,6 +2,7 @@
  *  Gmux driver for Apple laptops
  *
  *  Copyright (C) Canonical Ltd. <seth.forshee@canonical.com>
+ *  Copyright (C) 2010-2012 Andreas Heider <andreas@meetr.de>
  *
  *  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
@@ -20,13 +21,22 @@
 #include <linux/slab.h>
 #include <acpi/video.h>
 #include <asm/io.h>
+#include <linux/pci.h>
+#include <linux/vga_switcheroo.h>
+#include <linux/delay.h>
 
-struct apple_gmux_data {
+static struct apple_gmux_data {
 	unsigned long iostart;
 	unsigned long iolen;
+	acpi_handle dhandle;
+	enum vga_switcheroo_client_id resume_client_id;
 
 	struct backlight_device *bdev;
-};
+} gmux_data;
+
+static struct pci_dev *discrete;
+
+DECLARE_COMPLETION(powerchange_done);
 
 /*
  * gmux port offsets. Many of these are not yet used, but may be in the
@@ -59,32 +69,29 @@ struct apple_gmux_data {
 #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)
+static inline u8 gmux_read8(int port)
 {
-	return inb(gmux_data->iostart + port);
+	return inb(gmux_data.iostart + port);
 }
 
-static inline void gmux_write8(struct apple_gmux_data *gmux_data, int port,
-			       u8 val)
+static inline void gmux_write8(int port, u8 val)
 {
-	outb(val, gmux_data->iostart + port);
+	outb(val, gmux_data.iostart + port);
 }
 
-static inline u32 gmux_read32(struct apple_gmux_data *gmux_data, int port)
+static inline u32 gmux_read32(int port)
 {
-	return inl(gmux_data->iostart + 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) &
+	return gmux_read32(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;
 
 	if (bd->props.state & BL_CORE_SUSPENDED)
@@ -96,54 +103,225 @@ static int gmux_update_status(struct backlight_device *bd)
 	 * 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);
+	gmux_write8(GMUX_PORT_BRIGHTNESS, brightness);
+	gmux_write8(GMUX_PORT_BRIGHTNESS + 1, brightness >> 8);
+	gmux_write8(GMUX_PORT_BRIGHTNESS + 2, brightness >> 16);
+	gmux_write8(GMUX_PORT_BRIGHTNESS + 3, 0);
+
+	return 0;
+}
+
+static int gmux_switchto(enum vga_switcheroo_client_id id)
+{
+	if (id == VGA_SWITCHEROO_IGD) {
+		gmux_write8(GMUX_PORT_SWITCH_DDC, 1);
+		gmux_write8(GMUX_PORT_SWITCH_DISPLAY, 2);
+		gmux_write8(GMUX_PORT_SWITCH_EXTERNAL, 2);
+	} else {
+		gmux_write8(GMUX_PORT_SWITCH_DDC, 2);
+		gmux_write8(GMUX_PORT_SWITCH_DISPLAY, 3);
+		gmux_write8(GMUX_PORT_SWITCH_EXTERNAL, 3);
+	}
+
+	return 0;
+}
+
+static int gmux_call_acpi_pwrd(int arg)
+{
+	acpi_handle gfx_handle = DEVICE_ACPI_HANDLE(&discrete->dev);
+	acpi_handle pwrd_handle = NULL;
+	acpi_status status = AE_OK;
+	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
+	union acpi_object arg0 = { ACPI_TYPE_INTEGER };
+	struct acpi_object_list arg_list = { 1, &arg0 };
+
+	if (!gfx_handle) {
+		pr_err("Cannot find device ACPI handle\n");
+		return -ENODEV;
+	}
+
+	status = acpi_get_handle(gfx_handle, "PWRD", &pwrd_handle);
+	if (ACPI_FAILURE(status)) {
+		pr_err("Cannot get PWRD handle: %s\n",
+		       acpi_format_exception(status));
+		return -ENODEV;
+	}
 
+	arg0.integer.value = arg;
+
+	status = acpi_evaluate_object(pwrd_handle, NULL, &arg_list, &buffer);
+	if (ACPI_FAILURE(status)) {
+		pr_err("PWRD call failed: %s\n",
+		       acpi_format_exception(status));
+		return -ENODEV;
+	}
+
+	kfree(buffer.pointer);
 	return 0;
 }
 
+static int gmux_set_discrete_state(enum vga_switcheroo_state state)
+{
+	/* TODO: locking for completions needed? */
+	init_completion(&powerchange_done);
+
+	if (state == VGA_SWITCHEROO_ON) {
+		gmux_call_acpi_pwrd(0);
+		gmux_write8(GMUX_PORT_DISCRETE_POWER, 1);
+		gmux_write8(GMUX_PORT_DISCRETE_POWER, 3);
+	} else {
+		gmux_write8(GMUX_PORT_DISCRETE_POWER, 1);
+		gmux_write8(GMUX_PORT_DISCRETE_POWER, 0);
+		gmux_call_acpi_pwrd(1);
+	}
+
+	if (wait_for_completion_interruptible_timeout(&powerchange_done,
+						      msecs_to_jiffies(200)))
+		pr_info("completion timeout\n");
+
+	return 0;
+}
+
+static int gmux_set_power_state(enum vga_switcheroo_client_id id,
+				enum vga_switcheroo_state state)
+{
+	if (id == VGA_SWITCHEROO_IGD)
+		return 0;
+
+	return gmux_set_discrete_state(state);
+}
+
+static int gmux_init(void)
+{
+	return 0;
+}
+
+static int gmux_get_client_id(struct pci_dev *pdev)
+{
+	/* early mbps with switchable graphics use nvidia integrated graphics,
+	 * hardcode that the 9400M is integrated */
+	if (pdev->vendor == PCI_VENDOR_ID_INTEL) {
+		return VGA_SWITCHEROO_IGD;
+	} else if (pdev->vendor == PCI_VENDOR_ID_NVIDIA &&
+		   pdev->device == 0x0863) {
+		return VGA_SWITCHEROO_IGD;
+	} else {
+		discrete = pdev;
+		return VGA_SWITCHEROO_DIS;
+	}
+}
+
+static bool gmux_client_active(enum vga_switcheroo_client_id id)
+{
+	u8 state = gmux_read8(GMUX_PORT_SWITCH_DISPLAY);
+
+	if ((id == VGA_SWITCHEROO_IGD && state == 2) ||
+	    (id == VGA_SWITCHEROO_DIS && state == 3))
+		return true;
+
+	return false;
+}
+
+static struct vga_switcheroo_handler gmux_handler = {
+	.switchto = gmux_switchto,
+	.power_state = gmux_set_power_state,
+	.init = gmux_init,
+	.get_client_id = gmux_get_client_id,
+	.client_active = gmux_client_active,
+};
+
+static void gmux_disable_interrupts(void)
+{
+	gmux_write8(GMUX_PORT_INTERRUPT_ENABLE, GMUX_INTERRUPT_DISABLE);
+}
+
+static void gmux_enable_interrupts(void)
+{
+	gmux_write8(GMUX_PORT_INTERRUPT_ENABLE, GMUX_INTERRUPT_ENABLE);
+}
+
+static int gmux_interrupt_get_status(void)
+{
+	return gmux_read8(GMUX_PORT_INTERRUPT_STATUS);
+}
+
+static void gmux_interrupt_activate_status(void)
+{
+	int old_status;
+	int new_status;
+
+	/* to reactivate interrupts write back current status */
+	old_status = gmux_read8(GMUX_PORT_INTERRUPT_STATUS);
+	gmux_write8(GMUX_PORT_INTERRUPT_STATUS, old_status);
+	new_status = gmux_read8(GMUX_PORT_INTERRUPT_STATUS);
+
+	/* status = 0 indicates active interrupts */
+	if (new_status)
+		pr_info("gmux: error: activate_status, old_status %d new_status %d\n", old_status, new_status);
+}
+
+static void gmux_notify_handler(acpi_handle device, u32 value, void *context)
+{
+	int status;
+
+	status = gmux_interrupt_get_status();
+	gmux_disable_interrupts();
+
+	gmux_interrupt_activate_status();
+	gmux_enable_interrupts();
+
+	if (status & GMUX_INTERRUPT_STATUS_POWER)
+		complete(&powerchange_done);
+}
+
 static const struct backlight_ops gmux_bl_ops = {
 	.options = BL_CORE_SUSPENDRESUME,
 	.get_brightness = gmux_get_brightness,
 	.update_status = gmux_update_status,
 };
 
+static int gmux_suspend(struct pnp_dev *dev, pm_message_t state)
+{
+	gmux_data.resume_client_id = gmux_read8(GMUX_PORT_SWITCH_DISPLAY) == 2 ?
+		VGA_SWITCHEROO_IGD : VGA_SWITCHEROO_DIS;
+	return 0;
+}
+
+static int gmux_resume(struct pnp_dev *dev)
+{
+	gmux_switchto(gmux_data.resume_client_id);
+	return 0;
+}
+
 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;
+	acpi_status status;
 	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;
+		goto err_begin;
 	}
 
-	gmux_data->iostart = res->start;
-	gmux_data->iolen = res->end - res->start;
+	gmux_data.iostart = res->start;
+	gmux_data.iolen = res->end - res->start;
 
-	if (gmux_data->iolen < GMUX_MIN_IO_LEN) {
+	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;
+		       gmux_data.iolen, GMUX_MIN_IO_LEN);
+		goto err_begin;
 	}
 
-	if (!request_region(gmux_data->iostart, gmux_data->iolen,
+	if (!request_region(gmux_data.iostart, gmux_data.iolen,
 			    "Apple gmux")) {
 		pr_err("gmux I/O already in use\n");
-		goto err_free;
+		goto err_begin;
 	}
 
 	/*
@@ -151,9 +329,9 @@ static int __devinit gmux_probe(struct pnp_dev *pnp,
 	 * 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);
+	ver_major = gmux_read8(GMUX_PORT_VERSION_MAJOR);
+	ver_minor = gmux_read8(GMUX_PORT_VERSION_MINOR);
+	ver_release = gmux_read8(GMUX_PORT_VERSION_RELEASE);
 	if (ver_major == 0xff && ver_minor == 0xff && ver_release == 0xff) {
 		pr_info("gmux device not present\n");
 		ret = -ENODEV;
@@ -165,7 +343,7 @@ static int __devinit gmux_probe(struct pnp_dev *pnp,
 
 	memset(&props, 0, sizeof(props));
 	props.type = BACKLIGHT_PLATFORM;
-	props.max_brightness = gmux_read32(gmux_data, GMUX_PORT_MAX_BRIGHTNESS);
+	props.max_brightness = gmux_read32(GMUX_PORT_MAX_BRIGHTNESS);
 
 	/*
 	 * Currently it's assumed that the maximum brightness is less than
@@ -177,44 +355,73 @@ static int __devinit gmux_probe(struct pnp_dev *pnp,
 		props.max_brightness = GMUX_MAX_BRIGHTNESS;
 
 	bdev = backlight_device_register("gmux_backlight", &pnp->dev,
-					 gmux_data, &gmux_bl_ops, &props);
+					 NULL, &gmux_bl_ops, &props);
 	if (IS_ERR(bdev)) {
 		ret = PTR_ERR(bdev);
 		goto err_release;
 	}
 
-	gmux_data->bdev = bdev;
+	gmux_data.bdev = bdev;
 	bdev->props.brightness = gmux_get_brightness(bdev);
 	backlight_update_status(bdev);
 
-	/*
-	 * The backlight situation on Macs is complicated. If the gmux is
-	 * present it's the best choice, because it always works for
-	 * backlight control and supports more levels than other options.
-	 * Disable the other backlight choices.
-	 */
-	acpi_video_unregister();
-	apple_bl_unregister();
+	gmux_data.dhandle = DEVICE_ACPI_HANDLE(&pnp->dev);
+	if (!gmux_data.dhandle) {
+		pr_err("Cannot find acpi device for pnp device %s\n",
+		       dev_name(&pnp->dev));
+		goto err_release;
+	} else {
+		struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER, NULL };
+		acpi_get_name(gmux_data.dhandle, ACPI_SINGLE_NAME, &buf);
+		pr_info("Found acpi handle for pnp device %s: %s\n",
+			dev_name(&pnp->dev), (char *)buf.pointer);
+		kfree(buf.pointer);
+	}
+
+	status = acpi_install_notify_handler(gmux_data.dhandle,
+				ACPI_DEVICE_NOTIFY, &gmux_notify_handler, pnp);
+	if (ACPI_FAILURE(status)) {
+		pr_err("Install notify handler failed: %s\n",
+		       acpi_format_exception(status));
+		goto err_notify;
+	}
+
+	if (vga_switcheroo_register_handler(&gmux_handler))
+		goto err_register;
+
+	init_completion(&powerchange_done);
+	gmux_enable_interrupts();
 
 	return 0;
 
+err_register:
+	status = acpi_remove_notify_handler(gmux_data.dhandle,
+				     ACPI_DEVICE_NOTIFY, &gmux_notify_handler);
+	if (ACPI_FAILURE(status))
+		pr_err("Remove notify handler failed: %s\n",
+		       acpi_format_exception(status));
+err_notify:
+	backlight_device_unregister(bdev);
 err_release:
-	release_region(gmux_data->iostart, gmux_data->iolen);
-err_free:
-	kfree(gmux_data);
+	release_region(gmux_data.iostart, gmux_data.iolen);
+err_begin:
 	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);
-
-	acpi_video_register();
-	apple_bl_register();
+	acpi_status status;
+
+	vga_switcheroo_unregister_handler();
+	backlight_device_unregister(gmux_data.bdev);
+	gmux_disable_interrupts();
+	status = acpi_remove_notify_handler(gmux_data.dhandle,
+				     ACPI_DEVICE_NOTIFY, &gmux_notify_handler);
+	if (ACPI_FAILURE(status))
+		pr_err("Remove notify handler failed: %s\n",
+		       acpi_format_exception(status));
+
+	release_region(gmux_data.iostart, gmux_data.iolen);
 }
 
 static const struct pnp_device_id gmux_device_ids[] = {
@@ -227,6 +434,8 @@ static struct pnp_driver gmux_pnp_driver = {
 	.probe		= gmux_probe,
 	.remove		= __devexit_p(gmux_remove),
 	.id_table	= gmux_device_ids,
+	.suspend	= gmux_suspend,
+	.resume		= gmux_resume
 };
 
 static int __init apple_gmux_init(void)

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

Re: [PATCH] apple-gmux: Restore switch registers on suspend/resume

[Seth Forshee]

[Adding Anreas on the Cc, looks like he was forgotten]

On Tue, Jul 10, 2012 at 05:05:55PM +0100, Matthew Garrett wrote:
> On Tue, Jul 10, 2012 at 09:09:53AM +0530, Arun Raghavan wrote:
> > After suspend and resume, the values of these registers seem to change
> > from what they were at suspend time, potentially preventing the actual
> > output lines from being enabled post-resume. This saves relevant state
> > at suspend and restores it when resumed.
> > 
> > This is at least required on the MacBook Pro 8.2 when the Intel GPU is
> > manually selected in GRUB config before the kernel is loaded.
> 
> I'd really rather add proper switcheroo support. Andreas, I have a patch 
> with your name on it that appears to work (attached) - any objections to 
> getting this merged?

I agree completely. I've really been wanting to get the switcheroo
support added, but I don't have hardware with working discrete graphics
under EFI boot to work with.

The patch seems to have some rough edges to work out, but once that's
done I'd be in favor of merging it as long as it's working well.

Seth

> 
> commit 225a5a4ba90e9cf3de836c936cd01e9828cd0765
> Author: Matthew Garrett <mjg@redhat.com>
> Date:   Thu Mar 22 09:28:27 2012 -0400
> 
>     gmux switcheroo
> 
> diff --git a/drivers/platform/x86/apple-gmux.c b/drivers/platform/x86/apple-gmux.c
> index 694a15a..ef2c9ac 100644
> --- a/drivers/platform/x86/apple-gmux.c
> +++ b/drivers/platform/x86/apple-gmux.c
> @@ -2,6 +2,7 @@
>   *  Gmux driver for Apple laptops
>   *
>   *  Copyright (C) Canonical Ltd. <seth.forshee@canonical.com>
> + *  Copyright (C) 2010-2012 Andreas Heider <andreas@meetr.de>
>   *
>   *  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
> @@ -20,13 +21,22 @@
>  #include <linux/slab.h>
>  #include <acpi/video.h>
>  #include <asm/io.h>
> +#include <linux/pci.h>
> +#include <linux/vga_switcheroo.h>
> +#include <linux/delay.h>
>  
> -struct apple_gmux_data {
> +static struct apple_gmux_data {
>  	unsigned long iostart;
>  	unsigned long iolen;
> +	acpi_handle dhandle;
> +	enum vga_switcheroo_client_id resume_client_id;
>  
>  	struct backlight_device *bdev;
> -};
> +} gmux_data;
> +
> +static struct pci_dev *discrete;
> +
> +DECLARE_COMPLETION(powerchange_done);
>  
>  /*
>   * gmux port offsets. Many of these are not yet used, but may be in the
> @@ -59,32 +69,29 @@ struct apple_gmux_data {
>  #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)
> +static inline u8 gmux_read8(int port)
>  {
> -	return inb(gmux_data->iostart + port);
> +	return inb(gmux_data.iostart + port);
>  }
>  
> -static inline void gmux_write8(struct apple_gmux_data *gmux_data, int port,
> -			       u8 val)
> +static inline void gmux_write8(int port, u8 val)
>  {
> -	outb(val, gmux_data->iostart + port);
> +	outb(val, gmux_data.iostart + port);
>  }
>  
> -static inline u32 gmux_read32(struct apple_gmux_data *gmux_data, int port)
> +static inline u32 gmux_read32(int port)
>  {
> -	return inl(gmux_data->iostart + 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) &
> +	return gmux_read32(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;
>  
>  	if (bd->props.state & BL_CORE_SUSPENDED)
> @@ -96,54 +103,225 @@ static int gmux_update_status(struct backlight_device *bd)
>  	 * 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);
> +	gmux_write8(GMUX_PORT_BRIGHTNESS, brightness);
> +	gmux_write8(GMUX_PORT_BRIGHTNESS + 1, brightness >> 8);
> +	gmux_write8(GMUX_PORT_BRIGHTNESS + 2, brightness >> 16);
> +	gmux_write8(GMUX_PORT_BRIGHTNESS + 3, 0);
> +
> +	return 0;
> +}
> +
> +static int gmux_switchto(enum vga_switcheroo_client_id id)
> +{
> +	if (id == VGA_SWITCHEROO_IGD) {
> +		gmux_write8(GMUX_PORT_SWITCH_DDC, 1);
> +		gmux_write8(GMUX_PORT_SWITCH_DISPLAY, 2);
> +		gmux_write8(GMUX_PORT_SWITCH_EXTERNAL, 2);
> +	} else {
> +		gmux_write8(GMUX_PORT_SWITCH_DDC, 2);
> +		gmux_write8(GMUX_PORT_SWITCH_DISPLAY, 3);
> +		gmux_write8(GMUX_PORT_SWITCH_EXTERNAL, 3);
> +	}
> +
> +	return 0;
> +}
> +
> +static int gmux_call_acpi_pwrd(int arg)
> +{
> +	acpi_handle gfx_handle = DEVICE_ACPI_HANDLE(&discrete->dev);
> +	acpi_handle pwrd_handle = NULL;
> +	acpi_status status = AE_OK;
> +	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> +	union acpi_object arg0 = { ACPI_TYPE_INTEGER };
> +	struct acpi_object_list arg_list = { 1, &arg0 };
> +
> +	if (!gfx_handle) {
> +		pr_err("Cannot find device ACPI handle\n");
> +		return -ENODEV;
> +	}
> +
> +	status = acpi_get_handle(gfx_handle, "PWRD", &pwrd_handle);
> +	if (ACPI_FAILURE(status)) {
> +		pr_err("Cannot get PWRD handle: %s\n",
> +		       acpi_format_exception(status));
> +		return -ENODEV;
> +	}
>  
> +	arg0.integer.value = arg;
> +
> +	status = acpi_evaluate_object(pwrd_handle, NULL, &arg_list, &buffer);
> +	if (ACPI_FAILURE(status)) {
> +		pr_err("PWRD call failed: %s\n",
> +		       acpi_format_exception(status));
> +		return -ENODEV;
> +	}
> +
> +	kfree(buffer.pointer);
>  	return 0;
>  }
>  
> +static int gmux_set_discrete_state(enum vga_switcheroo_state state)
> +{
> +	/* TODO: locking for completions needed? */
> +	init_completion(&powerchange_done);
> +
> +	if (state == VGA_SWITCHEROO_ON) {
> +		gmux_call_acpi_pwrd(0);
> +		gmux_write8(GMUX_PORT_DISCRETE_POWER, 1);
> +		gmux_write8(GMUX_PORT_DISCRETE_POWER, 3);
> +	} else {
> +		gmux_write8(GMUX_PORT_DISCRETE_POWER, 1);
> +		gmux_write8(GMUX_PORT_DISCRETE_POWER, 0);
> +		gmux_call_acpi_pwrd(1);
> +	}
> +
> +	if (wait_for_completion_interruptible_timeout(&powerchange_done,
> +						      msecs_to_jiffies(200)))
> +		pr_info("completion timeout\n");
> +
> +	return 0;
> +}
> +
> +static int gmux_set_power_state(enum vga_switcheroo_client_id id,
> +				enum vga_switcheroo_state state)
> +{
> +	if (id == VGA_SWITCHEROO_IGD)
> +		return 0;
> +
> +	return gmux_set_discrete_state(state);
> +}
> +
> +static int gmux_init(void)
> +{
> +	return 0;
> +}
> +
> +static int gmux_get_client_id(struct pci_dev *pdev)
> +{
> +	/* early mbps with switchable graphics use nvidia integrated graphics,
> +	 * hardcode that the 9400M is integrated */
> +	if (pdev->vendor == PCI_VENDOR_ID_INTEL) {
> +		return VGA_SWITCHEROO_IGD;
> +	} else if (pdev->vendor == PCI_VENDOR_ID_NVIDIA &&
> +		   pdev->device == 0x0863) {
> +		return VGA_SWITCHEROO_IGD;
> +	} else {
> +		discrete = pdev;
> +		return VGA_SWITCHEROO_DIS;
> +	}
> +}
> +
> +static bool gmux_client_active(enum vga_switcheroo_client_id id)
> +{
> +	u8 state = gmux_read8(GMUX_PORT_SWITCH_DISPLAY);
> +
> +	if ((id == VGA_SWITCHEROO_IGD && state == 2) ||
> +	    (id == VGA_SWITCHEROO_DIS && state == 3))
> +		return true;
> +
> +	return false;
> +}
> +
> +static struct vga_switcheroo_handler gmux_handler = {
> +	.switchto = gmux_switchto,
> +	.power_state = gmux_set_power_state,
> +	.init = gmux_init,
> +	.get_client_id = gmux_get_client_id,
> +	.client_active = gmux_client_active,
> +};
> +
> +static void gmux_disable_interrupts(void)
> +{
> +	gmux_write8(GMUX_PORT_INTERRUPT_ENABLE, GMUX_INTERRUPT_DISABLE);
> +}
> +
> +static void gmux_enable_interrupts(void)
> +{
> +	gmux_write8(GMUX_PORT_INTERRUPT_ENABLE, GMUX_INTERRUPT_ENABLE);
> +}
> +
> +static int gmux_interrupt_get_status(void)
> +{
> +	return gmux_read8(GMUX_PORT_INTERRUPT_STATUS);
> +}
> +
> +static void gmux_interrupt_activate_status(void)
> +{
> +	int old_status;
> +	int new_status;
> +
> +	/* to reactivate interrupts write back current status */
> +	old_status = gmux_read8(GMUX_PORT_INTERRUPT_STATUS);
> +	gmux_write8(GMUX_PORT_INTERRUPT_STATUS, old_status);
> +	new_status = gmux_read8(GMUX_PORT_INTERRUPT_STATUS);
> +
> +	/* status = 0 indicates active interrupts */
> +	if (new_status)
> +		pr_info("gmux: error: activate_status, old_status %d new_status %d\n", old_status, new_status);
> +}
> +
> +static void gmux_notify_handler(acpi_handle device, u32 value, void *context)
> +{
> +	int status;
> +
> +	status = gmux_interrupt_get_status();
> +	gmux_disable_interrupts();
> +
> +	gmux_interrupt_activate_status();
> +	gmux_enable_interrupts();
> +
> +	if (status & GMUX_INTERRUPT_STATUS_POWER)
> +		complete(&powerchange_done);
> +}
> +
>  static const struct backlight_ops gmux_bl_ops = {
>  	.options = BL_CORE_SUSPENDRESUME,
>  	.get_brightness = gmux_get_brightness,
>  	.update_status = gmux_update_status,
>  };
>  
> +static int gmux_suspend(struct pnp_dev *dev, pm_message_t state)
> +{
> +	gmux_data.resume_client_id = gmux_read8(GMUX_PORT_SWITCH_DISPLAY) == 2 ?
> +		VGA_SWITCHEROO_IGD : VGA_SWITCHEROO_DIS;
> +	return 0;
> +}
> +
> +static int gmux_resume(struct pnp_dev *dev)
> +{
> +	gmux_switchto(gmux_data.resume_client_id);
> +	return 0;
> +}
> +
>  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;
> +	acpi_status status;
>  	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;
> +		goto err_begin;
>  	}
>  
> -	gmux_data->iostart = res->start;
> -	gmux_data->iolen = res->end - res->start;
> +	gmux_data.iostart = res->start;
> +	gmux_data.iolen = res->end - res->start;
>  
> -	if (gmux_data->iolen < GMUX_MIN_IO_LEN) {
> +	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;
> +		       gmux_data.iolen, GMUX_MIN_IO_LEN);
> +		goto err_begin;
>  	}
>  
> -	if (!request_region(gmux_data->iostart, gmux_data->iolen,
> +	if (!request_region(gmux_data.iostart, gmux_data.iolen,
>  			    "Apple gmux")) {
>  		pr_err("gmux I/O already in use\n");
> -		goto err_free;
> +		goto err_begin;
>  	}
>  
>  	/*
> @@ -151,9 +329,9 @@ static int __devinit gmux_probe(struct pnp_dev *pnp,
>  	 * 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);
> +	ver_major = gmux_read8(GMUX_PORT_VERSION_MAJOR);
> +	ver_minor = gmux_read8(GMUX_PORT_VERSION_MINOR);
> +	ver_release = gmux_read8(GMUX_PORT_VERSION_RELEASE);
>  	if (ver_major == 0xff && ver_minor == 0xff && ver_release == 0xff) {
>  		pr_info("gmux device not present\n");
>  		ret = -ENODEV;
> @@ -165,7 +343,7 @@ static int __devinit gmux_probe(struct pnp_dev *pnp,
>  
>  	memset(&props, 0, sizeof(props));
>  	props.type = BACKLIGHT_PLATFORM;
> -	props.max_brightness = gmux_read32(gmux_data, GMUX_PORT_MAX_BRIGHTNESS);
> +	props.max_brightness = gmux_read32(GMUX_PORT_MAX_BRIGHTNESS);
>  
>  	/*
>  	 * Currently it's assumed that the maximum brightness is less than
> @@ -177,44 +355,73 @@ static int __devinit gmux_probe(struct pnp_dev *pnp,
>  		props.max_brightness = GMUX_MAX_BRIGHTNESS;
>  
>  	bdev = backlight_device_register("gmux_backlight", &pnp->dev,
> -					 gmux_data, &gmux_bl_ops, &props);
> +					 NULL, &gmux_bl_ops, &props);
>  	if (IS_ERR(bdev)) {
>  		ret = PTR_ERR(bdev);
>  		goto err_release;
>  	}
>  
> -	gmux_data->bdev = bdev;
> +	gmux_data.bdev = bdev;
>  	bdev->props.brightness = gmux_get_brightness(bdev);
>  	backlight_update_status(bdev);
>  
> -	/*
> -	 * The backlight situation on Macs is complicated. If the gmux is
> -	 * present it's the best choice, because it always works for
> -	 * backlight control and supports more levels than other options.
> -	 * Disable the other backlight choices.
> -	 */
> -	acpi_video_unregister();
> -	apple_bl_unregister();
> +	gmux_data.dhandle = DEVICE_ACPI_HANDLE(&pnp->dev);
> +	if (!gmux_data.dhandle) {
> +		pr_err("Cannot find acpi device for pnp device %s\n",
> +		       dev_name(&pnp->dev));
> +		goto err_release;
> +	} else {
> +		struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER, NULL };
> +		acpi_get_name(gmux_data.dhandle, ACPI_SINGLE_NAME, &buf);
> +		pr_info("Found acpi handle for pnp device %s: %s\n",
> +			dev_name(&pnp->dev), (char *)buf.pointer);
> +		kfree(buf.pointer);
> +	}
> +
> +	status = acpi_install_notify_handler(gmux_data.dhandle,
> +				ACPI_DEVICE_NOTIFY, &gmux_notify_handler, pnp);
> +	if (ACPI_FAILURE(status)) {
> +		pr_err("Install notify handler failed: %s\n",
> +		       acpi_format_exception(status));
> +		goto err_notify;
> +	}
> +
> +	if (vga_switcheroo_register_handler(&gmux_handler))
> +		goto err_register;
> +
> +	init_completion(&powerchange_done);
> +	gmux_enable_interrupts();
>  
>  	return 0;
>  
> +err_register:
> +	status = acpi_remove_notify_handler(gmux_data.dhandle,
> +				     ACPI_DEVICE_NOTIFY, &gmux_notify_handler);
> +	if (ACPI_FAILURE(status))
> +		pr_err("Remove notify handler failed: %s\n",
> +		       acpi_format_exception(status));
> +err_notify:
> +	backlight_device_unregister(bdev);
>  err_release:
> -	release_region(gmux_data->iostart, gmux_data->iolen);
> -err_free:
> -	kfree(gmux_data);
> +	release_region(gmux_data.iostart, gmux_data.iolen);
> +err_begin:
>  	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);
> -
> -	acpi_video_register();
> -	apple_bl_register();
> +	acpi_status status;
> +
> +	vga_switcheroo_unregister_handler();
> +	backlight_device_unregister(gmux_data.bdev);
> +	gmux_disable_interrupts();
> +	status = acpi_remove_notify_handler(gmux_data.dhandle,
> +				     ACPI_DEVICE_NOTIFY, &gmux_notify_handler);
> +	if (ACPI_FAILURE(status))
> +		pr_err("Remove notify handler failed: %s\n",
> +		       acpi_format_exception(status));
> +
> +	release_region(gmux_data.iostart, gmux_data.iolen);
>  }
>  
>  static const struct pnp_device_id gmux_device_ids[] = {
> @@ -227,6 +434,8 @@ static struct pnp_driver gmux_pnp_driver = {
>  	.probe		= gmux_probe,
>  	.remove		= __devexit_p(gmux_remove),
>  	.id_table	= gmux_device_ids,
> +	.suspend	= gmux_suspend,
> +	.resume		= gmux_resume
>  };
>  
>  static int __init apple_gmux_init(void)
> 
> -- 
> Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] apple-gmux: Restore switch registers on suspend/resume

[Andreas Heider]

Thanks for adding me, seeing the gmux driver progress is always great.

Regarding the original patch: This is probably only useful when the gmux 
was switched in GRUB  and there's already a solution for the resume 
problem in userspace 
(http://ubuntuforums.org/showpost.php?p=10695119&postcount=261). Maybe 
the same could be achieved a bit cleaner by simply using a few parts of 
the switching patch?

Regarding the gmux switching patch: The patch itself should work and 
hopefully doesn't break stuff that wasn't broken before. But without 
additional fixes to i915/nouveau/etc. It won't be too useful either, 
although it should fix the resume issue.

I'll look into how well it works with the current kernel on the weekend, 
but I don't want to promise too much since time is still a bit of an 
issue for me.

Andreas

Am 10.07.12 18:35, schrieb Seth Forshee:
> [Adding Anreas on the Cc, looks like he was forgotten]
>
> On Tue, Jul 10, 2012 at 05:05:55PM +0100, Matthew Garrett wrote:
>> On Tue, Jul 10, 2012 at 09:09:53AM +0530, Arun Raghavan wrote:
>>> After suspend and resume, the values of these registers seem to change
>>> from what they were at suspend time, potentially preventing the actual
>>> output lines from being enabled post-resume. This saves relevant state
>>> at suspend and restores it when resumed.
>>>
>>> This is at least required on the MacBook Pro 8.2 when the Intel GPU is
>>> manually selected in GRUB config before the kernel is loaded.
>> I'd really rather add proper switcheroo support. Andreas, I have a patch
>> with your name on it that appears to work (attached) - any objections to
>> getting this merged?
> I agree completely. I've really been wanting to get the switcheroo
> support added, but I don't have hardware with working discrete graphics
> under EFI boot to work with.
>
> The patch seems to have some rough edges to work out, but once that's
> done I'd be in favor of merging it as long as it's working well.
>
> Seth
>
>> commit 225a5a4ba90e9cf3de836c936cd01e9828cd0765
>> Author: Matthew Garrett <mjg@redhat.com>
>> Date:   Thu Mar 22 09:28:27 2012 -0400
>>
>>      gmux switcheroo
>>
>> diff --git a/drivers/platform/x86/apple-gmux.c b/drivers/platform/x86/apple-gmux.c
>> index 694a15a..ef2c9ac 100644
>> --- a/drivers/platform/x86/apple-gmux.c
>> +++ b/drivers/platform/x86/apple-gmux.c
>> @@ -2,6 +2,7 @@
>>    *  Gmux driver for Apple laptops
>>    *
>>    *  Copyright (C) Canonical Ltd. <seth.forshee@canonical.com>
>> + *  Copyright (C) 2010-2012 Andreas Heider <andreas@meetr.de>
>>    *
>>    *  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
>> @@ -20,13 +21,22 @@
>>   #include <linux/slab.h>
>>   #include <acpi/video.h>
>>   #include <asm/io.h>
>> +#include <linux/pci.h>
>> +#include <linux/vga_switcheroo.h>
>> +#include <linux/delay.h>
>>   
>> -struct apple_gmux_data {
>> +static struct apple_gmux_data {
>>   	unsigned long iostart;
>>   	unsigned long iolen;
>> +	acpi_handle dhandle;
>> +	enum vga_switcheroo_client_id resume_client_id;
>>   
>>   	struct backlight_device *bdev;
>> -};
>> +} gmux_data;
>> +
>> +static struct pci_dev *discrete;
>> +
>> +DECLARE_COMPLETION(powerchange_done);
>>   
>>   /*
>>    * gmux port offsets. Many of these are not yet used, but may be in the
>> @@ -59,32 +69,29 @@ struct apple_gmux_data {
>>   #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)
>> +static inline u8 gmux_read8(int port)
>>   {
>> -	return inb(gmux_data->iostart + port);
>> +	return inb(gmux_data.iostart + port);
>>   }
>>   
>> -static inline void gmux_write8(struct apple_gmux_data *gmux_data, int port,
>> -			       u8 val)
>> +static inline void gmux_write8(int port, u8 val)
>>   {
>> -	outb(val, gmux_data->iostart + port);
>> +	outb(val, gmux_data.iostart + port);
>>   }
>>   
>> -static inline u32 gmux_read32(struct apple_gmux_data *gmux_data, int port)
>> +static inline u32 gmux_read32(int port)
>>   {
>> -	return inl(gmux_data->iostart + 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) &
>> +	return gmux_read32(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;
>>   
>>   	if (bd->props.state & BL_CORE_SUSPENDED)
>> @@ -96,54 +103,225 @@ static int gmux_update_status(struct backlight_device *bd)
>>   	 * 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);
>> +	gmux_write8(GMUX_PORT_BRIGHTNESS, brightness);
>> +	gmux_write8(GMUX_PORT_BRIGHTNESS + 1, brightness >> 8);
>> +	gmux_write8(GMUX_PORT_BRIGHTNESS + 2, brightness >> 16);
>> +	gmux_write8(GMUX_PORT_BRIGHTNESS + 3, 0);
>> +
>> +	return 0;
>> +}
>> +
>> +static int gmux_switchto(enum vga_switcheroo_client_id id)
>> +{
>> +	if (id == VGA_SWITCHEROO_IGD) {
>> +		gmux_write8(GMUX_PORT_SWITCH_DDC, 1);
>> +		gmux_write8(GMUX_PORT_SWITCH_DISPLAY, 2);
>> +		gmux_write8(GMUX_PORT_SWITCH_EXTERNAL, 2);
>> +	} else {
>> +		gmux_write8(GMUX_PORT_SWITCH_DDC, 2);
>> +		gmux_write8(GMUX_PORT_SWITCH_DISPLAY, 3);
>> +		gmux_write8(GMUX_PORT_SWITCH_EXTERNAL, 3);
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int gmux_call_acpi_pwrd(int arg)
>> +{
>> +	acpi_handle gfx_handle = DEVICE_ACPI_HANDLE(&discrete->dev);
>> +	acpi_handle pwrd_handle = NULL;
>> +	acpi_status status = AE_OK;
>> +	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
>> +	union acpi_object arg0 = { ACPI_TYPE_INTEGER };
>> +	struct acpi_object_list arg_list = { 1, &arg0 };
>> +
>> +	if (!gfx_handle) {
>> +		pr_err("Cannot find device ACPI handle\n");
>> +		return -ENODEV;
>> +	}
>> +
>> +	status = acpi_get_handle(gfx_handle, "PWRD", &pwrd_handle);
>> +	if (ACPI_FAILURE(status)) {
>> +		pr_err("Cannot get PWRD handle: %s\n",
>> +		       acpi_format_exception(status));
>> +		return -ENODEV;
>> +	}
>>   
>> +	arg0.integer.value = arg;
>> +
>> +	status = acpi_evaluate_object(pwrd_handle, NULL, &arg_list, &buffer);
>> +	if (ACPI_FAILURE(status)) {
>> +		pr_err("PWRD call failed: %s\n",
>> +		       acpi_format_exception(status));
>> +		return -ENODEV;
>> +	}
>> +
>> +	kfree(buffer.pointer);
>>   	return 0;
>>   }
>>   
>> +static int gmux_set_discrete_state(enum vga_switcheroo_state state)
>> +{
>> +	/* TODO: locking for completions needed? */
>> +	init_completion(&powerchange_done);
>> +
>> +	if (state == VGA_SWITCHEROO_ON) {
>> +		gmux_call_acpi_pwrd(0);
>> +		gmux_write8(GMUX_PORT_DISCRETE_POWER, 1);
>> +		gmux_write8(GMUX_PORT_DISCRETE_POWER, 3);
>> +	} else {
>> +		gmux_write8(GMUX_PORT_DISCRETE_POWER, 1);
>> +		gmux_write8(GMUX_PORT_DISCRETE_POWER, 0);
>> +		gmux_call_acpi_pwrd(1);
>> +	}
>> +
>> +	if (wait_for_completion_interruptible_timeout(&powerchange_done,
>> +						      msecs_to_jiffies(200)))
>> +		pr_info("completion timeout\n");
>> +
>> +	return 0;
>> +}
>> +
>> +static int gmux_set_power_state(enum vga_switcheroo_client_id id,
>> +				enum vga_switcheroo_state state)
>> +{
>> +	if (id == VGA_SWITCHEROO_IGD)
>> +		return 0;
>> +
>> +	return gmux_set_discrete_state(state);
>> +}
>> +
>> +static int gmux_init(void)
>> +{
>> +	return 0;
>> +}
>> +
>> +static int gmux_get_client_id(struct pci_dev *pdev)
>> +{
>> +	/* early mbps with switchable graphics use nvidia integrated graphics,
>> +	 * hardcode that the 9400M is integrated */
>> +	if (pdev->vendor == PCI_VENDOR_ID_INTEL) {
>> +		return VGA_SWITCHEROO_IGD;
>> +	} else if (pdev->vendor == PCI_VENDOR_ID_NVIDIA &&
>> +		   pdev->device == 0x0863) {
>> +		return VGA_SWITCHEROO_IGD;
>> +	} else {
>> +		discrete = pdev;
>> +		return VGA_SWITCHEROO_DIS;
>> +	}
>> +}
>> +
>> +static bool gmux_client_active(enum vga_switcheroo_client_id id)
>> +{
>> +	u8 state = gmux_read8(GMUX_PORT_SWITCH_DISPLAY);
>> +
>> +	if ((id == VGA_SWITCHEROO_IGD && state == 2) ||
>> +	    (id == VGA_SWITCHEROO_DIS && state == 3))
>> +		return true;
>> +
>> +	return false;
>> +}
>> +
>> +static struct vga_switcheroo_handler gmux_handler = {
>> +	.switchto = gmux_switchto,
>> +	.power_state = gmux_set_power_state,
>> +	.init = gmux_init,
>> +	.get_client_id = gmux_get_client_id,
>> +	.client_active = gmux_client_active,
>> +};
>> +
>> +static void gmux_disable_interrupts(void)
>> +{
>> +	gmux_write8(GMUX_PORT_INTERRUPT_ENABLE, GMUX_INTERRUPT_DISABLE);
>> +}
>> +
>> +static void gmux_enable_interrupts(void)
>> +{
>> +	gmux_write8(GMUX_PORT_INTERRUPT_ENABLE, GMUX_INTERRUPT_ENABLE);
>> +}
>> +
>> +static int gmux_interrupt_get_status(void)
>> +{
>> +	return gmux_read8(GMUX_PORT_INTERRUPT_STATUS);
>> +}
>> +
>> +static void gmux_interrupt_activate_status(void)
>> +{
>> +	int old_status;
>> +	int new_status;
>> +
>> +	/* to reactivate interrupts write back current status */
>> +	old_status = gmux_read8(GMUX_PORT_INTERRUPT_STATUS);
>> +	gmux_write8(GMUX_PORT_INTERRUPT_STATUS, old_status);
>> +	new_status = gmux_read8(GMUX_PORT_INTERRUPT_STATUS);
>> +
>> +	/* status = 0 indicates active interrupts */
>> +	if (new_status)
>> +		pr_info("gmux: error: activate_status, old_status %d new_status %d\n", old_status, new_status);
>> +}
>> +
>> +static void gmux_notify_handler(acpi_handle device, u32 value, void *context)
>> +{
>> +	int status;
>> +
>> +	status = gmux_interrupt_get_status();
>> +	gmux_disable_interrupts();
>> +
>> +	gmux_interrupt_activate_status();
>> +	gmux_enable_interrupts();
>> +
>> +	if (status & GMUX_INTERRUPT_STATUS_POWER)
>> +		complete(&powerchange_done);
>> +}
>> +
>>   static const struct backlight_ops gmux_bl_ops = {
>>   	.options = BL_CORE_SUSPENDRESUME,
>>   	.get_brightness = gmux_get_brightness,
>>   	.update_status = gmux_update_status,
>>   };
>>   
>> +static int gmux_suspend(struct pnp_dev *dev, pm_message_t state)
>> +{
>> +	gmux_data.resume_client_id = gmux_read8(GMUX_PORT_SWITCH_DISPLAY) == 2 ?
>> +		VGA_SWITCHEROO_IGD : VGA_SWITCHEROO_DIS;
>> +	return 0;
>> +}
>> +
>> +static int gmux_resume(struct pnp_dev *dev)
>> +{
>> +	gmux_switchto(gmux_data.resume_client_id);
>> +	return 0;
>> +}
>> +
>>   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;
>> +	acpi_status status;
>>   	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;
>> +		goto err_begin;
>>   	}
>>   
>> -	gmux_data->iostart = res->start;
>> -	gmux_data->iolen = res->end - res->start;
>> +	gmux_data.iostart = res->start;
>> +	gmux_data.iolen = res->end - res->start;
>>   
>> -	if (gmux_data->iolen < GMUX_MIN_IO_LEN) {
>> +	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;
>> +		       gmux_data.iolen, GMUX_MIN_IO_LEN);
>> +		goto err_begin;
>>   	}
>>   
>> -	if (!request_region(gmux_data->iostart, gmux_data->iolen,
>> +	if (!request_region(gmux_data.iostart, gmux_data.iolen,
>>   			    "Apple gmux")) {
>>   		pr_err("gmux I/O already in use\n");
>> -		goto err_free;
>> +		goto err_begin;
>>   	}
>>   
>>   	/*
>> @@ -151,9 +329,9 @@ static int __devinit gmux_probe(struct pnp_dev *pnp,
>>   	 * 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);
>> +	ver_major = gmux_read8(GMUX_PORT_VERSION_MAJOR);
>> +	ver_minor = gmux_read8(GMUX_PORT_VERSION_MINOR);
>> +	ver_release = gmux_read8(GMUX_PORT_VERSION_RELEASE);
>>   	if (ver_major == 0xff && ver_minor == 0xff && ver_release == 0xff) {
>>   		pr_info("gmux device not present\n");
>>   		ret = -ENODEV;
>> @@ -165,7 +343,7 @@ static int __devinit gmux_probe(struct pnp_dev *pnp,
>>   
>>   	memset(&props, 0, sizeof(props));
>>   	props.type = BACKLIGHT_PLATFORM;
>> -	props.max_brightness = gmux_read32(gmux_data, GMUX_PORT_MAX_BRIGHTNESS);
>> +	props.max_brightness = gmux_read32(GMUX_PORT_MAX_BRIGHTNESS);
>>   
>>   	/*
>>   	 * Currently it's assumed that the maximum brightness is less than
>> @@ -177,44 +355,73 @@ static int __devinit gmux_probe(struct pnp_dev *pnp,
>>   		props.max_brightness = GMUX_MAX_BRIGHTNESS;
>>   
>>   	bdev = backlight_device_register("gmux_backlight", &pnp->dev,
>> -					 gmux_data, &gmux_bl_ops, &props);
>> +					 NULL, &gmux_bl_ops, &props);
>>   	if (IS_ERR(bdev)) {
>>   		ret = PTR_ERR(bdev);
>>   		goto err_release;
>>   	}
>>   
>> -	gmux_data->bdev = bdev;
>> +	gmux_data.bdev = bdev;
>>   	bdev->props.brightness = gmux_get_brightness(bdev);
>>   	backlight_update_status(bdev);
>>   
>> -	/*
>> -	 * The backlight situation on Macs is complicated. If the gmux is
>> -	 * present it's the best choice, because it always works for
>> -	 * backlight control and supports more levels than other options.
>> -	 * Disable the other backlight choices.
>> -	 */
>> -	acpi_video_unregister();
>> -	apple_bl_unregister();
>> +	gmux_data.dhandle = DEVICE_ACPI_HANDLE(&pnp->dev);
>> +	if (!gmux_data.dhandle) {
>> +		pr_err("Cannot find acpi device for pnp device %s\n",
>> +		       dev_name(&pnp->dev));
>> +		goto err_release;
>> +	} else {
>> +		struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER, NULL };
>> +		acpi_get_name(gmux_data.dhandle, ACPI_SINGLE_NAME, &buf);
>> +		pr_info("Found acpi handle for pnp device %s: %s\n",
>> +			dev_name(&pnp->dev), (char *)buf.pointer);
>> +		kfree(buf.pointer);
>> +	}
>> +
>> +	status = acpi_install_notify_handler(gmux_data.dhandle,
>> +				ACPI_DEVICE_NOTIFY, &gmux_notify_handler, pnp);
>> +	if (ACPI_FAILURE(status)) {
>> +		pr_err("Install notify handler failed: %s\n",
>> +		       acpi_format_exception(status));
>> +		goto err_notify;
>> +	}
>> +
>> +	if (vga_switcheroo_register_handler(&gmux_handler))
>> +		goto err_register;
>> +
>> +	init_completion(&powerchange_done);
>> +	gmux_enable_interrupts();
>>   
>>   	return 0;
>>   
>> +err_register:
>> +	status = acpi_remove_notify_handler(gmux_data.dhandle,
>> +				     ACPI_DEVICE_NOTIFY, &gmux_notify_handler);
>> +	if (ACPI_FAILURE(status))
>> +		pr_err("Remove notify handler failed: %s\n",
>> +		       acpi_format_exception(status));
>> +err_notify:
>> +	backlight_device_unregister(bdev);
>>   err_release:
>> -	release_region(gmux_data->iostart, gmux_data->iolen);
>> -err_free:
>> -	kfree(gmux_data);
>> +	release_region(gmux_data.iostart, gmux_data.iolen);
>> +err_begin:
>>   	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);
>> -
>> -	acpi_video_register();
>> -	apple_bl_register();
>> +	acpi_status status;
>> +
>> +	vga_switcheroo_unregister_handler();
>> +	backlight_device_unregister(gmux_data.bdev);
>> +	gmux_disable_interrupts();
>> +	status = acpi_remove_notify_handler(gmux_data.dhandle,
>> +				     ACPI_DEVICE_NOTIFY, &gmux_notify_handler);
>> +	if (ACPI_FAILURE(status))
>> +		pr_err("Remove notify handler failed: %s\n",
>> +		       acpi_format_exception(status));
>> +
>> +	release_region(gmux_data.iostart, gmux_data.iolen);
>>   }
>>   
>>   static const struct pnp_device_id gmux_device_ids[] = {
>> @@ -227,6 +434,8 @@ static struct pnp_driver gmux_pnp_driver = {
>>   	.probe		= gmux_probe,
>>   	.remove		= __devexit_p(gmux_remove),
>>   	.id_table	= gmux_device_ids,
>> +	.suspend	= gmux_suspend,
>> +	.resume		= gmux_resume
>>   };
>>   
>>   static int __init apple_gmux_init(void)
>>
>> -- 
>> Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] apple-gmux: Restore switch registers on suspend/resume

[David Woodhouse]

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

On Wed, 2012-07-11 at 02:25 +0200, Andreas Heider wrote:
> Thanks for adding me, seeing the gmux driver progress is always great.
> 
> Regarding the original patch: This is probably only useful when the gmux 
> was switched in GRUB  and there's already a solution for the resume 
> problem in userspace 
> (http://ubuntuforums.org/showpost.php?p=10695119&postcount=261). Maybe 
> the same could be achieved a bit cleaner by simply using a few parts of 
> the switching patch?
> 
> Regarding the gmux switching patch: The patch itself should work and 
> hopefully doesn't break stuff that wasn't broken before. But without 
> additional fixes to i915/nouveau/etc. It won't be too useful either, 
> although it should fix the resume issue.
> 
> I'll look into how well it works with the current kernel on the weekend, 
> but I don't want to promise too much since time is still a bit of an 
> issue for me.

Using the patches from http://www.codon.org.uk/~mjg59/tmp/radeon_efi/
and http://www.codon.org.uk/~mjg59/tmp/gmux_switcheroo.diff I can almost
get this working on my MBP8,3, chainloading the kernel from grub1-efi.

I have to fix pci_map_rom() to return pdev->rom even if the resource has
the IORESOURCE_ROM_SHADOW flag (Matthew's patch still returned 0xC0000),
and I have to fix a bug in vga_switcheroo_enable() where it uses
client->id before actually setting it.

And then, if I hack head64.c to switch the mux over to IGD but leave the
Radeon powered up: ("outb(1, 0x728); outb(2, 0x710); outb(2, 0x740);")

... and on the occasions that it doesn't crash in fbcon_event_notify()
as shown at http://david.woodhou.se/vga-switcheroo-oops.txt


... it boots with a blank screen but I can log in over the network and I
*can* actually switch between displays. After switching to 'DIS' and
back to 'IGD' it does actually start working, and I seem to have both X
and fbcon working sanely on both. Although on an earlier boot I don't
think fbcon was working on the Radeon but X *did* work on both. So maybe
that's not 100% repeatable. Hard to say... I've only managed to get it
to boot twice; mostly it oopses as shown above.

If I don't hack it to switch the mux to IGD at boot time, I never manage
to get a sane picture out of the Intel device after switching to it.
It's late now, but I'll try to get a proper debug log of the working and
failing cases tomorrow. 

-- 
dwmw2



[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 6171 bytes --]

Re: [PATCH] apple-gmux: Restore switch registers on suspend/resume

[David Woodhouse]

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

On Sun, 2012-07-29 at 01:42 +0100, David Woodhouse wrote:
> If I don't hack it to switch the mux to IGD at boot time, I never manage
> to get a sane picture out of the Intel device after switching to it.
> It's late now, but I'll try to get a proper debug log of the working and
> failing cases tomorrow. 

From the hacked kernel (after fixing vga_switcheroo_enable() not to do:
		event.info = client->fb_info;
 		fb_notifier_call_chain(FB_EVENT_REMAP_ALL_CONSOLE, &event);
if client->fb_info is NULL)...

http://david.woodhou.se/dmesg-boot-to-IGD (blank screen)
http://david.woodhou.se/dmesg-boot-to-IGD-and-switch-to-DIS (works)
http://david.woodhou.se/dmesg-boot-to-IGD-and-switch-to-DIS-and-back-to-IGD (works)

Without switching to IGD at startup...

http://david.woodhou.se/dmesg-boot-to-DIS (works)
http://david.woodhou.se/dmesg-boot-to-DIS-then-switch-to-IGD (blank)

-- 
dwmw2

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 6171 bytes --]

Re: [PATCH] apple-gmux: Restore switch registers on suspend/resume

[Andreas Heider]

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

Am 29.07.12 03:18, schrieb David Woodhouse:
> On Sun, 2012-07-29 at 01:42 +0100, David Woodhouse wrote:
>> If I don't hack it to switch the mux to IGD at boot time, I never manage
>> to get a sane picture out of the Intel device after switching to it.
>> It's late now, but I'll try to get a proper debug log of the working and
>> failing cases tomorrow.
>
>  From the hacked kernel (after fixing vga_switcheroo_enable() not to do:
> 		event.info = client->fb_info;
>   		fb_notifier_call_chain(FB_EVENT_REMAP_ALL_CONSOLE, &event);
> if client->fb_info is NULL)...
>
> http://david.woodhou.se/dmesg-boot-to-IGD (blank screen)
> http://david.woodhou.se/dmesg-boot-to-IGD-and-switch-to-DIS (works)
> http://david.woodhou.se/dmesg-boot-to-IGD-and-switch-to-DIS-and-back-to-IGD (works)
>
> Without switching to IGD at startup...
>
> http://david.woodhou.se/dmesg-boot-to-DIS (works)
> http://david.woodhou.se/dmesg-boot-to-DIS-then-switch-to-IGD (blank)
>

The general problem, at least for the case of booting to DIS and then 
switching to IGD, is that the mode for the internal display isn't 
correct, so it stays black.

It could get the right mode via DDC but this line is muxed as well. 
0x728 controls the DDC mux, writing 1 lets the IGD access it, 2 is the DIS.

So what happens in dmesg-boot-to-DIS is that the intel card can't get 
the mode via DDC and falls back to the garbage mode from VBT.
Interestingly, there is a intel VBT on your MBP8,3, but it doesn't 
contain the right modes. On my MBP6,2 there isn't a VBT at all and it 
disables LVDS altogether if 0x728 isn't switched at boot.

Manually switching the mux is a bit messy, so I'm not too surprised that 
it's blank in dmesg-boot-to-IGD.

The gmux code you're using is a bit outdated, but if you use 
git://kernel.ubuntu.com/sforshee/linux.git gmux-switcheroo and connect 
an external display (I tested it with a DP one) you should be able to 
boot to DIS and switch to the IGD and get output on the external display.

This leaves the problem of how to get i915 the right mode. Generally, 
there are two options:

- Find a way so it gets the right mode when it initializes the LVDS. 
This could be done by switching the DDC mux during lvds intialization. 
See lock_ddc.patch for a crude prototype that works on my laptop if I'm 
lucky and apple_gmux gets loaded before i915 and nouveau doesn't try to 
use DDC at the same time.
Another way would be to get the right EDID without mux switching by 
querying either the DIS driver or EFI (if you boot into the rEFIt shell 
you can verify it's there, see http://andreas.meetr.de/efi/log/edid.txt, 
to get it search for EdidDiscovered/EdidActive and dump it with dmem). 
But to use this, we need a way to know which display this corresponds to 
and I'm not sure how to do that.

- Make vga_switcheroo reprobe handle this. This would mean that we start 
with a possibly broken mode and resize it once the mux get's switched 
through vga_switcheroo. I'm not sure if thats a good idea.

I'd love to get any input on this, this is the main problem that's 
keeping apple_gmux switching from working really well.

Andreas

[-- Attachment #2: lock_ddc.patch --]
[-- Type: text/plain, Size: 3281 bytes --]

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a8538ac..2f0428d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -31,6 +31,7 @@
 #include <linux/kernel.h>
 #include <linux/slab.h>
 #include <linux/vgaarb.h>
+#include <linux/vga_switcheroo.h>
 #include <drm/drm_edid.h>
 #include "drmP.h"
 #include "intel_drv.h"
@@ -6506,7 +6507,11 @@ static void intel_setup_outputs(struct drm_device *dev)
 	bool dpd_is_edp = false;
 	bool has_lvds;
 
+	/* Switch mux so lvds is detectable */
+	vga_switcheroo_lock_ddc(dev->pdev);
 	has_lvds = intel_lvds_init(dev);
+	vga_switcheroo_unlock_ddc(dev->pdev);
+
 	if (!has_lvds && !HAS_PCH_SPLIT(dev)) {
 		/* disable the panel fitter on everything but LVDS */
 		I915_WRITE(PFIT_CONTROL, 0);
diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c
index 5b3c7d1..ee137c3 100644
--- a/drivers/gpu/vga/vga_switcheroo.c
+++ b/drivers/gpu/vga/vga_switcheroo.c
@@ -41,6 +41,7 @@ struct vga_switcheroo_client {
 };
 
 static DEFINE_MUTEX(vgasr_mutex);
+static DEFINE_MUTEX(vgasr_ddc_mutex);
 
 struct vgasr_priv {
 
@@ -553,3 +554,45 @@ err:
 }
 EXPORT_SYMBOL(vga_switcheroo_process_delayed_switch);
 
+int vga_switcheroo_lock_ddc(struct pci_dev *pdev)
+{
+
+	printk(KERN_INFO "vga_switcheroo: ddc lock\n");
+
+	mutex_lock(&vgasr_mutex);
+
+	if (vgasr_priv.handler && vgasr_priv.handler->switchddc && 
+			vgasr_priv.handler->get_client_id) {
+		int client_id = vgasr_priv.handler->get_client_id(pdev);
+		printk(KERN_INFO "vga_switcheroo: ddc lock got handler\n");
+		mutex_unlock(&vgasr_mutex);
+		mutex_lock(&vgasr_ddc_mutex);
+		return vgasr_priv.handler->switchddc(client_id);
+	} else {
+		printk(KERN_INFO "vga_switcheroo: ddc lock no handler\n");
+	}
+
+	mutex_unlock(&vgasr_mutex);
+	return 0;
+}
+EXPORT_SYMBOL(vga_switcheroo_lock_ddc);
+
+void vga_switcheroo_unlock_ddc(struct pci_dev *pdev)
+{
+	static struct vga_switcheroo_client *active_client;
+
+	printk(KERN_INFO "vga_switcheroo: ddc unlock\n");
+
+	mutex_lock(&vgasr_mutex);
+	active_client = find_active_client(&vgasr_priv.clients);
+
+	if (vgasr_priv.handler && vgasr_priv.handler->switchddc &&
+			active_client)
+		vgasr_priv.handler->switchddc(active_client->id);
+	else 
+		printk(KERN_INFO "vga_switcheroo: ddc unlock no handler\n");
+
+	mutex_unlock(&vgasr_mutex);
+	mutex_unlock(&vgasr_ddc_mutex);
+}
+EXPORT_SYMBOL(vga_switcheroo_unlock_ddc);
diff --git a/include/linux/vga_switcheroo.h b/include/linux/vga_switcheroo.h
index ddb419c..4538df6 100644
--- a/include/linux/vga_switcheroo.h
+++ b/include/linux/vga_switcheroo.h
@@ -30,6 +30,7 @@ enum vga_switcheroo_client_id {
 
 struct vga_switcheroo_handler {
 	int (*switchto)(enum vga_switcheroo_client_id id);
+	int (*switchddc)(enum vga_switcheroo_client_id id);
 	int (*power_state)(enum vga_switcheroo_client_id id,
 			   enum vga_switcheroo_state state);
 	int (*init)(void);
@@ -60,6 +61,9 @@ int vga_switcheroo_process_delayed_switch(void);
 
 int vga_switcheroo_get_client_state(struct pci_dev *dev);
 
+int vga_switcheroo_lock_ddc(struct pci_dev *dev);
+void vga_switcheroo_unlock_ddc(struct pci_dev *dev);
+
 #else
 
 static inline void vga_switcheroo_unregister_client(struct pci_dev *dev) {}

Re: [PATCH] apple-gmux: Restore switch registers on suspend/resume

[David Woodhouse]

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

On Sun, 2012-07-29 at 09:46 +0200, Andreas Heider wrote:
> Am 29.07.12 03:18, schrieb David Woodhouse:
> > On Sun, 2012-07-29 at 01:42 +0100, David Woodhouse wrote:
> >> If I don't hack it to switch the mux to IGD at boot time, I never manage
> >> to get a sane picture out of the Intel device after switching to it.
> >> It's late now, but I'll try to get a proper debug log of the working and
> >> failing cases tomorrow.
> >
> >  From the hacked kernel (after fixing vga_switcheroo_enable() not to do:
> > 		event.info = client->fb_info;
> >   		fb_notifier_call_chain(FB_EVENT_REMAP_ALL_CONSOLE, &event);
> > if client->fb_info is NULL)...
> >
> > http://david.woodhou.se/dmesg-boot-to-IGD (blank screen)
> > http://david.woodhou.se/dmesg-boot-to-IGD-and-switch-to-DIS (works)
> > http://david.woodhou.se/dmesg-boot-to-IGD-and-switch-to-DIS-and-back-to-IGD (works)
> >
> > Without switching to IGD at startup...
> >
> > http://david.woodhou.se/dmesg-boot-to-DIS (works)
> > http://david.woodhou.se/dmesg-boot-to-DIS-then-switch-to-IGD (blank)
> >
> 
> The general problem, at least for the case of booting to DIS and then 
> switching to IGD, is that the mode for the internal display isn't 
> correct, so it stays black.
> 
> It could get the right mode via DDC but this line is muxed as well. 
> 0x728 controls the DDC mux, writing 1 lets the IGD access it, 2 is the DIS.

So? We *switch* it before telling the IGD to enable its display (hotplug
the LVDS). So the DDC *should* be working, surely? Is it just that the
Intel driver is probing the LVDS once at boot when the panel isn't even
connected, and it should be made hotplug-capable?

The Radeon seems to cope... maybe. I note it doesn't actually come up in
1920x1200 with fbcon, although ISTR X was getting the right resolution.

> So what happens in dmesg-boot-to-DIS is that the intel card can't get 
> the mode via DDC and falls back to the garbage mode from VBT.
> Interestingly, there is a intel VBT on your MBP8,3, but it doesn't 
> contain the right modes. On my MBP6,2 there isn't a VBT at all and it 
> disables LVDS altogether if 0x728 isn't switched at boot.
> 
> Manually switching the mux is a bit messy, so I'm not too surprised that 
> it's blank in dmesg-boot-to-IGD.

That does actually work, if I disable the Radeon completely. With this
in early boot:
+       outb(1, 0x728);
+       outb(2, 0x710);
+       outb(2, 0x740);
+       outb(0, 0x750);
... the Intel framebuffer then comes up just fine on the internal
display. It *can't* drive external displays in that case (via the
standard Apple DP-VGA or DP-DVI adapters; I haven't tried real DP), but
the LVDS is fine.

> The gmux code you're using is a bit outdated, but if you use 
> git://kernel.ubuntu.com/sforshee/linux.git gmux-switcheroo and connect 
> an external display (I tested it with a DP one) you should be able to 
> boot to DIS and switch to the IGD and get output on the external display.
> 
> This leaves the problem of how to get i915 the right mode. Generally, 
> there are two options:
> 
> - Find a way so it gets the right mode when it initializes the LVDS. 
> This could be done by switching the DDC mux during lvds intialization. 
> See lock_ddc.patch for a crude prototype that works on my laptop if I'm 
> lucky and apple_gmux gets loaded before i915 and nouveau doesn't try to 
> use DDC at the same time.

Again, why is the LVDS initialisation happening when the LVDS isn't
connected anyway? Fix that, and your concerns become irrelevant. You
*can't* switch to IGD before apple_gmux is loaded, and the discrete card
shouldn't be poking it DDC when the panel has been hot-unplugged from it
either.

> Another way would be to get the right EDID without mux switching by 
> querying either the DIS driver or EFI (if you boot into the rEFIt shell 
> you can verify it's there, see http://andreas.meetr.de/efi/log/edid.txt, 
> to get it search for EdidDiscovered/EdidActive and dump it with dmem). 
> But to use this, we need a way to know which display this corresponds to 
> and I'm not sure how to do that.
> 
> - Make vga_switcheroo reprobe handle this. This would mean that we start 
> with a possibly broken mode and resize it once the mux get's switched 
> through vga_switcheroo. I'm not sure if thats a good idea.

"Possibly broken mode", in the case where there is no monitor attached
to the display. I think we can live with that. Surely it's the case for
an external monitor (which we also support) anyway?

In fact speaking of external monitors... it looks like it should be
possible to support using one graphics cards for the LVDS, and another
for the external DisplayPort. Perhaps they could be different seats of a
multi-seat system, with completely separate X servers, or maybe we'd do
it just to work around size/bandwidth limitations or for performance
reasons. Do we have any plan to allow that? We only switch both together
for now...

-- 
dwmw2

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 6171 bytes --]

Re: [PATCH] apple-gmux: Restore switch registers on suspend/resume

[Andreas Heider]

Am 29.07.12 21:05, schrieb David Woodhouse:
> On Sun, 2012-07-29 at 09:46 +0200, Andreas Heider wrote:
>> Am 29.07.12 03:18, schrieb David Woodhouse:
>>> On Sun, 2012-07-29 at 01:42 +0100, David Woodhouse wrote:
>>>> If I don't hack it to switch the mux to IGD at boot time, I never manage
>>>> to get a sane picture out of the Intel device after switching to it.
>>>> It's late now, but I'll try to get a proper debug log of the working and
>>>> failing cases tomorrow.
>>>
>>>   From the hacked kernel (after fixing vga_switcheroo_enable() not to do:
>>> 		event.info = client->fb_info;
>>>    		fb_notifier_call_chain(FB_EVENT_REMAP_ALL_CONSOLE, &event);
>>> if client->fb_info is NULL)...
>>>
>>> http://david.woodhou.se/dmesg-boot-to-IGD (blank screen)
>>> http://david.woodhou.se/dmesg-boot-to-IGD-and-switch-to-DIS (works)
>>> http://david.woodhou.se/dmesg-boot-to-IGD-and-switch-to-DIS-and-back-to-IGD (works)
>>>
>>> Without switching to IGD at startup...
>>>
>>> http://david.woodhou.se/dmesg-boot-to-DIS (works)
>>> http://david.woodhou.se/dmesg-boot-to-DIS-then-switch-to-IGD (blank)
>>>
>>
>> The general problem, at least for the case of booting to DIS and then
>> switching to IGD, is that the mode for the internal display isn't
>> correct, so it stays black.
>>
>> It could get the right mode via DDC but this line is muxed as well.
>> 0x728 controls the DDC mux, writing 1 lets the IGD access it, 2 is the DIS.
>
> So? We *switch* it before telling the IGD to enable its display (hotplug
> the LVDS). So the DDC *should* be working, surely? Is it just that the
> Intel driver is probing the LVDS once at boot when the panel isn't even
> connected, and it should be made hotplug-capable?

That would work and I like this solution the best, but I'm not familiar 
enough with i915 to comment on how feasible it is or implement it. It 
should even be possible to switch the DDC mux first, let i915 do the 
detection and switch the display later to avoid long black screens.

>
> The Radeon seems to cope... maybe. I note it doesn't actually come up in
> 1920x1200 with fbcon, although ISTR X was getting the right resolution.
>
>> So what happens in dmesg-boot-to-DIS is that the intel card can't get
>> the mode via DDC and falls back to the garbage mode from VBT.
>> Interestingly, there is a intel VBT on your MBP8,3, but it doesn't
>> contain the right modes. On my MBP6,2 there isn't a VBT at all and it
>> disables LVDS altogether if 0x728 isn't switched at boot.
>>
>> Manually switching the mux is a bit messy, so I'm not too surprised that
>> it's blank in dmesg-boot-to-IGD.
>
> That does actually work, if I disable the Radeon completely. With this
> in early boot:
> +       outb(1, 0x728);
> +       outb(2, 0x710);
> +       outb(2, 0x740);
> +       outb(0, 0x750);
> ... the Intel framebuffer then comes up just fine on the internal
> display. It *can't* drive external displays in that case (via the
> standard Apple DP-VGA or DP-DVI adapters; I haven't tried real DP), but
> the LVDS is fine.
>

Not sure about it, but it might be because vga_default_device is wrong 
if you switch the mux manually.

>> The gmux code you're using is a bit outdated, but if you use
>> git://kernel.ubuntu.com/sforshee/linux.git gmux-switcheroo and connect
>> an external display (I tested it with a DP one) you should be able to
>> boot to DIS and switch to the IGD and get output on the external display.
>>
>> This leaves the problem of how to get i915 the right mode. Generally,
>> there are two options:
>>
>> - Find a way so it gets the right mode when it initializes the LVDS.
>> This could be done by switching the DDC mux during lvds intialization.
>> See lock_ddc.patch for a crude prototype that works on my laptop if I'm
>> lucky and apple_gmux gets loaded before i915 and nouveau doesn't try to
>> use DDC at the same time.
>
> Again, why is the LVDS initialisation happening when the LVDS isn't
> connected anyway? Fix that, and your concerns become irrelevant. You
> *can't* switch to IGD before apple_gmux is loaded, and the discrete card
> shouldn't be poking it DDC when the panel has been hot-unplugged from it
> either.
>
>> Another way would be to get the right EDID without mux switching by
>> querying either the DIS driver or EFI (if you boot into the rEFIt shell
>> you can verify it's there, see http://andreas.meetr.de/efi/log/edid.txt,
>> to get it search for EdidDiscovered/EdidActive and dump it with dmem).
>> But to use this, we need a way to know which display this corresponds to
>> and I'm not sure how to do that.
>>
>> - Make vga_switcheroo reprobe handle this. This would mean that we start
>> with a possibly broken mode and resize it once the mux get's switched
>> through vga_switcheroo. I'm not sure if thats a good idea.
>
> "Possibly broken mode", in the case where there is no monitor attached
> to the display. I think we can live with that. Surely it's the case for
> an external monitor (which we also support) anyway?
>
> In fact speaking of external monitors... it looks like it should be
> possible to support using one graphics cards for the LVDS, and another
> for the external DisplayPort. Perhaps they could be different seats of a
> multi-seat system, with completely separate X servers, or maybe we'd do
> it just to work around size/bandwidth limitations or for performance
> reasons. Do we have any plan to allow that? We only switch both together
> for now...
>

Just as a sidenote, I tested running two X server and it does work as 
expected.

Re: [PATCH] apple-gmux: Restore switch registers on suspend/resume

[Matthew Garrett]

Working: Modeline 10:"1920x1200" 60 
154000 1920 1968 2000 2080 1200 1203 1209 1235 0x48 0xa
Broken: Modeline 20:"1280x800" 60 72500 1280 1328 1360 1423 800 803 809 
846 0x8 0xa

And it looks like intel_lvds->edid is only set during intel_lvds_init(). 
That seems less than ideal. How about something like this entirely 
untested patch?

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 36822b9..adef587 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1250,6 +1250,7 @@ static void i915_switcheroo_set_state(struct pci_dev *pdev, enum vga_switcheroo_
 		dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
 		/* i915 resume handler doesn't set to D0 */
 		pci_set_power_state(dev->pdev, PCI_D0);
+		intel_lvds_get_edid(dev);
 		i915_resume(dev);
 		dev->switch_power_state = DRM_SWITCH_POWER_ON;
 	} else {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 3e09188..930542a 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -353,6 +353,7 @@ extern void intel_dvo_init(struct drm_device *dev);
 extern void intel_tv_init(struct drm_device *dev);
 extern void intel_mark_busy(struct drm_device *dev,
 			    struct drm_i915_gem_object *obj);
+extern bool intel_lvds_get_edid(struct drm_device *dev);
 extern bool intel_lvds_init(struct drm_device *dev);
 extern void intel_dp_init(struct drm_device *dev, int dp_reg);
 void
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 08eb04c..80832e5 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -894,6 +894,59 @@ static bool intel_lvds_supported(struct drm_device *dev)
 	return IS_MOBILE(dev) && !IS_I830(dev);
 }
 
+bool intel_lvds_get_edid(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_connector *connector = dev_priv->int_lvds_connector;
+	struct intel_lvds *intel_lvds;
+	struct drm_display_mode *scan; /* *modes, *bios_mode; */
+	u8 pin;	
+
+	if (!connector)
+		return false;
+
+	intel_lvds = intel_attached_lvds(connector);
+
+	pin = GMBUS_PORT_PANEL;
+
+	intel_lvds->edid = drm_get_edid(connector,
+					intel_gmbus_get_adapter(dev_priv,
+								pin));
+	if (intel_lvds->edid) {
+		if (drm_add_edid_modes(connector,
+				       intel_lvds->edid)) {
+			drm_mode_connector_update_edid_property(connector,
+								intel_lvds->edid);
+		} else {
+			kfree(intel_lvds->edid);
+			intel_lvds->edid = NULL;
+		}
+	}
+
+	if (!intel_lvds->edid) {
+		/* Didn't get an EDID, so
+		 * Set wide sync ranges so we get all modes
+		 * handed to valid_mode for checking
+		 */
+		connector->display_info.min_vfreq = 0;
+		connector->display_info.max_vfreq = 200;
+		connector->display_info.min_hfreq = 0;
+		connector->display_info.max_hfreq = 200;
+	}
+
+	list_for_each_entry(scan, &connector->probed_modes, head) {
+		if (scan->type & DRM_MODE_TYPE_PREFERRED) {
+			intel_lvds->fixed_mode =
+				drm_mode_duplicate(dev, scan);
+			intel_find_lvds_downclock(dev,
+						  intel_lvds->fixed_mode,
+						  connector);
+			return true;
+		}
+	}
+	return false;
+}
+
 /**
  * intel_lvds_init - setup LVDS connectors on this device
  * @dev: drm device
@@ -909,7 +962,6 @@ bool intel_lvds_init(struct drm_device *dev)
 	struct intel_connector *intel_connector;
 	struct drm_connector *connector;
 	struct drm_encoder *encoder;
-	struct drm_display_mode *scan; /* *modes, *bios_mode; */
 	struct drm_crtc *crtc;
 	u32 lvds;
 	int pipe;
@@ -976,6 +1028,8 @@ bool intel_lvds_init(struct drm_device *dev)
 	connector->interlace_allowed = false;
 	connector->doublescan_allowed = false;
 
+	dev_priv->int_lvds_connector = connector;
+
 	/* create the scaling mode property */
 	drm_mode_create_scaling_mode_property(dev);
 	/*
@@ -1000,40 +1054,8 @@ bool intel_lvds_init(struct drm_device *dev)
 	 * Attempt to get the fixed panel mode from DDC.  Assume that the
 	 * preferred mode is the right one.
 	 */
-	intel_lvds->edid = drm_get_edid(connector,
-					intel_gmbus_get_adapter(dev_priv,
-								pin));
-	if (intel_lvds->edid) {
-		if (drm_add_edid_modes(connector,
-				       intel_lvds->edid)) {
-			drm_mode_connector_update_edid_property(connector,
-								intel_lvds->edid);
-		} else {
-			kfree(intel_lvds->edid);
-			intel_lvds->edid = NULL;
-		}
-	}
-	if (!intel_lvds->edid) {
-		/* Didn't get an EDID, so
-		 * Set wide sync ranges so we get all modes
-		 * handed to valid_mode for checking
-		 */
-		connector->display_info.min_vfreq = 0;
-		connector->display_info.max_vfreq = 200;
-		connector->display_info.min_hfreq = 0;
-		connector->display_info.max_hfreq = 200;
-	}
-
-	list_for_each_entry(scan, &connector->probed_modes, head) {
-		if (scan->type & DRM_MODE_TYPE_PREFERRED) {
-			intel_lvds->fixed_mode =
-				drm_mode_duplicate(dev, scan);
-			intel_find_lvds_downclock(dev,
-						  intel_lvds->fixed_mode,
-						  connector);
-			goto out;
-		}
-	}
+	if (intel_lvds_get_edid(dev))
+		goto out;
 
 	/* Failed to get EDID, what about VBT? */
 	if (dev_priv->lfp_lvds_vbt_mode) {
@@ -1112,7 +1134,6 @@ out:
 		dev_priv->lid_notifier.notifier_call = NULL;
 	}
 	/* keep the LVDS connector */
-	dev_priv->int_lvds_connector = connector;
 	drm_sysfs_connector_add(connector);
 
 	intel_panel_setup_backlight(dev);
@@ -1121,6 +1142,7 @@ out:
 
 failed:
 	DRM_DEBUG_KMS("No LVDS modes found, disabling.\n");
+	dev_priv->int_lvds_connector = NULL;
 	drm_connector_cleanup(connector);
 	drm_encoder_cleanup(encoder);
 	kfree(intel_lvds);


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

Re: [PATCH] apple-gmux: Restore switch registers on suspend/resume

[David Woodhouse]

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

On Sun, 2012-07-29 at 20:39 +0100, Matthew Garrett wrote:
> 
> And it looks like intel_lvds->edid is only set during
> intel_lvds_init(). 
> That seems less than ideal. How about something like this entirely 
> untested patch?

Not at first attempt. Will poke a little more at it.

http://david.woodhou.se/dmesg2-DIS-to-IGD

-- 
dwmw2

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 6171 bytes --]

Re: [PATCH] apple-gmux: Restore switch registers on suspend/resume

[David Woodhouse]

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

On Sun, 2012-07-29 at 20:39 +0100, Matthew Garrett wrote:
> And it looks like intel_lvds->edid is only set during intel_lvds_init(). 
> That seems less than ideal. How about something like this entirely 
> untested patch? 

Actually, it works if I write 'MIGD' first and then 'IGD'. Looks like we
aren't switching the DDC over to the new gfx card soon enough in the
process?

-- 
dwmw2

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 6171 bytes --]

Re: [PATCH] apple-gmux: Restore switch registers on suspend/resume

[Matthew Garrett]

On Sun, Jul 29, 2012 at 09:52:51PM +0100, David Woodhouse wrote:
> On Sun, 2012-07-29 at 20:39 +0100, Matthew Garrett wrote:
> > And it looks like intel_lvds->edid is only set during intel_lvds_init(). 
> > That seems less than ideal. How about something like this entirely 
> > untested patch? 
> 
> Actually, it works if I write 'MIGD' first and then 'IGD'. Looks like we
> aren't switching the DDC over to the new gfx card soon enough in the
> process?

Yes, the call to the switcheroo code only comes after the card is 
powered on. Cc:ing Dave Airlie - what are the expectations here? It 
looks like i915 should have a reprobe function.

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

Re: [PATCH] apple-gmux: Restore switch registers on suspend/resume

[David Woodhouse]

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

On Sun, 2012-07-29 at 09:46 +0200, Andreas Heider wrote:
> The gmux code you're using is a bit outdated, but if you use 
> git://kernel.ubuntu.com/sforshee/linux.git gmux-switcheroo and connect 
> an external display (I tested it with a DP one) you should be able to 
> boot to DIS and switch to the IGD and get output on the external display. 

Matthew's version has changes to the core vga_switcheroo code too, to
add the ->client_active() method which lets it pick the right client at
init time. I'll let you work out how to merge the two, but in the
meantime here's what I needed to do to Matthew's one to make it work...

diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c
index 321d17c..e2149e6 100644
--- a/drivers/gpu/vga/vga_switcheroo.c
+++ b/drivers/gpu/vga/vga_switcheroo.c
@@ -108,6 +108,7 @@ static void vga_switcheroo_enable(void)
 		ret = vgasr_priv.handler->get_client_id(client->pdev);
 		if (ret < 0)
 			return;
+		client->id = ret;
 
 		if (vgasr_priv.handler->client_active) {
 			active = vgasr_priv.handler->client_active(client->id);
@@ -117,7 +118,7 @@ static void vga_switcheroo_enable(void)
 			 * but the client itself doesn't, update state
 			 */
 
-			if (active && !client->active) {
+			if (active && !client->active && client->fb_info) {
 				struct fb_event event;
 				event.info = client->fb_info;
 				fb_notifier_call_chain(FB_EVENT_REMAP_ALL_CONSOLE, &event);
@@ -125,8 +126,6 @@ static void vga_switcheroo_enable(void)
 
 			client->active = active;
 		}
-
-		client->id = ret;
 	}
 	vga_switcheroo_debugfs_init(&vgasr_priv);
 	vgasr_priv.active = true;


-- 
dwmw2

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 6171 bytes --]

Re: [PATCH] apple-gmux: Restore switch registers on suspend/resume

[Seth Forshee]

On Sun, Jul 29, 2012 at 11:44:49PM +0100, David Woodhouse wrote:
> On Sun, 2012-07-29 at 09:46 +0200, Andreas Heider wrote:
> > The gmux code you're using is a bit outdated, but if you use 
> > git://kernel.ubuntu.com/sforshee/linux.git gmux-switcheroo and connect 
> > an external display (I tested it with a DP one) you should be able to 
> > boot to DIS and switch to the IGD and get output on the external display. 
> 
> Matthew's version has changes to the core vga_switcheroo code too, to
> add the ->client_active() method which lets it pick the right client at
> init time. I'll let you work out how to merge the two, but in the
> meantime here's what I needed to do to Matthew's one to make it work...

But that code is only needed if you've hacked in the port writes to
switch to IGD in the early boot code, right? It's fine as a local hack
when trying to get things working, but I don't see it as something
that's going to be mainlined. Putting togethter something we can
mainline is my goal with the patches I've got queued up in the
referenced repository.

Fwiw on the Macbook Pro 8,2, using my tree with the patches from
http://www.codon.org.uk/~mjg59/tmp/radeon_efi/ and no port writes hacked
into early boot, the active client as seen by vga_switcheroo and
apple_gmux are the same.

Seth

Re: [PATCH] apple-gmux: Restore switch registers on suspend/resume

[Seth Forshee]

On Sun, Jul 29, 2012 at 09:59:00PM +0100, Matthew Garrett wrote:
> On Sun, Jul 29, 2012 at 09:52:51PM +0100, David Woodhouse wrote:
> > On Sun, 2012-07-29 at 20:39 +0100, Matthew Garrett wrote:
> > > And it looks like intel_lvds->edid is only set during intel_lvds_init(). 
> > > That seems less than ideal. How about something like this entirely 
> > > untested patch? 
> > 
> > Actually, it works if I write 'MIGD' first and then 'IGD'. Looks like we
> > aren't switching the DDC over to the new gfx card soon enough in the
> > process?
> 
> Yes, the call to the switcheroo code only comes after the card is 
> powered on. Cc:ing Dave Airlie - what are the expectations here? It 
> looks like i915 should have a reprobe function.

I dove into this yesterday and ended up with something similar to what
you had. I'm rescanning for the EDID from the reprobe callback, and I
also made it so that intel_lvds_init() will still keep the LVDS
connector even if the panel isn't attached at boot and
intel_lvds_detect() returns connected/disconnected based on whether or
not we found an EDID for the LVDS.

All of this is working to the extent that I can boot with the Radeon
card active, switch over to the Intel card, and get the EDID for the
internal panel and an external monitor (although oddly on an HDMI
connector, no on the DP like I expected). Both screens are remaining
blank though. However I'm also getting blank screens if I mux over to
the Intel GPU from grub before loading the kernel, which used to work
for the LVDS panel at least.

I've copied the whole series of patches I'm running on top of 3.5 to
http://people.canonical.com/~sforshee/apple-gmux-patches if anyone else
wants to give them a try. The i915 changes are in patches 13-17. I've
also pushed the i915 patches to my gmux-switcheroo branch.

Seth

Re: [PATCH] apple-gmux: Restore switch registers on suspend/resume

[Seth Forshee]

On Tue, Jul 31, 2012 at 10:18:56AM -0500, Seth Forshee wrote:
> On Sun, Jul 29, 2012 at 09:59:00PM +0100, Matthew Garrett wrote:
> > On Sun, Jul 29, 2012 at 09:52:51PM +0100, David Woodhouse wrote:
> > > On Sun, 2012-07-29 at 20:39 +0100, Matthew Garrett wrote:
> > > > And it looks like intel_lvds->edid is only set during intel_lvds_init(). 
> > > > That seems less than ideal. How about something like this entirely 
> > > > untested patch? 
> > > 
> > > Actually, it works if I write 'MIGD' first and then 'IGD'. Looks like we
> > > aren't switching the DDC over to the new gfx card soon enough in the
> > > process?
> > 
> > Yes, the call to the switcheroo code only comes after the card is 
> > powered on. Cc:ing Dave Airlie - what are the expectations here? It 
> > looks like i915 should have a reprobe function.
> 
> I dove into this yesterday and ended up with something similar to what
> you had. I'm rescanning for the EDID from the reprobe callback, and I
> also made it so that intel_lvds_init() will still keep the LVDS
> connector even if the panel isn't attached at boot and
> intel_lvds_detect() returns connected/disconnected based on whether or
> not we found an EDID for the LVDS.
> 
> All of this is working to the extent that I can boot with the Radeon
> card active, switch over to the Intel card, and get the EDID for the
> internal panel and an external monitor (although oddly on an HDMI
> connector, no on the DP like I expected). Both screens are remaining
> blank though. However I'm also getting blank screens if I mux over to
> the Intel GPU from grub before loading the kernel, which used to work
> for the LVDS panel at least.
> 
> I've copied the whole series of patches I'm running on top of 3.5 to
> http://people.canonical.com/~sforshee/apple-gmux-patches if anyone else
> wants to give them a try. The i915 changes are in patches 13-17. I've
> also pushed the i915 patches to my gmux-switcheroo branch.

Aha! I forgot to add i915.lvds_use_ssc=0 to the kernel parameters. So
with that and the above patches I can boot to the DGPU and successfully
switch between it and the integrated GPU. My VT console isn't showing up
with the IGPU, but the desktop does shows up.

Re: [PATCH] apple-gmux: Restore switch registers on suspend/resume

[David Woodhouse]

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

On Tue, 2012-07-31 at 10:18 -0500, Seth Forshee wrote:
> 
> All of this is working to the extent that I can boot with the Radeon
> card active, switch over to the Intel card, and get the EDID for the
> internal panel and an external monitor (although oddly on an HDMI
> connector, no on the DP like I expected). Both screens are remaining
> blank though. However I'm also getting blank screens if I mux over to
> the Intel GPU from grub before loading the kernel, which used to work
> for the LVDS panel at least. 

Hm, when I was changing over to the external screen before boot, I could
get the LVDS working but *not* an external VGA or DVI monitor (via the
standard Apple adaptors). Should I expect that to work if I change the
mux 'properly' at runtime?

-- 
dwmw2

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 6171 bytes --]

Re: [PATCH] apple-gmux: Restore switch registers on suspend/resume

[Seth Forshee]

On Wed, Aug 01, 2012 at 04:35:44PM +0100, David Woodhouse wrote:
> On Tue, 2012-07-31 at 10:18 -0500, Seth Forshee wrote:
> > 
> > All of this is working to the extent that I can boot with the Radeon
> > card active, switch over to the Intel card, and get the EDID for the
> > internal panel and an external monitor (although oddly on an HDMI
> > connector, no on the DP like I expected). Both screens are remaining
> > blank though. However I'm also getting blank screens if I mux over to
> > the Intel GPU from grub before loading the kernel, which used to work
> > for the LVDS panel at least. 
> 
> Hm, when I was changing over to the external screen before boot, I could
> get the LVDS working but *not* an external VGA or DVI monitor (via the
> standard Apple adaptors). Should I expect that to work if I change the
> mux 'properly' at runtime?

I don't think the mini-DP port works at all with the integrated
graphics, at least not on a Macbook Pro 8,2. I played around with it
yesterday under OS X. When using the DGPU with an external monitor it
works fine, but any attempts to switch to the IGPU were rejected by the
drivers. If I forced it to the IGPU prior to connecting the external
monitor the screen just remained blank when I plugged it in, until I
switched back to the DGPU. It's odd though that the DDC can be switched
over to the IGPU.

Iirc you've got the 8,3, and I'd expect it to be identical to the 8,2 in
this respect. Also, the only reason my LVDS didn't work was because I
wasn't telling i915 to disable SSC. I've got a patch to add a quirk for
this that I'll send soon, since there still doesn't seem to be any way
to get the vbios for the Intel graphics.

Seth

Re: [PATCH] apple-gmux: Restore switch registers on suspend/resume

[Andreas Heider]

Am 01.08.12 17:59, schrieb Seth Forshee:
> On Wed, Aug 01, 2012 at 04:35:44PM +0100, David Woodhouse wrote:
>> On Tue, 2012-07-31 at 10:18 -0500, Seth Forshee wrote:
>>>
>>> All of this is working to the extent that I can boot with the Radeon
>>> card active, switch over to the Intel card, and get the EDID for the
>>> internal panel and an external monitor (although oddly on an HDMI
>>> connector, no on the DP like I expected). Both screens are remaining
>>> blank though. However I'm also getting blank screens if I mux over to
>>> the Intel GPU from grub before loading the kernel, which used to work
>>> for the LVDS panel at least.
>>
>> Hm, when I was changing over to the external screen before boot, I could
>> get the LVDS working but *not* an external VGA or DVI monitor (via the
>> standard Apple adaptors). Should I expect that to work if I change the
>> mux 'properly' at runtime?
>
> I don't think the mini-DP port works at all with the integrated
> graphics, at least not on a Macbook Pro 8,2. I played around with it
> yesterday under OS X. When using the DGPU with an external monitor it
> works fine, but any attempts to switch to the IGPU were rejected by the
> drivers. If I forced it to the IGPU prior to connecting the external
> monitor the screen just remained blank when I plugged it in, until I
> switched back to the DGPU. It's odd though that the DDC can be switched
> over to the IGPU.

The situation is a bit different on the Macbook Pro 6,2. OS X also 
automatically switches to the DGPU when an external monitor is 
connected, but unlike the 8,2, the IGPU works fine under Linux with 
external monitors if port 0x740 is set accordingly. I'm pretty sure that 
this is also the case with for the older macbooks without dynamic switching.

> Iirc you've got the 8,3, and I'd expect it to be identical to the 8,2 in
> this respect. Also, the only reason my LVDS didn't work was because I
> wasn't telling i915 to disable SSC. I've got a patch to add a quirk for
> this that I'll send soon, since there still doesn't seem to be any way
> to get the vbios for the Intel graphics.
>
> Seth
>

Re: [PATCH] apple-gmux: Restore switch registers on suspend/resume

[David Woodhouse]

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

On Wed, 2012-08-01 at 10:59 -0500, Seth Forshee wrote:
> I don't think the mini-DP port works at all with the integrated
> graphics, at least not on a Macbook Pro 8,2. I played around with it
> yesterday under OS X. When using the DGPU with an external monitor it
> works fine, but any attempts to switch to the IGPU were rejected by the
> drivers. If I forced it to the IGPU prior to connecting the external
> monitor the screen just remained blank when I plugged it in, until I
> switched back to the DGPU. It's odd though that the DDC can be switched
> over to the IGPU.

I get a blank screen when I use the VGA adapter, and with the dual-link
DVI adapter I get a barely responsive machine continually printing 
[ 1076.439623] [drm:intel_dp_complete_link_train] *ERROR* failed to train DP, aborting
as described at https://bugzilla.redhat.com/show_bug.cgi?id=843779#c8
And a blank screen.

I thought someone had said they'd got it working with a native DP
monitor, rather than the converters.

> Iirc you've got the 8,3, and I'd expect it to be identical to the 8,2 in
> this respect. Also, the only reason my LVDS didn't work was because I
> wasn't telling i915 to disable SSC. I've got a patch to add a quirk for
> this that I'll send soon, since there still doesn't seem to be any way
> to get the vbios for the Intel graphics.

I think your patch won't cover the 8,3.

00:02.0 VGA compatible controller [0300]: Intel Corporation 2nd Generation Core Processor Family Integrated Graphics Controller [8086:0126] (rev 09) (prog-if 00 [VGA controller])
	Subsystem: Apple Computer Inc. Device [106b:00de]

-- 
dwmw2

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 6171 bytes --]

Re: [PATCH] apple-gmux: Restore switch registers on suspend/resume

[David Woodhouse]

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

On Wed, 2012-08-01 at 10:59 -0500, Seth Forshee wrote:
> since there still doesn't seem to be any way
> to get the vbios for the Intel graphics.

Hm, wait. With Matthew's patches to get the Radeon BIOS from EFI, there
*is* also a ROM image for the Intel device. Have we tried that?

-- 
dwmw2

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 6171 bytes --]

Re: [PATCH] apple-gmux: Restore switch registers on suspend/resume

[Matthew Garrett]

On Wed, Aug 01, 2012 at 08:43:58PM +0100, David Woodhouse wrote:
> On Wed, 2012-08-01 at 10:59 -0500, Seth Forshee wrote:
> > since there still doesn't seem to be any way
> > to get the vbios for the Intel graphics.
> 
> Hm, wait. With Matthew's patches to get the Radeon BIOS from EFI, there
> *is* also a ROM image for the Intel device. Have we tried that?

There is? Unexpected! The driver ought to be picking that up 
automatically...

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

Re: [PATCH] apple-gmux: Restore switch registers on suspend/resume

[Seth Forshee]

On Wed, Aug 01, 2012 at 08:41:42PM +0100, David Woodhouse wrote:
> On Wed, 2012-08-01 at 10:59 -0500, Seth Forshee wrote:
> > I don't think the mini-DP port works at all with the integrated
> > graphics, at least not on a Macbook Pro 8,2. I played around with it
> > yesterday under OS X. When using the DGPU with an external monitor it
> > works fine, but any attempts to switch to the IGPU were rejected by the
> > drivers. If I forced it to the IGPU prior to connecting the external
> > monitor the screen just remained blank when I plugged it in, until I
> > switched back to the DGPU. It's odd though that the DDC can be switched
> > over to the IGPU.
> 
> I get a blank screen when I use the VGA adapter, and with the dual-link
> DVI adapter I get a barely responsive machine continually printing 
> [ 1076.439623] [drm:intel_dp_complete_link_train] *ERROR* failed to train DP, aborting
> as described at https://bugzilla.redhat.com/show_bug.cgi?id=843779#c8
> And a blank screen.
> 
> I thought someone had said they'd got it working with a native DP
> monitor, rather than the converters.

Maybe, I don't have a native DP monitor.

> > Iirc you've got the 8,3, and I'd expect it to be identical to the 8,2 in
> > this respect. Also, the only reason my LVDS didn't work was because I
> > wasn't telling i915 to disable SSC. I've got a patch to add a quirk for
> > this that I'll send soon, since there still doesn't seem to be any way
> > to get the vbios for the Intel graphics.
> 
> I think your patch won't cover the 8,3.
> 
> 00:02.0 VGA compatible controller [0300]: Intel Corporation 2nd Generation Core Processor Family Integrated Graphics Controller [8086:0126] (rev 09) (prog-if 00 [VGA controller])
> 	Subsystem: Apple Computer Inc. Device [106b:00de]

Nope, the subsystem device id is different. But I can throw yours into
the patch.

Although Matthew did indicate earlier that he might have some ideas
about how to get the VBT, in which case the quirking wouldn't be
necessary.

Re: [PATCH] apple-gmux: Restore switch registers on suspend/resume

[Seth Forshee]

On Wed, Aug 01, 2012 at 08:43:58PM +0100, David Woodhouse wrote:
> On Wed, 2012-08-01 at 10:59 -0500, Seth Forshee wrote:
> > since there still doesn't seem to be any way
> > to get the vbios for the Intel graphics.
> 
> Hm, wait. With Matthew's patches to get the Radeon BIOS from EFI, there
> *is* also a ROM image for the Intel device. Have we tried that?

You get a ROM image? Because I'm still not getting one, and the opregion
VBT is invalid as well. I wonder if it's a difference in the machines or
in how you're booting it.

I've got the EFI stub enabled in my build, and I'm booting the kernel
directly from refit (no bootloader). I'm not doing any gmux writes to
switch to the IGPU when I boot. Are you doing something different?

Re: [PATCH] apple-gmux: Restore switch registers on suspend/resume

[Andreas Heider]

Am 01.08.12 21:43, schrieb David Woodhouse:
> On Wed, 2012-08-01 at 10:59 -0500, Seth Forshee wrote:
>> since there still doesn't seem to be any way
>> to get the vbios for the Intel graphics.
>
> Hm, wait. With Matthew's patches to get the Radeon BIOS from EFI, there
> *is* also a ROM image for the Intel device. Have we tried that?
>

If I understand your old dmesg 
(http://david.woodhou.se/dmesg-boot-to-DIS) correctly there is one, but 
it doesn't contain anything useful.

Also there isn't one on my MBP 6,2, see 
http://andreas.meetr.de/efi/log/dh111.txt