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 16/19] cpupower: enable boost state support for amd-pstate
 module
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>, Ingo Molnar <mingo@kernel.org>,
        linux-pm@vger.kernel.org
Cc:     Deepak Sharma <deepak.sharma@amd.com>,
        Alex Deucher <alexander.deucher@amd.com>,
        Mario Limonciello <mario.limonciello@amd.com>,
        Jinzhou Su <Jinzhou.Su@amd.com>,
        Xiaojian Du <Xiaojian.Du@amd.com>,
        linux-kernel@vger.kernel.org, x86@kernel.org
References: <20210908150001.3702552-1-ray.huang@amd.com>
 <20210908150001.3702552-17-ray.huang@amd.com>
From:   "Fontenot, Nathan" <Nathan.Fontenot@amd.com>
Message-ID: <5353e053-084d-9a29-bae5-c1e1d597d1e3@amd.com>
Date:   Wed, 8 Sep 2021 12:32:37 -0500
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101
 Thunderbird/78.13.0
In-Reply-To: <20210908150001.3702552-17-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?OWx3bU1BL0xoVjVJUzFlOEF1bmY5b3FaV1NkcTVzRUlra1habzA5dytvUnlU?=
 =?utf-8?B?aTNYWjNWYmFVM29Ma3VtZmI1QTRzeEdVTldaR2RnaUw4TnlKUk9nOTU3aTl6?=
 =?utf-8?B?blZqL1d0U3FadEFHRG94L1dMcDhiaEMzaXNOcENIREIvMllRWXh5L3dCVGdH?=
 =?utf-8?B?TjVQcEtoc2tVTlVHeCsrWFZWeUxPTHlBZ3IrZ2Vtb0VHQ0VaemIrdFNSc1Fw?=
 =?utf-8?B?Y3BPdHg4cEVhK0doZThucmFGNEJVSHF4aGVLai9HTnJ1NkFNbGRma1A1RDgr?=
 =?utf-8?B?aVVIZWtkdTFkVzFIRUVVT0YwZUNZTThrcENTL2E0NHVoekRnOWJRZ1hRU250?=
 =?utf-8?B?MlkxSFlmYlE5SGdqSnhYaDZ4WDhYd25hRUg3OXNSQzB0cG56S2NZTVZPVXRK?=
 =?utf-8?B?cEFSb0ptRXNJU0pYbzFyOTQwajU4T3c1bFRSZHBIaFVMY29QWVlkeUQ3Zm53?=
 =?utf-8?B?TS8raXh5ZlVhWlF5Q2R0bnZJUDd1d1V6cGZPNnN5VGduYUpvRlprRENTVEZK?=
 =?utf-8?B?VGNqSDJFUGVvVUZvdERURUVQOHh4WDNSQVF3ZXFvbWZ3dXdXc01WcEIyTHBI?=
 =?utf-8?B?Z1MzWHgwMTBCc3pnUDdjek5iUEhoZ0hac202Rmd0ekZTbUsvR1JnQkNWTEdl?=
 =?utf-8?B?dlBpdFpVQUZ5NkpyWDZSNHhqd1lyTTBzWjA3L1JMQWtFdVhKRjZ4Tzdoc09V?=
 =?utf-8?B?dzVaS1FIOUFoVWszQzRYSGZkam91elZBMlZtd0F6ZE5tWldmVXRTdUlRM0Yr?=
 =?utf-8?B?K2hUTW1rNVE1M20xYjNaeGlBblVCejZCWis1Y2o5VzVWbEdhWHpqUkpYTjBX?=
 =?utf-8?B?SktLL1cwQ2NnTlRodnd0Q1ExNUFwK1YvT2l4YmtMYTZSbWw3bjUzOUlWc2dy?=
 =?utf-8?B?bnhWT3JXRXlhOFJpcU96TzcwaFh3eE1Pbysxb3BUSzBJTFRGcHhvZ0tUMklL?=
 =?utf-8?B?TCtRaXFxS3V6ZVpMYWdoZS82TmRqeEVuNzVlQWpYVE84SEI1eVJzdVN0eloy?=
 =?utf-8?B?K2FJRnUrdk91QmZROThGVGtadDBDbWpHR3hUSXZUSVZuRmxJd2tEVm5GWWkx?=
 =?utf-8?B?NDhyaGFGWnVxTE5hVWtFaVR5NnBTbWk1Sy9yR1BRQkI2dC9qQjJyMUdUK1RD?=
 =?utf-8?B?MlFEMUthOVMwVU1pQWNZNlVyazNXQUhPUVpDdVl1VlJaVk5LNDBNZXUxRHlX?=
 =?utf-8?B?WmtzdHE2ZEsraEV5S3Y1N0xMNUh6bm1hZHloamhOR3hzcVBTWTVDVlpFaFcy?=
 =?utf-8?B?RTF4V2NaYU1MZlZ6WFh2REI2TDNyUGlLc0VBbkxBZ0RwUkpnRmpkeVAyUmdP?=
 =?utf-8?B?NUJLMCszTWR1ckhUSkg1NFkwa24yUW5XMm9mY1ZBNTR5b0E5WGppYXpjcWpD?=
 =?utf-8?B?Tmt5cjdzbWp5YVhVbE00TTk5TjNFSjU5dWdKUzZqQStrWW8rVC8zOEREYkly?=
 =?utf-8?B?SGhYdGw4YXBxOEtVNGY5ZzJlTitRdW4wZlUvM3BrSTExK1RDL3RlVFM3Q2VQ?=
 =?utf-8?B?cXpBcUJ4Y29BejgvTnZJYzFIVG1vazlZczdLRGV2SEJYU1N0VGF1a1JncmFH?=
 =?utf-8?B?QnorTVVueDRidWR3QXlJaWdKUk91NStNYUNzeGQ2WUtneHQ2eW5PVmQvWXhG?=
 =?utf-8?B?QnFsSXl5Q3F2TXBiMDV2emtOam9KK3dzZmt5cU0rZFJyL0l1b1ZZc25hQVZz?=
 =?utf-8?B?VUkrTWdCQ0JjcU1WT0FSbmdaWnlNV1E2ZS9NendQRlVYVWhrS1BoUkF5Z21G?=
 =?utf-8?B?U2dCa0l1S3BrMVQweVlKbkJUanM2R3FuSGVsQ0RQUE4zdTVlMGNJZldOc3or?=
 =?utf-8?B?Vkw5cTd1b1dSdFEzaWdGK1VVdTVJVXk4VlhFTzZ2N0w3UFkzM3d1TmMzaHg2?=
 =?utf-8?Q?LBfAKwpdNQ5Oj?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 0a124219-5d51-4254-669e-08d972eea78b
X-MS-Exchange-CrossTenant-AuthSource: SN6PR12MB4720.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Sep 2021 17:32:39.0518
 (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: apPUYmuHcL2hb12VGV2vd9gKVfgpKjkIq6onJ/wWWb1HoXvskBAtuWrJEMBVNuugtmlh7sAqxChUUT/Zf8fUkQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN1PR12MB2480
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 9/8/2021 9:59 AM, Huang Rui wrote:
> The AMD P-state boost API is different from ACPI hardware P-states, so
> implement the support for amd-pstate kernel module.
> 
> Signed-off-by: Huang Rui <ray.huang@amd.com>
> ---
>  tools/power/cpupower/lib/cpufreq.c        | 20 ++++++++++++++++++++
>  tools/power/cpupower/lib/cpufreq.h        |  3 +++
>  tools/power/cpupower/utils/helpers/misc.c |  7 +++++++
>  3 files changed, 30 insertions(+)
> 
> diff --git a/tools/power/cpupower/lib/cpufreq.c b/tools/power/cpupower/lib/cpufreq.c
> index 3f92ddadaad2..37da87bdcfb1 100644
> --- a/tools/power/cpupower/lib/cpufreq.c
> +++ b/tools/power/cpupower/lib/cpufreq.c
> @@ -790,3 +790,23 @@ unsigned long cpufreq_get_transitions(unsigned int cpu)
>  {
>  	return sysfs_cpufreq_get_one_value(cpu, STATS_NUM_TRANSITIONS);
>  }
> +
> +int amd_pstate_boost_support(unsigned int cpu)
> +{
> +	unsigned int highest_perf, nominal_perf;
> +
> +	highest_perf = sysfs_cpufreq_get_one_value(cpu, AMD_PSTATE_HIGHEST_PERF);
> +	nominal_perf = sysfs_cpufreq_get_one_value(cpu, AMD_PSTATE_NOMINAL_PERF);
> +
> +	return highest_perf > nominal_perf ? 1 : 0;
> +}
> +
> +int amd_pstate_boost_enabled(unsigned int cpu)
> +{
> +	unsigned int cpuinfo_max, amd_pstate_max;
> +
> +	cpuinfo_max = sysfs_cpufreq_get_one_value(cpu, CPUINFO_MAX_FREQ);
> +	amd_pstate_max = sysfs_cpufreq_get_one_value(cpu, AMD_PSTATE_MAX_FREQ);
> +
> +	return cpuinfo_max == amd_pstate_max ? 1 : 0;
> +}
> diff --git a/tools/power/cpupower/lib/cpufreq.h b/tools/power/cpupower/lib/cpufreq.h
> index 95f4fd9e2656..d54d02a7a4f4 100644
> --- a/tools/power/cpupower/lib/cpufreq.h
> +++ b/tools/power/cpupower/lib/cpufreq.h
> @@ -203,6 +203,9 @@ int cpufreq_modify_policy_governor(unsigned int cpu, char *governor);
>  int cpufreq_set_frequency(unsigned int cpu,
>  				unsigned long target_frequency);
>  
> +int amd_pstate_boost_support(unsigned int cpu);
> +int amd_pstate_boost_enabled(unsigned int cpu);
> +
>  #ifdef __cplusplus
>  }
>  #endif
> diff --git a/tools/power/cpupower/utils/helpers/misc.c b/tools/power/cpupower/utils/helpers/misc.c
> index 07d80775fb68..aba979320760 100644
> --- a/tools/power/cpupower/utils/helpers/misc.c
> +++ b/tools/power/cpupower/utils/helpers/misc.c
> @@ -10,6 +10,7 @@
>  #if defined(__i386__) || defined(__x86_64__)
>  
>  #include "cpupower_intern.h"
> +#include "cpufreq.h"
>  
>  #define MSR_AMD_HWCR	0xc0010015
>  
> @@ -39,6 +40,12 @@ int cpufreq_has_boost_support(unsigned int cpu, int *support, int *active,
>  			if (ret)
>  				return ret;
>  		}
> +	} if ((cpupower_cpu_info.caps & CPUPOWER_CAP_AMD_PSTATE) &&

Shouldn't this be

	} else if ((cpupower_cpu_info.caps & CPUPOWER_CAP_AMD_PSTATE) &&

-Nathan

> +	      amd_pstate_boost_support(cpu)) {
> +		*support = 1;
> +
> +		if (amd_pstate_boost_enabled(cpu))
> +			*active = 1;
>  	} else if (cpupower_cpu_info.caps & CPUPOWER_CAP_INTEL_IDA)
>  		*support = *active = 1;
>  	return 0;
> -- 
> 2.25.1
>