On 6/29/2023 4:23 PM, Rafael J. Wysocki wrote:
> On Tue, Jun 13, 2023 at 6:12 PM Michal Wilczynski
> <michal.wilczynski@xxxxxxxxx> wrote:
>> Change acpi_early_processor_osc() to return value in case of the failure.
>> Make it more generic - previously it served only to execute workaround
>> for buggy BIOS in Skylake systems. Now it will walk through ACPI
>> namespace looking for processor objects and will convey OSPM processor
>> capabilities using _OSC method.
>>
>> Prefer using _OSC method over deprecated _PDC in the acpi_bus_init(). In
>> case of the failure of the _OSC, try using _PDC as a fallback.
>>
>> Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
>> Signed-off-by: Michal Wilczynski <michal.wilczynski@xxxxxxxxx>
>> Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
>> ---
>> drivers/acpi/acpi_processor.c | 23 +++++++++++++----------
>> drivers/acpi/bus.c | 13 +++++++++----
>> drivers/acpi/internal.h | 9 +--------
>> 3 files changed, 23 insertions(+), 22 deletions(-)
>>
>> diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c
>> index 0de0b05b6f53..8965e01406e0 100644
>> --- a/drivers/acpi/acpi_processor.c
>> +++ b/drivers/acpi/acpi_processor.c
>> @@ -669,17 +669,20 @@ static acpi_status __init acpi_hwp_native_thermal_lvt_osc(acpi_handle handle,
>> return AE_OK;
>> }
>>
>> -void __init acpi_early_processor_osc(void)
> I would rename this to something like
> acpi_early_processor_control_setup() and would make it attempt to call
> _PDC if _OSC doesn't work.
>
> Then it could remain void and it could be put under a
> CONFIG_ARCH_MIGHT_HAVE_ACPI_PDC #ifdef.
Sure, makes sense to me
>
>> +acpi_status __init acpi_early_processor_osc(void)
>> {
>> - if (boot_cpu_has(X86_FEATURE_HWP)) {
>> - acpi_walk_namespace(ACPI_TYPE_PROCESSOR, ACPI_ROOT_OBJECT,
>> - ACPI_UINT32_MAX,
>> - acpi_hwp_native_thermal_lvt_osc,
>> - NULL, NULL, NULL);
>> - acpi_get_devices(ACPI_PROCESSOR_DEVICE_HID,
>> - acpi_hwp_native_thermal_lvt_osc,
>> - NULL, NULL);
>> - }
>> + acpi_status status;
>> +
>> + processor_dmi_check();
>> +
>> + status = acpi_walk_namespace(ACPI_TYPE_PROCESSOR, ACPI_ROOT_OBJECT,
>> + ACPI_UINT32_MAX, acpi_processor_osc, NULL,
>> + NULL, NULL);
>> + if (ACPI_FAILURE(status))
>> + return status;
>> +
>> + return acpi_get_devices(ACPI_PROCESSOR_DEVICE_HID, acpi_processor_osc,
>> + NULL, NULL);
>> }
>> #endif
>>
>> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
>> index d161ff707de4..e8d1f645224f 100644
>> --- a/drivers/acpi/bus.c
>> +++ b/drivers/acpi/bus.c
>> @@ -1317,9 +1317,6 @@ static int __init acpi_bus_init(void)
>> goto error1;
>> }
>>
>> - /* Set capability bits for _OSC under processor scope */
>> - acpi_early_processor_osc();
>> -
>> /*
>> * _OSC method may exist in module level code,
>> * so it must be run after ACPI_FULL_INITIALIZATION
>> @@ -1335,7 +1332,15 @@ static int __init acpi_bus_init(void)
>>
>> acpi_sysfs_init();
>>
>> - acpi_early_processor_set_pdc();
>> +#ifdef CONFIG_ARCH_MIGHT_HAVE_ACPI_PDC
>> + status = acpi_early_processor_osc();
>> + if (ACPI_FAILURE(status)) {
>> + pr_err("_OSC methods failed, trying _PDC\n");
>> + acpi_early_processor_set_pdc();
>> + } else {
>> + pr_info("_OSC methods ran successfully\n");
>> + }
>> +#endif
>>
>> /*
>> * Maybe EC region is required at bus_scan/acpi_get_devices. So it
>> diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
>> index f979a2f7077c..e7cc41313997 100644
>> --- a/drivers/acpi/internal.h
>> +++ b/drivers/acpi/internal.h
>> @@ -151,17 +151,10 @@ int acpi_wakeup_device_init(void);
>> -------------------------------------------------------------------------- */
>> #ifdef CONFIG_ARCH_MIGHT_HAVE_ACPI_PDC
>> void acpi_early_processor_set_pdc(void);
>> +acpi_status acpi_early_processor_osc(void);
>>
>> void processor_dmi_check(void);
>> bool processor_physically_present(acpi_handle handle);
>> -#else
>> -static inline void acpi_early_processor_set_pdc(void) {}
>> -#endif
>> -
>> -#ifdef CONFIG_X86
>> -void acpi_early_processor_osc(void);
>> -#else
>> -static inline void acpi_early_processor_osc(void) {}
>> #endif
>>
>> /* --------------------------------------------------------------------------
>> --
