[Intel-gfx] [PATCH] drm/i915: Introduce struct class_instance for engines across the uAPI

[Chris Wilson]

Quoting Tvrtko Ursulin (2019-04-12 08:02:04)
> 
> +Tony to comment on the media-driver views on this.
> 
> +Manually re-adding original Cc which did not appear for me from the ML.
> 
> On 12/04/2019 07:51, Chris Wilson wrote:
> > SSEU reprogramming of the context introduced the notion of engine class
> > and instance for a forwards compatible method of describing any engine
> > beyond the old execbuf interface. We wish to adopt this class:instance
> > description for more interfaces, so pull it out into a separate type for
> > userspace convenience.
> > 
> > References: e46c2e99f600 ("drm/i915: Expose RPCS (SSEU) configuration to userspace (Gen11 only)")
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> > Cc: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
> > Cc: Dmitry Rogozhkin <dmitry.v.rogozhkin at intel.com>
> > Cc: Andi Shyti <andi at etezian.org>
> > ---
> > Commit e46c2e99f600 ("drm/i915: Expose RPCS (SSEU) configuration to
> > userspace (Gen11 only)") is for v5.1, so we still do have a tiny bit of
> > time to enact this change.
> > ---
> >   drivers/gpu/drm/i915/i915_gem_context.c |  8 ++++----
> >   include/uapi/drm/i915_drm.h             | 11 +++++++++--
> >   2 files changed, 13 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> > index 7fc34ab6df87..dd728b26b5aa 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_context.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> > @@ -1386,8 +1386,8 @@ static int set_sseu(struct i915_gem_context *ctx,
> >               return -EINVAL;
> >   
> >       engine = intel_engine_lookup_user(i915,
> > -                                       user_sseu.engine_class,
> > -                                       user_sseu.engine_instance);
> > +                                       user_sseu.engine.engine_class,
> > +                                       user_sseu.engine.engine_instance);
> >       if (!engine)
> >               return -EINVAL;
> >   
> > @@ -1626,8 +1626,8 @@ static int get_sseu(struct i915_gem_context *ctx,
> >               return -EINVAL;
> >   
> >       engine = intel_engine_lookup_user(ctx->i915,
> > -                                       user_sseu.engine_class,
> > -                                       user_sseu.engine_instance);
> > +                                       user_sseu.engine.engine_class,
> > +                                       user_sseu.engine.engine_instance);
> >       if (!engine)
> >               return -EINVAL;
> >   
> > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> > index 52051d24d89d..f6369c462458 100644
> > --- a/include/uapi/drm/i915_drm.h
> > +++ b/include/uapi/drm/i915_drm.h
> > @@ -1500,6 +1500,14 @@ struct drm_i915_gem_context_param {
> >       __u64 value;
> >   };
> >   
> > +union drm_i915_engine_class_instance {
> > +     struct i915_engine_class_instance {
> > +             __u16 engine_class;
> > +             __u16 engine_instance;
> > +     } class_instance;
> > +     __u32 ci;
> 
> Is "ci" class instance or something to satisfy the test farm and what is 
> the purpose for the union?

No real purpose, it's just how I tend to play around with
class_instance. More a topic for discussion to see if anyone else would
find it useful enough to be a part of uapi.h (i.e. if it's common
practice).

This definition should be pulled next to the enum and cross-references
to the enum added. It will also be a good anchor for the various "not an
engine" flags to be described.
-Chris