[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <cc26e5fd-ee67-4720-bc58-322fa1ed0329@amd.com>
Date: Thu, 13 Jun 2024 12:55:20 -0500
From: Mario Limonciello <mario.limonciello@....com>
To: Perry Yuan <perry.yuan@....com>, gautham.shenoy@....com,
Borislav.Petkov@....com
Cc: rafael.j.wysocki@...el.com, viresh.kumar@...aro.org,
Alexander.Deucher@....com, Xinmei.Huang@....com, Xiaojian.Du@....com,
Li.Meng@....com, linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v11 3/9] cpufreq: introduce init_boost callback to
initialize boost state for pstate drivers
On 6/13/2024 02:25, Perry Yuan wrote:
> Introduce a new init_boost callback in cpufreq to initialize the boost
> state for specific pstate drivers. This initialization is required before
> calling the set_boost interface for each CPU.
>
> The init_boost callback will set up and synchronize each CPU's current
> boost state before invoking the set_boost function. Without this step,
> the boost state may be inconsistent across CPUs.
>
> Signed-off-by: Perry Yuan <perry.yuan@....com>
> ---
> drivers/cpufreq/cpufreq.c | 12 ++++++++++--
> include/linux/cpufreq.h | 2 ++
> 2 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 1fdabb660231..e1a4730f4f8c 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1429,8 +1429,16 @@ static int cpufreq_online(unsigned int cpu)
> goto out_free_policy;
> }
>
> - /* Let the per-policy boost flag mirror the cpufreq_driver boost during init */
> - policy->boost_enabled = cpufreq_boost_enabled() && policy_has_boost_freq(policy);
> + /* init boost state to prepare set_boost callback for each CPU */
> + if (cpufreq_driver->init_boost) {
> + ret = cpufreq_driver->init_boost(policy);
> + if (ret)
> + pr_debug("%s: %d: initialization boost failed\n", __func__,
> + __LINE__);
The message should have the subject at the beginning. IE:
"boost initialization failed\n"
Also, isn't this fatal if init failed? IE shouldn't failing have a:
goto out_free_policy;
> + } else {
> + /* Let the per-policy boost flag mirror the cpufreq_driver boost during init */
> + policy->boost_enabled = cpufreq_boost_enabled() && policy_has_boost_freq(policy);
> + }
>
> /*
> * The initialization has succeeded and the policy is online.
> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> index 20f7e98ee8af..0698c0292d8f 100644
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -409,6 +409,8 @@ struct cpufreq_driver {
> bool boost_enabled;
> int (*set_boost)(struct cpufreq_policy *policy, int state);
>
> + /* initialize boost state to be consistent before calling set_boost */
> + int (*init_boost)(struct cpufreq_policy *policy);
> /*
> * Set by drivers that want to register with the energy model after the
> * policy is properly initialized, but before the governor is started.
Powered by blists - more mailing lists