From patchwork Tue Mar 19 08:56:57 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jiri Kosina X-Patchwork-Id: 2299451 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by patchwork2.kernel.org (Postfix) with ESMTP id 5C8C4DFB79 for ; Tue, 19 Mar 2013 08:57:22 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 2F86AE648F for ; Tue, 19 Mar 2013 01:57:22 -0700 (PDT) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mx2.suse.de (cantor2.suse.de [195.135.220.15]) by gabe.freedesktop.org (Postfix) with ESMTP id 16109E5EC7 for ; Tue, 19 Mar 2013 01:57:07 -0700 (PDT) Received: from relay2.suse.de (unknown [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 8FF3BA51F5; Tue, 19 Mar 2013 09:57:02 +0100 (CET) Date: Tue, 19 Mar 2013 09:56:57 +0100 (CET) From: Jiri Kosina To: Chris Wilson Subject: Re: [PATCH] drm/i915: stop using GMBUS IRQs on Gen4 chips (was Re: [3.9-rc1] irq 16: nobody cared (was [3.9-rc1] very poor interrupt responses)) In-Reply-To: <20130318170424.GB5571@cantiga.alporthouse.com> Message-ID: References: <1363280764.26318.4.camel@thor.lan> <5657408.cYX6iimiA4@vostro.rjw.lan> <5142E7F0.4020002@gmail.com> <20130315153249.GA32425@kroah.com> <20130315154739.GA1024@kroah.com> <20130318082156.GO9021@phenom.ffwll.local> <20130318170424.GB5571@cantiga.alporthouse.com> User-Agent: Alpine 2.00 (LNX 1167 2008-08-23) MIME-Version: 1.0 Cc: Harald Arnesen , Peter Hurley , Greg KH , Daniel Vetter , USB list , Kernel development list , "Rafael J. Wysocki" , linux-acpi@vger.kernel.org, Alan Stern , dri-devel@lists.freedesktop.org, linux-pci@vger.kernel.org, Bjorn Helgaas , Yinghai Lu , Shawn Starr , Thomas Meyer X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+patchwork-dri-devel=patchwork.kernel.org@lists.freedesktop.org Errors-To: dri-devel-bounces+patchwork-dri-devel=patchwork.kernel.org@lists.freedesktop.org On Mon, 18 Mar 2013, Chris Wilson wrote: > > +#define HAS_GMBUS_IRQ(dev) (INTEL_INFO(dev)->gen >= 5) > > void > > intel_i2c_reset(struct drm_device *dev) > > { > > struct drm_i915_private *dev_priv = dev->dev_private; > > I915_WRITE(dev_priv->gpio_mmio_base + GMBUS0, 0); > > - I915_WRITE(dev_priv->gpio_mmio_base + GMBUS4, 0); > > + if (HAS_GMBUS_IRQ(dev)) > > + I915_WRITE(dev_priv->gpio_mmio_base + GMBUS4, 0); > > There should not be any harm in always clearing GMBUS4, it exists on all > platforms. > > > } > > > > static void intel_i2c_quirk_set(struct drm_i915_private *dev_priv, bool enable) > > @@ -203,7 +205,6 @@ intel_gpio_setup(struct intel_gmbus *bus, u32 pin) > > algo->data = bus; > > } > > > > -#define HAS_GMBUS_IRQ(dev) (INTEL_INFO(dev)->gen >= 4) > > static int > > gmbus_wait_hw_status(struct drm_i915_private *dev_priv, > > u32 gmbus2_status, > > @@ -214,6 +215,13 @@ gmbus_wait_hw_status(struct drm_i915_private *dev_priv, > > u32 gmbus2 = 0; > > DEFINE_WAIT(wait); > > > > + if (!HAS_GMBUS_IRQ(dev_priv->dev)) { > > + int ret; > > + ret = wait_for((gmbus2 = I915_READ(GMBUS2 + reg_offset)) & > > + (GMBUS_SATOER | gmbus2_status), > > + 50); > > This should couple up to the normal return code that chooses the > appropriate return value based on gmbus2. > > How about just using: > if (!HAS_GMBUS_IRQ(dev_priv->dev)) gmbus4_irq_en = 0; > and the existing wait loop? I explicitly wanted to avoid touching GMBUS4 register, as the real cause of the failure is not clear. But, as Yinghai Lu points out, the problem is most likely caused by interrupt disabling not working properly (see his very good point regarding DisINTx+ and INTx+ discrepancy), so zeroing the register out should work .... and it indeed does in my case, hence the (tested) patch below. I think it's a 3.9-rc material, and I am all open to debug this further for 3.10 so that the race is closed and gmbus irqs can be used on Gen4 platform properly. From: Jiri Kosina Subject: [PATCH] drm/i915: stop using GMBUS IRQs on Gen4 chips Commit 28c70f162 ("drm/i915: use the gmbus irq for waits") switched to using GMBUS irqs instead of GPIO bit-banging for chipset generations 4 and above. It turns out though that on many systems this leads to spurious interrupts being generated, long after the register write to disable the IRQs has been issued. Flushing of the register writes by POSTING_READ() directly after the register write doesn't work either. Disable using of GMBUS IRQs on Gen4 systems before the root cause is found and revert back to old behavior. Signed-off-by: Jiri Kosina Acked-by: Chris Wilson --- drivers/gpu/drm/i915/intel_i2c.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c index acf8aec..9934724 100644 --- a/drivers/gpu/drm/i915/intel_i2c.c +++ b/drivers/gpu/drm/i915/intel_i2c.c @@ -203,7 +203,7 @@ intel_gpio_setup(struct intel_gmbus *bus, u32 pin) algo->data = bus; } -#define HAS_GMBUS_IRQ(dev) (INTEL_INFO(dev)->gen >= 4) +#define HAS_GMBUS_IRQ(dev) (INTEL_INFO(dev)->gen >= 5) static int gmbus_wait_hw_status(struct drm_i915_private *dev_priv, u32 gmbus2_status, @@ -214,6 +214,8 @@ gmbus_wait_hw_status(struct drm_i915_private *dev_priv, u32 gmbus2 = 0; DEFINE_WAIT(wait); + if (!HAS_GMBUS_IRQ(dev_priv->dev)) + gmbus4_irq_en = 0; /* Important: The hw handles only the first bit, so set only one! Since * we also need to check for NAKs besides the hw ready/idle signal, we * need to wake up periodically and check that ourselves. */