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: <86f49ce84f32941185c961da8a5c671e7aed46b1.camel@mediatek.com>
Date:   Tue, 29 Aug 2023 06:57:01 +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 Mon, 2023-08-28 at 12:09 +0530, Viresh Kumar wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  Hi Mark,
> 
> I am not entirely clear by few things in the commit log.
> 
> On 18-08-23, 10:06, Mark Tseng wrote:
> > For MT8186, it has policy0 and policy6 by different governor
> thread,so
> > it may be call cpufreq->set_target_index() by different core.
> 
> Why does this matter ?

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.

> > In general
> > case, it must check BCPU, LCPU and CCI together then take about
> 10ms.
> 
> BCPU is Big CPU ? LCPU is Little CPU ?

Yes, it is.

> So are you saying that changing the frequency takes roughly 10 ms for
> MT8186 ?
> 
> > Atfer 44295af5019f this patch, it may call cpufreq_out_of_sync() by
> > cpufreq_verify_current_freq() because current frequency is bigger
> > than clk_get_rate() over 1 Mhz. By the same time, it may call
> 
> s/ouver/over/
> s/1Mh/1 MHz/
> 
> 
> > cpufreq->set_target_index() again.
> 
> Where was it called for the first time ?

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. 

> > So, the CCI freq may be too lower for
> > BCPU cause BCPU kernel panic.
> 
> I am not sure how a low frequency causes kernel panic here.

In MT8186, if CCI freq is lower than Bit CPU freq 70%, it will causes
kernel panic
on Big CPU.

> > So, it should change the default transition delay 1ms to 10ms. It
> can
> > promise the next freq setting then governor trigger new freq
> change.
> 
> There are few typos as well here, please fix them.
> 
> > Fixes: 44295af5019f ("cpufreq: use correct unit when verify cur
> freq")
> 
> I think you should drop this. The issue at hand may be visible now
> after 44295af5019f is applied, but it certainly didn't cause it.
> 
> -- 
> viresh
>