xserver: Branch 'master'

[Luc Verhaegen]

On Mon, Jan 29, 2007 at 09:55:54AM -0800, Eric Anholt wrote:
>  hw/xfree86/common/xf86Mode.c |   39 ---------------------------------------
>  1 files changed, 39 deletions(-)
> 
> New commits:
> diff-tree 31f2d4a57e04f5ea635fbb50c508405c4fc37b65 (from 1627af54497bee659ea30f2850b39cbbf576e22d)
> Author: Eric Anholt <eric at anholt.net>
> Date:   Mon Jan 29 09:39:33 2007 -0800
> 
>     Bug #9680: Remove bogus blank length limiting in xf86SetModeCrtc().
>     
>     Our modes typically come from EDID or default modes, and when the monitor
>     asks for a specific mode, deciding to tweak it usually results in incorrect
>     display.  And if the user is specifying a mode by hand, tweaking it then is
>     still pretty rude.
>     
>     Reviewed by: ajax
> 
> diff --git a/hw/xfree86/common/xf86Mode.c b/hw/xfree86/common/xf86Mode.c
> index 3cebac7..7f5ae36 100644
> --- a/hw/xfree86/common/xf86Mode.c
> +++ b/hw/xfree86/common/xf86Mode.c
> @@ -727,45 +727,6 @@ xf86SetModeCrtc(DisplayModePtr p, int ad
>      }
>      p->CrtcHAdjusted = FALSE;
>      p->CrtcVAdjusted = FALSE;
> -
> -    /*
> -     * XXX
> -     *
> -     * The following is taken from VGA, but applies to other cores as well.
> -     */
> -    p->CrtcVBlankStart = min(p->CrtcVSyncStart, p->CrtcVDisplay);
> -    p->CrtcVBlankEnd = max(p->CrtcVSyncEnd, p->CrtcVTotal);
> -    if ((p->CrtcVBlankEnd - p->CrtcVBlankStart) >= 127) {
> -        /* 
> -         * V Blanking size must be < 127.
> -         * Moving blank start forward is safer than moving blank end
> -         * back, since monitors clamp just AFTER the sync pulse (or in
> -         * the sync pulse), but never before.
> -         */
> -        p->CrtcVBlankStart = p->CrtcVBlankEnd - 127;
> -	/*
> -	 * If VBlankStart is now > VSyncStart move VBlankStart
> -	 * to VSyncStart using the maximum width that fits into
> -	 * VTotal.
> -	 */
> -	if (p->CrtcVBlankStart > p->CrtcVSyncStart) {
> -	    p->CrtcVBlankStart = p->CrtcVSyncStart;
> -	    p->CrtcVBlankEnd = min(p->CrtcHBlankStart + 127, p->CrtcVTotal);
> -	}
> -    }
> -    p->CrtcHBlankStart = min(p->CrtcHSyncStart, p->CrtcHDisplay);
> -    p->CrtcHBlankEnd = max(p->CrtcHSyncEnd, p->CrtcHTotal);
> -
> -    if ((p->CrtcHBlankEnd - p->CrtcHBlankStart) >= 63 * 8) {
> -        /*
> -         * H Blanking size must be < 63*8. Same remark as above.
> -         */
> -        p->CrtcHBlankStart = p->CrtcHBlankEnd - 63 * 8;
> -	if (p->CrtcHBlankStart > p->CrtcHSyncStart) {
> -	    p->CrtcHBlankStart = p->CrtcHSyncStart;
> -	    p->CrtcHBlankEnd = min(p->CrtcHBlankStart + 63 * 8, p->CrtcHTotal);
> -	}
> -    }
>  }
>  
>  /**
This is _very_ wrong.

You've subtly broken many drivers now.

I used this code in my EXDC slides to show an example of what things 
were still present in xf86Mode.c, and I've known about the effects of 
this code since the spring of 2005, yet I didn't dream of removing 
this before fixing things properly.

Almost all hardware has a, sometimes highly, limited blanking range, 
inherited from VGA. The work-around is to use overscan, and this is 
exactly what this code does. Most modern hardware adds one or several 
bits to the VGA blankend, but that's not always enough. Overscan is 
never a problem, CRTCs just need to have their LUTs set up correctly. 

All drivers have been tested with these VGA limitations, plainly 
removing this code will break many on some modes.

You need to either stop or work around wide blanking in _each_ drivers 
validmode. It's easy to provide a small helper (or a pair of), taking in 
the mode and the maximum blanking. Feel free to, once again (but do try 
to admit at this time), look at my unichrome code for a suited 
implementation, where i do handle this correctly.

So if you want to remove this, you need to go over each driver and find 
out about the device specific limitations. You're also able to easily 
work around the VGA style blanking in your specific driver's ValidMode.

Luc Verhaegen.
http://unichrome.sf.net/