Multiple uncore structures will share the debug infrastructure, so
move it to a common place and add extra locking around it.
Also, since we now have a separate object, it is cleaner to have
dedicated functions working on the object to stop and restart the
mmio debug. Apart from the cosmetic changes, this patch introduces
2 functional updates:
- All calls to check_for_unclaimed_mmio will now return false when
the debug is suspended, not just the ones that are active only when
i915_modparams.mmio_debug is set. If we don't trust the result of the
check while a user is doing mmio access then we shouldn't attempt the
check anywhere.
- i915_modparams.mmio_debug is not save/restored anymore around user
access. The value is now never touched by the kernel while debug is
disabled so no need for save/restore.
v2: squash mmio_debug patches, restrict mmio_debug lock usage (Chris)
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
---
drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
drivers/gpu/drm/i915/i915_drv.c | 3 +-
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/intel_uncore.c | 91 ++++++++++++++++++++---------
drivers/gpu/drm/i915/intel_uncore.h | 18 +++---
5 files changed, 79 insertions(+), 36 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 3b15266c54fd..2310512111ab 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1129,7 +1129,7 @@ static int i915_forcewake_domains(struct seq_file *m, void *data)
unsigned int tmp;
seq_printf(m, "user.bypass_count = %u\n",
- uncore->user_forcewake.count);
+ uncore->user_forcewake_count);
for_each_fw_domain(fw_domain, uncore, tmp)
seq_printf(m, "%s.wake_count = %u\n",
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 3480db36b63f..fbbff4a133ba 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -744,6 +744,7 @@ static int i915_driver_early_probe(struct drm_i915_private *dev_priv)
intel_device_info_subplatform_init(dev_priv);
+ intel_uncore_mmio_debug_init_early(&dev_priv->mmio_debug);
intel_uncore_init_early(&dev_priv->uncore, dev_priv);
spin_lock_init(&dev_priv->irq_lock);
@@ -2044,7 +2045,7 @@ static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation)
out:
enable_rpm_wakeref_asserts(rpm);
- if (!dev_priv->uncore.user_forcewake.count)
+ if (!dev_priv->uncore.user_forcewake_count)
intel_runtime_pm_driver_release(rpm);
return ret;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c9476f24f5c1..13c27a75dca8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1346,6 +1346,7 @@ struct drm_i915_private {
resource_size_t stolen_usable_size; /* Total size minus reserved ranges */
struct intel_uncore uncore;
+ struct intel_uncore_mmio_debug mmio_debug;
struct i915_virtual_gpu vgpu;
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 39e8710afdd9..9e583f13a9e4 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -34,6 +34,32 @@
#define __raw_posting_read(...) ((void)__raw_uncore_read32(__VA_ARGS__))
+void
+intel_uncore_mmio_debug_init_early(struct intel_uncore_mmio_debug *mmio_debug)
+{
+ spin_lock_init(&mmio_debug->lock);
+ mmio_debug->unclaimed_mmio_check = 1;
+}
+
+static void mmio_debug_suspend(struct intel_uncore_mmio_debug *mmio_debug)
+{
+ lockdep_assert_held(&mmio_debug->lock);
+
+ /* Save and disable mmio debugging for the user bypass */
+ if (!mmio_debug->suspend_count++) {
+ mmio_debug->saved_mmio_check = mmio_debug->unclaimed_mmio_check;
+ mmio_debug->unclaimed_mmio_check = 0;
+ }
+}
+
+static void mmio_debug_resume(struct intel_uncore_mmio_debug *mmio_debug)
+{
+ lockdep_assert_held(&mmio_debug->lock);
+
+ if (!--mmio_debug->suspend_count)
+ mmio_debug->unclaimed_mmio_check = mmio_debug->saved_mmio_check;
+}
+
static const char * const forcewake_domain_names[] = {
"render",
"blitter",
@@ -476,6 +502,11 @@ check_for_unclaimed_mmio(struct intel_uncore *uncore)
{
bool ret = false;
+ lockdep_assert_held(&uncore->debug->lock);
+
+ if (uncore->debug->suspend_count)
+ return false;
+
if (intel_uncore_has_fpga_dbg_unclaimed(uncore))
ret |= fpga_check_for_unclaimed_mmio(uncore);
@@ -608,17 +639,11 @@ void intel_uncore_forcewake_get(struct intel_uncore *uncore,
void intel_uncore_forcewake_user_get(struct intel_uncore *uncore)
{
spin_lock_irq(&uncore->lock);
- if (!uncore->user_forcewake.count++) {
+ if (!uncore->user_forcewake_count++) {
intel_uncore_forcewake_get__locked(uncore, FORCEWAKE_ALL);
-
- /* Save and disable mmio debugging for the user bypass */
- uncore->user_forcewake.saved_mmio_check =
- uncore->unclaimed_mmio_check;
- uncore->user_forcewake.saved_mmio_debug =
- i915_modparams.mmio_debug;
-
- uncore->unclaimed_mmio_check = 0;
- i915_modparams.mmio_debug = 0;
+ spin_lock(&uncore->debug->lock);
+ mmio_debug_suspend(uncore->debug);
+ spin_unlock(&uncore->debug->lock);
}
spin_unlock_irq(&uncore->lock);
}
@@ -633,15 +658,14 @@ void intel_uncore_forcewake_user_get(struct intel_uncore *uncore)
void intel_uncore_forcewake_user_put(struct intel_uncore *uncore)
{
spin_lock_irq(&uncore->lock);
- if (!--uncore->user_forcewake.count) {
- if (intel_uncore_unclaimed_mmio(uncore))
+ if (!--uncore->user_forcewake_count) {
+ spin_lock(&uncore->debug->lock);
+ mmio_debug_resume(uncore->debug);
+
+ if (check_for_unclaimed_mmio(uncore))
dev_info(uncore->i915->drm.dev,
"Invalid mmio detected during user access\n");
-
- uncore->unclaimed_mmio_check =
- uncore->user_forcewake.saved_mmio_check;
- i915_modparams.mmio_debug =
- uncore->user_forcewake.saved_mmio_debug;
+ spin_unlock(&uncore->debug->lock);
intel_uncore_forcewake_put__locked(uncore, FORCEWAKE_ALL);
}
@@ -1088,7 +1112,16 @@ unclaimed_reg_debug(struct intel_uncore *uncore,
if (likely(!i915_modparams.mmio_debug))
return;
+ /* interrupts are disabled and re-enabled around uncore->lock usage */
+ lockdep_assert_held(&uncore->lock);
+
+ if (before)
+ spin_lock(&uncore->debug->lock);
+
__unclaimed_reg_debug(uncore, reg, read, before);
+
+ if (!before)
+ spin_unlock(&uncore->debug->lock);
}
#define GEN2_READ_HEADER(x) \
@@ -1607,6 +1640,7 @@ void intel_uncore_init_early(struct intel_uncore *uncore,
spin_lock_init(&uncore->lock);
uncore->i915 = i915;
uncore->rpm = &i915->runtime_pm;
+ uncore->debug = &i915->mmio_debug;
}
static void uncore_raw_init(struct intel_uncore *uncore)
@@ -1632,7 +1666,6 @@ static int uncore_forcewake_init(struct intel_uncore *uncore)
ret = intel_uncore_fw_domains_init(uncore);
if (ret)
return ret;
-
forcewake_early_sanitize(uncore, 0);
if (IS_GEN_RANGE(i915, 6, 7)) {
@@ -1681,8 +1714,6 @@ int intel_uncore_init_mmio(struct intel_uncore *uncore)
if (INTEL_GEN(i915) > 5 && !intel_vgpu_active(i915))
uncore->flags |= UNCORE_HAS_FORCEWAKE;
- uncore->unclaimed_mmio_check = 1;
-
if (!intel_uncore_has_forcewake(uncore)) {
uncore_raw_init(uncore);
} else {
@@ -1707,7 +1738,7 @@ int intel_uncore_init_mmio(struct intel_uncore *uncore)
uncore->flags |= UNCORE_HAS_FIFO;
/* clear out unclaimed reg detection bit */
- if (check_for_unclaimed_mmio(uncore))
+ if (intel_uncore_unclaimed_mmio(uncore))
DRM_DEBUG("unclaimed mmio detected on uncore init, clearing\n");
return 0;
@@ -1952,7 +1983,13 @@ int __intel_wait_for_register(struct intel_uncore *uncore,
bool intel_uncore_unclaimed_mmio(struct intel_uncore *uncore)
{
- return check_for_unclaimed_mmio(uncore);
+ bool ret;
+
+ spin_lock_irq(&uncore->debug->lock);
+ ret = check_for_unclaimed_mmio(uncore);
+ spin_unlock_irq(&uncore->debug->lock);
+
+ return ret;
}
bool
@@ -1960,24 +1997,24 @@ intel_uncore_arm_unclaimed_mmio_detection(struct intel_uncore *uncore)
{
bool ret = false;
- spin_lock_irq(&uncore->lock);
+ spin_lock_irq(&uncore->debug->lock);
- if (unlikely(uncore->unclaimed_mmio_check <= 0))
+ if (unlikely(uncore->debug->unclaimed_mmio_check <= 0))
goto out;
- if (unlikely(intel_uncore_unclaimed_mmio(uncore))) {
+ if (unlikely(check_for_unclaimed_mmio(uncore))) {
if (!i915_modparams.mmio_debug) {
DRM_DEBUG("Unclaimed register detected, "
"enabling oneshot unclaimed register reporting. "
"Please use i915.mmio_debug=N for more information.\n");
i915_modparams.mmio_debug++;
}
- uncore->unclaimed_mmio_check--;
+ uncore->debug->unclaimed_mmio_check--;
ret = true;
}
out:
- spin_unlock_irq(&uncore->lock);
+ spin_unlock_irq(&uncore->debug->lock);
return ret;
}
diff --git a/drivers/gpu/drm/i915/intel_uncore.h b/drivers/gpu/drm/i915/intel_uncore.h
index e603d210a34d..414fc2cb0459 100644
--- a/drivers/gpu/drm/i915/intel_uncore.h
+++ b/drivers/gpu/drm/i915/intel_uncore.h
@@ -36,6 +36,13 @@ struct drm_i915_private;
struct intel_runtime_pm;
struct intel_uncore;
+struct intel_uncore_mmio_debug {
+ spinlock_t lock; /** lock is also taken in irq contexts. */
+ int unclaimed_mmio_check;
+ int saved_mmio_check;
+ u32 suspend_count;
+};
+
enum forcewake_domain_id {
FW_DOMAIN_ID_RENDER = 0,
FW_DOMAIN_ID_BLITTER,
@@ -137,14 +144,9 @@ struct intel_uncore {
u32 __iomem *reg_ack;
} *fw_domain[FW_DOMAIN_ID_COUNT];
- struct {
- unsigned int count;
-
- int saved_mmio_check;
- int saved_mmio_debug;
- } user_forcewake;
+ unsigned int user_forcewake_count;
- int unclaimed_mmio_check;
+ struct intel_uncore_mmio_debug *debug;
};
/* Iterate over initialised fw domains */
@@ -179,6 +181,8 @@ intel_uncore_has_fifo(const struct intel_uncore *uncore)
return uncore->flags & UNCORE_HAS_FIFO;
}
+void
+intel_uncore_mmio_debug_init_early(struct intel_uncore_mmio_debug *mmio_debug);
void intel_uncore_init_early(struct intel_uncore *uncore,
struct drm_i915_private *i915);
int intel_uncore_init_mmio(struct intel_uncore *uncore);
--
2.22.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
