Re: [PATCH] Revert "drm/i915: Disallow pin ioctl completely for kms drivers"

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Nov 25, 2014 at 11:42:56AM +0000, Chris Wilson wrote:
> This reverts commit c211a47c2c28562f8a3fff9e027be1a3ed9e154a.
> 
> This causes an unwarranteed API break for existing and active userspace.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Hm, SNA still seems to be able to cope with this and I really don't see
the point of keeping this interface going and patching it up. With GEM the
kernel should be in control of shared resources, letting userspace in to
the game just leads to tears. And we have them now. Keeping pinning around
just because we've forgotten to properly disable it was ok with me, but
fixing it up when it starts to fall apart really isn't.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_gem.c | 90 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 90 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 614bc2bc16fe..4a1ca7abd7f9 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4252,6 +4252,96 @@ i915_gem_object_unpin_fence(struct drm_i915_gem_object *obj)
>  }
>  
>  int
> +i915_gem_pin_ioctl(struct drm_device *dev, void *data,
> +		   struct drm_file *file)
> +{
> +	struct drm_i915_gem_pin *args = data;
> +	struct drm_i915_gem_object *obj;
> +	int ret;
> +
> +	ret = i915_mutex_lock_interruptible(dev);
> +	if (ret)
> +		return ret;
> +
> +	obj = to_intel_bo(drm_gem_object_lookup(dev, file, args->handle));
> +	if (&obj->base == NULL) {
> +		ret = -ENOENT;
> +		goto unlock;
> +	}
> +
> +	if (obj->madv != I915_MADV_WILLNEED) {
> +		DRM_DEBUG("Attempting to pin a purgeable buffer\n");
> +		ret = -EFAULT;
> +		goto out;
> +	}
> +
> +	if (obj->pin_filp != NULL && obj->pin_filp != file) {
> +		DRM_DEBUG("Already pinned in i915_gem_pin_ioctl(): %d\n",
> +			  args->handle);
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	if (obj->user_pin_count == ULONG_MAX) {
> +		ret = -EBUSY;
> +		goto out;
> +	}
> +
> +	if (obj->user_pin_count == 0) {
> +		ret = i915_gem_obj_ggtt_pin(obj, args->alignment, PIN_MAPPABLE);
> +		if (ret)
> +			goto out;
> +	}
> +
> +	obj->user_pin_count++;
> +	obj->pin_filp = file;
> +
> +	args->offset = i915_gem_obj_ggtt_offset(obj);
> +out:
> +	drm_gem_object_unreference(&obj->base);
> +unlock:
> +	mutex_unlock(&dev->struct_mutex);
> +	return ret;
> +}
> +
> +int
> +i915_gem_unpin_ioctl(struct drm_device *dev, void *data,
> +		     struct drm_file *file)
> +{
> +	struct drm_i915_gem_pin *args = data;
> +	struct drm_i915_gem_object *obj;
> +	int ret;
> +
> +	ret = i915_mutex_lock_interruptible(dev);
> +	if (ret)
> +		return ret;
> +
> +	obj = to_intel_bo(drm_gem_object_lookup(dev, file, args->handle));
> +	if (&obj->base == NULL) {
> +		ret = -ENOENT;
> +		goto unlock;
> +	}
> +
> +	if (obj->pin_filp != file) {
> +		DRM_DEBUG("Not pinned by caller in i915_gem_pin_ioctl(): %d\n",
> +			  args->handle);
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +	obj->user_pin_count--;
> +	if (obj->user_pin_count == 0) {
> +		obj->pin_filp = NULL;
> +		i915_gem_object_ggtt_unpin(obj);
> +	}
> +
> +out:
> +	drm_gem_object_unreference(&obj->base);
> +unlock:
> +	mutex_unlock(&dev->struct_mutex);
> +	return ret;
> +}
> +
> +int
>  i915_gem_busy_ioctl(struct drm_device *dev, void *data,
>  		    struct drm_file *file)
>  {
> -- 
> 2.1.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx