| Message ID | 20211115195011.52999-1-thara.gopinath@linaro.org (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | cpufreq: freq_table: Initialize cpuinfo.max_freq to correct max frequency. | expand |
Hi Thara, On 11/15/21 1:50 PM, Thara Gopinath wrote: > cpuinfo.max_freq reflects the maximum supported frequency of cpus in a > cpufreq policy. When cpus support boost frequency and if boost is disabled > during boot up (which is the default), cpuinfo.max_freq does not reflect > boost frequency as the maximum supported frequency till boost is explicitly > enabled via sysfs interface later. This also means that policy reports two > different cpuinfo.max_freq before and after turning on boost. Fix this by > separating out setting of policy->max and cpuinfo.max_freq in > cpufreq_frequency_table_cpuinfo. > > e.g. of the problem. Qualcomm sdm845 supports boost frequency for gold > cluster (cpus 4-7). After boot up (boost disabled), > > 1. cat /sys/devices/system/cpu/cpufreq/policy4/cpuinfo_max_freq 2649600 > <- This is wrong because boost frequency is > > 2. echo 1 > /sys/devices/system/cpu/cpufreq/boost <- Enable boost cat > /sys/devices/system/cpu/cpufreq/policy4/cpuinfo_max_freq 2803200 <- > max freq reflects boost freq. > > 3. echo 0 > /sys/devices/system/cpu/cpufreq/boost <- Disable boost cat > /sys/devices/system/cpu/cpufreq/policy4/cpuinfo_max_freq 2803200 <- > Discrepancy with step 1 as in both cases boost is disabled. > > Note that the other way to fix this is to set cpuinfo.max_freq in Soc > cpufreq driver during initialization. Fixing it in > cpufreq_frequency_table_cpuinfo seems more generic solution > > Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org> > --- > drivers/cpufreq/freq_table.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/cpufreq/freq_table.c b/drivers/cpufreq/freq_table.c > index 67e56cf638ef..6784f94124df 100644 > --- a/drivers/cpufreq/freq_table.c > +++ b/drivers/cpufreq/freq_table.c > @@ -35,11 +35,15 @@ int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy, > struct cpufreq_frequency_table *pos; > unsigned int min_freq = ~0; > unsigned int max_freq = 0; > + unsigned int cpuinfo_max_freq = 0; > unsigned int freq; > > cpufreq_for_each_valid_entry(pos, table) { > freq = pos->frequency; > > + if (freq > cpuinfo_max_freq) > + cpuinfo_max_freq = freq; > + > if (!cpufreq_boost_enabled() > && (pos->flags & CPUFREQ_BOOST_FREQ)) > continue; > @@ -57,8 +61,8 @@ int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy, > * If the driver has set its own cpuinfo.max_freq above max_freq, leave > * it as is. > */ > - if (policy->cpuinfo.max_freq < max_freq) > - policy->max = policy->cpuinfo.max_freq = max_freq; > + if (policy->cpuinfo.max_freq < cpuinfo_max_freq) > + policy->cpuinfo.max_freq = cpuinfo_max_freq; > > if (policy->min == ~0) > return -EINVAL; Something still isn't quite right... The setup is that I have an rc.local of #!/bin/sh echo 1 > /sys/devices/system/cpu/cpufreq/boost exit 0 After booting and logging in: steev@limitless:~$ cat /sys/devices/system/cpu/cpufreq/policy4/stats/time_in_state 825600 2499 <snip> 2649600 38 2745600 31 2841600 1473 2956800 0 After running a "cargo build --release" in an alacritty git checkout: teev@limitless:~$ cat /sys/devices/system/cpu/cpufreq/policy4/stats/time_in_state 825600 11220 <snip> 2649600 41 2745600 35 2841600 3065 2956800 0 however... If I then steev@limitless:~$ echo 0 | sudo tee /sys/devices/system/cpu/cpufreq/boost [sudo] password for steev: 0 steev@limitless:~$ echo 1 | sudo tee /sys/devices/system/cpu/cpufreq/boost 1 and run the build again... steev@limitless:~$ cat /sys/devices/system/cpu/cpufreq/policy4/stats/time_in_state 825600 21386 <snip> 2649600 45 2745600 38 2841600 3326 2956800 4815 As a workaround, I attempted to jiggle it 1-0-1 in rc.local, however that ends up giving steev@limitless:~$ cat /sys/devices/system/cpu/cpufreq/policy4/stats/time_in_state 825600 2902 <snip> 2649600 36 2745600 36 2841600 6050 2956800 13 And it doesn't go up, I even tried adding a sleep of 1 second between the echo 1/0/1 lines and while 2956800 goes up to 28 (but never uses it) it seems like, unless I do it manually once I've logged in, which I'm assuming is a lot slower than waiting 1 second between them, it's not quite giving us 2956800 "easily". If the email wasn't clear, please let me know! I tried to explain as best I could what I am seeing here. -- steev
On 11/15/21 7:23 PM, Steev Klimaszewski wrote: > Hi Thara, > <snip> > > And it doesn't go up, I even tried adding a sleep of 1 second between > the echo 1/0/1 lines and while 2956800 goes up to 28 (but never uses > it) it seems like, unless I do it manually once I've logged in, which > I'm assuming is a lot slower than waiting 1 second between them, it's > not quite giving us 2956800 "easily". > > If the email wasn't clear, please let me know! I tried to explain as > best I could what I am seeing here. > > -- steev > So after more testing... *sometimes* it works, with the jiggle, but not always. I'm really not sure why though, so if there's any guidance, I'm all for it! -- steev
On 15-11-21, 19:23, Steev Klimaszewski wrote: > Hi Thara, > > On 11/15/21 1:50 PM, Thara Gopinath wrote: > > cpuinfo.max_freq reflects the maximum supported frequency of cpus in a > > cpufreq policy. When cpus support boost frequency and if boost is disabled > > during boot up (which is the default), cpuinfo.max_freq does not reflect > > boost frequency as the maximum supported frequency till boost is explicitly > > enabled via sysfs interface later. This also means that policy reports two > > different cpuinfo.max_freq before and after turning on boost. Fix this by > > separating out setting of policy->max and cpuinfo.max_freq in > > cpufreq_frequency_table_cpuinfo. > > > > e.g. of the problem. Qualcomm sdm845 supports boost frequency for gold > > cluster (cpus 4-7). After boot up (boost disabled), > > > > 1. cat /sys/devices/system/cpu/cpufreq/policy4/cpuinfo_max_freq 2649600 > > <- This is wrong because boost frequency is > > > > 2. echo 1 > /sys/devices/system/cpu/cpufreq/boost <- Enable boost cat > > /sys/devices/system/cpu/cpufreq/policy4/cpuinfo_max_freq 2803200 <- > > max freq reflects boost freq. > > > > 3. echo 0 > /sys/devices/system/cpu/cpufreq/boost <- Disable boost cat > > /sys/devices/system/cpu/cpufreq/policy4/cpuinfo_max_freq 2803200 <- > > Discrepancy with step 1 as in both cases boost is disabled. > > > > Note that the other way to fix this is to set cpuinfo.max_freq in Soc > > cpufreq driver during initialization. Fixing it in > > cpufreq_frequency_table_cpuinfo seems more generic solution > > > > Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org> > > --- > > drivers/cpufreq/freq_table.c | 8 ++++++-- > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/cpufreq/freq_table.c b/drivers/cpufreq/freq_table.c > > index 67e56cf638ef..6784f94124df 100644 > > --- a/drivers/cpufreq/freq_table.c > > +++ b/drivers/cpufreq/freq_table.c > > @@ -35,11 +35,15 @@ int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy, > > struct cpufreq_frequency_table *pos; > > unsigned int min_freq = ~0; > > unsigned int max_freq = 0; > > + unsigned int cpuinfo_max_freq = 0; > > unsigned int freq; > > cpufreq_for_each_valid_entry(pos, table) { > > freq = pos->frequency; > > + if (freq > cpuinfo_max_freq) > > + cpuinfo_max_freq = freq; > > + > > if (!cpufreq_boost_enabled() > > && (pos->flags & CPUFREQ_BOOST_FREQ)) > > continue; > > @@ -57,8 +61,8 @@ int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy, > > * If the driver has set its own cpuinfo.max_freq above max_freq, leave > > * it as is. > > */ > > - if (policy->cpuinfo.max_freq < max_freq) > > - policy->max = policy->cpuinfo.max_freq = max_freq; > > + if (policy->cpuinfo.max_freq < cpuinfo_max_freq) > > + policy->cpuinfo.max_freq = cpuinfo_max_freq; You need to keep the check of policy->max here and update policy->max, else you will never run at boost freq. I think this is what Steev reported as well ? So basically something like this: if (policy->max < max_freq) policy->max = max_freq; if (policy->cpuinfo.max_freq < cpuinfo_max_freq) policy->cpuinfo.max_freq = cpuinfo_max_freq;
On 11/15/21 10:59 PM, Viresh Kumar wrote: > On 15-11-21, 19:23, Steev Klimaszewski wrote: >> Hi Thara, >> >> On 11/15/21 1:50 PM, Thara Gopinath wrote: >>> cpuinfo.max_freq reflects the maximum supported frequency of cpus in a >>> cpufreq policy. When cpus support boost frequency and if boost is disabled >>> during boot up (which is the default), cpuinfo.max_freq does not reflect >>> boost frequency as the maximum supported frequency till boost is explicitly >>> enabled via sysfs interface later. This also means that policy reports two >>> different cpuinfo.max_freq before and after turning on boost. Fix this by >>> separating out setting of policy->max and cpuinfo.max_freq in >>> cpufreq_frequency_table_cpuinfo. >>> >>> e.g. of the problem. Qualcomm sdm845 supports boost frequency for gold >>> cluster (cpus 4-7). After boot up (boost disabled), >>> >>> 1. cat /sys/devices/system/cpu/cpufreq/policy4/cpuinfo_max_freq 2649600 >>> <- This is wrong because boost frequency is >>> >>> 2. echo 1 > /sys/devices/system/cpu/cpufreq/boost <- Enable boost cat >>> /sys/devices/system/cpu/cpufreq/policy4/cpuinfo_max_freq 2803200 <- >>> max freq reflects boost freq. >>> >>> 3. echo 0 > /sys/devices/system/cpu/cpufreq/boost <- Disable boost cat >>> /sys/devices/system/cpu/cpufreq/policy4/cpuinfo_max_freq 2803200 <- >>> Discrepancy with step 1 as in both cases boost is disabled. >>> >>> Note that the other way to fix this is to set cpuinfo.max_freq in Soc >>> cpufreq driver during initialization. Fixing it in >>> cpufreq_frequency_table_cpuinfo seems more generic solution >>> >>> Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org> >>> --- >>> drivers/cpufreq/freq_table.c | 8 ++++++-- >>> 1 file changed, 6 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/cpufreq/freq_table.c b/drivers/cpufreq/freq_table.c >>> index 67e56cf638ef..6784f94124df 100644 >>> --- a/drivers/cpufreq/freq_table.c >>> +++ b/drivers/cpufreq/freq_table.c >>> @@ -35,11 +35,15 @@ int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy, >>> struct cpufreq_frequency_table *pos; >>> unsigned int min_freq = ~0; >>> unsigned int max_freq = 0; >>> + unsigned int cpuinfo_max_freq = 0; >>> unsigned int freq; >>> cpufreq_for_each_valid_entry(pos, table) { >>> freq = pos->frequency; >>> + if (freq > cpuinfo_max_freq) >>> + cpuinfo_max_freq = freq; >>> + >>> if (!cpufreq_boost_enabled() >>> && (pos->flags & CPUFREQ_BOOST_FREQ)) >>> continue; >>> @@ -57,8 +61,8 @@ int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy, >>> * If the driver has set its own cpuinfo.max_freq above max_freq, leave >>> * it as is. >>> */ >>> - if (policy->cpuinfo.max_freq < max_freq) >>> - policy->max = policy->cpuinfo.max_freq = max_freq; >>> + if (policy->cpuinfo.max_freq < cpuinfo_max_freq) >>> + policy->cpuinfo.max_freq = cpuinfo_max_freq; > > You need to keep the check of policy->max here and update policy->max, > else you will never run at boost freq. I think this is what Steev > reported as well ? Hi Viresh, policy->max is unconditionally set to max_freq in the line before "if (policy->cpuinfo.max_freq < max_freq)". So this is not the issue Steev is reporting. policy->max = max_freq > > So basically something like this: > > if (policy->max < max_freq) > policy->max = max_freq; > > if (policy->cpuinfo.max_freq < cpuinfo_max_freq) > policy->cpuinfo.max_freq = cpuinfo_max_freq; >
Hi Steev, Thanks for testing this. On 11/15/21 8:23 PM, Steev Klimaszewski wrote: --- snip >> >> diff --git a/drivers/cpufreq/freq_table.c b/drivers/cpufreq/freq_table.c >> index 67e56cf638ef..6784f94124df 100644 >> --- a/drivers/cpufreq/freq_table.c >> +++ b/drivers/cpufreq/freq_table.c >> @@ -35,11 +35,15 @@ int cpufreq_frequency_table_cpuinfo(struct >> cpufreq_policy *policy, >> struct cpufreq_frequency_table *pos; >> unsigned int min_freq = ~0; >> unsigned int max_freq = 0; >> + unsigned int cpuinfo_max_freq = 0; >> unsigned int freq; >> cpufreq_for_each_valid_entry(pos, table) { >> freq = pos->frequency; >> + if (freq > cpuinfo_max_freq) >> + cpuinfo_max_freq = freq; >> + >> if (!cpufreq_boost_enabled() >> && (pos->flags & CPUFREQ_BOOST_FREQ)) >> continue; >> @@ -57,8 +61,8 @@ int cpufreq_frequency_table_cpuinfo(struct >> cpufreq_policy *policy, >> * If the driver has set its own cpuinfo.max_freq above >> max_freq, leave >> * it as is. >> */ >> - if (policy->cpuinfo.max_freq < max_freq) >> - policy->max = policy->cpuinfo.max_freq = max_freq; >> + if (policy->cpuinfo.max_freq < cpuinfo_max_freq) >> + policy->cpuinfo.max_freq = cpuinfo_max_freq; >> if (policy->min == ~0) >> return -EINVAL; > > > Something still isn't quite right... > > The setup is that I have an rc.local of > > #!/bin/sh > > echo 1 > /sys/devices/system/cpu/cpufreq/boost > > exit 0 > > > After booting and logging in: > > steev@limitless:~$ cat > /sys/devices/system/cpu/cpufreq/policy4/stats/time_in_state > 825600 2499 > <snip> > 2649600 38 > 2745600 31 > 2841600 1473 > 2956800 0 Did you try debugging this ? As in did you read back boost and cpuinfo_max_freq at this point to ensure that everything is as expected ?
On 11/16/21 9:31 AM, Thara Gopinath wrote: > Hi Steev, > > Thanks for testing this. > > On 11/15/21 8:23 PM, Steev Klimaszewski wrote: > > --- snip >>> >>> diff --git a/drivers/cpufreq/freq_table.c >>> b/drivers/cpufreq/freq_table.c >>> index 67e56cf638ef..6784f94124df 100644 >>> --- a/drivers/cpufreq/freq_table.c >>> +++ b/drivers/cpufreq/freq_table.c >>> @@ -35,11 +35,15 @@ int cpufreq_frequency_table_cpuinfo(struct >>> cpufreq_policy *policy, >>> struct cpufreq_frequency_table *pos; >>> unsigned int min_freq = ~0; >>> unsigned int max_freq = 0; >>> + unsigned int cpuinfo_max_freq = 0; >>> unsigned int freq; >>> cpufreq_for_each_valid_entry(pos, table) { >>> freq = pos->frequency; >>> + if (freq > cpuinfo_max_freq) >>> + cpuinfo_max_freq = freq; >>> + >>> if (!cpufreq_boost_enabled() >>> && (pos->flags & CPUFREQ_BOOST_FREQ)) >>> continue; >>> @@ -57,8 +61,8 @@ int cpufreq_frequency_table_cpuinfo(struct >>> cpufreq_policy *policy, >>> * If the driver has set its own cpuinfo.max_freq above >>> max_freq, leave >>> * it as is. >>> */ >>> - if (policy->cpuinfo.max_freq < max_freq) >>> - policy->max = policy->cpuinfo.max_freq = max_freq; >>> + if (policy->cpuinfo.max_freq < cpuinfo_max_freq) >>> + policy->cpuinfo.max_freq = cpuinfo_max_freq; >>> if (policy->min == ~0) >>> return -EINVAL; >> >> >> Something still isn't quite right... >> >> The setup is that I have an rc.local of >> >> #!/bin/sh >> >> echo 1 > /sys/devices/system/cpu/cpufreq/boost >> >> exit 0 >> >> >> After booting and logging in: >> >> steev@limitless:~$ cat >> /sys/devices/system/cpu/cpufreq/policy4/stats/time_in_state >> 825600 2499 >> <snip> >> 2649600 38 >> 2745600 31 >> 2841600 1473 >> 2956800 0 > > Did you try debugging this ? As in did you read back boost and > cpuinfo_max_freq at this point to ensure that everything is as expected ? > > Hi Thara, I did - sorry I forgot to mention that boost does show 1 for enabled and cpuinfo_max_freq is set to 2956800. However, scaling_max_freq is still listed as 2841600 and scaling_available_frequencies still shows 2841600 as the max available. scaling_boost_freqencies does also list 2956800. steev@limitless:~$ grep . /sys/devices/system/cpu/cpufreq/policy4/* /sys/devices/system/cpu/cpufreq/policy4/affected_cpus:4 5 6 7 grep: /sys/devices/system/cpu/cpufreq/policy4/cpuinfo_cur_freq: Permission denied /sys/devices/system/cpu/cpufreq/policy4/cpuinfo_max_freq:2956800 /sys/devices/system/cpu/cpufreq/policy4/cpuinfo_min_freq:825600 /sys/devices/system/cpu/cpufreq/policy4/cpuinfo_transition_latency:0 /sys/devices/system/cpu/cpufreq/policy4/related_cpus:4 5 6 7 /sys/devices/system/cpu/cpufreq/policy4/scaling_available_frequencies:825600 902400 979200 1056000 1209600 1286400 1363200 1459200 1536000 1612800 1689600 1766400 1843200 1920000 1996800 2092800 2169600 2246400 2323200 2400000 2476800 2553600 2649600 2745600 2841600 /sys/devices/system/cpu/cpufreq/policy4/scaling_available_governors:ondemand conservative powersave userspace performance schedutil /sys/devices/system/cpu/cpufreq/policy4/scaling_boost_frequencies:2956800 /sys/devices/system/cpu/cpufreq/policy4/scaling_cur_freq:1920000 /sys/devices/system/cpu/cpufreq/policy4/scaling_driver:qcom-cpufreq-hw /sys/devices/system/cpu/cpufreq/policy4/scaling_governor:schedutil /sys/devices/system/cpu/cpufreq/policy4/scaling_max_freq:2841600 /sys/devices/system/cpu/cpufreq/policy4/scaling_min_freq:825600 /sys/devices/system/cpu/cpufreq/policy4/scaling_setspeed:<unsupported>
On 11/16/21 10:15 AM, Steev Klimaszewski wrote: > > On 11/16/21 9:31 AM, Thara Gopinath wrote: >> Hi Steev, >> >> Thanks for testing this. >> >> On 11/15/21 8:23 PM, Steev Klimaszewski wrote: >> >> --- snip >>>> >>>> diff --git a/drivers/cpufreq/freq_table.c >>>> b/drivers/cpufreq/freq_table.c >>>> index 67e56cf638ef..6784f94124df 100644 >>>> --- a/drivers/cpufreq/freq_table.c >>>> +++ b/drivers/cpufreq/freq_table.c >>>> @@ -35,11 +35,15 @@ int cpufreq_frequency_table_cpuinfo(struct >>>> cpufreq_policy *policy, >>>> struct cpufreq_frequency_table *pos; >>>> unsigned int min_freq = ~0; >>>> unsigned int max_freq = 0; >>>> + unsigned int cpuinfo_max_freq = 0; >>>> unsigned int freq; >>>> cpufreq_for_each_valid_entry(pos, table) { >>>> freq = pos->frequency; >>>> + if (freq > cpuinfo_max_freq) >>>> + cpuinfo_max_freq = freq; >>>> + >>>> if (!cpufreq_boost_enabled() >>>> && (pos->flags & CPUFREQ_BOOST_FREQ)) >>>> continue; >>>> @@ -57,8 +61,8 @@ int cpufreq_frequency_table_cpuinfo(struct >>>> cpufreq_policy *policy, >>>> * If the driver has set its own cpuinfo.max_freq above >>>> max_freq, leave >>>> * it as is. >>>> */ >>>> - if (policy->cpuinfo.max_freq < max_freq) >>>> - policy->max = policy->cpuinfo.max_freq = max_freq; >>>> + if (policy->cpuinfo.max_freq < cpuinfo_max_freq) >>>> + policy->cpuinfo.max_freq = cpuinfo_max_freq; >>>> if (policy->min == ~0) >>>> return -EINVAL; >>> >>> >>> Something still isn't quite right... >>> >>> The setup is that I have an rc.local of >>> >>> #!/bin/sh >>> >>> echo 1 > /sys/devices/system/cpu/cpufreq/boost >>> >>> exit 0 >>> >>> >>> After booting and logging in: >>> >>> steev@limitless:~$ cat >>> /sys/devices/system/cpu/cpufreq/policy4/stats/time_in_state >>> 825600 2499 >>> <snip> >>> 2649600 38 >>> 2745600 31 >>> 2841600 1473 >>> 2956800 0 >> >> Did you try debugging this ? As in did you read back boost and >> cpuinfo_max_freq at this point to ensure that everything is as >> expected ? >> >> > Hi Thara, > > I did - sorry I forgot to mention that boost does show 1 for enabled > and cpuinfo_max_freq is set to 2956800. However, scaling_max_freq is > still listed as 2841600 and scaling_available_frequencies still shows > 2841600 as the max available. scaling_boost_freqencies does also list > 2956800. > > steev@limitless:~$ grep . /sys/devices/system/cpu/cpufreq/policy4/* > /sys/devices/system/cpu/cpufreq/policy4/affected_cpus:4 5 6 7 > grep: /sys/devices/system/cpu/cpufreq/policy4/cpuinfo_cur_freq: > Permission denied > /sys/devices/system/cpu/cpufreq/policy4/cpuinfo_max_freq:2956800 > /sys/devices/system/cpu/cpufreq/policy4/cpuinfo_min_freq:825600 > /sys/devices/system/cpu/cpufreq/policy4/cpuinfo_transition_latency:0 > /sys/devices/system/cpu/cpufreq/policy4/related_cpus:4 5 6 7 > /sys/devices/system/cpu/cpufreq/policy4/scaling_available_frequencies:825600 > 902400 979200 1056000 1209600 1286400 1363200 1459200 1536000 1612800 > 1689600 1766400 1843200 1920000 1996800 2092800 2169600 2246400 > 2323200 2400000 2476800 2553600 2649600 2745600 2841600 > /sys/devices/system/cpu/cpufreq/policy4/scaling_available_governors:ondemand > conservative powersave userspace performance schedutil > /sys/devices/system/cpu/cpufreq/policy4/scaling_boost_frequencies:2956800 > /sys/devices/system/cpu/cpufreq/policy4/scaling_cur_freq:1920000 > /sys/devices/system/cpu/cpufreq/policy4/scaling_driver:qcom-cpufreq-hw > /sys/devices/system/cpu/cpufreq/policy4/scaling_governor:schedutil > /sys/devices/system/cpu/cpufreq/policy4/scaling_max_freq:2841600 > /sys/devices/system/cpu/cpufreq/policy4/scaling_min_freq:825600 > /sys/devices/system/cpu/cpufreq/policy4/scaling_setspeed:<unsupported> > Once it does start working (e.g. I've run echo 0 to turn off boost, and then echo 1 to turn it back one) steev@limitless:~$ grep . /sys/devices/system/cpu/cpufreq/policy4/* /sys/devices/system/cpu/cpufreq/policy4/affected_cpus:4 5 6 7 grep: /sys/devices/system/cpu/cpufreq/policy4/cpuinfo_cur_freq: Permission denied /sys/devices/system/cpu/cpufreq/policy4/cpuinfo_max_freq:2956800 /sys/devices/system/cpu/cpufreq/policy4/cpuinfo_min_freq:825600 /sys/devices/system/cpu/cpufreq/policy4/cpuinfo_transition_latency:0 /sys/devices/system/cpu/cpufreq/policy4/related_cpus:4 5 6 7 /sys/devices/system/cpu/cpufreq/policy4/scaling_available_frequencies:825600 902400 979200 1056000 1209600 1286400 1363200 1459200 1536000 1612800 1689600 1766400 1843200 1920000 1996800 2092800 2169600 2246400 2323200 2400000 2476800 2553600 2649600 2745600 2841600 /sys/devices/system/cpu/cpufreq/policy4/scaling_available_governors:ondemand conservative powersave userspace performance schedutil /sys/devices/system/cpu/cpufreq/policy4/scaling_boost_frequencies:2956800 /sys/devices/system/cpu/cpufreq/policy4/scaling_cur_freq:1920000 /sys/devices/system/cpu/cpufreq/policy4/scaling_driver:qcom-cpufreq-hw /sys/devices/system/cpu/cpufreq/policy4/scaling_governor:schedutil /sys/devices/system/cpu/cpufreq/policy4/scaling_max_freq:2956800 /sys/devices/system/cpu/cpufreq/policy4/scaling_min_freq:825600 /sys/devices/system/cpu/cpufreq/policy4/scaling_setspeed:<unsupported> Notice that the scaling_max_freq is now 2956800 instead of 2841600 when it isn't working. Sorry for forgetting and sending another mail :( -- steev
On 16-11-21, 10:27, Thara Gopinath wrote: > policy->max is unconditionally set to max_freq in the line before "if > (policy->cpuinfo.max_freq < max_freq)". So the current code is not correct then :)
diff --git a/drivers/cpufreq/freq_table.c b/drivers/cpufreq/freq_table.c index 67e56cf638ef..6784f94124df 100644 --- a/drivers/cpufreq/freq_table.c +++ b/drivers/cpufreq/freq_table.c @@ -35,11 +35,15 @@ int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy, struct cpufreq_frequency_table *pos; unsigned int min_freq = ~0; unsigned int max_freq = 0; + unsigned int cpuinfo_max_freq = 0; unsigned int freq; cpufreq_for_each_valid_entry(pos, table) { freq = pos->frequency; + if (freq > cpuinfo_max_freq) + cpuinfo_max_freq = freq; + if (!cpufreq_boost_enabled() && (pos->flags & CPUFREQ_BOOST_FREQ)) continue; @@ -57,8 +61,8 @@ int cpufreq_frequency_table_cpuinfo(struct cpufreq_policy *policy, * If the driver has set its own cpuinfo.max_freq above max_freq, leave * it as is. */ - if (policy->cpuinfo.max_freq < max_freq) - policy->max = policy->cpuinfo.max_freq = max_freq; + if (policy->cpuinfo.max_freq < cpuinfo_max_freq) + policy->cpuinfo.max_freq = cpuinfo_max_freq; if (policy->min == ~0) return -EINVAL;
cpuinfo.max_freq reflects the maximum supported frequency of cpus in a cpufreq policy. When cpus support boost frequency and if boost is disabled during boot up (which is the default), cpuinfo.max_freq does not reflect boost frequency as the maximum supported frequency till boost is explicitly enabled via sysfs interface later. This also means that policy reports two different cpuinfo.max_freq before and after turning on boost. Fix this by separating out setting of policy->max and cpuinfo.max_freq in cpufreq_frequency_table_cpuinfo. e.g. of the problem. Qualcomm sdm845 supports boost frequency for gold cluster (cpus 4-7). After boot up (boost disabled), 1. cat /sys/devices/system/cpu/cpufreq/policy4/cpuinfo_max_freq 2649600 <- This is wrong because boost frequency is 2. echo 1 > /sys/devices/system/cpu/cpufreq/boost <- Enable boost cat /sys/devices/system/cpu/cpufreq/policy4/cpuinfo_max_freq 2803200 <- max freq reflects boost freq. 3. echo 0 > /sys/devices/system/cpu/cpufreq/boost <- Disable boost cat /sys/devices/system/cpu/cpufreq/policy4/cpuinfo_max_freq 2803200 <- Discrepancy with step 1 as in both cases boost is disabled. Note that the other way to fix this is to set cpuinfo.max_freq in Soc cpufreq driver during initialization. Fixing it in cpufreq_frequency_table_cpuinfo seems more generic solution Signed-off-by: Thara Gopinath <thara.gopinath@linaro.org> --- drivers/cpufreq/freq_table.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)