Problem with xorg-server and s3c2410fb driver

[Michel Dänzer]

On Mon, 2010-08-02 at 11:51 -0400, Adam Jackson wrote: 
> On Mon, 2010-08-02 at 15:59 +0200, Michel Dänzer wrote:
> > On Son, 2010-07-25 at 09:35 +0300, Vasily Khoruzhick wrote: 
> > > Hi,
> > > 
> > > I'm trying to get xorg-server + xf86-video-fbdev working on my device, and I'm 
> > > hitting problem:
> > > 
> > > pixclock value of fb mode on my device is 260000 picosecs, and due to picosecs 
> > > <-> khz conversions in xorg-server I'm getting rounding error:
> > > 
> > > 1000000000 / 260000 = 3846
> > > 1000000000 / 3846 = 260010
> > > 
> > > (Look through xfree2fbdev_timing and fbdev2xfree_timing functions in fbdevhw.c
> > > for more details)
> > > 
> > > xorg-server gets mode via FBOIGET_VSCREENINFO (pixclock = 260000), converts it 
> > > to its own format, then converts it back to fbmode (due to rounding error 
> > > pixclock - 260010) and then tries to set mode with pixvalue 260010 via 
> > > FBIOPUT_VSCREENINFO ioctl, but driver sets it back to 260000, and xorg-server 
> > > fails to startup with this message:
> > > 
> > > (EE) FBDEV(0): FBIOPUT_VSCREENINFO succeeded but modified mode.
> > 
> > What version of xserver are you using? It sounds like your problem
> > should be fixed by Git commit bf333c2f9833a178887e7bdd7fc338f1e09c387f
> > ('fbdevhw: Remove pixclock check.') which should be included since 1.6.x
> > at least.
> 
> bf333c2 is kind of gross.  Seems like the right thing is to revert it
> and then apply:
> 
> ------
> From: Adam Jackson <ajax at redhat.com>
> Date: Mon, 2 Aug 2010 11:48:22 -0400
> Subject: [PATCH] fbdevhw: Avoid rounding error from psec/kHz conversion
> 
> Stash the real fbdev clock in mode->PrivFlags and use that for actual
> mode set calls.  ->Clock is then only used for display in the log.
> 
> Signed-off-by: Adam Jackson <ajax at redhat.com>
> ---
>  hw/xfree86/fbdevhw/fbdevhw.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/xfree86/fbdevhw/fbdevhw.c b/hw/xfree86/fbdevhw/fbdevhw.c
> index f160908..3a6df94 100644
> --- a/hw/xfree86/fbdevhw/fbdevhw.c
> +++ b/hw/xfree86/fbdevhw/fbdevhw.c
> @@ -201,7 +201,7 @@ xfree2fbdev_timing(DisplayModePtr mode, struct fb_var_screeninfo *var)
>  	if (var->yres_virtual < var->yres)
>  		var->yres_virtual = var->yres;
>  	var->xoffset = var->yoffset = 0;
> -	var->pixclock = mode->Clock ? 1000000000/mode->Clock : 0;
> +	var->pixclock = mode->PrivFlags;
>  	var->right_margin = mode->HSyncStart-mode->HDisplay;
>  	var->hsync_len = mode->HSyncEnd-mode->HSyncStart;
>  	var->left_margin = mode->HTotal-mode->HSyncEnd;
> @@ -267,6 +267,7 @@ fbdev2xfree_timing(struct fb_var_screeninfo *var, DisplayModePtr mode)
>  	else if ((var->vmode & FB_VMODE_MASK) == FB_VMODE_DOUBLE)
>  		mode->Flags |= V_DBLSCAN;
>  	mode->SynthClock = mode->Clock;
> +	mode->PrivFlags = var->pixclock;
>  	mode->CrtcHDisplay = mode->HDisplay;
>  	mode->CrtcHSyncStart = mode->HSyncStart;
>  	mode->CrtcHSyncEnd = mode->HSyncEnd;

AFAICT this will only work when using the 'built-in' mode, otherwise
mode->PrivFlags will never be set? One possible solution for that could
be

- var->pixclock = mode->Clock ? 1000000000/mode->Clock : 0;
+ var->pixclock = mode->PrivFlags ? mode->PrivFlags :
(mode->Clock ? 1000000000/mode->Clock : 0);

in xfree2fbdev_timing().


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