[Intel-gfx] [PATCH v2 3/3] drm/i915: Only allocate preempt context when required

[Chris Wilson]

Quoting Mika Kuoppala (2018-01-31 14:38:02)
> > @@ -1988,6 +1988,11 @@ static int logical_ring_init(struct intel_engine_cs *engine)
> >       engine->execlists.elsp =
> >               engine->i915->regs + i915_mmio_reg_offset(RING_ELSP(engine));
> >  
> > +     engine->execlists.preempt_complete_status = ~0u;
> 
> This is to ensure that we catch a rogue status? Atleast we will
> bug on of preempting with this status as the EXECLIST_ACTIVE_PREEMPT
> will be false.

0 is the value for the i915->kernel_context. ~0u is currently invalid
and that looks to be the case (that at least it will never match an
active context) for the near future.

> > +     if (engine->i915->preempt_context)
> > +             engine->execlists.preempt_complete_status =
> > +                     upper_32_bits(engine->i915->preempt_context->engine[engine->id].lrc_desc);
> 
> 
> We have not upgraded the descriptor yet, so just use preempt_context->hw_id;

The upper_32_bits should be set; and I think it is important that the
usage is consistent. If we change the upper_32_bits later, we should try
to remember to update the preempt_complete_status. I.e. I feel it is
vital to remember that the complete_status is the
upper_32_bits(lrc_desc) that just happens to be hw_id due to our limited
definition today.

> I would also like duplicate, from i915_gem_context.c, the assertion that
> hw_id needs to be <= INT_MAX but didn't find a good spot.

For what purpose? When defining lrc_desc checking that hw_id fits within
the field width would be sensible.
-Chris