On 6/23/2023 11:28 AM, Rafael J. Wysocki wrote:
On Fri, Jun 23, 2023 at 5:57 PM Limonciello, Mario
<mario.limonciello@xxxxxxx> wrote:
On 6/23/2023 9:52 AM, Rafael J. Wysocki wrote:
On Wed, Jun 21, 2023 at 7:47 AM Evan Quan <evan.quan@xxxxxxx> 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.
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
The prefix should be acpi_wbrf_ or acpi_amd_wbrf_ even, so it is clear
that this uses ACPI and is AMD-specific.
I guess if we end up with an intermediary library approach
wbrf_supported_producer makes sense and that could call acpi_wbrf_*.
But with no intermediate library your suggestion makes sense.
I would prefer not to make it acpi_amd as there is no reason that
this exact same problem couldn't happen on an
Wifi 6e + Intel SOC + AMD dGPU design too and OEMs could use the
same mitigation mechanism as Wifi6e + AMD SOC + AMD dGPU too.
The mitigation mechanism might be the same, but the AML interface very
well may be different.
Right. I suppose right now we should keep it prefixed as "amd",
and if it later is promoted as a standard it can be renamed.
My point is that this particular interface is AMD-specific ATM and I'm
not aware of any plans to make it "standard" in some way.
Yeah; this implementation is currently AMD specific AML, but I
expect the exact same AML would be delivered to OEMs using the
dGPUs.
Also if the given interface is specified somewhere, it would be good
to have a pointer to that place.
It's a code first implementation. I'm discussing with the
owners when they will release it.
Whether or not there needs to be an intermediate library wrapped
around this is a different matter.
IMO individual drivers should not be expected to use this interface
directly, as that would add to boilerplate code and overall bloat.
The thing is the ACPI method is not a platform method. It's
a function of the device (_DSM).
The reason for having acpi_wbrf.c in the first place is to
avoid the boilerplate of the _DSM implementation across multiple
drivers.
Also whoever uses it, would first need to check if the device in
question has an ACPI companion.
Which comes back to Andrew's point.
Either we:
Have a generic wbrf_ helper that takes struct *device and
internally checks if there is an ACPI companion and support.
or
Do the check for support in mac80211 + applicable drivers
and only call the AMD WBRF ACPI method in those drivers in
those cases.
