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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <DB8PR04MB6795E152DF55CE10E8E39B47E63F0@DB8PR04MB6795.eurprd04.prod.outlook.com>
Date:   Fri, 18 Sep 2020 08:56:46 +0000
From:   Joakim Zhang <qiangqing.zhang@....com>
To:     Sean Young <sean@...s.org>
CC:     "mchehab@...nel.org" <mchehab@...nel.org>,
        "linux-media@...r.kernel.org" <linux-media@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        dl-linux-imx <linux-imx@....com>
Subject: RE: [PATCH] media: rc: gpio-ir-recv: add QoS support for cpuidle
 system


Hi Sean,

> -----Original Message-----
> From: Sean Young <sean@...s.org>
> Sent: 2020年9月18日 16:24
> To: Joakim Zhang <qiangqing.zhang@....com>
> Cc: mchehab@...nel.org; linux-media@...r.kernel.org;
> linux-kernel@...r.kernel.org; dl-linux-imx <linux-imx@....com>
> 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@...s.org>
> > > Sent: 2020年9月18日 4:44
> > > To: Joakim Zhang <qiangqing.zhang@....com>
> > > Cc: mchehab@...nel.org; linux-media@...r.kernel.org;
> > > linux-kernel@...r.kernel.org; dl-linux-imx <linux-imx@....com>
> > > 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