drm/i915: Check live status before reading edid

Sonika Jindal · danvet · commit 237ed86c693d · 2015-09-23T10:18:45.000+02:00
The Bspec is very clear that Live status must be checked about before
trying to read EDID over DDC channel. This patch makes sure that HDMI
EDID is read only when live status is up.

The live status doesn't seem to perform very consistent across various
platforms when tested with different monitors. The reason behind that is
some monitors are late to provide right voltage to set live_status up.
So, after getting the interrupt, for a small duration, live status reg
fluctuates, and then settles down showing the correct staus.

This is explained here in, in a rough way:
HPD line  ________________
			 |\ T1 = Monitor Hotplug causing IRQ
			 | \______________________________________
			 | |
                         | |
			 | |   T2 = Live status is stable
			 | |  _____________________________________
			 | | /|
Live status _____________|_|/ |
			 | |  |
			 | |  |
			 | |  |
			T0 T1  T2

(Between T1 and T2 Live status fluctuates or can be even low, depending on
 the monitor)

After several experiments, we have concluded that a max delay
of 30ms is enough to allow the live status to settle down with
most of the monitors. This total delay of 30ms has been split into
a resolution of 3 retries of 10ms each, for the better cases.

This delay is kept at 30ms, keeping in consideration that, HDCP compliance
expect the HPD handler to respond a plug out in 100ms, by disabling port.

v2: Adding checks for VLV/CHV as well. Reusing old ibx and g4x functions
to check digital port status. Adding a separate function to get bxt live
status (Daniel)
v3: Using intel_encoder-&gt;hpd_pin to check the live status (Siva)
Moving the live status read to intel_hdmi_probe and passing parameter
to read/not to read the edid. (me)
v4:
* Added live status check for all platforms using
intel_digital_port_connected.
* Rebased on top of Jani's DP cleanup series
* Some monitors take time in setting the live status. So retry for few
times if this is a connect HPD
v5: Removed extra "drm/i915" from commit message. Adding Shashank's sob
    which was missed.
v6: Drop the (!detect_edid &amp;&amp; !live_status check) check because for DDI
ports which are enumerated as hdmi as well as DP, we don't have a
mechanism to differentiate between DP and hdmi inside the encoder's
hot_plug. This leads to call to the hdmi's hot_plug hook for DP as well
as hdmi which leads to issues during unplug because of the above check.
v7: Make intel_digital_port_connected global in this patch, some
reformatting of while loop, adding a print when live status is not
up. (Rodrigo)
v8: Rebase it on nightly which involved skipping the hot_plug hook for now
and letting the live_status check happen in detect until the hpd handling
part is finalized (Daniel)

Signed-off-by: Shashank Sharma &lt;shashank.sharma@intel.com&gt;
Signed-off-by: Sonika Jindal &lt;sonika.jindal@intel.com&gt;
Reviewed-by: Rodrigo Vivi &lt;rodrigo.vivi@intel.com&gt;
Signed-off-by: Daniel Vetter &lt;daniel.vetter@ffwll.ch&gt;
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
@@ -4672,7 +4672,7 @@ static bool bxt_digital_port_connected(struct drm_i915_private *dev_priv,
  *
  * Return %true if @port is connected, %false otherwise.
  */
-static bool intel_digital_port_connected(struct drm_i915_private *dev_priv,
+bool intel_digital_port_connected(struct drm_i915_private *dev_priv,
 					 struct intel_digital_port *port)
 {
 	if (HAS_PCH_IBX(dev_priv))
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
@@ -1218,6 +1218,8 @@ void intel_edp_drrs_disable(struct intel_dp *intel_dp);
 void intel_edp_drrs_invalidate(struct drm_device *dev,
 		unsigned frontbuffer_bits);
 void intel_edp_drrs_flush(struct drm_device *dev, unsigned frontbuffer_bits);
+bool intel_digital_port_connected(struct drm_i915_private *dev_priv,
+					 struct intel_digital_port *port);
 void hsw_dp_set_ddi_pll_sel(struct intel_crtc_state *pipe_config);
 
 /* intel_dp_mst.c */
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1329,22 +1329,23 @@ intel_hdmi_unset_edid(struct drm_connector *connector)
 }
 
 static bool
-intel_hdmi_set_edid(struct drm_connector *connector)
+intel_hdmi_set_edid(struct drm_connector *connector, bool force)
 {
 	struct drm_i915_private *dev_priv = to_i915(connector->dev);
 	struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
 	struct intel_encoder *intel_encoder =
 		&hdmi_to_dig_port(intel_hdmi)->base;
 	enum intel_display_power_domain power_domain;
-	struct edid *edid;
+	struct edid *edid = NULL;
 	bool connected = false;
 
 	power_domain = intel_display_port_power_domain(intel_encoder);
 	intel_display_power_get(dev_priv, power_domain);
 
-	edid = drm_get_edid(connector,
-			    intel_gmbus_get_adapter(dev_priv,
-						    intel_hdmi->ddc_bus));
+	if (force)
+		edid = drm_get_edid(connector,
+				    intel_gmbus_get_adapter(dev_priv,
+				    intel_hdmi->ddc_bus));
 
 	intel_display_power_put(dev_priv, power_domain);
 
@@ -1372,13 +1373,26 @@ static enum drm_connector_status
 intel_hdmi_detect(struct drm_connector *connector, bool force)
 {
 	enum drm_connector_status status;
+	struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
+	struct drm_i915_private *dev_priv = to_i915(connector->dev);
+	bool live_status = false;
+	unsigned int retry = 3;
 
 	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
 		      connector->base.id, connector->name);
 
+	while (!live_status && --retry) {
+		live_status = intel_digital_port_connected(dev_priv,
+				hdmi_to_dig_port(intel_hdmi));
+		mdelay(10);
+	}
+
+	if (!live_status)
+		DRM_DEBUG_KMS("Live status not up!");
+
 	intel_hdmi_unset_edid(connector);
 
-	if (intel_hdmi_set_edid(connector)) {
+	if (intel_hdmi_set_edid(connector, live_status)) {
 		struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
 
 		hdmi_to_dig_port(intel_hdmi)->base.type = INTEL_OUTPUT_HDMI;
@@ -1402,7 +1416,7 @@ intel_hdmi_force(struct drm_connector *connector)
 	if (connector->status != connector_status_connected)
 		return;
 
-	intel_hdmi_set_edid(connector);
+	intel_hdmi_set_edid(connector, true);
 	hdmi_to_dig_port(intel_hdmi)->base.type = INTEL_OUTPUT_HDMI;
 }
 

Original file line number	Diff line number	Diff line change
`@@ -4672,7 +4672,7 @@ static bool bxt_digital_port_connected(struct drm_i915_private *dev_priv,`
`4672`	`4672`	`*`
`4673`	`4673`	`* Return %true if @port is connected, %false otherwise.`
`4674`	`4674`	`*/`
`4675`		`-static bool intel_digital_port_connected(struct drm_i915_private *dev_priv,`
	`4675`	`+bool intel_digital_port_connected(struct drm_i915_private *dev_priv,`
`4676`	`4676`	`struct intel_digital_port *port)`
`4677`	`4677`	`{`
`4678`	`4678`	`if (HAS_PCH_IBX(dev_priv))`