On Fri, Sep 10, 2021 at 03:31:26AM +0800, Fontenot, Nathan wrote:

On 9/8/2021 9:59 AM, Huang Rui wrote:

quoted

amd-pstate is the AMD CPU performance scaling driver that introduces a
new CPU frequency control mechanism on AMD Zen based CPU series in Linux
kernel. The new mechanism is based on Collaborative processor
performance control (CPPC) which is finer grain frequency management
than legacy ACPI hardware P-States. Current AMD CPU platforms are using
the ACPI P-states driver to manage CPU frequency and clocks with
switching only in 3 P-states. AMD P-States is to replace the ACPI
P-states controls, allows a flexible, low-latency interface for the
Linux kernel to directly communicate the performance hints to hardware.

This patch seems like it is just enabling CPPC on AMD and not a new mechanism
based on CPPC. Can you clarify?

Also, if this is just enabling CPPC, shouldn't the driver be named something
like amd_cppc and not amd_pstate? This isn't using P-states.

That's just a name. We use the "amd-pstate" to indicate the new driver that
use the kernel governors such as schedutil and others for frequency
control. And "amd_cppc" indicates the legacy solution with userspace tool
for frequency control.

quoted

"amd-pstate" leverages the Linux kernel governors such as *schedutil*,
*ondemand*, etc. to manage the performance hints which are provided by CPPC
hardware functionality. The first version for amd-pstate is to support one
of the Zen3 processors, and we will support more in future after we verify
the hardware and SBIOS functionalities.

There are two types of hardware implementations for amd-pstate: one is full
MSR support and another is shared memory support. It can use
X86_FEATURE_AMD_CPPC_EXT feature flag to distinguish the different types.

Looking at the drivers/acpi code for CPPC I don't think this distinction
between MSRs and shared memory requires a feature flag. Shouldn't this be
handled properly in cpc_read|write if the ACPI tables are set up correctly?
Please correct me if I'm wrong.

MSR registers can be used for implementing the fast switch function which
has the better performance on schedutil and other governors.

This would also remove the need for the additional indirection pointed
out by Peter.

Could you also provide an explanation as to why a new CPPC driver is need
instead of updating the existing cppc_cpufreq driver.

Good question, I had explained these in last mail with Peter.

Thanks,
Ray