From: Chris Wilson Date: Thu, 21 Jan 2021 15:49:50 +0000 (+0000) Subject: drm/i915/gt: Call stop_ring() from ring resume, again X-Git-Tag: v5.15~1144^2~13^2~37 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=536f77b1caa0c50b90b762cc79cf01c447fbd785;p=platform%2Fkernel%2Flinux-starfive.git drm/i915/gt: Call stop_ring() from ring resume, again For reasons I cannot explain, except to say this is Sandybridge after all, call stop_ring() again dring ring resume in order to prevent mysterious hard hangs. Testcase: igt/i915_selftest/hangcheck # snb Signed-off-by: Chris Wilson Cc: Mika Kuoppala Acked-by: Mika Kuoppala Link: https://patchwork.freedesktop.org/patch/msgid/20210121154950.19898-3-chris@chris-wilson.co.uk Signed-off-by: Daniel Vetter --- diff --git a/drivers/gpu/drm/i915/gt/intel_ring_submission.c b/drivers/gpu/drm/i915/gt/intel_ring_submission.c index 4984ff5..e4db4318 100644 --- a/drivers/gpu/drm/i915/gt/intel_ring_submission.c +++ b/drivers/gpu/drm/i915/gt/intel_ring_submission.c @@ -183,15 +183,36 @@ static void set_pp_dir(struct intel_engine_cs *engine) } } +static bool stop_ring(struct intel_engine_cs *engine) +{ + /* Empty the ring by skipping to the end */ + ENGINE_WRITE_FW(engine, RING_HEAD, ENGINE_READ_FW(engine, RING_TAIL)); + ENGINE_POSTING_READ(engine, RING_HEAD); + + /* The ring must be empty before it is disabled */ + ENGINE_WRITE_FW(engine, RING_CTL, 0); + ENGINE_POSTING_READ(engine, RING_CTL); + + /* Then reset the disabled ring */ + ENGINE_WRITE_FW(engine, RING_HEAD, 0); + ENGINE_WRITE_FW(engine, RING_TAIL, 0); + + return (ENGINE_READ_FW(engine, RING_HEAD) & HEAD_ADDR) == 0; +} + static int xcs_resume(struct intel_engine_cs *engine) { - struct drm_i915_private *dev_priv = engine->i915; struct intel_ring *ring = engine->legacy.ring; ENGINE_TRACE(engine, "ring:{HEAD:%04x, TAIL:%04x}\n", ring->head, ring->tail); - if (HWS_NEEDS_PHYSICAL(dev_priv)) + /* Double check the ring is empty & disabled before we resume */ + synchronize_hardirq(engine->i915->drm.irq); + if (!stop_ring(engine)) + goto err; + + if (HWS_NEEDS_PHYSICAL(engine->i915)) ring_setup_phys_status_page(engine); else ring_setup_status_page(engine); @@ -228,21 +249,10 @@ static int xcs_resume(struct intel_engine_cs *engine) if (__intel_wait_for_register_fw(engine->uncore, RING_CTL(engine->mmio_base), RING_VALID, RING_VALID, - 5000, 0, NULL)) { - drm_err(&dev_priv->drm, - "%s initialization failed; " - "ctl %08x (valid? %d) head %08x [%08x] tail %08x [%08x] start %08x [expected %08x]\n", - engine->name, - ENGINE_READ(engine, RING_CTL), - ENGINE_READ(engine, RING_CTL) & RING_VALID, - ENGINE_READ(engine, RING_HEAD), ring->head, - ENGINE_READ(engine, RING_TAIL), ring->tail, - ENGINE_READ(engine, RING_START), - i915_ggtt_offset(ring->vma)); - return -EIO; - } + 5000, 0, NULL)) + goto err; - if (INTEL_GEN(dev_priv) > 2) + if (INTEL_GEN(engine->i915) > 2) ENGINE_WRITE_FW(engine, RING_MI_MODE, _MASKED_BIT_DISABLE(STOP_RING)); @@ -255,6 +265,19 @@ static int xcs_resume(struct intel_engine_cs *engine) /* Papering over lost _interrupts_ immediately following the restart */ intel_engine_signal_breadcrumbs(engine); return 0; + +err: + drm_err(&engine->i915->drm, + "%s initialization failed; " + "ctl %08x (valid? %d) head %08x [%08x] tail %08x [%08x] start %08x [expected %08x]\n", + engine->name, + ENGINE_READ(engine, RING_CTL), + ENGINE_READ(engine, RING_CTL) & RING_VALID, + ENGINE_READ(engine, RING_HEAD), ring->head, + ENGINE_READ(engine, RING_TAIL), ring->tail, + ENGINE_READ(engine, RING_START), + i915_ggtt_offset(ring->vma)); + return -EIO; } static void sanitize_hwsp(struct intel_engine_cs *engine) @@ -290,23 +313,6 @@ static void xcs_sanitize(struct intel_engine_cs *engine) clflush_cache_range(engine->status_page.addr, PAGE_SIZE); } -static bool stop_ring(struct intel_engine_cs *engine) -{ - /* Empty the ring by skipping to the end */ - ENGINE_WRITE_FW(engine, RING_HEAD, ENGINE_READ_FW(engine, RING_TAIL)); - ENGINE_POSTING_READ(engine, RING_HEAD); - - /* The ring must be empty before it is disabled */ - ENGINE_WRITE_FW(engine, RING_CTL, 0); - ENGINE_POSTING_READ(engine, RING_CTL); - - /* Then reset the disabled ring */ - ENGINE_WRITE_FW(engine, RING_HEAD, 0); - ENGINE_WRITE_FW(engine, RING_TAIL, 0); - - return (ENGINE_READ_FW(engine, RING_HEAD) & HEAD_ADDR) == 0; -} - static void reset_prepare(struct intel_engine_cs *engine) { /* @@ -329,25 +335,23 @@ static void reset_prepare(struct intel_engine_cs *engine) if (!stop_ring(engine)) { /* G45 ring initialization often fails to reset head to zero */ - drm_dbg(&engine->i915->drm, - "%s head not reset to zero " - "ctl %08x head %08x tail %08x start %08x\n", - engine->name, - ENGINE_READ_FW(engine, RING_CTL), - ENGINE_READ_FW(engine, RING_HEAD), - ENGINE_READ_FW(engine, RING_TAIL), - ENGINE_READ_FW(engine, RING_START)); - } - - if (!stop_ring(engine)) { - drm_err(&engine->i915->drm, - "failed to set %s head to zero " - "ctl %08x head %08x tail %08x start %08x\n", - engine->name, - ENGINE_READ_FW(engine, RING_CTL), - ENGINE_READ_FW(engine, RING_HEAD), - ENGINE_READ_FW(engine, RING_TAIL), - ENGINE_READ_FW(engine, RING_START)); + ENGINE_TRACE(engine, + "HEAD not reset to zero, " + "{ CTL:%08x, HEAD:%08x, TAIL:%08x, START:%08x }\n", + ENGINE_READ_FW(engine, RING_CTL), + ENGINE_READ_FW(engine, RING_HEAD), + ENGINE_READ_FW(engine, RING_TAIL), + ENGINE_READ_FW(engine, RING_START)); + if (!stop_ring(engine)) { + drm_err(&engine->i915->drm, + "failed to set %s head to zero " + "ctl %08x head %08x tail %08x start %08x\n", + engine->name, + ENGINE_READ_FW(engine, RING_CTL), + ENGINE_READ_FW(engine, RING_HEAD), + ENGINE_READ_FW(engine, RING_TAIL), + ENGINE_READ_FW(engine, RING_START)); + } } }