IR/imon: add proper auto-repeat support

PATCH Superseded Jarod Wilson 2 participants 4 messages Apr 28, 2010 – Apr 30, 2010

Subsystems (2)

Media (V4L/DVB) HID

lore

Share & tools

lore

Jarod Wilson

author

2010-04-28 17:37 UTC #0 lore raw

PATCH 1/1 IR/imon: add proper auto-repeat support

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 <[email protected]>

---
 drivers/media/IR/imon.c |   38 +++++++++++++++++++++++++++++++-------
 1 files changed, 31 insertions(+), 7 deletions(-)

drivers/media/IR/imon.c +31 -7

index b65c31a..16e2e7f 100644
@@ -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);

Signature · 2 lines

Jarod Wilson
[email protected]

Dmitry Torokhov

2010-04-28 20:41 UTC #1 lore raw

Quoted 4 lines · 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.

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

Signature · 1 line

Dmitry

Jarod Wilson

author

2010-04-28 21:00 UTC #2 lore raw

Quoted 8 lines · Dmitry Torokhov

Quoted 4 lines · 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.
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...

Signature · 2 lines

Jarod Wilson
[email protected]

Jarod Wilson

author

2010-04-30 19:06 UTC #3 lore raw

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

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 <[email protected]>

---
 drivers/media/IR/imon.c |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

drivers/media/IR/imon.c +6 -5

index b65c31a..09d4e44 100644
@@ -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);

Signature · 2 lines

Jarod Wilson
[email protected]