* [PATCH] drm/i915/icl: Correctly clear lost ctx-switch interrupts across reset for Gen11
@ 2018-04-24 21:39 Oscar Mateo
2018-04-24 22:19 ` ✓ Fi.CI.BAT: success for " Patchwork
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Oscar Mateo @ 2018-04-24 21:39 UTC (permalink / raw)
To: intel-gfx; +Cc: Rodrigo Vivi
Interrupt handling in Gen11 is quite different from previous platforms.
v2: Rebased (Michel)
v3: Rebased with wiggle
v4: Rebased, remove TODO warning correctly (Daniele)
v5: Rebased, made gen11_gtiir const while at it (Michel)
v6: Rebased
v7: Adapt to the style currently in upstream
Suggested-by: Michel Thierry <michel.thierry@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Michel Thierry <michel.thierry@intel.com>
Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
drivers/gpu/drm/i915/i915_irq.c | 6 ++--
drivers/gpu/drm/i915/intel_drv.h | 3 ++
drivers/gpu/drm/i915/intel_lrc.c | 60 ++++++++++++++++++++++++++++------------
3 files changed, 48 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 96547e0..f9bc3aa 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -247,9 +247,9 @@ void i915_hotplug_interrupt_update(struct drm_i915_private *dev_priv,
gen11_gt_engine_identity(struct drm_i915_private * const i915,
const unsigned int bank, const unsigned int bit);
-static bool gen11_reset_one_iir(struct drm_i915_private * const i915,
- const unsigned int bank,
- const unsigned int bit)
+bool gen11_reset_one_iir(struct drm_i915_private * const i915,
+ const unsigned int bank,
+ const unsigned int bit)
{
void __iomem * const regs = i915->regs;
u32 dw;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 58868b9..9bba035 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1333,6 +1333,9 @@ void intel_pch_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv,
void intel_check_pch_fifo_underruns(struct drm_i915_private *dev_priv);
/* i915_irq.c */
+bool gen11_reset_one_iir(struct drm_i915_private * const i915,
+ const unsigned int bank,
+ const unsigned int bit);
void gen5_enable_gt_irq(struct drm_i915_private *dev_priv, uint32_t mask);
void gen5_disable_gt_irq(struct drm_i915_private *dev_priv, uint32_t mask);
void gen6_mask_pm_irq(struct drm_i915_private *dev_priv, u32 mask);
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 2d6572a..7ea5f36 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -789,22 +789,9 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
static void clear_gtiir(struct intel_engine_cs *engine)
{
- static const u8 gtiir[] = {
- [RCS] = 0,
- [BCS] = 0,
- [VCS] = 1,
- [VCS2] = 1,
- [VECS] = 3,
- };
struct drm_i915_private *dev_priv = engine->i915;
int i;
- /* TODO: correctly reset irqs for gen11 */
- if (WARN_ON_ONCE(INTEL_GEN(engine->i915) >= 11))
- return;
-
- GEM_BUG_ON(engine->id >= ARRAY_SIZE(gtiir));
-
/*
* Clear any pending interrupt state.
*
@@ -812,13 +799,50 @@ static void clear_gtiir(struct intel_engine_cs *engine)
* double buffered, and so if we only reset it once there may
* still be an interrupt pending.
*/
- for (i = 0; i < 2; i++) {
- I915_WRITE(GEN8_GT_IIR(gtiir[engine->id]),
+ if (INTEL_GEN(dev_priv) >= 11) {
+ static const struct {
+ u8 bank;
+ u8 bit;
+ } gen11_gtiir[] = {
+ [RCS] = {0, GEN11_RCS0},
+ [BCS] = {0, GEN11_BCS},
+ [_VCS(0)] = {1, GEN11_VCS(0)},
+ [_VCS(1)] = {1, GEN11_VCS(1)},
+ [_VCS(2)] = {1, GEN11_VCS(2)},
+ [_VCS(3)] = {1, GEN11_VCS(3)},
+ [_VECS(0)] = {1, GEN11_VECS(0)},
+ [_VECS(1)] = {1, GEN11_VECS(1)},
+ };
+ unsigned long irqflags;
+
+ GEM_BUG_ON(engine->id >= ARRAY_SIZE(gen11_gtiir));
+
+ spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
+ for (i = 0; i < 2; i++) {
+ gen11_reset_one_iir(dev_priv,
+ gen11_gtiir[engine->id].bank,
+ gen11_gtiir[engine->id].bit);
+ }
+ spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
+ } else {
+ static const u8 gtiir[] = {
+ [RCS] = 0,
+ [BCS] = 0,
+ [VCS] = 1,
+ [VCS2] = 1,
+ [VECS] = 3,
+ };
+
+ GEM_BUG_ON(engine->id >= ARRAY_SIZE(gtiir));
+
+ for (i = 0; i < 2; i++) {
+ I915_WRITE(GEN8_GT_IIR(gtiir[engine->id]),
+ engine->irq_keep_mask);
+ POSTING_READ(GEN8_GT_IIR(gtiir[engine->id]));
+ }
+ GEM_BUG_ON(I915_READ(GEN8_GT_IIR(gtiir[engine->id])) &
engine->irq_keep_mask);
- POSTING_READ(GEN8_GT_IIR(gtiir[engine->id]));
}
- GEM_BUG_ON(I915_READ(GEN8_GT_IIR(gtiir[engine->id])) &
- engine->irq_keep_mask);
}
static void reset_irq(struct intel_engine_cs *engine)
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915/icl: Correctly clear lost ctx-switch interrupts across reset for Gen11
2018-04-24 21:39 [PATCH] drm/i915/icl: Correctly clear lost ctx-switch interrupts across reset for Gen11 Oscar Mateo
@ 2018-04-24 22:19 ` Patchwork
2018-04-24 22:27 ` [PATCH] " Michel Thierry
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2018-04-24 22:19 UTC (permalink / raw)
To: Oscar Mateo; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/icl: Correctly clear lost ctx-switch interrupts across reset for Gen11
URL : https://patchwork.freedesktop.org/series/42229/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4089 -> Patchwork_8793 =
== Summary - SUCCESS ==
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/42229/revisions/1/mbox/
== Known issues ==
Here are the changes found in Patchwork_8793 that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
fi-skl-6770hq: PASS -> FAIL (fdo#103481)
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
fi-ivb-3520m: PASS -> DMESG-WARN (fdo#106084) +1
==== Possible fixes ====
igt@kms_flip@basic-flip-vs-wf_vblank:
{fi-hsw-4200u}: FAIL (fdo#103928) -> PASS
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
fi-ivb-3520m: DMESG-WARN (fdo#106084) -> PASS
igt@prime_vgem@basic-fence-flip:
fi-ilk-650: FAIL (fdo#104008) -> PASS
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
fdo#103481 https://bugs.freedesktop.org/show_bug.cgi?id=103481
fdo#103928 https://bugs.freedesktop.org/show_bug.cgi?id=103928
fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
fdo#106084 https://bugs.freedesktop.org/show_bug.cgi?id=106084
== Participating hosts (37 -> 35) ==
Missing (2): fi-ilk-m540 fi-skl-6700hq
== Build changes ==
* Linux: CI_DRM_4089 -> Patchwork_8793
CI_DRM_4089: df42cb94f69b1260752f6ae5be3c837ae6fcee85 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4446: e5e8dafc991ee922ec159491c680caff0cfe9235 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_8793: 3050e2c42ee884722d081ddf0a616dbc8c477b28 @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4446: a2f486679f467cd6e82578384f56d4aabaa8cf2e @ git://anongit.freedesktop.org/piglit
== Linux commits ==
3050e2c42ee8 drm/i915/icl: Correctly clear lost ctx-switch interrupts across reset for Gen11
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8793/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/icl: Correctly clear lost ctx-switch interrupts across reset for Gen11
2018-04-24 21:39 [PATCH] drm/i915/icl: Correctly clear lost ctx-switch interrupts across reset for Gen11 Oscar Mateo
2018-04-24 22:19 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2018-04-24 22:27 ` Michel Thierry
2018-04-25 15:12 ` Chris Wilson
2018-04-24 23:05 ` ✗ Fi.CI.IGT: failure for " Patchwork
2018-07-13 8:28 ` [PATCH] " Chris Wilson
3 siblings, 1 reply; 6+ messages in thread
From: Michel Thierry @ 2018-04-24 22:27 UTC (permalink / raw)
To: Oscar Mateo, intel-gfx; +Cc: Rodrigo Vivi
On 4/24/2018 2:39 PM, Oscar Mateo wrote:
> Interrupt handling in Gen11 is quite different from previous platforms.
>
> v2: Rebased (Michel)
> v3: Rebased with wiggle
> v4: Rebased, remove TODO warning correctly (Daniele)
> v5: Rebased, made gen11_gtiir const while at it (Michel)
> v6: Rebased
> v7: Adapt to the style currently in upstream
>
> Suggested-by: Michel Thierry <michel.thierry@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Michel Thierry <michel.thierry@intel.com>
> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_irq.c | 6 ++--
> drivers/gpu/drm/i915/intel_drv.h | 3 ++
> drivers/gpu/drm/i915/intel_lrc.c | 60 ++++++++++++++++++++++++++++------------
> 3 files changed, 48 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 96547e0..f9bc3aa 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -247,9 +247,9 @@ void i915_hotplug_interrupt_update(struct drm_i915_private *dev_priv,
> gen11_gt_engine_identity(struct drm_i915_private * const i915,
> const unsigned int bank, const unsigned int bit);
>
> -static bool gen11_reset_one_iir(struct drm_i915_private * const i915,
> - const unsigned int bank,
> - const unsigned int bit)
> +bool gen11_reset_one_iir(struct drm_i915_private * const i915,
> + const unsigned int bank,
> + const unsigned int bit)
> {
> void __iomem * const regs = i915->regs;
> u32 dw;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 58868b9..9bba035 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1333,6 +1333,9 @@ void intel_pch_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv,
> void intel_check_pch_fifo_underruns(struct drm_i915_private *dev_priv);
>
> /* i915_irq.c */
> +bool gen11_reset_one_iir(struct drm_i915_private * const i915,
> + const unsigned int bank,
> + const unsigned int bit);
> void gen5_enable_gt_irq(struct drm_i915_private *dev_priv, uint32_t mask);
> void gen5_disable_gt_irq(struct drm_i915_private *dev_priv, uint32_t mask);
> void gen6_mask_pm_irq(struct drm_i915_private *dev_priv, u32 mask);
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 2d6572a..7ea5f36 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -789,22 +789,9 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
>
> static void clear_gtiir(struct intel_engine_cs *engine)
> {
> - static const u8 gtiir[] = {
> - [RCS] = 0,
> - [BCS] = 0,
> - [VCS] = 1,
> - [VCS2] = 1,
> - [VECS] = 3,
> - };
> struct drm_i915_private *dev_priv = engine->i915;
> int i;
>
> - /* TODO: correctly reset irqs for gen11 */
> - if (WARN_ON_ONCE(INTEL_GEN(engine->i915) >= 11))
> - return;
> -
> - GEM_BUG_ON(engine->id >= ARRAY_SIZE(gtiir));
> -
> /*
> * Clear any pending interrupt state.
> *
> @@ -812,13 +799,50 @@ static void clear_gtiir(struct intel_engine_cs *engine)
> * double buffered, and so if we only reset it once there may
> * still be an interrupt pending.
> */
> - for (i = 0; i < 2; i++) {
> - I915_WRITE(GEN8_GT_IIR(gtiir[engine->id]),
> + if (INTEL_GEN(dev_priv) >= 11) {
> + static const struct {
> + u8 bank;
> + u8 bit;
> + } gen11_gtiir[] = {
> + [RCS] = {0, GEN11_RCS0},
> + [BCS] = {0, GEN11_BCS},
> + [_VCS(0)] = {1, GEN11_VCS(0)},
> + [_VCS(1)] = {1, GEN11_VCS(1)},
> + [_VCS(2)] = {1, GEN11_VCS(2)},
> + [_VCS(3)] = {1, GEN11_VCS(3)},
> + [_VECS(0)] = {1, GEN11_VECS(0)},
> + [_VECS(1)] = {1, GEN11_VECS(1)},
> + };
bit,bank values are correct so
Reviewed-by: Michel Thierry <michel.thierry@intel.com>
> + unsigned long irqflags;
> +
> + GEM_BUG_ON(engine->id >= ARRAY_SIZE(gen11_gtiir));
> +
> + spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
> + for (i = 0; i < 2; i++) {
> + gen11_reset_one_iir(dev_priv,
> + gen11_gtiir[engine->id].bank,
> + gen11_gtiir[engine->id].bit);
> + }
> + spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
> + } else {
> + static const u8 gtiir[] = {
> + [RCS] = 0,
> + [BCS] = 0,
> + [VCS] = 1,
> + [VCS2] = 1,
> + [VECS] = 3,
> + };
> +
> + GEM_BUG_ON(engine->id >= ARRAY_SIZE(gtiir));
> +
> + for (i = 0; i < 2; i++) {
> + I915_WRITE(GEN8_GT_IIR(gtiir[engine->id]),
> + engine->irq_keep_mask);
> + POSTING_READ(GEN8_GT_IIR(gtiir[engine->id]));
> + }
> + GEM_BUG_ON(I915_READ(GEN8_GT_IIR(gtiir[engine->id])) &
> engine->irq_keep_mask);
> - POSTING_READ(GEN8_GT_IIR(gtiir[engine->id]));
> }
> - GEM_BUG_ON(I915_READ(GEN8_GT_IIR(gtiir[engine->id])) &
> - engine->irq_keep_mask);
> }
>
> static void reset_irq(struct intel_engine_cs *engine)
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* ✗ Fi.CI.IGT: failure for drm/i915/icl: Correctly clear lost ctx-switch interrupts across reset for Gen11
2018-04-24 21:39 [PATCH] drm/i915/icl: Correctly clear lost ctx-switch interrupts across reset for Gen11 Oscar Mateo
2018-04-24 22:19 ` ✓ Fi.CI.BAT: success for " Patchwork
2018-04-24 22:27 ` [PATCH] " Michel Thierry
@ 2018-04-24 23:05 ` Patchwork
2018-07-13 8:28 ` [PATCH] " Chris Wilson
3 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2018-04-24 23:05 UTC (permalink / raw)
To: Oscar Mateo; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/icl: Correctly clear lost ctx-switch interrupts across reset for Gen11
URL : https://patchwork.freedesktop.org/series/42229/
State : failure
== Summary ==
= CI Bug Log - changes from CI_DRM_4089_full -> Patchwork_8793_full =
== Summary - FAILURE ==
Serious unknown changes coming with Patchwork_8793_full absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_8793_full, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://patchwork.freedesktop.org/api/1.0/series/42229/revisions/1/mbox/
== Possible new issues ==
Here are the unknown changes that may have been introduced in Patchwork_8793_full:
=== IGT changes ===
==== Possible regressions ====
igt@gem_eio@in-flight-immediate:
shard-glk: PASS -> DMESG-FAIL
==== Warnings ====
igt@gem_exec_schedule@deep-vebox:
shard-kbl: SKIP -> PASS +1
igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-mmap-gtt:
shard-snb: PASS -> SKIP +1
igt@perf_pmu@rc6:
shard-kbl: PASS -> SKIP
== Known issues ==
Here are the changes found in Patchwork_8793_full that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@gem_exec_suspend@basic-s3-devices:
shard-hsw: PASS -> DMESG-WARN (fdo#106086)
igt@kms_flip@flip-vs-absolute-wf_vblank:
shard-hsw: PASS -> FAIL (fdo#103928)
igt@kms_flip@flip-vs-expired-vblank-interruptible:
shard-hsw: PASS -> FAIL (fdo#105707)
igt@kms_flip@modeset-vs-vblank-race-interruptible:
shard-glk: PASS -> FAIL (fdo#103060)
igt@kms_flip@wf_vblank-ts-check-interruptible:
shard-glk: PASS -> FAIL (fdo#100368) +1
==== Possible fixes ====
igt@kms_flip@2x-flip-vs-absolute-wf_vblank:
shard-hsw: FAIL (fdo#100368) -> PASS
igt@kms_flip@2x-flip-vs-expired-vblank:
shard-hsw: FAIL (fdo#102887) -> PASS
igt@kms_flip@blocking-absolute-wf_vblank-interruptible:
shard-glk: FAIL (fdo#106134) -> PASS
igt@kms_flip@modeset-vs-vblank-race:
shard-hsw: FAIL (fdo#103060) -> PASS
igt@kms_flip@plain-flip-fb-recreate-interruptible:
shard-glk: FAIL (fdo#100368) -> PASS +2
igt@kms_rotation_crc@sprite-rotation-90-pos-100-0:
shard-apl: DMESG-WARN -> PASS
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
fdo#103928 https://bugs.freedesktop.org/show_bug.cgi?id=103928
fdo#105707 https://bugs.freedesktop.org/show_bug.cgi?id=105707
fdo#106086 https://bugs.freedesktop.org/show_bug.cgi?id=106086
fdo#106134 https://bugs.freedesktop.org/show_bug.cgi?id=106134
== Participating hosts (6 -> 5) ==
Missing (1): shard-glkb
== Build changes ==
* Linux: CI_DRM_4089 -> Patchwork_8793
CI_DRM_4089: df42cb94f69b1260752f6ae5be3c837ae6fcee85 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4446: e5e8dafc991ee922ec159491c680caff0cfe9235 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_8793: 3050e2c42ee884722d081ddf0a616dbc8c477b28 @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4446: a2f486679f467cd6e82578384f56d4aabaa8cf2e @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8793/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/icl: Correctly clear lost ctx-switch interrupts across reset for Gen11
2018-04-24 22:27 ` [PATCH] " Michel Thierry
@ 2018-04-25 15:12 ` Chris Wilson
0 siblings, 0 replies; 6+ messages in thread
From: Chris Wilson @ 2018-04-25 15:12 UTC (permalink / raw)
To: Michel Thierry, Oscar Mateo, intel-gfx; +Cc: Rodrigo Vivi
Quoting Michel Thierry (2018-04-24 23:27:41)
> On 4/24/2018 2:39 PM, Oscar Mateo wrote:
> > Interrupt handling in Gen11 is quite different from previous platforms.
> >
> > v2: Rebased (Michel)
> > v3: Rebased with wiggle
> > v4: Rebased, remove TODO warning correctly (Daniele)
> > v5: Rebased, made gen11_gtiir const while at it (Michel)
> > v6: Rebased
> > v7: Adapt to the style currently in upstream
> >
> > Suggested-by: Michel Thierry <michel.thierry@intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: Michel Thierry <michel.thierry@intel.com>
> > Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_irq.c | 6 ++--
> > drivers/gpu/drm/i915/intel_drv.h | 3 ++
> > drivers/gpu/drm/i915/intel_lrc.c | 60 ++++++++++++++++++++++++++++------------
> > 3 files changed, 48 insertions(+), 21 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index 96547e0..f9bc3aa 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -247,9 +247,9 @@ void i915_hotplug_interrupt_update(struct drm_i915_private *dev_priv,
> > gen11_gt_engine_identity(struct drm_i915_private * const i915,
> > const unsigned int bank, const unsigned int bit);
> >
> > -static bool gen11_reset_one_iir(struct drm_i915_private * const i915,
> > - const unsigned int bank,
> > - const unsigned int bit)
> > +bool gen11_reset_one_iir(struct drm_i915_private * const i915,
> > + const unsigned int bank,
> > + const unsigned int bit)
> > {
> > void __iomem * const regs = i915->regs;
> > u32 dw;
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 58868b9..9bba035 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1333,6 +1333,9 @@ void intel_pch_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv,
> > void intel_check_pch_fifo_underruns(struct drm_i915_private *dev_priv);
> >
> > /* i915_irq.c */
> > +bool gen11_reset_one_iir(struct drm_i915_private * const i915,
> > + const unsigned int bank,
> > + const unsigned int bit);
> > void gen5_enable_gt_irq(struct drm_i915_private *dev_priv, uint32_t mask);
> > void gen5_disable_gt_irq(struct drm_i915_private *dev_priv, uint32_t mask);
> > void gen6_mask_pm_irq(struct drm_i915_private *dev_priv, u32 mask);
> > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> > index 2d6572a..7ea5f36 100644
> > --- a/drivers/gpu/drm/i915/intel_lrc.c
> > +++ b/drivers/gpu/drm/i915/intel_lrc.c
> > @@ -789,22 +789,9 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
> >
> > static void clear_gtiir(struct intel_engine_cs *engine)
> > {
> > - static const u8 gtiir[] = {
> > - [RCS] = 0,
> > - [BCS] = 0,
> > - [VCS] = 1,
> > - [VCS2] = 1,
> > - [VECS] = 3,
> > - };
> > struct drm_i915_private *dev_priv = engine->i915;
> > int i;
> >
> > - /* TODO: correctly reset irqs for gen11 */
> > - if (WARN_ON_ONCE(INTEL_GEN(engine->i915) >= 11))
> > - return;
> > -
> > - GEM_BUG_ON(engine->id >= ARRAY_SIZE(gtiir));
> > -
> > /*
> > * Clear any pending interrupt state.
> > *
> > @@ -812,13 +799,50 @@ static void clear_gtiir(struct intel_engine_cs *engine)
> > * double buffered, and so if we only reset it once there may
> > * still be an interrupt pending.
> > */
> > - for (i = 0; i < 2; i++) {
> > - I915_WRITE(GEN8_GT_IIR(gtiir[engine->id]),
> > + if (INTEL_GEN(dev_priv) >= 11) {
> > + static const struct {
> > + u8 bank;
> > + u8 bit;
> > + } gen11_gtiir[] = {
> > + [RCS] = {0, GEN11_RCS0},
> > + [BCS] = {0, GEN11_BCS},
> > + [_VCS(0)] = {1, GEN11_VCS(0)},
> > + [_VCS(1)] = {1, GEN11_VCS(1)},
> > + [_VCS(2)] = {1, GEN11_VCS(2)},
> > + [_VCS(3)] = {1, GEN11_VCS(3)},
> > + [_VECS(0)] = {1, GEN11_VECS(0)},
> > + [_VECS(1)] = {1, GEN11_VECS(1)},
> > + };
> bit,bank values are correct so
>
> Reviewed-by: Michel Thierry <michel.thierry@intel.com>
>
> > + unsigned long irqflags;
> > +
> > + GEM_BUG_ON(engine->id >= ARRAY_SIZE(gen11_gtiir));
> > +
> > + spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
> > + for (i = 0; i < 2; i++) {
> > + gen11_reset_one_iir(dev_priv,
> > + gen11_gtiir[engine->id].bank,
> > + gen11_gtiir[engine->id].bit);
> > + }
> > + spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
Strictly speaking we are already inside an irq-safe section.
Applied as is.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915/icl: Correctly clear lost ctx-switch interrupts across reset for Gen11
2018-04-24 21:39 [PATCH] drm/i915/icl: Correctly clear lost ctx-switch interrupts across reset for Gen11 Oscar Mateo
` (2 preceding siblings ...)
2018-04-24 23:05 ` ✗ Fi.CI.IGT: failure for " Patchwork
@ 2018-07-13 8:28 ` Chris Wilson
3 siblings, 0 replies; 6+ messages in thread
From: Chris Wilson @ 2018-07-13 8:28 UTC (permalink / raw)
To: Oscar Mateo, intel-gfx; +Cc: Rodrigo Vivi
Quoting Oscar Mateo (2018-04-24 22:39:55)
> Interrupt handling in Gen11 is quite different from previous platforms.
>
> v2: Rebased (Michel)
> v3: Rebased with wiggle
> v4: Rebased, remove TODO warning correctly (Daniele)
> v5: Rebased, made gen11_gtiir const while at it (Michel)
> v6: Rebased
> v7: Adapt to the style currently in upstream
>
> Suggested-by: Michel Thierry <michel.thierry@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Michel Thierry <michel.thierry@intel.com>
> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_irq.c | 6 ++--
> drivers/gpu/drm/i915/intel_drv.h | 3 ++
> drivers/gpu/drm/i915/intel_lrc.c | 60 ++++++++++++++++++++++++++++------------
> 3 files changed, 48 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 96547e0..f9bc3aa 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -247,9 +247,9 @@ void i915_hotplug_interrupt_update(struct drm_i915_private *dev_priv,
> gen11_gt_engine_identity(struct drm_i915_private * const i915,
> const unsigned int bank, const unsigned int bit);
>
> -static bool gen11_reset_one_iir(struct drm_i915_private * const i915,
> - const unsigned int bank,
> - const unsigned int bit)
> +bool gen11_reset_one_iir(struct drm_i915_private * const i915,
> + const unsigned int bank,
> + const unsigned int bit)
> {
> void __iomem * const regs = i915->regs;
> u32 dw;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 58868b9..9bba035 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1333,6 +1333,9 @@ void intel_pch_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv,
> void intel_check_pch_fifo_underruns(struct drm_i915_private *dev_priv);
>
> /* i915_irq.c */
> +bool gen11_reset_one_iir(struct drm_i915_private * const i915,
> + const unsigned int bank,
> + const unsigned int bit);
> void gen5_enable_gt_irq(struct drm_i915_private *dev_priv, uint32_t mask);
> void gen5_disable_gt_irq(struct drm_i915_private *dev_priv, uint32_t mask);
> void gen6_mask_pm_irq(struct drm_i915_private *dev_priv, u32 mask);
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 2d6572a..7ea5f36 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -789,22 +789,9 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
>
> static void clear_gtiir(struct intel_engine_cs *engine)
> {
> - static const u8 gtiir[] = {
> - [RCS] = 0,
> - [BCS] = 0,
> - [VCS] = 1,
> - [VCS2] = 1,
> - [VECS] = 3,
> - };
> struct drm_i915_private *dev_priv = engine->i915;
> int i;
>
> - /* TODO: correctly reset irqs for gen11 */
> - if (WARN_ON_ONCE(INTEL_GEN(engine->i915) >= 11))
> - return;
> -
> - GEM_BUG_ON(engine->id >= ARRAY_SIZE(gtiir));
> -
> /*
> * Clear any pending interrupt state.
> *
> @@ -812,13 +799,50 @@ static void clear_gtiir(struct intel_engine_cs *engine)
> * double buffered, and so if we only reset it once there may
> * still be an interrupt pending.
> */
> - for (i = 0; i < 2; i++) {
> - I915_WRITE(GEN8_GT_IIR(gtiir[engine->id]),
> + if (INTEL_GEN(dev_priv) >= 11) {
> + static const struct {
> + u8 bank;
> + u8 bit;
> + } gen11_gtiir[] = {
> + [RCS] = {0, GEN11_RCS0},
> + [BCS] = {0, GEN11_BCS},
> + [_VCS(0)] = {1, GEN11_VCS(0)},
> + [_VCS(1)] = {1, GEN11_VCS(1)},
> + [_VCS(2)] = {1, GEN11_VCS(2)},
> + [_VCS(3)] = {1, GEN11_VCS(3)},
> + [_VECS(0)] = {1, GEN11_VECS(0)},
> + [_VECS(1)] = {1, GEN11_VECS(1)},
> + };
> + unsigned long irqflags;
> +
> + GEM_BUG_ON(engine->id >= ARRAY_SIZE(gen11_gtiir));
> +
> + spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
> + for (i = 0; i < 2; i++) {
> + gen11_reset_one_iir(dev_priv,
> + gen11_gtiir[engine->id].bank,
> + gen11_gtiir[engine->id].bit);
> + }
> + spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
This is sunk by
[ 57.409776] ======================================================
[ 57.409779] WARNING: possible circular locking dependency detected
[ 57.409783] 4.18.0-rc4-CI-CI_DII_1137+ #1 Tainted: G U W
[ 57.409785] ------------------------------------------------------
[ 57.409788] swapper/6/0 is trying to acquire lock:
[ 57.409790] 000000004f304ee5 (&engine->timeline.lock/1){-.-.}, at: execlists_submit_request+0x2b/0x1a0 [i915]
[ 57.409841]
but task is already holding lock:
[ 57.409844] 00000000aad89594 (&(&rq->lock)->rlock#2){-.-.}, at: notify_ring+0x2b2/0x480 [i915]
[ 57.409869]
which lock already depends on the new lock.
[ 57.409872]
the existing dependency chain (in reverse order) is:
[ 57.409876]
-> #2 (&(&rq->lock)->rlock#2){-.-.}:
[ 57.409900] notify_ring+0x2b2/0x480 [i915]
[ 57.409922] gen8_cs_irq_handler+0x39/0xa0 [i915]
[ 57.409943] gen11_irq_handler+0x2f0/0x420 [i915]
[ 57.409949] __handle_irq_event_percpu+0x42/0x370
[ 57.409952] handle_irq_event_percpu+0x2b/0x70
[ 57.409956] handle_irq_event+0x2f/0x50
[ 57.409959] handle_edge_irq+0xe7/0x190
[ 57.409964] handle_irq+0x67/0x160
[ 57.409967] do_IRQ+0x5e/0x120
[ 57.409971] ret_from_intr+0x0/0x1d
[ 57.409974] _raw_spin_unlock_irqrestore+0x4e/0x60
[ 57.409979] tasklet_action_common.isra.5+0x47/0xb0
[ 57.409982] __do_softirq+0xd9/0x505
[ 57.409985] irq_exit+0xa9/0xc0
[ 57.409988] do_IRQ+0x9a/0x120
[ 57.409991] ret_from_intr+0x0/0x1d
[ 57.409995] cpuidle_enter_state+0xac/0x360
[ 57.409999] do_idle+0x1f3/0x250
[ 57.410004] cpu_startup_entry+0x6a/0x70
[ 57.410010] start_secondary+0x19d/0x1f0
[ 57.410015] secondary_startup_64+0xa5/0xb0
[ 57.410018]
-> #1 (&(&dev_priv->irq_lock)->rlock){-.-.}:
[ 57.410081] clear_gtiir+0x30/0x200 [i915]
[ 57.410116] execlists_reset+0x6e/0x2b0 [i915]
[ 57.410140] i915_reset_engine+0x111/0x190 [i915]
[ 57.410165] i915_handle_error+0x11a/0x4a0 [i915]
[ 57.410198] i915_hangcheck_elapsed+0x378/0x530 [i915]
[ 57.410204] process_one_work+0x248/0x6c0
[ 57.410207] worker_thread+0x37/0x380
[ 57.410211] kthread+0x119/0x130
[ 57.410215] ret_from_fork+0x3a/0x50
[ 57.410217]
-> #0 (&engine->timeline.lock/1){-.-.}:
[ 57.410224] _raw_spin_lock_irqsave+0x33/0x50
[ 57.410256] execlists_submit_request+0x2b/0x1a0 [i915]
[ 57.410289] submit_notify+0x8d/0x124 [i915]
[ 57.410314] __i915_sw_fence_complete+0x81/0x250 [i915]
[ 57.410339] dma_i915_sw_fence_wake+0xd/0x20 [i915]
[ 57.410344] dma_fence_signal_locked+0x79/0x200
[ 57.410368] notify_ring+0x2ba/0x480 [i915]
[ 57.410392] gen8_cs_irq_handler+0x39/0xa0 [i915]
[ 57.410416] gen11_irq_handler+0x2f0/0x420 [i915]
[ 57.410421] __handle_irq_event_percpu+0x42/0x370
[ 57.410425] handle_irq_event_percpu+0x2b/0x70
[ 57.410428] handle_irq_event+0x2f/0x50
[ 57.410432] handle_edge_irq+0xe7/0x190
[ 57.410436] handle_irq+0x67/0x160
[ 57.410439] do_IRQ+0x5e/0x120
[ 57.410445] ret_from_intr+0x0/0x1d
[ 57.410449] cpuidle_enter_state+0xac/0x360
[ 57.410453] do_idle+0x1f3/0x250
[ 57.410456] cpu_startup_entry+0x6a/0x70
[ 57.410460] start_secondary+0x19d/0x1f0
[ 57.410464] secondary_startup_64+0xa5/0xb0
[ 57.410466]
other info that might help us debug this:
[ 57.410471] Chain exists of:
&engine->timeline.lock/1 --> &(&dev_priv->irq_lock)->rlock --> &(&rq->lock)->rlock#2
[ 57.410481] Possible unsafe locking scenario:
[ 57.410485] CPU0 CPU1
[ 57.410487] ---- ----
[ 57.410490] lock(&(&rq->lock)->rlock#2);
[ 57.410494] lock(&(&dev_priv->irq_lock)->rlock);
[ 57.410498] lock(&(&rq->lock)->rlock#2);
[ 57.410503] lock(&engine->timeline.lock/1);
[ 57.410506]
*** DEADLOCK ***
[ 57.410511] 4 locks held by swapper/6/0:
[ 57.410514] #0: 0000000074575789 (&(&dev_priv->irq_lock)->rlock){-.-.}, at: gen11_irq_handler+0x8a/0x420 [i915]
[ 57.410542] #1: 000000009b29b30e (rcu_read_lock){....}, at: notify_ring+0x1a/0x480 [i915]
[ 57.410573] #2: 00000000aad89594 (&(&rq->lock)->rlock#2){-.-.}, at: notify_ring+0x2b2/0x480 [i915]
[ 57.410601] #3: 000000009b29b30e (rcu_read_lock){....}, at: submit_notify+0x35/0x124 [i915]
[ 57.410635]
stack backtrace:
[ 57.410640] CPU: 6 PID: 0 Comm: swapper/6 Tainted: G U W 4.18.0-rc4-CI-CI_DII_1137+ #1
[ 57.410644] Hardware name: Intel Corporation Ice Lake Client Platform/IceLake U DDR4 SODIMM PD RVP, BIOS ICLSFWR1.R00.2222.A01.1805300339 05/30/2018
[ 57.410650] Call Trace:
[ 57.410652] <IRQ>
[ 57.410657] dump_stack+0x67/0x9b
[ 57.410662] print_circular_bug.isra.16+0x1c8/0x2b0
[ 57.410666] __lock_acquire+0x1897/0x1b50
[ 57.410671] ? lock_acquire+0xa6/0x210
[ 57.410674] lock_acquire+0xa6/0x210
[ 57.410706] ? execlists_submit_request+0x2b/0x1a0 [i915]
[ 57.410711] _raw_spin_lock_irqsave+0x33/0x50
[ 57.410741] ? execlists_submit_request+0x2b/0x1a0 [i915]
[ 57.410769] execlists_submit_request+0x2b/0x1a0 [i915]
[ 57.410774] ? _raw_spin_unlock_irqrestore+0x39/0x60
[ 57.410804] submit_notify+0x8d/0x124 [i915]
[ 57.410828] __i915_sw_fence_complete+0x81/0x250 [i915]
[ 57.410854] dma_i915_sw_fence_wake+0xd/0x20 [i915]
[ 57.410858] dma_fence_signal_locked+0x79/0x200
[ 57.410882] notify_ring+0x2ba/0x480 [i915]
[ 57.410907] gen8_cs_irq_handler+0x39/0xa0 [i915]
[ 57.410933] gen11_irq_handler+0x2f0/0x420 [i915]
[ 57.410938] __handle_irq_event_percpu+0x42/0x370
[ 57.410943] handle_irq_event_percpu+0x2b/0x70
[ 57.410947] handle_irq_event+0x2f/0x50
[ 57.410951] handle_edge_irq+0xe7/0x190
[ 57.410955] handle_irq+0x67/0x160
[ 57.410958] do_IRQ+0x5e/0x120
[ 57.410962] common_interrupt+0xf/0xf
[ 57.410965] </IRQ>
[ 57.410969] RIP: 0010:cpuidle_enter_state+0xac/0x360
[ 57.410972] Code: 44 00 00 31 ff e8 84 93 91 ff 45 84 f6 74 12 9c 58 f6 c4 02 0f 85 31 02 00 00 31 ff e8 7d 30 98 ff e8 e8 0e 94 ff fb 4c 29 fb <48> ba cf f7 53 e3 a5 9b c4 20 48 89 d8 48 c1 fb 3f 48 f7 ea b8 ff
[ 57.411015] RSP: 0018:ffffc90000133e90 EFLAGS: 00000216 ORIG_RAX: ffffffffffffffdd
[ 57.411023] RAX: ffff8804ae748040 RBX: 000000000002a97d RCX: 0000000000000000
[ 57.411029] RDX: 0000000000000046 RSI: ffffffff82141263 RDI: ffffffff820f05a7
[ 57.411035] RBP: 0000000000000001 R08: 0000000000000001 R09: 0000000000000000
[ 57.411041] R10: 0000000000000000 R11: 0000000000000000 R12: ffffffff8229f078
[ 57.411045] R13: ffff8804ab2adfa8 R14: 0000000000000000 R15: 0000000d5de092e3
[ 57.411052] do_idle+0x1f3/0x250
[ 57.411055] cpu_startup_entry+0x6a/0x70
[ 57.411059] start_secondary+0x19d/0x1f0
[ 57.411064] secondary_startup_64+0xa5/0xb0
And that looks quite genuine, and I can't immediately see a way around
it. It's a global iir bank, and interrupts are only disabled on the
local cpu.
Fortunately in the current state of the CSB processing we should now
avoid the need to clear_gtiir.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-07-13 8:28 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-24 21:39 [PATCH] drm/i915/icl: Correctly clear lost ctx-switch interrupts across reset for Gen11 Oscar Mateo
2018-04-24 22:19 ` ✓ Fi.CI.BAT: success for " Patchwork
2018-04-24 22:27 ` [PATCH] " Michel Thierry
2018-04-25 15:12 ` Chris Wilson
2018-04-24 23:05 ` ✗ Fi.CI.IGT: failure for " Patchwork
2018-07-13 8:28 ` [PATCH] " Chris Wilson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.