Hi Rafael,
I wonder if you are OK with this patch?
Thanks!
On Fri, Jan 29, 2021 at 11:32:39AM +0300, Mika Westerberg wrote:
> From: Mario Limonciello <mario.limonciello@xxxxxxxx>
>
> The platform _OSC can change the hardware state when query bit is not
> set. According to ACPI spec it is recommended that the OS runs _OSC with
> query bit set until the platform does not mask any of the capabilities.
> Then it should run it with query bit clear in order to actually commit
> the changes. Linux has not been doing this for the reasons that there
> has not been anything to commit, until now.
>
> The ACPI 6.4 introduced _OSC for USB4 to allow the OS to negotiate
> native control over USB4 tunneling. The platform might implement this so
> that it only activates the software connection manager path when the OS
> calls the _OSC with the query bit clear. Otherwise it may default to the
> firmware connection manager, for instance.
>
> For this reason modify the _OSC support so that we first execute it with
> query bit set, then use the returned value as base of the features we
> want to control and run the _OSC again with query bit clear. This also
> follows what Windows is doing.
>
> 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..a52cb28c40d8 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 clear and with the caps
> + * 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
