RE: [PATCH v6] ACPI: bus: For platform OSC negotiate capabilities

From: "Limonciello, Mario" <Mario.Limonciello@amd.com>
To: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: "Rafael J . Wysocki" <rjw@rjwysocki.net>,
	ACPI Devel Maling List <linux-acpi@vger.kernel.org>,
	Mika Westerberg <mika.westerberg@linux.intel.com>,
	"Hou, Xiaomeng (Matthew)" <Xiaomeng.Hou@amd.com>,
	"Liu, Aaron" <Aaron.Liu@amd.com>,
	"Huang, Ray" <Ray.Huang@amd.com>,
	Hans de Goede <hdegoede@redhat.com>,
	Mika Westerberg <mika.westerberg@linux.intel.com>
Subject: RE: [PATCH v6] ACPI: bus: For platform OSC negotiate capabilities
Date: Sun, 13 Mar 2022 23:45:10 +0000	[thread overview]
Message-ID: <BL1PR12MB5157839B59321A1A6AD9F73FE20E9@BL1PR12MB5157.namprd12.prod.outlook.com> (raw)
In-Reply-To: <CAJZ5v0ibnaZZu_Gxngjbu5vzdQaJog8XZnJP6_msLqV_gi4Zig@mail.gmail.com>

[Public]

> I would do
> 
> if (capbuf[OSC_SUPPORT_DWORD] == capbuf_ret[OSC_SUPPORT_DWORD])
>         capbuf[OSC_QUERY_DWORD] = 0;
> else
>         capbuf[OSC_SUPPORT_DWORD] &= capbuf_ret[OSC_SUPPORT_DWORD];
> 
> so that the loop terminates even if the firmware does strange things
> and then it would only be necessary to check capbuf[OSC_QUERY_DWORD]
> in the loop termination condition.
> 
> Would that work?
> 

I think it will.  I'll try it and send up a v7 if so.

> > +               kfree(context.ret.pointer);
> > +       } while (capbuf[OSC_QUERY_DWORD] &&
> capbuf[OSC_SUPPORT_DWORD]);
> >
> > -       /* Now run _OSC again with query flag clear */
> > -       capbuf[OSC_QUERY_DWORD] = 0;
> > +       /*
> > +        * Avoid problems with BIOS dynamically loading tables by indicating
> > +        * support for CPPC even if it was masked.
> 
> What exactly do you mean by "BIOS dynamically loading tables"?

As mentioned in commit 159d8c274fd9:

    On certain systems the BIOS loads SSDT tables dynamically based on the
    capabilities the OS claims to support. However, on these systems the
    _OSC actually clears some of the bits (under certain conditions) so what
    happens is that now when we call the _OSC twice the second time we pass
    the cleared values and that results errors like below to appear on the
    system log:

      ACPI BIOS Error (bug): Could not resolve symbol [\_PR.PR00._CPC], AE_NOT_FOUND (20210105/psargs-330)
      ACPI Error: Aborting method \_PR.PR01._CPC due to previous error (AE_NOT_FOUND) (20210105/psparse-529)

This block  is to avoid regressing that again by forcing it on these systems.

> 
> > +        */
> > +#ifdef CONFIG_X86
> > +       if (boot_cpu_has(X86_FEATURE_HWP)) {
> > +               capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_CPC_SUPPORT;
> > +               capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_CPCV2_SUPPORT;
> > +       }
> > +#endif
> >
> > +       /* Now run _OSC again with query flag clear */
> >         if (ACPI_FAILURE(acpi_run_osc(handle, &context)))
> >                 return;
> >
> > --
> > 2.34.1
> >