[Intel-gfx] [PATCH 3/6] drm/i915: Avoid invariant conditionals in lrc interrupt handler

[Chris Wilson]

On Thu, Jan 07, 2016 at 04:36:18PM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> 
> There is no need to check on what Gen we are running on every
> interrupt and every command submission. We can instead set up
> some of that when engines are initialized, store it in the
> engine structure and use it directly at runtime.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_lrc.c        | 36 ++++++++++++++++++---------------
>  drivers/gpu/drm/i915/intel_ringbuffer.h |  2 ++
>  2 files changed, 22 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 8b6071fcd743..84977a6e6f3f 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -298,29 +298,15 @@ uint64_t intel_lr_context_descriptor(struct intel_context *ctx,
>  				     struct intel_engine_cs *ring)
>  {
>  	struct drm_i915_gem_object *ctx_obj = ctx->engine[ring->id].state;
> -	uint64_t desc;
> +	uint64_t desc = ring->ctx_desc_template;
>  	uint64_t lrca = i915_gem_obj_ggtt_offset(ctx_obj) +
>  			LRC_PPHWSP_PN * PAGE_SIZE;
>  
>  	WARN_ON(lrca & 0xFFFFFFFF00000FFFULL);
>  
> -	desc = GEN8_CTX_VALID;
> -	desc |= GEN8_CTX_ADDRESSING_MODE(dev) << GEN8_CTX_ADDRESSING_MODE_SHIFT;
> -	if (IS_GEN8(ctx_obj->base.dev))
> -		desc |= GEN8_CTX_L3LLC_COHERENT;
> -	desc |= GEN8_CTX_PRIVILEGE;
>  	desc |= lrca;
>  	desc |= (u64)intel_execlists_ctx_id(ctx_obj) << GEN8_CTX_ID_SHIFT;
>  
> -	/* TODO: WaDisableLiteRestore when we start using semaphore
> -	 * signalling between Command Streamers */
> -	/* desc |= GEN8_CTX_FORCE_RESTORE; */
> -
> -	/* WaEnableForceRestoreInCtxtDescForVCS:skl */
> -	/* WaEnableForceRestoreInCtxtDescForVCS:bxt */
> -	if (disable_lite_restore_wa(ring))
> -		desc |= GEN8_CTX_FORCE_RESTORE;
> -
>  	return desc;
>  }
>  
> @@ -556,7 +542,7 @@ void intel_lrc_irq_handler(struct intel_engine_cs *ring)
>  		}
>  	}
>  
> -	if (disable_lite_restore_wa(ring)) {
> +	if (ring->disable_lite_restore_wa) {
>  		/* Prevent a ctx to preempt itself */
>  		if ((status & GEN8_CTX_STATUS_ACTIVE_IDLE) &&
>  		    (submit_contexts != 0))

Didn't it occur to you that this code is garbage?

> @@ -1980,6 +1966,24 @@ static int logical_ring_init(struct drm_device *dev, struct intel_engine_cs *rin
>  		goto error;
>  	}
>  
> +	ring->disable_lite_restore_wa = disable_lite_restore_wa(ring);
> +
> +	ring->ctx_desc_template = GEN8_CTX_VALID;
> +	ring->ctx_desc_template |= GEN8_CTX_ADDRESSING_MODE(dev) <<
> +				   GEN8_CTX_ADDRESSING_MODE_SHIFT;
> +	if (IS_GEN8(dev))
> +		ring->ctx_desc_template |= GEN8_CTX_L3LLC_COHERENT;
> +	ring->ctx_desc_template |= GEN8_CTX_PRIVILEGE;
> +
> +	/* TODO: WaDisableLiteRestore when we start using semaphore
> +	 * signalling between Command Streamers */
> +	/* ring->ctx_desc_template |= GEN8_CTX_FORCE_RESTORE; */
> +
> +	/* WaEnableForceRestoreInCtxtDescForVCS:skl */
> +	/* WaEnableForceRestoreInCtxtDescForVCS:bxt */
> +	if (ring->disable_lite_restore_wa)
> +		ring->ctx_desc_template |= GEN8_CTX_FORCE_RESTORE;
> +
>  	return 0;
>  
>  error:
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index 49574ffe54bc..0b91a4b77359 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -268,6 +268,8 @@ struct  intel_engine_cs {
>  	struct list_head execlist_queue;
>  	struct list_head execlist_retired_req_list;
>  	u8 next_context_status_buffer;
> +	bool disable_lite_restore_wa;

You don't need that as a separate flag. You know I sent this patch last
year?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre