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

[Jani Nikula <jani.nikula@linux.intel.com>]

On Thu, 05 Jun 2014, Hans de Goede <hdegoede@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@59.8 on pipe 0 using LVDS1, position (0, 0), rotation normal
> [28225]: (II) intel(0): switch to mode 1920x1080@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@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, or the brightness sysfs file should be used
(which returns the cached current value). Or, perhaps, your patch is the
right answer, as I think we should save the value across disable/enable
anyway.

Chris?

BR,
Jani.


>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  src/sna/sna_display.c   | 3 ---
>  src/uxa/intel_display.c | 3 ---
>  2 files changed, 6 deletions(-)
>
> diff --git a/src/sna/sna_display.c b/src/sna/sna_display.c
> index 13dbf64..c9d4b08 100644
> --- a/src/sna/sna_display.c
> +++ b/src/sna/sna_display.c
> @@ -2323,9 +2323,6 @@ sna_output_dpms_backlight(xf86OutputPtr output, int oldmode, int mode)
>  			sna_output_backlight_set(output,
>  						   sna_output->backlight_active_level);
>  	} else {
> -		/* Only save the current backlight value if we're going from on to off. */
> -		if (oldmode == DPMSModeOn)
> -			sna_output->backlight_active_level = sna_output_backlight_get(output);
>  		sna_output_backlight_set(output, 0);
>  	}
>  }
> diff --git a/src/uxa/intel_display.c b/src/uxa/intel_display.c
> index 95ddcc8..62902f4 100644
> --- a/src/uxa/intel_display.c
> +++ b/src/uxa/intel_display.c
> @@ -963,9 +963,6 @@ intel_output_dpms_backlight(xf86OutputPtr output, int oldmode, int mode)
>  			intel_output_backlight_set(output,
>  						   intel_output->backlight_active_level);
>  	} else {
> -		/* Only save the current backlight value if we're going from on to off. */
> -		if (oldmode == DPMSModeOn)
> -			intel_output->backlight_active_level = intel_output_backlight_get(output);
>  		intel_output_backlight_set(output, 0);
>  	}
>  }
> -- 
> 2.0.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center