Re: [PATCH RESEND] ACPI: processor: Use Apple-specific methods for P/C-states

[Rafael J. Wysocki (Intel)]

On Thu, Oct 15, 2026 at 10:45 PM Atharva Tiwari
<atharvatiwarilinuxdev@xxxxxxxxx> wrote:
>
> Use APSS and ACST instead of _PSS and _CST when available on Intel Macs.
> These methods are used in X86PlatformPlugin.kext, and when not found we
> fallback to generic _PSS and _CST methods.
>
> Signed-off-by: Atharva Tiwari <atharvatiwarilinuxdev@xxxxxxxxx>
> ---
>  drivers/acpi/acpi_processor.c    | 22 +++++++++++++++++-----
>  drivers/acpi/processor_perflib.c | 10 ++++++++++
>  2 files changed, 27 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c
> index 5fbab5417..782676ef7 100644
> --- a/drivers/acpi/acpi_processor.c
> +++ b/drivers/acpi/acpi_processor.c
> @@ -18,6 +18,7 @@
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/pci.h>
> +#include <linux/platform_data/x86/apple.h>
>  #include <linux/platform_device.h>
>
>  #include <acpi/processor.h>
> @@ -837,27 +838,38 @@ int acpi_processor_evaluate_cst(acpi_handle handle, u32 cpu,
>         acpi_status status;
>         u64 count;
>         int last_index = 0;
> -       int i, ret = 0;
> +       int i, ret = 0, base = 0;
>
> +       if (x86_apple_machine) {
> +               status = acpi_evaluate_object(handle, "ACST", NULL, &buffer);
> +               if (ACPI_SUCCESS(status)) {
> +                       base = 1; /* ACST is a 4 field variable we need to skip the first field */

s/variable/package/ in the comment.

> +                       goto success;
> +               }
> +
> +               buffer.length = ACPI_ALLOCATE_BUFFER;
> +               buffer.pointer = NULL;
> +       }

I'd put the above into a separate function returning 1 on success and
0 on failure, say acpi_processor_apple_acst(), that would also reset
the buffer on failures, so the code in acpi_processor_evaluate_cst()
can be something like

base = acpi_processor_apple_acst(handle, &buffer);
if (!base) {

>         status = acpi_evaluate_object(handle, "_CST", NULL, &buffer);
>         if (ACPI_FAILURE(status)) {
>                 acpi_handle_debug(handle, "No _CST\n");
>                 return -ENODEV;
>         }
> +success:
>
>         cst = buffer.pointer;
>
>         /* There must be at least 2 elements. */

The above comment is not applicable to the ACST case, is it?

> -       if (!cst || cst->type != ACPI_TYPE_PACKAGE || cst->package.count < 2) {
> +       if (!cst || cst->type != ACPI_TYPE_PACKAGE || cst->package.count < base + 2) {
>                 acpi_handle_warn(handle, "Invalid _CST output\n");

You should also avoid printing "_CST" in this message if _CST is not
actually used (and analogously below).

>                 ret = -EFAULT;
>                 goto end;
>         }
>
> -       count = cst->package.elements[0].integer.value;
> +       count = cst->package.elements[base].integer.value;
>
>         /* Validate the number of C-states. */
> -       if (count < 1 || count != cst->package.count - 1) {
> +       if (count < 1 || count != cst->package.count - 1 - base) {
>                 acpi_handle_warn(handle, "Inconsistent _CST data\n");
>                 ret = -EFAULT;
>                 goto end;
> @@ -882,7 +894,7 @@ int acpi_processor_evaluate_cst(acpi_handle handle, u32 cpu,
>
>                 memset(&cx, 0, sizeof(cx));
>
> -               element = &cst->package.elements[i];
> +               element = &cst->package.elements[base + i];
>                 if (element->type != ACPI_TYPE_PACKAGE) {
>                         acpi_handle_info(handle, "_CST C%d type(%x) is not package, skip...\n",
>                                          i, element->type);
> diff --git a/drivers/acpi/processor_perflib.c b/drivers/acpi/processor_perflib.c
> index 9e25d6124..b4b36e60d 100644
> --- a/drivers/acpi/processor_perflib.c
> +++ b/drivers/acpi/processor_perflib.c
> @@ -17,6 +17,7 @@
>  #include <linux/cpufreq.h>
>  #include <linux/slab.h>
>  #include <linux/acpi.h>
> +#include <linux/platform_data/x86/apple.h>
>  #include <acpi/processor.h>
>  #ifdef CONFIG_X86
>  #include <asm/cpufeature.h>
> @@ -324,11 +325,20 @@ static int acpi_processor_get_performance_states(struct acpi_processor *pr)
>         int i;
>         int last_invalid = -1;
>
> +       if (x86_apple_machine) {
> +               status = acpi_evaluate_object(pr->handle, "APSS", NULL, &buffer);
> +               if (ACPI_SUCCESS(status))
> +                       goto success;

In analogy with the above, I'd introduce acpi_evaluate_apple_apss(),
so the code here can be written as

if (!acpi_evaluate_apple_apss(pr->handle, &buffer)) {
        status = acpi_evaluate_object(pr->handle, "_PSS", NULL, &buffer);

etc.

> +
> +               buffer.length = ACPI_ALLOCATE_BUFFER;
> +               buffer.pointer = NULL;
> +       }
>         status = acpi_evaluate_object(pr->handle, "_PSS", NULL, &buffer);
>         if (ACPI_FAILURE(status)) {
>                 acpi_evaluation_failure_warn(pr->handle, "_PSS", status);
>                 return -ENODEV;
>         }
> +success:
>
>         pss = buffer.pointer;
>         if (!pss || pss->type != ACPI_TYPE_PACKAGE) {
> --

And there are diagnostic messages below mentioning _PSS directly that
should not mention it when it is not actually used.