The following patchset extends vga_switcheroo and related drivers to enable gpu switching on the MacBook Pro.
On MBPs, the panel is not connected to one of the gpus, but to the gmux chip (the "handler" in vga_switcheroo speak). Initially the gmux is switched to the discrete gpu. The integrated i915 gpu is therefore unable to detect the LVDS (or eDP) connector on bootup and switching to the integrated gpu will lead to a black screen. The solution is to temporarily switch the DDC lines to the integrated gpu when reading the EDID, which the gmux is capable of. Lack of support for this in vga_switcheroo is pretty much the only obstacle to get gpu switching to work on MBPs.
Seth Forshee came up with a patchset in fall 2012 but only a portion of it got merged. Matthew Garrett developed this further in 2014: http://www.codon.org.uk/~mjg59/tmp/retina_patches/ http://lists.freedesktop.org/archives/dri-devel/2014-June/060947.html
Matthew Garrett's patches weren't merged either: Dave Airlie responded that he had rewritten Seth Forshee's code in December 2012. Whereas Seth Forshee had used a single function vga_switcheroo_switch_ddc() with a mutex in drm_get_edid() to avoid race conditions, Dave Airlie had moved the mutex to vga_switcheroo.c and used *two* functions vga_switcheroo_lock_ddc() and vga_switcheroo_unlock_ddc(): http://lists.freedesktop.org/archives/dri-devel/2014-June/061629.html http://cgit.freedesktop.org/~airlied/linux/log/?h=switchy-wip
Quoth Dave Airlie: "I can't figure out why I didn't like this, but I rewrote this way back to lock/unlock the ddc lines, bits are contained in this WIP mess. I think I'd prefer something like that otherwise the interface got really ugly."
I basically took Dave Airlies WIP code of December 2012, added all the missing pieces, applied one of Matthew Garrett's patches verbatim and rewrote two others.
This works on a MacBookPro9,1 with Intel HD4000 (Ivy Bridge) and Nvidia GT650M (NVE7 / GK107) plus a "classic" (non-indexed) gmux connected to a 1680x1050 panel with dual channel LVDS (applied on top of drm-next and using nouveau).
Dave Airlie's commit removes a portion of Seth Forshee's code and my commit in turn removes some of Dave Airlie's code. Because this makes reviewing difficult, I'm including below a diff which contains all commits squashed into one. Reviewers may want to look at that diff first instead of the individual commits.
One difficulty one needs to be aware of is that gmux may register after the i915 driver, making it impossible to switch the DDC lines upon i915 initialization. Seth Forshee solved this by deferring initialization of the graphics driver: http://cgit.freedesktop.org/~airlied/linux/commit/?h=switchy-wip&id=f7ac...
Matthew Garrett came up with a simpler approach, he added a callback reprobe_connectors() which gets invoked in vga_switcheroo_enable(): http://www.codon.org.uk/~mjg59/tmp/retina_patches/0014-vga_switcheroo-Add-su...
The latter approach is further refined in the following patchset by invoking the callback as soon as gmux registers, which is earlier than in Matthew Garrett's patch since vga_switcheroo_enable() waits for registration of a second gpu. (It may never register if its driver is blacklisted or not installed.)
The following issues currently remain:
- Sometimes there's flickering when switching, so the user experience is not yet as smooth as on OS X. (On OS X there's never any flickering.)
- If gmux registers *before* the i915 driver, the latter will find the LVDS connector plus a disconnected VGA connector. If gmux registers *after* the i915 driver, the latter initially only finds the VGA connector. Upon reprobe it will find the LVDS connector and *another* (ghost) VGA connector. That's because I call intel_setup_outputs() which probes for all sorts of connectors. Matthew Garrett duplicated that function and reduced it to just the eDP probing portion (which is not sufficient since pre-retina MBPs used LVDS): http://www.codon.org.uk/~mjg59/tmp/retina_patches/0024-i915-Add-support-for-... It would be great if Intel developers could give advice on how to best integrate reprobing with their driver or how detection of ghost VGA connectors can be avoided.
- The Nvidia gpu sometimes gets stuck in powersaving mode and refuses to wake up ("Refused to change power state, currently in D3"). It seems that a delay is necessary between putting the device in D3hot and cutting the power. (On MBPs, power is cut by the gmux.) I added a delay of 200 usec and this seems to work most of the time but not always. My impression is that the required length of the delay is somehow heat related: I stress tested the code by switching back and forth in an endless loop, causing the fans to spin fast as the constant powerup and powerdown seems to draw a lot of energy, and after about 5 minutes of constant switching the Nvidia gpu gets stuck in D3 even if the delay is increased to 2 seconds. Using D3cold instead of D3hot didn't help either.
Kind regards,
Lukas
Dave Airlie (1): vga_switcheroo: Lock/unlock DDC lines
Lukas Wunner (5): vga_switcheroo: Lock/unlock DDC lines harder Revert "vga_switcheroo: Add helper function to get the active client" vga_switcheroo: Reprobe connectors if handler registers late drm/i915: Reprobe connectors if vga_switcheroo handler registers late drm/nouveau: Pause between setting gpu to D3hot and cutting the power
Matthew Garrett (1): apple-gmux: Assign apple_gmux_data before registering
Seth Forshee (4): vga_switcheroo: Add support for switching only the DDC vga_switcheroo: Add helper function to get the active client apple-gmux: Add switch_ddc support drm/edid: Switch DDC when reading the EDID
drivers/gpu/drm/drm_edid.c | 10 +++- drivers/gpu/drm/i915/i915_dma.c | 40 +++++++++++++ drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/intel_display.c | 2 +- drivers/gpu/drm/i915/intel_dp.c | 6 +- drivers/gpu/drm/nouveau/nouveau_drm.c | 2 + drivers/gpu/vga/vga_switcheroo.c | 105 +++++++++++++++++++++++++++++++++- drivers/platform/x86/apple-gmux.c | 35 ++++++++++-- include/linux/vga_switcheroo.h | 7 +++ 9 files changed, 196 insertions(+), 12 deletions(-)
-- >8 --
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 53bc7a6..8950722 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -32,6 +32,7 @@ #include <linux/hdmi.h> #include <linux/i2c.h> #include <linux/module.h> +#include <linux/vga_switcheroo.h> #include <drm/drmP.h> #include <drm/drm_edid.h> #include <drm/drm_displayid.h> @@ -1326,12 +1327,19 @@ struct edid *drm_get_edid(struct drm_connector *connector, { struct edid *edid;
- if (!drm_probe_ddc(adapter)) + vga_switcheroo_lock_ddc(connector->dev->pdev); + + if (!drm_probe_ddc(adapter)) { + vga_switcheroo_unlock_ddc(connector->dev->pdev); return NULL; + }
edid = drm_do_get_edid(connector, drm_do_probe_ddc_edid, adapter); if (edid) drm_get_displayid(connector, edid); + + vga_switcheroo_unlock_ddc(connector->dev->pdev); + return edid; } EXPORT_SYMBOL(drm_get_edid); diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 68e0c85..86726b7 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -379,9 +379,49 @@ static bool i915_switcheroo_can_switch(struct pci_dev *pdev) return dev->open_count == 0; }
+static void i915_switcheroo_reprobe_connectors(struct pci_dev *pdev) +{ + struct drm_device *dev = pci_get_drvdata(pdev); + struct drm_i915_private *dev_priv = dev->dev_private; + struct intel_encoder *encoder; + + /* + * Check whether we've already found a panel. + * If so, we don't need to reprobe + */ + for_each_intel_encoder(dev, encoder) + if (encoder->type == INTEL_OUTPUT_LVDS || + encoder->type == INTEL_OUTPUT_EDP) + return; + + /* + * intel_modeset_gem_init() sets lvds_use_ssc to 0, + * reset to 1 so that the SSC gets used on the panel + */ + dev_priv->vbt.lvds_use_ssc = + !(i915.panel_use_ssc == 0 || + dev_priv->quirks & QUIRK_LVDS_SSC_DISABLE); + intel_setup_outputs(dev); + + /* Destroy default 1024x768 fbdev and reinitialize */ + intel_fbdev_fini(dev); + if (intel_fbdev_init(dev)) + goto cleanup_gem; + async_schedule(intel_fbdev_initial_config, dev_priv); + return; + +cleanup_gem: + DRM_ERROR("failed to reinitialize fbdev\n"); + mutex_lock(&dev->struct_mutex); + i915_gem_cleanup_ringbuffer(dev); + i915_gem_context_fini(dev); + mutex_unlock(&dev->struct_mutex); +} + static const struct vga_switcheroo_client_ops i915_switcheroo_ops = { .set_gpu_state = i915_switcheroo_set_state, .reprobe = NULL, + .reprobe_connectors = i915_switcheroo_reprobe_connectors, .can_switch = i915_switcheroo_can_switch, };
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index e326ac9..7f58858 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3120,6 +3120,7 @@ extern void intel_set_memory_cxsr(struct drm_i915_private *dev_priv, extern void intel_detect_pch(struct drm_device *dev); extern int intel_trans_dp_port_sel(struct drm_crtc *crtc); extern int intel_enable_rc6(const struct drm_device *dev); +extern void intel_setup_outputs(struct drm_device *dev);
extern bool i915_semaphore_is_enabled(struct drm_device *dev); int i915_reg_read_ioctl(struct drm_device *dev, void *data, diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index d547d9c8..9481206 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -13027,7 +13027,7 @@ static bool intel_crt_present(struct drm_device *dev) return true; }
-static void intel_setup_outputs(struct drm_device *dev) +void intel_setup_outputs(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; struct intel_encoder *encoder; diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index d023710..e9c40b1 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -30,6 +30,7 @@ #include <linux/export.h> #include <linux/notifier.h> #include <linux/reboot.h> +#include <linux/vga_switcheroo.h> #include <drm/drmP.h> #include <drm/drm_atomic_helper.h> #include <drm/drm_crtc.h> @@ -4079,6 +4080,7 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp) { uint8_t *dpcd = intel_dp->dpcd; uint8_t type; + struct pci_dev *pdev = intel_dp->attached_connector->base.dev->pdev;
if (!intel_dp_get_dpcd(intel_dp)) return connector_status_disconnected; @@ -4101,7 +4103,9 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp) }
/* If no HPD, poke DDC gently */ - if (drm_probe_ddc(&intel_dp->aux.ddc)) + if (vga_switcheroo_lock_ddc(pdev) >= 0 && + drm_probe_ddc(&intel_dp->aux.ddc) && + vga_switcheroo_unlock_ddc(pdev) >= 0) return connector_status_connected;
/* Well we tried, say unknown for unreliable port types */ diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c index 8904933..649024d 100644 --- a/drivers/gpu/drm/nouveau/nouveau_drm.c +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c @@ -23,6 +23,7 @@ */
#include <linux/console.h> +#include <linux/delay.h> #include <linux/module.h> #include <linux/pci.h> #include <linux/pm_runtime.h> @@ -666,6 +667,7 @@ nouveau_pmops_suspend(struct device *dev) pci_save_state(pdev); pci_disable_device(pdev); pci_set_power_state(pdev, PCI_D3hot); + udelay(200); return 0; }
diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c index 37ac7b5..3b13e9a 100644 --- a/drivers/gpu/vga/vga_switcheroo.c +++ b/drivers/gpu/vga/vga_switcheroo.c @@ -9,12 +9,13 @@
Switcher interface - methods require for ATPX and DCM - switchto - this throws the output MUX switch - - discrete_set_power - sets the power state for the discrete card + - switch_ddc - switch only DDC lines, return old DDC owner (or < 0 on failure) + - power_state - sets the power state for either GPU
GPU driver interface - set_gpu_state - this should do the equiv of s/r for the card - this should *not* set the discrete power state - - switch_check - check if the device is in a position to switch now + - can_switch - check if the device is in a position to switch now */
#include <linux/module.h> @@ -57,6 +58,9 @@ struct vgasr_priv { struct list_head clients;
struct vga_switcheroo_handler *handler; + + struct mutex ddc_lock; + enum vga_switcheroo_client_id old_ddc_owner; };
#define ID_BIT_AUDIO 0x100 @@ -70,6 +74,7 @@ static void vga_switcheroo_debugfs_fini(struct vgasr_priv *priv); /* only one switcheroo per system */ static struct vgasr_priv vgasr_priv = { .clients = LIST_HEAD_INIT(vgasr_priv.clients), + .ddc_lock = __MUTEX_INITIALIZER(vgasr_priv.ddc_lock), };
static bool vga_switcheroo_ready(void) @@ -103,6 +108,8 @@ static void vga_switcheroo_enable(void)
int vga_switcheroo_register_handler(struct vga_switcheroo_handler *handler) { + struct vga_switcheroo_client *client; + mutex_lock(&vgasr_mutex); if (vgasr_priv.handler) { mutex_unlock(&vgasr_mutex); @@ -110,6 +117,12 @@ int vga_switcheroo_register_handler(struct vga_switcheroo_handler *handler) }
vgasr_priv.handler = handler; + + /* clients which registered before the handler must reprobe */ + list_for_each_entry(client, &vgasr_priv.clients, list) + if (!client->active && client->ops->reprobe_connectors) + client->ops->reprobe_connectors(client->pdev); + if (vga_switcheroo_ready()) { printk(KERN_INFO "vga_switcheroo: enabled\n"); vga_switcheroo_enable(); @@ -256,6 +269,60 @@ void vga_switcheroo_client_fb_set(struct pci_dev *pdev, } EXPORT_SYMBOL(vga_switcheroo_client_fb_set);
+int vga_switcheroo_lock_ddc(struct pci_dev *pdev) +{ + int ret = 0; + int id; + + if (!vgasr_priv.handler) { + ret = -ENODEV; + goto out; + } + + if (vgasr_priv.handler->switch_ddc) { + mutex_lock(&vgasr_priv.ddc_lock); + + id = vgasr_priv.handler->get_client_id(pdev); + ret = vgasr_priv.handler->switch_ddc(id); + + if (ret < 0) { + mutex_unlock(&vgasr_priv.ddc_lock); + printk(KERN_ERR "vga_switcheroo: failed to switch DDC lines\n"); + } else + vgasr_priv.old_ddc_owner = ret; + } + +out: + return ret; +} +EXPORT_SYMBOL(vga_switcheroo_lock_ddc); + +int vga_switcheroo_unlock_ddc(struct pci_dev *pdev) +{ + int ret = 0; + int id; + + if (!vgasr_priv.handler) { + ret = -ENODEV; + goto out; + } + + if (vgasr_priv.handler->switch_ddc) { + id = vgasr_priv.handler->get_client_id(pdev); + + if (vgasr_priv.old_ddc_owner != id) + ret = vgasr_priv.handler->switch_ddc(vgasr_priv.old_ddc_owner); + if (ret < 0) + printk(KERN_ERR "vga_switcheroo: failed to switch DDC lines\n"); + + mutex_unlock(&vgasr_priv.ddc_lock); + } + +out: + return ret; +} +EXPORT_SYMBOL(vga_switcheroo_unlock_ddc); + static int vga_switcheroo_show(struct seq_file *m, void *v) { struct vga_switcheroo_client *client; @@ -353,9 +420,25 @@ static int vga_switchto_stage2(struct vga_switcheroo_client *new_client) console_unlock(); }
+ if (vgasr_priv.handler->switch_ddc) { + mutex_lock(&vgasr_priv.ddc_lock); + ret = vgasr_priv.handler->switch_ddc(new_client->id); + mutex_unlock(&vgasr_priv.ddc_lock); + if (ret < 0) { + printk(KERN_ERR "vga_switcheroo: failed to switch DDC lines\n"); + return ret; + } + } + ret = vgasr_priv.handler->switchto(new_client->id); - if (ret) + if (ret) { + printk(KERN_ERR "vga_switcheroo: failed to switch to client %d\n", new_client->id); + /* restore DDC lines */ + mutex_lock(&vgasr_priv.ddc_lock); + vgasr_priv.handler->switch_ddc(active->id); + mutex_unlock(&vgasr_priv.ddc_lock); return ret; + }
if (new_client->ops->reprobe) new_client->ops->reprobe(new_client->pdev); @@ -468,6 +551,15 @@ vga_switcheroo_debugfs_write(struct file *filp, const char __user *ubuf, vgasr_priv.delayed_switch_active = false;
if (just_mux) { + if (vgasr_priv.handler->switch_ddc) { + mutex_lock(&vgasr_priv.ddc_lock); + ret = vgasr_priv.handler->switch_ddc(client_id); + mutex_unlock(&vgasr_priv.ddc_lock); + if (ret < 0) { + printk(KERN_ERR "vga_switcheroo: failed to switch DDC lines\n"); + goto out; + } + } ret = vgasr_priv.handler->switchto(client_id); goto out; } @@ -623,6 +715,13 @@ static int vga_switcheroo_runtime_suspend(struct device *dev) ret = dev->bus->pm->runtime_suspend(dev); if (ret) return ret; + if (vgasr_priv.handler->switch_ddc) { + mutex_lock(&vgasr_priv.ddc_lock); + ret = vgasr_priv.handler->switch_ddc(VGA_SWITCHEROO_IGD); + mutex_unlock(&vgasr_priv.ddc_lock); + if (ret < 0) + printk(KERN_ERR "vga_switcheroo: failed to switch DDC lines\n"); + } if (vgasr_priv.handler->switchto) vgasr_priv.handler->switchto(VGA_SWITCHEROO_IGD); vga_switcheroo_power_switch(pdev, VGA_SWITCHEROO_OFF); diff --git a/drivers/platform/x86/apple-gmux.c b/drivers/platform/x86/apple-gmux.c index b9429fb..05bba92 100644 --- a/drivers/platform/x86/apple-gmux.c +++ b/drivers/platform/x86/apple-gmux.c @@ -271,14 +271,34 @@ static const struct backlight_ops gmux_bl_ops = { .update_status = gmux_update_status, };
+static int gmux_switch_ddc(enum vga_switcheroo_client_id id) +{ + enum vga_switcheroo_client_id old_ddc_owner; + + if (gmux_read8(apple_gmux_data, GMUX_PORT_SWITCH_DDC) == 1) + old_ddc_owner = VGA_SWITCHEROO_IGD; + else + old_ddc_owner = VGA_SWITCHEROO_DIS; + + pr_debug("Switching gmux DDC from %d to %d\n", old_ddc_owner, id); + + if (id == old_ddc_owner) + return old_ddc_owner; + + if (id == VGA_SWITCHEROO_IGD) + gmux_write8(apple_gmux_data, GMUX_PORT_SWITCH_DDC, 1); + else + gmux_write8(apple_gmux_data, GMUX_PORT_SWITCH_DDC, 2); + + return old_ddc_owner; +} + static int gmux_switchto(enum vga_switcheroo_client_id id) { if (id == VGA_SWITCHEROO_IGD) { - gmux_write8(apple_gmux_data, GMUX_PORT_SWITCH_DDC, 1); gmux_write8(apple_gmux_data, GMUX_PORT_SWITCH_DISPLAY, 2); gmux_write8(apple_gmux_data, GMUX_PORT_SWITCH_EXTERNAL, 2); } else { - gmux_write8(apple_gmux_data, GMUX_PORT_SWITCH_DDC, 2); gmux_write8(apple_gmux_data, GMUX_PORT_SWITCH_DISPLAY, 3); gmux_write8(apple_gmux_data, GMUX_PORT_SWITCH_EXTERNAL, 3); } @@ -345,6 +365,7 @@ gmux_active_client(struct apple_gmux_data *gmux_data) }
static struct vga_switcheroo_handler gmux_handler = { + .switch_ddc = gmux_switch_ddc, .switchto = gmux_switchto, .power_state = gmux_set_power_state, .get_client_id = gmux_get_client_id, @@ -552,18 +573,20 @@ static int gmux_probe(struct pnp_dev *pnp, const struct pnp_device_id *id) gmux_data->gpe = -1; }
+ apple_gmux_data = gmux_data; + init_completion(&gmux_data->powerchange_done); + gmux_enable_interrupts(gmux_data); + if (vga_switcheroo_register_handler(&gmux_handler)) { ret = -ENODEV; goto err_register_handler; }
- init_completion(&gmux_data->powerchange_done); - apple_gmux_data = gmux_data; - gmux_enable_interrupts(gmux_data); - return 0;
err_register_handler: + gmux_disable_interrupts(gmux_data); + apple_gmux_data = NULL; if (gmux_data->gpe >= 0) acpi_disable_gpe(NULL, gmux_data->gpe); err_enable_gpe: diff --git a/include/linux/vga_switcheroo.h b/include/linux/vga_switcheroo.h index b483abd..c3ad8bf 100644 --- a/include/linux/vga_switcheroo.h +++ b/include/linux/vga_switcheroo.h @@ -29,6 +29,7 @@ enum vga_switcheroo_client_id { };
struct vga_switcheroo_handler { + int (*switch_ddc)(enum vga_switcheroo_client_id id); int (*switchto)(enum vga_switcheroo_client_id id); int (*power_state)(enum vga_switcheroo_client_id id, enum vga_switcheroo_state state); @@ -39,6 +40,7 @@ struct vga_switcheroo_handler { struct vga_switcheroo_client_ops { void (*set_gpu_state)(struct pci_dev *dev, enum vga_switcheroo_state); void (*reprobe)(struct pci_dev *dev); + void (*reprobe_connectors)(struct pci_dev *dev); bool (*can_switch)(struct pci_dev *dev); };
@@ -54,6 +56,9 @@ int vga_switcheroo_register_audio_client(struct pci_dev *pdev, void vga_switcheroo_client_fb_set(struct pci_dev *dev, struct fb_info *info);
+int vga_switcheroo_lock_ddc(struct pci_dev *pdev); +int vga_switcheroo_unlock_ddc(struct pci_dev *pdev); + int vga_switcheroo_register_handler(struct vga_switcheroo_handler *handler); void vga_switcheroo_unregister_handler(void);
@@ -72,6 +77,8 @@ static inline void vga_switcheroo_unregister_client(struct pci_dev *dev) {} static inline int vga_switcheroo_register_client(struct pci_dev *dev, const struct vga_switcheroo_client_ops *ops, bool driver_power_control) { return 0; } static inline void vga_switcheroo_client_fb_set(struct pci_dev *dev, struct fb_info *info) {} +static inline int vga_switcheroo_lock_ddc(struct pci_dev *pdev) { return 0; } +static inline int vga_switcheroo_unlock_ddc(struct pci_dev *pdev) { return 0; } static inline int vga_switcheroo_register_handler(struct vga_switcheroo_handler *handler) { return 0; } static inline int vga_switcheroo_register_audio_client(struct pci_dev *pdev, const struct vga_switcheroo_client_ops *ops,