Do set var even if no fb_check_var() provided.

Hi,

The behavior of FBIOSET_VSCREENINFO seems to be weired on FB which does not
provides fbops->fb_check_var.
It doesn't set anything, but gets info->var instead, and returns no error. It is just same as FBIOGET_VSCREENINFO does.

IMHO, this should be one of the following candidates,
1. set var without check.
2. do nothing but return error (setting var is not supported).
or
3. it's a bug (fb_check_var should always be provided).

The patch at the bottom implements "1".

Because I don't know API specification, nor the history of the code, I would like people who knows well to discuss this.

Regards,
/yoshii
---
drivers/video/fbmem.c:fb_set_var()
Do set_var even if fbops->fb_check_var == NULL. Possible API change of ioctl(FBIOSET_VSCREENINFO).

index 776f7fc..6fc2ba6 100644
@@ -939,16 +939,12 @@ fb_set_var(struct fb_info *info, struct fb_var_screeninfo *var)
 	    memcmp(&info->var, var, sizeof(struct fb_var_screeninfo))) {
 		u32 activate = var->activate;
 
-		if (!info->fbops->fb_check_var) {
-			*var = info->var;
-			goto done;
+		if (info->fbops->fb_check_var) {
+			ret = info->fbops->fb_check_var(var, info);
+			if (ret)
+				goto done;
 		}
 
-		ret = info->fbops->fb_check_var(var, info);
-
-		if (ret)
-			goto done;
-
 		if ((var->activate & FB_ACTIVATE_MASK) == FB_ACTIVATE_NOW) {
 			struct fb_videomode mode;
 

1.5.4.5


-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

If the driver doesn't provide a fb_check_var(), it means it cannot change video mode. Hence this rules out #1.

#2 is not acceptable, as it will break existing applications. It's also incorrect, as FBIOPUT_VSCREENINFO should succeed for supported modes. For unsupported modes, the mode should be rounded up to a supported mode, if possible. In the case of drivers that support one fixed mode only, this rounding up is relaxed to `rounding' to the sole supported mode.

#3 is also wrong, as fb_check_var() has been deliberately made optional to simplify drivers that support one fixed mode only.

Conclusion: nothing should be changed?

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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 the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK & win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

I'm not sure.
On parisc we just stumbled over exactly this problem where I think Takashi's proposal "1." is probably the right solution.

Please see my Xorg bugzilla entry:
https://bugs.freedesktop.org/show_bug.cgi?id=17153
and this problem description/thread ("X won't start with VisEG and 2.6.22.19"):
http://thread.gmane.org/gmane.linux.ports.parisc/564/focus=592

Helge

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK & win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

I believe (bugs.freedesktop.org is currently down) that's currently waiting for information from you. Namely, have you tried not specifying a Modes line in the xorg.conf SubSection "Display"? I think the Xorg fbdev driver should come up with the currently active mode in that case. If that doesn't work, please attach the corresponding Xorg.0.log to the bug report once bugzilla is back online.

Earthling Michel Dänzer           |          http://tungstengraphics.com
Libre software enthusiast         |          Debian, X and DRI developer


-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Linux-fbdev-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-fbdev-devel

Bugzilla (https://bugs.freedesktop.org/show_bug.cgi?id=17153 ) is available again, and I've uploaded the xorg.conf file, as well as the Xorg.0.log log file as requested.

Your idea about testing a few mode lines is normally a good idea. On a PC I would do exactly this.

But on a pure framebuffer driver it's only necessary to know (and to check), that it runs (for example) at 1024x768 pixels with 8bpp. The monitor sync values shouldn't really matter and that is the problem in this thread. X should not try to force a specific monitor frequency. It won't suceed anyway. It's for some cards just not possible to switch monitor frequencies or to know the current ones (and this is why those drivers don't implement fb_check_var())...

Helge

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK & win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Linux-fbdev-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/linux-fbdev-devel

My idea is quite the opposite - to try *without* even the Modes line in SubSection "Display". Can you try that please?

Earthling Michel Dänzer           |          http://tungstengraphics.com
Libre software enthusiast         |          Debian, X and DRI developer


-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Linux-fbdev-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-fbdev-devel

Doesn't work either.

a) If I drop the "monitor" line in the Screen section it chooses a "default monitor" and fails:
(==) Using config file: "/etc/X11/xorg.conf"
(==) ServerLayout "Default Layout"

(**) |-->Screen "STI" (0)
(**) |   |-->Monitor "<default monitor>"
(**) |   |-->Device "Generic Video Card"

(==) No monitor specified for screen "STI".
Using a default monitor configuration.
-> same failure.

b) If I just drop the HorizSync and VertRefresh entries, it fails as well:
(II) Running in FRAMEBUFFER Mode
(**) FBDEV(0): Depth 8, (--) framebuffer bpp 8
(==) FBDEV(0): Default visual is PseudoColor
(==) FBDEV(0): Using gamma correction (1.0, 1.0, 1.0)
(II) FBDEV(0): hardware: stifb (video memory: 1536kB)
(**) FBDEV(0): Option "fbdev" "/dev/fb0"
(II) FBDEV(0): checking modes against framebuffer device... (II) FBDEV(0): mode "1024x768" test failed
(II) FBDEV(0): checking modes against monitor...
(--) FBDEV(0): Virtual size is 1024x768 (pitch 1024)
(**) FBDEV(0): Built-in mode "current": 28000.0 MHz, 27343.8 kHz, 35603.8 Hz
(II) FBDEV(0): Modeline "current"x0.0 28000.00 1024 1024 1024 1024 768 768 768 768 -hsync -vsync -csync (27343.8 kHz)
(==) FBDEV(0): DPI set to (96, 96)

Helge

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK & win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Linux-fbdev-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/linux-fbdev-devel

[...]

[...]

Which part of 'the Modes line in SubSection "Display"' is so hard to understand? *shrug*

Earthling Michel Dänzer           |          http://tungstengraphics.com
Libre software enthusiast         |          Debian, X and DRI developer


-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Linux-fbdev-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-fbdev-devel

Do you want me to change this (1024x768): ?

	SubSection "Display"
                 Depth           8
                 Modes           "1024x768"
         EndSubSection

Why?
There is no reason.
The hardware can only run at 1024x768 at 8bpp.
If I change this, it will clearly fail.
No need to try as the answer is clear.

Reason:

root@c3000:/usr/lib/xorg/modules/linux# fbset -i

mmode "1024x768"

     geometry 1024 768 1024 768 8
     timings 0 0 0 0 0 0 0
     rgba 8/0,8/0,8/0,0/0

endmode

Frame buffer device information:

     Name        : stifb
     Address     : 0xf9000000
     Size        : 1572864
     Type        : PACKED PIXELS
     Visual      : PSEUDOCOLOR
     XPanStep    : 0
     YPanStep    : 0
     YWrapStep   : 0
     LineLength  : 2048
     MMIO Address: 0xf8100000
     MMIO Size   : 4194304
     Accelerator : No

Helge

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK & win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Linux-fbdev-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/linux-fbdev-devel

Not change it, remove it (hence 'without').

I suspect the problem is that you're misunderstanding the meaning of this line. It tells the X server to try all the 1024x768 modes from its internal mode database, none of which match the mode of your framebuffer device unsurprisingly. If you omit the line, the fbdev driver will get the current mode from the framebuffer device and try that. Note that the pixclock 0 can still cause trouble with X servers before the upcoming 1.5 release.

Earthling Michel Dänzer           |          http://tungstengraphics.com
Libre software enthusiast         |          Debian, X and DRI developer


-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Linux-fbdev-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-fbdev-devel

No, doesn't work either (see below).
Seems to be the "pixclock 0" problem you mention.
What changed in 1.5? Any link to a patch?

Helge

X.Org X Server 1.4.2
Release Date: 11 June 2008
X Protocol Version 11, Revision 0
Build Operating System: Linux Debian (xorg-server 2:1.4.2-4) Current Operating System: Linux c3000 2.6.27-rc4 #7 SMP Thu Aug 28 23:15:29 CEST 2008 parisc
Build Date: 15 August 2008 06:24:13PM
...
(II) FBDEV(0): using /dev/fb0
(II) Running in FRAMEBUFFER Mode
(II) FBDEV(0): Creating default Display subsection in Screen section
"STI" for depth/fbbpp 8/8
(==) FBDEV(0): Depth 8, (==) framebuffer bpp 8
(==) FBDEV(0): Default visual is PseudoColor
(==) FBDEV(0): Using gamma correction (1.0, 1.0, 1.0) (II) FBDEV(0): hardware: stifb (video memory: 1536kB) (**) FBDEV(0): Option "fbdev" "/dev/fb0"
(II) FBDEV(0): checking modes against framebuffer device... (II) FBDEV(0): checking modes against monitor...
(--) FBDEV(0): Virtual size is 1024x768 (pitch 1024)
(**) FBDEV(0): Built-in mode "current": 28000.0 MHz, 27343.8 kHz, 35603.8 Hz
(II) FBDEV(0): Modeline "current"x0.0 28000.00 1024 1024 1024 1024 768 768 768 768 -hsync -vsync -csync (27343.8 kHz)
(==) FBDEV(0): DPI set to (96, 96)
...
(EE) FBDEV(0): FBIOPUT_VSCREENINFO succeeded but modified mode (EE) FBDEV(0): mode initialization failed

Fatal server error:
AddScreen/ScreenInit failed for driver 0

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK & win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Linux-fbdev-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/linux-fbdev-devel

http://cgit.freedesktop.org/xorg/xserver/commit/?id=c095da04fe7c73b6503ef5b93549b13796c51b22

Earthling Michel D채nzer           |          http://tungstengraphics.com
Libre software enthusiast         |          Debian, X and DRI developer


-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Linux-fbdev-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-fbdev-devel

What about this?

4. Provide a generic check_var() that does some basic sanity checking against info->var (eg. check xres, yres and bits_per_pixel).

Ville Syrjälä
[email protected]
http://www.sci.fi/~syrjala/

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

Yes, possible.
But that wouldn't solve issues with Xorg's current implementation, as Xorg is currently checking the monitor sync values as well (see
https://bugs.freedesktop.org/show_bug.cgi?id=17153)

Helge

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK & win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

Sounds good.

How about this.
Any comments? Expecially about what to check, what to restore.

Cheers,
/yoshii # Restoring pixclock would be a workaround for Xorg issue :)

drivers/video/fbmem.c:fb_check_var()
New function for fixed mode device which doesn't provide its own check_var function.

index c6b8e92..ff94eaf 100644
@@ -904,6 +904,56 @@ static int fb_check_caps(struct fb_info *info, struct fb_var_screeninfo *var,
 	return err;
 }
 
+/* Sanity check for drivers which can't change video mode */
+static int
+fb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
+{
+	struct fb_var_screeninfo *constant = &info->var;
+        __u32 xoffset = var->xoffset;
+	__u32 yoffset = var->yoffset;
+	__u32 activate = var->activate;
+	__u32 yres = (constant->vmode & FB_VMODE_YWRAP)? 0: var->yres; 
+
+	/* do round _up_ */
+	if (var->xres_virtual < xoffset + var->xres)
+		var->xres_virtual = xoffset + var->xres;
+	if (var->yres_virtual < yoffset + yres)
+		var->yres_virtual = yoffset + yres;
+
+	/* bigger is error, smaller is OK */
+	if( ( var->xres > constant->xres )
+	  ||( var->yres > constant->yres )
+	  ||( var->xres_virtual > constant->xres_virtual )
+	  ||( var->yres_virtual > constant->yres_virtual ))
+		return -EINVAL;
+
+	/* only length are checked for bitfields */
+	if( ( var->bits_per_pixel > constant->bits_per_pixel )
+	  ||( var->red.length > constant->red.length )
+	  ||( var->green.length > constant->green.length )
+	  ||( var->blue.length > constant->blue.length ))
+		return -EINVAL;
+
+	/* boolean parameters can't be rounded, should be equal */
+	if( ( !var->grayscale != !constant->grayscale )
+	  ||( !var->nonstd != !constant->nonstd )
+	  ||( var->vmode != constant->vmode ))
+		return -EINVAL;
+
+	/* pan is acceptable only if we have fb_pan_display) */
+	if ( (var->yoffset || var->xoffset) && !info->fbops->fb_pan_display )
+		return -EINVAL;
+
+	/* copy most */
+	*var = *constant;
+	/* resotore some that are not a part of viede mode */
+	var->xoffset = xoffset;
+	var->yoffset = yoffset;
+	var->activate = activate;
+
+	return 0;
+}
+
 int
 fb_set_var(struct fb_info *info, struct fb_var_screeninfo *var)
 {

@@ -940,9 +990,11 @@ fb_set_var(struct fb_info *info, struct fb_var_screeninfo *var)
 
 		if (info->fbops->fb_check_var) {
 			ret = info->fbops->fb_check_var(var, info);
-			if (ret)
-				goto done;
-		}
+		} else
+			ret = fb_check_var(var, info);
+
+		if (ret)
+			goto done;
 
 		if ((var->activate & FB_ACTIVATE_MASK) == FB_ACTIVATE_NOW) {
 			struct fb_videomode mode;

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK & win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

What 'Xorg issue'? I've only seen a misconfiguration and a pixclock 0 issue which has been fixed in Xorg.

Earthling Michel Dänzer           |          http://tungstengraphics.com
Libre software enthusiast         |          Debian, X and DRI developer


-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Linux-fbdev-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-fbdev-devel

pixclock 0 one. Which has beed fixed in git. But I don't expect all people
do/can build latest Xorg source. And there might be people who think modifying kernel is easier, especially because this is kernel related ML.
/yoshii

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK & win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

Take 2.

This adds sanitizing. But main effect is, as a result, adding
FBIOPUT_VSCREENINFO on fixed mode device following functionality.
. return error when request screen is bigger than the fixed mode. . round pan values if invalid
. do set pan
same as drivers which has own check_var.

This was to a part of pan check rounding [xy]res_virtual up instead of
rounding [xy]offset down, and check them in lines below there.
But as you pointed, it's weired for this(checking against constants) case.

I changed this non-error, but resetting pan values with info->var. This supports HW which can't change pan but have initial fixed offsets. # So far, not in kernel source, though.

I add explicit rounding down code this time, instead of returning error. I am not sure which is right. But because I found 10 drivers round them down,
6 return error, 2 set const value.

# and 80 (20(with pan) + 60(no pan)) ignore them.

/yoshii

drivers/video/fbmem.c:fb_check_var(): New function
for fixed mode device which doesn't provide its own check_var function.

index c6b8e92..243e5f4 100644
@@ -904,6 +904,68 @@ static int fb_check_caps(struct fb_info *info, struct fb_var_screeninfo *var,
 	return err;
 }
 
+/* Sanity check for drivers which can't change video mode */
+static int
+fb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
+{
+	struct fb_var_screeninfo *constant = &info->var;
+	struct fb_fix_screeninfo *fix = &info->fix;
+	__u32 xoffset = var->xoffset;
+	__u32 yoffset = var->yoffset;
+	__u32 activate = var->activate;
+	__u32 ywrap = var->vmode & FB_VMODE_YWRAP;
+
+	/* Bigger is error, smaller is OK */
+	if ((var->xres > constant->xres) ||
+	    (var->yres > constant->yres) ||
+	    (var->xres_virtual > constant->xres_virtual) ||
+	    (var->yres_virtual > constant->yres_virtual))
+		return -EINVAL;
+
+	/* Ohly length are checked for bitfields */
+	if ((var->bits_per_pixel > constant->bits_per_pixel) ||
+	    (var->red.length > constant->red.length) ||
+	    (var->green.length > constant->green.length) ||
+	    (var->blue.length > constant->blue.length))
+		return -EINVAL;
+
+	/* Boolean parameters can't be rounded, should be equal */
+	if ((!var->grayscale != !constant->grayscale) ||
+	    (!var->nonstd != !constant->nonstd) ||
+	    ((var->vmode ^ constant->vmode) &~FB_VMODE_YWRAP))
+		return -EINVAL;
+
+	/* round offsets */
+	if (xoffset > constant->xres_virtual - constant->xres)
+		xoffset = constant->xres_virtual - constant->xres;
+	if (fix->xpanstep)
+		xoffset -= (var->xoffset % fix->xpanstep);
+
+	if (ywrap) {
+		if (yoffset > constant->yres_virtual -1)
+			yoffset = constant->yres_virtual -1;
+		if (fix->ywrapstep)
+			yoffset -= (yoffset % fix->ywrapstep);
+	} else {
+		if (yoffset > constant->yres_virtual - constant->yres)
+			yoffset = constant->yres_virtual - constant->yres;
+		if (fix->ypanstep)
+			yoffset -= (yoffset % fix->ypanstep);
+	}
+
+	/* Copy most */
+	*var = *constant;
+	/* Restore some that are not a part of video mode */
+	var->activate = activate;
+	/*  You can't alter pan parameters without fb_pan_display() */
+	if ( info->fbops->fb_pan_display ) {
+		var->xoffset = xoffset;
+		var->yoffset = yoffset;
+		var->vmode = (constant->vmode &~FB_VMODE_YWRAP) | ywrap;
+	}
+	return 0;
+}
+
 int
 fb_set_var(struct fb_info *info, struct fb_var_screeninfo *var)
 {

@@ -940,9 +1002,11 @@ fb_set_var(struct fb_info *info, struct fb_var_screeninfo *var)
 
 		if (info->fbops->fb_check_var) {
 			ret = info->fbops->fb_check_var(var, info);
-			if (ret)
-				goto done;
-		}
+		} else
+			ret = fb_check_var(var, info);
+
+		if (ret)
+			goto done;
 
 		if ((var->activate & FB_ACTIVATE_MASK) == FB_ACTIVATE_NOW) {
 			struct fb_videomode mode;

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK & win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

It will be set later to the correct resolution. The above hunk just taks care of the rounding rules.

Anyway, I'm still wondering whether this check is really needed. If your application doesn't look at how fb_var_screeninfo was changed by calling FBIOPUT_VSCREENINFO, you're in deep trouble anyway, due to the rounding rules.

As I don't have access to a PA-RISC machine with the mentioned hardware, I modified ps3fb by commenting out its fb_check_var() and fb_set_par() routines (this also gave a black screen, but as far as the fbdev subsystem is concerned, it has a working frame buffer device that supports one single video mode only). Surprisingly, X indeed didn't like it:

| X.Org X Server 1.4.2
| Release Date: 11 June 2008
| X Protocol Version 11, Revision 0
| Build Operating System: Linux Debian (xorg-server 2:1.4.2-3)
| Current Operating System: Linux ps3 2.6.27-rc4-00176-gb8e6c91-dirty #1815 SMP Fri Aug 29 13:54:57 CEST 2008 ppc64
| Build Date: 03 August 2008  03:08:04AM
|  
|         Before reporting problems, check http://wiki.x.org
|         to make sure that you have the latest version.
| Module Loader present
| Markers: (--) probed, (**) from config file, (==) default setting,
|         (++) from command line, (!!) notice, (II) informational,
|         (WW) warning, (EE) error, (NI) not implemented, (??) unknown.
| (==) Log file: "/var/log/Xorg.0.log", Time: Fri Aug 29 13:57:26 2008
| (==) Using config file: "/etc/X11/xorg.conf"
| (EE) FBDEV(0): FBIOPUT_VSCREENINFO succeeded but modified mode
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| (EE) FBDEV(0): mode initialization failed
| 
| Fatal server error:
| AddScreen/ScreenInit failed for driver 0

The "Screen" / "Display" (sub)section has `Modes "1920x1200"'. Removing this indeed doesn't make a difference.

As I had added debug code to drivers/video/fbmem.c:fb_set_var(), I could see what happened in the working version with mode-setting vs. the non-working version without modesetting:
- working, with mode-setting:
o fb_set_var() fails for the standard 1920x1200 mode in the Xorg database,
o X fallbacks to the current 1920x1200 mode.
- non-working, without modesetting:
o fb_set_var() doesn't fail for the standard 1920x1200 mode in the Xorg database, but it returns the current 1920x1200 mode,
o X complains that the mode got modified.

Hence Xorg is broken on all embedded devices with a frame buffer driver that supports a single fixed video mode only?

BTW, I know current Xorg is broken on several m68k platforms with non-chunky frame buffer layouts, due to some serious bugs in the way some uncommon values of frame buffer parameters are handled, but IIRC they shouldn't affect `common' chunky frame buffers. Still have to look into it to fix it properly (anyone who can donate me some spare time? ;-)

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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 the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK & win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

_______________________________________________
Linux-fbdev-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-fbdev-devel

[...]

[...]

No. As discussed before, you're probably missing

http://cgit.freedesktop.org/xorg/xserver/commit/?id=c095da04fe7c73b6503ef5b93549b13796c51b22

which is in the upcoming xserver 1.5 release.

(BTW, would you happen to remember what the magic pixclock value was there for in the first place? :)

Earthling Michel Dänzer           |          http://tungstengraphics.com
Libre software enthusiast         |          Debian, X and DRI developer


-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Linux-fbdev-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-fbdev-devel

Ah, upcoming. That's not current/previous/... ;-)

To avoid a division by zero exception? I guess I just took a value that looked sensible.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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 the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK & win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

_______________________________________________
Linux-fbdev-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-fbdev-devel

Yeah, it might have been nice to backport this to the 1.4 branch, but it's kind of too late now. Of course, you can always bug your distributor to backport it. :)

Earthling Michel Dänzer           |          http://tungstengraphics.com
Libre software enthusiast         |          Debian, X and DRI developer


-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Linux-fbdev-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-fbdev-devel

The attached patch is a completely different approach to solve the issue. I've tested it, and it works even with the buggy version of X (1.4.2).

Basic idea is, that when we have fixed mode devices (aka drivers which don't implement fb_check_var) we just simply fill in a "emulated" valid monitor/modeline into the var struct during the register_framebuffer() initialization call. Since the modeline is valid, X does not has any problems with it.

The patch has a few benefits:
a) We still keep binary compatability/behavior for FBIOGET_VSCREENINFO and FBIOPUT_VSCREENINFO.
b) In Xorg.conf files, users can simply delete all monitor and display sections -> Xorg will autoconfigure itself to the only valid resolution/modeline itself on all devices (I really like this!!!)

Thus, the patch basically changes the fixed mode drivers to have some monitor timings by default. As an example, before I got:

root@c3000:~# fbset -i
mode "1024x768"
    geometry 1024 768 1024 768 8
    timings 0 0 0 0 0 0 0
    rgba 8/0,8/0,8/0,0/0
endmode

Now I see:

root@c3000:~# fbset -i
mode "1024x768-60"
    # D: 65.003 MHz, H: 48.365 kHz, V: 60.006 Hz
    geometry 1024 768 1024 768 8
    timings 15384 168 8 29 3 144 6
    rgba 8/0,8/0,8/0,0/0
endmode

Opinions ?

index 98843c2..72b3113 100644
@@ -1470,9 +1470,23 @@ register_framebuffer(struct fb_info *fb_info)
 	if (!fb_info->pixmap.blit_y)
 		fb_info->pixmap.blit_y = ~(u32)0;
 
-	if (!fb_info->modelist.prev || !fb_info->modelist.next)
+	if (!fb_info->modelist.prev || !fb_info->modelist.next) {
 		INIT_LIST_HEAD(&fb_info->modelist);
 
+		/* For drivers which does not support mode setting
+		 * initialize an emulated modeline */
+		if (!fb_info->fbops->fb_check_var &&
+		     fb_info->var.pixclock == 0) {
+			char modestr[32];
+			snprintf(modestr, sizeof(modestr), "%dx%d-%d",
+				fb_info->var.xres,
+				fb_info->var.yres,
+				fb_info->var.bits_per_pixel);
+			fb_find_mode(&fb_info->var, fb_info, modestr,
+				NULL, 0, NULL, fb_info->var.bits_per_pixel);
+		}
+	}
+
 	fb_var_to_videomode(&mode, &fb_info->var);
 	fb_add_videomode(&mode, &fb_info->modelist);
 	registered_fb[i] = fb_info;

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK & win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

_______________________________________________
Linux-fbdev-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-fbdev-devel

Sadly nobody answered yet....

Helge

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK & win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

FWIW, I like this approach much better.

Earthling Michel Dänzer           |          http://tungstengraphics.com
Libre software enthusiast         |          Debian, X and DRI developer


-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Linux-fbdev-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-fbdev-devel

Thanks Michel,

I'll send an updated patch shortly (in a new thread) which fixes the english comment as well.

Helge

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK & win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Linux-fbdev-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/linux-fbdev-devel

Hi,

It works nicely, and simple. but,
IMHO, It is not bad for workaround, but not for merging into mainline. If it is only for workaround, setting info->var->pixclock(and some others)
in your driver, is simpler (assuming we have source code).

Generally, pixclock is 0 in purpose.
It means there really is no clock, or author didn't have enough HW spec. Anyway, it indicate there is no information about pixclock. I think nothing other than 0 is suitable for this, because this is T of clock, and T=0 is logically invalid (f=INF?). .... I think.

If it is true as fbdev specification, that means applications must ignore
timings when pixclock==0.
/yoshii

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK & win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

It can be used to to quickly check a bunch of modes for with FB_ACTIVATE_TEST. In fact FB_ACTIVATE_TEST's comment says that rounding up should not happen here but no driver implements that. It could be implemented in the common code by keeping a copy of the original var and comparing them after the driver has done the rounding.

But what are the rounding rules anyway?
My take on the matter:
- xres_virtual/yres_virtual can be rounded up.
- xres/yres shouldn't be rounded since this is the first thing users usually care about.
- bits_per_pixel/red/gren/blue/transp shouldn't be rounded since it's the second thing users care about. I think most driver do round these though. In fact most drivers don't really even look at anything besides bits_per_pixel and green.length. Perhaps if the user has set some of them to zero the driver could pick freely.
- Timing values can be rounded. Which way or by how much I'm not sure. Refresh rate is the third thing users care about so huge rounding here could be problematic but some rounding is probably needed due to hardware constraints. In the "hardware supports only one mode" case the rounding should probably be unlimited though.
- xoffset/yoffset should be checked against panstep/wrap. FBIOPAN_DISPLAY doesn't allow any rounding but should FBIOPUT_VSCREENINFO?

Maybe some of the constraints (like limiting the refresh rate rounding to some %) should only be enforced with FB_ACTIVATE_TEST. That would prevent it breaking some stupid application that just sets some randomish mode and expects the driver to hand out something that's supported.

Ville Syrjälä
[email protected]
http://www.sci.fi/~syrjala/

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

Thank you for your explanation.
Sorry for slow, long response.
Summary:
1. I understand your strategy (no check == fix video).
2. pan should be fixed state after set var.
3. pixclock can be any, OK?

---
1.

Well, this logic looks like "A is B because A is B". But, anyway this should be this because you say so. Accepting this as a fixed rule, things becomes simple.

I guess this is based on the idea
var should always be corresponding to HW state.
My patch was based on the idea
var can be any, because HW accept all (by just ignoring all).

Right. It's important not to break existing application. I found Xorg server
does set var then check it to see if the operation was successful.
Perhaps that is the usage of this API. So, returning error seems bad.

OK. I don't think it is bad idea.

---
2. pan should be fixed state after set var.

# This is becoming another topic, though...
One small problem of current code is that
FBIOPUT_VSCREENINFO sometimes set PAN but sometimes does not.

# assuming pan is not a part of "video mode".

It will be 1.unchanged, or 2.info->var, or 3.var, or 4.var+rounding?, but generally pan state after FBIOPUT_VSCREENINFO should be considered
as unknown(but something valid), even if passing valid value(say, (0,0)).

It's ok if set or not. But, I believe it should be deterministic.

---
3. pixclock can be any, OK?

The real problem(for me;) might be
. Habit of applications that check the var by themselves. This is not actually a topic on this ML. But I want to confirm that
pixclock can be any. Even 0 is a kind of "Valid" value.
OK?

I'm asking because, there are drivers return timing parameters as 0.
- HW has clock, known, but set 0 (sh_mobile_lcdfb).
- HW must have clock, but unknown (hitfb, stifb, xilinxfb).
- No clock (xenfb).
There are drivers return false value.
- No clock but return some (vfb).

If 0 is valid, problem is Xorg's issue.
X(at least 1.4) doesn't accept clock=0. (and the reason looks be buggy)

Regards,
/yoshii

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK & win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

No, var cannot be any. Var can only be the single tuple of values the hardware supports.

Panning state is not unknown. It's in var.

If you don't know the pixclock, set it to 0.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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 the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK & win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

I don't think it is not always in var.
I found there are following three types of drivers.

1. ideal(?) driver
FBIOPUT_VSCREENINFO: pan (rounded a_set_varnd) done, no error, pan in returned var
FBIOGET_VSCREENINFO: returns pan

2. driver which doesn't provide own check_var (fixed mode drivers)
FBIOPUT_VSCREENINFO: no pan done, no error, false pan in returned var
FBIOGET_VSCREENINFO: returns pan

3. driver which doesn't provede pan_display nor check pan in check_var
FBIOPUT_VSCREENINFO: no pan done, no error, false pan in returned var
FBIOGET_VSCREENINFO: returns false pan

2 will be trouble if people expect they can initialize pan by FBIOPUT_VSCREENINFO. Actually, you need FBIOPAN_DISPLAY afterwards. Or when they believe returned var have valid value for pan. Actually, you need FBIOGET_VSCREENINFO (anyway, you need FBIOPAN_DISPLAY as
described above, this one is not so big problem).

3 is weired. info->var continuously have value which can not be set to HW. Many drivers which doesn't support pan seems to be this type (sstfb is one of exceptions).
Possibly, we can fix it at the middle of fb_set_var().
But because of the rule(?) that only *_check_var() can freely modify var, In this case, *_check_var() in each driver should do return error, or set
them 0. Right?

OK. Thank you.
/yoshii

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK & win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/