fix APM notify of apmd for on-AC/on-battery transitions

Hi there Cort,

It looks like my Vaio will need it as well -- loading the apm module with debug=1 doesn't show any on/off battery events when I yank the power. (It does log other events, though.)

A comment on the patch? How about pushing the specifics of the apm_bios_power_change_bug from check_events() down into either the get_event() or apm_get_event() routines? That way the specifics are abstracted a bit from the main event dispatch loop, and one gets to inherit the debug logging and whatnot.

And, in case you decide to tighten up the dmi matching, my laptop info follows:

BIOS Information Block
Vendor: Phoenix Technologies LTD
Version: R0117A0
Release: 04/25/00
System Information Block
Vendor: Sony Corporation
Product: PCG-XG29(UC)

Thanks,

Ray

Thanks for the report, I'll add that to the comments for completeness.

I think this affects all Vaio laptops. With enough reports maybe we'll find out something different, though.

Thanks for the suggestion to move the get_event(). I'll see about moving that (but not on a friday night).

} It looks like my Vaio will need it as well -- loading the apm module } with debug=1 doesn't show any on/off battery events when I yank the } power. (It does log other events, though.)
}
} A comment on the patch? How about pushing the specifics of the } apm_bios_power_change_bug from check_events() down into either the } get_event() or apm_get_event() routines? That way the specifics are } abstracted a bit from the main event dispatch loop, and one gets to } inherit the debug logging and whatnot.
}
} And, in case you decide to tighten up the dmi matching, my laptop info } follows:
}
} BIOS Information Block
} Vendor: Phoenix Technologies LTD
} Version: R0117A0
} Release: 04/25/00
} System Information Block
} Vendor: Sony Corporation
} Product: PCG-XG29(UC)

Agreed to both.

Heh, you mean you have a life? I'm jealous :-)

Below is my suggestion for the patch. Mostly it's just a rework of the structure, but one thing that cropped up is that 2.4.18 got rid of the send_event() call. Also, as you'll note from the patch I'm suggesting we back off from identifying systems one by one, and instead just always do the right thing: if the BIOS decides to step up and take responsibility for power change events, then disable the workaround code. Otherwise, let it be and send a notification as if the BIOS had done it correctly to begin with.

Comments?

diff -NurX dontdiff linux/arch/i386/kernel/apm.c apm-fixes/arch/i386/kernel/apm.c
@@ -385,6 +385,7 @@
 static int			ignore_sys_suspend;
 static int			ignore_normal_resume;
 static int			bounce_interval = DEFAULT_BOUNCE_INTERVAL;
+static u_short			last_power_status;
 
 #ifdef CONFIG_APM_RTC_IS_GMT
 #	define	clock_cmos_diff	0

@@ -1240,15 +1241,40 @@
 	apm_eventinfo_t	info;
 
 	static int notified;
+	static u_short	power_event_state = 0;
 
 	/* we don't use the eventinfo */
 	error = apm_get_event(&event, &info);
-	if (error == APM_SUCCESS)
+	if (error == APM_SUCCESS) {
+		/* if BIOS reports power changes, disable workaround */
+		if (event == APM_POWER_STATUS_CHANGE)
+			power_event_state = 2;
 		return event;
+	}
 
 	if ((error != APM_NO_EVENTS) && (notified++ == 0))
 		apm_error("get_event", error);
 
+	/*
+	 * Sony Vaios don't seem to want to notify us about AC line power
+	 * status changes.  So for those and any others like them, we keep
+	 * track of it by hand and emulate it here.
+	 */
+	if (power_event_state < 2) {
+		u_short status, bat, life;
+		error = apm_get_power_status(&status, &bat, &life);
+		if (error == APM_SUCCESS && (status ^ last_power_status) & 0xff00) {
+			/* fake an APM_POWER_STATUS_CHANGE event */
+			last_power_status = status;
+			if (power_event_state == 0) {
+				printk(KERN_WARNING "apm: power status notification workaround enabled\n");
+				power_event_state = 1;
+			}
+			return APM_POWER_STATUS_CHANGE;
+		} else if (error != APM_SUCCESS)
+			power_event_state=2;
+	}
+
 	return 0;
 }
 

@@ -1758,6 +1784,7 @@
 #if defined(CONFIG_APM_DISPLAY_BLANK) && defined(CONFIG_VT)
 		console_blank_hook = apm_console_blank;
 #endif
+		apm_get_power_status(&last_power_status, &cx, &dx);
 		apm_mainloop();
 #if defined(CONFIG_APM_DISPLAY_BLANK) && defined(CONFIG_VT)
 		console_blank_hook = NULL;

Hi all,

Below is a rework of Cort Dougan's APM patch of last Thursday and Friday. As a reminder, the issue is that some laptops, notably Vaios, don't send notification when the power status changes to AC or to on-battery. The below patch fixes this such that it always just works.

There are three cases. First, the BIOS doesn't send any notifications; this is fixed. Second, the BIOS sends notifications. In this case, the code notes that, and disables the workaround. Third, the BIOS sends notifications, but somehow we managed to notice the power change before the BIOS could tell us. This seems highly unlikely, but what the heck, it could theoretically happen. In that case, we disable the workaround, and drop the notification that the BIOS generated, as we already sent it onward up the call chain.

The rework was for a couple of small reasons. First, Cort's patch was against 2.4.16, and 2.4.18 changed a few things in apm.c. Second, this pushes the specifics of the workaround down to the get_event() routine, which exists pretty much solely as an abstraction point between the actual BIOS call (apm_get_event()) and the event dispatch function (check_events()). Lastly, I see no need to do dmi matching. The code can be structured to always do the right thing, so that's what we do.

I'm on the road from Thursday to Monday, and won't be reading email, but comments are welcome. The patch is pretty darn straight-forward, though.

Please consider for the next -ac patch, and 2.4.20-preX. Actual patch generated against 2.4.19-rc3-ac5.

Ray Lee

diff -NurX /usr/src/dontdiff linux-2.4.19-rc3-ac5/arch/i386/kernel/apm.c linux-2.4.19-rc3-ac5-apm-fixes/arch/i386/kernel/apm.c
@@ -385,6 +385,7 @@
 static int			ignore_sys_suspend;
 static int			ignore_normal_resume;
 static int			bounce_interval = DEFAULT_BOUNCE_INTERVAL;
+static u_short			last_power_status;
 
 #ifdef CONFIG_APM_RTC_IS_GMT
 #	define	clock_cmos_diff	0

@@ -1239,17 +1240,46 @@
 	int		error;
 	apm_event_t	event;
 	apm_eventinfo_t	info;
-
+	static u_short	power_event_workaround_enabled = 1;
 	static int notified;
 
 	/* we don't use the eventinfo */
 	error = apm_get_event(&event, &info);
-	if (error == APM_SUCCESS)
+	if (error == APM_SUCCESS) {
+		/* if BIOS reports power changes, disable workaround */
+		if (event == APM_POWER_STATUS_CHANGE) {
+			/* check to see if we jumped the gun and reported a
+			 * power change event that the BIOS would have (and
+			 * just did) on its own. If so, drop the duplicate.
+			 */
+			if (power_event_workaround_enabled)
+				event=get_event();
+			power_event_workaround_enabled = 0;
+		}
 		return event;
+	}
 
 	if ((error != APM_NO_EVENTS) && (notified++ == 0))
 		apm_error("get_event", error);
 
+	/*
+	 * Sony Vaios don't seem to want to notify us about AC line power
+	 * status changes.  So for those and any others like them, we keep
+	 * track of it by hand and emulate it here.
+	 */
+	if (power_event_workaround_enabled) {
+		u_short status, bat, life;
+		error = apm_get_power_status(&status, &bat, &life);
+		if (error == APM_SUCCESS) {
+			if ((status ^ last_power_status) & 0xff00) {
+				/* fake an APM_POWER_STATUS_CHANGE event */
+				last_power_status = status;
+				return APM_POWER_STATUS_CHANGE;
+			}
+		} else 
+			power_event_workaround_enabled=0;
+	}
+
 	return 0;
 }
 

@@ -1758,6 +1788,7 @@
 #if defined(CONFIG_APM_DISPLAY_BLANK) && defined(CONFIG_VT)
 		console_blank_hook = apm_console_blank;
 #endif
+		apm_get_power_status(&last_power_status, &cx, &dx);
 		apm_mainloop();
 #if defined(CONFIG_APM_DISPLAY_BLANK) && defined(CONFIG_VT)
 		console_blank_hook = NULL;

Actually there is one more case, where the BIOS unreliably tells you something has changed. I have an old Toshiba which I bought with Windows installed, and it always noticed pulling the plug and going line=>battery, but only sometimes noticed battery=>line. Of course this might be an o/s bug. Can't test that any more, the battery failed and the transition is now line=>dead.

Anyway, if you are paranoid you could just ignore the "I knew that" cases and leave the workaround in place, unless that would generate other issues.

bill davidsen <[email protected]>
  CTO, TMR Associates, Inc
Doing interesting things with little computers since 1979.

[Trimmed the cc:]

Well, that's just special. I wonder where the blame lies in that case.

Heh.

Hmm. I don't think that would cover everything. Taking your example case, and assuming it's the BIOS being flaky, we'd have to just ignore all transitions from the BIOS apm and just poll ourselves. Otherwise, we'd have line->battery (signaled), battery->line (not signaled), line->battery (signaled) and *then* we'd know to be paranoid. In the meantime, we lost the second transition, which could have been hours ago. The solution in that case would be to infrequently poll (say, twice a minute) to verify what the BIOS told us. If it's out of sync, give it a bit of a grace period, double-check, then take over the reins for reporting.

The bottom line is that I didn't want to incur an extra set of BIOS calls on systems that don't need it, on general principle. <Shrug> Heck, maybe it's fast and the overhead is unnoticeable, but I don't know (ISTR some low-latency issues when doing BIOS calls). Considering the APM thread is only getting invoked once a second, it's seems that it would be unnoticeable and zero risk, but hey, what do I know.

Anyway, a patch to do double-checking would be fairly straight-forward, but without any reports of hardware out there that fails like that... dunno. I'll work up a patch when I'm back from my road trip and see if it's as clean.

Ray

Okay, I said "other issues" and that certainly is one.

Bear in mind that I was being pedantic to mention the other case, I would think that if this is worth doing at all (is it?) just an option to ignore the BIOS might be fine:

modprobe apm my_bios_sucks=sad_but_true

or some such. If anyone was convinced there was such an issue they could do it. Again, it could have been the o/s just losing the int when running slow on battery. M$ losing ints? Nah, can't happen ;-)

bill davidsen <[email protected]>
  CTO, TMR Associates, Inc
Doing interesting things with little computers since 1979.