diff mbox series

[v14,2/9] platform/x86/amd: Add support for AMD ACPI based Wifi band RFI mitigation feature

Message ID 20231129091348.3972539-3-Jun.Ma2@amd.com (mailing list archive)
State Not Applicable
Delegated to: Johannes Berg
Headers show
Series Enable Wifi RFI interference mitigation feature support | expand

Commit Message

Ma Jun Nov. 29, 2023, 9:13 a.m. UTC
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.

Co-developed-by: Evan Quan <quanliangl@hotmail.com>
Signed-off-by: Evan Quan <quanliangl@hotmail.com>
Signed-off-by: Ma Jun <Jun.Ma2@amd.com>

--
v11:
 - fix typo(Simon)
v12:
 - Fix the code logic (Rafael)
 - Move amd_wbrf.c to drivers/platform/x86/amd/wbrf.c
 - Updated Evan's email because he's no longer at AMD.Thanks
for his work in earlier versions.
v13:
 - Fix the format issue (IIpo Jarvinen)
 - Add comment for some functions
v14:
 - Use the apci_check_dsm and acpi_evaluate_dsm (Hans de Goede)
---
 drivers/platform/x86/amd/Kconfig  |  15 ++
 drivers/platform/x86/amd/Makefile |   1 +
 drivers/platform/x86/amd/wbrf.c   | 337 ++++++++++++++++++++++++++++++
 include/linux/acpi_amd_wbrf.h     |  94 +++++++++
 4 files changed, 447 insertions(+)
 create mode 100644 drivers/platform/x86/amd/wbrf.c
 create mode 100644 include/linux/acpi_amd_wbrf.h

Comments

Limonciello, Mario Nov. 29, 2023, 5:24 p.m. UTC | #1
On 11/29/2023 03:13, Ma Jun 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.
> 
> Co-developed-by: Evan Quan <quanliangl@hotmail.com>
> Signed-off-by: Evan Quan <quanliangl@hotmail.com>
> Signed-off-by: Ma Jun <Jun.Ma2@amd.com>

I have one very minor nit below that if no other feedback is needed for 
the series can be fixed when committing.

Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>

> 
> --
> v11:
>   - fix typo(Simon)
> v12:
>   - Fix the code logic (Rafael)
>   - Move amd_wbrf.c to drivers/platform/x86/amd/wbrf.c
>   - Updated Evan's email because he's no longer at AMD.Thanks
> for his work in earlier versions.
> v13:
>   - Fix the format issue (IIpo Jarvinen)
>   - Add comment for some functions
> v14:
>   - Use the apci_check_dsm and acpi_evaluate_dsm (Hans de Goede)
> ---
>   drivers/platform/x86/amd/Kconfig  |  15 ++
>   drivers/platform/x86/amd/Makefile |   1 +
>   drivers/platform/x86/amd/wbrf.c   | 337 ++++++++++++++++++++++++++++++
>   include/linux/acpi_amd_wbrf.h     |  94 +++++++++
>   4 files changed, 447 insertions(+)
>   create mode 100644 drivers/platform/x86/amd/wbrf.c
>   create mode 100644 include/linux/acpi_amd_wbrf.h
> 
> diff --git a/drivers/platform/x86/amd/Kconfig b/drivers/platform/x86/amd/Kconfig
> index 55f3a2fc6aec..ac2b7758a04f 100644
> --- a/drivers/platform/x86/amd/Kconfig
> +++ b/drivers/platform/x86/amd/Kconfig
> @@ -18,3 +18,18 @@ config AMD_HSMP
>   
>   	  If you choose to compile this driver as a module the module will be
>   	  called amd_hsmp.
> +
> +config AMD_WBRF
> +	bool "AMD Wifi RF Band mitigations (WBRF)"
> +	depends on ACPI
> +	default n

I believe the "default n" is unnecessary as it's implied.

> +	help
> +	  WBRF(Wifi Band RFI mitigation) mechanism allows Wifi drivers
> +	  to notify the frequencies they are using so that other hardware
> +	  can be reconfigured to avoid harmonic conflicts.
> +
> +	  AMD provides an ACPI based mechanism to support WBRF on platform with
> +	  appropriate underlying support.
> +
> +	  This mechanism will only be activated on platforms that advertise a
> +	  need for it.
> diff --git a/drivers/platform/x86/amd/Makefile b/drivers/platform/x86/amd/Makefile
> index f04932b7a7d1..dcec0a46f8af 100644
> --- a/drivers/platform/x86/amd/Makefile
> +++ b/drivers/platform/x86/amd/Makefile
> @@ -8,3 +8,4 @@ obj-$(CONFIG_AMD_PMC)		+= pmc/
>   amd_hsmp-y			:= hsmp.o
>   obj-$(CONFIG_AMD_HSMP)		+= amd_hsmp.o
>   obj-$(CONFIG_AMD_PMF)		+= pmf/
> +obj-$(CONFIG_AMD_WBRF)		+= wbrf.o
> diff --git a/drivers/platform/x86/amd/wbrf.c b/drivers/platform/x86/amd/wbrf.c
> new file mode 100644
> index 000000000000..36e90a1159be
> --- /dev/null
> +++ b/drivers/platform/x86/amd/wbrf.c
> @@ -0,0 +1,337 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Wifi Frequency Band Manage Interface
> + * 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: WBRF supported.
> + * 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
> +
> +#define WBRF_REVISION		0x1
> +
> +/*
> + * The data structure used for WBRF_RETRIEVE is not naturally aligned.
> + * And unfortunately the design has been settled down.
> + */
> +struct amd_wbrf_ranges_out {
> +	u32			num_of_ranges;
> +	struct freq_band_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);
> +
> +/*
> + * Used to notify consumer (amdgpu driver currently) about
> + * the wifi frequency is change.
> + */
> +static BLOCKING_NOTIFIER_HEAD(wbrf_chain_head);
> +
> +static int wbrf_record(struct acpi_device *adev, uint8_t action, struct wbrf_ranges_in_out *in)
> +{
> +	union acpi_object argv4;
> +	union acpi_object *tmp;
> +	union acpi_object *obj;
> +	u32 num_of_ranges = 0;
> +	u32 num_of_elements;
> +	u32 arg_idx = 0;
> +	int ret;
> +	u32 i;
> +
> +	if (!in)
> +		return -EINVAL;
> +
> +	for (i = 0; i < ARRAY_SIZE(in->band_list); i++) {
> +		if (in->band_list[i].start && in->band_list[i].end)
> +			num_of_ranges++;
> +	}
> +
> +	/*
> +	 * The num_of_ranges value in the "in" object supplied by
> +	 * the caller is required to be equal to the number of
> +	 * entries in the band_list array in there.
> +	 */
> +	if (num_of_ranges != in->num_of_ranges)
> +		return -EINVAL;
> +
> +	/*
> +	 * Every input frequency band comes with two end points(start/end)
> +	 * and each 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 + 2;
> +
> +	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;
> +
> +	/* save the number of ranges*/
> +	tmp[0].integer.type = ACPI_TYPE_INTEGER;
> +	tmp[0].integer.value = num_of_ranges;
> +
> +	/* save the action(WBRF_RECORD_ADD/REMOVE/RETRIEVE) */
> +	tmp[1].integer.type = ACPI_TYPE_INTEGER;
> +	tmp[1].integer.value = action;
> +
> +	arg_idx = 2;
> +	for (i = 0; i < ARRAY_SIZE(in->band_list); i++) {
> +		if (!in->band_list[i].start || !in->band_list[i].end)
> +			continue;
> +
> +		tmp[arg_idx].integer.type = ACPI_TYPE_INTEGER;
> +		tmp[arg_idx++].integer.value = in->band_list[i].start;
> +		tmp[arg_idx].integer.type = ACPI_TYPE_INTEGER;
> +		tmp[arg_idx++].integer.value = in->band_list[i].end;
> +	}
> +
> +	obj = acpi_evaluate_dsm(adev->handle, &wifi_acpi_dsm_guid,
> +				WBRF_REVISION, WBRF_RECORD, &argv4);
> +
> +	if (!obj)
> +		return -EINVAL;
> +
> +	if (obj->type != ACPI_TYPE_INTEGER) {
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	ret = obj->integer.value;
> +	if (ret)
> +		ret = -EINVAL;
> +
> +out:
> +	ACPI_FREE(obj);
> +	kfree(tmp);
> +
> +	return ret;
> +}
> +
> +/**
> + * acpi_amd_wbrf_add_remove - add or remove the frequency band the device is using
> + *
> + * @dev: device pointer
> + * @action: remove or add the frequency band into bios
> + * @in: input structure containing the frequency band the device is using
> + *
> + * Broadcast to other consumers the frequency band the device starts
> + * to use. Underneath the surface the information is cached into an
> + * internal buffer first. Then a notification is sent to all those
> + * registered consumers. So then they can retrieve that buffer to
> + * know the latest active frequency bands. Consumers that haven't
> + * yet been registered can retrieve the information from the cache
> + * when they register.
> + *
> + * Return:
> + * 0 for success add/remove wifi frequency band.
> + * Returns a negative error code for failure.
> + */
> +int acpi_amd_wbrf_add_remove(struct device *dev, uint8_t action, struct wbrf_ranges_in_out *in)
> +{
> +	struct acpi_device *adev;
> +	int ret;
> +
> +	adev = ACPI_COMPANION(dev);
> +	if (!adev)
> +		return -ENODEV;
> +
> +	ret = wbrf_record(adev, action, in);
> +	if (ret)
> +		return ret;
> +
> +	blocking_notifier_call_chain(&wbrf_chain_head, WBRF_CHANGED, NULL);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(acpi_amd_wbrf_add_remove);
> +
> +static 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);
> +}
> +
> +/**
> + * acpi_amd_wbrf_supported_producer - determine if the WBRF can be enabled
> + *                                    for the device as a producer
> + *
> + * @dev: device pointer
> + *
> + * Check if the platform equipped with necessary implementations to
> + * support WBRF for the device as a producer.
> + *
> + * Return:
> + * true if WBRF is supported, otherwise returns false
> + */
> +bool acpi_amd_wbrf_supported_producer(struct device *dev)
> +{
> +	struct acpi_device *adev;
> +
> +	adev = ACPI_COMPANION(dev);
> +	if (!adev)
> +		return false;
> +
> +	if (!acpi_amd_wbrf_supported_system())
> +		return false;
> +
> +
> +	return acpi_check_dsm(adev->handle, &wifi_acpi_dsm_guid,
> +			      WBRF_REVISION, BIT(WBRF_RECORD));
> +}
> +EXPORT_SYMBOL_GPL(acpi_amd_wbrf_supported_producer);
> +
> +/**
> + * acpi_amd_wbrf_supported_consumer - determine if the WBRF can be enabled
> + *                                    for the device as a consumer
> + *
> + * @dev: device pointer
> + *
> + * Determine if the platform equipped with necessary implementations to
> + * support WBRF for the device as a consumer.
> + *
> + * Return:
> + * true if WBRF is supported, otherwise returns false.
> + */
> +bool acpi_amd_wbrf_supported_consumer(struct device *dev)
> +{
> +	struct acpi_device *adev;
> +
> +	adev = ACPI_COMPANION(dev);
> +	if (!adev)
> +		return false;
> +
> +	if (!acpi_amd_wbrf_supported_system())
> +		return false;
> +
> +	return acpi_check_dsm(adev->handle, &wifi_acpi_dsm_guid,
> +			      WBRF_REVISION, BIT(WBRF_RETRIEVE));
> +}
> +EXPORT_SYMBOL_GPL(acpi_amd_wbrf_supported_consumer);
> +
> +/**
> + * amd_wbrf_retrieve_freq_band - retrieve current active frequency
> + *                                     bands
> + *
> + * @dev: device pointer
> + * @out: output structure containing all the active frequency bands
> + *
> + * Retrieve the current active frequency bands which were broadcasted
> + * by other producers. The consumer who calls this API should take
> + * proper actions if any of the frequency band may cause RFI with its
> + * own frequency band used.
> + *
> + * Return:
> + * 0 for getting wifi freq band successfully.
> + * Returns a negative error code for failure.
> + */
> +int amd_wbrf_retrieve_freq_band(struct device *dev, struct wbrf_ranges_in_out *out)
> +{
> +	struct amd_wbrf_ranges_out acpi_out = {0};
> +	struct acpi_device *adev;
> +	union acpi_object *obj;
> +	union acpi_object param;
> +	int ret = 0;
> +
> +	adev = ACPI_COMPANION(dev);
> +	if (!adev)
> +		return -ENODEV;
> +
> +	param.type = ACPI_TYPE_STRING;
> +	param.string.length = 0;
> +	param.string.pointer = NULL;
> +
> +	obj = acpi_evaluate_dsm(adev->handle, &wifi_acpi_dsm_guid,
> +							WBRF_REVISION, WBRF_RETRIEVE, &param);
> +	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, "Wrong sized WBRT information");
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +	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));
> +
> +out:
> +	ACPI_FREE(obj);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(amd_wbrf_retrieve_freq_band);
> +
> +/**
> + * amd_wbrf_register_notifier - register for notifications of frequency
> + *                                   band update
> + *
> + * @nb: driver notifier block
> + *
> + * The consumer should register itself via this API so that it can get
> + * notified on the frequency band updates from other producers.
> + *
> + * Return:
> + * 0 for registering a consumer driver successfully.
> + * Returns a negative error code for failure.
> + */
> +int amd_wbrf_register_notifier(struct notifier_block *nb)
> +{
> +	return blocking_notifier_chain_register(&wbrf_chain_head, nb);
> +}
> +EXPORT_SYMBOL_GPL(amd_wbrf_register_notifier);
> +
> +/**
> + * amd_wbrf_unregister_notifier - unregister for notifications of
> + *                                     frequency band update
> + *
> + * @nb: driver notifier block
> + *
> + * The consumer should call this API when it is longer interested with
> + * the frequency band updates from other producers. Usually, this should
> + * be performed during driver cleanup.
> + *
> + * Return:
> + * 0 for unregistering a consumer driver.
> + * Returns a negative error code for failure.
> + */
> +int amd_wbrf_unregister_notifier(struct notifier_block *nb)
> +{
> +	return blocking_notifier_chain_unregister(&wbrf_chain_head, nb);
> +}
> +EXPORT_SYMBOL_GPL(amd_wbrf_unregister_notifier);
> diff --git a/include/linux/acpi_amd_wbrf.h b/include/linux/acpi_amd_wbrf.h
> new file mode 100644
> index 000000000000..15b54734a080
> --- /dev/null
> +++ b/include/linux/acpi_amd_wbrf.h
> @@ -0,0 +1,94 @@
> +/* 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/device.h>
> +#include <linux/notifier.h>
> +
> +/* The maximum number of frequency band ranges */
> +#define MAX_NUM_OF_WBRF_RANGES		11
> +
> +/* Record actions */
> +#define WBRF_RECORD_ADD		0x0
> +#define WBRF_RECORD_REMOVE	0x1
> +
> +/**
> + * struct freq_band_range - Wifi frequency band range definition
> + * @start: start frequency point (in Hz)
> + * @end: end frequency point (in Hz)
> + */
> +struct freq_band_range {
> +	u64		start;
> +	u64		end;
> +};
> +
> +/**
> + * struct wbrf_ranges_in_out - wbrf ranges info
> + * @num_of_ranges: total number of band ranges in this struct
> + * @band_list: array of Wifi band ranges
> + */
> +struct wbrf_ranges_in_out {
> +	u64			num_of_ranges;
> +	struct freq_band_range	band_list[MAX_NUM_OF_WBRF_RANGES];
> +};
> +
> +/**
> + * enum wbrf_notifier_actions - wbrf notifier actions index
> + * @WBRF_CHANGED: there was some frequency band updates. The consumers
> + *               should retrieve the latest active frequency bands.
> + */
> +enum wbrf_notifier_actions {
> +	WBRF_CHANGED,
> +};
> +
> +#if IS_ENABLED(CONFIG_AMD_WBRF)
> +bool acpi_amd_wbrf_supported_producer(struct device *dev);
> +int acpi_amd_wbrf_add_remove(struct device *dev, uint8_t action, struct wbrf_ranges_in_out *in);
> +bool acpi_amd_wbrf_supported_consumer(struct device *dev);
> +int amd_wbrf_retrieve_freq_band(struct device *dev, struct wbrf_ranges_in_out *out);
> +int amd_wbrf_register_notifier(struct notifier_block *nb);
> +int amd_wbrf_unregister_notifier(struct notifier_block *nb);
> +#else
> +static inline
> +bool acpi_amd_wbrf_supported_consumer(struct device *dev)
> +{
> +	return false;
> +}
> +static inline
> +int acpi_amd_wbrf_remove_exclusion(struct device *dev, struct wbrf_ranges_in_out *in)
> +{
> +	return -ENODEV;
> +}
> +static inline
> +int acpi_amd_wbrf_add_exclusion(struct device *dev, struct wbrf_ranges_in_out *in)
> +{
> +	return -ENODEV;
> +}
> +static inline
> +bool acpi_amd_wbrf_supported_producer(struct device *dev)
> +{
> +	return false;
> +}
> +static inline
> +int amd_wbrf_retrieve_freq_band(struct device *dev, struct wbrf_ranges_in_out *out)
> +{
> +	return -ENODEV;
> +}
> +static inline
> +int amd_wbrf_register_notifier(struct notifier_block *nb)
> +{
> +	return -ENODEV;
> +}
> +static inline
> +int amd_wbrf_unregister_notifier(struct notifier_block *nb)
> +{
> +	return -ENODEV;
> +}
> +#endif /* CONFIG_AMD_WBRF */
> +
> +#endif /* _ACPI_AMD_WBRF_H */
Hans de Goede Dec. 4, 2023, 1 p.m. UTC | #2
Hi,

On 11/29/23 10:13, Ma Jun 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.
> 
> Co-developed-by: Evan Quan <quanliangl@hotmail.com>
> Signed-off-by: Evan Quan <quanliangl@hotmail.com>
> Signed-off-by: Ma Jun <Jun.Ma2@amd.com>
> 
> --
> v11:
>  - fix typo(Simon)
> v12:
>  - Fix the code logic (Rafael)
>  - Move amd_wbrf.c to drivers/platform/x86/amd/wbrf.c
>  - Updated Evan's email because he's no longer at AMD.Thanks
> for his work in earlier versions.
> v13:
>  - Fix the format issue (IIpo Jarvinen)
>  - Add comment for some functions
> v14:
>  - Use the apci_check_dsm and acpi_evaluate_dsm (Hans de Goede)

Thank you this is much better.

I notice that the #define ACPI_AMD_WBRF_METHOD	"\\WBRF"
still exists though and that this is still used in
static bool acpi_amd_wbrf_supported_system(void).

I think it might be better to just remove
these 2 all together.

Checking if a DSM with the expected GUID is present
and if that has the correct bits set in its supported
mask should be enough.

And on future systems the implementer may decide to
not have a WBRF helper function at all and instead
handle everything in the _DSM method.

So the "\\WBRF" check seems to be checking for
what really is an implementation detail.

2 other very small remarks:

> +/**
> + * acpi_amd_wbrf_supported_producer - determine if the WBRF can be enabled
> + *                                    for the device as a producer
> + *
> + * @dev: device pointer
> + *
> + * Check if the platform equipped with necessary implementations to
> + * support WBRF for the device as a producer.
> + *
> + * Return:
> + * true if WBRF is supported, otherwise returns false
> + */
> +bool acpi_amd_wbrf_supported_producer(struct device *dev)
> +{
> +	struct acpi_device *adev;
> +
> +	adev = ACPI_COMPANION(dev);
> +	if (!adev)
> +		return false;
> +
> +	if (!acpi_amd_wbrf_supported_system())
> +		return false;
> +
> +
> +	return acpi_check_dsm(adev->handle, &wifi_acpi_dsm_guid,
> +			      WBRF_REVISION, BIT(WBRF_RECORD));
> +}
> +EXPORT_SYMBOL_GPL(acpi_amd_wbrf_supported_producer);

Please don't use double empty lines, one empty line to separate things
is enough.

> +
> +/**
> + * acpi_amd_wbrf_supported_consumer - determine if the WBRF can be enabled
> + *                                    for the device as a consumer
> + *
> + * @dev: device pointer
> + *
> + * Determine if the platform equipped with necessary implementations to
> + * support WBRF for the device as a consumer.
> + *
> + * Return:
> + * true if WBRF is supported, otherwise returns false.
> + */
> +bool acpi_amd_wbrf_supported_consumer(struct device *dev)
> +{
> +	struct acpi_device *adev;
> +
> +	adev = ACPI_COMPANION(dev);
> +	if (!adev)
> +		return false;
> +
> +	if (!acpi_amd_wbrf_supported_system())
> +		return false;
> +
> +	return acpi_check_dsm(adev->handle, &wifi_acpi_dsm_guid,
> +			      WBRF_REVISION, BIT(WBRF_RETRIEVE));
> +}
> +EXPORT_SYMBOL_GPL(acpi_amd_wbrf_supported_consumer);
> +
> +/**
> + * amd_wbrf_retrieve_freq_band - retrieve current active frequency
> + *                                     bands

You may go a bit over the 80 chars limit, please just make this
a single line:

 * amd_wbrf_retrieve_freq_band - retrieve current active frequency bands

> + *
> + * @dev: device pointer
> + * @out: output structure containing all the active frequency bands
> + *
> + * Retrieve the current active frequency bands which were broadcasted
> + * by other producers. The consumer who calls this API should take
> + * proper actions if any of the frequency band may cause RFI with its
> + * own frequency band used.
> + *
> + * Return:
> + * 0 for getting wifi freq band successfully.
> + * Returns a negative error code for failure.
> + */

Regards,

Hans
Ma, Jun Dec. 5, 2023, 7:22 a.m. UTC | #3
Hi Hans,

On 12/4/2023 9:00 PM, Hans de Goede wrote:
> Hi,
> 
> On 11/29/23 10:13, Ma Jun 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.
>>
>> Co-developed-by: Evan Quan <quanliangl@hotmail.com>
>> Signed-off-by: Evan Quan <quanliangl@hotmail.com>
>> Signed-off-by: Ma Jun <Jun.Ma2@amd.com>
>>
>> --
>> v11:
>>  - fix typo(Simon)
>> v12:
>>  - Fix the code logic (Rafael)
>>  - Move amd_wbrf.c to drivers/platform/x86/amd/wbrf.c
>>  - Updated Evan's email because he's no longer at AMD.Thanks
>> for his work in earlier versions.
>> v13:
>>  - Fix the format issue (IIpo Jarvinen)
>>  - Add comment for some functions
>> v14:
>>  - Use the apci_check_dsm and acpi_evaluate_dsm (Hans de Goede)
> 
> Thank you this is much better.
> 
> I notice that the #define ACPI_AMD_WBRF_METHOD	"\\WBRF"
> still exists though and that this is still used in
> static bool acpi_amd_wbrf_supported_system(void).
> 
> I think it might be better to just remove
> these 2 all together.
> 
> Checking if a DSM with the expected GUID is present
> and if that has the correct bits set in its supported
> mask should be enough.
> 
> And on future systems the implementer may decide to
> not have a WBRF helper function at all and instead
> handle everything in the _DSM method.
> 
> So the "\\WBRF" check seems to be checking for
> what really is an implementation detail.
> 
Yes,you are right. I will fix these issues.
Thanks for your review and suggestion. 

Regards,
Ma Jun
> 2 other very small remark
> 
>> +/**
>> + * acpi_amd_wbrf_supported_producer - determine if the WBRF can be enabled
>> + *                                    for the device as a producer
>> + *
>> + * @dev: device pointer
>> + *
>> + * Check if the platform equipped with necessary implementations to
>> + * support WBRF for the device as a producer.
>> + *
>> + * Return:
>> + * true if WBRF is supported, otherwise returns false
>> + */
>> +bool acpi_amd_wbrf_supported_producer(struct device *dev)
>> +{
>> +	struct acpi_device *adev;
>> +
>> +	adev = ACPI_COMPANION(dev);
>> +	if (!adev)
>> +		return false;
>> +
>> +	if (!acpi_amd_wbrf_supported_system())
>> +		return false;
>> +
>> +
>> +	return acpi_check_dsm(adev->handle, &wifi_acpi_dsm_guid,
>> +			      WBRF_REVISION, BIT(WBRF_RECORD));
>> +}
>> +EXPORT_SYMBOL_GPL(acpi_amd_wbrf_supported_producer);
> 
> Please don't use double empty lines, one empty line to separate things
> is enough.
> 
>> +
>> +/**
>> + * acpi_amd_wbrf_supported_consumer - determine if the WBRF can be enabled
>> + *                                    for the device as a consumer
>> + *
>> + * @dev: device pointer
>> + *
>> + * Determine if the platform equipped with necessary implementations to
>> + * support WBRF for the device as a consumer.
>> + *
>> + * Return:
>> + * true if WBRF is supported, otherwise returns false.
>> + */
>> +bool acpi_amd_wbrf_supported_consumer(struct device *dev)
>> +{
>> +	struct acpi_device *adev;
>> +
>> +	adev = ACPI_COMPANION(dev);
>> +	if (!adev)
>> +		return false;
>> +
>> +	if (!acpi_amd_wbrf_supported_system())
>> +		return false;
>> +
>> +	return acpi_check_dsm(adev->handle, &wifi_acpi_dsm_guid,
>> +			      WBRF_REVISION, BIT(WBRF_RETRIEVE));
>> +}
>> +EXPORT_SYMBOL_GPL(acpi_amd_wbrf_supported_consumer);
>> +
>> +/**
>> + * amd_wbrf_retrieve_freq_band - retrieve current active frequency
>> + *                                     bands
> 
> You may go a bit over the 80 chars limit, please just make this
> a single line:
> 
>  * amd_wbrf_retrieve_freq_band - retrieve current active frequency bands
> 
>> + *
>> + * @dev: device pointer
>> + * @out: output structure containing all the active frequency bands
>> + *
>> + * Retrieve the current active frequency bands which were broadcasted
>> + * by other producers. The consumer who calls this API should take
>> + * proper actions if any of the frequency band may cause RFI with its
>> + * own frequency band used.
>> + *
>> + * Return:
>> + * 0 for getting wifi freq band successfully.
>> + * Returns a negative error code for failure.
>> + */
> 
> Regards,
> 
> Hans
>
Ma, Jun Dec. 5, 2023, 7:24 a.m. UTC | #4
Hi Mario,

On 11/30/2023 1:24 AM, Mario Limonciello wrote:
> On 11/29/2023 03:13, Ma Jun 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.
>>
>> Co-developed-by: Evan Quan <quanliangl@hotmail.com>
>> Signed-off-by: Evan Quan <quanliangl@hotmail.com>
>> Signed-off-by: Ma Jun <Jun.Ma2@amd.com>
> 
> I have one very minor nit below that if no other feedback is needed for 
> the series can be fixed when committing.
> 
> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
> 
>>
>> --
>> v11:
>>   - fix typo(Simon)
>> v12:
>>   - Fix the code logic (Rafael)
>>   - Move amd_wbrf.c to drivers/platform/x86/amd/wbrf.c
>>   - Updated Evan's email because he's no longer at AMD.Thanks
>> for his work in earlier versions.
>> v13:
>>   - Fix the format issue (IIpo Jarvinen)
>>   - Add comment for some functions
>> v14:
>>   - Use the apci_check_dsm and acpi_evaluate_dsm (Hans de Goede)
>> ---
>>   drivers/platform/x86/amd/Kconfig  |  15 ++
>>   drivers/platform/x86/amd/Makefile |   1 +
>>   drivers/platform/x86/amd/wbrf.c   | 337 ++++++++++++++++++++++++++++++
>>   include/linux/acpi_amd_wbrf.h     |  94 +++++++++
>>   4 files changed, 447 insertions(+)
>>   create mode 100644 drivers/platform/x86/amd/wbrf.c
>>   create mode 100644 include/linux/acpi_amd_wbrf.h
>>
>> diff --git a/drivers/platform/x86/amd/Kconfig b/drivers/platform/x86/amd/Kconfig
>> index 55f3a2fc6aec..ac2b7758a04f 100644
>> --- a/drivers/platform/x86/amd/Kconfig
>> +++ b/drivers/platform/x86/amd/Kconfig
>> @@ -18,3 +18,18 @@ config AMD_HSMP
>>   
>>   	  If you choose to compile this driver as a module the module will be
>>   	  called amd_hsmp.
>> +
>> +config AMD_WBRF
>> +	bool "AMD Wifi RF Band mitigations (WBRF)"
>> +	depends on ACPI
>> +	default n
> 
> I believe the "default n" is unnecessary as it's implied.

Thanks for review. I'll fix this and other issues your commented
in the next version.

Regards,
Ma Jun

> 
>> +	help
>> +	  WBRF(Wifi Band RFI mitigation) mechanism allows Wifi drivers
>> +	  to notify the frequencies they are using so that other hardware
>> +	  can be reconfigured to avoid harmonic conflicts.
>> +
>> +	  AMD provides an ACPI based mechanism to support WBRF on platform with
>> +	  appropriate underlying support.
>> +
>> +	  This mechanism will only be activated on platforms that advertise a
>> +	  need for it.
>> diff --git a/drivers/platform/x86/amd/Makefile b/drivers/platform/x86/amd/Makefile
>> index f04932b7a7d1..dcec0a46f8af 100644
>> --- a/drivers/platform/x86/amd/Makefile
>> +++ b/drivers/platform/x86/amd/Makefile
>> @@ -8,3 +8,4 @@ obj-$(CONFIG_AMD_PMC)		+= pmc/
>>   amd_hsmp-y			:= hsmp.o
>>   obj-$(CONFIG_AMD_HSMP)		+= amd_hsmp.o
>>   obj-$(CONFIG_AMD_PMF)		+= pmf/
>> +obj-$(CONFIG_AMD_WBRF)		+= wbrf.o
>> diff --git a/drivers/platform/x86/amd/wbrf.c b/drivers/platform/x86/amd/wbrf.c
>> new file mode 100644
>> index 000000000000..36e90a1159be
>> --- /dev/null
>> +++ b/drivers/platform/x86/amd/wbrf.c
>> @@ -0,0 +1,337 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Wifi Frequency Band Manage Interface
>> + * 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: WBRF supported.
>> + * 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
>> +
>> +#define WBRF_REVISION		0x1
>> +
>> +/*
>> + * The data structure used for WBRF_RETRIEVE is not naturally aligned.
>> + * And unfortunately the design has been settled down.
>> + */
>> +struct amd_wbrf_ranges_out {
>> +	u32			num_of_ranges;
>> +	struct freq_band_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);
>> +
>> +/*
>> + * Used to notify consumer (amdgpu driver currently) about
>> + * the wifi frequency is change.
>> + */
>> +static BLOCKING_NOTIFIER_HEAD(wbrf_chain_head);
>> +
>> +static int wbrf_record(struct acpi_device *adev, uint8_t action, struct wbrf_ranges_in_out *in)
>> +{
>> +	union acpi_object argv4;
>> +	union acpi_object *tmp;
>> +	union acpi_object *obj;
>> +	u32 num_of_ranges = 0;
>> +	u32 num_of_elements;
>> +	u32 arg_idx = 0;
>> +	int ret;
>> +	u32 i;
>> +
>> +	if (!in)
>> +		return -EINVAL;
>> +
>> +	for (i = 0; i < ARRAY_SIZE(in->band_list); i++) {
>> +		if (in->band_list[i].start && in->band_list[i].end)
>> +			num_of_ranges++;
>> +	}
>> +
>> +	/*
>> +	 * The num_of_ranges value in the "in" object supplied by
>> +	 * the caller is required to be equal to the number of
>> +	 * entries in the band_list array in there.
>> +	 */
>> +	if (num_of_ranges != in->num_of_ranges)
>> +		return -EINVAL;
>> +
>> +	/*
>> +	 * Every input frequency band comes with two end points(start/end)
>> +	 * and each 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 + 2;
>> +
>> +	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;
>> +
>> +	/* save the number of ranges*/
>> +	tmp[0].integer.type = ACPI_TYPE_INTEGER;
>> +	tmp[0].integer.value = num_of_ranges;
>> +
>> +	/* save the action(WBRF_RECORD_ADD/REMOVE/RETRIEVE) */
>> +	tmp[1].integer.type = ACPI_TYPE_INTEGER;
>> +	tmp[1].integer.value = action;
>> +
>> +	arg_idx = 2;
>> +	for (i = 0; i < ARRAY_SIZE(in->band_list); i++) {
>> +		if (!in->band_list[i].start || !in->band_list[i].end)
>> +			continue;
>> +
>> +		tmp[arg_idx].integer.type = ACPI_TYPE_INTEGER;
>> +		tmp[arg_idx++].integer.value = in->band_list[i].start;
>> +		tmp[arg_idx].integer.type = ACPI_TYPE_INTEGER;
>> +		tmp[arg_idx++].integer.value = in->band_list[i].end;
>> +	}
>> +
>> +	obj = acpi_evaluate_dsm(adev->handle, &wifi_acpi_dsm_guid,
>> +				WBRF_REVISION, WBRF_RECORD, &argv4);
>> +
>> +	if (!obj)
>> +		return -EINVAL;
>> +
>> +	if (obj->type != ACPI_TYPE_INTEGER) {
>> +		ret = -EINVAL;
>> +		goto out;
>> +	}
>> +
>> +	ret = obj->integer.value;
>> +	if (ret)
>> +		ret = -EINVAL;
>> +
>> +out:
>> +	ACPI_FREE(obj);
>> +	kfree(tmp);
>> +
>> +	return ret;
>> +}
>> +
>> +/**
>> + * acpi_amd_wbrf_add_remove - add or remove the frequency band the device is using
>> + *
>> + * @dev: device pointer
>> + * @action: remove or add the frequency band into bios
>> + * @in: input structure containing the frequency band the device is using
>> + *
>> + * Broadcast to other consumers the frequency band the device starts
>> + * to use. Underneath the surface the information is cached into an
>> + * internal buffer first. Then a notification is sent to all those
>> + * registered consumers. So then they can retrieve that buffer to
>> + * know the latest active frequency bands. Consumers that haven't
>> + * yet been registered can retrieve the information from the cache
>> + * when they register.
>> + *
>> + * Return:
>> + * 0 for success add/remove wifi frequency band.
>> + * Returns a negative error code for failure.
>> + */
>> +int acpi_amd_wbrf_add_remove(struct device *dev, uint8_t action, struct wbrf_ranges_in_out *in)
>> +{
>> +	struct acpi_device *adev;
>> +	int ret;
>> +
>> +	adev = ACPI_COMPANION(dev);
>> +	if (!adev)
>> +		return -ENODEV;
>> +
>> +	ret = wbrf_record(adev, action, in);
>> +	if (ret)
>> +		return ret;
>> +
>> +	blocking_notifier_call_chain(&wbrf_chain_head, WBRF_CHANGED, NULL);
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(acpi_amd_wbrf_add_remove);
>> +
>> +static 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);
>> +}
>> +
>> +/**
>> + * acpi_amd_wbrf_supported_producer - determine if the WBRF can be enabled
>> + *                                    for the device as a producer
>> + *
>> + * @dev: device pointer
>> + *
>> + * Check if the platform equipped with necessary implementations to
>> + * support WBRF for the device as a producer.
>> + *
>> + * Return:
>> + * true if WBRF is supported, otherwise returns false
>> + */
>> +bool acpi_amd_wbrf_supported_producer(struct device *dev)
>> +{
>> +	struct acpi_device *adev;
>> +
>> +	adev = ACPI_COMPANION(dev);
>> +	if (!adev)
>> +		return false;
>> +
>> +	if (!acpi_amd_wbrf_supported_system())
>> +		return false;
>> +
>> +
>> +	return acpi_check_dsm(adev->handle, &wifi_acpi_dsm_guid,
>> +			      WBRF_REVISION, BIT(WBRF_RECORD));
>> +}
>> +EXPORT_SYMBOL_GPL(acpi_amd_wbrf_supported_producer);
>> +
>> +/**
>> + * acpi_amd_wbrf_supported_consumer - determine if the WBRF can be enabled
>> + *                                    for the device as a consumer
>> + *
>> + * @dev: device pointer
>> + *
>> + * Determine if the platform equipped with necessary implementations to
>> + * support WBRF for the device as a consumer.
>> + *
>> + * Return:
>> + * true if WBRF is supported, otherwise returns false.
>> + */
>> +bool acpi_amd_wbrf_supported_consumer(struct device *dev)
>> +{
>> +	struct acpi_device *adev;
>> +
>> +	adev = ACPI_COMPANION(dev);
>> +	if (!adev)
>> +		return false;
>> +
>> +	if (!acpi_amd_wbrf_supported_system())
>> +		return false;
>> +
>> +	return acpi_check_dsm(adev->handle, &wifi_acpi_dsm_guid,
>> +			      WBRF_REVISION, BIT(WBRF_RETRIEVE));
>> +}
>> +EXPORT_SYMBOL_GPL(acpi_amd_wbrf_supported_consumer);
>> +
>> +/**
>> + * amd_wbrf_retrieve_freq_band - retrieve current active frequency
>> + *                                     bands
>> + *
>> + * @dev: device pointer
>> + * @out: output structure containing all the active frequency bands
>> + *
>> + * Retrieve the current active frequency bands which were broadcasted
>> + * by other producers. The consumer who calls this API should take
>> + * proper actions if any of the frequency band may cause RFI with its
>> + * own frequency band used.
>> + *
>> + * Return:
>> + * 0 for getting wifi freq band successfully.
>> + * Returns a negative error code for failure.
>> + */
>> +int amd_wbrf_retrieve_freq_band(struct device *dev, struct wbrf_ranges_in_out *out)
>> +{
>> +	struct amd_wbrf_ranges_out acpi_out = {0};
>> +	struct acpi_device *adev;
>> +	union acpi_object *obj;
>> +	union acpi_object param;
>> +	int ret = 0;
>> +
>> +	adev = ACPI_COMPANION(dev);
>> +	if (!adev)
>> +		return -ENODEV;
>> +
>> +	param.type = ACPI_TYPE_STRING;
>> +	param.string.length = 0;
>> +	param.string.pointer = NULL;
>> +
>> +	obj = acpi_evaluate_dsm(adev->handle, &wifi_acpi_dsm_guid,
>> +							WBRF_REVISION, WBRF_RETRIEVE, &param);
>> +	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, "Wrong sized WBRT information");
>> +		ret = -EINVAL;
>> +		goto out;
>> +	}
>> +	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));
>> +
>> +out:
>> +	ACPI_FREE(obj);
>> +	return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(amd_wbrf_retrieve_freq_band);
>> +
>> +/**
>> + * amd_wbrf_register_notifier - register for notifications of frequency
>> + *                                   band update
>> + *
>> + * @nb: driver notifier block
>> + *
>> + * The consumer should register itself via this API so that it can get
>> + * notified on the frequency band updates from other producers.
>> + *
>> + * Return:
>> + * 0 for registering a consumer driver successfully.
>> + * Returns a negative error code for failure.
>> + */
>> +int amd_wbrf_register_notifier(struct notifier_block *nb)
>> +{
>> +	return blocking_notifier_chain_register(&wbrf_chain_head, nb);
>> +}
>> +EXPORT_SYMBOL_GPL(amd_wbrf_register_notifier);
>> +
>> +/**
>> + * amd_wbrf_unregister_notifier - unregister for notifications of
>> + *                                     frequency band update
>> + *
>> + * @nb: driver notifier block
>> + *
>> + * The consumer should call this API when it is longer interested with
>> + * the frequency band updates from other producers. Usually, this should
>> + * be performed during driver cleanup.
>> + *
>> + * Return:
>> + * 0 for unregistering a consumer driver.
>> + * Returns a negative error code for failure.
>> + */
>> +int amd_wbrf_unregister_notifier(struct notifier_block *nb)
>> +{
>> +	return blocking_notifier_chain_unregister(&wbrf_chain_head, nb);
>> +}
>> +EXPORT_SYMBOL_GPL(amd_wbrf_unregister_notifier);
>> diff --git a/include/linux/acpi_amd_wbrf.h b/include/linux/acpi_amd_wbrf.h
>> new file mode 100644
>> index 000000000000..15b54734a080
>> --- /dev/null
>> +++ b/include/linux/acpi_amd_wbrf.h
>> @@ -0,0 +1,94 @@
>> +/* 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/device.h>
>> +#include <linux/notifier.h>
>> +
>> +/* The maximum number of frequency band ranges */
>> +#define MAX_NUM_OF_WBRF_RANGES		11
>> +
>> +/* Record actions */
>> +#define WBRF_RECORD_ADD		0x0
>> +#define WBRF_RECORD_REMOVE	0x1
>> +
>> +/**
>> + * struct freq_band_range - Wifi frequency band range definition
>> + * @start: start frequency point (in Hz)
>> + * @end: end frequency point (in Hz)
>> + */
>> +struct freq_band_range {
>> +	u64		start;
>> +	u64		end;
>> +};
>> +
>> +/**
>> + * struct wbrf_ranges_in_out - wbrf ranges info
>> + * @num_of_ranges: total number of band ranges in this struct
>> + * @band_list: array of Wifi band ranges
>> + */
>> +struct wbrf_ranges_in_out {
>> +	u64			num_of_ranges;
>> +	struct freq_band_range	band_list[MAX_NUM_OF_WBRF_RANGES];
>> +};
>> +
>> +/**
>> + * enum wbrf_notifier_actions - wbrf notifier actions index
>> + * @WBRF_CHANGED: there was some frequency band updates. The consumers
>> + *               should retrieve the latest active frequency bands.
>> + */
>> +enum wbrf_notifier_actions {
>> +	WBRF_CHANGED,
>> +};
>> +
>> +#if IS_ENABLED(CONFIG_AMD_WBRF)
>> +bool acpi_amd_wbrf_supported_producer(struct device *dev);
>> +int acpi_amd_wbrf_add_remove(struct device *dev, uint8_t action, struct wbrf_ranges_in_out *in);
>> +bool acpi_amd_wbrf_supported_consumer(struct device *dev);
>> +int amd_wbrf_retrieve_freq_band(struct device *dev, struct wbrf_ranges_in_out *out);
>> +int amd_wbrf_register_notifier(struct notifier_block *nb);
>> +int amd_wbrf_unregister_notifier(struct notifier_block *nb);
>> +#else
>> +static inline
>> +bool acpi_amd_wbrf_supported_consumer(struct device *dev)
>> +{
>> +	return false;
>> +}
>> +static inline
>> +int acpi_amd_wbrf_remove_exclusion(struct device *dev, struct wbrf_ranges_in_out *in)
>> +{
>> +	return -ENODEV;
>> +}
>> +static inline
>> +int acpi_amd_wbrf_add_exclusion(struct device *dev, struct wbrf_ranges_in_out *in)
>> +{
>> +	return -ENODEV;
>> +}
>> +static inline
>> +bool acpi_amd_wbrf_supported_producer(struct device *dev)
>> +{
>> +	return false;
>> +}
>> +static inline
>> +int amd_wbrf_retrieve_freq_band(struct device *dev, struct wbrf_ranges_in_out *out)
>> +{
>> +	return -ENODEV;
>> +}
>> +static inline
>> +int amd_wbrf_register_notifier(struct notifier_block *nb)
>> +{
>> +	return -ENODEV;
>> +}
>> +static inline
>> +int amd_wbrf_unregister_notifier(struct notifier_block *nb)
>> +{
>> +	return -ENODEV;
>> +}
>> +#endif /* CONFIG_AMD_WBRF */
>> +
>> +#endif /* _ACPI_AMD_WBRF_H */
>
diff mbox series

Patch

diff --git a/drivers/platform/x86/amd/Kconfig b/drivers/platform/x86/amd/Kconfig
index 55f3a2fc6aec..ac2b7758a04f 100644
--- a/drivers/platform/x86/amd/Kconfig
+++ b/drivers/platform/x86/amd/Kconfig
@@ -18,3 +18,18 @@  config AMD_HSMP
 
 	  If you choose to compile this driver as a module the module will be
 	  called amd_hsmp.
+
+config AMD_WBRF
+	bool "AMD Wifi RF Band mitigations (WBRF)"
+	depends on ACPI
+	default n
+	help
+	  WBRF(Wifi Band RFI mitigation) mechanism allows Wifi drivers
+	  to notify the frequencies they are using so that other hardware
+	  can be reconfigured to avoid harmonic conflicts.
+
+	  AMD provides an ACPI based mechanism to support WBRF on platform with
+	  appropriate underlying support.
+
+	  This mechanism will only be activated on platforms that advertise a
+	  need for it.
diff --git a/drivers/platform/x86/amd/Makefile b/drivers/platform/x86/amd/Makefile
index f04932b7a7d1..dcec0a46f8af 100644
--- a/drivers/platform/x86/amd/Makefile
+++ b/drivers/platform/x86/amd/Makefile
@@ -8,3 +8,4 @@  obj-$(CONFIG_AMD_PMC)		+= pmc/
 amd_hsmp-y			:= hsmp.o
 obj-$(CONFIG_AMD_HSMP)		+= amd_hsmp.o
 obj-$(CONFIG_AMD_PMF)		+= pmf/
+obj-$(CONFIG_AMD_WBRF)		+= wbrf.o
diff --git a/drivers/platform/x86/amd/wbrf.c b/drivers/platform/x86/amd/wbrf.c
new file mode 100644
index 000000000000..36e90a1159be
--- /dev/null
+++ b/drivers/platform/x86/amd/wbrf.c
@@ -0,0 +1,337 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Wifi Frequency Band Manage Interface
+ * 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: WBRF supported.
+ * 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
+
+#define WBRF_REVISION		0x1
+
+/*
+ * The data structure used for WBRF_RETRIEVE is not naturally aligned.
+ * And unfortunately the design has been settled down.
+ */
+struct amd_wbrf_ranges_out {
+	u32			num_of_ranges;
+	struct freq_band_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);
+
+/*
+ * Used to notify consumer (amdgpu driver currently) about
+ * the wifi frequency is change.
+ */
+static BLOCKING_NOTIFIER_HEAD(wbrf_chain_head);
+
+static int wbrf_record(struct acpi_device *adev, uint8_t action, struct wbrf_ranges_in_out *in)
+{
+	union acpi_object argv4;
+	union acpi_object *tmp;
+	union acpi_object *obj;
+	u32 num_of_ranges = 0;
+	u32 num_of_elements;
+	u32 arg_idx = 0;
+	int ret;
+	u32 i;
+
+	if (!in)
+		return -EINVAL;
+
+	for (i = 0; i < ARRAY_SIZE(in->band_list); i++) {
+		if (in->band_list[i].start && in->band_list[i].end)
+			num_of_ranges++;
+	}
+
+	/*
+	 * The num_of_ranges value in the "in" object supplied by
+	 * the caller is required to be equal to the number of
+	 * entries in the band_list array in there.
+	 */
+	if (num_of_ranges != in->num_of_ranges)
+		return -EINVAL;
+
+	/*
+	 * Every input frequency band comes with two end points(start/end)
+	 * and each 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 + 2;
+
+	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;
+
+	/* save the number of ranges*/
+	tmp[0].integer.type = ACPI_TYPE_INTEGER;
+	tmp[0].integer.value = num_of_ranges;
+
+	/* save the action(WBRF_RECORD_ADD/REMOVE/RETRIEVE) */
+	tmp[1].integer.type = ACPI_TYPE_INTEGER;
+	tmp[1].integer.value = action;
+
+	arg_idx = 2;
+	for (i = 0; i < ARRAY_SIZE(in->band_list); i++) {
+		if (!in->band_list[i].start || !in->band_list[i].end)
+			continue;
+
+		tmp[arg_idx].integer.type = ACPI_TYPE_INTEGER;
+		tmp[arg_idx++].integer.value = in->band_list[i].start;
+		tmp[arg_idx].integer.type = ACPI_TYPE_INTEGER;
+		tmp[arg_idx++].integer.value = in->band_list[i].end;
+	}
+
+	obj = acpi_evaluate_dsm(adev->handle, &wifi_acpi_dsm_guid,
+				WBRF_REVISION, WBRF_RECORD, &argv4);
+
+	if (!obj)
+		return -EINVAL;
+
+	if (obj->type != ACPI_TYPE_INTEGER) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	ret = obj->integer.value;
+	if (ret)
+		ret = -EINVAL;
+
+out:
+	ACPI_FREE(obj);
+	kfree(tmp);
+
+	return ret;
+}
+
+/**
+ * acpi_amd_wbrf_add_remove - add or remove the frequency band the device is using
+ *
+ * @dev: device pointer
+ * @action: remove or add the frequency band into bios
+ * @in: input structure containing the frequency band the device is using
+ *
+ * Broadcast to other consumers the frequency band the device starts
+ * to use. Underneath the surface the information is cached into an
+ * internal buffer first. Then a notification is sent to all those
+ * registered consumers. So then they can retrieve that buffer to
+ * know the latest active frequency bands. Consumers that haven't
+ * yet been registered can retrieve the information from the cache
+ * when they register.
+ *
+ * Return:
+ * 0 for success add/remove wifi frequency band.
+ * Returns a negative error code for failure.
+ */
+int acpi_amd_wbrf_add_remove(struct device *dev, uint8_t action, struct wbrf_ranges_in_out *in)
+{
+	struct acpi_device *adev;
+	int ret;
+
+	adev = ACPI_COMPANION(dev);
+	if (!adev)
+		return -ENODEV;
+
+	ret = wbrf_record(adev, action, in);
+	if (ret)
+		return ret;
+
+	blocking_notifier_call_chain(&wbrf_chain_head, WBRF_CHANGED, NULL);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(acpi_amd_wbrf_add_remove);
+
+static 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);
+}
+
+/**
+ * acpi_amd_wbrf_supported_producer - determine if the WBRF can be enabled
+ *                                    for the device as a producer
+ *
+ * @dev: device pointer
+ *
+ * Check if the platform equipped with necessary implementations to
+ * support WBRF for the device as a producer.
+ *
+ * Return:
+ * true if WBRF is supported, otherwise returns false
+ */
+bool acpi_amd_wbrf_supported_producer(struct device *dev)
+{
+	struct acpi_device *adev;
+
+	adev = ACPI_COMPANION(dev);
+	if (!adev)
+		return false;
+
+	if (!acpi_amd_wbrf_supported_system())
+		return false;
+
+
+	return acpi_check_dsm(adev->handle, &wifi_acpi_dsm_guid,
+			      WBRF_REVISION, BIT(WBRF_RECORD));
+}
+EXPORT_SYMBOL_GPL(acpi_amd_wbrf_supported_producer);
+
+/**
+ * acpi_amd_wbrf_supported_consumer - determine if the WBRF can be enabled
+ *                                    for the device as a consumer
+ *
+ * @dev: device pointer
+ *
+ * Determine if the platform equipped with necessary implementations to
+ * support WBRF for the device as a consumer.
+ *
+ * Return:
+ * true if WBRF is supported, otherwise returns false.
+ */
+bool acpi_amd_wbrf_supported_consumer(struct device *dev)
+{
+	struct acpi_device *adev;
+
+	adev = ACPI_COMPANION(dev);
+	if (!adev)
+		return false;
+
+	if (!acpi_amd_wbrf_supported_system())
+		return false;
+
+	return acpi_check_dsm(adev->handle, &wifi_acpi_dsm_guid,
+			      WBRF_REVISION, BIT(WBRF_RETRIEVE));
+}
+EXPORT_SYMBOL_GPL(acpi_amd_wbrf_supported_consumer);
+
+/**
+ * amd_wbrf_retrieve_freq_band - retrieve current active frequency
+ *                                     bands
+ *
+ * @dev: device pointer
+ * @out: output structure containing all the active frequency bands
+ *
+ * Retrieve the current active frequency bands which were broadcasted
+ * by other producers. The consumer who calls this API should take
+ * proper actions if any of the frequency band may cause RFI with its
+ * own frequency band used.
+ *
+ * Return:
+ * 0 for getting wifi freq band successfully.
+ * Returns a negative error code for failure.
+ */
+int amd_wbrf_retrieve_freq_band(struct device *dev, struct wbrf_ranges_in_out *out)
+{
+	struct amd_wbrf_ranges_out acpi_out = {0};
+	struct acpi_device *adev;
+	union acpi_object *obj;
+	union acpi_object param;
+	int ret = 0;
+
+	adev = ACPI_COMPANION(dev);
+	if (!adev)
+		return -ENODEV;
+
+	param.type = ACPI_TYPE_STRING;
+	param.string.length = 0;
+	param.string.pointer = NULL;
+
+	obj = acpi_evaluate_dsm(adev->handle, &wifi_acpi_dsm_guid,
+							WBRF_REVISION, WBRF_RETRIEVE, &param);
+	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, "Wrong sized WBRT information");
+		ret = -EINVAL;
+		goto out;
+	}
+	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));
+
+out:
+	ACPI_FREE(obj);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(amd_wbrf_retrieve_freq_band);
+
+/**
+ * amd_wbrf_register_notifier - register for notifications of frequency
+ *                                   band update
+ *
+ * @nb: driver notifier block
+ *
+ * The consumer should register itself via this API so that it can get
+ * notified on the frequency band updates from other producers.
+ *
+ * Return:
+ * 0 for registering a consumer driver successfully.
+ * Returns a negative error code for failure.
+ */
+int amd_wbrf_register_notifier(struct notifier_block *nb)
+{
+	return blocking_notifier_chain_register(&wbrf_chain_head, nb);
+}
+EXPORT_SYMBOL_GPL(amd_wbrf_register_notifier);
+
+/**
+ * amd_wbrf_unregister_notifier - unregister for notifications of
+ *                                     frequency band update
+ *
+ * @nb: driver notifier block
+ *
+ * The consumer should call this API when it is longer interested with
+ * the frequency band updates from other producers. Usually, this should
+ * be performed during driver cleanup.
+ *
+ * Return:
+ * 0 for unregistering a consumer driver.
+ * Returns a negative error code for failure.
+ */
+int amd_wbrf_unregister_notifier(struct notifier_block *nb)
+{
+	return blocking_notifier_chain_unregister(&wbrf_chain_head, nb);
+}
+EXPORT_SYMBOL_GPL(amd_wbrf_unregister_notifier);
diff --git a/include/linux/acpi_amd_wbrf.h b/include/linux/acpi_amd_wbrf.h
new file mode 100644
index 000000000000..15b54734a080
--- /dev/null
+++ b/include/linux/acpi_amd_wbrf.h
@@ -0,0 +1,94 @@ 
+/* 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/device.h>
+#include <linux/notifier.h>
+
+/* The maximum number of frequency band ranges */
+#define MAX_NUM_OF_WBRF_RANGES		11
+
+/* Record actions */
+#define WBRF_RECORD_ADD		0x0
+#define WBRF_RECORD_REMOVE	0x1
+
+/**
+ * struct freq_band_range - Wifi frequency band range definition
+ * @start: start frequency point (in Hz)
+ * @end: end frequency point (in Hz)
+ */
+struct freq_band_range {
+	u64		start;
+	u64		end;
+};
+
+/**
+ * struct wbrf_ranges_in_out - wbrf ranges info
+ * @num_of_ranges: total number of band ranges in this struct
+ * @band_list: array of Wifi band ranges
+ */
+struct wbrf_ranges_in_out {
+	u64			num_of_ranges;
+	struct freq_band_range	band_list[MAX_NUM_OF_WBRF_RANGES];
+};
+
+/**
+ * enum wbrf_notifier_actions - wbrf notifier actions index
+ * @WBRF_CHANGED: there was some frequency band updates. The consumers
+ *               should retrieve the latest active frequency bands.
+ */
+enum wbrf_notifier_actions {
+	WBRF_CHANGED,
+};
+
+#if IS_ENABLED(CONFIG_AMD_WBRF)
+bool acpi_amd_wbrf_supported_producer(struct device *dev);
+int acpi_amd_wbrf_add_remove(struct device *dev, uint8_t action, struct wbrf_ranges_in_out *in);
+bool acpi_amd_wbrf_supported_consumer(struct device *dev);
+int amd_wbrf_retrieve_freq_band(struct device *dev, struct wbrf_ranges_in_out *out);
+int amd_wbrf_register_notifier(struct notifier_block *nb);
+int amd_wbrf_unregister_notifier(struct notifier_block *nb);
+#else
+static inline
+bool acpi_amd_wbrf_supported_consumer(struct device *dev)
+{
+	return false;
+}
+static inline
+int acpi_amd_wbrf_remove_exclusion(struct device *dev, struct wbrf_ranges_in_out *in)
+{
+	return -ENODEV;
+}
+static inline
+int acpi_amd_wbrf_add_exclusion(struct device *dev, struct wbrf_ranges_in_out *in)
+{
+	return -ENODEV;
+}
+static inline
+bool acpi_amd_wbrf_supported_producer(struct device *dev)
+{
+	return false;
+}
+static inline
+int amd_wbrf_retrieve_freq_band(struct device *dev, struct wbrf_ranges_in_out *out)
+{
+	return -ENODEV;
+}
+static inline
+int amd_wbrf_register_notifier(struct notifier_block *nb)
+{
+	return -ENODEV;
+}
+static inline
+int amd_wbrf_unregister_notifier(struct notifier_block *nb)
+{
+	return -ENODEV;
+}
+#endif /* CONFIG_AMD_WBRF */
+
+#endif /* _ACPI_AMD_WBRF_H */