> -----Original Message----- > From: Intel-gfx <intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of > Joonas Lahtinen > Sent: Wednesday, April 18, 2018 3:43 AM > To: Intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Tvrtko Ursulin <tursulin@xxxxxxxxxxx> > Subject: Re: [PATCH] drm/i915/icl: Adjust BSD2 semantics to mean > any second VCS instance > > Quoting Tvrtko Ursulin (2018-04-18 12:33:42) > > From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> > > > > Currently our driver assumes BSD2 means hardware engine instance > number > > two. This does not work for Icelake parts with two VCS engines, but which > > are hardware instances 0 and 2, and not 0 and 1 as with previous parts. > > > > This makes the second engine not discoverable via HAS_BSD2 get param, > nor > > it can be targetted by execbuf. > > > > While we are working on the next generation execbuf put in a hack which > > allows discovery and access to this second VCS engine using legacy ABI. > > > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> > > Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > > Cc: Jon Bloomfield <jon.bloomfield@xxxxxxxxx> > > Cc: Tony Ye <tony.ye@xxxxxxxxx> > > --- > > Compile tested only. > > > > Also, one could argue if this is just a temporary hack or could actually > > make sense to have this permanently in. It feels like the ABI semantics of > > BSD2 are blurry, or at least could be re-blurred for Gen11. > > --- > > drivers/gpu/drm/i915/i915_drv.c | 8 +++++++- > > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 10 +++++++++- > > 2 files changed, 16 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > b/drivers/gpu/drm/i915/i915_drv.c > > index b7dbeba72dec..a185366d9beb 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.c > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > @@ -331,7 +331,13 @@ static int i915_getparam_ioctl(struct drm_device > *dev, void *data, > > value = !!dev_priv->engine[VECS]; > > break; > > case I915_PARAM_HAS_BSD2: > > - value = !!dev_priv->engine[VCS2]; > > + /* > > + * FIXME: Temporary hack for Icelake. > > + * > > + * Make semantics of HAS_BSD2 "has second", or "has two" > VDBOXes > > + * instead of "has VDBOX 2nd hardware instance". > > + */ > > + value = dev_priv->engine[VCS2] || dev_priv->engine[VCS3]; > > There can be no temporary hacks for the uAPI... You either sign yourself > up to keep it working indefinitely or don't :) > > Regards, Joonas This doesn't really change the API does it? In fact I'd argue this is simply fixing a breakage in the API wrt to previous devices. It makes no sense to expose holes in the engine map to userspace. If a device has two useable VCS engines, HAS_BSD2 should reflect that, and the second engine (wherever it sits physically), should be addressable through the legacy BSD2 execbuf interface. _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
