Messages in this thread Patch in this message | | | Date | Fri, 3 May 2024 16:19:05 +0200 | Subject | Re: [PATCH] cpufreq/cppc: changing highest_perf to nominal_perf in cppc_cpufreq_cpu_init() | From | Pierre Gondois <> |
| |
Hello Liwei, The change itself seems ok, but I'm not sure I understand what the issue is exactly.
On 4/29/24 12:49, Viresh Kumar wrote: > CC'ing few folks who are working with the driver. > > On 28-04-24, 17:28, liwei wrote: >> When turning on turbo, if frequency configuration takes effect slowly, >> the updated policy->cur may be equal to the frequency configured in >> governor->limits(), performance governor will not adjust the frequency, >> configured frequency will remain at turbo-freq. >> >> Simplified call stack looks as follows: >> cpufreq_register_driver(&cppc_cpufreq_driver) >> ... >> cppc_cpufreq_cpu_init() >> cppc_get_perf_caps() >> policy->max = cppc_perf_to_khz(caps, caps->nominal_perf) >> cppc_set_perf(highest_perf) // set highest_perf >> policy->cur = cpufreq_driver->get() // if cur == policy->max
During the driver initialization, we have: cppc_cpufreq_cpu_init() \-policy->max = cppc_perf_to_khz(caps, caps->nominal_perf) \-policy->cur = cppc_perf_to_khz(caps, caps->highest_perf); \-cpu_data->perf_ctrls.desired_perf = caps->highest_perf; \-cppc_set_perf(cpu, &cpu_data->perf_ctrls); // set freq to highest_perf so here: policy->max = nominal_freq policy->cur = highest_freq
And then for the cpufreq framework: cpufreq_online() // IIUC there is some delay here, so policy->cur = nominal_freq ? // i.e. the freq. was requested to change to the highest_freq, // but the change is not effective yet ? \-policy->cur = cpufreq_driver->get(policy->cpu); \-cpufreq_init_policy() \-cpufreq_set_policy() \-cpufreq_start_governor() \-cpufreq_verify_current_freq() \-new_freq = cpufreq_driver->get(policy->cpu); // new_freq = nominal_freq ? \-if (policy->cur != new_freq) \- cpufreq_out_of_sync() \- policy->cur = new_freq; \-cpufreq_start_governor() \-cpufreq_gov_performance_limits() \-__cpufreq_driver_target(target_freq=policy->max) // with policy->max = nominal_freq ? \-if (target_freq == policy->cur) \- // do nothing
I am not sure I understand when you are turning the turbo on with: # echo 1 > /sys/devices/system/cpu/cpufreq/boost
Or do you mean that turbo is available but not turned on ?
Regards, Pierre
>> cpufreq_init_policy() >> ... >> cpufreq_start_governor() // governor: performance >> new_freq = cpufreq_driver->get() // if new_freq == policy->max >> if (policy->cur != new_freq) >> cpufreq_out_of_sync(policy, new_freq) >> ... >> policy->cur = new_freq >> ... >> policy->governor->limits() >> __cpufreq_driver_target(policy->max) >> if (policy->cur==target) >> // generate error, keep set highest_perf >> ret >> cppc_set_perf(target) >> >> Fix this by changing highest_perf to nominal_perf in cppc_cpufreq_cpu_init(). >> >> Fixes: 5477fb3bd1e8 ("ACPI / CPPC: Add a CPUFreq driver for use with CPPC") >> Signed-off-by: liwei <liwei728@huawei.com> >> --- >> drivers/cpufreq/cppc_cpufreq.c | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c >> index 64420d9cfd1e..db04a82b8a97 100644 >> --- a/drivers/cpufreq/cppc_cpufreq.c >> +++ b/drivers/cpufreq/cppc_cpufreq.c >> @@ -669,14 +669,14 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy) >> if (caps->highest_perf > caps->nominal_perf) >> boost_supported = true; >> >> - /* Set policy->cur to max now. The governors will adjust later. */ >> - policy->cur = cppc_perf_to_khz(caps, caps->highest_perf); >> - cpu_data->perf_ctrls.desired_perf = caps->highest_perf; >> + /* Set policy->cur to norm now. */ >> + policy->cur = cppc_perf_to_khz(caps, caps->nominal_perf); >> + cpu_data->perf_ctrls.desired_perf = caps->nominal_perf; >> >> ret = cppc_set_perf(cpu, &cpu_data->perf_ctrls); >> if (ret) { >> pr_debug("Err setting perf value:%d on CPU:%d. ret:%d\n", >> - caps->highest_perf, cpu, ret); >> + caps->nominal_perf, cpu, ret); >> goto out; >> } >> >> -- >> 2.25.1 >
| |