| Message ID | 1342679480-5336-10-git-send-email-rui.zhang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thursday, July 19, 2012, Zhang Rui wrote: > This function is used to update the cooling state of > all the cooling devices that are binded to an active trip point. s/binded/bound/ > This will be used for passive cooling as well, in the future patches. > as both active and passive cooling can share the same algorithm, > which is > > 1. if the temperature is higher than a trip point, > a. if the trend is THERMAL_TREND_RAISING, use higher cooling > state for this trip point > b. if the trend is THERMAL_TREND_DROPPING, use lower cooling > state for this trip point > > 2. if the temperature is lower than a trip point, use lower > cooling state for this trip point. > > Signed-off-by: Zhang Rui <rui.zhang@intel.com> > --- > drivers/acpi/thermal.c | 7 +++- > drivers/thermal/thermal_sys.c | 91 +++++++++++++++++++++++++++++------------ > 2 files changed, 71 insertions(+), 27 deletions(-) > > diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c > index 73e335f..14c4879 100644 > --- a/drivers/acpi/thermal.c > +++ b/drivers/acpi/thermal.c > @@ -715,7 +715,12 @@ static int thermal_get_trend(struct thermal_zone_device *thermal, > if (thermal_get_trip_type(thermal, trip, &type)) > return -EINVAL; > > - /* Only PASSIVE trip points need TREND */ > + if (type == THERMAL_TRIP_ACTIVE) { > + /* aggressive active cooling */ > + *trend = THERMAL_TREND_RAISING; > + return 0; Please move that into thermal_zone_trip_update() directly, unless you need it elsewhere. > + } > + > if (type != THERMAL_TRIP_PASSIVE) > return -EINVAL; > > diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c > index 59af3b8..011faba 100644 > --- a/drivers/thermal/thermal_sys.c > +++ b/drivers/thermal/thermal_sys.c > @@ -1076,6 +1076,70 @@ void thermal_cooling_device_unregister(struct > } > EXPORT_SYMBOL(thermal_cooling_device_unregister); > > +/* > + * Cooling algorithm for active trip points > + * > + * 1. if the temperature is higher than a trip point, > + * a. if the trend is THERMAL_TREND_RAISING, use higher cooling > + * state for this trip point > + * b. if the trend is THERMAL_TREND_DROPPING, use lower cooling > + * state for this trip point > + * > + * 2. if the temperature is lower than a trip point, use lower > + * cooling state for this trip point > + * > + * Note that this behaves the same as the previous passive cooling > + * algorithm. > + */ > + > +static void thermal_zone_trip_update(struct thermal_zone_device *tz, > + int trip, long temp) > +{ > + struct thermal_cooling_device_instance *instance; > + struct thermal_cooling_device *cdev = NULL; > + unsigned long cur_state, max_state; > + long trip_temp; > + enum thermal_trend trend; > + > + tz->ops->get_trip_temp(tz, trip, &trip_temp); > + > + if (temp >= trip_temp) { > + thermal_get_trend(tz, trip, &trend); > + > + list_for_each_entry(instance, &tz->cooling_devices, node) { > + if (instance->trip != trip) > + continue; > + > + cdev = instance->cdev; > + > + cdev->ops->get_cur_state(cdev, &cur_state); > + cdev->ops->get_max_state(cdev, &max_state); > + > + if (trend == THERMAL_TREND_RAISING) { > + cur_state = cur_state < instance->upper ? > + (cur_state + 1) : instance->upper; > + } else if (trend == THERMAL_TREND_DROPPING) { > + cur_state = cur_state > instance->lower ? > + (cur_state - 1) : instance->lower; > + } > + cdev->ops->set_cur_state(cdev, cur_state); > + } > + } else { /* below trip */ > + list_for_each_entry(instance, &tz->cooling_devices, node) { > + if (instance->trip != trip) > + continue; > + > + cdev = instance->cdev; > + cdev->ops->get_cur_state(cdev, &cur_state); > + > + cur_state = cur_state > instance->lower ? > + (cur_state - 1) : instance->lower; > + cdev->ops->set_cur_state(cdev, cur_state); > + } > + } > + > + return; > +} > /** > * thermal_zone_device_update - force an update of a thermal zone's state > * @ttz: the thermal zone to update > @@ -1086,9 +1150,6 @@ void thermal_zone_device_update(struct thermal_zone_device *tz) > int count, ret = 0; > long temp, trip_temp; > enum thermal_trip_type trip_type; > - struct thermal_cooling_device_instance *instance; > - struct thermal_cooling_device *cdev; > - unsigned long cur_state, max_state; > > mutex_lock(&tz->lock); > > @@ -1124,29 +1185,7 @@ void thermal_zone_device_update(struct thermal_zone_device *tz) > tz->ops->notify(tz, count, trip_type); > break; > case THERMAL_TRIP_ACTIVE: > - list_for_each_entry(instance, &tz->cooling_devices, > - node) { > - if (instance->trip != count) > - continue; > - > - cdev = instance->cdev; > - > - cdev->ops->get_cur_state(cdev, &cur_state); > - cdev->ops->get_max_state(cdev, &max_state); > - > - if (temp >= trip_temp) > - cur_state = > - cur_state < instance->upper ? > - (cur_state + 1) : > - instance->upper; > - else > - cur_state = > - cur_state > instance->lower ? > - (cur_state - 1) : > - instance->lower; > - > - cdev->ops->set_cur_state(cdev, cur_state); > - } > + thermal_zone_trip_update(tz, count, temp); > break; > case THERMAL_TRIP_PASSIVE: > if (temp >= trip_temp || tz->passive) > Thanks, Rafael -- 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 ?, 2012-07-19 at 23:19 +0200, Rafael J. Wysocki wrote: > On Thursday, July 19, 2012, Zhang Rui wrote: > > This function is used to update the cooling state of > > all the cooling devices that are binded to an active trip point. > > s/binded/bound/ > got it. > > This will be used for passive cooling as well, in the future patches. > > as both active and passive cooling can share the same algorithm, > > which is > > > > 1. if the temperature is higher than a trip point, > > a. if the trend is THERMAL_TREND_RAISING, use higher cooling > > state for this trip point > > b. if the trend is THERMAL_TREND_DROPPING, use lower cooling > > state for this trip point > > > > 2. if the temperature is lower than a trip point, use lower > > cooling state for this trip point. > > > > Signed-off-by: Zhang Rui <rui.zhang@intel.com> > > --- > > drivers/acpi/thermal.c | 7 +++- > > drivers/thermal/thermal_sys.c | 91 +++++++++++++++++++++++++++++------------ > > 2 files changed, 71 insertions(+), 27 deletions(-) > > > > diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c > > index 73e335f..14c4879 100644 > > --- a/drivers/acpi/thermal.c > > +++ b/drivers/acpi/thermal.c > > @@ -715,7 +715,12 @@ static int thermal_get_trend(struct thermal_zone_device *thermal, > > if (thermal_get_trip_type(thermal, trip, &type)) > > return -EINVAL; > > > > - /* Only PASSIVE trip points need TREND */ > > + if (type == THERMAL_TRIP_ACTIVE) { > > + /* aggressive active cooling */ > > + *trend = THERMAL_TREND_RAISING; > > + return 0; > > Please move that into thermal_zone_trip_update() directly, unless you > need it elsewhere. > IMO, I can say that ACPI thermal wants aggressive active cooling, as it will never spin down the fan when the temperature is higher than the trip point. But I'm not sure if this is true for other platforms. say if a fan have 5 speeds. it may want to run at the maximum speed when the temperature is raising, but if the temperature begins to drop, it may want to spin down the fan a little for some reason, say, power, noise, etc. so I want the platform thermal drivers to have the maximum flexibility. 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 ?, 2012-07-24 at 12:27 +0530, Amit Kachhap wrote: > > > On 19 July 2012 12:01, Zhang Rui <rui.zhang@intel.com> wrote: > This function is used to update the cooling state of > all the cooling devices that are binded to an active trip > point. > > This will be used for passive cooling as well, in the future > patches. > as both active and passive cooling can share the same > algorithm, > which is > > 1. if the temperature is higher than a trip point, > a. if the trend is THERMAL_TREND_RAISING, use higher > cooling > state for this trip point > b. if the trend is THERMAL_TREND_DROPPING, use lower > cooling > state for this trip point > > 2. if the temperature is lower than a trip point, use lower > cooling state for this trip point. > > Hi Rui, > > Your patches looks useful. > For my platform I need to have get_trend called even in the case when > current temp is less than the trip point and then use > THERMAL_TREND_DROPPING to actually lower the cooling state. > hmm, why? in the current cooling algorithm, when the temperature is lower than the trip point, the cooling state is decreased by 1 every time, unconditionally, isn't this what you want? thanks, rui > Thanks, > Amit > > > Signed-off-by: Zhang Rui <rui.zhang@intel.com> > --- > drivers/acpi/thermal.c | 7 +++- > drivers/thermal/thermal_sys.c | 91 > +++++++++++++++++++++++++++++------------ > 2 files changed, 71 insertions(+), 27 deletions(-) > > diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c > index 73e335f..14c4879 100644 > --- a/drivers/acpi/thermal.c > +++ b/drivers/acpi/thermal.c > @@ -715,7 +715,12 @@ static int thermal_get_trend(struct > thermal_zone_device *thermal, > if (thermal_get_trip_type(thermal, trip, &type)) > return -EINVAL; > > - /* Only PASSIVE trip points need TREND */ > + if (type == THERMAL_TRIP_ACTIVE) { > + /* aggressive active cooling */ > + *trend = THERMAL_TREND_RAISING; > + return 0; > + } > + > if (type != THERMAL_TRIP_PASSIVE) > return -EINVAL; > > diff --git a/drivers/thermal/thermal_sys.c > b/drivers/thermal/thermal_sys.c > index 59af3b8..011faba 100644 > --- a/drivers/thermal/thermal_sys.c > +++ b/drivers/thermal/thermal_sys.c > @@ -1076,6 +1076,70 @@ void > thermal_cooling_device_unregister(struct > } > EXPORT_SYMBOL(thermal_cooling_device_unregister); > > +/* > + * Cooling algorithm for active trip points > + * > + * 1. if the temperature is higher than a trip point, > + * a. if the trend is THERMAL_TREND_RAISING, use higher > cooling > + * state for this trip point > + * b. if the trend is THERMAL_TREND_DROPPING, use lower > cooling > + * state for this trip point > + * > + * 2. if the temperature is lower than a trip point, use > lower > + * cooling state for this trip point > + * > + * Note that this behaves the same as the previous passive > cooling > + * algorithm. > + */ > + > +static void thermal_zone_trip_update(struct > thermal_zone_device *tz, > + int trip, long temp) > +{ > + struct thermal_cooling_device_instance *instance; > + struct thermal_cooling_device *cdev = NULL; > + unsigned long cur_state, max_state; > + long trip_temp; > + enum thermal_trend trend; > + > + tz->ops->get_trip_temp(tz, trip, &trip_temp); > + > + if (temp >= trip_temp) { > + thermal_get_trend(tz, trip, &trend); > + > + list_for_each_entry(instance, > &tz->cooling_devices, node) { > + if (instance->trip != trip) > + continue; > + > + cdev = instance->cdev; > + > + cdev->ops->get_cur_state(cdev, > &cur_state); > + cdev->ops->get_max_state(cdev, > &max_state); > + > + if (trend == THERMAL_TREND_RAISING) { > + cur_state = cur_state < > instance->upper ? > + (cur_state + 1) : > instance->upper; > + } else if (trend == > THERMAL_TREND_DROPPING) { > + cur_state = cur_state > > instance->lower ? > + (cur_state - 1) : > instance->lower; > + } > + cdev->ops->set_cur_state(cdev, > cur_state); > + } > + } else { /* below trip */ > + list_for_each_entry(instance, > &tz->cooling_devices, node) { > + if (instance->trip != trip) > + continue; > + > + cdev = instance->cdev; > + cdev->ops->get_cur_state(cdev, > &cur_state); > + > + cur_state = cur_state > > instance->lower ? > + (cur_state - 1) : > instance->lower; > + cdev->ops->set_cur_state(cdev, > cur_state); > + } > + } > + > + return; > +} > /** > * thermal_zone_device_update - force an update of a thermal > zone's state > * @ttz: the thermal zone to update > @@ -1086,9 +1150,6 @@ void thermal_zone_device_update(struct > thermal_zone_device *tz) > int count, ret = 0; > long temp, trip_temp; > enum thermal_trip_type trip_type; > - struct thermal_cooling_device_instance *instance; > - struct thermal_cooling_device *cdev; > - unsigned long cur_state, max_state; > > mutex_lock(&tz->lock); > > @@ -1124,29 +1185,7 @@ void thermal_zone_device_update(struct > thermal_zone_device *tz) > tz->ops->notify(tz, > count, trip_type); > break; > case THERMAL_TRIP_ACTIVE: > - list_for_each_entry(instance, > &tz->cooling_devices, > - node) { > - if (instance->trip != count) > - continue; > - > - cdev = instance->cdev; > - > - cdev->ops->get_cur_state(cdev, > &cur_state); > - cdev->ops->get_max_state(cdev, > &max_state); > - > - if (temp >= trip_temp) > - cur_state = > - cur_state < > instance->upper ? > - (cur_state + > 1) : > - > instance->upper; > - else > - cur_state = > - cur_state > > instance->lower ? > - (cur_state - > 1) : > - > instance->lower; > - > - cdev->ops->set_cur_state(cdev, > cur_state); > - } > + thermal_zone_trip_update(tz, count, > temp); > break; > case THERMAL_TRIP_PASSIVE: > if (temp >= trip_temp || tz->passive) > -- > 1.7.9.5 > > -- 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 19 July 2012 12:01, Zhang Rui <rui.zhang@intel.com> wrote: > > This function is used to update the cooling state of > all the cooling devices that are binded to an active trip point. > > This will be used for passive cooling as well, in the future patches. > as both active and passive cooling can share the same algorithm, > which is > > 1. if the temperature is higher than a trip point, > a. if the trend is THERMAL_TREND_RAISING, use higher cooling > state for this trip point > b. if the trend is THERMAL_TREND_DROPPING, use lower cooling > state for this trip point > > 2. if the temperature is lower than a trip point, use lower > cooling state for this trip point. Hi Rui, Your patches looks useful. For my platform I need to have get_trend called even in the case when current temp is less than the trip point and then use THERMAL_TREND_DROPPING to actually lower the cooling state. Thanks, Amit > > Signed-off-by: Zhang Rui <rui.zhang@intel.com> > --- > drivers/acpi/thermal.c | 7 +++- > drivers/thermal/thermal_sys.c | 91 > +++++++++++++++++++++++++++++------------ > 2 files changed, 71 insertions(+), 27 deletions(-) > > diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c > index 73e335f..14c4879 100644 > --- a/drivers/acpi/thermal.c > +++ b/drivers/acpi/thermal.c > @@ -715,7 +715,12 @@ static int thermal_get_trend(struct > thermal_zone_device *thermal, > if (thermal_get_trip_type(thermal, trip, &type)) > return -EINVAL; > > - /* Only PASSIVE trip points need TREND */ > + if (type == THERMAL_TRIP_ACTIVE) { > + /* aggressive active cooling */ > + *trend = THERMAL_TREND_RAISING; > + return 0; > + } > + > if (type != THERMAL_TRIP_PASSIVE) > return -EINVAL; > > diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c > index 59af3b8..011faba 100644 > --- a/drivers/thermal/thermal_sys.c > +++ b/drivers/thermal/thermal_sys.c > @@ -1076,6 +1076,70 @@ void thermal_cooling_device_unregister(struct > } > EXPORT_SYMBOL(thermal_cooling_device_unregister); > > +/* > + * Cooling algorithm for active trip points > + * > + * 1. if the temperature is higher than a trip point, > + * a. if the trend is THERMAL_TREND_RAISING, use higher cooling > + * state for this trip point > + * b. if the trend is THERMAL_TREND_DROPPING, use lower cooling > + * state for this trip point > + * > + * 2. if the temperature is lower than a trip point, use lower > + * cooling state for this trip point > + * > + * Note that this behaves the same as the previous passive cooling > + * algorithm. > + */ > + > +static void thermal_zone_trip_update(struct thermal_zone_device *tz, > + int trip, long temp) > +{ > + struct thermal_cooling_device_instance *instance; > + struct thermal_cooling_device *cdev = NULL; > + unsigned long cur_state, max_state; > + long trip_temp; > + enum thermal_trend trend; > + > + tz->ops->get_trip_temp(tz, trip, &trip_temp); > + > + if (temp >= trip_temp) { > + thermal_get_trend(tz, trip, &trend); > + > + list_for_each_entry(instance, &tz->cooling_devices, node) > { > + if (instance->trip != trip) > + continue; > + > + cdev = instance->cdev; > + > + cdev->ops->get_cur_state(cdev, &cur_state); > + cdev->ops->get_max_state(cdev, &max_state); > + > + if (trend == THERMAL_TREND_RAISING) { > + cur_state = cur_state < instance->upper ? > + (cur_state + 1) : > instance->upper; > + } else if (trend == THERMAL_TREND_DROPPING) { > + cur_state = cur_state > instance->lower ? > + (cur_state - 1) : instance->lower; > + } > + cdev->ops->set_cur_state(cdev, cur_state); > + } > + } else { /* below trip */ > + list_for_each_entry(instance, &tz->cooling_devices, node) > { > + if (instance->trip != trip) > + continue; > + > + cdev = instance->cdev; > + cdev->ops->get_cur_state(cdev, &cur_state); > + > + cur_state = cur_state > instance->lower ? > + (cur_state - 1) : instance->lower; > + cdev->ops->set_cur_state(cdev, cur_state); > + } > + } > + > + return; > +} > /** > * thermal_zone_device_update - force an update of a thermal zone's state > * @ttz: the thermal zone to update > @@ -1086,9 +1150,6 @@ void thermal_zone_device_update(struct > thermal_zone_device *tz) > int count, ret = 0; > long temp, trip_temp; > enum thermal_trip_type trip_type; > - struct thermal_cooling_device_instance *instance; > - struct thermal_cooling_device *cdev; > - unsigned long cur_state, max_state; > > mutex_lock(&tz->lock); > > @@ -1124,29 +1185,7 @@ void thermal_zone_device_update(struct > thermal_zone_device *tz) > tz->ops->notify(tz, count, > trip_type); > break; > case THERMAL_TRIP_ACTIVE: > - list_for_each_entry(instance, > &tz->cooling_devices, > - node) { > - if (instance->trip != count) > - continue; > - > - cdev = instance->cdev; > - > - cdev->ops->get_cur_state(cdev, > &cur_state); > - cdev->ops->get_max_state(cdev, > &max_state); > - > - if (temp >= trip_temp) > - cur_state = > - cur_state < > instance->upper ? > - (cur_state + 1) : > - instance->upper; > - else > - cur_state = > - cur_state > > instance->lower ? > - (cur_state - 1) : > - instance->lower; > - > - cdev->ops->set_cur_state(cdev, cur_state); > - } > + thermal_zone_trip_update(tz, count, temp); > break; > case THERMAL_TRIP_PASSIVE: > if (temp >= trip_temp || tz->passive) > -- > 1.7.9.5 > -- 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 24 July 2012 12:41, Zhang Rui <rui.zhang@intel.com> wrote: > On ?, 2012-07-24 at 12:27 +0530, Amit Kachhap wrote: >> >> >> On 19 July 2012 12:01, Zhang Rui <rui.zhang@intel.com> wrote: >> This function is used to update the cooling state of >> all the cooling devices that are binded to an active trip >> point. >> >> This will be used for passive cooling as well, in the future >> patches. >> as both active and passive cooling can share the same >> algorithm, >> which is >> >> 1. if the temperature is higher than a trip point, >> a. if the trend is THERMAL_TREND_RAISING, use higher >> cooling >> state for this trip point >> b. if the trend is THERMAL_TREND_DROPPING, use lower >> cooling >> state for this trip point >> >> 2. if the temperature is lower than a trip point, use lower >> cooling state for this trip point. >> >> Hi Rui, >> >> Your patches looks useful. >> For my platform I need to have get_trend called even in the case when >> current temp is less than the trip point and then use >> THERMAL_TREND_DROPPING to actually lower the cooling state. >> > hmm, why? > in the current cooling algorithm, when the temperature is lower than the > trip point, the cooling state is decreased by 1 every time, > unconditionally, isn't this what you want? Basically the requirement is that I do not want to remove the cooling device when the temp just reaches below the trip point because this causes many throttling in this trip point so say I will remove cooling device when the temp is < (trip - 5) which can be done inside the new platform get_trend call. Thanks, Amit D > > thanks, > rui > >> Thanks, >> Amit >> >> >> Signed-off-by: Zhang Rui <rui.zhang@intel.com> >> --- >> drivers/acpi/thermal.c | 7 +++- >> drivers/thermal/thermal_sys.c | 91 >> +++++++++++++++++++++++++++++------------ >> 2 files changed, 71 insertions(+), 27 deletions(-) >> >> diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c >> index 73e335f..14c4879 100644 >> --- a/drivers/acpi/thermal.c >> +++ b/drivers/acpi/thermal.c >> @@ -715,7 +715,12 @@ static int thermal_get_trend(struct >> thermal_zone_device *thermal, >> if (thermal_get_trip_type(thermal, trip, &type)) >> return -EINVAL; >> >> - /* Only PASSIVE trip points need TREND */ >> + if (type == THERMAL_TRIP_ACTIVE) { >> + /* aggressive active cooling */ >> + *trend = THERMAL_TREND_RAISING; >> + return 0; >> + } >> + >> if (type != THERMAL_TRIP_PASSIVE) >> return -EINVAL; >> >> diff --git a/drivers/thermal/thermal_sys.c >> b/drivers/thermal/thermal_sys.c >> index 59af3b8..011faba 100644 >> --- a/drivers/thermal/thermal_sys.c >> +++ b/drivers/thermal/thermal_sys.c >> @@ -1076,6 +1076,70 @@ void >> thermal_cooling_device_unregister(struct >> } >> EXPORT_SYMBOL(thermal_cooling_device_unregister); >> >> +/* >> + * Cooling algorithm for active trip points >> + * >> + * 1. if the temperature is higher than a trip point, >> + * a. if the trend is THERMAL_TREND_RAISING, use higher >> cooling >> + * state for this trip point >> + * b. if the trend is THERMAL_TREND_DROPPING, use lower >> cooling >> + * state for this trip point >> + * >> + * 2. if the temperature is lower than a trip point, use >> lower >> + * cooling state for this trip point >> + * >> + * Note that this behaves the same as the previous passive >> cooling >> + * algorithm. >> + */ >> + >> +static void thermal_zone_trip_update(struct >> thermal_zone_device *tz, >> + int trip, long temp) >> +{ >> + struct thermal_cooling_device_instance *instance; >> + struct thermal_cooling_device *cdev = NULL; >> + unsigned long cur_state, max_state; >> + long trip_temp; >> + enum thermal_trend trend; >> + >> + tz->ops->get_trip_temp(tz, trip, &trip_temp); >> + >> + if (temp >= trip_temp) { >> + thermal_get_trend(tz, trip, &trend); >> + >> + list_for_each_entry(instance, >> &tz->cooling_devices, node) { >> + if (instance->trip != trip) >> + continue; >> + >> + cdev = instance->cdev; >> + >> + cdev->ops->get_cur_state(cdev, >> &cur_state); >> + cdev->ops->get_max_state(cdev, >> &max_state); >> + >> + if (trend == THERMAL_TREND_RAISING) { >> + cur_state = cur_state < >> instance->upper ? >> + (cur_state + 1) : >> instance->upper; >> + } else if (trend == >> THERMAL_TREND_DROPPING) { >> + cur_state = cur_state > >> instance->lower ? >> + (cur_state - 1) : >> instance->lower; >> + } >> + cdev->ops->set_cur_state(cdev, >> cur_state); >> + } >> + } else { /* below trip */ >> + list_for_each_entry(instance, >> &tz->cooling_devices, node) { >> + if (instance->trip != trip) >> + continue; >> + >> + cdev = instance->cdev; >> + cdev->ops->get_cur_state(cdev, >> &cur_state); >> + >> + cur_state = cur_state > >> instance->lower ? >> + (cur_state - 1) : >> instance->lower; >> + cdev->ops->set_cur_state(cdev, >> cur_state); >> + } >> + } >> + >> + return; >> +} >> /** >> * thermal_zone_device_update - force an update of a thermal >> zone's state >> * @ttz: the thermal zone to update >> @@ -1086,9 +1150,6 @@ void thermal_zone_device_update(struct >> thermal_zone_device *tz) >> int count, ret = 0; >> long temp, trip_temp; >> enum thermal_trip_type trip_type; >> - struct thermal_cooling_device_instance *instance; >> - struct thermal_cooling_device *cdev; >> - unsigned long cur_state, max_state; >> >> mutex_lock(&tz->lock); >> >> @@ -1124,29 +1185,7 @@ void thermal_zone_device_update(struct >> thermal_zone_device *tz) >> tz->ops->notify(tz, >> count, trip_type); >> break; >> case THERMAL_TRIP_ACTIVE: >> - list_for_each_entry(instance, >> &tz->cooling_devices, >> - node) { >> - if (instance->trip != count) >> - continue; >> - >> - cdev = instance->cdev; >> - >> - cdev->ops->get_cur_state(cdev, >> &cur_state); >> - cdev->ops->get_max_state(cdev, >> &max_state); >> - >> - if (temp >= trip_temp) >> - cur_state = >> - cur_state < >> instance->upper ? >> - (cur_state + >> 1) : >> - >> instance->upper; >> - else >> - cur_state = >> - cur_state > >> instance->lower ? >> - (cur_state - >> 1) : >> - >> instance->lower; >> - >> - cdev->ops->set_cur_state(cdev, >> cur_state); >> - } >> + thermal_zone_trip_update(tz, count, >> temp); >> break; >> case THERMAL_TRIP_PASSIVE: >> if (temp >= trip_temp || tz->passive) >> -- >> 1.7.9.5 >> >> > > -- 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 Tuesday, July 24, 2012, Zhang Rui wrote: > On ?, 2012-07-19 at 23:19 +0200, Rafael J. Wysocki wrote: > > On Thursday, July 19, 2012, Zhang Rui wrote: > > > This function is used to update the cooling state of > > > all the cooling devices that are binded to an active trip point. > > > > s/binded/bound/ > > > got it. > > > > This will be used for passive cooling as well, in the future patches. > > > as both active and passive cooling can share the same algorithm, > > > which is > > > > > > 1. if the temperature is higher than a trip point, > > > a. if the trend is THERMAL_TREND_RAISING, use higher cooling > > > state for this trip point > > > b. if the trend is THERMAL_TREND_DROPPING, use lower cooling > > > state for this trip point > > > > > > 2. if the temperature is lower than a trip point, use lower > > > cooling state for this trip point. > > > > > > Signed-off-by: Zhang Rui <rui.zhang@intel.com> > > > --- > > > drivers/acpi/thermal.c | 7 +++- > > > drivers/thermal/thermal_sys.c | 91 +++++++++++++++++++++++++++++------------ > > > 2 files changed, 71 insertions(+), 27 deletions(-) > > > > > > diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c > > > index 73e335f..14c4879 100644 > > > --- a/drivers/acpi/thermal.c > > > +++ b/drivers/acpi/thermal.c > > > @@ -715,7 +715,12 @@ static int thermal_get_trend(struct thermal_zone_device *thermal, > > > if (thermal_get_trip_type(thermal, trip, &type)) > > > return -EINVAL; > > > > > > - /* Only PASSIVE trip points need TREND */ > > > + if (type == THERMAL_TRIP_ACTIVE) { > > > + /* aggressive active cooling */ > > > + *trend = THERMAL_TREND_RAISING; > > > + return 0; > > > > Please move that into thermal_zone_trip_update() directly, unless you > > need it elsewhere. > > > IMO, I can say that ACPI thermal wants aggressive active cooling, as it > will never spin down the fan when the temperature is higher than the > trip point. I meant the code organization, not the functionality. :-) Since thermal_get_trend() is static, it is not used outside of this file, so you can move the "if (type == THERMAL_TRIP_ACTIVE)" conditional from it directly into the caller (unless there are more callers, which I'm not sure about without reading the whole code again). That would make the code cleaner, because right now the condition is hidden in thermal_get_trend() and it may not be clear to the reader of thermal_zone_trip_update() that it is actually checked. Thanks, Rafael -- 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 ?, 2012-07-24 at 11:27 +0200, Rafael J. Wysocki wrote: > On Tuesday, July 24, 2012, Zhang Rui wrote: > > On ?, 2012-07-19 at 23:19 +0200, Rafael J. Wysocki wrote: > > > On Thursday, July 19, 2012, Zhang Rui wrote: > > > > This function is used to update the cooling state of > > > > all the cooling devices that are binded to an active trip point. > > > > > > s/binded/bound/ > > > > > got it. > > > > > > This will be used for passive cooling as well, in the future patches. > > > > as both active and passive cooling can share the same algorithm, > > > > which is > > > > > > > > 1. if the temperature is higher than a trip point, > > > > a. if the trend is THERMAL_TREND_RAISING, use higher cooling > > > > state for this trip point > > > > b. if the trend is THERMAL_TREND_DROPPING, use lower cooling > > > > state for this trip point > > > > > > > > 2. if the temperature is lower than a trip point, use lower > > > > cooling state for this trip point. > > > > > > > > Signed-off-by: Zhang Rui <rui.zhang@intel.com> > > > > --- > > > > drivers/acpi/thermal.c | 7 +++- > > > > drivers/thermal/thermal_sys.c | 91 +++++++++++++++++++++++++++++------------ > > > > 2 files changed, 71 insertions(+), 27 deletions(-) > > > > > > > > diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c > > > > index 73e335f..14c4879 100644 > > > > --- a/drivers/acpi/thermal.c > > > > +++ b/drivers/acpi/thermal.c > > > > @@ -715,7 +715,12 @@ static int thermal_get_trend(struct thermal_zone_device *thermal, > > > > if (thermal_get_trip_type(thermal, trip, &type)) > > > > return -EINVAL; > > > > > > > > - /* Only PASSIVE trip points need TREND */ > > > > + if (type == THERMAL_TRIP_ACTIVE) { > > > > + /* aggressive active cooling */ > > > > + *trend = THERMAL_TREND_RAISING; > > > > + return 0; > > > > > > Please move that into thermal_zone_trip_update() directly, unless you > > > need it elsewhere. > > > > > IMO, I can say that ACPI thermal wants aggressive active cooling, as it > > will never spin down the fan when the temperature is higher than the > > trip point. > > I meant the code organization, not the functionality. :-) > sure. > Since thermal_get_trend() is static, it is not used outside of this file, > so you can move the "if (type == THERMAL_TRIP_ACTIVE)" conditional from it > directly into the caller (unless there are more callers, which I'm not sure > about without reading the whole code again). > sorry I still do not get it. the generic thermal layer is the caller of this callback. so you mean in thermal_zone_trip_update(), when updating an active trip point, we set the trend to THERMAL_TREND_RAISING explicitly? in this way, all the platform thermal drivers will spin up the fan step by step when the temperature is higher than the trip point, even if the temperature is dropping. (Note that, this is not a problem for ACPI, as there is only two state for ACPI FAN, on/off, it must be always on when the temperature is higher than the active trip point. but other platform may want to spin down the fan a little even if the system is still warm) > That would make the code cleaner, because right now the condition is > hidden in thermal_get_trend() and it may not be clear to the reader of > thermal_zone_trip_update() that it is actually checked. > why? Readers should trust the value returned by drivers/thermal/thermal_sys.c:thermal_get_trend(). And platform thermal drivers should provide the proper trend for active trip points as well. If they don't, we will use the default trend by comparing the current temperature and last temperature. thanks, rui > Thanks, > Rafael -- 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 Wednesday, July 25, 2012, Zhang Rui wrote: > On ?, 2012-07-24 at 11:27 +0200, Rafael J. Wysocki wrote: > > On Tuesday, July 24, 2012, Zhang Rui wrote: > > > On ?, 2012-07-19 at 23:19 +0200, Rafael J. Wysocki wrote: > > > > On Thursday, July 19, 2012, Zhang Rui wrote: > > > > > This function is used to update the cooling state of > > > > > all the cooling devices that are binded to an active trip point. > > > > > > > > s/binded/bound/ > > > > > > > got it. > > > > > > > > This will be used for passive cooling as well, in the future patches. > > > > > as both active and passive cooling can share the same algorithm, > > > > > which is > > > > > > > > > > 1. if the temperature is higher than a trip point, > > > > > a. if the trend is THERMAL_TREND_RAISING, use higher cooling > > > > > state for this trip point > > > > > b. if the trend is THERMAL_TREND_DROPPING, use lower cooling > > > > > state for this trip point > > > > > > > > > > 2. if the temperature is lower than a trip point, use lower > > > > > cooling state for this trip point. > > > > > > > > > > Signed-off-by: Zhang Rui <rui.zhang@intel.com> > > > > > --- > > > > > drivers/acpi/thermal.c | 7 +++- > > > > > drivers/thermal/thermal_sys.c | 91 +++++++++++++++++++++++++++++------------ > > > > > 2 files changed, 71 insertions(+), 27 deletions(-) > > > > > > > > > > diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c > > > > > index 73e335f..14c4879 100644 > > > > > --- a/drivers/acpi/thermal.c > > > > > +++ b/drivers/acpi/thermal.c > > > > > @@ -715,7 +715,12 @@ static int thermal_get_trend(struct thermal_zone_device *thermal, > > > > > if (thermal_get_trip_type(thermal, trip, &type)) > > > > > return -EINVAL; > > > > > > > > > > - /* Only PASSIVE trip points need TREND */ > > > > > + if (type == THERMAL_TRIP_ACTIVE) { > > > > > + /* aggressive active cooling */ > > > > > + *trend = THERMAL_TREND_RAISING; > > > > > + return 0; > > > > > > > > Please move that into thermal_zone_trip_update() directly, unless you > > > > need it elsewhere. > > > > > > > IMO, I can say that ACPI thermal wants aggressive active cooling, as it > > > will never spin down the fan when the temperature is higher than the > > > trip point. > > > > I meant the code organization, not the functionality. :-) > > > sure. > > > Since thermal_get_trend() is static, it is not used outside of this file, > > so you can move the "if (type == THERMAL_TRIP_ACTIVE)" conditional from it > > directly into the caller (unless there are more callers, which I'm not sure > > about without reading the whole code again). > > > sorry I still do not get it. > the generic thermal layer is the caller of this callback. I'm not talking about the callback, but of the static function with the same name you've introduced into drivers/thermal/thermal_sys.c in the previous patch. :-) The only caller of this function seems to be thermal_zone_device_passive() and my point is that it would be better to simply put the code from that function into thermal_zone_device_passive() directly, unless there are more callers added by the subsequent patches, which I'm not sure about. Thanks, Rafael -- 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 ?, 2012-07-25 at 13:07 +0200, Rafael J. Wysocki wrote: > On Wednesday, July 25, 2012, Zhang Rui wrote: > > On ?, 2012-07-24 at 11:27 +0200, Rafael J. Wysocki wrote: > > > On Tuesday, July 24, 2012, Zhang Rui wrote: > > > > On ?, 2012-07-19 at 23:19 +0200, Rafael J. Wysocki wrote: > > > > > On Thursday, July 19, 2012, Zhang Rui wrote: > > > > > > This function is used to update the cooling state of > > > > > > all the cooling devices that are binded to an active trip point. > > > > > > > > > > s/binded/bound/ > > > > > > > > > got it. > > > > > > > > > > This will be used for passive cooling as well, in the future patches. > > > > > > as both active and passive cooling can share the same algorithm, > > > > > > which is > > > > > > > > > > > > 1. if the temperature is higher than a trip point, > > > > > > a. if the trend is THERMAL_TREND_RAISING, use higher cooling > > > > > > state for this trip point > > > > > > b. if the trend is THERMAL_TREND_DROPPING, use lower cooling > > > > > > state for this trip point > > > > > > > > > > > > 2. if the temperature is lower than a trip point, use lower > > > > > > cooling state for this trip point. > > > > > > > > > > > > Signed-off-by: Zhang Rui <rui.zhang@intel.com> > > > > > > --- > > > > > > drivers/acpi/thermal.c | 7 +++- > > > > > > drivers/thermal/thermal_sys.c | 91 +++++++++++++++++++++++++++++------------ > > > > > > 2 files changed, 71 insertions(+), 27 deletions(-) > > > > > > > > > > > > diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c > > > > > > index 73e335f..14c4879 100644 > > > > > > --- a/drivers/acpi/thermal.c > > > > > > +++ b/drivers/acpi/thermal.c > > > > > > @@ -715,7 +715,12 @@ static int thermal_get_trend(struct thermal_zone_device *thermal, > > > > > > if (thermal_get_trip_type(thermal, trip, &type)) > > > > > > return -EINVAL; > > > > > > > > > > > > - /* Only PASSIVE trip points need TREND */ > > > > > > + if (type == THERMAL_TRIP_ACTIVE) { > > > > > > + /* aggressive active cooling */ > > > > > > + *trend = THERMAL_TREND_RAISING; > > > > > > + return 0; > > > > > > > > > > Please move that into thermal_zone_trip_update() directly, unless you > > > > > need it elsewhere. > > > > > > > > > IMO, I can say that ACPI thermal wants aggressive active cooling, as it > > > > will never spin down the fan when the temperature is higher than the > > > > trip point. > > > > > > I meant the code organization, not the functionality. :-) > > > > > sure. > > > > > Since thermal_get_trend() is static, it is not used outside of this file, > > > so you can move the "if (type == THERMAL_TRIP_ACTIVE)" conditional from it > > > directly into the caller (unless there are more callers, which I'm not sure > > > about without reading the whole code again). > > > > > sorry I still do not get it. > > the generic thermal layer is the caller of this callback. > > I'm not talking about the callback, but of the static function with the > same name you've introduced into drivers/thermal/thermal_sys.c in the > previous patch. :-) > > The only caller of this function seems to be thermal_zone_device_passive() > and my point is that it would be better to simply put the code from that > function into thermal_zone_device_passive() directly, unless there are > more callers added by the subsequent patches, which I'm not sure about. hah, I understand. yes, I can move thermal_sys.c thermal_get_trend() in to thermal_trip_update. and there will be no such confusion (two functions share the same name) any more. :) 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 ?, 2012-07-24 at 13:36 +0530, Amit Kachhap wrote: > On 24 July 2012 12:41, Zhang Rui <rui.zhang@intel.com> wrote: > > On ?, 2012-07-24 at 12:27 +0530, Amit Kachhap wrote: > >> > >> > >> On 19 July 2012 12:01, Zhang Rui <rui.zhang@intel.com> wrote: > >> This function is used to update the cooling state of > >> all the cooling devices that are binded to an active trip > >> point. > >> > >> This will be used for passive cooling as well, in the future > >> patches. > >> as both active and passive cooling can share the same > >> algorithm, > >> which is > >> > >> 1. if the temperature is higher than a trip point, > >> a. if the trend is THERMAL_TREND_RAISING, use higher > >> cooling > >> state for this trip point > >> b. if the trend is THERMAL_TREND_DROPPING, use lower > >> cooling > >> state for this trip point > >> > >> 2. if the temperature is lower than a trip point, use lower > >> cooling state for this trip point. > >> > >> Hi Rui, > >> > >> Your patches looks useful. > >> For my platform I need to have get_trend called even in the case when > >> current temp is less than the trip point and then use > >> THERMAL_TREND_DROPPING to actually lower the cooling state. > >> > > hmm, why? > > in the current cooling algorithm, when the temperature is lower than the > > trip point, the cooling state is decreased by 1 every time, > > unconditionally, isn't this what you want? > > Basically the requirement is that I do not want to remove the cooling > device when the temp just reaches below the trip point because this > causes many throttling in this trip point so say I will remove cooling > device when the temp is < (trip - 5) which can be done inside the new > platform get_trend call. > As Durga is working on the thermal governor patches, I think that one can handle this, right? Currently, as I do not want to change the behavior of the current drivers, ACPI passive cooling, etc, I did not use trend when the temperature is lower than the trip point. But anyway, if really needed, I can generate an incremental patch. what do you think? thanks, rui > Thanks, > Amit D > > > > thanks, > > rui > > > >> Thanks, > >> Amit > >> > >> > >> Signed-off-by: Zhang Rui <rui.zhang@intel.com> > >> --- > >> drivers/acpi/thermal.c | 7 +++- > >> drivers/thermal/thermal_sys.c | 91 > >> +++++++++++++++++++++++++++++------------ > >> 2 files changed, 71 insertions(+), 27 deletions(-) > >> > >> diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c > >> index 73e335f..14c4879 100644 > >> --- a/drivers/acpi/thermal.c > >> +++ b/drivers/acpi/thermal.c > >> @@ -715,7 +715,12 @@ static int thermal_get_trend(struct > >> thermal_zone_device *thermal, > >> if (thermal_get_trip_type(thermal, trip, &type)) > >> return -EINVAL; > >> > >> - /* Only PASSIVE trip points need TREND */ > >> + if (type == THERMAL_TRIP_ACTIVE) { > >> + /* aggressive active cooling */ > >> + *trend = THERMAL_TREND_RAISING; > >> + return 0; > >> + } > >> + > >> if (type != THERMAL_TRIP_PASSIVE) > >> return -EINVAL; > >> > >> diff --git a/drivers/thermal/thermal_sys.c > >> b/drivers/thermal/thermal_sys.c > >> index 59af3b8..011faba 100644 > >> --- a/drivers/thermal/thermal_sys.c > >> +++ b/drivers/thermal/thermal_sys.c > >> @@ -1076,6 +1076,70 @@ void > >> thermal_cooling_device_unregister(struct > >> } > >> EXPORT_SYMBOL(thermal_cooling_device_unregister); > >> > >> +/* > >> + * Cooling algorithm for active trip points > >> + * > >> + * 1. if the temperature is higher than a trip point, > >> + * a. if the trend is THERMAL_TREND_RAISING, use higher > >> cooling > >> + * state for this trip point > >> + * b. if the trend is THERMAL_TREND_DROPPING, use lower > >> cooling > >> + * state for this trip point > >> + * > >> + * 2. if the temperature is lower than a trip point, use > >> lower > >> + * cooling state for this trip point > >> + * > >> + * Note that this behaves the same as the previous passive > >> cooling > >> + * algorithm. > >> + */ > >> + > >> +static void thermal_zone_trip_update(struct > >> thermal_zone_device *tz, > >> + int trip, long temp) > >> +{ > >> + struct thermal_cooling_device_instance *instance; > >> + struct thermal_cooling_device *cdev = NULL; > >> + unsigned long cur_state, max_state; > >> + long trip_temp; > >> + enum thermal_trend trend; > >> + > >> + tz->ops->get_trip_temp(tz, trip, &trip_temp); > >> + > >> + if (temp >= trip_temp) { > >> + thermal_get_trend(tz, trip, &trend); > >> + > >> + list_for_each_entry(instance, > >> &tz->cooling_devices, node) { > >> + if (instance->trip != trip) > >> + continue; > >> + > >> + cdev = instance->cdev; > >> + > >> + cdev->ops->get_cur_state(cdev, > >> &cur_state); > >> + cdev->ops->get_max_state(cdev, > >> &max_state); > >> + > >> + if (trend == THERMAL_TREND_RAISING) { > >> + cur_state = cur_state < > >> instance->upper ? > >> + (cur_state + 1) : > >> instance->upper; > >> + } else if (trend == > >> THERMAL_TREND_DROPPING) { > >> + cur_state = cur_state > > >> instance->lower ? > >> + (cur_state - 1) : > >> instance->lower; > >> + } > >> + cdev->ops->set_cur_state(cdev, > >> cur_state); > >> + } > >> + } else { /* below trip */ > >> + list_for_each_entry(instance, > >> &tz->cooling_devices, node) { > >> + if (instance->trip != trip) > >> + continue; > >> + > >> + cdev = instance->cdev; > >> + cdev->ops->get_cur_state(cdev, > >> &cur_state); > >> + > >> + cur_state = cur_state > > >> instance->lower ? > >> + (cur_state - 1) : > >> instance->lower; > >> + cdev->ops->set_cur_state(cdev, > >> cur_state); > >> + } > >> + } > >> + > >> + return; > >> +} > >> /** > >> * thermal_zone_device_update - force an update of a thermal > >> zone's state > >> * @ttz: the thermal zone to update > >> @@ -1086,9 +1150,6 @@ void thermal_zone_device_update(struct > >> thermal_zone_device *tz) > >> int count, ret = 0; > >> long temp, trip_temp; > >> enum thermal_trip_type trip_type; > >> - struct thermal_cooling_device_instance *instance; > >> - struct thermal_cooling_device *cdev; > >> - unsigned long cur_state, max_state; > >> > >> mutex_lock(&tz->lock); > >> > >> @@ -1124,29 +1185,7 @@ void thermal_zone_device_update(struct > >> thermal_zone_device *tz) > >> tz->ops->notify(tz, > >> count, trip_type); > >> break; > >> case THERMAL_TRIP_ACTIVE: > >> - list_for_each_entry(instance, > >> &tz->cooling_devices, > >> - node) { > >> - if (instance->trip != count) > >> - continue; > >> - > >> - cdev = instance->cdev; > >> - > >> - cdev->ops->get_cur_state(cdev, > >> &cur_state); > >> - cdev->ops->get_max_state(cdev, > >> &max_state); > >> - > >> - if (temp >= trip_temp) > >> - cur_state = > >> - cur_state < > >> instance->upper ? > >> - (cur_state + > >> 1) : > >> - > >> instance->upper; > >> - else > >> - cur_state = > >> - cur_state > > >> instance->lower ? > >> - (cur_state - > >> 1) : > >> - > >> instance->lower; > >> - > >> - cdev->ops->set_cur_state(cdev, > >> cur_state); > >> - } > >> + thermal_zone_trip_update(tz, count, > >> temp); > >> break; > >> case THERMAL_TRIP_PASSIVE: > >> if (temp >= trip_temp || tz->passive) > >> -- > >> 1.7.9.5 > >> > >> > > > > -- 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, > -----Original Message----- > From: Zhang, Rui > Sent: Thursday, July 26, 2012 10:38 AM > To: Amit Kachhap > Cc: linux-acpi@vger.kernel.org; linux-pm@vger.kernel.org; Rafael J. Wysocki; > Matthew Garrett; Len Brown; R, Durgadoss; Eduardo Valentin; Wei Ni > Subject: Re: [PATCH 09/16] Thermal: Introduce thermal_zone_trip_update() > > On ?, 2012-07-24 at 13:36 +0530, Amit Kachhap wrote: > > On 24 July 2012 12:41, Zhang Rui <rui.zhang@intel.com> wrote: > > > On ?, 2012-07-24 at 12:27 +0530, Amit Kachhap wrote: > > >> > > >> > > >> On 19 July 2012 12:01, Zhang Rui <rui.zhang@intel.com> wrote: > > >> This function is used to update the cooling state of > > >> all the cooling devices that are binded to an active trip > > >> point. > > >> > > >> This will be used for passive cooling as well, in the future > > >> patches. > > >> as both active and passive cooling can share the same > > >> algorithm, > > >> which is > > >> > > >> 1. if the temperature is higher than a trip point, > > >> a. if the trend is THERMAL_TREND_RAISING, use higher > > >> cooling > > >> state for this trip point > > >> b. if the trend is THERMAL_TREND_DROPPING, use lower > > >> cooling > > >> state for this trip point > > >> > > >> 2. if the temperature is lower than a trip point, use lower > > >> cooling state for this trip point. > > >> > > >> Hi Rui, > > >> > > >> Your patches looks useful. > > >> For my platform I need to have get_trend called even in the case when > > >> current temp is less than the trip point and then use > > >> THERMAL_TREND_DROPPING to actually lower the cooling state. > > >> > > > hmm, why? > > > in the current cooling algorithm, when the temperature is lower than the > > > trip point, the cooling state is decreased by 1 every time, > > > unconditionally, isn't this what you want? > > > > Basically the requirement is that I do not want to remove the cooling > > device when the temp just reaches below the trip point because this > > causes many throttling in this trip point so say I will remove cooling > > device when the temp is < (trip - 5) which can be done inside the new > > platform get_trend call. > > > As Durga is working on the thermal governor patches, I think that one > can handle this, right? I am fine with this..Since we anyway want all throttling logic to be moved to the governor, I think it makes sense to put this there. So, I will pick it up, when I submit the governor patches. Amit, Please feel free to point out if I don’t pick this one in my patches. Thanks, Durga > > Currently, as I do not want to change the behavior of the current > drivers, ACPI passive cooling, etc, I did not use trend when the > temperature is lower than the trip point. > But anyway, if really needed, I can generate an incremental patch. > what do you think? > > thanks, > rui > > > Thanks, > > Amit D > > > > > > thanks, > > > rui > > > > > >> Thanks, > > >> Amit > > >> > > >> > > >> Signed-off-by: Zhang Rui <rui.zhang@intel.com> > > >> --- > > >> drivers/acpi/thermal.c | 7 +++- > > >> drivers/thermal/thermal_sys.c | 91 > > >> +++++++++++++++++++++++++++++------------ > > >> 2 files changed, 71 insertions(+), 27 deletions(-) > > >> > > >> diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c > > >> index 73e335f..14c4879 100644 > > >> --- a/drivers/acpi/thermal.c > > >> +++ b/drivers/acpi/thermal.c > > >> @@ -715,7 +715,12 @@ static int thermal_get_trend(struct > > >> thermal_zone_device *thermal, > > >> if (thermal_get_trip_type(thermal, trip, &type)) > > >> return -EINVAL; > > >> > > >> - /* Only PASSIVE trip points need TREND */ > > >> + if (type == THERMAL_TRIP_ACTIVE) { > > >> + /* aggressive active cooling */ > > >> + *trend = THERMAL_TREND_RAISING; > > >> + return 0; > > >> + } > > >> + > > >> if (type != THERMAL_TRIP_PASSIVE) > > >> return -EINVAL; > > >> > > >> diff --git a/drivers/thermal/thermal_sys.c > > >> b/drivers/thermal/thermal_sys.c > > >> index 59af3b8..011faba 100644 > > >> --- a/drivers/thermal/thermal_sys.c > > >> +++ b/drivers/thermal/thermal_sys.c > > >> @@ -1076,6 +1076,70 @@ void > > >> thermal_cooling_device_unregister(struct > > >> } > > >> EXPORT_SYMBOL(thermal_cooling_device_unregister); > > >> > > >> +/* > > >> + * Cooling algorithm for active trip points > > >> + * > > >> + * 1. if the temperature is higher than a trip point, > > >> + * a. if the trend is THERMAL_TREND_RAISING, use higher > > >> cooling > > >> + * state for this trip point > > >> + * b. if the trend is THERMAL_TREND_DROPPING, use lower > > >> cooling > > >> + * state for this trip point > > >> + * > > >> + * 2. if the temperature is lower than a trip point, use > > >> lower > > >> + * cooling state for this trip point > > >> + * > > >> + * Note that this behaves the same as the previous passive > > >> cooling > > >> + * algorithm. > > >> + */ > > >> + > > >> +static void thermal_zone_trip_update(struct > > >> thermal_zone_device *tz, > > >> + int trip, long temp) > > >> +{ > > >> + struct thermal_cooling_device_instance *instance; > > >> + struct thermal_cooling_device *cdev = NULL; > > >> + unsigned long cur_state, max_state; > > >> + long trip_temp; > > >> + enum thermal_trend trend; > > >> + > > >> + tz->ops->get_trip_temp(tz, trip, &trip_temp); > > >> + > > >> + if (temp >= trip_temp) { > > >> + thermal_get_trend(tz, trip, &trend); > > >> + > > >> + list_for_each_entry(instance, > > >> &tz->cooling_devices, node) { > > >> + if (instance->trip != trip) > > >> + continue; > > >> + > > >> + cdev = instance->cdev; > > >> + > > >> + cdev->ops->get_cur_state(cdev, > > >> &cur_state); > > >> + cdev->ops->get_max_state(cdev, > > >> &max_state); > > >> + > > >> + if (trend == THERMAL_TREND_RAISING) { > > >> + cur_state = cur_state < > > >> instance->upper ? > > >> + (cur_state + 1) : > > >> instance->upper; > > >> + } else if (trend == > > >> THERMAL_TREND_DROPPING) { > > >> + cur_state = cur_state > > > >> instance->lower ? > > >> + (cur_state - 1) : > > >> instance->lower; > > >> + } > > >> + cdev->ops->set_cur_state(cdev, > > >> cur_state); > > >> + } > > >> + } else { /* below trip */ > > >> + list_for_each_entry(instance, > > >> &tz->cooling_devices, node) { > > >> + if (instance->trip != trip) > > >> + continue; > > >> + > > >> + cdev = instance->cdev; > > >> + cdev->ops->get_cur_state(cdev, > > >> &cur_state); > > >> + > > >> + cur_state = cur_state > > > >> instance->lower ? > > >> + (cur_state - 1) : > > >> instance->lower; > > >> + cdev->ops->set_cur_state(cdev, > > >> cur_state); > > >> + } > > >> + } > > >> + > > >> + return; > > >> +} > > >> /** > > >> * thermal_zone_device_update - force an update of a thermal > > >> zone's state > > >> * @ttz: the thermal zone to update > > >> @@ -1086,9 +1150,6 @@ void thermal_zone_device_update(struct > > >> thermal_zone_device *tz) > > >> int count, ret = 0; > > >> long temp, trip_temp; > > >> enum thermal_trip_type trip_type; > > >> - struct thermal_cooling_device_instance *instance; > > >> - struct thermal_cooling_device *cdev; > > >> - unsigned long cur_state, max_state; > > >> > > >> mutex_lock(&tz->lock); > > >> > > >> @@ -1124,29 +1185,7 @@ void thermal_zone_device_update(struct > > >> thermal_zone_device *tz) > > >> tz->ops->notify(tz, > > >> count, trip_type); > > >> break; > > >> case THERMAL_TRIP_ACTIVE: > > >> - list_for_each_entry(instance, > > >> &tz->cooling_devices, > > >> - node) { > > >> - if (instance->trip != count) > > >> - continue; > > >> - > > >> - cdev = instance->cdev; > > >> - > > >> - cdev->ops->get_cur_state(cdev, > > >> &cur_state); > > >> - cdev->ops->get_max_state(cdev, > > >> &max_state); > > >> - > > >> - if (temp >= trip_temp) > > >> - cur_state = > > >> - cur_state < > > >> instance->upper ? > > >> - (cur_state + > > >> 1) : > > >> - > > >> instance->upper; > > >> - else > > >> - cur_state = > > >> - cur_state > > > >> instance->lower ? > > >> - (cur_state - > > >> 1) : > > >> - > > >> instance->lower; > > >> - > > >> - cdev->ops->set_cur_state(cdev, > > >> cur_state); > > >> - } > > >> + thermal_zone_trip_update(tz, count, > > >> temp); > > >> break; > > >> case THERMAL_TRIP_PASSIVE: > > >> if (temp >= trip_temp || tz->passive) > > >> -- > > >> 1.7.9.5 > > >> > > >> > > > > > > >
diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c index 73e335f..14c4879 100644 --- a/drivers/acpi/thermal.c +++ b/drivers/acpi/thermal.c @@ -715,7 +715,12 @@ static int thermal_get_trend(struct thermal_zone_device *thermal, if (thermal_get_trip_type(thermal, trip, &type)) return -EINVAL; - /* Only PASSIVE trip points need TREND */ + if (type == THERMAL_TRIP_ACTIVE) { + /* aggressive active cooling */ + *trend = THERMAL_TREND_RAISING; + return 0; + } + if (type != THERMAL_TRIP_PASSIVE) return -EINVAL; diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c index 59af3b8..011faba 100644 --- a/drivers/thermal/thermal_sys.c +++ b/drivers/thermal/thermal_sys.c @@ -1076,6 +1076,70 @@ void thermal_cooling_device_unregister(struct } EXPORT_SYMBOL(thermal_cooling_device_unregister); +/* + * Cooling algorithm for active trip points + * + * 1. if the temperature is higher than a trip point, + * a. if the trend is THERMAL_TREND_RAISING, use higher cooling + * state for this trip point + * b. if the trend is THERMAL_TREND_DROPPING, use lower cooling + * state for this trip point + * + * 2. if the temperature is lower than a trip point, use lower + * cooling state for this trip point + * + * Note that this behaves the same as the previous passive cooling + * algorithm. + */ + +static void thermal_zone_trip_update(struct thermal_zone_device *tz, + int trip, long temp) +{ + struct thermal_cooling_device_instance *instance; + struct thermal_cooling_device *cdev = NULL; + unsigned long cur_state, max_state; + long trip_temp; + enum thermal_trend trend; + + tz->ops->get_trip_temp(tz, trip, &trip_temp); + + if (temp >= trip_temp) { + thermal_get_trend(tz, trip, &trend); + + list_for_each_entry(instance, &tz->cooling_devices, node) { + if (instance->trip != trip) + continue; + + cdev = instance->cdev; + + cdev->ops->get_cur_state(cdev, &cur_state); + cdev->ops->get_max_state(cdev, &max_state); + + if (trend == THERMAL_TREND_RAISING) { + cur_state = cur_state < instance->upper ? + (cur_state + 1) : instance->upper; + } else if (trend == THERMAL_TREND_DROPPING) { + cur_state = cur_state > instance->lower ? + (cur_state - 1) : instance->lower; + } + cdev->ops->set_cur_state(cdev, cur_state); + } + } else { /* below trip */ + list_for_each_entry(instance, &tz->cooling_devices, node) { + if (instance->trip != trip) + continue; + + cdev = instance->cdev; + cdev->ops->get_cur_state(cdev, &cur_state); + + cur_state = cur_state > instance->lower ? + (cur_state - 1) : instance->lower; + cdev->ops->set_cur_state(cdev, cur_state); + } + } + + return; +} /** * thermal_zone_device_update - force an update of a thermal zone's state * @ttz: the thermal zone to update @@ -1086,9 +1150,6 @@ void thermal_zone_device_update(struct thermal_zone_device *tz) int count, ret = 0; long temp, trip_temp; enum thermal_trip_type trip_type; - struct thermal_cooling_device_instance *instance; - struct thermal_cooling_device *cdev; - unsigned long cur_state, max_state; mutex_lock(&tz->lock); @@ -1124,29 +1185,7 @@ void thermal_zone_device_update(struct thermal_zone_device *tz) tz->ops->notify(tz, count, trip_type); break; case THERMAL_TRIP_ACTIVE: - list_for_each_entry(instance, &tz->cooling_devices, - node) { - if (instance->trip != count) - continue; - - cdev = instance->cdev; - - cdev->ops->get_cur_state(cdev, &cur_state); - cdev->ops->get_max_state(cdev, &max_state); - - if (temp >= trip_temp) - cur_state = - cur_state < instance->upper ? - (cur_state + 1) : - instance->upper; - else - cur_state = - cur_state > instance->lower ? - (cur_state - 1) : - instance->lower; - - cdev->ops->set_cur_state(cdev, cur_state); - } + thermal_zone_trip_update(tz, count, temp); break; case THERMAL_TRIP_PASSIVE: if (temp >= trip_temp || tz->passive)
This function is used to update the cooling state of all the cooling devices that are binded to an active trip point. This will be used for passive cooling as well, in the future patches. as both active and passive cooling can share the same algorithm, which is 1. if the temperature is higher than a trip point, a. if the trend is THERMAL_TREND_RAISING, use higher cooling state for this trip point b. if the trend is THERMAL_TREND_DROPPING, use lower cooling state for this trip point 2. if the temperature is lower than a trip point, use lower cooling state for this trip point. Signed-off-by: Zhang Rui <rui.zhang@intel.com> --- drivers/acpi/thermal.c | 7 +++- drivers/thermal/thermal_sys.c | 91 +++++++++++++++++++++++++++++------------ 2 files changed, 71 insertions(+), 27 deletions(-)