Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <7cdf6256-d0be-0037-4ec1-405ad10f90fe@amd.com>
Date:   Mon, 14 Nov 2022 16:46:03 -0600
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.1
Subject: Re: [PATCH v4 3/9] cpufreq: amd-pstate: change amd-pstate driver to
 be built-in type
Content-Language: en-US
To:     Perry Yuan <Perry.Yuan@amd.com>, rafael.j.wysocki@intel.com,
        ray.huang@amd.com, viresh.kumar@linaro.org
Cc:     Deepak.Sharma@amd.com, Nathan.Fontenot@amd.com,
        Alexander.Deucher@amd.com, Shimmer.Huang@amd.com,
        Xiaojian.Du@amd.com, Li.Meng@amd.com, wyes.karny@amd.com,
        linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20221110175847.3098728-1-Perry.Yuan@amd.com>
 <20221110175847.3098728-4-Perry.Yuan@amd.com>
From:   "Limonciello, Mario" <mario.limonciello@amd.com>
In-Reply-To: <20221110175847.3098728-4-Perry.Yuan@amd.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?T1RJMnRkUTZGaFB3TER6UDVzMDVIYjRZdmEyaE92dnJka3ZOV3hUVVlWa3VW?=
 =?utf-8?B?eDdqUmpXT04vODdyWE5sUnV3d2Z6RXJQMC9Uamt3NDJFdTROaGdaVWFuL0VV?=
 =?utf-8?B?QXdGWFFTNUNCaTFBcHVjMFBnVWE3aUZlWFhLK2F2Y3dEMzhObnNId1Fpbk1v?=
 =?utf-8?B?OEUvRjc5eHRzNjhNQWhpTmNpT01oMXJpL1hnN2NVL2FhV2pPeEd1NE9qb2w3?=
 =?utf-8?B?aFZsblBoWkIzN1RQVm1OdDRwemlmZCs0S2FaWEVpNGdZNndMWG52cnBxZmht?=
 =?utf-8?B?RU1LZzlHVEtmbUNKQVVOVEJsNGhqNkJsMHdSWFZmYTVjVkcxWXJRMTRUY0xC?=
 =?utf-8?B?KzNuU1pXbVRLK3YxMisxWXNJVUVvbkpDRWgvTFhKTHU3aFdxTUhGNkUrTE1s?=
 =?utf-8?B?VCsrSGNQNEptczNXRDJLNDBNT3k0TU01ZUFpMXFTUGdVTDF5RVJpRnV1ZXB1?=
 =?utf-8?B?dUdpTUtZMnZFZmJRUS94Rk92eENoaWtNRitqWHppSktPQVp3VU5GbFBlSDlr?=
 =?utf-8?B?VTg5QXByRzVVOGhzamVhZW9GNHk1YUt1Z2prRGNRbjlvbU4vTnVBS0ZCZFNO?=
 =?utf-8?B?R3VuVUt3eEJpNnQ5NWkxYzV4VVluUUdQNDRKdVpENy82djBoSlVJYmlGRnBJ?=
 =?utf-8?B?Y0RmbU1RaXdzcFVCaVB0eVN4djhKdXR4YXhLV05IbjU1MFNSRHZsNzVaSW5w?=
 =?utf-8?B?NzFKc2VKK1cvODc4N25KR3lIL3BCYzM4TlVRTkVMVThsbHBFdVA3Y3lac0Jo?=
 =?utf-8?B?TnVaTXU2NEdoQVRpb3pVR2xURW5kbVBXVjBFdGFHTXpJWFlIWGxrOG5XZDVJ?=
 =?utf-8?B?SzQzdEFKMUZXWWdDeHlTUlhuODFDUndvVUt4ZGVldndwZHJNVDF6ay9uck5G?=
 =?utf-8?B?a0ppUUFTc1hDWWNUU2hYZXhBbkk1cjVVS0dGaDkwZG9ZK0l1dnI2WkJLMFJE?=
 =?utf-8?B?Sk9MTC9kcUNoZFg2T1Jaby9Ia1BVS0k0L3RFalY0SGo2WjF3NXZjUzMvRnJE?=
 =?utf-8?B?ZFFkSDZ4M1dsSXdieHZMNjJldzdER0RkUEswQWtZci9Nc3BDdnpXVXFUamRT?=
 =?utf-8?B?TDdrWVNBZ21pUkEwQU9Bci9IU0gxUWFNNG5JTGNZbDc0MGkweEJ3ek5kWVB1?=
 =?utf-8?B?dWp2OXpDT013cWYzcmhFaFlXVkF4WFBsSTZKbThKRTRGQlNvbEZ0K2RCMGRM?=
 =?utf-8?B?MjFzVnlYUTlPSmY1K1Jta0FrVW1WT2xFYktOblQzOVEyMUxGV2F5b1pGQ091?=
 =?utf-8?B?SUk0UWx5amhCR0xvVUhuOGJoTVd5Ylg3SiswZERHQUF4RzJlelFBM1VIblp0?=
 =?utf-8?B?dEllcG5Hc1dGRDV5RDF0Q2xkUFRNWjFERE5IRVZLYTgrKzhXU2FNNE9nTU4v?=
 =?utf-8?B?NmI4Z1BwUkZYUnFJTTVnZTY2ZWg5RTZqNS9WKzZKZlJDM1IzYXQ3OHcwWEJ1?=
 =?utf-8?B?RkUxVVZaTnFKaHV3MG42NEtTeHo0LzgwaU5iSHRRcDVrUlZsNk13TzJRcnFT?=
 =?utf-8?B?dW5Ub3oySVZwcy95bFdGOFlpYkFQWmYybGc1VjRLRzJUQTQ2bmlVdWFxeWcx?=
 =?utf-8?B?UVA4RVNPRDZUT0ZhYmFYTy9CTytCMklmSlNYRXV4bFpTNXVyS3VJa09XNVZU?=
 =?utf-8?B?bndaQnZ1UDVYZWNQUEVicHhmNE0zZ2tQdTZCSTk4REVPU1pnMVVuZitzWUVR?=
 =?utf-8?B?YW5VSzFkc1BxNXIxN1Nud1FkeVg1a0tuRW5iWk4vTW9ESC9xdENzVG9PS1BV?=
 =?utf-8?B?Q2hkNnpCR3pEN0lQTEsvVHkwWW0zcVNMMDhqUjlWczRBc0h4bjkxR21GeHFz?=
 =?utf-8?B?VC84TVZrMDZXUDRBSlRvWVRuWktBUVlUYXBWUVlxWUtoaHYyeGR3amNyQ1lI?=
 =?utf-8?B?UVZ3NGhOTHdncGFTalN4Y0pnN3B3QlcxTDB0SjljcmdsWndzekN3TStnVExk?=
 =?utf-8?B?OVUxVUxYWmptTTZWblFwY3JhcEIvMDVFZzAwNFk4RmFKWjM5MDdjcEhKTjRX?=
 =?utf-8?B?d2doYkZxSlJ1bGtxSHZjbWUvNDlWaWdqYkVKbDlKaWxHWXdMTm1rSnJqY2F6?=
 =?utf-8?B?Q0NvOWpaTWJJQjFnZStNZG1DbksxV0k1akJXT3p6dWFQTlFHMVZCcnppNTNB?=
 =?utf-8?Q?bgeOcFAXy743XtbxgCHatGL54?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: d651e76b-cc39-458a-593d-08dac6920453
X-MS-Exchange-CrossTenant-AuthSource: MN0PR12MB6101.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 Nov 2022 22:46:06.7380
 (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: gKXg3bciPSDAw3o+WP/49uSAL86xWDNtmq6Tu+JVI7D7Nm9sFW9PI60Epka5xH118UbYncYg/k5btVq8gZswtA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH2PR12MB5003
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 11/10/2022 11:58, Perry Yuan wrote:
> Change the `amd-pstate` driver as the built-in type which can help to
> load the driver before the acpi_cpufreq driver as the default pstate
> driver for the AMD processors.
> 
> for the processors do not have the dedicated MSR functions, add
> `amd-pstate=legacy_cppc` to grub which enable shared memory interface
> to communicate with cppc_acpi module to control pstate hints.

Did you sync with Wyes already as I had suggested?  Was this the outcome?

I was a bit surprised to see this still as legacy_cppc when reviewing v4.

> 
> Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
> ---
>   drivers/cpufreq/Kconfig.x86  |  2 +-
>   drivers/cpufreq/amd-pstate.c | 25 +++++++++++++++----------
>   2 files changed, 16 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/cpufreq/Kconfig.x86 b/drivers/cpufreq/Kconfig.x86
> index 310779b07daf..00476e94db90 100644
> --- a/drivers/cpufreq/Kconfig.x86
> +++ b/drivers/cpufreq/Kconfig.x86
> @@ -35,7 +35,7 @@ config X86_PCC_CPUFREQ
>   	  If in doubt, say N.
>   
>   config X86_AMD_PSTATE
> -	tristate "AMD Processor P-State driver"
> +	bool "AMD Processor P-State driver"
>   	depends on X86 && ACPI
>   	select ACPI_PROCESSOR
>   	select ACPI_CPPC_LIB if X86_64
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index ace7d50cf2ac..85a0b3fb56c2 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -59,10 +59,7 @@
>    * we disable it by default to go acpi-cpufreq on these processors and add a
>    * module parameter to be able to enable it manually for debugging.
>    */
> -static bool shared_mem = false;
> -module_param(shared_mem, bool, 0444);
> -MODULE_PARM_DESC(shared_mem,
> -		 "enable amd-pstate on processors with shared memory solution (false = disabled (default), true = enabled)");
> +static bool shared_mem __read_mostly;
>   
>   static struct cpufreq_driver amd_pstate_driver;
>   
> @@ -653,16 +650,24 @@ static int __init amd_pstate_init(void)
>   
>   	return ret;
>   }
> +device_initcall(amd_pstate_init);
>   
> -static void __exit amd_pstate_exit(void)
> +static int __init amd_pstate_param(char *str)
>   {
> -	cpufreq_unregister_driver(&amd_pstate_driver);
> +	if (!str)
> +		return -EINVAL;
>   
> -	amd_pstate_enable(false);
> -}
> +	/*
> +	 * support shared memory type CPPC which has no MSR function.
> +	 * enable amd-pstate on processors with shared memory solution
> +	 * (amd-pstate=legacy_cppc enabled), it is disabled by default.
> +	 */
> +	if (!strcmp(str, "legacy_cppc"))
> +		shared_mem = true;
>   
> -module_init(amd_pstate_init);
> -module_exit(amd_pstate_exit);
> +	return 0;
> +}
> +early_param("amd-pstate", amd_pstate_param);

Documentation/kernel-parameters.txt needs to be updated for this early 
parameter support.

>   
>   MODULE_AUTHOR("Huang Rui <ray.huang@amd.com>");
>   MODULE_DESCRIPTION("AMD Processor P-state Frequency Driver");