Re: [PATCH v2 3/4] drm/i915: make assert_device_not_suspended more precise

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Nov 18, 2015 at 04:58:46PM +0200, Imre Deak wrote:
> On ke, 2015-11-18 at 16:44 +0200, Imre Deak wrote:
> > 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.
> 
> Ok, misunderstood your question. assert_device_not_suspended() is
> called during runtime suspend since we're accessing the HW until the
> point we set dev_priv->pm.suspended = true. Atm this wouldn't trigger a
> WARN, since assert_device_not_suspended() only checks pm.suspended and
> that will check out fine, but once we start to check HW accesses
> against the actual RPM refcount we want to disable the asserts on those
> in the handlers, since there the refcount is zero. Hence disabling it
> explicitly around the handlers, but we would still keep checking
> pm.suspended.

That seems like we're mixing up 2 asserts:
- assert_device_not_suspended: To be used in runtime_suspend code.
- assert_holding_rpm_wakelock (or whatever, I'm bad at names): check the
  count.

What am I missing?
-Daniel
-- 
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