Re: [PATCH] intel_idle: Add RaptorLake support

["Zhang, Rui" <rui.zhang@intel.com>]

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