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

[Quan, Evan]

[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.
- 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@xxxxxxx>
> Sent: Tuesday, July 4, 2023 9:07 PM
> To: Quan, Evan <Evan.Quan@xxxxxxx>
> Cc: rafael@xxxxxxxxxx; lenb@xxxxxxxxxx; Deucher, Alexander
> <Alexander.Deucher@xxxxxxx>; Koenig, Christian
> <Christian.Koenig@xxxxxxx>; Pan, Xinhui <Xinhui.Pan@xxxxxxx>;
> airlied@xxxxxxxxx; daniel@xxxxxxxx; johannes@xxxxxxxxxxxxxxxx;
> davem@xxxxxxxxxxxxx; edumazet@xxxxxxxxxx; kuba@xxxxxxxxxx;
> pabeni@xxxxxxxxxx; Limonciello, Mario <Mario.Limonciello@xxxxxxx>;
> mdaenzer@xxxxxxxxxx; maarten.lankhorst@xxxxxxxxxxxxxxx;
> tzimmermann@xxxxxxx; hdegoede@xxxxxxxxxx; jingyuwang_vip@xxxxxxx;
> Lazar, Lijo <Lijo.Lazar@xxxxxxx>; jim.cromie@xxxxxxxxx;
> bellosilicio@xxxxxxxxx; andrealmeid@xxxxxxxxxx; trix@xxxxxxxxxx;
> jsg@xxxxxxxxx; arnd@xxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux-
> acpi@xxxxxxxxxxxxxxx; amd-gfx@xxxxxxxxxxxxxxxxxxxxx; dri-
> devel@xxxxxxxxxxxxxxxxxxxxx; linux-wireless@xxxxxxxxxxxxxxx;
> netdev@xxxxxxxxxxxxxxx
> 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