[PATCHv4 11/15] Thermal: Introduce a step_wise thermal governor

[PATCHv4 11/15] Thermal: Introduce a step_wise thermal governor

[Durgadoss R]

This patch adds a simple step_wise governor to the
generic thermal layer. This algorithm throttles the
cooling devices in a linear fashion. If the 'trend'
is heating, it throttles by one step. And if the
thermal trend is cooling it de-throttles by one step.

This actually moves the throttling logic from thermal_sys.c
and puts inside step_wise.c, without any change. This is
always selected as the default governor.

Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
---
 drivers/thermal/Kconfig     |    7 ++
 drivers/thermal/Makefile    |    1 +
 drivers/thermal/step_wise.c |  194 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 202 insertions(+)
 create mode 100644 drivers/thermal/step_wise.c

diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index 2200912..9f6a3aa 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -4,6 +4,7 @@
 
 menuconfig THERMAL
 	tristate "Generic Thermal sysfs driver"
+	select STEP_WISE
 	help
 	  Generic Thermal Sysfs driver offers a generic mechanism for
 	  thermal management. Usually it's made up of one or more thermal
@@ -59,3 +60,9 @@ config FAIR_SHARE
 	depends on THERMAL
 	help
 	  Enable this to manage platform thermals using fair-share governor.
+
+config STEP_WISE
+	bool "Step_wise thermal governor"
+	depends on THERMAL
+	help
+	  Enable this to manage platform thermals using a simple linear
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index 30540dd..4e53c00 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -8,3 +8,4 @@ obj-$(CONFIG_SPEAR_THERMAL)		+= spear_thermal.o
 obj-$(CONFIG_RCAR_THERMAL)	+= rcar_thermal.o
 obj-$(CONFIG_EXYNOS_THERMAL)		+= exynos_thermal.o
 obj-$(CONFIG_FAIR_SHARE)		+= fair_share.o
+obj-$(CONFIG_STEP_WISE)			+= step_wise.o
diff --git a/drivers/thermal/step_wise.c b/drivers/thermal/step_wise.c
new file mode 100644
index 0000000..3f9cfcf
--- /dev/null
+++ b/drivers/thermal/step_wise.c
@@ -0,0 +1,194 @@
+/*
+ *  step_wise.c - A step-by-step Thermal throttling governor
+ *
+ *  Copyright (C) 2012 Intel Corp
+ *  Copyright (C) 2012 Durgadoss R <durgadoss.r@intel.com>
+ *
+ *  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; version 2 of the License.
+ *
+ *  This program is distributed in the hope that it will be useful, but
+ *  WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ *  General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License along
+ *  with this program; if not, write to the Free Software Foundation, Inc.,
+ *  59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/module.h>
+#include <linux/thermal.h>
+
+#include "thermal_core.h"
+
+/*
+ * 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
+ */
+static unsigned long get_target_state(struct thermal_instance *instance,
+					enum thermal_trend trend)
+{
+	struct thermal_cooling_device *cdev = instance->cdev;
+	unsigned long cur_state;
+
+	cdev->ops->get_cur_state(cdev, &cur_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;
+	}
+
+	return cur_state;
+}
+
+static void update_passive_instance(struct thermal_zone_device *tz,
+				enum thermal_trip_type type, int value)
+{
+	/*
+	 * If value is +1, activate a passive instance.
+	 * If value is -1, deactivate a passive instance.
+	 */
+	if (type == THERMAL_TRIP_PASSIVE || type == THERMAL_TRIPS_NONE)
+		tz->passive += value;
+}
+
+static void update_instance_for_throttle(struct thermal_zone_device *tz,
+				int trip, enum thermal_trip_type trip_type,
+				enum thermal_trend trend)
+{
+	struct thermal_instance *instance;
+
+	list_for_each_entry(instance, &tz->thermal_instances, tz_node) {
+		if (instance->trip != trip)
+			continue;
+
+		instance->target = get_target_state(instance, trend);
+
+		/* Activate a passive thermal instance */
+		if (instance->target == THERMAL_NO_TARGET)
+			update_passive_instance(tz, trip_type, 1);
+
+		instance->cdev->updated = false; /* cdev needs update */
+	}
+}
+
+static void update_instance_for_dethrottle(struct thermal_zone_device *tz,
+				int trip, enum thermal_trip_type trip_type)
+{
+	struct thermal_instance *instance;
+	struct thermal_cooling_device *cdev;
+	unsigned long cur_state;
+
+	list_for_each_entry(instance, &tz->thermal_instances, tz_node) {
+		if (instance->trip != trip ||
+			instance->target == THERMAL_NO_TARGET)
+			continue;
+
+		cdev = instance->cdev;
+		cdev->ops->get_cur_state(cdev, &cur_state);
+
+		instance->target = cur_state > instance->lower ?
+			    (cur_state - 1) : THERMAL_NO_TARGET;
+
+		/* Deactivate a passive thermal instance */
+		if (instance->target == THERMAL_NO_TARGET)
+			update_passive_instance(tz, trip_type, -1);
+
+		cdev->updated = false; /* cdev needs update */
+	}
+}
+
+static void thermal_zone_trip_update(struct thermal_zone_device *tz, int trip)
+{
+	long trip_temp;
+	enum thermal_trip_type trip_type;
+	enum thermal_trend trend;
+
+	if (trip == THERMAL_TRIPS_NONE) {
+		trip_temp = tz->forced_passive;
+		trip_type = THERMAL_TRIPS_NONE;
+	} else {
+		tz->ops->get_trip_temp(tz, trip, &trip_temp);
+		tz->ops->get_trip_type(tz, trip, &trip_type);
+	}
+
+	trend = get_tz_trend(tz, trip);
+
+	mutex_lock(&tz->lock);
+
+	if (tz->temperature >= trip_temp)
+		update_instance_for_throttle(tz, trip, trip_type, trend);
+	else
+		update_instance_for_dethrottle(tz, trip, trip_type);
+
+	mutex_unlock(&tz->lock);
+}
+
+/**
+ * step_wise_throttle - throttles devices asscciated with the given zone
+ * @tz - thermal_zone_device
+ * @trip - the trip point
+ * @trip_type - type of the trip point
+ *
+ * Throttling Logic: This uses the trend of the thermal zone to throttle.
+ * If the thermal zone is 'heating up' this throttles all the cooling
+ * devices associated with the zone and its particular trip point, by one
+ * step. If the zone is 'cooling down' it brings back the performance of
+ * the devices by one step.
+ */
+int step_wise_throttle(struct thermal_zone_device *tz, int trip)
+{
+	struct thermal_instance *instance;
+
+	thermal_zone_trip_update(tz, trip);
+
+	if (tz->forced_passive)
+		thermal_zone_trip_update(tz, THERMAL_TRIPS_NONE);
+
+	mutex_lock(&tz->lock);
+
+	list_for_each_entry(instance, &tz->thermal_instances, tz_node)
+		thermal_cdev_update(instance->cdev);
+
+	mutex_unlock(&tz->lock);
+
+	return 0;
+}
+
+struct thermal_governor thermal_gov_step_wise = {
+	.name		= DEFAULT_THERMAL_GOVERNOR,
+	.throttle	= step_wise_throttle,
+	.owner		= THIS_MODULE,
+};
+
+static int __init thermal_gov_step_wise_init(void)
+{
+	return thermal_register_governor(&thermal_gov_step_wise);
+}
+
+static void __exit thermal_gov_step_wise_exit(void)
+{
+	thermal_unregister_governor(&thermal_gov_step_wise);
+}
+
+/* This should load after thermal framework */
+fs_initcall(thermal_gov_step_wise_init);
+module_exit(thermal_gov_step_wise_exit);
+
+MODULE_AUTHOR("Durgadoss R");
+MODULE_DESCRIPTION("A step-by-step thermal throttling governor");
+MODULE_LICENSE("GPL");
-- 
1.7.9.5

Re: [PATCHv4 11/15] Thermal: Introduce a step_wise thermal governor

[Zhang Rui]

On 二, 2012-09-18 at 11:05 +0530, Durgadoss R wrote:
> This patch adds a simple step_wise governor to the
> generic thermal layer. This algorithm throttles the
> cooling devices in a linear fashion. If the 'trend'
> is heating, it throttles by one step. And if the
> thermal trend is cooling it de-throttles by one step.
> 
> This actually moves the throttling logic from thermal_sys.c
> and puts inside step_wise.c, without any change. This is
> always selected as the default governor.
> 
> Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
> ---
>  drivers/thermal/Kconfig     |    7 ++
>  drivers/thermal/Makefile    |    1 +
>  drivers/thermal/step_wise.c |  194 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 202 insertions(+)
>  create mode 100644 drivers/thermal/step_wise.c
> 
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index 2200912..9f6a3aa 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -4,6 +4,7 @@
>  
>  menuconfig THERMAL
>  	tristate "Generic Thermal sysfs driver"
> +	select STEP_WISE
>  	help
>  	  Generic Thermal Sysfs driver offers a generic mechanism for
>  	  thermal management. Usually it's made up of one or more thermal
> @@ -59,3 +60,9 @@ config FAIR_SHARE
>  	depends on THERMAL
>  	help
>  	  Enable this to manage platform thermals using fair-share governor.
> +
> +config STEP_WISE
> +	bool "Step_wise thermal governor"
> +	depends on THERMAL
> +	help
> +	  Enable this to manage platform thermals using a simple linear

if we always wants CONFIG_STEP_WISE built in, why bother to export this
config option to users?

IMO, something like
 menuconfig THERMAL
  	tristate "Generic Thermal sysfs driver"
	select STEP_WISE

 config STEP_WISE
	bool
	depends on THERMAL

is enough, SETP_WISE will always be selected.

further more, we even do not need a config option for this governor,
just "obj-y += step_wise.o" in Makefile.

another question would be,
as this governor registration function is fs_initcall(),
what if a platform thermal driver is registered and gets a thermal event
before this governor being registered?

thanks,
rui

> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
> index 30540dd..4e53c00 100644
> --- a/drivers/thermal/Makefile
> +++ b/drivers/thermal/Makefile
> @@ -8,3 +8,4 @@ obj-$(CONFIG_SPEAR_THERMAL)		+= spear_thermal.o
>  obj-$(CONFIG_RCAR_THERMAL)	+= rcar_thermal.o
>  obj-$(CONFIG_EXYNOS_THERMAL)		+= exynos_thermal.o
>  obj-$(CONFIG_FAIR_SHARE)		+= fair_share.o
> +obj-$(CONFIG_STEP_WISE)			+= step_wise.o
> diff --git a/drivers/thermal/step_wise.c b/drivers/thermal/step_wise.c
> new file mode 100644
> index 0000000..3f9cfcf
> --- /dev/null
> +++ b/drivers/thermal/step_wise.c
> @@ -0,0 +1,194 @@
> +/*
> + *  step_wise.c - A step-by-step Thermal throttling governor
> + *
> + *  Copyright (C) 2012 Intel Corp
> + *  Copyright (C) 2012 Durgadoss R <durgadoss.r@intel.com>
> + *
> + *  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation; version 2 of the License.
> + *
> + *  This program is distributed in the hope that it will be useful, but
> + *  WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + *  General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License along
> + *  with this program; if not, write to the Free Software Foundation, Inc.,
> + *  59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
> + *
> + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/module.h>
> +#include <linux/thermal.h>
> +
> +#include "thermal_core.h"
> +
> +/*
> + * 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
> + */
> +static unsigned long get_target_state(struct thermal_instance *instance,
> +					enum thermal_trend trend)
> +{
> +	struct thermal_cooling_device *cdev = instance->cdev;
> +	unsigned long cur_state;
> +
> +	cdev->ops->get_cur_state(cdev, &cur_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;
> +	}
> +
> +	return cur_state;
> +}
> +
> +static void update_passive_instance(struct thermal_zone_device *tz,
> +				enum thermal_trip_type type, int value)
> +{
> +	/*
> +	 * If value is +1, activate a passive instance.
> +	 * If value is -1, deactivate a passive instance.
> +	 */
> +	if (type == THERMAL_TRIP_PASSIVE || type == THERMAL_TRIPS_NONE)
> +		tz->passive += value;
> +}
> +
> +static void update_instance_for_throttle(struct thermal_zone_device *tz,
> +				int trip, enum thermal_trip_type trip_type,
> +				enum thermal_trend trend)
> +{
> +	struct thermal_instance *instance;
> +
> +	list_for_each_entry(instance, &tz->thermal_instances, tz_node) {
> +		if (instance->trip != trip)
> +			continue;
> +
> +		instance->target = get_target_state(instance, trend);
> +
> +		/* Activate a passive thermal instance */
> +		if (instance->target == THERMAL_NO_TARGET)
> +			update_passive_instance(tz, trip_type, 1);
> +
> +		instance->cdev->updated = false; /* cdev needs update */
> +	}
> +}
> +
> +static void update_instance_for_dethrottle(struct thermal_zone_device *tz,
> +				int trip, enum thermal_trip_type trip_type)
> +{
> +	struct thermal_instance *instance;
> +	struct thermal_cooling_device *cdev;
> +	unsigned long cur_state;
> +
> +	list_for_each_entry(instance, &tz->thermal_instances, tz_node) {
> +		if (instance->trip != trip ||
> +			instance->target == THERMAL_NO_TARGET)
> +			continue;
> +
> +		cdev = instance->cdev;
> +		cdev->ops->get_cur_state(cdev, &cur_state);
> +
> +		instance->target = cur_state > instance->lower ?
> +			    (cur_state - 1) : THERMAL_NO_TARGET;
> +
> +		/* Deactivate a passive thermal instance */
> +		if (instance->target == THERMAL_NO_TARGET)
> +			update_passive_instance(tz, trip_type, -1);
> +
> +		cdev->updated = false; /* cdev needs update */
> +	}
> +}
> +
> +static void thermal_zone_trip_update(struct thermal_zone_device *tz, int trip)
> +{
> +	long trip_temp;
> +	enum thermal_trip_type trip_type;
> +	enum thermal_trend trend;
> +
> +	if (trip == THERMAL_TRIPS_NONE) {
> +		trip_temp = tz->forced_passive;
> +		trip_type = THERMAL_TRIPS_NONE;
> +	} else {
> +		tz->ops->get_trip_temp(tz, trip, &trip_temp);
> +		tz->ops->get_trip_type(tz, trip, &trip_type);
> +	}
> +
> +	trend = get_tz_trend(tz, trip);
> +
> +	mutex_lock(&tz->lock);
> +
> +	if (tz->temperature >= trip_temp)
> +		update_instance_for_throttle(tz, trip, trip_type, trend);
> +	else
> +		update_instance_for_dethrottle(tz, trip, trip_type);
> +
> +	mutex_unlock(&tz->lock);
> +}
> +
> +/**
> + * step_wise_throttle - throttles devices asscciated with the given zone
> + * @tz - thermal_zone_device
> + * @trip - the trip point
> + * @trip_type - type of the trip point
> + *
> + * Throttling Logic: This uses the trend of the thermal zone to throttle.
> + * If the thermal zone is 'heating up' this throttles all the cooling
> + * devices associated with the zone and its particular trip point, by one
> + * step. If the zone is 'cooling down' it brings back the performance of
> + * the devices by one step.
> + */
> +int step_wise_throttle(struct thermal_zone_device *tz, int trip)
> +{
> +	struct thermal_instance *instance;
> +
> +	thermal_zone_trip_update(tz, trip);
> +
> +	if (tz->forced_passive)
> +		thermal_zone_trip_update(tz, THERMAL_TRIPS_NONE);
> +
> +	mutex_lock(&tz->lock);
> +
> +	list_for_each_entry(instance, &tz->thermal_instances, tz_node)
> +		thermal_cdev_update(instance->cdev);
> +
> +	mutex_unlock(&tz->lock);
> +
> +	return 0;
> +}
> +
> +struct thermal_governor thermal_gov_step_wise = {
> +	.name		= DEFAULT_THERMAL_GOVERNOR,
> +	.throttle	= step_wise_throttle,
> +	.owner		= THIS_MODULE,
> +};
> +
> +static int __init thermal_gov_step_wise_init(void)
> +{
> +	return thermal_register_governor(&thermal_gov_step_wise);
> +}
> +
> +static void __exit thermal_gov_step_wise_exit(void)
> +{
> +	thermal_unregister_governor(&thermal_gov_step_wise);
> +}
> +
> +/* This should load after thermal framework */
> +fs_initcall(thermal_gov_step_wise_init);
> +module_exit(thermal_gov_step_wise_exit);
> +
> +MODULE_AUTHOR("Durgadoss R");
> +MODULE_DESCRIPTION("A step-by-step thermal throttling governor");
> +MODULE_LICENSE("GPL");


--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCHv4 11/15] Thermal: Introduce a step_wise thermal governor

[R, Durgadoss]

Hi Rui,


> -----Original Message-----
> From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-
> owner@vger.kernel.org] On Behalf Of Zhang Rui
> Sent: Tuesday, September 18, 2012 12:44 PM
> To: R, Durgadoss
> Cc: lenb@kernel.org; linux-acpi@vger.kernel.org; eduardo.valentin@ti.com
> Subject: Re: [PATCHv4 11/15] Thermal: Introduce a step_wise thermal
> governor
> 
> On 二, 2012-09-18 at 11:05 +0530, Durgadoss R wrote:
> > This patch adds a simple step_wise governor to the
> > generic thermal layer. This algorithm throttles the
> > cooling devices in a linear fashion. If the 'trend'
> > is heating, it throttles by one step. And if the
> > thermal trend is cooling it de-throttles by one step.
> >
> > This actually moves the throttling logic from thermal_sys.c
> > and puts inside step_wise.c, without any change. This is
> > always selected as the default governor.
> >
> > Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
> > ---
> >  drivers/thermal/Kconfig     |    7 ++
> >  drivers/thermal/Makefile    |    1 +
> >  drivers/thermal/step_wise.c |  194
> +++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 202 insertions(+)
> >  create mode 100644 drivers/thermal/step_wise.c
> >
> > diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> > index 2200912..9f6a3aa 100644
> > --- a/drivers/thermal/Kconfig
> > +++ b/drivers/thermal/Kconfig
> > @@ -4,6 +4,7 @@
> >
> >  menuconfig THERMAL
> >  	tristate "Generic Thermal sysfs driver"
> > +	select STEP_WISE
> >  	help
> >  	  Generic Thermal Sysfs driver offers a generic mechanism for
> >  	  thermal management. Usually it's made up of one or more thermal
> > @@ -59,3 +60,9 @@ config FAIR_SHARE
> >  	depends on THERMAL
> >  	help
> >  	  Enable this to manage platform thermals using fair-share governor.
> > +
> > +config STEP_WISE
> > +	bool "Step_wise thermal governor"
> > +	depends on THERMAL
> > +	help
> > +	  Enable this to manage platform thermals using a simple linear
> 
> if we always wants CONFIG_STEP_WISE built in, why bother to export this
> config option to users?
> 
> IMO, something like
>  menuconfig THERMAL
>   	tristate "Generic Thermal sysfs driver"
> 	select STEP_WISE
> 
>  config STEP_WISE
> 	bool
> 	depends on THERMAL
> 
> is enough, SETP_WISE will always be selected.
> 
> further more, we even do not need a config option for this governor,
> just "obj-y += step_wise.o" in Makefile.

Yes, Thought through this. But I thought it would be good to align this
with the rest of the governors.

> 
> another question would be,
> as this governor registration function is fs_initcall(),
> what if a platform thermal driver is registered and gets a thermal event
> before this governor being registered?

Wonder if this can happen with the current code, since the
thermal_sys.c also loads with fs_initcall.

Thanks,
Durga

RE: [PATCHv4 11/15] Thermal: Introduce a step_wise thermal governor

[Zhang Rui]

On 二, 2012-09-18 at 02:11 -0600, R, Durgadoss wrote:
> Hi Rui,
> 
> 
> > -----Original Message-----
> > From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-
> > owner@vger.kernel.org] On Behalf Of Zhang Rui
> > Sent: Tuesday, September 18, 2012 12:44 PM
> > To: R, Durgadoss
> > Cc: lenb@kernel.org; linux-acpi@vger.kernel.org; eduardo.valentin@ti.com
> > Subject: Re: [PATCHv4 11/15] Thermal: Introduce a step_wise thermal
> > governor
> > 
> > On 二, 2012-09-18 at 11:05 +0530, Durgadoss R wrote:
> > > This patch adds a simple step_wise governor to the
> > > generic thermal layer. This algorithm throttles the
> > > cooling devices in a linear fashion. If the 'trend'
> > > is heating, it throttles by one step. And if the
> > > thermal trend is cooling it de-throttles by one step.
> > >
> > > This actually moves the throttling logic from thermal_sys.c
> > > and puts inside step_wise.c, without any change. This is
> > > always selected as the default governor.
> > >
> > > Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
> > > ---
> > >  drivers/thermal/Kconfig     |    7 ++
> > >  drivers/thermal/Makefile    |    1 +
> > >  drivers/thermal/step_wise.c |  194
> > +++++++++++++++++++++++++++++++++++++++++++
> > >  3 files changed, 202 insertions(+)
> > >  create mode 100644 drivers/thermal/step_wise.c
> > >
> > > diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> > > index 2200912..9f6a3aa 100644
> > > --- a/drivers/thermal/Kconfig
> > > +++ b/drivers/thermal/Kconfig
> > > @@ -4,6 +4,7 @@
> > >
> > >  menuconfig THERMAL
> > >  	tristate "Generic Thermal sysfs driver"
> > > +	select STEP_WISE
> > >  	help
> > >  	  Generic Thermal Sysfs driver offers a generic mechanism for
> > >  	  thermal management. Usually it's made up of one or more thermal
> > > @@ -59,3 +60,9 @@ config FAIR_SHARE
> > >  	depends on THERMAL
> > >  	help
> > >  	  Enable this to manage platform thermals using fair-share governor.
> > > +
> > > +config STEP_WISE
> > > +	bool "Step_wise thermal governor"
> > > +	depends on THERMAL
> > > +	help
> > > +	  Enable this to manage platform thermals using a simple linear
> > 
> > if we always wants CONFIG_STEP_WISE built in, why bother to export this
> > config option to users?
> > 
> > IMO, something like
> >  menuconfig THERMAL
> >   	tristate "Generic Thermal sysfs driver"
> > 	select STEP_WISE
> > 
> >  config STEP_WISE
> > 	bool
> > 	depends on THERMAL
> > 
> > is enough, SETP_WISE will always be selected.
> > 
> > further more, we even do not need a config option for this governor,
> > just "obj-y += step_wise.o" in Makefile.
> 
> Yes, Thought through this. But I thought it would be good to align this
> with the rest of the governors.
> 

we'd better follow cpufreq governor to allow users to select the default
governor, or else this configure option is needless because we never
want it to be cleared with CONFIG_THERMAL=y.

> > 
> > another question would be,
> > as this governor registration function is fs_initcall(),
> > what if a platform thermal driver is registered and gets a thermal event
> > before this governor being registered?
> 
> Wonder if this can happen with the current code, since the
> thermal_sys.c also loads with fs_initcall.
> 
you're right. this is not a problem.

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

RE: [PATCHv4 11/15] Thermal: Introduce a step_wise thermal governor

[R, Durgadoss]

Hi Rui,


> -----Original Message-----
> From: Zhang, Rui
> Sent: Tuesday, September 18, 2012 1:52 PM
> To: R, Durgadoss
> Cc: lenb@kernel.org; linux-acpi@vger.kernel.org; eduardo.valentin@ti.com
> Subject: RE: [PATCHv4 11/15] Thermal: Introduce a step_wise thermal
> governor
> 
> On 二, 2012-09-18 at 02:11 -0600, R, Durgadoss wrote:
> > Hi Rui,
> >
> >
> > > -----Original Message-----
> > > From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-
> > > owner@vger.kernel.org] On Behalf Of Zhang Rui
> > > Sent: Tuesday, September 18, 2012 12:44 PM
> > > To: R, Durgadoss
> > > Cc: lenb@kernel.org; linux-acpi@vger.kernel.org;
> eduardo.valentin@ti.com
> > > Subject: Re: [PATCHv4 11/15] Thermal: Introduce a step_wise thermal
> > > governor
> > >
> > > On 二, 2012-09-18 at 11:05 +0530, Durgadoss R wrote:
> > > > This patch adds a simple step_wise governor to the
> > > > generic thermal layer. This algorithm throttles the
> > > > cooling devices in a linear fashion. If the 'trend'
> > > > is heating, it throttles by one step. And if the
> > > > thermal trend is cooling it de-throttles by one step.
> > > >
> > > > This actually moves the throttling logic from thermal_sys.c
> > > > and puts inside step_wise.c, without any change. This is
> > > > always selected as the default governor.
> > > >
> > > > Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
> > > > ---
> > > >  drivers/thermal/Kconfig     |    7 ++
> > > >  drivers/thermal/Makefile    |    1 +
> > > >  drivers/thermal/step_wise.c |  194
> > > +++++++++++++++++++++++++++++++++++++++++++
> > > >  3 files changed, 202 insertions(+)
> > > >  create mode 100644 drivers/thermal/step_wise.c
> > > >
> > > > diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> > > > index 2200912..9f6a3aa 100644
> > > > --- a/drivers/thermal/Kconfig
> > > > +++ b/drivers/thermal/Kconfig
> > > > @@ -4,6 +4,7 @@
> > > >
> > > >  menuconfig THERMAL
> > > >  	tristate "Generic Thermal sysfs driver"
> > > > +	select STEP_WISE
> > > >  	help
> > > >  	  Generic Thermal Sysfs driver offers a generic mechanism for
> > > >  	  thermal management. Usually it's made up of one or more thermal
> > > > @@ -59,3 +60,9 @@ config FAIR_SHARE
> > > >  	depends on THERMAL
> > > >  	help
> > > >  	  Enable this to manage platform thermals using fair-share governor.
> > > > +
> > > > +config STEP_WISE
> > > > +	bool "Step_wise thermal governor"
> > > > +	depends on THERMAL
> > > > +	help
> > > > +	  Enable this to manage platform thermals using a simple linear
> > >
> > > if we always wants CONFIG_STEP_WISE built in, why bother to export
> this
> > > config option to users?
> > >
> > > IMO, something like
> > >  menuconfig THERMAL
> > >   	tristate "Generic Thermal sysfs driver"
> > > 	select STEP_WISE
> > >
> > >  config STEP_WISE
> > > 	bool
> > > 	depends on THERMAL
> > >
> > > is enough, SETP_WISE will always be selected.
> > >
> > > further more, we even do not need a config option for this governor,
> > > just "obj-y += step_wise.o" in Makefile.
> >
> > Yes, Thought through this. But I thought it would be good to align this
> > with the rest of the governors.
> >
> 
> we'd better follow cpufreq governor to allow users to select the default
> governor, or else this configure option is needless because we never
> want it to be cleared with CONFIG_THERMAL=y.

I would prefer not to give any configure option, which exactly
is our old behavior. In this case, you want me to fix it and
re-send 11/15 or you are reviewing and should I wait for
comments on other patches ?

Thanks,
Durga

[PATCHv4 11/15] Thermal: Introduce a step_wise thermal governor

[Durgadoss R]

This patch adds a simple step_wise governor to the
generic thermal layer. This algorithm throttles the
cooling devices in a linear fashion. If the 'trend'
is heating, it throttles by one step. And if the
thermal trend is cooling it de-throttles by one step.

This actually moves the throttling logic from thermal_sys.c
and puts inside step_wise.c, without any change.

Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
---
 * This patch is a modified version of v4 series submitted here:
   http://www.spinics.net/lists/linux-acpi/msg37375.html
 * This patch removes the Kconfig option to always select
   step_wise as default governor.
 * I will submit a separate patch to let the user select
   a default thermal governor like how it is being done
   for cpufreq governors.
 drivers/thermal/Kconfig     |    6 ++
 drivers/thermal/Makefile    |    1 +
 drivers/thermal/step_wise.c |  194 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 201 insertions(+)
 create mode 100644 drivers/thermal/step_wise.c

diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index 2200912..becec31 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -59,3 +59,9 @@ config FAIR_SHARE
 	depends on THERMAL
 	help
 	  Enable this to manage platform thermals using fair-share governor.
+
+config STEP_WISE
+	bool "Step_wise thermal governor"
+	depends on THERMAL
+	help
+	  Enable this to manage platform thermals using a simple linear
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index 30540dd..4e53c00 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -8,3 +8,4 @@ obj-$(CONFIG_SPEAR_THERMAL)		+= spear_thermal.o
 obj-$(CONFIG_RCAR_THERMAL)	+= rcar_thermal.o
 obj-$(CONFIG_EXYNOS_THERMAL)		+= exynos_thermal.o
 obj-$(CONFIG_FAIR_SHARE)		+= fair_share.o
+obj-$(CONFIG_STEP_WISE)			+= step_wise.o
diff --git a/drivers/thermal/step_wise.c b/drivers/thermal/step_wise.c
new file mode 100644
index 0000000..3f9cfcf
--- /dev/null
+++ b/drivers/thermal/step_wise.c
@@ -0,0 +1,194 @@
+/*
+ *  step_wise.c - A step-by-step Thermal throttling governor
+ *
+ *  Copyright (C) 2012 Intel Corp
+ *  Copyright (C) 2012 Durgadoss R <durgadoss.r@intel.com>
+ *
+ *  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; version 2 of the License.
+ *
+ *  This program is distributed in the hope that it will be useful, but
+ *  WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ *  General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License along
+ *  with this program; if not, write to the Free Software Foundation, Inc.,
+ *  59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/module.h>
+#include <linux/thermal.h>
+
+#include "thermal_core.h"
+
+/*
+ * 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
+ */
+static unsigned long get_target_state(struct thermal_instance *instance,
+					enum thermal_trend trend)
+{
+	struct thermal_cooling_device *cdev = instance->cdev;
+	unsigned long cur_state;
+
+	cdev->ops->get_cur_state(cdev, &cur_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;
+	}
+
+	return cur_state;
+}
+
+static void update_passive_instance(struct thermal_zone_device *tz,
+				enum thermal_trip_type type, int value)
+{
+	/*
+	 * If value is +1, activate a passive instance.
+	 * If value is -1, deactivate a passive instance.
+	 */
+	if (type == THERMAL_TRIP_PASSIVE || type == THERMAL_TRIPS_NONE)
+		tz->passive += value;
+}
+
+static void update_instance_for_throttle(struct thermal_zone_device *tz,
+				int trip, enum thermal_trip_type trip_type,
+				enum thermal_trend trend)
+{
+	struct thermal_instance *instance;
+
+	list_for_each_entry(instance, &tz->thermal_instances, tz_node) {
+		if (instance->trip != trip)
+			continue;
+
+		instance->target = get_target_state(instance, trend);
+
+		/* Activate a passive thermal instance */
+		if (instance->target == THERMAL_NO_TARGET)
+			update_passive_instance(tz, trip_type, 1);
+
+		instance->cdev->updated = false; /* cdev needs update */
+	}
+}
+
+static void update_instance_for_dethrottle(struct thermal_zone_device *tz,
+				int trip, enum thermal_trip_type trip_type)
+{
+	struct thermal_instance *instance;
+	struct thermal_cooling_device *cdev;
+	unsigned long cur_state;
+
+	list_for_each_entry(instance, &tz->thermal_instances, tz_node) {
+		if (instance->trip != trip ||
+			instance->target == THERMAL_NO_TARGET)
+			continue;
+
+		cdev = instance->cdev;
+		cdev->ops->get_cur_state(cdev, &cur_state);
+
+		instance->target = cur_state > instance->lower ?
+			    (cur_state - 1) : THERMAL_NO_TARGET;
+
+		/* Deactivate a passive thermal instance */
+		if (instance->target == THERMAL_NO_TARGET)
+			update_passive_instance(tz, trip_type, -1);
+
+		cdev->updated = false; /* cdev needs update */
+	}
+}
+
+static void thermal_zone_trip_update(struct thermal_zone_device *tz, int trip)
+{
+	long trip_temp;
+	enum thermal_trip_type trip_type;
+	enum thermal_trend trend;
+
+	if (trip == THERMAL_TRIPS_NONE) {
+		trip_temp = tz->forced_passive;
+		trip_type = THERMAL_TRIPS_NONE;
+	} else {
+		tz->ops->get_trip_temp(tz, trip, &trip_temp);
+		tz->ops->get_trip_type(tz, trip, &trip_type);
+	}
+
+	trend = get_tz_trend(tz, trip);
+
+	mutex_lock(&tz->lock);
+
+	if (tz->temperature >= trip_temp)
+		update_instance_for_throttle(tz, trip, trip_type, trend);
+	else
+		update_instance_for_dethrottle(tz, trip, trip_type);
+
+	mutex_unlock(&tz->lock);
+}
+
+/**
+ * step_wise_throttle - throttles devices asscciated with the given zone
+ * @tz - thermal_zone_device
+ * @trip - the trip point
+ * @trip_type - type of the trip point
+ *
+ * Throttling Logic: This uses the trend of the thermal zone to throttle.
+ * If the thermal zone is 'heating up' this throttles all the cooling
+ * devices associated with the zone and its particular trip point, by one
+ * step. If the zone is 'cooling down' it brings back the performance of
+ * the devices by one step.
+ */
+int step_wise_throttle(struct thermal_zone_device *tz, int trip)
+{
+	struct thermal_instance *instance;
+
+	thermal_zone_trip_update(tz, trip);
+
+	if (tz->forced_passive)
+		thermal_zone_trip_update(tz, THERMAL_TRIPS_NONE);
+
+	mutex_lock(&tz->lock);
+
+	list_for_each_entry(instance, &tz->thermal_instances, tz_node)
+		thermal_cdev_update(instance->cdev);
+
+	mutex_unlock(&tz->lock);
+
+	return 0;
+}
+
+struct thermal_governor thermal_gov_step_wise = {
+	.name		= DEFAULT_THERMAL_GOVERNOR,
+	.throttle	= step_wise_throttle,
+	.owner		= THIS_MODULE,
+};
+
+static int __init thermal_gov_step_wise_init(void)
+{
+	return thermal_register_governor(&thermal_gov_step_wise);
+}
+
+static void __exit thermal_gov_step_wise_exit(void)
+{
+	thermal_unregister_governor(&thermal_gov_step_wise);
+}
+
+/* This should load after thermal framework */
+fs_initcall(thermal_gov_step_wise_init);
+module_exit(thermal_gov_step_wise_exit);
+
+MODULE_AUTHOR("Durgadoss R");
+MODULE_DESCRIPTION("A step-by-step thermal throttling governor");
+MODULE_LICENSE("GPL");
-- 
1.7.9.5