Enable GPU switching on pre-retina MacBook Pro

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 <[email protected]>

    [MBP  9,1 2012  intel IVB + nvidia GK107  pre-retina  15"]

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

---
 drivers/gpu/drm/i915/i915_drv.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

drivers/gpu/drm/i915/i915_drv.c +3 -0

index 3ac616d..4a5fc5d 100644
@@ -35,9 +35,12 @@
 #include "i915_trace.h"
 #include "intel_drv.h"
 
+#include <linux/apple-gmux.h>
 #include <linux/console.h>
 #include <linux/module.h>
 #include <linux/pm_runtime.h>
+#include <linux/vgaarb.h>
+#include <linux/vga_switcheroo.h>
 #include <drm/drm_crtc_helper.h>
 
 static struct drm_driver driver;

@@ -967,6 +970,15 @@ static int i915_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (PCI_FUNC(pdev->devfn))

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

+
 	return drm_get_pci_dev(pdev, ent, &driver);
 }
 

Signature · 1 line

1.8.5.2 (Apple Git-48)

On Mon, Jan 11, 2016 at 08:09:20PM +0100, Lukas Wunner wrote:

Review static int i915_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) · line 980

Quoted 12 lines

(quoted diff) +8 -0

@@ -967,6 +970,15 @@ static int i915_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
    if (PCI_FUNC(pdev->devfn))
            return -ENODEV;
 
+   /*
+    * 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;

I pulled in all patches to drm-misc, but this here is imo ugly and needs to be polished a bit. What about adding a vga_switcheroo_ready() function which contains this check (and might in the future contain even more checks)? Then i915/radeon/nouveau would just have a simple

	if (!vga_switcheroo_ready())
		return -EPROBE_DEFER;

and instead of duplicating the same comment 3 times we could have it once in one place. Plus some neat kerneldoc for this new helper to describe how it's supposed to be used. Plus better encapsulation of concepts.

Can you pls follow up with a patch/series to do that?

/**
+ * vga_switcheroo_client_probe_defer() - whether to defer probing a given GPU
+ * @pdev: pci device of GPU
+ *
+ * Determine whether any prerequisites are not fulfilled to probe a given GPU.

+ * DRM drivers should invoke this early on in their ->probe callback and return
+ * %-EPROBE_DEFER if it evaluates to %true. The GPU need not be registered with
+ * vga_switcheroo_register_client() beforehand.

+ *
+ * Return: %false unless one of the following applies:
+ *
+ * * On pre-retina MacBook Pros, the apple-gmux driver is needed to temporarily

+ *   switch DDC to the inactive GPU so that it can probe the panel's EDID.
+ *   Therefore return %true if gmux is built into the machine, @pdev is the
+ *   inactive GPU and the apple-gmux driver has not registered its handler
+ *   flags, signifying it has not yet loaded or has not finished initializing.

+ */

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

+bool vga_switcheroo_client_probe_defer(struct pci_dev *pdev)
+{
+       if (apple_gmux_present() && pdev != vga_default_device() &&
+           !vgasr_priv.handler_flags)
+               return true;
+
+       return false;
+}
+EXPORT_SYMBOL(vga_switcheroo_client_probe_defer);
+

Quoted 5 lines · Lukas Wunner

