[PATCH] IR/imon: add proper auto-repeat support

[PATCH] IR/imon: add proper auto-repeat support

[Jarod Wilson]

Set the EV_REP bit, so reported key repeats actually make their
way out to userspace, and fix up the handling of repeats a bit,
routines for which are shamelessly heisted from ati_remote2.c.

Signed-off-by: Jarod Wilson <jarod@redhat.com>
---
 drivers/media/IR/imon.c |   38 +++++++++++++++++++++++++++++++-------
 1 files changed, 31 insertions(+), 7 deletions(-)

diff --git a/drivers/media/IR/imon.c b/drivers/media/IR/imon.c
index b65c31a..16e2e7f 100644
--- a/drivers/media/IR/imon.c
+++ b/drivers/media/IR/imon.c
@@ -130,6 +130,7 @@ struct imon_context {
 	u64 ir_type;			/* iMON or MCE (RC6) IR protocol? */
 	u8 mce_toggle_bit;		/* last mce toggle bit */
 	bool release_code;		/* some keys send a release code */
+	unsigned long jiffies;		/* repeat timer */
 
 	u8 display_type;		/* store the display type */
 	bool pad_mouse;			/* toggle kbd(0)/mouse(1) mode */
@@ -146,7 +147,6 @@ struct imon_context {
 };
 
 #define TOUCH_TIMEOUT	(HZ/30)
-#define MCE_TIMEOUT_MS	200
 
 /* vfd character device file operations */
 static const struct file_operations vfd_fops = {
@@ -1394,6 +1394,8 @@ static int imon_parse_press_type(struct imon_context *ictx,
 				 unsigned char *buf, u8 ktype)
 {
 	int press_type = 0;
+	int rep_delay = ictx->idev->rep[REP_DELAY];
+	int rep_period = ictx->idev->rep[REP_PERIOD];
 
 	/* key release of 0x02XXXXXX key */
 	if (ictx->kc == KEY_RESERVED && buf[0] == 0x02 && buf[3] == 0x00)
@@ -1418,12 +1420,12 @@ static int imon_parse_press_type(struct imon_context *ictx,
 			ictx->mce_toggle_bit = buf[2];
 			press_type = 1;
 			mod_timer(&ictx->itimer,
-				  jiffies + msecs_to_jiffies(MCE_TIMEOUT_MS));
+				  jiffies + msecs_to_jiffies(rep_delay));
 		/* repeat */
 		} else {
 			press_type = 2;
 			mod_timer(&ictx->itimer,
-				  jiffies + msecs_to_jiffies(MCE_TIMEOUT_MS));
+				  jiffies + msecs_to_jiffies(rep_period));
 		}
 
 	/* incoherent or irrelevant data */
@@ -1458,12 +1460,14 @@ static void imon_incoming_packet(struct imon_context *ictx,
 	u32 remote_key = 0;
 	struct input_dev *idev = NULL;
 	int press_type = 0;
-	int msec;
+	int msec, rep_delay, rep_period;
 	struct timeval t;
 	static struct timeval prev_time = { 0, 0 };
 	u8 ktype = IMON_KEY_IMON;
 
 	idev = ictx->idev;
+	rep_delay = idev->rep[REP_DELAY];
+	rep_period = idev->rep[REP_PERIOD];
 
 	/* filter out junk data on the older 0xffdc imon devices */
 	if ((buf[0] == 0xff) && (buf[7] == 0xff))
@@ -1529,8 +1533,28 @@ static void imon_incoming_packet(struct imon_context *ictx,
 	}
 
 	press_type = imon_parse_press_type(ictx, buf, ktype);
-	if (press_type < 0)
+
+	switch (press_type) {
+	/* release */
+	case 0:
+		break;
+	/* press */
+	case 1:
+		ictx->jiffies = jiffies + msecs_to_jiffies(rep_delay);
+		break;
+	/* repeat */
+	case 2:
+		/* don't repeat too fast */
+		if (!time_after_eq(jiffies, ictx->jiffies))
+			return;
+
+		ictx->jiffies = jiffies + msecs_to_jiffies(rep_period);
+		break;
+	case -EINVAL:
+	default:
 		goto not_input_data;
+		break;
+	}
 
 	if (ictx->kc == KEY_UNKNOWN)
 		goto unknown_key;
@@ -1541,7 +1565,7 @@ static void imon_incoming_packet(struct imon_context *ictx,
 		do_gettimeofday(&t);
 		msec = tv2int(&t, &prev_time);
 		prev_time = t;
-		if (msec < 200)
+		if (msec < rep_delay)
 			return;
 	}
 
@@ -1686,7 +1710,7 @@ static struct input_dev *imon_init_idev(struct imon_context *ictx)
 	strlcat(ictx->phys_idev, "/input0", sizeof(ictx->phys_idev));
 	idev->phys = ictx->phys_idev;
 
-	idev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_REL);
+	idev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_REP) | BIT_MASK(EV_REL);
 
 	idev->keybit[BIT_WORD(BTN_MOUSE)] =
 		BIT_MASK(BTN_LEFT) | BIT_MASK(BTN_RIGHT);

-- 
Jarod Wilson
jarod@redhat.com

Re: [PATCH] IR/imon: add proper auto-repeat support

[Dmitry Torokhov]

On Wed, Apr 28, 2010 at 01:37:00PM -0400, Jarod Wilson wrote:
> Set the EV_REP bit, so reported key repeats actually make their
> way out to userspace, and fix up the handling of repeats a bit,
> routines for which are shamelessly heisted from ati_remote2.c.
>

Is it really needed? I'd expect input core handling auto-repeating
for you as long as you set EV_REP flag.

-- 
Dmitry

Re: [PATCH] IR/imon: add proper auto-repeat support

[Jarod Wilson]

On Wed, Apr 28, 2010 at 01:41:12PM -0700, Dmitry Torokhov wrote:
> On Wed, Apr 28, 2010 at 01:37:00PM -0400, Jarod Wilson wrote:
> > Set the EV_REP bit, so reported key repeats actually make their
> > way out to userspace, and fix up the handling of repeats a bit,
> > routines for which are shamelessly heisted from ati_remote2.c.
> >
> 
> Is it really needed? I'd expect input core handling auto-repeating
> for you as long as you set EV_REP flag.

In my own (albeit brief) testing, the heisted bits don't make a
significant difference, but I had a user say that it helped a bit at
suppressing what he perceived as extraneous repeats being passed through.
In reality, setting a slightly higher rep delay or rep period might be the
better answer here.

Actually, I think I know exactly what was happening for said user. He's
using a device that only supports mce mode, where these devices don't have
a release code, they're timer-based, I think the repeat timeout of 200ms
was triggering an auto-repeat every 33ms for 200ms, thus giving the
appearance of way more repeats than the hardware actually sent.

Okay, lemme rework this patch a bit, I think I can drop all that heisted
crud, and simply bumping the release timer in mce mode by rep period ms
instead of 200ms for repeats should be sufficient. Need to get my imon
device that supports both imon and mce proto back into a state where I can
use it for testing...

-- 
Jarod Wilson
jarod@redhat.com

[PATCH v2] IR/imon: add proper auto-repeat support

[Jarod Wilson]

Simplified from version 1, in that hacks heisted from ati_remote2.c
aren't actually necessary, the real fix for too many repeats was
from setting too long a timer release value (200ms) on repeats in
mce mode -- this patch drops the release timeout to 33ms, matching
the input subsystem default input_dev->rep[REP_PERIOD].

Signed-off-by: Jarod Wilson <jarod@redhat.com>
---
 drivers/media/IR/imon.c |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/media/IR/imon.c b/drivers/media/IR/imon.c
index b65c31a..09d4e44 100644
--- a/drivers/media/IR/imon.c
+++ b/drivers/media/IR/imon.c
@@ -146,7 +146,6 @@ struct imon_context {
 };
 
 #define TOUCH_TIMEOUT	(HZ/30)
-#define MCE_TIMEOUT_MS	200
 
 /* vfd character device file operations */
 static const struct file_operations vfd_fops = {
@@ -1394,6 +1393,8 @@ static int imon_parse_press_type(struct imon_context *ictx,
 				 unsigned char *buf, u8 ktype)
 {
 	int press_type = 0;
+	int rep_delay = ictx->idev->rep[REP_DELAY];
+	int rep_period = ictx->idev->rep[REP_PERIOD];
 
 	/* key release of 0x02XXXXXX key */
 	if (ictx->kc == KEY_RESERVED && buf[0] == 0x02 && buf[3] == 0x00)
@@ -1418,12 +1419,12 @@ static int imon_parse_press_type(struct imon_context *ictx,
 			ictx->mce_toggle_bit = buf[2];
 			press_type = 1;
 			mod_timer(&ictx->itimer,
-				  jiffies + msecs_to_jiffies(MCE_TIMEOUT_MS));
+				  jiffies + msecs_to_jiffies(rep_delay));
 		/* repeat */
 		} else {
 			press_type = 2;
 			mod_timer(&ictx->itimer,
-				  jiffies + msecs_to_jiffies(MCE_TIMEOUT_MS));
+				  jiffies + msecs_to_jiffies(rep_period));
 		}
 
 	/* incoherent or irrelevant data */
@@ -1541,7 +1542,7 @@ static void imon_incoming_packet(struct imon_context *ictx,
 		do_gettimeofday(&t);
 		msec = tv2int(&t, &prev_time);
 		prev_time = t;
-		if (msec < 200)
+		if (msec < idev->rep[REP_DELAY])
 			return;
 	}
 
@@ -1686,7 +1687,7 @@ static struct input_dev *imon_init_idev(struct imon_context *ictx)
 	strlcat(ictx->phys_idev, "/input0", sizeof(ictx->phys_idev));
 	idev->phys = ictx->phys_idev;
 
-	idev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_REL);
+	idev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_REP) | BIT_MASK(EV_REL);
 
 	idev->keybit[BIT_WORD(BTN_MOUSE)] =
 		BIT_MASK(BTN_LEFT) | BIT_MASK(BTN_RIGHT);

-- 
Jarod Wilson
jarod@redhat.com