diff mbox

[18/22] drm/i915: Choose not to evict faultable objects from the GGTT

Message ID 1471344168-28136-19-git-send-email-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Chris Wilson Aug. 16, 2016, 10:42 a.m. UTC
Often times we do not want to evict mapped objects from the GGTT as
these are quite expensive to teardown and frequently reused (causing an
equally, if not more so, expensive setup). In particular, when faulting
in a new object we want to avoid evicting an active object, or else we
may trigger a page-fault-of-doom as we ping-pong between evicting two
objects.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem.c       | 18 ++++++++++++++++--
 drivers/gpu/drm/i915/i915_gem_evict.c |  7 +++++--
 drivers/gpu/drm/i915/i915_gem_gtt.h   |  1 +
 3 files changed, 22 insertions(+), 4 deletions(-)

Comments

Joonas Lahtinen Aug. 16, 2016, 11:31 a.m. UTC | #1
On ti, 2016-08-16 at 11:42 +0100, Chris Wilson wrote:
> @@ -1751,6 +1759,12 @@ int i915_gem_fault(struct vm_area_struct *area, struct vm_fault *vmf)
>  			      (area->vm_end - area->vm_start) / PAGE_SIZE -
>  			      partial.params.partial.offset);
>  
> +		/* If the partial covers the entire object, just create a
> +		 * normal VMA.
> +		 */
> +		if (chunk_size <= obj->base.size >> PAGE_SHIFT)

Surely you meant >= ?

> +			partial.type = I915_GGTT_VIEW_NORMAL;
> +

The 'partial' name is now even worse, at least this is close to the
call site.

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Regards, Joonas
Chris Wilson Aug. 16, 2016, 11:46 a.m. UTC | #2
On Tue, Aug 16, 2016 at 02:31:00PM +0300, Joonas Lahtinen wrote:
> On ti, 2016-08-16 at 11:42 +0100, Chris Wilson wrote:
> > @@ -1751,6 +1759,12 @@ int i915_gem_fault(struct vm_area_struct *area, struct vm_fault *vmf)
> >  			      (area->vm_end - area->vm_start) / PAGE_SIZE -
> >  			      partial.params.partial.offset);
> >  
> > +		/* If the partial covers the entire object, just create a
> > +		 * normal VMA.
> > +		 */
> > +		if (chunk_size <= obj->base.size >> PAGE_SHIFT)
> 
> Surely you meant >= ?

Sigh. Yes.
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 74ea0926ad70..e28d283256f0 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1700,6 +1700,7 @@  int i915_gem_fault(struct vm_area_struct *area, struct vm_fault *vmf)
 	struct i915_vma *vma;
 	pgoff_t page_offset;
 	unsigned long pfn;
+	unsigned int flags;
 	int ret;
 
 	/* We don't use vmf->pgoff since that has the fake offset */
@@ -1729,9 +1730,16 @@  int i915_gem_fault(struct vm_area_struct *area, struct vm_fault *vmf)
 		goto err_unlock;
 	}
 
+	/* If the object is smaller than a couple of partial vma, it is
+	 * not worth only creating a single partial vma - we may as well
+	 * clear enough space for the full object.
+	 */
+	flags = PIN_MAPPABLE;
+	if (obj->base.size > 2 * MIN_CHUNK_PAGES << PAGE_SHIFT)
+		flags |= PIN_NONBLOCK | PIN_NONFAULT;
+
 	/* Now pin it into the GTT as needed */
-	vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0,
-				       PIN_MAPPABLE | PIN_NONBLOCK);
+	vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0, flags);
 	if (IS_ERR(vma)) {
 		struct i915_ggtt_view partial;
 		unsigned int chunk_size;
@@ -1751,6 +1759,12 @@  int i915_gem_fault(struct vm_area_struct *area, struct vm_fault *vmf)
 			      (area->vm_end - area->vm_start) / PAGE_SIZE -
 			      partial.params.partial.offset);
 
+		/* If the partial covers the entire object, just create a
+		 * normal VMA.
+		 */
+		if (chunk_size <= obj->base.size >> PAGE_SHIFT)
+			partial.type = I915_GGTT_VIEW_NORMAL;
+
 		vma = i915_gem_object_ggtt_pin(obj, &partial, 0, 0,
 					       PIN_MAPPABLE);
 	}
diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c
index f76c06e92677..815d5fbe07ac 100644
--- a/drivers/gpu/drm/i915/i915_gem_evict.c
+++ b/drivers/gpu/drm/i915/i915_gem_evict.c
@@ -47,7 +47,7 @@  gpu_is_idle(struct drm_i915_private *dev_priv)
 }
 
 static bool
-mark_free(struct i915_vma *vma, struct list_head *unwind)
+mark_free(struct i915_vma *vma, unsigned int flags, struct list_head *unwind)
 {
 	if (i915_vma_is_pinned(vma))
 		return false;
@@ -55,6 +55,9 @@  mark_free(struct i915_vma *vma, struct list_head *unwind)
 	if (WARN_ON(!list_empty(&vma->exec_list)))
 		return false;
 
+	if (flags & PIN_NONFAULT && vma->obj->fault_mappable)
+		return false;
+
 	list_add(&vma->exec_list, unwind);
 	return drm_mm_scan_add_block(&vma->node);
 }
@@ -129,7 +132,7 @@  search_again:
 	phase = phases;
 	do {
 		list_for_each_entry(vma, *phase, vm_link)
-			if (mark_free(vma, &eviction_list))
+			if (mark_free(vma, flags, &eviction_list))
 				goto found;
 	} while (*++phase);
 
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 4b67347834b5..52fded446390 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -639,6 +639,7 @@  void i915_gem_gtt_finish_object(struct drm_i915_gem_object *obj);
 #define PIN_NONBLOCK		BIT(0)
 #define PIN_MAPPABLE		BIT(1)
 #define PIN_ZONE_4G		BIT(2)
+#define PIN_NONFAULT		BIT(3)
 
 #define PIN_MBZ			BIT(5) /* I915_VMA_PIN_OVERFLOW */
 #define PIN_GLOBAL		BIT(6) /* I915_VMA_GLOBAL_BIND */