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;
Date:   Sun, 20 Feb 2022 22:35:52 +0800
From:   Huang Rui <ray.huang@amd.com>
To:     Shuah Khan <skhan@linuxfoundation.org>
Cc:     "Rafael J . Wysocki" <rafael.j.wysocki@intel.com>,
        "linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>,
        "Sharma, Deepak" <Deepak.Sharma@amd.com>,
        "Deucher, Alexander" <Alexander.Deucher@amd.com>,
        "Limonciello, Mario" <Mario.Limonciello@amd.com>,
        Steven Noonan <steven@valvesoftware.com>,
        "Fontenot, Nathan" <Nathan.Fontenot@amd.com>,
        "Su, Jinzhou (Joe)" <Jinzhou.Su@amd.com>,
        "Du, Xiaojian" <Xiaojian.Du@amd.com>,
        "Yuan, Perry" <Perry.Yuan@amd.com>,
        "Meng, Li (Jassmine)" <Li.Meng@amd.com>,
        Borislav Petkov <bp@alien8.de>,
        Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@kernel.org>,
        Viresh Kumar <viresh.kumar@linaro.org>,
        Steven Rostedt <rostedt@goodmis.org>,
        Giovanni Gherdovich <ggherdovich@suse.cz>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH RESEND v6 7/9] cpupower: Enable boost state support for
 AMD P-State module
Message-ID: <YhJRyNGA+bAEXT4Y@amd.com>
References: <20220216073558.751071-1-ray.huang@amd.com>
 <20220216073558.751071-8-ray.huang@amd.com>
 <f415df0b-9320-090c-7ca9-8597c0211e87@linuxfoundation.org>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <f415df0b-9320-090c-7ca9-8597c0211e87@linuxfoundation.org>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?llJ9CISV9cuZ0edbaoKU8JX7LuNgheO85JyVYxuJB7n6/HAcgrbjxWIEVMz2?=
 =?us-ascii?Q?MC+3I7s4HXP9r/fO+oGcxjWP6Bvpm82GcFrWCn7gRgifmXhd+OalVMkrD6Gc?=
 =?us-ascii?Q?7esl8lOdkUDVOxU72akJ9+bbzvGEDpqffnY2fhZn6m50HdCx9STBM6rXf0bb?=
 =?us-ascii?Q?PA8Xw/26cG1KnHJ3DSWRBCenjVTQPtyET6YqzNNNmRY1uSaTrBiISqqvTw5A?=
 =?us-ascii?Q?bekLOqkUrZ7Z2aMWEa2cfe8mtErYw9p0ZnC3+9YTEznpRJt/wovbmQS008ip?=
 =?us-ascii?Q?aPcYN1+BfR8zlFW9lI/PAIK3QjnVs7pXTCGRTKFaVueHzya5Yr2sE+xzETtP?=
 =?us-ascii?Q?u98yCj/jnbX1sOMCU4EjBorFU1cphZYqeBZ+D3pz6SjwcLxPUIvzmG0LsNOw?=
 =?us-ascii?Q?sFYxSxAEkgYMXBpuLWmUlgpvQZgkTO4LDUDoJdFypo0RuuxMsOXhUowyHfhA?=
 =?us-ascii?Q?iPmXBKxeRw7IDbj87ARHHv6cF8AxdxhJP7KRJJ/q/xScTedwSOZ52wLYzlp3?=
 =?us-ascii?Q?60GslSbfKwN9CM+hYAZvIm8DTzMAB3jbK560gLJAzNbwcW64HW3tNoaU20sa?=
 =?us-ascii?Q?q6wPSJ3bWxmk9m2txTyECtkFt8g/iNjXteZw1p7KDLZgv8FLQVfCUtzPAbyI?=
 =?us-ascii?Q?PxtEzeyKH6VhyjsPBQJ995cFD7cSAaRzPQoIsRmK4BSjdu11XScYrkw/bnV7?=
 =?us-ascii?Q?Y8qJolXPIaoFINzF6hU3T5fv2fz50QPa6uF4Mi0N8Tv6C24Yh5nYYT3Sqiw2?=
 =?us-ascii?Q?MAqz1D3kDvekwk4ar5eq2yl2fPhDFYGKG4wQFxOpDZ5A0dwug8puqnWzxk9+?=
 =?us-ascii?Q?14IuXdr2NHyqJAQ8Efj3W6cgsa8WgEpachAdrhtJxDtreAMA3uw2Q/AkjJS2?=
 =?us-ascii?Q?tr53fA8WWTSr6DSXa/87sgydsftDAH5Dgh+WzlkJvKubsvSHgt6h31IZVmvt?=
 =?us-ascii?Q?hIDL62xfgDdQylIK26bGyjc3nuQETUzbckVFtooOvxMXu3vEZXUlxBN+9dD2?=
 =?us-ascii?Q?hbNbfP1WPrwYt00qhfZBs0izhW4HHh2+D+0v5HQ1am7BU6gaVZJvPhLWcn20?=
 =?us-ascii?Q?20BjXWMbGLFAwp9x88HTT2dZZalE9G5+rCSQPU3+rQfrC2mI2HXapFrKtuDA?=
 =?us-ascii?Q?s1JYf25L5sr2pkb4sfrlThVt0PXTUYBZNEe6Aj4jn2X1WYDSK3zPsjskR3cZ?=
 =?us-ascii?Q?A5GvC+ZpsfTgUlQ70p5wauRpQ6DghjsWksQUYJDj2OP2bAB9Ds6yqLALWWd1?=
 =?us-ascii?Q?bVBZRZx9gYy66RNCfeYB1zYonWWNDPy/1XrEt5uWJTBklSkNyfF4cyqf3Cdj?=
 =?us-ascii?Q?c0JStS/GlPg8CCrfH9xRQ0ARF3jH6uqi1yHmkjJZAcJa01Ed76NxlSRj9bJ5?=
 =?us-ascii?Q?FGQ5O6JQ59Fp2taP+wUJBjU9fvNRo90BCQwXAMQ2vIdkva4+P/gVCxsA/OMz?=
 =?us-ascii?Q?VJJlyv0yBCrmcAzgdUQWvszK35drwJDtaBAUPd16D74PaQgg88DG8dXuHfJs?=
 =?us-ascii?Q?dwfVDID1/dWSy44mN7LceMg3lkZPYNFF7wKEk2KrdYw/R5OCoqaBLE6obmU3?=
 =?us-ascii?Q?cMsHZBNqScsZ/a8B5ppK5x+cyLA/BNeZyzDmwIsiNSi9qPf28HY9v2eiwABg?=
 =?us-ascii?Q?d0BwLYzRPNkrt4VTnktcDiA=3D?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 452af62b-c99f-4176-88a7-08d9f47e5a0e
X-MS-Exchange-CrossTenant-AuthSource: DM5PR12MB2504.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Feb 2022 14:36:16.5938
 (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: Et1EZk7RC08gEaOGzjDk1LxG7qhoxUn4EMVuLQhC9MIHujuvKaMv+ZGXIL+S1vLUQF1iAOcrZ7r0Xd4R2R9LZQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR12MB3147
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,T_SCC_BODY_TEXT_LINE
        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 Sat, Feb 19, 2022 at 08:01:59AM +0800, Shuah Khan wrote:
> On 2/16/22 12:35 AM, Huang Rui wrote:
> > The legacy ACPI hardware P-States function has 3 P-States on ACPI table,
> > the CPU frequency only can be switched between the 3 P-States. While the
> > processor supports the boost state, it will have another boost state
> > that the frequency can be higher than P0 state, and the state can be
> > decoded by the function of decode_pstates() and read by
> > amd_pci_get_num_boost_states().
> > 
> > However, the new AMD P-State function is different than legacy ACPI
> > hardware P-State on AMD processors. That has a finer grain frequency
> > range between the highest and lowest frequency. And boost frequency is
> > actually the frequency which is mapped on highest performance ratio. The
> > similiar previous P0 frequency is mapped on nominal performance ratio.
> 
> Nit - similar

Corrected.

> 
> > If the highest performance on the processor is higher than nominal
> > performance, then we think the current processor supports the boost
> > state. And it uses amd_pstate_boost_init() to initialize boost for AMD
> > P-State function.
> > 
> > Signed-off-by: Huang Rui <ray.huang@amd.com>
> > ---
> >   tools/power/cpupower/utils/helpers/amd.c     | 18 ++++++++++++++++++
> >   tools/power/cpupower/utils/helpers/helpers.h |  5 +++++
> >   tools/power/cpupower/utils/helpers/misc.c    |  2 ++
> >   3 files changed, 25 insertions(+)
> > 
> > diff --git a/tools/power/cpupower/utils/helpers/amd.c b/tools/power/cpupower/utils/helpers/amd.c
> > index 4d45d1b44164..f5ba528dc7db 100644
> > --- a/tools/power/cpupower/utils/helpers/amd.c
> > +++ b/tools/power/cpupower/utils/helpers/amd.c
> > @@ -175,5 +175,23 @@ static unsigned long amd_pstate_get_data(unsigned int cpu,
> >   						  MAX_AMD_PSTATE_VALUE_READ_FILES);
> >   }
> >   
> > +void amd_pstate_boost_init(unsigned int cpu, int *support, int *active)
> > +{
> > +	unsigned long highest_perf, nominal_perf, cpuinfo_min,
> > +		      cpuinfo_max, amd_pstate_max;
> > +
> > +	highest_perf = amd_pstate_get_data(cpu, AMD_PSTATE_HIGHEST_PERF);
> > +	nominal_perf = acpi_cppc_get_data(cpu, NOMINAL_PERF);
> > +
> > +	*support = highest_perf > nominal_perf ? 1 : 0;
> > +	if (!(*support))
> > +		return;
> > +
> > +	cpufreq_get_hardware_limits(cpu, &cpuinfo_min, &cpuinfo_max);
> > +	amd_pstate_max = amd_pstate_get_data(cpu, AMD_PSTATE_MAX_FREQ);
> > +
> > +	*active = cpuinfo_max == amd_pstate_max ? 1 : 0;
> > +}
> > +
> >   /* AMD P-State Helper Functions ************************************/
> >   #endif /* defined(__i386__) || defined(__x86_64__) */
> > diff --git a/tools/power/cpupower/utils/helpers/helpers.h b/tools/power/cpupower/utils/helpers/helpers.h
> > index 557524078e94..f142fbfa4a77 100644
> > --- a/tools/power/cpupower/utils/helpers/helpers.h
> > +++ b/tools/power/cpupower/utils/helpers/helpers.h
> > @@ -140,6 +140,8 @@ extern int cpufreq_has_boost_support(unsigned int cpu, int *support,
> >   
> >   /* AMD P-State stuff **************************/
> >   extern bool cpupower_amd_pstate_enabled(void);
> > +extern void amd_pstate_boost_init(unsigned int cpu,
> > +				  int *support, int *active);
> >   
> >   /* AMD P-State stuff **************************/
> >   
> > @@ -177,6 +179,9 @@ static inline int cpufreq_has_boost_support(unsigned int cpu, int *support,
> >   
> >   static inline bool cpupower_amd_pstate_enabled(void)
> >   { return false; }
> > +static void amd_pstate_boost_init(unsigned int cpu,
> > +				  int *support, int *active)
> > +{ return; }
> 
> No need for a return here
> 

Will remove "return".

Thanks,
Ray