Re: [PATCH V4 1/8] drivers/acpi: Add support for Wifi band RF mitigations

[Limonciello, Mario]

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.