Quoting Tvrtko Ursulin (2019-03-05 16:16:01)
>
> 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@xxxxxxxxxxxxxxxxxx>
> > ---
> > @@ -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.
I must have slipped :-p
> > @@ -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.
We could even initialise ce->ring, or just pull that
ce->gem_context->ring_size.
I didn't have a good reason to keep engine, nor a good enough reason to
kill it -- although having ce is enough of a change I guess.
> > 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.
My plan is to switch to ce->ppgtt at some point in the near future.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
