On Tue, Dec 16, 2014 at 11:10 PM, Kevin Hilman <[email protected]> wrote:
> At a deeper level, the problem with this approach is that this is more
> generically a runtime PM dependency problem, not a genpd problem. For
> example, what happens when the same kind of dependency exists on a
> platform using a custom PM domain instead of genpd (like ACPI.) ?
>
> What's needed to solve this problem is a generalized way to have runtime
> PM dependencies between devices. Runtime PM already automatically
> handles parent devices as one type of dependent device (e.g. a parent
> device needs to be runtime PM resumed before its child.) So what's
> needed is a generic way to other PM dependencies with the runtime PM
> core (not the genpd core.)
>
> If runtime PM handles the dependencies corretcly, then genpd (and any
> other PM domain) will get them "for free".
Having the proper dependencies is not sufficient. Currently drivers have to do
something to use runtime PM.
By default, runtime PM is disabled for a device
("device.power.disable_depth = 1").
However, if PM domains are active, drivers must be runtime PM-aware for the
gpd_dev_ops.start() method to be called in the first place (perhaps this is just
one bug that's easy to fix --- the device is "assumed suspended", but can be
used). They must
1. call pm_runtime_enable() to enable runtime PM for the device,
2. call pm_runtime_get_sync() to prevent the device from being put in a
low-power state at any time. This second call has the
"side-effect" of calling
gpd_dev_ops.start().
Hence, if PM domains are enabled, wouldn't it make sense to
1. enable runtime PM by default, for all devices (bound and unbound),
2. call pm_runtime_get_sync(), for all devices bound to a driver.
Of course we have to keep track if drivers call any of the pm_runtime_*()
methods theirselves, as that would have to move them from automatic to
manual mode.
Would this be feasible?
This would avoid the need to add minimal runtime PM support to each and every
driver, because:
- its device resides in a PM domain, cfr. e.g. commit df0c6c80232f2ad4
("gpio: rcar: Add minimal runtime PM support"),
- its device is a child of a device in a PM domain, cfr. e.g. commit
3a611e26e958b037 ("net/smsc911x: Add minimal runtime PM support").
There's a similar issue for "transparent" buses that don't need a kernel driver
(compatible = "simple-bus"). If they're in a PM domain, runtime PM must be
enabled for them, so their bus devices are resumed when a child device on the
bus is resumed by runtime PM. Having runtime PM enabled by default
would avoid the need to add a minimal driver for the bus which just
enables runtime PM, cfr. [PATCH v2 0/8] Add Simple Power-Managed Bus Support
(https://lkml.org/lkml/2014/12/17/359).
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
On Wednesday, December 17, 2014 08:33:13 PM Geert Uytterhoeven wrote:
> On Tue, Dec 16, 2014 at 11:10 PM, Kevin Hilman <[email protected]> wrote:
> > At a deeper level, the problem with this approach is that this is more
> > generically a runtime PM dependency problem, not a genpd problem. For
> > example, what happens when the same kind of dependency exists on a
> > platform using a custom PM domain instead of genpd (like ACPI.) ?
> >
> > What's needed to solve this problem is a generalized way to have runtime
> > PM dependencies between devices. Runtime PM already automatically
> > handles parent devices as one type of dependent device (e.g. a parent
> > device needs to be runtime PM resumed before its child.) So what's
> > needed is a generic way to other PM dependencies with the runtime PM
> > core (not the genpd core.)
> >
> > If runtime PM handles the dependencies corretcly, then genpd (and any
> > other PM domain) will get them "for free".
>
> Having the proper dependencies is not sufficient. Currently drivers have to do
> something to use runtime PM.
>
> By default, runtime PM is disabled for a device
> ("device.power.disable_depth = 1").
Which isn't the case for PCI devices.
> However, if PM domains are active, drivers must be runtime PM-aware for the
> gpd_dev_ops.start() method to be called in the first place (perhaps this is just
> one bug that's easy to fix --- the device is "assumed suspended", but can be
> used). They must
> 1. call pm_runtime_enable() to enable runtime PM for the device,
> 2. call pm_runtime_get_sync() to prevent the device from being put in a
> low-power state at any time. This second call has the
> "side-effect" of calling
> gpd_dev_ops.start().
>
> Hence, if PM domains are enabled, wouldn't it make sense to
> 1. enable runtime PM by default, for all devices (bound and unbound),
I guess you mean for devices with and without drivers here?
> 2. call pm_runtime_get_sync(), for all devices bound to a driver.
> Of course we have to keep track if drivers call any of the pm_runtime_*()
> methods theirselves, as that would have to move them from automatic to
> manual mode.
>
> Would this be feasible?
PCI does something similar and IMO it would make sense to do that for all
devices, at least where we have a known way to power them up/down without a
driver.
There's a couple of questions, though. First, how many drivers would break
if we enabled runtime PM by default? Second, if we do that, how do we
figure out the initial value of runtime PM status in general? Finally,
what about drivers that need to work with and without PM domains (for example,
some systems they run on have PM domains, while some other don't)?
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
Hi Rafael,
Thanks for your comments!
On Thu, Dec 18, 2014 at 1:57 AM, Rafael J. Wysocki <[email protected]> wrote:
> On Wednesday, December 17, 2014 08:33:13 PM Geert Uytterhoeven wrote:
>> On Tue, Dec 16, 2014 at 11:10 PM, Kevin Hilman <[email protected]> wrote:
>> > At a deeper level, the problem with this approach is that this is more
>> > generically a runtime PM dependency problem, not a genpd problem. For
>> > example, what happens when the same kind of dependency exists on a
>> > platform using a custom PM domain instead of genpd (like ACPI.) ?
>> >
>> > What's needed to solve this problem is a generalized way to have runtime
>> > PM dependencies between devices. Runtime PM already automatically
>> > handles parent devices as one type of dependent device (e.g. a parent
>> > device needs to be runtime PM resumed before its child.) So what's
>> > needed is a generic way to other PM dependencies with the runtime PM
>> > core (not the genpd core.)
>> >
>> > If runtime PM handles the dependencies corretcly, then genpd (and any
>> > other PM domain) will get them "for free".
>>
>> Having the proper dependencies is not sufficient. Currently drivers have to do
>> something to use runtime PM.
>>
>> By default, runtime PM is disabled for a device
>> ("device.power.disable_depth = 1").
>
> Which isn't the case for PCI devices.
I didn't know that.
The above code excerpt comes from pm_runtime_init(), which is called from
device_pm_init() / device_initialize() / device_register(), so I
assume it applies
to PCI, too? Can you please tell me where it's overridden by PCI?
>> However, if PM domains are active, drivers must be runtime PM-aware for the
>> gpd_dev_ops.start() method to be called in the first place (perhaps this is just
>> one bug that's easy to fix --- the device is "assumed suspended", but can be
>> used). They must
>> 1. call pm_runtime_enable() to enable runtime PM for the device,
>> 2. call pm_runtime_get_sync() to prevent the device from being put in a
>> low-power state at any time. This second call has the
>> "side-effect" of calling
>> gpd_dev_ops.start().
>>
>> Hence, if PM domains are enabled, wouldn't it make sense to
>> 1. enable runtime PM by default, for all devices (bound and unbound),
>
> I guess you mean for devices with and without drivers here?
Yes, indeed.
>> 2. call pm_runtime_get_sync(), for all devices bound to a driver.
>> Of course we have to keep track if drivers call any of the pm_runtime_*()
>> methods theirselves, as that would have to move them from automatic to
>> manual mode.
>>
>> Would this be feasible?
>
> PCI does something similar and IMO it would make sense to do that for all
> devices, at least where we have a known way to power them up/down without a
> driver.
OK.
> There's a couple of questions, though
> First, how many drivers would break if we enabled runtime PM by default?
If the default also calls pm_runtime_get_sync() automatically, I don't see
a big issue there, as the device won't be runtime-suspended automatically,
just like before?
> Second, if we do that, how do we figure out the initial value of runtime
> PM status in general?
Do you mean in the context of the following paragraph from
Documentation/power/runtime_pm.txt?
"In addition to that, the initial runtime PM status of all devices is
'suspended', but it need not reflect the actual physical state of the device."
> Finally, what about drivers that need to work with and without PM domains
> (for example, some systems they run on have PM domains, while some other don't)?
We already have the last issue now, but currently it breaks in subtle ways
on the systems where there are PM domains.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Geert Uytterhoeven <[email protected]> writes:
> On Tue, Dec 16, 2014 at 11:10 PM, Kevin Hilman <[email protected]> wrote:
>> At a deeper level, the problem with this approach is that this is more
>> generically a runtime PM dependency problem, not a genpd problem. For
>> example, what happens when the same kind of dependency exists on a
>> platform using a custom PM domain instead of genpd (like ACPI.) ?
>>
>> What's needed to solve this problem is a generalized way to have runtime
>> PM dependencies between devices. Runtime PM already automatically
>> handles parent devices as one type of dependent device (e.g. a parent
>> device needs to be runtime PM resumed before its child.) So what's
>> needed is a generic way to other PM dependencies with the runtime PM
>> core (not the genpd core.)
>>
>> If runtime PM handles the dependencies corretcly, then genpd (and any
>> other PM domain) will get them "for free".
>
> Having the proper dependencies is not sufficient. Currently drivers have to do
> something to use runtime PM.
Well, yes. I've been assuming runtime-PM aware drivers. But I agree
with the goal of not having to assume that.
> By default, runtime PM is disabled for a device
> ("device.power.disable_depth = 1").
>
> However, if PM domains are active, drivers must be runtime PM-aware for the
> gpd_dev_ops.start() method to be called in the first place (perhaps this is just
> one bug that's easy to fix --- the device is "assumed suspended", but can be
> used). They must
> 1. call pm_runtime_enable() to enable runtime PM for the device,
> 2. call pm_runtime_get_sync() to prevent the device from being put in a
> low-power state at any time. This second call has the
> "side-effect" of calling
> gpd_dev_ops.start().
>
> Hence, if PM domains are enabled, wouldn't it make sense to
> 1. enable runtime PM by default, for all devices (bound and unbound),
> 2. call pm_runtime_get_sync(), for all devices bound to a driver.
> Of course we have to keep track if drivers call any of the pm_runtime_*()
> methods theirselves, as that would have to move them from automatic to
> manual mode.
>
> Would this be feasible?
We have to be careful about where the PM core's _get_sync() call goes.
Because you're talking about "bound" devices, I guess you mean after the
driver probes? Otherwise, it gets tricky if the _get_sync() is before
the driver probes, because the device driver may have work it wants to
do in its runtime PM callbacks, which are not initialized/available
before the driver probes. Doing this before probe also makes it rather
difficult to know for sure the actual physical state of the device, and
make sure it matches the runtime PM state of the device. Rafael
mentioned this also, and I'm not sure how we can be sure of the physical
state.
Some thoughts: devices without drivers would be runtime resumed by the
core, but will never be suspended, so the PM domain will never shut
down. I guess the core will have to keep track of the devices it
automatically runtime resumed and decide to runtime suspend them too?
Hmm, where would that go?
Kevin
Hi Kevin,
Thanks for your comments!
On Thu, Dec 18, 2014 at 10:29 PM, Kevin Hilman <[email protected]> wrote:
>> However, if PM domains are active, drivers must be runtime PM-aware for the
>> gpd_dev_ops.start() method to be called in the first place (perhaps this is just
>> one bug that's easy to fix --- the device is "assumed suspended", but can be
>> used). They must
>> 1. call pm_runtime_enable() to enable runtime PM for the device,
>> 2. call pm_runtime_get_sync() to prevent the device from being put in a
>> low-power state at any time. This second call has the
>> "side-effect" of calling
>> gpd_dev_ops.start().
>>
>> Hence, if PM domains are enabled, wouldn't it make sense to
>> 1. enable runtime PM by default, for all devices (bound and unbound),
>> 2. call pm_runtime_get_sync(), for all devices bound to a driver.
>> Of course we have to keep track if drivers call any of the pm_runtime_*()
>> methods theirselves, as that would have to move them from automatic to
>> manual mode.
>>
>> Would this be feasible?
>
> We have to be careful about where the PM core's _get_sync() call goes.
>
> Because you're talking about "bound" devices, I guess you mean after the
> driver probes? Otherwise, it gets tricky if the _get_sync() is before
> the driver probes, because the device driver may have work it wants to
> do in its runtime PM callbacks, which are not initialized/available
> before the driver probes. Doing this before probe also makes it rather
> difficult to know for sure the actual physical state of the device, and
> make sure it matches the runtime PM state of the device. Rafael
> mentioned this also, and I'm not sure how we can be sure of the physical
> state.
Yes, it's complicated by the fact that there are multiple sets of callbacks
(PM domain, device type, class type, bus type, driver).
However, the PM domain one has the highest priority, and is always
(also for devices not bound to a driver) available.
> Some thoughts: devices without drivers would be runtime resumed by the
> core, but will never be suspended, so the PM domain will never shut
> down. I guess the core will have to keep track of the devices it
> automatically runtime resumed and decide to runtime suspend them too?
> Hmm, where would that go?
No, devices without a driver just need to become runtime PM enabled.
They will only be resumed when a dependent device (e.g. a child) is resumed,
and are suspended again after all dependents are suspended. That's how
simple-pm-bus behaves.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Geert Uytterhoeven <[email protected]> writes:
[...]
> On Thu, Dec 18, 2014 at 10:29 PM, Kevin Hilman <[email protected]> wrote:
>>> However, if PM domains are active, drivers must be runtime PM-aware for the
>>> gpd_dev_ops.start() method to be called in the first place (perhaps this is just
>>> one bug that's easy to fix --- the device is "assumed suspended", but can be
>>> used). They must
>>> 1. call pm_runtime_enable() to enable runtime PM for the device,
>>> 2. call pm_runtime_get_sync() to prevent the device from being put in a
>>> low-power state at any time. This second call has the
>>> "side-effect" of calling
>>> gpd_dev_ops.start().
>>>
>>> Hence, if PM domains are enabled, wouldn't it make sense to
>>> 1. enable runtime PM by default, for all devices (bound and unbound),
>>> 2. call pm_runtime_get_sync(), for all devices bound to a driver.
>>> Of course we have to keep track if drivers call any of the pm_runtime_*()
>>> methods theirselves, as that would have to move them from automatic to
>>> manual mode.
>>>
>>> Would this be feasible?
>>
>> We have to be careful about where the PM core's _get_sync() call goes.
>>
>> Because you're talking about "bound" devices, I guess you mean after the
>> driver probes? Otherwise, it gets tricky if the _get_sync() is before
>> the driver probes, because the device driver may have work it wants to
>> do in its runtime PM callbacks, which are not initialized/available
>> before the driver probes. Doing this before probe also makes it rather
>> difficult to know for sure the actual physical state of the device, and
>> make sure it matches the runtime PM state of the device. Rafael
>> mentioned this also, and I'm not sure how we can be sure of the physical
>> state.
>
> Yes, it's complicated by the fact that there are multiple sets of callbacks
> (PM domain, device type, class type, bus type, driver).
> However, the PM domain one has the highest priority, and is always
> (also for devices not bound to a driver) available.
Yes, but if a _get_sync() is called on a device which has not yet setup
its callbacks (e.g. before it has been probed), then the device may not
be properly initialized, and we may not be able to know its physical state.
>> Some thoughts: devices without drivers would be runtime resumed by the
>> core, but will never be suspended, so the PM domain will never shut
>> down. I guess the core will have to keep track of the devices it
>> automatically runtime resumed and decide to runtime suspend them too?
>> Hmm, where would that go?
>
> No, devices without a driver just need to become runtime PM enabled.
> They will only be resumed when a dependent device (e.g. a child) is resumed,
> and are suspended again after all dependents are suspended. That's how
> simple-pm-bus behaves.
Ah, OK. I thought you were proposing to _enable() and _get_sync() those
devices.
Thanks for the clarification,
Kevin