[AMD Official Use Only - General] > -----Original Message----- > From: Limonciello, Mario <Mario.Limonciello@xxxxxxx> > Sent: Saturday, July 1, 2023 12:41 AM > To: Quan, Evan <Evan.Quan@xxxxxxx>; rafael@xxxxxxxxxx; lenb@xxxxxxxxxx; > Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Koenig, Christian > <Christian.Koenig@xxxxxxx>; Pan, Xinhui <Xinhui.Pan@xxxxxxx>; > airlied@xxxxxxxxx; daniel@xxxxxxxx; johannes@xxxxxxxxxxxxxxxx; > davem@xxxxxxxxxxxxx; edumazet@xxxxxxxxxx; kuba@xxxxxxxxxx; > pabeni@xxxxxxxxxx; mdaenzer@xxxxxxxxxx; > maarten.lankhorst@xxxxxxxxxxxxxxx; tzimmermann@xxxxxxx; > hdegoede@xxxxxxxxxx; jingyuwang_vip@xxxxxxx; Lazar, Lijo > <Lijo.Lazar@xxxxxxx>; jim.cromie@xxxxxxxxx; bellosilicio@xxxxxxxxx; > andrealmeid@xxxxxxxxxx; trix@xxxxxxxxxx; jsg@xxxxxxxxx; arnd@xxxxxxxx > Cc: linux-kernel@xxxxxxxxxxxxxxx; linux-acpi@xxxxxxxxxxxxxxx; amd- > gfx@xxxxxxxxxxxxxxxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx; linux- > wireless@xxxxxxxxxxxxxxx; netdev@xxxxxxxxxxxxxxx > 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@xxxxxxx> > > Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx> > > Co-developed-by: Evan Quan <evan.quan@xxxxxxx> > > Signed-off-by: Evan Quan <evan.quan@xxxxxxx> > > -- > > 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? Evan > > > +#endif /* _LINUX_WBRF_H */