drm/crtc-helper: explicit DPMS on after modeset

Atm the crtc helper implementation of set_config has really
inconsisten semantics: If just an fb update is good enough, dpms state
will be left as-is, but if we do a full modeset we force everything to
dpms on.

This change has already been applied to the i915 modeset code in

commit 877b103b0951d1aaa04a35aadaff3f8737c0e2ff
Author: Imre Deak <imre.deak@intel.com>
Date:   Fri May 3 19:44:07 2013 +0200

    drm/i915: force full modeset if the connector is in DPMS OFF mode

which according to Greg KH seems to aim for a new record in most
Bugzilla: links in a commit message.

The history of this dpms forcing is pretty interesting. This patch
here is an almost-revert of

commit 425f4ea2dbcd97f9dde68fe6e520f5ed61f06120
Author: Keith Packard <keithp@keithp.com>
Date:   Thu Feb 3 16:57:28 2011 -0800

    drm: Only set DPMS ON when actually configuring a mode

which fixed the bug of trying to dpms on disabled outputs, but
introduced the new discrepancy between an fb update only and full
modesets. The actual introduction of this goes back to

commit f62662377f10ef8a36b68eda89806612056c6673
Author: Keith Packard <keithp@keithp.com>
Date:   Fri Nov 26 10:45:58 2010 -0800

    drm: Set connector DPMS status to ON in drm_crtc_helper_set_config

And if you'd dig around in the i915 driver code there's even more fun
around forcing dpms on and losing our heads and temper of the
resulting inconsistencies. Especially the DP re-training code had tons
of funny stuff in it.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Dave Airlie <airlied@redhat.com>

diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index 1ace9c11a3ed0f7214f9473160e9213ff5705d7c..738a4294d820e164d7534f830dd4eed7c880aa4d 100644 (file)
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -754,12 +754,6 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set)
                                ret = -EINVAL;
                                goto fail;
                        }
-                       DRM_DEBUG_KMS("Setting connector DPMS state to on\n");
-                       for (i = 0; i < set->num_connectors; i++) {
-                               DRM_DEBUG_KMS("\t[CONNECTOR:%d:%s] set DPMS on\n", set->connectors[i]->base.id,
-                                             drm_get_connector_name(set->connectors[i]));
-                               set->connectors[i]->funcs->dpms(set->connectors[i], DRM_MODE_DPMS_ON);
-                       }
                }
                drm_helper_disable_unused_functions(dev);
        } else if (fb_changed) {
@@ -777,6 +771,22 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set)
                }
        }
 
+       /*
+        * crtc set_config helpers implicit set the crtc and all connected
+        * encoders to DPMS on for a full mode set. But for just an fb update it
+        * doesn't do that. To not confuse userspace, do an explicit DPMS_ON
+        * unconditionally. This will also ensure driver internal dpms state is
+        * consistent again.
+        */
+       if (set->crtc->enabled) {
+               DRM_DEBUG_KMS("Setting connector DPMS state to on\n");
+               for (i = 0; i < set->num_connectors; i++) {
+                       DRM_DEBUG_KMS("\t[CONNECTOR:%d:%s] set DPMS on\n", set->connectors[i]->base.id,
+                                     drm_get_connector_name(set->connectors[i]));
+                       set->connectors[i]->funcs->dpms(set->connectors[i], DRM_MODE_DPMS_ON);
+               }
+       }
+
        kfree(save_connectors);
        kfree(save_encoders);
        kfree(save_crtcs);