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

[Joonas Lahtinen <joonas.lahtinen@linux.intel.com>]

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx