Nouveau - Jan 2016 - [PATCH v5 11/12] drm/nouveau: Defer probe if gmux is present but its driver isn't

gmux is a microcontroller built into dual GPU MacBook Pros.
On pre-retina MBPs, if we're the inactive GPU, we need apple-gmux
to temporarily switch DDC so that we can probe the panel's EDID.

The checks for CONFIG_VGA_ARB and CONFIG_VGA_SWITCHEROO are necessary
because if either of them is disabled but gmux is present, the driver
would never load, even if we're the active GPU. (vga_default_device()
would evaluate to NULL and vga_switcheroo_handler_flags() would
evaluate to 0.)

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=88861
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=61115
Tested-by: Lukas Wunner <lukas at wunner.de>
    [MBP  9,1 2012  intel IVB + nvidia GK107  pre-retina  15"]
Signed-off-by: Lukas Wunner <lukas at wunner.de>
---
 drivers/gpu/drm/nouveau/nouveau_drm.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c
b/drivers/gpu/drm/nouveau/nouveau_drm.c
index 2f2f252..bb8498c 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -22,11 +22,13 @@
  * Authors: Ben Skeggs
  */
 
+#include <linux/apple-gmux.h>
 #include <linux/console.h>
 #include <linux/delay.h>
 #include <linux/module.h>
 #include <linux/pci.h>
 #include <linux/pm_runtime.h>
+#include <linux/vgaarb.h>
 #include <linux/vga_switcheroo.h>
 
 #include "drmP.h"
@@ -312,6 +314,15 @@ static int nouveau_drm_probe(struct pci_dev *pdev,
 	bool boot = false;
 	int ret;
 
+	/*
+	 * apple-gmux is needed on dual GPU MacBook Pro
+	 * to probe the panel if we're the inactive GPU.
+	 */
+	if (IS_ENABLED(CONFIG_VGA_ARB) && IS_ENABLED(CONFIG_VGA_SWITCHEROO)
&&
+	    apple_gmux_present() && pdev != vga_default_device() &&
+	    !vga_switcheroo_handler_flags())
+		return -EPROBE_DEFER;
+
 	/* remove conflicting drivers (vesafb, efifb etc) */
 	aper = alloc_apertures(3);
 	if (!aper)
-- 
1.8.5.2 (Apple Git-48)

Enable GPU switching on the pre-retina MacBook Pro (2008 - 2013), v5.

The main obstacle on these machines is that the panel mode in VBIOS
is bogus. Fortunately gmux can switch DDC independently from the
display, thereby allowing the inactive GPU to probe the panel's EDID.

In short, vga_switcheroo and apple-gmux are amended with hooks to
switch DDC, DRM core is amended with a drm_get_edid_switcheroo() helper,
and relevant drivers are amended to call that for LVDS outputs.

The retina MacBook Pro (2012 - present) uses eDP and cannot switch
AUX independently from the main link. The main obstacle there is link
training, I'm currently working on this, it will be addressed in a
future patch set.

This series is also reviewable on GitHub:
https://github.com/l1k/linux/commits/mbp_switcheroo_v5

Changes:

* New patch [01/12]: vga_switcheroo handler flags
  Alex Deucher asked if this series might regress on non-Apple laptops.
  To address this concern, I let handlers declare their capabilities in
  a bitmask. DRM drivers call drm_get_edid_switcheroo() only if the
  handler has set the VGA_SWITCHEROO_CAN_SWITCH_DDC flag.
  Currently just one other flag is defined which is used on retinas.

* Changed patch [02/12]: vga_switcheroo DDC locking
  Rename ddc_lock to mux_hw_lock, suggested by Daniel Vetter.

* New patch [03/12]: track switch state of apple-gmux
  Fixes a bug in previous versions of this series which occurred if
  the system was suspended while DDC was temporarily switched:
  On resume DDC was switched to the wrong GPU.

* New patches [09/12 - 12/12]: deferred probing
  Previously I used connector reprobing if the inactive GPU's driver
  loaded before gmux. I've ditched that in favor of deferred driver
  probing, which is much simpler. Thanks to Daniel Vetter for the
  suggestion.

Caution: Patch [09/12] depends on a new acpi_dev_present() API which
will land in 4.5 via Rafael J. Wysocki's tree.

I would particularly be interested in feedback on the handler flags
patch [01/12]. I'm not 100% happy with the number of characters
required to query the flags (e.g.: if (vga_switcheroo_handler_flags() &
VGA_SWITCHEROO_CAN_SWITCH_DDC)), but failed to come up with something
shorter. Thierry Reding used a struct of bools instead of a bitmask
for his recent drm_dp_link_caps patches. Maybe use that instead?
http://lists.freedesktop.org/archives/dri-devel/2015-December/097025.html

Thanks,

Lukas


Lukas Wunner (12):
  vga_switcheroo: Add handler flags infrastructure
  vga_switcheroo: Add support for switching only the DDC
  apple-gmux: Track switch state
  apple-gmux: Add switch_ddc support
  drm/edid: Switch DDC when reading the EDID
  drm/i915: Switch DDC when reading the EDID
  drm/nouveau: Switch DDC when reading the EDID
  drm/radeon: Switch DDC when reading the EDID
  apple-gmux: Add helper for presence detect
  drm/i915: Defer probe if gmux is present but its driver isn't
  drm/nouveau: Defer probe if gmux is present but its driver isn't
  drm/radeon: Defer probe if gmux is present but its driver isn't

 Documentation/DocBook/gpu.tmpl                   |   5 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c |   3 +-
 drivers/gpu/drm/drm_edid.c                       |  26 +++++
 drivers/gpu/drm/i915/i915_drv.c                  |  12 +++
 drivers/gpu/drm/i915/intel_lvds.c                |   8 +-
 drivers/gpu/drm/nouveau/nouveau_acpi.c           |   2 +-
 drivers/gpu/drm/nouveau/nouveau_connector.c      |  21 +++-
 drivers/gpu/drm/nouveau/nouveau_drm.c            |  11 +++
 drivers/gpu/drm/radeon/radeon_atpx_handler.c     |   3 +-
 drivers/gpu/drm/radeon/radeon_connectors.c       |   6 ++
 drivers/gpu/drm/radeon/radeon_drv.c              |  11 +++
 drivers/gpu/vga/vga_switcheroo.c                 | 119 ++++++++++++++++++++++-
 drivers/platform/x86/apple-gmux.c                | 111 ++++++++++++++++-----
 include/drm/drm_crtc.h                           |   2 +
 include/linux/apple-gmux.h                       |  39 ++++++++
 include/linux/vga_switcheroo.h                   |  36 ++++++-
 16 files changed, 382 insertions(+), 33 deletions(-)
 create mode 100644 include/linux/apple-gmux.h

-- 
1.8.5.2 (Apple Git-48)

The pre-retina MacBook Pro uses an LVDS panel and a gmux controller
to switch the panel between its two GPUs. The panel mode in VBIOS
is notoriously bogus on these machines.

Use drm_get_edid_switcheroo() in lieu of drm_get_edid() on LVDS
if the vga_switcheroo handler is capable of temporarily switching
the panel's DDC lines to the discrete GPU. This allows us to retrieve
the EDID if the panel is currently muxed to the integrated GPU.
Likewise, ask vga_switcheroo to switch DDC before probing LVDS
connectors.

This only enables EDID probing on the pre-retina MBP (2008 - 2013).
The retina MBP (2012 - present) uses eDP and gmux is not capable of
switching AUX separately from the main link on these models.
This will be addressed in later patches.

List of pre-retina MBPs with dual GPUs, either or both Nvidia:
    [MBP  5,1 2008  nvidia MCP79 + G96        pre-retina  15"]
    [MBP  5,2 2009  nvidia MCP79 + G96        pre-retina  17"]
    [MBP  5,3 2009  nvidia MCP79 + G96        pre-retina  15"]
    [MBP  6,2 2010  intel ILK + nvidia GT216  pre-retina  15"]
    [MBP  6,1 2010  intel ILK + nvidia GT216  pre-retina  17"]
    [MBP  9,1 2012  intel IVB + nvidia GK107  pre-retina  15"]

v3: Commit newly added due to introduction of drm_get_edid_switcheroo()
    wrapper which drivers need to opt-in to.

v5: Rebase on "vga_switcheroo: Add handler flags infrastructure",
    i.e. call drm_get_edid_switcheroo() only if the handler
    indicates that DDC is switchable.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=88861
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=61115
Tested-by: Lukas Wunner <lukas at wunner.de>
    [MBP  9,1 2012  intel IVB + nvidia GK107  pre-retina  15"]
Signed-off-by: Lukas Wunner <lukas at wunner.de>
---
 drivers/gpu/drm/nouveau/nouveau_connector.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c
b/drivers/gpu/drm/nouveau/nouveau_connector.c
index fcebfae..ae96ebc 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.c
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
@@ -27,6 +27,7 @@
 #include <acpi/button.h>
 
 #include <linux/pm_runtime.h>
+#include <linux/vga_switcheroo.h>
 
 #include <drm/drmP.h>
 #include <drm/drm_edid.h>
@@ -153,6 +154,17 @@ nouveau_connector_ddc_detect(struct drm_connector
*connector)
 			if (ret == 0)
 				break;
 		} else
+		if ((vga_switcheroo_handler_flags() &
+		     VGA_SWITCHEROO_CAN_SWITCH_DDC) &&
+		    nv_encoder->dcb->type == DCB_OUTPUT_LVDS &&
+		    nv_encoder->i2c) {
+			int ret;
+			vga_switcheroo_lock_ddc(dev->pdev);
+			ret = nvkm_probe_i2c(nv_encoder->i2c, 0x50);
+			vga_switcheroo_unlock_ddc(dev->pdev);
+			if (ret)
+				break;
+		} else
 		if (nv_encoder->i2c) {
 			if (nvkm_probe_i2c(nv_encoder->i2c, 0x50))
 				break;
@@ -265,7 +277,14 @@ nouveau_connector_detect(struct drm_connector *connector,
bool force)
 
 	nv_encoder = nouveau_connector_ddc_detect(connector);
 	if (nv_encoder && (i2c = nv_encoder->i2c) != NULL) {
-		nv_connector->edid = drm_get_edid(connector, i2c);
+		if ((vga_switcheroo_handler_flags() &
+		     VGA_SWITCHEROO_CAN_SWITCH_DDC) &&
+		    nv_connector->type == DCB_CONNECTOR_LVDS)
+			nv_connector->edid = drm_get_edid_switcheroo(connector,
+								     i2c);
+		else
+			nv_connector->edid = drm_get_edid(connector, i2c);
+
 		drm_mode_connector_update_edid_property(connector,
 							nv_connector->edid);
 		if (!nv_connector->edid) {
-- 
1.8.5.2 (Apple Git-48)

Allow handlers to declare their capabilities and allow clients to
obtain that information. So far we have these use cases:

* If the handler is able to switch DDC separately, clients need to
  probe EDID with drm_get_edid_switcheroo(). We should allow them
  to detect a capable handler to ensure this function only gets
  called when needed.

* Likewise if the handler is unable to switch AUX separately, the active
  client needs to communicate link training parameters to the inactive
  client, which may then skip the AUX handshake and set up its output
  with these pre-calibrated values (DisplayPort specification v1.1a,
  section 2.5.3.3). Clients need a way to recognize such a situation.

The flags for the radeon_atpx_handler and amdgpu_atpx_handler are
initially set to 0, this can later on be amended with
  handler_flags |= VGA_SWITCHEROO_CAN_SWITCH_DDC;
when a ->switch_ddc callback is added.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=88861
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=61115
Tested-by: Lukas Wunner <lukas at wunner.de>
    [MBP  9,1 2012  intel IVB + nvidia GK107  pre-retina  15"]
Signed-off-by: Lukas Wunner <lukas at wunner.de>
---
 Documentation/DocBook/gpu.tmpl                   |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c |  3 ++-
 drivers/gpu/drm/nouveau/nouveau_acpi.c           |  2 +-
 drivers/gpu/drm/radeon/radeon_atpx_handler.c     |  3 ++-
 drivers/gpu/vga/vga_switcheroo.c                 | 22 ++++++++++++++++++-
 drivers/platform/x86/apple-gmux.c                |  2 +-
 include/linux/vga_switcheroo.h                   | 28 ++++++++++++++++++++++--
 7 files changed, 54 insertions(+), 7 deletions(-)

diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl
index 9e95aa1..88e7c39 100644
--- a/Documentation/DocBook/gpu.tmpl
+++ b/Documentation/DocBook/gpu.tmpl
@@ -3648,6 +3648,7 @@ int num_ioctls;</synopsis>
     </sect1>
     <sect1>
       <title>Public constants</title>
+!Finclude/linux/vga_switcheroo.h vga_switcheroo_handler_flags_t
 !Finclude/linux/vga_switcheroo.h vga_switcheroo_client_id
 !Finclude/linux/vga_switcheroo.h vga_switcheroo_state
     </sect1>
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
index 3c89586..fa948dc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c
@@ -552,13 +552,14 @@ static bool amdgpu_atpx_detect(void)
 void amdgpu_register_atpx_handler(void)
 {
 	bool r;
+	enum vga_switcheroo_handler_flags_t handler_flags = 0;
 
 	/* detect if we have any ATPX + 2 VGA in the system */
 	r = amdgpu_atpx_detect();
 	if (!r)
 		return;
 
-	vga_switcheroo_register_handler(&amdgpu_atpx_handler);
+	vga_switcheroo_register_handler(&amdgpu_atpx_handler, handler_flags);
 }
 
 /**
diff --git a/drivers/gpu/drm/nouveau/nouveau_acpi.c
b/drivers/gpu/drm/nouveau/nouveau_acpi.c
index d5e6938..cdf5227 100644
--- a/drivers/gpu/drm/nouveau/nouveau_acpi.c
+++ b/drivers/gpu/drm/nouveau/nouveau_acpi.c
@@ -314,7 +314,7 @@ void nouveau_register_dsm_handler(void)
 	if (!r)
 		return;
 
-	vga_switcheroo_register_handler(&nouveau_dsm_handler);
+	vga_switcheroo_register_handler(&nouveau_dsm_handler, 0);
 }
 
 /* Must be called for Optimus models before the card can be turned off */
diff --git a/drivers/gpu/drm/radeon/radeon_atpx_handler.c
b/drivers/gpu/drm/radeon/radeon_atpx_handler.c
index c4b4f29..56482e3 100644
--- a/drivers/gpu/drm/radeon/radeon_atpx_handler.c
+++ b/drivers/gpu/drm/radeon/radeon_atpx_handler.c
@@ -551,13 +551,14 @@ static bool radeon_atpx_detect(void)
 void radeon_register_atpx_handler(void)
 {
 	bool r;
+	enum vga_switcheroo_handler_flags_t handler_flags = 0;
 
 	/* detect if we have any ATPX + 2 VGA in the system */
 	r = radeon_atpx_detect();
 	if (!r)
 		return;
 
-	vga_switcheroo_register_handler(&radeon_atpx_handler);
+	vga_switcheroo_register_handler(&radeon_atpx_handler, handler_flags);
 }
 
 /**
diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c
index d64d905..81da941 100644
--- a/drivers/gpu/vga/vga_switcheroo.c
+++ b/drivers/gpu/vga/vga_switcheroo.c
@@ -125,6 +125,7 @@ static DEFINE_MUTEX(vgasr_mutex);
  * 	(counting only vga clients, not audio clients)
  * @clients: list of registered clients
  * @handler: registered handler
+ * @handler_flags: flags of registered handler
  *
  * vga_switcheroo private data. Currently only one vga_switcheroo instance
  * per system is supported.
@@ -141,6 +142,7 @@ struct vgasr_priv {
 	struct list_head clients;
 
 	const struct vga_switcheroo_handler *handler;
+	enum vga_switcheroo_handler_flags_t handler_flags;
 };
 
 #define ID_BIT_AUDIO		0x100
@@ -189,13 +191,15 @@ static void vga_switcheroo_enable(void)
 /**
  * vga_switcheroo_register_handler() - register handler
  * @handler: handler callbacks
+ * @handler_flags: handler flags
  *
  * Register handler. Enable vga_switcheroo if two vga clients have already
  * registered.
  *
  * Return: 0 on success, -EINVAL if a handler was already registered.
  */
-int vga_switcheroo_register_handler(const struct vga_switcheroo_handler
*handler)
+int vga_switcheroo_register_handler(const struct vga_switcheroo_handler
*handler,
+				    enum vga_switcheroo_handler_flags_t handler_flags)
 {
 	mutex_lock(&vgasr_mutex);
 	if (vgasr_priv.handler) {
@@ -204,6 +208,7 @@ int vga_switcheroo_register_handler(const struct
vga_switcheroo_handler *handler
 	}
 
 	vgasr_priv.handler = handler;
+	vgasr_priv.handler_flags = handler_flags;
 	if (vga_switcheroo_ready()) {
 		pr_info("enabled\n");
 		vga_switcheroo_enable();
@@ -221,6 +226,7 @@ EXPORT_SYMBOL(vga_switcheroo_register_handler);
 void vga_switcheroo_unregister_handler(void)
 {
 	mutex_lock(&vgasr_mutex);
+	vgasr_priv.handler_flags = 0;
 	vgasr_priv.handler = NULL;
 	if (vgasr_priv.active) {
 		pr_info("disabled\n");
@@ -231,6 +237,20 @@ void vga_switcheroo_unregister_handler(void)
 }
 EXPORT_SYMBOL(vga_switcheroo_unregister_handler);
 
+/**
+ * vga_switcheroo_handler_flags() - obtain handler flags
+ *
+ * Helper for clients to obtain the handler flags bitmask.
+ *
+ * Return: Handler flags. A value of 0 means that no handler is registered
+ * or that the handler has no special capabilities.
+ */
+enum vga_switcheroo_handler_flags_t vga_switcheroo_handler_flags(void)
+{
+	return vgasr_priv.handler_flags;
+}
+EXPORT_SYMBOL(vga_switcheroo_handler_flags);
+
 static int register_client(struct pci_dev *pdev,
 			   const struct vga_switcheroo_client_ops *ops,
 			   enum vga_switcheroo_client_id id, bool active,
diff --git a/drivers/platform/x86/apple-gmux.c
b/drivers/platform/x86/apple-gmux.c
index f236250..c401d49 100644
--- a/drivers/platform/x86/apple-gmux.c
+++ b/drivers/platform/x86/apple-gmux.c
@@ -705,7 +705,7 @@ static int gmux_probe(struct pnp_dev *pnp, const struct
pnp_device_id *id)
 	init_completion(&gmux_data->powerchange_done);
 	gmux_enable_interrupts(gmux_data);
 
-	if (vga_switcheroo_register_handler(&gmux_handler)) {
+	if (vga_switcheroo_register_handler(&gmux_handler, 0)) {
 		ret = -ENODEV;
 		goto err_register_handler;
 	}
diff --git a/include/linux/vga_switcheroo.h b/include/linux/vga_switcheroo.h
index 69e1d4a1..a745f4f0 100644
--- a/include/linux/vga_switcheroo.h
+++ b/include/linux/vga_switcheroo.h
@@ -36,6 +36,26 @@
 struct pci_dev;
 
 /**
+ * enum vga_switcheroo_handler_flags_t - handler flags bitmask
+ * @VGA_SWITCHEROO_CAN_SWITCH_DDC: whether the handler is able to switch the
+ * 	DDC lines separately. This signals to clients that they should call
+ * 	drm_get_edid_switcheroo() to probe the EDID
+ * @VGA_SWITCHEROO_NEEDS_EDP_CONFIG: whether the handler is unable to switch
+ * 	the AUX channel separately. This signals to clients that the active
+ * 	GPU needs to train the link and communicate the link parameters to the
+ * 	inactive GPU (mediated by vga_switcheroo). The inactive GPU may then
+ * 	skip the AUX handshake and set up its output with these pre-calibrated
+ * 	values (DisplayPort specification v1.1a, section 2.5.3.3)
+ *
+ * Handler flags bitmask. Used by handlers to declare their capabilities upon
+ * registering with vga_switcheroo.
+ */
+enum vga_switcheroo_handler_flags_t {
+	VGA_SWITCHEROO_CAN_SWITCH_DDC	= (1 << 0),
+	VGA_SWITCHEROO_NEEDS_EDP_CONFIG	= (1 << 1),
+};
+
+/**
  * enum vga_switcheroo_state - client power state
  * @VGA_SWITCHEROO_OFF: off
  * @VGA_SWITCHEROO_ON: on
@@ -132,8 +152,10 @@ 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_register_handler(const struct vga_switcheroo_handler
*handler);
+int vga_switcheroo_register_handler(const struct vga_switcheroo_handler
*handler,
+				    enum vga_switcheroo_handler_flags_t handler_flags);
 void vga_switcheroo_unregister_handler(void);
+enum vga_switcheroo_handler_flags_t vga_switcheroo_handler_flags(void);
 
 int vga_switcheroo_process_delayed_switch(void);
 
@@ -150,11 +172,13 @@ 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_register_handler(const struct
vga_switcheroo_handler *handler) { return 0; }
+static inline int vga_switcheroo_register_handler(const struct
vga_switcheroo_handler *handler,
+		enum vga_switcheroo_handler_flags_t handler_flags) { return 0; }
 static inline int vga_switcheroo_register_audio_client(struct pci_dev *pdev,
 	const struct vga_switcheroo_client_ops *ops,
 	enum vga_switcheroo_client_id id) { return 0; }
 static inline void vga_switcheroo_unregister_handler(void) {}
+static inline enum vga_switcheroo_handler_flags_t
vga_switcheroo_handler_flags(void) { return 0; }
 static inline int vga_switcheroo_process_delayed_switch(void) { return 0; }
 static inline enum vga_switcheroo_state vga_switcheroo_get_client_state(struct
pci_dev *dev) { return VGA_SWITCHEROO_ON; }
 
-- 
1.8.5.2 (Apple Git-48)

Hi,

On Mon, Jan 11, 2016 at 08:09:20PM +0100, Lukas Wunner
wrote:
> Enable GPU switching on the pre-retina MacBook Pro (2008 - 2013), v5.
This series hasn't seen any reviews or acks unfortunately.
Any takers?

Merging this would allow fdo #61115 to be closed
(currently assigned to intel-gfx).

FWIW this series has in the meantime been tested by more folks:

Tested-by: Pierre Moreau <pierre.morrow at free.fr>
    [MBP  5,3 2009  nvidia MCP79 + G96        pre-retina  15"]
Tested-by: Paul Hordiienko <pvt.gord at gmail.com>
    [MBP  6,2 2010  intel ILK + nvidia GT216  pre-retina  15"]
Tested-by: William Brown <william at blackhats.net.au>
    [MBP  8,2 2011  intel SNB + amd turks     pre-retina  15"]
Tested-by: Lukas Wunner <lukas at wunner.de>
    [MBP  9,1 2012  intel IVB + nvidia GK107  pre-retina  15"]

On the latter three models it worked fine. On Pierre Moreau's machine
the discrete nvidia G96 locks up when woken. This happened in the past
as well but not on the first wake but only on the 10th or so. Since it
works fine on the GT216 and GK107, I'm guessing we've got a regression
in the wakeup code for the G96 which is somehow triggered by this patch
set (more specifically: triggered by being able to retrieve the proper
panel resolution and configure a crtc). It needs to be fixed but isn't
a showstopper for this series IMHO. (Arguably being able to retrieve
EDID but then locking up on switching isn't really worse than not being
able to retrieve EDID in the first place.)

Thanks,

Lukas
> 
> The main obstacle on these machines is that the panel mode in VBIOS
> is bogus. Fortunately gmux can switch DDC independently from the
> display, thereby allowing the inactive GPU to probe the panel's EDID.
> 
> In short, vga_switcheroo and apple-gmux are amended with hooks to
> switch DDC, DRM core is amended with a drm_get_edid_switcheroo() helper,
> and relevant drivers are amended to call that for LVDS outputs.
> 
> The retina MacBook Pro (2012 - present) uses eDP and cannot switch
> AUX independently from the main link. The main obstacle there is link
> training, I'm currently working on this, it will be addressed in a
> future patch set.
> 
> This series is also reviewable on GitHub:
> https://github.com/l1k/linux/commits/mbp_switcheroo_v5
> 
> Changes:
> 
> * New patch [01/12]: vga_switcheroo handler flags
>   Alex Deucher asked if this series might regress on non-Apple laptops.
>   To address this concern, I let handlers declare their capabilities in
>   a bitmask. DRM drivers call drm_get_edid_switcheroo() only if the
>   handler has set the VGA_SWITCHEROO_CAN_SWITCH_DDC flag.
>   Currently just one other flag is defined which is used on retinas.
> 
> * Changed patch [02/12]: vga_switcheroo DDC locking
>   Rename ddc_lock to mux_hw_lock, suggested by Daniel Vetter.
> 
> * New patch [03/12]: track switch state of apple-gmux
>   Fixes a bug in previous versions of this series which occurred if
>   the system was suspended while DDC was temporarily switched:
>   On resume DDC was switched to the wrong GPU.
> 
> * New patches [09/12 - 12/12]: deferred probing
>   Previously I used connector reprobing if the inactive GPU's driver
>   loaded before gmux. I've ditched that in favor of deferred driver
>   probing, which is much simpler. Thanks to Daniel Vetter for the
>   suggestion.
> 
> Caution: Patch [09/12] depends on a new acpi_dev_present() API which
> will land in 4.5 via Rafael J. Wysocki's tree.
> 
> I would particularly be interested in feedback on the handler flags
> patch [01/12]. I'm not 100% happy with the number of characters
> required to query the flags (e.g.: if (vga_switcheroo_handler_flags() &
> VGA_SWITCHEROO_CAN_SWITCH_DDC)), but failed to come up with something
> shorter. Thierry Reding used a struct of bools instead of a bitmask
> for his recent drm_dp_link_caps patches. Maybe use that instead?
> http://lists.freedesktop.org/archives/dri-devel/2015-December/097025.html
> 
> Thanks,
> 
> Lukas
> 
> 
> Lukas Wunner (12):
>   vga_switcheroo: Add handler flags infrastructure
>   vga_switcheroo: Add support for switching only the DDC
>   apple-gmux: Track switch state
>   apple-gmux: Add switch_ddc support
>   drm/edid: Switch DDC when reading the EDID
>   drm/i915: Switch DDC when reading the EDID
>   drm/nouveau: Switch DDC when reading the EDID
>   drm/radeon: Switch DDC when reading the EDID
>   apple-gmux: Add helper for presence detect
>   drm/i915: Defer probe if gmux is present but its driver isn't
>   drm/nouveau: Defer probe if gmux is present but its driver isn't
>   drm/radeon: Defer probe if gmux is present but its driver isn't
> 
>  Documentation/DocBook/gpu.tmpl                   |   5 +
>  drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c |   3 +-
>  drivers/gpu/drm/drm_edid.c                       |  26 +++++
>  drivers/gpu/drm/i915/i915_drv.c                  |  12 +++
>  drivers/gpu/drm/i915/intel_lvds.c                |   8 +-
>  drivers/gpu/drm/nouveau/nouveau_acpi.c           |   2 +-
>  drivers/gpu/drm/nouveau/nouveau_connector.c      |  21 +++-
>  drivers/gpu/drm/nouveau/nouveau_drm.c            |  11 +++
>  drivers/gpu/drm/radeon/radeon_atpx_handler.c     |   3 +-
>  drivers/gpu/drm/radeon/radeon_connectors.c       |   6 ++
>  drivers/gpu/drm/radeon/radeon_drv.c              |  11 +++
>  drivers/gpu/vga/vga_switcheroo.c                 | 119
++++++++++++++++++++++-
>  drivers/platform/x86/apple-gmux.c                | 111
++++++++++++++++-----
>  include/drm/drm_crtc.h                           |   2 +
>  include/linux/apple-gmux.h                       |  39 ++++++++
>  include/linux/vga_switcheroo.h                   |  36 ++++++-
>  16 files changed, 382 insertions(+), 33 deletions(-)
>  create mode 100644 include/linux/apple-gmux.h
> 
> -- 
> 1.8.5.2 (Apple Git-48)
>

On 2 February 2016 at 08:49, Lukas Wunner <lukas at wunner.de>
wrote:
> Hi,
>
> On Mon, Jan 11, 2016 at 08:09:20PM +0100, Lukas Wunner wrote:
>> Enable GPU switching on the pre-retina MacBook Pro (2008 - 2013), v5.
>
> This series hasn't seen any reviews or acks unfortunately.
> Any takers?
Has the tree this depends on been merged? I got these patches and applied
them to drm-next and found I needed some acpi patches.

Can you start pushing these to github or somewhere and putting the link
here?

Dave.

Hello all,
> On 01 Feb 2016, at 23:49, Lukas Wunner <lukas at wunner.de> wrote:
> 
> Hi,
> 
>> On Mon, Jan 11, 2016 at 08:09:20PM +0100, Lukas Wunner wrote:
>> Enable GPU switching on the pre-retina MacBook Pro (2008 - 2013), v5.
> 
> This series hasn't seen any reviews or acks unfortunately.
> Any takers?
> 
> Merging this would allow fdo #61115 to be closed
> (currently assigned to intel-gfx).
> 
> FWIW this series has in the meantime been tested by more folks:
> 
> Tested-by: Pierre Moreau <pierre.morrow at free.fr>
>    [MBP  5,3 2009  nvidia MCP79 + G96        pre-retina  15"]
> Tested-by: Paul Hordiienko <pvt.gord at gmail.com>
>    [MBP  6,2 2010  intel ILK + nvidia GT216  pre-retina  15"]
> Tested-by: William Brown <william at blackhats.net.au>
>    [MBP  8,2 2011  intel SNB + amd turks     pre-retina  15"]
> Tested-by: Lukas Wunner <lukas at wunner.de>
>    [MBP  9,1 2012  intel IVB + nvidia GK107  pre-retina  15"]
> 
> On the latter three models it worked fine. On Pierre Moreau's machine
> the discrete nvidia G96 locks up when woken. This happened in the past
> as well but not on the first wake but only on the 10th or so. Since it
> works fine on the GT216 and GK107, I'm guessing we've got a
regression
> in the wakeup code for the G96 which is somehow triggered by this patch
> set (more specifically: triggered by being able to retrieve the proper
> panel resolution and configure a crtc). It needs to be fixed but isn't
> a showstopper for this series IMHO. (Arguably being able to retrieve
> EDID but then locking up on switching isn't really worse than not being
> able to retrieve EDID in the first place.)
I would say it is slightly worse, since the only "downside" of not
retrieving the EDID means  TTY is set to a default resolution rather than the
screen resolution, but this is fixed when starting X.
On the other hand, since DRI_PRIME works fine on the laptop, there isn't
much reason to switch between cards.

I'll have a look at the resume this week, now that FOSDEM is off my todo
list.

Regards,
Pierre

> 
> Thanks,
> 
> Lukas
> 
>> 
>> The main obstacle on these machines is that the panel mode in VBIOS
>> is bogus. Fortunately gmux can switch DDC independently from the
>> display, thereby allowing the inactive GPU to probe the panel's
EDID.
>> 
>> In short, vga_switcheroo and apple-gmux are amended with hooks to
>> switch DDC, DRM core is amended with a drm_get_edid_switcheroo()
helper,
>> and relevant drivers are amended to call that for LVDS outputs.
>> 
>> The retina MacBook Pro (2012 - present) uses eDP and cannot switch
>> AUX independently from the main link. The main obstacle there is link
>> training, I'm currently working on this, it will be addressed in a
>> future patch set.
>> 
>> This series is also reviewable on GitHub:
>> https://github.com/l1k/linux/commits/mbp_switcheroo_v5
>> 
>> Changes:
>> 
>> * New patch [01/12]: vga_switcheroo handler flags
>>  Alex Deucher asked if this series might regress on non-Apple laptops.
>>  To address this concern, I let handlers declare their capabilities in
>>  a bitmask. DRM drivers call drm_get_edid_switcheroo() only if the
>>  handler has set the VGA_SWITCHEROO_CAN_SWITCH_DDC flag.
>>  Currently just one other flag is defined which is used on retinas.
>> 
>> * Changed patch [02/12]: vga_switcheroo DDC locking
>>  Rename ddc_lock to mux_hw_lock, suggested by Daniel Vetter.
>> 
>> * New patch [03/12]: track switch state of apple-gmux
>>  Fixes a bug in previous versions of this series which occurred if
>>  the system was suspended while DDC was temporarily switched:
>>  On resume DDC was switched to the wrong GPU.
>> 
>> * New patches [09/12 - 12/12]: deferred probing
>>  Previously I used connector reprobing if the inactive GPU's driver
>>  loaded before gmux. I've ditched that in favor of deferred driver
>>  probing, which is much simpler. Thanks to Daniel Vetter for the
>>  suggestion.
>> 
>> Caution: Patch [09/12] depends on a new acpi_dev_present() API which
>> will land in 4.5 via Rafael J. Wysocki's tree.
>> 
>> I would particularly be interested in feedback on the handler flags
>> patch [01/12]. I'm not 100% happy with the number of characters
>> required to query the flags (e.g.: if (vga_switcheroo_handler_flags()
&
>> VGA_SWITCHEROO_CAN_SWITCH_DDC)), but failed to come up with something
>> shorter. Thierry Reding used a struct of bools instead of a bitmask
>> for his recent drm_dp_link_caps patches. Maybe use that instead?
>>
http://lists.freedesktop.org/archives/dri-devel/2015-December/097025.html
>> 
>> Thanks,
>> 
>> Lukas
>> 
>> 
>> Lukas Wunner (12):
>>  vga_switcheroo: Add handler flags infrastructure
>>  vga_switcheroo: Add support for switching only the DDC
>>  apple-gmux: Track switch state
>>  apple-gmux: Add switch_ddc support
>>  drm/edid: Switch DDC when reading the EDID
>>  drm/i915: Switch DDC when reading the EDID
>>  drm/nouveau: Switch DDC when reading the EDID
>>  drm/radeon: Switch DDC when reading the EDID
>>  apple-gmux: Add helper for presence detect
>>  drm/i915: Defer probe if gmux is present but its driver isn't
>>  drm/nouveau: Defer probe if gmux is present but its driver isn't
>>  drm/radeon: Defer probe if gmux is present but its driver isn't
>> 
>> Documentation/DocBook/gpu.tmpl                   |   5 +
>> drivers/gpu/drm/amd/amdgpu/amdgpu_atpx_handler.c |   3 +-
>> drivers/gpu/drm/drm_edid.c                       |  26 +++++
>> drivers/gpu/drm/i915/i915_drv.c                  |  12 +++
>> drivers/gpu/drm/i915/intel_lvds.c                |   8 +-
>> drivers/gpu/drm/nouveau/nouveau_acpi.c           |   2 +-
>> drivers/gpu/drm/nouveau/nouveau_connector.c      |  21 +++-
>> drivers/gpu/drm/nouveau/nouveau_drm.c            |  11 +++
>> drivers/gpu/drm/radeon/radeon_atpx_handler.c     |   3 +-
>> drivers/gpu/drm/radeon/radeon_connectors.c       |   6 ++
>> drivers/gpu/drm/radeon/radeon_drv.c              |  11 +++
>> drivers/gpu/vga/vga_switcheroo.c                 | 119
++++++++++++++++++++++-
>> drivers/platform/x86/apple-gmux.c                | 111
++++++++++++++++-----
>> include/drm/drm_crtc.h                           |   2 +
>> include/linux/apple-gmux.h                       |  39 ++++++++
>> include/linux/vga_switcheroo.h                   |  36 ++++++-
>> 16 files changed, 382 insertions(+), 33 deletions(-)
>> create mode 100644 include/linux/apple-gmux.h
>> 
>> -- 
>> 1.8.5.2 (Apple Git-48)
>>

On Mon, Jan 11, 2016 at 08:09:20PM +0100, Lukas Wunner
wrote:
> Enable GPU switching on the pre-retina MacBook Pro (2008 - 2013), v5.
> 
> The main obstacle on these machines is that the panel mode in VBIOS
> is bogus. Fortunately gmux can switch DDC independently from the
> display, thereby allowing the inactive GPU to probe the panel's EDID.
> 
> In short, vga_switcheroo and apple-gmux are amended with hooks to
> switch DDC, DRM core is amended with a drm_get_edid_switcheroo() helper,
> and relevant drivers are amended to call that for LVDS outputs.
> 
> The retina MacBook Pro (2012 - present) uses eDP and cannot switch
> AUX independently from the main link. The main obstacle there is link
> training, I'm currently working on this, it will be addressed in a
> future patch set.
> 
> This series is also reviewable on GitHub:
> https://github.com/l1k/linux/commits/mbp_switcheroo_v5
> 
> Changes:
> 
> * New patch [01/12]: vga_switcheroo handler flags
>   Alex Deucher asked if this series might regress on non-Apple laptops.
>   To address this concern, I let handlers declare their capabilities in
>   a bitmask. DRM drivers call drm_get_edid_switcheroo() only if the
>   handler has set the VGA_SWITCHEROO_CAN_SWITCH_DDC flag.
>   Currently just one other flag is defined which is used on retinas.
> 
> * Changed patch [02/12]: vga_switcheroo DDC locking
>   Rename ddc_lock to mux_hw_lock, suggested by Daniel Vetter.
> 
> * New patch [03/12]: track switch state of apple-gmux
>   Fixes a bug in previous versions of this series which occurred if
>   the system was suspended while DDC was temporarily switched:
>   On resume DDC was switched to the wrong GPU.
> 
> * New patches [09/12 - 12/12]: deferred probing
>   Previously I used connector reprobing if the inactive GPU's driver
>   loaded before gmux. I've ditched that in favor of deferred driver
>   probing, which is much simpler. Thanks to Daniel Vetter for the
>   suggestion.
> 
> Caution: Patch [09/12] depends on a new acpi_dev_present() API which
> will land in 4.5 via Rafael J. Wysocki's tree.
> 
> I would particularly be interested in feedback on the handler flags
> patch [01/12]. I'm not 100% happy with the number of characters
> required to query the flags (e.g.: if (vga_switcheroo_handler_flags() &
> VGA_SWITCHEROO_CAN_SWITCH_DDC)), but failed to come up with something
> shorter. Thierry Reding used a struct of bools instead of a bitmask
> for his recent drm_dp_link_caps patches. Maybe use that instead?
> http://lists.freedesktop.org/archives/dri-devel/2015-December/097025.html
No objection from the platform-driver-x86 side. I can pull these separately once
the core is in, or these can be included with that core (preferred) with my
Reviewed-by for 1, 3, 4, and 9.

Reviewed-by: Darren Hart <dvhart at linux.intel.com>

-- 
Darren Hart
Intel Open Source Technology Center

On Mon, Feb 08, 2016 at 10:10:00AM -0800, Darren Hart
wrote:
> On Mon, Jan 11, 2016 at 08:09:20PM +0100, Lukas Wunner wrote:
> > Enable GPU switching on the pre-retina MacBook Pro (2008 - 2013), v5.
> > 
> > The main obstacle on these machines is that the panel mode in VBIOS
> > is bogus. Fortunately gmux can switch DDC independently from the
> > display, thereby allowing the inactive GPU to probe the panel's
EDID.
> > 
> > In short, vga_switcheroo and apple-gmux are amended with hooks to
> > switch DDC, DRM core is amended with a drm_get_edid_switcheroo()
helper,
> > and relevant drivers are amended to call that for LVDS outputs.
> > 
> > The retina MacBook Pro (2012 - present) uses eDP and cannot switch
> > AUX independently from the main link. The main obstacle there is link
> > training, I'm currently working on this, it will be addressed in a
> > future patch set.
> > 
> > This series is also reviewable on GitHub:
> > https://github.com/l1k/linux/commits/mbp_switcheroo_v5
> > 
> > Changes:
> > 
> > * New patch [01/12]: vga_switcheroo handler flags
> >   Alex Deucher asked if this series might regress on non-Apple
laptops.
> >   To address this concern, I let handlers declare their capabilities
in
> >   a bitmask. DRM drivers call drm_get_edid_switcheroo() only if the
> >   handler has set the VGA_SWITCHEROO_CAN_SWITCH_DDC flag.
> >   Currently just one other flag is defined which is used on retinas.
> > 
> > * Changed patch [02/12]: vga_switcheroo DDC locking
> >   Rename ddc_lock to mux_hw_lock, suggested by Daniel Vetter.
> > 
> > * New patch [03/12]: track switch state of apple-gmux
> >   Fixes a bug in previous versions of this series which occurred if
> >   the system was suspended while DDC was temporarily switched:
> >   On resume DDC was switched to the wrong GPU.
> > 
> > * New patches [09/12 - 12/12]: deferred probing
> >   Previously I used connector reprobing if the inactive GPU's
driver
> >   loaded before gmux. I've ditched that in favor of deferred
driver
> >   probing, which is much simpler. Thanks to Daniel Vetter for the
> >   suggestion.
> > 
> > Caution: Patch [09/12] depends on a new acpi_dev_present() API which
> > will land in 4.5 via Rafael J. Wysocki's tree.
> > 
> > I would particularly be interested in feedback on the handler flags
> > patch [01/12]. I'm not 100% happy with the number of characters
> > required to query the flags (e.g.: if (vga_switcheroo_handler_flags()
&
> > VGA_SWITCHEROO_CAN_SWITCH_DDC)), but failed to come up with something
> > shorter. Thierry Reding used a struct of bools instead of a bitmask
> > for his recent drm_dp_link_caps patches. Maybe use that instead?
> >
http://lists.freedesktop.org/archives/dri-devel/2015-December/097025.html
> 
> No objection from the platform-driver-x86 side. I can pull these separately
once
> the core is in, or these can be included with that core (preferred) with my
> Reviewed-by for 1, 3, 4, and 9.
> 
> Reviewed-by: Darren Hart <dvhart at linux.intel.com>
I pulled them all in through drm-misc, thanks.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch