All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/i915: Assert the engine is idle before overwiting the HWS
@ 2017-04-05 15:30 Chris Wilson
  2017-04-05 15:30 ` [PATCH 2/2] drm/i915: Advance ring->head fully when idle Chris Wilson
                   ` (5 more replies)
  0 siblings, 6 replies; 12+ messages in thread
From: Chris Wilson @ 2017-04-05 15:30 UTC (permalink / raw)
  To: intel-gfx

When we update the global seqno (on the engine timeline), we modify HW
state (both registers and mapped pages). As we do this, we should be
sure that the HW is idle and we are not causing a conflict. The caller
is supposed to wait_for_idle before calling us to update the seqno, so
let's assert they have and the engine is indeed idle.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem_request.c | 2 +-
 drivers/gpu/drm/i915/intel_engine_cs.c  | 4 ++++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
index 6348353b91ec..2f8c5132b54e 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/i915_gem_request.c
@@ -214,8 +214,8 @@ static int reset_all_global_seqno(struct drm_i915_private *i915, u32 seqno)
 		}
 
 		/* Finally reset hw state */
-		tl->seqno = seqno;
 		intel_engine_init_global_seqno(engine, seqno);
+		tl->seqno = seqno;
 
 		list_for_each_entry(timeline, &i915->gt.timelines, link)
 			memset(timeline->engine[id].sync_seqno, 0,
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index 854e8e0c836b..92f871cf3265 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -223,6 +223,8 @@ void intel_engine_init_global_seqno(struct intel_engine_cs *engine, u32 seqno)
 {
 	struct drm_i915_private *dev_priv = engine->i915;
 
+	GEM_BUG_ON(!intel_engine_is_idle(engine));
+
 	/* Our semaphore implementation is strictly monotonic (i.e. we proceed
 	 * so long as the semaphore value in the register/page is greater
 	 * than the sync value), so whenever we reset the seqno,
@@ -260,6 +262,8 @@ void intel_engine_init_global_seqno(struct intel_engine_cs *engine, u32 seqno)
 	 * there are any waiters for that seqno.
 	 */
 	intel_engine_wakeup(engine);
+
+	GEM_BUG_ON(intel_engine_get_seqno(engine) != seqno);
 }
 
 static void intel_engine_init_timeline(struct intel_engine_cs *engine)
-- 
2.11.0

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/2] drm/i915: Advance ring->head fully when idle
  2017-04-05 15:30 [PATCH 1/2] drm/i915: Assert the engine is idle before overwiting the HWS Chris Wilson
@ 2017-04-05 15:30 ` Chris Wilson
  2017-04-05 16:06   ` [PATCH v2] " Chris Wilson
  2017-04-05 15:53 ` ✗ Fi.CI.BAT: failure for series starting with [1/2] drm/i915: Assert the engine is idle before overwiting the HWS Patchwork
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Chris Wilson @ 2017-04-05 15:30 UTC (permalink / raw)
  To: intel-gfx

When we retire the last request on the ring, before we ever access that
ring again we know it will be completely idle and so we can advance the
ring->head fully to the end (i.e. ring->tail) and not just to the start
of the breadcrumb. This allows us to skip re-emitting the breadcrumb
after resetting the GPU if the ring was entirely idle. This prevents us
from overwriting a seqno wraparound by re-executing a stale breadcrumb,
i.e.
	submit_request(1)
	intel_engine_init_global_seqno(0)
	i915_reset()
would then leave 1 in the HWS, but the next request to execute would
also be with seqno 1. The sanity checks upon submission detect this as a
timewarp and explode. By setting the ring as empty, upon reset the HWS
is left as 0, leaving it consistent with the timeline.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100144
Testcase: igt/gem_exec_whisper/hang-*
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem_request.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
index 2f8c5132b54e..2cc090ee1440 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/i915_gem_request.c
@@ -295,8 +295,11 @@ static void i915_gem_request_retire(struct drm_i915_gem_request *request)
 	 * Note this requires that we are always called in request
 	 * completion order.
 	 */
+	if (request->ring_link.prev == &request->ring->request_list)
+		request->ring->head = request->ring->tail;
+	else
+		request->ring->head = request->postfix;
 	list_del(&request->ring_link);
-	request->ring->head = request->postfix;
 	if (!--request->i915->gt.active_requests) {
 		GEM_BUG_ON(!request->i915->gt.awake);
 		mod_delayed_work(request->i915->wq,
-- 
2.11.0

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* ✗ Fi.CI.BAT: failure for series starting with [1/2] drm/i915: Assert the engine is idle before overwiting the HWS
  2017-04-05 15:30 [PATCH 1/2] drm/i915: Assert the engine is idle before overwiting the HWS Chris Wilson
  2017-04-05 15:30 ` [PATCH 2/2] drm/i915: Advance ring->head fully when idle Chris Wilson
@ 2017-04-05 15:53 ` Patchwork
  2017-04-05 16:26 ` ✗ Fi.CI.BAT: warning for series starting with [1/2] drm/i915: Assert the engine is idle before overwiting the HWS (rev2) Patchwork
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2017-04-05 15:53 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] drm/i915: Assert the engine is idle before overwiting the HWS
URL   : https://patchwork.freedesktop.org/series/22527/
State : failure

== Summary ==

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

Test gem_ctx_switch:
        Subgroup basic-default:
                pass       -> INCOMPLETE (fi-kbl-7560u)
                pass       -> INCOMPLETE (fi-bdw-5557u)
                pass       -> INCOMPLETE (fi-skl-6260u)
                pass       -> INCOMPLETE (fi-skl-6700hq)
                pass       -> INCOMPLETE (fi-skl-gvtdvm)
                pass       -> INCOMPLETE (fi-skl-6770hq)
                pass       -> INCOMPLETE (fi-kbl-7500u)
                pass       -> INCOMPLETE (fi-bxt-t5700)
                pass       -> INCOMPLETE (fi-skl-6700k)
        Subgroup basic-default-heavy:
                pass       -> INCOMPLETE (fi-bxt-j4205)
                pass       -> FAIL       (fi-snb-2520m)
                pass       -> INCOMPLETE (fi-bsw-n3050)
                pass       -> INCOMPLETE (fi-bdw-gvtdvm)
                pass       -> FAIL       (fi-snb-2600)

fi-bdw-5557u     total:21   pass:20   dwarn:0   dfail:0   fail:0   skip:0   time: 0s
fi-bdw-gvtdvm    total:22   pass:21   dwarn:0   dfail:0   fail:0   skip:0   time: 0s
fi-bsw-n3050     total:22   pass:21   dwarn:0   dfail:0   fail:0   skip:0   time: 0s
fi-bxt-j4205     total:22   pass:21   dwarn:0   dfail:0   fail:0   skip:0   time: 0s
fi-bxt-t5700     total:21   pass:20   dwarn:0   dfail:0   fail:0   skip:0   time: 0s
fi-byt-j1900     total:278  pass:254  dwarn:0   dfail:0   fail:0   skip:24  time: 485s
fi-byt-n2820     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time: 480s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 409s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 402s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time: 422s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 490s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 461s
fi-kbl-7500u     total:21   pass:20   dwarn:0   dfail:0   fail:0   skip:0   time: 0s
fi-kbl-7560u     total:21   pass:20   dwarn:0   dfail:0   fail:0   skip:0   time: 0s
fi-skl-6260u     total:21   pass:20   dwarn:0   dfail:0   fail:0   skip:0   time: 0s
fi-skl-6700hq    total:21   pass:20   dwarn:0   dfail:0   fail:0   skip:0   time: 0s
fi-skl-6700k     total:21   pass:20   dwarn:0   dfail:0   fail:0   skip:0   time: 0s
fi-skl-6770hq    total:21   pass:20   dwarn:0   dfail:0   fail:0   skip:0   time: 0s
fi-skl-gvtdvm    total:21   pass:20   dwarn:0   dfail:0   fail:0   skip:0   time: 0s
fi-snb-2520m     total:278  pass:249  dwarn:0   dfail:0   fail:1   skip:28  time: 538s
fi-snb-2600      total:278  pass:248  dwarn:0   dfail:0   fail:1   skip:29  time: 416s

