Message ID | 1452252592-24803-4-git-send-email-tvrtko.ursulin@linux.intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
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. -Daniel > > @@ -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)) > @@ -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; > + 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, > -- > 1.9.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
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
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)) @@ -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; + 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,