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

From: Viresh Kumar
Date: Tue Aug 29 2023 - 03:11:13 EST


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 Bit 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.

--
viresh