Re: [PATCH 2.6.29] eeepc-laptop: report brightness control events via the input layer

[Alan Jenkins]

On 14/06/2009, Corentin Chary <corentin.chary@xxxxxxxxx> wrote:
> CCed gnome-power-manager, as it seems to be the only userspace program
> concerned.
> You may be able to help us here.
>
> You can find the complet discussion here:
> http://groups.google.com/group/linux.kernel/browse_thread/thread/a7bef6cffb7c2d6b/c732f616555d5180?#c732f616555d5180

Since this is my complaint, I'll try to summarize.


Summary: g-p-m's reaction to brightness events makes the brightness
changes less reliable
Severity: cosmetic; decrease in quality of user experience

Hardware: Asus Eee PC.
Software: linux kernel, gnome-power-manager

Last working version: linux 2.6.29.4
First broken version: linux 2.6.30

Root cause:
1) The eeepc-laptop platform driver added support for brightness
events.  They occur when the brightness up/down keys are pressed, and
are exported as normal keypresses (on a specific input device).  This
is apparently the same thing other kernel drivers do on other systems.

2) g-p-m isn't sure whether the firmware is changing the brightness
when the brightness keys are pressed.  (The EeePc firmware does change
the brightness, like most laptops).  It has a workaround for this
problem, but it isn't completely reliable.  In some cases g-p-m gets
it wrong, and changes the brightness a second time.

You can see the problem by looking at the code, and considering what
happens when more than one input event is buffered at a time:

	/* check to see if the panel has changed */
	gpm_brightness_lcd_get_hw (brightness, &current_hw);

	/* the panel has been updated in firmware */
	if (current_hw != brightness->priv->last_set_hw) {
		brightness->priv->last_set_hw = current_hw;
	} else {
		[ increment the brightness ]
	}

The first event will be ignored as expected.  But on the second event,
g-p-m will re-apply the brightness change.  It doesn't notice that the
brightness jumped _several_ steps before it processed the first event.

Symptoms:
1) When thrashing the EeePC's cheap sold-state drive, the system
becomes much slower to respond.  If you tap the brightness up key
three times, you can see the brightness jump more than 3 steps.  The
first 3 steps are immediate, then g-p-m appears to "catch up", and
erroneously re-apply the brightness changes.

This is a neat way to demonstrate the problem (see upthread for exact
reproduction steps), but we don't really care too much about it.
Laggy systems are laggy and strange.  I don't find this surprising and
I think most users will accept it, even if we could do better.

2) Switching quickly between holding "brightness down" and "brightness
up" can cause a flicker/flash of brightness.  This flash goes away
when g-p-m is killed (or on older kernels).

More specifically:
i) Set the screen to maximum brightness
ii) Hold down "brightness down"
iii) When brightness is at minimum, immediately release it and hold
down "brightness up" (or quickly tap it multiple times).

What probably happens is that g-p-m "falls behind" during step ii).
During step ii), all this does is cause the brightness to change a
little bit faster.  However, it means that in step iii), it's still
trying to decrease the brightness when the firmware starts to increase
the brightness.  So during step iii) I think you see the firmware
increase the brightness, then g-p-m decrease the brightness, and then
g-p-m catches up and the brightness increases again.

This crosses my annoyance threshold.  That's partly because it's a
regression, and because I've spent a lot of time tracking down
brightness-change related regressions which ultimately crashed the
entire system.  But I do think it's an annoyance in it's own right.
It's definitely a realistic scenario.  One will often move these small
laptops around and adjust the brightness to suit different light
conditions.

Thanks
Alan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/