* [Intel-gfx] [CI 1/3] drm/i915/gt: Flush GT interrupt handler before changing interrupt state @ 2021-01-21 15:49 Chris Wilson 2021-01-21 15:49 ` [Intel-gfx] [CI 2/3] drm/i915/gt: Move execlists_reset() out of line Chris Wilson ` (3 more replies) 0 siblings, 4 replies; 5+ messages in thread From: Chris Wilson @ 2021-01-21 15:49 UTC (permalink / raw) To: intel-gfx Before we clear any state that may be being written by an interrupt handler on another core, flush the interrupt handlers. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/gt/intel_execlists_submission.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c index b31ce0d60028..e8c20f80e353 100644 --- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c +++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c @@ -2656,7 +2656,10 @@ static void enable_error_interrupt(struct intel_engine_cs *engine) { u32 status; + /* Flush ongoing GT interrupts before touching interrupt state */ + synchronize_hardirq(engine->i915->drm.irq); engine->execlists.error_interrupt = 0; + ENGINE_WRITE(engine, RING_EMR, ~0u); ENGINE_WRITE(engine, RING_EIR, ~0u); /* clear all existing errors */ -- 2.20.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [Intel-gfx] [CI 2/3] drm/i915/gt: Move execlists_reset() out of line 2021-01-21 15:49 [Intel-gfx] [CI 1/3] drm/i915/gt: Flush GT interrupt handler before changing interrupt state Chris Wilson @ 2021-01-21 15:49 ` Chris Wilson 2021-01-21 15:49 ` [Intel-gfx] [CI 3/3] drm/i915/gt: Call stop_ring() from ring resume, again Chris Wilson ` (2 subsequent siblings) 3 siblings, 0 replies; 5+ messages in thread From: Chris Wilson @ 2021-01-21 15:49 UTC (permalink / raw) To: intel-gfx Reduce the bulk of execlists_submission_tasklet by moving the unlikely reset function out of line. add/remove: 1/0 grow/shrink: 0/1 up/down: 960/-935 (25) Function old new delta execlists_reset - 960 +960 execlists_submission_tasklet 6629 5694 -935 Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> --- .../drm/i915/gt/intel_execlists_submission.c | 31 +++++++++---------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c index e8c20f80e353..3e680bfa9d13 100644 --- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c +++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c @@ -2293,10 +2293,12 @@ static void execlists_capture(struct intel_engine_cs *engine) kfree(cap); } -static void execlists_reset(struct intel_engine_cs *engine, const char *msg) +static noinline void execlists_reset(struct intel_engine_cs *engine) { const unsigned int bit = I915_RESET_ENGINE + engine->id; unsigned long *lock = &engine->gt->reset.flags; + unsigned long eir = fetch_and_zero(&engine->execlists.error_interrupt); + const char *msg; if (!intel_has_reset_engine(engine->gt)) return; @@ -2304,6 +2306,15 @@ static void execlists_reset(struct intel_engine_cs *engine, const char *msg) if (test_and_set_bit(bit, lock)) return; + /* Generate the error message in priority wrt to the user! */ + if (eir & GENMASK(15, 0)) + msg = "CS error"; /* thrown by a user payload */ + else if (eir & ERROR_CSB) + msg = "invalid CSB event"; + else if (eir & ERROR_PREEMPT) + msg = "preemption time out"; + else + msg = "internal error"; ENGINE_TRACE(engine, "reset for %s\n", msg); /* Mark this tasklet as disabled to avoid waiting for it to complete */ @@ -2349,22 +2360,8 @@ static void execlists_submission_tasklet(unsigned long data) engine->execlists.error_interrupt |= ERROR_PREEMPT; } - if (unlikely(READ_ONCE(engine->execlists.error_interrupt))) { - const char *msg; - - /* Generate the error message in priority wrt to the user! */ - if (engine->execlists.error_interrupt & GENMASK(15, 0)) - msg = "CS error"; /* thrown by a user payload */ - else if (engine->execlists.error_interrupt & ERROR_CSB) - msg = "invalid CSB event"; - else if (engine->execlists.error_interrupt & ERROR_PREEMPT) - msg = "preemption time out"; - else - msg = "internal error"; - - engine->execlists.error_interrupt = 0; - execlists_reset(engine, msg); - } + if (unlikely(READ_ONCE(engine->execlists.error_interrupt))) + execlists_reset(engine); if (!engine->execlists.pending[0]) { execlists_dequeue_irq(engine); -- 2.20.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [Intel-gfx] [CI 3/3] drm/i915/gt: Call stop_ring() from ring resume, again 2021-01-21 15:49 [Intel-gfx] [CI 1/3] drm/i915/gt: Flush GT interrupt handler before changing interrupt state Chris Wilson 2021-01-21 15:49 ` [Intel-gfx] [CI 2/3] drm/i915/gt: Move execlists_reset() out of line Chris Wilson @ 2021-01-21 15:49 ` Chris Wilson 2021-01-21 16:07 ` [Intel-gfx] [CI 1/3] drm/i915/gt: Flush GT interrupt handler before changing interrupt state Mika Kuoppala 2021-01-21 23:43 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [CI,1/3] " Patchwork 3 siblings, 0 replies; 5+ messages in thread From: Chris Wilson @ 2021-01-21 15:49 UTC (permalink / raw) To: intel-gfx 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 <chris@chris-wilson.co.uk> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Acked-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> --- .../gpu/drm/i915/gt/intel_ring_submission.c | 108 +++++++++--------- 1 file changed, 56 insertions(+), 52 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_ring_submission.c b/drivers/gpu/drm/i915/gt/intel_ring_submission.c index 4984ff565424..e4db4318f634 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)); + } } } -- 2.20.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Intel-gfx] [CI 1/3] drm/i915/gt: Flush GT interrupt handler before changing interrupt state 2021-01-21 15:49 [Intel-gfx] [CI 1/3] drm/i915/gt: Flush GT interrupt handler before changing interrupt state Chris Wilson 2021-01-21 15:49 ` [Intel-gfx] [CI 2/3] drm/i915/gt: Move execlists_reset() out of line Chris Wilson 2021-01-21 15:49 ` [Intel-gfx] [CI 3/3] drm/i915/gt: Call stop_ring() from ring resume, again Chris Wilson @ 2021-01-21 16:07 ` Mika Kuoppala 2021-01-21 23:43 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [CI,1/3] " Patchwork 3 siblings, 0 replies; 5+ messages in thread From: Mika Kuoppala @ 2021-01-21 16:07 UTC (permalink / raw) To: Chris Wilson, intel-gfx Chris Wilson <chris@chris-wilson.co.uk> writes: > Before we clear any state that may be being written by an interrupt > handler on another core, flush the interrupt handlers. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> > --- > drivers/gpu/drm/i915/gt/intel_execlists_submission.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c > index b31ce0d60028..e8c20f80e353 100644 > --- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c > +++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c > @@ -2656,7 +2656,10 @@ static void enable_error_interrupt(struct intel_engine_cs *engine) > { > u32 status; > > + /* Flush ongoing GT interrupts before touching interrupt state */ > + synchronize_hardirq(engine->i915->drm.irq); > engine->execlists.error_interrupt = 0; > + > ENGINE_WRITE(engine, RING_EMR, ~0u); > ENGINE_WRITE(engine, RING_EIR, ~0u); /* clear all existing errors */ > > -- > 2.20.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 5+ messages in thread
* [Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [CI,1/3] drm/i915/gt: Flush GT interrupt handler before changing interrupt state 2021-01-21 15:49 [Intel-gfx] [CI 1/3] drm/i915/gt: Flush GT interrupt handler before changing interrupt state Chris Wilson ` (2 preceding siblings ...) 2021-01-21 16:07 ` [Intel-gfx] [CI 1/3] drm/i915/gt: Flush GT interrupt handler before changing interrupt state Mika Kuoppala @ 2021-01-21 23:43 ` Patchwork 3 siblings, 0 replies; 5+ messages in thread From: Patchwork @ 2021-01-21 23:43 UTC (permalink / raw) To: Chris Wilson; +Cc: intel-gfx [-- Attachment #1.1: Type: text/plain, Size: 1051 bytes --] == Series Details == Series: series starting with [CI,1/3] drm/i915/gt: Flush GT interrupt handler before changing interrupt state URL : https://patchwork.freedesktop.org/series/86144/ State : failure == Summary == Applying: drm/i915/gt: Flush GT interrupt handler before changing interrupt state Using index info to reconstruct a base tree... M drivers/gpu/drm/i915/gt/intel_execlists_submission.c Falling back to patching base and 3-way merge... Auto-merging drivers/gpu/drm/i915/gt/intel_execlists_submission.c No changes -- Patch already applied. Applying: drm/i915/gt: Move execlists_reset() out of line Using index info to reconstruct a base tree... M drivers/gpu/drm/i915/gt/intel_execlists_submission.c Falling back to patching base and 3-way merge... No changes -- Patch already applied. Applying: drm/i915/gt: Call stop_ring() from ring resume, again Using index info to reconstruct a base tree... M drivers/gpu/drm/i915/gt/intel_ring_submission.c Falling back to patching base and 3-way merge... No changes -- Patch already applied. [-- Attachment #1.2: Type: text/html, Size: 1615 bytes --] [-- Attachment #2: Type: text/plain, Size: 160 bytes --] _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-01-21 23:43 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-01-21 15:49 [Intel-gfx] [CI 1/3] drm/i915/gt: Flush GT interrupt handler before changing interrupt state Chris Wilson 2021-01-21 15:49 ` [Intel-gfx] [CI 2/3] drm/i915/gt: Move execlists_reset() out of line Chris Wilson 2021-01-21 15:49 ` [Intel-gfx] [CI 3/3] drm/i915/gt: Call stop_ring() from ring resume, again Chris Wilson 2021-01-21 16:07 ` [Intel-gfx] [CI 1/3] drm/i915/gt: Flush GT interrupt handler before changing interrupt state Mika Kuoppala 2021-01-21 23:43 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [CI,1/3] " Patchwork
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).