[PATCH 0/7] Add USB typeC based DP support for BXT platform

[PATCH 0/7] Add USB typeC based DP support for BXT platform

[Durgadoss R]

This patch series adds upfront link training support to enable
USB type C based DP on BXT platform.

To support USB type C alternate DP mode, the display driver needs to
know the number of lanes required by the DP panel as well as number
of lanes that can be supported by the type-C cable. Sometimes, the
type-C cable may limit the bandwidth even if Panel can support
more lanes.

The goal is to find out the number of lanes which can be supported
using a particular cable so that we can cap 'max_available_lanes'
to that number during modeset.

Patches 1-4/7 :Refactoring/exporting DDI functions required
	       to do upfront link train
Patch     5/7 :Exports a common function to update DPCD
Patch     6/7 :Moves finding unused crtc to a common function
Patch     7/7 :Upfront implementation for DDI platforms, that is
               for now, tested on BXT A1.

Changes from RFCv2:
* Rebased on top of atomic and latest-nightly
* Re-used code in load_detect to find unused crtc as per Ander's comment
* Moved DPCD update to common function which can be used by upfront
  link train code for other platforms also.
* intel_crtc_control() does not exist in atomic world; so implemented
  the required dpms_off/on calls through get/release_load_detect()
  functions which take care of locking semantics.
  The other option I tried was using intel_crtc_disable_noatomic()
  But this does not have a corresponding _enable part (and hence
  needed a connector->func->dpms_on() anyway). Hence re-using the
  load_detect functions makes the implementation cleaner.
* Replaced all unnecessary local variables in upfront link train
  code with the corresponding ones in intel_dp struct.
* RFCv2 link: https://patchwork.freedesktop.org/patch/61776/

As per Daniel's suggestion on RFCv1:
* Added the last patch 6/6 that has implementation for CHV
* Made intel_dp_upfront_link_train as common for all platforms
  and added a intel_ddi_upfront_* for all DDI platforms.
  Currently, have restricted it to only for SKL/BXT.
* Moved the upfront code for DDI platforms into intel_ddi.c
  from display.c, since that aligned better with other
  ddi* functions.
* Kept the CHV implementation in display.c as of now
  since we are using some pll functions defined in display.c
  We can discuss and finalize an appropriate place for this
  and then refactor/export required functions.

Durgadoss R (7):
  drm/i915/dp: Reuse encoder if it is already available
  drm/i915/dp: Reuse shared DPLL if it exists already
  drm/i915/dp: Abstract all get_ddi_pll methods
  drm/i915/dp: Export enable/disable_shared_dpll methods
  drm/i915/dp: Add methods to update link train params
  drm/i915: Make finding unused crtc as a generic function
  drm/i915/dp: Enable Upfront link training for typeC DP support on BXT

 drivers/gpu/drm/i915/intel_ddi.c     | 160 ++++++++++++++++++++++++++++-------
 drivers/gpu/drm/i915/intel_display.c |  61 ++++++++-----
 drivers/gpu/drm/i915/intel_dp.c      | 111 +++++++++++++++++++++++-
 drivers/gpu/drm/i915/intel_drv.h     |  12 ++-
 4 files changed, 287 insertions(+), 57 deletions(-)

-- 
1.9.1

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

[PATCH 1/7] drm/i915/dp: Reuse encoder if it is already available

[Durgadoss R]

We do not need to loop through crtc_state to get the
encoder if we already have a valid one available.

Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c     | 11 ++++++++---
 drivers/gpu/drm/i915/intel_display.c |  2 +-
 drivers/gpu/drm/i915/intel_drv.h     |  3 ++-
 3 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 7f618cf..3a71e3c 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1775,11 +1775,16 @@ bxt_ddi_pll_select(struct intel_crtc *intel_crtc,
  * function should be folded into compute_config() eventually.
  */
 bool intel_ddi_pll_select(struct intel_crtc *intel_crtc,
-			  struct intel_crtc_state *crtc_state)
+			  struct intel_crtc_state *crtc_state,
+			  struct intel_encoder *valid_encoder)
 {
 	struct drm_device *dev = intel_crtc->base.dev;
-	struct intel_encoder *intel_encoder =
-		intel_ddi_get_crtc_new_encoder(crtc_state);
+	struct intel_encoder *intel_encoder;
+
+	if (valid_encoder)
+		intel_encoder = valid_encoder;
+	else
+		intel_encoder = intel_ddi_get_crtc_new_encoder(crtc_state);
 
 	if (IS_SKYLAKE(dev) || IS_KABYLAKE(dev))
 		return skl_ddi_pll_select(intel_crtc, crtc_state,
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 70e1b71..0b86a17 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9741,7 +9741,7 @@ static void broadwell_modeset_commit_cdclk(struct drm_atomic_state *old_state)
 static int haswell_crtc_compute_clock(struct intel_crtc *crtc,
 				      struct intel_crtc_state *crtc_state)
 {
-	if (!intel_ddi_pll_select(crtc, crtc_state))
+	if (!intel_ddi_pll_select(crtc, crtc_state, NULL))
 		return -EINVAL;
 
 	crtc->lowfreq_avail = false;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 50f83d2..0ae4dc8 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1007,7 +1007,8 @@ void intel_ddi_disable_transcoder_func(struct drm_i915_private *dev_priv,
 void intel_ddi_enable_pipe_clock(struct intel_crtc *intel_crtc);
 void intel_ddi_disable_pipe_clock(struct intel_crtc *intel_crtc);
 bool intel_ddi_pll_select(struct intel_crtc *crtc,
-			  struct intel_crtc_state *crtc_state);
+			  struct intel_crtc_state *crtc_state,
+			  struct intel_encoder *encoder);
 void intel_ddi_set_pipe_settings(struct drm_crtc *crtc);
 void intel_ddi_prepare_link_retrain(struct intel_dp *intel_dp);
 bool intel_ddi_connector_get_hw_state(struct intel_connector *intel_connector);
-- 
1.9.1

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

[PATCH 2/7] drm/i915/dp: Reuse shared DPLL if it exists already

[Durgadoss R]

Do not call intel_get_shared_dpll() if there exists a
valid shared DPLL already.

Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c     | 70 ++++++++++++++++++++----------------
 drivers/gpu/drm/i915/intel_display.c |  2 +-
 drivers/gpu/drm/i915/intel_drv.h     |  2 +-
 3 files changed, 42 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 3a71e3c..632044a 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1259,7 +1259,8 @@ hsw_ddi_calculate_wrpll(int clock /* in Hz */,
 static bool
 hsw_ddi_pll_select(struct intel_crtc *intel_crtc,
 		   struct intel_crtc_state *crtc_state,
-		   struct intel_encoder *intel_encoder)
+		   struct intel_encoder *intel_encoder,
+		   bool find_dpll)
 {
 	int clock = crtc_state->port_clock;
 
@@ -1279,14 +1280,16 @@ hsw_ddi_pll_select(struct intel_crtc *intel_crtc,
 
 		crtc_state->dpll_hw_state.wrpll = val;
 
-		pll = intel_get_shared_dpll(intel_crtc, crtc_state);
-		if (pll == NULL) {
-			DRM_DEBUG_DRIVER("failed to find PLL for pipe %c\n",
-					 pipe_name(intel_crtc->pipe));
-			return false;
-		}
+		if (find_dpll) {
+			pll = intel_get_shared_dpll(intel_crtc, crtc_state);
+			if (pll == NULL) {
+				DRM_DEBUG_DRIVER("failed to find PLL for pipe %c\n",
+						 pipe_name(intel_crtc->pipe));
+				return false;
+			}
 
-		crtc_state->ddi_pll_sel = PORT_CLK_SEL_WRPLL(pll->id);
+			crtc_state->ddi_pll_sel = PORT_CLK_SEL_WRPLL(pll->id);
+		}
 	} else if (crtc_state->ddi_pll_sel == PORT_CLK_SEL_SPLL) {
 		struct drm_atomic_state *state = crtc_state->base.state;
 		struct intel_shared_dpll_config *spll =
@@ -1553,7 +1556,8 @@ skip_remaining_dividers:
 static bool
 skl_ddi_pll_select(struct intel_crtc *intel_crtc,
 		   struct intel_crtc_state *crtc_state,
-		   struct intel_encoder *intel_encoder)
+		   struct intel_encoder *intel_encoder,
+		   bool find_dpll)
 {
 	struct intel_shared_dpll *pll;
 	uint32_t ctrl1, cfgcr1, cfgcr2;
@@ -1607,15 +1611,17 @@ skl_ddi_pll_select(struct intel_crtc *intel_crtc,
 	crtc_state->dpll_hw_state.cfgcr1 = cfgcr1;
 	crtc_state->dpll_hw_state.cfgcr2 = cfgcr2;
 
-	pll = intel_get_shared_dpll(intel_crtc, crtc_state);
-	if (pll == NULL) {
-		DRM_DEBUG_DRIVER("failed to find PLL for pipe %c\n",
-				 pipe_name(intel_crtc->pipe));
-		return false;
-	}
+	if (find_dpll) {
+		pll = intel_get_shared_dpll(intel_crtc, crtc_state);
+		if (pll == NULL) {
+			DRM_DEBUG_DRIVER("failed to find PLL for pipe %c\n",
+					 pipe_name(intel_crtc->pipe));
+			return false;
+		}
 
-	/* shared DPLL id 0 is DPLL 1 */
-	crtc_state->ddi_pll_sel = pll->id + 1;
+		/* shared DPLL id 0 is DPLL 1 */
+		crtc_state->ddi_pll_sel = pll->id + 1;
+	}
 
 	return true;
 }
@@ -1645,7 +1651,8 @@ static const struct bxt_clk_div bxt_dp_clk_val[] = {
 static bool
 bxt_ddi_pll_select(struct intel_crtc *intel_crtc,
 		   struct intel_crtc_state *crtc_state,
-		   struct intel_encoder *intel_encoder)
+		   struct intel_encoder *intel_encoder,
+		   bool find_pll)
 {
 	struct intel_shared_dpll *pll;
 	struct bxt_clk_div clk_div = {0};
@@ -1754,15 +1761,17 @@ bxt_ddi_pll_select(struct intel_crtc *intel_crtc,
 	crtc_state->dpll_hw_state.pcsdw12 =
 		LANESTAGGER_STRAP_OVRD | lanestagger;
 
-	pll = intel_get_shared_dpll(intel_crtc, crtc_state);
-	if (pll == NULL) {
-		DRM_DEBUG_DRIVER("failed to find PLL for pipe %c\n",
-			pipe_name(intel_crtc->pipe));
-		return false;
-	}
+	if (find_pll) {
+		pll = intel_get_shared_dpll(intel_crtc, crtc_state);
+		if (pll == NULL) {
+			DRM_DEBUG_DRIVER("failed to find PLL for pipe %c\n",
+				pipe_name(intel_crtc->pipe));
+			return false;
+		}
 
-	/* shared DPLL id 0 is DPLL A */
-	crtc_state->ddi_pll_sel = pll->id;
+		/* shared DPLL id 0 is DPLL A */
+		crtc_state->ddi_pll_sel = pll->id;
+	}
 
 	return true;
 }
@@ -1776,7 +1785,8 @@ bxt_ddi_pll_select(struct intel_crtc *intel_crtc,
  */
 bool intel_ddi_pll_select(struct intel_crtc *intel_crtc,
 			  struct intel_crtc_state *crtc_state,
-			  struct intel_encoder *valid_encoder)
+			  struct intel_encoder *valid_encoder,
+			  bool find_dpll)
 {
 	struct drm_device *dev = intel_crtc->base.dev;
 	struct intel_encoder *intel_encoder;
@@ -1788,13 +1798,13 @@ bool intel_ddi_pll_select(struct intel_crtc *intel_crtc,
 
 	if (IS_SKYLAKE(dev) || IS_KABYLAKE(dev))
 		return skl_ddi_pll_select(intel_crtc, crtc_state,
-					  intel_encoder);
+					  intel_encoder, find_dpll);
 	else if (IS_BROXTON(dev))
 		return bxt_ddi_pll_select(intel_crtc, crtc_state,
-					  intel_encoder);
+					  intel_encoder, find_dpll);
 	else
 		return hsw_ddi_pll_select(intel_crtc, crtc_state,
-					  intel_encoder);
+					  intel_encoder, find_dpll);
 }
 
 void intel_ddi_set_pipe_settings(struct drm_crtc *crtc)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 0b86a17..8259104 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9741,7 +9741,7 @@ static void broadwell_modeset_commit_cdclk(struct drm_atomic_state *old_state)
 static int haswell_crtc_compute_clock(struct intel_crtc *crtc,
 				      struct intel_crtc_state *crtc_state)
 {
-	if (!intel_ddi_pll_select(crtc, crtc_state, NULL))
+	if (!intel_ddi_pll_select(crtc, crtc_state, NULL, true))
 		return -EINVAL;
 
 	crtc->lowfreq_avail = false;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 0ae4dc8..2d4c4d3 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1008,7 +1008,7 @@ void intel_ddi_enable_pipe_clock(struct intel_crtc *intel_crtc);
 void intel_ddi_disable_pipe_clock(struct intel_crtc *intel_crtc);
 bool intel_ddi_pll_select(struct intel_crtc *crtc,
 			  struct intel_crtc_state *crtc_state,
-			  struct intel_encoder *encoder);
+			  struct intel_encoder *encoder, bool find_dpll);
 void intel_ddi_set_pipe_settings(struct drm_crtc *crtc);
 void intel_ddi_prepare_link_retrain(struct intel_dp *intel_dp);
 bool intel_ddi_connector_get_hw_state(struct intel_connector *intel_connector);
-- 
1.9.1

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

[PATCH 3/7] drm/i915/dp: Abstract all get_ddi_pll methods

[Durgadoss R]

This patch wraps the get_ddi_pll() methods for
SKL/BXT/HSW+ with a common intel_get_ddi_pll()
method, and exports it, so that it can be shared
by other users also.

Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 18 ++++++++++++------
 drivers/gpu/drm/i915/intel_drv.h     |  2 ++
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8259104..d047947 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9821,6 +9821,17 @@ static void haswell_get_ddi_pll(struct drm_i915_private *dev_priv,
 	}
 }
 
+void intel_get_ddi_pll(struct drm_i915_private *dev_priv, enum port port,
+			struct intel_crtc_state *pipe_config)
+{
+	if (IS_SKYLAKE(dev_priv))
+		skylake_get_ddi_pll(dev_priv, port, pipe_config);
+	else if (IS_BROXTON(dev_priv))
+		bxt_get_ddi_pll(dev_priv, port, pipe_config);
+	else
+		haswell_get_ddi_pll(dev_priv, port, pipe_config);
+}
+
 static void haswell_get_ddi_port_state(struct intel_crtc *crtc,
 				       struct intel_crtc_state *pipe_config)
 {
@@ -9834,12 +9845,7 @@ static void haswell_get_ddi_port_state(struct intel_crtc *crtc,
 
 	port = (tmp & TRANS_DDI_PORT_MASK) >> TRANS_DDI_PORT_SHIFT;
 
-	if (IS_SKYLAKE(dev) || IS_KABYLAKE(dev))
-		skylake_get_ddi_pll(dev_priv, port, pipe_config);
-	else if (IS_BROXTON(dev))
-		bxt_get_ddi_pll(dev_priv, port, pipe_config);
-	else
-		haswell_get_ddi_pll(dev_priv, port, pipe_config);
+	intel_get_ddi_pll(dev_priv, port, pipe_config);
 
 	if (pipe_config->shared_dpll >= 0) {
 		pll = &dev_priv->shared_dplls[pipe_config->shared_dpll];
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 2d4c4d3..1663156 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1009,6 +1009,8 @@ void intel_ddi_disable_pipe_clock(struct intel_crtc *intel_crtc);
 bool intel_ddi_pll_select(struct intel_crtc *crtc,
 			  struct intel_crtc_state *crtc_state,
 			  struct intel_encoder *encoder, bool find_dpll);
+void intel_get_ddi_pll(struct drm_i915_private *dev_priv, enum port port,
+			struct intel_crtc_state *pipe_config);
 void intel_ddi_set_pipe_settings(struct drm_crtc *crtc);
 void intel_ddi_prepare_link_retrain(struct intel_dp *intel_dp);
 bool intel_ddi_connector_get_hw_state(struct intel_connector *intel_connector);
-- 
1.9.1

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

[PATCH 4/7] drm/i915/dp: Export enable/disable_shared_dpll methods

[Durgadoss R]

This patch exports the intel_{enable/disable}_shared_dpll
methods so that they can be called from other files also.
Subsequent patches need to call this from intel_ddi.c

Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 4 ++--
 drivers/gpu/drm/i915/intel_drv.h     | 2 ++
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index d047947..16ad5cd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1891,7 +1891,7 @@ static void intel_prepare_shared_dpll(struct intel_crtc *crtc)
  * The PCH PLL needs to be enabled before the PCH transcoder, since it
  * drives the transcoder clock.
  */
-static void intel_enable_shared_dpll(struct intel_crtc *crtc)
+void intel_enable_shared_dpll(struct intel_crtc *crtc)
 {
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -1921,7 +1921,7 @@ static void intel_enable_shared_dpll(struct intel_crtc *crtc)
 	pll->on = true;
 }
 
-static void intel_disable_shared_dpll(struct intel_crtc *crtc)
+void intel_disable_shared_dpll(struct intel_crtc *crtc)
 {
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 1663156..faa91fc 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1141,6 +1141,8 @@ void intel_create_rotation_property(struct drm_device *dev,
 
 /* shared dpll functions */
 struct intel_shared_dpll *intel_crtc_to_shared_dpll(struct intel_crtc *crtc);
+void intel_enable_shared_dpll(struct intel_crtc *crtc);
+void intel_disable_shared_dpll(struct intel_crtc *crtc);
 void assert_shared_dpll(struct drm_i915_private *dev_priv,
 			struct intel_shared_dpll *pll,
 			bool state);
-- 
1.9.1

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

[PATCH 5/7] drm/i915/dp: Add methods to update link train params

[Durgadoss R]

Retrying with reduced lanes/bw and updating the final
available lanes/bw to DPCD is needed for upfront link
train logic. Hence, this patch adds these methods
and exports them so that these can be called from
other files like ddi.c/display.c.

Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c  | 34 ++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h |  2 ++
 2 files changed, 36 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index f335c92..d8f7830 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1660,6 +1660,40 @@ void intel_dp_set_link_params(struct intel_dp *intel_dp,
 	intel_dp->lane_count = pipe_config->lane_count;
 }
 
+void intel_dp_update_dpcd_params(struct intel_dp *intel_dp)
+{
+	intel_dp->dpcd[DP_MAX_LANE_COUNT] &= ~DP_MAX_LANE_COUNT_MASK;
+	intel_dp->dpcd[DP_MAX_LANE_COUNT] |=
+			intel_dp->lane_count & DP_MAX_LANE_COUNT_MASK;
+
+	intel_dp->dpcd[DP_MAX_LINK_RATE] =
+			drm_dp_link_rate_to_bw_code(intel_dp->link_rate);
+}
+
+bool intel_dp_get_link_retry_params(uint8_t *lane_count, uint8_t *link_bw)
+{
+	/*
+	 * As per DP1.3 Spec, retry all link rates for a particular
+	 * lane count value, before reducing number of lanes.
+	 */
+	if (*link_bw == DP_LINK_BW_5_4) {
+		*link_bw = DP_LINK_BW_2_7;
+	} else if (*link_bw == DP_LINK_BW_2_7) {
+		*link_bw = DP_LINK_BW_1_62;
+	} else if (*lane_count == 4) {
+		*lane_count = 2;
+		*link_bw = DP_LINK_BW_5_4;
+	} else if (*lane_count == 2) {
+		*lane_count = 1;
+		*link_bw = DP_LINK_BW_5_4;
+	} else {
+		/* Tried all combinations, so exit */
+		return false;
+	}
+
+	return true;
+}
+
 static void intel_dp_prepare(struct intel_encoder *encoder)
 {
 	struct drm_device *dev = encoder->base.dev;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index faa91fc..5cefb0e 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1233,6 +1233,8 @@ bool intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 			     struct intel_connector *intel_connector);
 void intel_dp_set_link_params(struct intel_dp *intel_dp,
 			      const struct intel_crtc_state *pipe_config);
+void intel_dp_update_dpcd_params(struct intel_dp *intel_dp);
+bool intel_dp_get_link_retry_params(uint8_t *lane_count, uint8_t *link_bw);
 void intel_dp_start_link_train(struct intel_dp *intel_dp);
 void intel_dp_stop_link_train(struct intel_dp *intel_dp);
 void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode);
-- 
1.9.1

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

[PATCH 6/7] drm/i915: Make finding unused crtc as a generic function

[Durgadoss R]

Looping over the crtc list and finding an unused crtc
has users other than load_detect(). Hence move it to
a common function so that we can re-use the logic.

Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 37 ++++++++++++++++++++++--------------
 drivers/gpu/drm/i915/intel_drv.h     |  1 +
 2 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 16ad5cd..fb0008f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10325,6 +10325,27 @@ static int intel_modeset_setup_plane_state(struct drm_atomic_state *state,
 	return 0;
 }
 
+struct drm_crtc *intel_get_unused_crtc(struct drm_encoder *encoder)
+{
+	struct drm_crtc *possible_crtc;
+	struct drm_crtc *crtc = NULL;
+	struct drm_device *dev = encoder->dev;
+	int i = -1;
+
+	for_each_crtc(dev, possible_crtc) {
+		i++;
+		if (!(encoder->possible_crtcs & (1 << i)))
+			continue;
+		if (possible_crtc->state->enable)
+			continue;
+
+		crtc = possible_crtc;
+		break;
+	}
+
+	return crtc;
+}
+
 bool intel_get_load_detect_pipe(struct drm_connector *connector,
 				struct drm_display_mode *mode,
 				struct intel_load_detect_pipe *old,
@@ -10333,7 +10354,6 @@ bool intel_get_load_detect_pipe(struct drm_connector *connector,
 	struct intel_crtc *intel_crtc;
 	struct intel_encoder *intel_encoder =
 		intel_attached_encoder(connector);
-	struct drm_crtc *possible_crtc;
 	struct drm_encoder *encoder = &intel_encoder->base;
 	struct drm_crtc *crtc = NULL;
 	struct drm_device *dev = encoder->dev;
@@ -10342,7 +10362,7 @@ bool intel_get_load_detect_pipe(struct drm_connector *connector,
 	struct drm_atomic_state *state = NULL;
 	struct drm_connector_state *connector_state;
 	struct intel_crtc_state *crtc_state;
-	int ret, i = -1;
+	int ret;
 
 	DRM_DEBUG_KMS("[CONNECTOR:%d:%s], [ENCODER:%d:%s]\n",
 		      connector->base.id, connector->name,
@@ -10384,21 +10404,10 @@ retry:
 		return true;
 	}
 
-	/* Find an unused one (if possible) */
-	for_each_crtc(dev, possible_crtc) {
-		i++;
-		if (!(encoder->possible_crtcs & (1 << i)))
-			continue;
-		if (possible_crtc->state->enable)
-			continue;
-
-		crtc = possible_crtc;
-		break;
-	}
-
 	/*
 	 * If we didn't find an unused CRTC, don't use any.
 	 */
+	crtc = intel_get_unused_crtc(encoder);
 	if (!crtc) {
 		DRM_DEBUG_KMS("no pipe available for load-detect\n");
 		goto fail;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 5cefb0e..5912955 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1100,6 +1100,7 @@ bool intel_get_load_detect_pipe(struct drm_connector *connector,
 void intel_release_load_detect_pipe(struct drm_connector *connector,
 				    struct intel_load_detect_pipe *old,
 				    struct drm_modeset_acquire_ctx *ctx);
+struct drm_crtc *intel_get_unused_crtc(struct drm_encoder *encoder);
 int intel_pin_and_fence_fb_obj(struct drm_plane *plane,
 			       struct drm_framebuffer *fb,
 			       const struct drm_plane_state *plane_state);
-- 
1.9.1

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

[PATCH 7/7] drm/i915/dp: Enable Upfront link training for typeC DP support on BXT

[Durgadoss R]

To support USB type C alternate DP mode, the display driver needs to
know the number of lanes required by the DP panel as well as number
of lanes that can be supported by the type-C cable. Sometimes, the
type-C cable may limit the bandwidth even if Panel can support
more lanes. To address these scenarios, the display driver will
start link training with max lanes, and if that fails, the driver
falls back to x2 lanes; and repeats this procedure for all
bandwidth/lane configurations.

* Since link training is done before modeset only the port
  (and not pipe/planes) and its associated PLLs are enabled.
* On DP hotplug: Directly start link training on the crtc
  associated with the DP encoder.
* On Connected boot scenarios: When booted with an LFP and a DP,
  typically, BIOS brings up DP. In these cases, we disable the
  crtc, do upfront link training and then re-enable it back.
* All local changes made for upfront link training are reset
  to their previous values once it is done; so that the
  subsequent modeset is not aware of these changes.

Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 81 ++++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_dp.c  | 77 +++++++++++++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/intel_drv.h |  2 +
 3 files changed, 159 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 632044a..43efc26 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -3274,6 +3274,87 @@ intel_ddi_init_hdmi_connector(struct intel_digital_port *intel_dig_port)
 	return connector;
 }
 
+bool intel_ddi_upfront_link_train(struct intel_dp *intel_dp,
+				struct intel_crtc *crtc)
+{
+	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
+	struct intel_connector *connector = intel_dp->attached_connector;
+	struct intel_encoder *encoder = connector->encoder;
+	struct drm_device *dev = encoder->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_shared_dpll *pll;
+	struct drm_crtc *drm_crtc = NULL;
+	struct intel_crtc_state *tmp_crtc_config;
+	struct intel_dpll_hw_state tmp_dpll_hw_state;
+	uint8_t link_bw, lane_count;
+
+	if (!crtc) {
+		drm_crtc = intel_get_unused_crtc(&encoder->base);
+		if (!drm_crtc) {
+			DRM_ERROR("No crtc for upfront link training\n");
+			return false;
+		}
+		encoder->base.crtc = drm_crtc;
+		crtc = to_intel_crtc(drm_crtc);
+	}
+
+	/* Initialize with Max Link rate & lane count supported by panel */
+	link_bw =  intel_dp->dpcd[DP_MAX_LINK_RATE];
+	lane_count = drm_dp_max_lane_count(intel_dp->dpcd);
+
+	/* Save the crtc->config */
+	tmp_crtc_config = crtc->config;
+	tmp_dpll_hw_state = crtc->config->dpll_hw_state;
+
+	/* Select the shared DPLL to use for this port */
+	intel_get_ddi_pll(dev_priv, dig_port->port, crtc->config);
+	pll = intel_crtc_to_shared_dpll(crtc);
+	if (!pll) {
+		DRM_ERROR("Could not get shared DPLL\n");
+		goto exit;
+	}
+	DRM_DEBUG_KMS("Using %s for pipe %c\n", pll->name, pipe_name(crtc->pipe));
+
+	do {
+		crtc->config->port_clock = drm_dp_bw_code_to_link_rate(link_bw);
+		crtc->config->lane_count = lane_count;
+		if (!intel_ddi_pll_select(crtc, crtc->config, encoder, false))
+			goto exit;
+
+		pll->config.crtc_mask |= (1 << crtc->pipe);
+		pll->config.hw_state = crtc->config->dpll_hw_state;
+
+		/* Enable PLL followed by port */
+		intel_enable_shared_dpll(crtc);
+		encoder->pre_enable(encoder);
+
+		/* Check if link training passed; if so update DPCD */
+		if (intel_dp->train_set_valid)
+			intel_dp_update_dpcd_params(intel_dp);
+
+		/* Disable port followed by PLL for next retry/clean up */
+		encoder->post_disable(encoder);
+		intel_disable_shared_dpll(crtc);
+
+	} while (!intel_dp->train_set_valid &&
+		!intel_dp_get_link_retry_params(&lane_count, &link_bw));
+
+	/* Reset pll state as before */
+	pll->config.crtc_mask &= ~(1 << crtc->pipe);
+	pll->config.hw_state = tmp_dpll_hw_state;
+
+exit:
+	/* Reset local associations made */
+	if (drm_crtc)
+		encoder->base.crtc = NULL;
+	crtc->config = tmp_crtc_config;
+
+	DRM_DEBUG_KMS("Upfront link train %s: lanes:%d bw:%d\n",
+	intel_dp->train_set_valid ? "Passed" : "Failed", lane_count, link_bw);
+
+	return intel_dp->train_set_valid;
+}
+
 void intel_ddi_init(struct drm_device *dev, enum port port)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index d8f7830..47b6266 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4622,6 +4622,71 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
 	intel_dp->has_audio = false;
 }
 
+static void intel_dp_upfront_dpms_off(struct drm_connector *connector,
+				struct drm_modeset_acquire_ctx *ctx)
+{
+	struct intel_dp *intel_dp = intel_attached_dp(connector);
+	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+	struct drm_crtc *crtc = intel_dig_port->base.base.crtc;
+	struct intel_load_detect_pipe tmp;
+
+	if (intel_get_load_detect_pipe(connector, NULL, &tmp, ctx)) {
+		crtc->acquire_ctx = ctx;
+		tmp.dpms_mode = DRM_MODE_DPMS_OFF;
+		intel_release_load_detect_pipe(connector, &tmp, ctx);
+	}
+}
+
+static void intel_dp_upfront_dpms_on(struct drm_connector *connector,
+				struct drm_modeset_acquire_ctx *ctx)
+{
+	struct intel_load_detect_pipe tmp;
+
+	intel_get_load_detect_pipe(connector, NULL, &tmp, ctx);
+
+	drm_modeset_drop_locks(ctx);
+	drm_modeset_acquire_fini(ctx);
+}
+
+static bool intel_dp_upfront_link_train(struct drm_connector *connector)
+{
+	struct intel_dp *intel_dp = intel_attached_dp(connector);
+	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+	struct intel_encoder *intel_encoder = &intel_dig_port->base;
+	struct drm_device *dev = intel_encoder->base.dev;
+	struct drm_crtc *crtc = intel_dig_port->base.base.crtc;
+	struct intel_crtc *intel_crtc = crtc ? to_intel_crtc(crtc) : NULL;
+	struct drm_modeset_acquire_ctx ctx, *old_ctx = NULL;
+	bool ret = true, need_dpms_on = false;
+
+	if (!IS_BROXTON(dev))
+		return true;
+	/*
+	 * On hotplug cases, we call _upfront_link_train directly.
+	 * In connected boot scenarios (boot with {MIPI/eDP} + DP),
+	 * BIOS typically brings up DP. Hence, we disable the crtc
+	 * to do _upfront_link_training and then re-enable it back.
+	 */
+	if (crtc && crtc->enabled && intel_crtc->active) {
+		DRM_DEBUG_KMS("Disabling pipe %c\n", pipe_name(intel_crtc->pipe));
+		old_ctx = crtc->acquire_ctx;
+		drm_modeset_acquire_init(&ctx, 0);
+		intel_dp_upfront_dpms_off(connector, &ctx);
+		need_dpms_on = true;
+	}
+
+	if (HAS_DDI(dev))
+		ret = intel_ddi_upfront_link_train(intel_dp, intel_crtc);
+		/* Other platforms upfront link train call goes here..*/
+
+	if (need_dpms_on) {
+		intel_dp_upfront_dpms_on(connector, &ctx);
+		crtc->acquire_ctx = old_ctx;
+	}
+	return ret;
+}
+
+
 static enum drm_connector_status
 intel_dp_detect(struct drm_connector *connector, bool force)
 {
@@ -4631,7 +4696,7 @@ intel_dp_detect(struct drm_connector *connector, bool force)
 	struct drm_device *dev = connector->dev;
 	enum drm_connector_status status;
 	enum intel_display_power_domain power_domain;
-	bool ret;
+	bool ret, do_upfront_link_train;
 	u8 sink_irq_vector;
 
 	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
@@ -4705,6 +4770,16 @@ intel_dp_detect(struct drm_connector *connector, bool force)
 			DRM_DEBUG_DRIVER("CP or sink specific irq unhandled\n");
 	}
 
+	/* Do not do upfront link train, if it is a compliance request */
+	do_upfront_link_train =
+		intel_encoder->type == INTEL_OUTPUT_DISPLAYPORT &&
+		intel_dp->compliance_test_type != DP_TEST_LINK_TRAINING;
+
+	if (do_upfront_link_train) {
+		ret = intel_dp_upfront_link_train(connector);
+		if (!ret)
+			status = connector_status_disconnected;
+	}
 out:
 	intel_display_power_put(to_i915(dev), power_domain);
 	return status;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 5912955..3cfab20 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1025,6 +1025,8 @@ void intel_ddi_clock_get(struct intel_encoder *encoder,
 			 struct intel_crtc_state *pipe_config);
 void intel_ddi_set_vc_payload_alloc(struct drm_crtc *crtc, bool state);
 uint32_t ddi_signal_levels(struct intel_dp *intel_dp);
+bool intel_ddi_upfront_link_train(struct intel_dp *intel_dp,
+				struct intel_crtc *crtc);
 
 /* intel_frontbuffer.c */
 void intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
-- 
1.9.1

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

Re: [PATCH 7/7] drm/i915/dp: Enable Upfront link training for typeC DP support on BXT

[Ander Conselvan De Oliveira]

On Fri, 2015-12-11 at 15:09 +0530, Durgadoss R wrote:
> To support USB type C alternate DP mode, the display driver needs to
> know the number of lanes required by the DP panel as well as number
> of lanes that can be supported by the type-C cable. Sometimes, the
> type-C cable may limit the bandwidth even if Panel can support
> more lanes. To address these scenarios, the display driver will
> start link training with max lanes, and if that fails, the driver
> falls back to x2 lanes; and repeats this procedure for all
> bandwidth/lane configurations.
> 
> * Since link training is done before modeset only the port
>   (and not pipe/planes) and its associated PLLs are enabled.
> * On DP hotplug: Directly start link training on the crtc
>   associated with the DP encoder.
> * On Connected boot scenarios: When booted with an LFP and a DP,
>   typically, BIOS brings up DP. In these cases, we disable the
>   crtc, do upfront link training and then re-enable it back.
> * All local changes made for upfront link training are reset
>   to their previous values once it is done; so that the
>   subsequent modeset is not aware of these changes.
> 
> Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 81
> ++++++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_dp.c  | 77 +++++++++++++++++++++++++++++++++++++-
>  drivers/gpu/drm/i915/intel_drv.h |  2 +
>  3 files changed, 159 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> b/drivers/gpu/drm/i915/intel_ddi.c
> index 632044a..43efc26 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -3274,6 +3274,87 @@ intel_ddi_init_hdmi_connector(struct intel_digital_port
> *intel_dig_port)
>  	return connector;
>  }
>  
> +bool intel_ddi_upfront_link_train(struct intel_dp *intel_dp,
> +				struct intel_crtc *crtc)
> +{
> +	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> +	struct intel_connector *connector = intel_dp->attached_connector;
> +	struct intel_encoder *encoder = connector->encoder;
> +	struct drm_device *dev = encoder->base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_shared_dpll *pll;
> +	struct drm_crtc *drm_crtc = NULL;
> +	struct intel_crtc_state *tmp_crtc_config;
> +	struct intel_dpll_hw_state tmp_dpll_hw_state;
> +	uint8_t link_bw, lane_count;
> +
> +	if (!crtc) {
> +		drm_crtc = intel_get_unused_crtc(&encoder->base);
> +		if (!drm_crtc) {
> +			DRM_ERROR("No crtc for upfront link training\n");
> +			return false;
> +		}
> +		encoder->base.crtc = drm_crtc;
> +		crtc = to_intel_crtc(drm_crtc);
> +	}
> +
> +	/* Initialize with Max Link rate & lane count supported by panel */
> +	link_bw =  intel_dp->dpcd[DP_MAX_LINK_RATE];
> +	lane_count = drm_dp_max_lane_count(intel_dp->dpcd);
> +
> +	/* Save the crtc->config */
> +	tmp_crtc_config = crtc->config;

> +	tmp_dpll_hw_state = crtc->config->dpll_hw_state;
> +
> +	/* Select the shared DPLL to use for this port */
> +	intel_get_ddi_pll(dev_priv, dig_port->port, crtc->config)

This reads the current programmed DPLL from the hardware. Is there any reason we
can't trust the value that is in crtc->config already? I don't think this code
would run before reading out and sanitizing the hardware state.

> ;
> +	pll = intel_crtc_to_shared_dpll(crtc);
> +	if (!pll) {
> +		DRM_ERROR("Could not get shared DPLL\n");
> +		goto exit;
> +	}
> +	DRM_DEBUG_KMS("Using %s for pipe %c\n", pll->name, pipe_name(crtc
> ->pipe));
> +
> +	do {
> +		crtc->config->port_clock =
> drm_dp_bw_code_to_link_rate(link_bw);
> +		crtc->config->lane_count = lane_count;
> +		if (!intel_ddi_pll_select(crtc, crtc->config, encoder,
> false))


> +			goto exit;
> +
> +		pll->config.crtc_mask |= (1 << crtc->pipe);
> +		pll->config.hw_state = crtc->config->dpll_hw_state;
> +
> +		/* Enable PLL followed by port */
> +		intel_enable_shared_dpll(crtc);
> +		encoder->pre_enable(encoder);

The pll handling here seems dodgy. Instead of using intel_get_shared_dpll(),
this fiddles with pll internals itself. I think this will work for broxton,
since it doesn't actually have shared DPLLs (their chosen based on the encoder).
It might just work for haswell too since the plls used by DP are not shared.

But to do this cleanly we need the DPLL interface to just give us the right pll.

Ander


> +
> +		/* Check if link training passed; if so update DPCD */
> +		if (intel_dp->train_set_valid)
> +			intel_dp_update_dpcd_params(intel_dp);
> +
> +		/* Disable port followed by PLL for next retry/clean up */
> +		encoder->post_disable(encoder);
> +		intel_disable_shared_dpll(crtc);
> +
> +	} while (!intel_dp->train_set_valid &&
> +		!intel_dp_get_link_retry_params(&lane_count, &link_bw));
> +
> +	/* Reset pll state as before */
> +	pll->config.crtc_mask &= ~(1 << crtc->pipe);
> +	pll->config.hw_state = tmp_dpll_hw_state;
> +
> +exit:
> +	/* Reset local associations made */
> +	if (drm_crtc)
> +		encoder->base.crtc = NULL;
> +	crtc->config = tmp_crtc_config;
> +
> +	DRM_DEBUG_KMS("Upfront link train %s: lanes:%d bw:%d\n",
> +	intel_dp->train_set_valid ? "Passed" : "Failed", lane_count,
> link_bw);
> +
> +	return intel_dp->train_set_valid;
> +}
> +
>  void intel_ddi_init(struct drm_device *dev, enum port port)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index d8f7830..47b6266 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4622,6 +4622,71 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
>  	intel_dp->has_audio = false;
>  }
>  
> +static void intel_dp_upfront_dpms_off(struct drm_connector *connector,
> +				struct drm_modeset_acquire_ctx *ctx)
> +{
> +	struct intel_dp *intel_dp = intel_attached_dp(connector);
> +	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> +	struct drm_crtc *crtc = intel_dig_port->base.base.crtc;
> +	struct intel_load_detect_pipe tmp;
> +
> +	if (intel_get_load_detect_pipe(connector, NULL, &tmp, ctx)) {
> +		crtc->acquire_ctx = ctx;
> +		tmp.dpms_mode = DRM_MODE_DPMS_OFF;
> +		intel_release_load_detect_pipe(connector, &tmp, ctx);
> +	}
> +}
> +
> +static void intel_dp_upfront_dpms_on(struct drm_connector *connector,
> +				struct drm_modeset_acquire_ctx *ctx)
> +{
> +	struct intel_load_detect_pipe tmp;
> +
> +	intel_get_load_detect_pipe(connector, NULL, &tmp, ctx);
> +
> +	drm_modeset_drop_locks(ctx);
> +	drm_modeset_acquire_fini(ctx);
> +}
> +
> +static bool intel_dp_upfront_link_train(struct drm_connector *connector)
> +{
> +	struct intel_dp *intel_dp = intel_attached_dp(connector);
> +	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> +	struct intel_encoder *intel_encoder = &intel_dig_port->base;
> +	struct drm_device *dev = intel_encoder->base.dev;
> +	struct drm_crtc *crtc = intel_dig_port->base.base.crtc;
> +	struct intel_crtc *intel_crtc = crtc ? to_intel_crtc(crtc) : NULL;
> +	struct drm_modeset_acquire_ctx ctx, *old_ctx = NULL;
> +	bool ret = true, need_dpms_on = false;
> +
> +	if (!IS_BROXTON(dev))
> +		return true;
> +	/*
> +	 * On hotplug cases, we call _upfront_link_train directly.
> +	 * In connected boot scenarios (boot with {MIPI/eDP} + DP),
> +	 * BIOS typically brings up DP. Hence, we disable the crtc
> +	 * to do _upfront_link_training and then re-enable it back.
> +	 */
> +	if (crtc && crtc->enabled && intel_crtc->active) {
> +		DRM_DEBUG_KMS("Disabling pipe %c\n", pipe_name(intel_crtc
> ->pipe));
> +		old_ctx = crtc->acquire_ctx;
> +		drm_modeset_acquire_init(&ctx, 0);
> +		intel_dp_upfront_dpms_off(connector, &ctx);
> +		need_dpms_on = true;
> +	}
> +
> +	if (HAS_DDI(dev))
> +		ret = intel_ddi_upfront_link_train(intel_dp, intel_crtc);
> +		/* Other platforms upfront link train call goes here..*/
> +
> +	if (need_dpms_on) {
> +		intel_dp_upfront_dpms_on(connector, &ctx);
> +		crtc->acquire_ctx = old_ctx;
> +	}
> +	return ret;
> +}
> +
> +
>  static enum drm_connector_status
>  intel_dp_detect(struct drm_connector *connector, bool force)
>  {
> @@ -4631,7 +4696,7 @@ intel_dp_detect(struct drm_connector *connector, bool
> force)
>  	struct drm_device *dev = connector->dev;
>  	enum drm_connector_status status;
>  	enum intel_display_power_domain power_domain;
> -	bool ret;
> +	bool ret, do_upfront_link_train;
>  	u8 sink_irq_vector;
>  
>  	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
> @@ -4705,6 +4770,16 @@ intel_dp_detect(struct drm_connector *connector, bool
> force)
>  			DRM_DEBUG_DRIVER("CP or sink specific irq
> unhandled\n");
>  	}
>  
> +	/* Do not do upfront link train, if it is a compliance request */
> +	do_upfront_link_train =
> +		intel_encoder->type == INTEL_OUTPUT_DISPLAYPORT &&
> +		intel_dp->compliance_test_type != DP_TEST_LINK_TRAINING;
> +
> +	if (do_upfront_link_train) {
> +		ret = intel_dp_upfront_link_train(connector);
> +		if (!ret)
> +			status = connector_status_disconnected;
> +	}
>  out:
>  	intel_display_power_put(to_i915(dev), power_domain);
>  	return status;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 5912955..3cfab20 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1025,6 +1025,8 @@ void intel_ddi_clock_get(struct intel_encoder *encoder,
>  			 struct intel_crtc_state *pipe_config);
>  void intel_ddi_set_vc_payload_alloc(struct drm_crtc *crtc, bool state);
>  uint32_t ddi_signal_levels(struct intel_dp *intel_dp);
> +bool intel_ddi_upfront_link_train(struct intel_dp *intel_dp,
> +				struct intel_crtc *crtc);
>  
>  /* intel_frontbuffer.c */
>  void intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 7/7] drm/i915/dp: Enable Upfront link training for typeC DP support on BXT

[R, Durgadoss]

Hi Ander,

Thanks for looking into this..

>-----Original Message-----
>From: Ander Conselvan De Oliveira [mailto:conselvan2@gmail.com]
>Sent: Tuesday, December 29, 2015 10:52 PM
>To: R, Durgadoss; intel-gfx@lists.freedesktop.org
>Subject: Re: [PATCH 7/7] drm/i915/dp: Enable Upfront link training for typeC DP support on BXT
>
>On Fri, 2015-12-11 at 15:09 +0530, Durgadoss R wrote:
>> To support USB type C alternate DP mode, the display driver needs to
>> know the number of lanes required by the DP panel as well as number
>> of lanes that can be supported by the type-C cable. Sometimes, the
>> type-C cable may limit the bandwidth even if Panel can support
>> more lanes. To address these scenarios, the display driver will
>> start link training with max lanes, and if that fails, the driver
>> falls back to x2 lanes; and repeats this procedure for all
>> bandwidth/lane configurations.
>>
>> * Since link training is done before modeset only the port
>>   (and not pipe/planes) and its associated PLLs are enabled.
>> * On DP hotplug: Directly start link training on the crtc
>>   associated with the DP encoder.
>> * On Connected boot scenarios: When booted with an LFP and a DP,
>>   typically, BIOS brings up DP. In these cases, we disable the
>>   crtc, do upfront link training and then re-enable it back.
>> * All local changes made for upfront link training are reset
>>   to their previous values once it is done; so that the
>>   subsequent modeset is not aware of these changes.
>>
>> Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_ddi.c | 81
>> ++++++++++++++++++++++++++++++++++++++++
>>  drivers/gpu/drm/i915/intel_dp.c  | 77 +++++++++++++++++++++++++++++++++++++-
>>  drivers/gpu/drm/i915/intel_drv.h |  2 +
>>  3 files changed, 159 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
>> b/drivers/gpu/drm/i915/intel_ddi.c
>> index 632044a..43efc26 100644
>> --- a/drivers/gpu/drm/i915/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
>> @@ -3274,6 +3274,87 @@ intel_ddi_init_hdmi_connector(struct intel_digital_port
>> *intel_dig_port)
>>  	return connector;
>>  }
>>
>> +bool intel_ddi_upfront_link_train(struct intel_dp *intel_dp,
>> +				struct intel_crtc *crtc)
>> +{
>> +	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
>> +	struct intel_connector *connector = intel_dp->attached_connector;
>> +	struct intel_encoder *encoder = connector->encoder;
>> +	struct drm_device *dev = encoder->base.dev;
>> +	struct drm_i915_private *dev_priv = dev->dev_private;
>> +	struct intel_shared_dpll *pll;
>> +	struct drm_crtc *drm_crtc = NULL;
>> +	struct intel_crtc_state *tmp_crtc_config;
>> +	struct intel_dpll_hw_state tmp_dpll_hw_state;
>> +	uint8_t link_bw, lane_count;
>> +
>> +	if (!crtc) {
>> +		drm_crtc = intel_get_unused_crtc(&encoder->base);
>> +		if (!drm_crtc) {
>> +			DRM_ERROR("No crtc for upfront link training\n");
>> +			return false;
>> +		}
>> +		encoder->base.crtc = drm_crtc;
>> +		crtc = to_intel_crtc(drm_crtc);
>> +	}
>> +
>> +	/* Initialize with Max Link rate & lane count supported by panel */
>> +	link_bw =  intel_dp->dpcd[DP_MAX_LINK_RATE];
>> +	lane_count = drm_dp_max_lane_count(intel_dp->dpcd);
>> +
>> +	/* Save the crtc->config */
>> +	tmp_crtc_config = crtc->config;
>
>> +	tmp_dpll_hw_state = crtc->config->dpll_hw_state;
>> +
>> +	/* Select the shared DPLL to use for this port */
>> +	intel_get_ddi_pll(dev_priv, dig_port->port, crtc->config)
>
>This reads the current programmed DPLL from the hardware. Is there any reason we
>can't trust the value that is in crtc->config already? I don't think this code
>would run before reading out and sanitizing the hardware state.

I was not sure of what will be the DPLL attached to crtc->config when the
crtc is found by 'get_unused_crtc' logic. So, we select the DPLL based
on port again..

>
>> ;
>> +	pll = intel_crtc_to_shared_dpll(crtc);
>> +	if (!pll) {
>> +		DRM_ERROR("Could not get shared DPLL\n");
>> +		goto exit;
>> +	}
>> +	DRM_DEBUG_KMS("Using %s for pipe %c\n", pll->name, pipe_name(crtc
>> ->pipe));
>> +
>> +	do {
>> +		crtc->config->port_clock =
>> drm_dp_bw_code_to_link_rate(link_bw);
>> +		crtc->config->lane_count = lane_count;
>> +		if (!intel_ddi_pll_select(crtc, crtc->config, encoder,
>> false))
>
>
>> +			goto exit;
>> +
>> +		pll->config.crtc_mask |= (1 << crtc->pipe);
>> +		pll->config.hw_state = crtc->config->dpll_hw_state;
>> +
>> +		/* Enable PLL followed by port */
>> +		intel_enable_shared_dpll(crtc);
>> +		encoder->pre_enable(encoder);
>
>The pll handling here seems dodgy. Instead of using intel_get_shared_dpll(),

I initially tried this, but intel_get_shared_dpll() uses crtc_state which is not
set (in outside modeset contexts) thus creating crashes. Specifically, 
the call to intel_ddi_get_crtc_new_encoder() for broxton code path.

That’s why I had to create some initial code to not call get_shared_dpll
If we have valid encoder attached. (patches 1/7 and 2/7 of this series)

So, If you have suggestions on how to fix this in a neat way,
I would be happy to try them out.

Thanks,
Durga

>this fiddles with pll internals itself. I think this will work for broxton,
>since it doesn't actually have shared DPLLs (their chosen based on the encoder).
>It might just work for haswell too since the plls used by DP are not shared.
>
>But to do this cleanly we need the DPLL interface to just give us the right pll.
>
>Ander
>
>
>> +
>> +		/* Check if link training passed; if so update DPCD */
>> +		if (intel_dp->train_set_valid)
>> +			intel_dp_update_dpcd_params(intel_dp);
>> +
>> +		/* Disable port followed by PLL for next retry/clean up */
>> +		encoder->post_disable(encoder);
>> +		intel_disable_shared_dpll(crtc);
>> +
>> +	} while (!intel_dp->train_set_valid &&
>> +		!intel_dp_get_link_retry_params(&lane_count, &link_bw));
>> +
>> +	/* Reset pll state as before */
>> +	pll->config.crtc_mask &= ~(1 << crtc->pipe);
>> +	pll->config.hw_state = tmp_dpll_hw_state;
>> +
>> +exit:
>> +	/* Reset local associations made */
>> +	if (drm_crtc)
>> +		encoder->base.crtc = NULL;
>> +	crtc->config = tmp_crtc_config;
>> +
>> +	DRM_DEBUG_KMS("Upfront link train %s: lanes:%d bw:%d\n",
>> +	intel_dp->train_set_valid ? "Passed" : "Failed", lane_count,
>> link_bw);
>> +
>> +	return intel_dp->train_set_valid;
>> +}
>> +
>>  void intel_ddi_init(struct drm_device *dev, enum port port)
>>  {
>>  	struct drm_i915_private *dev_priv = dev->dev_private;
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index d8f7830..47b6266 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -4622,6 +4622,71 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
>>  	intel_dp->has_audio = false;
>>  }
>>
>> +static void intel_dp_upfront_dpms_off(struct drm_connector *connector,
>> +				struct drm_modeset_acquire_ctx *ctx)
>> +{
>> +	struct intel_dp *intel_dp = intel_attached_dp(connector);
>> +	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>> +	struct drm_crtc *crtc = intel_dig_port->base.base.crtc;
>> +	struct intel_load_detect_pipe tmp;
>> +
>> +	if (intel_get_load_detect_pipe(connector, NULL, &tmp, ctx)) {
>> +		crtc->acquire_ctx = ctx;
>> +		tmp.dpms_mode = DRM_MODE_DPMS_OFF;
>> +		intel_release_load_detect_pipe(connector, &tmp, ctx);
>> +	}
>> +}
>> +
>> +static void intel_dp_upfront_dpms_on(struct drm_connector *connector,
>> +				struct drm_modeset_acquire_ctx *ctx)
>> +{
>> +	struct intel_load_detect_pipe tmp;
>> +
>> +	intel_get_load_detect_pipe(connector, NULL, &tmp, ctx);
>> +
>> +	drm_modeset_drop_locks(ctx);
>> +	drm_modeset_acquire_fini(ctx);
>> +}
>> +
>> +static bool intel_dp_upfront_link_train(struct drm_connector *connector)
>> +{
>> +	struct intel_dp *intel_dp = intel_attached_dp(connector);
>> +	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>> +	struct intel_encoder *intel_encoder = &intel_dig_port->base;
>> +	struct drm_device *dev = intel_encoder->base.dev;
>> +	struct drm_crtc *crtc = intel_dig_port->base.base.crtc;
>> +	struct intel_crtc *intel_crtc = crtc ? to_intel_crtc(crtc) : NULL;
>> +	struct drm_modeset_acquire_ctx ctx, *old_ctx = NULL;
>> +	bool ret = true, need_dpms_on = false;
>> +
>> +	if (!IS_BROXTON(dev))
>> +		return true;
>> +	/*
>> +	 * On hotplug cases, we call _upfront_link_train directly.
>> +	 * In connected boot scenarios (boot with {MIPI/eDP} + DP),
>> +	 * BIOS typically brings up DP. Hence, we disable the crtc
>> +	 * to do _upfront_link_training and then re-enable it back.
>> +	 */
>> +	if (crtc && crtc->enabled && intel_crtc->active) {
>> +		DRM_DEBUG_KMS("Disabling pipe %c\n", pipe_name(intel_crtc
>> ->pipe));
>> +		old_ctx = crtc->acquire_ctx;
>> +		drm_modeset_acquire_init(&ctx, 0);
>> +		intel_dp_upfront_dpms_off(connector, &ctx);
>> +		need_dpms_on = true;
>> +	}
>> +
>> +	if (HAS_DDI(dev))
>> +		ret = intel_ddi_upfront_link_train(intel_dp, intel_crtc);
>> +		/* Other platforms upfront link train call goes here..*/
>> +
>> +	if (need_dpms_on) {
>> +		intel_dp_upfront_dpms_on(connector, &ctx);
>> +		crtc->acquire_ctx = old_ctx;
>> +	}
>> +	return ret;
>> +}
>> +
>> +
>>  static enum drm_connector_status
>>  intel_dp_detect(struct drm_connector *connector, bool force)
>>  {
>> @@ -4631,7 +4696,7 @@ intel_dp_detect(struct drm_connector *connector, bool
>> force)
>>  	struct drm_device *dev = connector->dev;
>>  	enum drm_connector_status status;
>>  	enum intel_display_power_domain power_domain;
>> -	bool ret;
>> +	bool ret, do_upfront_link_train;
>>  	u8 sink_irq_vector;
>>
>>  	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
>> @@ -4705,6 +4770,16 @@ intel_dp_detect(struct drm_connector *connector, bool
>> force)
>>  			DRM_DEBUG_DRIVER("CP or sink specific irq
>> unhandled\n");
>>  	}
>>
>> +	/* Do not do upfront link train, if it is a compliance request */
>> +	do_upfront_link_train =
>> +		intel_encoder->type == INTEL_OUTPUT_DISPLAYPORT &&
>> +		intel_dp->compliance_test_type != DP_TEST_LINK_TRAINING;
>> +
>> +	if (do_upfront_link_train) {
>> +		ret = intel_dp_upfront_link_train(connector);
>> +		if (!ret)
>> +			status = connector_status_disconnected;
>> +	}
>>  out:
>>  	intel_display_power_put(to_i915(dev), power_domain);
>>  	return status;
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h
>> b/drivers/gpu/drm/i915/intel_drv.h
>> index 5912955..3cfab20 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -1025,6 +1025,8 @@ void intel_ddi_clock_get(struct intel_encoder *encoder,
>>  			 struct intel_crtc_state *pipe_config);
>>  void intel_ddi_set_vc_payload_alloc(struct drm_crtc *crtc, bool state);
>>  uint32_t ddi_signal_levels(struct intel_dp *intel_dp);
>> +bool intel_ddi_upfront_link_train(struct intel_dp *intel_dp,
>> +				struct intel_crtc *crtc);
>>
>>  /* intel_frontbuffer.c */
>>  void intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 7/7] drm/i915/dp: Enable Upfront link training for typeC DP support on BXT

[Dave Airlie]

[-- Attachment #1.1: Type: text/plain, Size: 10698 bytes --]

On 11 Dec 2015 7:09 PM, "Durgadoss R" <durgadoss.r@intel.com> wrote:
>
> To support USB type C alternate DP mode, the display driver needs to
> know the number of lanes required by the DP panel as well as number
> of lanes that can be supported by the type-C cable. Sometimes, the
> type-C cable may limit the bandwidth even if Panel can support
> more lanes. To address these scenarios, the display driver will
> start link training with max lanes, and if that fails, the driver
> falls back to x2 lanes; and repeats this procedure for all
> bandwidth/lane configurations.
>
> * Since link training is done before modeset only the port
>   (and not pipe/planes) and its associated PLLs are enabled.
> * On DP hotplug: Directly start link training on the crtc
>   associated with the DP encoder.
> * On Connected boot scenarios: When booted with an LFP and a DP,
>   typically, BIOS brings up DP. In these cases, we disable the
>   crtc, do upfront link training and then re-enable it back.
> * All local changes made for upfront link training are reset
>   to their previous values once it is done; so that the
>   subsequent modeset is not aware of these changes.

This is just an aside but do we know if other OSes just always link train
at plug time.

I do wonder if we should make an OS level decision to require driver do
this always.

Dave.

>
> Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 81
++++++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_dp.c  | 77
+++++++++++++++++++++++++++++++++++++-
>  drivers/gpu/drm/i915/intel_drv.h |  2 +
>  3 files changed, 159 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
b/drivers/gpu/drm/i915/intel_ddi.c
> index 632044a..43efc26 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -3274,6 +3274,87 @@ intel_ddi_init_hdmi_connector(struct
intel_digital_port *intel_dig_port)
>         return connector;
>  }
>
> +bool intel_ddi_upfront_link_train(struct intel_dp *intel_dp,
> +                               struct intel_crtc *crtc)
> +{
> +       struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> +       struct intel_connector *connector = intel_dp->attached_connector;
> +       struct intel_encoder *encoder = connector->encoder;
> +       struct drm_device *dev = encoder->base.dev;
> +       struct drm_i915_private *dev_priv = dev->dev_private;
> +       struct intel_shared_dpll *pll;
> +       struct drm_crtc *drm_crtc = NULL;
> +       struct intel_crtc_state *tmp_crtc_config;
> +       struct intel_dpll_hw_state tmp_dpll_hw_state;
> +       uint8_t link_bw, lane_count;
> +
> +       if (!crtc) {
> +               drm_crtc = intel_get_unused_crtc(&encoder->base);
> +               if (!drm_crtc) {
> +                       DRM_ERROR("No crtc for upfront link training\n");
> +                       return false;
> +               }
> +               encoder->base.crtc = drm_crtc;
> +               crtc = to_intel_crtc(drm_crtc);
> +       }
> +
> +       /* Initialize with Max Link rate & lane count supported by panel
*/
> +       link_bw =  intel_dp->dpcd[DP_MAX_LINK_RATE];
> +       lane_count = drm_dp_max_lane_count(intel_dp->dpcd);
> +
> +       /* Save the crtc->config */
> +       tmp_crtc_config = crtc->config;
> +       tmp_dpll_hw_state = crtc->config->dpll_hw_state;
> +
> +       /* Select the shared DPLL to use for this port */
> +       intel_get_ddi_pll(dev_priv, dig_port->port, crtc->config);
> +       pll = intel_crtc_to_shared_dpll(crtc);
> +       if (!pll) {
> +               DRM_ERROR("Could not get shared DPLL\n");
> +               goto exit;
> +       }
> +       DRM_DEBUG_KMS("Using %s for pipe %c\n", pll->name,
pipe_name(crtc->pipe));
> +
> +       do {
> +               crtc->config->port_clock =
drm_dp_bw_code_to_link_rate(link_bw);
> +               crtc->config->lane_count = lane_count;
> +               if (!intel_ddi_pll_select(crtc, crtc->config, encoder,
false))
> +                       goto exit;
> +
> +               pll->config.crtc_mask |= (1 << crtc->pipe);
> +               pll->config.hw_state = crtc->config->dpll_hw_state;
> +
> +               /* Enable PLL followed by port */
> +               intel_enable_shared_dpll(crtc);
> +               encoder->pre_enable(encoder);
> +
> +               /* Check if link training passed; if so update DPCD */
> +               if (intel_dp->train_set_valid)
> +                       intel_dp_update_dpcd_params(intel_dp);
> +
> +               /* Disable port followed by PLL for next retry/clean up */
> +               encoder->post_disable(encoder);
> +               intel_disable_shared_dpll(crtc);
> +
> +       } while (!intel_dp->train_set_valid &&
> +               !intel_dp_get_link_retry_params(&lane_count, &link_bw));
> +
> +       /* Reset pll state as before */
> +       pll->config.crtc_mask &= ~(1 << crtc->pipe);
> +       pll->config.hw_state = tmp_dpll_hw_state;
> +
> +exit:
> +       /* Reset local associations made */
> +       if (drm_crtc)
> +               encoder->base.crtc = NULL;
> +       crtc->config = tmp_crtc_config;
> +
> +       DRM_DEBUG_KMS("Upfront link train %s: lanes:%d bw:%d\n",
> +       intel_dp->train_set_valid ? "Passed" : "Failed", lane_count,
link_bw);
> +
> +       return intel_dp->train_set_valid;
> +}
> +
>  void intel_ddi_init(struct drm_device *dev, enum port port)
>  {
>         struct drm_i915_private *dev_priv = dev->dev_private;
> diff --git a/drivers/gpu/drm/i915/intel_dp.c
b/drivers/gpu/drm/i915/intel_dp.c
> index d8f7830..47b6266 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4622,6 +4622,71 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
>         intel_dp->has_audio = false;
>  }
>
> +static void intel_dp_upfront_dpms_off(struct drm_connector *connector,
> +                               struct drm_modeset_acquire_ctx *ctx)
> +{
> +       struct intel_dp *intel_dp = intel_attached_dp(connector);
> +       struct intel_digital_port *intel_dig_port =
dp_to_dig_port(intel_dp);
> +       struct drm_crtc *crtc = intel_dig_port->base.base.crtc;
> +       struct intel_load_detect_pipe tmp;
> +
> +       if (intel_get_load_detect_pipe(connector, NULL, &tmp, ctx)) {
> +               crtc->acquire_ctx = ctx;
> +               tmp.dpms_mode = DRM_MODE_DPMS_OFF;
> +               intel_release_load_detect_pipe(connector, &tmp, ctx);
> +       }
> +}
> +
> +static void intel_dp_upfront_dpms_on(struct drm_connector *connector,
> +                               struct drm_modeset_acquire_ctx *ctx)
> +{
> +       struct intel_load_detect_pipe tmp;
> +
> +       intel_get_load_detect_pipe(connector, NULL, &tmp, ctx);
> +
> +       drm_modeset_drop_locks(ctx);
> +       drm_modeset_acquire_fini(ctx);
> +}
> +
> +static bool intel_dp_upfront_link_train(struct drm_connector *connector)
> +{
> +       struct intel_dp *intel_dp = intel_attached_dp(connector);
> +       struct intel_digital_port *intel_dig_port =
dp_to_dig_port(intel_dp);
> +       struct intel_encoder *intel_encoder = &intel_dig_port->base;
> +       struct drm_device *dev = intel_encoder->base.dev;
> +       struct drm_crtc *crtc = intel_dig_port->base.base.crtc;
> +       struct intel_crtc *intel_crtc = crtc ? to_intel_crtc(crtc) : NULL;
> +       struct drm_modeset_acquire_ctx ctx, *old_ctx = NULL;
> +       bool ret = true, need_dpms_on = false;
> +
> +       if (!IS_BROXTON(dev))
> +               return true;
> +       /*
> +        * On hotplug cases, we call _upfront_link_train directly.
> +        * In connected boot scenarios (boot with {MIPI/eDP} + DP),
> +        * BIOS typically brings up DP. Hence, we disable the crtc
> +        * to do _upfront_link_training and then re-enable it back.
> +        */
> +       if (crtc && crtc->enabled && intel_crtc->active) {
> +               DRM_DEBUG_KMS("Disabling pipe %c\n",
pipe_name(intel_crtc->pipe));
> +               old_ctx = crtc->acquire_ctx;
> +               drm_modeset_acquire_init(&ctx, 0);
> +               intel_dp_upfront_dpms_off(connector, &ctx);
> +               need_dpms_on = true;
> +       }
> +
> +       if (HAS_DDI(dev))
> +               ret = intel_ddi_upfront_link_train(intel_dp, intel_crtc);
> +               /* Other platforms upfront link train call goes here..*/
> +
> +       if (need_dpms_on) {
> +               intel_dp_upfront_dpms_on(connector, &ctx);
> +               crtc->acquire_ctx = old_ctx;
> +       }
> +       return ret;
> +}
> +
> +
>  static enum drm_connector_status
>  intel_dp_detect(struct drm_connector *connector, bool force)
>  {
> @@ -4631,7 +4696,7 @@ intel_dp_detect(struct drm_connector *connector,
bool force)
>         struct drm_device *dev = connector->dev;
>         enum drm_connector_status status;
>         enum intel_display_power_domain power_domain;
> -       bool ret;
> +       bool ret, do_upfront_link_train;
>         u8 sink_irq_vector;
>
>         DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
> @@ -4705,6 +4770,16 @@ intel_dp_detect(struct drm_connector *connector,
bool force)
>                         DRM_DEBUG_DRIVER("CP or sink specific irq
unhandled\n");
>         }
>
> +       /* Do not do upfront link train, if it is a compliance request */
> +       do_upfront_link_train =
> +               intel_encoder->type == INTEL_OUTPUT_DISPLAYPORT &&
> +               intel_dp->compliance_test_type != DP_TEST_LINK_TRAINING;
> +
> +       if (do_upfront_link_train) {
> +               ret = intel_dp_upfront_link_train(connector);
> +               if (!ret)
> +                       status = connector_status_disconnected;
> +       }
>  out:
>         intel_display_power_put(to_i915(dev), power_domain);
>         return status;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
b/drivers/gpu/drm/i915/intel_drv.h
> index 5912955..3cfab20 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1025,6 +1025,8 @@ void intel_ddi_clock_get(struct intel_encoder
*encoder,
>                          struct intel_crtc_state *pipe_config);
>  void intel_ddi_set_vc_payload_alloc(struct drm_crtc *crtc, bool state);
>  uint32_t ddi_signal_levels(struct intel_dp *intel_dp);
> +bool intel_ddi_upfront_link_train(struct intel_dp *intel_dp,
> +                               struct intel_crtc *crtc);
>
>  /* intel_frontbuffer.c */
>  void intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[-- Attachment #1.2: Type: text/html, Size: 14114 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [PATCH 7/7] drm/i915/dp: Enable Upfront link training for typeC DP support on BXT

[Jim Bride]

On Wed, Dec 30, 2015 at 09:48:43AM +1000, Dave Airlie wrote:
> On 11 Dec 2015 7:09 PM, "Durgadoss R" <durgadoss.r@intel.com> wrote:
> >
> > To support USB type C alternate DP mode, the display driver needs to
> > know the number of lanes required by the DP panel as well as number
> > of lanes that can be supported by the type-C cable. Sometimes, the
> > type-C cable may limit the bandwidth even if Panel can support
> > more lanes. To address these scenarios, the display driver will
> > start link training with max lanes, and if that fails, the driver
> > falls back to x2 lanes; and repeats this procedure for all
> > bandwidth/lane configurations.
> >
> > * Since link training is done before modeset only the port
> >   (and not pipe/planes) and its associated PLLs are enabled.
> > * On DP hotplug: Directly start link training on the crtc
> >   associated with the DP encoder.
> > * On Connected boot scenarios: When booted with an LFP and a DP,
> >   typically, BIOS brings up DP. In these cases, we disable the
> >   crtc, do upfront link training and then re-enable it back.
> > * All local changes made for upfront link training are reset
> >   to their previous values once it is done; so that the
> >   subsequent modeset is not aware of these changes.
> 
> This is just an aside but do we know if other OSes just always link train
> at plug time.
> 
> I do wonder if we should make an OS level decision to require driver do
> this always.

Based on conversations I've had with folks in VPG, Windows appears to do
upfront link training at plug time.

Jim



> Dave.
> 
> >
> > Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_ddi.c | 81
> ++++++++++++++++++++++++++++++++++++++++
> >  drivers/gpu/drm/i915/intel_dp.c  | 77
> +++++++++++++++++++++++++++++++++++++-
> >  drivers/gpu/drm/i915/intel_drv.h |  2 +
> >  3 files changed, 159 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> b/drivers/gpu/drm/i915/intel_ddi.c
> > index 632044a..43efc26 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -3274,6 +3274,87 @@ intel_ddi_init_hdmi_connector(struct
> intel_digital_port *intel_dig_port)
> >         return connector;
> >  }
> >
> > +bool intel_ddi_upfront_link_train(struct intel_dp *intel_dp,
> > +                               struct intel_crtc *crtc)
> > +{
> > +       struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> > +       struct intel_connector *connector = intel_dp->attached_connector;
> > +       struct intel_encoder *encoder = connector->encoder;
> > +       struct drm_device *dev = encoder->base.dev;
> > +       struct drm_i915_private *dev_priv = dev->dev_private;
> > +       struct intel_shared_dpll *pll;
> > +       struct drm_crtc *drm_crtc = NULL;
> > +       struct intel_crtc_state *tmp_crtc_config;
> > +       struct intel_dpll_hw_state tmp_dpll_hw_state;
> > +       uint8_t link_bw, lane_count;
> > +
> > +       if (!crtc) {
> > +               drm_crtc = intel_get_unused_crtc(&encoder->base);
> > +               if (!drm_crtc) {
> > +                       DRM_ERROR("No crtc for upfront link training\n");
> > +                       return false;
> > +               }
> > +               encoder->base.crtc = drm_crtc;
> > +               crtc = to_intel_crtc(drm_crtc);
> > +       }
> > +
> > +       /* Initialize with Max Link rate & lane count supported by panel
> */
> > +       link_bw =  intel_dp->dpcd[DP_MAX_LINK_RATE];
> > +       lane_count = drm_dp_max_lane_count(intel_dp->dpcd);
> > +
> > +       /* Save the crtc->config */
> > +       tmp_crtc_config = crtc->config;
> > +       tmp_dpll_hw_state = crtc->config->dpll_hw_state;
> > +
> > +       /* Select the shared DPLL to use for this port */
> > +       intel_get_ddi_pll(dev_priv, dig_port->port, crtc->config);
> > +       pll = intel_crtc_to_shared_dpll(crtc);
> > +       if (!pll) {
> > +               DRM_ERROR("Could not get shared DPLL\n");
> > +               goto exit;
> > +       }
> > +       DRM_DEBUG_KMS("Using %s for pipe %c\n", pll->name,
> pipe_name(crtc->pipe));
> > +
> > +       do {
> > +               crtc->config->port_clock =
> drm_dp_bw_code_to_link_rate(link_bw);
> > +               crtc->config->lane_count = lane_count;
> > +               if (!intel_ddi_pll_select(crtc, crtc->config, encoder,
> false))
> > +                       goto exit;
> > +
> > +               pll->config.crtc_mask |= (1 << crtc->pipe);
> > +               pll->config.hw_state = crtc->config->dpll_hw_state;
> > +
> > +               /* Enable PLL followed by port */
> > +               intel_enable_shared_dpll(crtc);
> > +               encoder->pre_enable(encoder);
> > +
> > +               /* Check if link training passed; if so update DPCD */
> > +               if (intel_dp->train_set_valid)
> > +                       intel_dp_update_dpcd_params(intel_dp);
> > +
> > +               /* Disable port followed by PLL for next retry/clean up */
> > +               encoder->post_disable(encoder);
> > +               intel_disable_shared_dpll(crtc);
> > +
> > +       } while (!intel_dp->train_set_valid &&
> > +               !intel_dp_get_link_retry_params(&lane_count, &link_bw));
> > +
> > +       /* Reset pll state as before */
> > +       pll->config.crtc_mask &= ~(1 << crtc->pipe);
> > +       pll->config.hw_state = tmp_dpll_hw_state;
> > +
> > +exit:
> > +       /* Reset local associations made */
> > +       if (drm_crtc)
> > +               encoder->base.crtc = NULL;
> > +       crtc->config = tmp_crtc_config;
> > +
> > +       DRM_DEBUG_KMS("Upfront link train %s: lanes:%d bw:%d\n",
> > +       intel_dp->train_set_valid ? "Passed" : "Failed", lane_count,
> link_bw);
> > +
> > +       return intel_dp->train_set_valid;
> > +}
> > +
> >  void intel_ddi_init(struct drm_device *dev, enum port port)
> >  {
> >         struct drm_i915_private *dev_priv = dev->dev_private;
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> b/drivers/gpu/drm/i915/intel_dp.c
> > index d8f7830..47b6266 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -4622,6 +4622,71 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
> >         intel_dp->has_audio = false;
> >  }
> >
> > +static void intel_dp_upfront_dpms_off(struct drm_connector *connector,
> > +                               struct drm_modeset_acquire_ctx *ctx)
> > +{
> > +       struct intel_dp *intel_dp = intel_attached_dp(connector);
> > +       struct intel_digital_port *intel_dig_port =
> dp_to_dig_port(intel_dp);
> > +       struct drm_crtc *crtc = intel_dig_port->base.base.crtc;
> > +       struct intel_load_detect_pipe tmp;
> > +
> > +       if (intel_get_load_detect_pipe(connector, NULL, &tmp, ctx)) {
> > +               crtc->acquire_ctx = ctx;
> > +               tmp.dpms_mode = DRM_MODE_DPMS_OFF;
> > +               intel_release_load_detect_pipe(connector, &tmp, ctx);
> > +       }
> > +}
> > +
> > +static void intel_dp_upfront_dpms_on(struct drm_connector *connector,
> > +                               struct drm_modeset_acquire_ctx *ctx)
> > +{
> > +       struct intel_load_detect_pipe tmp;
> > +
> > +       intel_get_load_detect_pipe(connector, NULL, &tmp, ctx);
> > +
> > +       drm_modeset_drop_locks(ctx);
> > +       drm_modeset_acquire_fini(ctx);
> > +}
> > +
> > +static bool intel_dp_upfront_link_train(struct drm_connector *connector)
> > +{
> > +       struct intel_dp *intel_dp = intel_attached_dp(connector);
> > +       struct intel_digital_port *intel_dig_port =
> dp_to_dig_port(intel_dp);
> > +       struct intel_encoder *intel_encoder = &intel_dig_port->base;
> > +       struct drm_device *dev = intel_encoder->base.dev;
> > +       struct drm_crtc *crtc = intel_dig_port->base.base.crtc;
> > +       struct intel_crtc *intel_crtc = crtc ? to_intel_crtc(crtc) : NULL;
> > +       struct drm_modeset_acquire_ctx ctx, *old_ctx = NULL;
> > +       bool ret = true, need_dpms_on = false;
> > +
> > +       if (!IS_BROXTON(dev))
> > +               return true;
> > +       /*
> > +        * On hotplug cases, we call _upfront_link_train directly.
> > +        * In connected boot scenarios (boot with {MIPI/eDP} + DP),
> > +        * BIOS typically brings up DP. Hence, we disable the crtc
> > +        * to do _upfront_link_training and then re-enable it back.
> > +        */
> > +       if (crtc && crtc->enabled && intel_crtc->active) {
> > +               DRM_DEBUG_KMS("Disabling pipe %c\n",
> pipe_name(intel_crtc->pipe));
> > +               old_ctx = crtc->acquire_ctx;
> > +               drm_modeset_acquire_init(&ctx, 0);
> > +               intel_dp_upfront_dpms_off(connector, &ctx);
> > +               need_dpms_on = true;
> > +       }
> > +
> > +       if (HAS_DDI(dev))
> > +               ret = intel_ddi_upfront_link_train(intel_dp, intel_crtc);
> > +               /* Other platforms upfront link train call goes here..*/
> > +
> > +       if (need_dpms_on) {
> > +               intel_dp_upfront_dpms_on(connector, &ctx);
> > +               crtc->acquire_ctx = old_ctx;
> > +       }
> > +       return ret;
> > +}
> > +
> > +
> >  static enum drm_connector_status
> >  intel_dp_detect(struct drm_connector *connector, bool force)
> >  {
> > @@ -4631,7 +4696,7 @@ intel_dp_detect(struct drm_connector *connector,
> bool force)
> >         struct drm_device *dev = connector->dev;
> >         enum drm_connector_status status;
> >         enum intel_display_power_domain power_domain;
> > -       bool ret;
> > +       bool ret, do_upfront_link_train;
> >         u8 sink_irq_vector;
> >
> >         DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
> > @@ -4705,6 +4770,16 @@ intel_dp_detect(struct drm_connector *connector,
> bool force)
> >                         DRM_DEBUG_DRIVER("CP or sink specific irq
> unhandled\n");
> >         }
> >
> > +       /* Do not do upfront link train, if it is a compliance request */
> > +       do_upfront_link_train =
> > +               intel_encoder->type == INTEL_OUTPUT_DISPLAYPORT &&
> > +               intel_dp->compliance_test_type != DP_TEST_LINK_TRAINING;
> > +
> > +       if (do_upfront_link_train) {
> > +               ret = intel_dp_upfront_link_train(connector);
> > +               if (!ret)
> > +                       status = connector_status_disconnected;
> > +       }
> >  out:
> >         intel_display_power_put(to_i915(dev), power_domain);
> >         return status;
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> > index 5912955..3cfab20 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1025,6 +1025,8 @@ void intel_ddi_clock_get(struct intel_encoder
> *encoder,
> >                          struct intel_crtc_state *pipe_config);
> >  void intel_ddi_set_vc_payload_alloc(struct drm_crtc *crtc, bool state);
> >  uint32_t ddi_signal_levels(struct intel_dp *intel_dp);
> > +bool intel_ddi_upfront_link_train(struct intel_dp *intel_dp,
> > +                               struct intel_crtc *crtc);
> >
> >  /* intel_frontbuffer.c */
> >  void intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
> > --
> > 1.9.1
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

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

Re: [PATCH 7/7] drm/i915/dp: Enable Upfront link training for typeC DP support on BXT

[Ander Conselvan De Oliveira]

On Tue, 2015-12-29 at 18:50 +0000, R, Durgadoss wrote:
> Hi Ander,
> 
> Thanks for looking into this..
> 
> > -----Original Message-----
> > From: Ander Conselvan De Oliveira [mailto:conselvan2@gmail.com]
> > Sent: Tuesday, December 29, 2015 10:52 PM
> > To: R, Durgadoss; intel-gfx@lists.freedesktop.org
> > Subject: Re: [PATCH 7/7] drm/i915/dp: Enable Upfront link training for typeC
> > DP support on BXT
> > 
> > On Fri, 2015-12-11 at 15:09 +0530, Durgadoss R wrote:
> > > To support USB type C alternate DP mode, the display driver needs to
> > > know the number of lanes required by the DP panel as well as number
> > > of lanes that can be supported by the type-C cable. Sometimes, the
> > > type-C cable may limit the bandwidth even if Panel can support
> > > more lanes. To address these scenarios, the display driver will
> > > start link training with max lanes, and if that fails, the driver
> > > falls back to x2 lanes; and repeats this procedure for all
> > > bandwidth/lane configurations.
> > > 
> > > * Since link training is done before modeset only the port
> > >   (and not pipe/planes) and its associated PLLs are enabled.
> > > * On DP hotplug: Directly start link training on the crtc
> > >   associated with the DP encoder.
> > > * On Connected boot scenarios: When booted with an LFP and a DP,
> > >   typically, BIOS brings up DP. In these cases, we disable the
> > >   crtc, do upfront link training and then re-enable it back.
> > > * All local changes made for upfront link training are reset
> > >   to their previous values once it is done; so that the
> > >   subsequent modeset is not aware of these changes.
> > > 
> > > Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_ddi.c | 81
> > > ++++++++++++++++++++++++++++++++++++++++
> > >  drivers/gpu/drm/i915/intel_dp.c  | 77
> > > +++++++++++++++++++++++++++++++++++++-
> > >  drivers/gpu/drm/i915/intel_drv.h |  2 +
> > >  3 files changed, 159 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> > > b/drivers/gpu/drm/i915/intel_ddi.c
> > > index 632044a..43efc26 100644
> > > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > > @@ -3274,6 +3274,87 @@ intel_ddi_init_hdmi_connector(struct
> > > intel_digital_port
> > > *intel_dig_port)
> > >  	return connector;
> > >  }
> > > 
> > > +bool intel_ddi_upfront_link_train(struct intel_dp *intel_dp,
> > > +				struct intel_crtc *crtc)
> > > +{
> > > +	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> > > +	struct intel_connector *connector = intel_dp->attached_connector;
> > > +	struct intel_encoder *encoder = connector->encoder;
> > > +	struct drm_device *dev = encoder->base.dev;
> > > +	struct drm_i915_private *dev_priv = dev->dev_private;
> > > +	struct intel_shared_dpll *pll;
> > > +	struct drm_crtc *drm_crtc = NULL;
> > > +	struct intel_crtc_state *tmp_crtc_config;
> > > +	struct intel_dpll_hw_state tmp_dpll_hw_state;
> > > +	uint8_t link_bw, lane_count;
> > > +
> > > +	if (!crtc) {
> > > +		drm_crtc = intel_get_unused_crtc(&encoder->base);
> > > +		if (!drm_crtc) {
> > > +			DRM_ERROR("No crtc for upfront link training\n");
> > > +			return false;
> > > +		}
> > > +		encoder->base.crtc = drm_crtc;
> > > +		crtc = to_intel_crtc(drm_crtc);
> > > +	}
> > > +
> > > +	/* Initialize with Max Link rate & lane count supported by panel
> > > */
> > > +	link_bw =  intel_dp->dpcd[DP_MAX_LINK_RATE];
> > > +	lane_count = drm_dp_max_lane_count(intel_dp->dpcd);
> > > +
> > > +	/* Save the crtc->config */
> > > +	tmp_crtc_config = crtc->config;
> > 
> > > +	tmp_dpll_hw_state = crtc->config->dpll_hw_state;
> > > +
> > > +	/* Select the shared DPLL to use for this port */
> > > +	intel_get_ddi_pll(dev_priv, dig_port->port, crtc->config)
> > 
> > This reads the current programmed DPLL from the hardware. Is there any
> > reason we
> > can't trust the value that is in crtc->config already? I don't think this
> > code
> > would run before reading out and sanitizing the hardware state.
> 
> I was not sure of what will be the DPLL attached to crtc->config when the
> crtc is found by 'get_unused_crtc' logic. So, we select the DPLL based
> on port again..
> 
> > 
> > > ;
> > > +	pll = intel_crtc_to_shared_dpll(crtc);
> > > +	if (!pll) {
> > > +		DRM_ERROR("Could not get shared DPLL\n");
> > > +		goto exit;
> > > +	}
> > > +	DRM_DEBUG_KMS("Using %s for pipe %c\n", pll->name, pipe_name(crtc
> > > ->pipe));
> > > +
> > > +	do {
> > > +		crtc->config->port_clock =
> > > drm_dp_bw_code_to_link_rate(link_bw);
> > > +		crtc->config->lane_count = lane_count;
> > > +		if (!intel_ddi_pll_select(crtc, crtc->config, encoder,
> > > false))
> > 
> > 
> > > +			goto exit;
> > > +
> > > +		pll->config.crtc_mask |= (1 << crtc->pipe);
> > > +		pll->config.hw_state = crtc->config->dpll_hw_state;
> > > +
> > > +		/* Enable PLL followed by port */
> > > +		intel_enable_shared_dpll(crtc);
> > > +		encoder->pre_enable(encoder);
> > 
> > The pll handling here seems dodgy. Instead of using intel_get_shared_dpll(),
> 
> I initially tried this, but intel_get_shared_dpll() uses crtc_state which is
> not
> set (in outside modeset contexts) thus creating crashes. Specifically, 
> the call to intel_ddi_get_crtc_new_encoder() for broxton code path.
> 
> That’s why I had to create some initial code to not call get_shared_dpll
> If we have valid encoder attached. (patches 1/7 and 2/7 of this series)
> 
> So, If you have suggestions on how to fix this in a neat way,
> I would be happy to try them out.

One idea would be split the part of intel_get_shared_dpll() that needs atomic
interfaces to a separate function. Then it would be possible to add a second
entry point that just takes crtc, encoder and the pll parameters and calls the
logic that doesn't depend on the atomic state.

Then you could move the calls to intel_get_shared_dpll() from *_ddi_pll_select()
to haswell_crtc_compute_clock() so they don't get on the way during upfront link
training, where the new non-atomic entry point is called instead.

Ander

> 
> Thanks,
> Durga
> 
> > this fiddles with pll internals itself. I think this will work for broxton,
> > since it doesn't actually have shared DPLLs (their chosen based on the
> > encoder).
> > It might just work for haswell too since the plls used by DP are not shared.
> > 
> > But to do this cleanly we need the DPLL interface to just give us the right
> > pll.
> > 
> > Ander
> > 
> > 
> > > +
> > > +		/* Check if link training passed; if so update DPCD */
> > > +		if (intel_dp->train_set_valid)
> > > +			intel_dp_update_dpcd_params(intel_dp);
> > > +
> > > +		/* Disable port followed by PLL for next retry/clean up
> > > */
> > > +		encoder->post_disable(encoder);
> > > +		intel_disable_shared_dpll(crtc);
> > > +
> > > +	} while (!intel_dp->train_set_valid &&
> > > +		!intel_dp_get_link_retry_params(&lane_count, &link_bw));
> > > +
> > > +	/* Reset pll state as before */
> > > +	pll->config.crtc_mask &= ~(1 << crtc->pipe);
> > > +	pll->config.hw_state = tmp_dpll_hw_state;
> > > +
> > > +exit:
> > > +	/* Reset local associations made */
> > > +	if (drm_crtc)
> > > +		encoder->base.crtc = NULL;
> > > +	crtc->config = tmp_crtc_config;
> > > +
> > > +	DRM_DEBUG_KMS("Upfront link train %s: lanes:%d bw:%d\n",
> > > +	intel_dp->train_set_valid ? "Passed" : "Failed", lane_count,
> > > link_bw);
> > > +
> > > +	return intel_dp->train_set_valid;
> > > +}
> > > +
> > >  void intel_ddi_init(struct drm_device *dev, enum port port)
> > >  {
> > >  	struct drm_i915_private *dev_priv = dev->dev_private;
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > b/drivers/gpu/drm/i915/intel_dp.c
> > > index d8f7830..47b6266 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -4622,6 +4622,71 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
> > >  	intel_dp->has_audio = false;
> > >  }
> > > 
> > > +static void intel_dp_upfront_dpms_off(struct drm_connector *connector,
> > > +				struct drm_modeset_acquire_ctx *ctx)
> > > +{
> > > +	struct intel_dp *intel_dp = intel_attached_dp(connector);
> > > +	struct intel_digital_port *intel_dig_port =
> > > dp_to_dig_port(intel_dp);
> > > +	struct drm_crtc *crtc = intel_dig_port->base.base.crtc;
> > > +	struct intel_load_detect_pipe tmp;
> > > +
> > > +	if (intel_get_load_detect_pipe(connector, NULL, &tmp, ctx)) {
> > > +		crtc->acquire_ctx = ctx;
> > > +		tmp.dpms_mode = DRM_MODE_DPMS_OFF;
> > > +		intel_release_load_detect_pipe(connector, &tmp, ctx);
> > > +	}
> > > +}
> > > +
> > > +static void intel_dp_upfront_dpms_on(struct drm_connector *connector,
> > > +				struct drm_modeset_acquire_ctx *ctx)
> > > +{
> > > +	struct intel_load_detect_pipe tmp;
> > > +
> > > +	intel_get_load_detect_pipe(connector, NULL, &tmp, ctx);
> > > +
> > > +	drm_modeset_drop_locks(ctx);
> > > +	drm_modeset_acquire_fini(ctx);
> > > +}
> > > +
> > > +static bool intel_dp_upfront_link_train(struct drm_connector *connector)
> > > +{
> > > +	struct intel_dp *intel_dp = intel_attached_dp(connector);
> > > +	struct intel_digital_port *intel_dig_port =
> > > dp_to_dig_port(intel_dp);
> > > +	struct intel_encoder *intel_encoder = &intel_dig_port->base;
> > > +	struct drm_device *dev = intel_encoder->base.dev;
> > > +	struct drm_crtc *crtc = intel_dig_port->base.base.crtc;
> > > +	struct intel_crtc *intel_crtc = crtc ? to_intel_crtc(crtc) :
> > > NULL;
> > > +	struct drm_modeset_acquire_ctx ctx, *old_ctx = NULL;
> > > +	bool ret = true, need_dpms_on = false;
> > > +
> > > +	if (!IS_BROXTON(dev))
> > > +		return true;
> > > +	/*
> > > +	 * On hotplug cases, we call _upfront_link_train directly.
> > > +	 * In connected boot scenarios (boot with {MIPI/eDP} + DP),
> > > +	 * BIOS typically brings up DP. Hence, we disable the crtc
> > > +	 * to do _upfront_link_training and then re-enable it back.
> > > +	 */
> > > +	if (crtc && crtc->enabled && intel_crtc->active) {
> > > +		DRM_DEBUG_KMS("Disabling pipe %c\n", pipe_name(intel_crtc
> > > ->pipe));
> > > +		old_ctx = crtc->acquire_ctx;
> > > +		drm_modeset_acquire_init(&ctx, 0);
> > > +		intel_dp_upfront_dpms_off(connector, &ctx);
> > > +		need_dpms_on = true;
> > > +	}
> > > +
> > > +	if (HAS_DDI(dev))
> > > +		ret = intel_ddi_upfront_link_train(intel_dp, intel_crtc);
> > > +		/* Other platforms upfront link train call goes here..*/
> > > +
> > > +	if (need_dpms_on) {
> > > +		intel_dp_upfront_dpms_on(connector, &ctx);
> > > +		crtc->acquire_ctx = old_ctx;
> > > +	}
> > > +	return ret;
> > > +}
> > > +
> > > +
> > >  static enum drm_connector_status
> > >  intel_dp_detect(struct drm_connector *connector, bool force)
> > >  {
> > > @@ -4631,7 +4696,7 @@ intel_dp_detect(struct drm_connector *connector,
> > > bool
> > > force)
> > >  	struct drm_device *dev = connector->dev;
> > >  	enum drm_connector_status status;
> > >  	enum intel_display_power_domain power_domain;
> > > -	bool ret;
> > > +	bool ret, do_upfront_link_train;
> > >  	u8 sink_irq_vector;
> > > 
> > >  	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
> > > @@ -4705,6 +4770,16 @@ intel_dp_detect(struct drm_connector *connector,
> > > bool
> > > force)
> > >  			DRM_DEBUG_DRIVER("CP or sink specific irq
> > > unhandled\n");
> > >  	}
> > > 
> > > +	/* Do not do upfront link train, if it is a compliance request */
> > > +	do_upfront_link_train =
> > > +		intel_encoder->type == INTEL_OUTPUT_DISPLAYPORT &&
> > > +		intel_dp->compliance_test_type != DP_TEST_LINK_TRAINING;
> > > +
> > > +	if (do_upfront_link_train) {
> > > +		ret = intel_dp_upfront_link_train(connector);
> > > +		if (!ret)
> > > +			status = connector_status_disconnected;
> > > +	}
> > >  out:
> > >  	intel_display_power_put(to_i915(dev), power_domain);
> > >  	return status;
> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > > b/drivers/gpu/drm/i915/intel_drv.h
> > > index 5912955..3cfab20 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -1025,6 +1025,8 @@ void intel_ddi_clock_get(struct intel_encoder
> > > *encoder,
> > >  			 struct intel_crtc_state *pipe_config);
> > >  void intel_ddi_set_vc_payload_alloc(struct drm_crtc *crtc, bool state);
> > >  uint32_t ddi_signal_levels(struct intel_dp *intel_dp);
> > > +bool intel_ddi_upfront_link_train(struct intel_dp *intel_dp,
> > > +				struct intel_crtc *crtc);
> > > 
> > >  /* intel_frontbuffer.c */
> > >  void intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/7] drm/i915/dp: Add methods to update link train params

[Ander Conselvan De Oliveira]

On Fri, 2015-12-11 at 15:09 +0530, Durgadoss R wrote:
> Retrying with reduced lanes/bw and updating the final
> available lanes/bw to DPCD is needed for upfront link
> train logic. Hence, this patch adds these methods
> and exports them so that these can be called from
> other files like ddi.c/display.c.
> 
> Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c  | 34 ++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h |  2 ++
>  2 files changed, 36 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index f335c92..d8f7830 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1660,6 +1660,40 @@ void intel_dp_set_link_params(struct intel_dp
> *intel_dp,
>  	intel_dp->lane_count = pipe_config->lane_count;
>  }
>  
> +void intel_dp_update_dpcd_params(struct intel_dp *intel_dp)
> +{
> +	intel_dp->dpcd[DP_MAX_LANE_COUNT] &= ~DP_MAX_LANE_COUNT_MASK;
> +	intel_dp->dpcd[DP_MAX_LANE_COUNT] |=
> +			intel_dp->lane_count & DP_MAX_LANE_COUNT_MASK;
> +
> +	intel_dp->dpcd[DP_MAX_LINK_RATE] =
> +			drm_dp_link_rate_to_bw_code(intel_dp->link_rate);
> +}
> +
> +bool intel_dp_get_link_retry_params(uint8_t *lane_count, uint8_t *link_bw)
> +{
> +	/*
> +	 * As per DP1.3 Spec, retry all link rates for a particular
> +	 * lane count value, before reducing number of lanes.
> +	 */
> +	if (*link_bw == DP_LINK_BW_5_4) {
> +		*link_bw = DP_LINK_BW_2_7;
> +	} else if (*link_bw == DP_LINK_BW_2_7) {
> +		*link_bw = DP_LINK_BW_1_62;
> +	} else if (*lane_count == 4) {
> +		*lane_count = 2;
> +		*link_bw = DP_LINK_BW_5_4;
> +	} else if (*lane_count == 2) {
> +		*lane_count = 1;
> +		*link_bw = DP_LINK_BW_5_4;

If the sink doesn't support 5.4 Gbps, shouldn't we skip that rate?

Also, it would be easier to review these functions if they were in same patch
were they are used.

Ander

> +	} else {
> +		/* Tried all combinations, so exit */
> +		return false;
> +	}
> +
> +	return true;
> +}
> +
>  static void intel_dp_prepare(struct intel_encoder *encoder)
>  {
>  	struct drm_device *dev = encoder->base.dev;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index faa91fc..5cefb0e 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1233,6 +1233,8 @@ bool intel_dp_init_connector(struct intel_digital_port
> *intel_dig_port,
>  			     struct intel_connector *intel_connector);
>  void intel_dp_set_link_params(struct intel_dp *intel_dp,
>  			      const struct intel_crtc_state *pipe_config);
> +void intel_dp_update_dpcd_params(struct intel_dp *intel_dp);
> +bool intel_dp_get_link_retry_params(uint8_t *lane_count, uint8_t *link_bw);
>  void intel_dp_start_link_train(struct intel_dp *intel_dp);
>  void intel_dp_stop_link_train(struct intel_dp *intel_dp);
>  void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 7/7] drm/i915/dp: Enable Upfront link training for typeC DP support on BXT

[Ander Conselvan De Oliveira]

On Fri, 2015-12-11 at 15:09 +0530, Durgadoss R wrote:
> To support USB type C alternate DP mode, the display driver needs to
> know the number of lanes required by the DP panel as well as number
> of lanes that can be supported by the type-C cable. Sometimes, the
> type-C cable may limit the bandwidth even if Panel can support
> more lanes. To address these scenarios, the display driver will
> start link training with max lanes, and if that fails, the driver
> falls back to x2 lanes; and repeats this procedure for all
> bandwidth/lane configurations.
> 
> * Since link training is done before modeset only the port
>   (and not pipe/planes) and its associated PLLs are enabled.
> * On DP hotplug: Directly start link training on the crtc
>   associated with the DP encoder.
> * On Connected boot scenarios: When booted with an LFP and a DP,
>   typically, BIOS brings up DP. In these cases, we disable the
>   crtc, do upfront link training and then re-enable it back.
> * All local changes made for upfront link training are reset
>   to their previous values once it is done; so that the
>   subsequent modeset is not aware of these changes.
> 
> Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 81
> ++++++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_dp.c  | 77 +++++++++++++++++++++++++++++++++++++-
>  drivers/gpu/drm/i915/intel_drv.h |  2 +
>  3 files changed, 159 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> b/drivers/gpu/drm/i915/intel_ddi.c
> index 632044a..43efc26 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -3274,6 +3274,87 @@ intel_ddi_init_hdmi_connector(struct intel_digital_port
> *intel_dig_port)
>  	return connector;
>  }
>  
> +bool intel_ddi_upfront_link_train(struct intel_dp *intel_dp,
> +				struct intel_crtc *crtc)
> +{
> +	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> +	struct intel_connector *connector = intel_dp->attached_connector;
> +	struct intel_encoder *encoder = connector->encoder;
> +	struct drm_device *dev = encoder->base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_shared_dpll *pll;
> +	struct drm_crtc *drm_crtc = NULL;
> +	struct intel_crtc_state *tmp_crtc_config;
> +	struct intel_dpll_hw_state tmp_dpll_hw_state;
> +	uint8_t link_bw, lane_count;
> +
> +	if (!crtc) {
> +		drm_crtc = intel_get_unused_crtc(&encoder->base);
> +		if (!drm_crtc) {
> +			DRM_ERROR("No crtc for upfront link training\n");
> +			return false;
> +		}
> +		encoder->base.crtc = drm_crtc;
> +		crtc = to_intel_crtc(drm_crtc);
> +	}
> +
> +	/* Initialize with Max Link rate & lane count supported by panel */
> +	link_bw =  intel_dp->dpcd[DP_MAX_LINK_RATE];
> +	lane_count = drm_dp_max_lane_count(intel_dp->dpcd);
> +
> +	/* Save the crtc->config */
> +	tmp_crtc_config = crtc->config;
> +	tmp_dpll_hw_state = crtc->config->dpll_hw_state;
> +
> +	/* Select the shared DPLL to use for this port */
> +	intel_get_ddi_pll(dev_priv, dig_port->port, crtc->config);
> +	pll = intel_crtc_to_shared_dpll(crtc);
> +	if (!pll) {
> +		DRM_ERROR("Could not get shared DPLL\n");
> +		goto exit;
> +	}
> +	DRM_DEBUG_KMS("Using %s for pipe %c\n", pll->name, pipe_name(crtc
> ->pipe));
> +
> +	do {
> +		crtc->config->port_clock =
> drm_dp_bw_code_to_link_rate(link_bw);
> +		crtc->config->lane_count = lane_count;
> +		if (!intel_ddi_pll_select(crtc, crtc->config, encoder,
> false))
> +			goto exit;
> +
> +		pll->config.crtc_mask |= (1 << crtc->pipe);
> +		pll->config.hw_state = crtc->config->dpll_hw_state;
> +
> +		/* Enable PLL followed by port */
> +		intel_enable_shared_dpll(crtc);
> +		encoder->pre_enable(encoder);
> +
> +		/* Check if link training passed; if so update DPCD */
> +		if (intel_dp->train_set_valid)
> +			intel_dp_update_dpcd_params(intel_dp);
> +
> +		/* Disable port followed by PLL for next retry/clean up */
> +		encoder->post_disable(encoder);
> +		intel_disable_shared_dpll(crtc);
> +
> +	} while (!intel_dp->train_set_valid &&
> +		!intel_dp_get_link_retry_params(&lane_count, &link_bw));
> +
> +	/* Reset pll state as before */
> +	pll->config.crtc_mask &= ~(1 << crtc->pipe);
> +	pll->config.hw_state = tmp_dpll_hw_state;
> +
> +exit:
> +	/* Reset local associations made */
> +	if (drm_crtc)
> +		encoder->base.crtc = NULL;
> +	crtc->config = tmp_crtc_config;
> +
> +	DRM_DEBUG_KMS("Upfront link train %s: lanes:%d bw:%d\n",
> +	intel_dp->train_set_valid ? "Passed" : "Failed", lane_count,
> link_bw);
> +
> +	return intel_dp->train_set_valid;
> +}
> +
>  void intel_ddi_init(struct drm_device *dev, enum port port)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index d8f7830..47b6266 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4622,6 +4622,71 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
>  	intel_dp->has_audio = false;
>  }
>  
> +static void intel_dp_upfront_dpms_off(struct drm_connector *connector,
> +				struct drm_modeset_acquire_ctx *ctx)
> +{
> +	struct intel_dp *intel_dp = intel_attached_dp(connector);
> +	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> +	struct drm_crtc *crtc = intel_dig_port->base.base.crtc;
> +	struct intel_load_detect_pipe tmp;
> +
> +	if (intel_get_load_detect_pipe(connector, NULL, &tmp, ctx)) {
> +		crtc->acquire_ctx = ctx;
> +		tmp.dpms_mode = DRM_MODE_DPMS_OFF;
> +		intel_release_load_detect_pipe(connector, &tmp, ctx);
> +	}
> +}
> +
> +static void intel_dp_upfront_dpms_on(struct drm_connector *connector,
> +				struct drm_modeset_acquire_ctx *ctx)
> +{
> +	struct intel_load_detect_pipe tmp;
> +
> +	intel_get_load_detect_pipe(connector, NULL, &tmp, ctx);
> +
> +	drm_modeset_drop_locks(ctx);
> +	drm_modeset_acquire_fini(ctx);
> +}

Are these supposed to disable/enable the crtc so that the upfront link training
can be performed? The load detect pipe really doesn't belong here, its only
purpose is to detect whether there is an output connected in platforms that
didn't support hotplug. You should do a normal atomic operation instead.

Ander

> +
> +static bool intel_dp_upfront_link_train(struct drm_connector *connector)
> +{
> +	struct intel_dp *intel_dp = intel_attached_dp(connector);
> +	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> +	struct intel_encoder *intel_encoder = &intel_dig_port->base;
> +	struct drm_device *dev = intel_encoder->base.dev;
> +	struct drm_crtc *crtc = intel_dig_port->base.base.crtc;
> +	struct intel_crtc *intel_crtc = crtc ? to_intel_crtc(crtc) : NULL;
> +	struct drm_modeset_acquire_ctx ctx, *old_ctx = NULL;
> +	bool ret = true, need_dpms_on = false;
> +
> +	if (!IS_BROXTON(dev))
> +		return true;
> +	/*
> +	 * On hotplug cases, we call _upfront_link_train directly.
> +	 * In connected boot scenarios (boot with {MIPI/eDP} + DP),
> +	 * BIOS typically brings up DP. Hence, we disable the crtc
> +	 * to do _upfront_link_training and then re-enable it back.
> +	 */
> +	if (crtc && crtc->enabled && intel_crtc->active) {
> +		DRM_DEBUG_KMS("Disabling pipe %c\n", pipe_name(intel_crtc
> ->pipe));
> +		old_ctx = crtc->acquire_ctx;
> +		drm_modeset_acquire_init(&ctx, 0);
> +		intel_dp_upfront_dpms_off(connector, &ctx);
> +		need_dpms_on = true;
> +	}
> +
> +	if (HAS_DDI(dev))
> +		ret = intel_ddi_upfront_link_train(intel_dp, intel_crtc);
> +		/* Other platforms upfront link train call goes here..*/
> +
> +	if (need_dpms_on) {
> +		intel_dp_upfront_dpms_on(connector, &ctx);
> +		crtc->acquire_ctx = old_ctx;
> +	}
> +	return ret;
> +}
> +
> +
>  static enum drm_connector_status
>  intel_dp_detect(struct drm_connector *connector, bool force)
>  {
> @@ -4631,7 +4696,7 @@ intel_dp_detect(struct drm_connector *connector, bool
> force)
>  	struct drm_device *dev = connector->dev;
>  	enum drm_connector_status status;
>  	enum intel_display_power_domain power_domain;
> -	bool ret;
> +	bool ret, do_upfront_link_train;
>  	u8 sink_irq_vector;
>  
>  	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
> @@ -4705,6 +4770,16 @@ intel_dp_detect(struct drm_connector *connector, bool
> force)
>  			DRM_DEBUG_DRIVER("CP or sink specific irq
> unhandled\n");
>  	}
>  
> +	/* Do not do upfront link train, if it is a compliance request */
> +	do_upfront_link_train =
> +		intel_encoder->type == INTEL_OUTPUT_DISPLAYPORT &&
> +		intel_dp->compliance_test_type != DP_TEST_LINK_TRAINING;
> +
> +	if (do_upfront_link_train) {
> +		ret = intel_dp_upfront_link_train(connector);
> +		if (!ret)
> +			status = connector_status_disconnected;
> +	}
>  out:
>  	intel_display_power_put(to_i915(dev), power_domain);
>  	return status;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 5912955..3cfab20 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1025,6 +1025,8 @@ void intel_ddi_clock_get(struct intel_encoder *encoder,
>  			 struct intel_crtc_state *pipe_config);
>  void intel_ddi_set_vc_payload_alloc(struct drm_crtc *crtc, bool state);
>  uint32_t ddi_signal_levels(struct intel_dp *intel_dp);
> +bool intel_ddi_upfront_link_train(struct intel_dp *intel_dp,
> +				struct intel_crtc *crtc);
>  
>  /* intel_frontbuffer.c */
>  void intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 7/7] drm/i915/dp: Enable Upfront link training for typeC DP support on BXT

[R, Durgadoss]

>-----Original Message-----
>From: Ander Conselvan De Oliveira [mailto:conselvan2@gmail.com]
>Sent: Monday, January 11, 2016 7:40 PM
>To: R, Durgadoss; intel-gfx@lists.freedesktop.org
>Subject: Re: [PATCH 7/7] drm/i915/dp: Enable Upfront link training for typeC DP support on BXT
>
>On Tue, 2015-12-29 at 18:50 +0000, R, Durgadoss wrote:
>> Hi Ander,
>>
>> Thanks for looking into this..
>>
>> > -----Original Message-----
>> > From: Ander Conselvan De Oliveira [mailto:conselvan2@gmail.com]
>> > Sent: Tuesday, December 29, 2015 10:52 PM
>> > To: R, Durgadoss; intel-gfx@lists.freedesktop.org
>> > Subject: Re: [PATCH 7/7] drm/i915/dp: Enable Upfront link training for typeC
>> > DP support on BXT
>> >
>> > On Fri, 2015-12-11 at 15:09 +0530, Durgadoss R wrote:
>> > > To support USB type C alternate DP mode, the display driver needs to
>> > > know the number of lanes required by the DP panel as well as number
>> > > of lanes that can be supported by the type-C cable. Sometimes, the
>> > > type-C cable may limit the bandwidth even if Panel can support
>> > > more lanes. To address these scenarios, the display driver will
>> > > start link training with max lanes, and if that fails, the driver
>> > > falls back to x2 lanes; and repeats this procedure for all
>> > > bandwidth/lane configurations.
>> > >
>> > > * Since link training is done before modeset only the port
>> > >   (and not pipe/planes) and its associated PLLs are enabled.
>> > > * On DP hotplug: Directly start link training on the crtc
>> > >   associated with the DP encoder.
>> > > * On Connected boot scenarios: When booted with an LFP and a DP,
>> > >   typically, BIOS brings up DP. In these cases, we disable the
>> > >   crtc, do upfront link training and then re-enable it back.
>> > > * All local changes made for upfront link training are reset
>> > >   to their previous values once it is done; so that the
>> > >   subsequent modeset is not aware of these changes.
>> > >
>> > > Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
>> > > ---
>> > >  drivers/gpu/drm/i915/intel_ddi.c | 81
>> > > ++++++++++++++++++++++++++++++++++++++++
>> > >  drivers/gpu/drm/i915/intel_dp.c  | 77
>> > > +++++++++++++++++++++++++++++++++++++-
>> > >  drivers/gpu/drm/i915/intel_drv.h |  2 +
>> > >  3 files changed, 159 insertions(+), 1 deletion(-)
>> > >
>> > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
>> > > b/drivers/gpu/drm/i915/intel_ddi.c
>> > > index 632044a..43efc26 100644
>> > > --- a/drivers/gpu/drm/i915/intel_ddi.c
>> > > +++ b/drivers/gpu/drm/i915/intel_ddi.c
>> > > @@ -3274,6 +3274,87 @@ intel_ddi_init_hdmi_connector(struct
>> > > intel_digital_port
>> > > *intel_dig_port)
>> > >  	return connector;
>> > >  }
>> > >
>> > > +bool intel_ddi_upfront_link_train(struct intel_dp *intel_dp,
>> > > +				struct intel_crtc *crtc)
>> > > +{
>> > > +	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
>> > > +	struct intel_connector *connector = intel_dp->attached_connector;
>> > > +	struct intel_encoder *encoder = connector->encoder;
>> > > +	struct drm_device *dev = encoder->base.dev;
>> > > +	struct drm_i915_private *dev_priv = dev->dev_private;
>> > > +	struct intel_shared_dpll *pll;
>> > > +	struct drm_crtc *drm_crtc = NULL;
>> > > +	struct intel_crtc_state *tmp_crtc_config;
>> > > +	struct intel_dpll_hw_state tmp_dpll_hw_state;
>> > > +	uint8_t link_bw, lane_count;
>> > > +
>> > > +	if (!crtc) {
>> > > +		drm_crtc = intel_get_unused_crtc(&encoder->base);
>> > > +		if (!drm_crtc) {
>> > > +			DRM_ERROR("No crtc for upfront link training\n");
>> > > +			return false;
>> > > +		}
>> > > +		encoder->base.crtc = drm_crtc;
>> > > +		crtc = to_intel_crtc(drm_crtc);
>> > > +	}
>> > > +
>> > > +	/* Initialize with Max Link rate & lane count supported by panel
>> > > */
>> > > +	link_bw =  intel_dp->dpcd[DP_MAX_LINK_RATE];
>> > > +	lane_count = drm_dp_max_lane_count(intel_dp->dpcd);
>> > > +
>> > > +	/* Save the crtc->config */
>> > > +	tmp_crtc_config = crtc->config;
>> >
>> > > +	tmp_dpll_hw_state = crtc->config->dpll_hw_state;
>> > > +
>> > > +	/* Select the shared DPLL to use for this port */
>> > > +	intel_get_ddi_pll(dev_priv, dig_port->port, crtc->config)
>> >
>> > This reads the current programmed DPLL from the hardware. Is there any
>> > reason we
>> > can't trust the value that is in crtc->config already? I don't think this
>> > code
>> > would run before reading out and sanitizing the hardware state.
>>
>> I was not sure of what will be the DPLL attached to crtc->config when the
>> crtc is found by 'get_unused_crtc' logic. So, we select the DPLL based
>> on port again..
>>
>> >
>> > > ;
>> > > +	pll = intel_crtc_to_shared_dpll(crtc);
>> > > +	if (!pll) {
>> > > +		DRM_ERROR("Could not get shared DPLL\n");
>> > > +		goto exit;
>> > > +	}
>> > > +	DRM_DEBUG_KMS("Using %s for pipe %c\n", pll->name, pipe_name(crtc
>> > > ->pipe));
>> > > +
>> > > +	do {
>> > > +		crtc->config->port_clock =
>> > > drm_dp_bw_code_to_link_rate(link_bw);
>> > > +		crtc->config->lane_count = lane_count;
>> > > +		if (!intel_ddi_pll_select(crtc, crtc->config, encoder,
>> > > false))
>> >
>> >
>> > > +			goto exit;
>> > > +
>> > > +		pll->config.crtc_mask |= (1 << crtc->pipe);
>> > > +		pll->config.hw_state = crtc->config->dpll_hw_state;
>> > > +
>> > > +		/* Enable PLL followed by port */
>> > > +		intel_enable_shared_dpll(crtc);
>> > > +		encoder->pre_enable(encoder);
>> >
>> > The pll handling here seems dodgy. Instead of using intel_get_shared_dpll(),
>>
>> I initially tried this, but intel_get_shared_dpll() uses crtc_state which is
>> not
>> set (in outside modeset contexts) thus creating crashes. Specifically,
>> the call to intel_ddi_get_crtc_new_encoder() for broxton code path.
>>
>> That’s why I had to create some initial code to not call get_shared_dpll
>> If we have valid encoder attached. (patches 1/7 and 2/7 of this series)
>>
>> So, If you have suggestions on how to fix this in a neat way,
>> I would be happy to try them out.
>
>One idea would be split the part of intel_get_shared_dpll() that needs atomic
>interfaces to a separate function. Then it would be possible to add a second
>entry point that just takes crtc, encoder and the pll parameters and calls the
>logic that doesn't depend on the atomic state.
>
>Then you could move the calls to intel_get_shared_dpll() from *_ddi_pll_select()
>to haswell_crtc_compute_clock() so they don't get on the way during upfront link
>training, where the new non-atomic entry point is called instead.

Ok, Will try this implementation.

Thanks,
Durga

>
>Ander
>
>>
>> Thanks,
>> Durga
>>
>> > this fiddles with pll internals itself. I think this will work for broxton,
>> > since it doesn't actually have shared DPLLs (their chosen based on the
>> > encoder).
>> > It might just work for haswell too since the plls used by DP are not shared.
>> >
>> > But to do this cleanly we need the DPLL interface to just give us the right
>> > pll.
>> >
>> > Ander
>> >
>> >
>> > > +
>> > > +		/* Check if link training passed; if so update DPCD */
>> > > +		if (intel_dp->train_set_valid)
>> > > +			intel_dp_update_dpcd_params(intel_dp);
>> > > +
>> > > +		/* Disable port followed by PLL for next retry/clean up
>> > > */
>> > > +		encoder->post_disable(encoder);
>> > > +		intel_disable_shared_dpll(crtc);
>> > > +
>> > > +	} while (!intel_dp->train_set_valid &&
>> > > +		!intel_dp_get_link_retry_params(&lane_count, &link_bw));
>> > > +
>> > > +	/* Reset pll state as before */
>> > > +	pll->config.crtc_mask &= ~(1 << crtc->pipe);
>> > > +	pll->config.hw_state = tmp_dpll_hw_state;
>> > > +
>> > > +exit:
>> > > +	/* Reset local associations made */
>> > > +	if (drm_crtc)
>> > > +		encoder->base.crtc = NULL;
>> > > +	crtc->config = tmp_crtc_config;
>> > > +
>> > > +	DRM_DEBUG_KMS("Upfront link train %s: lanes:%d bw:%d\n",
>> > > +	intel_dp->train_set_valid ? "Passed" : "Failed", lane_count,
>> > > link_bw);
>> > > +
>> > > +	return intel_dp->train_set_valid;
>> > > +}
>> > > +
>> > >  void intel_ddi_init(struct drm_device *dev, enum port port)
>> > >  {
>> > >  	struct drm_i915_private *dev_priv = dev->dev_private;
>> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
>> > > b/drivers/gpu/drm/i915/intel_dp.c
>> > > index d8f7830..47b6266 100644
>> > > --- a/drivers/gpu/drm/i915/intel_dp.c
>> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
>> > > @@ -4622,6 +4622,71 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
>> > >  	intel_dp->has_audio = false;
>> > >  }
>> > >
>> > > +static void intel_dp_upfront_dpms_off(struct drm_connector *connector,
>> > > +				struct drm_modeset_acquire_ctx *ctx)
>> > > +{
>> > > +	struct intel_dp *intel_dp = intel_attached_dp(connector);
>> > > +	struct intel_digital_port *intel_dig_port =
>> > > dp_to_dig_port(intel_dp);
>> > > +	struct drm_crtc *crtc = intel_dig_port->base.base.crtc;
>> > > +	struct intel_load_detect_pipe tmp;
>> > > +
>> > > +	if (intel_get_load_detect_pipe(connector, NULL, &tmp, ctx)) {
>> > > +		crtc->acquire_ctx = ctx;
>> > > +		tmp.dpms_mode = DRM_MODE_DPMS_OFF;
>> > > +		intel_release_load_detect_pipe(connector, &tmp, ctx);
>> > > +	}
>> > > +}
>> > > +
>> > > +static void intel_dp_upfront_dpms_on(struct drm_connector *connector,
>> > > +				struct drm_modeset_acquire_ctx *ctx)
>> > > +{
>> > > +	struct intel_load_detect_pipe tmp;
>> > > +
>> > > +	intel_get_load_detect_pipe(connector, NULL, &tmp, ctx);
>> > > +
>> > > +	drm_modeset_drop_locks(ctx);
>> > > +	drm_modeset_acquire_fini(ctx);
>> > > +}
>> > > +
>> > > +static bool intel_dp_upfront_link_train(struct drm_connector *connector)
>> > > +{
>> > > +	struct intel_dp *intel_dp = intel_attached_dp(connector);
>> > > +	struct intel_digital_port *intel_dig_port =
>> > > dp_to_dig_port(intel_dp);
>> > > +	struct intel_encoder *intel_encoder = &intel_dig_port->base;
>> > > +	struct drm_device *dev = intel_encoder->base.dev;
>> > > +	struct drm_crtc *crtc = intel_dig_port->base.base.crtc;
>> > > +	struct intel_crtc *intel_crtc = crtc ? to_intel_crtc(crtc) :
>> > > NULL;
>> > > +	struct drm_modeset_acquire_ctx ctx, *old_ctx = NULL;
>> > > +	bool ret = true, need_dpms_on = false;
>> > > +
>> > > +	if (!IS_BROXTON(dev))
>> > > +		return true;
>> > > +	/*
>> > > +	 * On hotplug cases, we call _upfront_link_train directly.
>> > > +	 * In connected boot scenarios (boot with {MIPI/eDP} + DP),
>> > > +	 * BIOS typically brings up DP. Hence, we disable the crtc
>> > > +	 * to do _upfront_link_training and then re-enable it back.
>> > > +	 */
>> > > +	if (crtc && crtc->enabled && intel_crtc->active) {
>> > > +		DRM_DEBUG_KMS("Disabling pipe %c\n", pipe_name(intel_crtc
>> > > ->pipe));
>> > > +		old_ctx = crtc->acquire_ctx;
>> > > +		drm_modeset_acquire_init(&ctx, 0);
>> > > +		intel_dp_upfront_dpms_off(connector, &ctx);
>> > > +		need_dpms_on = true;
>> > > +	}
>> > > +
>> > > +	if (HAS_DDI(dev))
>> > > +		ret = intel_ddi_upfront_link_train(intel_dp, intel_crtc);
>> > > +		/* Other platforms upfront link train call goes here..*/
>> > > +
>> > > +	if (need_dpms_on) {
>> > > +		intel_dp_upfront_dpms_on(connector, &ctx);
>> > > +		crtc->acquire_ctx = old_ctx;
>> > > +	}
>> > > +	return ret;
>> > > +}
>> > > +
>> > > +
>> > >  static enum drm_connector_status
>> > >  intel_dp_detect(struct drm_connector *connector, bool force)
>> > >  {
>> > > @@ -4631,7 +4696,7 @@ intel_dp_detect(struct drm_connector *connector,
>> > > bool
>> > > force)
>> > >  	struct drm_device *dev = connector->dev;
>> > >  	enum drm_connector_status status;
>> > >  	enum intel_display_power_domain power_domain;
>> > > -	bool ret;
>> > > +	bool ret, do_upfront_link_train;
>> > >  	u8 sink_irq_vector;
>> > >
>> > >  	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
>> > > @@ -4705,6 +4770,16 @@ intel_dp_detect(struct drm_connector *connector,
>> > > bool
>> > > force)
>> > >  			DRM_DEBUG_DRIVER("CP or sink specific irq
>> > > unhandled\n");
>> > >  	}
>> > >
>> > > +	/* Do not do upfront link train, if it is a compliance request */
>> > > +	do_upfront_link_train =
>> > > +		intel_encoder->type == INTEL_OUTPUT_DISPLAYPORT &&
>> > > +		intel_dp->compliance_test_type != DP_TEST_LINK_TRAINING;
>> > > +
>> > > +	if (do_upfront_link_train) {
>> > > +		ret = intel_dp_upfront_link_train(connector);
>> > > +		if (!ret)
>> > > +			status = connector_status_disconnected;
>> > > +	}
>> > >  out:
>> > >  	intel_display_power_put(to_i915(dev), power_domain);
>> > >  	return status;
>> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h
>> > > b/drivers/gpu/drm/i915/intel_drv.h
>> > > index 5912955..3cfab20 100644
>> > > --- a/drivers/gpu/drm/i915/intel_drv.h
>> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
>> > > @@ -1025,6 +1025,8 @@ void intel_ddi_clock_get(struct intel_encoder
>> > > *encoder,
>> > >  			 struct intel_crtc_state *pipe_config);
>> > >  void intel_ddi_set_vc_payload_alloc(struct drm_crtc *crtc, bool state);
>> > >  uint32_t ddi_signal_levels(struct intel_dp *intel_dp);
>> > > +bool intel_ddi_upfront_link_train(struct intel_dp *intel_dp,
>> > > +				struct intel_crtc *crtc);
>> > >
>> > >  /* intel_frontbuffer.c */
>> > >  void intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 7/7] drm/i915/dp: Enable Upfront link training for typeC DP support on BXT

[R, Durgadoss]

>-----Original Message-----
>From: Ander Conselvan De Oliveira [mailto:conselvan2@gmail.com]
>Sent: Monday, January 11, 2016 8:46 PM
>To: R, Durgadoss; intel-gfx@lists.freedesktop.org
>Subject: Re: [PATCH 7/7] drm/i915/dp: Enable Upfront link training for typeC DP support on BXT
>
>On Fri, 2015-12-11 at 15:09 +0530, Durgadoss R wrote:
>> To support USB type C alternate DP mode, the display driver needs to
>> know the number of lanes required by the DP panel as well as number
>> of lanes that can be supported by the type-C cable. Sometimes, the
>> type-C cable may limit the bandwidth even if Panel can support
>> more lanes. To address these scenarios, the display driver will
>> start link training with max lanes, and if that fails, the driver
>> falls back to x2 lanes; and repeats this procedure for all
>> bandwidth/lane configurations.
>>
>> * Since link training is done before modeset only the port
>>   (and not pipe/planes) and its associated PLLs are enabled.
>> * On DP hotplug: Directly start link training on the crtc
>>   associated with the DP encoder.
>> * On Connected boot scenarios: When booted with an LFP and a DP,
>>   typically, BIOS brings up DP. In these cases, we disable the
>>   crtc, do upfront link training and then re-enable it back.
>> * All local changes made for upfront link training are reset
>>   to their previous values once it is done; so that the
>>   subsequent modeset is not aware of these changes.
>>
>> Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_ddi.c | 81
>> ++++++++++++++++++++++++++++++++++++++++
>>  drivers/gpu/drm/i915/intel_dp.c  | 77 +++++++++++++++++++++++++++++++++++++-
>>  drivers/gpu/drm/i915/intel_drv.h |  2 +
>>  3 files changed, 159 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
>> b/drivers/gpu/drm/i915/intel_ddi.c
>> index 632044a..43efc26 100644
>> --- a/drivers/gpu/drm/i915/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
>> @@ -3274,6 +3274,87 @@ intel_ddi_init_hdmi_connector(struct intel_digital_port
>> *intel_dig_port)
>>  	return connector;
>>  }
>>
>> +bool intel_ddi_upfront_link_train(struct intel_dp *intel_dp,
>> +				struct intel_crtc *crtc)
>> +{
>> +	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
>> +	struct intel_connector *connector = intel_dp->attached_connector;
>> +	struct intel_encoder *encoder = connector->encoder;
>> +	struct drm_device *dev = encoder->base.dev;
>> +	struct drm_i915_private *dev_priv = dev->dev_private;
>> +	struct intel_shared_dpll *pll;
>> +	struct drm_crtc *drm_crtc = NULL;
>> +	struct intel_crtc_state *tmp_crtc_config;
>> +	struct intel_dpll_hw_state tmp_dpll_hw_state;
>> +	uint8_t link_bw, lane_count;
>> +
>> +	if (!crtc) {
>> +		drm_crtc = intel_get_unused_crtc(&encoder->base);
>> +		if (!drm_crtc) {
>> +			DRM_ERROR("No crtc for upfront link training\n");
>> +			return false;
>> +		}
>> +		encoder->base.crtc = drm_crtc;
>> +		crtc = to_intel_crtc(drm_crtc);
>> +	}
>> +
>> +	/* Initialize with Max Link rate & lane count supported by panel */
>> +	link_bw =  intel_dp->dpcd[DP_MAX_LINK_RATE];
>> +	lane_count = drm_dp_max_lane_count(intel_dp->dpcd);
>> +
>> +	/* Save the crtc->config */
>> +	tmp_crtc_config = crtc->config;
>> +	tmp_dpll_hw_state = crtc->config->dpll_hw_state;
>> +
>> +	/* Select the shared DPLL to use for this port */
>> +	intel_get_ddi_pll(dev_priv, dig_port->port, crtc->config);
>> +	pll = intel_crtc_to_shared_dpll(crtc);
>> +	if (!pll) {
>> +		DRM_ERROR("Could not get shared DPLL\n");
>> +		goto exit;
>> +	}
>> +	DRM_DEBUG_KMS("Using %s for pipe %c\n", pll->name, pipe_name(crtc
>> ->pipe));
>> +
>> +	do {
>> +		crtc->config->port_clock =
>> drm_dp_bw_code_to_link_rate(link_bw);
>> +		crtc->config->lane_count = lane_count;
>> +		if (!intel_ddi_pll_select(crtc, crtc->config, encoder,
>> false))
>> +			goto exit;
>> +
>> +		pll->config.crtc_mask |= (1 << crtc->pipe);
>> +		pll->config.hw_state = crtc->config->dpll_hw_state;
>> +
>> +		/* Enable PLL followed by port */
>> +		intel_enable_shared_dpll(crtc);
>> +		encoder->pre_enable(encoder);
>> +
>> +		/* Check if link training passed; if so update DPCD */
>> +		if (intel_dp->train_set_valid)
>> +			intel_dp_update_dpcd_params(intel_dp);
>> +
>> +		/* Disable port followed by PLL for next retry/clean up */
>> +		encoder->post_disable(encoder);
>> +		intel_disable_shared_dpll(crtc);
>> +
>> +	} while (!intel_dp->train_set_valid &&
>> +		!intel_dp_get_link_retry_params(&lane_count, &link_bw));
>> +
>> +	/* Reset pll state as before */
>> +	pll->config.crtc_mask &= ~(1 << crtc->pipe);
>> +	pll->config.hw_state = tmp_dpll_hw_state;
>> +
>> +exit:
>> +	/* Reset local associations made */
>> +	if (drm_crtc)
>> +		encoder->base.crtc = NULL;
>> +	crtc->config = tmp_crtc_config;
>> +
>> +	DRM_DEBUG_KMS("Upfront link train %s: lanes:%d bw:%d\n",
>> +	intel_dp->train_set_valid ? "Passed" : "Failed", lane_count,
>> link_bw);
>> +
>> +	return intel_dp->train_set_valid;
>> +}
>> +
>>  void intel_ddi_init(struct drm_device *dev, enum port port)
>>  {
>>  	struct drm_i915_private *dev_priv = dev->dev_private;
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index d8f7830..47b6266 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -4622,6 +4622,71 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
>>  	intel_dp->has_audio = false;
>>  }
>>
>> +static void intel_dp_upfront_dpms_off(struct drm_connector *connector,
>> +				struct drm_modeset_acquire_ctx *ctx)
>> +{
>> +	struct intel_dp *intel_dp = intel_attached_dp(connector);
>> +	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>> +	struct drm_crtc *crtc = intel_dig_port->base.base.crtc;
>> +	struct intel_load_detect_pipe tmp;
>> +
>> +	if (intel_get_load_detect_pipe(connector, NULL, &tmp, ctx)) {
>> +		crtc->acquire_ctx = ctx;
>> +		tmp.dpms_mode = DRM_MODE_DPMS_OFF;
>> +		intel_release_load_detect_pipe(connector, &tmp, ctx);
>> +	}
>> +}
>> +
>> +static void intel_dp_upfront_dpms_on(struct drm_connector *connector,
>> +				struct drm_modeset_acquire_ctx *ctx)
>> +{
>> +	struct intel_load_detect_pipe tmp;
>> +
>> +	intel_get_load_detect_pipe(connector, NULL, &tmp, ctx);
>> +
>> +	drm_modeset_drop_locks(ctx);
>> +	drm_modeset_acquire_fini(ctx);
>> +}
>
>Are these supposed to disable/enable the crtc so that the upfront link training
>can be performed? The load detect pipe really doesn't belong here, its only
>purpose is to detect whether there is an output connected in platforms that
>didn't support hotplug. You should do a normal atomic operation instead.

I think I should add more comment on this usage here..

During RFC, we were discussing about re-using code from load_detect logic 
to find unused Crtc's, to do upfront link training. While doing that, I figured out
that the same can be used here (replacing intel_crtc_control() from non-atomic world.)

Now, if BIOS has enabled the crtc during boot, we disable it during upfront link
training and then turn it on once we are done. This set of dpms_on/off operations
require the modeset locking infrastructure which are (already) provided by 
*_load_detect() functions.

If using the functions as such does not make logical sense, then we can separate
out the required code from *_load_detect() functions, and then re-use them.
(like we did for get_unused_crtc() code). Does this sound ok to you ?

Thanks,
Durga

>
>Ander
>
>> +
>> +static bool intel_dp_upfront_link_train(struct drm_connector *connector)
>> +{
>> +	struct intel_dp *intel_dp = intel_attached_dp(connector);
>> +	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>> +	struct intel_encoder *intel_encoder = &intel_dig_port->base;
>> +	struct drm_device *dev = intel_encoder->base.dev;
>> +	struct drm_crtc *crtc = intel_dig_port->base.base.crtc;
>> +	struct intel_crtc *intel_crtc = crtc ? to_intel_crtc(crtc) : NULL;
>> +	struct drm_modeset_acquire_ctx ctx, *old_ctx = NULL;
>> +	bool ret = true, need_dpms_on = false;
>> +
>> +	if (!IS_BROXTON(dev))
>> +		return true;
>> +	/*
>> +	 * On hotplug cases, we call _upfront_link_train directly.
>> +	 * In connected boot scenarios (boot with {MIPI/eDP} + DP),
>> +	 * BIOS typically brings up DP. Hence, we disable the crtc
>> +	 * to do _upfront_link_training and then re-enable it back.
>> +	 */
>> +	if (crtc && crtc->enabled && intel_crtc->active) {
>> +		DRM_DEBUG_KMS("Disabling pipe %c\n", pipe_name(intel_crtc
>> ->pipe));
>> +		old_ctx = crtc->acquire_ctx;
>> +		drm_modeset_acquire_init(&ctx, 0);
>> +		intel_dp_upfront_dpms_off(connector, &ctx);
>> +		need_dpms_on = true;
>> +	}
>> +
>> +	if (HAS_DDI(dev))
>> +		ret = intel_ddi_upfront_link_train(intel_dp, intel_crtc);
>> +		/* Other platforms upfront link train call goes here..*/
>> +
>> +	if (need_dpms_on) {
>> +		intel_dp_upfront_dpms_on(connector, &ctx);
>> +		crtc->acquire_ctx = old_ctx;
>> +	}
>> +	return ret;
>> +}
>> +
>> +
>>  static enum drm_connector_status
>>  intel_dp_detect(struct drm_connector *connector, bool force)
>>  {
>> @@ -4631,7 +4696,7 @@ intel_dp_detect(struct drm_connector *connector, bool
>> force)
>>  	struct drm_device *dev = connector->dev;
>>  	enum drm_connector_status status;
>>  	enum intel_display_power_domain power_domain;
>> -	bool ret;
>> +	bool ret, do_upfront_link_train;
>>  	u8 sink_irq_vector;
>>
>>  	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
>> @@ -4705,6 +4770,16 @@ intel_dp_detect(struct drm_connector *connector, bool
>> force)
>>  			DRM_DEBUG_DRIVER("CP or sink specific irq
>> unhandled\n");
>>  	}
>>
>> +	/* Do not do upfront link train, if it is a compliance request */
>> +	do_upfront_link_train =
>> +		intel_encoder->type == INTEL_OUTPUT_DISPLAYPORT &&
>> +		intel_dp->compliance_test_type != DP_TEST_LINK_TRAINING;
>> +
>> +	if (do_upfront_link_train) {
>> +		ret = intel_dp_upfront_link_train(connector);
>> +		if (!ret)
>> +			status = connector_status_disconnected;
>> +	}
>>  out:
>>  	intel_display_power_put(to_i915(dev), power_domain);
>>  	return status;
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h
>> b/drivers/gpu/drm/i915/intel_drv.h
>> index 5912955..3cfab20 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -1025,6 +1025,8 @@ void intel_ddi_clock_get(struct intel_encoder *encoder,
>>  			 struct intel_crtc_state *pipe_config);
>>  void intel_ddi_set_vc_payload_alloc(struct drm_crtc *crtc, bool state);
>>  uint32_t ddi_signal_levels(struct intel_dp *intel_dp);
>> +bool intel_ddi_upfront_link_train(struct intel_dp *intel_dp,
>> +				struct intel_crtc *crtc);
>>
>>  /* intel_frontbuffer.c */
>>  void intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/7] drm/i915/dp: Add methods to update link train params

[R, Durgadoss]

>-----Original Message-----
>From: Ander Conselvan De Oliveira [mailto:conselvan2@gmail.com]
>Sent: Monday, January 11, 2016 8:07 PM
>To: R, Durgadoss; intel-gfx@lists.freedesktop.org
>Subject: Re: [PATCH 5/7] drm/i915/dp: Add methods to update link train params
>
>On Fri, 2015-12-11 at 15:09 +0530, Durgadoss R wrote:
>> Retrying with reduced lanes/bw and updating the final
>> available lanes/bw to DPCD is needed for upfront link
>> train logic. Hence, this patch adds these methods
>> and exports them so that these can be called from
>> other files like ddi.c/display.c.
>>
>> Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_dp.c  | 34 ++++++++++++++++++++++++++++++++++
>>  drivers/gpu/drm/i915/intel_drv.h |  2 ++
>>  2 files changed, 36 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index f335c92..d8f7830 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -1660,6 +1660,40 @@ void intel_dp_set_link_params(struct intel_dp
>> *intel_dp,
>>  	intel_dp->lane_count = pipe_config->lane_count;
>>  }
>>
>> +void intel_dp_update_dpcd_params(struct intel_dp *intel_dp)
>> +{
>> +	intel_dp->dpcd[DP_MAX_LANE_COUNT] &= ~DP_MAX_LANE_COUNT_MASK;
>> +	intel_dp->dpcd[DP_MAX_LANE_COUNT] |=
>> +			intel_dp->lane_count & DP_MAX_LANE_COUNT_MASK;
>> +
>> +	intel_dp->dpcd[DP_MAX_LINK_RATE] =
>> +			drm_dp_link_rate_to_bw_code(intel_dp->link_rate);
>> +}
>> +
>> +bool intel_dp_get_link_retry_params(uint8_t *lane_count, uint8_t *link_bw)
>> +{
>> +	/*
>> +	 * As per DP1.3 Spec, retry all link rates for a particular
>> +	 * lane count value, before reducing number of lanes.
>> +	 */
>> +	if (*link_bw == DP_LINK_BW_5_4) {
>> +		*link_bw = DP_LINK_BW_2_7;
>> +	} else if (*link_bw == DP_LINK_BW_2_7) {
>> +		*link_bw = DP_LINK_BW_1_62;
>> +	} else if (*lane_count == 4) {
>> +		*lane_count = 2;
>> +		*link_bw = DP_LINK_BW_5_4;
>> +	} else if (*lane_count == 2) {
>> +		*lane_count = 1;
>> +		*link_bw = DP_LINK_BW_5_4;
>
>If the sink doesn't support 5.4 Gbps, shouldn't we skip that rate?
>
>Also, it would be easier to review these functions if they were in same patch
>were they are used.

Ok, will take care of this next version.

Thanks,
Durga

>
>Ander
>
>> +	} else {
>> +		/* Tried all combinations, so exit */
>> +		return false;
>> +	}
>> +
>> +	return true;
>> +}
>> +
>>  static void intel_dp_prepare(struct intel_encoder *encoder)
>>  {
>>  	struct drm_device *dev = encoder->base.dev;
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h
>> b/drivers/gpu/drm/i915/intel_drv.h
>> index faa91fc..5cefb0e 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -1233,6 +1233,8 @@ bool intel_dp_init_connector(struct intel_digital_port
>> *intel_dig_port,
>>  			     struct intel_connector *intel_connector);
>>  void intel_dp_set_link_params(struct intel_dp *intel_dp,
>>  			      const struct intel_crtc_state *pipe_config);
>> +void intel_dp_update_dpcd_params(struct intel_dp *intel_dp);
>> +bool intel_dp_get_link_retry_params(uint8_t *lane_count, uint8_t *link_bw);
>>  void intel_dp_start_link_train(struct intel_dp *intel_dp);
>>  void intel_dp_stop_link_train(struct intel_dp *intel_dp);
>>  void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 7/7] drm/i915/dp: Enable Upfront link training for typeC DP support on BXT

[Ander Conselvan De Oliveira]

On Mon, 2016-01-11 at 17:53 +0000, R, Durgadoss wrote:
> > -----Original Message-----
> > From: Ander Conselvan De Oliveira [mailto:conselvan2@gmail.com]
> > Sent: Monday, January 11, 2016 8:46 PM
> > To: R, Durgadoss; intel-gfx@lists.freedesktop.org
> > Subject: Re: [PATCH 7/7] drm/i915/dp: Enable Upfront link training for typeC
> > DP support on BXT
> > 
> > On Fri, 2015-12-11 at 15:09 +0530, Durgadoss R wrote:
> > > To support USB type C alternate DP mode, the display driver needs to
> > > know the number of lanes required by the DP panel as well as number
> > > of lanes that can be supported by the type-C cable. Sometimes, the
> > > type-C cable may limit the bandwidth even if Panel can support
> > > more lanes. To address these scenarios, the display driver will
> > > start link training with max lanes, and if that fails, the driver
> > > falls back to x2 lanes; and repeats this procedure for all
> > > bandwidth/lane configurations.
> > > 
> > > * Since link training is done before modeset only the port
> > >   (and not pipe/planes) and its associated PLLs are enabled.
> > > * On DP hotplug: Directly start link training on the crtc
> > >   associated with the DP encoder.
> > > * On Connected boot scenarios: When booted with an LFP and a DP,
> > >   typically, BIOS brings up DP. In these cases, we disable the
> > >   crtc, do upfront link training and then re-enable it back.
> > > * All local changes made for upfront link training are reset
> > >   to their previous values once it is done; so that the
> > >   subsequent modeset is not aware of these changes.
> > > 
> > > Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_ddi.c | 81
> > > ++++++++++++++++++++++++++++++++++++++++
> > >  drivers/gpu/drm/i915/intel_dp.c  | 77
> > > +++++++++++++++++++++++++++++++++++++-
> > >  drivers/gpu/drm/i915/intel_drv.h |  2 +
> > >  3 files changed, 159 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> > > b/drivers/gpu/drm/i915/intel_ddi.c
> > > index 632044a..43efc26 100644
> > > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > > @@ -3274,6 +3274,87 @@ intel_ddi_init_hdmi_connector(struct
> > > intel_digital_port
> > > *intel_dig_port)
> > >  	return connector;
> > >  }
> > > 
> > > +bool intel_ddi_upfront_link_train(struct intel_dp *intel_dp,
> > > +				struct intel_crtc *crtc)
> > > +{
> > > +	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> > > +	struct intel_connector *connector = intel_dp->attached_connector;
> > > +	struct intel_encoder *encoder = connector->encoder;
> > > +	struct drm_device *dev = encoder->base.dev;
> > > +	struct drm_i915_private *dev_priv = dev->dev_private;
> > > +	struct intel_shared_dpll *pll;
> > > +	struct drm_crtc *drm_crtc = NULL;
> > > +	struct intel_crtc_state *tmp_crtc_config;
> > > +	struct intel_dpll_hw_state tmp_dpll_hw_state;
> > > +	uint8_t link_bw, lane_count;
> > > +
> > > +	if (!crtc) {
> > > +		drm_crtc = intel_get_unused_crtc(&encoder->base);
> > > +		if (!drm_crtc) {
> > > +			DRM_ERROR("No crtc for upfront link training\n");
> > > +			return false;
> > > +		}
> > > +		encoder->base.crtc = drm_crtc;
> > > +		crtc = to_intel_crtc(drm_crtc);
> > > +	}
> > > +
> > > +	/* Initialize with Max Link rate & lane count supported by panel
> > > */
> > > +	link_bw =  intel_dp->dpcd[DP_MAX_LINK_RATE];
> > > +	lane_count = drm_dp_max_lane_count(intel_dp->dpcd);
> > > +
> > > +	/* Save the crtc->config */
> > > +	tmp_crtc_config = crtc->config;
> > > +	tmp_dpll_hw_state = crtc->config->dpll_hw_state;
> > > +
> > > +	/* Select the shared DPLL to use for this port */
> > > +	intel_get_ddi_pll(dev_priv, dig_port->port, crtc->config);
> > > +	pll = intel_crtc_to_shared_dpll(crtc);
> > > +	if (!pll) {
> > > +		DRM_ERROR("Could not get shared DPLL\n");
> > > +		goto exit;
> > > +	}
> > > +	DRM_DEBUG_KMS("Using %s for pipe %c\n", pll->name, pipe_name(crtc
> > > ->pipe));
> > > +
> > > +	do {
> > > +		crtc->config->port_clock =
> > > drm_dp_bw_code_to_link_rate(link_bw);
> > > +		crtc->config->lane_count = lane_count;
> > > +		if (!intel_ddi_pll_select(crtc, crtc->config, encoder,
> > > false))
> > > +			goto exit;
> > > +
> > > +		pll->config.crtc_mask |= (1 << crtc->pipe);
> > > +		pll->config.hw_state = crtc->config->dpll_hw_state;
> > > +
> > > +		/* Enable PLL followed by port */
> > > +		intel_enable_shared_dpll(crtc);
> > > +		encoder->pre_enable(encoder);
> > > +
> > > +		/* Check if link training passed; if so update DPCD */
> > > +		if (intel_dp->train_set_valid)
> > > +			intel_dp_update_dpcd_params(intel_dp);
> > > +
> > > +		/* Disable port followed by PLL for next retry/clean up
> > > */
> > > +		encoder->post_disable(encoder);
> > > +		intel_disable_shared_dpll(crtc);
> > > +
> > > +	} while (!intel_dp->train_set_valid &&
> > > +		!intel_dp_get_link_retry_params(&lane_count, &link_bw));
> > > +
> > > +	/* Reset pll state as before */
> > > +	pll->config.crtc_mask &= ~(1 << crtc->pipe);
> > > +	pll->config.hw_state = tmp_dpll_hw_state;
> > > +
> > > +exit:
> > > +	/* Reset local associations made */
> > > +	if (drm_crtc)
> > > +		encoder->base.crtc = NULL;
> > > +	crtc->config = tmp_crtc_config;
> > > +
> > > +	DRM_DEBUG_KMS("Upfront link train %s: lanes:%d bw:%d\n",
> > > +	intel_dp->train_set_valid ? "Passed" : "Failed", lane_count,
> > > link_bw);
> > > +
> > > +	return intel_dp->train_set_valid;
> > > +}
> > > +
> > >  void intel_ddi_init(struct drm_device *dev, enum port port)
> > >  {
> > >  	struct drm_i915_private *dev_priv = dev->dev_private;
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > b/drivers/gpu/drm/i915/intel_dp.c
> > > index d8f7830..47b6266 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -4622,6 +4622,71 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
> > >  	intel_dp->has_audio = false;
> > >  }
> > > 
> > > +static void intel_dp_upfront_dpms_off(struct drm_connector *connector,
> > > +				struct drm_modeset_acquire_ctx *ctx)
> > > +{
> > > +	struct intel_dp *intel_dp = intel_attached_dp(connector);
> > > +	struct intel_digital_port *intel_dig_port =
> > > dp_to_dig_port(intel_dp);
> > > +	struct drm_crtc *crtc = intel_dig_port->base.base.crtc;
> > > +	struct intel_load_detect_pipe tmp;
> > > +
> > > +	if (intel_get_load_detect_pipe(connector, NULL, &tmp, ctx)) {
> > > +		crtc->acquire_ctx = ctx;
> > > +		tmp.dpms_mode = DRM_MODE_DPMS_OFF;
> > > +		intel_release_load_detect_pipe(connector, &tmp, ctx);
> > > +	}
> > > +}
> > > +
> > > +static void intel_dp_upfront_dpms_on(struct drm_connector *connector,
> > > +				struct drm_modeset_acquire_ctx *ctx)
> > > +{
> > > +	struct intel_load_detect_pipe tmp;
> > > +
> > > +	intel_get_load_detect_pipe(connector, NULL, &tmp, ctx);
> > > +
> > > +	drm_modeset_drop_locks(ctx);
> > > +	drm_modeset_acquire_fini(ctx);
> > > +}
> > 
> > Are these supposed to disable/enable the crtc so that the upfront link
> > training
> > can be performed? The load detect pipe really doesn't belong here, its only
> > purpose is to detect whether there is an output connected in platforms that
> > didn't support hotplug. You should do a normal atomic operation instead.
> 
> I think I should add more comment on this usage here..
> 
> During RFC, we were discussing about re-using code from load_detect logic 
> to find unused Crtc's, to do upfront link training. While doing that, I
> figured out
> that the same can be used here (replacing intel_crtc_control() from non-atomic
> world.)

DPMS is now implemented using atomic, by setting the connector dpms function to
drm_atomic_helper_connector_dpms(). It should be ok to call that directly here.

> Now, if BIOS has enabled the crtc during boot, we disable it during upfront
> link
> training and then turn it on once we are done. This set of dpms_on/off
> operations
> require the modeset locking infrastructure which are (already) provided by 
> *_load_detect() functions.
> 
> If using the functions as such does not make logical sense, then we can
> separate
> out the required code from *_load_detect() functions, and then re-use them.
> (like we did for get_unused_crtc() code). Does this sound ok to you ?

The load detect pipe does the opposite of what you want here. It enables a
disabled crtc using an fb allocated by that function. After the pipe has been
used for load detection, a call to intel_release_load_detect_pipe() turns the
pipe back off and releases the fb.

In the code above, there is a call to intel_get_load_detect_pipe() without a
matching release. At that point, the crtc is left on with the wrong fb (the one
allocated for the load detect pipe instead of the fb it was using before) and
that fb is never freed.

So I think the best solution is to call drm_atomic_helper_connector_dpms()
directly.


Ander


> Thanks,
> Durga
> 
> > 
> > Ander
> > 
> > > +
> > > +static bool intel_dp_upfront_link_train(struct drm_connector *connector)
> > > +{
> > > +	struct intel_dp *intel_dp = intel_attached_dp(connector);
> > > +	struct intel_digital_port *intel_dig_port =
> > > dp_to_dig_port(intel_dp);
> > > +	struct intel_encoder *intel_encoder = &intel_dig_port->base;
> > > +	struct drm_device *dev = intel_encoder->base.dev;
> > > +	struct drm_crtc *crtc = intel_dig_port->base.base.crtc;
> > > +	struct intel_crtc *intel_crtc = crtc ? to_intel_crtc(crtc) :
> > > NULL;
> > > +	struct drm_modeset_acquire_ctx ctx, *old_ctx = NULL;
> > > +	bool ret = true, need_dpms_on = false;
> > > +
> > > +	if (!IS_BROXTON(dev))
> > > +		return true;
> > > +	/*
> > > +	 * On hotplug cases, we call _upfront_link_train directly.
> > > +	 * In connected boot scenarios (boot with {MIPI/eDP} + DP),
> > > +	 * BIOS typically brings up DP. Hence, we disable the crtc
> > > +	 * to do _upfront_link_training and then re-enable it back.
> > > +	 */
> > > +	if (crtc && crtc->enabled && intel_crtc->active) {
> > > +		DRM_DEBUG_KMS("Disabling pipe %c\n", pipe_name(intel_crtc
> > > ->pipe));
> > > +		old_ctx = crtc->acquire_ctx;
> > > +		drm_modeset_acquire_init(&ctx, 0);
> > > +		intel_dp_upfront_dpms_off(connector, &ctx);
> > > +		need_dpms_on = true;
> > > +	}
> > > +
> > > +	if (HAS_DDI(dev))
> > > +		ret = intel_ddi_upfront_link_train(intel_dp, intel_crtc);
> > > +		/* Other platforms upfront link train call goes here..*/
> > > +
> > > +	if (need_dpms_on) {
> > > +		intel_dp_upfront_dpms_on(connector, &ctx);
> > > +		crtc->acquire_ctx = old_ctx;
> > > +	}
> > > +	return ret;
> > > +}
> > > +
> > > +
> > >  static enum drm_connector_status
> > >  intel_dp_detect(struct drm_connector *connector, bool force)
> > >  {
> > > @@ -4631,7 +4696,7 @@ intel_dp_detect(struct drm_connector *connector,
> > > bool
> > > force)
> > >  	struct drm_device *dev = connector->dev;
> > >  	enum drm_connector_status status;
> > >  	enum intel_display_power_domain power_domain;
> > > -	bool ret;
> > > +	bool ret, do_upfront_link_train;
> > >  	u8 sink_irq_vector;
> > > 
> > >  	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
> > > @@ -4705,6 +4770,16 @@ intel_dp_detect(struct drm_connector *connector,
> > > bool
> > > force)
> > >  			DRM_DEBUG_DRIVER("CP or sink specific irq
> > > unhandled\n");
> > >  	}
> > > 
> > > +	/* Do not do upfront link train, if it is a compliance request */
> > > +	do_upfront_link_train =
> > > +		intel_encoder->type == INTEL_OUTPUT_DISPLAYPORT &&
> > > +		intel_dp->compliance_test_type != DP_TEST_LINK_TRAINING;
> > > +
> > > +	if (do_upfront_link_train) {
> > > +		ret = intel_dp_upfront_link_train(connector);
> > > +		if (!ret)
> > > +			status = connector_status_disconnected;
> > > +	}
> > >  out:
> > >  	intel_display_power_put(to_i915(dev), power_domain);
> > >  	return status;
> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > > b/drivers/gpu/drm/i915/intel_drv.h
> > > index 5912955..3cfab20 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -1025,6 +1025,8 @@ void intel_ddi_clock_get(struct intel_encoder
> > > *encoder,
> > >  			 struct intel_crtc_state *pipe_config);
> > >  void intel_ddi_set_vc_payload_alloc(struct drm_crtc *crtc, bool state);
> > >  uint32_t ddi_signal_levels(struct intel_dp *intel_dp);
> > > +bool intel_ddi_upfront_link_train(struct intel_dp *intel_dp,
> > > +				struct intel_crtc *crtc);
> > > 
> > >  /* intel_frontbuffer.c */
> > >  void intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx