drivers: thermal: do not clobber cooling dev state from userspace
Subsystems (2)
PATCH 1/1 drivers: thermal: do not clobber cooling dev state from userspace
From: Ram Chandrasekar <[email protected]>
Let userspace be another voter for cooling device state instead of the overriding authority. It is possible that the thermal governor may find a higher cooling state desirable than what is recommended by the userspace. Separate out the current cooling device state from the userspace request and aggregate the userspace request with the governors' recommendation.
--- drivers/thermal/thermal_core.c | 1 + drivers/thermal/thermal_helpers.c | 2 +- drivers/thermal/thermal_sysfs.c | 8 +++++++- include/linux/thermal.h | 1 + 4 files changed, 10 insertions(+), 2 deletions(-)
drivers/thermal/thermal_core.c +1 -0
@@ -970,6 +970,7 @@ __thermal_cooling_device_register(struct device_node *np, cdev->updated = false; cdev->device.class = &thermal_class; cdev->devdata = devdata; + cdev->sysfs_req =0 ;thermal_cooling_device_setup_sysfs (cdev);dev_set_name (&cdev->device,"cooling_device%d" , cdev->id); result =device_register (&cdev->device);
drivers/thermal/thermal_helpers.c +1 -1
@@ -166,7 +166,7 @@ EXPORT_SYMBOL_GPL(thermal_zone_set_trips);void thermal_cdev_update (struct thermal_cooling_device *cdev) {struct thermal_instance *instance; -unsigned long target =0 ; +unsigned long target = cdev->sysfs_req;mutex_lock (&cdev->lock);/* cooling device is updated*/
drivers/thermal/thermal_sysfs.c +7 -1
@@ -719,7 +719,13 @@ thermal_cooling_device_cur_state_store(struct device *dev, result = cdev->ops->set_cur_state (cdev, state);if (result)return result; -thermal_cooling_device_stats_update (cdev, state); + + cdev->sysfs_req = state; +mutex_lock (&cdev->lock); + cdev->updated = false; +mutex_unlock (&cdev->lock); +thermal_cdev_update (cdev); +return count; }
include/linux/thermal.h +1 -0
@@ -139,6 +139,7 @@ struct thermal_cooling_device {struct mutex lock;/* protect thermal_instances list */ struct list_head thermal_instances;struct list_head node; +unsigned long sysfs_req; };struct thermal_attr {
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Zhang Rui
reviewerQuoted 11 lines · Lina Iyer
From: Ram Chandrasekar <[email protected]>
Let userspace be another voter for cooling device state instead of the
overriding authority. It is possible that the thermal governor may find
a higher cooling state desirable than what is recommended by the userspace. Separate out the current cooling device state from the userspace request and aggregate the userspace request with the governors' recommendation.
hmmm, I don't understand this.
If the governor does not work well, we should either improve the governor or use userspace governor instead.
do you have any examples that the kernel governor chooses improper cooling state?
thanks,
rui
Quoted 69 lines
Signed-off-by Lina Iyer <[email protected]>---
drivers/thermal/thermal_core.c | 1 +
drivers/thermal/thermal_helpers.c | 2 +-
drivers/thermal/thermal_sysfs.c | 8 +++++++-
include/linux/thermal.h | 1 +4 files changed, 10 insertions(+), 2 deletions(-)drivers/thermal/thermal_core.c +1
-0@@ -970,6 +970,7 @@ __thermal_cooling_device_register(structdevice_node *np, cdev->updated = false; cdev->device.class = &thermal_class; cdev->devdata = devdata; + cdev->sysfs_req =0 ; thermal_cooling_device_setup_sysfs(cdev); dev_set_name(&cdev->device, "cooling_device%d", cdev->id);result =
device_register(&cdev->device);drivers/thermal/thermal_helpers.c +1
-1@@ -166,7 +166,7 @@ EXPORT_SYMBOL_GPL(thermal_zone_set_trips); void thermal_cdev_update(struct thermal_cooling_device *cdev) { struct thermal_instance *instance; -unsigned long target =0 ; +unsigned long target = cdev->sysfs_req;
mutex_lock(&cdev->lock);
/* cooling device is updated*/drivers/thermal/thermal_sysfs.c +7
-1@@ -719,7 +719,13 @@ thermal_cooling_device_cur_state_store(structdevice *dev, result = cdev->ops->set_cur_state(cdev, state); if (result) return result; -thermal_cooling_device_stats_update (cdev, state); + + cdev->sysfs_req = state; +mutex_lock (&cdev->lock); + cdev->updated = false; +mutex_unlock (&cdev->lock); +thermal_cdev_update (cdev); + return count; }include/linux/thermal.h +0
-0@@ -139,6 +139,7 @@ struct thermal_cooling_device {struct mutex lock; /* protect
thermal_instanceslist */ structlist_headthermal_instances;
structlist_headnode;include/linux/thermal.h +1
-0+unsigned long sysfs_req;};
structthermal_attr{
Adding Ram, so he can respond.
-- Lina
Quoted 91 lines · Zhang Rui
Quoted 11 lines · Lina Iyer
From: Ram Chandrasekar <[email protected]>
Let userspace be another voter for cooling device state instead of the
overriding authority. It is possible that the thermal governor may find
a higher cooling state desirable than what is recommended by the userspace. Separate out the current cooling device state from the userspace request and aggregate the userspace request with the governors' recommendation.hmmm, I don't understand this.
If the governor does not work well, we should either improve the governor or use userspace governor instead.
do you have any examples that the kernel governor chooses improper cooling state?thanks,
ruiQuoted 69 lines
Signed-off-by Lina Iyer <[email protected]>---
drivers/thermal/thermal_core.c | 1 +
drivers/thermal/thermal_helpers.c | 2 +-
drivers/thermal/thermal_sysfs.c | 8 +++++++-
include/linux/thermal.h | 1 +4 files changed, 10 insertions(+), 2 deletions(-)drivers/thermal/thermal_core.c +1
-0@@ -970,6 +970,7 @@ __thermal_cooling_device_register(structdevice_node *np, cdev->updated = false; cdev->device.class = &thermal_class; cdev->devdata = devdata; + cdev->sysfs_req =0 ; thermal_cooling_device_setup_sysfs(cdev); dev_set_name(&cdev->device, "cooling_device%d", cdev->id);result =
device_register(&cdev->device);drivers/thermal/thermal_helpers.c +1
-1@@ -166,7 +166,7 @@ EXPORT_SYMBOL_GPL(thermal_zone_set_trips); void thermal_cdev_update(struct thermal_cooling_device *cdev) { struct thermal_instance *instance; -unsigned long target =0 ; +unsigned long target = cdev->sysfs_req;
mutex_lock(&cdev->lock);
/* cooling device is updated*/drivers/thermal/thermal_sysfs.c +7
-1@@ -719,7 +719,13 @@ thermal_cooling_device_cur_state_store(structdevice *dev, result = cdev->ops->set_cur_state(cdev, state); if (result) return result; -thermal_cooling_device_stats_update (cdev, state); + + cdev->sysfs_req = state; +mutex_lock (&cdev->lock); + cdev->updated = false; +mutex_unlock (&cdev->lock); +thermal_cdev_update (cdev); + return count; }include/linux/thermal.h +0
-0@@ -139,6 +139,7 @@ struct thermal_cooling_device {struct mutex lock; /* protect
thermal_instanceslist */ structlist_headthermal_instances;
structlist_headnode;include/linux/thermal.h +1
-0+unsigned long sysfs_req;};
structthermal_attr{
Quoted 23 lines · Lina Iyer
Adding Ram, so he can respond.
-- LinaQuoted 19 lines · Zhang Rui
Quoted 11 lines · Lina Iyer
From: Ram Chandrasekar <[email protected]>
Let userspace be another voter for cooling device state instead of the
overriding authority. It is possible that the thermal governor may find
a higher cooling state desirable than what is recommended by the userspace. Separate out the current cooling device state from the userspace request and aggregate the userspace request with the governors' recommendation.hmmm, I don't understand this.
If the governor does not work well, we should either improve the governor or use userspace governor instead.
do you have any examples that the kernel governor chooses improper cooling state?
Userspace governor and kernel governor could monitor different thermal zones for different thermal conditions and mitigate the same cooling device. In this case, the cooling device state request from userspace governor shouldn’t overwrite the request from kernel governor, rather it needs to be aggregated. This change is to aggregate the cooling device state requests between the userspace and kernel governors.
Thanks,
Ram