Re: [PATCH] fix APM notify of apmd for on-AC/on-battery transitions

Re: [PATCH] fix APM notify of apmd for on-AC/on-battery transitions

[Ray Lee]

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

Re: [PATCH] fix APM notify of apmd for on-AC/on-battery transitions

[cort]

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)        

Re: [PATCH] fix APM notify of apmd for on-AC/on-battery transitions

[Ray Lee]

On Fri, 2002-07-26 at 19:17, cort@fsmlabs.com wrote:
> I think this affects all Vaio laptops.  With enough reports maybe
> we'll find out something different, though.

Agreed to both.

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

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
--- linux/arch/i386/kernel/apm.c	2002-06-08 08:31:24.000000000 -0700
+++ apm-fixes/arch/i386/kernel/apm.c	2002-07-28 17:48:42.000000000 -0700
@@ -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;

[PATCH] Guarantee APM power status change notifications

[Ray Lee]

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
--- linux-2.4.19-rc3-ac5/arch/i386/kernel/apm.c	2002-07-31 11:27:39.000000000 -0700
+++ linux-2.4.19-rc3-ac5-apm-fixes/arch/i386/kernel/apm.c	2002-07-31 11:30:19.000000000 -0700
@@ -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;

Re: [PATCH] Guarantee APM power status change notifications

[Bill Davidsen]

On 31 Jul 2002, Ray Lee wrote:

> 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.

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 <davidsen@tmr.com>
  CTO, TMR Associates, Inc
Doing interesting things with little computers since 1979.

Re: [PATCH] Guarantee APM power status change notifications

[Ray Lee]

[Trimmed the cc:]
On Wed, 2002-07-31 at 13:10, Bill Davidsen wrote:
> 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.

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

> Can't test that any more, the battery failed and the transition is
> now line=>dead.

Heh.

> 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.

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

Re: [PATCH] Guarantee APM power status change notifications

[Bill Davidsen]

On 1 Aug 2002, Ray Lee wrote:

> [Trimmed the cc:]
> On Wed, 2002-07-31 at 13:10, Bill Davidsen wrote:
> > 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.
> 
> Well, that's just special. I wonder where the blame lies in that case.
> 
> > Can't test that any more, the battery failed and the transition is
> > now line=>dead.
> 
> Heh.
> 
> > 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.
> 
> 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.

Okay, I said "other issues" and that certainly is one.
 
> 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.

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 <davidsen@tmr.com>
  CTO, TMR Associates, Inc
Doing interesting things with little computers since 1979.