[AMD Official Use Only - General] Hi Thomas,
> -----Original Message----- > From: Thomas Monjalon <tho...@monjalon.net> > Sent: Thursday, May 25, 2023 12:17 AM > To: Tummala, Sivaprasad <sivaprasad.tumm...@amd.com> > Cc: david.h...@intel.com; dev@dpdk.org; Yigit, Ferruh <ferruh.yi...@amd.com>; > anatoly.bura...@intel.com; Laatz, Kevin <kevin.la...@intel.com> > Subject: Re: [PATCH v1] power: support amd-pstate cpufreq driver > > Caution: This message originated from an External Source. Use proper caution > when opening attachments, clicking links, or responding. > > > 12/04/2023 11:52, Sivaprasad Tummala: > > amd-pstate introduces a new CPU frequency control mechanism for AMD > > processors using the ACPI Collaborative Performance Power Control > > feature for a finer grained frequency management. > > > > Patch to add support for amd-pstate driver. > > > > Signed-off-by: Sivaprasad Tummala <sivaprasad.tumm...@amd.com> > > --- > > app/test/test_power.c | 1 + > > app/test/test_power_cpufreq.c | 5 +- > > doc/guides/rel_notes/release_23_07.rst | 3 + > > examples/l3fwd-power/main.c | 1 + > > lib/power/meson.build | 1 + > > lib/power/power_amd_pstate_cpufreq.c | 698 > +++++++++++++++++++++++++ > > lib/power/power_amd_pstate_cpufreq.h | 219 ++++++++ > > lib/power/rte_power.c | 26 + > > lib/power/rte_power.h | 3 +- > > lib/power/rte_power_pmd_mgmt.c | 6 +- > > 10 files changed, 958 insertions(+), 5 deletions(-) > > I'm not comfortable to merge this patch without a word from David Hunt. > Given there is 0 review, what do we do? Yes, we are waiting for feedback from the community on this patch. > > > > Also, make sure to start the actual text at the margin. > > > ==================================================== > === > > > > + * **Added amd-pstate driver support to power management library.** > > + > > + Added support for amd-pstate driver which works on AMD Zen > > processors. > > Looks like the indent is not correct. Sure, will fix it in v2 patch > > > 'power_pstate_cpufreq.c', > > + 'power_amd_pstate_cpufreq.c', > > Can you say briefly why AMD has a different pstate? > Does it mean power_pstate_cpufreq.c should be renamed > power_intel_pstate_cpufreq.c? amd-pstate is the new AMD CPU performance scaling driver based on CPPC. It is implemented as a new kernel driver and we need a different library from Power_pstate_cpufreq. Yes, it makes sense to rename this, inline with the Linux kernel as indicated below. linux/latest/source/drivers/cpufreq - amd-pstate.c - cppc_cpufreq.c - intel_pstate.c > > > +++ b/lib/power/power_amd_pstate_cpufreq.c > > @@ -0,0 +1,698 @@ > > +/* SPDX-License-Identifier: BSD-3-Clause > > + * Copyright(c) 2010-2021 Intel Corporation > > + * Copyright(c) 2021 Arm Limited > > Why is there copyright for Intel and Arm? > Does it mean you copied some code and did not try to keep common code in a > common place? Yes, few internal functions follow similar notion as power_cppc_cpufreq.c > > > + * Copyright(c) 2023 Amd Limited > > Thanks & Regards, Sivaprasad
