LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] drivers: thermal: do not clobber cooling dev state from userspace
@ 2018-05-07 17:55 Lina Iyer
  2018-07-26  8:55 ` Zhang Rui
  0 siblings, 1 reply; 4+ messages in thread
From: Lina Iyer @ 2018-05-07 17:55 UTC (permalink / raw)
  To: edubezval, rui.zhang; +Cc: linux-pm, linux-kernel, linux-arm-msm, Lina Iyer

From: Ram Chandrasekar <rkumbako@codeaurora.org>

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.

Signed-off-by: Lina Iyer <ilina@codeaurora.org>
---
 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(-)

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 589925ac0994..c2e13e122934 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -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);
diff --git a/drivers/thermal/thermal_helpers.c b/drivers/thermal/thermal_helpers.c
index 2ba756af76b7..f550fdee0f9b 100644
--- a/drivers/thermal/thermal_helpers.c
+++ b/drivers/thermal/thermal_helpers.c
@@ -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*/
diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c
index 275ffee292bf..eddada715ad2 100644
--- a/drivers/thermal/thermal_sysfs.c
+++ b/drivers/thermal/thermal_sysfs.c
@@ -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;
 }
 
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 5f4705f46c2f..7a133bd6ff86 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -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 {
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] drivers: thermal: do not clobber cooling dev state from userspace
  2018-05-07 17:55 [PATCH] drivers: thermal: do not clobber cooling dev state from userspace Lina Iyer
@ 2018-07-26  8:55 ` Zhang Rui
  2018-08-14 18:04   ` Lina Iyer
  0 siblings, 1 reply; 4+ messages in thread
From: Zhang Rui @ 2018-07-26  8:55 UTC (permalink / raw)
  To: Lina Iyer, edubezval; +Cc: linux-pm, linux-kernel, linux-arm-msm

On 一, 2018-05-07 at 11:55 -0600, Lina Iyer wrote:
> From: Ram Chandrasekar <rkumbako@codeaurora.org>
> 
> 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

> Signed-off-by: Lina Iyer <ilina@codeaurora.org>
> ---
>  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(-)
> 
> diff --git a/drivers/thermal/thermal_core.c
> b/drivers/thermal/thermal_core.c
> index 589925ac0994..c2e13e122934 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -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);
> diff --git a/drivers/thermal/thermal_helpers.c
> b/drivers/thermal/thermal_helpers.c
> index 2ba756af76b7..f550fdee0f9b 100644
> --- a/drivers/thermal/thermal_helpers.c
> +++ b/drivers/thermal/thermal_helpers.c
> @@ -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*/
> diff --git a/drivers/thermal/thermal_sysfs.c
> b/drivers/thermal/thermal_sysfs.c
> index 275ffee292bf..eddada715ad2 100644
> --- a/drivers/thermal/thermal_sysfs.c
> +++ b/drivers/thermal/thermal_sysfs.c
> @@ -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;
>  }
>  
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index 5f4705f46c2f..7a133bd6ff86 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -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 {

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] drivers: thermal: do not clobber cooling dev state from userspace
  2018-07-26  8:55 ` Zhang Rui
@ 2018-08-14 18:04   ` Lina Iyer
  2018-08-22 16:48     ` Ram Chandrasekar
  0 siblings, 1 reply; 4+ messages in thread
From: Lina Iyer @ 2018-08-14 18:04 UTC (permalink / raw)
  To: Zhang Rui; +Cc: edubezval, linux-pm, linux-kernel, linux-arm-msm, rkumbako

Adding Ram, so he can respond.
-- Lina

On Thu, Jul 26 2018 at 02:55 -0600, Zhang Rui wrote:
>On 一, 2018-05-07 at 11:55 -0600, Lina Iyer wrote:
>> From: Ram Chandrasekar <rkumbako@codeaurora.org>
>>
>> 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
>
>> Signed-off-by: Lina Iyer <ilina@codeaurora.org>
>> ---
>>  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(-)
>>
>> diff --git a/drivers/thermal/thermal_core.c
>> b/drivers/thermal/thermal_core.c
>> index 589925ac0994..c2e13e122934 100644
>> --- a/drivers/thermal/thermal_core.c
>> +++ b/drivers/thermal/thermal_core.c
>> @@ -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);
>> diff --git a/drivers/thermal/thermal_helpers.c
>> b/drivers/thermal/thermal_helpers.c
>> index 2ba756af76b7..f550fdee0f9b 100644
>> --- a/drivers/thermal/thermal_helpers.c
>> +++ b/drivers/thermal/thermal_helpers.c
>> @@ -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*/
>> diff --git a/drivers/thermal/thermal_sysfs.c
>> b/drivers/thermal/thermal_sysfs.c
>> index 275ffee292bf..eddada715ad2 100644
>> --- a/drivers/thermal/thermal_sysfs.c
>> +++ b/drivers/thermal/thermal_sysfs.c
>> @@ -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;
>>  }
>>  
>> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
>> index 5f4705f46c2f..7a133bd6ff86 100644
>> --- a/include/linux/thermal.h
>> +++ b/include/linux/thermal.h
>> @@ -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 {

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] drivers: thermal: do not clobber cooling dev state from userspace
  2018-08-14 18:04   ` Lina Iyer
@ 2018-08-22 16:48     ` Ram Chandrasekar
  0 siblings, 0 replies; 4+ messages in thread
From: Ram Chandrasekar @ 2018-08-22 16:48 UTC (permalink / raw)
  To: Lina Iyer, Zhang Rui; +Cc: edubezval, linux-pm, linux-kernel, linux-arm-msm



On 8/14/2018 12:04 PM, Lina Iyer wrote:
> Adding Ram, so he can respond.
> -- Lina
> 
> On Thu, Jul 26 2018 at 02:55 -0600, Zhang Rui wrote:
>> On 一, 2018-05-07 at 11:55 -0600, Lina Iyer wrote:
>>> From: Ram Chandrasekar <rkumbako@codeaurora.org>
>>>
>>> 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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2018-08-22 16:48 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-07 17:55 [PATCH] drivers: thermal: do not clobber cooling dev state from userspace Lina Iyer
2018-07-26  8:55 ` Zhang Rui
2018-08-14 18:04   ` Lina Iyer
2018-08-22 16:48     ` Ram Chandrasekar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).