[1/6] drm/i915: Stop tracking timeline->inflight_seqnos

Commit Message

Chris Wilson April 23, 2018, 10:13 a.m. UTC

In commit 9b6586ae9f6b ("drm/i915: Keep a global seqno per-engine"), we
moved from a global inflight counter to per-engine counters in the
hope that will be easy to run concurrently in future. However, with the
advent of the desire to move requests between engines, we do need a
global counter to preserve the semantics that no engine wraps in the
middle of a submit. (Although this semantic is now only required for gen7
semaphore support, which only supports greater-then comparisons!)

References: 9b6586ae9f6b ("drm/i915: Keep a global seqno per-engine")
Signed-off-by: Chris Wilson <[e-mail]>
Cc: Tvrtko Ursulin <[e-mail]>
Reviewed-by: Tvrtko Ursulin <[e-mail]>
---
 drivers/gpu/drm/i915/i915_debugfs.c      |  5 ++---
 drivers/gpu/drm/i915/i915_gem_timeline.h |  6 ------
 drivers/gpu/drm/i915/i915_request.c      | 12 +++---------
 drivers/gpu/drm/i915/intel_engine_cs.c   |  5 ++---
 4 files changed, 7 insertions(+), 21 deletions(-)

Comments

Chris Wilson April 23, 2018, 1:49 p.m. UTC | #1

Quoting Patchwork (2018-04-23 14:40:51)
>     igt@core_auth@basic-auth:
>       fi-glk-j4005:       PASS -> INCOMPLETE (k.org#198133, fdo#103359)
>       fi-bdw-gvtdvm:      PASS -> INCOMPLETE (fdo#105600)
>       fi-bxt-dsi:         PASS -> INCOMPLETE (fdo#103927)
>       fi-cnl-psr:         PASS -> INCOMPLETE (fdo#105086)
>       fi-cnl-y3:          PASS -> INCOMPLETE (fdo#105086)
>       fi-bxt-j4205:       PASS -> INCOMPLETE (fdo#103927)
>       fi-skl-gvtdvm:      PASS -> INCOMPLETE (fdo#105600)

Oh sausage. engine->last_retired_context needs to be in engine order,
but ring->head needs to be in ring order.
-Chris

Patch

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 2f05f5262bba..547e97102b85 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1340,10 +1340,9 @@  static int i915_hangcheck_info(struct seq_file *m, void *unused)
 		struct rb_node *rb;
 
 		seq_printf(m, "%s:\n", engine->name);
-		seq_printf(m, "\tseqno = %x [current %x, last %x], inflight %d\n",
+		seq_printf(m, "\tseqno = %x [current %x, last %x]\n",
 			   engine->hangcheck.seqno, seqno[id],
-			   intel_engine_last_submit(engine),
-			   engine->timeline->inflight_seqnos);
+			   intel_engine_last_submit(engine));
 		seq_printf(m, "\twaiters? %s, fake irq active? %s, stalled? %s\n",
 			   yesno(intel_engine_has_waiter(engine)),
 			   yesno(test_bit(engine->id,
diff --git a/drivers/gpu/drm/i915/i915_gem_timeline.h b/drivers/gpu/drm/i915/i915_gem_timeline.h
index 33e01bf6aa36..6e82119e2cd8 100644
--- a/drivers/gpu/drm/i915/i915_gem_timeline.h
+++ b/drivers/gpu/drm/i915/i915_gem_timeline.h
@@ -37,12 +37,6 @@  struct intel_timeline {
 	u64 fence_context;
 	u32 seqno;
 
-	/**
-	 * Count of outstanding requests, from the time they are constructed
-	 * to the moment they are retired. Loosely coupled to hardware.
-	 */
-	u32 inflight_seqnos;
-
 	spinlock_t lock;
 
 	/**
diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index b692a9f7c357..1437538d5bfa 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -260,17 +260,14 @@  int i915_gem_set_global_seqno(struct drm_device *dev, u32 seqno)
 static int reserve_engine(struct intel_engine_cs *engine)
 {
 	struct drm_i915_private *i915 = engine->i915;
-	u32 active = ++engine->timeline->inflight_seqnos;
-	u32 seqno = engine->timeline->seqno;
 	int ret;
 
 	/* Reservation is fine until we need to wrap around */
-	if (unlikely(add_overflows(seqno, active))) {
+	if (unlikely(add_overflows(engine->timeline->seqno,
+				   i915->gt.active_requests + 1))) {
 		ret = reset_all_global_seqno(i915, 0);
-		if (ret) {
-			engine->timeline->inflight_seqnos--;
+		if (ret)
 			return ret;
-		}
 	}
 
 	if (!i915->gt.active_requests++)
@@ -285,9 +282,6 @@  static void unreserve_engine(struct intel_engine_cs *engine)
 
 	if (!--i915->gt.active_requests)
 		i915_gem_park(i915);
-
-	GEM_BUG_ON(!engine->timeline->inflight_seqnos);
-	engine->timeline->inflight_seqnos--;
 }
 
 void i915_gem_retire_noop(struct i915_gem_active *active,
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index be608f7111f5..a55a849b81b6 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -1318,12 +1318,11 @@  void intel_engine_dump(struct intel_engine_cs *engine,
 	if (i915_terminally_wedged(&engine->i915->gpu_error))
 		drm_printf(m, "*** WEDGED ***\n");
 
-	drm_printf(m, "\tcurrent seqno %x, last %x, hangcheck %x [%d ms], inflight %d\n",
+	drm_printf(m, "\tcurrent seqno %x, last %x, hangcheck %x [%d ms]\n",
 		   intel_engine_get_seqno(engine),
 		   intel_engine_last_submit(engine),
 		   engine->hangcheck.seqno,
-		   jiffies_to_msecs(jiffies - engine->hangcheck.action_timestamp),
-		   engine->timeline->inflight_seqnos);
+		   jiffies_to_msecs(jiffies - engine->hangcheck.action_timestamp));
 	drm_printf(m, "\tReset count: %d (global %d)\n",
 		   i915_reset_engine_count(error, engine),
 		   i915_reset_count(error));