Re: [Mesa-dev] [RFC v3] drm/i915: Select engines via class and instance in execbuffer2
On Thu, May 18, 2017 at 01:55:59PM +0300, Joonas Lahtinen wrote: > On ke, 2017-05-17 at 16:40 +0100, Tvrtko Ursulin wrote: > > From: Tvrtko Ursulin> > > > Building on top of the previous patch which exported the concept > > of engine classes and instances, we can also use this instead of > > the current awkward engine selection uAPI. > > > > This is primarily interesting for the VCS engine selection which > > is a) currently done via disjoint set of flags, and b) the > > current I915_EXEC_BSD flags has different semantics depending on > > the underlying hardware which is bad. > > > > Proposed idea here is to reserve 16-bits of flags, to pass in > > the engine class and instance (8 bits each), and a new flag > > named I915_EXEC_CLASS_INSTACE to tell the kernel this new engine > > selection API is in use. Note this text doesn't describe the interface in v3. > Would it make sense to use bitmask for future proofing? Then we could > allow passing multiple options in the future. No. The first use case has to be explicit control of engine. That's orthogonal to asking to select any of a particular class. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC v3] drm/i915: Select engines via class and instance in execbuffer2
On ke, 2017-05-17 at 16:40 +0100, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin> > Building on top of the previous patch which exported the concept > of engine classes and instances, we can also use this instead of > the current awkward engine selection uAPI. > > This is primarily interesting for the VCS engine selection which > is a) currently done via disjoint set of flags, and b) the > current I915_EXEC_BSD flags has different semantics depending on > the underlying hardware which is bad. > > Proposed idea here is to reserve 16-bits of flags, to pass in > the engine class and instance (8 bits each), and a new flag > named I915_EXEC_CLASS_INSTACE to tell the kernel this new engine > selection API is in use. Would it make sense to use bitmask for future proofing? Then we could allow passing multiple options in the future. Other than that, looks good. Chris already commented some on the code itself. Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [RFC v3] drm/i915: Select engines via class and instance in execbuffer2
From: Tvrtko UrsulinBuilding on top of the previous patch which exported the concept of engine classes and instances, we can also use this instead of the current awkward engine selection uAPI. This is primarily interesting for the VCS engine selection which is a) currently done via disjoint set of flags, and b) the current I915_EXEC_BSD flags has different semantics depending on the underlying hardware which is bad. Proposed idea here is to reserve 16-bits of flags, to pass in the engine class and instance (8 bits each), and a new flag named I915_EXEC_CLASS_INSTACE to tell the kernel this new engine selection API is in use. The new uAPI also removes access to the weak VCS engine balancing as currently existing in the driver. Example usage to send a command to VCS0: eb.flags = i915_execbuffer2_engine(DRM_I915_ENGINE_CLASS_VIDEO_DECODE, 0); Or to send a command to VCS1: eb.flags = i915_execbuffer2_engine(DRM_I915_ENGINE_CLASS_VIDEO_DECODE, 1); v2: * Fix unknown flags mask. * Use I915_EXEC_RING_MASK for class. (Chris Wilson) v3: * Add a map for fast class-instance engine lookup. (Chris Wilson) Signed-off-by: Tvrtko Ursulin Cc: Ben Widawsky Cc: Chris Wilson Cc: Daniel Vetter Cc: Joonas Lahtinen Cc: Jon Bloomfield Cc: Daniel Charles Cc: "Rogozhkin, Dmitry V" Cc: Oscar Mateo Cc: "Gong, Zhipeng" Cc: intel-vaapi-me...@lists.01.org Cc: mesa-dev@lists.freedesktop.org --- drivers/gpu/drm/i915/i915_drv.h| 1 + drivers/gpu/drm/i915/i915_gem_execbuffer.c | 30 ++ drivers/gpu/drm/i915/i915_reg.h| 3 +++ drivers/gpu/drm/i915/intel_engine_cs.c | 7 +++ include/uapi/drm/i915_drm.h| 11 ++- 5 files changed, 51 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 5dfa4a12e647..7bf4fd42480c 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2066,6 +2066,7 @@ struct drm_i915_private { struct pci_dev *bridge_dev; struct i915_gem_context *kernel_context; struct intel_engine_cs *engine[I915_NUM_ENGINES]; + struct intel_engine_cs *engine_class[MAX_ENGINE_CLASS + 1][MAX_ENGINE_INSTANCE + 1]; struct i915_vma *semaphore; struct drm_dma_handle *status_page_dmah; diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index af1965774e7b..c1ad49ab64cd 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -1492,6 +1492,33 @@ gen8_dispatch_bsd_engine(struct drm_i915_private *dev_priv, return file_priv->bsd_engine; } +extern u8 user_class_map[DRM_I915_ENGINE_CLASS_MAX]; + +static struct intel_engine_cs *get_engine_class(struct drm_i915_private *i915, + u8 class, u8 instance) +{ + if (class > MAX_ENGINE_CLASS || instance > MAX_ENGINE_INSTANCE) + return NULL; + + return i915->engine_class[class][instance]; +} + +static struct intel_engine_cs * +eb_select_engine_class_instance(struct drm_i915_private *i915, + struct drm_i915_gem_execbuffer2 *args) +{ + u8 class, instance; + + class = args->flags & I915_EXEC_RING_MASK; + if (class >= DRM_I915_ENGINE_CLASS_MAX) + return NULL; + + instance = (args->flags >> I915_EXEC_INSTANCE_SHIFT) && + I915_EXEC_INSTANCE_MASK; + + return get_engine_class(i915, user_class_map[class], instance); +} + #define I915_USER_RINGS (4) static const enum intel_engine_id user_ring_map[I915_USER_RINGS + 1] = { @@ -1510,6 +1537,9 @@ eb_select_engine(struct drm_i915_private *dev_priv, unsigned int user_ring_id = args->flags & I915_EXEC_RING_MASK; struct intel_engine_cs *engine; + if (args->flags & I915_EXEC_CLASS_INSTANCE) + return eb_select_engine_class_instance(dev_priv, args); + if (user_ring_id > I915_USER_RINGS) { DRM_DEBUG("execbuf with unknown ring: %u\n", user_ring_id); return NULL; diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index ee144ec57935..a3b59043b991 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -92,6 +92,9 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg) #define VIDEO_ENHANCEMENT_CLASS2 #define COPY_ENGINE_CLASS 3 #define OTHER_CLASS4 +#define MAX_ENGINE_CLASS 4 + +#define MAX_ENGINE_INSTANCE1 /* PCI config space */ diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c
Re: [Mesa-dev] [RFC v3] drm/i915: Select engines via class and instance in execbuffer2
On Wed, May 17, 2017 at 04:40:57PM +0100, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin> > Building on top of the previous patch which exported the concept > of engine classes and instances, we can also use this instead of > the current awkward engine selection uAPI. > > This is primarily interesting for the VCS engine selection which > is a) currently done via disjoint set of flags, and b) the > current I915_EXEC_BSD flags has different semantics depending on > the underlying hardware which is bad. > > Proposed idea here is to reserve 16-bits of flags, to pass in > the engine class and instance (8 bits each), and a new flag > named I915_EXEC_CLASS_INSTACE to tell the kernel this new engine > selection API is in use. > > The new uAPI also removes access to the weak VCS engine > balancing as currently existing in the driver. > > Example usage to send a command to VCS0: > > eb.flags = i915_execbuffer2_engine(DRM_I915_ENGINE_CLASS_VIDEO_DECODE, 0); > > Or to send a command to VCS1: > > eb.flags = i915_execbuffer2_engine(DRM_I915_ENGINE_CLASS_VIDEO_DECODE, 1); > > v2: > * Fix unknown flags mask. > * Use I915_EXEC_RING_MASK for class. (Chris Wilson) > > v3: > * Add a map for fast class-instance engine lookup. (Chris Wilson) > > Signed-off-by: Tvrtko Ursulin > Cc: Ben Widawsky > Cc: Chris Wilson > Cc: Daniel Vetter > Cc: Joonas Lahtinen > Cc: Jon Bloomfield > Cc: Daniel Charles > Cc: "Rogozhkin, Dmitry V" > Cc: Oscar Mateo > Cc: "Gong, Zhipeng" > Cc: intel-vaapi-me...@lists.01.org > Cc: mesa-dev@lists.freedesktop.org > --- > drivers/gpu/drm/i915/i915_drv.h| 1 + > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 30 > ++ > drivers/gpu/drm/i915/i915_reg.h| 3 +++ > drivers/gpu/drm/i915/intel_engine_cs.c | 7 +++ > include/uapi/drm/i915_drm.h| 11 ++- > 5 files changed, 51 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 5dfa4a12e647..7bf4fd42480c 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -2066,6 +2066,7 @@ struct drm_i915_private { > struct pci_dev *bridge_dev; > struct i915_gem_context *kernel_context; > struct intel_engine_cs *engine[I915_NUM_ENGINES]; > + struct intel_engine_cs *engine_class[MAX_ENGINE_CLASS + > 1][MAX_ENGINE_INSTANCE + 1]; > struct i915_vma *semaphore; > > struct drm_dma_handle *status_page_dmah; > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > index af1965774e7b..c1ad49ab64cd 100644 > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > @@ -1492,6 +1492,33 @@ gen8_dispatch_bsd_engine(struct drm_i915_private > *dev_priv, > return file_priv->bsd_engine; > } > > +extern u8 user_class_map[DRM_I915_ENGINE_CLASS_MAX]; > + > +static struct intel_engine_cs *get_engine_class(struct drm_i915_private > *i915, > + u8 class, u8 instance) > +{ > + if (class > MAX_ENGINE_CLASS || instance > MAX_ENGINE_INSTANCE) > + return NULL; > + > + return i915->engine_class[class][instance]; > +} Be bold make this this an intel_engine_lookup(), I forsee some other users appearing very shortly. > +static struct intel_engine_cs * > +eb_select_engine_class_instance(struct drm_i915_private *i915, > + struct drm_i915_gem_execbuffer2 *args) > +{ > + u8 class, instance; > + > + class = args->flags & I915_EXEC_RING_MASK; > + if (class >= DRM_I915_ENGINE_CLASS_MAX) > + return NULL; > + > + instance = (args->flags >> I915_EXEC_INSTANCE_SHIFT) && > +I915_EXEC_INSTANCE_MASK; > + > + return get_engine_class(i915, user_class_map[class], instance); > +} > + > #define I915_USER_RINGS (4) > > static const enum intel_engine_id user_ring_map[I915_USER_RINGS + 1] = { > @@ -1510,6 +1537,9 @@ eb_select_engine(struct drm_i915_private *dev_priv, > unsigned int user_ring_id = args->flags & I915_EXEC_RING_MASK; > struct intel_engine_cs *engine; > > + if (args->flags & I915_EXEC_CLASS_INSTANCE) > + return eb_select_engine_class_instance(dev_priv, args); > + > if (user_ring_id > I915_USER_RINGS) { > DRM_DEBUG("execbuf with unknown ring: %u\n", user_ring_id); > return NULL; > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index ee144ec57935..a3b59043b991 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@