[Intel-gfx] TV detect

[Peter Clifton]

On Tue, 2010-01-05 at 10:48 +0800, ykzhao wrote:
> On Tue, 2010-01-05 at 10:17 +0800, Peter Clifton wrote:
> > Does anyone know what the expected (s-video output) electrical waveforms
> > for a "correct" load-detect look like (with / without output connected).

[snip]

> Have you tried to add more waiting time and see whether the TV is
> detected correctly on your box?

This patch seems to have some effect. (Although it could just be in one
of its "working" moods at the moment. It was flaky just before I
rebooted, and hasn't mis-detected since I applied this patch:

diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index 1d5b9b7..ee92335 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -1391,8 +1391,8 @@ intel_tv_detect_type (struct drm_crtc *crtc, struct intel_
        tv_dac &= ~DAC_A_MASK;
        tv_dac &= ~DAC_B_MASK;
        tv_dac &= ~DAC_C_MASK;
-       tv_dac |= (TVDAC_STATE_CHG_EN |
-                  TVDAC_A_SENSE_CTL |
+       tv_dac &= ~TVDAC_STATE_CHG_EN;
+       tv_dac |= (TVDAC_A_SENSE_CTL |
                   TVDAC_B_SENSE_CTL |
                   TVDAC_C_SENSE_CTL |
                   DAC_CTL_OVERRIDE |

(We aren't using the DAC state change logic for polling based load
detection, so I figured I might try turning it off).

I'm running: "watch -n 0.5 xrandr" as a trigger for the bug. With the
stock driver I get display glitches, and spurious TV-detection now and
again. With the patch applied, I'm not getting that (at the moment!)


I had this hunk applied too, from Zhenyu Wang:

@@ -1840,8 +1840,6 @@ intel_tv_init(struct drm_device *dev)
        drm_connector_attach_property(connector,
                                   dev->mode_config.tv_bottom_margin_property,
                                   tv_priv->margin[TV_MARGIN_BOTTOM]);
-
-       dev_priv->hotplug_supported_mask |= TV_HOTPLUG_INT_STATUS;
 out:
        drm_sysfs_connector_add(connector);
 }

Although on its own, it didn't have any effect. (I've not tried the
former without the latter).


A few other things I noted..

Adding an extra intel_wait_for_vblank() before the read of the detected
values didn't help.


Kernel log fills up with:
[  418.536217] [drm] TV-16: set mode NTSC 480i 0

Which is the mode setup for the load detect pipe during programming.
This probably ought to be less verbose eventually?


Prior to the load detect code, we disable TV hotplug interrupts with
this:

	/* Disable TV interrupts around load detect or we'll recurse */
	spin_lock_irqsave(&dev_priv->user_irq_lock, irqflags);
	i915_disable_pipestat(dev_priv, 0, PIPE_HOTPLUG_INTERRUPT_ENABLE |
			      PIPE_HOTPLUG_TV_INTERRUPT_ENABLE);
	spin_unlock_irqrestore(&dev_priv->user_irq_lock, irqflags);


I don't think we ever enabled them in the first place.. AIUI, hotplug TV
was disabled, so this interrupt disable can be removed?

And this block:


	/* Restore interrupt config */
	spin_lock_irqsave(&dev_priv->user_irq_lock, irqflags);
	i915_enable_pipestat(dev_priv, 0, PIPE_HOTPLUG_INTERRUPT_ENABLE |
			     PIPE_HOTPLUG_TV_INTERRUPT_ENABLE);
	spin_unlock_irqrestore(&dev_priv->user_irq_lock, irqflags);

Turns it back on again (unconditionally). Presumably we don't want to do
that at all. Perhaps removing these blocks could go into Zhenyu's patch?

Or for robustness, do we need to leave a call to disable the TV hotplug
interrupt "somewhere" in the driver, to ensure that the BIOS didn't
leave them enabled?

Best regards,

-- 
Peter Clifton <pcjc2 at cam.ac.uk>