Re: [RFC PATCH 2/4] cpufreq: acpi-cpufreq: Add processor to the ignore PSD override list

[Punit Agrawal <punitagrawal@gmail.com>]

Borislav Petkov <bp@alien8.de> writes:

> On Mon, Dec 07, 2020 at 04:07:52PM -0600, Wei Huang wrote:
>> I think we shouldn't override zen2 if _PSD is correct. In my opinion,
>> there are two approaches:
>> 
>> * Keep override_acpi_psd()
>> Let us keep the original quirk and override_acpi_psd() function. Over
>> the time, people may want to add new CPUs to override_acpi_psd(). The
>> maintainer may declare that only CPUs >= family 17h will be fixed, to
>> avoid exploding the check-list.
>> 
>> * Remove the quirk completely
>> We can completely remove commit acd316248205 ("acpi-cpufreq: Add quirk
>> to disable _PSD usage on all AMD CPUs")? I am not sure what "AMD desktop
>> boards" was referring to in the original commit message of acd316248205.
>> Maybe such machines aren't in use anymore.
>
> * Third option: do not do anything. Why?
>
> - Let sleeping dogs lie and leave the workaround acd316248205 for old
> machines.

According to the commit log, acd316248205 seems to be only targeted at
powernow-K8 -

    "in order to use the hardware to its full potential and keep the
    original powernow-k8 behavior, lets override the _PSD table setting on
    AMD hardware."

It's unfortunate that it has continued to apply to all systems since. An
alternate to this and 5368512abe08 would be to restrict the original
workaround to the system mentioned in the commit log.

> - Make a clear cut that the override is not needed from Zen3 on, i.e.,
> your patch
>
>    5368512abe08 ("acpi-cpufreq: Honor _PSD table setting on new AMD CPUs")
>
>
> Punit's commit message reads "...indicates that the override is not
> required for Zen3 onwards, it seems that domain information can be
> trusted even on certain earlier systems."
>
> That's not nearly a justification in my book to do this on anything <
> Zen3.
>
> This way you have a clear cut, you don't need to deal with adding any
> more models to override_acpi_psd() and all is good.

PSD (P-State Dependency) object is used to express the coupling of
processors that share the same frequency domain. Ignoring the hardware
topology information will cause both power management and scheduling to
make sub-optimal choices.

Wasn't this the motivation for taking it into account for Zen3 based
systems in 5368512abe08? Or the powernow-k8 in the original workaround
(though there it required ignoring firmware)? The exact same argument
applies here to the Zen2 system I have running in a thermally
constrained environment.

In an ideal case, I was hoping AMD folks would come back with
confirmation of how far back the override can be dropped and the
workaround condition could be relaxed further. But if that is not
available, the only way we have is to include systems that have been
verified to not need the override and somebody that cares to send the
patch.