[Intel-gfx] [RFC 5/8] drm/i915: split out uncore_mmio_debug
Daniele Ceraolo Spurio
daniele.ceraolospurio at intel.com
Thu Jun 6 21:58:22 UTC 2019
On 6/6/19 2:52 PM, Daniele Ceraolo Spurio wrote:
> Multiple uncore structures will share the debug infrastructure, so
> move it to a common place and add extra locking around it.
>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.c | 1 +
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> drivers/gpu/drm/i915/intel_uncore.c | 66 ++++++++++++++++++++---------
> drivers/gpu/drm/i915/intel_uncore.h | 10 ++++-
> 4 files changed, 58 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 9094736af5da..8fdd668eb7c7 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -897,6 +897,7 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv)
>
> spin_lock_init(&dev_priv->irq_lock);
> spin_lock_init(&dev_priv->gpu_error.lock);
> + spin_lock_init(&dev_priv->mmio_debug.lock);
This should be intel_uncore_mmio_debug_init_early() and done before
intel_uncore_init_early, I forgot to squash the fix in.
Daniele
> mutex_init(&dev_priv->backlight_lock);
>
> mutex_init(&dev_priv->sb_lock);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index a0539b837df5..5522132a2ad2 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1398,6 +1398,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 7ebea00207a6..8e42476ea4a7 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -34,6 +34,13 @@
>
> #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 const char * const forcewake_domain_names[] = {
> "render",
> "blitter",
> @@ -473,6 +480,8 @@ check_for_unclaimed_mmio(struct intel_uncore *uncore)
> {
> bool ret = false;
>
> + lockdep_assert_held(&uncore->debug->lock);
> +
> if (intel_uncore_has_fpga_dbg_unclaimed(uncore))
> ret |= fpga_check_for_unclaimed_mmio(uncore);
>
> @@ -489,7 +498,7 @@ static void __intel_uncore_early_sanitize(struct intel_uncore *uncore,
> unsigned int restore_forcewake)
> {
> /* 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");
>
> /* WaDisableShadowRegForCpd:chv */
> @@ -595,18 +604,20 @@ void intel_uncore_forcewake_get(struct intel_uncore *uncore,
> void intel_uncore_forcewake_user_get(struct intel_uncore *uncore)
> {
> spin_lock_irq(&uncore->lock);
> + spin_lock_irq(&uncore->debug->lock);
> 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->debug->unclaimed_mmio_check;
> uncore->user_forcewake.saved_mmio_debug =
> i915_modparams.mmio_debug;
>
> - uncore->unclaimed_mmio_check = 0;
> + uncore->debug->unclaimed_mmio_check = 0;
> i915_modparams.mmio_debug = 0;
> }
> + spin_unlock_irq(&uncore->debug->lock);
> spin_unlock_irq(&uncore->lock);
> }
>
> @@ -620,18 +631,20 @@ 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);
> + spin_lock_irq(&uncore->debug->lock);
> if (!--uncore->user_forcewake.count) {
> - if (intel_uncore_unclaimed_mmio(uncore))
> + if (check_for_unclaimed_mmio(uncore))
> dev_info(uncore_to_i915(uncore)->drm.dev,
> "Invalid mmio detected during user access\n");
>
> - uncore->unclaimed_mmio_check =
> + uncore->debug->unclaimed_mmio_check =
> uncore->user_forcewake.saved_mmio_check;
> i915_modparams.mmio_debug =
> uncore->user_forcewake.saved_mmio_debug;
>
> intel_uncore_forcewake_put__locked(uncore, FORCEWAKE_ALL);
> }
> + spin_unlock_irq(&uncore->debug->lock);
> spin_unlock_irq(&uncore->lock);
> }
>
> @@ -1044,12 +1057,19 @@ static inline void
> unclaimed_reg_debug(struct intel_uncore *uncore,
> const i915_reg_t reg,
> const bool read,
> - const bool before)
> + const bool before,
> + unsigned long *irqflags)
> {
> if (likely(!i915_modparams.mmio_debug))
> return;
>
> + if (before)
> + spin_lock_irqsave(&uncore->debug->lock, *irqflags);
> +
> __unclaimed_reg_debug(uncore, reg, read, before);
> +
> + if (!before)
> + spin_unlock_irqrestore(&uncore->debug->lock, *irqflags);
> }
>
> #define GEN2_READ_HEADER(x) \
> @@ -1095,13 +1115,14 @@ __gen2_read(64)
> #define GEN6_READ_HEADER(x) \
> u32 offset = i915_mmio_reg_offset(reg); \
> unsigned long irqflags; \
> + unsigned long debug_irqflags; \
> u##x val = 0; \
> __assert_rpm_wakelock_held(uncore->rpm); \
> spin_lock_irqsave(&uncore->lock, irqflags); \
> - unclaimed_reg_debug(uncore, reg, true, true)
> + unclaimed_reg_debug(uncore, reg, true, true, &debug_irqflags)
>
> #define GEN6_READ_FOOTER \
> - unclaimed_reg_debug(uncore, reg, true, false); \
> + unclaimed_reg_debug(uncore, reg, true, false, &debug_irqflags); \
> spin_unlock_irqrestore(&uncore->lock, irqflags); \
> trace_i915_reg_rw(false, reg, val, sizeof(val), trace); \
> return val
> @@ -1204,13 +1225,14 @@ __gen2_write(32)
> #define GEN6_WRITE_HEADER \
> u32 offset = i915_mmio_reg_offset(reg); \
> unsigned long irqflags; \
> + unsigned long debug_irqflags; \
> trace_i915_reg_rw(true, reg, val, sizeof(val), trace); \
> __assert_rpm_wakelock_held(uncore->rpm); \
> spin_lock_irqsave(&uncore->lock, irqflags); \
> - unclaimed_reg_debug(uncore, reg, false, true)
> + unclaimed_reg_debug(uncore, reg, false, true, &debug_irqflags)
>
> #define GEN6_WRITE_FOOTER \
> - unclaimed_reg_debug(uncore, reg, false, false); \
> + unclaimed_reg_debug(uncore, reg, false, false, &debug_irqflags); \
> spin_unlock_irqrestore(&uncore->lock, irqflags)
>
> #define __gen6_write(x) \
> @@ -1574,6 +1596,9 @@ int intel_uncore_init_mmio(struct intel_uncore *uncore)
> struct drm_i915_private *i915 = uncore_to_i915(uncore);
> int ret;
>
> + uncore->rpm = &i915->runtime_pm;
> + uncore->debug = &i915->mmio_debug;
> +
> ret = uncore_mmio_setup(uncore);
> if (ret)
> return ret;
> @@ -1591,12 +1616,9 @@ int intel_uncore_init_mmio(struct intel_uncore *uncore)
>
> __intel_uncore_early_sanitize(uncore, 0);
>
> - uncore->unclaimed_mmio_check = 1;
> uncore->pmic_bus_access_nb.notifier_call =
> i915_pmic_bus_access_notifier;
>
> - uncore->rpm = &i915->runtime_pm;
> -
> if (!intel_uncore_has_forcewake(uncore)) {
> if (IS_GEN(i915, 5)) {
> ASSIGN_WRITE_MMIO_VFUNCS_NO_FW(uncore, gen5);
> @@ -1858,7 +1880,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
> @@ -1866,24 +1894,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 b2de47da053f..53c1a334e2ae 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.h
> +++ b/drivers/gpu/drm/i915/intel_uncore.h
> @@ -36,6 +36,11 @@ struct drm_i915_private;
> struct i915_runtime_pm;
> struct intel_uncore;
>
> +struct intel_uncore_mmio_debug {
> + spinlock_t lock; /** lock is also taken in irq contexts. */
> + int unclaimed_mmio_check;
> +};
> +
> enum forcewake_domain_id {
> FW_DOMAIN_ID_RENDER = 0,
> FW_DOMAIN_ID_BLITTER,
> @@ -142,7 +147,7 @@ struct intel_uncore {
> int saved_mmio_debug;
> } user_forcewake;
>
> - int unclaimed_mmio_check;
> + struct intel_uncore_mmio_debug *debug;
> };
>
> /* Iterate over initialised fw domains */
> @@ -177,6 +182,9 @@ 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);
> int intel_uncore_init_mmio(struct intel_uncore *uncore);
> void intel_uncore_fw_domain_prune(struct intel_uncore *uncore,
>
More information about the Intel-gfx
mailing list