[Intel-gfx] [PATCH v2] drm/i915/execlists: Inhibit context save/restore for the fake preempt context

[Chris Wilson]

Quoting Michel Thierry (2018-01-24 01:24:25)
> On 1/23/2018 1:04 PM, Chris Wilson wrote:
> > We only use the preempt context to inject an idle point into execlists.
> > We never need to reference its logical state, so tell the GPU never to
> > load it or save it.
> > 
> > v2: BIT(2) for save-inhibit.
> > 
> > N.B. Daniele mentioned this bit mbz for ICL, and has been moved into the
> > submission process rather than the context image.
> > 
> > Suggested-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Michal Winiarski <michal.winiarski at intel.com>
> > Cc: Michel Thierry <michel.thierry at intel.com>
> > Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> > Cc: Mika Kuoppala <mika.kuoppala at intel.com>
> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> > ---
> >   drivers/gpu/drm/i915/intel_lrc.c | 4 ++++
> >   drivers/gpu/drm/i915/intel_lrc.h | 1 +
> >   2 files changed, 5 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> > index 22d471a4228d..c28f267a8417 100644
> > --- a/drivers/gpu/drm/i915/intel_lrc.c
> > +++ b/drivers/gpu/drm/i915/intel_lrc.c
> > @@ -2285,6 +2285,10 @@ populate_lr_context(struct i915_gem_context *ctx,
> >       if (!engine->default_state)
> >               regs[CTX_CONTEXT_CONTROL + 1] |=
> >                       _MASKED_BIT_ENABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT);
> > +     if (ctx->hw_id == PREEMPT_ID)
> > +             regs[CTX_CONTEXT_CONTROL + 1] |=
> > +                     _MASKED_BIT_ENABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT |
> > +                                        CTX_CTRL_ENGINE_CTX_SAVE_INHIBIT);
> >
> 
> This shouldn't break anything and ICL is not merged yet (plus things may 
> still change) so if you want to merge this,

I think it's the right thing conceptually to do.
 
> Reviewed-by: Michel Thierry <michel.thierry at intel.com>

Thanks.
 
> >       i915_gem_object_unpin_map(ctx_obj);
> >   
> > diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
> > index 6d4f9b995a11..636ced41225d 100644
> > --- a/drivers/gpu/drm/i915/intel_lrc.h
> > +++ b/drivers/gpu/drm/i915/intel_lrc.h
> > @@ -37,6 +37,7 @@
> >   #define       CTX_CTRL_INHIBIT_SYN_CTX_SWITCH       (1 << 3)
> >   #define       CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT   (1 << 0)
> >   #define   CTX_CTRL_RS_CTX_ENABLE                (1 << 1)
> > +#define        CTX_CTRL_ENGINE_CTX_SAVE_INHIBIT      (1 << 2)
> 
> CTX_CTRL_INHIBIT_SYN_CTX_SWITCH should be here, but that's for another 
> patch ;)

Also, now we have intel_lrc_reg.h. Hint, hint ;)

Should we also start using include/linux/bitfield.h for our register
descriptions? It looks a bit ungainly, but it's main advantage is static
checking of all the constants to catch trivial mistakes.
-Chris