Bug 79699 - XRandr backlight notifications are lost
Summary: XRandr backlight notifications are lost
Status: RESOLVED FIXED
Alias: None
Product: xorg
Classification: Unclassified
Component: Driver/intel (show other bugs)
Version: git
Hardware: Other Linux (All)
: low normal
Assignee: Chris Wilson
QA Contact: Intel GFX Bugs mailing list
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-06-05 18:53 UTC by Alexander Mezin
Modified: 2014-06-06 07:15 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
Xorg.0.log (23.44 KB, text/plain)
2014-06-05 18:58 UTC, Alexander Mezin
no flags Details
Hook up a backlight udev monitor (4.02 KB, patch)
2014-06-05 21:48 UTC, Chris Wilson
no flags Details | Splinter Review
Hook up a backlight udev monitor (4.27 KB, patch)
2014-06-05 21:56 UTC, Chris Wilson
no flags Details | Splinter Review
Hook up a backlight monitor (4.64 KB, patch)
2014-06-06 06:00 UTC, Chris Wilson
no flags Details | Splinter Review

Description Alexander Mezin 2014-06-05 18:53:03 UTC
When I press "brightness up" or "brightness down" keys, backlight brightness changes. But KDE power manager thinks backlight didn't change.

udevadm --monitor:
KERNEL[280.185564] change   /devices/pci0000:00/0000:00:02.0/backlight/acpi_video0 (backlight)
UDEV  [280.187550] change   /devices/pci0000:00/0000:00:02.0/backlight/acpi_video0 (backlight)
KERNEL[280.431646] change   /devices/pci0000:00/0000:00:02.0/backlight/acpi_video0 (backlight)
UDEV  [280.432869] change   /devices/pci0000:00/0000:00:02.0/backlight/acpi_video0 (backlight)
KERNEL[280.462208] change   /devices/pci0000:00/0000:00:02.0/backlight/acpi_video0 (backlight)
UDEV  [280.462858] change   /devices/pci0000:00/0000:00:02.0/backlight/acpi_video0 (backlight)
KERNEL[280.492426] change   /devices/pci0000:00/0000:00:02.0/backlight/acpi_video0 (backlight)
UDEV  [280.494159] change   /devices/pci0000:00/0000:00:02.0/backlight/acpi_video0 (backlight)
.....

acpi_listen:
video/brightnessup BRTUP 00000086 00000000
video/brightnessup BRTUP 00000086 00000000
video/brightnessup BRTUP 00000086 00000000
 PNP0C14:00 000000bc 00000000
 PNP0C14:00 000000bc 00000000
video/brightnessup BRTUP 00000086 00000000
 PNP0C14:00 000000bc 00000000
 PNP0C14:00 000000bc 00000000
 PNP0C14:00 000000bc 00000000
video/brightnessup BRTUP 00000086 00000000
.......

Sometimes randr notification is triggered, but it happens at random time and very infrequently.

xev:
RRNotify event, serial 52, synthetic NO, window 0x4800001,
    subtype XRROutputPropertyChangeNotifyEvent
    output eDP1, property Backlight, timestamp 524659, state NewValue

only single notification for ~50 keypresses

I have xf86-video-git built from git master using SNA backend
Comment 1 Alexander Mezin 2014-06-05 18:58:52 UTC
Created attachment 100485 [details]
Xorg.0.log
Comment 2 Chris Wilson 2014-06-05 20:19:12 UTC
Hmm, interesting thought about udev notifcations. When I last looked at this, I looked at how you could poll(/sys/class/backlight/*/brightness) to detect the changes. That doesn't work very well with X's select() mechanism. However, we could use the udev notification...
Comment 3 Chris Wilson 2014-06-05 21:48:44 UTC
Created attachment 100492 [details] [review]
Hook up a backlight udev monitor

Something like this should work. It will likely lose the backlight value over modesetting, but that is due to another bug that needs to be fixed first. However, this should be enough for you to test whether the GUI updates when you press the backlight keys.
Comment 4 Chris Wilson 2014-06-05 21:56:44 UTC
Created attachment 100493 [details] [review]
Hook up a backlight udev monitor
Comment 5 Alexander Mezin 2014-06-06 05:47:02 UTC
Sadly, no improvement at all.

Xev prints "RRNotify" only when backlight is at 100% brightness, or at some random moments as before.
Comment 6 Chris Wilson 2014-06-06 06:00:15 UTC
Created attachment 100503 [details] [review]
Hook up a backlight monitor

Ah, the arguments to RRChangeOutputProperty() specify whether to send an event or not. Let's try that again.
Comment 7 Alexander Mezin 2014-06-06 06:17:20 UTC
Now it works, thanks
Comment 8 Chris Wilson 2014-06-06 07:15:13 UTC
commit c6cd10f536e099277cdc46643725a5a50ea8b525
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Thu Jun 5 22:43:37 2014 +0100

    sna: Hook up a backlight udev monitor for external changes
    
    Changes to the backlights are notified through uevents. Hooking up a
    udev monitor to listen out for external changes to the backlight (e.g.
    through ACPI function keys, or by the user writing to
    /sys/class/backlight directly) is easier than enabling polling on the
    backlight sysfs file using X's select() mechanism.
    
    Since we listen to backlight changes, we have to be careful not to
    confuse the side-effects of disabling connectors (which may cause either
    ourselves or the kernel to turn off the backlight) with the user value.
    
    Many thanks to Alexander Mezin for the suggestion to use udev for
    tracking the notifications for external changes to the backlight.
    
    Reported-by: Alexander Mezin <mezin.alexander@gmail.com>
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=79699
    Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.