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

Tvrtko Ursulin
2016-01-11T09:43:15Z

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