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

[Chris Wilson]

On Thu, Jun 05, 2014 at 05:01:23PM +0300, Jani Nikula wrote:
> On Thu, 05 Jun 2014, Hans de Goede <hdegoede at redhat.com> wrote:
> > We've several reports from users where the backlight comes up turned off
> > on starting X, when using video.use_native_backlight=1 (true dmi quirks, will
> > be the default for 3.16), in combination with having an external display
> > plugged in:
> >
> > https://bugzilla.redhat.com/show_bug.cgi?id=1032978
> > https://bugzilla.redhat.com/show_bug.cgi?id=1103806
> >
> > This seems to be caused by /sys/class/backlight/intel_backlight/brightness
> > reading back 0 when re-initializing the outputs. Unlike
> > /sys/class/backlight/acpi_video0/brightness which is used without the
> > video.use_native_backlight=1 param, which keeps returning the value from before
> >
> > Here is an excerpt from Xorg.log when this happens:
> >
> > [28225]: (II) intel(0): resizing framebuffer to 3286x1080
> > [28225]: (II) intel(0): switch to mode 1366x768 at 59.8 on pipe 0 using LVDS1, position (0, 0), rotation normal
> > [28225]: (II) intel(0): switch to mode 1920x1080 at 60.0 on pipe 0 using HDMI2, position (1366, 0), rotation normal
> > [28225]: (II) intel(0): DPMS off mode 3, saved backlight level 0
> > ^^^ This is an extra debug line I added, mode == the mode parameter to
> >  xxx_output_dpms_backlight, saved backlight level is the value of
> >  backlight_active_level after the xxx_output_backlight_get call.
> >
> > Note how backlight_active_level becomes 0 here.
> >
> > [28225]: (II) intel(0): switch to mode 1366x768 at 59.8 on pipe 1 using LVDS1, position (0, 0), rotation normal
> > [28225]: (II) intel(0): DPMS on mode 0, setting backlight to 0
> >
> > And here we restore the backlight to backlight_active_level which now is 0.
> >
> > This commit fixes this by not reading back the backlight setting from the
> > kernel on DPMS off.
> 
> I'm not at all familiar with the code base you're changing, and I'm just
> speculating here, but this seems a little odd.
> 
> My guess is that the sna_output_backlight_get and/or
> intel_output_backlight_get functions read the actual_brightness sysfs
> file, which reads back the value from hardware. This is the contract for
> backlight class device. The acpi video implementation returns the cached
> value if there's no BQC or BCQ method.
> 
> I think perhaps either the current brightness should be read before
> switching off the output.

It is read before we switch off the CRTC (and thus the output). So where
it is getting the zero from is a puzzle - it should be the current state
of the hardware, ergo what the user set by some other path.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre