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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 6/21/2023 12:26 PM, Andrew Lunn wrote:
I think what you're asking for is another layer of indirection
like CONFIG_WBRF in addition to CONFIG_ACPI_WBRF.

Producers would call functions like wbrf_supported_producer()
where the source file is not guarded behind CONFIG_ACPI_WBRF,
but instead by CONFIG_WBRF and locally use CONFIG_ACPI_WBRF within
it.  So a producer could look like this:

bool wbrf_supported_producer(struct device *dev)
{
#ifdef CONFIG_ACPI_WBRF
     struct acpi_device *adev = ACPI_COMPANION(dev);

     if (adev)
         return check_acpi_wbrf(adev->handle,
                        WBRF_REVISION,
                        1ULL << WBRF_RECORD);
#endif
     return -ENODEV;

}
EXPORT_SYMBOL_GPL(wbrf_supported_producer);

And then adding/removing could look something like this

int wbrf_add_exclusion(struct device *dev,
                struct wbrf_ranges_in *in)
{
#ifdef CONFIG_ACPI_WBRF
     struct acpi_device *adev = ACPI_COMPANION(dev);

     if (adev)
         return wbrf_record(adev, WBRF_RECORD_ADD, in);
#endif
     return -ENODEV;
}
EXPORT_SYMBOL_GPL(wbrf_add_exclusion);

int wbrf_remove_exclusion(struct device *dev,
                struct wbrf_ranges_in *in)
{
#ifdef CONFIG_ACPI_WBRF
     struct acpi_device *adev = ACPI_COMPANION(dev);

     if (adev)
         return wbrf_record(adev, WBRF_RECORD_REMOVE, in);
#endif
     return -ENODEV;
}
EXPORT_SYMBOL_GPL(wbrf_remove_exclusion);
Yes, this looks a lot better.

But what about notifications?
Once you implement this it gets a lot more complex and the driver consumers would need to know more about the kernel's implementation.  For example consumers need a
notifier block like:

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index e3e2e6e3b485..146fe3c43343 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1066,6 +1066,8 @@ struct amdgpu_device {

        bool                            job_hang;
        bool                            dc_enabled;
+
+       struct notifier_block           wbrf_notifier;
 };

 static inline struct amdgpu_device *drm_to_adev(struct drm_device *ddev)

And then would need matching notifier functions like:

static int amdgpu_wbrf_frequencies_notifier(struct notifier_block *nb,
                                    unsigned long action, void *_arg)

And we'd need to set up a chain to be used in this case in the WBRF code:

static BLOCKING_NOTIFIER_HEAD(wbrf_chain_head);

int wbrf_register_notifier(struct notifier_block *nb)
{
    return blocking_notifier_chain_register(&wbrf_chain_head, nb);
}
EXPORT_SYMBOL_GPL(wbrf_register_notifier);

int wbrf_unregister_notifier(struct notifier_block *nb)
{
    return blocking_notifier_chain_unregister(&wbrf_chain_head, nb);
}
EXPORT_SYMBOL_GPL(wbrf_unregister_notifier);

And consumer would need to call it, but only if CONFIG_WBRF_ACPI isn't set.

Add/remove functions can easily call something like:

blocking_notifier_call_chain(&wbrf_chain_head, action, data);

With all of this complexity and (effectively) dead code for ACPI vs non-ACPI
path I really have to ask why wouldn't a non-AMD implementation be able to
do this as ACPI?

I don't see why it couldn't be a DT/ACPI hybrid solution for ARM64.




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux