Re: [PATCH 08/17] drm/i915: Remove lrc default desc from GEM context

Kumar Valsan, Prathap
2019-07-30T22:41:44Z

On Tue, Jul 30, 2019 at 02:30:26PM +0100, Chris Wilson wrote:
> We only compute the lrc_descriptor() on pinning the context, i.e.
> infrequently, so we do not benefit from storing the template as the
> addressing mode is also fixed for the lifetime of the intel_context.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Prathap Kumar Valsan <prathap.kumar.valsan@intel.com>
> ---
> drivers/gpu/drm/i915/gem/i915_gem_context.c | 28 ++-----------------
> .../gpu/drm/i915/gem/i915_gem_context_types.h | 2 --
> drivers/gpu/drm/i915/gt/intel_lrc.c | 12 +++++---
> drivers/gpu/drm/i915/gvt/scheduler.c | 3 --
> 4 files changed, 10 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> index b28c7ca681a8..1b3dc7258ef2 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> @@ -397,30 +397,6 @@ static void context_close(struct i915_gem_context *ctx)
> i915_gem_context_put(ctx);
> }
>
> -static u32 default_desc_template(const struct drm_i915_private *i915,
> - const struct i915_address_space *vm)
> -{
> - u32 address_mode;
> - u32 desc;
> -
> - desc = GEN8_CTX_VALID | GEN8_CTX_PRIVILEGE;
> -
> - address_mode = INTEL_LEGACY_32B_CONTEXT;
> - if (vm && i915_vm_is_4lvl(vm))
> - address_mode = INTEL_LEGACY_64B_CONTEXT;
> - desc |= address_mode << GEN8_CTX_ADDRESSING_MODE_SHIFT;
> -
> - if (IS_GEN(i915, 8))
> - desc |= GEN8_CTX_L3LLC_COHERENT;
> -
> - /* TODO: WaDisableLiteRestore when we start using semaphore
> - * signalling between Command Streamers
> - * ring->ctx_desc_template |= GEN8_CTX_FORCE_RESTORE;
> - */
> -
> - return desc;
> -}
> -
> static struct i915_gem_context *
> __create_context(struct drm_i915_private *i915)
> {
> @@ -459,7 +435,6 @@ __create_context(struct drm_i915_private *i915)
> i915_gem_context_set_recoverable(ctx);
>
> ctx->ring_size = 4 * PAGE_SIZE;
> - ctx->desc_template = default_desc_template(i915, NULL);
>
> for (i = 0; i < ARRAY_SIZE(ctx->hang_timestamp); i++)
> ctx->hang_timestamp[i] = jiffies - CONTEXT_FAST_HANG_JIFFIES;
> @@ -478,8 +453,9 @@ __set_ppgtt(struct i915_gem_context *ctx, struct i915_address_space *vm)
> struct i915_gem_engines_iter it;
> struct intel_context *ce;
>
> + GEM_BUG_ON(old && i915_vm_is_4lvl(vm) != i915_vm_is_4lvl(old));
> +
> ctx->vm = i915_vm_get(vm);
> - ctx->desc_template = default_desc_template(ctx->i915, vm);
>
> for_each_gem_engine(ce, i915_gem_context_lock_engines(ctx), it) {
> i915_vm_put(ce->vm);
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
> index 0ee61482ef94..a02d98494078 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
> @@ -171,8 +171,6 @@ struct i915_gem_context {
>
> /** ring_size: size for allocating the per-engine ring buffer */
> u32 ring_size;
> - /** desc_template: invariant fields for the HW context descriptor */
> - u32 desc_template;
>
> /** guilty_count: How many times this context has caused a GPU hang. */
> atomic_t guilty_count;
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index 5181d29d272e..232f40fcb490 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -417,13 +417,17 @@ lrc_descriptor(struct intel_context *ce, struct intel_engine_cs *engine)
> BUILD_BUG_ON(MAX_CONTEXT_HW_ID > (BIT(GEN8_CTX_ID_WIDTH)));
> BUILD_BUG_ON(GEN11_MAX_CONTEXT_HW_ID > (BIT(GEN11_SW_CTX_ID_WIDTH)));
>
> - desc = ctx->desc_template; /* bits 0-11 */
> - GEM_BUG_ON(desc & GENMASK_ULL(63, 12));
> + desc = INTEL_LEGACY_32B_CONTEXT;
> + if (i915_vm_is_4lvl(ce->vm))
> + desc = INTEL_LEGACY_64B_CONTEXT;
> + desc <<= GEN8_CTX_ADDRESSING_MODE_SHIFT;
> +
> + desc |= GEN8_CTX_VALID | GEN8_CTX_PRIVILEGE;
> + if (IS_GEN(engine->i915, 8))
> + desc |= GEN8_CTX_L3LLC_COHERENT;
>
> desc |= i915_ggtt_offset(ce->state) + LRC_HEADER_PAGES * PAGE_SIZE;
> /* bits 12-31 */
> - GEM_BUG_ON(desc & GENMASK_ULL(63, 32));
> -
> /*
> * The following 32bits are copied into the OA reports (dword 2).
> * Consider updating oa_get_render_ctx_id in i915_perf.c when changing
> diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c
> index f40524b0e300..32ae6b5b7e16 100644
> --- a/drivers/gpu/drm/i915/gvt/scheduler.c
> +++ b/drivers/gpu/drm/i915/gvt/scheduler.c
> @@ -291,9 +291,6 @@ shadow_context_descriptor_update(struct intel_context *ce,
> * Update bits 0-11 of the context descriptor which includes flags
> * like GEN8_CTX_* cached in desc_template
> */
> - desc &= U64_MAX << 12;
> - desc |= ce->gem_context->desc_template & ((1ULL << 12) - 1);
> -
> desc &= ~(0x3 << GEN8_CTX_ADDRESSING_MODE_SHIFT);
> desc |= workload->ctx_desc.addressing_mode <<
> GEN8_CTX_ADDRESSING_MODE_SHIFT;
> --
> 2.22.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx