Re: [PATCH] cpufreq: freq_table: Initialize cpuinfo.max_freq to correct max frequency.

[Thara Gopinath]

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 ?


--
Warm Regards
Thara (She/Her/Hers)
> 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