[PATCH 0/4] thermal: Add support for interrupt based notification to thermal layer

[PATCH 0/4] thermal: Add support for interrupt based notification to thermal layer

[Amit Daniel Kachhap]

The patch submitted by Jonghwa Lee (https://patchwork.kernel.org/patch/1683441/)
adds support for interrupt based notification to thermal layer. This is a good
feature but the current thermal framework needs polling/regular notification for
invoking suitable cooling action. So adding 2 new thermal trend type to implement
this feature.

All these patches are based on thermal maintainer next tree.
git://git.kernel.org/pub/scm/linux/kernel/git/rzhang/linux.git next

Amit Daniel Kachhap (3):
  thermal: Add new thermal trend type to support quick cooling
  thermal: exynos: Miscellaneous fixes to support falling threshold
    interrupt
  thermal: exynos: Use the new thermal trend type for quick cooling
    action.

Jonghwa Lee (1):
  Thermal: exynos: Add support for temperature falling interrupt.

 drivers/thermal/exynos_thermal.c             |  105 +++++++++++++++-----------
 drivers/thermal/step_wise.c                  |   19 ++++-
 include/linux/platform_data/exynos_thermal.h |    3 +
 include/linux/thermal.h                      |    2 +
 4 files changed, 80 insertions(+), 49 deletions(-)

[PATCH 1/4] thermal: Add new thermal trend type to support quick cooling

[Amit Daniel Kachhap]

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. 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.daniel@samsung.com>
Signed-off-by: Amit Daniel Kachhap <amit.kachhap@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 is stable */
 	THERMAL_TREND_RAISING, /* temperature is raising */
 	THERMAL_TREND_DROPPING, /* temperature is dropping */
+	THERMAL_TREND_RAISE_FULL, /* Apply highest cooling action*/
+	THERMAL_TREND_DROP_FULL, /* Apply lowest cooling action*/
 };
 
 /* Events supported by Thermal Netlink */
-- 
1.7.1

[PATCH 2/4] Thermal: exynos: Add support for temperature falling interrupt.

[Amit Daniel Kachhap]

From: Jonghwa Lee <jonghwa3.lee@samsung.com>

This patch introduces using temperature falling interrupt in exynos
thermal driver. Former patch, it only use polling way to check
whether if system themperature is fallen. However, exynos SOC also
provides temperature falling interrupt way to do same things by hw.
This feature is not supported in exynos4210.

Signed-off-by: Jonghwa Lee <jonghwa3.lee@samsung.com>
Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
---
 drivers/thermal/exynos_thermal.c             |   81 +++++++++++++++-----------
 include/linux/platform_data/exynos_thermal.h |    3 +
 2 files changed, 49 insertions(+), 35 deletions(-)

diff --git a/drivers/thermal/exynos_thermal.c b/drivers/thermal/exynos_thermal.c
index 7772d16..f6667e8 100644
--- a/drivers/thermal/exynos_thermal.c
+++ b/drivers/thermal/exynos_thermal.c
@@ -94,6 +94,7 @@
 #define SENSOR_NAME_LEN	16
 #define MAX_TRIP_COUNT	8
 #define MAX_COOLING_DEVICE 4
+#define MAX_THRESHOLD_LEVS 4
 
 #define ACTIVE_INTERVAL 500
 #define IDLE_INTERVAL 10000
@@ -125,6 +126,7 @@ struct exynos_tmu_data {
 struct	thermal_trip_point_conf {
 	int trip_val[MAX_TRIP_COUNT];
 	int trip_count;
+	u8 trigger_falling;
 };
 
 struct	thermal_cooling_conf {
@@ -174,7 +176,8 @@ static int exynos_set_mode(struct thermal_zone_device *thermal,
 
 	mutex_lock(&th_zone->therm_dev->lock);
 
-	if (mode == THERMAL_DEVICE_ENABLED)
+	if (mode == THERMAL_DEVICE_ENABLED &&
+		!th_zone->sensor_conf->trip_data.trigger_falling)
 		th_zone->therm_dev->polling_delay = IDLE_INTERVAL;
 	else
 		th_zone->therm_dev->polling_delay = 0;
@@ -413,7 +416,8 @@ static void exynos_report_trigger(void)
 			break;
 	}
 
-	if (th_zone->mode == THERMAL_DEVICE_ENABLED) {
+	if (th_zone->mode == THERMAL_DEVICE_ENABLED &&
+		!th_zone->sensor_conf->trip_data.trigger_falling) {
 		if (i > 0)
 			th_zone->therm_dev->polling_delay = ACTIVE_INTERVAL;
 		else
@@ -452,7 +456,8 @@ static int exynos_register_thermal(struct thermal_sensor_conf *sensor_conf)
 
 	th_zone->therm_dev = thermal_zone_device_register(sensor_conf->name,
 			EXYNOS_ZONE_COUNT, 0, NULL, &exynos_dev_ops, NULL, 0,
-			IDLE_INTERVAL);
+			sensor_conf->trip_data.trigger_falling ?
+			0 : IDLE_INTERVAL);
 
 	if (IS_ERR(th_zone->therm_dev)) {
 		pr_err("Failed to register thermal zone device\n");
@@ -559,8 +564,9 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
 {
 	struct exynos_tmu_data *data = platform_get_drvdata(pdev);
 	struct exynos_tmu_platform_data *pdata = data->pdata;
-	unsigned int status, trim_info, rising_threshold;
-	int ret = 0, threshold_code;
+	unsigned int status, trim_info;
+	unsigned int rising_threshold = 0, falling_threshold = 0;
+	int ret = 0, threshold_code, i, trigger_levs = 0;
 
 	mutex_lock(&data->lock);
 	clk_enable(data->clk);
@@ -585,6 +591,11 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
 			(data->temp_error2 != 0))
 		data->temp_error1 = pdata->efuse_value;
 
+	/* Count trigger levels to be enabled */
+	for (i = 0; i < MAX_THRESHOLD_LEVS; i++)
+		if (pdata->trigger_levels[i])
+			trigger_levs++;
+
 	if (data->soc == SOC_ARCH_EXYNOS4210) {
 		/* Write temperature code for threshold */
 		threshold_code = temp_to_code(data, pdata->threshold);
@@ -594,44 +605,38 @@ static int exynos_tmu_initialize(struct platform_device *pdev)
 		}
 		writeb(threshold_code,
 			data->base + EXYNOS4210_TMU_REG_THRESHOLD_TEMP);
-
-		writeb(pdata->trigger_levels[0],
-			data->base + EXYNOS4210_TMU_REG_TRIG_LEVEL0);
-		writeb(pdata->trigger_levels[1],
-			data->base + EXYNOS4210_TMU_REG_TRIG_LEVEL1);
-		writeb(pdata->trigger_levels[2],
-			data->base + EXYNOS4210_TMU_REG_TRIG_LEVEL2);
-		writeb(pdata->trigger_levels[3],
-			data->base + EXYNOS4210_TMU_REG_TRIG_LEVEL3);
+		for (i = 0; i < trigger_levs; i++)
+			writeb(pdata->trigger_levels[i],
+			data->base + EXYNOS4210_TMU_REG_TRIG_LEVEL0 + i * 4);
 
 		writel(EXYNOS4210_TMU_INTCLEAR_VAL,
 			data->base + EXYNOS_TMU_REG_INTCLEAR);
 	} else if (data->soc == SOC_ARCH_EXYNOS) {
-		/* Write temperature code for threshold */
-		threshold_code = temp_to_code(data, pdata->trigger_levels[0]);
-		if (threshold_code < 0) {
-			ret = threshold_code;
-			goto out;
-		}
-		rising_threshold = threshold_code;
-		threshold_code = temp_to_code(data, pdata->trigger_levels[1]);
-		if (threshold_code < 0) {
-			ret = threshold_code;
-			goto out;
-		}
-		rising_threshold |= (threshold_code << 8);
-		threshold_code = temp_to_code(data, pdata->trigger_levels[2]);
-		if (threshold_code < 0) {
-			ret = threshold_code;
-			goto out;
+		/* Write temperature code for rising and falling threshold */
+		for (i = 0; i < trigger_levs; i++) {
+			threshold_code = temp_to_code(data,
+						pdata->trigger_levels[i]);
+			if (threshold_code < 0) {
+				ret = threshold_code;
+				goto out;
+			}
+			rising_threshold |= threshold_code << 8 * i;
+			if (pdata->threshold_falling) {
+				threshold_code = temp_to_code(data,
+						pdata->trigger_levels[i] -
+						pdata->threshold_falling);
+				if (threshold_code > 0)
+					falling_threshold |=
+						threshold_code << 8 * i;
+			}
 		}
-		rising_threshold |= (threshold_code << 16);
 
 		writel(rising_threshold,
 				data->base + EXYNOS_THD_TEMP_RISE);
-		writel(0, data->base + EXYNOS_THD_TEMP_FALL);
+		writel(falling_threshold,
+				data->base + EXYNOS_THD_TEMP_FALL);
 
-		writel(EXYNOS_TMU_CLEAR_RISE_INT|EXYNOS_TMU_CLEAR_FALL_INT,
+		writel(EXYNOS_TMU_CLEAR_RISE_INT | EXYNOS_TMU_CLEAR_FALL_INT,
 				data->base + EXYNOS_TMU_REG_INTCLEAR);
 	}
 out:
@@ -664,6 +669,8 @@ static void exynos_tmu_control(struct platform_device *pdev, bool on)
 			pdata->trigger_level2_en << 8 |
 			pdata->trigger_level1_en << 4 |
 			pdata->trigger_level0_en;
+		if (pdata->threshold_falling)
+			interrupt_en |= interrupt_en << 16;
 	} else {
 		con |= EXYNOS_TMU_CORE_OFF;
 		interrupt_en = 0; /* Disable all interrupts */
@@ -702,7 +709,8 @@ static void exynos_tmu_work(struct work_struct *work)
 
 
 	if (data->soc == SOC_ARCH_EXYNOS)
-		writel(EXYNOS_TMU_CLEAR_RISE_INT,
+		writel(EXYNOS_TMU_CLEAR_RISE_INT |
+				EXYNOS_TMU_CLEAR_FALL_INT,
 				data->base + EXYNOS_TMU_REG_INTCLEAR);
 	else
 		writel(EXYNOS4210_TMU_INTCLEAR_VAL,
@@ -759,6 +767,7 @@ static struct exynos_tmu_platform_data const exynos4210_default_tmu_data = {
 
 #if defined(CONFIG_SOC_EXYNOS5250) || defined(CONFIG_SOC_EXYNOS4412)
 static struct exynos_tmu_platform_data const exynos_default_tmu_data = {
+	.threshold_falling = 10,
 	.trigger_levels[0] = 85,
 	.trigger_levels[1] = 103,
 	.trigger_levels[2] = 110,
@@ -916,6 +925,8 @@ static int __devinit exynos_tmu_probe(struct platform_device *pdev)
 		exynos_sensor_conf.trip_data.trip_val[i] =
 			pdata->threshold + pdata->trigger_levels[i];
 
+	exynos_sensor_conf.trip_data.trigger_falling = pdata->threshold_falling;
+
 	exynos_sensor_conf.cooling_data.freq_clip_count =
 						pdata->freq_tab_count;
 	for (i = 0; i < pdata->freq_tab_count; i++) {
diff --git a/include/linux/platform_data/exynos_thermal.h b/include/linux/platform_data/exynos_thermal.h
index a7bdb2f..da7e627 100644
--- a/include/linux/platform_data/exynos_thermal.h
+++ b/include/linux/platform_data/exynos_thermal.h
@@ -53,6 +53,8 @@ struct freq_clip_table {
  * struct exynos_tmu_platform_data
  * @threshold: basic temperature for generating interrupt
  *	       25 <= threshold <= 125 [unit: degree Celsius]
+ * @threshold_falling: differntial value for setting threshold
+ *		       of temperature falling interrupt.
  * @trigger_levels: array for each interrupt levels
  *	[unit: degree Celsius]
  *	0: temperature for trigger_level0 interrupt
@@ -97,6 +99,7 @@ struct freq_clip_table {
  */
 struct exynos_tmu_platform_data {
 	u8 threshold;
+	u8 threshold_falling;
 	u8 trigger_levels[4];
 	bool trigger_level0_en;
 	bool trigger_level1_en;
-- 
1.7.1

[PATCH 3/4] thermal: exynos: Miscellaneous fixes to support falling threshold interrupt

[Amit Daniel Kachhap]

Below fixes are done to support falling threshold interrupt,
* Falling interrupt status macro corrected according to exynos5 data sheet.
* The get trend function modified to calculate trip temperature correctly.
* The clearing of interrupt status in the isr is now done after handling
  the the event that caused the interrupt.

Signed-off-by: Amit Daniel Kachhap <amit.daniel@samsung.com>
Signed-off-by: Amit Daniel Kachhap <amit.kachhap@linaro.org>
---
 drivers/thermal/exynos_thermal.c |   18 +++++++++++-------
 1 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/thermal/exynos_thermal.c b/drivers/thermal/exynos_thermal.c
index f6667e8..bbaff56 100644
--- a/drivers/thermal/exynos_thermal.c
+++ b/drivers/thermal/exynos_thermal.c
@@ -82,7 +82,7 @@
 
 #define EXYNOS_TRIMINFO_RELOAD		0x1
 #define EXYNOS_TMU_CLEAR_RISE_INT	0x111
-#define EXYNOS_TMU_CLEAR_FALL_INT	(0x111 << 16)
+#define EXYNOS_TMU_CLEAR_FALL_INT	(0x111 << 12)
 #define EXYNOS_MUX_ADDR_VALUE		6
 #define EXYNOS_MUX_ADDR_SHIFT		20
 #define EXYNOS_TMU_TRIP_MODE_SHIFT	13
@@ -365,12 +365,19 @@ static int exynos_get_temp(struct thermal_zone_device *thermal,
 static int exynos_get_trend(struct thermal_zone_device *thermal,
 			int trip, enum thermal_trend *trend)
 {
-	if (thermal->temperature >= trip)
+	int ret = 0;
+	unsigned long trip_temp;
+
+	ret = exynos_get_trip_temp(thermal, trip, &trip_temp);
+	if (ret < 0)
+		return ret;
+
+	if (thermal->temperature >= trip_temp)
 		*trend = THERMAL_TREND_RAISING;
 	else
 		*trend = THERMAL_TREND_DROPPING;
 
-	return 0;
+	return ret;
 }
 /* Operation callback functions for thermal zone */
 static struct thermal_zone_device_ops const exynos_dev_ops = {
@@ -704,10 +711,9 @@ static void exynos_tmu_work(struct work_struct *work)
 	struct exynos_tmu_data *data = container_of(work,
 			struct exynos_tmu_data, irq_work);
 
+	exynos_report_trigger();
 	mutex_lock(&data->lock);
 	clk_enable(data->clk);
-
-
 	if (data->soc == SOC_ARCH_EXYNOS)
 		writel(EXYNOS_TMU_CLEAR_RISE_INT |
 				EXYNOS_TMU_CLEAR_FALL_INT,
@@ -715,10 +721,8 @@ static void exynos_tmu_work(struct work_struct *work)
 	else
 		writel(EXYNOS4210_TMU_INTCLEAR_VAL,
 				data->base + EXYNOS_TMU_REG_INTCLEAR);
-
 	clk_disable(data->clk);
 	mutex_unlock(&data->lock);
-	exynos_report_trigger();
 	enable_irq(data->irq);
 }
 
-- 
1.7.1

Re: [PATCH 1/4] thermal: Add new thermal trend type to support quick cooling

[Zhang Rui]

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.

I can write such a governor if you do not have time to.

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.daniel@samsung.com>
> Signed-off-by: Amit Daniel Kachhap <amit.kachhap@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 is stable */
>  	THERMAL_TREND_RAISING, /* temperature is raising */
>  	THERMAL_TREND_DROPPING, /* temperature is dropping */
> +	THERMAL_TREND_RAISE_FULL, /* Apply highest cooling action*/
> +	THERMAL_TREND_DROP_FULL, /* Apply lowest cooling action*/
>  };
>  
>  /* Events supported by Thermal Netlink */

Re: [PATCH 1/4] thermal: Add new thermal trend type to support quick cooling

[Amit Kachhap]

On 8 November 2012 11:31, Zhang Rui <rui.zhang@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.
>
> 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.daniel@samsung.com>
>> Signed-off-by: Amit Daniel Kachhap <amit.kachhap@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 is stable */
>>       THERMAL_TREND_RAISING, /* temperature is raising */
>>       THERMAL_TREND_DROPPING, /* temperature is dropping */
>> +     THERMAL_TREND_RAISE_FULL, /* Apply highest cooling action*/
>> +     THERMAL_TREND_DROP_FULL, /* Apply lowest cooling action*/
>>  };
>>
>>  /* Events supported by Thermal Netlink */
>
>

Re: [PATCH 1/4] thermal: Add new thermal trend type to support quick cooling

[Zhang Rui]

On Thu, 2012-11-08 at 11:56 +0530, Amit Kachhap wrote:
> On 8 November 2012 11:31, Zhang Rui <rui.zhang@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?

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.daniel@samsung.com>
> >> Signed-off-by: Amit Daniel Kachhap <amit.kachhap@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 is stable */
> >>       THERMAL_TREND_RAISING, /* temperature is raising */
> >>       THERMAL_TREND_DROPPING, /* temperature is dropping */
> >> +     THERMAL_TREND_RAISE_FULL, /* Apply highest cooling action*/
> >> +     THERMAL_TREND_DROP_FULL, /* Apply lowest cooling action*/
> >>  };
> >>
> >>  /* Events supported by Thermal Netlink */
> >
> >

RE: [PATCH 1/4] thermal: Add new thermal trend type to support quick cooling

[R, Durgadoss]

Hi Rui/Amit,

Sorry for the late response..

> -----Original Message-----
> From: Amit Kachhap [mailto:amit.kachhap@linaro.org]
> Sent: Thursday, November 08, 2012 11:56 AM
> To: Zhang, Rui
> Cc: linux-pm@lists.linux-foundation.org; linux-samsung-
> soc@vger.kernel.org; linux-kernel@vger.kernel.org; R, Durgadoss;
> lenb@kernel.org; linux-acpi@vger.kernel.org; jonghwa3.lee@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.zhang@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.daniel@samsung.com>
> >> Signed-off-by: Amit Daniel Kachhap <amit.kachhap@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 is stable */
> >>       THERMAL_TREND_RAISING, /* temperature is raising */
> >>       THERMAL_TREND_DROPPING, /* temperature is dropping */
> >> +     THERMAL_TREND_RAISE_FULL, /* Apply highest cooling action*/
> >> +     THERMAL_TREND_DROP_FULL, /* Apply lowest cooling action*/
> >>  };
> >>
> >>  /* Events supported by Thermal Netlink */
> >
> >

RE: [PATCH 1/4] thermal: Add new thermal trend type to support quick cooling

[R, Durgadoss]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 8003 bytes --]

Hi Amit/Rui,

> -----Original Message-----
> From: Zhang, Rui
> Sent: Friday, November 09, 2012 9:21 AM
> To: Amit Kachhap
> Cc: linux-pm@lists.linux-foundation.org; linux-samsung-
> soc@vger.kernel.org; linux-kernel@vger.kernel.org; R, Durgadoss;
> lenb@kernel.org; linux-acpi@vger.kernel.org; jonghwa3.lee@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.zhang@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.daniel@samsung.com>
> > >> Signed-off-by: Amit Daniel Kachhap <amit.kachhap@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 is stable */
> > >>       THERMAL_TREND_RAISING, /* temperature is raising */
> > >>       THERMAL_TREND_DROPPING, /* temperature is dropping */
> > >> +     THERMAL_TREND_RAISE_FULL, /* Apply highest cooling action*/
> > >> +     THERMAL_TREND_DROP_FULL, /* Apply lowest cooling action*/
> > >>  };
> > >>
> > >>  /* Events supported by Thermal Netlink */
> > >
> > >
> 

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH 1/4] thermal: Add new thermal trend type to support quick cooling

[Amit Kachhap]

On 9 November 2012 09:21, Zhang Rui <rui.zhang@intel.com> wrote:
> On Thu, 2012-11-08 at 11:56 +0530, Amit Kachhap wrote:
>> On 8 November 2012 11:31, Zhang Rui <rui.zhang@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.

I am not sure if it is the best way . How do you feel ?

> what do you think?
>
> 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.daniel@samsung.com>
>> >> Signed-off-by: Amit Daniel Kachhap <amit.kachhap@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 is stable */
>> >>       THERMAL_TREND_RAISING, /* temperature is raising */
>> >>       THERMAL_TREND_DROPPING, /* temperature is dropping */
>> >> +     THERMAL_TREND_RAISE_FULL, /* Apply highest cooling action*/
>> >> +     THERMAL_TREND_DROP_FULL, /* Apply lowest cooling action*/
>> >>  };
>> >>
>> >>  /* Events supported by Thermal Netlink */
>> >
>> >
>
>

Re: [PATCH 1/4] thermal: Add new thermal trend type to support quick cooling

[Zhang Rui]

On Fri, 2012-11-09 at 11:51 +0530, Amit Kachhap wrote:
> On 9 November 2012 09:21, Zhang Rui <rui.zhang@intel.com> wrote:
> > On Thu, 2012-11-08 at 11:56 +0530, Amit Kachhap wrote:
> >> On 8 November 2012 11:31, Zhang Rui <rui.zhang@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.
> 
> I am not sure if it is the best way . How do you feel ?
> 
this sounds good to me.
I'd like to see Durga' comments on this as well.

thanks,
rui
> > what do you think?
> >
> > 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.daniel@samsung.com>
> >> >> Signed-off-by: Amit Daniel Kachhap <amit.kachhap@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 is stable */
> >> >>       THERMAL_TREND_RAISING, /* temperature is raising */
> >> >>       THERMAL_TREND_DROPPING, /* temperature is dropping */
> >> >> +     THERMAL_TREND_RAISE_FULL, /* Apply highest cooling action*/
> >> >> +     THERMAL_TREND_DROP_FULL, /* Apply lowest cooling action*/
> >> >>  };
> >> >>
> >> >>  /* Events supported by Thermal Netlink */
> >> >
> >> >
> >
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@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

[R, Durgadoss]

Hi Amit/Rui,

> -----Original Message-----
> From: Amit Kachhap [mailto:amit.kachhap@linaro.org]
> Sent: Friday, November 09, 2012 11:52 AM
> To: Zhang, Rui
> Cc: linux-pm@lists.linux-foundation.org; linux-samsung-
> soc@vger.kernel.org; linux-kernel@vger.kernel.org; R, Durgadoss;
> lenb@kernel.org; linux-acpi@vger.kernel.org; jonghwa3.lee@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.zhang@intel.com> wrote:
> > On Thu, 2012-11-08 at 11:56 +0530, Amit Kachhap wrote:
> >> On 8 November 2012 11:31, Zhang Rui <rui.zhang@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

RE: [PATCH 1/4] thermal: Add new thermal trend type to support quick cooling

[Zhang Rui]

On Sun, 2012-11-11 at 22:55 -0700, R, Durgadoss wrote:
> Hi Amit/Rui,
> 
> > -----Original Message-----
> > From: Amit Kachhap [mailto:amit.kachhap@linaro.org]
> > Sent: Friday, November 09, 2012 11:52 AM
> > To: Zhang, Rui
> > Cc: linux-pm@lists.linux-foundation.org; linux-samsung-
> > soc@vger.kernel.org; linux-kernel@vger.kernel.org; R, Durgadoss;
> > lenb@kernel.org; linux-acpi@vger.kernel.org; jonghwa3.lee@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.zhang@intel.com> wrote:
> > > On Thu, 2012-11-08 at 11:56 +0530, Amit Kachhap wrote:
> > >> On 8 November 2012 11:31, Zhang Rui <rui.zhang@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.

thanks,
rui

RE: [PATCH 1/4] thermal: Add new thermal trend type to support quick cooling

[R, Durgadoss]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 3752 bytes --]



> -----Original Message-----
> From: Zhang, Rui
> Sent: Monday, November 12, 2012 12:03 PM
> To: R, Durgadoss
> Cc: Amit Kachhap; linux-pm@lists.linux-foundation.org; linux-samsung-
> soc@vger.kernel.org; linux-kernel@vger.kernel.org; lenb@kernel.org; linux-
> acpi@vger.kernel.org; jonghwa3.lee@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.kachhap@linaro.org]
> > > Sent: Friday, November 09, 2012 11:52 AM
> > > To: Zhang, Rui
> > > Cc: linux-pm@lists.linux-foundation.org; linux-samsung-
> > > soc@vger.kernel.org; linux-kernel@vger.kernel.org; R, Durgadoss;
> > > lenb@kernel.org; linux-acpi@vger.kernel.org;
> jonghwa3.lee@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.zhang@intel.com> wrote:
> > > > On Thu, 2012-11-08 at 11:56 +0530, Amit Kachhap wrote:
> > > >> On 8 November 2012 11:31, Zhang Rui <rui.zhang@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Ç+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

RE: [PATCH 0/4] thermal: Add support for interrupt based notification to thermal layer

[Zhang, Rui]

Hi, Amit,

As THERMAL_TREND_RAISE_FULL/THERMAL_TREND_DROP_FULL
has been introduced to thermal next tree,
I'd like to get your plan about this patch set?

Thanks,
Rui

> -----Original Message-----
> From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-
> owner@vger.kernel.org] On Behalf Of Amit Daniel Kachhap
> Sent: Thursday, November 08, 2012 12:26 PM
> To: linux-pm@lists.linux-foundation.org
> Cc: linux-samsung-soc@vger.kernel.org; linux-kernel@vger.kernel.org; R,
> Durgadoss; lenb@kernel.org; Zhang, Rui; linux-acpi@vger.kernel.org;
> amit.kachhap@linaro.org; jonghwa3.lee@samsung.com
> Subject: [PATCH 0/4] thermal: Add support for interrupt based
> notification to thermal layer
> Importance: High
> 
> The patch submitted by Jonghwa Lee
> (https://patchwork.kernel.org/patch/1683441/)
> adds support for interrupt based notification to thermal layer. This is
> a good feature but the current thermal framework needs polling/regular
> notification for invoking suitable cooling action. So adding 2 new
> thermal trend type to implement this feature.
> 
> All these patches are based on thermal maintainer next tree.
> git://git.kernel.org/pub/scm/linux/kernel/git/rzhang/linux.git next
> 
> Amit Daniel Kachhap (3):
>   thermal: Add new thermal trend type to support quick cooling
>   thermal: exynos: Miscellaneous fixes to support falling threshold
>     interrupt
>   thermal: exynos: Use the new thermal trend type for quick cooling
>     action.
> 
> Jonghwa Lee (1):
>   Thermal: exynos: Add support for temperature falling interrupt.
> 
>  drivers/thermal/exynos_thermal.c             |  105 +++++++++++++++---
> --------
>  drivers/thermal/step_wise.c                  |   19 ++++-
>  include/linux/platform_data/exynos_thermal.h |    3 +
>  include/linux/thermal.h                      |    2 +
>  4 files changed, 80 insertions(+), 49 deletions(-)
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi"
> in the body of a message to majordomo@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

[Zhang Rui]

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. 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.daniel@samsung.com>
> Signed-off-by: Amit Daniel Kachhap <amit.kachhap@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;
>  
what do you expect to happen if the trend is THERMAL_TREND_RAISE_FULL at
this time?

thanks,
rui

Re: [PATCH 1/4] thermal: Add new thermal trend type to support quick cooling

[Amit Kachhap]

On 22 November 2012 06:52, Zhang Rui <rui.zhang@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. 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.daniel@samsung.com>
>> Signed-off-by: Amit Daniel Kachhap <amit.kachhap@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;
>>
> what do you expect to happen if the trend is THERMAL_TREND_RAISE_FULL at
> this time?
>
Hi Rui,

I suppose this is dethrotle routine and hence this will be called when
only drop in temperature happens. Also I did not used get_target_state
here because I thought it might cause regression in the other existing
thermal drivers(I am not sure) But I guess calling get_target_state is
the good way to know next target state and is fine if you agree.
Also one suggestion, 2 functions for throttle/dethrottle can be merged
as both look same and just get_target_state can be used in that
function

Thanks,
Amit Daniel

> thanks,
> rui
>

Re: [PATCH 1/4] thermal: Add new thermal trend type to support quick cooling

[Zhang Rui]

On Thu, 2012-11-22 at 10:11 +0530, Amit Kachhap wrote:
> On 22 November 2012 06:52, Zhang Rui <rui.zhang@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. 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.daniel@samsung.com>
> >> Signed-off-by: Amit Daniel Kachhap <amit.kachhap@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;
> >>
> > what do you expect to happen if the trend is THERMAL_TREND_RAISE_FULL at
> > this time?
> >
> Hi Rui,
> 
> I suppose this is dethrotle routine and hence this will be called when
> only drop in temperature happens. Also I did not used get_target_state
> here because I thought it might cause regression in the other existing
> thermal drivers(I am not sure) But I guess calling get_target_state is
> the good way to know next target state and is fine if you agree.
> Also one suggestion, 2 functions for throttle/dethrottle can be merged
> as both look same and just get_target_state can be used in that
> function
> 
agree.
patches have been refreshed, please review.

thanks,
rui

Re: [PATCH 1/4] thermal: Add new thermal trend type to support quick cooling

[Amit Kachhap]

On 22 November 2012 13:42, Zhang Rui <rui.zhang@intel.com> wrote:
> On Thu, 2012-11-22 at 10:11 +0530, Amit Kachhap wrote:
>> On 22 November 2012 06:52, Zhang Rui <rui.zhang@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. 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.daniel@samsung.com>
>> >> Signed-off-by: Amit Daniel Kachhap <amit.kachhap@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;
>> >>
>> > what do you expect to happen if the trend is THERMAL_TREND_RAISE_FULL at
>> > this time?
>> >
>> Hi Rui,
>>
>> I suppose this is dethrotle routine and hence this will be called when
>> only drop in temperature happens. Also I did not used get_target_state
>> here because I thought it might cause regression in the other existing
>> thermal drivers(I am not sure) But I guess calling get_target_state is
>> the good way to know next target state and is fine if you agree.
>> Also one suggestion, 2 functions for throttle/dethrottle can be merged
>> as both look same and just get_target_state can be used in that
>> function
>>
> agree.
> patches have been refreshed, please review.

Thanks Rui, Your patches looks nice. I will re-base my patches against
your implementation and submit them shortly.

Thanks,
Amit D
>
> thanks,
> rui
>

Re: [PATCH 1/4] thermal: Add new thermal trend type to support quick cooling

[Zhang Rui]

On Fri, 2012-11-23 at 09:35 +0530, Amit Kachhap wrote:
> On 22 November 2012 13:42, Zhang Rui <rui.zhang@intel.com> wrote:
> > On Thu, 2012-11-22 at 10:11 +0530, Amit Kachhap wrote:
> >> On 22 November 2012 06:52, Zhang Rui <rui.zhang@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. 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.daniel@samsung.com>
> >> >> Signed-off-by: Amit Daniel Kachhap <amit.kachhap@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;
> >> >>
> >> > what do you expect to happen if the trend is THERMAL_TREND_RAISE_FULL at
> >> > this time?
> >> >
> >> Hi Rui,
> >>
> >> I suppose this is dethrotle routine and hence this will be called when
> >> only drop in temperature happens. Also I did not used get_target_state
> >> here because I thought it might cause regression in the other existing
> >> thermal drivers(I am not sure) But I guess calling get_target_state is
> >> the good way to know next target state and is fine if you agree.
> >> Also one suggestion, 2 functions for throttle/dethrottle can be merged
> >> as both look same and just get_target_state can be used in that
> >> function
> >>
> > agree.
> > patches have been refreshed, please review.
> 
> Thanks Rui, Your patches looks nice. I will re-base my patches against
> your implementation and submit them shortly.
> 
great. please rebase your patch on top of thermal-thermal tree.

thanks,
rui