Re: [V11 1/8] ACPI: Add support for AMD ACPI based Wifi band RFI mitigation feature
On Thu, Aug 31, 2023 at 8:21 AM Evan Quan wrote:
>
> Due to electrical and mechanical constraints in certain platform designs
> there may be likely interference of relatively high-powered harmonics of
> the (G-)DDR memory clocks with local radio module frequency bands used
> by Wifi 6/6e/7.
>
> To mitigate this, AMD has introduced a mechanism that devices can use to
> notify active use of particular frequencies so that other devices can make
> relative internal adjustments as necessary to avoid this resonance.
The changelog is only marginally useful IMV.
It doesn't even mention the role of ACPI in all this, so it is quite
unclear what the patch is all about, why it does what it does and what
is actually done in it.
It is also unclear why this code is put into drivers/acpi/, which
should be explained.
> Signed-off-by: Evan Quan
> --
> v10->v11:
> - fix typo(Simon)
> ---
> drivers/acpi/Kconfig | 17 ++
> drivers/acpi/Makefile | 2 +
> drivers/acpi/amd_wbrf.c | 414 ++
> include/linux/acpi_amd_wbrf.h | 140
> 4 files changed, 573 insertions(+)
> create mode 100644 drivers/acpi/amd_wbrf.c
> create mode 100644 include/linux/acpi_amd_wbrf.h
>
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index 00dd309b6682..a092ea72d152 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -594,6 +594,23 @@ config ACPI_PRMT
> substantially increase computational overhead related to the
> initialization of some server systems.
>
> +config WBRF_AMD_ACPI
> + bool "ACPI based WBRF mechanism introduced by AMD"
> + depends on ACPI
> + default n
> + help
> + Wifi band RFI mitigation mechanism allows multiple drivers from
> + different domains to notify the frequencies in use so that hardware
> + can be reconfigured to avoid harmonic conflicts.
So drivers can notify the platform firmware IIUC, but that is not
really clear from the above. I'm not even sure what the phrase
"notify the frequencies in use" is supposed to mean.
> +
> + AMD has introduced an ACPI based mechanism to support WBRF for some
> + platforms with AMD dGPU and WLAN. This needs support from BIOS
> equipped
> + with necessary AML implementations and dGPU firmwares.
> +
> + Before enabling this ACPI based mechanism, it is suggested to
> confirm
> + with the hardware designer/provider first whether your platform
> + equipped with necessary BIOS and firmwares.
No, this doesn't work.
Say you are a distro and you want to supply all of your users with the
same binary kernel image. What are you expected to do to address the
above?
> +
> endif # ACPI
>
> config X86_PM_TIMER
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index eaa09bf52f17..a3d2f259d0a5 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -132,3 +132,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 ..8ee0e2977a30
> --- /dev/null
> +++ b/drivers/acpi/amd_wbrf.c
> @@ -0,0 +1,414 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Wifi Band Exclusion Interface (AMD ACPI Implementation)
> + * Copyright (C) 2023 Advanced Micro Devices
> + *
Please document the code in this file at least basically.
You don't even explain what Wifi Band Exclusion means.
The OS-firmware interface that this code is based on should be
described here or a link to its description should be provided at the
very least.
As it is now, one needs to reverse engineer the patch in order to get
any idea about how this interface is designed.
> + */
> +
> +#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.
> + */
Without any additional information, the comment above is meaningless.
> +#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 naturally 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 =
> +
Re: [V11 1/8] ACPI: Add support for AMD ACPI based Wifi band RFI mitigation feature
On 8/31/2023 01:20, Evan Quan wrote:
Due to electrical and mechanical constraints in certain platform designs
there may be likely interference of relatively high-powered harmonics of
the (G-)DDR memory clocks with local radio module frequency bands used
by Wifi 6/6e/7.
To mitigate this, AMD has introduced a mechanism that devices can use to
notify active use of particular frequencies so that other devices can make
relative internal adjustments as necessary to avoid this resonance.
Signed-off-by: Evan Quan
--
v10->v11:
- fix typo(Simon)
Rafael,
Friendly ping on reviewing patch 1 from v11.
Thank you,
---
drivers/acpi/Kconfig | 17 ++
drivers/acpi/Makefile | 2 +
drivers/acpi/amd_wbrf.c | 414 ++
include/linux/acpi_amd_wbrf.h | 140
4 files changed, 573 insertions(+)
create mode 100644 drivers/acpi/amd_wbrf.c
create mode 100644 include/linux/acpi_amd_wbrf.h
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 00dd309b6682..a092ea72d152 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -594,6 +594,23 @@ config ACPI_PRMT
substantially increase computational overhead related to the
initialization of some server systems.
+config WBRF_AMD_ACPI
+ bool "ACPI based WBRF mechanism introduced by AMD"
+ depends on ACPI
+ default n
+ help
+ Wifi band RFI mitigation mechanism allows multiple drivers from
+ different domains to notify the frequencies in use so that hardware
+ can be reconfigured to avoid harmonic conflicts.
+
+ AMD has introduced an ACPI based mechanism to support WBRF for some
+ platforms with AMD dGPU and WLAN. This needs support from BIOS
equipped
+ with necessary AML implementations and dGPU firmwares.
+
+ Before enabling this ACPI based mechanism, it is suggested to confirm
+ with the hardware designer/provider first whether your platform
+ equipped with necessary BIOS and firmwares.
+
endif # ACPI
config X86_PM_TIMER
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index eaa09bf52f17..a3d2f259d0a5 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -132,3 +132,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 ..8ee0e2977a30
--- /dev/null
+++ b/drivers/acpi/amd_wbrf.c
@@ -0,0 +1,414 @@
+// 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 naturally 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 BLOCKING_NOTIFIER_HEAD(wbrf_chain_head);
+
+static int wbrf_dsm(struct acpi_device *adev,
+ u8 fn,
+ union acpi_object *argv4)
+{
+ 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_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_out *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 &&