/**
+ * vga_switcheroo_client_probe_defer() - whether to defer probing a given GPU
+ * @pdev: pci device of GPU
+ *
+ * Determine whether any prerequisites are not fulfilled to probe a given GPU.

I'd phrase this as "Determine whether the vgaswitcheroor support is fully loaded" and drop the GPU part - it could be needed by snd drivers eventually too.

Quoted 3 lines

+ * DRM drivers should invoke this early on in their ->probe callback and return
+ * %-EPROBE_DEFER if it evaluates to %true. The GPU need not be registered with
+ * vga_switcheroo_register_client() beforehand.

s/need not/must not/ ... is your native language German by any chance?

Aside from that ... should vga_switcheroo_register_client call this helper instead directly and return the appropriate -EDEFER_PROBE error? I realize for some drivers it might be way too late to unrol from that point on, but e.g. i915 already uses -EDEFER_PROBE. And calling it unconditionally will make sure that it's easier to notice when people forget this. So maybe call it both from the register functions, and export as a separate hook? And for i915 we should be able to remove that early check entirely.

Quoted 8 lines

+ *
+ * Return: %false unless one of the following applies:
+ *
+ * * On pre-retina MacBook Pros, the apple-gmux driver is needed to temporarily

+ *   switch DDC to the inactive GPU so that it can probe the panel's EDID.
+ *   Therefore return %true if gmux is built into the machine, @pdev is the
+ *   inactive GPU and the apple-gmux driver has not registered its handler
+ *   flags, signifying it has not yet loaded or has not finished initializing.

I think the apple-specific comment here should be a separate comment right above the if condition below - it doesn't explain the interface, only one specific case. And we might grow more of those.

Hi,

Quoted 10 lines · Daniel Vetter

Quoted 5 lines · Lukas Wunner

/**
+ * vga_switcheroo_client_probe_defer() - whether to defer probing a given GPU
+ * @pdev: pci device of GPU
+ *
+ * Determine whether any prerequisites are not fulfilled to probe a given GPU.

I'd phrase this as "Determine whether the vgaswitcheroor support is fully loaded" and drop the GPU part - it could be needed by snd drivers eventually too.

Ok, I've rephrased the kerneldoc to refer to "client" instead of "GPU" and moved the single existing check in an if block for PCI_CLASS_DISPLAY_VGA devices.

Quoted 5 lines

Quoted 3 lines

+ * DRM drivers should invoke this early on in their ->probe callback and return
+ * %-EPROBE_DEFER if it evaluates to %true. The GPU need not be registered with
+ * vga_switcheroo_register_client() beforehand.

s/need not/must not/ ... is your native language German by any chance?

In principle there's no harm in registering the client first, then checking if probing should be deferred, as long as the client is unregistered before deferring. Thus the language above is intentionally "need not" (muss nicht) rather than "must not" (darf nicht). I didn't want to mandate something that isn't actually required. The above sentence is merely an aid for driver developers who might be confused in which order to call what.

Quoted 8 lines

Aside from that ... should vga_switcheroo_register_client call this helper instead directly and return the appropriate -EDEFER_PROBE error? I realize for some drivers it might be way too late to unrol from that point on, but e.g. i915 already uses -EDEFER_PROBE. And calling it unconditionally will make sure that it's easier to notice when people forget this. So maybe call it both from the register functions, and export as a separate hook? And for i915 we should be able to remove that early check entirely.

I'm afraid that wouldn't be a good idea. The ->probe hook is potentially called dozens of times during boot until it finally succeeds and vga_switcheroo_register_client() is usually called in a fairly late driver load stage. In i915, we already have a working GEM at that point and an almost fully brought up GPU. Repeating bring up and tear down dozens of times is a nice stress test but nothing I'd inflict on production systems. I imagine it would also severely impact boot time and clutter the kernel log.

So a separate helper seems to be the only sensible option.

Quoted 9 lines

Quoted 8 lines

+ *
+ * Return: %false unless one of the following applies:
+ *
+ * * On pre-retina MacBook Pros, the apple-gmux driver is needed to temporarily

+ *   switch DDC to the inactive GPU so that it can probe the panel's EDID.
+ *   Therefore return %true if gmux is built into the machine, @pdev is the
+ *   inactive GPU and the apple-gmux driver has not registered its handler
+ *   flags, signifying it has not yet loaded or has not finished initializing.

Quoted 3 lines

I think the apple-specific comment here should be a separate comment right above the if condition below - it doesn't explain the interface, only one specific case. And we might grow more of those.

Ok, I've moved that to a comment.

Updated patch follows after the scissors & perforation.

Thanks for the feedback!

Lukas

-- >8 --
Subject: [PATCH] vga_switcheroo: Allow clients to determine whether to defer
probing

So far we've got one condition when DRM drivers need to defer probing on a dual GPU system and it's coded separately into each of the relevant drivers. As suggested by Daniel Vetter, deduplicate that code in the drivers and move it to a new vga_switcheroo helper. This yields better encapsulation of concepts and lets us add further checks in a central place. (The existing check pertains to pre-retina MacBook Pros and an additional check is expected to be needed for retinas.)

v2: This helper could eventually be used by audio clients as well,
so rephrase kerneldoc to refer to "client" instead of "GPU" and move the single existing check in an if block specific to PCI_CLASS_DISPLAY_VGA devices. Move documentation on that check from kerneldoc to a comment. (Daniel Vetter)

Cc Daniel Vetter <[email protected]>

Cc Ben Skeggs <[email protected]>

Cc Alex Deucher <[email protected]>

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

---
 drivers/gpu/drm/i915/i915_drv.c       | 10 +---------
 drivers/gpu/drm/nouveau/nouveau_drm.c | 10 +---------
 drivers/gpu/drm/radeon/radeon_drv.c   | 10 +---------
 drivers/gpu/vga/vga_switcheroo.c      | 28 ++++++++++++++++++++++++++++
 include/linux/vga_switcheroo.h        |  2 ++
 5 files changed, 33 insertions(+), 27 deletions(-)

drivers/gpu/drm/i915/i915_drv.c +1 -9

index e8d0f17..01ef24a 100644
@@ -35,11 +35,9 @@
 #include "i915_trace.h"
 #include "intel_drv.h"
 
-#include <linux/apple-gmux.h>
 #include <linux/console.h>
 #include <linux/module.h>
 #include <linux/pm_runtime.h>
-#include <linux/vgaarb.h>
 #include <linux/vga_switcheroo.h>
 #include <drm/drm_crtc_helper.h>
 

@@ -970,13 +968,7 @@ static int i915_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (PCI_FUNC(pdev->devfn))
 		return -ENODEV;
 
-	/*
-	 * 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())
+	if (vga_switcheroo_client_probe_defer(pdev))
 		return -EPROBE_DEFER;
 
 	return drm_get_pci_dev(pdev, ent, &driver);

drivers/gpu/drm/nouveau/nouveau_drm.c +1 -9

index bb8498c..9141bcd 100644
@@ -22,13 +22,11 @@
  * 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"

@@ -314,13 +312,7 @@ 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())
+	if (vga_switcheroo_client_probe_defer(pdev))
 		return -EPROBE_DEFER;
 
 	/* remove conflicting drivers (vesafb, efifb etc) */

drivers/gpu/drm/radeon/radeon_drv.c +1 -9

index cad2555..7be0c38 100644
@@ -34,11 +34,9 @@
 #include "radeon_drv.h"
 
 #include <drm/drm_pciids.h>
-#include <linux/apple-gmux.h>
 #include <linux/console.h>
 #include <linux/module.h>
 #include <linux/pm_runtime.h>
-#include <linux/vgaarb.h>
 #include <linux/vga_switcheroo.h>
 #include <drm/drm_gem.h>
 

@@ -321,13 +319,7 @@ static int radeon_pci_probe(struct pci_dev *pdev,
 {
 	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())
+	if (vga_switcheroo_client_probe_defer(pdev))
 		return -EPROBE_DEFER;
 
 	/* Get rid of things like offb */

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

index 56287ae..6108ebe 100644
@@ -30,6 +30,7 @@
 
 #define pr_fmt(fmt) "vga_switcheroo: " fmt
 
+#include <linux/apple-gmux.h>
 #include <linux/console.h>
 #include <linux/debugfs.h>
 #include <linux/fb.h>

@@ -375,6 +376,33 @@ find_active_client(struct list_head *head)
 }
 
 /**
+ * vga_switcheroo_client_probe_defer() - whether to defer probing a given client
+ * @pdev: client pci device
+ *
+ * Determine whether any prerequisites are not fulfilled to probe a given
+ * client. Drivers should invoke this early on in their ->probe callback
+ * and return %-EPROBE_DEFER if it evaluates to %true. The client need
+ * not be registered with vga_switcheroo_register_client() beforehand.
+ *
+ * Return: %true if probing should be deferred, otherwise %false.
+ */
+bool vga_switcheroo_client_probe_defer(struct pci_dev *pdev)
+{
+	if ((pdev->class >> 8) == PCI_CLASS_DISPLAY_VGA) {
+		/*
+		 * apple-gmux is needed on pre-retina MacBook Pro
+		 * to probe the panel if pdev is the inactive GPU.
+		 */
+		if (apple_gmux_present() && pdev != vga_default_device() &&
+		    !vgasr_priv.handler_flags)
+			return true;
+	}
+
+	return false;
+}
+EXPORT_SYMBOL(vga_switcheroo_client_probe_defer);
+
+/**
  * vga_switcheroo_get_client_state() - obtain power state of a given client
  * @pdev: client pci device
  *

include/linux/vga_switcheroo.h +2 -0

index b39a5f3..960bedb 100644
@@ -165,6 +165,7 @@ int vga_switcheroo_unlock_ddc(struct pci_dev *pdev);
 
 int vga_switcheroo_process_delayed_switch(void);
 
+bool vga_switcheroo_client_probe_defer(struct pci_dev *pdev);
 enum vga_switcheroo_state vga_switcheroo_get_client_state(struct pci_dev *dev);
 
 void vga_switcheroo_set_dynamic_switch(struct pci_dev *pdev, enum vga_switcheroo_state dynamic);

@@ -188,6 +189,7 @@ static inline enum vga_switcheroo_handler_flags_t vga_switcheroo_handler_flags(v
 static inline int vga_switcheroo_lock_ddc(struct pci_dev *pdev) { return -ENODEV; }
 static inline int vga_switcheroo_unlock_ddc(struct pci_dev *pdev) { return -ENODEV; }
 static inline int vga_switcheroo_process_delayed_switch(void) { return 0; }
+static inline bool vga_switcheroo_client_probe_defer(struct pci_dev *pdev) { return false; }
 static inline enum vga_switcheroo_state 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 · 1 line

1.8.5.2 (Apple Git-48)

Quoted 8 lines · Lukas Wunner

Quoted 6 lines · Daniel Vetter

Quoted 3 lines · Lukas Wunner

+ * DRM drivers should invoke this early on in their ->probe callback and return
+ * %-EPROBE_DEFER if it evaluates to %true. The GPU need not be registered with
+ * vga_switcheroo_register_client() beforehand.

s/need not/must not/ ... is your native language German by any chance?

Quoted 8 lines

In principle there's no harm in registering the client first, then checking if probing should be deferred, as long as the client is unregistered before deferring. Thus the language above is intentionally "need not" (muss nicht) rather than "must not" (darf nicht). I didn't want to mandate something that isn't actually required. The above sentence is merely an aid for driver developers who might be confused in which order to call what.

I'd reject any driver that does this, registering, then checking, then unregistering seems extermely unsafe. I'd really stick with mandatory language here to make this clear.

Ok, makes sense. I still think adding the check to the client_register function would be good, just as a safety measure.

Quoted 3 lines

Ok, makes sense. I still think adding the check to the client_register function would be good, just as a safety measure.

Hm, the idea of calling vga_switcheroo_client_probe_defer() twice causes me pain in the stomach. It's surprising for drivers which just don't need it at the moment (amdgpu and snd_hda_intel) and it feels like overengineering and pampering driver developers beyond reasonable measure. Also while the single existing check is cheap, we might later on add checks that take more time and slow things down.

Quoted 11 lines · Lukas Wunner

Quoted 3 lines

Ok, makes sense. I still think adding the check to the client_register function would be good, just as a safety measure.

Hm, the idea of calling vga_switcheroo_client_probe_defer() twice causes me pain in the stomach. It's surprising for drivers which just don't need it at the moment (amdgpu and snd_hda_intel) and it feels like overengineering and pampering driver developers beyond reasonable measure. Also while the single existing check is cheap, we might later on add checks that take more time and slow things down.

It's motivated by Rusty's API Manifesto:

http://sweng.the-davies.net/Home/rustys-api-design-manifesto

With the mandatory check in _register we reach level 5 - it'll blow up at runtime when we try to register it.

For more context: We have tons of fun with EPROBE_DEFER handling between i915 and snd-hda

Hi,

Quoted 13 lines · Daniel Vetter

Quoted 11 lines · Lukas Wunner

Quoted 3 lines

Ok, makes sense. I still think adding the check to the client_register function would be good, just as a safety measure.

Hm, the idea of calling vga_switcheroo_client_probe_defer() twice causes me pain in the stomach. It's surprising for drivers which just don't need it at the moment (amdgpu and snd_hda_intel) and it feels like overengineering and pampering driver developers beyond reasonable measure. Also while the single existing check is cheap, we might later on add checks that take more time and slow things down.

Quoted 3 lines

It's motivated by Rusty's API Manifesto:

http://sweng.the-davies.net/Home/rustys-api-design-manifesto

Interesting, thank you.

Quoted 2 lines

With the mandatory check in _register we reach level 5 - it'll blow up at runtime when we try to register it.

The manifesto says "5. Do it right or it will always break at runtime".

However even if we add a WARN_ON(vga_switcheroo_client_probe_defer(pdev)) to register_client(), it will not *always* spew a stacktrace but only on the machines this concerns (MacBook Pros). Since failure to defer breaks GPU switching, level 5 is already reached. Chances are this won't go unnoticed by the user.

Quoted 2 lines

For more context: We have tons of fun with EPROBE_DEFER handling between i915 and snd-hda

I don't understand, there is currently not a single occurrence of EPROBE_DEFER in i915, apart from the one I added.

In sound/ there are 88 occurrences of EPROBE_DEFER in soc/, plus 1 in ppc/ and that's it. So not a single one in pci/hda/ where hda_intel.c resides.

Is the fun with EPROBE_DEFER handling caused by the lack thereof?

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Daniel Vetter
Sent: Thursday, February 18, 2016 6:11 PM
To: Lukas Wunner
Cc: dri-devel; [email protected]; intel-gfx; Ben Skeggs; Deucher, Alexander
Subject: Re: [PATCH v5 10/12] drm/i915: Defer probe if gmux is present but its driver isn't

Quoted 4 lines · Lukas Wunner

Hi,

Quoted 1 line · Daniel Vetter

On Thu, Feb 18, 2016 at 9:34 PM, Lukas Wunner <[email protected]>

Quoted 19 lines · wrote:

Quoted 12 lines

Quoted 11 lines

Quoted 3 lines

Ok, makes sense. I still think adding the check to the client_register function would be good, just as a safety measure.

Hm, the idea of calling vga_switcheroo_client_probe_defer() twice causes me pain in the stomach. It's surprising for drivers which just don't need it at the moment (amdgpu and snd_hda_intel) and it feels like overengineering and pampering driver developers beyond reasonable measure. Also while the single existing check is cheap, we might later on add checks that take more time and slow things down.

Quoted 3 lines

It's motivated by Rusty's API Manifesto:

http://sweng.the-davies.net/Home/rustys-api-design-manifesto

Interesting, thank you.

Quoted 6 lines

Quoted 2 lines

With the mandatory check in _register we reach level 5 - it'll blow up at runtime when we try to register it.

The manifesto says "5. Do it right or it will always break at runtime".

However even if we add a

WARN_ON(vga_switcheroo_client_probe_defer(pdev))

Quoted 4 lines

to register_client(), it will not *always* spew a stacktrace but only on the machines this concerns (MacBook Pros). Since failure to defer breaks GPU switching, level 5 is already reached. Chances are this won't go unnoticed by the user.

If we fail the register hopefully the driver checks for that and might blow up somewhere in untested error handling code. But there's a good chance it'll fail (we can encourage that more by adding must_check to the function declaration). In that case you get a nice bug report with splat from users hitting this.

Without this it'll silently work, and all the reports you get is "linux is shit, gpu switching doesn't work".

In both cases it can sometimes succeed, which is not great indeed. But I'm trying to fix that by injection EDEFER points artificially somehow. Not yet figured out that one.

But irrespective of the precise failure mode making the defer check mandatory by just including it in _register() is better since it makes it impossible to forget to call it when its needed. So imo clearly the more robust API. And that's my metric for evaluating new API - how easy/hard is it to abuse/get wrong.

Quoted 10 lines

Quoted 2 lines

For more context: We have tons of fun with EPROBE_DEFER handling between i915 and snd-hda

I don't understand, there is currently not a single occurrence of EPROBE_DEFER in i915, apart from the one I added.

In sound/ there are 88 occurrences of EPROBE_DEFER in soc/, plus 1 in ppc/ and that's it. So not a single one in pci/hda/ where hda_intel.c resides.

Quoted 1 line

Is the fun with EPROBE_DEFER handling caused by the lack thereof?

Yes, there's one instance where i915 shoudl defer missing. The real trouble is that snd-hda has some really close ties with i915, and resolves those with probe-defer. And blows up all the time since we started using this, and with hdmi/dp you really always have to test both together in CI, snd-hda is pretty much a part of the intel gfx driver nowadays. Deferred probing is ime real trouble.

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

Signature · 1 line

1.8.5.2 (Apple Git-48)

Hi,

Quoted 1 line · Lukas Wunner

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

This series hasn't seen any reviews or acks unfortunately. Any takers?

Quoted 5 lines · Lukas Wunner

Hi,

Quoted 1 line · Lukas Wunner

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

Quoted 2 lines

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?

Quoted 5 lines · Lukas Wunner

Quoted 1 line · Lukas Wunner

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.

Hi,

Quoted 2 lines · Lukas Wunner

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 <[email protected]>

[MBP 5,3 2009 nvidia MCP79 + G96 pre-retina 15"]

Tested-by Paul Hordiienko <[email protected]>

[MBP 6,2 2010 intel ILK + nvidia GT216 pre-retina 15"]

Tested-by William Brown <[email protected]>

[MBP 8,2 2011 intel SNB + amd turks pre-retina 15"]

Tested-by Lukas Wunner <[email protected]>

[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

Quoted 92 lines

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

Signature · 1 line

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

Quoted 44 lines · Lukas Wunner

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.

Quoted 7 lines

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 <[email protected]>

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

Quoted 1 line · Lukas Wunner <lukas <at> wunner.de> writes:

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

I've tested your patchset on a MacBookPro8,1, with an integrated Intel and discrete AMD/ATI GPUs.

I've used the COPR repository here to cut down on my compilation time:
https://copr.fedorainfracloud.org/coprs/firstyear/kernel-mbp/

I'm not certain how to test out your changes, or what the consequences should be on a stock Fedora 23/GNOME 3.18 installation. After booting (note that I did not change any command-line options in grub), a gnome-shell/gdm X11 session comes up (I disabled Wayland, to rule out behavioural changes), I'd log in to GNOME and gnome-shell (which starts another X11 session on another VT).

I did not use any external screens to test this.

Hi Bastien,

Quoted 3 lines · Bastien Nocera

Quoted 1 line · Lukas Wunner <lukas <at> wunner.de> writes:

Quoted 5 lines

Quoted 2 lines

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

I've tested your patchset on a MacBookPro8,1, with an integrated Intel and
discrete AMD/ATI GPUs.

Hm, it must be either an 8,2 or 8,3. The 8,1 was a 13" machine and only
had an integrated GPU.

Quoted 5 lines

I've used the COPR repository here to cut down on my compilation time:
https://copr.fedorainfracloud.org/coprs/firstyear/kernel-mbp/

Quoted 11 lines

I'm not certain how to test out your changes, or what the consequences should
be on a stock Fedora 23/GNOME 3.18 installation. After booting (note that I
did not change any command-line options in grub), a gnome-shell/gdm X11
session comes up (I disabled Wayland, to rule out behavioural changes), I'd
log in to GNOME and gnome-shell (which starts another X11 session on
another VT).

Switching and power control currently requires manual intervention by echoing commands to /sys/kernel/debug/vgaswitcheroo/switch as documented here:
https://01.org/linuxgraphics/gfx-docs/drm/modes_of_use.html

As you've correctly observed, the machine is initially switched to the discrete GPU and both GPUs are turned on. By echoing "IGD" to the sysfs file, you'll switch to the integrated GPU and turn off the discrete GPU.

It's possible to let the EFI firmware switch to the integrated GPU on boot by using this tool: https://github.com/0xbb/gpu-switch
However still both GPUs will be powered up, so you have to issue the "OFF" command to sysfs to power the discrete GPU down. Also, once you boot into OS X, the setting made by the gpu-switch tool will be overwritten and the machine will be switched to the discrete GPU again the next time you boot Linux.

Note that switching is only possible from the text console, with X11/Wayland shut down. Obviously this is not great in terms of UX. A few years ago there was a GSoC proposal to get hot GPU switching to work on Linux (akin to what OS X does) but nothing ever came of it:
http://www.phoronix.com/scan.php?page=news_item&px=OTIyMQ
https://lists.x.org/archives/xorg/2011-March/052522.html

Unfortunately this seems to be a low priority item for kernel graphics
developers since nowadays most dual GPU notebooks no longer have a mux
and cannot switch. The MacBook Pro seems to be the last one supporting
this but I've witnessed a bit of an anti-Apple sentiment among kernel graphics developers since everything is non-standard there. Which is unfortunate because these machines have a large market share and Apple
software quality is deteriorating rapidly so a lot of Mac users are ripe for converting to Linux.

Anyway, one short-term improvement will be to add runtime pm support (called "Driver power control" in the vga_switcheroo documentation linked above). That way it'll no longer be necessary to power the discrete GPU up and down manually, this will happen automatically as needed (when switching or using render offloading with DRI PRIME).

I have patches to enable this for radeon but they're completely untested:

http://wunner.de/mbp_switcheroo_v5-4.5.tar.gz       => gpu switching for 4.5

http://wunner.de/mbp_switcheroo_v5-4.5-runpm.tar.gz => runtime pm for radeon

I have an Nvidia based machine and runtime pm doesn't work there yet because of bugs in nouveau that I haven't had the time to look into.

Quoted 2 lines

I did not use any external screens to test this.

Since your machine has Thunderbolt, the external port is no longer switchable between GPUs, it can only be driven by the discrete GPU. So you need to power it up manually for this to work. You don't need to switch to it, but it's probably recommendable to save energy. (Otherwise both GPUs are on with the integrated GPU driving the panel and the discrete GPU driving the DP port.)

The runpm tarball linked above contains a patch to automatically wake the discrete GPU on hotplug.

I've heard that the AMD GPU is picky about external monitors and doesn't recognize them unless they're plugged in at exactly the right moment, so you may need to retry a couple of times until it works.

To sum up, and if we take the above patches into consideration:
- on boot, one or more GPUs are powered on, an init script would queue a GPU switch to the integrated one. The other GPU would be switched off (automatically?)
- when X/wayland is running, queue the requests using DIGD or DDIS. If the integrated GPU is used, allow offloading to the discrete GPU with DRI_PRIME (which will again power up automatically thanks to the runtime PM patches above).

My concerns here would be:
- Do we know how to turn off the integrated GPU automatically, if the user only used the internal screen and wanted to use the discrete GPU?
- If only the discrete GPU is powered on, do we know how to power on the integrated GPU so it can drive the external screen when plugged in?
- While plymouth is short-lived at boot, Wayland and X11 GNOME sessions use the GPU. The first user session will run on a VT that's separate from the greeter to implement fast-user switching. So, if we wanted to force using the other GPU for future sessions, we'd need to tell gdm to kill its graphical session and to respawn it so it doesn't hog the GPU and avoid the switch happening. Correct?

FWIW, this is what I had written down a couple of months ago, about supporting dual-GPU computers with GNOME:
https://wiki.gnome.org/Design/OS/DualGPU

Those use-cases are a lot simpler than what could be achieved with the vga_switcheroo sub-system, but they probably cover the "90%" use cases we're interested in.

Once I've managed to get the MacBook Pro into a good state, I also have a Lenovo machine around with dual GPU, though I couldn't tell you what the discrete one is.

Okay so I'm not sure you are heading in the best direction here.

My first suggestion is to stop using the MBP, start using the Lenovo. At least from a Fedora perspective, that is the hw we have more installs of and
care more about.

Apple HW is not the same as PC hw in this case and we aren't going to achieve
the same level of integration that OSX has, not without some serious rewrites of
mutter and the whole X stack.

You shouldn't be caring about the MUX.

So I'm missing what the overall goal here is. To provide better support for dual-gpu
laptops and hotpluggable USB devices in the DE?

Under X, Fedora carries a server patch to autoconfigure providers, we'd need to drop
that and have something in the DE notice when a new provider shows up and configures it,
perhaps something to allow removal of providers that are already bound
(so we could detach
a secondary GPU for boxes to passthrough).

Then we need something in the DE to allow us to launch or have some app info that would
decide to launch certain 3D using apps on the more powerful processor.

However since
nouveau doesn't quite reclock most of the secondary GPUs that can often end up not being
that much more powerful.

We also want reverse prime to work properly, so if you plug in an external monitor to
a port connected to the secondary GPU that we can pick it up and configure it just like
all the other monitors.

As for the MBP, if we want to spend time chasing the rainbow of OS X, then we've a lot of work
to do. OSX can smoothly switch the compositor from rendering on the intel gpu to the nvidia
gpu in a vblank. It's truly seamless. To do that we'd need to a) move to wayland, b) get mutter
to be a lot smarter than mutter currently is.

Quoted 4 lines

Then we need something in the DE to allow us to launch or have some app info that would
decide to launch certain 3D using apps on the more powerful processor.

That's what I started working on, exporting the fact that 2 GPUs are available through a D-Bus service, which also ensures that we only

We could, on boot, force using the integrated GPU, turning off the discrete GPU that we're not interested in.

So I'd push DIGD to the switch sysfs entry on boot. But I'm guessing that won't turn off the other output we're not interested in.

DIGD and DDIS should help (you need to log out/log in again), and if the current GPU is the integrated one, you could force running, say, a game on the discrete GPU using DRI_PRIME=1, correct?

FWIW, using OFF at runtime made my machine hang, and without any ways for me to get debug output.

Ok, so using GIGD or DDIS would just switch the output, but not turn off the unused GPU without runtime PM management.

Quoted 2 lines

http://wunner.de/mbp_switcheroo_v5-4.5.tar.gz       => gpu switching for 4.5

That's the same patch that's already applied to the kernel above (the ones from this patchset thread), right?

My concerns here would be:
- Do we know how to turn off the integrated GPU automatically, if the user only used the internal screen and wanted to use the discrete GPU?

- If only the discrete GPU is powered on, do we know how to power on the integrated GPU so it can drive the external screen when plugged in?

- While plymouth is short-lived at boot, Wayland and X11 GNOME sessions use the GPU. The first user session will run on a VT that's separate from the greeter to implement fast-user switching. So, if we wanted to force using the other GPU for future sessions, we'd need to tell gdm to kill its graphical session and to respawn it so it doesn't hog the GPU and avoid the switch happening. Correct?

On the 8,2, still, and with the kernel from the COPR repo[1].

I tried running:
echo DIGD > switch

to (later) switch to the integrated GPU. Log out, get to gdm, log back in to a black screen with the cursor visible and nothing else.

I'm wondering if it's the gdm X11 session running in the background that makes this fail.

I'd like to try and get this working properly before bringing the runtime PM support into it.

Hi Bastien,

sorry for the delay.

Quoted 1 line · Bastien Nocera

Quoted 2 lines

We could, on boot, force using the integrated GPU, turning off the discrete GPU that we're not interested in.

Yes, many people "solve" this by having grub write the requisite commands
to gmux' I/O ports, however this approach won't work with gummiboot. Also, the commands that need to be sent to gmux differ between retinas
and pre-retinas.

Quoted 3 lines

So I'd push DIGD to the switch sysfs entry on boot. But I'm guessing
that won't turn off the other output we're not interested in.

IGD and DIGD switch to the integrated GPU and also turn off the discrete
GPU. However if the machine is already switched to the integrated GPU,
the commands are no-ops and the discrete GPU is not turned off.

In other words you need to check (by reading the switch file) which GPU
the machine is switched to. If it's the integrated GPU, write OFF to the
switch file. If it's the discrete GPU, write DIGD to the switch file.

Once runtime pm works, writing DIGD is sufficient.

Quoted 5 lines

DIGD and DDIS should help (you need to log out/log in again), and if
the current GPU is the integrated one, you could force running, say, a
game on the discrete GPU using DRI_PRIME=1, correct?

If runtime pm works, then yes. Otherwise you'd have to manually power up
the GPU before using DRI_PRIME=1 and power it down afterwards.

Quoted 3 lines

FWIW, using OFF at runtime made my machine hang, and without any ways
for me to get debug output.

Which GPU were you switched to? Did you issue the command while X11 was
running? Since the machine is switched to the AMD on boot, I guess you
were powering down the Intel GPU. This works on my machine, I get a log
entry "i915: switched off". If this doesn't work on your machine it might
be an i915 bug on your Sandy Bridge GPU or it might be because X11 is running.

Try booting with "drm.debug=0xf log_buf_len=10M" and see if this gets you
log output. If it doesn't, netconsole might help if the hang occurs while
the console is locked.

Quoted 3 lines

Ok, so using GIGD or DDIS would just switch the output, but not turn
off the unused GPU without runtime PM management.

No. They do switch off the other GPU if runtime pm is disabled.

Quoted 2 lines

Quoted 2 lines

Quoted 6 lines

Quoted 3 lines

http://wunner.de/mbp_switcheroo_v5-4.5.tar.gz       => gpu switching
for 4.5

That's the same patch that's already applied to the kernel above (the
ones from this patchset thread), right?

I'm not sure, the patches in the copr repository might be an earlier test version.

Quoted 5 lines

My concerns here would be:
- Do we know how to turn off the integrated GPU automatically, if the
user only used the internal screen and wanted to use the discrete GPU?

Runtime pm is currently disabled by default for i915. The Intel folks are on it. Until that works, the integrated GPU will be powered down when the user manually switches to the discrete GPU with DIS / DDIS and powered up when switching back with IGD / DIGD.

Quoted 4 lines

- If only the discrete GPU is powered on, do we know how to power on
the integrated GPU so it can drive the external screen when plugged in?

On the MBP5 2008/09 and MBP6 2010, the external DP port can be switched
between GPUs and we switch it together with the panel. So if you switch
to the discrete GPU, you can also drive the external DP port on these models and the integrated GPU can be turned off.

On the MBP8 2011 and newer, external ports are combined DP/Thunderbolt
ports which can only be driven by the discrete GPU. For some reason the
HPD/AUX pins of the ports can still be switched but not the other pins.
We should nail these ports to the discrete GPU and not switch those pins
together with the panel as we currently do. There's a patch in mbp_switcheroo_v5-4.5-runpm.tar.gz which fixes that. The patch also wakes
up the discrete GPU on hotplug, which obviously needs runtime pm.

Quoted 11 lines

- While plymouth is short-lived at boot, Wayland and X11 GNOME sessions
use the GPU. The first user session will run on a VT that's separate
from the greeter to implement fast-user switching. So, if we wanted to
force using the other GPU for future sessions, we'd need to tell gdm to
kill its graphical session and to respawn it so it doesn't hog the GPU
and avoid the switch happening. Correct?

I think so, yes.

Quoted 11 lines

On the 8,2, still, and with the kernel from the COPR repo[1].

I tried running:
echo DIGD > switch

to (later) switch to the integrated GPU. Log out, get to gdm, log back
in to a black screen with the cursor visible and nothing else.

I'm wondering if it's the gdm X11 session running in the background that makes this fail.

That's possible, I have no idea. I have zero issues with GPU switching
on my Nvidia-based MacBook Pro, though I only switch on the console with
no X11 running. On the AMD-based MBP8 that you're using, the patches have
only been tested by William Brown, a colleague of yours at Red Hat Australia.

If you send me dmesg output with "drm.debug=0xf log_buf_len=10M" I can see if I spot any issues.

Quoted 2 lines

I'd like to try and get this working properly before bringing the runtime PM support into it.

Absolutely.

Do you have references for the i915 runtime PM support, a bugzilla or mailing-list thread?

I guess that's only useful until we get runtime PM support.

After looking at our use cases in the GNOME wiki, I think that might not be necessary as we'll want to always run the desktop on the integrated GPU. That'll something to keep in mind if we ever want to

Reading through the whole mail it seems to me that it's close to impossible to implement a decent integration without runtime PM support:
- DRI_PRIME wouldn't work
- no external display detection on some machines

Do you have references for the i915 runtime PM support, a bugzilla or mailing-list thread?

Quoted 3 lines

I guess that's only useful until we get runtime PM support.

For the discrete GPUs on regular laptops we have runtime PM support for
powerdown already. Some newer laptops need a bit of work in the PCIE layer
but for most things we have it covered. The known broken ones are Apple
laptops. If the apple-gmux code is working well enough to power off GPUs,
then it should be possible to hook up runtime-pm on those machines pretty
simply.

So there shouldn't really be a case we care about.

runtime PM for the Intel GPU isn't as important. We don't even want to
turn the i915 fully off anymore.

Quoted 3 lines

So I'd push DIGD to the switch sysfs entry on boot. But I'm guessing
that won't turn off the other output we're not interested in.

IGD and DIGD switch to the integrated GPU and also turn off the discrete
GPU. However if the machine is already switched to the integrated GPU,
the commands are no-ops and the discrete GPU is not turned off.

In other words you need to check (by reading the switch file) which GPU
the machine is switched to. If it's the integrated GPU, write OFF to the
switch file. If it's the discrete GPU, write DIGD to the switch file.

Once runtime pm works, writing DIGD is sufficient.

I tested the runtime patches for Radeon on top of 4.6.0-rc2, and writing DIGD failed. I also saw a number of messages with the vga_switcheroo core in the kernel trying to switch GPUs but failed because "client 1" was keeping it busy.

Is there any way to see what this "client 1" actually is? I'm guessing plymouth. In any case, once I'm logged in, the "DIS" is powered and used, and the IGD is powered as well.

Hi Bastien,

Quoted 3 lines · Bastien Nocera

Quoted 1 line · Lukas Wunner <lukas <at> wunner.de> writes:

Quoted 5 lines

Quoted 2 lines

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

I've tested your patchset on a MacBookPro8,1, with an integrated Intel and
discrete AMD/ATI GPUs.

Hm, it must be either an 8,2 or 8,3. The 8,1 was a 13" machine and only
had an integrated GPU.

Hi Bastien,

Quoted 5 lines · Bastien Nocera

Quoted 1 line · Lukas Wunner <lukas <at> wunner.de> writes:

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

I've tested your patchset on a MacBookPro8,1, with an integrated Intel and discrete AMD/ATI GPUs.

Hm, it must be either an 8,2 or 8,3. The 8,1 was a 13" machine and only had an integrated GPU.

Quoted 9 lines

I've used the COPR repository here to cut down on my compilation time:
https://copr.fedorainfracloud.org/coprs/firstyear/kernel-mbp/

I'm not certain how to test out your changes, or what the consequences should be on a stock Fedora 23/GNOME 3.18 installation. After booting (note that I did not change any command-line options in grub), a gnome-shell/gdm X11 session comes up (I disabled Wayland, to rule out behavioural changes), I'd log in to GNOME and gnome-shell (which starts another X11 session on another VT).

Switching and power control currently requires manual intervention by echoing commands to /sys/kernel/debug/vgaswitcheroo/switch as documented here:
https://01.org/linuxgraphics/gfx-docs/drm/modes_of_use.html

As you've correctly observed, the machine is initially switched to the discrete GPU and both GPUs are turned on. By echoing "IGD" to the sysfs file, you'll switch to the integrated GPU and turn off the discrete GPU.

It's possible to let the EFI firmware switch to the integrated GPU on boot by using this tool: https://github.com/0xbb/gpu-switch
However still both GPUs will be powered up, so you have to issue the "OFF" command to sysfs to power the discrete GPU down. Also, once you boot into OS X, the setting made by the gpu-switch tool will be overwritten and the machine will be switched to the discrete GPU again the next time you boot Linux.

Note that switching is only possible from the text console, with X11/Wayland shut down. Obviously this is not great in terms of UX. A few years ago there was a GSoC proposal to get hot GPU switching to work on Linux (akin to what OS X does) but nothing ever came of it:
http://www.phoronix.com/scan.php?page=news_item&px=OTIyMQ
https://lists.x.org/archives/xorg/2011-March/052522.html

Unfortunately this seems to be a low priority item for kernel graphics developers since nowadays most dual GPU notebooks no longer have a mux and cannot switch. The MacBook Pro seems to be the last one supporting this but I've witnessed a bit of an anti-Apple sentiment among kernel graphics developers since everything is non-standard there. Which is unfortunate because these machines have a large market share and Apple software quality is deteriorating rapidly so a lot of Mac users are ripe for converting to Linux.

Anyway, one short-term improvement will be to add runtime pm support (called "Driver power control" in the vga_switcheroo documentation linked above). That way it'll no longer be necessary to power the discrete GPU up and down manually, this will happen automatically as needed (when switching or using render offloading with DRI PRIME). I have patches to enable this for radeon but they're completely untested:

http://wunner.de/mbp_switcheroo_v5-4.5.tar.gz => gpu switching for 4.5
http://wunner.de/mbp_switcheroo_v5-4.5-runpm.tar.gz => runtime pm for radeon

I have an Nvidia based machine and runtime pm doesn't work there yet because of bugs in nouveau that I haven't had the time to look into.

Quoted 1 line

I did not use any external screens to test this.

Since your machine has Thunderbolt, the external port is no longer switchable between GPUs, it can only be driven by the discrete GPU. So you need to power it up manually for this to work. You don't need to switch to it, but it's probably recommendable to save energy. (Otherwise both GPUs are on with the integrated GPU driving the panel and the discrete GPU driving the DP port.)

The runpm tarball linked above contains a patch to automatically wake the discrete GPU on hotplug.

I've heard that the AMD GPU is picky about external monitors and doesn't recognize them unless they're plugged in at exactly the right moment, so you may need to retry a couple of times until it works.

Is there any reason to make use of the mux?

Quoted 4 lines

I've heard that the AMD GPU is picky about external monitors and doesn't recognize them unless they're plugged in at exactly the right moment, so you may need to retry a couple of times until it works.

Are talking about some issue specific to these muxed apple systems or in general?

If you are having issues, please file a bug.

Hi Alex,

Quoted 1 line · Alex Deucher

Is there any reason to make use of the mux?

Performance (lower latency => no need for framebuffer writes over PCIe), improved battery life (no need to use 2 GPUs simultaneously).

Technically you can't just ignore that the mux is there on the MBP because the kernel has no control over the GPU used on boot. (It's determined by EFI).

Quoted 8 lines

Quoted 5 lines

I've heard that the AMD GPU is picky about external monitors and doesn't recognize them unless they're plugged in at exactly the right moment, so you may need to retry a couple of times until it works.

Are talking about some issue specific to these muxed apple systems or in general?

Feedback I got from William Brown of Red Hat who tested the GPU switching patches on an MBP8,2 and reported that (independently of the patches), a display connected with an original Apple DP-to-DVI adapter would only be recognized if plugged in at exactly the right moment and in the correct order (first adapter, then display). However it doesn't seem to work better on OS X.

Quoted 3 lines

If you are having issues, please file a bug.

I'm not having issues so can't file a bug. Besides, filing a bug is no guarantee that things get fixed. He had opened a bug for GPU switching 3 years ago (https://bugs.freedesktop.org/show_bug.cgi?id=61115) and nobody did a thing. Obviously whether something gets fixed is a function of the perceived importance by maintainers, unless a volunteer comes along and does the dirty work.

Quoted 6 lines · Lukas Wunner

Quoted 1 line · Alex Deucher

Is there any reason to make use of the mux?

Performance (lower latency => no need for framebuffer writes over PCIe), improved battery life (no need to use 2 GPUs simultaneously).

Quoted 4 lines

Technically you can't just ignore that the mux is there on the MBP because the kernel has no control over the GPU used on boot. (It's determined by EFI).

Is GPU power switching also handled by the mux? Is it independent of the display mux?