[PATCH 8/9] Input: Block suspend while event queue is not empty.

[PATCH 8/9] Input: Block suspend while event queue is not empty.

[Arve Hjønnevåg]

Add an ioctl, EVIOCSSUSPENDBLOCK, to enable a suspend_blocker that will block
suspend while the event queue is not empty. This allows userspace code to
process input events while the device appears to be asleep.

Signed-off-by: Arve Hjønnevåg <arve@android.com>
---
 drivers/input/evdev.c |   22 ++++++++++++++++++++++
 include/linux/input.h |    3 +++
 2 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
index 2ee6c7a..66e0d16 100644
--- a/drivers/input/evdev.c
+++ b/drivers/input/evdev.c
@@ -20,6 +20,7 @@
 #include <linux/input.h>
 #include <linux/major.h>
 #include <linux/device.h>
+#include <linux/suspend_blocker.h>
 #include "input-compat.h"
 
 struct evdev {
@@ -43,6 +44,8 @@ struct evdev_client {
 	struct fasync_struct *fasync;
 	struct evdev *evdev;
 	struct list_head node;
+	struct suspend_blocker suspend_blocker;
+	bool use_suspend_blocker;
 };
 
 static struct evdev *evdev_table[EVDEV_MINORS];
@@ -55,6 +58,8 @@ static void evdev_pass_event(struct evdev_client *client,
 	 * Interrupts are disabled, just acquire the lock
 	 */
 	spin_lock(&client->buffer_lock);
+	if (client->use_suspend_blocker)
+		suspend_block(&client->suspend_blocker);
 	client->buffer[client->head++] = *event;
 	client->head &= EVDEV_BUFFER_SIZE - 1;
 	spin_unlock(&client->buffer_lock);
@@ -234,6 +239,8 @@ static int evdev_release(struct inode *inode, struct file *file)
 	mutex_unlock(&evdev->mutex);
 
 	evdev_detach_client(evdev, client);
+	if (client->use_suspend_blocker)
+		suspend_blocker_destroy(&client->suspend_blocker);
 	kfree(client);
 
 	evdev_close_device(evdev);
@@ -335,6 +342,8 @@ static int evdev_fetch_next_event(struct evdev_client *client,
 	if (have_event) {
 		*event = client->buffer[client->tail++];
 		client->tail &= EVDEV_BUFFER_SIZE - 1;
+		if (client->use_suspend_blocker && client->head == client->tail)
+			suspend_unblock(&client->suspend_blocker);
 	}
 
 	spin_unlock_irq(&client->buffer_lock);
@@ -585,6 +594,19 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
 		else
 			return evdev_ungrab(evdev, client);
 
+	case EVIOCGSUSPENDBLOCK:
+		return put_user(client->use_suspend_blocker, ip);
+
+	case EVIOCSSUSPENDBLOCK:
+		spin_lock_irq(&client->buffer_lock);
+		if (!client->use_suspend_blocker && p)
+			suspend_blocker_init(&client->suspend_blocker, "evdev");
+		else if (client->use_suspend_blocker && !p)
+			suspend_blocker_destroy(&client->suspend_blocker);
+		client->use_suspend_blocker = !!p;
+		spin_unlock_irq(&client->buffer_lock);
+		return 0;
+
 	default:
 
 		if (_IOC_TYPE(cmd) != 'E')
diff --git a/include/linux/input.h b/include/linux/input.h
index 7ed2251..b2d93b4 100644
--- a/include/linux/input.h
+++ b/include/linux/input.h
@@ -82,6 +82,9 @@ struct input_absinfo {
 
 #define EVIOCGRAB		_IOW('E', 0x90, int)			/* Grab/Release device */
 
+#define EVIOCGSUSPENDBLOCK	_IOR('E', 0x91, int)			/* get suspend block enable */
+#define EVIOCSSUSPENDBLOCK	_IOW('E', 0x91, int)			/* set suspend block enable */
+
 /*
  * Event types
  */
-- 
1.6.5.1

Re: [PATCH 8/9] Input: Block suspend while event queue is not empty.

[Randy Dunlap]

On Thu, 22 Apr 2010 18:08:57 -0700 Arve Hjønnevåg wrote:

> Add an ioctl, EVIOCSSUSPENDBLOCK, to enable a suspend_blocker that will block
> suspend while the event queue is not empty. This allows userspace code to
> process input events while the device appears to be asleep.


All new ioctls need to be added to Documentation/ioctl/ioctl-number.txt, please.

> Signed-off-by: Arve Hjønnevåg <arve@android.com>
> ---
>  drivers/input/evdev.c |   22 ++++++++++++++++++++++
>  include/linux/input.h |    3 +++
>  2 files changed, 25 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
> index 2ee6c7a..66e0d16 100644
> --- a/drivers/input/evdev.c
> +++ b/drivers/input/evdev.c
> @@ -585,6 +594,19 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
>  		else
>  			return evdev_ungrab(evdev, client);
>  
> +	case EVIOCGSUSPENDBLOCK:
> +		return put_user(client->use_suspend_blocker, ip);
> +
> +	case EVIOCSSUSPENDBLOCK:
> +		spin_lock_irq(&client->buffer_lock);
> +		if (!client->use_suspend_blocker && p)
> +			suspend_blocker_init(&client->suspend_blocker, "evdev");
> +		else if (client->use_suspend_blocker && !p)
> +			suspend_blocker_destroy(&client->suspend_blocker);
> +		client->use_suspend_blocker = !!p;
> +		spin_unlock_irq(&client->buffer_lock);
> +		return 0;
> +
>  	default:
>  
>  		if (_IOC_TYPE(cmd) != 'E')
> diff --git a/include/linux/input.h b/include/linux/input.h
> index 7ed2251..b2d93b4 100644
> --- a/include/linux/input.h
> +++ b/include/linux/input.h
> @@ -82,6 +82,9 @@ struct input_absinfo {
>  
>  #define EVIOCGRAB		_IOW('E', 0x90, int)			/* Grab/Release device */
>  
> +#define EVIOCGSUSPENDBLOCK	_IOR('E', 0x91, int)			/* get suspend block enable */
> +#define EVIOCSSUSPENDBLOCK	_IOW('E', 0x91, int)			/* set suspend block enable */
> +
>  /*
>   * Event types
>   */
> -- 


thanks,
---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: [PATCH 8/9] Input: Block suspend while event queue is not empty.

[Dmitry Torokhov]

On Friday 23 April 2010 01:56:25 pm Randy Dunlap wrote:
> On Thu, 22 Apr 2010 18:08:57 -0700 Arve Hjønnevåg wrote:
> > Add an ioctl, EVIOCSSUSPENDBLOCK, to enable a suspend_blocker that will
> > block suspend while the event queue is not empty. This allows userspace
> > code to process input events while the device appears to be asleep.
> 
> All new ioctls need to be added to Documentation/ioctl/ioctl-number.txt,
> please.

I do not see the reason for it to be in the kernel still. Have a process
that listens to all input devices (or subset of them), once events stop
coming initiate suspend.

> 
> > Signed-off-by: Arve Hjønnevåg <arve@android.com>
> > ---
> > 
> >  drivers/input/evdev.c |   22 ++++++++++++++++++++++
> >  include/linux/input.h |    3 +++
> >  2 files changed, 25 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
> > index 2ee6c7a..66e0d16 100644
> > --- a/drivers/input/evdev.c
> > +++ b/drivers/input/evdev.c
> > @@ -585,6 +594,19 @@ static long evdev_do_ioctl(struct file *file,
> > unsigned int cmd,
> > 
> >  		else
> >  		
> >  			return evdev_ungrab(evdev, client);
> > 
> > +	case EVIOCGSUSPENDBLOCK:
> > +		return put_user(client->use_suspend_blocker, ip);
> > +
> > +	case EVIOCSSUSPENDBLOCK:
> > +		spin_lock_irq(&client->buffer_lock);
> > +		if (!client->use_suspend_blocker && p)
> > +			suspend_blocker_init(&client->suspend_blocker, "evdev");
> > +		else if (client->use_suspend_blocker && !p)
> > +			suspend_blocker_destroy(&client->suspend_blocker);
> > +		client->use_suspend_blocker = !!p;
> > +		spin_unlock_irq(&client->buffer_lock);
> > +		return 0;
> > +
> > 
> >  	default:
> >  		if (_IOC_TYPE(cmd) != 'E')
> > 
> > diff --git a/include/linux/input.h b/include/linux/input.h
> > index 7ed2251..b2d93b4 100644
> > --- a/include/linux/input.h
> > +++ b/include/linux/input.h
> > @@ -82,6 +82,9 @@ struct input_absinfo {
> > 
> >  #define EVIOCGRAB		_IOW('E', 0x90, int)			/* Grab/Release 
device */
> > 
> > +#define EVIOCGSUSPENDBLOCK	_IOR('E', 0x91, int)			/* get 
suspend block
> > enable */ +#define EVIOCSSUSPENDBLOCK	_IOW('E', 0x91, int)			/* 
set
> > suspend block enable */ +
> > 
> >  /*
> >  
> >   * Event types
> >   */
> 
> thanks,
> ---
> ~Randy
> *** Remember to use Documentation/SubmitChecklist when testing your code
> *** --
> 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/

-- 
Dmitry
--
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

Re: [PATCH 8/9] Input: Block suspend while event queue is not empty.

[Arve Hjønnevåg]

2010/4/23 Randy Dunlap <randy.dunlap@oracle.com>:
> On Thu, 22 Apr 2010 18:08:57 -0700 Arve Hjønnevåg wrote:
>
>> Add an ioctl, EVIOCSSUSPENDBLOCK, to enable a suspend_blocker that will block
>> suspend while the event queue is not empty. This allows userspace code to
>> process input events while the device appears to be asleep.
>
>
> All new ioctls need to be added to Documentation/ioctl/ioctl-number.txt, please.
>

These two appear to already be covered by "'E'     all
linux/input.h           conflict!", but I will add a line the user
space suspend blocker ioctls added earlier in this patch set.

-- 
Arve Hjønnevåg
--
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

Re: [PATCH 8/9] Input: Block suspend while event queue is not empty.

[Arve Hjønnevåg]

On Fri, Apr 23, 2010 at 2:08 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Friday 23 April 2010 01:56:25 pm Randy Dunlap wrote:
>> On Thu, 22 Apr 2010 18:08:57 -0700 Arve Hjønnevåg wrote:
>> > Add an ioctl, EVIOCSSUSPENDBLOCK, to enable a suspend_blocker that will
>> > block suspend while the event queue is not empty. This allows userspace
>> > code to process input events while the device appears to be asleep.
>>
>> All new ioctls need to be added to Documentation/ioctl/ioctl-number.txt,
>> please.
>
> I do not see the reason for it to be in the kernel still. Have a process
> that listens to all input devices (or subset of them), once events stop
> coming initiate suspend.
>

I think the document added by the first patch explains this. The
solution you propose above will ignore a wakeup key pressed right
after user space decides to initiate suspend.

-- 
Arve Hjønnevåg
--
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

Re: [linux-pm] [PATCH 8/9] Input: Block suspend while event queue is not empty.

[Alan Stern]

On Fri, 23 Apr 2010, Arve Hjønnevåg wrote:

> On Fri, Apr 23, 2010 at 2:08 PM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> > On Friday 23 April 2010 01:56:25 pm Randy Dunlap wrote:
> >> On Thu, 22 Apr 2010 18:08:57 -0700 Arve Hjønnevåg wrote:
> >> > Add an ioctl, EVIOCSSUSPENDBLOCK, to enable a suspend_blocker that will
> >> > block suspend while the event queue is not empty. This allows userspace
> >> > code to process input events while the device appears to be asleep.
> >>
> >> All new ioctls need to be added to Documentation/ioctl/ioctl-number.txt,
> >> please.
> >
> > I do not see the reason for it to be in the kernel still. Have a process
> > that listens to all input devices (or subset of them), once events stop
> > coming initiate suspend.
> >
> 
> I think the document added by the first patch explains this. The
> solution you propose above will ignore a wakeup key pressed right
> after user space decides to initiate suspend.

Is there some reason why this feature needs to be enabled by an 
ioctl?  Why not make this suspend blocker permanently enabled?

Alan Stern

--
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

Re: [linux-pm] [PATCH 8/9] Input: Block suspend while event queue is not empty.

[Rafael J. Wysocki]

On Saturday 24 April 2010, Alan Stern wrote:
> On Fri, 23 Apr 2010, Arve Hjønnevåg wrote:
> 
> > On Fri, Apr 23, 2010 at 2:08 PM, Dmitry Torokhov
> > <dmitry.torokhov@gmail.com> wrote:
> > > On Friday 23 April 2010 01:56:25 pm Randy Dunlap wrote:
> > >> On Thu, 22 Apr 2010 18:08:57 -0700 Arve Hjønnevåg wrote:
> > >> > Add an ioctl, EVIOCSSUSPENDBLOCK, to enable a suspend_blocker that will
> > >> > block suspend while the event queue is not empty. This allows userspace
> > >> > code to process input events while the device appears to be asleep.
> > >>
> > >> All new ioctls need to be added to Documentation/ioctl/ioctl-number.txt,
> > >> please.
> > >
> > > I do not see the reason for it to be in the kernel still. Have a process
> > > that listens to all input devices (or subset of them), once events stop
> > > coming initiate suspend.

This solution is not practical with the Android user space AFAICT.
 
> > I think the document added by the first patch explains this. The
> > solution you propose above will ignore a wakeup key pressed right
> > after user space decides to initiate suspend.
> 
> Is there some reason why this feature needs to be enabled by an 
> ioctl?  Why not make this suspend blocker permanently enabled?

The ioctl is there so that user space can use suspend blockers, which is
needed because only user space know that some activities are going to continue
and therefore the system should not be suspended (like playing music "in the
background").

From all of the interfaces that could be used for this purpose ioctl appears to
be the most convenient (we need something that is per process and allows us
to carry out four operations: creat, destroy, activate, deactivate).

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

Re: [linux-pm] [PATCH 8/9] Input: Block suspend while event queue is not empty.

[Alan Stern]

On Sun, 25 Apr 2010, Rafael J. Wysocki wrote:

> On Saturday 24 April 2010, Alan Stern wrote:
> > On Fri, 23 Apr 2010, Arve Hjønnevåg wrote:
> > 
> > > On Fri, Apr 23, 2010 at 2:08 PM, Dmitry Torokhov
> > > <dmitry.torokhov@gmail.com> wrote:
> > > > On Friday 23 April 2010 01:56:25 pm Randy Dunlap wrote:
> > > >> On Thu, 22 Apr 2010 18:08:57 -0700 Arve Hjønnevåg wrote:
> > > >> > Add an ioctl, EVIOCSSUSPENDBLOCK, to enable a suspend_blocker that will
> > > >> > block suspend while the event queue is not empty. This allows userspace
> > > >> > code to process input events while the device appears to be asleep.

> > Is there some reason why this feature needs to be enabled by an 
> > ioctl?  Why not make this suspend blocker permanently enabled?
> 
> The ioctl is there so that user space can use suspend blockers, which is
> needed because only user space know that some activities are going to continue
> and therefore the system should not be suspended (like playing music "in the
> background").

No, you're thinking of a different ioctl: SUSPEND_BLOCKER_IOCTL_BLOCK.  
This one (EVIOCSSUSPENDBLOCK) is present _only_ to enable one specific
suspend blocker, which is activated when the input event queue is
non-empty.  I don't see any reason why it shouldn't be enabled all the
time.

Alan Stern

--
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

Re: [linux-pm] [PATCH 8/9] Input: Block suspend while event queue is not empty.

[Arve Hjønnevåg]

On Sun, Apr 25, 2010 at 8:29 AM, Alan Stern <stern@rowland.harvard.edu> wrote:
> On Sun, 25 Apr 2010, Rafael J. Wysocki wrote:
>
>> On Saturday 24 April 2010, Alan Stern wrote:
>> > On Fri, 23 Apr 2010, Arve Hjønnevåg wrote:
>> >
>> > > On Fri, Apr 23, 2010 at 2:08 PM, Dmitry Torokhov
>> > > <dmitry.torokhov@gmail.com> wrote:
>> > > > On Friday 23 April 2010 01:56:25 pm Randy Dunlap wrote:
>> > > >> On Thu, 22 Apr 2010 18:08:57 -0700 Arve Hjønnevåg wrote:
>> > > >> > Add an ioctl, EVIOCSSUSPENDBLOCK, to enable a suspend_blocker that will
>> > > >> > block suspend while the event queue is not empty. This allows userspace
>> > > >> > code to process input events while the device appears to be asleep.
>
>> > Is there some reason why this feature needs to be enabled by an
>> > ioctl?  Why not make this suspend blocker permanently enabled?
>>
>> The ioctl is there so that user space can use suspend blockers, which is
>> needed because only user space know that some activities are going to continue
>> and therefore the system should not be suspended (like playing music "in the
>> background").
>
> No, you're thinking of a different ioctl: SUSPEND_BLOCKER_IOCTL_BLOCK.
> This one (EVIOCSSUSPENDBLOCK) is present _only_ to enable one specific
> suspend blocker, which is activated when the input event queue is
> non-empty.  I don't see any reason why it shouldn't be enabled all the
> time.
>

Some user space processes keep input devices open that they don't
currently read from. The current android kernel use a wakelock with a
timeout limit the damage caused by this code, but not blocking suspend
at all by default will handle this case better. Also, not all input
devices are used for wakeup events, so there is no need to block
suspend when their event queue is not empty.

-- 
Arve Hjønnevåg
--
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