Re: [PATCH] [media] rc: per-protocol repeat period

2017-08-10 Thread Hans Verkuil
On 09/08/17 19:19, Sean Young wrote:
> CEC needs a keypress timeout of 550ms, which is too high for the IR
> protocols. Also fill in known repeat times, with 50ms error margin.
> 
> Also, combine all protocol data into one structure.
> 
> Signed-off-by: Sean Young 
> Suggested-by: Hans Verkuil 

Much better! I'll mark my rc-main patch as superseded.

Acked-by: Hans Verkuil 

Thanks!

Hans

> ---
>  drivers/media/rc/rc-main.c | 138 
> +
>  1 file changed, 65 insertions(+), 73 deletions(-)
> 
> diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
> index c494a4ddc138..981cccd6b988 100644
> --- a/drivers/media/rc/rc-main.c
> +++ b/drivers/media/rc/rc-main.c
> @@ -30,8 +30,54 @@
>  #define IR_TAB_MAX_SIZE  8192
>  #define RC_DEV_MAX   256
>  
> -/* FIXME: IR_KEYPRESS_TIMEOUT should be protocol specific */
> -#define IR_KEYPRESS_TIMEOUT 250
> +static const struct {
> + const char *name;
> + unsigned int repeat_period;
> + unsigned int scancode_bits;
> +} protocols[] = {
> + [RC_PROTO_UNKNOWN] = { .name = "unknown", .repeat_period = 250 },
> + [RC_PROTO_OTHER] = { .name = "other", .repeat_period = 250 },
> + [RC_PROTO_RC5] = { .name = "rc-5",
> + .scancode_bits = 0x1f7f, .repeat_period = 164 },
> + [RC_PROTO_RC5X_20] = { .name = "rc-5x-20",
> + .scancode_bits = 0x1f7f3f, .repeat_period = 164 },
> + [RC_PROTO_RC5_SZ] = { .name = "rc-5-sz",
> + .scancode_bits = 0x2fff, .repeat_period = 164 },
> + [RC_PROTO_JVC] = { .name = "jvc",
> + .scancode_bits = 0x, .repeat_period = 250 },
> + [RC_PROTO_SONY12] = { .name = "sony-12",
> + .scancode_bits = 0x1f007f, .repeat_period = 100 },
> + [RC_PROTO_SONY15] = { .name = "sony-15",
> + .scancode_bits = 0xff007f, .repeat_period = 100 },
> + [RC_PROTO_SONY20] = { .name = "sony-20",
> + .scancode_bits = 0x1fff7f, .repeat_period = 100 },
> + [RC_PROTO_NEC] = { .name = "nec",
> + .scancode_bits = 0x, .repeat_period = 160 },
> + [RC_PROTO_NECX] = { .name = "nec-x",
> + .scancode_bits = 0xff, .repeat_period = 160 },
> + [RC_PROTO_NEC32] = { .name = "nec-32",
> + .scancode_bits = 0x, .repeat_period = 160 },
> + [RC_PROTO_SANYO] = { .name = "sanyo",
> + .scancode_bits = 0x1f, .repeat_period = 250 },
> + [RC_PROTO_MCIR2_KBD] = { .name = "mcir2-kbd",
> + .scancode_bits = 0x, .repeat_period = 150 },
> + [RC_PROTO_MCIR2_MSE] = { .name = "mcir2-mse",
> + .scancode_bits = 0x1f, .repeat_period = 150 },
> + [RC_PROTO_RC6_0] = { .name = "rc-6-0",
> + .scancode_bits = 0x, .repeat_period = 164 },
> + [RC_PROTO_RC6_6A_20] = { .name = "rc-6-6a-20",
> + .scancode_bits = 0xf, .repeat_period = 164 },
> + [RC_PROTO_RC6_6A_24] = { .name = "rc-6-6a-24",
> + .scancode_bits = 0xff, .repeat_period = 164 },
> + [RC_PROTO_RC6_6A_32] = { .name = "rc-6-6a-32",
> + .scancode_bits = 0x, .repeat_period = 164 },
> + [RC_PROTO_RC6_MCE] = { .name = "rc-6-mce",
> + .scancode_bits = 0x7fff, .repeat_period = 164 },
> + [RC_PROTO_SHARP] = { .name = "sharp",
> + .scancode_bits = 0x1fff, .repeat_period = 250 },
> + [RC_PROTO_XMP] = { .name = "xmp", .repeat_period = 250 },
> + [RC_PROTO_CEC] = { .name = "cec", .repeat_period = 550 },
> +};
>  
>  /* Used to keep track of known keymaps */
>  static LIST_HEAD(rc_map_list);
> @@ -613,6 +659,7 @@ static void ir_timer_keyup(unsigned long cookie)
>  void rc_repeat(struct rc_dev *dev)
>  {
>   unsigned long flags;
> + unsigned int timeout = protocols[dev->last_protocol].repeat_period;
>  
>   spin_lock_irqsave(&dev->keylock, flags);
>  
> @@ -622,7 +669,7 @@ void rc_repeat(struct rc_dev *dev)
>   input_event(dev->input_dev, EV_MSC, MSC_SCAN, dev->last_scancode);
>   input_sync(dev->input_dev);
>  
> - dev->keyup_jiffies = jiffies + msecs_to_jiffies(IR_KEYPRESS_TIMEOUT);
> + dev->keyup_jiffies = jiffies + msecs_to_jiffies(timeout);
>   mod_timer(&dev->timer_keyup, dev->keyup_jiffies);
>  
>  out:
> @@ -693,7 +740,8 @@ void rc_keydown(struct rc_dev *dev, enum rc_proto 
> protocol, u32 scancode,
>   ir_do_keydown(dev, protocol, scancode, keycode, toggle);
>  
>   if (dev->keypressed) {
> - dev->keyup_jiffies = jiffies + 
> msecs_to_jiffies(IR_KEYPRESS_TIMEOUT);
> + dev->keyup_jiffies = jiffies +
> + msecs_to_jiffies(protocols[protocol].repeat_period);
>   mod_timer(&dev->timer_keyup, dev->keyup_jiffies);
>   }
>   spin_unlock_irqrestore(&dev->keylock, flags);
> @@ -734,33 +782,14 @@ EXPORT_SYMBOL_GPL(rc_keydown_notimeout);
>  static int rc_validate_filter(struct rc_dev *dev,
> struct rc_scancode_filter

[PATCH] [media] rc: per-protocol repeat period

2017-08-09 Thread Sean Young
CEC needs a keypress timeout of 550ms, which is too high for the IR
protocols. Also fill in known repeat times, with 50ms error margin.

Also, combine all protocol data into one structure.

Signed-off-by: Sean Young 
Suggested-by: Hans Verkuil 
---
 drivers/media/rc/rc-main.c | 138 +
 1 file changed, 65 insertions(+), 73 deletions(-)

diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index c494a4ddc138..981cccd6b988 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -30,8 +30,54 @@
 #define IR_TAB_MAX_SIZE8192
 #define RC_DEV_MAX 256
 
-/* FIXME: IR_KEYPRESS_TIMEOUT should be protocol specific */
-#define IR_KEYPRESS_TIMEOUT 250
+static const struct {
+   const char *name;
+   unsigned int repeat_period;
+   unsigned int scancode_bits;
+} protocols[] = {
+   [RC_PROTO_UNKNOWN] = { .name = "unknown", .repeat_period = 250 },
+   [RC_PROTO_OTHER] = { .name = "other", .repeat_period = 250 },
+   [RC_PROTO_RC5] = { .name = "rc-5",
+   .scancode_bits = 0x1f7f, .repeat_period = 164 },
+   [RC_PROTO_RC5X_20] = { .name = "rc-5x-20",
+   .scancode_bits = 0x1f7f3f, .repeat_period = 164 },
+   [RC_PROTO_RC5_SZ] = { .name = "rc-5-sz",
+   .scancode_bits = 0x2fff, .repeat_period = 164 },
+   [RC_PROTO_JVC] = { .name = "jvc",
+   .scancode_bits = 0x, .repeat_period = 250 },
+   [RC_PROTO_SONY12] = { .name = "sony-12",
+   .scancode_bits = 0x1f007f, .repeat_period = 100 },
+   [RC_PROTO_SONY15] = { .name = "sony-15",
+   .scancode_bits = 0xff007f, .repeat_period = 100 },
+   [RC_PROTO_SONY20] = { .name = "sony-20",
+   .scancode_bits = 0x1fff7f, .repeat_period = 100 },
+   [RC_PROTO_NEC] = { .name = "nec",
+   .scancode_bits = 0x, .repeat_period = 160 },
+   [RC_PROTO_NECX] = { .name = "nec-x",
+   .scancode_bits = 0xff, .repeat_period = 160 },
+   [RC_PROTO_NEC32] = { .name = "nec-32",
+   .scancode_bits = 0x, .repeat_period = 160 },
+   [RC_PROTO_SANYO] = { .name = "sanyo",
+   .scancode_bits = 0x1f, .repeat_period = 250 },
+   [RC_PROTO_MCIR2_KBD] = { .name = "mcir2-kbd",
+   .scancode_bits = 0x, .repeat_period = 150 },
+   [RC_PROTO_MCIR2_MSE] = { .name = "mcir2-mse",
+   .scancode_bits = 0x1f, .repeat_period = 150 },
+   [RC_PROTO_RC6_0] = { .name = "rc-6-0",
+   .scancode_bits = 0x, .repeat_period = 164 },
+   [RC_PROTO_RC6_6A_20] = { .name = "rc-6-6a-20",
+   .scancode_bits = 0xf, .repeat_period = 164 },
+   [RC_PROTO_RC6_6A_24] = { .name = "rc-6-6a-24",
+   .scancode_bits = 0xff, .repeat_period = 164 },
+   [RC_PROTO_RC6_6A_32] = { .name = "rc-6-6a-32",
+   .scancode_bits = 0x, .repeat_period = 164 },
+   [RC_PROTO_RC6_MCE] = { .name = "rc-6-mce",
+   .scancode_bits = 0x7fff, .repeat_period = 164 },
+   [RC_PROTO_SHARP] = { .name = "sharp",
+   .scancode_bits = 0x1fff, .repeat_period = 250 },
+   [RC_PROTO_XMP] = { .name = "xmp", .repeat_period = 250 },
+   [RC_PROTO_CEC] = { .name = "cec", .repeat_period = 550 },
+};
 
 /* Used to keep track of known keymaps */
 static LIST_HEAD(rc_map_list);
@@ -613,6 +659,7 @@ static void ir_timer_keyup(unsigned long cookie)
 void rc_repeat(struct rc_dev *dev)
 {
unsigned long flags;
+   unsigned int timeout = protocols[dev->last_protocol].repeat_period;
 
spin_lock_irqsave(&dev->keylock, flags);
 
@@ -622,7 +669,7 @@ void rc_repeat(struct rc_dev *dev)
input_event(dev->input_dev, EV_MSC, MSC_SCAN, dev->last_scancode);
input_sync(dev->input_dev);
 
-   dev->keyup_jiffies = jiffies + msecs_to_jiffies(IR_KEYPRESS_TIMEOUT);
+   dev->keyup_jiffies = jiffies + msecs_to_jiffies(timeout);
mod_timer(&dev->timer_keyup, dev->keyup_jiffies);
 
 out:
@@ -693,7 +740,8 @@ void rc_keydown(struct rc_dev *dev, enum rc_proto protocol, 
u32 scancode,
ir_do_keydown(dev, protocol, scancode, keycode, toggle);
 
if (dev->keypressed) {
-   dev->keyup_jiffies = jiffies + 
msecs_to_jiffies(IR_KEYPRESS_TIMEOUT);
+   dev->keyup_jiffies = jiffies +
+   msecs_to_jiffies(protocols[protocol].repeat_period);
mod_timer(&dev->timer_keyup, dev->keyup_jiffies);
}
spin_unlock_irqrestore(&dev->keylock, flags);
@@ -734,33 +782,14 @@ EXPORT_SYMBOL_GPL(rc_keydown_notimeout);
 static int rc_validate_filter(struct rc_dev *dev,
  struct rc_scancode_filter *filter)
 {
-   static const u32 masks[] = {
-   [RC_PROTO_RC5] = 0x1f7f,
-   [RC_PROTO_RC5X_20] = 0x1f7f3f,
-   [RC_PROTO_RC5_SZ] = 0x2fff,
-   [RC_PROTO_SONY12] = 0x1f007