Messages in this thread | | | From | "Rafael J. Wysocki" <> | Date | Tue, 5 Jul 2022 20:11:31 +0200 | Subject | Re: [PATCH v2] ACPI: CPPC: Don't require _OSC if X86_FEATURE_CPPC is supported |
| |
On Wed, Jun 29, 2022 at 10:04 PM Mario Limonciello <mario.limonciello@amd.com> wrote: > > commit 72f2ecb7ece7 ("ACPI: bus: Set CPPC _OSC bits for all and > when CPPC_LIB is supported") added support for claiming to > support CPPC in _OSC on non-Intel platforms. > > This unfortunately caused a regression on a vartiety of AMD > platforms in the field because a number of AMD platforms don't set > the `_OSC` bit 5 or 6 to indicate CPPC or CPPC v2 support. > > As these AMD platforms already claim CPPC support via a dedicated > MSR from `X86_FEATURE_CPPC`, use that to enable this feature rather > than requiring the `_OSC` on platforms with a dedicated MSR. > > If there is additional breakage on the shared memory designs also > missing this _OSC, additional follow up changes may be needed. > > Fixes: 72f2ecb7ece7 ("Set CPPC _OSC bits for all and when CPPC_LIB is supported") > Reported-by: Perry Yuan <perry.yuan@amd.com> > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
I'm still concerned about the possible cases in which there is _CPC, but it returns garbage, because the firmware thinks that the OS will not use _CPC due to the _OSC handshake.
> --- > v1->v2: > * Make the code easier to follow (suggested by Rafael) > * Update commit message to reflect this is only fixing the MSR case > and that any other breakage from 72f2ecb7ece7 will need additional > follow ups > drivers/acpi/cppc_acpi.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c > index 903528f7e187..cc154519c608 100644 > --- a/drivers/acpi/cppc_acpi.c > +++ b/drivers/acpi/cppc_acpi.c > @@ -629,7 +629,15 @@ static bool is_cppc_supported(int revision, int num_ent) > return false; > } > > - return true; > + if (!osc_sb_cppc_not_supported) > + return true; > + > + pr_debug("Firmware missing _OSC support\n"); > +#ifdef CONFIG_X86 > + return boot_cpu_has(X86_FEATURE_CPPC);
So can you please add an additional X86_VENDOR_AMD check to the above?
> +#else > + return false; > +#endif > } > > /* > @@ -684,9 +692,6 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr) > acpi_status status; > int ret = -ENODATA; > > - if (osc_sb_cppc_not_supported) > - return -ENODEV; > - > /* Parse the ACPI _CPC table for this CPU. */ > status = acpi_evaluate_object_typed(handle, "_CPC", NULL, &output, > ACPI_TYPE_PACKAGE); > -- > 2.25.1 >
| |