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: <85925bce-f30a-b2df-309f-bd4b510a117c@amd.com>
Date:   Thu, 12 Jan 2023 23:37:03 -0600
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.5.1
Subject: Re: [PATCH v2 1/6] acpi: cppc: Add min and max perf reg writing
 support
Content-Language: en-US
To:     Wyes Karny <wyes.karny@amd.com>,
        Rafael J Wysocki <rafael@kernel.org>,
        Huang Rui <ray.huang@amd.com>,
        Jonathan Corbet <corbet@lwn.net>,
        Viresh Kumar <viresh.kumar@linaro.org>, Perry.Yuan@amd.com,
        Ananth Narayan <ananth.narayan@amd.com>, gautham.shenoy@amd.com
Cc:     linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-pm@vger.kernel.org, Bagas Sanjaya <bagasdotme@gmail.com>,
        santosh.shukla@amd.com
References: <20230113052141.2874296-1-wyes.karny@amd.com>
 <20230113052141.2874296-2-wyes.karny@amd.com>
From:   Mario Limonciello <mario.limonciello@amd.com>
In-Reply-To: <20230113052141.2874296-2-wyes.karny@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?SGJjK3o0Mk54bitjWVQ4eTBZd2xSajJQaGF2OVNVcVIrV2p0L3YzelJ2U0Nk?=
 =?utf-8?B?WGEyT3ZqNkVSSFg3dDBZK1NhN0lBaDYrYjhRV0tkTDBIMjhDbzhSZzMzeXhh?=
 =?utf-8?B?dnU5ME9KbWREOWFmRy9CWUhEV05EOUZ3eHVoM0prZk5Nd1pROWFzTmdtU08r?=
 =?utf-8?B?SkZoNEhJWm9BVlRXcm45VU9hY3VoNWsyK0pmSFF6ck1ON3ArMG5hc0d0YnEr?=
 =?utf-8?B?cjlSTGNvTFZYMmdVZlIvRlpvUUdUSVpFUmx3NEw3MWFZWXk3K3VHY1hNYS9K?=
 =?utf-8?B?TC9BaTVJbW1iSy8rend2R08rbjFKejEwVUZ3akllRjNBMVV5R0dvUWVrcmVo?=
 =?utf-8?B?ZjlzR0RzVkxkS1UvazVGdVZDUldZYWJSNmttRUkzUjBxRldtcGZabTZOWjFy?=
 =?utf-8?B?L2xwVnAxMnFnem5pcnpIQTA3dklNbmJxMHJiRlJ6SnZ0SjVGZ0RhZ2xmeVox?=
 =?utf-8?B?S1pTb1dmRml0M28rUFFFOFNyTUpGSG51czNHTVFpNm90NGFpb1l5QUZicWVF?=
 =?utf-8?B?cno3TGZYUzRNeVRDMXY4bjd5eC9aL1lLaEh5N00wZW0yR05aNzhCS3UxL0Uw?=
 =?utf-8?B?RElTTlZrYkoxbFFkYnN1NjkvOGN1YVpxT1l6VWtwU1hlMklaTjQyRlA2NG83?=
 =?utf-8?B?c1NQWnRrYVdzY3BGdlVobm13bllYSTRZdjBMUW9OZ1ZhNXlaaDhUZnh4N0k2?=
 =?utf-8?B?UnBTWVIvUXhSSzFIYTJKWlJxTDRpYldwZHo0UUJRUnJjZXZYb0d5Y0dIcjFX?=
 =?utf-8?B?Z0EyQ2F3bGpXS2ZDOFhhNlhSWFpKV2xucEpZQXdxQmVYN2FoeldEbHo0VVMx?=
 =?utf-8?B?eVNjMlFrdFUreVJ0UHFpQ0g5T3doWk9CUldqd2psa3p3QjF0NDQvUzh3elFt?=
 =?utf-8?B?cHJINjVPRDNGdk1pZTVOdW1nR3ZQeEhwMkJiM2RMcmF4RGtkeVdIeHBST1NS?=
 =?utf-8?B?bG1sZHFoSzRWRHZQSmo5b29QenFEb2JvQ24vZDZQWmcvblhha1lYUlVhaDJ6?=
 =?utf-8?B?MzdPUkN6bE1tOWRneko0Ny9XZzdDSS9VRlJHQUpLL3lLOVE1RU1iR0NqdXRy?=
 =?utf-8?B?QWRUbVRxZ0c5QmJZc09DbGRzMjlpMnVmWXZBOS9JVWdlWkZ2RU5QOGF2U3NE?=
 =?utf-8?B?V3hUNU9qU252eVBZR3lPZVViVU9BeDVHMHhheVpWWHhROGlJT2ZIVFlvSHlw?=
 =?utf-8?B?Q1dpQU9ib29HVkpodTM0NTA2VUJsekE5cjRLRHp2eDBlNnN5Vk9CTTJmRWRG?=
 =?utf-8?B?T1d4T0tjZmlxbUhhZTNMMGxMbmhURFJZNFNIK0ZEbUJpc1J1VkpLV1hTSWFI?=
 =?utf-8?B?QlM4a2NubDM5L0hSeEdOTStnOThydWoyblBCSktMaTJzbW5rV0Vya0RURGN6?=
 =?utf-8?B?VHRZZHJENGdnM2t5TFpDdHhVaVBNeWJHMGl5WjNXKzB6aUl1alNlSEhkam1V?=
 =?utf-8?B?bi9SZVVzN2t3UUtsc29INGZkUDhYZEwyTFhTZW1KV0cwMmFZck9tbUJ1UkxD?=
 =?utf-8?B?ZUhQbFhkaFRJTW1DMmFRNkVzRmw5cUthUENWbmxQVlNSeE5kUFRRYk1CaWdh?=
 =?utf-8?B?U3gxTGRwZUlKOVdTSnY2ZG51LzRYRFgxNUd1a3hkdUlmYURZTHlFOXIzOHR4?=
 =?utf-8?B?U1lUN0tiaUI1NGFCV2hrSHpGN1pLZ0NvVjJ0T0lNZ293UktMbkpuWWtMZDhJ?=
 =?utf-8?B?czVqWVIvOVNKOUJnY1A1bUoxMzI3R1pqdzV4TWR1ZnJuWTYwMjc3OVdYQ2FO?=
 =?utf-8?B?NEdlbEpINUlmRjN1WlFDZG5MeDZnZWZKcTB6ZCtHVUlOQ1pnREMwRmxZOFNG?=
 =?utf-8?B?UVY4dFpFZFZzS05OdTdRNVQ5aEplWUhrWUxJd3k0WlBhb2x5NGE5ZjZ0WjUv?=
 =?utf-8?B?UEtDOVM0RFFidE5kYXl1SFN4MGVPVFdrUkRhaUY4TUZPMEdYM2krc1ZUaE81?=
 =?utf-8?B?NHF6bjE2QWEycnNVWVd2QUVydEJVZ2cyQ3ZLYXN1dGZFbXg2R2pmNHoycTlK?=
 =?utf-8?B?VGtIRDlxNy9OYnlsSkdrb2RsV0h3RW1IcGFNa1JnNjFLWDgyODFnOHVzOHdE?=
 =?utf-8?B?RUpSRmljRHE4Sy91TlVNTUpNWFhBbVY1TWMwQVVIakhmY3ZKY1greDRwOXRj?=
 =?utf-8?B?TFBONEdJaHBTSDYxRnZweExwT0MvL2Z5VXc3Vkw5OUQ1WVZsNGVIQ0hibGxw?=
 =?utf-8?Q?cw/wgjbeJpNW7s4sDVCvNOd8qOH64HYvWNIG+3j6FQ/f?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 67d5ce29-7681-4f9d-67ae-08daf5283547
X-MS-Exchange-CrossTenant-AuthSource: MN0PR12MB6101.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Jan 2023 05:37:06.9902
 (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: 9ybvW0OhsRc6IJcQSMWYxPmYSpFTwCAf4x4bhnPtMxouwvCgabBnxIaehjTqNwFa6BuEoeZsEId5C+sV4q6zFQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ1PR12MB6146
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 1/12/23 23:21, Wyes Karny wrote:
> Currently writing of min and max perf register is deferred in
> cppc_set_perf function. In CPPC guided mode, these registers needed to
> be written to guide PMFW about min and max perf levels. Add this support

This is generic code, so  I think rather than PMFW you should just say 
"the platform".

> to make guided mode work properly on shared memory systems.

on AMD shared memory systems.

> 
> Signed-off-by: Wyes Karny <wyes.karny@amd.com>

With the commit message cleaned up:

Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>   drivers/acpi/cppc_acpi.c | 16 ++++++++--------
>   1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
> index 02d83c807271..c936ff503965 100644
> --- a/drivers/acpi/cppc_acpi.c
> +++ b/drivers/acpi/cppc_acpi.c
> @@ -1487,7 +1487,7 @@ EXPORT_SYMBOL_GPL(cppc_set_enable);
>   int cppc_set_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls)
>   {
>   	struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpu);
> -	struct cpc_register_resource *desired_reg;
> +	struct cpc_register_resource *desired_reg, *min_perf_reg, *max_perf_reg;
>   	int pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpu);
>   	struct cppc_pcc_data *pcc_ss_data = NULL;
>   	int ret = 0;
> @@ -1498,6 +1498,8 @@ int cppc_set_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls)
>   	}
>   
>   	desired_reg = &cpc_desc->cpc_regs[DESIRED_PERF];
> +	min_perf_reg = &cpc_desc->cpc_regs[MIN_PERF];
> +	max_perf_reg = &cpc_desc->cpc_regs[MAX_PERF];
>   
>   	/*
>   	 * This is Phase-I where we want to write to CPC registers
> @@ -1506,7 +1508,7 @@ int cppc_set_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls)
>   	 * Since read_lock can be acquired by multiple CPUs simultaneously we
>   	 * achieve that goal here
>   	 */
> -	if (CPC_IN_PCC(desired_reg)) {
> +	if (CPC_IN_PCC(desired_reg) || CPC_IN_PCC(min_perf_reg) || CPC_IN_PCC(max_perf_reg)) {
>   		if (pcc_ss_id < 0) {
>   			pr_debug("Invalid pcc_ss_id\n");
>   			return -ENODEV;
> @@ -1529,13 +1531,11 @@ int cppc_set_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls)
>   		cpc_desc->write_cmd_status = 0;
>   	}
>   
> -	/*
> -	 * Skip writing MIN/MAX until Linux knows how to come up with
> -	 * useful values.
> -	 */
>   	cpc_write(cpu, desired_reg, perf_ctrls->desired_perf);
> +	cpc_write(cpu, min_perf_reg, perf_ctrls->min_perf);
> +	cpc_write(cpu, max_perf_reg, perf_ctrls->max_perf);
>   
> -	if (CPC_IN_PCC(desired_reg))
> +	if (CPC_IN_PCC(desired_reg) || CPC_IN_PCC(min_perf_reg) || CPC_IN_PCC(max_perf_reg))
>   		up_read(&pcc_ss_data->pcc_lock);	/* END Phase-I */
>   	/*
>   	 * This is Phase-II where we transfer the ownership of PCC to Platform
> @@ -1583,7 +1583,7 @@ int cppc_set_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls)
>   	 * case during a CMD_READ and if there are pending writes it delivers
>   	 * the write command before servicing the read command
>   	 */
> -	if (CPC_IN_PCC(desired_reg)) {
> +	if (CPC_IN_PCC(desired_reg) || CPC_IN_PCC(min_perf_reg) || CPC_IN_PCC(max_perf_reg)) {
>   		if (down_write_trylock(&pcc_ss_data->pcc_lock)) {/* BEGIN Phase-II */
>   			/* Update only if there are pending write commands */
>   			if (pcc_ss_data->pending_pcc_write_cmd)