[PATCH] intel_idle: Add RaptorLake support

[PATCH] intel_idle: Add RaptorLake support

[Álvaro Fernández Rojas]

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 <noltari@gmail.com>
---
 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

Re: [PATCH] intel_idle: Add RaptorLake support

[Zhang, Rui]

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 <noltari@gmail.com>
> ---
>  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;
>  	}

Re: [PATCH] intel_idle: Add RaptorLake support

[Rafael J. Wysocki]

On Thu, Jan 19, 2023 at 8:02 AM Álvaro Fernández Rojas
<noltari@gmail.com> 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 <noltari@gmail.com>

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
>

Re: [PATCH] intel_idle: Add RaptorLake support

[Zhang, Rui]

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 <noltari@gmail.com>
> ---
>  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;
>  	}

Re: [PATCH] intel_idle: Add RaptorLake support

[Guillaume Martres]

On 1/19/23 17:13, Zhang, Rui wrote:
> 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.

Hi, I just stumbled upon this patch series as I was wondering about the
lack of support for Raptor Lake in intel_idle. Your last message from
January mentions a future patch, is it available anywhere or is this
still future work?

Thank you,
Guillaume

Re: [PATCH] intel_idle: Add RaptorLake support

[Zhang, Rui]

On Sat, 2023-08-19 at 21:41 +0200, Guillaume Martres wrote:
> On 1/19/23 17:13, Zhang, Rui wrote:
> > 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.
> 
> Hi, I just stumbled upon this patch series as I was wondering about
> the
> lack of support for Raptor Lake in intel_idle.

intel_idle support for RaptorLake, and also other platforms that don't
have a custom table, is always there as long as we have BIOS support.
The custom table is just an optimization.

>  Your last message from
> January mentions a future patch, is it available anywhere or is this
> still future work?
> 

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

Re: [PATCH] intel_idle: Add RaptorLake support

[Guillaume Martres]

Le 20/08/2023 à 11:20, Zhang, Rui a écrit :
> On Sat, 2023-08-19 at 21:41 +0200, Guillaume Martres wrote:
>> On 1/19/23 17:13, Zhang, Rui wrote:
>>> 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.
>>
>> Hi, I just stumbled upon this patch series as I was wondering about
>> the
>> lack of support for Raptor Lake in intel_idle.
> 
> intel_idle support for RaptorLake, and also other platforms that don't
> have a custom table, is always there as long as we have BIOS support.
> The custom table is just an optimization.

Thanks for the information, I might be misinterpreting the effect of
this patch then. I can report that on a Thinkpad P1 Gen 6 using a stock
6.4.11 kernel, the list of C-states looks like this:

$ cat /sys/devices/system/cpu/cpu0/cpuidle/state*/name
POLL
C1_ACPI
C2_ACPI
C3_ACPI

Whereas with this patch they look like this:

$ cat /sys/devices/system/cpu/cpu0/cpuidle/state*/name
POLL
C1E
C6
C8
C10

Neither of which looks quite complete (and
/sys/module/intel_idle/parameters/max_cstate is set to 9). Is this
something I should open a bug report about?

Thanks,
Guillaume

Re: [PATCH] intel_idle: Add RaptorLake support

[Zhang, Rui]

On Sun, 2023-08-20 at 12:28 +0200, Guillaume Martres wrote:
> 
> 
> Le 20/08/2023 à 11:20, Zhang, Rui a écrit :
> > On Sat, 2023-08-19 at 21:41 +0200, Guillaume Martres wrote:
> > > On 1/19/23 17:13, Zhang, Rui wrote:
> > > > 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.
> > > 
> > > Hi, I just stumbled upon this patch series as I was wondering
> > > about
> > > the
> > > lack of support for Raptor Lake in intel_idle.
> > 
> > intel_idle support for RaptorLake, and also other platforms that
> > don't
> > have a custom table, is always there as long as we have BIOS
> > support.
> > The custom table is just an optimization.
> 
> Thanks for the information, I might be misinterpreting the effect of
> this patch then. I can report that on a Thinkpad P1 Gen 6 using a
> stock
> 6.4.11 kernel, the list of C-states looks like this:
> 
> $ cat /sys/devices/system/cpu/cpu0/cpuidle/state*/name
> POLL
> C1_ACPI
> C2_ACPI
> C3_ACPI
> 
> Whereas with this patch they look like this:
> 
> $ cat /sys/devices/system/cpu/cpu0/cpuidle/state*/name
> POLL
> C1E
> C6
> C8
> C10

Yeah, both of them looks reasonable.

> 
> Neither of which looks quite complete

Yeah, cpu can support more cstates but
   ACPI can expose 3 different cstates only.
   custom table can expose more, but there is no need to do so. To get
better PnP, only part of them are actually needed.

>  (and
> /sys/module/intel_idle/parameters/max_cstate is set to 9).

	static int max_cstate = CPUIDLE_STATE_MAX - 1;

So 9 is the default value when the intel_idle.max_cstate parameter is
not set.

>  Is this
> something I should open a bug report about?
> 
TBH, I don't see any problem here.

thanks,
rui

Re: [PATCH] intel_idle: Add RaptorLake support

[László Péter]

> On 20 Aug 2023, at 11:20, Zhang, Rui <rui.zhang@intel.com> 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

Re: [PATCH] intel_idle: Add RaptorLake support

[Zhang, Rui]

On Sun, 2024-04-07 at 19:37 +0200, László Péter wrote:
> 
> 
> > On 20 Aug 2023, at 11:20, Zhang, Rui <rui.zhang@intel.com> 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