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: <a2ac526b-477d-277c-07ce-2b05c35b643d@amd.com>
Date:   Thu, 29 Sep 2022 09:08:10 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.11.0
Subject: Re: [PATCH 4/7] cpufreq: amd_pstate: add AMD pstate EPP support for
 shared memory type processor
Content-Language: en-US
To:     "Yuan, Perry" <Perry.Yuan@amd.com>,
        "rafael.j.wysocki@intel.com" <rafael.j.wysocki@intel.com>,
        "Huang, Ray" <Ray.Huang@amd.com>,
        "viresh.kumar@linaro.org" <viresh.kumar@linaro.org>
Cc:     "Sharma, Deepak" <Deepak.Sharma@amd.com>,
        "Limonciello, Mario" <Mario.Limonciello@amd.com>,
        "Deucher, Alexander" <Alexander.Deucher@amd.com>,
        "Su, Jinzhou (Joe)" <Jinzhou.Su@amd.com>,
        "Huang, Shimmer" <Shimmer.Huang@amd.com>,
        "Du, Xiaojian" <Xiaojian.Du@amd.com>,
        "Meng, Li (Jassmine)" <Li.Meng@amd.com>,
        "linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
References: <20220909164534.71864-1-Perry.Yuan@amd.com>
 <20220909164534.71864-5-Perry.Yuan@amd.com>
 <a21bdd7d-3ade-4a54-1a7f-42831ce8e83a@amd.com>
 <DM4PR12MB5278639EB4E0FD416653743A9C539@DM4PR12MB5278.namprd12.prod.outlook.com>
