[PATCH 1/5] drm/i915: Add support for mapping an object page by page

[PATCH 1/5] drm/i915: Add support for mapping an object page by page

[ankitprasad.r.sharma]

From: Chris Wilson <chris@chris-wilson.co.uk>

Introduced a new vm specfic callback insert_page() to program a single pte in
ggtt or ppgtt. This allows us to map a single page in to the mappable aperture
space. This can be iterated over to access the whole object by using space as
meagre as page size.

v2: Added low level rpm assertions to insert_page routines (Chris)

v3: Added POSTING_READ post register write (Tvrtko)

v4: Rebase (Ankit)

v5: Removed wmb() and FLUSH_CTL from insert_page, caller to take care
of it (Chris)

v6: insert_page not working correctly without FLSH_CNTL write, added the
write again.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/char/agp/intel-gtt.c        |  8 +++++
 drivers/gpu/drm/i915/i915_gem_gtt.c | 66 ++++++++++++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/i915_gem_gtt.h |  5 +++
 include/drm/intel-gtt.h             |  3 ++
 4 files changed, 81 insertions(+), 1 deletion(-)

diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c
index aef87fd..4431129 100644
--- a/drivers/char/agp/intel-gtt.c
+++ b/drivers/char/agp/intel-gtt.c
@@ -840,6 +840,14 @@ static bool i830_check_flags(unsigned int flags)
 	return false;
 }
 
+void intel_gtt_insert_page(dma_addr_t addr,
+			   unsigned int pg,
+			   unsigned int flags)
+{
+	intel_private.driver->write_entry(addr, pg, flags);
+}
+EXPORT_SYMBOL(intel_gtt_insert_page);
+
 void intel_gtt_insert_sg_entries(struct sg_table *st,
 				 unsigned int pg_start,
 				 unsigned int flags)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 4668477..7a139a6 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2355,6 +2355,28 @@ static void gen8_set_pte(void __iomem *addr, gen8_pte_t pte)
 #endif
 }
 
+static void gen8_ggtt_insert_page(struct i915_address_space *vm,
+				  dma_addr_t addr,
+				  uint64_t offset,
+				  enum i915_cache_level level,
+				  u32 unused)
+{
+	struct drm_i915_private *dev_priv = to_i915(vm->dev);
+	gen8_pte_t __iomem *pte =
+		(gen8_pte_t __iomem *)dev_priv->ggtt.gsm +
+		(offset >> PAGE_SHIFT);
+	int rpm_atomic_seq;
+
+	rpm_atomic_seq = assert_rpm_atomic_begin(dev_priv);
+
+	gen8_set_pte(pte, gen8_pte_encode(addr, level, true));
+
+	I915_WRITE(GFX_FLSH_CNTL_GEN6, GFX_FLSH_CNTL_EN);
+	POSTING_READ(GFX_FLSH_CNTL_GEN6);
+
+	assert_rpm_atomic_end(dev_priv, rpm_atomic_seq);
+}
+
 static void gen8_ggtt_insert_entries(struct i915_address_space *vm,
 				     struct sg_table *st,
 				     uint64_t start,
@@ -2424,6 +2446,28 @@ static void gen8_ggtt_insert_entries__BKL(struct i915_address_space *vm,
 	stop_machine(gen8_ggtt_insert_entries__cb, &arg, NULL);
 }
 
+static void gen6_ggtt_insert_page(struct i915_address_space *vm,
+				  dma_addr_t addr,
+				  uint64_t offset,
+				  enum i915_cache_level level,
+				  u32 flags)
+{
+	struct drm_i915_private *dev_priv = to_i915(vm->dev);
+	gen6_pte_t __iomem *pte =
+		(gen6_pte_t __iomem *)dev_priv->ggtt.gsm +
+		(offset >> PAGE_SHIFT);
+	int rpm_atomic_seq;
+
+	rpm_atomic_seq = assert_rpm_atomic_begin(dev_priv);
+
+	iowrite32(vm->pte_encode(addr, level, true, flags), pte);
+
+	I915_WRITE(GFX_FLSH_CNTL_GEN6, GFX_FLSH_CNTL_EN);
+	POSTING_READ(GFX_FLSH_CNTL_GEN6);
+
+	assert_rpm_atomic_end(dev_priv, rpm_atomic_seq);
+}
+
 /*
  * Binds an object into the global gtt with the specified cache level. The object
  * will be accessible to the GPU via commands whose operands reference offsets
@@ -2543,6 +2587,24 @@ static void gen6_ggtt_clear_range(struct i915_address_space *vm,
 	assert_rpm_atomic_end(dev_priv, rpm_atomic_seq);
 }
 
+static void i915_ggtt_insert_page(struct i915_address_space *vm,
+				  dma_addr_t addr,
+				  uint64_t offset,
+				  enum i915_cache_level cache_level,
+				  u32 unused)
+{
+	struct drm_i915_private *dev_priv = to_i915(vm->dev);
+	unsigned int flags = (cache_level == I915_CACHE_NONE) ?
+		AGP_USER_MEMORY : AGP_USER_CACHED_MEMORY;
+	int rpm_atomic_seq;
+
+	rpm_atomic_seq = assert_rpm_atomic_begin(dev_priv);
+
+	intel_gtt_insert_page(addr, offset >> PAGE_SHIFT, flags);
+
+	assert_rpm_atomic_end(dev_priv, rpm_atomic_seq);
+}
+
 static void i915_ggtt_insert_entries(struct i915_address_space *vm,
 				     struct sg_table *pages,
 				     uint64_t start,
@@ -3076,7 +3138,7 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt)
 
 	ggtt->base.bind_vma = ggtt_bind_vma;
 	ggtt->base.unbind_vma = ggtt_unbind_vma;
-
+	ggtt->base.insert_page = gen8_ggtt_insert_page;
 	ggtt->base.clear_range = nop_clear_range;
 	if (!USES_FULL_PPGTT(dev_priv))
 		ggtt->base.clear_range = gen8_ggtt_clear_range;
@@ -3116,6 +3178,7 @@ static int gen6_gmch_probe(struct i915_ggtt *ggtt)
 	ret = ggtt_probe_common(dev, ggtt->size);
 
 	ggtt->base.clear_range = gen6_ggtt_clear_range;
+	ggtt->base.insert_page = gen6_ggtt_insert_page;
 	ggtt->base.insert_entries = gen6_ggtt_insert_entries;
 	ggtt->base.bind_vma = ggtt_bind_vma;
 	ggtt->base.unbind_vma = ggtt_unbind_vma;
@@ -3147,6 +3210,7 @@ static int i915_gmch_probe(struct i915_ggtt *ggtt)
 		      &ggtt->mappable_base, &ggtt->mappable_end);
 
 	ggtt->do_idle_maps = needs_idle_maps(dev_priv->dev);
+	ggtt->base.insert_page = i915_ggtt_insert_page;
 	ggtt->base.insert_entries = i915_ggtt_insert_entries;
 	ggtt->base.clear_range = i915_ggtt_clear_range;
 	ggtt->base.bind_vma = ggtt_bind_vma;
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 62be77c..163b564 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -319,6 +319,11 @@ struct i915_address_space {
 			    uint64_t start,
 			    uint64_t length,
 			    bool use_scratch);
+	void (*insert_page)(struct i915_address_space *vm,
+			    dma_addr_t addr,
+			    uint64_t offset,
+			    enum i915_cache_level cache_level,
+			    u32 flags);
 	void (*insert_entries)(struct i915_address_space *vm,
 			       struct sg_table *st,
 			       uint64_t start,
diff --git a/include/drm/intel-gtt.h b/include/drm/intel-gtt.h
index 9e9bddaa5..f49edec 100644
--- a/include/drm/intel-gtt.h
+++ b/include/drm/intel-gtt.h
@@ -13,6 +13,9 @@ void intel_gmch_remove(void);
 bool intel_enable_gtt(void);
 
 void intel_gtt_chipset_flush(void);
+void intel_gtt_insert_page(dma_addr_t addr,
+			   unsigned int pg,
+			   unsigned int flags);
 void intel_gtt_insert_sg_entries(struct sg_table *st,
 				 unsigned int pg_start,
 				 unsigned int flags);
-- 
1.9.1

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

[PATCH 2/5] drm/i915: Introduce i915_gem_object_get_dma_address()

[ankitprasad.r.sharma]

From: Chris Wilson <chris@chris-wilson.co.uk>

This utility function is a companion to i915_gem_object_get_page() that
uses the same cached iterator for the scatterlist to perform fast
sequential lookup of the dma address associated with any page within the
object.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 53d9e3f..0349c5f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3118,6 +3118,23 @@ static inline int __sg_page_count(struct scatterlist *sg)
 struct page *
 i915_gem_object_get_dirty_page(struct drm_i915_gem_object *obj, int n);
 
+static inline dma_addr_t
+i915_gem_object_get_dma_address(struct drm_i915_gem_object *obj, int n)
+{
+	if (n < obj->get_page.last) {
+		obj->get_page.sg = obj->pages->sgl;
+		obj->get_page.last = 0;
+	}
+
+	while (obj->get_page.last + __sg_page_count(obj->get_page.sg) <= n) {
+		obj->get_page.last += __sg_page_count(obj->get_page.sg++);
+		if (unlikely(sg_is_chain(obj->get_page.sg)))
+			obj->get_page.sg = sg_chain_ptr(obj->get_page.sg);
+	}
+
+	return sg_dma_address(obj->get_page.sg) + ((n - obj->get_page.last) << PAGE_SHIFT);
+}
+
 static inline struct page *
 i915_gem_object_get_page(struct drm_i915_gem_object *obj, int n)
 {
-- 
1.9.1

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

[PATCH 3/5] drm/i915: Use insert_page for pwrite_fast

[ankitprasad.r.sharma]

From: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>

In pwrite_fast, map an object page by page if obj_ggtt_pin fails. First,
we try a nonblocking pin for the whole object (since that is fastest if
reused), then failing that we try to grab one page in the mappable
aperture. It also allows us to handle objects larger than the mappable
aperture (e.g. if we need to pwrite with vGPU restricting the aperture
to a measely 8MiB or something like that).

v2: Pin pages before starting pwrite, Combined duplicate loops (Chris)

v3: Combined loops based on local patch by Chris (Chris)

v4: Added i915 wrapper function for drm_mm_insert_node_in_range (Chris)

v5: Renamed wrapper function for drm_mm_insert_node_in_range (Chris)

v5: Added wrapper for drm_mm_remove_node() (Chris)

v6: Added get_pages call before pinning the pages (Tvrtko)
Added remove_mappable_node() wrapper for drm_mm_remove_node() (Chris)

v7: Added size argument for insert_mappable_node (Tvrtko)

v8: Do not put_pages after pwrite, do memset of node in the wrapper
function (insert_mappable_node) (Chris)

v9: Rebase (Ankit)

Signed-off-by: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 90 +++++++++++++++++++++++++++++++----------
 1 file changed, 68 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index eae8d7a..452178c 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -60,6 +60,24 @@ static bool cpu_write_needs_clflush(struct drm_i915_gem_object *obj)
 	return obj->pin_display;
 }
 
+static int
+insert_mappable_node(struct drm_i915_private *i915,
+                     struct drm_mm_node *node, u32 size)
+{
+	memset(node, 0, sizeof(*node));
+	return drm_mm_insert_node_in_range_generic(&i915->ggtt.base.mm, node,
+						   size, 0, 0, 0,
+						   i915->ggtt.mappable_end,
+						   DRM_MM_SEARCH_DEFAULT,
+						   DRM_MM_CREATE_DEFAULT);
+}
+
+static void
+remove_mappable_node(struct drm_mm_node *node)
+{
+	drm_mm_remove_node(node);
+}
+
 /* some bookkeeping */
 static void i915_gem_info_add_obj(struct drm_i915_private *dev_priv,
 				  size_t size)
@@ -765,21 +783,34 @@ fast_user_write(struct io_mapping *mapping,
  * @file: drm file pointer
  */
 static int
-i915_gem_gtt_pwrite_fast(struct drm_device *dev,
+i915_gem_gtt_pwrite_fast(struct drm_i915_private *i915,
 			 struct drm_i915_gem_object *obj,
 			 struct drm_i915_gem_pwrite *args,
 			 struct drm_file *file)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
-	struct i915_ggtt *ggtt = &dev_priv->ggtt;
-	ssize_t remain;
-	loff_t offset, page_base;
+	struct i915_ggtt *ggtt = &i915->ggtt;
+	struct drm_mm_node node;
+	uint64_t remain, offset;
 	char __user *user_data;
-	int page_offset, page_length, ret;
+	int ret;
 
 	ret = i915_gem_obj_ggtt_pin(obj, 0, PIN_MAPPABLE | PIN_NONBLOCK);
-	if (ret)
-		goto out;
+	if (ret) {
+		ret = insert_mappable_node(i915, &node, PAGE_SIZE);
+		if (ret)
+			goto out;
+
+		ret = i915_gem_object_get_pages(obj);
+		if (ret) {
+			remove_mappable_node(&node);
+			goto out;
+		}
+
+		i915_gem_object_pin_pages(obj);
+	} else {
+		node.start = i915_gem_obj_ggtt_offset(obj);
+		node.allocated = false;
+	}
 
 	ret = i915_gem_object_set_to_gtt_domain(obj, true);
 	if (ret)
@@ -789,26 +820,32 @@ i915_gem_gtt_pwrite_fast(struct drm_device *dev,
 	if (ret)
 		goto out_unpin;
 
-	user_data = u64_to_user_ptr(args->data_ptr);
-	remain = args->size;
-
-	offset = i915_gem_obj_ggtt_offset(obj) + args->offset;
-
 	intel_fb_obj_invalidate(obj, ORIGIN_GTT);
+	obj->dirty = true;
 
-	while (remain > 0) {
+	user_data = u64_to_user_ptr(args->data_ptr);
+	offset = args->offset;
+	remain = args->size;
+	while (remain) {
 		/* Operation in this page
 		 *
 		 * page_base = page offset within aperture
 		 * page_offset = offset within page
 		 * page_length = bytes to copy for this page
 		 */
-		page_base = offset & PAGE_MASK;
-		page_offset = offset_in_page(offset);
-		page_length = remain;
-		if ((page_offset + remain) > PAGE_SIZE)
-			page_length = PAGE_SIZE - page_offset;
-
+		u32 page_base = node.start;
+		unsigned page_offset = offset_in_page(offset);
+		unsigned page_length = PAGE_SIZE - page_offset;
+		page_length = remain < page_length ? remain : page_length;
+		if (node.allocated) {
+			wmb(); /* flush the write before we modify the GGTT */
+			ggtt->base.insert_page(&ggtt->base,
+					       i915_gem_object_get_dma_address(obj, offset >> PAGE_SHIFT),
+					       node.start, I915_CACHE_NONE, 0);
+			wmb(); /* flush modifications to the GGTT (insert_page) */
+		} else {
+			page_base += offset & PAGE_MASK;
+		}
 		/* If we get a fault while copying data, then (presumably) our
 		 * source page isn't available.  Return the error and we'll
 		 * retry in the slow path.
@@ -827,7 +864,16 @@ i915_gem_gtt_pwrite_fast(struct drm_device *dev,
 out_flush:
 	intel_fb_obj_flush(obj, false, ORIGIN_GTT);
 out_unpin:
-	i915_gem_object_ggtt_unpin(obj);
+	if (node.allocated) {
+		wmb();
+		ggtt->base.clear_range(&ggtt->base,
+				       node.start, node.size,
+				       true);
+		i915_gem_object_unpin_pages(obj);
+		remove_mappable_node(&node);
+	} else {
+		i915_gem_object_ggtt_unpin(obj);
+	}
 out:
 	return ret;
 }
@@ -1095,7 +1141,7 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
 	if (obj->tiling_mode == I915_TILING_NONE &&
 	    obj->base.write_domain != I915_GEM_DOMAIN_CPU &&
 	    cpu_write_needs_clflush(obj)) {
-		ret = i915_gem_gtt_pwrite_fast(dev, obj, args, file);
+		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
 		 * textures). Fallback to the shmem path in that case. */
-- 
1.9.1

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

[PATCH 4/5] drm/i915: Clearing buffer objects via CPU/GTT

[ankitprasad.r.sharma]

From: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>

This patch adds support for clearing buffer objects via CPU/GTT. This
is particularly useful for clearing out the non shmem backed objects.
Currently intend to use this only for buffers allocated from stolen
region.

v2: Added kernel doc for i915_gem_clear_object(), corrected/removed
variable assignments (Tvrtko)

v3: Map object page by page to the gtt if the pinning of the whole object
to the ggtt fails, Corrected function name (Chris)

v4: Clear the buffer page by page, and not map the whole object in the gtt
aperture. Use i915 wrapper function in place of drm_mm_insert_node_in_range.

v5: Use renamed wrapper function for drm_mm_insert_node_in_range,
updated barrier positioning (Chris)

v6: Use PAGE_SIZE instead of 4096, use get_pages call before pinning pages
(Tvrtko)

v7: Fixed the onion (undo operation in reverse order) (Chris)

v8: Rebase (Ankit)

Testcase: igt/gem_stolen

Signed-off-by: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.h |  1 +
 drivers/gpu/drm/i915/i915_gem.c | 45 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 0349c5f..e72e6af 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3109,6 +3109,7 @@ int i915_gem_obj_prepare_shmem_read(struct drm_i915_gem_object *obj,
 				    int *needs_clflush);
 
 int __must_check i915_gem_object_get_pages(struct drm_i915_gem_object *obj);
+int i915_gem_object_clear(struct drm_i915_gem_object *obj);
 
 static inline int __sg_page_count(struct scatterlist *sg)
 {
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 452178c..d658d46 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5418,3 +5418,48 @@ fail:
 	drm_gem_object_unreference(&obj->base);
 	return ERR_PTR(ret);
 }
+
+/**
+ * i915_gem_object_clear() - Clear buffer object via CPU/GTT
+ * @obj: Buffer object to be cleared
+ *
+ * Return: 0 - success, non-zero - failure
+ */
+int i915_gem_object_clear(struct drm_i915_gem_object *obj)
+{
+	struct drm_i915_private *i915 = to_i915(obj->base.dev);
+	struct i915_ggtt *ggtt = &i915->ggtt;
+	struct drm_mm_node node;
+	char __iomem *base;
+	uint64_t size = obj->base.size;
+	int ret, i;
+
+	lockdep_assert_held(&obj->base.dev->struct_mutex);
+	ret = insert_mappable_node(i915, &node, PAGE_SIZE);
+	if (ret)
+		return ret;
+
+	ret = i915_gem_object_get_pages(obj);
+	if (ret)
+		goto err_remove_node;
+
+	i915_gem_object_pin_pages(obj);
+	base = io_mapping_map_wc(ggtt->mappable, node.start, PAGE_SIZE);
+
+	for (i = 0; i < size/PAGE_SIZE; i++) {
+		ggtt->base.insert_page(&ggtt->base,
+				       i915_gem_object_get_dma_address(obj, i),
+				       node.start, I915_CACHE_NONE, 0);
+		wmb(); /* flush modifications to the GGTT (insert_page) */
+		memset_io(base, 0, PAGE_SIZE);
+		wmb(); /* flush the write before we modify the GGTT */
+	}
+
+	io_mapping_unmap(base);
+	ggtt->base.clear_range(&ggtt->base, node.start, node.size, true);
+	i915_gem_object_unpin_pages(obj);
+
+err_remove_node:
+	remove_mappable_node(&node);
+	return ret;
+}
-- 
1.9.1

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

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

[ankitprasad.r.sharma]

From: Ankitprasad Sharma <ankitprasad.r.sharma@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 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)

v9: Corrected check during pread_ioctl, to avoid shmem_pread being
called for non-shmem backed objects (Tvrtko)

v10: Moved the write_domain check to needs_clflush and tiling mode check
to pwrite_fast (Chris)

v11: Use pwrite_fast fallback for all objects (shmem and non-shmem backed),
call fast_user_write regardless of pagefault in previous iteration

v12: Use page-by-page copy for slow user access too (Chris)

v13: Handled EFAULT, Avoid use of WARN_ON, put_fence only if whole obj
pinned (Chris)

v14: Corrected datatypes/initializations (Tvrtko)

Testcase: igt/gem_stolen, igt/gem_pread, igt/gem_pwrite

Signed-off-by: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 218 ++++++++++++++++++++++++++++++++++------
 1 file changed, 188 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index d658d46..1777202 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -54,6 +54,9 @@ static bool cpu_cache_is_coherent(struct drm_device *dev,
 
 static bool cpu_write_needs_clflush(struct drm_i915_gem_object *obj)
 {
+	if (obj->base.write_domain == I915_GEM_DOMAIN_CPU)
+		return false;
+
 	if (!cpu_cache_is_coherent(obj->base.dev, obj->cache_level))
 		return true;
 
@@ -606,6 +609,142 @@ shmem_pread_slow(struct page *page, int shmem_page_offset, int page_length,
 	return ret ? - EFAULT : 0;
 }
 
+static inline unsigned long
+slow_user_access(struct io_mapping *mapping,
+		 uint64_t page_base, int page_offset,
+		 char __user *user_data,
+		 unsigned long length, bool pwrite)
+{
+	void __iomem *ioaddr;
+	void *vaddr;
+	uint64_t unwritten;
+
+	ioaddr = io_mapping_map_wc(mapping, page_base, PAGE_SIZE);
+	/* We can use the cpu mem copy function because this is X86. */
+	vaddr = (void __force *)ioaddr + page_offset;
+	if (pwrite)
+		unwritten = __copy_from_user(vaddr, user_data, length);
+	else
+		unwritten = __copy_to_user(user_data, vaddr, length);
+
+	io_mapping_unmap(ioaddr);
+	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;
+	struct i915_ggtt *ggtt = &dev_priv->ggtt;
+	struct drm_mm_node node;
+	char __user *user_data;
+	uint64_t remain;
+	uint64_t offset;
+	int ret;
+
+	ret = i915_gem_obj_ggtt_pin(obj, 0, PIN_MAPPABLE);
+	if (ret) {
+		ret = insert_mappable_node(dev_priv, &node, PAGE_SIZE);
+		if (ret)
+			goto out;
+
+		ret = i915_gem_object_get_pages(obj);
+		if (ret) {
+			remove_mappable_node(&node);
+			goto out;
+		}
+
+		i915_gem_object_pin_pages(obj);
+	} else {
+		node.start = i915_gem_obj_ggtt_offset(obj);
+		node.allocated = false;
+		ret = i915_gem_object_put_fence(obj);
+		if (ret)
+			goto out_unpin;
+	}
+
+	ret = i915_gem_object_set_to_gtt_domain(obj, false);
+	if (ret)
+		goto out_unpin;
+
+	user_data = u64_to_user_ptr(data_ptr);
+	remain = size;
+	offset = data_offset;
+
+	mutex_unlock(&dev->struct_mutex);
+	if (likely(!i915.prefault_disable)) {
+		ret = fault_in_multipages_writeable(user_data, remain);
+		if (ret) {
+			mutex_lock(&dev->struct_mutex);
+			goto out_unpin;
+		}
+	}
+
+	while (remain > 0) {
+		/* Operation in this page
+		 *
+		 * page_base = page offset within aperture
+		 * page_offset = offset within page
+		 * page_length = bytes to copy for this page
+		 */
+		u32 page_base = node.start;
+		unsigned page_offset = offset_in_page(offset);
+		unsigned page_length = PAGE_SIZE - page_offset;
+		page_length = remain < page_length ? remain : page_length;
+		if (node.allocated) {
+			wmb();
+			ggtt->base.insert_page(&ggtt->base,
+					       i915_gem_object_get_dma_address(obj, offset >> PAGE_SHIFT),
+					       node.start,
+					       I915_CACHE_NONE, 0);
+			wmb();
+		} else {
+			page_base += offset & PAGE_MASK;
+		}
+		/* This is a slow read/write as it tries to read from
+		 * and write to user memory which may result into page
+		 * faults, and so we cannot perform this under struct_mutex.
+		 */
+		if (slow_user_access(ggtt->mappable, page_base,
+				     page_offset, user_data,
+				     page_length, false)) {
+			ret = -EFAULT;
+			break;
+		}
+
+		remain -= page_length;
+		user_data += page_length;
+		offset += page_length;
+	}
+
+	mutex_lock(&dev->struct_mutex);
+	if (ret == 0 && (obj->base.read_domains & I915_GEM_DOMAIN_GTT) == 0) {
+		/* The user has modified the object whilst we tried
+		 * reading from it, and we now have no idea what domain
+		 * the pages should be in. As we have just been touching
+		 * them directly, flush everything back to the GTT
+		 * domain.
+		 */
+		ret = i915_gem_object_set_to_gtt_domain(obj, false);
+	}
+
+out_unpin:
+	if (node.allocated) {
+		wmb();
+		ggtt->base.clear_range(&ggtt->base,
+				       node.start, node.size,
+				       true);
+		i915_gem_object_unpin_pages(obj);
+		remove_mappable_node(&node);
+	} else {
+		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,
@@ -621,6 +760,9 @@ i915_gem_shmem_pread(struct drm_device *dev,
 	int needs_clflush = 0;
 	struct sg_page_iter sg_iter;
 
+	if (!obj->base.filp)
+		return -ENODEV;
+
 	user_data = u64_to_user_ptr(args->data_ptr);
 	remain = args->size;
 
@@ -732,18 +874,15 @@ 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 (ret == -EFAULT || ret == -ENODEV)
+		ret = i915_gem_gtt_pread(dev, obj, args->size,
+					args->offset, args->data_ptr);
+
 out:
 	drm_gem_object_unreference(&obj->base);
 unlock:
@@ -789,10 +928,15 @@ i915_gem_gtt_pwrite_fast(struct drm_i915_private *i915,
 			 struct drm_file *file)
 {
 	struct i915_ggtt *ggtt = &i915->ggtt;
+	struct drm_device *dev = obj->base.dev;
 	struct drm_mm_node node;
 	uint64_t remain, offset;
 	char __user *user_data;
 	int ret;
+	bool hit_slow_path = false;
+
+	if (obj->tiling_mode != I915_TILING_NONE)
+		return -EFAULT;
 
 	ret = i915_gem_obj_ggtt_pin(obj, 0, PIN_MAPPABLE | PIN_NONBLOCK);
 	if (ret) {
@@ -810,16 +954,15 @@ i915_gem_gtt_pwrite_fast(struct drm_i915_private *i915,
 	} else {
 		node.start = i915_gem_obj_ggtt_offset(obj);
 		node.allocated = false;
+		ret = i915_gem_object_put_fence(obj);
+		if (ret)
+			goto out_unpin;
 	}
 
 	ret = i915_gem_object_set_to_gtt_domain(obj, true);
 	if (ret)
 		goto out_unpin;
 
-	ret = i915_gem_object_put_fence(obj);
-	if (ret)
-		goto out_unpin;
-
 	intel_fb_obj_invalidate(obj, ORIGIN_GTT);
 	obj->dirty = true;
 
@@ -849,11 +992,23 @@ i915_gem_gtt_pwrite_fast(struct drm_i915_private *i915,
 		/* If we get a fault while copying data, then (presumably) our
 		 * source page isn't available.  Return the error and we'll
 		 * retry in the slow path.
+		 * If the object is non-shmem backed, we retry again with the
+		 * path that handles page fault.
 		 */
 		if (fast_user_write(ggtt->mappable, page_base,
 				    page_offset, user_data, page_length)) {
-			ret = -EFAULT;
-			goto out_flush;
+			hit_slow_path = true;
+			mutex_unlock(&dev->struct_mutex);
+			if (slow_user_access(ggtt->mappable,
+					     page_base,
+					     page_offset, user_data,
+					     page_length, true)) {
+				ret = -EFAULT;
+				mutex_lock(&dev->struct_mutex);
+				goto out_flush;
+			}
+
+			mutex_lock(&dev->struct_mutex);
 		}
 
 		remain -= page_length;
@@ -862,6 +1017,19 @@ i915_gem_gtt_pwrite_fast(struct drm_i915_private *i915,
 	}
 
 out_flush:
+	if (hit_slow_path) {
+		if (ret == 0 &&
+		    (obj->base.read_domains & I915_GEM_DOMAIN_GTT) == 0) {
+			/* The user has modified the object whilst we tried
+			 * reading from it, and we now have no idea what domain
+			 * the pages should be in. As we have just been touching
+			 * them directly, flush everything back to the GTT
+			 * domain.
+			 */
+			ret = i915_gem_object_set_to_gtt_domain(obj, false);
+		}
+	}
+
 	intel_fb_obj_flush(obj, false, ORIGIN_GTT);
 out_unpin:
 	if (node.allocated) {
@@ -1121,14 +1289,6 @@ 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;
@@ -1138,20 +1298,20 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
 	 * pread/pwrite currently are reading and writing from the CPU
 	 * perspective, requiring manual detiling by the client.
 	 */
-	if (obj->tiling_mode == I915_TILING_NONE &&
-	    obj->base.write_domain != I915_GEM_DOMAIN_CPU &&
-	    cpu_write_needs_clflush(obj)) {
+	if (!obj->base.filp || 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
 		 * textures). Fallback to the shmem path in that case. */
 	}
 
-	if (ret == -EFAULT || ret == -ENOSPC) {
+	if (ret == -EFAULT) {
 		if (obj->phys_handle)
 			ret = i915_gem_phys_pwrite(obj, args, file);
-		else
+		else if (obj->base.filp)
 			ret = i915_gem_shmem_pwrite(dev, obj, args, file);
+		else
+			ret = -ENODEV;
 	}
 
 out:
@@ -4014,9 +4174,7 @@ out:
 	 * object is now coherent at its new cache level (with respect
 	 * to the access domain).
 	 */
-	if (obj->cache_dirty &&
-	    obj->base.write_domain != I915_GEM_DOMAIN_CPU &&
-	    cpu_write_needs_clflush(obj)) {
+	if (obj->cache_dirty && cpu_write_needs_clflush(obj)) {
 		if (i915_gem_clflush_object(obj, true))
 			i915_gem_chipset_flush(to_i915(obj->base.dev));
 	}
-- 
1.9.1

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

✗ Ro.CI.BAT: failure for series starting with [1/5] drm/i915: Add support for mapping an object page by page

[Patchwork]

== Series Details ==

Series: series starting with [1/5] drm/i915: Add support for mapping an object page by page
URL   : https://patchwork.freedesktop.org/series/8528/
State : failure

== Summary ==

Series 8528v1 Series without cover letter
http://patchwork.freedesktop.org/api/1.0/series/8528/revisions/1/mbox

Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-cmd:
                pass       -> FAIL       (ro-byt-n2820)
Test kms_flip:
        Subgroup basic-flip-vs-wf_vblank:
                fail       -> PASS       (ro-bdw-i7-5600u)
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                skip       -> DMESG-WARN (ro-bdw-i5-5250u)

ro-bdw-i5-5250u  total:213  pass:197  dwarn:3   dfail:0   fail:0   skip:13 
ro-bdw-i7-5600u  total:213  pass:185  dwarn:0   dfail:0   fail:0   skip:28 
ro-bsw-n3050     total:213  pass:172  dwarn:0   dfail:0   fail:2   skip:39 
ro-byt-n2820     total:213  pass:173  dwarn:0   dfail:0   fail:3   skip:37 
ro-hsw-i3-4010u  total:213  pass:190  dwarn:0   dfail:0   fail:0   skip:23 
ro-hsw-i7-4770r  total:213  pass:190  dwarn:0   dfail:0   fail:0   skip:23 
ro-ilk-i7-620lm  total:213  pass:150  dwarn:0   dfail:0   fail:1   skip:62 
ro-ilk1-i5-650   total:208  pass:150  dwarn:0   dfail:0   fail:1   skip:57 
ro-ivb-i7-3770   total:213  pass:181  dwarn:0   dfail:0   fail:0   skip:32 
ro-ivb2-i7-3770  total:213  pass:185  dwarn:0   dfail:0   fail:0   skip:28 
ro-snb-i7-2620M  total:213  pass:174  dwarn:0   dfail:0   fail:1   skip:38 
fi-hsw-i7-4770k failed to connect after reboot
ro-bdw-i7-5557U failed to connect after reboot

Results at /archive/results/CI_IGT_test/RO_Patchwork_1156/

b373842 drm-intel-nightly: 2016y-06m-09d-16h-49m-09s UTC integration manifest
165ff1a drm/i915: Support for pread/pwrite from/to non shmem backed objects
7c4d2d9 drm/i915: Clearing buffer objects via CPU/GTT
2c60c194 drm/i915: Use insert_page for pwrite_fast
3f97215 drm/i915: Introduce i915_gem_object_get_dma_address()
fbba107 drm/i915: Add support for mapping an object page by page

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

Re: ✗ Ro.CI.BAT: failure for series starting with [1/5] drm/i915: Add support for mapping an object page by page

[Tvrtko Ursulin]

On 10/06/16 10:40, Patchwork wrote:
> == Series Details ==
>
> Series: series starting with [1/5] drm/i915: Add support for mapping an object page by page
> URL   : https://patchwork.freedesktop.org/series/8528/
> State : failure
>
> == Summary ==
>
> Series 8528v1 Series without cover letter
> http://patchwork.freedesktop.org/api/1.0/series/8528/revisions/1/mbox
>
> Test gem_exec_flush:
>          Subgroup basic-batch-kernel-default-cmd:
>                  pass       -> FAIL       (ro-byt-n2820)
> Test kms_flip:
>          Subgroup basic-flip-vs-wf_vblank:
>                  fail       -> PASS       (ro-bdw-i7-5600u)
> Test kms_pipe_crc_basic:
>          Subgroup suspend-read-crc-pipe-a:
>                  skip       -> DMESG-WARN (ro-bdw-i5-5250u)
>
> ro-bdw-i5-5250u  total:213  pass:197  dwarn:3   dfail:0   fail:0   skip:13
> ro-bdw-i7-5600u  total:213  pass:185  dwarn:0   dfail:0   fail:0   skip:28
> ro-bsw-n3050     total:213  pass:172  dwarn:0   dfail:0   fail:2   skip:39
> ro-byt-n2820     total:213  pass:173  dwarn:0   dfail:0   fail:3   skip:37
> ro-hsw-i3-4010u  total:213  pass:190  dwarn:0   dfail:0   fail:0   skip:23
> ro-hsw-i7-4770r  total:213  pass:190  dwarn:0   dfail:0   fail:0   skip:23
> ro-ilk-i7-620lm  total:213  pass:150  dwarn:0   dfail:0   fail:1   skip:62
> ro-ilk1-i5-650   total:208  pass:150  dwarn:0   dfail:0   fail:1   skip:57
> ro-ivb-i7-3770   total:213  pass:181  dwarn:0   dfail:0   fail:0   skip:32
> ro-ivb2-i7-3770  total:213  pass:185  dwarn:0   dfail:0   fail:0   skip:28
> ro-snb-i7-2620M  total:213  pass:174  dwarn:0   dfail:0   fail:1   skip:38
> fi-hsw-i7-4770k failed to connect after reboot
> ro-bdw-i7-5557U failed to connect after reboot
>
> Results at /archive/results/CI_IGT_test/RO_Patchwork_1156/
>
> b373842 drm-intel-nightly: 2016y-06m-09d-16h-49m-09s UTC integration manifest
> 165ff1a drm/i915: Support for pread/pwrite from/to non shmem backed objects
> 7c4d2d9 drm/i915: Clearing buffer objects via CPU/GTT
> 2c60c194 drm/i915: Use insert_page for pwrite_fast
> 3f97215 drm/i915: Introduce i915_gem_object_get_dma_address()
> fbba107 drm/i915: Add support for mapping an object page by page

Copy&paste of Ankit's result analysis from another thread:

"""
Hi,

The failures seen are not introduced by the patches.
Following are the bug numbers related to the failures
https://bugs.freedesktop.org/show_bug.cgi?id=95372 -
igt/gem_exec_flush@basic-batch-kernel-default-cmd

https://bugs.freedesktop.org/show_bug.cgi?id=86365 -
igt/kms_pipe_crc_basic
"""

Chris, are you happy with merging this reduced series?

Thanks,
Ankit
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✗ Ro.CI.BAT: failure for series starting with [1/5] drm/i915: Add support for mapping an object page by page

[Chris Wilson]

On Fri, Jun 10, 2016 at 01:49:18PM +0100, Tvrtko Ursulin wrote:
> 
> On 10/06/16 10:40, Patchwork wrote:
> >== Series Details ==
> >
> >Series: series starting with [1/5] drm/i915: Add support for mapping an object page by page
> >URL   : https://patchwork.freedesktop.org/series/8528/
> >State : failure
> >
> >== Summary ==
> >
> >Series 8528v1 Series without cover letter
> >http://patchwork.freedesktop.org/api/1.0/series/8528/revisions/1/mbox
> >
> >Test gem_exec_flush:
> >         Subgroup basic-batch-kernel-default-cmd:
> >                 pass       -> FAIL       (ro-byt-n2820)
> >Test kms_flip:
> >         Subgroup basic-flip-vs-wf_vblank:
> >                 fail       -> PASS       (ro-bdw-i7-5600u)
> >Test kms_pipe_crc_basic:
> >         Subgroup suspend-read-crc-pipe-a:
> >                 skip       -> DMESG-WARN (ro-bdw-i5-5250u)
> >
> >ro-bdw-i5-5250u  total:213  pass:197  dwarn:3   dfail:0   fail:0   skip:13
> >ro-bdw-i7-5600u  total:213  pass:185  dwarn:0   dfail:0   fail:0   skip:28
> >ro-bsw-n3050     total:213  pass:172  dwarn:0   dfail:0   fail:2   skip:39
> >ro-byt-n2820     total:213  pass:173  dwarn:0   dfail:0   fail:3   skip:37
> >ro-hsw-i3-4010u  total:213  pass:190  dwarn:0   dfail:0   fail:0   skip:23
> >ro-hsw-i7-4770r  total:213  pass:190  dwarn:0   dfail:0   fail:0   skip:23
> >ro-ilk-i7-620lm  total:213  pass:150  dwarn:0   dfail:0   fail:1   skip:62
> >ro-ilk1-i5-650   total:208  pass:150  dwarn:0   dfail:0   fail:1   skip:57
> >ro-ivb-i7-3770   total:213  pass:181  dwarn:0   dfail:0   fail:0   skip:32
> >ro-ivb2-i7-3770  total:213  pass:185  dwarn:0   dfail:0   fail:0   skip:28
> >ro-snb-i7-2620M  total:213  pass:174  dwarn:0   dfail:0   fail:1   skip:38
> >fi-hsw-i7-4770k failed to connect after reboot
> >ro-bdw-i7-5557U failed to connect after reboot
> >
> >Results at /archive/results/CI_IGT_test/RO_Patchwork_1156/
> >
> >b373842 drm-intel-nightly: 2016y-06m-09d-16h-49m-09s UTC integration manifest
> >165ff1a drm/i915: Support for pread/pwrite from/to non shmem backed objects
> >7c4d2d9 drm/i915: Clearing buffer objects via CPU/GTT
> >2c60c194 drm/i915: Use insert_page for pwrite_fast
> >3f97215 drm/i915: Introduce i915_gem_object_get_dma_address()
> >fbba107 drm/i915: Add support for mapping an object page by page
> 
> Copy&paste of Ankit's result analysis from another thread:
> 
> """
> Hi,
> 
> The failures seen are not introduced by the patches.
> Following are the bug numbers related to the failures
> https://bugs.freedesktop.org/show_bug.cgi?id=95372 -
> igt/gem_exec_flush@basic-batch-kernel-default-cmd
> 
> https://bugs.freedesktop.org/show_bug.cgi?id=86365 -
> igt/kms_pipe_crc_basic
> """
> 
> Chris, are you happy with merging this reduced series?

Patch 4 (clear via CPU) is unused.

Other than that the pread/pwrite fix a hole in the API for imported
dma-buf objects, so yes they are useful.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✗ Ro.CI.BAT: failure for series starting with [1/5] drm/i915: Add support for mapping an object page by page

[Tvrtko Ursulin]

On 10/06/16 13:56, Chris Wilson wrote:
> On Fri, Jun 10, 2016 at 01:49:18PM +0100, Tvrtko Ursulin wrote:
>>
>> On 10/06/16 10:40, Patchwork wrote:
>>> == Series Details ==
>>>
>>> Series: series starting with [1/5] drm/i915: Add support for mapping an object page by page
>>> URL   : https://patchwork.freedesktop.org/series/8528/
>>> State : failure
>>>
>>> == Summary ==
>>>
>>> Series 8528v1 Series without cover letter
>>> http://patchwork.freedesktop.org/api/1.0/series/8528/revisions/1/mbox
>>>
>>> Test gem_exec_flush:
>>>          Subgroup basic-batch-kernel-default-cmd:
>>>                  pass       -> FAIL       (ro-byt-n2820)
>>> Test kms_flip:
>>>          Subgroup basic-flip-vs-wf_vblank:
>>>                  fail       -> PASS       (ro-bdw-i7-5600u)
>>> Test kms_pipe_crc_basic:
>>>          Subgroup suspend-read-crc-pipe-a:
>>>                  skip       -> DMESG-WARN (ro-bdw-i5-5250u)
>>>
>>> ro-bdw-i5-5250u  total:213  pass:197  dwarn:3   dfail:0   fail:0   skip:13
>>> ro-bdw-i7-5600u  total:213  pass:185  dwarn:0   dfail:0   fail:0   skip:28
>>> ro-bsw-n3050     total:213  pass:172  dwarn:0   dfail:0   fail:2   skip:39
>>> ro-byt-n2820     total:213  pass:173  dwarn:0   dfail:0   fail:3   skip:37
>>> ro-hsw-i3-4010u  total:213  pass:190  dwarn:0   dfail:0   fail:0   skip:23
>>> ro-hsw-i7-4770r  total:213  pass:190  dwarn:0   dfail:0   fail:0   skip:23
>>> ro-ilk-i7-620lm  total:213  pass:150  dwarn:0   dfail:0   fail:1   skip:62
>>> ro-ilk1-i5-650   total:208  pass:150  dwarn:0   dfail:0   fail:1   skip:57
>>> ro-ivb-i7-3770   total:213  pass:181  dwarn:0   dfail:0   fail:0   skip:32
>>> ro-ivb2-i7-3770  total:213  pass:185  dwarn:0   dfail:0   fail:0   skip:28
>>> ro-snb-i7-2620M  total:213  pass:174  dwarn:0   dfail:0   fail:1   skip:38
>>> fi-hsw-i7-4770k failed to connect after reboot
>>> ro-bdw-i7-5557U failed to connect after reboot
>>>
>>> Results at /archive/results/CI_IGT_test/RO_Patchwork_1156/
>>>
>>> b373842 drm-intel-nightly: 2016y-06m-09d-16h-49m-09s UTC integration manifest
>>> 165ff1a drm/i915: Support for pread/pwrite from/to non shmem backed objects
>>> 7c4d2d9 drm/i915: Clearing buffer objects via CPU/GTT
>>> 2c60c194 drm/i915: Use insert_page for pwrite_fast
>>> 3f97215 drm/i915: Introduce i915_gem_object_get_dma_address()
>>> fbba107 drm/i915: Add support for mapping an object page by page
>>
>> Copy&paste of Ankit's result analysis from another thread:
>>
>> """
>> Hi,
>>
>> The failures seen are not introduced by the patches.
>> Following are the bug numbers related to the failures
>> https://bugs.freedesktop.org/show_bug.cgi?id=95372 -
>> igt/gem_exec_flush@basic-batch-kernel-default-cmd
>>
>> https://bugs.freedesktop.org/show_bug.cgi?id=86365 -
>> igt/kms_pipe_crc_basic
>> """
>>
>> Chris, are you happy with merging this reduced series?
>
> Patch 4 (clear via CPU) is unused.
>
> Other than that the pread/pwrite fix a hole in the API for imported
> dma-buf objects, so yes they are useful.

I've merged patches 1-3 and 5. Thanks for the patches and review!

Regards,

Tvrtko



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

[PATCH] drm/i915: pwrite/pread do not require obj->base.filp, just pages

[Chris Wilson]

The idea behind relaxing the restriction for pread/pwrite was to handle
!obj->base.flip, i.e. non-shmemfs backed objects, which only requires
that the object provide struct pages.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 21d0dea57312..0a29c57dae59 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -760,7 +760,7 @@ i915_gem_shmem_pread(struct drm_device *dev,
 	int needs_clflush = 0;
 	struct sg_page_iter sg_iter;
 
-	if (!obj->base.filp)
+	if (((obj->ops->flags & I915_GEM_OBJECT_HAS_STRUCT_PAGE) == 0))
 		return -ENODEV;
 
 	user_data = u64_to_user_ptr(args->data_ptr);
@@ -1298,7 +1298,8 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
 	 * pread/pwrite currently are reading and writing from the CPU
 	 * perspective, requiring manual detiling by the client.
 	 */
-	if (!obj->base.filp || cpu_write_needs_clflush(obj)) {
+	if (((obj->ops->flags & I915_GEM_OBJECT_HAS_STRUCT_PAGE) == 0) ||
+	    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
@@ -1308,7 +1309,7 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
 	if (ret == -EFAULT) {
 		if (obj->phys_handle)
 			ret = i915_gem_phys_pwrite(obj, args, file);
-		else if (obj->base.filp)
+		else if ((obj->ops->flags & I915_GEM_OBJECT_HAS_STRUCT_PAGE))
 			ret = i915_gem_shmem_pwrite(dev, obj, args, file);
 		else
 			ret = -ENODEV;
-- 
2.8.1

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

✗ Ro.CI.BAT: failure for series starting with drm/i915: pwrite/pread do not require obj->base.filp, just pages (rev2)

[Patchwork]

== Series Details ==

Series: series starting with drm/i915: pwrite/pread do not require obj->base.filp, just pages (rev2)
URL   : https://patchwork.freedesktop.org/series/8528/
State : failure

== Summary ==

Applying: drm/i915: pwrite/pread do not require obj->base.filp, just pages
Applying: drm/i915: Introduce i915_gem_object_get_dma_address()
Using index info to reconstruct a base tree...
M	drivers/gpu/drm/i915/i915_drv.h
Falling back to patching base and 3-way merge...
No changes -- Patch already applied.
Applying: drm/i915: Use insert_page for pwrite_fast
Using index info to reconstruct a base tree...
M	drivers/gpu/drm/i915/i915_gem.c
Falling back to patching base and 3-way merge...
Auto-merging drivers/gpu/drm/i915/i915_gem.c
CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/i915_gem.c
error: Failed to merge in the changes.
Patch failed at 0003 drm/i915: Use insert_page for pwrite_fast
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

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

[PATCH] drm/i915: pwrite/pread do not require obj->base.filp, just pages

[Chris Wilson]

The idea behind relaxing the restriction for pread/pwrite was to handle
!obj->base.flip, i.e. non-shmemfs backed objects, which only requires
that the object provide struct pages.

v2: Remove excess (). Note enough editing after copy'n'paste.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 21d0dea57312..6f950c37efab 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -760,7 +760,7 @@ i915_gem_shmem_pread(struct drm_device *dev,
 	int needs_clflush = 0;
 	struct sg_page_iter sg_iter;
 
-	if (!obj->base.filp)
+	if ((obj->ops->flags & I915_GEM_OBJECT_HAS_STRUCT_PAGE) == 0)
 		return -ENODEV;
 
 	user_data = u64_to_user_ptr(args->data_ptr);
@@ -1298,7 +1298,8 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
 	 * pread/pwrite currently are reading and writing from the CPU
 	 * perspective, requiring manual detiling by the client.
 	 */
-	if (!obj->base.filp || cpu_write_needs_clflush(obj)) {
+	if ((obj->ops->flags & I915_GEM_OBJECT_HAS_STRUCT_PAGE) == 0 ||
+	    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
@@ -1308,7 +1309,7 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
 	if (ret == -EFAULT) {
 		if (obj->phys_handle)
 			ret = i915_gem_phys_pwrite(obj, args, file);
-		else if (obj->base.filp)
+		else if (obj->ops->flags & I915_GEM_OBJECT_HAS_STRUCT_PAGE)
 			ret = i915_gem_shmem_pwrite(dev, obj, args, file);
 		else
 			ret = -ENODEV;
-- 
2.8.1

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

Re: [PATCH] drm/i915: pwrite/pread do not require obj->base.filp, just pages

[Tvrtko Ursulin]

On 13/06/16 13:40, Chris Wilson wrote:
> The idea behind relaxing the restriction for pread/pwrite was to handle
> !obj->base.flip, i.e. non-shmemfs backed objects, which only requires
> that the object provide struct pages.
>
> v2: Remove excess (). Note enough editing after copy'n'paste.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_gem.c | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 21d0dea57312..6f950c37efab 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -760,7 +760,7 @@ i915_gem_shmem_pread(struct drm_device *dev,
>   	int needs_clflush = 0;
>   	struct sg_page_iter sg_iter;
>
> -	if (!obj->base.filp)
> +	if ((obj->ops->flags & I915_GEM_OBJECT_HAS_STRUCT_PAGE) == 0)
>   		return -ENODEV;
>
>   	user_data = u64_to_user_ptr(args->data_ptr);
> @@ -1298,7 +1298,8 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
>   	 * pread/pwrite currently are reading and writing from the CPU
>   	 * perspective, requiring manual detiling by the client.
>   	 */
> -	if (!obj->base.filp || cpu_write_needs_clflush(obj)) {
> +	if ((obj->ops->flags & I915_GEM_OBJECT_HAS_STRUCT_PAGE) == 0 ||
> +	    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
> @@ -1308,7 +1309,7 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
>   	if (ret == -EFAULT) {
>   		if (obj->phys_handle)
>   			ret = i915_gem_phys_pwrite(obj, args, file);
> -		else if (obj->base.filp)
> +		else if (obj->ops->flags & I915_GEM_OBJECT_HAS_STRUCT_PAGE)
>   			ret = i915_gem_shmem_pwrite(dev, obj, args, file);
>   		else
>   			ret = -ENODEV;
>

To enable on userptr or there is more to it? Would it even make more 
sense to keep rejecting it on userptr to discourage suboptimal userspace?

Regards,

Tvrtko

P.S. You need to send it outside this thread to get a CI run.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: pwrite/pread do not require obj->base.filp, just pages

[Chris Wilson]

On Mon, Jun 13, 2016 at 01:45:56PM +0100, Tvrtko Ursulin wrote:
> 
> On 13/06/16 13:40, Chris Wilson wrote:
> >The idea behind relaxing the restriction for pread/pwrite was to handle
> >!obj->base.flip, i.e. non-shmemfs backed objects, which only requires
> >that the object provide struct pages.
> >
> >v2: Remove excess (). Note enough editing after copy'n'paste.
> >
> >Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >Cc: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
> >Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >---
> >  drivers/gpu/drm/i915/i915_gem.c | 7 ++++---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> >index 21d0dea57312..6f950c37efab 100644
> >--- a/drivers/gpu/drm/i915/i915_gem.c
> >+++ b/drivers/gpu/drm/i915/i915_gem.c
> >@@ -760,7 +760,7 @@ i915_gem_shmem_pread(struct drm_device *dev,
> >  	int needs_clflush = 0;
> >  	struct sg_page_iter sg_iter;
> >
> >-	if (!obj->base.filp)
> >+	if ((obj->ops->flags & I915_GEM_OBJECT_HAS_STRUCT_PAGE) == 0)
> >  		return -ENODEV;
> >
> >  	user_data = u64_to_user_ptr(args->data_ptr);
> >@@ -1298,7 +1298,8 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
> >  	 * pread/pwrite currently are reading and writing from the CPU
> >  	 * perspective, requiring manual detiling by the client.
> >  	 */
> >-	if (!obj->base.filp || cpu_write_needs_clflush(obj)) {
> >+	if ((obj->ops->flags & I915_GEM_OBJECT_HAS_STRUCT_PAGE) == 0 ||
> >+	    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
> >@@ -1308,7 +1309,7 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
> >  	if (ret == -EFAULT) {
> >  		if (obj->phys_handle)
> >  			ret = i915_gem_phys_pwrite(obj, args, file);
> >-		else if (obj->base.filp)
> >+		else if (obj->ops->flags & I915_GEM_OBJECT_HAS_STRUCT_PAGE)
> >  			ret = i915_gem_shmem_pwrite(dev, obj, args, file);
> >  		else
> >  			ret = -ENODEV;
> >
> 
> To enable on userptr or there is more to it? Would it even make more
> sense to keep rejecting it on userptr to discourage suboptimal
> userspace?

And prime objects, and everything in future not backed by a filp.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Ro.CI.BAT: failure for series starting with drm/i915: pwrite/pread do not require obj->base.filp, just pages (rev3)

[Patchwork]

== Series Details ==

Series: series starting with drm/i915: pwrite/pread do not require obj->base.filp, just pages (rev3)
URL   : https://patchwork.freedesktop.org/series/8528/
State : failure

== Summary ==

Applying: drm/i915: pwrite/pread do not require obj->base.filp, just pages
Applying: drm/i915: Introduce i915_gem_object_get_dma_address()
Using index info to reconstruct a base tree...
M	drivers/gpu/drm/i915/i915_drv.h
Falling back to patching base and 3-way merge...
No changes -- Patch already applied.
Applying: drm/i915: Use insert_page for pwrite_fast
Using index info to reconstruct a base tree...
M	drivers/gpu/drm/i915/i915_gem.c
Falling back to patching base and 3-way merge...
Auto-merging drivers/gpu/drm/i915/i915_gem.c
CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/i915_gem.c
error: Failed to merge in the changes.
Patch failed at 0003 drm/i915: Use insert_page for pwrite_fast
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

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

Re: [PATCH] drm/i915: pwrite/pread do not require obj->base.filp, just pages

[Tvrtko Ursulin]

On 13/06/16 13:52, Chris Wilson wrote:
> On Mon, Jun 13, 2016 at 01:45:56PM +0100, Tvrtko Ursulin wrote:
>>
>> On 13/06/16 13:40, Chris Wilson wrote:
>>> The idea behind relaxing the restriction for pread/pwrite was to handle
>>> !obj->base.flip, i.e. non-shmemfs backed objects, which only requires
>>> that the object provide struct pages.
>>>
>>> v2: Remove excess (). Note enough editing after copy'n'paste.
>>>
>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>>> Cc: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
>>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/i915_gem.c | 7 ++++---
>>>   1 file changed, 4 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
>>> index 21d0dea57312..6f950c37efab 100644
>>> --- a/drivers/gpu/drm/i915/i915_gem.c
>>> +++ b/drivers/gpu/drm/i915/i915_gem.c
>>> @@ -760,7 +760,7 @@ i915_gem_shmem_pread(struct drm_device *dev,
>>>   	int needs_clflush = 0;
>>>   	struct sg_page_iter sg_iter;
>>>
>>> -	if (!obj->base.filp)
>>> +	if ((obj->ops->flags & I915_GEM_OBJECT_HAS_STRUCT_PAGE) == 0)
>>>   		return -ENODEV;
>>>
>>>   	user_data = u64_to_user_ptr(args->data_ptr);
>>> @@ -1298,7 +1298,8 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
>>>   	 * pread/pwrite currently are reading and writing from the CPU
>>>   	 * perspective, requiring manual detiling by the client.
>>>   	 */
>>> -	if (!obj->base.filp || cpu_write_needs_clflush(obj)) {
>>> +	if ((obj->ops->flags & I915_GEM_OBJECT_HAS_STRUCT_PAGE) == 0 ||
>>> +	    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
>>> @@ -1308,7 +1309,7 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
>>>   	if (ret == -EFAULT) {
>>>   		if (obj->phys_handle)
>>>   			ret = i915_gem_phys_pwrite(obj, args, file);
>>> -		else if (obj->base.filp)
>>> +		else if (obj->ops->flags & I915_GEM_OBJECT_HAS_STRUCT_PAGE)
>>>   			ret = i915_gem_shmem_pwrite(dev, obj, args, file);
>>>   		else
>>>   			ret = -ENODEV;
>>>
>>
>> To enable on userptr or there is more to it? Would it even make more
>> sense to keep rejecting it on userptr to discourage suboptimal
>> userspace?
>
> And prime objects, and everything in future not backed by a filp.

Hmm.. I sense a hole in the IGT coverage. :)

You definitely do not mind allowing it with userptr?

Actually, we don't need to go through the aperture for anything but 
stolen, right? A third, more optimal path could be added for page backed 
objects which are not shmem, not userptr and not stolen.

Regards,

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

Re: [PATCH] drm/i915: pwrite/pread do not require obj->base.filp, just pages

[Chris Wilson]

On Mon, Jun 13, 2016 at 02:12:46PM +0100, Tvrtko Ursulin wrote:
> 
> On 13/06/16 13:52, Chris Wilson wrote:
> >On Mon, Jun 13, 2016 at 01:45:56PM +0100, Tvrtko Ursulin wrote:
> >>
> >>On 13/06/16 13:40, Chris Wilson wrote:
> >>>The idea behind relaxing the restriction for pread/pwrite was to handle
> >>>!obj->base.flip, i.e. non-shmemfs backed objects, which only requires
> >>>that the object provide struct pages.
> >>>
> >>>v2: Remove excess (). Note enough editing after copy'n'paste.
> >>>
> >>>Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >>>Cc: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
> >>>Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>>---
> >>>  drivers/gpu/drm/i915/i915_gem.c | 7 ++++---
> >>>  1 file changed, 4 insertions(+), 3 deletions(-)
> >>>
> >>>diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> >>>index 21d0dea57312..6f950c37efab 100644
> >>>--- a/drivers/gpu/drm/i915/i915_gem.c
> >>>+++ b/drivers/gpu/drm/i915/i915_gem.c
> >>>@@ -760,7 +760,7 @@ i915_gem_shmem_pread(struct drm_device *dev,
> >>>  	int needs_clflush = 0;
> >>>  	struct sg_page_iter sg_iter;
> >>>
> >>>-	if (!obj->base.filp)
> >>>+	if ((obj->ops->flags & I915_GEM_OBJECT_HAS_STRUCT_PAGE) == 0)
> >>>  		return -ENODEV;
> >>>
> >>>  	user_data = u64_to_user_ptr(args->data_ptr);
> >>>@@ -1298,7 +1298,8 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
> >>>  	 * pread/pwrite currently are reading and writing from the CPU
> >>>  	 * perspective, requiring manual detiling by the client.
> >>>  	 */
> >>>-	if (!obj->base.filp || cpu_write_needs_clflush(obj)) {
> >>>+	if ((obj->ops->flags & I915_GEM_OBJECT_HAS_STRUCT_PAGE) == 0 ||
> >>>+	    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
> >>>@@ -1308,7 +1309,7 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
> >>>  	if (ret == -EFAULT) {
> >>>  		if (obj->phys_handle)
> >>>  			ret = i915_gem_phys_pwrite(obj, args, file);
> >>>-		else if (obj->base.filp)
> >>>+		else if (obj->ops->flags & I915_GEM_OBJECT_HAS_STRUCT_PAGE)
> >>>  			ret = i915_gem_shmem_pwrite(dev, obj, args, file);
> >>>  		else
> >>>  			ret = -ENODEV;
> >>>
> >>
> >>To enable on userptr or there is more to it? Would it even make more
> >>sense to keep rejecting it on userptr to discourage suboptimal
> >>userspace?
> >
> >And prime objects, and everything in future not backed by a filp.
> 
> Hmm.. I sense a hole in the IGT coverage. :)
> 
> You definitely do not mind allowing it with userptr?

Don't mind. People shouldn't use it, but I'd rather have fewer special
cases in the ABI.
 
> Actually, we don't need to go through the aperture for anything but
> stolen, right? A third, more optimal path could be added for page
> backed objects which are not shmem, not userptr and not stolen.

Hence s/obj->base.filp/obj->op->flags & HAS_STRUCT_PAGE/. The shmem path
is for direct access to struct page, everything else requires
indirect access through the GTT.

In the future, we may want indirect access through another set of pfn,
but that is likely overkill.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx