All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel.vetter@ffwll.ch>
To: DRI Development <dri-devel@lists.freedesktop.org>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Subject: [PATCH] drm/crtc-helper: explicit DPMS on after modeset
Date: Sat, 15 Jun 2013 22:41:11 +0200	[thread overview]
Message-ID: <1371328871-16858-1-git-send-email-daniel.vetter@ffwll.ch> (raw)
In-Reply-To: <1371247996-6052-4-git-send-email-daniel.vetter@ffwll.ch>

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@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@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@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@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@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

  reply	other threads:[~2013-06-15 20:41 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-14 22:13 [PATCH 0/6] fb refcount and crtc helper patches Daniel Vetter
2013-06-14 22:13 ` [PATCH 1/6] drm/crtc-helpers: Enforce sane set_config api Daniel Vetter
2013-06-14 22:13 ` [PATCH 2/6] drm/crtc-helper: no need to check for fb->depth/bpp Daniel Vetter
2013-06-14 22:13 ` [PATCH 3/6] drm/crtc-helper: explicit DPMS on after modeset Daniel Vetter
2013-06-15 20:41   ` Daniel Vetter [this message]
2013-06-14 22:13 ` [PATCH 4/6] drm/crtc-helper: don't disable disconnected outputs Daniel Vetter
2013-06-15 20:41   ` [PATCH] " Daniel Vetter
2013-06-25  1:09     ` Dave Airlie
2013-06-25  9:29       ` Daniel Vetter
2013-06-14 22:13 ` [PATCH 5/6] drm: check that ->set_config properly updates the fb Daniel Vetter
2013-06-14 22:13 ` [PATCH 6/6] drm: fix fb leak in setcrtc Daniel Vetter
2013-07-19 16:57 [PATCH] drm/crtc-helper: explicit DPMS on after modeset Daniel Vetter
2013-07-19 19:33 ` Alex Deucher

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1371328871-16858-1-git-send-email-daniel.vetter@ffwll.ch \
    --to=daniel.vetter@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.