[PATCH] drm/i915: fixup TV load detect

[PATCH] drm/i915: fixup TV load detect

[Daniel Vetter]

The docs recommend that we use one high sense to detect composite and
S-Video, and low/inverted sense for component (or general output
detection, but we don't bother with that).

This might or might not finally clear up the confusion around TV
detection. Historically we've flip-flopped an aweful lot between high
and low sense, with varying amounts of justification (usually none
that would have survived scrunitation). The last just change was

commit d42c9e2c24f7e7897405b85816bdf4ac924881c0
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Sun Mar 25 22:56:14 2012 +0200

    drm/i915: reinstate GM45 TV detection fix

Motivated by the last TV detection bug I could find in our bugzilla.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=31519
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_tv.c |   56 ++++++++++++++++++++++++---------------
 1 file changed, 34 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index 62bb048..b8d783f 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -1167,7 +1167,8 @@ static const struct drm_display_mode reported_modes[] = {
  */
 static int
 intel_tv_detect_type(struct intel_tv *intel_tv,
-		      struct drm_connector *connector)
+		     struct drm_connector *connector,
+		     bool invert_sense)
 {
 	struct drm_encoder *encoder = &intel_tv->base.base;
 	struct drm_crtc *crtc = encoder->crtc;
@@ -1201,9 +1202,6 @@ intel_tv_detect_type(struct intel_tv *intel_tv,
 
 	tv_dac &= ~(TVDAC_SENSE_MASK | DAC_A_MASK | DAC_B_MASK | DAC_C_MASK);
 	tv_dac |= (TVDAC_STATE_CHG_EN |
-		   TVDAC_A_SENSE_CTL |
-		   TVDAC_B_SENSE_CTL |
-		   TVDAC_C_SENSE_CTL |
 		   DAC_CTL_OVERRIDE |
 		   DAC_A_0_7_V |
 		   DAC_B_0_7_V |
@@ -1211,12 +1209,13 @@ intel_tv_detect_type(struct intel_tv *intel_tv,
 
 
 	/*
-	 * The TV sense state should be cleared to zero on cantiga platform. Otherwise
-	 * the TV is misdetected. This is hardware requirement.
+	 * We need to have two runs with opposite sense, since otherwise we
+	 * won't be able to differentiate 
 	 */
-	if (IS_GM45(dev))
-		tv_dac &= ~(TVDAC_STATE_CHG_EN | TVDAC_A_SENSE_CTL |
-			    TVDAC_B_SENSE_CTL | TVDAC_C_SENSE_CTL);
+	if (invert_sense)
+		tv_dac &= ~(TVDAC_A_SENSE_CTL | TVDAC_B_SENSE_CTL | TVDAC_C_SENSE_CTL);
+	else
+		tv_dac |= TVDAC_A_SENSE_CTL | TVDAC_B_SENSE_CTL | TVDAC_C_SENSE_CTL;
 
 	I915_WRITE(TV_CTL, tv_ctl);
 	I915_WRITE(TV_DAC, tv_dac);
@@ -1229,23 +1228,28 @@ intel_tv_detect_type(struct intel_tv *intel_tv,
 	tv_dac = I915_READ(TV_DAC);
 	DRM_DEBUG_KMS("TV detected: %x, %x\n", tv_ctl, tv_dac);
 	/*
+	 *  TV sense for non-inverted sense controls ...
+	 *
 	 *  A B C
 	 *  0 1 1 Composite
 	 *  1 0 X svideo
-	 *  0 0 0 Component
+	 *  0 0 0 Component (test with inverted sense)
 	 */
-	if ((tv_dac & TVDAC_SENSE_MASK) == (TVDAC_B_SENSE | TVDAC_C_SENSE)) {
-		DRM_DEBUG_KMS("Detected Composite TV connection\n");
-		type = DRM_MODE_CONNECTOR_Composite;
-	} else if ((tv_dac & (TVDAC_A_SENSE|TVDAC_B_SENSE)) == TVDAC_A_SENSE) {
-		DRM_DEBUG_KMS("Detected S-Video TV connection\n");
-		type = DRM_MODE_CONNECTOR_SVIDEO;
-	} else if ((tv_dac & TVDAC_SENSE_MASK) == 0) {
-		DRM_DEBUG_KMS("Detected Component TV connection\n");
-		type = DRM_MODE_CONNECTOR_Component;
+	if (invert_sense) {
+		if ((tv_dac & TVDAC_SENSE_MASK) == TVDAC_SENSE_MASK) {
+			DRM_DEBUG_KMS("Detected Component TV connection\n");
+			type = DRM_MODE_CONNECTOR_Component;
+		}
 	} else {
-		DRM_DEBUG_KMS("Unrecognised TV connection\n");
-		type = -1;
+		if ((tv_dac & TVDAC_SENSE_MASK) ==
+		    (TVDAC_B_SENSE | TVDAC_C_SENSE)) {
+			DRM_DEBUG_KMS("Detected Composite TV connection\n");
+			type = DRM_MODE_CONNECTOR_Composite;
+		} else if ((tv_dac & (TVDAC_A_SENSE|TVDAC_B_SENSE)) ==
+			   TVDAC_A_SENSE) {
+			DRM_DEBUG_KMS("Detected S-Video TV connection\n");
+			type = DRM_MODE_CONNECTOR_SVIDEO;
+		}
 	}
 
 	I915_WRITE(TV_DAC, save_tv_dac & ~TVDAC_STATE_CHG_EN);
@@ -1315,7 +1319,15 @@ intel_tv_detect(struct drm_connector *connector, bool force)
 		struct intel_load_detect_pipe tmp;
 
 		if (intel_get_load_detect_pipe(connector, &mode, &tmp)) {
-			type = intel_tv_detect_type(intel_tv, connector);
+			/*
+			 * We need to run the load detection twice, since we
+			 * can't detect all output types correctly with just one
+			 * run.
+			 */
+			type = intel_tv_detect_type(intel_tv, connector, false);
+			if (type == -1)
+				type = intel_tv_detect_type(intel_tv, connector,
+							    true);
 			intel_release_load_detect_pipe(connector, &tmp);
 		} else
 			return connector_status_unknown;
-- 
1.7.10.4

Re: [PATCH] drm/i915: fixup TV load detect

[Daniel Vetter]

On Tue, Nov 13, 2012 at 02:23:17PM +0100, Daniel Vetter wrote:
> The docs recommend that we use one high sense to detect composite and
> S-Video, and low/inverted sense for component (or general output
> detection, but we don't bother with that).
> 
> This might or might not finally clear up the confusion around TV
> detection. Historically we've flip-flopped an aweful lot between high
> and low sense, with varying amounts of justification (usually none
> that would have survived scrunitation). The last just change was
> 
> commit d42c9e2c24f7e7897405b85816bdf4ac924881c0
> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> Date:   Sun Mar 25 22:56:14 2012 +0200
> 
>     drm/i915: reinstate GM45 TV detection fix
> 
> Motivated by the last TV detection bug I could find in our bugzilla.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=31519
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

This actually makes matters worse as I've just found out on my gm45 - now
it detects a spurious TV. Back to the drawing board.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch