* [V9 1/9] drivers core: Add support for Wifi band RF mitigations
2023-08-18 3:26 [V9 0/9] Enable Wifi RFI interference mitigation feature support Evan Quan
@ 2023-08-18 3:26 ` Evan Quan
2023-08-18 16:41 ` Rafael J. Wysocki
2023-08-18 21:24 ` Greg KH
2023-08-18 3:26 ` [V9 2/9] drivers core: add ACPI based WBRF mechanism introduced by AMD Evan Quan
` (7 subsequent siblings)
8 siblings, 2 replies; 25+ messages in thread
From: Evan Quan @ 2023-08-18 3:26 UTC (permalink / raw)
To: gregkh, rafael, lenb, johannes, davem, edumazet, kuba, pabeni,
alexander.deucher, andrew, rdunlap, quic_jjohnson, horms
Cc: linux-doc, linux-kernel, linux-acpi, amd-gfx, linux-wireless,
netdev, Evan Quan, Mario Limonciello
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.
In order for a device to support this, the expected flow for device
driver or subsystems:
Drivers/subsystems contributing frequencies:
1) During probe, check `wbrf_supported_producer` to see if WBRF supported
for the device.
2) If adding frequencies, then call `wbrf_add_exclusion` with the
start and end ranges of the frequencies.
3) If removing frequencies, then call `wbrf_remove_exclusion` with
start and end ranges of the frequencies.
Drivers/subsystems responding to frequencies:
1) During probe, check `wbrf_supported_consumer` to see if WBRF is supported
for the device.
2) Call the `wbrf_register_notifier` to register for notifications of
frequency changes from other devices.
3) Call the `wbrf_retrieve_exclusions` to retrieve the current exclusions
range on receiving a notification and response correspondingly.
Meanwhile a kernel parameter `wbrf` with default setting as "auto" is
introduced to specify what the policy is.
- With `wbrf=on`, the WBRF features will be enabled forcely.
- With `wbrf=off`, the WBRF features will be disabled forcely.
- With `wbrf=auto`, it will be up to the system to do proper checks
to determine the WBRF features should be enabled or not.
Co-developed-by: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
Co-developed-by: Evan Quan <evan.quan@amd.com>
Signed-off-by: Evan Quan <evan.quan@amd.com>
--
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)
v6->v7:
- revised the `struct wbrf_ranges_out` to be naturally aligned(Andrew)
- revised some code comments(Andrew)
v8->v9:
- update the document to be more readable(Randy)
---
.../admin-guide/kernel-parameters.txt | 8 +
drivers/base/Makefile | 1 +
drivers/base/wbrf.c | 280 ++++++++++++++++++
include/linux/wbrf.h | 47 +++
4 files changed, 336 insertions(+)
create mode 100644 drivers/base/wbrf.c
create mode 100644 include/linux/wbrf.h
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index a1457995fd41..5566fefeffdf 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -7152,3 +7152,11 @@
xmon commands.
off xmon is disabled.
+ wbrf= [KNL]
+ Format: { on | auto (default) | off }
+ Controls if WBRF features should be forced on or off.
+ on Force enable the WBRF features.
+ auto Up to the system to do proper checks to
+ determine the WBRF features should be enabled
+ or not.
+ off Force disable the WBRF features.
diff --git a/drivers/base/Makefile b/drivers/base/Makefile
index 3079bfe53d04..7b3cef898c19 100644
--- a/drivers/base/Makefile
+++ b/drivers/base/Makefile
@@ -26,6 +26,7 @@ obj-$(CONFIG_GENERIC_MSI_IRQ) += platform-msi.o
obj-$(CONFIG_GENERIC_ARCH_TOPOLOGY) += arch_topology.o
obj-$(CONFIG_GENERIC_ARCH_NUMA) += arch_numa.o
obj-$(CONFIG_ACPI) += physical_location.o
+obj-y += wbrf.o
obj-y += test/
diff --git a/drivers/base/wbrf.c b/drivers/base/wbrf.c
new file mode 100644
index 000000000000..678f245c12c6
--- /dev/null
+++ b/drivers/base/wbrf.c
@@ -0,0 +1,280 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Wifi Band Exclusion Interface
+ * Copyright (C) 2023 Advanced Micro Devices
+ *
+ */
+
+#include <linux/wbrf.h>
+
+static BLOCKING_NOTIFIER_HEAD(wbrf_chain_head);
+static DEFINE_MUTEX(wbrf_mutex);
+static enum WBRF_POLICY_MODE {
+ WBRF_POLICY_FORCE_DISABLE,
+ WBRF_POLICY_AUTO,
+ WBRF_POLICY_FORCE_ENABLE,
+} wbrf_policy = WBRF_POLICY_AUTO;
+
+static int __init parse_wbrf_policy_mode(char *p)
+{
+ if (!strncmp(p, "auto", 4))
+ wbrf_policy = WBRF_POLICY_AUTO;
+ else if (!strncmp(p, "on", 2))
+ wbrf_policy = WBRF_POLICY_FORCE_ENABLE;
+ else if (!strncmp(p, "off", 3))
+ wbrf_policy = WBRF_POLICY_FORCE_DISABLE;
+ else
+ return -EINVAL;
+
+ return 0;
+}
+early_param("wbrf", parse_wbrf_policy_mode);
+
+static struct exclusion_range_pool {
+ struct exclusion_range band_list[MAX_NUM_OF_WBRF_RANGES];
+ u64 ref_counter[MAX_NUM_OF_WBRF_RANGES];
+} wbrf_pool;
+
+static int _wbrf_add_exclusion_ranges(struct wbrf_ranges_in *in)
+{
+ int i, j;
+
+ for (i = 0; i < ARRAY_SIZE(in->band_list); i++) {
+ if (!in->band_list[i].start &&
+ !in->band_list[i].end)
+ continue;
+
+ for (j = 0; j < ARRAY_SIZE(wbrf_pool.band_list); j++) {
+ if (wbrf_pool.band_list[j].start == in->band_list[i].start &&
+ wbrf_pool.band_list[j].end == in->band_list[i].end) {
+ wbrf_pool.ref_counter[j]++;
+ break;
+ }
+ }
+ if (j < ARRAY_SIZE(wbrf_pool.band_list))
+ continue;
+
+ for (j = 0; j < ARRAY_SIZE(wbrf_pool.band_list); j++) {
+ if (!wbrf_pool.band_list[j].start &&
+ !wbrf_pool.band_list[j].end) {
+ wbrf_pool.band_list[j].start = in->band_list[i].start;
+ wbrf_pool.band_list[j].end = in->band_list[i].end;
+ wbrf_pool.ref_counter[j] = 1;
+ break;
+ }
+ }
+ if (j >= ARRAY_SIZE(wbrf_pool.band_list))
+ return -ENOSPC;
+ }
+
+ return 0;
+}
+
+static int _wbrf_remove_exclusion_ranges(struct wbrf_ranges_in *in)
+{
+ int i, j;
+
+ for (i = 0; i < ARRAY_SIZE(in->band_list); i++) {
+ if (!in->band_list[i].start &&
+ !in->band_list[i].end)
+ continue;
+
+ for (j = 0; j < ARRAY_SIZE(wbrf_pool.band_list); j++) {
+ if (wbrf_pool.band_list[j].start == in->band_list[i].start &&
+ wbrf_pool.band_list[j].end == in->band_list[i].end) {
+ wbrf_pool.ref_counter[j]--;
+ if (!wbrf_pool.ref_counter[j]) {
+ wbrf_pool.band_list[j].start = 0;
+ wbrf_pool.band_list[j].end = 0;
+ }
+ break;
+ }
+ }
+ }
+
+ return 0;
+}
+
+static int _wbrf_retrieve_exclusion_ranges(struct wbrf_ranges_out *out)
+{
+ int out_idx = 0;
+ int i;
+
+ memset(out, 0, sizeof(*out));
+
+ for (i = 0; i < ARRAY_SIZE(wbrf_pool.band_list); i++) {
+ if (!wbrf_pool.band_list[i].start &&
+ !wbrf_pool.band_list[i].end)
+ continue;
+
+ out->band_list[out_idx].start = wbrf_pool.band_list[i].start;
+ out->band_list[out_idx++].end = wbrf_pool.band_list[i].end;
+ }
+
+ out->num_of_ranges = out_idx;
+
+ return 0;
+}
+
+/**
+ * wbrf_supported_system - Determine if the system supports WBRF features
+ *
+ * WBRF is used to mitigate devices that cause harmonic interference.
+ * This function will determine if the platform is able to support the
+ * WBRF features.
+ */
+static bool wbrf_supported_system(void)
+{
+ switch (wbrf_policy) {
+ case WBRF_POLICY_FORCE_ENABLE:
+ return true;
+ case WBRF_POLICY_FORCE_DISABLE:
+ return false;
+ case WBRF_POLICY_AUTO:
+ return false;
+ }
+
+ return false;
+}
+
+/**
+ * wbrf_supported_producer - Determine if the device should report frequencies
+ *
+ * @dev: device pointer
+ *
+ * WBRF is used to mitigate devices that cause harmonic interference.
+ * This function will determine if this device should report such frequencies.
+ */
+bool wbrf_supported_producer(struct device *dev)
+{
+ if (!wbrf_supported_system())
+ return false;
+
+ return true;
+}
+EXPORT_SYMBOL_GPL(wbrf_supported_producer);
+
+/**
+ * wbrf_add_exclusion - Add frequency ranges to the exclusion list
+ *
+ * @dev: device pointer
+ * @in: input structure containing the frequency ranges to be added
+ *
+ * Add frequencies into the exclusion list for supported consumers
+ * to react to.
+ */
+int wbrf_add_exclusion(struct device *dev,
+ struct wbrf_ranges_in *in)
+{
+ int r;
+
+ mutex_lock(&wbrf_mutex);
+
+ r = _wbrf_add_exclusion_ranges(in);
+
+ mutex_unlock(&wbrf_mutex);
+ if (r)
+ return r;
+
+ blocking_notifier_call_chain(&wbrf_chain_head, WBRF_CHANGED, NULL);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(wbrf_add_exclusion);
+
+/**
+ * wbrf_remove_exclusion - Remove frequency ranges from the exclusion list
+ *
+ * @dev: device pointer
+ * @in: input structure containing the frequency ranges to be removed
+ *
+ * Remove frequencies from the exclusion list for supported consumers
+ * to react to.
+ */
+int wbrf_remove_exclusion(struct device *dev,
+ struct wbrf_ranges_in *in)
+{
+ int r;
+
+ mutex_lock(&wbrf_mutex);
+
+ r = _wbrf_remove_exclusion_ranges(in);
+
+ mutex_unlock(&wbrf_mutex);
+ if (r)
+ return r;
+
+ blocking_notifier_call_chain(&wbrf_chain_head, WBRF_CHANGED, NULL);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(wbrf_remove_exclusion);
+
+/**
+ * wbrf_supported_consumer - Determine if the device should react to frequencies
+ *
+ * @dev: device pointer
+ *
+ * WBRF is used to mitigate devices that cause harmonic interference.
+ * This function will determine if this device should react to reports from
+ * other devices for such frequencies.
+ */
+bool wbrf_supported_consumer(struct device *dev)
+{
+ if (!wbrf_supported_system())
+ return false;
+
+ return true;
+}
+EXPORT_SYMBOL_GPL(wbrf_supported_consumer);
+
+/**
+ * wbrf_register_notifier - Register for notifications of frequency changes
+ *
+ * @nb: driver notifier block
+ *
+ * WBRF is used to mitigate devices that cause harmonic interference.
+ * This function will allow consumers to register for frequency notifications.
+ */
+int wbrf_register_notifier(struct notifier_block *nb)
+{
+ return blocking_notifier_chain_register(&wbrf_chain_head, nb);
+}
+EXPORT_SYMBOL_GPL(wbrf_register_notifier);
+
+/**
+ * wbrf_unregister_notifier - Unregister for notifications of frequency changes
+ *
+ * @nb: driver notifier block
+ *
+ * WBRF is used to mitigate devices that cause harmonic interference.
+ * This function will allow consumers to unregister for frequency notifications.
+ */
+int wbrf_unregister_notifier(struct notifier_block *nb)
+{
+ return blocking_notifier_chain_unregister(&wbrf_chain_head, nb);
+}
+EXPORT_SYMBOL_GPL(wbrf_unregister_notifier);
+
+/**
+ * wbrf_retrieve_exclusions - Retrieve the exclusion list
+ *
+ * @dev: device pointer
+ * @out: output structure containing the frequency ranges to be excluded
+ *
+ * Retrieve the current exclusion list
+ */
+int wbrf_retrieve_exclusions(struct device *dev,
+ struct wbrf_ranges_out *out)
+{
+ int r;
+
+ mutex_lock(&wbrf_mutex);
+
+ r = _wbrf_retrieve_exclusion_ranges(out);
+
+ mutex_unlock(&wbrf_mutex);
+
+ return r;
+}
+EXPORT_SYMBOL_GPL(wbrf_retrieve_exclusions);
diff --git a/include/linux/wbrf.h b/include/linux/wbrf.h
new file mode 100644
index 000000000000..476a28fec27a
--- /dev/null
+++ b/include/linux/wbrf.h
@@ -0,0 +1,47 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Wifi Band Exclusion Interface
+ * Copyright (C) 2023 Advanced Micro Devices
+ */
+
+#ifndef _LINUX_WBRF_H
+#define _LINUX_WBRF_H
+
+#include <linux/device.h>
+
+/* Maximum number of wbrf ranges */
+#define MAX_NUM_OF_WBRF_RANGES 11
+
+struct exclusion_range {
+ /* start and end point of the frequency range in Hz */
+ u64 start;
+ u64 end;
+};
+
+struct wbrf_ranges_in {
+ /* valid entry: `start` and `end` filled with non-zero values */
+ struct exclusion_range band_list[MAX_NUM_OF_WBRF_RANGES];
+};
+
+struct wbrf_ranges_out {
+ u64 num_of_ranges;
+ struct exclusion_range band_list[MAX_NUM_OF_WBRF_RANGES];
+};
+
+enum wbrf_notifier_actions {
+ WBRF_CHANGED,
+};
+
+bool wbrf_supported_producer(struct device *dev);
+int wbrf_add_exclusion(struct device *adev,
+ struct wbrf_ranges_in *in);
+int wbrf_remove_exclusion(struct device *dev,
+ struct wbrf_ranges_in *in);
+int wbrf_retrieve_exclusions(struct device *dev,
+ struct wbrf_ranges_out *out);
+bool wbrf_supported_consumer(struct device *dev);
+
+int wbrf_register_notifier(struct notifier_block *nb);
+int wbrf_unregister_notifier(struct notifier_block *nb);
+
+#endif /* _LINUX_WBRF_H */
--
2.34.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [V9 1/9] drivers core: Add support for Wifi band RF mitigations
2023-08-18 3:26 ` [V9 1/9] drivers core: Add support for Wifi band RF mitigations Evan Quan
@ 2023-08-18 16:41 ` Rafael J. Wysocki
2023-08-18 21:24 ` Greg KH
1 sibling, 0 replies; 25+ messages in thread
From: Rafael J. Wysocki @ 2023-08-18 16:41 UTC (permalink / raw)
To: Evan Quan
Cc: gregkh, rafael, lenb, johannes, davem, edumazet, kuba, pabeni,
alexander.deucher, andrew, rdunlap, quic_jjohnson, horms,
linux-doc, linux-kernel, linux-acpi, amd-gfx, linux-wireless,
netdev, Mario Limonciello
On Fri, Aug 18, 2023 at 5:27 AM Evan Quan <evan.quan@amd.com> 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.
>
> In order for a device to support this, the expected flow for device
> driver or subsystems:
>
> Drivers/subsystems contributing frequencies:
>
> 1) During probe, check `wbrf_supported_producer` to see if WBRF supported
> for the device.
> 2) If adding frequencies, then call `wbrf_add_exclusion` with the
> start and end ranges of the frequencies.
> 3) If removing frequencies, then call `wbrf_remove_exclusion` with
> start and end ranges of the frequencies.
>
> Drivers/subsystems responding to frequencies:
>
> 1) During probe, check `wbrf_supported_consumer` to see if WBRF is supported
> for the device.
> 2) Call the `wbrf_register_notifier` to register for notifications of
> frequency changes from other devices.
> 3) Call the `wbrf_retrieve_exclusions` to retrieve the current exclusions
> range on receiving a notification and response correspondingly.
>
> Meanwhile a kernel parameter `wbrf` with default setting as "auto" is
> introduced to specify what the policy is.
> - With `wbrf=on`, the WBRF features will be enabled forcely.
> - With `wbrf=off`, the WBRF features will be disabled forcely.
> - With `wbrf=auto`, it will be up to the system to do proper checks
> to determine the WBRF features should be enabled or not.
>
> Co-developed-by: Mario Limonciello <mario.limonciello@amd.com>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> Co-developed-by: Evan Quan <evan.quan@amd.com>
> Signed-off-by: Evan Quan <evan.quan@amd.com>
In the first place, this requires quite a bit of driver API
documentation that is missing.
To a minimum, it should explain what the interface is for and how it
is supposed to be used by drivers (both "producers" and "consumers").
And how to determine whether or not a given device is a "producer" or
"consumer" from the WBRF perspective.
> --
> 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)
> v6->v7:
> - revised the `struct wbrf_ranges_out` to be naturally aligned(Andrew)
> - revised some code comments(Andrew)
> v8->v9:
> - update the document to be more readable(Randy)
> ---
> .../admin-guide/kernel-parameters.txt | 8 +
> drivers/base/Makefile | 1 +
> drivers/base/wbrf.c | 280 ++++++++++++++++++
> include/linux/wbrf.h | 47 +++
> 4 files changed, 336 insertions(+)
> create mode 100644 drivers/base/wbrf.c
> create mode 100644 include/linux/wbrf.h
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index a1457995fd41..5566fefeffdf 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -7152,3 +7152,11 @@
> xmon commands.
> off xmon is disabled.
>
> + wbrf= [KNL]
> + Format: { on | auto (default) | off }
> + Controls if WBRF features should be forced on or off.
> + on Force enable the WBRF features.
> + auto Up to the system to do proper checks to
> + determine the WBRF features should be enabled
> + or not.
> + off Force disable the WBRF features.
Well, how's a casual reader of this file supposed to find out what
WBRF is and whether or not they should care?
> diff --git a/drivers/base/Makefile b/drivers/base/Makefile
> index 3079bfe53d04..7b3cef898c19 100644
> --- a/drivers/base/Makefile
> +++ b/drivers/base/Makefile
> @@ -26,6 +26,7 @@ obj-$(CONFIG_GENERIC_MSI_IRQ) += platform-msi.o
> obj-$(CONFIG_GENERIC_ARCH_TOPOLOGY) += arch_topology.o
> obj-$(CONFIG_GENERIC_ARCH_NUMA) += arch_numa.o
> obj-$(CONFIG_ACPI) += physical_location.o
> +obj-y += wbrf.o
>
> obj-y += test/
>
> diff --git a/drivers/base/wbrf.c b/drivers/base/wbrf.c
> new file mode 100644
> index 000000000000..678f245c12c6
> --- /dev/null
> +++ b/drivers/base/wbrf.c
> @@ -0,0 +1,280 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Wifi Band Exclusion Interface
> + * Copyright (C) 2023 Advanced Micro Devices
> + *
I would expect some explanation of the interface design and purpose here.
So I don't have to wonder what WBRF_POLICY_MODE is or what the
"exclusion ranges" are below.
> + */
> +
> +#include <linux/wbrf.h>
> +
> +static BLOCKING_NOTIFIER_HEAD(wbrf_chain_head);
> +static DEFINE_MUTEX(wbrf_mutex);
> +static enum WBRF_POLICY_MODE {
> + WBRF_POLICY_FORCE_DISABLE,
> + WBRF_POLICY_AUTO,
> + WBRF_POLICY_FORCE_ENABLE,
> +} wbrf_policy = WBRF_POLICY_AUTO;
> +
> +static int __init parse_wbrf_policy_mode(char *p)
> +{
> + if (!strncmp(p, "auto", 4))
> + wbrf_policy = WBRF_POLICY_AUTO;
> + else if (!strncmp(p, "on", 2))
> + wbrf_policy = WBRF_POLICY_FORCE_ENABLE;
> + else if (!strncmp(p, "off", 3))
> + wbrf_policy = WBRF_POLICY_FORCE_DISABLE;
> + else
> + return -EINVAL;
> +
> + return 0;
> +}
> +early_param("wbrf", parse_wbrf_policy_mode);
> +
> +static struct exclusion_range_pool {
> + struct exclusion_range band_list[MAX_NUM_OF_WBRF_RANGES];
> + u64 ref_counter[MAX_NUM_OF_WBRF_RANGES];
> +} wbrf_pool;
> +
> +static int _wbrf_add_exclusion_ranges(struct wbrf_ranges_in *in)
> +{
> + int i, j;
> +
> + for (i = 0; i < ARRAY_SIZE(in->band_list); i++) {
> + if (!in->band_list[i].start &&
> + !in->band_list[i].end)
> + continue;
> +
> + for (j = 0; j < ARRAY_SIZE(wbrf_pool.band_list); j++) {
> + if (wbrf_pool.band_list[j].start == in->band_list[i].start &&
> + wbrf_pool.band_list[j].end == in->band_list[i].end) {
> + wbrf_pool.ref_counter[j]++;
> + break;
> + }
> + }
> + if (j < ARRAY_SIZE(wbrf_pool.band_list))
> + continue;
> +
> + for (j = 0; j < ARRAY_SIZE(wbrf_pool.band_list); j++) {
> + if (!wbrf_pool.band_list[j].start &&
> + !wbrf_pool.band_list[j].end) {
> + wbrf_pool.band_list[j].start = in->band_list[i].start;
> + wbrf_pool.band_list[j].end = in->band_list[i].end;
> + wbrf_pool.ref_counter[j] = 1;
> + break;
> + }
> + }
> + if (j >= ARRAY_SIZE(wbrf_pool.band_list))
> + return -ENOSPC;
> + }
> +
> + return 0;
> +}
> +
> +static int _wbrf_remove_exclusion_ranges(struct wbrf_ranges_in *in)
> +{
> + int i, j;
> +
> + for (i = 0; i < ARRAY_SIZE(in->band_list); i++) {
> + if (!in->band_list[i].start &&
> + !in->band_list[i].end)
> + continue;
> +
> + for (j = 0; j < ARRAY_SIZE(wbrf_pool.band_list); j++) {
> + if (wbrf_pool.band_list[j].start == in->band_list[i].start &&
> + wbrf_pool.band_list[j].end == in->band_list[i].end) {
> + wbrf_pool.ref_counter[j]--;
> + if (!wbrf_pool.ref_counter[j]) {
> + wbrf_pool.band_list[j].start = 0;
> + wbrf_pool.band_list[j].end = 0;
> + }
> + break;
> + }
> + }
> + }
> +
> + return 0;
It never returns anything else. Should it be void?
> +}
> +
> +static int _wbrf_retrieve_exclusion_ranges(struct wbrf_ranges_out *out)
> +{
> + int out_idx = 0;
> + int i;
> +
> + memset(out, 0, sizeof(*out));
> +
> + for (i = 0; i < ARRAY_SIZE(wbrf_pool.band_list); i++) {
> + if (!wbrf_pool.band_list[i].start &&
> + !wbrf_pool.band_list[i].end)
> + continue;
> +
> + out->band_list[out_idx].start = wbrf_pool.band_list[i].start;
> + out->band_list[out_idx++].end = wbrf_pool.band_list[i].end;
> + }
> +
> + out->num_of_ranges = out_idx;
> +
> + return 0;
Same here.
> +}
> +
> +/**
> + * wbrf_supported_system - Determine if the system supports WBRF features
> + *
> + * WBRF is used to mitigate devices that cause harmonic interference.
> + * This function will determine if the platform is able to support the
> + * WBRF features.
The code doesn't quite match the description above. I guess the code
is temporary?
> + */
> +static bool wbrf_supported_system(void)
> +{
> + switch (wbrf_policy) {
> + case WBRF_POLICY_FORCE_ENABLE:
> + return true;
> + case WBRF_POLICY_FORCE_DISABLE:
> + return false;
> + case WBRF_POLICY_AUTO:
> + return false;
> + }
> +
> + return false;
> +}
> +
> +/**
> + * wbrf_supported_producer - Determine if the device should report frequencies
> + *
> + * @dev: device pointer
> + *
> + * WBRF is used to mitigate devices that cause harmonic interference.
> + * This function will determine if this device should report such frequencies.
It is not clear how "harmonic interference" is related to "such
frequencies" from the above.
> + */
> +bool wbrf_supported_producer(struct device *dev)
> +{
> + if (!wbrf_supported_system())
> + return false;
> +
> + return true;
> +}
> +EXPORT_SYMBOL_GPL(wbrf_supported_producer);
> +
> +/**
> + * wbrf_add_exclusion - Add frequency ranges to the exclusion list
> + *
> + * @dev: device pointer
> + * @in: input structure containing the frequency ranges to be added
> + *
> + * Add frequencies into the exclusion list for supported consumers
> + * to react to.
Well, the above isn't particularly helpful IMV.
What's "the exclusion list"? What are "supported consumers" and how
are they going to "react" and to what exactly (the exclusion list or
the frequencies)?
Why is the notifier chain not mentioned in the kerneldoc description
of the function?
> + */
> +int wbrf_add_exclusion(struct device *dev,
> + struct wbrf_ranges_in *in)
> +{
> + int r;
> +
> + mutex_lock(&wbrf_mutex);
> +
> + r = _wbrf_add_exclusion_ranges(in);
> +
> + mutex_unlock(&wbrf_mutex);
> + if (r)
> + return r;
> +
> + blocking_notifier_call_chain(&wbrf_chain_head, WBRF_CHANGED, NULL);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(wbrf_add_exclusion);
> +
> +/**
> + * wbrf_remove_exclusion - Remove frequency ranges from the exclusion list
> + *
> + * @dev: device pointer
> + * @in: input structure containing the frequency ranges to be removed
> + *
> + * Remove frequencies from the exclusion list for supported consumers
> + * to react to.
This has the same problems as the above.
> + */
> +int wbrf_remove_exclusion(struct device *dev,
> + struct wbrf_ranges_in *in)
> +{
> + int r;
> +
> + mutex_lock(&wbrf_mutex);
> +
> + r = _wbrf_remove_exclusion_ranges(in);
> +
> + mutex_unlock(&wbrf_mutex);
> + if (r)
> + return r;
> +
> + blocking_notifier_call_chain(&wbrf_chain_head, WBRF_CHANGED, NULL);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(wbrf_remove_exclusion);
> +
> +/**
> + * wbrf_supported_consumer - Determine if the device should react to frequencies
> + *
> + * @dev: device pointer
> + *
> + * WBRF is used to mitigate devices that cause harmonic interference.
What does this mean? How can a device be "mitigated" and what
"harmonic interference" is this about?
> + * This function will determine if this device should react to reports from
> + * other devices for such frequencies.
What are "such frequencies"?
> + */
> +bool wbrf_supported_consumer(struct device *dev)
> +{
> + if (!wbrf_supported_system())
> + return false;
> +
> + return true;
> +}
> +EXPORT_SYMBOL_GPL(wbrf_supported_consumer);
> +
> +/**
> + * wbrf_register_notifier - Register for notifications of frequency changes
> + *
> + * @nb: driver notifier block
> + *
> + * WBRF is used to mitigate devices that cause harmonic interference.
> + * This function will allow consumers to register for frequency notifications.
What's a "frequency notification"?
> + */
> +int wbrf_register_notifier(struct notifier_block *nb)
> +{
> + return blocking_notifier_chain_register(&wbrf_chain_head, nb);
> +}
> +EXPORT_SYMBOL_GPL(wbrf_register_notifier);
> +
> +/**
> + * wbrf_unregister_notifier - Unregister for notifications of frequency changes
> + *
> + * @nb: driver notifier block
> + *
> + * WBRF is used to mitigate devices that cause harmonic interference.
> + * This function will allow consumers to unregister for frequency notifications.
> + */
> +int wbrf_unregister_notifier(struct notifier_block *nb)
> +{
> + return blocking_notifier_chain_unregister(&wbrf_chain_head, nb);
> +}
> +EXPORT_SYMBOL_GPL(wbrf_unregister_notifier);
> +
> +/**
> + * wbrf_retrieve_exclusions - Retrieve the exclusion list
> + *
> + * @dev: device pointer
> + * @out: output structure containing the frequency ranges to be excluded
Excluded from what?
> + *
> + * Retrieve the current exclusion list
What's "the current exclusion list"?
> + */
> +int wbrf_retrieve_exclusions(struct device *dev,
> + struct wbrf_ranges_out *out)
> +{
> + int r;
> +
> + mutex_lock(&wbrf_mutex);
> +
> + r = _wbrf_retrieve_exclusion_ranges(out);
> +
> + mutex_unlock(&wbrf_mutex);
> +
> + return r;
> +}
> +EXPORT_SYMBOL_GPL(wbrf_retrieve_exclusions);
> diff --git a/include/linux/wbrf.h b/include/linux/wbrf.h
> new file mode 100644
> index 000000000000..476a28fec27a
> --- /dev/null
> +++ b/include/linux/wbrf.h
> @@ -0,0 +1,47 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Wifi Band Exclusion Interface
> + * Copyright (C) 2023 Advanced Micro Devices
> + */
> +
> +#ifndef _LINUX_WBRF_H
> +#define _LINUX_WBRF_H
> +
> +#include <linux/device.h>
> +
> +/* Maximum number of wbrf ranges */
> +#define MAX_NUM_OF_WBRF_RANGES 11
> +
> +struct exclusion_range {
> + /* start and end point of the frequency range in Hz */
I would put the comment above the whole struct definition and use the
kerneldoc format for it.
> + u64 start;
> + u64 end;
> +};
> +
> +struct wbrf_ranges_in {
> + /* valid entry: `start` and `end` filled with non-zero values */
Same here.
> + struct exclusion_range band_list[MAX_NUM_OF_WBRF_RANGES];
> +};
> +
> +struct wbrf_ranges_out {
> + u64 num_of_ranges;
> + struct exclusion_range band_list[MAX_NUM_OF_WBRF_RANGES];
> +};
> +
> +enum wbrf_notifier_actions {
> + WBRF_CHANGED,
> +};
No description.
> +
> +bool wbrf_supported_producer(struct device *dev);
> +int wbrf_add_exclusion(struct device *adev,
> + struct wbrf_ranges_in *in);
> +int wbrf_remove_exclusion(struct device *dev,
> + struct wbrf_ranges_in *in);
> +int wbrf_retrieve_exclusions(struct device *dev,
> + struct wbrf_ranges_out *out);
> +bool wbrf_supported_consumer(struct device *dev);
> +
> +int wbrf_register_notifier(struct notifier_block *nb);
> +int wbrf_unregister_notifier(struct notifier_block *nb);
> +
> +#endif /* _LINUX_WBRF_H */
> --
Overall, readers should be able to understand stuff they read. The
above doesn't follow this rule IMO.
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [V9 1/9] drivers core: Add support for Wifi band RF mitigations
2023-08-18 3:26 ` [V9 1/9] drivers core: Add support for Wifi band RF mitigations Evan Quan
2023-08-18 16:41 ` Rafael J. Wysocki
@ 2023-08-18 21:24 ` Greg KH
2023-08-18 22:49 ` Limonciello, Mario
1 sibling, 1 reply; 25+ messages in thread
From: Greg KH @ 2023-08-18 21:24 UTC (permalink / raw)
To: Evan Quan
Cc: rafael, lenb, johannes, davem, edumazet, kuba, pabeni,
alexander.deucher, andrew, rdunlap, quic_jjohnson, horms,
linux-doc, linux-kernel, linux-acpi, amd-gfx, linux-wireless,
netdev, Mario Limonciello
On Fri, Aug 18, 2023 at 11:26:11AM +0800, Evan Quan wrote:
> drivers/base/Makefile | 1 +
> drivers/base/wbrf.c | 280 ++++++++++++++++++
Why is a wifi-specific thing going into drivers/base/?
confused,
greg k-h
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [V9 1/9] drivers core: Add support for Wifi band RF mitigations
2023-08-18 21:24 ` Greg KH
@ 2023-08-18 22:49 ` Limonciello, Mario
2023-08-19 10:50 ` Greg KH
0 siblings, 1 reply; 25+ messages in thread
From: Limonciello, Mario @ 2023-08-18 22:49 UTC (permalink / raw)
To: Greg KH, Evan Quan, Andrew Lunn
Cc: rafael, lenb, johannes, davem, edumazet, kuba, pabeni,
alexander.deucher, rdunlap, quic_jjohnson, horms, linux-doc,
linux-kernel, linux-acpi, amd-gfx, linux-wireless, netdev
On 8/18/2023 4:24 PM, Greg KH wrote:
> On Fri, Aug 18, 2023 at 11:26:11AM +0800, Evan Quan wrote:
>> drivers/base/Makefile | 1 +
>> drivers/base/wbrf.c | 280 ++++++++++++++++++
>
> Why is a wifi-specific thing going into drivers/base/?
>
> confused,
>
> greg k-h
The original problem statement was at a high level 'there can be
interference between different devices operating at high frequencies'.
The original patches introduced some ACPI library code that enabled a
mitigated for this interference between mac80211 devices and amdgpu devices.
Andrew Lunn wanted to see something more generic, so the series has
morphed into base code for things to advertise frequencies in use and
other things to listen to frequencies in use and react.
The idea is supposed to be that if the platform knows that these
mitigations are needed then the producers send the frequencies in use,
consumers react to them. The AMD implementation of getting this info
from the platform plugs into the base code (patch 2).
If users don't want this behavior they can turn it off on kernel command
line.
If the platform doesn't know mitigations are needed but user wants to
turn them on anyway they can turn it on kernel command line.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [V9 1/9] drivers core: Add support for Wifi band RF mitigations
2023-08-18 22:49 ` Limonciello, Mario
@ 2023-08-19 10:50 ` Greg KH
2023-08-22 3:13 ` Limonciello, Mario
0 siblings, 1 reply; 25+ messages in thread
From: Greg KH @ 2023-08-19 10:50 UTC (permalink / raw)
To: Limonciello, Mario
Cc: Evan Quan, Andrew Lunn, rafael, lenb, johannes, davem, edumazet,
kuba, pabeni, alexander.deucher, rdunlap, quic_jjohnson, horms,
linux-doc, linux-kernel, linux-acpi, amd-gfx, linux-wireless,
netdev
On Fri, Aug 18, 2023 at 05:49:14PM -0500, Limonciello, Mario wrote:
>
>
> On 8/18/2023 4:24 PM, Greg KH wrote:
> > On Fri, Aug 18, 2023 at 11:26:11AM +0800, Evan Quan wrote:
> > > drivers/base/Makefile | 1 +
> > > drivers/base/wbrf.c | 280 ++++++++++++++++++
> >
> > Why is a wifi-specific thing going into drivers/base/?
> >
> > confused,
> >
> > greg k-h
>
> The original problem statement was at a high level 'there can be
> interference between different devices operating at high frequencies'. The
> original patches introduced some ACPI library code that enabled a mitigated
> for this interference between mac80211 devices and amdgpu devices.
>
> Andrew Lunn wanted to see something more generic, so the series has morphed
> into base code for things to advertise frequencies in use and other things
> to listen to frequencies in use and react.
>
> The idea is supposed to be that if the platform knows that these mitigations
> are needed then the producers send the frequencies in use, consumers react
> to them. The AMD implementation of getting this info from the platform
> plugs into the base code (patch 2).
>
> If users don't want this behavior they can turn it off on kernel command
> line.
>
> If the platform doesn't know mitigations are needed but user wants to turn
> them on anyway they can turn it on kernel command line.
That's all fine, I don't object to that at all. But bus/device-specific
stuff should NOT be in drivers/base/ if at all possible (yes, we do have
some exceptions with hypervisor.c and memory and cpu stuff) but for a
frequency thing like this, why can't it live with the other
wifi/frequency code in drivers/net/wireless/?
In other words, what's the benefit to having me be the maintainer of
this, someone who knows nothing about this subsystem, other than you
passing off that work to me? :)
thanks,
greg k-h
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [V9 1/9] drivers core: Add support for Wifi band RF mitigations
2023-08-19 10:50 ` Greg KH
@ 2023-08-22 3:13 ` Limonciello, Mario
2023-08-22 6:39 ` Greg KH
0 siblings, 1 reply; 25+ messages in thread
From: Limonciello, Mario @ 2023-08-22 3:13 UTC (permalink / raw)
To: Greg KH
Cc: Evan Quan, Andrew Lunn, rafael, lenb, johannes, davem, edumazet,
kuba, pabeni, alexander.deucher, rdunlap, quic_jjohnson, horms,
linux-doc, linux-kernel, linux-acpi, amd-gfx, linux-wireless,
netdev
On 8/19/2023 5:50 AM, Greg KH wrote:
> On Fri, Aug 18, 2023 at 05:49:14PM -0500, Limonciello, Mario wrote:
>>
>>
>> On 8/18/2023 4:24 PM, Greg KH wrote:
>>> On Fri, Aug 18, 2023 at 11:26:11AM +0800, Evan Quan wrote:
>>>> drivers/base/Makefile | 1 +
>>>> drivers/base/wbrf.c | 280 ++++++++++++++++++
>>>
>>> Why is a wifi-specific thing going into drivers/base/?
>>>
>>> confused,
>>>
>>> greg k-h
>>
>> The original problem statement was at a high level 'there can be
>> interference between different devices operating at high frequencies'. The
>> original patches introduced some ACPI library code that enabled a mitigated
>> for this interference between mac80211 devices and amdgpu devices.
>>
>> Andrew Lunn wanted to see something more generic, so the series has morphed
>> into base code for things to advertise frequencies in use and other things
>> to listen to frequencies in use and react.
>>
>> The idea is supposed to be that if the platform knows that these mitigations
>> are needed then the producers send the frequencies in use, consumers react
>> to them. The AMD implementation of getting this info from the platform
>> plugs into the base code (patch 2).
>>
>> If users don't want this behavior they can turn it off on kernel command
>> line.
>>
>> If the platform doesn't know mitigations are needed but user wants to turn
>> them on anyway they can turn it on kernel command line.
>
> That's all fine, I don't object to that at all. But bus/device-specific
> stuff should NOT be in drivers/base/ if at all possible (yes, we do have
> some exceptions with hypervisor.c and memory and cpu stuff) but for a
> frequency thing like this, why can't it live with the other
> wifi/frequency code in drivers/net/wireless/?
>
> In other words, what's the benefit to having me be the maintainer of
> this, someone who knows nothing about this subsystem, other than you
> passing off that work to me? :)
>
> thanks,
>
> greg k-h
The reason drivers/base was proposed was because although the initial
implementation is for producers from mac80211, Andrew pointed out that
many other things can technically be producers and cause interference
if not properly shielded.
So by making it part of base that sets up the policy that if something
"can" produce certain problematic harmonics that it can participate.
Whether or not other devices /will/ use this is another question though.
You need deep platform knowledge and proper equipment to diagnose a
problem and conclude it can be helped with this kind of mitigation.
So I wonder if the right answer is to put it in drivers/net/wireless
initially and if we come up with a need later for non wifi producers we
can discuss moving it at that time.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [V9 1/9] drivers core: Add support for Wifi band RF mitigations
2023-08-22 3:13 ` Limonciello, Mario
@ 2023-08-22 6:39 ` Greg KH
2023-08-23 7:53 ` Kalle Valo
0 siblings, 1 reply; 25+ messages in thread
From: Greg KH @ 2023-08-22 6:39 UTC (permalink / raw)
To: Limonciello, Mario
Cc: Evan Quan, Andrew Lunn, rafael, lenb, johannes, davem, edumazet,
kuba, pabeni, alexander.deucher, rdunlap, quic_jjohnson, horms,
linux-doc, linux-kernel, linux-acpi, amd-gfx, linux-wireless,
netdev
On Mon, Aug 21, 2023 at 10:13:45PM -0500, Limonciello, Mario wrote:
> So I wonder if the right answer is to put it in drivers/net/wireless
> initially and if we come up with a need later for non wifi producers we can
> discuss moving it at that time.
Please do so.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [V9 1/9] drivers core: Add support for Wifi band RF mitigations
2023-08-22 6:39 ` Greg KH
@ 2023-08-23 7:53 ` Kalle Valo
2023-08-23 8:15 ` Greg KH
0 siblings, 1 reply; 25+ messages in thread
From: Kalle Valo @ 2023-08-23 7:53 UTC (permalink / raw)
To: Greg KH
Cc: Limonciello, Mario, Evan Quan, Andrew Lunn, rafael, lenb,
johannes, davem, edumazet, kuba, pabeni, alexander.deucher,
rdunlap, quic_jjohnson, horms, linux-doc, linux-kernel,
linux-acpi, amd-gfx, linux-wireless, netdev
Greg KH <gregkh@linuxfoundation.org> writes:
> On Mon, Aug 21, 2023 at 10:13:45PM -0500, Limonciello, Mario wrote:
>> So I wonder if the right answer is to put it in drivers/net/wireless
>> initially and if we come up with a need later for non wifi producers we can
>> discuss moving it at that time.
>
> Please do so.
Sorry, I haven't been able to follow the discussion in detail but just a
quick comment: if there's supposed to be code which is shared with
different wifi drivers then drivers/net/wireless sounds wrong,
net/wireless or net/mac80211 would be more approriate location.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [V9 1/9] drivers core: Add support for Wifi band RF mitigations
2023-08-23 7:53 ` Kalle Valo
@ 2023-08-23 8:15 ` Greg KH
0 siblings, 0 replies; 25+ messages in thread
From: Greg KH @ 2023-08-23 8:15 UTC (permalink / raw)
To: Kalle Valo
Cc: Limonciello, Mario, Evan Quan, Andrew Lunn, rafael, lenb,
johannes, davem, edumazet, kuba, pabeni, alexander.deucher,
rdunlap, quic_jjohnson, horms, linux-doc, linux-kernel,
linux-acpi, amd-gfx, linux-wireless, netdev
On Wed, Aug 23, 2023 at 10:53:43AM +0300, Kalle Valo wrote:
> Greg KH <gregkh@linuxfoundation.org> writes:
>
> > On Mon, Aug 21, 2023 at 10:13:45PM -0500, Limonciello, Mario wrote:
> >> So I wonder if the right answer is to put it in drivers/net/wireless
> >> initially and if we come up with a need later for non wifi producers we can
> >> discuss moving it at that time.
> >
> > Please do so.
>
> Sorry, I haven't been able to follow the discussion in detail but just a
> quick comment: if there's supposed to be code which is shared with
> different wifi drivers then drivers/net/wireless sounds wrong,
> net/wireless or net/mac80211 would be more approriate location.
That's fine with me as well, just not drivers/core/ please :)
thanks,
greg k-h
^ permalink raw reply [flat|nested] 25+ messages in thread
* [V9 2/9] drivers core: add ACPI based WBRF mechanism introduced by AMD
2023-08-18 3:26 [V9 0/9] Enable Wifi RFI interference mitigation feature support Evan Quan
2023-08-18 3:26 ` [V9 1/9] drivers core: Add support for Wifi band RF mitigations Evan Quan
@ 2023-08-18 3:26 ` Evan Quan
2023-08-18 17:41 ` Rafael J. Wysocki
2023-08-18 3:26 ` [V9 3/9] cfg80211: expose nl80211_chan_width_to_mhz for wide sharing Evan Quan
` (6 subsequent siblings)
8 siblings, 1 reply; 25+ messages in thread
From: Evan Quan @ 2023-08-18 3:26 UTC (permalink / raw)
To: gregkh, rafael, lenb, johannes, davem, edumazet, kuba, pabeni,
alexander.deucher, andrew, rdunlap, quic_jjohnson, horms
Cc: linux-doc, linux-kernel, linux-acpi, amd-gfx, linux-wireless,
netdev, Evan Quan, Mario Limonciello
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 <mario.limonciello@amd.com>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
Co-developed-by: Evan Quan <evan.quan@amd.com>
Signed-off-by: Evan Quan <evan.quan@amd.com>
--
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)
v8->v9:
- correct some coding style(Simon)
---
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 000000000000..0e46de3dfac7
--- /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 <linux/acpi.h>
+#include <linux/acpi_amd_wbrf.h>
+
+#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_RECORD 0x1
+#define WBRF_RETRIEVE 0x2
+
+/* record actions */
+#define WBRF_RECORD_ADD 0x0
+#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, &wifi_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)
+ return -ENOMEM;
+
+ argv4.package.type = ACPI_TYPE_PACKAGE;
+ argv4.package.count = num_of_elements;
+ argv4.package.elements = tmp;
+
+ tmp[arg_idx].integer.type = ACPI_TYPE_INTEGER;
+ tmp[arg_idx++].integer.value = num_of_ranges;
+ tmp[arg_idx].integer.type = ACPI_TYPE_INTEGER;
+ tmp[arg_idx++].integer.value = action;
+
+ 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)
+ continue;
+
+ tmp[arg_idx].integer.type = ACPI_TYPE_INTEGER;
+ tmp[arg_idx++].integer.value = in->band_list[loop_idx].start;
+ tmp[arg_idx].integer.type = ACPI_TYPE_INTEGER;
+ tmp[arg_idx++].integer.value = in->band_list[loop_idx].end;
+ }
+
+ ret = wbrf_dsm(adev, WBRF_RECORD, &argv4, NULL);
+
+ kfree(tmp);
+
+ return ret;
+}
+
+int acpi_amd_wbrf_add_exclusion(struct device *dev,
+ struct wbrf_ranges_in *in)
+{
+ struct acpi_device *adev = ACPI_COMPANION(dev);
+
+ if (!adev)
+ return -ENODEV;
+
+ return wbrf_record(adev, WBRF_RECORD_ADD, in);
+}
+
+int acpi_amd_wbrf_remove_exclusion(struct device *dev,
+ struct wbrf_ranges_in *in)
+{
+ struct acpi_device *adev = ACPI_COMPANION(dev);
+
+ if (!adev)
+ return -ENODEV;
+
+ return wbrf_record(adev, WBRF_RECORD_REMOVE, in);
+}
+
+bool acpi_amd_wbrf_supported_system(void)
+{
+ acpi_status status;
+ acpi_handle handle;
+
+ status = acpi_get_handle(NULL, ACPI_AMD_WBRF_METHOD, &handle);
+
+ return ACPI_SUCCESS(status);
+}
+
+bool acpi_amd_wbrf_supported_producer(struct device *dev)
+{
+ struct acpi_device *adev = ACPI_COMPANION(dev);
+
+ if (!adev)
+ return false;
+
+ return acpi_check_dsm(adev->handle, &wifi_acpi_dsm_guid,
+ WBRF_REVISION,
+ BIT(WBRF_RECORD));
+}
+
+static union acpi_object *
+acpi_evaluate_wbrf(acpi_handle handle, u64 rev, u64 func)
+{
+ acpi_status ret;
+ struct acpi_buffer buf = {ACPI_ALLOCATE_BUFFER, NULL};
+ union acpi_object params[4];
+ struct acpi_object_list input = {
+ .count = 4,
+ .pointer = params,
+ };
+
+ params[0].type = ACPI_TYPE_INTEGER;
+ params[0].integer.value = rev;
+ params[1].type = ACPI_TYPE_INTEGER;
+ params[1].integer.value = func;
+ params[2].type = ACPI_TYPE_PACKAGE;
+ params[2].package.count = 0;
+ params[2].package.elements = NULL;
+ params[3].type = ACPI_TYPE_STRING;
+ params[3].string.length = 0;
+ params[3].string.pointer = NULL;
+
+ ret = acpi_evaluate_object(handle, "WBRF", &input, &buf);
+ if (ACPI_SUCCESS(ret))
+ return (union acpi_object *)buf.pointer;
+
+ return NULL;
+}
+
+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)
+ return true;
+
+ return false;
+}
+
+bool acpi_amd_wbrf_supported_consumer(struct device *dev)
+{
+ struct acpi_device *adev = ACPI_COMPANION(dev);
+
+ if (!adev)
+ return false;
+
+ return check_acpi_wbrf(adev->handle,
+ WBRF_REVISION,
+ BIT(WBRF_RETRIEVE));
+}
+
+int acpi_amd_wbrf_retrieve_exclusions(struct device *dev,
+ struct wbrf_ranges_out *out)
+{
+ struct acpi_device *adev = ACPI_COMPANION(dev);
+ struct amd_wbrf_ranges_out acpi_out = {0};
+ union acpi_object *obj;
+
+ if (!adev)
+ return -ENODEV;
+
+ obj = acpi_evaluate_wbrf(adev->handle,
+ WBRF_REVISION,
+ WBRF_RETRIEVE);
+ if (!obj)
+ return -EINVAL;
+
+ /*
+ * The return buffer is with variable length and the format below:
+ * number_of_entries(1 DWORD): Number of entries
+ * start_freq of 1st entry(1 QWORD): Start frequency of the 1st entry
+ * end_freq of 1st entry(1 QWORD): End frequency of the 1st entry
+ * ...
+ * ...
+ * start_freq of the last entry(1 QWORD)
+ * end_freq of the last entry(1 QWORD)
+ *
+ * Thus the buffer length is determined by the number of entries.
+ * - For zero entry scenario, the buffer length will be 4 bytes.
+ * - For one entry scenario, the buffer length will be 20 bytes.
+ */
+ if (obj->buffer.length > sizeof(acpi_out) ||
+ obj->buffer.length < 4) {
+ dev_err(dev, "BIOS FUBAR, ignoring wrong sized WBRT information");
+ ACPI_FREE(obj);
+ return -EINVAL;
+ }
+ memcpy(&acpi_out, obj->buffer.pointer, obj->buffer.length);
+
+ out->num_of_ranges = acpi_out.num_of_ranges;
+ memcpy(out->band_list, acpi_out.band_list, sizeof(acpi_out.band_list));
+
+ ACPI_FREE(obj);
+
+ return 0;
+}
diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
index 2b8fd6bb7da0..feb6f5625728 100644
--- a/drivers/base/Kconfig
+++ b/drivers/base/Kconfig
@@ -242,4 +242,24 @@ config FW_DEVLINK_SYNC_STATE_TIMEOUT
command line option on every system/board your kernel is expected to
work on.
+menu "Wifi band RF mitigation mechanism"
+
+config WBRF_AMD_ACPI
+ bool "ACPI based mechanism introduced by AMD"
+ depends on ACPI
+ help
+ Wifi band RF 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.
+
+endmenu
+
endmenu
diff --git a/drivers/base/wbrf.c b/drivers/base/wbrf.c
index 678f245c12c6..751e252d0039 100644
--- a/drivers/base/wbrf.c
+++ b/drivers/base/wbrf.c
@@ -6,9 +6,25 @@
*/
#include <linux/wbrf.h>
+#include <linux/acpi_amd_wbrf.h>
static BLOCKING_NOTIFIER_HEAD(wbrf_chain_head);
+
static DEFINE_MUTEX(wbrf_mutex);
+
+static struct exclusion_range_pool {
+ struct exclusion_range band_list[MAX_NUM_OF_WBRF_RANGES];
+ u64 ref_counter[MAX_NUM_OF_WBRF_RANGES];
+} wbrf_pool;
+
+enum WBRF_SUPPORT_CHECK {
+ WBRF_SUPPORT_UNCHECKED,
+ WBRF_SUPPORT_NONE,
+ WBRF_SUPPORT_GENERIC,
+ WBRF_SUPPORT_OTHERS,
+};
+static atomic_t wbrf_support_check = ATOMIC_INIT(WBRF_SUPPORT_UNCHECKED);
+
static enum WBRF_POLICY_MODE {
WBRF_POLICY_FORCE_DISABLE,
WBRF_POLICY_AUTO,
@@ -30,11 +46,6 @@ static int __init parse_wbrf_policy_mode(char *p)
}
early_param("wbrf", parse_wbrf_policy_mode);
-static struct exclusion_range_pool {
- struct exclusion_range band_list[MAX_NUM_OF_WBRF_RANGES];
- u64 ref_counter[MAX_NUM_OF_WBRF_RANGES];
-} wbrf_pool;
-
static int _wbrf_add_exclusion_ranges(struct wbrf_ranges_in *in)
{
int i, j;
@@ -121,20 +132,45 @@ static int _wbrf_retrieve_exclusion_ranges(struct wbrf_ranges_out *out)
*
* WBRF is used to mitigate devices that cause harmonic interference.
* This function will determine if the platform is able to support the
- * WBRF features.
+ * WBRF features. For example, for AMD ACPI implementation it should say
+ * true only when the necessary AML code/logic supporting wbrf feature
+ * available.
*/
-static bool wbrf_supported_system(void)
+static enum WBRF_SUPPORT_CHECK wbrf_supported_system(void)
{
+ enum WBRF_SUPPORT_CHECK support_check;
+
+ support_check = atomic_read(&wbrf_support_check);
+ if (support_check != WBRF_SUPPORT_UNCHECKED)
+ return support_check;
+
+ support_check = WBRF_SUPPORT_NONE;
+
switch (wbrf_policy) {
case WBRF_POLICY_FORCE_ENABLE:
- return true;
+#if IS_ENABLED(CONFIG_WBRF_AMD_ACPI)
+ if (acpi_amd_wbrf_supported_system()) {
+ support_check = WBRF_SUPPORT_OTHERS;
+ break;
+ }
+ pr_warn_once("Force WBRF w/o acpi_amd_wbrf support\n");
+ pr_warn_once("Fall back to generic version\n");
+#endif
+ support_check = WBRF_SUPPORT_GENERIC;
+ break;
case WBRF_POLICY_FORCE_DISABLE:
- return false;
+ break;
case WBRF_POLICY_AUTO:
- return false;
+#if IS_ENABLED(CONFIG_WBRF_AMD_ACPI)
+ if (acpi_amd_wbrf_supported_system())
+ support_check = WBRF_SUPPORT_OTHERS;
+#endif
+ break;
}
- return false;
+ atomic_set(&wbrf_support_check, support_check);
+
+ return support_check;
}
/**
@@ -144,13 +180,22 @@ static bool wbrf_supported_system(void)
*
* WBRF is used to mitigate devices that cause harmonic interference.
* This function will determine if this device should report such frequencies.
+ * For example, for AMD ACPI implementation it should say true only when the
+ * necessary AML code/logic supporting wbrf feature available for this device.
*/
bool wbrf_supported_producer(struct device *dev)
{
- if (!wbrf_supported_system())
+ switch (wbrf_supported_system()) {
+ case WBRF_SUPPORT_GENERIC:
+ return true;
+ case WBRF_SUPPORT_OTHERS:
+#if IS_ENABLED(CONFIG_WBRF_AMD_ACPI)
+ return acpi_amd_wbrf_supported_producer(dev);
+#endif
+ fallthrough;
+ default:
return false;
-
- return true;
+ }
}
EXPORT_SYMBOL_GPL(wbrf_supported_producer);
@@ -166,11 +211,22 @@ EXPORT_SYMBOL_GPL(wbrf_supported_producer);
int wbrf_add_exclusion(struct device *dev,
struct wbrf_ranges_in *in)
{
- int r;
+ int r = -ENODEV;
mutex_lock(&wbrf_mutex);
- r = _wbrf_add_exclusion_ranges(in);
+ switch (wbrf_supported_system()) {
+ case WBRF_SUPPORT_OTHERS:
+#if IS_ENABLED(CONFIG_WBRF_AMD_ACPI)
+ r = acpi_amd_wbrf_add_exclusion(dev, in);
+#endif
+ break;
+ case WBRF_SUPPORT_GENERIC:
+ r = _wbrf_add_exclusion_ranges(in);
+ break;
+ default:
+ break;
+ }
mutex_unlock(&wbrf_mutex);
if (r)
@@ -194,11 +250,22 @@ EXPORT_SYMBOL_GPL(wbrf_add_exclusion);
int wbrf_remove_exclusion(struct device *dev,
struct wbrf_ranges_in *in)
{
- int r;
+ int r = -ENODEV;
mutex_lock(&wbrf_mutex);
- r = _wbrf_remove_exclusion_ranges(in);
+ switch (wbrf_supported_system()) {
+ case WBRF_SUPPORT_OTHERS:
+#if IS_ENABLED(CONFIG_WBRF_AMD_ACPI)
+ r = acpi_amd_wbrf_remove_exclusion(dev, in);
+#endif
+ break;
+ case WBRF_SUPPORT_GENERIC:
+ r = _wbrf_remove_exclusion_ranges(in);
+ break;
+ default:
+ break;
+ }
mutex_unlock(&wbrf_mutex);
if (r)
@@ -217,14 +284,23 @@ EXPORT_SYMBOL_GPL(wbrf_remove_exclusion);
*
* WBRF is used to mitigate devices that cause harmonic interference.
* This function will determine if this device should react to reports from
- * other devices for such frequencies.
+ * other devices for such frequencies. For example, for AMD ACPI implementation
+ * it should say true only when the necessary AML code/logic supporting wbrf
+ * feature available for this device.
*/
bool wbrf_supported_consumer(struct device *dev)
{
- if (!wbrf_supported_system())
+ switch (wbrf_supported_system()) {
+ case WBRF_SUPPORT_GENERIC:
+ return true;
+ case WBRF_SUPPORT_OTHERS:
+#if IS_ENABLED(CONFIG_WBRF_AMD_ACPI)
+ return acpi_amd_wbrf_supported_consumer(dev);
+#endif
+ fallthrough;
+ default:
return false;
-
- return true;
+ }
}
EXPORT_SYMBOL_GPL(wbrf_supported_consumer);
@@ -267,11 +343,22 @@ EXPORT_SYMBOL_GPL(wbrf_unregister_notifier);
int wbrf_retrieve_exclusions(struct device *dev,
struct wbrf_ranges_out *out)
{
- int r;
+ int r = -ENODEV;
mutex_lock(&wbrf_mutex);
- r = _wbrf_retrieve_exclusion_ranges(out);
+ switch (wbrf_supported_system()) {
+ case WBRF_SUPPORT_OTHERS:
+#if IS_ENABLED(CONFIG_WBRF_AMD_ACPI)
+ r = acpi_amd_wbrf_retrieve_exclusions(dev, out);
+#endif
+ break;
+ case WBRF_SUPPORT_GENERIC:
+ r = _wbrf_retrieve_exclusion_ranges(out);
+ break;
+ default:
+ break;
+ }
mutex_unlock(&wbrf_mutex);
diff --git a/include/linux/acpi_amd_wbrf.h b/include/linux/acpi_amd_wbrf.h
new file mode 100644
index 000000000000..40c59e9f626d
--- /dev/null
+++ b/include/linux/acpi_amd_wbrf.h
@@ -0,0 +1,25 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Wifi Band Exclusion Interface (AMD ACPI Implementation)
+ * Copyright (C) 2023 Advanced Micro Devices
+ *
+ */
+
+#ifndef _ACPI_AMD_WBRF_H
+#define _ACPI_AMD_WBRF_H
+
+#include <linux/wbrf.h>
+
+#if IS_ENABLED(CONFIG_WBRF_AMD_ACPI)
+bool acpi_amd_wbrf_supported_system(void);
+bool acpi_amd_wbrf_supported_consumer(struct device *dev);
+bool acpi_amd_wbrf_supported_producer(struct device *dev);
+int acpi_amd_wbrf_remove_exclusion(struct device *dev,
+ struct wbrf_ranges_in *in);
+int acpi_amd_wbrf_add_exclusion(struct device *dev,
+ struct wbrf_ranges_in *in);
+int acpi_amd_wbrf_retrieve_exclusions(struct device *dev,
+ struct wbrf_ranges_out *out);
+#endif
+
+#endif /* _ACPI_AMD_WBRF_H */
--
2.34.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [V9 2/9] drivers core: add ACPI based WBRF mechanism introduced by AMD
2023-08-18 3:26 ` [V9 2/9] drivers core: add ACPI based WBRF mechanism introduced by AMD Evan Quan
@ 2023-08-18 17:41 ` Rafael J. Wysocki
0 siblings, 0 replies; 25+ messages in thread
From: Rafael J. Wysocki @ 2023-08-18 17:41 UTC (permalink / raw)
To: Evan Quan
Cc: gregkh, rafael, lenb, johannes, davem, edumazet, kuba, pabeni,
alexander.deucher, andrew, rdunlap, quic_jjohnson, horms,
linux-doc, linux-kernel, linux-acpi, amd-gfx, linux-wireless,
netdev, Mario Limonciello
On Fri, Aug 18, 2023 at 5:27 AM Evan Quan <evan.quan@amd.com> 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.
This needs a problem statement in the first place: What exactly caused
AMD to come up with this design?
> 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".
OK, so I suppose that the patch implements support for the AMD WBRF
firmware interface? That needs to be stated somewhere.
From patch reverse-engineering it looks like the generic WBRF code is
updated by it to hook up to the ACPI implementation if supported. If
my understanding is correct, it would be nice to state that in the
changelog too.
> Co-developed-by: Mario Limonciello <mario.limonciello@amd.com>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> Co-developed-by: Evan Quan <evan.quan@amd.com>
> Signed-off-by: Evan Quan <evan.quan@amd.com>
> --
> 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)
> v8->v9:
> - correct some coding style(Simon)
> ---
> 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 000000000000..0e46de3dfac7
> --- /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
> + *
It would be nice to have a description of the firmware interface here,
at least in general terms.
In particular, the terminology used throughout the code must be explained.
Without it, qualifying the validity and/or usefulness of the code is
rather hard.
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/acpi_amd_wbrf.h>
> +
> +#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_RECORD 0x1
> +#define WBRF_RETRIEVE 0x2
> +
> +/* record actions */
> +#define WBRF_RECORD_ADD 0x0
> +#define WBRF_RECORD_REMOVE 0x1
> +
> +#define WBRF_REVISION 0x1
> +
> +/*
> + * The data structure used for WBRF_RETRIEVE is not natually aligned.
"naturally"
> + * 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, &wifi_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;
> +}
The calling convention of this function isn't particularly straightforward.
Also, AFAICS, it has only one caller which passes NULL as the last
argument (which is not checked above when obj->type is
ACPI_TYPE_BUFFER, so I guess it's never been the case in practice) and
discards whatever is passed via arg4.
Why is arg4 even needed and why is the ACPI_TYPE_BUFFER case regarded
as a valid one?
> +
> +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
Every range of what?
> + * 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)
> + return -ENOMEM;
> +
> + argv4.package.type = ACPI_TYPE_PACKAGE;
> + argv4.package.count = num_of_elements;
> + argv4.package.elements = tmp;
> +
> + tmp[arg_idx].integer.type = ACPI_TYPE_INTEGER;
> + tmp[arg_idx++].integer.value = num_of_ranges;
> + tmp[arg_idx].integer.type = ACPI_TYPE_INTEGER;
> + tmp[arg_idx++].integer.value = action;
> +
> + 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)
> + continue;
> +
> + tmp[arg_idx].integer.type = ACPI_TYPE_INTEGER;
> + tmp[arg_idx++].integer.value = in->band_list[loop_idx].start;
> + tmp[arg_idx].integer.type = ACPI_TYPE_INTEGER;
> + tmp[arg_idx++].integer.value = in->band_list[loop_idx].end;
> + }
> +
> + ret = wbrf_dsm(adev, WBRF_RECORD, &argv4, NULL);
> +
> + kfree(tmp);
> +
> + return ret;
> +}
> +
> +int acpi_amd_wbrf_add_exclusion(struct device *dev,
> + struct wbrf_ranges_in *in)
> +{
> + struct acpi_device *adev = ACPI_COMPANION(dev);
> +
> + if (!adev)
> + return -ENODEV;
> +
> + return wbrf_record(adev, WBRF_RECORD_ADD, in);
> +}
> +
> +int acpi_amd_wbrf_remove_exclusion(struct device *dev,
> + struct wbrf_ranges_in *in)
> +{
> + struct acpi_device *adev = ACPI_COMPANION(dev);
> +
> + if (!adev)
> + return -ENODEV;
> +
> + return wbrf_record(adev, WBRF_RECORD_REMOVE, in);
> +}
> +
> +bool acpi_amd_wbrf_supported_system(void)
> +{
> + acpi_status status;
> + acpi_handle handle;
> +
> + status = acpi_get_handle(NULL, ACPI_AMD_WBRF_METHOD, &handle);
> +
> + return ACPI_SUCCESS(status);
> +}
> +
> +bool acpi_amd_wbrf_supported_producer(struct device *dev)
> +{
> + struct acpi_device *adev = ACPI_COMPANION(dev);
> +
> + if (!adev)
> + return false;
> +
> + return acpi_check_dsm(adev->handle, &wifi_acpi_dsm_guid,
> + WBRF_REVISION,
> + BIT(WBRF_RECORD));
> +}
All of the non-static functions need kerneldoc comments. None of them has one.
> +
> +static union acpi_object *
> +acpi_evaluate_wbrf(acpi_handle handle, u64 rev, u64 func)
> +{
> + acpi_status ret;
> + struct acpi_buffer buf = {ACPI_ALLOCATE_BUFFER, NULL};
> + union acpi_object params[4];
> + struct acpi_object_list input = {
> + .count = 4,
> + .pointer = params,
> + };
> +
> + params[0].type = ACPI_TYPE_INTEGER;
> + params[0].integer.value = rev;
> + params[1].type = ACPI_TYPE_INTEGER;
> + params[1].integer.value = func;
> + params[2].type = ACPI_TYPE_PACKAGE;
> + params[2].package.count = 0;
> + params[2].package.elements = NULL;
> + params[3].type = ACPI_TYPE_STRING;
> + params[3].string.length = 0;
> + params[3].string.pointer = NULL;
> +
> + ret = acpi_evaluate_object(handle, "WBRF", &input, &buf);
> + if (ACPI_SUCCESS(ret))
> + return (union acpi_object *)buf.pointer;
> +
> + return NULL;
I would do it the other way around, that is
if (ACPI_FAILURE(ret))
return NULL;
return buf.pointer;
and the pointer cast is not necessary, because buf.pointer is void anyway.
> +}
> +
> +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));
The parens in the above expression are all redundant AFAICS.
And why does this need to be so complicated? There's only one caller
that passes only one bit in funcs, so why is this processing needed at
all? It looks like it would be better to fold this into its caller.
> +
> + ACPI_FREE(obj);
> +
> + if ((mask & BIT(WBRF_ENABLED)) &&
> + (mask & funcs) == funcs)
> + return true;
> +
> + return false;
return mask & BIT(WBRF_ENABLED) && (mask & funcs) == funcs;
> +}
> +
> +bool acpi_amd_wbrf_supported_consumer(struct device *dev)
> +{
> + struct acpi_device *adev = ACPI_COMPANION(dev);
> +
> + if (!adev)
> + return false;
> +
> + return check_acpi_wbrf(adev->handle,
> + WBRF_REVISION,
> + BIT(WBRF_RETRIEVE));
> +}
> +
> +int acpi_amd_wbrf_retrieve_exclusions(struct device *dev,
> + struct wbrf_ranges_out *out)
> +{
> + struct acpi_device *adev = ACPI_COMPANION(dev);
> + struct amd_wbrf_ranges_out acpi_out = {0};
> + union acpi_object *obj;
> +
> + if (!adev)
> + return -ENODEV;
> +
> + obj = acpi_evaluate_wbrf(adev->handle,
> + WBRF_REVISION,
> + WBRF_RETRIEVE);
> + if (!obj)
> + return -EINVAL;
> +
> + /*
> + * The return buffer is with variable length and the format below:
> + * number_of_entries(1 DWORD): Number of entries
> + * start_freq of 1st entry(1 QWORD): Start frequency of the 1st entry
> + * end_freq of 1st entry(1 QWORD): End frequency of the 1st entry
> + * ...
> + * ...
> + * start_freq of the last entry(1 QWORD)
> + * end_freq of the last entry(1 QWORD)
> + *
> + * Thus the buffer length is determined by the number of entries.
> + * - For zero entry scenario, the buffer length will be 4 bytes.
> + * - For one entry scenario, the buffer length will be 20 bytes.
> + */
> + if (obj->buffer.length > sizeof(acpi_out) ||
> + obj->buffer.length < 4) {
> + dev_err(dev, "BIOS FUBAR, ignoring wrong sized WBRT information");
What does FUBAR mean here?
Why is it printed with dev_err()?
> + ACPI_FREE(obj);
> + return -EINVAL;
This can jump to a label instead of doing a duplicate ACPI_FREE(obj).
> + }
> + memcpy(&acpi_out, obj->buffer.pointer, obj->buffer.length);
> +
> + out->num_of_ranges = acpi_out.num_of_ranges;
> + memcpy(out->band_list, acpi_out.band_list, sizeof(acpi_out.band_list));
While using memcpy() here seems nicer, I would copy the list in a
proper loop item by item. Then it wouldn't be necessary to match the
sizes of elements of the source and destination arrays.
> +
> + ACPI_FREE(obj);
> +
> + return 0;
> +}
> diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
> index 2b8fd6bb7da0..feb6f5625728 100644
> --- a/drivers/base/Kconfig
> +++ b/drivers/base/Kconfig
> @@ -242,4 +242,24 @@ config FW_DEVLINK_SYNC_STATE_TIMEOUT
> command line option on every system/board your kernel is expected to
> work on.
>
> +menu "Wifi band RF mitigation mechanism"
> +
> +config WBRF_AMD_ACPI
> + bool "ACPI based mechanism introduced by AMD"
> + depends on ACPI
> + help
> + Wifi band RF mitigation mechanism allows multiple drivers from
> + different domains to notify the frequencies in use so that hardware
s/notify/report/ I think.
> + 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.
The above is completely unworkable for distro kernel providers who
release one binary kernel that needs to work on all platforms.
> +
> +endmenu
> +
> endmenu
> diff --git a/drivers/base/wbrf.c b/drivers/base/wbrf.c
> index 678f245c12c6..751e252d0039 100644
> --- a/drivers/base/wbrf.c
> +++ b/drivers/base/wbrf.c
> @@ -6,9 +6,25 @@
> */
>
> #include <linux/wbrf.h>
> +#include <linux/acpi_amd_wbrf.h>
>
> static BLOCKING_NOTIFIER_HEAD(wbrf_chain_head);
> +
> static DEFINE_MUTEX(wbrf_mutex);
> +
> +static struct exclusion_range_pool {
> + struct exclusion_range band_list[MAX_NUM_OF_WBRF_RANGES];
> + u64 ref_counter[MAX_NUM_OF_WBRF_RANGES];
> +} wbrf_pool;
> +
> +enum WBRF_SUPPORT_CHECK {
> + WBRF_SUPPORT_UNCHECKED,
> + WBRF_SUPPORT_NONE,
> + WBRF_SUPPORT_GENERIC,
> + WBRF_SUPPORT_OTHERS,
> +};
> +static atomic_t wbrf_support_check = ATOMIC_INIT(WBRF_SUPPORT_UNCHECKED);
> +
> static enum WBRF_POLICY_MODE {
> WBRF_POLICY_FORCE_DISABLE,
> WBRF_POLICY_AUTO,
> @@ -30,11 +46,6 @@ static int __init parse_wbrf_policy_mode(char *p)
> }
> early_param("wbrf", parse_wbrf_policy_mode);
>
> -static struct exclusion_range_pool {
> - struct exclusion_range band_list[MAX_NUM_OF_WBRF_RANGES];
> - u64 ref_counter[MAX_NUM_OF_WBRF_RANGES];
> -} wbrf_pool;
> -
So the previous patch should not add this definition here, should it?
> static int _wbrf_add_exclusion_ranges(struct wbrf_ranges_in *in)
> {
> int i, j;
> @@ -121,20 +132,45 @@ static int _wbrf_retrieve_exclusion_ranges(struct wbrf_ranges_out *out)
> *
> * WBRF is used to mitigate devices that cause harmonic interference.
> * This function will determine if the platform is able to support the
> - * WBRF features.
> + * WBRF features. For example, for AMD ACPI implementation it should say
> + * true only when the necessary AML code/logic supporting wbrf feature
> + * available.
The additional kerneldoc text doesn't seem to be particularly useful to me.
> */
> -static bool wbrf_supported_system(void)
> +static enum WBRF_SUPPORT_CHECK wbrf_supported_system(void)
> {
> + enum WBRF_SUPPORT_CHECK support_check;
> +
> + support_check = atomic_read(&wbrf_support_check);
> + if (support_check != WBRF_SUPPORT_UNCHECKED)
> + return support_check;
> +
> + support_check = WBRF_SUPPORT_NONE;
> +
> switch (wbrf_policy) {
> case WBRF_POLICY_FORCE_ENABLE:
> - return true;
> +#if IS_ENABLED(CONFIG_WBRF_AMD_ACPI)
No #if here, please.
> + if (acpi_amd_wbrf_supported_system()) {
> + support_check = WBRF_SUPPORT_OTHERS;
> + break;
> + }
> + pr_warn_once("Force WBRF w/o acpi_amd_wbrf support\n");
> + pr_warn_once("Fall back to generic version\n");
> +#endif
> + support_check = WBRF_SUPPORT_GENERIC;
> + break;
> case WBRF_POLICY_FORCE_DISABLE:
> - return false;
> + break;
> case WBRF_POLICY_AUTO:
> - return false;
> +#if IS_ENABLED(CONFIG_WBRF_AMD_ACPI)
And same here.
> + if (acpi_amd_wbrf_supported_system())
> + support_check = WBRF_SUPPORT_OTHERS;
> +#endif
> + break;
> }
>
> - return false;
> + atomic_set(&wbrf_support_check, support_check);
> +
> + return support_check;
> }
>
> /**
> @@ -144,13 +180,22 @@ static bool wbrf_supported_system(void)
> *
> * WBRF is used to mitigate devices that cause harmonic interference.
> * This function will determine if this device should report such frequencies.
> + * For example, for AMD ACPI implementation it should say true only when the
> + * necessary AML code/logic supporting wbrf feature available for this device.
Again, the usefulness of the added kerneldoc text is quite questionable IMV.
> */
> bool wbrf_supported_producer(struct device *dev)
> {
> - if (!wbrf_supported_system())
> + switch (wbrf_supported_system()) {
> + case WBRF_SUPPORT_GENERIC:
> + return true;
> + case WBRF_SUPPORT_OTHERS:
> +#if IS_ENABLED(CONFIG_WBRF_AMD_ACPI)
Again, no #if here, please.
> + return acpi_amd_wbrf_supported_producer(dev);
> +#endif
> + fallthrough;
> + default:
> return false;
> -
> - return true;
> + }
> }
> EXPORT_SYMBOL_GPL(wbrf_supported_producer);
>
> @@ -166,11 +211,22 @@ EXPORT_SYMBOL_GPL(wbrf_supported_producer);
> int wbrf_add_exclusion(struct device *dev,
> struct wbrf_ranges_in *in)
> {
> - int r;
> + int r = -ENODEV;
>
> mutex_lock(&wbrf_mutex);
>
> - r = _wbrf_add_exclusion_ranges(in);
> + switch (wbrf_supported_system()) {
> + case WBRF_SUPPORT_OTHERS:
> +#if IS_ENABLED(CONFIG_WBRF_AMD_ACPI)
Same here.
> + r = acpi_amd_wbrf_add_exclusion(dev, in);
> +#endif
> + break;
> + case WBRF_SUPPORT_GENERIC:
> + r = _wbrf_add_exclusion_ranges(in);
> + break;
> + default:
> + break;
> + }
>
> mutex_unlock(&wbrf_mutex);
> if (r)
> @@ -194,11 +250,22 @@ EXPORT_SYMBOL_GPL(wbrf_add_exclusion);
> int wbrf_remove_exclusion(struct device *dev,
> struct wbrf_ranges_in *in)
> {
> - int r;
> + int r = -ENODEV;
>
> mutex_lock(&wbrf_mutex);
>
> - r = _wbrf_remove_exclusion_ranges(in);
> + switch (wbrf_supported_system()) {
> + case WBRF_SUPPORT_OTHERS:
> +#if IS_ENABLED(CONFIG_WBRF_AMD_ACPI)
Same here.
> + r = acpi_amd_wbrf_remove_exclusion(dev, in);
> +#endif
> + break;
> + case WBRF_SUPPORT_GENERIC:
> + r = _wbrf_remove_exclusion_ranges(in);
> + break;
> + default:
> + break;
> + }
>
> mutex_unlock(&wbrf_mutex);
> if (r)
> @@ -217,14 +284,23 @@ EXPORT_SYMBOL_GPL(wbrf_remove_exclusion);
> *
> * WBRF is used to mitigate devices that cause harmonic interference.
> * This function will determine if this device should react to reports from
> - * other devices for such frequencies.
> + * other devices for such frequencies. For example, for AMD ACPI implementation
> + * it should say true only when the necessary AML code/logic supporting wbrf
> + * feature available for this device.
> */
> bool wbrf_supported_consumer(struct device *dev)
> {
> - if (!wbrf_supported_system())
> + switch (wbrf_supported_system()) {
> + case WBRF_SUPPORT_GENERIC:
> + return true;
> + case WBRF_SUPPORT_OTHERS:
> +#if IS_ENABLED(CONFIG_WBRF_AMD_ACPI)
Same here.
> + return acpi_amd_wbrf_supported_consumer(dev);
> +#endif
> + fallthrough;
> + default:
> return false;
> -
> - return true;
> + }
> }
> EXPORT_SYMBOL_GPL(wbrf_supported_consumer);
>
> @@ -267,11 +343,22 @@ EXPORT_SYMBOL_GPL(wbrf_unregister_notifier);
> int wbrf_retrieve_exclusions(struct device *dev,
> struct wbrf_ranges_out *out)
> {
> - int r;
> + int r = -ENODEV;
>
> mutex_lock(&wbrf_mutex);
>
> - r = _wbrf_retrieve_exclusion_ranges(out);
> + switch (wbrf_supported_system()) {
> + case WBRF_SUPPORT_OTHERS:
> +#if IS_ENABLED(CONFIG_WBRF_AMD_ACPI)
Same here.
> + r = acpi_amd_wbrf_retrieve_exclusions(dev, out);
> +#endif
> + break;
> + case WBRF_SUPPORT_GENERIC:
> + r = _wbrf_retrieve_exclusion_ranges(out);
> + break;
> + default:
> + break;
> + }
>
> mutex_unlock(&wbrf_mutex);
>
> diff --git a/include/linux/acpi_amd_wbrf.h b/include/linux/acpi_amd_wbrf.h
> new file mode 100644
> index 000000000000..40c59e9f626d
> --- /dev/null
> +++ b/include/linux/acpi_amd_wbrf.h
> @@ -0,0 +1,25 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Wifi Band Exclusion Interface (AMD ACPI Implementation)
> + * Copyright (C) 2023 Advanced Micro Devices
> + *
> + */
> +
> +#ifndef _ACPI_AMD_WBRF_H
> +#define _ACPI_AMD_WBRF_H
> +
> +#include <linux/wbrf.h>
> +
> +#if IS_ENABLED(CONFIG_WBRF_AMD_ACPI)
> +bool acpi_amd_wbrf_supported_system(void);
> +bool acpi_amd_wbrf_supported_consumer(struct device *dev);
> +bool acpi_amd_wbrf_supported_producer(struct device *dev);
> +int acpi_amd_wbrf_remove_exclusion(struct device *dev,
> + struct wbrf_ranges_in *in);
> +int acpi_amd_wbrf_add_exclusion(struct device *dev,
> + struct wbrf_ranges_in *in);
> +int acpi_amd_wbrf_retrieve_exclusions(struct device *dev,
> + struct wbrf_ranges_out *out);
#else
Empty subs for all of the requisite functions go here.
> +#endif
> +
> +#endif /* _ACPI_AMD_WBRF_H */
> --
^ permalink raw reply [flat|nested] 25+ messages in thread
* [V9 3/9] cfg80211: expose nl80211_chan_width_to_mhz for wide sharing
2023-08-18 3:26 [V9 0/9] Enable Wifi RFI interference mitigation feature support Evan Quan
2023-08-18 3:26 ` [V9 1/9] drivers core: Add support for Wifi band RF mitigations Evan Quan
2023-08-18 3:26 ` [V9 2/9] drivers core: add ACPI based WBRF mechanism introduced by AMD Evan Quan
@ 2023-08-18 3:26 ` Evan Quan
2023-08-18 3:26 ` [V9 4/9] wifi: mac80211: Add support for WBRF features Evan Quan
` (5 subsequent siblings)
8 siblings, 0 replies; 25+ messages in thread
From: Evan Quan @ 2023-08-18 3:26 UTC (permalink / raw)
To: gregkh, rafael, lenb, johannes, davem, edumazet, kuba, pabeni,
alexander.deucher, andrew, rdunlap, quic_jjohnson, horms
Cc: linux-doc, linux-kernel, linux-acpi, amd-gfx, linux-wireless,
netdev, Evan Quan
The newly added WBRF feature needs this interface for channel
width calculation.
Signed-off-by: Evan Quan <evan.quan@amd.com>
--
v8->v9:
- correct typo(Mhz -> MHz) (Johnson)
---
include/net/cfg80211.h | 8 ++++++++
net/wireless/chan.c | 3 ++-
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 7c7d03aa9d06..8c2a9b748621 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -920,6 +920,14 @@ const struct cfg80211_chan_def *
cfg80211_chandef_compatible(const struct cfg80211_chan_def *chandef1,
const struct cfg80211_chan_def *chandef2);
+/**
+ * nl80211_chan_width_to_mhz - get the channel width in MHz
+ * @chan_width: the channel width from &enum nl80211_chan_width
+ * Return: channel width in MHz if the chan_width from &enum nl80211_chan_width
+ * is valid. -1 otherwise.
+ */
+int nl80211_chan_width_to_mhz(enum nl80211_chan_width chan_width);
+
/**
* cfg80211_chandef_valid - check if a channel definition is valid
* @chandef: the channel definition to check
diff --git a/net/wireless/chan.c b/net/wireless/chan.c
index 0b7e81db383d..227db04eac42 100644
--- a/net/wireless/chan.c
+++ b/net/wireless/chan.c
@@ -141,7 +141,7 @@ static bool cfg80211_edmg_chandef_valid(const struct cfg80211_chan_def *chandef)
return true;
}
-static int nl80211_chan_width_to_mhz(enum nl80211_chan_width chan_width)
+int nl80211_chan_width_to_mhz(enum nl80211_chan_width chan_width)
{
int mhz;
@@ -190,6 +190,7 @@ static int nl80211_chan_width_to_mhz(enum nl80211_chan_width chan_width)
}
return mhz;
}
+EXPORT_SYMBOL(nl80211_chan_width_to_mhz);
static int cfg80211_chandef_get_width(const struct cfg80211_chan_def *c)
{
--
2.34.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [V9 4/9] wifi: mac80211: Add support for WBRF features
2023-08-18 3:26 [V9 0/9] Enable Wifi RFI interference mitigation feature support Evan Quan
` (2 preceding siblings ...)
2023-08-18 3:26 ` [V9 3/9] cfg80211: expose nl80211_chan_width_to_mhz for wide sharing Evan Quan
@ 2023-08-18 3:26 ` Evan Quan
2023-08-21 9:44 ` Johannes Berg
2023-08-18 3:26 ` [V9 5/9] drm/amd/pm: update driver_if and ppsmc headers for coming wbrf feature Evan Quan
` (4 subsequent siblings)
8 siblings, 1 reply; 25+ messages in thread
From: Evan Quan @ 2023-08-18 3:26 UTC (permalink / raw)
To: gregkh, rafael, lenb, johannes, davem, edumazet, kuba, pabeni,
alexander.deucher, andrew, rdunlap, quic_jjohnson, horms
Cc: linux-doc, linux-kernel, linux-acpi, amd-gfx, linux-wireless,
netdev, Evan Quan, Mario Limonciello
To support the WBRF mechanism, Wifi adapters utilized in the system must
register the frequencies in use(or unregister those frequencies no longer
used) via the dedicated calls. So that, other drivers responding to the
frequencies can take proper actions to mitigate possible interference.
Co-developed-by: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
Co-developed-by: Evan Quan <evan.quan@amd.com>
Signed-off-by: Evan Quan <evan.quan@amd.com>
--
v1->v2:
- place the new added member(`wbrf_supported`) in
ieee80211_local(Johannes)
- handle chandefs change scenario properly(Johannes)
- some minor fixes around code sharing and possible invalid input
checks(Johannes)
v2->v3:
- drop unnecessary input checks and intermediate APIs(Mario)
- Separate some mac80211 common code(Mario, Johannes)
v3->v4:
- some minor fixes around return values(Johannes)
---
include/linux/ieee80211.h | 1 +
net/mac80211/Makefile | 2 +
net/mac80211/chan.c | 9 ++++
net/mac80211/ieee80211_i.h | 9 ++++
net/mac80211/main.c | 2 +
net/mac80211/wbrf.c | 103 +++++++++++++++++++++++++++++++++++++
6 files changed, 126 insertions(+)
create mode 100644 net/mac80211/wbrf.c
diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
index 4b998090898e..f995d06da87f 100644
--- a/include/linux/ieee80211.h
+++ b/include/linux/ieee80211.h
@@ -4335,6 +4335,7 @@ static inline int ieee80211_get_tdls_action(struct sk_buff *skb, u32 hdr_size)
/* convert frequencies */
#define MHZ_TO_KHZ(freq) ((freq) * 1000)
#define KHZ_TO_MHZ(freq) ((freq) / 1000)
+#define KHZ_TO_HZ(freq) ((freq) * 1000)
#define PR_KHZ(f) KHZ_TO_MHZ(f), f % 1000
#define KHZ_F "%d.%03d"
diff --git a/net/mac80211/Makefile b/net/mac80211/Makefile
index b8de44da1fb8..d46c36f55fd3 100644
--- a/net/mac80211/Makefile
+++ b/net/mac80211/Makefile
@@ -65,4 +65,6 @@ rc80211_minstrel-$(CONFIG_MAC80211_DEBUGFS) += \
mac80211-$(CONFIG_MAC80211_RC_MINSTREL) += $(rc80211_minstrel-y)
+mac80211-y += wbrf.o
+
ccflags-y += -DDEBUG
diff --git a/net/mac80211/chan.c b/net/mac80211/chan.c
index 68952752b599..458469c224ae 100644
--- a/net/mac80211/chan.c
+++ b/net/mac80211/chan.c
@@ -506,11 +506,16 @@ static void _ieee80211_change_chanctx(struct ieee80211_local *local,
WARN_ON(!cfg80211_chandef_compatible(&ctx->conf.def, chandef));
+ ieee80211_remove_wbrf(local, &ctx->conf.def);
+
ctx->conf.def = *chandef;
/* check if min chanctx also changed */
changed = IEEE80211_CHANCTX_CHANGE_WIDTH |
_ieee80211_recalc_chanctx_min_def(local, ctx, rsvd_for);
+
+ ieee80211_add_wbrf(local, &ctx->conf.def);
+
drv_change_chanctx(local, ctx, changed);
if (!local->use_chanctx) {
@@ -668,6 +673,8 @@ static int ieee80211_add_chanctx(struct ieee80211_local *local,
lockdep_assert_held(&local->mtx);
lockdep_assert_held(&local->chanctx_mtx);
+ ieee80211_add_wbrf(local, &ctx->conf.def);
+
if (!local->use_chanctx)
local->hw.conf.radar_enabled = ctx->conf.radar_enabled;
@@ -748,6 +755,8 @@ static void ieee80211_del_chanctx(struct ieee80211_local *local,
}
ieee80211_recalc_idle(local);
+
+ ieee80211_remove_wbrf(local, &ctx->conf.def);
}
static void ieee80211_free_chanctx(struct ieee80211_local *local,
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 91633a0b723e..719f2c892132 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1600,6 +1600,8 @@ struct ieee80211_local {
/* extended capabilities provided by mac80211 */
u8 ext_capa[8];
+
+ bool wbrf_supported;
};
static inline struct ieee80211_sub_if_data *
@@ -2638,4 +2640,11 @@ ieee80211_eht_cap_ie_to_sta_eht_cap(struct ieee80211_sub_if_data *sdata,
const struct ieee80211_eht_cap_elem *eht_cap_ie_elem,
u8 eht_cap_len,
struct link_sta_info *link_sta);
+
+void ieee80211_check_wbrf_support(struct ieee80211_local *local);
+void ieee80211_add_wbrf(struct ieee80211_local *local,
+ struct cfg80211_chan_def *chandef);
+void ieee80211_remove_wbrf(struct ieee80211_local *local,
+ struct cfg80211_chan_def *chandef);
+
#endif /* IEEE80211_I_H */
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index 24315d7b3126..b20bdaac84db 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -1396,6 +1396,8 @@ int ieee80211_register_hw(struct ieee80211_hw *hw)
debugfs_hw_add(local);
rate_control_add_debugfs(local);
+ ieee80211_check_wbrf_support(local);
+
rtnl_lock();
wiphy_lock(hw->wiphy);
diff --git a/net/mac80211/wbrf.c b/net/mac80211/wbrf.c
new file mode 100644
index 000000000000..7ddb29d128b1
--- /dev/null
+++ b/net/mac80211/wbrf.c
@@ -0,0 +1,103 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Wifi Band Exclusion Interface for WWAN
+ * Copyright (C) 2023 Advanced Micro Devices
+ *
+ */
+
+#include <linux/wbrf.h>
+#include <net/cfg80211.h>
+#include "ieee80211_i.h"
+
+void ieee80211_check_wbrf_support(struct ieee80211_local *local)
+{
+ struct wiphy *wiphy = local->hw.wiphy;
+ struct device *dev;
+
+ if (!wiphy)
+ return;
+
+ dev = wiphy->dev.parent;
+ if (!dev)
+ return;
+
+ local->wbrf_supported = wbrf_supported_producer(dev);
+ dev_dbg(dev, "WBRF is %s supported\n",
+ local->wbrf_supported ? "" : "not");
+}
+
+static void get_chan_freq_boundary(u32 center_freq,
+ u32 bandwidth,
+ u64 *start,
+ u64 *end)
+{
+ bandwidth = MHZ_TO_KHZ(bandwidth);
+ center_freq = MHZ_TO_KHZ(center_freq);
+
+ *start = center_freq - bandwidth / 2;
+ *end = center_freq + bandwidth / 2;
+
+ /* Frequency in HZ is expected */
+ *start = KHZ_TO_HZ(*start);
+ *end = KHZ_TO_HZ(*end);
+}
+
+static void wbrf_get_ranges_from_chandef(struct cfg80211_chan_def *chandef,
+ struct wbrf_ranges_in *ranges_in)
+{
+ u64 start_freq1, end_freq1;
+ u64 start_freq2, end_freq2;
+ int bandwidth;
+
+ bandwidth = nl80211_chan_width_to_mhz(chandef->width);
+
+ get_chan_freq_boundary(chandef->center_freq1,
+ bandwidth,
+ &start_freq1,
+ &end_freq1);
+
+ ranges_in->band_list[0].start = start_freq1;
+ ranges_in->band_list[0].end = end_freq1;
+
+ if (chandef->width == NL80211_CHAN_WIDTH_80P80) {
+ get_chan_freq_boundary(chandef->center_freq2,
+ bandwidth,
+ &start_freq2,
+ &end_freq2);
+
+ ranges_in->band_list[1].start = start_freq2;
+ ranges_in->band_list[1].end = end_freq2;
+ }
+}
+
+void ieee80211_add_wbrf(struct ieee80211_local *local,
+ struct cfg80211_chan_def *chandef)
+{
+ struct wbrf_ranges_in ranges_in = {0};
+ struct device *dev;
+
+ if (!local->wbrf_supported)
+ return;
+
+ dev = local->hw.wiphy->dev.parent;
+
+ wbrf_get_ranges_from_chandef(chandef, &ranges_in);
+
+ wbrf_add_exclusion(dev, &ranges_in);
+}
+
+void ieee80211_remove_wbrf(struct ieee80211_local *local,
+ struct cfg80211_chan_def *chandef)
+{
+ struct wbrf_ranges_in ranges_in = {0};
+ struct device *dev;
+
+ if (!local->wbrf_supported)
+ return;
+
+ dev = local->hw.wiphy->dev.parent;
+
+ wbrf_get_ranges_from_chandef(chandef, &ranges_in);
+
+ wbrf_remove_exclusion(dev, &ranges_in);
+}
--
2.34.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [V9 4/9] wifi: mac80211: Add support for WBRF features
2023-08-18 3:26 ` [V9 4/9] wifi: mac80211: Add support for WBRF features Evan Quan
@ 2023-08-21 9:44 ` Johannes Berg
2023-08-25 8:47 ` Quan, Evan
0 siblings, 1 reply; 25+ messages in thread
From: Johannes Berg @ 2023-08-21 9:44 UTC (permalink / raw)
To: Evan Quan, gregkh, rafael, lenb, davem, edumazet, kuba, pabeni,
alexander.deucher, andrew, rdunlap, quic_jjohnson, horms
Cc: linux-doc, linux-kernel, linux-acpi, amd-gfx, linux-wireless,
netdev, Mario Limonciello
On Fri, 2023-08-18 at 11:26 +0800, Evan Quan wrote:
> To support the WBRF mechanism, Wifi adapters utilized in the system must
> register the frequencies in use(or unregister those frequencies no longer
> used) via the dedicated calls. So that, other drivers responding to the
> frequencies can take proper actions to mitigate possible interference.
>
> Co-developed-by: Mario Limonciello <mario.limonciello@amd.com>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> Co-developed-by: Evan Quan <evan.quan@amd.com>
> Signed-off-by: Evan Quan <evan.quan@amd.com>
From WiFi POV, this looks _almost_ fine to me.
> +static void wbrf_get_ranges_from_chandef(struct cfg80211_chan_def *chandef,
> + struct wbrf_ranges_in *ranges_in)
> +{
> + u64 start_freq1, end_freq1;
> + u64 start_freq2, end_freq2;
> + int bandwidth;
> +
> + bandwidth = nl80211_chan_width_to_mhz(chandef->width);
> +
> + get_chan_freq_boundary(chandef->center_freq1,
> + bandwidth,
> + &start_freq1,
> + &end_freq1);
> +
> + ranges_in->band_list[0].start = start_freq1;
> + ranges_in->band_list[0].end = end_freq1;
> +
> + if (chandef->width == NL80211_CHAN_WIDTH_80P80) {
> + get_chan_freq_boundary(chandef->center_freq2,
> + bandwidth,
> + &start_freq2,
> + &end_freq2);
> +
> + ranges_in->band_list[1].start = start_freq2;
> + ranges_in->band_list[1].end = end_freq2;
> + }
> +}
This has to setup ranges_in->num_of_ranges, no?
(Also no real good reason for num_of_ranges to be a u64, btw, since it
can only go up to 11)
With that fixed, you can add
Reviewed-by: Johannes Berg <johannes@sipsolutions.net>
johannes
^ permalink raw reply [flat|nested] 25+ messages in thread* RE: [V9 4/9] wifi: mac80211: Add support for WBRF features
2023-08-21 9:44 ` Johannes Berg
@ 2023-08-25 8:47 ` Quan, Evan
0 siblings, 0 replies; 25+ messages in thread
From: Quan, Evan @ 2023-08-25 8:47 UTC (permalink / raw)
To: Johannes Berg, gregkh@linuxfoundation.org, rafael@kernel.org,
lenb@kernel.org, davem@davemloft.net, edumazet@google.com,
kuba@kernel.org, pabeni@redhat.com, Deucher, Alexander,
andrew@lunn.ch, rdunlap@infradead.org, quic_jjohnson@quicinc.com,
horms@kernel.org
Cc: linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-acpi@vger.kernel.org, amd-gfx@lists.freedesktop.org,
linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
Limonciello, Mario
[AMD Official Use Only - General]
> -----Original Message-----
> From: Johannes Berg <johannes@sipsolutions.net>
> Sent: Monday, August 21, 2023 5:44 PM
> To: Quan, Evan <Evan.Quan@amd.com>; gregkh@linuxfoundation.org;
> rafael@kernel.org; lenb@kernel.org; davem@davemloft.net;
> edumazet@google.com; kuba@kernel.org; pabeni@redhat.com; Deucher,
> Alexander <Alexander.Deucher@amd.com>; andrew@lunn.ch;
> rdunlap@infradead.org; quic_jjohnson@quicinc.com; horms@kernel.org
> Cc: linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
> acpi@vger.kernel.org; amd-gfx@lists.freedesktop.org; linux-
> wireless@vger.kernel.org; netdev@vger.kernel.org; Limonciello, Mario
> <Mario.Limonciello@amd.com>
> Subject: Re: [V9 4/9] wifi: mac80211: Add support for WBRF features
>
> On Fri, 2023-08-18 at 11:26 +0800, Evan Quan wrote:
> > To support the WBRF mechanism, Wifi adapters utilized in the system
> > must register the frequencies in use(or unregister those frequencies
> > no longer
> > used) via the dedicated calls. So that, other drivers responding to
> > the frequencies can take proper actions to mitigate possible interference.
> >
> > Co-developed-by: Mario Limonciello <mario.limonciello@amd.com>
> > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> > Co-developed-by: Evan Quan <evan.quan@amd.com>
> > Signed-off-by: Evan Quan <evan.quan@amd.com>
>
> From WiFi POV, this looks _almost_ fine to me.
>
> > +static void wbrf_get_ranges_from_chandef(struct cfg80211_chan_def
> *chandef,
> > + struct wbrf_ranges_in *ranges_in) {
> > + u64 start_freq1, end_freq1;
> > + u64 start_freq2, end_freq2;
> > + int bandwidth;
> > +
> > + bandwidth = nl80211_chan_width_to_mhz(chandef->width);
> > +
> > + get_chan_freq_boundary(chandef->center_freq1,
> > + bandwidth,
> > + &start_freq1,
> > + &end_freq1);
> > +
> > + ranges_in->band_list[0].start = start_freq1;
> > + ranges_in->band_list[0].end = end_freq1;
> > +
> > + if (chandef->width == NL80211_CHAN_WIDTH_80P80) {
> > + get_chan_freq_boundary(chandef->center_freq2,
> > + bandwidth,
> > + &start_freq2,
> > + &end_freq2);
> > +
> > + ranges_in->band_list[1].start = start_freq2;
> > + ranges_in->band_list[1].end = end_freq2;
> > + }
> > +}
>
> This has to setup ranges_in->num_of_ranges, no?
Yes, better to have that. I add this in V10.
> (Also no real good reason for num_of_ranges to be a u64, btw, since it can
> only go up to 11)
Mainly for data structure alignment. Since other members come with u64.
So, to make the data structure naturally aligned, 'num_of_ranges' also comes with u64.
Evan
>
> With that fixed, you can add
>
> Reviewed-by: Johannes Berg <johannes@sipsolutions.net>
>
> johannes
^ permalink raw reply [flat|nested] 25+ messages in thread
* [V9 5/9] drm/amd/pm: update driver_if and ppsmc headers for coming wbrf feature
2023-08-18 3:26 [V9 0/9] Enable Wifi RFI interference mitigation feature support Evan Quan
` (3 preceding siblings ...)
2023-08-18 3:26 ` [V9 4/9] wifi: mac80211: Add support for WBRF features Evan Quan
@ 2023-08-18 3:26 ` Evan Quan
2023-08-18 3:26 ` [V9 6/9] drm/amd/pm: setup the framework to support Wifi RFI mitigation feature Evan Quan
` (3 subsequent siblings)
8 siblings, 0 replies; 25+ messages in thread
From: Evan Quan @ 2023-08-18 3:26 UTC (permalink / raw)
To: gregkh, rafael, lenb, johannes, davem, edumazet, kuba, pabeni,
alexander.deucher, andrew, rdunlap, quic_jjohnson, horms
Cc: linux-doc, linux-kernel, linux-acpi, amd-gfx, linux-wireless,
netdev, Evan Quan, Mario Limonciello
Add those data structures to support Wifi RFI mitigation feature.
Signed-off-by: Evan Quan <evan.quan@amd.com>
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
---
.../pm/swsmu/inc/pmfw_if/smu13_driver_if_v13_0_0.h | 14 +++++++++++++-
.../pm/swsmu/inc/pmfw_if/smu13_driver_if_v13_0_7.h | 14 +++++++++++++-
.../amd/pm/swsmu/inc/pmfw_if/smu_v13_0_0_ppsmc.h | 3 ++-
.../amd/pm/swsmu/inc/pmfw_if/smu_v13_0_7_ppsmc.h | 3 ++-
4 files changed, 30 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu13_driver_if_v13_0_0.h b/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu13_driver_if_v13_0_0.h
index 9dd1ed5b8940..e481407b6584 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu13_driver_if_v13_0_0.h
+++ b/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu13_driver_if_v13_0_0.h
@@ -391,6 +391,17 @@ typedef struct {
EccInfo_t EccInfo[24];
} EccInfoTable_t;
+typedef struct {
+ uint16_t LowFreq;
+ uint16_t HighFreq;
+} WifiOneBand_t;
+
+typedef struct {
+ uint32_t WifiBandEntryNum;
+ WifiOneBand_t WifiBandEntry[11];
+ uint32_t MmHubPadding[8];
+} WifiBandEntryTable_t;
+
//D3HOT sequences
typedef enum {
BACO_SEQUENCE,
@@ -1615,7 +1626,8 @@ typedef struct {
#define TABLE_I2C_COMMANDS 9
#define TABLE_DRIVER_INFO 10
#define TABLE_ECCINFO 11
-#define TABLE_COUNT 12
+#define TABLE_WIFIBAND 12
+#define TABLE_COUNT 13
//IH Interupt ID
#define IH_INTERRUPT_ID_TO_DRIVER 0xFE
diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu13_driver_if_v13_0_7.h b/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu13_driver_if_v13_0_7.h
index 62b7c0daff68..1530ca002c6c 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu13_driver_if_v13_0_7.h
+++ b/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu13_driver_if_v13_0_7.h
@@ -392,6 +392,17 @@ typedef struct {
EccInfo_t EccInfo[24];
} EccInfoTable_t;
+typedef struct {
+ uint16_t LowFreq;
+ uint16_t HighFreq;
+} WifiOneBand_t;
+
+typedef struct {
+ uint32_t WifiBandEntryNum;
+ WifiOneBand_t WifiBandEntry[11];
+ uint32_t MmHubPadding[8];
+} WifiBandEntryTable_t;
+
//D3HOT sequences
typedef enum {
BACO_SEQUENCE,
@@ -1605,7 +1616,8 @@ typedef struct {
#define TABLE_I2C_COMMANDS 9
#define TABLE_DRIVER_INFO 10
#define TABLE_ECCINFO 11
-#define TABLE_COUNT 12
+#define TABLE_WIFIBAND 12
+#define TABLE_COUNT 13
//IH Interupt ID
#define IH_INTERRUPT_ID_TO_DRIVER 0xFE
diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu_v13_0_0_ppsmc.h b/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu_v13_0_0_ppsmc.h
index 10cff75b44d5..c98cc32d11bd 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu_v13_0_0_ppsmc.h
+++ b/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu_v13_0_0_ppsmc.h
@@ -138,7 +138,8 @@
#define PPSMC_MSG_SetBadMemoryPagesRetiredFlagsPerChannel 0x4A
#define PPSMC_MSG_SetPriorityDeltaGain 0x4B
#define PPSMC_MSG_AllowIHHostInterrupt 0x4C
-#define PPSMC_Message_Count 0x4D
+#define PPSMC_MSG_EnableUCLKShadow 0x51
+#define PPSMC_Message_Count 0x52
//Debug Dump Message
#define DEBUGSMC_MSG_TestMessage 0x1
diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu_v13_0_7_ppsmc.h b/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu_v13_0_7_ppsmc.h
index 6aaefca9b595..a6bf9cdd130e 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu_v13_0_7_ppsmc.h
+++ b/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu_v13_0_7_ppsmc.h
@@ -134,6 +134,7 @@
#define PPSMC_MSG_SetBadMemoryPagesRetiredFlagsPerChannel 0x4A
#define PPSMC_MSG_SetPriorityDeltaGain 0x4B
#define PPSMC_MSG_AllowIHHostInterrupt 0x4C
-#define PPSMC_Message_Count 0x4D
+#define PPSMC_MSG_EnableUCLKShadow 0x51
+#define PPSMC_Message_Count 0x52
#endif
--
2.34.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [V9 6/9] drm/amd/pm: setup the framework to support Wifi RFI mitigation feature
2023-08-18 3:26 [V9 0/9] Enable Wifi RFI interference mitigation feature support Evan Quan
` (4 preceding siblings ...)
2023-08-18 3:26 ` [V9 5/9] drm/amd/pm: update driver_if and ppsmc headers for coming wbrf feature Evan Quan
@ 2023-08-18 3:26 ` Evan Quan
2023-08-18 9:12 ` Lazar, Lijo
2023-08-18 3:26 ` [V9 7/9] drm/amd/pm: add flood detection for wbrf events Evan Quan
` (2 subsequent siblings)
8 siblings, 1 reply; 25+ messages in thread
From: Evan Quan @ 2023-08-18 3:26 UTC (permalink / raw)
To: gregkh, rafael, lenb, johannes, davem, edumazet, kuba, pabeni,
alexander.deucher, andrew, rdunlap, quic_jjohnson, horms
Cc: linux-doc, linux-kernel, linux-acpi, amd-gfx, linux-wireless,
netdev, Evan Quan, Mario Limonciello
With WBRF feature supported, as a driver responding to the frequencies,
amdgpu driver is able to do shadow pstate switching to mitigate possible
interference(between its (G-)DDR memory clocks and local radio module
frequency bands used by Wifi 6/6e/7).
Signed-off-by: Evan Quan <evan.quan@amd.com>
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
--
v1->v2:
- update the prompt for feature support(Lijo)
v8->v9:
- update parameter document for smu_wbrf_event_handler(Simon)
---
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 +
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 17 ++
drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 194 ++++++++++++++++++
drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h | 23 +++
drivers/gpu/drm/amd/pm/swsmu/smu_internal.h | 3 +
5 files changed, 239 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index a3b86b86dc47..2bfc9111ab00 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -247,6 +247,8 @@ extern int amdgpu_sg_display;
extern int amdgpu_user_partt_mode;
+extern int amdgpu_wbrf;
+
#define AMDGPU_VM_MAX_NUM_CTX 4096
#define AMDGPU_SG_THRESHOLD (256*1024*1024)
#define AMDGPU_WAIT_IDLE_TIMEOUT_IN_MS 3000
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 0593ef8fe0a6..1c574bd3b60d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -195,6 +195,7 @@ int amdgpu_use_xgmi_p2p = 1;
int amdgpu_vcnfw_log;
int amdgpu_sg_display = -1; /* auto */
int amdgpu_user_partt_mode = AMDGPU_AUTO_COMPUTE_PARTITION_MODE;
+int amdgpu_wbrf = -1;
static void amdgpu_drv_delayed_reset_work_handler(struct work_struct *work);
@@ -981,6 +982,22 @@ module_param_named(user_partt_mode, amdgpu_user_partt_mode, uint, 0444);
module_param(enforce_isolation, bool, 0444);
MODULE_PARM_DESC(enforce_isolation, "enforce process isolation between graphics and compute . enforce_isolation = on");
+/**
+ * DOC: wbrf (int)
+ * Enable Wifi RFI interference mitigation feature.
+ * Due to electrical and mechanical constraints 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 the possible RFI interference,
+ * with this feature enabled, PMFW will use either “shadowed P-State” or “P-State” based
+ * on active list of frequencies in-use (to be avoided) as part of initial setting or
+ * P-state transition. However, there may be potential performance impact with this
+ * feature enabled.
+ * (0 = disabled, 1 = enabled, -1 = auto (default setting, will be enabled if supported))
+ */
+MODULE_PARM_DESC(wbrf,
+ "Enable Wifi RFI interference mitigation (0 = disabled, 1 = enabled, -1 = auto(default)");
+module_param_named(wbrf, amdgpu_wbrf, int, 0444);
+
/* These devices are not supported by amdgpu.
* They are supported by the mach64, r128, radeon drivers
*/
diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
index ce41a8309582..704442ce1da3 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
@@ -1228,6 +1228,173 @@ static int smu_get_thermal_temperature_range(struct smu_context *smu)
return ret;
}
+/**
+ * smu_wbrf_handle_exclusion_ranges - consume the wbrf exclusion ranges
+ *
+ * @smu: smu_context pointer
+ *
+ * Retrieve the wbrf exclusion ranges and send them to PMFW for proper handling.
+ * Returns 0 on success, error on failure.
+ */
+static int smu_wbrf_handle_exclusion_ranges(struct smu_context *smu)
+{
+ struct wbrf_ranges_out wbrf_exclusion = {0};
+ struct exclusion_range *wifi_bands = wbrf_exclusion.band_list;
+ struct amdgpu_device *adev = smu->adev;
+ uint64_t start, end;
+ int ret, i, j;
+
+ ret = wbrf_retrieve_exclusions(adev->dev, &wbrf_exclusion);
+ if (ret) {
+ dev_err(adev->dev, "Failed to retrieve exclusion ranges!\n");
+ return ret;
+ }
+
+ /*
+ * The exclusion ranges array we got might be filled with holes and duplicate
+ * entries. For example:
+ * {(2400, 2500), (0, 0), (6882, 6962), (2400, 2500), (0, 0), (6117, 6189), (0, 0)...}
+ * We need to do some sortups to eliminate those holes and duplicate entries.
+ * Expected output: {(2400, 2500), (6117, 6189), (6882, 6962), (0, 0)...}
+ */
+ for (i = 0; i < MAX_NUM_OF_WBRF_RANGES; i++) {
+ start = wifi_bands[i].start;
+ end = wifi_bands[i].end;
+
+ /* get the last valid entry to fill the intermediate hole */
+ if (!start && !end) {
+ for (j = MAX_NUM_OF_WBRF_RANGES - 1; j > i; j--)
+ if (wifi_bands[j].start &&
+ wifi_bands[j].end)
+ break;
+
+ if (j > i) {
+ wifi_bands[i].start = wifi_bands[j].start;
+ wifi_bands[i].end = wifi_bands[j].end;
+ wifi_bands[j].start = 0;
+ wifi_bands[j].end = 0;
+ }
+
+ continue;
+ }
+
+ /* eliminate duplicate entries */
+ for (j = i + 1; j < MAX_NUM_OF_WBRF_RANGES; j++) {
+ if ((wifi_bands[j].start == start) &&
+ (wifi_bands[j].end == end)) {
+ wifi_bands[j].start = 0;
+ wifi_bands[j].end = 0;
+ continue;
+ }
+ }
+ }
+
+ /* Send the sorted wifi_bands to PMFW */
+ ret = smu_set_wbrf_exclusion_ranges(smu, wifi_bands);
+ /* Give it another chance */
+ if (unlikely(ret == -EBUSY)) {
+ mdelay(5);
+ ret = smu_set_wbrf_exclusion_ranges(smu, wifi_bands);
+ }
+
+ return ret;
+}
+
+/**
+ * smu_wbrf_event_handler - handle notify events
+ *
+ * @nb: notifier block
+ * @action: event type
+ * @_arg: event data
+ *
+ * Calls relevant amdgpu function in response to wbrf event
+ * notification from kernel.
+ */
+static int smu_wbrf_event_handler(struct notifier_block *nb,
+ unsigned long action, void *_arg)
+{
+ struct smu_context *smu = container_of(nb, struct smu_context,
+ wbrf_notifier);
+
+ switch (action) {
+ case WBRF_CHANGED:
+ smu_wbrf_handle_exclusion_ranges(smu);
+ break;
+ default:
+ return NOTIFY_DONE;
+ };
+
+ return NOTIFY_OK;
+}
+
+/**
+ * smu_wbrf_support_check - check wbrf support
+ *
+ * @smu: smu_context pointer
+ *
+ * Verifies the ACPI interface whether wbrf is supported.
+ */
+static void smu_wbrf_support_check(struct smu_context *smu)
+{
+ struct amdgpu_device *adev = smu->adev;
+
+ smu->wbrf_supported = smu_is_asic_wbrf_supported(smu) &&
+ !!amdgpu_wbrf &&
+ wbrf_supported_consumer(adev->dev);
+
+ if (smu->wbrf_supported)
+ dev_info(adev->dev, "RF interference mitigation is supported\n");
+}
+
+/**
+ * smu_wbrf_init - init driver wbrf support
+ *
+ * @smu: smu_context pointer
+ *
+ * Verifies the AMD ACPI interfaces and registers with the wbrf
+ * notifier chain if wbrf feature is supported.
+ * Returns 0 on success, error on failure.
+ */
+static int smu_wbrf_init(struct smu_context *smu)
+{
+ struct amdgpu_device *adev = smu->adev;
+ int ret;
+
+ if (!smu->wbrf_supported)
+ return 0;
+
+ smu->wbrf_notifier.notifier_call = smu_wbrf_event_handler;
+ ret = wbrf_register_notifier(&smu->wbrf_notifier);
+ if (ret)
+ return ret;
+
+ /*
+ * Some wifiband exclusion ranges may be already there
+ * before our driver loaded. To make sure our driver
+ * is awared of those exclusion ranges.
+ */
+ ret = smu_wbrf_handle_exclusion_ranges(smu);
+ if (ret)
+ dev_err(adev->dev, "Failed to handle wbrf exclusion ranges\n");
+
+ return ret;
+}
+
+/**
+ * smu_wbrf_fini - tear down driver wbrf support
+ *
+ * @smu: smu_context pointer
+ *
+ * Unregisters with the wbrf notifier chain.
+ */
+static void smu_wbrf_fini(struct smu_context *smu)
+{
+ if (!smu->wbrf_supported)
+ return;
+
+ wbrf_unregister_notifier(&smu->wbrf_notifier);
+}
+
static int smu_smc_hw_setup(struct smu_context *smu)
{
struct smu_feature *feature = &smu->smu_feature;
@@ -1320,6 +1487,15 @@ static int smu_smc_hw_setup(struct smu_context *smu)
if (ret)
return ret;
+ /* Enable UclkShadow on wbrf supported */
+ if (smu->wbrf_supported) {
+ ret = smu_enable_uclk_shadow(smu, true);
+ if (ret) {
+ dev_err(adev->dev, "Failed to enable UclkShadow feature to support wbrf!\n");
+ return ret;
+ }
+ }
+
/*
* With SCPM enabled, these actions(and relevant messages) are
* not needed and permitted.
@@ -1416,6 +1592,15 @@ static int smu_smc_hw_setup(struct smu_context *smu)
*/
ret = smu_set_min_dcef_deep_sleep(smu,
smu->smu_table.boot_values.dcefclk / 100);
+ if (ret) {
+ dev_err(adev->dev, "Error setting min deepsleep dcefclk\n");
+ return ret;
+ }
+
+ /* Init wbrf support. Properly setup the notifier */
+ ret = smu_wbrf_init(smu);
+ if (ret)
+ dev_err(adev->dev, "Error during wbrf init call\n");
return ret;
}
@@ -1471,6 +1656,13 @@ static int smu_hw_init(void *handle)
return ret;
}
+ /*
+ * Check whether wbrf is supported. This needs to be done
+ * before SMU setup starts since part of SMU configuration
+ * relies on this.
+ */
+ smu_wbrf_support_check(smu);
+
if (smu->is_apu) {
ret = smu_set_gfx_imu_enable(smu);
if (ret)
@@ -1623,6 +1815,8 @@ static int smu_smc_hw_cleanup(struct smu_context *smu)
struct amdgpu_device *adev = smu->adev;
int ret = 0;
+ smu_wbrf_fini(smu);
+
cancel_work_sync(&smu->throttling_logging_work);
cancel_work_sync(&smu->interrupt_work);
diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
index 6e2069dcb6b9..244297979f92 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
+++ b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
@@ -22,6 +22,8 @@
#ifndef __AMDGPU_SMU_H__
#define __AMDGPU_SMU_H__
+#include <linux/wbrf.h>
+
#include "amdgpu.h"
#include "kgd_pp_interface.h"
#include "dm_pp_interface.h"
@@ -575,6 +577,10 @@ struct smu_context
u32 debug_resp_reg;
struct delayed_work swctf_delayed_work;
+
+ /* data structures for wbrf feature support */
+ bool wbrf_supported;
+ struct notifier_block wbrf_notifier;
};
struct i2c_adapter;
@@ -1356,6 +1362,23 @@ struct pptable_funcs {
* @init_pptable_microcode: Prepare the pptable microcode to upload via PSP
*/
int (*init_pptable_microcode)(struct smu_context *smu);
+
+ /**
+ * @is_asic_wbrf_supported: check whether PMFW supports the wbrf feature
+ */
+ bool (*is_asic_wbrf_supported)(struct smu_context *smu);
+
+ /**
+ * @enable_uclk_shadow: Enable the uclk shadow feature on wbrf supported
+ */
+ int (*enable_uclk_shadow)(struct smu_context *smu,
+ bool enablement);
+
+ /**
+ * @set_wbrf_exclusion_ranges: notify SMU the wifi bands occupied
+ */
+ int (*set_wbrf_exclusion_ranges)(struct smu_context *smu,
+ struct exclusion_range *exclusion_ranges);
};
typedef enum {
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu_internal.h b/drivers/gpu/drm/amd/pm/swsmu/smu_internal.h
index ceb13c838067..67d7495ab49e 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu_internal.h
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu_internal.h
@@ -97,6 +97,9 @@
#define smu_get_default_config_table_settings(smu, config_table) smu_ppt_funcs(get_default_config_table_settings, -EOPNOTSUPP, smu, config_table)
#define smu_set_config_table(smu, config_table) smu_ppt_funcs(set_config_table, -EOPNOTSUPP, smu, config_table)
#define smu_init_pptable_microcode(smu) smu_ppt_funcs(init_pptable_microcode, 0, smu)
+#define smu_is_asic_wbrf_supported(smu) smu_ppt_funcs(is_asic_wbrf_supported, false, smu)
+#define smu_enable_uclk_shadow(smu, enablement) smu_ppt_funcs(enable_uclk_shadow, 0, smu, enablement)
+#define smu_set_wbrf_exclusion_ranges(smu, exclusion_ranges) smu_ppt_funcs(set_wbrf_exclusion_ranges, -EOPNOTSUPP, smu, exclusion_ranges)
#endif
#endif
--
2.34.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [V9 6/9] drm/amd/pm: setup the framework to support Wifi RFI mitigation feature
2023-08-18 3:26 ` [V9 6/9] drm/amd/pm: setup the framework to support Wifi RFI mitigation feature Evan Quan
@ 2023-08-18 9:12 ` Lazar, Lijo
2023-08-25 8:48 ` Quan, Evan
0 siblings, 1 reply; 25+ messages in thread
From: Lazar, Lijo @ 2023-08-18 9:12 UTC (permalink / raw)
To: Evan Quan, gregkh, rafael, lenb, johannes, davem, edumazet, kuba,
pabeni, alexander.deucher, andrew, rdunlap, quic_jjohnson, horms
Cc: linux-doc, netdev, linux-wireless, linux-kernel, amd-gfx,
linux-acpi, Mario Limonciello
On 8/18/2023 8:56 AM, Evan Quan wrote:
> With WBRF feature supported, as a driver responding to the frequencies,
> amdgpu driver is able to do shadow pstate switching to mitigate possible
> interference(between its (G-)DDR memory clocks and local radio module
> frequency bands used by Wifi 6/6e/7).
>
> Signed-off-by: Evan Quan <evan.quan@amd.com>
> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
> --
> v1->v2:
> - update the prompt for feature support(Lijo)
> v8->v9:
> - update parameter document for smu_wbrf_event_handler(Simon)
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 +
> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 17 ++
> drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 194 ++++++++++++++++++
> drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h | 23 +++
> drivers/gpu/drm/amd/pm/swsmu/smu_internal.h | 3 +
> 5 files changed, 239 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index a3b86b86dc47..2bfc9111ab00 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -247,6 +247,8 @@ extern int amdgpu_sg_display;
>
> extern int amdgpu_user_partt_mode;
>
> +extern int amdgpu_wbrf;
> +
> #define AMDGPU_VM_MAX_NUM_CTX 4096
> #define AMDGPU_SG_THRESHOLD (256*1024*1024)
> #define AMDGPU_WAIT_IDLE_TIMEOUT_IN_MS 3000
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 0593ef8fe0a6..1c574bd3b60d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -195,6 +195,7 @@ int amdgpu_use_xgmi_p2p = 1;
> int amdgpu_vcnfw_log;
> int amdgpu_sg_display = -1; /* auto */
> int amdgpu_user_partt_mode = AMDGPU_AUTO_COMPUTE_PARTITION_MODE;
> +int amdgpu_wbrf = -1;
>
> static void amdgpu_drv_delayed_reset_work_handler(struct work_struct *work);
>
> @@ -981,6 +982,22 @@ module_param_named(user_partt_mode, amdgpu_user_partt_mode, uint, 0444);
> module_param(enforce_isolation, bool, 0444);
> MODULE_PARM_DESC(enforce_isolation, "enforce process isolation between graphics and compute . enforce_isolation = on");
>
> +/**
> + * DOC: wbrf (int)
> + * Enable Wifi RFI interference mitigation feature.
> + * Due to electrical and mechanical constraints 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 the possible RFI interference,
> + * with this feature enabled, PMFW will use either “shadowed P-State” or “P-State” based
> + * on active list of frequencies in-use (to be avoided) as part of initial setting or
> + * P-state transition. However, there may be potential performance impact with this
> + * feature enabled.
> + * (0 = disabled, 1 = enabled, -1 = auto (default setting, will be enabled if supported))
> + */
> +MODULE_PARM_DESC(wbrf,
> + "Enable Wifi RFI interference mitigation (0 = disabled, 1 = enabled, -1 = auto(default)");
> +module_param_named(wbrf, amdgpu_wbrf, int, 0444);
> +
> /* These devices are not supported by amdgpu.
> * They are supported by the mach64, r128, radeon drivers
> */
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> index ce41a8309582..704442ce1da3 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> @@ -1228,6 +1228,173 @@ static int smu_get_thermal_temperature_range(struct smu_context *smu)
> return ret;
> }
>
> +/**
> + * smu_wbrf_handle_exclusion_ranges - consume the wbrf exclusion ranges
> + *
> + * @smu: smu_context pointer
> + *
> + * Retrieve the wbrf exclusion ranges and send them to PMFW for proper handling.
> + * Returns 0 on success, error on failure.
> + */
> +static int smu_wbrf_handle_exclusion_ranges(struct smu_context *smu)
> +{
> + struct wbrf_ranges_out wbrf_exclusion = {0};
> + struct exclusion_range *wifi_bands = wbrf_exclusion.band_list;
> + struct amdgpu_device *adev = smu->adev;
> + uint64_t start, end;
> + int ret, i, j;
> +
> + ret = wbrf_retrieve_exclusions(adev->dev, &wbrf_exclusion);
> + if (ret) {
> + dev_err(adev->dev, "Failed to retrieve exclusion ranges!\n");
> + return ret;
> + }
> +
> + /*
> + * The exclusion ranges array we got might be filled with holes and duplicate
> + * entries. For example:
> + * {(2400, 2500), (0, 0), (6882, 6962), (2400, 2500), (0, 0), (6117, 6189), (0, 0)...}
> + * We need to do some sortups to eliminate those holes and duplicate entries.
> + * Expected output: {(2400, 2500), (6117, 6189), (6882, 6962), (0, 0)...}
> + */
> + for (i = 0; i < MAX_NUM_OF_WBRF_RANGES; i++) {
> + start = wifi_bands[i].start;
> + end = wifi_bands[i].end;
> +
> + /* get the last valid entry to fill the intermediate hole */
> + if (!start && !end) {
> + for (j = MAX_NUM_OF_WBRF_RANGES - 1; j > i; j--)
> + if (wifi_bands[j].start &&
> + wifi_bands[j].end)
> + break;
> +
> + if (j > i) {
> + wifi_bands[i].start = wifi_bands[j].start;
> + wifi_bands[i].end = wifi_bands[j].end;
Since the last value is now 0, one way to optimize is to keep max as j
-1 for the next iteration.
> + wifi_bands[j].start = 0;
> + wifi_bands[j].end = 0;
> + }
> +
> + continue;
If continued at this point, it won't eliminate all duplicates.
Example:
(2000,2100)(0,0)(1100,1200)(0, 0)(2500,2600)(3000,3200)(1100,1200)
Once it places (1100,1200) at index 1, it will continue the loop and
then it skips duplicate entry at index 2.
Once replaced, you may assign the new start/end
start = wifi_bands[i].start = wifi_bands[j].start;
end= wifi_bands[i].end = wifi_bands[j].end;
and then proceed to the duplicate entries scan loop below.
> + }
> +
> + /* eliminate duplicate entries */
> + for (j = i + 1; j < MAX_NUM_OF_WBRF_RANGES; j++) {
> + if ((wifi_bands[j].start == start) &&
> + (wifi_bands[j].end == end)) {
> + wifi_bands[j].start = 0;
> + wifi_bands[j].end = 0;
> + continue;
I think this'continue' is not required.
Thanks,
Lijo
> + }
> + }
> + }
> +
> + /* Send the sorted wifi_bands to PMFW */
> + ret = smu_set_wbrf_exclusion_ranges(smu, wifi_bands);
> + /* Give it another chance */
> + if (unlikely(ret == -EBUSY)) {
> + mdelay(5);
> + ret = smu_set_wbrf_exclusion_ranges(smu, wifi_bands);
> + }
> +
> + return ret;
> +}
> +
> +/**
> + * smu_wbrf_event_handler - handle notify events
> + *
> + * @nb: notifier block
> + * @action: event type
> + * @_arg: event data
> + *
> + * Calls relevant amdgpu function in response to wbrf event
> + * notification from kernel.
> + */
> +static int smu_wbrf_event_handler(struct notifier_block *nb,
> + unsigned long action, void *_arg)
> +{
> + struct smu_context *smu = container_of(nb, struct smu_context,
> + wbrf_notifier);
> +
> + switch (action) {
> + case WBRF_CHANGED:
> + smu_wbrf_handle_exclusion_ranges(smu);
> + break;
> + default:
> + return NOTIFY_DONE;
> + };
> +
> + return NOTIFY_OK;
> +}
> +
> +/**
> + * smu_wbrf_support_check - check wbrf support
> + *
> + * @smu: smu_context pointer
> + *
> + * Verifies the ACPI interface whether wbrf is supported.
> + */
> +static void smu_wbrf_support_check(struct smu_context *smu)
> +{
> + struct amdgpu_device *adev = smu->adev;
> +
> + smu->wbrf_supported = smu_is_asic_wbrf_supported(smu) &&
> + !!amdgpu_wbrf &&
> + wbrf_supported_consumer(adev->dev);
> +
> + if (smu->wbrf_supported)
> + dev_info(adev->dev, "RF interference mitigation is supported\n");
> +}
> +
> +/**
> + * smu_wbrf_init - init driver wbrf support
> + *
> + * @smu: smu_context pointer
> + *
> + * Verifies the AMD ACPI interfaces and registers with the wbrf
> + * notifier chain if wbrf feature is supported.
> + * Returns 0 on success, error on failure.
> + */
> +static int smu_wbrf_init(struct smu_context *smu)
> +{
> + struct amdgpu_device *adev = smu->adev;
> + int ret;
> +
> + if (!smu->wbrf_supported)
> + return 0;
> +
> + smu->wbrf_notifier.notifier_call = smu_wbrf_event_handler;
> + ret = wbrf_register_notifier(&smu->wbrf_notifier);
> + if (ret)
> + return ret;
> +
> + /*
> + * Some wifiband exclusion ranges may be already there
> + * before our driver loaded. To make sure our driver
> + * is awared of those exclusion ranges.
> + */
> + ret = smu_wbrf_handle_exclusion_ranges(smu);
> + if (ret)
> + dev_err(adev->dev, "Failed to handle wbrf exclusion ranges\n");
> +
> + return ret;
> +}
> +
> +/**
> + * smu_wbrf_fini - tear down driver wbrf support
> + *
> + * @smu: smu_context pointer
> + *
> + * Unregisters with the wbrf notifier chain.
> + */
> +static void smu_wbrf_fini(struct smu_context *smu)
> +{
> + if (!smu->wbrf_supported)
> + return;
> +
> + wbrf_unregister_notifier(&smu->wbrf_notifier);
> +}
> +
> static int smu_smc_hw_setup(struct smu_context *smu)
> {
> struct smu_feature *feature = &smu->smu_feature;
> @@ -1320,6 +1487,15 @@ static int smu_smc_hw_setup(struct smu_context *smu)
> if (ret)
> return ret;
>
> + /* Enable UclkShadow on wbrf supported */
> + if (smu->wbrf_supported) {
> + ret = smu_enable_uclk_shadow(smu, true);
> + if (ret) {
> + dev_err(adev->dev, "Failed to enable UclkShadow feature to support wbrf!\n");
> + return ret;
> + }
> + }
> +
> /*
> * With SCPM enabled, these actions(and relevant messages) are
> * not needed and permitted.
> @@ -1416,6 +1592,15 @@ static int smu_smc_hw_setup(struct smu_context *smu)
> */
> ret = smu_set_min_dcef_deep_sleep(smu,
> smu->smu_table.boot_values.dcefclk / 100);
> + if (ret) {
> + dev_err(adev->dev, "Error setting min deepsleep dcefclk\n");
> + return ret;
> + }
> +
> + /* Init wbrf support. Properly setup the notifier */
> + ret = smu_wbrf_init(smu);
> + if (ret)
> + dev_err(adev->dev, "Error during wbrf init call\n");
>
> return ret;
> }
> @@ -1471,6 +1656,13 @@ static int smu_hw_init(void *handle)
> return ret;
> }
>
> + /*
> + * Check whether wbrf is supported. This needs to be done
> + * before SMU setup starts since part of SMU configuration
> + * relies on this.
> + */
> + smu_wbrf_support_check(smu);
> +
> if (smu->is_apu) {
> ret = smu_set_gfx_imu_enable(smu);
> if (ret)
> @@ -1623,6 +1815,8 @@ static int smu_smc_hw_cleanup(struct smu_context *smu)
> struct amdgpu_device *adev = smu->adev;
> int ret = 0;
>
> + smu_wbrf_fini(smu);
> +
> cancel_work_sync(&smu->throttling_logging_work);
> cancel_work_sync(&smu->interrupt_work);
>
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> index 6e2069dcb6b9..244297979f92 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> +++ b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> @@ -22,6 +22,8 @@
> #ifndef __AMDGPU_SMU_H__
> #define __AMDGPU_SMU_H__
>
> +#include <linux/wbrf.h>
> +
> #include "amdgpu.h"
> #include "kgd_pp_interface.h"
> #include "dm_pp_interface.h"
> @@ -575,6 +577,10 @@ struct smu_context
> u32 debug_resp_reg;
>
> struct delayed_work swctf_delayed_work;
> +
> + /* data structures for wbrf feature support */
> + bool wbrf_supported;
> + struct notifier_block wbrf_notifier;
> };
>
> struct i2c_adapter;
> @@ -1356,6 +1362,23 @@ struct pptable_funcs {
> * @init_pptable_microcode: Prepare the pptable microcode to upload via PSP
> */
> int (*init_pptable_microcode)(struct smu_context *smu);
> +
> + /**
> + * @is_asic_wbrf_supported: check whether PMFW supports the wbrf feature
> + */
> + bool (*is_asic_wbrf_supported)(struct smu_context *smu);
> +
> + /**
> + * @enable_uclk_shadow: Enable the uclk shadow feature on wbrf supported
> + */
> + int (*enable_uclk_shadow)(struct smu_context *smu,
> + bool enablement);
> +
> + /**
> + * @set_wbrf_exclusion_ranges: notify SMU the wifi bands occupied
> + */
> + int (*set_wbrf_exclusion_ranges)(struct smu_context *smu,
> + struct exclusion_range *exclusion_ranges);
> };
>
> typedef enum {
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu_internal.h b/drivers/gpu/drm/amd/pm/swsmu/smu_internal.h
> index ceb13c838067..67d7495ab49e 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu_internal.h
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu_internal.h
> @@ -97,6 +97,9 @@
> #define smu_get_default_config_table_settings(smu, config_table) smu_ppt_funcs(get_default_config_table_settings, -EOPNOTSUPP, smu, config_table)
> #define smu_set_config_table(smu, config_table) smu_ppt_funcs(set_config_table, -EOPNOTSUPP, smu, config_table)
> #define smu_init_pptable_microcode(smu) smu_ppt_funcs(init_pptable_microcode, 0, smu)
> +#define smu_is_asic_wbrf_supported(smu) smu_ppt_funcs(is_asic_wbrf_supported, false, smu)
> +#define smu_enable_uclk_shadow(smu, enablement) smu_ppt_funcs(enable_uclk_shadow, 0, smu, enablement)
> +#define smu_set_wbrf_exclusion_ranges(smu, exclusion_ranges) smu_ppt_funcs(set_wbrf_exclusion_ranges, -EOPNOTSUPP, smu, exclusion_ranges)
>
> #endif
> #endif
^ permalink raw reply [flat|nested] 25+ messages in thread* RE: [V9 6/9] drm/amd/pm: setup the framework to support Wifi RFI mitigation feature
2023-08-18 9:12 ` Lazar, Lijo
@ 2023-08-25 8:48 ` Quan, Evan
0 siblings, 0 replies; 25+ messages in thread
From: Quan, Evan @ 2023-08-25 8:48 UTC (permalink / raw)
To: Lazar, Lijo, gregkh@linuxfoundation.org, rafael@kernel.org,
lenb@kernel.org, johannes@sipsolutions.net, davem@davemloft.net,
edumazet@google.com, kuba@kernel.org, pabeni@redhat.com,
Deucher, Alexander, andrew@lunn.ch, rdunlap@infradead.org,
quic_jjohnson@quicinc.com, horms@kernel.org
Cc: linux-doc@vger.kernel.org, netdev@vger.kernel.org,
linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
amd-gfx@lists.freedesktop.org, linux-acpi@vger.kernel.org,
Limonciello, Mario
[AMD Official Use Only - General]
> -----Original Message-----
> From: Lazar, Lijo <Lijo.Lazar@amd.com>
> Sent: Friday, August 18, 2023 5:12 PM
> To: Quan, Evan <Evan.Quan@amd.com>; gregkh@linuxfoundation.org;
> rafael@kernel.org; lenb@kernel.org; johannes@sipsolutions.net;
> davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> pabeni@redhat.com; Deucher, Alexander <Alexander.Deucher@amd.com>;
> andrew@lunn.ch; rdunlap@infradead.org; quic_jjohnson@quicinc.com;
> horms@kernel.org
> Cc: linux-doc@vger.kernel.org; netdev@vger.kernel.org; linux-
> wireless@vger.kernel.org; linux-kernel@vger.kernel.org; amd-
> gfx@lists.freedesktop.org; linux-acpi@vger.kernel.org; Limonciello, Mario
> <Mario.Limonciello@amd.com>
> Subject: Re: [V9 6/9] drm/amd/pm: setup the framework to support Wifi RFI
> mitigation feature
>
>
>
> On 8/18/2023 8:56 AM, Evan Quan wrote:
> > With WBRF feature supported, as a driver responding to the
> > frequencies, amdgpu driver is able to do shadow pstate switching to
> > mitigate possible interference(between its (G-)DDR memory clocks and
> > local radio module frequency bands used by Wifi 6/6e/7).
> >
> > Signed-off-by: Evan Quan <evan.quan@amd.com>
> > Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
> > --
> > v1->v2:
> > - update the prompt for feature support(Lijo)
> > v8->v9:
> > - update parameter document for smu_wbrf_event_handler(Simon)
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 +
> > drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 17 ++
> > drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 194
> ++++++++++++++++++
> > drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h | 23 +++
> > drivers/gpu/drm/amd/pm/swsmu/smu_internal.h | 3 +
> > 5 files changed, 239 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > index a3b86b86dc47..2bfc9111ab00 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > @@ -247,6 +247,8 @@ extern int amdgpu_sg_display;
> >
> > extern int amdgpu_user_partt_mode;
> >
> > +extern int amdgpu_wbrf;
> > +
> > #define AMDGPU_VM_MAX_NUM_CTX 4096
> > #define AMDGPU_SG_THRESHOLD (256*1024*1024)
> > #define AMDGPU_WAIT_IDLE_TIMEOUT_IN_MS 3000
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > index 0593ef8fe0a6..1c574bd3b60d 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > @@ -195,6 +195,7 @@ int amdgpu_use_xgmi_p2p = 1;
> > int amdgpu_vcnfw_log;
> > int amdgpu_sg_display = -1; /* auto */
> > int amdgpu_user_partt_mode =
> AMDGPU_AUTO_COMPUTE_PARTITION_MODE;
> > +int amdgpu_wbrf = -1;
> >
> > static void amdgpu_drv_delayed_reset_work_handler(struct work_struct
> > *work);
> >
> > @@ -981,6 +982,22 @@ module_param_named(user_partt_mode,
> amdgpu_user_partt_mode, uint, 0444);
> > module_param(enforce_isolation, bool, 0444);
> > MODULE_PARM_DESC(enforce_isolation, "enforce process isolation
> > between graphics and compute . enforce_isolation = on");
> >
> > +/**
> > + * DOC: wbrf (int)
> > + * Enable Wifi RFI interference mitigation feature.
> > + * Due to electrical and mechanical constraints 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 the
> > +possible RFI interference,
> > + * with this feature enabled, PMFW will use either “shadowed P-State”
> > +or “P-State” based
> > + * on active list of frequencies in-use (to be avoided) as part of
> > +initial setting or
> > + * P-state transition. However, there may be potential performance
> > +impact with this
> > + * feature enabled.
> > + * (0 = disabled, 1 = enabled, -1 = auto (default setting, will be
> > +enabled if supported)) */ MODULE_PARM_DESC(wbrf,
> > + "Enable Wifi RFI interference mitigation (0 = disabled, 1 = enabled,
> > +-1 = auto(default)"); module_param_named(wbrf, amdgpu_wbrf, int,
> > +0444);
> > +
> > /* These devices are not supported by amdgpu.
> > * They are supported by the mach64, r128, radeon drivers
> > */
> > diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> > b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> > index ce41a8309582..704442ce1da3 100644
> > --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> > +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> > @@ -1228,6 +1228,173 @@ static int
> smu_get_thermal_temperature_range(struct smu_context *smu)
> > return ret;
> > }
> >
> > +/**
> > + * smu_wbrf_handle_exclusion_ranges - consume the wbrf exclusion
> > +ranges
> > + *
> > + * @smu: smu_context pointer
> > + *
> > + * Retrieve the wbrf exclusion ranges and send them to PMFW for proper
> handling.
> > + * Returns 0 on success, error on failure.
> > + */
> > +static int smu_wbrf_handle_exclusion_ranges(struct smu_context *smu)
> > +{
> > + struct wbrf_ranges_out wbrf_exclusion = {0};
> > + struct exclusion_range *wifi_bands = wbrf_exclusion.band_list;
> > + struct amdgpu_device *adev = smu->adev;
> > + uint64_t start, end;
> > + int ret, i, j;
> > +
> > + ret = wbrf_retrieve_exclusions(adev->dev, &wbrf_exclusion);
> > + if (ret) {
> > + dev_err(adev->dev, "Failed to retrieve exclusion ranges!\n");
> > + return ret;
> > + }
> > +
> > + /*
> > + * The exclusion ranges array we got might be filled with holes and
> duplicate
> > + * entries. For example:
> > + * {(2400, 2500), (0, 0), (6882, 6962), (2400, 2500), (0, 0), (6117,
> 6189), (0, 0)...}
> > + * We need to do some sortups to eliminate those holes and duplicate
> entries.
> > + * Expected output: {(2400, 2500), (6117, 6189), (6882, 6962), (0,
> 0)...}
> > + */
> > + for (i = 0; i < MAX_NUM_OF_WBRF_RANGES; i++) {
> > + start = wifi_bands[i].start;
> > + end = wifi_bands[i].end;
> > +
> > + /* get the last valid entry to fill the intermediate hole */
> > + if (!start && !end) {
> > + for (j = MAX_NUM_OF_WBRF_RANGES - 1; j > i; j--)
> > + if (wifi_bands[j].start &&
> > + wifi_bands[j].end)
> > + break;
> > +
> > + if (j > i) {
> > + wifi_bands[i].start = wifi_bands[j].start;
> > + wifi_bands[i].end = wifi_bands[j].end;
>
> Since the last value is now 0, one way to optimize is to keep max as j
> -1 for the next iteration.
>
> > + wifi_bands[j].start = 0;
> > + wifi_bands[j].end = 0;
> > + }
> > +
> > + continue;
>
> If continued at this point, it won't eliminate all duplicates.
>
> Example:
> (2000,2100)(0,0)(1100,1200)(0, 0)(2500,2600)(3000,3200)(1100,1200)
>
> Once it places (1100,1200) at index 1, it will continue the loop and then it
> skips duplicate entry at index 2.
Thanks for pointing this out. Fixed in V10.
Evan
>
> Once replaced, you may assign the new start/end
> start = wifi_bands[i].start = wifi_bands[j].start;
> end= wifi_bands[i].end = wifi_bands[j].end;
>
> and then proceed to the duplicate entries scan loop below.
>
> > + }
> > +
> > + /* eliminate duplicate entries */
> > + for (j = i + 1; j < MAX_NUM_OF_WBRF_RANGES; j++) {
> > + if ((wifi_bands[j].start == start) &&
> > + (wifi_bands[j].end == end)) {
> > + wifi_bands[j].start = 0;
> > + wifi_bands[j].end = 0;
> > + continue;
>
> I think this'continue' is not required.
>
> Thanks,
> Lijo
>
> > + }
> > + }
> > + }
> > +
> > + /* Send the sorted wifi_bands to PMFW */
> > + ret = smu_set_wbrf_exclusion_ranges(smu, wifi_bands);
> > + /* Give it another chance */
> > + if (unlikely(ret == -EBUSY)) {
> > + mdelay(5);
> > + ret = smu_set_wbrf_exclusion_ranges(smu, wifi_bands);
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +/**
> > + * smu_wbrf_event_handler - handle notify events
> > + *
> > + * @nb: notifier block
> > + * @action: event type
> > + * @_arg: event data
> > + *
> > + * Calls relevant amdgpu function in response to wbrf event
> > + * notification from kernel.
> > + */
> > +static int smu_wbrf_event_handler(struct notifier_block *nb,
> > + unsigned long action, void *_arg) {
> > + struct smu_context *smu = container_of(nb, struct smu_context,
> > + wbrf_notifier);
> > +
> > + switch (action) {
> > + case WBRF_CHANGED:
> > + smu_wbrf_handle_exclusion_ranges(smu);
> > + break;
> > + default:
> > + return NOTIFY_DONE;
> > + };
> > +
> > + return NOTIFY_OK;
> > +}
> > +
> > +/**
> > + * smu_wbrf_support_check - check wbrf support
> > + *
> > + * @smu: smu_context pointer
> > + *
> > + * Verifies the ACPI interface whether wbrf is supported.
> > + */
> > +static void smu_wbrf_support_check(struct smu_context *smu) {
> > + struct amdgpu_device *adev = smu->adev;
> > +
> > + smu->wbrf_supported = smu_is_asic_wbrf_supported(smu) &&
> > + !!amdgpu_wbrf &&
> > + wbrf_supported_consumer(adev->dev);
> > +
> > + if (smu->wbrf_supported)
> > + dev_info(adev->dev, "RF interference mitigation is
> supported\n"); }
> > +
> > +/**
> > + * smu_wbrf_init - init driver wbrf support
> > + *
> > + * @smu: smu_context pointer
> > + *
> > + * Verifies the AMD ACPI interfaces and registers with the wbrf
> > + * notifier chain if wbrf feature is supported.
> > + * Returns 0 on success, error on failure.
> > + */
> > +static int smu_wbrf_init(struct smu_context *smu) {
> > + struct amdgpu_device *adev = smu->adev;
> > + int ret;
> > +
> > + if (!smu->wbrf_supported)
> > + return 0;
> > +
> > + smu->wbrf_notifier.notifier_call = smu_wbrf_event_handler;
> > + ret = wbrf_register_notifier(&smu->wbrf_notifier);
> > + if (ret)
> > + return ret;
> > +
> > + /*
> > + * Some wifiband exclusion ranges may be already there
> > + * before our driver loaded. To make sure our driver
> > + * is awared of those exclusion ranges.
> > + */
> > + ret = smu_wbrf_handle_exclusion_ranges(smu);
> > + if (ret)
> > + dev_err(adev->dev, "Failed to handle wbrf exclusion
> ranges\n");
> > +
> > + return ret;
> > +}
> > +
> > +/**
> > + * smu_wbrf_fini - tear down driver wbrf support
> > + *
> > + * @smu: smu_context pointer
> > + *
> > + * Unregisters with the wbrf notifier chain.
> > + */
> > +static void smu_wbrf_fini(struct smu_context *smu) {
> > + if (!smu->wbrf_supported)
> > + return;
> > +
> > + wbrf_unregister_notifier(&smu->wbrf_notifier);
> > +}
> > +
> > static int smu_smc_hw_setup(struct smu_context *smu)
> > {
> > struct smu_feature *feature = &smu->smu_feature; @@ -1320,6
> > +1487,15 @@ static int smu_smc_hw_setup(struct smu_context *smu)
> > if (ret)
> > return ret;
> >
> > + /* Enable UclkShadow on wbrf supported */
> > + if (smu->wbrf_supported) {
> > + ret = smu_enable_uclk_shadow(smu, true);
> > + if (ret) {
> > + dev_err(adev->dev, "Failed to enable UclkShadow
> feature to support wbrf!\n");
> > + return ret;
> > + }
> > + }
> > +
> > /*
> > * With SCPM enabled, these actions(and relevant messages) are
> > * not needed and permitted.
> > @@ -1416,6 +1592,15 @@ static int smu_smc_hw_setup(struct
> smu_context *smu)
> > */
> > ret = smu_set_min_dcef_deep_sleep(smu,
> > smu->smu_table.boot_values.dcefclk
> / 100);
> > + if (ret) {
> > + dev_err(adev->dev, "Error setting min deepsleep dcefclk\n");
> > + return ret;
> > + }
> > +
> > + /* Init wbrf support. Properly setup the notifier */
> > + ret = smu_wbrf_init(smu);
> > + if (ret)
> > + dev_err(adev->dev, "Error during wbrf init call\n");
> >
> > return ret;
> > }
> > @@ -1471,6 +1656,13 @@ static int smu_hw_init(void *handle)
> > return ret;
> > }
> >
> > + /*
> > + * Check whether wbrf is supported. This needs to be done
> > + * before SMU setup starts since part of SMU configuration
> > + * relies on this.
> > + */
> > + smu_wbrf_support_check(smu);
> > +
> > if (smu->is_apu) {
> > ret = smu_set_gfx_imu_enable(smu);
> > if (ret)
> > @@ -1623,6 +1815,8 @@ static int smu_smc_hw_cleanup(struct
> smu_context *smu)
> > struct amdgpu_device *adev = smu->adev;
> > int ret = 0;
> >
> > + smu_wbrf_fini(smu);
> > +
> > cancel_work_sync(&smu->throttling_logging_work);
> > cancel_work_sync(&smu->interrupt_work);
> >
> > diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> > b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> > index 6e2069dcb6b9..244297979f92 100644
> > --- a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> > +++ b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> > @@ -22,6 +22,8 @@
> > #ifndef __AMDGPU_SMU_H__
> > #define __AMDGPU_SMU_H__
> >
> > +#include <linux/wbrf.h>
> > +
> > #include "amdgpu.h"
> > #include "kgd_pp_interface.h"
> > #include "dm_pp_interface.h"
> > @@ -575,6 +577,10 @@ struct smu_context
> > u32 debug_resp_reg;
> >
> > struct delayed_work swctf_delayed_work;
> > +
> > + /* data structures for wbrf feature support */
> > + bool wbrf_supported;
> > + struct notifier_block wbrf_notifier;
> > };
> >
> > struct i2c_adapter;
> > @@ -1356,6 +1362,23 @@ struct pptable_funcs {
> > * @init_pptable_microcode: Prepare the pptable microcode to upload
> via PSP
> > */
> > int (*init_pptable_microcode)(struct smu_context *smu);
> > +
> > + /**
> > + * @is_asic_wbrf_supported: check whether PMFW supports the wbrf
> feature
> > + */
> > + bool (*is_asic_wbrf_supported)(struct smu_context *smu);
> > +
> > + /**
> > + * @enable_uclk_shadow: Enable the uclk shadow feature on wbrf
> supported
> > + */
> > + int (*enable_uclk_shadow)(struct smu_context *smu,
> > + bool enablement);
> > +
> > + /**
> > + * @set_wbrf_exclusion_ranges: notify SMU the wifi bands occupied
> > + */
> > + int (*set_wbrf_exclusion_ranges)(struct smu_context *smu,
> > + struct exclusion_range
> *exclusion_ranges);
> > };
> >
> > typedef enum {
> > diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu_internal.h
> > b/drivers/gpu/drm/amd/pm/swsmu/smu_internal.h
> > index ceb13c838067..67d7495ab49e 100644
> > --- a/drivers/gpu/drm/amd/pm/swsmu/smu_internal.h
> > +++ b/drivers/gpu/drm/amd/pm/swsmu/smu_internal.h
> > @@ -97,6 +97,9 @@
> > #define smu_get_default_config_table_settings(smu, config_table)
> smu_ppt_funcs(get_default_config_table_settings, -EOPNOTSUPP,
> smu, config_table)
> > #define smu_set_config_table(smu, config_table)
> smu_ppt_funcs(set_config_table, -EOPNOTSUPP, smu, config_table)
> > #define smu_init_pptable_microcode(smu)
> smu_ppt_funcs(init_pptable_microcode, 0, smu)
> > +#define smu_is_asic_wbrf_supported(smu)
> smu_ppt_funcs(is_asic_wbrf_supported, false, smu)
> > +#define smu_enable_uclk_shadow(smu, enablement)
> smu_ppt_funcs(enable_uclk_shadow, 0, smu, enablement)
> > +#define smu_set_wbrf_exclusion_ranges(smu, exclusion_ranges)
> smu_ppt_funcs(set_wbrf_exclusion_ranges, -EOPNOTSUPP, smu,
> exclusion_ranges)
> >
> > #endif
> > #endif
^ permalink raw reply [flat|nested] 25+ messages in thread
* [V9 7/9] drm/amd/pm: add flood detection for wbrf events
2023-08-18 3:26 [V9 0/9] Enable Wifi RFI interference mitigation feature support Evan Quan
` (5 preceding siblings ...)
2023-08-18 3:26 ` [V9 6/9] drm/amd/pm: setup the framework to support Wifi RFI mitigation feature Evan Quan
@ 2023-08-18 3:26 ` Evan Quan
2023-08-18 9:49 ` Lazar, Lijo
2023-08-18 3:26 ` [V9 8/9] drm/amd/pm: enable Wifi RFI mitigation feature support for SMU13.0.0 Evan Quan
2023-08-18 3:26 ` [V9 9/9] drm/amd/pm: enable Wifi RFI mitigation feature support for SMU13.0.7 Evan Quan
8 siblings, 1 reply; 25+ messages in thread
From: Evan Quan @ 2023-08-18 3:26 UTC (permalink / raw)
To: gregkh, rafael, lenb, johannes, davem, edumazet, kuba, pabeni,
alexander.deucher, andrew, rdunlap, quic_jjohnson, horms
Cc: linux-doc, linux-kernel, linux-acpi, amd-gfx, linux-wireless,
netdev, Evan Quan, Mario Limonciello
To protect PMFW from being overloaded.
Signed-off-by: Evan Quan <evan.quan@amd.com>
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
---
drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 31 +++++++++++++++----
drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h | 7 +++++
2 files changed, 32 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
index 704442ce1da3..6c8bcdc17a15 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
@@ -1318,7 +1318,8 @@ static int smu_wbrf_event_handler(struct notifier_block *nb,
switch (action) {
case WBRF_CHANGED:
- smu_wbrf_handle_exclusion_ranges(smu);
+ schedule_delayed_work(&smu->wbrf_delayed_work,
+ msecs_to_jiffies(SMU_WBRF_EVENT_HANDLING_PACE));
break;
default:
return NOTIFY_DONE;
@@ -1327,6 +1328,21 @@ static int smu_wbrf_event_handler(struct notifier_block *nb,
return NOTIFY_OK;
}
+/**
+ * smu_wbrf_delayed_work_handler - callback on delayed work timer expired
+ *
+ * @work: struct work_struct pointer
+ *
+ * Flood is over and driver will consume the latest exclusion ranges.
+ */
+static void smu_wbrf_delayed_work_handler(struct work_struct *work)
+{
+ struct smu_context *smu =
+ container_of(work, struct smu_context, wbrf_delayed_work.work);
+
+ smu_wbrf_handle_exclusion_ranges(smu);
+}
+
/**
* smu_wbrf_support_check - check wbrf support
*
@@ -1357,12 +1373,14 @@ static void smu_wbrf_support_check(struct smu_context *smu)
*/
static int smu_wbrf_init(struct smu_context *smu)
{
- struct amdgpu_device *adev = smu->adev;
int ret;
if (!smu->wbrf_supported)
return 0;
+ INIT_DELAYED_WORK(&smu->wbrf_delayed_work,
+ smu_wbrf_delayed_work_handler);
+
smu->wbrf_notifier.notifier_call = smu_wbrf_event_handler;
ret = wbrf_register_notifier(&smu->wbrf_notifier);
if (ret)
@@ -1373,11 +1391,10 @@ static int smu_wbrf_init(struct smu_context *smu)
* before our driver loaded. To make sure our driver
* is awared of those exclusion ranges.
*/
- ret = smu_wbrf_handle_exclusion_ranges(smu);
- if (ret)
- dev_err(adev->dev, "Failed to handle wbrf exclusion ranges\n");
+ schedule_delayed_work(&smu->wbrf_delayed_work,
+ msecs_to_jiffies(SMU_WBRF_EVENT_HANDLING_PACE));
- return ret;
+ return 0;
}
/**
@@ -1393,6 +1410,8 @@ static void smu_wbrf_fini(struct smu_context *smu)
return;
wbrf_unregister_notifier(&smu->wbrf_notifier);
+
+ cancel_delayed_work_sync(&smu->wbrf_delayed_work);
}
static int smu_smc_hw_setup(struct smu_context *smu)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
index 244297979f92..4d5cb1b511e5 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
+++ b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
@@ -480,6 +480,12 @@ struct stb_context {
#define WORKLOAD_POLICY_MAX 7
+/*
+ * Configure wbrf event handling pace as there can be only one
+ * event processed every SMU_WBRF_EVENT_HANDLING_PACE ms.
+ */
+#define SMU_WBRF_EVENT_HANDLING_PACE 10
+
struct smu_context
{
struct amdgpu_device *adev;
@@ -581,6 +587,7 @@ struct smu_context
/* data structures for wbrf feature support */
bool wbrf_supported;
struct notifier_block wbrf_notifier;
+ struct delayed_work wbrf_delayed_work;
};
struct i2c_adapter;
--
2.34.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [V9 7/9] drm/amd/pm: add flood detection for wbrf events
2023-08-18 3:26 ` [V9 7/9] drm/amd/pm: add flood detection for wbrf events Evan Quan
@ 2023-08-18 9:49 ` Lazar, Lijo
0 siblings, 0 replies; 25+ messages in thread
From: Lazar, Lijo @ 2023-08-18 9:49 UTC (permalink / raw)
To: Evan Quan, gregkh, rafael, lenb, johannes, davem, edumazet, kuba,
pabeni, alexander.deucher, andrew, rdunlap, quic_jjohnson, horms
Cc: linux-doc, netdev, linux-wireless, linux-kernel, amd-gfx,
linux-acpi, Mario Limonciello
On 8/18/2023 8:56 AM, Evan Quan wrote:
> To protect PMFW from being overloaded.
>
> Signed-off-by: Evan Quan <evan.quan@amd.com>
> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 31 +++++++++++++++----
> drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h | 7 +++++
> 2 files changed, 32 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> index 704442ce1da3..6c8bcdc17a15 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> @@ -1318,7 +1318,8 @@ static int smu_wbrf_event_handler(struct notifier_block *nb,
>
> switch (action) {
> case WBRF_CHANGED:
> - smu_wbrf_handle_exclusion_ranges(smu);
> + schedule_delayed_work(&smu->wbrf_delayed_work,
> + msecs_to_jiffies(SMU_WBRF_EVENT_HANDLING_PACE));
> break;
> default:
> return NOTIFY_DONE;
> @@ -1327,6 +1328,21 @@ static int smu_wbrf_event_handler(struct notifier_block *nb,
> return NOTIFY_OK;
> }
>
> +/**
> + * smu_wbrf_delayed_work_handler - callback on delayed work timer expired
> + *
> + * @work: struct work_struct pointer
> + *
> + * Flood is over and driver will consume the latest exclusion ranges.
> + */
> +static void smu_wbrf_delayed_work_handler(struct work_struct *work)
> +{
> + struct smu_context *smu =
> + container_of(work, struct smu_context, wbrf_delayed_work.work);
> +
> + smu_wbrf_handle_exclusion_ranges(smu);
> +}
> +
> /**
> * smu_wbrf_support_check - check wbrf support
> *
> @@ -1357,12 +1373,14 @@ static void smu_wbrf_support_check(struct smu_context *smu)
> */
> static int smu_wbrf_init(struct smu_context *smu)
> {
> - struct amdgpu_device *adev = smu->adev;
> int ret;
>
> if (!smu->wbrf_supported)
> return 0;
>
> + INIT_DELAYED_WORK(&smu->wbrf_delayed_work,
> + smu_wbrf_delayed_work_handler);
> +
This is one-time and can be part of sw_init.
Thanks,
Lijo
> smu->wbrf_notifier.notifier_call = smu_wbrf_event_handler;
> ret = wbrf_register_notifier(&smu->wbrf_notifier);
> if (ret)
> @@ -1373,11 +1391,10 @@ static int smu_wbrf_init(struct smu_context *smu)
> * before our driver loaded. To make sure our driver
> * is awared of those exclusion ranges.
> */
> - ret = smu_wbrf_handle_exclusion_ranges(smu);
> - if (ret)
> - dev_err(adev->dev, "Failed to handle wbrf exclusion ranges\n");
> + schedule_delayed_work(&smu->wbrf_delayed_work,
> + msecs_to_jiffies(SMU_WBRF_EVENT_HANDLING_PACE));
>
> - return ret;
> + return 0;
> }
>
> /**
> @@ -1393,6 +1410,8 @@ static void smu_wbrf_fini(struct smu_context *smu)
> return;
>
> wbrf_unregister_notifier(&smu->wbrf_notifier);
> +
> + cancel_delayed_work_sync(&smu->wbrf_delayed_work);
> }
>
> static int smu_smc_hw_setup(struct smu_context *smu)
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> index 244297979f92..4d5cb1b511e5 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> +++ b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> @@ -480,6 +480,12 @@ struct stb_context {
>
> #define WORKLOAD_POLICY_MAX 7
>
> +/*
> + * Configure wbrf event handling pace as there can be only one
> + * event processed every SMU_WBRF_EVENT_HANDLING_PACE ms.
> + */
> +#define SMU_WBRF_EVENT_HANDLING_PACE 10
> +
> struct smu_context
> {
> struct amdgpu_device *adev;
> @@ -581,6 +587,7 @@ struct smu_context
> /* data structures for wbrf feature support */
> bool wbrf_supported;
> struct notifier_block wbrf_notifier;
> + struct delayed_work wbrf_delayed_work;
> };
>
> struct i2c_adapter;
^ permalink raw reply [flat|nested] 25+ messages in thread
* [V9 8/9] drm/amd/pm: enable Wifi RFI mitigation feature support for SMU13.0.0
2023-08-18 3:26 [V9 0/9] Enable Wifi RFI interference mitigation feature support Evan Quan
` (6 preceding siblings ...)
2023-08-18 3:26 ` [V9 7/9] drm/amd/pm: add flood detection for wbrf events Evan Quan
@ 2023-08-18 3:26 ` Evan Quan
2023-08-18 9:54 ` Lazar, Lijo
2023-08-18 3:26 ` [V9 9/9] drm/amd/pm: enable Wifi RFI mitigation feature support for SMU13.0.7 Evan Quan
8 siblings, 1 reply; 25+ messages in thread
From: Evan Quan @ 2023-08-18 3:26 UTC (permalink / raw)
To: gregkh, rafael, lenb, johannes, davem, edumazet, kuba, pabeni,
alexander.deucher, andrew, rdunlap, quic_jjohnson, horms
Cc: linux-doc, linux-kernel, linux-acpi, amd-gfx, linux-wireless,
netdev, Evan Quan, Mario Limonciello
Fulfill the SMU13.0.0 support for Wifi RFI mitigation feature.
Signed-off-by: Evan Quan <evan.quan@amd.com>
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
---
drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h | 3 +
drivers/gpu/drm/amd/pm/swsmu/inc/smu_types.h | 3 +-
drivers/gpu/drm/amd/pm/swsmu/inc/smu_v13_0.h | 3 +
.../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c | 9 +++
.../drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c | 60 +++++++++++++++++++
5 files changed, 77 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
index 4d5cb1b511e5..54e76d6e66ce 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
+++ b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
@@ -325,6 +325,7 @@ enum smu_table_id
SMU_TABLE_PACE,
SMU_TABLE_ECCINFO,
SMU_TABLE_COMBO_PPTABLE,
+ SMU_TABLE_WIFIBAND,
SMU_TABLE_COUNT,
};
@@ -1501,6 +1502,8 @@ enum smu_baco_seq {
__dst_size); \
})
+#define HZ_IN_MHZ 1000000U
+
#if !defined(SWSMU_CODE_LAYER_L2) && !defined(SWSMU_CODE_LAYER_L3) && !defined(SWSMU_CODE_LAYER_L4)
int smu_get_power_limit(void *handle,
uint32_t *limit,
diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/smu_types.h b/drivers/gpu/drm/amd/pm/swsmu/inc/smu_types.h
index 297b70b9388f..5bbb60289a79 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/inc/smu_types.h
+++ b/drivers/gpu/drm/amd/pm/swsmu/inc/smu_types.h
@@ -245,7 +245,8 @@
__SMU_DUMMY_MAP(AllowGpo), \
__SMU_DUMMY_MAP(Mode2Reset), \
__SMU_DUMMY_MAP(RequestI2cTransaction), \
- __SMU_DUMMY_MAP(GetMetricsTable),
+ __SMU_DUMMY_MAP(GetMetricsTable), \
+ __SMU_DUMMY_MAP(EnableUCLKShadow),
#undef __SMU_DUMMY_MAP
#define __SMU_DUMMY_MAP(type) SMU_MSG_##type
diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/smu_v13_0.h b/drivers/gpu/drm/amd/pm/swsmu/inc/smu_v13_0.h
index 355c156d871a..dd70b56aa71e 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/inc/smu_v13_0.h
+++ b/drivers/gpu/drm/amd/pm/swsmu/inc/smu_v13_0.h
@@ -299,5 +299,8 @@ int smu_v13_0_update_pcie_parameters(struct smu_context *smu,
uint32_t pcie_gen_cap,
uint32_t pcie_width_cap);
+int smu_v13_0_enable_uclk_shadow(struct smu_context *smu,
+ bool enablement);
+
#endif
#endif
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c
index 9b62b45ebb7f..6a5cb582aa92 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c
@@ -2472,3 +2472,12 @@ int smu_v13_0_update_pcie_parameters(struct smu_context *smu,
return 0;
}
+
+int smu_v13_0_enable_uclk_shadow(struct smu_context *smu,
+ bool enablement)
+{
+ return smu_cmn_send_smc_msg_with_param(smu,
+ SMU_MSG_EnableUCLKShadow,
+ enablement,
+ NULL);
+}
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
index 3d188616ba24..fd3ac18653ed 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
@@ -154,6 +154,7 @@ static struct cmn2asic_msg_mapping smu_v13_0_0_message_map[SMU_MSG_MAX_COUNT] =
MSG_MAP(AllowGpo, PPSMC_MSG_SetGpoAllow, 0),
MSG_MAP(AllowIHHostInterrupt, PPSMC_MSG_AllowIHHostInterrupt, 0),
MSG_MAP(ReenableAcDcInterrupt, PPSMC_MSG_ReenableAcDcInterrupt, 0),
+ MSG_MAP(EnableUCLKShadow, PPSMC_MSG_EnableUCLKShadow, 0),
};
static struct cmn2asic_mapping smu_v13_0_0_clk_map[SMU_CLK_COUNT] = {
@@ -237,6 +238,7 @@ static struct cmn2asic_mapping smu_v13_0_0_table_map[SMU_TABLE_COUNT] = {
TAB_MAP(I2C_COMMANDS),
TAB_MAP(ECCINFO),
TAB_MAP(OVERDRIVE),
+ TAB_MAP(WIFIBAND),
};
static struct cmn2asic_mapping smu_v13_0_0_pwr_src_map[SMU_POWER_SOURCE_COUNT] = {
@@ -481,6 +483,9 @@ static int smu_v13_0_0_tables_init(struct smu_context *smu)
PAGE_SIZE, AMDGPU_GEM_DOMAIN_VRAM);
SMU_TABLE_INIT(tables, SMU_TABLE_ECCINFO, sizeof(EccInfoTable_t),
PAGE_SIZE, AMDGPU_GEM_DOMAIN_VRAM);
+ SMU_TABLE_INIT(tables, SMU_TABLE_WIFIBAND,
+ sizeof(WifiBandEntryTable_t), PAGE_SIZE,
+ AMDGPU_GEM_DOMAIN_VRAM);
smu_table->metrics_table = kzalloc(sizeof(SmuMetricsExternal_t), GFP_KERNEL);
if (!smu_table->metrics_table)
@@ -2593,6 +2598,58 @@ static ssize_t smu_v13_0_0_get_ecc_info(struct smu_context *smu,
return ret;
}
+static bool smu_v13_0_0_wbrf_support_check(struct smu_context *smu)
+{
+ struct amdgpu_device *adev = smu->adev;
+
+ switch (adev->ip_versions[MP1_HWIP][0]) {
+ case IP_VERSION(13, 0, 0):
+ return smu->smc_fw_version >= 0x004e6300;
+ case IP_VERSION(13, 0, 10):
+ return smu->smc_fw_version >= 0x00503300;
+ default:
+ return false;
+ }
+}
+
+static int smu_v13_0_0_set_wbrf_exclusion_ranges(struct smu_context *smu,
+ struct exclusion_range *exclusion_ranges)
+{
+ WifiBandEntryTable_t wifi_bands;
+ int valid_entries = 0;
+ int ret, i;
+
+ memset(&wifi_bands, 0, sizeof(wifi_bands));
+ for (i = 0; i < ARRAY_SIZE(wifi_bands.WifiBandEntry); i++) {
+ if (!exclusion_ranges[i].start &&
+ !exclusion_ranges[i].end)
+ break;
+
+ /* PMFW expects the inputs to be in Mhz unit */
+ wifi_bands.WifiBandEntry[valid_entries].LowFreq =
+ DIV_ROUND_DOWN_ULL(exclusion_ranges[i].start, HZ_IN_MHZ);
+ wifi_bands.WifiBandEntry[valid_entries++].HighFreq =
+ DIV_ROUND_UP_ULL(exclusion_ranges[i].end, HZ_IN_MHZ);
+ }
+ wifi_bands.WifiBandEntryNum = valid_entries;
+
+ /*
+ * Per confirm with PMFW team, WifiBandEntryNum = 0
+ * is a valid setting. So, there should be no direct
+ * return on that.
+ */
+
+ ret = smu_cmn_update_table(smu,
+ SMU_TABLE_WIFIBAND,
+ 0,
+ (void *)(&wifi_bands),
+ true);
+ if (ret)
+ dev_err(smu->adev->dev, "Failed to set wifiband!");
+
+ return ret;
+}
+
static const struct pptable_funcs smu_v13_0_0_ppt_funcs = {
.get_allowed_feature_mask = smu_v13_0_0_get_allowed_feature_mask,
.set_default_dpm_table = smu_v13_0_0_set_default_dpm_table,
@@ -2672,6 +2729,9 @@ static const struct pptable_funcs smu_v13_0_0_ppt_funcs = {
.send_hbm_bad_channel_flag = smu_v13_0_0_send_bad_mem_channel_flag,
.gpo_control = smu_v13_0_gpo_control,
.get_ecc_info = smu_v13_0_0_get_ecc_info,
+ .is_asic_wbrf_supported = smu_v13_0_0_wbrf_support_check,
+ .enable_uclk_shadow = smu_v13_0_enable_uclk_shadow,
+ .set_wbrf_exclusion_ranges = smu_v13_0_0_set_wbrf_exclusion_ranges,
};
void smu_v13_0_0_set_ppt_funcs(struct smu_context *smu)
--
2.34.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [V9 8/9] drm/amd/pm: enable Wifi RFI mitigation feature support for SMU13.0.0
2023-08-18 3:26 ` [V9 8/9] drm/amd/pm: enable Wifi RFI mitigation feature support for SMU13.0.0 Evan Quan
@ 2023-08-18 9:54 ` Lazar, Lijo
0 siblings, 0 replies; 25+ messages in thread
From: Lazar, Lijo @ 2023-08-18 9:54 UTC (permalink / raw)
To: Evan Quan, gregkh, rafael, lenb, johannes, davem, edumazet, kuba,
pabeni, alexander.deucher, andrew, rdunlap, quic_jjohnson, horms
Cc: linux-doc, netdev, linux-wireless, linux-kernel, amd-gfx,
linux-acpi, Mario Limonciello
On 8/18/2023 8:56 AM, Evan Quan wrote:
> Fulfill the SMU13.0.0 support for Wifi RFI mitigation feature.
>
> Signed-off-by: Evan Quan <evan.quan@amd.com>
> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h | 3 +
> drivers/gpu/drm/amd/pm/swsmu/inc/smu_types.h | 3 +-
> drivers/gpu/drm/amd/pm/swsmu/inc/smu_v13_0.h | 3 +
> .../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c | 9 +++
> .../drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c | 60 +++++++++++++++++++
> 5 files changed, 77 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> index 4d5cb1b511e5..54e76d6e66ce 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> +++ b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> @@ -325,6 +325,7 @@ enum smu_table_id
> SMU_TABLE_PACE,
> SMU_TABLE_ECCINFO,
> SMU_TABLE_COMBO_PPTABLE,
> + SMU_TABLE_WIFIBAND,
> SMU_TABLE_COUNT,
> };
>
> @@ -1501,6 +1502,8 @@ enum smu_baco_seq {
> __dst_size); \
> })
>
> +#define HZ_IN_MHZ 1000000U
> +
> #if !defined(SWSMU_CODE_LAYER_L2) && !defined(SWSMU_CODE_LAYER_L3) && !defined(SWSMU_CODE_LAYER_L4)
> int smu_get_power_limit(void *handle,
> uint32_t *limit,
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/smu_types.h b/drivers/gpu/drm/amd/pm/swsmu/inc/smu_types.h
> index 297b70b9388f..5bbb60289a79 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/inc/smu_types.h
> +++ b/drivers/gpu/drm/amd/pm/swsmu/inc/smu_types.h
> @@ -245,7 +245,8 @@
> __SMU_DUMMY_MAP(AllowGpo), \
> __SMU_DUMMY_MAP(Mode2Reset), \
> __SMU_DUMMY_MAP(RequestI2cTransaction), \
> - __SMU_DUMMY_MAP(GetMetricsTable),
> + __SMU_DUMMY_MAP(GetMetricsTable), \
> + __SMU_DUMMY_MAP(EnableUCLKShadow),
>
> #undef __SMU_DUMMY_MAP
> #define __SMU_DUMMY_MAP(type) SMU_MSG_##type
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/smu_v13_0.h b/drivers/gpu/drm/amd/pm/swsmu/inc/smu_v13_0.h
> index 355c156d871a..dd70b56aa71e 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/inc/smu_v13_0.h
> +++ b/drivers/gpu/drm/amd/pm/swsmu/inc/smu_v13_0.h
> @@ -299,5 +299,8 @@ int smu_v13_0_update_pcie_parameters(struct smu_context *smu,
> uint32_t pcie_gen_cap,
> uint32_t pcie_width_cap);
>
> +int smu_v13_0_enable_uclk_shadow(struct smu_context *smu,
> + bool enablement);
> +
> #endif
> #endif
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c
> index 9b62b45ebb7f..6a5cb582aa92 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c
> @@ -2472,3 +2472,12 @@ int smu_v13_0_update_pcie_parameters(struct smu_context *smu,
>
> return 0;
> }
> +
> +int smu_v13_0_enable_uclk_shadow(struct smu_context *smu,
> + bool enablement)
> +{
> + return smu_cmn_send_smc_msg_with_param(smu,
> + SMU_MSG_EnableUCLKShadow,
> + enablement,
> + NULL);
> +}
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
> index 3d188616ba24..fd3ac18653ed 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
> @@ -154,6 +154,7 @@ static struct cmn2asic_msg_mapping smu_v13_0_0_message_map[SMU_MSG_MAX_COUNT] =
> MSG_MAP(AllowGpo, PPSMC_MSG_SetGpoAllow, 0),
> MSG_MAP(AllowIHHostInterrupt, PPSMC_MSG_AllowIHHostInterrupt, 0),
> MSG_MAP(ReenableAcDcInterrupt, PPSMC_MSG_ReenableAcDcInterrupt, 0),
> + MSG_MAP(EnableUCLKShadow, PPSMC_MSG_EnableUCLKShadow, 0),
> };
>
> static struct cmn2asic_mapping smu_v13_0_0_clk_map[SMU_CLK_COUNT] = {
> @@ -237,6 +238,7 @@ static struct cmn2asic_mapping smu_v13_0_0_table_map[SMU_TABLE_COUNT] = {
> TAB_MAP(I2C_COMMANDS),
> TAB_MAP(ECCINFO),
> TAB_MAP(OVERDRIVE),
> + TAB_MAP(WIFIBAND),
> };
>
> static struct cmn2asic_mapping smu_v13_0_0_pwr_src_map[SMU_POWER_SOURCE_COUNT] = {
> @@ -481,6 +483,9 @@ static int smu_v13_0_0_tables_init(struct smu_context *smu)
> PAGE_SIZE, AMDGPU_GEM_DOMAIN_VRAM);
> SMU_TABLE_INIT(tables, SMU_TABLE_ECCINFO, sizeof(EccInfoTable_t),
> PAGE_SIZE, AMDGPU_GEM_DOMAIN_VRAM);
> + SMU_TABLE_INIT(tables, SMU_TABLE_WIFIBAND,
> + sizeof(WifiBandEntryTable_t), PAGE_SIZE,
> + AMDGPU_GEM_DOMAIN_VRAM);
>
> smu_table->metrics_table = kzalloc(sizeof(SmuMetricsExternal_t), GFP_KERNEL);
> if (!smu_table->metrics_table)
> @@ -2593,6 +2598,58 @@ static ssize_t smu_v13_0_0_get_ecc_info(struct smu_context *smu,
> return ret;
> }
>
> +static bool smu_v13_0_0_wbrf_support_check(struct smu_context *smu)
> +{
> + struct amdgpu_device *adev = smu->adev;
> +
> + switch (adev->ip_versions[MP1_HWIP][0]) {
> + case IP_VERSION(13, 0, 0):
> + return smu->smc_fw_version >= 0x004e6300;
> + case IP_VERSION(13, 0, 10):
> + return smu->smc_fw_version >= 0x00503300;
> + default:
> + return false;
> + }
> +}
> +
> +static int smu_v13_0_0_set_wbrf_exclusion_ranges(struct smu_context *smu,
> + struct exclusion_range *exclusion_ranges)
> +{
> + WifiBandEntryTable_t wifi_bands;
> + int valid_entries = 0;
> + int ret, i;
> +
> + memset(&wifi_bands, 0, sizeof(wifi_bands));
> + for (i = 0; i < ARRAY_SIZE(wifi_bands.WifiBandEntry); i++) {
> + if (!exclusion_ranges[i].start &&
> + !exclusion_ranges[i].end)
> + break;
> +
> + /* PMFW expects the inputs to be in Mhz unit */
> + wifi_bands.WifiBandEntry[valid_entries].LowFreq =
> + DIV_ROUND_DOWN_ULL(exclusion_ranges[i].start, HZ_IN_MHZ);
> + wifi_bands.WifiBandEntry[valid_entries++].HighFreq =
> + DIV_ROUND_UP_ULL(exclusion_ranges[i].end, HZ_IN_MHZ);
> + }
> + wifi_bands.WifiBandEntryNum = valid_entries;
> +
> + /*
> + * Per confirm with PMFW team, WifiBandEntryNum = 0
> + * is a valid setting. So, there should be no direct
> + * return on that.
> + */
> +
> + ret = smu_cmn_update_table(smu,
> + SMU_TABLE_WIFIBAND,
> + 0,
> + (void *)(&wifi_bands),
> + true);
> + if (ret)
> + dev_err(smu->adev->dev, "Failed to set wifiband!");
Since system could be operational even after this, can we downgrade this?
Thanks,
Lijo
> +
> + return ret;
> +}
> +
> static const struct pptable_funcs smu_v13_0_0_ppt_funcs = {
> .get_allowed_feature_mask = smu_v13_0_0_get_allowed_feature_mask,
> .set_default_dpm_table = smu_v13_0_0_set_default_dpm_table,
> @@ -2672,6 +2729,9 @@ static const struct pptable_funcs smu_v13_0_0_ppt_funcs = {
> .send_hbm_bad_channel_flag = smu_v13_0_0_send_bad_mem_channel_flag,
> .gpo_control = smu_v13_0_gpo_control,
> .get_ecc_info = smu_v13_0_0_get_ecc_info,
> + .is_asic_wbrf_supported = smu_v13_0_0_wbrf_support_check,
> + .enable_uclk_shadow = smu_v13_0_enable_uclk_shadow,
> + .set_wbrf_exclusion_ranges = smu_v13_0_0_set_wbrf_exclusion_ranges,
> };
>
> void smu_v13_0_0_set_ppt_funcs(struct smu_context *smu)
^ permalink raw reply [flat|nested] 25+ messages in thread
* [V9 9/9] drm/amd/pm: enable Wifi RFI mitigation feature support for SMU13.0.7
2023-08-18 3:26 [V9 0/9] Enable Wifi RFI interference mitigation feature support Evan Quan
` (7 preceding siblings ...)
2023-08-18 3:26 ` [V9 8/9] drm/amd/pm: enable Wifi RFI mitigation feature support for SMU13.0.0 Evan Quan
@ 2023-08-18 3:26 ` Evan Quan
8 siblings, 0 replies; 25+ messages in thread
From: Evan Quan @ 2023-08-18 3:26 UTC (permalink / raw)
To: gregkh, rafael, lenb, johannes, davem, edumazet, kuba, pabeni,
alexander.deucher, andrew, rdunlap, quic_jjohnson, horms
Cc: linux-doc, linux-kernel, linux-acpi, amd-gfx, linux-wireless,
netdev, Evan Quan, Mario Limonciello
Fulfill the SMU13.0.7 support for Wifi RFI mitigation feature.
Signed-off-by: Evan Quan <evan.quan@amd.com>
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
---
.../drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c | 59 +++++++++++++++++++
1 file changed, 59 insertions(+)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c
index b1f0937ccade..d02fe284b05d 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c
@@ -126,6 +126,7 @@ static struct cmn2asic_msg_mapping smu_v13_0_7_message_map[SMU_MSG_MAX_COUNT] =
MSG_MAP(AllowGpo, PPSMC_MSG_SetGpoAllow, 0),
MSG_MAP(GetPptLimit, PPSMC_MSG_GetPptLimit, 0),
MSG_MAP(NotifyPowerSource, PPSMC_MSG_NotifyPowerSource, 0),
+ MSG_MAP(EnableUCLKShadow, PPSMC_MSG_EnableUCLKShadow, 0),
};
static struct cmn2asic_mapping smu_v13_0_7_clk_map[SMU_CLK_COUNT] = {
@@ -207,6 +208,7 @@ static struct cmn2asic_mapping smu_v13_0_7_table_map[SMU_TABLE_COUNT] = {
TAB_MAP(ACTIVITY_MONITOR_COEFF),
[SMU_TABLE_COMBO_PPTABLE] = {1, TABLE_COMBO_PPTABLE},
TAB_MAP(OVERDRIVE),
+ TAB_MAP(WIFIBAND),
};
static struct cmn2asic_mapping smu_v13_0_7_pwr_src_map[SMU_POWER_SOURCE_COUNT] = {
@@ -497,6 +499,9 @@ static int smu_v13_0_7_tables_init(struct smu_context *smu)
AMDGPU_GEM_DOMAIN_VRAM);
SMU_TABLE_INIT(tables, SMU_TABLE_COMBO_PPTABLE, MP0_MP1_DATA_REGION_SIZE_COMBOPPTABLE,
PAGE_SIZE, AMDGPU_GEM_DOMAIN_VRAM);
+ SMU_TABLE_INIT(tables, SMU_TABLE_WIFIBAND,
+ sizeof(WifiBandEntryTable_t), PAGE_SIZE,
+ AMDGPU_GEM_DOMAIN_VRAM);
smu_table->metrics_table = kzalloc(sizeof(SmuMetricsExternal_t), GFP_KERNEL);
if (!smu_table->metrics_table)
@@ -2173,6 +2178,57 @@ static int smu_v13_0_7_set_df_cstate(struct smu_context *smu,
NULL);
}
+static bool smu_v13_0_7_wbrf_support_check(struct smu_context *smu)
+{
+ return smu->smc_fw_version > 0x00524600;
+}
+
+static int smu_v13_0_7_set_wbrf_exclusion_ranges(struct smu_context *smu,
+ struct exclusion_range *exclusion_ranges)
+{
+ WifiBandEntryTable_t wifi_bands;
+ int valid_entries = 0;
+ int ret, i;
+
+ memset(&wifi_bands, 0, sizeof(wifi_bands));
+ for (i = 0; i < ARRAY_SIZE(wifi_bands.WifiBandEntry); i++) {
+ if (!exclusion_ranges[i].start &&
+ !exclusion_ranges[i].end)
+ break;
+
+ /* PMFW expects the inputs to be in Mhz unit */
+ wifi_bands.WifiBandEntry[valid_entries].LowFreq =
+ DIV_ROUND_DOWN_ULL(exclusion_ranges[i].start, HZ_IN_MHZ);
+ wifi_bands.WifiBandEntry[valid_entries++].HighFreq =
+ DIV_ROUND_UP_ULL(exclusion_ranges[i].end, HZ_IN_MHZ);
+ }
+ wifi_bands.WifiBandEntryNum = valid_entries;
+
+ /*
+ * Per confirm with PMFW team, WifiBandEntryNum = 0 is a valid setting.
+ * Considering the scenarios below:
+ * - At first the wifi device adds an exclusion range e.g. (2400,2500) to
+ * BIOS and our driver gets notified. We will set WifiBandEntryNum = 1
+ * and pass the WifiBandEntry (2400, 2500) to PMFW.
+ *
+ * - Later the wifi device removes the wifiband list added above and
+ * our driver gets notified again. At this time, driver will set
+ * WifiBandEntryNum = 0 and pass an empty WifiBandEntry list to PMFW.
+ * - PMFW may still need to do some uclk shadow update(e.g. switching
+ * from shadow clock back to primary clock) on receiving this.
+ */
+
+ ret = smu_cmn_update_table(smu,
+ SMU_TABLE_WIFIBAND,
+ 0,
+ (void *)(&wifi_bands),
+ true);
+ if (ret)
+ dev_err(smu->adev->dev, "Failed to set wifiband!");
+
+ return ret;
+}
+
static const struct pptable_funcs smu_v13_0_7_ppt_funcs = {
.get_allowed_feature_mask = smu_v13_0_7_get_allowed_feature_mask,
.set_default_dpm_table = smu_v13_0_7_set_default_dpm_table,
@@ -2241,6 +2297,9 @@ static const struct pptable_funcs smu_v13_0_7_ppt_funcs = {
.set_mp1_state = smu_v13_0_7_set_mp1_state,
.set_df_cstate = smu_v13_0_7_set_df_cstate,
.gpo_control = smu_v13_0_gpo_control,
+ .is_asic_wbrf_supported = smu_v13_0_7_wbrf_support_check,
+ .enable_uclk_shadow = smu_v13_0_enable_uclk_shadow,
+ .set_wbrf_exclusion_ranges = smu_v13_0_7_set_wbrf_exclusion_ranges,
};
void smu_v13_0_7_set_ppt_funcs(struct smu_context *smu)
--
2.34.1
^ permalink raw reply related [flat|nested] 25+ messages in thread