On 20/09/18 07:33, Mika Kuoppala wrote:
If we ack the identities immediately after they have been
handled, it should unblock next interrupt accumulation
in the gathering register earlier. It also allows us to
remove time based polling of valid bit. If we don't get a valid
sample now, we will likely get a valid sample on next interrupt,
which will be generated due to skipping the ack. The downside
is that we will have as many ack writes as there were identities
handled.
I'm not sure this is going to work. It looks like if we skip the
clearing of the identity register we need to skip the clearing of
INTR_DW as well because the HW doesn't seem to set that again. If we
clear it, when the interrupt gets re-triggered the driver won't be able
to handle it and so it'll keep being re-triggered in a loop. I've just
confirmed this behavior on the current code (i.e. without this series
applied) by simply pretending the ident was not valid for the first
interrupt:
[ 728.765749] [drm:gen11_gt_engine_identity [i915]] *ERROR*
INTR_IDENTITY_REG0:15 0x not valid!
[ 728.765800] [drm:gen11_irq_handler [i915]] *ERROR* GT_INTR_DW0 blank!
[ 728.765865] [drm:gen11_irq_handler [i915]] *ERROR* GT_INTR_DW0 blank!
[ 728.766050] [drm:gen11_irq_handler [i915]] *ERROR* GT_INTR_DW0 blank!
[... more of the same ...]
[ 782.395257] [drm:gen11_irq_handler [i915]] *ERROR* GT_INTR_DW0 blank!
[ 782.395293] [drm:gen11_irq_handler [i915]] *ERROR* GT_INTR_DW0 blank!
I haven't checked in detail all the other patches in the series yet so
not sure if any of them can mitigate this issue.
Daniele
Leave small retry loop for safety and with debugs to see if we
ever encounter read with valid not set.
Cc: Daniele Ceraolo Spurio
Cc: Chris Wilson
Signed-off-by: Mika Kuoppala
---
drivers/gpu/drm/i915/i915_irq.c | 26 +++---
1 file changed, 11 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 27116e3f21af..beb9fe4abf1f 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2965,22 +2965,18 @@ gen11_gt_engine_identity(struct drm_i915_private *
const i915,
const unsigned int bank, const unsigned int bit)
{
void __iomem * const regs = i915->regs;
- u32 timeout_ts;
- u32 ident;
+ u32 ident, retry = 0;
lockdep_assert_held(&i915->irq_lock);
raw_reg_write(regs, GEN11_IIR_REG_SELECTOR(bank), BIT(bit));
- /*
-* NB: Specs do not specify how long to spin wait,
-* so we do ~100us as an educated guess.
-*/
- timeout_ts = (local_clock() >> 10) + 100;
do {
ident = raw_reg_read(regs, GEN11_INTR_IDENTITY_REG(bank));
- } while (!(ident & GEN11_INTR_DATA_VALID) &&
-!time_after32(local_clock() >> 10, timeout_ts));
+ } while (!(ident & GEN11_INTR_DATA_VALID) && ++retry <= 10);
+
+ if (unlikely(GEM_SHOW_DEBUG() && retry))
+ WARN_ONCE(1, "INTR_IDENTITY took %u reads to settle\n", retry);
if (unlikely(!(ident & GEN11_INTR_DATA_VALID))) {
DRM_ERROR("INTR_IDENTITY_REG%u:%u 0x%08x not valid!\n",
@@ -3031,9 +3027,6 @@ gen11_gt_identity_handler(struct drm_i915_private * const
i915,
const u8 instance = GEN11_INTR_ENGINE_INSTANCE(identity);
const u16 intr = GEN11_INTR_ENGINE_INTR(identity);
- if (unlikely(!intr))
- return;
-
if (class <= COPY_ENGINE_CLASS)
return gen11_engine_irq_handler(i915, class, instance, intr);
@@ -3065,11 +3058,14 @@ gen11_gt_bank_handler(struct drm_i915_private * const i915,
const u32 ident = gen11_gt_engine_identity(i915,
bank, bit);
+ if (unlikely(!ident))
+ continue;
+
gen11_gt_identity_handler(i915, ident);
- }
- /* Clear must be after shared has been served for engine */
- raw_reg_write(regs, GEN11_GT_INTR_DW(bank), intr_dw);
+ /* Clear must be after shared has been served for engine */
+ raw_reg_write(regs, GEN11_GT_INTR_DW(bank), BIT(bit));
+ }
}
static void
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
