All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/4] drm/i915/gen8: Disable master intr before reading
@ 2018-10-02 14:05 Mika Kuoppala
  2018-10-02 14:05 ` [PATCH 2/4] drm/i915/icl: No need to ack intr through master control Mika Kuoppala
                   ` (5 more replies)
  0 siblings, 6 replies; 12+ messages in thread
From: Mika Kuoppala @ 2018-10-02 14:05 UTC (permalink / raw)
  To: intel-gfx

Disable master interrupt before reading level indications.
This will close a race where we get a level indication between
reading and disabling, generating an extra interrupt where we
could have avoided one.

Further, as the reading acts also as a post, replace the
write/post on the irq reset with the helper. On enabling side,
posting doesn't serve any purpose so it can also be replaced
with helper.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 36 ++++++++++++++++++++++++---------
 1 file changed, 26 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 2e242270e270..cbc04dd59041 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2887,21 +2887,39 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
 	return ret;
 }
 
+static inline u32 gen8_master_intr_disable(void __iomem * const regs)
+{
+	raw_reg_write(regs, GEN8_MASTER_IRQ, 0);
+
+	/*
+	 * Now with master disabled, get a sample of level indications
+	 * for this interrupt. Indications will be cleared on related acks.
+	 * New indications can and will light up during processing,
+	 * and will generate new interrupt after enabling master.
+	 */
+	return raw_reg_read(regs, GEN8_MASTER_IRQ);
+}
+
+static inline void gen8_master_intr_enable(void __iomem * const regs)
+{
+	raw_reg_write(regs, GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL);
+}
+
 static irqreturn_t gen8_irq_handler(int irq, void *arg)
 {
 	struct drm_i915_private *dev_priv = to_i915(arg);
+	void __iomem * const regs = dev_priv->regs;
 	u32 master_ctl;
 	u32 gt_iir[4];
 
 	if (!intel_irqs_enabled(dev_priv))
 		return IRQ_NONE;
 
-	master_ctl = I915_READ_FW(GEN8_MASTER_IRQ);
-	master_ctl &= ~GEN8_MASTER_IRQ_CONTROL;
-	if (!master_ctl)
+	master_ctl = gen8_master_intr_disable(regs);
+	if (!master_ctl) {
+		gen8_master_intr_enable(regs);
 		return IRQ_NONE;
-
-	I915_WRITE_FW(GEN8_MASTER_IRQ, 0);
+	}
 
 	/* Find, clear, then process each source of interrupt */
 	gen8_gt_irq_ack(dev_priv, master_ctl, gt_iir);
@@ -2913,7 +2931,7 @@ static irqreturn_t gen8_irq_handler(int irq, void *arg)
 		enable_rpm_wakeref_asserts(dev_priv);
 	}
 
-	I915_WRITE_FW(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL);
+	gen8_master_intr_enable(regs);
 
 	gen8_gt_irq_handler(dev_priv, master_ctl, gt_iir);
 
@@ -3598,8 +3616,7 @@ static void gen8_irq_reset(struct drm_device *dev)
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	int pipe;
 
-	I915_WRITE(GEN8_MASTER_IRQ, 0);
-	POSTING_READ(GEN8_MASTER_IRQ);
+	gen8_master_intr_disable(dev_priv->regs);
 
 	gen8_gt_irq_reset(dev_priv);
 
@@ -4244,8 +4261,7 @@ static int gen8_irq_postinstall(struct drm_device *dev)
 	if (HAS_PCH_SPLIT(dev_priv))
 		ibx_irq_postinstall(dev);
 
-	I915_WRITE(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL);
-	POSTING_READ(GEN8_MASTER_IRQ);
+	gen8_master_intr_enable(dev_priv->regs);
 
 	return 0;
 }
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/4] drm/i915/icl: No need to ack intr through master control
  2018-10-02 14:05 [PATCH 1/4] drm/i915/gen8: Disable master intr before reading Mika Kuoppala
@ 2018-10-02 14:05 ` Mika Kuoppala
  2018-10-12 11:12   ` Matthew Auld
  2018-10-02 14:05 ` [PATCH 3/4] drm/i915/icl: Disable master intr before reading Mika Kuoppala
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Mika Kuoppala @ 2018-10-02 14:05 UTC (permalink / raw)
  To: intel-gfx; +Cc: Dhinakaran Pandiyan

All other master control register bits, except the enable,
are read only and they are level indications of the second
level interrupt status. Only touch enable bit and rectify
the comment.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index cbc04dd59041..e0310ebd9c8d 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -3165,8 +3165,8 @@ static irqreturn_t gen11_irq_handler(int irq, void *arg)
 
 	gu_misc_iir = gen11_gu_misc_irq_ack(i915, master_ctl);
 
-	/* Acknowledge and enable interrupts. */
-	raw_reg_write(regs, GEN11_GFX_MSTR_IRQ, GEN11_MASTER_IRQ | master_ctl);
+	/* Enable interrupts. */
+	raw_reg_write(regs, GEN11_GFX_MSTR_IRQ, GEN11_MASTER_IRQ);
 
 	gen11_gu_misc_irq_handler(i915, gu_misc_iir);
 
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/4] drm/i915/icl: Disable master intr before reading
  2018-10-02 14:05 [PATCH 1/4] drm/i915/gen8: Disable master intr before reading Mika Kuoppala
  2018-10-02 14:05 ` [PATCH 2/4] drm/i915/icl: No need to ack intr through master control Mika Kuoppala
@ 2018-10-02 14:05 ` Mika Kuoppala
  2018-10-02 15:18   ` Chris Wilson
  2018-10-02 14:05 ` [PATCH 4/4] drm/i915/icl: Handle GT interrupts after enabling master Mika Kuoppala
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Mika Kuoppala @ 2018-10-02 14:05 UTC (permalink / raw)
  To: intel-gfx

Disable master interrupt before reading level indications.
This will close a race where we get a level indication between
reading and disabling, generating an extra interrupt where we
could have avoided one.

Further, as the reading acts also as a post, replace the
write/post on the irq reset with the helper. On enabling side,
posting doesn't serve any purpose so it can also be replaced
with helper.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 37 ++++++++++++++++++++++-----------
 1 file changed, 25 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index e0310ebd9c8d..5d1f53723388 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -3129,6 +3129,24 @@ gen11_gu_misc_irq_handler(struct drm_i915_private *dev_priv, const u32 iir)
 		intel_opregion_asle_intr(dev_priv);
 }
 
+static inline u32 gen11_master_intr_disable(void __iomem * const regs)
+{
+	raw_reg_write(regs, GEN11_GFX_MSTR_IRQ, 0);
+
+	/*
+	 * Now with master disabled, get a sample of level indications
+	 * for this interrupt. Indications will be cleared on related acks.
+	 * New indications can and will light up during processing,
+	 * and will generate new interrupt after enabling master.
+	 */
+	return raw_reg_read(regs, GEN11_GFX_MSTR_IRQ);
+}
+
+static inline void gen11_master_intr_enable(void __iomem * const regs)
+{
+	raw_reg_write(regs, GEN11_GFX_MSTR_IRQ, GEN11_MASTER_IRQ);
+}
+
 static irqreturn_t gen11_irq_handler(int irq, void *arg)
 {
 	struct drm_i915_private * const i915 = to_i915(arg);
@@ -3139,13 +3157,11 @@ static irqreturn_t gen11_irq_handler(int irq, void *arg)
 	if (!intel_irqs_enabled(i915))
 		return IRQ_NONE;
 
-	master_ctl = raw_reg_read(regs, GEN11_GFX_MSTR_IRQ);
-	master_ctl &= ~GEN11_MASTER_IRQ;
-	if (!master_ctl)
+	master_ctl = gen11_master_intr_disable(regs);
+	if (!master_ctl) {
+		gen11_master_intr_enable(regs);
 		return IRQ_NONE;
-
-	/* Disable interrupts. */
-	raw_reg_write(regs, GEN11_GFX_MSTR_IRQ, 0);
+	}
 
 	/* Find, clear, then process each source of interrupt. */
 	gen11_gt_irq_handler(i915, master_ctl);
@@ -3165,8 +3181,7 @@ static irqreturn_t gen11_irq_handler(int irq, void *arg)
 
 	gu_misc_iir = gen11_gu_misc_irq_ack(i915, master_ctl);
 
-	/* Enable interrupts. */
-	raw_reg_write(regs, GEN11_GFX_MSTR_IRQ, GEN11_MASTER_IRQ);
+	gen11_master_intr_enable(regs);
 
 	gen11_gu_misc_irq_handler(i915, gu_misc_iir);
 
@@ -3658,8 +3673,7 @@ static void gen11_irq_reset(struct drm_device *dev)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	int pipe;
 
-	I915_WRITE(GEN11_GFX_MSTR_IRQ, 0);
-	POSTING_READ(GEN11_GFX_MSTR_IRQ);
+	gen11_master_intr_disable(dev_priv->regs);
 
 	gen11_gt_irq_reset(dev_priv);
 
@@ -4323,8 +4337,7 @@ static int gen11_irq_postinstall(struct drm_device *dev)
 
 	I915_WRITE(GEN11_DISPLAY_INT_CTL, GEN11_DISPLAY_IRQ_ENABLE);
 
-	I915_WRITE(GEN11_GFX_MSTR_IRQ, GEN11_MASTER_IRQ);
-	POSTING_READ(GEN11_GFX_MSTR_IRQ);
+	gen11_master_intr_enable(dev_priv->regs);
 
 	return 0;
 }
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 4/4] drm/i915/icl: Handle GT interrupts after enabling master
  2018-10-02 14:05 [PATCH 1/4] drm/i915/gen8: Disable master intr before reading Mika Kuoppala
  2018-10-02 14:05 ` [PATCH 2/4] drm/i915/icl: No need to ack intr through master control Mika Kuoppala
  2018-10-02 14:05 ` [PATCH 3/4] drm/i915/icl: Disable master intr before reading Mika Kuoppala
@ 2018-10-02 14:05 ` Mika Kuoppala
  2018-10-02 15:21   ` Chris Wilson
  2018-10-02 14:38 ` ✓ Fi.CI.BAT: success for series starting with [1/4] drm/i915/gen8: Disable master intr before reading Patchwork
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Mika Kuoppala @ 2018-10-02 14:05 UTC (permalink / raw)
  To: intel-gfx

