| Message ID | 20250117101457.1530653-2-zhenglifeng1@huawei.com (mailing list archive) |
|---|---|
| State | Queued |
| Delegated to: | Rafael Wysocki |
| Headers | show |
| Series | cpufreq: Fix some boost errors related to CPU online and offline. | expand |
Hi, I am bit confused by the sequence of events here and need some clarification. Lets assume that CPU can go from 1 GHz to 1.5 GHz without boost enabled and with boost it can go to 2 GHz. On 17-01-25, 18:14, Lifeng Zheng wrote: > It turns out that cpuX will stay on the base frequency after performing > these operations: > > 1. boost all cpus: echo 1 > /sys/devices/system/cpu/cpufreq/boost Boost enabled here, max_freq_req = 2 GHz. > 2. offline the cpu: echo 0 > /sys/devices/system/cpu/cpuX/online > > 3. deboost all cpus: echo 0 > /sys/devices/system/cpu/cpufreq/boost > > 4. online the cpu: echo 1 > /sys/devices/system/cpu/cpuX/online Boost is disabled currently here, but max_freq_req = 2 GHz, which is incorrect and the current change you are proposing fixes it I think. But it is not what you are claiming to fix. > 5. boost all cpus again: echo 1 > /sys/devices/system/cpu/cpufreq/boost Boost enabled again here, and max_freq_req = 2 GHz is the correct value. So the CPU doesn't stay at base frequency here, but 2 GHz only. > This is because max_freq_req of the policy is not updated during the online > process, and the value of max_freq_req before the last offline is retained. which was 2 GHz in your example. > When the CPU is boosted again, freq_qos_update_request() will do nothing > because the old value is the same as the new one. This causes the CPU stay > on the base frequency. Update max_freq_req in cpufreq_online() will solve > this problem.
On 2025/1/20 16:27, Viresh Kumar wrote: > Hi, > > I am bit confused by the sequence of events here and need some > clarification. Lets assume that CPU can go from 1 GHz to 1.5 GHz > without boost enabled and with boost it can go to 2 GHz. > > On 17-01-25, 18:14, Lifeng Zheng wrote: >> It turns out that cpuX will stay on the base frequency after performing >> these operations: >> >> 1. boost all cpus: echo 1 > /sys/devices/system/cpu/cpufreq/boost > > Boost enabled here, max_freq_req = 2 GHz. > >> 2. offline the cpu: echo 0 > /sys/devices/system/cpu/cpuX/online >> >> 3. deboost all cpus: echo 0 > /sys/devices/system/cpu/cpufreq/boost >> >> 4. online the cpu: echo 1 > /sys/devices/system/cpu/cpuX/online > > Boost is disabled currently here, but max_freq_req = 2 GHz, which is > incorrect and the current change you are proposing fixes it I think. > But it is not what you are claiming to fix. Since boost is disabled, policy->max and policy->cpuinfo.max_freq will be 1.5GHz, this limits the actual frequency of the final. > >> 5. boost all cpus again: echo 1 > /sys/devices/system/cpu/cpufreq/boost > > Boost enabled again here, and max_freq_req = 2 GHz is the correct > value. In freq_qos_update_request(), if req->pnode.prio == new_value, it will directly return 0 and not excecute freq_qos_apply(), in which will refresh frequency. So the frequency will stay on base. > > So the CPU doesn't stay at base frequency here, but 2 GHz only. > >> This is because max_freq_req of the policy is not updated during the online >> process, and the value of max_freq_req before the last offline is retained. > > which was 2 GHz in your example. > >> When the CPU is boosted again, freq_qos_update_request() will do nothing >> because the old value is the same as the new one. This causes the CPU stay >> on the base frequency. Update max_freq_req in cpufreq_online() will solve >> this problem. >
On 20-01-25, 17:10, zhenglifeng (A) wrote: > On 2025/1/20 16:27, Viresh Kumar wrote: > > On 17-01-25, 18:14, Lifeng Zheng wrote: > >> 1. boost all cpus: echo 1 > /sys/devices/system/cpu/cpufreq/boost > > > > Boost enabled here, max_freq_req = 2 GHz. > > > >> 2. offline the cpu: echo 0 > /sys/devices/system/cpu/cpuX/online > >> > >> 3. deboost all cpus: echo 0 > /sys/devices/system/cpu/cpufreq/boost > >> > >> 4. online the cpu: echo 1 > /sys/devices/system/cpu/cpuX/online > > > > Boost is disabled currently here, but max_freq_req = 2 GHz, which is > > incorrect and the current change you are proposing fixes it I think. > > But it is not what you are claiming to fix. > > Since boost is disabled, policy->max and policy->cpuinfo.max_freq will be > 1.5GHz, this limits the actual frequency of the final. Okay, I was thinking about the case (!new_policy && cpufreq_driver->online), where cpufreq_table_validate_and_sort() isn't called and max_freq isn't updated eventually. But for the other case, we will see max-freq as 1.5GHz. > >> 5. boost all cpus again: echo 1 > /sys/devices/system/cpu/cpufreq/boost > > > > Boost enabled again here, and max_freq_req = 2 GHz is the correct > > value. > > In freq_qos_update_request(), if req->pnode.prio == new_value, it will > directly return 0 and not excecute freq_qos_apply(), in which will refresh > frequency. So the frequency will stay on base.
On 17-01-25, 18:14, Lifeng Zheng wrote: > It turns out that cpuX will stay on the base frequency after performing > these operations: > > 1. boost all cpus: echo 1 > /sys/devices/system/cpu/cpufreq/boost > > 2. offline the cpu: echo 0 > /sys/devices/system/cpu/cpuX/online > > 3. deboost all cpus: echo 0 > /sys/devices/system/cpu/cpufreq/boost > > 4. online the cpu: echo 1 > /sys/devices/system/cpu/cpuX/online > > 5. boost all cpus again: echo 1 > /sys/devices/system/cpu/cpufreq/boost > > This is because max_freq_req of the policy is not updated during the online > process, and the value of max_freq_req before the last offline is retained. > When the CPU is boosted again, freq_qos_update_request() will do nothing > because the old value is the same as the new one. This causes the CPU stay > on the base frequency. Update max_freq_req in cpufreq_online() will solve > this problem. > > Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com> > --- > drivers/cpufreq/cpufreq.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c > index 1a4cae54a01b..5882d7f5e3c1 100644 > --- a/drivers/cpufreq/cpufreq.c > +++ b/drivers/cpufreq/cpufreq.c > @@ -1475,6 +1475,10 @@ static int cpufreq_online(unsigned int cpu) > > blocking_notifier_call_chain(&cpufreq_policy_notifier_list, > CPUFREQ_CREATE_POLICY, policy); > + } else { > + ret = freq_qos_update_request(policy->max_freq_req, policy->max); > + if (ret < 0) > + goto out_destroy_policy; > } Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 1a4cae54a01b..5882d7f5e3c1 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -1475,6 +1475,10 @@ static int cpufreq_online(unsigned int cpu) blocking_notifier_call_chain(&cpufreq_policy_notifier_list, CPUFREQ_CREATE_POLICY, policy); + } else { + ret = freq_qos_update_request(policy->max_freq_req, policy->max); + if (ret < 0) + goto out_destroy_policy; } if (cpufreq_driver->get && has_target()) {
It turns out that cpuX will stay on the base frequency after performing these operations: 1. boost all cpus: echo 1 > /sys/devices/system/cpu/cpufreq/boost 2. offline the cpu: echo 0 > /sys/devices/system/cpu/cpuX/online 3. deboost all cpus: echo 0 > /sys/devices/system/cpu/cpufreq/boost 4. online the cpu: echo 1 > /sys/devices/system/cpu/cpuX/online 5. boost all cpus again: echo 1 > /sys/devices/system/cpu/cpufreq/boost This is because max_freq_req of the policy is not updated during the online process, and the value of max_freq_req before the last offline is retained. When the CPU is boosted again, freq_qos_update_request() will do nothing because the old value is the same as the new one. This causes the CPU stay on the base frequency. Update max_freq_req in cpufreq_online() will solve this problem. Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com> --- drivers/cpufreq/cpufreq.c | 4 ++++ 1 file changed, 4 insertions(+)