On 6/21/2023 10:39 AM, Johannes Berg wrote:
On Wed, 2023-06-21 at 17:36 +0200, Andrew Lunn wrote:
On Wed, Jun 21, 2023 at 01:45:56PM +0800, Evan Quan wrote:
From: Mario Limonciello <mario.limonciello@xxxxxxx>
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 an ACPI based mechanism that
devices can use to notify active use of particular frequencies so
that devices can make relative internal adjustments as necessary
to avoid this resonance.
Do only ACPI based systems have:
interference of relatively high-powered harmonics of the (G-)DDR
memory clocks with local radio module frequency bands used by
Wifi 6/6e/7."
Could Device Tree based systems not experience this problem?
They could, of course, but they'd need some other driver to change
_something_ in the system? I don't even know what this is doing
precisely under the hood in the ACPI BIOS, perhaps it adjusts the DDR
memory clock frequency in response to WiFi using a frequency that will
cause interference with harmonics.
The way that WBRF has been architected, it's intended to be able
to scale to any type of device pair that has harmonic issues.
In the first use (Wifi 6e + specific AMD dGPUs) that matches this
series BIOS has the following purposes:
1) The existence of _DSM indicates that the system may not have
adequate shielding and should be using these mitigations.
2) Notification mechanism of frequency use.
For the first problematic devices we *could* have done notifications
entirely in native Linux kernel code with notifier chains.
However that still means you need a hint from the platform that the
functionality is needed like a _DSD bit.
It's also done this way so that AML could do some of the notifications
directly to applicable devices in the future without needing "consumer"
driver participation.
+/**
+ * APIs needed by drivers/subsystems for contributing frequencies:
+ * During probe, check `wbrf_supported_producer` to see if WBRF is supported.
+ * If adding frequencies, then call `wbrf_add_exclusion` with the
+ * start and end points specified for the frequency ranges added.
+ * If removing frequencies, then call `wbrf_remove_exclusion` with
+ * start and end points specified for the frequency ranges added.
+ */
+bool wbrf_supported_producer(struct acpi_device *adev);
+int wbrf_add_exclusion(struct acpi_device *adev,
+ struct wbrf_ranges_in *in);
+int wbrf_remove_exclusion(struct acpi_device *adev,
+ struct wbrf_ranges_in *in);
Could struct device be used here, to make the API agnostic to where
the information is coming from? That would then allow somebody in the
future to implement a device tree based information provider.
That does make sense, and it wouldn't even be that much harder if we
assume in a given platform there's only one provider - but once you go
beyond that these would need to call function pointers I guess? Though
that could be left for "future improvement" too.
johannes
There's more to it than just sending in the frequency that is
added or removed. The notification path comes from ACPI as well.
This first implementation only has one provider and consumer
but yes, we envision that there could be multiple of each party
and that AML may be the mechanism for some consumers to react.
