Re: [RFC][PATCH 1/3] PM / sleep: Flag to speed up suspend-resume of runtime-suspended devices

2014-05-12 Thread Rafael J. Wysocki
On Sunday, May 11, 2014 12:46:10 PM Alan Stern wrote:
> On Sat, 10 May 2014, Rafael J. Wysocki wrote:
> 
> > On Thursday, May 08, 2014 09:52:18 PM Alan Stern wrote:
> > > On Thu, 8 May 2014, Rafael J. Wysocki wrote:
> > > 
> > > > Well, no.
> > > > 
> > > > The reason why that doesn't work is because ->prepare() callbacks are
> > > > executed in the reverse order, so the perent's ones will be run before
> > > > the ->prepare() of the children.  Thus if ->prepare() sets the flag
> > > > with the expectation that ->suspend() (and the subsequent callbacks)
> > > > won't be executed, that expectation may not be met actually.
> > > 
> > > That's true also if the flag gets set in ->suspend(), isn't it?  A
> > > driver may set direct_resume in its ->suspend() callback, expecting
> > > that the subsequent callbacks won't be executed.  But if a descendant
> > > hasn't also set its flag then the callbacks _will_ be executed.
> > 
> > No, that's not possible with the current patch, because __device_suspend() 
> > is
> > executed for descendants first and then for ancestors and it clears
> > direct_suspend for the parents of devices that don't have it set.  This 
> > means
> > that the ancestor's ->suspend() will see the flag clear if it is unset for
> > any of its descendants.
> > 
> > IOW, the only case in which the ancestor's ->suspend() sees the flag set is
> > when it has been set for all of its descendants.  Thus, if it leaves the
> > flag set, the late/early and noirq callbacks won't be executed for it.
> > 
> > Now, there is a reason for concern in that, because ->suspend() may set the
> > flag as a result of an error and that may lead to unexpected consequences.
> 
> Ah, my mistake; I should have read the patch more carefully.  I didn't
> realize that your plan was for subsystems/drivers to set the flag
> during ->prepare() and then clear it (or leave it set) during
> ->suspend().
> 
> > Then the parent will have direct_resume unset.  That is not a concern.
> > The only concern to me is possible errors in ->suspend() setting the
> > flag when it shouldn't.
> 
> So now one question is: Why would a subsystem or driver want to clear a
> flag that it had set earlier?  I can't think of any good reasons.  The
> only obvious possibility would be if the wakeup requirements got
> changed between ->prepare() and ->suspend(), but that should never
> happen because wakeup settings are changed by userspace and userspace
> will be frozen.
> 
> Another question is: Does a subsystem or driver need to know if the
> original flag setting couldn't be honored?  Again, I don't think it is 
> necessary to call ->suspend() just for this reason.  More precisely, I 
> think it will be good enough to call ->suspend() when the flag is clear 
> (either because a descendant device didn't set its flag or because the 
> device is no longer in runtime suspend); if the flag is set then there 
> is no reason to call ->suspend().  The subsystem can assume that 
> ->suspend() won't be called; then if it does get called, the subsystem 
> will realize something has changed.
> 
> Thus, a suitable algorithm now appears to be:
> 
>   Have subsystems/drivers set the flag during ->prepare().  They
>   don't even have to check if the device is runtime-suspended;
>   if it isn't then the PM core will turn off the flag later.
> 
>   In __device_suspend(), before invoking the ->supend() callback, 
>   check the flag.  If it is still set and if the device is
>   runtime-suspended (a barrier may be necessary here), skip 
>   ->suspend() and the following callbacks.  Otherwise clear the
>   parent's flag and proceed as usual.
> 
> > > Several of these questions are a lot easier to answer if the flag gets 
> > > set during ->prepare() rather than ->suspend().

I actually decided to go that way with one difference.  I think it's better
to make the PM core own the new flag, so that bus types/drivers don't have
to set/clear it, so I got back to my very first idea about possibly returning
positive values from ->prepare().

The idea is this:

 - If ->prepare() returns a positive number, that means "this device is
   runtime-suspended and you can leave it like that if you do the same
   thing for all of its descendants".

 - If that happens, the PM core sets the new flag for the device in
   question *if* the device is indeed runtime-suspended *and* *if*
   the transition is a suspend (and not hibernation, for example).
   Otherwise, it clears the flag for the device.  All of that happens in
   device_prepare().

 - In __device_suspend() the PM core clears the flag for the device's
   parent if it is clear for the device to ensure that the flag will only
   be set for a device if it is also set for all of its descendants.

 - PM core skips ->suspend/late/noirq and ->resume/early/noirq for all devices
   having the flag set - so the flag can be called "direct_complete" as it
   causes the PM core to go directy for the ->complete() cal

Re: [RFC][PATCH 1/3] PM / sleep: Flag to speed up suspend-resume of runtime-suspended devices

2014-05-12 Thread Kevin Hilman
"Rafael J. Wysocki"  writes:

> On Friday, May 09, 2014 03:48:21 PM Kevin Hilman wrote:
>> "Rafael J. Wysocki"  writes:
>> 
>> > From: Rafael J. Wysocki 
>> >
>> > Currently, some subsystems (e.g. PCI and the ACPI PM domain) have to
>> > resume all runtime-suspended devices during system suspend, mostly
>> > because those devices may need to be reprogrammed due to different
>> > wakeup settings for system sleep and for runtime PM.
>> >
>> > For some devices, though, it's OK to remain in runtime suspend 
>> > throughout a complete system suspend/resume cycle (if the device was in
>> > runtime suspend at the start of the cycle).  We would like to do this
>> > whenever possible, to avoid the overhead of extra power-up and power-down
>> > events.
>> >
>> > However, problems may arise because the device's descendants may require
>> > it to be at full power at various points during the cycle.  Therefore the
>> > most straightforward way to do this safely is if the device and all its
>> > descendants can remain runtime suspended until the resume stage of system
>> > resume.
>> >
>> > To this end, introduce dev->power.leave_runtime_suspended.
>> > If a subsystem or driver sets this flag during the ->prepare() callback,
>> > and if the flag is set in all of the device's descendants, and if the
>> > device is still in runtime suspend at the beginning of the ->suspend()
>> > callback, that callback is allowed to return 0 without clearing
>> > power.leave_runtime_suspended and without changing the state of the
>> > device, unless the current state of the device is not appropriate for
>> > the upcoming system sleep state (for example, the device is supposed to
>> > wake up the system from that state and its current wakeup settings are
>> > not suitable for that).  Then, the PM core will not invoke the device's
>> > ->suspend_late(), ->suspend_irq(), ->resume_irq(), ->resume_early(), or
>> > ->resume() callbacks.  
>> 
>> Up to here, this sounds great.
>> 
>> > Instead, it will invoke ->runtime_resume() during the device resume
>> > stage of system resume.
>> 
>> But this part I'm not fully following...
>
> You're not looking at the most recent one. :-)

Sorry about that, I haven't been able to keep up with the versions.

> Please look here: https://patchwork.kernel.org/patch/4139181/

OK.  

>> > By leaving this flag set after ->suspend(), a driver or subsystem tells
>> > the PM core that the device is runtime suspended, it is in a suitable
>> > state for system suspend (for example, the wakeup setting does not
>> > need to be changed), and it does not need to return to full
>> > power until the resume stage.
>> 
>> But taking this "leave runtime suspended" idea the next logical step,
>> why would/should a device need to return to full power at the ->resume()
>> stage?  especially when it wasn't at full power when ->suspend()
>> happened?
>
> Good question and I've been thinking about that for a while.
>
> Generally, the main reason for resuming is that on some platforms devices are
> automatically powered up by firmware and in those cases it's better to
> resume them (to make the runtime PM status reflect the physical state) and
> suspend again later.
>
> Generally speaking, subsystems that need to do that know what they are and
> that's what I was talking about in the most recent reply to Alan:
>
> http://marc.info/?l=linux-pm&m=139967477806094&w=4
>
> Currently, I think, there are two options on the table really.
>
>  1. Do more or less what https://patchwork.kernel.org/patch/4139181/ does
> with a modification to check that ->suspend() doesn't "cheat" (by setting
> the flag that had been unset before it was called).  The subsystem's
> ->resume() would then decide what to do with the device (resume it or
> leave it suspended).
>
>  2. Do what Alan was suggesting, that is set the flag in ->prepare() and
> make the PM core skip *all* of the system suspend/resume callbacks
> for devices with that flag set and let the ->complete() callback
> decide what to do with the device.
>
> I'm leaning a bit towards 2, but still considering 1 too.

If it matters, I have a slight preference for 2 also, though as long as
the subsytem/device gets to decide whether to resume, I think I'm OK
with either approach.

Kevin

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC][PATCH 1/3] PM / sleep: Flag to speed up suspend-resume of runtime-suspended devices

2014-05-11 Thread Alan Stern
On Sat, 10 May 2014, Rafael J. Wysocki wrote:

> On Thursday, May 08, 2014 09:52:18 PM Alan Stern wrote:
> > On Thu, 8 May 2014, Rafael J. Wysocki wrote:
> > 
> > > Well, no.
> > > 
> > > The reason why that doesn't work is because ->prepare() callbacks are
> > > executed in the reverse order, so the perent's ones will be run before
> > > the ->prepare() of the children.  Thus if ->prepare() sets the flag
> > > with the expectation that ->suspend() (and the subsequent callbacks)
> > > won't be executed, that expectation may not be met actually.
> > 
> > That's true also if the flag gets set in ->suspend(), isn't it?  A
> > driver may set direct_resume in its ->suspend() callback, expecting
> > that the subsequent callbacks won't be executed.  But if a descendant
> > hasn't also set its flag then the callbacks _will_ be executed.
> 
> No, that's not possible with the current patch, because __device_suspend() is
> executed for descendants first and then for ancestors and it clears
> direct_suspend for the parents of devices that don't have it set.  This means
> that the ancestor's ->suspend() will see the flag clear if it is unset for
> any of its descendants.
> 
> IOW, the only case in which the ancestor's ->suspend() sees the flag set is
> when it has been set for all of its descendants.  Thus, if it leaves the
> flag set, the late/early and noirq callbacks won't be executed for it.
> 
> Now, there is a reason for concern in that, because ->suspend() may set the
> flag as a result of an error and that may lead to unexpected consequences.

Ah, my mistake; I should have read the patch more carefully.  I didn't
realize that your plan was for subsystems/drivers to set the flag
during ->prepare() and then clear it (or leave it set) during
->suspend().

> Then the parent will have direct_resume unset.  That is not a concern.
> The only concern to me is possible errors in ->suspend() setting the
> flag when it shouldn't.

So now one question is: Why would a subsystem or driver want to clear a
flag that it had set earlier?  I can't think of any good reasons.  The
only obvious possibility would be if the wakeup requirements got
changed between ->prepare() and ->suspend(), but that should never
happen because wakeup settings are changed by userspace and userspace
will be frozen.

Another question is: Does a subsystem or driver need to know if the
original flag setting couldn't be honored?  Again, I don't think it is 
necessary to call ->suspend() just for this reason.  More precisely, I 
think it will be good enough to call ->suspend() when the flag is clear 
(either because a descendant device didn't set its flag or because the 
device is no longer in runtime suspend); if the flag is set then there 
is no reason to call ->suspend().  The subsystem can assume that 
->suspend() won't be called; then if it does get called, the subsystem 
will realize something has changed.

Thus, a suitable algorithm now appears to be:

Have subsystems/drivers set the flag during ->prepare().  They
don't even have to check if the device is runtime-suspended;
if it isn't then the PM core will turn off the flag later.

In __device_suspend(), before invoking the ->supend() callback, 
check the flag.  If it is still set and if the device is
runtime-suspended (a barrier may be necessary here), skip 
->suspend() and the following callbacks.  Otherwise clear the
parent's flag and proceed as usual.

> > Several of these questions are a lot easier to answer if the flag gets 
> > set during ->prepare() rather than ->suspend().
> 
> I agree with that, but I have one concern about this approach.  Namely,
> in that case the PM core has to use pm_runtime_resume() or equivalent to
> resume devices with the flag set during the device resume stage.  Now,
> in the next step we may want to leave certain devices suspended at that
> point and the PM core has no way to tell which ones.  Also subsystems
> don't really have a chance to tell it about that (they would need to
> know in advance during ->prepare(), which is kind of unrealistic, or
> perhaps it isn't).
> 
> However, if ->resume() is called for devices with the flag set, like in
> my most recent patch, the subsystem may decide not to resume the device
> if it knows enough about it.
> 
> This pretty much is my only concern here, so I'm open to ideas how to deal
> with leaving devices suspended (if possible) during the device resume stage. 
> :-)

This is a good question.  I'm not sure of the best answer at the 
moment.

> For one, postponing the resume to ->complete() is an option, but it will have
> to be done with care, because the ->complete() callbacks are executed
> sequentially, so calling pm_runtime_resume() from there is rather out of the
> question.

Calling pm_request_resume() would be okay, though.

There's another aspect to this we need to consider: hibernation.  I'm
quite sure we don't want to come out of hibernation think

Re: [RFC][PATCH 1/3] PM / sleep: Flag to speed up suspend-resume of runtime-suspended devices

2014-05-09 Thread Rafael J. Wysocki
On Friday, May 09, 2014 03:48:21 PM Kevin Hilman wrote:
> "Rafael J. Wysocki"  writes:
> 
> > From: Rafael J. Wysocki 
> >
> > Currently, some subsystems (e.g. PCI and the ACPI PM domain) have to
> > resume all runtime-suspended devices during system suspend, mostly
> > because those devices may need to be reprogrammed due to different
> > wakeup settings for system sleep and for runtime PM.
> >
> > For some devices, though, it's OK to remain in runtime suspend 
> > throughout a complete system suspend/resume cycle (if the device was in
> > runtime suspend at the start of the cycle).  We would like to do this
> > whenever possible, to avoid the overhead of extra power-up and power-down
> > events.
> >
> > However, problems may arise because the device's descendants may require
> > it to be at full power at various points during the cycle.  Therefore the
> > most straightforward way to do this safely is if the device and all its
> > descendants can remain runtime suspended until the resume stage of system
> > resume.
> >
> > To this end, introduce dev->power.leave_runtime_suspended.
> > If a subsystem or driver sets this flag during the ->prepare() callback,
> > and if the flag is set in all of the device's descendants, and if the
> > device is still in runtime suspend at the beginning of the ->suspend()
> > callback, that callback is allowed to return 0 without clearing
> > power.leave_runtime_suspended and without changing the state of the
> > device, unless the current state of the device is not appropriate for
> > the upcoming system sleep state (for example, the device is supposed to
> > wake up the system from that state and its current wakeup settings are
> > not suitable for that).  Then, the PM core will not invoke the device's
> > ->suspend_late(), ->suspend_irq(), ->resume_irq(), ->resume_early(), or
> > ->resume() callbacks.  
> 
> Up to here, this sounds great.
> 
> > Instead, it will invoke ->runtime_resume() during the device resume
> > stage of system resume.
> 
> But this part I'm not fully following...

You're not looking at the most recent one. :-)

Please look here: https://patchwork.kernel.org/patch/4139181/

> > By leaving this flag set after ->suspend(), a driver or subsystem tells
> > the PM core that the device is runtime suspended, it is in a suitable
> > state for system suspend (for example, the wakeup setting does not
> > need to be changed), and it does not need to return to full
> > power until the resume stage.
> 
> But taking this "leave runtime suspended" idea the next logical step,
> why would/should a device need to return to full power at the ->resume()
> stage?  especially when it wasn't at full power when ->suspend()
> happened?

Good question and I've been thinking about that for a while.

Generally, the main reason for resuming is that on some platforms devices are
automatically powered up by firmware and in those cases it's better to
resume them (to make the runtime PM status reflect the physical state) and
suspend again later.

Generally speaking, subsystems that need to do that know what they are and
that's what I was talking about in the most recent reply to Alan:

http://marc.info/?l=linux-pm&m=139967477806094&w=4

Currently, I think, there are two options on the table really.

 1. Do more or less what https://patchwork.kernel.org/patch/4139181/ does
with a modification to check that ->suspend() doesn't "cheat" (by setting
the flag that had been unset before it was called).  The subsystem's
->resume() would then decide what to do with the device (resume it or
leave it suspended).

 2. Do what Alan was suggesting, that is set the flag in ->prepare() and
make the PM core skip *all* of the system suspend/resume callbacks
for devices with that flag set and let the ->complete() callback
decide what to do with the device.

I'm leaning a bit towards 2, but still considering 1 too.

Thanks!


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC][PATCH 1/3] PM / sleep: Flag to speed up suspend-resume of runtime-suspended devices

2014-05-09 Thread Kevin Hilman
"Rafael J. Wysocki"  writes:

> From: Rafael J. Wysocki 
>
> Currently, some subsystems (e.g. PCI and the ACPI PM domain) have to
> resume all runtime-suspended devices during system suspend, mostly
> because those devices may need to be reprogrammed due to different
> wakeup settings for system sleep and for runtime PM.
>
> For some devices, though, it's OK to remain in runtime suspend 
> throughout a complete system suspend/resume cycle (if the device was in
> runtime suspend at the start of the cycle).  We would like to do this
> whenever possible, to avoid the overhead of extra power-up and power-down
> events.
>
> However, problems may arise because the device's descendants may require
> it to be at full power at various points during the cycle.  Therefore the
> most straightforward way to do this safely is if the device and all its
> descendants can remain runtime suspended until the resume stage of system
> resume.
>
> To this end, introduce dev->power.leave_runtime_suspended.
> If a subsystem or driver sets this flag during the ->prepare() callback,
> and if the flag is set in all of the device's descendants, and if the
> device is still in runtime suspend at the beginning of the ->suspend()
> callback, that callback is allowed to return 0 without clearing
> power.leave_runtime_suspended and without changing the state of the
> device, unless the current state of the device is not appropriate for
> the upcoming system sleep state (for example, the device is supposed to
> wake up the system from that state and its current wakeup settings are
> not suitable for that).  Then, the PM core will not invoke the device's
> ->suspend_late(), ->suspend_irq(), ->resume_irq(), ->resume_early(), or
> ->resume() callbacks.  

Up to here, this sounds great.

> Instead, it will invoke ->runtime_resume() during the device resume
> stage of system resume.

But this part I'm not fully following...

> By leaving this flag set after ->suspend(), a driver or subsystem tells
> the PM core that the device is runtime suspended, it is in a suitable
> state for system suspend (for example, the wakeup setting does not
> need to be changed), and it does not need to return to full
> power until the resume stage.

But taking this "leave runtime suspended" idea the next logical step,
why would/should a device need to return to full power at the ->resume()
stage?  especially when it wasn't at full power when ->suspend()
happened?

IOW, why doesn't "leave runtime suspended" mean "leave runtime suspended
until runtime resumed on demand."

Forcing ->runtime_resume() during device resume means that in most
cases, a device will be forcibly runtime resumed, only to have nothing
to do but go idle and runtime suspend again, resulting in a(nother)
unnessary power-up, power-down cycle this patch is trying to avoid
during ->suspend().

Hmm, but wait a minute...

[...]

> @@ -735,6 +735,11 @@ static int device_resume(struct device *
>   if (dev->power.syscore)
>   goto Complete;
>  
> + if (pm_leave_runtime_suspended(dev)) {
> + pm_runtime_resume(dev);
> + goto Complete;
> + }

... maybe I'm forgetting how this works (since it's Friday and my brain
is already shutting down for the week) but after pm_runtime_resume() is
called, won't the device remain runtime active until
pm_runtime_suspend() is called, or until a
pm_runtime_get()/pm_runtime_put() cycle happens?

That means that on device resume, the device is forced into full power
state (even though it was runtime suspended when ->suspend() happened)
and will stay there until its used again.  

That seems like a rather unpleasant (and non-intuitive) side-effect of
"leave runtime suspended".

Kevin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC][PATCH 1/3] PM / sleep: Flag to speed up suspend-resume of runtime-suspended devices

2014-05-09 Thread Rafael J. Wysocki
On Thursday, May 08, 2014 09:52:18 PM Alan Stern wrote:
> On Thu, 8 May 2014, Rafael J. Wysocki wrote:
> 
> > Well, no.
> > 
> > The reason why that doesn't work is because ->prepare() callbacks are
> > executed in the reverse order, so the perent's ones will be run before
> > the ->prepare() of the children.  Thus if ->prepare() sets the flag
> > with the expectation that ->suspend() (and the subsequent callbacks)
> > won't be executed, that expectation may not be met actually.
> 
> That's true also if the flag gets set in ->suspend(), isn't it?  A
> driver may set direct_resume in its ->suspend() callback, expecting
> that the subsequent callbacks won't be executed.  But if a descendant
> hasn't also set its flag then the callbacks _will_ be executed.

No, that's not possible with the current patch, because __device_suspend() is
executed for descendants first and then for ancestors and it clears
direct_suspend for the parents of devices that don't have it set.  This means
that the ancestor's ->suspend() will see the flag clear if it is unset for
any of its descendants.

IOW, the only case in which the ancestor's ->suspend() sees the flag set is
when it has been set for all of its descendants.  Thus, if it leaves the
flag set, the late/early and noirq callbacks won't be executed for it.

Now, there is a reason for concern in that, because ->suspend() may set the
flag as a result of an error and that may lead to unexpected consequences.

> > So I'm going to do what I said above.  I prefer it anyway. :-)
> 
> In your most recent patch (and in the earlier ones too), after you call
> dev's ->suspend() routine, if dev->power.direct_resume isn't set then
> you clear dev->parent->direct_resume.  But what good will that do if
> dev->parent's ->suspend() routine turns the flag back on when it gets
> called later?

In fact, ->suspend() is not supposed to set the flag when it is clear.
It can clear it when it is set, which means that we have "normal" suspend.

> I can think of two ways to make this work.
> 
>   Expect subsystems and drivers to set the flag during 
>   ->suspend().  Turn on the flag in every device during 
>   device_prepare().  Then in __device_suspend(), remember the
>   flag's value and turn it off before invoking the callback.

That doesn't work, because ->suspend() has to decide whether or not
to resume the device and do things it would do normally, so it needs to know
the value of the flag.

>   If the flag is on again when the callback returns, set the flag 
>   back to the remembered value.  If the flag ends up being off
>   then turn off the parent's flag.

That'd be too late.  The only thing we can do to kind of protect the PM
core from errors in drivers in that case would be to remember the value of
the flag before calling ->suspend() and return an error if it the flag after
->suspend() is set, but it wasn't before.

>   Expect subsystems and drivers to set the flag during 
>   ->prepare().  Whenever a callback returns with the flag not
>   set, clear the flag in all of the device's ancestors.
> 
> Both are somewhat awkward, and both involve turning the flag off after 
> the callback has turned it on.

After the callback set it on while it shouldn't, it might have done something
wrong already.

> Also, how do you expect direct_resume to work with the PCI subsystem?  
> Will the PCI core set the flag appropriately on behalf of the driver?

Yes.

> If the core does set the flag, will it invoke the driver's ->suspend()
> callback or skip the callback?

It will skip the driver's callback.  [It would actually help if you looked
at patch [3/3] which is there to illustrate my idea of how to do those
things in a subsystem.]

> If it invokes the driver's callback but
> leaves the device in runtime suspend, what happens if the driver
> expects the device always to be at full power when its ->suspend()  
> routine runs?  If the core skips the driver's ->suspend() callback,
> what happens if one of the device's children did not set direct_resume 
> and so the later PM callbacks do get invoked?

Then the parent will have direct_resume unset.  That is not a concern.
The only concern to me is possible errors in ->suspend() setting the
flag when it shouldn't.

> Several of these questions are a lot easier to answer if the flag gets 
> set during ->prepare() rather than ->suspend().

I agree with that, but I have one concern about this approach.  Namely,
in that case the PM core has to use pm_runtime_resume() or equivalent to
resume devices with the flag set during the device resume stage.  Now,
in the next step we may want to leave certain devices suspended at that
point and the PM core has no way to tell which ones.  Also subsystems
don't really have a chance to tell it about that (they would need to
know in advance during ->prepare(), which is kind of unrealistic, or
perhaps it isn't).

However, if ->resume() is called for devices with the flag set, like in
my mos

Re: [RFC][PATCH 1/3] PM / sleep: Flag to speed up suspend-resume of runtime-suspended devices

2014-05-09 Thread Rafael J. Wysocki
On Friday, May 09, 2014 09:23:50 AM Ulf Hansson wrote:
> Hi Rafael,
> 
> > @@ -1485,3 +1486,10 @@ out:
> > return ret;
> >  }
> >  EXPORT_SYMBOL_GPL(pm_runtime_force_resume);
> > +
> > +void pm_set_direct_resume(struct device *dev, bool val)
> > +{
> > +   spin_lock_irq(&dev->power.lock);
> > +   __set_direct_resume(dev, val);
> > +   spin_unlock_irq(&dev->power.lock);
> > +}
> 
> I believe you have to move the implementation of this function inside
> #ifdef CONFIG_PM_RUNTIME.

You're right, I forgot about the recent change in there.

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC][PATCH 1/3] PM / sleep: Flag to speed up suspend-resume of runtime-suspended devices

2014-05-09 Thread Ulf Hansson
Hi Rafael,

> @@ -1485,3 +1486,10 @@ out:
> return ret;
>  }
>  EXPORT_SYMBOL_GPL(pm_runtime_force_resume);
> +
> +void pm_set_direct_resume(struct device *dev, bool val)
> +{
> +   spin_lock_irq(&dev->power.lock);
> +   __set_direct_resume(dev, val);
> +   spin_unlock_irq(&dev->power.lock);
> +}

I believe you have to move the implementation of this function inside
#ifdef CONFIG_PM_RUNTIME.

Kind regards
Ulf Hansson
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC][PATCH 1/3] PM / sleep: Flag to speed up suspend-resume of runtime-suspended devices

2014-05-08 Thread Alan Stern
On Thu, 8 May 2014, Rafael J. Wysocki wrote:

> Well, no.
> 
> The reason why that doesn't work is because ->prepare() callbacks are
> executed in the reverse order, so the perent's ones will be run before
> the ->prepare() of the children.  Thus if ->prepare() sets the flag
> with the expectation that ->suspend() (and the subsequent callbacks)
> won't be executed, that expectation may not be met actually.

That's true also if the flag gets set in ->suspend(), isn't it?  A
driver may set direct_resume in its ->suspend() callback, expecting
that the subsequent callbacks won't be executed.  But if a descendant
hasn't also set its flag then the callbacks _will_ be executed.

> So I'm going to do what I said above.  I prefer it anyway. :-)

In your most recent patch (and in the earlier ones too), after you call
dev's ->suspend() routine, if dev->power.direct_resume isn't set then
you clear dev->parent->direct_resume.  But what good will that do if
dev->parent's ->suspend() routine turns the flag back on when it gets
called later?

I can think of two ways to make this work.

Expect subsystems and drivers to set the flag during 
->suspend().  Turn on the flag in every device during 
device_prepare().  Then in __device_suspend(), remember the
flag's value and turn it off before invoking the callback.  If 
the flag is on again when the callback returns, set the flag 
back to the remembered value.  If the flag ends up being off
then turn off the parent's flag.

Expect subsystems and drivers to set the flag during 
->prepare().  Whenever a callback returns with the flag not
set, clear the flag in all of the device's ancestors.

Both are somewhat awkward, and both involve turning the flag off after 
the callback has turned it on.

Also, how do you expect direct_resume to work with the PCI subsystem?  
Will the PCI core set the flag appropriately on behalf of the driver?  
If the core does set the flag, will it invoke the driver's ->suspend()
callback or skip the callback?  If it invokes the driver's callback but
leaves the device in runtime suspend, what happens if the driver
expects the device always to be at full power when its ->suspend()  
routine runs?  If the core skips the driver's ->suspend() callback,
what happens if one of the device's children did not set direct_resume 
and so the later PM callbacks do get invoked?

Several of these questions are a lot easier to answer if the flag gets 
set during ->prepare() rather than ->suspend().

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC][PATCH 1/3] PM / sleep: Flag to speed up suspend-resume of runtime-suspended devices

2014-05-08 Thread Rafael J. Wysocki
On Thursday, May 08, 2014 11:42:01 PM Rafael J. Wysocki wrote:
> On Thursday, May 08, 2014 05:20:43 PM Alan Stern wrote:
> > On Thu, 8 May 2014, Rafael J. Wysocki wrote:
> > 
> > > > > Wait a minute.  Following ->runtime_suspend(), you are going to call 
> > > > > ->suspend() and then ->runtime_resume()?  That doesn't seem like what 
> > > > > you really want; a ->suspend() call should always have a matching 
> > > > > ->resume().
> > > > 
> > > > Yes, it should, but I didn't see any other way to do that.
> > > 
> > > Actually, that's kind of easy to resolve. :-)
> > > 
> > > When ->suspend() leaves power.leave_runtime_suspended set, the PM core can
> > > simply skip the early/late and noirq callbacks and then call ->resume()
> > > that will be responsible for using whatever is necessary to resume the
> > > device.
> > > 
> > > And perhaps the flag should be called something different then, like
> > > direct_resume (meaning go directly for ->resume() without executing
> > > the intermediate callbacks)?
> > 
> > In light of what I wrote earlier, it should be okay for the ->prepare() 
> > callback to be responsible for setting leave_runtime_suspended.  Then 
> > there will be no need to call either ->suspend() or ->resume().
> 
> Hmm.  OK, let's try that.

Well, no.

The reason why that doesn't work is because ->prepare() callbacks are
executed in the reverse order, so the perent's ones will be run before
the ->prepare() of the children.  Thus if ->prepare() sets the flag
with the expectation that ->suspend() (and the subsequent callbacks)
won't be executed, that expectation may not be met actually.

So I'm going to do what I said above.  I prefer it anyway. :-)

Rafael

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC][PATCH 1/3] PM / sleep: Flag to speed up suspend-resume of runtime-suspended devices

2014-05-08 Thread Rafael J. Wysocki
On Thursday, May 08, 2014 05:20:43 PM Alan Stern wrote:
> On Thu, 8 May 2014, Rafael J. Wysocki wrote:
> 
> > > > Wait a minute.  Following ->runtime_suspend(), you are going to call 
> > > > ->suspend() and then ->runtime_resume()?  That doesn't seem like what 
> > > > you really want; a ->suspend() call should always have a matching 
> > > > ->resume().
> > > 
> > > Yes, it should, but I didn't see any other way to do that.
> > 
> > Actually, that's kind of easy to resolve. :-)
> > 
> > When ->suspend() leaves power.leave_runtime_suspended set, the PM core can
> > simply skip the early/late and noirq callbacks and then call ->resume()
> > that will be responsible for using whatever is necessary to resume the
> > device.
> > 
> > And perhaps the flag should be called something different then, like
> > direct_resume (meaning go directly for ->resume() without executing
> > the intermediate callbacks)?
> 
> In light of what I wrote earlier, it should be okay for the ->prepare() 
> callback to be responsible for setting leave_runtime_suspended.  Then 
> there will be no need to call either ->suspend() or ->resume().

Hmm.  OK, let's try that.

Rafael

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC][PATCH 1/3] PM / sleep: Flag to speed up suspend-resume of runtime-suspended devices

2014-05-08 Thread Alan Stern
On Thu, 8 May 2014, Rafael J. Wysocki wrote:

> > > Wait a minute.  Following ->runtime_suspend(), you are going to call 
> > > ->suspend() and then ->runtime_resume()?  That doesn't seem like what 
> > > you really want; a ->suspend() call should always have a matching 
> > > ->resume().
> > 
> > Yes, it should, but I didn't see any other way to do that.
> 
> Actually, that's kind of easy to resolve. :-)
> 
> When ->suspend() leaves power.leave_runtime_suspended set, the PM core can
> simply skip the early/late and noirq callbacks and then call ->resume()
> that will be responsible for using whatever is necessary to resume the
> device.
> 
> And perhaps the flag should be called something different then, like
> direct_resume (meaning go directly for ->resume() without executing
> the intermediate callbacks)?

In light of what I wrote earlier, it should be okay for the ->prepare() 
callback to be responsible for setting leave_runtime_suspended.  Then 
there will be no need to call either ->suspend() or ->resume().

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC][PATCH 1/3] PM / sleep: Flag to speed up suspend-resume of runtime-suspended devices

2014-05-08 Thread Alan Stern
On Thu, 8 May 2014, Rafael J. Wysocki wrote:

> > Wait a minute.  Following ->runtime_suspend(), you are going to call 
> > ->suspend() and then ->runtime_resume()?  That doesn't seem like what 
> > you really want; a ->suspend() call should always have a matching 
> > ->resume().
> 
> Yes, it should, but I didn't see any other way to do that.
> 
> > I guess you did it this way to allow for runtime-resumes and -suspends 
> > between ->prepare() and ->suspend(), but it still seems wrong.
> 
> No.  I did that to allow ->suspend() to check whether or not the device is
> in the right state.  ->prepare() could do that, arguably, but then there's
> the case when ->runtime_suspend() may still be running in parallel with it.
> And the device may be runtime-suspended immediately before its ->suspend()
> in theory if its children do pm_runtime_put_sync(parent).
> 
> Also, this is a bus type ->suspend(), so the *driver* ->suspend()
> won't be called at this point in the ACPI PM domain case for example.
> 
> > How about asking drivers to set leave_runtime_suspended in their
> > ->runtime_suspend() callbacks, as well as during ->prepare()?  Then
> > intervening runtime resume/suspend cycles wouldn't matter and you
> > wouldn't need to call ->suspend(); you could skip it along with the
> > other PM callbacks.
> 
> That wouldn't work, because they cannot know the target sleep state of the
> system in advance.  This only is known during the given suspend sequence.

Argh!  We're both being foolish.  Runtime suspends can't occur between 
->prepare() and ->suspend(), because device_prepare() does a 
pm_runtime_get_noresume.

You might still have to worry about a runtime suspend concurrent with
->prepare(), though.  An appropriate barrier could fix that.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC][PATCH 1/3] PM / sleep: Flag to speed up suspend-resume of runtime-suspended devices

2014-05-08 Thread Rafael J. Wysocki
On Thursday, May 08, 2014 10:17:50 PM Rafael J. Wysocki wrote:
> On Thursday, May 08, 2014 10:57:36 AM Alan Stern wrote:
> > On Thu, 8 May 2014, Rafael J. Wysocki wrote:
> > 
> > > From: Rafael J. Wysocki 
> > > 
> > > Currently, some subsystems (e.g. PCI and the ACPI PM domain) have to
> > > resume all runtime-suspended devices during system suspend, mostly
> > > because those devices may need to be reprogrammed due to different
> > > wakeup settings for system sleep and for runtime PM.
> > > 
> > > For some devices, though, it's OK to remain in runtime suspend 
> > > throughout a complete system suspend/resume cycle (if the device was in
> > > runtime suspend at the start of the cycle).  We would like to do this
> > > whenever possible, to avoid the overhead of extra power-up and power-down
> > > events.
> > > 
> > > However, problems may arise because the device's descendants may require
> > > it to be at full power at various points during the cycle.  Therefore the
> > > most straightforward way to do this safely is if the device and all its
> > > descendants can remain runtime suspended until the resume stage of system
> > > resume.
> > > 
> > > To this end, introduce dev->power.leave_runtime_suspended.
> > > If a subsystem or driver sets this flag during the ->prepare() callback,
> > > and if the flag is set in all of the device's descendants, and if the
> > > device is still in runtime suspend at the beginning of the ->suspend()
> > > callback, that callback is allowed to return 0 without clearing
> > > power.leave_runtime_suspended and without changing the state of the
> > > device, unless the current state of the device is not appropriate for
> > > the upcoming system sleep state (for example, the device is supposed to
> > > wake up the system from that state and its current wakeup settings are
> > > not suitable for that).  Then, the PM core will not invoke the device's
> > > ->suspend_late(), ->suspend_irq(), ->resume_irq(), ->resume_early(), or
> > > ->resume() callbacks.  Instead, it will invoke ->runtime_resume() during
> > > the device resume stage of system resume.
> > 
> > Wait a minute.  Following ->runtime_suspend(), you are going to call 
> > ->suspend() and then ->runtime_resume()?  That doesn't seem like what 
> > you really want; a ->suspend() call should always have a matching 
> > ->resume().
> 
> Yes, it should, but I didn't see any other way to do that.

Actually, that's kind of easy to resolve. :-)

When ->suspend() leaves power.leave_runtime_suspended set, the PM core can
simply skip the early/late and noirq callbacks and then call ->resume()
that will be responsible for using whatever is necessary to resume the
device.

And perhaps the flag should be called something different then, like
direct_resume (meaning go directly for ->resume() without executing
the intermediate callbacks)?

Rafael

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC][PATCH 1/3] PM / sleep: Flag to speed up suspend-resume of runtime-suspended devices

2014-05-08 Thread Rafael J. Wysocki
On Thursday, May 08, 2014 10:57:36 AM Alan Stern wrote:
> On Thu, 8 May 2014, Rafael J. Wysocki wrote:
> 
> > From: Rafael J. Wysocki 
> > 
> > Currently, some subsystems (e.g. PCI and the ACPI PM domain) have to
> > resume all runtime-suspended devices during system suspend, mostly
> > because those devices may need to be reprogrammed due to different
> > wakeup settings for system sleep and for runtime PM.
> > 
> > For some devices, though, it's OK to remain in runtime suspend 
> > throughout a complete system suspend/resume cycle (if the device was in
> > runtime suspend at the start of the cycle).  We would like to do this
> > whenever possible, to avoid the overhead of extra power-up and power-down
> > events.
> > 
> > However, problems may arise because the device's descendants may require
> > it to be at full power at various points during the cycle.  Therefore the
> > most straightforward way to do this safely is if the device and all its
> > descendants can remain runtime suspended until the resume stage of system
> > resume.
> > 
> > To this end, introduce dev->power.leave_runtime_suspended.
> > If a subsystem or driver sets this flag during the ->prepare() callback,
> > and if the flag is set in all of the device's descendants, and if the
> > device is still in runtime suspend at the beginning of the ->suspend()
> > callback, that callback is allowed to return 0 without clearing
> > power.leave_runtime_suspended and without changing the state of the
> > device, unless the current state of the device is not appropriate for
> > the upcoming system sleep state (for example, the device is supposed to
> > wake up the system from that state and its current wakeup settings are
> > not suitable for that).  Then, the PM core will not invoke the device's
> > ->suspend_late(), ->suspend_irq(), ->resume_irq(), ->resume_early(), or
> > ->resume() callbacks.  Instead, it will invoke ->runtime_resume() during
> > the device resume stage of system resume.
> 
> Wait a minute.  Following ->runtime_suspend(), you are going to call 
> ->suspend() and then ->runtime_resume()?  That doesn't seem like what 
> you really want; a ->suspend() call should always have a matching 
> ->resume().

