linux-kernel - Re: [PATCH] cpufreq: mediatek: change transition delay for MT8186

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <d0745e1cee9fae33252bb8d3db741c2a463983d6.camel@mediatek.com>
Date:   Tue, 29 Aug 2023 08:25:11 +0000
From:   Chun-Jen Tseng (曾俊仁) 
        <Chun-Jen.Tseng@...iatek.com>
To:     "viresh.kumar@...aro.org" <viresh.kumar@...aro.org>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-mediatek@...ts.infradead.org" 
        <linux-mediatek@...ts.infradead.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
        "sumitg@...dia.com" <sumitg@...dia.com>,
        "sanjayc@...dia.com" <sanjayc@...dia.com>,
        "rafael.j.wysocki@...el.com" <rafael.j.wysocki@...el.com>,
        Project_Global_Chrome_Upstream_Group 
        <Project_Global_Chrome_Upstream_Group@...iatek.com>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "matthias.bgg@...il.com" <matthias.bgg@...il.com>,
        "rafael@...nel.org" <rafael@...nel.org>,
        "angelogioacchino.delregno@...labora.com" 
        <angelogioacchino.delregno@...labora.com>
Subject: Re: [PATCH] cpufreq: mediatek: change transition delay for MT8186

On Tue, 2023-08-29 at 12:40 +0530, Viresh Kumar wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  
> On 29-08-23, 06:57, Chun-Jen Tseng (曾俊仁) wrote:
> > For MT8186 set freq must Big CPU -> Little CPU -> CCI like this
> order
> > and it 
> > takes 10 ms.
> > 
> > But in cpufreq & devfreq flow , when Big CPU or Little CPU change
> freq
> > , it will call
> > CCI setting by different policy. It will become Big CPU -> CCI
> setting
> > or Little CPU ->
> > CCI setting. Howevery, It will cause CCI setting to wrong value
> when
> > per 1ms call governor
> > and change freq.
>  
> > this patch (44295af5019f) , fixed cpufreq_out_of_sync() condition
> from
> > 1000 Mhz to 1 Mhz.
> > So, when cpufreq_verify_current_freq() call clk_get_rate() over 1
> Mhz,
> > it must to do freq sync
> > by cpufreq_out_of_sync(). In MT8186, it offen over 1 Mhz when call
> > clk_get_rate(), so I modify
> > transition delay from 1 ms to 10 ms for make sure freq setting
> done. 
>  
> > In MT8186, if CCI freq is lower than Big CPU freq 70%, it will
> causes
> > kernel panic
> > on Big CPU.
> 
> Okay, I get it now. First of all, the kernel shouldn't crash because
> of a simple delay anywhere, like the latency delay here. If that is
> happening it means something else is wrong somewhere else.
> 
> From what I understand now, your CCI have a constraint compared to
> the
> frequency of the BIG CPU (and little CPU too). You need something
> else
> that guarantees that the CCI is always configured in the right range.
> Perhaps a devfreq governor or something else that takes care of this.
> It shall evaluate the next state based on both big and little CPU
> frequencies and not only the caller via which we reached CCI. Please
> see how others have solved this.
> 
> I am very sure this is working by chance here and will break with
> some
> other delay somewhere else. Fix the real cause of it.
> 

Actually, the root cause is the CPU freq setting finish time. If MT8186
needs 10 ms for two clusters findish setting CPU clock done, I should
set transition delay 10 ms which avoid call clk_get_rate() get previous
clock value. If I get previous CPU clock and it over 1 Mhz, the
cpufreq_out_of_sync() will set CPU freq again but it wrong CPU freq.

Howervery, transition delay seting is by individual SoC , it should not
force 1 ms for all SoC. So, I wish I can do this patch here.


> -- 
> viresh