| Message ID | 20250113044107.566-1-gautham.shenoy@amd.com (mailing list archive) |
|---|---|
| State | Queued |
| Delegated to: | Rafael Wysocki |
| Headers | show |
| Series | acpi-cpufreq: Fix max-frequency computation | expand |
On 1/12/2025 22:41, Gautham R. Shenoy wrote: > commit 3c55e94c0ade ("cpufreq: ACPI: Extend frequency tables to cover > boost frequencies") introduces an assumption in > acpi_cpufreq_cpu_init() that the first entry in the P-state table is > the nominal frequency. This assumption is incorrect. The frequency > corresponding to the P0 P-State need not be the same as the nominal > frequency advertised via CPPC. > > Since the driver is using the CPPC.highest_perf and CPPC.nominal_perf > to compute the boost-ratio, it makes sense to use CPPC.nominal_freq to > compute the max-frequency. CPPC.nominal_freq is advertised on > platforms supporting CPPC revisions 3 or higher. > > Hence, fallback to using the first entry in the P-State table only on > platforms that do not advertise CPPC.nominal_freq. > > Fixes: 3c55e94c0ade ("cpufreq: ACPI: Extend frequency tables to cover boost frequencies") > Tested-by: Dhananjay Ugwekar <Dhananjay.Ugwekar@amd.com> > Signed-off-by: Gautham R. Shenoy <gautham.shenoy@amd.com> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com> > --- > drivers/cpufreq/acpi-cpufreq.c | 36 +++++++++++++++++++++++++--------- > 1 file changed, 27 insertions(+), 9 deletions(-) > > diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c > index c9ebacf5c88e..744fcdeab173 100644 > --- a/drivers/cpufreq/acpi-cpufreq.c > +++ b/drivers/cpufreq/acpi-cpufreq.c > @@ -623,7 +623,14 @@ static int acpi_cpufreq_blacklist(struct cpuinfo_x86 *c) > #endif > > #ifdef CONFIG_ACPI_CPPC_LIB > -static u64 get_max_boost_ratio(unsigned int cpu) > +/* > + * get_max_boost_ratio: Computes the max_boost_ratio as the ratio > + * between the highest_perf and the nominal_perf. > + * > + * Returns the max_boost_ratio for @cpu. Returns the CPPC nominal > + * frequency via @nominal_freq if it is non-NULL pointer. > + */ > +static u64 get_max_boost_ratio(unsigned int cpu, u64 *nominal_freq) > { > struct cppc_perf_caps perf_caps; > u64 highest_perf, nominal_perf; > @@ -652,6 +659,9 @@ static u64 get_max_boost_ratio(unsigned int cpu) > > nominal_perf = perf_caps.nominal_perf; > > + if (nominal_freq) > + *nominal_freq = perf_caps.nominal_freq; > + > if (!highest_perf || !nominal_perf) { > pr_debug("CPU%d: highest or nominal performance missing\n", cpu); > return 0; > @@ -664,8 +674,12 @@ static u64 get_max_boost_ratio(unsigned int cpu) > > return div_u64(highest_perf << SCHED_CAPACITY_SHIFT, nominal_perf); > } > + > #else > -static inline u64 get_max_boost_ratio(unsigned int cpu) { return 0; } > +static inline u64 get_max_boost_ratio(unsigned int cpu, u64 *nominal_freq) > +{ > + return 0; > +} > #endif > > static int acpi_cpufreq_cpu_init(struct cpufreq_policy *policy) > @@ -677,7 +691,7 @@ static int acpi_cpufreq_cpu_init(struct cpufreq_policy *policy) > struct cpuinfo_x86 *c = &cpu_data(cpu); > unsigned int valid_states = 0; > unsigned int result = 0; > - u64 max_boost_ratio; > + u64 max_boost_ratio, nominal_freq = 0; > unsigned int i; > #ifdef CONFIG_SMP > static int blacklisted; > @@ -827,16 +841,20 @@ static int acpi_cpufreq_cpu_init(struct cpufreq_policy *policy) > } > freq_table[valid_states].frequency = CPUFREQ_TABLE_END; > > - max_boost_ratio = get_max_boost_ratio(cpu); > + max_boost_ratio = get_max_boost_ratio(cpu, &nominal_freq); > if (max_boost_ratio) { > - unsigned int freq = freq_table[0].frequency; > + unsigned int freq = nominal_freq; > > /* > - * Because the loop above sorts the freq_table entries in the > - * descending order, freq is the maximum frequency in the table. > - * Assume that it corresponds to the CPPC nominal frequency and > - * use it to set cpuinfo.max_freq. > + * The loop above sorts the freq_table entries in the > + * descending order. If ACPI CPPC has not advertised > + * the nominal frequency (this is possible in CPPC > + * revisions prior to 3), then use the first entry in > + * the pstate table as a proxy for nominal frequency. > */ > + if (!freq) > + freq = freq_table[0].frequency; > + > policy->cpuinfo.max_freq = freq * max_boost_ratio >> SCHED_CAPACITY_SHIFT; > } else { > /*
On Mon, Jan 13, 2025 at 3:23 PM Mario Limonciello <mario.limonciello@amd.com> wrote: > > On 1/12/2025 22:41, Gautham R. Shenoy wrote: > > commit 3c55e94c0ade ("cpufreq: ACPI: Extend frequency tables to cover > > boost frequencies") introduces an assumption in > > acpi_cpufreq_cpu_init() that the first entry in the P-state table is > > the nominal frequency. This assumption is incorrect. The frequency > > corresponding to the P0 P-State need not be the same as the nominal > > frequency advertised via CPPC. > > > > Since the driver is using the CPPC.highest_perf and CPPC.nominal_perf > > to compute the boost-ratio, it makes sense to use CPPC.nominal_freq to > > compute the max-frequency. CPPC.nominal_freq is advertised on > > platforms supporting CPPC revisions 3 or higher. > > > > Hence, fallback to using the first entry in the P-State table only on > > platforms that do not advertise CPPC.nominal_freq. > > > > Fixes: 3c55e94c0ade ("cpufreq: ACPI: Extend frequency tables to cover boost frequencies") > > Tested-by: Dhananjay Ugwekar <Dhananjay.Ugwekar@amd.com> > > Signed-off-by: Gautham R. Shenoy <gautham.shenoy@amd.com> > > Reviewed-by: Mario Limonciello <mario.limonciello@amd.com> Applied as 6.14 material, thanks!
diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c index c9ebacf5c88e..744fcdeab173 100644 --- a/drivers/cpufreq/acpi-cpufreq.c +++ b/drivers/cpufreq/acpi-cpufreq.c @@ -623,7 +623,14 @@ static int acpi_cpufreq_blacklist(struct cpuinfo_x86 *c) #endif #ifdef CONFIG_ACPI_CPPC_LIB -static u64 get_max_boost_ratio(unsigned int cpu) +/* + * get_max_boost_ratio: Computes the max_boost_ratio as the ratio + * between the highest_perf and the nominal_perf. + * + * Returns the max_boost_ratio for @cpu. Returns the CPPC nominal + * frequency via @nominal_freq if it is non-NULL pointer. + */ +static u64 get_max_boost_ratio(unsigned int cpu, u64 *nominal_freq) { struct cppc_perf_caps perf_caps; u64 highest_perf, nominal_perf; @@ -652,6 +659,9 @@ static u64 get_max_boost_ratio(unsigned int cpu) nominal_perf = perf_caps.nominal_perf; + if (nominal_freq) + *nominal_freq = perf_caps.nominal_freq; + if (!highest_perf || !nominal_perf) { pr_debug("CPU%d: highest or nominal performance missing\n", cpu); return 0; @@ -664,8 +674,12 @@ static u64 get_max_boost_ratio(unsigned int cpu) return div_u64(highest_perf << SCHED_CAPACITY_SHIFT, nominal_perf); } + #else -static inline u64 get_max_boost_ratio(unsigned int cpu) { return 0; } +static inline u64 get_max_boost_ratio(unsigned int cpu, u64 *nominal_freq) +{ + return 0; +} #endif static int acpi_cpufreq_cpu_init(struct cpufreq_policy *policy) @@ -677,7 +691,7 @@ static int acpi_cpufreq_cpu_init(struct cpufreq_policy *policy) struct cpuinfo_x86 *c = &cpu_data(cpu); unsigned int valid_states = 0; unsigned int result = 0; - u64 max_boost_ratio; + u64 max_boost_ratio, nominal_freq = 0; unsigned int i; #ifdef CONFIG_SMP static int blacklisted; @@ -827,16 +841,20 @@ static int acpi_cpufreq_cpu_init(struct cpufreq_policy *policy) } freq_table[valid_states].frequency = CPUFREQ_TABLE_END; - max_boost_ratio = get_max_boost_ratio(cpu); + max_boost_ratio = get_max_boost_ratio(cpu, &nominal_freq); if (max_boost_ratio) { - unsigned int freq = freq_table[0].frequency; + unsigned int freq = nominal_freq; /* - * Because the loop above sorts the freq_table entries in the - * descending order, freq is the maximum frequency in the table. - * Assume that it corresponds to the CPPC nominal frequency and - * use it to set cpuinfo.max_freq. + * The loop above sorts the freq_table entries in the + * descending order. If ACPI CPPC has not advertised + * the nominal frequency (this is possible in CPPC + * revisions prior to 3), then use the first entry in + * the pstate table as a proxy for nominal frequency. */ + if (!freq) + freq = freq_table[0].frequency; + policy->cpuinfo.max_freq = freq * max_boost_ratio >> SCHED_CAPACITY_SHIFT; } else { /*
commit 3c55e94c0ade ("cpufreq: ACPI: Extend frequency tables to cover boost frequencies") introduces an assumption in acpi_cpufreq_cpu_init() that the first entry in the P-state table is the nominal frequency. This assumption is incorrect. The frequency corresponding to the P0 P-State need not be the same as the nominal frequency advertised via CPPC. Since the driver is using the CPPC.highest_perf and CPPC.nominal_perf to compute the boost-ratio, it makes sense to use CPPC.nominal_freq to compute the max-frequency. CPPC.nominal_freq is advertised on platforms supporting CPPC revisions 3 or higher. Hence, fallback to using the first entry in the P-State table only on platforms that do not advertise CPPC.nominal_freq. Fixes: 3c55e94c0ade ("cpufreq: ACPI: Extend frequency tables to cover boost frequencies") Tested-by: Dhananjay Ugwekar <Dhananjay.Ugwekar@amd.com> Signed-off-by: Gautham R. Shenoy <gautham.shenoy@amd.com> --- drivers/cpufreq/acpi-cpufreq.c | 36 +++++++++++++++++++++++++--------- 1 file changed, 27 insertions(+), 9 deletions(-)