[6/6] drm/i915/userptr: Enable read-only support on gen8+

Commit Message

Chris Wilson July 12, 2018, 6:53 p.m. UTC

On gen8 and onwards, we can mark GPU accesses through the ppGTT as being
read-only, that is cause any GPU write onto that page to be discarded
(not triggering a fault). This is all that we need to finally support
the read-only flag for userptr!

Testcase: igt/gem_userptr_blits/readonly*
Signed-off-by: Chris Wilson <[e-mail]>
Cc: Jon Bloomfield <[e-mail]>
Cc: Joonas Lahtinen <[e-mail]>
Reviewed-by: Joonas Lahtinen <[e-mail]>
Reviewed-by: Matthew Auld <[e-mail]>
Reviewed-by: Jon Bloomfield <[e-mail]>
---
 drivers/gpu/drm/i915/i915_gem_object.h  |  1 -
 drivers/gpu/drm/i915/i915_gem_userptr.c | 15 +++++++++------
 2 files changed, 9 insertions(+), 7 deletions(-)

Comments

Chris Wilson July 12, 2018, 7:08 p.m. UTC | #1

Quoting Chris Wilson (2018-07-12 19:53:15)
> @@ -789,10 +789,12 @@ i915_gem_userptr_ioctl(struct drm_device *dev,
>                 return -EFAULT;
>  
>         if (args->flags & I915_USERPTR_READ_ONLY) {
> -               /* On almost all of the current hw, we cannot tell the GPU that a
> -                * page is readonly, so this is just a placeholder in the uAPI.
> +               /*
> +                * On almost all of the older hw, we cannot tell the GPU that
> +                * a page is readonly.
>                  */
> -               return -ENODEV;
> +               if (INTEL_GEN(dev_priv) < 8 || !USES_PPGTT(dev_priv))
> +                       return -ENODEV;

Hmm, I need to be more careful here considering gvt.
-Chris

Patch

diff --git a/drivers/gpu/drm/i915/i915_gem_object.h b/drivers/gpu/drm/i915/i915_gem_object.h
index 56e9f00d2c4c..83e5e01fa9ea 100644
--- a/drivers/gpu/drm/i915/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/i915_gem_object.h
@@ -267,7 +267,6 @@  struct drm_i915_gem_object {
 	union {
 		struct i915_gem_userptr {
 			uintptr_t ptr;
-			unsigned read_only :1;
 
 			struct i915_mm_struct *mm;
 			struct i915_mmu_object *mmu_object;
diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c b/drivers/gpu/drm/i915/i915_gem_userptr.c
index 854bd51b9478..045db5ef17ac 100644
--- a/drivers/gpu/drm/i915/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
@@ -507,7 +507,7 @@  __i915_gem_userptr_get_pages_worker(struct work_struct *_work)
 		struct mm_struct *mm = obj->userptr.mm->mm;
 		unsigned int flags = 0;
 
-		if (!obj->userptr.read_only)
+		if (!i915_gem_object_is_readonly(obj))
 			flags |= FOLL_WRITE;
 
 		ret = -EFAULT;
@@ -643,7 +643,7 @@  static int i915_gem_userptr_get_pages(struct drm_i915_gem_object *obj)
 		if (pvec) /* defer to worker if malloc fails */
 			pinned = __get_user_pages_fast(obj->userptr.ptr,
 						       num_pages,
-						       !obj->userptr.read_only,
+						       !i915_gem_object_is_readonly(obj),
 						       pvec);
 	}
 
@@ -789,10 +789,12 @@  i915_gem_userptr_ioctl(struct drm_device *dev,
 		return -EFAULT;
 
 	if (args->flags & I915_USERPTR_READ_ONLY) {
-		/* On almost all of the current hw, we cannot tell the GPU that a
-		 * page is readonly, so this is just a placeholder in the uAPI.
+		/*
+		 * On almost all of the older hw, we cannot tell the GPU that
+		 * a page is readonly.
 		 */
-		return -ENODEV;
+		if (INTEL_GEN(dev_priv) < 8 || !USES_PPGTT(dev_priv))
+			return -ENODEV;
 	}
 
 	obj = i915_gem_object_alloc(dev_priv);
@@ -806,7 +808,8 @@  i915_gem_userptr_ioctl(struct drm_device *dev,
 	i915_gem_object_set_cache_coherency(obj, I915_CACHE_LLC);
 
 	obj->userptr.ptr = args->user_ptr;
-	obj->userptr.read_only = !!(args->flags & I915_USERPTR_READ_ONLY);
+	if (args->flags & I915_USERPTR_READ_ONLY)
+		i915_gem_object_set_readonly(obj);
 
 	/* And keep a pointer to the current->mm for resolving the user pages
 	 * at binding. This means that we need to hook into the mmu_notifier