[v5,10/12] drm/i915: Defer probe if gmux is present but its driver isn't
diff mbox

Message ID f56ee6a0600a3e1bb5bed4d0db4ed9ade7445c47.1452525860.git.lukas@wunner.de
State New
Headers show

Commit Message

Lukas Wunner Jan. 11, 2016, 7:09 p.m. UTC
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@wunner.de>
    [MBP  9,1 2012  intel IVB + nvidia GK107  pre-retina  15"]
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/gpu/drm/i915/i915_drv.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Comments

Daniel Vetter Feb. 9, 2016, 9:04 a.m. UTC | #1
On Mon, Jan 11, 2016 at 08:09:20PM +0100, Lukas Wunner wrote:
> 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@wunner.de>
>     [MBP  9,1 2012  intel IVB + nvidia GK107  pre-retina  15"]
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> ---
>  drivers/gpu/drm/i915/i915_drv.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 3ac616d..4a5fc5d 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -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))
>  		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?

Thanks, Daniel

> +
>  	return drm_get_pci_dev(pdev, ent, &driver);
>  }
>  
> -- 
> 1.8.5.2 (Apple Git-48)
>

Patch
diff mbox

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 3ac616d..4a5fc5d 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -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))
 		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;
+
 	return drm_get_pci_dev(pdev, ent, &driver);
 }