From owner-acpi-jp@jp.freebsd.org  Sat Oct 27 18:14:40 2001
Received: (from daemon@localhost)
	by castle.jp.freebsd.org (8.9.3+3.2W/8.7.3) id SAA27393;
	Sat, 27 Oct 2001 18:14:40 +0900 (JST)
	(envelope-from owner-acpi-jp@jp.FreeBSD.org)
Received: from tasogare.imasy.or.jp (root@tasogare.imasy.or.jp [202.227.24.5])
	by castle.jp.freebsd.org (8.9.3+3.2W/8.7.3) with ESMTP id SAA27388
	for <acpi-jp@jp.freebsd.org>; Sat, 27 Oct 2001 18:14:39 +0900 (JST)
	(envelope-from iwasaki@jp.FreeBSD.org)
Received: from localhost (iwasaki.imasy.or.jp [202.227.24.92])
	(authenticated as iwa with CRAM-MD5)
	by tasogare.imasy.or.jp (8.11.6+3.4W/8.11.6/tasogare) with ESMTP/inet id f9R9EVm29777
	for <acpi-jp@jp.freebsd.org>; Sat, 27 Oct 2001 18:14:31 +0900 (JST)
	(envelope-from iwasaki@jp.FreeBSD.org)
Date: Sat, 27 Oct 2001 18:14:27 +0900 (JST)
Message-Id: <20011027.181427.41653970.iwasaki@jp.FreeBSD.org>
To: acpi-jp@jp.freebsd.org
From: Mitsuru IWASAKI <iwasaki@jp.freebsd.org>
In-Reply-To: <200110261855.f9QIt5v02254@mass.dis.org>
References: <20011027.033154.59470171.iwasaki@jp.FreeBSD.org>
	<200110261855.f9QIt5v02254@mass.dis.org>
X-Mailer: Mew version 2.0 on Emacs 20.7 / Mule 4.0 (HANANOEN)
Mime-Version: 1.0
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Reply-To: acpi-jp@jp.freebsd.org
Precedence: list
X-Distribute: distribute version 2.1 (Alpha) patchlevel 24e+010328
X-Sequence: acpi-jp 1381
Subject: [acpi-jp 1381] Re: acpi_thermal: small improvement, delayed
 cooler off and verbose 
Errors-To: owner-acpi-jp@jp.freebsd.org
Sender: owner-acpi-jp@jp.freebsd.org
X-Originator: iwasaki@jp.freebsd.org

Hi, thanks mike for your comments.

> I think it would be better to simply save time_second in the softc 
> (remember that there can be more than one thermal zone) and refuse to 
> switch to "none" when the difference between the saved time_second and 
> the current value is smaller than the preset delay.

Agreed.  It's much better than my original idea.
Also I changed to have minimum run time and verbose mode in
acpi_thermal module global, instead of having MIBs for each thermal
zones.

And how about generalizing this idea and applying this whenever
getting _ACx level down too rather than only for switching cooling off?
I don't have hardware with many _ACx levels than _AC0 around here but
I think it would be worth to consider, in order to recude too many
Oct 26 23:43:33 fivavl ... : acpi_tz0: _AC0: temperature 68.0 >= setpoint 68.0
Oct 26 23:43:33 fivavl ... : acpi_tz0: switched from _AC1 to _AC0: 68.0C
Oct 26 23:43:37 fivavl ... : acpi_tz0: switched from _AC0 to _AC1: 67.0C

> This gives a guaranteed minimum run time, without affecting the cooling 
> system's ability to react quickly to increases in temperature, or 
> preventing a more advanced cooling system from cycling within several _AC 
> levels.

`minimum run time' seems to be good naming for me :)
I created hw.acpi.thermal.min_runtime in my revised patch.

Thanks

Index: acpi_thermal.c
===================================================================
RCS file: /home/ncvs/src/sys/dev/acpica/acpi_thermal.c,v
retrieving revision 1.16
diff -u -r1.16 acpi_thermal.c
--- acpi_thermal.c	4 Sep 2001 15:40:12 -0000	1.16
+++ acpi_thermal.c	27 Oct 2001 08:01:57 -0000
@@ -84,6 +84,7 @@
     int				tz_flags;
 #define TZ_FLAG_NO_SCP		(1<<0)			/* no _SCP method */
 #define TZ_FLAG_GETPROFILE	(1<<1)			/* fetch powerprofile in timeout */
+    struct timespec		tz_cooling_started;	/* current cooling starting time */
 
     struct sysctl_ctx_list	tz_sysctl_ctx;		/* sysctl tree */
     struct sysctl_oid		*tz_sysctl_tree;
@@ -125,6 +126,9 @@
 static struct sysctl_ctx_list	acpi_tz_sysctl_ctx;
 static struct sysctl_oid	*acpi_tz_sysctl_tree;
 
+static int			acpi_tz_min_runtime = 0;/* minimum cooling run time */
+static int			acpi_tz_verbose = 0;	/* verbose mode */
+
 /*
  * Match an ACPI thermal zone.
  */
@@ -168,6 +172,8 @@
     sc->tz_dev = dev;
     sc->tz_handle = acpi_get_handle(dev);
     sc->tz_requested = TZ_ACTIVE_NONE;
+    if (bootverbose)
+	acpi_tz_verbose = 1;
 
     /*
      * Parse the current state of the thermal zone and build control
@@ -193,6 +199,14 @@
 	acpi_tz_sysctl_tree = SYSCTL_ADD_NODE(&acpi_tz_sysctl_ctx,
 					      SYSCTL_CHILDREN(acpi_sc->acpi_sysctl_tree),
 					      OID_AUTO, "thermal", CTLFLAG_RD, 0, "");
+	SYSCTL_ADD_INT(&acpi_tz_sysctl_ctx,
+		       SYSCTL_CHILDREN(acpi_tz_sysctl_tree),
+		       OID_AUTO, "min_runtime", CTLFLAG_RD | CTLFLAG_RW,
+		       &acpi_tz_min_runtime, 0, "minimum cooling run time in sec");
+	SYSCTL_ADD_INT(&acpi_tz_sysctl_ctx,
+		       SYSCTL_CHILDREN(acpi_tz_sysctl_tree),
+		       OID_AUTO, "verbose", CTLFLAG_RD | CTLFLAG_RW,
+		       &acpi_tz_verbose, 0, "verbose mode");
     }
     sysctl_ctx_init(&sc->tz_sysctl_ctx);
     sprintf(oidname, "tz%d", device_get_unit(dev));
@@ -324,6 +338,20 @@
     return_VALUE(0);
 }
 
+static char	*aclevel_string[] =	{
+	"NONE", "_AC0", "_AC1", "_AC2", "_AC3", "_AC4",
+	"_AC5", "_AC6", "_AC7", "_AC8", "_AC9" };
+
+static __inline const char *
+acpi_tz_aclevel_string(int active)
+{
+	if (active < -1 || active >= TZ_NUMLEVELS) {
+		return (aclevel_string[0]);
+	}
+
+	return (aclevel_string[active+1]);
+}
+
 /*
  * Evaluate the condition of a thermal zone, take appropriate actions.
  */
@@ -333,6 +361,7 @@
     int		temp;
     int		i;
     int		newactive, newflags;
+    struct	timespec curtime;
 
     FUNCTION_TRACE(__func__);
 
@@ -359,13 +388,29 @@
 	if ((sc->tz_zone.ac[i] != -1) && (temp >= sc->tz_zone.ac[i])) {
 	    newactive = i;
 	    if (sc->tz_active != newactive) {
-		device_printf(sc->tz_dev,
-			      "_AC%d: temperature %d.%d >= setpoint %d.%d\n", i,
-			      TZ_KELVTOC(temp), TZ_KELVTOC(sc->tz_zone.ac[i]));
+		if (acpi_tz_verbose) {
+		    device_printf(sc->tz_dev,
+			"_AC%d: temperature %d.%d >= setpoint %d.%d\n", i,
+			TZ_KELVTOC(temp), TZ_KELVTOC(sc->tz_zone.ac[i]));
+		}
+		getnanotime(&sc->tz_cooling_started);
+		break;
 	    }
 	}
     }
 
+    /*
+     * We are going to get _ACx level down, but give a guaranteed
+     * minimum cooling run time if requested.
+     */
+    if (newactive < sc->tz_active && acpi_tz_min_runtime > 0) {
+	getnanotime(&curtime);
+	timespecsub(&curtime, &sc->tz_cooling_started);
+	if (curtime.tv_sec < acpi_tz_min_runtime) {
+	    newactive = sc->tz_active;
+	}
+    }
+
     /* handle user override of active mode */
     if (sc->tz_requested > newactive)
 	newactive = sc->tz_requested;
@@ -393,7 +438,11 @@
 	if (newactive != TZ_ACTIVE_NONE)
 	    acpi_ForeachPackageObject((ACPI_OBJECT *)sc->tz_zone.al[newactive].Pointer,
 				      acpi_tz_switch_cooler_on, sc);
-	device_printf(sc->tz_dev, "switched from _AC%d to _AC%d\n", sc->tz_active, newactive);
+	if (acpi_tz_verbose) {
+	    device_printf(sc->tz_dev, "switched from %s to %s: %d.%dC\n",
+		acpi_tz_aclevel_string(sc->tz_active),
+		acpi_tz_aclevel_string(newactive), TZ_KELVTOC(temp));
+	}
 	sc->tz_active = newactive;
     }
 
