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 15/18] drm/i915: Reserve space in the global seqno during request allocation
Date: Wed, 14 Sep 2016 07:52:47 +0100	[thread overview]
Message-ID: <20160914065250.15482-16-chris@chris-wilson.co.uk> (raw)
In-Reply-To: <20160914065250.15482-1-chris@chris-wilson.co.uk>

A restriction on our global seqno is that they cannot wrap, and that we
cannot use the value 0. This allows us to detect when a request has not
yet been submitted, its global seqno is still 0, and ensures that
hardware semaphores are monotonic as required by older hardware. To
meet these restrictions when we defer the assignment of the global
seqno, we must check that we have an available slot in the global seqno
space during request construction. If that test fails, we wait for all
requests to be completed and reset the hardware back to 0.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_debugfs.c      | 10 ++---
 drivers/gpu/drm/i915/i915_drv.h          |  2 +-
 drivers/gpu/drm/i915/i915_gem.c          |  6 +--
 drivers/gpu/drm/i915/i915_gem_request.c  | 74 +++++++++++++++-----------------
 drivers/gpu/drm/i915/i915_gem_timeline.h |  2 +-
 5 files changed, 44 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index f7c5e09122ab..d23047bb2fe9 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -557,7 +557,7 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data)
 				seq_printf(m, "Flip queued on %s at seqno %x, next seqno %x [current breadcrumb %x], completed? %d\n",
 					   engine->name,
 					   i915_gem_request_get_seqno(work->flip_queued_req),
-					   dev_priv->gt.global_timeline.next_seqno,
+					   atomic_read(&dev_priv->gt.global_timeline.next_seqno),
 					   intel_engine_get_seqno(engine),
 					   i915_gem_request_completed(work->flip_queued_req));
 			} else
@@ -1029,7 +1029,7 @@ i915_next_seqno_get(void *data, u64 *val)
 {
 	struct drm_i915_private *dev_priv = data;
 
-	*val = dev_priv->gt.global_timeline.next_seqno;
+	*val = atomic_read(&dev_priv->gt.global_timeline.next_seqno);
 	return 0;
 }
 
@@ -2304,8 +2304,8 @@ static int i915_rps_boost_info(struct seq_file *m, void *data)
 	struct drm_file *file;
 
 	seq_printf(m, "RPS enabled? %d\n", dev_priv->rps.enabled);
-	seq_printf(m, "GPU busy? %s [%x]\n",
-		   yesno(dev_priv->gt.awake), dev_priv->gt.active_engines);
+	seq_printf(m, "GPU busy? %s [%d requests]\n",
+		   yesno(dev_priv->gt.awake), dev_priv->gt.active_requests);
 	seq_printf(m, "CPU waiting? %d\n", count_irq_waiters(dev_priv));
 	seq_printf(m, "Frequency requested %d\n",
 		   intel_gpu_freq(dev_priv, dev_priv->rps.cur_freq));
@@ -2340,7 +2340,7 @@ static int i915_rps_boost_info(struct seq_file *m, void *data)
 
 	if (INTEL_GEN(dev_priv) >= 6 &&
 	    dev_priv->rps.enabled &&
-	    dev_priv->gt.active_engines) {
+	    dev_priv->gt.active_requests) {
 		u32 rpup, rpupei;
 		u32 rpdown, rpdownei;
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index cc5fa593e4c4..45fd20af0c6d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2048,6 +2048,7 @@ struct drm_i915_private {
 
 		struct list_head timelines;
 		struct i915_gem_timeline global_timeline;
+		u32 active_requests;
 
 		/**
 		 * Is the GPU currently considered idle, or busy executing
@@ -2056,7 +2057,6 @@ struct drm_i915_private {
 		 * In order to reduce the effect on performance, there
 		 * is a slight delay before we do so.
 		 */
-		unsigned int active_engines;
 		bool awake;
 
 		/**
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 76d6fc146adf..cd9d3a2f72e7 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2714,8 +2714,6 @@ static void i915_gem_cleanup_engine(struct intel_engine_cs *engine)
 		memset(engine->execlist_port, 0, sizeof(engine->execlist_port));
 		spin_unlock(&engine->execlist_lock);
 	}
-
-	engine->i915->gt.active_engines &= ~intel_engine_flag(engine);
 }
 
 void i915_gem_set_wedged(struct drm_i915_private *dev_priv)
@@ -2770,7 +2768,7 @@ i915_gem_idle_work_handler(struct work_struct *work)
 	if (!READ_ONCE(dev_priv->gt.awake))
 		return;
 
-	if (READ_ONCE(dev_priv->gt.active_engines))
+	if (READ_ONCE(dev_priv->gt.active_requests))
 		return;
 
 	rearm_hangcheck =
@@ -2784,7 +2782,7 @@ i915_gem_idle_work_handler(struct work_struct *work)
 		goto out_rearm;
 	}
 
-	if (dev_priv->gt.active_engines)
+	if (dev_priv->gt.active_requests)
 		goto out_unlock;
 
 	for_each_engine(engine, dev_priv)
diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
index 74e1a875988c..bd84bb757e39 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/i915_gem_request.c
@@ -156,6 +156,7 @@ static void i915_gem_request_retire(struct drm_i915_gem_request *request)
 	 */
 	list_del(&request->ring_link);
 	request->ring->last_retired_head = request->postfix;
+	request->i915->gt.active_requests--;
 
 	/* Walk through the active list, calling retire on each. This allows
 	 * objects to track their GPU activity and mark themselves as idle
@@ -253,7 +254,7 @@ static int i915_gem_init_global_seqno(struct drm_i915_private *dev_priv,
 
 	/* If the seqno wraps around, we need to clear the breadcrumb rbtree */
 	if (!i915_seqno_passed(seqno,
-			       dev_priv->gt.global_timeline.next_seqno)) {
+			       atomic_read(&dev_priv->gt.global_timeline.next_seqno))) {
 		while (intel_kick_waiters(dev_priv) ||
 		       intel_kick_signalers(dev_priv))
 			yield();
@@ -269,13 +270,13 @@ static int i915_gem_init_global_seqno(struct drm_i915_private *dev_priv,
 			       sizeof(tl->engine[i].sync_seqno));
 	}
 
+	atomic_set(&dev_priv->gt.global_timeline.next_seqno, seqno);
 	return 0;
 }
 
 int i915_gem_set_global_seqno(struct drm_device *dev, u32 seqno)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
-	int ret;
 
 	if (seqno == 0)
 		return -EINVAL;
@@ -283,34 +284,32 @@ int i915_gem_set_global_seqno(struct drm_device *dev, u32 seqno)
 	/* HWS page needs to be set less than what we
 	 * will inject to ring
 	 */
-	ret = i915_gem_init_global_seqno(dev_priv, seqno - 1);
-	if (ret)
-		return ret;
-
-	dev_priv->gt.global_timeline.next_seqno = seqno;
-	return 0;
+	return i915_gem_init_global_seqno(dev_priv, seqno - 1);
 }
 
-static int i915_gem_get_global_seqno(struct drm_i915_private *dev_priv,
-				     u32 *seqno)
+static int reserve_global_seqno(struct drm_i915_private *i915)
 {
-	struct i915_gem_timeline *tl = &dev_priv->gt.global_timeline;
+	struct i915_gem_timeline *tl = &i915->gt.global_timeline;
+	u32 next_seqno = atomic_read(&tl->next_seqno);
 
-	/* reserve 0 for non-seqno */
-	if (unlikely(tl->next_seqno == 0)) {
+	if (unlikely(next_seqno + ++i915->gt.active_requests <= next_seqno)) {
 		int ret;
 
-		ret = i915_gem_init_global_seqno(dev_priv, 0);
-		if (ret)
+		ret = i915_gem_init_global_seqno(i915, 0);
+		if (ret) {
+			i915->gt.active_requests--;
 			return ret;
-
-		tl->next_seqno = 1;
+		}
 	}
 
-	*seqno = tl->next_seqno++;
 	return 0;
 }
 
+static u32 timeline_get_seqno(struct i915_gem_timeline *tl)
+{
+	return atomic_inc_return(&tl->next_seqno);
+}
+
 static int __i915_sw_fence_call
 submit_notify(struct i915_sw_fence *fence, enum i915_sw_fence_notify state)
 {
@@ -349,7 +348,6 @@ i915_gem_request_alloc(struct intel_engine_cs *engine,
 {
 	struct drm_i915_private *dev_priv = engine->i915;
 	struct drm_i915_gem_request *req;
-	u32 seqno;
 	int ret;
 
 	/* ABI: Before userspace accesses the GPU (e.g. execbuffer), report
@@ -360,6 +358,10 @@ i915_gem_request_alloc(struct intel_engine_cs *engine,
 	if (ret)
 		return ERR_PTR(ret);
 
+	ret = reserve_global_seqno(dev_priv);
+	if (ret)
+		return ERR_PTR(ret);
+
 	/* Move the oldest request to the slab-cache (if not in use!) */
 	req = list_first_entry_or_null(&engine->timeline->requests,
 				       typeof(*req), link);
@@ -395,12 +397,10 @@ i915_gem_request_alloc(struct intel_engine_cs *engine,
 	 * Do not use kmem_cache_zalloc() here!
 	 */
 	req = kmem_cache_alloc(dev_priv->requests, GFP_KERNEL);
-	if (!req)
-		return ERR_PTR(-ENOMEM);
-
-	ret = i915_gem_get_global_seqno(dev_priv, &seqno);
-	if (ret)
-		goto err;
+	if (!req) {
+		ret = -ENOMEM;
+		goto err_unreserve;
+	}
 
 	req->timeline = engine->timeline;
 
@@ -409,14 +409,14 @@ i915_gem_request_alloc(struct intel_engine_cs *engine,
 		   &i915_fence_ops,
 		   &req->lock,
 		   req->timeline->fence_context,
-		   seqno);
+		   timeline_get_seqno(req->timeline->common));
 
 	i915_sw_fence_init(&req->submit, submit_notify);
 
 	INIT_LIST_HEAD(&req->active_list);
 	req->i915 = dev_priv;
 	req->engine = engine;
-	req->global_seqno = seqno;
+	req->global_seqno = req->fence.seqno;
 	req->ctx = i915_gem_context_get(ctx);
 
 	/* No zalloc, must clear what we need by hand */
@@ -452,8 +452,9 @@ i915_gem_request_alloc(struct intel_engine_cs *engine,
 
 err_ctx:
 	i915_gem_context_put(ctx);
-err:
 	kmem_cache_free(dev_priv->requests, req);
+err_unreserve:
+	dev_priv->gt.active_requests--;
 	return ERR_PTR(ret);
 }
 
@@ -608,7 +609,6 @@ static void i915_gem_mark_busy(const struct intel_engine_cs *engine)
 {
 	struct drm_i915_private *dev_priv = engine->i915;
 
-	dev_priv->gt.active_engines |= intel_engine_flag(engine);
 	if (dev_priv->gt.awake)
 		return;
 
@@ -936,38 +936,34 @@ complete:
 	return timeout;
 }
 
-static bool engine_retire_requests(struct intel_engine_cs *engine)
+static void engine_retire_requests(struct intel_engine_cs *engine)
 {
 	struct drm_i915_gem_request *request, *next;
 
 	list_for_each_entry_safe(request, next,
 				 &engine->timeline->requests, link) {
 		if (!__i915_gem_request_completed(request))
-			return false;
+			return;
 
 		i915_gem_request_retire(request);
 	}
-
-	return true;
 }
 
 void i915_gem_retire_requests(struct drm_i915_private *dev_priv)
 {
 	struct intel_engine_cs *engine;
-	unsigned int tmp;
 
 	lockdep_assert_held(&dev_priv->drm.struct_mutex);
 
-	if (dev_priv->gt.active_engines == 0)
+	if (!dev_priv->gt.active_requests)
 		return;
 
 	GEM_BUG_ON(!dev_priv->gt.awake);
 
-	for_each_engine_masked(engine, dev_priv, dev_priv->gt.active_engines, tmp)
-		if (engine_retire_requests(engine))
-			dev_priv->gt.active_engines &= ~intel_engine_flag(engine);
+	for_each_engine(engine, dev_priv)
+		engine_retire_requests(engine);
 
-	if (dev_priv->gt.active_engines == 0)
+	if (!dev_priv->gt.active_requests)
 		queue_delayed_work(dev_priv->wq,
 				   &dev_priv->gt.idle_work,
 				   msecs_to_jiffies(100));
diff --git a/drivers/gpu/drm/i915/i915_gem_timeline.h b/drivers/gpu/drm/i915/i915_gem_timeline.h
index 96c765ba3abc..0fbcf8a37076 100644
--- a/drivers/gpu/drm/i915/i915_gem_timeline.h
+++ b/drivers/gpu/drm/i915/i915_gem_timeline.h
@@ -54,7 +54,7 @@ struct intel_timeline {
 
 struct i915_gem_timeline {
 	struct list_head link;
-	u32 next_seqno;
+	atomic_t next_seqno;
 
 	struct drm_i915_private *i915;
 	const char *name;
-- 
2.9.3

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

  parent reply	other threads:[~2016-09-14  6:53 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-14  6:52 Tracking multiple timelines (full-ppgtt) Chris Wilson
2016-09-14  6:52 ` [PATCH 01/18] drm/i915: Support asynchronous waits on struct fence from i915_gem_request Chris Wilson
2016-09-14  7:37   ` Joonas Lahtinen
2016-09-19 11:26     ` Chris Wilson
2016-09-14  6:52 ` [PATCH 02/18] drm/i915: Allow i915_sw_fence_await_sw_fence() to allocate Chris Wilson
2016-09-14  7:51   ` Joonas Lahtinen
2016-09-14  8:46     ` Chris Wilson
2016-09-14  6:52 ` [PATCH 03/18] drm/i915: Rearrange i915_wait_request() accounting with callers Chris Wilson
2016-09-14  8:47   ` Joonas Lahtinen
2016-09-14  6:52 ` [PATCH 04/18] drm/i915: Remove unused i915_gem_active_wait() in favour of _unlocked() Chris Wilson
2016-09-14  8:48   ` Joonas Lahtinen
2016-09-14  6:52 ` [PATCH 05/18] drm/i915: Move GEM activity tracking into a common struct reservation_object Chris Wilson
2016-09-14  9:44   ` Joonas Lahtinen
2016-09-14 17:35     ` Chris Wilson
2016-09-15  9:38       ` Dave Gordon
2016-09-15  9:55         ` Jani Nikula
2016-09-16 11:40   ` Chris Wilson
2016-09-14  6:52 ` [PATCH 06/18] drm: Add reference counting to drm_atomic_state Chris Wilson
2016-09-14  6:52 ` [PATCH 07/18] drm/i915: Restore nonblocking awaits for modesetting Chris Wilson
2016-09-19 16:01   ` Joonas Lahtinen
2016-09-14  6:52 ` [PATCH 08/18] drm/i915: Combine seqno + tracking into a global timeline struct Chris Wilson
2016-09-14 15:42   ` Joonas Lahtinen
2016-09-14  6:52 ` [PATCH 09/18] drm/i915: Wait first for submission, before waiting for request completion Chris Wilson
2016-09-19  8:59   ` Joonas Lahtinen
2016-09-14  6:52 ` [PATCH 10/18] drm/i915: Introduce a global_seqno for each request Chris Wilson
2016-09-19 10:36   ` Joonas Lahtinen
2016-09-14  6:52 ` [PATCH 11/18] drm/i915: Record space required for request emission Chris Wilson
2016-09-14 13:30   ` Tvrtko Ursulin
2016-09-14 17:33     ` Chris Wilson
2016-09-15  8:59       ` Tvrtko Ursulin
2016-09-19 10:47   ` Joonas Lahtinen
2016-09-19 11:32     ` Chris Wilson
2016-09-19 16:09       ` Joonas Lahtinen
2016-09-14  6:52 ` [PATCH 12/18] drm/i915: Defer " Chris Wilson
2016-09-19 12:06   ` Joonas Lahtinen
2016-09-14  6:52 ` [PATCH 13/18] drm/i915: Move the global sync optimisation to the timeline Chris Wilson
2016-09-19 13:16   ` Joonas Lahtinen
2016-09-14  6:52 ` [PATCH 14/18] drm/i915: Create a unique name for the context Chris Wilson
2016-09-19 13:23   ` Joonas Lahtinen
2016-09-14  6:52 ` Chris Wilson [this message]
2016-09-19 13:47   ` [PATCH 15/18] drm/i915: Reserve space in the global seqno during request allocation Joonas Lahtinen
2016-09-19 15:35     ` Jani Nikula
2016-09-19 16:07       ` Joonas Lahtinen
2016-09-14  6:52 ` [PATCH 16/18] drm/i915: Enable multiple timelines Chris Wilson
2016-09-19 15:52   ` Joonas Lahtinen
2016-10-20 12:49     ` Chris Wilson
2016-10-20 15:25       ` Joonas Lahtinen
2016-09-14  6:52 ` [PATCH 17/18] drm/i915: Enable userspace to opt-out of implicit fencing Chris Wilson
2016-09-14  6:52 ` [PATCH 18/18] drm/i915: Support explicit fencing for execbuf Chris Wilson
2016-09-14  9:16 ` ✗ Fi.CI.BAT: failure for series starting with [01/18] drm/i915: Support asynchronous waits on struct fence from i915_gem_request Patchwork

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=20160914065250.15482-16-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.