From: Seth Forshee seth.forshee@canonical.com

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

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=88861 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=61115 Tested-by: Paul Hordiienko pvt.gord@gmail.com [MBP 6,2 2010 intel ILK + nvidia GT216 pre-retina] Tested-by: William Brown william@blackhats.net.au [MBP 8,2 2011 intel SNB + amd turks pre-retina] Tested-by: Lukas Wunner lukas@wunner.de [MBP 9,1 2012 intel IVB + nvidia GK107 pre-retina] Tested-by: Bruno Bierbaumer bruno@bierbaumer.net [MBP 11,3 2013 intel HSW + nvidia GK107 retina -- work in progress]

Signed-off-by: Seth Forshee seth.forshee@canonical.com Signed-off-by: Dave Airlie airlied@gmail.com Signed-off-by: Lukas Wunner lukas@wunner.de --- drivers/gpu/vga/vga_switcheroo.c | 14 ++++++++++++++ include/linux/vga_switcheroo.h | 2 ++ 2 files changed, 16 insertions(+)

diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c index 0d3ac20..620c4ac 100644 --- a/drivers/gpu/vga/vga_switcheroo.c +++ b/drivers/gpu/vga/vga_switcheroo.c @@ -209,6 +209,20 @@ find_active_client(struct list_head *head) return NULL; }

+struct pci_dev *vga_switcheroo_get_active_client(void) +{ + struct vga_switcheroo_client *client; + struct pci_dev *pdev = NULL; + + mutex_lock(&vgasr_mutex); + client = find_active_client(&vgasr_priv.clients); + if (client) + pdev = client->pdev; + mutex_unlock(&vgasr_mutex); + return pdev; +} +EXPORT_SYMBOL(vga_switcheroo_get_active_client); + int vga_switcheroo_get_client_state(struct pci_dev *pdev) { struct vga_switcheroo_client *client; diff --git a/include/linux/vga_switcheroo.h b/include/linux/vga_switcheroo.h index 2fef78b..c81a686 100644 --- a/include/linux/vga_switcheroo.h +++ b/include/linux/vga_switcheroo.h @@ -62,6 +62,7 @@ void vga_switcheroo_unregister_handler(void);

int vga_switcheroo_process_delayed_switch(void);

+struct pci_dev *vga_switcheroo_get_active_client(void); int vga_switcheroo_get_client_state(struct pci_dev *dev);

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

static inline void vga_switcheroo_set_dynamic_switch(struct pci_dev *pdev, enum vga_switcheroo_state dynamic) {}

From: Seth Forshee seth.forshee@canonical.com

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

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=88861 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=61115 Tested-by: Paul Hordiienko pvt.gord@gmail.com [MBP 6,2 2010 intel ILK + nvidia GT216 pre-retina] Tested-by: William Brown william@blackhats.net.au [MBP 8,2 2011 intel SNB + amd turks pre-retina] Tested-by: Lukas Wunner lukas@wunner.de [MBP 9,1 2012 intel IVB + nvidia GK107 pre-retina] Tested-by: Bruno Bierbaumer bruno@bierbaumer.net [MBP 11,3 2013 intel HSW + nvidia GK107 retina -- work in progress]

Signed-off-by: Seth Forshee seth.forshee@canonical.com Signed-off-by: Dave Airlie airlied@gmail.com Signed-off-by: Lukas Wunner lukas@wunner.de --- drivers/gpu/drm/drm_edid.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index e6e05bb..2424aef 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> @@ -87,6 +88,8 @@ struct detailed_mode_closure { #define LEVEL_GTF2 2 #define LEVEL_CVT 3

+static DEFINE_MUTEX(drm_edid_mutex); + static struct edid_quirk { char vendor[4]; int product_id; @@ -1377,6 +1380,16 @@ struct edid *drm_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter) { struct edid *edid; + struct pci_dev *pdev = connector->dev->pdev; + struct pci_dev *active_pdev = NULL; + + mutex_lock(&drm_edid_mutex); + + if (pdev) { + active_pdev = vga_switcheroo_get_active_client(); + if (active_pdev != pdev) + vga_switcheroo_switch_ddc(pdev); + }

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

Unlock DDC lines if drm_probe_ddc() fails.

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

Use mutex_trylock(&vgasr_mutex) in vga_switcheroo_unlock_ddc() to avoid locking inversion when one client locks vgasr_mutex on entering vga_switcheroo_lock_ddc() and tries to acquire ddc_lock while another client is holding ddc_lock and tries to acquire vgasr_mutex on entering vga_switcheroo_unlock_ddc().

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

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

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=88861 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=61115 Tested-by: Paul Hordiienko pvt.gord@gmail.com [MBP 6,2 2010 intel ILK + nvidia GT216 pre-retina] Tested-by: William Brown william@blackhats.net.au [MBP 8,2 2011 intel SNB + amd turks pre-retina] Tested-by: Lukas Wunner lukas@wunner.de [MBP 9,1 2012 intel IVB + nvidia GK107 pre-retina] Tested-by: Bruno Bierbaumer bruno@bierbaumer.net [MBP 11,3 2013 intel HSW + nvidia GK107 retina -- work in progress]

Signed-off-by: Lukas Wunner lukas@wunner.de --- drivers/gpu/drm/drm_edid.c | 9 ++-- drivers/gpu/vga/vga_switcheroo.c | 110 ++++++++++++++++++++++---------------- drivers/platform/x86/apple-gmux.c | 15 +++++- include/linux/vga_switcheroo.h | 3 +- 4 files changed, 87 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 505eed1..cdb2fa1 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -1378,17 +1378,20 @@ struct edid *drm_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter) { struct edid *edid; + struct pci_dev *pdev = connector->dev->pdev;

- vga_switcheroo_lock_ddc(connector->dev->pdev); + vga_switcheroo_lock_ddc(pdev);

- if (!drm_probe_ddc(adapter)) + if (!drm_probe_ddc(adapter)) { + vga_switcheroo_unlock_ddc(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); + vga_switcheroo_unlock_ddc(pdev);

return edid; } diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c index 0223020..f02e7fc 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> @@ -59,7 +60,7 @@ struct vgasr_priv { struct vga_switcheroo_handler *handler;

struct mutex ddc_lock; - struct pci_dev *old_ddc_owner; + enum vga_switcheroo_client_id old_ddc_owner; };

#define ID_BIT_AUDIO 0x100 @@ -276,33 +277,24 @@ EXPORT_SYMBOL(vga_switcheroo_client_fb_set);

int vga_switcheroo_lock_ddc(struct pci_dev *pdev) { - struct vga_switcheroo_client *client; - int ret = 0; - int id; + int ret, id;

mutex_lock(&vgasr_mutex);

- if (!vgasr_priv.handler) { + if (!vgasr_priv.handler || !vgasr_priv.handler->switch_ddc) { ret = -ENODEV; goto out; }

- if (vgasr_priv.handler->switch_ddc) { - mutex_lock(&vgasr_priv.ddc_lock); + mutex_lock(&vgasr_priv.ddc_lock); + id = vgasr_priv.handler->get_client_id(pdev); + ret = vgasr_priv.handler->switch_ddc(id);

- client = find_active_client(&vgasr_priv.clients); - if (!client) { - mutex_unlock(&vgasr_priv.ddc_lock); - ret = -ENODEV; - goto out; - } - vgasr_priv.old_ddc_owner = client->pdev; - if (client->pdev == pdev) - goto out; - - id = vgasr_priv.handler->get_client_id(pdev); - ret = vgasr_priv.handler->switch_ddc(id); - } + if (ret < 0) { + pr_err("vga_switcheroo: failed to switch DDC lines\n"); + mutex_unlock(&vgasr_priv.ddc_lock); + } else + vgasr_priv.old_ddc_owner = ret;

out: mutex_unlock(&vgasr_mutex); @@ -312,25 +304,33 @@ EXPORT_SYMBOL(vga_switcheroo_lock_ddc);

int vga_switcheroo_unlock_ddc(struct pci_dev *pdev) { - int ret = 0; - int id; - mutex_lock(&vgasr_mutex); + int ret, id; + bool vgasr_mutex_acquired = mutex_trylock(&vgasr_mutex);

- if (!vgasr_priv.handler) { - ret = -ENODEV; + if (!mutex_is_locked(&vgasr_priv.ddc_lock)) { + ret = -EINVAL; goto out; }

- if (vgasr_priv.handler->switch_ddc) { - if (vgasr_priv.old_ddc_owner != pdev) { - id = vgasr_priv.handler->get_client_id(vgasr_priv.old_ddc_owner); - ret = vgasr_priv.handler->switch_ddc(id); - } - vgasr_priv.old_ddc_owner = NULL; + if (!vgasr_priv.handler || !vgasr_priv.handler->switch_ddc) { + pr_err("vga_switcheroo: handler disappeared\n"); mutex_unlock(&vgasr_priv.ddc_lock); + ret = -ENODEV; + goto out; } + + 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) + pr_err("vga_switcheroo: failed to switch DDC lines\n"); + } else + ret = vgasr_priv.old_ddc_owner; + mutex_unlock(&vgasr_priv.ddc_lock); + out: - mutex_unlock(&vgasr_mutex); + if (vgasr_mutex_acquired) + mutex_unlock(&vgasr_mutex); return ret; } EXPORT_SYMBOL(vga_switcheroo_unlock_ddc); @@ -433,14 +433,26 @@ static int vga_switchto_stage2(struct vga_switcheroo_client *new_client) }

if (vgasr_priv.handler->switch_ddc) { + mutex_lock(&vgasr_priv.ddc_lock); ret = vgasr_priv.handler->switch_ddc(new_client->id); - if (ret) + mutex_unlock(&vgasr_priv.ddc_lock); + if (ret < 0) { + pr_err("vga_switcheroo: failed to switch DDC lines\n"); return ret; + } }

ret = vgasr_priv.handler->switchto(new_client->id); - if (ret) - goto restore_ddc; + if (ret) { + pr_err("vga_switcheroo: failed to switch to client %d\n", + new_client->id); + active->active = true; + /* 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); @@ -452,14 +464,6 @@ static int vga_switchto_stage2(struct vga_switcheroo_client *new_client)

new_client->active = true; return 0; - -restore_ddc: - if (vgasr_priv.handler->switch_ddc) { - int id = (new_client->id == VGA_SWITCHEROO_IGD) ? - VGA_SWITCHEROO_DIS : VGA_SWITCHEROO_IGD; - ret = vgasr_priv.handler->switch_ddc(id); - } - return ret; }

static bool check_can_switch(void) @@ -561,6 +565,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) { + pr_err("vga_switcheroo: failed to switch DDC lines\n"); + goto out; + } + } ret = vgasr_priv.handler->switchto(client_id); goto out; } @@ -716,6 +729,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) + pr_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 f0a55a4..08bdf1e 100644 --- a/drivers/platform/x86/apple-gmux.c +++ b/drivers/platform/x86/apple-gmux.c @@ -275,11 +275,24 @@ static const struct backlight_ops gmux_bl_ops = {

static int gmux_switch_ddc(enum vga_switcheroo_client_id id) { + enum vga_switcheroo_client_id old_ddc_owner; + + if (gmux_read8(apple_gmux_data, GMUX_PORT_SWITCH_DDC) == 1) + old_ddc_owner = VGA_SWITCHEROO_IGD; + else + old_ddc_owner = VGA_SWITCHEROO_DIS; + + pr_debug("Switching gmux DDC from %d to %d\n", old_ddc_owner, id); + + if (id == old_ddc_owner) + return old_ddc_owner; + if (id == VGA_SWITCHEROO_IGD) gmux_write8(apple_gmux_data, GMUX_PORT_SWITCH_DDC, 1); else gmux_write8(apple_gmux_data, GMUX_PORT_SWITCH_DDC, 2); - return 0; + + return old_ddc_owner; }

static int gmux_switchto(enum vga_switcheroo_client_id id) diff --git a/include/linux/vga_switcheroo.h b/include/linux/vga_switcheroo.h index 352bef3..39b25b0 100644 --- a/include/linux/vga_switcheroo.h +++ b/include/linux/vga_switcheroo.h @@ -77,7 +77,8 @@ static inline void vga_switcheroo_unregister_client(struct pci_dev *dev) {} static inline int vga_switcheroo_register_client(struct pci_dev *dev, const struct vga_switcheroo_client_ops *ops, bool driver_power_control) { return 0; } static inline void vga_switcheroo_client_fb_set(struct pci_dev *dev, struct fb_info *info) {} -static inline void vga_switcheroo_switch_ddc(struct pci_dev *pdev) { return NULL; } +static inline int vga_switcheroo_lock_ddc(struct pci_dev *pdev) { return -ENODEV; } +static inline int vga_switcheroo_unlock_ddc(struct pci_dev *pdev) { return -ENODEV; } 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,

This reverts commit 8d608aa6295242fe4c4b6105b8c59c6a5b232d89.

Reprobing must happen asynchronously as the driver may grab vgasr_mutex when invoking vga_switcheroo_client_fb_set(), when locking the DDC lines or when using the upcoming DDC/AUX proxy functionality. And that lock is already held in stage2 by vga_switcheroo_debugfs_write().

Infrastructure for asynchronous reprobing is added to vga_switcheroo with a subsequent commit and could easily be used in stage2, but:

- The ->reprobe callback was only ever used by a single driver, nouveau.

- Drivers need to reprobe their outputs anyway after waking up, this is nothing specific to dual GPU laptops and should be added to the resume code of the driver.

- It would seem there's no need anymore to delay reprobing until after switching the mux: We have two methods now to make reprobing work with the display not switched (switching only the DDC lines and proxying), at least one of them should always work.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=88861 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=61115 Tested-by: Paul Hordiienko pvt.gord@gmail.com [MBP 6,2 2010 intel ILK + nvidia GT216 pre-retina] Tested-by: William Brown william@blackhats.net.au [MBP 8,2 2011 intel SNB + amd turks pre-retina] Tested-by: Lukas Wunner lukas@wunner.de [MBP 9,1 2012 intel IVB + nvidia GK107 pre-retina] Tested-by: Bruno Bierbaumer bruno@bierbaumer.net [MBP 11,3 2013 intel HSW + nvidia GK107 retina -- work in progress]

Signed-off-by: Lukas Wunner lukas@wunner.de --- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 1 - drivers/gpu/drm/i915/i915_dma.c | 1 - drivers/gpu/drm/nouveau/nouveau_vga.c | 8 -------- drivers/gpu/drm/radeon/radeon_device.c | 1 - drivers/gpu/vga/vga_switcheroo.c | 3 --- include/linux/vga_switcheroo.h | 1 - 6 files changed, 15 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index d79009b..55c8d51 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -1076,7 +1076,6 @@ static bool amdgpu_switcheroo_can_switch(struct pci_dev *pdev)

static const struct vga_switcheroo_client_ops amdgpu_switcheroo_ops = { .set_gpu_state = amdgpu_switcheroo_set_state, - .reprobe = NULL, .can_switch = amdgpu_switcheroo_can_switch, };

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index b1f9e55..c240944 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -388,7 +388,6 @@ static bool i915_switcheroo_can_switch(struct pci_dev *pdev)

static const struct vga_switcheroo_client_ops i915_switcheroo_ops = { .set_gpu_state = i915_switcheroo_set_state, - .reprobe = NULL, .can_switch = i915_switcheroo_can_switch, };

diff --git a/drivers/gpu/drm/nouveau/nouveau_vga.c b/drivers/gpu/drm/nouveau/nouveau_vga.c index c7592ec..42779f4 100644 --- a/drivers/gpu/drm/nouveau/nouveau_vga.c +++ b/drivers/gpu/drm/nouveau/nouveau_vga.c @@ -55,13 +55,6 @@ nouveau_switcheroo_set_state(struct pci_dev *pdev, } }

-static void -nouveau_switcheroo_reprobe(struct pci_dev *pdev) -{ - struct drm_device *dev = pci_get_drvdata(pdev); - nouveau_fbcon_output_poll_changed(dev); -} - static bool nouveau_switcheroo_can_switch(struct pci_dev *pdev) { @@ -78,7 +71,6 @@ nouveau_switcheroo_can_switch(struct pci_dev *pdev) static const struct vga_switcheroo_client_ops nouveau_switcheroo_ops = { .set_gpu_state = nouveau_switcheroo_set_state, - .reprobe = nouveau_switcheroo_reprobe, .can_switch = nouveau_switcheroo_can_switch, };

diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c index d8319da..2cb513f 100644 --- a/drivers/gpu/drm/radeon/radeon_device.c +++ b/drivers/gpu/drm/radeon/radeon_device.c @@ -1258,7 +1258,6 @@ static bool radeon_switcheroo_can_switch(struct pci_dev *pdev)

static const struct vga_switcheroo_client_ops radeon_switcheroo_ops = { .set_gpu_state = radeon_switcheroo_set_state, - .reprobe = NULL, .can_switch = radeon_switcheroo_can_switch, };

diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c index f0d5475..8027c9f 100644 --- a/drivers/gpu/vga/vga_switcheroo.c +++ b/drivers/gpu/vga/vga_switcheroo.c @@ -440,9 +440,6 @@ static int vga_switchto_stage2(struct vga_switcheroo_client *new_client) return ret; }

- if (new_client->ops->reprobe) - new_client->ops->reprobe(new_client->pdev); - if (active->pwr_state == VGA_SWITCHEROO_ON) vga_switchoff(active);

diff --git a/include/linux/vga_switcheroo.h b/include/linux/vga_switcheroo.h index 62f303f..b935d83 100644 --- a/include/linux/vga_switcheroo.h +++ b/include/linux/vga_switcheroo.h @@ -39,7 +39,6 @@ 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); bool (*can_switch)(struct pci_dev *dev); };

From: Matthew Garrett matthew.garrett@nebula.com

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

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=88861 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=61115 Tested-by: Paul Hordiienko pvt.gord@gmail.com [MBP 6,2 2010 intel ILK + nvidia GT216 pre-retina] Tested-by: William Brown william@blackhats.net.au [MBP 8,2 2011 intel SNB + amd turks pre-retina] Tested-by: Lukas Wunner lukas@wunner.de [MBP 9,1 2012 intel IVB + nvidia GK107 pre-retina] Tested-by: Bruno Bierbaumer bruno@bierbaumer.net [MBP 11,3 2013 intel HSW + nvidia GK107 retina -- work in progress]

Signed-off-by: Matthew Garrett matthew.garrett@nebula.com Signed-off-by: Lukas Wunner lukas@wunner.de --- drivers/platform/x86/apple-gmux.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/x86/apple-gmux.c b/drivers/platform/x86/apple-gmux.c index 08bdf1e..b120a11 100644 --- a/drivers/platform/x86/apple-gmux.c +++ b/drivers/platform/x86/apple-gmux.c @@ -609,18 +609,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:

Commit 92122789b2d6 ("drm/i915: preserve SSC if previously set v3") added code to intel_modeset_gem_init to override the SSC status read from VBT with the SSC status set by BIOS.

However, intel_modeset_gem_init is invoked *after* intel_modeset_init, which calls intel_setup_outputs, which *modifies* SSC status by way of intel_init_pch_refclk. So unlike advertised, intel_modeset_gem_init doesn't preserve the SSC status set by BIOS but whatever intel_init_pch_refclk decided on.

This is a problem on dual gpu laptops such as the MacBook Pro which require either a handler to switch DDC lines, or the discrete gpu to proxy DDC/AUX communication: Both the handler and the discrete gpu may initialize after the i915 driver, and consequently, an LVDS connector may initially seem disconnected and the SSC therefore is disabled by intel_init_pch_refclk, but on reprobe the connector may turn out to be connected and the SSC must then be enabled.

Due to 92122789b2d6 however, the SSC is not enabled on reprobe since it is assumed BIOS disabled it while in fact it was disabled by intel_init_pch_refclk.

Also, because the SSC status is preserved so late, the preserved value only ever gets used on resume but not on panel initialization: intel_modeset_init calls intel_init_display which indirectly calls intel_panel_use_ssc via multiple subroutines, *before* the BIOS value overrides the VBT value in intel_modeset_gem_init (intel_panel_use_ssc is the sole user of dev_priv->vbt.lvds_use_ssc).

Fix this by moving the code introduced by 92122789b2d6 from intel_modeset_gem_init to intel_modeset_init before the invocation of intel_setup_outputs and intel_init_display.

Add a DRM_DEBUG_KMS as suggested way back by Jani: http://lists.freedesktop.org/archives/intel-gfx/2014-June/046666.html

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=88861 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=61115 Tested-by: Paul Hordiienko pvt.gord@gmail.com [MBP 6,2 2010 intel ILK + nvidia GT216 pre-retina] Tested-by: William Brown william@blackhats.net.au [MBP 8,2 2011 intel SNB + amd turks pre-retina] Tested-by: Lukas Wunner lukas@wunner.de [MBP 9,1 2012 intel IVB + nvidia GK107 pre-retina] Tested-by: Bruno Bierbaumer bruno@bierbaumer.net [MBP 11,3 2013 intel HSW + nvidia GK107 retina -- work in progress]

Fixes: 92122789b2d6 ("drm/i915: preserve SSC if previously set v3") Signed-off-by: Lukas Wunner lukas@wunner.de --- drivers/gpu/drm/i915/intel_display.c | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index af0bcfe..6335883 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -14893,6 +14893,24 @@ void intel_modeset_init(struct drm_device *dev) if (INTEL_INFO(dev)->num_pipes == 0) return;

+ /* + * There may be no VBT; and if the BIOS enabled SSC we can + * just keep using it to avoid unnecessary flicker. Whereas if the + * BIOS isn't using it, don't assume it will work even if the VBT + * indicates as much. + */ + if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)) { + bool bios_lvds_use_ssc = !!(I915_READ(PCH_DREF_CONTROL) & + DREF_SSC1_ENABLE); + + if (dev_priv->vbt.lvds_use_ssc != bios_lvds_use_ssc) { + DRM_DEBUG_KMS("SSC %sabled by BIOS, overriding VBT which says %sabled\n", + bios_lvds_use_ssc ? "en" : "dis", + dev_priv->vbt.lvds_use_ssc ? "en" : "dis"); + dev_priv->vbt.lvds_use_ssc = bios_lvds_use_ssc; + } + } + intel_init_display(dev); intel_init_audio(dev);

@@ -15446,7 +15464,6 @@ err:

void intel_modeset_gem_init(struct drm_device *dev) { - struct drm_i915_private *dev_priv = dev->dev_private; struct drm_crtc *c; struct drm_i915_gem_object *obj; int ret; @@ -15455,16 +15472,6 @@ void intel_modeset_gem_init(struct drm_device *dev) intel_init_gt_powersave(dev); mutex_unlock(&dev->struct_mutex);

- /* - * There may be no VBT; and if the BIOS enabled SSC we can - * just keep using it to avoid unnecessary flicker. Whereas if the - * BIOS isn't using it, don't assume it will work even if the VBT - * indicates as much. - */ - if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)) - dev_priv->vbt.lvds_use_ssc = !!(I915_READ(PCH_DREF_CONTROL) & - DREF_SSC1_ENABLE); - intel_modeset_init_hw(dev);

intel_setup_overlay(dev);

From: Tvrtko Ursulin tvrtko.ursulin@intel.com

We had two failure modes here:

1. Deadlock in intelfb_alloc failure path where it calls drm_framebuffer_remove, which grabs the struct mutex and intelfb_create (caller of intelfb_alloc) was already holding it.

2. Deadlock in intelfb_create failure path where it calls drm_framebuffer_unreference, which grabs the struct mutex and intelfb_create was already holding it.

v2: * Reformat commit msg to 72 chars. (Lukas Wunner) * Add third failure mode. (Lukas Wunner)

v3: * On fb alloc failure, unref gem object where it gets refed, fix double unref in separate commit. (Ville Syrjälä)

v4: * Lock struct mutex on unref. (Chris Wilson)

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=88861 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=61115 Tested-by: Paul Hordiienko pvt.gord@gmail.com [MBP 6,2 2010 intel ILK + nvidia GT216 pre-retina] Tested-by: William Brown william@blackhats.net.au [MBP 8,2 2011 intel SNB + amd turks pre-retina] Tested-by: Lukas Wunner lukas@wunner.de [MBP 9,1 2012 intel IVB + nvidia GK107 pre-retina] Tested-by: Bruno Bierbaumer bruno@bierbaumer.net [MBP 11,3 2013 intel HSW + nvidia GK107 retina -- work in progress]

Signed-off-by: Tvrtko Ursulin tvrtko.ursulin@intel.com Fixes: 60a5ca015ffd ("drm/i915: Add locking around framebuffer_references--") Reported-by: Lukas Wunner lukas@wunner.de [Lukas: Create v3 + v4 based on Tvrtko's v2] Signed-off-by: Lukas Wunner lukas@wunner.de Cc: Chris Wilson chris@chris-wilson.co.uk Cc: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/i915/intel_fbdev.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c index 7eff33f..dd138aa 100644 --- a/drivers/gpu/drm/i915/intel_fbdev.c +++ b/drivers/gpu/drm/i915/intel_fbdev.c @@ -140,7 +140,7 @@ static int intelfb_alloc(struct drm_fb_helper *helper, { struct intel_fbdev *ifbdev = container_of(helper, struct intel_fbdev, helper); - struct drm_framebuffer *fb; + struct drm_framebuffer *fb = NULL; struct drm_device *dev = helper->dev; struct drm_mode_fb_cmd2 mode_cmd = {}; struct drm_i915_gem_object *obj; @@ -158,6 +158,8 @@ static int intelfb_alloc(struct drm_fb_helper *helper, mode_cmd.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp, sizes->surface_depth);

+ mutex_lock(&dev->struct_mutex); + size = mode_cmd.pitches[0] * mode_cmd.height; size = PAGE_ALIGN(size); obj = i915_gem_object_create_stolen(dev, size); @@ -179,18 +181,21 @@ static int intelfb_alloc(struct drm_fb_helper *helper, ret = intel_pin_and_fence_fb_obj(NULL, fb, NULL, NULL, NULL); if (ret) { DRM_ERROR("failed to pin obj: %d\n", ret); - goto out_fb; + goto out_unref; }

+ mutex_unlock(&dev->struct_mutex); + ifbdev->fb = to_intel_framebuffer(fb);

return 0;

-out_fb: - drm_framebuffer_remove(fb); out_unref: drm_gem_object_unreference(&obj->base); out: + mutex_unlock(&dev->struct_mutex); + if (fb) + drm_framebuffer_remove(fb); return ret; }

@@ -208,8 +213,6 @@ static int intelfb_create(struct drm_fb_helper *helper, int size, ret; bool prealloc = false;

- mutex_lock(&dev->struct_mutex); - if (intel_fb && (sizes->fb_width > intel_fb->base.width || sizes->fb_height > intel_fb->base.height)) { @@ -224,7 +227,7 @@ static int intelfb_create(struct drm_fb_helper *helper, DRM_DEBUG_KMS("no BIOS fb, allocating a new one\n"); ret = intelfb_alloc(helper, sizes); if (ret) - goto out_unlock; + return ret; intel_fb = ifbdev->fb; } else { DRM_DEBUG_KMS("re-using BIOS fb\n"); @@ -236,6 +239,8 @@ static int intelfb_create(struct drm_fb_helper *helper, obj = intel_fb->obj; size = obj->base.size;

+ mutex_lock(&dev->struct_mutex); + info = framebuffer_alloc(0, &dev->pdev->dev); if (!info) { ret = -ENOMEM; @@ -306,7 +311,6 @@ static int intelfb_create(struct drm_fb_helper *helper, out_unpin: i915_gem_object_ggtt_unpin(obj); drm_gem_object_unreference(&obj->base); -out_unlock: mutex_unlock(&dev->struct_mutex); return ret; }

If no connectors with modes are found, drm_fb_helper_single_fb_probe() will allocate a default 1024x768 fb. This becomes a problem if one of the connectors subsequently changes to connected status: We're stuck with the default fb even if the display allows a larger resolution (or doesn't support 1024x768 at all) since modes larger than the existing fb will be deemed ineligible by drm_fb_helper_hotplug_event().

One relevant use case are dual gpu laptops such as the MacBook Pro which require either a handler to switch DDC lines, or the active gpu to proxy the DDC/AUX communication: Both the handler and the active gpu may register late, and consequently, the inactive gpu's eDP and LVDS connectors may seem disconnected at startup but may later on turn out to be connected.

Although primarily aimed at vga_switcheroo setups, the patch may come in handy in other use cases: One example is a desktop machine that is (inadvertantly or otherwise) booted headless, and the user later on plugs in a display. Another example is unplugging of the display by the user and replacement with a larger one.

Solve this by checking if there aren't any modes at all. If so, don't constrain probed modes to the existing fb size but rather allow them to occupy the maximum surface size. After probing, check if we found any modes. If we did, discard the existing fb and recreate it from scratch by invoking the driver's ->fb_probe callback by way of drm_fb_helper_single_fb_probe().

That function is normally called only once on driver initialization. The fb_helper is kept so ensure that it's not added once more to the panic handler list.

Because multiple hotplug events may fire simultaneously, protect connector probing and fb recreation with drm_modeset_lock_all(). In particular, drm_fb_helper_hotplug_event() may itself fire another hotplug event by way of drm_fb_helper_probe_connector_modes(), which invokes the ->fill_modes callback, which for most drivers is drm_helper_probe_single_connector_modes(), which schedules output_poll_execute().

Amend the ->fb_probe callback in the i915, nouveau and radeon drivers to discard an existing fb. Because the _destroy() function is now used further up in intel_fbdev.c, nouveau_fbcon.c and radeon_fb.c, add a prototype for it. This is intended to ease reviewing, the prototype shall be replaced by the body of the function before this gets merged.

Drivers other than these three will just create an additional fb under the limited circumstances described above, which wastes memory but is otherwise "mostly harmless".

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=88861 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=61115 Tested-by: Paul Hordiienko pvt.gord@gmail.com [MBP 6,2 2010 intel ILK + nvidia GT216 pre-retina] Tested-by: William Brown william@blackhats.net.au [MBP 8,2 2011 intel SNB + amd turks pre-retina] Tested-by: Lukas Wunner lukas@wunner.de [MBP 9,1 2012 intel IVB + nvidia GK107 pre-retina] Tested-by: Bruno Bierbaumer bruno@bierbaumer.net [MBP 11,3 2013 intel HSW + nvidia GK107 retina -- work in progress]

Signed-off-by: Lukas Wunner lukas@wunner.de --- drivers/gpu/drm/drm_fb_helper.c | 41 ++++++++++++++++++++++++++------- drivers/gpu/drm/i915/intel_fbdev.c | 26 ++++++++++----------- drivers/gpu/drm/nouveau/nouveau_fbcon.c | 10 +++++++- drivers/gpu/drm/radeon/radeon_fb.c | 11 ++++++--- 4 files changed, 63 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 73f90f7..6df0ee8 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -1097,7 +1097,8 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper, register_sysrq_key('v', &sysrq_drm_fb_helper_restore_op); }

- list_add(&fb_helper->kernel_fb_list, &kernel_fb_helper_list); + if (list_empty(&fb_helper->kernel_fb_list)) + list_add(&fb_helper->kernel_fb_list, &kernel_fb_helper_list);

return 0; } @@ -1770,23 +1771,47 @@ int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper) { struct drm_device *dev = fb_helper->dev; u32 max_width, max_height; + bool modes_before_probe = false; + bool modes_after_probe = false; + int i;

- mutex_lock(&fb_helper->dev->mode_config.mutex); + drm_modeset_lock_all(dev); if (!fb_helper->fb || !drm_fb_helper_is_bound(fb_helper)) { fb_helper->delayed_hotplug = true; - mutex_unlock(&fb_helper->dev->mode_config.mutex); + drm_modeset_unlock_all(dev); return 0; } DRM_DEBUG_KMS("\n");

- max_width = fb_helper->fb->width; - max_height = fb_helper->fb->height; + /* If so far we have no modes and thus only the default 1024x768 fb, + * allow probed modes to occupy the maximum surface size */ + for (i = 0; i < fb_helper->crtc_count; i++) + if (fb_helper->crtc_info[i].desired_mode) { + modes_before_probe = true; + break; + } + if (!modes_before_probe) { + max_width = dev->mode_config.max_width; + max_height = dev->mode_config.max_width; + } else { + max_width = fb_helper->fb->width; + max_height = fb_helper->fb->height; + }

drm_fb_helper_probe_connector_modes(fb_helper, max_width, max_height); - mutex_unlock(&fb_helper->dev->mode_config.mutex); - - drm_modeset_lock_all(dev); drm_setup_crtcs(fb_helper); + + /* If previously there were no connectors with modes and now we + * found some, create new fb and replace default 1024x768 fb */ + for (i = 0; i < fb_helper->crtc_count; i++) + if (fb_helper->crtc_info[i].desired_mode) { + modes_after_probe = true; + break; + } + if (!modes_before_probe && modes_after_probe) + drm_fb_helper_single_fb_probe(fb_helper, + fb_helper->fb->bits_per_pixel); + drm_modeset_unlock_all(dev); drm_fb_helper_set_par(fb_helper->fbdev);

diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c index dd138aa..637146f 100644 --- a/drivers/gpu/drm/i915/intel_fbdev.c +++ b/drivers/gpu/drm/i915/intel_fbdev.c @@ -199,6 +199,8 @@ out: return ret; }

+static void intel_fbdev_destroy(struct fb_info *info); + static int intelfb_create(struct drm_fb_helper *helper, struct drm_fb_helper_surface_size *sizes) { @@ -220,6 +222,7 @@ static int intelfb_create(struct drm_fb_helper *helper, " releasing it\n", intel_fb->base.width, intel_fb->base.height, sizes->fb_width, sizes->fb_height); + intel_fbdev_destroy(ifbdev->helper.fbdev); drm_framebuffer_unreference(&intel_fb->base); intel_fb = ifbdev->fb = NULL; } @@ -545,12 +548,9 @@ static const struct drm_fb_helper_funcs intel_fb_helper_funcs = { .fb_probe = intelfb_create, };

-static void intel_fbdev_destroy(struct drm_device *dev, - struct intel_fbdev *ifbdev) +static void intel_fbdev_destroy(struct fb_info *info) { - if (ifbdev->helper.fbdev) { - struct fb_info *info = ifbdev->helper.fbdev; - + if (info) { unregister_framebuffer(info); iounmap(info->screen_base); if (info->cmap.len) @@ -558,11 +558,6 @@ static void intel_fbdev_destroy(struct drm_device *dev,

framebuffer_release(info); } - - drm_fb_helper_fini(&ifbdev->helper); - - drm_framebuffer_unregister_private(&ifbdev->fb->base); - drm_framebuffer_remove(&ifbdev->fb->base); }

/* @@ -750,13 +745,18 @@ void intel_fbdev_initial_config(void *data, async_cookie_t cookie) void intel_fbdev_fini(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; - if (!dev_priv->fbdev) + struct intel_fbdev *ifbdev = dev_priv->fbdev; + + if (!ifbdev) return;

flush_work(&dev_priv->fbdev_suspend_work); - async_synchronize_full(); - intel_fbdev_destroy(dev, dev_priv->fbdev); + + intel_fbdev_destroy(ifbdev->helper.fbdev); + drm_fb_helper_fini(&ifbdev->helper); + drm_framebuffer_unregister_private(&ifbdev->fb->base); + drm_framebuffer_remove(&ifbdev->fb->base); kfree(dev_priv->fbdev); dev_priv->fbdev = NULL; } diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c b/drivers/gpu/drm/nouveau/nouveau_fbcon.c index 6751553..7b7b112 100644 --- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c +++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c @@ -305,6 +305,9 @@ nouveau_fbcon_zfill(struct drm_device *dev, struct nouveau_fbdev *fbcon) }

static int +nouveau_fbcon_destroy(struct drm_device *dev, struct nouveau_fbdev *fbcon); + +static int nouveau_fbcon_create(struct drm_fb_helper *helper, struct drm_fb_helper_surface_size *sizes) { @@ -322,6 +325,11 @@ nouveau_fbcon_create(struct drm_fb_helper *helper, struct pci_dev *pdev = dev->pdev; int size, ret;

+ if (helper->fbdev) { + nouveau_fbcon_accel_fini(dev); + nouveau_fbcon_destroy(dev, fbcon); + } + mode_cmd.width = sizes->surface_width; mode_cmd.height = sizes->surface_height;

@@ -467,7 +475,6 @@ nouveau_fbcon_destroy(struct drm_device *dev, struct nouveau_fbdev *fbcon) drm_gem_object_unreference_unlocked(&nouveau_fb->nvbo->gem); nouveau_fb->nvbo = NULL; } - drm_fb_helper_fini(&fbcon->helper); drm_framebuffer_unregister_private(&nouveau_fb->base); drm_framebuffer_cleanup(&nouveau_fb->base); return 0; @@ -567,6 +574,7 @@ nouveau_fbcon_fini(struct drm_device *dev)

nouveau_fbcon_accel_fini(dev); nouveau_fbcon_destroy(dev, drm->fbcon); + drm_fb_helper_fini(&drm->fbcon->helper); kfree(drm->fbcon); drm->fbcon = NULL; } diff --git a/drivers/gpu/drm/radeon/radeon_fb.c b/drivers/gpu/drm/radeon/radeon_fb.c index aeb6767..88b9f32 100644 --- a/drivers/gpu/drm/radeon/radeon_fb.c +++ b/drivers/gpu/drm/radeon/radeon_fb.c @@ -216,6 +216,8 @@ out_unref: return ret; }

+static int radeon_fbdev_destroy(struct radeon_fbdev *rfbdev); + static int radeonfb_create(struct drm_fb_helper *helper, struct drm_fb_helper_surface_size *sizes) { @@ -231,6 +233,9 @@ static int radeonfb_create(struct drm_fb_helper *helper, int ret; unsigned long tmp;

+ if (helper->fbdev) + radeon_fbdev_destroy(rfbdev); + mode_cmd.width = sizes->surface_width; mode_cmd.height = sizes->surface_height;

@@ -337,7 +342,7 @@ void radeon_fb_output_poll_changed(struct radeon_device *rdev) drm_fb_helper_hotplug_event(&rdev->mode_info.rfbdev->helper); }

-static int radeon_fbdev_destroy(struct drm_device *dev, struct radeon_fbdev *rfbdev) +static int radeon_fbdev_destroy(struct radeon_fbdev *rfbdev) { struct fb_info *info; struct radeon_framebuffer *rfb = &rfbdev->rfb; @@ -355,7 +360,6 @@ static int radeon_fbdev_destroy(struct drm_device *dev, struct radeon_fbdev *rfb radeonfb_destroy_pinned_object(rfb->obj); rfb->obj = NULL; } - drm_fb_helper_fini(&rfbdev->helper); drm_framebuffer_unregister_private(&rfb->base); drm_framebuffer_cleanup(&rfb->base);

@@ -419,7 +423,8 @@ void radeon_fbdev_fini(struct radeon_device *rdev) if (!rdev->mode_info.rfbdev) return;

- radeon_fbdev_destroy(rdev->ddev, rdev->mode_info.rfbdev); + radeon_fbdev_destroy(rdev->mode_info.rfbdev); + drm_fb_helper_fini(&rdev->mode_info.rfbdev->helper); kfree(rdev->mode_info.rfbdev); rdev->mode_info.rfbdev = NULL; }

The retina MacBook Pro uses an eDP panel and a gmux controller to switch the panel between its two GPUs. Unfortunately it seems that it cannot switch the AUX channel separately from the main link.

But we can emulate switching of DDC/AUX in software by using the active client as a proxy to talk to the panel.

Allow storing pointers to each client's struct i2c_adapter (for DDC) and struct drm_dp_aux. Allow retrieving the active client's structures but constrain access to vga_switcheroo clients to prevent non-clients from using proxying.

Drivers store AUX first, then DDC because they access the DPCD before the EDID. Retrieving AUX is only allowed if DDC is also stored, thereby avoiding race condition where AUX is already stored but not DDC and the inactive client uses AUX then fails on retrieving the EDID via DDC.

Upon storing DDC, generate hotplug event so that already registered inactive clients reprobe once the active client has registered its DDC/AUX structures.

Based (loosely) on patches by Matthew Garrett mjg59@srcf.ucam.org who let the active client stash the EDID and the first 8 bytes of the DPCD (receiver capabilities) in vga_switcheroo where the inactive client would subsequently pick it up. It turns out that the drivers are unhappy with just 8 bytes of DPCD, they need access to the full DPCD to set up their outputs. Switching in software gives us more options (even write access to the DPCD if need be): http://www.codon.org.uk/~mjg59/tmp/retina_patches/0016-vga_switcheroo-Allow-...

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=88861 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=61115 Tested-by: Paul Hordiienko pvt.gord@gmail.com [MBP 6,2 2010 intel ILK + nvidia GT216 pre-retina] Tested-by: William Brown william@blackhats.net.au [MBP 8,2 2011 intel SNB + amd turks pre-retina] Tested-by: Lukas Wunner lukas@wunner.de [MBP 9,1 2012 intel IVB + nvidia GK107 pre-retina] Tested-by: Bruno Bierbaumer bruno@bierbaumer.net [MBP 11,3 2013 intel HSW + nvidia GK107 retina -- work in progress]

Signed-off-by: Lukas Wunner lukas@wunner.de --- drivers/gpu/vga/vga_switcheroo.c | 62 ++++++++++++++++++++++++++++++++++++++++ include/linux/vga_switcheroo.h | 11 +++++++ 2 files changed, 73 insertions(+)

diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c index 94b0b6f..0c52eb4 100644 --- a/drivers/gpu/vga/vga_switcheroo.c +++ b/drivers/gpu/vga/vga_switcheroo.c @@ -19,6 +19,7 @@ */

#include <drm/drm_crtc_helper.h> +#include <drm/drm_dp_helper.h>

#include <linux/module.h> #include <linux/seq_file.h> @@ -27,6 +28,7 @@ #include <linux/debugfs.h> #include <linux/fb.h>

+#include <linux/i2c.h> #include <linux/pci.h> #include <linux/console.h> #include <linux/vga_switcheroo.h> @@ -37,6 +39,8 @@ struct vga_switcheroo_client { struct pci_dev *pdev; struct fb_info *fb_info; + struct i2c_adapter *ddc; + struct drm_dp_aux *aux; struct work_struct reprobe_work; int pwr_state; const struct vga_switcheroo_client_ops *ops; @@ -355,6 +359,64 @@ out: } EXPORT_SYMBOL(vga_switcheroo_unlock_ddc);

+void vga_switcheroo_set_ddc(struct pci_dev *pdev, struct i2c_adapter *ddc) +{ + struct vga_switcheroo_client *client; + + mutex_lock(&vgasr_mutex); + client = find_client_from_pci(&vgasr_priv.clients, pdev); + if (client) + client->ddc = ddc; + mutex_unlock(&vgasr_mutex); + + /* DDC is stored after AUX on eDP, so we have both now */ + if (client->active) + vga_switcheroo_reprobe_inactive_clients(); +} +EXPORT_SYMBOL(vga_switcheroo_set_ddc); + +struct i2c_adapter *vga_switcheroo_get_ddc(struct pci_dev *pdev) +{ + struct vga_switcheroo_client *active = NULL; + + mutex_lock(&vgasr_mutex); + if (find_client_from_pci(&vgasr_priv.clients, pdev)) + active = find_active_client(&vgasr_priv.clients); + mutex_unlock(&vgasr_mutex); + if (!active) + return NULL; + + return active->ddc; +} +EXPORT_SYMBOL(vga_switcheroo_get_ddc); + +void vga_switcheroo_set_aux(struct pci_dev *pdev, struct drm_dp_aux *aux) +{ + struct vga_switcheroo_client *client; + + mutex_lock(&vgasr_mutex); + client = find_client_from_pci(&vgasr_priv.clients, pdev); + if (client) + client->aux = aux; + mutex_unlock(&vgasr_mutex); +} +EXPORT_SYMBOL(vga_switcheroo_set_aux); + +struct drm_dp_aux *vga_switcheroo_get_aux(struct pci_dev *pdev) +{ + struct vga_switcheroo_client *active = NULL; + + mutex_lock(&vgasr_mutex); + if (find_client_from_pci(&vgasr_priv.clients, pdev)) + active = find_active_client(&vgasr_priv.clients); + mutex_unlock(&vgasr_mutex); + if (!active || !active->ddc) + return NULL; + + return active->aux; +} +EXPORT_SYMBOL(vga_switcheroo_get_aux); + static int vga_switcheroo_show(struct seq_file *m, void *v) { struct vga_switcheroo_client *client; diff --git a/include/linux/vga_switcheroo.h b/include/linux/vga_switcheroo.h index b935d83..1d4c07e 100644 --- a/include/linux/vga_switcheroo.h +++ b/include/linux/vga_switcheroo.h @@ -10,7 +10,10 @@ #ifndef _LINUX_VGA_SWITCHEROO_H_ #define _LINUX_VGA_SWITCHEROO_H_

+#include <drm/drm_dp_helper.h> + #include <linux/fb.h> +#include <linux/i2c.h>

struct pci_dev;

@@ -56,6 +59,10 @@ void vga_switcheroo_client_fb_set(struct pci_dev *dev,

int vga_switcheroo_lock_ddc(struct pci_dev *pdev); int vga_switcheroo_unlock_ddc(struct pci_dev *pdev); +void vga_switcheroo_set_ddc(struct pci_dev *pdev, struct i2c_adapter *ddc); +struct i2c_adapter *vga_switcheroo_get_ddc(struct pci_dev *pdev); +void vga_switcheroo_set_aux(struct pci_dev *pdev, struct drm_dp_aux *aux); +struct drm_dp_aux *vga_switcheroo_get_aux(struct pci_dev *pdev);

int vga_switcheroo_register_handler(struct vga_switcheroo_handler *handler); void vga_switcheroo_unregister_handler(void); @@ -77,6 +84,10 @@ static inline int vga_switcheroo_register_client(struct pci_dev *dev, 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 -ENODEV; } static inline int vga_switcheroo_unlock_ddc(struct pci_dev *pdev) { return -ENODEV; } +static inline void vga_switcheroo_set_ddc(struct pci_dev *pdev, struct i2c_adapter *ddc) {} +static inline struct i2c_adapter *vga_switcheroo_get_ddc(struct pci_dev *pdev) { return NULL; } +static inline void vga_switcheroo_set_aux(struct pci_dev *pdev, struct drm_dp_aux *aux) {} +static inline struct drm_dp_aux *vga_switcheroo_get_aux(struct pci_dev *pdev) { return NULL; } static inline int vga_switcheroo_register_handler(struct vga_switcheroo_handler *handler) { return 0; } static inline int vga_switcheroo_register_audio_client(struct pci_dev *pdev, const struct vga_switcheroo_client_ops *ops,

The retina MacBook Pro uses an eDP panel and a gmux controller to switch the panel between its two GPUs. Unfortunately it seems that it cannot switch the AUX channel separately from the main link.

But we can emulate switching of DDC/AUX in software by using the active client as a proxy to talk to the panel.

When reading the DPCD or EDID, store the struct drm_dp_aux and struct i2c_adapter (for DDC) in vga_switcheroo. Retrieve the active client's structures and use them in lieu of the client's own structures.

To avoid using proxying with external DP-connected displays, this is constrained to eDP connectors and further constrained by vga_switcheroo to its clients.

Based (loosely) on patches by Matthew Garrett mjg59@srcf.ucam.org who used stashing instead of proxying and changed each driver rather than the generic helper functions: http://www.codon.org.uk/~mjg59/tmp/retina_patches/0025-i915-Use-vga_switcher... http://www.codon.org.uk/~mjg59/tmp/retina_patches/0026-nouveau-Use-vga_switc...

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=88861 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=61115 Tested-by: Paul Hordiienko pvt.gord@gmail.com [MBP 6,2 2010 intel ILK + nvidia GT216 pre-retina] Tested-by: William Brown william@blackhats.net.au [MBP 8,2 2011 intel SNB + amd turks pre-retina] Tested-by: Lukas Wunner lukas@wunner.de [MBP 9,1 2012 intel IVB + nvidia GK107 pre-retina] Tested-by: Bruno Bierbaumer bruno@bierbaumer.net [MBP 11,3 2013 intel HSW + nvidia GK107 retina -- work in progress]

Signed-off-by: Lukas Wunner lukas@wunner.de --- drivers/gpu/drm/drm_dp_helper.c | 14 ++++++++++++++ drivers/gpu/drm/drm_edid.c | 12 ++++++++++++ 2 files changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index 80a02a4..f34622f 100644 --- a/drivers/gpu/drm/drm_dp_helper.c +++ b/drivers/gpu/drm/drm_dp_helper.c @@ -27,6 +27,7 @@ #include <linux/errno.h> #include <linux/sched.h> #include <linux/i2c.h> +#include <linux/vga_switcheroo.h> #include <drm/drm_dp_helper.h> #include <drm/drmP.h>

@@ -239,6 +240,19 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request, ssize_t drm_dp_dpcd_read(struct drm_dp_aux *aux, unsigned int offset, void *buffer, size_t size) { + struct pci_dev *pdev = to_pci_dev(aux->dev); + struct drm_dp_aux *proxy_aux; + + if (aux->connector && + aux->connector->connector_type == DRM_MODE_CONNECTOR_eDP) { + vga_switcheroo_set_aux(pdev, aux); + proxy_aux = vga_switcheroo_get_aux(pdev); + if (proxy_aux && proxy_aux != aux) { + DRM_DEBUG_KMS("Using vga_switcheroo active client as proxy\n"); + aux = proxy_aux; + } + } + return drm_dp_dpcd_access(aux, DP_AUX_NATIVE_READ, offset, buffer, size); } diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index cdb2fa1..b4b0e01 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -1379,6 +1379,18 @@ struct edid *drm_get_edid(struct drm_connector *connector, { struct edid *edid; struct pci_dev *pdev = connector->dev->pdev; + struct i2c_adapter *proxy_ddc; + + if (connector->connector_type == DRM_MODE_CONNECTOR_LVDS || + connector->connector_type == DRM_MODE_CONNECTOR_eDP) { + vga_switcheroo_set_ddc(pdev, adapter); + proxy_ddc = vga_switcheroo_get_ddc(pdev); + if (proxy_ddc && proxy_ddc != adapter) { + DRM_DEBUG_KMS("Using vga_switcheroo active client as proxy\n"); + adapter = proxy_ddc; + pdev = to_pci_dev(proxy_ddc->dev.parent); + } + }

vga_switcheroo_lock_ddc(pdev);

The pre-retina MacBook Pro uses an LVDS panel and a gmux controller to switch the panel between its two GPUs. While the gmux is able to switch the DDC lines, we can also emulate DDC switching in software by using the active client as a proxy to talk to the panel. This gives us two ways to switch, one requiring the apple-gmux driver and the other requiring the active client's driver.

To that end, when probing DDC, if we're unable to talk to the panel ourselves and the connector in question is of type LVDS, try proxying via the vga_switcheroo active client.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=88861 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=61115 Tested-by: Paul Hordiienko pvt.gord@gmail.com [MBP 6,2 2010 intel ILK + nvidia GT216 pre-retina] Tested-by: William Brown william@blackhats.net.au [MBP 8,2 2011 intel SNB + amd turks pre-retina] Tested-by: Lukas Wunner lukas@wunner.de [MBP 9,1 2012 intel IVB + nvidia GK107 pre-retina] Tested-by: Bruno Bierbaumer bruno@bierbaumer.net [MBP 11,3 2013 intel HSW + nvidia GK107 retina -- work in progress]

Signed-off-by: Lukas Wunner lukas@wunner.de --- drivers/gpu/drm/nouveau/nouveau_connector.c | 6 ++++++ 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c index 159df7f..d1a6982 100644 --- a/drivers/gpu/drm/nouveau/nouveau_connector.c +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c @@ -149,12 +149,18 @@ nouveau_connector_ddc_detect(struct drm_connector *connector) break; } else if (nv_encoder->i2c) { + struct i2c_adapter *proxy_ddc; int ret; vga_switcheroo_lock_ddc(dev->pdev); ret = nv_probe_i2c(nv_encoder->i2c, 0x50); vga_switcheroo_unlock_ddc(dev->pdev); if (ret) break; + proxy_ddc = vga_switcheroo_get_ddc(dev->pdev); + if (nv_encoder->dcb->type == DCB_OUTPUT_LVDS && + nv_connector->type == DCB_CONNECTOR_LVDS && + proxy_ddc && drm_probe_ddc(proxy_ddc)) + break; } }

On Mon, 31 Aug 2015, Jesse Barnes jbarnes@virtuousgeek.org wrote:

On 07/15/2015 04:57 AM, Lukas Wunner wrote:

...

Commit 92122789b2d6 ("drm/i915: preserve SSC if previously set v3") added code to intel_modeset_gem_init to override the SSC status read from VBT with the SSC status set by BIOS.

However, intel_modeset_gem_init is invoked *after* intel_modeset_init, which calls intel_setup_outputs, which *modifies* SSC status by way of intel_init_pch_refclk. So unlike advertised, intel_modeset_gem_init doesn't preserve the SSC status set by BIOS but whatever intel_init_pch_refclk decided on.

This is a problem on dual gpu laptops such as the MacBook Pro which require either a handler to switch DDC lines, or the discrete gpu to proxy DDC/AUX communication: Both the handler and the discrete gpu may initialize after the i915 driver, and consequently, an LVDS connector may initially seem disconnected and the SSC therefore is disabled by intel_init_pch_refclk, but on reprobe the connector may turn out to be connected and the SSC must then be enabled.

Due to 92122789b2d6 however, the SSC is not enabled on reprobe since it is assumed BIOS disabled it while in fact it was disabled by intel_init_pch_refclk.

Also, because the SSC status is preserved so late, the preserved value only ever gets used on resume but not on panel initialization: intel_modeset_init calls intel_init_display which indirectly calls intel_panel_use_ssc via multiple subroutines, *before* the BIOS value overrides the VBT value in intel_modeset_gem_init (intel_panel_use_ssc is the sole user of dev_priv->vbt.lvds_use_ssc).

Fix this by moving the code introduced by 92122789b2d6 from intel_modeset_gem_init to intel_modeset_init before the invocation of intel_setup_outputs and intel_init_display.

Add a DRM_DEBUG_KMS as suggested way back by Jani: http://lists.freedesktop.org/archives/intel-gfx/2014-June/046666.html

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=88861 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=61115 Tested-by: Paul Hordiienko pvt.gord@gmail.com [MBP 6,2 2010 intel ILK + nvidia GT216 pre-retina] Tested-by: William Brown william@blackhats.net.au [MBP 8,2 2011 intel SNB + amd turks pre-retina] Tested-by: Lukas Wunner lukas@wunner.de [MBP 9,1 2012 intel IVB + nvidia GK107 pre-retina] Tested-by: Bruno Bierbaumer bruno@bierbaumer.net [MBP 11,3 2013 intel HSW + nvidia GK107 retina -- work in progress]

Fixes: 92122789b2d6 ("drm/i915: preserve SSC if previously set v3") Signed-off-by: Lukas Wunner lukas@wunner.de

---

drivers/gpu/drm/i915/intel_display.c | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index af0bcfe..6335883 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -14893,6 +14893,24 @@ void intel_modeset_init(struct drm_device *dev) if (INTEL_INFO(dev)->num_pipes == 0) return;

• /*

• * There may be no VBT; and if the BIOS enabled SSC we can
  

• * just keep using it to avoid unnecessary flicker.  Whereas if the
  

• * BIOS isn't using it, don't assume it will work even if the VBT
  

• * indicates as much.
  

• */
  

• if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)) {

• bool bios_lvds_use_ssc = !!(I915_READ(PCH_DREF_CONTROL) &
  

• 			    DREF_SSC1_ENABLE);
  

• if (dev_priv->vbt.lvds_use_ssc != bios_lvds_use_ssc) {
  

• 	DRM_DEBUG_KMS("SSC %sabled by BIOS, overriding VBT which says %sabled\n",
  

• 		     bios_lvds_use_ssc ? "en" : "dis",
  

• 		     dev_priv->vbt.lvds_use_ssc ? "en" : "dis");
  

• 	dev_priv->vbt.lvds_use_ssc = bios_lvds_use_ssc;
  

• }
  

• }
• intel_init_display(dev); intel_init_audio(dev);

@@ -15446,7 +15464,6 @@ err:

void intel_modeset_gem_init(struct drm_device *dev) {

• struct drm_i915_private *dev_priv = dev->dev_private; struct drm_crtc *c; struct drm_i915_gem_object *obj; int ret;

@@ -15455,16 +15472,6 @@ void intel_modeset_gem_init(struct drm_device *dev) intel_init_gt_powersave(dev); mutex_unlock(&dev->struct_mutex);

• /*

• * There may be no VBT; and if the BIOS enabled SSC we can
  

• * just keep using it to avoid unnecessary flicker.  Whereas if the
  

• * BIOS isn't using it, don't assume it will work even if the VBT
  

• * indicates as much.
  

• */
  

• if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev))

• dev_priv->vbt.lvds_use_ssc = !!(I915_READ(PCH_DREF_CONTROL) &
  

• 				DREF_SSC1_ENABLE);
  

• intel_modeset_init_hw(dev);

  intel_setup_overlay(dev);

Yeah looks good (and I'm having deja vu here; I thought I ran into the same ordering issue at one point in a report from krh, but I guess I never fixed it; didn't have test hw at the time).

Reviewed-by: Jesse Barnes jbarnes@virtuousgeek.org

Pushed this one patch to drm-intel-next-fixes, thanks for the patch and review.

BR, Jani.

Thanks, Jesse _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Hi Daniel,

thanks for taking a look at the patch set.

On Wed, Aug 12, 2015 at 04:25:52PM +0200, Daniel Vetter wrote:

On Tue, Apr 21, 2015 at 10:39:45AM +0200, Lukas Wunner wrote:

...

This reverts commit 26814ce68904c9faf977c90edac798156311981f.

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

[...]

...

--- a/drivers/gpu/vga/vga_switcheroo.c +++ b/drivers/gpu/vga/vga_switcheroo.c @@ -214,20 +214,6 @@ find_active_client(struct list_head *head) return NULL; }

-struct pci_dev *vga_switcheroo_get_active_client(void) -{

• struct vga_switcheroo_client *client;
• struct pci_dev *pdev = NULL;
• mutex_lock(&vgasr_mutex);
• client = find_active_client(&vgasr_priv.clients);
• if (client)

• pdev = client->pdev;
  

• mutex_unlock(&vgasr_mutex);
• return pdev;

-} -EXPORT_SYMBOL(vga_switcheroo_get_active_client);

you just added this earlier in this very series. Please reorder/squash patches so that this isn't required.

I would have to squash patches 2, 4 (by Seth Forshee), 5 (by Dave Airlie), 6 and 7 (mine). The work of two of these authors would only be acknowledged in the commit message and the history how the code evolved over the course of 3 years would not be reflected in the git repo.

Are you sure? (y/n)

I deliberately didn't squash to preserve authorship and history but if you're forcing me at point blank I'll do it. ;-)

Context: Seth Forshee of Canonical came up with patches in 2012 which Dave Airlie didn't like. He rewrote them and left them as a WIP in his git repo where I picked them up. Matthew Garrett posted patches of his own last year but since they were based on Seth Forshee's code, they didn't get merged either.

The first MacBooks with dual GPUs were introduced 2008, it's 2015 now and we're still missing support in the mainline kernel. The issue is not so much that GPU switching doesn't work (the screen just turns black) but energy consumption because the discrete GPU is used by default and the integrated GPU isn't turned off.

So, machines with huge marketshare + shoddy dual GPU support for years = problem.

We need to fix this, hence the patch set.

Thanks,

Lukas

On Fri, Mar 27, 2015 at 12:29:40PM +0100, Lukas Wunner wrote:

Unlock DDC lines if drm_probe_ddc() fails.

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

Use mutex_trylock(&vgasr_mutex) in vga_switcheroo_unlock_ddc() to avoid locking inversion when one client locks vgasr_mutex on entering vga_switcheroo_lock_ddc() and tries to acquire ddc_lock while another client is holding ddc_lock and tries to acquire vgasr_mutex on entering vga_switcheroo_unlock_ddc().

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

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

So essentially you have

bool locked = mutex_trylock();

/* nothing that looks at locked at all */

if (locked) mutex_unlock;

This doesn't protect anything at all and makes it look _very_ fishy.

I haven't dug deeper yet. -Daniel

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=88861 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=61115 Tested-by: Paul Hordiienko pvt.gord@gmail.com [MBP 6,2 2010 intel ILK + nvidia GT216 pre-retina] Tested-by: William Brown william@blackhats.net.au [MBP 8,2 2011 intel SNB + amd turks pre-retina] Tested-by: Lukas Wunner lukas@wunner.de [MBP 9,1 2012 intel IVB + nvidia GK107 pre-retina] Tested-by: Bruno Bierbaumer bruno@bierbaumer.net [MBP 11,3 2013 intel HSW + nvidia GK107 retina -- work in progress]

Signed-off-by: Lukas Wunner lukas@wunner.de

drivers/gpu/drm/drm_edid.c | 9 ++-- drivers/gpu/vga/vga_switcheroo.c | 110 ++++++++++++++++++++++---------------- drivers/platform/x86/apple-gmux.c | 15 +++++- include/linux/vga_switcheroo.h | 3 +- 4 files changed, 87 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 505eed1..cdb2fa1 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -1378,17 +1378,20 @@ struct edid *drm_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter) { struct edid *edid;

• struct pci_dev *pdev = connector->dev->pdev;

• vga_switcheroo_lock_ddc(connector->dev->pdev);

• vga_switcheroo_lock_ddc(pdev);

• if (!drm_probe_ddc(adapter))

• if (!drm_probe_ddc(adapter)) {

• vga_switcheroo_unlock_ddc(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);

• vga_switcheroo_unlock_ddc(pdev);

  return edid;

} diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c index 0223020..f02e7fc 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> @@ -59,7 +60,7 @@ struct vgasr_priv { struct vga_switcheroo_handler *handler;

struct mutex ddc_lock;

• struct pci_dev *old_ddc_owner;

• enum vga_switcheroo_client_id old_ddc_owner;

};

#define ID_BIT_AUDIO 0x100 @@ -276,33 +277,24 @@ EXPORT_SYMBOL(vga_switcheroo_client_fb_set);

int vga_switcheroo_lock_ddc(struct pci_dev *pdev) {

• struct vga_switcheroo_client *client;
• int ret = 0;
• int id;

• int ret, id;

  mutex_lock(&vgasr_mutex);

• if (!vgasr_priv.handler) {

• if (!vgasr_priv.handler || !vgasr_priv.handler->switch_ddc) { ret = -ENODEV; goto out; }

• if (vgasr_priv.handler->switch_ddc) {

• mutex_lock(&vgasr_priv.ddc_lock);
  

• mutex_lock(&vgasr_priv.ddc_lock);
• id = vgasr_priv.handler->get_client_id(pdev);
• ret = vgasr_priv.handler->switch_ddc(id);

• client = find_active_client(&vgasr_priv.clients);
  

• if (!client) {
  

• 	mutex_unlock(&vgasr_priv.ddc_lock);
  

• 	ret = -ENODEV;
  

• 	goto out;
  

• }
  

• vgasr_priv.old_ddc_owner = client->pdev;
  

• if (client->pdev == pdev)
  

• 	goto out;
  

• id = vgasr_priv.handler->get_client_id(pdev);
  

• ret = vgasr_priv.handler->switch_ddc(id);
  

• }

• if (ret < 0) {

• pr_err("vga_switcheroo: failed to switch DDC lines\n");
  

• mutex_unlock(&vgasr_priv.ddc_lock);
  

• } else

• vgasr_priv.old_ddc_owner = ret;
  
  

out: mutex_unlock(&vgasr_mutex); @@ -312,25 +304,33 @@ EXPORT_SYMBOL(vga_switcheroo_lock_ddc);

int vga_switcheroo_unlock_ddc(struct pci_dev *pdev) {

• int ret = 0;
• int id;
• mutex_lock(&vgasr_mutex);

• int ret, id;
• bool vgasr_mutex_acquired = mutex_trylock(&vgasr_mutex);

• if (!vgasr_priv.handler) {

• ret = -ENODEV;
  

• if (!mutex_is_locked(&vgasr_priv.ddc_lock)) {

• ret = -EINVAL;
  

  goto out; }

• if (vgasr_priv.handler->switch_ddc) {

• if (vgasr_priv.old_ddc_owner != pdev) {
  

• 	id = vgasr_priv.handler->get_client_id(vgasr_priv.old_ddc_owner);
  

• 	ret = vgasr_priv.handler->switch_ddc(id);
  

• }
  

• vgasr_priv.old_ddc_owner = NULL;
  

• if (!vgasr_priv.handler || !vgasr_priv.handler->switch_ddc) {

• pr_err("vga_switcheroo: handler disappeared\n");
  

  mutex_unlock(&vgasr_priv.ddc_lock);

• ret = -ENODEV;
  

• goto out;
  

  }
• 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)
  

• 	pr_err("vga_switcheroo: failed to switch DDC lines\n");
  

• } else

• ret = vgasr_priv.old_ddc_owner;
  

• mutex_unlock(&vgasr_priv.ddc_lock);

out:

• mutex_unlock(&vgasr_mutex);

• if (vgasr_mutex_acquired)

• mutex_unlock(&vgasr_mutex);
  

  return ret;

} EXPORT_SYMBOL(vga_switcheroo_unlock_ddc); @@ -433,14 +433,26 @@ static int vga_switchto_stage2(struct vga_switcheroo_client *new_client) }

if (vgasr_priv.handler->switch_ddc) {

• mutex_lock(&vgasr_priv.ddc_lock);
  

  ret = vgasr_priv.handler->switch_ddc(new_client->id);

• if (ret)
  

• mutex_unlock(&vgasr_priv.ddc_lock);
  

• if (ret < 0) {
  

• 	pr_err("vga_switcheroo: failed to switch DDC lines\n");
  return ret;
  

• }
  

  }

  ret = vgasr_priv.handler->switchto(new_client->id);

• if (ret)

• goto restore_ddc;
  

• if (ret) {

• pr_err("vga_switcheroo: failed to switch to client %d\n",
  

•        new_client->id);
  

• active->active = true;
  

• /* 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);

@@ -452,14 +464,6 @@ static int vga_switchto_stage2(struct vga_switcheroo_client *new_client)

new_client->active = true; return 0;

-restore_ddc:

• if (vgasr_priv.handler->switch_ddc) {

• int id = (new_client->id == VGA_SWITCHEROO_IGD) ?
  

• 		VGA_SWITCHEROO_DIS : VGA_SWITCHEROO_IGD;
  

• ret = vgasr_priv.handler->switch_ddc(id);
  

• }
• return ret;

}

static bool check_can_switch(void) @@ -561,6 +565,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) {
  

• 		pr_err("vga_switcheroo: failed to switch DDC lines\n");
  

• 		goto out;
  

• 	}
  

• }
  

  ret = vgasr_priv.handler->switchto(client_id); goto out; }

@@ -716,6 +729,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)
  

• 	pr_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 f0a55a4..08bdf1e 100644 --- a/drivers/platform/x86/apple-gmux.c +++ b/drivers/platform/x86/apple-gmux.c @@ -275,11 +275,24 @@ static const struct backlight_ops gmux_bl_ops = {

static int gmux_switch_ddc(enum vga_switcheroo_client_id id) {

• enum vga_switcheroo_client_id old_ddc_owner;
• if (gmux_read8(apple_gmux_data, GMUX_PORT_SWITCH_DDC) == 1)

• old_ddc_owner = VGA_SWITCHEROO_IGD;
  

• else

• old_ddc_owner = VGA_SWITCHEROO_DIS;
  

• pr_debug("Switching gmux DDC from %d to %d\n", old_ddc_owner, id);
• if (id == old_ddc_owner)

• return old_ddc_owner;
  

• if (id == VGA_SWITCHEROO_IGD) gmux_write8(apple_gmux_data, GMUX_PORT_SWITCH_DDC, 1); else gmux_write8(apple_gmux_data, GMUX_PORT_SWITCH_DDC, 2);

• return 0;

• return old_ddc_owner;

}

static int gmux_switchto(enum vga_switcheroo_client_id id) diff --git a/include/linux/vga_switcheroo.h b/include/linux/vga_switcheroo.h index 352bef3..39b25b0 100644 --- a/include/linux/vga_switcheroo.h +++ b/include/linux/vga_switcheroo.h @@ -77,7 +77,8 @@ static inline void vga_switcheroo_unregister_client(struct pci_dev *dev) {} static inline int vga_switcheroo_register_client(struct pci_dev *dev, const struct vga_switcheroo_client_ops *ops, bool driver_power_control) { return 0; } static inline void vga_switcheroo_client_fb_set(struct pci_dev *dev, struct fb_info *info) {} -static inline void vga_switcheroo_switch_ddc(struct pci_dev *pdev) { return NULL; } +static inline int vga_switcheroo_lock_ddc(struct pci_dev *pdev) { return -ENODEV; } +static inline int vga_switcheroo_unlock_ddc(struct pci_dev *pdev) { return -ENODEV; } 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, -- 1.8.5.2 (Apple Git-48)

---

dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel