| Message ID | 1352348786-24255-2-git-send-email-amit.kachhap@linaro.org (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Zhang Rui |
| Headers | show |
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 */ -- 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
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 */ > > -- 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
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 */ > > > > -- 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
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 */ > > > > -- 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
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 */ > > > > > > >
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 */ >> > >> > > > -- 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
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 -- 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
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 -- 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
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 -- 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
DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogWmhhbmcsIFJ1aQ0KPiBT ZW50OiBNb25kYXksIE5vdmVtYmVyIDEyLCAyMDEyIDEyOjAzIFBNDQo+IFRvOiBSLCBEdXJnYWRv c3MNCj4gQ2M6IEFtaXQgS2FjaGhhcDsgbGludXgtcG1AbGlzdHMubGludXgtZm91bmRhdGlvbi5v cmc7IGxpbnV4LXNhbXN1bmctDQo+IHNvY0B2Z2VyLmtlcm5lbC5vcmc7IGxpbnV4LWtlcm5lbEB2 Z2VyLmtlcm5lbC5vcmc7IGxlbmJAa2VybmVsLm9yZzsgbGludXgtDQo+IGFjcGlAdmdlci5rZXJu ZWwub3JnOyBqb25naHdhMy5sZWVAc2Ftc3VuZy5jb20NCj4gU3ViamVjdDogUkU6IFtQQVRDSCAx LzRdIHRoZXJtYWw6IEFkZCBuZXcgdGhlcm1hbCB0cmVuZCB0eXBlIHRvIHN1cHBvcnQNCj4gcXVp Y2sgY29vbGluZw0KPiANCj4gT24gU3VuLCAyMDEyLTExLTExIGF0IDIyOjU1IC0wNzAwLCBSLCBE dXJnYWRvc3Mgd3JvdGU6DQo+ID4gSGkgQW1pdC9SdWksDQo+ID4NCj4gPiA+IC0tLS0tT3JpZ2lu YWwgTWVzc2FnZS0tLS0tDQo+ID4gPiBGcm9tOiBBbWl0IEthY2hoYXAgW21haWx0bzphbWl0Lmth Y2hoYXBAbGluYXJvLm9yZ10NCj4gPiA+IFNlbnQ6IEZyaWRheSwgTm92ZW1iZXIgMDksIDIwMTIg MTE6NTIgQU0NCj4gPiA+IFRvOiBaaGFuZywgUnVpDQo+ID4gPiBDYzogbGludXgtcG1AbGlzdHMu bGludXgtZm91bmRhdGlvbi5vcmc7IGxpbnV4LXNhbXN1bmctDQo+ID4gPiBzb2NAdmdlci5rZXJu ZWwub3JnOyBsaW51eC1rZXJuZWxAdmdlci5rZXJuZWwub3JnOyBSLCBEdXJnYWRvc3M7DQo+ID4g PiBsZW5iQGtlcm5lbC5vcmc7IGxpbnV4LWFjcGlAdmdlci5rZXJuZWwub3JnOw0KPiBqb25naHdh My5sZWVAc2Ftc3VuZy5jb20NCj4gPiA+IFN1YmplY3Q6IFJlOiBbUEFUQ0ggMS80XSB0aGVybWFs OiBBZGQgbmV3IHRoZXJtYWwgdHJlbmQgdHlwZSB0bw0KPiBzdXBwb3J0DQo+ID4gPiBxdWljayBj b29saW5nDQo+ID4gPg0KPiA+ID4gT24gOSBOb3ZlbWJlciAyMDEyIDA5OjIxLCBaaGFuZyBSdWkg PHJ1aS56aGFuZ0BpbnRlbC5jb20+IHdyb3RlOg0KPiA+ID4gPiBPbiBUaHUsIDIwMTItMTEtMDgg YXQgMTE6NTYgKzA1MzAsIEFtaXQgS2FjaGhhcCB3cm90ZToNCj4gPiA+ID4+IE9uIDggTm92ZW1i ZXIgMjAxMiAxMTozMSwgWmhhbmcgUnVpIDxydWkuemhhbmdAaW50ZWwuY29tPiB3cm90ZToNCj4g PiA+ID4+ID4gT24gVGh1LCAyMDEyLTExLTA4IGF0IDA5OjU2ICswNTMwLCBBbWl0IERhbmllbCBL YWNoaGFwIHdyb3RlOg0KPiA+ID4gPj4gPj4gVGhpcyBtb2RpZmljYXRpb24gYWRkcyAyIG5ldyB0 aGVybWFsIHRyZW5kIHR5cGUNCj4gPiA+IFRIRVJNQUxfVFJFTkRfUkFJU0VfRlVMTA0KPiA+ID4g Pj4gPj4gYW5kIFRIRVJNQUxfVFJFTkRfRFJPUF9GVUxMLiBUaGlzIHRoZXJtYWwgdHJlbmQgY2Fu IGJlIHVzZWQNCj4gdG8NCj4gPiA+IHF1aWNrbHkNCj4gPiA+ID4+ID4+IGp1bXAgdG8gdGhlIHVw cGVyIG9yIGxvd2VyIGNvb2xpbmcgbGV2ZWwgaW5zdGVhZCBvZiBpbmNyZW1lbnRhbA0KPiA+ID4g aW5jcmVhc2UNCj4gPiA+ID4+ID4+IG9yIGRlY3JlYXNlLg0KPiA+ID4gPj4gPg0KPiA+ID4gPj4g PiBJTU8sIHdoYXQgd2UgbmVlZCBpcyBhIG5ldyBtb3JlIGFnZ3Jlc3NpdmUgY29vbGluZyBnb3Zl cm5vcg0KPiB3aGljaA0KPiA+ID4gYWx3YXlzDQo+ID4gPiA+PiA+IHVzZXMgdXBwZXIgbGltaXQg d2hlbiB0aGUgdGVtcGVyYXR1cmUgaXMgcmFpc2luZyBhbmQgbG93ZXIgbGltaXQNCj4gd2hlbg0K PiA+ID4gPj4gPiB0aGUgdGVtcGVyYXR1cmUgaXMgZHJvcHBpbmcuDQo+ID4gPiA+PiBZZXMgSSBh Z3JlZSB0aGF0IGEgbmV3IGFnZ3Jlc3NpdmUgZ292ZXJub3IgaXMgdGhlIGJlc3QgYXBwcm9hY2gg YnV0DQo+ID4gPiA+PiB0aGVuIGkgdGhvdWdodCBhZGRpbmcgYSBuZXcgdHJlbmQgdHlwZSBpcyBh IHNpbXBsZSBzb2x1dGlvbiB0byBhY2hpZXZlDQo+ID4gPiA+PiB0aGlzIGFuZCBzaW5jZSBtb3N0 IG9mIHRoZSBnb3Zlcm5vciBsb2dpYyBtaWdodCBiZSBzYW1lIGFzIHRoZQ0KPiA+ID4gPj4gc3Rl cC13aXNlIGdvdmVybm9yLiBJIGhhdmUgbm8gb2JqZWN0aW9uIGluIGRvaW5nIGl0IHRocm91Z2gg Z292ZXJub3IuDQo+ID4gPiA+PiA+DQo+ID4gPiA+IGhtbW0sDQo+ID4gPiA+IEkgdGhpbmsgYSBt b3JlIHByb3BlciB3YXkgaXMgdG8gc2V0IHRoZSBjb29saW5nIHN0YXRlIHRvIHVwcGVyIGxpbWl0 DQo+ID4gPiA+IHdoZW4gaXQgb3ZlcmhlYXRzIGFuZCByZWR1Y2UgdGhlIGNvb2xpbmcgc3RhdGUg c3RlcCBieSBzdGVwIHdoZW4gdGhlDQo+ID4gPiA+IHRlbXBlcmF0dXJlIGRyb3BzLg0KPiA+ID4N Cj4gPiA+IE5vIGFjdHVhbGx5IEkgd2FzIHRoaW5raW5nIG9mIGhhdmluZyBhICBzaW1wbGUgZ292 ZXJub3Igd2l0aCBhIGZlYXR1cmUNCj4gPiA+IGxpa2UgaXQgb25seSBzZXRzIHRvIHVwcGVyIGxl dmVsIGFuZCBsb3dlciBsZXZlbC4gQWxzbyBzaW5jZSB0aGUNCj4gPiA+IHRlbXBlcmF0dXJlIHNl bnNvciBpcyBjYXBhYmxlIG9mIGludGVycnVwdGluZyBmb3IgYm90aCBpbmNyZWFzZSBpbg0KPiA+ ID4gdGhyZXNob2xkKHNheSAxMDBDKSAgYW5kIGZhbGwgaW4gdGhyZXNob2xkIChzYXkgOTBDKSwg c28gcG9sbGluZyBvcg0KPiA+ID4gc3RlcCBpbmNyZW1lbnRzIGlzIG5vdCBuZWVkZWQgYXQgYWxs Lg0KPiA+ID4gQ3VycmVudGx5IHN0ZXB3aXNlIGdvdmVybm9yIGdvdmVybm9yIGRvZXMgdGhhdCBz byB3ZSBtaWdodCBjaGFuZ2UgdGhlDQo+ID4gPiBtYWNybyBuYW1lIGFzLA0KPiA+ID4gVEhFUk1B TF9UUkVORF9SQUlTRV9TVEVQLA0KPiA+ID4gVEhFUk1BTF9UUkVORF9EUk9QX1NURVAsDQo+ID4g PiBUSEVSTUFMX1RSRU5EX1JBSVNFX01BWCwNCj4gPiA+IFRIRVJNQUxfVFJFTkRfRFJPUF9NQVgs DQo+ID4gPg0KPiA+ID4gYW5kIGZpbGUgc3RlcF93aXNlLmMgY2FuIGJlIG5hbWVkIGFzIHN0YXRl X3dpc2UuYyBvciB0cmVuZF93aXNlLmMuDQo+ID4NCj4gPiBZZXMsIGluIHRoaXMgcGFydGljdWxh ciBjYXNlLCB3ZSBuZWl0aGVyIG5lZWQgdG8gcG9sbCBub3IgZG8gc3RlcCB3aXNlDQo+ID4gb3Bl cmF0aW9ucy4gQnV0LCBtb3N0IG9mIHRoZSBvdGhlciBzZW5zb3JzIG5lZWQgYXQgbGVhc3Qgb25l IG9mIHRoZW0uDQo+ID4NCj4gPiBTbywgSSB0aGluayB3ZSBjYW4gdHJ5IGl0IHRoaXMgd2F5Og0K PiA+IAlpZiAoc2Vuc29yIHN1cHBvcnRzIGludGVycnVwdCkgew0KPiA+IAkJJ2Fsd2F5cycgdXNl IFJBSVNFX01BWCBhbmQgRFJPUF9NQVg7DQo+ID4gCX0gZWxzZSB7DQo+ID4gCQlEbyBTdGVwIHdp c2Ugb3BlcmF0aW9ucw0KPiA+IAl9DQo+ID4NCj4gd2h5IHNob3VsZCB0aGUgZ2VuZXJpYyB0aGVy bWFsIGxheWVyIGJlIGF3YXJlIG9mIHRoaXM/DQo+IA0KPiBJTU8sIGl0IGlzIHRoZSBwbGF0Zm9y bSB0aGVybWFsIGRyaXZlciB0aGF0IGlzIHJlc3BvbnNpYmxlIGZvciByZXR1cm5pbmcNCj4gVEhF Uk1BTF9UUkVORF9SQUlTRV9TVEVQIG9yIFRIRVJNQUxfVFJFTkRfUkFJU0VfTUFYLg0KPiANCj4g YW5kIHRoZSBzdGVwX3dpc2UgZ292ZXJub3IganVzdCB0YWtlcyBhY3Rpb24gYmFzZWQgb24gdGhl IHJldHVybiB2YWx1ZQ0KPiBvZiAuZ2V0X3RyZW5kKCkgY2FsbGJhY2suDQoNClllcywgYWdyZWUg d2l0aCB0aGUgZmxvdyAuLg0KDQpUaGFua3MsDQpEdXJnYQ0K -- 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
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 -- 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
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 > -- 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
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 -- 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
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 > -- 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
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 -- 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
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 */