Don't keep master disabled while we handle the current
interrupts. This should help a little on latency of
generating the next interrupt.

Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 5d1f53723388..9d8d5fb68c84 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -3163,9 +3163,6 @@ static irqreturn_t gen11_irq_handler(int irq, void *arg)
 		return IRQ_NONE;
 	}
 
-	/* Find, clear, then process each source of interrupt. */
-	gen11_gt_irq_handler(i915, master_ctl);
-
 	/* IRQs are synced during runtime_suspend, we don't require a wakeref */
 	if (master_ctl & GEN11_DISPLAY_IRQ) {
 		const u32 disp_ctl = raw_reg_read(regs, GEN11_DISPLAY_INT_CTL);
@@ -3183,6 +3180,7 @@ static irqreturn_t gen11_irq_handler(int irq, void *arg)
 
 	gen11_master_intr_enable(regs);
 
+	gen11_gt_irq_handler(i915, master_ctl);
 	gen11_gu_misc_irq_handler(i915, gu_misc_iir);
 
 	return IRQ_HANDLED;
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* ✓ Fi.CI.BAT: success for series starting with [1/4] drm/i915/gen8: Disable master intr before reading
  2018-10-02 14:05 [PATCH 1/4] drm/i915/gen8: Disable master intr before reading Mika Kuoppala
                   ` (2 preceding siblings ...)
  2018-10-02 14:05 ` [PATCH 4/4] drm/i915/icl: Handle GT interrupts after enabling master Mika Kuoppala
@ 2018-10-02 14:38 ` Patchwork
  2018-10-02 15:17 ` [PATCH 1/4] " Chris Wilson
  2018-10-03  6:29 ` ✗ Fi.CI.IGT: failure for series starting with [1/4] " Patchwork
  5 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2018-10-02 14:38 UTC (permalink / raw)
  To: Mika Kuoppala; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/4] drm/i915/gen8: Disable master intr before reading
URL   : https://patchwork.freedesktop.org/series/50446/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4915 -> Patchwork_10326 =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_10326 need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_10326, 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/50446/revisions/1/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_10326:

  === IGT changes ===

    ==== Warnings ====

    igt@drv_selftest@live_guc:
      fi-glk-j4005:       SKIP -> PASS

    
== Known issues ==

  Here are the changes found in Patchwork_10326 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
      fi-byt-clapper:     PASS -> FAIL (fdo#103191, fdo#107362)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
      fi-bdw-samus:       NOTRUN -> INCOMPLETE (fdo#107773)

    
    ==== Possible fixes ====

    igt@drv_selftest@live_coherency:
      fi-gdg-551:         DMESG-FAIL (fdo#107164) -> PASS

    igt@drv_selftest@live_execlists:
      fi-glk-j4005:       INCOMPLETE (fdo#103359, k.org#198133) -> PASS

    igt@gem_exec_suspend@basic-s3:
      fi-bdw-samus:       INCOMPLETE (fdo#107773) -> PASS

    igt@kms_pipe_crc_basic@read-crc-pipe-a-frame-sequence:
      fi-byt-clapper:     FAIL (fdo#103191, fdo#107362) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-skl-guc:         FAIL (fdo#103191) -> PASS

    
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#107164 https://bugs.freedesktop.org/show_bug.cgi?id=107164
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107773 https://bugs.freedesktop.org/show_bug.cgi?id=107773
  k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133


== Participating hosts (46 -> 43) ==

  Missing    (3): fi-bsw-cyan fi-byt-squawks fi-icl-u2 


== Build changes ==

    * Linux: CI_DRM_4915 -> Patchwork_10326

  CI_DRM_4915: 26e7a7d954a9c28b97af8ca7813f430fd9117232 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4660: d0975646c50568e66e65b44b81d28232d059b94e @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10326: c5f3d928e43187341db9763681a44588bf317b64 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

c5f3d928e431 drm/i915/icl: Handle GT interrupts after enabling master
6ff752befaeb drm/i915/icl: Disable master intr before reading
e8edf396a914 drm/i915/icl: No need to ack intr through master control
3fb62f2ecfab drm/i915/gen8: Disable master intr before reading

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10326/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/4] drm/i915/gen8: Disable master intr before reading
  2018-10-02 14:05 [PATCH 1/4] drm/i915/gen8: Disable master intr before reading Mika Kuoppala
                   ` (3 preceding siblings ...)
  2018-10-02 14:38 ` ✓ Fi.CI.BAT: success for series starting with [1/4] drm/i915/gen8: Disable master intr before reading Patchwork
@ 2018-10-02 15:17 ` Chris Wilson
  2018-10-03  6:29 ` ✗ Fi.CI.IGT: failure for series starting with [1/4] " Patchwork
  5 siblings, 0 replies; 12+ messages in thread
From: Chris Wilson @ 2018-10-02 15:17 UTC (permalink / raw)
  To: Mika Kuoppala, intel-gfx

Quoting Mika Kuoppala (2018-10-02 15:05:49)
> Disable master interrupt before reading level indications.
> This will close a race where we get a level indication between
> reading and disabling, generating an extra interrupt where we
> could have avoided one.
> 
> Further, as the reading acts also as a post, replace the
> write/post on the irq reset with the helper. On enabling side,
> posting doesn't serve any purpose so it can also be replaced
> with helper.
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

Proof is in the eating and your experiments have shown that MASTER_IRQ
is not cleared until the sublevels are acked. If it were the opposite
then we would quickly see missed interrupts.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Though I would run it through CI a few times before merging just in
case.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/4] drm/i915/icl: Disable master intr before reading
  2018-10-02 14:05 ` [PATCH 3/4] drm/i915/icl: Disable master intr before reading Mika Kuoppala
@ 2018-10-02 15:18   ` Chris Wilson
  0 siblings, 0 replies; 12+ messages in thread
From: Chris Wilson @ 2018-10-02 15:18 UTC (permalink / raw)
  To: Mika Kuoppala, intel-gfx

Quoting Mika Kuoppala (2018-10-02 15:05:51)
> Disable master interrupt before reading level indications.
> This will close a race where we get a level indication between
> reading and disabling, generating an extra interrupt where we
> could have avoided one.
> 
> Further, as the reading acts also as a post, replace the
> write/post on the irq reset with the helper. On enabling side,
> posting doesn't serve any purpose so it can also be replaced
> with helper.
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

Same argument and complementary pattern to gen8, so hopefully they apply
equally :)

Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 4/4] drm/i915/icl: Handle GT interrupts after enabling master
  2018-10-02 14:05 ` [PATCH 4/4] drm/i915/icl: Handle GT interrupts after enabling master Mika Kuoppala
@ 2018-10-02 15:21   ` Chris Wilson
  2018-10-11 14:19     ` Mika Kuoppala
  0 siblings, 1 reply; 12+ messages in thread
From: Chris Wilson @ 2018-10-02 15:21 UTC (permalink / raw)
  To: Mika Kuoppala, intel-gfx

Quoting Mika Kuoppala (2018-10-02 15:05:52)
> Don't keep master disabled while we handle the current
> interrupts. This should help a little on latency of
> generating the next interrupt.
> 
> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 5d1f53723388..9d8d5fb68c84 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -3163,9 +3163,6 @@ static irqreturn_t gen11_irq_handler(int irq, void *arg)
>                 return IRQ_NONE;
>         }
>  
> -       /* Find, clear, then process each source of interrupt. */
> -       gen11_gt_irq_handler(i915, master_ctl);
> -
>         /* IRQs are synced during runtime_suspend, we don't require a wakeref */
>         if (master_ctl & GEN11_DISPLAY_IRQ) {
>                 const u32 disp_ctl = raw_reg_read(regs, GEN11_DISPLAY_INT_CTL);
> @@ -3183,6 +3180,7 @@ static irqreturn_t gen11_irq_handler(int irq, void *arg)
>  
>         gen11_master_intr_enable(regs);
>  
> +       gen11_gt_irq_handler(i915, master_ctl);

Are we not still acking GT_INTR_DW at this point?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* ✗ Fi.CI.IGT: failure for series starting with [1/4] drm/i915/gen8: Disable master intr before reading
  2018-10-02 14:05 [PATCH 1/4] drm/i915/gen8: Disable master intr before reading Mika Kuoppala
                   ` (4 preceding siblings ...)
  2018-10-02 15:17 ` [PATCH 1/4] " Chris Wilson
@ 2018-10-03  6:29 ` Patchwork
  5 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2018-10-03  6:29 UTC (permalink / raw)
  To: Mika Kuoppala; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/4] drm/i915/gen8: Disable master intr before reading
URL   : https://patchwork.freedesktop.org/series/50446/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4915_full -> Patchwork_10326_full =

== Summary - FAILURE ==

  Serious unknown changes coming with Patchwork_10326_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_10326_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_10326_full:

  === IGT changes ===

    ==== Possible regressions ====

    igt@kms_plane_alpha_blend@pipe-b-alpha-basic:
      shard-skl:          NOTRUN -> FAIL +1

    
== Known issues ==

  Here are the changes found in Patchwork_10326_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_schedule@pi-ringfull-bsd:
      shard-skl:          NOTRUN -> FAIL (fdo#103158)

    igt@gem_exec_suspend@basic-s3:
      shard-snb:          PASS -> INCOMPLETE (fdo#105411)

    igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-c:
      shard-skl:          NOTRUN -> DMESG-WARN (fdo#107956)

    igt@kms_cursor_crc@cursor-256x256-random:
      shard-skl:          NOTRUN -> FAIL (fdo#103232)

    igt@kms_cursor_crc@cursor-256x256-suspend:
      shard-apl:          PASS -> FAIL (fdo#103232, fdo#103191)

    igt@kms_cursor_crc@cursor-256x85-random:
      shard-apl:          PASS -> FAIL (fdo#103232) +3

    igt@kms_cursor_crc@cursor-64x64-sliding:
      shard-glk:          PASS -> FAIL (fdo#103232) +2

    igt@kms_flip@flip-vs-expired-vblank-interruptible:
      shard-skl:          PASS -> FAIL (fdo#105363)

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-wc:
      shard-glk:          PASS -> FAIL (fdo#103167) +2

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-render:
      shard-apl:          PASS -> FAIL (fdo#103167) +2

    igt@kms_plane@pixel-format-pipe-b-planes:
      shard-skl:          NOTRUN -> DMESG-WARN (fdo#106885)

    igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes:
      shard-kbl:          PASS -> INCOMPLETE (fdo#103665)

    igt@kms_plane@plane-position-covered-pipe-a-planes:
      shard-apl:          PASS -> FAIL (fdo#103166)

    igt@kms_setmode@basic:
      shard-snb:          NOTRUN -> FAIL (fdo#99912)

    
    ==== Possible fixes ====

    igt@kms_busy@extended-modeset-hang-newfb-render-b:
      shard-kbl:          DMESG-WARN (fdo#107956) -> PASS

    igt@kms_ccs@pipe-b-missing-ccs-buffer:
      shard-kbl:          DMESG-WARN (fdo#105602, fdo#103558) -> PASS +14

    igt@kms_color@pipe-b-ctm-max:
      shard-apl:          FAIL -> PASS

    igt@kms_cursor_crc@cursor-128x128-suspend:
      shard-glk:          FAIL (fdo#103232) -> PASS
      shard-apl:          FAIL (fdo#103232, fdo#103191) -> PASS +1

    igt@kms_cursor_crc@cursor-256x85-sliding:
      shard-apl:          FAIL (fdo#103232) -> PASS +1

    igt@kms_cursor_legacy@cursorb-vs-flipb-toggle:
      shard-glk:          DMESG-WARN (fdo#105763, fdo#106538) -> PASS

    igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
      shard-glk:          FAIL (fdo#105363) -> PASS

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-pwrite:
      shard-apl:          FAIL (fdo#103167) -> PASS +1

    igt@kms_frontbuffer_tracking@fbc-2p-primscrn-pri-shrfb-draw-mmap-cpu:
      shard-glk:          FAIL (fdo#103167) -> PASS +1

    igt@kms_plane@plane-position-covered-pipe-a-planes:
      shard-glk:          FAIL (fdo#103166) -> PASS

    igt@kms_plane_multiple@atomic-pipe-a-tiling-x:
      shard-apl:          FAIL (fdo#103166) -> PASS +1

    igt@kms_setmode@basic:
      shard-apl:          FAIL (fdo#99912) -> PASS

    
  fdo#103158 https://bugs.freedesktop.org/show_bug.cgi?id=103158
  fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#103232 https://bugs.freedesktop.org/show_bug.cgi?id=103232
  fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
  fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
  fdo#105763 https://bugs.freedesktop.org/show_bug.cgi?id=105763
  fdo#106538 https://bugs.freedesktop.org/show_bug.cgi?id=106538
  fdo#106885 https://bugs.freedesktop.org/show_bug.cgi?id=106885
  fdo#107956 https://bugs.freedesktop.org/show_bug.cgi?id=107956
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (6 -> 6) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4915 -> Patchwork_10326

  CI_DRM_4915: 26e7a7d954a9c28b97af8ca7813f430fd9117232 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4660: d0975646c50568e66e65b44b81d28232d059b94e @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10326: c5f3d928e43187341db9763681a44588bf317b64 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10326/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 4/4] drm/i915/icl: Handle GT interrupts after enabling master
  2018-10-02 15:21   ` Chris Wilson
@ 2018-10-11 14:19     ` Mika Kuoppala
  2018-10-11 14:56       ` Chris Wilson
  0 siblings, 1 reply; 12+ messages in thread
From: Mika Kuoppala @ 2018-10-11 14:19 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Quoting Mika Kuoppala (2018-10-02 15:05:52)
>> Don't keep master disabled while we handle the current
>> interrupts. This should help a little on latency of
>> generating the next interrupt.
>> 
>> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_irq.c | 4 +---
>>  1 file changed, 1 insertion(+), 3 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>> index 5d1f53723388..9d8d5fb68c84 100644
>> --- a/drivers/gpu/drm/i915/i915_irq.c
>> +++ b/drivers/gpu/drm/i915/i915_irq.c
>> @@ -3163,9 +3163,6 @@ static irqreturn_t gen11_irq_handler(int irq, void *arg)
>>                 return IRQ_NONE;
>>         }
>>  
>> -       /* Find, clear, then process each source of interrupt. */
>> -       gen11_gt_irq_handler(i915, master_ctl);
>> -
>>         /* IRQs are synced during runtime_suspend, we don't require a wakeref */
>>         if (master_ctl & GEN11_DISPLAY_IRQ) {
>>                 const u32 disp_ctl = raw_reg_read(regs, GEN11_DISPLAY_INT_CTL);
>> @@ -3183,6 +3180,7 @@ static irqreturn_t gen11_irq_handler(int irq, void *arg)
>>  
>>         gen11_master_intr_enable(regs);
>>  
>> +       gen11_gt_irq_handler(i915, master_ctl);
>
> Are we not still acking GT_INTR_DW at this point?

We are.
-Mika

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 4/4] drm/i915/icl: Handle GT interrupts after enabling master
  2018-10-11 14:19     ` Mika Kuoppala
@ 2018-10-11 14:56       ` Chris Wilson
  0 siblings, 0 replies; 12+ messages in thread
From: Chris Wilson @ 2018-10-11 14:56 UTC (permalink / raw)
  To: Mika Kuoppala, intel-gfx

Quoting Mika Kuoppala (2018-10-11 15:19:24)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > Quoting Mika Kuoppala (2018-10-02 15:05:52)
> >> Don't keep master disabled while we handle the current
> >> interrupts. This should help a little on latency of
> >> generating the next interrupt.
> >> 
> >> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> >> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> >> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/i915_irq.c | 4 +---
> >>  1 file changed, 1 insertion(+), 3 deletions(-)
> >> 
> >> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> >> index 5d1f53723388..9d8d5fb68c84 100644
> >> --- a/drivers/gpu/drm/i915/i915_irq.c
> >> +++ b/drivers/gpu/drm/i915/i915_irq.c
> >> @@ -3163,9 +3163,6 @@ static irqreturn_t gen11_irq_handler(int irq, void *arg)
> >>                 return IRQ_NONE;
> >>         }
> >>  
> >> -       /* Find, clear, then process each source of interrupt. */
> >> -       gen11_gt_irq_handler(i915, master_ctl);
> >> -
> >>         /* IRQs are synced during runtime_suspend, we don't require a wakeref */
> >>         if (master_ctl & GEN11_DISPLAY_IRQ) {
> >>                 const u32 disp_ctl = raw_reg_read(regs, GEN11_DISPLAY_INT_CTL);
> >> @@ -3183,6 +3180,7 @@ static irqreturn_t gen11_irq_handler(int irq, void *arg)
> >>  
> >>         gen11_master_intr_enable(regs);
> >>  
> >> +       gen11_gt_irq_handler(i915, master_ctl);
> >
> > Are we not still acking GT_INTR_DW at this point?
> 
> We are.

Then we re-enable MASTER before we ack the in the individual GT_IIR, and
by the logic before, that means MASTER still has the GT bits asserted
(as we said they are not cleared until we ack the individual GT_IIR).
Doesn't that mean that the interrupt will be raised again immediately?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/4] drm/i915/icl: No need to ack intr through master control
  2018-10-02 14:05 ` [PATCH 2/4] drm/i915/icl: No need to ack intr through master control Mika Kuoppala
@ 2018-10-12 11:12   ` Matthew Auld
  0 siblings, 0 replies; 12+ messages in thread
From: Matthew Auld @ 2018-10-12 11:12 UTC (permalink / raw)
  To: Mika Kuoppala; +Cc: Intel Graphics Development, dhinakaran.pandiyan

On Tue, 2 Oct 2018 at 15:07, Mika Kuoppala
<mika.kuoppala@linux.intel.com> wrote:
>
> All other master control register bits, except the enable,
> are read only and they are level indications of the second
> level interrupt status. Only touch enable bit and rectify
> the comment.
>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2018-10-12 11:13 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-02 14:05 [PATCH 1/4] drm/i915/gen8: Disable master intr before reading Mika Kuoppala
2018-10-02 14:05 ` [PATCH 2/4] drm/i915/icl: No need to ack intr through master control Mika Kuoppala
2018-10-12 11:12   ` Matthew Auld
2018-10-02 14:05 ` [PATCH 3/4] drm/i915/icl: Disable master intr before reading Mika Kuoppala
2018-10-02 15:18   ` Chris Wilson
2018-10-02 14:05 ` [PATCH 4/4] drm/i915/icl: Handle GT interrupts after enabling master Mika Kuoppala
2018-10-02 15:21   ` Chris Wilson
2018-10-11 14:19     ` Mika Kuoppala
2018-10-11 14:56       ` Chris Wilson
2018-10-02 14:38 ` ✓ Fi.CI.BAT: success for series starting with [1/4] drm/i915/gen8: Disable master intr before reading Patchwork
2018-10-02 15:17 ` [PATCH 1/4] " Chris Wilson
2018-10-03  6:29 ` ✗ Fi.CI.IGT: failure for series starting with [1/4] " Patchwork

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.