On Mon, Nov 09, 2015 at 09:13:45PM +0200, Imre Deak wrote: > Atm, we assert that the device is not suspended after the point when the > HW is truly put to a suspended state. This is fine, but we can catch > more problems if we check the RPM refcount. After that one drops to zero > we shouldn't access the HW any more, although the actual suspend may be > delayed. The only complication is that we want to avoid asserts while > the suspend handler itself is running, so add a flag to handle this > case.
Why do we want to avoid asserts firing while we go through the suspend handler? Calling assert_device_not_suspended from within rpm suspend/resume code sounds like a bug. Where/why does this happen? -Daniel > > While at it remove the HAS_RUNTIME_PM check, the pm.suspended flag is > false and the RPM refcount is non-zero on all platforms that don't > support RPM. > > This caught additional WARNs from the atomic path, those will be fixed > as a follow-up. > > v2: > - remove the redundant HAS_RUNTIME_PM check (Ville) > > Signed-off-by: Imre Deak <[email protected]> > --- > drivers/gpu/drm/i915/i915_drv.c | 5 +++++ > drivers/gpu/drm/i915/i915_drv.h | 5 +++++ > drivers/gpu/drm/i915/intel_runtime_pm.c | 14 ++++++++++++-- > 3 files changed, 22 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 77d183d..caeb218 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -1494,6 +1494,9 @@ static int intel_runtime_suspend(struct device *device) > > return -EAGAIN; > } > + > + dev_priv->pm.disable_suspended_assert = true; > + > /* > * We are safe here against re-faults, since the fault handler takes > * an RPM reference. > @@ -1518,6 +1521,8 @@ static int intel_runtime_suspend(struct device *device) > intel_uncore_forcewake_reset(dev, false); > dev_priv->pm.suspended = true; > > + dev_priv->pm.disable_suspended_assert = false; > + > /* > * FIXME: We really should find a document that references the arguments > * used below! > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 5628c5a..43fd341 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1599,6 +1599,11 @@ struct skl_wm_level { > * For more, read the Documentation/power/runtime_pm.txt. > */ > struct i915_runtime_pm { > + /* > + * Used for the duration of runtime suspend to avoid false device > + * suspended asserts. > + */ > + bool disable_suspended_assert; > bool suspended; > bool irqs_enabled; > }; > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c > b/drivers/gpu/drm/i915/intel_runtime_pm.c > index 4d39b3c..2bdbcd4 100644 > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > @@ -2120,8 +2120,18 @@ void intel_power_domains_init_hw(struct > drm_i915_private *dev_priv, bool resume) > > void assert_device_not_suspended(struct drm_i915_private *dev_priv) > { > - WARN_ONCE(HAS_RUNTIME_PM(dev_priv->dev) && dev_priv->pm.suspended, > - "Device suspended\n"); > + int rpm_usage; > + > + if (dev_priv->pm.disable_suspended_assert) > + return; > + > +#ifdef CONFIG_PM > + rpm_usage = atomic_read(&dev_priv->dev->dev->power.usage_count); > +#else > + rpm_usage = 1; > +#endif > + > + WARN_ONCE(dev_priv->pm.suspended || !rpm_usage, "Device suspended\n"); > } > > /** > -- > 2.5.0 > > _______________________________________________ > Intel-gfx mailing list > [email protected] > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
