Unlock DDC lines if drm_probe_ddc() fails.
intel_dp_detect_dpcd() calls drm_probe_ddc(), needs to lock DDC lines.
Don't lock vgasr_mutex in vga_switcheroo_lock/unlock_ddc(), it's already locked in vga_switcheroo_debugfs_write(), leading to a deadlock upon reprobe in stage2 when the new client reads the EDID. Locking ddc_lock is sufficient.
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.
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.
Signed-off-by: Lukas Wunner lukas@wunner.de --- drivers/gpu/drm/drm_edid.c | 4 +- drivers/gpu/drm/i915/intel_dp.c | 6 ++- drivers/gpu/vga/vga_switcheroo.c | 82 ++++++++++++++++++++++----------------- drivers/platform/x86/apple-gmux.c | 15 ++++++- include/linux/vga_switcheroo.h | 3 +- 5 files changed, 71 insertions(+), 39 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index f91593b..8950722 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -1329,8 +1329,10 @@ struct edid *drm_get_edid(struct drm_connector *connector,
vga_switcheroo_lock_ddc(connector->dev->pdev);
- if (!drm_probe_ddc(adapter)) + 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) 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/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c index 0223020..2534d84 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,12 +277,9 @@ 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;
- mutex_lock(&vgasr_mutex); - if (!vgasr_priv.handler) { ret = -ENODEV; goto out; @@ -290,22 +288,17 @@ int vga_switcheroo_lock_ddc(struct pci_dev *pdev) if (vgasr_priv.handler->switch_ddc) { mutex_lock(&vgasr_priv.ddc_lock);
- 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) { + 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: - mutex_unlock(&vgasr_mutex); return ret; } EXPORT_SYMBOL(vga_switcheroo_lock_ddc); @@ -314,7 +307,6 @@ int vga_switcheroo_unlock_ddc(struct pci_dev *pdev) { int ret = 0; int id; - mutex_lock(&vgasr_mutex);
if (!vgasr_priv.handler) { ret = -ENODEV; @@ -322,15 +314,17 @@ int vga_switcheroo_unlock_ddc(struct pci_dev *pdev) }
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; + 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: - mutex_unlock(&vgasr_mutex); return ret; } EXPORT_SYMBOL(vga_switcheroo_unlock_ddc); @@ -433,14 +427,24 @@ 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) { + printk(KERN_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) { + 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); @@ -452,14 +456,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 +557,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; } @@ -716,6 +721,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 1cf242c..9aafbde 100644 --- a/drivers/platform/x86/apple-gmux.c +++ b/drivers/platform/x86/apple-gmux.c @@ -273,11 +273,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..8963799 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 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,