Re: [PATCH] drm/i915/ilk: special case enabling of PCU_EVENT interrupt

From: Daniel Vetter <daniel@ffwll.ch>
To: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: socketcan@hartkopp.net, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915/ilk: special case enabling of PCU_EVENT interrupt
Date: Tue, 26 Aug 2014 09:23:54 +0200	[thread overview]
Message-ID: <20140826072354.GY15520@phenom.ffwll.local> (raw)
In-Reply-To: <1409009095-3621-1-git-send-email-jbarnes@virtuousgeek.org>

On Mon, Aug 25, 2014 at 04:24:55PM -0700, Jesse Barnes wrote:
> This happens in irq_postinstall before we've set the pm._irqs_disabled flag,
> but shouldn't warn.  So add a nowarn variant to allow this to happen w/o
> a backtrace and keep the rest of the IRQ tracking code happy.
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>

Shouldn't we instead just move the pm._irqs_disabled = false in i915_dma.c
right above the drm_irq_install call? In
intel_runtime_pm_restore_interrupts we also set it to false before we call
the various hooks.

Also the commit message is a bit thin on the usual details like which
commits introduced this regression, so that maintainers know where to
apply this to.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_irq.c |   18 ++++++++++++------
>  1 files changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index d5445e7..ec1d9fe 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -132,6 +132,16 @@ static const u32 hpd_status_i915[] = { /* i915 and valleyview are the same */
>  
>  /* For display hotplug interrupt */
>  static void
> +ironlake_enable_display_irq_nowarn(struct drm_i915_private *dev_priv, u32 mask)
> +{
> +	if ((dev_priv->irq_mask & mask) != 0) {
> +		dev_priv->irq_mask &= ~mask;
> +		I915_WRITE(DEIMR, dev_priv->irq_mask);
> +		POSTING_READ(DEIMR);
> +	}
> +}
> +
> +static void
>  ironlake_enable_display_irq(struct drm_i915_private *dev_priv, u32 mask)
>  {
>  	assert_spin_locked(&dev_priv->irq_lock);
> @@ -139,11 +149,7 @@ ironlake_enable_display_irq(struct drm_i915_private *dev_priv, u32 mask)
>  	if (WARN_ON(!intel_irqs_enabled(dev_priv)))
>  		return;
>  
> -	if ((dev_priv->irq_mask & mask) != 0) {
> -		dev_priv->irq_mask &= ~mask;
> -		I915_WRITE(DEIMR, dev_priv->irq_mask);
> -		POSTING_READ(DEIMR);
> -	}
> +	ironlake_enable_display_irq_nowarn(dev_priv, mask);
>  }
>  
>  static void
> @@ -3665,7 +3671,7 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
>  		 * setup is guaranteed to run in single-threaded context. But we
>  		 * need it to make the assert_spin_locked happy. */
>  		spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
> -		ironlake_enable_display_irq(dev_priv, DE_PCU_EVENT);
> +		ironlake_enable_display_irq_nowarn(dev_priv, DE_PCU_EVENT);
>  		spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
>  	}
>  
> -- 
> 1.7.5.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch