drm/i915: Don't register CRT connector when it's fused off

Commit Message

Ville Syrjala Nov. 20, 2015, 8:35 p.m. UTC

From: Ville Syrjälä <[e-mail]>

On some machines the CRT connector may be fused off. The weird thing
about this setup is that the ADPA register works otherwise normally,
except the enable bit is hardwired to 0. No one knows of any fuse
register that would tell us if this is the case, so the only thing we
can do (apart from a blacklist) is to try and set the enable bit and see
if it sticks. If not, we don't register the connector at all. Obviously
if the bit is already set when loading the driver we can just assume it
works.

I've smoke tested this approach on several machines (GMCH and PCH),
some with actual CRT connectors, some with shadow connectors, and
obviously the machine (IVB) with the fused off connector. So far
I've not seen any ill effects from this probe.

The main benefit is that we can actually run igt on machines with
fused off connectors, without totally upsetting the state checker.

Signed-off-by: Ville Syrjälä <[e-mail]>
---
 drivers/gpu/drm/i915/intel_crt.c | 33 +++++++++++++++++++++++++++------
 1 file changed, 27 insertions(+), 6 deletions(-)

Comments

Chris Wilson Nov. 21, 2015, 10:44 a.m. UTC | #1

On Fri, Nov 20, 2015 at 10:35:41PM +0200, [e-mail] wrote:
> From: Ville Syrjälä <[e-mail]>
> 
> On some machines the CRT connector may be fused off. The weird thing
> about this setup is that the ADPA register works otherwise normally,
> except the enable bit is hardwired to 0. No one knows of any fuse
> register that would tell us if this is the case, so the only thing we
> can do (apart from a blacklist) is to try and set the enable bit and see
> if it sticks. If not, we don't register the connector at all. Obviously
> if the bit is already set when loading the driver we can just assume it
> works.
> 
> I've smoke tested this approach on several machines (GMCH and PCH),
> some with actual CRT connectors, some with shadow connectors, and
> obviously the machine (IVB) with the fused off connector. So far
> I've not seen any ill effects from this probe.
> 
> The main benefit is that we can actually run igt on machines with
> fused off connectors, without totally upsetting the state checker.
> 
> Signed-off-by: Ville Syrjälä <[e-mail]>

If we can't enable the VGA port, there's not much we can do with it.

Acked-by: Chris Wilson <[e-mail]>

I'm just a bit hesistant, in case there are machine out there that
simply do not report the hw state back (doubtful) or there are people
who make use of the false VGA as a fake output (like me, or perhaps
kvm-over-ip). I'll survive, I just use a fake HDMI output if there is no
VGA.
-Chris

Ville Syrjala Nov. 23, 2015, 3:07 p.m. UTC | #2

On Sat, Nov 21, 2015 at 10:44:12AM +0000, Chris Wilson wrote:
> On Fri, Nov 20, 2015 at 10:35:41PM +0200, [e-mail] wrote:
> > From: Ville Syrjälä <[e-mail]>
> > 
> > On some machines the CRT connector may be fused off. The weird thing
> > about this setup is that the ADPA register works otherwise normally,
> > except the enable bit is hardwired to 0. No one knows of any fuse
> > register that would tell us if this is the case, so the only thing we
> > can do (apart from a blacklist) is to try and set the enable bit and see
> > if it sticks. If not, we don't register the connector at all. Obviously
> > if the bit is already set when loading the driver we can just assume it
> > works.
> > 
> > I've smoke tested this approach on several machines (GMCH and PCH),
> > some with actual CRT connectors, some with shadow connectors, and
> > obviously the machine (IVB) with the fused off connector. So far
> > I've not seen any ill effects from this probe.
> > 
> > The main benefit is that we can actually run igt on machines with
> > fused off connectors, without totally upsetting the state checker.
> > 
> > Signed-off-by: Ville Syrjälä <[e-mail]>
> 
> If we can't enable the VGA port, there's not much we can do with it.
> 
> Acked-by: Chris Wilson <[e-mail]>
> 
> I'm just a bit hesistant, in case there are machine out there that
> simply do not report the hw state back (doubtful) or there are people
> who make use of the false VGA as a fake output (like me, or perhaps
> kvm-over-ip). I'll survive, I just use a fake HDMI output if there is no
> VGA.

Based on my experience most machines that lack the physical connector
still have a fully working ADPA register. I have two such machines
myself. There's just this one machine at our office where it's
totally fused off, and running igt on it is no fun unless we get rid
of the connector.

Ville Syrjala Nov. 24, 2015, 2:59 p.m. UTC | #3

On Mon, Nov 23, 2015 at 05:07:29PM +0200, Ville Syrjälä wrote:
> On Sat, Nov 21, 2015 at 10:44:12AM +0000, Chris Wilson wrote:
> > On Fri, Nov 20, 2015 at 10:35:41PM +0200, [e-mail] wrote:
> > > From: Ville Syrjälä <[e-mail]>
> > > 
> > > On some machines the CRT connector may be fused off. The weird thing
> > > about this setup is that the ADPA register works otherwise normally,
> > > except the enable bit is hardwired to 0. No one knows of any fuse
> > > register that would tell us if this is the case, so the only thing we
> > > can do (apart from a blacklist) is to try and set the enable bit and see
> > > if it sticks. If not, we don't register the connector at all. Obviously
> > > if the bit is already set when loading the driver we can just assume it
> > > works.
> > > 
> > > I've smoke tested this approach on several machines (GMCH and PCH),
> > > some with actual CRT connectors, some with shadow connectors, and
> > > obviously the machine (IVB) with the fused off connector. So far
> > > I've not seen any ill effects from this probe.
> > > 
> > > The main benefit is that we can actually run igt on machines with
> > > fused off connectors, without totally upsetting the state checker.
> > > 
> > > Signed-off-by: Ville Syrjälä <[e-mail]>
> > 
> > If we can't enable the VGA port, there's not much we can do with it.
> > 
> > Acked-by: Chris Wilson <[e-mail]>
> > 
> > I'm just a bit hesistant, in case there are machine out there that
> > simply do not report the hw state back (doubtful) or there are people
> > who make use of the false VGA as a fake output (like me, or perhaps
> > kvm-over-ip). I'll survive, I just use a fake HDMI output if there is no
> > VGA.
> 
> Based on my experience most machines that lack the physical connector
> still have a fully working ADPA register. I have two such machines
> myself. There's just this one machine at our office where it's
> totally fused off, and running igt on it is no fun unless we get rid
> of the connector.

Patch merged, thanks for the ack.

Patch

diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 27b3e610e8f0..12008af797bd 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -777,11 +777,37 @@  void intel_crt_init(struct drm_device *dev)
 	struct intel_crt *crt;
 	struct intel_connector *intel_connector;
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	i915_reg_t adpa_reg;
+	u32 adpa;
 
 	/* Skip machines without VGA that falsely report hotplug events */
 	if (dmi_check_system(intel_no_crt))
 		return;
 
+	if (HAS_PCH_SPLIT(dev))
+		adpa_reg = PCH_ADPA;
+	else if (IS_VALLEYVIEW(dev))
+		adpa_reg = VLV_ADPA;
+	else
+		adpa_reg = ADPA;
+
+	adpa = I915_READ(adpa_reg);
+	if ((adpa & ADPA_DAC_ENABLE) == 0) {
+		/*
+		 * On some machines (some IVB at least) CRT can be
+		 * fused off, but there's no known fuse bit to
+		 * indicate that. On these machine the ADPA register
+		 * works normally, except the DAC enable bit won't
+		 * take. So the only way to tell is attempt to enable
+		 * it and see what happens.
+		 */
+		I915_WRITE(adpa_reg, adpa | ADPA_DAC_ENABLE |
+			   ADPA_HSYNC_CNTL_DISABLE | ADPA_VSYNC_CNTL_DISABLE);
+		if ((I915_READ(adpa_reg) & ADPA_DAC_ENABLE) == 0)
+			return;
+		I915_WRITE(adpa_reg, adpa);
+	}
+
 	crt = kzalloc(sizeof(struct intel_crt), GFP_KERNEL);
 	if (!crt)
 		return;
@@ -815,12 +841,7 @@  void intel_crt_init(struct drm_device *dev)
 		connector->interlace_allowed = 1;
 	connector->doublescan_allowed = 0;
 
-	if (HAS_PCH_SPLIT(dev))
-		crt->adpa_reg = PCH_ADPA;
-	else if (IS_VALLEYVIEW(dev))
-		crt->adpa_reg = VLV_ADPA;
-	else
-		crt->adpa_reg = ADPA;
+	crt->adpa_reg = adpa_reg;
 
 	crt->base.compute_config = intel_crt_compute_config;
 	if (HAS_PCH_SPLIT(dev) && !HAS_DDI(dev)) {