Re: [PATCH] drm/i915: Dump power well states on unclaimed trace

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Jan 13, 2016 at 06:33:10PM +0200, Mika Kuoppala wrote:
> It is beneficial to know the exact sw states of power wells
> at the moment when unclaimed register access is detect.
> 
> When the backtrace has been printed to dmesg, it is
> followed by a power well states, for example:
> 
> <warn on call trace for unclaimed access>
> --[power wells, wakeref_count 2] --
> Name                 sw state    count
> display              off         0
> dpio-tx-b-01         off         0
> dpio-tx-b-23         off         0
> dpio-tx-c-01         off         0
> dpio-tx-c-23         off         0
> dpio-common          off         0
> --------- [power wells end] --------
> 
> This helps bug triaging as it is immediately obvious that the
> unclaimed access trace is not a fluke and not about out of bounds access.
> Rather the call chain shown by above warn on trace have failed
> to enable required power well.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_drv.h        |  1 +
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 26 ++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_uncore.c     |  4 +++-
>  3 files changed, 30 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index e27954d2edad..b83faec2d526 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1445,6 +1445,7 @@ int intel_power_domains_init(struct drm_i915_private *);
>  void intel_power_domains_fini(struct drm_i915_private *);
>  void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume);
>  void intel_power_domains_suspend(struct drm_i915_private *dev_priv);
> +void intel_power_domains_dump_wells(struct drm_i915_private *dev_priv);
>  void skl_pw1_misc_io_init(struct drm_i915_private *dev_priv);
>  void skl_pw1_misc_io_fini(struct drm_i915_private *dev_priv);
>  void intel_runtime_pm_enable(struct drm_i915_private *dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index bbca527184d0..43af603aebe6 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -2217,6 +2217,32 @@ void intel_power_domains_suspend(struct drm_i915_private *dev_priv)
>  		intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
>  }
>  
> +void intel_power_domains_dump_wells(struct drm_i915_private *dev_priv)
> +{
> +	struct i915_power_domains *power_domains;
> +	struct i915_power_well *power_well;
> +	int i;
> +
> +	power_domains = &dev_priv->power_domains;
> +
> +	/* Intentionally omitting power domain lock */
> +
> +	pr_info("--[power wells, wakeref_count %d] --\n",
> +	       atomic_read(&dev_priv->pm.wakeref_count));
> +	pr_info("%-20s %-11s %-6s\n", "Name", "sw state", "count");
> +
> +	for_each_power_well(i, power_well, POWER_DOMAIN_MASK, power_domains) {
> +		if (power_well->always_on)
> +			continue;
> +
> +		pr_info("%-20s %-11s %-6d\n",
> +			power_well->name,
> +			power_well->hw_enabled ? "on" : "off",
> +			power_well->count);
> +	}
> +	pr_info("--------- [power wells end] --------\n");

pr_info is a bit heavy, imo this should be debug level at most. Otherwise
sounds like a good idea, with that changed Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-Daniel

> +}
> +
>  /**
>   * intel_runtime_pm_get - grab a runtime pm reference
>   * @dev_priv: i915 device instance
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index c3c13dc929cb..90875009f789 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -635,8 +635,10 @@ __unclaimed_reg_debug(struct drm_i915_private *dev_priv,
>  		 "Unclaimed register detected %s %s register 0x%x\n",
>  		 before ? "before" : "after",
>  		 read ? "reading" : "writing to",
> -		 i915_mmio_reg_offset(reg)))
> +		 i915_mmio_reg_offset(reg))) {
>  		i915.mmio_debug--; /* Only report the first N failures */
> +		intel_power_domains_dump_wells(dev_priv);
> +	}
>  }
>  
>  static inline void
> -- 
> 2.5.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx