| Message ID | 20240826211358.2694603-6-superm1@kernel.org (mailing list archive) |
|---|---|
| State | Handled Elsewhere, archived |
| Headers | show |
| Series | Adjustments for preferred core detection | expand |
On Mon, Aug 26, 2024 at 04:13:55PM -0500, Mario Limonciello wrote: > From: Mario Limonciello <mario.limonciello@amd.com> > > AMD systems that support preferred cores will use "166" as their > numerator for max frequency calculations instead of "255". > > Add a function for detecting preferred cores by looking at the > highest perf value on all cores. > > If preferred cores are enabled return 166 and if disabled the > value in the highest perf register. As the function will be called > multiple times, cache the values for the boost numerator and if > preferred cores will be enabled in global variables. > > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> > --- [..snip..] > /** > * amd_get_boost_ratio_numerator: Get the numerator to use for boost ratio calculation > * @cpu: CPU to get numerator for. > @@ -162,20 +232,19 @@ EXPORT_SYMBOL_GPL(amd_get_highest_perf); > */ > int amd_get_boost_ratio_numerator(unsigned int cpu, u64 *numerator) > { > - struct cpuinfo_x86 *c = &boot_cpu_data; > + bool prefcore; > + int ret; > > - if (c->x86 == 0x17 && ((c->x86_model >= 0x30 && c->x86_model < 0x40) || > - (c->x86_model >= 0x70 && c->x86_model < 0x80))) { > - *numerator = 166; > - return 0; > - } > + ret = amd_detect_prefcore(&prefcore); > + if (ret) > + return ret; > > - if (c->x86 == 0x19 && ((c->x86_model >= 0x20 && c->x86_model < 0x30) || > - (c->x86_model >= 0x40 && c->x86_model < 0x70))) { > - *numerator = 166; > + /* without preferred cores, return the highest perf register value */ > + if (!prefcore) { > + *numerator = boost_numerator; > return 0; > } > - *numerator = 255; > + *numerator = CPPC_HIGHEST_PERF_PREFCORE; Interesting. So even when the user boots a system that supports preferred-cores with "amd_preferred=disable", amd_get_boost_ratio_numerator() will return CPPC_HIGHEST_PERF_PREFCORE as the call prefcore == true here. I suppose that is as intended, since even though the user may not want to use the preferred core logic for task-scheduling/load-balancing, the numerator for the boost-ratio is purely dependent on the h/w capability. Is this understanding correct? If so, can this be included as a comment in the code ? The rest of the patch looks good to me. Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com> -- Thanks and Regards gautham. > > return 0; > } > diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c > index f470b5700db58..ec32c830abc1d 100644 > --- a/drivers/cpufreq/amd-pstate.c > +++ b/drivers/cpufreq/amd-pstate.c > @@ -807,32 +807,18 @@ static DECLARE_WORK(sched_prefcore_work, amd_pstste_sched_prefcore_workfn); > > static void amd_pstate_init_prefcore(struct amd_cpudata *cpudata) > { > - int ret, prio; > - u32 highest_perf; > - > - ret = amd_get_highest_perf(cpudata->cpu, &highest_perf); > - if (ret) > + /* user disabled or not detected */ > + if (!amd_pstate_prefcore) > return; > > cpudata->hw_prefcore = true; > - /* check if CPPC preferred core feature is enabled*/ > - if (highest_perf < CPPC_MAX_PERF) > - prio = (int)highest_perf; > - else { > - pr_debug("AMD CPPC preferred core is unsupported!\n"); > - cpudata->hw_prefcore = false; > - return; > - } > - > - if (!amd_pstate_prefcore) > - return; > > /* > * The priorities can be set regardless of whether or not > * sched_set_itmt_support(true) has been called and it is valid to > * update them at any time after it has been called. > */ > - sched_set_itmt_core_prio(prio, cpudata->cpu); > + sched_set_itmt_core_prio((int)READ_ONCE(cpudata->highest_perf), cpudata->cpu); > > schedule_work(&sched_prefcore_work); > } > @@ -998,12 +984,12 @@ static int amd_pstate_cpu_init(struct cpufreq_policy *policy) > > cpudata->cpu = policy->cpu; > > - amd_pstate_init_prefcore(cpudata); > - > ret = amd_pstate_init_perf(cpudata); > if (ret) > goto free_cpudata1; > > + amd_pstate_init_prefcore(cpudata); > + > ret = amd_pstate_init_freq(cpudata); > if (ret) > goto free_cpudata1; > @@ -1453,12 +1439,12 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy) > cpudata->cpu = policy->cpu; > cpudata->epp_policy = 0; > > - amd_pstate_init_prefcore(cpudata); > - > ret = amd_pstate_init_perf(cpudata); > if (ret) > goto free_cpudata1; > > + amd_pstate_init_prefcore(cpudata); > + > ret = amd_pstate_init_freq(cpudata); > if (ret) > goto free_cpudata1; > @@ -1903,6 +1889,12 @@ static int __init amd_pstate_init(void) > static_call_update(amd_pstate_update_perf, cppc_update_perf); > } > > + if (amd_pstate_prefcore) { > + ret = amd_detect_prefcore(&amd_pstate_prefcore); > + if (ret) > + return ret; > + } > + > /* enable amd pstate feature */ > ret = amd_pstate_enable(true); > if (ret) { > diff --git a/include/acpi/cppc_acpi.h b/include/acpi/cppc_acpi.h > index 2246ce0630362..1d79320a23490 100644 > --- a/include/acpi/cppc_acpi.h > +++ b/include/acpi/cppc_acpi.h > @@ -137,10 +137,12 @@ struct cppc_cpudata { > }; > > #ifdef CONFIG_CPU_SUP_AMD > +extern int amd_detect_prefcore(bool *detected); > extern int amd_get_highest_perf(unsigned int cpu, u32 *highest_perf); > extern int amd_get_boost_ratio_numerator(unsigned int cpu, u64 *numerator); > #else /* !CONFIG_CPU_SUP_AMD */ > static inline int amd_get_highest_perf(unsigned int cpu, u32 *highest_perf) { return -ENODEV; } > +static inline int amd_detect_prefcore(bool *detected) { return -ENODEV; } > static inline int amd_get_boost_ratio_numerator(unsigned int cpu, u64 *numerator) { return -ENODEV; } > #endif /* !CONFIG_CPU_SUP_AMD */ > > -- > 2.43.0 >
On 8/27/2024 10:43, Gautham R. Shenoy wrote: > > On Mon, Aug 26, 2024 at 04:13:55PM -0500, Mario Limonciello wrote: >> From: Mario Limonciello <mario.limonciello@amd.com> >> >> AMD systems that support preferred cores will use "166" as their >> numerator for max frequency calculations instead of "255". >> >> Add a function for detecting preferred cores by looking at the >> highest perf value on all cores. >> >> If preferred cores are enabled return 166 and if disabled the >> value in the highest perf register. As the function will be called >> multiple times, cache the values for the boost numerator and if >> preferred cores will be enabled in global variables. >> >> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> >> --- > > [..snip..] > >> /** >> * amd_get_boost_ratio_numerator: Get the numerator to use for boost ratio calculation >> * @cpu: CPU to get numerator for. >> @@ -162,20 +232,19 @@ EXPORT_SYMBOL_GPL(amd_get_highest_perf); >> */ >> int amd_get_boost_ratio_numerator(unsigned int cpu, u64 *numerator) >> { >> - struct cpuinfo_x86 *c = &boot_cpu_data; >> + bool prefcore; >> + int ret; >> >> - if (c->x86 == 0x17 && ((c->x86_model >= 0x30 && c->x86_model < 0x40) || >> - (c->x86_model >= 0x70 && c->x86_model < 0x80))) { >> - *numerator = 166; >> - return 0; >> - } >> + ret = amd_detect_prefcore(&prefcore); >> + if (ret) >> + return ret; >> >> - if (c->x86 == 0x19 && ((c->x86_model >= 0x20 && c->x86_model < 0x30) || >> - (c->x86_model >= 0x40 && c->x86_model < 0x70))) { >> - *numerator = 166; >> + /* without preferred cores, return the highest perf register value */ >> + if (!prefcore) { >> + *numerator = boost_numerator; >> return 0; >> } >> - *numerator = 255; >> + *numerator = CPPC_HIGHEST_PERF_PREFCORE; > > > Interesting. So even when the user boots a system that supports > preferred-cores with "amd_preferred=disable", > amd_get_boost_ratio_numerator() will return CPPC_HIGHEST_PERF_PREFCORE > as the call prefcore == true here. > > I suppose that is as intended, since even though the user may not want > to use the preferred core logic for task-scheduling/load-balancing, > the numerator for the boost-ratio is purely dependent on the h/w > capability. > > Is this understanding correct? If so, can this be included as a > comment in the code ? > Yup, you got it all right. I'll fold some of this into the function comment for v2. > The rest of the patch looks good to me. > > Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com> > > -- > Thanks and Regards > gautham. > > > >> >> return 0; >> } >> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c >> index f470b5700db58..ec32c830abc1d 100644 >> --- a/drivers/cpufreq/amd-pstate.c >> +++ b/drivers/cpufreq/amd-pstate.c >> @@ -807,32 +807,18 @@ static DECLARE_WORK(sched_prefcore_work, amd_pstste_sched_prefcore_workfn); >> >> static void amd_pstate_init_prefcore(struct amd_cpudata *cpudata) >> { >> - int ret, prio; >> - u32 highest_perf; >> - >> - ret = amd_get_highest_perf(cpudata->cpu, &highest_perf); >> - if (ret) >> + /* user disabled or not detected */ >> + if (!amd_pstate_prefcore) >> return; >> >> cpudata->hw_prefcore = true; >> - /* check if CPPC preferred core feature is enabled*/ >> - if (highest_perf < CPPC_MAX_PERF) >> - prio = (int)highest_perf; >> - else { >> - pr_debug("AMD CPPC preferred core is unsupported!\n"); >> - cpudata->hw_prefcore = false; >> - return; >> - } >> - >> - if (!amd_pstate_prefcore) >> - return; >> >> /* >> * The priorities can be set regardless of whether or not >> * sched_set_itmt_support(true) has been called and it is valid to >> * update them at any time after it has been called. >> */ >> - sched_set_itmt_core_prio(prio, cpudata->cpu); >> + sched_set_itmt_core_prio((int)READ_ONCE(cpudata->highest_perf), cpudata->cpu); >> >> schedule_work(&sched_prefcore_work); >> } >> @@ -998,12 +984,12 @@ static int amd_pstate_cpu_init(struct cpufreq_policy *policy) >> >> cpudata->cpu = policy->cpu; >> >> - amd_pstate_init_prefcore(cpudata); >> - >> ret = amd_pstate_init_perf(cpudata); >> if (ret) >> goto free_cpudata1; >> >> + amd_pstate_init_prefcore(cpudata); >> + >> ret = amd_pstate_init_freq(cpudata); >> if (ret) >> goto free_cpudata1; >> @@ -1453,12 +1439,12 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy) >> cpudata->cpu = policy->cpu; >> cpudata->epp_policy = 0; >> >> - amd_pstate_init_prefcore(cpudata); >> - >> ret = amd_pstate_init_perf(cpudata); >> if (ret) >> goto free_cpudata1; >> >> + amd_pstate_init_prefcore(cpudata); >> + >> ret = amd_pstate_init_freq(cpudata); >> if (ret) >> goto free_cpudata1; >> @@ -1903,6 +1889,12 @@ static int __init amd_pstate_init(void) >> static_call_update(amd_pstate_update_perf, cppc_update_perf); >> } >> >> + if (amd_pstate_prefcore) { >> + ret = amd_detect_prefcore(&amd_pstate_prefcore); >> + if (ret) >> + return ret; >> + } >> + >> /* enable amd pstate feature */ >> ret = amd_pstate_enable(true); >> if (ret) { >> diff --git a/include/acpi/cppc_acpi.h b/include/acpi/cppc_acpi.h >> index 2246ce0630362..1d79320a23490 100644 >> --- a/include/acpi/cppc_acpi.h >> +++ b/include/acpi/cppc_acpi.h >> @@ -137,10 +137,12 @@ struct cppc_cpudata { >> }; >> >> #ifdef CONFIG_CPU_SUP_AMD >> +extern int amd_detect_prefcore(bool *detected); >> extern int amd_get_highest_perf(unsigned int cpu, u32 *highest_perf); >> extern int amd_get_boost_ratio_numerator(unsigned int cpu, u64 *numerator); >> #else /* !CONFIG_CPU_SUP_AMD */ >> static inline int amd_get_highest_perf(unsigned int cpu, u32 *highest_perf) { return -ENODEV; } >> +static inline int amd_detect_prefcore(bool *detected) { return -ENODEV; } >> static inline int amd_get_boost_ratio_numerator(unsigned int cpu, u64 *numerator) { return -ENODEV; } >> #endif /* !CONFIG_CPU_SUP_AMD */ >> >> -- >> 2.43.0 >>
diff --git a/arch/x86/kernel/acpi/cppc.c b/arch/x86/kernel/acpi/cppc.c index 5a6c01a1b0d95..729b35e84f5eb 100644 --- a/arch/x86/kernel/acpi/cppc.c +++ b/arch/x86/kernel/acpi/cppc.c @@ -9,6 +9,16 @@ #include <asm/processor.h> #include <asm/topology.h> +#define CPPC_HIGHEST_PERF_PREFCORE 166 + +enum amd_pref_core { + AMD_PREF_CORE_UNKNOWN = 0, + AMD_PREF_CORE_SUPPORTED, + AMD_PREF_CORE_UNSUPPORTED, +}; +static enum amd_pref_core amd_pref_core_detected; +static u64 boost_numerator; + /* Refer to drivers/acpi/cppc_acpi.c for the description of functions */ bool cpc_supported_by_cpu(void) @@ -149,6 +159,66 @@ int amd_get_highest_perf(unsigned int cpu, u32 *highest_perf) } EXPORT_SYMBOL_GPL(amd_get_highest_perf); +/** + * amd_detect_prefcore: Detect if CPUs in the system support preferred cores + * @detected: Output variable for the result of the detection. + * + * Determine whether CPUs in the system support preferred cores. On systems + * that support preferred cores, different highest perf values will be found + * on different cores. On other systems, the highest perf value will be the + * same on all cores. + * + * The result of the detection will be stored in the 'detected' parameter. + * + * Return: 0 for success, negative error code otherwise + */ +int amd_detect_prefcore(bool *detected) +{ + int cpu, count = 0; + u64 highest_perf[2] = {0}; + + if (WARN_ON(!detected)) + return -EINVAL; + + switch (amd_pref_core_detected) { + case AMD_PREF_CORE_SUPPORTED: + *detected = true; + return 0; + case AMD_PREF_CORE_UNSUPPORTED: + *detected = false; + return 0; + default: + break; + } + + for_each_present_cpu(cpu) { + u32 tmp; + int ret; + + ret = amd_get_highest_perf(cpu, &tmp); + if (ret) + return ret; + + if (!count || (count == 1 && tmp != highest_perf[0])) + highest_perf[count++] = tmp; + + if (count == 2) + break; + } + + *detected = (count == 2); + boost_numerator = highest_perf[0]; + + amd_pref_core_detected = *detected ? AMD_PREF_CORE_SUPPORTED : + AMD_PREF_CORE_UNSUPPORTED; + + pr_debug("AMD CPPC preferred core is %ssupported (highest perf: 0x%llx)\n", + *detected ? "" : "un", highest_perf[0]); + + return 0; +} +EXPORT_SYMBOL_GPL(amd_detect_prefcore); + /** * amd_get_boost_ratio_numerator: Get the numerator to use for boost ratio calculation * @cpu: CPU to get numerator for. @@ -162,20 +232,19 @@ EXPORT_SYMBOL_GPL(amd_get_highest_perf); */ int amd_get_boost_ratio_numerator(unsigned int cpu, u64 *numerator) { - struct cpuinfo_x86 *c = &boot_cpu_data; + bool prefcore; + int ret; - if (c->x86 == 0x17 && ((c->x86_model >= 0x30 && c->x86_model < 0x40) || - (c->x86_model >= 0x70 && c->x86_model < 0x80))) { - *numerator = 166; - return 0; - } + ret = amd_detect_prefcore(&prefcore); + if (ret) + return ret; - if (c->x86 == 0x19 && ((c->x86_model >= 0x20 && c->x86_model < 0x30) || - (c->x86_model >= 0x40 && c->x86_model < 0x70))) { - *numerator = 166; + /* without preferred cores, return the highest perf register value */ + if (!prefcore) { + *numerator = boost_numerator; return 0; } - *numerator = 255; + *numerator = CPPC_HIGHEST_PERF_PREFCORE; return 0; } diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c index f470b5700db58..ec32c830abc1d 100644 --- a/drivers/cpufreq/amd-pstate.c +++ b/drivers/cpufreq/amd-pstate.c @@ -807,32 +807,18 @@ static DECLARE_WORK(sched_prefcore_work, amd_pstste_sched_prefcore_workfn); static void amd_pstate_init_prefcore(struct amd_cpudata *cpudata) { - int ret, prio; - u32 highest_perf; - - ret = amd_get_highest_perf(cpudata->cpu, &highest_perf); - if (ret) + /* user disabled or not detected */ + if (!amd_pstate_prefcore) return; cpudata->hw_prefcore = true; - /* check if CPPC preferred core feature is enabled*/ - if (highest_perf < CPPC_MAX_PERF) - prio = (int)highest_perf; - else { - pr_debug("AMD CPPC preferred core is unsupported!\n"); - cpudata->hw_prefcore = false; - return; - } - - if (!amd_pstate_prefcore) - return; /* * The priorities can be set regardless of whether or not * sched_set_itmt_support(true) has been called and it is valid to * update them at any time after it has been called. */ - sched_set_itmt_core_prio(prio, cpudata->cpu); + sched_set_itmt_core_prio((int)READ_ONCE(cpudata->highest_perf), cpudata->cpu); schedule_work(&sched_prefcore_work); } @@ -998,12 +984,12 @@ static int amd_pstate_cpu_init(struct cpufreq_policy *policy) cpudata->cpu = policy->cpu; - amd_pstate_init_prefcore(cpudata); - ret = amd_pstate_init_perf(cpudata); if (ret) goto free_cpudata1; + amd_pstate_init_prefcore(cpudata); + ret = amd_pstate_init_freq(cpudata); if (ret) goto free_cpudata1; @@ -1453,12 +1439,12 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy) cpudata->cpu = policy->cpu; cpudata->epp_policy = 0; - amd_pstate_init_prefcore(cpudata); - ret = amd_pstate_init_perf(cpudata); if (ret) goto free_cpudata1; + amd_pstate_init_prefcore(cpudata); + ret = amd_pstate_init_freq(cpudata); if (ret) goto free_cpudata1; @@ -1903,6 +1889,12 @@ static int __init amd_pstate_init(void) static_call_update(amd_pstate_update_perf, cppc_update_perf); } + if (amd_pstate_prefcore) { + ret = amd_detect_prefcore(&amd_pstate_prefcore); + if (ret) + return ret; + } + /* enable amd pstate feature */ ret = amd_pstate_enable(true); if (ret) { diff --git a/include/acpi/cppc_acpi.h b/include/acpi/cppc_acpi.h index 2246ce0630362..1d79320a23490 100644 --- a/include/acpi/cppc_acpi.h +++ b/include/acpi/cppc_acpi.h @@ -137,10 +137,12 @@ struct cppc_cpudata { }; #ifdef CONFIG_CPU_SUP_AMD +extern int amd_detect_prefcore(bool *detected); extern int amd_get_highest_perf(unsigned int cpu, u32 *highest_perf); extern int amd_get_boost_ratio_numerator(unsigned int cpu, u64 *numerator); #else /* !CONFIG_CPU_SUP_AMD */ static inline int amd_get_highest_perf(unsigned int cpu, u32 *highest_perf) { return -ENODEV; } +static inline int amd_detect_prefcore(bool *detected) { return -ENODEV; } static inline int amd_get_boost_ratio_numerator(unsigned int cpu, u64 *numerator) { return -ENODEV; } #endif /* !CONFIG_CPU_SUP_AMD */