RE: [PATCH 4/6] ACPI: Execute platform _OSC also with query bit clear

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."

quoted

quoted

quoted

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/

quoted

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 <redacted>
Signed-off-by: Mario Limonciello <redacted>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>

---
 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;

quoted

-                       osc_pc_lpi_support_confirmed =
-                               capbuf_ret[OSC_SUPPORT_DWORD] &

OSC_SB_PCLPI_SUPPORT;

quoted

-               }
+
+       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/

quoted

+        * both platform and OS supports.

s/both platform and OS supports/supported by both the OS and the platform/

quoted

+        */
+       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;

quoted

quoted

quoted

+               osc_pc_lpi_support_confirmed =
+                       capbuf_ret[OSC_SUPPORT_DWORD] &

OSC_SB_PCLPI_SUPPORT;

quoted

+       }
+
+       kfree(context.ret.pointer);
 }

 /* --------------------------------------------------------------------

----

quoted

quoted

--

quoted

@@ -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