Yes, it should, but I didn't see any other way to do that.

> I guess you did it this way to allow for runtime-resumes and -suspends 
> between ->prepare() and ->suspend(), but it still seems wrong.

No.  I did that to allow ->suspend() to check whether or not the device is
in the right state.  ->prepare() could do that, arguably, but then there's
the case when ->runtime_suspend() may still be running in parallel with it.
And the device may be runtime-suspended immediately before its ->suspend()
in theory if its children do pm_runtime_put_sync(parent).

Also, this is a bus type ->suspend(), so the *driver* ->suspend()
won't be called at this point in the ACPI PM domain case for example.

> How about asking drivers to set leave_runtime_suspended in their
> ->runtime_suspend() callbacks, as well as during ->prepare()?  Then
> intervening runtime resume/suspend cycles wouldn't matter and you
> wouldn't need to call ->suspend(); you could skip it along with the
> other PM callbacks.

That wouldn't work, because they cannot know the target sleep state of the
system in advance.  This only is known during the given suspend sequence.

> > By leaving this flag set after ->suspend(), a driver or subsystem tells
> > the PM core that the device is runtime suspended, it is in a suitable
> > state for system suspend (for example, the wakeup setting does not
> > need to be changed), and it does not need to return to full
> > power until the resume stage.
> 
> So: By setting this flag during ->runtime_suspend() and ->prepare(), a
> driver or subsystem tells the PM core that the device is in a suitable
> state for system suspend (for example, the wakeup setting would not
> need to be changed), if one should occur before the next runtime
> resume, and the device would not need to return to full power until the
> resume stage.
>
> > --- linux-pm.orig/include/linux/pm_runtime.h
> > +++ linux-pm/include/linux/pm_runtime.h
> > @@ -264,4 +264,20 @@ static inline void pm_runtime_dont_use_a
> > __pm_runtime_use_autosuspend(dev, false);
> >  }
> >  
> > +#ifdef CONFIG_PM_BOTH
> > +static inline void __set_leave_runtime_suspended(struct device *dev, bool 
> > val)
> > +{
> > +   dev->power.leave_runtime_suspended = val;
> > +}
> > +extern void pm_set_leave_runtime_suspended(struct device *dev, bool val);
> > +static inline bool pm_leave_runtime_suspended(struct device *dev)
> > +{
> > +   return dev->power.leave_runtime_suspended;
> > +}
> 
> Is it generally your custom to use "set_" and "" rather than "set_" and 
> "get_"?

But (dev->power.syscore || pm_get_leave_runtime_suspended(dev)) looks awkward. 
:-)
 
