Re: [PATCH RFC 1/4] drm/i915: add create_context2 ioctl

Joonas Lahtinen
2016-08-15T12:04:45Z

On ma, 2016-08-15 at 14:48 +0300, Mika Kuoppala wrote:
> @@ -2566,6 +2566,7 @@ static const struct drm_ioctl_desc i915_ioctls[] = {
>   DRM_IOCTL_DEF_DRV(I915_GEM_USERPTR, i915_gem_userptr_ioctl, DRM_RENDER_ALLOW),
>   DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_GETPARAM, i915_gem_context_getparam_ioctl, DRM_RENDER_ALLOW),
>   DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_SETPARAM, i915_gem_context_setparam_ioctl, DRM_RENDER_ALLOW),
> + DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_CREATE2, i915_gem_context_create2_ioctl, DRM_UNLOCKED),

Why DRM_UNLOCKED?

> @@ -394,7 +398,8 @@ i915_gem_context_create_gvt(struct drm_device *dev)
>   if (ret)
>   return ERR_PTR(ret);
>
> - ctx = i915_gem_create_context(dev, NULL);
> + ctx = i915_gem_create_context(dev, NULL, USES_FULL_PPGTT(dev) ?
> +       I915_GEM_CONTEXT_FULL_PPGTT : 0);

Could use flags variable here just like below this point in code.

> @@ -552,7 +561,8 @@ int i915_gem_context_open(struct drm_device *dev, struct drm_file *file)
>   idr_init(&file_priv->context_idr);
>
>   mutex_lock(&dev->struct_mutex);
> - ctx = i915_gem_create_context(dev, file_priv);
> + ctx = i915_gem_create_context(dev, file_priv, USES_FULL_PPGTT(dev) ?
> +       I915_GEM_CONTEXT_FULL_PPGTT : 0);

Ditto.

> +int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
> +   struct drm_file *file)
> +{
> + struct drm_i915_gem_context_create *args = data;
> + struct drm_i915_gem_context_create2 tmp;

'args2' just as we have create2?

> @@ -1142,6 +1144,22 @@ struct drm_i915_gem_context_create {
>   __u32 pad;
>  };
>
> +/*
> + * SVM handling
> + *
> + * A context can opt in to SVM support (thereby using its CPU page tables
> + * when accessing data from the GPU) by using the %I915_ENABLE_SVM flag

s/I915_ENABLE_SVM/I915_GEM_CONTEXT_ENABLE_SVM/ ?

> + * and passing an existing context id.  This is a one way transition; SVM
> + * contexts can not be downgraded into PPGTT contexts once converted.
> + */
> +#define I915_GEM_CONTEXT_ENABLE_SVM (1<<0)
> +#define I915_GEM_CONTEXT_FULL_PPGTT (1<<1)

BIT()

With the above addressed;

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation