xf86-video-intel: src/intel_display.c src/sna/sna_display.c

[Chris Wilson]

 src/intel_display.c   |   12 +++++++++---
 src/sna/sna_display.c |   21 ++++++++++++++++++---
 2 files changed, 27 insertions(+), 6 deletions(-)

New commits:
commit bc081420a5d6d28c4e6fd5527a70caaa3a8361c4
Author: Simon Que <sque at chromium.org>
Date:   Wed Dec 7 16:20:41 2011 -0800

    xf86-video-intel: change order of DPMS operations
    
    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 at chromium.org>
    Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>

diff --git a/src/intel_display.c b/src/intel_display.c
index 2183f4d..5ad8625 100644
--- a/src/intel_display.c
+++ b/src/intel_display.c
@@ -944,13 +944,19 @@ intel_output_dpms(xf86OutputPtr output, int dpms)
 			continue;
 
 		if (!strcmp(props->name, "DPMS")) {
+			/* Make sure to reverse the order between on and off. */
+			if (dpms == DPMSModeOff)
+				intel_output_dpms_backlight(output,
+							    intel_output->dpms_mode,
+							    dpms);
 			drmModeConnectorSetProperty(mode->fd,
 						    intel_output->output_id,
 						    props->prop_id,
 						    dpms);
-			intel_output_dpms_backlight(output,
-						      intel_output->dpms_mode,
-						      dpms);
+			if (dpms != DPMSModeOff)
+				intel_output_dpms_backlight(output,
+							    intel_output->dpms_mode,
+							    dpms);
 			intel_output->dpms_mode = dpms;
 			drmModeFreeProperty(props);
 			return;
diff --git a/src/sna/sna_display.c b/src/sna/sna_display.c
index 32162ef..b1550a2 100644
--- a/src/sna/sna_display.c
+++ b/src/sna/sna_display.c
@@ -1142,13 +1142,28 @@ sna_output_dpms(xf86OutputPtr output, int dpms)
 			continue;
 
 		if (!strcmp(props->name, "DPMS")) {
+			/* Record thevalue of the backlight before turning
+			 * off the display, and reset if after turnging it on.
+			 * Order is important as the kernel may record and also
+			 * reset the backlight across DPMS. Hence we need to
+			 * record the value before the kernel modifies it
+			 * and reapply it afterwards.
+			 */
+			if (dpms == DPMSModeOff)
+				sna_output_dpms_backlight(output,
+							  sna_output->dpms_mode,
+							  dpms);
+
 			drmModeConnectorSetProperty(sna->kgem.fd,
 						    sna_output->output_id,
 						    props->prop_id,
 						    dpms);
-			sna_output_dpms_backlight(output,
-						      sna_output->dpms_mode,
-						      dpms);
+
+			if (dpms != DPMSModeOff)
+				sna_output_dpms_backlight(output,
+							  sna_output->dpms_mode,
+							  dpms);
+
 			sna_output->dpms_mode = dpms;
 			drmModeFreeProperty(props);
 			return;