On ke, 2015-11-18 at 15:37 +0100, Daniel Vetter wrote:
> 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?
Yea, disable_rpm_asserts() is misnamed. Should be
disable_rpm_wakelock_asserts(). Will change that in the next iteration.
> -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 <imre.deak@xxxxxxxxx>
> > ---
> > 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
> > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
