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@chromium.org>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

2 files changed, 27 insertions, 6 deletions

diff --git a/src/intel_display.c b/src/intel_display.c
index 2183f4de..5ad8625e 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 32162ef2..b1550a24 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;