Re: PM domain using _noirq methods to "finish" pending runtime PM transistions

["Rafael J. Wysocki" <rjw@sisk.pl>]

Hi,

On Saturday, July 09, 2011, Kevin Hilman wrote:
> Hi Rafael,
> 
> So I'm now experimenting with your suggestion of using the noirq
> callbacks of the PM domain to ensure device low-power state transitions
> for the cases where runtime PM has been disabled from userspace (or a
> driver has used runtime PM calls in it's suspend/resume path but they
> have no effect since runtime PM is disabled.)
> 
> Before I get too far, I want to be sure I understood your suggestion
> correctly, and run my current implemtation past you.
> 
> For starters, I just set the driver's noirq callbacks to point to the
> runtime PM callbacks.
> 
> Then, in the PM domain's suspend_noirq, I basically do
> 
> 	ret = pm_generic_suspend_noirq(dev);
> 	if (!ret && !(dev->power.runtime_status == RPM_SUSPENDED))
> 		magic_device_power_down(dev); /* shared with pm_domain runtime PM methods */
>                 priv->flags |= MAGIC_DEVICE_SUSPENDED;
> 	}
> 	return ret;
> 
> and in the PM domain's resume_norq, I basically do:
> 
> 	if ((priv->flags & MAGIC_DEVICE_SUSPENDED) &&
> 	    !(dev->power.runtime_status == RPM_SUSPENDED)) {
> 		priv->flags &= ~OMAP_DEVICE_SUSPENDED;
> 		magic_device_power_up(dev);
> 	}
> 	return pm_generic_resume_noirq(dev);
> 
> Note: The MAGIC_DEVICE_SUSPENDED flag is used so resume_noirq only
> reenables devices that were disabled by suspend_noirq so that devices
> which were disabled by runtime PM are left untouched (similar to how
> you've implmented generic PM domains.)
> 
> Since the driver's noirq callbacks point to the runtime PM callbacks
> this all works quite well. 
> 
> I believe this was basically the suggestion you were recommending, right?

That's correct.

> However, what I need is for the driver's runtime PM callbacks not to be
> called unconditionally, but only if the PM domain's noirq methods are
> actually going to disable/power-down the device (e.g. it's not already
> runtime suspended.)
> 
> The first obvious solution is to create new driver noirq methods that
> check if the device is not already RPM_SUSPENDED and only then call the
> runtime PM callbacks.   That works, but hey, it's a Friday night so I
> decided to take it to the next level...
> 
> Instead of making all the drivers add new noirq methods that just
> conditionally call the runtime PM methods, what if I just handle it in
> the PM domain?  IOW, what if I just call pm_generic_runtime_* from the
> PM domain's noirq methods?  e.g. call pm_generic_runtime_suspend() just
> before magic_device_power_down() and call pm_generic_runtime_resume()
> just after magic_device_power_up()?

That should be fine.

> I tested this today with a handful of our drivers that do all their PM
> in terms of the runtime PM API (including get_sync/put_sync in their
> suspend path) and they all work as expected without modification.
> 
> I know it's not much to add a couple new callbacks to each of these
> drivers, but if I can handle it at the PM domain level, I'd rather allow
> the drivers to stay simple.
> 
> What do you think?

I don't see anything wrong with your approach. :-)

Thanks,
Rafael


PS
I wonder what you think about this patch:
https://patchwork.kernel.org/patch/959672/