[Intel-gfx] [PATCH 45/53] drm/i915/bdw: Do not call intel_runtime_pm_get() in an interrupt

[Daniel Vetter]

On Fri, Jun 13, 2014 at 04:38:03PM +0100, oscar.mateo at intel.com wrote:
> From: Oscar Mateo <oscar.mateo at intel.com>
> 
> Or with a spinlock grabbed, because it might sleep, which is not
> a nice thing to do. Instead, do the runtime_pm get/put together
> with the create/destroy request, and handle the forcewake get/put
> directly.
> 
> Signed-off-by: Oscar Mateo <oscar.mateo at intel.com>

Looks like a fixup that should be squashed into relevant earlier patches.
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_lrc.c | 26 +++++++++++++++++++++-----
>  1 file changed, 21 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index d33e622..ea4b358 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -159,6 +159,7 @@ static void execlists_elsp_write(struct intel_engine_cs *ring,
>  	struct drm_i915_private *dev_priv = ring->dev->dev_private;
>  	uint64_t temp = 0;
>  	uint32_t desc[4];
> +	unsigned long flags;
>  
>  	/* XXX: You must always write both descriptors in the order below. */
>  	if (ctx_obj1)
> @@ -172,9 +173,17 @@ static void execlists_elsp_write(struct intel_engine_cs *ring,
>  	desc[3] = (u32)(temp >> 32);
>  	desc[2] = (u32)temp;
>  
> -	/* Set Force Wakeup bit to prevent GT from entering C6 while
> -	 * ELSP writes are in progress */
> -	gen6_gt_force_wake_get(dev_priv, FORCEWAKE_ALL);
> +	/* Set Force Wakeup bit to prevent GT from entering C6 while ELSP writes
> +	 * are in progress.
> +	 *
> +	 * The other problem is that we can't just call gen6_gt_force_wake_get()
> +	 * because that function calls intel_runtime_pm_get(), which might sleep.
> +	 * Instead, we do the runtime_pm_get/put when creating/destroying requests.
> +	 */
> +	spin_lock_irqsave(&dev_priv->uncore.lock, flags);
> +	if (dev_priv->uncore.forcewake_count++ == 0)
> +		dev_priv->uncore.funcs.force_wake_get(dev_priv, FORCEWAKE_ALL);
> +	spin_unlock_irqrestore(&dev_priv->uncore.lock, flags);
>  
>  	I915_WRITE(RING_ELSP(ring), desc[1]);
>  	I915_WRITE(RING_ELSP(ring), desc[0]);
> @@ -185,8 +194,11 @@ static void execlists_elsp_write(struct intel_engine_cs *ring,
>  	/* ELSP is a write only register, so this serves as a posting read */
>  	POSTING_READ(RING_EXECLIST_STATUS(ring));
>  
> -	/* Release Force Wakeup */
> -	gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL);
> +	/* Release Force Wakeup (see the big comment above). */
> +	spin_lock_irqsave(&dev_priv->uncore.lock, flags);
> +	if (--dev_priv->uncore.forcewake_count == 0)
> +		dev_priv->uncore.funcs.force_wake_put(dev_priv, FORCEWAKE_ALL);
> +	spin_unlock_irqrestore(&dev_priv->uncore.lock, flags);
>  }
>  
>  static int execlists_ctx_write_tail(struct drm_i915_gem_object *ctx_obj, u32 tail)
> @@ -353,6 +365,9 @@ static void execlists_free_request_task(struct work_struct *work)
>  	struct intel_ctx_submit_request *req =
>  			container_of(work, struct intel_ctx_submit_request, work);
>  	struct drm_device *dev = req->ring->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +
> +	intel_runtime_pm_put(dev_priv);
>  
>  	mutex_lock(&dev->struct_mutex);
>  	i915_gem_context_unreference(req->ctx);
> @@ -378,6 +393,7 @@ static int execlists_context_queue(struct intel_engine_cs *ring,
>  	req->ring = ring;
>  	req->tail = tail;
>  	INIT_WORK(&req->work, execlists_free_request_task);
> +	intel_runtime_pm_get(dev_priv);
>  
>  	spin_lock_irqsave(&ring->execlist_lock, flags);
>  
> -- 
> 1.9.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at 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