[Intel-gfx] [PATCH] backlight: Don't read back backlight setting from kernel on DPMS off

[Chris Wilson]

On Fri, Jun 13, 2014 at 10:49:12AM +0200, Hans de Goede wrote:
> Hi,
> 
> On 06/07/2014 01:12 PM, Chris Wilson wrote:
> > On Sat, Jun 07, 2014 at 12:18:35PM +0200, Hans de Goede wrote:
> >> Hi,
> >>
> >> On 06/06/2014 04:51 PM, Chris Wilson wrote:
> >>> commit c6cd10f536e099277cdc46643725a5a50ea8b525
> >>> Author: Chris Wilson <chris at chris-wilson.co.uk>
> >>> Date:   Thu Jun 5 22:43:37 2014 +0100
> >>
> >> Thanks, I fail to see how this addresses the original problem though,
> >> current master still reads back the backlight from the kernel at DPMS
> >> off, so the problem my original patch in this thread tries to fix
> >> still exists AFAIK, we will still read back 0 on DPMS off in the
> >> scenario my patch tries to address.
> > 
> > It changes the sequence in which output->funcs->dpms is called to
> > prevent the bug.
> 
> Thanks, I don't see that being changed in the above commit though,
> and I cannot find the commit in which it does change, can you point
> me to the commit where this is changed ?


commit c6cd10f536e099277cdc46643725a5a50ea8b525
Author: Chris Wilson <chris at 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 at gmail.com>
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=79699
    Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>

@@ -820,6 +949,14 @@ sna_crtc_apply(xf86CrtcPtr crtc)
        for (i = 0; i < config->num_output; i++) {
                xf86OutputPtr output = config->output[i];
 
+               /* Make sure we mark the output as off (and save the backlight)
+                * before the kernel turns it off due to changing the pipe.
+                * This is necessary as the kernel may turn off the backlight
+                * and we lose track of the user settings.
+                */
+               if (output->crtc == NULL)
+                       output->funcs->dpms(output, DPMSModeOff);
+
                if (output->crtc != crtc)
                        continue;

It does depend on those other outputs being marked as disconnected first
when switching pipes, which seems true looking at xf86RandR12CrtcSet().
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre