* [PATCH v1 0/4] cpufreq: Fixes and cleanups related to CPUFREQ_ETERNAL
@ 2025-09-25 15:42 Rafael J. Wysocki
2025-09-25 15:44 ` [PATCH v1 1/4] cpufreq: Make drivers using CPUFREQ_ETERNAL specify transition latency Rafael J. Wysocki
` (4 more replies)
0 siblings, 5 replies; 18+ messages in thread
From: Rafael J. Wysocki @ 2025-09-25 15:42 UTC (permalink / raw)
To: Linux PM
Cc: Shawn Guo, Qais Yousef, LKML, Viresh Kumar, Prashanth Prakash,
Pierre Gondois, Mario Limonciello, Linux ACPI
Hi All,
The first patch in this series is meant to address the failure discussed in
https://lore.kernel.org/linux-pm/20250922125929.453444-1-shawnguo2@yeah.net/
but in a different way than proposed by Shawn.
The second one is a CPPC cpufreq driver fix preventing it from using an
overly large transition delay in the cases when that delay cannot be
obtained from the platform firmware.
Patch [3/4] makes CPPC use a specific symbol instead of CPUFREQ_ETERNAL for
signaling error conditions while attempting to retrieve a transition latency
value from the platform firmware.
The last patch removes CPUFREQ_ETERNAL (which has no users any more) from
cpufreq, including all references to it in cpufreq documentation.
Thanks!
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v1 1/4] cpufreq: Make drivers using CPUFREQ_ETERNAL specify transition latency
2025-09-25 15:42 [PATCH v1 0/4] cpufreq: Fixes and cleanups related to CPUFREQ_ETERNAL Rafael J. Wysocki
@ 2025-09-25 15:44 ` Rafael J. Wysocki
2025-09-25 16:37 ` Mario Limonciello
2025-09-26 9:46 ` Jie Zhan
2025-09-25 15:44 ` [PATCH v1 2/4] cpufreq: CPPC: Avoid using CPUFREQ_ETERNAL as transition delay Rafael J. Wysocki
` (3 subsequent siblings)
4 siblings, 2 replies; 18+ messages in thread
From: Rafael J. Wysocki @ 2025-09-25 15:44 UTC (permalink / raw)
To: Linux PM
Cc: Shawn Guo, Qais Yousef, LKML, Viresh Kumar, Prashanth Prakash,
Pierre Gondois, Mario Limonciello, Linux ACPI
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Commit a755d0e2d41b ("cpufreq: Honour transition_latency over
transition_delay_us") caused platforms where cpuinfo.transition_latency
is CPUFREQ_ETERNAL to get a very large transition latency whereas
previously it had been capped at 10 ms (and later at 2 ms).
This led to a user-observable regression between 6.6 and 6.12 as
described by Shawn:
"The dbs sampling_rate was 10000 us on 6.6 and suddently becomes
6442450 us (4294967295 / 1000 * 1.5) on 6.12 for these platforms
because the default transition delay was dropped [...].
It slows down dbs governor's reacting to CPU loading change
dramatically. Also, as transition_delay_us is used by schedutil
governor as rate_limit_us, it shows a negative impact on device
idle power consumption, because the device gets slightly less time
in the lowest OPP."
Evidently, the expectation of the drivers using CPUFREQ_ETERNAL as
cpuinfo.transition_latency was that it would be capped by the core,
but they may as well return a default transition latency value instead
of CPUFREQ_ETERNAL and the core need not do anything with it.
Accordingly, introduce CPUFREQ_DEFAULT_TRANSITION_LATENCY_NS and make
all of the drivers in question use it instead of CPUFREQ_ETERNAL.
Fixes: a755d0e2d41b ("cpufreq: Honour transition_latency over transition_delay_us")
Closes: https://lore.kernel.org/linux-pm/20250922125929.453444-1-shawnguo2@yeah.net/
Reported-by: Shawn Guo <shawnguo@kernel.org>
Cc: 6.6+ <stable@vger.kernel.org> # 6.6+
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/cpufreq/cpufreq-dt.c | 2 +-
drivers/cpufreq/imx6q-cpufreq.c | 2 +-
drivers/cpufreq/mediatek-cpufreq-hw.c | 2 +-
drivers/cpufreq/scmi-cpufreq.c | 2 +-
drivers/cpufreq/scpi-cpufreq.c | 2 +-
drivers/cpufreq/spear-cpufreq.c | 2 +-
include/linux/cpufreq.h | 3 +++
7 files changed, 9 insertions(+), 6 deletions(-)
--- a/drivers/cpufreq/cpufreq-dt.c
+++ b/drivers/cpufreq/cpufreq-dt.c
@@ -104,7 +104,7 @@ static int cpufreq_init(struct cpufreq_p
transition_latency = dev_pm_opp_get_max_transition_latency(cpu_dev);
if (!transition_latency)
- transition_latency = CPUFREQ_ETERNAL;
+ transition_latency = CPUFREQ_DEFAULT_TRANSITION_LATENCY_NS;
cpumask_copy(policy->cpus, priv->cpus);
policy->driver_data = priv;
--- a/drivers/cpufreq/imx6q-cpufreq.c
+++ b/drivers/cpufreq/imx6q-cpufreq.c
@@ -442,7 +442,7 @@ soc_opp_out:
}
if (of_property_read_u32(np, "clock-latency", &transition_latency))
- transition_latency = CPUFREQ_ETERNAL;
+ transition_latency = CPUFREQ_DEFAULT_TRANSITION_LATENCY_NS;
/*
* Calculate the ramp time for max voltage change in the
--- a/drivers/cpufreq/mediatek-cpufreq-hw.c
+++ b/drivers/cpufreq/mediatek-cpufreq-hw.c
@@ -309,7 +309,7 @@ static int mtk_cpufreq_hw_cpu_init(struc
latency = readl_relaxed(data->reg_bases[REG_FREQ_LATENCY]) * 1000;
if (!latency)
- latency = CPUFREQ_ETERNAL;
+ latency = CPUFREQ_DEFAULT_TRANSITION_LATENCY_NS;
policy->cpuinfo.transition_latency = latency;
policy->fast_switch_possible = true;
--- a/drivers/cpufreq/scmi-cpufreq.c
+++ b/drivers/cpufreq/scmi-cpufreq.c
@@ -294,7 +294,7 @@ static int scmi_cpufreq_init(struct cpuf
latency = perf_ops->transition_latency_get(ph, domain);
if (!latency)
- latency = CPUFREQ_ETERNAL;
+ latency = CPUFREQ_DEFAULT_TRANSITION_LATENCY_NS;
policy->cpuinfo.transition_latency = latency;
--- a/drivers/cpufreq/scpi-cpufreq.c
+++ b/drivers/cpufreq/scpi-cpufreq.c
@@ -157,7 +157,7 @@ static int scpi_cpufreq_init(struct cpuf
latency = scpi_ops->get_transition_latency(cpu_dev);
if (!latency)
- latency = CPUFREQ_ETERNAL;
+ latency = CPUFREQ_DEFAULT_TRANSITION_LATENCY_NS;
policy->cpuinfo.transition_latency = latency;
--- a/drivers/cpufreq/spear-cpufreq.c
+++ b/drivers/cpufreq/spear-cpufreq.c
@@ -182,7 +182,7 @@ static int spear_cpufreq_probe(struct pl
if (of_property_read_u32(np, "clock-latency",
&spear_cpufreq.transition_latency))
- spear_cpufreq.transition_latency = CPUFREQ_ETERNAL;
+ spear_cpufreq.transition_latency = CPUFREQ_DEFAULT_TRANSITION_LATENCY_NS;
cnt = of_property_count_u32_elems(np, "cpufreq_tbl");
if (cnt <= 0) {
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -32,6 +32,9 @@
*/
#define CPUFREQ_ETERNAL (-1)
+
+#define CPUFREQ_DEFAULT_TANSITION_LATENCY_NS NSEC_PER_MSEC
+
#define CPUFREQ_NAME_LEN 16
/* Print length for names. Extra 1 space for accommodating '\n' in prints */
#define CPUFREQ_NAME_PLEN (CPUFREQ_NAME_LEN + 1)
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v1 2/4] cpufreq: CPPC: Avoid using CPUFREQ_ETERNAL as transition delay
2025-09-25 15:42 [PATCH v1 0/4] cpufreq: Fixes and cleanups related to CPUFREQ_ETERNAL Rafael J. Wysocki
2025-09-25 15:44 ` [PATCH v1 1/4] cpufreq: Make drivers using CPUFREQ_ETERNAL specify transition latency Rafael J. Wysocki
@ 2025-09-25 15:44 ` Rafael J. Wysocki
2025-09-25 16:36 ` Mario Limonciello
2025-09-26 9:41 ` Jie Zhan
2025-09-25 15:46 ` [PATCH v1 3/4] ACPI: CPPC: Replace CPUFREQ_ETERNAL with CPPC-specific symbol Rafael J. Wysocki
` (2 subsequent siblings)
4 siblings, 2 replies; 18+ messages in thread
From: Rafael J. Wysocki @ 2025-09-25 15:44 UTC (permalink / raw)
To: Linux PM
Cc: Shawn Guo, Qais Yousef, LKML, Viresh Kumar, Prashanth Prakash,
Pierre Gondois, Mario Limonciello, Linux ACPI
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
If cppc_get_transition_latency() returns CPUFREQ_ETERNAL to indicate a
failure to retrieve the transition latency value from the platform
firmware, the CPPC cpufreq driver will use that value (converted to
microseconds) as the policy transition delay, but it is way too large
for any practical use.
Address this by making the driver use the cpufreq's default
transition latency value (in microseconds) as the transition delay
if CPUFREQ_ETERNAL is returned by cppc_get_transition_latency().
Fixes: d4f3388afd48 ("cpufreq / CPPC: Set platform specific transition_delay_us")
Cc: 5.19+ <stable@vger.kernel.org> # 5.19
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/cpufreq/cppc_cpufreq.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
--- a/drivers/cpufreq/cppc_cpufreq.c
+++ b/drivers/cpufreq/cppc_cpufreq.c
@@ -308,6 +308,16 @@ static int cppc_verify_policy(struct cpu
return 0;
}
+static unsigned int get_transition_latency_from_cppc(unsigned int cpu)
+{
+ unsigned int transition_latency_ns = cppc_get_transition_latency(cpu);
+
+ if (transition_latency_ns == CPUFREQ_ETERNAL)
+ return CPUFREQ_DEFAULT_TRANSITION_LATENCY_NS / NSEC_PER_USEC;
+
+ return transition_latency_ns / NSEC_PER_USEC;
+}
+
/*
* The PCC subspace describes the rate at which platform can accept commands
* on the shared PCC channel (including READs which do not count towards freq
@@ -330,12 +340,12 @@ static unsigned int cppc_cpufreq_get_tra
return 10000;
}
}
- return cppc_get_transition_latency(cpu) / NSEC_PER_USEC;
+ return get_transition_latency_from_cppc(cpu);
}
#else
static unsigned int cppc_cpufreq_get_transition_delay_us(unsigned int cpu)
{
- return cppc_get_transition_latency(cpu) / NSEC_PER_USEC;
+ return get_transition_latency_from_cppc(cpu);
}
#endif
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v1 3/4] ACPI: CPPC: Replace CPUFREQ_ETERNAL with CPPC-specific symbol
2025-09-25 15:42 [PATCH v1 0/4] cpufreq: Fixes and cleanups related to CPUFREQ_ETERNAL Rafael J. Wysocki
2025-09-25 15:44 ` [PATCH v1 1/4] cpufreq: Make drivers using CPUFREQ_ETERNAL specify transition latency Rafael J. Wysocki
2025-09-25 15:44 ` [PATCH v1 2/4] cpufreq: CPPC: Avoid using CPUFREQ_ETERNAL as transition delay Rafael J. Wysocki
@ 2025-09-25 15:46 ` Rafael J. Wysocki
2025-09-25 16:35 ` Mario Limonciello
2025-09-25 17:23 ` [PATCH v2 3/4] ACPI: CPPC: Do not use CPUFREQ_ETERNAL as an error value Rafael J. Wysocki
2025-09-25 15:47 ` [PATCH v1 4/4] cpufreq: Drop unused symbol CPUFREQ_ETERNAL Rafael J. Wysocki
2025-09-29 7:28 ` [PATCH v1 0/4] cpufreq: Fixes and cleanups related to CPUFREQ_ETERNAL Viresh Kumar
4 siblings, 2 replies; 18+ messages in thread
From: Rafael J. Wysocki @ 2025-09-25 15:46 UTC (permalink / raw)
To: Linux PM
Cc: Shawn Guo, Qais Yousef, LKML, Viresh Kumar, Prashanth Prakash,
Pierre Gondois, Mario Limonciello, Linux ACPI
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Instead of using CPUFREQ_ETERNAL for signaling error conditions in
cppc_get_transition_latency(), introduce CPPC_NO_DATA specifically
for this purpose and update all of the callers of this function to
use it.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/acpi/cppc_acpi.c | 6 +++---
drivers/cpufreq/amd-pstate.c | 4 ++--
drivers/cpufreq/cppc_cpufreq.c | 2 +-
include/acpi/cppc_acpi.h | 4 +++-
include/linux/cpufreq.h | 3 ---
5 files changed, 9 insertions(+), 10 deletions(-)
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -1897,16 +1897,16 @@ unsigned int cppc_get_transition_latency
cpc_desc = per_cpu(cpc_desc_ptr, cpu_num);
if (!cpc_desc)
- return CPUFREQ_ETERNAL;
+ return CPPC_NO_DATA;
desired_reg = &cpc_desc->cpc_regs[DESIRED_PERF];
if (CPC_IN_SYSTEM_MEMORY(desired_reg) || CPC_IN_SYSTEM_IO(desired_reg))
return 0;
else if (!CPC_IN_PCC(desired_reg))
- return CPUFREQ_ETERNAL;
+ return CPPC_NO_DATA;
if (pcc_ss_id < 0)
- return CPUFREQ_ETERNAL;
+ return CPPC_NO_DATA;
pcc_ss_data = pcc_data[pcc_ss_id];
if (pcc_ss_data->pcc_mpar)
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -875,7 +875,7 @@ static u32 amd_pstate_get_transition_del
u32 transition_delay_ns;
transition_delay_ns = cppc_get_transition_latency(cpu);
- if (transition_delay_ns == CPUFREQ_ETERNAL) {
+ if (transition_delay_ns == CPPC_NO_DATA) {
if (cpu_feature_enabled(X86_FEATURE_AMD_FAST_CPPC))
return AMD_PSTATE_FAST_CPPC_TRANSITION_DELAY;
else
@@ -894,7 +894,7 @@ static u32 amd_pstate_get_transition_lat
u32 transition_latency;
transition_latency = cppc_get_transition_latency(cpu);
- if (transition_latency == CPUFREQ_ETERNAL)
+ if (transition_latency == CPPC_NO_DATA)
return AMD_PSTATE_TRANSITION_LATENCY;
return transition_latency;
--- a/drivers/cpufreq/cppc_cpufreq.c
+++ b/drivers/cpufreq/cppc_cpufreq.c
@@ -312,7 +312,7 @@ static unsigned int get_transition_laten
{
unsigned int transition_latency_ns = cppc_get_transition_latency(cpu);
- if (transition_latency_ns == CPUFREQ_ETERNAL)
+ if (transition_latency_ns == CPPC_NO_DATA)
return CPUFREQ_DEFAULT_TRANSITION_LATENCY_NS / NSEC_PER_USEC;
return transition_latency_ns / NSEC_PER_USEC;
--- a/include/acpi/cppc_acpi.h
+++ b/include/acpi/cppc_acpi.h
@@ -41,6 +41,8 @@
#define CPPC_ENERGY_PERF_MAX (0xFF)
+#define CPPC_NO_DATA (unsigned int)(-1)
+
/* Each register has the folowing format. */
struct cpc_reg {
u8 descriptor;
@@ -218,7 +220,7 @@ static inline bool cppc_allow_fast_switc
}
static inline unsigned int cppc_get_transition_latency(int cpu)
{
- return CPUFREQ_ETERNAL;
+ return CPPC_NO_DATA;
}
static inline bool cpc_ffh_supported(void)
{
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v1 4/4] cpufreq: Drop unused symbol CPUFREQ_ETERNAL
2025-09-25 15:42 [PATCH v1 0/4] cpufreq: Fixes and cleanups related to CPUFREQ_ETERNAL Rafael J. Wysocki
` (2 preceding siblings ...)
2025-09-25 15:46 ` [PATCH v1 3/4] ACPI: CPPC: Replace CPUFREQ_ETERNAL with CPPC-specific symbol Rafael J. Wysocki
@ 2025-09-25 15:47 ` Rafael J. Wysocki
2025-09-25 16:36 ` Mario Limonciello (AMD) (kernel.org)
2025-09-26 9:47 ` Jie Zhan
2025-09-29 7:28 ` [PATCH v1 0/4] cpufreq: Fixes and cleanups related to CPUFREQ_ETERNAL Viresh Kumar
4 siblings, 2 replies; 18+ messages in thread
From: Rafael J. Wysocki @ 2025-09-25 15:47 UTC (permalink / raw)
To: Linux PM
Cc: Shawn Guo, Qais Yousef, LKML, Viresh Kumar, Prashanth Prakash,
Pierre Gondois, Mario Limonciello, Linux ACPI
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Drop CPUFREQ_ETERNAL that has no users any more along with all
references to it in the documentation.
No functional impact.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
Documentation/admin-guide/pm/cpufreq.rst | 4 ----
Documentation/cpu-freq/cpu-drivers.rst | 3 +--
Documentation/translations/zh_CN/cpu-freq/cpu-drivers.rst | 3 +--
Documentation/translations/zh_TW/cpu-freq/cpu-drivers.rst | 3 +--
include/linux/cpufreq.h | 5 -----
5 files changed, 3 insertions(+), 15 deletions(-)
--- a/Documentation/admin-guide/pm/cpufreq.rst
+++ b/Documentation/admin-guide/pm/cpufreq.rst
@@ -274,10 +274,6 @@ are the following:
The time it takes to switch the CPUs belonging to this policy from one
P-state to another, in nanoseconds.
- If unknown or if known to be so high that the scaling driver does not
- work with the `ondemand`_ governor, -1 (:c:macro:`CPUFREQ_ETERNAL`)
- will be returned by reads from this attribute.
-
``related_cpus``
List of all (online and offline) CPUs belonging to this policy.
--- a/Documentation/cpu-freq/cpu-drivers.rst
+++ b/Documentation/cpu-freq/cpu-drivers.rst
@@ -109,8 +109,7 @@ Then, the driver must fill in the follow
+-----------------------------------+--------------------------------------+
|policy->cpuinfo.transition_latency | the time it takes on this CPU to |
| | switch between two frequencies in |
-| | nanoseconds (if appropriate, else |
-| | specify CPUFREQ_ETERNAL) |
+| | nanoseconds |
+-----------------------------------+--------------------------------------+
|policy->cur | The current operating frequency of |
| | this CPU (if appropriate) |
--- a/Documentation/translations/zh_CN/cpu-freq/cpu-drivers.rst
+++ b/Documentation/translations/zh_CN/cpu-freq/cpu-drivers.rst
@@ -112,8 +112,7 @@ CPUfreq核心层注册一个cpufreq_driv
| | |
+-----------------------------------+--------------------------------------+
|policy->cpuinfo.transition_latency | CPU在两个频率之间切换所需的时间,以 |
-| | 纳秒为单位(如不适用,设定为 |
-| | CPUFREQ_ETERNAL) |
+| | 纳秒为单位 |
| | |
+-----------------------------------+--------------------------------------+
|policy->cur | 该CPU当前的工作频率(如适用) |
--- a/Documentation/translations/zh_TW/cpu-freq/cpu-drivers.rst
+++ b/Documentation/translations/zh_TW/cpu-freq/cpu-drivers.rst
@@ -112,8 +112,7 @@ CPUfreq核心層註冊一個cpufreq_driv
| | |
+-----------------------------------+--------------------------------------+
|policy->cpuinfo.transition_latency | CPU在兩個頻率之間切換所需的時間,以 |
-| | 納秒爲單位(如不適用,設定爲 |
-| | CPUFREQ_ETERNAL) |
+| | 納秒爲單位 |
| | |
+-----------------------------------+--------------------------------------+
|policy->cur | 該CPU當前的工作頻率(如適用) |
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -26,13 +26,8 @@
*********************************************************************/
/*
* Frequency values here are CPU kHz
- *
- * Maximum transition latency is in nanoseconds - if it's unknown,
- * CPUFREQ_ETERNAL shall be used.
*/
-#define CPUFREQ_ETERNAL (-1)
-
#define CPUFREQ_DEFAULT_TANSITION_LATENCY_NS NSEC_PER_MSEC
#define CPUFREQ_NAME_LEN 16
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v1 3/4] ACPI: CPPC: Replace CPUFREQ_ETERNAL with CPPC-specific symbol
2025-09-25 15:46 ` [PATCH v1 3/4] ACPI: CPPC: Replace CPUFREQ_ETERNAL with CPPC-specific symbol Rafael J. Wysocki
@ 2025-09-25 16:35 ` Mario Limonciello
2025-09-25 16:57 ` Rafael J. Wysocki
2025-09-25 17:23 ` [PATCH v2 3/4] ACPI: CPPC: Do not use CPUFREQ_ETERNAL as an error value Rafael J. Wysocki
1 sibling, 1 reply; 18+ messages in thread
From: Mario Limonciello @ 2025-09-25 16:35 UTC (permalink / raw)
To: Rafael J. Wysocki, Linux PM
Cc: Shawn Guo, Qais Yousef, LKML, Viresh Kumar, Prashanth Prakash,
Pierre Gondois, Linux ACPI
On 9/25/2025 10:46 AM, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Instead of using CPUFREQ_ETERNAL for signaling error conditions in
> cppc_get_transition_latency(), introduce CPPC_NO_DATA specifically
> for this purpose and update all of the callers of this function to
> use it.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> drivers/acpi/cppc_acpi.c | 6 +++---
> drivers/cpufreq/amd-pstate.c | 4 ++--
> drivers/cpufreq/cppc_cpufreq.c | 2 +-
> include/acpi/cppc_acpi.h | 4 +++-
> include/linux/cpufreq.h | 3 ---
> 5 files changed, 9 insertions(+), 10 deletions(-)
>
> --- a/drivers/acpi/cppc_acpi.c
> +++ b/drivers/acpi/cppc_acpi.c
> @@ -1897,16 +1897,16 @@ unsigned int cppc_get_transition_latency
>
> cpc_desc = per_cpu(cpc_desc_ptr, cpu_num);
> if (!cpc_desc)
> - return CPUFREQ_ETERNAL;
> + return CPPC_NO_DATA;
>
> desired_reg = &cpc_desc->cpc_regs[DESIRED_PERF];
> if (CPC_IN_SYSTEM_MEMORY(desired_reg) || CPC_IN_SYSTEM_IO(desired_reg))
> return 0;
> else if (!CPC_IN_PCC(desired_reg))
> - return CPUFREQ_ETERNAL;
> + return CPPC_NO_DATA;
>
> if (pcc_ss_id < 0)
> - return CPUFREQ_ETERNAL;
> + return CPPC_NO_DATA;
>
> pcc_ss_data = pcc_data[pcc_ss_id];
> if (pcc_ss_data->pcc_mpar)
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -875,7 +875,7 @@ static u32 amd_pstate_get_transition_del
> u32 transition_delay_ns;
>
> transition_delay_ns = cppc_get_transition_latency(cpu);
> - if (transition_delay_ns == CPUFREQ_ETERNAL) {
> + if (transition_delay_ns == CPPC_NO_DATA) {
> if (cpu_feature_enabled(X86_FEATURE_AMD_FAST_CPPC))
> return AMD_PSTATE_FAST_CPPC_TRANSITION_DELAY;
> else
> @@ -894,7 +894,7 @@ static u32 amd_pstate_get_transition_lat
> u32 transition_latency;
>
> transition_latency = cppc_get_transition_latency(cpu);
> - if (transition_latency == CPUFREQ_ETERNAL)
> + if (transition_latency == CPPC_NO_DATA)
> return AMD_PSTATE_TRANSITION_LATENCY;
>
> return transition_latency;
> --- a/drivers/cpufreq/cppc_cpufreq.c
> +++ b/drivers/cpufreq/cppc_cpufreq.c
> @@ -312,7 +312,7 @@ static unsigned int get_transition_laten
> {
> unsigned int transition_latency_ns = cppc_get_transition_latency(cpu);
>
> - if (transition_latency_ns == CPUFREQ_ETERNAL)
> + if (transition_latency_ns == CPPC_NO_DATA)
> return CPUFREQ_DEFAULT_TRANSITION_LATENCY_NS / NSEC_PER_USEC;
>
> return transition_latency_ns / NSEC_PER_USEC;
> --- a/include/acpi/cppc_acpi.h
> +++ b/include/acpi/cppc_acpi.h
> @@ -41,6 +41,8 @@
>
> #define CPPC_ENERGY_PERF_MAX (0xFF)
>
> +#define CPPC_NO_DATA (unsigned int)(-1)
> +
Would it be cleaner to just change all the functions that can return
CPPC_NO_DATA to int instead of unsigned int and instead use -ENODATA?
> /* Each register has the folowing format. */
> struct cpc_reg {
> u8 descriptor;
> @@ -218,7 +220,7 @@ static inline bool cppc_allow_fast_switc
> }
> static inline unsigned int cppc_get_transition_latency(int cpu)
> {
> - return CPUFREQ_ETERNAL;
> + return CPPC_NO_DATA;
> }
> static inline bool cpc_ffh_supported(void)
> {
>
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v1 4/4] cpufreq: Drop unused symbol CPUFREQ_ETERNAL
2025-09-25 15:47 ` [PATCH v1 4/4] cpufreq: Drop unused symbol CPUFREQ_ETERNAL Rafael J. Wysocki
@ 2025-09-25 16:36 ` Mario Limonciello (AMD) (kernel.org)
2025-09-26 9:47 ` Jie Zhan
1 sibling, 0 replies; 18+ messages in thread
From: Mario Limonciello (AMD) (kernel.org) @ 2025-09-25 16:36 UTC (permalink / raw)
To: Rafael J. Wysocki, Linux PM
Cc: Shawn Guo, Qais Yousef, LKML, Viresh Kumar, Prashanth Prakash,
Pierre Gondois, Linux ACPI
On 9/25/2025 10:47 AM, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Drop CPUFREQ_ETERNAL that has no users any more along with all
> references to it in the documentation.
>
> No functional impact.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>> ---
> Documentation/admin-guide/pm/cpufreq.rst | 4 ----
> Documentation/cpu-freq/cpu-drivers.rst | 3 +--
> Documentation/translations/zh_CN/cpu-freq/cpu-drivers.rst | 3 +--
> Documentation/translations/zh_TW/cpu-freq/cpu-drivers.rst | 3 +--
> include/linux/cpufreq.h | 5 -----
> 5 files changed, 3 insertions(+), 15 deletions(-)
>
> --- a/Documentation/admin-guide/pm/cpufreq.rst
> +++ b/Documentation/admin-guide/pm/cpufreq.rst
> @@ -274,10 +274,6 @@ are the following:
> The time it takes to switch the CPUs belonging to this policy from one
> P-state to another, in nanoseconds.
>
> - If unknown or if known to be so high that the scaling driver does not
> - work with the `ondemand`_ governor, -1 (:c:macro:`CPUFREQ_ETERNAL`)
> - will be returned by reads from this attribute.
> -
> ``related_cpus``
> List of all (online and offline) CPUs belonging to this policy.
>
> --- a/Documentation/cpu-freq/cpu-drivers.rst
> +++ b/Documentation/cpu-freq/cpu-drivers.rst
> @@ -109,8 +109,7 @@ Then, the driver must fill in the follow
> +-----------------------------------+--------------------------------------+
> |policy->cpuinfo.transition_latency | the time it takes on this CPU to |
> | | switch between two frequencies in |
> -| | nanoseconds (if appropriate, else |
> -| | specify CPUFREQ_ETERNAL) |
> +| | nanoseconds |
> +-----------------------------------+--------------------------------------+
> |policy->cur | The current operating frequency of |
> | | this CPU (if appropriate) |
> --- a/Documentation/translations/zh_CN/cpu-freq/cpu-drivers.rst
> +++ b/Documentation/translations/zh_CN/cpu-freq/cpu-drivers.rst
> @@ -112,8 +112,7 @@ CPUfreq核心层注册一个cpufreq_driv
> | | |
> +-----------------------------------+--------------------------------------+
> |policy->cpuinfo.transition_latency | CPU在两个频率之间切换所需的时间,以 |
> -| | 纳秒为单位(如不适用,设定为 |
> -| | CPUFREQ_ETERNAL) |
> +| | 纳秒为单位 |
> | | |
> +-----------------------------------+--------------------------------------+
> |policy->cur | 该CPU当前的工作频率(如适用) |
> --- a/Documentation/translations/zh_TW/cpu-freq/cpu-drivers.rst
> +++ b/Documentation/translations/zh_TW/cpu-freq/cpu-drivers.rst
> @@ -112,8 +112,7 @@ CPUfreq核心層註冊一個cpufreq_driv
> | | |
> +-----------------------------------+--------------------------------------+
> |policy->cpuinfo.transition_latency | CPU在兩個頻率之間切換所需的時間,以 |
> -| | 納秒爲單位(如不適用,設定爲 |
> -| | CPUFREQ_ETERNAL) |
> +| | 納秒爲單位 |
> | | |
> +-----------------------------------+--------------------------------------+
> |policy->cur | 該CPU當前的工作頻率(如適用) |
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -26,13 +26,8 @@
> *********************************************************************/
> /*
> * Frequency values here are CPU kHz
> - *
> - * Maximum transition latency is in nanoseconds - if it's unknown,
> - * CPUFREQ_ETERNAL shall be used.
> */
>
> -#define CPUFREQ_ETERNAL (-1)
> -
> #define CPUFREQ_DEFAULT_TANSITION_LATENCY_NS NSEC_PER_MSEC
>
> #define CPUFREQ_NAME_LEN 16
>
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v1 2/4] cpufreq: CPPC: Avoid using CPUFREQ_ETERNAL as transition delay
2025-09-25 15:44 ` [PATCH v1 2/4] cpufreq: CPPC: Avoid using CPUFREQ_ETERNAL as transition delay Rafael J. Wysocki
@ 2025-09-25 16:36 ` Mario Limonciello
2025-09-26 9:41 ` Jie Zhan
1 sibling, 0 replies; 18+ messages in thread
From: Mario Limonciello @ 2025-09-25 16:36 UTC (permalink / raw)
To: Rafael J. Wysocki, Linux PM
Cc: Shawn Guo, Qais Yousef, LKML, Viresh Kumar, Prashanth Prakash,
Pierre Gondois, Linux ACPI
On 9/25/2025 10:44 AM, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> If cppc_get_transition_latency() returns CPUFREQ_ETERNAL to indicate a
> failure to retrieve the transition latency value from the platform
> firmware, the CPPC cpufreq driver will use that value (converted to
> microseconds) as the policy transition delay, but it is way too large
> for any practical use.
>
> Address this by making the driver use the cpufreq's default
> transition latency value (in microseconds) as the transition delay
> if CPUFREQ_ETERNAL is returned by cppc_get_transition_latency().
>
> Fixes: d4f3388afd48 ("cpufreq / CPPC: Set platform specific transition_delay_us")
> Cc: 5.19+ <stable@vger.kernel.org> # 5.19
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>> ---
> drivers/cpufreq/cppc_cpufreq.c | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> --- a/drivers/cpufreq/cppc_cpufreq.c
> +++ b/drivers/cpufreq/cppc_cpufreq.c
> @@ -308,6 +308,16 @@ static int cppc_verify_policy(struct cpu
> return 0;
> }
>
> +static unsigned int get_transition_latency_from_cppc(unsigned int cpu)
> +{
> + unsigned int transition_latency_ns = cppc_get_transition_latency(cpu);
> +
> + if (transition_latency_ns == CPUFREQ_ETERNAL)
> + return CPUFREQ_DEFAULT_TRANSITION_LATENCY_NS / NSEC_PER_USEC;
> +
> + return transition_latency_ns / NSEC_PER_USEC;
> +}
> +
> /*
> * The PCC subspace describes the rate at which platform can accept commands
> * on the shared PCC channel (including READs which do not count towards freq
> @@ -330,12 +340,12 @@ static unsigned int cppc_cpufreq_get_tra
> return 10000;
> }
> }
> - return cppc_get_transition_latency(cpu) / NSEC_PER_USEC;
> + return get_transition_latency_from_cppc(cpu);
> }
> #else
> static unsigned int cppc_cpufreq_get_transition_delay_us(unsigned int cpu)
> {
> - return cppc_get_transition_latency(cpu) / NSEC_PER_USEC;
> + return get_transition_latency_from_cppc(cpu);
> }
> #endif
>
>
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v1 1/4] cpufreq: Make drivers using CPUFREQ_ETERNAL specify transition latency
2025-09-25 15:44 ` [PATCH v1 1/4] cpufreq: Make drivers using CPUFREQ_ETERNAL specify transition latency Rafael J. Wysocki
@ 2025-09-25 16:37 ` Mario Limonciello
2025-09-26 9:46 ` Jie Zhan
1 sibling, 0 replies; 18+ messages in thread
From: Mario Limonciello @ 2025-09-25 16:37 UTC (permalink / raw)
To: Rafael J. Wysocki, Linux PM
Cc: Shawn Guo, Qais Yousef, LKML, Viresh Kumar, Prashanth Prakash,
Pierre Gondois, Linux ACPI
On 9/25/2025 10:44 AM, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Commit a755d0e2d41b ("cpufreq: Honour transition_latency over
> transition_delay_us") caused platforms where cpuinfo.transition_latency
> is CPUFREQ_ETERNAL to get a very large transition latency whereas
> previously it had been capped at 10 ms (and later at 2 ms).
>
> This led to a user-observable regression between 6.6 and 6.12 as
> described by Shawn:
>
> "The dbs sampling_rate was 10000 us on 6.6 and suddently becomes
> 6442450 us (4294967295 / 1000 * 1.5) on 6.12 for these platforms
> because the default transition delay was dropped [...].
>
> It slows down dbs governor's reacting to CPU loading change
> dramatically. Also, as transition_delay_us is used by schedutil
> governor as rate_limit_us, it shows a negative impact on device
> idle power consumption, because the device gets slightly less time
> in the lowest OPP."
>
> Evidently, the expectation of the drivers using CPUFREQ_ETERNAL as
> cpuinfo.transition_latency was that it would be capped by the core,
> but they may as well return a default transition latency value instead
> of CPUFREQ_ETERNAL and the core need not do anything with it.
>
> Accordingly, introduce CPUFREQ_DEFAULT_TRANSITION_LATENCY_NS and make
> all of the drivers in question use it instead of CPUFREQ_ETERNAL.
>
> Fixes: a755d0e2d41b ("cpufreq: Honour transition_latency over transition_delay_us")
> Closes: https://lore.kernel.org/linux-pm/20250922125929.453444-1-shawnguo2@yeah.net/
> Reported-by: Shawn Guo <shawnguo@kernel.org>
Not 100% sure, but I think checkpatch gets pedantic about Closes
followed by Reported-by and instead wants Reported-by followed by Closes.
> Cc: 6.6+ <stable@vger.kernel.org> # 6.6+
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>> ---
> drivers/cpufreq/cpufreq-dt.c | 2 +-
> drivers/cpufreq/imx6q-cpufreq.c | 2 +-
> drivers/cpufreq/mediatek-cpufreq-hw.c | 2 +-
> drivers/cpufreq/scmi-cpufreq.c | 2 +-
> drivers/cpufreq/scpi-cpufreq.c | 2 +-
> drivers/cpufreq/spear-cpufreq.c | 2 +-
> include/linux/cpufreq.h | 3 +++
> 7 files changed, 9 insertions(+), 6 deletions(-)
>
> --- a/drivers/cpufreq/cpufreq-dt.c
> +++ b/drivers/cpufreq/cpufreq-dt.c
> @@ -104,7 +104,7 @@ static int cpufreq_init(struct cpufreq_p
>
> transition_latency = dev_pm_opp_get_max_transition_latency(cpu_dev);
> if (!transition_latency)
> - transition_latency = CPUFREQ_ETERNAL;
> + transition_latency = CPUFREQ_DEFAULT_TRANSITION_LATENCY_NS;
>
> cpumask_copy(policy->cpus, priv->cpus);
> policy->driver_data = priv;
> --- a/drivers/cpufreq/imx6q-cpufreq.c
> +++ b/drivers/cpufreq/imx6q-cpufreq.c
> @@ -442,7 +442,7 @@ soc_opp_out:
> }
>
> if (of_property_read_u32(np, "clock-latency", &transition_latency))
> - transition_latency = CPUFREQ_ETERNAL;
> + transition_latency = CPUFREQ_DEFAULT_TRANSITION_LATENCY_NS;
>
> /*
> * Calculate the ramp time for max voltage change in the
> --- a/drivers/cpufreq/mediatek-cpufreq-hw.c
> +++ b/drivers/cpufreq/mediatek-cpufreq-hw.c
> @@ -309,7 +309,7 @@ static int mtk_cpufreq_hw_cpu_init(struc
>
> latency = readl_relaxed(data->reg_bases[REG_FREQ_LATENCY]) * 1000;
> if (!latency)
> - latency = CPUFREQ_ETERNAL;
> + latency = CPUFREQ_DEFAULT_TRANSITION_LATENCY_NS;
>
> policy->cpuinfo.transition_latency = latency;
> policy->fast_switch_possible = true;
> --- a/drivers/cpufreq/scmi-cpufreq.c
> +++ b/drivers/cpufreq/scmi-cpufreq.c
> @@ -294,7 +294,7 @@ static int scmi_cpufreq_init(struct cpuf
>
> latency = perf_ops->transition_latency_get(ph, domain);
> if (!latency)
> - latency = CPUFREQ_ETERNAL;
> + latency = CPUFREQ_DEFAULT_TRANSITION_LATENCY_NS;
>
> policy->cpuinfo.transition_latency = latency;
>
> --- a/drivers/cpufreq/scpi-cpufreq.c
> +++ b/drivers/cpufreq/scpi-cpufreq.c
> @@ -157,7 +157,7 @@ static int scpi_cpufreq_init(struct cpuf
>
> latency = scpi_ops->get_transition_latency(cpu_dev);
> if (!latency)
> - latency = CPUFREQ_ETERNAL;
> + latency = CPUFREQ_DEFAULT_TRANSITION_LATENCY_NS;
>
> policy->cpuinfo.transition_latency = latency;
>
> --- a/drivers/cpufreq/spear-cpufreq.c
> +++ b/drivers/cpufreq/spear-cpufreq.c
> @@ -182,7 +182,7 @@ static int spear_cpufreq_probe(struct pl
>
> if (of_property_read_u32(np, "clock-latency",
> &spear_cpufreq.transition_latency))
> - spear_cpufreq.transition_latency = CPUFREQ_ETERNAL;
> + spear_cpufreq.transition_latency = CPUFREQ_DEFAULT_TRANSITION_LATENCY_NS;
>
> cnt = of_property_count_u32_elems(np, "cpufreq_tbl");
> if (cnt <= 0) {
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -32,6 +32,9 @@
> */
>
> #define CPUFREQ_ETERNAL (-1)
> +
> +#define CPUFREQ_DEFAULT_TANSITION_LATENCY_NS NSEC_PER_MSEC
> +
> #define CPUFREQ_NAME_LEN 16
> /* Print length for names. Extra 1 space for accommodating '\n' in prints */
> #define CPUFREQ_NAME_PLEN (CPUFREQ_NAME_LEN + 1)
>
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v1 3/4] ACPI: CPPC: Replace CPUFREQ_ETERNAL with CPPC-specific symbol
2025-09-25 16:35 ` Mario Limonciello
@ 2025-09-25 16:57 ` Rafael J. Wysocki
0 siblings, 0 replies; 18+ messages in thread
From: Rafael J. Wysocki @ 2025-09-25 16:57 UTC (permalink / raw)
To: Mario Limonciello
Cc: Rafael J. Wysocki, Linux PM, Shawn Guo, Qais Yousef, LKML,
Viresh Kumar, Prashanth Prakash, Pierre Gondois, Linux ACPI
On Thu, Sep 25, 2025 at 6:35 PM Mario Limonciello
<mario.limonciello@amd.com> wrote:
>
>
>
> On 9/25/2025 10:46 AM, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > Instead of using CPUFREQ_ETERNAL for signaling error conditions in
> > cppc_get_transition_latency(), introduce CPPC_NO_DATA specifically
> > for this purpose and update all of the callers of this function to
> > use it.
> >
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
> > drivers/acpi/cppc_acpi.c | 6 +++---
> > drivers/cpufreq/amd-pstate.c | 4 ++--
> > drivers/cpufreq/cppc_cpufreq.c | 2 +-
> > include/acpi/cppc_acpi.h | 4 +++-
> > include/linux/cpufreq.h | 3 ---
> > 5 files changed, 9 insertions(+), 10 deletions(-)
> >
> > --- a/drivers/acpi/cppc_acpi.c
> > +++ b/drivers/acpi/cppc_acpi.c
> > @@ -1897,16 +1897,16 @@ unsigned int cppc_get_transition_latency
> >
> > cpc_desc = per_cpu(cpc_desc_ptr, cpu_num);
> > if (!cpc_desc)
> > - return CPUFREQ_ETERNAL;
> > + return CPPC_NO_DATA;
> >
> > desired_reg = &cpc_desc->cpc_regs[DESIRED_PERF];
> > if (CPC_IN_SYSTEM_MEMORY(desired_reg) || CPC_IN_SYSTEM_IO(desired_reg))
> > return 0;
> > else if (!CPC_IN_PCC(desired_reg))
> > - return CPUFREQ_ETERNAL;
> > + return CPPC_NO_DATA;
> >
> > if (pcc_ss_id < 0)
> > - return CPUFREQ_ETERNAL;
> > + return CPPC_NO_DATA;
> >
> > pcc_ss_data = pcc_data[pcc_ss_id];
> > if (pcc_ss_data->pcc_mpar)
> > --- a/drivers/cpufreq/amd-pstate.c
> > +++ b/drivers/cpufreq/amd-pstate.c
> > @@ -875,7 +875,7 @@ static u32 amd_pstate_get_transition_del
> > u32 transition_delay_ns;
> >
> > transition_delay_ns = cppc_get_transition_latency(cpu);
> > - if (transition_delay_ns == CPUFREQ_ETERNAL) {
> > + if (transition_delay_ns == CPPC_NO_DATA) {
> > if (cpu_feature_enabled(X86_FEATURE_AMD_FAST_CPPC))
> > return AMD_PSTATE_FAST_CPPC_TRANSITION_DELAY;
> > else
> > @@ -894,7 +894,7 @@ static u32 amd_pstate_get_transition_lat
> > u32 transition_latency;
> >
> > transition_latency = cppc_get_transition_latency(cpu);
> > - if (transition_latency == CPUFREQ_ETERNAL)
> > + if (transition_latency == CPPC_NO_DATA)
> > return AMD_PSTATE_TRANSITION_LATENCY;
> >
> > return transition_latency;
> > --- a/drivers/cpufreq/cppc_cpufreq.c
> > +++ b/drivers/cpufreq/cppc_cpufreq.c
> > @@ -312,7 +312,7 @@ static unsigned int get_transition_laten
> > {
> > unsigned int transition_latency_ns = cppc_get_transition_latency(cpu);
> >
> > - if (transition_latency_ns == CPUFREQ_ETERNAL)
> > + if (transition_latency_ns == CPPC_NO_DATA)
> > return CPUFREQ_DEFAULT_TRANSITION_LATENCY_NS / NSEC_PER_USEC;
> >
> > return transition_latency_ns / NSEC_PER_USEC;
> > --- a/include/acpi/cppc_acpi.h
> > +++ b/include/acpi/cppc_acpi.h
> > @@ -41,6 +41,8 @@
> >
> > #define CPPC_ENERGY_PERF_MAX (0xFF)
> >
> > +#define CPPC_NO_DATA (unsigned int)(-1)
> > +
>
> Would it be cleaner to just change all the functions that can return
> CPPC_NO_DATA to int instead of unsigned int and instead use -ENODATA?
Yes, that'll work too.
I'll send an update of this patch shortly.
> > /* Each register has the folowing format. */
> > struct cpc_reg {
> > u8 descriptor;
> > @@ -218,7 +220,7 @@ static inline bool cppc_allow_fast_switc
> > }
> > static inline unsigned int cppc_get_transition_latency(int cpu)
> > {
> > - return CPUFREQ_ETERNAL;
> > + return CPPC_NO_DATA;
> > }
> > static inline bool cpc_ffh_supported(void)
> > {
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 3/4] ACPI: CPPC: Do not use CPUFREQ_ETERNAL as an error value
2025-09-25 15:46 ` [PATCH v1 3/4] ACPI: CPPC: Replace CPUFREQ_ETERNAL with CPPC-specific symbol Rafael J. Wysocki
2025-09-25 16:35 ` Mario Limonciello
@ 2025-09-25 17:23 ` Rafael J. Wysocki
2025-09-25 18:33 ` Mario Limonciello (AMD) (kernel.org)
2025-09-26 9:30 ` Jie Zhan
1 sibling, 2 replies; 18+ messages in thread
From: Rafael J. Wysocki @ 2025-09-25 17:23 UTC (permalink / raw)
To: Linux PM, Mario Limonciello
Cc: Shawn Guo, Qais Yousef, LKML, Viresh Kumar, Pierre Gondois,
Linux ACPI
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Instead of using CPUFREQ_ETERNAL for signaling an error condition
in cppc_get_transition_latency(), change the return value type of
that function to int and make it return a proper negative error
code on failures.
No intentional functional impact.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
v1 -> v2:
* Change cppc_get_transition_latency() return value data type to int
* Make it return -ENODATA on errors (Mario)
* Update its callers accordingly
* Adjust the subject and changelog
* Add a missing empty code line to cppc_get_transition_latency()
The modifications of this patch don't affect any other patches in the series:
https://lore.kernel.org/linux-pm/8605612.T7Z3S40VBb@rafael.j.wysocki/
---
drivers/acpi/cppc_acpi.c | 15 ++++++++-------
drivers/cpufreq/amd-pstate.c | 8 ++++----
drivers/cpufreq/cppc_cpufreq.c | 4 ++--
include/acpi/cppc_acpi.h | 6 +++---
4 files changed, 17 insertions(+), 16 deletions(-)
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -1876,7 +1876,7 @@ EXPORT_SYMBOL_GPL(cppc_set_perf);
* If desired_reg is in the SystemMemory or SystemIo ACPI address space,
* then assume there is no latency.
*/
-unsigned int cppc_get_transition_latency(int cpu_num)
+int cppc_get_transition_latency(int cpu_num)
{
/*
* Expected transition latency is based on the PCCT timing values
@@ -1889,31 +1889,32 @@ unsigned int cppc_get_transition_latency
* completion of a command before issuing the next command,
* in microseconds.
*/
- unsigned int latency_ns = 0;
struct cpc_desc *cpc_desc;
struct cpc_register_resource *desired_reg;
int pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpu_num);
struct cppc_pcc_data *pcc_ss_data;
+ int latency_ns = 0;
cpc_desc = per_cpu(cpc_desc_ptr, cpu_num);
if (!cpc_desc)
- return CPUFREQ_ETERNAL;
+ return -ENODATA;
desired_reg = &cpc_desc->cpc_regs[DESIRED_PERF];
if (CPC_IN_SYSTEM_MEMORY(desired_reg) || CPC_IN_SYSTEM_IO(desired_reg))
return 0;
+
else if (!CPC_IN_PCC(desired_reg))
- return CPUFREQ_ETERNAL;
+ return -ENODATA;
if (pcc_ss_id < 0)
- return CPUFREQ_ETERNAL;
+ return -ENODATA;
pcc_ss_data = pcc_data[pcc_ss_id];
if (pcc_ss_data->pcc_mpar)
latency_ns = 60 * (1000 * 1000 * 1000 / pcc_ss_data->pcc_mpar);
- latency_ns = max(latency_ns, pcc_ss_data->pcc_nominal * 1000);
- latency_ns = max(latency_ns, pcc_ss_data->pcc_mrtt * 1000);
+ latency_ns = max_t(int, latency_ns, pcc_ss_data->pcc_nominal * 1000);
+ latency_ns = max_t(int, latency_ns, pcc_ss_data->pcc_mrtt * 1000);
return latency_ns;
}
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -872,10 +872,10 @@ static void amd_pstate_update_limits(str
*/
static u32 amd_pstate_get_transition_delay_us(unsigned int cpu)
{
- u32 transition_delay_ns;
+ int transition_delay_ns;
transition_delay_ns = cppc_get_transition_latency(cpu);
- if (transition_delay_ns == CPUFREQ_ETERNAL) {
+ if (transition_delay_ns < 0) {
if (cpu_feature_enabled(X86_FEATURE_AMD_FAST_CPPC))
return AMD_PSTATE_FAST_CPPC_TRANSITION_DELAY;
else
@@ -891,10 +891,10 @@ static u32 amd_pstate_get_transition_del
*/
static u32 amd_pstate_get_transition_latency(unsigned int cpu)
{
- u32 transition_latency;
+ int transition_latency;
transition_latency = cppc_get_transition_latency(cpu);
- if (transition_latency == CPUFREQ_ETERNAL)
+ if (transition_latency < 0)
return AMD_PSTATE_TRANSITION_LATENCY;
return transition_latency;
--- a/drivers/cpufreq/cppc_cpufreq.c
+++ b/drivers/cpufreq/cppc_cpufreq.c
@@ -310,9 +310,9 @@ static int cppc_verify_policy(struct cpu
static unsigned int get_transition_latency_from_cppc(unsigned int cpu)
{
- unsigned int transition_latency_ns = cppc_get_transition_latency(cpu);
+ int transition_latency_ns = cppc_get_transition_latency(cpu);
- if (transition_latency_ns == CPUFREQ_ETERNAL)
+ if (transition_latency_ns < 0)
return CPUFREQ_DEFAULT_TRANSITION_LATENCY_NS / NSEC_PER_USEC;
return transition_latency_ns / NSEC_PER_USEC;
--- a/include/acpi/cppc_acpi.h
+++ b/include/acpi/cppc_acpi.h
@@ -160,7 +160,7 @@ extern unsigned int cppc_khz_to_perf(str
extern bool acpi_cpc_valid(void);
extern bool cppc_allow_fast_switch(void);
extern int acpi_get_psd_map(unsigned int cpu, struct cppc_cpudata *cpu_data);
-extern unsigned int cppc_get_transition_latency(int cpu);
+extern int cppc_get_transition_latency(int cpu);
extern bool cpc_ffh_supported(void);
extern bool cpc_supported_by_cpu(void);
extern int cpc_read_ffh(int cpunum, struct cpc_reg *reg, u64 *val);
@@ -216,9 +216,9 @@ static inline bool cppc_allow_fast_switc
{
return false;
}
-static inline unsigned int cppc_get_transition_latency(int cpu)
+static inline int cppc_get_transition_latency(int cpu)
{
- return CPUFREQ_ETERNAL;
+ return -ENODATA;
}
static inline bool cpc_ffh_supported(void)
{
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 3/4] ACPI: CPPC: Do not use CPUFREQ_ETERNAL as an error value
2025-09-25 17:23 ` [PATCH v2 3/4] ACPI: CPPC: Do not use CPUFREQ_ETERNAL as an error value Rafael J. Wysocki
@ 2025-09-25 18:33 ` Mario Limonciello (AMD) (kernel.org)
2025-09-26 9:30 ` Jie Zhan
1 sibling, 0 replies; 18+ messages in thread
From: Mario Limonciello (AMD) (kernel.org) @ 2025-09-25 18:33 UTC (permalink / raw)
To: Rafael J. Wysocki, Linux PM
Cc: Shawn Guo, Qais Yousef, LKML, Viresh Kumar, Pierre Gondois,
Linux ACPI
On 9/25/2025 12:23 PM, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Instead of using CPUFREQ_ETERNAL for signaling an error condition
> in cppc_get_transition_latency(), change the return value type of
> that function to int and make it return a proper negative error
> code on failures.
>
> No intentional functional impact.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
> ---
>
> v1 -> v2:
> * Change cppc_get_transition_latency() return value data type to int
> * Make it return -ENODATA on errors (Mario)
> * Update its callers accordingly
> * Adjust the subject and changelog
> * Add a missing empty code line to cppc_get_transition_latency()
>
> The modifications of this patch don't affect any other patches in the series:
>
> https://lore.kernel.org/linux-pm/8605612.T7Z3S40VBb@rafael.j.wysocki/
>
> ---
> drivers/acpi/cppc_acpi.c | 15 ++++++++-------
> drivers/cpufreq/amd-pstate.c | 8 ++++----
> drivers/cpufreq/cppc_cpufreq.c | 4 ++--
> include/acpi/cppc_acpi.h | 6 +++---
> 4 files changed, 17 insertions(+), 16 deletions(-)
>
> --- a/drivers/acpi/cppc_acpi.c
> +++ b/drivers/acpi/cppc_acpi.c
> @@ -1876,7 +1876,7 @@ EXPORT_SYMBOL_GPL(cppc_set_perf);
> * If desired_reg is in the SystemMemory or SystemIo ACPI address space,
> * then assume there is no latency.
> */
> -unsigned int cppc_get_transition_latency(int cpu_num)
> +int cppc_get_transition_latency(int cpu_num)
> {
> /*
> * Expected transition latency is based on the PCCT timing values
> @@ -1889,31 +1889,32 @@ unsigned int cppc_get_transition_latency
> * completion of a command before issuing the next command,
> * in microseconds.
> */
> - unsigned int latency_ns = 0;
> struct cpc_desc *cpc_desc;
> struct cpc_register_resource *desired_reg;
> int pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpu_num);
> struct cppc_pcc_data *pcc_ss_data;
> + int latency_ns = 0;
>
> cpc_desc = per_cpu(cpc_desc_ptr, cpu_num);
> if (!cpc_desc)
> - return CPUFREQ_ETERNAL;
> + return -ENODATA;
>
> desired_reg = &cpc_desc->cpc_regs[DESIRED_PERF];
> if (CPC_IN_SYSTEM_MEMORY(desired_reg) || CPC_IN_SYSTEM_IO(desired_reg))
> return 0;
> +
> else if (!CPC_IN_PCC(desired_reg))
> - return CPUFREQ_ETERNAL;
> + return -ENODATA;
>
> if (pcc_ss_id < 0)
> - return CPUFREQ_ETERNAL;
> + return -ENODATA;
>
> pcc_ss_data = pcc_data[pcc_ss_id];
> if (pcc_ss_data->pcc_mpar)
> latency_ns = 60 * (1000 * 1000 * 1000 / pcc_ss_data->pcc_mpar);
>
> - latency_ns = max(latency_ns, pcc_ss_data->pcc_nominal * 1000);
> - latency_ns = max(latency_ns, pcc_ss_data->pcc_mrtt * 1000);
> + latency_ns = max_t(int, latency_ns, pcc_ss_data->pcc_nominal * 1000);
> + latency_ns = max_t(int, latency_ns, pcc_ss_data->pcc_mrtt * 1000);
>
> return latency_ns;
> }
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -872,10 +872,10 @@ static void amd_pstate_update_limits(str
> */
> static u32 amd_pstate_get_transition_delay_us(unsigned int cpu)
> {
> - u32 transition_delay_ns;
> + int transition_delay_ns;
>
> transition_delay_ns = cppc_get_transition_latency(cpu);
> - if (transition_delay_ns == CPUFREQ_ETERNAL) {
> + if (transition_delay_ns < 0) {
> if (cpu_feature_enabled(X86_FEATURE_AMD_FAST_CPPC))
> return AMD_PSTATE_FAST_CPPC_TRANSITION_DELAY;
> else
> @@ -891,10 +891,10 @@ static u32 amd_pstate_get_transition_del
> */
> static u32 amd_pstate_get_transition_latency(unsigned int cpu)
> {
> - u32 transition_latency;
> + int transition_latency;
>
> transition_latency = cppc_get_transition_latency(cpu);
> - if (transition_latency == CPUFREQ_ETERNAL)
> + if (transition_latency < 0)
> return AMD_PSTATE_TRANSITION_LATENCY;
>
> return transition_latency;
> --- a/drivers/cpufreq/cppc_cpufreq.c
> +++ b/drivers/cpufreq/cppc_cpufreq.c
> @@ -310,9 +310,9 @@ static int cppc_verify_policy(struct cpu
>
> static unsigned int get_transition_latency_from_cppc(unsigned int cpu)
> {
> - unsigned int transition_latency_ns = cppc_get_transition_latency(cpu);
> + int transition_latency_ns = cppc_get_transition_latency(cpu);
>
> - if (transition_latency_ns == CPUFREQ_ETERNAL)
> + if (transition_latency_ns < 0)
> return CPUFREQ_DEFAULT_TRANSITION_LATENCY_NS / NSEC_PER_USEC;
>
> return transition_latency_ns / NSEC_PER_USEC;
> --- a/include/acpi/cppc_acpi.h
> +++ b/include/acpi/cppc_acpi.h
> @@ -160,7 +160,7 @@ extern unsigned int cppc_khz_to_perf(str
> extern bool acpi_cpc_valid(void);
> extern bool cppc_allow_fast_switch(void);
> extern int acpi_get_psd_map(unsigned int cpu, struct cppc_cpudata *cpu_data);
> -extern unsigned int cppc_get_transition_latency(int cpu);
> +extern int cppc_get_transition_latency(int cpu);
> extern bool cpc_ffh_supported(void);
> extern bool cpc_supported_by_cpu(void);
> extern int cpc_read_ffh(int cpunum, struct cpc_reg *reg, u64 *val);
> @@ -216,9 +216,9 @@ static inline bool cppc_allow_fast_switc
> {
> return false;
> }
> -static inline unsigned int cppc_get_transition_latency(int cpu)
> +static inline int cppc_get_transition_latency(int cpu)
> {
> - return CPUFREQ_ETERNAL;
> + return -ENODATA;
> }
> static inline bool cpc_ffh_supported(void)
> {
>
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 3/4] ACPI: CPPC: Do not use CPUFREQ_ETERNAL as an error value
2025-09-25 17:23 ` [PATCH v2 3/4] ACPI: CPPC: Do not use CPUFREQ_ETERNAL as an error value Rafael J. Wysocki
2025-09-25 18:33 ` Mario Limonciello (AMD) (kernel.org)
@ 2025-09-26 9:30 ` Jie Zhan
2025-09-26 10:22 ` Rafael J. Wysocki
1 sibling, 1 reply; 18+ messages in thread
From: Jie Zhan @ 2025-09-26 9:30 UTC (permalink / raw)
To: Rafael J. Wysocki, Linux PM, Mario Limonciello
Cc: Shawn Guo, Qais Yousef, LKML, Viresh Kumar, Pierre Gondois,
Linux ACPI
On 9/26/2025 1:23 AM, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Instead of using CPUFREQ_ETERNAL for signaling an error condition
> in cppc_get_transition_latency(), change the return value type of
> that function to int and make it return a proper negative error
> code on failures.
>
> No intentional functional impact.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
A minor comment inline. Other than that,
Reviewed-by: Jie Zhan <zhanjie9@hisilicon.com>
> ---
>
> v1 -> v2:
> * Change cppc_get_transition_latency() return value data type to int
> * Make it return -ENODATA on errors (Mario)
> * Update its callers accordingly
> * Adjust the subject and changelog
> * Add a missing empty code line to cppc_get_transition_latency()
>
> The modifications of this patch don't affect any other patches in the series:
>
> https://lore.kernel.org/linux-pm/8605612.T7Z3S40VBb@rafael.j.wysocki/
>
> ---
> drivers/acpi/cppc_acpi.c | 15 ++++++++-------
> drivers/cpufreq/amd-pstate.c | 8 ++++----
> drivers/cpufreq/cppc_cpufreq.c | 4 ++--
> include/acpi/cppc_acpi.h | 6 +++---
> 4 files changed, 17 insertions(+), 16 deletions(-)
>
> --- a/drivers/acpi/cppc_acpi.c
> +++ b/drivers/acpi/cppc_acpi.c
> @@ -1876,7 +1876,7 @@ EXPORT_SYMBOL_GPL(cppc_set_perf);
> * If desired_reg is in the SystemMemory or SystemIo ACPI address space,
> * then assume there is no latency.
> */
> -unsigned int cppc_get_transition_latency(int cpu_num)
> +int cppc_get_transition_latency(int cpu_num)
> {
> /*
> * Expected transition latency is based on the PCCT timing values
> @@ -1889,31 +1889,32 @@ unsigned int cppc_get_transition_latency
> * completion of a command before issuing the next command,
> * in microseconds.
> */
> - unsigned int latency_ns = 0;
> struct cpc_desc *cpc_desc;
> struct cpc_register_resource *desired_reg;
> int pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpu_num);
> struct cppc_pcc_data *pcc_ss_data;
> + int latency_ns = 0;
>
> cpc_desc = per_cpu(cpc_desc_ptr, cpu_num);
> if (!cpc_desc)
> - return CPUFREQ_ETERNAL;
> + return -ENODATA;
>
> desired_reg = &cpc_desc->cpc_regs[DESIRED_PERF];
> if (CPC_IN_SYSTEM_MEMORY(desired_reg) || CPC_IN_SYSTEM_IO(desired_reg))
> return 0;
> +
An extra line here.
Alternatively, can we cleanup a bit:
if (!CPC_IN_PCC(desired_reg) || pcc_ss_id < 0)
return -ENODATA;
> else if (!CPC_IN_PCC(desired_reg))
> - return CPUFREQ_ETERNAL;
> + return -ENODATA;
>
> if (pcc_ss_id < 0)
> - return CPUFREQ_ETERNAL;
> + return -ENODATA;
>
> pcc_ss_data = pcc_data[pcc_ss_id];
> if (pcc_ss_data->pcc_mpar)
> latency_ns = 60 * (1000 * 1000 * 1000 / pcc_ss_data->pcc_mpar);
>
> - latency_ns = max(latency_ns, pcc_ss_data->pcc_nominal * 1000);
> - latency_ns = max(latency_ns, pcc_ss_data->pcc_mrtt * 1000);
> + latency_ns = max_t(int, latency_ns, pcc_ss_data->pcc_nominal * 1000);
> + latency_ns = max_t(int, latency_ns, pcc_ss_data->pcc_mrtt * 1000);
>
> return latency_ns;
> }
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -872,10 +872,10 @@ static void amd_pstate_update_limits(str
> */
> static u32 amd_pstate_get_transition_delay_us(unsigned int cpu)
> {
> - u32 transition_delay_ns;
> + int transition_delay_ns;
>
> transition_delay_ns = cppc_get_transition_latency(cpu);
> - if (transition_delay_ns == CPUFREQ_ETERNAL) {
> + if (transition_delay_ns < 0) {
> if (cpu_feature_enabled(X86_FEATURE_AMD_FAST_CPPC))
> return AMD_PSTATE_FAST_CPPC_TRANSITION_DELAY;
> else
> @@ -891,10 +891,10 @@ static u32 amd_pstate_get_transition_del
> */
> static u32 amd_pstate_get_transition_latency(unsigned int cpu)
> {
> - u32 transition_latency;
> + int transition_latency;
>
> transition_latency = cppc_get_transition_latency(cpu);
> - if (transition_latency == CPUFREQ_ETERNAL)
> + if (transition_latency < 0)
> return AMD_PSTATE_TRANSITION_LATENCY;
>
> return transition_latency;
> --- a/drivers/cpufreq/cppc_cpufreq.c
> +++ b/drivers/cpufreq/cppc_cpufreq.c
> @@ -310,9 +310,9 @@ static int cppc_verify_policy(struct cpu
>
> static unsigned int get_transition_latency_from_cppc(unsigned int cpu)
> {
> - unsigned int transition_latency_ns = cppc_get_transition_latency(cpu);
> + int transition_latency_ns = cppc_get_transition_latency(cpu);
>
> - if (transition_latency_ns == CPUFREQ_ETERNAL)
> + if (transition_latency_ns < 0)
> return CPUFREQ_DEFAULT_TRANSITION_LATENCY_NS / NSEC_PER_USEC;
>
> return transition_latency_ns / NSEC_PER_USEC;
> --- a/include/acpi/cppc_acpi.h
> +++ b/include/acpi/cppc_acpi.h
> @@ -160,7 +160,7 @@ extern unsigned int cppc_khz_to_perf(str
> extern bool acpi_cpc_valid(void);
> extern bool cppc_allow_fast_switch(void);
> extern int acpi_get_psd_map(unsigned int cpu, struct cppc_cpudata *cpu_data);
> -extern unsigned int cppc_get_transition_latency(int cpu);
> +extern int cppc_get_transition_latency(int cpu);
> extern bool cpc_ffh_supported(void);
> extern bool cpc_supported_by_cpu(void);
> extern int cpc_read_ffh(int cpunum, struct cpc_reg *reg, u64 *val);
> @@ -216,9 +216,9 @@ static inline bool cppc_allow_fast_switc
> {
> return false;
> }
> -static inline unsigned int cppc_get_transition_latency(int cpu)
> +static inline int cppc_get_transition_latency(int cpu)
> {
> - return CPUFREQ_ETERNAL;
> + return -ENODATA;
> }
> static inline bool cpc_ffh_supported(void)
> {
>
>
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v1 2/4] cpufreq: CPPC: Avoid using CPUFREQ_ETERNAL as transition delay
2025-09-25 15:44 ` [PATCH v1 2/4] cpufreq: CPPC: Avoid using CPUFREQ_ETERNAL as transition delay Rafael J. Wysocki
2025-09-25 16:36 ` Mario Limonciello
@ 2025-09-26 9:41 ` Jie Zhan
1 sibling, 0 replies; 18+ messages in thread
From: Jie Zhan @ 2025-09-26 9:41 UTC (permalink / raw)
To: Rafael J. Wysocki, Linux PM
Cc: Shawn Guo, Qais Yousef, LKML, Viresh Kumar, Prashanth Prakash,
Pierre Gondois, Mario Limonciello, Linux ACPI
On 9/25/2025 11:44 PM, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> If cppc_get_transition_latency() returns CPUFREQ_ETERNAL to indicate a
> failure to retrieve the transition latency value from the platform
> firmware, the CPPC cpufreq driver will use that value (converted to
> microseconds) as the policy transition delay, but it is way too large
> for any practical use.
>
> Address this by making the driver use the cpufreq's default
> transition latency value (in microseconds) as the transition delay
> if CPUFREQ_ETERNAL is returned by cppc_get_transition_latency().
>
> Fixes: d4f3388afd48 ("cpufreq / CPPC: Set platform specific transition_delay_us")
> Cc: 5.19+ <stable@vger.kernel.org> # 5.19
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> drivers/cpufreq/cppc_cpufreq.c | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> --- a/drivers/cpufreq/cppc_cpufreq.c
> +++ b/drivers/cpufreq/cppc_cpufreq.c
> @@ -308,6 +308,16 @@ static int cppc_verify_policy(struct cpu
> return 0;
> }
>
> +static unsigned int get_transition_latency_from_cppc(unsigned int cpu)
Minor bit:
Can we name it as __cppc_cpufreq_get_transition_delay_us() because:
1. 'get_xxx' naming looks a bit different from other funcs in cppc_cpufreq.c
2. This is actually the main routine of cppc_cpufreq_get_transition_delay_us().
It's factored out just for the qualcomm workaround.
> +{
> + unsigned int transition_latency_ns = cppc_get_transition_latency(cpu);
> +
> + if (transition_latency_ns == CPUFREQ_ETERNAL)
> + return CPUFREQ_DEFAULT_TRANSITION_LATENCY_NS / NSEC_PER_USEC;
> +
> + return transition_latency_ns / NSEC_PER_USEC;
> +}
> +
> /*
> * The PCC subspace describes the rate at which platform can accept commands
> * on the shared PCC channel (including READs which do not count towards freq
> @@ -330,12 +340,12 @@ static unsigned int cppc_cpufreq_get_tra
> return 10000;
> }
> }
> - return cppc_get_transition_latency(cpu) / NSEC_PER_USEC;
> + return get_transition_latency_from_cppc(cpu);
> }
> #else
> static unsigned int cppc_cpufreq_get_transition_delay_us(unsigned int cpu)
> {
> - return cppc_get_transition_latency(cpu) / NSEC_PER_USEC;
> + return get_transition_latency_from_cppc(cpu);
> }
> #endif
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v1 1/4] cpufreq: Make drivers using CPUFREQ_ETERNAL specify transition latency
2025-09-25 15:44 ` [PATCH v1 1/4] cpufreq: Make drivers using CPUFREQ_ETERNAL specify transition latency Rafael J. Wysocki
2025-09-25 16:37 ` Mario Limonciello
@ 2025-09-26 9:46 ` Jie Zhan
1 sibling, 0 replies; 18+ messages in thread
From: Jie Zhan @ 2025-09-26 9:46 UTC (permalink / raw)
To: Rafael J. Wysocki, Linux PM
Cc: Shawn Guo, Qais Yousef, LKML, Viresh Kumar, Prashanth Prakash,
Pierre Gondois, Mario Limonciello, Linux ACPI
On 9/25/2025 11:44 PM, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Commit a755d0e2d41b ("cpufreq: Honour transition_latency over
> transition_delay_us") caused platforms where cpuinfo.transition_latency
> is CPUFREQ_ETERNAL to get a very large transition latency whereas
> previously it had been capped at 10 ms (and later at 2 ms).
>
> This led to a user-observable regression between 6.6 and 6.12 as
> described by Shawn:
>
> "The dbs sampling_rate was 10000 us on 6.6 and suddently becomes
> 6442450 us (4294967295 / 1000 * 1.5) on 6.12 for these platforms
> because the default transition delay was dropped [...].
>
> It slows down dbs governor's reacting to CPU loading change
> dramatically. Also, as transition_delay_us is used by schedutil
> governor as rate_limit_us, it shows a negative impact on device
> idle power consumption, because the device gets slightly less time
> in the lowest OPP."
>
> Evidently, the expectation of the drivers using CPUFREQ_ETERNAL as
> cpuinfo.transition_latency was that it would be capped by the core,
> but they may as well return a default transition latency value instead
> of CPUFREQ_ETERNAL and the core need not do anything with it.
>
> Accordingly, introduce CPUFREQ_DEFAULT_TRANSITION_LATENCY_NS and make
> all of the drivers in question use it instead of CPUFREQ_ETERNAL.
>
> Fixes: a755d0e2d41b ("cpufreq: Honour transition_latency over transition_delay_us")
> Closes: https://lore.kernel.org/linux-pm/20250922125929.453444-1-shawnguo2@yeah.net/
> Reported-by: Shawn Guo <shawnguo@kernel.org>
> Cc: 6.6+ <stable@vger.kernel.org> # 6.6+
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Thanks, we've seen similar issues.
Reviewed-by: Jie Zhan <zhanjie9@hisilicon.com>
> ---
> drivers/cpufreq/cpufreq-dt.c | 2 +-
> drivers/cpufreq/imx6q-cpufreq.c | 2 +-
> drivers/cpufreq/mediatek-cpufreq-hw.c | 2 +-
> drivers/cpufreq/scmi-cpufreq.c | 2 +-
> drivers/cpufreq/scpi-cpufreq.c | 2 +-
> drivers/cpufreq/spear-cpufreq.c | 2 +-
> include/linux/cpufreq.h | 3 +++
> 7 files changed, 9 insertions(+), 6 deletions(-)
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v1 4/4] cpufreq: Drop unused symbol CPUFREQ_ETERNAL
2025-09-25 15:47 ` [PATCH v1 4/4] cpufreq: Drop unused symbol CPUFREQ_ETERNAL Rafael J. Wysocki
2025-09-25 16:36 ` Mario Limonciello (AMD) (kernel.org)
@ 2025-09-26 9:47 ` Jie Zhan
1 sibling, 0 replies; 18+ messages in thread
From: Jie Zhan @ 2025-09-26 9:47 UTC (permalink / raw)
To: Rafael J. Wysocki, Linux PM
Cc: Shawn Guo, Qais Yousef, LKML, Viresh Kumar, Prashanth Prakash,
Pierre Gondois, Mario Limonciello, Linux ACPI
On 9/25/2025 11:47 PM, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Drop CPUFREQ_ETERNAL that has no users any more along with all
> references to it in the documentation.
>
> No functional impact.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Jie Zhan <zhanjie9@hisilicon.com>
> ---
> Documentation/admin-guide/pm/cpufreq.rst | 4 ----
> Documentation/cpu-freq/cpu-drivers.rst | 3 +--
> Documentation/translations/zh_CN/cpu-freq/cpu-drivers.rst | 3 +--
> Documentation/translations/zh_TW/cpu-freq/cpu-drivers.rst | 3 +--
> include/linux/cpufreq.h | 5 -----
> 5 files changed, 3 insertions(+), 15 deletions(-)
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 3/4] ACPI: CPPC: Do not use CPUFREQ_ETERNAL as an error value
2025-09-26 9:30 ` Jie Zhan
@ 2025-09-26 10:22 ` Rafael J. Wysocki
0 siblings, 0 replies; 18+ messages in thread
From: Rafael J. Wysocki @ 2025-09-26 10:22 UTC (permalink / raw)
To: Jie Zhan
Cc: Rafael J. Wysocki, Linux PM, Mario Limonciello, Shawn Guo,
Qais Yousef, LKML, Viresh Kumar, Pierre Gondois, Linux ACPI
On Fri, Sep 26, 2025 at 11:30 AM Jie Zhan <zhanjie9@hisilicon.com> wrote:
>
>
>
> On 9/26/2025 1:23 AM, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > Instead of using CPUFREQ_ETERNAL for signaling an error condition
> > in cppc_get_transition_latency(), change the return value type of
> > that function to int and make it return a proper negative error
> > code on failures.
> >
> > No intentional functional impact.
> >
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> A minor comment inline. Other than that,
> Reviewed-by: Jie Zhan <zhanjie9@hisilicon.com>
Thanks!
> > ---
> >
> > v1 -> v2:
> > * Change cppc_get_transition_latency() return value data type to int
> > * Make it return -ENODATA on errors (Mario)
> > * Update its callers accordingly
> > * Adjust the subject and changelog
> > * Add a missing empty code line to cppc_get_transition_latency()
> >
> > The modifications of this patch don't affect any other patches in the series:
> >
> > https://lore.kernel.org/linux-pm/8605612.T7Z3S40VBb@rafael.j.wysocki/
> >
> > ---
> > drivers/acpi/cppc_acpi.c | 15 ++++++++-------
> > drivers/cpufreq/amd-pstate.c | 8 ++++----
> > drivers/cpufreq/cppc_cpufreq.c | 4 ++--
> > include/acpi/cppc_acpi.h | 6 +++---
> > 4 files changed, 17 insertions(+), 16 deletions(-)
> >
> > --- a/drivers/acpi/cppc_acpi.c
> > +++ b/drivers/acpi/cppc_acpi.c
> > @@ -1876,7 +1876,7 @@ EXPORT_SYMBOL_GPL(cppc_set_perf);
> > * If desired_reg is in the SystemMemory or SystemIo ACPI address space,
> > * then assume there is no latency.
> > */
> > -unsigned int cppc_get_transition_latency(int cpu_num)
> > +int cppc_get_transition_latency(int cpu_num)
> > {
> > /*
> > * Expected transition latency is based on the PCCT timing values
> > @@ -1889,31 +1889,32 @@ unsigned int cppc_get_transition_latency
> > * completion of a command before issuing the next command,
> > * in microseconds.
> > */
> > - unsigned int latency_ns = 0;
> > struct cpc_desc *cpc_desc;
> > struct cpc_register_resource *desired_reg;
> > int pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpu_num);
> > struct cppc_pcc_data *pcc_ss_data;
> > + int latency_ns = 0;
> >
> > cpc_desc = per_cpu(cpc_desc_ptr, cpu_num);
> > if (!cpc_desc)
> > - return CPUFREQ_ETERNAL;
> > + return -ENODATA;
> >
> > desired_reg = &cpc_desc->cpc_regs[DESIRED_PERF];
> > if (CPC_IN_SYSTEM_MEMORY(desired_reg) || CPC_IN_SYSTEM_IO(desired_reg))
> > return 0;
> > +
> An extra line here.
Yes, and it is intentional (see the changes between v1 and v2).
> Alternatively, can we cleanup a bit:
That can be done.
> if (!CPC_IN_PCC(desired_reg) || pcc_ss_id < 0)
> return -ENODATA;
>
> > else if (!CPC_IN_PCC(desired_reg))
> > - return CPUFREQ_ETERNAL;
> > + return -ENODATA;
> >
> > if (pcc_ss_id < 0)
> > - return CPUFREQ_ETERNAL;
> > + return -ENODATA;
> >
> > pcc_ss_data = pcc_data[pcc_ss_id];
> > if (pcc_ss_data->pcc_mpar)
> > latency_ns = 60 * (1000 * 1000 * 1000 / pcc_ss_data->pcc_mpar);
> >
> > - latency_ns = max(latency_ns, pcc_ss_data->pcc_nominal * 1000);
> > - latency_ns = max(latency_ns, pcc_ss_data->pcc_mrtt * 1000);
> > + latency_ns = max_t(int, latency_ns, pcc_ss_data->pcc_nominal * 1000);
> > + latency_ns = max_t(int, latency_ns, pcc_ss_data->pcc_mrtt * 1000);
> >
> > return latency_ns;
> > }
> > --- a/drivers/cpufreq/amd-pstate.c
> > +++ b/drivers/cpufreq/amd-pstate.c
> > @@ -872,10 +872,10 @@ static void amd_pstate_update_limits(str
> > */
> > static u32 amd_pstate_get_transition_delay_us(unsigned int cpu)
> > {
> > - u32 transition_delay_ns;
> > + int transition_delay_ns;
> >
> > transition_delay_ns = cppc_get_transition_latency(cpu);
> > - if (transition_delay_ns == CPUFREQ_ETERNAL) {
> > + if (transition_delay_ns < 0) {
> > if (cpu_feature_enabled(X86_FEATURE_AMD_FAST_CPPC))
> > return AMD_PSTATE_FAST_CPPC_TRANSITION_DELAY;
> > else
> > @@ -891,10 +891,10 @@ static u32 amd_pstate_get_transition_del
> > */
> > static u32 amd_pstate_get_transition_latency(unsigned int cpu)
> > {
> > - u32 transition_latency;
> > + int transition_latency;
> >
> > transition_latency = cppc_get_transition_latency(cpu);
> > - if (transition_latency == CPUFREQ_ETERNAL)
> > + if (transition_latency < 0)
> > return AMD_PSTATE_TRANSITION_LATENCY;
> >
> > return transition_latency;
> > --- a/drivers/cpufreq/cppc_cpufreq.c
> > +++ b/drivers/cpufreq/cppc_cpufreq.c
> > @@ -310,9 +310,9 @@ static int cppc_verify_policy(struct cpu
> >
> > static unsigned int get_transition_latency_from_cppc(unsigned int cpu)
> > {
> > - unsigned int transition_latency_ns = cppc_get_transition_latency(cpu);
> > + int transition_latency_ns = cppc_get_transition_latency(cpu);
> >
> > - if (transition_latency_ns == CPUFREQ_ETERNAL)
> > + if (transition_latency_ns < 0)
> > return CPUFREQ_DEFAULT_TRANSITION_LATENCY_NS / NSEC_PER_USEC;
> >
> > return transition_latency_ns / NSEC_PER_USEC;
> > --- a/include/acpi/cppc_acpi.h
> > +++ b/include/acpi/cppc_acpi.h
> > @@ -160,7 +160,7 @@ extern unsigned int cppc_khz_to_perf(str
> > extern bool acpi_cpc_valid(void);
> > extern bool cppc_allow_fast_switch(void);
> > extern int acpi_get_psd_map(unsigned int cpu, struct cppc_cpudata *cpu_data);
> > -extern unsigned int cppc_get_transition_latency(int cpu);
> > +extern int cppc_get_transition_latency(int cpu);
> > extern bool cpc_ffh_supported(void);
> > extern bool cpc_supported_by_cpu(void);
> > extern int cpc_read_ffh(int cpunum, struct cpc_reg *reg, u64 *val);
> > @@ -216,9 +216,9 @@ static inline bool cppc_allow_fast_switc
> > {
> > return false;
> > }
> > -static inline unsigned int cppc_get_transition_latency(int cpu)
> > +static inline int cppc_get_transition_latency(int cpu)
> > {
> > - return CPUFREQ_ETERNAL;
> > + return -ENODATA;
> > }
> > static inline bool cpc_ffh_supported(void)
> > {
> >
> >
> >
> >
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v1 0/4] cpufreq: Fixes and cleanups related to CPUFREQ_ETERNAL
2025-09-25 15:42 [PATCH v1 0/4] cpufreq: Fixes and cleanups related to CPUFREQ_ETERNAL Rafael J. Wysocki
` (3 preceding siblings ...)
2025-09-25 15:47 ` [PATCH v1 4/4] cpufreq: Drop unused symbol CPUFREQ_ETERNAL Rafael J. Wysocki
@ 2025-09-29 7:28 ` Viresh Kumar
4 siblings, 0 replies; 18+ messages in thread
From: Viresh Kumar @ 2025-09-29 7:28 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Linux PM, Shawn Guo, Qais Yousef, LKML, Prashanth Prakash,
Pierre Gondois, Mario Limonciello, Linux ACPI
On 25-09-25, 17:42, Rafael J. Wysocki wrote:
> Hi All,
>
> The first patch in this series is meant to address the failure discussed in
>
> https://lore.kernel.org/linux-pm/20250922125929.453444-1-shawnguo2@yeah.net/
>
> but in a different way than proposed by Shawn.
>
> The second one is a CPPC cpufreq driver fix preventing it from using an
> overly large transition delay in the cases when that delay cannot be
> obtained from the platform firmware.
>
> Patch [3/4] makes CPPC use a specific symbol instead of CPUFREQ_ETERNAL for
> signaling error conditions while attempting to retrieve a transition latency
> value from the platform firmware.
>
> The last patch removes CPUFREQ_ETERNAL (which has no users any more) from
> cpufreq, including all references to it in cpufreq documentation.
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
--
viresh
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2025-09-29 7:28 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-25 15:42 [PATCH v1 0/4] cpufreq: Fixes and cleanups related to CPUFREQ_ETERNAL Rafael J. Wysocki
2025-09-25 15:44 ` [PATCH v1 1/4] cpufreq: Make drivers using CPUFREQ_ETERNAL specify transition latency Rafael J. Wysocki
2025-09-25 16:37 ` Mario Limonciello
2025-09-26 9:46 ` Jie Zhan
2025-09-25 15:44 ` [PATCH v1 2/4] cpufreq: CPPC: Avoid using CPUFREQ_ETERNAL as transition delay Rafael J. Wysocki
2025-09-25 16:36 ` Mario Limonciello
2025-09-26 9:41 ` Jie Zhan
2025-09-25 15:46 ` [PATCH v1 3/4] ACPI: CPPC: Replace CPUFREQ_ETERNAL with CPPC-specific symbol Rafael J. Wysocki
2025-09-25 16:35 ` Mario Limonciello
2025-09-25 16:57 ` Rafael J. Wysocki
2025-09-25 17:23 ` [PATCH v2 3/4] ACPI: CPPC: Do not use CPUFREQ_ETERNAL as an error value Rafael J. Wysocki
2025-09-25 18:33 ` Mario Limonciello (AMD) (kernel.org)
2025-09-26 9:30 ` Jie Zhan
2025-09-26 10:22 ` Rafael J. Wysocki
2025-09-25 15:47 ` [PATCH v1 4/4] cpufreq: Drop unused symbol CPUFREQ_ETERNAL Rafael J. Wysocki
2025-09-25 16:36 ` Mario Limonciello (AMD) (kernel.org)
2025-09-26 9:47 ` Jie Zhan
2025-09-29 7:28 ` [PATCH v1 0/4] cpufreq: Fixes and cleanups related to CPUFREQ_ETERNAL Viresh Kumar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox