"Rafael J. Wysocki" <rjw@xxxxxxx> writes:
> 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.
> PS
> I wonder what you think about this patch:
> https://patchwork.kernel.org/patch/959672/
I responded to that thread.
Kevin
_______________________________________________
linux-pm mailing list
linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/linux-pm
