Received-SPF: pass (google.com: domain of linux-kernel+bounces-213782-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <cc26e5fd-ee67-4720-bc58-322fa1ed0329@amd.com>
Date: Thu, 13 Jun 2024 12:55:20 -0500
User-Agent: Mozilla Thunderbird
From: Mario Limonciello <mario.limonciello@amd.com>
Subject: Re: [PATCH v11 3/9] cpufreq: introduce init_boost callback to
 initialize boost state for pstate drivers
To: Perry Yuan <perry.yuan@amd.com>, gautham.shenoy@amd.com,
 Borislav.Petkov@amd.com
Cc: rafael.j.wysocki@intel.com, viresh.kumar@linaro.org,
 Alexander.Deucher@amd.com, Xinmei.Huang@amd.com, Xiaojian.Du@amd.com,
 Li.Meng@amd.com, linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org
References: <cover.1718262992.git.perry.yuan@amd.com>
 <688d8e1ce875655c344ac3b29876da5dc0456739.1718262992.git.perry.yuan@amd.com>
Content-Language: en-US
In-Reply-To: <688d8e1ce875655c344ac3b29876da5dc0456739.1718262992.git.perry.yuan@amd.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?UURab1BITUs3S3JZZ1p6N2FLWGtqYUs2UzE0OEU1MmVDT1ZUZ1pRQUlDNlVG?=
 =?utf-8?B?VDROZG02VWhHMXI3REdsNnEwZ0VYa1JHS3BEbkE2OUJjbm45d0tneUVHQTFT?=
 =?utf-8?B?ZHdrS2daUW9xWXZqZXE0c0k3VzdaMHdwYzlGWSt2OVdGcWlUbVVUcm15Vlgy?=
 =?utf-8?B?R3dXMVV3UCtVWVptbUtRejRidE9KZnk0b1VkMkdkL09hYWVGSVRKdHkxMDZ4?=
 =?utf-8?B?enVSWmViL2R1Z3ZvWGhvRnNiUGFPSitWN0I1Z0pmTVIwT3lJYk5EekY2Y3I0?=
 =?utf-8?B?MHdmTCtCNjMyUDFqNTlRZ0ROZlF0QVp3bzlwcGpPQUhwdklrMElDb0FpTllJ?=
 =?utf-8?B?b1hvNURXcmcyRUxNeUh4OVFmTkd6ZVcrN09qSjUvSlpxKzJscm9TQjhaSjQy?=
 =?utf-8?B?UTBwUytxMVJlVUowRGdzZ3VnWitVZXRiUXMwcWY4MjBUcFR1WFZUT2hmWmxh?=
 =?utf-8?B?OHB4UStlMDZGeGJYU0djZis0dkhnZVhVaFhFaEMzQks5N0xZY294ZUhabmdO?=
 =?utf-8?B?U3IyUU44SCtFUWdRYzV4c2VxMWFUb3RVOTAyUWZZLyt6dzN4cXZZWTA1YTFE?=
 =?utf-8?B?U0JpT0FIM29pWUNvdXFuNE1hSyt4V1E4MmZUSXJGL2UvblpBMWtqUzVhb1lq?=
 =?utf-8?B?MTBseHFVRjV4N3hOdnRmc3oxM004LzFxMXZ4WWUzbWQvVFFLZ1VUanllZTZC?=
 =?utf-8?B?T1FGamgrWTAybDN5M2RkaXliRTErYnFWUUk5UU9UN29sR2tmaWtZUTVxeTJR?=
 =?utf-8?B?RTFBdnU2Z3REWVowKzUzcW01TTdoY0sreitNYXdIME9IdzRyU0lIa0R0V1lW?=
 =?utf-8?B?dHJOTnFWZlBoY2hnSEw0d2UxeHM5UmZiVGxhbUc3U2ZoWUVxSVEweTQ3RWdx?=
 =?utf-8?B?YmFGNURSR1FZSlo2MDNQaDgxcEhrYzNhVGFYeFg3RUJyanVUdlMyRWR3aG5n?=
 =?utf-8?B?VVBFSUs2QU5ZQkpKaGI3bnVXUlVhL29hQld4dlEzKzRaUzE4N2x6NFNLWWsx?=
 =?utf-8?B?cHY1cnkwQkx5MWFteE40cnJXQk9SM3pXNTZpOUk4eFNqRkRPWXRnalFuays5?=
 =?utf-8?B?N1pnYU4veWVXbFl3MkRqN05FWmYwWWZzL0p4V08xK1YvU01JN3g0V0pxNzMv?=
 =?utf-8?B?WXIxTkdkRU1QN0FiOTJVTmZmMG15WUl6QjBzSWVZMUg3TXBFZ0RKM1QxZGNR?=
 =?utf-8?B?WDB5ZS9HazloRkJkczR1SFdTVjkzRjUxSkxaVEUzWE42ZUoybWJmVjRmQmVP?=
 =?utf-8?B?Mk03K09RL2ZnSE9XK2FrNHdRRlcvaXVXZW5sWVVZRVRWbm9JbUxlNys5WTBE?=
 =?utf-8?B?QmIzTk9jQlRMTm9yWWpQdmFMN201QU1QS1h1OHQvaWNGR2NDOTQyZEFOQzVv?=
 =?utf-8?B?ZzhrWjEzWWlYMnd3RjdxS2FPNUY0YS8rUEgvTlB2eCtwaE5SZkdYc0dpVzFZ?=
 =?utf-8?B?R0tSdzM4dklGZGRObHJDSWRBQ3RWcTFjOTZ2amxHN2RCM1drVGhXbk9JVWxI?=
 =?utf-8?B?cGxubTl6TW1VaXZjbFJrZGlGM2dWQUlEc3ZNeXhQZUcvYUhUQnhFQk1jUlUy?=
 =?utf-8?B?QzRqbnBudFgwK2FTcExRdDZRcHViK3NidmxVU0g4U2ovc3ZjcFh1VzdoTjVz?=
 =?utf-8?B?VEVDK0xDb2NWbnNpaENuamJJOHltOXlhYzVOR1dBbEViWWJQRFpVWXhaRmFP?=
 =?utf-8?B?eWlSUzZmZHRtTDlnOHl3NTJKSHBJdG9EbVVpTU5pNTdlUHBPMStmcEYxUGcr?=
 =?utf-8?B?VlVWV1kzV3hmSjNPNzY3bnRUQm9YNnpmR0MrMUVWeHR3a1hDSnhmKzhNQ2FZ?=
 =?utf-8?B?QnN5bXI0N1IxTk9iQ1RkWUxTRGlzUWRBMDU3MUptSHFmcTJCSWZvbndqaHpq?=
 =?utf-8?B?QWxHVVRjSjVHZGJ0SU9icWI5bnNZS2w0dXZ6ZU8wYzlzU2labUhTdlBHSkdw?=
 =?utf-8?B?YUYvRUdLNkUzTkZFMDlQOU93Skw3ZnRQeU41YjBUSDdPZ0xLcTY0U2RTMXdK?=
 =?utf-8?B?Y2FqUjI4Ny94cXdlWjlXQ0RXV2l1dXVXdjJFTzYzOWtLbVJTTVRvQ2Y3N2JI?=
 =?utf-8?B?R0l1TDlzZUxIZmhVbW9yczA2MlhBcXZmOHZwdUNycFMwUkhjNG1zZ1lvRmQ5?=
 =?utf-8?Q?XFUuY3qxzsK7RJvLP+Ha/CvE+?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 61c618c9-fed1-4a73-281e-08dc8bd1fe78
X-MS-Exchange-CrossTenant-AuthSource: MN0PR12MB6101.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Jun 2024 17:55:21.5921
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: CZug2hbrv9My7BRrftMXVF19WQ4eEWxIF1e3RF7bBCiDBX+wN8QNMftgnjsobyjDhruXKXzy+bqgYzeDvTWeSQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH3PR12MB8074

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@amd.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.