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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
