We have done unclaimed register access check in normal
(mmio_debug=0) mode once per write. This adds probability
of finding the exact sequence where we did the bad access, but
also adds burden to each write.
As we have mmio_debug available for more fine grained analysis,
give up accuracy of detecting correct spot at the first occurrence
by doing the one shot detection and arming of mmio_debug in hangcheck
and in modeset. This removes the write path performance burden.
v2: Remove gratuitous DRM_DEBUG and return value, comments (Chris)
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Paulo Zanoni <przanoni@xxxxxxxxx>
Signed-off-by: Mika Kuoppala <mika.kuoppala@xxxxxxxxx>
Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
---
drivers/gpu/drm/i915/i915_drv.h | 3 +++
drivers/gpu/drm/i915/i915_irq.c | 11 ++++++-----
drivers/gpu/drm/i915/intel_display.c | 13 +++++++++++++
drivers/gpu/drm/i915/intel_uncore.c | 37 ++++++++++++++++++------------------
4 files changed, 40 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 82c43b6..554092e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -723,6 +723,8 @@ struct intel_uncore {
i915_reg_t reg_post;
u32 val_reset;
} fw_domain[FW_DOMAIN_ID_COUNT];
+
+ int unclaimed_mmio_check;
};
/* Iterate over initialised fw domains */
@@ -2732,6 +2734,7 @@ extern void intel_uncore_early_sanitize(struct drm_device *dev,
bool restore_forcewake);
extern void intel_uncore_init(struct drm_device *dev);
extern bool intel_uncore_unclaimed_mmio(struct drm_i915_private *dev_priv);
+extern void intel_uncore_arm_unclaimed_mmio_detection(struct drm_i915_private *dev_priv);
extern void intel_uncore_fini(struct drm_device *dev);
extern void intel_uncore_forcewake_reset(struct drm_device *dev, bool restore);
const char *intel_uncore_forcewake_domain_to_str(const enum forcewake_domain_id id);
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index a20dc64..5128422 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2165,11 +2165,6 @@ static irqreturn_t ironlake_irq_handler(int irq, void *arg)
if (!intel_irqs_enabled(dev_priv))
return IRQ_NONE;
- /* We get interrupts on unclaimed registers, so check for this before we
- * do any I915_{READ,WRITE}. */
- if (intel_uncore_unclaimed_mmio(dev_priv))
- DRM_ERROR("Unclaimed register before interrupt\n");
-
/* disable master interrupt before clearing iir */
de_ier = I915_READ(DEIER);
I915_WRITE(DEIER, de_ier & ~DE_MASTER_IRQ_CONTROL);
@@ -2990,6 +2985,12 @@ static void i915_hangcheck_elapsed(struct work_struct *work)
if (!i915.enable_hangcheck)
return;
+ /* As enabling the GPU requires fairly extensive mmio access,
+ * periodically arm the mmio checker to see if we are triggering
+ * any invalid access.
+ */
+ intel_uncore_arm_unclaimed_mmio_detection(dev_priv);
+
for_each_ring(ring, dev_priv, i) {
u64 acthd;
u32 seqno;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 471f120..7dcabda 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13545,6 +13545,19 @@ static int intel_atomic_commit(struct drm_device *dev,
drm_atomic_state_free(state);
+ /* As one of the primary mmio accessors, KMS has a high likelihood
+ * of triggering bugs in unclaimed access. After we finish
+ * modesetting, see if an error has been flagged, and if so
+ * enable debugging for the next modeset - and hope we catch
+ * the culprit.
+ *
+ * XXX note that we assume display power is on at this point.
+ * This might hold true now but we need to add pm helper to check
+ * unclaimed only when the hardware is on, as atomic commits
+ * can happen also when the device is completely off.
+ */
+ intel_uncore_arm_unclaimed_mmio_detection(dev_priv);
+
return 0;
}
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 3d8af69..51d3036 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -626,22 +626,6 @@ hsw_unclaimed_reg_debug(struct drm_i915_private *dev_priv,
}
}
-static void
-hsw_unclaimed_reg_detect(struct drm_i915_private *dev_priv)
-{
- static bool mmio_debug_once = true;
-
- if (i915.mmio_debug || !mmio_debug_once)
- return;
-
- 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");
- i915.mmio_debug = mmio_debug_once--;
- }
-}
-
#define GEN2_READ_HEADER(x) \
u##x val = 0; \
assert_device_not_suspended(dev_priv);
@@ -921,7 +905,6 @@ hsw_write##x(struct drm_i915_private *dev_priv, i915_reg_t reg, u##x val, bool t
gen6_gt_check_fifodbg(dev_priv); \
} \
hsw_unclaimed_reg_debug(dev_priv, reg, false, false); \
- hsw_unclaimed_reg_detect(dev_priv); \
GEN6_WRITE_FOOTER; \
}
@@ -956,7 +939,6 @@ gen8_write##x(struct drm_i915_private *dev_priv, i915_reg_t reg, u##x val, bool
__force_wake_get(dev_priv, FORCEWAKE_RENDER); \
__raw_i915_write##x(dev_priv, reg, val); \
hsw_unclaimed_reg_debug(dev_priv, reg, false, false); \
- hsw_unclaimed_reg_detect(dev_priv); \
GEN6_WRITE_FOOTER; \
}
@@ -1026,7 +1008,6 @@ gen9_write##x(struct drm_i915_private *dev_priv, i915_reg_t reg, u##x val, \
__force_wake_get(dev_priv, fw_engine); \
__raw_i915_write##x(dev_priv, reg, val); \
hsw_unclaimed_reg_debug(dev_priv, reg, false, false); \
- hsw_unclaimed_reg_detect(dev_priv); \
GEN6_WRITE_FOOTER; \
}
@@ -1246,6 +1227,8 @@ void intel_uncore_init(struct drm_device *dev)
intel_uncore_fw_domains_init(dev);
__intel_uncore_early_sanitize(dev, false);
+ dev_priv->uncore.unclaimed_mmio_check = 1;
+
switch (INTEL_INFO(dev)->gen) {
default:
case 9:
@@ -1607,3 +1590,19 @@ bool intel_uncore_unclaimed_mmio(struct drm_i915_private *dev_priv)
{
return check_for_unclaimed_mmio(dev_priv);
}
+
+void
+intel_uncore_arm_unclaimed_mmio_detection(struct drm_i915_private *dev_priv)
+{
+ if (unlikely(i915.mmio_debug ||
+ dev_priv->uncore.unclaimed_mmio_check <= 0))
+ return;
+
+ if (unlikely(intel_uncore_unclaimed_mmio(dev_priv))) {
+ DRM_DEBUG("Unclaimed register detected, "
+ "enabling oneshot unclaimed register reporting. "
+ "Please use i915.mmio_debug=N for more information.\n");
+ i915.mmio_debug++;
+ dev_priv->uncore.unclaimed_mmio_check--;
+ }
+}
--
2.5.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
