Re: [PATCH 20/21] drm/i915: Simplify enabling user-interrupts with L3-remapping

From: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 20/21] drm/i915: Simplify enabling user-interrupts with L3-remapping
Date: Tue, 7 Jun 2016 13:50:42 +0100	[thread overview]
Message-ID: <5756C322.2040606@linux.intel.com> (raw)
In-Reply-To: <1464970133-29859-21-git-send-email-chris@chris-wilson.co.uk>


On 03/06/16 17:08, Chris Wilson wrote:
> Borrow the idea from intel_lrc.c to precompute the mask of interrupts we
> wish to always enable to avoid having lots of conditionals inside the
> interrupt enabling.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/intel_ringbuffer.c | 35 +++++++++++----------------------
>   drivers/gpu/drm/i915/intel_ringbuffer.h |  4 ++--
>   2 files changed, 14 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index ba84b469f13f..161c0792b1bf 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -1227,8 +1227,7 @@ static int init_render_ring(struct intel_engine_cs *engine)
>   	if (IS_GEN(dev_priv, 6, 7))
>   		I915_WRITE(INSTPM, _MASKED_BIT_ENABLE(INSTPM_FORCE_ORDERING));
>
> -	if (HAS_L3_DPF(dev_priv))
> -		I915_WRITE_IMR(engine, ~GT_PARITY_ERROR(dev_priv));
> +	I915_WRITE_IMR(engine, ~engine->irq_keep_mask);
>
>   	return init_workarounds_ring(engine);
>   }
> @@ -1644,12 +1643,9 @@ gen6_ring_enable_irq(struct intel_engine_cs *engine)
>   {
>   	struct drm_i915_private *dev_priv = engine->i915;
>
> -	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);
> +	I915_WRITE_IMR(engine,
> +		       ~(engine->irq_enable_mask |
> +			 engine->irq_keep_mask));
>   	gen5_enable_gt_irq(dev_priv, engine->irq_enable_mask);
>   }
>
> @@ -1658,10 +1654,7 @@ gen6_ring_disable_irq(struct intel_engine_cs *engine)
>   {
>   	struct drm_i915_private *dev_priv = engine->i915;
>
> -	if (HAS_L3_DPF(dev_priv) && engine->id == RCS)
> -		I915_WRITE_IMR(engine, ~GT_PARITY_ERROR(dev_priv));
> -	else
> -		I915_WRITE_IMR(engine, ~0);
> +	I915_WRITE_IMR(engine, ~engine->irq_keep_mask);
>   	gen5_disable_gt_irq(dev_priv, engine->irq_enable_mask);
>   }
>
> @@ -1688,12 +1681,9 @@ gen8_ring_enable_irq(struct intel_engine_cs *engine)
>   {
>   	struct drm_i915_private *dev_priv = engine->i915;
>
> -	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);
> +	I915_WRITE_IMR(engine,
> +		       ~(engine->irq_enable_mask |
> +			 engine->irq_keep_mask));
>   	POSTING_READ_FW(RING_IMR(engine->mmio_base));
>   }
>
> @@ -1702,11 +1692,7 @@ gen8_ring_disable_irq(struct intel_engine_cs *engine)
>   {
>   	struct drm_i915_private *dev_priv = engine->i915;
>
> -	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);
> +	I915_WRITE_IMR(engine, ~engine->irq_keep_mask);
>   }
>
>   static int
> @@ -2621,6 +2607,9 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
>   	engine->hw_id = 0;
>   	engine->mmio_base = RENDER_RING_BASE;
>
> +	if (HAS_L3_DPF(dev_priv))
> +		engine->irq_keep_mask = GT_RENDER_L3_PARITY_ERROR_INTERRUPT;
> +
>   	if (INTEL_GEN(dev_priv) >= 8) {
>   		if (i915_semaphore_is_enabled(dev_priv)) {
>   			obj = i915_gem_object_create(dev, 4096);
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index 182cae767bf1..166f1a3829b0 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -202,7 +202,8 @@ struct intel_engine_cs {
>   	struct i915_ctx_workarounds wa_ctx;
>
>   	bool		irq_posted;
> -	u32		irq_enable_mask;	/* bitmask to enable ring interrupt */
> +	u32             irq_keep_mask; /* bitmask for interrupts that should not be masked */
> +	u32		irq_enable_mask;/* bitmask to enable ring interrupt */
>   	void		(*irq_enable)(struct intel_engine_cs *ring);
>   	void		(*irq_disable)(struct intel_engine_cs *ring);
>
> @@ -299,7 +300,6 @@ struct intel_engine_cs {
>   	unsigned int idle_lite_restore_wa;
>   	bool disable_lite_restore_wa;
>   	u32 ctx_desc_template;
> -	u32             irq_keep_mask; /* bitmask for interrupts that should not be masked */
>   	int		(*emit_request)(struct drm_i915_gem_request *request);
>   	int		(*emit_flush)(struct drm_i915_gem_request *request,
>   				      u32 invalidate_domains,
>

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko

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