From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 1/6] drm/i915: assert_spin_locked for pipestat interrupt enable/disable Date: Thu, 4 Jul 2013 21:22:26 +0200 Message-ID: <20130704192226.GJ18285@phenom.ffwll.local> References: <1372348335-6824-1-git-send-email-daniel.vetter@ffwll.ch> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-ee0-f48.google.com (mail-ee0-f48.google.com [74.125.83.48]) by gabe.freedesktop.org (Postfix) with ESMTP id 2E344E5ECD for ; Thu, 4 Jul 2013 12:22:27 -0700 (PDT) Received: by mail-ee0-f48.google.com with SMTP id b47so932687eek.35 for ; Thu, 04 Jul 2013 12:22:27 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1372348335-6824-1-git-send-email-daniel.vetter@ffwll.ch> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Intel Graphics Development Cc: Daniel Vetter List-Id: intel-gfx@lists.freedesktop.org On Thu, Jun 27, 2013 at 05:52:10PM +0200, Daniel Vetter wrote: > Just to keep the paranoia equal also sprinkle locking asserts over the > pipestat interrupt enable/disable functions. > > Again this results in false positives in the interrupt setup. Add > bogo-locking for these and a big comment explaining why it's there and > that it's indeed unnecessary. > > v2: Fix up the spelling fail Paulo spotted in comments. > > Reviewed-by: Paulo Zanoni > Signed-off-by: Daniel Vetter Oops, somehow I've missed this patch here. It's now merged into dinq, too. -Daniel > --- > drivers/gpu/drm/i915/i915_irq.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index 465588f..e7c879b 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -322,6 +322,8 @@ i915_enable_pipestat(drm_i915_private_t *dev_priv, int pipe, u32 mask) > u32 reg = PIPESTAT(pipe); > u32 pipestat = I915_READ(reg) & 0x7fff0000; > > + assert_spin_locked(&dev_priv->irq_lock); > + > if ((pipestat & mask) == mask) > return; > > @@ -337,6 +339,8 @@ i915_disable_pipestat(drm_i915_private_t *dev_priv, int pipe, u32 mask) > u32 reg = PIPESTAT(pipe); > u32 pipestat = I915_READ(reg) & 0x7fff0000; > > + assert_spin_locked(&dev_priv->irq_lock); > + > if ((pipestat & mask) == 0) > return; > > @@ -2826,6 +2830,7 @@ static int valleyview_irq_postinstall(struct drm_device *dev) > u32 gt_irqs; > u32 enable_mask; > u32 pipestat_enable = PLANE_FLIP_DONE_INT_EN_VLV; > + unsigned long irqflags; > > enable_mask = I915_DISPLAY_PORT_INTERRUPT; > enable_mask |= I915_DISPLAY_PIPE_A_EVENT_INTERRUPT | > @@ -2851,9 +2856,13 @@ static int valleyview_irq_postinstall(struct drm_device *dev) > I915_WRITE(PIPESTAT(1), 0xffff); > POSTING_READ(VLV_IER); > > + /* Interrupt setup is already guaranteed to be single-threaded, this is > + * just to make the assert_spin_locked check happy. */ > + spin_lock_irqsave(&dev_priv->irq_lock, irqflags); > i915_enable_pipestat(dev_priv, 0, pipestat_enable); > i915_enable_pipestat(dev_priv, 0, PIPE_GMBUS_EVENT_ENABLE); > i915_enable_pipestat(dev_priv, 1, pipestat_enable); > + spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); > > I915_WRITE(VLV_IIR, 0xffffffff); > I915_WRITE(VLV_IIR, 0xffffffff); > @@ -3335,6 +3344,7 @@ static int i965_irq_postinstall(struct drm_device *dev) > drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private; > u32 enable_mask; > u32 error_mask; > + unsigned long irqflags; > > /* Unmask the interrupts that we always want on. */ > dev_priv->irq_mask = ~(I915_ASLE_INTERRUPT | > @@ -3353,7 +3363,11 @@ static int i965_irq_postinstall(struct drm_device *dev) > if (IS_G4X(dev)) > enable_mask |= I915_BSD_USER_INTERRUPT; > > + /* Interrupt setup is already guaranteed to be single-threaded, this is > + * just to make the assert_spin_locked check happy. */ > + spin_lock_irqsave(&dev_priv->irq_lock, irqflags); > i915_enable_pipestat(dev_priv, 0, PIPE_GMBUS_EVENT_ENABLE); > + spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); > > /* > * Enable some error detection, note the instruction error mask > -- > 1.8.1.4 > -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch