| Message ID | 1452702790-16767-1-git-send-email-mika.kuoppala@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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"); +} + /** * 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
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(-)