Messages in this thread | | | Date | Mon, 3 Jul 2023 22:53:26 -0500 | Subject | Re: [PATCH V5 1/9] drivers core: Add support for Wifi band RF mitigations | From | Mario Limonciello <> |
| |
On 7/3/23 22:40, Quan, Evan wrote: > [AMD Official Use Only - General] > >> -----Original Message----- >> From: Limonciello, Mario <Mario.Limonciello@amd.com> >> Sent: Saturday, July 1, 2023 12:41 AM >> To: Quan, Evan <Evan.Quan@amd.com>; rafael@kernel.org; lenb@kernel.org; >> Deucher, Alexander <Alexander.Deucher@amd.com>; Koenig, Christian >> <Christian.Koenig@amd.com>; Pan, Xinhui <Xinhui.Pan@amd.com>; >> airlied@gmail.com; daniel@ffwll.ch; johannes@sipsolutions.net; >> davem@davemloft.net; edumazet@google.com; kuba@kernel.org; >> pabeni@redhat.com; mdaenzer@redhat.com; >> maarten.lankhorst@linux.intel.com; tzimmermann@suse.de; >> hdegoede@redhat.com; jingyuwang_vip@163.com; Lazar, Lijo >> <Lijo.Lazar@amd.com>; jim.cromie@gmail.com; bellosilicio@gmail.com; >> andrealmeid@igalia.com; trix@redhat.com; jsg@jsg.id.au; arnd@arndb.de >> Cc: linux-kernel@vger.kernel.org; linux-acpi@vger.kernel.org; amd- >> gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; linux- >> wireless@vger.kernel.org; netdev@vger.kernel.org >> Subject: Re: [PATCH V5 1/9] drivers core: Add support for Wifi band RF >> mitigations >> >> On 6/30/2023 05:32, Evan Quan wrote: >>> Due to electrical and mechanical constraints in certain platform >>> designs there may be likely interference of relatively high-powered >>> harmonics of the (G-)DDR memory clocks with local radio module >>> frequency bands used by Wifi 6/6e/7. >>> >>> To mitigate this, AMD has introduced a mechanism that devices can use >>> to notify active use of particular frequencies so that other devices >>> can make relative internal adjustments as necessary to avoid this resonance. >>> >>> 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_retrieve_exclusions` to retrieve the current >>> exclusions on receiving an ACPI notification for a new frequency >>> change. >>> >>> 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) >>> --- >>> drivers/base/Kconfig | 8 ++ >>> drivers/base/Makefile | 1 + >>> drivers/base/wbrf.c | 227 >> ++++++++++++++++++++++++++++++++++++++++++ >>> include/linux/wbrf.h | 65 ++++++++++++ >>> 4 files changed, 301 insertions(+) >>> create mode 100644 drivers/base/wbrf.c >>> create mode 100644 include/linux/wbrf.h >>> >>> diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig index >>> 2b8fd6bb7da0..5b441017b225 100644 >>> --- a/drivers/base/Kconfig >>> +++ b/drivers/base/Kconfig >>> @@ -242,4 +242,12 @@ config FW_DEVLINK_SYNC_STATE_TIMEOUT >>> command line option on every system/board your kernel is expected >> to >>> work on. >>> >>> +config WBRF >>> + bool "Wifi band RF mitigation mechanism" >>> + default n >>> + 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. >>> + >>> endmenu >>> diff --git a/drivers/base/Makefile b/drivers/base/Makefile index >>> 3079bfe53d04..c844f68a6830 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-$(CONFIG_WBRF) += wbrf.o >>> >>> obj-y += test/ >>> >>> diff --git a/drivers/base/wbrf.c b/drivers/base/wbrf.c new file mode >>> 100644 index 000000000000..2163a8ec8a9a >>> --- /dev/null >>> +++ b/drivers/base/wbrf.c >>> @@ -0,0 +1,227 @@ >>> +// 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 struct exclusion_range_pool 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; >>> + } >>> + >>> + return 0; >>> +} >>> + >>> +/** >>> + * wbrf_supported_producer - Determine if the device can report >>> +frequencies >>> + * >>> + * @dev: device pointer >>> + * >>> + * WBRF is used to mitigate devices that cause harmonic interference. >>> + * This function will determine if this device needs to report such >> frequencies. >>> + */ >>> +bool wbrf_supported_producer(struct device *dev) { >>> + 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 can react to >>> +frequencies >>> + * >>> + * @dev: device pointer >>> + * >>> + * WBRF is used to mitigate devices that cause harmonic interference. >>> + * This function will determine if this device needs to react to >>> +reports from >>> + * other devices for such frequencies. >>> + */ >>> +bool wbrf_supported_consumer(struct device *dev) { >>> + 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..3ca95786cef5 >>> --- /dev/null >>> +++ b/include/linux/wbrf.h >>> @@ -0,0 +1,65 @@ >>> +/* 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 */ >>> + uint64_t start; >>> + uint64_t end; >>> +}; >>> + >>> +struct exclusion_range_pool { >>> + struct exclusion_range band_list[MAX_NUM_OF_WBRF_RANGES]; >>> + uint64_t >> ref_counter[MAX_NUM_OF_WBRF_RANGES]; >>> +}; >>> + >>> +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 { >>> + uint32_t num_of_ranges; >>> + struct exclusion_range band_list[MAX_NUM_OF_WBRF_RANGES]; >>> +} __packed; >>> + >>> +enum wbrf_notifier_actions { >>> + WBRF_CHANGED, >>> +}; >>> + >>> +#ifdef CONFIG_WBRF >>> +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); #else static >>> +inline bool wbrf_supported_producer(struct device *dev) { return >>> +false; } static inline int wbrf_add_exclusion(struct device *adev, >>> + struct wbrf_ranges_in *in) { return - >> ENODEV; } static inline >>> +int wbrf_remove_exclusion(struct device *dev, >>> + struct wbrf_ranges_in *in) { return - >> ENODEV; } static inline int >>> +wbrf_retrieve_exclusions(struct device *dev, >>> + struct wbrf_ranges_out *out) >> { return -ENODEV; } static >>> +inline bool wbrf_supported_consumer(struct device *dev) { return >>> +false; } static inline int wbrf_register_notifier(struct >>> +notifier_block *nb) { return -ENODEV; } static inline int >>> +wbrf_unregister_notifier(struct notifier_block *nb) { return -ENODEV; >>> +} #endif >>> + >> >> Right now there are stubs for non CONFIG_WBRF as well as other patches are >> using #ifdef CONFIG_WBRF or having their own stubs. Like mac80211 patch >> looks for #ifdef CONFIG_WBRF. >> >> I think we should pick one or the other. > Right.. >> >> Having other subsystems #ifdef CONFIG_WBRF will make the series easier to >> land through multiple trees; so I have a slight leaning in that direction. > I kind of expecting to use the other way. That is to make CONFIG_WBRF agnostic to other subsystems or drivers. > They (other subsystems or drivers) can always assume those wbrf_xxxxx interfaces are available. > What they need to care only are the return values of those interfaces. > How do you think?
That's fine, thanks.
> > Evan >> >>> +#endif /* _LINUX_WBRF_H */ >
| |