On Tue, Sep 03, 2024 at 03:36:53PM -0500, Mario Limonciello wrote:
> From: Mario Limonciello <mario.limonciello@xxxxxxx>
>
> The function name is ambiguous because it returns an intermediate value
> for calculating maximum frequency rather than the CPPC 'Highest Perf'
> register.
>
> Rename the function to clarify its use and allow the function to return
> errors. Adjust the consumer in acpi-cpufreq to catch errors.
>
> Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
LGTM
Reviewed-by: Gautham R. Shenoy <gautham.shenoy@xxxxxxx>
--
Thanks and Regards
gautham.
> ---
> v1->v2:
> * Rename variable from `highest_perf` to `numerator`
> * Fail when unable to return boost numerator
> * Move prototype behind CONFIG_ACPI_CPPC_LIB (lkp robot)
> ---
> arch/x86/include/asm/processor.h | 3 ---
> arch/x86/kernel/acpi/cppc.c | 44 +++++++++++++++++++++++---------
> drivers/cpufreq/acpi-cpufreq.c | 12 ++++++---
> include/acpi/cppc_acpi.h | 5 ++++
> 4 files changed, 46 insertions(+), 18 deletions(-)
>
> diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
> index a75a07f4931fd..775acbdea1a96 100644
> --- a/arch/x86/include/asm/processor.h
> +++ b/arch/x86/include/asm/processor.h
> @@ -691,8 +691,6 @@ static inline u32 per_cpu_l2c_id(unsigned int cpu)
> }
>
> #ifdef CONFIG_CPU_SUP_AMD
> -extern u32 amd_get_highest_perf(void);
> -
> /*
> * Issue a DIV 0/1 insn to clear any division data from previous DIV
> * operations.
> @@ -705,7 +703,6 @@ static __always_inline void amd_clear_divider(void)
>
> extern void amd_check_microcode(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) { }
> #endif
> diff --git a/arch/x86/kernel/acpi/cppc.c b/arch/x86/kernel/acpi/cppc.c
> index 7ec8f2ce859c8..660cfeb6384ba 100644
> --- a/arch/x86/kernel/acpi/cppc.c
> +++ b/arch/x86/kernel/acpi/cppc.c
> @@ -69,7 +69,7 @@ int cpc_write_ffh(int cpunum, struct cpc_reg *reg, u64 val)
> static void amd_set_max_freq_ratio(void)
> {
> struct cppc_perf_caps perf_caps;
> - u64 highest_perf, nominal_perf;
> + u64 numerator, nominal_perf;
> u64 perf_ratio;
> int rc;
>
> @@ -79,15 +79,19 @@ static void amd_set_max_freq_ratio(void)
> return;
> }
>
> - highest_perf = amd_get_highest_perf();
> + rc = amd_get_boost_ratio_numerator(0, &numerator);
> + if (rc) {
> + pr_debug("Could not retrieve highest performance (%d)\n", rc);
> + return;
> + }
> nominal_perf = perf_caps.nominal_perf;
>
> - if (!highest_perf || !nominal_perf) {
> - pr_debug("Could not retrieve highest or nominal performance\n");
> + if (!nominal_perf) {
> + pr_debug("Could not retrieve nominal performance\n");
> return;
> }
>
> - perf_ratio = div_u64(highest_perf * SCHED_CAPACITY_SCALE, nominal_perf);
> + perf_ratio = div_u64(numerator * SCHED_CAPACITY_SCALE, nominal_perf);
> /* midpoint between max_boost and max_P */
> perf_ratio = (perf_ratio + SCHED_CAPACITY_SCALE) >> 1;
> if (!perf_ratio) {
> @@ -117,18 +121,34 @@ void init_freq_invariance_cppc(void)
> mutex_unlock(&freq_invariance_lock);
> }
>
> -u32 amd_get_highest_perf(void)
> +/**
> + * amd_get_boost_ratio_numerator: Get the numerator to use for boost ratio calculation
> + * @cpu: CPU to get numerator for.
> + * @numerator: Output variable for numerator.
> + *
> + * Determine the numerator to use for calculating the boost ratio on
> + * a CPU. On systems that support preferred cores, this will be a hardcoded
> + * value. On other systems this will the highest performance register value.
> + *
> + * Return: 0 for success, negative error code otherwise.
> + */
> +int amd_get_boost_ratio_numerator(unsigned int cpu, u64 *numerator)
> {
> struct cpuinfo_x86 *c = &boot_cpu_data;
>
> if (c->x86 == 0x17 && ((c->x86_model >= 0x30 && c->x86_model < 0x40) ||
> - (c->x86_model >= 0x70 && c->x86_model < 0x80)))
> - return 166;
> + (c->x86_model >= 0x70 && c->x86_model < 0x80))) {
> + *numerator = 166;
> + return 0;
> + }
>
> if (c->x86 == 0x19 && ((c->x86_model >= 0x20 && c->x86_model < 0x30) ||
> - (c->x86_model >= 0x40 && c->x86_model < 0x70)))
> - return 166;
> + (c->x86_model >= 0x40 && c->x86_model < 0x70))) {
> + *numerator = 166;
> + return 0;
> + }
> + *numerator = 255;
>
> - return 255;
> + return 0;
> }
> -EXPORT_SYMBOL_GPL(amd_get_highest_perf);
> +EXPORT_SYMBOL_GPL(amd_get_boost_ratio_numerator);
> diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
> index a8ca625a98b89..0f04feb6cafaf 100644
> --- a/drivers/cpufreq/acpi-cpufreq.c
> +++ b/drivers/cpufreq/acpi-cpufreq.c
> @@ -642,10 +642,16 @@ static u64 get_max_boost_ratio(unsigned int cpu)
> return 0;
> }
>
> - if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD)
> - highest_perf = amd_get_highest_perf();
> - else
> + if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD) {
> + ret = amd_get_boost_ratio_numerator(cpu, &highest_perf);
> + if (ret) {
> + pr_debug("CPU%d: Unable to get boost ratio numerator (%d)\n",
> + cpu, ret);
> + return 0;
> + }
> + } else {
> highest_perf = perf_caps.highest_perf;
> + }
>
> nominal_perf = perf_caps.nominal_perf;
>
> diff --git a/include/acpi/cppc_acpi.h b/include/acpi/cppc_acpi.h
> index 409a7190a4a86..97861abc5f5b8 100644
> --- a/include/acpi/cppc_acpi.h
> +++ b/include/acpi/cppc_acpi.h
> @@ -159,6 +159,7 @@ extern int cppc_get_epp_perf(int cpunum, u64 *epp_perf);
> extern int cppc_set_epp_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls, bool enable);
> extern int cppc_get_auto_sel_caps(int cpunum, struct cppc_perf_caps *perf_caps);
> extern int cppc_set_auto_sel(int cpu, bool enable);
> +extern int amd_get_boost_ratio_numerator(unsigned int cpu, u64 *numerator);
> #else /* !CONFIG_ACPI_CPPC_LIB */
> static inline int cppc_get_desired_perf(int cpunum, u64 *desired_perf)
> {
> @@ -232,6 +233,10 @@ static inline int cppc_get_auto_sel_caps(int cpunum, struct cppc_perf_caps *perf
> {
> return -EOPNOTSUPP;
> }
> +static inline int amd_get_boost_ratio_numerator(unsigned int cpu, u64 *numerator)
> +{
> + return -EOPNOTSUPP;
> +}
> #endif /* !CONFIG_ACPI_CPPC_LIB */
>
> #endif /* _CPPC_ACPI_H*/
> --
> 2.43.0
>
