All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Arve Hjønnevåg" <arve@android.com>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: swetland@google.com, Nigel Cunningham <ncunningham@crca.org.au>,
	linux-pm@lists.linux-foundation.org
Subject: Re: [RFC][PATCH 00/11] Android PM extensions
Date: Sat, 31 Jan 2009 15:28:27 -0800	[thread overview]
Message-ID: <d6200be20901311528t29f2549ct307e2f8566ca3ee@mail.gmail.com> (raw)
In-Reply-To: <Pine.LNX.4.44L0.0901311049260.6305-100000@netrider.rowland.org>

On Sat, Jan 31, 2009 at 8:19 AM, Alan Stern <stern@rowland.harvard.edu> wrote:
> On Fri, 30 Jan 2009, Arve Hjønnevåg wrote:
>> On Fri, Jan 30, 2009 at 7:13 AM, Alan Stern <stern@rowland.harvard.edu> wrote:
>> > Personally, I think we need to be able to suspend computers even when
>> > there are some unconsumed type-ahead characters in the input buffer.
>> > But that's merely an implementation detail.
>>
>> For us, the key that will turn the screen back on could be in this buffer.
>
> Why do you have this fixation on the screen?  Forget about the screen
> and consider the system as a whole.
>
> If the keyboard is enabled for wakeup, then typing a key _will_ wake
> the system up from suspend -- provided the key was typed after the
> system went to sleep.  It's silly to think that a key which was typed
> before the computer was suspended should cause it to wake up.

Only if you ignore the screen state. If the screen is off and the user
presses the power button, I expect it to turn on, not off.

>> If we go to sleep while the keypad wakelock is locked, then the keypad
>> can no longer wake the system up. Note that this is specific to a
>> keypad driver that scans the keypad matrix in software. A keyboard
>> that generates an interrupt on every key change does not need its own
>> wakelock.
>
> Are you sure about this?  IIRC, you said earlier that your keyboard
> generates an interrupt on a key press only when no other keys are
> already pressed.  Okay -- so imagine the system suspends while a key is
> pressed.  Pressing another key won't generate an interrupt and hence
> won't wake up the system.  But releasing all the keys and then pressing
> one _will_ generate an interrupt and so will wake up the system.  I
> don't see any problem with this.

A keypad matrix as outputs and inputs. To get an interrupt when any
key is pressed, all the output are driven and the interrupt is enabled
on the input. When you get this interrupt you turn off the interrupt
and stop driving all the outputs. Instead you drive one output, wait
for some settle time, and then read the inputs. If you go to sleep in
this state, no key will every wake the system up.

That said, if the interrupt is edge triggered, it is possible to
implement the behaviour you describe. You can pretend no keys are held
when the driver's suspend hook is called and start driving all the
output.

>> Also, consider the case where the user presses the power button to go
>> to sleep. Before this sleep request has finished a phone call comes
>> in. The driver gets an interrupt, enqueues the message and locks a
>> wakelock to make sure user-space gets a chance to process the message.
>> If we ignore this wakelock, the phone will either not ring at all, or
>> it will ring later when the device wakes up for some other reason.
>
> For situations like this, the driver can simply refuse to suspend.  You
> don't need to use a wakelock.

How is this better than using a wakelock? Is I understand you
correctly, that would mean that the user pressed the power button, and
instead of entering suspend as soon as all wakelocks are unlocked, you
don't enter suspend at all.

Also, when should the driver stop refusing suspend? As soon as it has
delivered a message to user-space? After some delay? When do you try
to enter suspend again after a driver refused a request?

> In fact, if you did use a wakelock the behavior would be very strange.
> The user presses the power button, an instant later a call comes in,
> the device doesn't go to sleep, the user answers the call, and as soon
> as he hangs up (perhaps 10 minutes later) the wakelock is released and
> the device immediately goes to sleep!  Not what the user would expect.

Normally when the user presses a key, the auto off timer in userspace
gets reset. This prevents the system from going back to sleep
immediately.

>
>> There is a class of applications where we do want the behaviour you
>> describe, but so far all of them also require the screen to stay on.
>
> Again this fixation with the screen.  Forget about the screen!
>
>> This is handled entirely by our user-space framework which also has
>> other options (like keeping the keypad backlight on).
>>
>> > For example: Suppose some program happens to hold a wakelock, perhaps
>> > because of a simple bug, when the user closes the laptop lid and throws
>> > the laptop into a backpack.  We don't want the computer to remain awake
>> > under those circumstances!
>>
>> If the program hold the wakelock because of a bug we don't want the
>> computer on, but if it operating correctly we do.
>
> _I_ don't.  I want my device to suspend itself when I tell it to.
> Nothing is more annoying than a machine that doesn't turn itself off
> when told to do so.
>
>> Since we don't have
>> a good way to detect if the program locked the wakelock because of a
>> bug, we assume it is correct and honor the request. We have debugging
>> interfaces to report how often and for how long wakelocks are locked.
>> We also do not allow applications to use wakelocks unless they have
>> the required permission.
>
> I admit that in a properly-operating system there won't be any
> wakelocks held because of bugs.  But there might be wakelocks held for
> other reasons, and I don't want _them_ to interfere when I suspend the
> machine either.

But the purpose of wakelocks is to interfere with suspend. I keep
bringing up the screen state, because it is the state that is visible
to the user on our device. If the screen is off we enter suspend
whenever possible, if the screen is on we do not.

>> And, every pm_suspend
>> call is the result of unlocking the last wakelock.
>
> Not true.  What if all the wakelocks are already unlocked when the user
> writes "mem" to /sys/power/state?

In the current implementation, early-suspend handlers are called, then
the "main" wakelock is unlocked.

>> > So what this example really shows is how wakelocks can be used to
>> > prevent auto suspend from kicking back in the moment a keystroke is
>> > received, before userspace has even had a chance to decide whether or
>> > not to turn auto suspend off.  That's how you should describe it -- not
>> > as a way of preventing keystrokes from waking the system up.
>>
>> I was trying to show that user-space decides which keys allow the
>> system to wake up. From the kernels point of view, every key causes a
>> wakeup, but if user-space decides that the key should not wake the
>> system, the system will go back to sleep as soon as possible (when all
>> wakelocks are unlocked).
>
> This paragraph is an excellent example of the muddiness of your
> thinking.  Userspace does _not_ decide which keys allow the system to
> wake up.  _All_ keys cause the system to wake up; userspace then
> decides which ones should cause the system to go right back into
> suspend.
>
> But you don't need wakelocks to do this.  You can get the same effect
> without them, just by running a program that writes "mem" (not
> "auto-mem"!) to /sys/power/state whenever it sees a keystroke it
> doesn't like.  So this example does not illustrate the power of
> wakelocks.

That would not work very well. All threads would be frozen, some
drivers suspended, the driver that knows that there are more keys in
the queue rejects suspend, and the process is reversed.

>> The user-space notion of suspend matches the early-suspend state.
>
> Then call it "early-suspend"!  Don't call it "suspend" or "the
> user-space notion of suspend".

I was trying to keep the wakelock discussion separate from the
early-suspend. Perhaps this was a mistake.

>>  I
>> consider early-suspend another stage, not a different concept. The
>> difference between early-suspend and suspend is similar to the
>> difference between device suspend and sysdev suspend. What would you
>> call the the first stage of suspend that we have added?
>
> "Early-suspend" is an okay name.  However I have the impression that
> you want to prolong the early-suspend stage, and sometimes even go back
> to full power directly from early-suspend.  There's nothing wrong with
> that; just make it clear that this is what you're doing.
>
> In fact, I get the impression that much of what you're talking about
> has to do with providing ways to short-circuit a complete suspend and
> go directly from early-suspend back to full power.

Yes we go from fully awake to early suspend and back to fully awake,
and we also go from fully suspended to early-suspend and back to fully
suspended.

-- 
Arve Hjønnevåg

  reply	other threads:[~2009-01-31 23:28 UTC|newest]

Thread overview: 89+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-01-14  1:27 [RFC][PATCH 00/11] Android PM extensions Arve Hjønnevåg
2009-01-14  1:27 ` [PATCH 01/11] PM: Add wake lock api Arve Hjønnevåg
2009-01-14  1:27   ` [PATCH 02/11] PM: Add early suspend api Arve Hjønnevåg
2009-01-14  1:27     ` [PATCH 03/11] PM: Implement wakelock api Arve Hjønnevåg
2009-01-14  1:27       ` [PATCH 04/11] PM: Implement early suspend api Arve Hjønnevåg
2009-01-14  1:27         ` [PATCH 05/11] PM: Enable early suspend through /sys/power/state Arve Hjønnevåg
2009-01-14  1:27           ` [PATCH 06/11] PM: Add user-space wake lock api Arve Hjønnevåg
2009-01-14  1:27             ` [PATCH 07/11] PM: wakelock: Abort task freezing if a wake lock is held Arve Hjønnevåg
2009-01-14  1:27               ` [PATCH 08/11] PM: earlysuspend: Add console switch when user requested sleep state changes Arve Hjønnevåg
2009-01-14  1:27                 ` [PATCH 09/11] PM: earlysuspend: Removing dependence on console Arve Hjønnevåg
2009-01-14  1:27                   ` [PATCH 10/11] Input: Hold wake lock while event queue is not empty Arve Hjønnevåg
2009-01-14  1:27                     ` [PATCH 11/11] ledtrig-sleep: Add led trigger for sleep debugging Arve Hjønnevåg
2009-01-30 12:43             ` [PATCH 06/11] PM: Add user-space wake lock api Uli Luckas
2009-01-31  0:17               ` Arve Hjønnevåg
2009-01-31  7:24               ` Brian Swetland
2009-01-28 19:34           ` [PATCH 05/11] PM: Enable early suspend through /sys/power/state Pavel Machek
2009-01-31  3:13             ` Arve Hjønnevåg
2009-01-31 15:49               ` Alan Stern
2009-02-02 11:44                 ` Pavel Machek
2009-02-02 11:45               ` Pavel Machek
2009-02-02 22:36                 ` Arve Hjønnevåg
2009-01-14  9:48         ` [PATCH 04/11] PM: Implement early suspend api Nigel Cunningham
2009-01-14 23:57           ` Arve Hjønnevåg
2009-01-14  9:30       ` [PATCH 03/11] PM: Implement wakelock api Nigel Cunningham
2009-01-14 23:28         ` Arve Hjønnevåg
2009-01-14  9:17     ` [PATCH 02/11] PM: Add early suspend api Nigel Cunningham
2009-01-14 23:18       ` Arve Hjønnevåg
2009-01-14  9:09   ` [PATCH 01/11] PM: Add wake lock api Nigel Cunningham
2009-01-14 23:07     ` Arve Hjønnevåg
2009-01-14  9:01 ` [RFC][PATCH 00/11] Android PM extensions Nigel Cunningham
2009-01-15  0:10   ` Arve Hjønnevåg
2009-01-15  4:42   ` Arve Hjønnevåg
2009-01-15 15:08     ` Alan Stern
2009-01-15 20:34       ` Arve Hjønnevåg
2009-01-29 13:04       ` Pavel Machek
2009-01-30  1:16         ` Arve Hjønnevåg
2009-01-30  3:27           ` Alan Stern
2009-01-30  4:40             ` Arve Hjønnevåg
2009-01-30  6:04               ` Arve Hjønnevåg
2009-02-02 11:49                 ` Pavel Machek
2009-01-30  9:11               ` Pavel Machek
2009-01-30 12:34                 ` Uli Luckas
2009-02-02 11:46                   ` Pavel Machek
2009-01-30 15:13               ` Alan Stern
2009-01-31  0:02                 ` Arve Hjønnevåg
2009-01-31 16:19                   ` Alan Stern
2009-01-31 23:28                     ` Arve Hjønnevåg [this message]
2009-02-02 10:42                     ` Uli Luckas
2009-02-02 15:05                       ` Alan Stern
2009-02-02 16:15                         ` Uli Luckas
2009-02-02 16:35                           ` Alan Stern
2009-02-03 20:15                           ` Pavel Machek
2009-01-31  7:47                 ` Brian Swetland
2009-01-31 15:41                   ` Alan Stern
2009-01-31 18:39                     ` Rafael J. Wysocki
2009-01-31 18:54                       ` Igor Stoppa
2009-02-01  1:04                       ` Arve Hjønnevåg
2009-02-02 11:55                       ` Pavel Machek
2009-01-31 22:41                     ` Arve Hjønnevåg
2009-01-31 23:20                       ` Rafael J. Wysocki
2009-01-31 23:32                         ` Arve Hjønnevåg
2009-02-01  0:18                           ` Rafael J. Wysocki
2009-02-01  1:17                             ` Arve Hjønnevåg
2009-02-01  1:32                               ` Rafael J. Wysocki
2009-02-01  2:14                                 ` Arve Hjønnevåg
2009-02-01 12:30                                   ` Rafael J. Wysocki
2009-02-01 14:03                                     ` Woodruff, Richard
2009-02-01 17:43                                     ` Alan Stern
2009-02-01 19:27                                       ` Woodruff, Richard
2009-02-02 11:00                                       ` Uli Luckas
2009-02-02 15:09                                         ` Alan Stern
2009-02-02 16:24                                           ` Uli Luckas
2009-02-02 21:47                                           ` Nigel Cunningham
2009-02-02 23:21                                             ` Arve Hjønnevåg
2009-02-02 23:51                                               ` Nigel Cunningham
2009-02-03  0:08                                                 ` Arve Hjønnevåg
2009-02-04 13:25                                               ` Pavel Machek
2009-02-02 23:10                                           ` Arve Hjønnevåg
2009-02-03  3:27                                             ` Alan Stern
2009-02-03  4:18                                               ` Arve Hjønnevåg
2009-02-03 20:30                                                 ` Alan Stern
2009-02-04 13:29                                                 ` Pavel Machek
2009-02-02 11:56                         ` Pavel Machek
2009-02-02 12:38                           ` Uli Luckas
2009-01-30  9:08           ` Pavel Machek
2009-01-30  9:25             ` Brian Swetland
2009-01-28 19:31 ` Pavel Machek
2009-02-05  2:50 Arve Hjønnevåg
2009-02-06 23:51 ` Rafael J. Wysocki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d6200be20901311528t29f2549ct307e2f8566ca3ee@mail.gmail.com \
    --to=arve@android.com \
    --cc=linux-pm@lists.linux-foundation.org \
    --cc=ncunningham@crca.org.au \
    --cc=stern@rowland.harvard.edu \
    --cc=swetland@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.