Re: [PATCH v2] cpufreq/cppc: Take policy->cur into judge when set target

[Riwen Lu]

在 2024/5/29 13:36, Viresh Kumar 写道:
> On 29-05-24, 11:22, Riwen Lu wrote:
> > From: Riwen Lu <luriwen@xxxxxxxxxx>
> >
> > There is a case that desired_perf is exactly the same with the old perf,
> > but the actual current freq is not. Add a judgment condition to return
> > only when the three values are exactly the same.
> >
> > This happened in S3 while the cpufreq governor is set to powersave.
> > During resume process, the CPU frequency is adjusted to the highest
> > perf. For the booting CPU, there's a warning that "CPU frequency out of
> > sync:", because the policy->cur is the lowest_perf while the actual
> > current frequency is the highest_perf that obtained via
> > cppc_cpufreq_get_rate(), then set policy->cur to highest_perf. The
> > governor->limits() intent to configure the CPU frequency to lowest_perf
> > and the governor->target() returned because the desired_perf is equal to
> > cpu->perf_ctrls.desired_perf leaving the actual current frequency and
> > policy->cur are remain the highest_perf. Add a judgement that if
> > policy->cur is the same with desired_perf to decide whther to return.
> >
> > Signed-off-by: Riwen Lu <luriwen@xxxxxxxxxx>
> >
> > ---
> > v1 -> v2:
> >   - Update commit message and email.
> > ---
> >   drivers/cpufreq/cppc_cpufreq.c | 3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
> > index 15f1d41920a3..802f7c7c0ad8 100644
> > --- a/drivers/cpufreq/cppc_cpufreq.c
> > +++ b/drivers/cpufreq/cppc_cpufreq.c
> > @@ -296,7 +296,8 @@ static int cppc_cpufreq_set_target(struct cpufreq_policy *policy,
> >   
> >   	desired_perf = cppc_khz_to_perf(&cpu_data->perf_caps, target_freq);
> >   	/* Return if it is exactly the same perf */
> > -	if (desired_perf == cpu_data->perf_ctrls.desired_perf)
> > +	if (desired_perf == cpu_data->perf_ctrls.desired_perf &&
> > +	    desired_perf == policy->cur)
>
>  From my earlier understanding, desired_perf is a derived interpretation of the
> frequency and isn't an actual frequency value which can be compared with
> policy->cur.
>
> Shouldn't we compare policy->cur with target_freq instead ? If yes, than the
> core must already be doing that somewhere I guess.
Yes, you are right， I didn't think it through. In this circumstance, the 
policy->cur is the highest frequency, desired_perf converted from 
target_freq is the same with cpu_data->perf_ctrls.desired_perf which 
shouldn't.