[PATCH] drm: Recover DPMS properly after XRandr re-enablement

[PATCH] drm: Recover DPMS properly after XRandr re-enablement

[Chris Wilson]

From: Takashi Iwai <tiwai@suse.de>

When the output is turned off via "xrandr --off" and re-enabled again
with the same mode, drm doesn't reset DPMS, thus it results in a black
screen.  A typical example is something like:

     % xrandr --output LVDS1 --mode 1024x768
     % xrandr --output VGA1 --mode 1024x768
     % xrandr --output LVDS1 --off
     % xrandr --output LVDS1 --mode 1024x768

Also, there is another problem with DPMS: the sysfs entries don't
match with the actual state.  In the case above, LVDS1 shows always
Off, while VGA1 shows On.

A part of the cause is that there are (still) two places to keep
the actual DPMS values.  In dpms field of drm_connector and in the
device property.  Thus we need to sync between them.

Another problem is that the DPMS state is always set to ON forcibly
in drm_crtc_helper_set_config() (via commit
bf9dc102e284a5aa78c73fc9d72e11d5ccd8669f).  This results in another
inconsistency.  For recovering the DPMS, we need to set DPMS actaully
ON there.

This patch adds a new helper function to manage the drm_connector
DPMS so that it can be called commonly in both places.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/gpu/drm/drm_crtc.c        |   25 ++++++++++++++++++-------
 drivers/gpu/drm/drm_crtc_helper.c |    7 ++++---
 include/drm/drm_crtc.h            |    1 +
 3 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 2baa670..3a58337 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -2383,6 +2383,18 @@ int drm_mode_connector_update_edid_property(struct drm_connector *connector,
 }
 EXPORT_SYMBOL(drm_mode_connector_update_edid_property);
 
+int drm_connector_set_dpms(struct drm_connector *connector, int mode)
+{
+	if (connector->funcs->dpms)
+		(*connector->funcs->dpms)(connector, mode);
+	connector->dpms = mode;
+	drm_connector_property_set_value(connector,
+					 connector->dev->mode_config.dpms_property,
+					 mode);
+	return 0;
+}
+EXPORT_SYMBOL(drm_connector_set_dpms);
+
 int drm_mode_connector_property_set_ioctl(struct drm_device *dev,
 				       void *data, struct drm_file *file_priv)
 {
@@ -2440,15 +2452,14 @@ int drm_mode_connector_property_set_ioctl(struct drm_device *dev,
 
 	/* Do DPMS ourselves */
 	if (property == connector->dev->mode_config.dpms_property) {
-		if (connector->funcs->dpms)
-			(*connector->funcs->dpms)(connector, (int) out_resp->value);
+		drm_connector_set_dpms(connector, (int) out_resp->value);
 		ret = 0;
-	} else if (connector->funcs->set_property)
+	} else if (connector->funcs->set_property) {
 		ret = connector->funcs->set_property(connector, property, out_resp->value);
-
-	/* store the property value if successful */
-	if (!ret)
-		drm_connector_property_set_value(connector, property, out_resp->value);
+		/* store the property value if successful */
+		if (!ret)
+			drm_connector_property_set_value(connector, property, out_resp->value);
+	}
 out:
 	mutex_unlock(&dev->mode_config.mutex);
 	return ret;
diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index bede10a..fee755d 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -669,9 +669,10 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set)
 	}
 	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]->dpms = DRM_MODE_DPMS_ON;
+		struct drm_connector *connector = set->connectors[i];
+		DRM_DEBUG_KMS("\t[CONNECTOR:%d:%s] set DPMS on\n", connector->base.id,
+			      drm_get_connector_name(connector));
+		drm_connector_set_dpms(connector, DRM_MODE_DPMS_ON);
 	}
 
 	kfree(save_connectors);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 029aa68..c9a5a80 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -676,6 +676,7 @@ extern void drm_mode_set_crtcinfo(struct drm_display_mode *p,
 extern void drm_mode_connector_list_update(struct drm_connector *connector);
 extern int drm_mode_connector_update_edid_property(struct drm_connector *connector,
 						struct edid *edid);
+extern int drm_connector_set_dpms(struct drm_connector *connector, int mode);
 extern int drm_connector_property_set_value(struct drm_connector *connector,
 					 struct drm_property *property,
 					 uint64_t value);
-- 
1.7.3.1

Re: [PATCH] drm: Recover DPMS properly after XRandr re-enablement

[Chris Wilson]

On Wed, 22 Dec 2010 12:42:32 +0000, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> From: Takashi Iwai <tiwai@suse.de>

> This patch adds a new helper function to manage the drm_connector
> DPMS so that it can be called commonly in both places.
> 
> Signed-off-by: Takashi Iwai <tiwai@suse.de>

FWIW,

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm: Recover DPMS properly after XRandr re-enablement

[Florian Mickler]

On Wed, 22 Dec 2010 12:53:07 +0000
Chris Wilson <chris@chris-wilson.co.uk> wrote:

> On Wed, 22 Dec 2010 12:42:32 +0000, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > From: Takashi Iwai <tiwai@suse.de>
> 
> > This patch adds a new helper function to manage the drm_connector
> > DPMS so that it can be called commonly in both places.
> > 
> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> 
> FWIW,
> 
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> -Chris
> 

Is this patch still necessary for 2.6.39?

It was needed to fix a regression from 2.6.36 (at least in February). 
( https://bugzilla.kernel.org/show_bug.cgi?id=24982 )

Regards,
Flo

Re: [PATCH] drm: Recover DPMS properly after XRandr re-enablement

[Takashi Iwai]

At Sun, 17 Apr 2011 18:26:54 +0200,
Florian Mickler wrote:
> 
> On Wed, 22 Dec 2010 12:53:07 +0000
> Chris Wilson <chris@chris-wilson.co.uk> wrote:
> 
> > On Wed, 22 Dec 2010 12:42:32 +0000, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > > From: Takashi Iwai <tiwai@suse.de>
> > 
> > > This patch adds a new helper function to manage the drm_connector
> > > DPMS so that it can be called commonly in both places.
> > > 
> > > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > 
> > FWIW,
> > 
> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> > 
> > -Chris
> > 
> 
> Is this patch still necessary for 2.6.39?

Yes, I think so.


Takashi

> It was needed to fix a regression from 2.6.36 (at least in February). 
> ( https://bugzilla.kernel.org/show_bug.cgi?id=24982 )
> 
> Regards,
> Flo
>