intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
From: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 19/21] drm/i915: Move the get/put irq locking into the caller
Date: Tue, 7 Jun 2016 13:46:53 +0100	[thread overview]
Message-ID: <5756C23D.5090508@linux.intel.com> (raw)
In-Reply-To: <1464970133-29859-20-git-send-email-chris@chris-wilson.co.uk>


On 03/06/16 17:08, Chris Wilson wrote:
> With only a single callsite for intel_engine_cs->irq_get and ->irq_put,
> we can reduce the code size by moving the common preamble into the
> caller, and we can also eliminate the reference counting.
>
> For completeness, as we are no longer doing reference counting on irq,
> rename the get/put vfunctions to enable/disable respectively.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/i915_irq.c          |   8 +-
>   drivers/gpu/drm/i915/intel_breadcrumbs.c |  10 +-
>   drivers/gpu/drm/i915/intel_lrc.c         |  34 +---
>   drivers/gpu/drm/i915/intel_ringbuffer.c  | 269 ++++++++++---------------------
>   drivers/gpu/drm/i915/intel_ringbuffer.h  |   5 +-
>   5 files changed, 108 insertions(+), 218 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 14b3d65bb604..5bdb433dde8c 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -259,12 +259,12 @@ static void ilk_update_gt_irq(struct drm_i915_private *dev_priv,
>   	dev_priv->gt_irq_mask &= ~interrupt_mask;
>   	dev_priv->gt_irq_mask |= (~enabled_irq_mask & interrupt_mask);
>   	I915_WRITE(GTIMR, dev_priv->gt_irq_mask);
> -	POSTING_READ(GTIMR);
>   }
>
>   void gen5_enable_gt_irq(struct drm_i915_private *dev_priv, uint32_t mask)
>   {
>   	ilk_update_gt_irq(dev_priv, mask, mask);
> +	POSTING_READ_FW(GTIMR);
>   }

Unrelated hunks?

How is POSTING_READ_FW correct?

Also removes the posting read from disable, OK?

