Messages in this thread | | | Date | Fri, 16 Oct 2020 11:08:24 -0500 | From | Wei Huang <> | Subject | Re: [PATCH 1/1] acpi-cpufreq: Honor _PSD table setting in CPU frequency control |
| |
On 10/16 04:58, Rafael J. Wysocki wrote: > On Wed, Oct 7, 2020 at 10:44 PM Wei Huang <wei.huang2@amd.com> wrote: > > > > acpi-cpufreq has a old quirk that overrides the _PSD table supplied by > > BIOS on AMD CPUs. However the _PSD table of new AMD CPUs (Family 19h+) > > now accurately reports the P-state dependency of CPU cores. Hence this > > quirk needs to be fixed in order to support new CPUs' frequency control. > > > > Fixes: acd316248205 ("acpi-cpufreq: Add quirk to disable _PSD usage on all AMD CPUs") > > Signed-off-by: Wei Huang <wei.huang2@amd.com> > > --- > > drivers/cpufreq/acpi-cpufreq.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c > > index e4ff681faaaa..1e6e2abde428 100644 > > --- a/drivers/cpufreq/acpi-cpufreq.c > > +++ b/drivers/cpufreq/acpi-cpufreq.c > > @@ -691,7 +691,8 @@ static int acpi_cpufreq_cpu_init(struct cpufreq_policy *policy) > > cpumask_copy(policy->cpus, topology_core_cpumask(cpu)); > > } > > > > - if (check_amd_hwpstate_cpu(cpu) && !acpi_pstate_strict) { > > + if (check_amd_hwpstate_cpu(cpu) && (c->x86 < 0x19) && > > Why don't you use boot_cpu_data instead of *c?
Thanks for your review. c->x86 contains the same level of information as boot_cpu_data when acpi_cpufreq_cpu_init() starts to execute. But you are right, it is better to use boot_cpu_data, consistent with the rest code in the same function.
> > And why don't you do the extra check in check_amd_hwpstate_cpu()?
check_amd_hwpstate_cpu() is called at various locations. This _PSD fix doesn't apply at other callers.
> > Also the parens around it are not necessary here and is there any
I will remove it in the next rev.
> chance for having a proper symbol instead of the raw 0x19 in that > check?
Unfortunately I didn't find a replacement. Only x86_vendor has an acronym. The rest (fam/model/stepping) use numerical values, including in arch/x86 boot code.
> > > + !acpi_pstate_strict) { > > cpumask_clear(policy->cpus); > > cpumask_set_cpu(cpu, policy->cpus); > > cpumask_copy(data->freqdomain_cpus, > > -- > > 2.26.2 > >
| |