From 86cad9b7783b04bb198307e765f483f809eca083 Mon Sep 17 00:00:00 2001 From: Mika Kuoppala Date: Wed, 13 Jan 2016 17:52:51 +0200 Subject: [PATCH] drm/i915: Force ordering on request submission and hangcheck Hangcheck is run on irq context and might be active on a completely different CPU that is submitting requests. And as we have been very careful not to add locking to hangcheck to guard against driver failures, we need to be careful with the coherency. Update ring last seqno and add to request lists early, with write memory barrier, before pushing the request to ring. On hangcheck side, use read memory barrier before inspecting the ring last seqno. This ensures that when hangcheck is inspecting the state, it will not see a active ring without requests in it, and then falsely report it as a missed irq situation. References: https://bugs.freedesktop.org/show_bug.cgi?id=93693 Cc: Chris Wilson Cc: Daniel Vetter Signed-off-by: Mika Kuoppala --- drivers/gpu/drm/i915/i915_gem.c | 14 +++++++++++--- drivers/gpu/drm/i915/i915_irq.c | 7 ++++--- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index ddc21d4b388d..497420de0079 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2584,6 +2584,17 @@ void __i915_add_request(struct drm_i915_gem_request *request, */ request->postfix = intel_ring_get_tail(ringbuf); + /* Hangcheck runs in irq context and might be even + * on different CPU. So we need to the do last seqno and + * list addition early with memory barrier. Otherwise hangcheck + * might see active ring without any requests and + * consider it falsely as a missed irq situation. + */ + request->previous_seqno = ring->last_submitted_seqno; + ring->last_submitted_seqno = request->seqno; + list_add_tail(&request->list, &ring->request_list); + wmb(); + if (i915.enable_execlists) ret = ring->emit_request(request); else { @@ -2605,9 +2616,6 @@ void __i915_add_request(struct drm_i915_gem_request *request, request->batch_obj = obj; request->emitted_jiffies = jiffies; - request->previous_seqno = ring->last_submitted_seqno; - ring->last_submitted_seqno = request->seqno; - list_add_tail(&request->list, &ring->request_list); trace_i915_gem_request_add(request); diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 25a89373df63..637a468368c4 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -2801,8 +2801,9 @@ static void gen8_disable_vblank(struct drm_device *dev, unsigned int pipe) static bool ring_idle(struct intel_engine_cs *ring, u32 seqno) { - return (list_empty(&ring->request_list) || - i915_seqno_passed(seqno, ring->last_submitted_seqno)); + rmb(); + return (i915_seqno_passed(seqno, ring->last_submitted_seqno) + || list_empty(&ring->request_list)); } static bool @@ -3101,8 +3102,8 @@ static void i915_hangcheck_elapsed(struct work_struct *work) semaphore_clear_deadlocks(dev_priv); - seqno = ring->get_seqno(ring, false); acthd = intel_ring_get_active_head(ring); + seqno = ring->get_seqno(ring, false); if (ring->hangcheck.seqno == seqno) { if (ring_idle(ring, seqno)) { -- 2.5.0