Re: [PATCH 2/2] ACPI: processor: Add support for ACPI C4 state

Mario Limonciello
2025-09-16T18:55:26Z

On 9/16/25 6:24 AM, Rafael J. Wysocki wrote:
> On Mon, Sep 15, 2025 at 10:43 PM Mario Limonciello (AMD)
> <superm1@kernel.org> wrote:
>>
>> The _CST object described in the ACPI specification [1] allows processors
>> to support any arbitrary number of power states that are described by
>> a package describing the register used, the type of C-state, latency
>> and the power consumption.
>>
>> Currently the Linux kernel supports up to ACPI C3, and if a system
>> supports any further states they are ignored. This causes problems on
>> some AMD hardware which can support up to ACPI C4.
>>
>> AMD systems that support up to C3 will enter CPUOFF and VDDOFF
>> when entering C3. Systems that support up to C4 will enter CPUOFF
>> when going to C3 and will enter VDDOFF when entering into C4.
>>
>> The existing semantics for bus mastering around C3 are also valid for C4,
>> so instead of hardcoding to C3, map then >= C3. In the case of s2idle
>> detect the deepest C-state supported and enter the deepest.
>>
>> Link: https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/08_Processor_Configuration_and_Control/declaring-processors.html?#cst-c-states [1]
>> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
>> ---
>> drivers/acpi/processor_idle.c | 10 +++++++---
>> include/acpi/actypes.h | 5 +++--
>> 2 files changed, 10 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
>> index 5dacf41d7cc0a..537b0119535ea 100644
>> --- a/drivers/acpi/processor_idle.c
>> +++ b/drivers/acpi/processor_idle.c
>> @@ -444,6 +444,10 @@ static int acpi_processor_power_verify(struct acpi_processor *pr)
>> case ACPI_STATE_C3:
>> acpi_processor_power_verify_c3(pr, cx);
>> break;
>> + case ACPI_STATE_C4:
>> + if (!cx->latency || !cx->address)
>> + break;
>> + cx->valid = 1;
>> }
>> if (!cx->valid)
>> continue;
>> @@ -685,7 +689,7 @@ static int __cpuidle acpi_idle_enter(struct cpuidle_device *dev,
>> return -EINVAL;
>>
>> if (cx->type != ACPI_STATE_C1) {
>> - if (cx->type == ACPI_STATE_C3 && pr->flags.bm_check)
>> + if (cx->type >= ACPI_STATE_C3 && pr->flags.bm_check)
>> return acpi_idle_enter_bm(drv, pr, cx, index);
>>
>> /* C2 to C1 demotion. */
>> @@ -708,7 +712,7 @@ static int __cpuidle acpi_idle_enter_s2idle(struct cpuidle_device *dev,
>> {
>> struct acpi_processor_cx *cx = per_cpu(acpi_cstate[index], dev->cpu);
>>
>> - if (cx->type == ACPI_STATE_C3) {
>> + if (cx->index == drv->state_count - 1) {
>> struct acpi_processor *pr = __this_cpu_read(processors);
>>
>> if (unlikely(!pr))
>> @@ -754,7 +758,7 @@ static int acpi_processor_setup_cpuidle_cx(struct acpi_processor *pr,
>> if (lapic_timer_needs_broadcast(pr, cx))
>> state->flags |= CPUIDLE_FLAG_TIMER_STOP;
>>
>> - if (cx->type == ACPI_STATE_C3) {
>> + if (cx->type >= ACPI_STATE_C3) {
>> state->flags |= CPUIDLE_FLAG_TLB_FLUSHED;
>> if (pr->flags.bm_check)
>> state->flags |= CPUIDLE_FLAG_RCU_IDLE;
>> diff --git a/include/acpi/actypes.h b/include/acpi/actypes.h
>> index 8fe893d776dde..6c9f472af482c 100644
>> --- a/include/acpi/actypes.h
>> +++ b/include/acpi/actypes.h
>> @@ -600,8 +600,9 @@ typedef u64 acpi_integer;
>> #define ACPI_STATE_C1 (u8) 1
>> #define ACPI_STATE_C2 (u8) 2
>> #define ACPI_STATE_C3 (u8) 3
>> -#define ACPI_C_STATES_MAX ACPI_STATE_C3
>> -#define ACPI_C_STATE_COUNT 4
>> +#define ACPI_STATE_C4 (u8) 4
>> +#define ACPI_C_STATES_MAX ACPI_STATE_C4
>> +#define ACPI_C_STATE_COUNT 5
>
> Don't you need to update ACPICA accordingly?

Thanks, I've got a PR opened.

>
>>
>> /*
>> * Sleep type invalid value
>> --
>
> The current point in the cycle is kind of unsuitable for changes like
> this. I'd prefer to revisit it after 6.18-rc1.

OK! I wanted this out a few weeks ago but was waiting for positive Q/A
results.

I'll rebase/resend after 6.18-rc1.