[Intel-gfx] [PATCH 26/38] drm/i915: Pass around the intel_context

[Chris Wilson]

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 at chris-wilson.co.uk>
> > ---
> > @@ -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