[PATCH 6/6] drm/i915: Work around Parad Tech LSPCON adaptor wake-up bug

Imre Deak imre.deak at intel.com
Wed Nov 16 20:44:10 UTC 2016


At least on one SKL with a Parad Tech PS175 adaptor (see Reference:) there
is a problem where the firmware occasionally ignores the offset part of I2C
transfers, so any register access can end up targeting address 0.

The problem can be easily triggered by a sequence of I2C transfers after
the adaptor entered low-power state in LS mode. Trying to wake up the
device first by setting DP_DUAL_MODE_TMDS_OEN to 0 doesn't get rid of
the problem. After some testing my assumption is that during LSPCON
probing the adaptor is in this semi-broken state for a period of time
where it cannot handle back-to-back I2C accesses.

One thing that got rid of the problem for me is to add a 50usec delay
between each transfer, so let's try this as an adaptor specific
workaround.

Reference: https://bugs.freedesktop.org/show_bug.cgi?id=98353
Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
Cc: Shashank Sharma <shashank.sharma at intel.com>
Signed-off-by: Imre Deak <imre.deak at intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h    |  1 +
 drivers/gpu/drm/i915/intel_lspcon.c | 42 +++++++++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 8060ee5..3d518d0 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -962,6 +962,7 @@ struct intel_lspcon {
 	enum drm_lspcon_mode mode;
 	bool desc_valid;
 	struct drm_dp_dual_mode_adaptor dual_mode_adaptor;
+	bool ddc_delay_wa;
 };
 
 struct intel_digital_port {
diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
index d75ca05..aec52da 100644
--- a/drivers/gpu/drm/i915/intel_lspcon.c
+++ b/drivers/gpu/drm/i915/intel_lspcon.c
@@ -27,6 +27,12 @@
 #include <drm/drm_dp_dual_mode_helper.h>
 #include "intel_drv.h"
 
+static void lspcon_ddc_delay_wa(struct intel_lspcon *lspcon)
+{
+	if (lspcon->ddc_delay_wa)
+		usleep_range(50, 100);
+}
+
 static struct intel_dp *lspcon_to_intel_dp(struct intel_lspcon *lspcon)
 {
 	struct intel_digital_port *dig_port =
@@ -39,6 +45,7 @@ static enum drm_lspcon_mode lspcon_get_current_mode(struct intel_lspcon *lspcon)
 {
 	enum drm_lspcon_mode current_mode = DRM_LSPCON_MODE_INVALID;
 
+	lspcon_ddc_delay_wa(lspcon);
 	if (drm_lspcon_get_mode(&lspcon->dual_mode_adaptor, &current_mode))
 		DRM_ERROR("Error reading LSPCON mode\n");
 	else
@@ -55,6 +62,7 @@ static int lspcon_change_mode(struct intel_lspcon *lspcon,
 	struct drm_dp_dual_mode_adaptor *dual_mode_adaptor =
 		&lspcon->dual_mode_adaptor;
 
+	lspcon_ddc_delay_wa(lspcon);
 	err = drm_lspcon_get_mode(dual_mode_adaptor, &current_mode);
 	if (err) {
 		DRM_ERROR("Error reading LSPCON mode\n");
@@ -66,6 +74,7 @@ static int lspcon_change_mode(struct intel_lspcon *lspcon,
 		return 0;
 	}
 
+	lspcon_ddc_delay_wa(lspcon);
 	err = drm_lspcon_set_mode(dual_mode_adaptor, mode);
 	if (err < 0) {
 		DRM_ERROR("LSPCON mode change failed\n");
@@ -77,6 +86,30 @@ static int lspcon_change_mode(struct intel_lspcon *lspcon,
 	return 0;
 }
 
+#define DUAL_MODE_HW_REV_VAL(major, minor) (((major) << 4) | (minor))
+#define DUAL_MODE_SW_REV_VAL(major, minor) ((major) << 8 | (minor))
+
+static bool adaptor_needs_ddc_delay_wa(struct drm_dp_dual_mode_adaptor *adaptor)
+{
+	const uint8_t oui[] = { 0x00, 0x1c, 0xf8 };
+	const char device_id[6] = "PS1750";
+	struct drm_dp_dual_mode_desc *desc = &adaptor->regs.desc;
+
+	if (memcmp(desc->oui, oui, sizeof(oui)))
+		return false;
+
+	if (strncmp(desc->device_id, device_id, sizeof(device_id)))
+		return false;
+
+	if (desc->hw_rev > DUAL_MODE_HW_REV_VAL(0xb, 0))
+		return false;
+
+	if (DUAL_MODE_SW_REV_VAL(desc->sw_major_rev, desc->sw_minor_rev) > 0)
+		return false;
+
+	return true;
+}
+
 static bool lspcon_probe(struct intel_lspcon *lspcon)
 {
 	struct i2c_adapter *i2c_adapter = &lspcon_to_intel_dp(lspcon)->aux.ddc;
@@ -91,6 +124,15 @@ static bool lspcon_probe(struct intel_lspcon *lspcon)
 		return false;
 	}
 
+	/*
+	 * On some buggy adaptors we need to insert a delay between consecutive
+	 * DDC transfers to avoid errors where the adaptor firmware ignores the
+	 * transfer offset value ending up reading/writing always at offset 0.
+	 */
+	lspcon->ddc_delay_wa = adaptor_needs_ddc_delay_wa(dual_mode_adaptor);
+	if (lspcon->ddc_delay_wa)
+		DRM_DEBUG_KMS("Adaptor needs DDC delay workaround\n");
+
 	/* Yay ... got a LSPCON device */
 	DRM_DEBUG_KMS("LSPCON detected\n");
 	lspcon->mode = lspcon_get_current_mode(lspcon);
-- 
2.5.0



More information about the Intel-gfx-trybot mailing list