From patchwork Fri Jul 19 16:57:11 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 2830633 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 080EA9F4E2 for ; Fri, 19 Jul 2013 16:58:00 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id D1BF22037F for ; Fri, 19 Jul 2013 16:57:58 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 0787A20144 for ; Fri, 19 Jul 2013 16:57:57 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id C9D3DE6A3B for ; Fri, 19 Jul 2013 09:57:56 -0700 (PDT) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-ee0-f51.google.com (mail-ee0-f51.google.com [74.125.83.51]) by gabe.freedesktop.org (Postfix) with ESMTP id 04BE4E6920 for ; Fri, 19 Jul 2013 09:57:14 -0700 (PDT) Received: by mail-ee0-f51.google.com with SMTP id e52so2468212eek.38 for ; Fri, 19 Jul 2013 09:57:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=from:to:cc:subject:date:message-id:x-mailer; bh=nvLb1tkrmWEIfJRtlrkE3sj0107+521tZ7QSzib/z6w=; b=K6At9dq0MPjkUc0jHFpN4gTExBJD2oFnGCymwrDzdpOGKIouhcGPyuYE60VRqh93RU cDE75PzMwmP5moAfYj9a7q0UuRJNxKu4pddtCWIFyUAwMcJsQV+Uk9JINtRCGSlyw4p2 vD+43NP4RfIg3wXK8FzF5YZvta2t+dtnzL2+E= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=from:to:cc:subject:date:message-id:x-mailer:x-gm-message-state; bh=nvLb1tkrmWEIfJRtlrkE3sj0107+521tZ7QSzib/z6w=; b=ldRsXvYTfG0qdfATp48FZDyeIbk/GAfampjrZYDh/sX17bKg6/CVcti97gczU0OKZg CL8+m+TsIuARgvifb78SUbg3BWJYbdXn1pJHFh8XAbszkShOF39DrXHXsDoMxThxufNs 9wqPQyrFoxBoz6mCUnDA2eg3JdyWg+kNO6itKS4oVGIuzWTaGi49Xa2evYoLCdaQAQBb 7OIePuu1fmDDMPbIeOt3UGUX9lf8T/a2zpgovWOcsR3/b6//Q0sI7Cvgb2A+C+tyl6ZP euXLvPF56wqNCLjl3TQnF7NOif3ffnL0yi4dOzBxJkQAYR7i/7JWIv/TxhhPgoTIGF7n jr7g== X-Received: by 10.15.93.134 with SMTP id w6mr16929679eez.25.1374253034011; Fri, 19 Jul 2013 09:57:14 -0700 (PDT) Received: from phenom.ffwll.local (178-83-130-250.dynamic.hispeed.ch. [178.83.130.250]) by mx.google.com with ESMTPSA id a4sm28896135eez.0.2013.07.19.09.57.12 for (version=TLSv1.2 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Fri, 19 Jul 2013 09:57:13 -0700 (PDT) From: Daniel Vetter To: DRI Development Subject: [PATCH] drm/crtc-helper: explicit DPMS on after modeset Date: Fri, 19 Jul 2013 18:57:11 +0200 Message-Id: <1374253031-32092-1-git-send-email-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 1.8.3.2 X-Gm-Message-State: ALoCoQlHrf9gNS4v7yKejj2TaIvZIue6QDKRWQtTt2AK8qR8oFjsvwuuOevhUmdYm7U7W0fEKeUd Cc: Daniel Vetter , Dave Airlie X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: dri-devel-bounces+patchwork-dri-devel=patchwork.kernel.org@lists.freedesktop.org Errors-To: dri-devel-bounces+patchwork-dri-devel=patchwork.kernel.org@lists.freedesktop.org X-Spam-Status: No, score=-4.5 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED,RP_MATCHES_RCVD,T_DKIM_INVALID,UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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 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 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 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 that the real hw state was obviously something completely different). 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. Since I can't review all kms drivers for such tricky ordering constraints v2 opts for 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 ensuring that the pipe is ultimately turned on, even if we just end up updating the fb. This is the same approac 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. v3: Dave Airlie merged the broken first version of this patch, so squash in the revert of commit 372835a8527f85b3eff20a18c2c339e827dfd4e4 Author: Daniel Vetter Date: Sat Jun 15 00:13:13 2013 +0200 drm/crtc-helper: explicit DPMS on after modeset Also fix up the spelling fail a bit in the commit message while at it. Cc: Dave Airlie Cc: Alex Deucher Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=67043 Signed-off-by: Daniel Vetter Reviewed-by: Alex Deucher --- drivers/gpu/drm/drm_crtc_helper.c | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c index edb0de5..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; + } } } @@ -746,6 +751,12 @@ 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) { @@ -763,22 +774,6 @@ 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);