On Tue, Mar 15, 2022 at 9:32 PM Limonciello, Mario
<Mario.Limonciello@xxxxxxx> wrote:
>
> [Public]
>
> > > > > That latter piece appears to be the bug in question here and IMO
> > > > > fixing it doesn't even require calling acpi_run_osc() with the query
> > > > > flag set for multiple times.
> > > >
> > > > I think just taking the results will re-introduce the CPC bug though
> > > > won't it? So how to avoid it but also to take the results?
> > >
> > > I think that the OS should not ask for the control of the CPPC bits if
> > > they are masked by the firmware and it should avoid invoking _CPC
> > > then.
> > >
> > > Otherwise we risk breaking legitimate cases in which the firmware
> > > actually doesn't want the OS to control those bits.
> >
> > I'm basically talking about reverting commit 159d8c274fd9, as the part
> > of the _OSC definition in the spec it is based on appears to be bogus
> > (that will be addressed separately via the ACPI spec process), and
> > applying the attached change on top of that.
> >
> > If this looks good to you, I'll take care of it.
>
> Yes, that looks great and I checked with 159d8c274fd9 reverted and that applying
> the problem does not exist. I do think it has the possibility to cause CPPC to not
> be enabled outside of Intel though since HWP is only set there so I would suggest
> this other change on top of it:
Good point.
I'll make it x86-specific for the time being and that can be changed later.
> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> index 4df749b82568..e61dbd7f7108 100644
> --- a/drivers/acpi/bus.c
> +++ b/drivers/acpi/bus.c
> @@ -314,10 +314,8 @@ static void acpi_bus_osc_negotiate_platform_control(void)
> #endif
> #ifdef CONFIG_X86
> capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_GENERIC_INITIATOR_SUPPORT;
> - if (boot_cpu_has(X86_FEATURE_HWP)) {
> - capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_CPC_SUPPORT;
> - capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_CPCV2_SUPPORT;
> - }
> + capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_CPC_SUPPORT;
> + capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_CPCV2_SUPPORT;
> #endif
>
> if (IS_ENABLED(CONFIG_SCHED_MC_PRIO))
