Hi,

On 07/22/2015 02:51 PM, ankitprasad.r.sha...@intel.com wrote:
From: Ankitprasad Sharma <ankitprasad.r.sha...@intel.com>

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.

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)

Testcase: igt/gem_stolen

Signed-off-by: Ankitprasad Sharma <ankitprasad.r.sha...@intel.com>
---
  drivers/gpu/drm/i915/i915_gem.c | 138 +++++++++++++++++++++++++++++++++++-----
  1 file changed, 121 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 9e7e182..4321178 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -631,6 +631,102 @@ 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_pwrite(struct drm_device *dev,
+                         struct drm_i915_gem_object *obj, uint64_t size,
+                         uint64_t data_offset, uint64_t data_ptr, bool pwrite)
+{
+       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, pwrite);
+       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;
+
+       if (pwrite)
+               intel_fb_obj_invalidate(obj, ORIGIN_GTT);
+
+       mutex_unlock(&dev->struct_mutex);
+       if (!pwrite && 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, pwrite);
+
+               if (ret) {
+                       ret = -EFAULT;
+                       break;
+               }
+
+               remain -= page_length;
+               user_data += page_length;
+               page_base += page_length;
+               page_offset = 0;
+       }
+
+       mutex_lock(&dev->struct_mutex);

My objection here was that we are re-acquiring the mutex in non-interruptible mode, while the caller had it interruptible. It am not 100% what the conclusion back then was?

But also, why it is even necessary to drop the mutex here?

Regards,

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

Reply via email to