2023-01-19 07:28:54

by Álvaro Fernández Rojas

[permalink] [raw]
Subject: [PATCH] intel_idle: Add RaptorLake support

This patch adds RaptorLake support to the intel_idle driver.

Since RaptorLake and AlderLake C-state are characteristics the same, we use
AlderLake C-states tables for RaptorLake as well.

Signed-off-by: Álvaro Fernández Rojas <[email protected]>
---
drivers/idle/intel_idle.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
index cfeb24d40d37..1a35b98d9bae 100644
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -1429,6 +1429,9 @@ static const struct x86_cpu_id intel_idle_ids[] __initconst = {
X86_MATCH_INTEL_FAM6_MODEL(ALDERLAKE, &idle_cpu_adl),
X86_MATCH_INTEL_FAM6_MODEL(ALDERLAKE_L, &idle_cpu_adl_l),
X86_MATCH_INTEL_FAM6_MODEL(ALDERLAKE_N, &idle_cpu_adl_n),
+ X86_MATCH_INTEL_FAM6_MODEL(RAPTORLAKE, &idle_cpu_adl),
+ X86_MATCH_INTEL_FAM6_MODEL(RAPTORLAKE_P, &idle_cpu_adl_n),
+ X86_MATCH_INTEL_FAM6_MODEL(RAPTORLAKE_S, &idle_cpu_adl),
X86_MATCH_INTEL_FAM6_MODEL(SAPPHIRERAPIDS_X, &idle_cpu_spr),
X86_MATCH_INTEL_FAM6_MODEL(XEON_PHI_KNL, &idle_cpu_knl),
X86_MATCH_INTEL_FAM6_MODEL(XEON_PHI_KNM, &idle_cpu_knl),
@@ -1867,6 +1870,9 @@ static void __init intel_idle_init_cstates_icpu(struct cpuidle_driver *drv)
case INTEL_FAM6_ALDERLAKE:
case INTEL_FAM6_ALDERLAKE_L:
case INTEL_FAM6_ALDERLAKE_N:
+ case INTEL_FAM6_RAPTORLAKE:
+ case INTEL_FAM6_RAPTORLAKE_P:
+ case INTEL_FAM6_RAPTORLAKE_S:
adl_idle_state_table_update();
break;
}
--
2.34.1


2023-01-19 14:25:57

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH] intel_idle: Add RaptorLake support

On Thu, Jan 19, 2023 at 8:02 AM Álvaro Fernández Rojas
<[email protected]> wrote:
>
> This patch adds RaptorLake support to the intel_idle driver.
>
> Since RaptorLake and AlderLake C-state are characteristics the same, we use
> AlderLake C-states tables for RaptorLake as well.
>
> Signed-off-by: Álvaro Fernández Rojas <[email protected]>

Rui, Artem, what do you think?

> ---
> drivers/idle/intel_idle.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
> index cfeb24d40d37..1a35b98d9bae 100644
> --- a/drivers/idle/intel_idle.c
> +++ b/drivers/idle/intel_idle.c
> @@ -1429,6 +1429,9 @@ static const struct x86_cpu_id intel_idle_ids[] __initconst = {
> X86_MATCH_INTEL_FAM6_MODEL(ALDERLAKE, &idle_cpu_adl),
> X86_MATCH_INTEL_FAM6_MODEL(ALDERLAKE_L, &idle_cpu_adl_l),
> X86_MATCH_INTEL_FAM6_MODEL(ALDERLAKE_N, &idle_cpu_adl_n),
> + X86_MATCH_INTEL_FAM6_MODEL(RAPTORLAKE, &idle_cpu_adl),
> + X86_MATCH_INTEL_FAM6_MODEL(RAPTORLAKE_P, &idle_cpu_adl_n),
> + X86_MATCH_INTEL_FAM6_MODEL(RAPTORLAKE_S, &idle_cpu_adl),
> X86_MATCH_INTEL_FAM6_MODEL(SAPPHIRERAPIDS_X, &idle_cpu_spr),
> X86_MATCH_INTEL_FAM6_MODEL(XEON_PHI_KNL, &idle_cpu_knl),
> X86_MATCH_INTEL_FAM6_MODEL(XEON_PHI_KNM, &idle_cpu_knl),
> @@ -1867,6 +1870,9 @@ static void __init intel_idle_init_cstates_icpu(struct cpuidle_driver *drv)
> case INTEL_FAM6_ALDERLAKE:
> case INTEL_FAM6_ALDERLAKE_L:
> case INTEL_FAM6_ALDERLAKE_N:
> + case INTEL_FAM6_RAPTORLAKE:
> + case INTEL_FAM6_RAPTORLAKE_P:
> + case INTEL_FAM6_RAPTORLAKE_S:
> adl_idle_state_table_update();
> break;
> }
> --
> 2.34.1
>

2023-01-19 17:13:10

by Zhang, Rui

[permalink] [raw]
Subject: Re: [PATCH] intel_idle: Add RaptorLake support

On Thu, 2023-01-19 at 08:02 +0100, Álvaro Fernández Rojas wrote:
> This patch adds RaptorLake support to the intel_idle driver.
>
> Since RaptorLake and AlderLake C-state are characteristics the same,
> we use
> AlderLake C-states tables for RaptorLake as well.

RPL and ADL have same cstates and use the same mwait hints, but the
latency of each c-state are still different on different platforms.
So we can not just duplicate the ADL table on RPL.

There is an effort ongoing that measures the latency of each
cstate on the RPL platforms. And based on the measurement result, we
can decide if a new custom table is needed or we can just copy the
previous platform. Hopefully we will have a patch in a couple of weeks.

thanks,
rui
>
> Signed-off-by: Álvaro Fernández Rojas <[email protected]>
> ---
> drivers/idle/intel_idle.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
> index cfeb24d40d37..1a35b98d9bae 100644
> --- a/drivers/idle/intel_idle.c
> +++ b/drivers/idle/intel_idle.c
> @@ -1429,6 +1429,9 @@ static const struct x86_cpu_id intel_idle_ids[]
> __initconst = {
> X86_MATCH_INTEL_FAM6_MODEL(ALDERLAKE, &idle_cpu_adl
> ),
> X86_MATCH_INTEL_FAM6_MODEL(ALDERLAKE_L, &idle_cpu_adl
> _l),
> X86_MATCH_INTEL_FAM6_MODEL(ALDERLAKE_N, &idle_cpu_adl
> _n),
> + X86_MATCH_INTEL_FAM6_MODEL(RAPTORLAKE, &idle_cpu_adl
> ),
> + X86_MATCH_INTEL_FAM6_MODEL(RAPTORLAKE_P, &idle_cpu_adl_n),
> + X86_MATCH_INTEL_FAM6_MODEL(RAPTORLAKE_S, &idle_cpu_adl),
> X86_MATCH_INTEL_FAM6_MODEL(SAPPHIRERAPIDS_X, &idle_cpu_spr
> ),
> X86_MATCH_INTEL_FAM6_MODEL(XEON_PHI_KNL, &idle_cpu_knl),
> X86_MATCH_INTEL_FAM6_MODEL(XEON_PHI_KNM, &idle_cpu_knl),
> @@ -1867,6 +1870,9 @@ static void __init
> intel_idle_init_cstates_icpu(struct cpuidle_driver *drv)
> case INTEL_FAM6_ALDERLAKE:
> case INTEL_FAM6_ALDERLAKE_L:
> case INTEL_FAM6_ALDERLAKE_N:
> + case INTEL_FAM6_RAPTORLAKE:
> + case INTEL_FAM6_RAPTORLAKE_P:
> + case INTEL_FAM6_RAPTORLAKE_S:
> adl_idle_state_table_update();
> break;
> }

2023-01-20 05:28:05

by Zhang, Rui

[permalink] [raw]
Subject: Re: [PATCH] intel_idle: Add RaptorLake support

On Thu, 2023-01-19 at 08:02 +0100, Álvaro Fernández Rojas wrote:
> This patch adds RaptorLake support to the intel_idle driver.
>
> Since RaptorLake and AlderLake C-state are characteristics the same,
> we use
> AlderLake C-states tables for RaptorLake as well.

They may use the same mwait hints, but the the latency of each c-state
are still different on different platforms.

Just FYI, there is an effort ongoing that measures the latency of each
cstate on the RPL platforms. And based on the measurement result, we
can decide if a new custom table is needed or we can just copy the
previous platform.

thanks,
rui

>
> Signed-off-by: Álvaro Fernández Rojas <[email protected]>
> ---
> drivers/idle/intel_idle.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
> index cfeb24d40d37..1a35b98d9bae 100644
> --- a/drivers/idle/intel_idle.c
> +++ b/drivers/idle/intel_idle.c
> @@ -1429,6 +1429,9 @@ static const struct x86_cpu_id intel_idle_ids[]
> __initconst = {
> X86_MATCH_INTEL_FAM6_MODEL(ALDERLAKE, &idle_cpu_adl
> ),
> X86_MATCH_INTEL_FAM6_MODEL(ALDERLAKE_L, &idle_cpu_adl
> _l),
> X86_MATCH_INTEL_FAM6_MODEL(ALDERLAKE_N, &idle_cpu_adl
> _n),
> + X86_MATCH_INTEL_FAM6_MODEL(RAPTORLAKE, &idle_cpu_adl
> ),
> + X86_MATCH_INTEL_FAM6_MODEL(RAPTORLAKE_P, &idle_cpu_adl_n),
> + X86_MATCH_INTEL_FAM6_MODEL(RAPTORLAKE_S, &idle_cpu_adl),
> X86_MATCH_INTEL_FAM6_MODEL(SAPPHIRERAPIDS_X, &idle_cpu_spr
> ),
> X86_MATCH_INTEL_FAM6_MODEL(XEON_PHI_KNL, &idle_cpu_knl),
> X86_MATCH_INTEL_FAM6_MODEL(XEON_PHI_KNM, &idle_cpu_knl),
> @@ -1867,6 +1870,9 @@ static void __init
> intel_idle_init_cstates_icpu(struct cpuidle_driver *drv)
> case INTEL_FAM6_ALDERLAKE:
> case INTEL_FAM6_ALDERLAKE_L:
> case INTEL_FAM6_ALDERLAKE_N:
> + case INTEL_FAM6_RAPTORLAKE:
> + case INTEL_FAM6_RAPTORLAKE_P:
> + case INTEL_FAM6_RAPTORLAKE_S:
> adl_idle_state_table_update();
> break;
> }

2024-04-07 17:37:44

by László Péter

[permalink] [raw]
Subject: Re: [PATCH] intel_idle: Add RaptorLake support



> On 20 Aug 2023, at 11:20, Zhang, Rui <[email protected]> wrote:
>
> This is still work in progress because there are still some open
> questions that we cannot answer from our measurement, and the table is
> not finalized yet.
>
> thanks,
> rui
>
>

Hi,

I also just stumbled upon this patch series as I was wondering about the lack of specific support for RaptorLake in intel_idle. The AlderLake specific part of the intel_idle.c code mentions that "On AlderLake C1 has to be disabled if C1E is enabled, and vice versa …. By default we enable C1E and disable C1 by marking it with...”. Without a patch on RaptorLake (which I assume works the same way) this cannot be controlled with the preferred_cstates kernel parameter. Also on my NUC 13 Pro i5-1340P the latency for C10 looks suspiciously large compared to the AlderLake cstates table.

grep . /sys/devices/system/cpu/cpu0/cpuidle/state*/desc
/sys/devices/system/cpu/cpu0/cpuidle/state0/desc:CPUIDLE CORE POLL IDLE
/sys/devices/system/cpu/cpu0/cpuidle/state1/desc:ACPI FFH MWAIT 0x0
/sys/devices/system/cpu/cpu0/cpuidle/state2/desc:ACPI FFH MWAIT 0x21
/sys/devices/system/cpu/cpu0/cpuidle/state3/desc:ACPI FFH MWAIT 0x60

grep . /sys/devices/system/cpu/cpu0/cpuidle/state*/latency
/sys/devices/system/cpu/cpu0/cpuidle/state0/latency:0
/sys/devices/system/cpu/cpu0/cpuidle/state1/latency:1
/sys/devices/system/cpu/cpu0/cpuidle/state2/latency:127
/sys/devices/system/cpu/cpu0/cpuidle/state3/latency:1048

Thanks,
Peter


2024-04-08 02:02:09

by Zhang, Rui

[permalink] [raw]
Subject: Re: [PATCH] intel_idle: Add RaptorLake support

On Sun, 2024-04-07 at 19:37 +0200, László Péter wrote:
>
>
> > On 20 Aug 2023, at 11:20, Zhang, Rui <[email protected]> wrote:
> >
> > This is still work in progress because there are still some open
> > questions that we cannot answer from our measurement, and the table
> > is
> > not finalized yet.
> >
> > thanks,
> > rui
> >
> >
>
> Hi,
>
> I also just stumbled upon this patch series as I was wondering about
> the lack of specific support for RaptorLake in intel_idle. The
> AlderLake specific part of the intel_idle.c code mentions that "On
> AlderLake C1 has to be disabled if C1E is enabled, and vice versa ….
> By default we enable C1E and disable C1 by marking it with...”.  
> Without a patch on RaptorLake (which I assume works the same way)
> this cannot be controlled with the preferred_cstates kernel
> parameter.

That is true, but that only applies when you have a custom table which
expose C1 and C1E as two separate states.

The default behavior (intel_idle + _CST c-states) doesn't have this
issue. We will just follow the system default.

> Also on my NUC 13 Pro i5-1340P the latency for C10 looks
> suspiciously large compared to the AlderLake cstates table.

Does this bring any real issue in your case?

We do have finished a series of evaluation on RPL, but we didn't find
obvious PnP benefit by introducing a custom table. That is why we
stopped shipping one for RPL.

If you find any real case that this impacts, it would be nice to share
your proposal and test data.

thanks,
rui