Re: [PATCH 4/4] drm/i915: Support for pread/pwrite from/to non shmem backed objects

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 7/22/2015 8:09 PM, Chris Wilson wrote:
On Wed, Jul 22, 2015 at 07:21:49PM +0530, ankitprasad.r.sharma@xxxxxxxxx wrote:
  static int
  i915_gem_shmem_pread(struct drm_device *dev,
  		     struct drm_i915_gem_object *obj,
@@ -754,17 +850,20 @@ i915_gem_pread_ioctl(struct drm_device *dev, void *data,
  		goto out;
  	}

-	/* prime objects have no backing filp to GEM pread/pwrite
-	 * pages from.
-	 */
-	if (!obj->base.filp) {
-		ret = -EINVAL;
-		goto out;
-	}
-
  	trace_i915_gem_object_pread(obj, args->offset, args->size);

-	ret = i915_gem_shmem_pread(dev, obj, args, file);
+	/* pread for non shmem backed objects */
+	if (!obj->base.filp) {
+		if (obj->tiling_mode == I915_TILING_NONE)

pread/pwrite is defined as a cpu linear, the restriction upon tiling is
a simplification of handling swizzling.

+			ret = i915_gem_gtt_pread_pwrite(dev, obj, args->size,
+							args->offset,
+							args->data_ptr,
+							false);
+		else
+			ret = -EINVAL;
+	} else {
+		ret = i915_gem_shmem_pread(dev, obj, args, file);
+	}

  out:
  	drm_gem_object_unreference(&obj->base);
@@ -1105,17 +1204,22 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
  		goto out;
  	}

-	/* prime objects have no backing filp to GEM pread/pwrite
-	 * pages from.
-	 */
-	if (!obj->base.filp) {
-		ret = -EINVAL;
-		goto out;
-	}
-
  	trace_i915_gem_object_pwrite(obj, args->offset, args->size);

  	ret = -EFAULT;
+
+	/* pwrite for non shmem backed objects */
+	if (!obj->base.filp) {
+		if (obj->tiling_mode == I915_TILING_NONE)
+			ret = i915_gem_gtt_pread_pwrite(dev, obj, args->size,
+							args->offset,
+							args->data_ptr,
+							true);
+		else
+			ret = -EINVAL;
+
+		goto out;

The fast pwrite code always works for obj->base.filp == NULL. To easily
make it generic and handle the cases where we cannot fallback to shem,
undo the PIN_NONBLOCK.

Then you just want something like
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index d3016f37cd4d..f2284a27dd6d 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1114,8 +1114,9 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
          * perspective, requiring manual detiling by the client.
          */
         if (obj->tiling_mode == I915_TILING_NONE &&

Since the suggestion is to reuse the gtt_pwrite_fast function only for non-shmem backed objects, can we also relax the Tiling constraint here, apart from removing the PIN_NONBLOCK flag. As anyways there is a put_fence already being done in gtt_pwrite_fast function.

Also currently the gtt_pwrite_fast function is non-tolerant to faults, incurred while accessing the User space buffer, so can that also be relaxed (at least for the non-shmem backed objects) ??

Best regards
Akash

-           obj->base.write_domain != I915_GEM_DOMAIN_CPU &&
-           cpu_write_needs_clflush(obj)) {
+           (obj->base.filp == NULL ||
+            (obj->base.write_domain != I915_GEM_DOMAIN_CPU &&
+             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
@@ -1125,7 +1126,7 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
         if (ret == -EFAULT || ret == -ENOSPC) {
                 if (obj->phys_handle)
                         ret = i915_gem_phys_pwrite(obj, args, file);
-               else
+               else if (obj->filp)
                         ret = i915_gem_shmem_pwrite(dev, obj, args, file);
         }


to enable pwrite access to stolen.
-Chris

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux