[PATCH v5 3/6] thermal: Added Bang-bang thermal governor

[PATCH v5 3/6] thermal: Added Bang-bang thermal governor

[Peter Feuerer]

The bang-bang thermal governor uses a hysteresis to switch abruptly on
or off a cooling device.  It is intended to control fans, which can
not be throttled but just switched on or off.
Bang-bang cannot be set as default governor as it is intended for
special devices only.  For those special devices the driver needs to
explicitely request it.

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Zhang Rui <rui.zhang@intel.com>
Cc: Andreas Mohr <andi@lisas.de>
Cc: Borislav Petkov <bp@suse.de>
Cc: Javi Merino <javi.merino@arm.com>
Cc: linux-pm@vger.kernel.org
Signed-off-by: Peter Feuerer <peter@piie.net>
---
 drivers/thermal/Kconfig         |  10 +++
 drivers/thermal/Makefile        |   1 +
 drivers/thermal/gov_bang_bang.c | 131 ++++++++++++++++++++++++++++++++++++++++
 drivers/thermal/thermal_core.c  |   5 ++
 drivers/thermal/thermal_core.h  |   8 +++
 5 files changed, 155 insertions(+)
 create mode 100644 drivers/thermal/gov_bang_bang.c

diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index f9a1386..fa14d56 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -84,6 +84,16 @@ config THERMAL_GOV_STEP_WISE
 	  Enable this to manage platform thermals using a simple linear
 	  governor.
 
+config THERMAL_GOV_BANG_BANG
+	bool "Bang Bang thermal governor"
+	default n
+	help
+	  Enable this to manage platform thermals using bang bang governor.
+
+	  Say 'Y' here if you want to use two point temperature regulation
+	  used for fans without throttling.  Some fan drivers depend on this
+	  governor to be enabled (e.g. acerhdf).
+
 config THERMAL_GOV_USER_SPACE
 	bool "User_space thermal governor"
 	help
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index de0636a..a675980 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -11,6 +11,7 @@ thermal_sys-$(CONFIG_THERMAL_OF)		+= of-thermal.o
 
 # governors
 thermal_sys-$(CONFIG_THERMAL_GOV_FAIR_SHARE)	+= fair_share.o
+thermal_sys-$(CONFIG_THERMAL_GOV_BANG_BANG)	+= gov_bang_bang.o
 thermal_sys-$(CONFIG_THERMAL_GOV_STEP_WISE)	+= step_wise.o
 thermal_sys-$(CONFIG_THERMAL_GOV_USER_SPACE)	+= user_space.o
 
diff --git a/drivers/thermal/gov_bang_bang.c b/drivers/thermal/gov_bang_bang.c
new file mode 100644
index 0000000..c5dd76b
--- /dev/null
+++ b/drivers/thermal/gov_bang_bang.c
@@ -0,0 +1,131 @@
+/*
+ *  gov_bang_bang.c - A simple thermal throttling governor using hysteresis
+ *
+ *  Copyright (C) 2014 Peter Feuerer <peter@piie.net>
+ *
+ *  Based on step_wise.c with following Copyrights:
+ *  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.
+ *
+ * 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.
+ *
+ */
+
+#include <linux/thermal.h>
+
+#include "thermal_core.h"
+
+static void thermal_zone_trip_update(struct thermal_zone_device *tz, int trip)
+{
+	long trip_temp;
+	unsigned long trip_hyst;
+	struct thermal_instance *instance;
+
+	tz->ops->get_trip_temp(tz, trip, &trip_temp);
+	tz->ops->get_trip_hyst(tz, trip, &trip_hyst);
+
+	dev_dbg(&tz->device, "Trip%d[temp=%ld]:temp=%d:hyst=%ld\n",
+				trip, trip_temp, tz->temperature,
+				trip_hyst);
+
+	mutex_lock(&tz->lock);
+
+	list_for_each_entry(instance, &tz->thermal_instances, tz_node) {
+		if (instance->trip != trip)
+			continue;
+
+		/* in case fan is in initial state, switch the fan off */
+		if (instance->target == THERMAL_NO_TARGET)
+			instance->target = 0;
+
+		/* in case fan is neither on nor off set the fan to active */
+		if (instance->target != 0 && instance->target != 1) {
+			pr_warn("Thermal instance %s controlled by bang-bang has unexpected state: %ld\n",
+					instance->name, instance->target);
+			instance->target = 1;
+		}
+
+		/*
+		 * enable fan when temperature exceeds trip_temp and disable
+		 * the fan in case it falls below trip_temp minus hysteresis
+		 */
+		if (instance->target == 0 && tz->temperature >= trip_temp)
+			instance->target = 1;
+		else if (instance->target == 1 &&
+				tz->temperature < trip_temp - trip_hyst)
+			instance->target = 0;
+
+		dev_dbg(&instance->cdev->device, "target=%d\n",
+					(int)instance->target);
+
+		instance->cdev->updated = false; /* cdev needs update */
+	}
+
+	mutex_unlock(&tz->lock);
+}
+
+/**
+ * bang_bang_control - controls devices associated with the given zone
+ * @tz - thermal_zone_device
+ * @trip - the trip point
+ *
+ * Regulation Logic: a two point regulation, deliver cooling state depending
+ * on the previous state shown in this diagram:
+ *
+ *                Fan:   OFF    ON
+ *
+ *                              |
+ *                              |
+ *          trip_temp:    +---->+
+ *                        |     |        ^
+ *                        |     |        |
+ *                        |     |   Temperature
+ * (trip_temp - hyst):    +<----+
+ *                        |
+ *                        |
+ *                        |
+ *
+ *   * If the fan is not running and temperature exceeds trip_temp, the fan
+ *     gets turned on.
+ *   * In case the fan is running, temperature must fall below
+ *     (trip_temp - hyst) so that the fan gets turned off again.
+ *
+ */
+static int bang_bang_control(struct thermal_zone_device *tz, int trip)
+{
+	struct thermal_instance *instance;
+
+	thermal_zone_trip_update(tz, trip);
+
+	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;
+}
+
+static struct thermal_governor thermal_gov_bang_bang = {
+	.name		= "bang_bang",
+	.throttle	= bang_bang_control,
+};
+
+int thermal_gov_bang_bang_register(void)
+{
+	return thermal_register_governor(&thermal_gov_bang_bang);
+}
+
+void thermal_gov_bang_bang_unregister(void)
+{
+	thermal_unregister_governor(&thermal_gov_bang_bang);
+}
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 71b0ec0..4c2726b 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -1790,6 +1790,10 @@ static int __init thermal_register_governors(void)
 	if (result)
 		return result;
 
+	result = thermal_gov_bang_bang_register();
+	if (result)
+		return result;
+
 	return thermal_gov_user_space_register();
 }
 
@@ -1797,6 +1801,7 @@ static void thermal_unregister_governors(void)
 {
 	thermal_gov_step_wise_unregister();
 	thermal_gov_fair_share_unregister();
+	thermal_gov_bang_bang_unregister();
 	thermal_gov_user_space_unregister();
 }
 
diff --git a/drivers/thermal/thermal_core.h b/drivers/thermal/thermal_core.h
index 3db339f..d15d243 100644
--- a/drivers/thermal/thermal_core.h
+++ b/drivers/thermal/thermal_core.h
@@ -69,6 +69,14 @@ static inline int thermal_gov_fair_share_register(void) { return 0; }
 static inline void thermal_gov_fair_share_unregister(void) {}
 #endif /* CONFIG_THERMAL_GOV_FAIR_SHARE */
 
+#ifdef CONFIG_THERMAL_GOV_BANG_BANG
+int thermal_gov_bang_bang_register(void);
+void thermal_gov_bang_bang_unregister(void);
+#else
+static inline int thermal_gov_bang_bang_register(void) { return 0; }
+static inline void thermal_gov_bang_bang_unregister(void) {}
+#endif /* CONFIG_THERMAL_GOV_BANG_BANG */
+
 #ifdef CONFIG_THERMAL_GOV_USER_SPACE
 int thermal_gov_user_space_register(void);
 void thermal_gov_user_space_unregister(void);
-- 
2.0.2

Re: [PATCH v5 3/6] thermal: Added Bang-bang thermal governor

[Peter Feuerer]

Hi Rui,

Peter Feuerer writes:

> The bang-bang thermal governor uses a hysteresis to switch abruptly on
> or off a cooling device.  It is intended to control fans, which can
> not be throttled but just switched on or off.
> Bang-bang cannot be set as default governor as it is intended for
> special devices only.  For those special devices the driver needs to
> explicitely request it.
> 
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Zhang Rui <rui.zhang@intel.com>

Anything that prevents you from giving your acked-by?

[...]

-- 
kind regards,
--peter;

Re: [PATCH v5 3/6] thermal: Added Bang-bang thermal governor

[Zhang Rui]

On Sat, 2014-07-26 at 16:14 +0200, Peter Feuerer wrote:
> Hi Rui,
> 
> Peter Feuerer writes:
> 
> > The bang-bang thermal governor uses a hysteresis to switch abruptly on
> > or off a cooling device.  It is intended to control fans, which can
> > not be throttled but just switched on or off.
> > Bang-bang cannot be set as default governor as it is intended for
> > special devices only.  For those special devices the driver needs to
> > explicitely request it.
> > 
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Cc: Zhang Rui <rui.zhang@intel.com>
> 
> Anything that prevents you from giving your acked-by?
> 
NO.

I'll queue them for 3.18.

thanks,
rui

Re: [PATCH v5 3/6] thermal: Added Bang-bang thermal governor

[Peter Feuerer]

Hi Rui,

Zhang Rui writes:

> On Sat, 2014-07-26 at 16:14 +0200, Peter Feuerer wrote:
>> Hi Rui,
>> 
>> Peter Feuerer writes:
>> 
>> > The bang-bang thermal governor uses a hysteresis to switch abruptly on
>> > or off a cooling device.  It is intended to control fans, which can
>> > not be throttled but just switched on or off.
>> > Bang-bang cannot be set as default governor as it is intended for
>> > special devices only.  For those special devices the driver needs to
>> > explicitely request it.
>> > 
>> > Cc: Andrew Morton <akpm@linux-foundation.org>
>> > Cc: Zhang Rui <rui.zhang@intel.com>
>> 
>> Anything that prevents you from giving your acked-by?
>> 
> NO.
> 
> I'll queue them for 3.18.

Are all 6 patches in for 3.18 as you promissed?

kind regards,
--peter;

Re: [PATCH v5 3/6] thermal: Added Bang-bang thermal governor

[Peter Feuerer]

Hi Rui,

I wonder whether you've had time to apply my set of patches already?  Would 
you please be so kind to just send me a short reply?

thanks and kind regards,
--peter;


Peter Feuerer writes:

> Hi Rui,
> 
> Zhang Rui writes:
> 
>> On Sat, 2014-07-26 at 16:14 +0200, Peter Feuerer wrote:
>>> Hi Rui,
>>> 
>>> Peter Feuerer writes:
>>> 
>>> > The bang-bang thermal governor uses a hysteresis to switch abruptly on
>>> > or off a cooling device.  It is intended to control fans, which can
>>> > not be throttled but just switched on or off.
>>> > Bang-bang cannot be set as default governor as it is intended for
>>> > special devices only.  For those special devices the driver needs to
>>> > explicitely request it.
>>> > 
>>> > Cc: Andrew Morton <akpm@linux-foundation.org>
>>> > Cc: Zhang Rui <rui.zhang@intel.com>
>>> 
>>> Anything that prevents you from giving your acked-by?
>>> 
>> NO.
>> 
>> I'll queue them for 3.18.
> 
> Are all 6 patches in for 3.18 as you promissed?
> 
> kind regards,
> --peter;

Re: [PATCH v5 3/6] thermal: Added Bang-bang thermal governor

[Javi Merino]

On Tue, Oct 28, 2014 at 07:33:39PM +0000, Peter Feuerer wrote:
> Hi Rui,
> 
> I wonder whether you've had time to apply my set of patches already?  Would 
> you please be so kind to just send me a short reply?

The bang-bang governor was merged and is part of v3.18-rc2:

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=8264fce6de03f3915e2301f52f181a982718a8cb

Cheers,
Javi

> Peter Feuerer writes:
> 
> > Hi Rui,
> > 
> > Zhang Rui writes:
> > 
> >> On Sat, 2014-07-26 at 16:14 +0200, Peter Feuerer wrote:
> >>> Hi Rui,
> >>> 
> >>> Peter Feuerer writes:
> >>> 
> >>> > The bang-bang thermal governor uses a hysteresis to switch abruptly on
> >>> > or off a cooling device.  It is intended to control fans, which can
> >>> > not be throttled but just switched on or off.
> >>> > Bang-bang cannot be set as default governor as it is intended for
> >>> > special devices only.  For those special devices the driver needs to
> >>> > explicitely request it.
> >>> > 
> >>> > Cc: Andrew Morton <akpm@linux-foundation.org>
> >>> > Cc: Zhang Rui <rui.zhang@intel.com>
> >>> 
> >>> Anything that prevents you from giving your acked-by?
> >>> 
> >> NO.
> >> 
> >> I'll queue them for 3.18.
> > 
> > Are all 6 patches in for 3.18 as you promissed?
> > 
> > kind regards,
> > --peter;
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

Re: [PATCH v5 3/6] thermal: Added Bang-bang thermal governor

[Peter Feuerer]

Hi Javi,

Javi Merino writes:

> On Tue, Oct 28, 2014 at 07:33:39PM +0000, Peter Feuerer wrote:
>> Hi Rui,
>> 
>> I wonder whether you've had time to apply my set of patches already?  Would 
>> you please be so kind to just send me a short reply?
> 
> The bang-bang governor was merged and is part of v3.18-rc2:
> 
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=8264fce6de03f3915e2301f52f181a982718a8cb

Thanks for your mail.

Seems like the other 5 patches of the series were not applied.  So I 
need to take care again, or?

kind regards,
--peter;


> 
> Cheers,
> Javi
> 
>> Peter Feuerer writes:
>> 
>> > Hi Rui,
>> > 
>> > Zhang Rui writes:
>> > 
>> >> On Sat, 2014-07-26 at 16:14 +0200, Peter Feuerer wrote:
>> >>> Hi Rui,
>> >>> 
>> >>> Peter Feuerer writes:
>> >>> 
>> >>> > The bang-bang thermal governor uses a hysteresis to switch abruptly on
>> >>> > or off a cooling device.  It is intended to control fans, which can
>> >>> > not be throttled but just switched on or off.
>> >>> > Bang-bang cannot be set as default governor as it is intended for
>> >>> > special devices only.  For those special devices the driver needs to
>> >>> > explicitely request it.
>> >>> > 
>> >>> > Cc: Andrew Morton <akpm@linux-foundation.org>
>> >>> > Cc: Zhang Rui <rui.zhang@intel.com>
>> >>> 
>> >>> Anything that prevents you from giving your acked-by?
>> >>> 
>> >> NO.
>> >> 
>> >> I'll queue them for 3.18.
>> > 
>> > Are all 6 patches in for 3.18 as you promissed?
>> > 
>> > kind regards,
>> > --peter;
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at  http://www.tux.org/lkml/
>> 
> 

Re: [RESEND PATCH v5 3/5] acerhdf: Use bang-bang thermal governor

[Darren Hart]

On Thu, Dec 04, 2014 at 08:21:06AM +0100, Peter Feuerer wrote:
> Hi again,
> 
> Peter Feuerer writes:
> 
> >Hi Darren,
> >
> >thank you very much for your reply.
> >
> >
> >Darren Hart writes:
> >
> >>On Fri, Nov 28, 2014 at 03:20:50PM +0100, Peter Feuerer wrote:
> >>>acerhdf has been doing an on-off fan control using hysteresis by
> >>>post-manipulating the outcome of thermal subsystem trip point handling.
> >>>This patch enables acerhdf to use the bang-bang governor, which is
> >>>intended for on-off controlled fans.
> >>>
> >>>Cc: platform-driver-x86@vger.kernel.org
> >>>Cc: Darren Hart <dvhart@infradead.org>
> >>>Cc: Andrew Morton <akpm@linux-foundation.org>
> >>>CC: Zhang Rui <rui.zhang@intel.com>
> >>>Cc: Andreas Mohr <andi@lisas.de>
> >>>Cc: Javi Merino <javi.merino@arm.com>
> >>>Acked-and-tested-by: Borislav Petkov <bp@suse.de>
> >>>Signed-off-by: Peter Feuerer <peter@piie.net>
> >>>---
> >>> drivers/platform/x86/Kconfig   |  3 ++-
> >>> drivers/platform/x86/acerhdf.c | 36 +++++++++++++++++++++++++++++++-----
> >>> 2 files changed, 33 insertions(+), 6 deletions(-)
> >>>
> >>>diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> >>>index a2eabe6..c173266 100644
> >>>--- a/drivers/platform/x86/Kconfig
> >>>+++ b/drivers/platform/x86/Kconfig
> >>>@@ -38,7 +38,8 @@ config ACER_WMI
> >>> config ACERHDF
> >>> 	tristate "Acer Aspire One temperature and fan driver"
> >>>-	depends on THERMAL && ACPI
> >>>+	select THERMAL_GOV_BANG_BANG
> >>
> >>So we use select sparingly as it does implicit things.
> >>
> >>I checked the THERMAL_GOV_BANG_BANG Kconfig entry, and the help says acerhdf
> >>already depends on it (which it doesn't appear to). Any particular reason to add
> >>select here instead of adding it as a depends.
> >>
> >>Why did you drop THERMAL?
> >
> >I had it like this in my first version of patches:
> >+ depends on THERMAL && ACPI && THERMAL_GOV_BANG_BANG
> >
> >But after some discussion with lkml community we ended up with the select
> >line and dropped THERMAL dependency, as it is implied by
> >THEMAL_GOV_BANG_BANG.  I'm not so experienced with Kconfig, so I must rely
> >on the statements of the community in this case.
> 
> Just found the link about this discussion I had with Rui and Boris:
> http://linux-kernel.2935.n7.nabble.com/PATCH-0-4-acerhdf-thermal-adding-new-models-and-appropriate-governor-tp848572p908256.html
> 

Rui,

I see you said you agree with the approach to "select THERMAL_GOV_BANG_BANG",
and I'm OK with that. But that doesn't ensure, as far as I can see, a
sufficient set of Kconfig dependencies for ACERHDF as THERMAL_GOV_BANG_BANG
doesn't in turn select THERMAL.

While it's unlikely to get ACERHDF available without THERMAL enabled, it does
look possible. Should this driver also select or depends on THERMAL?

Maybe:

depends on ACPI
depends on THERMAL
select THERMAL_GOV_BANG_BANG

?

> [...]
> 
> -- 
> kind regards,
> --peter;
> 

-- 
Darren Hart
Intel Open Source Technology Center