2010/4/28 Rafael J. Wysocki <rjw@xxxxxxx>:
> On Wednesday 28 April 2010, Alan Stern wrote:
>> On Tue, 27 Apr 2010, [UTF-8] Arve Hjønnevåg wrote:
>>
>> > +For example, in cell phones or other embedded systems, where powering the screen
>> > +is a significant drain on the battery, suspend blockers can be used to allow
>> > +user-space to decide whether a keystroke received while the system is suspended
>> > +should cause the screen to be turned back on or allow the system to go back into
>> > +suspend. Use set_irq_wake or a platform specific api to make sure the keypad
>> > +interrupt wakes up the cpu. Once the keypad driver has resumed, the sequence of
>> > +events can look like this:
>> > +
>> > +- The Keypad driver gets an interrupt. It then calls suspend_block on the
>> > + keypad-scan suspend_blocker and starts scanning the keypad matrix.
>> > +- The keypad-scan code detects a key change and reports it to the input-event
>> > + driver.
>> > +- The input-event driver sees the key change, enqueues an event, and calls
>> > + suspend_block on the input-event-queue suspend_blocker.
>> > +- The keypad-scan code detects that no keys are held and calls suspend_unblock
>> > + on the keypad-scan suspend_blocker.
>> > +- The user-space input-event thread returns from select/poll, calls
>> > + suspend_block on the process-input-events suspend_blocker and then calls read
>> > + on the input-event device.
>> > +- The input-event driver dequeues the key-event and, since the queue is now
>> > + empty, it calls suspend_unblock on the input-event-queue suspend_blocker.
>> > +- The user-space input-event thread returns from read. If it determines that
>> > + the key should leave the screen off, it calls suspend_unblock on the
>> > + process_input_events suspend_blocker and then calls select or poll. The
>> > + system will automatically suspend again, since now no suspend blockers are
>> > + active.
>> > +
>> > + Key pressed Key released
>> > + | |
>> > +keypad-scan ++++++++++++++++++
>> > +input-event-queue +++ +++
>> > +process-input-events +++ +++
>>
>> This is better than before, but it still isn't ideal. Here's what I
>> mean:
>>
>> > suspend blockers can be used to allow
>> > +user-space to decide whether a keystroke received while the system is suspended
>> > +should cause the screen to be turned back on or allow the system to go back into
>> > +suspend.
>>
>> That's not right. Handling the screen doesn't need suspend blockers:
>> The program decides what to do and then either turns on the screen or
>> else writes "mem" to /sys/power/state.
That does not work though. Unless every key turns the screen on you
will have a race every time the user presses a key you want to ignore.
>> What suspend blockers add is
>> the ability to resolve races and satisfy multiple constraints when
>> going into suspend -- which has nothing to do with operating the
>> screen.
I'm not sure I agree with this. You cannot reliably turn the screen on
from user space when the user presses a wakeup-key without suspend
blockers.
>>
>> I _think_ what you're trying to get at can be expressed this way:
>>
>> Here's an example showing how a cell phone or other embedded
>> system can handle keystrokes (or other input events) in the
>> presence of suspend blockers. Use set_irq_wake...
OK, but the last version was what you (Alan) suggested last year.
>>
>> ...
>>
>> - The user-space input-event thread returns from read. It
>> carries out whatever activities are appropriate (for example,
>> powering up the display screen, running other programs, and so
>> on). When it is finished, it calls suspend_unblock on the
>> process_input_events suspend_blocker and then calls select or
>> poll. The system will automatically suspend again when it is
>> idle and no suspend blockers remain active.
>
> Yeah, that sounds better. Arve, what do you think?
>
Idle is irrelevant and needs to be removed. This new last step is also
no longer a concrete example, but if you really think is it better I
can change it.
>> > +/**
>> > + * suspend_block() - Block suspend
>> > + * @blocker: The suspend blocker to use
>> > + *
>> > + * It is safe to call this function from interrupt context.
>> > + */
>> > +void suspend_block(struct suspend_blocker *blocker)
>> > +{
>> > + unsigned long irqflags;
>> > +
>> > + if (WARN_ON(!(blocker->flags & SB_INITIALIZED)))
>> > + return;
>> > +
>> > + spin_lock_irqsave(&list_lock, irqflags);
>> > + blocker->flags |= SB_ACTIVE;
>> > + list_del(&blocker->link);
>> > +
>> > + if (debug_mask & DEBUG_SUSPEND_BLOCKER)
>> > + pr_info("suspend_block: %s\n", blocker->name);
>> > +
>> > + list_add(&blocker->link, &active_blockers);
>>
>> Here and in suspend_unblock(), you can use list_move() in place of
>> list_del() followed by list_add().
>
OK.
> Indeed. And the debug statement might be moved out of the critical section IMHO.
>
If I move the debug statements out of the critical section you could
end entering suspend while the debug log claims a suspend blocker was
active, but I can move the debug statement to the start of the
critical section.
--
Arve Hjønnevåg
_______________________________________________
linux-pm mailing list
linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/linux-pm
