Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH v3 11/21] cpufreq: amd: add amd-pstate frequencies
 attributes
To:     Huang Rui <ray.huang@amd.com>,
        "Rafael J . Wysocki" <rafael.j.wysocki@intel.com>,
        Viresh Kumar <viresh.kumar@linaro.org>,
        Shuah Khan <skhan@linuxfoundation.org>,
        Borislav Petkov <bp@suse.de>,
        Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@kernel.org>,
        Giovanni Gherdovich <ggherdovich@suse.cz>,
        linux-pm@vger.kernel.org
Cc:     Deepak Sharma <deepak.sharma@amd.com>,
        Alex Deucher <alexander.deucher@amd.com>,
        Mario Limonciello <mario.limonciello@amd.com>,
        Steven Noonan <steven@valvesoftware.com>,
        Nathan Fontenot <nathan.fontenot@amd.com>,
        Jinzhou Su <Jinzhou.Su@amd.com>,
        Xiaojian Du <Xiaojian.Du@amd.com>,
        linux-kernel@vger.kernel.org, x86@kernel.org
References: <20211029130241.1984459-1-ray.huang@amd.com>
 <20211029130241.1984459-12-ray.huang@amd.com>
From:   Nathan Fontenot <nafonten@amd.com>
Message-ID: <b4a2b9df-1074-dc86-be76-7f47af202b9e@amd.com>
Date:   Fri, 5 Nov 2021 13:59:55 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.13.0
In-Reply-To: <20211029130241.1984459-12-ray.huang@amd.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?N2c1RXJTbUtkWUk1bHV1VDNqcDRERE1hTGhUTlUvUEw3K2Jtc0tseUZUTjdS?=
 =?utf-8?B?aUhGMUVzaWNhZE10Y3R4L21jREpqQ1JQU2NCT0ZQTzNOdFpPMW1mUWoycjQ3?=
 =?utf-8?B?NnQyRFFuMkpBTjNnRGFkbUtpcUc2RmM2alNVZWpsU2E4L0FJNUp4SWx0b2FV?=
 =?utf-8?B?cVB5d0tnYXlSb1AzeHdwV3dXdGhSK3loUWYxNDRvZy93dVdNN3BRdSs1cUxN?=
 =?utf-8?B?QVBCaytTMG1LeFd3elUyWjhwSElaNCtZZVZMVnlpU2JRMllNMDYva2hGa04x?=
 =?utf-8?B?am1JeGdBNCt3N2FhQnhzRjgrLytpT2xhYWFlaHlteU93bGhMMlJFMTFJd1FY?=
 =?utf-8?B?eEU5UXJnZVBHNFZ0T1lQRkFzVTZMKzM2K3pJOXlzYzJiZGlFNHJUcW1DK21v?=
 =?utf-8?B?NmxHMXlSbXQ4WWdaVVJDQnAxOFZ1eUJzMEMwbUVHQ0dOUU1wYlNwTXEwd1VT?=
 =?utf-8?B?MnArL0h2SHBWK2hDUUptY05GWTJZZFNSeVdCMjd3Skc5T005cWNwY1Z0S1Ny?=
 =?utf-8?B?YnU2S0RLc1VQZmlyU0puSUlhb0ovTnVVNTNlZE4rZzFYd2FUV1h2aEJnYkhZ?=
 =?utf-8?B?aTd3M1FoMS9KQVQ2a25FdFpjK2sxV2VPNXo4S2VsMzFmd2RWc1U2dmZGOWpa?=
 =?utf-8?B?WFVITW95TnMyT1BKeEQwTVJmaWFCbUg5cFBTS2RCZjh6cVdCMWw0NjNJR1Fj?=
 =?utf-8?B?dk4xb2xhUjI0UGREZ0pEdlBQYk5LdTdPcnd1MVJWRWVISTRQVUE3aWJOdmNW?=
 =?utf-8?B?ZFNsSVlxbU9ja29ZYzlqZnV4OFg1QkNXKzNxT2FGMkM2QUlHWEsxblVoQWRU?=
 =?utf-8?B?dUFXZDJpUjN5TWVUMFpkZC94NXduRHNkN1JtbmVHNncyN3pRa2hBSzJwOG9G?=
 =?utf-8?B?RGRVMGlwQmZUdDZnQ0VXR3dhdkE4dDBkUUphWEtLSUxJMGxJZC9hViszSE5E?=
 =?utf-8?B?RlRjQ293c3BTR0dhNlh2WjZZM3pFNXJGS1RDMFpoL2ZCcFI1b2ZZN29Sekhq?=
 =?utf-8?B?NnhrZ1dGTmxraVBEZ1hpTUxCdHpCeW95ZlpiUUNsL2JNeFpvemFlSnZHRE5E?=
 =?utf-8?B?MTVNWmlJcVRRYjB0M0Ezakt0c1JRNmFkVk9ZWDZQVzdkRDVzcTlxZjVsemtr?=
 =?utf-8?B?bEJBZG9Hci9BS1dBSFJIVG5tTUpsWXpBTVpCYmV0OHR5dUdCRC9MVjhCRjhi?=
 =?utf-8?B?b2ZCM0R5U0tIcTIxeGpodlJqNkJnWTNlVUtsYWMvVTAxTEp4Wm12SnVZMktu?=
 =?utf-8?B?WHdkTGhMRWJqMjBaa2NxSEpFbjUrMkc3M1RxM3B5aWJ3TTlURW51bnV6OUNn?=
 =?utf-8?B?RGxxczd1QU1DQmUzdnZYK2tGY2czZFZIQTRKajRobHl0Vnc0RlptK1lqTC9X?=
 =?utf-8?B?ZEt4ejdDMkZnNitxc2JLLytIY0VRN0ZEVlFRM0NtcU1aSENmVCtZTlVwelMx?=
 =?utf-8?B?dDhlYlhsTHhXOTY2bW9lb3VXd2ZKM2E0UUFSbmJ0MkJ4Umg0MHlxKzIxTVE2?=
 =?utf-8?B?MWZ2eHRZSjM4MXFQM3dDcFc5bTRzVkxMbWxBWU9CRGMveHlsbmk4andOcjRZ?=
 =?utf-8?B?bndoYjdMU2VIOXZkYllORDRNb3dSN0tqQmdYcTlVVGV3Yi85SHVvK0JGZVlG?=
 =?utf-8?B?RmpQZFZkOHNkQXZDV0hPS0F4RkFOaDh1VXNWQnkwMEMza3JqS3V1V1ExQklZ?=
 =?utf-8?B?dTNkK0ZPYnUzeWl3Nks3dlBzcWhxRzl1OU51UDBtRkl2SElBWUxVQjhXT2dC?=
 =?utf-8?B?bHlreHNJbEZ2ZjgvUWZXM1FYR0RQZFRqZmRrQ1p4QjBqZ1FMSmdhSXUrMVVR?=
 =?utf-8?B?VzR5Y2NtSlZzN0ZNemxmTDJHUTlXWWpQWlFIQ3V5VUxnVzdEOVJxQm83NGhJ?=
 =?utf-8?B?ZW80MlBNR0MxVUtLd0hzR2hmL3lwSzZuQkpBbEQyS1I1MTNpdy80aytmSFE0?=
 =?utf-8?B?b1A1YW9pdEhHdEg3UGpTdmdMSVZreFFaUVZtaGVFNHd2b0xRTlF1TGpiOURz?=
 =?utf-8?B?TmwzeTFycGJhOE8xbGdwNm5LNjVJQUJnUHoyM3h0Yjlyb3Awcm5RYkw1OFZ6?=
 =?utf-8?B?ZXNuMy85QldjT0RORlRZY2kwTis4Z0gwMUpEU3pmZWtiN2NNVWprY0NBSTBM?=
 =?utf-8?B?amZnY3d3ZW44N1drS0JScjFhdFJCRnR5MExRcUgvWk1WRmh6c3dacktUdU14?=
 =?utf-8?B?L0h0c2pLY0hmZ3FLdjFGSmVPUkJxcmNOcDdhaHQ2NnVkMlF2N1g1dzIvQ1c2?=
 =?utf-8?Q?dgw8+ppee35rCv94UB5TZ74AFWavSDFbihYUloFf50=3D?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: de0142fa-fd83-46cc-cedc-08d9a08e76b4
X-MS-Exchange-CrossTenant-AuthSource: SN6PR12MB4720.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 05 Nov 2021 18:59:58.8116
 (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: 6k+87BY+mvI5trvA6WZZFy93tkdh5PyaNqFcAIGVRjbj4+dRy2yEhHXedi02kzhA2BU65Fks9Pm4C3JCm5JDWw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN6PR12MB2798
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 10/29/21 8:02 AM, Huang Rui wrote:
> Introduce sysfs attributes to get the different level processor
> frequencies.
> 
> Signed-off-by: Huang Rui <ray.huang@amd.com>
> ---
>  drivers/cpufreq/amd-pstate.c | 63 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 63 insertions(+)
> 
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 9af27ac1f818..8cf1e80f44e0 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -485,6 +485,68 @@ static int amd_pstate_cpu_exit(struct cpufreq_policy *policy)
>  	return 0;
>  }
>  
> +/* Sysfs attributes */
> +
> +/* This frequency is to indicate the maximum hardware frequency.
> + * If boost is not active but supported, the frequency will be larger than the
> + * one in cpuinfo.
> + */
> +static ssize_t show_amd_pstate_max_freq(struct cpufreq_policy *policy,
> +					char *buf)
> +{
> +	int max_freq;
> +	struct amd_cpudata *cpudata;
> +
> +	cpudata = policy->driver_data;
> +
> +	max_freq = amd_get_max_freq(cpudata);
> +	if (max_freq < 0)
> +		return max_freq;
> +
> +	return sprintf(&buf[0], "%u\n", max_freq);
> +}
> +
> +static ssize_t show_amd_pstate_nominal_freq(struct cpufreq_policy *policy,
> +					    char *buf)
> +{
> +	int nominal_freq;
> +	struct amd_cpudata *cpudata;
> +
> +	cpudata = policy->driver_data;
> +
> +	nominal_freq = amd_get_nominal_freq(cpudata);
> +	if (nominal_freq < 0)
> +		return nominal_freq;
> +
> +	return sprintf(&buf[0], "%u\n", nominal_freq);
> +}

The nominal_freq valus is already reported in sysfs by drivers/acpi since this
value is part of the ACPI spec. Is there a reason to have multiple sysfs entries 
for the same value?

-Nathan
 
> +
> +static ssize_t show_amd_pstate_lowest_nonlinear_freq(struct cpufreq_policy *policy,
> +						     char *buf)
> +{
> +	int freq;
> +	struct amd_cpudata *cpudata;
> +
> +	cpudata = policy->driver_data;
> +
> +	freq = amd_get_lowest_nonlinear_freq(cpudata);
> +	if (freq < 0)
> +		return freq;
> +
> +	return sprintf(&buf[0], "%u\n", freq);
> +}
> +
> +cpufreq_freq_attr_ro(amd_pstate_max_freq);
> +cpufreq_freq_attr_ro(amd_pstate_nominal_freq);
> +cpufreq_freq_attr_ro(amd_pstate_lowest_nonlinear_freq);
> +
> +static struct freq_attr *amd_pstate_attr[] = {
> +	&amd_pstate_max_freq,
> +	&amd_pstate_nominal_freq,
> +	&amd_pstate_lowest_nonlinear_freq,
> +	NULL,
> +};
> +
>  static struct cpufreq_driver amd_pstate_driver = {
>  	.flags		= CPUFREQ_CONST_LOOPS | CPUFREQ_NEED_UPDATE_LIMITS,
>  	.verify		= amd_pstate_verify,
> @@ -493,6 +555,7 @@ static struct cpufreq_driver amd_pstate_driver = {
>  	.exit		= amd_pstate_cpu_exit,
>  	.set_boost	= amd_pstate_set_boost,
>  	.name		= "amd-pstate",
> +	.attr           = amd_pstate_attr,
>  };
>  
>  static int __init amd_pstate_init(void)
>