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

[Andreas Heider]

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
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) {}