RE: [PATCH V8 2/9] drivers core: add ACPI based WBRF mechanism introduced by AMD
[AMD Official Use Only - General] > -Original Message- > From: Simon Horman > Sent: Friday, August 11, 2023 5:38 PM > To: Quan, Evan > Cc: raf...@kernel.org; l...@kernel.org; Deucher, Alexander > ; Koenig, Christian > ; Pan, Xinhui ; > airl...@gmail.com; dan...@ffwll.ch; johan...@sipsolutions.net; > da...@davemloft.net; eduma...@google.com; k...@kernel.org; > pab...@redhat.com; Limonciello, Mario ; > mdaen...@redhat.com; maarten.lankho...@linux.intel.com; > tzimmerm...@suse.de; hdego...@redhat.com; jingyuwang_...@163.com; > Lazar, Lijo ; jim.cro...@gmail.com; > bellosili...@gmail.com; andrealm...@igalia.com; t...@redhat.com; > j...@jsg.id.au; a...@arndb.de; and...@lunn.ch; linux- > ker...@vger.kernel.org; linux-a...@vger.kernel.org; amd- > g...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; linux- > wirel...@vger.kernel.org; net...@vger.kernel.org > Subject: Re: [PATCH V8 2/9] drivers core: add ACPI based WBRF mechanism > introduced by AMD > > On Thu, Aug 10, 2023 at 03:37:56PM +0800, Evan Quan wrote: > > AMD has introduced an ACPI based mechanism to support WBRF for some > > platforms with AMD dGPU + WLAN. This needs support from BIOS equipped > > with necessary AML implementations and dGPU firmwares. > > > > For those systems without the ACPI mechanism and developing solutions, > > user can use/fall-back the generic WBRF solution for diagnosing potential > > interference issues. > > > > And for the platform which does not equip with the necessary AMD ACPI > > implementations but with CONFIG_WBRF_AMD_ACPI built as 'y', it will > > fall back to generic WBRF solution if the `wbrf` is set as "on". > > > > Co-developed-by: Mario Limonciello > > Signed-off-by: Mario Limonciello > > Co-developed-by: Evan Quan > > Signed-off-by: Evan Quan > > ... > > > diff --git a/drivers/acpi/amd_wbrf.c b/drivers/acpi/amd_wbrf.c > > ... > > > +static bool check_acpi_wbrf(acpi_handle handle, u64 rev, u64 funcs) > > +{ > > + int i; > > + u64 mask = 0; > > + union acpi_object *obj; > > + > > + if (funcs == 0) > > + return false; > > + > > + obj = acpi_evaluate_wbrf(handle, rev, 0); > > + if (!obj) > > + return false; > > + > > + if (obj->type != ACPI_TYPE_BUFFER) > > + return false; > > + > > + /* > > +* Bit vector providing supported functions information. > > +* Each bit marks support for one specific function of the WBRF > method. > > +*/ > > + for (i = 0; i < obj->buffer.length && i < 8; i++) > > + mask |= (((u64)obj->buffer.pointer[i]) << (i * 8)); > > + > > + ACPI_FREE(obj); > > + > > + if ((mask & BIT(WBRF_ENABLED)) && > > +(mask & funcs) == funcs) > > Hi Evan, > > a minor nit from my side: the indentation of the line above seems odd. Thanks. Will update this. Evan > > if ((mask & BIT(WBRF_ENABLED)) && > (mask & funcs) == funcs) > > > + return true; > > + > > + return false; > > +} > > ...
Re: [PATCH V8 2/9] drivers core: add ACPI based WBRF mechanism introduced by AMD
On Thu, Aug 10, 2023 at 03:37:56PM +0800, Evan Quan wrote: > AMD has introduced an ACPI based mechanism to support WBRF for some > platforms with AMD dGPU + WLAN. This needs support from BIOS equipped > with necessary AML implementations and dGPU firmwares. > > For those systems without the ACPI mechanism and developing solutions, > user can use/fall-back the generic WBRF solution for diagnosing potential > interference issues. > > And for the platform which does not equip with the necessary AMD ACPI > implementations but with CONFIG_WBRF_AMD_ACPI built as 'y', it will > fall back to generic WBRF solution if the `wbrf` is set as "on". > > Co-developed-by: Mario Limonciello > Signed-off-by: Mario Limonciello > Co-developed-by: Evan Quan > Signed-off-by: Evan Quan ... > diff --git a/drivers/acpi/amd_wbrf.c b/drivers/acpi/amd_wbrf.c ... > +static bool check_acpi_wbrf(acpi_handle handle, u64 rev, u64 funcs) > +{ > + int i; > + u64 mask = 0; > + union acpi_object *obj; > + > + if (funcs == 0) > + return false; > + > + obj = acpi_evaluate_wbrf(handle, rev, 0); > + if (!obj) > + return false; > + > + if (obj->type != ACPI_TYPE_BUFFER) > + return false; > + > + /* > + * Bit vector providing supported functions information. > + * Each bit marks support for one specific function of the WBRF method. > + */ > + for (i = 0; i < obj->buffer.length && i < 8; i++) > + mask |= (((u64)obj->buffer.pointer[i]) << (i * 8)); > + > + ACPI_FREE(obj); > + > + if ((mask & BIT(WBRF_ENABLED)) && > + (mask & funcs) == funcs) Hi Evan, a minor nit from my side: the indentation of the line above seems odd. if ((mask & BIT(WBRF_ENABLED)) && (mask & funcs) == funcs) > + return true; > + > + return false; > +} ...
[PATCH V8 2/9] drivers core: add ACPI based WBRF mechanism introduced by AMD
AMD has introduced an ACPI based mechanism to support WBRF for some platforms with AMD dGPU + WLAN. This needs support from BIOS equipped with necessary AML implementations and dGPU firmwares. For those systems without the ACPI mechanism and developing solutions, user can use/fall-back the generic WBRF solution for diagnosing potential interference issues. And for the platform which does not equip with the necessary AMD ACPI implementations but with CONFIG_WBRF_AMD_ACPI built as 'y', it will fall back to generic WBRF solution if the `wbrf` is set as "on". Co-developed-by: Mario Limonciello Signed-off-by: Mario Limonciello Co-developed-by: Evan Quan Signed-off-by: Evan Quan -- v4->v5: - promote this to be a more generic solution with input argument taking `struct device` and provide better scalability to support non-ACPI scenarios(Andrew) - update the APIs naming and some other minor fixes(Rafael) v5->v6: - make the code more readable and some other fixes(Andrew) v6->v8: - drop CONFIG_WBRF_GENERIC(Mario) - add `wbrf` kernel parameter for policy control(Mario) --- drivers/acpi/Makefile | 2 + drivers/acpi/amd_wbrf.c | 294 ++ drivers/base/Kconfig | 20 +++ drivers/base/wbrf.c | 135 +--- include/linux/acpi_amd_wbrf.h | 25 +++ 5 files changed, 452 insertions(+), 24 deletions(-) create mode 100644 drivers/acpi/amd_wbrf.c create mode 100644 include/linux/acpi_amd_wbrf.h diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile index 3fc5a0d54f6e..9185d16e4495 100644 --- a/drivers/acpi/Makefile +++ b/drivers/acpi/Makefile @@ -133,3 +133,5 @@ obj-$(CONFIG_ARM64) += arm64/ obj-$(CONFIG_ACPI_VIOT)+= viot.o obj-$(CONFIG_RISCV)+= riscv/ + +obj-$(CONFIG_WBRF_AMD_ACPI)+= amd_wbrf.o diff --git a/drivers/acpi/amd_wbrf.c b/drivers/acpi/amd_wbrf.c new file mode 100644 index ..a3390d91cbea --- /dev/null +++ b/drivers/acpi/amd_wbrf.c @@ -0,0 +1,294 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Wifi Band Exclusion Interface (AMD ACPI Implementation) + * Copyright (C) 2023 Advanced Micro Devices + * + */ + +#include +#include + +#define ACPI_AMD_WBRF_METHOD "\\WBRF" + +/* + * Functions bit vector for WBRF method + * + * Bit 0: Supported for any functions other than function 0. + * Bit 1: Function 1 (Add / Remove frequency) is supported. + * Bit 2: Function 2 (Get frequency list) is supported. + */ +#define WBRF_ENABLED 0x0 +#define WBRF_RECORD0x1 +#define WBRF_RETRIEVE 0x2 + +/* record actions */ +#define WBRF_RECORD_ADD0x0 +#define WBRF_RECORD_REMOVE 0x1 + +#define WBRF_REVISION 0x1 + +/* + * The data structure used for WBRF_RETRIEVE is not natually aligned. + * And unfortunately the design has been settled down. + */ +struct amd_wbrf_ranges_out { + u32 num_of_ranges; + struct exclusion_range band_list[MAX_NUM_OF_WBRF_RANGES]; +} __packed; + +static const guid_t wifi_acpi_dsm_guid = + GUID_INIT(0x7b7656cf, 0xdc3d, 0x4c1c, + 0x83, 0xe9, 0x66, 0xe7, 0x21, 0xde, 0x30, 0x70); + +static int wbrf_dsm(struct acpi_device *adev, u8 fn, + union acpi_object *argv4, + union acpi_object **out) +{ + union acpi_object *obj; + int rc; + + obj = acpi_evaluate_dsm(adev->handle, _acpi_dsm_guid, + WBRF_REVISION, fn, argv4); + if (!obj) + return -ENXIO; + + switch (obj->type) { + case ACPI_TYPE_BUFFER: + *out = obj; + return 0; + + case ACPI_TYPE_INTEGER: + rc = obj->integer.value ? -EINVAL : 0; + break; + + default: + rc = -EOPNOTSUPP; + } + + ACPI_FREE(obj); + + return rc; +} + +static int wbrf_record(struct acpi_device *adev, uint8_t action, + struct wbrf_ranges_in *in) +{ + union acpi_object argv4; + union acpi_object *tmp; + u32 num_of_ranges = 0; + u32 num_of_elements; + u32 arg_idx = 0; + u32 loop_idx; + int ret; + + if (!in) + return -EINVAL; + + for (loop_idx = 0; loop_idx < ARRAY_SIZE(in->band_list); +loop_idx++) + if (in->band_list[loop_idx].start && + in->band_list[loop_idx].end) + num_of_ranges++; + + /* +* Every range comes with two end points(start and end) and +* each of them is accounted as an element. Meanwhile the range +* count and action type are accounted as an element each. +* So, the total element count = 2 * num_of_ranges + 1 + 1. +*/ + num_of_elements = 2 * num_of_ranges + 1 + 1; + + tmp = kcalloc(num_of_elements, sizeof(*tmp), GFP_KERNEL); + if (!tmp) +