Message-ID: <DM6PR12MB2619446782F82D9990773F6CE42AA@DM6PR12MB2619.namprd12.prod.outlook.com>
Date:   Fri, 30 Jun 2023 10:37:20 +0000
From:   "Quan, Evan" <Evan.Quan@....com>
To:     "Rafael J. Wysocki" <rafael@...nel.org>,
        "andrew@...n.ch" <andrew@...n.ch>
CC:     "lenb@...nel.org" <lenb@...nel.org>,
        "Deucher, Alexander" <Alexander.Deucher@....com>,
        "Koenig, Christian" <Christian.Koenig@....com>,
        "Pan, Xinhui" <Xinhui.Pan@....com>,
        "airlied@...il.com" <airlied@...il.com>,
        "daniel@...ll.ch" <daniel@...ll.ch>,
        "johannes@...solutions.net" <johannes@...solutions.net>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "edumazet@...gle.com" <edumazet@...gle.com>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "pabeni@...hat.com" <pabeni@...hat.com>,
        "mdaenzer@...hat.com" <mdaenzer@...hat.com>,
        "maarten.lankhorst@...ux.intel.com" 
        <maarten.lankhorst@...ux.intel.com>,
        "tzimmermann@...e.de" <tzimmermann@...e.de>,
        "hdegoede@...hat.com" <hdegoede@...hat.com>,
        "jingyuwang_vip@....com" <jingyuwang_vip@....com>,
        "Lazar, Lijo" <Lijo.Lazar@....com>,
        "jim.cromie@...il.com" <jim.cromie@...il.com>,
        "bellosilicio@...il.com" <bellosilicio@...il.com>,
        "andrealmeid@...lia.com" <andrealmeid@...lia.com>,
        "trix@...hat.com" <trix@...hat.com>,
        "jsg@....id.au" <jsg@....id.au>, "arnd@...db.de" <arnd@...db.de>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
        "amd-gfx@...ts.freedesktop.org" <amd-gfx@...ts.freedesktop.org>,
        "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
        "linux-wireless@...r.kernel.org" <linux-wireless@...r.kernel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: [PATCH V4 1/8] drivers/acpi: Add support for Wifi band RF
 mitigations

[AMD Official Use Only - General]

Hi Rafael & Andrew,

I just posted a new V5 series based on the discussions here and offline discussions with Mario.
Please share your comments/insights there.

Thanks,
Evan
> -----Original Message-----
> From: Rafael J. Wysocki <rafael@...nel.org>
> Sent: Saturday, June 24, 2023 1:16 AM
> To: Limonciello, Mario <Mario.Limonciello@....com>
> Cc: Rafael J. Wysocki <rafael@...nel.org>; Quan, Evan
> <Evan.Quan@....com>; lenb@...nel.org; Deucher, Alexander
> <Alexander.Deucher@....com>; Koenig, Christian
> <Christian.Koenig@....com>; Pan, Xinhui <Xinhui.Pan@....com>;
> airlied@...il.com; daniel@...ll.ch; johannes@...solutions.net;
> davem@...emloft.net; edumazet@...gle.com; kuba@...nel.org;
> pabeni@...hat.com; mdaenzer@...hat.com;
> maarten.lankhorst@...ux.intel.com; tzimmermann@...e.de;
> hdegoede@...hat.com; jingyuwang_vip@....com; Lazar, Lijo
> <Lijo.Lazar@....com>; jim.cromie@...il.com; bellosilicio@...il.com;
> andrealmeid@...lia.com; trix@...hat.com; jsg@....id.au; arnd@...db.de;
> linux-kernel@...r.kernel.org; linux-acpi@...r.kernel.org; amd-
> gfx@...ts.freedesktop.org; dri-devel@...ts.freedesktop.org; linux-
> wireless@...r.kernel.org; netdev@...r.kernel.org
> Subject: Re: [PATCH V4 1/8] drivers/acpi: Add support for Wifi band RF
> mitigations
>
> On Fri, Jun 23, 2023 at 6:48 PM Limonciello, Mario
> <mario.limonciello@....com> wrote:
> >
> >
> > On 6/23/2023 11:28 AM, Rafael J. Wysocki wrote:
> > > On Fri, Jun 23, 2023 at 5:57 PM Limonciello, Mario
> > > <mario.limonciello@....com> 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@....com>
> wrote:
> > >>>> From: Mario Limonciello <mario.limonciello@....com>
> > >>>>
> > >>>> 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).
>
> _DSM is an interface to the platform like any other AML, so I'm not really sure
> what you mean.
>
> > The reason for having acpi_wbrf.c in the first place is to avoid the
> > boilerplate of the _DSM implementation across multiple drivers.
>
> Absolutely, drivers should not be bothered with having to use _DSM in any
> case.  However, they may not even realize that they are running on a system
> using ACPI and I'm not sure if they really should care.
>
> > >
> > > 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.
>
> Either of the above has problems IMO.
>
> The problem with the wbrf_ helper approach is that it adds
> (potentially) several pieces of interaction with the platform, potentially for
> every driver, in places where drivers don't do such things as a rule.
>
> The problem with the other approach is that the drivers in question now need
> to be aware of ACPI in general and the AMD WBRF interface in particular and if
> other similar interfaces are added by other vendors, they will have to learn
> about those as well.
>
> I think that we need to start over with a general problem statement that in
> some cases the platform needs to be consulted regarding radio frequencies
> that drivers would like to use, because it may need to adjust or simply say
> which ranges are "noisy" (or even completely unusable for that matter).  That
> should allow us to figure out how the interface should look like from the driver
> side and it should be possible to hook up the existing platform interface to
> that.

Powered by blists - more mailing lists