From: "Rafael J. Wysocki" <rjw@sisk.pl>
To: "Arve Hjønnevåg" <arve@android.com>
Cc: linux-input@vger.kernel.org, linux-pm@vger.kernel.org,
Dmitry Torokhov <dmitry.torokhov@gmail.com>,
Matthew Garrett <mjg@redhat.com>,
Chase Douglas <chase.douglas@canonical.com>,
Mark Brown <broonie@opensource.wolfsonmicro.com>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Input: Add ioctl to block suspend while event queue is not empty.
Date: Thu, 16 Feb 2012 00:51:15 +0100 [thread overview]
Message-ID: <201202160051.16011.rjw@sisk.pl> (raw)
In-Reply-To: <CAMP5XgfrG=3qeFasg4prBu88-Ox59_PtFw-rpgb=PN=uo6VPuw@mail.gmail.com>
Hi,
On Tuesday, February 14, 2012, Arve Hjønnevåg wrote:
> 2012/2/11 Rafael J. Wysocki <rjw@sisk.pl>:
> > Hi,
> >
> > On Saturday, January 21, 2012, Arve Hjønnevåg wrote:
> >> Add an ioctl, EVIOCSSUSPENDBLOCK, to block suspend while the event
> >> queue is not empty. This allows userspace code to process input
> >> events while the device appears to be asleep.
> >
> > I have two questions to start with, if you don't mind:
> >
> > (1) Does Android user space use an analogous interface right now or is it
> > a prototype?
> >
>
> Yes (for the next version), but with a wakelock based implementation.
That's exactly what I wanted to know. :-)
> > (2) What exactly are the use cases for it (ie. what problem does it address
> > that cannot be addressed in a different way)?
> >
>
> The reason for adding an ioctl versus the old android version with
> used a wakelock for all input events, is that not all input events are
> wakeup events. Specifically some sensors generate input events at such
> a high rate that they prevent suspend while the sensor is on even
> though the driver has a suspend hook that turns the sensor off.
>
> If you are asking why input events need to block suspend at all, this
> was the example used in the suspend blocker documentation:
> - 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 be ignored, 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.
That looks reasonable to me.
Still, I have one issue with adding those ioctls. Namely, wouldn't it be
simpler to treat all events coming from wakeup devices as wakeup events?
With the new ioctls user space can "mark" a queue of events coming out of
a device that's not a wakeup one as a "wakeup source", which doesn't seem to
be correct.
Conversely, with those ioctls user space can effectively turn events coming
out of a wakeup device into non-wakeup ones, which doesn't seem to be correct
either.
So, I think evdev client queue's wakeup source (or wakelock) should stay active
if there are any events in the queue and the underlying device is a wakeup one,
i.e. device_may_wakeup() returns true for it. Then, we won't need the new
ioctls at all and user space will still be able to control which events are to
be regarded as wakeup ones by changing the wakeup settings of the devices that
generate them.
Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2012-02-15 23:47 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-01-21 2:24 [PATCH] Input: Add ioctl to block suspend while event queue is not empty Arve Hjønnevåg
2012-02-11 23:28 ` Rafael J. Wysocki
2012-02-13 23:52 ` Arve Hjønnevåg
2012-02-15 23:51 ` Rafael J. Wysocki [this message]
2012-02-16 5:24 ` Arve Hjønnevåg
2012-02-16 22:31 ` Rafael J. Wysocki
2012-02-16 22:33 ` Mark Brown
2012-02-14 3:25 ` NeilBrown
2012-02-15 23:30 ` Rafael J. Wysocki
2012-02-16 1:53 ` Paul Fox
2012-02-16 21:44 ` Rafael J. Wysocki
2012-02-15 23:18 ` 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=201202160051.16011.rjw@sisk.pl \
--to=rjw@sisk.pl \
--cc=arve@android.com \
--cc=broonie@opensource.wolfsonmicro.com \
--cc=chase.douglas@canonical.com \
--cc=dmitry.torokhov@gmail.com \
--cc=linux-input@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=mjg@redhat.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).