mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Corentin Chary <corentin.chary@gmail.com>
To: Alan Jenkins <alan-jenkins@tuffmail.co.uk>
Cc: Darren Salt <linux@youmustbejoking.demon.co.uk>,
	linux-kernel@vger.kernel.org,
	acpi4asus-user@lists.sourceforge.net,
	Matthew Garrett <mjg59@srcf.ucam.org>
Subject: Re: [PATCH 2.6.29] eeepc-laptop: report brightness control events via  the input layer
Date: Sat, 13 Jun 2009 12:06:18 +0200	[thread overview]
Message-ID: <71cd59b00906130306w319c0fc2i376bb03323845b80@mail.gmail.com> (raw)
In-Reply-To: <4A337251.7010705@tuffmail.co.uk>

On Sat, Jun 13, 2009 at 11:33 AM, Alan
Jenkins<alan-jenkins@tuffmail.co.uk> wrote:
> Corentin Chary wrote:
>>
>> On Mon, Jun 8, 2009 at 5:24 PM, Alan
>> Jenkins<sourcejedi.lkml@googlemail.com> wrote:
>>
>>>
>>> On 4/4/09, Matthew Garrett <mjg59@srcf.ucam.org> wrote:
>>>
>>>>
>>>> On Fri, Apr 03, 2009 at 06:57:50PM +0100, Darren Salt wrote:
>>>>
>>>>>
>>>>> This maps the brightness control events to one of two keys, either
>>>>> KEY_BRIGHTNESSDOWN or KEY_BRIGHTNESSUP, as needed.
>>>>>
>>>>> Some mapping has to be done due to the fact that the BIOS reports them
>>>>> as
>>>>> <base value> + <current brightness index>; the selection is done
>>>>> according
>>>>> to
>>>>> the sign of the change in brightness (if this is 0, no keypress is
>>>>> reported).
>>>>>
>>>>> (Ref.
>>>>>
>>>>> http://lists.alioth.debian.org/pipermail/debian-eeepc-devel/2009-April/002001.html)
>>>>>
>>>>> Signed-off-by: Darren Salt <linux@youmustbejoking.demon.co.uk>
>>>>>
>>>>
>>>> The reason I didn't do this is that the Eee changes the input brightness
>>>> in hardware, which means reporting it via the input layer as well can
>>>> cause a single keypress to raise the brightness by two steps - one in
>>>> hardware and one triggered by userland's response to the key press. I'd
>>>> be a little bit wary of this causing problems.
>>>>
>>>> On the other hand, the default behaviour of the acpi video driver is to
>>>> change the brightness itself and then also to send the even to
>>>> userspace, so I guess if it was going to break things it probably would
>>>> have done already...
>>>>
>>>
>>> Actually, I think userspace has learnt to hack around it but it
>>> doesn't work perfectly.  I would like to request that this change be
>>> reverted, or otherwise improved.
>>>
>>> Before this patch (2.6.29.4), gnome-power-manager doesn't interfere
>>> with the brightness keys, and they work smoothly.
>>>
>>> After this patch (2.6.30-rc7), g-p-m produces a "nice" popup in the
>>> middle of my tiny netbook screen.  Unfortunately it can't be disabled,
>>> but that's not your fault :-).  The brightness controls generally work
>>> ok.  It doesn't jump two steps in response to one brightness keypress.
>>>  But:
>>>
>>> 1) If I'm thrashing the SSD.  I get jerky after-effects, where g-p-m
>>> seems to take too long to "catch up" with the brightness change.
>>>
>>
>> There is the same "lag" problem with sound :/
>>
>
> Yeah :/.  At it's worst, it isn't a pure "lag".  It's a bit harder to
> explain.  The firmware still changes the brightness immediately.  It seems
> that when g-p-m gets delayed, it responds _wrongly_. It doesn't realize that
> the firmware already changed the brightness, so it changes the brightness
> again.
>
> That's why I think this is a bad "feature".  User-space is working around
> it, but the workaround isn't reliable.  I think the proper solution is that
> if userspace wants to respond to firmware-initiated brightness changes, it
> should listen for uevents on the brightness class device.
>
> You can see the problem most clearly by pressing the brightness keys in
> quick succession e.g. 3 times in a row, and see 3+3 brightness changes.  I
> reproduced this with
>
> 1) 2 writers:
>   F=1; while true do dd if=/dev/zero of=$F bs=1M count=1 conv=sync; done &
>   F=2; while true do dd if=/dev/zero of=$F bs=1M count=1 conv=sync; done &
>
> 2) 1 reader:
>   dd if=/dev/sda of=/dev/null
>
> 3) Drop caches before pressing the brightness keys
>   echo 1 > /sys/proc/vm/drop_caches
>
>
>>> 2) If I go all the way down from full (holding down the "brightness
>>> down" key), and then back up a few steps.  I get a noticable flash
>>> where the brightness looks to go up two steps, then down one.  It's
>>> probably most noticable here because the step change between the
>>> lowest and the second lowest brightness is much more visible than any
>>> of the other steps.
>>>
>>>
>>
>> I tried to install gnome-power-manager to test that, but there is no
>> "popup".
>> What do I have to install to test that ? entire gnome desktop :/ ?
>>
>> Thanks
>>
>
> That's weird.  I'm running it from KDE here (g-p-m package version 2.22.1-4
> on debian stable).  I'm pretty sure the only other GNOME app I have
> installed is Pidgin.  I would know if I had much more installed, because I'm
> often running short of disk space :-).
>
> Maybe you have a newer version, that doesn't try to do such unreliable
> things?
>
> Thanks for your time
> Alan
>
Hum .. running g-p-m as root, I can get the popup, strange
Version: 2.24.2-2ubuntu8
Ok I can reproduce that.

I want to check if we can't fix g-p-m before reverting the patch. If
there is no way to fix it, I'll revert.

-- 
Corentin Chary
http://xf.iksaif.net - http://uffs.org

  reply	other threads:[~2009-06-13 10:06 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-04-03 17:57 Darren Salt
2009-04-04  4:18 ` Matthew Garrett
2009-04-04  8:33   ` Corentin Chary
2009-04-04 12:20     ` Darren Salt
2009-04-04 12:35       ` Corentin Chary
2009-04-04 22:10         ` Darren Salt
2009-04-05  8:22           ` Corentin Chary
2009-06-08 15:24   ` Alan Jenkins
2009-06-13  8:55     ` Corentin Chary
2009-06-13  9:33       ` Alan Jenkins
2009-06-13 10:06         ` Corentin Chary [this message]
2009-06-13 12:55           ` Darren Salt
2009-06-13 17:51             ` Alan Jenkins
2009-06-14 19:26               ` Corentin Chary
2009-06-15  8:09                 ` Alan Jenkins
2009-06-15  8:12                   ` Alan Jenkins
2009-06-16  8:33                   ` [gpm] " Richard Hughes
2009-06-16  8:34                     ` Alan Jenkins
2009-06-16  8:47                       ` Richard Hughes
2009-06-16  9:44                         ` Corentin Chary
2009-06-16 10:04                           ` Richard Hughes
2009-06-18 13:33                             ` Alan Jenkins
2009-06-18 22:44                               ` Corentin Chary

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=71cd59b00906130306w319c0fc2i376bb03323845b80@mail.gmail.com \
    --to=corentin.chary@gmail.com \
    --cc=acpi4asus-user@lists.sourceforge.net \
    --cc=alan-jenkins@tuffmail.co.uk \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@youmustbejoking.demon.co.uk \
    --cc=mjg59@srcf.ucam.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome