[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5c7426ab-71ef-4279-8235-69f5646af8f9@huawei.com>
Date: Mon, 6 May 2024 10:21:59 +0800
From: "liwei (JK)" <liwei728@...wei.com>
To: Pierre Gondois <pierre.gondois@....com>, Viresh Kumar
<viresh.kumar@...aro.org>, Ionela Voinescu <ionela.voinescu@....com>, Beata
Michalska <beata.michalska@....com>, Vanshidhar Konda
<vanshikonda@...amperecomputing.com>
CC: <rafael@...nel.org>, <al.stone@...aro.org>, <ashwin.chaugule@...aro.org>,
<linux-pm@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<liwei391@...wei.com>, <liaoyu15@...wei.com>
Subject: Re: [PATCH] cpufreq/cppc: changing highest_perf to nominal_perf in
cppc_cpufreq_cpu_init()
在 2024/5/3 22:19, 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 ?
>
Sorry, my description is not clear enough. The scenario described above
is during the kernel initialization process, turbo is available but
boost is not turned on.
I found this problem is to read
/sys/devices/system/cpu/cpufreq/policyX/cpuinfo_cur_freq directly after
OS startup, and found that some frequencies are in turbo state and
/sys/devices/system/cpu/cpufreq/boost has not been modified, its value
is still 0.
LiWei
>
> 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@...wei.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
>>
Powered by blists - more mailing lists