Re: [Intel-gfx] [PATCH 5/6] drm/i915: Support for pread/pwrite from/to non shmem backed objects

2015-12-15 Thread Dave Gordon

On 11/12/15 18:15, Daniel Vetter wrote:

On Wed, Dec 09, 2015 at 07:39:56PM +, Dave Gordon wrote:

On 09/12/15 16:15, Tvrtko Ursulin wrote:


Hi,

On 09/12/15 12:46, ankitprasad.r.sha...@intel.com wrote:

From: Ankitprasad Sharma 

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 
---
  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@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 5/6] drm/i915: Support for pread/pwrite from/to non shmem backed objects

2015-12-11 Thread Daniel Vetter
On Wed, Dec 09, 2015 at 07:39:56PM +, Dave Gordon wrote:
> On 09/12/15 16:15, Tvrtko Ursulin wrote:
> >
> >Hi,
> >
> >On 09/12/15 12:46, ankitprasad.r.sha...@intel.com wrote:
> >>From: Ankitprasad Sharma 
> >>
> >>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 
> >>---
> >>  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
> >>b/drivers/gpu/drm/i915/i915_gem.c
> >>index ed97de6..68ed67a 100644
> >>--- a/drivers/gpu/drm/i915/i915_gem.c
> >>+++ b/drivers/gpu/drm/i915/i915_gem.c
> >>@@ -614,6 +614,99 @@ shmem_pread_slow(struct page *page, int
> >>shmem_page_offset, int page_length,
> >>  return ret ? - EFAULT : 0;
> >>  }
> >>
> >>+static inline uint64_t
> >>+slow_user_access(struct io_mapping *mapping,
> >>+ uint64_t page_base, int page_offset,
> >>+ char __user *user_data,
> >>+ int length, bool pwrite)
> >>+{
> >>+void __iomem *vaddr_inatomic;
> >>+void *vaddr;
> >>+uint64_t unwritten;
> >>+
> >>+vaddr_inatomic = io_mapping_map_wc(mapping, page_base);
> >>+/* We can use the cpu mem copy function because this is X86. */
> >>+vaddr = (void __force *)vaddr_inatomic + page_offset;
> >>+if (pwrite)
> >>+unwritten = __copy_from_user(vaddr, user_data, length);
> >>+else
> >>+unwritten = __copy_to_user(user_data, vaddr, length);
> >>+
> >>+io_mapping_unmap(vaddr_inatomic);
> >>+return unwritten;
> >>+}
> >>+
> >>+static int
> >>+i915_gem_gtt_copy(struct drm_device *dev,
> >>+   struct drm_i915_gem_object *obj, uint64_t size,
> >>+   uint64_t data_offset, uint64_t data_ptr)
> >>+{
> >>+struct drm_i915_private *dev_priv = dev->dev_private;
> >>+char __user *user_data;
> >>+uint64_t remain;
> >>+uint64_t offset, page_base;
> >>+int page_offset, page_length, ret = 0;
> >>+
> >>+ret = i915_gem_obj_ggtt_pin(obj, 0, PIN_MAPPABLE);
> >>+if (ret)
> >>+goto out;
> >>+
> >>+ret = i915_gem_object_set_to_gtt_domain(obj, false);
> >>+if (ret)
> >>+goto out_unpin;
> >>+
> >>+ret = i915_gem_object_put_fence(obj);
> >>+if (ret)
> >>+goto out_unpin;
> >>+
> >>+user_data = to_user_ptr(data_ptr);
> >>+remain = size;
> >>+offset = i915_gem_obj_ggtt_offset(obj) + data_offset;
> >>+
> >>+mutex_unlock(&dev->struct_mutex);
> >>+if (likely(!i915.prefault_disable))
> >>+ret = fault_in_multipages_writeable(user_data, remain);
> >>+
> >>+/*
> >>+ * page_offset = offset within page
> >>+ * page_base = page offset within aperture
> >>+ */
> >>+page_offset = offset_in_page(offset);
> >>+page_base = offset & PAGE_MASK;
> >>+
> >>+while (remain > 0) {
> >>+/* page_length = bytes to copy for this page */
> >>+page_length = remain;
> >>+if ((page_offset + remain) > PAGE_SIZE)
> >>+page_length = PAGE_SIZE - page_offset;
> >>+
> >>+/* This is a slow read/write as it tries to read from
> >>+ * and write to user memory which may result into page
> >>+ * faults
> >>+ */
> >>+ret = slow_user_access(dev_priv->gtt.mappable, page_base,
> >>+   page_offset, user_data,
> >>+   page_length, false);
> >>+
> >>+if (ret) {
> >>+ret = -EFAULT;
> >>+break;
> >>+}
> >>+
> >>+remain -= page_length;
> >>+user_data += page_length;
> >>+page_base += page_length;
> >>+page_offset = 0;
> >>+}
> >>+
> >>+mutex_lock(&dev->struct_mutex);
> >>+
> >>+out_unpin:
> >>+i915_gem_object_ggtt_unpin(obj);
> >>+out:
> >>+return ret;
> >>+}
> >>+
> >>  static int
> >>  i915_gem_shmem_pread(struct drm_device *dev,
> >>   struct drm_i915

Re: [Intel-gfx] [PATCH 5/6] drm/i915: Support for pread/pwrite from/to non shmem backed objects

2015-12-10 Thread Ankitprasad Sharma
On Thu, 2015-12-10 at 18:18 +, Dave Gordon wrote:
> On 10/12/15 11:12, Ankitprasad Sharma wrote:
> > On Wed, 2015-12-09 at 19:39 +, Dave Gordon wrote:
> >> On 09/12/15 16:15, Tvrtko Ursulin wrote:
> >>>
> >>> Hi,
> >>>
> >>> On 09/12/15 12:46, ankitprasad.r.sha...@intel.com wrote:
>  From: Ankitprasad Sharma 
> 
>  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 
>  ---
> 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
>  b/drivers/gpu/drm/i915/i915_gem.c
>  index ed97de6..68ed67a 100644
>  --- a/drivers/gpu/drm/i915/i915_gem.c
>  +++ b/drivers/gpu/drm/i915/i915_gem.c
>  @@ -614,6 +614,99 @@ shmem_pread_slow(struct page *page, int
>  shmem_page_offset, int page_length,
> return ret ? - EFAULT : 0;
> }
> 
>  +static inline uint64_t
>  +slow_user_access(struct io_mapping *mapping,
>  + uint64_t page_base, int page_offset,
>  + char __user *user_data,
>  + int length, bool pwrite)
>  +{
>  +void __iomem *vaddr_inatomic;
>  +void *vaddr;
>  +uint64_t unwritten;
>  +
>  +vaddr_inatomic = io_mapping_map_wc(mapping, page_base);
>  +/* We can use the cpu mem copy function because this is X86. */
>  +vaddr = (void __force *)vaddr_inatomic + page_offset;
>  +if (pwrite)
>  +unwritten = __copy_from_user(vaddr, user_data, length);
>  +else
>  +unwritten = __copy_to_user(user_data, vaddr, length);
>  +
>  +io_mapping_unmap(vaddr_inatomic);
>  +return unwritten;
>  +}
>  +
>  +static int
>  +i915_gem_gtt_copy(struct drm_device *dev,
>  +   struct drm_i915_gem_object *obj, uint64_t size,
>  +   uint64_t data_offset, uint64_t data_ptr)
>  +{
>  +struct drm_i915_private *dev_priv = dev->dev_private;
>  +char __user *user_data;
>  +uint64_t remain;
>  +uint64_t offset, page_base;
>  +int page_offset, page_length, ret = 0;
>  +
>  +ret = i915_gem_obj_ggtt_pin(obj, 0, PIN_MAPPABLE);
>  +if (ret)
>  +goto out;
>  +
>  +ret = i915_gem_object_set_to_gtt_domain(obj, false);
>  +if (ret)
>  +goto out_unpin;
>  +
>  +ret = i915_gem_object_put_fence(obj);
>  +if (ret)
>  +goto out_unpin;
>  +
>  +user_data = to_user_ptr(data_ptr);
>  +remain = size;
>  +offset = i915_gem_obj_ggtt_offset(obj) + data_offset;
>  +
>  +mutex_unlock(&dev->struct_mutex);
>  +if (likely(!i915.prefault_disable))
>  +ret = fault_in_multipages_writeable(user_data, remain);
>  +
>  +/*
>  + * page_offset = offset within page
>  + * page_base = page offset within aperture
>  + */
>  +page_offset = offset_in_page(offset);
>  +page_base = offset & PAGE_MASK;
>  +
>  +while (remain > 0) {
>  +/* page_length = bytes to copy for this page */
>  +page_length = remain;
>  +if ((page_offset + remain) > PAGE_SIZE)
>  +page_length = PAGE_SIZE - page_offset;
>  +
>  +/* This is a slow read/write as it tries to read from
>  + * and write to user memory which may result into page
>  + * faults
>  + */
>  +ret = slow_user_access(dev_priv->gtt.mappable, page_base,
>  +   page_offset, user_data,
>  +   page_length, false);
>  +
>  +if (ret) {
>  +ret

Re: [Intel-gfx] [PATCH 5/6] drm/i915: Support for pread/pwrite from/to non shmem backed objects

2015-12-10 Thread Dave Gordon

On 10/12/15 11:12, Ankitprasad Sharma wrote:

On Wed, 2015-12-09 at 19:39 +, Dave Gordon wrote:

On 09/12/15 16:15, Tvrtko Ursulin wrote:


Hi,

On 09/12/15 12:46, ankitprasad.r.sha...@intel.com wrote:

From: Ankitprasad Sharma 

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 
---
   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
b/drivers/gpu/drm/i915/i915_gem.c
index ed97de6..68ed67a 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -614,6 +614,99 @@ shmem_pread_slow(struct page *page, int
shmem_page_offset, int page_length,
   return ret ? - EFAULT : 0;
   }

+static inline uint64_t
+slow_user_access(struct io_mapping *mapping,
+ uint64_t page_base, int page_offset,
+ char __user *user_data,
+ int length, bool pwrite)
+{
+void __iomem *vaddr_inatomic;
+void *vaddr;
+uint64_t unwritten;
+
+vaddr_inatomic = io_mapping_map_wc(mapping, page_base);
+/* We can use the cpu mem copy function because this is X86. */
+vaddr = (void __force *)vaddr_inatomic + page_offset;
+if (pwrite)
+unwritten = __copy_from_user(vaddr, user_data, length);
+else
+unwritten = __copy_to_user(user_data, vaddr, length);
+
+io_mapping_unmap(vaddr_inatomic);
+return unwritten;
+}
+
+static int
+i915_gem_gtt_copy(struct drm_device *dev,
+   struct drm_i915_gem_object *obj, uint64_t size,
+   uint64_t data_offset, uint64_t data_ptr)
+{
+struct drm_i915_private *dev_priv = dev->dev_private;
+char __user *user_data;
+uint64_t remain;
+uint64_t offset, page_base;
+int page_offset, page_length, ret = 0;
+
+ret = i915_gem_obj_ggtt_pin(obj, 0, PIN_MAPPABLE);
+if (ret)
+goto out;
+
+ret = i915_gem_object_set_to_gtt_domain(obj, false);
+if (ret)
+goto out_unpin;
+
+ret = i915_gem_object_put_fence(obj);
+if (ret)
+goto out_unpin;
+
+user_data = to_user_ptr(data_ptr);
+remain = size;
+offset = i915_gem_obj_ggtt_offset(obj) + data_offset;
+
+mutex_unlock(&dev->struct_mutex);
+if (likely(!i915.prefault_disable))
+ret = fault_in_multipages_writeable(user_data, remain);
+
+/*
+ * page_offset = offset within page
+ * page_base = page offset within aperture
+ */
+page_offset = offset_in_page(offset);
+page_base = offset & PAGE_MASK;
+
+while (remain > 0) {
+/* page_length = bytes to copy for this page */
+page_length = remain;
+if ((page_offset + remain) > PAGE_SIZE)
+page_length = PAGE_SIZE - page_offset;
+
+/* This is a slow read/write as it tries to read from
+ * and write to user memory which may result into page
+ * faults
+ */
+ret = slow_user_access(dev_priv->gtt.mappable, page_base,
+   page_offset, user_data,
+   page_length, false);
+
+if (ret) {
+ret = -EFAULT;
+break;
+}
+
+remain -= page_length;
+user_data += page_length;
+page_base += page_length;
+page_offset = 0;
+}
+
+mutex_lock(&dev->struct_mutex);
+
+out_unpin:
+i915_gem_object_ggtt_unpin(obj);
+out:
+return ret;
+}
+
   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_NON

Re: [Intel-gfx] [PATCH 5/6] drm/i915: Support for pread/pwrite from/to non shmem backed objects

2015-12-10 Thread Ankitprasad Sharma
On Wed, 2015-12-09 at 19:39 +, Dave Gordon wrote:
> On 09/12/15 16:15, Tvrtko Ursulin wrote:
> >
> > Hi,
> >
> > On 09/12/15 12:46, ankitprasad.r.sha...@intel.com wrote:
> >> From: Ankitprasad Sharma 
> >>
> >> 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 
> >> ---
> >>   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
> >> b/drivers/gpu/drm/i915/i915_gem.c
> >> index ed97de6..68ed67a 100644
> >> --- a/drivers/gpu/drm/i915/i915_gem.c
> >> +++ b/drivers/gpu/drm/i915/i915_gem.c
> >> @@ -614,6 +614,99 @@ shmem_pread_slow(struct page *page, int
> >> shmem_page_offset, int page_length,
> >>   return ret ? - EFAULT : 0;
> >>   }
> >>
> >> +static inline uint64_t
> >> +slow_user_access(struct io_mapping *mapping,
> >> + uint64_t page_base, int page_offset,
> >> + char __user *user_data,
> >> + int length, bool pwrite)
> >> +{
> >> +void __iomem *vaddr_inatomic;
> >> +void *vaddr;
> >> +uint64_t unwritten;
> >> +
> >> +vaddr_inatomic = io_mapping_map_wc(mapping, page_base);
> >> +/* We can use the cpu mem copy function because this is X86. */
> >> +vaddr = (void __force *)vaddr_inatomic + page_offset;
> >> +if (pwrite)
> >> +unwritten = __copy_from_user(vaddr, user_data, length);
> >> +else
> >> +unwritten = __copy_to_user(user_data, vaddr, length);
> >> +
> >> +io_mapping_unmap(vaddr_inatomic);
> >> +return unwritten;
> >> +}
> >> +
> >> +static int
> >> +i915_gem_gtt_copy(struct drm_device *dev,
> >> +   struct drm_i915_gem_object *obj, uint64_t size,
> >> +   uint64_t data_offset, uint64_t data_ptr)
> >> +{
> >> +struct drm_i915_private *dev_priv = dev->dev_private;
> >> +char __user *user_data;
> >> +uint64_t remain;
> >> +uint64_t offset, page_base;
> >> +int page_offset, page_length, ret = 0;
> >> +
> >> +ret = i915_gem_obj_ggtt_pin(obj, 0, PIN_MAPPABLE);
> >> +if (ret)
> >> +goto out;
> >> +
> >> +ret = i915_gem_object_set_to_gtt_domain(obj, false);
> >> +if (ret)
> >> +goto out_unpin;
> >> +
> >> +ret = i915_gem_object_put_fence(obj);
> >> +if (ret)
> >> +goto out_unpin;
> >> +
> >> +user_data = to_user_ptr(data_ptr);
> >> +remain = size;
> >> +offset = i915_gem_obj_ggtt_offset(obj) + data_offset;
> >> +
> >> +mutex_unlock(&dev->struct_mutex);
> >> +if (likely(!i915.prefault_disable))
> >> +ret = fault_in_multipages_writeable(user_data, remain);
> >> +
> >> +/*
> >> + * page_offset = offset within page
> >> + * page_base = page offset within aperture
> >> + */
> >> +page_offset = offset_in_page(offset);
> >> +page_base = offset & PAGE_MASK;
> >> +
> >> +while (remain > 0) {
> >> +/* page_length = bytes to copy for this page */
> >> +page_length = remain;
> >> +if ((page_offset + remain) > PAGE_SIZE)
> >> +page_length = PAGE_SIZE - page_offset;
> >> +
> >> +/* This is a slow read/write as it tries to read from
> >> + * and write to user memory which may result into page
> >> + * faults
> >> + */
> >> +ret = slow_user_access(dev_priv->gtt.mappable, page_base,
> >> +   page_offset, user_data,
> >> +   page_length, false);
> >> +
> >> +if (ret) {
> >> +ret = -EFAULT;
> >> +break;
> >> +}
> >> +
> >> +remain -= page_length;
> >> +user_data += page_length;
> >> +page_base += page_length;
> >> +page_offset = 0;
> >> +}
> >> +
> >> +mutex_lock(&dev->struct_mutex);
> >> +
> >> +out_unpin:
> >> +i915_gem_object_ggtt_unpin(obj);
> >> +out:
> >> +return 

Re: [Intel-gfx] [PATCH 5/6] drm/i915: Support for pread/pwrite from/to non shmem backed objects

2015-12-10 Thread Ankitprasad Sharma
On Thu, 2015-12-10 at 16:24 +0530, Ankitprasad Sharma wrote:
Missed Chris in last mail, adding him
On Wed, 2015-12-09 at 16:15 +, Tvrtko Ursulin wrote:
> Hi,
> 
> On 09/12/15 12:46, ankitprasad.r.sha...@intel.com wrote:
> > From: Ankitprasad Sharma 
> >
> > 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 
> > ---
> >   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 
> > b/drivers/gpu/drm/i915/i915_gem.c
> > index ed97de6..68ed67a 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -614,6 +614,99 @@ shmem_pread_slow(struct page *page, int 
> > shmem_page_offset, int page_length,
> > return ret ? - EFAULT : 0;
> >   }
> >
> > +static inline uint64_t
> > +slow_user_access(struct io_mapping *mapping,
> > +uint64_t page_base, int page_offset,
> > +char __user *user_data,
> > +int length, bool pwrite)
> > +{
> > +   void __iomem *vaddr_inatomic;
> > +   void *vaddr;
> > +   uint64_t unwritten;
> > +
> > +   vaddr_inatomic = io_mapping_map_wc(mapping, page_base);
> > +   /* We can use the cpu mem copy function because this is X86. */
> > +   vaddr = (void __force *)vaddr_inatomic + page_offset;
> > +   if (pwrite)
> > +   unwritten = __copy_from_user(vaddr, user_data, length);
> > +   else
> > +   unwritten = __copy_to_user(user_data, vaddr, length);
> > +
> > +   io_mapping_unmap(vaddr_inatomic);
> > +   return unwritten;
> > +}
> > +
> > +static int
> > +i915_gem_gtt_copy(struct drm_device *dev,
> > +  struct drm_i915_gem_object *obj, uint64_t size,
> > +  uint64_t data_offset, uint64_t data_ptr)
> > +{
> > +   struct drm_i915_private *dev_priv = dev->dev_private;
> > +   char __user *user_data;
> > +   uint64_t remain;
> > +   uint64_t offset, page_base;
> > +   int page_offset, page_length, ret = 0;
> > +
> > +   ret = i915_gem_obj_ggtt_pin(obj, 0, PIN_MAPPABLE);
> > +   if (ret)
> > +   goto out;
> > +
> > +   ret = i915_gem_object_set_to_gtt_domain(obj, false);
> > +   if (ret)
> > +   goto out_unpin;
> > +
> > +   ret = i915_gem_object_put_fence(obj);
> > +   if (ret)
> > +   goto out_unpin;
> > +
> > +   user_data = to_user_ptr(data_ptr);
> > +   remain = size;
> > +   offset = i915_gem_obj_ggtt_offset(obj) + data_offset;
> > +
> > +   mutex_unlock(&dev->struct_mutex);
> > +   if (likely(!i915.prefault_disable))
> > +   ret = fault_in_multipages_writeable(user_data, remain);
> > +
> > +   /*
> > +* page_offset = offset within page
> > +* page_base = page offset within aperture
> > +*/
> > +   page_offset = offset_in_page(offset);
> > +   page_base = offset & PAGE_MASK;
> > +
> > +   while (remain > 0) {
> > +   /* page_length = bytes to copy for this page */
> > +   page_length = remain;
> > +   if ((page_offset + remain) > PAGE_SIZE)
> > +   page_length = PAGE_SIZE - page_offset;
> > +
> > +   /* This is a slow read/write as it tries to read from
> > +* and write to user memory which may result into page
> > +* faults
> > +*/
> > +   ret = slow_user_access(dev_priv->gtt.mappable, page_base,
> > +  page_offset, user_data,
> > +  page_length, false);
> > +
> > +   if (ret) {
> > +   ret = -EFAULT;
> > +   break;
> > +   }
> > +
> > +   remain -= page_length;
> > +   user_data += page_length;
> > +   page_base += page_length;
> > +   page_offset = 0;
> > +   }
> > +
> > +   mutex_lock(&dev->struct_mutex);
> > +
> > +out_unpin:
> > +   i915_gem_object_ggtt_unpin(obj);
> > +out:
> > +   return ret;
> > 

Re: [Intel-gfx] [PATCH 5/6] drm/i915: Support for pread/pwrite from/to non shmem backed objects

2015-12-10 Thread Ankitprasad Sharma
On Wed, 2015-12-09 at 16:15 +, Tvrtko Ursulin wrote:
> Hi,
> 
> On 09/12/15 12:46, ankitprasad.r.sha...@intel.com wrote:
> > From: Ankitprasad Sharma 
> >
> > 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 
> > ---
> >   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 
> > b/drivers/gpu/drm/i915/i915_gem.c
> > index ed97de6..68ed67a 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -614,6 +614,99 @@ shmem_pread_slow(struct page *page, int 
> > shmem_page_offset, int page_length,
> > return ret ? - EFAULT : 0;
> >   }
> >
> > +static inline uint64_t
> > +slow_user_access(struct io_mapping *mapping,
> > +uint64_t page_base, int page_offset,
> > +char __user *user_data,
> > +int length, bool pwrite)
> > +{
> > +   void __iomem *vaddr_inatomic;
> > +   void *vaddr;
> > +   uint64_t unwritten;
> > +
> > +   vaddr_inatomic = io_mapping_map_wc(mapping, page_base);
> > +   /* We can use the cpu mem copy function because this is X86. */
> > +   vaddr = (void __force *)vaddr_inatomic + page_offset;
> > +   if (pwrite)
> > +   unwritten = __copy_from_user(vaddr, user_data, length);
> > +   else
> > +   unwritten = __copy_to_user(user_data, vaddr, length);
> > +
> > +   io_mapping_unmap(vaddr_inatomic);
> > +   return unwritten;
> > +}
> > +
> > +static int
> > +i915_gem_gtt_copy(struct drm_device *dev,
> > +  struct drm_i915_gem_object *obj, uint64_t size,
> > +  uint64_t data_offset, uint64_t data_ptr)
> > +{
> > +   struct drm_i915_private *dev_priv = dev->dev_private;
> > +   char __user *user_data;
> > +   uint64_t remain;
> > +   uint64_t offset, page_base;
> > +   int page_offset, page_length, ret = 0;
> > +
> > +   ret = i915_gem_obj_ggtt_pin(obj, 0, PIN_MAPPABLE);
> > +   if (ret)
> > +   goto out;
> > +
> > +   ret = i915_gem_object_set_to_gtt_domain(obj, false);
> > +   if (ret)
> > +   goto out_unpin;
> > +
> > +   ret = i915_gem_object_put_fence(obj);
> > +   if (ret)
> > +   goto out_unpin;
> > +
> > +   user_data = to_user_ptr(data_ptr);
> > +   remain = size;
> > +   offset = i915_gem_obj_ggtt_offset(obj) + data_offset;
> > +
> > +   mutex_unlock(&dev->struct_mutex);
> > +   if (likely(!i915.prefault_disable))
> > +   ret = fault_in_multipages_writeable(user_data, remain);
> > +
> > +   /*
> > +* page_offset = offset within page
> > +* page_base = page offset within aperture
> > +*/
> > +   page_offset = offset_in_page(offset);
> > +   page_base = offset & PAGE_MASK;
> > +
> > +   while (remain > 0) {
> > +   /* page_length = bytes to copy for this page */
> > +   page_length = remain;
> > +   if ((page_offset + remain) > PAGE_SIZE)
> > +   page_length = PAGE_SIZE - page_offset;
> > +
> > +   /* This is a slow read/write as it tries to read from
> > +* and write to user memory which may result into page
> > +* faults
> > +*/
> > +   ret = slow_user_access(dev_priv->gtt.mappable, page_base,
> > +  page_offset, user_data,
> > +  page_length, false);
> > +
> > +   if (ret) {
> > +   ret = -EFAULT;
> > +   break;
> > +   }
> > +
> > +   remain -= page_length;
> > +   user_data += page_length;
> > +   page_base += page_length;
> > +   page_offset = 0;
> > +   }
> > +
> > +   mutex_lock(&dev->struct_mutex);
> > +
> > +out_unpin:
> > +   i915_gem_object_ggtt_unpin(obj);
> > +out:
> > +   return ret;
> > +}
> > +
> >   static int
> >   i915_gem_shmem_pread(struct drm_device *dev,
> >  s

Re: [Intel-gfx] [PATCH 5/6] drm/i915: Support for pread/pwrite from/to non shmem backed objects

2015-12-09 Thread Dave Gordon

On 09/12/15 16:15, Tvrtko Ursulin wrote:


Hi,

On 09/12/15 12:46, ankitprasad.r.sha...@intel.com wrote:

From: Ankitprasad Sharma 

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 
---
  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
b/drivers/gpu/drm/i915/i915_gem.c
index ed97de6..68ed67a 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -614,6 +614,99 @@ shmem_pread_slow(struct page *page, int
shmem_page_offset, int page_length,
  return ret ? - EFAULT : 0;
  }

+static inline uint64_t
+slow_user_access(struct io_mapping *mapping,
+ uint64_t page_base, int page_offset,
+ char __user *user_data,
+ int length, bool pwrite)
+{
+void __iomem *vaddr_inatomic;
+void *vaddr;
+uint64_t unwritten;
+
+vaddr_inatomic = io_mapping_map_wc(mapping, page_base);
+/* We can use the cpu mem copy function because this is X86. */
+vaddr = (void __force *)vaddr_inatomic + page_offset;
+if (pwrite)
+unwritten = __copy_from_user(vaddr, user_data, length);
+else
+unwritten = __copy_to_user(user_data, vaddr, length);
+
+io_mapping_unmap(vaddr_inatomic);
+return unwritten;
+}
+
+static int
+i915_gem_gtt_copy(struct drm_device *dev,
+   struct drm_i915_gem_object *obj, uint64_t size,
+   uint64_t data_offset, uint64_t data_ptr)
+{
+struct drm_i915_private *dev_priv = dev->dev_private;
+char __user *user_data;
+uint64_t remain;
+uint64_t offset, page_base;
+int page_offset, page_length, ret = 0;
+
+ret = i915_gem_obj_ggtt_pin(obj, 0, PIN_MAPPABLE);
+if (ret)
+goto out;
+
+ret = i915_gem_object_set_to_gtt_domain(obj, false);
+if (ret)
+goto out_unpin;
+
+ret = i915_gem_object_put_fence(obj);
+if (ret)
+goto out_unpin;
+
+user_data = to_user_ptr(data_ptr);
+remain = size;
+offset = i915_gem_obj_ggtt_offset(obj) + data_offset;
+
+mutex_unlock(&dev->struct_mutex);
+if (likely(!i915.prefault_disable))
+ret = fault_in_multipages_writeable(user_data, remain);
+
+/*
+ * page_offset = offset within page
+ * page_base = page offset within aperture
+ */
+page_offset = offset_in_page(offset);
+page_base = offset & PAGE_MASK;
+
+while (remain > 0) {
+/* page_length = bytes to copy for this page */
+page_length = remain;
+if ((page_offset + remain) > PAGE_SIZE)
+page_length = PAGE_SIZE - page_offset;
+
+/* This is a slow read/write as it tries to read from
+ * and write to user memory which may result into page
+ * faults
+ */
+ret = slow_user_access(dev_priv->gtt.mappable, page_base,
+   page_offset, user_data,
+   page_length, false);
+
+if (ret) {
+ret = -EFAULT;
+break;
+}
+
+remain -= page_length;
+user_data += page_length;
+page_base += page_length;
+page_offset = 0;
+}
+
+mutex_lock(&dev->struct_mutex);
+
+out_unpin:
+i915_gem_object_ggtt_unpin(obj);
+out:
+return ret;
+}
+
  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);
+

Re: [Intel-gfx] [PATCH 5/6] drm/i915: Support for pread/pwrite from/to non shmem backed objects

2015-12-09 Thread Tvrtko Ursulin


Hi,

On 09/12/15 12:46, ankitprasad.r.sha...@intel.com wrote:

From: Ankitprasad Sharma 

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 
---
  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 b/drivers/gpu/drm/i915/i915_gem.c
index ed97de6..68ed67a 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -614,6 +614,99 @@ shmem_pread_slow(struct page *page, int shmem_page_offset, 
int page_length,
return ret ? - EFAULT : 0;
  }

+static inline uint64_t
+slow_user_access(struct io_mapping *mapping,
+uint64_t page_base, int page_offset,
+char __user *user_data,
+int length, bool pwrite)
+{
+   void __iomem *vaddr_inatomic;
+   void *vaddr;
+   uint64_t unwritten;
+
+   vaddr_inatomic = io_mapping_map_wc(mapping, page_base);
+   /* We can use the cpu mem copy function because this is X86. */
+   vaddr = (void __force *)vaddr_inatomic + page_offset;
+   if (pwrite)
+   unwritten = __copy_from_user(vaddr, user_data, length);
+   else
+   unwritten = __copy_to_user(user_data, vaddr, length);
+
+   io_mapping_unmap(vaddr_inatomic);
+   return unwritten;
+}
+
+static int
+i915_gem_gtt_copy(struct drm_device *dev,
+  struct drm_i915_gem_object *obj, uint64_t size,
+  uint64_t data_offset, uint64_t data_ptr)
+{
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   char __user *user_data;
+   uint64_t remain;
+   uint64_t offset, page_base;
+   int page_offset, page_length, ret = 0;
+
+   ret = i915_gem_obj_ggtt_pin(obj, 0, PIN_MAPPABLE);
+   if (ret)
+   goto out;
+
+   ret = i915_gem_object_set_to_gtt_domain(obj, false);
+   if (ret)
+   goto out_unpin;
+
+   ret = i915_gem_object_put_fence(obj);
+   if (ret)
+   goto out_unpin;
+
+   user_data = to_user_ptr(data_ptr);
+   remain = size;
+   offset = i915_gem_obj_ggtt_offset(obj) + data_offset;
+
+   mutex_unlock(&dev->struct_mutex);
+   if (likely(!i915.prefault_disable))
+   ret = fault_in_multipages_writeable(user_data, remain);
+
+   /*
+* page_offset = offset within page
+* page_base = page offset within aperture
+*/
+   page_offset = offset_in_page(offset);
+   page_base = offset & PAGE_MASK;
+
+   while (remain > 0) {
+   /* page_length = bytes to copy for this page */
+   page_length = remain;
+   if ((page_offset + remain) > PAGE_SIZE)
+   page_length = PAGE_SIZE - page_offset;
+
+   /* This is a slow read/write as it tries to read from
+* and write to user memory which may result into page
+* faults
+*/
+   ret = slow_user_access(dev_priv->gtt.mappable, page_base,
+  page_offset, user_data,
+  page_length, false);
+
+   if (ret) {
+   ret = -EFAULT;
+   break;
+   }
+
+   remain -= page_length;
+   user_data += page_length;
+   page_base += page_length;
+   page_offset = 0;
+   }
+
+   mutex_lock(&dev->struct_mutex);
+
+out_unpin:
+   i915_gem_object_ggtt_unpin(obj);
+out:
+   return ret;
+}
+
  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;
-

Re: [Intel-gfx] [PATCH 5/6] drm/i915: Support for pread/pwrite from/to non shmem backed objects

2015-11-20 Thread Ankitprasad Sharma
On Fri, 2015-11-13 at 17:23 +, Tvrtko Ursulin wrote:
> 
> On 11/11/15 10:36, ankitprasad.r.sha...@intel.com wrote:
> > From: Ankitprasad Sharma 
> >
> > 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 (Tvrtko)
> 
> Since v6 you have also renamed i915_gem_gtt_read to i915_gem_gtt_copy 
> and added the pwrite slow path so the commit should say that.
Yes, I need to update this.
> 
> >
> > Testcase: igt/gem_stolen
> >
> > Signed-off-by: Ankitprasad Sharma 
> > ---
> >   drivers/gpu/drm/i915/i915_gem.c | 146 
> > +---
> >   1 file changed, 122 insertions(+), 24 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c 
> > b/drivers/gpu/drm/i915/i915_gem.c
> > index 2d8c9e0..e0b9502 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -614,6 +614,99 @@ shmem_pread_slow(struct page *page, int 
> > shmem_page_offset, int page_length,
> > return ret ? - EFAULT : 0;
> >   }
> >
> > +static inline uint64_t
> > +slow_user_access(struct io_mapping *mapping,
> > +uint64_t page_base, int page_offset,
> > +char __user *user_data,
> > +int length, bool pwrite)
> > +{
> > +   void __iomem *vaddr_inatomic;
> > +   void *vaddr;
> > +   uint64_t unwritten;
> > +
> > +   vaddr_inatomic = io_mapping_map_wc(mapping, page_base);
> > +   /* We can use the cpu mem copy function because this is X86. */
> > +   vaddr = (void __force *)vaddr_inatomic + page_offset;
> > +   if (pwrite)
> > +   unwritten = __copy_from_user(vaddr, user_data, length);
> > +   else
> > +   unwritten = __copy_to_user(user_data, vaddr, length);
> > +
> > +   io_mapping_unmap(vaddr_inatomic);
> > +   return unwritten;
> > +}
> > +
> > +static int
> > +i915_gem_gtt_copy(struct drm_device *dev,
> > +  struct drm_i915_gem_object *obj, uint64_t size,
> > +  uint64_t data_offset, uint64_t data_ptr)
> > +{
> > +   struct drm_i915_private *dev_priv = dev->dev_private;
> > +   char __user *user_data;
> > +   uint64_t remain;
> > +   uint64_t offset, page_base;
> > +   int page_offset, page_length, ret = 0;
> > +
> > +   ret = i915_gem_obj_ggtt_pin(obj, 0, PIN_MAPPABLE);
> > +   if (ret)
> > +   goto out;
> > +
> > +   ret = i915_gem_object_set_to_gtt_domain(obj, false);
> > +   if (ret)
> > +   goto out_unpin;
> > +
> > +   ret = i915_gem_object_put_fence(obj);
> > +   if (ret)
> > +   goto out_unpin;
> > +
> > +   user_data = to_user_ptr(data_ptr);
> > +   remain = size;
> > +   offset = i915_gem_obj_ggtt_offset(obj) + data_offset;
> > +
> > +   mutex_unlock(&dev->struct_mutex);
> > +   if (likely(!i915.prefault_disable))
> > +   ret = fault_in_multipages_writeable(user_data, remain);
> > +
> > +   /*
> > +* page_offset = offset within page
> > +* page_base = page offset within aperture
> > +*/
> > +   page_offset = offset_in_page(offset);
> > +   page_base = offset & PAGE_MASK;
> > +
> > +   while (remain > 0) {
> > +   /* page_length = bytes to copy for this page */
> > +   page_length = remain;
> > +   if ((page_offset + remain) > PAGE_SIZE)
> > +   page_length = PAGE_SIZE - page_offset;
> > +
> > +   /* This is a slow read/write as it tries to read from
> > +* and write to user memory which may result into page
> > +* faults
> > +*/
> > +   ret = slow_user_access(dev_priv->gtt.mappable, page_base,
> > +  page_offset, user_data,
> > +  page_length, false);
> > +
> > +   if (ret) {
> > +   ret = -EFAULT;
> > +   break;
> > +   }
> > +
> > +   remain -= page_length;
> > +   user_data += page_length;
> > +   page_base += page_length;
> > +   page_offset = 0;
> > +   }
> > +
> > +   mutex_lock(&dev->struct_mutex);
> > +
> > +out_unpin:
> > +   i915_gem_object_ggtt_unpin(obj);
> > +out:
> > +   return ret;
> > +}
> > +
> >   static int
> >   i915_gem_shmem_pread(struct drm_device *dev,
> >  struct drm_i915_gem_object *obj,
> > @@ -737,17 +830,14 @@ i915_g

Re: [Intel-gfx] [PATCH 5/6] drm/i915: Support for pread/pwrite from/to non shmem backed objects

2015-11-13 Thread Tvrtko Ursulin



On 11/11/15 10:36, ankitprasad.r.sha...@intel.com wrote:

From: Ankitprasad Sharma 

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 (Tvrtko)


Since v6 you have also renamed i915_gem_gtt_read to i915_gem_gtt_copy 
and added the pwrite slow path so the commit should say that.




Testcase: igt/gem_stolen

Signed-off-by: Ankitprasad Sharma 
---
  drivers/gpu/drm/i915/i915_gem.c | 146 +---
  1 file changed, 122 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 2d8c9e0..e0b9502 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -614,6 +614,99 @@ shmem_pread_slow(struct page *page, int shmem_page_offset, 
int page_length,
return ret ? - EFAULT : 0;
  }

+static inline uint64_t
+slow_user_access(struct io_mapping *mapping,
+uint64_t page_base, int page_offset,
+char __user *user_data,
+int length, bool pwrite)
+{
+   void __iomem *vaddr_inatomic;
+   void *vaddr;
+   uint64_t unwritten;
+
+   vaddr_inatomic = io_mapping_map_wc(mapping, page_base);
+   /* We can use the cpu mem copy function because this is X86. */
+   vaddr = (void __force *)vaddr_inatomic + page_offset;
+   if (pwrite)
+   unwritten = __copy_from_user(vaddr, user_data, length);
+   else
+   unwritten = __copy_to_user(user_data, vaddr, length);
+
+   io_mapping_unmap(vaddr_inatomic);
+   return unwritten;
+}
+
+static int
+i915_gem_gtt_copy(struct drm_device *dev,
+  struct drm_i915_gem_object *obj, uint64_t size,
+  uint64_t data_offset, uint64_t data_ptr)
+{
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   char __user *user_data;
+   uint64_t remain;
+   uint64_t offset, page_base;
+   int page_offset, page_length, ret = 0;
+
+   ret = i915_gem_obj_ggtt_pin(obj, 0, PIN_MAPPABLE);
+   if (ret)
+   goto out;
+
+   ret = i915_gem_object_set_to_gtt_domain(obj, false);
+   if (ret)
+   goto out_unpin;
+
+   ret = i915_gem_object_put_fence(obj);
+   if (ret)
+   goto out_unpin;
+
+   user_data = to_user_ptr(data_ptr);
+   remain = size;
+   offset = i915_gem_obj_ggtt_offset(obj) + data_offset;
+
+   mutex_unlock(&dev->struct_mutex);
+   if (likely(!i915.prefault_disable))
+   ret = fault_in_multipages_writeable(user_data, remain);
+
+   /*
+* page_offset = offset within page
+* page_base = page offset within aperture
+*/
+   page_offset = offset_in_page(offset);
+   page_base = offset & PAGE_MASK;
+
+   while (remain > 0) {
+   /* page_length = bytes to copy for this page */
+   page_length = remain;
+   if ((page_offset + remain) > PAGE_SIZE)
+   page_length = PAGE_SIZE - page_offset;
+
+   /* This is a slow read/write as it tries to read from
+* and write to user memory which may result into page
+* faults
+*/
+   ret = slow_user_access(dev_priv->gtt.mappable, page_base,
+  page_offset, user_data,
+  page_length, false);
+
+   if (ret) {
+   ret = -EFAULT;
+   break;
+   }
+
+   remain -= page_length;
+   user_data += page_length;
+   page_base += page_length;
+   page_offset = 0;
+   }
+
+   mutex_lock(&dev->struct_mutex);
+
+out_unpin:
+   i915_gem_object_ggtt_unpin(obj);
+out:
+   return ret;
+}
+
  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);


Re: [Intel-gfx] [PATCH 5/6] drm/i915: Support for pread/pwrite from/to non shmem backed objects

2015-10-28 Thread Ankitprasad Sharma
On Thu, 2015-10-08 at 14:56 +0100, Tvrtko Ursulin wrote:
> Hi,
> 
> On 08/10/15 07:24, ankitprasad.r.sha...@intel.com wrote:
> > From: Ankitprasad Sharma 
> >
> > 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 prime objects as well as stolen memory backed objects
> > but for userptr objects it is still forbidden.
> 
> Where is the part which forbids it for userptr objects?
In version 5, updated the patch handle pwrite/pread for all non-shmem
backed objects, including userptr objects

Will update the Commit message
> 
> > 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)
> >
> > Testcase: igt/gem_stolen
> >
> > Signed-off-by: Ankitprasad Sharma 
> > ---
> >   drivers/gpu/drm/i915/i915_gem.c | 125 
> > +---
> >   1 file changed, 104 insertions(+), 21 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c 
> > b/drivers/gpu/drm/i915/i915_gem.c
> > index 91a2e97..2c94e22 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -614,6 +614,99 @@ shmem_pread_slow(struct page *page, int 
> > shmem_page_offset, int page_length,
> > return ret ? - EFAULT : 0;
> >   }
> >
> > +static inline uint64_t
> > +slow_user_access(struct io_mapping *mapping,
> > +uint64_t page_base, int page_offset,
> > +char __user *user_data,
> > +int length, bool pwrite)
> > +{
> > +   void __iomem *vaddr_inatomic;
> > +   void *vaddr;
> > +   uint64_t unwritten;
> > +
> > +   vaddr_inatomic = io_mapping_map_wc(mapping, page_base);
> > +   /* We can use the cpu mem copy function because this is X86. */
> > +   vaddr = (void __force *)vaddr_inatomic + page_offset;
> > +   if (pwrite)
> > +   unwritten = __copy_from_user(vaddr, user_data, length);
> > +   else
> > +   unwritten = __copy_to_user(user_data, vaddr, length);
> > +
> > +   io_mapping_unmap(vaddr_inatomic);
> > +   return unwritten;
> > +}
> > +
> > +static int
> > +i915_gem_gtt_pread(struct drm_device *dev,
> > +  struct drm_i915_gem_object *obj, uint64_t size,
> > +  uint64_t data_offset, uint64_t data_ptr)
> > +{
> > +   struct drm_i915_private *dev_priv = dev->dev_private;
> > +   char __user *user_data;
> > +   uint64_t remain;
> > +   uint64_t offset, page_base;
> > +   int page_offset, page_length, ret = 0;
> > +
> > +   ret = i915_gem_obj_ggtt_pin(obj, 0, PIN_MAPPABLE);
> > +   if (ret)
> > +   goto out;
> > +
> > +   ret = i915_gem_object_set_to_gtt_domain(obj, false);
> > +   if (ret)
> > +   goto out_unpin;
> > +
> > +   ret = i915_gem_object_put_fence(obj);
> > +   if (ret)
> > +   goto out_unpin;
> > +
> > +   user_data = to_user_ptr(data_ptr);
> > +   remain = size;
> > +   offset = i915_gem_obj_ggtt_offset(obj) + data_offset;
> > +
> > +   mutex_unlock(&dev->struct_mutex);
> > +   if (likely(!i915.prefault_disable))
> > +   ret = fault_in_multipages_writeable(user_data, remain);
> > +
> > +   /*
> > +* page_offset = offset within page
> > +* page_base = page offset within aperture
> > +*/
> > +   page_offset = offset_in_page(offset);
> > +   page_base = offset & PAGE_MASK;
> > +
> > +   while (remain > 0) {
> > +   /* page_length = bytes to copy for this page */
> > +   page_length = remain;
> > +   if ((page_offset + remain) > PAGE_SIZE)
> > +   page_length = PAGE_SIZE - page_offset;
> > +
> > +   /* This is a slow read/write as it tries to read from
> > +* and write to user memory which may result into page
> > +* faults
> > +*/
> > +   ret = slow_user_access(dev_priv->gtt.mappable, page_base,
> > +  page_offset, user_data,
> > +  page_length, false);
> > +
> > +   if (ret) {
> > +   ret = -EFAULT;
> > +   break;
> > +   }
> > +
> > +   remain -= page_length;
> > +   user_data += page_length;
> > +   page_base += page_length;
> > +   page_offset = 0;
> > +   }
> > +
> > +   mutex_lock(&dev->struct_mutex);
> > +
> > +out_unpin:
> > +   i915_gem_object_ggtt_unpin(obj);
> > +out:
> > +   return ret;
> > +}
> > +
> >   static int
> >   i915_gem_shmem_pread(struct drm_device *dev,
> >  struct drm_i915_gem_object *obj,
> > @@

Re: [Intel-gfx] [PATCH 5/6] drm/i915: Support for pread/pwrite from/to non shmem backed objects

2015-10-08 Thread Tvrtko Ursulin


Hi,

On 08/10/15 07:24, ankitprasad.r.sha...@intel.com wrote:

From: Ankitprasad Sharma 

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 prime objects as well as stolen memory backed objects
but for userptr objects it is still forbidden.


Where is the part which forbids it for userptr 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)

Testcase: igt/gem_stolen

Signed-off-by: Ankitprasad Sharma 
---
  drivers/gpu/drm/i915/i915_gem.c | 125 +---
  1 file changed, 104 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 91a2e97..2c94e22 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -614,6 +614,99 @@ shmem_pread_slow(struct page *page, int shmem_page_offset, 
int page_length,
return ret ? - EFAULT : 0;
  }

+static inline uint64_t
+slow_user_access(struct io_mapping *mapping,
+uint64_t page_base, int page_offset,
+char __user *user_data,
+int length, bool pwrite)
+{
+   void __iomem *vaddr_inatomic;
+   void *vaddr;
+   uint64_t unwritten;
+
+   vaddr_inatomic = io_mapping_map_wc(mapping, page_base);
+   /* We can use the cpu mem copy function because this is X86. */
+   vaddr = (void __force *)vaddr_inatomic + page_offset;
+   if (pwrite)
+   unwritten = __copy_from_user(vaddr, user_data, length);
+   else
+   unwritten = __copy_to_user(user_data, vaddr, length);
+
+   io_mapping_unmap(vaddr_inatomic);
+   return unwritten;
+}
+
+static int
+i915_gem_gtt_pread(struct drm_device *dev,
+  struct drm_i915_gem_object *obj, uint64_t size,
+  uint64_t data_offset, uint64_t data_ptr)
+{
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   char __user *user_data;
+   uint64_t remain;
+   uint64_t offset, page_base;
+   int page_offset, page_length, ret = 0;
+
+   ret = i915_gem_obj_ggtt_pin(obj, 0, PIN_MAPPABLE);
+   if (ret)
+   goto out;
+
+   ret = i915_gem_object_set_to_gtt_domain(obj, false);
+   if (ret)
+   goto out_unpin;
+
+   ret = i915_gem_object_put_fence(obj);
+   if (ret)
+   goto out_unpin;
+
+   user_data = to_user_ptr(data_ptr);
+   remain = size;
+   offset = i915_gem_obj_ggtt_offset(obj) + data_offset;
+
+   mutex_unlock(&dev->struct_mutex);
+   if (likely(!i915.prefault_disable))
+   ret = fault_in_multipages_writeable(user_data, remain);
+
+   /*
+* page_offset = offset within page
+* page_base = page offset within aperture
+*/
+   page_offset = offset_in_page(offset);
+   page_base = offset & PAGE_MASK;
+
+   while (remain > 0) {
+   /* page_length = bytes to copy for this page */
+   page_length = remain;
+   if ((page_offset + remain) > PAGE_SIZE)
+   page_length = PAGE_SIZE - page_offset;
+
+   /* This is a slow read/write as it tries to read from
+* and write to user memory which may result into page
+* faults
+*/
+   ret = slow_user_access(dev_priv->gtt.mappable, page_base,
+  page_offset, user_data,
+  page_length, false);
+
+   if (ret) {
+   ret = -EFAULT;
+   break;
+   }
+
+   remain -= page_length;
+   user_data += page_length;
+   page_base += page_length;
+   page_offset = 0;
+   }
+
+   mutex_lock(&dev->struct_mutex);
+
+out_unpin:
+   i915_gem_object_ggtt_unpin(obj);
+out:
+   return ret;
+}
+
  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 sh