39a0f48c8bc7c528cc705016dafa08a9dedfd36b drm-tip: 2017y-04m-05d-13h-22m-47s UTC integration manifest
0a69b3e drm/i915: Advance ring->head fully when idle
b5914e6 drm/i915: Assert the engine is idle before overwiting the HWS

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4409/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2] drm/i915: Advance ring->head fully when idle
  2017-04-05 15:30 ` [PATCH 2/2] drm/i915: Advance ring->head fully when idle Chris Wilson
@ 2017-04-05 16:06   ` Chris Wilson
  2017-04-06 15:52     ` [PATCH v3] " Chris Wilson
  0 siblings, 1 reply; 12+ messages in thread
From: Chris Wilson @ 2017-04-05 16:06 UTC (permalink / raw)
  To: intel-gfx

When we retire the last request on the ring, before we ever access that
ring again we know it will be completely idle and so we can advance the
ring->head fully to the end (i.e. ring->tail) and not just to the start
of the breadcrumb. This allows us to skip re-emitting the breadcrumb
after resetting the GPU if the ring was entirely idle. This prevents us
from overwriting a seqno wraparound by re-executing a stale breadcrumb,
i.e.
	submit_request(1)
	intel_engine_init_global_seqno(0)
	i915_reset()
would then leave 1 in the HWS, but the next request to execute would
also be with seqno 1. The sanity checks upon submission detect this as a
timewarp and explode. By setting the ring as empty, upon reset the HWS
is left as 0, leaving it consistent with the timeline.

v2: Fix check for deleting last element of list. We know that this
request is always the first element of the ring, so only if next
points back to the start will this be the only request in flight.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100144
Testcase: igt/gem_exec_whisper/hang-*
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem_request.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
index 2f8c5132b54e..e535036132b0 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/i915_gem_request.c
@@ -295,8 +295,11 @@ static void i915_gem_request_retire(struct drm_i915_gem_request *request)
 	 * Note this requires that we are always called in request
 	 * completion order.
 	 */
+	if (request->ring_link.next == &request->ring->request_list)
+		request->ring->head = request->ring->tail;
+	else
+		request->ring->head = request->postfix;
 	list_del(&request->ring_link);
-	request->ring->head = request->postfix;
 	if (!--request->i915->gt.active_requests) {
 		GEM_BUG_ON(!request->i915->gt.awake);
 		mod_delayed_work(request->i915->wq,
-- 
2.11.0

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* ✗ Fi.CI.BAT: warning for series starting with [1/2] drm/i915: Assert the engine is idle before overwiting the HWS (rev2)
  2017-04-05 15:30 [PATCH 1/2] drm/i915: Assert the engine is idle before overwiting the HWS Chris Wilson
  2017-04-05 15:30 ` [PATCH 2/2] drm/i915: Advance ring->head fully when idle Chris Wilson
  2017-04-05 15:53 ` ✗ Fi.CI.BAT: failure for series starting with [1/2] drm/i915: Assert the engine is idle before overwiting the HWS Patchwork
@ 2017-04-05 16:26 ` Patchwork
  2017-04-06 16:40 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Assert the engine is idle before overwiting the HWS (rev3) Patchwork
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2017-04-05 16:26 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] drm/i915: Assert the engine is idle before overwiting the HWS (rev2)
URL   : https://patchwork.freedesktop.org/series/22527/
State : warning

== Summary ==

Series 22527v2 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/22527/revisions/2/mbox/

Test kms_force_connector_basic:
        Subgroup force-connector-state:
                pass       -> SKIP       (fi-ivb-3520m)
        Subgroup force-edid:
                pass       -> SKIP       (fi-ivb-3520m)
        Subgroup force-load-detect:
                pass       -> SKIP       (fi-ivb-3520m)
        Subgroup prune-stale-modes:
                pass       -> SKIP       (fi-ivb-3520m)

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time: 434s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time: 424s
fi-bsw-n3050     total:278  pass:242  dwarn:0   dfail:0   fail:0   skip:36  time: 583s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time: 507s
fi-bxt-t5700     total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  time: 554s
fi-byt-j1900     total:278  pass:254  dwarn:0   dfail:0   fail:0   skip:24  time: 476s
fi-byt-n2820     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time: 488s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 408s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 407s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time: 426s
fi-ivb-3520m     total:278  pass:256  dwarn:0   dfail:0   fail:0   skip:22  time: 487s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 469s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 453s
fi-kbl-7560u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 565s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 458s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time: 573s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time: 455s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 494s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time: 433s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time: 520s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time: 400s

39a0f48c8bc7c528cc705016dafa08a9dedfd36b drm-tip: 2017y-04m-05d-13h-22m-47s UTC integration manifest
ffc28f6 drm/i915: Advance ring->head fully when idle
61cbb1b drm/i915: Assert the engine is idle before overwiting the HWS

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4411/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v3] drm/i915: Advance ring->head fully when idle
  2017-04-05 16:06   ` [PATCH v2] " Chris Wilson
@ 2017-04-06 15:52     ` Chris Wilson
  2017-04-06 17:00       ` [PATCH v4] " Chris Wilson
  0 siblings, 1 reply; 12+ messages in thread
From: Chris Wilson @ 2017-04-06 15:52 UTC (permalink / raw)
  To: intel-gfx; +Cc: Mika Kuoppala

When we retire the last request on the ring, before we ever access that
ring again we know it will be completely idle and so we can advance the
ring->head fully to the end (i.e. ring->tail) and not just to the start
of the breadcrumb. This allows us to skip re-emitting the breadcrumb
after resetting the GPU if the ring was entirely idle. This prevents us
from overwriting a seqno wraparound by re-executing a stale breadcrumb,
i.e.
	submit_request(1)
	intel_engine_init_global_seqno(0)
	i915_reset()
would then leave 1 in the HWS, but the next request to execute would
also be with seqno 1. The sanity checks upon submission detect this as a
timewarp and explode. By setting the ring as empty, upon reset the HWS
is left as 0, leaving it consistent with the timeline.

v2: Fix check for deleting last element of list. We know that this
request is always the first element of the ring, so only if next
points back to the start will this be the only request in flight.
v3: Remove opencoding of list_is_last()

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100144
Testcase: igt/gem_exec_whisper/hang-*
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_request.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
index 2f8c5132b54e..314b852fa342 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/i915_gem_request.c
@@ -295,8 +295,11 @@ static void i915_gem_request_retire(struct drm_i915_gem_request *request)
 	 * Note this requires that we are always called in request
 	 * completion order.
 	 */
+	if (list_is_last(&request->ring_link, &request->ring->request_list))
+		request->ring->head = request->ring->tail;
+	else
+		request->ring->head = request->postfix;
 	list_del(&request->ring_link);
-	request->ring->head = request->postfix;
 	if (!--request->i915->gt.active_requests) {
 		GEM_BUG_ON(!request->i915->gt.awake);
 		mod_delayed_work(request->i915->wq,
-- 
2.11.0

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Assert the engine is idle before overwiting the HWS (rev3)
  2017-04-05 15:30 [PATCH 1/2] drm/i915: Assert the engine is idle before overwiting the HWS Chris Wilson
                   ` (2 preceding siblings ...)
  2017-04-05 16:26 ` ✗ Fi.CI.BAT: warning for series starting with [1/2] drm/i915: Assert the engine is idle before overwiting the HWS (rev2) Patchwork
@ 2017-04-06 16:40 ` Patchwork
  2017-04-06 17:30 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Assert the engine is idle before overwiting the HWS (rev4) Patchwork
  2017-04-07  7:22 ` [PATCH 1/2] drm/i915: Assert the engine is idle before overwiting the HWS Joonas Lahtinen
  5 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2017-04-06 16:40 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] drm/i915: Assert the engine is idle before overwiting the HWS (rev3)
URL   : https://patchwork.freedesktop.org/series/22527/
State : success

== Summary ==

Series 22527v3 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/22527/revisions/3/mbox/

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time: 441s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time: 429s
fi-bsw-n3050     total:278  pass:242  dwarn:0   dfail:0   fail:0   skip:36  time: 581s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time: 509s
fi-bxt-t5700     total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  time: 551s
fi-byt-j1900     total:278  pass:254  dwarn:0   dfail:0   fail:0   skip:24  time: 482s
fi-byt-n2820     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time: 486s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 410s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 406s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time: 422s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 491s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 482s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 452s
fi-kbl-7560u     total:278  pass:267  dwarn:1   dfail:0   fail:0   skip:10  time: 570s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 453s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time: 575s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time: 454s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 490s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time: 440s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time: 530s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time: 405s

e087f8395ca39c6988de8680bd6f80a20b08c0f4 drm-tip: 2017y-04m-06d-13h-28m-42s UTC integration manifest
1109c5f drm/i915: Advance ring->head fully when idle
6b57d38 drm/i915: Assert the engine is idle before overwiting the HWS

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4423/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v4] drm/i915: Advance ring->head fully when idle
  2017-04-06 15:52     ` [PATCH v3] " Chris Wilson
@ 2017-04-06 17:00       ` Chris Wilson
  2017-04-07  7:25         ` Joonas Lahtinen
  0 siblings, 1 reply; 12+ messages in thread
From: Chris Wilson @ 2017-04-06 17:00 UTC (permalink / raw)
  To: intel-gfx; +Cc: Mika Kuoppala

When we retire the last request on the ring, before we ever access that
ring again we know it will be completely idle and so we can advance the
ring->head fully to the end (i.e. ring->tail) and not just to the start
of the breadcrumb. This allows us to skip re-emitting the breadcrumb
after resetting the GPU if the ring was entirely idle. This prevents us
from overwriting a seqno wraparound by re-executing a stale breadcrumb,
i.e.
	submit_request(1)
	intel_engine_init_global_seqno(0)
	i915_reset()
would then leave 1 in the HWS, but the next request to execute would
also be with seqno 1. The sanity checks upon submission detect this as a
timewarp and explode. By setting the ring as empty, upon reset the HWS
is left as 0, leaving it consistent with the timeline.

v2: Fix check for deleting last element of list. We know that this
request is always the first element of the ring, so only if next
points back to the start will this be the only request in flight.
v3: Remove opencoding of list_is_last()
v4: Move the block to its own function for some clarity.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100144
Testcase: igt/gem_exec_whisper/hang-*
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_request.c | 32 ++++++++++++++++++++++----------
 1 file changed, 22 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
index 2f8c5132b54e..313cdff7c6dd 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/i915_gem_request.c
@@ -271,6 +271,27 @@ void i915_gem_retire_noop(struct i915_gem_active *active,
 	/* Space left intentionally blank */
 }
 
+static void advance_ring(struct drm_i915_gem_request *request)
+{
+	unsigned int tail;
+
+	/* We know the GPU must have read the request to have
+	 * sent us the seqno + interrupt, so use the position
+	 * of tail of the request to update the last known position
+	 * of the GPU head.
+	 *
+	 * Note this requires that we are always called in request
+	 * completion order.
+	 */
+	if (list_is_last(&request->ring_link, &request->ring->request_list))
+		tail = request->ring->tail;
+	else
+		tail = request->postfix;
+	list_del(&request->ring_link);
+
+	request->ring->head = tail;
+}
+
 static void i915_gem_request_retire(struct drm_i915_gem_request *request)
 {
 	struct intel_engine_cs *engine = request->engine;
@@ -287,16 +308,6 @@ static void i915_gem_request_retire(struct drm_i915_gem_request *request)
 	list_del_init(&request->link);
 	spin_unlock_irq(&engine->timeline->lock);
 
-	/* We know the GPU must have read the request to have
-	 * sent us the seqno + interrupt, so use the position
-	 * of tail of the request to update the last known position
-	 * of the GPU head.
-	 *
-	 * Note this requires that we are always called in request
-	 * completion order.
-	 */
-	list_del(&request->ring_link);
-	request->ring->head = request->postfix;
 	if (!--request->i915->gt.active_requests) {
 		GEM_BUG_ON(!request->i915->gt.awake);
 		mod_delayed_work(request->i915->wq,
@@ -304,6 +315,7 @@ static void i915_gem_request_retire(struct drm_i915_gem_request *request)
 				 msecs_to_jiffies(100));
 	}
 	unreserve_seqno(request->engine);
+	advance_ring(request);
 
 	/* Walk through the active list, calling retire on each. This allows
 	 * objects to track their GPU activity and mark themselves as idle
-- 
2.11.0

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Assert the engine is idle before overwiting the HWS (rev4)
  2017-04-05 15:30 [PATCH 1/2] drm/i915: Assert the engine is idle before overwiting the HWS Chris Wilson
                   ` (3 preceding siblings ...)
  2017-04-06 16:40 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Assert the engine is idle before overwiting the HWS (rev3) Patchwork
@ 2017-04-06 17:30 ` Patchwork
  2017-04-07  7:22 ` [PATCH 1/2] drm/i915: Assert the engine is idle before overwiting the HWS Joonas Lahtinen
  5 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2017-04-06 17:30 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] drm/i915: Assert the engine is idle before overwiting the HWS (rev4)
URL   : https://patchwork.freedesktop.org/series/22527/
State : success

== Summary ==

Series 22527v4 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/22527/revisions/4/mbox/

Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                pass       -> FAIL       (fi-snb-2600) fdo#100007
Test gem_exec_suspend:
        Subgroup basic-s4-devices:
                pass       -> DMESG-WARN (fi-bxt-t5700) fdo#100125
                dmesg-warn -> PASS       (fi-kbl-7560u) fdo#100125

fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007
fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time: 430s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time: 427s
fi-bsw-n3050     total:278  pass:242  dwarn:0   dfail:0   fail:0   skip:36  time: 577s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time: 508s
fi-bxt-t5700     total:278  pass:257  dwarn:1   dfail:0   fail:0   skip:20  time: 560s
fi-byt-j1900     total:278  pass:254  dwarn:0   dfail:0   fail:0   skip:24  time: 487s
fi-byt-n2820     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time: 483s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 413s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 404s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time: 414s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 489s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 462s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 451s
fi-kbl-7560u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 570s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 456s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time: 572s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time: 459s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 489s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time: 435s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time: 532s
fi-snb-2600      total:278  pass:248  dwarn:0   dfail:0   fail:1   skip:29  time: 406s

e087f8395ca39c6988de8680bd6f80a20b08c0f4 drm-tip: 2017y-04m-06d-13h-28m-42s UTC integration manifest
c804b46 drm/i915: Advance ring->head fully when idle
a1da060 drm/i915: Assert the engine is idle before overwiting the HWS

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4424/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] drm/i915: Assert the engine is idle before overwiting the HWS
  2017-04-05 15:30 [PATCH 1/2] drm/i915: Assert the engine is idle before overwiting the HWS Chris Wilson
                   ` (4 preceding siblings ...)
  2017-04-06 17:30 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Assert the engine is idle before overwiting the HWS (rev4) Patchwork
@ 2017-04-07  7:22 ` Joonas Lahtinen
  5 siblings, 0 replies; 12+ messages in thread
From: Joonas Lahtinen @ 2017-04-07  7:22 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx

On ke, 2017-04-05 at 16:30 +0100, Chris Wilson wrote:
> When we update the global seqno (on the engine timeline), we modify HW
> state (both registers and mapped pages). As we do this, we should be
> sure that the HW is idle and we are not causing a conflict. The caller
> is supposed to wait_for_idle before calling us to update the seqno, so
> let's assert they have and the engine is indeed idle.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

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

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4] drm/i915: Advance ring->head fully when idle
  2017-04-06 17:00       ` [PATCH v4] " Chris Wilson
@ 2017-04-07  7:25         ` Joonas Lahtinen
  2017-04-07  9:15           ` Chris Wilson
  0 siblings, 1 reply; 12+ messages in thread
From: Joonas Lahtinen @ 2017-04-07  7:25 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx; +Cc: Mika Kuoppala

On to, 2017-04-06 at 18:00 +0100, Chris Wilson wrote:
> When we retire the last request on the ring, before we ever access that
> ring again we know it will be completely idle and so we can advance the
> ring->head fully to the end (i.e. ring->tail) and not just to the start
> of the breadcrumb. This allows us to skip re-emitting the breadcrumb
> after resetting the GPU if the ring was entirely idle. This prevents us
> from overwriting a seqno wraparound by re-executing a stale breadcrumb,
> i.e.
> 	submit_request(1)
> 	intel_engine_init_global_seqno(0)
> 	i915_reset()
> would then leave 1 in the HWS, but the next request to execute would
> also be with seqno 1. The sanity checks upon submission detect this as a
> timewarp and explode. By setting the ring as empty, upon reset the HWS
> is left as 0, leaving it consistent with the timeline.
> 
> v2: Fix check for deleting last element of list. We know that this
> request is always the first element of the ring, so only if next
> points back to the start will this be the only request in flight.
> v3: Remove opencoding of list_is_last()
> v4: Move the block to its own function for some clarity.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100144
> Testcase: igt/gem_exec_whisper/hang-*
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>

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

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4] drm/i915: Advance ring->head fully when idle
  2017-04-07  7:25         ` Joonas Lahtinen
@ 2017-04-07  9:15           ` Chris Wilson
  0 siblings, 0 replies; 12+ messages in thread
From: Chris Wilson @ 2017-04-07  9:15 UTC (permalink / raw)
  To: Joonas Lahtinen; +Cc: intel-gfx, Mika Kuoppala

On Fri, Apr 07, 2017 at 10:25:13AM +0300, Joonas Lahtinen wrote:
> On to, 2017-04-06 at 18:00 +0100, Chris Wilson wrote:
> > When we retire the last request on the ring, before we ever access that
> > ring again we know it will be completely idle and so we can advance the
> > ring->head fully to the end (i.e. ring->tail) and not just to the start
> > of the breadcrumb. This allows us to skip re-emitting the breadcrumb
> > after resetting the GPU if the ring was entirely idle. This prevents us
> > from overwriting a seqno wraparound by re-executing a stale breadcrumb,
> > i.e.
> > 	submit_request(1)
> > 	intel_engine_init_global_seqno(0)
> > 	i915_reset()
> > would then leave 1 in the HWS, but the next request to execute would
> > also be with seqno 1. The sanity checks upon submission detect this as a
> > timewarp and explode. By setting the ring as empty, upon reset the HWS
> > is left as 0, leaving it consistent with the timeline.
> > 
> > v2: Fix check for deleting last element of list. We know that this
> > request is always the first element of the ring, so only if next
> > points back to the start will this be the only request in flight.
> > v3: Remove opencoding of list_is_last()
> > v4: Move the block to its own function for some clarity.
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100144
> > Testcase: igt/gem_exec_whisper/hang-*
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> 
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Thanks for the review, and pushed. One more mysterious reset fix done.
-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

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2017-04-07  9:15 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-05 15:30 [PATCH 1/2] drm/i915: Assert the engine is idle before overwiting the HWS Chris Wilson
2017-04-05 15:30 ` [PATCH 2/2] drm/i915: Advance ring->head fully when idle Chris Wilson
2017-04-05 16:06   ` [PATCH v2] " Chris Wilson
2017-04-06 15:52     ` [PATCH v3] " Chris Wilson
2017-04-06 17:00       ` [PATCH v4] " Chris Wilson
2017-04-07  7:25         ` Joonas Lahtinen
2017-04-07  9:15           ` Chris Wilson
2017-04-05 15:53 ` ✗ Fi.CI.BAT: failure for series starting with [1/2] drm/i915: Assert the engine is idle before overwiting the HWS Patchwork
2017-04-05 16:26 ` ✗ Fi.CI.BAT: warning for series starting with [1/2] drm/i915: Assert the engine is idle before overwiting the HWS (rev2) Patchwork
2017-04-06 16:40 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Assert the engine is idle before overwiting the HWS (rev3) Patchwork
2017-04-06 17:30 ` ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Assert the engine is idle before overwiting the HWS (rev4) Patchwork
2017-04-07  7:22 ` [PATCH 1/2] drm/i915: Assert the engine is idle before overwiting the HWS Joonas Lahtinen

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.