linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] thermal: re-calculate k_po/k_pu when update sustainable power
@ 2015-12-30 10:38 Leo Yan
  2015-12-31 17:21 ` Eduardo Valentin
  0 siblings, 1 reply; 10+ messages in thread
From: Leo Yan @ 2015-12-30 10:38 UTC (permalink / raw)
  To: Zhang Rui, Eduardo Valentin, linux-pm, linux-kernel, javi.merino; +Cc: Leo Yan

k_po/k_pu are in essence ratio values compared with sustainable power.
So when update sustainable power, we can recalculate k_po/k_pu simply
with below formula:

               sustainable_power(new)
    k_p(new) = ---------------------- * k_p(old)
               sustainable_power(old)

Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 drivers/thermal/thermal_core.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index d9e525c..223f8df 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -908,7 +908,7 @@ sustainable_power_store(struct device *dev, struct device_attribute *devattr,
 			const char *buf, size_t count)
 {
 	struct thermal_zone_device *tz = to_thermal_zone(dev);
-	u32 sustainable_power;
+	u32 sustainable_power, old_val;
 
 	if (!tz->tzp)
 		return -EIO;
@@ -916,8 +916,12 @@ sustainable_power_store(struct device *dev, struct device_attribute *devattr,
 	if (kstrtou32(buf, 10, &sustainable_power))
 		return -EINVAL;
 
+	old_val = tz->tzp->sustainable_power;
+
 	tz->tzp->sustainable_power = sustainable_power;
 
+	tz->tzp->k_po = (tz->tzp->k_po * sustainable_power) / old_val;
+	tz->tzp->k_pu = (tz->tzp->k_pu * sustainable_power) / old_val;
 	return count;
 }
 static DEVICE_ATTR(sustainable_power, S_IWUSR | S_IRUGO, sustainable_power_show,
-- 
1.9.1


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

* Re: [PATCH v2] thermal: re-calculate k_po/k_pu when update sustainable power
  2015-12-30 10:38 [PATCH v2] thermal: re-calculate k_po/k_pu when update sustainable power Leo Yan
@ 2015-12-31 17:21 ` Eduardo Valentin
  2016-01-01  0:03   ` Leo Yan
  0 siblings, 1 reply; 10+ messages in thread
From: Eduardo Valentin @ 2015-12-31 17:21 UTC (permalink / raw)
  To: Leo Yan; +Cc: Zhang Rui, linux-pm, linux-kernel, javi.merino

On Wed, Dec 30, 2015 at 06:38:40PM +0800, Leo Yan wrote:
> k_po/k_pu are in essence ratio values compared with sustainable power.
> So when update sustainable power, we can recalculate k_po/k_pu simply
> with below formula:
> 
>                sustainable_power(new)
>     k_p(new) = ---------------------- * k_p(old)
>                sustainable_power(old)
> 
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
>  drivers/thermal/thermal_core.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index d9e525c..223f8df 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -908,7 +908,7 @@ sustainable_power_store(struct device *dev, struct device_attribute *devattr,
>  			const char *buf, size_t count)
>  {
>  	struct thermal_zone_device *tz = to_thermal_zone(dev);
> -	u32 sustainable_power;
> +	u32 sustainable_power, old_val;
>  
>  	if (!tz->tzp)
>  		return -EIO;
> @@ -916,8 +916,12 @@ sustainable_power_store(struct device *dev, struct device_attribute *devattr,
>  	if (kstrtou32(buf, 10, &sustainable_power))
>  		return -EINVAL;
>  
> +	old_val = tz->tzp->sustainable_power;
> +
>  	tz->tzp->sustainable_power = sustainable_power;
>  
> +	tz->tzp->k_po = (tz->tzp->k_po * sustainable_power) / old_val;
> +	tz->tzp->k_pu = (tz->tzp->k_pu * sustainable_power) / old_val;

I believe this has to be done by the governor. These properties are
power_allocator specific. thermal_core should not really care about
them.

>  	return count;
>  }
>  static DEVICE_ATTR(sustainable_power, S_IWUSR | S_IRUGO, sustainable_power_show,
> -- 
> 1.9.1
> 

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

* Re: [PATCH v2] thermal: re-calculate k_po/k_pu when update sustainable power
  2015-12-31 17:21 ` Eduardo Valentin
@ 2016-01-01  0:03   ` Leo Yan
  2016-01-04 11:22     ` Daniel Thompson
  2016-01-05 16:31     ` Javi Merino
  0 siblings, 2 replies; 10+ messages in thread
From: Leo Yan @ 2016-01-01  0:03 UTC (permalink / raw)
  To: Eduardo Valentin; +Cc: Zhang Rui, linux-pm, linux-kernel, javi.merino

Hi Eduardo,

Thanks for review.

On Thu, Dec 31, 2015 at 09:21:26AM -0800, Eduardo Valentin wrote:
> On Wed, Dec 30, 2015 at 06:38:40PM +0800, Leo Yan wrote:
> > k_po/k_pu are in essence ratio values compared with sustainable power.
> > So when update sustainable power, we can recalculate k_po/k_pu simply
> > with below formula:
> > 
> >                sustainable_power(new)
> >     k_p(new) = ---------------------- * k_p(old)
> >                sustainable_power(old)
> > 
> > Signed-off-by: Leo Yan <leo.yan@linaro.org>
> > ---
> >  drivers/thermal/thermal_core.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> > index d9e525c..223f8df 100644
> > --- a/drivers/thermal/thermal_core.c
> > +++ b/drivers/thermal/thermal_core.c
> > @@ -908,7 +908,7 @@ sustainable_power_store(struct device *dev, struct device_attribute *devattr,
> >  			const char *buf, size_t count)
> >  {
> >  	struct thermal_zone_device *tz = to_thermal_zone(dev);
> > -	u32 sustainable_power;
> > +	u32 sustainable_power, old_val;
> >  
> >  	if (!tz->tzp)
> >  		return -EIO;
> > @@ -916,8 +916,12 @@ sustainable_power_store(struct device *dev, struct device_attribute *devattr,
> >  	if (kstrtou32(buf, 10, &sustainable_power))
> >  		return -EINVAL;
> >  
> > +	old_val = tz->tzp->sustainable_power;
> > +
> >  	tz->tzp->sustainable_power = sustainable_power;
> >  
> > +	tz->tzp->k_po = (tz->tzp->k_po * sustainable_power) / old_val;
> > +	tz->tzp->k_pu = (tz->tzp->k_pu * sustainable_power) / old_val;
> 
> I believe this has to be done by the governor. These properties are
> power_allocator specific. thermal_core should not really care about
> them.

Okay, I will try to update these properties in power_allocator.c.
Javi, do you think this is fine for you?

Thanks,
Leo Yan

> 
> >  	return count;
> >  }
> >  static DEVICE_ATTR(sustainable_power, S_IWUSR | S_IRUGO, sustainable_power_show,
> > -- 
> > 1.9.1
> > 

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

* Re: [PATCH v2] thermal: re-calculate k_po/k_pu when update sustainable power
  2016-01-01  0:03   ` Leo Yan
@ 2016-01-04 11:22     ` Daniel Thompson
  2016-01-04 14:31       ` Leo Yan
  2016-01-05 16:33       ` Javi Merino
  2016-01-05 16:31     ` Javi Merino
  1 sibling, 2 replies; 10+ messages in thread
From: Daniel Thompson @ 2016-01-04 11:22 UTC (permalink / raw)
  To: Leo Yan, Eduardo Valentin; +Cc: Zhang Rui, linux-pm, linux-kernel, javi.merino

On 01/01/16 00:03, Leo Yan wrote:
> Hi Eduardo,
>
> Thanks for review.
>
> On Thu, Dec 31, 2015 at 09:21:26AM -0800, Eduardo Valentin wrote:
>> On Wed, Dec 30, 2015 at 06:38:40PM +0800, Leo Yan wrote:
>>> k_po/k_pu are in essence ratio values compared with sustainable power.
>>> So when update sustainable power, we can recalculate k_po/k_pu simply
>>> with below formula:
>>>
>>>                 sustainable_power(new)
>>>      k_p(new) = ---------------------- * k_p(old)
>>>                 sustainable_power(old)
>>>
>>> Signed-off-by: Leo Yan <leo.yan@linaro.org>
>>> ---
>>>   drivers/thermal/thermal_core.c | 6 +++++-
>>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
>>> index d9e525c..223f8df 100644
>>> --- a/drivers/thermal/thermal_core.c
>>> +++ b/drivers/thermal/thermal_core.c
>>> @@ -908,7 +908,7 @@ sustainable_power_store(struct device *dev, struct device_attribute *devattr,
>>>   			const char *buf, size_t count)
>>>   {
>>>   	struct thermal_zone_device *tz = to_thermal_zone(dev);
>>> -	u32 sustainable_power;
>>> +	u32 sustainable_power, old_val;
>>>
>>>   	if (!tz->tzp)
>>>   		return -EIO;
>>> @@ -916,8 +916,12 @@ sustainable_power_store(struct device *dev, struct device_attribute *devattr,
>>>   	if (kstrtou32(buf, 10, &sustainable_power))
>>>   		return -EINVAL;
>>>
>>> +	old_val = tz->tzp->sustainable_power;
>>> +
>>>   	tz->tzp->sustainable_power = sustainable_power;
>>>
>>> +	tz->tzp->k_po = (tz->tzp->k_po * sustainable_power) / old_val;
>>> +	tz->tzp->k_pu = (tz->tzp->k_pu * sustainable_power) / old_val;
>>
>> I believe this has to be done by the governor. These properties are
>> power_allocator specific. thermal_core should not really care about
>> them.
>
> Okay, I will try to update these properties in power_allocator.c.
> Javi, do you think this is fine for you?

If sustainable power were to change frequently (perhaps on entry/exit of 
a fan inhibitor mode?) then we would accumulate rounding errors here...





>
> Thanks,
> Leo Yan
>
>>
>>>   	return count;
>>>   }
>>>   static DEVICE_ATTR(sustainable_power, S_IWUSR | S_IRUGO, sustainable_power_show,
>>> --
>>> 1.9.1
>>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>


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

* Re: [PATCH v2] thermal: re-calculate k_po/k_pu when update sustainable power
  2016-01-04 11:22     ` Daniel Thompson
@ 2016-01-04 14:31       ` Leo Yan
  2016-01-05 16:33       ` Javi Merino
  1 sibling, 0 replies; 10+ messages in thread
From: Leo Yan @ 2016-01-04 14:31 UTC (permalink / raw)
  To: Daniel Thompson
  Cc: Eduardo Valentin, Zhang Rui, linux-pm, linux-kernel, javi.merino

On Mon, Jan 04, 2016 at 11:22:26AM +0000, Daniel Thompson wrote:
> On 01/01/16 00:03, Leo Yan wrote:
> >Hi Eduardo,
> >
> >Thanks for review.
> >
> >On Thu, Dec 31, 2015 at 09:21:26AM -0800, Eduardo Valentin wrote:
> >>On Wed, Dec 30, 2015 at 06:38:40PM +0800, Leo Yan wrote:
> >>>k_po/k_pu are in essence ratio values compared with sustainable power.
> >>>So when update sustainable power, we can recalculate k_po/k_pu simply
> >>>with below formula:
> >>>
> >>>                sustainable_power(new)
> >>>     k_p(new) = ---------------------- * k_p(old)
> >>>                sustainable_power(old)
> >>>
> >>>Signed-off-by: Leo Yan <leo.yan@linaro.org>
> >>>---
> >>>  drivers/thermal/thermal_core.c | 6 +++++-
> >>>  1 file changed, 5 insertions(+), 1 deletion(-)
> >>>
> >>>diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> >>>index d9e525c..223f8df 100644
> >>>--- a/drivers/thermal/thermal_core.c
> >>>+++ b/drivers/thermal/thermal_core.c
> >>>@@ -908,7 +908,7 @@ sustainable_power_store(struct device *dev, struct device_attribute *devattr,
> >>>  			const char *buf, size_t count)
> >>>  {
> >>>  	struct thermal_zone_device *tz = to_thermal_zone(dev);
> >>>-	u32 sustainable_power;
> >>>+	u32 sustainable_power, old_val;
> >>>
> >>>  	if (!tz->tzp)
> >>>  		return -EIO;
> >>>@@ -916,8 +916,12 @@ sustainable_power_store(struct device *dev, struct device_attribute *devattr,
> >>>  	if (kstrtou32(buf, 10, &sustainable_power))
> >>>  		return -EINVAL;
> >>>
> >>>+	old_val = tz->tzp->sustainable_power;
> >>>+
> >>>  	tz->tzp->sustainable_power = sustainable_power;
> >>>
> >>>+	tz->tzp->k_po = (tz->tzp->k_po * sustainable_power) / old_val;
> >>>+	tz->tzp->k_pu = (tz->tzp->k_pu * sustainable_power) / old_val;
> >>
> >>I believe this has to be done by the governor. These properties are
> >>power_allocator specific. thermal_core should not really care about
> >>them.
> >
> >Okay, I will try to update these properties in power_allocator.c.
> >Javi, do you think this is fine for you?
> 
> If sustainable power were to change frequently (perhaps on
> entry/exit of a fan inhibitor mode?) then we would accumulate
> rounding errors here...

Correct, I observed it accumulates some minor deviation after change
sustainable power several times.

I think we should change to use k_po_ratio and k_pu_ratio to represent
relationship between k_po/k_pu and sustainable power, and update
k_po/k_pu when sustainable power has been changed.

[...]

Thanks,
Leo Yan

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

* Re: [PATCH v2] thermal: re-calculate k_po/k_pu when update sustainable power
  2016-01-01  0:03   ` Leo Yan
  2016-01-04 11:22     ` Daniel Thompson
@ 2016-01-05 16:31     ` Javi Merino
  1 sibling, 0 replies; 10+ messages in thread
From: Javi Merino @ 2016-01-05 16:31 UTC (permalink / raw)
  To: Leo Yan
  Cc: Eduardo Valentin, Zhang Rui, Punit Agrawal, linux-pm, linux-kernel

On Fri, Jan 01, 2016 at 08:03:47AM +0800, Leo Yan wrote:
> Hi Eduardo,
> 
> Thanks for review.
> 
> On Thu, Dec 31, 2015 at 09:21:26AM -0800, Eduardo Valentin wrote:
> > On Wed, Dec 30, 2015 at 06:38:40PM +0800, Leo Yan wrote:
> > > k_po/k_pu are in essence ratio values compared with sustainable power.
> > > So when update sustainable power, we can recalculate k_po/k_pu simply
> > > with below formula:
> > > 
> > >                sustainable_power(new)
> > >     k_p(new) = ---------------------- * k_p(old)
> > >                sustainable_power(old)
> > > 
> > > Signed-off-by: Leo Yan <leo.yan@linaro.org>
> > > ---
> > >  drivers/thermal/thermal_core.c | 6 +++++-
> > >  1 file changed, 5 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> > > index d9e525c..223f8df 100644
> > > --- a/drivers/thermal/thermal_core.c
> > > +++ b/drivers/thermal/thermal_core.c
> > > @@ -908,7 +908,7 @@ sustainable_power_store(struct device *dev, struct device_attribute *devattr,
> > >  			const char *buf, size_t count)
> > >  {
> > >  	struct thermal_zone_device *tz = to_thermal_zone(dev);
> > > -	u32 sustainable_power;
> > > +	u32 sustainable_power, old_val;
> > >  
> > >  	if (!tz->tzp)
> > >  		return -EIO;
> > > @@ -916,8 +916,12 @@ sustainable_power_store(struct device *dev, struct device_attribute *devattr,
> > >  	if (kstrtou32(buf, 10, &sustainable_power))
> > >  		return -EINVAL;
> > >  
> > > +	old_val = tz->tzp->sustainable_power;
> > > +
> > >  	tz->tzp->sustainable_power = sustainable_power;
> > >  
> > > +	tz->tzp->k_po = (tz->tzp->k_po * sustainable_power) / old_val;
> > > +	tz->tzp->k_pu = (tz->tzp->k_pu * sustainable_power) / old_val;
> > 
> > I believe this has to be done by the governor. These properties are
> > power_allocator specific. thermal_core should not really care about
> > them.
> 
> Okay, I will try to update these properties in power_allocator.c.
> Javi, do you think this is fine for you?

Yes, I'm happy with the idea.  If Eduardo prefers this in
power_allocator.c, let's do it there :)

Javi

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

* Re: [PATCH v2] thermal: re-calculate k_po/k_pu when update sustainable power
  2016-01-04 11:22     ` Daniel Thompson
  2016-01-04 14:31       ` Leo Yan
@ 2016-01-05 16:33       ` Javi Merino
  2016-01-05 16:50         ` Daniel Thompson
  1 sibling, 1 reply; 10+ messages in thread
From: Javi Merino @ 2016-01-05 16:33 UTC (permalink / raw)
  To: Daniel Thompson
  Cc: Leo Yan, Eduardo Valentin, Zhang Rui, linux-pm, linux-kernel,
	Punit Agrawal

On Mon, Jan 04, 2016 at 11:22:26AM +0000, Daniel Thompson wrote:
> On 01/01/16 00:03, Leo Yan wrote:
> >Hi Eduardo,
> >
> >Thanks for review.
> >
> >On Thu, Dec 31, 2015 at 09:21:26AM -0800, Eduardo Valentin wrote:
> >>On Wed, Dec 30, 2015 at 06:38:40PM +0800, Leo Yan wrote:
> >>>k_po/k_pu are in essence ratio values compared with sustainable power.
> >>>So when update sustainable power, we can recalculate k_po/k_pu simply
> >>>with below formula:
> >>>
> >>>                sustainable_power(new)
> >>>     k_p(new) = ---------------------- * k_p(old)
> >>>                sustainable_power(old)
> >>>
> >>>Signed-off-by: Leo Yan <leo.yan@linaro.org>
> >>>---
> >>>  drivers/thermal/thermal_core.c | 6 +++++-
> >>>  1 file changed, 5 insertions(+), 1 deletion(-)
> >>>
> >>>diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> >>>index d9e525c..223f8df 100644
> >>>--- a/drivers/thermal/thermal_core.c
> >>>+++ b/drivers/thermal/thermal_core.c
> >>>@@ -908,7 +908,7 @@ sustainable_power_store(struct device *dev, struct device_attribute *devattr,
> >>>  			const char *buf, size_t count)
> >>>  {
> >>>  	struct thermal_zone_device *tz = to_thermal_zone(dev);
> >>>-	u32 sustainable_power;
> >>>+	u32 sustainable_power, old_val;
> >>>
> >>>  	if (!tz->tzp)
> >>>  		return -EIO;
> >>>@@ -916,8 +916,12 @@ sustainable_power_store(struct device *dev, struct device_attribute *devattr,
> >>>  	if (kstrtou32(buf, 10, &sustainable_power))
> >>>  		return -EINVAL;
> >>>
> >>>+	old_val = tz->tzp->sustainable_power;
> >>>+
> >>>  	tz->tzp->sustainable_power = sustainable_power;
> >>>
> >>>+	tz->tzp->k_po = (tz->tzp->k_po * sustainable_power) / old_val;
> >>>+	tz->tzp->k_pu = (tz->tzp->k_pu * sustainable_power) / old_val;
> >>
> >>I believe this has to be done by the governor. These properties are
> >>power_allocator specific. thermal_core should not really care about
> >>them.
> >
> >Okay, I will try to update these properties in power_allocator.c.
> >Javi, do you think this is fine for you?
> 
> If sustainable power were to change frequently (perhaps on
> entry/exit of a fan inhibitor mode?) then we would accumulate
> rounding errors here...

Can you elaborate on the use case?  What is this fan inhibitor mode?

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

* Re: [PATCH v2] thermal: re-calculate k_po/k_pu when update sustainable power
  2016-01-05 16:33       ` Javi Merino
@ 2016-01-05 16:50         ` Daniel Thompson
  2016-01-05 17:40           ` Punit Agrawal
  0 siblings, 1 reply; 10+ messages in thread
From: Daniel Thompson @ 2016-01-05 16:50 UTC (permalink / raw)
  To: Javi Merino
  Cc: Leo Yan, Eduardo Valentin, Zhang Rui, linux-pm, linux-kernel,
	Punit Agrawal

On 05/01/16 16:33, Javi Merino wrote:
> On Mon, Jan 04, 2016 at 11:22:26AM +0000, Daniel Thompson wrote:
>> On 01/01/16 00:03, Leo Yan wrote:
>>> Hi Eduardo,
>>>
>>> Thanks for review.
>>>
>>> On Thu, Dec 31, 2015 at 09:21:26AM -0800, Eduardo Valentin wrote:
>>>> On Wed, Dec 30, 2015 at 06:38:40PM +0800, Leo Yan wrote:
>>>>> k_po/k_pu are in essence ratio values compared with sustainable power.
>>>>> So when update sustainable power, we can recalculate k_po/k_pu simply
>>>>> with below formula:
>>>>>
>>>>>                 sustainable_power(new)
>>>>>      k_p(new) = ---------------------- * k_p(old)
>>>>>                 sustainable_power(old)
>>>>>
>>>>> Signed-off-by: Leo Yan <leo.yan@linaro.org>
>>>>> ---
>>>>>   drivers/thermal/thermal_core.c | 6 +++++-
>>>>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
>>>>> index d9e525c..223f8df 100644
>>>>> --- a/drivers/thermal/thermal_core.c
>>>>> +++ b/drivers/thermal/thermal_core.c
>>>>> @@ -908,7 +908,7 @@ sustainable_power_store(struct device *dev, struct device_attribute *devattr,
>>>>>   			const char *buf, size_t count)
>>>>>   {
>>>>>   	struct thermal_zone_device *tz = to_thermal_zone(dev);
>>>>> -	u32 sustainable_power;
>>>>> +	u32 sustainable_power, old_val;
>>>>>
>>>>>   	if (!tz->tzp)
>>>>>   		return -EIO;
>>>>> @@ -916,8 +916,12 @@ sustainable_power_store(struct device *dev, struct device_attribute *devattr,
>>>>>   	if (kstrtou32(buf, 10, &sustainable_power))
>>>>>   		return -EINVAL;
>>>>>
>>>>> +	old_val = tz->tzp->sustainable_power;
>>>>> +
>>>>>   	tz->tzp->sustainable_power = sustainable_power;
>>>>>
>>>>> +	tz->tzp->k_po = (tz->tzp->k_po * sustainable_power) / old_val;
>>>>> +	tz->tzp->k_pu = (tz->tzp->k_pu * sustainable_power) / old_val;
>>>>
>>>> I believe this has to be done by the governor. These properties are
>>>> power_allocator specific. thermal_core should not really care about
>>>> them.
>>>
>>> Okay, I will try to update these properties in power_allocator.c.
>>> Javi, do you think this is fine for you?
>>
>> If sustainable power were to change frequently (perhaps on
>> entry/exit of a fan inhibitor mode?) then we would accumulate
>> rounding errors here...
>
> Can you elaborate on the use case?  What is this fan inhibitor mode?

It's made up.

I was trying to think of use cases which might result in the userspace 
wishing to make frequent changes the maximum sustainable power. For 
designs where the system remains thermally overcommited even with the 
fan running then having a mode where the fan must not spin up would 
require such a change.

Inhibiting the fan could be useful for:

1. A distraction-free mode for a laptop. One of the more elderly
    laptops in my household tends to spin up its fans as a background
    process indexes the disc which can be annoying (but not quite
    annoying enough for me to actually try and implement a fan
    inhibition mode).

2. A way to ensure a set-top box, game console or other device that is
    "off" can do background processing without risking bursts of CPU
    activity that makes the fan spin up.


Daniel.

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

* Re: [PATCH v2] thermal: re-calculate k_po/k_pu when update sustainable power
  2016-01-05 16:50         ` Daniel Thompson
@ 2016-01-05 17:40           ` Punit Agrawal
  2016-01-05 18:02             ` Daniel Thompson
  0 siblings, 1 reply; 10+ messages in thread
From: Punit Agrawal @ 2016-01-05 17:40 UTC (permalink / raw)
  To: Daniel Thompson
  Cc: Javi Merino, Leo Yan, Eduardo Valentin, Zhang Rui, linux-pm,
	linux-kernel

Daniel Thompson <daniel.thompson@linaro.org> writes:

> On 05/01/16 16:33, Javi Merino wrote:
>> On Mon, Jan 04, 2016 at 11:22:26AM +0000, Daniel Thompson wrote:
>>> On 01/01/16 00:03, Leo Yan wrote:
>>>> Hi Eduardo,
>>>>
>>>> Thanks for review.
>>>>
>>>> On Thu, Dec 31, 2015 at 09:21:26AM -0800, Eduardo Valentin wrote:
>>>>> On Wed, Dec 30, 2015 at 06:38:40PM +0800, Leo Yan wrote:
>>>>>> k_po/k_pu are in essence ratio values compared with sustainable power.
>>>>>> So when update sustainable power, we can recalculate k_po/k_pu simply
>>>>>> with below formula:
>>>>>>
>>>>>>                 sustainable_power(new)
>>>>>>      k_p(new) = ---------------------- * k_p(old)
>>>>>>                 sustainable_power(old)
>>>>>>
>>>>>> Signed-off-by: Leo Yan <leo.yan@linaro.org>
>>>>>> ---
>>>>>>   drivers/thermal/thermal_core.c | 6 +++++-
>>>>>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
>>>>>> index d9e525c..223f8df 100644
>>>>>> --- a/drivers/thermal/thermal_core.c
>>>>>> +++ b/drivers/thermal/thermal_core.c
>>>>>> @@ -908,7 +908,7 @@ sustainable_power_store(struct device *dev, struct device_attribute *devattr,
>>>>>>   			const char *buf, size_t count)
>>>>>>   {
>>>>>>   	struct thermal_zone_device *tz = to_thermal_zone(dev);
>>>>>> -	u32 sustainable_power;
>>>>>> +	u32 sustainable_power, old_val;
>>>>>>
>>>>>>   	if (!tz->tzp)
>>>>>>   		return -EIO;
>>>>>> @@ -916,8 +916,12 @@ sustainable_power_store(struct device *dev, struct device_attribute *devattr,
>>>>>>   	if (kstrtou32(buf, 10, &sustainable_power))
>>>>>>   		return -EINVAL;
>>>>>>
>>>>>> +	old_val = tz->tzp->sustainable_power;
>>>>>> +
>>>>>>   	tz->tzp->sustainable_power = sustainable_power;
>>>>>>
>>>>>> +	tz->tzp->k_po = (tz->tzp->k_po * sustainable_power) / old_val;
>>>>>> +	tz->tzp->k_pu = (tz->tzp->k_pu * sustainable_power) / old_val;
>>>>>
>>>>> I believe this has to be done by the governor. These properties are
>>>>> power_allocator specific. thermal_core should not really care about
>>>>> them.
>>>>
>>>> Okay, I will try to update these properties in power_allocator.c.
>>>> Javi, do you think this is fine for you?
>>>
>>> If sustainable power were to change frequently (perhaps on
>>> entry/exit of a fan inhibitor mode?) then we would accumulate
>>> rounding errors here...
>>
>> Can you elaborate on the use case?  What is this fan inhibitor mode?
>
> It's made up.
>
> I was trying to think of use cases which might result in the userspace
> wishing to make frequent changes the maximum sustainable power. For
> designs where the system remains thermally overcommited even with the
> fan running then having a mode where the fan must not spin up would
> require such a change.

If there isn't an immediate use case, we should hold off making any
changes until they're really needed.

[...]


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

* Re: [PATCH v2] thermal: re-calculate k_po/k_pu when update sustainable power
  2016-01-05 17:40           ` Punit Agrawal
@ 2016-01-05 18:02             ` Daniel Thompson
  0 siblings, 0 replies; 10+ messages in thread
From: Daniel Thompson @ 2016-01-05 18:02 UTC (permalink / raw)
  To: Punit Agrawal
  Cc: Javi Merino, Leo Yan, Eduardo Valentin, Zhang Rui, linux-pm,
	linux-kernel

On 05/01/16 17:40, Punit Agrawal wrote:
> Daniel Thompson <daniel.thompson@linaro.org> writes:
>
>> On 05/01/16 16:33, Javi Merino wrote:
>>> On Mon, Jan 04, 2016 at 11:22:26AM +0000, Daniel Thompson wrote:
>>>> On 01/01/16 00:03, Leo Yan wrote:
>>>>> Hi Eduardo,
>>>>>
>>>>> Thanks for review.
>>>>>
>>>>> On Thu, Dec 31, 2015 at 09:21:26AM -0800, Eduardo Valentin wrote:
>>>>>> On Wed, Dec 30, 2015 at 06:38:40PM +0800, Leo Yan wrote:
>>>>>>> k_po/k_pu are in essence ratio values compared with sustainable power.
>>>>>>> So when update sustainable power, we can recalculate k_po/k_pu simply
>>>>>>> with below formula:
>>>>>>>
>>>>>>>                  sustainable_power(new)
>>>>>>>       k_p(new) = ---------------------- * k_p(old)
>>>>>>>                  sustainable_power(old)
>>>>>>>
>>>>>>> Signed-off-by: Leo Yan <leo.yan@linaro.org>
>>>>>>> ---
>>>>>>>    drivers/thermal/thermal_core.c | 6 +++++-
>>>>>>>    1 file changed, 5 insertions(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
>>>>>>> index d9e525c..223f8df 100644
>>>>>>> --- a/drivers/thermal/thermal_core.c
>>>>>>> +++ b/drivers/thermal/thermal_core.c
>>>>>>> @@ -908,7 +908,7 @@ sustainable_power_store(struct device *dev, struct device_attribute *devattr,
>>>>>>>    			const char *buf, size_t count)
>>>>>>>    {
>>>>>>>    	struct thermal_zone_device *tz = to_thermal_zone(dev);
>>>>>>> -	u32 sustainable_power;
>>>>>>> +	u32 sustainable_power, old_val;
>>>>>>>
>>>>>>>    	if (!tz->tzp)
>>>>>>>    		return -EIO;
>>>>>>> @@ -916,8 +916,12 @@ sustainable_power_store(struct device *dev, struct device_attribute *devattr,
>>>>>>>    	if (kstrtou32(buf, 10, &sustainable_power))
>>>>>>>    		return -EINVAL;
>>>>>>>
>>>>>>> +	old_val = tz->tzp->sustainable_power;
>>>>>>> +
>>>>>>>    	tz->tzp->sustainable_power = sustainable_power;
>>>>>>>
>>>>>>> +	tz->tzp->k_po = (tz->tzp->k_po * sustainable_power) / old_val;
>>>>>>> +	tz->tzp->k_pu = (tz->tzp->k_pu * sustainable_power) / old_val;
>>>>>>
>>>>>> I believe this has to be done by the governor. These properties are
>>>>>> power_allocator specific. thermal_core should not really care about
>>>>>> them.
>>>>>
>>>>> Okay, I will try to update these properties in power_allocator.c.
>>>>> Javi, do you think this is fine for you?
>>>>
>>>> If sustainable power were to change frequently (perhaps on
>>>> entry/exit of a fan inhibitor mode?) then we would accumulate
>>>> rounding errors here...
>>>
>>> Can you elaborate on the use case?  What is this fan inhibitor mode?
>>
>> It's made up.
>>
>> I was trying to think of use cases which might result in the userspace
>> wishing to make frequent changes the maximum sustainable power. For
>> designs where the system remains thermally overcommited even with the
>> fan running then having a mode where the fan must not spin up would
>> require such a change.
>
> If there isn't an immediate use case, we should hold off making any
> changes until they're really needed.

Fair point but do note that the fan inhibition comments were used to 
illustrate a bug in the patch. They were not an argument for (or 
against) the value of patch itself.


Daniel.

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

end of thread, other threads:[~2016-01-05 18:03 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-30 10:38 [PATCH v2] thermal: re-calculate k_po/k_pu when update sustainable power Leo Yan
2015-12-31 17:21 ` Eduardo Valentin
2016-01-01  0:03   ` Leo Yan
2016-01-04 11:22     ` Daniel Thompson
2016-01-04 14:31       ` Leo Yan
2016-01-05 16:33       ` Javi Merino
2016-01-05 16:50         ` Daniel Thompson
2016-01-05 17:40           ` Punit Agrawal
2016-01-05 18:02             ` Daniel Thompson
2016-01-05 16:31     ` Javi Merino

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).