>
>   void gen5_disable_gt_irq(struct drm_i915_private *dev_priv, uint32_t mask)
> @@ -2818,9 +2818,9 @@ ring_idle(struct intel_engine_cs *engine, u32 seqno)
>   }
>
>   static bool
> -ipehr_is_semaphore_wait(struct drm_i915_private *dev_priv, u32 ipehr)
> +ipehr_is_semaphore_wait(struct intel_engine_cs *engine, u32 ipehr)
>   {
> -	if (INTEL_GEN(dev_priv) >= 8) {
> +	if (INTEL_GEN(engine->i915) >= 8) {
>   		return (ipehr >> 23) == 0x1c;
>   	} else {
>   		ipehr &= ~MI_SEMAPHORE_SYNC_MASK;
> @@ -2891,7 +2891,7 @@ semaphore_waits_for(struct intel_engine_cs *engine, u32 *seqno)
>   		return NULL;
>
>   	ipehr = I915_READ(RING_IPEHR(engine->mmio_base));
> -	if (!ipehr_is_semaphore_wait(engine->i915, ipehr))
> +	if (!ipehr_is_semaphore_wait(engine, ipehr))
>   		return NULL;

Two hunks of meh as some would say. :)

>
>   	/*
> diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c
> index 8ab508ed4248..dc65a007fa20 100644
> --- a/drivers/gpu/drm/i915/intel_breadcrumbs.c
> +++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c
> @@ -50,12 +50,18 @@ static void irq_enable(struct intel_engine_cs *engine)
>   	 * just in case.
>   	 */
>   	engine->irq_posted = true;
> -	WARN_ON(!engine->irq_get(engine));
> +
> +	spin_lock_irq(&engine->i915->irq_lock);
> +	engine->irq_enable(engine);
> +	spin_unlock_irq(&engine->i915->irq_lock);
>   }
>
>   static void irq_disable(struct intel_engine_cs *engine)
>   {
> -	engine->irq_put(engine);
> +	spin_lock_irq(&engine->i915->irq_lock);
> +	engine->irq_disable(engine);
> +	spin_unlock_irq(&engine->i915->irq_lock);
> +
>   	engine->irq_posted = false;
>   }
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 32b5eae7dd11..9e19b2c5b3ae 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1578,36 +1578,18 @@ static int gen8_emit_bb_start(struct drm_i915_gem_request *req,
>   	return 0;
>   }
>
> -static bool gen8_logical_ring_get_irq(struct intel_engine_cs *engine)
> +static void gen8_logical_ring_enable_irq(struct intel_engine_cs *engine)
>   {
>   	struct drm_i915_private *dev_priv = engine->i915;
> -	unsigned long flags;
> -
> -	if (WARN_ON(!intel_irqs_enabled(dev_priv)))
> -		return false;
> -
> -	spin_lock_irqsave(&dev_priv->irq_lock, flags);
> -	if (engine->irq_refcount++ == 0) {
> -		I915_WRITE_IMR(engine,
> -			       ~(engine->irq_enable_mask | engine->irq_keep_mask));
> -		POSTING_READ(RING_IMR(engine->mmio_base));
> -	}
> -	spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
> -
> -	return true;
> +	I915_WRITE_IMR(engine,
> +		       ~(engine->irq_enable_mask | engine->irq_keep_mask));
> +	POSTING_READ_FW(RING_IMR(engine->mmio_base));

Hm, more of _FW following normal access. What am I missing? You are not 
by any chance banking on the auto-release window?

>   }
>
> -static void gen8_logical_ring_put_irq(struct intel_engine_cs *engine)
> +static void gen8_logical_ring_disable_irq(struct intel_engine_cs *engine)
>   {
>   	struct drm_i915_private *dev_priv = engine->i915;
> -	unsigned long flags;
> -
> -	spin_lock_irqsave(&dev_priv->irq_lock, flags);
> -	if (--engine->irq_refcount == 0) {
> -		I915_WRITE_IMR(engine, ~engine->irq_keep_mask);
> -		POSTING_READ(RING_IMR(engine->mmio_base));
> -	}
> -	spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
> +	I915_WRITE_IMR(engine, ~engine->irq_keep_mask);

Another posting read gone here?

>   }
>
>   static int gen8_emit_flush(struct drm_i915_gem_request *request,
> @@ -1895,8 +1877,8 @@ logical_ring_default_vfuncs(struct intel_engine_cs *engine)
>   	engine->init_hw = gen8_init_common_ring;
>   	engine->emit_request = gen8_emit_request;
>   	engine->emit_flush = gen8_emit_flush;
> -	engine->irq_get = gen8_logical_ring_get_irq;
> -	engine->irq_put = gen8_logical_ring_put_irq;
> +	engine->irq_enable = gen8_logical_ring_enable_irq;
> +	engine->irq_disable = gen8_logical_ring_disable_irq;
>   	engine->emit_bb_start = gen8_emit_bb_start;
>   	if (IS_BXT_REVID(engine->i915, 0, BXT_REVID_A1))
>   		engine->irq_seqno_barrier = bxt_a_seqno_barrier;
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 30e400d77d23..ba84b469f13f 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -1551,103 +1551,54 @@ gen6_seqno_barrier(struct intel_engine_cs *engine)
>   	spin_unlock_irq(&dev_priv->uncore.lock);
>   }
>
> -static bool
> -gen5_ring_get_irq(struct intel_engine_cs *engine)
> +static void
> +gen5_ring_enable_irq(struct intel_engine_cs *engine)
>   {
> -	struct drm_i915_private *dev_priv = engine->i915;
> -	unsigned long flags;
> -
> -	if (WARN_ON(!intel_irqs_enabled(dev_priv)))
> -		return false;
> -
> -	spin_lock_irqsave(&dev_priv->irq_lock, flags);
> -	if (engine->irq_refcount++ == 0)
> -		gen5_enable_gt_irq(dev_priv, engine->irq_enable_mask);
> -	spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
> -
> -	return true;
> +	gen5_enable_gt_irq(engine->i915, engine->irq_enable_mask);
>   }
>
>   static void
> -gen5_ring_put_irq(struct intel_engine_cs *engine)
> +gen5_ring_disable_irq(struct intel_engine_cs *engine)
>   {
> -	struct drm_i915_private *dev_priv = engine->i915;
> -	unsigned long flags;
> -
> -	spin_lock_irqsave(&dev_priv->irq_lock, flags);
> -	if (--engine->irq_refcount == 0)
> -		gen5_disable_gt_irq(dev_priv, engine->irq_enable_mask);
> -	spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
> +	gen5_disable_gt_irq(engine->i915, engine->irq_enable_mask);
>   }
>
> -static bool
> -i9xx_ring_get_irq(struct intel_engine_cs *engine)
> +static void
> +i9xx_ring_enable_irq(struct intel_engine_cs *engine)
>   {
>   	struct drm_i915_private *dev_priv = engine->i915;
> -	unsigned long flags;
> -
> -	if (!intel_irqs_enabled(dev_priv))
> -		return false;
> -
> -	spin_lock_irqsave(&dev_priv->irq_lock, flags);
> -	if (engine->irq_refcount++ == 0) {
> -		dev_priv->irq_mask &= ~engine->irq_enable_mask;
> -		I915_WRITE(IMR, dev_priv->irq_mask);
> -		POSTING_READ(IMR);
> -	}
> -	spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
>
> -	return true;
> +	dev_priv->irq_mask &= ~engine->irq_enable_mask;
> +	I915_WRITE(IMR, dev_priv->irq_mask);
> +	POSTING_READ_FW(RING_IMR(engine->mmio_base));
>   }
>
>   static void
> -i9xx_ring_put_irq(struct intel_engine_cs *engine)
> +i9xx_ring_disable_irq(struct intel_engine_cs *engine)
>   {
>   	struct drm_i915_private *dev_priv = engine->i915;
> -	unsigned long flags;
>
> -	spin_lock_irqsave(&dev_priv->irq_lock, flags);
> -	if (--engine->irq_refcount == 0) {
> -		dev_priv->irq_mask |= engine->irq_enable_mask;
> -		I915_WRITE(IMR, dev_priv->irq_mask);
> -		POSTING_READ(IMR);
> -	}
> -	spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
> +	dev_priv->irq_mask |= engine->irq_enable_mask;
> +	I915_WRITE(IMR, dev_priv->irq_mask);
>   }
>
> -static bool
> -i8xx_ring_get_irq(struct intel_engine_cs *engine)
> +static void
> +i8xx_ring_enable_irq(struct intel_engine_cs *engine)
>   {
>   	struct drm_i915_private *dev_priv = engine->i915;
> -	unsigned long flags;
>
> -	if (!intel_irqs_enabled(dev_priv))
> -		return false;
> -
> -	spin_lock_irqsave(&dev_priv->irq_lock, flags);
> -	if (engine->irq_refcount++ == 0) {
> -		dev_priv->irq_mask &= ~engine->irq_enable_mask;
> -		I915_WRITE16(IMR, dev_priv->irq_mask);
> -		POSTING_READ16(IMR);
> -	}
> -	spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
> -
> -	return true;
> +	dev_priv->irq_mask &= ~engine->irq_enable_mask;
> +	I915_WRITE16(IMR, dev_priv->irq_mask);
> +	POSTING_READ16(RING_IMR(engine->mmio_base));
>   }
>
>   static void
> -i8xx_ring_put_irq(struct intel_engine_cs *engine)
> +i8xx_ring_disable_irq(struct intel_engine_cs *engine)
>   {
>   	struct drm_i915_private *dev_priv = engine->i915;
> -	unsigned long flags;
>
> -	spin_lock_irqsave(&dev_priv->irq_lock, flags);
> -	if (--engine->irq_refcount == 0) {
> -		dev_priv->irq_mask |= engine->irq_enable_mask;
> -		I915_WRITE16(IMR, dev_priv->irq_mask);
> -		POSTING_READ16(IMR);
> -	}
> -	spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
> +	dev_priv->irq_mask |= engine->irq_enable_mask;
> +	I915_WRITE16(IMR, dev_priv->irq_mask);
>   }
>
>   static int
> @@ -1688,122 +1639,74 @@ i9xx_add_request(struct drm_i915_gem_request *req)
>   	return 0;
>   }
>
> -static bool
> -gen6_ring_get_irq(struct intel_engine_cs *engine)
> +static void
> +gen6_ring_enable_irq(struct intel_engine_cs *engine)
>   {
>   	struct drm_i915_private *dev_priv = engine->i915;
> -	unsigned long flags;
> -
> -	if (WARN_ON(!intel_irqs_enabled(dev_priv)))
> -		return false;
>
> -	spin_lock_irqsave(&dev_priv->irq_lock, flags);
> -	if (engine->irq_refcount++ == 0) {
> -		if (HAS_L3_DPF(dev_priv) && engine->id == RCS)
> -			I915_WRITE_IMR(engine,
> -				       ~(engine->irq_enable_mask |
> -					 GT_PARITY_ERROR(dev_priv)));
> -		else
> -			I915_WRITE_IMR(engine, ~engine->irq_enable_mask);
> -		gen5_enable_gt_irq(dev_priv, engine->irq_enable_mask);
> -	}
> -	spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
> -
> -	return true;
> +	if (HAS_L3_DPF(dev_priv) && engine->id == RCS)
> +		I915_WRITE_IMR(engine,
> +			       ~(engine->irq_enable_mask |
> +				 GT_PARITY_ERROR(dev_priv)));
> +	else
> +		I915_WRITE_IMR(engine, ~engine->irq_enable_mask);
> +	gen5_enable_gt_irq(dev_priv, engine->irq_enable_mask);
>   }
>
>   static void
> -gen6_ring_put_irq(struct intel_engine_cs *engine)
> +gen6_ring_disable_irq(struct intel_engine_cs *engine)
>   {
>   	struct drm_i915_private *dev_priv = engine->i915;
> -	unsigned long flags;
>
> -	spin_lock_irqsave(&dev_priv->irq_lock, flags);
> -	if (--engine->irq_refcount == 0) {
> -		if (HAS_L3_DPF(dev_priv) && engine->id == RCS)
> -			I915_WRITE_IMR(engine, ~GT_PARITY_ERROR(dev_priv));
> -		else
> -			I915_WRITE_IMR(engine, ~0);
> -		gen5_disable_gt_irq(dev_priv, engine->irq_enable_mask);
> -	}
> -	spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
> +	if (HAS_L3_DPF(dev_priv) && engine->id == RCS)
> +		I915_WRITE_IMR(engine, ~GT_PARITY_ERROR(dev_priv));
> +	else
> +		I915_WRITE_IMR(engine, ~0);
> +	gen5_disable_gt_irq(dev_priv, engine->irq_enable_mask);
>   }
>
> -static bool
> -hsw_vebox_get_irq(struct intel_engine_cs *engine)
> +static void
> +hsw_vebox_enable_irq(struct intel_engine_cs *engine)
>   {
>   	struct drm_i915_private *dev_priv = engine->i915;
> -	unsigned long flags;
> -
> -	if (WARN_ON(!intel_irqs_enabled(dev_priv)))
> -		return false;
>
> -	spin_lock_irqsave(&dev_priv->irq_lock, flags);
> -	if (engine->irq_refcount++ == 0) {
> -		I915_WRITE_IMR(engine, ~engine->irq_enable_mask);
> -		gen6_enable_pm_irq(dev_priv, engine->irq_enable_mask);
> -	}
> -	spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
> -
> -	return true;
> +	I915_WRITE_IMR(engine, ~engine->irq_enable_mask);
> +	gen6_enable_pm_irq(dev_priv, engine->irq_enable_mask);
>   }
>
>   static void
> -hsw_vebox_put_irq(struct intel_engine_cs *engine)
> +hsw_vebox_disable_irq(struct intel_engine_cs *engine)
>   {
>   	struct drm_i915_private *dev_priv = engine->i915;
> -	unsigned long flags;
>
> -	spin_lock_irqsave(&dev_priv->irq_lock, flags);
> -	if (--engine->irq_refcount == 0) {
> -		I915_WRITE_IMR(engine, ~0);
> -		gen6_disable_pm_irq(dev_priv, engine->irq_enable_mask);
> -	}
> -	spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
> +	I915_WRITE_IMR(engine, ~0);
> +	gen6_disable_pm_irq(dev_priv, engine->irq_enable_mask);
>   }
>
> -static bool
> -gen8_ring_get_irq(struct intel_engine_cs *engine)
> +static void
> +gen8_ring_enable_irq(struct intel_engine_cs *engine)
>   {
>   	struct drm_i915_private *dev_priv = engine->i915;
> -	unsigned long flags;
>
> -	if (WARN_ON(!intel_irqs_enabled(dev_priv)))
> -		return false;
> -
> -	spin_lock_irqsave(&dev_priv->irq_lock, flags);
> -	if (engine->irq_refcount++ == 0) {
> -		if (HAS_L3_DPF(dev_priv) && engine->id == RCS) {
> -			I915_WRITE_IMR(engine,
> -				       ~(engine->irq_enable_mask |
> -					 GT_RENDER_L3_PARITY_ERROR_INTERRUPT));
> -		} else {
> -			I915_WRITE_IMR(engine, ~engine->irq_enable_mask);
> -		}
> -		POSTING_READ(RING_IMR(engine->mmio_base));
> -	}
> -	spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
> -
> -	return true;
> +	if (HAS_L3_DPF(dev_priv) && engine->id == RCS)
> +		I915_WRITE_IMR(engine,
> +			       ~(engine->irq_enable_mask |
> +				 GT_RENDER_L3_PARITY_ERROR_INTERRUPT));
> +	else
> +		I915_WRITE_IMR(engine, ~engine->irq_enable_mask);
> +	POSTING_READ_FW(RING_IMR(engine->mmio_base));
>   }
>
>   static void
> -gen8_ring_put_irq(struct intel_engine_cs *engine)
> +gen8_ring_disable_irq(struct intel_engine_cs *engine)
>   {
>   	struct drm_i915_private *dev_priv = engine->i915;
> -	unsigned long flags;
>
> -	spin_lock_irqsave(&dev_priv->irq_lock, flags);
> -	if (--engine->irq_refcount == 0) {
> -		if (HAS_L3_DPF(dev_priv) && engine->id == RCS) {
> -			I915_WRITE_IMR(engine,
> -				       ~GT_RENDER_L3_PARITY_ERROR_INTERRUPT);
> -		} else {
> -			I915_WRITE_IMR(engine, ~0);
> -		}
> -		POSTING_READ(RING_IMR(engine->mmio_base));
> -	}
> -	spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
> +	if (HAS_L3_DPF(dev_priv) && engine->id == RCS)
> +		I915_WRITE_IMR(engine,
> +			       ~GT_RENDER_L3_PARITY_ERROR_INTERRUPT);
> +	else
> +		I915_WRITE_IMR(engine, ~0);
>   }
>
>   static int
> @@ -2739,8 +2642,8 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
>   		engine->init_context = intel_rcs_ctx_init;
>   		engine->add_request = gen8_render_add_request;
>   		engine->flush = gen8_render_ring_flush;
> -		engine->irq_get = gen8_ring_get_irq;
> -		engine->irq_put = gen8_ring_put_irq;
> +		engine->irq_enable = gen8_ring_enable_irq;
> +		engine->irq_disable = gen8_ring_disable_irq;
>   		engine->irq_enable_mask = GT_RENDER_USER_INTERRUPT;
>   		if (i915_semaphore_is_enabled(dev_priv)) {
>   			WARN_ON(!dev_priv->semaphore_obj);
> @@ -2754,8 +2657,8 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
>   		engine->flush = gen7_render_ring_flush;
>   		if (IS_GEN6(dev_priv))
>   			engine->flush = gen6_render_ring_flush;
> -		engine->irq_get = gen6_ring_get_irq;
> -		engine->irq_put = gen6_ring_put_irq;
> +		engine->irq_enable = gen6_ring_enable_irq;
> +		engine->irq_disable = gen6_ring_disable_irq;
>   		engine->irq_enable_mask = GT_RENDER_USER_INTERRUPT;
>   		engine->irq_seqno_barrier = gen6_seqno_barrier;
>   		if (i915_semaphore_is_enabled(dev_priv)) {
> @@ -2782,8 +2685,8 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
>   	} else if (IS_GEN5(dev_priv)) {
>   		engine->add_request = i9xx_add_request;
>   		engine->flush = gen4_render_ring_flush;
> -		engine->irq_get = gen5_ring_get_irq;
> -		engine->irq_put = gen5_ring_put_irq;
> +		engine->irq_enable = gen5_ring_enable_irq;
> +		engine->irq_disable = gen5_ring_disable_irq;
>   		engine->irq_seqno_barrier = gen5_seqno_barrier;
>   		engine->irq_enable_mask = GT_RENDER_USER_INTERRUPT;
>   	} else {
> @@ -2793,11 +2696,11 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
>   		else
>   			engine->flush = gen4_render_ring_flush;
>   		if (IS_GEN2(dev_priv)) {
> -			engine->irq_get = i8xx_ring_get_irq;
> -			engine->irq_put = i8xx_ring_put_irq;
> +			engine->irq_enable = i8xx_ring_enable_irq;
> +			engine->irq_disable = i8xx_ring_disable_irq;
>   		} else {
> -			engine->irq_get = i9xx_ring_get_irq;
> -			engine->irq_put = i9xx_ring_put_irq;
> +			engine->irq_enable = i9xx_ring_enable_irq;
> +			engine->irq_disable = i9xx_ring_disable_irq;
>   		}
>   		engine->irq_enable_mask = I915_USER_INTERRUPT;
>   	}
> @@ -2857,8 +2760,8 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
>   		if (INTEL_GEN(dev_priv) >= 8) {
>   			engine->irq_enable_mask =
>   				GT_RENDER_USER_INTERRUPT << GEN8_VCS1_IRQ_SHIFT;
> -			engine->irq_get = gen8_ring_get_irq;
> -			engine->irq_put = gen8_ring_put_irq;
> +			engine->irq_enable = gen8_ring_enable_irq;
> +			engine->irq_disable = gen8_ring_disable_irq;
>   			engine->dispatch_execbuffer =
>   				gen8_ring_dispatch_execbuffer;
>   			if (i915_semaphore_is_enabled(dev_priv)) {
> @@ -2868,8 +2771,8 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
>   			}
>   		} else {
>   			engine->irq_enable_mask = GT_BSD_USER_INTERRUPT;
> -			engine->irq_get = gen6_ring_get_irq;
> -			engine->irq_put = gen6_ring_put_irq;
> +			engine->irq_enable = gen6_ring_enable_irq;
> +			engine->irq_disable = gen6_ring_disable_irq;
>   			engine->dispatch_execbuffer =
>   				gen6_ring_dispatch_execbuffer;
>   			if (i915_semaphore_is_enabled(dev_priv)) {
> @@ -2893,13 +2796,13 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
>   		engine->add_request = i9xx_add_request;
>   		if (IS_GEN5(dev_priv)) {
>   			engine->irq_enable_mask = ILK_BSD_USER_INTERRUPT;
> -			engine->irq_get = gen5_ring_get_irq;
> -			engine->irq_put = gen5_ring_put_irq;
> +			engine->irq_enable = gen5_ring_enable_irq;
> +			engine->irq_disable = gen5_ring_disable_irq;
>   			engine->irq_seqno_barrier = gen5_seqno_barrier;
>   		} else {
>   			engine->irq_enable_mask = I915_BSD_USER_INTERRUPT;
> -			engine->irq_get = i9xx_ring_get_irq;
> -			engine->irq_put = i9xx_ring_put_irq;
> +			engine->irq_enable = i9xx_ring_enable_irq;
> +			engine->irq_disable = i9xx_ring_disable_irq;
>   		}
>   		engine->dispatch_execbuffer = i965_dispatch_execbuffer;
>   	}
> @@ -2928,8 +2831,8 @@ int intel_init_bsd2_ring_buffer(struct drm_device *dev)
>   	engine->irq_seqno_barrier = gen6_seqno_barrier;
>   	engine->irq_enable_mask =
>   			GT_RENDER_USER_INTERRUPT << GEN8_VCS2_IRQ_SHIFT;
> -	engine->irq_get = gen8_ring_get_irq;
> -	engine->irq_put = gen8_ring_put_irq;
> +	engine->irq_enable = gen8_ring_enable_irq;
> +	engine->irq_disable = gen8_ring_disable_irq;
>   	engine->dispatch_execbuffer =
>   			gen8_ring_dispatch_execbuffer;
>   	if (i915_semaphore_is_enabled(dev_priv)) {
> @@ -2960,8 +2863,8 @@ int intel_init_blt_ring_buffer(struct drm_device *dev)
>   	if (INTEL_GEN(dev_priv) >= 8) {
>   		engine->irq_enable_mask =
>   			GT_RENDER_USER_INTERRUPT << GEN8_BCS_IRQ_SHIFT;
> -		engine->irq_get = gen8_ring_get_irq;
> -		engine->irq_put = gen8_ring_put_irq;
> +		engine->irq_enable = gen8_ring_enable_irq;
> +		engine->irq_disable = gen8_ring_disable_irq;
>   		engine->dispatch_execbuffer = gen8_ring_dispatch_execbuffer;
>   		if (i915_semaphore_is_enabled(dev_priv)) {
>   			engine->semaphore.sync_to = gen8_ring_sync;
> @@ -2970,8 +2873,8 @@ int intel_init_blt_ring_buffer(struct drm_device *dev)
>   		}
>   	} else {
>   		engine->irq_enable_mask = GT_BLT_USER_INTERRUPT;
> -		engine->irq_get = gen6_ring_get_irq;
> -		engine->irq_put = gen6_ring_put_irq;
> +		engine->irq_enable = gen6_ring_enable_irq;
> +		engine->irq_disable = gen6_ring_disable_irq;
>   		engine->dispatch_execbuffer = gen6_ring_dispatch_execbuffer;
>   		if (i915_semaphore_is_enabled(dev_priv)) {
>   			engine->semaphore.signal = gen6_signal;
> @@ -3019,8 +2922,8 @@ int intel_init_vebox_ring_buffer(struct drm_device *dev)
>   	if (INTEL_GEN(dev_priv) >= 8) {
>   		engine->irq_enable_mask =
>   			GT_RENDER_USER_INTERRUPT << GEN8_VECS_IRQ_SHIFT;
> -		engine->irq_get = gen8_ring_get_irq;
> -		engine->irq_put = gen8_ring_put_irq;
> +		engine->irq_enable = gen8_ring_enable_irq;
> +		engine->irq_disable = gen8_ring_disable_irq;
>   		engine->dispatch_execbuffer = gen8_ring_dispatch_execbuffer;
>   		if (i915_semaphore_is_enabled(dev_priv)) {
>   			engine->semaphore.sync_to = gen8_ring_sync;
> @@ -3029,8 +2932,8 @@ int intel_init_vebox_ring_buffer(struct drm_device *dev)
>   		}
>   	} else {
>   		engine->irq_enable_mask = PM_VEBOX_USER_INTERRUPT;
> -		engine->irq_get = hsw_vebox_get_irq;
> -		engine->irq_put = hsw_vebox_put_irq;
> +		engine->irq_enable = hsw_vebox_enable_irq;
> +		engine->irq_disable = hsw_vebox_disable_irq;
>   		engine->dispatch_execbuffer = gen6_ring_dispatch_execbuffer;
>   		if (i915_semaphore_is_enabled(dev_priv)) {
>   			engine->semaphore.sync_to = gen6_ring_sync;
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index 5f7cb3d0ea1c..182cae767bf1 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -201,11 +201,10 @@ struct intel_engine_cs {
>   	struct intel_hw_status_page status_page;
>   	struct i915_ctx_workarounds wa_ctx;
>
> -	unsigned irq_refcount; /* protected by dev_priv->irq_lock */
>   	bool		irq_posted;
>   	u32		irq_enable_mask;	/* bitmask to enable ring interrupt */
> -	bool __must_check (*irq_get)(struct intel_engine_cs *ring);
> -	void		(*irq_put)(struct intel_engine_cs *ring);
> +	void		(*irq_enable)(struct intel_engine_cs *ring);
> +	void		(*irq_disable)(struct intel_engine_cs *ring);
>
>   	int		(*init_hw)(struct intel_engine_cs *ring);
>
>

Some more instances of things I've already asked about.

Apart from those opens looks OK.

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2016-06-07 12:46 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-03 16:08 Breadcrumbs, again Chris Wilson
2016-06-03 16:08 ` [PATCH 01/21] drm/i915/shrinker: Flush active on objects before counting Chris Wilson
2016-06-03 16:08 ` [PATCH 02/21] drm/i915: Delay queuing hangcheck to wait-request Chris Wilson
2016-06-08  8:42   ` Daniel Vetter
2016-06-08  9:13     ` Chris Wilson
2016-06-03 16:08 ` [PATCH 03/21] drm/i915: Remove the dedicated hangcheck workqueue Chris Wilson
2016-06-06 12:52   ` Tvrtko Ursulin
2016-06-03 16:08 ` [PATCH 04/21] drm/i915: Make queueing the hangcheck work inline Chris Wilson
2016-06-03 16:08 ` [PATCH 05/21] drm/i915: Separate GPU hang waitqueue from advance Chris Wilson
2016-06-06 13:00   ` Tvrtko Ursulin
2016-06-07 12:11     ` Arun Siluvery
2016-06-03 16:08 ` [PATCH 06/21] drm/i915: Slaughter the thundering i915_wait_request herd Chris Wilson
2016-06-06 13:58   ` Tvrtko Ursulin
2016-06-03 16:08 ` [PATCH 07/21] drm/i915: Spin after waking up for an interrupt Chris Wilson
2016-06-06 14:39   ` Tvrtko Ursulin
2016-06-03 16:08 ` [PATCH 08/21] drm/i915: Use HWS for seqno tracking everywhere Chris Wilson
2016-06-06 14:55   ` Tvrtko Ursulin
2016-06-08  9:24     ` Chris Wilson
2016-06-03 16:08 ` [PATCH 09/21] drm/i915: Stop mapping the scratch page into CPU space Chris Wilson
2016-06-06 15:03   ` Tvrtko Ursulin
2016-06-03 16:08 ` [PATCH 10/21] drm/i915: Allocate scratch page from stolen Chris Wilson
2016-06-06 15:05   ` Tvrtko Ursulin
2016-06-03 16:08 ` [PATCH 11/21] drm/i915: Refactor scratch object allocation for gen2 w/a buffer Chris Wilson
2016-06-06 15:09   ` Tvrtko Ursulin
2016-06-08  9:27     ` Chris Wilson
2016-06-03 16:08 ` [PATCH 12/21] drm/i915: Add a delay between interrupt and inspecting the final seqno (ilk) Chris Wilson
2016-06-03 16:08 ` [PATCH 13/21] drm/i915: Check the CPU cached value of seqno after waking the waiter Chris Wilson
2016-06-06 15:10   ` Tvrtko Ursulin
2016-06-03 16:08 ` [PATCH 14/21] drm/i915: Only apply one barrier after a breadcrumb interrupt is posted Chris Wilson
2016-06-06 15:34   ` Tvrtko Ursulin
2016-06-08  9:35     ` Chris Wilson
2016-06-08  9:57       ` Tvrtko Ursulin
2016-06-03 16:08 ` [PATCH 15/21] drm/i915: Stop setting wraparound seqno on initialisation Chris Wilson
2016-06-08  8:54   ` Daniel Vetter
2016-06-03 16:08 ` [PATCH 16/21] drm/i915: Only query timestamp when measuring elapsed time Chris Wilson
2016-06-06 13:50   ` Tvrtko Ursulin
2016-06-03 16:08 ` [PATCH 17/21] drm/i915: Convert trace-irq to the breadcrumb waiter Chris Wilson
2016-06-07 12:04   ` Tvrtko Ursulin
2016-06-08  9:48     ` Chris Wilson
2016-06-08 10:16       ` Tvrtko Ursulin
2016-06-08 11:24         ` Chris Wilson
2016-06-08 11:47           ` Tvrtko Ursulin
2016-06-08 12:34             ` Chris Wilson
2016-06-08 12:44               ` Tvrtko Ursulin
2016-06-08 13:47                 ` Chris Wilson
2016-06-03 16:08 ` [PATCH 18/21] drm/i915: Embed signaling node into the GEM request Chris Wilson
2016-06-07 12:31   ` Tvrtko Ursulin
2016-06-08  9:54     ` Chris Wilson
2016-06-03 16:08 ` [PATCH 19/21] drm/i915: Move the get/put irq locking into the caller Chris Wilson
2016-06-07 12:46   ` Tvrtko Ursulin [this message]
2016-06-08 10:01     ` Chris Wilson
2016-06-08 10:18       ` Tvrtko Ursulin
2016-06-08 11:10         ` Chris Wilson
2016-06-08 11:49           ` Tvrtko Ursulin
2016-06-08 12:54             ` Chris Wilson
2016-06-03 16:08 ` [PATCH 20/21] drm/i915: Simplify enabling user-interrupts with L3-remapping Chris Wilson
2016-06-07 12:50   ` Tvrtko Ursulin
2016-06-03 16:08 ` [PATCH 21/21] drm/i915: Remove debug noise on detecting fault-injection of missed interrupts Chris Wilson
2016-06-07 12:51   ` Tvrtko Ursulin
2016-06-03 16:35 ` ✗ Ro.CI.BAT: failure for series starting with [01/21] drm/i915/shrinker: Flush active on objects before counting Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5756C23D.5090508@linux.intel.com \
    --to=tvrtko.ursulin@linux.intel.com \
    --cc=chris@chris-wilson.co.uk \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).