Re: [PATCH] drm/i915: pwrite/pread do not require obj->base.filp, just pages

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 13/06/16 13:40, Chris Wilson wrote:
> The idea behind relaxing the restriction for pread/pwrite was to handle
> !obj->base.flip, i.e. non-shmemfs backed objects, which only requires
> that the object provide struct pages.
>
> v2: Remove excess (). Note enough editing after copy'n'paste.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_gem.c | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 21d0dea57312..6f950c37efab 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -760,7 +760,7 @@ i915_gem_shmem_pread(struct drm_device *dev,
>   	int needs_clflush = 0;
>   	struct sg_page_iter sg_iter;
>
> -	if (!obj->base.filp)
> +	if ((obj->ops->flags & I915_GEM_OBJECT_HAS_STRUCT_PAGE) == 0)
>   		return -ENODEV;
>
>   	user_data = u64_to_user_ptr(args->data_ptr);
> @@ -1298,7 +1298,8 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
>   	 * pread/pwrite currently are reading and writing from the CPU
>   	 * perspective, requiring manual detiling by the client.
>   	 */
> -	if (!obj->base.filp || cpu_write_needs_clflush(obj)) {
> +	if ((obj->ops->flags & I915_GEM_OBJECT_HAS_STRUCT_PAGE) == 0 ||
> +	    cpu_write_needs_clflush(obj)) {
>   		ret = i915_gem_gtt_pwrite_fast(dev_priv, obj, args, file);
>   		/* Note that the gtt paths might fail with non-page-backed user
>   		 * pointers (e.g. gtt mappings when moving data between
> @@ -1308,7 +1309,7 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
>   	if (ret == -EFAULT) {
>   		if (obj->phys_handle)
>   			ret = i915_gem_phys_pwrite(obj, args, file);
> -		else if (obj->base.filp)
> +		else if (obj->ops->flags & I915_GEM_OBJECT_HAS_STRUCT_PAGE)
>   			ret = i915_gem_shmem_pwrite(dev, obj, args, file);
>   		else
>   			ret = -ENODEV;
>

To enable on userptr or there is more to it? Would it even make more 
sense to keep rejecting it on userptr to discourage suboptimal userspace?

Regards,

Tvrtko

P.S. You need to send it outside this thread to get a CI run.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx