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;
Date:   Wed, 3 Nov 2021 15:04:08 +0800
From:   Huang Rui <ray.huang@amd.com>
To:     "Fontenot, Nathan" <Nathan.Fontenot@amd.com>
Cc:     "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" <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>,
        "Su, Jinzhou (Joe)" <Jinzhou.Su@amd.com>,
        "Du, Xiaojian" <Xiaojian.Du@amd.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "x86@kernel.org" <x86@kernel.org>
Subject: Re: [PATCH v3 18/21] cpupower: enable boost state support for
 amd-pstate module
Message-ID: <YYI0aPAw5t1WbC+E@hr-amd>
References: <20211029130241.1984459-1-ray.huang@amd.com>
 <20211029130241.1984459-19-ray.huang@amd.com>
 <5ec8afe8-d157-84e5-c877-649d4484ddc3@amd.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <5ec8afe8-d157-84e5-c877-649d4484ddc3@amd.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?120zdfDwU8cnmcfAupCk6f2azBlwfWwAtUjEqK0alXD+NO4fdM/HxMQASg0j?=
 =?us-ascii?Q?yctXS9SjFU/48VggNBwvdemCow1x3zuRomewaOGHFljN7QY3VuBKDLkeQqVF?=
 =?us-ascii?Q?ckyKJOM3bpMnx8550DhwYa5f7kZu3Xh+TupQ4fqv/JtId9VsrmRMDRWQajKX?=
 =?us-ascii?Q?gGv1rLrq1JPKtsv+ST9l1+jUEBfiQebAZIDAaTq0H1EwY0TdjFSQRH71RxGP?=
 =?us-ascii?Q?wqsY144OLpX4srlJhKHXkT0PSuOpdtgACODRKS4bcyM1tanPR7T4GZ3rbuDJ?=
 =?us-ascii?Q?G3+6EeZClJA1wyeSVcHPYsuWr+0lYV9TD9nsw46MLX9vwSauuX2G+wEQPGcF?=
 =?us-ascii?Q?0O6rToiUzRet4soxxRQAS7byvsUBTmXMIkAx+Oy08G4MxA4vNEyLyfahOpod?=
 =?us-ascii?Q?0fmc1c0Dxp1mFDKDpfjgD2fK4NYwDlbxKCnS5l09VxctKX1Qi7uX44bazs+J?=
 =?us-ascii?Q?k5To8yWP3COkc7PMZhPQ5x5HGAdkq9LQ7wK1ceZzLCKg/A9CXPou228X03zw?=
 =?us-ascii?Q?tsaGvCx3zj/t0/tSLrOnUFQlKVVfPr+3mOI426jk0egqwojqeGcDU6guIbzF?=
 =?us-ascii?Q?erdhvMjGLFfTpIv27W+3lR609H8kyblaybdz71PVgZeuNzcMwXWH+TwccPmS?=
 =?us-ascii?Q?WyTdgIS3Ug7BTQM6pfYWGqYTW4tDYzuTJuIOwPw8n12aFM/rvhPnfYVKFd8S?=
 =?us-ascii?Q?eRe3EDeAXyuo2X3P2qWwaQ+dGfwi6Oe4Sae7IJO+0q5xLjtXjswVG5zwDF0z?=
 =?us-ascii?Q?YGVeOJS5B/Ovi+F+iF66HPyfSzlNXAlFPN0/ZIBaEJdAf3a3XYEgf/SW0Sg9?=
 =?us-ascii?Q?kOSkyjcePppdaK6P/P7TYqBM3HNiDqwUtLvnh8/nqFNoh/0O6OnHxL9cvVK+?=
 =?us-ascii?Q?hIqKkkVVq1rrdaut3rrGN/00f+/YlXnBKGU7BiDzRa4QzIglI8cf1Giis/xM?=
 =?us-ascii?Q?5lDdxCklH++uHWZHlD0VpQWS1RykKKKFpvAeZXqnzdwLXJi4Rm1DKCeQKUWd?=
 =?us-ascii?Q?wnpp2LRqP0YA/TrVBKfbxJlNlngdJaVGVTwkxoowbmxtrL7qRPCpXxBmzmOE?=
 =?us-ascii?Q?BquMvBbRwmZJ0g+Nzjl2C+6uWbXIFTxLSClQDeFhfW3/3KQWp0kfX87tEt8/?=
 =?us-ascii?Q?P0sLXEKAoIVgF4MCX/PJPYwFQdrA0lbzHJE3LsO27Y9mAH67kTysLjEYmbrY?=
 =?us-ascii?Q?UVVaXs89JSiV5FRA+ZU7iFoNpxrjSiVCmmraOOqATdtgvk3qKlcUpmvVXBrg?=
 =?us-ascii?Q?yfpVYqbPqXpMSA1dhuxW8C9cTBHeKwHAj4zn4rmgePg97To1BfwSgZtgvLMO?=
 =?us-ascii?Q?/47pGj6Gf3ozIG/5j/bztwociFmY3Kiro+tdjfJXyZlPZygxMxEQp4Ph81p2?=
 =?us-ascii?Q?wPPw/ll3lhRLs+swwGyHyg/aFfc0FvYRBRq/fn2fbCf3j2c3hbHmlUB8kGdx?=
 =?us-ascii?Q?XOo7K83YxymDw5evPrb6JeIkeB1amdJHjk/VaEwqbsNoM9ezud7T2nHBVH3P?=
 =?us-ascii?Q?18mX2cQQ62JdMlZHrIAWrT3CjOm+WBXYyR6AIpVT2KJjtfne5H1opC/EsltO?=
 =?us-ascii?Q?RRfLgoPpxwFywIToeQB0+4Y0qD0zATiSPbWYn3jTimC+WEaXWMAUgIkdEQ+4?=
 =?us-ascii?Q?sKYihSwURqwK9MEU/vcDoJ4=3D?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 1f3f391c-f248-46da-7edc-08d99e9832ad
X-MS-Exchange-CrossTenant-AuthSource: CY4PR1201MB0246.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 03 Nov 2021 07:04:37.3703
 (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: 1G6X4PVhHojpzvbJMRA4anelEhePqhCNjFjDouIPJTwJndGow0myddYAl6t5KtiljUfFxZqVlJXenPz3X1tr9A==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY4PR12MB1461
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Wed, Nov 03, 2021 at 04:11:09AM +0800, Fontenot, Nathan wrote:
> 
> 
> On 10/29/21 8:02 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-States 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.
> 
> s/similiar/similar/
> 
> > 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-States 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 f233a6ab75ac..92b9fb631768 100644
> > --- a/tools/power/cpupower/utils/helpers/amd.c
> > +++ b/tools/power/cpupower/utils/helpers/amd.c
> > @@ -182,5 +182,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 = amd_pstate_get_data(cpu, AMD_PSTATE_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-States 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 e03cc97297aa..c03925bea655 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-States stuff **************************/
> >  extern bool cpupower_amd_pstate_enabled(void);
> > +extern void amd_pstate_boost_init(unsigned int cpu,
> > +				  int *support, int *active);
> >  
> >  /* AMD P-States 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; }
> 
> I don't believe the return statement is needed here, can just be {}
> 

Updated.

Thanks,
Ray