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
