[PATCH V5 1/9] drivers core: Add support for Wifi band RF mitigations

[Mario Limonciello]

On 7/5/23 21:58, Quan, Evan wrote:
> [AMD Official Use Only - General]
> 
> Hi Andrew,
> 
> I discussed with Mario about your proposal/concerns here.
> We believe some changes below might address your concerns.
> - place/move the wbrf_supported_producer check inside acpi_amd_wbrf_add_exclusion and acpi_amd_wbrf_add_exclusion
> - place the wbrf_supported_consumer check inside acpi_amd_wbrf_retrieve_exclusions
> So that the wbrf_supported_producer and wbrf_supported_consumer can be dropped.
> We made some prototypes and even performed some tests which showed technically it is absolutely practicable.
> 
> However, we found several issues with that.
> - The overhead caused by the extra _producer/_consumer check on every calling of wbrf_add/remove/retrieve_ecxclusion.
>    Especially when you consider there might be multiple producers and consumers in the system at the same time. And some of
>    them might do in-use band/frequency switching frequently.

One more piece of overhead that is in this same theme that Evan didn't 
mention is the case of a system "without" AMD's ACPI WBRF but the kernel 
was configured with it enabled.  Think like a distro kernel.

Moving it into add/remove exclusion would mean that every single time 
frequency changed by a producer the _DSM would attempt to be evaluated 
and fail.  To avoid that extra call overhead after the first time would 
mean needing to keep a variable somewhere, and at that point what did 
you save?

> - Some extra costs caused by the "know it only at the last minute". For example, to support WBRF, amdgpu driver needs some preparations: install the notification hander,
>    setup the delay workqueue(to handle possible events flooding) and even notify firmware engine to be ready. However, only on the 1st notification receiving,
>    it is realized(reported by wbrf_supported_consumer check) the WBRF feature is actually not supported. All those extra costs can be actually avoided if we can know the WBRF is not supported at first.
>    This could happen to other consumers and producers too.
> 
> After a careful consideration, we think the changes do not benefit us much. It does not deserve us to spend extra efforts.
> Thus we would like to stick with original implementations. That is to have wbrf_supported_producer and wbrf_supported_consumer interfaces exposed.
> Then other drivers/subsystems can do necessary wbrf support check in advance and coordinate their actions accordingly.
> Please let us know your thoughts.
> 
> BR,
> Evan
>> -----Original Message-----
>> From: Andrew Lunn <andrew at lunn.ch>
>> Sent: Tuesday, July 4, 2023 9:07 PM
>> To: Quan, Evan <Evan.Quan at amd.com>
>> Cc: rafael at kernel.org; lenb at kernel.org; Deucher, Alexander
>> <Alexander.Deucher at amd.com>; Koenig, Christian
>> <Christian.Koenig at amd.com>; Pan, Xinhui <Xinhui.Pan at amd.com>;
>> airlied at gmail.com; daniel at ffwll.ch; johannes at sipsolutions.net;
>> davem at davemloft.net; edumazet at google.com; kuba at kernel.org;
>> pabeni at redhat.com; Limonciello, Mario <Mario.Limonciello at amd.com>;
>> mdaenzer at redhat.com; maarten.lankhorst at linux.intel.com;
>> tzimmermann at suse.de; hdegoede at redhat.com; jingyuwang_vip at 163.com;
>> Lazar, Lijo <Lijo.Lazar at amd.com>; jim.cromie at gmail.com;
>> bellosilicio at gmail.com; andrealmeid at igalia.com; trix at redhat.com;
>> jsg at jsg.id.au; arnd at arndb.de; linux-kernel at vger.kernel.org; linux-
>> acpi at vger.kernel.org; amd-gfx at lists.freedesktop.org; dri-
>> devel at lists.freedesktop.org; linux-wireless at vger.kernel.org;
>> netdev at vger.kernel.org
>> Subject: Re: [PATCH V5 1/9] drivers core: Add support for Wifi band RF
>> mitigations
>>
>>>> What is the purpose of this stage? Why would it not be supported for
>>>> this device?
>>> This is needed for wbrf support via ACPI mechanism. If BIOS(AML code)
>>> does not support the wbrf adding/removing for some device, it should
>> speak that out so that the device can be aware of that.
>>
>> How much overhead is this adding? How deep do you need to go to find the
>> BIOS does not support it? And how often is this called?
>>
>> Where do we want to add complexity? In the generic API? Or maybe a little
>> deeper in the ACPI specific code?
>>
>>         Andrew
>