On 24-08-22, 15:04, Lukasz Luba wrote: > On 8/24/22 07:14, Viresh Kumar wrote: > > On 18-08-22, 16:16, Jeremy Linton wrote: > > > FIE is mostly implemented as PCC mailboxes on arm machines. This was > > > enabled by default without any data suggesting that it does anything > > > but hurt system performance. Lets change the default to 'n' until > > > hardware appears which clearly benefits. > > > > > > Signed-off-by: Jeremy Linton <jeremy.linton@xxxxxxx> > > > --- > > > drivers/cpufreq/Kconfig.arm | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm > > > index 954749afb5fe..ad66d8f15db0 100644 > > > --- a/drivers/cpufreq/Kconfig.arm > > > +++ b/drivers/cpufreq/Kconfig.arm > > > @@ -22,7 +22,7 @@ config ACPI_CPPC_CPUFREQ > > > config ACPI_CPPC_CPUFREQ_FIE > > > bool "Frequency Invariance support for CPPC cpufreq driver" > > > depends on ACPI_CPPC_CPUFREQ && GENERIC_ARCH_TOPOLOGY > > > - default y > > > + default n > > > help > > > This extends frequency invariance support in the CPPC cpufreq driver, > > > by using CPPC delivered and reference performance counters. > > > > Why is this required after we have the first patch in ? > > Well, my question was for the way the patches were added. If we are disabling the feature based on platform, then there is no need to disable the feature by default. > There are a few issues with this ACPI_CPPC_CPUFREQ_FIE solution: > 1. The design is very heavy and that kernel thread can be ~512 util > (that's what we have been told by one of our partners from servers' > world) > 2. The HW & FW design is not suited for this task. Newer HW will just > have AMU counters (on Arm64) for FIE > 3. The patches haven't been tested in terms of performance overhead > AFAIK. Although, it affects existing Arm64 servers with their > workloads. > 4. AFAIK non of our server partners wasn't complaining about issues with > old FIE mechanism. > > In our team we are not allowed to send code that we cannot prove in many > ways. > > I would just not compile this at all (or even revert this feature). > If someone compiled in this by accident - make sure we disable it > after checks like in the patch 1/2. I'll add also some comments > to that patch. If we don't really want the feature, which is open for discussion (added Vincent to have a look as well), then we better mark it BROKEN in Kconfig. -- viresh
