Subject: apm (Re: Anyone running NetBSD on an IBM ThinkPad [A|T]21?)
To: None <port-i386@netbsd.org>
From: Chuck Cranor <chuck@research.att.com>
List: port-i386
Date: 01/18/2001 23:49:21
On Sun, Jan 14, 2001 at 10:24:40PM -0400, Jared D. McNeill wrote:
> On Sun, 14 Jan 2001, Chuck Cranor wrote:
> > same here, its an old bug.   it only happens if you are running "apmd" ...
> > kill "apmd" and close the top and it will not happen.  the same problem
> > occurs on an old IBM 560 thinkpad i have.   i'm going to look into fixing
> > it (suspect it has already been fixed elsewhere, so it just a matter of
> > pulling in the right diffs).
 
> FWIW, the same thing happens on my AST Ascentia 910N.


two things regarding apm:
first, i noticed today that between 1.4P and 1.5 some change
was made that prevents APM suspend from working properly on 
a thinkpad 560.  (with newer kernels the APM BIOS rejects the 
suspend request with "0x60 Unable to enter requested state".)  
i have not isolated what the cause of this problem is yet.


second, i have determined the cause of the "closing the laptop lid
puts my computer to sleep twice" problem quoted above.   i even have 
a patch that seems to fix it.  the problem is the while loop in 
apm_periodic_check and how it interacts with the Thinkpad APM BIOS:   

        while (apm_get_event(&regs) == 0 && !apm_damn_fool_bios)
                apm_event_handle(sc, &regs);

it seems that when the Thinkpad APM BIOS posts a suspend event the 
event remains queued until it is either rejected or accepted (doing 
a APM_LASTREQ_INPROG does not clear it).

there is a 16 event queue that the apm driver uses to spool events 
to the user process (apmd).   this queue is only used if apmd 
is running (otherwise the apm driver handles all requests directly).


when you close the lid a suspend event is posted.  the driver then
hits the while loop.    it apm_get_event's the suspend event, and
calls apm_event_handle().   apm_event_handle() queues the event
for apmd and then does a APM_LASTREQ_INPROG set power command.

it then does another apm_get_event() and gets a duplicate copy of
the suspend event and repeats the above.  eventually the 16 item
queue fills because the kernel has not given apmd a chance to run.
once the queue is full, the apm driver decides to handle the suspend
even itself and suspends the computer (it break the loop by setting
"apm_damn_fool_bios").

the computer goes to sleep.   then when you open the lid, the computer
wakes up.   as part of that apmd eventually runs and collects the 
suspend events posted to it before the lid was closed.   it responds
by putting the computer back into suspend mode.

i reworked the duplicate event detection code to handle this case.
i also added improved debugging output to apmd.c to make it easier
to understand what is going on inside the driver.   a preliminary
version of my changes are appended below (still needs some minor 
cleanup).

chuck



Index: apm.c
===================================================================
RCS file: /cvsroot/syssrc/sys/arch/i386/i386/apm.c,v
retrieving revision 1.50.2.1
diff -c -r1.50.2.1 apm.c
*** apm.c	2000/08/04 05:34:29	1.50.2.1
--- apm.c	2001/01/19 04:45:24
***************
*** 47,52 ****
--- 47,53 ----
  #error APM_NOIDLE option deprecated; use APM_NO_IDLE instead
  #endif
  
+ #define APMDEBUG
  #if defined(DEBUG) && !defined(APMDEBUG)
  #define	APMDEBUG
  #endif
***************
*** 143,149 ****
  static void	apm_devpowmgt_enable __P((int, u_int));
  static void	apm_disconnect __P((void *));
  #endif
! static void	apm_event_handle __P((struct apm_softc *, struct bioscallregs *));
  static int	apm_get_event __P((struct bioscallregs *));
  static int	apm_get_powstat __P((struct bioscallregs *));
  #if 0
--- 144,150 ----
  static void	apm_devpowmgt_enable __P((int, u_int));
  static void	apm_disconnect __P((void *));
  #endif
! static int	apm_event_handle __P((struct apm_softc *, struct bioscallregs *));
  static int	apm_get_event __P((struct bioscallregs *));
  static int	apm_get_powstat __P((struct bioscallregs *));
  #if 0
***************
*** 217,229 ****
  struct apm_connect_info apminfo;
  u_char	apm_majver, apm_minver;
  int	apm_inited;
- int	apm_standbys, apm_userstandbys, apm_suspends, apm_battlow;
- int	apm_damn_fool_bios, apm_op_inprog;
  int	apm_evindex;
  
  #ifdef APMDEBUG
  int	apmcall_debug(int, struct bioscallregs *, int);
  
  int
  apmcall_debug(func, regs, line)
  	int func;
--- 218,315 ----
  struct apm_connect_info apminfo;
  u_char	apm_majver, apm_minver;
  int	apm_inited;
  int	apm_evindex;
  
+ /* set if we should standby/suspend at the end of next periodic event */
+ int apm_standby_now, apm_suspend_now;
+ 
+ /* 
+  * set if kernel is planning on doing a standby/suspend, or if we are
+  * waiting for an external program to process a standby/suspend event.
+  */
+ int apm_standby_pending, apm_suspend_pending;
+ 
  #ifdef APMDEBUG
  int	apmcall_debug(int, struct bioscallregs *, int);
  
+ #define ACPF_AX		0x00000001
+ #define ACPF_AX_HI	0x00000002
+ #define ACPF_EAX	0x00000004
+ #define ACPF_BX 	0x00000008
+ #define ACPF_BX_HI 	0x00000010
+ #define ACPF_EBX 	0x00000020
+ #define ACPF_CX 	0x00000040
+ #define ACPF_CX_HI 	0x00000080
+ #define ACPF_ECX 	0x00000100
+ #define ACPF_DX 	0x00000200
+ #define ACPF_DX_HI 	0x00000400
+ #define ACPF_EDX 	0x00000800
+ #define ACPF_SI 	0x00001000
+ #define ACPF_SI_HI 	0x00002000
+ #define ACPF_ESI 	0x00004000
+ #define ACPF_DI 	0x00008000
+ #define ACPF_DI_HI 	0x00010000
+ #define ACPF_EDI 	0x00020000
+ #define ACPF_FLAGS 	0x00040000
+ #define ACPF_FLAGS_HI	0x00080000
+ #define ACPF_EFLAGS 	0x00100000
+ 
+ struct acallinfo {
+ 	char *name;
+ 	int inflag;
+ 	int outflag;
+ };
+ 
+ static struct acallinfo aci[] = {
+   { "install_check", ACPF_BX, ACPF_AX|ACPF_BX|ACPF_CX },
+   { "real_connect", ACPF_BX, 0 },
+   { "p16_connect", ACPF_BX, ACPF_AX|ACPF_BX|ACPF_CX|ACPF_SI|ACPF_DI },
+   { "p32_connect", ACPF_BX, ACPF_AX|ACPF_EBX|ACPF_CX|ACPF_DX|ACPF_ESI|ACPF_DI },
+   { "disconnect", ACPF_BX, 0 },
+   { "cpu_idle", 0, 0 },
+   { "cpu_busy", 0, 0 },
+   { "set_power_state", ACPF_BX|ACPF_CX, 0 },
+   { "enable_power_state", ACPF_BX|ACPF_CX, 0 }, 
+   { "restore_defaults", ACPF_BX, 0 },
+   { "get_power_status", ACPF_BX, ACPF_BX|ACPF_CX|ACPF_DX|ACPF_SI },
+   { "get_event", 0, ACPF_BX|ACPF_CX },
+   { "get_power_state" , ACPF_BX, ACPF_CX }, 
+   { "enable_dev_power_mgt", ACPF_BX|ACPF_CX, 0 },
+   { "driver_version", ACPF_BX|ACPF_CX, ACPF_AX },
+   { "engage_power_mgt",  ACPF_BX|ACPF_CX, 0 },
+   { "get_caps", ACPF_BX, ACPF_BX|ACPF_CX },
+   { "resume_timer", ACPF_BX|ACPF_CX|ACPF_SI|ACPF_DI, ACPF_CX|ACPF_SI|ACPF_DI },
+   { "resume_ring", ACPF_BX|ACPF_CX, ACPF_CX },
+   { "timer_reqs", ACPF_BX|ACPF_CX, ACPF_CX },
+ };
+ 
+ static void acallpr(int, char *, struct bioscallregs *);
+ static void acallpr(int flag, char *tag, struct bioscallregs *b) {
+   if (!flag) return;
+   printf("%s ", tag);
+   if (flag & ACPF_AX) 		printf("ax=%#x ", b->AX);
+   if (flag & ACPF_AX_HI) 	printf("ax_hi=%#x ", b->AX_HI);
+   if (flag & ACPF_EAX) 		printf("eax=%#x ", b->EAX);
+   if (flag & ACPF_BX ) 		printf("bx=%#x ", b->BX);
+   if (flag & ACPF_BX_HI ) 	printf("bx_hi=%#x ", b->BX_HI);
+   if (flag & ACPF_EBX ) 	printf("ebx=%#x ", b->EBX);
+   if (flag & ACPF_CX ) 		printf("cx=%#x ", b->CX);
+   if (flag & ACPF_CX_HI ) 	printf("cx_hi=%#x ", b->CX_HI);
+   if (flag & ACPF_ECX ) 	printf("ecx=%#x ", b->ECX);
+   if (flag & ACPF_DX ) 		printf("dx=%#x ", b->DX);
+   if (flag & ACPF_DX_HI ) 	printf("dx_hi=%#x ", b->DX_HI);
+   if (flag & ACPF_EDX ) 	printf("edx=%#x ", b->EDX);
+   if (flag & ACPF_SI ) 		printf("si=%#x ", b->SI);
+   if (flag & ACPF_SI_HI ) 	printf("si_hi=%#x ", b->SI_HI);
+   if (flag & ACPF_ESI ) 	printf("esi=%#x ", b->ESI);
+   if (flag & ACPF_DI ) 		printf("di=%#x ", b->DI);
+   if (flag & ACPF_DI_HI ) 	printf("di_hi=%#x ", b->DI_HI);
+   if (flag & ACPF_EDI ) 	printf("edi=%#x ", b->EDI);
+   if (flag & ACPF_FLAGS ) 	printf("flags=%#x ", b->FLAGS);
+   if (flag & ACPF_FLAGS_HI) 	printf("flags_hi=%#x ", b->FLAGS_HI);
+   if (flag & ACPF_EFLAGS ) 	printf("eflags=%#x ", b->EFLAGS);
+ }
+ 
  int
  apmcall_debug(func, regs, line)
  	int func;
***************
*** 231,241 ****
  	int line;
  {
  	int rv;
! 
! 	DPRINTF(APMDEBUG_APMCALLS, ("apmcall: func %d from line %d",
! 	    func, line));
      	rv = apmcall(func, regs);
! 	DPRINTF(APMDEBUG_APMCALLS, (" -> 0x%x\n", rv));
  	return (rv);
  }
  
--- 317,355 ----
  	int line;
  {
  	int rv;
! 	int print = (apmdebug & APMDEBUG_APMCALLS) != 0;
! 	char *name;
! 	int inf, outf;
! 		
! 	if (print) {
! 		if (func >= sizeof(aci) / sizeof(aci[0])) {
! 			name = 0;
! 			inf = outf = 0;
! 		} else {
! 			name = aci[func].name;
! 			inf = aci[func].inflag;
! 			outf = aci[func].outflag;
! 		}
! 		inittodr(time.tv_sec);
! 		if (name)
! 			printf("apmcall@%03ld: %s/%#x (line=%d) ", 
! 				time.tv_sec % 1000, name, func, line);
! 		else
! 			printf("apmcall@%03ld: %#x (line=%d) ", 
! 				time.tv_sec % 1000, func, line);
! 		acallpr(inf, "in:", regs);
! 	}
      	rv = apmcall(func, regs);
! 	if (print) {
! 		if (rv) {
! 			printf(" => error %#x (%s)\n", regs->AX >> 8,
! 				apm_strerror(regs->AX >> 8));
! 		} else {
! 			printf(" => ");
! 			acallpr(outf, "out:", regs);
! 			printf("\n");
! 		}
! 	}
  	return (rv);
  }
  
***************
*** 461,468 ****
  
  	if ((sc->sc_flags & SCFLAG_OPEN) == 0)
  		return 1;		/* no user waiting */
! 	if (sc->event_count == APM_NEVENTS)
  		return 1;			/* overflow */
  	evp = &sc->event_list[sc->event_ptr];
  	sc->event_count++;
  	sc->event_ptr++;
--- 575,584 ----
  
  	if ((sc->sc_flags & SCFLAG_OPEN) == 0)
  		return 1;		/* no user waiting */
! 	if (sc->event_count == APM_NEVENTS) {
! 		printf("apm_record_event: queue full!\n");
  		return 1;			/* overflow */
+ 	}
  	evp = &sc->event_list[sc->event_ptr];
  	sc->event_count++;
  	sc->event_ptr++;
***************
*** 473,494 ****
  	return (sc->sc_flags & SCFLAG_OWRITE) ? 0 : 1; /* user may handle */
  }
  
! static void
  apm_event_handle(sc, regs)
  	struct apm_softc *sc;
  	struct bioscallregs *regs;
  {
! 	int error;
  	struct bioscallregs nregs;
  	char *code;
  
  	switch (regs->BX) {
  	case APM_USER_STANDBY_REQ:
  		DPRINTF(APMDEBUG_EVENTS, ("apmev: user standby request\n"));
  		if (apm_do_standby) {
! 			if (apm_record_event(sc, regs->BX))
! 				apm_userstandbys++;
! 			apm_op_inprog++;
  			(void)apm_set_powstate(APM_DEV_ALLDEVS,
  			    APM_LASTREQ_INPROG);
  		} else {
--- 589,620 ----
  	return (sc->sc_flags & SCFLAG_OWRITE) ? 0 : 1; /* user may handle */
  }
  
! /*
!  * apm_event_handle: handle an event.  returns 1 if event handled, 0 if
!  * event is a duplicate of an event we are already handling.
!  */
! static int
  apm_event_handle(sc, regs)
  	struct apm_softc *sc;
  	struct bioscallregs *regs;
  {
! 	int error, retval;
  	struct bioscallregs nregs;
  	char *code;
  
+ 	retval = 1;		/* assume we are going to make progress */
+ 
  	switch (regs->BX) {
  	case APM_USER_STANDBY_REQ:
  		DPRINTF(APMDEBUG_EVENTS, ("apmev: user standby request\n"));
  		if (apm_do_standby) {
! 			if (apm_standby_pending)
! 				retval = 0;		/* duplicate request */
! 			else {
! 				if (apm_record_event(sc, regs->BX))
! 					apm_standby_now++; /* kernel handles */
! 				apm_standby_pending++;
! 			}
  			(void)apm_set_powstate(APM_DEV_ALLDEVS,
  			    APM_LASTREQ_INPROG);
  		} else {
***************
*** 499,516 ****
  		}
  		break;
  
! 	case APM_STANDBY_REQ:
  		DPRINTF(APMDEBUG_EVENTS, ("apmev: system standby request\n"));
- 		if (apm_standbys || apm_suspends) {
- 			DPRINTF(APMDEBUG_EVENTS | APMDEBUG_ANOM,
- 			    ("damn fool BIOS did not wait for answer\n"));
- 			/* just give up the fight */
- 			apm_damn_fool_bios = 1;
- 		}
  		if (apm_do_standby) {
! 			if (apm_record_event(sc, regs->BX))
! 				apm_standbys++;
! 			apm_op_inprog++;
  			(void)apm_set_powstate(APM_DEV_ALLDEVS,
  			    APM_LASTREQ_INPROG);
  		} else {
--- 625,640 ----
  		}
  		break;
  
! 	case APM_STANDBY_REQ:	/* XXXCDC: combine with above */
  		DPRINTF(APMDEBUG_EVENTS, ("apmev: system standby request\n"));
  		if (apm_do_standby) {
! 			if (apm_standby_pending)
! 				retval = 0;		/* duplicate request */
! 			else {
! 				if (apm_record_event(sc, regs->BX))
! 					apm_standby_now++;  /* kernel handles */
! 				apm_standby_pending++;
! 			}
  			(void)apm_set_powstate(APM_DEV_ALLDEVS,
  			    APM_LASTREQ_INPROG);
  		} else {
***************
*** 523,545 ****
  
  	case APM_USER_SUSPEND_REQ:
  		DPRINTF(APMDEBUG_EVENTS, ("apmev: user suspend request\n"));
! 		if (apm_record_event(sc, regs->BX))
! 			apm_suspends++;
! 		apm_op_inprog++;
  		(void)apm_set_powstate(APM_DEV_ALLDEVS, APM_LASTREQ_INPROG);
  		break;
  
! 	case APM_SUSPEND_REQ:
  		DPRINTF(APMDEBUG_EVENTS, ("apmev: system suspend request\n"));
! 		if (apm_standbys || apm_suspends) {
! 			DPRINTF(APMDEBUG_EVENTS | APMDEBUG_ANOM,
! 			    ("damn fool BIOS did not wait for answer\n"));
! 			/* just give up the fight */
! 			apm_damn_fool_bios = 1;
! 		}
! 		if (apm_record_event(sc, regs->BX))
! 			apm_suspends++;
! 		apm_op_inprog++;
  		(void)apm_set_powstate(APM_DEV_ALLDEVS, APM_LASTREQ_INPROG);
  		break;
  
--- 647,671 ----
  
  	case APM_USER_SUSPEND_REQ:
  		DPRINTF(APMDEBUG_EVENTS, ("apmev: user suspend request\n"));
! 		if (apm_suspend_pending)
! 			retval = 0;		/* duplicate request */
! 		else {
! 			if (apm_record_event(sc, regs->BX))
! 				apm_suspend_now++;	/* kernel handles */
! 			apm_suspend_pending++;
! 		}
  		(void)apm_set_powstate(APM_DEV_ALLDEVS, APM_LASTREQ_INPROG);
  		break;
  
! 	case APM_SUSPEND_REQ:	/* XXXCDC: combine with above? */
  		DPRINTF(APMDEBUG_EVENTS, ("apmev: system suspend request\n"));
! 		if (apm_suspend_pending)
! 			retval = 0;		/* duplicate request */
! 		else {
! 			if (apm_record_event(sc, regs->BX))
! 				apm_suspend_now++;	/* kernel handles */
! 			apm_suspend_pending++;
! 		}
  		(void)apm_set_powstate(APM_DEV_ALLDEVS, APM_LASTREQ_INPROG);
  		break;
  
***************
*** 583,589 ****
  
  	case APM_BATTERY_LOW:
  		DPRINTF(APMDEBUG_EVENTS, ("apmev: battery low\n"));
- 		apm_battlow++;
  		apm_record_event(sc, regs->BX);
  		break;
  
--- 709,714 ----
***************
*** 614,619 ****
--- 739,745 ----
  		}	
  		printf("APM: %s event code %x\n", code, regs->BX);
  	}
+ 	return(retval);
  }
  
  static int
***************
*** 631,656 ****
  	struct bioscallregs regs;
  
  	/*
! 	 * tell the BIOS we're working on it, if asked to do a
! 	 * suspend/standby
  	 */
! 	if (apm_op_inprog)
  		apm_set_powstate(APM_DEV_ALLDEVS, APM_LASTREQ_INPROG);
  
! 	while (apm_get_event(&regs) == 0 && !apm_damn_fool_bios)
! 		apm_event_handle(sc, &regs);
  
  	if (APM_ERR_CODE(&regs) != APM_ERR_NOEVENTS)
  		apm_perror("get event", &regs);
! 	if (apm_suspends) {
! 		apm_op_inprog = 0;
  		apm_suspend(sc);
! 	} else if (apm_standbys || apm_userstandbys) {
! 		apm_op_inprog = 0;
  		apm_standby(sc);
  	}
! 	apm_suspends = apm_standbys = apm_battlow = apm_userstandbys = 0;
! 	apm_damn_fool_bios = 0;
  }
  
  static void
--- 757,794 ----
  	struct bioscallregs regs;
  
  	/*
! 	 * if we are waiting for user (apmd) to process a suspend or 
! 	 * standby tell the BIOS we are working on it.
  	 */
! 	if (apm_standby_pending || apm_suspend_pending)
  		apm_set_powstate(APM_DEV_ALLDEVS, APM_LASTREQ_INPROG);
  
! 	/*
! 	 * continue processing events until we run out or we get a 
! 	 * duplicate.   duplicates occur on some APM BIOS (e.g. IBM thinkpad)
! 	 * where it keeps posting the standby/suspend event until 
! 	 * forward progress is made.
! 	 */
! 	while (apm_get_event(&regs) == 0) {
! 		if (!apm_event_handle(sc, & regs)) {
! 			DPRINTF(APMDEBUG_EVENTS | APMDEBUG_ANOM,
! 			  ("apm_periodic_check: duplicate event (break)\n"));
! 			break;
! 		}
! 	}
  
  	if (APM_ERR_CODE(&regs) != APM_ERR_NOEVENTS)
  		apm_perror("get event", &regs);
! 	if (apm_suspend_now) {
! 		apm_suspend_pending = 0;
  		apm_suspend(sc);
! 	} else if (apm_standby_now) {
! 		apm_standby_pending = 0;
  		apm_standby(sc);
  	}
! 
! 	/* reset for next loop */
! 	apm_suspend_now = apm_standby_now = 0;
  }
  
  static void
***************
*** 1490,1496 ****
  			error = EBADF;
  			break;
  		}
! 		apm_userstandbys++;
  		break;
  
  	case APM_IOC_SUSPEND:
--- 1628,1634 ----
  			error = EBADF;
  			break;
  		}
! 		apm_standby_now++;	/* flag for periodic event */ 
  		break;
  
  	case APM_IOC_SUSPEND:
***************
*** 1498,1504 ****
  			error = EBADF;
  			break;
  		}
! 		apm_suspends++;
  		break;
  
  #if 0 /* is this used at all? */
--- 1636,1642 ----
  			error = EBADF;
  			break;
  		}
! 		apm_suspend_now++;	/* flag for peroidic event */
  		break;
  
  #if 0 /* is this used at all? */