Attachment 82700 Details for Bug 67043 – possible fix

[patch] possible fix

0001-drm-crtc-helper-explicit-DPMS-on-after-modeset.patch (text/plain), 5.70 KB, created by Alex Deucher on 2013-07-19 17:02:01 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Alex Deucher

Created: 2013-07-19 17:02:01 UTC

Size: 5.70 KB

patch

obsolete

>From 2f8212b59a54cef90d1b9fd31f908ca7b25a21c5 Mon Sep 17 00:00:00 2001
>From: Daniel Vetter <daniel.vetter@ffwll.ch>
>Date: Fri, 19 Jul 2013 18:57:11 +0200
>Subject: [PATCH] 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 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 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 <daniel.vetter@ffwll.ch>
>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 <airlied@redhat.com>
>Cc: Alex Deucher <alexdeucher@gmail.com>
>Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=67043
>Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>---
> drivers/gpu/drm/drm_crtc_helper.c |   27 +++++++++++----------------
> 1 files changed, 11 insertions(+), 16 deletions(-)
>
>diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
>index 738a429..6a64749 100644
>--- a/drivers/gpu/drm/drm_crtc_helper.c
>+++ b/drivers/gpu/drm/drm_crtc_helper.c
>@@ -677,6 +677,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;
>+				}
> 			}
> 		}
> 
>@@ -754,6 +759,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) {
>@@ -771,22 +782,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);
>-- 
>1.7.7.5
>

Actions: View | Splinter Review

Attachments on bug 67043: 82603 | 82613 | 82700 | 83581

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.