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

[Alan Jenkins]

On 6/15/09, Alan Jenkins <alan-jenkins@xxxxxxxxxxxxxx> wrote:
> On 14/06/2009, Corentin Chary <corentin.chary@xxxxxxxxx> wrote:
>> CCed gnome-power-manager, as it seems to be the only userspace program
>> concerned.

Warning pour les autres: subscription-only mailing list.

Posts on the g-p-m list will be subject to moderation and delays.

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