Messages in this thread | | | Subject | Re: [PATCH] cpufreq: freq_table: Initialize cpuinfo.max_freq to correct max frequency. | From | Thara Gopinath <> | Date | Tue, 16 Nov 2021 10:27:33 -0500 |
| |
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; >
-- Warm Regards Thara (She/Her/Hers)
| |