lkml.org 
[lkml]   [2020]   [Jun]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH v2 2/2] cpufreq: Specify default governor on command line
    On Thu, Jun 25, 2020 at 3:50 PM Quentin Perret <qperret@google.com> wrote:
    >
    > On Thursday 25 Jun 2020 at 15:28:43 (+0200), Rafael J. Wysocki wrote:
    > > On Thu, Jun 25, 2020 at 1:53 PM Quentin Perret <qperret@google.com> wrote:
    > > >
    > > > On Thursday 25 Jun 2020 at 13:44:34 (+0200), Rafael J. Wysocki wrote:
    > > > > On Thu, Jun 25, 2020 at 1:36 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
    > > > > > This change is not right IMO. This part handles the set-policy case,
    > > > > > where there are no governors. Right now this code, for some reasons
    > > > > > unknown to me, forcefully uses the default governor set to indicate
    > > > > > the policy, which is not a great idea in my opinion TBH. This doesn't
    > > > > > and shouldn't care about governor modules and should only be looking
    > > > > > at strings instead of governor pointer.
    > > > >
    > > > > Sounds right.
    > > > >
    > > > > > Rafael, I even think we should remove this code completely and just
    > > > > > rely on what the driver has sent to us. Using the selected governor
    > > > > > for set policy drivers is very confusing and also we shouldn't be
    > > > > > forced to compiling any governor for the set-policy case.
    > > > >
    > > > > Well, AFAICS the idea was to use the default governor as a kind of
    > > > > default policy proxy, but I agree that strings should be sufficient
    > > > > for that.
    > > >
    > > > I agree with all the above. I'd much rather not rely on the default
    > > > governor name to populate the default policy, too, so +1 from me.
    > >
    > > So before this series the default governor was selected at the kernel
    > > configuration time (pre-build) and was always built-in. Because it
    > > could not go away, its name could be used to indicate the default
    > > policy for the "setpolicy" drivers.
    > >
    > > After this series, however, it cannot be used this way reliably, but
    > > you can still pass cpufreq_param_governor to cpufreq_parse_policy()
    > > instead of def_gov->name in cpufreq_init_policy(), can't you?
    >
    > Good point. I also need to fallback to the default builtin governor if
    > the command line parameter isn't valid (or non-existent), so perhaps
    > something like so?

    Yes, that should work if I haven't missed anything.

    > iff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
    > index dad6b85f4c89..20a2020abf88 100644
    > --- a/drivers/cpufreq/cpufreq.c
    > +++ b/drivers/cpufreq/cpufreq.c
    > @@ -653,6 +653,23 @@ static unsigned int cpufreq_parse_policy(char *str_governor)
    > return CPUFREQ_POLICY_UNKNOWN;
    > }
    >
    > +static unsigned int cpufreq_default_policy(void)
    > +{
    > + unsigned int pol;
    > +
    > + pol = cpufreq_parse_policy(cpufreq_param_governor);
    > + if (pol != CPUFREQ_POLICY_UNKNOWN)
    > + return pol;
    > +
    > + if (IS_BUILTIN(CONFIG_CPU_FREQ_DEFAULT_GOV_PERFORMANCE))
    > + return CPUFREQ_POLICY_PERFORMANCE;
    > +
    > + if (IS_BUILTIN(CONFIG_CPU_FREQ_DEFAULT_GOV_POWERSAVE))
    > + return CPUFREQ_POLICY_POWERSAVE;
    > +
    > + return CPUFREQ_POLICY_UNKNOWN;
    > +}
    > +
    > /**
    > * cpufreq_parse_governor - parse a governor string only for has_target()
    > * @str_governor: Governor name.
    > @@ -1085,8 +1102,8 @@ static int cpufreq_init_policy(struct cpufreq_policy *policy)
    > /* Use the default policy if there is no last_policy. */
    > if (policy->last_policy) {
    > pol = policy->last_policy;
    > - } else if (default_governor) {
    > - pol = cpufreq_parse_policy(default_governor->name);
    > + } else {
    > + pol = cpufreq_default_policy();
    > /*
    > * In case the default governor is neiter "performance"
    > * nor "powersave", fall back to the initial policy

    \
     
     \ /
      Last update: 2020-06-25 16:09    [W:4.696 / U:0.080 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site