[PATCH] fbdev: workaround for broken X servers

[PATCH] fbdev: workaround for broken X servers

[Benjamin Herrenschmidt]

Hi !

XFree (and X.org) has both always been broken in their conversion of the DPMS
blanking mode when passing to the kernel ioctl FBIOBLANK (in fbdev mode).

This patch makes sure that at least the value passed by userland stays in the
legal range, which has the side effect that an X DPMS POWERDOWN request will
result as an fbdev VESA_POWERDOWN request at the fbdev level now, instead of
an out-of-range value.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

Index: linux-work/drivers/video/fbmem.c
===================================================================
--- linux-work.orig/drivers/video/fbmem.c	2004-10-26 13:15:55.000000000 +1000
+++ linux-work/drivers/video/fbmem.c	2004-11-03 13:10:41.324932256 +1100
@@ -748,6 +748,10 @@
 	u16 *black = NULL;
 	int err = 0;
 	
+	/* Workaround for broken X servers */
+	if (blank > VESA_POWERDOWN)
+		blank = VESA_POWERDOWN;
+
 	if (info->fbops->fb_blank && !info->fbops->fb_blank(blank, info))
 		return 0;
 




-------------------------------------------------------
This SF.Net email is sponsored by:
Sybase ASE Linux Express Edition - download now for FREE
LinuxWorld Reader's Choice Award Winner for best database on Linux.
http://ads.osdn.com/?ad_id=5588&alloc_id=12065&op=click

Re: [PATCH] fbdev: workaround for broken X servers

[Ville Syrjälä]

On Wed, Nov 03, 2004 at 01:13:39PM +1100, Benjamin Herrenschmidt wrote:
> Hi !
> 
> XFree (and X.org) has both always been broken in their conversion of the DPMS
> blanking mode when passing to the kernel ioctl FBIOBLANK (in fbdev mode).
> 
> This patch makes sure that at least the value passed by userland stays in the
> legal range, which has the side effect that an X DPMS POWERDOWN request will
> result as an fbdev VESA_POWERDOWN request at the fbdev level now, instead of
> an out-of-range value.
> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> 
> Index: linux-work/drivers/video/fbmem.c
> ===================================================================
> --- linux-work.orig/drivers/video/fbmem.c	2004-10-26 13:15:55.000000000 +1000
> +++ linux-work/drivers/video/fbmem.c	2004-11-03 13:10:41.324932256 +1100
> @@ -748,6 +748,10 @@
>  	u16 *black = NULL;
>  	int err = 0;
>  	
> +	/* Workaround for broken X servers */
> +	if (blank > VESA_POWERDOWN)
> +		blank = VESA_POWERDOWN;
> +
>  	if (info->fbops->fb_blank && !info->fbops->fb_blank(blank, info))
>  		return 0;
>  

This goes against documented behaviour (see fb.h). Also drivers/char/vt.c 
seems to agree with fb.h, as do at least atyfb and matroxfb. I also used 
the vesa level+1 convention in DirectFB. So AFAICS this patch would 
confuse console power management, XFree86/X.Org, DirectFB and some fb 
drivers.

-- 
Ville Syrjälä
syrjala@sci.fi
http://www.sci.fi/~syrjala/


-------------------------------------------------------
This SF.Net email is sponsored by:
Sybase ASE Linux Express Edition - download now for FREE
LinuxWorld Reader's Choice Award Winner for best database on Linux.
http://ads.osdn.com/?ad_idU88&alloc_id\x12065&op=click

Re: [PATCH] fbdev: workaround for broken X servers

[Antonino A. Daplas]

On Wednesday 03 November 2004 11:26, Ville Syrjälä wrote:
> On Wed, Nov 03, 2004 at 01:13:39PM +1100, Benjamin Herrenschmidt wrote:
> > Hi !
> >
> > +	/* Workaround for broken X servers */
> > +	if (blank > VESA_POWERDOWN)
> > +		blank = VESA_POWERDOWN;
> > +
> >  	if (info->fbops->fb_blank && !info->fbops->fb_blank(blank, info))
> >  		return 0;
>
> This goes against documented behaviour (see fb.h). Also drivers/char/vt.c
> seems to agree with fb.h, as do at least atyfb and matroxfb. I also used
> the vesa level+1 convention in DirectFB. So AFAICS this patch would
> confuse console power management, XFree86/X.Org, DirectFB and some fb
> drivers.

True enough. The convention is if the display is active, then suspend. If
it is suspended, then powerdown.  And if already powered down, then do
nothing.  It does so by by sending a blank flag of VESA_POWERDOWN + 1.

This is the convention used by the vt layer, I don't what is used by user apps.
The above patch won't break console power management as the console calls
fbcon_blank(), not fb_blank().

Tony




-------------------------------------------------------
This SF.Net email is sponsored by:
Sybase ASE Linux Express Edition - download now for FREE
LinuxWorld Reader's Choice Award Winner for best database on Linux.
http://ads.osdn.com/?ad_idU88&alloc_id\x12065&op=click

Re: [PATCH] fbdev: workaround for broken X servers

[Ville Syrjälä]

On Wed, Nov 03, 2004 at 09:09:29PM +0800, Antonino A. Daplas wrote:
> On Wednesday 03 November 2004 11:26, Ville Syrjälä wrote:
> > On Wed, Nov 03, 2004 at 01:13:39PM +1100, Benjamin Herrenschmidt wrote:
> > > Hi !
> > >
> > > +	/* Workaround for broken X servers */
> > > +	if (blank > VESA_POWERDOWN)
> > > +		blank = VESA_POWERDOWN;
> > > +
> > >  	if (info->fbops->fb_blank && !info->fbops->fb_blank(blank, info))
> > >  		return 0;
> >
> > This goes against documented behaviour (see fb.h). Also drivers/char/vt.c
> > seems to agree with fb.h, as do at least atyfb and matroxfb. I also used
> > the vesa level+1 convention in DirectFB. So AFAICS this patch would
> > confuse console power management, XFree86/X.Org, DirectFB and some fb
> > drivers.
> 
> True enough. The convention is if the display is active, then suspend. If
> it is suspended, then powerdown.  And if already powered down, then do
> nothing.  It does so by by sending a blank flag of VESA_POWERDOWN + 1.
> 
> This is the convention used by the vt layer, I don't what is used by 
> user apps.

XFree86/X.Org and DirectFB both use vesa level+1. I'm not sure if there 
are other userspace users.

> The above patch won't break console power management as the console calls
> fbcon_blank(), not fb_blank().

But fbcon_blank() calls fb_blank() does it not?

-- 
Ville Syrjälä
syrjala@sci.fi
http://www.sci.fi/~syrjala/


-------------------------------------------------------
This SF.Net email is sponsored by:
Sybase ASE Linux Express Edition - download now for FREE
LinuxWorld Reader's Choice Award Winner for best database on Linux.
http://ads.osdn.com/?ad_idU88&alloc_id\x12065&op=click

Re: [PATCH] fbdev: workaround for broken X servers

[Antonino A. Daplas]

On Wednesday 03 November 2004 22:01, Ville Syrjälä wrote:
> On Wed, Nov 03, 2004 at 09:09:29PM +0800, Antonino A. Daplas wrote:
> > On Wednesday 03 November 2004 11:26, Ville Syrjälä wrote:
> > > On Wed, Nov 03, 2004 at 01:13:39PM +1100, Benjamin Herrenschmidt wrote:
> > > > Hi !

> > The above patch won't break console power management as the console calls
> > fbcon_blank(), not fb_blank().
>
> But fbcon_blank() calls fb_blank() does it not?

I just changed it so it doesn't.  It calls info->fbops->fb_blank().

Tony




-------------------------------------------------------
This SF.Net email is sponsored by:
Sybase ASE Linux Express Edition - download now for FREE
LinuxWorld Reader's Choice Award Winner for best database on Linux.
http://ads.osdn.com/?ad_idU88&alloc_id\x12065&op=click

Re: [PATCH] fbdev: workaround for broken X servers

[Benjamin Herrenschmidt]

> > Index: linux-work/drivers/video/fbmem.c
> > ===================================================================
> > --- linux-work.orig/drivers/video/fbmem.c	2004-10-26 13:15:55.000000000 +1000
> > +++ linux-work/drivers/video/fbmem.c	2004-11-03 13:10:41.324932256 +1100
> > @@ -748,6 +748,10 @@
> >  	u16 *black = NULL;
> >  	int err = 0;
> >  	
> > +	/* Workaround for broken X servers */
> > +	if (blank > VESA_POWERDOWN)
> > +		blank = VESA_POWERDOWN;
> > +
> >  	if (info->fbops->fb_blank && !info->fbops->fb_blank(blank, info))
> >  		return 0;
> >  
> 
> This goes against documented behaviour (see fb.h). Also drivers/char/vt.c 
> seems to agree with fb.h, as do at least atyfb and matroxfb. I also used 
> the vesa level+1 convention in DirectFB. So AFAICS this patch would 
> confuse console power management, XFree86/X.Org, DirectFB and some fb 
> drivers.

Ok, can we agree here on what has to be done ? X is hard-coding numbers
that don't seem to be make any sense vs. our constants. In radeonfb, I
used switch case on constants, not +1, so there is something I'm not
getting. Besides, radeon always try to power down TFT panels when
blank != 0, maybe I should improve that ... What is this +1 thing, where
does it comes from ?

Ben.





-------------------------------------------------------
This SF.Net email is sponsored by:
Sybase ASE Linux Express Edition - download now for FREE
LinuxWorld Reader's Choice Award Winner for best database on Linux.
http://ads.osdn.com/?ad_id=5588&alloc_id=12065&op=click

Re: [PATCH] fbdev: workaround for broken X servers

[Geert Uytterhoeven]

On Fri, 5 Nov 2004, Benjamin Herrenschmidt wrote:
> > > Index: linux-work/drivers/video/fbmem.c
> > > ===================================================================
> > > --- linux-work.orig/drivers/video/fbmem.c	2004-10-26 13:15:55.000000000 +1000
> > > +++ linux-work/drivers/video/fbmem.c	2004-11-03 13:10:41.324932256 +1100
> > > @@ -748,6 +748,10 @@
> > >  	u16 *black = NULL;
> > >  	int err = 0;
> > >  	
> > > +	/* Workaround for broken X servers */
> > > +	if (blank > VESA_POWERDOWN)
> > > +		blank = VESA_POWERDOWN;
> > > +
> > >  	if (info->fbops->fb_blank && !info->fbops->fb_blank(blank, info))
> > >  		return 0;
> > >  
> > 
> > This goes against documented behaviour (see fb.h). Also drivers/char/vt.c 
> > seems to agree with fb.h, as do at least atyfb and matroxfb. I also used 
> > the vesa level+1 convention in DirectFB. So AFAICS this patch would 
> > confuse console power management, XFree86/X.Org, DirectFB and some fb 
> > drivers.
> 
> Ok, can we agree here on what has to be done ? X is hard-coding numbers
> that don't seem to be make any sense vs. our constants. In radeonfb, I
> used switch case on constants, not +1, so there is something I'm not
> getting. Besides, radeon always try to power down TFT panels when
> blank != 0, maybe I should improve that ... What is this +1 thing, where
> does it comes from ?

Hmm, I don't remember where this +1 comes from...

But I find all of this a bit strange: IIRC it was PaulM who asked me to add the
blank ioctl to fb.h, after he added support for blanking to X. So it would
surprise me a bit if it was wrong :-)

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds


-------------------------------------------------------
This SF.Net email is sponsored by:
Sybase ASE Linux Express Edition - download now for FREE
LinuxWorld Reader's Choice Award Winner for best database on Linux.
http://ads.osdn.com/?ad_id=5588&alloc_id=12065&op=click

Re: [PATCH] fbdev: workaround for broken X servers

[Ville Syrjälä]

On Fri, Nov 05, 2004 at 04:49:36PM +1100, Benjamin Herrenschmidt wrote:
> 
> > > Index: linux-work/drivers/video/fbmem.c
> > > ===================================================================
> > > --- linux-work.orig/drivers/video/fbmem.c	2004-10-26 13:15:55.000000000 +1000
> > > +++ linux-work/drivers/video/fbmem.c	2004-11-03 13:10:41.324932256 +1100
> > > @@ -748,6 +748,10 @@
> > >  	u16 *black = NULL;
> > >  	int err = 0;
> > >  	
> > > +	/* Workaround for broken X servers */
> > > +	if (blank > VESA_POWERDOWN)
> > > +		blank = VESA_POWERDOWN;
> > > +
> > >  	if (info->fbops->fb_blank && !info->fbops->fb_blank(blank, info))
> > >  		return 0;
> > >  
> > 
> > This goes against documented behaviour (see fb.h). Also drivers/char/vt.c 
> > seems to agree with fb.h, as do at least atyfb and matroxfb. I also used 
> > the vesa level+1 convention in DirectFB. So AFAICS this patch would 
> > confuse console power management, XFree86/X.Org, DirectFB and some fb 
> > drivers.
> 
> Ok, can we agree here on what has to be done ? X is hard-coding numbers
> that don't seem to be make any sense vs. our constants. In radeonfb, I
> used switch case on constants, not +1, so there is something I'm not
> getting. Besides, radeon always try to power down TFT panels when
> blank != 0, maybe I should improve that ... What is this +1 thing, where
> does it comes from ?

Maybe it's there because someone wanted to blank the display without 
disabling sync signals.

I suggest a new set of constants that cover all legal values. Here's the 
first drawft.

--- linux-orig/include/linux/fb.h	2004-11-05 12:25:14.087807848 +0200
+++ linux/include/linux/fb.h	2004-11-05 12:28:02.734169704 +0200
@@ -27,7 +27,7 @@
 /* #define FBIOSWITCH_MONIBIT	0x460E */
 #define FBIOGET_CON2FBMAP	0x460F
 #define FBIOPUT_CON2FBMAP	0x4610
-#define FBIOBLANK		0x4611		/* arg: 0 or vesa level + 1 */
+#define FBIOBLANK		0x4611		/* see FB_BLANK_* */
 #define FBIOGET_VBLANK		_IOR('F', 0x12, struct fb_vblank)
 #define FBIO_ALLOC              0x4613
 #define FBIO_FREE               0x4614
@@ -258,6 +258,12 @@
 #define VESA_HSYNC_SUSPEND      2
 #define VESA_POWERDOWN          3
 
+#define FB_BLANK_NO_BLANKING	0
+#define FB_BLANK_BLANKING	1
+#define FB_BLANK_VSYNC_SUSPEND	2
+#define FB_BLANK_HSYNC_SUSPEND	3
+#define FB_BLANK_POWERDOWN	4
+
 #define FB_VBLANK_VBLANKING	0x001	/* currently in a vertical blank */
 #define FB_VBLANK_HBLANKING	0x002	/* currently in a horizontal blank */
 #define FB_VBLANK_HAVE_VBLANK	0x004	/* vertical blanks can be detected */

-- 
Ville Syrjälä
syrjala@sci.fi
http://www.sci.fi/~syrjala/


-------------------------------------------------------
This SF.Net email is sponsored by:
Sybase ASE Linux Express Edition - download now for FREE
LinuxWorld Reader's Choice Award Winner for best database on Linux.
http://ads.osdn.com/?ad_idU88&alloc_id\x12065&op=click

Re: [PATCH] fbdev: workaround for broken X servers

[Geert Uytterhoeven]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2090 bytes --]

On Fri, 5 Nov 2004, Ville [iso-8859-1] Syrjälä wrote:
> On Fri, Nov 05, 2004 at 04:49:36PM +1100, Benjamin Herrenschmidt wrote:
> > Ok, can we agree here on what has to be done ? X is hard-coding numbers
> > that don't seem to be make any sense vs. our constants. In radeonfb, I
> > used switch case on constants, not +1, so there is something I'm not
> > getting. Besides, radeon always try to power down TFT panels when
> > blank != 0, maybe I should improve that ... What is this +1 thing, where
> > does it comes from ?
> 
> Maybe it's there because someone wanted to blank the display without 
> disabling sync signals.

Yes of course! (/me wasn't completely awake yet)

> I suggest a new set of constants that cover all legal values. Here's the 
> first drawft.
> 
> --- linux-orig/include/linux/fb.h	2004-11-05 12:25:14.087807848 +0200
> +++ linux/include/linux/fb.h	2004-11-05 12:28:02.734169704 +0200
> @@ -27,7 +27,7 @@
>  /* #define FBIOSWITCH_MONIBIT	0x460E */
>  #define FBIOGET_CON2FBMAP	0x460F
>  #define FBIOPUT_CON2FBMAP	0x4610
> -#define FBIOBLANK		0x4611		/* arg: 0 or vesa level + 1 */
> +#define FBIOBLANK		0x4611		/* see FB_BLANK_* */
>  #define FBIOGET_VBLANK		_IOR('F', 0x12, struct fb_vblank)
>  #define FBIO_ALLOC              0x4613
>  #define FBIO_FREE               0x4614
> @@ -258,6 +258,12 @@
>  #define VESA_HSYNC_SUSPEND      2
>  #define VESA_POWERDOWN          3
>  
> +#define FB_BLANK_NO_BLANKING	0
> +#define FB_BLANK_BLANKING	1
> +#define FB_BLANK_VSYNC_SUSPEND	2
> +#define FB_BLANK_HSYNC_SUSPEND	3
> +#define FB_BLANK_POWERDOWN	4
> +

Yep, these nicely cover the current intended behavior.

But every existing application that uses (shiver) the kernel headers will break
after this change...

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

Re: [PATCH] fbdev: workaround for broken X servers

[Ville Syrjälä]

On Fri, Nov 05, 2004 at 10:56:04AM +0100, Geert Uytterhoeven wrote:
> On Fri, 5 Nov 2004, Ville [iso-8859-1] Syrjälä wrote:
> > On Fri, Nov 05, 2004 at 04:49:36PM +1100, Benjamin Herrenschmidt wrote:
> > > Ok, can we agree here on what has to be done ? X is hard-coding numbers
> > > that don't seem to be make any sense vs. our constants. In radeonfb, I
> > > used switch case on constants, not +1, so there is something I'm not
> > > getting. Besides, radeon always try to power down TFT panels when
> > > blank != 0, maybe I should improve that ... What is this +1 thing, where
> > > does it comes from ?
> > 
> > Maybe it's there because someone wanted to blank the display without 
> > disabling sync signals.
> 
> Yes of course! (/me wasn't completely awake yet)
> 
> > I suggest a new set of constants that cover all legal values. Here's the 
> > first drawft.
> > 
> > --- linux-orig/include/linux/fb.h	2004-11-05 12:25:14.087807848 +0200
> > +++ linux/include/linux/fb.h	2004-11-05 12:28:02.734169704 +0200
> > @@ -27,7 +27,7 @@
> >  /* #define FBIOSWITCH_MONIBIT	0x460E */
> >  #define FBIOGET_CON2FBMAP	0x460F
> >  #define FBIOPUT_CON2FBMAP	0x4610
> > -#define FBIOBLANK		0x4611		/* arg: 0 or vesa level + 1 */
> > +#define FBIOBLANK		0x4611		/* see FB_BLANK_* */
> >  #define FBIOGET_VBLANK		_IOR('F', 0x12, struct fb_vblank)
> >  #define FBIO_ALLOC              0x4613
> >  #define FBIO_FREE               0x4614
> > @@ -258,6 +258,12 @@
> >  #define VESA_HSYNC_SUSPEND      2
> >  #define VESA_POWERDOWN          3
> >  
> > +#define FB_BLANK_NO_BLANKING	0
> > +#define FB_BLANK_BLANKING	1
> > +#define FB_BLANK_VSYNC_SUSPEND	2
> > +#define FB_BLANK_HSYNC_SUSPEND	3
> > +#define FB_BLANK_POWERDOWN	4
> > +
> 
> Yep, these nicely cover the current intended behavior.
> 
> But every existing application that uses (shiver) the kernel headers will break
> after this change...

If the VESA constants are left intact nothing should change. BTW neither 
XFree86/X.Org nor DirectFB use them so chances are nothing else does.

-- 
Ville Syrjälä
syrjala@sci.fi
http://www.sci.fi/~syrjala/


-------------------------------------------------------
This SF.Net email is sponsored by:
Sybase ASE Linux Express Edition - download now for FREE
LinuxWorld Reader's Choice Award Winner for best database on Linux.
http://ads.osdn.com/?ad_idU88&alloc_id\x12065&op=click

Re: [PATCH] fbdev: workaround for broken X servers

[Benjamin Herrenschmidt]

> Maybe it's there because someone wanted to blank the display without 
> disabling sync signals.

Is there a point doing so ?

Ben.




-------------------------------------------------------
This SF.Net email is sponsored by:
Sybase ASE Linux Express Edition - download now for FREE
LinuxWorld Reader's Choice Award Winner for best database on Linux.
http://ads.osdn.com/?ad_id=5588&alloc_id=12065&op=click

Re: [PATCH] fbdev: workaround for broken X servers

[Benjamin Herrenschmidt]

> But every existing application that uses (shiver) the kernel headers will break
> after this change...

Geert, can you explain the whole story please ?

Ben.




-------------------------------------------------------
This SF.Net email is sponsored by:
Sybase ASE Linux Express Edition - download now for FREE
LinuxWorld Reader's Choice Award Winner for best database on Linux.
http://ads.osdn.com/?ad_id=5588&alloc_id=12065&op=click

Re: [PATCH] fbdev: workaround for broken X servers

[Geert Uytterhoeven]

On Fri, 5 Nov 2004, Benjamin Herrenschmidt wrote:
> > But every existing application that uses (shiver) the kernel headers will break
> > after this change...
> 
> Geert, can you explain the whole story please ?

The story about the +1 or the story about the breakage?

The Story About The +1
----------------------
Since the VESA levels do not provide a way to blank (`make it black') the
screen, the +1 is introduced. Hence 0 means unblank, 1 means black screen, 2
means lowest power save level, and so on...

The Story About The Breakage
----------------------------
Every application that passes VESA_*+1 will break when recompiled, since (most
of) the VESA_* values are incremented by 1 by the patch.

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds


-------------------------------------------------------
This SF.Net email is sponsored by:
Sybase ASE Linux Express Edition - download now for FREE
LinuxWorld Reader's Choice Award Winner for best database on Linux.
http://ads.osdn.com/?ad_id=5588&alloc_id=12065&op=click

Re: [PATCH] fbdev: workaround for broken X servers

[Geert Uytterhoeven]

On Fri, 5 Nov 2004, Benjamin Herrenschmidt wrote:
> > Maybe it's there because someone wanted to blank the display without 
> > disabling sync signals.
> 
> Is there a point doing so ?

Yes. Usually you start by making the screen black, and after a while you enter
deeper power save modes.

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds


-------------------------------------------------------
This SF.Net email is sponsored by:
Sybase ASE Linux Express Edition - download now for FREE
LinuxWorld Reader's Choice Award Winner for best database on Linux.
http://ads.osdn.com/?ad_id=5588&alloc_id=12065&op=click

Re: [PATCH] fbdev: workaround for broken X servers

[Antonino A. Daplas]

On Friday 05 November 2004 20:15, Geert Uytterhoeven wrote:
> On Fri, 5 Nov 2004, Benjamin Herrenschmidt wrote:
> > > But every existing application that uses (shiver) the kernel headers
> > > will break after this change...
> >
> > Geert, can you explain the whole story please ?
>
> The story about the +1 or the story about the breakage?
>
> The Story About The +1
> ----------------------
> Since the VESA levels do not provide a way to blank (`make it black') the
> screen, the +1 is introduced. Hence 0 means unblank, 1 means black screen,
> 2 means lowest power save level, and so on...

And make it black is usually done in software (by memsetting the screen with
the black color).  

So why not this:  In fbcon_blank(), if the blank parameter is:

0 - unblank the screen, and call info->fbops->fb_blank(VESA_NO_BLANKING), 
     then return a nonzero value to force a redraw.
1,2,3,4 - clear the screen with all black
2,3,4 - call info->fbops->fb_blank(blank - 1);

Similarly, we can do the same thing in fbmem.c:fb_blank(), but without the software
blanking.

Drivers will just need to follow the VESA_* defines in fb.h (which happens to be also
defined in console.h)

Tony 




-------------------------------------------------------
This SF.Net email is sponsored by:
Sybase ASE Linux Express Edition - download now for FREE
LinuxWorld Reader's Choice Award Winner for best database on Linux.
http://ads.osdn.com/?ad_id=5588&alloc_id=12065&op=click

Re: [PATCH] fbdev: workaround for broken X servers

[Geert Uytterhoeven]

On Fri, 5 Nov 2004, Antonino A. Daplas wrote:
> On Friday 05 November 2004 20:15, Geert Uytterhoeven wrote:
> > On Fri, 5 Nov 2004, Benjamin Herrenschmidt wrote:
> > > > But every existing application that uses (shiver) the kernel headers
> > > > will break after this change...
> > > Geert, can you explain the whole story please ?
> >
> > The story about the +1 or the story about the breakage?
> >
> > The Story About The +1
> > ----------------------
> > Since the VESA levels do not provide a way to blank (`make it black') the
> > screen, the +1 is introduced. Hence 0 means unblank, 1 means black screen,
> > 2 means lowest power save level, and so on...
> 
> And make it black is usually done in software (by memsetting the screen with
> the black color).  

Or by setting all color palette registers to black, if available, since it's
cheaper...

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds


-------------------------------------------------------
This SF.Net email is sponsored by:
Sybase ASE Linux Express Edition - download now for FREE
LinuxWorld Reader's Choice Award Winner for best database on Linux.
http://ads.osdn.com/?ad_id=5588&alloc_id=12065&op=click

Re: [PATCH] fbdev: workaround for broken X servers

[Antonino A. Daplas]

On Friday 05 November 2004 20:58, Geert Uytterhoeven wrote:
> On Fri, 5 Nov 2004, Antonino A. Daplas wrote:
> > On Friday 05 November 2004 20:15, Geert Uytterhoeven wrote:
> > > On Fri, 5 Nov 2004, Benjamin Herrenschmidt wrote:
> > > > > But every existing application that uses (shiver) the kernel
> > > > > headers will break after this change...
> > > >
> > > > Geert, can you explain the whole story please ?
> > >
> > > The story about the +1 or the story about the breakage?
> > >
> > > The Story About The +1
> > > ----------------------
> > > Since the VESA levels do not provide a way to blank (`make it black')
> > > the screen, the +1 is introduced. Hence 0 means unblank, 1 means black
> > > screen, 2 means lowest power save level, and so on...
> >
> > And make it black is usually done in software (by memsetting the screen
> > with the black color).
>
> Or by setting all color palette registers to black, if available, since
> it's cheaper...
>

Ok. So, if pseudocolor/directcolor, set palette to all black, otherwise memset
the screen.

Tony




-------------------------------------------------------
This SF.Net email is sponsored by:
Sybase ASE Linux Express Edition - download now for FREE
LinuxWorld Reader's Choice Award Winner for best database on Linux.
http://ads.osdn.com/?ad_id=5588&alloc_id=12065&op=click

Re: [PATCH] fbdev: workaround for broken X servers

[Maciej W. Rozycki]

On Fri, 5 Nov 2004, Antonino A. Daplas wrote:

> > > And make it black is usually done in software (by memsetting the screen
> > > with the black color).
> >
> > Or by setting all color palette registers to black, if available, since
> > it's cheaper...
> >
> 
> Ok. So, if pseudocolor/directcolor, set palette to all black, otherwise memset
> the screen.

 Still not there -- some RAMDACs can be asked to do blanking themselves.  
And there are these interesting implementations like the Bt463 that have a
palette even though using the truecolor mode.

  Maciej


-------------------------------------------------------
This SF.Net email is sponsored by:
Sybase ASE Linux Express Edition - download now for FREE
LinuxWorld Reader's Choice Award Winner for best database on Linux.
http://ads.osdn.com/?ad_id=5588&alloc_id=12065&op=click

Re: [PATCH] fbdev: workaround for broken X servers

[Geert Uytterhoeven]

On Fri, 5 Nov 2004, Maciej W. Rozycki wrote:
> On Fri, 5 Nov 2004, Antonino A. Daplas wrote:
> > > > And make it black is usually done in software (by memsetting the screen
> > > > with the black color).
> > >
> > > Or by setting all color palette registers to black, if available, since
> > > it's cheaper...
> > 
> > Ok. So, if pseudocolor/directcolor, set palette to all black, otherwise memset
> > the screen.
> 
>  Still not there -- some RAMDACs can be asked to do blanking themselves.  

In that case fb_info->fb_ops->fb_blank() should not be NULL and take care of
blanking, so no fallback to software blanking is necessary.

> And there are these interesting implementations like the Bt463 that have a
> palette even though using the truecolor mode.

Doesn't that one do directcolor instead of truecolor?

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds


-------------------------------------------------------
This SF.Net email is sponsored by:
Sybase ASE Linux Express Edition - download now for FREE
LinuxWorld Reader's Choice Award Winner for best database on Linux.
http://ads.osdn.com/?ad_id=5588&alloc_id=12065&op=click

Re: [PATCH] fbdev: workaround for broken X servers

[Ville Syrjälä]

On Fri, Nov 05, 2004 at 01:15:35PM +0100, Geert Uytterhoeven wrote:
> On Fri, 5 Nov 2004, Benjamin Herrenschmidt wrote:
> > > But every existing application that uses (shiver) the kernel headers will break
> > > after this change...
> > 
> > Geert, can you explain the whole story please ?
> 
> The story about the +1 or the story about the breakage?
> 
> The Story About The +1
> ----------------------
> Since the VESA levels do not provide a way to blank (`make it black') the
> screen, the +1 is introduced. Hence 0 means unblank, 1 means black screen, 2
> means lowest power save level, and so on...
> 
> The Story About The Breakage
> ----------------------------
> Every application that passes VESA_*+1 will break when recompiled, since (most
> of) the VESA_* values are incremented by 1 by the patch.

My proposed patch doesn't touch the VESA_* values. It just adds new 
constants which match the VESA_*+1 convention. That way old apps will 
continue to work and developers don't have to guess what values are 
actully legal.

-- 
Ville Syrjälä
syrjala@sci.fi
http://www.sci.fi/~syrjala/


-------------------------------------------------------
This SF.Net email is sponsored by:
Sybase ASE Linux Express Edition - download now for FREE
LinuxWorld Reader's Choice Award Winner for best database on Linux.
http://ads.osdn.com/?ad_idU88&alloc_id\x12065&op=click

Re: [PATCH] fbdev: workaround for broken X servers

[Geert Uytterhoeven]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1599 bytes --]

On Fri, 5 Nov 2004, Ville [iso-8859-1] Syrjälä wrote:
> On Fri, Nov 05, 2004 at 01:15:35PM +0100, Geert Uytterhoeven wrote:
> > On Fri, 5 Nov 2004, Benjamin Herrenschmidt wrote:
> > > > But every existing application that uses (shiver) the kernel headers will break
> > > > after this change...
> > > 
> > > Geert, can you explain the whole story please ?
> > 
> > The story about the +1 or the story about the breakage?
> > 
> > The Story About The +1
> > ----------------------
> > Since the VESA levels do not provide a way to blank (`make it black') the
> > screen, the +1 is introduced. Hence 0 means unblank, 1 means black screen, 2
> > means lowest power save level, and so on...
> > 
> > The Story About The Breakage
> > ----------------------------
> > Every application that passes VESA_*+1 will break when recompiled, since (most
> > of) the VESA_* values are incremented by 1 by the patch.
> 
> My proposed patch doesn't touch the VESA_* values. It just adds new 
> constants which match the VESA_*+1 convention. That way old apps will 
> continue to work and developers don't have to guess what values are 
> actully legal.

<embarassed>
Oops, you're right.

Sorry, I misread you patch to modify the values.
<embarassed>

So yes, it's OK!!

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

Re: [PATCH] fbdev: workaround for broken X servers

[Maciej W. Rozycki]

On Fri, 5 Nov 2004, Geert Uytterhoeven wrote:

> > And there are these interesting implementations like the Bt463 that have a
> > palette even though using the truecolor mode.
> 
> Doesn't that one do directcolor instead of truecolor?

 Hmm, I guess so.

  Maciej


-------------------------------------------------------
This SF.Net email is sponsored by:
Sybase ASE Linux Express Edition - download now for FREE
LinuxWorld Reader's Choice Award Winner for best database on Linux.
http://ads.osdn.com/?ad_id=5588&alloc_id=12065&op=click

Re: [PATCH] fbdev: workaround for broken X servers

[Benjamin Herrenschmidt]

On Fri, 2004-11-05 at 13:15 +0100, Geert Uytterhoeven wrote:
> On Fri, 5 Nov 2004, Benjamin Herrenschmidt wrote:
> > > But every existing application that uses (shiver) the kernel headers will break
> > > after this change...
> > 
> > Geert, can you explain the whole story please ?
> 
> The story about the +1 or the story about the breakage?
> 
> The Story About The +1
> ----------------------
> Since the VESA levels do not provide a way to blank (`make it black') the
> screen, the +1 is introduced. Hence 0 means unblank, 1 means black screen, 2
> means lowest power save level, and so on...

So why don't we have a nice set of #define's or an enum at least
describing those ? :)
> 
> The Story About The Breakage
> ----------------------------
> Every application that passes VESA_*+1 will break when recompiled, since (most
> of) the VESA_* values are incremented by 1 by the patch.

What patch ? Mine ? It doesn't increment the VESA values, it just clamps
the max. But then, everything seem to be totally inconsistent. So can we
instead define a  set of FB_BLANK_**** values to use and have fbcon
convert VESA->FB_BLANK and FBIOBLANK pass FB_BLANK_* as-is, thus the
drivers would switch/case on those and no more +1 games ?

> Gr{oetje,eeting}s,
> 
> 						Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> 							    -- Linus Torvalds
-- 
Benjamin Herrenschmidt <benh@kernel.crashing.org>



-------------------------------------------------------
This SF.Net email is sponsored by:
Sybase ASE Linux Express Edition - download now for FREE
LinuxWorld Reader's Choice Award Winner for best database on Linux.
http://ads.osdn.com/?ad_id=5588&alloc_id=12065&op=click

Re: [PATCH] fbdev: workaround for broken X servers

[Benjamin Herrenschmidt]

On Fri, 2004-11-05 at 20:55 +0800, Antonino A. Daplas wrote:

> And make it black is usually done in software (by memsetting the screen with
> the black color).  
> 
> So why not this:  In fbcon_blank(), if the blank parameter is:
> 
> 0 - unblank the screen, and call info->fbops->fb_blank(VESA_NO_BLANKING), 
>      then return a nonzero value to force a redraw.
> 1,2,3,4 - clear the screen with all black
> 2,3,4 - call info->fbops->fb_blank(blank - 1);
> 
> Similarly, we can do the same thing in fbmem.c:fb_blank(), but without the software
> blanking.
> 
> Drivers will just need to follow the VESA_* defines in fb.h (which happens to be also
> defined in console.h)

Ah... so radeonfb is correct then, and fb_blank() is not ? X is
definitely passing +1 values it seems.

Ben.




-------------------------------------------------------
This SF.Net email is sponsored by:
Sybase ASE Linux Express Edition - download now for FREE
LinuxWorld Reader's Choice Award Winner for best database on Linux.
http://ads.osdn.com/?ad_id=5588&alloc_id=12065&op=click

Re: [PATCH] fbdev: workaround for broken X servers

[Antonino A. Daplas]

On Saturday 06 November 2004 08:38, Benjamin Herrenschmidt wrote:
> On Fri, 2004-11-05 at 20:55 +0800, Antonino A. Daplas wrote:
> > And make it black is usually done in software (by memsetting the screen
> > with the black color).
> >
> > So why not this:  In fbcon_blank(), if the blank parameter is:
> >
> > 0 - unblank the screen, and call info->fbops->fb_blank(VESA_NO_BLANKING),
> >      then return a nonzero value to force a redraw.
> > 1,2,3,4 - clear the screen with all black
> > 2,3,4 - call info->fbops->fb_blank(blank - 1);
> >
> > Similarly, we can do the same thing in fbmem.c:fb_blank(), but without
> > the software blanking.
> >
> > Drivers will just need to follow the VESA_* defines in fb.h (which
> > happens to be also defined in console.h)
>
> Ah... so radeonfb is correct then, and fb_blank() is not ? X is
> definitely passing +1 values it seems.
>

Well, depends on what approach is adopted:

Most drivers do this:
if (blank)
	switch(blank-1) {
	...
	}

Which means fb_blank() and fbcon_blank() should pass the 
blank parameter to info->fbops->fb_blank() unchanged.

If we adopt the approach of passing (blank -1) to 
info->fbops->fb_blank(), then drivers simply do a:

switch(blank) {
...
}

Currently, fbcon_blank() does the first method (pass blank unchanged),
which means radeonfb is wrong. Adopting the second method will make
radeonfb do the right thing, but will require changes to a lot of drivers.

So, what will it be then?

Tony




-------------------------------------------------------
This SF.Net email is sponsored by:
Sybase ASE Linux Express Edition - download now for FREE
LinuxWorld Reader's Choice Award Winner for best database on Linux.
http://ads.osdn.com/?ad_id=5588&alloc_id=12065&op=click

Re: [PATCH] fbdev: workaround for broken X servers

[Antonino A. Daplas]

On Saturday 06 November 2004 08:37, Benjamin Herrenschmidt wrote:
> On Fri, 2004-11-05 at 13:15 +0100, Geert Uytterhoeven wrote:
> > On Fri, 5 Nov 2004, Benjamin Herrenschmidt wrote:
> > > > But every existing application that uses (shiver) the kernel headers
> > > > will break after this change...
> > >
> > > Geert, can you explain the whole story please ?
> >
> > The story about the +1 or the story about the breakage?
> >
> > The Story About The +1
> > ----------------------
> > Since the VESA levels do not provide a way to blank (`make it black') the
> > screen, the +1 is introduced. Hence 0 means unblank, 1 means black
> > screen, 2 means lowest power save level, and so on...
>
> So why don't we have a nice set of #define's or an enum at least
> describing those ? :)
>
> > The Story About The Breakage
> > ----------------------------
> > Every application that passes VESA_*+1 will break when recompiled, since
> > (most of) the VESA_* values are incremented by 1 by the patch.
>
> What patch ? Mine ? It doesn't increment the VESA values, it just clamps
> the max. But then, everything seem to be totally inconsistent. So can we
> instead define a  set of FB_BLANK_**** values to use and have fbcon
> convert VESA->FB_BLANK and FBIOBLANK pass FB_BLANK_* as-is, thus the
> drivers would switch/case on those and no more +1 games ?
>

Currently, yes, the patch will break many drivers:

VESA_POWERDOWN + 1 is interpreted by most drivers as
VESA_POWERDOWN. If clamped, it will be misinterpreted as
VESA_HSYNC_SUSPEND.

Tony




-------------------------------------------------------
This SF.Net email is sponsored by:
Sybase ASE Linux Express Edition - download now for FREE
LinuxWorld Reader's Choice Award Winner for best database on Linux.
http://ads.osdn.com/?ad_id=5588&alloc_id=12065&op=click

Re: [PATCH] fbdev: workaround for broken X servers

[Benjamin Herrenschmidt]

On Sat, 2004-11-06 at 10:02 +0800, Antonino A. Daplas wrote:

> Currently, fbcon_blank() does the first method (pass blank unchanged),
> which means radeonfb is wrong. Adopting the second method will make
> radeonfb do the right thing, but will require changes to a lot of drivers.
> 
> So, what will it be then?

I'd rather define FB_BLANK_* constants and convert all drivers... the
conversion is trivial.

Ben.




-------------------------------------------------------
This SF.Net email is sponsored by:
Sybase ASE Linux Express Edition - download now for FREE
LinuxWorld Reader's Choice Award Winner for best database on Linux.
http://ads.osdn.com/?ad_id=5588&alloc_id=12065&op=click

Re: [PATCH] fbdev: workaround for broken X servers

[Antonino A. Daplas]

On Saturday 06 November 2004 11:06, Benjamin Herrenschmidt wrote:
> On Sat, 2004-11-06 at 10:02 +0800, Antonino A. Daplas wrote:
> > Currently, fbcon_blank() does the first method (pass blank unchanged),
> > which means radeonfb is wrong. Adopting the second method will make
> > radeonfb do the right thing, but will require changes to a lot of
> > drivers.
> >
> > So, what will it be then?
>
> I'd rather define FB_BLANK_* constants and convert all drivers... the
> conversion is trivial.

Ok, I also vote for implementing the new FB_BLANK_* constants. I'll submit a
preliminary patch in a day or so, if nobody disagrees.

Tony




-------------------------------------------------------
This SF.Net email is sponsored by:
Sybase ASE Linux Express Edition - download now for FREE
LinuxWorld Reader's Choice Award Winner for best database on Linux.
http://ads.osdn.com/?ad_id=5588&alloc_id=12065&op=click