From: Mark Brown on
On Fri, Jun 11, 2010 at 10:46:27AM -0400, Alan Stern wrote:
> On Fri, 11 Jun 2010, James Bottomley wrote:

> > The one thing that does look difficult is that these power constraints
> > are device (and sometimes SoC) specific. Expressing them in a generic
> > way for the cpu govenors to make sense of might be hard.

> Doesn't the clock framework already handle this sort of thing?

The clock framework is implemented independantly for each CPU.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: James Bottomley on
On Fri, 2010-06-11 at 10:46 -0400, Alan Stern wrote:
> On Fri, 11 Jun 2010, James Bottomley wrote:
>
> > On Thu, 2010-06-10 at 21:21 -0700, David Brownell wrote:
> > > Do we at least have a clean way that a driver can
> > > reject a system suspend? I've lost track of many
> > > issues, but maybe this could be phrased as a QOS
> > > constraint: the current config of driver X needs
> > > clock Y active to enter the target system suspend
> > > state, driver's suspend() method reports as much. Then the entry to
> > > that system state gets blocked
> > > if the clock isn't enabled.
> >
> > So in QoS modifications to android patches, the answer is "yes" ...
> > except that the current android patch set didn't actually have this type
> > of wakelock in it.
> >
> > Android wants an idleness suspend block (or pm qos constraint) that a
> > driver can set to prevent the system idleness power govenor from
> > dropping into a power state too low for the driver, so in USB terms this
> > would prevent the states that shut down the clock. For android, it
> > prevented shutdown of an internal i2c bus.
> >
> > The one thing that does look difficult is that these power constraints
> > are device (and sometimes SoC) specific. Expressing them in a generic
> > way for the cpu govenors to make sense of might be hard.
>
> Doesn't the clock framework already handle this sort of thing?

Well, there are two elements to "this sort of thing":

1. Allow a driver to request that a given clock not be turned off.
2. Make the cpuidle governors aware of a pending "don't turn off X
clock source" so they can keep the system in a state where the
clock doesn't get powered down.

As far as I can tell from the code, neither currently exists at the
moment.

James


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Alan Stern on
On Fri, 11 Jun 2010, Mark Brown wrote:

> On Fri, Jun 11, 2010 at 10:46:27AM -0400, Alan Stern wrote:
> > On Fri, 11 Jun 2010, James Bottomley wrote:
>
> > > The one thing that does look difficult is that these power constraints
> > > are device (and sometimes SoC) specific. Expressing them in a generic
> > > way for the cpu govenors to make sense of might be hard.
>
> > Doesn't the clock framework already handle this sort of thing?
>
> The clock framework is implemented independantly for each CPU.

That's not an impediment, since drivers' requirements regarding which
clocks remain running in which power states are necessarily
platform-dependent also.


On Fri, 11 Jun 2010, James Bottomley wrote:

> Well, there are two elements to "this sort of thing":
>
> 1. Allow a driver to request that a given clock not be turned off.
> 2. Make the cpuidle governors aware of a pending "don't turn off X
> clock source" so they can keep the system in a state where the
> clock doesn't get powered down.
>
> As far as I can tell from the code, neither currently exists at the
> moment.

Well then, can (or should) the clock framework interact with the
pm-qos subsystem so that drivers don't have to worry about it?

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: James Bottomley on
On Fri, 2010-06-11 at 16:48 -0400, Alan Stern wrote:
> On Fri, 11 Jun 2010, Mark Brown wrote:
>
> > On Fri, Jun 11, 2010 at 10:46:27AM -0400, Alan Stern wrote:
> > > On Fri, 11 Jun 2010, James Bottomley wrote:
> >
> > > > The one thing that does look difficult is that these power constraints
> > > > are device (and sometimes SoC) specific. Expressing them in a generic
> > > > way for the cpu govenors to make sense of might be hard.
> >
> > > Doesn't the clock framework already handle this sort of thing?
> >
> > The clock framework is implemented independantly for each CPU.
>
> That's not an impediment, since drivers' requirements regarding which
> clocks remain running in which power states are necessarily
> platform-dependent also.
>
>
> On Fri, 11 Jun 2010, James Bottomley wrote:
>
> > Well, there are two elements to "this sort of thing":
> >
> > 1. Allow a driver to request that a given clock not be turned off.
> > 2. Make the cpuidle governors aware of a pending "don't turn off X
> > clock source" so they can keep the system in a state where the
> > clock doesn't get powered down.
> >
> > As far as I can tell from the code, neither currently exists at the
> > moment.
>
> Well then, can (or should) the clock framework interact with the
> pm-qos subsystem so that drivers don't have to worry about it?

So the implementation of this seems to be a bit complex: We could have
clockevents_register() do a per clock pm_qos variable but then the
cpuidle governors need to know which to listen for so they don't
transition to a state too low for them to be active if pm_qos says keep
them running. Even if that gets sorted out, how would USB know which
platform specific clock source is driving the wakeup events on its bus?

How complex can SoC clocksources be? If it's just a simple binary
do/don't potentially stop all clocks, I think it's easy. If SoC's have
a hierarchical shutdown sequence, and they really need this to save
power, then expressing that generically becomes rather problematic.

James




--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Arve Hjønnevåg on
2010/6/11 Alan Stern <stern(a)rowland.harvard.edu>:
> On Thu, 10 Jun 2010, Arve Hj�nnev�g wrote:
>
>> > You've lost me. �If the power manager is sitting inside a select/poll,
>> > how can it miss the event (given that the event will make data
>> > available to be read on one of the descriptors being polled)?
>> >
>>
>> It cannot sit inside of select/poll all the time.
>>
>> > Or put it another way: With wakelocks, if the app doesn't use a suspend
>> > blocker then once it reads the event data and the timed wakelock is
>> > deactivated, there is nothing to prevent the system from immediately
>> > going into opportunistic suspend. �My scheme can fail in the same way.
>> > Is that what you meant?
>> >
>>
>> No, if an app reads from a file descriptor and block suspend when the
>> read call returns, then suspend is blocked while processing the data.
>> If the driver uses a wakelock with a timeout this will fail if the
>> thread does not get to the suspend block call before the timeout
>> expires, but unrelated events that don't prevent the app from running
>> will not cause any problems.
>
> Wait a second. �Maybe I have misunderstood how timeouts are supposed to
> work with wakelocks. �I thought the idea was that the wakelock would be
> released when the timeout expires or the event queue is emptied,

That is one way to use it, and I did this so code that opened an input
device without reading from it would not prevent suspend forever. In
the last patchset I posted, I instead used an ioctl to enable the
suspend blocker.

> whichever comes first. �Now it sounds like you're saying that the
> wakelock doesn't get released until the timeout expires, even if
> userspace finishes processing all pending events before then.
>

For incoming network traffic we use a wakelock with a timeout to
prevent suspend long enough for the data to make it to user-space
since we have not added wakelocks to the network stack.

>> In your scheme the user-space power
>> manager may miss events on this file descriptor since select/poll will
>> not see an event if the app read that event right before the power
>> manager called select/poll.
>
> If the wakelock is supposed to remain active until the timeout expires
> then you are right. �On the other hand, this seems like a rather
> strange and suspicious way of handling wakelocks. �Why would you want
> to do it that way?
>

We did this to avoid changing to the network stack, tty layer, etc.

>> > There's one question that I don't remember ever seeing answered. �To
>> > which kernel drivers do you intend to add suspend blockers?
>> >
>>
>> All drivers that generate wakeup events need to either use suspend
>> blockers directly or call into something else that does. For instance,
>> with the patch to block suspend while input events are queued to
>> user-space, an input driver that fully handles its events in its
>> interrupt handler does not need any additional suspend blockers, but
>> if the driver needs a work function or a timer to run before it
>> reports the event it needs to block suspend until it has reported the
>> event.
>
> Sure. �But specifically, which drivers on Android generate wakeup
> events? �And which of them don't fully handle their events in their
> interrupt handlers?
>

Keypad, network, charger, rtc, but I'm sure I forgot some.

> Maybe another way to put this is: Where in the kernel do you intend to
> add suspend blockers?
>

In addition to the drivers that enable the wakeup events, we have
added suspend blockers to the input event code and power supply
framework. The tty layer and network stack would also need suspend
blockers to avoid using timeouts.

--
Arve Hj�nnev�g
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/