RE: SONY IR issue

[Joakim Zhang <qiangqing.zhang@xxxxxxx>]

Hi Sean,

We have already found the root cause, unbalanced interrupt response cause timing incorrect when cpuidle active. After disable cpuidle, GPIO IR works fine.
However, that's a barbaric practice. It's impossible for us to disable cpuidle in a real system. 

Now two issues in front of me:
One is high cpu loading, after testing, GPIO IR can always decode, it seems not a severe problem. Another is cpuidle, it leads to a high probability of decoding failed.

I have a half-baked idea, can you help evaluate it? Is it possible to dynamically disable and enable cpuidle from gpio_ir_recv_irq? Or do this somewhere else in IR framework?

When first entering gpio_ir_recv_irq, invokes cpu_latency_qos_add_request to disable cpuidle, concurrently start a timer. Then entering gpio_ir_recv_irq again and again,
check the timer. If timer value smaller than VALUE_TIMEOUT, just reset the timer. If timer value large than VALUE_TIMEOUT, means one loop finish, invokes 
cpu_latency_qos_remove_request to enable the cpuidle. The VALUE_TIMEOUT is the gap between continuous signals. The difficult is how to tune VALUE_TIMEOUT to satisfy all IR protocols.

I wrote a draft, and it can work fine at my side:
--- a/drivers/media/rc/gpio-ir-recv.c
+++ b/drivers/media/rc/gpio-ir-recv.c
@@ -11,6 +11,7 @@
 #include <linux/of.h>
 #include <linux/of_gpio.h>
 #include <linux/platform_device.h>
+#include <linux/pm_qos.h>
 #include <linux/irq.h>
 #include <media/rc-core.h>

@@ -20,13 +21,32 @@ struct gpio_rc_dev {
        struct rc_dev *rcdev;
        struct gpio_desc *gpiod;
        int irq;
+       struct timer_list cpuidle_change;
+       bool cpuidle_active;
+       struct pm_qos_request qos;
 };

+static void gpio_ir_cpuidle_change(struct timer_list *t)
+{
+       struct gpio_rc_dev *gpio_dev = from_timer(gpio_dev, t, cpuidle_change);
+
+       if (!(gpio_dev->cpuidle_active)) {
+               pm_qos_remove_request(&gpio_dev->qos);
+               gpio_dev->cpuidle_active = true;
+       }
+}
+
 static irqreturn_t gpio_ir_recv_irq(int irq, void *dev_id)
 {
        int val;
        struct gpio_rc_dev *gpio_dev = dev_id;

+       if (gpio_dev->cpuidle_active) {
+               pm_qos_add_request(&gpio_dev->qos, 1, 0);
+               gpio_dev->cpuidle_active = false;
+       }
+       mod_timer(&gpio_dev->cpuidle_change, jiffies + msecs_to_jiffies(125));
+
        val = gpiod_get_value(gpio_dev->gpiod);
        if (val >= 0)
                ir_raw_event_store_edge(gpio_dev->rcdev, val == 1);
@@ -61,6 +81,9 @@ static int gpio_ir_recv_probe(struct platform_device *pdev)
        if (gpio_dev->irq < 0)
                return gpio_dev->irq;

+       gpio_dev->cpuidle_active = true;
+       timer_setup(&gpio_dev->cpuidle_change, gpio_ir_cpuidle_change, 0);
+
        rcdev = devm_rc_allocate_device(dev, RC_DRIVER_IR_RAW);
        if (!rcdev)
                return -ENOMEM;

Indeed, to a certain degree, it cannot ensure the first signal to be decoded, since at that point cpuidle is active, interrupt may be delayed, so the header may not satisfy the protocol.
Actually, I have not meet such case under my test. Luckily, it would transmit multiple signals at once press.

Best Regards,
Joakim Zhang

> -----Original Message-----
> From: Joakim Zhang
> Sent: 2020年9月8日 16:41
> To: Sean Young <sean@xxxxxxxx>
> Cc: linux-media@xxxxxxxxxxxxxxx; Andy Duan <fugang.duan@xxxxxxx>;
> linux-gpio@xxxxxxxxxxxxxxx; Bartosz Golaszewski <bgolaszewski@xxxxxxxxxxxx>
> Subject: RE: SONY IR issue
> 
> 
> Hi Sean,
> 
> GPIO IR Recv extremely rely on the real-time performance of the interrupt
> response, after turn off cpuidle, IR can work steadily now.
> 
> Much thanks for your kindly help. 😊
> 
> Best Regards,
> Joakim Zhang