From:   Nathan Fontenot <nathan.fontenot@amd.com>
In-Reply-To: <DM4PR12MB5278639EB4E0FD416653743A9C539@DM4PR12MB5278.namprd12.prod.outlook.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?STNxRnZjR2RBMnZmdTh1dnY3Ui9iVU1CUURPZERkWEhKM21UZXhkT0dlUzJ3?=
 =?utf-8?B?cHY2MDhmbU91Lyt4NWMzUDV6MjUxeFZvb0g5QlZRWWt6OE9FZ1A4NUNxRzFN?=
 =?utf-8?B?MnArVjVXUzAyVFdRZEwvN3gvSEY1clR0RG52MVUvYmVZK01ybEVRTWRDL3FW?=
 =?utf-8?B?eG83S0R5d0R2aTJ0ZXpDaXg3VE85bnkrb21ESDlDUEJkeWdQcWk4R1pWSTZa?=
 =?utf-8?B?SUdVcnNBak1sSmxUVFpPYmdFcTFhVXJUWlROU3FiK096dlV6MFNycy9qTmlm?=
 =?utf-8?B?U0sxUWZvR3ZEOXYyVm9PNDN5ZjBpc0lwUENBc1Zxc05hSmgrOXBSdmNPYmhs?=
 =?utf-8?B?YzRSYnBqR0h1T1VHZG5XYVhXd2dzVHFyRkxKcDhQak5wUnZtdXdKVmlRTHBY?=
 =?utf-8?B?bXNmWU1qMXpqS1N6NXZVWTlGUUNHdUFWV0VOb05xd1BwYVJ4OXVQR1VGY1dx?=
 =?utf-8?B?Y3dxbktHRlhxMEpoR1IvR01VclFTS3RmU0JwNHVJUmNwV2ZiZE0yemRraFdw?=
 =?utf-8?B?SGNIM2V3QkFiT3A5WjZHZHpQWHBuN09tWUxHaHpwN1NFalJuRXJMYlRRVkFR?=
 =?utf-8?B?V2YwbVFjakZxVVovY3hTcGsxUmloZi9YUmZ0emVSREVuMkpQRE84Y0EvaWs1?=
 =?utf-8?B?TXphcVk0Ync2YnBqQmMzclBmcEhwVjJyUlZoemgwb2p1MXlmcGI5RE95V2lH?=
 =?utf-8?B?N2hsVndhZkVORkRKbFdMZy8xVHhpNEVERUdmSTdLSDA2OVJZMXBJdkxQYWls?=
 =?utf-8?B?Ry9KZWJCNGNhcEpNVEh4aHdoZW1pbjZoSHhZL1kxNnpZaXJpTEJFMVEyVW00?=
 =?utf-8?B?RFN4TmhCS2xlQVJ1Z2J3WFNuQzFyRDg2ckxCYitHQzF1dWJMekFaTTBpNCtL?=
 =?utf-8?B?Tk9tUG5kaFR1UGF6ZXc1L2tRajB2U1pVemZYZ1BEQVhMTXdQT2FGb2ptbkly?=
 =?utf-8?B?UGM2WXpCbFE4aDZyZk02aE9UY3JPczM1ejhsd3BkSHZDMVB3R1dsTVpiVTBJ?=
 =?utf-8?B?eDhFNVNGaVVGazJsMnNDbjJobGFmTFBnbnNwNlNDcm41RCt1eUl3dVREVnZJ?=
 =?utf-8?B?dmhIenQ2a2pTNENDMjdKeCthdXBtWG16ZlM2RFE4NGRBb1gwdE9WTXFYUXox?=
 =?utf-8?B?TXVURWNxWThGK3pBcVh2UG1Fd2dJaVczZEZwR1RaK0tTTUJsUS96eFBjV25s?=
 =?utf-8?B?KzQra2xSdkFvVGtsMWMwOCt3RW5qVDd0Y1VsREJYWU5tRjZKWVdhRDh1Q2hO?=
 =?utf-8?B?Tjh1RVR5ak9IQWZieGIrQnRHemQzNmZ1anJRTisxdCtxR3pBTW12L0M0WFJj?=
 =?utf-8?B?dzZjVDFydmJYdnErZFA3OGxwVGt1UGx0NDQrWjRsT1pPNEJsc1pvc2VqY0FX?=
 =?utf-8?B?UEh0U2xSQTkramI1SWQwUXgyWFdqaEI1RnF6b2RKTktBUmxsMThuMC9uVWNL?=
 =?utf-8?B?ditUSGZ1T250Vmw2ZUVBSTBrcG5xbEFHb0NPWVlzREdldU5nUU1zRkxPWWk3?=
 =?utf-8?B?T09PbzlxSjE2eSthdEZrb2l3TEJLMW4ralRMSmhLdlgwajVuY29JR2d0M25q?=
 =?utf-8?B?WU43WUY1WUpsajg2czVQRWVueEFtNVZSaHZoS3JvTnF0Z3hiUFZUUEE3T0Fl?=
 =?utf-8?B?dFlwMjNkVnNFbzFsaHhjRms2NEhab296cnRldy9mMkt5aUVUTXBPOUxOM0Y4?=
 =?utf-8?B?all2Tlk1Znh4VU51UjdrUG1Gb2RaR0hqN2E2ZE4zOHI1MThGdGJ1eStWRFlD?=
 =?utf-8?B?TzZ6ZjdBSElYVitDZmdDRzlTdjBhVUpHUjdVNjBZR2hzQlE5ZGhabzFlOVZI?=
 =?utf-8?B?a2FOQWk5SllpOUhSQmF3RytDMHFWUE1DMnNKQy9UL1pZanordS9CWC9uRVpo?=
 =?utf-8?B?bkVDTUpSM0QybnAwc2YxdWVoWlRHeW4zSmRlTzFQM2dFVEVuM1BOR2QyN0Q3?=
 =?utf-8?B?b3NLUmI0b1dtbmtYUFJXRDd5MWQxUVRmT2h2N0Z5TSttdGE3ZE92R1dIbWJl?=
 =?utf-8?B?VEVKOVltTmt0ejFzcGdFRGJ5TzNPS1h4UXJlTjYyNzRwNVl0M1pZODNNQjBI?=
 =?utf-8?B?WUhldXl5UDhXRERwL25OZlJTYzd5UWUvZThLTmozSHdjTFBQOUpuZmh5ZUFG?=
 =?utf-8?Q?jjHwAnxNLBl8NkkPrSTIkQO4H?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 4fca4ccc-feb1-4a01-0c02-08daa2240bb6
X-MS-Exchange-CrossTenant-AuthSource: MN0PR12MB6222.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 29 Sep 2022 14:08:12.7067
 (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: Bt27C6rkXQfkn04ydNMc9cYVIYJkSSOn7m2/iYVEFvlkS0lTiMPf6ilbSM3r5sYUohVi1zKi69F3a/spxjQepQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY5PR12MB6478
X-Spam-Status: No, score=-6.2 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 9/25/22 07:23, Yuan, Perry wrote:
> [AMD Official Use Only - General]
> 
> Hi Nathan.
> 
> 
>> -----Original Message-----
>> From: Fontenot, Nathan <Nathan.Fontenot@amd.com>
>> Sent: Friday, September 16, 2022 12:24 AM
>> To: Yuan, Perry <Perry.Yuan@amd.com>; rafael.j.wysocki@intel.com; Huang,
>> Ray <Ray.Huang@amd.com>; viresh.kumar@linaro.org
>> Cc: Sharma, Deepak <Deepak.Sharma@amd.com>; Limonciello, Mario
>> <Mario.Limonciello@amd.com>; Deucher, Alexander
>> <Alexander.Deucher@amd.com>; Su, Jinzhou (Joe) <Jinzhou.Su@amd.com>;
>> Huang, Shimmer <Shimmer.Huang@amd.com>; Du, Xiaojian
>> <Xiaojian.Du@amd.com>; Meng, Li (Jassmine) <Li.Meng@amd.com>; linux-
>> pm@vger.kernel.org; linux-kernel@vger.kernel.org
>> Subject: Re: [PATCH 4/7] cpufreq: amd_pstate: add AMD pstate EPP support
>> for shared memory type processor
>>
>> On 9/9/22 11:45, Perry Yuan wrote:
>>> Add Energy Performance Preference support for AMD SOCs which only
>>> support the shared memory interface that implemented on Zen2 and Zen3
>>> processors, because this type CPU has no MSR supported, it will use
>>> ACPI PCC channel to enable EPP and reset desired perf to be zero.
>>>
>>> Signed-off-by: Perry Yuan <Perry.Yuan@amd.com>
>>> ---
>>>  drivers/cpufreq/amd-pstate.c | 12 ++++++++++++
>>>  1 file changed, 12 insertions(+)
>>>
>>> diff --git a/drivers/cpufreq/amd-pstate.c
>>> b/drivers/cpufreq/amd-pstate.c index 451295284a26..fff298744a8e 100644
>>> --- a/drivers/cpufreq/amd-pstate.c
>>> +++ b/drivers/cpufreq/amd-pstate.c
>>> @@ -133,12 +133,24 @@ static inline int pstate_enable(bool enable)
>>>
>>>  static int cppc_enable(bool enable)
>>>  {
>>> +	struct cppc_perf_ctrls perf_ctrls;
>>>  	int cpu, ret = 0;
>>>
>>>  	for_each_present_cpu(cpu) {
>>>  		ret = cppc_set_enable(cpu, enable);
>>>  		if (ret)
>>>  			return ret;
>>> +
>>> +	/* Enable active mode for EPP */
>>> +	ret = cppc_set_auto_epp(cpu, enable);
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	/* Set zero to desired perf to enable EPP control*/
>>> +	perf_ctrls.desired_perf = 0;
>>> +	ret = cppc_set_perf(cpu, &perf_ctrls);
>>> +	if (ret)
>>> +		return ret;
>>
>> Shouldn't this entire block be indented one additional tab over since its part
>> of the for_each_present_cpu() loop?
>>
>> -Nathan
> 
> Yes, I can see the indent in my local vscode editor, I do not know why the patch didn`t show that.
> Is it correct as below ?
> 
> 	for_each_present_cpu(cpu) {
> 		ret = cppc_set_enable(cpu, enable);
> 		if (ret)
> 			return ret;
> 
> 		if (epp_enabled) {
> 			/* Enable autonomous mode for EPP */
> 			ret = cppc_set_auto_epp(cpu, enable);
> 			if (ret)
> 				return ret;
> 
> 			/* Set zero to desired perf to allow EPP firmware control*/
> 			perf_ctrls.desired_perf = 0;
> 			ret = cppc_set_perf(cpu, &perf_ctrls);
> 			if (ret)
> 				return ret;
> 		}
> 	}
> 
> Perry. 

That looks much better.

-Nathan

> 
>>
>>>  	}
>>>
>>>  	return ret;