[PATCH 1/3] drm/i915/gen8: Disable master intr before reading

[PATCH 1/3] drm/i915/gen8: Disable master intr before reading

[Mika Kuoppala]

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>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 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

[PATCH 2/3] drm/i915/icl: No need to ack intr through master control

[Mika Kuoppala]

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>
---
 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

[PATCH 3/3] drm/i915/icl: Disable master intr before reading

[Mika Kuoppala]

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>
Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 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

✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915/gen8: Disable master intr before reading

[Patchwork]

== Series Details ==

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

== Summary ==

= CI Bug Log - changes from CI_DRM_4981 -> Patchwork_10458 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/51009/revisions/1/mbox/

== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b:
      fi-byt-clapper:     PASS -> FAIL (fdo#107362)

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

    
    ==== Possible fixes ====

    igt@drv_selftest@live_hangcheck:
      fi-icl-u2:          INCOMPLETE (fdo#108315) -> PASS

    igt@gem_exec_suspend@basic-s3:
      fi-icl-u:           INCOMPLETE (fdo#107713) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-blb-e6850:       INCOMPLETE (fdo#107718) -> PASS

    igt@pm_rpm@module-reload:
      fi-skl-6600u:       INCOMPLETE (fdo#107807) -> PASS

    
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107713 https://bugs.freedesktop.org/show_bug.cgi?id=107713
  fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718
  fdo#107807 https://bugs.freedesktop.org/show_bug.cgi?id=107807
  fdo#108315 https://bugs.freedesktop.org/show_bug.cgi?id=108315


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

  Missing    (8): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-apl-guc fi-ctg-p8600 fi-gdg-551 fi-pnv-d510 


== Build changes ==

    * Linux: CI_DRM_4981 -> Patchwork_10458

  CI_DRM_4981: 79887268bfe4128788d7cfcf38b62308346fd7f1 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4677: 68ff28a022dbaa26a20c8a3c0212011a006614b0 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10458: 584e9458bfbd1176f076e88bc556c5b9dded2923 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

584e9458bfbd drm/i915/icl: Disable master intr before reading
d6f38f088417 drm/i915/icl: No need to ack intr through master control
156a3aff56b5 drm/i915/gen8: Disable master intr before reading

== Logs ==

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

Re: [PATCH 3/3] drm/i915/icl: Disable master intr before reading

[Daniele Ceraolo Spurio]

On 15/10/18 07:14, Mika Kuoppala wrote:
> 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>
> Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---

Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

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

✓ Fi.CI.IGT: success for series starting with [1/3] drm/i915/gen8: Disable master intr before reading

[Patchwork]

== Series Details ==

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

== Summary ==

= CI Bug Log - changes from CI_DRM_4981_full -> Patchwork_10458_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_10458_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_10458_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_10458_full:

  === IGT changes ===

    ==== Warnings ====

    igt@pm_rc6_residency@rc6-accuracy:
      shard-kbl:          SKIP -> PASS

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_ppgtt@blt-vs-render-ctxn:
      shard-skl:          NOTRUN -> TIMEOUT (fdo#108039)

    igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-a:
      shard-hsw:          PASS -> DMESG-WARN (fdo#107956)

    igt@kms_color@pipe-a-ctm-max:
      shard-apl:          PASS -> FAIL (fdo#108147)

    igt@kms_cursor_crc@cursor-256x256-random:
      shard-glk:          PASS -> FAIL (fdo#103232) +2

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

    igt@kms_cursor_crc@cursor-64x64-dpms:
      shard-apl:          PASS -> FAIL (fdo#103232)

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

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render:
      shard-skl:          NOTRUN -> FAIL (fdo#105682)

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

    igt@kms_frontbuffer_tracking@psr-1p-primscrn-indfb-pgflip-blt:
      shard-skl:          NOTRUN -> FAIL (fdo#103167) +4

    igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes:
      shard-skl:          NOTRUN -> FAIL (fdo#103166)

    igt@kms_plane_alpha_blend@pipe-a-alpha-7efc:
      shard-skl:          NOTRUN -> FAIL (fdo#108145) +3

    igt@kms_plane_alpha_blend@pipe-b-alpha-opaque-fb:
      shard-glk:          NOTRUN -> FAIL (fdo#108145)

    
    ==== Possible fixes ====

    igt@kms_color@pipe-a-ctm-negative:
      shard-skl:          FAIL -> PASS

    igt@kms_cursor_crc@cursor-128x42-sliding:
      shard-glk:          FAIL (fdo#103232) -> PASS +1

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

    igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-indfb-draw-mmap-wc:
      shard-skl:          FAIL (fdo#103167) -> PASS +2

    igt@kms_frontbuffer_tracking@fbc-2p-primscrn-cur-indfb-move:
      shard-glk:          FAIL (fdo#103167) -> PASS +4

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

    igt@kms_plane_alpha_blend@pipe-a-coverage-7efc:
      shard-skl:          FAIL (fdo#108145) -> PASS

    igt@kms_plane_alpha_blend@pipe-b-constant-alpha-mid:
      shard-glk:          DMESG-WARN (fdo#105763, fdo#106538) -> PASS +2

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

    igt@kms_rotation_crc@sprite-rotation-180:
      shard-kbl:          DMESG-WARN (fdo#105345) -> PASS

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

    
  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#105345 https://bugs.freedesktop.org/show_bug.cgi?id=105345
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#105682 https://bugs.freedesktop.org/show_bug.cgi?id=105682
  fdo#105763 https://bugs.freedesktop.org/show_bug.cgi?id=105763
  fdo#106538 https://bugs.freedesktop.org/show_bug.cgi?id=106538
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107956 https://bugs.freedesktop.org/show_bug.cgi?id=107956
  fdo#108039 https://bugs.freedesktop.org/show_bug.cgi?id=108039
  fdo#108145 https://bugs.freedesktop.org/show_bug.cgi?id=108145
  fdo#108147 https://bugs.freedesktop.org/show_bug.cgi?id=108147
  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_4981 -> Patchwork_10458

  CI_DRM_4981: 79887268bfe4128788d7cfcf38b62308346fd7f1 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4677: 68ff28a022dbaa26a20c8a3c0212011a006614b0 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10458: 584e9458bfbd1176f076e88bc556c5b9dded2923 @ 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_10458/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915/icl: Disable master intr before reading

[Mika Kuoppala]

Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> writes:

> On 15/10/18 07:14, Mika Kuoppala wrote:
>> 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>
>> Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
>> ---
>
> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

All 3 patches pushed. Thank you all for review.
-Mika
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx