[Intel-gfx] [PATCH 26/38] drm/i915: Pass around the intel_context
Tvrtko Ursulin
tvrtko.ursulin at linux.intel.com
Tue Mar 5 16:16:01 UTC 2019
On 01/03/2019 14:03, Chris Wilson wrote:
> Instead of passing the gem_context and engine to find the instance of
> the intel_context to use, pass around the intel_context instead. This is
> useful for the next few patches, where the intel_context is no longer a
> direct lookup.
>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 2 +-
> drivers/gpu/drm/i915/i915_perf.c | 32 ++++++++++++++--------------
> drivers/gpu/drm/i915/intel_lrc.c | 36 +++++++++++++++++---------------
> 3 files changed, 36 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index cbc2b59722ae..493cba8a76aa 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3134,7 +3134,7 @@ int i915_perf_add_config_ioctl(struct drm_device *dev, void *data,
> int i915_perf_remove_config_ioctl(struct drm_device *dev, void *data,
> struct drm_file *file);
> void i915_oa_init_reg_state(struct intel_engine_cs *engine,
> - struct i915_gem_context *ctx,
> + struct intel_context *ce,
> u32 *reg_state);
>
> /* i915_gem_evict.c */
> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
> index 9ebf99f3d8d3..f969a0512465 100644
> --- a/drivers/gpu/drm/i915/i915_perf.c
> +++ b/drivers/gpu/drm/i915/i915_perf.c
> @@ -1629,13 +1629,14 @@ static void hsw_disable_metric_set(struct drm_i915_private *dev_priv)
> * It's fine to put out-of-date values into these per-context registers
> * in the case that the OA unit has been disabled.
> */
> -static void gen8_update_reg_state_unlocked(struct i915_gem_context *ctx,
> - u32 *reg_state,
> - const struct i915_oa_config *oa_config)
> +static void
> +gen8_update_reg_state_unlocked(struct intel_context *ce,
> + u32 *reg_state,
> + const struct i915_oa_config *oa_config)
> {
> - struct drm_i915_private *dev_priv = ctx->i915;
> - u32 ctx_oactxctrl = dev_priv->perf.oa.ctx_oactxctrl_offset;
> - u32 ctx_flexeu0 = dev_priv->perf.oa.ctx_flexeu0_offset;
> + struct drm_i915_private *i915 = ce->gem_context->i915;
> + u32 ctx_oactxctrl = i915->perf.oa.ctx_oactxctrl_offset;
> + u32 ctx_flexeu0 = i915->perf.oa.ctx_flexeu0_offset;
> /* The MMIO offsets for Flex EU registers aren't contiguous */
> i915_reg_t flex_regs[] = {
> EU_PERF_CNTL0,
> @@ -1649,8 +1650,8 @@ static void gen8_update_reg_state_unlocked(struct i915_gem_context *ctx,
> int i;
>
> CTX_REG(reg_state, ctx_oactxctrl, GEN8_OACTXCONTROL,
> - (dev_priv->perf.oa.period_exponent << GEN8_OA_TIMER_PERIOD_SHIFT) |
> - (dev_priv->perf.oa.periodic ? GEN8_OA_TIMER_ENABLE : 0) |
> + (i915->perf.oa.period_exponent << GEN8_OA_TIMER_PERIOD_SHIFT) |
> + (i915->perf.oa.periodic ? GEN8_OA_TIMER_ENABLE : 0) |
> GEN8_OA_COUNTER_RESUME);
>
> for (i = 0; i < ARRAY_SIZE(flex_regs); i++) {
> @@ -1678,10 +1679,9 @@ static void gen8_update_reg_state_unlocked(struct i915_gem_context *ctx,
> CTX_REG(reg_state, state_offset, flex_regs[i], value);
> }
>
> - CTX_REG(reg_state, CTX_R_PWR_CLK_STATE, GEN8_R_PWR_CLK_STATE,
> - gen8_make_rpcs(dev_priv,
> - &to_intel_context(ctx,
> - dev_priv->engine[RCS])->sseu));
> + CTX_REG(reg_state,
> + CTX_R_PWR_CLK_STATE, GEN8_R_PWR_CLK_STATE,
> + gen8_make_rpcs(i915, &ce->sseu));
> }
>
> /*
> @@ -1754,7 +1754,7 @@ static int gen8_configure_all_contexts(struct drm_i915_private *dev_priv,
> ce->state->obj->mm.dirty = true;
> regs += LRC_STATE_PN * PAGE_SIZE / sizeof(*regs);
>
> - gen8_update_reg_state_unlocked(ctx, regs, oa_config);
> + gen8_update_reg_state_unlocked(ce, regs, oa_config);
>
> i915_gem_object_unpin_map(ce->state->obj);
> }
> @@ -2138,8 +2138,8 @@ static int i915_oa_stream_init(struct i915_perf_stream *stream,
> }
>
> void i915_oa_init_reg_state(struct intel_engine_cs *engine,
> - struct i915_gem_context *ctx,
> - u32 *reg_state)
> + struct intel_context *ce,
> + u32 *regs)
> {
> struct i915_perf_stream *stream;
>
> @@ -2148,7 +2148,7 @@ void i915_oa_init_reg_state(struct intel_engine_cs *engine,
>
> stream = engine->i915->perf.oa.exclusive_stream;
> if (stream)
> - gen8_update_reg_state_unlocked(ctx, reg_state, stream->oa_config);
> + gen8_update_reg_state_unlocked(ce, regs, stream->oa_config);
> }
>
> /**
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index d50a33c578c5..a2210f79dc67 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -170,7 +170,7 @@ static int execlists_context_deferred_alloc(struct i915_gem_context *ctx,
> struct intel_engine_cs *engine,
> struct intel_context *ce);
> static void execlists_init_reg_state(u32 *reg_state,
> - struct i915_gem_context *ctx,
> + struct intel_context *ce,
> struct intel_engine_cs *engine,
> struct intel_ring *ring);
>
> @@ -1314,9 +1314,10 @@ __execlists_update_reg_state(struct intel_engine_cs *engine,
> regs[CTX_RING_TAIL + 1] = ring->tail;
>
> /* RPCS */
> - if (engine->class == RENDER_CLASS)
> - regs[CTX_R_PWR_CLK_STATE + 1] = gen8_make_rpcs(engine->i915,
> - &ce->sseu);
> + if (engine->class == RENDER_CLASS) {
> + regs[CTX_R_PWR_CLK_STATE + 1] =
> + gen8_make_rpcs(engine->i915, &ce->sseu);
> + }
Rebasing artifact I guess.
> }
>
> static struct intel_context *
> @@ -2021,7 +2022,7 @@ static void execlists_reset(struct intel_engine_cs *engine, bool stalled)
> rq->ring->head = intel_ring_wrap(rq->ring, rq->head);
> intel_ring_update_space(rq->ring);
>
> - execlists_init_reg_state(regs, rq->gem_context, engine, rq->ring);
> + execlists_init_reg_state(regs, rq->hw_context, engine, rq->ring);
> __execlists_update_reg_state(engine, rq->hw_context);
>
> out_unlock:
> @@ -2659,7 +2660,7 @@ static u32 intel_lr_indirect_ctx_offset(struct intel_engine_cs *engine)
> }
>
> static void execlists_init_reg_state(u32 *regs,
> - struct i915_gem_context *ctx,
> + struct intel_context *ce,
> struct intel_engine_cs *engine,
I can't remember if separate engine param is needed since now there's
ce->engine. Maybe it comes useful later.
> struct intel_ring *ring)
> {
> @@ -2735,24 +2736,24 @@ static void execlists_init_reg_state(u32 *regs,
> CTX_REG(regs, CTX_PDP0_UDW, GEN8_RING_PDP_UDW(engine, 0), 0);
> CTX_REG(regs, CTX_PDP0_LDW, GEN8_RING_PDP_LDW(engine, 0), 0);
>
> - if (i915_vm_is_48bit(&ctx->ppgtt->vm)) {
> + if (i915_vm_is_48bit(&ce->gem_context->ppgtt->vm)) {
> /* 64b PPGTT (48bit canonical)
> * PDP0_DESCRIPTOR contains the base address to PML4 and
> * other PDP Descriptors are ignored.
> */
> - ASSIGN_CTX_PML4(ctx->ppgtt, regs);
> + ASSIGN_CTX_PML4(ce->gem_context->ppgtt, regs);
> } else {
> - ASSIGN_CTX_PDP(ctx->ppgtt, regs, 3);
> - ASSIGN_CTX_PDP(ctx->ppgtt, regs, 2);
> - ASSIGN_CTX_PDP(ctx->ppgtt, regs, 1);
> - ASSIGN_CTX_PDP(ctx->ppgtt, regs, 0);
> + ASSIGN_CTX_PDP(ce->gem_context->ppgtt, regs, 3);
> + ASSIGN_CTX_PDP(ce->gem_context->ppgtt, regs, 2);
> + ASSIGN_CTX_PDP(ce->gem_context->ppgtt, regs, 1);
> + ASSIGN_CTX_PDP(ce->gem_context->ppgtt, regs, 0);
Could be worth caching the ppgtt for aesthetics.
> }
>
> if (rcs) {
> regs[CTX_LRI_HEADER_2] = MI_LOAD_REGISTER_IMM(1);
> CTX_REG(regs, CTX_R_PWR_CLK_STATE, GEN8_R_PWR_CLK_STATE, 0);
>
> - i915_oa_init_reg_state(engine, ctx, regs);
> + i915_oa_init_reg_state(engine, ce, regs);
> }
>
> regs[CTX_END] = MI_BATCH_BUFFER_END;
> @@ -2761,7 +2762,7 @@ static void execlists_init_reg_state(u32 *regs,
> }
>
> static int
> -populate_lr_context(struct i915_gem_context *ctx,
> +populate_lr_context(struct intel_context *ce,
> struct drm_i915_gem_object *ctx_obj,
> struct intel_engine_cs *engine,
> struct intel_ring *ring)
> @@ -2807,11 +2808,12 @@ populate_lr_context(struct i915_gem_context *ctx,
> /* The second page of the context object contains some fields which must
> * be set up prior to the first execution. */
> regs = vaddr + LRC_STATE_PN * PAGE_SIZE;
> - execlists_init_reg_state(regs, ctx, engine, ring);
> + execlists_init_reg_state(regs, ce, engine, ring);
> if (!engine->default_state)
> regs[CTX_CONTEXT_CONTROL + 1] |=
> _MASKED_BIT_ENABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT);
> - if (ctx == ctx->i915->preempt_context && INTEL_GEN(engine->i915) < 11)
> + if (ce->gem_context == engine->i915->preempt_context &&
> + INTEL_GEN(engine->i915) < 11)
> regs[CTX_CONTEXT_CONTROL + 1] |=
> _MASKED_BIT_ENABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT |
> CTX_CTRL_ENGINE_CTX_SAVE_INHIBIT);
> @@ -2869,7 +2871,7 @@ static int execlists_context_deferred_alloc(struct i915_gem_context *ctx,
> goto error_deref_obj;
> }
>
> - ret = populate_lr_context(ctx, ctx_obj, engine, ring);
> + ret = populate_lr_context(ce, ctx_obj, engine, ring);
> if (ret) {
> DRM_DEBUG_DRIVER("Failed to populate LRC: %d\n", ret);
> goto error_ring_free;
>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
Regards,
Tvrtko
More information about the Intel-gfx
mailing list