[PATCH] drm/crtc-helper: explicit DPMS on after modeset

[Daniel Vetter]

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 e3de42b68478a8c95dd27520e9adead2af9477a5
Author: Imre Deak <imre.deak at 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 811aaa55ba21ab37407018cfc01770d6b037d3fb
Author: Keith Packard <keithp at 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 bf9dc102e284a5aa78c73fc9d72e11d5ccd8669f
Author: Keith Packard <keithp at 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.

v2: So v1 totally blew up on resume on my radeon system here. After
much head-scraching I've figured out that the radeon resume functions
resumes the console system _before_ it actually restores all the
modeset state. And resuming the console systems means that fbdev doeas
an immediate ->set_par call.

Now up to this patch that ->set_par did absolutely nothing: All the
old sw state from pre-suspend was still around (since the modeset
reset wasn't done yet), which means that the set_config calls done as
a result of the ->set_par where all treated as no-ops (despite the the
real hw state was obviously something completely else).

Since v1 of this patch just added a bunch of ->dpms calls if the crtc
was enabled, those set_config calls suddenly stopped being no-ops. But
because the hw state wasn't restored the ->dpms callbacks resulted in
decent amounts of hilarity and eventual full hangs.

Sinc I can't review all kms drivers for such tricky ordering
constraints v2 opts of a different approach and forces a full modeset
if the connector dpms state isnt' DPMS_ON. Since the ->dpms callbacks
implemented by the modeset helpers update the connector->dpms property
we have the same effect of ensure that the pipe is ultimately turned
on, even if we just end up updating the fb. This is the same approache
we ended up using in the intel driver.

Note that besides i915.ko only all other drivers eventually call
drm_helper_connector_dpms with the exception of vmwgfx, which does not
support dmps at all.

Cc: Alex Deucher <alexdeucher at gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
---
 drivers/gpu/drm/drm_crtc_helper.c |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index 72282af..eb51dc4 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -669,6 +669,11 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set)
 					/* don't break so fail path works correct */
 					fail = 1;
 				break;
+
+				if (connector->dpms != DRM_MODE_DPMS_ON) {
+					DRM_DEBUG_KMS("connector dpms not on, full mode switch\n");
+					mode_changed = true;
+				}
 			}
 		}
 
-- 
1.7.10.4