diff mbox

[05/17] drm/i915/tv: Clear state sense detection for Cantiga

Message ID 1303420712-6369-6-git-send-email-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Chris Wilson April 21, 2011, 9:18 p.m. UTC
From: Zhao Yakui <yakui.zhao@intel.com>

... otherwise the TV type will be misdetected and cause spurious
connections.

This was originally applied as fb8b5a39b6310379d7b54c0c7113703a8eaf4a57
(drm/i915: Configure the TV sense state correctly on GM45 to make TV
detection reliable)

Eric: Shortly after applying this patch you requested it to be reverted,
d4b74bf07873da2e94219a7b67a334fc1c3ce649 (Revert "drm/i915: Configure
the TV sense state correctly on GM45 to make TV), but we have no clear
information just what is broken by this patch and how to resolve it.

Reported-and-tested-by: darkbasic4@gmail.com
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=27168
Signed-off-by: Zhao Yakui <yakui.zhao@intel.com>
Tested-by: Santi <santi@agolina.net>
Cc: Eric Anholt <eric@anholt.net>
---
 drivers/gpu/drm/i915/intel_tv.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

Comments

Eric Anholt April 21, 2011, 11:36 p.m. UTC | #1
On Thu, 21 Apr 2011 22:18:20 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> From: Zhao Yakui <yakui.zhao@intel.com>
> 
> ... otherwise the TV type will be misdetected and cause spurious
> connections.
> 
> This was originally applied as fb8b5a39b6310379d7b54c0c7113703a8eaf4a57
> (drm/i915: Configure the TV sense state correctly on GM45 to make TV
> detection reliable)
> 
> Eric: Shortly after applying this patch you requested it to be reverted,
> d4b74bf07873da2e94219a7b67a334fc1c3ce649 (Revert "drm/i915: Configure
> the TV sense state correctly on GM45 to make TV), but we have no clear
> information just what is broken by this patch and how to resolve it.

The patch, as explained at the time, basically said "TV detection is
unreliable", and the contents of the patch looked like it was turning
off the detection by disabling TVDAC_STATE_CHG_EN.  The tester said that
it successfully made his TV appear disconnected.  Thus it looked to me
like a patch that was just disabling TV detection on this platform in a
roundabout way, and because of that I hadn't meant to send it upstream.

Maybe it actually makes things work (both for not-detecting no TV, and
detecting a real TV).  But I also didn't like the "because HW
requirement", instead of some specific explanation (some reason why we
need low sense level on the channels instead of high, and some reason to
disable tvdac_state_chg_en at the same time) or a pointer at some docs.
Chris Wilson April 22, 2011, 5:28 a.m. UTC | #2
On Thu, 21 Apr 2011 16:36:08 -0700, Eric Anholt <eric@anholt.net> wrote:
> Maybe it actually makes things work (both for not-detecting no TV, and
> detecting a real TV).  But I also didn't like the "because HW
> requirement", instead of some specific explanation (some reason why we
> need low sense level on the channels instead of high, and some reason to
> disable tvdac_state_chg_en at the same time) or a pointer at some docs.

Thanks Eric, that clears up that a lot. So the question is: does TV
detection work at all after the patch? Let's see which is quicker to get
an answer, searching fruitlessly through the docs or asking for testers...
-Chris
Peter Clifton April 22, 2011, 9:44 p.m. UTC | #3
On Thu, 2011-04-21 at 16:36 -0700, Eric Anholt wrote:
> On Thu, 21 Apr 2011 22:18:20 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > From: Zhao Yakui <yakui.zhao@intel.com>
> > 
> > ... otherwise the TV type will be misdetected and cause spurious
> > connections.
> > 
> > This was originally applied as fb8b5a39b6310379d7b54c0c7113703a8eaf4a57
> > (drm/i915: Configure the TV sense state correctly on GM45 to make TV
> > detection reliable)
> > 
> > Eric: Shortly after applying this patch you requested it to be reverted,
> > d4b74bf07873da2e94219a7b67a334fc1c3ce649 (Revert "drm/i915: Configure
> > the TV sense state correctly on GM45 to make TV), but we have no clear
> > information just what is broken by this patch and how to resolve it.

This is an absolute must for my GM45, and I've been patching it back in
ever since it was dropped.

I had meant to follow up after having contacted the original patch
author - who claimed the patch was derived from an internally documented
errata or some such - and the requirements were contrary to the GM45
publicly released documentation.

The one vital testing step I still wasn't able to take is actually
hooking up the S-Video output of this laptop and making sure the patch
did not stop it detecting a connected TV. I don't actually own a TV, but
could get access to one.. if I had an S-Video cable I could do the test!
Niccolò Belli May 20, 2011, 9:16 a.m. UTC | #4
Il 22/04/2011 23:44, Peter Clifton ha scritto:
> This is an absolute must for my GM45, and I've been patching it back in
> ever since it was dropped.

I absolutely agree.

> The one vital testing step I still wasn't able to take is actually
> hooking up the S-Video output of this laptop and making sure the patch
> did not stop it detecting a connected TV. I don't actually own a TV, but
> could get access to one.. if I had an S-Video cable I could do the test!

Did you test it? Unfortunately my laptop (Samsung X360) doesn't have a
tv-out connector :(

Thank you,
Darkbasic
Keith Packard May 20, 2011, 10 a.m. UTC | #5
On Fri, 20 May 2011 11:16:43 +0200, Niccolò Belli <darkbasic4@gmail.com> wrote:

> Did you test it? Unfortunately my laptop (Samsung X360) doesn't have a
> tv-out connector :(

I've posted a couple of patches that make my machines reliably detect TV
connections now. They're the last two patches on the drm-intel-tv-detect
branch in my kernel repository:

git://git.kernel.org/pub/scm/linux/kernel/git/keithp/linux-2.6.git

I'd like to hear whether these resolve the issue for you as they are
known to not break TV detect on at least two machines, one 965GM and one
GM45.
Niccolò Belli May 20, 2011, 1:37 p.m. UTC | #6
Il 20/05/2011 12:00, Keith Packard ha scritto:
> I've posted a couple of patches that make my machines reliably detect TV
> connections now. They're the last two patches on the drm-intel-tv-detect
> branch in my kernel repository:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/keithp/linux-2.6.git
> 
> I'd like to hear whether these resolve the issue for you as they are
> known to not break TV detect on at least two machines, one 965GM and one
> GM45.

Unfortunately it doesn't work....

Any chance that someone will test tv-out with this
(https://bugs.freedesktop.org/attachment.cgi?id=42814) patch so that it
will eventually be pushed before the 2.6.40 merge window closes?
It's a long outstanding regression, since 2.6.33-rc1 :/

Thank you,
Darkbasic
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index 6b22c1d..3047a66 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -1269,6 +1269,15 @@  intel_tv_detect_type (struct intel_tv *intel_tv,
 		   DAC_B_0_7_V |
 		   DAC_C_0_7_V);
 
+
+	/*
+	 * The TV sense state should be cleared to zero on cantiga platform. Otherwise
+	 * the TV is misdetected. This is hardware requirement.
+	 */
+	if (IS_GM45(dev))
+		tv_dac &= ~(TVDAC_STATE_CHG_EN | TVDAC_A_SENSE_CTL |
+			    TVDAC_B_SENSE_CTL | TVDAC_C_SENSE_CTL);
+
 	I915_WRITE(TV_CTL, tv_ctl);
 	I915_WRITE(TV_DAC, tv_dac);
 	POSTING_READ(TV_DAC);