Re: [PATCH V4 07/14] cpufreq: mediatek: Add .get function

[Rex-BC Chen]

On Wed, 2022-04-27 at 08:41 +0530, Viresh Kumar wrote:
> On 26-04-22, 19:13, Rex-BC Chen wrote:
> > We have a non-upstream driver which tries to get frequency by
> > 'cpufreq_get'.
> 
> This is the right thing to do there.
> 
> > When we use that non-upstream driver, 'cpufreq_verify_current_freq'
> > will be further invoked by 'cpufreq_get' and it would cause voltage
> > pulse issue as I described previously.
> 
> I see this will eventually resolve to __cpufreq_driver_target(),
> which
> should return without any frequency updates.
> 

Hello Viresh,

Yes, the call stack will eventually go to __cpufreq_driver_target.
However, we can observe the mismatch between target_freq and policy-cur 
with a tiny difference.
e.g.
[ 553.065356] cpufreq: target for CPU 0: 500000 kHz, relation 0,
requested 500000 kHz
[ 553.066366] cpufreq: target_freq/policy->cur: 500000/499999 kHz

We check the assignment of policy->cur could be either from
cpufreq_driver->get_intermediate or from cpufreq_driver->get.
But it is strange to have the frequency value like 499999 kHz.
Is the result of tiny frequency difference expected from your point of
view?

> What do you mean by "voltage pulse" here? What actually happens which
> you want to avoid.
> 

When cpufreq is fixed to lowest opp, "voltage pulse" is a quick voltage
rising and falling phenomenon which can be observed if 'cpufreq_get' is
invoked.
>From top of view, if 'cpufreq_get' is NOT invoked in that condition,
the voltage pulse will no longer occur.
That's why we add this patch for this series.

> > Therefore, we apply the solution in this series.
> 
> I won't call it a solution but a Bug as .get() is supposed to read
> real freq of the hardware.
> 
> > Recently, we found that using 'cpufreq_generic_get' directly in our
> > non-upstream driver can do the same thing without pulse issue.
> 
> That would be an abuse of the cpufreq_generic_get() API. It is ONLY
> allowed to be used while setting .get callback in the driver.
> 

Thank you for sharing the correct information.
Is it possible to get frequency (API) a simple way, like get current
opp frequency?

BRs,
Rex