RE: [PATCH 1/4] thermal: Add new thermal trend type to support quick cooling
Hi Amit/Rui,
-Original Message-
From: Amit Kachhap [mailto:amit.kach...@linaro.org]
Sent: Friday, November 09, 2012 11:52 AM
To: Zhang, Rui
Cc: linux...@lists.linux-foundation.org; linux-samsung-
s...@vger.kernel.org; linux-ker...@vger.kernel.org; R, Durgadoss;
l...@kernel.org; linux-a...@vger.kernel.org; jonghwa3@samsung.com
Subject: Re: [PATCH 1/4] thermal: Add new thermal trend type to support
quick cooling
On 9 November 2012 09:21, Zhang Rui rui.zh...@intel.com wrote:
On Thu, 2012-11-08 at 11:56 +0530, Amit Kachhap wrote:
On 8 November 2012 11:31, Zhang Rui rui.zh...@intel.com wrote:
On Thu, 2012-11-08 at 09:56 +0530, Amit Daniel Kachhap wrote:
This modification adds 2 new thermal trend type
THERMAL_TREND_RAISE_FULL
and THERMAL_TREND_DROP_FULL. This thermal trend can be used to
quickly
jump to the upper or lower cooling level instead of incremental
increase
or decrease.
IMO, what we need is a new more aggressive cooling governor which
always
uses upper limit when the temperature is raising and lower limit when
the temperature is dropping.
Yes I agree that a new aggressive governor is the best approach but
then i thought adding a new trend type is a simple solution to achieve
this and since most of the governor logic might be same as the
step-wise governor. I have no objection in doing it through governor.
hmmm,
I think a more proper way is to set the cooling state to upper limit
when it overheats and reduce the cooling state step by step when the
temperature drops.
No actually I was thinking of having a simple governor with a feature
like it only sets to upper level and lower level. Also since the
temperature sensor is capable of interrupting for both increase in
threshold(say 100C) and fall in threshold (say 90C), so polling or
step increments is not needed at all.
Currently stepwise governor governor does that so we might change the
macro name as,
THERMAL_TREND_RAISE_STEP,
THERMAL_TREND_DROP_STEP,
THERMAL_TREND_RAISE_MAX,
THERMAL_TREND_DROP_MAX,
and file step_wise.c can be named as state_wise.c or trend_wise.c.
Yes, in this particular case, we neither need to poll nor do step wise
operations. But, most of the other sensors need at least one of them.
So, I think we can try it this way:
if (sensor supports interrupt) {
'always' use RAISE_MAX and DROP_MAX;
} else {
Do Step wise operations
}
And this could be plugged into step wise. I don't think we need a
complete new governor just to get this case working.
For this to work, we need a way for the governor to know 'whether the
sensor can work on interrupt mode'. May be introducing a new field in
tzd structure can help us here.
I am fine with any name for the governor :-)
Thanks,
Durga
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/4] thermal: Add new thermal trend type to support quick cooling
-Original Message-
From: Zhang, Rui
Sent: Monday, November 12, 2012 12:03 PM
To: R, Durgadoss
Cc: Amit Kachhap; linux...@lists.linux-foundation.org; linux-samsung-
s...@vger.kernel.org; linux-ker...@vger.kernel.org; l...@kernel.org; linux-
a...@vger.kernel.org; jonghwa3@samsung.com
Subject: RE: [PATCH 1/4] thermal: Add new thermal trend type to support
quick cooling
On Sun, 2012-11-11 at 22:55 -0700, R, Durgadoss wrote:
Hi Amit/Rui,
-Original Message-
From: Amit Kachhap [mailto:amit.kach...@linaro.org]
Sent: Friday, November 09, 2012 11:52 AM
To: Zhang, Rui
Cc: linux...@lists.linux-foundation.org; linux-samsung-
s...@vger.kernel.org; linux-ker...@vger.kernel.org; R, Durgadoss;
l...@kernel.org; linux-a...@vger.kernel.org;
jonghwa3@samsung.com
Subject: Re: [PATCH 1/4] thermal: Add new thermal trend type to
support
quick cooling
On 9 November 2012 09:21, Zhang Rui rui.zh...@intel.com wrote:
On Thu, 2012-11-08 at 11:56 +0530, Amit Kachhap wrote:
On 8 November 2012 11:31, Zhang Rui rui.zh...@intel.com wrote:
On Thu, 2012-11-08 at 09:56 +0530, Amit Daniel Kachhap wrote:
This modification adds 2 new thermal trend type
THERMAL_TREND_RAISE_FULL
and THERMAL_TREND_DROP_FULL. This thermal trend can be used
to
quickly
jump to the upper or lower cooling level instead of incremental
increase
or decrease.
IMO, what we need is a new more aggressive cooling governor
which
always
uses upper limit when the temperature is raising and lower limit
when
the temperature is dropping.
Yes I agree that a new aggressive governor is the best approach but
then i thought adding a new trend type is a simple solution to achieve
this and since most of the governor logic might be same as the
step-wise governor. I have no objection in doing it through governor.
hmmm,
I think a more proper way is to set the cooling state to upper limit
when it overheats and reduce the cooling state step by step when the
temperature drops.
No actually I was thinking of having a simple governor with a feature
like it only sets to upper level and lower level. Also since the
temperature sensor is capable of interrupting for both increase in
threshold(say 100C) and fall in threshold (say 90C), so polling or
step increments is not needed at all.
Currently stepwise governor governor does that so we might change the
macro name as,
THERMAL_TREND_RAISE_STEP,
THERMAL_TREND_DROP_STEP,
THERMAL_TREND_RAISE_MAX,
THERMAL_TREND_DROP_MAX,
and file step_wise.c can be named as state_wise.c or trend_wise.c.
Yes, in this particular case, we neither need to poll nor do step wise
operations. But, most of the other sensors need at least one of them.
So, I think we can try it this way:
if (sensor supports interrupt) {
'always' use RAISE_MAX and DROP_MAX;
} else {
Do Step wise operations
}
why should the generic thermal layer be aware of this?
IMO, it is the platform thermal driver that is responsible for returning
THERMAL_TREND_RAISE_STEP or THERMAL_TREND_RAISE_MAX.
and the step_wise governor just takes action based on the return value
of .get_trend() callback.
Yes, agree with the flow ..
Thanks,
Durga
N�r��yb�X��ǧv�^�){.n�+{�x,�ȧ���ܨ}���Ơz�j:+v���zZ+��+zf���h���~i���z� �w���?��)ߢf
RE: [PATCH 1/4] thermal: Add new thermal trend type to support quick cooling
Hi Rui/Amit,
Sorry for the late response..
-Original Message-
From: Amit Kachhap [mailto:amit.kach...@linaro.org]
Sent: Thursday, November 08, 2012 11:56 AM
To: Zhang, Rui
Cc: linux...@lists.linux-foundation.org; linux-samsung-
s...@vger.kernel.org; linux-ker...@vger.kernel.org; R, Durgadoss;
l...@kernel.org; linux-a...@vger.kernel.org; jonghwa3@samsung.com
Subject: Re: [PATCH 1/4] thermal: Add new thermal trend type to support
quick cooling
On 8 November 2012 11:31, Zhang Rui rui.zh...@intel.com wrote:
On Thu, 2012-11-08 at 09:56 +0530, Amit Daniel Kachhap wrote:
This modification adds 2 new thermal trend type
THERMAL_TREND_RAISE_FULL
and THERMAL_TREND_DROP_FULL. This thermal trend can be used to
quickly
jump to the upper or lower cooling level instead of incremental increase
or decrease.
IMO, what we need is a new more aggressive cooling governor which
always
uses upper limit when the temperature is raising and lower limit when
the temperature is dropping.
Yes I agree that a new aggressive governor is the best approach but
then i thought adding a new trend type is a simple solution to achieve
this and since most of the governor logic might be same as the
step-wise governor. I have no objection in doing it through governor.
Yes, this sounds like a feasible and not-so-complicated implementation for now.
In future, if we see a lot of drivers requiring this sudden raise/drop
functionality,
at that time we can introduce an 'aggressive' governor.
Thanks,
Durga
I can write such a governor if you do not have time to.
ok. thanks
thanks,
rui
This is needed for temperature sensors which support rising/falling
threshold interrupts and polling can be totally avoided.
Signed-off-by: Amit Daniel Kachhap amit.dan...@samsung.com
Signed-off-by: Amit Daniel Kachhap amit.kach...@linaro.org
---
drivers/thermal/step_wise.c | 19 +++
include/linux/thermal.h |2 ++
2 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/drivers/thermal/step_wise.c b/drivers/thermal/step_wise.c
index 1242cff..0d2d8d6 100644
--- a/drivers/thermal/step_wise.c
+++ b/drivers/thermal/step_wise.c
@@ -35,6 +35,10 @@
* state for this trip point
*b. if the trend is THERMAL_TREND_DROPPING, use lower cooling
* state for this trip point
+ *c. if the trend is THERMAL_TREND_RAISE_FULL, use highest cooling
+ * state for this trip point
+ *d. if the trend is THERMAL_TREND_DROP_FULL, use lowest cooling
+ * state for this trip point
*/
static unsigned long get_target_state(struct thermal_instance *instance,
enum thermal_trend trend)
@@ -50,7 +54,10 @@ static unsigned long get_target_state(struct
thermal_instance *instance,
} else if (trend == THERMAL_TREND_DROPPING) {
cur_state = cur_state instance-lower ?
(cur_state - 1) : instance-lower;
- }
+ } else if (trend == THERMAL_TREND_RAISE_FULL)
+ cur_state = instance-upper;
+ else if (trend == THERMAL_TREND_DROP_FULL)
+ cur_state = instance-lower;
return cur_state;
}
@@ -87,7 +94,8 @@ static void update_instance_for_throttle(struct
thermal_zone_device *tz,
}
static void update_instance_for_dethrottle(struct thermal_zone_device
*tz,
- int trip, enum thermal_trip_type trip_type)
+ int trip, enum thermal_trip_type trip_type,
+ enum thermal_trend trend)
{
struct thermal_instance *instance;
struct thermal_cooling_device *cdev;
@@ -101,7 +109,10 @@ static void
update_instance_for_dethrottle(struct thermal_zone_device *tz,
cdev = instance-cdev;
cdev-ops-get_cur_state(cdev, cur_state);
- instance-target = cur_state instance-lower ?
+ if (trend == THERMAL_TREND_DROP_FULL)
+ instance-target = instance-lower;
+ else
+ instance-target = cur_state instance-lower ?
(cur_state - 1) : THERMAL_NO_TARGET;
/* Deactivate a passive thermal instance */
@@ -133,7 +144,7 @@ static void thermal_zone_trip_update(struct
thermal_zone_device *tz, int trip)
if (tz-temperature = trip_temp)
update_instance_for_throttle(tz, trip, trip_type, trend);
else
- update_instance_for_dethrottle(tz, trip, trip_type);
+ update_instance_for_dethrottle(tz, trip, trip_type, trend);
mutex_unlock(tz-lock);
}
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 807f214..8bce3ec 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -68,6 +68,8 @@ enum thermal_trend {
THERMAL_TREND_STABLE, /* temperature
RE: [PATCH 1/4] thermal: Add new thermal trend type to support quick cooling
Hi Amit/Rui,
-Original Message-
From: Zhang, Rui
Sent: Friday, November 09, 2012 9:21 AM
To: Amit Kachhap
Cc: linux...@lists.linux-foundation.org; linux-samsung-
s...@vger.kernel.org; linux-ker...@vger.kernel.org; R, Durgadoss;
l...@kernel.org; linux-a...@vger.kernel.org; jonghwa3@samsung.com
Subject: Re: [PATCH 1/4] thermal: Add new thermal trend type to support
quick cooling
On Thu, 2012-11-08 at 11:56 +0530, Amit Kachhap wrote:
On 8 November 2012 11:31, Zhang Rui rui.zh...@intel.com wrote:
On Thu, 2012-11-08 at 09:56 +0530, Amit Daniel Kachhap wrote:
This modification adds 2 new thermal trend type
THERMAL_TREND_RAISE_FULL
and THERMAL_TREND_DROP_FULL. This thermal trend can be used to
quickly
jump to the upper or lower cooling level instead of incremental increase
or decrease.
IMO, what we need is a new more aggressive cooling governor which
always
uses upper limit when the temperature is raising and lower limit when
the temperature is dropping.
Yes I agree that a new aggressive governor is the best approach but
then i thought adding a new trend type is a simple solution to achieve
this and since most of the governor logic might be same as the
step-wise governor. I have no objection in doing it through governor.
hmmm,
I think a more proper way is to set the cooling state to upper limit
when it overheats and reduce the cooling state step by step when the
temperature drops.
what do you think?
I have only one concern here: (mostly on Passive cooling cases)
Setting the cooling state to upper limit will surely help in rapid cooling,
but it will also disrupt the thermal steady state, and the performance might
be jittery.
Let me explain a bit:
On small form factors (like smartphones, tablets, netbooks), when we run
CPU intensive benchmarks, we can easily observe this jittery performance.
The CPU will run in a very high freq for few seconds(which means temperature is
well below trip point), and then switch back to very low frequency in the next
few seconds(which means temperature hit the trip point). This switch will keep
happening for every few seconds. So, the temperature never settles (say for
example,
somewhere in the middle of [low CPU temp, CPU Trip temp].
I could see two reasons for this:
1. The poll delay: Between two successive polls, however small the poll
delay(~20s) may be,
the CPU temperature can raise up to 15C (Just my observation)
2. Sudden passive cooling. The freq switches between HFM and LFM and never
something in between.
That’s why for passive cooling cases, this behavior might not be welcomed
always.
So, I would prefer not to set the cooling state to upper limit always. Instead,
we will
keep the existing behavior but introduce new trend types (something like what
Amit
has done). In this case, the user/tester is explicitly is setting the cooling
trend to
'SUDDEN cooling' which means he/she is 'Ok' with Jitter in performance. Things
are
explicitly said here, which makes it easy to identify performance issues, if
any arise.
In fact, this is one of the reasons, why we have the 'weight' and the
'cur_trip_level'
variables in the fair share governor. Together, both these variables, ensure
that
we do not throttle a cooling device, to more than what is necessary.
I do not think any of this matters for active cooling, where we do not impact
performance :-)
Sorry again for the late response. Thanks both of you for bringing this up..
Thanks,
Durga
thanks,
rui
I can write such a governor if you do not have time to.
ok. thanks
thanks,
rui
This is needed for temperature sensors which support rising/falling
threshold interrupts and polling can be totally avoided.
Signed-off-by: Amit Daniel Kachhap amit.dan...@samsung.com
Signed-off-by: Amit Daniel Kachhap amit.kach...@linaro.org
---
drivers/thermal/step_wise.c | 19 +++
include/linux/thermal.h |2 ++
2 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/drivers/thermal/step_wise.c b/drivers/thermal/step_wise.c
index 1242cff..0d2d8d6 100644
--- a/drivers/thermal/step_wise.c
+++ b/drivers/thermal/step_wise.c
@@ -35,6 +35,10 @@
* state for this trip point
*b. if the trend is THERMAL_TREND_DROPPING, use lower cooling
* state for this trip point
+ *c. if the trend is THERMAL_TREND_RAISE_FULL, use highest cooling
+ * state for this trip point
+ *d. if the trend is THERMAL_TREND_DROP_FULL, use lowest cooling
+ * state for this trip point
*/
static unsigned long get_target_state(struct thermal_instance
*instance,
enum thermal_trend trend)
@@ -50,7 +54,10 @@ static unsigned long get_target_state(struct
thermal_instance *instance,
} else if (trend == THERMAL_TREND_DROPPING) {
cur_state = cur_state instance-lower
RE: [PATCH 1/4] thermal: exynos: Add thermal interface support for linux thermal layer
Hi Amit,
Thanks for keeping this up. And Sorry for late reply.
-Original Message-
From: amit kachhap [mailto:amitdani...@gmail.com] On Behalf Of Amit Daniel
Kachhap
Sent: Saturday, March 03, 2012 4:36 PM
To: linux...@lists.linux-foundation.org; linux-samsung-soc@vger.kernel.org
Cc: linux-ker...@vger.kernel.org; mj...@srcf.ucam.org; linux-
a...@vger.kernel.org; l...@kernel.org; linaro-...@lists.linaro.org; lm-
sens...@lm-sensors.org; amit.kach...@linaro.org; eduardo.valen...@ti.com; R,
Durgadoss; patc...@linaro.org
Subject: [PATCH 1/4] thermal: exynos: Add thermal interface support for linux
thermal layer
This codes uses the generic linux thermal layer and creates a bridge
between temperature sensors, linux thermal framework and cooling devices
for samsung exynos platform. This layer recieves or monitor the
temperature from the sensor and informs the generic thermal layer to take
the necessary cooling action.
Signed-off-by: Amit Daniel Kachhap amit.kach...@linaro.org
---
drivers/thermal/Kconfig |8 +
drivers/thermal/Makefile |1 +
drivers/thermal/exynos_thermal.c | 272
++
include/linux/exynos_thermal.h | 72 ++
4 files changed, 353 insertions(+), 0 deletions(-)
create mode 100644 drivers/thermal/exynos_thermal.c
create mode 100644 include/linux/exynos_thermal.h
diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index 298c1cd..4e8df56 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -29,3 +29,11 @@ config CPU_THERMAL
This will be useful for platforms using the generic thermal interface
and not the ACPI interface.
If you want this support, you should say Y or M here.
+
+config SAMSUNG_THERMAL_INTERFACE
+ bool Samsung Thermal interface support
+ depends on THERMAL CPU_THERMAL
+ help
+ This is a samsung thermal interface which will be used as
+ a link between sensors and cooling devices with linux thermal
+ framework.
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index 655cbc4..c67b6b2 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -4,3 +4,4 @@
obj-$(CONFIG_THERMAL)+= thermal_sys.o
obj-$(CONFIG_CPU_THERMAL)+= cpu_cooling.o
+obj-$(CONFIG_SAMSUNG_THERMAL_INTERFACE) += exynos_thermal.o
diff --git a/drivers/thermal/exynos_thermal.c
b/drivers/thermal/exynos_thermal.c
new file mode 100644
index 000..878d45c
--- /dev/null
+++ b/drivers/thermal/exynos_thermal.c
@@ -0,0 +1,272 @@
+/* linux/drivers/thermal/exynos_thermal.c
+ *
+ * Copyright (c) 2010-2011 Samsung Electronics Co., Ltd.
+ * http://www.samsung.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+*/
+
+#include linux/kernel.h
+#include linux/module.h
+#include linux/thermal.h
+#include linux/platform_device.h
+#include linux/cpufreq.h
+#include linux/err.h
+#include linux/slab.h
+#include linux/cpu_cooling.h
+#include linux/exynos_thermal.h
+
+#define MAX_COOLING_DEVICE 4
+struct exynos4_thermal_zone {
+ unsigned int idle_interval;
+ unsigned int active_interval;
+ struct thermal_zone_device *therm_dev;
+ struct thermal_cooling_device *cool_dev[MAX_COOLING_DEVICE];
+ unsigned int cool_dev_size;
+ struct platform_device *exynos4_dev;
+ struct thermal_sensor_conf *sensor_conf;
+};
+
+static struct exynos4_thermal_zone *th_zone;
+
+static int exynos4_get_mode(struct thermal_zone_device *thermal,
+ enum thermal_device_mode *mode)
+{
+ if (th_zone-sensor_conf) {
+ pr_info(Temperature sensor not initialised\n);
+ *mode = THERMAL_DEVICE_DISABLED;
+ } else
+ *mode = THERMAL_DEVICE_ENABLED;
+ return 0;
+}
+
+static int exynos4_set_mode(struct thermal_zone_device *thermal,
+ enum thermal_device_mode mode)
+{
+ if (!th_zone-therm_dev) {
+ pr_notice(thermal zone not registered\n);
+ return 0;
+ }
+ if (mode == THERMAL_DEVICE_ENABLED)
+ th_zone-therm_dev-polling_delay =
+ th_zone-active_interval*1000;
+ else
+ th_zone-therm_dev-polling_delay =
+ th_zone-idle_interval*1000;
If it is 'DISABLED' mode, shouldn't the polling delay be just 0 ?
+
+ thermal_zone_device_update(th_zone-therm_dev);
+ pr_info(thermal polling set for duration=%d sec\n,
+ th_zone-therm_dev-polling_delay/1000);
+ return 0;
+}
+
+/*This may be called from interrupt based temperature sensor*/
+void exynos4_report_trigger(void)
+{
+ unsigned int monitor_temp;
+
+ if (!th_zone
RE: [PATCH 1/4] thermal: exynos: Add thermal interface support for linux thermal layer
Hi Amit, [snip.] + + kobject_uevent(th_zone-therm_dev-device.kobj, KOBJ_CHANGE); Wouldn't it make more sense to pass the trip point id also as an 'env' parameter ? This way, the user space can easily figure out which trip point has been crossed. Its a good suggestion. I will check if some uevent property allows that. The kobject_uevent_env(...) call allows that. The third argument takes a char *envp[]. For example, We could pass trip=0 to indicate the trip point. I should have mentioned this in my previous reply itself..missed it.. Thanks, Durga -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [linux-pm] [lm-sensors] [RFC PATCH 2/3] thermal: exynos4: Register the tmu sensor with the thermal interface layer
Hi Amit Daniel, Hi Guenter, The main idea of this work is to leave the current userspace based notification scheme and add the kernel based cooling scheme on top of it. Anyway, It is a good idea to move the file hwmon/exynos4_tmu.c as But, What I feel is, kernel based cooling scheme will work only for Controlling 'CPU' frequency. But in SoC's there are other devices that Contribute to Thermal. For example, GPU, Display, Battery (during charging) etc.. In this case, we need a user space to control these devices. So, in a way, the user space notification mechanism is a unified solution for throttling all devices and keeps the kernel code light weight. I am also curious to know why the existing mechanism did not work for you ? Thanks, Durga this creates 2 hwmon entries. Adding CC: Donggeun Kim to know his opinion. Thanks, Amit Daniel [snip.] -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
