From patchwork Fri Aug 14 16:50:15 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Lukas Wunner X-Patchwork-Id: 7022591 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id CEB48C05AC for ; Sun, 16 Aug 2015 19:12:15 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id A3DA62055B for ; Sun, 16 Aug 2015 19:12:14 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 16DDF20558 for ; Sun, 16 Aug 2015 19:12:13 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 425F4895C1; Sun, 16 Aug 2015 12:12:12 -0700 (PDT) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mailout3.hostsharing.net (mailout3.hostsharing.net [176.9.242.54]) by gabe.freedesktop.org (Postfix) with ESMTPS id 908BD895C1 for ; Sun, 16 Aug 2015 12:12:10 -0700 (PDT) Received: from h08.hostsharing.net (h08.hostsharing.net [83.223.95.28]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mailout3.hostsharing.net (Postfix) with ESMTPS id 390131003D156; Sun, 16 Aug 2015 21:12:09 +0200 (CEST) Received: from localhost (6-38-90-81.adsl.cmo.de [81.90.38.6]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) by h08.hostsharing.net (Postfix) with ESMTPSA id A327B603DB40; Sun, 16 Aug 2015 21:12:05 +0200 (CEST) X-Mailbox-Line: From 7984d8eacbb0386ca52e3fdf2ad2554dc90ff1fe Mon Sep 17 00:00:00 2001 Message-Id: <7984d8eacbb0386ca52e3fdf2ad2554dc90ff1fe.1439739853.git.lukas@wunner.de> From: Lukas Wunner Date: Fri, 14 Aug 2015 18:50:15 +0200 Subject: [PATCH v2.1 1/3] vga_switcheroo: Add support for switching only the DDC To: dri-devel@lists.freedesktop.org Cc: Daniel Vetter , Dave Airlie , Seth Forshee X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Spam-Status: No, score=-4.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Originally by Seth Forshee , 2012-10-04: During graphics driver initialization it's useful to be able to mux only the DDC to the inactive client in order to read the EDID. Add a switch_ddc callback to allow capable handlers to provide this functionality, and add vga_switcheroo_switch_ddc() to allow DRM to mux only the DDC. Modified by Dave Airlie , 2012-12-22: I can't figure out why I didn't like this, but I rewrote this [...] to lock/unlock the ddc lines [...]. I think I'd prefer something like that otherwise the interface got really ugly. Modified by Lukas Wunner , 2015-08-14: Don't grab vgasr_mutex in lock_ddc/unlock_ddc to avoid the following deadlock scenarios: (a) During switch (with vgasr_mutex already held), GPU is woken and probes its outputs, tries to re-acquire vgasr_mutex to lock DDC lines. (b) Likewise during switch, GPU is suspended and calls cancel_delayed_work_sync to stop output polling, if poll task is running at this moment we may wait forever for it to finish. If we don't grab vgasr_mutex the only bad thing that can happen is that the handler suddenly disappears. So block handler deregistration until DDC lines are unlocked again. WARN_ON_ONCE if unlock_ddc is called without calling lock_ddc first. 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. Fix bug in stage2 where no client had the active attribute set if switching failed. Fix erroneous interface documentation. 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. v2.1: Overhaul locking, squash commits (requested by Daniel Vetter) Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=88861 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=61115 Tested-by: Lukas Wunner [MBP 9,1 2012 intel IVB + nvidia GK107 pre-retina] Cc: Seth Forshee Cc: Dave Airlie Signed-off-by: Lukas Wunner --- drivers/gpu/vga/vga_switcheroo.c | 73 +++++++++++++++++++++++++++++++++++++--- include/linux/vga_switcheroo.h | 6 ++++ 2 files changed, 74 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c index 37ac7b5..ac4ac12 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 + - 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 @@ -57,6 +58,8 @@ struct vgasr_priv { struct list_head clients; struct vga_switcheroo_handler *handler; + struct mutex ddc_lock; + int old_ddc_owner; }; #define ID_BIT_AUDIO 0x100 @@ -70,6 +73,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) @@ -122,12 +126,14 @@ EXPORT_SYMBOL(vga_switcheroo_register_handler); void vga_switcheroo_unregister_handler(void) { mutex_lock(&vgasr_mutex); + mutex_lock(&vgasr_priv.ddc_lock); vgasr_priv.handler = NULL; if (vgasr_priv.active) { pr_info("vga_switcheroo: disabled\n"); vga_switcheroo_debugfs_fini(&vgasr_priv); vgasr_priv.active = false; } + mutex_unlock(&vgasr_priv.ddc_lock); mutex_unlock(&vgasr_mutex); } EXPORT_SYMBOL(vga_switcheroo_unregister_handler); @@ -256,6 +262,43 @@ 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 id; + + mutex_lock(&vgasr_priv.ddc_lock); + + if (!vgasr_priv.handler || !vgasr_priv.handler->switch_ddc) + return vgasr_priv.old_ddc_owner = -ENODEV; + + id = vgasr_priv.handler->get_client_id(pdev); + return vgasr_priv.old_ddc_owner = vgasr_priv.handler->switch_ddc(id); +} +EXPORT_SYMBOL(vga_switcheroo_lock_ddc); + +int vga_switcheroo_unlock_ddc(struct pci_dev *pdev) +{ + int ret, id; + + if (WARN_ON_ONCE(!mutex_is_locked(&vgasr_priv.ddc_lock))) + return -EINVAL; + + if (vgasr_priv.old_ddc_owner < 0) { + mutex_unlock(&vgasr_priv.ddc_lock); + return -ENODEV; + } + + 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); + else + ret = vgasr_priv.old_ddc_owner; + mutex_unlock(&vgasr_priv.ddc_lock); + + return ret; +} +EXPORT_SYMBOL(vga_switcheroo_unlock_ddc); + static int vga_switcheroo_show(struct seq_file *m, void *v) { struct vga_switcheroo_client *client; @@ -341,8 +384,6 @@ static int vga_switchto_stage2(struct vga_switcheroo_client *new_client) if (!active) return 0; - active->active = false; - set_audio_state(active->id, VGA_SWITCHEROO_OFF); if (new_client->fb_info) { @@ -353,9 +394,21 @@ 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); + vgasr_priv.handler->switch_ddc(new_client->id); + mutex_unlock(&vgasr_priv.ddc_lock); + } ret = vgasr_priv.handler->switchto(new_client->id); - if (ret) + if (ret) { + if (vgasr_priv.handler->switch_ddc) { + mutex_lock(&vgasr_priv.ddc_lock); + vgasr_priv.handler->switch_ddc(active->id); + mutex_unlock(&vgasr_priv.ddc_lock); + } return ret; + } + active->active = false; if (new_client->ops->reprobe) new_client->ops->reprobe(new_client->pdev); @@ -468,6 +521,11 @@ 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); + vgasr_priv.handler->switch_ddc(client_id); + mutex_unlock(&vgasr_priv.ddc_lock); + } ret = vgasr_priv.handler->switchto(client_id); goto out; } @@ -623,6 +681,11 @@ 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); + vgasr_priv.handler->switch_ddc(VGA_SWITCHEROO_IGD); + mutex_unlock(&vgasr_priv.ddc_lock); + } if (vgasr_priv.handler->switchto) vgasr_priv.handler->switchto(VGA_SWITCHEROO_IGD); vga_switcheroo_power_switch(pdev, VGA_SWITCHEROO_OFF); diff --git a/include/linux/vga_switcheroo.h b/include/linux/vga_switcheroo.h index b483abd..62f303f 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); @@ -54,6 +55,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 +76,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 -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,