Re: [PATCH 03/13] drm/i915: Avoid invariant conditionals in lrc interrupt handler

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 11/01/16 08:29, Daniel Vetter wrote:
> On Fri, Jan 08, 2016 at 11:29:42AM +0000, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@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@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;
>>   }
>
> tbh I'd go full monty and just cache the entire context descriptor.

Yeah you're right - this was more a consequence of me learning about 
this code as making small cleanups.

Regards,

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