Eureka, washed out intel GM965 tv-out solved

[Jesse Barnes]

On Thursday, November 01, 2007 7:22 pm rglowery at exemail.com.au wrote:
> > On Sunday, October 28, 2007 4:03 rglowery at exemail.com.au wrote:
> >> >> Browsing through i810_reg.h I came across
> >> >>
> >> >> 1573 /** @defgroup TV_CSC_KNOBS
> >> >> 1574  * @{
> >> >> 1575  */
> >> >> 1576 #define TV_CLR_KNOBS            0x68028
> >> >> 1577 /** 2s-complement brightness adjustment */
> >> >> 1578 # define TV_BRIGHTNESS_MASK             0xff000000
> >> >> 1579 # define TV_BRIGHTNESS_SHIFT            24
> >> >> 1580 /** Contrast adjustment, as a 2.6 unsigned floating point
> >> >> number */ 1581 # define TV_CONTRAST_MASK               0x00ff0000
> >> >> 1582 # define TV_CONTRAST_SHIFT              16
> >> >> 1583 /** Saturation adjustment, as a 2.6 unsigned floating point
> >> >> number */
> >> >> 1584 # define TV_SATURATION_MASK             0x0000ff00
> >> >> 1585 # define TV_SATURATION_SHIFT            8
> >> >> 1586 /** Hue adjustment, as an integer phase angle in degrees */
> >> >> 1587 # define TV_HUE_MASK                    0x000000ff
> >> >> 1588 # define TV_HUE_SHIFT                   0
> >> >> 1589 /** @} */
> >> >>
> >> >> This led me to the following patch which solves the problem for me
> >> >> diff --git a/src/i830_tv.c b/src/i830_tv.c
> >> >> index ee2538a..5f82188 100644
> >> >> --- a/src/i830_tv.c
> >> >> +++ b/src/i830_tv.c
> >> >> @@ -1178,7 +1178,7 @@ i830_tv_mode_set(xf86OutputPtr output,
> >> >> DisplayModePtr mode,
> >> >>             (i830_float_to_csc(color_conversion->bv) << 16) |
> >> >>             (i830_float_to_luma(color_conversion->av)));
> >> >>
> >> >> -    OUTREG(TV_CLR_KNOBS, 0x10606000);
> >> >> +    //OUTREG(TV_CLR_KNOBS, 0x10606000);
> >> >>      OUTREG(TV_CLR_LEVEL, ((video_levels->black <<
> >> >> TV_BLACK_LEVEL_SHIFT)
> >> >>
> >> >>                 (video_levels->blank << TV_BLANK_LEVEL_SHIFT)));
> >> >>      {
> >> >>
> >> >>
> >> >> I guess the BIOS defaults these to something more reasonable.
> >> >
> >> > ModeDebug shows the TV_CLR_KNOBS registry defaulting to 0x00404000
> >> >
> >> > I'll leave it up to the intel people with doco to determine what
> >> > the appropriate fix is.
> >>
> >> Sorry for the multiple posts, but it looks like the TV_CLR_KNOBS
> >> register is not initialized by the BIOS if a DVI display is also
> >> connected, so for me, the patch to resolve the washed out colours
> >> regardless of the connected devices is
> >>
> >> diff --git a/src/i830_tv.c b/src/i830_tv.c
> >> index ee2538a..e6d2ee5 100644
> >> --- a/src/i830_tv.c
> >> +++ b/src/i830_tv.c
> >> @@ -1178,7 +1178,7 @@ i830_tv_mode_set(xf86OutputPtr output,
> >> DisplayModePtr mode
> >>             (i830_float_to_csc(color_conversion->bv) << 16) |
> >>             (i830_float_to_luma(color_conversion->av)));
> >>
> >> -    OUTREG(TV_CLR_KNOBS, 0x10606000);
> >> +    OUTREG(TV_CLR_KNOBS, 0x00404000);
> >>      OUTREG(TV_CLR_LEVEL, ((video_levels->black <<
> >> TV_BLACK_LEVEL_SHIFT) | (video_levels->blank <<
> >> TV_BLANK_LEVEL_SHIFT))); {
> >
> > Hi Rob.  Thanks for tracking down this problem.  Can you make sure there
> > are bugs filed for both this issue and the 1920x1080 mode problem you
> > discovered earlier (preferably with your patches attached :)?  I don't
> > want to lose the fixes... we'll review them soon.
>
> Jesse,
>
> Investigating the washed out colour issue further, I see the intel driver
> was originally written to use the TV_CLR_KNOBS value of 0x00606000, the
> 0x10 brightness was introduced by Zou Nan hai on 9 Feb 2007 with a fix
> titled "TV Enable".  I suspect the 0x10 brigtness is the real cause of the
> washed out issue.
>
> Now if my 8 bit (2.6) floating point understanding is correct, 0x60 = 2^1
> * 0.5 = 1.0 and 0x40 = 2^1 = 2.0.  Could you please look at the intel doco
> and check which is likely to be a better default.  I suspect it might be
> the 1.0 in which case, the patch should be just to revert to the original
> behaviour.
>
> >> -    OUTREG(TV_CLR_KNOBS, 0x10606000);
> >> +    OUTREG(TV_CLR_KNOBS, 0x00606000);
>
> In anycase, when I get home from work tonight, I'll try both and see which
> gives better results and let you know

I was going to change the defaults, but it sounds like the the current ones 
work for some people, while other people need other defaults, so I think I'll 
leave them alone for the 2.2 release.  If distros or users want, they can 
patch in my TV output properties patch and control them by hand, but that'll 
be solved in a better way with Randr 1.3, so I've reassigned the bug to 
Keith.

Thanks,
Jesse