| Message ID | cover.1718606975.git.perry.yuan@amd.com |
|---|---|
| Headers | show |
| Series | AMD Pstate Driver Fixes and Improvements | expand |
On 6/17/2024 01:59, Perry Yuan wrote: > Introduces an optimization to the AMD-Pstate driver by implementing > a heterogeneous core topology for the initialization of the highest > performance value while driver loading. > The two core types supported are "performance" and "efficiency". > Each core type has different highest performance and frequency values > configured by the platform. The `amd_pstate` driver needs to identify > the type of core to correctly set an appropriate highest perf value. > > X86_FEATURE_HETERO_CORE_TOPOLOGY is used to identify whether the > processor support heterogeneous core type by reading CPUID leaf > Fn_0x80000026_EAX and bit 30. if the bit is set as one, then amd_pstate > driver will check EBX 30:28 bits to get the core type. > > Reference: > See the page 119 of PPR for AMD Family 19h Model 61h B1, docID 56713 > > Signed-off-by: Perry Yuan <perry.yuan@amd.com> > --- > arch/x86/include/asm/processor.h | 2 ++ > arch/x86/kernel/cpu/amd.c | 19 ++++++++++++ > drivers/cpufreq/amd-pstate.c | 53 ++++++++++++++++++++++++++++++-- > 3 files changed, 71 insertions(+), 3 deletions(-) > > diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h > index cb4f6c513c48..223aa58e2d5c 100644 > --- a/arch/x86/include/asm/processor.h > +++ b/arch/x86/include/asm/processor.h > @@ -694,10 +694,12 @@ static inline u32 per_cpu_l2c_id(unsigned int cpu) > extern u32 amd_get_highest_perf(void); > extern void amd_clear_divider(void); > extern void amd_check_microcode(void); > +extern int amd_get_this_core_type(void); > #else > static inline u32 amd_get_highest_perf(void) { return 0; } > static inline void amd_clear_divider(void) { } > static inline void amd_check_microcode(void) { } > +static inline int amd_get_this_core_type(void) { return -1; } > #endif > > extern unsigned long arch_align_stack(unsigned long sp); > diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c > index 44df3f11e731..62a4ef21ef79 100644 > --- a/arch/x86/kernel/cpu/amd.c > +++ b/arch/x86/kernel/cpu/amd.c > @@ -1231,3 +1231,22 @@ void noinstr amd_clear_divider(void) > :: "a" (0), "d" (0), "r" (1)); > } > EXPORT_SYMBOL_GPL(amd_clear_divider); > + > +#define X86_CPU_TYPE_ID_SHIFT 28 > + > +/** > + * amd_get_this_core_type - Get the type of this heterogeneous CPU > + * > + * Returns the CPU type [31:28] (i.e., performance or efficient) of > + * a CPU in the processor. > + * If the processor has no core type support, returns -1. > + */ > + > +int amd_get_this_core_type(void) Did you miss my feedback from v3? I don't see changes for the return type or for returning CPU_CORE_TYPE_NO_HETERO_SUP instead of -1. > +{ > + if (!cpu_feature_enabled(X86_FEATURE_HETERO_CORE_TOPOLOGY)) > + return -1; > + > + return cpuid_ebx(0x80000026) >> X86_CPU_TYPE_ID_SHIFT; > +} > +EXPORT_SYMBOL_GPL(amd_get_this_core_type); > diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c > index cb750ef305fe..cf68343219d1 100644 > --- a/drivers/cpufreq/amd-pstate.c > +++ b/drivers/cpufreq/amd-pstate.c > @@ -52,8 +52,10 @@ > #define AMD_PSTATE_TRANSITION_LATENCY 20000 > #define AMD_PSTATE_TRANSITION_DELAY 1000 > #define AMD_PSTATE_FAST_CPPC_TRANSITION_DELAY 600 > -#define CPPC_HIGHEST_PERF_PERFORMANCE 196 > -#define CPPC_HIGHEST_PERF_DEFAULT 166 > + > +#define CPPC_HIGHEST_PERF_EFFICIENT 132 > +#define CPPC_HIGHEST_PERF_PERFORMANCE 196 > +#define CPPC_HIGHEST_PERF_DEFAULT 166 > > #define AMD_CPPC_EPP_PERFORMANCE 0x00 > #define AMD_CPPC_EPP_BALANCE_PERFORMANCE 0x80 > @@ -86,6 +88,14 @@ struct quirk_entry { > u32 lowest_freq; > }; > > +/* defined by CPUID_Fn80000026_EBX BIT [31:28] */ > +enum amd_core_type { > + CPU_CORE_TYPE_NO_HETERO_SUP = -1, > + CPU_CORE_TYPE_PERFORMANCE = 0, > + CPU_CORE_TYPE_EFFICIENCY = 1, > + CPU_CORE_TYPE_UNDEFINED = 2, > +}; > + > /* > * TODO: We need more time to fine tune processors with shared memory solution > * with community together. > @@ -358,9 +368,27 @@ static inline int amd_pstate_enable(bool enable) > return static_call(amd_pstate_enable)(enable); > } > > +static void get_this_core_type(void *data) > +{ > + enum amd_core_type *cpu_type = data; > + > + *cpu_type = amd_get_this_core_type(); > +} > + > +static enum amd_core_type amd_pstate_get_cpu_type(int cpu) > +{ > + enum amd_core_type cpu_type; > + > + smp_call_function_single(cpu, get_this_core_type, &cpu_type, 1); > + > + return cpu_type; > +} > + > static u32 amd_pstate_highest_perf_set(struct amd_cpudata *cpudata) > { > struct cpuinfo_x86 *c = &cpu_data(0); > + u32 highest_perf; > + enum amd_core_type core_type; > > /* > * For AMD CPUs with Family ID 19H and Model ID range 0x70 to 0x7f, > @@ -370,7 +398,26 @@ static u32 amd_pstate_highest_perf_set(struct amd_cpudata *cpudata) > if (c->x86 == 0x19 && (c->x86_model >= 0x70 && c->x86_model <= 0x7f)) > return CPPC_HIGHEST_PERF_PERFORMANCE; > > - return CPPC_HIGHEST_PERF_DEFAULT; > + core_type = amd_pstate_get_cpu_type(cpudata->cpu); > + pr_debug("core_type %d found\n", core_type); > + > + switch (core_type) { > + case CPU_CORE_TYPE_NO_HETERO_SUP: > + highest_perf = CPPC_HIGHEST_PERF_DEFAULT; > + break; > + case CPU_CORE_TYPE_PERFORMANCE: > + highest_perf = CPPC_HIGHEST_PERF_PERFORMANCE; > + break; > + case CPU_CORE_TYPE_EFFICIENCY: > + highest_perf = CPPC_HIGHEST_PERF_EFFICIENT; > + break; > + default: > + highest_perf = CPPC_HIGHEST_PERF_DEFAULT; > + WARN_ONCE(true, "WARNING: Undefined core type found"); > + break; > + } > + > + return highest_perf; > } > > static int pstate_init_perf(struct amd_cpudata *cpudata)
On 6/17/2024 01:59, Perry Yuan wrote: > To address issues with the loading of the amd-pstate driver on certain platforms, > It needs to enable dynamic debugging to capture debug messages during the driver > loading process. By adding "amd_pstate.dyndbg=+p cppc_acpi.dyndbg=+p debug > amd_pstate=active" and loglevel to the kernel command line, then driver debug > logging is enabled. > > Signed-off-by: Perry Yuan <perry.yuan@amd.com> > --- > Documentation/admin-guide/pm/amd-pstate.rst | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/Documentation/admin-guide/pm/amd-pstate.rst b/Documentation/admin-guide/pm/amd-pstate.rst > index 1e0d101b020a..ceeb073c9ada 100644 > --- a/Documentation/admin-guide/pm/amd-pstate.rst > +++ b/Documentation/admin-guide/pm/amd-pstate.rst > @@ -472,6 +472,20 @@ operations for the new ``amd-pstate`` module with this tool. :: > Diagnostics and Tuning > ======================= > > +Debugging AMD P-State Driver Loading Issues > +------------------------------------------ > + > +If the amd-pstate driver fails to load, additional debug information > +may be necessary. > +To capture debug messages for issue analysis, users can add below parameter, > +"amd_pstate.dyndbg=+p cppc_acpi.dyndbg=+p debug" What keys off "debug"? I would expect that dynamic debugging for the first two is sufficient. That being said; isn't everything important shown as a warning now with this series? Is this patch still needed? > +to the kernel command line. This will enable dynamic debugging and allow better > +analysis and troubleshooting of the driver loading process. > + > +Please note that adding this parameter will only enable debug logging during the > +driver loading phase and may affect system behavior. Use this option with caution > +and only for debugging purposes. > + > Trace Events > -------------- >
On 6/17/2024 01:59, Perry Yuan wrote: > The amd-pstate-epp driver has been implemented and resolves the > performance drop issue seen in passive mode. Users who enable the > active mode driver will not experience a performance drop compared > to the passive mode driver. Therefore, the EPP driver should be > loaded by default at system boot. I think this commit message should specifically reference that it's being enabled by default on shared memory designs as that's the net new. The code change looks good, go ahead and add Reviewed-by: Mario Limonciello <mario.limonciello@amd.com> on the next version as long as you've added a sentence about shared memory designs to commit message. > > Signed-off-by: Perry Yuan <perry.yuan@amd.com> > --- > drivers/cpufreq/amd-pstate.c | 13 +------------ > 1 file changed, 1 insertion(+), 12 deletions(-) > > diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c > index b48fd60cbc6d..eca2f7dcf7ce 100644 > --- a/drivers/cpufreq/amd-pstate.c > +++ b/drivers/cpufreq/amd-pstate.c > @@ -96,15 +96,6 @@ enum amd_core_type { > CPU_CORE_TYPE_UNDEFINED = 2, > }; > > -/* > - * TODO: We need more time to fine tune processors with shared memory solution > - * with community together. > - * > - * There are some performance drops on the CPU benchmarks which reports from > - * Suse. We are co-working with them to fine tune the shared memory solution. So > - * we disable it by default to go acpi-cpufreq on these processors and add a > - * module parameter to be able to enable it manually for debugging. > - */ > static struct cpufreq_driver *current_pstate_driver; > static struct cpufreq_driver amd_pstate_driver; > static struct cpufreq_driver amd_pstate_epp_driver; > @@ -1867,11 +1858,9 @@ static int __init amd_pstate_init(void) > /* Disable on the following configs by default: > * 1. Undefined platforms > * 2. Server platforms > - * 3. Shared memory designs > */ > if (amd_pstate_acpi_pm_profile_undefined() || > - amd_pstate_acpi_pm_profile_server() || > - !cpu_feature_enabled(X86_FEATURE_CPPC)) { > + amd_pstate_acpi_pm_profile_server()) { > pr_info("driver load is disabled, boot with specific mode to enable this\n"); > return -ENODEV; > }
On Mon, Jun 17, 2024 at 02:59:11PM +0800, Perry Yuan wrote: > Introduces an optimization to the AMD-Pstate driver by implementing > a heterogeneous core topology for the initialization of the highest > performance value while driver loading. > The two core types supported are "performance" and "efficiency". > Each core type has different highest performance and frequency values > configured by the platform. The `amd_pstate` driver needs to identify > the type of core to correctly set an appropriate highest perf value. > > X86_FEATURE_HETERO_CORE_TOPOLOGY is used to identify whether the > processor support heterogeneous core type by reading CPUID leaf > Fn_0x80000026_EAX and bit 30. if the bit is set as one, then amd_pstate > driver will check EBX 30:28 bits to get the core type. There will be a special ->cpu_type member for that eventually: https://lore.kernel.org/r/20240617-add-cpu-type-v1-1-b88998c01e76@linux.intel.com
[AMD Official Use Only - AMD Internal Distribution Only] > -----Original Message----- > From: Limonciello, Mario <Mario.Limonciello@amd.com> > Sent: Wednesday, June 19, 2024 3:25 AM > To: Yuan, Perry <Perry.Yuan@amd.com>; rafael.j.wysocki@intel.com; > viresh.kumar@linaro.org; Huang, Ray <Ray.Huang@amd.com>; Shenoy, > Gautham Ranjal <gautham.shenoy@amd.com>; Petkov, Borislav > <Borislav.Petkov@amd.com> > Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Huang, Shimmer > <Shimmer.Huang@amd.com>; Du, Xiaojian <Xiaojian.Du@amd.com>; Meng, > Li (Jassmine) <Li.Meng@amd.com>; linux-pm@vger.kernel.org; linux- > kernel@vger.kernel.org > Subject: Re: [PATCH v4 11/11] cpufreq: amd-pstate: enable shared memory > type CPPC by default > > On 6/17/2024 01:59, Perry Yuan wrote: > > The amd-pstate-epp driver has been implemented and resolves the > > performance drop issue seen in passive mode. Users who enable the > > active mode driver will not experience a performance drop compared to > > the passive mode driver. Therefore, the EPP driver should be loaded by > > default at system boot. > > I think this commit message should specifically reference that it's being > enabled by default on shared memory designs as that's the net new. > > The code change looks good, go ahead and add > > Reviewed-by: Mario Limonciello <mario.limonciello@amd.com> > > on the next version as long as you've added a sentence about shared > memory designs to commit message. Thanks for the review, will add the shared memory systems words to commit log in the next version. > > > > > Signed-off-by: Perry Yuan <perry.yuan@amd.com> > > --- > > drivers/cpufreq/amd-pstate.c | 13 +------------ > > 1 file changed, 1 insertion(+), 12 deletions(-) > > > > diff --git a/drivers/cpufreq/amd-pstate.c > > b/drivers/cpufreq/amd-pstate.c index b48fd60cbc6d..eca2f7dcf7ce 100644 > > --- a/drivers/cpufreq/amd-pstate.c > > +++ b/drivers/cpufreq/amd-pstate.c > > @@ -96,15 +96,6 @@ enum amd_core_type { > > CPU_CORE_TYPE_UNDEFINED = 2, > > }; > > > > -/* > > - * TODO: We need more time to fine tune processors with shared memory > > solution > > - * with community together. > > - * > > - * There are some performance drops on the CPU benchmarks which > > reports from > > - * Suse. We are co-working with them to fine tune the shared memory > > solution. So > > - * we disable it by default to go acpi-cpufreq on these processors > > and add a > > - * module parameter to be able to enable it manually for debugging. > > - */ > > static struct cpufreq_driver *current_pstate_driver; > > static struct cpufreq_driver amd_pstate_driver; > > static struct cpufreq_driver amd_pstate_epp_driver; @@ -1867,11 > > +1858,9 @@ static int __init amd_pstate_init(void) > > /* Disable on the following configs by default: > > * 1. Undefined platforms > > * 2. Server platforms > > - * 3. Shared memory designs > > */ > > if (amd_pstate_acpi_pm_profile_undefined() || > > - amd_pstate_acpi_pm_profile_server() || > > - !cpu_feature_enabled(X86_FEATURE_CPPC)) { > > + amd_pstate_acpi_pm_profile_server()) { > > pr_info("driver load is disabled, boot with specific > mode to enable this\n"); > > return -ENODEV; > > }
[AMD Official Use Only - AMD Internal Distribution Only] > -----Original Message----- > From: Limonciello, Mario <Mario.Limonciello@amd.com> > Sent: Wednesday, June 19, 2024 3:23 AM > To: Yuan, Perry <Perry.Yuan@amd.com>; rafael.j.wysocki@intel.com; > viresh.kumar@linaro.org; Huang, Ray <Ray.Huang@amd.com>; Shenoy, > Gautham Ranjal <gautham.shenoy@amd.com>; Petkov, Borislav > <Borislav.Petkov@amd.com> > Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Huang, Shimmer > <Shimmer.Huang@amd.com>; Du, Xiaojian <Xiaojian.Du@amd.com>; Meng, > Li (Jassmine) <Li.Meng@amd.com>; linux-pm@vger.kernel.org; linux- > kernel@vger.kernel.org > Subject: Re: [PATCH v4 09/11] cpufreq: amd-pstate: implement > heterogeneous core topology for highest performance initialization > > On 6/17/2024 01:59, Perry Yuan wrote: > > Introduces an optimization to the AMD-Pstate driver by implementing a > > heterogeneous core topology for the initialization of the highest > > performance value while driver loading. > > The two core types supported are "performance" and "efficiency". > > Each core type has different highest performance and frequency values > > configured by the platform. The `amd_pstate` driver needs to identify > > the type of core to correctly set an appropriate highest perf value. > > > > X86_FEATURE_HETERO_CORE_TOPOLOGY is used to identify whether the > > processor support heterogeneous core type by reading CPUID leaf > > Fn_0x80000026_EAX and bit 30. if the bit is set as one, then > > amd_pstate driver will check EBX 30:28 bits to get the core type. > > > > Reference: > > See the page 119 of PPR for AMD Family 19h Model 61h B1, docID 56713 > > > > Signed-off-by: Perry Yuan <perry.yuan@amd.com> > > --- > > arch/x86/include/asm/processor.h | 2 ++ > > arch/x86/kernel/cpu/amd.c | 19 ++++++++++++ > > drivers/cpufreq/amd-pstate.c | 53 ++++++++++++++++++++++++++++++- > - > > 3 files changed, 71 insertions(+), 3 deletions(-) > > > > diff --git a/arch/x86/include/asm/processor.h > > b/arch/x86/include/asm/processor.h > > index cb4f6c513c48..223aa58e2d5c 100644 > > --- a/arch/x86/include/asm/processor.h > > +++ b/arch/x86/include/asm/processor.h > > @@ -694,10 +694,12 @@ static inline u32 per_cpu_l2c_id(unsigned int > cpu) > > extern u32 amd_get_highest_perf(void); > > extern void amd_clear_divider(void); > > extern void amd_check_microcode(void); > > +extern int amd_get_this_core_type(void); > > #else > > static inline u32 amd_get_highest_perf(void) { return 0; } > > static inline void amd_clear_divider(void) { } > > static inline void amd_check_microcode(void) { } > > +static inline int amd_get_this_core_type(void) { return -1; } > > #endif > > > > extern unsigned long arch_align_stack(unsigned long sp); diff --git > > a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c index > > 44df3f11e731..62a4ef21ef79 100644 > > --- a/arch/x86/kernel/cpu/amd.c > > +++ b/arch/x86/kernel/cpu/amd.c > > @@ -1231,3 +1231,22 @@ void noinstr amd_clear_divider(void) > > :: "a" (0), "d" (0), "r" (1)); > > } > > EXPORT_SYMBOL_GPL(amd_clear_divider); > > + > > +#define X86_CPU_TYPE_ID_SHIFT 28 > > + > > +/** > > + * amd_get_this_core_type - Get the type of this heterogeneous CPU > > + * > > + * Returns the CPU type [31:28] (i.e., performance or efficient) of > > + * a CPU in the processor. > > + * If the processor has no core type support, returns -1. > > + */ > > + > > +int amd_get_this_core_type(void) > > > Did you miss my feedback from v3? I don't see changes for the return type > or for returning CPU_CORE_TYPE_NO_HETERO_SUP instead of -1. This CPU_CORE_TYPE_NO_HETERO_SUP is defined in the amd_pstate.c, if we want to use it, it will need to define them in another header. Boris also mentioned that there is another patchset working to export core types in future, we can use "-1" in short term. Once the core type patches finalize the solution, we can rework the pstate driver. Firstly, let`s provide a workable solution, then improve the driver with coming patches. Perry. > > > > +{ > > + if (!cpu_feature_enabled(X86_FEATURE_HETERO_CORE_TOPOLOGY)) > > + return -1; > > + > > + return cpuid_ebx(0x80000026) >> X86_CPU_TYPE_ID_SHIFT; > > +} > > +EXPORT_SYMBOL_GPL(amd_get_this_core_type); > > diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c > > index cb750ef305fe..cf68343219d1 100644 > > --- a/drivers/cpufreq/amd-pstate.c > > +++ b/drivers/cpufreq/amd-pstate.c > > @@ -52,8 +52,10 @@ > > #define AMD_PSTATE_TRANSITION_LATENCY 20000 > > #define AMD_PSTATE_TRANSITION_DELAY 1000 > > #define AMD_PSTATE_FAST_CPPC_TRANSITION_DELAY 600 > > -#define CPPC_HIGHEST_PERF_PERFORMANCE 196 > > -#define CPPC_HIGHEST_PERF_DEFAULT 166 > > + > > +#define CPPC_HIGHEST_PERF_EFFICIENT 132 > > +#define CPPC_HIGHEST_PERF_PERFORMANCE 196 > > +#define CPPC_HIGHEST_PERF_DEFAULT 166 > > > > #define AMD_CPPC_EPP_PERFORMANCE 0x00 > > #define AMD_CPPC_EPP_BALANCE_PERFORMANCE 0x80 > > @@ -86,6 +88,14 @@ struct quirk_entry { > > u32 lowest_freq; > > }; > > > > +/* defined by CPUID_Fn80000026_EBX BIT [31:28] */ > > +enum amd_core_type { > > + CPU_CORE_TYPE_NO_HETERO_SUP = -1, > > + CPU_CORE_TYPE_PERFORMANCE = 0, > > + CPU_CORE_TYPE_EFFICIENCY = 1, > > + CPU_CORE_TYPE_UNDEFINED = 2, > > +}; > > + > > /* > > * TODO: We need more time to fine tune processors with shared memory > solution > > * with community together. > > @@ -358,9 +368,27 @@ static inline int amd_pstate_enable(bool enable) > > return static_call(amd_pstate_enable)(enable); > > } > > > > +static void get_this_core_type(void *data) > > +{ > > + enum amd_core_type *cpu_type = data; > > + > > + *cpu_type = amd_get_this_core_type(); > > +} > > + > > +static enum amd_core_type amd_pstate_get_cpu_type(int cpu) > > +{ > > + enum amd_core_type cpu_type; > > + > > + smp_call_function_single(cpu, get_this_core_type, &cpu_type, 1); > > + > > + return cpu_type; > > +} > > + > > static u32 amd_pstate_highest_perf_set(struct amd_cpudata *cpudata) > > { > > struct cpuinfo_x86 *c = &cpu_data(0); > > + u32 highest_perf; > > + enum amd_core_type core_type; > > > > /* > > * For AMD CPUs with Family ID 19H and Model ID range 0x70 to 0x7f, > > @@ -370,7 +398,26 @@ static u32 amd_pstate_highest_perf_set(struct > amd_cpudata *cpudata) > > if (c->x86 == 0x19 && (c->x86_model >= 0x70 && c->x86_model <= > 0x7f)) > > return CPPC_HIGHEST_PERF_PERFORMANCE; > > > > - return CPPC_HIGHEST_PERF_DEFAULT; > > + core_type = amd_pstate_get_cpu_type(cpudata->cpu); > > + pr_debug("core_type %d found\n", core_type); > > + > > + switch (core_type) { > > + case CPU_CORE_TYPE_NO_HETERO_SUP: > > + highest_perf = CPPC_HIGHEST_PERF_DEFAULT; > > + break; > > + case CPU_CORE_TYPE_PERFORMANCE: > > + highest_perf = CPPC_HIGHEST_PERF_PERFORMANCE; > > + break; > > + case CPU_CORE_TYPE_EFFICIENCY: > > + highest_perf = CPPC_HIGHEST_PERF_EFFICIENT; > > + break; > > + default: > > + highest_perf = CPPC_HIGHEST_PERF_DEFAULT; > > + WARN_ONCE(true, "WARNING: Undefined core type found"); > > + break; > > + } > > + > > + return highest_perf; > > } > > > > static int pstate_init_perf(struct amd_cpudata *cpudata)
[AMD Official Use Only - AMD Internal Distribution Only] Hi Boris, > -----Original Message----- > From: Borislav Petkov <bp@alien8.de> > Sent: Wednesday, June 19, 2024 5:24 AM > To: Yuan, Perry <Perry.Yuan@amd.com> > Cc: rafael.j.wysocki@intel.com; Limonciello, Mario > <Mario.Limonciello@amd.com>; viresh.kumar@linaro.org; Huang, Ray > <Ray.Huang@amd.com>; Shenoy, Gautham Ranjal > <gautham.shenoy@amd.com>; Deucher, Alexander > <Alexander.Deucher@amd.com>; Huang, Shimmer > <Shimmer.Huang@amd.com>; Du, Xiaojian <Xiaojian.Du@amd.com>; Meng, > Li (Jassmine) <Li.Meng@amd.com>; linux-pm@vger.kernel.org; linux- > kernel@vger.kernel.org > Subject: Re: [PATCH v4 09/11] cpufreq: amd-pstate: implement > heterogeneous core topology for highest performance initialization > > On Mon, Jun 17, 2024 at 02:59:11PM +0800, Perry Yuan wrote: > > Introduces an optimization to the AMD-Pstate driver by implementing a > > heterogeneous core topology for the initialization of the highest > > performance value while driver loading. > > The two core types supported are "performance" and "efficiency". > > Each core type has different highest performance and frequency values > > configured by the platform. The `amd_pstate` driver needs to identify > > the type of core to correctly set an appropriate highest perf value. > > > > X86_FEATURE_HETERO_CORE_TOPOLOGY is used to identify whether the > > processor support heterogeneous core type by reading CPUID leaf > > Fn_0x80000026_EAX and bit 30. if the bit is set as one, then > > amd_pstate driver will check EBX 30:28 bits to get the core type. > > There will be a special ->cpu_type member for that eventually: > > https://lore.kernel.org/r/20240617-add-cpu-type-v1-1- > b88998c01e76@linux.intel.com Great, I saw your comments in that patchset, hopefully Intel and AMD can have a common design for the core type. Feel free to let me know if you want me to try any testing patches. > > -- > Regards/Gruss, > Boris. > > https://people.kernel.org/tglx/notes-about-netiquette
Hello everyone, This patchset addresses critical issues and enhances performance settings for CPUs with heterogeneous core types in the AMD pstate driver. Specifically, it resolves problems related to calculating the highest performance and frequency on the latest CPUs with preferred cores. Additionally, the patchset includes documentation improvements in amd-pstate.rst, offering a comprehensive guide covering topics such as recommended reboot requirements during driver switching, debugging procedures for driver loading failures. Your feedback and suggestions for improvement are highly appreciated. Please review the patches and provide your valuable input. Thank you. Best regards, Perry. Changes from V3: * add one new patch to enable shared memory type CPPC by default * pick all the RB by and ACk by flags from Mario and Gautham * update the patch #7 PPR link with doc id (Boris & Mario) * fix the highest perf initialization issue with preferred core check for patch #9 (Gautham) * rework return core type and commit log for the patch #9 (Mario) * address feedback for patch #5 for the debugging suggestions.(Mario) * retest the patches on MSR and shared memory type CPPC systems, no regression seen. Changes from V2: * pick review by and ack by flags from Mario and Gautham * rebase to latest linux-pm bleeding edge branch * fix driver loading block issue for patch 4, make sure the warning will not abort the driver loading in case there are some new family/model id. * fix the driver loading sequence issue for patch 10, it allows command line and kernel config option together. command line will override kconfig option. * add back the AMD CPUs with Family ID 19H and Model ID range 0x70 to 0x7f to return the highest perf and check others CPU core type in the following codes. * run some testing on the local system. * move the amd_core_type to amd-pstate.c because of the amd-pstate.h was removed lately. Changes from V1: * drop patch 11 which has been merged in a separate patch. (Mario) * fix some typos in commit log and tile (Mario) * fix the patch 11 regression issue of kernel command line (Oleksandr Natalenko) * pick ack flag for patch 7 (Mario) * drop patch 4 which is not recommended for user(Mario) * rebase to linux-pm/bleeding-edge branch * fix some build warning * rework the patch 3 for CPU ID matching(Mario) * address feedback for patch 5 (Mario) * move the acpi pm profile after got default mode(Mario) Perry Yuan (11): cpufreq: amd-pstate: optimize the initial frequency values verification cpufreq: amd-pstate: remove unused variable nominal_freq cpufreq: amd-pstate: show CPPC debug message if CPPC is not supported cpufreq: amd-pstate: add debug message while CPPC is supported and disabled by SBIOS Documentation: PM: amd-pstate: add debugging section for driver loading failure Documentation: PM: amd-pstate: add guided mode to the Operation mode cpufreq: amd-pstate: switch boot_cpu_has() to cpu_feature_enabled() x86/cpufeatures: Add feature bits for AMD heterogeneous processor cpufreq: amd-pstate: implement heterogeneous core topology for highest performance initialization cpufreq: amd-pstate: auto-load pstate driver by default cpufreq: amd-pstate: enable shared memory type CPPC by default Documentation/admin-guide/pm/amd-pstate.rst | 16 +- arch/x86/include/asm/cpufeatures.h | 1 + arch/x86/include/asm/processor.h | 2 + arch/x86/kernel/cpu/amd.c | 19 ++ arch/x86/kernel/cpu/scattered.c | 1 + drivers/cpufreq/amd-pstate.c | 198 ++++++++++++++------ 6 files changed, 182 insertions(+), 55 deletions(-)