> >   End:
> > if (!error) {
> > +   struct device *parent = dev->parent;
> > +
> > dev->power.is_suspended = true;
> > -   

Re: [RFC][PATCH 1/3] PM / sleep: Flag to speed up suspend-resume of runtime-suspended devices

2014-05-08 Thread Rafael J. Wysocki
On Thursday, May 08, 2014 02:25:06 PM Ulf Hansson wrote:
> On 8 May 2014 13:44, Rafael J. Wysocki  wrote:
> > On Thursday, May 08, 2014 12:59:20 PM Ulf Hansson wrote:
> >> On 8 May 2014 12:53, Rafael J. Wysocki  wrote:
> >> > On Thursday, May 08, 2014 09:49:36 AM Ulf Hansson wrote:
> >> >> On 8 May 2014 01:29, Rafael J. Wysocki  wrote:
> >> >> > From: Rafael J. Wysocki 
> >> >> >
> >> >> > Currently, some subsystems (e.g. PCI and the ACPI PM domain) have to
> >> >> > resume all runtime-suspended devices during system suspend, mostly
> >> >> > because those devices may need to be reprogrammed due to different
> >> >> > wakeup settings for system sleep and for runtime PM.
> >> >> >
> >> >> > For some devices, though, it's OK to remain in runtime suspend
> >> >> > throughout a complete system suspend/resume cycle (if the device was 
> >> >> > in
> >> >> > runtime suspend at the start of the cycle).  We would like to do this
> >> >> > whenever possible, to avoid the overhead of extra power-up and 
> >> >> > power-down
> >> >> > events.
> >> >> >
> >> >> > However, problems may arise because the device's descendants may 
> >> >> > require
> >> >> > it to be at full power at various points during the cycle.  Therefore 
> >> >> > the
> >> >> > most straightforward way to do this safely is if the device and all 
> >> >> > its
> >> >> > descendants can remain runtime suspended until the resume stage of 
> >> >> > system
> >> >> > resume.
> >> >> >
> >> >> > To this end, introduce dev->power.leave_runtime_suspended.
> >> >> > If a subsystem or driver sets this flag during the ->prepare() 
> >> >> > callback,
> >> >> > and if the flag is set in all of the device's descendants, and if the
> >> >> > device is still in runtime suspend at the beginning of the ->suspend()
> >> >> > callback, that callback is allowed to return 0 without clearing
> >> >> > power.leave_runtime_suspended and without changing the state of the
> >> >> > device, unless the current state of the device is not appropriate for
> >> >> > the upcoming system sleep state (for example, the device is supposed 
> >> >> > to
> >> >> > wake up the system from that state and its current wakeup settings are
> >> >> > not suitable for that).  Then, the PM core will not invoke the 
> >> >> > device's
> >> >> > ->suspend_late(), ->suspend_irq(), ->resume_irq(), ->resume_early(), 
> >> >> > or
> >> >> > ->resume() callbacks.  Instead, it will invoke ->runtime_resume() 
> >> >> > during
> >> >> > the device resume stage of system resume.
> >> >> >
> >> >> > By leaving this flag set after ->suspend(), a driver or subsystem 
> >> >> > tells
> >> >> > the PM core that the device is runtime suspended, it is in a suitable
> >> >> > state for system suspend (for example, the wakeup setting does not
> >> >> > need to be changed), and it does not need to return to full
> >> >> > power until the resume stage.
> >> >> >
> >> >> > Changelog based on an Alan Stern's description of the idea
> >> >> > (http://marc.info/?l=linux-pm&m=139940466625569&w=2).
> >> >> >
> >> >> > Signed-off-by: Rafael J. Wysocki 
> >> >> > ---
> >> >> >  drivers/base/power/main.c|   31 ---
> >> >> >  drivers/base/power/runtime.c |   10 ++
> >> >> >  include/linux/pm.h   |3 +++
> >> >> >  include/linux/pm_runtime.h   |   16 
> >> >> >  kernel/power/Kconfig |4 
> >> >> >  5 files changed, 57 insertions(+), 7 deletions(-)
> >> >> >
> >> >> > Index: linux-pm/kernel/power/Kconfig
> >> >> > ===
> >> >> > --- linux-pm.orig/kernel/power/Kconfig
> >> >> > +++ linux-pm/kernel/power/Kconfig
> >> >> > @@ -147,6 +147,10 @@ config PM
> >> >> > def_bool y
> >> >> > depends on PM_SLEEP || PM_RUNTIME
> >> >> >
> >> >> > +config PM_BOTH
> >> >> > +   def_bool y
> >> >> > +   depends on PM_SLEEP && PM_RUNTIME
> >> >> > +
> >> >>
> >> >> Should we not depend on PM_RUNTIME only? Thus we don't need the new
> >> >> Kconfig,
> >> >
> >> > Well, OK.  I guess we can tolerate one useless statement in rpm_resume()
> >> > in case CONFIG_PM_SLEEP is unset.
> >> >
> >> >> and then we could rename the new APIs to pm_runtime_* instead.
> >> >
> >> > That would just make the name longer - for what value?
> >>
> >> Only "__set_leave_runtime_suspended" will be a bit longer.
> >>
> >> The idea I had was to clearly indicate, these functions is a part of
> >> PM_RUNTIME API.
> >>
> >> Compare what you have:
> >> __set_leave_runtime_suspended
> >> pm_set_leave_runtime_suspended
> >> pm_leave_runtime_suspended
> >>
> >> To what I suggest:
> >> __pm_runtime_set_leave_suspended
> >> pm_runtime_set_leave_suspended
> >> pm_runtime_leave_suspended
> >
> > And why exactly do you think these are any better?
> 
> Because that's how all (almost all) other functions in the runtime PM
> API are specified - I believe it makes sense to keep them aligned.
> 
> Anyway, if you insist in 

Re: [RFC][PATCH 1/3] PM / sleep: Flag to speed up suspend-resume of runtime-suspended devices

2014-05-08 Thread Alan Stern
On Thu, 8 May 2014, Rafael J. Wysocki wrote:

> From: Rafael J. Wysocki 
> 
> Currently, some subsystems (e.g. PCI and the ACPI PM domain) have to
> resume all runtime-suspended devices during system suspend, mostly
> because those devices may need to be reprogrammed due to different
> wakeup settings for system sleep and for runtime PM.
> 
> For some devices, though, it's OK to remain in runtime suspend 
> throughout a complete system suspend/resume cycle (if the device was in
> runtime suspend at the start of the cycle).  We would like to do this
> whenever possible, to avoid the overhead of extra power-up and power-down
> events.
> 
> However, problems may arise because the device's descendants may require
> it to be at full power at various points during the cycle.  Therefore the
> most straightforward way to do this safely is if the device and all its
> descendants can remain runtime suspended until the resume stage of system
> resume.
> 
> To this end, introduce dev->power.leave_runtime_suspended.
> If a subsystem or driver sets this flag during the ->prepare() callback,
> and if the flag is set in all of the device's descendants, and if the
> device is still in runtime suspend at the beginning of the ->suspend()
> callback, that callback is allowed to return 0 without clearing
> power.leave_runtime_suspended and without changing the state of the
> device, unless the current state of the device is not appropriate for
> the upcoming system sleep state (for example, the device is supposed to
> wake up the system from that state and its current wakeup settings are
> not suitable for that).  Then, the PM core will not invoke the device's
> ->suspend_late(), ->suspend_irq(), ->resume_irq(), ->resume_early(), or
> ->resume() callbacks.  Instead, it will invoke ->runtime_resume() during
> the device resume stage of system resume.

Wait a minute.  Following ->runtime_suspend(), you are going to call 
->suspend() and then ->runtime_resume()?  That doesn't seem like what 
you really want; a ->suspend() call should always have a matching 
->resume().

I guess you did it this way to allow for runtime-resumes and -suspends 
between ->prepare() and ->suspend(), but it still seems wrong.

How about asking drivers to set leave_runtime_suspended in their
->runtime_suspend() callbacks, as well as during ->prepare()?  Then
intervening runtime resume/suspend cycles wouldn't matter and you
wouldn't need to call ->suspend(); you could skip it along with the
other PM callbacks.

> By leaving this flag set after ->suspend(), a driver or subsystem tells
> the PM core that the device is runtime suspended, it is in a suitable
> state for system suspend (for example, the wakeup setting does not
> need to be changed), and it does not need to return to full
> power until the resume stage.

So: By setting this flag during ->runtime_suspend() and ->prepare(), a
driver or subsystem tells the PM core that the device is in a suitable
state for system suspend (for example, the wakeup setting would not
need to be changed), if one should occur before the next runtime
resume, and the device would not need to return to full power until the
resume stage.

> --- linux-pm.orig/include/linux/pm_runtime.h
> +++ linux-pm/include/linux/pm_runtime.h
> @@ -264,4 +264,20 @@ static inline void pm_runtime_dont_use_a
>   __pm_runtime_use_autosuspend(dev, false);
>  }
>  
> +#ifdef CONFIG_PM_BOTH
> +static inline void __set_leave_runtime_suspended(struct device *dev, bool 
> val)
> +{
> + dev->power.leave_runtime_suspended = val;
> +}
> +extern void pm_set_leave_runtime_suspended(struct device *dev, bool val);
> +static inline bool pm_leave_runtime_suspended(struct device *dev)
> +{
> + return dev->power.leave_runtime_suspended;
> +}

Is it generally your custom to use "set_" and "" rather than "set_" and 
"get_"?

>   End:
>   if (!error) {
> + struct device *parent = dev->parent;
> +
>   dev->power.is_suspended = true;
> - if (dev->power.wakeup_path
> - && dev->parent && !dev->parent->power.ignore_children)
> - dev->parent->power.wakeup_path = true;
> + if (parent) {
> + spin_lock_irq(&parent->power.lock);
> +
> + if (dev->power.wakeup_path
> + && !parent->power.ignore_children)
> + parent->power.wakeup_path = true;
> +
> + if (!pm_leave_runtime_suspended(dev))
> + __set_leave_runtime_suspended(parent, false);
> +
> + spin_unlock_irq(&parent->power.lock);
> + }

Then of course, this code would move up, before the callback, and the 
callback would be skipped if leave_runtime_suspended was set.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majord

Re: [RFC][PATCH 1/3] PM / sleep: Flag to speed up suspend-resume of runtime-suspended devices

2014-05-08 Thread Alan Stern
On Thu, 8 May 2014, Ulf Hansson wrote:

> >> Should we not depend on PM_RUNTIME only? Thus we don't need the new
> >> Kconfig,
> >
> > Well, OK.  I guess we can tolerate one useless statement in rpm_resume()
> > in case CONFIG_PM_SLEEP is unset.

It isn't a big deal.  However, Ulf, you need to understand that this 
API belongs to _both_ PM_SLEEP _and_ PM_RUNTIME.  It doesn't mean 
anything unless both are present.

But as Rafael said, the extra overhead if !CONFIG_PM_SLEEP is minimal.  
(Although I would move the new flag to be next to the other bitflags, 
so that it doesn't add an entire extra word to the dev_pm_info 
structure.)

> >> and then we could rename the new APIs to pm_runtime_* instead.
> >
> > That would just make the name longer - for what value?
> 
> Only "__set_leave_runtime_suspended" will be a bit longer.
> 
> The idea I had was to clearly indicate, these functions is a part of
> PM_RUNTIME API.

Not so.  They are part of both PM_SLEEP and PM_RUNTIME, which means
they are really just part of PM.  You can tell by the fact that they 
are used in both main.c and runtime.c.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC][PATCH 1/3] PM / sleep: Flag to speed up suspend-resume of runtime-suspended devices

2014-05-08 Thread Ulf Hansson
On 8 May 2014 13:44, Rafael J. Wysocki  wrote:
> On Thursday, May 08, 2014 12:59:20 PM Ulf Hansson wrote:
>> On 8 May 2014 12:53, Rafael J. Wysocki  wrote:
>> > On Thursday, May 08, 2014 09:49:36 AM Ulf Hansson wrote:
>> >> On 8 May 2014 01:29, Rafael J. Wysocki  wrote:
>> >> > From: Rafael J. Wysocki 
>> >> >
>> >> > Currently, some subsystems (e.g. PCI and the ACPI PM domain) have to
>> >> > resume all runtime-suspended devices during system suspend, mostly
>> >> > because those devices may need to be reprogrammed due to different
>> >> > wakeup settings for system sleep and for runtime PM.
>> >> >
>> >> > For some devices, though, it's OK to remain in runtime suspend
>> >> > throughout a complete system suspend/resume cycle (if the device was in
>> >> > runtime suspend at the start of the cycle).  We would like to do this
>> >> > whenever possible, to avoid the overhead of extra power-up and 
>> >> > power-down
>> >> > events.
>> >> >
>> >> > However, problems may arise because the device's descendants may require
>> >> > it to be at full power at various points during the cycle.  Therefore 
>> >> > the
>> >> > most straightforward way to do this safely is if the device and all its
>> >> > descendants can remain runtime suspended until the resume stage of 
>> >> > system
>> >> > resume.
>> >> >
>> >> > To this end, introduce dev->power.leave_runtime_suspended.
>> >> > If a subsystem or driver sets this flag during the ->prepare() callback,
>> >> > and if the flag is set in all of the device's descendants, and if the
>> >> > device is still in runtime suspend at the beginning of the ->suspend()
>> >> > callback, that callback is allowed to return 0 without clearing
>> >> > power.leave_runtime_suspended and without changing the state of the
>> >> > device, unless the current state of the device is not appropriate for
>> >> > the upcoming system sleep state (for example, the device is supposed to
>> >> > wake up the system from that state and its current wakeup settings are
>> >> > not suitable for that).  Then, the PM core will not invoke the device's
>> >> > ->suspend_late(), ->suspend_irq(), ->resume_irq(), ->resume_early(), or
>> >> > ->resume() callbacks.  Instead, it will invoke ->runtime_resume() during
>> >> > the device resume stage of system resume.
>> >> >
>> >> > By leaving this flag set after ->suspend(), a driver or subsystem tells
>> >> > the PM core that the device is runtime suspended, it is in a suitable
>> >> > state for system suspend (for example, the wakeup setting does not
>> >> > need to be changed), and it does not need to return to full
>> >> > power until the resume stage.
>> >> >
>> >> > Changelog based on an Alan Stern's description of the idea
>> >> > (http://marc.info/?l=linux-pm&m=139940466625569&w=2).
>> >> >
>> >> > Signed-off-by: Rafael J. Wysocki 
>> >> > ---
>> >> >  drivers/base/power/main.c|   31 ---
>> >> >  drivers/base/power/runtime.c |   10 ++
>> >> >  include/linux/pm.h   |3 +++
>> >> >  include/linux/pm_runtime.h   |   16 
>> >> >  kernel/power/Kconfig |4 
>> >> >  5 files changed, 57 insertions(+), 7 deletions(-)
>> >> >
>> >> > Index: linux-pm/kernel/power/Kconfig
>> >> > ===
>> >> > --- linux-pm.orig/kernel/power/Kconfig
>> >> > +++ linux-pm/kernel/power/Kconfig
>> >> > @@ -147,6 +147,10 @@ config PM
>> >> > def_bool y
>> >> > depends on PM_SLEEP || PM_RUNTIME
>> >> >
>> >> > +config PM_BOTH
>> >> > +   def_bool y
>> >> > +   depends on PM_SLEEP && PM_RUNTIME
>> >> > +
>> >>
>> >> Should we not depend on PM_RUNTIME only? Thus we don't need the new
>> >> Kconfig,
>> >
>> > Well, OK.  I guess we can tolerate one useless statement in rpm_resume()
>> > in case CONFIG_PM_SLEEP is unset.
>> >
>> >> and then we could rename the new APIs to pm_runtime_* instead.
>> >
>> > That would just make the name longer - for what value?
>>
>> Only "__set_leave_runtime_suspended" will be a bit longer.
>>
>> The idea I had was to clearly indicate, these functions is a part of
>> PM_RUNTIME API.
>>
>> Compare what you have:
>> __set_leave_runtime_suspended
>> pm_set_leave_runtime_suspended
>> pm_leave_runtime_suspended
>>
>> To what I suggest:
>> __pm_runtime_set_leave_suspended
>> pm_runtime_set_leave_suspended
>> pm_runtime_leave_suspended
>
> And why exactly do you think these are any better?

Because that's how all (almost all) other functions in the runtime PM
API are specified - I believe it makes sense to keep them aligned.

Anyway, if you insist in keeping your functions names, it's not that
of a big deal for me.

Reviewed-by: Ulf Hansson 

>
> The flag is not called leave_suspended surely?

To me that doesn't matter, the flag has nothing to do with the
function names in an API.

>
>
> --
> I speak only for myself.
> Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list

Re: [RFC][PATCH 1/3] PM / sleep: Flag to speed up suspend-resume of runtime-suspended devices

2014-05-08 Thread Rafael J. Wysocki
On Thursday, May 08, 2014 12:59:20 PM Ulf Hansson wrote:
> On 8 May 2014 12:53, Rafael J. Wysocki  wrote:
> > On Thursday, May 08, 2014 09:49:36 AM Ulf Hansson wrote:
> >> On 8 May 2014 01:29, Rafael J. Wysocki  wrote:
> >> > From: Rafael J. Wysocki 
> >> >
> >> > Currently, some subsystems (e.g. PCI and the ACPI PM domain) have to
> >> > resume all runtime-suspended devices during system suspend, mostly
> >> > because those devices may need to be reprogrammed due to different
> >> > wakeup settings for system sleep and for runtime PM.
> >> >
> >> > For some devices, though, it's OK to remain in runtime suspend
> >> > throughout a complete system suspend/resume cycle (if the device was in
> >> > runtime suspend at the start of the cycle).  We would like to do this
> >> > whenever possible, to avoid the overhead of extra power-up and power-down
> >> > events.
> >> >
> >> > However, problems may arise because the device's descendants may require
> >> > it to be at full power at various points during the cycle.  Therefore the
> >> > most straightforward way to do this safely is if the device and all its
> >> > descendants can remain runtime suspended until the resume stage of system
> >> > resume.
> >> >
> >> > To this end, introduce dev->power.leave_runtime_suspended.
> >> > If a subsystem or driver sets this flag during the ->prepare() callback,
> >> > and if the flag is set in all of the device's descendants, and if the
> >> > device is still in runtime suspend at the beginning of the ->suspend()
> >> > callback, that callback is allowed to return 0 without clearing
> >> > power.leave_runtime_suspended and without changing the state of the
> >> > device, unless the current state of the device is not appropriate for
> >> > the upcoming system sleep state (for example, the device is supposed to
> >> > wake up the system from that state and its current wakeup settings are
> >> > not suitable for that).  Then, the PM core will not invoke the device's
> >> > ->suspend_late(), ->suspend_irq(), ->resume_irq(), ->resume_early(), or
> >> > ->resume() callbacks.  Instead, it will invoke ->runtime_resume() during
> >> > the device resume stage of system resume.
> >> >
> >> > By leaving this flag set after ->suspend(), a driver or subsystem tells
> >> > the PM core that the device is runtime suspended, it is in a suitable
> >> > state for system suspend (for example, the wakeup setting does not
> >> > need to be changed), and it does not need to return to full
> >> > power until the resume stage.
> >> >
> >> > Changelog based on an Alan Stern's description of the idea
> >> > (http://marc.info/?l=linux-pm&m=139940466625569&w=2).
> >> >
> >> > Signed-off-by: Rafael J. Wysocki 
> >> > ---
> >> >  drivers/base/power/main.c|   31 ---
> >> >  drivers/base/power/runtime.c |   10 ++
> >> >  include/linux/pm.h   |3 +++
> >> >  include/linux/pm_runtime.h   |   16 
> >> >  kernel/power/Kconfig |4 
> >> >  5 files changed, 57 insertions(+), 7 deletions(-)
> >> >
> >> > Index: linux-pm/kernel/power/Kconfig
> >> > ===
> >> > --- linux-pm.orig/kernel/power/Kconfig
> >> > +++ linux-pm/kernel/power/Kconfig
> >> > @@ -147,6 +147,10 @@ config PM
> >> > def_bool y
> >> > depends on PM_SLEEP || PM_RUNTIME
> >> >
> >> > +config PM_BOTH
> >> > +   def_bool y
> >> > +   depends on PM_SLEEP && PM_RUNTIME
> >> > +
> >>
> >> Should we not depend on PM_RUNTIME only? Thus we don't need the new
> >> Kconfig,
> >
> > Well, OK.  I guess we can tolerate one useless statement in rpm_resume()
> > in case CONFIG_PM_SLEEP is unset.
> >
> >> and then we could rename the new APIs to pm_runtime_* instead.
> >
> > That would just make the name longer - for what value?
> 
> Only "__set_leave_runtime_suspended" will be a bit longer.
> 
> The idea I had was to clearly indicate, these functions is a part of
> PM_RUNTIME API.
> 
> Compare what you have:
> __set_leave_runtime_suspended
> pm_set_leave_runtime_suspended
> pm_leave_runtime_suspended
> 
> To what I suggest:
> __pm_runtime_set_leave_suspended
> pm_runtime_set_leave_suspended
> pm_runtime_leave_suspended

And why exactly do you think these are any better?

The flag is not called leave_suspended surely?


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC][PATCH 1/3] PM / sleep: Flag to speed up suspend-resume of runtime-suspended devices

2014-05-08 Thread Ulf Hansson
On 8 May 2014 12:53, Rafael J. Wysocki  wrote:
> On Thursday, May 08, 2014 09:49:36 AM Ulf Hansson wrote:
>> On 8 May 2014 01:29, Rafael J. Wysocki  wrote:
>> > From: Rafael J. Wysocki 
>> >
>> > Currently, some subsystems (e.g. PCI and the ACPI PM domain) have to
>> > resume all runtime-suspended devices during system suspend, mostly
>> > because those devices may need to be reprogrammed due to different
>> > wakeup settings for system sleep and for runtime PM.
>> >
>> > For some devices, though, it's OK to remain in runtime suspend
>> > throughout a complete system suspend/resume cycle (if the device was in
>> > runtime suspend at the start of the cycle).  We would like to do this
>> > whenever possible, to avoid the overhead of extra power-up and power-down
>> > events.
>> >
>> > However, problems may arise because the device's descendants may require
>> > it to be at full power at various points during the cycle.  Therefore the
>> > most straightforward way to do this safely is if the device and all its
>> > descendants can remain runtime suspended until the resume stage of system
>> > resume.
>> >
>> > To this end, introduce dev->power.leave_runtime_suspended.
>> > If a subsystem or driver sets this flag during the ->prepare() callback,
>> > and if the flag is set in all of the device's descendants, and if the
>> > device is still in runtime suspend at the beginning of the ->suspend()
>> > callback, that callback is allowed to return 0 without clearing
>> > power.leave_runtime_suspended and without changing the state of the
>> > device, unless the current state of the device is not appropriate for
>> > the upcoming system sleep state (for example, the device is supposed to
>> > wake up the system from that state and its current wakeup settings are
>> > not suitable for that).  Then, the PM core will not invoke the device's
>> > ->suspend_late(), ->suspend_irq(), ->resume_irq(), ->resume_early(), or
>> > ->resume() callbacks.  Instead, it will invoke ->runtime_resume() during
>> > the device resume stage of system resume.
>> >
>> > By leaving this flag set after ->suspend(), a driver or subsystem tells
>> > the PM core that the device is runtime suspended, it is in a suitable
>> > state for system suspend (for example, the wakeup setting does not
>> > need to be changed), and it does not need to return to full
>> > power until the resume stage.
>> >
>> > Changelog based on an Alan Stern's description of the idea
>> > (http://marc.info/?l=linux-pm&m=139940466625569&w=2).
>> >
>> > Signed-off-by: Rafael J. Wysocki 
>> > ---
>> >  drivers/base/power/main.c|   31 ---
>> >  drivers/base/power/runtime.c |   10 ++
>> >  include/linux/pm.h   |3 +++
>> >  include/linux/pm_runtime.h   |   16 
>> >  kernel/power/Kconfig |4 
>> >  5 files changed, 57 insertions(+), 7 deletions(-)
>> >
>> > Index: linux-pm/kernel/power/Kconfig
>> > ===
>> > --- linux-pm.orig/kernel/power/Kconfig
>> > +++ linux-pm/kernel/power/Kconfig
>> > @@ -147,6 +147,10 @@ config PM
>> > def_bool y
>> > depends on PM_SLEEP || PM_RUNTIME
>> >
>> > +config PM_BOTH
>> > +   def_bool y
>> > +   depends on PM_SLEEP && PM_RUNTIME
>> > +
>>
>> Should we not depend on PM_RUNTIME only? Thus we don't need the new
>> Kconfig,
>
> Well, OK.  I guess we can tolerate one useless statement in rpm_resume()
> in case CONFIG_PM_SLEEP is unset.
>
>> and then we could rename the new APIs to pm_runtime_* instead.
>
> That would just make the name longer - for what value?

Only "__set_leave_runtime_suspended" will be a bit longer.

The idea I had was to clearly indicate, these functions is a part of
PM_RUNTIME API.

Compare what you have:
__set_leave_runtime_suspended
pm_set_leave_runtime_suspended
pm_leave_runtime_suspended

To what I suggest:
__pm_runtime_set_leave_suspended
pm_runtime_set_leave_suspended
pm_runtime_leave_suspended

Kind regards
Ulf Hansson
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC][PATCH 1/3] PM / sleep: Flag to speed up suspend-resume of runtime-suspended devices

2014-05-08 Thread Rafael J. Wysocki
On Thursday, May 08, 2014 09:49:36 AM Ulf Hansson wrote:
> On 8 May 2014 01:29, Rafael J. Wysocki  wrote:
> > From: Rafael J. Wysocki 
> >
> > Currently, some subsystems (e.g. PCI and the ACPI PM domain) have to
> > resume all runtime-suspended devices during system suspend, mostly
> > because those devices may need to be reprogrammed due to different
> > wakeup settings for system sleep and for runtime PM.
> >
> > For some devices, though, it's OK to remain in runtime suspend
> > throughout a complete system suspend/resume cycle (if the device was in
> > runtime suspend at the start of the cycle).  We would like to do this
> > whenever possible, to avoid the overhead of extra power-up and power-down
> > events.
> >
> > However, problems may arise because the device's descendants may require
> > it to be at full power at various points during the cycle.  Therefore the
> > most straightforward way to do this safely is if the device and all its
> > descendants can remain runtime suspended until the resume stage of system
> > resume.
> >
> > To this end, introduce dev->power.leave_runtime_suspended.
> > If a subsystem or driver sets this flag during the ->prepare() callback,
> > and if the flag is set in all of the device's descendants, and if the
> > device is still in runtime suspend at the beginning of the ->suspend()
> > callback, that callback is allowed to return 0 without clearing
> > power.leave_runtime_suspended and without changing the state of the
> > device, unless the current state of the device is not appropriate for
> > the upcoming system sleep state (for example, the device is supposed to
> > wake up the system from that state and its current wakeup settings are
> > not suitable for that).  Then, the PM core will not invoke the device's
> > ->suspend_late(), ->suspend_irq(), ->resume_irq(), ->resume_early(), or
> > ->resume() callbacks.  Instead, it will invoke ->runtime_resume() during
> > the device resume stage of system resume.
> >
> > By leaving this flag set after ->suspend(), a driver or subsystem tells
> > the PM core that the device is runtime suspended, it is in a suitable
> > state for system suspend (for example, the wakeup setting does not
> > need to be changed), and it does not need to return to full
> > power until the resume stage.
> >
> > Changelog based on an Alan Stern's description of the idea
> > (http://marc.info/?l=linux-pm&m=139940466625569&w=2).
> >
> > Signed-off-by: Rafael J. Wysocki 
> > ---
> >  drivers/base/power/main.c|   31 ---
> >  drivers/base/power/runtime.c |   10 ++
> >  include/linux/pm.h   |3 +++
> >  include/linux/pm_runtime.h   |   16 
> >  kernel/power/Kconfig |4 
> >  5 files changed, 57 insertions(+), 7 deletions(-)
> >
> > Index: linux-pm/kernel/power/Kconfig
> > ===
> > --- linux-pm.orig/kernel/power/Kconfig
> > +++ linux-pm/kernel/power/Kconfig
> > @@ -147,6 +147,10 @@ config PM
> > def_bool y
> > depends on PM_SLEEP || PM_RUNTIME
> >
> > +config PM_BOTH
> > +   def_bool y
> > +   depends on PM_SLEEP && PM_RUNTIME
> > +
> 
> Should we not depend on PM_RUNTIME only? Thus we don't need the new
> Kconfig,

Well, OK.  I guess we can tolerate one useless statement in rpm_resume()
in case CONFIG_PM_SLEEP is unset.

> and then we could rename the new APIs to pm_runtime_* instead.

That would just make the name longer - for what value?

> >  config PM_DEBUG
> > bool "Power Management Debug Support"
> > depends on PM
> > Index: linux-pm/include/linux/pm.h
> > ===
> > --- linux-pm.orig/include/linux/pm.h
> > +++ linux-pm/include/linux/pm.h
> > @@ -583,6 +583,9 @@ struct dev_pm_info {
> > unsigned long   suspended_jiffies;
> > unsigned long   accounting_timestamp;
> >  #endif
> > +#ifdef CONFIG_PM_BOTH
> > +   boolleave_runtime_suspended:1;
> > +#endif
> > struct pm_subsys_data   *subsys_data;  /* Owned by the subsystem. */
> > void (*set_latency_tolerance)(struct device *, s32);
> > struct dev_pm_qos   *qos;
> > Index: linux-pm/include/linux/pm_runtime.h
> > ===
> > --- linux-pm.orig/include/linux/pm_runtime.h
> > +++ linux-pm/include/linux/pm_runtime.h
> > @@ -264,4 +264,20 @@ static inline void pm_runtime_dont_use_a
> > __pm_runtime_use_autosuspend(dev, false);
> >  }
> >
> > +#ifdef CONFIG_PM_BOTH
> > +static inline void __set_leave_runtime_suspended(struct device *dev, bool 
> > val)
> > +{
> > +   dev->power.leave_runtime_suspended = val;
> > +}
> > +extern void pm_set_leave_runtime_suspended(struct device *dev, bool val);
> > +static inline bool pm_leave_runtime_suspended(struct device *dev)
> > +{
> > +   return dev->power.leave_runtime_sus

Re: [RFC][PATCH 1/3] PM / sleep: Flag to speed up suspend-resume of runtime-suspended devices

2014-05-08 Thread Ulf Hansson
On 8 May 2014 01:29, Rafael J. Wysocki  wrote:
> From: Rafael J. Wysocki 
>
> Currently, some subsystems (e.g. PCI and the ACPI PM domain) have to
> resume all runtime-suspended devices during system suspend, mostly
> because those devices may need to be reprogrammed due to different
> wakeup settings for system sleep and for runtime PM.
>
> For some devices, though, it's OK to remain in runtime suspend
> throughout a complete system suspend/resume cycle (if the device was in
> runtime suspend at the start of the cycle).  We would like to do this
> whenever possible, to avoid the overhead of extra power-up and power-down
> events.
>
> However, problems may arise because the device's descendants may require
> it to be at full power at various points during the cycle.  Therefore the
> most straightforward way to do this safely is if the device and all its
> descendants can remain runtime suspended until the resume stage of system
> resume.
>
> To this end, introduce dev->power.leave_runtime_suspended.
> If a subsystem or driver sets this flag during the ->prepare() callback,
> and if the flag is set in all of the device's descendants, and if the
> device is still in runtime suspend at the beginning of the ->suspend()
> callback, that callback is allowed to return 0 without clearing
> power.leave_runtime_suspended and without changing the state of the
> device, unless the current state of the device is not appropriate for
> the upcoming system sleep state (for example, the device is supposed to
> wake up the system from that state and its current wakeup settings are
> not suitable for that).  Then, the PM core will not invoke the device's
> ->suspend_late(), ->suspend_irq(), ->resume_irq(), ->resume_early(), or
> ->resume() callbacks.  Instead, it will invoke ->runtime_resume() during
> the device resume stage of system resume.
>
> By leaving this flag set after ->suspend(), a driver or subsystem tells
> the PM core that the device is runtime suspended, it is in a suitable
> state for system suspend (for example, the wakeup setting does not
> need to be changed), and it does not need to return to full
> power until the resume stage.
>
> Changelog based on an Alan Stern's description of the idea
> (http://marc.info/?l=linux-pm&m=139940466625569&w=2).
>
> Signed-off-by: Rafael J. Wysocki 
> ---
>  drivers/base/power/main.c|   31 ---
>  drivers/base/power/runtime.c |   10 ++
>  include/linux/pm.h   |3 +++
>  include/linux/pm_runtime.h   |   16 
>  kernel/power/Kconfig |4 
>  5 files changed, 57 insertions(+), 7 deletions(-)
>
> Index: linux-pm/kernel/power/Kconfig
> ===
> --- linux-pm.orig/kernel/power/Kconfig
> +++ linux-pm/kernel/power/Kconfig
> @@ -147,6 +147,10 @@ config PM
> def_bool y
> depends on PM_SLEEP || PM_RUNTIME
>
> +config PM_BOTH
> +   def_bool y
> +   depends on PM_SLEEP && PM_RUNTIME
> +

Should we not depend on PM_RUNTIME only? Thus we don't need the new
Kconfig, and then we could rename the new APIs to pm_runtime_*
instead.

>  config PM_DEBUG
> bool "Power Management Debug Support"
> depends on PM
> Index: linux-pm/include/linux/pm.h
> ===
> --- linux-pm.orig/include/linux/pm.h
> +++ linux-pm/include/linux/pm.h
> @@ -583,6 +583,9 @@ struct dev_pm_info {
> unsigned long   suspended_jiffies;
> unsigned long   accounting_timestamp;
>  #endif
> +#ifdef CONFIG_PM_BOTH
> +   boolleave_runtime_suspended:1;
> +#endif
> struct pm_subsys_data   *subsys_data;  /* Owned by the subsystem. */
> void (*set_latency_tolerance)(struct device *, s32);
> struct dev_pm_qos   *qos;
> Index: linux-pm/include/linux/pm_runtime.h
> ===
> --- linux-pm.orig/include/linux/pm_runtime.h
> +++ linux-pm/include/linux/pm_runtime.h
> @@ -264,4 +264,20 @@ static inline void pm_runtime_dont_use_a
> __pm_runtime_use_autosuspend(dev, false);
>  }
>
> +#ifdef CONFIG_PM_BOTH
> +static inline void __set_leave_runtime_suspended(struct device *dev, bool 
> val)
> +{
> +   dev->power.leave_runtime_suspended = val;
> +}
> +extern void pm_set_leave_runtime_suspended(struct device *dev, bool val);
> +static inline bool pm_leave_runtime_suspended(struct device *dev)
> +{
> +   return dev->power.leave_runtime_suspended;
> +}
> +#else
> +static inline void __set_leave_runtime_suspended(struct device *dev, bool 
> val) {}
> +static inline void pm_set_leave_runtime_suspended(struct device *dev, bool 
> val) {}
> +static inline bool pm_leave_runtime_suspended(struct device *dev) { return 
> false; }
> +#endif
> +
>  #endif
> Index: linux-pm/drivers/base/power/runtime.c
> ==