> I would put that information into the changelog.
Thanks, @Mika Westerberg can you collapse that in when you re-spin the
series?
>
> Moreover, have you looked at acpi_pci_osc_control_set()?
>
> What it does is analogous to what you are proposing, but a bit
> different, and I would like to preserve consistency between _OSC use
> cases.
>
> So would it be possible to adjust the _SB _OSC evaluation flow to
> follow the PCI _OSC one? That is, if any control bits are there, pass
> them along with the last evaluation of _OSC with the query flag clear.
> Or is the latter defective and if so then why?
Basically the only difference is another line cloning OSC_CONTROL_DWORD from
capbuf_ret to capbuf?
Yes, this actually sounds like it better adheres to the spec to me.
Quoting spec:
" If the OS is granted control of a feature in the Control Field in one call to
_OSC, then it must preserve the set state of that bit (requesting that feature)
in all subsequent calls."
>
> >
> > >
> > > > and this is going to cause problems with the USB4 CM (Connection
> > > > Manager) switch that is going to commit the switch only when the OS
> > > > requests control over the feature.
> > > >
> > > > For this reason modify the _OSC support so that we first execute it with
> > > > query bit set, then use the returned valu as base of the features we
> > >
> > > s/valu/value/
> > >
> > > > want to control and run the _OSC again with query bit clear.
> > > >
> > > > Also rename the function to better match what it does.
> > > >
> > > > Cc: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> > > > Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxxx>
> > > > Signed-off-by: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx>
> > > >
> > > > ---
> > > > drivers/acpi/bus.c | 43 +++++++++++++++++++++++++++++++------------
> > > > 1 file changed, 31 insertions(+), 12 deletions(-)
> > > >
> > > > diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> > > > index 1682f8b454a2..ca7c7b2bf56e 100644
> > > > --- a/drivers/acpi/bus.c
> > > > +++ b/drivers/acpi/bus.c
> > > > @@ -282,9 +282,9 @@ bool osc_pc_lpi_support_confirmed;
> > > > EXPORT_SYMBOL_GPL(osc_pc_lpi_support_confirmed);
> > > >
> > > > static u8 sb_uuid_str[] = "0811B06E-4A27-44F9-8D60-3CBBC22E7B48";
> > > > -static void acpi_bus_osc_support(void)
> > > > +static void acpi_bus_osc_negotiate_platform_control(void)
> > > > {
> > > > - u32 capbuf[2];
> > > > + u32 capbuf[2], *capbuf_ret;
> > > > struct acpi_osc_context context = {
> > > > .uuid_str = sb_uuid_str,
> > > > .rev = 1,
> > > > @@ -321,17 +321,36 @@ static void acpi_bus_osc_support(void)
> > > > capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_APEI_SUPPORT;
> > > > if (ACPI_FAILURE(acpi_get_handle(NULL, "\\_SB", &handle)))
> > > > return;
> > > > - if (ACPI_SUCCESS(acpi_run_osc(handle, &context))) {
> > > > - u32 *capbuf_ret = context.ret.pointer;
> > > > - if (context.ret.length > OSC_SUPPORT_DWORD) {
> > > > - osc_sb_apei_support_acked =
> > > > - capbuf_ret[OSC_SUPPORT_DWORD] &
> > > OSC_SB_APEI_SUPPORT;
> > > > - osc_pc_lpi_support_confirmed =
> > > > - capbuf_ret[OSC_SUPPORT_DWORD] &
> > > OSC_SB_PCLPI_SUPPORT;
> > > > - }
> > > > +
> > > > + if (ACPI_FAILURE(acpi_run_osc(handle, &context)))
> > > > + return;
> > > > +
> > > > + capbuf_ret = context.ret.pointer;
> > > > + if (context.ret.length <= OSC_SUPPORT_DWORD) {
> > > > kfree(context.ret.pointer);
> > > > + return;
> > > > }
> > > > - /* do we need to check other returned cap? Sounds no */
> > > > +
> > > > + /*
> > > > + * Now run _OSC again with query flag clean and with the caps
> > >
> > > s/clean/clear/
> > >
> > > > + * both platform and OS supports.
> > >
> > > s/both platform and OS supports/supported by both the OS and the platform/
> > >
> > > > + */
> > > > + capbuf[OSC_QUERY_DWORD] = 0;
> > > > + capbuf[OSC_SUPPORT_DWORD] = capbuf_ret[OSC_SUPPORT_DWORD];
> > > > + kfree(context.ret.pointer);
> > > > +
> > > > + if (ACPI_FAILURE(acpi_run_osc(handle, &context)))
> > > > + return;
> > > > +
> > > > + capbuf_ret = context.ret.pointer;
> > > > + if (context.ret.length > OSC_SUPPORT_DWORD) {
> > > > + osc_sb_apei_support_acked =
> > > > + capbuf_ret[OSC_SUPPORT_DWORD] &
> OSC_SB_APEI_SUPPORT;
> > > > + osc_pc_lpi_support_confirmed =
> > > > + capbuf_ret[OSC_SUPPORT_DWORD] &
> > > OSC_SB_PCLPI_SUPPORT;
> > > > + }
> > > > +
> > > > + kfree(context.ret.pointer);
> > > > }
> > > >
> > > > /* --------------------------------------------------------------------
> ----
> > > --
> > > > @@ -1168,7 +1187,7 @@ static int __init acpi_bus_init(void)
> > > > * _OSC method may exist in module level code,
> > > > * so it must be run after ACPI_FULL_INITIALIZATION
> > > > */
> > > > - acpi_bus_osc_support();
> > > > + acpi_bus_osc_negotiate_platform_control();
> > > >
> > > > /*
> > > > * _PDC control method may load dynamic SSDT tables,
> > > > --
> > > > 2.29.2
> > > >
