Re: [PATCH] xf86-video-intel: change order of DPMS operations

[Chris Wilson <chris@chris-wilson.co.uk>]

On Wed,  7 Dec 2011 16:20:41 -0800, Simon Que <sque@chromium.org> wrote:
> The operations when setting dpms on should be in the order opposite
> of what's done when setting dpms off.
> 
> This is because of potentially conflicting effects:
> ~ drmModeConnectoSetProperty() enables/disables the backlight driver.
> Some backlight drivers such as intel_backlight set the backlight to 0
> when disabled and to max when enabled.
> ~ intel_output_dpms_backlight() saves the backlight value when turning
> DPMS off and restores it when turning DPMS on.
> 
> Here's the current order of operations:
> 
> xset dpms force off (backlight is nonzero)
>    drmModeConnectoSetProperty(DPMSModeOff)
>       kernel: disable backlight, backlight=0
>    intel_output_dpms_backlight(DPMSModeOff)
>       save backlight value (0) <-- it has been set to 0 by kernel
>       set backlight to 0
> 
> xset dpms force on
>    drmModeConnectoSetProperty(DPMSModeOn)
>       kernel: enable backlight, backlight=max
>    intel_output_dpms_backlight(DPMSModeOn)
>       set backlight to saved value (0)
> 
> The correct way to do this would be to reverse the operations during
> xset dpms force off:
>    intel_output_dpms_backlight(DPMSModeOff)
>       save backlight value (nonzero)
>       set backlight to 0
>    drmModeConnectoSetProperty(DPMSModeOff)
>       kernel: enable backlight, backlight=0
> 
> This restores the saved nonzero backlight value during the force on.
> 
> Signed-off-by: Simon Que <sque@chromium.org>

I had to remind myself why both the driver and the kernel are both
touching the backlight across DPMS; the answer as I see it is that the
kernel only knows about the raw backlight interface whereas the driver
adjusts it via the preferred interface (which should handle the cases
where the backlight modulation is handled independently of the PWM
registers). So given that there seems to be a need for the driver to
set the backlight, the order it does so is important given the
interaction with the kernel.

Thanks,
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre