[PATCH v3] drm/i915: intel_dp_link_is_valid() should only return status of link

* [PATCH v3] drm/i915: intel_dp_link_is_valid() should only return status of link
@ 2016-08-13  1:19 Manasi Navare
  2016-08-13  6:09 ` ✗ Ro.CI.BAT: failure for drm/i915: intel_dp_link_is_valid() should only return status of link (rev3) Patchwork
  0 siblings, 1 reply; 2+ messages in thread
From: Manasi Navare @ 2016-08-13  1:19 UTC (permalink / raw)
  To: intel-gfx; +Cc: dhinakaran.pandiyan

Intel_dp_link_is_valid() function reads the Link status registers
and returns a boolean to indicate link is valid or not.
If the link has lost lock and is not valid any more, link
training is performed outside the function else previously trained link
is retained.
This gives us flexibility of checking whether link is valid and training
it independently.

v3:
* Removed some unnecessary DEBUG prints
* Optimized the conditional checking (Dhinakaran Pandiyan)
v2:
* Changed the function name from intel_dp_check_link_status()
to intel_dp_link_is_valid()  (Lukas Wunner)
* Checks for CRTC and active CRTC are moved outside the
intel_dp_link_is_valid() function (Rodrigo Vivi)

Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 53 ++++++++++++++++++++++++++---------------
 1 file changed, 34 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 364db90..d234042 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3881,36 +3881,32 @@ go_again:
 	return -EINVAL;
 }
 
-static void
-intel_dp_check_link_status(struct intel_dp *intel_dp)
+static bool
+intel_dp_link_is_valid(struct intel_dp *intel_dp)
 {
-	struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)->base;
 	struct drm_device *dev = intel_dp_to_dev(intel_dp);
 	u8 link_status[DP_LINK_STATUS_SIZE];
 
 	WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
 
 	if (!intel_dp_get_link_status(intel_dp, link_status)) {
-		DRM_ERROR("Failed to get link status\n");
-		return;
+		DRM_DEBUG_KMS("Failed to get link status\n");
+		return false;
 	}
 
-	if (!intel_encoder->base.crtc)
-		return;
+	/* Check if the link is valid by reading the bits of Link status
+	 * registers
+	 */
+	if (!drm_dp_channel_eq_ok(link_status, intel_dp->lane_count)) {
+		DRM_DEBUG_KMS("Channel EQ or CR not ok, need to retrain\n");
+		return false;
+	}
 
-	if (!to_intel_crtc(intel_encoder->base.crtc)->active)
-		return;
+	return true;
 
-	/* if link training is requested we should perform it always */
-	if ((intel_dp->compliance_test_type == DP_TEST_LINK_TRAINING) ||
-	    (!drm_dp_channel_eq_ok(link_status, intel_dp->lane_count))) {
-		DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n",
-			      intel_encoder->base.name);
-		intel_dp_start_link_train(intel_dp);
-		intel_dp_stop_link_train(intel_dp);
-	}
 }
 
+
 /*
  * According to DP spec
  * 5.1.2:
@@ -3928,6 +3924,8 @@ static bool
 intel_dp_short_pulse(struct intel_dp *intel_dp)
 {
 	struct drm_device *dev = intel_dp_to_dev(intel_dp);
+	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+	struct intel_encoder *intel_encoder = &intel_dig_port->base;
 	u8 sink_irq_vector = 0;
 	u8 old_sink_count = intel_dp->sink_count;
 	bool ret;
@@ -3968,8 +3966,17 @@ intel_dp_short_pulse(struct intel_dp *intel_dp)
 			DRM_DEBUG_DRIVER("CP or sink specific irq unhandled\n");
 	}
 
+	/* Do not train the link if there is no crtc */
+	if (!intel_encoder->base.crtc ||
+	    !to_intel_crtc(intel_encoder->base.crtc)->active)
+		return true;
+
 	drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
-	intel_dp_check_link_status(intel_dp);
+	if (!intel_dp_link_is_valid(intel_dp) ||
+	    intel_dp->compliance_test_type == DP_TEST_LINK_TRAINING) {
+		intel_dp_start_link_train(intel_dp);
+		intel_dp_stop_link_train(intel_dp);
+	}
 	drm_modeset_unlock(&dev->mode_config.connection_mutex);
 
 	return true;
@@ -4298,8 +4305,16 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
 		 * check links status, there has been known issues of
 		 * link loss triggerring long pulse!!!!
 		 */
+		/* Do not train the link if there is no crtc */
+		if (!intel_encoder->base.crtc ||
+		    !to_intel_crtc(intel_encoder->base.crtc)->active)
+			goto out;
+
 		drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
-		intel_dp_check_link_status(intel_dp);
+		if (!intel_dp_link_is_valid(intel_dp)) {
+			intel_dp_start_link_train(intel_dp);
+			intel_dp_stop_link_train(intel_dp);
+		}
 		drm_modeset_unlock(&dev->mode_config.connection_mutex);
 		goto out;
 	}
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 2+ messages in thread