From patchwork Mon Mar 2 13:36:01 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alan Cox X-Patchwork-Id: 5940331 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 8A22BBF440 for ; Wed, 4 Mar 2015 20:51:25 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id AD02A202EC for ; Wed, 4 Mar 2015 20:51:24 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 673B82034F for ; Wed, 4 Mar 2015 20:51:23 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id A4BE46E71F; Wed, 4 Mar 2015 12:50:42 -0800 (PST) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org X-Greylist: delayed 2565 seconds by postgrey-1.34 at gabe; Mon, 02 Mar 2015 06:19:01 PST Received: from lxorguk.ukuu.org.uk (251.110.2.81.in-addr.arpa [81.2.110.251]) by gabe.freedesktop.org (Postfix) with ESMTP id 856D689232 for ; Mon, 2 Mar 2015 06:19:01 -0800 (PST) Received: from localhost.localdomain (proxy [81.2.110.250]) by lxorguk.ukuu.org.uk (8.14.8/8.14.1) with ESMTP id t22DqEVe014508; Mon, 2 Mar 2015 13:52:19 GMT Date: Mon, 2 Mar 2015 13:36:01 +0000 From: One Thousand Gnomes To: Alexey Khoroshilov Subject: Re: drm/gma500: Possible deadlock in gma_power_begin() Message-ID: <20150302133601.06d009c4@lxorguk.ukuu.org.uk> In-Reply-To: <54F1D889.40207@ispras.ru> References: <54F1D889.40207@ispras.ru> Organization: Intel Corporation X-Mailer: Claws Mail 3.11.1 (GTK+ 2.24.25; x86_64-redhat-linux-gnu) MIME-Version: 1.0 X-Mailman-Approved-At: Wed, 04 Mar 2015 12:50:37 -0800 Cc: ldv-project@linuxtesting.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org 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: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, T_RP_MATCHES_RCVD,UNPARSEABLE_RELAY autolearn=ham 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 On Sat, 28 Feb 2015 18:02:33 +0300 Alexey Khoroshilov wrote: > gma_power_begin() starts with locking power_ctrl_lock spinlock and then, > if gma_resume_pci(dev->pdev) succeed, it calls > psb_irq_preinstall(dev); > psb_irq_postinstall(dev); > > psb_irq_postinstall() does some pipestat enabling/disabling dance: > if (dev->vblank[0].enabled) > psb_enable_pipestat(dev_priv, 0, PIPE_VBLANK_INTERRUPT_ENABLE); > else > psb_disable_pipestat(dev_priv, 0, PIPE_VBLANK_INTERRUPT_ENABLE); > > where > void psb_enable_pipestat(struct drm_psb_private *dev_priv, int pipe, u32 > mask) > { > if ((dev_priv->pipestat[pipe] & mask) != mask) { > u32 reg = psb_pipestat(pipe); > dev_priv->pipestat[pipe] |= mask; > /* Enable the interrupt, clear any pending status */ > if (gma_power_begin(dev_priv->dev, false)) { > u32 writeVal = PSB_RVDC32(reg); > writeVal |= (mask | (mask >> 16)); > PSB_WVDC32(writeVal, reg); > (void) PSB_RVDC32(reg); > gma_power_end(dev_priv->dev); > } > } > } > > So, if a flag in dev_priv->pipestat[pipe] is not in agreement with > dev->vblank[0].enabled, > we will have a call to gma_power_begin() again and got an unavoidable > deadlock. I don't think it can ever happen in the current driver but the fix also looks trivial. I think its sufficient to do this commit 76d209fc77516f638c6db0342fddd0930d8e5957 Author: Alan Date: Mon Mar 2 13:32:46 2015 +0000 gma500: drop the power lock before doing the irq pre/postinstall The irq install is not locked or managed by the power lock. It will take the lock again internally to do the vblank update so we would deadlock if a change was ever needed. diff --git a/drivers/gpu/drm/gma500/power.c b/drivers/gpu/drm/gma500/power.c index b6b135f..8d101f4 100644 --- a/drivers/gpu/drm/gma500/power.c +++ b/drivers/gpu/drm/gma500/power.c @@ -266,11 +266,11 @@ bool gma_power_begin(struct drm_device *dev, bool force_on) /* Ok power up needed */ ret = gma_resume_pci(dev->pdev); if (ret == 0) { - psb_irq_preinstall(dev); - psb_irq_postinstall(dev); pm_runtime_get(&dev->pdev->dev); dev_priv->display_count++; spin_unlock_irqrestore(&power_ctrl_lock, flags); + psb_irq_preinstall(dev); + psb_irq_postinstall(dev); return true; } out_false: