Re: [Regression] ACPI: video: Change how we determine if brightness key-presses are handled

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 7/13/22 15:08, Ben Greening wrote:
> Hi Hans, thanks for getting back to me.
> 
> evemu-record shows events for both "Video Bus" and "Dell WMI hotkeys":
> 
> Video Bus
> E: 0.000001 0001 00e0 0001 # EV_KEY / KEY_BRIGHTNESSDOWN   1
> E: 0.000001 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +0ms
> E: 0.000020 0001 00e0 0000 # EV_KEY / KEY_BRIGHTNESSDOWN   0
> E: 0.000020 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +0ms
> 
> Dell WMI hotkeys
> E: 0.000001 0004 0004 57349 # EV_MSC / MSC_SCAN             57349
> E: 0.000001 0001 00e0 0001 # EV_KEY / KEY_BRIGHTNESSDOWN   1
> E: 0.000001 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +0ms
> E: 0.000020 0001 00e0 0000 # EV_KEY / KEY_BRIGHTNESSDOWN   0
> E: 0.000020 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- +0ms
> 
> Adding video.report_key_events=1 with acpi_backlight=video makes
> things work like you said it would.
> 
> 
> With acpi_backlight=video just has intel_backlight.
> 
> Without acpi_backlight=video:
>     intel_backlight:
>         max_brightness: 4882
>         backlight control works with echo
>         brightness keys make no change to brightness value
> 
>     dell_backlight:
>         max_brightness: 15
>         backlight control doesn't work immediately, but does on reboot
> to set brightness at POST.
>         brightness keys change brightness value, but you don't see the
> change until reboot.

Ok, so your system lacks ACPI video backlight control, yet still reports
brightness keypresses through the ACPI Video Bus. Interesting (weird)...

I think I believe I know how to fix the regression, 1 patch coming up.

For the need to pass acpi_backlight=video, what you are in essence
doing is setting acpi_backlight=native.

The auto-detect code goes to acpi_backlight=vendor because of the lacking
ACPI video backlight control and manually setting acpi_backlight != vendor
disables the dell_backlight. ATM the native (intel) backlight ingnoes
the acpi_backlight setting so it loads unconditionally. But in the near
future this will change and then you need to pass acpi_backlight=native
otherwise the intel backlight will not register because you requested
video.

So I plan to fix this part by adding a quirk to make native the default
on your machine. Can you do:

sudo dmidecode > dmidecode.txt

And email me the generated dmidecode.txt (this will contain serialnumbers
so you may want to send it off-list) ? Then I can also prepare a patch
to add a quirk to make native the default on your model.

Regards,

Hans










> 
> Thanks again,
> 
> Ben
> 
> On Wed, Jul 13, 2022 at 2:43 AM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Hi Ben,
>>
>> On 7/13/22 07:27, Ben Greening wrote:
>>> (resending because of HTML formatting)
>>> Hi, I'm on Arch Linux and upgraded from kernel 5.18.9.arch1-1 to
>>> 5.18.10.arch1-1. The brightness keys don't work as well as before.
>>> Gnome had 20 degrees of brightness, now it's 10, and Xfce went from 10
>>> to 5. Additionally, on Gnome the brightness keys are a little slow to
>>> respond and there's a bit of a stutter. Don't know why Xfce doesn't
>>> stutter, but halving the degrees of brightness for both makes me
>>> wonder if each press is being counted twice.
>>
>> Author of the troublesome patch here, sorry that this broke things
>> for you.
>>
>> So this sounds like you are getting duplicate key-events reported,
>> causing the brightness to take 2 steps on each key-press which is
>> likely also causing the perceived stutter.
>>
>> This suggests that acpi_video_handles_brightness_key_presses()
>> was returning true on your system and is now returning false.
>>
>> Lets confirm this theory, please run either evtest or evemu-record
>> as root and then record events from the "Video Bus" device and then
>> press the brightness up/down keys. Press CTRL+C to exit. After this
>> repeat selecting the "Dell WMI hotkeys" device as input device.
>>
>> I expect both tests/recordings to show brightness key events with
>> the troublesome kernel, showing that you are getting duplicate events.
>>
>> If this is the case then as a workaround you can add:
>>
>> video.report_key_events=1
>>
>> to the kernel commandline. This should silence the "Video Bus"
>> events. Also can you provide the output of:
>>
>> ls /sys/class/backlight
>>
>> please?
>>
>>
>>> Reverting commit 3a0cf7ab8d in acpi_video.c and rebuilding
>>> 5.18.10.arch1-1 fixed it.
>>
>>> The laptop is a Dell Inspiron n4010 and I use "acpi_backlight=video"
>>> to make the brightness keys work. Please let me know if there's any
>>> hardware info you need.
>>
>> Note needing to add a commandline argument like this to get things
>> to work is something which really should always be reported upstream,
>> so that we can either adjust our heuristics; or add a quirk for your
>> laptop-model so that things will just work when another user tries
>> Linux on the same model.
>>
>> So while at it lets look into fixing this properly to.
>>
>> When you do not pass anything on the kernel commandline, what
>> is then the output of:
>>
>> ls /sys/class/backlight
>>
>> And for each directory under there, please cd into the dir
>> and then (as root) do:
>>
>> cat max_brightness # this gives you the range of this backlight intf.
>> echo $some-value > brightness
>>
>> picking some-value in a range of 0-max_brightness, repeating the
>> echo with different values (e.g. half-range + max) and see if
>> the screens brightness changes. Please let me know which directories
>> under /sys/class/backlight result in working backlight control
>> and which ones do not.
>>
>> Also what is the output of "ls /sys/class/backlight" when
>> "acpi_backlight=video" is present on the kernel commandline ?
>>
>> Regards,
>>
>> Hans
>>
>