Linux Power Management development
 help / color / mirror / Atom feed
From: srinivas pandruvada <srinivas.pandruvada@linux.intel.com>
To: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: viresh.kumar@linaro.org, linux-pm@vger.kernel.org,
	 linux-kernel@vger.kernel.org, Henry Tseng <henrytseng@qnap.com>,
	 stable@vger.kernel.org
Subject: Re: [PATCH] cpufreq: intel_pstate: Fix Raptor Lake-E cpufreq limits
Date: Tue, 12 May 2026 11:45:04 -0700	[thread overview]
Message-ID: <d2c5b841302df2538ffb634d9a86c5643122a509.camel@linux.intel.com> (raw)
In-Reply-To: <CAJZ5v0imv=-NRy1e1mvEYFcyryr7kqyCu+nEzGW=S+PSzDJHWA@mail.gmail.com>

On Tue, 2026-05-12 at 14:37 +0200, Rafael J. Wysocki wrote:
> On Tue, May 12, 2026 at 1:15 PM srinivas pandruvada
> <srinivas.pandruvada@linux.intel.com> wrote:
> > 
> > On Tue, 2026-05-12 at 12:20 +0200, Rafael J. Wysocki wrote:
> > > On Tue, May 12, 2026 at 1:53 AM Srinivas Pandruvada
> > > <srinivas.pandruvada@linux.intel.com> wrote:
> > > > 
> > > > Raptor Lake-E processors are not correctly showing cpufreq
> > > > frequency
> > > > limits.
> > > > 
> > > > These CPUs don't set X86_FEATURE_HYBRID_CPU and have no E-
> > > > cores,
> > > > but
> > > > P-cores still use hybrid scaling factor.
> > > > 
> > > > commit 0fcfc9e51990 ("cpufreq: intel_pstate: Fix scaling for
> > > > hybrid-capable systems with disabled E-cores") added support
> > > > for
> > > > such configuration. Here using CPPC nominal freq and perf was
> > > > compared
> > > > to still return hybrid scaling factor.
> > > > 
> > > > Commit 9b18d536b124 ("cpufreq: intel_pstate: Use CPPC to get
> > > > scaling
> > > > factors") restructured hwp_get_cpu_scaling() and added an
> > > > explicit
> > > > check
> > > > for X86_FEATURE_HYBRID_CPU and when not set returns core
> > > > scaling
> > > > factor.
> > > > 
> > > > To address this remove check for X86_FEATURE_HYBRID_CPU and
> > > > call
> > > > intel_pstate_cppc_get_scaling().
> > > > 
> > > > Ideally this change should be enough. But using CPPC for
> > > > scaling
> > > > factor
> > > > results in rounding error, so still doesn't restore the
> > > > original
> > > > behavior.
> > > > 
> > > > In intel_pstate_cppc_get_scaling() return core scaling factor
> > > > when
> > > > ACPI CPPC is not present or when CPPC nominal frequency or
> > > > nominal
> > > > performance are invalid.
> > > > 
> > > > Use hybrid_scaling_factor for P-cores when defined for a CPU,
> > > > if
> > > > not
> > > > calculate from ACPI CPPC nominal frequency and performance.
> > > > 
> > > > Fixes: 9b18d536b124 ("cpufreq: intel_pstate: Use CPPC to get
> > > > scaling factors")
> > > > Reported-by: Henry Tseng <henrytseng@qnap.com>
> > > > Closes:
> > > > https://lore.kernel.org/linux-pm/20260508063032.3248602-1-henrytseng@qnap.com/
> > > > Signed-off-by: Srinivas Pandruvada
> > > > <srinivas.pandruvada@linux.intel.com>
> > > > Cc: stable@vger.kernel.org
> > > > ---
> > > >  drivers/cpufreq/intel_pstate.c | 29 +++++++++++++++-----------
> > > > ---
> > > >  1 file changed, 15 insertions(+), 14 deletions(-)
> > > > 
> > > > diff --git a/drivers/cpufreq/intel_pstate.c
> > > > b/drivers/cpufreq/intel_pstate.c
> > > > index 1292da53e5fc..0379efdee5f8 100644
> > > > --- a/drivers/cpufreq/intel_pstate.c
> > > > +++ b/drivers/cpufreq/intel_pstate.c
> > > > @@ -421,15 +421,23 @@ static int
> > > > intel_pstate_cppc_get_scaling(int
> > > > cpu)
> > > >  {
> > > >         struct cppc_perf_caps cppc_perf;
> > > > 
> > > > +       if (cppc_get_perf_caps(cpu, &cppc_perf) ||
> > > > !cppc_perf.nominal_freq ||
> > > > +           !cppc_perf.nominal_perf)
> > > > +               goto core_scaling;
> > > > +
> > > > +       if (cppc_perf.nominal_perf * 100 ==
> > > > cppc_perf.nominal_freq)
> > > > +               goto core_scaling;
> > > > +
> > > > +       if (hybrid_scaling_factor)
> > > > +               return hybrid_scaling_factor;
> > > > +
> > > >         /*
> > > > -        * Compute the perf-to-frequency scaling factor for the
> > > > given CPU if
> > > > -        * possible, unless it would be 0.
> > > > +        * Compute the perf-to-frequency scaling factor for the
> > > > given CPU
> > > > +        * from nominal freq and nominal_perf
> > > >          */
> > > > -       if (!cppc_get_perf_caps(cpu, &cppc_perf) &&
> > > > -           cppc_perf.nominal_perf && cppc_perf.nominal_freq)
> > > > -               return div_u64(cppc_perf.nominal_freq *
> > > > KHZ_PER_MHZ,
> > > > -                              cppc_perf.nominal_perf);
> > > > +       return div_u64(cppc_perf.nominal_freq * KHZ_PER_MHZ,
> > > > cppc_perf.nominal_perf);
> > > > 
> > > > +core_scaling:
> > > >         return core_get_scaling();
> > > >  }
> > > > 
> > > > @@ -2281,17 +2289,10 @@ static int hwp_get_cpu_scaling(int cpu)
> > > >                  */
> > > >                 if (hybrid_get_cpu_type(cpu) ==
> > > > INTEL_CPU_TYPE_CORE)
> > > >                         return hybrid_scaling_factor;
> > > > -
> > > > -               return core_get_scaling();
> > > 
> > > Why is this change necessary or even useful?
> > > 
> > > This is about E-cores (because P-cores have been covered above)
> > > and
> > > if
> > > hybrid_scaling_factor is set, it is known that the processor is
> > > hybrid
> > > and E-cores have the "core" scaling factor.
> > > 
> > > Or is Raptor Lake-E covered by one of the
> > > intel_hybrid_scaling_factor[] entries and
> > > hybrid_get_cpu_type(cpu)
> > > doesn't return INTEL_CPU_TYPE_CORE on it?  This piece of
> > > information
> > > is missing from the changelog.
> > 
> > Raptor Lake-E (Xeon) uses CPU model as Raptor Lake-S, for which
> > there
> > is already a hardcoded scaling factor in the driver.
> 
> This piece of information needs to be added to the changelog in the
> first place because it is key here.
> 
> > So this "if" block will enter. But since there is no hybrid CPUID
> > feature is defined,
> > hybrid_get_cpu_type(cpu) will return 0 for P-core or E-core. Here
> > there
> > are no E-cores. So need to remove core_get_scaling() as this will
> > return non hybrid factor.
> 
> Well, what about this:
> 
> ---
>  drivers/cpufreq/intel_pstate.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -2279,7 +2279,7 @@ static int hwp_get_cpu_scaling(int cpu)
>           * Return the hybrid scaling factor for P-cores and use the
>           * default core scaling for E-cores.
>           */
> -        if (hybrid_get_cpu_type(cpu) == INTEL_CPU_TYPE_CORE)
> +        if (hybrid_get_cpu_type(cpu) != INTEL_CPU_TYPE_ATOM)
>              return hybrid_scaling_factor;
> 
>          return core_get_scaling();
> 
> Or is the original Raptor Lake-S scaling factor unsuitable for Raptor
> Lake-E?

This will work for RPL-E. But the original change also accounted for
core scaling on hybrid. There was some embedded hybrid capable with P
core only, used core scaling. Don't find that system details anymore. 

But fine, we can live with this change with added Bartlett Lake scaling
factor.

Thanks,
Srinivas

> 
> > 
> > 
> > > 
> > > >         }
> > > > 
> > > > -       /* Use core scaling on non-hybrid systems. */
> > > > -       if (!cpu_feature_enabled(X86_FEATURE_HYBRID_CPU))
> > > > -               return core_get_scaling();
> > > > -
> > > 
> > > So we're now exposing all of the non-hybrid processors to the fun
> > > with
> > > possibly incorrectly populated CPPC, which is kind of risky.
> > > 
> > 
> > This was already used before with
> > commit0fcfc9e51990246a9813475716746ff5eb98c6aa
> > relying that all non hybrid processor (including servers) didn't
> > set
> > nominal frequency, so will return core_scaling without using CPPC.
> > I retested change on servers and non hybrids.
> > 
> > > If Raptor Lake-E is not covered by an existing
> > > intel_hybrid_scaling_factor[] entry, why don't we add one for it
> > > with
> > > a "scaling factor" value indicating that CPPC needs to be used
> > > for
> > > computing it on all CPUs?
> > 
> > It is already covered by existing, but we can only call
> > for intel_pstate_cppc_get_scaling() when hybrid_scaling_factor is
> > defined. This will require a hardcoding for Bartlett Lake also
> > which
> > uses different CPU model, which Henry Tseng is planing to send.
> 
> I would add a new intel_hybrid_scaling_factor[] entry for Bartlett
> Lake then with a proper scaling factor along with the change above.

  reply	other threads:[~2026-05-12 18:45 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-11 23:53 [PATCH] cpufreq: intel_pstate: Fix Raptor Lake-E cpufreq limits Srinivas Pandruvada
2026-05-12  9:04 ` Henry Tseng
2026-05-12 10:20 ` Rafael J. Wysocki
2026-05-12 11:15   ` srinivas pandruvada
2026-05-12 12:37     ` Rafael J. Wysocki
2026-05-12 18:45       ` srinivas pandruvada [this message]
2026-05-12 18:54         ` Rafael J. Wysocki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d2c5b841302df2538ffb634d9a86c5643122a509.camel@linux.intel.com \
    --to=srinivas.pandruvada@linux.intel.com \
    --cc=henrytseng@qnap.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=rafael@kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=viresh.kumar@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox