diff mbox

[1/7] drm/i915: Consolidate unclaimed mmio detection

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

Commit Message

Mika Kuoppala Dec. 15, 2015, 2:25 p.m. UTC
Access the unclaimed reg detection register through
one helper which also does cleanup. Note that we now access
the register only if the platform has the actual non claimed
access bit. This prevents reading the register with gens that
doesn't have the register or the unclaimed bit,
when debug_mmio > 0.

Note that we post after clearing the bit. This makes sure
that the next unclaimed write access would get detected
also if it happened right after clearing, and not fold
into the previous detection.

v2: s/unclaimed_reg_access/check_for_unclaimed_mmio (Chris)
    debug log on unclaimed detection on uncore init (Joonas)

Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Paulo Zanoni <przanoni@gmail.com>
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/intel_uncore.c | 38 +++++++++++++++++++++++++------------
 1 file changed, 26 insertions(+), 12 deletions(-)

Comments

Chris Wilson Dec. 15, 2015, 2:38 p.m. UTC | #1
On Tue, Dec 15, 2015 at 04:25:06PM +0200, Mika Kuoppala wrote:
> Access the unclaimed reg detection register through
> one helper which also does cleanup. Note that we now access
> the register only if the platform has the actual non claimed
> access bit. This prevents reading the register with gens that
> doesn't have the register or the unclaimed bit,
> when debug_mmio > 0.
> 
> Note that we post after clearing the bit. This makes sure
> that the next unclaimed write access would get detected
> also if it happened right after clearing, and not fold
> into the previous detection.

I didn't like 6 (adding checks to rpm get/put), and I didn't see the
removal of the check from the interrupt? Or adding it to the modesetting
code?

Did I just miss those?

The other patches (1-5?) (aside from adding chv as I haven't validated the
registers myself) are
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
Mika Kuoppala Dec. 15, 2015, 2:40 p.m. UTC | #2
Chris Wilson <chris@chris-wilson.co.uk> writes:

> On Tue, Dec 15, 2015 at 04:25:06PM +0200, Mika Kuoppala wrote:
>> Access the unclaimed reg detection register through
>> one helper which also does cleanup. Note that we now access
>> the register only if the platform has the actual non claimed
>> access bit. This prevents reading the register with gens that
>> doesn't have the register or the unclaimed bit,
>> when debug_mmio > 0.
>> 
>> Note that we post after clearing the bit. This makes sure
>> that the next unclaimed write access would get detected
>> also if it happened right after clearing, and not fold
>> into the previous detection.
>
> I didn't like 6 (adding checks to rpm get/put), and I didn't see the
> removal of the check from the interrupt? Or adding it to the modesetting
> code?
>
> Did I just miss those?
>

They should be included in 4/7.
-Mika

> The other patches (1-5?) (aside from adding chv as I haven't validated the
> registers myself) are
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> -Chris
>
> -- 
> Chris Wilson, Intel Open Source Technology Centre
Mika Kuoppala Jan. 8, 2016, 2:58 p.m. UTC | #3
Chris Wilson <chris@chris-wilson.co.uk> writes:

> On Tue, Dec 15, 2015 at 04:25:06PM +0200, Mika Kuoppala wrote:
>> Access the unclaimed reg detection register through
>> one helper which also does cleanup. Note that we now access
>> the register only if the platform has the actual non claimed
>> access bit. This prevents reading the register with gens that
>> doesn't have the register or the unclaimed bit,
>> when debug_mmio > 0.
>> 
>> Note that we post after clearing the bit. This makes sure
>> that the next unclaimed write access would get detected
>> also if it happened right after clearing, and not fold
>> into the previous detection.
>
> I didn't like 6 (adding checks to rpm get/put), and I didn't see the
> removal of the check from the interrupt? Or adding it to the modesetting
> code?
>
> Did I just miss those?
>
> The other patches (1-5?) (aside from adding chv as I haven't validated the
> registers myself) are
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

1-5 queued for -next. Thanks for review.
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index fcf04fe..95e7803 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -334,13 +334,34 @@  static void intel_uncore_ellc_detect(struct drm_device *dev)
 	}
 }
 
+static bool
+check_for_unclaimed_mmio(struct drm_i915_private *dev_priv)
+{
+	u32 dbg;
+
+	if (!HAS_FPGA_DBG_UNCLAIMED(dev_priv))
+		return false;
+
+	dbg = __raw_i915_read32(dev_priv, FPGA_DBG);
+	if (likely(!(dbg & FPGA_DBG_RM_NOCLAIM)))
+		return false;
+
+	__raw_i915_write32(dev_priv, FPGA_DBG, FPGA_DBG_RM_NOCLAIM);
+
+	/* We want to clear it asap, so post */
+	__raw_i915_read32(dev_priv, FPGA_DBG);
+
+	return true;
+}
+
 static void __intel_uncore_early_sanitize(struct drm_device *dev,
 					  bool restore_forcewake)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
-	if (HAS_FPGA_DBG_UNCLAIMED(dev))
-		__raw_i915_write32(dev_priv, FPGA_DBG, FPGA_DBG_RM_NOCLAIM);
+	/* clear out unclaimed reg detection bit */
+	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) || IS_GEN7(dev))
@@ -601,10 +622,9 @@  hsw_unclaimed_reg_debug(struct drm_i915_private *dev_priv,
 	if (!i915.mmio_debug)
 		return;
 
-	if (__raw_i915_read32(dev_priv, FPGA_DBG) & FPGA_DBG_RM_NOCLAIM) {
+	if (check_for_unclaimed_mmio(dev_priv)) {
 		WARN(1, "Unclaimed register detected %s %s register 0x%x\n",
 		     when, op, i915_mmio_reg_offset(reg));
-		__raw_i915_write32(dev_priv, FPGA_DBG, FPGA_DBG_RM_NOCLAIM);
 		i915.mmio_debug--; /* Only report the first N failures */
 	}
 }
@@ -617,11 +637,10 @@  hsw_unclaimed_reg_detect(struct drm_i915_private *dev_priv)
 	if (i915.mmio_debug || !mmio_debug_once)
 		return;
 
-	if (__raw_i915_read32(dev_priv, FPGA_DBG) & FPGA_DBG_RM_NOCLAIM) {
+	if (check_for_unclaimed_mmio(dev_priv)) {
 		DRM_DEBUG("Unclaimed register detected, "
 			  "enabling oneshot unclaimed register reporting. "
 			  "Please use i915.mmio_debug=N for more information.\n");
-		__raw_i915_write32(dev_priv, FPGA_DBG, FPGA_DBG_RM_NOCLAIM);
 		i915.mmio_debug = mmio_debug_once--;
 	}
 }
@@ -1589,11 +1608,6 @@  bool intel_has_gpu_reset(struct drm_device *dev)
 
 void intel_uncore_check_errors(struct drm_device *dev)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
-
-	if (HAS_FPGA_DBG_UNCLAIMED(dev) &&
-	    (__raw_i915_read32(dev_priv, FPGA_DBG) & FPGA_DBG_RM_NOCLAIM)) {
+	if (check_for_unclaimed_mmio(to_i915(dev)))
 		DRM_ERROR("Unclaimed register before interrupt\n");
-		__raw_i915_write32(dev_priv, FPGA_DBG, FPGA_DBG_RM_NOCLAIM);
-	}
 }