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@xxxxxxxxx> [MBP 6,2 2010 intel ILK + nvidia GT216 pre-retina] Tested-by: William Brown <william@xxxxxxxxxxxxxxxx> [MBP 8,2 2011 intel SNB + amd turks pre-retina] Tested-by: Lukas Wunner <lukas@xxxxxxxxx> [MBP 9,1 2012 intel IVB + nvidia GK107 pre-retina] Tested-by: Bruno Bierbaumer <bruno@xxxxxxxxxxxxxx> [MBP 11,3 2013 intel HSW + nvidia GK107 retina -- work in progress] Signed-off-by: Lukas Wunner <lukas@xxxxxxxxx> --- 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@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
