[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZnMbshMhyoSKyClb@kf-XE>
Date: Wed, 19 Jun 2024 12:56:02 -0500
From: Aaron Rainbolt <arainbolt@...cus.org>
To: "Rafael J. Wysocki" <rjw@...ysocki.net>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>,
Mario Limonciello <mario.limonciello@....com>,
linux-acpi@...r.kernel.org, linux-kernel@...r.kernel.org,
lenb@...nel.org, mmikowski@...cus.org, Perry.Yuan@....com,
Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>
Subject: Re: [PATCH V3] acpi: Allow ignoring _OSC CPPC v2 bit via kernel
parameter
On Wed, Jun 19, 2024 at 07:30:55PM +0200, Rafael J. Wysocki wrote:
> On Wednesday, June 19, 2024 7:09:35 PM CEST Rafael J. Wysocki wrote:
> > On Wed, Jun 19, 2024 at 6:33 AM Aaron Rainbolt <arainbolt@...cus.org> wrote:
> > >
> > > acpi: Allow ignoring _OSC CPPC v2 bit via kernel parameter
> > >
> > > The _OSC is supposed to contain a bit indicating whether the hardware
> > > supports CPPC v2 or not. This bit is not always set, causing CPPC v2 to
> > > be considered absent. This results in severe single-core performance
> > > issues with the EEVDF scheduler on heterogenous-core Intel processors.
> >
> > While some things can be affected by this, I don't immediately see a
> > connection between CPPC v2, Intel hybrid processors and EEVDF.
> >
> > In particular, why would EEVDF alone be affected?
> >
> > Care to explain this?
>
> And the reason why I am asking is because I think that you really need
> something like this (untested beyond compilation):
>
> ---
> drivers/cpufreq/intel_pstate.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> Index: linux-pm/drivers/cpufreq/intel_pstate.c
> ===================================================================
> --- linux-pm.orig/drivers/cpufreq/intel_pstate.c
> +++ linux-pm/drivers/cpufreq/intel_pstate.c
> @@ -355,16 +355,16 @@ static void intel_pstate_set_itmt_prio(i
> int ret;
>
> ret = cppc_get_perf_caps(cpu, &cppc_perf);
> - if (ret)
> - return;
> -
> /*
> - * On some systems with overclocking enabled, CPPC.highest_perf is hardcoded to 0xff.
> - * In this case we can't use CPPC.highest_perf to enable ITMT.
> - * In this case we can look at MSR_HWP_CAPABILITIES bits [8:0] to decide.
> + * If CPPC is not available, fall back to MSR_HWP_CAPABILITIES bits [8:0].
> + *
> + * Also, on some systems with overclocking enabled, CPPC.highest_perf is
> + * hardcoded to 0xff, so CPPC.highest_perf cannot be used to enable ITMT.
> + * Fall back to MSR_HWP_CAPABILITIES then too.
> */
> - if (cppc_perf.highest_perf == CPPC_MAX_PERF)
> - cppc_perf.highest_perf = HWP_HIGHEST_PERF(READ_ONCE(all_cpu_data[cpu]->hwp_cap_cached));
> + if (ret || cppc_perf.highest_perf == CPPC_MAX_PERF)
> + cppc_perf.highest_perf =
> + HWP_HIGHEST_PERF(READ_ONCE(all_cpu_data[cpu]->hwp_cap_cached));
>
> /*
> * The priorities can be set regardless of whether or not
>
>
>
Gah. I can't read apparently. That patch may very well work because I
just realized the "if (ret) return;" means to return if ret is NOT 0. I
had it confused with "return if ret is 0".
That patch looks like it may very well work, and better than what I had
because it doesn't require manually setting a kernel parameter. I'll apply
it and test it. (That may take me a bit, I don't have access to the
hardware with the problem, only my boss does, but I should be able to get
it done before the end of today.)
Powered by blists - more mailing lists