Re: [PATCH 1/4] drm/i915: Configure the TV sense state correctly on GM45 to make TV detection reliable

[ykzhao <yakui.zhao@intel.com>]

On Sat, 2010-04-10 at 05:14 +0800, Eric Anholt wrote:
> On Wed,  7 Apr 2010 17:11:19 +0800, Zhenyu Wang <zhenyuw@linux.intel.com> wrote:
> > From: ykzhao <yakui.zhao@intel.com>
> > 
> > The TV detection logic is not reliable on the Cantiga platform.
> > Sometimes the TV will be misdetected as the following two cases:
> > - TV is misdetected on some laptops. e.g. There is no TV connector
> > port or no TV is attached. But the TV is shown as connected.
> > - TV connector type is misdetected. e.g. the component TV is
> > attached, but the TV is shown as S-video type.
> > 
> > According to the hardware requirement, the TV sense state bits of TV DAC
> > register should be cleared to zero on Cantiga platfrom.
> > 
> > https://bugzilla.kernel.org/show_bug.cgi?id=14792
> 
> As far as I can tell from reading the specs, this patch just completely
> breaks TV detect on GM45.  And the logic of "set all the bits for the
> register setting we're going to do and then if it's gm45 skip a few of
> them" is unintuitive and backwards, even if it was correct.\\

This is from bios code. And the bios team also helps to confirm that TV
DAC sense state bits should be cleared to zero on cantiga platform.

How about the following commit log?

   The TV detection logic is not reliable on the cantiga platform. Sometimes the
TV will be misdetected as the following two cases:
   a. TV is misdetected on some laptops. e.g. There is no TV connector
port or no TV is attaced. But the TV is shown as connected.
   b. TV connector type is misdetected. E.g. the component TV is
attached, but the TV is shown as S-video type.

It is confirmed that in bios code the TV DAC sense bits should be cleared
to zero on GM45 in course of TV detection, which is different with other
platforms. It uses the following conditional definition:
  IF   CTG
    TVDAC_SENSE_CTL EQU 0               ; Cantiga to Low level
  ELSE
    TVDAC_SENSE_CTL EQU BIT27 + BIT26 + BIT25 + BIT24
  ENDIF         ; CTG

https://bugzilla.kernel.org/show_bug.cgi?id=14792

> 
> You don't get to just say "according to the hardware requirement."  You
> need to refer to either the b-spec (so someone can confirm what you
> read) or BIOS traces (so someone knows how you did it) or to a personal
> conversation with a HW engineer (so people can be appropriately
> dubious), or to randomly shuffling bits of the registers until people
> reported that their bug was fixed, as this patch appears to do.