Re: [PATCH 3/3] thermal: intel: hfi: Enable interface only when required
From: Ricardo Neri <hidden>
Date: 2024-02-01 01:46:47
Also in:
linux-pm
On Wed, Jan 31, 2024 at 01:05:35PM +0100, Stanislaw Gruszka wrote:
Enable and disable hardware feedback interface (HFI) when user space handler is present. For example, enable HFI, when intel-speed-select or Intel Low Power daemon is running and subscribing to thermal netlink events,. When user space handlers exit or remove subscription for thermal netlink events, disable HFI.
I'd rephrase as: Enable and disable HFI when a user space handler is running and listening to thermal netlink events (e.g., intel-speed-select, Intel Low Power daemon). When the user space handlers exit or remove subscription, disable HFI.
Summary of changes: - When CPU is online, instead of blindly enabling HFI by calling hfi_enable(), check if the thermal netlink group has any listener. This will make sure that HFI is not enabled by default during boot time.
s/by calling hfi_enable()//
- Register a netlink notifier. - In the notifier process reason code NETLINK_CHANGE and
s/notifier/notifier,/
NETLINK_URELEASE. If thermal netlink group has any listener enable HFI on all packages. If there are no listener disable HFI on all packages. - Actual processing to enable/disable matches what is done in suspend/resume callbacks. So, create two functions hfi_do_enable()
s/So,//
quoted hunk ↗ jump to hunk
and hfi_do_disable(), which can be called from the netlink notifier callback and suspend/resume callbacks. Signed-off-by: Stanislaw Gruszka <redacted> --- drivers/thermal/intel/intel_hfi.c | 82 +++++++++++++++++++++++++++---- 1 file changed, 73 insertions(+), 9 deletions(-)diff --git a/drivers/thermal/intel/intel_hfi.c b/drivers/thermal/intel/intel_hfi.c index 3b04c6ec4fca..50601f75f6dc 100644 --- a/drivers/thermal/intel/intel_hfi.c +++ b/drivers/thermal/intel/intel_hfi.c@@ -30,6 +30,7 @@ #include <linux/kernel.h> #include <linux/math.h> #include <linux/mutex.h> +#include <linux/netlink.h> #include <linux/percpu-defs.h> #include <linux/printk.h> #include <linux/processor.h>@@ -480,7 +481,8 @@ void intel_hfi_online(unsigned int cpu) /* Enable this HFI instance if this is its first online CPU. */ if (cpumask_weight(hfi_instance->cpus) == 1) { hfi_set_hw_table(hfi_instance); - hfi_enable(); + if (thermal_group_has_listeners(THERMAL_GENL_EVENT_GROUP)) + hfi_enable();
You could avoid the extra level of indentation if you did: if (cpumask_weight() == 1 && has_listeners()) You would need to also update the comment. My two cents.