Re: [PATCH] intel_idle: Add Jasper Lake and Elkhart Lake support

From: Kai-Heng Feng
Date: Mon Jul 29 2024 - 00:08:15 EST


[+Cc Rafael, Srinivas]

On Fri, Jul 26, 2024 at 2:52 PM Zhang, Rui <rui.zhang@xxxxxxxxx> wrote:
>
> On Fri, 2024-07-26 at 14:26 +0800, Kai-Heng Feng wrote:
> > Without proper C-state support, the CPU can take long time to exit to
> > C0
> > to handle IRQ and perform DMA.
>
> Can you provide more details?
>
> Say, what cstate is entered w/ and w/o this patch?

Without the patch it's ACPI C1, C2 and C3.

>
> can you show the output of "grep .
> /sys/devices/system/cpu/cpu0/cpuidle/state*/*" without this patch?

/sys/devices/system/cpu/cpu0/cpuidle$ grep . */*
state0/above:0
state0/below:631
state0/default_status:enabled
state0/desc:CPUIDLE CORE POLL IDLE
state0/disable:0
state0/latency:0
state0/name:POLL
state0/power:4294967295
state0/rejected:0
state0/residency:0
state0/time:19513
state0/usage:635
state1/above:26
state1/below:12437
state1/default_status:enabled
state1/desc:ACPI FFH MWAIT 0x0
state1/disable:0
state1/latency:1
state1/name:C1_ACPI
state1/power:0
state1/rejected:0
state1/residency:1
grep: state1/s2idle: Is a directory
state1/time:18621370
state1/usage:74523
state2/above:1690
state2/below:17
state2/default_status:enabled
state2/desc:ACPI FFH MWAIT 0x31
state2/disable:0
state2/latency:253
state2/name:C2_ACPI
state2/power:0
state2/rejected:0
state2/residency:759
grep: state2/s2idle: Is a directory
state2/time:7063052
state2/usage:7909
state3/above:13111
state3/below:0
state3/default_status:enabled
state3/desc:ACPI FFH MWAIT 0x60
state3/disable:0
state3/latency:1048
state3/name:C3_ACPI
state3/power:0
state3/rejected:0
state3/residency:3144
grep: state3/s2idle: Is a directory
state3/time:4451519230
state3/usage:55467


>
> >
> > The data collect via wult shows the latency is similar to Broxton, so
> > use the existing table to support C-state on JSL and EHL.
>
> so you have done cstate measurement on the EHL using wult?
> can you share more details about the measurement results?

I look at the "spikes" of the aggregated graph. Not sure if it's the
right way to interpret the graph though.

It will be much better if Intel can add the proper C-states table for
JSL and EHL.

Kai-Heng

>
> thanks,
> rui
>
> >
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=219023
> > Signed-off-by: Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx>
> > ---
> > drivers/idle/intel_idle.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
> > index 9aab7abc2ae9..eb6975a1d083 100644
> > --- a/drivers/idle/intel_idle.c
> > +++ b/drivers/idle/intel_idle.c
> > @@ -1538,6 +1538,8 @@ static const struct x86_cpu_id intel_idle_ids[]
> > __initconst = {
> > X86_MATCH_VFM(INTEL_ATOM_GOLDMONT, &idle_cpu_bxt),
> > X86_MATCH_VFM(INTEL_ATOM_GOLDMONT_PLUS, &idle_cpu_bxt),
> > X86_MATCH_VFM(INTEL_ATOM_GOLDMONT_D, &idle_cpu_dnv),
> > + X86_MATCH_VFM(INTEL_ATOM_TREMONT, &idle_cpu_bxt),
> > + X86_MATCH_VFM(INTEL_ATOM_TREMONT_L, &idle_cpu_bxt),
> > X86_MATCH_VFM(INTEL_ATOM_TREMONT_D, &idle_cpu_snr),
> > X86_MATCH_VFM(INTEL_ATOM_CRESTMONT, &idle_cpu_grr),
> > X86_MATCH_VFM(INTEL_ATOM_CRESTMONT_X, &idle_cpu_srf),
>