All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chris Wilson <chris@chris-wilson.co.uk>
To: intel-gfx@lists.freedesktop.org
Subject: [PATCH 05/20] drm/i915: Implement fair lru eviction across both rings. (v2)
Date: Sat,  7 Aug 2010 11:01:24 +0100	[thread overview]
Message-ID: <1281175299-1379-6-git-send-email-chris@chris-wilson.co.uk> (raw)
In-Reply-To: <1281175299-1379-1-git-send-email-chris@chris-wilson.co.uk>

Based in a large part upon Daniel Vetter's implementation and adapted
for handling multiple rings in a single pass.

This should lead to better gtt usage and fixes the page-fault-of-doom
triggered. The fairness is provided by scanning through the GTT space
amalgamating space in rendering order. As soon as we have a contiguous
space in the GTT large enough for the new object (and its alignment),
evict any object which lies within that space. This should keep more
objects resident in the GTT.

Doing throughput testing on a PineView machine with cairo-perf-trace
indicates that there is very little difference with the new LRU scan,
perhaps a small improvement... Except oddly for the poppler trace.

Reference:

  Bug 15911 - Intermittent X crash (freeze)
  https://bugzilla.kernel.org/show_bug.cgi?id=15911

  Bug 20152 - cannot view JPG in firefox when running UXA
  https://bugs.freedesktop.org/show_bug.cgi?id=20152

  Bug 24369 - Hang when scrolling firefox page with window in front
  https://bugs.freedesktop.org/show_bug.cgi?id=24369

  Bug 28478 - Intermittent graphics lockups due to overflow/loop
  https://bugs.freedesktop.org/show_bug.cgi?id=28478

v2: Attempt to clarify the logic and order of eviction through the use
of comments and macros.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Daniel Vetter <daniel@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_drv.h       |    2 +
 drivers/gpu/drm/i915/i915_gem_evict.c |  279 +++++++++++++++++----------------
 2 files changed, 147 insertions(+), 134 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 12c8f47..6221f23 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -673,6 +673,8 @@ struct drm_i915_gem_object {
 	struct list_head list;
 	/** This object's place on GPU write list */
 	struct list_head gpu_write_list;
+	/** This object's place on eviction list */
+	struct list_head evict_list;
 
 	/**
 	 * This is set if the object is on the active or flushing lists
diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c
index 479e450..72cae3c 100644
--- a/drivers/gpu/drm/i915/i915_gem_evict.c
+++ b/drivers/gpu/drm/i915/i915_gem_evict.c
@@ -31,167 +31,178 @@
 #include "i915_drv.h"
 #include "i915_drm.h"
 
-static inline int
-i915_gem_object_is_purgeable(struct drm_i915_gem_object *obj_priv)
-{
-	return obj_priv->madv == I915_MADV_DONTNEED;
-}
-
-static int
-i915_gem_scan_inactive_list_and_evict(struct drm_device *dev, int min_size,
-				      unsigned alignment, int *found)
+static struct drm_i915_gem_object *
+i915_gem_next_active_object(struct drm_device *dev,
+			    struct list_head **render_iter,
+			    struct list_head **bsd_iter)
 {
 	drm_i915_private_t *dev_priv = dev->dev_private;
-	struct drm_gem_object *obj;
-	struct drm_i915_gem_object *obj_priv;
-	struct drm_gem_object *best = NULL;
-	struct drm_gem_object *first = NULL;
-
-	/* Try to find the smallest clean object */
-	list_for_each_entry(obj_priv, &dev_priv->mm.inactive_list, list) {
-		struct drm_gem_object *obj = &obj_priv->base;
-		if (obj->size >= min_size) {
-			if ((!obj_priv->dirty ||
-			     i915_gem_object_is_purgeable(obj_priv)) &&
-			    (!best || obj->size < best->size)) {
-				best = obj;
-				if (best->size == min_size)
-					break;
-			}
-			if (!first)
-			    first = obj;
-		}
-	}
+	struct drm_i915_gem_object *render_obj = NULL, *bsd_obj = NULL;
 
-	obj = best ? best : first;
+	if (*render_iter != &dev_priv->render_ring.active_list)
+		render_obj = list_entry(*render_iter,
+					struct drm_i915_gem_object,
+					list);
 
-	if (!obj) {
-		*found = 0;
-		return 0;
-	}
+	if (HAS_BSD(dev)) {
+		if (*bsd_iter != &dev_priv->bsd_ring.active_list)
+			bsd_obj = list_entry(*bsd_iter,
+					     struct drm_i915_gem_object,
+					     list);
 
-	*found = 1;
+		if (render_obj == NULL) {
+			*bsd_iter = (*bsd_iter)->next;
+			return bsd_obj;
+		}
 
-#if WATCH_LRU
-	DRM_INFO("%s: evicting %p\n", __func__, obj);
-#endif
-	obj_priv = to_intel_bo(obj);
-	BUG_ON(obj_priv->pin_count != 0);
-	BUG_ON(obj_priv->active);
+		if (bsd_obj == NULL) {
+			*render_iter = (*render_iter)->next;
+			return render_obj;
+		}
 
-	/* Wait on the rendering and unbind the buffer. */
-	return i915_gem_object_unbind(obj);
+		/* XXX can we handle seqno wrapping? */
+		if (render_obj->last_rendering_seqno < bsd_obj->last_rendering_seqno) {
+			*render_iter = (*render_iter)->next;
+			return render_obj;
+		} else {
+			*bsd_iter = (*bsd_iter)->next;
+			return bsd_obj;
+		}
+	} else {
+		*render_iter = (*render_iter)->next;
+		return render_obj;
+	}
 }
 
-static void
-i915_gem_flush_ring(struct drm_device *dev,
-	       uint32_t invalidate_domains,
-	       uint32_t flush_domains,
-	       struct intel_ring_buffer *ring)
+static bool
+mark_free(struct drm_i915_gem_object *obj_priv,
+	   struct list_head *unwind)
 {
-	if (flush_domains & I915_GEM_DOMAIN_CPU)
-		drm_agp_chipset_flush(dev);
-	ring->flush(dev, ring,
-			invalidate_domains,
-			flush_domains);
+	list_add(&obj_priv->evict_list, unwind);
+	return drm_mm_scan_add_block(obj_priv->gtt_space);
 }
 
+#define i915_for_each_active_object(OBJ, R, B) \
+	*(R) = dev_priv->render_ring.active_list.next; \
+	*(B) = dev_priv->bsd_ring.active_list.next; \
+	while (((OBJ) = i915_gem_next_active_object(dev, (R), (B))) != NULL)
+
 int
-i915_gem_evict_something(struct drm_device *dev,
-			 int min_size, unsigned alignment)
+i915_gem_evict_something(struct drm_device *dev, int min_size, unsigned alignment)
 {
 	drm_i915_private_t *dev_priv = dev->dev_private;
-	int ret, found;
-
-	struct intel_ring_buffer *render_ring = &dev_priv->render_ring;
-	struct intel_ring_buffer *bsd_ring = &dev_priv->bsd_ring;
-	for (;;) {
-		i915_gem_retire_requests(dev);
-
-		/* If there's an inactive buffer available now, grab it
-		 * and be done.
-		 */
-		ret = i915_gem_scan_inactive_list_and_evict(dev, min_size,
-							    alignment,
-							    &found);
-		if (found)
-			return ret;
+	struct list_head eviction_list, unwind_list;
+	struct drm_i915_gem_object *obj_priv, *tmp_obj_priv;
+	struct list_head *render_iter, *bsd_iter;
+	int ret = 0;
 
-		/* If we didn't get anything, but the ring is still processing
-		 * things, wait for the next to finish and hopefully leave us
-		 * a buffer to evict.
-		 */
-		if (!list_empty(&render_ring->request_list)) {
-			struct drm_i915_gem_request *request;
+	i915_gem_retire_requests(dev);
 
-			request = list_first_entry(&render_ring->request_list,
-						   struct drm_i915_gem_request,
-						   list);
+	/* Re-check for free space after retiring requests */
+	if (drm_mm_search_free(&dev_priv->mm.gtt_space,
+			       min_size, alignment, 0))
+		return 0;
 
-			ret = i915_do_wait_request(dev, request->seqno, true, request->ring);
-			if (ret)
-				return ret;
+	/*
+	 * The goal is to evict objects and amalgamate space in LRU order.
+	 * The oldest idle objects reside on the inactive list, which is in
+	 * retirement order. The next objects to retire are those on the (per
+	 * ring) active list that do not have an outstanding flush. Once the
+	 * hardware reports completion (the seqno is updated after the
+	 * batchbuffer has been finished) the clean buffer objects would
+	 * be retired to the inactive list. Any dirty objects would be added
+	 * to the tail of the flushing list. So after processing the clean
+	 * active objects we need to emit a MI_FLUSH to retire the flushing
+	 * list, hence the retirement order of the flushing list is in
+	 * advance of the dirty objects on the active lists.
+	 *
+	 * The retirement sequence is thus:
+	 *   1. Inactive objects (already retired)
+	 *   2. Clean active objects
+	 *   3. Flushing list
+	 *   4. Dirty active objects.
+	 *
+	 * On each list, the oldest objects lie at the HEAD with the freshest
+	 * object on the TAIL.
+	 */
+
+	INIT_LIST_HEAD(&unwind_list);
+	drm_mm_init_scan(&dev_priv->mm.gtt_space, min_size, alignment);
+
+	/* First see if there is a large enough contiguous idle region... */
+	list_for_each_entry(obj_priv, &dev_priv->mm.inactive_list, list) {
+		if (mark_free(obj_priv, &unwind_list))
+			goto found;
+	}
 
+	/* Now merge in the soon-to-be-expired objects... */
+	i915_for_each_active_object(obj_priv, &render_iter, &bsd_iter) {
+		/* Does the object require an outstanding flush? */
+		if (obj_priv->base.write_domain || obj_priv->pin_count)
 			continue;
-		}
 
-		if (HAS_BSD(dev) && !list_empty(&bsd_ring->request_list)) {
-			struct drm_i915_gem_request *request;
-
-			request = list_first_entry(&bsd_ring->request_list,
-						   struct drm_i915_gem_request,
-						   list);
+		if (mark_free(obj_priv, &unwind_list))
+			goto found;
+	}
 
-			ret = i915_do_wait_request(dev, request->seqno, true, request->ring);
-			if (ret)
-				return ret;
+	/* Finally add anything with a pending flush (in order of retirement) */
+	list_for_each_entry(obj_priv, &dev_priv->mm.flushing_list, list) {
+		if (obj_priv->pin_count)
+			continue;
 
+		if (mark_free(obj_priv, &unwind_list))
+			goto found;
+	}
+	i915_for_each_active_object(obj_priv, &render_iter, &bsd_iter) {
+		if (! obj_priv->base.write_domain || obj_priv->pin_count)
 			continue;
-		}
 
-		/* If we didn't have anything on the request list but there
-		 * are buffers awaiting a flush, emit one and try again.
-		 * When we wait on it, those buffers waiting for that flush
-		 * will get moved to inactive.
-		 */
-		if (!list_empty(&dev_priv->mm.flushing_list)) {
-			struct drm_gem_object *obj = NULL;
-			struct drm_i915_gem_object *obj_priv;
-
-			/* Find an object that we can immediately reuse */
-			list_for_each_entry(obj_priv, &dev_priv->mm.flushing_list, list) {
-				obj = &obj_priv->base;
-				if (obj->size >= min_size)
-					break;
-
-				obj = NULL;
-			}
-
-			if (obj != NULL) {
-				uint32_t seqno;
-
-				i915_gem_flush_ring(dev,
-					       obj->write_domain,
-					       obj->write_domain,
-					       obj_priv->ring);
-				seqno = i915_add_request(dev, NULL,
-						obj->write_domain,
-						obj_priv->ring);
-				if (seqno == 0)
-					return -ENOMEM;
-				continue;
-			}
+		if (mark_free(obj_priv, &unwind_list))
+			goto found;
+	}
+
+	/* Nothing found, clean up and bail out! */
+	list_for_each_entry(obj_priv, &unwind_list, evict_list) {
+		ret = drm_mm_scan_remove_block(obj_priv->gtt_space);
+		BUG_ON(ret);
+	}
+
+	/* We expect the caller to unpin, evict all and try again, or give up.
+	 * So calling i915_gem_evict_everything() is unnecessary.
+	 */
+	return -ENOSPC;
+
+found:
+	INIT_LIST_HEAD(&eviction_list);
+	list_for_each_entry_safe(obj_priv, tmp_obj_priv,
+				 &unwind_list, evict_list) {
+		if (drm_mm_scan_remove_block(obj_priv->gtt_space)) {
+			/* drm_mm doesn't allow any other other operations while
+			 * scanning, therefore store to be evicted objects on a
+			 * temporary list. */
+			list_move(&obj_priv->evict_list, &eviction_list);
 		}
+	}
 
-		/* If we didn't do any of the above, there's no single buffer
-		 * large enough to swap out for the new one, so just evict
-		 * everything and start again. (This should be rare.)
-		 */
-		if (!list_empty(&dev_priv->mm.inactive_list))
-			return i915_gem_evict_inactive(dev);
-		else
-			return i915_gem_evict_everything(dev);
+	/* Unbinding will emit any required flushes */
+	list_for_each_entry_safe(obj_priv, tmp_obj_priv,
+				 &eviction_list, evict_list) {
+#if WATCH_LRU
+		DRM_INFO("%s: evicting %p\n", __func__, obj);
+#endif
+		ret = i915_gem_object_unbind(&obj_priv->base);
+		if (ret)
+			return ret;
 	}
+
+	/* The just created free hole should be on the top of the free stack
+	 * maintained by drm_mm, so this BUG_ON actually executes in O(1).
+	 * Furthermore all accessed data has just recently been used, so it
+	 * should be really fast, too. */
+	BUG_ON(!drm_mm_search_free(&dev_priv->mm.gtt_space, min_size,
+				   alignment, 0));
+
+	return 0;
 }
 
 int
-- 
1.7.1

  parent reply	other threads:[~2010-08-07 10:02 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-08-07 10:01 Rebased series on drm-intel-next Chris Wilson
2010-08-07 10:01 ` [PATCH 01/20] drm/i915: Append the object onto the inactive list on binding Chris Wilson
2010-08-07 10:01 ` [PATCH 02/20] drm/i915: prepare for fair lru eviction Chris Wilson
2010-08-07 10:01 ` [PATCH 03/20] drm/i915: Use a common seqno for all rings Chris Wilson
2010-08-07 10:01 ` [PATCH 04/20] drm/i915: Move the eviction logic to its own file Chris Wilson
2010-08-07 10:01 ` Chris Wilson [this message]
2010-08-07 10:01 ` [PATCH 06/20] drm/i915: Maintain LRU order of inactive objects upon access by CPU Chris Wilson
2010-08-07 19:34   ` Daniel Vetter
2010-08-07 20:45     ` [PATCH] drm/i915: Maintain LRU order of inactive objects upon access by CPU (v2) Chris Wilson
2010-08-07 10:01 ` [PATCH 07/20] drm/i915: Record error batch buffers using iomem Chris Wilson
2010-08-07 10:01 ` [PATCH 08/20] drm/i915/sdvo: Markup a few constant strings Chris Wilson
2010-08-07 10:01 ` [PATCH 09/20] drm/i915: Enable aspect/centering panel fitting for Ironlake Chris Wilson
2010-08-07 10:01 ` [PATCH 10/20] drm/i915: Write to display base last Chris Wilson
2010-08-07 10:01 ` [PATCH 11/20] drm/i915: Truncate the shmem backing pages on purge Chris Wilson
2010-08-07 10:01 ` [PATCH 12/20] drm/i915/display: Add pipe/plane information to dpms debugging Chris Wilson
2010-08-07 10:01 ` [PATCH 13/20] drm/i915/opregion: Use ASLE response codes defined in 0.1 Chris Wilson
2010-08-07 10:01 ` [PATCH 14/20] drm/i915: Update watermarks for Ironlake after dpms changes Chris Wilson
2010-08-07 10:01 ` [PATCH 15/20] drm/i915/ringbuffer: Set ring->gem_buffer = NULL on init unwind Chris Wilson
2010-08-07 10:01 ` [PATCH 16/20] drm/i915: Ensure that while(INREG()) are bounded (v2) Chris Wilson
2010-09-21 14:22   ` Carlos R. Mafra
2010-09-21 14:35     ` Chris Wilson
2010-09-21 15:55       ` Jesse Barnes
2010-09-22  8:32         ` Carlos R. Mafra
2010-09-22  8:43           ` Chris Wilson
2010-09-22 10:48             ` Carlos R. Mafra
2010-09-22 10:57               ` Chris Wilson
2010-09-22 13:42                 ` Carlos R. Mafra
2010-09-22 14:29                   ` Chris Wilson
2010-09-22 15:26                     ` Carlos R. Mafra
2010-09-22 15:47                       ` Chris Wilson
2010-08-07 10:01 ` [PATCH 17/20] drm/i915/edp: Flush the write before waiting for PLLs Chris Wilson
2010-08-07 10:01 ` [PATCH 18/20] drm/i915: FBC is updated within set_base() so remove second call in mode_set() Chris Wilson
2010-08-07 10:01 ` [PATCH 19/20] drm/i915: Only update i845/i865 CURBASE when disabled (v2) Chris Wilson
2010-08-07 10:01 ` [PATCH 20/20] drm/i915: Apply i830 errata for cursor alignment Chris Wilson
2010-08-08 18:31   ` Eric Anholt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1281175299-1379-6-git-send-email-chris@chris-wilson.co.uk \
    --to=chris@chris-wilson.co.uk \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.