[PATCH] drm: fix regression in fb blank handling

[PATCH] drm: fix regression in fb blank handling

[Zhenyu Wang]

commit 731b5a15a3b1474a41c2ca29b4c32b0f21bc852e
Author: James Simmons <jsimmons@infradead.org>
Date:   Thu Oct 29 20:39:07 2009 +0000

    drm/kms: properly handle fbdev blanking

uses DRM_MODE_DPMS_ON for FB_BLANK_NORMAL, but DRM_MODE_DPMS_ON
is actually for turning output on instead of blank.

This makes fb blank broken on my T61, it put LVDS on but leave
pipe disabled which made screen totally white or caused some
'burning' effect.

Cc: James Simmons <jsimmons@infradead.org>
Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
---
 drivers/gpu/drm/drm_fb_helper.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 1c2b7d4..0f9e905 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -389,7 +389,7 @@ int drm_fb_helper_blank(int blank, struct fb_info *info)
 		break;
 	/* Display: Off; HSync: On, VSync: On */
 	case FB_BLANK_NORMAL:
-		drm_fb_helper_off(info, DRM_MODE_DPMS_ON);
+		drm_fb_helper_off(info, DRM_MODE_DPMS_STANDBY);
 		break;
 	/* Display: Off; HSync: Off, VSync: On */
 	case FB_BLANK_HSYNC_SUSPEND:
-- 
1.6.3.3


------------------------------------------------------------------------------
Throughout its 18-year history, RSA Conference consistently attracts the
world's best and brightest in the field, creating opportunities for Conference
attendees to learn about information security's most important issues through
interactions with peers, luminaries and emerging and established companies.
http://p.sf.net/sfu/rsaconf-dev2dev
--

Re: [PATCH] drm: fix regression in fb blank handling

[James Simmons]

> commit 731b5a15a3b1474a41c2ca29b4c32b0f21bc852e
> Author: James Simmons <jsimmons@infradead.org>
> Date:   Thu Oct 29 20:39:07 2009 +0000
> 
>     drm/kms: properly handle fbdev blanking
> 
> uses DRM_MODE_DPMS_ON for FB_BLANK_NORMAL, but DRM_MODE_DPMS_ON
> is actually for turning output on instead of blank.
> 
> This makes fb blank broken on my T61, it put LVDS on but leave
> pipe disabled which made screen totally white or caused some
> 'burning' effect.
> 
> Cc: James Simmons <jsimmons@infradead.org>
> Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_fb_helper.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 1c2b7d4..0f9e905 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -389,7 +389,7 @@ int drm_fb_helper_blank(int blank, struct fb_info *info)
>  		break;
>  	/* Display: Off; HSync: On, VSync: On */
>  	case FB_BLANK_NORMAL:
> -		drm_fb_helper_off(info, DRM_MODE_DPMS_ON);
> +		drm_fb_helper_off(info, DRM_MODE_DPMS_STANDBY);
>  		break;
>  	/* Display: Off; HSync: Off, VSync: On */
>  	case FB_BLANK_HSYNC_SUSPEND:

Nak. The problem is the intel drm drivers doesn't handle it power 
management state seperate from the encoder state.


------------------------------------------------------------------------------
Throughout its 18-year history, RSA Conference consistently attracts the
world's best and brightest in the field, creating opportunities for Conference
attendees to learn about information security's most important issues through
interactions with peers, luminaries and emerging and established companies.
http://p.sf.net/sfu/rsaconf-dev2dev
--

Re: [PATCH] drm: fix regression in fb blank handling

[Zhenyu Wang]

[-- Attachment #1.1: Type: text/plain, Size: 1865 bytes --]

On 2010.01.18 13:30:00 +0000, James Simmons wrote:
> 
> > commit 731b5a15a3b1474a41c2ca29b4c32b0f21bc852e
> > Author: James Simmons <jsimmons@infradead.org>
> > Date:   Thu Oct 29 20:39:07 2009 +0000
> > 
> >     drm/kms: properly handle fbdev blanking
> > 
> > uses DRM_MODE_DPMS_ON for FB_BLANK_NORMAL, but DRM_MODE_DPMS_ON
> > is actually for turning output on instead of blank.
> > 
> > This makes fb blank broken on my T61, it put LVDS on but leave
> > pipe disabled which made screen totally white or caused some
> > 'burning' effect.
> > 
> > Cc: James Simmons <jsimmons@infradead.org>
> > Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
> > ---
> >  drivers/gpu/drm/drm_fb_helper.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> > index 1c2b7d4..0f9e905 100644
> > --- a/drivers/gpu/drm/drm_fb_helper.c
> > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > @@ -389,7 +389,7 @@ int drm_fb_helper_blank(int blank, struct fb_info *info)
> >  		break;
> >  	/* Display: Off; HSync: On, VSync: On */
> >  	case FB_BLANK_NORMAL:
> > -		drm_fb_helper_off(info, DRM_MODE_DPMS_ON);
> > +		drm_fb_helper_off(info, DRM_MODE_DPMS_STANDBY);
> >  		break;
> >  	/* Display: Off; HSync: Off, VSync: On */
> >  	case FB_BLANK_HSYNC_SUSPEND:
> 
> Nak. The problem is the intel drm drivers doesn't handle it power 
> management state seperate from the encoder state.

drm_fb_helper_off() will find fb's crtc and attached encoders, then
call encoder_funcs->dpms() and turn off crtc, so for your DRM_MODE_DPMS_ON
change, it will actually turn encoder on, but what we need for
FB_BLANK_NORMAL is to turn off the display. No?

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 420 bytes --]

------------------------------------------------------------------------------
Throughout its 18-year history, RSA Conference consistently attracts the
world's best and brightest in the field, creating opportunities for Conference
attendees to learn about information security's most important issues through
interactions with peers, luminaries and emerging and established companies.
http://p.sf.net/sfu/rsaconf-dev2dev

[-- Attachment #3: Type: text/plain, Size: 161 bytes --]

--
_______________________________________________
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel

Re: [PATCH] drm: fix regression in fb blank handling

[James Simmons]

> > > uses DRM_MODE_DPMS_ON for FB_BLANK_NORMAL, but DRM_MODE_DPMS_ON
> > > is actually for turning output on instead of blank.
> > > 
> > > This makes fb blank broken on my T61, it put LVDS on but leave
> > > pipe disabled which made screen totally white or caused some
> > > 'burning' effect.
> > > 
> > > Cc: James Simmons <jsimmons@infradead.org>
> > > Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/drm_fb_helper.c |    2 +-
> > >  1 files changed, 1 insertions(+), 1 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> > > index 1c2b7d4..0f9e905 100644
> > > --- a/drivers/gpu/drm/drm_fb_helper.c
> > > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > > @@ -389,7 +389,7 @@ int drm_fb_helper_blank(int blank, struct fb_info *info)
> > >  		break;
> > >  	/* Display: Off; HSync: On, VSync: On */
> > >  	case FB_BLANK_NORMAL:
> > > -		drm_fb_helper_off(info, DRM_MODE_DPMS_ON);
> > > +		drm_fb_helper_off(info, DRM_MODE_DPMS_STANDBY);
> > >  		break;
> > >  	/* Display: Off; HSync: Off, VSync: On */
> > >  	case FB_BLANK_HSYNC_SUSPEND:
> > 
> > Nak. The problem is the intel drm drivers doesn't handle it power 
> > management state seperate from the encoder state.

Sorry I meant the backlight power management state seperate from the 
encoder state.
 
> drm_fb_helper_off() will find fb's crtc and attached encoders, then
> call encoder_funcs->dpms() and turn off crtc, so for your DRM_MODE_DPMS_ON
> change, it will actually turn encoder on, but what we need for
> FB_BLANK_NORMAL is to turn off the display. No?

FB_BLANK_NORMAL is not a full power down. Only CRTC is powerdown in this 
mode. FB_BLANK_POWERDOWN turns off everything. You are wondering why this 
is the case. If the console blanks and you go to use it again it comes 
back very fast. In FB_BLANK_POWERDOWN mode it takes awhile to come back 
for certain types of displays. I have a old Sony CRT at home. Once powered 
down it takes about 5 to 10 seconds to come back up.

Besides this the real issue is backlight != part of encoder. 

A patch will be coming shortly.


------------------------------------------------------------------------------
Throughout its 18-year history, RSA Conference consistently attracts the
world's best and brightest in the field, creating opportunities for Conference
attendees to learn about information security's most important issues through
interactions with peers, luminaries and emerging and established companies.
http://p.sf.net/sfu/rsaconf-dev2dev
--

Re: [PATCH] drm: fix regression in fb blank handling

[Zhenyu Wang]

[-- Attachment #1.1: Type: text/plain, Size: 1784 bytes --]

On 2010.01.19 16:17:56 +0000, James Simmons wrote:
> Sorry I meant the backlight power management state seperate from the 
> encoder state.
>  
> > drm_fb_helper_off() will find fb's crtc and attached encoders, then
> > call encoder_funcs->dpms() and turn off crtc, so for your DRM_MODE_DPMS_ON
> > change, it will actually turn encoder on, but what we need for
> > FB_BLANK_NORMAL is to turn off the display. No?
> 
> FB_BLANK_NORMAL is not a full power down. Only CRTC is powerdown in this 
> mode.

So how the encoder could tell the meaning of you passed DRM_MODE_DPMS_ON in
this case? which is really turning on everything, or keep port on but turn
off panel power?

> FB_BLANK_POWERDOWN turns off everything. You are wondering why this 
> is the case. If the console blanks and you go to use it again it comes 
> back very fast. In FB_BLANK_POWERDOWN mode it takes awhile to come back 
> for certain types of displays. I have a old Sony CRT at home. Once powered 
> down it takes about 5 to 10 seconds to come back up.

Isn't this monitor specific issue instead of display device problem? ;)
I don't think any LCD monitor nowadays could have this issue.

> 
> Besides this the real issue is backlight != part of encoder. 
> 
> A patch will be coming shortly.

Panel power sequencing for Intel LVDS is controlled by encoder, and closely
related to port state, although we haven't really taken LVDS port enable/disable
step before Ironlake, I think this is normal case for encoders having backlight. 

Could we just restore back to origin behavior for fixing this in 2.6.33?
Otherwise I don't know if your patch would be too intrusive...

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 420 bytes --]

------------------------------------------------------------------------------
Throughout its 18-year history, RSA Conference consistently attracts the
world's best and brightest in the field, creating opportunities for Conference
attendees to learn about information security's most important issues through
interactions with peers, luminaries and emerging and established companies.
http://p.sf.net/sfu/rsaconf-dev2dev

[-- Attachment #3: Type: text/plain, Size: 161 bytes --]

--
_______________________________________________
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel

Re: [PATCH] drm: fix regression in fb blank handling

[James Simmons]

> On 2010.01.19 16:17:56 +0000, James Simmons wrote:
> > Sorry I meant the backlight power management state seperate from the 
> > encoder state.
> >  
> > > drm_fb_helper_off() will find fb's crtc and attached encoders, then
> > > call encoder_funcs->dpms() and turn off crtc, so for your DRM_MODE_DPMS_ON
> > > change, it will actually turn encoder on, but what we need for
> > > FB_BLANK_NORMAL is to turn off the display. No?
> > 
> > FB_BLANK_NORMAL is not a full power down. Only CRTC is powerdown in this 
> > mode.
> 
> So how the encoder could tell the meaning of you passed DRM_MODE_DPMS_ON in
> this case? which is really turning on everything, or keep port on but turn
> off panel power?

So you are saying that certian combos of crtc dpms states and encoder dpms states 
are not possible. Since this can from a user application I assume the DRI 
layer has to fix this up.         

> > FB_BLANK_POWERDOWN turns off everything. You are wondering why this 
> > is the case. If the console blanks and you go to use it again it comes 
> > back very fast. In FB_BLANK_POWERDOWN mode it takes awhile to come back 
> > for certain types of displays. I have a old Sony CRT at home. Once powered 
> > down it takes about 5 to 10 seconds to come back up.
> 
> Isn't this monitor specific issue instead of display device problem? ;)
> I don't think any LCD monitor nowadays could have this issue.

Don't forget about embedded devices. They can behave in the same way.

> > Besides this the real issue is backlight != part of encoder. 
> > 
> > A patch will be coming shortly.
> 
> Panel power sequencing for Intel LVDS is controlled by encoder, and closely
> related to port state, although we haven't really taken LVDS port enable/disable
> step before Ironlake, I think this is normal case for encoders having backlight. 

For non embedded platforms yes. Actually I worked on a embedded platform 
that used a SiS graphics chip. That chipset also has a backlight for the 
encoder but the manufacture instead disabled that backlight and instead 
used a external backlight powered by a external gpio.

> Could we just restore back to origin behavior for fixing this in 2.6.33?
> Otherwise I don't know if your patch would be too intrusive...

It's just adding the backlight api to the intel driver. In fact it gives 
the user the ablity to control the brightness of the backlight which I see 
is lacking in the intel driver.

------------------------------------------------------------------------------
Throughout its 18-year history, RSA Conference consistently attracts the
world's best and brightest in the field, creating opportunities for Conference
attendees to learn about information security's most important issues through
interactions with peers, luminaries and emerging and established companies.
http://p.sf.net/sfu/rsaconf-dev2dev
--

Re: [PATCH] drm: fix regression in fb blank handling

[Zhenyu Wang]

[-- Attachment #1.1: Type: text/plain, Size: 834 bytes --]

On 2010.01.20 13:14:23 +0000, James Simmons wrote:
> 
> It's just adding the backlight api to the intel driver. In fact it gives 
> the user the ablity to control the brightness of the backlight which I see 
> is lacking in the intel driver.

Wait, this regression has nothing to do with backlight control. We already
have backlight drivers for platform specific or ACPI interface, the reason
intel driver doesn't hook up one is because if we provide native interface
which would be much possible to have state conflict with platform or ACPI way. 

This problem is just the wrong DPMS state is used for FB blank request.
Dave, does my patch look sane? This regression isn't in 2.6.32, but only
for .33-rc kernel.

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 420 bytes --]

------------------------------------------------------------------------------
Throughout its 18-year history, RSA Conference consistently attracts the
world's best and brightest in the field, creating opportunities for Conference
attendees to learn about information security's most important issues through
interactions with peers, luminaries and emerging and established companies.
http://p.sf.net/sfu/rsaconf-dev2dev

[-- Attachment #3: Type: text/plain, Size: 161 bytes --]

--
_______________________________________________
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel

Re: [PATCH] drm: fix regression in fb blank handling

[Dave Airlie]

On Thu, 2010-01-21 at 09:09 +0800, Zhenyu Wang wrote:
> On 2010.01.20 13:14:23 +0000, James Simmons wrote:
> > 
> > It's just adding the backlight api to the intel driver. In fact it gives 
> > the user the ablity to control the brightness of the backlight which I see 
> > is lacking in the intel driver.
> 
> Wait, this regression has nothing to do with backlight control. We already
> have backlight drivers for platform specific or ACPI interface, the reason
> intel driver doesn't hook up one is because if we provide native interface
> which would be much possible to have state conflict with platform or ACPI way. 
> 
> This problem is just the wrong DPMS state is used for FB blank request.
> Dave, does my patch look sane? This regression isn't in 2.6.32, but only
> for .33-rc kernel.

We should revisit this for 2.6.34, but for now I've applied Zhenyu's
patch until we can figure out where the problems are, as it breaks a few
too many laptops at this stage for 2.6.33.

Dave.

> 



------------------------------------------------------------------------------
Throughout its 18-year history, RSA Conference consistently attracts the
world's best and brightest in the field, creating opportunities for Conference
attendees to learn about information security's most important issues through
interactions with peers, luminaries and emerging and established companies.
http://p.sf.net/sfu/rsaconf-dev2dev
--

Re: [PATCH] drm: fix regression in fb blank handling

[James Simmons]

> On Thu, 2010-01-21 at 09:09 +0800, Zhenyu Wang wrote:
> > On 2010.01.20 13:14:23 +0000, James Simmons wrote:
> > > 
> > > It's just adding the backlight api to the intel driver. In fact it gives 
> > > the user the ablity to control the brightness of the backlight which I see 
> > > is lacking in the intel driver.
> > 
> > Wait, this regression has nothing to do with backlight control. We already
> > have backlight drivers for platform specific or ACPI interface, the reason
> > intel driver doesn't hook up one is because if we provide native interface
> > which would be much possible to have state conflict with platform or ACPI way. 
> > 
> > This problem is just the wrong DPMS state is used for FB blank request.
> > Dave, does my patch look sane? This regression isn't in 2.6.32, but only
> > for .33-rc kernel.
> 
> We should revisit this for 2.6.34, but for now I've applied Zhenyu's
> patch until we can figure out where the problems are, as it breaks a few
> too many laptops at this stage for 2.6.33.

I agree at this point. I need to get the hardware to test the changes. BTW 
is the GMA 950 supported by the i915 driver? I will pick up a net book in 
the next few days.
 

------------------------------------------------------------------------------
Throughout its 18-year history, RSA Conference consistently attracts the
world's best and brightest in the field, creating opportunities for Conference
attendees to learn about information security's most important issues through
interactions with peers, luminaries and emerging and established companies.
http://p.sf.net/sfu/rsaconf-dev2dev
--

Re: [PATCH] drm: fix regression in fb blank handling

[Zhenyu Wang]

[-- Attachment #1.1: Type: text/plain, Size: 593 bytes --]

On 2010.01.25 18:12:51 +0000, James Simmons wrote:
> 
> > We should revisit this for 2.6.34, but for now I've applied Zhenyu's
> > patch until we can figure out where the problems are, as it breaks a few
> > too many laptops at this stage for 2.6.33.
> 
> I agree at this point. I need to get the hardware to test the changes. BTW 
> is the GMA 950 supported by the i915 driver? I will pick up a net book in 
> the next few days.
>  

Yeah, that should be 945GM.

thanks.

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 408 bytes --]

------------------------------------------------------------------------------
The Planet: dedicated and managed hosting, cloud storage, colocation
Stay online with enterprise data centers and the best network in the business
Choose flexible plans and management services without long-term contracts
Personal 24x7 support from experience hosting pros just a phone call away.
http://p.sf.net/sfu/theplanet-com

[-- Attachment #3: Type: text/plain, Size: 161 bytes --]

--
_______________________________________________
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel