[PATCH 1/2] drm: Set connector DPMS status to ON in drm_crtc_helper_set_config

[PATCH 1/2] drm: Set connector DPMS status to ON in drm_crtc_helper_set_config

[Keith Packard]

When setting a new crtc configuration, force the DPMS state of all
connectors to ON. Otherwise, they'll be left at OFF and a future mode set
that disables the specified connector will not turn the connector off.

Signed-off-by: Keith Packard <keithp@keithp.com>
---
 drivers/gpu/drm/drm_crtc_helper.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index f7af91c..232ee93 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -471,6 +471,7 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set)
 	int count = 0, ro, fail = 0;
 	struct drm_crtc_helper_funcs *crtc_funcs;
 	int ret = 0;
+	int i;
 
 	DRM_DEBUG_KMS("\n");
 
@@ -666,6 +667,12 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set)
 		if (ret != 0)
 			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]->dpms = DRM_MODE_DPMS_ON;
+	}
 
 	kfree(save_connectors);
 	kfree(save_encoders);
-- 
1.7.2.3

[PATCH 2/2] drm: record monitor status in output_poll_execute

[Keith Packard]

In order to correctly report monitor connected status changes, the
previous monitor status must be recorded in the connector->status
value instead of being discarded.

Signed-off-by: Keith Packard <keithp@keithp.com>
---
 drivers/gpu/drm/drm_crtc_helper.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index 232ee93..7ca5935 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -848,7 +848,7 @@ static void output_poll_execute(struct work_struct *work)
 	struct delayed_work *delayed_work = to_delayed_work(work);
 	struct drm_device *dev = container_of(delayed_work, struct drm_device, mode_config.output_poll_work);
 	struct drm_connector *connector;
-	enum drm_connector_status old_status, status;
+	enum drm_connector_status old_status;
 	bool repoll = false, changed = false;
 
 	if (!drm_kms_helper_poll)
@@ -873,8 +873,9 @@ static void output_poll_execute(struct work_struct *work)
 		    !(connector->polled & DRM_CONNECTOR_POLL_HPD))
 			continue;
 
-		status = connector->funcs->detect(connector, false);
-		if (old_status != status)
+		connector->status = connector->funcs->detect(connector, false);
+		DRM_DEBUG_KMS("connector status updated to %d\n", connector->status);
+		if (old_status != connector->status)
 			changed = true;
 	}
 
-- 
1.7.2.3

Re: [Intel-gfx] [PATCH 2/2] drm: record monitor status in output_poll_execute

[Florian Mickler]

On Fri, 26 Nov 2010 10:45:59 -0800
Keith Packard <keithp@keithp.com> wrote:

> In order to correctly report monitor connected status changes, the
> previous monitor status must be recorded in the connector->status
> value instead of being discarded.
> 
> Signed-off-by: Keith Packard <keithp@keithp.com>

Keith, am I right to assume that these address 
https://bugzilla.kernel.org/show_bug.cgi?id=22672
and probably:
https://bugzilla.kernel.org/show_bug.cgi?id=23472 

Regards,
Flo

> ---
>  drivers/gpu/drm/drm_crtc_helper.c |    7 ++++---
>  1 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
> index 232ee93..7ca5935 100644
> --- a/drivers/gpu/drm/drm_crtc_helper.c
> +++ b/drivers/gpu/drm/drm_crtc_helper.c
> @@ -848,7 +848,7 @@ static void output_poll_execute(struct work_struct *work)
>  	struct delayed_work *delayed_work = to_delayed_work(work);
>  	struct drm_device *dev = container_of(delayed_work, struct drm_device, mode_config.output_poll_work);
>  	struct drm_connector *connector;
> -	enum drm_connector_status old_status, status;
> +	enum drm_connector_status old_status;
>  	bool repoll = false, changed = false;
>  
>  	if (!drm_kms_helper_poll)
> @@ -873,8 +873,9 @@ static void output_poll_execute(struct work_struct *work)
>  		    !(connector->polled & DRM_CONNECTOR_POLL_HPD))
>  			continue;
>  
> -		status = connector->funcs->detect(connector, false);
> -		if (old_status != status)
> +		connector->status = connector->funcs->detect(connector, false);
> +		DRM_DEBUG_KMS("connector status updated to %d\n", connector->status);
> +		if (old_status != connector->status)
>  			changed = true;
>  	}
>  

Re: [Intel-gfx] [PATCH 2/2] drm: record monitor status in output_poll_execute

[Keith Packard]

[-- Attachment #1: Type: text/plain, Size: 759 bytes --]

On Sun, 5 Dec 2010 13:27:43 +0100, Florian Mickler <florian@mickler.org> wrote:
> On Fri, 26 Nov 2010 10:45:59 -0800
> Keith Packard <keithp@keithp.com> wrote:
> 
> > In order to correctly report monitor connected status changes, the
> > previous monitor status must be recorded in the connector->status
> > value instead of being discarded.
> > 
> > Signed-off-by: Keith Packard <keithp@keithp.com>
> 
> Keith, am I right to assume that these address 
> https://bugzilla.kernel.org/show_bug.cgi?id=22672
> and probably:
> https://bugzilla.kernel.org/show_bug.cgi?id=23472

Yes, this is part of the patch for that. The other piece is in the 2D
driver which has a matching bug. That patch is already on master.

-- 
keith.packard@intel.com

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [Intel-gfx] [PATCH 2/2] drm: record monitor status in output_poll_execute

[Florian Mickler]

On Sun, 05 Dec 2010 13:08:18 -0800
Keith Packard <keithp@keithp.com> wrote:

> On Sun, 5 Dec 2010 13:27:43 +0100, Florian Mickler <florian@mickler.org> wrote:
> > On Fri, 26 Nov 2010 10:45:59 -0800
> > Keith Packard <keithp@keithp.com> wrote:
> > 
> > > In order to correctly report monitor connected status changes, the
> > > previous monitor status must be recorded in the connector->status
> > > value instead of being discarded.
> > > 
> > > Signed-off-by: Keith Packard <keithp@keithp.com>
> > 
> > Keith, am I right to assume that these address 
> > https://bugzilla.kernel.org/show_bug.cgi?id=22672
> > and probably:
> > https://bugzilla.kernel.org/show_bug.cgi?id=23472
> 
> Yes, this is part of the patch for that. The other piece is in the 2D
> driver which has a matching bug. That patch is already on master.
> 

Does that mean that the kernel regression will not be
fixed/worked-around for old userspace? 

Regards,
Flo

Re: [Intel-gfx] [PATCH 2/2] drm: record monitor status in output_poll_execute

[Chris Wilson]

On Wed, 8 Dec 2010 17:34:24 +0100, Florian Mickler <florian@mickler.org> wrote:
> Does that mean that the kernel regression will not be
> fixed/worked-around for old userspace? 

I think there is some confusion in that I believe there is more than one
backlight bug at play here.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [Intel-gfx] [PATCH 2/2] drm: record monitor status in output_poll_execute

[Keith Packard]

[-- Attachment #1: Type: text/plain, Size: 857 bytes --]

On Wed, 08 Dec 2010 17:08:04 +0000, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Wed, 8 Dec 2010 17:34:24 +0100, Florian Mickler <florian@mickler.org> wrote:
> > Does that mean that the kernel regression will not be
> > fixed/worked-around for old userspace? 
> 
> I think there is some confusion in that I believe there is more than one
> backlight bug at play here.

The kernel and user space fixes addressed the same  technical problem
(failing to note that mode setting turned on the devices), but the
symptoms that this caused were different; in kernel mode, the result
would be that monitors would be left turned on when user space asked
that they be turned off (and left pointing at random memory too, which
was amusing). I think that the backlight issue was caused by the
user-space bug though.

-- 
keith.packard@intel.com

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]