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

[Mario Limonciello <superm1@kernel.org>]

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.