Quoting Tvrtko Ursulin (2017-11-15 13:26:09)
>
> On 14/11/2017 13:19, Chris Wilson wrote:
> > We introduce the concept of classes that each engine may belong to.
> > There may be more than one engine available in each class, but each
> > engine only belongs to one class. Each class has a unique set of
> > capabilities and purpose (e.g. 3D rendering or video decode), but
> > different engines within each class may have different features (e.g.
> > only the first decoder instance may have the full set of fixed function
> > blocks, but most of the work can still be offload onto the other decoders
> > which shared the same ISA etc).
> >
> > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> > ---
> > lib/igt_gt.c | 16 ++++++++--------
> > lib/igt_gt.h | 7 +++++++
> > 2 files changed, 15 insertions(+), 8 deletions(-)
> >
> > diff --git a/lib/igt_gt.c b/lib/igt_gt.c
> > index 89727d22..8496fe4d 100644
> > --- a/lib/igt_gt.c
> > +++ b/lib/igt_gt.c
> > @@ -578,14 +578,14 @@ unsigned intel_detect_and_clear_missed_interrupts(int fd)
> > }
> >
> > const struct intel_execution_engine intel_execution_engines[] = {
> > - { "default", NULL, 0, 0 },
> > - { "render", "rcs0", I915_EXEC_RENDER, 0 },
> > - { "bsd", "vcs0", I915_EXEC_BSD, 0 },
> > - { "bsd1", "vcs0", I915_EXEC_BSD, 1<<13 /*I915_EXEC_BSD_RING1*/ },
> > - { "bsd2", "vcs1", I915_EXEC_BSD, 2<<13 /*I915_EXEC_BSD_RING2*/ },
> > - { "blt", "bcs0", I915_EXEC_BLT, 0 },
> > - { "vebox", "vecs0", I915_EXEC_VEBOX, 0 },
> > - { NULL, 0, 0 }
> > + { "default", NULL, 0, LOCAL_ENGINE_CLASS_INVALID, 0 },
>
> A bit strange that you have marked it with invalid here. Will cause
> random issues for tests which will use class_id from this array. On the
> other hand maybe it is for the better since it will remove the "default"
> engine from the subtests ran once we (hopefully) switch to
> class-instance execbuf. Don't know, hard to guess, but it works so OK.
Default doesn't map onto a class-instance, it is vague and may map on to
whatever the kernel considers most convenient. And yes, when we are
iterating over HW engines and not I915_EXEC_$RING, we don't care for the
ambiguously defined uabi.
> > + { "render", "rcs0", I915_EXEC_RENDER, LOCAL_ENGINE_CLASS_RENDER, 0 },
> > + { "bsd", "vcs0", I915_EXEC_BSD, LOCAL_ENGINE_CLASS_VIDEO, 0 },
> > + { "bsd1", "vcs0", I915_EXEC_BSD, LOCAL_ENGINE_CLASS_VIDEO, 1<<13 /*I915_EXEC_BSD_RING1*/ },
> > + { "bsd2", "vcs1", I915_EXEC_BSD, LOCAL_ENGINE_CLASS_VIDEO, 2<<13 /*I915_EXEC_BSD_RING2*/ },
> > + { "blt", "bcs0", I915_EXEC_BLT, LOCAL_ENGINE_CLASS_COPY, 0 },
> > + { "vebox", "vecs0", I915_EXEC_VEBOX, LOCAL_ENGINE_CLASS_VIDEO_ENHANCE, 0 },
> > + { NULL }
> > };
> >
> > bool gem_can_store_dword(int fd, unsigned int engine)
> > diff --git a/lib/igt_gt.h b/lib/igt_gt.h
> > index 2579cbd3..4f13f86f 100644
> > --- a/lib/igt_gt.h
> > +++ b/lib/igt_gt.h
> > @@ -63,10 +63,17 @@ void igt_clflush_range(void *addr, int size);
> >
> > unsigned intel_detect_and_clear_missed_interrupts(int fd);
> >
> > +#define LOCAL_ENGINE_CLASS_INVALID -1
> > +#define LOCAL_ENGINE_CLASS_RENDER 0
> > +#define LOCAL_ENGINE_CLASS_COPY 1
> > +#define LOCAL_ENGINE_CLASS_VIDEO 2
> > +#define LOCAL_ENGINE_CLASS_VIDEO_ENHANCE 3
>
> Why not make a local copy of the enum to be consistent with the kernel
> uPAI headers?
Shrug. It has to be temporary. #define have the advantage that the
compiler only complains when they differ, which was why I suggesting we
didn't use enum ;)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
