[Intel-gfx] [PATCH 45/53] drm/i915/bdw: Do not call intel_runtime_pm_get() in an interrupt

[Alan Stern]

On Sat, 9 Aug 2014, Rafael J. Wysocki wrote:

> > > > Well it works currently. So where do you see the problem?
> > > 
> > > Sampling registers from an timer - in particular, we really do not want
> > > to disable runtime pm whilst trying to monitor the impact of runtime pm.
> > 
> > In that case you can grab a runtime pm reference iff the device is powered
> > on already. Which won't call anything scary, just amounts to an
> > atomic_add_unless or so, and then drop it again. 
> > 
> > Unfortunately there doesn't seem to be such a thing around already, so
> > need to add it first. Greg, how much would you freak out if we add
> > something like
> > 
> > /**
> >  * pm_runtime_get_unless_suspended - grab a rpm ref if the device is on
> >  * 
> >  * Returns true if an rpm ref has been acquire, false otherwise. Can be
> >  * called from atomic context to e.g. sample perfomance counters (where we
> >  * obviously don't want to disturb system state if everything is off atm).
> >  */
> > static inline bool pm_runtime_get_unless_suspended(struct device *dev)
> > {
> > 	return atomic_add_unless(&dev->power.usage_count, 1, 0);
> > }
> 
> I don't think it'll work universally.
> 
> That'd need to be synchronized with other stuff done under the spinlock
> and in fact, what you're interested in is runtime_status (and that being
> RPM_ACTIVE) and not just the usage count.

That's right.  You'd need to acquire the spinlock, test runtime_status, 
do the register sampling if the status is RPM_ACTIVE, and then drop the 
spinlock.

I suppose wrapper routines for acquiring and releasing the spinlock
could be added to the runtime-PM API.  Something like this:

#define pm_runtime_lock(dev, flags)			\
		spin_lock_irqsave(&(dev)->power.lock, flags)
#define pm_runtime_unlock(dev, flags)			\
		spin_unlock_irqrestore(&(dev)->power.lock, flags)

It looks a little silly but it would work.

Alan Stern