diff mbox

[1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework

Message ID 1491468259-5198-1-git-send-email-mika.kuoppala@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Mika Kuoppala April 6, 2017, 8:44 a.m. UTC 
From: Chris Wilson <chris@chris-wilson.co.uk>

Remove the per-mmio checking of the FIFO debug register into the common
conditional mmio debug handling. Based on patch from Chris Wilson.

Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/intel_uncore.c | 74 ++++++++++++++-----------------------
 1 file changed, 28 insertions(+), 46 deletions(-)

Comments

Chris Wilson April 6, 2017, 9:03 a.m. UTC | #1 
On Thu, Apr 06, 2017 at 11:44:18AM +0300, Mika Kuoppala wrote:
> From: Chris Wilson <chris@chris-wilson.co.uk>
From: Mika Kuoppala <mika.kuoppala@intel.com>

> Remove the per-mmio checking of the FIFO debug register into the common
> conditional mmio debug handling. Based on patch from Chris Wilson.
> 
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Or, keep me as the author, add your
r-b
[mkuoppal: keep a distinct error message for fifodbg]
s-o-b.
-Chris
Chris Wilson April 6, 2017, 9:14 a.m. UTC | #2 
On Thu, Apr 06, 2017 at 10:03:23AM +0100, Chris Wilson wrote:
> On Thu, Apr 06, 2017 at 11:44:18AM +0300, Mika Kuoppala wrote:
> > From: Chris Wilson <chris@chris-wilson.co.uk>
> From: Mika Kuoppala <mika.kuoppala@intel.com>
> 
> > Remove the per-mmio checking of the FIFO debug register into the common
> > conditional mmio debug handling. Based on patch from Chris Wilson.
> > 
> > Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> Or, keep me as the author, add your
> r-b
> [mkuoppal: keep a distinct error message for fifodbg]
> s-o-b.

Actually you fixed check_for_unclaimed_mmio() for me, which is more or
less the heart of the patch, so please do assign yourself authorship
with my r-b.
-Chris
Mika Kuoppala April 6, 2017, 11:13 a.m. UTC | #3 
Patchwork <patchwork@emeril.freedesktop.org> writes:

> == Series Details ==
>
> Series: series starting with [1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework
> URL   : https://patchwork.freedesktop.org/series/22571/
> State : warning
>
> == Summary ==
>
> Series 22571v1 Series without cover letter
> https://patchwork.freedesktop.org/api/1.0/series/22571/revisions/1/mbox/
>
> Test gem_exec_suspend:
>         Subgroup basic-s3:
>                 pass       -> DMESG-WARN (fi-byt-j1900)

GT_FIFO_SB_READ_ABORTERR set on resume. We have squashed this
previously.

-Mika


>         Subgroup basic-s4-devices:
>                 dmesg-warn -> PASS       (fi-kbl-7560u) fdo#100125
> Test kms_pipe_crc_basic:
>         Subgroup suspend-read-crc-pipe-a:
>                 pass       -> DMESG-WARN (fi-byt-j1900) fdo#100126
>         Subgroup suspend-read-crc-pipe-b:
>                 pass       -> DMESG-WARN (fi-byt-j1900)
>
> fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125
> fdo#100126 https://bugs.freedesktop.org/show_bug.cgi?id=100126
>
> fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time: 428s
> fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time: 425s
> fi-bsw-n3050     total:278  pass:242  dwarn:0   dfail:0   fail:0   skip:36  time: 581s
> fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time: 508s
> fi-bxt-t5700     total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  time: 565s
> fi-byt-j1900     total:278  pass:251  dwarn:3   dfail:0   fail:0   skip:24  time: 484s
> fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 413s
> fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 402s
> fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time: 418s
> fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 478s
> fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 463s
> fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 458s
> fi-kbl-7560u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 568s
> fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 451s
> fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time: 575s
> fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time: 465s
> fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 500s
> fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time: 433s
> fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time: 530s
> fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time: 401s
>
> 7303b362817aa4b4c62cdc2dfa586c272982917d drm-tip: 2017y-04m-06d-08h-26m-30s UTC integration manifest
> b8cf5ad drm/i915: Use wait_for_atomic_us when waiting for gt fifo
> 49b367b drm/i915: Move the GTFIFODBG to the common mmio dbg framework
>
> == Logs ==
>
> For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4417/
Chris Wilson April 6, 2017, 11:32 a.m. UTC | #4 
On Thu, Apr 06, 2017 at 02:13:44PM +0300, Mika Kuoppala wrote:
> Patchwork <patchwork@emeril.freedesktop.org> writes:
> 
> > == Series Details ==
> >
> > Series: series starting with [1/2] drm/i915: Move the GTFIFODBG to the common mmio dbg framework
> > URL   : https://patchwork.freedesktop.org/series/22571/
> > State : warning
> >
> > == Summary ==
> >
> > Series 22571v1 Series without cover letter
> > https://patchwork.freedesktop.org/api/1.0/series/22571/revisions/1/mbox/
> >
> > Test gem_exec_suspend:
> >         Subgroup basic-s3:
> >                 pass       -> DMESG-WARN (fi-byt-j1900)
> 
> GT_FIFO_SB_READ_ABORTERR set on resume. We have squashed this
> previously.

Ah, but the warn is now coming from the code to clear BIOS errors before
we begin.

It's reminding us that we can't use a WARN inside check_fifodbg. We have
the WARN already in __unclaimed_reg_debug, just make it a DRM_DEBUG
telltale.
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 6d1ea26..a129a73 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -172,22 +172,6 @@  static void fw_domains_get_with_thread_status(struct drm_i915_private *dev_priv,
 	__gen6_gt_wait_for_thread_c0(dev_priv);
 }
 
-static void gen6_gt_check_fifodbg(struct drm_i915_private *dev_priv)
-{
-	u32 gtfifodbg;
-
-	gtfifodbg = __raw_i915_read32(dev_priv, GTFIFODBG);
-	if (WARN(gtfifodbg, "GT wake FIFO error 0x%x\n", gtfifodbg))
-		__raw_i915_write32(dev_priv, GTFIFODBG, gtfifodbg);
-}
-
-static void fw_domains_put_with_fifo(struct drm_i915_private *dev_priv,
-				     enum forcewake_domains fw_domains)
-{
-	fw_domains_put(dev_priv, fw_domains);
-	gen6_gt_check_fifodbg(dev_priv);
-}
-
 static inline u32 fifo_free_entries(struct drm_i915_private *dev_priv)
 {
 	u32 count = __raw_i915_read32(dev_priv, GTFIFOCTL);
@@ -384,15 +368,33 @@  vlv_check_for_unclaimed_mmio(struct drm_i915_private *dev_priv)
 }
 
 static bool
+gen6_check_for_fifo_debug(struct drm_i915_private *dev_priv)
+{
+	u32 fifodbg;
+
+	fifodbg = __raw_i915_read32(dev_priv, GTFIFODBG);
+
+	if (WARN(fifodbg, "GTFIFODBG = 0x%08x\n", fifodbg))
+		__raw_i915_write32(dev_priv, GTFIFODBG, fifodbg);
+
+	return fifodbg;
+}
+
+static bool
 check_for_unclaimed_mmio(struct drm_i915_private *dev_priv)
 {
+	bool ret = false;
+
 	if (HAS_FPGA_DBG_UNCLAIMED(dev_priv))
-		return fpga_check_for_unclaimed_mmio(dev_priv);
+		ret |= fpga_check_for_unclaimed_mmio(dev_priv);
 
 	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
-		return vlv_check_for_unclaimed_mmio(dev_priv);
+		ret |= vlv_check_for_unclaimed_mmio(dev_priv);
 
-	return false;
+	if (IS_GEN6(dev_priv) || IS_GEN7(dev_priv))
+		ret |= gen6_check_for_fifo_debug(dev_priv);
+
+	return ret;
 }
 
 static void __intel_uncore_early_sanitize(struct drm_i915_private *dev_priv,
@@ -404,11 +406,6 @@  static void __intel_uncore_early_sanitize(struct drm_i915_private *dev_priv,
 	if (check_for_unclaimed_mmio(dev_priv))
 		DRM_DEBUG("unclaimed mmio detected on uncore init, clearing\n");
 
-	/* clear out old GT FIFO errors */
-	if (IS_GEN6(dev_priv) || IS_GEN7(dev_priv))
-		__raw_i915_write32(dev_priv, GTFIFODBG,
-				   __raw_i915_read32(dev_priv, GTFIFODBG));
-
 	/* WaDisableShadowRegForCpd:chv */
 	if (IS_CHERRYVIEW(dev_priv)) {
 		__raw_i915_write32(dev_priv, GTFIFOCTL,
@@ -1047,15 +1044,10 @@  __gen2_write(32)
 #define __gen6_write(x) \
 static void \
 gen6_write##x(struct drm_i915_private *dev_priv, i915_reg_t reg, u##x val, bool trace) { \
-	u32 __fifo_ret = 0; \
 	GEN6_WRITE_HEADER; \
-	if (NEEDS_FORCE_WAKE(offset)) { \
-		__fifo_ret = __gen6_gt_wait_for_fifo(dev_priv); \
-	} \
+	if (NEEDS_FORCE_WAKE(offset)) \
+		__gen6_gt_wait_for_fifo(dev_priv); \
 	__raw_i915_write##x(dev_priv, reg, val); \
-	if (unlikely(__fifo_ret)) { \
-		gen6_gt_check_fifodbg(dev_priv); \
-	} \
 	GEN6_WRITE_FOOTER; \
 }
 
@@ -1190,11 +1182,7 @@  static void intel_uncore_fw_domains_init(struct drm_i915_private *dev_priv)
 			       FORCEWAKE_MEDIA_GEN9, FORCEWAKE_ACK_MEDIA_GEN9);
 	} else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
 		dev_priv->uncore.funcs.force_wake_get = fw_domains_get;
-		if (!IS_CHERRYVIEW(dev_priv))
-			dev_priv->uncore.funcs.force_wake_put =
-				fw_domains_put_with_fifo;
-		else
-			dev_priv->uncore.funcs.force_wake_put = fw_domains_put;
+		dev_priv->uncore.funcs.force_wake_put = fw_domains_put;
 		fw_domain_init(dev_priv, FW_DOMAIN_ID_RENDER,
 			       FORCEWAKE_VLV, FORCEWAKE_ACK_VLV);
 		fw_domain_init(dev_priv, FW_DOMAIN_ID_MEDIA,
@@ -1202,11 +1190,7 @@  static void intel_uncore_fw_domains_init(struct drm_i915_private *dev_priv)
 	} else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
 		dev_priv->uncore.funcs.force_wake_get =
 			fw_domains_get_with_thread_status;
-		if (IS_HASWELL(dev_priv))
-			dev_priv->uncore.funcs.force_wake_put =
-				fw_domains_put_with_fifo;
-		else
-			dev_priv->uncore.funcs.force_wake_put = fw_domains_put;
+		dev_priv->uncore.funcs.force_wake_put = fw_domains_put;
 		fw_domain_init(dev_priv, FW_DOMAIN_ID_RENDER,
 			       FORCEWAKE_MT, FORCEWAKE_ACK_HSW);
 	} else if (IS_IVYBRIDGE(dev_priv)) {
@@ -1223,8 +1207,7 @@  static void intel_uncore_fw_domains_init(struct drm_i915_private *dev_priv)
 		 */
 		dev_priv->uncore.funcs.force_wake_get =
 			fw_domains_get_with_thread_status;
-		dev_priv->uncore.funcs.force_wake_put =
-			fw_domains_put_with_fifo;
+		dev_priv->uncore.funcs.force_wake_put = fw_domains_put;
 
 		/* We need to init first for ECOBUS access and then
 		 * determine later if we want to reinit, in case of MT access is
@@ -1242,7 +1225,7 @@  static void intel_uncore_fw_domains_init(struct drm_i915_private *dev_priv)
 		spin_lock_irq(&dev_priv->uncore.lock);
 		fw_domains_get_with_thread_status(dev_priv, FORCEWAKE_RENDER);
 		ecobus = __raw_i915_read32(dev_priv, ECOBUS);
-		fw_domains_put_with_fifo(dev_priv, FORCEWAKE_RENDER);
+		fw_domains_put(dev_priv, FORCEWAKE_RENDER);
 		spin_unlock_irq(&dev_priv->uncore.lock);
 
 		if (!(ecobus & FORCEWAKE_MT_ENABLE)) {
@@ -1254,8 +1237,7 @@  static void intel_uncore_fw_domains_init(struct drm_i915_private *dev_priv)
 	} else if (IS_GEN6(dev_priv)) {
 		dev_priv->uncore.funcs.force_wake_get =
 			fw_domains_get_with_thread_status;
-		dev_priv->uncore.funcs.force_wake_put =
-			fw_domains_put_with_fifo;
+		dev_priv->uncore.funcs.force_wake_put = fw_domains_put;
 		fw_domain_init(dev_priv, FW_DOMAIN_ID_RENDER,
 			       FORCEWAKE, FORCEWAKE_ACK);
 	}