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

[Chris Wilson]

On Mon, Jun 13, 2016 at 01:45:56PM +0100, Tvrtko Ursulin wrote:
> 
> 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 at chris-wilson.co.uk>
> >Cc: Ankitprasad Sharma <ankitprasad.r.sharma at intel.com>
> >Cc: Tvrtko Ursulin <tvrtko.ursulin at 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?

And prime objects, and everything in future not backed by a filp.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre