[Intel-gfx] [PATCH] drm/i915/icl: Adjust BSD2 semantics to mean any second VCS instance

[Joonas Lahtinen]

Quoting Tvrtko Ursulin (2018-04-18 12:33:42)
> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> 
> 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 at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Jon Bloomfield <jon.bloomfield at intel.com>
> Cc: Tony Ye <tony.ye at intel.com>
> ---
> 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

>                 break;
>         case I915_PARAM_HAS_LLC:
>                 value = HAS_LLC(dev_priv);
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index c74f5df3fb5a..80b32460567d 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -2052,7 +2052,9 @@ eb_select_engine(struct drm_i915_private *dev_priv,
>                 return NULL;
>         }
>  
> -       if (user_ring_id == I915_EXEC_BSD && HAS_BSD2(dev_priv)) {
> +       if (user_ring_id == I915_EXEC_BSD &&
> +           (INTEL_INFO(dev_priv)->ring_mask &
> +            (ENGINE_MASK(_VCS(1)) | ENGINE_MASK(_VCS(2))))) {
>                 unsigned int bsd_idx = args->flags & I915_EXEC_BSD_MASK;
>  
>                 if (bsd_idx == I915_EXEC_BSD_DEFAULT) {
> @@ -2068,6 +2070,12 @@ eb_select_engine(struct drm_i915_private *dev_priv,
>                 }
>  
>                 engine = dev_priv->engine[_VCS(bsd_idx)];
> +
> +               /*
> +                * FIXME Temporarily map VCS2 to VCS1 on Icelake.
> +                */
> +               if (!engine && bsd_idx)
> +                       engine = dev_priv->engine[_VCS(2)];
>         } else {
>                 engine = dev_priv->engine[user_ring_map[user_ring_id]];
>         }
> -- 
> 2.14.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx