Re: [PATCH 0/8] Suspend block api (version 6)

Re: [PATCH 0/8] Suspend block api (version 6)

[Florian Mickler]

On Thu, 27 May 2010 09:46:51 +0200
Linus WALLEIJ <linus.walleij@stericsson.com> wrote:


> If yes then OK, it's not totally elegant but if that is
> where we have to go, I can live with it. There will likely
> be people who implement for only one or the other semantic
> behaviour, but we have worse things to cope with already.

Alan Cox suggested, that this kind of explicit requirement definition
might be necessary for all drivers anyway in the long run.

That way, the semantic differences between those two cases would
vanish. 

Cheers,
Flo

[PATCH 0/8] Suspend block api (version 6)

[Arve Hjønnevåg]

This patch series adds a suspend-block api that provides the same
functionality as the android wakelock api. This version fixes a race
in suspend blocking work, has some documentation changes and
opportunistic suspend now uses the same workqueue as runtime pm.

--
Arve Hjønnevåg <arve@android.com>

Re: [PATCH 0/8] Suspend block api (version 6)

[Kevin Hilman]

Arve Hjønnevåg <arve@android.com> writes:

> This patch series adds a suspend-block api that provides the same
> functionality as the android wakelock api. This version fixes a race
> in suspend blocking work, has some documentation changes and
> opportunistic suspend now uses the same workqueue as runtime pm.

Earlier this month, several folks intersted in embedded PM had a BoF
as part of the Embedded Linux Conference[1] in San Francisco.  Many of
us had concerns about wakelocks/suspend-blockers and I wanted to share
some of mine here, since I don't know if embedded folks (other than
Google) were included in discussions during the LF Collab summmit.

I hope other embedded folks will chime in here as well.  My background
is in embedded as one of the kernel developers on the TI OMAP SoCs,
and I work primarily on PM stuff.

My comments are not about this implementation of suspend blockers in
particular, but rather on the potential implications of suspend
blockers in general.  

Sorry for the lengthy mail, it's broken up in to 3 parts:

- suspend blockers vs. runtime PM
- how to handle PM aware drivers?
- what about dumb or untrusted apps


Suspend blockers vs runtime PM
------------------------------

My primary concern is that suspend blockers attempt to address the
same problem(s) as runtime PM, but with a very different approach.
Suspend blockers use one very large hammer whereas runtime PM hands
out many little hammers.  Since I believe power management to be a
problem of many little nails, I think many little hammers are better
suited for the job.

Currently in the kernel, we have two main forms of PM

- static PM (system PM, traditional suspend/resume etc.)
- dynamic PM (runtime PM, CPUfreq, CPUidle, etc.)

And with the addition of suspend blockers we have something in
between.  In my simple world, I think of suspend_blockers as static PM
with a retrofit of some basic dynamic capabilities.  In my view, a
poor man's dynamic PM.

The current design of suspend blockers was (presumably) taken due to
major limitations and/or problems in dynamic PM when it was designed.
However, since then, some very signifcant improvements in dynamic PM
have come along, particularily in the form of runtime PM.  What I
still feel is missing from this discussion are details about why the
issues addressed by suspend blockers cannot be solved with runtime PM.

It seems to me the keypad/screen example given in the doc can very
easily be solved with runtime PM.  The goal of that example is that
the keypad not turn on the screen unless a specific key is pressed.
That is rather easy to accomplish using runtime PM:
 
1. system is idle, all devices/drivers runtime suspended
   (display and keypad drivers are both runtime suspended)
-  keypress triggers wakeup ->runtime_resume() of keypad (screen is
   still runtime suspended)
-  key press trickles up to userspace
-  keypad driver is done, goes idle and is runtime supended
-  userspace decides whether or not to turn on screen based on key
-  if not, goto 1, (display is still runtime suspended)
-  if so, start using display and it will be runtime resumed

I realize this keypad example was only one example usage of suspend
blockers, but I suspect the others would be solved similarily using
runtime PM.

But anyways, to get back to the main point: 

I feel the main problems tackled by _kernel_ suspend blockers (as I
understand them) are the same problems already addressed by runtime
PM.  First and formost, both have the same guiding principle:

  Rule #1: Always try for lowest power state, unless X

For runtime PM, X = "activity"
For suspend blockers, X = a held suspend_blocker

In addition, both have the same secondary goals:

- keep device PM independent of other devices (e.g. don't wake up
  screen just because keypad was pressed)

- wakeups/events can be handled in a device specific way, without
  affecting other devices or rest of the system, unless desired

So, the goals are the same, but the approaches are different.  Runtime
PM makes each of the drivers and subsystems do the work, where suspend
blockers just forces the issue from on high.  IMHO, the more flexible
and generic approach of runtime PM is more suited to a general purpose
kernel than the one-big-hammer approach currently taken by suspend
blockers.


What about PM aware drivers?
----------------------------

All of this brings up a second major concern regarding how to write PM
aware drivers.

At least from the kernel perspective, both suspend blockers and
runtime PM have the same goal.  Given that, which framework should the
driver writer target?  Both?  Seems like duplicate effort.  Using
suspend blockers assumes the system is in opportunitstic suspend mode
and (at least in the keypad example given) assumes a suspend-blocker
aware userspace (Android.) Without both, targeted power savings will
not be acheived.

To me, runtime PM is a generic and flexible approach that can be used
with any userspace.  Driver writers should not have to care whether
the system is in "opportunistic" mode or about whether userspace is
suspend blocker capable.  They should only have to think about when
the device is (or should be) idle.

>From my experience with OMAP, we *really* do not want to care about
what userspace is or isn't capable of, or what suspend-mode the kernel
is in.  Among other things, the OMAP linux kernel is used in the Nokia
N900 (Maemo), the Motorola Droid (Android) and the Palm Pre (webOS).
Comments on the future of each SW stack aside, we really want to run
the same kernel and drivers across all of those platforms as well as
whatever comes next.


What about dumb or untrusted apps?
---------------------------------------

In my view, the truly significant difference between suspend blockers
and runtime PM is what happens to userspace.  So far, to me the only
compelling argument for suspend blockers is the goal of forcibly
shutting down userspace and thus forcing the system into idle
(although drivers could still reject a suspend request.)

Again, since suspend blockers were designed, there have been major
efforts to track and fix userspace as well as underlying timer issues
(deferrable timers, coalescing, timer slack ...) that led to
unnecessary wakeups from userspace.  Wouldn't it be better to spend
our collective efforts in continuing in that direction instead of just
hiding the underlying problems by forcing suspend?  Fixing the root
causes will be better for everyone, not just those using Android.

And if untrusted userspace apps remain as the major problem, maybe we
should aim for a solution directly targetting that problem.  I'm just
shooting from the hip now, but maybe containing (cgroups?) untrusted
processes together into a set that could be frozen/idled so that runtime PM
would be more effective would be a workable solution?

Anyways, that's enough rambling for now.  I hope that sheds some light
on the concerns I have with suspend blockers.

Kevin

[1] http://embeddedlinuxconference.com/elc_2010/index.html

Re: [PATCH 0/8] Suspend block api (version 6)

[Alan Stern]

On Mon, 3 May 2010, Kevin Hilman wrote:

> Suspend blockers vs runtime PM
> ------------------------------
> 
> My primary concern is that suspend blockers attempt to address the
> same problem(s) as runtime PM, but with a very different approach.
> Suspend blockers use one very large hammer whereas runtime PM hands
> out many little hammers.  Since I believe power management to be a
> problem of many little nails, I think many little hammers are better
> suited for the job.

There is a major difference between suspend blockers and runtime PM 
which was not discussed in your email.  Namely: Runtime PM is aimed at 
suspending individual devices and not the system as a whole (in 
particular, not CPUs), whereas suspend blockers are aimed at suspending 
-- or more accurately, blocking suspends -- of the system as a whole 
and not individual devices.

So for example, runtime PM cannot be used to put the system into an S3 
sleep state.  But suspend blockers _are_ used to keep the system from 
going into S3.

> Currently in the kernel, we have two main forms of PM
> 
> - static PM (system PM, traditional suspend/resume etc.)
> - dynamic PM (runtime PM, CPUfreq, CPUidle, etc.)
> 
> And with the addition of suspend blockers we have something in
> between.  In my simple world, I think of suspend_blockers as static PM
> with a retrofit of some basic dynamic capabilities.  In my view, a
> poor man's dynamic PM.

I wouldn't describe it like that.  Consider dividing PM along a 
different axis:

- entire system (ACPI sleep states, CPUs turned off, etc.)
- single, independent devices (e.g., USB autosuspend)

Then system PM combines static + entire, whereas runtime PM combines 
dynamic + single.  By contrast, suspend blockers are aimed at the 
dynamic + entire combination.  This makes them different from anything 
already present in the kernel.

> The current design of suspend blockers was (presumably) taken due to
> major limitations and/or problems in dynamic PM when it was designed.
> However, since then, some very signifcant improvements in dynamic PM
> have come along, particularily in the form of runtime PM.  What I
> still feel is missing from this discussion are details about why the
> issues addressed by suspend blockers cannot be solved with runtime PM.

The simplest example is that suspend blockers can be used to control 
whether or not the CPU shuts down, whereas runtime PM can't.

> To me, runtime PM is a generic and flexible approach that can be used
> with any userspace.  Driver writers should not have to care whether
> the system is in "opportunistic" mode or about whether userspace is
> suspend blocker capable.  They should only have to think about when
> the device is (or should be) idle.
> 
> From my experience with OMAP, we *really* do not want to care about
> what userspace is or isn't capable of, or what suspend-mode the kernel
> is in.  Among other things, the OMAP linux kernel is used in the Nokia
> N900 (Maemo), the Motorola Droid (Android) and the Palm Pre (webOS).
> Comments on the future of each SW stack aside, we really want to run
> the same kernel and drivers across all of those platforms as well as
> whatever comes next.

That is indeed a weak point of the proposal.  Kernel drivers' use of
suspend blockers appears to be somewhat arbitrary and directed by the
needs of userspace.  It's not at all clear how drivers can use suspend
blockers in a way that will work on all systems.

Alan Stern

Re: [PATCH 0/8] Suspend block api (version 6)

[Kevin Hilman]

Alan Stern <stern@rowland.harvard.edu> writes:

> On Mon, 3 May 2010, Kevin Hilman wrote:
>
>> Suspend blockers vs runtime PM
>> ------------------------------
>> 
>> My primary concern is that suspend blockers attempt to address the
>> same problem(s) as runtime PM, but with a very different approach.
>> Suspend blockers use one very large hammer whereas runtime PM hands
>> out many little hammers.  Since I believe power management to be a
>> problem of many little nails, I think many little hammers are better
>> suited for the job.
>
> There is a major difference between suspend blockers and runtime PM 
> which was not discussed in your email.  Namely: Runtime PM is aimed at 
> suspending individual devices and not the system as a whole (in 
> particular, not CPUs), whereas suspend blockers are aimed at suspending 
> -- or more accurately, blocking suspends -- of the system as a whole 
> and not individual devices.
>
> So for example, runtime PM cannot be used to put the system into an S3 
> sleep state.  But suspend blockers _are_ used to keep the system from 
> going into S3.

Good point.  However, adding CPUidle to runtime PM you indeed achieve
a dynamic PM that affects the whole system. 

The main difference then comes down to what prevents system-wide
low-power state: activity vs. suspend blocker held.

>> Currently in the kernel, we have two main forms of PM
>> 
>> - static PM (system PM, traditional suspend/resume etc.)
>> - dynamic PM (runtime PM, CPUfreq, CPUidle, etc.)
>> 
>> And with the addition of suspend blockers we have something in
>> between.  In my simple world, I think of suspend_blockers as static PM
>> with a retrofit of some basic dynamic capabilities.  In my view, a
>> poor man's dynamic PM.
>
> I wouldn't describe it like that.  Consider dividing PM along a 
> different axis:
>
> - entire system (ACPI sleep states, CPUs turned off, etc.)
> - single, independent devices (e.g., USB autosuspend)
>
> Then system PM combines static + entire, whereas runtime PM combines 
> dynamic + single.  By contrast, suspend blockers are aimed at the 
> dynamic + entire combination.  This makes them different from anything 
> already present in the kernel.

Adding CPUidle to runtime PM (dynamic + single), you add 'entire' as
well, as CPUidle manages the CPU states.

However, one problem I have with your proposed division is it doesn't
seem to match well to the hardware available on modern embedded SoCs,
mainly because these are not ACPI devices.  The CPU is just another
"independent" device whose runtime PM is managed by CPUidle.

IOW, The 'entire system' state is just a function of the CPU state
and the state of all the on- and off-chip devices.  With CPUidle
managing the CPUs, and runtime PM managing the rest of the devices,
you have a fully flexible dynamic PM for the entire system.

>> The current design of suspend blockers was (presumably) taken due to
>> major limitations and/or problems in dynamic PM when it was designed.
>> However, since then, some very signifcant improvements in dynamic PM
>> have come along, particularily in the form of runtime PM.  What I
>> still feel is missing from this discussion are details about why the
>> issues addressed by suspend blockers cannot be solved with runtime PM.
>
> The simplest example is that suspend blockers can be used to control 
> whether or not the CPU shuts down, whereas runtime PM can't.

True, by iteslf, runtime PM does not shut down CPUs, but in
combination with CPUidle the CPU(s) can be shut down based on
activity.  In a way, you could think of CPUidle as runtime PM for the
CPU(s).

Again, the primary difference comes down to: why should I not enter a
low-power state?  activity (runtime PM + CPUidle) or suspend blocker.

>> To me, runtime PM is a generic and flexible approach that can be used
>> with any userspace.  Driver writers should not have to care whether
>> the system is in "opportunistic" mode or about whether userspace is
>> suspend blocker capable.  They should only have to think about when
>> the device is (or should be) idle.
>> 
>> From my experience with OMAP, we *really* do not want to care about
>> what userspace is or isn't capable of, or what suspend-mode the kernel
>> is in.  Among other things, the OMAP linux kernel is used in the Nokia
>> N900 (Maemo), the Motorola Droid (Android) and the Palm Pre (webOS).
>> Comments on the future of each SW stack aside, we really want to run
>> the same kernel and drivers across all of those platforms as well as
>> whatever comes next.
>
> That is indeed a weak point of the proposal.  Kernel drivers' use of
> suspend blockers appears to be somewhat arbitrary and directed by the
> needs of userspace.  It's not at all clear how drivers can use suspend
> blockers in a way that will work on all systems.

Nor is it clear (to me) what drivers would/should look like that would
need to do suspend blockers in addition to runtime PM.  

Kevin

Re: [PATCH 0/8] Suspend block api (version 6)

[Mark Brown]

On Mon, May 03, 2010 at 09:40:26AM -0700, Kevin Hilman wrote:

> At least from the kernel perspective, both suspend blockers and
> runtime PM have the same goal.  Given that, which framework should the
> driver writer target?  Both?  Seems like duplicate effort.  Using
> suspend blockers assumes the system is in opportunitstic suspend mode
> and (at least in the keypad example given) assumes a suspend-blocker
> aware userspace (Android.) Without both, targeted power savings will
> not be acheived.

The other concern here is that for many mobile systems the actual
semantic intended by "suspend" as it's currently used is more runtime PM
like than full suspend - the classic example of this is that when
suspending while on a call in a phone you don't want to suspend the
modem or audio CODEC, you want to leave them running.  If you use a full
system suspend then the drivers for affected components have to play
guessing games (or add currently non-standard knobs for apps to twiddle)
to decide if the system intends them to actually implement the suspend
or not but with runtime PM it all falls out very naturally without any
effort on the part of the driver.

> To me, runtime PM is a generic and flexible approach that can be used
> with any userspace.  Driver writers should not have to care whether
> the system is in "opportunistic" mode or about whether userspace is
> suspend blocker capable.  They should only have to think about when
> the device is (or should be) idle.

I fully agree with this.  We do need to ensure that a runtime PM based
system can suspend the CPU core and RAM as well as system suspend can
but that seems doable.

Re: [PATCH 0/8] Suspend block api (version 6)

[Rafael J. Wysocki]

On Monday 03 May 2010, Mark Brown wrote:
> On Mon, May 03, 2010 at 09:40:26AM -0700, Kevin Hilman wrote:
> 
> > At least from the kernel perspective, both suspend blockers and
> > runtime PM have the same goal.  Given that, which framework should the
> > driver writer target?  Both?  Seems like duplicate effort.  Using
> > suspend blockers assumes the system is in opportunitstic suspend mode
> > and (at least in the keypad example given) assumes a suspend-blocker
> > aware userspace (Android.) Without both, targeted power savings will
> > not be acheived.
> 
> The other concern here is that for many mobile systems the actual
> semantic intended by "suspend" as it's currently used is more runtime PM
> like than full suspend - the classic example of this is that when
> suspending while on a call in a phone you don't want to suspend the
> modem or audio CODEC, you want to leave them running.  If you use a full
> system suspend then the drivers for affected components have to play
> guessing games (or add currently non-standard knobs for apps to twiddle)
> to decide if the system intends them to actually implement the suspend
> or not but with runtime PM it all falls out very naturally without any
> effort on the part of the driver.
> 
> > To me, runtime PM is a generic and flexible approach that can be used
> > with any userspace.  Driver writers should not have to care whether
> > the system is in "opportunistic" mode or about whether userspace is
> > suspend blocker capable.  They should only have to think about when
> > the device is (or should be) idle.
> 
> I fully agree with this.  We do need to ensure that a runtime PM based
> system can suspend the CPU core and RAM as well as system suspend can
> but that seems doable.

I _think_ it would be hard at least.  On ACPI-based systems it's not doable at
all AFAICS.

However, the real question is whether or not the opportunistic suspend feature
is worth adding to the kernel as such and I think it is.

To me, it doesn't duplicate the runtime PM framework which is aimed at the power
management of individual devices rather than the system as a whole.

Thanks,
Rafael

Re: [PATCH 0/8] Suspend block api (version 6)

[Kevin Hilman]

"Rafael J. Wysocki" <rjw@sisk.pl> writes:

> On Monday 03 May 2010, Mark Brown wrote:
>> On Mon, May 03, 2010 at 09:40:26AM -0700, Kevin Hilman wrote:
>> 
>> > At least from the kernel perspective, both suspend blockers and
>> > runtime PM have the same goal.  Given that, which framework should the
>> > driver writer target?  Both?  Seems like duplicate effort.  Using
>> > suspend blockers assumes the system is in opportunitstic suspend mode
>> > and (at least in the keypad example given) assumes a suspend-blocker
>> > aware userspace (Android.) Without both, targeted power savings will
>> > not be acheived.
>> 
>> The other concern here is that for many mobile systems the actual
>> semantic intended by "suspend" as it's currently used is more runtime PM
>> like than full suspend - the classic example of this is that when
>> suspending while on a call in a phone you don't want to suspend the
>> modem or audio CODEC, you want to leave them running.  If you use a full
>> system suspend then the drivers for affected components have to play
>> guessing games (or add currently non-standard knobs for apps to twiddle)
>> to decide if the system intends them to actually implement the suspend
>> or not but with runtime PM it all falls out very naturally without any
>> effort on the part of the driver.
>> 
>> > To me, runtime PM is a generic and flexible approach that can be used
>> > with any userspace.  Driver writers should not have to care whether
>> > the system is in "opportunistic" mode or about whether userspace is
>> > suspend blocker capable.  They should only have to think about when
>> > the device is (or should be) idle.
>> 
>> I fully agree with this.  We do need to ensure that a runtime PM based
>> system can suspend the CPU core and RAM as well as system suspend can
>> but that seems doable.
>
> I _think_ it would be hard at least.  On ACPI-based systems it's not doable at
> all AFAICS.

Please forgive the ignorance of ACPI (in embedded, we thankfully live
in magical world without ACPI) but doesn't that already happen with
CPUidle and C-states?  I think of CPUidle as basically runtime PM for
the CPU.  IOW, runtime PM manages the devices, CPUidle manages the CPU
(via C-states), resulting in dynaimc PM for the entire system.  What
am I missing?

> However, the real question is whether or not the opportunistic suspend feature
> is worth adding to the kernel as such and I think it is.
>
> To me, it doesn't duplicate the runtime PM framework which is aimed at the power
> management of individual devices rather than the system as a whole.

>From the use cases presented, the *usage* of suspend blockers is aimed
at power management of individual devices or subsystems, just like
usage of runtime PM.

So I still see a large duplication in the usage and the goals of both
frameworks.  The goal of both is to always enter lowest-power state
except

 - if there's activity (runtime PM for devices, CPUidle for CPU)
 - if there's a suspend blocker (opportunitic suspend)

In addition, it will likely cause duplicate work to be done in
drivers.  Presumably, PM aware drivers will want to know if the system
is in opportunistic mode.  For example, for many drivers, doing
runtime PM may not be worth the effort if the system is in
opportunistic mode.

This last point is especially troubling.  I don't find it a comforting
path to go down if the drivers have to start caring about which PM
policy is currently in use.

Kevin

Re: [PATCH 0/8] Suspend block api (version 6)

[Arve Hjønnevåg]

On Mon, May 3, 2010 at 4:37 PM, Kevin Hilman
<khilman@deeprootsystems.com> wrote:
> "Rafael J. Wysocki" <rjw@sisk.pl> writes:
>
>> On Monday 03 May 2010, Mark Brown wrote:
>>> On Mon, May 03, 2010 at 09:40:26AM -0700, Kevin Hilman wrote:
>>>
>>> > At least from the kernel perspective, both suspend blockers and
>>> > runtime PM have the same goal.  Given that, which framework should the
>>> > driver writer target?  Both?  Seems like duplicate effort.  Using
>>> > suspend blockers assumes the system is in opportunitstic suspend mode
>>> > and (at least in the keypad example given) assumes a suspend-blocker
>>> > aware userspace (Android.) Without both, targeted power savings will
>>> > not be acheived.
>>>
>>> The other concern here is that for many mobile systems the actual
>>> semantic intended by "suspend" as it's currently used is more runtime PM
>>> like than full suspend - the classic example of this is that when
>>> suspending while on a call in a phone you don't want to suspend the
>>> modem or audio CODEC, you want to leave them running.  If you use a full
>>> system suspend then the drivers for affected components have to play
>>> guessing games (or add currently non-standard knobs for apps to twiddle)
>>> to decide if the system intends them to actually implement the suspend
>>> or not but with runtime PM it all falls out very naturally without any
>>> effort on the part of the driver.
>>>
>>> > To me, runtime PM is a generic and flexible approach that can be used
>>> > with any userspace.  Driver writers should not have to care whether
>>> > the system is in "opportunistic" mode or about whether userspace is
>>> > suspend blocker capable.  They should only have to think about when
>>> > the device is (or should be) idle.
>>>
>>> I fully agree with this.  We do need to ensure that a runtime PM based
>>> system can suspend the CPU core and RAM as well as system suspend can
>>> but that seems doable.
>>
>> I _think_ it would be hard at least.  On ACPI-based systems it's not doable at
>> all AFAICS.
>
> Please forgive the ignorance of ACPI (in embedded, we thankfully live
> in magical world without ACPI) but doesn't that already happen with
> CPUidle and C-states?  I think of CPUidle as basically runtime PM for
> the CPU.  IOW, runtime PM manages the devices, CPUidle manages the CPU
> (via C-states), resulting in dynaimc PM for the entire system.  What
> am I missing?
>

I'm not that familiar with ACPI either, but I think the S-states
entered by suspend are much lower power than the C-states entered by
idle.

>> However, the real question is whether or not the opportunistic suspend feature
>> is worth adding to the kernel as such and I think it is.
>>
>> To me, it doesn't duplicate the runtime PM framework which is aimed at the power
>> management of individual devices rather than the system as a whole.
>
> From the use cases presented, the *usage* of suspend blockers is aimed
> at power management of individual devices or subsystems, just like
> usage of runtime PM.
>
No, suspend blockers are mostly used to ensure wakeup events are not
ignored, and to ensure tasks triggered by these wakeup events
complete.

> So I still see a large duplication in the usage and the goals of both
> frameworks.  The goal of both is to always enter lowest-power state
> except
>
>  - if there's activity (runtime PM for devices, CPUidle for CPU)
>  - if there's a suspend blocker (opportunitic suspend)
>
> In addition, it will likely cause duplicate work to be done in
> drivers.  Presumably, PM aware drivers will want to know if the system
> is in opportunistic mode.  For example, for many drivers, doing
> runtime PM may not be worth the effort if the system is in
> opportunistic mode.

Why? If a device is not in use it should be off regardless of what
state the rest of the system is in.

>
> This last point is especially troubling.  I don't find it a comforting
> path to go down if the drivers have to start caring about which PM
> policy is currently in use.
>
> Kevin
>



-- 
Arve Hjønnevåg

Re: [PATCH 0/8] Suspend block api (version 6)

[Brian Swetland]

On Mon, May 3, 2010 at 5:09 PM, Arve Hjønnevåg <arve@android.com> wrote:
> On Mon, May 3, 2010 at 4:37 PM, Kevin Hilman
>>
>> In addition, it will likely cause duplicate work to be done in
>> drivers.  Presumably, PM aware drivers will want to know if the system
>> is in opportunistic mode.  For example, for many drivers, doing
>> runtime PM may not be worth the effort if the system is in
>> opportunistic mode.
>
> Why? If a device is not in use it should be off regardless of what
> state the rest of the system is in.
>
>> This last point is especially troubling.  I don't find it a comforting
>> path to go down if the drivers have to start caring about which PM
>> policy is currently in use.

I'll echo Arve here -- all drivers should seek to be in the lowest
power state possible at all times.  We've never suggested that
suspend_block is a substitute for that.

Suspend blockers give us some flexibility on systems where runtime pm
will not get us all the way there. If you can meet your power needs
without needing suspend blockers, awesome, you don't need them on that
platform.  The patchset Arve sent out makes this feature an
off-by-default kernel configuration option that compiles out to no-ops
when disabled.

In our experience, periodic timers and polling, both userspace side
and kernelside make suspend blockers a win even on platforms like OMAP
which have pretty flexible hardware power management.  Going to low
power states in idle results in higher average power consumption than
going to the same states in full suspend, at least on Android devices
shipping today.

Brian

Re: [PATCH 0/8] Suspend block api (version 6)

[Mark Brown]

On Mon, May 03, 2010 at 05:43:34PM -0700, Brian Swetland wrote:
> On Mon, May 3, 2010 at 5:09 PM, Arve Hjønnevåg <arve@android.com> wrote:
> > On Mon, May 3, 2010 at 4:37 PM, Kevin Hilman

> >> This last point is especially troubling.  I don't find it a comforting
> >> path to go down if the drivers have to start caring about which PM
> >> policy is currently in use.

> I'll echo Arve here -- all drivers should seek to be in the lowest
> power state possible at all times.  We've never suggested that
> suspend_block is a substitute for that.

Looking at this from a subsystem/driver author point of view the problem
I'm faced with is that as a result of using system suspend much more
aggressively the subsystem and driver layers are getting conflicting
instructions about what the lowest power state possible is.

> Suspend blockers give us some flexibility on systems where runtime pm
> will not get us all the way there. If you can meet your power needs
> without needing suspend blockers, awesome, you don't need them on that
> platform.  The patchset Arve sent out makes this feature an
> off-by-default kernel configuration option that compiles out to no-ops
> when disabled.

I think a big part of this for me is that this approach changes the
intended use of the system-wide suspend a bit, complicating it a bit
further than it already is.  We end up doing a suspend (which in the
normal course of affairs means that the driver should shut everything
down) partly as a substitute for runtime PM on the core system devices
and partly because our runtime PM isn't doing as well as it should on
the individual devices.

I'd be a lot more comfortable with this if it came with a mechanism for
communicating the intended effect of a suspend on a per-device level so
that the drivers have a clear way of figuring out what to do when the
system suspends.  If there isn't one we can add subsystem or driver
specific hooks, of course, but it'd be better to address this at the
system level.

> In our experience, periodic timers and polling, both userspace side
> and kernelside make suspend blockers a win even on platforms like OMAP
> which have pretty flexible hardware power management.  Going to low
> power states in idle results in higher average power consumption than
> going to the same states in full suspend, at least on Android devices
> shipping today.

There's definite work to be done here, yes.

Re: [PATCH 0/8] Suspend block api (version 6)

[Kevin Hilman]

Mark Brown <broonie@opensource.wolfsonmicro.com> writes:

> On Mon, May 03, 2010 at 05:43:34PM -0700, Brian Swetland wrote:
>> On Mon, May 3, 2010 at 5:09 PM, Arve Hjønnevåg <arve@android.com> wrote:
>> > On Mon, May 3, 2010 at 4:37 PM, Kevin Hilman
>
>> >> This last point is especially troubling.  I don't find it a comforting
>> >> path to go down if the drivers have to start caring about which PM
>> >> policy is currently in use.
>
>> I'll echo Arve here -- all drivers should seek to be in the lowest
>> power state possible at all times.  We've never suggested that
>> suspend_block is a substitute for that.
>
> Looking at this from a subsystem/driver author point of view the problem
> I'm faced with is that as a result of using system suspend much more
> aggressively the subsystem and driver layers are getting conflicting
> instructions about what the lowest power state possible is.

Exactly.

With runtime PM, there is flexibility in choosing the lowest power
state at the device/subsystem level, based on activity, timeouts,
bitrate, dependencies, latency/throughput constraints, etc.

With opportunistic suspend, all of this flexibility is gone, and the
device/subsystem is told to go into the lowest power, highest latency
state, period.

>> Suspend blockers give us some flexibility on systems where runtime pm
>> will not get us all the way there. If you can meet your power needs
>> without needing suspend blockers, awesome, you don't need them on that
>> platform.  The patchset Arve sent out makes this feature an
>> off-by-default kernel configuration option that compiles out to no-ops
>> when disabled.
>
> I think a big part of this for me is that this approach changes the
> intended use of the system-wide suspend a bit, complicating it a bit
> further than it already is.  We end up doing a suspend (which in the
> normal course of affairs means that the driver should shut everything
> down) partly as a substitute for runtime PM on the core system devices
> and partly because our runtime PM isn't doing as well as it should on
> the individual devices.

Agreed, and because of this, part of my concern is that opportunistic
suspend will take the place of doing the "right thing" which (IMHO) 
would be to improve runtime PM for devices.

> I'd be a lot more comfortable with this if it came with a mechanism for
> communicating the intended effect of a suspend on a per-device level so
> that the drivers have a clear way of figuring out what to do when the
> system suspends.  If there isn't one we can add subsystem or driver
> specific hooks, of course, but it'd be better to address this at the
> system level.

I agree.

I think there needs to be more discussion on the indended usage
of suspend blockers by drivers/subsystems, especially those PM aware
drivers and subsystems already doing runtime PM with constraints etc.

>> In our experience, periodic timers and polling, both userspace side
>> and kernelside make suspend blockers a win even on platforms like OMAP
>> which have pretty flexible hardware power management.  Going to low
>> power states in idle results in higher average power consumption than
>> going to the same states in full suspend, at least on Android devices
>> shipping today.
>
> There's definite work to be done here, yes.

And I've admitted this to be the only compelling reason for
opportunistic suspend so far.

But the driver/subsystem implications of opportunistic suspend still
need some fleshing out IMO.

Kevin

Re: [PATCH 0/8] Suspend block api (version 6)

[Mark Brown]

On Tue, May 04, 2010 at 11:06:39AM -0700, Kevin Hilman wrote:

> With opportunistic suspend, all of this flexibility is gone, and the
> device/subsystem is told to go into the lowest power, highest latency
> state, period.

Well, half the problem I have is that unfortunately it's not a case of
doing that period.  The prime example I'm familiar with is that for
understandable reasons users become irate when you power down the audio
CODEC while they're in the middle of a call so if opportunistic PM is in
use then the audio subsystem needs some additional help interpreting a
suspend request so that it can figure out how to handle it.  Similar
issues apply to PMICs, though less pressingly for various reasons.

Just to be clear, I do understand and mostly agree with the idea that
opportunistic suspend presents a reasonable workaround for our current
inability to deliver good power savings with runtime PM methods on many
important platforms but I do think that if we're going to make this
standard Linux PM functionality then we need to be clearer about how
everything is intended to hang together.

Re: [PATCH 0/8] Suspend block api (version 6)

[Rafael J. Wysocki]

On Tuesday 04 May 2010, Mark Brown wrote:
> On Tue, May 04, 2010 at 11:06:39AM -0700, Kevin Hilman wrote:
> 
> > With opportunistic suspend, all of this flexibility is gone, and the
> > device/subsystem is told to go into the lowest power, highest latency
> > state, period.
> 
> Well, half the problem I have is that unfortunately it's not a case of
> doing that period.  The prime example I'm familiar with is that for
> understandable reasons users become irate when you power down the audio
> CODEC while they're in the middle of a call so if opportunistic PM is in
> use then the audio subsystem needs some additional help interpreting a
> suspend request so that it can figure out how to handle it.  Similar
> issues apply to PMICs, though less pressingly for various reasons.
> 
> Just to be clear, I do understand and mostly agree with the idea that
> opportunistic suspend presents a reasonable workaround for our current
> inability to deliver good power savings with runtime PM methods on many
> important platforms but I do think that if we're going to make this
> standard Linux PM functionality then we need to be clearer about how
> everything is intended to hang together.

At the moment the rule of thumb is: if you don't need the opportunistic
suspend, don't use it.  It is not going to be enabled by default on anything
other than Android right now.

However, since Android is a legitimate user of the Linux kernel, I see no
reason to reject this feature right away.  There are many kernel features
that aren't used by all platforms.

Rafael

Re: [PATCH 0/8] Suspend block api (version 6)

[Kevin Hilman]

"Rafael J. Wysocki" <rjw@sisk.pl> writes:

> On Tuesday 04 May 2010, Mark Brown wrote:
>> On Tue, May 04, 2010 at 11:06:39AM -0700, Kevin Hilman wrote:
>> 
>> > With opportunistic suspend, all of this flexibility is gone, and the
>> > device/subsystem is told to go into the lowest power, highest latency
>> > state, period.
>> 
>> Well, half the problem I have is that unfortunately it's not a case of
>> doing that period.  The prime example I'm familiar with is that for
>> understandable reasons users become irate when you power down the audio
>> CODEC while they're in the middle of a call so if opportunistic PM is in
>> use then the audio subsystem needs some additional help interpreting a
>> suspend request so that it can figure out how to handle it.  Similar
>> issues apply to PMICs, though less pressingly for various reasons.
>> 
>> Just to be clear, I do understand and mostly agree with the idea that
>> opportunistic suspend presents a reasonable workaround for our current
>> inability to deliver good power savings with runtime PM methods on many
>> important platforms but I do think that if we're going to make this
>> standard Linux PM functionality then we need to be clearer about how
>> everything is intended to hang together.
>
> At the moment the rule of thumb is: if you don't need the opportunistic
> suspend, don't use it.  It is not going to be enabled by default on anything
> other than Android right now.

Sure, but there are driver authors and subsystem maintainers who care
about optimal PM in "normal" Linux *and* in Android.

As a PM maintainer for an embedded platform (OMAP) used in both, I
certainly care about both, so "disable it if you don't like it" is not
really an option.

IMO, driver/subsystem authors should not have to care if the userspace
is Android or not.  We should be working towards a world where Android
is not a special case.

Kevin

Re: [PATCH 0/8] Suspend block api (version 6)

[Rafael J. Wysocki]

On Wednesday 05 May 2010, Kevin Hilman wrote:
> "Rafael J. Wysocki" <rjw@sisk.pl> writes:
> 
> > On Tuesday 04 May 2010, Mark Brown wrote:
> >> On Tue, May 04, 2010 at 11:06:39AM -0700, Kevin Hilman wrote:
> >> 
> >> > With opportunistic suspend, all of this flexibility is gone, and the
> >> > device/subsystem is told to go into the lowest power, highest latency
> >> > state, period.
> >> 
> >> Well, half the problem I have is that unfortunately it's not a case of
> >> doing that period.  The prime example I'm familiar with is that for
> >> understandable reasons users become irate when you power down the audio
> >> CODEC while they're in the middle of a call so if opportunistic PM is in
> >> use then the audio subsystem needs some additional help interpreting a
> >> suspend request so that it can figure out how to handle it.  Similar
> >> issues apply to PMICs, though less pressingly for various reasons.
> >> 
> >> Just to be clear, I do understand and mostly agree with the idea that
> >> opportunistic suspend presents a reasonable workaround for our current
> >> inability to deliver good power savings with runtime PM methods on many
> >> important platforms but I do think that if we're going to make this
> >> standard Linux PM functionality then we need to be clearer about how
> >> everything is intended to hang together.
> >
> > At the moment the rule of thumb is: if you don't need the opportunistic
> > suspend, don't use it.  It is not going to be enabled by default on anything
> > other than Android right now.
> 
> Sure, but there are driver authors and subsystem maintainers who care
> about optimal PM in "normal" Linux *and* in Android.
> 
> As a PM maintainer for an embedded platform (OMAP) used in both, I
> certainly care about both, so "disable it if you don't like it" is not
> really an option.
> 
> IMO, driver/subsystem authors should not have to care if the userspace
> is Android or not.  We should be working towards a world where Android
> is not a special case.

I agree, but "working on towards a world where ..." need not mean "be there
from day one" IMO.  Also, I guess you'll agree that using the same _binary_
kernel with Android and non-Android user spaces doesn't necessarily make sense
(regardless of the fact that Android kernels have hardware-specific board files
included), so probably you'll still disable opportunistic suspend building the
kernel for non-Android systems (at least for the time being).

It looks like you're thinking the opportunistic suspend somehow is (or can be
used as) a replacement for runtime PM, but this is not the case.  The reason
why it's not the case is that runtime PM works with the assumption that user
space is not frozen and it works on individual devices.

OTOH, generally, there is a limit on the amount of energy savings you can
achieve with runtime PM alone and something like the opportunistic suspend is
necessary to go beyond that limit.

Now, I can easily imagine using suspend blockers and runtime PM in the same
driver with the general rule that you'll probably want to call suspend_unblock()
whenever the device is suspended with the help of the runtime PM framework.
That really depends on the device and the driver in question, though.

Rafael

Re: [PATCH 0/8] Suspend block api (version 6)

[Rafael J. Wysocki]

On Tuesday 04 May 2010, Kevin Hilman wrote:
> Mark Brown <broonie@opensource.wolfsonmicro.com> writes:
> 
> > On Mon, May 03, 2010 at 05:43:34PM -0700, Brian Swetland wrote:
> >> On Mon, May 3, 2010 at 5:09 PM, Arve Hjønnevåg <arve@android.com> wrote:
> >> > On Mon, May 3, 2010 at 4:37 PM, Kevin Hilman
> >
> >> >> This last point is especially troubling.  I don't find it a comforting
> >> >> path to go down if the drivers have to start caring about which PM
> >> >> policy is currently in use.
> >
> >> I'll echo Arve here -- all drivers should seek to be in the lowest
> >> power state possible at all times.  We've never suggested that
> >> suspend_block is a substitute for that.
> >
> > Looking at this from a subsystem/driver author point of view the problem
> > I'm faced with is that as a result of using system suspend much more
> > aggressively the subsystem and driver layers are getting conflicting
> > instructions about what the lowest power state possible is.
> 
> Exactly.
> 
> With runtime PM, there is flexibility in choosing the lowest power
> state at the device/subsystem level, based on activity, timeouts,
> bitrate, dependencies, latency/throughput constraints, etc.
> 
> With opportunistic suspend, all of this flexibility is gone, and the
> device/subsystem is told to go into the lowest power, highest latency
> state, period.

Guys, please.

The opportunistic suspend feature is _not_ to replace runtime PM by any means!

However, there are situations in which runtime PM is clearly insufficient.
The idea behind runtime PM is that subsystems and device drivers will know
when to put devices into low power states and save energy this way.  Still,
even if all subsystems do that 100% efficiently, there may be more savings
possible by putting the entire system into a sleep state (like on ACPI-based
PCs) and we can reach there by runtime PM alone.

> >> Suspend blockers give us some flexibility on systems where runtime pm
> >> will not get us all the way there. If you can meet your power needs
> >> without needing suspend blockers, awesome, you don't need them on that
> >> platform.  The patchset Arve sent out makes this feature an
> >> off-by-default kernel configuration option that compiles out to no-ops
> >> when disabled.
> >
> > I think a big part of this for me is that this approach changes the
> > intended use of the system-wide suspend a bit, complicating it a bit
> > further than it already is.  We end up doing a suspend (which in the
> > normal course of affairs means that the driver should shut everything
> > down) partly as a substitute for runtime PM on the core system devices
> > and partly because our runtime PM isn't doing as well as it should on
> > the individual devices.
> 
> Agreed, and because of this, part of my concern is that opportunistic
> suspend will take the place of doing the "right thing" which (IMHO) 
> would be to improve runtime PM for devices.

It really can't, at least on some platforms.  For example, resuming a PC
notebook from suspend to RAM takes more than 1s (with an SSD; rotational disks
add more latency here) which is way too much for such a replacement.

Besides, there also is room for runtime PM in the Android framework, because
it needs to suspend certain devices without freezing processes, for example.

The freezing of processes is the most important difference to me.  Runtime PM
works with the assumption that processes are not frozen (hence the name), but
arguably energy savings you can get without going behind that point are
naturally limited.

> > I'd be a lot more comfortable with this if it came with a mechanism for
> > communicating the intended effect of a suspend on a per-device level so
> > that the drivers have a clear way of figuring out what to do when the
> > system suspends.  If there isn't one we can add subsystem or driver
> > specific hooks, of course, but it'd be better to address this at the
> > system level.
> 
> I agree.

I'm not sure.  Why _exactly_ would you need that?

> I think there needs to be more discussion on the indended usage
> of suspend blockers by drivers/subsystems, especially those PM aware
> drivers and subsystems already doing runtime PM with constraints etc.
> 
> >> In our experience, periodic timers and polling, both userspace side
> >> and kernelside make suspend blockers a win even on platforms like OMAP
> >> which have pretty flexible hardware power management.  Going to low
> >> power states in idle results in higher average power consumption than
> >> going to the same states in full suspend, at least on Android devices
> >> shipping today.
> >
> > There's definite work to be done here, yes.
> 
> And I've admitted this to be the only compelling reason for
> opportunistic suspend so far.
> 
> But the driver/subsystem implications of opportunistic suspend still
> need some fleshing out IMO.

At the moment, if you're not on Android, there are none.

Thanks,
Rafael

Re: [PATCH 0/8] Suspend block api (version 6)

[Rafael J. Wysocki]

On Tuesday 04 May 2010, Rafael J. Wysocki wrote:
> On Tuesday 04 May 2010, Kevin Hilman wrote:
> > Mark Brown <broonie@opensource.wolfsonmicro.com> writes:
> > 
> > > On Mon, May 03, 2010 at 05:43:34PM -0700, Brian Swetland wrote:
> > >> On Mon, May 3, 2010 at 5:09 PM, Arve Hjønnevåg <arve@android.com> wrote:
> > >> > On Mon, May 3, 2010 at 4:37 PM, Kevin Hilman
> > >
> > >> >> This last point is especially troubling.  I don't find it a comforting
> > >> >> path to go down if the drivers have to start caring about which PM
> > >> >> policy is currently in use.
> > >
> > >> I'll echo Arve here -- all drivers should seek to be in the lowest
> > >> power state possible at all times.  We've never suggested that
> > >> suspend_block is a substitute for that.
> > >
> > > Looking at this from a subsystem/driver author point of view the problem
> > > I'm faced with is that as a result of using system suspend much more
> > > aggressively the subsystem and driver layers are getting conflicting
> > > instructions about what the lowest power state possible is.
> > 
> > Exactly.
> > 
> > With runtime PM, there is flexibility in choosing the lowest power
> > state at the device/subsystem level, based on activity, timeouts,
> > bitrate, dependencies, latency/throughput constraints, etc.
> > 
> > With opportunistic suspend, all of this flexibility is gone, and the
> > device/subsystem is told to go into the lowest power, highest latency
> > state, period.
> 
> Guys, please.
> 
> The opportunistic suspend feature is _not_ to replace runtime PM by any means!
> 
> However, there are situations in which runtime PM is clearly insufficient.
> The idea behind runtime PM is that subsystems and device drivers will know
> when to put devices into low power states and save energy this way.  Still,
> even if all subsystems do that 100% efficiently, there may be more savings
> possible by putting the entire system into a sleep state (like on ACPI-based
> PCs) and we can reach there by runtime PM alone.

s/can/can't/; s/reach/go/

Rafael

Re: [PATCH 0/8] Suspend block api (version 6)

[Mark Brown]

On Tue, May 04, 2010 at 10:23:41PM +0200, Rafael J. Wysocki wrote:
> On Tuesday 04 May 2010, Kevin Hilman wrote:
> > Mark Brown <broonie@opensource.wolfsonmicro.com> writes:

> Guys, please.

> The opportunistic suspend feature is _not_ to replace runtime PM by any means!

Certainly in my case and I think Kevin's I agree with the need for the
bodge at the minute if we can get a clearer idea of how it's supposed to
work.

> when to put devices into low power states and save energy this way.  Still,
> even if all subsystems do that 100% efficiently, there may be more savings
> possible by putting the entire system into a sleep state (like on ACPI-based
> PCs) and we can reach there by runtime PM alone.

Right, this is likely to be a win for PCs - for embedded systems we
rarely have other software to interoperate with so if you can make the
hardware do it then it's unlikely there would be any fundamental issue
with Linux doing it at runtime.

> > > I'd be a lot more comfortable with this if it came with a mechanism for
> > > communicating the intended effect of a suspend on a per-device level so
> > > that the drivers have a clear way of figuring out what to do when the
> > > system suspends.  If there isn't one we can add subsystem or driver
> > > specific hooks, of course, but it'd be better to address this at the
> > > system level.

> > I agree.

> I'm not sure.  Why _exactly_ would you need that?

The use case that causes serious issues with this at the minute in the
domains I work is this:

On a mobile phone when the system is in a voice call the data flow for
the call is entirely handled outside the CPU (normally referred to as
Applications Processor or AP here) by the baseband and audio CODEC,
which are either integrated or directly connected by analogue or digital
audio links on the board.  If the user has the phone to their ear and is
talking away with the screen blanked then the AP is just waiting for
input, will appear idle and so trigger an opportunistic suspend.  If the
audio CODEC is managed by Linux (as is standard) then running through
the suspend process will as things stand cause the audio subsystem to be
suspended.  Since in the normal course of affairs suspend means power
down that's what will happen, but this is clearly highly undesirable in
this situation.

Now, the solution here if we're going to use opportunistic suspend like
this is to provide some method for the audio subsystem to figure out
that the system doesn't actually want it to suspend when it gets told do
do so.  Like I say ideally we'd provide some standard interface in the
PM subsystem for userspace to communicate this to subsystems and drivers
so that we've got a joined up story when people run into issues in cases
like this, though obviously if this goes in then we'll have to put in
something subsystem or driver specific for affected areas.  I know what
I'd implement generically for audio, but I've held back since the option
is fairly messy when not used in conjunction with a deliberate choice to
use opportunistic suspend and I was expecting a standard mechanism to
slot into to provide control for userspace.

In other words, the issue is that we run into situations where we need
an element of suspend control to go along with the opportunistic suspend
in order to allow some elements to be kept running - since suspend
blocking is taken suspend suppression seems like a reasonable term.

Re: [PATCH 0/8] Suspend block api (version 6)

[Rafael J. Wysocki]

On Wednesday 05 May 2010, Mark Brown wrote:
> On Tue, May 04, 2010 at 10:23:41PM +0200, Rafael J. Wysocki wrote:
> > On Tuesday 04 May 2010, Kevin Hilman wrote:
> > > Mark Brown <broonie@opensource.wolfsonmicro.com> writes:
> 
> > Guys, please.
> 
> > The opportunistic suspend feature is _not_ to replace runtime PM by any means!
> 
> Certainly in my case and I think Kevin's I agree with the need for the
> bodge at the minute if we can get a clearer idea of how it's supposed to
> work.
> 
> > when to put devices into low power states and save energy this way.  Still,
> > even if all subsystems do that 100% efficiently, there may be more savings
> > possible by putting the entire system into a sleep state (like on ACPI-based
> > PCs) and we can reach there by runtime PM alone.
> 
> Right, this is likely to be a win for PCs - for embedded systems we
> rarely have other software to interoperate with so if you can make the
> hardware do it then it's unlikely there would be any fundamental issue
> with Linux doing it at runtime.

Evidently, the Android developers had a problem with that.  Of course, you can
argue that they didn't consider using runtime PM for this purpose, but the real
question is how much time it would take to achieve the same level of energy
saving using runtime PM without opportunistic suspend.

> > > > I'd be a lot more comfortable with this if it came with a mechanism for
> > > > communicating the intended effect of a suspend on a per-device level so
> > > > that the drivers have a clear way of figuring out what to do when the
> > > > system suspends.  If there isn't one we can add subsystem or driver
> > > > specific hooks, of course, but it'd be better to address this at the
> > > > system level.
> 
> > > I agree.
> 
> > I'm not sure.  Why _exactly_ would you need that?
> 
> The use case that causes serious issues with this at the minute in the
> domains I work is this:
> 
> On a mobile phone when the system is in a voice call the data flow for
> the call is entirely handled outside the CPU (normally referred to as
> Applications Processor or AP here) by the baseband and audio CODEC,
> which are either integrated or directly connected by analogue or digital
> audio links on the board.  If the user has the phone to their ear and is
> talking away with the screen blanked then the AP is just waiting for
> input, will appear idle and so trigger an opportunistic suspend.  If the
> audio CODEC is managed by Linux (as is standard) then running through
> the suspend process will as things stand cause the audio subsystem to be
> suspended.  Since in the normal course of affairs suspend means power
> down that's what will happen, but this is clearly highly undesirable in
> this situation.

In that case someone (either a driver or, most likely, user space) will need to
keep a suspend blocker active.

> Now, the solution here if we're going to use opportunistic suspend like
> this is to provide some method for the audio subsystem to figure out
> that the system doesn't actually want it to suspend when it gets told do
> do so.  Like I say ideally we'd provide some standard interface in the
> PM subsystem for userspace to communicate this to subsystems and drivers
> so that we've got a joined up story when people run into issues in cases
> like this, though obviously if this goes in then we'll have to put in
> something subsystem or driver specific for affected areas.

My understanding is that on Android suspend blockers are used for this
purpose.

> I know what I'd implement generically for audio, but I've held back since
> the option is fairly messy when not used in conjunction with a deliberate
> choice to use opportunistic suspend and I was expecting a standard mechanism
> to slot into to provide control for userspace.
> 
> In other words, the issue is that we run into situations where we need
> an element of suspend control to go along with the opportunistic suspend
> in order to allow some elements to be kept running - since suspend
> blocking is taken suspend suppression seems like a reasonable term.

Suspend really is a sledgehammer thing.  Either you suspend the whole system
or you don't suspend at all.  You can prevent opportunistic suspend from
happening using suspend blockers (that's what they are for), but that's pretty
much everything you can do about it.  If you want power savings while some
parts of the system are active, use runtime PM for that.

What I'd use opportunistic suspend for is not the saving of energy when there's
a call in progress, because in that case some parts of the system need to be
active, but to save energy when the device is completely idle, ie. the user
interface is not used, music is not played "in the background" etc. (my phone
is in such a state 90% of the time at least).

Rafael

Re: [PATCH 0/8] Suspend block api (version 6)

[Brian Swetland]

On Tue, May 4, 2010 at 5:22 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>>
>> In other words, the issue is that we run into situations where we need
>> an element of suspend control to go along with the opportunistic suspend
>> in order to allow some elements to be kept running - since suspend
>> blocking is taken suspend suppression seems like a reasonable term.
>
> Suspend really is a sledgehammer thing.  Either you suspend the whole system
> or you don't suspend at all.  You can prevent opportunistic suspend from
> happening using suspend blockers (that's what they are for), but that's pretty
> much everything you can do about it.  If you want power savings while some
> parts of the system are active, use runtime PM for that.
>
> What I'd use opportunistic suspend for is not the saving of energy when there's
> a call in progress, because in that case some parts of the system need to be
> active, but to save energy when the device is completely idle, ie. the user
> interface is not used, music is not played "in the background" etc. (my phone
> is in such a state 90% of the time at least).

We actually do use opportunistic suspend for cases like this on
Android today.  It mostly comes down to a question of latency for
return from full suspend.  For example:

Some MSM7201 based devices typically resume from power collapse in
3-5ms, but absolute worst case (because the L1 radio services are
higher priority than the wake-the-apps-cpu services) is ~90ms.  On
these devices we hold a suspend_blocker while playing audio.  Before
discovery of this we went into full suspend while playing audio which
worked fine, but in certain network conditions we would drop buffers
if we could not wake fast enough.

So, it's somewhat system dependent, but it can be very helpful in a
lot of cases.

Brian

Re: [PATCH 0/8] Suspend block api (version 6)

[Mark Brown]

On Wed, May 05, 2010 at 02:22:30AM +0200, Rafael J. Wysocki wrote:
> On Wednesday 05 May 2010, Mark Brown wrote:

> > On a mobile phone when the system is in a voice call the data flow for
> > the call is entirely handled outside the CPU (normally referred to as
> > Applications Processor or AP here) by the baseband and audio CODEC,

> In that case someone (either a driver or, most likely, user space) will need to
> keep a suspend blocker active.

That is not actually what Android systems are doing, and if it is what's
supposed to happen then I'd really expect to see a patch to ASoC as part
of this series which shows how this is supposed to be integrated - it's
the sort of thing I'd expect to see some kernel space management for.

Honestly I don't think that's a very good solution for actual systems,
though.  A part of the reasoning behind designing systems in this way is
allowing the AP to go into the lowest possible power state while on a
voice call so it doesn't seem at all unreasonable for the system
integrator to expect that the AP will be driven into the standard low
power state the system uses for it during a call and in a system using
opportunistic suspend that's suspend mode.

> > In other words, the issue is that we run into situations where we need
> > an element of suspend control to go along with the opportunistic suspend
> > in order to allow some elements to be kept running - since suspend
> > blocking is taken suspend suppression seems like a reasonable term.

> Suspend really is a sledgehammer thing.  Either you suspend the whole system
> or you don't suspend at all.  You can prevent opportunistic suspend from
> happening using suspend blockers (that's what they are for), but that's pretty
> much everything you can do about it.  If you want power savings while some
> parts of the system are active, use runtime PM for that.

On the one hand that's the answer that works well with the existing
Linux design here so great.  On the other hand as discussed above that
doesn't match so well with user expectations.

> What I'd use opportunistic suspend for is not the saving of energy when there's
> a call in progress, because in that case some parts of the system need to be
> active, but to save energy when the device is completely idle, ie. the user
> interface is not used, music is not played "in the background" etc. (my phone
> is in such a state 90% of the time at least).

Remember that even in your full system suspend the system is not
actually fully idle - the baseband is still sitting there looking after
itself, keeping the phone on the network.  The only difference between
on call and off call from the point of view of the Linux system is that
there is an active audio path which happens to have been set up by the
Linux system.

Re: [PATCH 0/8] Suspend block api (version 6)

[Brian Swetland]

On Wed, May 5, 2010 at 4:06 AM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Wed, May 05, 2010 at 02:22:30AM +0200, Rafael J. Wysocki wrote:
>> On Wednesday 05 May 2010, Mark Brown wrote:
>
>> > On a mobile phone when the system is in a voice call the data flow for
>> > the call is entirely handled outside the CPU (normally referred to as
>> > Applications Processor or AP here) by the baseband and audio CODEC,
>
>> In that case someone (either a driver or, most likely, user space) will need to
>> keep a suspend blocker active.
>
> That is not actually what Android systems are doing, and if it is what's
> supposed to happen then I'd really expect to see a patch to ASoC as part
> of this series which shows how this is supposed to be integrated - it's
> the sort of thing I'd expect to see some kernel space management for.
>
> Honestly I don't think that's a very good solution for actual systems,
> though.  A part of the reasoning behind designing systems in this way is
> allowing the AP to go into the lowest possible power state while on a
> voice call so it doesn't seem at all unreasonable for the system
> integrator to expect that the AP will be driven into the standard low
> power state the system uses for it during a call and in a system using
> opportunistic suspend that's suspend mode.

Yup.  And that's exactly what happens on the platforms we've shipped
on so far -- the apps side of the world fully suspends while in a
voice call.  Some events (prox sensor, buttons, modem sending a state
notification) will wake it up, of course.

I haven't spent much time looking at alsa/asoc yet, but it's on my
list for 2010 -- I'm hoping to migrate the very simple audio drivers I
wrote for the msm platform originally to actually be alsa drivers as
part of the general "try to use existing interfaces if they fit" plan
we've been working toward.

The suspend_block system gets us what we need today (well what we
needed three years ago too!) to ship and hit reasonable power targets
on a number of platforms.  There's been a lot of various handwaving
about "android kernel forks" and what have you, but here we are again,
trying to work out perhaps the only "new subsystem" type change that
our driver code depends on (almost all other contentious stuff is self
contained and you can take or leave it).

Our hope here is to get something out there in the near term so that
the various drivers we maintain would "just work" in mainline (your
choice of if you use suspend_block or not -- it's designed to be an
option) and we can move forward.  If in the future runtime power
management or other systems completely obsolete suspend_blockers,
awesome, we remove 'em.

Brian

Re: [PATCH 0/8] Suspend block api (version 6)

[Mark Brown]

On Wed, May 05, 2010 at 05:00:33AM -0700, Brian Swetland wrote:

> I haven't spent much time looking at alsa/asoc yet, but it's on my
> list for 2010 -- I'm hoping to migrate the very simple audio drivers I
> wrote for the msm platform originally to actually be alsa drivers as
> part of the general "try to use existing interfaces if they fit" plan
> we've been working toward.

Yup, that'd be good - even with the AP/CP/CODEC SoCs like the MSM
devices we really need to get ASoC integration since systems are being
built hanging external components such as speaker drivers off the line
outputs, and some of those have registers so really do benefit from the
sequencing, automated PM and so on that ASoC offers.

There was some work on MSM ASoC support posted last year back but there
were a number of review issues with it.  Daniel Walker also talked about
submitting stuff just before Christmas, quite possibly independently of
the other work which looked like a community effort, but I've not seen
anything through from that yet.

> The suspend_block system gets us what we need today (well what we
> needed three years ago too!) to ship and hit reasonable power targets
> on a number of platforms.  There's been a lot of various handwaving
> about "android kernel forks" and what have you, but here we are again,
> trying to work out perhaps the only "new subsystem" type change that
> our driver code depends on (almost all other contentious stuff is self
> contained and you can take or leave it).

> Our hope here is to get something out there in the near term so that
> the various drivers we maintain would "just work" in mainline (your
> choice of if you use suspend_block or not -- it's designed to be an
> option) and we can move forward.  If in the future runtime power
> management or other systems completely obsolete suspend_blockers,
> awesome, we remove 'em.

Like I've said a few times here I broadly agree with your goals - they
seem to be a reasonable solution to the engineering problems we face
presently, even though they're not where we actually want to be.  What
I do miss from the current proposal is more consideration of how things
that do need to ignore the suspend should integrate.

If the conclusion is that we don't have anything generic within the
kernel then it'd be good to at least have this explicitly spelled out so
that we're clear what everyone thinks is going on here and how things
are supposed to work.  At the minute it doesn't feel like we've had the
discussion so we could end up working at cross purposes.  I don't want
to end up in the situation where I have to work around the APIs I'm
using without the relevant maintainers having sight of that since that
not only am I likely to be missing some existing solution to the problem
but is also prone to causing problems maintaining the underlying API.

This hasn't been a pressing issue while the Android code is out of tree
since we can just say it's an out of tree patch that has a different
design approach to current mainline and it's fairly straightforward for
users to introduce suitable system-specific changes.  Once it's mainline
and part of the standard Linux PM toolkit then obviously subsystems and
drivers need to support opportunistic suspend properly in mainline.

Re: [PATCH 0/8] Suspend block api (version 6)

[Matthew Garrett]

On Wed, May 05, 2010 at 02:56:22PM +0100, Mark Brown wrote:

> If the conclusion is that we don't have anything generic within the
> kernel then it'd be good to at least have this explicitly spelled out so
> that we're clear what everyone thinks is going on here and how things
> are supposed to work.  At the minute it doesn't feel like we've had the
> discussion so we could end up working at cross purposes.  I don't want
> to end up in the situation where I have to work around the APIs I'm
> using without the relevant maintainers having sight of that since that
> not only am I likely to be missing some existing solution to the problem
> but is also prone to causing problems maintaining the underlying API.

We seem to have ended up managing most of our PM infrastructure 
iteratively. If the concern is more about best practices than intrinsic 
incompatibilities, I'd lean towards us being better off merging this now 
and then working things out over the next few releases as we get a 
better understanding of the implications. The main thing that we have to 
get right in the short term is the userspace API - everything else is 
easier to fix up as we go along.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH 0/8] Suspend block api (version 6)

[Alan Stern]

On Wed, 5 May 2010, Matthew Garrett wrote:

> On Wed, May 05, 2010 at 02:56:22PM +0100, Mark Brown wrote:
> 
> > If the conclusion is that we don't have anything generic within the
> > kernel then it'd be good to at least have this explicitly spelled out so
> > that we're clear what everyone thinks is going on here and how things
> > are supposed to work.  At the minute it doesn't feel like we've had the
> > discussion so we could end up working at cross purposes.  I don't want
> > to end up in the situation where I have to work around the APIs I'm
> > using without the relevant maintainers having sight of that since that
> > not only am I likely to be missing some existing solution to the problem
> > but is also prone to causing problems maintaining the underlying API.
> 
> We seem to have ended up managing most of our PM infrastructure 
> iteratively. If the concern is more about best practices than intrinsic 
> incompatibilities, I'd lean towards us being better off merging this now 
> and then working things out over the next few releases as we get a 
> better understanding of the implications. The main thing that we have to 
> get right in the short term is the userspace API - everything else is 
> easier to fix up as we go along.

This particular question could use a little more discussion.  I'm
interested to know, for example, under what conditions you would or
would not want to shut down an autonomous codec while going into system 
suspend on a cell phone.

Clearly if there's a call in progress you don't want to shut the codec
down.  Are there any other circumstances?  Would they vary according to
whether the suspend was forced or opportunistic?

In short, I'm trying to get at how much information drivers _really_ 
need to have about the reason for a system suspend.

Alan Stern

Re: [PATCH 0/8] Suspend block api (version 6)

[Matthew Garrett]

On Wed, May 05, 2010 at 02:36:10PM -0400, Alan Stern wrote:

> This particular question could use a little more discussion.  I'm
> interested to know, for example, under what conditions you would or
> would not want to shut down an autonomous codec while going into system 
> suspend on a cell phone.
> 
> Clearly if there's a call in progress you don't want to shut the codec
> down.  Are there any other circumstances?  Would they vary according to
> whether the suspend was forced or opportunistic?

Yeah, ok. We probably do need to figure this out.

(Cc:ing Rebecca to see how this got handled on Droid)

The current state of affairs is that a system suspend request is 
expected to put the device in as low a power state as possible given the 
required wakeup events. Runtime power management is expected to put the 
device in as low a power state as possible given its usage constraints. 
If opportunistic suspend does the former then it'll tear down devices 
that may be in use, but we don't have any real way to indicate usage 
constraints other than the phone app taking a wakelock - and that means 
leaving userspace running during calls, which seems excessive.

Mark's right in that the only case I can think of that's really relevant 
right now is the audio hardware, so the inelegant solution is that this 
is something that could be provided at the audio level. Is this 
something we want a generic solution for? If so, what should it look 
like?

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH 0/8] Suspend block api (version 6)

[Alan Stern]

On Wed, 5 May 2010, Matthew Garrett wrote:

> > Clearly if there's a call in progress you don't want to shut the codec
> > down.  Are there any other circumstances?  Would they vary according to
> > whether the suspend was forced or opportunistic?
> 
> Yeah, ok. We probably do need to figure this out.
> 
> (Cc:ing Rebecca to see how this got handled on Droid)
> 
> The current state of affairs is that a system suspend request is 
> expected to put the device in as low a power state as possible given the 
> required wakeup events. Runtime power management is expected to put the 
> device in as low a power state as possible given its usage constraints. 
> If opportunistic suspend does the former then it'll tear down devices 
> that may be in use, but we don't have any real way to indicate usage 
> constraints other than the phone app taking a wakelock - and that means 
> leaving userspace running during calls, which seems excessive.
> 
> Mark's right in that the only case I can think of that's really relevant 
> right now is the audio hardware, so the inelegant solution is that this 
> is something that could be provided at the audio level. Is this 
> something we want a generic solution for? If so, what should it look 
> like?

Should the audio driver be smart enough to know that the codec needs to
remain powered up during every opportunistic suspend?

Or only during opportunistic suspends while a call is in progress?  

Does it know when a call is in progress?

Does Android use forced suspends?

If it does, are they supposed to shut down the codec?

Alan Stern

Re: [PATCH 0/8] Suspend block api (version 6)

[Matthew Garrett]

On Wed, May 05, 2010 at 03:13:52PM -0400, Alan Stern wrote:

> Should the audio driver be smart enough to know that the codec needs to
> remain powered up during every opportunistic suspend?
> 
> Or only during opportunistic suspends while a call is in progress?  

The latter is obviously preferable...

> Does it know when a call is in progress?

Perhaps you could infer that from the audio routing setup? I don't know 
enough about embedded codecs.

> Does Android use forced suspends?

No.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH 0/8] Suspend block api (version 6)

[Mark Brown]

On Wed, May 05, 2010 at 08:22:08PM +0100, Matthew Garrett wrote:
> On Wed, May 05, 2010 at 03:13:52PM -0400, Alan Stern wrote:

> > Does it know when a call is in progress?

> Perhaps you could infer that from the audio routing setup? I don't know 
> enough about embedded codecs.

Yes, you can providing we extract a bit more information from the
machine drivers and/or userspace - we already do our power management
based on paths.  The implementation I'd do here is to provide a facility
for marking some of the path endpoints as suspend insensitive then on
suspend leave those endpoints in their current state, force the other
endpoints off temporarily and re-run the standard power checks.

One thing I'll need to have a bit of a think about is if we need to
provide the ability to offer control to userspace for the suspend state
of path endpoints separately to their runtime state; I suspect the
answer is yes.

Re: [PATCH 0/8] Suspend block api (version 6)

[tytso]

Hi Mark,

I confess I've completely lost track of (a) what problem you are
trying to solve, and (b) how this might relate to some change that
you'd like to see in the suspend block API.  Could you do a quick
summary and recap?  I've gone over the entire thread, and it's still
not clear what change you're advocating for in suspend blockers.

Thanks, regards,

						- Ted

On Wed, May 05, 2010 at 08:52:50PM +0100, Mark Brown wrote:
> On Wed, May 05, 2010 at 08:22:08PM +0100, Matthew Garrett wrote:
> > On Wed, May 05, 2010 at 03:13:52PM -0400, Alan Stern wrote:
> 
> > > Does it know when a call is in progress?
> 
> > Perhaps you could infer that from the audio routing setup? I don't know 
> > enough about embedded codecs.
> 
> Yes, you can providing we extract a bit more information from the
> machine drivers and/or userspace - we already do our power management
> based on paths.  The implementation I'd do here is to provide a facility
> for marking some of the path endpoints as suspend insensitive then on
> suspend leave those endpoints in their current state, force the other
> endpoints off temporarily and re-run the standard power checks.
> 
> One thing I'll need to have a bit of a think about is if we need to
> provide the ability to offer control to userspace for the suspend state
> of path endpoints separately to their runtime state; I suspect the
> answer is yes.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@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: [PATCH 0/8] Suspend block api (version 6)

[Mark Brown]

On Wed, May 05, 2010 at 03:55:34PM -0400, tytso@mit.edu wrote:

> I confess I've completely lost track of (a) what problem you are
> trying to solve, and (b) how this might relate to some change that
> you'd like to see in the suspend block API.  Could you do a quick
> summary and recap?  I've gone over the entire thread, and it's still
> not clear what change you're advocating for in suspend blockers.

The issue isn't suspend blockers, it's the opportunistic suspend stuff
that goes along with them.  When that is in use the system suspends
vastly more aggressively, including in situations where a runtime PM
based approach like mainline had been adopting would not suspend since
some devices still need to be active, the classic case being keeping the
audio subsystem and baseband live when in a phone call.  This problem
did not appear to have been considered as things stood.  

I'm not really advocating a change in what's there.  What I'm looking
for is some sort of agreement as to how subsystems and drivers that need
to not act on suspend requests from the core in these situations should
do that.  If there is a generic solution it'd probably be an additional
mostly orthogonal interface rather than a change to what's being
proposed here.

What we look like we're converging on is a subsystem/driver local
solution since it doesn't look like a terribly widespread problem.
That's totally OK, it's just that as I have said I don't want to go off
and do that without the general PM community being aware of it so we
avoid anyone running into nasty surprises further down the line.

Re: [PATCH 0/8] Suspend block api (version 6)

[Rafael J. Wysocki]

On Wednesday 05 May 2010, Mark Brown wrote:
> On Wed, May 05, 2010 at 03:55:34PM -0400, tytso@mit.edu wrote:
> 
> > I confess I've completely lost track of (a) what problem you are
> > trying to solve, and (b) how this might relate to some change that
> > you'd like to see in the suspend block API.  Could you do a quick
> > summary and recap?  I've gone over the entire thread, and it's still
> > not clear what change you're advocating for in suspend blockers.
> 
> The issue isn't suspend blockers, it's the opportunistic suspend stuff
> that goes along with them.  When that is in use the system suspends
> vastly more aggressively, including in situations where a runtime PM
> based approach like mainline had been adopting would not suspend since
> some devices still need to be active, the classic case being keeping the
> audio subsystem and baseband live when in a phone call.  This problem
> did not appear to have been considered as things stood.  
> 
> I'm not really advocating a change in what's there.  What I'm looking
> for is some sort of agreement as to how subsystems and drivers that need
> to not act on suspend requests from the core in these situations should
> do that.  If there is a generic solution it'd probably be an additional
> mostly orthogonal interface rather than a change to what's being
> proposed here.
> 
> What we look like we're converging on is a subsystem/driver local
> solution since it doesn't look like a terribly widespread problem.
> That's totally OK, it's just that as I have said I don't want to go off
> and do that without the general PM community being aware of it so we
> avoid anyone running into nasty surprises further down the line.

To me, the above may be summarized that in your opinion some components of
the system will generally need to stay powered when it's suspended
opportunistically, so we need an interface to specify which components they are.
Is that correct?

Rafael

Re: [PATCH 0/8] Suspend block api (version 6)

[Mark Brown]

On Wed, May 05, 2010 at 10:44:03PM +0200, Rafael J. Wysocki wrote:

> To me, the above may be summarized that in your opinion some components of
> the system will generally need to stay powered when it's suspended
> opportunistically, so we need an interface to specify which components they are.
> Is that correct?

Yes, though I think I'd be inclined to treat the problem orthogonally to
opportunistic suspend to allow more flexibility in use and since
treating it as part of opportunistic suspend would imply that there's
some meaningful difference between the end result of that and a manual
suspend which AIUI there isn't.

Re: [PATCH 0/8] Suspend block api (version 6)

[Brian Swetland]

On Wed, May 5, 2010 at 2:57 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Wed, May 05, 2010 at 10:44:03PM +0200, Rafael J. Wysocki wrote:
>
>> To me, the above may be summarized that in your opinion some components of
>> the system will generally need to stay powered when it's suspended
>> opportunistically, so we need an interface to specify which components they are.
>> Is that correct?
>
> Yes, though I think I'd be inclined to treat the problem orthogonally to
> opportunistic suspend to allow more flexibility in use and since
> treating it as part of opportunistic suspend would imply that there's
> some meaningful difference between the end result of that and a manual
> suspend which AIUI there isn't.

I'd agree with this.  This is one of the reasons I haven't felt
opportunistic suspend is a big departure from other work here -- there
are always some cases in which drivers will need to keep external
resources active even when suspended.  Also, as I mentioned earlier,
we (Android at least, but hopefully this is non-controversial) would
always like drivers to put everything in the lowest possible power
state they can get away with.  Every little savings adds up.

Brian

Re: [PATCH 0/8] Suspend block api (version 6)

[Rafael J. Wysocki]

On Wednesday 05 May 2010, Mark Brown wrote:
> On Wed, May 05, 2010 at 10:44:03PM +0200, Rafael J. Wysocki wrote:
> 
> > To me, the above may be summarized that in your opinion some components of
> > the system will generally need to stay powered when it's suspended
> > opportunistically, so we need an interface to specify which components they are.
> > Is that correct?
> 
> Yes, though I think I'd be inclined to treat the problem orthogonally to
> opportunistic suspend to allow more flexibility in use and since
> treating it as part of opportunistic suspend would imply that there's
> some meaningful difference between the end result of that and a manual
> suspend which AIUI there isn't.

No, there's no such difference.

So, gnerenally, we may need a mechanism to specify which components of the
system need to stay powered while the whole system is suspended (in addition to
wakeup devices, that is).

That certainly I can agree with.

I'm not sure, however, in what way this is relevant to the $subject patchset.

Rafael

Re: [PATCH 0/8] Suspend block api (version 6)

[Mark Brown]

On Thu, May 06, 2010 at 12:05:01AM +0200, Rafael J. Wysocki wrote:

> So, gnerenally, we may need a mechanism to specify which components of the
> system need to stay powered while the whole system is suspended (in addition to
> wakeup devices, that is).

> That certainly I can agree with.

> I'm not sure, however, in what way this is relevant to the $subject patchset.

The patch set essentially makes using a full system suspend as the
lowest power state for runtime PM part of the standard Linux power
management toolkit which means that it's no longer clear as it used to
be that suspend is an instruction to cease all activity and go into a
minimal power state if the device is not a wake source.  In the primary
existing application this change interoperates very poorly with at least
the current audio subsystem since that handles suspend by ceasing all
activity and powering as much as it can off, which is sensible for
manual only suspends but highly undesirable for opportunistic suspend in
phones.  We should therefore have some idea how this and any other
affected areas are supposed to work.

As I said in my reply to Ted earlier I think we may actually be
converging on not worrying too much about it at the global level and
doing subsystem specific things to discover and handle affected cases,
at least for the time being.  Ideally we'd have something standard to
hook into but no subsystems apart from audio have actually been
identified as being affected so it's not clear to me that a general
solution is going to be worth the effort, if there's no actual users it
may just confuse people by adding yet more power managment stuff to
learn.

Re: [PATCH 0/8] Suspend block api (version 6)

[Rafael J. Wysocki]

On Thursday 06 May 2010, Mark Brown wrote:
> On Thu, May 06, 2010 at 12:05:01AM +0200, Rafael J. Wysocki wrote:
> 
> > So, gnerenally, we may need a mechanism to specify which components of the
> > system need to stay powered while the whole system is suspended (in addition to
> > wakeup devices, that is).
> 
> > That certainly I can agree with.
> 
> > I'm not sure, however, in what way this is relevant to the $subject patchset.
> 
> The patch set essentially makes using a full system suspend as the
> lowest power state for runtime PM part of the standard Linux power
> management toolkit which means that it's no longer clear as it used to
> be that suspend is an instruction to cease all activity and go into a
> minimal power state if the device is not a wake source.

I don't see why, really.  This patchset doesn't change the meaning of suspend
at all, it only changes the way in which suspend is triggered (if specifically
set up that way).  Even without the patchset you may implement a power
manager in user space that will suspend the system whenever it thinks it's
idle.

> In the primary existing application this change interoperates very poorly
> with at least the current audio subsystem since that handles suspend by
> ceasing all activity and powering as much as it can off, which is sensible for
> manual only suspends but highly undesirable for opportunistic suspend in
> phones.

You said that there's no fundamental difference between manual and
opportunistic suspend.  It only matters what _you_ are going to use suspend
for.  I agree that at the moment it's not suitable for aggressive power
management in phones because of the audio problem, but that applies to
"manual" as well as to "opportunistic" suspend.

You're saying that suspend is not suitable for one particular purpose in its
current form, which is entirely correct, but that doesn't imply that the
patchset is wrong.

Rafael

Re: [PATCH 0/8] Suspend block api (version 6)

[Mark Brown]

On Thu, May 06, 2010 at 01:33:59AM +0200, Rafael J. Wysocki wrote:

> set up that way).  Even without the patchset you may implement a power
> manager in user space that will suspend the system whenever it thinks it's
> idle.

Clearly, but...

> On Thursday 06 May 2010, Mark Brown wrote:

> > In the primary existing application this change interoperates very poorly
> > with at least the current audio subsystem since that handles suspend by
> > ceasing all activity and powering as much as it can off, which is sensible for
> > manual only suspends but highly undesirable for opportunistic suspend in
> > phones.

> You said that there's no fundamental difference between manual and
> opportunistic suspend.  It only matters what _you_ are going to use suspend
> for.  I agree that at the moment it's not suitable for aggressive power
> management in phones because of the audio problem, but that applies to
> "manual" as well as to "opportunistic" suspend.

...on the other hand there's exactly one existing application for this,
and that's the one that's most likely to run into problems since it's a
phone OS and aggressive power management is pretty important for phones.

Merging a feature into mainline makes it much more something which one
would expect to play nicely with the rest of the kernel - if it's
something that isn't part of the standard kernel or userspaces it's much
less surprising that additional changes may be required to produce a
well integrated system.

> You're saying that suspend is not suitable for one particular purpose in its
> current form, which is entirely correct, but that doesn't imply that the
> patchset is wrong.

As I keep saying I agree that merging this is reasonable given the
additional power savings it brings in practical systems today.  As I
also keep saying I do want to have some understanding about what the
story is for dealing with the problems so that people can easily use
this feature out of the box.

Like I say, my current impression is that the best approach is for
affected subsystems or drivers to implement a custom solution - does
that match your understanding and that of the other PM maintainers?

Re: [PATCH 0/8] Suspend block api (version 6)

[Rafael J. Wysocki]

On Thursday 06 May 2010, Mark Brown wrote:
> On Thu, May 06, 2010 at 01:33:59AM +0200, Rafael J. Wysocki wrote:
> 
> > set up that way).  Even without the patchset you may implement a power
> > manager in user space that will suspend the system whenever it thinks it's
> > idle.
> 
> Clearly, but...
> 
> > On Thursday 06 May 2010, Mark Brown wrote:
> 
> > > In the primary existing application this change interoperates very poorly
> > > with at least the current audio subsystem since that handles suspend by
> > > ceasing all activity and powering as much as it can off, which is sensible for
> > > manual only suspends but highly undesirable for opportunistic suspend in
> > > phones.
> 
> > You said that there's no fundamental difference between manual and
> > opportunistic suspend.  It only matters what _you_ are going to use suspend
> > for.  I agree that at the moment it's not suitable for aggressive power
> > management in phones because of the audio problem, but that applies to
> > "manual" as well as to "opportunistic" suspend.
> 
> ...on the other hand there's exactly one existing application for this,
> and that's the one that's most likely to run into problems since it's a
> phone OS and aggressive power management is pretty important for phones.
> 
> Merging a feature into mainline makes it much more something which one
> would expect to play nicely with the rest of the kernel - if it's
> something that isn't part of the standard kernel or userspaces it's much
> less surprising that additional changes may be required to produce a
> well integrated system.
> 
> > You're saying that suspend is not suitable for one particular purpose in its
> > current form, which is entirely correct, but that doesn't imply that the
> > patchset is wrong.
> 
> As I keep saying I agree that merging this is reasonable given the
> additional power savings it brings in practical systems today.  As I
> also keep saying I do want to have some understanding about what the
> story is for dealing with the problems so that people can easily use
> this feature out of the box.
> 
> Like I say, my current impression is that the best approach is for
> affected subsystems or drivers to implement a custom solution - does
> that match your understanding and that of the other PM maintainers?

I agree that this appears to be the best approach for the time being, although
I can only speak for myself.

Rafael

Re: [PATCH 0/8] Suspend block api (version 6)

[Matthew Garrett]

On Wed, May 05, 2010 at 08:52:50PM +0100, Mark Brown wrote:
> On Wed, May 05, 2010 at 08:22:08PM +0100, Matthew Garrett wrote:
> > Perhaps you could infer that from the audio routing setup? I don't know 
> > enough about embedded codecs.
> 
> Yes, you can providing we extract a bit more information from the
> machine drivers and/or userspace - we already do our power management
> based on paths.  The implementation I'd do here is to provide a facility
> for marking some of the path endpoints as suspend insensitive then on
> suspend leave those endpoints in their current state, force the other
> endpoints off temporarily and re-run the standard power checks.

One thing that's been mentioned before is that some vendors can swap the 
kernel much more easily than they can swap userspace, with the added 
constraint that they have little control over the bootloader. So if 
there's a userspace interface, it may well also be helpful to provide a 
kernel hook to allow the platform setup code to configure things. But 
otherwise, I think that'd work for audio - the only question is whether 
we need it to be more generic.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH 0/8] Suspend block api (version 6)

[Mark Brown]

On Wed, May 05, 2010 at 09:02:43PM +0100, Matthew Garrett wrote:

> One thing that's been mentioned before is that some vendors can swap the 
> kernel much more easily than they can swap userspace, with the added 
> constraint that they have little control over the bootloader. So if 
> there's a userspace interface, it may well also be helpful to provide a 
> kernel hook to allow the platform setup code to configure things. But 

Oh, there's definitely going to be a platform-specific hook to configure
things.  The platform needs to specify how the devices that make up the
audio subsystem are interconnected on the board, and that's done in
kernel since it depends on the hardware and can potentially lead to
physical damage if done incorrectly.

> otherwise, I think that'd work for audio - the only question is whether 
> we need it to be more generic.

Agreed.

Re: [PATCH 0/8] Suspend block api (version 6)

[Mark Brown]

On Wed, May 05, 2010 at 03:13:52PM -0400, Alan Stern wrote:

> Should the audio driver be smart enough to know that the codec needs to
> remain powered up during every opportunistic suspend?

> Or only during opportunistic suspends while a call is in progress?  

I think for simplicity and maximum flexibility we should just ignore the
reason for suspend here, if I'm adding support for this in the drivers
the suspend reason is not going to be terribly useful and it makes the
feature more widely useful should people come up with other applications.

Note that we already have advanced runtime PM for embedded audio which
will drive modern CODECs into full off state when the device is idle
anyway, independently of any suspend or runtime PM that the system does.
Older CODECs need their bias levels maintaining but we can ignore the
difference there from this point of view.

> Does it know when a call is in progress?

Not at present.  This is relatively straightforward to add within ASoC,
especially for analogue links, if not completely trivial.  Digital
basebands are a bit more fun but not insurmountable (and need cleaning
up anyway, there's some ongoing work which should hit soon).

We currently drive everything into off state when we're told to suspend
since it's the right thing to do for most non-phone devices.  We'll
still need to drive some of the system into an off state (eg, some of
the drivers in the CPU) even when this is handled.

> Does Android use forced suspends?

Pass, but it's not been an issue users have raised with me - but then
we can't tell why the suspend happens and need to handle opportunistic
suspend anyway.

> If it does, are they supposed to shut down the codec?

I suspect it's a non-issue; if we're not actually using audio then ASoC
will drive a modern CODEC into power off anyway as part of its runtime
PM.  However, I believe some non-Android phone stacks are doing
something along that line.

Re: [PATCH 0/8] Suspend block api (version 6)

[Brian Swetland]

On Wed, May 5, 2010 at 12:13 PM, Alan Stern <stern@rowland.harvard.edu> wrote:
> On Wed, 5 May 2010, Matthew Garrett wrote:
>
>> > Clearly if there's a call in progress you don't want to shut the codec
>> > down.  Are there any other circumstances?  Would they vary according to
>> > whether the suspend was forced or opportunistic?
>>
>> Yeah, ok. We probably do need to figure this out.
>>
>> (Cc:ing Rebecca to see how this got handled on Droid)
>>
>> The current state of affairs is that a system suspend request is
>> expected to put the device in as low a power state as possible given the
>> required wakeup events. Runtime power management is expected to put the
>> device in as low a power state as possible given its usage constraints.
>> If opportunistic suspend does the former then it'll tear down devices
>> that may be in use, but we don't have any real way to indicate usage
>> constraints other than the phone app taking a wakelock - and that means
>> leaving userspace running during calls, which seems excessive.
>>
>> Mark's right in that the only case I can think of that's really relevant
>> right now is the audio hardware, so the inelegant solution is that this
>> is something that could be provided at the audio level. Is this
>> something we want a generic solution for? If so, what should it look
>> like?
>
> Should the audio driver be smart enough to know that the codec needs to
> remain powered up during every opportunistic suspend?
>
> Or only during opportunistic suspends while a call is in progress?
>
> Does it know when a call is in progress?

>From my point of view, the codec should be powered up while audio is
playing and powered down when it's not -- that's the approach I've
taken on MSM, regardless of suspend state.  I don't view suspend as
something that stops audio playback in progress.  Sort of similar to
how the modem doesn't power off down when the system is in suspend,
etc.

In effect, we have followed a sort of runtime pm model of
clocking/powering peripherals at the lowest possible level at all
times, which is why I've never viewed suspend_block as somehow
competing with runtime pm -- we always want all drivers/peripherals in
the lowers power state.  Sometimes the lowest power state possible is
higher than "full suspend" (for example when we can't meet latency
requirements on audio on 7201A) and suspend_blockers cover that case.

Brian

Re: [PATCH 0/8] Suspend block api (version 6)

[Mark Brown]

On Wed, May 05, 2010 at 01:56:07PM -0700, Brian Swetland wrote:

> From my point of view, the codec should be powered up while audio is
> playing and powered down when it's not -- that's the approach I've

This is what we've got already with Linux - we've had rather aggressive
runtime PM for embedded audio as standard for years now.  It's only very
recently that mobile hardware has got to the point where you can
actually totally kill the power without introducing problems but it's
been getting pretty much as close as possible.

> taken on MSM, regardless of suspend state.  I don't view suspend as
> something that stops audio playback in progress.  Sort of similar to
> how the modem doesn't power off down when the system is in suspend,
> etc.

This really does depend heavily on system design and the intended
function of the suspend on the part of the initiator.  In many systems
suspend will remove sufficient power to stop audio running even if it
were desired, and there's the idea with existing manually initiated
suspends that the system should stop what it's doing and go into a low
power, low responsiveness state.  Some systems will initiate a suspend
with active audio paths (especially those to or from the AP) which they
expect to be stopped.

> In effect, we have followed a sort of runtime pm model of
> clocking/powering peripherals at the lowest possible level at all
> times, which is why I've never viewed suspend_block as somehow
> competing with runtime pm -- we always want all drivers/peripherals in

Right, and this will work well on systems like your current devices
because the problem hardware is not directly visible to the AP (the
actual audio hardware is hidden behind the DSP which functions as
autonomously as the modem does) so it's not affected by the Linux
suspend process in the first place.  It causes confusion when the power
for the audio device is managed by Linux since the opportunistic suspend
will include suspending the audio device and so the code handling the
suspend then has to decide what exactly it's being asked to do.

> the lowers power state.  Sometimes the lowest power state possible is
> higher than "full suspend" (for example when we can't meet latency
> requirements on audio on 7201A) and suspend_blockers cover that case.

In this situation there's no physical reason why the lower power state
is not achievable so blocking the suspend would just waste power, it's
just that our way of getting into it is creating some confusion.

Re: [PATCH 0/8] Suspend block api (version 6)

[Arve Hjønnevåg]

On Wed, May 5, 2010 at 4:40 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Wed, May 05, 2010 at 01:56:07PM -0700, Brian Swetland wrote:
...
>> taken on MSM, regardless of suspend state.  I don't view suspend as
>> something that stops audio playback in progress.  Sort of similar to
>> how the modem doesn't power off down when the system is in suspend,
>> etc.
>
> This really does depend heavily on system design and the intended
> function of the suspend on the part of the initiator.  In many systems
> suspend will remove sufficient power to stop audio running even if it
> were desired, and there's the idea with existing manually initiated
> suspends that the system should stop what it's doing and go into a low
> power, low responsiveness state.  Some systems will initiate a suspend
> with active audio paths (especially those to or from the AP) which they
> expect to be stopped.
>

You can support both in the same driver in some cases (without a
switch). If the hardware cannot wake the system when it needs more
audio buffers, then the driver needs to block suspend while audio is
playing. Since suspend blockers only block opportunistic suspend, the
driver can also implement suspend to stop audio playback and it will
only be called for forced suspend.

-- 
Arve Hjønnevåg

Re: [PATCH 0/8] Suspend block api (version 6)

[Mark Brown]

On Wed, May 05, 2010 at 09:25:18PM -0700, Arve Hjønnevåg wrote:
> On Wed, May 5, 2010 at 4:40 PM, Mark Brown

> > This really does depend heavily on system design and the intended
> > function of the suspend on the part of the initiator.  In many systems
> > suspend will remove sufficient power to stop audio running even if it
> > were desired, and there's the idea with existing manually initiated
> > suspends that the system should stop what it's doing and go into a low
> > power, low responsiveness state.  Some systems will initiate a suspend
> > with active audio paths (especially those to or from the AP) which they
> > expect to be stopped.

> You can support both in the same driver in some cases (without a
> switch). If the hardware cannot wake the system when it needs more
> audio buffers, then the driver needs to block suspend while audio is
> playing. Since suspend blockers only block opportunistic suspend, the
> driver can also implement suspend to stop audio playback and it will
> only be called for forced suspend.

As discussed elsewhere in the thread a suspend blocker is not desirable
here - the AP is not doing anything useful on a voice call so blocking
the suspend will just waste power unless runtime PM is good enough to
mean opportunistic suspend isn't adding anything anyway.  It will avoid
the immediate issue with loosing audio but it's not really what we want
to happen.

Re: [PATCH 0/8] Suspend block api (version 6)

[Arve Hjønnevåg]

2010/5/7 Mark Brown <broonie@opensource.wolfsonmicro.com>:
> On Wed, May 05, 2010 at 09:25:18PM -0700, Arve Hjønnevåg wrote:
>> On Wed, May 5, 2010 at 4:40 PM, Mark Brown
>
>> > This really does depend heavily on system design and the intended
>> > function of the suspend on the part of the initiator.  In many systems
>> > suspend will remove sufficient power to stop audio running even if it
>> > were desired, and there's the idea with existing manually initiated
>> > suspends that the system should stop what it's doing and go into a low
>> > power, low responsiveness state.  Some systems will initiate a suspend
>> > with active audio paths (especially those to or from the AP) which they
>> > expect to be stopped.
>
>> You can support both in the same driver in some cases (without a
>> switch). If the hardware cannot wake the system when it needs more
>> audio buffers, then the driver needs to block suspend while audio is
>> playing. Since suspend blockers only block opportunistic suspend, the
>> driver can also implement suspend to stop audio playback and it will
>> only be called for forced suspend.
>
> As discussed elsewhere in the thread a suspend blocker is not desirable
> here - the AP is not doing anything useful on a voice call so blocking
> the suspend will just waste power unless runtime PM is good enough to
> mean opportunistic suspend isn't adding anything anyway.  It will avoid
> the immediate issue with loosing audio but it's not really what we want
> to happen.
>

I was talking about audio from the AP. Why would you ever turn off
another core's audio path on suspend?

-- 
Arve Hjønnevåg

Re: [PATCH 0/8] Suspend block api (version 6)

[Mark Brown]

On Fri, May 07, 2010 at 03:57:37AM -0700, Arve Hjønnevåg wrote:
> 2010/5/7 Mark Brown <broonie@opensource.wolfsonmicro.com>:

> > As discussed elsewhere in the thread a suspend blocker is not desirable
> > here - the AP is not doing anything useful on a voice call so blocking
> > the suspend will just waste power unless runtime PM is good enough to
> > mean opportunistic suspend isn't adding anything anyway.  It will avoid
> > the immediate issue with loosing audio but it's not really what we want
> > to happen.

> I was talking about audio from the AP. Why would you ever turn off
> another core's audio path on suspend?

This goes back to the thing about a full system suspend being a
sledgehammer which doesn't give enough information about what's going
on when it's used like this.  As discussed we need a way to know that
the connections involved are able to stay live during suspend (on a
large proportion of systems suspend is going to mean that the relevant
bits of the board will loose power so need to be shut down) and that
that the intention of the user is that they should do so (this isn't
clear in the general system, especially not if the suspend is initiated
manually).  

With a runtime PM approach this is trivial - we just turn off anything
that isn't in use at the current time.  I'll need to extend ASoC to add
information about what to do on suspend to the existing power data.

Re: [PATCH 0/8] Suspend block api (version 6)

[Theodore Tso]

On May 7, 2010, at 7:21 AM, Mark Brown wrote:
> 
> This goes back to the thing about a full system suspend being a
> sledgehammer which doesn't give enough information about what's going
> on when it's used like this.  As discussed we need a way to know that
> the connections involved are able to stay live during suspend (on a
> large proportion of systems suspend is going to mean that the relevant
> bits of the board will loose power so need to be shut down) and that
> that the intention of the user is that they should do so (this isn't
> clear in the general system, especially not if the suspend is initiated
> manually).  

This sounds like it may be something unique to your board/product.  Or am I missing something?

One of the challenges with PM in the embedded world is that everybody seems to have slightly different assumptions, and hardware that doesn't behave the same way.

More than once this discussion has wandered off into the weeds wrt to whether this patch series is ready to be merged, since there are so many drivers blocked on it....

-- Ted

Re: [PATCH 0/8] Suspend block api (version 6)

[Mark Brown]

On Fri, May 07, 2010 at 07:29:47AM -0400, Theodore Tso wrote:

> This sounds like it may be something unique to your board/product.  Or
> am I missing something?

I suspect you're missing something here - I'm approaching this as one of
the maintainers of the embedded audio subsystem for the kernel.  I need
to worry about any system running Linux which has an embedded style
audio subsystem.  The problem might be unique to audio, but I can't say
that for certain.

> One of the challenges with PM in the embedded world is that everybody
> seems to have slightly different assumptions, and hardware that doesn't
> behave the same way.

This isn't really a problem for audio - we've already got a subsystem
which copes well with pretty much all systems and does runtime PM that's
equivalent to suspending already, which is half the problem here.  If we
had less generalisation this would probably not have come up.

> More than once this discussion has wandered off into the weeds wrt to
> whether this patch series is ready to be merged, since there are so many
> drivers blocked on it....

Once more, my main objective here has been to make sure that when this
is merged we've got a joined up story about how people think this hangs
together, which I think we have now.  As I say now that we have that
understanding I don't see any reason to block merge.

It's unfortunate that I only noticed that this was actually wakelocks
very late in the day but I think I can get an implementation which
handles paths that ignore suspends done quickly.

Re: [PATCH 0/8] Suspend block api (version 6)

[Brian Swetland]

On Fri, May 7, 2010 at 5:25 AM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Fri, May 07, 2010 at 07:29:47AM -0400, Theodore Tso wrote:
>
>> This sounds like it may be something unique to your board/product.  Or
>> am I missing something?
>
> I suspect you're missing something here - I'm approaching this as one of
> the maintainers of the embedded audio subsystem for the kernel.  I need
> to worry about any system running Linux which has an embedded style
> audio subsystem.  The problem might be unique to audio, but I can't say
> that for certain.
>
>> One of the challenges with PM in the embedded world is that everybody
>> seems to have slightly different assumptions, and hardware that doesn't
>> behave the same way.
>
> This isn't really a problem for audio - we've already got a subsystem
> which copes well with pretty much all systems and does runtime PM that's
> equivalent to suspending already, which is half the problem here.  If we
> had less generalisation this would probably not have come up.
>
>> More than once this discussion has wandered off into the weeds wrt to
>> whether this patch series is ready to be merged, since there are so many
>> drivers blocked on it....
>
> Once more, my main objective here has been to make sure that when this
> is merged we've got a joined up story about how people think this hangs
> together, which I think we have now.  As I say now that we have that
> understanding I don't see any reason to block merge.
>
> It's unfortunate that I only noticed that this was actually wakelocks
> very late in the day but I think I can get an implementation which
> handles paths that ignore suspends done quickly.

Do you mean an implementation of embedded linux audio or an
implementation of suspend blockers "which handles paths that ignore
suspends"?  I assume you mean the former, but maybe I misunderstand.

We've done a lot of work around multicore SoCs where the baseband
generally owns a lot of the audio routing and so on, but we do as
general policy not disable audio, codecs, and amps while playback or
record is underway.  I suppose for environments more like a pc laptop
where the expectation is the world stops when you close the lid a
different policy would be preferable.

We typically fire up codecs and amps when playback starts (if they
were off), and usually set a timer for a couple seconds after playback
stops to spin them down (since you tend to have to delay while they're
powering up to avoid pops, etc, and if the system just played a short
sound there's a reasonable chance it'll follow that with another).

Apologies about the name confusion -- last year when we first started
pushing these patches out there was much dislike for the name wakelock
and after a lot of back and forth suspend_block ended up as the least
hated of the suggested alternatives (I kinda like "wakelock" as a
name, but maybe that's just brain rot after dealing with 'em for four
years now).

Brian

Re: [PATCH 0/8] Suspend block api (version 6)

[Mark Brown]

On Fri, May 07, 2010 at 05:37:21AM -0700, Brian Swetland wrote:
> On Fri, May 7, 2010 at 5:25 AM, Mark Brown

> > It's unfortunate that I only noticed that this was actually wakelocks
> > very late in the day but I think I can get an implementation which
> > handles paths that ignore suspends done quickly.

> Do you mean an implementation of embedded linux audio or an
> implementation of suspend blockers "which handles paths that ignore
> suspends"?  I assume you mean the former, but maybe I misunderstand.

I mean that I will extend the embedded audio subsystem that the kernel
already has (ASoC, in sound/soc) to support ignoring suspends for some
audio paths so that they can be kept up during suspend.  This will be
primarily intended for use with opportunistic suspend but not specific
to it.

I have no intention of touching suspend blockers, except in that some of
the drivers I'm responsible for should probably use suspend blockers for
similar reasons to the other users you're merging but that's less urgent.

> We've done a lot of work around multicore SoCs where the baseband
> generally owns a lot of the audio routing and so on,

Multicore isn't really the term here - obviously on something like the
OMAP4 or the current nVidia devices you've got a multicore AP but may or
may not have an integrated baseband.  The distinction is down to how
much visibility the AP has of the audio hardware, which is in general
orthogonal to how the silicon is split and packaged.

>                                                      but we do as
> general policy not disable audio, codecs, and amps while playback or
> record is underway.

Yeah, I know.  I have been keeping an eye on the Android stuff, though
none of the audio side has been mainlined yet.

>                      I suppose for environments more like a pc laptop
> where the expectation is the world stops when you close the lid a
> different policy would be preferable.

Yes, quite.  It all depends on what the intention of the user is when
they do a suspend.  Clearly if the suspend was initiated automatically
because the system is apparently idle the intention is different to if
it was initiated because the user performed some explicit action.

> We typically fire up codecs and amps when playback starts (if they
> were off), and usually set a timer for a couple seconds after playback
> stops to spin them down (since you tend to have to delay while they're
> powering up to avoid pops, etc, and if the system just played a short
> sound there's a reasonable chance it'll follow that with another).

This is exactly what ASoC does - the two biggest wins it offers are that
it allows reuse of the code specific to a given piece of silicon on all
boards using that silicon and that it provides automatic runtime power
management in the core by keeping track of which audio paths are live.

There is the slight wrinkle that you don't really want to *fully* power
down devices that don't have ground referenced outputs since they take
a relatively long time to bring their reference level up without audible
artifacts in the output.

Re: [PATCH 0/8] Suspend block api (version 6)

[Mark Brown]

On Fri, May 07, 2010 at 02:30:25PM +0100, Mark Brown wrote:

> I mean that I will extend the embedded audio subsystem that the kernel
> already has (ASoC, in sound/soc) to support ignoring suspends for some
> audio paths so that they can be kept up during suspend.  This will be
> primarily intended for use with opportunistic suspend but not specific
> to it.

Just for the record I've now done an implementation of this which should
show up in -next when it's rebuilt and 2.6.35.  It's not the most
thoroughly tested code I've ever written but I'm fairly happy it'll do
the right thing, especially for analogue basebands.  The functionality
needs to be explicitly requested by machine drivers so there should be
no impact on systems using suspend in a more standard fashion.

This means that there should be even less of an issue merging this from
an audio point of view.

Re: [PATCH 0/8] Suspend block api (version 6)

[Arve Hjønnevåg]

2010/5/7 Mark Brown <broonie@opensource.wolfsonmicro.com>:
> On Fri, May 07, 2010 at 03:57:37AM -0700, Arve Hjønnevåg wrote:
>> 2010/5/7 Mark Brown <broonie@opensource.wolfsonmicro.com>:
>
>> > As discussed elsewhere in the thread a suspend blocker is not desirable
>> > here - the AP is not doing anything useful on a voice call so blocking
>> > the suspend will just waste power unless runtime PM is good enough to
>> > mean opportunistic suspend isn't adding anything anyway.  It will avoid
>> > the immediate issue with loosing audio but it's not really what we want
>> > to happen.
>
>> I was talking about audio from the AP. Why would you ever turn off
>> another core's audio path on suspend?
>
> This goes back to the thing about a full system suspend being a
> sledgehammer which doesn't give enough information about what's going
> on when it's used like this.  As discussed we need a way to know that
> the connections involved are able to stay live during suspend (on a
> large proportion of systems suspend is going to mean that the relevant
> bits of the board will loose power so need to be shut down) and that

Are you trying to use the same driver on systems that can support
audio while suspend and on systems where this is impossible? If audio
cannot work while suspended, supporting opportunistic suspend is easy.
You block suspend while audio is active.

If the hardware does support some audio paths while suspended, I'll
ask again, is there a reason to turn them off on suspend?

> that the intention of the user is that they should do so (this isn't
> clear in the general system, especially not if the suspend is initiated
> manually).
>
> With a runtime PM approach this is trivial - we just turn off anything
> that isn't in use at the current time.  I'll need to extend ASoC to add
> information about what to do on suspend to the existing power data.
>
OK, but is this at all related to opportunistic suspend?

-- 
Arve Hjønnevåg

Re: [PATCH 0/8] Suspend block api (version 6)

[Mark Brown]

On Fri, May 07, 2010 at 04:41:02AM -0700, Arve Hjønnevåg wrote:
> 2010/5/7 Mark Brown <broonie@opensource.wolfsonmicro.com>:

> > This goes back to the thing about a full system suspend being a
> > sledgehammer which doesn't give enough information about what's going
> > on when it's used like this.  As discussed we need a way to know that
> > the connections involved are able to stay live during suspend (on a
> > large proportion of systems suspend is going to mean that the relevant
> > bits of the board will loose power so need to be shut down) and that

> Are you trying to use the same driver on systems that can support
> audio while suspend and on systems where this is impossible? If audio

Yes, each chip in the audio subsystem gets its own device driver.  All
Linux systems using a given CODEC share the device specific driver code
(modulo out of tree code, obviously).  Further, the power management is
mostly abstracted out of the individual device drivers into the embedded
audio core so the individual drivers don't really take any decisions
about power for themselves.

> cannot work while suspended, supporting opportunistic suspend is easy.
> You block suspend while audio is active.

Right, but that's not really an interesting case here.

> If the hardware does support some audio paths while suspended, I'll
> ask again, is there a reason to turn them off on suspend?

As the mail you're replying to says:

> > that the intention of the user is that they should do so (this isn't
> > clear in the general system, especially not if the suspend is initiated
> > manually).

this all comes down to the intention of the user when they initiated the
suspend.

> > With a runtime PM approach this is trivial - we just turn off anything
> > that isn't in use at the current time.  I'll need to extend ASoC to add
> > information about what to do on suspend to the existing power data.

> OK, but is this at all related to opportunistic suspend?

Well, opportunistic suspend is essentially blurring the lines between a
suspend and runtime PM - it's using suspend as part of the automatic
drive down into the lowest possible power state.  I'm just saying that
in a pure runtime PM model this issue doesn't come up since the suspend
semantic is much clearer.

Re: [PATCH 0/8] Suspend block api (version 6)

[Mark Brown]

On Wed, May 05, 2010 at 02:36:10PM -0400, Alan Stern wrote:

> Clearly if there's a call in progress you don't want to shut the codec
> down.  Are there any other circumstances?  Would they vary according to
> whether the suspend was forced or opportunistic?

Aside from things where the CODEC is acting as a wake source (for stuff
like jack detect) which are obviously already handled it's basically
just when you've got an external audio source flowing through the device
which is going to continue to function during suspend.  Things like FM
radios, for example.  I'm not aware of non-audio examples that are use
case specific and don't just involve utterly ignoring AP suspends.

> In short, I'm trying to get at how much information drivers _really_ 
> need to have about the reason for a system suspend.

It's not exactly the *reason* that makes the difference, it's more that
this aggressive use of suspend makes much more apparent a problem which
might exist anyway for this sort of hardware.

When we get runtime PM delviering similar power levels we'll sidestep
the problem since we won't need to do a system wide suspend.

Re: [PATCH 0/8] Suspend block api (version 6)

[Alan Stern]

On Wed, 5 May 2010, Mark Brown wrote:

> > In short, I'm trying to get at how much information drivers _really_ 
> > need to have about the reason for a system suspend.
> 
> It's not exactly the *reason* that makes the difference, it's more that
> this aggressive use of suspend makes much more apparent a problem which
> might exist anyway for this sort of hardware.

Then the underlying problem should be solved -- hopefully in a nice, 
system-independent way.  But I'm still trying to understand exactly 
what that underlying problem _is_.

That means understanding when the codec needs to be shut down and when
it doesn't, and knowing how much of this information is available to
the driver.

> When we get runtime PM delviering similar power levels we'll sidestep
> the problem since we won't need to do a system wide suspend.

One the face of it, a runtime-PM solution would dictate that the
codec's driver ought to turn off the codec whenever the driver thinks
it isn't being used.  Ergo, if the driver didn't know when a call was
in progress, it would use runtime PM to turn off the codec during a
call.

For this reason I don't see how using runtime PM instead of suspend
blockers would solve anything.

Alan Stern

Re: [PATCH 0/8] Suspend block api (version 6)

[Matthew Garrett]

On Wed, May 05, 2010 at 03:20:40PM -0400, Alan Stern wrote:

> One the face of it, a runtime-PM solution would dictate that the
> codec's driver ought to turn off the codec whenever the driver thinks
> it isn't being used.  Ergo, if the driver didn't know when a call was
> in progress, it would use runtime PM to turn off the codec during a
> call.

Well, part of the problem is that right now most of our beliefs about 
imposed constraints tend to be based on what userspace is doing - "Don't 
power down the audio codec when userspace has it open", for instance. 
But that goes away with opportunistic suspend. In most cases you don't 
want the audio codec to stay awake just because userspace was using it 
to make bouncing cow noises, especially if you've just frozen userspace. 
So the problem becomes more complicated than it would otherwise be.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH 0/8] Suspend block api (version 6)

[Alan Stern]

On Wed, 5 May 2010, Matthew Garrett wrote:

> On Wed, May 05, 2010 at 03:20:40PM -0400, Alan Stern wrote:
> 
> > One the face of it, a runtime-PM solution would dictate that the
> > codec's driver ought to turn off the codec whenever the driver thinks
> > it isn't being used.  Ergo, if the driver didn't know when a call was
> > in progress, it would use runtime PM to turn off the codec during a
> > call.
> 
> Well, part of the problem is that right now most of our beliefs about 
> imposed constraints tend to be based on what userspace is doing - "Don't 
> power down the audio codec when userspace has it open", for instance. 
> But that goes away with opportunistic suspend. In most cases you don't 
> want the audio codec to stay awake just because userspace was using it 
> to make bouncing cow noises, especially if you've just frozen userspace. 
> So the problem becomes more complicated than it would otherwise be.

It sounds like the problem can be stated simply enough: At the moment, 
nobody knows when the codec should be powered down!  Userspace might 
have some idea, but even if its ideas are right it has no way of 
communicating them to the kernel.

The power/control sysfs attribute was intended for just that purpose,
although it was aimed at runtime PM rather than system PM.  
Nevertheless, it or something like it could be used.  Of course, there 
would still remain the issue of userspace telling the kernel not to 
power down the codec while making bouncing cow noises -- but at this 
point it's not really a kernel problem any more.

Alan Stern

P.S.: What happens if a suspend occurs while a call is in progress, so
the codec is left powered up, and then the call ends?  Does the system
wake up at that point?  Or does it stay suspended with the codec still
powered, even though it's not needed any more?

Re: [PATCH 0/8] Suspend block api (version 6)

[Mark Brown]

On Wed, May 05, 2010 at 04:04:00PM -0400, Alan Stern wrote:

> P.S.: What happens if a suspend occurs while a call is in progress, so
> the codec is left powered up, and then the call ends?  Does the system
> wake up at that point?  Or does it stay suspended with the codec still
> powered, even though it's not needed any more?

Call end will be a system wake event in pretty much all systems,
userspace will want to do some work when the call goes away anyway.
Anyone who doesn't do that will see the CODEC stay powered, hopefully
the baseband will send it a nice mute signal instead of popping.

Re: [PATCH 0/8] Suspend block api (version 6)

[Rafael J. Wysocki]

On Wednesday 05 May 2010, Alan Stern wrote:
> On Wed, 5 May 2010, Matthew Garrett wrote:
> 
> > On Wed, May 05, 2010 at 03:20:40PM -0400, Alan Stern wrote:
> > 
> > > One the face of it, a runtime-PM solution would dictate that the
> > > codec's driver ought to turn off the codec whenever the driver thinks
> > > it isn't being used.  Ergo, if the driver didn't know when a call was
> > > in progress, it would use runtime PM to turn off the codec during a
> > > call.
> > 
> > Well, part of the problem is that right now most of our beliefs about 
> > imposed constraints tend to be based on what userspace is doing - "Don't 
> > power down the audio codec when userspace has it open", for instance. 
> > But that goes away with opportunistic suspend. In most cases you don't 
> > want the audio codec to stay awake just because userspace was using it 
> > to make bouncing cow noises, especially if you've just frozen userspace. 
> > So the problem becomes more complicated than it would otherwise be.
> 
> It sounds like the problem can be stated simply enough: At the moment, 
> nobody knows when the codec should be powered down!  Userspace might 
> have some idea, but even if its ideas are right it has no way of 
> communicating them to the kernel.
> 
> The power/control sysfs attribute was intended for just that purpose,
> although it was aimed at runtime PM rather than system PM.  
> Nevertheless, it or something like it could be used.  Of course, there 
> would still remain the issue of userspace telling the kernel not to 
> power down the codec while making bouncing cow noises -- but at this 
> point it's not really a kernel problem any more.

We'd need a separate control, I think.  The problem is, though, that the
Android user space, for example, doesn't use the sysfs attributes of devices,
so this control wouldn't be particularly useful for it.

> P.S.: What happens if a suspend occurs while a call is in progress, so
> the codec is left powered up, and then the call ends?  Does the system
> wake up at that point?  Or does it stay suspended with the codec still
> powered, even though it's not needed any more?

I guess you end up telling the user that the call ended, so the system wakes up.
That's only a guess, though.

Rafael

Re: [PATCH 0/8] Suspend block api (version 6)

[Kevin Hilman]

Alan Stern <stern@rowland.harvard.edu> writes:

> On Wed, 5 May 2010, Matthew Garrett wrote:
>
>> On Wed, May 05, 2010 at 03:20:40PM -0400, Alan Stern wrote:
>> 
>> > One the face of it, a runtime-PM solution would dictate that the
>> > codec's driver ought to turn off the codec whenever the driver thinks
>> > it isn't being used.  Ergo, if the driver didn't know when a call was
>> > in progress, it would use runtime PM to turn off the codec during a
>> > call.
>> 
>> Well, part of the problem is that right now most of our beliefs about 
>> imposed constraints tend to be based on what userspace is doing - "Don't 
>> power down the audio codec when userspace has it open", for instance. 
>> But that goes away with opportunistic suspend. In most cases you don't 
>> want the audio codec to stay awake just because userspace was using it 
>> to make bouncing cow noises, especially if you've just frozen userspace. 
>> So the problem becomes more complicated than it would otherwise be.
>
> It sounds like the problem can be stated simply enough: At the moment, 
> nobody knows when the codec should be powered down!  Userspace might 
> have some idea, but even if its ideas are right it has no way of 
> communicating them to the kernel.
>
> The power/control sysfs attribute was intended for just that purpose,
> although it was aimed at runtime PM rather than system PM.  
> Nevertheless, it or something like it could be used.  Of course, there 
> would still remain the issue of userspace telling the kernel not to 
> power down the codec while making bouncing cow noises -- but at this 
> point it's not really a kernel problem any more.

I guess what we're talking about here is a set of per-device
constraints that could be used by both [opportunistic|system] suspend
and runtime PM.  For lack of a better term, per-device PM QoS (as
compared to the current system-wide PM QoS.)

For example, if userspace (or some other device) has communicated that
it has a constraint on the audio HW, then both the suspend path and the
runtime PM path could check those constraints before making a decision
on how to act.  Hopefully the phone app would set a constraint and the
cow-noise app would not.  :)

On OMAP, we keep track of per-device constraints (currently latency
and throughput) in order to make proper run-time PM decicions in the
kernel, but we are realizing that we need a way for userspace to
communicate these constraints as well, so that userspace can make
power vs. performance policy decisions instead of the kernel.

Probably generalizing these into the LDM is the direction to go so
userspace can set constraints on a per-device (or per-class?) basis:

/sys/devices/.../power/constraint/throughput
/sys/devices/.../power/constraint/wakeup_latency
/sys/devices/.../power/constraint/... ?

Kevin

Re: [PATCH 0/8] Suspend block api (version 6)

[Rafael J. Wysocki]

On Thursday 06 May 2010, Kevin Hilman wrote:
> Alan Stern <stern@rowland.harvard.edu> writes:
> 
> > On Wed, 5 May 2010, Matthew Garrett wrote:
> >
> >> On Wed, May 05, 2010 at 03:20:40PM -0400, Alan Stern wrote:
> >> 
> >> > One the face of it, a runtime-PM solution would dictate that the
> >> > codec's driver ought to turn off the codec whenever the driver thinks
> >> > it isn't being used.  Ergo, if the driver didn't know when a call was
> >> > in progress, it would use runtime PM to turn off the codec during a
> >> > call.
> >> 
> >> Well, part of the problem is that right now most of our beliefs about 
> >> imposed constraints tend to be based on what userspace is doing - "Don't 
> >> power down the audio codec when userspace has it open", for instance. 
> >> But that goes away with opportunistic suspend. In most cases you don't 
> >> want the audio codec to stay awake just because userspace was using it 
> >> to make bouncing cow noises, especially if you've just frozen userspace. 
> >> So the problem becomes more complicated than it would otherwise be.
> >
> > It sounds like the problem can be stated simply enough: At the moment, 
> > nobody knows when the codec should be powered down!  Userspace might 
> > have some idea, but even if its ideas are right it has no way of 
> > communicating them to the kernel.
> >
> > The power/control sysfs attribute was intended for just that purpose,
> > although it was aimed at runtime PM rather than system PM.  
> > Nevertheless, it or something like it could be used.  Of course, there 
> > would still remain the issue of userspace telling the kernel not to 
> > power down the codec while making bouncing cow noises -- but at this 
> > point it's not really a kernel problem any more.
> 
> I guess what we're talking about here is a set of per-device
> constraints that could be used by both [opportunistic|system] suspend
> and runtime PM.  For lack of a better term, per-device PM QoS (as
> compared to the current system-wide PM QoS.)
> 
> For example, if userspace (or some other device) has communicated that
> it has a constraint on the audio HW, then both the suspend path and the
> runtime PM path could check those constraints before making a decision
> on how to act.  Hopefully the phone app would set a constraint and the
> cow-noise app would not.  :)
> 
> On OMAP, we keep track of per-device constraints (currently latency
> and throughput) in order to make proper run-time PM decicions in the
> kernel, but we are realizing that we need a way for userspace to
> communicate these constraints as well, so that userspace can make
> power vs. performance policy decisions instead of the kernel.
> 
> Probably generalizing these into the LDM is the direction to go so
> userspace can set constraints on a per-device (or per-class?) basis:
> 
> /sys/devices/.../power/constraint/throughput
> /sys/devices/.../power/constraint/wakeup_latency
> /sys/devices/.../power/constraint/... ?

That sounds reasonable although it may be a challenge to find a set of
universal constraints common to all devices.

Rafael

Re: [PATCH 0/8] Suspend block api (version 6)

[Brian Swetland]

On Wed, May 5, 2010 at 4:03 PM, Kevin Hilman
<khilman@deeprootsystems.com> wrote:
>
> I guess what we're talking about here is a set of per-device
> constraints that could be used by both [opportunistic|system] suspend
> and runtime PM.  For lack of a better term, per-device PM QoS (as
> compared to the current system-wide PM QoS.)
>
> For example, if userspace (or some other device) has communicated that
> it has a constraint on the audio HW, then both the suspend path and the
> runtime PM path could check those constraints before making a decision
> on how to act.  Hopefully the phone app would set a constraint and the
> cow-noise app would not.  :)
>
> On OMAP, we keep track of per-device constraints (currently latency
> and throughput) in order to make proper run-time PM decicions in the
> kernel, but we are realizing that we need a way for userspace to
> communicate these constraints as well, so that userspace can make
> power vs. performance policy decisions instead of the kernel.
>
> Probably generalizing these into the LDM is the direction to go so
> userspace can set constraints on a per-device (or per-class?) basis:
>
> /sys/devices/.../power/constraint/throughput
> /sys/devices/.../power/constraint/wakeup_latency
> /sys/devices/.../power/constraint/... ?

The constraint stuff is definitely something I'd love to talk about in
detail.  It's a problem that I think is common to every SoC I've
worked with.  Having a general solution for this problem (of
specifying and observing various constraints for clock, power, qos,
etc) kernel-wide would seem like a big win.

Might be worth kicking some design ideas around and getting a bunch of
the interested parties together at some of the upcoming linux
conference things this fall on the east coast?

Brian

Re: [PATCH 0/8] Suspend block api (version 6)

[Alan Stern]

On Wed, 5 May 2010, Brian Swetland wrote:

> > Probably generalizing these into the LDM is the direction to go so
> > userspace can set constraints on a per-device (or per-class?) basis:
> >
> > /sys/devices/.../power/constraint/throughput
> > /sys/devices/.../power/constraint/wakeup_latency
> > /sys/devices/.../power/constraint/... ?
> 
> The constraint stuff is definitely something I'd love to talk about in
> detail.  It's a problem that I think is common to every SoC I've
> worked with.  Having a general solution for this problem (of
> specifying and observing various constraints for clock, power, qos,
> etc) kernel-wide would seem like a big win.
> 
> Might be worth kicking some design ideas around and getting a bunch of
> the interested parties together at some of the upcoming linux
> conference things this fall on the east coast?

The Linux Power-Management Summit is scheduled for August 9, at the 
start of the LinuxCon meeting in Boston.  This would make an excellent 
topic for discussion.

Alan Stern

Re: [PATCH 0/8] Suspend block api (version 6)

[Rafael J. Wysocki]

On Thursday 06 May 2010, Alan Stern wrote:
> On Wed, 5 May 2010, Brian Swetland wrote:
> 
> > > Probably generalizing these into the LDM is the direction to go so
> > > userspace can set constraints on a per-device (or per-class?) basis:
> > >
> > > /sys/devices/.../power/constraint/throughput
> > > /sys/devices/.../power/constraint/wakeup_latency
> > > /sys/devices/.../power/constraint/... ?
> > 
> > The constraint stuff is definitely something I'd love to talk about in
> > detail.  It's a problem that I think is common to every SoC I've
> > worked with.  Having a general solution for this problem (of
> > specifying and observing various constraints for clock, power, qos,
> > etc) kernel-wide would seem like a big win.
> > 
> > Might be worth kicking some design ideas around and getting a bunch of
> > the interested parties together at some of the upcoming linux
> > conference things this fall on the east coast?
> 
> The Linux Power-Management Summit is scheduled for August 9, at the 
> start of the LinuxCon meeting in Boston.  This would make an excellent 
> topic for discussion.

Agreed.

Rafael

Re: [PATCH 0/8] Suspend block api (version 6)

[Mark Brown]

On Wed, May 05, 2010 at 06:33:37PM +0100, Matthew Garrett wrote:

> We seem to have ended up managing most of our PM infrastructure 
> iteratively. If the concern is more about best practices than intrinsic 
> incompatibilities, I'd lean towards us being better off merging this now 
> and then working things out over the next few releases as we get a 
> better understanding of the implications. The main thing that we have to 
> get right in the short term is the userspace API - everything else is 
> easier to fix up as we go along.

Right, the big issue for me is that there's likely to be some form of
userspace API visible for controlling this.  I think I can keep it
mostly in-kernel for audio (by providing new APIs to mark some inputs
and outputs as being live during suspend) but need to check.

Re: [PATCH 0/8] Suspend block api (version 6)

[Alan Stern]

On Wed, 5 May 2010, Mark Brown wrote:

> Now, the solution here if we're going to use opportunistic suspend like
> this is to provide some method for the audio subsystem to figure out
> that the system doesn't actually want it to suspend when it gets told do
> do so.  Like I say ideally we'd provide some standard interface in the
> PM subsystem for userspace to communicate this to subsystems and drivers
> so that we've got a joined up story when people run into issues in cases
> like this, though obviously if this goes in then we'll have to put in
> something subsystem or driver specific for affected areas.  I know what
> I'd implement generically for audio, but I've held back since the option
> is fairly messy when not used in conjunction with a deliberate choice to
> use opportunistic suspend and I was expecting a standard mechanism to
> slot into to provide control for userspace.
> 
> In other words, the issue is that we run into situations where we need
> an element of suspend control to go along with the opportunistic suspend
> in order to allow some elements to be kept running - since suspend
> blocking is taken suspend suppression seems like a reasonable term.

Android must already include some solution to this problem.  Why not 
use that solution more generally?

Alan Stern

Re: [PATCH 0/8] Suspend block api (version 6)

[Mark Brown]

On Wed, May 05, 2010 at 11:35:56AM -0400, Alan Stern wrote:
> On Wed, 5 May 2010, Mark Brown wrote:

> > In other words, the issue is that we run into situations where we need
> > an element of suspend control to go along with the opportunistic suspend
> > in order to allow some elements to be kept running - since suspend
> > blocking is taken suspend suppression seems like a reasonable term.

> Android must already include some solution to this problem.  Why not 
> use that solution more generally?

Not so much; at least for audio it's being handled with platform
specific bodges as far as I've seen so far.  If there had been a
standard way of doing this I'd have expected to see it in this patch
series.  The Google systems all use Qualcomm chipsets which aren't
affected by this since the audio CODEC is hidden from Linux and none of
the audio actually uses ALSA at all in the BSPs I looked at.

It's entirely possible I'm missing something here, everything I know has
come from either talking to users or trawling code on the internet.

Re: [PATCH 0/8] Suspend block api (version 6)

[Kevin Hilman]

Arve Hjønnevåg <arve@android.com> writes:

> On Mon, May 3, 2010 at 4:37 PM, Kevin Hilman
> <khilman@deeprootsystems.com> wrote:
>> "Rafael J. Wysocki" <rjw@sisk.pl> writes:
>>

[...]

>>> However, the real question is whether or not the opportunistic suspend feature
>>> is worth adding to the kernel as such and I think it is.
>>>
>>> To me, it doesn't duplicate the runtime PM framework which is aimed at the power
>>> management of individual devices rather than the system as a whole.
>>
>> From the use cases presented, the *usage* of suspend blockers is aimed
>> at power management of individual devices or subsystems, just like
>> usage of runtime PM.
>>
> No, suspend blockers are mostly used to ensure wakeup events are not
> ignored, and to ensure tasks triggered by these wakeup events
> complete.

OK, but my point was that their *usage* is at the level of inidividual
devices and subsystems, just like runtime PM.  Hence, duplicate work.

>> So I still see a large duplication in the usage and the goals of both
>> frameworks.  The goal of both is to always enter lowest-power state
>> except
>>
>>  - if there's activity (runtime PM for devices, CPUidle for CPU)
>>  - if there's a suspend blocker (opportunitic suspend)
>>
>> In addition, it will likely cause duplicate work to be done in
>> drivers.  Presumably, PM aware drivers will want to know if the system
>> is in opportunistic mode.  For example, for many drivers, doing
>> runtime PM may not be worth the effort if the system is in
>> opportunistic mode.
>
> Why? If a device is not in use it should be off regardless of what
> state the rest of the system is in.

Not necessarily.

If a device is not in use, what power state it goes into depends on
many device/subsystem specific things.  For example, recent activity
(timeouts), whether it will be busy soon (pending transfers),
latency/throughput constraints, dependency on other devices, or any
other device/subsystem specific reason.

All of these can be handled with runtime PM.  None of which are taken
into consideration with opportunistic suspend.

Kevin

Re: [PATCH 0/8] Suspend block api (version 6)

[Matthew Garrett]

On Mon, May 03, 2010 at 04:37:22PM -0700, Kevin Hilman wrote:

> Please forgive the ignorance of ACPI (in embedded, we thankfully live
> in magical world without ACPI) but doesn't that already happen with
> CPUidle and C-states?  I think of CPUidle as basically runtime PM for
> the CPU.  IOW, runtime PM manages the devices, CPUidle manages the CPU
> (via C-states), resulting in dynaimc PM for the entire system.  What
> am I missing?

ACPI doesn't provide any functionality for cutting power to most devices 
other than shifting into full system suspend. The number of wakeup 
events available to us on a given machine is usually small and the 
wakeup latency large, so it's not terribly practical to do this 
transparently on most hardware.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH 0/8] Suspend block api (version 6)

[Alan Stern]

On Tue, 4 May 2010, Matthew Garrett wrote:

> On Mon, May 03, 2010 at 04:37:22PM -0700, Kevin Hilman wrote:
> 
> > Please forgive the ignorance of ACPI (in embedded, we thankfully live
> > in magical world without ACPI) but doesn't that already happen with
> > CPUidle and C-states?  I think of CPUidle as basically runtime PM for
> > the CPU.  IOW, runtime PM manages the devices, CPUidle manages the CPU
> > (via C-states), resulting in dynaimc PM for the entire system.  What
> > am I missing?
> 
> ACPI doesn't provide any functionality for cutting power to most devices 
> other than shifting into full system suspend. The number of wakeup 
> events available to us on a given machine is usually small and the 
> wakeup latency large, so it's not terribly practical to do this 
> transparently on most hardware.

Another thing that Kevin is missing: There is more to the system than
the devices and the CPU.  For example: RAM, an embedded controller (on
modern desktop/laptop systems), a power supply, and so on.  Dynamic PM
for the CPU and the devices won't power-down these things, but system
PM will.

Alan Stern

Re: [PATCH 0/8] Suspend block api (version 6)

[Mark Brown]

On Tue, May 04, 2010 at 09:51:39AM -0400, Alan Stern wrote:
> On Tue, 4 May 2010, Matthew Garrett wrote:
> > On Mon, May 03, 2010 at 04:37:22PM -0700, Kevin Hilman wrote:

> > > Please forgive the ignorance of ACPI (in embedded, we thankfully live
> > > in magical world without ACPI) but doesn't that already happen with
> > > CPUidle and C-states?  I think of CPUidle as basically runtime PM for
> > > the CPU.  IOW, runtime PM manages the devices, CPUidle manages the CPU
> > > (via C-states), resulting in dynaimc PM for the entire system.  What
> > > am I missing?

> > ACPI doesn't provide any functionality for cutting power to most devices 
> > other than shifting into full system suspend. The number of wakeup 
> > events available to us on a given machine is usually small and the 
> > wakeup latency large, so it's not terribly practical to do this 
> > transparently on most hardware.

> Another thing that Kevin is missing: There is more to the system than
> the devices and the CPU.  For example: RAM, an embedded controller (on
> modern desktop/laptop systems), a power supply, and so on.  Dynamic PM
> for the CPU and the devices won't power-down these things, but system
> PM will.

In an embedded system I'd expect that these other system devices would
fall naturally out through the management of the CPUs and devices - for
example, the drivers for the individual devices could use the regulator
API to manage their supplies and runtime PM is being used to manage CPU
core stuff - or could at least readily be handled in a similar fashion.

This isn't to say that we're there yet from an implementation point of
view, of course.

Re: [PATCH 0/8] Suspend block api (version 6)

[Kevin Hilman]

Alan Stern <stern@rowland.harvard.edu> writes:

> On Tue, 4 May 2010, Matthew Garrett wrote:
>
>> On Mon, May 03, 2010 at 04:37:22PM -0700, Kevin Hilman wrote:
>> 
>> > Please forgive the ignorance of ACPI (in embedded, we thankfully live
>> > in magical world without ACPI) but doesn't that already happen with
>> > CPUidle and C-states?  I think of CPUidle as basically runtime PM for
>> > the CPU.  IOW, runtime PM manages the devices, CPUidle manages the CPU
>> > (via C-states), resulting in dynaimc PM for the entire system.  What
>> > am I missing?
>> 
>> ACPI doesn't provide any functionality for cutting power to most devices 
>> other than shifting into full system suspend. The number of wakeup 
>> events available to us on a given machine is usually small and the 
>> wakeup latency large, so it's not terribly practical to do this 
>> transparently on most hardware.
>
> Another thing that Kevin is missing: There is more to the system than
> the devices and the CPU.  For example: RAM, an embedded controller (on
> modern desktop/laptop systems), a power supply, and so on.  Dynamic PM
> for the CPU and the devices won't power-down these things, but system
> PM will.

I consider all of those things devices.

On non-ACPI systems where the kernel has to manage all of the above
directly, we have drivers for all of them using runtime PM as well as
the regulator framework for dynamic PM power supplies.

Kevin

Re: [PATCH 0/8] Suspend block api (version 6)

[Matthew Garrett]

On Tue, May 04, 2010 at 09:51:39AM -0400, Alan Stern wrote:

> Another thing that Kevin is missing: There is more to the system than
> the devices and the CPU.  For example: RAM, an embedded controller (on
> modern desktop/laptop systems), a power supply, and so on.  Dynamic PM
> for the CPU and the devices won't power-down these things, but system
> PM will.

Somewhat true - machines with C1E support will put the RAM into self 
refresh when the cpu is idle, but you're right that there are various 
components that we simply don't have control over in the desktop world 
at present.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH 0/8] Suspend block api (version 6)

[Magnus Damm]

On Wed, May 5, 2010 at 12:27 AM, Matthew Garrett <mjg@redhat.com> wrote:
> On Tue, May 04, 2010 at 09:51:39AM -0400, Alan Stern wrote:
>
>> Another thing that Kevin is missing: There is more to the system than
>> the devices and the CPU.  For example: RAM, an embedded controller (on
>> modern desktop/laptop systems), a power supply, and so on.  Dynamic PM
>> for the CPU and the devices won't power-down these things, but system
>> PM will.
>
> Somewhat true - machines with C1E support will put the RAM into self
> refresh when the cpu is idle, but you're right that there are various
> components that we simply don't have control over in the desktop world
> at present.

As a contrast, at least on some embedded SoCs without firmware
limitations the low level sleep code for Runtime PM / CPU Idle is
shared with system wide suspend. So the RAM is put into self refresh
regardless of entering a half-deep CPU Idle state or entering
suspend-to-memory.

I've heard that newer SH-Mobile ARM hardware monitors the memory bus
activity behind the scenes and can put the system RAM into self
refresh automatically. On older parts we had to manage that from CPU
idle context.

/ magnus

Re: [PATCH 0/8] Suspend block api (version 6)

[Kevin Hilman]

Matthew Garrett <mjg@redhat.com> writes:

> On Mon, May 03, 2010 at 04:37:22PM -0700, Kevin Hilman wrote:
>
>> Please forgive the ignorance of ACPI (in embedded, we thankfully live
>> in magical world without ACPI) but doesn't that already happen with
>> CPUidle and C-states?  I think of CPUidle as basically runtime PM for
>> the CPU.  IOW, runtime PM manages the devices, CPUidle manages the CPU
>> (via C-states), resulting in dynaimc PM for the entire system.  What
>> am I missing?
>
> ACPI doesn't provide any functionality for cutting power to most
> devices other than shifting into full system suspend. The number of
> wakeup events available to us on a given machine is usually small
> and the wakeup latency large, so it's not terribly practical to do
> this transparently on most hardware.

OK, that's a major difference with embedded SoCs where the kernel must
directly manage the power state of all devices using runtime PM.

So basically, on ACPI systems, runtime PM doesn't get you any power
savings for most devices.  

Kevin

Re: [PATCH 0/8] Suspend block api (version 6)

[Matthew Garrett]

On Tue, May 04, 2010 at 08:13:09AM -0700, Kevin Hilman wrote:

> So basically, on ACPI systems, runtime PM doesn't get you any power
> savings for most devices.  

I'd say it does for most devices, but the power savings may not be as 
great as they would be with fine-grained control over power rails.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH 0/8] Suspend block api (version 6)

[Matthew Garrett]

On Mon, May 03, 2010 at 09:40:26AM -0700, Kevin Hilman wrote:

> In my view, the truly significant difference between suspend blockers
> and runtime PM is what happens to userspace.  So far, to me the only
> compelling argument for suspend blockers is the goal of forcibly
> shutting down userspace and thus forcing the system into idle
> (although drivers could still reject a suspend request.)

I'd say that this is certainly the main issue, though the remaining 
periodic timers in the kernel are also inconvenient.

> And if untrusted userspace apps remain as the major problem, maybe we
> should aim for a solution directly targetting that problem.  I'm just
> shooting from the hip now, but maybe containing (cgroups?) untrusted
> processes together into a set that could be frozen/idled so that runtime PM
> would be more effective would be a workable solution?

I considered this. The problem is that not all of your wakeup events 
pass through trusted code. Assume we've used a freezer cgroup and the 
applications are now frozen. One of them is blocking on a network 
socket. A packet arrives and triggers a wakeup of the hardware. How do 
we unfreeze the userspace app?

I agree that the runtime scenario is a far more appealing one from an 
aesthetic standpoint, but so far we don't have a very compelling 
argument for dealing with the starting and stopping of userspace. The 
use-cases that Google have provided are valid and they have an 
implementation that addresses them, and while we're unable to provide an 
alternative that provides the same level of functionality I think we're 
in a poor position to prevent this from going in.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH 0/8] Suspend block api (version 6)

[Pavel Machek]

Hi!

> I agree that the runtime scenario is a far more appealing one from an 
> aesthetic standpoint, but so far we don't have a very compelling 
> argument for dealing with the starting and stopping of userspace. The 
> use-cases that Google have provided are valid and they have an 
> implementation that addresses them, and while we're unable to provide an 
> alternative that provides the same level of functionality I think we're 
> in a poor position to prevent this from going in.

Uhuh?

"We have this ugly code here, but it works and we don't have better
one, so lets merge it"?

I don't really like this line of reasoning. I would not want to judge
wakelocks here, but... "it works, merge it" should not be used as
argument.

And btw I do have wakelock-less implementation of autosleep, that only
sleeped the machine when nothing was ready to run. It was called
"sleepy linux". Should I dig it out?

Major difference was that it only sleeped the machine when it was
absolutely certain machine is idle and no timers are close to firing
-- needing elimination or at least markup of all short timers. It
erred on side of not sleeping the machine when it would break
something.

Still I believe it is better design than wakelocks -- that need
markup/fixes to all places where machine must not sleep -- effectively
sleeping the machine too often than fixing stuff with wakelocks all
over kernel and userspace...
								Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH 0/8] Suspend block api (version 6)

[Matthew Garrett]

On Mon, May 24, 2010 at 08:57:54PM +0200, Pavel Machek wrote:

> And btw I do have wakelock-less implementation of autosleep, that only
> sleeped the machine when nothing was ready to run. It was called
> "sleepy linux". Should I dig it out?

As has been explained several times now, that's not equivalent. The 
difference is what makes this a hard problem. If a usecase is valid and 
nobody can come up with a convincing explanation of what an elegant 
solution would look like, then the ugly solution wins.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH 0/8] Suspend block api (version 6)

[Arve Hjønnevåg]

On Mon, May 24, 2010 at 11:57 AM, Pavel Machek <pavel@ucw.cz> wrote:
> Hi!
>
>> I agree that the runtime scenario is a far more appealing one from an
>> aesthetic standpoint, but so far we don't have a very compelling
>> argument for dealing with the starting and stopping of userspace. The
>> use-cases that Google have provided are valid and they have an
>> implementation that addresses them, and while we're unable to provide an
>> alternative that provides the same level of functionality I think we're
>> in a poor position to prevent this from going in.
>
> Uhuh?
>
> "We have this ugly code here, but it works and we don't have better
> one, so lets merge it"?
>
> I don't really like this line of reasoning. I would not want to judge
> wakelocks here, but... "it works, merge it" should not be used as
> argument.
>
> And btw I do have wakelock-less implementation of autosleep, that only
> sleeped the machine when nothing was ready to run. It was called
> "sleepy linux". Should I dig it out?
>
> Major difference was that it only sleeped the machine when it was
> absolutely certain machine is idle and no timers are close to firing
> -- needing elimination or at least markup of all short timers. It
> erred on side of not sleeping the machine when it would break
> something.
>

How did you handle external events that occur right after you decided to sleep?

> Still I believe it is better design than wakelocks -- that need
> markup/fixes to all places where machine must not sleep -- effectively
> sleeping the machine too often than fixing stuff with wakelocks all
> over kernel and userspace...

-- 
Arve Hjønnevåg

Re: [PATCH 0/8] Suspend block api (version 6)

[Pavel Machek]

On Mon 2010-05-24 18:16:15, Arve Hj?nnev?g wrote:
> On Mon, May 24, 2010 at 11:57 AM, Pavel Machek <pavel@ucw.cz> wrote:
> > Hi!
> >
> >> I agree that the runtime scenario is a far more appealing one from an
> >> aesthetic standpoint, but so far we don't have a very compelling
> >> argument for dealing with the starting and stopping of userspace. The
> >> use-cases that Google have provided are valid and they have an
> >> implementation that addresses them, and while we're unable to provide an
> >> alternative that provides the same level of functionality I think we're
> >> in a poor position to prevent this from going in.
> >
> > Uhuh?
> >
> > "We have this ugly code here, but it works and we don't have better
> > one, so lets merge it"?
> >
> > I don't really like this line of reasoning. I would not want to judge
> > wakelocks here, but... "it works, merge it" should not be used as
> > argument.
> >
> > And btw I do have wakelock-less implementation of autosleep, that only
> > sleeped the machine when nothing was ready to run. It was called
> > "sleepy linux". Should I dig it out?
> >
> > Major difference was that it only sleeped the machine when it was
> > absolutely certain machine is idle and no timers are close to firing
> > -- needing elimination or at least markup of all short timers. It
> > erred on side of not sleeping the machine when it would break
> > something.
> >
> 
> How did you handle external events that occur right after you decided to sleep?

I decided to go to sleep with interrupts disabled... it was prototype
on x86, so it was rather tricky.

I'd expect external events after sleep decision to just wakeup machine
immediately, similar to entering deep CPU sleep mode...
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH 0/8] Suspend block api (version 6)

[mark gross]

On Mon, May 03, 2010 at 09:40:26AM -0700, Kevin Hilman wrote:
> Arve Hjønnevåg <arve@android.com> writes:
> 
> > This patch series adds a suspend-block api that provides the same
> > functionality as the android wakelock api. This version fixes a race
> > in suspend blocking work, has some documentation changes and
> > opportunistic suspend now uses the same workqueue as runtime pm.
> 
> Earlier this month, several folks intersted in embedded PM had a BoF
> as part of the Embedded Linux Conference[1] in San Francisco.  Many of
> us had concerns about wakelocks/suspend-blockers and I wanted to share
> some of mine here, since I don't know if embedded folks (other than
> Google) were included in discussions during the LF Collab summmit.
> 
> I hope other embedded folks will chime in here as well.  My background
> is in embedded as one of the kernel developers on the TI OMAP SoCs,
> and I work primarily on PM stuff.
> 
> My comments are not about this implementation of suspend blockers in
> particular, but rather on the potential implications of suspend
> blockers in general.  

I also think we need to take a hard look at the process here. 

--mgross

> Sorry for the lengthy mail, it's broken up in to 3 parts:
> 
> - suspend blockers vs. runtime PM
> - how to handle PM aware drivers?
> - what about dumb or untrusted apps
> 
> 
> Suspend blockers vs runtime PM
> ------------------------------
> 
> My primary concern is that suspend blockers attempt to address the
> same problem(s) as runtime PM, but with a very different approach.
> Suspend blockers use one very large hammer whereas runtime PM hands
> out many little hammers.  Since I believe power management to be a
> problem of many little nails, I think many little hammers are better
> suited for the job.
> 
> Currently in the kernel, we have two main forms of PM
> 
> - static PM (system PM, traditional suspend/resume etc.)
> - dynamic PM (runtime PM, CPUfreq, CPUidle, etc.)
> 
> And with the addition of suspend blockers we have something in
> between.  In my simple world, I think of suspend_blockers as static PM
> with a retrofit of some basic dynamic capabilities.  In my view, a
> poor man's dynamic PM.
> 
> The current design of suspend blockers was (presumably) taken due to
> major limitations and/or problems in dynamic PM when it was designed.
> However, since then, some very signifcant improvements in dynamic PM
> have come along, particularily in the form of runtime PM.  What I
> still feel is missing from this discussion are details about why the
> issues addressed by suspend blockers cannot be solved with runtime PM.
> 
> It seems to me the keypad/screen example given in the doc can very
> easily be solved with runtime PM.  The goal of that example is that
> the keypad not turn on the screen unless a specific key is pressed.
> That is rather easy to accomplish using runtime PM:
>  
> 1. system is idle, all devices/drivers runtime suspended
>    (display and keypad drivers are both runtime suspended)
> -  keypress triggers wakeup ->runtime_resume() of keypad (screen is
>    still runtime suspended)
> -  key press trickles up to userspace
> -  keypad driver is done, goes idle and is runtime supended
> -  userspace decides whether or not to turn on screen based on key
> -  if not, goto 1, (display is still runtime suspended)
> -  if so, start using display and it will be runtime resumed
> 
> I realize this keypad example was only one example usage of suspend
> blockers, but I suspect the others would be solved similarily using
> runtime PM.
> 
> But anyways, to get back to the main point: 
> 
> I feel the main problems tackled by _kernel_ suspend blockers (as I
> understand them) are the same problems already addressed by runtime
> PM.  First and formost, both have the same guiding principle:
> 
>   Rule #1: Always try for lowest power state, unless X
> 
> For runtime PM, X = "activity"
> For suspend blockers, X = a held suspend_blocker
> 
> In addition, both have the same secondary goals:
> 
> - keep device PM independent of other devices (e.g. don't wake up
>   screen just because keypad was pressed)
> 
> - wakeups/events can be handled in a device specific way, without
>   affecting other devices or rest of the system, unless desired
> 
> So, the goals are the same, but the approaches are different.  Runtime
> PM makes each of the drivers and subsystems do the work, where suspend
> blockers just forces the issue from on high.  IMHO, the more flexible
> and generic approach of runtime PM is more suited to a general purpose
> kernel than the one-big-hammer approach currently taken by suspend
> blockers.
> 
> 
> What about PM aware drivers?
> ----------------------------
> 
> All of this brings up a second major concern regarding how to write PM
> aware drivers.
> 
> At least from the kernel perspective, both suspend blockers and
> runtime PM have the same goal.  Given that, which framework should the
> driver writer target?  Both?  Seems like duplicate effort.  Using
> suspend blockers assumes the system is in opportunitstic suspend mode
> and (at least in the keypad example given) assumes a suspend-blocker
> aware userspace (Android.) Without both, targeted power savings will
> not be acheived.
> 
> To me, runtime PM is a generic and flexible approach that can be used
> with any userspace.  Driver writers should not have to care whether
> the system is in "opportunistic" mode or about whether userspace is
> suspend blocker capable.  They should only have to think about when
> the device is (or should be) idle.
> 
> >From my experience with OMAP, we *really* do not want to care about
> what userspace is or isn't capable of, or what suspend-mode the kernel
> is in.  Among other things, the OMAP linux kernel is used in the Nokia
> N900 (Maemo), the Motorola Droid (Android) and the Palm Pre (webOS).
> Comments on the future of each SW stack aside, we really want to run
> the same kernel and drivers across all of those platforms as well as
> whatever comes next.
> 
> 
> What about dumb or untrusted apps?
> ---------------------------------------
> 
> In my view, the truly significant difference between suspend blockers
> and runtime PM is what happens to userspace.  So far, to me the only
> compelling argument for suspend blockers is the goal of forcibly
> shutting down userspace and thus forcing the system into idle
> (although drivers could still reject a suspend request.)
> 
> Again, since suspend blockers were designed, there have been major
> efforts to track and fix userspace as well as underlying timer issues
> (deferrable timers, coalescing, timer slack ...) that led to
> unnecessary wakeups from userspace.  Wouldn't it be better to spend
> our collective efforts in continuing in that direction instead of just
> hiding the underlying problems by forcing suspend?  Fixing the root
> causes will be better for everyone, not just those using Android.
> 
> And if untrusted userspace apps remain as the major problem, maybe we
> should aim for a solution directly targetting that problem.  I'm just
> shooting from the hip now, but maybe containing (cgroups?) untrusted
> processes together into a set that could be frozen/idled so that runtime PM
> would be more effective would be a workable solution?
> 
> Anyways, that's enough rambling for now.  I hope that sheds some light
> on the concerns I have with suspend blockers.
> 
> Kevin
> 
> [1] http://embeddedlinuxconference.com/elc_2010/index.html

Re: [PATCH 0/8] Suspend block api (version 6)

[Kevin Hilman]

Hello,

I think many folks are still confused about exactly the problem being
solved by this series as well as mixed up between opportunistic
suspend and suspend blockers.  Also, how this series impatcs the rest
of the kernel (especially PM-aware drivers and subsystems) has caused
a bit of confusion.

To help with the confusion, I think a much clearer description of the
problem being solved and the proposed solution is needed.  

To that end, I created a starting point for that below which
summarizes how I understand the problem and the proposed solution, but
of course this should be filled out in more detail and updated as part
of the documentation that goes with this series.

Hope this helps improve the understanding of this feature,

Kevin



Table of Contents
=================
1 Problem Statement 
2 Solution: Opportunistic suspend 
    2.1 When to use a suspend blocker 
    2.2 Usage in PM aware drivers 


1 Problem Statement 
~~~~~~~~~~~~~~~~~~~~

Want to to hit deep power state, even when the system is not actually
idle.

Why?

- some hardware is not capable of deep power states in idle
- difficulty getting userspace and/or kernel to be idle

2 Solution: Opportunistic suspend 
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Create an additional "idle path" which has new rules for determining
idleness.  When this new "idle" is reached, trigger full-system
suspend.  Since a suspend is triggered whenever the opportunity
arises, this is called opportunistic suspend.

The new rules for making the idleness decision are simple:

1.  system may suspend if and only if no suspend blockers are held

2.1 When to use a suspend blocker 
==================================

[A list of reasons why suspend blockers might be used would be very
 helpful here.] 

- ensure wakeup events propagate to userspace (e.g. keypad example in doc)

- screen is on

- someone mentioned "Google use cases"
  (would be nice to hear about more of these)

2.2 Usage in PM aware drivers 
==============================

[An example of how a driver already using runtime PM would use 
 a suspend blocker would also be helpful.

Re: [PATCH 0/8] Suspend block api (version 6)

[Rafael J. Wysocki]

On Monday 10 May 2010, Kevin Hilman wrote:
> Hello,

Hi Kevin,

> I think many folks are still confused about exactly the problem being
> solved by this series as well as mixed up between opportunistic
> suspend and suspend blockers.  Also, how this series impatcs the rest
> of the kernel (especially PM-aware drivers and subsystems) has caused
> a bit of confusion.
> 
> To help with the confusion, I think a much clearer description of the
> problem being solved and the proposed solution is needed.  
> 
> To that end, I created a starting point for that below which
> summarizes how I understand the problem and the proposed solution, but
> of course this should be filled out in more detail and updated as part
> of the documentation that goes with this series.
> 
> Hope this helps improve the understanding of this feature,

Yes, I think this is helpful.

> Table of Contents
> =================
> 1 Problem Statement 
> 2 Solution: Opportunistic suspend 
>     2.1 When to use a suspend blocker 
>     2.2 Usage in PM aware drivers 
> 
> 
> 1 Problem Statement 
> ~~~~~~~~~~~~~~~~~~~~
> 
> Want to to hit deep power state, even when the system is not actually
> idle.
> 
> Why?
> 
> - some hardware is not capable of deep power states in idle
> - difficulty getting userspace and/or kernel to be idle
> 
> 2 Solution: Opportunistic suspend 
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Create an additional "idle path" which has new rules for determining
> idleness.  When this new "idle" is reached, trigger full-system
> suspend.  Since a suspend is triggered whenever the opportunity
> arises, this is called opportunistic suspend.
> 
> The new rules for making the idleness decision are simple:
> 
> 1.  system may suspend if and only if no suspend blockers are held
> 
> 2.1 When to use a suspend blocker 
> ==================================
> 
> [A list of reasons why suspend blockers might be used would be very
>  helpful here.] 
> 
> - ensure wakeup events propagate to userspace (e.g. keypad example in doc)
> 
> - screen is on
> 
> - someone mentioned "Google use cases"
>   (would be nice to hear about more of these)

Yes, I think the Android developers know quite a few cases where suspend
blockers are useful.

> 2.2 Usage in PM aware drivers 
> ==============================
> 
> [An example of how a driver already using runtime PM would use 
>  a suspend blocker would also be helpful.

When we have any drivers using both in the tree, they will be used as examples
here.

Thanks,
Rafael

Re: [PATCH 0/8] Suspend block api (version 6)

[Tony Lindgren]

* Rafael J. Wysocki <rjw@sisk.pl> [100510 13:27]:
> On Monday 10 May 2010, Kevin Hilman wrote:
> > Hello,
> 
> Hi Kevin,
> 
> > I think many folks are still confused about exactly the problem being
> > solved by this series as well as mixed up between opportunistic
> > suspend and suspend blockers.  Also, how this series impatcs the rest
> > of the kernel (especially PM-aware drivers and subsystems) has caused
> > a bit of confusion.
> > 
> > To help with the confusion, I think a much clearer description of the
> > problem being solved and the proposed solution is needed.  
> > 
> > To that end, I created a starting point for that below which
> > summarizes how I understand the problem and the proposed solution, but
> > of course this should be filled out in more detail and updated as part
> > of the documentation that goes with this series.
> > 
> > Hope this helps improve the understanding of this feature,
> 
> Yes, I think this is helpful.
> 
> > Table of Contents
> > =================
> > 1 Problem Statement 
> > 2 Solution: Opportunistic suspend 
> >     2.1 When to use a suspend blocker 
> >     2.2 Usage in PM aware drivers 
> > 
> > 
> > 1 Problem Statement 
> > ~~~~~~~~~~~~~~~~~~~~
> > 
> > Want to to hit deep power state, even when the system is not actually
> > idle.
> > 
> > Why?
> > 
> > - some hardware is not capable of deep power states in idle
> > - difficulty getting userspace and/or kernel to be idle
> > 
> > 2 Solution: Opportunistic suspend 
> > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > 
> > Create an additional "idle path" which has new rules for determining
> > idleness.  When this new "idle" is reached, trigger full-system
> > suspend.  Since a suspend is triggered whenever the opportunity
> > arises, this is called opportunistic suspend.

I agree, this is is the right way to handle when to enter suspend.

Especially if the system needs to run while waiting for something
to happen before being able to suspend.

> > The new rules for making the idleness decision are simple:
> > 
> > 1.  system may suspend if and only if no suspend blockers are held

To me it sounds like this should only be allowed to happen when you do:

# echo 1 > /sys/power/suspend_while_idle

As it kills the timers and leads to non-standard behaviour of the apps
as they won't run :)

And then the remaining question is how to make sure the use cases
below can be handled in a clean way.
 
> > 2.1 When to use a suspend blocker 
> > ==================================
> > 
> > [A list of reasons why suspend blockers might be used would be very
> >  helpful here.] 
> > 
> > - ensure wakeup events propagate to userspace (e.g. keypad example in doc)
> > 
> > - screen is on
> > 
> > - someone mentioned "Google use cases"
> >   (would be nice to hear about more of these)
> 
> Yes, I think the Android developers know quite a few cases where suspend
> blockers are useful.
> 
> > 2.2 Usage in PM aware drivers 
> > ==============================
> > 
> > [An example of how a driver already using runtime PM would use 
> >  a suspend blocker would also be helpful.
> 
> When we have any drivers using both in the tree, they will be used as examples
> here.
> 
> Thanks,
> Rafael
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@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: [PATCH 0/8] Suspend block api (version 6)

[Matthew Garrett]

On Tue, May 11, 2010 at 09:12:28AM -0700, Tony Lindgren wrote:

> To me it sounds like this should only be allowed to happen when you do:
> 
> # echo 1 > /sys/power/suspend_while_idle
> 
> As it kills the timers and leads to non-standard behaviour of the apps
> as they won't run :)
> 
> And then the remaining question is how to make sure the use cases
> below can be handled in a clean way.

That's handled by the /sys/power/policy opportunistic/forced switch.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH 0/8] Suspend block api (version 6)

[Tony Lindgren]

* Matthew Garrett <mjg@redhat.com> [100511 09:10]:
> On Tue, May 11, 2010 at 09:12:28AM -0700, Tony Lindgren wrote:
> 
> > To me it sounds like this should only be allowed to happen when you do:
> > 
> > # echo 1 > /sys/power/suspend_while_idle
> > 
> > As it kills the timers and leads to non-standard behaviour of the apps
> > as they won't run :)
> > 
> > And then the remaining question is how to make sure the use cases
> > below can be handled in a clean way.
> 
> That's handled by the /sys/power/policy opportunistic/forced switch.

OK, so can the suspend blocker then become just:

# Block suspend while idle, system stays running
# echo default > /sys/power/policy

and the when it's OK to suspend:

# Allow suspend while idle, system suspends when it hits kernel idle loop
# echo opportunistic > /sys/power/policy

or do you still need something more to ensure the data gets into your
app and be handled?

The part I really don't like is the idea of patching all over the drivers
and userspace for the wakelocks/suspendblocks.

Regards,

Tony

Re: [PATCH 0/8] Suspend block api (version 6)

[Matthew Garrett]

On Tue, May 11, 2010 at 09:36:33AM -0700, Tony Lindgren wrote:
> OK, so can the suspend blocker then become just:
> 
> # Block suspend while idle, system stays running
> # echo default > /sys/power/policy
> 
> and the when it's OK to suspend:
> 
> # Allow suspend while idle, system suspends when it hits kernel idle loop
> # echo opportunistic > /sys/power/policy
> 
> or do you still need something more to ensure the data gets into your
> app and be handled?

Yes. You still need suspend blocks.

> The part I really don't like is the idea of patching all over the drivers
> and userspace for the wakelocks/suspendblocks.

I don't like the idea either, but given that nobody has actually 
provided any other ideas that would actually work then I don't think 
we've got a great deal of choice.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH 0/8] Suspend block api (version 6)

[Tony Lindgren]

* Matthew Garrett <mjg@redhat.com> [100511 09:41]:
> On Tue, May 11, 2010 at 09:36:33AM -0700, Tony Lindgren wrote:
> > OK, so can the suspend blocker then become just:
> > 
> > # Block suspend while idle, system stays running
> > # echo default > /sys/power/policy
> > 
> > and the when it's OK to suspend:
> > 
> > # Allow suspend while idle, system suspends when it hits kernel idle loop
> > # echo opportunistic > /sys/power/policy
> > 
> > or do you still need something more to ensure the data gets into your
> > app and be handled?
> 
> Yes. You still need suspend blocks.

Maybe not if echo opportunistic > /sys/power/policy gets cleared by
the kernel if the kernel idle loop can't make it. That means something
has blocked the suspend attempt in a driver for example. The system
keeps running, and the userspace can deal with the situation.
 
> > The part I really don't like is the idea of patching all over the drivers
> > and userspace for the wakelocks/suspendblocks.
> 
> I don't like the idea either, but given that nobody has actually 
> provided any other ideas that would actually work then I don't think 
> we've got a great deal of choice.

If the opportunistic kernel flag is one time attempt only, then you
could take care of the wakelock/suspendblock handling in the userspace
completely. And once the userspace wakelock/suspendblock is cleared,
the userspace can again echo opportunistic > /sys/power/policy.

Regards,

Tony

Re: [PATCH 0/8] Suspend block api (version 6)

[Matthew Garrett]

On Tue, May 11, 2010 at 09:58:21AM -0700, Tony Lindgren wrote:
> * Matthew Garrett <mjg@redhat.com> [100511 09:41]:
> > Yes. You still need suspend blocks.
> 
> Maybe not if echo opportunistic > /sys/power/policy gets cleared by
> the kernel if the kernel idle loop can't make it. That means something
> has blocked the suspend attempt in a driver for example. The system
> keeps running, and the userspace can deal with the situation.

So an event arrives just as userspace does this write. How do you avoid 
the race? Plausible answers mostly appear to end up looking like suspend 
blockers.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH 0/8] Suspend block api (version 6)

[Tony Lindgren]

* Matthew Garrett <mjg@redhat.com> [100511 09:59]:
> On Tue, May 11, 2010 at 09:58:21AM -0700, Tony Lindgren wrote:
> > * Matthew Garrett <mjg@redhat.com> [100511 09:41]:
> > > Yes. You still need suspend blocks.
> > 
> > Maybe not if echo opportunistic > /sys/power/policy gets cleared by
> > the kernel if the kernel idle loop can't make it. That means something
> > has blocked the suspend attempt in a driver for example. The system
> > keeps running, and the userspace can deal with the situation.
> 
> So an event arrives just as userspace does this write. How do you avoid 
> the race? Plausible answers mostly appear to end up looking like suspend 
> blockers.

Assuming you attempt suspend in a custom pm_idle function, any driver
handling the event can fail the suspend attempt.

And that would clear the opportunistic suspend flag. And the userspace
would be still running and could handle the event. And when the userspace
is done, it can again echo opportunistic > /sys/power/policy.

For the failed suspend path in the kernel, currently the kernel would
unwind back all the drivers because of the failed driver, but that path
should be possible to optimize.

Regards,

Tony

Re: [PATCH 0/8] Suspend block api (version 6)

[Matthew Garrett]

On Tue, May 11, 2010 at 10:24:43AM -0700, Tony Lindgren wrote:

> For the failed suspend path in the kernel, currently the kernel would
> unwind back all the drivers because of the failed driver, but that path
> should be possible to optimize.

If you think it's possible to make this work then feel free to. But at 
the point where you're adding code to every driver's suspend function to 
determine whether or not it's got any pending events that userspace 
hasn't consumed yet, and adding code to every bit of userspace to allow 
it to indicate whether or not it's busy consuming events or just busy 
drawing 3D bouncing cattle, I think you've reinvented suspend blocks.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH 0/8] Suspend block api (version 6)

[Tony Lindgren]

* Matthew Garrett <mjg@redhat.com> [100511 10:25]:
> On Tue, May 11, 2010 at 10:24:43AM -0700, Tony Lindgren wrote:
> 
> > For the failed suspend path in the kernel, currently the kernel would
> > unwind back all the drivers because of the failed driver, but that path
> > should be possible to optimize.
> 
> If you think it's possible to make this work then feel free to. But at 
> the point where you're adding code to every driver's suspend function to 
> determine whether or not it's got any pending events that userspace 
> hasn't consumed yet, and adding code to every bit of userspace to allow 
> it to indicate whether or not it's busy consuming events or just busy 
> drawing 3D bouncing cattle, I think you've reinvented suspend blocks.

Sorry, I have a working system that idles nicely and stays up on
batteries for a long time while running. I don't need to implement
anything like this :)

Cheers,

Tony

Re: [PATCH 0/8] Suspend block api (version 6)

[Matthew Garrett]

On Tue, May 11, 2010 at 10:48:58AM -0700, Tony Lindgren wrote:
> * Matthew Garrett <mjg@redhat.com> [100511 10:25]:
> > On Tue, May 11, 2010 at 10:24:43AM -0700, Tony Lindgren wrote:
> > 
> > > For the failed suspend path in the kernel, currently the kernel would
> > > unwind back all the drivers because of the failed driver, but that path
> > > should be possible to optimize.
> > 
> > If you think it's possible to make this work then feel free to. But at 
> > the point where you're adding code to every driver's suspend function to 
> > determine whether or not it's got any pending events that userspace 
> > hasn't consumed yet, and adding code to every bit of userspace to allow 
> > it to indicate whether or not it's busy consuming events or just busy 
> > drawing 3D bouncing cattle, I think you've reinvented suspend blocks.
> 
> Sorry, I have a working system that idles nicely and stays up on
> batteries for a long time while running. I don't need to implement
> anything like this :)

Right, but your system will only idle nicely if all of your userspace is 
well-written. It's not reasonable to expect that all userspace will be 
well-written and thus it's necessary to implement a power management 
strategy that doesn't require that. Refusing an implementation that 
achieves that on the basis that there's hypothetically a better way of 
doing it is entirely unreasonable.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH 0/8] Suspend block api (version 6)

[Rafael J. Wysocki]

On Tuesday 11 May 2010, Matthew Garrett wrote:
> On Tue, May 11, 2010 at 10:24:43AM -0700, Tony Lindgren wrote:
> 
> > For the failed suspend path in the kernel, currently the kernel would
> > unwind back all the drivers because of the failed driver, but that path
> > should be possible to optimize.
> 
> If you think it's possible to make this work then feel free to. But at 
> the point where you're adding code to every driver's suspend function to 
> determine whether or not it's got any pending events that userspace 
> hasn't consumed yet, and adding code to every bit of userspace to allow 
> it to indicate whether or not it's busy consuming events or just busy 
> drawing 3D bouncing cattle, I think you've reinvented suspend blocks.

Yeah, pretty much so.

Thanks,
Rafael

Re: [PATCH 0/8] Suspend block api (version 6)

[Arve Hjønnevåg]

2010/5/10 Kevin Hilman <khilman@deeprootsystems.com>:
> Hello,
>
> I think many folks are still confused about exactly the problem being
> solved by this series as well as mixed up between opportunistic
> suspend and suspend blockers.  Also, how this series impatcs the rest
> of the kernel (especially PM-aware drivers and subsystems) has caused
> a bit of confusion.
>
> To help with the confusion, I think a much clearer description of the
> problem being solved and the proposed solution is needed.
>
> To that end, I created a starting point for that below which
> summarizes how I understand the problem and the proposed solution, but
> of course this should be filled out in more detail and updated as part
> of the documentation that goes with this series.
>
> Hope this helps improve the understanding of this feature,
>
> Kevin
>
>
>
> Table of Contents
> =================
> 1 Problem Statement
> 2 Solution: Opportunistic suspend
>    2.1 When to use a suspend blocker
>    2.2 Usage in PM aware drivers
>
>
> 1 Problem Statement
> ~~~~~~~~~~~~~~~~~~~~
>
> Want to to hit deep power state, even when the system is not actually
> idle.
>
> Why?
>
> - some hardware is not capable of deep power states in idle
> - difficulty getting userspace and/or kernel to be idle

There are two parts to this. Suspend freezes tasks that don't idle at
all, and it stops the monotonic clock which in turn stops tasks that
do (frequent) periodic work.

>
> 2 Solution: Opportunistic suspend
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Create an additional "idle path" which has new rules for determining
> idleness.  When this new "idle" is reached, trigger full-system
> suspend.  Since a suspend is triggered whenever the opportunity
> arises, this is called opportunistic suspend.
>
> The new rules for making the idleness decision are simple:
>
> 1.  system may suspend if and only if no suspend blockers are held
>
> 2.1 When to use a suspend blocker
> ==================================
>
> [A list of reasons why suspend blockers might be used would be very
>  helpful here.]
>
> - ensure wakeup events propagate to userspace (e.g. keypad example in doc)
>
> - screen is on
>
> - someone mentioned "Google use cases"
>  (would be nice to hear about more of these)

Most uses of suspend blockers are in some way tied to a potential
wakeup event. We use (a) suspend blocker(s) to make sure the event
propagates to the code that will handle the event, and that code then
uses another suspend blocker while it handles the event.

Some examples:

The battery monitor on Nexus One uses a periodic alarm to wake up the
system. The alarm driver will block suspend so the timer can fire, and
the battery monitor will block suspend while reading the battery
status and changing the charge mode.

Phone rings: We use a few suspend blockers to process the low level
message from the modem which end up returning a message on a tty. The
last step in the kernel currently uses a wakelock with a timeout since
we have not modified the tty layer to block suspend. The user space
ril process then blocks suspend while it handles this message.

USB: We get a (wakeup) message from the modem that there is power on
the usb connector and we block suspend while we detect what is
connected. If we are connected to a USB host, we block suspend and
turn on the usb interface.

>
> 2.2 Usage in PM aware drivers
> ==============================
>
> [An example of how a driver already using runtime PM would use
>  a suspend blocker would also be helpful.
>

An audio driver can block suspend while audio is playing. We don't
currently use the runtime pm framework since this is new, but we do
runtime pm by turning off clocks and power when the device is
inactive.

-- 
Arve Hjønnevåg

Re: [PATCH 0/8] Suspend block api (version 6)

[Mark Brown]

On Tue, May 11, 2010 at 06:11:09PM -0700, Arve Hj?nnev?g wrote:

> > 2.2 Usage in PM aware drivers
> > ==============================

> > [An example of how a driver already using runtime PM would use
> > ?a suspend blocker would also be helpful.

> An audio driver can block suspend while audio is playing. We don't
> currently use the runtime pm framework since this is new, but we do
> runtime pm by turning off clocks and power when the device is
> inactive.

Could you clarify what's going on here please?  What do you mean by an
"audio driver" here - there's several different classes of driver which
work together to produce the embedded audio subsystem and I'm not clear
which you're referring to here.  I'd not expect a chip-specific audio
driver to be taking suspend blockers at all except during things like
accessory detect handling which can take a long time.  A system-specific
driver could reasonably block during playback but doing it in a chip
specific driver sounds like a bit too much of a policy decision.

The kernel runtime PM framwwork tends not to come into play for anything
except MFD and SoC drivers with audio where they're a component of PM on
the larger chip and it's useful for coordinating with the other drivers
in play.  Otherwise we already have very detailed automatic power
management (especially for the CODECs) and there's no benefit in
bouncing through runtime PM.