Re: [PATCH 5/6] 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 11/12/15 18:15, Daniel Vetter wrote:
On Wed, Dec 09, 2015 at 07:39:56PM +0000, Dave Gordon wrote:
On 09/12/15 16:15, Tvrtko Ursulin wrote:

Hi,

On 09/12/15 12:46, ankitprasad.r.sharma@xxxxxxxxx wrote:
From: Ankitprasad Sharma <ankitprasad.r.sharma@xxxxxxxxx>

This patch adds support for extending the pread/pwrite functionality
for objects not backed by shmem. The access will be made through
gtt interface. This will cover objects backed by stolen memory as well
as other non-shmem backed objects.

v2: Drop locks around slow_user_access, prefault the pages before
access (Chris)

v3: Rebased to the latest drm-intel-nightly (Ankit)

v4: Moved page base & offset calculations outside the copy loop,
corrected data types for size and offset variables, corrected if-else
braces format (Tvrtko/kerneldocs)

v5: Enabled pread/pwrite for all non-shmem backed objects including
without tiling restrictions (Ankit)

v6: Using pwrite_fast for non-shmem backed objects as well (Chris)

v7: Updated commit message, Renamed i915_gem_gtt_read to
i915_gem_gtt_copy,
added pwrite slow path for non-shmem backed objects (Chris/Tvrtko)

v8: Updated v7 commit message, mutex unlock around pwrite slow path for
non-shmem backed objects (Tvrtko)

Testcase: igt/gem_stolen

Signed-off-by: Ankitprasad Sharma <ankitprasad.r.sharma@xxxxxxxxx>
---
  drivers/gpu/drm/i915/i915_gem.c | 151
+++++++++++++++++++++++++++++++++-------
  1 file changed, 127 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c
[snip!]
  static int
  i915_gem_shmem_pread(struct drm_device *dev,
               struct drm_i915_gem_object *obj,
@@ -737,17 +830,14 @@ 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 && obj->tiling_mode == I915_TILING_NONE)
+        ret = i915_gem_gtt_copy(dev, obj, args->size,
+                    args->offset, args->data_ptr);
+    else
+        ret = i915_gem_shmem_pread(dev, obj, args, file);

Hm, it will end up calling i915_gem_shmem_pread for non-shmem backed
objects if tiling is set. Sounds wrong to me unless I am missing something?

Which GEM objects have obj->base.filp set? Is it ONLY regular gtt-type

obj->base.filp is for shmem backed stuff. gtt is irrelevant for backing
storage, well except if you can't read the shmem stuff directly with the
cpu the only way is to go through a gtt device mapping.
-Daniel

So obj->base.filp is set for both phys and shmem (default) object types; I called the latter a "gtt" type just because the get/put_pages() functions have "gtt" in their names). But I note that the naming of GEM object vfuncs (and vfunc tables) isn't consistent :( Maybe they should be named "i915_gem_object_{get,put}_pages_shmem()", and the table would then be i915_gem_object_shmem_ops :)

.Dave.
_______________________________________________
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