RE: [PATCH] media: rc: gpio-ir-recv: add QoS support for cpuidle system

[Joakim Zhang]

Hi Sean,

> -----Original Message-----
> From: Sean Young <sean@xxxxxxxx>
> Sent: 2020年9月18日 16:24
> To: Joakim Zhang <qiangqing.zhang@xxxxxxx>
> Cc: mchehab@xxxxxxxxxx; linux-media@xxxxxxxxxxxxxxx;
> linux-kernel@xxxxxxxxxxxxxxx; dl-linux-imx <linux-imx@xxxxxxx>
> Subject: Re: [PATCH] media: rc: gpio-ir-recv: add QoS support for cpuidle
> system
> 
> Hi Joakim,
> 
> On Fri, Sep 18, 2020 at 01:42:15AM +0000, Joakim Zhang wrote:
> > > -----Original Message-----
> > > From: Sean Young <sean@xxxxxxxx>
> > > Sent: 2020年9月18日 4:44
> > > To: Joakim Zhang <qiangqing.zhang@xxxxxxx>
> > > Cc: mchehab@xxxxxxxxxx; linux-media@xxxxxxxxxxxxxxx;
> > > linux-kernel@xxxxxxxxxxxxxxx; dl-linux-imx <linux-imx@xxxxxxx>
> > > Subject: Re: [PATCH] media: rc: gpio-ir-recv: add QoS support for
> > > cpuidle system
> 
> -snip-
> 
> > > > Autosuspend delay should be fixed value, should be set to gpio
> > > > device timeout
> > > value, which is 125ms.
> > >
> > > So the idea was that cpuidle is only enabled while IR frames are
> > > being received, that's why I suggested it.
> >
> > May be a typo, "cpuidle is only DISABLED while IR frames are being receive,",
> this is not I want to implement, experiments have also shown poor results.
> 
> Sorry, yes I got this wrong. You are right.
> 
> > > If you set the autosuspend delay to 125ms, then the cpuidle will not
> > > be enabled between IR frames. Maybe this is what you want, but it
> > > does mean cpuidle is totally suspended while anyone is pressing buttons on
> a remote.
> >
> > Yes, this is what I want, cpuidle is totally disabled while pressing buttons,
> disable cpuidle at the first frame then keep disabled until there is no activity for
> a while.
> > So that we only can not decode the first frame, such as, if transmitting 4
> frames once, we can correctly decode 3 times.
> >
> > I also try your suggestion, set autosuspend delay time to protocol's
> > timeout value, but the result is terrible. If transmitting 4 frames once, we
> can't correctly decode 3 times, even can't decode it sometime. The sequence is,
> cpu in idle state when the first frame coming, then disable cpu idle until
> protocols' timeout, cpu in idle state again, the first frame can't be decoded.
> > The second frame coming, it will repeat the behavior of the first frame, may
> cause the second frame can't be decode......
> >
> > Can you take account of I have done in the first version, autosuspend delay
> time is set to 125ms?
> 
> Yes, in retrospect you are right. Trying to shorten the cpuidle suspended period
> will not work. I am sorry about this.
> 
> How about setting the autosuspend period in devicetree, and 0 will turn this
> feature off completely?

Of course, we can do this. Such as add a linux,delaytime property:
For those systems that don't suffer this issue, need not add this property, instead of check the value is 0 as you suggested.
For those systems that would suffer this issue, need add this property and then give a appropriate value

So that users can change the autosuspend delay time via device tree, it is more flexible for different systems. If you agree with it, I will send a V2.

Best Regards,
Joakim Zhang
> Thanks,
> 
> Sean