[Intel-gfx] [PATCH 06/15] drm/i915/selftests: Check known register values within the context

[Chris Wilson]

Quoting Tvrtko Ursulin (2019-10-14 10:59:44)
> 
> On 14/10/2019 10:07, Chris Wilson wrote:
> > +static int __live_lrc_state(struct i915_gem_context *fixme,
> > +                         struct intel_engine_cs *engine,
> > +                         struct i915_vma *scratch)
> > +{
> > +     struct intel_context *ce;
> > +     struct i915_request *rq;
> > +     enum {
> > +             RING_START_IDX = 0,
> > +             RING_TAIL_IDX,
> > +             MAX_IDX
> > +     };
> > +     u32 expected[MAX_IDX];
> > +     u32 *cs;
> > +     int err;
> > +     int n;
> > +
> > +     ce = intel_context_create(fixme, engine);
> 
> Calling the context fixme imo just makes the code less readable.

If you review some other patches, I wouldn't need a GEM context here :-p
Vacations!

> >   int intel_lrc_live_selftests(struct drm_i915_private *i915)
> >   {
> >       static const struct i915_subtest tests[] = {
> >               SUBTEST(live_lrc_layout),
> > +             SUBTEST(live_lrc_state),
> >       };
> >   
> >       if (!HAS_LOGICAL_RING_CONTEXTS(i915))
> > 
> 
> I don't know.. guess it has some extra value compared to basic MI_NOOP 
> tests.

Yeah, it's very much chicken and egg. I like the idea of confirming that
the lrc we execute is the same as we setup - but that's kind of implied
by successful execution (or so one would assume). I regard it as mostly
a placeholder, a seed of a test idea, and plan to add more state checks
as soon as I think of some more reliable checks. Or anyone else for that
matter. :) I did think that verifying the vm matches the ce->vm would be
a good addition.
-Chris