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

From: Chris Wilson <chris@chris-wilson.co.uk>
To: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>,
	intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 06/15] drm/i915/selftests: Check known register values within the context
Date: Mon, 14 Oct 2019 11:06:53 +0100	[thread overview]
Message-ID: <157104761361.18859.2555020038515542881@skylake-alporthouse-com> (raw)
In-Reply-To: <73a01df1-6278-2a9f-d03d-4031936c8c7e@linux.intel.com>

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