Received: by 2002:ac0:a594:0:0:0:0:0 with SMTP id m20-v6csp3403351imm; Fri, 25 May 2018 05:14:57 -0700 (PDT) X-Google-Smtp-Source: AB8JxZpYtygALSu/VYkzeBrtUZ3pV/ip5MWfxBrkC1o0ulhBtr6HYltZEP94fudb0StXvQP0DB6D X-Received: by 2002:a17:902:6e4:: with SMTP id 91-v6mr2357931plh.63.1527250497076; Fri, 25 May 2018 05:14:57 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1527250497; cv=none; d=google.com; s=arc-20160816; b=BJOUV3OWeunuswojIUaSGCyGifBOIfrzqxtGfFzZCeqmnhIyQvjQCxkQlGWbaT10uZ uew0EIruW1p1aRofOiCifj3wBrRO+o6HBn0pd4nIcuJoriWmVtXRUwM8dz3+GOBveu8v xH8Ke9w6owe2g3nABF9bqcZghZnkMECg6TfG5HTSPDHNXRIclNJqhsKEci9b7oknZLuf d34430amIxdbcbv2kpMabGptV6ZUX9BMySY36dN/rbQmzRpeZ/h5pDYzv8jndjBqbVxs zwyGNCK7qRF6XKmbM/JHyRAISEQetnLPDhq7jngElZyFGbqNsWG92tBUJjIWDg086wdM qY0g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :arc-authentication-results; bh=139HbGW5Uz8HTHxT6t10WmdVrBe1OYSm+8+DE4VzHg4=; b=O/neDHmi1zkhctLuQZS2TLjeyM82rGoSYLF+XokH97SUCP/alq/LcXpcJ00N7w7/4Z Tw+LtfSTCIuzXbWkFzApk5crjX8FmcKC3YL03FWuftKREOBjNVJhQ/8UjUBszHmQPvPA oJGqHbv/aw/2yPu+z6bVrYg9qFHuQxPuZlZu8D4g04czW8f9TwPLaGBvMygiWfPy/PWe 0JWF5fgHgxSUT8zQi30UImb3yat7ejH/wV6WRpYmnkhkhEkj8I9imo9txK/CpJW9YuXU QEP5PXocX2r6Sk0ELpLReKUZmarcPItezd4AyfsdE9dIYpdmm9UTBNTtWVeBuUDsYlrK J2Jw== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id j10-v6si23902155pfn.87.2018.05.25.05.14.42; Fri, 25 May 2018 05:14:57 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S966528AbeEYMOc convert rfc822-to-8bit (ORCPT + 99 others); Fri, 25 May 2018 08:14:32 -0400 Received: from gloria.sntech.de ([95.129.55.99]:60766 "EHLO gloria.sntech.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S966243AbeEYMOa (ORCPT ); Fri, 25 May 2018 08:14:30 -0400 Received: from natm.fit.cvut.cz ([147.32.232.146] helo=phil.localnet) by gloria.sntech.de with esmtpsa (TLS1.1:DHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.80) (envelope-from ) id 1fMBc2-0006g1-Vs; Fri, 25 May 2018 14:14:27 +0200 From: Heiko Stuebner To: Marc Zyngier Cc: Ezequiel Garcia , xxm@rock-chips.com, Joerg Roedel , Jeffy Chen , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, Tomeu Vizoso , jcliang@chromium.org, linux-rockchip@lists.infradead.org, iommu@lists.linux-foundation.org, Enric =?ISO-8859-1?Q?Balletb=F2?= , tfiga@chromium.org, Robin Murphy , linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH] drm/rockchip: vop: fix irq disabled after vop driver probed Date: Fri, 25 May 2018 14:14:20 +0200 Message-ID: <4318500.8KycslRrNH@phil> In-Reply-To: <8495bffd-2a52-ca85-1bc9-afc3ea0af8e4@arm.com> References: <25470133.K8n9sLBzRS@diego> <8495bffd-2a52-ca85-1bc9-afc3ea0af8e4@arm.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8BIT Content-Type: text/plain; charset="iso-8859-1" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Marc, Am Freitag, 25. Mai 2018, 13:07:02 CEST schrieb Marc Zyngier: > [Thanks Robin for pointing me to that patch.] > > Hi Heiko, > > On 24/05/18 23:06, Heiko St?bner wrote: > > From: Sandy Huang > > > > The vop irq is shared between vop and iommu and irq probing in the > > iommu driver moved to the probe function recently. This can in some > > cases lead to a stall if the irq is triggered while the vop driver > > still has it disabled. > > > > But there is no real need to disable the irq, as the vop can simply > > also track its enabled state and ignore irqs in that case. > > > > So remove the enable/disable handling and add appropriate condition > > to the irq handler. > > > > Signed-off-by: Sandy Huang > > [added an actual commit message] > > Signed-off-by: Heiko Stuebner > > --- > > Hi Ezequiel, > > > > this patch came from a discussion I had with Rockchip people over the > > iommu changes and resulting issues back in april, but somehow was > > forgotten and not posted to the lists. Correcting that now. > > > > So removing the enable/disable voodoo on the shared interrupt is > > the preferred way. > > > > drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 13 ++++++------- > > 1 file changed, 7 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > > index 510cdf0..61493d4 100644 > > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > > @@ -549,8 +549,6 @@ static int vop_enable(struct drm_crtc *crtc) > > > > spin_unlock(&vop->reg_lock); > > > > - enable_irq(vop->irq); > > - > > drm_crtc_vblank_on(crtc); > > > > return 0; > > @@ -596,8 +594,6 @@ static void vop_crtc_atomic_disable(struct drm_crtc *crtc, > > > > vop_dsp_hold_valid_irq_disable(vop); > > > > - disable_irq(vop->irq); > > - > > vop->is_enabled = false; > > > > /* > > @@ -1168,6 +1164,13 @@ static irqreturn_t vop_isr(int irq, void *data) > > int ret = IRQ_NONE; > > > > /* > > + * since the irq is shared with iommu, iommu irq is enabled before vop > > + * enable, so before vop enable we do nothing. > > + */ > > + if (!vop->is_enabled) > > + return IRQ_NONE; > > + > > What guarantee do we have that the IOMMU will actually service that > interrupt? Can't we get into a situation where the interrupt gets > ignored by both drivers and subsequently disabled by the core irq code > as being spurious? > > From what I understood of the way things are plugged together on the > rockchip platforms, each individual VOP is behind a single IOMMU, hence > there there is hardly any point in handling IOMMU interrupts if the VOP > is off. Yeah, which is probably the reason that patch fell through the cracks initially. I resurrected it from the internal discussion because of the issue described below. > But I'm missing the actual reason behind this patch. Could you enlighten me? Recently the iommu driver moved the irq request from the iommu enablement to its probe function. With that change Ezequiel got various stalls, see https://lkml.org/lkml/2018/5/24/1347 Heiko