[PATCH 00/10] DDI buf trans cleanup

[PATCH 00/10] DDI buf trans cleanup

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Second batch of goodies extracted from the big DDI encoder->type series.
This one tries to clean up a lot of the mess in DDI buf trans handling.
The end result is DDI buf trans code that isn't reliant on
encoder->type to tell DP vs. HDMI apart, and as a nice bonus we also
bring BXT more in line with the rest of platforms.

Entire series available here (with further work on top):
git://github.com/vsyrjala/linux.git ddi_output_types_3

Ville Syrjälä (10):
  drm/i915: Relocate intel_ddi_get_buf_trans_*() functions
  drm/i915: Extract intel_ddi_get_buf_trans_hdmi()
  drm/i915: Pass the encoder type explicitly to skl_set_iboost()
  drm/i915: Pass the level to intel_prepare_hdmi_ddi_buffers()
  drm/i915: Integrate BXT into intel_ddi_dp_voltage_max()
  drm/i915: Pass encoder type to cnl_ddi_vswing_sequence() explicitly
  drm/i915: Kill off the BXT buf_trans default_index
  drm/i915: Centralize the SKL DDI A/E vs. B/C/D buf trans handling
  drm/i915: Unify error handling for missing DDI buf trans tables
  drm/i915: Drop the redundant hdmi prefix/suffix from a lot of
    variables

 drivers/gpu/drm/i915/intel_ddi.c | 515 ++++++++++++++++++++-------------------
 drivers/gpu/drm/i915/intel_dp.c  |   4 +-
 2 files changed, 265 insertions(+), 254 deletions(-)

-- 
2.13.6

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

[PATCH v2 01/10] drm/i915: Relocate intel_ddi_get_buf_trans_*() functions

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We'll want to use the intel_ddi_get_buf_trans_*() functions a bit
earlier in the file, so move them up. While at it start using them
in the iboost setup to get rid of the platform checks there.

v2: Rebase due to BDW FDI buf trans fix

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 115 +++++++++++++++++++--------------------
 1 file changed, 55 insertions(+), 60 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 8ef65941b8fd..55937abda61f 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -588,6 +588,59 @@ skl_get_buf_trans_hdmi(struct drm_i915_private *dev_priv, int *n_entries)
 	}
 }
 
+static const struct ddi_buf_trans *
+intel_ddi_get_buf_trans_dp(struct drm_i915_private *dev_priv,
+			   int *n_entries)
+{
+	if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv)) {
+		return kbl_get_buf_trans_dp(dev_priv, n_entries);
+	} else if (IS_SKYLAKE(dev_priv)) {
+		return skl_get_buf_trans_dp(dev_priv, n_entries);
+	} else if (IS_BROADWELL(dev_priv)) {
+		*n_entries = ARRAY_SIZE(bdw_ddi_translations_dp);
+		return  bdw_ddi_translations_dp;
+	} else if (IS_HASWELL(dev_priv)) {
+		*n_entries = ARRAY_SIZE(hsw_ddi_translations_dp);
+		return hsw_ddi_translations_dp;
+	}
+
+	*n_entries = 0;
+	return NULL;
+}
+
+static const struct ddi_buf_trans *
+intel_ddi_get_buf_trans_edp(struct drm_i915_private *dev_priv,
+			    int *n_entries)
+{
+	if (IS_GEN9_BC(dev_priv)) {
+		return skl_get_buf_trans_edp(dev_priv, n_entries);
+	} else if (IS_BROADWELL(dev_priv)) {
+		return bdw_get_buf_trans_edp(dev_priv, n_entries);
+	} else if (IS_HASWELL(dev_priv)) {
+		*n_entries = ARRAY_SIZE(hsw_ddi_translations_dp);
+		return hsw_ddi_translations_dp;
+	}
+
+	*n_entries = 0;
+	return NULL;
+}
+
+static const struct ddi_buf_trans *
+intel_ddi_get_buf_trans_fdi(struct drm_i915_private *dev_priv,
+			    int *n_entries)
+{
+	if (IS_BROADWELL(dev_priv)) {
+		*n_entries = ARRAY_SIZE(bdw_ddi_translations_fdi);
+		return bdw_ddi_translations_fdi;
+	} else if (IS_HASWELL(dev_priv)) {
+		*n_entries = ARRAY_SIZE(hsw_ddi_translations_fdi);
+		return hsw_ddi_translations_fdi;
+	}
+
+	*n_entries = 0;
+	return NULL;
+}
+
 static const struct cnl_ddi_buf_trans *
 cnl_get_buf_trans_hdmi(struct drm_i915_private *dev_priv, int *n_entries)
 {
@@ -692,59 +745,6 @@ static int intel_ddi_hdmi_level(struct drm_i915_private *dev_priv, enum port por
 	return hdmi_level;
 }
 
-static const struct ddi_buf_trans *
-intel_ddi_get_buf_trans_dp(struct drm_i915_private *dev_priv,
-			   int *n_entries)
-{
-	if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv)) {
-		return kbl_get_buf_trans_dp(dev_priv, n_entries);
-	} else if (IS_SKYLAKE(dev_priv)) {
-		return skl_get_buf_trans_dp(dev_priv, n_entries);
-	} else if (IS_BROADWELL(dev_priv)) {
-		*n_entries = ARRAY_SIZE(bdw_ddi_translations_dp);
-		return  bdw_ddi_translations_dp;
-	} else if (IS_HASWELL(dev_priv)) {
-		*n_entries = ARRAY_SIZE(hsw_ddi_translations_dp);
-		return hsw_ddi_translations_dp;
-	}
-
-	*n_entries = 0;
-	return NULL;
-}
-
-static const struct ddi_buf_trans *
-intel_ddi_get_buf_trans_edp(struct drm_i915_private *dev_priv,
-			    int *n_entries)
-{
-	if (IS_GEN9_BC(dev_priv)) {
-		return skl_get_buf_trans_edp(dev_priv, n_entries);
-	} else if (IS_BROADWELL(dev_priv)) {
-		return bdw_get_buf_trans_edp(dev_priv, n_entries);
-	} else if (IS_HASWELL(dev_priv)) {
-		*n_entries = ARRAY_SIZE(hsw_ddi_translations_dp);
-		return hsw_ddi_translations_dp;
-	}
-
-	*n_entries = 0;
-	return NULL;
-}
-
-static const struct ddi_buf_trans *
-intel_ddi_get_buf_trans_fdi(struct drm_i915_private *dev_priv,
-			    int *n_entries)
-{
-	if (IS_BROADWELL(dev_priv)) {
-		*n_entries = ARRAY_SIZE(bdw_ddi_translations_fdi);
-		return bdw_ddi_translations_fdi;
-	} else if (IS_HASWELL(dev_priv)) {
-		*n_entries = ARRAY_SIZE(hsw_ddi_translations_fdi);
-		return hsw_ddi_translations_fdi;
-	}
-
-	*n_entries = 0;
-	return NULL;
-}
-
 /*
  * Starting with Haswell, DDI port buffers must be programmed with correct
  * values in advance. This function programs the correct values for
@@ -1801,19 +1801,14 @@ static void skl_ddi_set_iboost(struct intel_encoder *encoder, u32 level)
 		if (dp_iboost) {
 			iboost = dp_iboost;
 		} else {
-			if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv))
-				ddi_translations = kbl_get_buf_trans_dp(dev_priv,
-									&n_entries);
-			else
-				ddi_translations = skl_get_buf_trans_dp(dev_priv,
-									&n_entries);
+			ddi_translations = intel_ddi_get_buf_trans_dp(dev_priv, &n_entries);
 			iboost = ddi_translations[level].i_boost;
 		}
 	} else if (type == INTEL_OUTPUT_EDP) {
 		if (dp_iboost) {
 			iboost = dp_iboost;
 		} else {
-			ddi_translations = skl_get_buf_trans_edp(dev_priv, &n_entries);
+			ddi_translations = intel_ddi_get_buf_trans_edp(dev_priv, &n_entries);
 
 			if (WARN_ON(port != PORT_A &&
 				    port != PORT_E && n_entries > 9))
-- 
2.13.6

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

[PATCH 02/10] drm/i915: Extract intel_ddi_get_buf_trans_hdmi()

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Introduce intel_ddi_get_buf_trans_hdmi() and start using it where we
can.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 50 ++++++++++++++++++++++------------------
 1 file changed, 28 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 55937abda61f..e6c884a6d408 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -641,6 +641,24 @@ intel_ddi_get_buf_trans_fdi(struct drm_i915_private *dev_priv,
 	return NULL;
 }
 
+static const struct ddi_buf_trans *
+intel_ddi_get_buf_trans_hdmi(struct drm_i915_private *dev_priv,
+			     int *n_entries)
+{
+	if (IS_GEN9_BC(dev_priv)) {
+		return skl_get_buf_trans_hdmi(dev_priv, n_entries);
+	} else if (IS_BROADWELL(dev_priv)) {
+		*n_entries = ARRAY_SIZE(bdw_ddi_translations_hdmi);
+		return bdw_ddi_translations_hdmi;
+	} else if (IS_HASWELL(dev_priv)) {
+		*n_entries = ARRAY_SIZE(hsw_ddi_translations_hdmi);
+		return hsw_ddi_translations_hdmi;
+	}
+
+	*n_entries = 0;
+	return NULL;
+}
+
 static const struct cnl_ddi_buf_trans *
 cnl_get_buf_trans_hdmi(struct drm_i915_private *dev_priv, int *n_entries)
 {
@@ -723,18 +741,17 @@ static int intel_ddi_hdmi_level(struct drm_i915_private *dev_priv, enum port por
 		cnl_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
 		hdmi_default_entry = n_hdmi_entries - 1;
 	} else if (IS_GEN9_BC(dev_priv)) {
-		skl_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
+		intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
 		hdmi_default_entry = 8;
 	} else if (IS_BROADWELL(dev_priv)) {
-		n_hdmi_entries = ARRAY_SIZE(bdw_ddi_translations_hdmi);
+		intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
 		hdmi_default_entry = 7;
 	} else if (IS_HASWELL(dev_priv)) {
-		n_hdmi_entries = ARRAY_SIZE(hsw_ddi_translations_hdmi);
+		intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
 		hdmi_default_entry = 6;
 	} else {
 		WARN(1, "ddi translation table missing\n");
-		n_hdmi_entries = ARRAY_SIZE(bdw_ddi_translations_hdmi);
-		hdmi_default_entry = 7;
+		return 0;
 	}
 
 	/* Choose a good default if VBT is badly populated */
@@ -810,23 +827,12 @@ static void intel_prepare_hdmi_ddi_buffers(struct intel_encoder *encoder)
 
 	hdmi_level = intel_ddi_hdmi_level(dev_priv, port);
 
-	if (IS_GEN9_BC(dev_priv)) {
-		ddi_translations_hdmi = skl_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
+	ddi_translations_hdmi = intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
 
-		/* If we're boosting the current, set bit 31 of trans1 */
-		if (dev_priv->vbt.ddi_port_info[port].hdmi_boost_level)
-			iboost_bit = DDI_BUF_BALANCE_LEG_ENABLE;
-	} else if (IS_BROADWELL(dev_priv)) {
-		ddi_translations_hdmi = bdw_ddi_translations_hdmi;
-		n_hdmi_entries = ARRAY_SIZE(bdw_ddi_translations_hdmi);
-	} else if (IS_HASWELL(dev_priv)) {
-		ddi_translations_hdmi = hsw_ddi_translations_hdmi;
-		n_hdmi_entries = ARRAY_SIZE(hsw_ddi_translations_hdmi);
-	} else {
-		WARN(1, "ddi translation table missing\n");
-		ddi_translations_hdmi = bdw_ddi_translations_hdmi;
-		n_hdmi_entries = ARRAY_SIZE(bdw_ddi_translations_hdmi);
-	}
+	/* If we're boosting the current, set bit 31 of trans1 */
+	if (IS_GEN9_BC(dev_priv) &&
+	    dev_priv->vbt.ddi_port_info[port].hdmi_boost_level)
+		iboost_bit = DDI_BUF_BALANCE_LEG_ENABLE;
 
 	/* Entry 9 is for HDMI: */
 	I915_WRITE(DDI_BUF_TRANS_LO(port, 9),
@@ -1820,7 +1826,7 @@ static void skl_ddi_set_iboost(struct intel_encoder *encoder, u32 level)
 		if (hdmi_iboost) {
 			iboost = hdmi_iboost;
 		} else {
-			ddi_translations = skl_get_buf_trans_hdmi(dev_priv, &n_entries);
+			ddi_translations = intel_ddi_get_buf_trans_hdmi(dev_priv, &n_entries);
 			iboost = ddi_translations[level].i_boost;
 		}
 	} else {
-- 
2.13.6

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

[PATCH v2 03/10] drm/i915: Pass the encoder type explicitly to skl_set_iboost()

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

encoder->type isn't reliable for DP/HDMI encoders, so pass the type
explicity to skl_set_iboost(). Also take the opportunity to streamline
the code.

v2: Clean up the argument types to skl_ddi_set_iboost() while at it

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 59 ++++++++++++++++------------------------
 1 file changed, 23 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index e6c884a6d408..cf0b2d3de15f 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1788,49 +1788,36 @@ static void _skl_ddi_set_iboost(struct drm_i915_private *dev_priv,
 	I915_WRITE(DISPIO_CR_TX_BMU_CR0, tmp);
 }
 
-static void skl_ddi_set_iboost(struct intel_encoder *encoder, u32 level)
+static void skl_ddi_set_iboost(struct intel_encoder *encoder,
+			       int level, enum intel_output_type type)
 {
 	struct intel_digital_port *intel_dig_port = enc_to_dig_port(&encoder->base);
 	struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
 	enum port port = intel_dig_port->port;
-	int type = encoder->type;
-	const struct ddi_buf_trans *ddi_translations;
 	uint8_t iboost;
-	uint8_t dp_iboost, hdmi_iboost;
-	int n_entries;
 
-	/* VBT may override standard boost values */
-	dp_iboost = dev_priv->vbt.ddi_port_info[port].dp_boost_level;
-	hdmi_iboost = dev_priv->vbt.ddi_port_info[port].hdmi_boost_level;
+	if (type == INTEL_OUTPUT_HDMI)
+		iboost = dev_priv->vbt.ddi_port_info[port].hdmi_boost_level;
+	else
+		iboost = dev_priv->vbt.ddi_port_info[port].dp_boost_level;
 
-	if (type == INTEL_OUTPUT_DP) {
-		if (dp_iboost) {
-			iboost = dp_iboost;
-		} else {
-			ddi_translations = intel_ddi_get_buf_trans_dp(dev_priv, &n_entries);
-			iboost = ddi_translations[level].i_boost;
-		}
-	} else if (type == INTEL_OUTPUT_EDP) {
-		if (dp_iboost) {
-			iboost = dp_iboost;
-		} else {
-			ddi_translations = intel_ddi_get_buf_trans_edp(dev_priv, &n_entries);
+	if (iboost == 0) {
+		const struct ddi_buf_trans *ddi_translations;
+		int n_entries;
 
-			if (WARN_ON(port != PORT_A &&
-				    port != PORT_E && n_entries > 9))
-				n_entries = 9;
-
-			iboost = ddi_translations[level].i_boost;
-		}
-	} else if (type == INTEL_OUTPUT_HDMI) {
-		if (hdmi_iboost) {
-			iboost = hdmi_iboost;
-		} else {
+		if (type == INTEL_OUTPUT_HDMI)
 			ddi_translations = intel_ddi_get_buf_trans_hdmi(dev_priv, &n_entries);
-			iboost = ddi_translations[level].i_boost;
-		}
-	} else {
-		return;
+		else if (type == INTEL_OUTPUT_EDP)
+			ddi_translations = intel_ddi_get_buf_trans_edp(dev_priv, &n_entries);
+		else
+			ddi_translations = intel_ddi_get_buf_trans_dp(dev_priv, &n_entries);
+
+		if (WARN_ON(type != INTEL_OUTPUT_HDMI &&
+			    port != PORT_A &&
+			    port != PORT_E && n_entries > 9))
+			n_entries = 9;
+
+		iboost = ddi_translations[level].i_boost;
 	}
 
 	/* Make sure that the requested I_boost is valid */
@@ -2096,7 +2083,7 @@ uint32_t ddi_signal_levels(struct intel_dp *intel_dp)
 	uint32_t level = intel_ddi_dp_level(intel_dp);
 
 	if (IS_GEN9_BC(dev_priv))
-	    skl_ddi_set_iboost(encoder, level);
+		skl_ddi_set_iboost(encoder, level, encoder->type);
 
 	return DDI_BUF_TRANS_SELECT(level);
 }
@@ -2219,7 +2206,7 @@ static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
 		intel_prepare_hdmi_ddi_buffers(encoder);
 
 	if (IS_GEN9_BC(dev_priv))
-		skl_ddi_set_iboost(encoder, level);
+		skl_ddi_set_iboost(encoder, level, INTEL_OUTPUT_HDMI);
 
 	intel_dig_port->set_infoframes(&encoder->base,
 				       crtc_state->has_infoframe,
-- 
2.13.6

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

[PATCH 04/10] drm/i915: Pass the level to intel_prepare_hdmi_ddi_buffers()

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The caller of intel_prepare_hdmi_ddi_buffers() alreday figured out the
level, so let's just pass it in instead if figuring it out again.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index cf0b2d3de15f..f61b6c20005e 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -817,16 +817,15 @@ static void intel_prepare_dp_ddi_buffers(struct intel_encoder *encoder)
  * values in advance. This function programs the correct values for
  * HDMI/DVI use cases.
  */
-static void intel_prepare_hdmi_ddi_buffers(struct intel_encoder *encoder)
+static void intel_prepare_hdmi_ddi_buffers(struct intel_encoder *encoder,
+					   int hdmi_level)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	u32 iboost_bit = 0;
-	int n_hdmi_entries, hdmi_level;
+	int n_hdmi_entries;
 	enum port port = intel_ddi_get_encoder_port(encoder);
 	const struct ddi_buf_trans *ddi_translations_hdmi;
 
-	hdmi_level = intel_ddi_hdmi_level(dev_priv, port);
-
 	ddi_translations_hdmi = intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
 
 	/* If we're boosting the current, set bit 31 of trans1 */
@@ -2203,7 +2202,7 @@ static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
 		bxt_ddi_vswing_sequence(dev_priv, level, port,
 					INTEL_OUTPUT_HDMI);
 	else
-		intel_prepare_hdmi_ddi_buffers(encoder);
+		intel_prepare_hdmi_ddi_buffers(encoder, level);
 
 	if (IS_GEN9_BC(dev_priv))
 		skl_ddi_set_iboost(encoder, level, INTEL_OUTPUT_HDMI);
-- 
2.13.6

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

[PATCH v2 05/10] drm/i915: Integrate BXT into intel_ddi_dp_voltage_max()

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Make BXT less special by following the CNL approach and handling
it in intel_ddi_dp_voltage_max() alognside every other DDI platform.

v2: Clean up the argument types to bxt_ddi_vswing_sequence() while at it

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 67 ++++++++++++++++++++++++++--------------
 drivers/gpu/drm/i915/intel_dp.c  |  4 +--
 2 files changed, 45 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index f61b6c20005e..2d886148a653 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -659,6 +659,31 @@ intel_ddi_get_buf_trans_hdmi(struct drm_i915_private *dev_priv,
 	return NULL;
 }
 
+static const struct bxt_ddi_buf_trans *
+bxt_get_buf_trans_dp(struct drm_i915_private *dev_priv, int *n_entries)
+{
+	*n_entries = ARRAY_SIZE(bxt_ddi_translations_dp);
+	return bxt_ddi_translations_dp;
+}
+
+static const struct bxt_ddi_buf_trans *
+bxt_get_buf_trans_edp(struct drm_i915_private *dev_priv, int *n_entries)
+{
+	if (dev_priv->vbt.edp.low_vswing) {
+		*n_entries = ARRAY_SIZE(bxt_ddi_translations_edp);
+		return bxt_ddi_translations_edp;
+	}
+
+	return bxt_get_buf_trans_dp(dev_priv, n_entries);
+}
+
+static const struct bxt_ddi_buf_trans *
+bxt_get_buf_trans_hdmi(struct drm_i915_private *dev_priv, int *n_entries)
+{
+	*n_entries = ARRAY_SIZE(bxt_ddi_translations_hdmi);
+	return bxt_ddi_translations_hdmi;
+}
+
 static const struct cnl_ddi_buf_trans *
 cnl_get_buf_trans_hdmi(struct drm_i915_private *dev_priv, int *n_entries)
 {
@@ -1831,27 +1856,20 @@ static void skl_ddi_set_iboost(struct intel_encoder *encoder,
 		_skl_ddi_set_iboost(dev_priv, PORT_E, iboost);
 }
 
-static void bxt_ddi_vswing_sequence(struct drm_i915_private *dev_priv,
-				    u32 level, enum port port, int type)
+static void bxt_ddi_vswing_sequence(struct intel_encoder *encoder,
+				    int level, enum intel_output_type type)
 {
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	const struct bxt_ddi_buf_trans *ddi_translations;
-	u32 n_entries, i;
+	enum port port = encoder->port;
+	int n_entries, i;
 
-	if (type == INTEL_OUTPUT_EDP && dev_priv->vbt.edp.low_vswing) {
-		n_entries = ARRAY_SIZE(bxt_ddi_translations_edp);
-		ddi_translations = bxt_ddi_translations_edp;
-	} else if (type == INTEL_OUTPUT_DP
-			|| type == INTEL_OUTPUT_EDP) {
-		n_entries = ARRAY_SIZE(bxt_ddi_translations_dp);
-		ddi_translations = bxt_ddi_translations_dp;
-	} else if (type == INTEL_OUTPUT_HDMI) {
-		n_entries = ARRAY_SIZE(bxt_ddi_translations_hdmi);
-		ddi_translations = bxt_ddi_translations_hdmi;
-	} else {
-		DRM_DEBUG_KMS("Vswing programming not done for encoder %d\n",
-				type);
-		return;
-	}
+	if (type == INTEL_OUTPUT_HDMI)
+		ddi_translations = bxt_get_buf_trans_hdmi(dev_priv, &n_entries);
+	else if (type == INTEL_OUTPUT_EDP)
+		ddi_translations = bxt_get_buf_trans_edp(dev_priv, &n_entries);
+	else
+		ddi_translations = bxt_get_buf_trans_dp(dev_priv, &n_entries);
 
 	/* Check if default value has to be used */
 	if (level >= n_entries ||
@@ -1881,6 +1899,11 @@ u8 intel_ddi_dp_voltage_max(struct intel_encoder *encoder)
 			cnl_get_buf_trans_edp(dev_priv, &n_entries);
 		else
 			cnl_get_buf_trans_dp(dev_priv, &n_entries);
+	} else if (IS_GEN9_LP(dev_priv)) {
+		if (encoder->type == INTEL_OUTPUT_EDP)
+			bxt_get_buf_trans_edp(dev_priv, &n_entries);
+		else
+			bxt_get_buf_trans_dp(dev_priv, &n_entries);
 	} else {
 		if (encoder->type == INTEL_OUTPUT_EDP)
 			intel_ddi_get_buf_trans_edp(dev_priv, &n_entries);
@@ -2063,13 +2086,12 @@ u32 bxt_signal_levels(struct intel_dp *intel_dp)
 	struct intel_digital_port *dport = dp_to_dig_port(intel_dp);
 	struct drm_i915_private *dev_priv = to_i915(dport->base.base.dev);
 	struct intel_encoder *encoder = &dport->base;
-	enum port port = dport->port;
 	u32 level = intel_ddi_dp_level(intel_dp);
 
 	if (IS_CANNONLAKE(dev_priv))
 		cnl_ddi_vswing_sequence(encoder, level);
 	else
-		bxt_ddi_vswing_sequence(dev_priv, level, port, encoder->type);
+		bxt_ddi_vswing_sequence(encoder, level, encoder->type);
 
 	return 0;
 }
@@ -2168,7 +2190,7 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
 	if (IS_CANNONLAKE(dev_priv))
 		cnl_ddi_vswing_sequence(encoder, level);
 	else if (IS_GEN9_LP(dev_priv))
-		bxt_ddi_vswing_sequence(dev_priv, level, port, encoder->type);
+		bxt_ddi_vswing_sequence(encoder, level, encoder->type);
 	else
 		intel_prepare_dp_ddi_buffers(encoder);
 
@@ -2199,8 +2221,7 @@ static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
 	if (IS_CANNONLAKE(dev_priv))
 		cnl_ddi_vswing_sequence(encoder, level);
 	else if (IS_GEN9_LP(dev_priv))
-		bxt_ddi_vswing_sequence(dev_priv, level, port,
-					INTEL_OUTPUT_HDMI);
+		bxt_ddi_vswing_sequence(encoder, level, INTEL_OUTPUT_HDMI);
 	else
 		intel_prepare_hdmi_ddi_buffers(encoder, level);
 
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 4b65cf137f79..aa75f55eeb61 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3155,9 +3155,7 @@ intel_dp_voltage_max(struct intel_dp *intel_dp)
 	struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
 	enum port port = dp_to_dig_port(intel_dp)->port;
 
-	if (IS_GEN9_LP(dev_priv))
-		return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
-	else if (INTEL_GEN(dev_priv) >= 9) {
+	if (INTEL_GEN(dev_priv) >= 9) {
 		struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
 		return intel_ddi_dp_voltage_max(encoder);
 	} else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
-- 
2.13.6

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

[PATCH v2 06/10] drm/i915: Pass encoder type to cnl_ddi_vswing_sequence() explicitly

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

encoder->type is unreliable for DP/HDMI, so pass it in explicity into
cnl_ddi_vswing_sequence(). This matches what we do for BXT.

v2: Pass intel_encoder down to cnl_ddi_vswing_program(), and
    clean up the argument types while at it

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 53 +++++++++++++++++++---------------------
 1 file changed, 25 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 2d886148a653..cab72177299c 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1920,20 +1920,21 @@ u8 intel_ddi_dp_voltage_max(struct intel_encoder *encoder)
 		DP_TRAIN_VOLTAGE_SWING_MASK;
 }
 
-static void cnl_ddi_vswing_program(struct drm_i915_private *dev_priv,
-				    u32 level, enum port port, int type)
+static void cnl_ddi_vswing_program(struct intel_encoder *encoder,
+				   int level, enum intel_output_type type)
 {
-	const struct cnl_ddi_buf_trans *ddi_translations = NULL;
-	u32 n_entries, val;
-	int ln;
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+	enum port port = intel_ddi_get_encoder_port(encoder);
+	const struct cnl_ddi_buf_trans *ddi_translations;
+	int n_entries, ln;
+	u32 val;
 
-	if (type == INTEL_OUTPUT_HDMI) {
+	if (type == INTEL_OUTPUT_HDMI)
 		ddi_translations = cnl_get_buf_trans_hdmi(dev_priv, &n_entries);
-	} else if (type == INTEL_OUTPUT_DP) {
-		ddi_translations = cnl_get_buf_trans_dp(dev_priv, &n_entries);
-	} else if (type == INTEL_OUTPUT_EDP) {
+	else if (type == INTEL_OUTPUT_EDP)
 		ddi_translations = cnl_get_buf_trans_edp(dev_priv, &n_entries);
-	}
+	else
+		ddi_translations = cnl_get_buf_trans_dp(dev_priv, &n_entries);
 
 	if (WARN_ON(ddi_translations == NULL))
 		return;
@@ -1986,26 +1987,22 @@ static void cnl_ddi_vswing_program(struct drm_i915_private *dev_priv,
 	I915_WRITE(CNL_PORT_TX_DW7_GRP(port), val);
 }
 
-static void cnl_ddi_vswing_sequence(struct intel_encoder *encoder, u32 level)
+static void cnl_ddi_vswing_sequence(struct intel_encoder *encoder,
+				    int level, enum intel_output_type type)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
 	enum port port = intel_ddi_get_encoder_port(encoder);
-	int type = encoder->type;
-	int width = 0;
-	int rate = 0;
+	int width, rate, ln;
 	u32 val;
-	int ln = 0;
 
-	if ((intel_dp) && (type == INTEL_OUTPUT_EDP || type == INTEL_OUTPUT_DP)) {
+	if (type == INTEL_OUTPUT_HDMI) {
+		width = 4;
+		rate = 0; /* Rate is always < than 6GHz for HDMI */
+	} else {
+		struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
+
 		width = intel_dp->lane_count;
 		rate = intel_dp->link_rate;
-	} else if (type == INTEL_OUTPUT_HDMI) {
-		width = 4;
-		/* Rate is always < than 6GHz for HDMI */
-	} else {
-		MISSING_CASE(type);
-		return;
 	}
 
 	/*
@@ -2014,7 +2011,7 @@ static void cnl_ddi_vswing_sequence(struct intel_encoder *encoder, u32 level)
 	 * else clear to 0b.
 	 */
 	val = I915_READ(CNL_PORT_PCS_DW1_LN0(port));
-	if (type == INTEL_OUTPUT_EDP || type == INTEL_OUTPUT_DP)
+	if (type != INTEL_OUTPUT_HDMI)
 		val |= COMMON_KEEPER_EN;
 	else
 		val &= ~COMMON_KEEPER_EN;
@@ -2049,7 +2046,7 @@ static void cnl_ddi_vswing_sequence(struct intel_encoder *encoder, u32 level)
 	I915_WRITE(CNL_PORT_TX_DW5_GRP(port), val);
 
 	/* 5. Program swing and de-emphasis */
-	cnl_ddi_vswing_program(dev_priv, level, port, type);
+	cnl_ddi_vswing_program(encoder, level, type);
 
 	/* 6. Set training enable to trigger update */
 	val = I915_READ(CNL_PORT_TX_DW5_LN0(port));
@@ -2089,7 +2086,7 @@ u32 bxt_signal_levels(struct intel_dp *intel_dp)
 	u32 level = intel_ddi_dp_level(intel_dp);
 
 	if (IS_CANNONLAKE(dev_priv))
-		cnl_ddi_vswing_sequence(encoder, level);
+		cnl_ddi_vswing_sequence(encoder, level, encoder->type);
 	else
 		bxt_ddi_vswing_sequence(encoder, level, encoder->type);
 
@@ -2188,7 +2185,7 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
 	intel_display_power_get(dev_priv, dig_port->ddi_io_power_domain);
 
 	if (IS_CANNONLAKE(dev_priv))
-		cnl_ddi_vswing_sequence(encoder, level);
+		cnl_ddi_vswing_sequence(encoder, level, encoder->type);
 	else if (IS_GEN9_LP(dev_priv))
 		bxt_ddi_vswing_sequence(encoder, level, encoder->type);
 	else
@@ -2219,7 +2216,7 @@ static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
 	intel_display_power_get(dev_priv, dig_port->ddi_io_power_domain);
 
 	if (IS_CANNONLAKE(dev_priv))
-		cnl_ddi_vswing_sequence(encoder, level);
+		cnl_ddi_vswing_sequence(encoder, level, INTEL_OUTPUT_HDMI);
 	else if (IS_GEN9_LP(dev_priv))
 		bxt_ddi_vswing_sequence(encoder, level, INTEL_OUTPUT_HDMI);
 	else
-- 
2.13.6

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

[PATCH 07/10] drm/i915: Kill off the BXT buf_trans default_index

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

default_index contained in the BXT buf_trans tables is actually useless.
For DP we should always have a valid level selected (otherwise the link
training logic would be buggy), and for HDMI we can just do what the
other platforms do and pick the correct entry in intel_ddi_hdmi_level().

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 80 +++++++++++++++++-----------------------
 1 file changed, 34 insertions(+), 46 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index cab72177299c..3bce6bddeaa9 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -305,35 +305,34 @@ struct bxt_ddi_buf_trans {
 	u8 scale;	/* scale value */
 	u8 enable;	/* scale enable */
 	u8 deemphasis;
-	bool default_index; /* true if the entry represents default value */
 };
 
 static const struct bxt_ddi_buf_trans bxt_ddi_translations_dp[] = {
 					/* Idx	NT mV diff	db  */
-	{ 52,  0x9A, 0, 128, true  },	/* 0:	400		0   */
-	{ 78,  0x9A, 0, 85,  false },	/* 1:	400		3.5 */
-	{ 104, 0x9A, 0, 64,  false },	/* 2:	400		6   */
-	{ 154, 0x9A, 0, 43,  false },	/* 3:	400		9.5 */
-	{ 77,  0x9A, 0, 128, false },	/* 4:	600		0   */
-	{ 116, 0x9A, 0, 85,  false },	/* 5:	600		3.5 */
-	{ 154, 0x9A, 0, 64,  false },	/* 6:	600		6   */
-	{ 102, 0x9A, 0, 128, false },	/* 7:	800		0   */
-	{ 154, 0x9A, 0, 85,  false },	/* 8:	800		3.5 */
-	{ 154, 0x9A, 1, 128, false },	/* 9:	1200		0   */
+	{ 52,  0x9A, 0, 128, },	/* 0:	400		0   */
+	{ 78,  0x9A, 0, 85,  },	/* 1:	400		3.5 */
+	{ 104, 0x9A, 0, 64,  },	/* 2:	400		6   */
+	{ 154, 0x9A, 0, 43,  },	/* 3:	400		9.5 */
+	{ 77,  0x9A, 0, 128, },	/* 4:	600		0   */
+	{ 116, 0x9A, 0, 85,  },	/* 5:	600		3.5 */
+	{ 154, 0x9A, 0, 64,  },	/* 6:	600		6   */
+	{ 102, 0x9A, 0, 128, },	/* 7:	800		0   */
+	{ 154, 0x9A, 0, 85,  },	/* 8:	800		3.5 */
+	{ 154, 0x9A, 1, 128, },	/* 9:	1200		0   */
 };
 
 static const struct bxt_ddi_buf_trans bxt_ddi_translations_edp[] = {
 					/* Idx	NT mV diff	db  */
-	{ 26, 0, 0, 128, false },	/* 0:	200		0   */
-	{ 38, 0, 0, 112, false },	/* 1:	200		1.5 */
-	{ 48, 0, 0, 96,  false },	/* 2:	200		4   */
-	{ 54, 0, 0, 69,  false },	/* 3:	200		6   */
-	{ 32, 0, 0, 128, false },	/* 4:	250		0   */
-	{ 48, 0, 0, 104, false },	/* 5:	250		1.5 */
-	{ 54, 0, 0, 85,  false },	/* 6:	250		4   */
-	{ 43, 0, 0, 128, false },	/* 7:	300		0   */
-	{ 54, 0, 0, 101, false },	/* 8:	300		1.5 */
-	{ 48, 0, 0, 128, false },	/* 9:	300		0   */
+	{ 26, 0, 0, 128, },	/* 0:	200		0   */
+	{ 38, 0, 0, 112, },	/* 1:	200		1.5 */
+	{ 48, 0, 0, 96,  },	/* 2:	200		4   */
+	{ 54, 0, 0, 69,  },	/* 3:	200		6   */
+	{ 32, 0, 0, 128, },	/* 4:	250		0   */
+	{ 48, 0, 0, 104, },	/* 5:	250		1.5 */
+	{ 54, 0, 0, 85,  },	/* 6:	250		4   */
+	{ 43, 0, 0, 128, },	/* 7:	300		0   */
+	{ 54, 0, 0, 101, },	/* 8:	300		1.5 */
+	{ 48, 0, 0, 128, },	/* 9:	300		0   */
 };
 
 /* BSpec has 2 recommended values - entries 0 and 8.
@@ -341,16 +340,16 @@ static const struct bxt_ddi_buf_trans bxt_ddi_translations_edp[] = {
  */
 static const struct bxt_ddi_buf_trans bxt_ddi_translations_hdmi[] = {
 					/* Idx	NT mV diff	db  */
-	{ 52,  0x9A, 0, 128, false },	/* 0:	400		0   */
-	{ 52,  0x9A, 0, 85,  false },	/* 1:	400		3.5 */
-	{ 52,  0x9A, 0, 64,  false },	/* 2:	400		6   */
-	{ 42,  0x9A, 0, 43,  false },	/* 3:	400		9.5 */
-	{ 77,  0x9A, 0, 128, false },	/* 4:	600		0   */
-	{ 77,  0x9A, 0, 85,  false },	/* 5:	600		3.5 */
-	{ 77,  0x9A, 0, 64,  false },	/* 6:	600		6   */
-	{ 102, 0x9A, 0, 128, false },	/* 7:	800		0   */
-	{ 102, 0x9A, 0, 85,  false },	/* 8:	800		3.5 */
-	{ 154, 0x9A, 1, 128, true },	/* 9:	1200		0   */
+	{ 52,  0x9A, 0, 128, },	/* 0:	400		0   */
+	{ 52,  0x9A, 0, 85,  },	/* 1:	400		3.5 */
+	{ 52,  0x9A, 0, 64,  },	/* 2:	400		6   */
+	{ 42,  0x9A, 0, 43,  },	/* 3:	400		9.5 */
+	{ 77,  0x9A, 0, 128, },	/* 4:	600		0   */
+	{ 77,  0x9A, 0, 85,  },	/* 5:	600		3.5 */
+	{ 77,  0x9A, 0, 64,  },	/* 6:	600		6   */
+	{ 102, 0x9A, 0, 128, },	/* 7:	800		0   */
+	{ 102, 0x9A, 0, 85,  },	/* 8:	800		3.5 */
+	{ 154, 0x9A, 1, 128, },	/* 9:	1200		0   */
 };
 
 struct cnl_ddi_buf_trans {
@@ -759,12 +758,12 @@ static int intel_ddi_hdmi_level(struct drm_i915_private *dev_priv, enum port por
 
 	hdmi_level = dev_priv->vbt.ddi_port_info[port].hdmi_level_shift;
 
-	if (IS_GEN9_LP(dev_priv))
-		return hdmi_level;
-
 	if (IS_CANNONLAKE(dev_priv)) {
 		cnl_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
 		hdmi_default_entry = n_hdmi_entries - 1;
+	} else if (IS_GEN9_LP(dev_priv)) {
+		bxt_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
+		hdmi_default_entry = n_hdmi_entries - 1;
 	} else if (IS_GEN9_BC(dev_priv)) {
 		intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
 		hdmi_default_entry = 8;
@@ -1862,7 +1861,7 @@ static void bxt_ddi_vswing_sequence(struct intel_encoder *encoder,
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	const struct bxt_ddi_buf_trans *ddi_translations;
 	enum port port = encoder->port;
-	int n_entries, i;
+	int n_entries;
 
 	if (type == INTEL_OUTPUT_HDMI)
 		ddi_translations = bxt_get_buf_trans_hdmi(dev_priv, &n_entries);
@@ -1871,17 +1870,6 @@ static void bxt_ddi_vswing_sequence(struct intel_encoder *encoder,
 	else
 		ddi_translations = bxt_get_buf_trans_dp(dev_priv, &n_entries);
 
-	/* Check if default value has to be used */
-	if (level >= n_entries ||
-	    (type == INTEL_OUTPUT_HDMI && level == HDMI_LEVEL_SHIFT_UNKNOWN)) {
-		for (i = 0; i < n_entries; i++) {
-			if (ddi_translations[i].default_index) {
-				level = i;
-				break;
-			}
-		}
-	}
-
 	bxt_ddi_phy_set_signal_level(dev_priv, port,
 				     ddi_translations[level].margin,
 				     ddi_translations[level].scale,
-- 
2.13.6

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

[PATCH 08/10] drm/i915: Centralize the SKL DDI A/E vs. B/C/D buf trans handling

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

SKL DDI B/C/D only have 9 usable buf trans registers for DP/eDP. That
matches the normal DP buf trans tables, but the low vswing eDP tables
have 10 entries. Thus the eDP tables can only be used safely with DDI A
and E.

We try to catch cases where DDI B/C/D gets used with the wrong number of
entires in some parts of the code, but not everywhere. Let's move the
code to deal with that deeper into intel_ddi_get_buf_trans_edp(). And
for sake of symmetry do the same in intel_ddi_get_buf_trans_dp(). That
would also avoid explosions in the rather unlikely case that the DP
tables would get revised to 10 entries as well.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 60 +++++++++++++++++++++++-----------------
 1 file changed, 34 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 3bce6bddeaa9..f0a709b66e00 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -587,14 +587,29 @@ skl_get_buf_trans_hdmi(struct drm_i915_private *dev_priv, int *n_entries)
 	}
 }
 
+static int skl_buf_trans_num_entries(enum port port, int n_entries)
+{
+	/* Only DDIA and DDIE can select the 10th register with DP */
+	if (port == PORT_A || port == PORT_E)
+		return min(n_entries, 10);
+	else
+		return min(n_entries, 9);
+}
+
 static const struct ddi_buf_trans *
 intel_ddi_get_buf_trans_dp(struct drm_i915_private *dev_priv,
-			   int *n_entries)
+			   enum port port, int *n_entries)
 {
 	if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv)) {
-		return kbl_get_buf_trans_dp(dev_priv, n_entries);
+		const struct ddi_buf_trans *ddi_translations =
+			kbl_get_buf_trans_dp(dev_priv, n_entries);
+		*n_entries = skl_buf_trans_num_entries(port, *n_entries);
+		return ddi_translations;
 	} else if (IS_SKYLAKE(dev_priv)) {
-		return skl_get_buf_trans_dp(dev_priv, n_entries);
+		const struct ddi_buf_trans *ddi_translations =
+			skl_get_buf_trans_dp(dev_priv, n_entries);
+		*n_entries = skl_buf_trans_num_entries(port, *n_entries);
+		return ddi_translations;
 	} else if (IS_BROADWELL(dev_priv)) {
 		*n_entries = ARRAY_SIZE(bdw_ddi_translations_dp);
 		return  bdw_ddi_translations_dp;
@@ -609,10 +624,13 @@ intel_ddi_get_buf_trans_dp(struct drm_i915_private *dev_priv,
 
 static const struct ddi_buf_trans *
 intel_ddi_get_buf_trans_edp(struct drm_i915_private *dev_priv,
-			    int *n_entries)
+			    enum port port, int *n_entries)
 {
 	if (IS_GEN9_BC(dev_priv)) {
-		return skl_get_buf_trans_edp(dev_priv, n_entries);
+		const struct ddi_buf_trans *ddi_translations =
+			skl_get_buf_trans_edp(dev_priv, n_entries);
+		*n_entries = skl_buf_trans_num_entries(port, *n_entries);
+		return ddi_translations;
 	} else if (IS_BROADWELL(dev_priv)) {
 		return bdw_get_buf_trans_edp(dev_priv, n_entries);
 	} else if (IS_HASWELL(dev_priv)) {
@@ -801,11 +819,11 @@ static void intel_prepare_dp_ddi_buffers(struct intel_encoder *encoder)
 
 	switch (encoder->type) {
 	case INTEL_OUTPUT_EDP:
-		ddi_translations = intel_ddi_get_buf_trans_edp(dev_priv,
+		ddi_translations = intel_ddi_get_buf_trans_edp(dev_priv, port,
 							       &n_entries);
 		break;
 	case INTEL_OUTPUT_DP:
-		ddi_translations = intel_ddi_get_buf_trans_dp(dev_priv,
+		ddi_translations = intel_ddi_get_buf_trans_dp(dev_priv, port,
 							      &n_entries);
 		break;
 	case INTEL_OUTPUT_ANALOG:
@@ -817,16 +835,10 @@ static void intel_prepare_dp_ddi_buffers(struct intel_encoder *encoder)
 		return;
 	}
 
-	if (IS_GEN9_BC(dev_priv)) {
-		/* If we're boosting the current, set bit 31 of trans1 */
-		if (dev_priv->vbt.ddi_port_info[port].dp_boost_level)
-			iboost_bit = DDI_BUF_BALANCE_LEG_ENABLE;
-
-		if (WARN_ON(encoder->type == INTEL_OUTPUT_EDP &&
-			    port != PORT_A && port != PORT_E &&
-			    n_entries > 9))
-			n_entries = 9;
-	}
+	/* If we're boosting the current, set bit 31 of trans1 */
+	if (IS_GEN9_BC(dev_priv) &&
+	    dev_priv->vbt.ddi_port_info[port].dp_boost_level)
+		iboost_bit = DDI_BUF_BALANCE_LEG_ENABLE;
 
 	for (i = 0; i < n_entries; i++) {
 		I915_WRITE(DDI_BUF_TRANS_LO(port, i),
@@ -1831,14 +1843,9 @@ static void skl_ddi_set_iboost(struct intel_encoder *encoder,
 		if (type == INTEL_OUTPUT_HDMI)
 			ddi_translations = intel_ddi_get_buf_trans_hdmi(dev_priv, &n_entries);
 		else if (type == INTEL_OUTPUT_EDP)
-			ddi_translations = intel_ddi_get_buf_trans_edp(dev_priv, &n_entries);
+			ddi_translations = intel_ddi_get_buf_trans_edp(dev_priv, port, &n_entries);
 		else
-			ddi_translations = intel_ddi_get_buf_trans_dp(dev_priv, &n_entries);
-
-		if (WARN_ON(type != INTEL_OUTPUT_HDMI &&
-			    port != PORT_A &&
-			    port != PORT_E && n_entries > 9))
-			n_entries = 9;
+			ddi_translations = intel_ddi_get_buf_trans_dp(dev_priv, port, &n_entries);
 
 		iboost = ddi_translations[level].i_boost;
 	}
@@ -1880,6 +1887,7 @@ static void bxt_ddi_vswing_sequence(struct intel_encoder *encoder,
 u8 intel_ddi_dp_voltage_max(struct intel_encoder *encoder)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+	enum port port = encoder->port;
 	int n_entries;
 
 	if (IS_CANNONLAKE(dev_priv)) {
@@ -1894,9 +1902,9 @@ u8 intel_ddi_dp_voltage_max(struct intel_encoder *encoder)
 			bxt_get_buf_trans_dp(dev_priv, &n_entries);
 	} else {
 		if (encoder->type == INTEL_OUTPUT_EDP)
-			intel_ddi_get_buf_trans_edp(dev_priv, &n_entries);
+			intel_ddi_get_buf_trans_edp(dev_priv, port, &n_entries);
 		else
-			intel_ddi_get_buf_trans_dp(dev_priv, &n_entries);
+			intel_ddi_get_buf_trans_dp(dev_priv, port, &n_entries);
 	}
 
 	if (WARN_ON(n_entries < 1))
-- 
2.13.6

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

[PATCH 09/10] drm/i915: Unify error handling for missing DDI buf trans tables

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Handle missing buf trans tables, or out of bounds buf trans levels
the same way everywhere. These should never be hit under normal
conditions, but let's play it safe for now.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index f0a709b66e00..5f1c546e5e61 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -801,6 +801,9 @@ static int intel_ddi_hdmi_level(struct drm_i915_private *dev_priv, enum port por
 	    hdmi_level >= n_hdmi_entries)
 		hdmi_level = hdmi_default_entry;
 
+	if (WARN_ON_ONCE(hdmi_level >= n_hdmi_entries))
+		hdmi_level = n_hdmi_entries - 1;
+
 	return hdmi_level;
 }
 
@@ -864,6 +867,11 @@ static void intel_prepare_hdmi_ddi_buffers(struct intel_encoder *encoder,
 
 	ddi_translations_hdmi = intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
 
+	if (WARN_ON_ONCE(!ddi_translations_hdmi))
+		return;
+	if (WARN_ON_ONCE(hdmi_level >= n_hdmi_entries))
+		hdmi_level = n_hdmi_entries - 1;
+
 	/* If we're boosting the current, set bit 31 of trans1 */
 	if (IS_GEN9_BC(dev_priv) &&
 	    dev_priv->vbt.ddi_port_info[port].hdmi_boost_level)
@@ -1847,6 +1855,11 @@ static void skl_ddi_set_iboost(struct intel_encoder *encoder,
 		else
 			ddi_translations = intel_ddi_get_buf_trans_dp(dev_priv, port, &n_entries);
 
+		if (WARN_ON_ONCE(!ddi_translations))
+			return;
+		if (WARN_ON_ONCE(level >= n_entries))
+			level = n_entries - 1;
+
 		iboost = ddi_translations[level].i_boost;
 	}
 
@@ -1877,6 +1890,11 @@ static void bxt_ddi_vswing_sequence(struct intel_encoder *encoder,
 	else
 		ddi_translations = bxt_get_buf_trans_dp(dev_priv, &n_entries);
 
+	if (WARN_ON_ONCE(!ddi_translations))
+		return;
+	if (WARN_ON_ONCE(level >= n_entries))
+		level = n_entries - 1;
+
 	bxt_ddi_phy_set_signal_level(dev_priv, port,
 				     ddi_translations[level].margin,
 				     ddi_translations[level].scale,
@@ -1932,13 +1950,10 @@ static void cnl_ddi_vswing_program(struct intel_encoder *encoder,
 	else
 		ddi_translations = cnl_get_buf_trans_dp(dev_priv, &n_entries);
 
-	if (WARN_ON(ddi_translations == NULL))
+	if (WARN_ON_ONCE(!ddi_translations))
 		return;
-
-	if (level >= n_entries) {
-		DRM_DEBUG_KMS("DDI translation not found for level %d. Using %d instead.", level, n_entries - 1);
+	if (WARN_ON_ONCE(level >= n_entries))
 		level = n_entries - 1;
-	}
 
 	/* Set PORT_TX_DW5 Scaling Mode Sel to 010b. */
 	val = I915_READ(CNL_PORT_TX_DW5_LN0(port));
-- 
2.13.6

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

[PATCH 10/10] drm/i915: Drop the redundant hdmi prefix/suffix from a lot of variables

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

A bunch of functions are now exclusively used for HDMI, so naming the
variables with hdmi prefix/suffix is redundant. Also use int rather
than u32 for the translation level consistently.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 61 +++++++++++++++++++---------------------
 1 file changed, 29 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 5f1c546e5e61..cd550aab332b 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -770,41 +770,38 @@ cnl_get_buf_trans_edp(struct drm_i915_private *dev_priv, int *n_entries)
 
 static int intel_ddi_hdmi_level(struct drm_i915_private *dev_priv, enum port port)
 {
-	int n_hdmi_entries;
-	int hdmi_level;
-	int hdmi_default_entry;
+	int n_entries, level, default_entry;
 
-	hdmi_level = dev_priv->vbt.ddi_port_info[port].hdmi_level_shift;
+	level = dev_priv->vbt.ddi_port_info[port].hdmi_level_shift;
 
 	if (IS_CANNONLAKE(dev_priv)) {
-		cnl_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
-		hdmi_default_entry = n_hdmi_entries - 1;
+		cnl_get_buf_trans_hdmi(dev_priv, &n_entries);
+		default_entry = n_entries - 1;
 	} else if (IS_GEN9_LP(dev_priv)) {
-		bxt_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
-		hdmi_default_entry = n_hdmi_entries - 1;
+		bxt_get_buf_trans_hdmi(dev_priv, &n_entries);
+		default_entry = n_entries - 1;
 	} else if (IS_GEN9_BC(dev_priv)) {
-		intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
-		hdmi_default_entry = 8;
+		intel_ddi_get_buf_trans_hdmi(dev_priv, &n_entries);
+		default_entry = 8;
 	} else if (IS_BROADWELL(dev_priv)) {
-		intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
-		hdmi_default_entry = 7;
+		intel_ddi_get_buf_trans_hdmi(dev_priv, &n_entries);
+		default_entry = 7;
 	} else if (IS_HASWELL(dev_priv)) {
-		intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
-		hdmi_default_entry = 6;
+		intel_ddi_get_buf_trans_hdmi(dev_priv, &n_entries);
+		default_entry = 6;
 	} else {
 		WARN(1, "ddi translation table missing\n");
 		return 0;
 	}
 
 	/* Choose a good default if VBT is badly populated */
-	if (hdmi_level == HDMI_LEVEL_SHIFT_UNKNOWN ||
-	    hdmi_level >= n_hdmi_entries)
-		hdmi_level = hdmi_default_entry;
+	if (level == HDMI_LEVEL_SHIFT_UNKNOWN || level >= n_entries)
+		level = default_entry;
 
-	if (WARN_ON_ONCE(hdmi_level >= n_hdmi_entries))
-		hdmi_level = n_hdmi_entries - 1;
+	if (WARN_ON_ONCE(level >= n_entries))
+		level = n_entries - 1;
 
-	return hdmi_level;
+	return level;
 }
 
 /*
@@ -857,20 +854,20 @@ static void intel_prepare_dp_ddi_buffers(struct intel_encoder *encoder)
  * HDMI/DVI use cases.
  */
 static void intel_prepare_hdmi_ddi_buffers(struct intel_encoder *encoder,
-					   int hdmi_level)
+					   int level)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	u32 iboost_bit = 0;
-	int n_hdmi_entries;
+	int n_entries;
 	enum port port = intel_ddi_get_encoder_port(encoder);
-	const struct ddi_buf_trans *ddi_translations_hdmi;
+	const struct ddi_buf_trans *ddi_translations;
 
-	ddi_translations_hdmi = intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
+	ddi_translations = intel_ddi_get_buf_trans_hdmi(dev_priv, &n_entries);
 
-	if (WARN_ON_ONCE(!ddi_translations_hdmi))
+	if (WARN_ON_ONCE(!ddi_translations))
 		return;
-	if (WARN_ON_ONCE(hdmi_level >= n_hdmi_entries))
-		hdmi_level = n_hdmi_entries - 1;
+	if (WARN_ON_ONCE(level >= n_entries))
+		level = n_entries - 1;
 
 	/* If we're boosting the current, set bit 31 of trans1 */
 	if (IS_GEN9_BC(dev_priv) &&
@@ -879,9 +876,9 @@ static void intel_prepare_hdmi_ddi_buffers(struct intel_encoder *encoder,
 
 	/* Entry 9 is for HDMI: */
 	I915_WRITE(DDI_BUF_TRANS_LO(port, 9),
-		   ddi_translations_hdmi[hdmi_level].trans1 | iboost_bit);
+		   ddi_translations[level].trans1 | iboost_bit);
 	I915_WRITE(DDI_BUF_TRANS_HI(port, 9),
-		   ddi_translations_hdmi[hdmi_level].trans2);
+		   ddi_translations[level].trans2);
 }
 
 static void intel_wait_ddi_buf_idle(struct drm_i915_private *dev_priv,
@@ -2094,7 +2091,7 @@ u32 bxt_signal_levels(struct intel_dp *intel_dp)
 	struct intel_digital_port *dport = dp_to_dig_port(intel_dp);
 	struct drm_i915_private *dev_priv = to_i915(dport->base.base.dev);
 	struct intel_encoder *encoder = &dport->base;
-	u32 level = intel_ddi_dp_level(intel_dp);
+	int level = intel_ddi_dp_level(intel_dp);
 
 	if (IS_CANNONLAKE(dev_priv))
 		cnl_ddi_vswing_sequence(encoder, level, encoder->type);
@@ -2109,7 +2106,7 @@ uint32_t ddi_signal_levels(struct intel_dp *intel_dp)
 	struct intel_digital_port *dport = dp_to_dig_port(intel_dp);
 	struct drm_i915_private *dev_priv = to_i915(dport->base.base.dev);
 	struct intel_encoder *encoder = &dport->base;
-	uint32_t level = intel_ddi_dp_level(intel_dp);
+	int level = intel_ddi_dp_level(intel_dp);
 
 	if (IS_GEN9_BC(dev_priv))
 		skl_ddi_set_iboost(encoder, level, encoder->type);
@@ -2182,7 +2179,7 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
 	enum port port = intel_ddi_get_encoder_port(encoder);
 	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
 	bool is_mst = intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST);
-	uint32_t level = intel_ddi_dp_level(intel_dp);
+	int level = intel_ddi_dp_level(intel_dp);
 
 	WARN_ON(is_mst && (port == PORT_A || port == PORT_E));
 
-- 
2.13.6

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

✓ Fi.CI.BAT: success for DDI buf trans cleanup

[Patchwork]

== Series Details ==

Series: DDI buf trans cleanup
URL   : https://patchwork.freedesktop.org/series/32049/
State : success

== Summary ==

Series 32049v1 DDI buf trans cleanup
https://patchwork.freedesktop.org/api/1.0/series/32049/revisions/1/mbox/

Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-legacy:
                pass       -> FAIL       (fi-gdg-551) fdo#102618

fdo#102618 https://bugs.freedesktop.org/show_bug.cgi?id=102618

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:458s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:467s
fi-blb-e6850     total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:388s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:574s
fi-bwr-2160      total:289  pass:183  dwarn:0   dfail:0   fail:0   skip:106 time:285s
fi-bxt-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:525s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:519s
fi-byt-j1900     total:289  pass:253  dwarn:1   dfail:0   fail:0   skip:35  time:535s
fi-cfl-s         total:289  pass:253  dwarn:4   dfail:0   fail:0   skip:32  time:565s
fi-elk-e7500     total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:432s
fi-gdg-551       total:289  pass:177  dwarn:1   dfail:0   fail:2   skip:109 time:282s
fi-glk-1         total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:600s
fi-hsw-4770r     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:443s
fi-ilk-650       total:289  pass:228  dwarn:0   dfail:0   fail:0   skip:61  time:464s
fi-ivb-3520m     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:508s
fi-ivb-3770      total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:478s
fi-kbl-7500u     total:289  pass:264  dwarn:1   dfail:0   fail:0   skip:24  time:503s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:585s
fi-kbl-7567u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:492s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:593s
fi-pnv-d510      total:289  pass:222  dwarn:1   dfail:0   fail:0   skip:66  time:656s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:473s
fi-skl-6700hq    total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:653s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:538s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:515s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:475s
fi-snb-2520m     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:584s
fi-snb-2600      total:289  pass:249  dwarn:0   dfail:0   fail:0   skip:40  time:434s
fi-byt-n2820 failed to connect after reboot

d6b500e96a1b961f4b74ea2b70e1f2f8ce59e1d6 drm-tip: 2017y-10m-16d-13h-20m-58s UTC integration manifest
0bf1fcd816cf drm/i915: Drop the redundant hdmi prefix/suffix from a lot of variables
cf0b718172ba drm/i915: Unify error handling for missing DDI buf trans tables
ecd40f3a135d drm/i915: Centralize the SKL DDI A/E vs. B/C/D buf trans handling
36b64d673b47 drm/i915: Kill off the BXT buf_trans default_index
659a357ed897 drm/i915: Pass encoder type to cnl_ddi_vswing_sequence() explicitly
f63486d8f6c8 drm/i915: Integrate BXT into intel_ddi_dp_voltage_max()
aa1c74fa393a drm/i915: Pass the level to intel_prepare_hdmi_ddi_buffers()
9725b835a0b6 drm/i915: Pass the encoder type explicitly to skl_set_iboost()
e0d1603fc396 drm/i915: Extract intel_ddi_get_buf_trans_hdmi()
e5158d329b5d drm/i915: Relocate intel_ddi_get_buf_trans_*() functions

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6056/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 01/10] drm/i915: Relocate intel_ddi_get_buf_trans_*() functions

[Ausmus, James]

On Mon, Oct 16, 2017 at 7:56 AM, Ville Syrjala
<ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> We'll want to use the intel_ddi_get_buf_trans_*() functions a bit
> earlier in the file, so move them up. While at it start using them
> in the iboost setup to get rid of the platform checks there.
>
> v2: Rebase due to BDW FDI buf trans fix
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: James Ausmus <james.ausmus@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 115 +++++++++++++++++++--------------------
>  1 file changed, 55 insertions(+), 60 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 8ef65941b8fd..55937abda61f 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -588,6 +588,59 @@ skl_get_buf_trans_hdmi(struct drm_i915_private *dev_priv, int *n_entries)
>         }
>  }
>
> +static const struct ddi_buf_trans *
> +intel_ddi_get_buf_trans_dp(struct drm_i915_private *dev_priv,
> +                          int *n_entries)
> +{
> +       if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv)) {
> +               return kbl_get_buf_trans_dp(dev_priv, n_entries);
> +       } else if (IS_SKYLAKE(dev_priv)) {
> +               return skl_get_buf_trans_dp(dev_priv, n_entries);
> +       } else if (IS_BROADWELL(dev_priv)) {
> +               *n_entries = ARRAY_SIZE(bdw_ddi_translations_dp);
> +               return  bdw_ddi_translations_dp;
> +       } else if (IS_HASWELL(dev_priv)) {
> +               *n_entries = ARRAY_SIZE(hsw_ddi_translations_dp);
> +               return hsw_ddi_translations_dp;
> +       }
> +
> +       *n_entries = 0;
> +       return NULL;
> +}
> +
> +static const struct ddi_buf_trans *
> +intel_ddi_get_buf_trans_edp(struct drm_i915_private *dev_priv,
> +                           int *n_entries)
> +{
> +       if (IS_GEN9_BC(dev_priv)) {
> +               return skl_get_buf_trans_edp(dev_priv, n_entries);
> +       } else if (IS_BROADWELL(dev_priv)) {
> +               return bdw_get_buf_trans_edp(dev_priv, n_entries);
> +       } else if (IS_HASWELL(dev_priv)) {
> +               *n_entries = ARRAY_SIZE(hsw_ddi_translations_dp);
> +               return hsw_ddi_translations_dp;
> +       }
> +
> +       *n_entries = 0;
> +       return NULL;
> +}
> +
> +static const struct ddi_buf_trans *
> +intel_ddi_get_buf_trans_fdi(struct drm_i915_private *dev_priv,
> +                           int *n_entries)
> +{
> +       if (IS_BROADWELL(dev_priv)) {
> +               *n_entries = ARRAY_SIZE(bdw_ddi_translations_fdi);
> +               return bdw_ddi_translations_fdi;
> +       } else if (IS_HASWELL(dev_priv)) {
> +               *n_entries = ARRAY_SIZE(hsw_ddi_translations_fdi);
> +               return hsw_ddi_translations_fdi;
> +       }
> +
> +       *n_entries = 0;
> +       return NULL;
> +}
> +
>  static const struct cnl_ddi_buf_trans *
>  cnl_get_buf_trans_hdmi(struct drm_i915_private *dev_priv, int *n_entries)
>  {
> @@ -692,59 +745,6 @@ static int intel_ddi_hdmi_level(struct drm_i915_private *dev_priv, enum port por
>         return hdmi_level;
>  }
>
> -static const struct ddi_buf_trans *
> -intel_ddi_get_buf_trans_dp(struct drm_i915_private *dev_priv,
> -                          int *n_entries)
> -{
> -       if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv)) {
> -               return kbl_get_buf_trans_dp(dev_priv, n_entries);
> -       } else if (IS_SKYLAKE(dev_priv)) {
> -               return skl_get_buf_trans_dp(dev_priv, n_entries);
> -       } else if (IS_BROADWELL(dev_priv)) {
> -               *n_entries = ARRAY_SIZE(bdw_ddi_translations_dp);
> -               return  bdw_ddi_translations_dp;
> -       } else if (IS_HASWELL(dev_priv)) {
> -               *n_entries = ARRAY_SIZE(hsw_ddi_translations_dp);
> -               return hsw_ddi_translations_dp;
> -       }
> -
> -       *n_entries = 0;
> -       return NULL;
> -}
> -
> -static const struct ddi_buf_trans *
> -intel_ddi_get_buf_trans_edp(struct drm_i915_private *dev_priv,
> -                           int *n_entries)
> -{
> -       if (IS_GEN9_BC(dev_priv)) {
> -               return skl_get_buf_trans_edp(dev_priv, n_entries);
> -       } else if (IS_BROADWELL(dev_priv)) {
> -               return bdw_get_buf_trans_edp(dev_priv, n_entries);
> -       } else if (IS_HASWELL(dev_priv)) {
> -               *n_entries = ARRAY_SIZE(hsw_ddi_translations_dp);
> -               return hsw_ddi_translations_dp;
> -       }
> -
> -       *n_entries = 0;
> -       return NULL;
> -}
> -
> -static const struct ddi_buf_trans *
> -intel_ddi_get_buf_trans_fdi(struct drm_i915_private *dev_priv,
> -                           int *n_entries)
> -{
> -       if (IS_BROADWELL(dev_priv)) {
> -               *n_entries = ARRAY_SIZE(bdw_ddi_translations_fdi);
> -               return bdw_ddi_translations_fdi;
> -       } else if (IS_HASWELL(dev_priv)) {
> -               *n_entries = ARRAY_SIZE(hsw_ddi_translations_fdi);
> -               return hsw_ddi_translations_fdi;
> -       }
> -
> -       *n_entries = 0;
> -       return NULL;
> -}
> -
>  /*
>   * Starting with Haswell, DDI port buffers must be programmed with correct
>   * values in advance. This function programs the correct values for
> @@ -1801,19 +1801,14 @@ static void skl_ddi_set_iboost(struct intel_encoder *encoder, u32 level)
>                 if (dp_iboost) {
>                         iboost = dp_iboost;
>                 } else {
> -                       if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv))
> -                               ddi_translations = kbl_get_buf_trans_dp(dev_priv,
> -                                                                       &n_entries);
> -                       else
> -                               ddi_translations = skl_get_buf_trans_dp(dev_priv,
> -                                                                       &n_entries);
> +                       ddi_translations = intel_ddi_get_buf_trans_dp(dev_priv, &n_entries);
>                         iboost = ddi_translations[level].i_boost;
>                 }
>         } else if (type == INTEL_OUTPUT_EDP) {
>                 if (dp_iboost) {
>                         iboost = dp_iboost;
>                 } else {
> -                       ddi_translations = skl_get_buf_trans_edp(dev_priv, &n_entries);
> +                       ddi_translations = intel_ddi_get_buf_trans_edp(dev_priv, &n_entries);
>
>                         if (WARN_ON(port != PORT_A &&
>                                     port != PORT_E && n_entries > 9))
> --
> 2.13.6
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 


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

Re: [PATCH 02/10] drm/i915: Extract intel_ddi_get_buf_trans_hdmi()

[Ausmus, James]

On Mon, Oct 16, 2017 at 7:56 AM, Ville Syrjala
<ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Introduce intel_ddi_get_buf_trans_hdmi() and start using it where we
> can.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 50 ++++++++++++++++++++++------------------
>  1 file changed, 28 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 55937abda61f..e6c884a6d408 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -641,6 +641,24 @@ intel_ddi_get_buf_trans_fdi(struct drm_i915_private *dev_priv,
>         return NULL;
>  }
>
> +static const struct ddi_buf_trans *
> +intel_ddi_get_buf_trans_hdmi(struct drm_i915_private *dev_priv,
> +                            int *n_entries)
> +{
> +       if (IS_GEN9_BC(dev_priv)) {
> +               return skl_get_buf_trans_hdmi(dev_priv, n_entries);
> +       } else if (IS_BROADWELL(dev_priv)) {
> +               *n_entries = ARRAY_SIZE(bdw_ddi_translations_hdmi);
> +               return bdw_ddi_translations_hdmi;
> +       } else if (IS_HASWELL(dev_priv)) {
> +               *n_entries = ARRAY_SIZE(hsw_ddi_translations_hdmi);
> +               return hsw_ddi_translations_hdmi;
> +       }
> +
> +       *n_entries = 0;
> +       return NULL;
> +}
> +
>  static const struct cnl_ddi_buf_trans *
>  cnl_get_buf_trans_hdmi(struct drm_i915_private *dev_priv, int *n_entries)
>  {
> @@ -723,18 +741,17 @@ static int intel_ddi_hdmi_level(struct drm_i915_private *dev_priv, enum port por
>                 cnl_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
>                 hdmi_default_entry = n_hdmi_entries - 1;

Why are you skipping the CNL case? If you extract it in to
intel_ddi_get_buf_trans_hdmi as well, you could just
unconditionally call intel_ddi_get_buf_trans_hdmi, and just set
hdmi_default_entry in the platform-specific section.

>         } else if (IS_GEN9_BC(dev_priv)) {
> -               skl_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> +               intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
>                 hdmi_default_entry = 8;
>         } else if (IS_BROADWELL(dev_priv)) {
> -               n_hdmi_entries = ARRAY_SIZE(bdw_ddi_translations_hdmi);
> +               intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
>                 hdmi_default_entry = 7;
>         } else if (IS_HASWELL(dev_priv)) {
> -               n_hdmi_entries = ARRAY_SIZE(hsw_ddi_translations_hdmi);
> +               intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
>                 hdmi_default_entry = 6;
>         } else {
>                 WARN(1, "ddi translation table missing\n");
> -               n_hdmi_entries = ARRAY_SIZE(bdw_ddi_translations_hdmi);
> -               hdmi_default_entry = 7;
> +               return 0;
>         }
>
>         /* Choose a good default if VBT is badly populated */
> @@ -810,23 +827,12 @@ static void intel_prepare_hdmi_ddi_buffers(struct intel_encoder *encoder)
>
>         hdmi_level = intel_ddi_hdmi_level(dev_priv, port);
>
> -       if (IS_GEN9_BC(dev_priv)) {
> -               ddi_translations_hdmi = skl_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> +       ddi_translations_hdmi = intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
>
> -               /* If we're boosting the current, set bit 31 of trans1 */
> -               if (dev_priv->vbt.ddi_port_info[port].hdmi_boost_level)
> -                       iboost_bit = DDI_BUF_BALANCE_LEG_ENABLE;
> -       } else if (IS_BROADWELL(dev_priv)) {
> -               ddi_translations_hdmi = bdw_ddi_translations_hdmi;
> -               n_hdmi_entries = ARRAY_SIZE(bdw_ddi_translations_hdmi);
> -       } else if (IS_HASWELL(dev_priv)) {
> -               ddi_translations_hdmi = hsw_ddi_translations_hdmi;
> -               n_hdmi_entries = ARRAY_SIZE(hsw_ddi_translations_hdmi);
> -       } else {
> -               WARN(1, "ddi translation table missing\n");
> -               ddi_translations_hdmi = bdw_ddi_translations_hdmi;
> -               n_hdmi_entries = ARRAY_SIZE(bdw_ddi_translations_hdmi);
> -       }
> +       /* If we're boosting the current, set bit 31 of trans1 */
> +       if (IS_GEN9_BC(dev_priv) &&
> +           dev_priv->vbt.ddi_port_info[port].hdmi_boost_level)
> +               iboost_bit = DDI_BUF_BALANCE_LEG_ENABLE;
>
>         /* Entry 9 is for HDMI: */
>         I915_WRITE(DDI_BUF_TRANS_LO(port, 9),
> @@ -1820,7 +1826,7 @@ static void skl_ddi_set_iboost(struct intel_encoder *encoder, u32 level)
>                 if (hdmi_iboost) {
>                         iboost = hdmi_iboost;
>                 } else {
> -                       ddi_translations = skl_get_buf_trans_hdmi(dev_priv, &n_entries);
> +                       ddi_translations = intel_ddi_get_buf_trans_hdmi(dev_priv, &n_entries);
>                         iboost = ddi_translations[level].i_boost;
>                 }
>         } else {
> --
> 2.13.6
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 


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

Re: [PATCH v2 03/10] drm/i915: Pass the encoder type explicitly to skl_set_iboost()

[Ausmus, James]

On Mon, Oct 16, 2017 at 7:56 AM, Ville Syrjala
<ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> encoder->type isn't reliable for DP/HDMI encoders, so pass the type
> explicity to skl_set_iboost(). Also take the opportunity to streamline
> the code.
>
> v2: Clean up the argument types to skl_ddi_set_iboost() while at it
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

That's a much cleaner read now!

Reviewed-by: James Ausmus <james.ausmus@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 59 ++++++++++++++++------------------------
>  1 file changed, 23 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index e6c884a6d408..cf0b2d3de15f 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1788,49 +1788,36 @@ static void _skl_ddi_set_iboost(struct drm_i915_private *dev_priv,
>         I915_WRITE(DISPIO_CR_TX_BMU_CR0, tmp);
>  }
>
> -static void skl_ddi_set_iboost(struct intel_encoder *encoder, u32 level)
> +static void skl_ddi_set_iboost(struct intel_encoder *encoder,
> +                              int level, enum intel_output_type type)
>  {
>         struct intel_digital_port *intel_dig_port = enc_to_dig_port(&encoder->base);
>         struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
>         enum port port = intel_dig_port->port;
> -       int type = encoder->type;
> -       const struct ddi_buf_trans *ddi_translations;
>         uint8_t iboost;
> -       uint8_t dp_iboost, hdmi_iboost;
> -       int n_entries;
>
> -       /* VBT may override standard boost values */
> -       dp_iboost = dev_priv->vbt.ddi_port_info[port].dp_boost_level;
> -       hdmi_iboost = dev_priv->vbt.ddi_port_info[port].hdmi_boost_level;
> +       if (type == INTEL_OUTPUT_HDMI)
> +               iboost = dev_priv->vbt.ddi_port_info[port].hdmi_boost_level;
> +       else
> +               iboost = dev_priv->vbt.ddi_port_info[port].dp_boost_level;
>
> -       if (type == INTEL_OUTPUT_DP) {
> -               if (dp_iboost) {
> -                       iboost = dp_iboost;
> -               } else {
> -                       ddi_translations = intel_ddi_get_buf_trans_dp(dev_priv, &n_entries);
> -                       iboost = ddi_translations[level].i_boost;
> -               }
> -       } else if (type == INTEL_OUTPUT_EDP) {
> -               if (dp_iboost) {
> -                       iboost = dp_iboost;
> -               } else {
> -                       ddi_translations = intel_ddi_get_buf_trans_edp(dev_priv, &n_entries);
> +       if (iboost == 0) {
> +               const struct ddi_buf_trans *ddi_translations;
> +               int n_entries;
>
> -                       if (WARN_ON(port != PORT_A &&
> -                                   port != PORT_E && n_entries > 9))
> -                               n_entries = 9;
> -
> -                       iboost = ddi_translations[level].i_boost;
> -               }
> -       } else if (type == INTEL_OUTPUT_HDMI) {
> -               if (hdmi_iboost) {
> -                       iboost = hdmi_iboost;
> -               } else {
> +               if (type == INTEL_OUTPUT_HDMI)
>                         ddi_translations = intel_ddi_get_buf_trans_hdmi(dev_priv, &n_entries);
> -                       iboost = ddi_translations[level].i_boost;
> -               }
> -       } else {
> -               return;
> +               else if (type == INTEL_OUTPUT_EDP)
> +                       ddi_translations = intel_ddi_get_buf_trans_edp(dev_priv, &n_entries);
> +               else
> +                       ddi_translations = intel_ddi_get_buf_trans_dp(dev_priv, &n_entries);
> +
> +               if (WARN_ON(type != INTEL_OUTPUT_HDMI &&
> +                           port != PORT_A &&
> +                           port != PORT_E && n_entries > 9))
> +                       n_entries = 9;
> +
> +               iboost = ddi_translations[level].i_boost;
>         }
>
>         /* Make sure that the requested I_boost is valid */
> @@ -2096,7 +2083,7 @@ uint32_t ddi_signal_levels(struct intel_dp *intel_dp)
>         uint32_t level = intel_ddi_dp_level(intel_dp);
>
>         if (IS_GEN9_BC(dev_priv))
> -           skl_ddi_set_iboost(encoder, level);
> +               skl_ddi_set_iboost(encoder, level, encoder->type);
>
>         return DDI_BUF_TRANS_SELECT(level);
>  }
> @@ -2219,7 +2206,7 @@ static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
>                 intel_prepare_hdmi_ddi_buffers(encoder);
>
>         if (IS_GEN9_BC(dev_priv))
> -               skl_ddi_set_iboost(encoder, level);
> +               skl_ddi_set_iboost(encoder, level, INTEL_OUTPUT_HDMI);
>
>         intel_dig_port->set_infoframes(&encoder->base,
>                                        crtc_state->has_infoframe,
> --
> 2.13.6
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 


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

Re: [PATCH 04/10] drm/i915: Pass the level to intel_prepare_hdmi_ddi_buffers()

[Ausmus, James]

On Mon, Oct 16, 2017 at 7:56 AM, Ville Syrjala
<ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> The caller of intel_prepare_hdmi_ddi_buffers() alreday figured out the
> level, so let's just pass it in instead if figuring it out again.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: James Ausmus <james.ausmus@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index cf0b2d3de15f..f61b6c20005e 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -817,16 +817,15 @@ static void intel_prepare_dp_ddi_buffers(struct intel_encoder *encoder)
>   * values in advance. This function programs the correct values for
>   * HDMI/DVI use cases.
>   */
> -static void intel_prepare_hdmi_ddi_buffers(struct intel_encoder *encoder)
> +static void intel_prepare_hdmi_ddi_buffers(struct intel_encoder *encoder,
> +                                          int hdmi_level)
>  {
>         struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>         u32 iboost_bit = 0;
> -       int n_hdmi_entries, hdmi_level;
> +       int n_hdmi_entries;
>         enum port port = intel_ddi_get_encoder_port(encoder);
>         const struct ddi_buf_trans *ddi_translations_hdmi;
>
> -       hdmi_level = intel_ddi_hdmi_level(dev_priv, port);
> -
>         ddi_translations_hdmi = intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
>
>         /* If we're boosting the current, set bit 31 of trans1 */
> @@ -2203,7 +2202,7 @@ static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
>                 bxt_ddi_vswing_sequence(dev_priv, level, port,
>                                         INTEL_OUTPUT_HDMI);
>         else
> -               intel_prepare_hdmi_ddi_buffers(encoder);
> +               intel_prepare_hdmi_ddi_buffers(encoder, level);
>
>         if (IS_GEN9_BC(dev_priv))
>                 skl_ddi_set_iboost(encoder, level, INTEL_OUTPUT_HDMI);
> --
> 2.13.6
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 


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

✗ Fi.CI.IGT: warning for DDI buf trans cleanup

[Patchwork]

== Series Details ==

Series: DDI buf trans cleanup
URL   : https://patchwork.freedesktop.org/series/32049/
State : warning

== Summary ==

Test kms_flip:
        Subgroup dpms-vs-vblank-race-interruptible:
                fail       -> PASS       (shard-hsw) fdo#103060
Test kms_plane:
        Subgroup plane-panning-bottom-right-suspend-pipe-B-planes:
                pass       -> SKIP       (shard-hsw)
Test pm_rpm:
        Subgroup modeset-non-lpsp:
                pass       -> SKIP       (shard-hsw)
Test kms_setmode:
        Subgroup basic:
                pass       -> FAIL       (shard-hsw) fdo#99912

fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912

shard-hsw        total:2553 pass:1439 dwarn:0   dfail:0   fail:9   skip:1105 time:9617s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6056/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 05/10] drm/i915: Integrate BXT into intel_ddi_dp_voltage_max()

[James Ausmus]

On Mon, Oct 16, 2017 at 05:57:00PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Make BXT less special by following the CNL approach and handling
> it in intel_ddi_dp_voltage_max() alognside every other DDI platform.
> 
> v2: Clean up the argument types to bxt_ddi_vswing_sequence() while at it
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: James Ausmus <james.ausmus@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 67 ++++++++++++++++++++++++++--------------
>  drivers/gpu/drm/i915/intel_dp.c  |  4 +--
>  2 files changed, 45 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index f61b6c20005e..2d886148a653 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -659,6 +659,31 @@ intel_ddi_get_buf_trans_hdmi(struct drm_i915_private *dev_priv,
>  	return NULL;
>  }
>  
> +static const struct bxt_ddi_buf_trans *
> +bxt_get_buf_trans_dp(struct drm_i915_private *dev_priv, int *n_entries)
> +{
> +	*n_entries = ARRAY_SIZE(bxt_ddi_translations_dp);
> +	return bxt_ddi_translations_dp;
> +}
> +
> +static const struct bxt_ddi_buf_trans *
> +bxt_get_buf_trans_edp(struct drm_i915_private *dev_priv, int *n_entries)
> +{
> +	if (dev_priv->vbt.edp.low_vswing) {
> +		*n_entries = ARRAY_SIZE(bxt_ddi_translations_edp);
> +		return bxt_ddi_translations_edp;
> +	}
> +
> +	return bxt_get_buf_trans_dp(dev_priv, n_entries);
> +}
> +
> +static const struct bxt_ddi_buf_trans *
> +bxt_get_buf_trans_hdmi(struct drm_i915_private *dev_priv, int *n_entries)
> +{
> +	*n_entries = ARRAY_SIZE(bxt_ddi_translations_hdmi);
> +	return bxt_ddi_translations_hdmi;
> +}
> +
>  static const struct cnl_ddi_buf_trans *
>  cnl_get_buf_trans_hdmi(struct drm_i915_private *dev_priv, int *n_entries)
>  {
> @@ -1831,27 +1856,20 @@ static void skl_ddi_set_iboost(struct intel_encoder *encoder,
>  		_skl_ddi_set_iboost(dev_priv, PORT_E, iboost);
>  }
>  
> -static void bxt_ddi_vswing_sequence(struct drm_i915_private *dev_priv,
> -				    u32 level, enum port port, int type)
> +static void bxt_ddi_vswing_sequence(struct intel_encoder *encoder,
> +				    int level, enum intel_output_type type)
>  {
> +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	const struct bxt_ddi_buf_trans *ddi_translations;
> -	u32 n_entries, i;
> +	enum port port = encoder->port;
> +	int n_entries, i;
>  
> -	if (type == INTEL_OUTPUT_EDP && dev_priv->vbt.edp.low_vswing) {
> -		n_entries = ARRAY_SIZE(bxt_ddi_translations_edp);
> -		ddi_translations = bxt_ddi_translations_edp;
> -	} else if (type == INTEL_OUTPUT_DP
> -			|| type == INTEL_OUTPUT_EDP) {
> -		n_entries = ARRAY_SIZE(bxt_ddi_translations_dp);
> -		ddi_translations = bxt_ddi_translations_dp;
> -	} else if (type == INTEL_OUTPUT_HDMI) {
> -		n_entries = ARRAY_SIZE(bxt_ddi_translations_hdmi);
> -		ddi_translations = bxt_ddi_translations_hdmi;
> -	} else {
> -		DRM_DEBUG_KMS("Vswing programming not done for encoder %d\n",
> -				type);
> -		return;
> -	}
> +	if (type == INTEL_OUTPUT_HDMI)
> +		ddi_translations = bxt_get_buf_trans_hdmi(dev_priv, &n_entries);
> +	else if (type == INTEL_OUTPUT_EDP)
> +		ddi_translations = bxt_get_buf_trans_edp(dev_priv, &n_entries);
> +	else
> +		ddi_translations = bxt_get_buf_trans_dp(dev_priv, &n_entries);
>  
>  	/* Check if default value has to be used */
>  	if (level >= n_entries ||
> @@ -1881,6 +1899,11 @@ u8 intel_ddi_dp_voltage_max(struct intel_encoder *encoder)
>  			cnl_get_buf_trans_edp(dev_priv, &n_entries);
>  		else
>  			cnl_get_buf_trans_dp(dev_priv, &n_entries);
> +	} else if (IS_GEN9_LP(dev_priv)) {
> +		if (encoder->type == INTEL_OUTPUT_EDP)
> +			bxt_get_buf_trans_edp(dev_priv, &n_entries);
> +		else
> +			bxt_get_buf_trans_dp(dev_priv, &n_entries);
>  	} else {
>  		if (encoder->type == INTEL_OUTPUT_EDP)
>  			intel_ddi_get_buf_trans_edp(dev_priv, &n_entries);
> @@ -2063,13 +2086,12 @@ u32 bxt_signal_levels(struct intel_dp *intel_dp)
>  	struct intel_digital_port *dport = dp_to_dig_port(intel_dp);
>  	struct drm_i915_private *dev_priv = to_i915(dport->base.base.dev);
>  	struct intel_encoder *encoder = &dport->base;
> -	enum port port = dport->port;
>  	u32 level = intel_ddi_dp_level(intel_dp);
>  
>  	if (IS_CANNONLAKE(dev_priv))
>  		cnl_ddi_vswing_sequence(encoder, level);
>  	else
> -		bxt_ddi_vswing_sequence(dev_priv, level, port, encoder->type);
> +		bxt_ddi_vswing_sequence(encoder, level, encoder->type);
>  
>  	return 0;
>  }
> @@ -2168,7 +2190,7 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
>  	if (IS_CANNONLAKE(dev_priv))
>  		cnl_ddi_vswing_sequence(encoder, level);
>  	else if (IS_GEN9_LP(dev_priv))
> -		bxt_ddi_vswing_sequence(dev_priv, level, port, encoder->type);
> +		bxt_ddi_vswing_sequence(encoder, level, encoder->type);
>  	else
>  		intel_prepare_dp_ddi_buffers(encoder);
>  
> @@ -2199,8 +2221,7 @@ static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
>  	if (IS_CANNONLAKE(dev_priv))
>  		cnl_ddi_vswing_sequence(encoder, level);
>  	else if (IS_GEN9_LP(dev_priv))
> -		bxt_ddi_vswing_sequence(dev_priv, level, port,
> -					INTEL_OUTPUT_HDMI);
> +		bxt_ddi_vswing_sequence(encoder, level, INTEL_OUTPUT_HDMI);
>  	else
>  		intel_prepare_hdmi_ddi_buffers(encoder, level);
>  
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 4b65cf137f79..aa75f55eeb61 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3155,9 +3155,7 @@ intel_dp_voltage_max(struct intel_dp *intel_dp)
>  	struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
>  	enum port port = dp_to_dig_port(intel_dp)->port;
>  
> -	if (IS_GEN9_LP(dev_priv))
> -		return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
> -	else if (INTEL_GEN(dev_priv) >= 9) {
> +	if (INTEL_GEN(dev_priv) >= 9) {
>  		struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
>  		return intel_ddi_dp_voltage_max(encoder);
>  	} else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> -- 
> 2.13.6
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 06/10] drm/i915: Pass encoder type to cnl_ddi_vswing_sequence() explicitly

[James Ausmus]

On Mon, Oct 16, 2017 at 05:57:01PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> encoder->type is unreliable for DP/HDMI, so pass it in explicity into
> cnl_ddi_vswing_sequence(). This matches what we do for BXT.
> 
> v2: Pass intel_encoder down to cnl_ddi_vswing_program(), and
>     clean up the argument types while at it
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: James Ausmus <james.ausmus@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 53 +++++++++++++++++++---------------------
>  1 file changed, 25 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 2d886148a653..cab72177299c 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1920,20 +1920,21 @@ u8 intel_ddi_dp_voltage_max(struct intel_encoder *encoder)
>  		DP_TRAIN_VOLTAGE_SWING_MASK;
>  }
>  
> -static void cnl_ddi_vswing_program(struct drm_i915_private *dev_priv,
> -				    u32 level, enum port port, int type)
> +static void cnl_ddi_vswing_program(struct intel_encoder *encoder,
> +				   int level, enum intel_output_type type)
>  {
> -	const struct cnl_ddi_buf_trans *ddi_translations = NULL;
> -	u32 n_entries, val;
> -	int ln;
> +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> +	enum port port = intel_ddi_get_encoder_port(encoder);
> +	const struct cnl_ddi_buf_trans *ddi_translations;
> +	int n_entries, ln;
> +	u32 val;
>  
> -	if (type == INTEL_OUTPUT_HDMI) {
> +	if (type == INTEL_OUTPUT_HDMI)
>  		ddi_translations = cnl_get_buf_trans_hdmi(dev_priv, &n_entries);
> -	} else if (type == INTEL_OUTPUT_DP) {
> -		ddi_translations = cnl_get_buf_trans_dp(dev_priv, &n_entries);
> -	} else if (type == INTEL_OUTPUT_EDP) {
> +	else if (type == INTEL_OUTPUT_EDP)
>  		ddi_translations = cnl_get_buf_trans_edp(dev_priv, &n_entries);
> -	}
> +	else
> +		ddi_translations = cnl_get_buf_trans_dp(dev_priv, &n_entries);
>  
>  	if (WARN_ON(ddi_translations == NULL))
>  		return;
> @@ -1986,26 +1987,22 @@ static void cnl_ddi_vswing_program(struct drm_i915_private *dev_priv,
>  	I915_WRITE(CNL_PORT_TX_DW7_GRP(port), val);
>  }
>  
> -static void cnl_ddi_vswing_sequence(struct intel_encoder *encoder, u32 level)
> +static void cnl_ddi_vswing_sequence(struct intel_encoder *encoder,
> +				    int level, enum intel_output_type type)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> -	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
>  	enum port port = intel_ddi_get_encoder_port(encoder);
> -	int type = encoder->type;
> -	int width = 0;
> -	int rate = 0;
> +	int width, rate, ln;
>  	u32 val;
> -	int ln = 0;
>  
> -	if ((intel_dp) && (type == INTEL_OUTPUT_EDP || type == INTEL_OUTPUT_DP)) {
> +	if (type == INTEL_OUTPUT_HDMI) {
> +		width = 4;
> +		rate = 0; /* Rate is always < than 6GHz for HDMI */
> +	} else {
> +		struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> +
>  		width = intel_dp->lane_count;
>  		rate = intel_dp->link_rate;
> -	} else if (type == INTEL_OUTPUT_HDMI) {
> -		width = 4;
> -		/* Rate is always < than 6GHz for HDMI */
> -	} else {
> -		MISSING_CASE(type);
> -		return;
>  	}
>  
>  	/*
> @@ -2014,7 +2011,7 @@ static void cnl_ddi_vswing_sequence(struct intel_encoder *encoder, u32 level)
>  	 * else clear to 0b.
>  	 */
>  	val = I915_READ(CNL_PORT_PCS_DW1_LN0(port));
> -	if (type == INTEL_OUTPUT_EDP || type == INTEL_OUTPUT_DP)
> +	if (type != INTEL_OUTPUT_HDMI)
>  		val |= COMMON_KEEPER_EN;
>  	else
>  		val &= ~COMMON_KEEPER_EN;
> @@ -2049,7 +2046,7 @@ static void cnl_ddi_vswing_sequence(struct intel_encoder *encoder, u32 level)
>  	I915_WRITE(CNL_PORT_TX_DW5_GRP(port), val);
>  
>  	/* 5. Program swing and de-emphasis */
> -	cnl_ddi_vswing_program(dev_priv, level, port, type);
> +	cnl_ddi_vswing_program(encoder, level, type);
>  
>  	/* 6. Set training enable to trigger update */
>  	val = I915_READ(CNL_PORT_TX_DW5_LN0(port));
> @@ -2089,7 +2086,7 @@ u32 bxt_signal_levels(struct intel_dp *intel_dp)
>  	u32 level = intel_ddi_dp_level(intel_dp);
>  
>  	if (IS_CANNONLAKE(dev_priv))
> -		cnl_ddi_vswing_sequence(encoder, level);
> +		cnl_ddi_vswing_sequence(encoder, level, encoder->type);
>  	else
>  		bxt_ddi_vswing_sequence(encoder, level, encoder->type);
>  
> @@ -2188,7 +2185,7 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
>  	intel_display_power_get(dev_priv, dig_port->ddi_io_power_domain);
>  
>  	if (IS_CANNONLAKE(dev_priv))
> -		cnl_ddi_vswing_sequence(encoder, level);
> +		cnl_ddi_vswing_sequence(encoder, level, encoder->type);
>  	else if (IS_GEN9_LP(dev_priv))
>  		bxt_ddi_vswing_sequence(encoder, level, encoder->type);
>  	else
> @@ -2219,7 +2216,7 @@ static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
>  	intel_display_power_get(dev_priv, dig_port->ddi_io_power_domain);
>  
>  	if (IS_CANNONLAKE(dev_priv))
> -		cnl_ddi_vswing_sequence(encoder, level);
> +		cnl_ddi_vswing_sequence(encoder, level, INTEL_OUTPUT_HDMI);
>  	else if (IS_GEN9_LP(dev_priv))
>  		bxt_ddi_vswing_sequence(encoder, level, INTEL_OUTPUT_HDMI);
>  	else
> -- 
> 2.13.6
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 07/10] drm/i915: Kill off the BXT buf_trans default_index

[James Ausmus]

On Mon, Oct 16, 2017 at 05:57:02PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> default_index contained in the BXT buf_trans tables is actually useless.
> For DP we should always have a valid level selected (otherwise the link
> training logic would be buggy), and for HDMI we can just do what the
> other platforms do and pick the correct entry in intel_ddi_hdmi_level().
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: James Ausmus <james.ausmus@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 80 +++++++++++++++++-----------------------
>  1 file changed, 34 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index cab72177299c..3bce6bddeaa9 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -305,35 +305,34 @@ struct bxt_ddi_buf_trans {
>  	u8 scale;	/* scale value */
>  	u8 enable;	/* scale enable */
>  	u8 deemphasis;
> -	bool default_index; /* true if the entry represents default value */
>  };
>  
>  static const struct bxt_ddi_buf_trans bxt_ddi_translations_dp[] = {
>  					/* Idx	NT mV diff	db  */
> -	{ 52,  0x9A, 0, 128, true  },	/* 0:	400		0   */
> -	{ 78,  0x9A, 0, 85,  false },	/* 1:	400		3.5 */
> -	{ 104, 0x9A, 0, 64,  false },	/* 2:	400		6   */
> -	{ 154, 0x9A, 0, 43,  false },	/* 3:	400		9.5 */
> -	{ 77,  0x9A, 0, 128, false },	/* 4:	600		0   */
> -	{ 116, 0x9A, 0, 85,  false },	/* 5:	600		3.5 */
> -	{ 154, 0x9A, 0, 64,  false },	/* 6:	600		6   */
> -	{ 102, 0x9A, 0, 128, false },	/* 7:	800		0   */
> -	{ 154, 0x9A, 0, 85,  false },	/* 8:	800		3.5 */
> -	{ 154, 0x9A, 1, 128, false },	/* 9:	1200		0   */
> +	{ 52,  0x9A, 0, 128, },	/* 0:	400		0   */
> +	{ 78,  0x9A, 0, 85,  },	/* 1:	400		3.5 */
> +	{ 104, 0x9A, 0, 64,  },	/* 2:	400		6   */
> +	{ 154, 0x9A, 0, 43,  },	/* 3:	400		9.5 */
> +	{ 77,  0x9A, 0, 128, },	/* 4:	600		0   */
> +	{ 116, 0x9A, 0, 85,  },	/* 5:	600		3.5 */
> +	{ 154, 0x9A, 0, 64,  },	/* 6:	600		6   */
> +	{ 102, 0x9A, 0, 128, },	/* 7:	800		0   */
> +	{ 154, 0x9A, 0, 85,  },	/* 8:	800		3.5 */
> +	{ 154, 0x9A, 1, 128, },	/* 9:	1200		0   */
>  };
>  
>  static const struct bxt_ddi_buf_trans bxt_ddi_translations_edp[] = {
>  					/* Idx	NT mV diff	db  */
> -	{ 26, 0, 0, 128, false },	/* 0:	200		0   */
> -	{ 38, 0, 0, 112, false },	/* 1:	200		1.5 */
> -	{ 48, 0, 0, 96,  false },	/* 2:	200		4   */
> -	{ 54, 0, 0, 69,  false },	/* 3:	200		6   */
> -	{ 32, 0, 0, 128, false },	/* 4:	250		0   */
> -	{ 48, 0, 0, 104, false },	/* 5:	250		1.5 */
> -	{ 54, 0, 0, 85,  false },	/* 6:	250		4   */
> -	{ 43, 0, 0, 128, false },	/* 7:	300		0   */
> -	{ 54, 0, 0, 101, false },	/* 8:	300		1.5 */
> -	{ 48, 0, 0, 128, false },	/* 9:	300		0   */
> +	{ 26, 0, 0, 128, },	/* 0:	200		0   */
> +	{ 38, 0, 0, 112, },	/* 1:	200		1.5 */
> +	{ 48, 0, 0, 96,  },	/* 2:	200		4   */
> +	{ 54, 0, 0, 69,  },	/* 3:	200		6   */
> +	{ 32, 0, 0, 128, },	/* 4:	250		0   */
> +	{ 48, 0, 0, 104, },	/* 5:	250		1.5 */
> +	{ 54, 0, 0, 85,  },	/* 6:	250		4   */
> +	{ 43, 0, 0, 128, },	/* 7:	300		0   */
> +	{ 54, 0, 0, 101, },	/* 8:	300		1.5 */
> +	{ 48, 0, 0, 128, },	/* 9:	300		0   */
>  };
>  
>  /* BSpec has 2 recommended values - entries 0 and 8.
> @@ -341,16 +340,16 @@ static const struct bxt_ddi_buf_trans bxt_ddi_translations_edp[] = {
>   */
>  static const struct bxt_ddi_buf_trans bxt_ddi_translations_hdmi[] = {
>  					/* Idx	NT mV diff	db  */
> -	{ 52,  0x9A, 0, 128, false },	/* 0:	400		0   */
> -	{ 52,  0x9A, 0, 85,  false },	/* 1:	400		3.5 */
> -	{ 52,  0x9A, 0, 64,  false },	/* 2:	400		6   */
> -	{ 42,  0x9A, 0, 43,  false },	/* 3:	400		9.5 */
> -	{ 77,  0x9A, 0, 128, false },	/* 4:	600		0   */
> -	{ 77,  0x9A, 0, 85,  false },	/* 5:	600		3.5 */
> -	{ 77,  0x9A, 0, 64,  false },	/* 6:	600		6   */
> -	{ 102, 0x9A, 0, 128, false },	/* 7:	800		0   */
> -	{ 102, 0x9A, 0, 85,  false },	/* 8:	800		3.5 */
> -	{ 154, 0x9A, 1, 128, true },	/* 9:	1200		0   */
> +	{ 52,  0x9A, 0, 128, },	/* 0:	400		0   */
> +	{ 52,  0x9A, 0, 85,  },	/* 1:	400		3.5 */
> +	{ 52,  0x9A, 0, 64,  },	/* 2:	400		6   */
> +	{ 42,  0x9A, 0, 43,  },	/* 3:	400		9.5 */
> +	{ 77,  0x9A, 0, 128, },	/* 4:	600		0   */
> +	{ 77,  0x9A, 0, 85,  },	/* 5:	600		3.5 */
> +	{ 77,  0x9A, 0, 64,  },	/* 6:	600		6   */
> +	{ 102, 0x9A, 0, 128, },	/* 7:	800		0   */
> +	{ 102, 0x9A, 0, 85,  },	/* 8:	800		3.5 */
> +	{ 154, 0x9A, 1, 128, },	/* 9:	1200		0   */
>  };
>  
>  struct cnl_ddi_buf_trans {
> @@ -759,12 +758,12 @@ static int intel_ddi_hdmi_level(struct drm_i915_private *dev_priv, enum port por
>  
>  	hdmi_level = dev_priv->vbt.ddi_port_info[port].hdmi_level_shift;
>  
> -	if (IS_GEN9_LP(dev_priv))
> -		return hdmi_level;
> -
>  	if (IS_CANNONLAKE(dev_priv)) {
>  		cnl_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
>  		hdmi_default_entry = n_hdmi_entries - 1;
> +	} else if (IS_GEN9_LP(dev_priv)) {
> +		bxt_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> +		hdmi_default_entry = n_hdmi_entries - 1;
>  	} else if (IS_GEN9_BC(dev_priv)) {
>  		intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
>  		hdmi_default_entry = 8;
> @@ -1862,7 +1861,7 @@ static void bxt_ddi_vswing_sequence(struct intel_encoder *encoder,
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	const struct bxt_ddi_buf_trans *ddi_translations;
>  	enum port port = encoder->port;
> -	int n_entries, i;
> +	int n_entries;
>  
>  	if (type == INTEL_OUTPUT_HDMI)
>  		ddi_translations = bxt_get_buf_trans_hdmi(dev_priv, &n_entries);
> @@ -1871,17 +1870,6 @@ static void bxt_ddi_vswing_sequence(struct intel_encoder *encoder,
>  	else
>  		ddi_translations = bxt_get_buf_trans_dp(dev_priv, &n_entries);
>  
> -	/* Check if default value has to be used */
> -	if (level >= n_entries ||
> -	    (type == INTEL_OUTPUT_HDMI && level == HDMI_LEVEL_SHIFT_UNKNOWN)) {
> -		for (i = 0; i < n_entries; i++) {
> -			if (ddi_translations[i].default_index) {
> -				level = i;
> -				break;
> -			}
> -		}
> -	}
> -
>  	bxt_ddi_phy_set_signal_level(dev_priv, port,
>  				     ddi_translations[level].margin,
>  				     ddi_translations[level].scale,
> -- 
> 2.13.6
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 02/10] drm/i915: Extract intel_ddi_get_buf_trans_hdmi()

[Ville Syrjälä]

On Mon, Oct 16, 2017 at 04:41:49PM -0700, Ausmus, James wrote:
> On Mon, Oct 16, 2017 at 7:56 AM, Ville Syrjala
> <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Introduce intel_ddi_get_buf_trans_hdmi() and start using it where we
> > can.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_ddi.c | 50 ++++++++++++++++++++++------------------
> >  1 file changed, 28 insertions(+), 22 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > index 55937abda61f..e6c884a6d408 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -641,6 +641,24 @@ intel_ddi_get_buf_trans_fdi(struct drm_i915_private *dev_priv,
> >         return NULL;
> >  }
> >
> > +static const struct ddi_buf_trans *
> > +intel_ddi_get_buf_trans_hdmi(struct drm_i915_private *dev_priv,
> > +                            int *n_entries)
> > +{
> > +       if (IS_GEN9_BC(dev_priv)) {
> > +               return skl_get_buf_trans_hdmi(dev_priv, n_entries);
> > +       } else if (IS_BROADWELL(dev_priv)) {
> > +               *n_entries = ARRAY_SIZE(bdw_ddi_translations_hdmi);
> > +               return bdw_ddi_translations_hdmi;
> > +       } else if (IS_HASWELL(dev_priv)) {
> > +               *n_entries = ARRAY_SIZE(hsw_ddi_translations_hdmi);
> > +               return hsw_ddi_translations_hdmi;
> > +       }
> > +
> > +       *n_entries = 0;
> > +       return NULL;
> > +}
> > +
> >  static const struct cnl_ddi_buf_trans *
> >  cnl_get_buf_trans_hdmi(struct drm_i915_private *dev_priv, int *n_entries)
> >  {
> > @@ -723,18 +741,17 @@ static int intel_ddi_hdmi_level(struct drm_i915_private *dev_priv, enum port por
> >                 cnl_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> >                 hdmi_default_entry = n_hdmi_entries - 1;
> 
> Why are you skipping the CNL case? If you extract it in to
> intel_ddi_get_buf_trans_hdmi as well, you could just
> unconditionally call intel_ddi_get_buf_trans_hdmi, and just set
> hdmi_default_entry in the platform-specific section.

The problem is that CNL (and BXT) use a different structure for their
buf trans values. So far I've not bothered to check if we could change
them to use the HSW+ structure. I guess the other options would be to
slap a union into the structure, or make these functions return
void *, but neither option seems very appealing since we'd lose some
type safety.

In any case that'd be material for followup work. Maybe someone else
could take a look at it? ;)

> 
> >         } else if (IS_GEN9_BC(dev_priv)) {
> > -               skl_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> > +               intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> >                 hdmi_default_entry = 8;
> >         } else if (IS_BROADWELL(dev_priv)) {
> > -               n_hdmi_entries = ARRAY_SIZE(bdw_ddi_translations_hdmi);
> > +               intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> >                 hdmi_default_entry = 7;
> >         } else if (IS_HASWELL(dev_priv)) {
> > -               n_hdmi_entries = ARRAY_SIZE(hsw_ddi_translations_hdmi);
> > +               intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> >                 hdmi_default_entry = 6;
> >         } else {
> >                 WARN(1, "ddi translation table missing\n");
> > -               n_hdmi_entries = ARRAY_SIZE(bdw_ddi_translations_hdmi);
> > -               hdmi_default_entry = 7;
> > +               return 0;
> >         }
> >
> >         /* Choose a good default if VBT is badly populated */
> > @@ -810,23 +827,12 @@ static void intel_prepare_hdmi_ddi_buffers(struct intel_encoder *encoder)
> >
> >         hdmi_level = intel_ddi_hdmi_level(dev_priv, port);
> >
> > -       if (IS_GEN9_BC(dev_priv)) {
> > -               ddi_translations_hdmi = skl_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> > +       ddi_translations_hdmi = intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> >
> > -               /* If we're boosting the current, set bit 31 of trans1 */
> > -               if (dev_priv->vbt.ddi_port_info[port].hdmi_boost_level)
> > -                       iboost_bit = DDI_BUF_BALANCE_LEG_ENABLE;
> > -       } else if (IS_BROADWELL(dev_priv)) {
> > -               ddi_translations_hdmi = bdw_ddi_translations_hdmi;
> > -               n_hdmi_entries = ARRAY_SIZE(bdw_ddi_translations_hdmi);
> > -       } else if (IS_HASWELL(dev_priv)) {
> > -               ddi_translations_hdmi = hsw_ddi_translations_hdmi;
> > -               n_hdmi_entries = ARRAY_SIZE(hsw_ddi_translations_hdmi);
> > -       } else {
> > -               WARN(1, "ddi translation table missing\n");
> > -               ddi_translations_hdmi = bdw_ddi_translations_hdmi;
> > -               n_hdmi_entries = ARRAY_SIZE(bdw_ddi_translations_hdmi);
> > -       }
> > +       /* If we're boosting the current, set bit 31 of trans1 */
> > +       if (IS_GEN9_BC(dev_priv) &&
> > +           dev_priv->vbt.ddi_port_info[port].hdmi_boost_level)
> > +               iboost_bit = DDI_BUF_BALANCE_LEG_ENABLE;
> >
> >         /* Entry 9 is for HDMI: */
> >         I915_WRITE(DDI_BUF_TRANS_LO(port, 9),
> > @@ -1820,7 +1826,7 @@ static void skl_ddi_set_iboost(struct intel_encoder *encoder, u32 level)
> >                 if (hdmi_iboost) {
> >                         iboost = hdmi_iboost;
> >                 } else {
> > -                       ddi_translations = skl_get_buf_trans_hdmi(dev_priv, &n_entries);
> > +                       ddi_translations = intel_ddi_get_buf_trans_hdmi(dev_priv, &n_entries);
> >                         iboost = ddi_translations[level].i_boost;
> >                 }
> >         } else {
> > --
> > 2.13.6
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
> -- 
> 
> 
> James Ausmus

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 08/10] drm/i915: Centralize the SKL DDI A/E vs. B/C/D buf trans handling

[James Ausmus]

On Mon, Oct 16, 2017 at 05:57:03PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> SKL DDI B/C/D only have 9 usable buf trans registers for DP/eDP. That
> matches the normal DP buf trans tables, but the low vswing eDP tables
> have 10 entries. Thus the eDP tables can only be used safely with DDI A
> and E.
> 
> We try to catch cases where DDI B/C/D gets used with the wrong number of
> entires in some parts of the code, but not everywhere. Let's move the
> code to deal with that deeper into intel_ddi_get_buf_trans_edp(). And
> for sake of symmetry do the same in intel_ddi_get_buf_trans_dp(). That
> would also avoid explosions in the rather unlikely case that the DP
> tables would get revised to 10 entries as well.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: James Ausmus <james.ausmus@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 60 +++++++++++++++++++++++-----------------
>  1 file changed, 34 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 3bce6bddeaa9..f0a709b66e00 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -587,14 +587,29 @@ skl_get_buf_trans_hdmi(struct drm_i915_private *dev_priv, int *n_entries)
>  	}
>  }
>  
> +static int skl_buf_trans_num_entries(enum port port, int n_entries)
> +{
> +	/* Only DDIA and DDIE can select the 10th register with DP */
> +	if (port == PORT_A || port == PORT_E)
> +		return min(n_entries, 10);
> +	else
> +		return min(n_entries, 9);
> +}
> +
>  static const struct ddi_buf_trans *
>  intel_ddi_get_buf_trans_dp(struct drm_i915_private *dev_priv,
> -			   int *n_entries)
> +			   enum port port, int *n_entries)
>  {
>  	if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv)) {
> -		return kbl_get_buf_trans_dp(dev_priv, n_entries);
> +		const struct ddi_buf_trans *ddi_translations =
> +			kbl_get_buf_trans_dp(dev_priv, n_entries);
> +		*n_entries = skl_buf_trans_num_entries(port, *n_entries);
> +		return ddi_translations;
>  	} else if (IS_SKYLAKE(dev_priv)) {
> -		return skl_get_buf_trans_dp(dev_priv, n_entries);
> +		const struct ddi_buf_trans *ddi_translations =
> +			skl_get_buf_trans_dp(dev_priv, n_entries);
> +		*n_entries = skl_buf_trans_num_entries(port, *n_entries);
> +		return ddi_translations;
>  	} else if (IS_BROADWELL(dev_priv)) {
>  		*n_entries = ARRAY_SIZE(bdw_ddi_translations_dp);
>  		return  bdw_ddi_translations_dp;
> @@ -609,10 +624,13 @@ intel_ddi_get_buf_trans_dp(struct drm_i915_private *dev_priv,
>  
>  static const struct ddi_buf_trans *
>  intel_ddi_get_buf_trans_edp(struct drm_i915_private *dev_priv,
> -			    int *n_entries)
> +			    enum port port, int *n_entries)
>  {
>  	if (IS_GEN9_BC(dev_priv)) {
> -		return skl_get_buf_trans_edp(dev_priv, n_entries);
> +		const struct ddi_buf_trans *ddi_translations =
> +			skl_get_buf_trans_edp(dev_priv, n_entries);
> +		*n_entries = skl_buf_trans_num_entries(port, *n_entries);
> +		return ddi_translations;
>  	} else if (IS_BROADWELL(dev_priv)) {
>  		return bdw_get_buf_trans_edp(dev_priv, n_entries);
>  	} else if (IS_HASWELL(dev_priv)) {
> @@ -801,11 +819,11 @@ static void intel_prepare_dp_ddi_buffers(struct intel_encoder *encoder)
>  
>  	switch (encoder->type) {
>  	case INTEL_OUTPUT_EDP:
> -		ddi_translations = intel_ddi_get_buf_trans_edp(dev_priv,
> +		ddi_translations = intel_ddi_get_buf_trans_edp(dev_priv, port,
>  							       &n_entries);
>  		break;
>  	case INTEL_OUTPUT_DP:
> -		ddi_translations = intel_ddi_get_buf_trans_dp(dev_priv,
> +		ddi_translations = intel_ddi_get_buf_trans_dp(dev_priv, port,
>  							      &n_entries);
>  		break;
>  	case INTEL_OUTPUT_ANALOG:
> @@ -817,16 +835,10 @@ static void intel_prepare_dp_ddi_buffers(struct intel_encoder *encoder)
>  		return;
>  	}
>  
> -	if (IS_GEN9_BC(dev_priv)) {
> -		/* If we're boosting the current, set bit 31 of trans1 */
> -		if (dev_priv->vbt.ddi_port_info[port].dp_boost_level)
> -			iboost_bit = DDI_BUF_BALANCE_LEG_ENABLE;
> -
> -		if (WARN_ON(encoder->type == INTEL_OUTPUT_EDP &&
> -			    port != PORT_A && port != PORT_E &&
> -			    n_entries > 9))
> -			n_entries = 9;
> -	}
> +	/* If we're boosting the current, set bit 31 of trans1 */
> +	if (IS_GEN9_BC(dev_priv) &&
> +	    dev_priv->vbt.ddi_port_info[port].dp_boost_level)
> +		iboost_bit = DDI_BUF_BALANCE_LEG_ENABLE;
>  
>  	for (i = 0; i < n_entries; i++) {
>  		I915_WRITE(DDI_BUF_TRANS_LO(port, i),
> @@ -1831,14 +1843,9 @@ static void skl_ddi_set_iboost(struct intel_encoder *encoder,
>  		if (type == INTEL_OUTPUT_HDMI)
>  			ddi_translations = intel_ddi_get_buf_trans_hdmi(dev_priv, &n_entries);
>  		else if (type == INTEL_OUTPUT_EDP)
> -			ddi_translations = intel_ddi_get_buf_trans_edp(dev_priv, &n_entries);
> +			ddi_translations = intel_ddi_get_buf_trans_edp(dev_priv, port, &n_entries);
>  		else
> -			ddi_translations = intel_ddi_get_buf_trans_dp(dev_priv, &n_entries);
> -
> -		if (WARN_ON(type != INTEL_OUTPUT_HDMI &&
> -			    port != PORT_A &&
> -			    port != PORT_E && n_entries > 9))
> -			n_entries = 9;
> +			ddi_translations = intel_ddi_get_buf_trans_dp(dev_priv, port, &n_entries);
>  
>  		iboost = ddi_translations[level].i_boost;
>  	}
> @@ -1880,6 +1887,7 @@ static void bxt_ddi_vswing_sequence(struct intel_encoder *encoder,
>  u8 intel_ddi_dp_voltage_max(struct intel_encoder *encoder)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> +	enum port port = encoder->port;
>  	int n_entries;
>  
>  	if (IS_CANNONLAKE(dev_priv)) {
> @@ -1894,9 +1902,9 @@ u8 intel_ddi_dp_voltage_max(struct intel_encoder *encoder)
>  			bxt_get_buf_trans_dp(dev_priv, &n_entries);
>  	} else {
>  		if (encoder->type == INTEL_OUTPUT_EDP)
> -			intel_ddi_get_buf_trans_edp(dev_priv, &n_entries);
> +			intel_ddi_get_buf_trans_edp(dev_priv, port, &n_entries);
>  		else
> -			intel_ddi_get_buf_trans_dp(dev_priv, &n_entries);
> +			intel_ddi_get_buf_trans_dp(dev_priv, port, &n_entries);
>  	}
>  
>  	if (WARN_ON(n_entries < 1))
> -- 
> 2.13.6
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 09/10] drm/i915: Unify error handling for missing DDI buf trans tables

[James Ausmus]

On Mon, Oct 16, 2017 at 05:57:04PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Handle missing buf trans tables, or out of bounds buf trans levels
> the same way everywhere. These should never be hit under normal
> conditions, but let's play it safe for now.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 25 ++++++++++++++++++++-----
>  1 file changed, 20 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index f0a709b66e00..5f1c546e5e61 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -801,6 +801,9 @@ static int intel_ddi_hdmi_level(struct drm_i915_private *dev_priv, enum port por
>  	    hdmi_level >= n_hdmi_entries)
>  		hdmi_level = hdmi_default_entry;
>  
> +	if (WARN_ON_ONCE(hdmi_level >= n_hdmi_entries))
> +		hdmi_level = n_hdmi_entries - 1;

intel_ddi_get_buf_trans_* can set n_entries to 0 on failure,
which would cause this to return a level of -1 - maybe those functions
need to return 1 on failure instead, similar to the cnl_get_buf_trans_*
functions?

> +
>  	return hdmi_level;
>  }
>  
> @@ -864,6 +867,11 @@ static void intel_prepare_hdmi_ddi_buffers(struct intel_encoder *encoder,
>  
>  	ddi_translations_hdmi = intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
>  
> +	if (WARN_ON_ONCE(!ddi_translations_hdmi))
> +		return;
> +	if (WARN_ON_ONCE(hdmi_level >= n_hdmi_entries))
> +		hdmi_level = n_hdmi_entries - 1;
> +
>  	/* If we're boosting the current, set bit 31 of trans1 */
>  	if (IS_GEN9_BC(dev_priv) &&
>  	    dev_priv->vbt.ddi_port_info[port].hdmi_boost_level)
> @@ -1847,6 +1855,11 @@ static void skl_ddi_set_iboost(struct intel_encoder *encoder,
>  		else
>  			ddi_translations = intel_ddi_get_buf_trans_dp(dev_priv, port, &n_entries);
>  
> +		if (WARN_ON_ONCE(!ddi_translations))
> +			return;
> +		if (WARN_ON_ONCE(level >= n_entries))
> +			level = n_entries - 1;


> +
>  		iboost = ddi_translations[level].i_boost;
>  	}
>  
> @@ -1877,6 +1890,11 @@ static void bxt_ddi_vswing_sequence(struct intel_encoder *encoder,
>  	else
>  		ddi_translations = bxt_get_buf_trans_dp(dev_priv, &n_entries);
>  
> +	if (WARN_ON_ONCE(!ddi_translations))
> +		return;
> +	if (WARN_ON_ONCE(level >= n_entries))
> +		level = n_entries - 1;
> +
>  	bxt_ddi_phy_set_signal_level(dev_priv, port,
>  				     ddi_translations[level].margin,
>  				     ddi_translations[level].scale,
> @@ -1932,13 +1950,10 @@ static void cnl_ddi_vswing_program(struct intel_encoder *encoder,
>  	else
>  		ddi_translations = cnl_get_buf_trans_dp(dev_priv, &n_entries);
>  
> -	if (WARN_ON(ddi_translations == NULL))
> +	if (WARN_ON_ONCE(!ddi_translations))
>  		return;
> -
> -	if (level >= n_entries) {
> -		DRM_DEBUG_KMS("DDI translation not found for level %d. Using %d instead.", level, n_entries - 1);
> +	if (WARN_ON_ONCE(level >= n_entries))
>  		level = n_entries - 1;
> -	}
>  
>  	/* Set PORT_TX_DW5 Scaling Mode Sel to 010b. */
>  	val = I915_READ(CNL_PORT_TX_DW5_LN0(port));
> -- 
> 2.13.6
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 10/10] drm/i915: Drop the redundant hdmi prefix/suffix from a lot of variables

[James Ausmus]

On Mon, Oct 16, 2017 at 05:57:05PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> A bunch of functions are now exclusively used for HDMI, so naming the
> variables with hdmi prefix/suffix is redundant. Also use int rather
> than u32 for the translation level consistently.

Is there a case for level to be negative, ever? If not, keeping it u32
could help catch cases where we accidently try to set level to -1.

> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 61 +++++++++++++++++++---------------------
>  1 file changed, 29 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 5f1c546e5e61..cd550aab332b 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -770,41 +770,38 @@ cnl_get_buf_trans_edp(struct drm_i915_private *dev_priv, int *n_entries)
>  
>  static int intel_ddi_hdmi_level(struct drm_i915_private *dev_priv, enum port port)
>  {
> -	int n_hdmi_entries;
> -	int hdmi_level;
> -	int hdmi_default_entry;
> +	int n_entries, level, default_entry;
>  
> -	hdmi_level = dev_priv->vbt.ddi_port_info[port].hdmi_level_shift;
> +	level = dev_priv->vbt.ddi_port_info[port].hdmi_level_shift;
>  
>  	if (IS_CANNONLAKE(dev_priv)) {
> -		cnl_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> -		hdmi_default_entry = n_hdmi_entries - 1;
> +		cnl_get_buf_trans_hdmi(dev_priv, &n_entries);
> +		default_entry = n_entries - 1;
>  	} else if (IS_GEN9_LP(dev_priv)) {
> -		bxt_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> -		hdmi_default_entry = n_hdmi_entries - 1;
> +		bxt_get_buf_trans_hdmi(dev_priv, &n_entries);
> +		default_entry = n_entries - 1;
>  	} else if (IS_GEN9_BC(dev_priv)) {
> -		intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> -		hdmi_default_entry = 8;
> +		intel_ddi_get_buf_trans_hdmi(dev_priv, &n_entries);
> +		default_entry = 8;
>  	} else if (IS_BROADWELL(dev_priv)) {
> -		intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> -		hdmi_default_entry = 7;
> +		intel_ddi_get_buf_trans_hdmi(dev_priv, &n_entries);
> +		default_entry = 7;
>  	} else if (IS_HASWELL(dev_priv)) {
> -		intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> -		hdmi_default_entry = 6;
> +		intel_ddi_get_buf_trans_hdmi(dev_priv, &n_entries);
> +		default_entry = 6;
>  	} else {
>  		WARN(1, "ddi translation table missing\n");
>  		return 0;
>  	}
>  
>  	/* Choose a good default if VBT is badly populated */
> -	if (hdmi_level == HDMI_LEVEL_SHIFT_UNKNOWN ||
> -	    hdmi_level >= n_hdmi_entries)
> -		hdmi_level = hdmi_default_entry;
> +	if (level == HDMI_LEVEL_SHIFT_UNKNOWN || level >= n_entries)
> +		level = default_entry;
>  
> -	if (WARN_ON_ONCE(hdmi_level >= n_hdmi_entries))
> -		hdmi_level = n_hdmi_entries - 1;
> +	if (WARN_ON_ONCE(level >= n_entries))
> +		level = n_entries - 1;
>  
> -	return hdmi_level;
> +	return level;
>  }
>  
>  /*
> @@ -857,20 +854,20 @@ static void intel_prepare_dp_ddi_buffers(struct intel_encoder *encoder)
>   * HDMI/DVI use cases.
>   */
>  static void intel_prepare_hdmi_ddi_buffers(struct intel_encoder *encoder,
> -					   int hdmi_level)
> +					   int level)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	u32 iboost_bit = 0;
> -	int n_hdmi_entries;
> +	int n_entries;
>  	enum port port = intel_ddi_get_encoder_port(encoder);
> -	const struct ddi_buf_trans *ddi_translations_hdmi;
> +	const struct ddi_buf_trans *ddi_translations;
>  
> -	ddi_translations_hdmi = intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> +	ddi_translations = intel_ddi_get_buf_trans_hdmi(dev_priv, &n_entries);
>  
> -	if (WARN_ON_ONCE(!ddi_translations_hdmi))
> +	if (WARN_ON_ONCE(!ddi_translations))
>  		return;
> -	if (WARN_ON_ONCE(hdmi_level >= n_hdmi_entries))
> -		hdmi_level = n_hdmi_entries - 1;
> +	if (WARN_ON_ONCE(level >= n_entries))
> +		level = n_entries - 1;
>  
>  	/* If we're boosting the current, set bit 31 of trans1 */
>  	if (IS_GEN9_BC(dev_priv) &&
> @@ -879,9 +876,9 @@ static void intel_prepare_hdmi_ddi_buffers(struct intel_encoder *encoder,
>  
>  	/* Entry 9 is for HDMI: */
>  	I915_WRITE(DDI_BUF_TRANS_LO(port, 9),
> -		   ddi_translations_hdmi[hdmi_level].trans1 | iboost_bit);
> +		   ddi_translations[level].trans1 | iboost_bit);
>  	I915_WRITE(DDI_BUF_TRANS_HI(port, 9),
> -		   ddi_translations_hdmi[hdmi_level].trans2);
> +		   ddi_translations[level].trans2);
>  }
>  
>  static void intel_wait_ddi_buf_idle(struct drm_i915_private *dev_priv,
> @@ -2094,7 +2091,7 @@ u32 bxt_signal_levels(struct intel_dp *intel_dp)
>  	struct intel_digital_port *dport = dp_to_dig_port(intel_dp);
>  	struct drm_i915_private *dev_priv = to_i915(dport->base.base.dev);
>  	struct intel_encoder *encoder = &dport->base;
> -	u32 level = intel_ddi_dp_level(intel_dp);
> +	int level = intel_ddi_dp_level(intel_dp);
>  
>  	if (IS_CANNONLAKE(dev_priv))
>  		cnl_ddi_vswing_sequence(encoder, level, encoder->type);
> @@ -2109,7 +2106,7 @@ uint32_t ddi_signal_levels(struct intel_dp *intel_dp)
>  	struct intel_digital_port *dport = dp_to_dig_port(intel_dp);
>  	struct drm_i915_private *dev_priv = to_i915(dport->base.base.dev);
>  	struct intel_encoder *encoder = &dport->base;
> -	uint32_t level = intel_ddi_dp_level(intel_dp);
> +	int level = intel_ddi_dp_level(intel_dp);
>  
>  	if (IS_GEN9_BC(dev_priv))
>  		skl_ddi_set_iboost(encoder, level, encoder->type);
> @@ -2182,7 +2179,7 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
>  	enum port port = intel_ddi_get_encoder_port(encoder);
>  	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
>  	bool is_mst = intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST);
> -	uint32_t level = intel_ddi_dp_level(intel_dp);
> +	int level = intel_ddi_dp_level(intel_dp);
>  
>  	WARN_ON(is_mst && (port == PORT_A || port == PORT_E));
>  
> -- 
> 2.13.6
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 02/10] drm/i915: Extract intel_ddi_get_buf_trans_hdmi()

[James Ausmus]

On Wed, Oct 18, 2017 at 08:09:24PM +0300, Ville Syrjälä wrote:
> On Mon, Oct 16, 2017 at 04:41:49PM -0700, Ausmus, James wrote:
> > On Mon, Oct 16, 2017 at 7:56 AM, Ville Syrjala
> > <ville.syrjala@linux.intel.com> wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >
> > > Introduce intel_ddi_get_buf_trans_hdmi() and start using it where we
> > > can.
> > >
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_ddi.c | 50 ++++++++++++++++++++++------------------
> > >  1 file changed, 28 insertions(+), 22 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > > index 55937abda61f..e6c884a6d408 100644
> > > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > > @@ -641,6 +641,24 @@ intel_ddi_get_buf_trans_fdi(struct drm_i915_private *dev_priv,
> > >         return NULL;
> > >  }
> > >
> > > +static const struct ddi_buf_trans *
> > > +intel_ddi_get_buf_trans_hdmi(struct drm_i915_private *dev_priv,
> > > +                            int *n_entries)
> > > +{
> > > +       if (IS_GEN9_BC(dev_priv)) {
> > > +               return skl_get_buf_trans_hdmi(dev_priv, n_entries);
> > > +       } else if (IS_BROADWELL(dev_priv)) {
> > > +               *n_entries = ARRAY_SIZE(bdw_ddi_translations_hdmi);
> > > +               return bdw_ddi_translations_hdmi;
> > > +       } else if (IS_HASWELL(dev_priv)) {
> > > +               *n_entries = ARRAY_SIZE(hsw_ddi_translations_hdmi);
> > > +               return hsw_ddi_translations_hdmi;
> > > +       }
> > > +
> > > +       *n_entries = 0;
> > > +       return NULL;
> > > +}
> > > +
> > >  static const struct cnl_ddi_buf_trans *
> > >  cnl_get_buf_trans_hdmi(struct drm_i915_private *dev_priv, int *n_entries)
> > >  {
> > > @@ -723,18 +741,17 @@ static int intel_ddi_hdmi_level(struct drm_i915_private *dev_priv, enum port por
> > >                 cnl_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> > >                 hdmi_default_entry = n_hdmi_entries - 1;
> > 
> > Why are you skipping the CNL case? If you extract it in to
> > intel_ddi_get_buf_trans_hdmi as well, you could just
> > unconditionally call intel_ddi_get_buf_trans_hdmi, and just set
> > hdmi_default_entry in the platform-specific section.
> 
> The problem is that CNL (and BXT) use a different structure for their
> buf trans values. So far I've not bothered to check if we could change
> them to use the HSW+ structure. I guess the other options would be to
> slap a union into the structure, or make these functions return
> void *, but neither option seems very appealing since we'd lose some
> type safety.

Ah yes - hadn't noticed the return types, as they're not used here -
thanks for explaining.

Reviewed-by: James Ausmus <james.ausmus@intel.com>

> 
> In any case that'd be material for followup work. Maybe someone else
> could take a look at it? ;)
> 
> > 
> > >         } else if (IS_GEN9_BC(dev_priv)) {
> > > -               skl_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> > > +               intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> > >                 hdmi_default_entry = 8;
> > >         } else if (IS_BROADWELL(dev_priv)) {
> > > -               n_hdmi_entries = ARRAY_SIZE(bdw_ddi_translations_hdmi);
> > > +               intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> > >                 hdmi_default_entry = 7;
> > >         } else if (IS_HASWELL(dev_priv)) {
> > > -               n_hdmi_entries = ARRAY_SIZE(hsw_ddi_translations_hdmi);
> > > +               intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> > >                 hdmi_default_entry = 6;
> > >         } else {
> > >                 WARN(1, "ddi translation table missing\n");
> > > -               n_hdmi_entries = ARRAY_SIZE(bdw_ddi_translations_hdmi);
> > > -               hdmi_default_entry = 7;
> > > +               return 0;
> > >         }
> > >
> > >         /* Choose a good default if VBT is badly populated */
> > > @@ -810,23 +827,12 @@ static void intel_prepare_hdmi_ddi_buffers(struct intel_encoder *encoder)
> > >
> > >         hdmi_level = intel_ddi_hdmi_level(dev_priv, port);
> > >
> > > -       if (IS_GEN9_BC(dev_priv)) {
> > > -               ddi_translations_hdmi = skl_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> > > +       ddi_translations_hdmi = intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> > >
> > > -               /* If we're boosting the current, set bit 31 of trans1 */
> > > -               if (dev_priv->vbt.ddi_port_info[port].hdmi_boost_level)
> > > -                       iboost_bit = DDI_BUF_BALANCE_LEG_ENABLE;
> > > -       } else if (IS_BROADWELL(dev_priv)) {
> > > -               ddi_translations_hdmi = bdw_ddi_translations_hdmi;
> > > -               n_hdmi_entries = ARRAY_SIZE(bdw_ddi_translations_hdmi);
> > > -       } else if (IS_HASWELL(dev_priv)) {
> > > -               ddi_translations_hdmi = hsw_ddi_translations_hdmi;
> > > -               n_hdmi_entries = ARRAY_SIZE(hsw_ddi_translations_hdmi);
> > > -       } else {
> > > -               WARN(1, "ddi translation table missing\n");
> > > -               ddi_translations_hdmi = bdw_ddi_translations_hdmi;
> > > -               n_hdmi_entries = ARRAY_SIZE(bdw_ddi_translations_hdmi);
> > > -       }
> > > +       /* If we're boosting the current, set bit 31 of trans1 */
> > > +       if (IS_GEN9_BC(dev_priv) &&
> > > +           dev_priv->vbt.ddi_port_info[port].hdmi_boost_level)
> > > +               iboost_bit = DDI_BUF_BALANCE_LEG_ENABLE;
> > >
> > >         /* Entry 9 is for HDMI: */
> > >         I915_WRITE(DDI_BUF_TRANS_LO(port, 9),
> > > @@ -1820,7 +1826,7 @@ static void skl_ddi_set_iboost(struct intel_encoder *encoder, u32 level)
> > >                 if (hdmi_iboost) {
> > >                         iboost = hdmi_iboost;
> > >                 } else {
> > > -                       ddi_translations = skl_get_buf_trans_hdmi(dev_priv, &n_entries);
> > > +                       ddi_translations = intel_ddi_get_buf_trans_hdmi(dev_priv, &n_entries);
> > >                         iboost = ddi_translations[level].i_boost;
> > >                 }
> > >         } else {
> > > --
> > > 2.13.6
> > >
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > 
> > 
> > -- 
> > 
> > 
> > James Ausmus
> 
> -- 
> Ville Syrjälä
> Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 09/10] drm/i915: Unify error handling for missing DDI buf trans tables

[Ville Syrjälä]

On Wed, Oct 18, 2017 at 10:41:44AM -0700, James Ausmus wrote:
> On Mon, Oct 16, 2017 at 05:57:04PM +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Handle missing buf trans tables, or out of bounds buf trans levels
> > the same way everywhere. These should never be hit under normal
> > conditions, but let's play it safe for now.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_ddi.c | 25 ++++++++++++++++++++-----
> >  1 file changed, 20 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > index f0a709b66e00..5f1c546e5e61 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -801,6 +801,9 @@ static int intel_ddi_hdmi_level(struct drm_i915_private *dev_priv, enum port por
> >  	    hdmi_level >= n_hdmi_entries)
> >  		hdmi_level = hdmi_default_entry;
> >  
> > +	if (WARN_ON_ONCE(hdmi_level >= n_hdmi_entries))
> > +		hdmi_level = n_hdmi_entries - 1;
> 
> intel_ddi_get_buf_trans_* can set n_entries to 0 on failure,
> which would cause this to return a level of -1 - maybe those functions
> need to return 1 on failure instead, similar to the cnl_get_buf_trans_*
> functions?

Hmm. I guess we could just do

if (WARN_ON(hdmi_level == 0))
	return;

before the other check. That would be equivalent to the
!ddi_translations checks done in the other places.

> 
> > +
> >  	return hdmi_level;
> >  }
> >  
> > @@ -864,6 +867,11 @@ static void intel_prepare_hdmi_ddi_buffers(struct intel_encoder *encoder,
> >  
> >  	ddi_translations_hdmi = intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> >  
> > +	if (WARN_ON_ONCE(!ddi_translations_hdmi))
> > +		return;
> > +	if (WARN_ON_ONCE(hdmi_level >= n_hdmi_entries))
> > +		hdmi_level = n_hdmi_entries - 1;
> > +
> >  	/* If we're boosting the current, set bit 31 of trans1 */
> >  	if (IS_GEN9_BC(dev_priv) &&
> >  	    dev_priv->vbt.ddi_port_info[port].hdmi_boost_level)
> > @@ -1847,6 +1855,11 @@ static void skl_ddi_set_iboost(struct intel_encoder *encoder,
> >  		else
> >  			ddi_translations = intel_ddi_get_buf_trans_dp(dev_priv, port, &n_entries);
> >  
> > +		if (WARN_ON_ONCE(!ddi_translations))
> > +			return;
> > +		if (WARN_ON_ONCE(level >= n_entries))
> > +			level = n_entries - 1;
> 
> 
> > +
> >  		iboost = ddi_translations[level].i_boost;
> >  	}
> >  
> > @@ -1877,6 +1890,11 @@ static void bxt_ddi_vswing_sequence(struct intel_encoder *encoder,
> >  	else
> >  		ddi_translations = bxt_get_buf_trans_dp(dev_priv, &n_entries);
> >  
> > +	if (WARN_ON_ONCE(!ddi_translations))
> > +		return;
> > +	if (WARN_ON_ONCE(level >= n_entries))
> > +		level = n_entries - 1;
> > +
> >  	bxt_ddi_phy_set_signal_level(dev_priv, port,
> >  				     ddi_translations[level].margin,
> >  				     ddi_translations[level].scale,
> > @@ -1932,13 +1950,10 @@ static void cnl_ddi_vswing_program(struct intel_encoder *encoder,
> >  	else
> >  		ddi_translations = cnl_get_buf_trans_dp(dev_priv, &n_entries);
> >  
> > -	if (WARN_ON(ddi_translations == NULL))
> > +	if (WARN_ON_ONCE(!ddi_translations))
> >  		return;
> > -
> > -	if (level >= n_entries) {
> > -		DRM_DEBUG_KMS("DDI translation not found for level %d. Using %d instead.", level, n_entries - 1);
> > +	if (WARN_ON_ONCE(level >= n_entries))
> >  		level = n_entries - 1;
> > -	}
> >  
> >  	/* Set PORT_TX_DW5 Scaling Mode Sel to 010b. */
> >  	val = I915_READ(CNL_PORT_TX_DW5_LN0(port));
> > -- 
> > 2.13.6
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 10/10] drm/i915: Drop the redundant hdmi prefix/suffix from a lot of variables

[Ville Syrjälä]

On Wed, Oct 18, 2017 at 10:43:54AM -0700, James Ausmus wrote:
> On Mon, Oct 16, 2017 at 05:57:05PM +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > A bunch of functions are now exclusively used for HDMI, so naming the
> > variables with hdmi prefix/suffix is redundant. Also use int rather
> > than u32 for the translation level consistently.
> 
> Is there a case for level to be negative, ever? If not, keeping it u32
> could help catch cases where we accidently try to set level to -1.

Is UINT_MAX any better than -1?

> 
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_ddi.c | 61 +++++++++++++++++++---------------------
> >  1 file changed, 29 insertions(+), 32 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > index 5f1c546e5e61..cd550aab332b 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -770,41 +770,38 @@ cnl_get_buf_trans_edp(struct drm_i915_private *dev_priv, int *n_entries)
> >  
> >  static int intel_ddi_hdmi_level(struct drm_i915_private *dev_priv, enum port port)
> >  {
> > -	int n_hdmi_entries;
> > -	int hdmi_level;
> > -	int hdmi_default_entry;
> > +	int n_entries, level, default_entry;
> >  
> > -	hdmi_level = dev_priv->vbt.ddi_port_info[port].hdmi_level_shift;
> > +	level = dev_priv->vbt.ddi_port_info[port].hdmi_level_shift;
> >  
> >  	if (IS_CANNONLAKE(dev_priv)) {
> > -		cnl_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> > -		hdmi_default_entry = n_hdmi_entries - 1;
> > +		cnl_get_buf_trans_hdmi(dev_priv, &n_entries);
> > +		default_entry = n_entries - 1;
> >  	} else if (IS_GEN9_LP(dev_priv)) {
> > -		bxt_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> > -		hdmi_default_entry = n_hdmi_entries - 1;
> > +		bxt_get_buf_trans_hdmi(dev_priv, &n_entries);
> > +		default_entry = n_entries - 1;
> >  	} else if (IS_GEN9_BC(dev_priv)) {
> > -		intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> > -		hdmi_default_entry = 8;
> > +		intel_ddi_get_buf_trans_hdmi(dev_priv, &n_entries);
> > +		default_entry = 8;
> >  	} else if (IS_BROADWELL(dev_priv)) {
> > -		intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> > -		hdmi_default_entry = 7;
> > +		intel_ddi_get_buf_trans_hdmi(dev_priv, &n_entries);
> > +		default_entry = 7;
> >  	} else if (IS_HASWELL(dev_priv)) {
> > -		intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> > -		hdmi_default_entry = 6;
> > +		intel_ddi_get_buf_trans_hdmi(dev_priv, &n_entries);
> > +		default_entry = 6;
> >  	} else {
> >  		WARN(1, "ddi translation table missing\n");
> >  		return 0;
> >  	}
> >  
> >  	/* Choose a good default if VBT is badly populated */
> > -	if (hdmi_level == HDMI_LEVEL_SHIFT_UNKNOWN ||
> > -	    hdmi_level >= n_hdmi_entries)
> > -		hdmi_level = hdmi_default_entry;
> > +	if (level == HDMI_LEVEL_SHIFT_UNKNOWN || level >= n_entries)
> > +		level = default_entry;
> >  
> > -	if (WARN_ON_ONCE(hdmi_level >= n_hdmi_entries))
> > -		hdmi_level = n_hdmi_entries - 1;
> > +	if (WARN_ON_ONCE(level >= n_entries))
> > +		level = n_entries - 1;
> >  
> > -	return hdmi_level;
> > +	return level;
> >  }
> >  
> >  /*
> > @@ -857,20 +854,20 @@ static void intel_prepare_dp_ddi_buffers(struct intel_encoder *encoder)
> >   * HDMI/DVI use cases.
> >   */
> >  static void intel_prepare_hdmi_ddi_buffers(struct intel_encoder *encoder,
> > -					   int hdmi_level)
> > +					   int level)
> >  {
> >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> >  	u32 iboost_bit = 0;
> > -	int n_hdmi_entries;
> > +	int n_entries;
> >  	enum port port = intel_ddi_get_encoder_port(encoder);
> > -	const struct ddi_buf_trans *ddi_translations_hdmi;
> > +	const struct ddi_buf_trans *ddi_translations;
> >  
> > -	ddi_translations_hdmi = intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> > +	ddi_translations = intel_ddi_get_buf_trans_hdmi(dev_priv, &n_entries);
> >  
> > -	if (WARN_ON_ONCE(!ddi_translations_hdmi))
> > +	if (WARN_ON_ONCE(!ddi_translations))
> >  		return;
> > -	if (WARN_ON_ONCE(hdmi_level >= n_hdmi_entries))
> > -		hdmi_level = n_hdmi_entries - 1;
> > +	if (WARN_ON_ONCE(level >= n_entries))
> > +		level = n_entries - 1;
> >  
> >  	/* If we're boosting the current, set bit 31 of trans1 */
> >  	if (IS_GEN9_BC(dev_priv) &&
> > @@ -879,9 +876,9 @@ static void intel_prepare_hdmi_ddi_buffers(struct intel_encoder *encoder,
> >  
> >  	/* Entry 9 is for HDMI: */
> >  	I915_WRITE(DDI_BUF_TRANS_LO(port, 9),
> > -		   ddi_translations_hdmi[hdmi_level].trans1 | iboost_bit);
> > +		   ddi_translations[level].trans1 | iboost_bit);
> >  	I915_WRITE(DDI_BUF_TRANS_HI(port, 9),
> > -		   ddi_translations_hdmi[hdmi_level].trans2);
> > +		   ddi_translations[level].trans2);
> >  }
> >  
> >  static void intel_wait_ddi_buf_idle(struct drm_i915_private *dev_priv,
> > @@ -2094,7 +2091,7 @@ u32 bxt_signal_levels(struct intel_dp *intel_dp)
> >  	struct intel_digital_port *dport = dp_to_dig_port(intel_dp);
> >  	struct drm_i915_private *dev_priv = to_i915(dport->base.base.dev);
> >  	struct intel_encoder *encoder = &dport->base;
> > -	u32 level = intel_ddi_dp_level(intel_dp);
> > +	int level = intel_ddi_dp_level(intel_dp);
> >  
> >  	if (IS_CANNONLAKE(dev_priv))
> >  		cnl_ddi_vswing_sequence(encoder, level, encoder->type);
> > @@ -2109,7 +2106,7 @@ uint32_t ddi_signal_levels(struct intel_dp *intel_dp)
> >  	struct intel_digital_port *dport = dp_to_dig_port(intel_dp);
> >  	struct drm_i915_private *dev_priv = to_i915(dport->base.base.dev);
> >  	struct intel_encoder *encoder = &dport->base;
> > -	uint32_t level = intel_ddi_dp_level(intel_dp);
> > +	int level = intel_ddi_dp_level(intel_dp);
> >  
> >  	if (IS_GEN9_BC(dev_priv))
> >  		skl_ddi_set_iboost(encoder, level, encoder->type);
> > @@ -2182,7 +2179,7 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
> >  	enum port port = intel_ddi_get_encoder_port(encoder);
> >  	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
> >  	bool is_mst = intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST);
> > -	uint32_t level = intel_ddi_dp_level(intel_dp);
> > +	int level = intel_ddi_dp_level(intel_dp);
> >  
> >  	WARN_ON(is_mst && (port == PORT_A || port == PORT_E));
> >  
> > -- 
> > 2.13.6
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 09/10] drm/i915: Unify error handling for missing DDI buf trans tables

[Ville Syrjälä]

On Wed, Oct 18, 2017 at 08:52:40PM +0300, Ville Syrjälä wrote:
> On Wed, Oct 18, 2017 at 10:41:44AM -0700, James Ausmus wrote:
> > On Mon, Oct 16, 2017 at 05:57:04PM +0300, Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Handle missing buf trans tables, or out of bounds buf trans levels
> > > the same way everywhere. These should never be hit under normal
> > > conditions, but let's play it safe for now.
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_ddi.c | 25 ++++++++++++++++++++-----
> > >  1 file changed, 20 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > > index f0a709b66e00..5f1c546e5e61 100644
> > > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > > @@ -801,6 +801,9 @@ static int intel_ddi_hdmi_level(struct drm_i915_private *dev_priv, enum port por
> > >  	    hdmi_level >= n_hdmi_entries)
> > >  		hdmi_level = hdmi_default_entry;
> > >  
> > > +	if (WARN_ON_ONCE(hdmi_level >= n_hdmi_entries))
> > > +		hdmi_level = n_hdmi_entries - 1;
> > 
> > intel_ddi_get_buf_trans_* can set n_entries to 0 on failure,
> > which would cause this to return a level of -1 - maybe those functions
> > need to return 1 on failure instead, similar to the cnl_get_buf_trans_*
> > functions?
> 
> Hmm. I guess we could just do
> 
> if (WARN_ON(hdmi_level == 0))
> 	return;
> 
> before the other check. That would be equivalent to the
> !ddi_translations checks done in the other places.

Hmm. Actually now that I read things again, the -1 shouldn't actually
cause us any problems since all uses of it would do a second
!ddi_translations check before indexing anything with the -1.

-1 and 0 are both wrong indexes when dealing with a zero sized array,
but I suppose in theory 0 is a bit better since it would trip up on
firther level>=n_entries checks. So I guess I could try to avoid
the negative value.

Another way to do that would be just
- return hdmi_level;
+ return max(hdmi_level, 0);

Or your other idea about keeping things unsigned would also take care
of it. Except having a wild UINT_MAX roaming around seems potentially
more dangerous to me.

> > 
> > > +
> > >  	return hdmi_level;
> > >  }
> > >  
> > > @@ -864,6 +867,11 @@ static void intel_prepare_hdmi_ddi_buffers(struct intel_encoder *encoder,
> > >  
> > >  	ddi_translations_hdmi = intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> > >  
> > > +	if (WARN_ON_ONCE(!ddi_translations_hdmi))
> > > +		return;
> > > +	if (WARN_ON_ONCE(hdmi_level >= n_hdmi_entries))
> > > +		hdmi_level = n_hdmi_entries - 1;
> > > +
> > >  	/* If we're boosting the current, set bit 31 of trans1 */
> > >  	if (IS_GEN9_BC(dev_priv) &&
> > >  	    dev_priv->vbt.ddi_port_info[port].hdmi_boost_level)
> > > @@ -1847,6 +1855,11 @@ static void skl_ddi_set_iboost(struct intel_encoder *encoder,
> > >  		else
> > >  			ddi_translations = intel_ddi_get_buf_trans_dp(dev_priv, port, &n_entries);
> > >  
> > > +		if (WARN_ON_ONCE(!ddi_translations))
> > > +			return;
> > > +		if (WARN_ON_ONCE(level >= n_entries))
> > > +			level = n_entries - 1;
> > 
> > 
> > > +
> > >  		iboost = ddi_translations[level].i_boost;
> > >  	}
> > >  
> > > @@ -1877,6 +1890,11 @@ static void bxt_ddi_vswing_sequence(struct intel_encoder *encoder,
> > >  	else
> > >  		ddi_translations = bxt_get_buf_trans_dp(dev_priv, &n_entries);
> > >  
> > > +	if (WARN_ON_ONCE(!ddi_translations))
> > > +		return;
> > > +	if (WARN_ON_ONCE(level >= n_entries))
> > > +		level = n_entries - 1;
> > > +
> > >  	bxt_ddi_phy_set_signal_level(dev_priv, port,
> > >  				     ddi_translations[level].margin,
> > >  				     ddi_translations[level].scale,
> > > @@ -1932,13 +1950,10 @@ static void cnl_ddi_vswing_program(struct intel_encoder *encoder,
> > >  	else
> > >  		ddi_translations = cnl_get_buf_trans_dp(dev_priv, &n_entries);
> > >  
> > > -	if (WARN_ON(ddi_translations == NULL))
> > > +	if (WARN_ON_ONCE(!ddi_translations))
> > >  		return;
> > > -
> > > -	if (level >= n_entries) {
> > > -		DRM_DEBUG_KMS("DDI translation not found for level %d. Using %d instead.", level, n_entries - 1);
> > > +	if (WARN_ON_ONCE(level >= n_entries))
> > >  		level = n_entries - 1;
> > > -	}
> > >  
> > >  	/* Set PORT_TX_DW5 Scaling Mode Sel to 010b. */
> > >  	val = I915_READ(CNL_PORT_TX_DW5_LN0(port));
> > > -- 
> > > 2.13.6
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 09/10] drm/i915: Unify error handling for missing DDI buf trans tables

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Handle missing buf trans tables, or out of bounds buf trans levels
the same way everywhere. These should never be hit under normal
conditions, but let's play it safe for now.

v2: Avoid the hdmi_level=-1 case (James)

Cc: James Ausmus <james.ausmus@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 27 ++++++++++++++++++++++-----
 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 5ee0e33e2c00..cd7a0d979a10 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -801,6 +801,11 @@ static int intel_ddi_hdmi_level(struct drm_i915_private *dev_priv, enum port por
 	    hdmi_level >= n_hdmi_entries)
 		hdmi_level = hdmi_default_entry;
 
+	if (WARN_ON_ONCE(n_hdmi_entries == 0))
+		return 0;
+	if (WARN_ON_ONCE(hdmi_level >= n_hdmi_entries))
+		hdmi_level = n_hdmi_entries - 1;
+
 	return hdmi_level;
 }
 
@@ -864,6 +869,11 @@ static void intel_prepare_hdmi_ddi_buffers(struct intel_encoder *encoder,
 
 	ddi_translations_hdmi = intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
 
+	if (WARN_ON_ONCE(!ddi_translations_hdmi))
+		return;
+	if (WARN_ON_ONCE(hdmi_level >= n_hdmi_entries))
+		hdmi_level = n_hdmi_entries - 1;
+
 	/* If we're boosting the current, set bit 31 of trans1 */
 	if (IS_GEN9_BC(dev_priv) &&
 	    dev_priv->vbt.ddi_port_info[port].hdmi_boost_level)
@@ -1847,6 +1857,11 @@ static void skl_ddi_set_iboost(struct intel_encoder *encoder,
 		else
 			ddi_translations = intel_ddi_get_buf_trans_dp(dev_priv, port, &n_entries);
 
+		if (WARN_ON_ONCE(!ddi_translations))
+			return;
+		if (WARN_ON_ONCE(level >= n_entries))
+			level = n_entries - 1;
+
 		iboost = ddi_translations[level].i_boost;
 	}
 
@@ -1877,6 +1892,11 @@ static void bxt_ddi_vswing_sequence(struct intel_encoder *encoder,
 	else
 		ddi_translations = bxt_get_buf_trans_dp(dev_priv, &n_entries);
 
+	if (WARN_ON_ONCE(!ddi_translations))
+		return;
+	if (WARN_ON_ONCE(level >= n_entries))
+		level = n_entries - 1;
+
 	bxt_ddi_phy_set_signal_level(dev_priv, port,
 				     ddi_translations[level].margin,
 				     ddi_translations[level].scale,
@@ -1932,13 +1952,10 @@ static void cnl_ddi_vswing_program(struct intel_encoder *encoder,
 	else
 		ddi_translations = cnl_get_buf_trans_dp(dev_priv, &n_entries);
 
-	if (WARN_ON(ddi_translations == NULL))
+	if (WARN_ON_ONCE(!ddi_translations))
 		return;
-
-	if (level >= n_entries) {
-		DRM_DEBUG_KMS("DDI translation not found for level %d. Using %d instead.", level, n_entries - 1);
+	if (WARN_ON_ONCE(level >= n_entries))
 		level = n_entries - 1;
-	}
 
 	/* Set PORT_TX_DW5 Scaling Mode Sel to 010b. */
 	val = I915_READ(CNL_PORT_TX_DW5_LN0(port));
-- 
2.13.6

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

[PATCH v2 10/10] drm/i915: Drop the redundant hdmi prefix/suffix from a lot of variables

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

A bunch of functions are now exclusively used for HDMI, so naming the
variables with hdmi prefix/suffix is redundant. Also use int rather
than u32 for the translation level consistently.

v2: Rebase due to hdmi_level=-1 avoidance

Cc: James Ausmus <james.ausmus@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 63 +++++++++++++++++++---------------------
 1 file changed, 30 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index cd7a0d979a10..b8925bc82f30 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -770,43 +770,40 @@ cnl_get_buf_trans_edp(struct drm_i915_private *dev_priv, int *n_entries)
 
 static int intel_ddi_hdmi_level(struct drm_i915_private *dev_priv, enum port port)
 {
-	int n_hdmi_entries;
-	int hdmi_level;
-	int hdmi_default_entry;
+	int n_entries, level, default_entry;
 
-	hdmi_level = dev_priv->vbt.ddi_port_info[port].hdmi_level_shift;
+	level = dev_priv->vbt.ddi_port_info[port].hdmi_level_shift;
 
 	if (IS_CANNONLAKE(dev_priv)) {
-		cnl_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
-		hdmi_default_entry = n_hdmi_entries - 1;
+		cnl_get_buf_trans_hdmi(dev_priv, &n_entries);
+		default_entry = n_entries - 1;
 	} else if (IS_GEN9_LP(dev_priv)) {
-		bxt_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
-		hdmi_default_entry = n_hdmi_entries - 1;
+		bxt_get_buf_trans_hdmi(dev_priv, &n_entries);
+		default_entry = n_entries - 1;
 	} else if (IS_GEN9_BC(dev_priv)) {
-		intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
-		hdmi_default_entry = 8;
+		intel_ddi_get_buf_trans_hdmi(dev_priv, &n_entries);
+		default_entry = 8;
 	} else if (IS_BROADWELL(dev_priv)) {
-		intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
-		hdmi_default_entry = 7;
+		intel_ddi_get_buf_trans_hdmi(dev_priv, &n_entries);
+		default_entry = 7;
 	} else if (IS_HASWELL(dev_priv)) {
-		intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
-		hdmi_default_entry = 6;
+		intel_ddi_get_buf_trans_hdmi(dev_priv, &n_entries);
+		default_entry = 6;
 	} else {
 		WARN(1, "ddi translation table missing\n");
 		return 0;
 	}
 
 	/* Choose a good default if VBT is badly populated */
-	if (hdmi_level == HDMI_LEVEL_SHIFT_UNKNOWN ||
-	    hdmi_level >= n_hdmi_entries)
-		hdmi_level = hdmi_default_entry;
+	if (level == HDMI_LEVEL_SHIFT_UNKNOWN || level >= n_entries)
+		level = default_entry;
 
-	if (WARN_ON_ONCE(n_hdmi_entries == 0))
+	if (WARN_ON_ONCE(n_entries == 0))
 		return 0;
-	if (WARN_ON_ONCE(hdmi_level >= n_hdmi_entries))
-		hdmi_level = n_hdmi_entries - 1;
+	if (WARN_ON_ONCE(level >= n_entries))
+		level = n_entries - 1;
 
-	return hdmi_level;
+	return level;
 }
 
 /*
@@ -859,20 +856,20 @@ static void intel_prepare_dp_ddi_buffers(struct intel_encoder *encoder)
  * HDMI/DVI use cases.
  */
 static void intel_prepare_hdmi_ddi_buffers(struct intel_encoder *encoder,
-					   int hdmi_level)
+					   int level)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	u32 iboost_bit = 0;
-	int n_hdmi_entries;
+	int n_entries;
 	enum port port = intel_ddi_get_encoder_port(encoder);
-	const struct ddi_buf_trans *ddi_translations_hdmi;
+	const struct ddi_buf_trans *ddi_translations;
 
-	ddi_translations_hdmi = intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
+	ddi_translations = intel_ddi_get_buf_trans_hdmi(dev_priv, &n_entries);
 
-	if (WARN_ON_ONCE(!ddi_translations_hdmi))
+	if (WARN_ON_ONCE(!ddi_translations))
 		return;
-	if (WARN_ON_ONCE(hdmi_level >= n_hdmi_entries))
-		hdmi_level = n_hdmi_entries - 1;
+	if (WARN_ON_ONCE(level >= n_entries))
+		level = n_entries - 1;
 
 	/* If we're boosting the current, set bit 31 of trans1 */
 	if (IS_GEN9_BC(dev_priv) &&
@@ -881,9 +878,9 @@ static void intel_prepare_hdmi_ddi_buffers(struct intel_encoder *encoder,
 
 	/* Entry 9 is for HDMI: */
 	I915_WRITE(DDI_BUF_TRANS_LO(port, 9),
-		   ddi_translations_hdmi[hdmi_level].trans1 | iboost_bit);
+		   ddi_translations[level].trans1 | iboost_bit);
 	I915_WRITE(DDI_BUF_TRANS_HI(port, 9),
-		   ddi_translations_hdmi[hdmi_level].trans2);
+		   ddi_translations[level].trans2);
 }
 
 static void intel_wait_ddi_buf_idle(struct drm_i915_private *dev_priv,
@@ -2096,7 +2093,7 @@ u32 bxt_signal_levels(struct intel_dp *intel_dp)
 	struct intel_digital_port *dport = dp_to_dig_port(intel_dp);
 	struct drm_i915_private *dev_priv = to_i915(dport->base.base.dev);
 	struct intel_encoder *encoder = &dport->base;
-	u32 level = intel_ddi_dp_level(intel_dp);
+	int level = intel_ddi_dp_level(intel_dp);
 
 	if (IS_CANNONLAKE(dev_priv))
 		cnl_ddi_vswing_sequence(encoder, level, encoder->type);
@@ -2111,7 +2108,7 @@ uint32_t ddi_signal_levels(struct intel_dp *intel_dp)
 	struct intel_digital_port *dport = dp_to_dig_port(intel_dp);
 	struct drm_i915_private *dev_priv = to_i915(dport->base.base.dev);
 	struct intel_encoder *encoder = &dport->base;
-	uint32_t level = intel_ddi_dp_level(intel_dp);
+	int level = intel_ddi_dp_level(intel_dp);
 
 	if (IS_GEN9_BC(dev_priv))
 		skl_ddi_set_iboost(encoder, level, encoder->type);
@@ -2183,7 +2180,7 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
 	enum port port = intel_ddi_get_encoder_port(encoder);
 	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
 	bool is_mst = intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST);
-	uint32_t level = intel_ddi_dp_level(intel_dp);
+	int level = intel_ddi_dp_level(intel_dp);
 
 	WARN_ON(is_mst && (port == PORT_A || port == PORT_E));
 
-- 
2.13.6

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

Re: [PATCH 09/10] drm/i915: Unify error handling for missing DDI buf trans tables

[James Ausmus]

On Wed, Oct 18, 2017 at 09:15:09PM +0300, Ville Syrjälä wrote:
> On Wed, Oct 18, 2017 at 08:52:40PM +0300, Ville Syrjälä wrote:
> > On Wed, Oct 18, 2017 at 10:41:44AM -0700, James Ausmus wrote:
> > > On Mon, Oct 16, 2017 at 05:57:04PM +0300, Ville Syrjala wrote:
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > 
> > > > Handle missing buf trans tables, or out of bounds buf trans levels
> > > > the same way everywhere. These should never be hit under normal
> > > > conditions, but let's play it safe for now.
> > > > 
> > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_ddi.c | 25 ++++++++++++++++++++-----
> > > >  1 file changed, 20 insertions(+), 5 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > > > index f0a709b66e00..5f1c546e5e61 100644
> > > > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > > > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > > > @@ -801,6 +801,9 @@ static int intel_ddi_hdmi_level(struct drm_i915_private *dev_priv, enum port por
> > > >  	    hdmi_level >= n_hdmi_entries)
> > > >  		hdmi_level = hdmi_default_entry;
> > > >  
> > > > +	if (WARN_ON_ONCE(hdmi_level >= n_hdmi_entries))
> > > > +		hdmi_level = n_hdmi_entries - 1;
> > > 
> > > intel_ddi_get_buf_trans_* can set n_entries to 0 on failure,
> > > which would cause this to return a level of -1 - maybe those functions
> > > need to return 1 on failure instead, similar to the cnl_get_buf_trans_*
> > > functions?
> > 
> > Hmm. I guess we could just do
> > 
> > if (WARN_ON(hdmi_level == 0))
> > 	return;
> > 
> > before the other check. That would be equivalent to the
> > !ddi_translations checks done in the other places.
> 
> Hmm. Actually now that I read things again, the -1 shouldn't actually
> cause us any problems since all uses of it would do a second
> !ddi_translations check before indexing anything with the -1.

Yeah, that's true, but it doesn't feel right to have inconsistent error
values between cnl and the rest of the world, and ending up with an
index of -1 feels wrong as well - OTOH, the cnl functions claiming to
have 1 entry when erroring out also feels wrong, so maybe there's not a
completely clean solution without over-engineering a simple array &
index mechanism.

Hmm - maybe keep the switch to int, and on failure have all the paths
set n_entries to -1, which makes it an obvious error value, rather than
trying to guess error on a potential real or invalid value of 1 or 0.

> 
> -1 and 0 are both wrong indexes when dealing with a zero sized array,
> but I suppose in theory 0 is a bit better since it would trip up on
> firther level>=n_entries checks. So I guess I could try to avoid
> the negative value.
> 
> Another way to do that would be just
> - return hdmi_level;
> + return max(hdmi_level, 0);
> 
> Or your other idea about keeping things unsigned would also take care
> of it. Except having a wild UINT_MAX roaming around seems potentially
> more dangerous to me.
> 
> > > 
> > > > +
> > > >  	return hdmi_level;
> > > >  }
> > > >  
> > > > @@ -864,6 +867,11 @@ static void intel_prepare_hdmi_ddi_buffers(struct intel_encoder *encoder,
> > > >  
> > > >  	ddi_translations_hdmi = intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> > > >  
> > > > +	if (WARN_ON_ONCE(!ddi_translations_hdmi))
> > > > +		return;
> > > > +	if (WARN_ON_ONCE(hdmi_level >= n_hdmi_entries))
> > > > +		hdmi_level = n_hdmi_entries - 1;
> > > > +
> > > >  	/* If we're boosting the current, set bit 31 of trans1 */
> > > >  	if (IS_GEN9_BC(dev_priv) &&
> > > >  	    dev_priv->vbt.ddi_port_info[port].hdmi_boost_level)
> > > > @@ -1847,6 +1855,11 @@ static void skl_ddi_set_iboost(struct intel_encoder *encoder,
> > > >  		else
> > > >  			ddi_translations = intel_ddi_get_buf_trans_dp(dev_priv, port, &n_entries);
> > > >  
> > > > +		if (WARN_ON_ONCE(!ddi_translations))
> > > > +			return;
> > > > +		if (WARN_ON_ONCE(level >= n_entries))
> > > > +			level = n_entries - 1;
> > > 
> > > 
> > > > +
> > > >  		iboost = ddi_translations[level].i_boost;
> > > >  	}
> > > >  
> > > > @@ -1877,6 +1890,11 @@ static void bxt_ddi_vswing_sequence(struct intel_encoder *encoder,
> > > >  	else
> > > >  		ddi_translations = bxt_get_buf_trans_dp(dev_priv, &n_entries);
> > > >  
> > > > +	if (WARN_ON_ONCE(!ddi_translations))
> > > > +		return;
> > > > +	if (WARN_ON_ONCE(level >= n_entries))
> > > > +		level = n_entries - 1;
> > > > +
> > > >  	bxt_ddi_phy_set_signal_level(dev_priv, port,
> > > >  				     ddi_translations[level].margin,
> > > >  				     ddi_translations[level].scale,
> > > > @@ -1932,13 +1950,10 @@ static void cnl_ddi_vswing_program(struct intel_encoder *encoder,
> > > >  	else
> > > >  		ddi_translations = cnl_get_buf_trans_dp(dev_priv, &n_entries);
> > > >  
> > > > -	if (WARN_ON(ddi_translations == NULL))
> > > > +	if (WARN_ON_ONCE(!ddi_translations))
> > > >  		return;
> > > > -
> > > > -	if (level >= n_entries) {
> > > > -		DRM_DEBUG_KMS("DDI translation not found for level %d. Using %d instead.", level, n_entries - 1);
> > > > +	if (WARN_ON_ONCE(level >= n_entries))
> > > >  		level = n_entries - 1;
> > > > -	}
> > > >  
> > > >  	/* Set PORT_TX_DW5 Scaling Mode Sel to 010b. */
> > > >  	val = I915_READ(CNL_PORT_TX_DW5_LN0(port));
> > > > -- 
> > > > 2.13.6
> > > > 
> > > > _______________________________________________
> > > > Intel-gfx mailing list
> > > > Intel-gfx@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > -- 
> > Ville Syrjälä
> > Intel OTC
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for DDI buf trans cleanup (rev3)

[Patchwork]

== Series Details ==

Series: DDI buf trans cleanup (rev3)
URL   : https://patchwork.freedesktop.org/series/32049/
State : success

== Summary ==

Series 32049v3 DDI buf trans cleanup
https://patchwork.freedesktop.org/api/1.0/series/32049/revisions/3/mbox/

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:441s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:449s
fi-blb-e6850     total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:375s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:531s
fi-bwr-2160      total:289  pass:183  dwarn:0   dfail:0   fail:0   skip:106 time:262s
fi-bxt-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:497s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:493s
fi-byt-j1900     total:289  pass:253  dwarn:1   dfail:0   fail:0   skip:35  time:497s
fi-byt-n2820     total:289  pass:249  dwarn:1   dfail:0   fail:0   skip:39  time:485s
fi-elk-e7500     total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:414s
fi-gdg-551       total:289  pass:178  dwarn:1   dfail:0   fail:1   skip:109 time:249s
fi-glk-1         total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:579s
fi-hsw-4770      total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:440s
fi-hsw-4770r     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:422s
fi-ilk-650       total:289  pass:228  dwarn:0   dfail:0   fail:0   skip:61  time:436s
fi-ivb-3520m     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:478s
fi-ivb-3770      total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:455s
fi-kbl-7500u     total:289  pass:264  dwarn:1   dfail:0   fail:0   skip:24  time:488s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:571s
fi-kbl-7567u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:477s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:578s
fi-pnv-d510      total:289  pass:222  dwarn:1   dfail:0   fail:0   skip:66  time:542s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:448s
fi-skl-6700hq    total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:647s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:522s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:498s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:455s
fi-snb-2520m     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:558s
fi-snb-2600      total:289  pass:249  dwarn:0   dfail:0   fail:0   skip:40  time:417s

7edf6833df0ac2c6d24969d48074203bd305e623 drm-tip: 2017y-10m-18d-16h-29m-02s UTC integration manifest
6abd41ecee32 drm/i915: Drop the redundant hdmi prefix/suffix from a lot of variables
73b2ff655481 drm/i915: Unify error handling for missing DDI buf trans tables
6709042d1593 drm/i915: Centralize the SKL DDI A/E vs. B/C/D buf trans handling
02fdd35a0141 drm/i915: Kill off the BXT buf_trans default_index
4f9cfff5a70d drm/i915: Pass encoder type to cnl_ddi_vswing_sequence() explicitly
2a2ed4fe9119 drm/i915: Integrate BXT into intel_ddi_dp_voltage_max()
40f9697f9d99 drm/i915: Pass the level to intel_prepare_hdmi_ddi_buffers()
3c8082696b79 drm/i915: Pass the encoder type explicitly to skl_set_iboost()
5d730eb4e952 drm/i915: Extract intel_ddi_get_buf_trans_hdmi()
ba4d1dbbe627 drm/i915: Relocate intel_ddi_get_buf_trans_*() functions

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6094/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 10/10] drm/i915: Drop the redundant hdmi prefix/suffix from a lot of variables

[James Ausmus]

On Wed, Oct 18, 2017 at 08:54:12PM +0300, Ville Syrjälä wrote:
> On Wed, Oct 18, 2017 at 10:43:54AM -0700, James Ausmus wrote:
> > On Mon, Oct 16, 2017 at 05:57:05PM +0300, Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > A bunch of functions are now exclusively used for HDMI, so naming the
> > > variables with hdmi prefix/suffix is redundant. Also use int rather
> > > than u32 for the translation level consistently.
> > 
> > Is there a case for level to be negative, ever? If not, keeping it u32
> > could help catch cases where we accidently try to set level to -1.
> 
> Is UINT_MAX any better than -1?
> 

I was hoping gcc could be smart enough to throw a warning on an unsigned
underflow, but it doesn't appear to be.

> > 
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_ddi.c | 61 +++++++++++++++++++---------------------
> > >  1 file changed, 29 insertions(+), 32 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > > index 5f1c546e5e61..cd550aab332b 100644
> > > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > > @@ -770,41 +770,38 @@ cnl_get_buf_trans_edp(struct drm_i915_private *dev_priv, int *n_entries)
> > >  
> > >  static int intel_ddi_hdmi_level(struct drm_i915_private *dev_priv, enum port port)
> > >  {
> > > -	int n_hdmi_entries;
> > > -	int hdmi_level;
> > > -	int hdmi_default_entry;
> > > +	int n_entries, level, default_entry;
> > >  
> > > -	hdmi_level = dev_priv->vbt.ddi_port_info[port].hdmi_level_shift;
> > > +	level = dev_priv->vbt.ddi_port_info[port].hdmi_level_shift;
> > >  
> > >  	if (IS_CANNONLAKE(dev_priv)) {
> > > -		cnl_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> > > -		hdmi_default_entry = n_hdmi_entries - 1;
> > > +		cnl_get_buf_trans_hdmi(dev_priv, &n_entries);
> > > +		default_entry = n_entries - 1;
> > >  	} else if (IS_GEN9_LP(dev_priv)) {
> > > -		bxt_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> > > -		hdmi_default_entry = n_hdmi_entries - 1;
> > > +		bxt_get_buf_trans_hdmi(dev_priv, &n_entries);
> > > +		default_entry = n_entries - 1;
> > >  	} else if (IS_GEN9_BC(dev_priv)) {
> > > -		intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> > > -		hdmi_default_entry = 8;
> > > +		intel_ddi_get_buf_trans_hdmi(dev_priv, &n_entries);
> > > +		default_entry = 8;
> > >  	} else if (IS_BROADWELL(dev_priv)) {
> > > -		intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> > > -		hdmi_default_entry = 7;
> > > +		intel_ddi_get_buf_trans_hdmi(dev_priv, &n_entries);
> > > +		default_entry = 7;
> > >  	} else if (IS_HASWELL(dev_priv)) {
> > > -		intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> > > -		hdmi_default_entry = 6;
> > > +		intel_ddi_get_buf_trans_hdmi(dev_priv, &n_entries);
> > > +		default_entry = 6;
> > >  	} else {
> > >  		WARN(1, "ddi translation table missing\n");
> > >  		return 0;
> > >  	}
> > >  
> > >  	/* Choose a good default if VBT is badly populated */
> > > -	if (hdmi_level == HDMI_LEVEL_SHIFT_UNKNOWN ||
> > > -	    hdmi_level >= n_hdmi_entries)
> > > -		hdmi_level = hdmi_default_entry;
> > > +	if (level == HDMI_LEVEL_SHIFT_UNKNOWN || level >= n_entries)
> > > +		level = default_entry;
> > >  
> > > -	if (WARN_ON_ONCE(hdmi_level >= n_hdmi_entries))
> > > -		hdmi_level = n_hdmi_entries - 1;
> > > +	if (WARN_ON_ONCE(level >= n_entries))
> > > +		level = n_entries - 1;
> > >  
> > > -	return hdmi_level;
> > > +	return level;
> > >  }
> > >  
> > >  /*
> > > @@ -857,20 +854,20 @@ static void intel_prepare_dp_ddi_buffers(struct intel_encoder *encoder)
> > >   * HDMI/DVI use cases.
> > >   */
> > >  static void intel_prepare_hdmi_ddi_buffers(struct intel_encoder *encoder,
> > > -					   int hdmi_level)
> > > +					   int level)
> > >  {
> > >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > >  	u32 iboost_bit = 0;
> > > -	int n_hdmi_entries;
> > > +	int n_entries;
> > >  	enum port port = intel_ddi_get_encoder_port(encoder);
> > > -	const struct ddi_buf_trans *ddi_translations_hdmi;
> > > +	const struct ddi_buf_trans *ddi_translations;
> > >  
> > > -	ddi_translations_hdmi = intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> > > +	ddi_translations = intel_ddi_get_buf_trans_hdmi(dev_priv, &n_entries);
> > >  
> > > -	if (WARN_ON_ONCE(!ddi_translations_hdmi))
> > > +	if (WARN_ON_ONCE(!ddi_translations))
> > >  		return;
> > > -	if (WARN_ON_ONCE(hdmi_level >= n_hdmi_entries))
> > > -		hdmi_level = n_hdmi_entries - 1;
> > > +	if (WARN_ON_ONCE(level >= n_entries))
> > > +		level = n_entries - 1;
> > >  
> > >  	/* If we're boosting the current, set bit 31 of trans1 */
> > >  	if (IS_GEN9_BC(dev_priv) &&
> > > @@ -879,9 +876,9 @@ static void intel_prepare_hdmi_ddi_buffers(struct intel_encoder *encoder,
> > >  
> > >  	/* Entry 9 is for HDMI: */
> > >  	I915_WRITE(DDI_BUF_TRANS_LO(port, 9),
> > > -		   ddi_translations_hdmi[hdmi_level].trans1 | iboost_bit);
> > > +		   ddi_translations[level].trans1 | iboost_bit);
> > >  	I915_WRITE(DDI_BUF_TRANS_HI(port, 9),
> > > -		   ddi_translations_hdmi[hdmi_level].trans2);
> > > +		   ddi_translations[level].trans2);
> > >  }
> > >  
> > >  static void intel_wait_ddi_buf_idle(struct drm_i915_private *dev_priv,
> > > @@ -2094,7 +2091,7 @@ u32 bxt_signal_levels(struct intel_dp *intel_dp)
> > >  	struct intel_digital_port *dport = dp_to_dig_port(intel_dp);
> > >  	struct drm_i915_private *dev_priv = to_i915(dport->base.base.dev);
> > >  	struct intel_encoder *encoder = &dport->base;
> > > -	u32 level = intel_ddi_dp_level(intel_dp);
> > > +	int level = intel_ddi_dp_level(intel_dp);
> > >  
> > >  	if (IS_CANNONLAKE(dev_priv))
> > >  		cnl_ddi_vswing_sequence(encoder, level, encoder->type);
> > > @@ -2109,7 +2106,7 @@ uint32_t ddi_signal_levels(struct intel_dp *intel_dp)
> > >  	struct intel_digital_port *dport = dp_to_dig_port(intel_dp);
> > >  	struct drm_i915_private *dev_priv = to_i915(dport->base.base.dev);
> > >  	struct intel_encoder *encoder = &dport->base;
> > > -	uint32_t level = intel_ddi_dp_level(intel_dp);
> > > +	int level = intel_ddi_dp_level(intel_dp);
> > >  
> > >  	if (IS_GEN9_BC(dev_priv))
> > >  		skl_ddi_set_iboost(encoder, level, encoder->type);
> > > @@ -2182,7 +2179,7 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
> > >  	enum port port = intel_ddi_get_encoder_port(encoder);
> > >  	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
> > >  	bool is_mst = intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST);
> > > -	uint32_t level = intel_ddi_dp_level(intel_dp);
> > > +	int level = intel_ddi_dp_level(intel_dp);
> > >  
> > >  	WARN_ON(is_mst && (port == PORT_A || port == PORT_E));
> > >  
> > > -- 
> > > 2.13.6
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 10/10] drm/i915: Drop the redundant hdmi prefix/suffix from a lot of variables

[James Ausmus]

On Wed, Oct 18, 2017 at 09:19:58PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> A bunch of functions are now exclusively used for HDMI, so naming the
> variables with hdmi prefix/suffix is redundant. Also use int rather
> than u32 for the translation level consistently.
> 
> v2: Rebase due to hdmi_level=-1 avoidance
> 
> Cc: James Ausmus <james.ausmus@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: James Ausmus <james.ausmus@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 63 +++++++++++++++++++---------------------
>  1 file changed, 30 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index cd7a0d979a10..b8925bc82f30 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -770,43 +770,40 @@ cnl_get_buf_trans_edp(struct drm_i915_private *dev_priv, int *n_entries)
>  
>  static int intel_ddi_hdmi_level(struct drm_i915_private *dev_priv, enum port port)
>  {
> -	int n_hdmi_entries;
> -	int hdmi_level;
> -	int hdmi_default_entry;
> +	int n_entries, level, default_entry;
>  
> -	hdmi_level = dev_priv->vbt.ddi_port_info[port].hdmi_level_shift;
> +	level = dev_priv->vbt.ddi_port_info[port].hdmi_level_shift;
>  
>  	if (IS_CANNONLAKE(dev_priv)) {
> -		cnl_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> -		hdmi_default_entry = n_hdmi_entries - 1;
> +		cnl_get_buf_trans_hdmi(dev_priv, &n_entries);
> +		default_entry = n_entries - 1;
>  	} else if (IS_GEN9_LP(dev_priv)) {
> -		bxt_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> -		hdmi_default_entry = n_hdmi_entries - 1;
> +		bxt_get_buf_trans_hdmi(dev_priv, &n_entries);
> +		default_entry = n_entries - 1;
>  	} else if (IS_GEN9_BC(dev_priv)) {
> -		intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> -		hdmi_default_entry = 8;
> +		intel_ddi_get_buf_trans_hdmi(dev_priv, &n_entries);
> +		default_entry = 8;
>  	} else if (IS_BROADWELL(dev_priv)) {
> -		intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> -		hdmi_default_entry = 7;
> +		intel_ddi_get_buf_trans_hdmi(dev_priv, &n_entries);
> +		default_entry = 7;
>  	} else if (IS_HASWELL(dev_priv)) {
> -		intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> -		hdmi_default_entry = 6;
> +		intel_ddi_get_buf_trans_hdmi(dev_priv, &n_entries);
> +		default_entry = 6;
>  	} else {
>  		WARN(1, "ddi translation table missing\n");
>  		return 0;
>  	}
>  
>  	/* Choose a good default if VBT is badly populated */
> -	if (hdmi_level == HDMI_LEVEL_SHIFT_UNKNOWN ||
> -	    hdmi_level >= n_hdmi_entries)
> -		hdmi_level = hdmi_default_entry;
> +	if (level == HDMI_LEVEL_SHIFT_UNKNOWN || level >= n_entries)
> +		level = default_entry;
>  
> -	if (WARN_ON_ONCE(n_hdmi_entries == 0))
> +	if (WARN_ON_ONCE(n_entries == 0))
>  		return 0;
> -	if (WARN_ON_ONCE(hdmi_level >= n_hdmi_entries))
> -		hdmi_level = n_hdmi_entries - 1;
> +	if (WARN_ON_ONCE(level >= n_entries))
> +		level = n_entries - 1;
>  
> -	return hdmi_level;
> +	return level;
>  }
>  
>  /*
> @@ -859,20 +856,20 @@ static void intel_prepare_dp_ddi_buffers(struct intel_encoder *encoder)
>   * HDMI/DVI use cases.
>   */
>  static void intel_prepare_hdmi_ddi_buffers(struct intel_encoder *encoder,
> -					   int hdmi_level)
> +					   int level)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	u32 iboost_bit = 0;
> -	int n_hdmi_entries;
> +	int n_entries;
>  	enum port port = intel_ddi_get_encoder_port(encoder);
> -	const struct ddi_buf_trans *ddi_translations_hdmi;
> +	const struct ddi_buf_trans *ddi_translations;
>  
> -	ddi_translations_hdmi = intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> +	ddi_translations = intel_ddi_get_buf_trans_hdmi(dev_priv, &n_entries);
>  
> -	if (WARN_ON_ONCE(!ddi_translations_hdmi))
> +	if (WARN_ON_ONCE(!ddi_translations))
>  		return;
> -	if (WARN_ON_ONCE(hdmi_level >= n_hdmi_entries))
> -		hdmi_level = n_hdmi_entries - 1;
> +	if (WARN_ON_ONCE(level >= n_entries))
> +		level = n_entries - 1;
>  
>  	/* If we're boosting the current, set bit 31 of trans1 */
>  	if (IS_GEN9_BC(dev_priv) &&
> @@ -881,9 +878,9 @@ static void intel_prepare_hdmi_ddi_buffers(struct intel_encoder *encoder,
>  
>  	/* Entry 9 is for HDMI: */
>  	I915_WRITE(DDI_BUF_TRANS_LO(port, 9),
> -		   ddi_translations_hdmi[hdmi_level].trans1 | iboost_bit);
> +		   ddi_translations[level].trans1 | iboost_bit);
>  	I915_WRITE(DDI_BUF_TRANS_HI(port, 9),
> -		   ddi_translations_hdmi[hdmi_level].trans2);
> +		   ddi_translations[level].trans2);
>  }
>  
>  static void intel_wait_ddi_buf_idle(struct drm_i915_private *dev_priv,
> @@ -2096,7 +2093,7 @@ u32 bxt_signal_levels(struct intel_dp *intel_dp)
>  	struct intel_digital_port *dport = dp_to_dig_port(intel_dp);
>  	struct drm_i915_private *dev_priv = to_i915(dport->base.base.dev);
>  	struct intel_encoder *encoder = &dport->base;
> -	u32 level = intel_ddi_dp_level(intel_dp);
> +	int level = intel_ddi_dp_level(intel_dp);
>  
>  	if (IS_CANNONLAKE(dev_priv))
>  		cnl_ddi_vswing_sequence(encoder, level, encoder->type);
> @@ -2111,7 +2108,7 @@ uint32_t ddi_signal_levels(struct intel_dp *intel_dp)
>  	struct intel_digital_port *dport = dp_to_dig_port(intel_dp);
>  	struct drm_i915_private *dev_priv = to_i915(dport->base.base.dev);
>  	struct intel_encoder *encoder = &dport->base;
> -	uint32_t level = intel_ddi_dp_level(intel_dp);
> +	int level = intel_ddi_dp_level(intel_dp);
>  
>  	if (IS_GEN9_BC(dev_priv))
>  		skl_ddi_set_iboost(encoder, level, encoder->type);
> @@ -2183,7 +2180,7 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
>  	enum port port = intel_ddi_get_encoder_port(encoder);
>  	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
>  	bool is_mst = intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST);
> -	uint32_t level = intel_ddi_dp_level(intel_dp);
> +	int level = intel_ddi_dp_level(intel_dp);
>  
>  	WARN_ON(is_mst && (port == PORT_A || port == PORT_E));
>  
> -- 
> 2.13.6
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 09/10] drm/i915: Unify error handling for missing DDI buf trans tables

[James Ausmus]

On Wed, Oct 18, 2017 at 09:19:34PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Handle missing buf trans tables, or out of bounds buf trans levels
> the same way everywhere. These should never be hit under normal
> conditions, but let's play it safe for now.
> 
> v2: Avoid the hdmi_level=-1 case (James)
> 
> Cc: James Ausmus <james.ausmus@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: James Ausmus <james.ausmus@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 27 ++++++++++++++++++++++-----
>  1 file changed, 22 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 5ee0e33e2c00..cd7a0d979a10 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -801,6 +801,11 @@ static int intel_ddi_hdmi_level(struct drm_i915_private *dev_priv, enum port por
>  	    hdmi_level >= n_hdmi_entries)
>  		hdmi_level = hdmi_default_entry;
>  
> +	if (WARN_ON_ONCE(n_hdmi_entries == 0))
> +		return 0;
> +	if (WARN_ON_ONCE(hdmi_level >= n_hdmi_entries))
> +		hdmi_level = n_hdmi_entries - 1;
> +
>  	return hdmi_level;
>  }
>  
> @@ -864,6 +869,11 @@ static void intel_prepare_hdmi_ddi_buffers(struct intel_encoder *encoder,
>  
>  	ddi_translations_hdmi = intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
>  
> +	if (WARN_ON_ONCE(!ddi_translations_hdmi))
> +		return;
> +	if (WARN_ON_ONCE(hdmi_level >= n_hdmi_entries))
> +		hdmi_level = n_hdmi_entries - 1;
> +
>  	/* If we're boosting the current, set bit 31 of trans1 */
>  	if (IS_GEN9_BC(dev_priv) &&
>  	    dev_priv->vbt.ddi_port_info[port].hdmi_boost_level)
> @@ -1847,6 +1857,11 @@ static void skl_ddi_set_iboost(struct intel_encoder *encoder,
>  		else
>  			ddi_translations = intel_ddi_get_buf_trans_dp(dev_priv, port, &n_entries);
>  
> +		if (WARN_ON_ONCE(!ddi_translations))
> +			return;
> +		if (WARN_ON_ONCE(level >= n_entries))
> +			level = n_entries - 1;
> +
>  		iboost = ddi_translations[level].i_boost;
>  	}
>  
> @@ -1877,6 +1892,11 @@ static void bxt_ddi_vswing_sequence(struct intel_encoder *encoder,
>  	else
>  		ddi_translations = bxt_get_buf_trans_dp(dev_priv, &n_entries);
>  
> +	if (WARN_ON_ONCE(!ddi_translations))
> +		return;
> +	if (WARN_ON_ONCE(level >= n_entries))
> +		level = n_entries - 1;
> +
>  	bxt_ddi_phy_set_signal_level(dev_priv, port,
>  				     ddi_translations[level].margin,
>  				     ddi_translations[level].scale,
> @@ -1932,13 +1952,10 @@ static void cnl_ddi_vswing_program(struct intel_encoder *encoder,
>  	else
>  		ddi_translations = cnl_get_buf_trans_dp(dev_priv, &n_entries);
>  
> -	if (WARN_ON(ddi_translations == NULL))
> +	if (WARN_ON_ONCE(!ddi_translations))
>  		return;
> -
> -	if (level >= n_entries) {
> -		DRM_DEBUG_KMS("DDI translation not found for level %d. Using %d instead.", level, n_entries - 1);
> +	if (WARN_ON_ONCE(level >= n_entries))
>  		level = n_entries - 1;
> -	}
>  
>  	/* Set PORT_TX_DW5 Scaling Mode Sel to 010b. */
>  	val = I915_READ(CNL_PORT_TX_DW5_LN0(port));
> -- 
> 2.13.6
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 02/10] drm/i915: Extract intel_ddi_get_buf_trans_hdmi()

[Manasi Navare]

On Wed, Oct 18, 2017 at 08:09:24PM +0300, Ville Syrjälä wrote:
> On Mon, Oct 16, 2017 at 04:41:49PM -0700, Ausmus, James wrote:
> > On Mon, Oct 16, 2017 at 7:56 AM, Ville Syrjala
> > <ville.syrjala@linux.intel.com> wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >
> > > Introduce intel_ddi_get_buf_trans_hdmi() and start using it where we
> > > can.
> > >
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_ddi.c | 50 ++++++++++++++++++++++------------------
> > >  1 file changed, 28 insertions(+), 22 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > > index 55937abda61f..e6c884a6d408 100644
> > > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > > @@ -641,6 +641,24 @@ intel_ddi_get_buf_trans_fdi(struct drm_i915_private *dev_priv,
> > >         return NULL;
> > >  }
> > >
> > > +static const struct ddi_buf_trans *
> > > +intel_ddi_get_buf_trans_hdmi(struct drm_i915_private *dev_priv,
> > > +                            int *n_entries)
> > > +{
> > > +       if (IS_GEN9_BC(dev_priv)) {
> > > +               return skl_get_buf_trans_hdmi(dev_priv, n_entries);
> > > +       } else if (IS_BROADWELL(dev_priv)) {
> > > +               *n_entries = ARRAY_SIZE(bdw_ddi_translations_hdmi);
> > > +               return bdw_ddi_translations_hdmi;
> > > +       } else if (IS_HASWELL(dev_priv)) {
> > > +               *n_entries = ARRAY_SIZE(hsw_ddi_translations_hdmi);
> > > +               return hsw_ddi_translations_hdmi;
> > > +       }
> > > +
> > > +       *n_entries = 0;
> > > +       return NULL;
> > > +}
> > > +
> > >  static const struct cnl_ddi_buf_trans *
> > >  cnl_get_buf_trans_hdmi(struct drm_i915_private *dev_priv, int *n_entries)
> > >  {
> > > @@ -723,18 +741,17 @@ static int intel_ddi_hdmi_level(struct drm_i915_private *dev_priv, enum port por
> > >                 cnl_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> > >                 hdmi_default_entry = n_hdmi_entries - 1;
> > 
> > Why are you skipping the CNL case? If you extract it in to
> > intel_ddi_get_buf_trans_hdmi as well, you could just
> > unconditionally call intel_ddi_get_buf_trans_hdmi, and just set
> > hdmi_default_entry in the platform-specific section.
> 
> The problem is that CNL (and BXT) use a different structure for their
> buf trans values. So far I've not bothered to check if we could change
> them to use the HSW+ structure. I guess the other options would be to
> slap a union into the structure, or make these functions return
> void *, but neither option seems very appealing since we'd lose some
> type safety.
> 
> In any case that'd be material for followup work. Maybe someone else
> could take a look at it? ;)
>

I like the idea of the union in the buf_trans structures.
But yes for now, this looks good.

Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>

Manasi
 
> > 
> > >         } else if (IS_GEN9_BC(dev_priv)) {
> > > -               skl_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> > > +               intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> > >                 hdmi_default_entry = 8;
> > >         } else if (IS_BROADWELL(dev_priv)) {
> > > -               n_hdmi_entries = ARRAY_SIZE(bdw_ddi_translations_hdmi);
> > > +               intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> > >                 hdmi_default_entry = 7;
> > >         } else if (IS_HASWELL(dev_priv)) {
> > > -               n_hdmi_entries = ARRAY_SIZE(hsw_ddi_translations_hdmi);
> > > +               intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> > >                 hdmi_default_entry = 6;
> > >         } else {
> > >                 WARN(1, "ddi translation table missing\n");
> > > -               n_hdmi_entries = ARRAY_SIZE(bdw_ddi_translations_hdmi);
> > > -               hdmi_default_entry = 7;
> > > +               return 0;
> > >         }
> > >
> > >         /* Choose a good default if VBT is badly populated */
> > > @@ -810,23 +827,12 @@ static void intel_prepare_hdmi_ddi_buffers(struct intel_encoder *encoder)
> > >
> > >         hdmi_level = intel_ddi_hdmi_level(dev_priv, port);
> > >
> > > -       if (IS_GEN9_BC(dev_priv)) {
> > > -               ddi_translations_hdmi = skl_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> > > +       ddi_translations_hdmi = intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> > >
> > > -               /* If we're boosting the current, set bit 31 of trans1 */
> > > -               if (dev_priv->vbt.ddi_port_info[port].hdmi_boost_level)
> > > -                       iboost_bit = DDI_BUF_BALANCE_LEG_ENABLE;
> > > -       } else if (IS_BROADWELL(dev_priv)) {
> > > -               ddi_translations_hdmi = bdw_ddi_translations_hdmi;
> > > -               n_hdmi_entries = ARRAY_SIZE(bdw_ddi_translations_hdmi);
> > > -       } else if (IS_HASWELL(dev_priv)) {
> > > -               ddi_translations_hdmi = hsw_ddi_translations_hdmi;
> > > -               n_hdmi_entries = ARRAY_SIZE(hsw_ddi_translations_hdmi);
> > > -       } else {
> > > -               WARN(1, "ddi translation table missing\n");
> > > -               ddi_translations_hdmi = bdw_ddi_translations_hdmi;
> > > -               n_hdmi_entries = ARRAY_SIZE(bdw_ddi_translations_hdmi);
> > > -       }
> > > +       /* If we're boosting the current, set bit 31 of trans1 */
> > > +       if (IS_GEN9_BC(dev_priv) &&
> > > +           dev_priv->vbt.ddi_port_info[port].hdmi_boost_level)
> > > +               iboost_bit = DDI_BUF_BALANCE_LEG_ENABLE;
> > >
> > >         /* Entry 9 is for HDMI: */
> > >         I915_WRITE(DDI_BUF_TRANS_LO(port, 9),
> > > @@ -1820,7 +1826,7 @@ static void skl_ddi_set_iboost(struct intel_encoder *encoder, u32 level)
> > >                 if (hdmi_iboost) {
> > >                         iboost = hdmi_iboost;
> > >                 } else {
> > > -                       ddi_translations = skl_get_buf_trans_hdmi(dev_priv, &n_entries);
> > > +                       ddi_translations = intel_ddi_get_buf_trans_hdmi(dev_priv, &n_entries);
> > >                         iboost = ddi_translations[level].i_boost;
> > >                 }
> > >         } else {
> > > --
> > > 2.13.6
> > >
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > 
> > 
> > -- 
> > 
> > 
> > James Ausmus
> 
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 06/10] drm/i915: Pass encoder type to cnl_ddi_vswing_sequence() explicitly

[Manasi Navare]

On Mon, Oct 16, 2017 at 05:57:01PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> encoder->type is unreliable for DP/HDMI, so pass it in explicity into
> cnl_ddi_vswing_sequence(). This matches what we do for BXT.
>

I still dont get why we cant use encoder->type reliably?
Since if I trace back on the caller of intel_ddi_pre_enable_hdmi or
intel_ddi_pre_enable_dp, the caler intel_ddi_pre_enable() calls
either of these based on encoder->type itself.
So why do we explicitly need to pass encoder->type instead
of deriving it from encoder inside the vswing_sequenc_ functions?

May be I am missing something?

Manasi
 
> v2: Pass intel_encoder down to cnl_ddi_vswing_program(), and
>     clean up the argument types while at it
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 53 +++++++++++++++++++---------------------
>  1 file changed, 25 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 2d886148a653..cab72177299c 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1920,20 +1920,21 @@ u8 intel_ddi_dp_voltage_max(struct intel_encoder *encoder)
>  		DP_TRAIN_VOLTAGE_SWING_MASK;
>  }
>  
> -static void cnl_ddi_vswing_program(struct drm_i915_private *dev_priv,
> -				    u32 level, enum port port, int type)
> +static void cnl_ddi_vswing_program(struct intel_encoder *encoder,
> +				   int level, enum intel_output_type type)
>  {
> -	const struct cnl_ddi_buf_trans *ddi_translations = NULL;
> -	u32 n_entries, val;
> -	int ln;
> +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> +	enum port port = intel_ddi_get_encoder_port(encoder);
> +	const struct cnl_ddi_buf_trans *ddi_translations;
> +	int n_entries, ln;
> +	u32 val;
>  
> -	if (type == INTEL_OUTPUT_HDMI) {
> +	if (type == INTEL_OUTPUT_HDMI)
>  		ddi_translations = cnl_get_buf_trans_hdmi(dev_priv, &n_entries);
> -	} else if (type == INTEL_OUTPUT_DP) {
> -		ddi_translations = cnl_get_buf_trans_dp(dev_priv, &n_entries);
> -	} else if (type == INTEL_OUTPUT_EDP) {
> +	else if (type == INTEL_OUTPUT_EDP)
>  		ddi_translations = cnl_get_buf_trans_edp(dev_priv, &n_entries);
> -	}
> +	else
> +		ddi_translations = cnl_get_buf_trans_dp(dev_priv, &n_entries);
>  
>  	if (WARN_ON(ddi_translations == NULL))
>  		return;
> @@ -1986,26 +1987,22 @@ static void cnl_ddi_vswing_program(struct drm_i915_private *dev_priv,
>  	I915_WRITE(CNL_PORT_TX_DW7_GRP(port), val);
>  }
>  
> -static void cnl_ddi_vswing_sequence(struct intel_encoder *encoder, u32 level)
> +static void cnl_ddi_vswing_sequence(struct intel_encoder *encoder,
> +				    int level, enum intel_output_type type)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> -	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
>  	enum port port = intel_ddi_get_encoder_port(encoder);
> -	int type = encoder->type;
> -	int width = 0;
> -	int rate = 0;
> +	int width, rate, ln;
>  	u32 val;
> -	int ln = 0;
>  
> -	if ((intel_dp) && (type == INTEL_OUTPUT_EDP || type == INTEL_OUTPUT_DP)) {
> +	if (type == INTEL_OUTPUT_HDMI) {
> +		width = 4;
> +		rate = 0; /* Rate is always < than 6GHz for HDMI */
> +	} else {
> +		struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> +
>  		width = intel_dp->lane_count;
>  		rate = intel_dp->link_rate;
> -	} else if (type == INTEL_OUTPUT_HDMI) {
> -		width = 4;
> -		/* Rate is always < than 6GHz for HDMI */
> -	} else {
> -		MISSING_CASE(type);
> -		return;
>  	}
>  
>  	/*
> @@ -2014,7 +2011,7 @@ static void cnl_ddi_vswing_sequence(struct intel_encoder *encoder, u32 level)
>  	 * else clear to 0b.
>  	 */
>  	val = I915_READ(CNL_PORT_PCS_DW1_LN0(port));
> -	if (type == INTEL_OUTPUT_EDP || type == INTEL_OUTPUT_DP)
> +	if (type != INTEL_OUTPUT_HDMI)
>  		val |= COMMON_KEEPER_EN;
>  	else
>  		val &= ~COMMON_KEEPER_EN;
> @@ -2049,7 +2046,7 @@ static void cnl_ddi_vswing_sequence(struct intel_encoder *encoder, u32 level)
>  	I915_WRITE(CNL_PORT_TX_DW5_GRP(port), val);
>  
>  	/* 5. Program swing and de-emphasis */
> -	cnl_ddi_vswing_program(dev_priv, level, port, type);
> +	cnl_ddi_vswing_program(encoder, level, type);
>  
>  	/* 6. Set training enable to trigger update */
>  	val = I915_READ(CNL_PORT_TX_DW5_LN0(port));
> @@ -2089,7 +2086,7 @@ u32 bxt_signal_levels(struct intel_dp *intel_dp)
>  	u32 level = intel_ddi_dp_level(intel_dp);
>  
>  	if (IS_CANNONLAKE(dev_priv))
> -		cnl_ddi_vswing_sequence(encoder, level);
> +		cnl_ddi_vswing_sequence(encoder, level, encoder->type);
>  	else
>  		bxt_ddi_vswing_sequence(encoder, level, encoder->type);
>  
> @@ -2188,7 +2185,7 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
>  	intel_display_power_get(dev_priv, dig_port->ddi_io_power_domain);
>  
>  	if (IS_CANNONLAKE(dev_priv))
> -		cnl_ddi_vswing_sequence(encoder, level);
> +		cnl_ddi_vswing_sequence(encoder, level, encoder->type);
>  	else if (IS_GEN9_LP(dev_priv))
>  		bxt_ddi_vswing_sequence(encoder, level, encoder->type);
>  	else
> @@ -2219,7 +2216,7 @@ static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
>  	intel_display_power_get(dev_priv, dig_port->ddi_io_power_domain);
>  
>  	if (IS_CANNONLAKE(dev_priv))
> -		cnl_ddi_vswing_sequence(encoder, level);
> +		cnl_ddi_vswing_sequence(encoder, level, INTEL_OUTPUT_HDMI);
>  	else if (IS_GEN9_LP(dev_priv))
>  		bxt_ddi_vswing_sequence(encoder, level, INTEL_OUTPUT_HDMI);
>  	else
> -- 
> 2.13.6
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for DDI buf trans cleanup (rev3)

[Patchwork]

== Series Details ==

Series: DDI buf trans cleanup (rev3)
URL   : https://patchwork.freedesktop.org/series/32049/
State : success

== Summary ==

Test kms_setmode:
        Subgroup basic:
                pass       -> FAIL       (shard-hsw) fdo#99912
Test kms_chv_cursor_fail:
        Subgroup pipe-A-64x64-top-edge:
                skip       -> PASS       (shard-hsw)
Test prime_self_import:
        Subgroup export-vs-gem_close-race:
                pass       -> FAIL       (shard-hsw) fdo#102655
Test kms_flip:
        Subgroup modeset-vs-vblank-race:
                pass       -> FAIL       (shard-hsw) fdo#103060

fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
fdo#102655 https://bugs.freedesktop.org/show_bug.cgi?id=102655
fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060

shard-hsw        total:2540 pass:1428 dwarn:0   dfail:0   fail:11  skip:1101 time:9228s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6094/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 06/10] drm/i915: Pass encoder type to cnl_ddi_vswing_sequence() explicitly

[Ville Syrjälä]

On Wed, Oct 18, 2017 at 02:11:46PM -0700, Manasi Navare wrote:
> On Mon, Oct 16, 2017 at 05:57:01PM +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > encoder->type is unreliable for DP/HDMI, so pass it in explicity into
> > cnl_ddi_vswing_sequence(). This matches what we do for BXT.
> >
> 
> I still dont get why we cant use encoder->type reliably?
> Since if I trace back on the caller of intel_ddi_pre_enable_hdmi or
> intel_ddi_pre_enable_dp, the caler intel_ddi_pre_enable() calls
> either of these based on encoder->type itself.
> So why do we explicitly need to pass encoder->type instead
> of deriving it from encoder inside the vswing_sequenc_ functions?
> 
> May be I am missing something?

Currently DDI encoder->type changes at unpredicatble times whenever
we detect a DP or HDMI sink, or force the driver to think one or the
other is connected. Thus it can flip flop pretty much randomly at any
time leading to hilarity if we try to drive the port in one mode but
someone plugs in another type of sink.

I'm trying to eliminate that flip-flopping by making sure the 
encoder->type just says "DDI", and instead we figure out the real type
from crtc state output_types. So once I'm done (one more patch set
after this one at least), we will not use encoder->type anymore, except
in a very limited fashion which doesn't lead to misprogramming of the
hardware.

> 
> Manasi
>  
> > v2: Pass intel_encoder down to cnl_ddi_vswing_program(), and
> >     clean up the argument types while at it
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_ddi.c | 53 +++++++++++++++++++---------------------
> >  1 file changed, 25 insertions(+), 28 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > index 2d886148a653..cab72177299c 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -1920,20 +1920,21 @@ u8 intel_ddi_dp_voltage_max(struct intel_encoder *encoder)
> >  		DP_TRAIN_VOLTAGE_SWING_MASK;
> >  }
> >  
> > -static void cnl_ddi_vswing_program(struct drm_i915_private *dev_priv,
> > -				    u32 level, enum port port, int type)
> > +static void cnl_ddi_vswing_program(struct intel_encoder *encoder,
> > +				   int level, enum intel_output_type type)
> >  {
> > -	const struct cnl_ddi_buf_trans *ddi_translations = NULL;
> > -	u32 n_entries, val;
> > -	int ln;
> > +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > +	enum port port = intel_ddi_get_encoder_port(encoder);
> > +	const struct cnl_ddi_buf_trans *ddi_translations;
> > +	int n_entries, ln;
> > +	u32 val;
> >  
> > -	if (type == INTEL_OUTPUT_HDMI) {
> > +	if (type == INTEL_OUTPUT_HDMI)
> >  		ddi_translations = cnl_get_buf_trans_hdmi(dev_priv, &n_entries);
> > -	} else if (type == INTEL_OUTPUT_DP) {
> > -		ddi_translations = cnl_get_buf_trans_dp(dev_priv, &n_entries);
> > -	} else if (type == INTEL_OUTPUT_EDP) {
> > +	else if (type == INTEL_OUTPUT_EDP)
> >  		ddi_translations = cnl_get_buf_trans_edp(dev_priv, &n_entries);
> > -	}
> > +	else
> > +		ddi_translations = cnl_get_buf_trans_dp(dev_priv, &n_entries);
> >  
> >  	if (WARN_ON(ddi_translations == NULL))
> >  		return;
> > @@ -1986,26 +1987,22 @@ static void cnl_ddi_vswing_program(struct drm_i915_private *dev_priv,
> >  	I915_WRITE(CNL_PORT_TX_DW7_GRP(port), val);
> >  }
> >  
> > -static void cnl_ddi_vswing_sequence(struct intel_encoder *encoder, u32 level)
> > +static void cnl_ddi_vswing_sequence(struct intel_encoder *encoder,
> > +				    int level, enum intel_output_type type)
> >  {
> >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > -	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> >  	enum port port = intel_ddi_get_encoder_port(encoder);
> > -	int type = encoder->type;
> > -	int width = 0;
> > -	int rate = 0;
> > +	int width, rate, ln;
> >  	u32 val;
> > -	int ln = 0;
> >  
> > -	if ((intel_dp) && (type == INTEL_OUTPUT_EDP || type == INTEL_OUTPUT_DP)) {
> > +	if (type == INTEL_OUTPUT_HDMI) {
> > +		width = 4;
> > +		rate = 0; /* Rate is always < than 6GHz for HDMI */
> > +	} else {
> > +		struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> > +
> >  		width = intel_dp->lane_count;
> >  		rate = intel_dp->link_rate;
> > -	} else if (type == INTEL_OUTPUT_HDMI) {
> > -		width = 4;
> > -		/* Rate is always < than 6GHz for HDMI */
> > -	} else {
> > -		MISSING_CASE(type);
> > -		return;
> >  	}
> >  
> >  	/*
> > @@ -2014,7 +2011,7 @@ static void cnl_ddi_vswing_sequence(struct intel_encoder *encoder, u32 level)
> >  	 * else clear to 0b.
> >  	 */
> >  	val = I915_READ(CNL_PORT_PCS_DW1_LN0(port));
> > -	if (type == INTEL_OUTPUT_EDP || type == INTEL_OUTPUT_DP)
> > +	if (type != INTEL_OUTPUT_HDMI)
> >  		val |= COMMON_KEEPER_EN;
> >  	else
> >  		val &= ~COMMON_KEEPER_EN;
> > @@ -2049,7 +2046,7 @@ static void cnl_ddi_vswing_sequence(struct intel_encoder *encoder, u32 level)
> >  	I915_WRITE(CNL_PORT_TX_DW5_GRP(port), val);
> >  
> >  	/* 5. Program swing and de-emphasis */
> > -	cnl_ddi_vswing_program(dev_priv, level, port, type);
> > +	cnl_ddi_vswing_program(encoder, level, type);
> >  
> >  	/* 6. Set training enable to trigger update */
> >  	val = I915_READ(CNL_PORT_TX_DW5_LN0(port));
> > @@ -2089,7 +2086,7 @@ u32 bxt_signal_levels(struct intel_dp *intel_dp)
> >  	u32 level = intel_ddi_dp_level(intel_dp);
> >  
> >  	if (IS_CANNONLAKE(dev_priv))
> > -		cnl_ddi_vswing_sequence(encoder, level);
> > +		cnl_ddi_vswing_sequence(encoder, level, encoder->type);
> >  	else
> >  		bxt_ddi_vswing_sequence(encoder, level, encoder->type);
> >  
> > @@ -2188,7 +2185,7 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
> >  	intel_display_power_get(dev_priv, dig_port->ddi_io_power_domain);
> >  
> >  	if (IS_CANNONLAKE(dev_priv))
> > -		cnl_ddi_vswing_sequence(encoder, level);
> > +		cnl_ddi_vswing_sequence(encoder, level, encoder->type);
> >  	else if (IS_GEN9_LP(dev_priv))
> >  		bxt_ddi_vswing_sequence(encoder, level, encoder->type);
> >  	else
> > @@ -2219,7 +2216,7 @@ static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
> >  	intel_display_power_get(dev_priv, dig_port->ddi_io_power_domain);
> >  
> >  	if (IS_CANNONLAKE(dev_priv))
> > -		cnl_ddi_vswing_sequence(encoder, level);
> > +		cnl_ddi_vswing_sequence(encoder, level, INTEL_OUTPUT_HDMI);
> >  	else if (IS_GEN9_LP(dev_priv))
> >  		bxt_ddi_vswing_sequence(encoder, level, INTEL_OUTPUT_HDMI);
> >  	else
> > -- 
> > 2.13.6
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 10/10] drm/i915: Drop the redundant hdmi prefix/suffix from a lot of variables

[Ville Syrjälä]

On Wed, Oct 18, 2017 at 12:59:05PM -0700, James Ausmus wrote:
> On Wed, Oct 18, 2017 at 09:19:58PM +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > A bunch of functions are now exclusively used for HDMI, so naming the
> > variables with hdmi prefix/suffix is redundant. Also use int rather
> > than u32 for the translation level consistently.
> > 
> > v2: Rebase due to hdmi_level=-1 avoidance
> > 
> > Cc: James Ausmus <james.ausmus@intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Reviewed-by: James Ausmus <james.ausmus@intel.com>

Entire series pushed to dinq. Thanks for the reviews.

> 
> > ---
> >  drivers/gpu/drm/i915/intel_ddi.c | 63 +++++++++++++++++++---------------------
> >  1 file changed, 30 insertions(+), 33 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > index cd7a0d979a10..b8925bc82f30 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -770,43 +770,40 @@ cnl_get_buf_trans_edp(struct drm_i915_private *dev_priv, int *n_entries)
> >  
> >  static int intel_ddi_hdmi_level(struct drm_i915_private *dev_priv, enum port port)
> >  {
> > -	int n_hdmi_entries;
> > -	int hdmi_level;
> > -	int hdmi_default_entry;
> > +	int n_entries, level, default_entry;
> >  
> > -	hdmi_level = dev_priv->vbt.ddi_port_info[port].hdmi_level_shift;
> > +	level = dev_priv->vbt.ddi_port_info[port].hdmi_level_shift;
> >  
> >  	if (IS_CANNONLAKE(dev_priv)) {
> > -		cnl_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> > -		hdmi_default_entry = n_hdmi_entries - 1;
> > +		cnl_get_buf_trans_hdmi(dev_priv, &n_entries);
> > +		default_entry = n_entries - 1;
> >  	} else if (IS_GEN9_LP(dev_priv)) {
> > -		bxt_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> > -		hdmi_default_entry = n_hdmi_entries - 1;
> > +		bxt_get_buf_trans_hdmi(dev_priv, &n_entries);
> > +		default_entry = n_entries - 1;
> >  	} else if (IS_GEN9_BC(dev_priv)) {
> > -		intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> > -		hdmi_default_entry = 8;
> > +		intel_ddi_get_buf_trans_hdmi(dev_priv, &n_entries);
> > +		default_entry = 8;
> >  	} else if (IS_BROADWELL(dev_priv)) {
> > -		intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> > -		hdmi_default_entry = 7;
> > +		intel_ddi_get_buf_trans_hdmi(dev_priv, &n_entries);
> > +		default_entry = 7;
> >  	} else if (IS_HASWELL(dev_priv)) {
> > -		intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> > -		hdmi_default_entry = 6;
> > +		intel_ddi_get_buf_trans_hdmi(dev_priv, &n_entries);
> > +		default_entry = 6;
> >  	} else {
> >  		WARN(1, "ddi translation table missing\n");
> >  		return 0;
> >  	}
> >  
> >  	/* Choose a good default if VBT is badly populated */
> > -	if (hdmi_level == HDMI_LEVEL_SHIFT_UNKNOWN ||
> > -	    hdmi_level >= n_hdmi_entries)
> > -		hdmi_level = hdmi_default_entry;
> > +	if (level == HDMI_LEVEL_SHIFT_UNKNOWN || level >= n_entries)
> > +		level = default_entry;
> >  
> > -	if (WARN_ON_ONCE(n_hdmi_entries == 0))
> > +	if (WARN_ON_ONCE(n_entries == 0))
> >  		return 0;
> > -	if (WARN_ON_ONCE(hdmi_level >= n_hdmi_entries))
> > -		hdmi_level = n_hdmi_entries - 1;
> > +	if (WARN_ON_ONCE(level >= n_entries))
> > +		level = n_entries - 1;
> >  
> > -	return hdmi_level;
> > +	return level;
> >  }
> >  
> >  /*
> > @@ -859,20 +856,20 @@ static void intel_prepare_dp_ddi_buffers(struct intel_encoder *encoder)
> >   * HDMI/DVI use cases.
> >   */
> >  static void intel_prepare_hdmi_ddi_buffers(struct intel_encoder *encoder,
> > -					   int hdmi_level)
> > +					   int level)
> >  {
> >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> >  	u32 iboost_bit = 0;
> > -	int n_hdmi_entries;
> > +	int n_entries;
> >  	enum port port = intel_ddi_get_encoder_port(encoder);
> > -	const struct ddi_buf_trans *ddi_translations_hdmi;
> > +	const struct ddi_buf_trans *ddi_translations;
> >  
> > -	ddi_translations_hdmi = intel_ddi_get_buf_trans_hdmi(dev_priv, &n_hdmi_entries);
> > +	ddi_translations = intel_ddi_get_buf_trans_hdmi(dev_priv, &n_entries);
> >  
> > -	if (WARN_ON_ONCE(!ddi_translations_hdmi))
> > +	if (WARN_ON_ONCE(!ddi_translations))
> >  		return;
> > -	if (WARN_ON_ONCE(hdmi_level >= n_hdmi_entries))
> > -		hdmi_level = n_hdmi_entries - 1;
> > +	if (WARN_ON_ONCE(level >= n_entries))
> > +		level = n_entries - 1;
> >  
> >  	/* If we're boosting the current, set bit 31 of trans1 */
> >  	if (IS_GEN9_BC(dev_priv) &&
> > @@ -881,9 +878,9 @@ static void intel_prepare_hdmi_ddi_buffers(struct intel_encoder *encoder,
> >  
> >  	/* Entry 9 is for HDMI: */
> >  	I915_WRITE(DDI_BUF_TRANS_LO(port, 9),
> > -		   ddi_translations_hdmi[hdmi_level].trans1 | iboost_bit);
> > +		   ddi_translations[level].trans1 | iboost_bit);
> >  	I915_WRITE(DDI_BUF_TRANS_HI(port, 9),
> > -		   ddi_translations_hdmi[hdmi_level].trans2);
> > +		   ddi_translations[level].trans2);
> >  }
> >  
> >  static void intel_wait_ddi_buf_idle(struct drm_i915_private *dev_priv,
> > @@ -2096,7 +2093,7 @@ u32 bxt_signal_levels(struct intel_dp *intel_dp)
> >  	struct intel_digital_port *dport = dp_to_dig_port(intel_dp);
> >  	struct drm_i915_private *dev_priv = to_i915(dport->base.base.dev);
> >  	struct intel_encoder *encoder = &dport->base;
> > -	u32 level = intel_ddi_dp_level(intel_dp);
> > +	int level = intel_ddi_dp_level(intel_dp);
> >  
> >  	if (IS_CANNONLAKE(dev_priv))
> >  		cnl_ddi_vswing_sequence(encoder, level, encoder->type);
> > @@ -2111,7 +2108,7 @@ uint32_t ddi_signal_levels(struct intel_dp *intel_dp)
> >  	struct intel_digital_port *dport = dp_to_dig_port(intel_dp);
> >  	struct drm_i915_private *dev_priv = to_i915(dport->base.base.dev);
> >  	struct intel_encoder *encoder = &dport->base;
> > -	uint32_t level = intel_ddi_dp_level(intel_dp);
> > +	int level = intel_ddi_dp_level(intel_dp);
> >  
> >  	if (IS_GEN9_BC(dev_priv))
> >  		skl_ddi_set_iboost(encoder, level, encoder->type);
> > @@ -2183,7 +2180,7 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
> >  	enum port port = intel_ddi_get_encoder_port(encoder);
> >  	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
> >  	bool is_mst = intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST);
> > -	uint32_t level = intel_ddi_dp_level(intel_dp);
> > +	int level = intel_ddi_dp_level(intel_dp);
> >  
> >  	WARN_ON(is_mst && (port == PORT_A || port == PORT_E));
> >  
> > -- 
> > 2.13.6
> > 

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 06/10] drm/i915: Pass encoder type to cnl_ddi_vswing_sequence() explicitly

[Manasi Navare]

On Thu, Oct 19, 2017 at 01:38:06PM +0300, Ville Syrjälä wrote:
> On Wed, Oct 18, 2017 at 02:11:46PM -0700, Manasi Navare wrote:
> > On Mon, Oct 16, 2017 at 05:57:01PM +0300, Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > encoder->type is unreliable for DP/HDMI, so pass it in explicity into
> > > cnl_ddi_vswing_sequence(). This matches what we do for BXT.
> > >
> > 
> > I still dont get why we cant use encoder->type reliably?
> > Since if I trace back on the caller of intel_ddi_pre_enable_hdmi or
> > intel_ddi_pre_enable_dp, the caler intel_ddi_pre_enable() calls
> > either of these based on encoder->type itself.
> > So why do we explicitly need to pass encoder->type instead
> > of deriving it from encoder inside the vswing_sequenc_ functions?
> > 
> > May be I am missing something?
> 
> Currently DDI encoder->type changes at unpredicatble times whenever
> we detect a DP or HDMI sink, or force the driver to think one or the
> other is connected. Thus it can flip flop pretty much randomly at any
> time leading to hilarity if we try to drive the port in one mode but
> someone plugs in another type of sink.
> 
> I'm trying to eliminate that flip-flopping by making sure the 
> encoder->type just says "DDI", and instead we figure out the real type
> from crtc state output_types. So once I'm done (one more patch set
> after this one at least), we will not use encoder->type anymore, except
> in a very limited fashion which doesn't lead to misprogramming of the
> hardware.
>

Thanks a lot for the explanation.
That makes it loud and clear now. So right now intel_ddi_pre_enable()
still uses encoder->type to determine whether to call into
pre_enable_dp() or pre_enable_hdmi(). 
Shouldnt we change that to use crtc state output type instead as well?
Or do you plan to do in the next patch set?

Manasi

> > 
> > Manasi
> >  
> > > v2: Pass intel_encoder down to cnl_ddi_vswing_program(), and
> > >     clean up the argument types while at it
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_ddi.c | 53 +++++++++++++++++++---------------------
> > >  1 file changed, 25 insertions(+), 28 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > > index 2d886148a653..cab72177299c 100644
> > > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > > @@ -1920,20 +1920,21 @@ u8 intel_ddi_dp_voltage_max(struct intel_encoder *encoder)
> > >  		DP_TRAIN_VOLTAGE_SWING_MASK;
> > >  }
> > >  
> > > -static void cnl_ddi_vswing_program(struct drm_i915_private *dev_priv,
> > > -				    u32 level, enum port port, int type)
> > > +static void cnl_ddi_vswing_program(struct intel_encoder *encoder,
> > > +				   int level, enum intel_output_type type)
> > >  {
> > > -	const struct cnl_ddi_buf_trans *ddi_translations = NULL;
> > > -	u32 n_entries, val;
> > > -	int ln;
> > > +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > > +	enum port port = intel_ddi_get_encoder_port(encoder);
> > > +	const struct cnl_ddi_buf_trans *ddi_translations;
> > > +	int n_entries, ln;
> > > +	u32 val;
> > >  
> > > -	if (type == INTEL_OUTPUT_HDMI) {
> > > +	if (type == INTEL_OUTPUT_HDMI)
> > >  		ddi_translations = cnl_get_buf_trans_hdmi(dev_priv, &n_entries);
> > > -	} else if (type == INTEL_OUTPUT_DP) {
> > > -		ddi_translations = cnl_get_buf_trans_dp(dev_priv, &n_entries);
> > > -	} else if (type == INTEL_OUTPUT_EDP) {
> > > +	else if (type == INTEL_OUTPUT_EDP)
> > >  		ddi_translations = cnl_get_buf_trans_edp(dev_priv, &n_entries);
> > > -	}
> > > +	else
> > > +		ddi_translations = cnl_get_buf_trans_dp(dev_priv, &n_entries);
> > >  
> > >  	if (WARN_ON(ddi_translations == NULL))
> > >  		return;
> > > @@ -1986,26 +1987,22 @@ static void cnl_ddi_vswing_program(struct drm_i915_private *dev_priv,
> > >  	I915_WRITE(CNL_PORT_TX_DW7_GRP(port), val);
> > >  }
> > >  
> > > -static void cnl_ddi_vswing_sequence(struct intel_encoder *encoder, u32 level)
> > > +static void cnl_ddi_vswing_sequence(struct intel_encoder *encoder,
> > > +				    int level, enum intel_output_type type)
> > >  {
> > >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > > -	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> > >  	enum port port = intel_ddi_get_encoder_port(encoder);
> > > -	int type = encoder->type;
> > > -	int width = 0;
> > > -	int rate = 0;
> > > +	int width, rate, ln;
> > >  	u32 val;
> > > -	int ln = 0;
> > >  
> > > -	if ((intel_dp) && (type == INTEL_OUTPUT_EDP || type == INTEL_OUTPUT_DP)) {
> > > +	if (type == INTEL_OUTPUT_HDMI) {
> > > +		width = 4;
> > > +		rate = 0; /* Rate is always < than 6GHz for HDMI */
> > > +	} else {
> > > +		struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> > > +
> > >  		width = intel_dp->lane_count;
> > >  		rate = intel_dp->link_rate;
> > > -	} else if (type == INTEL_OUTPUT_HDMI) {
> > > -		width = 4;
> > > -		/* Rate is always < than 6GHz for HDMI */
> > > -	} else {
> > > -		MISSING_CASE(type);
> > > -		return;
> > >  	}
> > >  
> > >  	/*
> > > @@ -2014,7 +2011,7 @@ static void cnl_ddi_vswing_sequence(struct intel_encoder *encoder, u32 level)
> > >  	 * else clear to 0b.
> > >  	 */
> > >  	val = I915_READ(CNL_PORT_PCS_DW1_LN0(port));
> > > -	if (type == INTEL_OUTPUT_EDP || type == INTEL_OUTPUT_DP)
> > > +	if (type != INTEL_OUTPUT_HDMI)
> > >  		val |= COMMON_KEEPER_EN;
> > >  	else
> > >  		val &= ~COMMON_KEEPER_EN;
> > > @@ -2049,7 +2046,7 @@ static void cnl_ddi_vswing_sequence(struct intel_encoder *encoder, u32 level)
> > >  	I915_WRITE(CNL_PORT_TX_DW5_GRP(port), val);
> > >  
> > >  	/* 5. Program swing and de-emphasis */
> > > -	cnl_ddi_vswing_program(dev_priv, level, port, type);
> > > +	cnl_ddi_vswing_program(encoder, level, type);
> > >  
> > >  	/* 6. Set training enable to trigger update */
> > >  	val = I915_READ(CNL_PORT_TX_DW5_LN0(port));
> > > @@ -2089,7 +2086,7 @@ u32 bxt_signal_levels(struct intel_dp *intel_dp)
> > >  	u32 level = intel_ddi_dp_level(intel_dp);
> > >  
> > >  	if (IS_CANNONLAKE(dev_priv))
> > > -		cnl_ddi_vswing_sequence(encoder, level);
> > > +		cnl_ddi_vswing_sequence(encoder, level, encoder->type);
> > >  	else
> > >  		bxt_ddi_vswing_sequence(encoder, level, encoder->type);
> > >  
> > > @@ -2188,7 +2185,7 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
> > >  	intel_display_power_get(dev_priv, dig_port->ddi_io_power_domain);
> > >  
> > >  	if (IS_CANNONLAKE(dev_priv))
> > > -		cnl_ddi_vswing_sequence(encoder, level);
> > > +		cnl_ddi_vswing_sequence(encoder, level, encoder->type);
> > >  	else if (IS_GEN9_LP(dev_priv))
> > >  		bxt_ddi_vswing_sequence(encoder, level, encoder->type);
> > >  	else
> > > @@ -2219,7 +2216,7 @@ static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
> > >  	intel_display_power_get(dev_priv, dig_port->ddi_io_power_domain);
> > >  
> > >  	if (IS_CANNONLAKE(dev_priv))
> > > -		cnl_ddi_vswing_sequence(encoder, level);
> > > +		cnl_ddi_vswing_sequence(encoder, level, INTEL_OUTPUT_HDMI);
> > >  	else if (IS_GEN9_LP(dev_priv))
> > >  		bxt_ddi_vswing_sequence(encoder, level, INTEL_OUTPUT_HDMI);
> > >  	else
> > > -- 
> > > 2.13.6
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx