[PATCH] input: input: stop autorepeat timer on key release

[PATCH] input: input: stop autorepeat timer on key release

[Johannes Berg]

Whenever you press and then release a key, the CPU wakes up
three times:
 * press
 * release
 * autorepeat timer exactly 250ms after press

The autorepeat timer has nothing to do, obviously, since you already
have released the key, so stop it on key release.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
Cc: Arjan van de Ven <arjan@infradead.org>
---
 drivers/input/input.c |    7 +++++++
 1 file changed, 7 insertions(+)

--- wireless-testing.orig/drivers/input/input.c	2009-01-06 12:15:56.000000000 +0100
+++ wireless-testing/drivers/input/input.c	2009-01-06 12:17:13.000000000 +0100
@@ -132,6 +132,11 @@ static void input_start_autorepeat(struc
 	}
 }
 
+static void input_stop_autorepeat(struct input_dev *dev)
+{
+	del_timer(&dev->timer);
+}
+
 #define INPUT_IGNORE_EVENT	0
 #define INPUT_PASS_TO_HANDLERS	1
 #define INPUT_PASS_TO_DEVICE	2
@@ -167,6 +172,8 @@ static void input_handle_event(struct in
 				__change_bit(code, dev->key);
 				if (value)
 					input_start_autorepeat(dev, code);
+				else
+					input_stop_autorepeat(dev);
 			}
 
 			disposition = INPUT_PASS_TO_HANDLERS;

Re: [PATCH] input: input: stop autorepeat timer on key release

[Dmitry Torokhov]

Hi Johannes,

On Tue, Jan 06, 2009 at 12:26:24PM +0100, Johannes Berg wrote:
> Whenever you press and then release a key, the CPU wakes up
> three times:
>  * press
>  * release
>  * autorepeat timer exactly 250ms after press
> 
> The autorepeat timer has nothing to do, obviously, since you already
> have released the key, so stop it on key release.
> 

This introduces a slight change in behaviour (the key that is released
may not be one that is being autorepeated) but I think it still makes
sense to do it.

-- 
Dmitry

Re: [PATCH] input: input: stop autorepeat timer on key release

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1679 bytes --]

On Sun, 2009-01-11 at 23:36 -0800, Dmitry Torokhov wrote:
> Hi Johannes,
> 
> On Tue, Jan 06, 2009 at 12:26:24PM +0100, Johannes Berg wrote:
> > Whenever you press and then release a key, the CPU wakes up
> > three times:
> >  * press
> >  * release
> >  * autorepeat timer exactly 250ms after press
> > 
> > The autorepeat timer has nothing to do, obviously, since you already
> > have released the key, so stop it on key release.
> > 
> 
> This introduces a slight change in behaviour (the key that is released
> may not be one that is being autorepeated) but I think it still makes
> sense to do it.

Hmm, good point, I thought it didn't, but it does when you press a, b
wait, release a then b doesn't continue autorepeating. Seems like a
corner case though, so I agree. We could fix that by checking which key
is being autorepeated though, I'd think, see below (untested as of now)

johannes

--- wireless-testing.orig/drivers/input/input.c	2009-01-10 11:24:06.000000000 +0100
+++ wireless-testing/drivers/input/input.c	2009-01-12 09:49:58.000000000 +0100
@@ -132,6 +132,14 @@ static void input_start_autorepeat(struc
 	}
 }
 
+static void input_stop_autorepeat(struct input_dev *dev, int code)
+{
+	if (dev->repeat_key != code)
+		return;
+
+	del_timer(&dev->timer);
+}
+
 #define INPUT_IGNORE_EVENT	0
 #define INPUT_PASS_TO_HANDLERS	1
 #define INPUT_PASS_TO_DEVICE	2
@@ -167,6 +175,8 @@ static void input_handle_event(struct in
 				__change_bit(code, dev->key);
 				if (value)
 					input_start_autorepeat(dev, code);
+				else
+					input_stop_autorepeat(dev, code);
 			}
 
 			disposition = INPUT_PASS_TO_HANDLERS;


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] input: input: stop autorepeat timer on key release

[Dmitry Torokhov]

On Mon, Jan 12, 2009 at 09:51:03AM +0100, Johannes Berg wrote:
> On Sun, 2009-01-11 at 23:36 -0800, Dmitry Torokhov wrote:
> > Hi Johannes,
> > 
> > On Tue, Jan 06, 2009 at 12:26:24PM +0100, Johannes Berg wrote:
> > > Whenever you press and then release a key, the CPU wakes up
> > > three times:
> > >  * press
> > >  * release
> > >  * autorepeat timer exactly 250ms after press
> > > 
> > > The autorepeat timer has nothing to do, obviously, since you already
> > > have released the key, so stop it on key release.
> > > 
> > 
> > This introduces a slight change in behaviour (the key that is released
> > may not be one that is being autorepeated) but I think it still makes
> > sense to do it.
> 
> Hmm, good point, I thought it didn't, but it does when you press a, b
> wait, release a then b doesn't continue autorepeating. Seems like a
> corner case though, so I agree. We could fix that by checking which key
> is being autorepeated though, I'd think, see below (untested as of now)
> 

I actually think the original change was better - sometimes when shift or
alt or ctrl key gets stuck it takes time to figure out what happened. If
we stop autorepeat upon release of any key it will recover much quicker.

-- 
Dmitry

Re: [PATCH] input: input: stop autorepeat timer on key release

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 953 bytes --]

On Mon, 2009-01-12 at 21:02 -0800, Dmitry Torokhov wrote:

> > > This introduces a slight change in behaviour (the key that is released
> > > may not be one that is being autorepeated) but I think it still makes
> > > sense to do it.
> > 
> > Hmm, good point, I thought it didn't, but it does when you press a, b
> > wait, release a then b doesn't continue autorepeating. Seems like a
> > corner case though, so I agree. We could fix that by checking which key
> > is being autorepeated though, I'd think, see below (untested as of now)
> > 
> 
> I actually think the original change was better - sometimes when shift or
> alt or ctrl key gets stuck it takes time to figure out what happened. If
> we stop autorepeat upon release of any key it will recover much quicker.

Heh, good point too. Well, I was just confused by seeing the timer in
there until I realised what was going on, so feel free to take either
patch. :)

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]