[PATCH v3] ACPI / idle: override c-state latency when not in conformance with s0ix

[PATCH v3] ACPI / idle: override c-state latency when not in conformance with s0ix

[Mario Limonciello]

Generally the C-state latency is provided by the _CST method or FADT but
some OEM platforms using AMD Picasso, Renoir, Van Gogh, and Cezanne set
the C2 latency greater than C3's which causes the C2 state to be skipped.
That will block the core entering PC6, which prevents s0ix working
properly on Linux systems.

In other operating systems the latency values are not validated and this
does not cause problems by skipping states.

To avoid this issue happening on Linux, detect when latencies are not an
arithmetic progression and sort them.

Link: https://gitlab.freedesktop.org/agd5f/linux/-/commit/026d186e4592c1ee9c1cb44295912d0294508725
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1230#note_712174
Suggested-by: Prike Liang <Prike.Liang@amd.com>
Suggested-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/acpi/processor_idle.c | 40 +++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index 4e2d76b8b697..6790df5a2462 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -16,6 +16,7 @@
 #include <linux/acpi.h>
 #include <linux/dmi.h>
 #include <linux/sched.h>       /* need_resched() */
+#include <linux/sort.h>
 #include <linux/tick.h>
 #include <linux/cpuidle.h>
 #include <linux/cpu.h>
@@ -388,10 +389,37 @@ static void acpi_processor_power_verify_c3(struct acpi_processor *pr,
 	return;
 }
 
+static int acpi_cst_latency_cmp(const void *a, const void *b)
+{
+	const struct acpi_processor_cx *x = a, *y = b;
+
+	if (!(x->valid && y->valid))
+		return 0;
+	if (x->latency > y->latency)
+		return 1;
+	if (x->latency < y->latency)
+		return -1;
+	return 0;
+}
+static void acpi_cst_latency_swap(void *a, void *b, int n)
+{
+	struct acpi_processor_cx *x = a, *y = b;
+	u32 tmp;
+
+	if (!(x->valid && y->valid))
+		return;
+	tmp = x->latency;
+	x->latency = y->latency;
+	y->latency = tmp;
+}
+
 static int acpi_processor_power_verify(struct acpi_processor *pr)
 {
 	unsigned int i;
 	unsigned int working = 0;
+	unsigned int last_latency = 0;
+	unsigned int last_type = 0;
+	bool buggy_latency = false;
 
 	pr->power.timer_broadcast_on_state = INT_MAX;
 
@@ -415,12 +443,24 @@ static int acpi_processor_power_verify(struct acpi_processor *pr)
 		}
 		if (!cx->valid)
 			continue;
+		if (cx->type >= last_type && cx->latency < last_latency)
+			buggy_latency = true;
+		last_latency = cx->latency;
+		last_type = cx->type;
 
 		lapic_timer_check_state(i, pr, cx);
 		tsc_check_state(cx->type);
 		working++;
 	}
 
+	if (buggy_latency) {
+		pr_notice("FW issue: working around C-state latencies out of order\n");
+		sort(&pr->power.states[1], max_cstate,
+		     sizeof(struct acpi_processor_cx),
+		     acpi_cst_latency_cmp,
+		     acpi_cst_latency_swap);
+	}
+
 	lapic_timer_propagate_broadcast(pr);
 
 	return (working);
-- 
2.25.1

Re: [PATCH v3] ACPI / idle: override c-state latency when not in conformance with s0ix

[Rafael J. Wysocki]

On Thu, May 13, 2021 at 1:12 AM Mario Limonciello
<mario.limonciello@amd.com> wrote:
>
> Generally the C-state latency is provided by the _CST method or FADT but
> some OEM platforms using AMD Picasso, Renoir, Van Gogh, and Cezanne set
> the C2 latency greater than C3's which causes the C2 state to be skipped.
> That will block the core entering PC6, which prevents s0ix working
> properly on Linux systems.
>
> In other operating systems the latency values are not validated and this
> does not cause problems by skipping states.
>
> To avoid this issue happening on Linux, detect when latencies are not an
> arithmetic progression and sort them.
>
> Link: https://gitlab.freedesktop.org/agd5f/linux/-/commit/026d186e4592c1ee9c1cb44295912d0294508725
> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1230#note_712174
> Suggested-by: Prike Liang <Prike.Liang@amd.com>
> Suggested-by: Alex Deucher <alexander.deucher@amd.com>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>

Applied as 5.14 material under a different subject ("ACPI: processor
idle: Fix up C-state latency if not ordered") and with some edits in
the changelog.

Thanks!

> ---
>  drivers/acpi/processor_idle.c | 40 +++++++++++++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
>
> diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
> index 4e2d76b8b697..6790df5a2462 100644
> --- a/drivers/acpi/processor_idle.c
> +++ b/drivers/acpi/processor_idle.c
> @@ -16,6 +16,7 @@
>  #include <linux/acpi.h>
>  #include <linux/dmi.h>
>  #include <linux/sched.h>       /* need_resched() */
> +#include <linux/sort.h>
>  #include <linux/tick.h>
>  #include <linux/cpuidle.h>
>  #include <linux/cpu.h>
> @@ -388,10 +389,37 @@ static void acpi_processor_power_verify_c3(struct acpi_processor *pr,
>         return;
>  }
>
> +static int acpi_cst_latency_cmp(const void *a, const void *b)
> +{
> +       const struct acpi_processor_cx *x = a, *y = b;
> +
> +       if (!(x->valid && y->valid))
> +               return 0;
> +       if (x->latency > y->latency)
> +               return 1;
> +       if (x->latency < y->latency)
> +               return -1;
> +       return 0;
> +}
> +static void acpi_cst_latency_swap(void *a, void *b, int n)
> +{
> +       struct acpi_processor_cx *x = a, *y = b;
> +       u32 tmp;
> +
> +       if (!(x->valid && y->valid))
> +               return;
> +       tmp = x->latency;
> +       x->latency = y->latency;
> +       y->latency = tmp;
> +}
> +
>  static int acpi_processor_power_verify(struct acpi_processor *pr)
>  {
>         unsigned int i;
>         unsigned int working = 0;
> +       unsigned int last_latency = 0;
> +       unsigned int last_type = 0;
> +       bool buggy_latency = false;
>
>         pr->power.timer_broadcast_on_state = INT_MAX;
>
> @@ -415,12 +443,24 @@ static int acpi_processor_power_verify(struct acpi_processor *pr)
>                 }
>                 if (!cx->valid)
>                         continue;
> +               if (cx->type >= last_type && cx->latency < last_latency)
> +                       buggy_latency = true;
> +               last_latency = cx->latency;
> +               last_type = cx->type;
>
>                 lapic_timer_check_state(i, pr, cx);
>                 tsc_check_state(cx->type);
>                 working++;
>         }
>
> +       if (buggy_latency) {
> +               pr_notice("FW issue: working around C-state latencies out of order\n");
> +               sort(&pr->power.states[1], max_cstate,
> +                    sizeof(struct acpi_processor_cx),
> +                    acpi_cst_latency_cmp,
> +                    acpi_cst_latency_swap);
> +       }
> +
>         lapic_timer_propagate_broadcast(pr);
>
>         return (working);
> --
> 2.25.1
>