Enable gpu switching on the MacBook Pro

PATCH Superseded Lukas Wunner 4 participants 19 messages Apr 21, 2015 – May 2, 2015

Subsystems (6)

DRM drivers DRM misc Intel i915 x86 platform drivers +2 more subsystems

lore

Share & tools

lore

Lukas Wunner

author reviewer

2015-04-21 11:38 UTC #0 lore raw

cover Enable gpu switching on the MacBook Pro

The following patchset extends vga_switcheroo and related drivers to enable gpu switching on the MacBook Pro.

On MBPs, the panel is not connected to one of the gpus, but to the gmux chip (the "handler" in vga_switcheroo speak). Initially the gmux is switched to the discrete gpu. The integrated i915 gpu is therefore unable to detect the LVDS (or eDP) connector on bootup and switching to the integrated gpu will lead to a black screen. The solution is to temporarily switch the DDC lines to the integrated gpu when reading the EDID, which the gmux is capable of. Lack of support for this in vga_switcheroo is pretty much the only obstacle to get gpu switching to work on MBPs.

Seth Forshee came up with a patchset in fall 2012 but only a portion of it got merged. Matthew Garrett developed this further in 2014:
http://www.codon.org.uk/~mjg59/tmp/retina_patches/
http://lists.freedesktop.org/archives/dri-devel/2014-June/060947.html

Matthew Garrett's patches weren't merged either: Dave Airlie responded that he had rewritten Seth Forshee's code in December 2012. Whereas Seth Forshee had used a single function vga_switcheroo_switch_ddc() with a mutex in drm_get_edid() to avoid race conditions, Dave Airlie had moved the mutex to vga_switcheroo.c and used *two* functions vga_switcheroo_lock_ddc() and vga_switcheroo_unlock_ddc():
http://lists.freedesktop.org/archives/dri-devel/2014-June/061629.html
http://cgit.freedesktop.org/~airlied/linux/log/?h=switchy-wip

Quoth Dave Airlie: "I can't figure out why I didn't like this, but I rewrote this way back to lock/unlock the ddc lines, bits are contained in this WIP mess. I think I'd prefer something like that otherwise the interface got really ugly."

I basically took Dave Airlies WIP code of December 2012, added all the missing pieces, applied one of Matthew Garrett's patches verbatim and rewrote two others.

This works on a MacBookPro9,1 with Intel HD4000 (Ivy Bridge) and Nvidia GT650M (NVE7 / GK107) plus a "classic" (non-indexed) gmux connected to a 1680x1050 panel with dual channel LVDS (applied on top of drm-next and using nouveau).

Dave Airlie's commit removes a portion of Seth Forshee's code and my commit in turn removes some of Dave Airlie's code. Because this makes reviewing difficult, I'm including below a diff which contains all commits squashed into one. Reviewers may want to look at that diff first instead of the individual commits.

One difficulty one needs to be aware of is that gmux may register after the i915 driver, making it impossible to switch the DDC lines upon i915 initialization. Seth Forshee solved this by deferring initialization of the graphics driver:
http://cgit.freedesktop.org/~airlied/linux/commit/?h=switchy-wip&id=f7acbeae3e997750c3d23a56ad1501d4aeb0d896

Matthew Garrett came up with a simpler approach, he added a callback reprobe_connectors() which gets invoked in vga_switcheroo_enable():
http://www.codon.org.uk/~mjg59/tmp/retina_patches/0014-vga_switcheroo-Add-support-for-reprobing-connectors.patch

The latter approach is further refined in the following patchset by invoking the callback as soon as gmux registers, which is earlier than in Matthew Garrett's patch since vga_switcheroo_enable() waits for registration of a second gpu. (It may never register if its driver is blacklisted or not installed.)

The following issues currently remain:

- Sometimes there's flickering when switching, so the user experience is not yet as smooth as on OS X. (On OS X there's never any flickering.)

- If gmux registers *before* the i915 driver, the latter will find the LVDS connector plus a disconnected VGA connector. If gmux registers *after* the i915 driver, the latter initially only finds the VGA connector. Upon reprobe it will find the LVDS connector and *another* (ghost) VGA connector. That's because I call intel_setup_outputs() which probes for all sorts of connectors. Matthew Garrett duplicated that function and reduced it to just the eDP probing portion (which is not sufficient since pre-retina MBPs used LVDS):
http://www.codon.org.uk/~mjg59/tmp/retina_patches/0024-i915-Add-support-for-reprobing-for-a-panel.patch
It would be great if Intel developers could give advice on how to best integrate reprobing with their driver or how detection of ghost VGA connectors can be avoided.

- The Nvidia gpu sometimes gets stuck in powersaving mode and refuses to wake up ("Refused to change power state, currently in D3"). It seems that a delay is necessary between putting the device in D3hot and cutting the power. (On MBPs, power is cut by the gmux.) I added a delay of 200 usec and this seems to work most of the time but not always. My impression is that the required length of the delay is somehow heat related: I stress tested the code by switching back and forth in an endless loop, causing the fans to spin fast as the constant powerup and powerdown seems to draw a lot of energy, and after about 5 minutes of constant switching the Nvidia gpu gets stuck in D3 even if the delay is increased to 2 seconds. Using D3cold instead of D3hot didn't help either.

Kind regards,

Lukas

Dave Airlie (1):
vga_switcheroo: Lock/unlock DDC lines

Lukas Wunner (5):
vga_switcheroo: Lock/unlock DDC lines harder
Revert "vga_switcheroo: Add helper function to get the active client"
vga_switcheroo: Reprobe connectors if handler registers late drm/i915: Reprobe connectors if vga_switcheroo handler registers late drm/nouveau: Pause between setting gpu to D3hot and cutting the power

Matthew Garrett (1):
apple-gmux: Assign apple_gmux_data before registering

Seth Forshee (4):
vga_switcheroo: Add support for switching only the DDC
vga_switcheroo: Add helper function to get the active client
apple-gmux: Add switch_ddc support
drm/edid: Switch DDC when reading the EDID

 drivers/gpu/drm/drm_edid.c            |  10 +++-
 drivers/gpu/drm/i915/i915_dma.c       |  40 +++++++++++++
 drivers/gpu/drm/i915/i915_drv.h       |   1 +
 drivers/gpu/drm/i915/intel_display.c  |   2 +-
 drivers/gpu/drm/i915/intel_dp.c       |   6 +-
 drivers/gpu/drm/nouveau/nouveau_drm.c |   2 +
 drivers/gpu/vga/vga_switcheroo.c      | 105 +++++++++++++++++++++++++++++++++-
 drivers/platform/x86/apple-gmux.c     |  35 ++++++++++--
 include/linux/vga_switcheroo.h        |   7 +++
 9 files changed, 196 insertions(+), 12 deletions(-)

-- >8 --

drivers/gpu/drm/drm_edid.c +9 -1

index 53bc7a6..8950722 100644
@@ -32,6 +32,7 @@
 #include <linux/hdmi.h>
 #include <linux/i2c.h>
 #include <linux/module.h>
+#include <linux/vga_switcheroo.h>
 #include <drm/drmP.h>
 #include <drm/drm_edid.h>
 #include <drm/drm_displayid.h>

@@ -1326,12 +1327,19 @@ struct edid *drm_get_edid(struct drm_connector *connector,
 {
 	struct edid *edid;
 
-	if (!drm_probe_ddc(adapter))
+	vga_switcheroo_lock_ddc(connector->dev->pdev);
+
+	if (!drm_probe_ddc(adapter)) {
+		vga_switcheroo_unlock_ddc(connector->dev->pdev);
 		return NULL;
+	}
 
 	edid = drm_do_get_edid(connector, drm_do_probe_ddc_edid, adapter);
 	if (edid)
 		drm_get_displayid(connector, edid);
+
+	vga_switcheroo_unlock_ddc(connector->dev->pdev);
+
 	return edid;
 }
 EXPORT_SYMBOL(drm_get_edid);

drivers/gpu/drm/i915/i915_dma.c +40 -0

index 68e0c85..86726b7 100644
@@ -379,9 +379,49 @@ static bool i915_switcheroo_can_switch(struct pci_dev *pdev)
 	return dev->open_count == 0;
 }
 
+static void i915_switcheroo_reprobe_connectors(struct pci_dev *pdev)
+{
+	struct drm_device *dev = pci_get_drvdata(pdev);
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_encoder *encoder;
+
+	/*
+	 * Check whether we've already found a panel.
+	 * If so, we don't need to reprobe
+	 */
+	for_each_intel_encoder(dev, encoder)
+		if (encoder->type == INTEL_OUTPUT_LVDS ||
+		    encoder->type == INTEL_OUTPUT_EDP)
+			return;
+
+	/*
+	 * intel_modeset_gem_init() sets lvds_use_ssc to 0,
+	 * reset to 1 so that the SSC gets used on the panel
+	 */
+	dev_priv->vbt.lvds_use_ssc =
+		!(i915.panel_use_ssc == 0 ||
+		  dev_priv->quirks & QUIRK_LVDS_SSC_DISABLE);
+	intel_setup_outputs(dev);
+
+	/* Destroy default 1024x768 fbdev and reinitialize */
+	intel_fbdev_fini(dev);
+	if (intel_fbdev_init(dev))
+		goto cleanup_gem;
+	async_schedule(intel_fbdev_initial_config, dev_priv);
+	return;
+
+cleanup_gem:
+	DRM_ERROR("failed to reinitialize fbdev\n");
+	mutex_lock(&dev->struct_mutex);
+	i915_gem_cleanup_ringbuffer(dev);
+	i915_gem_context_fini(dev);
+	mutex_unlock(&dev->struct_mutex);
+}
+
 static const struct vga_switcheroo_client_ops i915_switcheroo_ops = {
 	.set_gpu_state = i915_switcheroo_set_state,
 	.reprobe = NULL,
+	.reprobe_connectors = i915_switcheroo_reprobe_connectors,
 	.can_switch = i915_switcheroo_can_switch,
 };

drivers/gpu/drm/i915/i915_drv.h +1 -0

index e326ac9..7f58858 100644
@@ -3120,6 +3120,7 @@ extern void intel_set_memory_cxsr(struct drm_i915_private *dev_priv,
 extern void intel_detect_pch(struct drm_device *dev);
 extern int intel_trans_dp_port_sel(struct drm_crtc *crtc);
 extern int intel_enable_rc6(const struct drm_device *dev);
+extern void intel_setup_outputs(struct drm_device *dev);
 
 extern bool i915_semaphore_is_enabled(struct drm_device *dev);
 int i915_reg_read_ioctl(struct drm_device *dev, void *data,

drivers/gpu/drm/i915/intel_display.c +1 -1

index d547d9c8..9481206 100644
@@ -13027,7 +13027,7 @@ static bool intel_crt_present(struct drm_device *dev)
 	return true;
 }
 
-static void intel_setup_outputs(struct drm_device *dev)
+void intel_setup_outputs(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_encoder *encoder;

drivers/gpu/drm/i915/intel_dp.c +5 -1

index d023710..e9c40b1 100644
@@ -30,6 +30,7 @@
 #include <linux/export.h>
 #include <linux/notifier.h>
 #include <linux/reboot.h>
+#include <linux/vga_switcheroo.h>
 #include <drm/drmP.h>
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_crtc.h>

@@ -4079,6 +4080,7 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp)
 {
 	uint8_t *dpcd = intel_dp->dpcd;
 	uint8_t type;
+	struct pci_dev *pdev = intel_dp->attached_connector->base.dev->pdev;
 
 	if (!intel_dp_get_dpcd(intel_dp))
 		return connector_status_disconnected;

@@ -4101,7 +4103,9 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp)
 	}
 
 	/* If no HPD, poke DDC gently */
-	if (drm_probe_ddc(&intel_dp->aux.ddc))
+	if (vga_switcheroo_lock_ddc(pdev) >= 0 &&
+	    drm_probe_ddc(&intel_dp->aux.ddc) &&
+	    vga_switcheroo_unlock_ddc(pdev) >= 0)
 		return connector_status_connected;
 
 	/* Well we tried, say unknown for unreliable port types */

drivers/gpu/drm/nouveau/nouveau_drm.c +2 -0

index 8904933..649024d 100644
@@ -23,6 +23,7 @@
  */
 
 #include <linux/console.h>
+#include <linux/delay.h>
 #include <linux/module.h>
 #include <linux/pci.h>
 #include <linux/pm_runtime.h>

@@ -666,6 +667,7 @@ nouveau_pmops_suspend(struct device *dev)
 	pci_save_state(pdev);
 	pci_disable_device(pdev);
 	pci_set_power_state(pdev, PCI_D3hot);
+	udelay(200);
 	return 0;
 }

drivers/gpu/vga/vga_switcheroo.c +102 -3

165 lines not on this page · raw message

drivers/platform/x86/apple-gmux.c +29 -6

index b9429fb..05bba92 100644
@@ -271,14 +271,34 @@ static const struct backlight_ops gmux_bl_ops = {
 	.update_status = gmux_update_status,
 };
 
+static int gmux_switch_ddc(enum vga_switcheroo_client_id id)
+{
+	enum vga_switcheroo_client_id old_ddc_owner;
+
+	if (gmux_read8(apple_gmux_data, GMUX_PORT_SWITCH_DDC) == 1)
+		old_ddc_owner = VGA_SWITCHEROO_IGD;
+	else
+		old_ddc_owner = VGA_SWITCHEROO_DIS;
+
+	pr_debug("Switching gmux DDC from %d to %d\n", old_ddc_owner, id);
+
+	if (id == old_ddc_owner)
+		return old_ddc_owner;
+
+	if (id == VGA_SWITCHEROO_IGD)
+		gmux_write8(apple_gmux_data, GMUX_PORT_SWITCH_DDC, 1);
+	else
+		gmux_write8(apple_gmux_data, GMUX_PORT_SWITCH_DDC, 2);
+
+	return old_ddc_owner;
+}
+
 static int gmux_switchto(enum vga_switcheroo_client_id id)
 {
 	if (id == VGA_SWITCHEROO_IGD) {
-		gmux_write8(apple_gmux_data, GMUX_PORT_SWITCH_DDC, 1);
 		gmux_write8(apple_gmux_data, GMUX_PORT_SWITCH_DISPLAY, 2);
 		gmux_write8(apple_gmux_data, GMUX_PORT_SWITCH_EXTERNAL, 2);
 	} else {
-		gmux_write8(apple_gmux_data, GMUX_PORT_SWITCH_DDC, 2);
 		gmux_write8(apple_gmux_data, GMUX_PORT_SWITCH_DISPLAY, 3);
 		gmux_write8(apple_gmux_data, GMUX_PORT_SWITCH_EXTERNAL, 3);
 	}

@@ -345,6 +365,7 @@ gmux_active_client(struct apple_gmux_data *gmux_data)
 }
 
 static struct vga_switcheroo_handler gmux_handler = {
+	.switch_ddc = gmux_switch_ddc,
 	.switchto = gmux_switchto,
 	.power_state = gmux_set_power_state,
 	.get_client_id = gmux_get_client_id,

@@ -552,18 +573,20 @@ static int gmux_probe(struct pnp_dev *pnp, const struct pnp_device_id *id)
 		gmux_data->gpe = -1;
 	}
 
+	apple_gmux_data = gmux_data;
+	init_completion(&gmux_data->powerchange_done);
+	gmux_enable_interrupts(gmux_data);
+
 	if (vga_switcheroo_register_handler(&gmux_handler)) {
 		ret = -ENODEV;
 		goto err_register_handler;
 	}
 
-	init_completion(&gmux_data->powerchange_done);
-	apple_gmux_data = gmux_data;
-	gmux_enable_interrupts(gmux_data);
-
 	return 0;
 
 err_register_handler:
+	gmux_disable_interrupts(gmux_data);
+	apple_gmux_data = NULL;
 	if (gmux_data->gpe >= 0)
 		acpi_disable_gpe(NULL, gmux_data->gpe);
 err_enable_gpe:

include/linux/vga_switcheroo.h +7 -0

index b483abd..c3ad8bf 100644
@@ -29,6 +29,7 @@ enum vga_switcheroo_client_id {
 };
 
 struct vga_switcheroo_handler {
+	int (*switch_ddc)(enum vga_switcheroo_client_id id);
 	int (*switchto)(enum vga_switcheroo_client_id id);
 	int (*power_state)(enum vga_switcheroo_client_id id,
 			   enum vga_switcheroo_state state);

@@ -39,6 +40,7 @@ struct vga_switcheroo_handler {
 struct vga_switcheroo_client_ops {
 	void (*set_gpu_state)(struct pci_dev *dev, enum vga_switcheroo_state);
 	void (*reprobe)(struct pci_dev *dev);
+	void (*reprobe_connectors)(struct pci_dev *dev);
 	bool (*can_switch)(struct pci_dev *dev);
 };

@@ -54,6 +56,9 @@ int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
 void vga_switcheroo_client_fb_set(struct pci_dev *dev,
 				  struct fb_info *info);
 
+int vga_switcheroo_lock_ddc(struct pci_dev *pdev);
+int vga_switcheroo_unlock_ddc(struct pci_dev *pdev);
+
 int vga_switcheroo_register_handler(struct vga_switcheroo_handler *handler);
 void vga_switcheroo_unregister_handler(void);

@@ -72,6 +77,8 @@ static inline void vga_switcheroo_unregister_client(struct pci_dev *dev) {}
 static inline int vga_switcheroo_register_client(struct pci_dev *dev,
 		const struct vga_switcheroo_client_ops *ops, bool driver_power_control) { return 0; }
 static inline void vga_switcheroo_client_fb_set(struct pci_dev *dev, struct fb_info *info) {}
+static inline int vga_switcheroo_lock_ddc(struct pci_dev *pdev) { return 0; }
+static inline int vga_switcheroo_unlock_ddc(struct pci_dev *pdev) { return 0; }
 static inline int vga_switcheroo_register_handler(struct vga_switcheroo_handler *handler) { return 0; }
 static inline int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
 	const struct vga_switcheroo_client_ops *ops,

Signature · 6 lines

1.8.5.2 (Apple Git-48)

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Lukas Wunner

author reviewer

2015-04-21 11:36 UTC #1 lore raw

PATCH 1/11 vga_switcheroo: Add support for switching only the DDC

From: Seth Forshee <[email protected]>

During graphics driver initialization its useful to be able to mux only the DDC to the inactive client in order to read the EDID. Add a switch_ddc callback to allow capable handlers to provide this functionality, and add vga_switcheroo_switch_ddc() to allow DRM to mux only the DDC.

Signed-off-by Seth Forshee <[email protected]>

Signed-off-by Dave Airlie <[email protected]>

Signed-off-by Lukas Wunner <[email protected]>

---
 drivers/gpu/vga/vga_switcheroo.c | 39 ++++++++++++++++++++++++++++++++++++++-
 include/linux/vga_switcheroo.h   |  4 ++++
 2 files changed, 42 insertions(+), 1 deletion(-)

drivers/gpu/vga/vga_switcheroo.c +38 -1

index 37ac7b5..0d3ac20 100644
@@ -256,6 +256,29 @@ void vga_switcheroo_client_fb_set(struct pci_dev *pdev,
 }
 EXPORT_SYMBOL(vga_switcheroo_client_fb_set);
 
+int vga_switcheroo_switch_ddc(struct pci_dev *pdev)
+{
+	int ret = 0;
+	int id;
+
+	mutex_lock(&vgasr_mutex);
+
+	if (!vgasr_priv.handler) {
+		ret = -ENODEV;
+		goto out;
+	}
+
+	if (vgasr_priv.handler->switch_ddc) {
+		id = vgasr_priv.handler->get_client_id(pdev);
+		ret = vgasr_priv.handler->switch_ddc(id);
+	}
+
+out:
+	mutex_unlock(&vgasr_mutex);
+	return ret;
+}
+EXPORT_SYMBOL(vga_switcheroo_switch_ddc);
+
 static int vga_switcheroo_show(struct seq_file *m, void *v)
 {
 	struct vga_switcheroo_client *client;

@@ -353,9 +376,15 @@ static int vga_switchto_stage2(struct vga_switcheroo_client *new_client)
 		console_unlock();
 	}
 
+	if (vgasr_priv.handler->switch_ddc) {
+		ret = vgasr_priv.handler->switch_ddc(new_client->id);
+		if (ret)
+			return ret;
+	}
+
 	ret = vgasr_priv.handler->switchto(new_client->id);
 	if (ret)
-		return ret;
+		goto restore_ddc;
 
 	if (new_client->ops->reprobe)
 		new_client->ops->reprobe(new_client->pdev);

@@ -367,6 +396,14 @@ static int vga_switchto_stage2(struct vga_switcheroo_client *new_client)
 
 	new_client->active = true;
 	return 0;
+
+restore_ddc:
+	if (vgasr_priv.handler->switch_ddc) {
+		int id = (new_client->id == VGA_SWITCHEROO_IGD) ?
+				VGA_SWITCHEROO_DIS : VGA_SWITCHEROO_IGD;
+		ret = vgasr_priv.handler->switch_ddc(id);
+	}
+	return ret;
 }
 
 static bool check_can_switch(void)

include/linux/vga_switcheroo.h +4 -0

index b483abd..2fef78b 100644
@@ -29,6 +29,7 @@ enum vga_switcheroo_client_id {
 };
 
 struct vga_switcheroo_handler {
+	int (*switch_ddc)(enum vga_switcheroo_client_id id);
 	int (*switchto)(enum vga_switcheroo_client_id id);
 	int (*power_state)(enum vga_switcheroo_client_id id,
 			   enum vga_switcheroo_state state);

@@ -54,6 +55,8 @@ int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
 void vga_switcheroo_client_fb_set(struct pci_dev *dev,
 				  struct fb_info *info);
 
+int vga_switcheroo_switch_ddc(struct pci_dev *pdev);
+
 int vga_switcheroo_register_handler(struct vga_switcheroo_handler *handler);
 void vga_switcheroo_unregister_handler(void);

@@ -72,6 +75,7 @@ static inline void vga_switcheroo_unregister_client(struct pci_dev *dev) {}
 static inline int vga_switcheroo_register_client(struct pci_dev *dev,
 		const struct vga_switcheroo_client_ops *ops, bool driver_power_control) { return 0; }
 static inline void vga_switcheroo_client_fb_set(struct pci_dev *dev, struct fb_info *info) {}
+static inline void vga_switcheroo_switch_ddc(struct pci_dev *pdev) { return NULL; }
 static inline int vga_switcheroo_register_handler(struct vga_switcheroo_handler *handler) { return 0; }
 static inline int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
 	const struct vga_switcheroo_client_ops *ops,

Signature · 6 lines

1.8.5.2 (Apple Git-48)

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Lukas Wunner

author reviewer

2015-04-21 11:39 UTC #2 lore raw

PATCH 2/11 vga_switcheroo: Add helper function to get the active client

In reply to Lukas Wunner

From: Seth Forshee <[email protected]>

Add vga_switcheroo_get_active_client() to allow drivers to get the active video client. This will be used by drivers wishing to temporarily mux only the DDC to the inactive client.

Signed-off-by Seth Forshee <[email protected]>

Signed-off-by Dave Airlie <[email protected]>

Signed-off-by Lukas Wunner <[email protected]>

---
 drivers/gpu/vga/vga_switcheroo.c | 14 ++++++++++++++
 include/linux/vga_switcheroo.h   |  2 ++
 2 files changed, 16 insertions(+)

drivers/gpu/vga/vga_switcheroo.c +14 -0

index 0d3ac20..620c4ac 100644
@@ -209,6 +209,20 @@ find_active_client(struct list_head *head)
 	return NULL;
 }
 
+struct pci_dev *vga_switcheroo_get_active_client(void)
+{
+	struct vga_switcheroo_client *client;
+	struct pci_dev *pdev = NULL;
+
+	mutex_lock(&vgasr_mutex);
+	client = find_active_client(&vgasr_priv.clients);
+	if (client)
+		pdev = client->pdev;
+	mutex_unlock(&vgasr_mutex);
+	return pdev;
+}
+EXPORT_SYMBOL(vga_switcheroo_get_active_client);
+
 int vga_switcheroo_get_client_state(struct pci_dev *pdev)
 {
 	struct vga_switcheroo_client *client;

include/linux/vga_switcheroo.h +2 -0

index 2fef78b..c81a686 100644
@@ -62,6 +62,7 @@ void vga_switcheroo_unregister_handler(void);
 
 int vga_switcheroo_process_delayed_switch(void);
 
+struct pci_dev *vga_switcheroo_get_active_client(void);
 int vga_switcheroo_get_client_state(struct pci_dev *dev);
 
 void vga_switcheroo_set_dynamic_switch(struct pci_dev *pdev, enum vga_switcheroo_state dynamic);

@@ -82,6 +83,7 @@ static inline int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
 	int id, bool active) { return 0; }
 static inline void vga_switcheroo_unregister_handler(void) {}
 static inline int vga_switcheroo_process_delayed_switch(void) { return 0; }
+static inline struct pci_dev *vga_switcheroo_get_active_client(void) { return NULL; }
 static inline int vga_switcheroo_get_client_state(struct pci_dev *dev) { return VGA_SWITCHEROO_ON; }
 
 static inline void vga_switcheroo_set_dynamic_switch(struct pci_dev *pdev, enum vga_switcheroo_state dynamic) {}

Signature · 6 lines

1.8.5.2 (Apple Git-48)

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Lukas Wunner

author reviewer

2015-04-21 11:36 UTC #3 lore raw

PATCH 3/11 apple-gmux: Add switch_ddc support

In reply to Lukas Wunner

From: Seth Forshee <[email protected]>

The gmux allows muxing the DDC independently from the display, so support this functionality. This will allow reading the EDID for the inactive GPU, fixing issues with machines that either don't have a VBT or have invalid mode data in the VBT.

Signed-off-by Seth Forshee <[email protected]>

Signed-off-by Dave Airlie <[email protected]>

Signed-off-by Lukas Wunner <[email protected]>

---
 drivers/platform/x86/apple-gmux.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

drivers/platform/x86/apple-gmux.c +10 -2

index b9429fb..1cf242c 100644
@@ -271,14 +271,21 @@ static const struct backlight_ops gmux_bl_ops = {
 	.update_status = gmux_update_status,
 };
 
+static int gmux_switch_ddc(enum vga_switcheroo_client_id id)
+{
+	if (id == VGA_SWITCHEROO_IGD)
+		gmux_write8(apple_gmux_data, GMUX_PORT_SWITCH_DDC, 1);
+	else
+		gmux_write8(apple_gmux_data, GMUX_PORT_SWITCH_DDC, 2);
+	return 0;
+}
+
 static int gmux_switchto(enum vga_switcheroo_client_id id)
 {
 	if (id == VGA_SWITCHEROO_IGD) {
-		gmux_write8(apple_gmux_data, GMUX_PORT_SWITCH_DDC, 1);
 		gmux_write8(apple_gmux_data, GMUX_PORT_SWITCH_DISPLAY, 2);
 		gmux_write8(apple_gmux_data, GMUX_PORT_SWITCH_EXTERNAL, 2);
 	} else {
-		gmux_write8(apple_gmux_data, GMUX_PORT_SWITCH_DDC, 2);
 		gmux_write8(apple_gmux_data, GMUX_PORT_SWITCH_DISPLAY, 3);
 		gmux_write8(apple_gmux_data, GMUX_PORT_SWITCH_EXTERNAL, 3);
 	}

@@ -345,6 +352,7 @@ gmux_active_client(struct apple_gmux_data *gmux_data)
 }
 
 static struct vga_switcheroo_handler gmux_handler = {
+	.switch_ddc = gmux_switch_ddc,
 	.switchto = gmux_switchto,
 	.power_state = gmux_set_power_state,
 	.get_client_id = gmux_get_client_id,

Signature · 6 lines

1.8.5.2 (Apple Git-48)

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Lukas Wunner

author reviewer

2015-04-21 11:39 UTC #4 lore raw

PATCH 4/11 drm/edid: Switch DDC when reading the EDID

In reply to Lukas Wunner

From: Seth Forshee <[email protected]>

Some dual graphics machines support muxing the DDC separately from the display, so make use of this functionality when reading the EDID on the inactive GPU. Also serialize drm_get_edid() with a mutex to avoid races on the DDC mux state.

Signed-off-by Seth Forshee <[email protected]>

Signed-off-by Dave Airlie <[email protected]>

Signed-off-by Lukas Wunner <[email protected]>

---
 drivers/gpu/drm/drm_edid.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

drivers/gpu/drm/drm_edid.c +18 -0

index 53bc7a6..f208bb3 100644
@@ -32,6 +32,7 @@
 #include <linux/hdmi.h>
 #include <linux/i2c.h>
 #include <linux/module.h>
+#include <linux/vga_switcheroo.h>
 #include <drm/drmP.h>
 #include <drm/drm_edid.h>
 #include <drm/drm_displayid.h>

@@ -87,6 +88,8 @@ struct detailed_mode_closure {
 #define LEVEL_GTF2	2
 #define LEVEL_CVT	3
 
+static DEFINE_MUTEX(drm_edid_mutex);
+
 static struct edid_quirk {
 	char vendor[4];
 	int product_id;

@@ -1325,6 +1328,16 @@ struct edid *drm_get_edid(struct drm_connector *connector,
 			  struct i2c_adapter *adapter)
 {
 	struct edid *edid;
+	struct pci_dev *pdev = connector->dev->pdev;
+	struct pci_dev *active_pdev = NULL;
+
+	mutex_lock(&drm_edid_mutex);
+
+	if (pdev) {
+		active_pdev = vga_switcheroo_get_active_client();
+		if (active_pdev != pdev)
+			vga_switcheroo_switch_ddc(pdev);
+	}
 
 	if (!drm_probe_ddc(adapter))
 		return NULL;

@@ -1332,6 +1345,11 @@ struct edid *drm_get_edid(struct drm_connector *connector,
 	edid = drm_do_get_edid(connector, drm_do_probe_ddc_edid, adapter);
 	if (edid)
 		drm_get_displayid(connector, edid);
+
+	if (active_pdev && active_pdev != pdev)
+		vga_switcheroo_switch_ddc(active_pdev);
+
+	mutex_unlock(&drm_edid_mutex);
 	return edid;
 }
 EXPORT_SYMBOL(drm_get_edid);

Signature · 6 lines

1.8.5.2 (Apple Git-48)

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Lukas Wunner

author reviewer

2015-04-21 11:39 UTC #5 lore raw

PATCH 5/11 vga_switcheroo: Lock/unlock DDC lines

In reply to Lukas Wunner

From: Dave Airlie <[email protected]>

Replace vga_switcheroo_switch_ddc() with vga_switcheroo_lock_ddc() and vga_switcheroo_unlock_ddc(), move mutex from drm_get_edid() to vga_switcheroo.c

Motivation for these changes according to Dave Airlie:
"I can't figure out why I didn't like this, but I rewrote this way back to lock/unlock the ddc lines, bits are contained in this WIP mess. I think I'd prefer something like that otherwise the interface got really ugly."
http://lists.freedesktop.org/archives/dri-devel/2014-June/061629.html

Original commit by Dave Airlie contained additional experimental and unused code. Reduced to the bare minimum and amended with this commit message by Lukas Wunner <[email protected]>

Signed-off-by Lukas Wunner <[email protected]>

---
 drivers/gpu/drm/drm_edid.c       | 16 ++------------
 drivers/gpu/vga/vga_switcheroo.c | 46 ++++++++++++++++++++++++++++++++++++++--
 include/linux/vga_switcheroo.h   |  3 ++-
 3 files changed, 48 insertions(+), 17 deletions(-)

drivers/gpu/drm/drm_edid.c +2 ~~-14~~

index f208bb3..f91593b 100644
@@ -88,8 +88,6 @@ struct detailed_mode_closure {
 #define LEVEL_GTF2	2
 #define LEVEL_CVT	3
 
-static DEFINE_MUTEX(drm_edid_mutex);
-
 static struct edid_quirk {
 	char vendor[4];
 	int product_id;

@@ -1328,16 +1326,8 @@ struct edid *drm_get_edid(struct drm_connector *connector,
 			  struct i2c_adapter *adapter)
 {
 	struct edid *edid;
-	struct pci_dev *pdev = connector->dev->pdev;
-	struct pci_dev *active_pdev = NULL;
-
-	mutex_lock(&drm_edid_mutex);
 
-	if (pdev) {
-		active_pdev = vga_switcheroo_get_active_client();
-		if (active_pdev != pdev)
-			vga_switcheroo_switch_ddc(pdev);
-	}
+	vga_switcheroo_lock_ddc(connector->dev->pdev);
 
 	if (!drm_probe_ddc(adapter))
 		return NULL;

@@ -1346,10 +1336,8 @@ struct edid *drm_get_edid(struct drm_connector *connector,
 	if (edid)
 		drm_get_displayid(connector, edid);
 
-	if (active_pdev && active_pdev != pdev)
-		vga_switcheroo_switch_ddc(active_pdev);
+	vga_switcheroo_unlock_ddc(connector->dev->pdev);
 
-	mutex_unlock(&drm_edid_mutex);
 	return edid;
 }
 EXPORT_SYMBOL(drm_get_edid);

drivers/gpu/vga/vga_switcheroo.c +44 -2

77 lines not on this page · raw message

include/linux/vga_switcheroo.h +2 -1

index c81a686..352bef3 100644
@@ -55,7 +55,8 @@ int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
 void vga_switcheroo_client_fb_set(struct pci_dev *dev,
 				  struct fb_info *info);
 
-int vga_switcheroo_switch_ddc(struct pci_dev *pdev);
+int vga_switcheroo_lock_ddc(struct pci_dev *pdev);
+int vga_switcheroo_unlock_ddc(struct pci_dev *pdev);
 
 int vga_switcheroo_register_handler(struct vga_switcheroo_handler *handler);
 void vga_switcheroo_unregister_handler(void);

Signature · 6 lines

1.8.5.2 (Apple Git-48)

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Lukas Wunner

author reviewer

2015-04-21 11:36 UTC #6 lore raw

PATCH 6/11 vga_switcheroo: Lock/unlock DDC lines harder

In reply to Lukas Wunner

Unlock DDC lines if drm_probe_ddc() fails.

intel_dp_detect_dpcd() calls drm_probe_ddc(), needs to lock DDC lines.

Don't lock vgasr_mutex in vga_switcheroo_lock/unlock_ddc(), it's already locked in vga_switcheroo_debugfs_write(), leading to a deadlock upon reprobe in stage2 when the new client reads the EDID. Locking ddc_lock is sufficient.

If the inactive client registers before the active client then old_ddc_owner cannot be determined with find_active_client() (null pointer dereference). Therefore change semantics of the switch_ddc() handler callback to return old_ddc_owner on success or a negative int on failure.

Lock ddc_lock in stage2 to avoid race condition where reading the EDID and switching happens simultaneously.

Switch DDC lines on MIGD / MDIS commands and on runtime suspend.

Signed-off-by Lukas Wunner <[email protected]>

---
 drivers/gpu/drm/drm_edid.c        |  4 +-
 drivers/gpu/drm/i915/intel_dp.c   |  6 ++-
 drivers/gpu/vga/vga_switcheroo.c  | 82 ++++++++++++++++++++++-----------------
 drivers/platform/x86/apple-gmux.c | 15 ++++++-
 include/linux/vga_switcheroo.h    |  3 +-
 5 files changed, 71 insertions(+), 39 deletions(-)

drivers/gpu/drm/drm_edid.c +3 -1

index f91593b..8950722 100644
@@ -1329,8 +1329,10 @@ struct edid *drm_get_edid(struct drm_connector *connector,
 
 	vga_switcheroo_lock_ddc(connector->dev->pdev);
 
-	if (!drm_probe_ddc(adapter))
+	if (!drm_probe_ddc(adapter)) {
+		vga_switcheroo_unlock_ddc(connector->dev->pdev);
 		return NULL;
+	}
 
 	edid = drm_do_get_edid(connector, drm_do_probe_ddc_edid, adapter);
 	if (edid)

drivers/gpu/drm/i915/intel_dp.c +5 -1

index d023710..e9c40b1 100644
@@ -30,6 +30,7 @@
 #include <linux/export.h>
 #include <linux/notifier.h>
 #include <linux/reboot.h>
+#include <linux/vga_switcheroo.h>
 #include <drm/drmP.h>
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_crtc.h>

@@ -4079,6 +4080,7 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp)
 {
 	uint8_t *dpcd = intel_dp->dpcd;
 	uint8_t type;
+	struct pci_dev *pdev = intel_dp->attached_connector->base.dev->pdev;
 
 	if (!intel_dp_get_dpcd(intel_dp))
 		return connector_status_disconnected;

@@ -4101,7 +4103,9 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp)
 	}
 
 	/* If no HPD, poke DDC gently */
-	if (drm_probe_ddc(&intel_dp->aux.ddc))
+	if (vga_switcheroo_lock_ddc(pdev) >= 0 &&
+	    drm_probe_ddc(&intel_dp->aux.ddc) &&
+	    vga_switcheroo_unlock_ddc(pdev) >= 0)
 		return connector_status_connected;
 
 	/* Well we tried, say unknown for unreliable port types */

drivers/gpu/vga/vga_switcheroo.c +47 ~~-35~~

162 lines not on this page · raw message

drivers/platform/x86/apple-gmux.c +14 -1

index 1cf242c..9aafbde 100644
@@ -273,11 +273,24 @@ static const struct backlight_ops gmux_bl_ops = {
 
 static int gmux_switch_ddc(enum vga_switcheroo_client_id id)
 {
+	enum vga_switcheroo_client_id old_ddc_owner;
+
+	if (gmux_read8(apple_gmux_data, GMUX_PORT_SWITCH_DDC) == 1)
+		old_ddc_owner = VGA_SWITCHEROO_IGD;
+	else
+		old_ddc_owner = VGA_SWITCHEROO_DIS;
+
+	pr_debug("Switching gmux DDC from %d to %d\n", old_ddc_owner, id);
+
+	if (id == old_ddc_owner)
+		return old_ddc_owner;
+
 	if (id == VGA_SWITCHEROO_IGD)
 		gmux_write8(apple_gmux_data, GMUX_PORT_SWITCH_DDC, 1);
 	else
 		gmux_write8(apple_gmux_data, GMUX_PORT_SWITCH_DDC, 2);
-	return 0;
+
+	return old_ddc_owner;
 }
 
 static int gmux_switchto(enum vga_switcheroo_client_id id)

include/linux/vga_switcheroo.h +2 -1

index 352bef3..8963799 100644
@@ -77,7 +77,8 @@ static inline void vga_switcheroo_unregister_client(struct pci_dev *dev) {}
 static inline int vga_switcheroo_register_client(struct pci_dev *dev,
 		const struct vga_switcheroo_client_ops *ops, bool driver_power_control) { return 0; }
 static inline void vga_switcheroo_client_fb_set(struct pci_dev *dev, struct fb_info *info) {}
-static inline void vga_switcheroo_switch_ddc(struct pci_dev *pdev) { return NULL; }
+static inline int vga_switcheroo_lock_ddc(struct pci_dev *pdev) { return 0; }
+static inline int vga_switcheroo_unlock_ddc(struct pci_dev *pdev) { return 0; }
 static inline int vga_switcheroo_register_handler(struct vga_switcheroo_handler *handler) { return 0; }
 static inline int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
 	const struct vga_switcheroo_client_ops *ops,

Signature · 6 lines

1.8.5.2 (Apple Git-48)

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Lukas Wunner

author reviewer

2015-04-21 11:39 UTC #7 lore raw

PATCH 7/11 Revert "vga_switcheroo: Add helper function to get the active client"

In reply to Lukas Wunner

This reverts commit 26814ce68904c9faf977c90edac798156311981f.

The helper function is no longer needed after Dave Airlie's rewrite of vga_switcheroo_switch_ddc(), the commit introducing it was only included because commit 31f23c3d488e43d61e5e812a1830a9ea5589df1d (drm/edid: Switch DDC when reading the EDID) does not compile without it.

Signed-off-by Lukas Wunner <[email protected]>

---
 drivers/gpu/vga/vga_switcheroo.c | 14 --------------
 include/linux/vga_switcheroo.h   |  2 --
 2 files changed, 16 deletions(-)

drivers/gpu/vga/vga_switcheroo.c +0 ~~-14~~

index 2534d84..062fafe 100644
@@ -214,20 +214,6 @@ find_active_client(struct list_head *head)
 	return NULL;
 }
 
-struct pci_dev *vga_switcheroo_get_active_client(void)
-{
-	struct vga_switcheroo_client *client;
-	struct pci_dev *pdev = NULL;
-
-	mutex_lock(&vgasr_mutex);
-	client = find_active_client(&vgasr_priv.clients);
-	if (client)
-		pdev = client->pdev;
-	mutex_unlock(&vgasr_mutex);
-	return pdev;
-}
-EXPORT_SYMBOL(vga_switcheroo_get_active_client);
-
 int vga_switcheroo_get_client_state(struct pci_dev *pdev)
 {
 	struct vga_switcheroo_client *client;

include/linux/vga_switcheroo.h +0 -2

index 8963799..60c9d65 100644
@@ -63,7 +63,6 @@ void vga_switcheroo_unregister_handler(void);
 
 int vga_switcheroo_process_delayed_switch(void);
 
-struct pci_dev *vga_switcheroo_get_active_client(void);
 int vga_switcheroo_get_client_state(struct pci_dev *dev);
 
 void vga_switcheroo_set_dynamic_switch(struct pci_dev *pdev, enum vga_switcheroo_state dynamic);

@@ -85,7 +84,6 @@ static inline int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
 	int id, bool active) { return 0; }
 static inline void vga_switcheroo_unregister_handler(void) {}
 static inline int vga_switcheroo_process_delayed_switch(void) { return 0; }
-static inline struct pci_dev *vga_switcheroo_get_active_client(void) { return NULL; }
 static inline int vga_switcheroo_get_client_state(struct pci_dev *dev) { return VGA_SWITCHEROO_ON; }
 
 static inline void vga_switcheroo_set_dynamic_switch(struct pci_dev *pdev, enum vga_switcheroo_state dynamic) {}

Signature · 6 lines

1.8.5.2 (Apple Git-48)

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Lukas Wunner

author reviewer

2015-04-21 11:38 UTC #8 lore raw

PATCH 8/11 vga_switcheroo: Reprobe connectors if handler registers late

In reply to Lukas Wunner

If the handler registers late, already registered clients must reprobe their connectors in case they require the handler to switch DDC lines. That is the case on MacBook Pros, which use a gmux chip as handler.

Inspired by Matthew Garrett, who did this in vga_switcheroo_enable() instead, which delayed reprobing until a second gpu registers. Probably makes sense to do this as early as possible, particularly since a second gpu may never register if its driver is blacklisted or not installed:
http://lists.freedesktop.org/archives/dri-devel/2014-June/060941.html

Signed-off-by Lukas Wunner <[email protected]>

---
 drivers/gpu/vga/vga_switcheroo.c | 8 ++++++++
 include/linux/vga_switcheroo.h   | 1 +
 2 files changed, 9 insertions(+)

drivers/gpu/vga/vga_switcheroo.c +8 -0

index 062fafe..3b13e9a 100644
@@ -108,6 +108,8 @@ static void vga_switcheroo_enable(void)
 
 int vga_switcheroo_register_handler(struct vga_switcheroo_handler *handler)
 {
+	struct vga_switcheroo_client *client;
+
 	mutex_lock(&vgasr_mutex);
 	if (vgasr_priv.handler) {
 		mutex_unlock(&vgasr_mutex);

@@ -115,6 +117,12 @@ int vga_switcheroo_register_handler(struct vga_switcheroo_handler *handler)
 	}
 
 	vgasr_priv.handler = handler;
+
+	/* clients which registered before the handler must reprobe */
+	list_for_each_entry(client, &vgasr_priv.clients, list)
+		if (!client->active && client->ops->reprobe_connectors)
+			client->ops->reprobe_connectors(client->pdev);
+
 	if (vga_switcheroo_ready()) {
 		printk(KERN_INFO "vga_switcheroo: enabled\n");
 		vga_switcheroo_enable();

include/linux/vga_switcheroo.h +1 -0

index 60c9d65..c3ad8bf 100644
@@ -40,6 +40,7 @@ struct vga_switcheroo_handler {
 struct vga_switcheroo_client_ops {
 	void (*set_gpu_state)(struct pci_dev *dev, enum vga_switcheroo_state);
 	void (*reprobe)(struct pci_dev *dev);
+	void (*reprobe_connectors)(struct pci_dev *dev);
 	bool (*can_switch)(struct pci_dev *dev);
 };

Signature · 6 lines

1.8.5.2 (Apple Git-48)

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Lukas Wunner

author reviewer

2015-04-21 11:36 UTC #9 lore raw

PATCH 9/11 apple-gmux: Assign apple_gmux_data before registering

In reply to Lukas Wunner

From: Matthew Garrett <[email protected]>

Registering the handler after both GPUs will trigger a DDC switch for connector reprobing. This will oops if apple_gmux_data hasn't already been assigned. Reorder the code to do that.

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

Signed-off-by Lukas Wunner <[email protected]>

---
 drivers/platform/x86/apple-gmux.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

drivers/platform/x86/apple-gmux.c +6 -4

index 9aafbde..05bba92 100644
@@ -573,18 +573,20 @@ static int gmux_probe(struct pnp_dev *pnp, const struct pnp_device_id *id)
 		gmux_data->gpe = -1;
 	}
 
+	apple_gmux_data = gmux_data;
+	init_completion(&gmux_data->powerchange_done);
+	gmux_enable_interrupts(gmux_data);
+
 	if (vga_switcheroo_register_handler(&gmux_handler)) {
 		ret = -ENODEV;
 		goto err_register_handler;
 	}
 
-	init_completion(&gmux_data->powerchange_done);
-	apple_gmux_data = gmux_data;
-	gmux_enable_interrupts(gmux_data);
-
 	return 0;
 
 err_register_handler:
+	gmux_disable_interrupts(gmux_data);
+	apple_gmux_data = NULL;
 	if (gmux_data->gpe >= 0)
 		acpi_disable_gpe(NULL, gmux_data->gpe);
 err_enable_gpe:

Signature · 6 lines

1.8.5.2 (Apple Git-48)

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Lukas Wunner

author reviewer

2015-04-21 11:39 UTC #10 lore raw

PATCH 10/11 drm/i915: Reprobe connectors if vga_switcheroo handler registers late

In reply to Lukas Wunner

MacBook Pro hybrid graphics use a gmux chip to switch DDC lines between gpus. It may register after the i915 driver, necessitating a reprobe of the connectors and reinitialization of the fbdev.

Inspired by Matthew Garrett, who duplicated intel_setup_outputs() and reduced it to just the eDP probing portion (which is not sufficient since pre-retina MBPs used LVDS):
http://www.codon.org.uk/~mjg59/tmp/retina_patches/0024-i915-Add-support-for-reprobing-for-a-panel.patch

Commit 92122789b2d699a1e82dca502940e0dd37bf6f3b (drm/i915: preserve SSC if previously set v3) sets lvds_use_ssc to 0, it must be reset to 1 so that the SSC gets used on the panel.

Signed-off-by Lukas Wunner <[email protected]>

---
 drivers/gpu/drm/i915/i915_dma.c      | 40 ++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_drv.h      |  1 +
 drivers/gpu/drm/i915/intel_display.c |  2 +-
 3 files changed, 42 insertions(+), 1 deletion(-)

drivers/gpu/drm/i915/i915_dma.c +40 -0

index 68e0c85..86726b7 100644
@@ -379,9 +379,49 @@ static bool i915_switcheroo_can_switch(struct pci_dev *pdev)
 	return dev->open_count == 0;
 }
 
+static void i915_switcheroo_reprobe_connectors(struct pci_dev *pdev)
+{
+	struct drm_device *dev = pci_get_drvdata(pdev);
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_encoder *encoder;
+
+	/*
+	 * Check whether we've already found a panel.
+	 * If so, we don't need to reprobe
+	 */
+	for_each_intel_encoder(dev, encoder)
+		if (encoder->type == INTEL_OUTPUT_LVDS ||
+		    encoder->type == INTEL_OUTPUT_EDP)
+			return;
+
+	/*
+	 * intel_modeset_gem_init() sets lvds_use_ssc to 0,
+	 * reset to 1 so that the SSC gets used on the panel
+	 */
+	dev_priv->vbt.lvds_use_ssc =
+		!(i915.panel_use_ssc == 0 ||
+		  dev_priv->quirks & QUIRK_LVDS_SSC_DISABLE);
+	intel_setup_outputs(dev);
+
+	/* Destroy default 1024x768 fbdev and reinitialize */
+	intel_fbdev_fini(dev);
+	if (intel_fbdev_init(dev))
+		goto cleanup_gem;
+	async_schedule(intel_fbdev_initial_config, dev_priv);
+	return;
+
+cleanup_gem:
+	DRM_ERROR("failed to reinitialize fbdev\n");
+	mutex_lock(&dev->struct_mutex);
+	i915_gem_cleanup_ringbuffer(dev);
+	i915_gem_context_fini(dev);
+	mutex_unlock(&dev->struct_mutex);
+}
+
 static const struct vga_switcheroo_client_ops i915_switcheroo_ops = {
 	.set_gpu_state = i915_switcheroo_set_state,
 	.reprobe = NULL,
+	.reprobe_connectors = i915_switcheroo_reprobe_connectors,
 	.can_switch = i915_switcheroo_can_switch,
 };

drivers/gpu/drm/i915/i915_drv.h +1 -0

index e326ac9..7f58858 100644
@@ -3120,6 +3120,7 @@ extern void intel_set_memory_cxsr(struct drm_i915_private *dev_priv,
 extern void intel_detect_pch(struct drm_device *dev);
 extern int intel_trans_dp_port_sel(struct drm_crtc *crtc);
 extern int intel_enable_rc6(const struct drm_device *dev);
+extern void intel_setup_outputs(struct drm_device *dev);
 
 extern bool i915_semaphore_is_enabled(struct drm_device *dev);
 int i915_reg_read_ioctl(struct drm_device *dev, void *data,

drivers/gpu/drm/i915/intel_display.c +1 -1

index d547d9c8..9481206 100644
@@ -13027,7 +13027,7 @@ static bool intel_crt_present(struct drm_device *dev)
 	return true;
 }
 
-static void intel_setup_outputs(struct drm_device *dev)
+void intel_setup_outputs(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_encoder *encoder;

Signature · 6 lines

1.8.5.2 (Apple Git-48)

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Chris Wilson

2015-04-21 11:50 UTC #11 lore raw

Re: [PATCH 10/11] drm/i915: Reprobe connectors if vga_switcheroo handler registers late

drivers/gpu/drm/i915/i915_dma.c static bool i915_switcheroo_can_switch(struct pci_dev *pdev) · line 418

Quoted 6 lines · Lukas Wunner

drivers/gpu/drm/i915/i915_dma.c +6 -0

+cleanup_gem:
+	DRM_ERROR("failed to reinitialize fbdev\n");
+	mutex_lock(&dev->struct_mutex);
+	i915_gem_cleanup_ringbuffer(dev);
+	i915_gem_context_fini(dev);
+	mutex_unlock(&dev->struct_mutex);

This is a bit random since you don't initialize GEM here and presumably the GPU is still available even if it has no outputs.
-Chris

Signature · 5 lines

Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Lukas Wunner

author reviewer

2015-04-21 12:16 UTC #12 lore raw

Re: [PATCH 10/11] drm/i915: Reprobe connectors if vga_switcheroo handler registers late

Hi Chris,

Quoted 10 lines · Chris Wilson

drivers/gpu/drm/i915/i915_dma.c static bool i915_switcheroo_can_switch(struct pci_dev *pdev) · line 418
Quoted 6 lines · Lukas Wunner
drivers/gpu/drm/i915/i915_dma.c +6 -0
+cleanup_gem:
+	DRM_ERROR("failed to reinitialize fbdev\n");
+	mutex_lock(&dev->struct_mutex);
+	i915_gem_cleanup_ringbuffer(dev);
+	i915_gem_context_fini(dev);
+	mutex_unlock(&dev->struct_mutex);
This is a bit random since you don't initialize GEM here and presumably the GPU is still available even if it has no outputs.

Hm, I copied this verbatim from i915_dma.c:i915_load_modeset_init(), I was under the impression that this is required if fbdev initialization failed. Am I missing something?

Thanks,

Lukas
_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Chris Wilson

2015-04-21 12:38 UTC #13 lore raw

Re: [PATCH 10/11] drm/i915: Reprobe connectors if vga_switcheroo handler registers late

Quoted 14 lines · Lukas Wunner

Hi Chris,
Quoted 10 lines · Chris Wilson
drivers/gpu/drm/i915/i915_dma.c static bool i915_switcheroo_can_switch(struct pci_dev *pdev) · line 418
Quoted 6 lines · Lukas Wunner
drivers/gpu/drm/i915/i915_dma.c +6 -0
+cleanup_gem:
+	DRM_ERROR("failed to reinitialize fbdev\n");
+	mutex_lock(&dev->struct_mutex);
+	i915_gem_cleanup_ringbuffer(dev);
+	i915_gem_context_fini(dev);
+	mutex_unlock(&dev->struct_mutex);
This is a bit random since you don't initialize GEM here and presumably the GPU is still available even if it has no outputs.

Quoted 3 lines

Hm, I copied this verbatim from i915_dma.c:i915_load_modeset_init(), I was under the impression that this is required if fbdev initialization failed. Am I missing something?

Just that we haven't given much thought to its error path before. Here it's even more different since we already have a working GEM, just no connectors.
-Chris

Signature · 5 lines

Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Lukas Wunner

author reviewer

2015-04-21 11:38 UTC #14 lore raw

PATCH 11/11 drm/nouveau: Pause between setting gpu to D3hot and cutting the power

In reply to Lukas Wunner

On the MacBook Pro, power of the gpu is cut by a gmux chip. Sometimes the gpu gets stuck in powersaving mode and refuses to wake up ("Refused to change power state, currently in D3"). Inserting a delay between setting the gpu to D3hot and cutting the power seems to help (most of the time). This issue and its (partial) remediation by the patch was observed with an Nvidia GT650M (NVE7 / GK107).

Signed-off-by Lukas Wunner <[email protected]>

---
 drivers/gpu/drm/nouveau/nouveau_drm.c | 2 ++
 1 file changed, 2 insertions(+)

drivers/gpu/drm/nouveau/nouveau_drm.c +2 -0

index 8904933..649024d 100644
@@ -23,6 +23,7 @@
  */
 
 #include <linux/console.h>
+#include <linux/delay.h>
 #include <linux/module.h>
 #include <linux/pci.h>
 #include <linux/pm_runtime.h>

@@ -666,6 +667,7 @@ nouveau_pmops_suspend(struct device *dev)
 	pci_save_state(pdev);
 	pci_disable_device(pdev);
 	pci_set_power_state(pdev, PCI_D3hot);
+	udelay(200);
 	return 0;
 }

Signature · 6 lines

1.8.5.2 (Apple Git-48)

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Matthew Garrett

2015-04-21 19:49 UTC #15 lore raw

In reply to Lukas Wunner

Quoted 9 lines · Lukas Wunner

On MBPs, the panel is not connected to one of the gpus, but to the gmux chip (the "handler" in vga_switcheroo speak). Initially the gmux is switched to the discrete gpu. The integrated i915 gpu is therefore unable to detect the LVDS (or eDP) connector on bootup and switching to the integrated gpu will lead to a black screen. The solution is to temporarily switch the DDC lines to the integrated gpu when reading the EDID, which the gmux is capable of. Lack of support for this in vga_switcheroo is pretty much the only obstacle to get gpu switching to work on MBPs.

My testing suggested that changing the DDC lines didn't change auxch, so this approach doesn't work for eDP. Have you found otherwise?

Signature · 5 lines

Matthew Garrett | [email protected]
_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Lukas Wunner

author reviewer

2015-04-22 13:48 UTC #16 lore raw

Hi Matthew,

Quoted 2 lines · Matthew Garrett

My testing suggested that changing the DDC lines didn't change auxch, so this approach doesn't work for eDP. Have you found otherwise?

I only have a MacBookPro9,1 available for testing, which was the last non-retina and thus uses LVDS. So I can vouch for proper functioning of the patchset on pre-retinas, not on retinas (which use eDP because dual channel LVDS maxes out 2048×1536@60 Hz, the retinas have 2880x1800).

Background:
The last non-retina (MacBookPro9,1) and the first retina (MacBookPro10,1) were sold alongside each other, from June 2012 to October 2013. The following generation (MacBookPro11, October 2013) was retina only. Pre-retina MacBookPros with dual GPUs Intel/Nvidia or Intel/AMD were available starting with the MacBookPro6, April 2010.

I cannot verify that gmux doesn't switch auxch but I understand you worked around this by stashing the EDID after nouveau has read it and retrieving the stashed version from i915:
http://www.codon.org.uk/~mjg59/tmp/retina_patches/0016-vga_switcheroo-Allow-stashing-of-panel-data.patch
http://www.codon.org.uk/~mjg59/tmp/retina_patches/0025-i915-Use-vga_switcheroo-to-obtain-and-stash-EDID-DPC.patch
http://www.codon.org.uk/~mjg59/tmp/retina_patches/0026-nouveau-Use-vga_switcheroo-to-obtain-and-stash-EDID-.patch

It would be great if someone with a retina MBP could test my patchset, and if it doesn't work, try it with these three patches applied on top.

Thanks,

Lukas
_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Lukas Wunner

author reviewer

2015-04-25 14:20 UTC #17 lore raw

In reply to Matthew Garrett

Hi,

Quoted 2 lines · Matthew Garrett

My testing suggested that changing the DDC lines didn't change auxch, so this approach doesn't work for eDP. Have you found otherwise?

Disassembling the OS X gmux driver (AppleMuxControl 3.6.22) revealed that register 0x7F is set prior to accessing the eDP connector. It seems plausible that this register switches the AUX channel between GPUs.

More specifically, there's a method AppleMuxControl::TickleStateMachine() which puts the gmux into various states. The code to switch only the DDC lines starts at address d4bc and sets register 0x7F, then calls AppleMuxControl::copyEDPConfig(), then sets register 0x28 (DDC).

Likewise, the code to switch the entire display starts at address db6a, it sets registers 0x10 (DISPLAY), 0x28 (DDC), 0x40 (EXTERNAL) and 0x7F, then calls calling AppleMuxControl::fbDoEDPLinkConfig().

Included below is a tentative patch to set register 0x7F when switching the DDC lines. It would be great if someone with a retina could test the patchset I've posted plus this tentative patch since I can't test it myself.

Here's the lowdown on which patches are needed for each MBP generation:

* MacBookPro6 / 8 / 9 (pre-retina):
The patchset I've posted plus this one to enable dual channel LVDS:
http://lists.freedesktop.org/archives/intel-gfx/2015-April/064850.html
(This obviates the need to specify i915.lvds_channel_mode=2)

* MacBookPro10 (first-gen retina):
The patchset I've posted plus the tentative patch below.

* MacBookPro11 (second-gen retina):
The patchset I've posted plus the tentative patch below. Fellow gmux hackers Andreas Heider and Bruno Bierbaumer tell me that additionally, this patch is needed otherwise the i915 gpu is turned off:
https://www.marc.info/?l=grub-deavel&m=141586614924917&w=2

Kind regards,

Lukas

-- >8 --

drivers/platform/x86/apple-gmux.c +6 -2

index 05bba92..719cfd3 100644
@@ -60,6 +60,7 @@ static struct apple_gmux_data *apple_gmux_data;
 #define GMUX_PORT_DISCRETE_POWER	0x50
 #define GMUX_PORT_MAX_BRIGHTNESS	0x70
 #define GMUX_PORT_BRIGHTNESS		0x74
+#define GMUX_PORT_SWITCH_AUXCH		0x7F
 #define GMUX_PORT_VALUE			0xc2
 #define GMUX_PORT_READ			0xd0
 #define GMUX_PORT_WRITE			0xd4

@@ -285,10 +286,13 @@ static int gmux_switch_ddc(enum vga_switcheroo_client_id id)
 	if (id == old_ddc_owner)
 		return old_ddc_owner;
 
-	if (id == VGA_SWITCHEROO_IGD)
+	if (id == VGA_SWITCHEROO_IGD) {
+		gmux_write8(apple_gmux_data, GMUX_PORT_SWITCH_AUXCH, 1);
 		gmux_write8(apple_gmux_data, GMUX_PORT_SWITCH_DDC, 1);
-	else
+	} else {
+		gmux_write8(apple_gmux_data, GMUX_PORT_SWITCH_AUXCH, 2);
 		gmux_write8(apple_gmux_data, GMUX_PORT_SWITCH_DDC, 2);
+	}
 
 	return old_ddc_owner;
 }

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Lu, Ran

2015-05-02 20:40 UTC #18 lore raw

Hi Lukas,

Quoted 10 lines · Lukas Wunner

Included below is a tentative patch to set register 0x7F when switching the DDC lines. It would be great if someone with a retina could test the patchset I've posted plus this tentative patch since I can't test it myself.
* MacBookPro11 (second-gen retina):
The patchset I've posted plus the tentative patch below. Fellow gmux hackers Andreas Heider and Bruno Bierbaumer tell me that additionally, this patch is needed otherwise the i915 gpu is turned off:
https://www.marc.info/?l=grub-deavel&m=141586614924917&w=2

Thank you very much for your effort of bringing gpu switching to RMBP, I tried your patchset, including the one in this email, on my laptop. It is a Retina MacBookPro, late 2013 version, which is how ifixit calls it. It seems to work at least partially. Here is the results I observed.

First of all, I use gfxstatus in OSX to force the system to use the intel gpu. So when I boot into Linux the intel gpu will be activated and I can turn of the discrete nvidia gpu to save power.

After booted with the patched kernel (on top of v4.0). I can use "echo DIS > /sys/kernel/debug/vgaswitcheroo/switch" to switch to the nvidia gpu. The console turned black, I guess the system is incapable of switching to the nouveau's framebuffer at this moment. But after I startx through ssh, I can use everything without any problem. After quit the X the framebuffer is recovered as well. I can switch back to intel gpu and start X again no problem. However when I activated the discrete gpu again, I got a black screen after startx, the log of X seems normal, just no display on the screen.

I also tried to use the gfxstatus to resume the "normal" (dynamical switching) behavior in OSX. Then the nvidia gpu will be used when booting to linux. I switched to the intel gpu but it appears the gpu did not find the correct screen. After startx I got a black screen, xrandr does not give me the eDP output, only some Virtual and VGA outputs.

I hope these information can help to improve those patches. I am happy to try any (hopefully not dangerous) patches if you have any new ones.

Signature · 7 lines

Best Regards,
LR

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel