* [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).