All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCHv6 0/5] Add USB typeC based DP support for BXT platform
@ 2016-05-20  8:58 Durgadoss R
  2016-05-20  8:58 ` [PATCH 1/5] drm/i915: Don't pass crtc_state to intel_dp_set_link_params() Durgadoss R
                   ` (6 more replies)
  0 siblings, 7 replies; 16+ messages in thread
From: Durgadoss R @ 2016-05-20  8:58 UTC (permalink / raw)
  To: intel-gfx; +Cc: ander.conselvan.de.oliveira

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.

Patch   1-3/5 : Refactor ddi_pll code so that upfront link
                training does not need crtc_state.
Patch   4/5 :   Split ddi_pll_select to be able to calculate
                pll config for DP during upfront link train.
Patch   5/5 :   Upfront implementation for BXT platform.

Changes from v5:
* Moved the retry logic from upfront code to intel_dp.c
Changes from v4:
* Don't use crtc_state in upfront link train
* Use separate variables to track max lane count
  and link bw obtained through upfront link train
Changes from v3:
* Used an earlier patch from Ander that splits ddi_pll_select
  to be able to use it from upfront link train logic.
Changes from v2:
* Rebased on top of intel_dpll_mgr.c code.
* Use drm_atomic_commit instead of atomic_helper_dpms, since the
  later uses crtc->acquire_ctx used also in legacy paths.
* Using a drm_atomic_state helps in using the shared_dpll APIs
  as is, thus keeping the upfront code away from changing pll
  internals directly.
* Fixed locking/retry/backoff logic in upfront according to
  atomic implementation.
Changes from v1:
* Using atomic_helper_dpms() to do DPMS off for upfront link
  training, instead of using load detect functions.
* Made intel_get_shared_dpll handle non-atomic paths by
  duplicating the required shared_dpll_config and then working
  on top of it. This helps in upfront link train code not
  directly touch the 'pll/pll->config' internals.
* Fixed the link_bw update logic in intel_dp_get_link_retry_params()
  for non-HBR2 panels.
* As per comments on earlier version, merged few patches
  (that added new functions) with the upfront link train patch,
  to facilitate easy review.

Link for v5:    https://patchwork.freedesktop.org/patch/87327/
Link for v4:    https://patchwork.freedesktop.org/patch/81656/
Link for v3:    https://patchwork.freedesktop.org/patch/79792/
Link for v2:    https://patchwork.freedesktop.org/patch/72207/
Link for v1:    https://patchwork.freedesktop.org/patch/67784/
Link for RFCv2: https://patchwork.freedesktop.org/patch/61776/
Link for RFCv1: https://patchwork.freedesktop.org/patch/59589/

Ander Conselvan de Oliveira (3):
  drm/i915: Don't pass crtc_state to intel_dp_set_link_params()
  drm/i915: Remove ddi_pll_sel from intel_crtc_state
  drm/i915: Split intel_ddi_pre_enable() into DP and HDMI versions

Durgadoss R (2):
  drm/i915: Split bxt_ddi_pll_select()
  drm/i915/dp: Enable Upfront link training for typeC DP support on BXT

 drivers/gpu/drm/i915/intel_ddi.c      | 150 +++++++++++++++++++-------
 drivers/gpu/drm/i915/intel_display.c  |  43 ++------
 drivers/gpu/drm/i915/intel_dp.c       | 188 +++++++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/intel_dp_mst.c   |   7 +-
 drivers/gpu/drm/i915/intel_dpll_mgr.c | 192 ++++++++++++++++++----------------
 drivers/gpu/drm/i915/intel_dpll_mgr.h |   3 +
 drivers/gpu/drm/i915/intel_drv.h      |  12 ++-
 7 files changed, 420 insertions(+), 175 deletions(-)

-- 
1.9.1

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

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

* [PATCH 1/5] drm/i915: Don't pass crtc_state to intel_dp_set_link_params()
  2016-05-20  8:58 [PATCHv6 0/5] Add USB typeC based DP support for BXT platform Durgadoss R
@ 2016-05-20  8:58 ` Durgadoss R
  2016-05-20  8:58 ` [PATCH 2/5] drm/i915: Remove ddi_pll_sel from intel_crtc_state Durgadoss R
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Durgadoss R @ 2016-05-20  8:58 UTC (permalink / raw)
  To: intel-gfx; +Cc: ander.conselvan.de.oliveira

From: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>

Decouple intel_dp_set_link_params() from struct intel_crtc_state. This
will be useful for implementing DP upfront link training.

Reviewed-by: Durgadoss R <durgadoss.r@intel.com>
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c    | 3 ++-
 drivers/gpu/drm/i915/intel_dp.c     | 9 +++++----
 drivers/gpu/drm/i915/intel_dp_mst.c | 4 +++-
 drivers/gpu/drm/i915/intel_drv.h    | 2 +-
 4 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index c454744..1cbdd66 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1619,7 +1619,8 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder)
 	if (type == INTEL_OUTPUT_DISPLAYPORT || type == INTEL_OUTPUT_EDP) {
 		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
 
-		intel_dp_set_link_params(intel_dp, crtc->config);
+		intel_dp_set_link_params(intel_dp, crtc->config->port_clock,
+					 crtc->config->lane_count);
 
 		intel_ddi_init_dp_buf_reg(intel_encoder);
 
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 3633002..95ba5aa 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1584,10 +1584,10 @@ found:
 }
 
 void intel_dp_set_link_params(struct intel_dp *intel_dp,
-			      const struct intel_crtc_state *pipe_config)
+			      int link_rate, uint8_t lane_count)
 {
-	intel_dp->link_rate = pipe_config->port_clock;
-	intel_dp->lane_count = pipe_config->lane_count;
+	intel_dp->link_rate = link_rate;
+	intel_dp->lane_count = lane_count;
 }
 
 static void intel_dp_prepare(struct intel_encoder *encoder)
@@ -1599,7 +1599,8 @@ static void intel_dp_prepare(struct intel_encoder *encoder)
 	struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
 	const struct drm_display_mode *adjusted_mode = &crtc->config->base.adjusted_mode;
 
-	intel_dp_set_link_params(intel_dp, crtc->config);
+	intel_dp_set_link_params(intel_dp, crtc->config->port_clock,
+				 crtc->config->lane_count);
 
 	/*
 	 * There are four kinds of DP registers:
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index 7a34090..553a25e 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -175,7 +175,9 @@ static void intel_mst_pre_enable_dp(struct intel_encoder *encoder)
 
 		intel_ddi_clk_select(&intel_dig_port->base, intel_crtc->config);
 
-		intel_dp_set_link_params(intel_dp, intel_crtc->config);
+		intel_dp_set_link_params(intel_dp,
+					 intel_crtc->config->port_clock,
+					 intel_crtc->config->lane_count);
 
 		intel_ddi_init_dp_buf_reg(&intel_dig_port->base);
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 0741b2d..91fee9f 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1320,7 +1320,7 @@ void intel_dp_init(struct drm_device *dev, i915_reg_t output_reg, enum port port
 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);
+				int link_rate, uint8_t lane_count);
 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
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [PATCH 2/5] drm/i915: Remove ddi_pll_sel from intel_crtc_state
  2016-05-20  8:58 [PATCHv6 0/5] Add USB typeC based DP support for BXT platform Durgadoss R
  2016-05-20  8:58 ` [PATCH 1/5] drm/i915: Don't pass crtc_state to intel_dp_set_link_params() Durgadoss R
@ 2016-05-20  8:58 ` Durgadoss R
  2016-05-24  7:32   ` [PATCH v2] " Ander Conselvan de Oliveira
  2016-05-20  8:59 ` [PATCH 3/5] drm/i915: Split intel_ddi_pre_enable() into DP and HDMI versions Durgadoss R
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 16+ messages in thread
From: Durgadoss R @ 2016-05-20  8:58 UTC (permalink / raw)
  To: intel-gfx; +Cc: ander.conselvan.de.oliveira

From: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>

The value of ddi_pll_sel is derived from the selection of shared dpll,
so just calculate the final value when necessary.

Reviewed-by: Durgadoss R <durgadoss.r@intel.com>
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c      | 33 ++++++++++++++++++++++-----
 drivers/gpu/drm/i915/intel_display.c  | 43 +++++++----------------------------
 drivers/gpu/drm/i915/intel_dp_mst.c   |  3 ++-
 drivers/gpu/drm/i915/intel_dpll_mgr.c | 27 ----------------------
 drivers/gpu/drm/i915/intel_drv.h      |  2 +-
 5 files changed, 38 insertions(+), 70 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 1cbdd66..65485ef 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1567,14 +1567,36 @@ uint32_t ddi_signal_levels(struct intel_dp *intel_dp)
 	return DDI_BUF_TRANS_SELECT(level);
 }
 
+static uint32_t hsw_pll_to_ddi_pll_sel(struct intel_shared_dpll *pll)
+{
+	switch (pll->id) {
+	case DPLL_ID_WRPLL1:
+		return PORT_CLK_SEL_WRPLL1;
+	case DPLL_ID_WRPLL2:
+		return PORT_CLK_SEL_WRPLL2;
+	case DPLL_ID_SPLL:
+		return PORT_CLK_SEL_SPLL;
+	case DPLL_ID_LCPLL_810:
+		return PORT_CLK_SEL_LCPLL_810;
+	case DPLL_ID_LCPLL_1350:
+		return PORT_CLK_SEL_LCPLL_1350;
+	case DPLL_ID_LCPLL_2700:
+		return PORT_CLK_SEL_LCPLL_2700;
+	default:
+		return PORT_CLK_SEL_NONE;
+	}
+}
+
 void intel_ddi_clk_select(struct intel_encoder *encoder,
-			  const struct intel_crtc_state *pipe_config)
+			  struct intel_shared_dpll *pll)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	enum port port = intel_ddi_get_encoder_port(encoder);
 
+	if (WARN_ON(!pll))
+		return;
+
 	if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) {
-		uint32_t dpll = pipe_config->ddi_pll_sel;
 		uint32_t val;
 
 		/* DDI -> PLL mapping  */
@@ -1582,14 +1604,13 @@ void intel_ddi_clk_select(struct intel_encoder *encoder,
 
 		val &= ~(DPLL_CTRL2_DDI_CLK_OFF(port) |
 			DPLL_CTRL2_DDI_CLK_SEL_MASK(port));
-		val |= (DPLL_CTRL2_DDI_CLK_SEL(dpll, port) |
+		val |= (DPLL_CTRL2_DDI_CLK_SEL(pll->id, port) |
 			DPLL_CTRL2_DDI_SEL_OVERRIDE(port));
 
 		I915_WRITE(DPLL_CTRL2, val);
 
 	} else if (INTEL_INFO(dev_priv)->gen < 9) {
-		WARN_ON(pipe_config->ddi_pll_sel == PORT_CLK_SEL_NONE);
-		I915_WRITE(PORT_CLK_SEL(port), pipe_config->ddi_pll_sel);
+		I915_WRITE(PORT_CLK_SEL(port), hsw_pll_to_ddi_pll_sel(pll));
 	}
 }
 
@@ -1614,7 +1635,7 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder)
 		intel_edp_panel_on(intel_dp);
 	}
 
-	intel_ddi_clk_select(intel_encoder, crtc->config);
+	intel_ddi_clk_select(intel_encoder, crtc->config->shared_dpll);
 
 	if (type == INTEL_OUTPUT_DISPLAYPORT || type == INTEL_OUTPUT_EDP) {
 		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a500f08..0251841 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9669,15 +9669,12 @@ static void bxt_get_ddi_pll(struct drm_i915_private *dev_priv,
 
 	switch (port) {
 	case PORT_A:
-		pipe_config->ddi_pll_sel = SKL_DPLL0;
 		id = DPLL_ID_SKL_DPLL0;
 		break;
 	case PORT_B:
-		pipe_config->ddi_pll_sel = SKL_DPLL1;
 		id = DPLL_ID_SKL_DPLL1;
 		break;
 	case PORT_C:
-		pipe_config->ddi_pll_sel = SKL_DPLL2;
 		id = DPLL_ID_SKL_DPLL2;
 		break;
 	default:
@@ -9696,25 +9693,10 @@ static void skylake_get_ddi_pll(struct drm_i915_private *dev_priv,
 	u32 temp;
 
 	temp = I915_READ(DPLL_CTRL2) & DPLL_CTRL2_DDI_CLK_SEL_MASK(port);
-	pipe_config->ddi_pll_sel = temp >> (port * 3 + 1);
+	id = temp >> (port * 3 + 1);
 
-	switch (pipe_config->ddi_pll_sel) {
-	case SKL_DPLL0:
-		id = DPLL_ID_SKL_DPLL0;
-		break;
-	case SKL_DPLL1:
-		id = DPLL_ID_SKL_DPLL1;
-		break;
-	case SKL_DPLL2:
-		id = DPLL_ID_SKL_DPLL2;
-		break;
-	case SKL_DPLL3:
-		id = DPLL_ID_SKL_DPLL3;
-		break;
-	default:
-		MISSING_CASE(pipe_config->ddi_pll_sel);
+	if (WARN_ON(id < SKL_DPLL0 || id > SKL_DPLL3))
 		return;
-	}
 
 	pipe_config->shared_dpll = intel_get_shared_dpll_by_id(dev_priv, id);
 }
@@ -9724,10 +9706,9 @@ static void haswell_get_ddi_pll(struct drm_i915_private *dev_priv,
 				struct intel_crtc_state *pipe_config)
 {
 	enum intel_dpll_id id;
+	uint32_t ddi_pll_sel = I915_READ(PORT_CLK_SEL(port));
 
-	pipe_config->ddi_pll_sel = I915_READ(PORT_CLK_SEL(port));
-
-	switch (pipe_config->ddi_pll_sel) {
+	switch (ddi_pll_sel) {
 	case PORT_CLK_SEL_WRPLL1:
 		id = DPLL_ID_WRPLL1;
 		break;
@@ -9747,7 +9728,7 @@ static void haswell_get_ddi_pll(struct drm_i915_private *dev_priv,
 		id = DPLL_ID_LCPLL_2700;
 		break;
 	default:
-		MISSING_CASE(pipe_config->ddi_pll_sel);
+		MISSING_CASE(ddi_pll_sel);
 		/* fall through */
 	case PORT_CLK_SEL_NONE:
 		return;
@@ -11484,10 +11465,9 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
 	DRM_DEBUG_KMS("double wide: %i\n", pipe_config->double_wide);
 
 	if (IS_BROXTON(dev)) {
-		DRM_DEBUG_KMS("ddi_pll_sel: %u; dpll_hw_state: ebb0: 0x%x, ebb4: 0x%x,"
+		DRM_DEBUG_KMS("dpll_hw_state: ebb0: 0x%x, ebb4: 0x%x,"
 			      "pll0: 0x%x, pll1: 0x%x, pll2: 0x%x, pll3: 0x%x, "
 			      "pll6: 0x%x, pll8: 0x%x, pll9: 0x%x, pll10: 0x%x, pcsdw12: 0x%x\n",
-			      pipe_config->ddi_pll_sel,
 			      pipe_config->dpll_hw_state.ebb0,
 			      pipe_config->dpll_hw_state.ebb4,
 			      pipe_config->dpll_hw_state.pll0,
@@ -11500,15 +11480,13 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
 			      pipe_config->dpll_hw_state.pll10,
 			      pipe_config->dpll_hw_state.pcsdw12);
 	} else if (IS_SKYLAKE(dev) || IS_KABYLAKE(dev)) {
-		DRM_DEBUG_KMS("ddi_pll_sel: %u; dpll_hw_state: "
+		DRM_DEBUG_KMS("dpll_hw_state: "
 			      "ctrl1: 0x%x, cfgcr1: 0x%x, cfgcr2: 0x%x\n",
-			      pipe_config->ddi_pll_sel,
 			      pipe_config->dpll_hw_state.ctrl1,
 			      pipe_config->dpll_hw_state.cfgcr1,
 			      pipe_config->dpll_hw_state.cfgcr2);
 	} else if (HAS_DDI(dev)) {
-		DRM_DEBUG_KMS("ddi_pll_sel: 0x%x; dpll_hw_state: wrpll: 0x%x spll: 0x%x\n",
-			      pipe_config->ddi_pll_sel,
+		DRM_DEBUG_KMS("dpll_hw_state: wrpll: 0x%x spll: 0x%x\n",
 			      pipe_config->dpll_hw_state.wrpll,
 			      pipe_config->dpll_hw_state.spll);
 	} else {
@@ -11611,7 +11589,6 @@ clear_intel_crtc_state(struct intel_crtc_state *crtc_state)
 	struct intel_crtc_scaler_state scaler_state;
 	struct intel_dpll_hw_state dpll_hw_state;
 	struct intel_shared_dpll *shared_dpll;
-	uint32_t ddi_pll_sel;
 	bool force_thru;
 
 	/* FIXME: before the switch to atomic started, a new pipe_config was
@@ -11623,7 +11600,6 @@ clear_intel_crtc_state(struct intel_crtc_state *crtc_state)
 	scaler_state = crtc_state->scaler_state;
 	shared_dpll = crtc_state->shared_dpll;
 	dpll_hw_state = crtc_state->dpll_hw_state;
-	ddi_pll_sel = crtc_state->ddi_pll_sel;
 	force_thru = crtc_state->pch_pfit.force_thru;
 
 	memset(crtc_state, 0, sizeof *crtc_state);
@@ -11632,7 +11608,6 @@ clear_intel_crtc_state(struct intel_crtc_state *crtc_state)
 	crtc_state->scaler_state = scaler_state;
 	crtc_state->shared_dpll = shared_dpll;
 	crtc_state->dpll_hw_state = dpll_hw_state;
-	crtc_state->ddi_pll_sel = ddi_pll_sel;
 	crtc_state->pch_pfit.force_thru = force_thru;
 }
 
@@ -12043,8 +12018,6 @@ intel_pipe_config_compare(struct drm_device *dev,
 
 	PIPE_CONF_CHECK_I(double_wide);
 
-	PIPE_CONF_CHECK_X(ddi_pll_sel);
-
 	PIPE_CONF_CHECK_P(shared_dpll);
 	PIPE_CONF_CHECK_X(dpll_hw_state.dpll);
 	PIPE_CONF_CHECK_X(dpll_hw_state.dpll_md);
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index 553a25e..ad1913f 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -173,7 +173,8 @@ static void intel_mst_pre_enable_dp(struct intel_encoder *encoder)
 	if (intel_dp->active_mst_links == 0) {
 		intel_prepare_ddi_buffer(&intel_dig_port->base);
 
-		intel_ddi_clk_select(&intel_dig_port->base, intel_crtc->config);
+		intel_ddi_clk_select(&intel_dig_port->base,
+				     intel_crtc->config->shared_dpll);
 
 		intel_dp_set_link_params(intel_dp,
 					 intel_crtc->config->port_clock,
diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index c283ba4..70b3c12 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -449,26 +449,6 @@ static bool hsw_ddi_spll_get_hw_state(struct drm_i915_private *dev_priv,
 	return val & SPLL_PLL_ENABLE;
 }
 
-static uint32_t hsw_pll_to_ddi_pll_sel(struct intel_shared_dpll *pll)
-{
-	switch (pll->id) {
-	case DPLL_ID_WRPLL1:
-		return PORT_CLK_SEL_WRPLL1;
-	case DPLL_ID_WRPLL2:
-		return PORT_CLK_SEL_WRPLL2;
-	case DPLL_ID_SPLL:
-		return PORT_CLK_SEL_SPLL;
-	case DPLL_ID_LCPLL_810:
-		return PORT_CLK_SEL_LCPLL_810;
-	case DPLL_ID_LCPLL_1350:
-		return PORT_CLK_SEL_LCPLL_1350;
-	case DPLL_ID_LCPLL_2700:
-		return PORT_CLK_SEL_LCPLL_2700;
-	default:
-		return PORT_CLK_SEL_NONE;
-	}
-}
-
 #define LC_FREQ 2700
 #define LC_FREQ_2K U64_C(LC_FREQ * 2000)
 
@@ -748,8 +728,6 @@ hsw_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
 	if (!pll)
 		return NULL;
 
-	crtc_state->ddi_pll_sel = hsw_pll_to_ddi_pll_sel(pll);
-
 	intel_reference_shared_dpll(pll, crtc_state);
 
 	return pll;
@@ -1270,8 +1248,6 @@ skl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
 	if (!pll)
 		return NULL;
 
-	crtc_state->ddi_pll_sel = pll->id;
-
 	intel_reference_shared_dpll(pll, crtc_state);
 
 	return pll;
@@ -1618,9 +1594,6 @@ bxt_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
 
 	intel_reference_shared_dpll(pll, crtc_state);
 
-	/* shared DPLL id 0 is DPLL A */
-	crtc_state->ddi_pll_sel = pll->id;
-
 	return pll;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 91fee9f..bcf570f 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1086,7 +1086,7 @@ void intel_crt_init(struct drm_device *dev);
 
 /* intel_ddi.c */
 void intel_ddi_clk_select(struct intel_encoder *encoder,
-			  const struct intel_crtc_state *pipe_config);
+			  struct intel_shared_dpll *pll);
 void intel_prepare_ddi_buffer(struct intel_encoder *encoder);
 void hsw_fdi_link_train(struct drm_crtc *crtc);
 void intel_ddi_init(struct drm_device *dev, enum port port);
-- 
1.9.1

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

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

* [PATCH 3/5] drm/i915: Split intel_ddi_pre_enable() into DP and HDMI versions
  2016-05-20  8:58 [PATCHv6 0/5] Add USB typeC based DP support for BXT platform Durgadoss R
  2016-05-20  8:58 ` [PATCH 1/5] drm/i915: Don't pass crtc_state to intel_dp_set_link_params() Durgadoss R
  2016-05-20  8:58 ` [PATCH 2/5] drm/i915: Remove ddi_pll_sel from intel_crtc_state Durgadoss R
@ 2016-05-20  8:59 ` Durgadoss R
  2016-05-20  8:59 ` [PATCHv2 4/5] drm/i915: Split bxt_ddi_pll_select() Durgadoss R
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Durgadoss R @ 2016-05-20  8:59 UTC (permalink / raw)
  To: intel-gfx; +Cc: ander.conselvan.de.oliveira

From: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>

Split intel_ddi_pre_enable() into encoder type specific versions that
don't depend on crtc_state. The necessary parameters are passed as
function arguments. This split will be necessary for implementing DP
upfront link training.

Reviewed-by: Durgadoss R <durgadoss.r@intel.com>
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 75 +++++++++++++++++++++++-----------------
 1 file changed, 44 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 65485ef..7e6331a 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1614,48 +1614,61 @@ void intel_ddi_clk_select(struct intel_encoder *encoder,
 	}
 }
 
-static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder)
+static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
+				int link_rate, uint32_t lane_count,
+				struct intel_shared_dpll *pll)
 {
-	struct drm_encoder *encoder = &intel_encoder->base;
-	struct drm_i915_private *dev_priv = to_i915(encoder->dev);
-	struct intel_crtc *crtc = to_intel_crtc(encoder->crtc);
-	enum port port = intel_ddi_get_encoder_port(intel_encoder);
-	int type = intel_encoder->type;
+	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+	enum port port = intel_ddi_get_encoder_port(encoder);
 
-	if (type == INTEL_OUTPUT_HDMI) {
-		struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
+	intel_prepare_ddi_buffer(encoder);
 
-		intel_dp_dual_mode_set_tmds_output(intel_hdmi, true);
-	}
+	if (encoder->type == INTEL_OUTPUT_EDP)
+		intel_edp_panel_on(intel_dp);
 
-	intel_prepare_ddi_buffer(intel_encoder);
+	intel_ddi_clk_select(encoder, pll);
 
-	if (type == INTEL_OUTPUT_EDP) {
-		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
-		intel_edp_panel_on(intel_dp);
-	}
+	intel_dp_set_link_params(intel_dp, link_rate, lane_count);
+	intel_ddi_init_dp_buf_reg(encoder);
 
-	intel_ddi_clk_select(intel_encoder, crtc->config->shared_dpll);
+	intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
+	intel_dp_start_link_train(intel_dp);
+	if (port != PORT_A || INTEL_INFO(dev_priv)->gen >= 9)
+		intel_dp_stop_link_train(intel_dp);
+}
 
-	if (type == INTEL_OUTPUT_DISPLAYPORT || type == INTEL_OUTPUT_EDP) {
-		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
+static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
+					bool has_hdmi_sink,
+					struct drm_display_mode *adjusted_mode,
+					struct intel_shared_dpll *pll)
+{
+	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
+	struct drm_encoder *drm_encoder = &encoder->base;
 
-		intel_dp_set_link_params(intel_dp, crtc->config->port_clock,
-					 crtc->config->lane_count);
+	intel_dp_dual_mode_set_tmds_output(intel_hdmi, true);
+	intel_prepare_ddi_buffer(encoder);
+	intel_ddi_clk_select(encoder, pll);
+	intel_hdmi->set_infoframes(drm_encoder, has_hdmi_sink, adjusted_mode);
+}
 
-		intel_ddi_init_dp_buf_reg(intel_encoder);
+static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder)
+{
+	struct drm_encoder *encoder = &intel_encoder->base;
+	struct intel_crtc *crtc = to_intel_crtc(encoder->crtc);
+	int type = intel_encoder->type;
 
-		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
-		intel_dp_start_link_train(intel_dp);
-		if (port != PORT_A || INTEL_INFO(dev_priv)->gen >= 9)
-			intel_dp_stop_link_train(intel_dp);
-	} else if (type == INTEL_OUTPUT_HDMI) {
-		struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
+	if (type == INTEL_OUTPUT_DISPLAYPORT || type == INTEL_OUTPUT_EDP)
+		intel_ddi_pre_enable_dp(intel_encoder,
+					crtc->config->port_clock,
+					crtc->config->lane_count,
+					crtc->config->shared_dpll);
 
-		intel_hdmi->set_infoframes(encoder,
-					   crtc->config->has_hdmi_sink,
-					   &crtc->config->base.adjusted_mode);
-	}
+	if (type == INTEL_OUTPUT_HDMI)
+		intel_ddi_pre_enable_hdmi(intel_encoder,
+					crtc->config->has_hdmi_sink,
+					&crtc->config->base.adjusted_mode,
+					crtc->config->shared_dpll);
 }
 
 static void intel_ddi_post_disable(struct intel_encoder *intel_encoder)
-- 
1.9.1

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

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

* [PATCHv2 4/5] drm/i915: Split bxt_ddi_pll_select()
  2016-05-20  8:58 [PATCHv6 0/5] Add USB typeC based DP support for BXT platform Durgadoss R
                   ` (2 preceding siblings ...)
  2016-05-20  8:59 ` [PATCH 3/5] drm/i915: Split intel_ddi_pre_enable() into DP and HDMI versions Durgadoss R
@ 2016-05-20  8:59 ` Durgadoss R
  2016-05-20  8:59 ` [PATCHv6 5/5] drm/i915/dp: Enable Upfront link training for typeC DP support on BXT Durgadoss R
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Durgadoss R @ 2016-05-20  8:59 UTC (permalink / raw)
  To: intel-gfx; +Cc: ander.conselvan.de.oliveira

Split out of bxt_ddi_pll_select() the logic that calculates the pll
dividers and dpll_hw_state into a new function that doesn't depend on
crtc state. This will be used for enabling the port pll when doing
upfront link training.

v2:
* Refactored code so that bxt_clk_div need not be exported (Durga)
v1:
* Rebased on top of intel_dpll_mgr.c (Durga)
* Initial version from Ander on top of intel_ddi.c

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
---
 drivers/gpu/drm/i915/intel_dpll_mgr.c | 165 +++++++++++++++++++++-------------
 drivers/gpu/drm/i915/intel_dpll_mgr.h |   3 +
 2 files changed, 104 insertions(+), 64 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index 70b3c12..be56816 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -1456,6 +1456,8 @@ struct bxt_clk_div {
 	uint32_t m2_frac;
 	bool m2_frac_en;
 	uint32_t n;
+
+	int vco;
 };
 
 /* pre-calculated values for DP linkrates */
@@ -1469,57 +1471,60 @@ static const struct bxt_clk_div bxt_dp_clk_val[] = {
 	{432000, 3, 1, 32, 1677722, 1, 1}
 };
 
-static struct intel_shared_dpll *
-bxt_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
-	     struct intel_encoder *encoder)
+static bool
+bxt_ddi_hdmi_pll_dividers(struct intel_crtc *intel_crtc,
+			  struct intel_crtc_state *crtc_state, int clock,
+			  struct bxt_clk_div *clk_div)
 {
-	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-	struct intel_shared_dpll *pll;
-	enum intel_dpll_id i;
-	struct intel_digital_port *intel_dig_port;
-	struct bxt_clk_div clk_div = {0};
-	int vco = 0;
-	uint32_t prop_coef, int_coef, gain_ctl, targ_cnt;
-	uint32_t lanestagger;
-	int clock = crtc_state->port_clock;
+	struct dpll best_clock;
 
-	if (encoder->type == INTEL_OUTPUT_HDMI) {
-		struct dpll best_clock;
+	/* Calculate HDMI div */
+	/*
+	 * FIXME: tie the following calculation into
+	 * i9xx_crtc_compute_clock
+	 */
+	if (!bxt_find_best_dpll(crtc_state, clock, &best_clock)) {
+		DRM_DEBUG_DRIVER("no PLL dividers found for clock %d pipe %c\n",
+				 clock, pipe_name(intel_crtc->pipe));
+		return false;
+	}
 
-		/* Calculate HDMI div */
-		/*
-		 * FIXME: tie the following calculation into
-		 * i9xx_crtc_compute_clock
-		 */
-		if (!bxt_find_best_dpll(crtc_state, clock, &best_clock)) {
-			DRM_DEBUG_DRIVER("no PLL dividers found for clock %d pipe %c\n",
-					 clock, pipe_name(crtc->pipe));
-			return NULL;
-		}
+	clk_div->p1 = best_clock.p1;
+	clk_div->p2 = best_clock.p2;
+	WARN_ON(best_clock.m1 != 2);
+	clk_div->n = best_clock.n;
+	clk_div->m2_int = best_clock.m2 >> 22;
+	clk_div->m2_frac = best_clock.m2 & ((1 << 22) - 1);
+	clk_div->m2_frac_en = clk_div->m2_frac != 0;
 
-		clk_div.p1 = best_clock.p1;
-		clk_div.p2 = best_clock.p2;
-		WARN_ON(best_clock.m1 != 2);
-		clk_div.n = best_clock.n;
-		clk_div.m2_int = best_clock.m2 >> 22;
-		clk_div.m2_frac = best_clock.m2 & ((1 << 22) - 1);
-		clk_div.m2_frac_en = clk_div.m2_frac != 0;
+	clk_div->vco = best_clock.vco;
 
-		vco = best_clock.vco;
-	} else if (encoder->type == INTEL_OUTPUT_DISPLAYPORT ||
-		   encoder->type == INTEL_OUTPUT_EDP) {
-		int i;
+	return true;
+}
 
-		clk_div = bxt_dp_clk_val[0];
-		for (i = 0; i < ARRAY_SIZE(bxt_dp_clk_val); ++i) {
-			if (bxt_dp_clk_val[i].clock == clock) {
-				clk_div = bxt_dp_clk_val[i];
-				break;
-			}
+static void bxt_ddi_dp_pll_dividers(int clock, struct bxt_clk_div *clk_div)
+{
+	int i;
+
+	*clk_div = bxt_dp_clk_val[0];
+	for (i = 0; i < ARRAY_SIZE(bxt_dp_clk_val); ++i) {
+		if (bxt_dp_clk_val[i].clock == clock) {
+			*clk_div = bxt_dp_clk_val[i];
+			break;
 		}
-		vco = clock * 10 / 2 * clk_div.p1 * clk_div.p2;
 	}
 
+	clk_div->vco = clock * 10 / 2 * clk_div->p1 * clk_div->p2;
+}
+
+static bool bxt_ddi_set_dpll_hw_state(int clock,
+			  struct bxt_clk_div *clk_div,
+			  struct intel_dpll_hw_state *dpll_hw_state)
+{
+	int vco = clk_div->vco;
+	uint32_t prop_coef, int_coef, gain_ctl, targ_cnt;
+	uint32_t lanestagger;
+
 	if (vco >= 6200000 && vco <= 6700000) {
 		prop_coef = 4;
 		int_coef = 9;
@@ -1538,12 +1543,9 @@ bxt_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
 		targ_cnt = 9;
 	} else {
 		DRM_ERROR("Invalid VCO\n");
-		return NULL;
+		return false;
 	}
 
-	memset(&crtc_state->dpll_hw_state, 0,
-	       sizeof(crtc_state->dpll_hw_state));
-
 	if (clock > 270000)
 		lanestagger = 0x18;
 	else if (clock > 135000)
@@ -1555,33 +1557,68 @@ bxt_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
 	else
 		lanestagger = 0x02;
 
-	crtc_state->dpll_hw_state.ebb0 =
-		PORT_PLL_P1(clk_div.p1) | PORT_PLL_P2(clk_div.p2);
-	crtc_state->dpll_hw_state.pll0 = clk_div.m2_int;
-	crtc_state->dpll_hw_state.pll1 = PORT_PLL_N(clk_div.n);
-	crtc_state->dpll_hw_state.pll2 = clk_div.m2_frac;
+	dpll_hw_state->ebb0 = PORT_PLL_P1(clk_div->p1) | PORT_PLL_P2(clk_div->p2);
+	dpll_hw_state->pll0 = clk_div->m2_int;
+	dpll_hw_state->pll1 = PORT_PLL_N(clk_div->n);
+	dpll_hw_state->pll2 = clk_div->m2_frac;
 
-	if (clk_div.m2_frac_en)
-		crtc_state->dpll_hw_state.pll3 =
-			PORT_PLL_M2_FRAC_ENABLE;
+	if (clk_div->m2_frac_en)
+		dpll_hw_state->pll3 = PORT_PLL_M2_FRAC_ENABLE;
 
-	crtc_state->dpll_hw_state.pll6 =
-		prop_coef | PORT_PLL_INT_COEFF(int_coef);
-	crtc_state->dpll_hw_state.pll6 |=
-		PORT_PLL_GAIN_CTL(gain_ctl);
+	dpll_hw_state->pll6 = prop_coef | PORT_PLL_INT_COEFF(int_coef);
+	dpll_hw_state->pll6 |= PORT_PLL_GAIN_CTL(gain_ctl);
 
-	crtc_state->dpll_hw_state.pll8 = targ_cnt;
+	dpll_hw_state->pll8 = targ_cnt;
 
-	crtc_state->dpll_hw_state.pll9 = 5 << PORT_PLL_LOCK_THRESHOLD_SHIFT;
+	dpll_hw_state->pll9 = 5 << PORT_PLL_LOCK_THRESHOLD_SHIFT;
 
-	crtc_state->dpll_hw_state.pll10 =
+	dpll_hw_state->pll10 =
 		PORT_PLL_DCO_AMP(PORT_PLL_DCO_AMP_DEFAULT)
 		| PORT_PLL_DCO_AMP_OVR_EN_H;
 
-	crtc_state->dpll_hw_state.ebb4 = PORT_PLL_10BIT_CLK_ENABLE;
+	dpll_hw_state->ebb4 = PORT_PLL_10BIT_CLK_ENABLE;
+
+	dpll_hw_state->pcsdw12 = LANESTAGGER_STRAP_OVRD | lanestagger;
+
+	return true;
+}
+
+bool bxt_ddi_dp_set_dpll_hw_state(int clock,
+			  struct intel_dpll_hw_state *dpll_hw_state)
+{
+	struct bxt_clk_div clk_div = {0};
+
+	bxt_ddi_dp_pll_dividers(clock, &clk_div);
+
+	return bxt_ddi_set_dpll_hw_state(clock, &clk_div, dpll_hw_state);
+}
+
+static struct intel_shared_dpll *
+bxt_get_dpll(struct intel_crtc *crtc,
+		struct intel_crtc_state *crtc_state,
+		struct intel_encoder *encoder)
+{
+	struct bxt_clk_div clk_div = {0};
+	struct intel_dpll_hw_state dpll_hw_state = {0};
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	struct intel_digital_port *intel_dig_port;
+	struct intel_shared_dpll *pll;
+	int i, clock = crtc_state->port_clock;
+
+	if (encoder->type == INTEL_OUTPUT_HDMI
+	    && !bxt_ddi_hdmi_pll_dividers(crtc, crtc_state,
+					  clock, &clk_div))
+			return false;
+
+	if ((encoder->type == INTEL_OUTPUT_DISPLAYPORT ||
+	    encoder->type == INTEL_OUTPUT_EDP) &&
+		!bxt_ddi_dp_set_dpll_hw_state(clock, &dpll_hw_state))
+			return false;
+
+	memset(&crtc_state->dpll_hw_state, 0,
+	       sizeof(crtc_state->dpll_hw_state));
 
-	crtc_state->dpll_hw_state.pcsdw12 =
-		LANESTAGGER_STRAP_OVRD | lanestagger;
+	crtc_state->dpll_hw_state = dpll_hw_state;
 
 	intel_dig_port = enc_to_dig_port(&encoder->base);
 
diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.h b/drivers/gpu/drm/i915/intel_dpll_mgr.h
index 89c5ada..11a85a5 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.h
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.h
@@ -160,5 +160,8 @@ void intel_disable_shared_dpll(struct intel_crtc *crtc);
 void intel_shared_dpll_commit(struct drm_atomic_state *state);
 void intel_shared_dpll_init(struct drm_device *dev);
 
+/* BXT dpll related functions */
+bool bxt_ddi_dp_set_dpll_hw_state(int clock,
+			  struct intel_dpll_hw_state *dpll_hw_state);
 
 #endif /* _INTEL_DPLL_MGR_H_ */
-- 
1.9.1

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

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

* [PATCHv6 5/5] drm/i915/dp: Enable Upfront link training for typeC DP support on BXT
  2016-05-20  8:58 [PATCHv6 0/5] Add USB typeC based DP support for BXT platform Durgadoss R
                   ` (3 preceding siblings ...)
  2016-05-20  8:59 ` [PATCHv2 4/5] drm/i915: Split bxt_ddi_pll_select() Durgadoss R
@ 2016-05-20  8:59 ` Durgadoss R
  2016-05-24  7:40   ` Ander Conselvan De Oliveira
  2016-05-25 15:35   ` Ville Syrjälä
  2016-05-20  9:41 ` ✗ Ro.CI.BAT: warning for Add USB typeC based DP support for BXT platform (rev7) Patchwork
  2016-05-24  7:56 ` ✗ Ro.CI.BAT: warning for Add USB typeC based DP support for BXT platform (rev8) Patchwork
  6 siblings, 2 replies; 16+ messages in thread
From: Durgadoss R @ 2016-05-20  8:59 UTC (permalink / raw)
  To: intel-gfx; +Cc: ander.conselvan.de.oliveira

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 DP encoder.
* On Connected boot scenarios: When booted with an LFP and a DP,
  sometimes BIOS brings up DP. In these cases, we disable the
  crtc and then do upfront link training; and bring it back up.
* 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.

Changes since v5:
* Moved retry logic in upfront to intel_dp.c so that it
  can be used for all platforms.
Changes since v4:
* Removed usage of crtc_state in upfront link training;
  Hence no need to find free crtc to do upfront now.
* Re-enable crtc if it was disabled for upfront.
* Use separate variables to track max lane count
  and link rate found by upfront, without modifying
  the original DPCD read from panel.
Changes since v3:
* Fixed a return value on BXT check
* Reworked on top of bxt_ddi_pll_select split from Ander
* Renamed from ddi_upfront to bxt_upfront since the
  upfront logic includes BXT specific functions for now.
Changes since v2:
* Rebased on top of latest dpll_mgr.c code and
  latest HPD related clean ups.
* Corrected return values from upfront (Ander)
* Corrected atomic locking for upfront in intel_dp.c (Ville)
Changes since v1:
*  all pll related functions inside ddi.c

Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c |  45 ++++++++++
 drivers/gpu/drm/i915/intel_dp.c  | 179 +++++++++++++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/intel_drv.h |   8 ++
 3 files changed, 226 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 7e6331a..8d224bf 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2330,6 +2330,51 @@ intel_ddi_init_hdmi_connector(struct intel_digital_port *intel_dig_port)
 	return connector;
 }
 
+bool intel_bxt_upfront_link_train(struct intel_dp *intel_dp,
+				int clock, uint8_t lane_count)
+{
+	struct intel_connector *connector = intel_dp->attached_connector;
+	struct intel_encoder *encoder = connector->encoder;
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+	struct intel_shared_dpll *pll;
+	struct intel_shared_dpll_config tmp_pll_config;
+	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
+	enum intel_dpll_id dpll_id = (enum intel_dpll_id)dig_port->port;
+
+	/*
+	 * FIXME: Works only for BXT.
+	 * Select the required PLL. This works for platforms where
+	 * there is no shared DPLL.
+	 */
+	pll = &dev_priv->shared_dplls[dpll_id];
+	if (WARN_ON(pll->active_mask)) {
+		DRM_ERROR("Shared DPLL already in use. active_mask:%x\n", pll->active_mask);
+		return false;
+	}
+
+	tmp_pll_config = pll->config;
+
+	if (!bxt_ddi_dp_set_dpll_hw_state(clock, &pll->config.hw_state)) {
+		DRM_ERROR("Could not setup DPLL\n");
+		pll->config = tmp_pll_config;
+		return false;
+	}
+
+	/* Enable PLL followed by port */
+	pll->funcs.enable(dev_priv, pll);
+	intel_ddi_pre_enable_dp(encoder, clock, lane_count, pll);
+
+	DRM_DEBUG_KMS("Upfront link train %s: link_clock:%d lanes:%d\n",
+	intel_dp->train_set_valid ? "Passed" : "Failed", clock, lane_count);
+
+	/* Disable port followed by PLL for next retry/clean up */
+	intel_ddi_post_disable(encoder);
+	pll->funcs.disable(dev_priv, pll);
+
+	pll->config = tmp_pll_config;
+	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 95ba5aa..6ecaa30 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -147,18 +147,28 @@ intel_dp_max_link_bw(struct intel_dp  *intel_dp)
 		max_link_bw = DP_LINK_BW_1_62;
 		break;
 	}
-	return max_link_bw;
+	/*
+	 * Limit max link bw w.r.t to the max value found
+	 * using Upfront link training also.
+	 */
+	return min(max_link_bw, intel_dp->max_link_bw_upfront);
 }
 
 static u8 intel_dp_max_lane_count(struct intel_dp *intel_dp)
 {
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
-	u8 source_max, sink_max;
+	u8 temp, source_max, sink_max;
 
 	source_max = intel_dig_port->max_lanes;
 	sink_max = drm_dp_max_lane_count(intel_dp->dpcd);
 
-	return min(source_max, sink_max);
+	temp = min(source_max, sink_max);
+
+	/*
+	 * Limit max lanes w.r.t to the max value found
+	 * using Upfront link training also.
+	 */
+	return min(temp, intel_dp->max_lanes_upfront);
 }
 
 /*
@@ -1590,6 +1600,15 @@ void intel_dp_set_link_params(struct intel_dp *intel_dp,
 	intel_dp->lane_count = lane_count;
 }
 
+static void intel_dp_init_upfront_params(struct intel_dp *intel_dp)
+{
+	if (WARN_ON(intel_dp->upfront_done))
+		return;
+
+	intel_dp->max_link_bw_upfront = intel_dp->dpcd[DP_MAX_LINK_RATE];
+	intel_dp->max_lanes_upfront = drm_dp_max_lane_count(intel_dp->dpcd);
+}
+
 static void intel_dp_prepare(struct intel_encoder *encoder)
 {
 	struct drm_device *dev = encoder->base.dev;
@@ -3424,6 +3443,16 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
 		intel_dp->num_sink_rates = i;
 	}
 
+	/*
+	 * The link_bw and lane count vaues are initialized to MAX possible
+	 * value for all encoder types i.e DP, eDP, DP-MST, so that the
+	 * intel_dp_max_{link_bw/lane_count} APIs do not have to worry
+	 * about encoder types. They further cap the max w.r.t the upfront
+	 * values also.
+	 */
+	if (!intel_dp->upfront_done)
+		intel_dp_init_upfront_params(intel_dp);
+
 	intel_dp_print_rates(intel_dp);
 
 	if (!(intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] &
@@ -4140,6 +4169,132 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
 	intel_dp->has_audio = false;
 }
 
+static int intel_dp_upfront_crtc_disable(struct intel_crtc *crtc,
+				struct drm_modeset_acquire_ctx *ctx,
+				bool enable)
+{
+	int ret;
+	struct drm_atomic_state *state;
+	struct intel_crtc_state *crtc_state;
+	struct drm_device *dev = crtc->base.dev;
+	enum pipe pipe = crtc->pipe;
+
+	state = drm_atomic_state_alloc(dev);
+	if (!state)
+		return -ENOMEM;
+
+	state->acquire_ctx = ctx;
+
+	crtc_state = intel_atomic_get_crtc_state(state, crtc);
+	if (IS_ERR(crtc_state)) {
+		ret = PTR_ERR(crtc_state);
+		drm_atomic_state_free(state);
+		return ret;
+	}
+
+	DRM_DEBUG_KMS("%sabling crtc %c %s upfront link train\n",
+			enable ? "En" : "Dis",
+			pipe_name(pipe),
+			enable ? "after" : "before");
+
+	crtc_state->base.active = enable;
+	ret = drm_atomic_commit(state);
+	if (ret)
+		drm_atomic_state_free(state);
+
+	return ret;
+}
+
+static bool __intel_dp_upfront_link_train(struct intel_dp *intel_dp,
+				int clock, uint8_t lane_count)
+{
+	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+	struct drm_i915_private *dev_priv =
+				to_i915(intel_dig_port->base.base.dev);
+
+	if (IS_BROXTON(dev_priv))
+		return intel_bxt_upfront_link_train(intel_dp, clock, lane_count);
+	/* Other platform calls go here */
+
+	/* Return true for unsupported platforms */
+	return true;
+}
+static bool intel_dp_upfront_link_train(struct intel_dp *intel_dp)
+{
+	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_i915_private *dev_priv = to_i915(dev);
+	struct drm_mode_config *config = &dev->mode_config;
+	struct drm_modeset_acquire_ctx ctx;
+	struct intel_crtc *intel_crtc;
+	struct drm_crtc *crtc = NULL;
+	int ret;
+	bool done = false;
+	uint8_t lane_count, max_lanes = intel_dp->max_lanes_upfront;
+	int common_rates[DP_MAX_SUPPORTED_RATES] = {};
+	int clock, common_len;
+
+	common_len = intel_dp_common_rates(intel_dp, common_rates);
+	if (WARN_ON(common_len <= 0))
+		return 0;
+
+	if (!IS_BROXTON(dev))
+		return 0;
+
+	drm_modeset_acquire_init(&ctx, 0);
+retry:
+	ret = drm_modeset_lock(&config->connection_mutex, &ctx);
+	if (ret)
+		goto exit_fail;
+
+	if (intel_encoder->base.crtc) {
+		crtc = intel_encoder->base.crtc;
+
+		ret = drm_modeset_lock(&crtc->mutex, &ctx);
+		if (ret)
+			goto exit_fail;
+
+		ret = drm_modeset_lock(&crtc->primary->mutex, &ctx);
+		if (ret)
+			goto exit_fail;
+
+		intel_crtc = to_intel_crtc(crtc);
+		ret = intel_dp_upfront_crtc_disable(intel_crtc, &ctx, false);
+		if (ret)
+			goto exit_fail;
+	}
+
+	mutex_lock(&dev_priv->dpll_lock);
+
+	for (lane_count = max_lanes; lane_count >= 1 && !done; lane_count >>= 1) {
+		for (clock = common_len - 1; clock >= 0 && !done; clock--) {
+			done = __intel_dp_upfront_link_train(intel_dp,
+					common_rates[clock], lane_count);
+			if (done) {
+				intel_dp->max_lanes_upfront = lane_count;
+				intel_dp->max_link_bw_upfront =
+					drm_dp_link_rate_to_bw_code(common_rates[clock]);
+				break;
+			}
+		}
+	}
+
+	mutex_unlock(&dev_priv->dpll_lock);
+
+	if (crtc)
+		ret = intel_dp_upfront_crtc_disable(intel_crtc, &ctx, true);
+
+exit_fail:
+	if (ret == -EDEADLK) {
+		drm_modeset_backoff(&ctx);
+		goto retry;
+	}
+	drm_modeset_drop_locks(&ctx);
+	drm_modeset_acquire_fini(&ctx);
+	return done;
+}
+
 static void
 intel_dp_long_pulse(struct intel_connector *intel_connector)
 {
@@ -4150,7 +4305,7 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
 	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;
 
 	power_domain = intel_display_port_aux_power_domain(intel_encoder);
@@ -4235,10 +4390,22 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
 			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_dp->upfront_done &&
+		intel_encoder->type == INTEL_OUTPUT_DISPLAYPORT &&
+		intel_dp->compliance_test_type != DP_TEST_LINK_TRAINING;
+
+	if (do_upfront_link_train) {
+		intel_dp->upfront_done = intel_dp_upfront_link_train(intel_dp);
+		if (!intel_dp->upfront_done)
+			status = connector_status_disconnected;
+	}
+
 out:
-	if ((status != connector_status_connected) &&
-	    (intel_dp->is_mst == false))
+	if (status != connector_status_connected && !intel_dp->is_mst) {
 		intel_dp_unset_edid(intel_dp);
+		intel_dp->upfront_done = false;
+	}
 
 	intel_display_power_put(to_i915(dev), power_domain);
 	return;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index bcf570f..060ea9b 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -832,6 +832,12 @@ struct intel_dp {
 	enum hdmi_force_audio force_audio;
 	bool limited_color_range;
 	bool color_range_auto;
+
+	/* Upfront link train parameters */
+	int max_link_bw_upfront;
+	uint8_t max_lanes_upfront;
+	bool upfront_done;
+
 	uint8_t dpcd[DP_RECEIVER_CAP_SIZE];
 	uint8_t psr_dpcd[EDP_PSR_RECEIVER_CAP_SIZE];
 	uint8_t downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
@@ -1113,6 +1119,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_bxt_upfront_link_train(struct intel_dp *intel_dp,
+				int clock, uint8_t lane_count);
 
 /* 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
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* ✗ Ro.CI.BAT: warning for Add USB typeC based DP support for BXT platform (rev7)
  2016-05-20  8:58 [PATCHv6 0/5] Add USB typeC based DP support for BXT platform Durgadoss R
                   ` (4 preceding siblings ...)
  2016-05-20  8:59 ` [PATCHv6 5/5] drm/i915/dp: Enable Upfront link training for typeC DP support on BXT Durgadoss R
@ 2016-05-20  9:41 ` Patchwork
  2016-05-24  7:56 ` ✗ Ro.CI.BAT: warning for Add USB typeC based DP support for BXT platform (rev8) Patchwork
  6 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2016-05-20  9:41 UTC (permalink / raw)
  To: Ander Conselvan de Oliveira; +Cc: intel-gfx

== Series Details ==

Series: Add USB typeC based DP support for BXT platform (rev7)
URL   : https://patchwork.freedesktop.org/series/1731/
State : warning

== Summary ==

Series 1731v7 Add USB typeC based DP support for BXT platform
http://patchwork.freedesktop.org/api/1.0/series/1731/revisions/7/mbox

Test drv_module_reload_basic:
                pass       -> DMESG-WARN (ro-hsw-i7-4770r)
                pass       -> DMESG-WARN (fi-hsw-i7-4770r)
                pass       -> DMESG-WARN (ro-hsw-i3-4010u)
                pass       -> DMESG-WARN (fi-bdw-i7-5557u)
Test gem_exec_suspend:
        Subgroup basic-s3:
                pass       -> DMESG-WARN (ro-hsw-i3-4010u)
                pass       -> DMESG-WARN (ro-bdw-i7-5600u)
                pass       -> DMESG-WARN (fi-hsw-i7-4770r)
                pass       -> DMESG-WARN (fi-skl-i7-6700k)
                pass       -> DMESG-WARN (ro-hsw-i7-4770r)
                pass       -> DMESG-WARN (fi-bdw-i7-5557u)
Test kms_flip:
        Subgroup basic-flip-vs-dpms:
                pass       -> SKIP       (ro-bdw-i7-5557U)
                pass       -> DMESG-WARN (ro-hsw-i3-4010u)
                pass       -> DMESG-WARN (ro-bdw-i7-5600u)
                pass       -> DMESG-WARN (fi-hsw-i7-4770r)
                pass       -> DMESG-WARN (fi-skl-i7-6700k)
                pass       -> DMESG-WARN (ro-hsw-i7-4770r)
                pass       -> DMESG-WARN (fi-bdw-i7-5557u)
        Subgroup basic-flip-vs-modeset:
                pass       -> SKIP       (ro-bdw-i7-5557U)
                pass       -> DMESG-WARN (ro-hsw-i3-4010u)
                pass       -> DMESG-WARN (ro-bdw-i7-5600u)
                pass       -> DMESG-WARN (fi-hsw-i7-4770r)
                pass       -> DMESG-WARN (fi-skl-i7-6700k)
                pass       -> DMESG-WARN (ro-hsw-i7-4770r)
                pass       -> DMESG-WARN (fi-bdw-i7-5557u)
        Subgroup basic-flip-vs-wf_vblank:
                pass       -> SKIP       (ro-bdw-i7-5557U)
                pass       -> DMESG-WARN (ro-hsw-i3-4010u)
                pass       -> DMESG-WARN (ro-bdw-i7-5600u)
                pass       -> DMESG-WARN (fi-hsw-i7-4770r)
                pass       -> DMESG-WARN (fi-skl-i7-6700k)
                pass       -> DMESG-WARN (ro-hsw-i7-4770r)
                pass       -> DMESG-WARN (fi-bdw-i7-5557u)
        Subgroup basic-plain-flip:
                pass       -> SKIP       (ro-bdw-i7-5557U)
                pass       -> DMESG-WARN (ro-hsw-i3-4010u)
                pass       -> DMESG-WARN (ro-bdw-i7-5600u)
                pass       -> DMESG-WARN (fi-hsw-i7-4770r)
                pass       -> DMESG-WARN (fi-skl-i7-6700k)
                pass       -> DMESG-WARN (ro-hsw-i7-4770r)
                pass       -> DMESG-WARN (fi-bdw-i7-5557u)
Test kms_frontbuffer_tracking:
        Subgroup basic:
                pass       -> SKIP       (ro-bdw-i7-5557U)
                pass       -> DMESG-WARN (ro-hsw-i3-4010u)
                fail       -> DMESG-FAIL (ro-bdw-i7-5600u)
                pass       -> DMESG-WARN (fi-hsw-i7-4770r)
                pass       -> DMESG-WARN (fi-skl-i7-6700k)
                pass       -> DMESG-WARN (ro-hsw-i7-4770r)
                pass       -> DMESG-WARN (fi-bdw-i7-5557u)
Test kms_pipe_crc_basic:
        Subgroup hang-read-crc-pipe-a:
                pass       -> SKIP       (ro-bdw-i7-5557U)
                pass       -> DMESG-WARN (ro-hsw-i3-4010u)
                pass       -> DMESG-WARN (ro-bdw-i7-5600u)
                pass       -> DMESG-WARN (fi-hsw-i7-4770r)
                pass       -> DMESG-WARN (fi-skl-i7-6700k)
                pass       -> DMESG-WARN (ro-hsw-i7-4770r)
                pass       -> DMESG-WARN (fi-bdw-i7-5557u)
        Subgroup hang-read-crc-pipe-b:
                pass       -> SKIP       (ro-bdw-i7-5557U)
                pass       -> DMESG-WARN (ro-hsw-i3-4010u)
                pass       -> DMESG-WARN (ro-bdw-i7-5600u)
                pass       -> DMESG-WARN (fi-hsw-i7-4770r)
                pass       -> DMESG-WARN (fi-skl-i7-6700k)
                pass       -> DMESG-WARN (ro-hsw-i7-4770r)
                pass       -> DMESG-WARN (fi-bdw-i7-5557u)
        Subgroup hang-read-crc-pipe-c:
                pass       -> SKIP       (ro-bdw-i7-5557U)
                pass       -> DMESG-WARN (ro-hsw-i3-4010u)
                pass       -> DMESG-WARN (ro-bdw-i7-5600u)
                pass       -> DMESG-WARN (fi-hsw-i7-4770r)
                pass       -> DMESG-WARN (fi-skl-i7-6700k)
                pass       -> DMESG-WARN (ro-hsw-i7-4770r)
                pass       -> DMESG-WARN (fi-bdw-i7-5557u)
        Subgroup nonblocking-crc-pipe-a:
                pass       -> SKIP       (ro-bdw-i7-5557U)
                pass       -> DMESG-WARN (ro-hsw-i3-4010u)
                pass       -> DMESG-WARN (ro-bdw-i7-5600u)
                pass       -> DMESG-WARN (fi-hsw-i7-4770r)
                pass       -> DMESG-WARN (fi-skl-i7-6700k)
                pass       -> DMESG-WARN (ro-hsw-i7-4770r)
                pass       -> DMESG-WARN (fi-bdw-i7-5557u)
        Subgroup nonblocking-crc-pipe-a-frame-sequence:
                pass       -> SKIP       (ro-bdw-i7-5557U)
                pass       -> DMESG-WARN (ro-hsw-i3-4010u)
                pass       -> DMESG-WARN (ro-bdw-i7-5600u)
                pass       -> DMESG-WARN (fi-hsw-i7-4770r)
                pass       -> DMESG-WARN (fi-skl-i7-6700k)
                pass       -> DMESG-WARN (ro-hsw-i7-4770r)
                pass       -> DMESG-WARN (fi-bdw-i7-5557u)
        Subgroup nonblocking-crc-pipe-b:
WARNING: Long output truncated
fi-bsw-n3050 failed to connect after reboot
fi-hsw-i7-4770k failed to connect after reboot
fi-snb-i7-2600 failed to connect after reboot
ro-bsw-n3050 failed to connect after reboot

Results at /archive/results/CI_IGT_test/RO_Patchwork_946/

9d15199 drm-intel-nightly: 2016y-05m-20d-07h-54m-59s UTC integration manifest
28ee239 drm/i915/dp: Enable Upfront link training for typeC DP support on BXT
f0df4fa drm/i915: Split bxt_ddi_pll_select()
2c7fe1b drm/i915: Split intel_ddi_pre_enable() into DP and HDMI versions
6fd4164 drm/i915: Remove ddi_pll_sel from intel_crtc_state
f014956 drm/i915: Don't pass crtc_state to intel_dp_set_link_params()

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

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

* [PATCH v2] drm/i915: Remove ddi_pll_sel from intel_crtc_state
  2016-05-20  8:58 ` [PATCH 2/5] drm/i915: Remove ddi_pll_sel from intel_crtc_state Durgadoss R
@ 2016-05-24  7:32   ` Ander Conselvan de Oliveira
  2016-05-24  7:51     ` R, Durgadoss
  0 siblings, 1 reply; 16+ messages in thread
From: Ander Conselvan de Oliveira @ 2016-05-24  7:32 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ander Conselvan de Oliveira

The value of ddi_pll_sel is derived from the selection of shared dpll,
so just calculate the final value when necessary.

v2: Actually remove it from crtc state and delete remaining usages. (CI)

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c      | 45 ++++++++++++++++++++++++++---------
 drivers/gpu/drm/i915/intel_display.c  | 43 +++++++--------------------------
 drivers/gpu/drm/i915/intel_dp_mst.c   |  3 ++-
 drivers/gpu/drm/i915/intel_dpll_mgr.c | 27 ---------------------
 drivers/gpu/drm/i915/intel_drv.h      |  8 +------
 5 files changed, 45 insertions(+), 81 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 1cbdd66..2ee5c4c 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -531,6 +531,27 @@ static void intel_wait_ddi_buf_idle(struct drm_i915_private *dev_priv,
 	DRM_ERROR("Timeout waiting for DDI BUF %c idle bit\n", port_name(port));
 }
 
+static uint32_t hsw_pll_to_ddi_pll_sel(struct intel_shared_dpll *pll)
+{
+	switch (pll->id) {
+	case DPLL_ID_WRPLL1:
+		return PORT_CLK_SEL_WRPLL1;
+	case DPLL_ID_WRPLL2:
+		return PORT_CLK_SEL_WRPLL2;
+	case DPLL_ID_SPLL:
+		return PORT_CLK_SEL_SPLL;
+	case DPLL_ID_LCPLL_810:
+		return PORT_CLK_SEL_LCPLL_810;
+	case DPLL_ID_LCPLL_1350:
+		return PORT_CLK_SEL_LCPLL_1350;
+	case DPLL_ID_LCPLL_2700:
+		return PORT_CLK_SEL_LCPLL_2700;
+	default:
+		MISSING_CASE(pll->id);
+		return PORT_CLK_SEL_NONE;
+	}
+}
+
 /* Starting with Haswell, different DDI ports can work in FDI mode for
  * connection to the PCH-located connectors. For this, it is necessary to train
  * both the DDI port and PCH receiver for the desired DDI buffer settings.
@@ -546,7 +567,7 @@ void hsw_fdi_link_train(struct drm_crtc *crtc)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct intel_encoder *encoder;
-	u32 temp, i, rx_ctl_val;
+	u32 temp, i, rx_ctl_val, ddi_pll_sel;
 
 	for_each_encoder_on_crtc(dev, crtc, encoder) {
 		WARN_ON(encoder->type != INTEL_OUTPUT_ANALOG);
@@ -577,8 +598,9 @@ void hsw_fdi_link_train(struct drm_crtc *crtc)
 	I915_WRITE(FDI_RX_CTL(PIPE_A), rx_ctl_val);
 
 	/* Configure Port Clock Select */
-	I915_WRITE(PORT_CLK_SEL(PORT_E), intel_crtc->config->ddi_pll_sel);
-	WARN_ON(intel_crtc->config->ddi_pll_sel != PORT_CLK_SEL_SPLL);
+	ddi_pll_sel = hsw_pll_to_ddi_pll_sel(intel_crtc->config->shared_dpll);
+	I915_WRITE(PORT_CLK_SEL(PORT_E), ddi_pll_sel);
+	WARN_ON(ddi_pll_sel != PORT_CLK_SEL_SPLL);
 
 	/* Start the training iterating through available voltages and emphasis,
 	 * testing each value twice. */
@@ -855,7 +877,7 @@ static void skl_ddi_clock_get(struct intel_encoder *encoder,
 	int link_clock = 0;
 	uint32_t dpll_ctl1, dpll;
 
-	dpll = pipe_config->ddi_pll_sel;
+	dpll = intel_get_shared_dpll_id(dev_priv, pipe_config->shared_dpll);
 
 	dpll_ctl1 = I915_READ(DPLL_CTRL1);
 
@@ -903,7 +925,7 @@ static void hsw_ddi_clock_get(struct intel_encoder *encoder,
 	int link_clock = 0;
 	u32 val, pll;
 
-	val = pipe_config->ddi_pll_sel;
+	val = hsw_pll_to_ddi_pll_sel(pipe_config->shared_dpll);
 	switch (val & PORT_CLK_SEL_MASK) {
 	case PORT_CLK_SEL_LCPLL_810:
 		link_clock = 81000;
@@ -1568,13 +1590,15 @@ uint32_t ddi_signal_levels(struct intel_dp *intel_dp)
 }
 
 void intel_ddi_clk_select(struct intel_encoder *encoder,
-			  const struct intel_crtc_state *pipe_config)
+			  struct intel_shared_dpll *pll)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	enum port port = intel_ddi_get_encoder_port(encoder);
 
+	if (WARN_ON(!pll))
+		return;
+
 	if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) {
-		uint32_t dpll = pipe_config->ddi_pll_sel;
 		uint32_t val;
 
 		/* DDI -> PLL mapping  */
@@ -1582,14 +1606,13 @@ void intel_ddi_clk_select(struct intel_encoder *encoder,
 
 		val &= ~(DPLL_CTRL2_DDI_CLK_OFF(port) |
 			DPLL_CTRL2_DDI_CLK_SEL_MASK(port));
-		val |= (DPLL_CTRL2_DDI_CLK_SEL(dpll, port) |
+		val |= (DPLL_CTRL2_DDI_CLK_SEL(pll->id, port) |
 			DPLL_CTRL2_DDI_SEL_OVERRIDE(port));
 
 		I915_WRITE(DPLL_CTRL2, val);
 
 	} else if (INTEL_INFO(dev_priv)->gen < 9) {
-		WARN_ON(pipe_config->ddi_pll_sel == PORT_CLK_SEL_NONE);
-		I915_WRITE(PORT_CLK_SEL(port), pipe_config->ddi_pll_sel);
+		I915_WRITE(PORT_CLK_SEL(port), hsw_pll_to_ddi_pll_sel(pll));
 	}
 }
 
@@ -1614,7 +1637,7 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder)
 		intel_edp_panel_on(intel_dp);
 	}
 
-	intel_ddi_clk_select(intel_encoder, crtc->config);
+	intel_ddi_clk_select(intel_encoder, crtc->config->shared_dpll);
 
 	if (type == INTEL_OUTPUT_DISPLAYPORT || type == INTEL_OUTPUT_EDP) {
 		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a500f08..0251841 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9669,15 +9669,12 @@ static void bxt_get_ddi_pll(struct drm_i915_private *dev_priv,
 
 	switch (port) {
 	case PORT_A:
-		pipe_config->ddi_pll_sel = SKL_DPLL0;
 		id = DPLL_ID_SKL_DPLL0;
 		break;
 	case PORT_B:
-		pipe_config->ddi_pll_sel = SKL_DPLL1;
 		id = DPLL_ID_SKL_DPLL1;
 		break;
 	case PORT_C:
-		pipe_config->ddi_pll_sel = SKL_DPLL2;
 		id = DPLL_ID_SKL_DPLL2;
 		break;
 	default:
@@ -9696,25 +9693,10 @@ static void skylake_get_ddi_pll(struct drm_i915_private *dev_priv,
 	u32 temp;
 
 	temp = I915_READ(DPLL_CTRL2) & DPLL_CTRL2_DDI_CLK_SEL_MASK(port);
-	pipe_config->ddi_pll_sel = temp >> (port * 3 + 1);
+	id = temp >> (port * 3 + 1);
 
-	switch (pipe_config->ddi_pll_sel) {
-	case SKL_DPLL0:
-		id = DPLL_ID_SKL_DPLL0;
-		break;
-	case SKL_DPLL1:
-		id = DPLL_ID_SKL_DPLL1;
-		break;
-	case SKL_DPLL2:
-		id = DPLL_ID_SKL_DPLL2;
-		break;
-	case SKL_DPLL3:
-		id = DPLL_ID_SKL_DPLL3;
-		break;
-	default:
-		MISSING_CASE(pipe_config->ddi_pll_sel);
+	if (WARN_ON(id < SKL_DPLL0 || id > SKL_DPLL3))
 		return;
-	}
 
 	pipe_config->shared_dpll = intel_get_shared_dpll_by_id(dev_priv, id);
 }
@@ -9724,10 +9706,9 @@ static void haswell_get_ddi_pll(struct drm_i915_private *dev_priv,
 				struct intel_crtc_state *pipe_config)
 {
 	enum intel_dpll_id id;
+	uint32_t ddi_pll_sel = I915_READ(PORT_CLK_SEL(port));
 
-	pipe_config->ddi_pll_sel = I915_READ(PORT_CLK_SEL(port));
-
-	switch (pipe_config->ddi_pll_sel) {
+	switch (ddi_pll_sel) {
 	case PORT_CLK_SEL_WRPLL1:
 		id = DPLL_ID_WRPLL1;
 		break;
@@ -9747,7 +9728,7 @@ static void haswell_get_ddi_pll(struct drm_i915_private *dev_priv,
 		id = DPLL_ID_LCPLL_2700;
 		break;
 	default:
-		MISSING_CASE(pipe_config->ddi_pll_sel);
+		MISSING_CASE(ddi_pll_sel);
 		/* fall through */
 	case PORT_CLK_SEL_NONE:
 		return;
@@ -11484,10 +11465,9 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
 	DRM_DEBUG_KMS("double wide: %i\n", pipe_config->double_wide);
 
 	if (IS_BROXTON(dev)) {
-		DRM_DEBUG_KMS("ddi_pll_sel: %u; dpll_hw_state: ebb0: 0x%x, ebb4: 0x%x,"
+		DRM_DEBUG_KMS("dpll_hw_state: ebb0: 0x%x, ebb4: 0x%x,"
 			      "pll0: 0x%x, pll1: 0x%x, pll2: 0x%x, pll3: 0x%x, "
 			      "pll6: 0x%x, pll8: 0x%x, pll9: 0x%x, pll10: 0x%x, pcsdw12: 0x%x\n",
-			      pipe_config->ddi_pll_sel,
 			      pipe_config->dpll_hw_state.ebb0,
 			      pipe_config->dpll_hw_state.ebb4,
 			      pipe_config->dpll_hw_state.pll0,
@@ -11500,15 +11480,13 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
 			      pipe_config->dpll_hw_state.pll10,
 			      pipe_config->dpll_hw_state.pcsdw12);
 	} else if (IS_SKYLAKE(dev) || IS_KABYLAKE(dev)) {
-		DRM_DEBUG_KMS("ddi_pll_sel: %u; dpll_hw_state: "
+		DRM_DEBUG_KMS("dpll_hw_state: "
 			      "ctrl1: 0x%x, cfgcr1: 0x%x, cfgcr2: 0x%x\n",
-			      pipe_config->ddi_pll_sel,
 			      pipe_config->dpll_hw_state.ctrl1,
 			      pipe_config->dpll_hw_state.cfgcr1,
 			      pipe_config->dpll_hw_state.cfgcr2);
 	} else if (HAS_DDI(dev)) {
-		DRM_DEBUG_KMS("ddi_pll_sel: 0x%x; dpll_hw_state: wrpll: 0x%x spll: 0x%x\n",
-			      pipe_config->ddi_pll_sel,
+		DRM_DEBUG_KMS("dpll_hw_state: wrpll: 0x%x spll: 0x%x\n",
 			      pipe_config->dpll_hw_state.wrpll,
 			      pipe_config->dpll_hw_state.spll);
 	} else {
@@ -11611,7 +11589,6 @@ clear_intel_crtc_state(struct intel_crtc_state *crtc_state)
 	struct intel_crtc_scaler_state scaler_state;
 	struct intel_dpll_hw_state dpll_hw_state;
 	struct intel_shared_dpll *shared_dpll;
-	uint32_t ddi_pll_sel;
 	bool force_thru;
 
 	/* FIXME: before the switch to atomic started, a new pipe_config was
@@ -11623,7 +11600,6 @@ clear_intel_crtc_state(struct intel_crtc_state *crtc_state)
 	scaler_state = crtc_state->scaler_state;
 	shared_dpll = crtc_state->shared_dpll;
 	dpll_hw_state = crtc_state->dpll_hw_state;
-	ddi_pll_sel = crtc_state->ddi_pll_sel;
 	force_thru = crtc_state->pch_pfit.force_thru;
 
 	memset(crtc_state, 0, sizeof *crtc_state);
@@ -11632,7 +11608,6 @@ clear_intel_crtc_state(struct intel_crtc_state *crtc_state)
 	crtc_state->scaler_state = scaler_state;
 	crtc_state->shared_dpll = shared_dpll;
 	crtc_state->dpll_hw_state = dpll_hw_state;
-	crtc_state->ddi_pll_sel = ddi_pll_sel;
 	crtc_state->pch_pfit.force_thru = force_thru;
 }
 
@@ -12043,8 +12018,6 @@ intel_pipe_config_compare(struct drm_device *dev,
 
 	PIPE_CONF_CHECK_I(double_wide);
 
-	PIPE_CONF_CHECK_X(ddi_pll_sel);
-
 	PIPE_CONF_CHECK_P(shared_dpll);
 	PIPE_CONF_CHECK_X(dpll_hw_state.dpll);
 	PIPE_CONF_CHECK_X(dpll_hw_state.dpll_md);
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index 553a25e..ad1913f 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -173,7 +173,8 @@ static void intel_mst_pre_enable_dp(struct intel_encoder *encoder)
 	if (intel_dp->active_mst_links == 0) {
 		intel_prepare_ddi_buffer(&intel_dig_port->base);
 
-		intel_ddi_clk_select(&intel_dig_port->base, intel_crtc->config);
+		intel_ddi_clk_select(&intel_dig_port->base,
+				     intel_crtc->config->shared_dpll);
 
 		intel_dp_set_link_params(intel_dp,
 					 intel_crtc->config->port_clock,
diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index c283ba4..70b3c12 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -449,26 +449,6 @@ static bool hsw_ddi_spll_get_hw_state(struct drm_i915_private *dev_priv,
 	return val & SPLL_PLL_ENABLE;
 }
 
-static uint32_t hsw_pll_to_ddi_pll_sel(struct intel_shared_dpll *pll)
-{
-	switch (pll->id) {
-	case DPLL_ID_WRPLL1:
-		return PORT_CLK_SEL_WRPLL1;
-	case DPLL_ID_WRPLL2:
-		return PORT_CLK_SEL_WRPLL2;
-	case DPLL_ID_SPLL:
-		return PORT_CLK_SEL_SPLL;
-	case DPLL_ID_LCPLL_810:
-		return PORT_CLK_SEL_LCPLL_810;
-	case DPLL_ID_LCPLL_1350:
-		return PORT_CLK_SEL_LCPLL_1350;
-	case DPLL_ID_LCPLL_2700:
-		return PORT_CLK_SEL_LCPLL_2700;
-	default:
-		return PORT_CLK_SEL_NONE;
-	}
-}
-
 #define LC_FREQ 2700
 #define LC_FREQ_2K U64_C(LC_FREQ * 2000)
 
@@ -748,8 +728,6 @@ hsw_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
 	if (!pll)
 		return NULL;
 
-	crtc_state->ddi_pll_sel = hsw_pll_to_ddi_pll_sel(pll);
-
 	intel_reference_shared_dpll(pll, crtc_state);
 
 	return pll;
@@ -1270,8 +1248,6 @@ skl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
 	if (!pll)
 		return NULL;
 
-	crtc_state->ddi_pll_sel = pll->id;
-
 	intel_reference_shared_dpll(pll, crtc_state);
 
 	return pll;
@@ -1618,9 +1594,6 @@ bxt_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
 
 	intel_reference_shared_dpll(pll, crtc_state);
 
-	/* shared DPLL id 0 is DPLL A */
-	crtc_state->ddi_pll_sel = pll->id;
-
 	return pll;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 91fee9f..6dd851b 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -545,12 +545,6 @@ struct intel_crtc_state {
 	/* Selected dpll when shared or NULL. */
 	struct intel_shared_dpll *shared_dpll;
 
-	/*
-	 * - PORT_CLK_SEL for DDI ports on HSW/BDW.
-	 * - enum skl_dpll on SKL
-	 */
-	uint32_t ddi_pll_sel;
-
 	/* Actual register state of the dpll, for shared dpll cross-checking. */
 	struct intel_dpll_hw_state dpll_hw_state;
 
@@ -1086,7 +1080,7 @@ void intel_crt_init(struct drm_device *dev);
 
 /* intel_ddi.c */
 void intel_ddi_clk_select(struct intel_encoder *encoder,
-			  const struct intel_crtc_state *pipe_config);
+			  struct intel_shared_dpll *pll);
 void intel_prepare_ddi_buffer(struct intel_encoder *encoder);
 void hsw_fdi_link_train(struct drm_crtc *crtc);
 void intel_ddi_init(struct drm_device *dev, enum port port);
-- 
2.5.5

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

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

* Re: [PATCHv6 5/5] drm/i915/dp: Enable Upfront link training for typeC DP support on BXT
  2016-05-20  8:59 ` [PATCHv6 5/5] drm/i915/dp: Enable Upfront link training for typeC DP support on BXT Durgadoss R
@ 2016-05-24  7:40   ` Ander Conselvan De Oliveira
  2016-05-25 15:35   ` Ville Syrjälä
  1 sibling, 0 replies; 16+ messages in thread
From: Ander Conselvan De Oliveira @ 2016-05-24  7:40 UTC (permalink / raw)
  To: Durgadoss R, intel-gfx

On Fri, 2016-05-20 at 14:29 +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 DP encoder.
> * On Connected boot scenarios: When booted with an LFP and a DP,
>   sometimes BIOS brings up DP. In these cases, we disable the
>   crtc and then do upfront link training; and bring it back up.
> * 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.
> 
> Changes since v5:
> * Moved retry logic in upfront to intel_dp.c so that it
>   can be used for all platforms.
> Changes since v4:
> * Removed usage of crtc_state in upfront link training;
>   Hence no need to find free crtc to do upfront now.
> * Re-enable crtc if it was disabled for upfront.
> * Use separate variables to track max lane count
>   and link rate found by upfront, without modifying
>   the original DPCD read from panel.
> Changes since v3:
> * Fixed a return value on BXT check
> * Reworked on top of bxt_ddi_pll_select split from Ander
> * Renamed from ddi_upfront to bxt_upfront since the
>   upfront logic includes BXT specific functions for now.
> Changes since v2:
> * Rebased on top of latest dpll_mgr.c code and
>   latest HPD related clean ups.
> * Corrected return values from upfront (Ander)
> * Corrected atomic locking for upfront in intel_dp.c (Ville)
> Changes since v1:
> *  all pll related functions inside ddi.c
> 
> Signed-off-by: Durgadoss R <durgadoss.r@intel.com>

I have one comment below, but this is

Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com

I'd still like an ack from Ville and/or Jani before merging, though. And of
course, it needs to pass CI.

> ---
>  drivers/gpu/drm/i915/intel_ddi.c |  45 ++++++++++
>  drivers/gpu/drm/i915/intel_dp.c  | 179 +++++++++++++++++++++++++++++++++++++-
> -
>  drivers/gpu/drm/i915/intel_drv.h |   8 ++
>  3 files changed, 226 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> b/drivers/gpu/drm/i915/intel_ddi.c
> index 7e6331a..8d224bf 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2330,6 +2330,51 @@ intel_ddi_init_hdmi_connector(struct intel_digital_port
> *intel_dig_port)
>  	return connector;
>  }
>  
> +bool intel_bxt_upfront_link_train(struct intel_dp *intel_dp,
> +				int clock, uint8_t lane_count)
> +{
> +	struct intel_connector *connector = intel_dp->attached_connector;
> +	struct intel_encoder *encoder = connector->encoder;
> +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> +	struct intel_shared_dpll *pll;
> +	struct intel_shared_dpll_config tmp_pll_config;
> +	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> +	enum intel_dpll_id dpll_id = (enum intel_dpll_id)dig_port->port;
> +
> +	/*
> +	 * FIXME: Works only for BXT.
> +	 * Select the required PLL. This works for platforms where
> +	 * there is no shared DPLL.
> +	 */
> +	pll = &dev_priv->shared_dplls[dpll_id];
> +	if (WARN_ON(pll->active_mask)) {
> +		DRM_ERROR("Shared DPLL already in use. active_mask:%x\n",
> pll->active_mask);
> +		return false;
> +	}
> +
> +	tmp_pll_config = pll->config;
> +
> +	if (!bxt_ddi_dp_set_dpll_hw_state(clock, &pll->config.hw_state)) {
> +		DRM_ERROR("Could not setup DPLL\n");
> +		pll->config = tmp_pll_config;
> +		return false;
> +	}
> +
> +	/* Enable PLL followed by port */
> +	pll->funcs.enable(dev_priv, pll);
> +	intel_ddi_pre_enable_dp(encoder, clock, lane_count, pll);
> +
> +	DRM_DEBUG_KMS("Upfront link train %s: link_clock:%d lanes:%d\n",
> +	intel_dp->train_set_valid ? "Passed" : "Failed", clock, lane_count);
> +
> +	/* Disable port followed by PLL for next retry/clean up */
> +	intel_ddi_post_disable(encoder);
> +	pll->funcs.disable(dev_priv, pll);
> +
> +	pll->config = tmp_pll_config;
> +	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 95ba5aa..6ecaa30 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -147,18 +147,28 @@ intel_dp_max_link_bw(struct intel_dp  *intel_dp)
>  		max_link_bw = DP_LINK_BW_1_62;
>  		break;
>  	}
> -	return max_link_bw;
> +	/*
> +	 * Limit max link bw w.r.t to the max value found
> +	 * using Upfront link training also.
> +	 */
> +	return min(max_link_bw, intel_dp->max_link_bw_upfront);
>  }
>  
>  static u8 intel_dp_max_lane_count(struct intel_dp *intel_dp)
>  {
>  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> -	u8 source_max, sink_max;
> +	u8 temp, source_max, sink_max;
>  
>  	source_max = intel_dig_port->max_lanes;
>  	sink_max = drm_dp_max_lane_count(intel_dp->dpcd);
>  
> -	return min(source_max, sink_max);
> +	temp = min(source_max, sink_max);
> +
> +	/*
> +	 * Limit max lanes w.r.t to the max value found
> +	 * using Upfront link training also.
> +	 */
> +	return min(temp, intel_dp->max_lanes_upfront);
>  }
>  
>  /*
> @@ -1590,6 +1600,15 @@ void intel_dp_set_link_params(struct intel_dp
> *intel_dp,
>  	intel_dp->lane_count = lane_count;
>  }
>  
> +static void intel_dp_init_upfront_params(struct intel_dp *intel_dp)
> +{
> +	if (WARN_ON(intel_dp->upfront_done))
> +		return;
> +
> +	intel_dp->max_link_bw_upfront = intel_dp->dpcd[DP_MAX_LINK_RATE];
> +	intel_dp->max_lanes_upfront = drm_dp_max_lane_count(intel_dp->dpcd);
> +}
> +
>  static void intel_dp_prepare(struct intel_encoder *encoder)
>  {
>  	struct drm_device *dev = encoder->base.dev;
> @@ -3424,6 +3443,16 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
>  		intel_dp->num_sink_rates = i;
>  	}
>  
> +	/*
> +	 * The link_bw and lane count vaues are initialized to MAX possible
> +	 * value for all encoder types i.e DP, eDP, DP-MST, so that the
> +	 * intel_dp_max_{link_bw/lane_count} APIs do not have to worry
> +	 * about encoder types. They further cap the max w.r.t the upfront
> +	 * values also.
> +	 */
> +	if (!intel_dp->upfront_done)
> +		intel_dp_init_upfront_params(intel_dp);
> +
>  	intel_dp_print_rates(intel_dp);
>  
>  	if (!(intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] &
> @@ -4140,6 +4169,132 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
>  	intel_dp->has_audio = false;
>  }
>  
> +static int intel_dp_upfront_crtc_disable(struct intel_crtc *crtc,
> +				struct drm_modeset_acquire_ctx *ctx,
> +				bool enable)
> +{
> +	int ret;
> +	struct drm_atomic_state *state;
> +	struct intel_crtc_state *crtc_state;
> +	struct drm_device *dev = crtc->base.dev;
> +	enum pipe pipe = crtc->pipe;
> +
> +	state = drm_atomic_state_alloc(dev);
> +	if (!state)
> +		return -ENOMEM;
> +
> +	state->acquire_ctx = ctx;
> +
> +	crtc_state = intel_atomic_get_crtc_state(state, crtc);
> +	if (IS_ERR(crtc_state)) {
> +		ret = PTR_ERR(crtc_state);
> +		drm_atomic_state_free(state);
> +		return ret;
> +	}
> +
> +	DRM_DEBUG_KMS("%sabling crtc %c %s upfront link train\n",
> +			enable ? "En" : "Dis",
> +			pipe_name(pipe),
> +			enable ? "after" : "before");
> +
> +	crtc_state->base.active = enable;
> +	ret = drm_atomic_commit(state);
> +	if (ret)
> +		drm_atomic_state_free(state);
> +
> +	return ret;
> +}
> +
> +static bool __intel_dp_upfront_link_train(struct intel_dp *intel_dp,
> +				int clock, uint8_t lane_count)
> +{
> +	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> +	struct drm_i915_private *dev_priv =
> +				to_i915(intel_dig_port->base.base.dev);
> +
> +	if (IS_BROXTON(dev_priv))
> +		return intel_bxt_upfront_link_train(intel_dp, clock,
> lane_count);

I think this could be a function pointer in the intel_dp struct. That way,
instead of sprinkling IS_* macros, we would initialize it properly once and then
just check if it not NULL elsewhere. But that can be a follow up patch.


Ander

> +	/* Other platform calls go here */
> +
> +	/* Return true for unsupported platforms */
> +	return true;
> +}
> +static bool intel_dp_upfront_link_train(struct intel_dp *intel_dp)
> +{
> +	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_i915_private *dev_priv = to_i915(dev);
> +	struct drm_mode_config *config = &dev->mode_config;
> +	struct drm_modeset_acquire_ctx ctx;
> +	struct intel_crtc *intel_crtc;
> +	struct drm_crtc *crtc = NULL;
> +	int ret;
> +	bool done = false;
> +	uint8_t lane_count, max_lanes = intel_dp->max_lanes_upfront;
> +	int common_rates[DP_MAX_SUPPORTED_RATES] = {};
> +	int clock, common_len;
> +
> +	common_len = intel_dp_common_rates(intel_dp, common_rates);
> +	if (WARN_ON(common_len <= 0))
> +		return 0;
> +
> +	if (!IS_BROXTON(dev))
> +		return 0;
> +
> +	drm_modeset_acquire_init(&ctx, 0);
> +retry:
> +	ret = drm_modeset_lock(&config->connection_mutex, &ctx);
> +	if (ret)
> +		goto exit_fail;
> +
> +	if (intel_encoder->base.crtc) {
> +		crtc = intel_encoder->base.crtc;
> +
> +		ret = drm_modeset_lock(&crtc->mutex, &ctx);
> +		if (ret)
> +			goto exit_fail;
> +
> +		ret = drm_modeset_lock(&crtc->primary->mutex, &ctx);
> +		if (ret)
> +			goto exit_fail;
> +
> +		intel_crtc = to_intel_crtc(crtc);
> +		ret = intel_dp_upfront_crtc_disable(intel_crtc, &ctx, false);
> +		if (ret)
> +			goto exit_fail;
> +	}
> +
> +	mutex_lock(&dev_priv->dpll_lock);
> +
> +	for (lane_count = max_lanes; lane_count >= 1 && !done; lane_count >>=
> 1) {
> +		for (clock = common_len - 1; clock >= 0 && !done; clock--) {
> +			done = __intel_dp_upfront_link_train(intel_dp,
> +					common_rates[clock], lane_count);
> +			if (done) {
> +				intel_dp->max_lanes_upfront = lane_count;
> +				intel_dp->max_link_bw_upfront =
> +					drm_dp_link_rate_to_bw_code(common_ra
> tes[clock]);
> +				break;
> +			}
> +		}
> +	}
> +
> +	mutex_unlock(&dev_priv->dpll_lock);
> +
> +	if (crtc)
> +		ret = intel_dp_upfront_crtc_disable(intel_crtc, &ctx, true);
> +
> +exit_fail:
> +	if (ret == -EDEADLK) {
> +		drm_modeset_backoff(&ctx);
> +		goto retry;
> +	}
> +	drm_modeset_drop_locks(&ctx);
> +	drm_modeset_acquire_fini(&ctx);
> +	return done;
> +}
> +
>  static void
>  intel_dp_long_pulse(struct intel_connector *intel_connector)
>  {
> @@ -4150,7 +4305,7 @@ intel_dp_long_pulse(struct intel_connector
> *intel_connector)
>  	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;
>  
>  	power_domain = intel_display_port_aux_power_domain(intel_encoder);
> @@ -4235,10 +4390,22 @@ intel_dp_long_pulse(struct intel_connector
> *intel_connector)
>  			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_dp->upfront_done &&
> +		intel_encoder->type == INTEL_OUTPUT_DISPLAYPORT &&
> +		intel_dp->compliance_test_type != DP_TEST_LINK_TRAINING;
> +
> +	if (do_upfront_link_train) {
> +		intel_dp->upfront_done =
> intel_dp_upfront_link_train(intel_dp);
> +		if (!intel_dp->upfront_done)
> +			status = connector_status_disconnected;
> +	}
> +
>  out:
> -	if ((status != connector_status_connected) &&
> -	    (intel_dp->is_mst == false))
> +	if (status != connector_status_connected && !intel_dp->is_mst) {
>  		intel_dp_unset_edid(intel_dp);
> +		intel_dp->upfront_done = false;
> +	}
>  
>  	intel_display_power_put(to_i915(dev), power_domain);
>  	return;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index bcf570f..060ea9b 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -832,6 +832,12 @@ struct intel_dp {
>  	enum hdmi_force_audio force_audio;
>  	bool limited_color_range;
>  	bool color_range_auto;
> +
> +	/* Upfront link train parameters */
> +	int max_link_bw_upfront;
> +	uint8_t max_lanes_upfront;
> +	bool upfront_done;
> +
>  	uint8_t dpcd[DP_RECEIVER_CAP_SIZE];
>  	uint8_t psr_dpcd[EDP_PSR_RECEIVER_CAP_SIZE];
>  	uint8_t downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
> @@ -1113,6 +1119,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_bxt_upfront_link_train(struct intel_dp *intel_dp,
> +				int clock, uint8_t lane_count);
>  
>  /* intel_frontbuffer.c */
>  void intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v2] drm/i915: Remove ddi_pll_sel from intel_crtc_state
  2016-05-24  7:32   ` [PATCH v2] " Ander Conselvan de Oliveira
@ 2016-05-24  7:51     ` R, Durgadoss
  0 siblings, 0 replies; 16+ messages in thread
From: R, Durgadoss @ 2016-05-24  7:51 UTC (permalink / raw)
  To: Conselvan De Oliveira, Ander, intel-gfx

> -----Original Message-----
> From: Conselvan De Oliveira, Ander
> Sent: Tuesday, May 24, 2016 1:03 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: R, Durgadoss <durgadoss.r@intel.com>; Conselvan De Oliveira, Ander
> <ander.conselvan.de.oliveira@intel.com>
> Subject: [PATCH v2] drm/i915: Remove ddi_pll_sel from intel_crtc_state
> 
> The value of ddi_pll_sel is derived from the selection of shared dpll,
> so just calculate the final value when necessary.
> 
> v2: Actually remove it from crtc state and delete remaining usages. (CI)
> 

Looks good to me,

Reviewed-by: Durgadoss R <durgadoss.r@intel.com>

Thanks,
Durga

> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c      | 45 ++++++++++++++++++++++++++--
> -------
>  drivers/gpu/drm/i915/intel_display.c  | 43 +++++++--------------------------
>  drivers/gpu/drm/i915/intel_dp_mst.c   |  3 ++-
>  drivers/gpu/drm/i915/intel_dpll_mgr.c | 27 ---------------------
>  drivers/gpu/drm/i915/intel_drv.h      |  8 +------
>  5 files changed, 45 insertions(+), 81 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> b/drivers/gpu/drm/i915/intel_ddi.c
> index 1cbdd66..2ee5c4c 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -531,6 +531,27 @@ static void intel_wait_ddi_buf_idle(struct
> drm_i915_private *dev_priv,
>  	DRM_ERROR("Timeout waiting for DDI BUF %c idle bit\n",
> port_name(port));
>  }
> 
> +static uint32_t hsw_pll_to_ddi_pll_sel(struct intel_shared_dpll *pll)
> +{
> +	switch (pll->id) {
> +	case DPLL_ID_WRPLL1:
> +		return PORT_CLK_SEL_WRPLL1;
> +	case DPLL_ID_WRPLL2:
> +		return PORT_CLK_SEL_WRPLL2;
> +	case DPLL_ID_SPLL:
> +		return PORT_CLK_SEL_SPLL;
> +	case DPLL_ID_LCPLL_810:
> +		return PORT_CLK_SEL_LCPLL_810;
> +	case DPLL_ID_LCPLL_1350:
> +		return PORT_CLK_SEL_LCPLL_1350;
> +	case DPLL_ID_LCPLL_2700:
> +		return PORT_CLK_SEL_LCPLL_2700;
> +	default:
> +		MISSING_CASE(pll->id);
> +		return PORT_CLK_SEL_NONE;
> +	}
> +}
> +
>  /* Starting with Haswell, different DDI ports can work in FDI mode for
>   * connection to the PCH-located connectors. For this, it is necessary to train
>   * both the DDI port and PCH receiver for the desired DDI buffer settings.
> @@ -546,7 +567,7 @@ void hsw_fdi_link_train(struct drm_crtc *crtc)
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	struct intel_encoder *encoder;
> -	u32 temp, i, rx_ctl_val;
> +	u32 temp, i, rx_ctl_val, ddi_pll_sel;
> 
>  	for_each_encoder_on_crtc(dev, crtc, encoder) {
>  		WARN_ON(encoder->type != INTEL_OUTPUT_ANALOG);
> @@ -577,8 +598,9 @@ void hsw_fdi_link_train(struct drm_crtc *crtc)
>  	I915_WRITE(FDI_RX_CTL(PIPE_A), rx_ctl_val);
> 
>  	/* Configure Port Clock Select */
> -	I915_WRITE(PORT_CLK_SEL(PORT_E), intel_crtc->config-
> >ddi_pll_sel);
> -	WARN_ON(intel_crtc->config->ddi_pll_sel != PORT_CLK_SEL_SPLL);
> +	ddi_pll_sel = hsw_pll_to_ddi_pll_sel(intel_crtc->config-
> >shared_dpll);
> +	I915_WRITE(PORT_CLK_SEL(PORT_E), ddi_pll_sel);
> +	WARN_ON(ddi_pll_sel != PORT_CLK_SEL_SPLL);
> 
>  	/* Start the training iterating through available voltages and
> emphasis,
>  	 * testing each value twice. */
> @@ -855,7 +877,7 @@ static void skl_ddi_clock_get(struct intel_encoder
> *encoder,
>  	int link_clock = 0;
>  	uint32_t dpll_ctl1, dpll;
> 
> -	dpll = pipe_config->ddi_pll_sel;
> +	dpll = intel_get_shared_dpll_id(dev_priv, pipe_config->shared_dpll);
> 
>  	dpll_ctl1 = I915_READ(DPLL_CTRL1);
> 
> @@ -903,7 +925,7 @@ static void hsw_ddi_clock_get(struct intel_encoder
> *encoder,
>  	int link_clock = 0;
>  	u32 val, pll;
> 
> -	val = pipe_config->ddi_pll_sel;
> +	val = hsw_pll_to_ddi_pll_sel(pipe_config->shared_dpll);
>  	switch (val & PORT_CLK_SEL_MASK) {
>  	case PORT_CLK_SEL_LCPLL_810:
>  		link_clock = 81000;
> @@ -1568,13 +1590,15 @@ uint32_t ddi_signal_levels(struct intel_dp
> *intel_dp)
>  }
> 
>  void intel_ddi_clk_select(struct intel_encoder *encoder,
> -			  const struct intel_crtc_state *pipe_config)
> +			  struct intel_shared_dpll *pll)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	enum port port = intel_ddi_get_encoder_port(encoder);
> 
> +	if (WARN_ON(!pll))
> +		return;
> +
>  	if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) {
> -		uint32_t dpll = pipe_config->ddi_pll_sel;
>  		uint32_t val;
> 
>  		/* DDI -> PLL mapping  */
> @@ -1582,14 +1606,13 @@ void intel_ddi_clk_select(struct intel_encoder
> *encoder,
> 
>  		val &= ~(DPLL_CTRL2_DDI_CLK_OFF(port) |
>  			DPLL_CTRL2_DDI_CLK_SEL_MASK(port));
> -		val |= (DPLL_CTRL2_DDI_CLK_SEL(dpll, port) |
> +		val |= (DPLL_CTRL2_DDI_CLK_SEL(pll->id, port) |
>  			DPLL_CTRL2_DDI_SEL_OVERRIDE(port));
> 
>  		I915_WRITE(DPLL_CTRL2, val);
> 
>  	} else if (INTEL_INFO(dev_priv)->gen < 9) {
> -		WARN_ON(pipe_config->ddi_pll_sel ==
> PORT_CLK_SEL_NONE);
> -		I915_WRITE(PORT_CLK_SEL(port), pipe_config->ddi_pll_sel);
> +		I915_WRITE(PORT_CLK_SEL(port),
> hsw_pll_to_ddi_pll_sel(pll));
>  	}
>  }
> 
> @@ -1614,7 +1637,7 @@ static void intel_ddi_pre_enable(struct
> intel_encoder *intel_encoder)
>  		intel_edp_panel_on(intel_dp);
>  	}
> 
> -	intel_ddi_clk_select(intel_encoder, crtc->config);
> +	intel_ddi_clk_select(intel_encoder, crtc->config->shared_dpll);
> 
>  	if (type == INTEL_OUTPUT_DISPLAYPORT || type ==
> INTEL_OUTPUT_EDP) {
>  		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index a500f08..0251841 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9669,15 +9669,12 @@ static void bxt_get_ddi_pll(struct
> drm_i915_private *dev_priv,
> 
>  	switch (port) {
>  	case PORT_A:
> -		pipe_config->ddi_pll_sel = SKL_DPLL0;
>  		id = DPLL_ID_SKL_DPLL0;
>  		break;
>  	case PORT_B:
> -		pipe_config->ddi_pll_sel = SKL_DPLL1;
>  		id = DPLL_ID_SKL_DPLL1;
>  		break;
>  	case PORT_C:
> -		pipe_config->ddi_pll_sel = SKL_DPLL2;
>  		id = DPLL_ID_SKL_DPLL2;
>  		break;
>  	default:
> @@ -9696,25 +9693,10 @@ static void skylake_get_ddi_pll(struct
> drm_i915_private *dev_priv,
>  	u32 temp;
> 
>  	temp = I915_READ(DPLL_CTRL2) &
> DPLL_CTRL2_DDI_CLK_SEL_MASK(port);
> -	pipe_config->ddi_pll_sel = temp >> (port * 3 + 1);
> +	id = temp >> (port * 3 + 1);
> 
> -	switch (pipe_config->ddi_pll_sel) {
> -	case SKL_DPLL0:
> -		id = DPLL_ID_SKL_DPLL0;
> -		break;
> -	case SKL_DPLL1:
> -		id = DPLL_ID_SKL_DPLL1;
> -		break;
> -	case SKL_DPLL2:
> -		id = DPLL_ID_SKL_DPLL2;
> -		break;
> -	case SKL_DPLL3:
> -		id = DPLL_ID_SKL_DPLL3;
> -		break;
> -	default:
> -		MISSING_CASE(pipe_config->ddi_pll_sel);
> +	if (WARN_ON(id < SKL_DPLL0 || id > SKL_DPLL3))
>  		return;
> -	}
> 
>  	pipe_config->shared_dpll = intel_get_shared_dpll_by_id(dev_priv,
> id);
>  }
> @@ -9724,10 +9706,9 @@ static void haswell_get_ddi_pll(struct
> drm_i915_private *dev_priv,
>  				struct intel_crtc_state *pipe_config)
>  {
>  	enum intel_dpll_id id;
> +	uint32_t ddi_pll_sel = I915_READ(PORT_CLK_SEL(port));
> 
> -	pipe_config->ddi_pll_sel = I915_READ(PORT_CLK_SEL(port));
> -
> -	switch (pipe_config->ddi_pll_sel) {
> +	switch (ddi_pll_sel) {
>  	case PORT_CLK_SEL_WRPLL1:
>  		id = DPLL_ID_WRPLL1;
>  		break;
> @@ -9747,7 +9728,7 @@ static void haswell_get_ddi_pll(struct
> drm_i915_private *dev_priv,
>  		id = DPLL_ID_LCPLL_2700;
>  		break;
>  	default:
> -		MISSING_CASE(pipe_config->ddi_pll_sel);
> +		MISSING_CASE(ddi_pll_sel);
>  		/* fall through */
>  	case PORT_CLK_SEL_NONE:
>  		return;
> @@ -11484,10 +11465,9 @@ static void intel_dump_pipe_config(struct
> intel_crtc *crtc,
>  	DRM_DEBUG_KMS("double wide: %i\n", pipe_config-
> >double_wide);
> 
>  	if (IS_BROXTON(dev)) {
> -		DRM_DEBUG_KMS("ddi_pll_sel: %u; dpll_hw_state: ebb0:
> 0x%x, ebb4: 0x%x,"
> +		DRM_DEBUG_KMS("dpll_hw_state: ebb0: 0x%x, ebb4:
> 0x%x,"
>  			      "pll0: 0x%x, pll1: 0x%x, pll2: 0x%x, pll3: 0x%x, "
>  			      "pll6: 0x%x, pll8: 0x%x, pll9: 0x%x, pll10: 0x%x,
> pcsdw12: 0x%x\n",
> -			      pipe_config->ddi_pll_sel,
>  			      pipe_config->dpll_hw_state.ebb0,
>  			      pipe_config->dpll_hw_state.ebb4,
>  			      pipe_config->dpll_hw_state.pll0,
> @@ -11500,15 +11480,13 @@ static void intel_dump_pipe_config(struct
> intel_crtc *crtc,
>  			      pipe_config->dpll_hw_state.pll10,
>  			      pipe_config->dpll_hw_state.pcsdw12);
>  	} else if (IS_SKYLAKE(dev) || IS_KABYLAKE(dev)) {
> -		DRM_DEBUG_KMS("ddi_pll_sel: %u; dpll_hw_state: "
> +		DRM_DEBUG_KMS("dpll_hw_state: "
>  			      "ctrl1: 0x%x, cfgcr1: 0x%x, cfgcr2: 0x%x\n",
> -			      pipe_config->ddi_pll_sel,
>  			      pipe_config->dpll_hw_state.ctrl1,
>  			      pipe_config->dpll_hw_state.cfgcr1,
>  			      pipe_config->dpll_hw_state.cfgcr2);
>  	} else if (HAS_DDI(dev)) {
> -		DRM_DEBUG_KMS("ddi_pll_sel: 0x%x; dpll_hw_state: wrpll:
> 0x%x spll: 0x%x\n",
> -			      pipe_config->ddi_pll_sel,
> +		DRM_DEBUG_KMS("dpll_hw_state: wrpll: 0x%x spll:
> 0x%x\n",
>  			      pipe_config->dpll_hw_state.wrpll,
>  			      pipe_config->dpll_hw_state.spll);
>  	} else {
> @@ -11611,7 +11589,6 @@ clear_intel_crtc_state(struct intel_crtc_state
> *crtc_state)
>  	struct intel_crtc_scaler_state scaler_state;
>  	struct intel_dpll_hw_state dpll_hw_state;
>  	struct intel_shared_dpll *shared_dpll;
> -	uint32_t ddi_pll_sel;
>  	bool force_thru;
> 
>  	/* FIXME: before the switch to atomic started, a new pipe_config
> was
> @@ -11623,7 +11600,6 @@ clear_intel_crtc_state(struct intel_crtc_state
> *crtc_state)
>  	scaler_state = crtc_state->scaler_state;
>  	shared_dpll = crtc_state->shared_dpll;
>  	dpll_hw_state = crtc_state->dpll_hw_state;
> -	ddi_pll_sel = crtc_state->ddi_pll_sel;
>  	force_thru = crtc_state->pch_pfit.force_thru;
> 
>  	memset(crtc_state, 0, sizeof *crtc_state);
> @@ -11632,7 +11608,6 @@ clear_intel_crtc_state(struct intel_crtc_state
> *crtc_state)
>  	crtc_state->scaler_state = scaler_state;
>  	crtc_state->shared_dpll = shared_dpll;
>  	crtc_state->dpll_hw_state = dpll_hw_state;
> -	crtc_state->ddi_pll_sel = ddi_pll_sel;
>  	crtc_state->pch_pfit.force_thru = force_thru;
>  }
> 
> @@ -12043,8 +12018,6 @@ intel_pipe_config_compare(struct drm_device
> *dev,
> 
>  	PIPE_CONF_CHECK_I(double_wide);
> 
> -	PIPE_CONF_CHECK_X(ddi_pll_sel);
> -
>  	PIPE_CONF_CHECK_P(shared_dpll);
>  	PIPE_CONF_CHECK_X(dpll_hw_state.dpll);
>  	PIPE_CONF_CHECK_X(dpll_hw_state.dpll_md);
> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c
> b/drivers/gpu/drm/i915/intel_dp_mst.c
> index 553a25e..ad1913f 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -173,7 +173,8 @@ static void intel_mst_pre_enable_dp(struct
> intel_encoder *encoder)
>  	if (intel_dp->active_mst_links == 0) {
>  		intel_prepare_ddi_buffer(&intel_dig_port->base);
> 
> -		intel_ddi_clk_select(&intel_dig_port->base, intel_crtc-
> >config);
> +		intel_ddi_clk_select(&intel_dig_port->base,
> +				     intel_crtc->config->shared_dpll);
> 
>  		intel_dp_set_link_params(intel_dp,
>  					 intel_crtc->config->port_clock,
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> index c283ba4..70b3c12 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> @@ -449,26 +449,6 @@ static bool hsw_ddi_spll_get_hw_state(struct
> drm_i915_private *dev_priv,
>  	return val & SPLL_PLL_ENABLE;
>  }
> 
> -static uint32_t hsw_pll_to_ddi_pll_sel(struct intel_shared_dpll *pll)
> -{
> -	switch (pll->id) {
> -	case DPLL_ID_WRPLL1:
> -		return PORT_CLK_SEL_WRPLL1;
> -	case DPLL_ID_WRPLL2:
> -		return PORT_CLK_SEL_WRPLL2;
> -	case DPLL_ID_SPLL:
> -		return PORT_CLK_SEL_SPLL;
> -	case DPLL_ID_LCPLL_810:
> -		return PORT_CLK_SEL_LCPLL_810;
> -	case DPLL_ID_LCPLL_1350:
> -		return PORT_CLK_SEL_LCPLL_1350;
> -	case DPLL_ID_LCPLL_2700:
> -		return PORT_CLK_SEL_LCPLL_2700;
> -	default:
> -		return PORT_CLK_SEL_NONE;
> -	}
> -}
> -
>  #define LC_FREQ 2700
>  #define LC_FREQ_2K U64_C(LC_FREQ * 2000)
> 
> @@ -748,8 +728,6 @@ hsw_get_dpll(struct intel_crtc *crtc, struct
> intel_crtc_state *crtc_state,
>  	if (!pll)
>  		return NULL;
> 
> -	crtc_state->ddi_pll_sel = hsw_pll_to_ddi_pll_sel(pll);
> -
>  	intel_reference_shared_dpll(pll, crtc_state);
> 
>  	return pll;
> @@ -1270,8 +1248,6 @@ skl_get_dpll(struct intel_crtc *crtc, struct
> intel_crtc_state *crtc_state,
>  	if (!pll)
>  		return NULL;
> 
> -	crtc_state->ddi_pll_sel = pll->id;
> -
>  	intel_reference_shared_dpll(pll, crtc_state);
> 
>  	return pll;
> @@ -1618,9 +1594,6 @@ bxt_get_dpll(struct intel_crtc *crtc, struct
> intel_crtc_state *crtc_state,
> 
>  	intel_reference_shared_dpll(pll, crtc_state);
> 
> -	/* shared DPLL id 0 is DPLL A */
> -	crtc_state->ddi_pll_sel = pll->id;
> -
>  	return pll;
>  }
> 
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 91fee9f..6dd851b 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -545,12 +545,6 @@ struct intel_crtc_state {
>  	/* Selected dpll when shared or NULL. */
>  	struct intel_shared_dpll *shared_dpll;
> 
> -	/*
> -	 * - PORT_CLK_SEL for DDI ports on HSW/BDW.
> -	 * - enum skl_dpll on SKL
> -	 */
> -	uint32_t ddi_pll_sel;
> -
>  	/* Actual register state of the dpll, for shared dpll cross-checking. */
>  	struct intel_dpll_hw_state dpll_hw_state;
> 
> @@ -1086,7 +1080,7 @@ void intel_crt_init(struct drm_device *dev);
> 
>  /* intel_ddi.c */
>  void intel_ddi_clk_select(struct intel_encoder *encoder,
> -			  const struct intel_crtc_state *pipe_config);
> +			  struct intel_shared_dpll *pll);
>  void intel_prepare_ddi_buffer(struct intel_encoder *encoder);
>  void hsw_fdi_link_train(struct drm_crtc *crtc);
>  void intel_ddi_init(struct drm_device *dev, enum port port);
> --
> 2.5.5

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

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

* ✗ Ro.CI.BAT: warning for Add USB typeC based DP support for BXT platform (rev8)
  2016-05-20  8:58 [PATCHv6 0/5] Add USB typeC based DP support for BXT platform Durgadoss R
                   ` (5 preceding siblings ...)
  2016-05-20  9:41 ` ✗ Ro.CI.BAT: warning for Add USB typeC based DP support for BXT platform (rev7) Patchwork
@ 2016-05-24  7:56 ` Patchwork
  6 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2016-05-24  7:56 UTC (permalink / raw)
  To: Ander Conselvan de Oliveira; +Cc: intel-gfx

== Series Details ==

Series: Add USB typeC based DP support for BXT platform (rev8)
URL   : https://patchwork.freedesktop.org/series/1731/
State : warning

== Summary ==

Series 1731v8 Add USB typeC based DP support for BXT platform
http://patchwork.freedesktop.org/api/1.0/series/1731/revisions/8/mbox

Test gem_busy:
        Subgroup basic-bsd:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
        Subgroup basic-parallel-vebox:
                dmesg-warn -> PASS       (ro-skl-i7-6700hq)
Test gem_exec_basic:
        Subgroup gtt-vebox:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
Test gem_ringfill:
        Subgroup basic-default-interruptible:
                dmesg-warn -> PASS       (ro-skl-i7-6700hq)
Test gem_storedw_loop:
        Subgroup basic-default:
                dmesg-warn -> PASS       (ro-skl-i7-6700hq)
Test kms_flip:
        Subgroup basic-flip-vs-dpms:
                pass       -> SKIP       (ro-bdw-i7-5557U)
        Subgroup basic-flip-vs-modeset:
                pass       -> SKIP       (ro-bdw-i7-5557U)
        Subgroup basic-flip-vs-wf_vblank:
                pass       -> SKIP       (ro-bdw-i7-5557U)
        Subgroup basic-plain-flip:
                pass       -> SKIP       (ro-bdw-i7-5557U)
Test kms_frontbuffer_tracking:
        Subgroup basic:
                pass       -> SKIP       (ro-bdw-i7-5557U)
Test kms_pipe_crc_basic:
        Subgroup bad-pipe:
                dmesg-warn -> PASS       (ro-skl-i7-6700hq)
        Subgroup hang-read-crc-pipe-a:
                pass       -> SKIP       (ro-bdw-i7-5557U)
        Subgroup hang-read-crc-pipe-b:
                pass       -> SKIP       (ro-bdw-i7-5557U)
        Subgroup hang-read-crc-pipe-c:
                pass       -> SKIP       (ro-bdw-i7-5557U)
        Subgroup nonblocking-crc-pipe-a:
                pass       -> SKIP       (ro-bdw-i7-5557U)
        Subgroup nonblocking-crc-pipe-a-frame-sequence:
                pass       -> SKIP       (ro-bdw-i7-5557U)
        Subgroup nonblocking-crc-pipe-b:
                pass       -> SKIP       (ro-bdw-i7-5557U)
        Subgroup nonblocking-crc-pipe-b-frame-sequence:
                pass       -> SKIP       (ro-bdw-i7-5557U)
        Subgroup nonblocking-crc-pipe-c:
                pass       -> SKIP       (ro-bdw-i7-5557U)
        Subgroup nonblocking-crc-pipe-c-frame-sequence:
                pass       -> SKIP       (ro-bdw-i7-5557U)
        Subgroup read-crc-pipe-a:
                pass       -> SKIP       (ro-bdw-i7-5557U)
        Subgroup read-crc-pipe-a-frame-sequence:
                pass       -> SKIP       (ro-bdw-i7-5557U)
        Subgroup read-crc-pipe-b:
                pass       -> SKIP       (ro-bdw-i7-5557U)
        Subgroup read-crc-pipe-b-frame-sequence:
                pass       -> SKIP       (ro-bdw-i7-5557U)
        Subgroup read-crc-pipe-c:
                pass       -> SKIP       (ro-bdw-i7-5557U)
        Subgroup read-crc-pipe-c-frame-sequence:
                pass       -> SKIP       (ro-bdw-i7-5557U)
        Subgroup suspend-read-crc-pipe-a:
                pass       -> SKIP       (ro-bdw-i7-5557U)
        Subgroup suspend-read-crc-pipe-b:
                pass       -> SKIP       (ro-bdw-i7-5557U)
        Subgroup suspend-read-crc-pipe-c:
                pass       -> SKIP       (ro-bdw-i7-5557U)
Test kms_psr_sink_crc:
        Subgroup psr_basic:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
Test kms_setmode:
        Subgroup basic-clone-single-crtc:
                pass       -> DMESG-WARN (ro-skl-i7-6700hq)
Test pm_rpm:
        Subgroup basic-pci-d3-state:
                fail       -> DMESG-WARN (ro-skl-i7-6700hq)
                pass       -> SKIP       (ro-bdw-i7-5557U)
        Subgroup basic-rte:
                pass       -> SKIP       (ro-bdw-i7-5557U)
Test pm_rps:
        Subgroup basic-api:
                dmesg-warn -> PASS       (ro-skl-i7-6700hq)

ro-bdw-i5-5250u  total:209  pass:172  dwarn:0   dfail:0   fail:0   skip:37 
ro-bdw-i7-5557U  total:209  pass:172  dwarn:0   dfail:0   fail:0   skip:37 
ro-bdw-i7-5600u  total:209  pass:180  dwarn:0   dfail:0   fail:1   skip:28 
ro-bsw-n3050     total:209  pass:168  dwarn:0   dfail:0   fail:2   skip:39 
ro-hsw-i3-4010u  total:209  pass:186  dwarn:0   dfail:0   fail:0   skip:23 
ro-hsw-i7-4770r  total:209  pass:186  dwarn:0   dfail:0   fail:0   skip:23 
ro-ilk1-i5-650   total:204  pass:146  dwarn:0   dfail:0   fail:1   skip:57 
ro-ivb-i7-3770   total:209  pass:177  dwarn:0   dfail:0   fail:0   skip:32 
ro-ivb2-i7-3770  total:209  pass:181  dwarn:0   dfail:0   fail:0   skip:28 
ro-skl-i7-6700hq total:204  pass:176  dwarn:7   dfail:0   fail:0   skip:21 
ro-snb-i7-2620M  total:209  pass:170  dwarn:0   dfail:0   fail:1   skip:38 
ro-byt-n2820 failed to connect after reboot

Results at /archive/results/CI_IGT_test/RO_Patchwork_986/

8621fb5 drm-intel-nightly: 2016y-05m-23d-18h-18m-33s UTC integration manifest
70401b1 drm/i915/dp: Enable Upfront link training for typeC DP support on BXT
00f6c1c drm/i915: Split bxt_ddi_pll_select()
3debdd9 drm/i915: Split intel_ddi_pre_enable() into DP and HDMI versions
363cc81 drm/i915: Remove ddi_pll_sel from intel_crtc_state
b5fa7cf drm/i915: Don't pass crtc_state to intel_dp_set_link_params()

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

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

* Re: [PATCHv6 5/5] drm/i915/dp: Enable Upfront link training for typeC DP support on BXT
  2016-05-20  8:59 ` [PATCHv6 5/5] drm/i915/dp: Enable Upfront link training for typeC DP support on BXT Durgadoss R
  2016-05-24  7:40   ` Ander Conselvan De Oliveira
@ 2016-05-25 15:35   ` Ville Syrjälä
  2016-05-26 10:03     ` R, Durgadoss
  1 sibling, 1 reply; 16+ messages in thread
From: Ville Syrjälä @ 2016-05-25 15:35 UTC (permalink / raw)
  To: Durgadoss R; +Cc: ander.conselvan.de.oliveira, intel-gfx

On Fri, May 20, 2016 at 02:29:02PM +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 DP encoder.
> * On Connected boot scenarios: When booted with an LFP and a DP,
>   sometimes BIOS brings up DP. In these cases, we disable the
>   crtc and then do upfront link training; and bring it back up.
> * 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.
> 
> Changes since v5:
> * Moved retry logic in upfront to intel_dp.c so that it
>   can be used for all platforms.
> Changes since v4:
> * Removed usage of crtc_state in upfront link training;
>   Hence no need to find free crtc to do upfront now.
> * Re-enable crtc if it was disabled for upfront.
> * Use separate variables to track max lane count
>   and link rate found by upfront, without modifying
>   the original DPCD read from panel.
> Changes since v3:
> * Fixed a return value on BXT check
> * Reworked on top of bxt_ddi_pll_select split from Ander
> * Renamed from ddi_upfront to bxt_upfront since the
>   upfront logic includes BXT specific functions for now.
> Changes since v2:
> * Rebased on top of latest dpll_mgr.c code and
>   latest HPD related clean ups.
> * Corrected return values from upfront (Ander)
> * Corrected atomic locking for upfront in intel_dp.c (Ville)
> Changes since v1:
> *  all pll related functions inside ddi.c
> 
> Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c |  45 ++++++++++
>  drivers/gpu/drm/i915/intel_dp.c  | 179 +++++++++++++++++++++++++++++++++++++--
>  drivers/gpu/drm/i915/intel_drv.h |   8 ++
>  3 files changed, 226 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 7e6331a..8d224bf 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2330,6 +2330,51 @@ intel_ddi_init_hdmi_connector(struct intel_digital_port *intel_dig_port)
>  	return connector;
>  }
>  
> +bool intel_bxt_upfront_link_train(struct intel_dp *intel_dp,
> +				int clock, uint8_t lane_count)
> +{
> +	struct intel_connector *connector = intel_dp->attached_connector;
> +	struct intel_encoder *encoder = connector->encoder;
> +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> +	struct intel_shared_dpll *pll;
> +	struct intel_shared_dpll_config tmp_pll_config;
> +	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> +	enum intel_dpll_id dpll_id = (enum intel_dpll_id)dig_port->port;
> +
> +	/*
> +	 * FIXME: Works only for BXT.
> +	 * Select the required PLL. This works for platforms where
> +	 * there is no shared DPLL.
> +	 */
> +	pll = &dev_priv->shared_dplls[dpll_id];
> +	if (WARN_ON(pll->active_mask)) {
> +		DRM_ERROR("Shared DPLL already in use. active_mask:%x\n", pll->active_mask);
> +		return false;
> +	}
> +
> +	tmp_pll_config = pll->config;
> +
> +	if (!bxt_ddi_dp_set_dpll_hw_state(clock, &pll->config.hw_state)) {
> +		DRM_ERROR("Could not setup DPLL\n");
> +		pll->config = tmp_pll_config;
> +		return false;
> +	}
> +
> +	/* Enable PLL followed by port */
> +	pll->funcs.enable(dev_priv, pll);
> +	intel_ddi_pre_enable_dp(encoder, clock, lane_count, pll);
> +
> +	DRM_DEBUG_KMS("Upfront link train %s: link_clock:%d lanes:%d\n",
> +	intel_dp->train_set_valid ? "Passed" : "Failed", clock, lane_count);
> +
> +	/* Disable port followed by PLL for next retry/clean up */
> +	intel_ddi_post_disable(encoder);
> +	pll->funcs.disable(dev_priv, pll);
> +
> +	pll->config = tmp_pll_config;
> +	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 95ba5aa..6ecaa30 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -147,18 +147,28 @@ intel_dp_max_link_bw(struct intel_dp  *intel_dp)
>  		max_link_bw = DP_LINK_BW_1_62;
>  		break;
>  	}
> -	return max_link_bw;
> +	/*
> +	 * Limit max link bw w.r.t to the max value found
> +	 * using Upfront link training also.
> +	 */
> +	return min(max_link_bw, intel_dp->max_link_bw_upfront);

This don't feel like the right place for this. I've tried to move
us away from the link_bw usage to using proper rate numbers.

This should probably be handled somewhere in intel_dp_common_rates()
or perhaps just in intel_dp_max_link_rate().

>  }
>  
>  static u8 intel_dp_max_lane_count(struct intel_dp *intel_dp)
>  {
>  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> -	u8 source_max, sink_max;
> +	u8 temp, source_max, sink_max;
>  
>  	source_max = intel_dig_port->max_lanes;
>  	sink_max = drm_dp_max_lane_count(intel_dp->dpcd);
>  
> -	return min(source_max, sink_max);
> +	temp = min(source_max, sink_max);
> +
> +	/*
> +	 * Limit max lanes w.r.t to the max value found
> +	 * using Upfront link training also.
> +	 */
> +	return min(temp, intel_dp->max_lanes_upfront);
>  }
>  
>  /*
> @@ -1590,6 +1600,15 @@ void intel_dp_set_link_params(struct intel_dp *intel_dp,
>  	intel_dp->lane_count = lane_count;
>  }
>  
> +static void intel_dp_init_upfront_params(struct intel_dp *intel_dp)
> +{
> +	if (WARN_ON(intel_dp->upfront_done))
> +		return;
> +
> +	intel_dp->max_link_bw_upfront = intel_dp->dpcd[DP_MAX_LINK_RATE];
> +	intel_dp->max_lanes_upfront = drm_dp_max_lane_count(intel_dp->dpcd);
> +}
> +
>  static void intel_dp_prepare(struct intel_encoder *encoder)
>  {
>  	struct drm_device *dev = encoder->base.dev;
> @@ -3424,6 +3443,16 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
>  		intel_dp->num_sink_rates = i;
>  	}
>  
> +	/*
> +	 * The link_bw and lane count vaues are initialized to MAX possible
> +	 * value for all encoder types i.e DP, eDP, DP-MST, so that the
> +	 * intel_dp_max_{link_bw/lane_count} APIs do not have to worry
> +	 * about encoder types. They further cap the max w.r.t the upfront
> +	 * values also.
> +	 */
> +	if (!intel_dp->upfront_done)
> +		intel_dp_init_upfront_params(intel_dp);

With all the modeset locks involved there, I have a bad feeling this
ends up getting called from the hpd pulse work at the wrong time
perhaps leading to a deadlock.

> +
>  	intel_dp_print_rates(intel_dp);
>  
>  	if (!(intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] &
> @@ -4140,6 +4169,132 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
>  	intel_dp->has_audio = false;
>  }
>  
> +static int intel_dp_upfront_crtc_disable(struct intel_crtc *crtc,
> +				struct drm_modeset_acquire_ctx *ctx,
> +				bool enable)
> +{
> +	int ret;
> +	struct drm_atomic_state *state;
> +	struct intel_crtc_state *crtc_state;
> +	struct drm_device *dev = crtc->base.dev;
> +	enum pipe pipe = crtc->pipe;
> +
> +	state = drm_atomic_state_alloc(dev);
> +	if (!state)
> +		return -ENOMEM;
> +
> +	state->acquire_ctx = ctx;
> +
> +	crtc_state = intel_atomic_get_crtc_state(state, crtc);
> +	if (IS_ERR(crtc_state)) {
> +		ret = PTR_ERR(crtc_state);
> +		drm_atomic_state_free(state);
> +		return ret;
> +	}
> +
> +	DRM_DEBUG_KMS("%sabling crtc %c %s upfront link train\n",
> +			enable ? "En" : "Dis",
> +			pipe_name(pipe),
> +			enable ? "after" : "before");
> +
> +	crtc_state->base.active = enable;
> +	ret = drm_atomic_commit(state);
> +	if (ret)
> +		drm_atomic_state_free(state);
> +
> +	return ret;
> +}
> +
> +static bool __intel_dp_upfront_link_train(struct intel_dp *intel_dp,
> +				int clock, uint8_t lane_count)
> +{
> +	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> +	struct drm_i915_private *dev_priv =
> +				to_i915(intel_dig_port->base.base.dev);
> +
> +	if (IS_BROXTON(dev_priv))
> +		return intel_bxt_upfront_link_train(intel_dp, clock, lane_count);
> +	/* Other platform calls go here */
> +
> +	/* Return true for unsupported platforms */
> +	return true;
> +}
> +static bool intel_dp_upfront_link_train(struct intel_dp *intel_dp)
> +{
> +	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_i915_private *dev_priv = to_i915(dev);
> +	struct drm_mode_config *config = &dev->mode_config;
> +	struct drm_modeset_acquire_ctx ctx;
> +	struct intel_crtc *intel_crtc;
> +	struct drm_crtc *crtc = NULL;
> +	int ret;
> +	bool done = false;
> +	uint8_t lane_count, max_lanes = intel_dp->max_lanes_upfront;
> +	int common_rates[DP_MAX_SUPPORTED_RATES] = {};
> +	int clock, common_len;
> +
> +	common_len = intel_dp_common_rates(intel_dp, common_rates);
> +	if (WARN_ON(common_len <= 0))
> +		return 0;
> +
> +	if (!IS_BROXTON(dev))
> +		return 0;
> +
> +	drm_modeset_acquire_init(&ctx, 0);
> +retry:
> +	ret = drm_modeset_lock(&config->connection_mutex, &ctx);
> +	if (ret)
> +		goto exit_fail;
> +
> +	if (intel_encoder->base.crtc) {
> +		crtc = intel_encoder->base.crtc;
> +
> +		ret = drm_modeset_lock(&crtc->mutex, &ctx);
> +		if (ret)
> +			goto exit_fail;
> +
> +		ret = drm_modeset_lock(&crtc->primary->mutex, &ctx);
> +		if (ret)
> +			goto exit_fail;
> +
> +		intel_crtc = to_intel_crtc(crtc);
> +		ret = intel_dp_upfront_crtc_disable(intel_crtc, &ctx, false);
> +		if (ret)
> +			goto exit_fail;

Magically disabling stuff doens't seem right. OTOH I don't have a good
idea how we'd do this if the user yanks the cable and plugs it back in
before userspace has had a chance to shut down the pipe(s). Postpone the
detection until it's all clear, or something? Anyone have good ideas for
that?

> +	}
> +
> +	mutex_lock(&dev_priv->dpll_lock);
> +
> +	for (lane_count = max_lanes; lane_count >= 1 && !done; lane_count >>= 1) {
> +		for (clock = common_len - 1; clock >= 0 && !done; clock--) {
> +			done = __intel_dp_upfront_link_train(intel_dp,
> +					common_rates[clock], lane_count);
> +			if (done) {
> +				intel_dp->max_lanes_upfront = lane_count;
> +				intel_dp->max_link_bw_upfront =
> +					drm_dp_link_rate_to_bw_code(common_rates[clock]);
> +				break;
> +			}
> +		}
> +	}
> +
> +	mutex_unlock(&dev_priv->dpll_lock);
> +
> +	if (crtc)
> +		ret = intel_dp_upfront_crtc_disable(intel_crtc, &ctx, true);
> +
> +exit_fail:
> +	if (ret == -EDEADLK) {
> +		drm_modeset_backoff(&ctx);
> +		goto retry;
> +	}
> +	drm_modeset_drop_locks(&ctx);
> +	drm_modeset_acquire_fini(&ctx);
> +	return done;
> +}
> +
>  static void
>  intel_dp_long_pulse(struct intel_connector *intel_connector)
>  {
> @@ -4150,7 +4305,7 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
>  	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;
>  
>  	power_domain = intel_display_port_aux_power_domain(intel_encoder);
> @@ -4235,10 +4390,22 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
>  			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_dp->upfront_done &&
> +		intel_encoder->type == INTEL_OUTPUT_DISPLAYPORT &&
> +		intel_dp->compliance_test_type != DP_TEST_LINK_TRAINING;
> +
> +	if (do_upfront_link_train) {
> +		intel_dp->upfront_done = intel_dp_upfront_link_train(intel_dp);
> +		if (!intel_dp->upfront_done)
> +			status = connector_status_disconnected;
> +	}
> +
>  out:
> -	if ((status != connector_status_connected) &&
> -	    (intel_dp->is_mst == false))
> +	if (status != connector_status_connected && !intel_dp->is_mst) {
>  		intel_dp_unset_edid(intel_dp);
> +		intel_dp->upfront_done = false;
> +	}
>  
>  	intel_display_power_put(to_i915(dev), power_domain);
>  	return;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index bcf570f..060ea9b 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -832,6 +832,12 @@ struct intel_dp {
>  	enum hdmi_force_audio force_audio;
>  	bool limited_color_range;
>  	bool color_range_auto;
> +
> +	/* Upfront link train parameters */
> +	int max_link_bw_upfront;
> +	uint8_t max_lanes_upfront;
> +	bool upfront_done;
> +
>  	uint8_t dpcd[DP_RECEIVER_CAP_SIZE];
>  	uint8_t psr_dpcd[EDP_PSR_RECEIVER_CAP_SIZE];
>  	uint8_t downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
> @@ -1113,6 +1119,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_bxt_upfront_link_train(struct intel_dp *intel_dp,
> +				int clock, uint8_t lane_count);
>  
>  /* 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
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

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

* Re: [PATCHv6 5/5] drm/i915/dp: Enable Upfront link training for typeC DP support on BXT
  2016-05-25 15:35   ` Ville Syrjälä
@ 2016-05-26 10:03     ` R, Durgadoss
  2016-05-26 11:30       ` Ville Syrjälä
  0 siblings, 1 reply; 16+ messages in thread
From: R, Durgadoss @ 2016-05-26 10:03 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: Conselvan De Oliveira, Ander, intel-gfx


> -----Original Message-----
> From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
> Sent: Wednesday, May 25, 2016 9:05 PM
> To: R, Durgadoss <durgadoss.r@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; Conselvan De Oliveira, Ander
> <ander.conselvan.de.oliveira@intel.com>
> Subject: Re: [Intel-gfx] [PATCHv6 5/5] drm/i915/dp: Enable Upfront link
> training for typeC DP support on BXT
> 
> On Fri, May 20, 2016 at 02:29:02PM +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 DP encoder.
> > * On Connected boot scenarios: When booted with an LFP and a DP,
> >   sometimes BIOS brings up DP. In these cases, we disable the
> >   crtc and then do upfront link training; and bring it back up.
> > * 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.
> >
> > Changes since v5:
> > * Moved retry logic in upfront to intel_dp.c so that it
> >   can be used for all platforms.
> > Changes since v4:
> > * Removed usage of crtc_state in upfront link training;
> >   Hence no need to find free crtc to do upfront now.
> > * Re-enable crtc if it was disabled for upfront.
> > * Use separate variables to track max lane count
> >   and link rate found by upfront, without modifying
> >   the original DPCD read from panel.
> > Changes since v3:
> > * Fixed a return value on BXT check
> > * Reworked on top of bxt_ddi_pll_select split from Ander
> > * Renamed from ddi_upfront to bxt_upfront since the
> >   upfront logic includes BXT specific functions for now.
> > Changes since v2:
> > * Rebased on top of latest dpll_mgr.c code and
> >   latest HPD related clean ups.
> > * Corrected return values from upfront (Ander)
> > * Corrected atomic locking for upfront in intel_dp.c (Ville)
> > Changes since v1:
> > *  all pll related functions inside ddi.c
> >
> > Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_ddi.c |  45 ++++++++++
> >  drivers/gpu/drm/i915/intel_dp.c  | 179
> +++++++++++++++++++++++++++++++++++++--
> >  drivers/gpu/drm/i915/intel_drv.h |   8 ++
> >  3 files changed, 226 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> b/drivers/gpu/drm/i915/intel_ddi.c
> > index 7e6331a..8d224bf 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -2330,6 +2330,51 @@ intel_ddi_init_hdmi_connector(struct
> intel_digital_port *intel_dig_port)
> >  	return connector;
> >  }
> >
> > +bool intel_bxt_upfront_link_train(struct intel_dp *intel_dp,
> > +				int clock, uint8_t lane_count)
> > +{
> > +	struct intel_connector *connector = intel_dp->attached_connector;
> > +	struct intel_encoder *encoder = connector->encoder;
> > +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > +	struct intel_shared_dpll *pll;
> > +	struct intel_shared_dpll_config tmp_pll_config;
> > +	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> > +	enum intel_dpll_id dpll_id = (enum intel_dpll_id)dig_port->port;
> > +
> > +	/*
> > +	 * FIXME: Works only for BXT.
> > +	 * Select the required PLL. This works for platforms where
> > +	 * there is no shared DPLL.
> > +	 */
> > +	pll = &dev_priv->shared_dplls[dpll_id];
> > +	if (WARN_ON(pll->active_mask)) {
> > +		DRM_ERROR("Shared DPLL already in use.
> active_mask:%x\n", pll->active_mask);
> > +		return false;
> > +	}
> > +
> > +	tmp_pll_config = pll->config;
> > +
> > +	if (!bxt_ddi_dp_set_dpll_hw_state(clock, &pll->config.hw_state)) {
> > +		DRM_ERROR("Could not setup DPLL\n");
> > +		pll->config = tmp_pll_config;
> > +		return false;
> > +	}
> > +
> > +	/* Enable PLL followed by port */
> > +	pll->funcs.enable(dev_priv, pll);
> > +	intel_ddi_pre_enable_dp(encoder, clock, lane_count, pll);
> > +
> > +	DRM_DEBUG_KMS("Upfront link train %s: link_clock:%d
> lanes:%d\n",
> > +	intel_dp->train_set_valid ? "Passed" : "Failed", clock, lane_count);
> > +
> > +	/* Disable port followed by PLL for next retry/clean up */
> > +	intel_ddi_post_disable(encoder);
> > +	pll->funcs.disable(dev_priv, pll);
> > +
> > +	pll->config = tmp_pll_config;
> > +	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 95ba5aa..6ecaa30 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -147,18 +147,28 @@ intel_dp_max_link_bw(struct intel_dp
> *intel_dp)
> >  		max_link_bw = DP_LINK_BW_1_62;
> >  		break;
> >  	}
> > -	return max_link_bw;
> > +	/*
> > +	 * Limit max link bw w.r.t to the max value found
> > +	 * using Upfront link training also.
> > +	 */
> > +	return min(max_link_bw, intel_dp->max_link_bw_upfront);
> 
> This don't feel like the right place for this. I've tried to move
> us away from the link_bw usage to using proper rate numbers.

Agreed, While doing these changes, I also felt it would be good to
use any one of them consistently. So, will make it all  use link_rate.

> 
> This should probably be handled somewhere in intel_dp_common_rates()
> or perhaps just in intel_dp_max_link_rate().

Ok, moving it to intel_dp_common_rates() in the next version.

> 
> >  }
> >
> >  static u8 intel_dp_max_lane_count(struct intel_dp *intel_dp)
> >  {
> >  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > -	u8 source_max, sink_max;
> > +	u8 temp, source_max, sink_max;
> >
> >  	source_max = intel_dig_port->max_lanes;
> >  	sink_max = drm_dp_max_lane_count(intel_dp->dpcd);
> >
> > -	return min(source_max, sink_max);
> > +	temp = min(source_max, sink_max);
> > +
> > +	/*
> > +	 * Limit max lanes w.r.t to the max value found
> > +	 * using Upfront link training also.
> > +	 */
> > +	return min(temp, intel_dp->max_lanes_upfront);
> >  }
> >
> >  /*
> > @@ -1590,6 +1600,15 @@ void intel_dp_set_link_params(struct intel_dp
> *intel_dp,
> >  	intel_dp->lane_count = lane_count;
> >  }
> >
> > +static void intel_dp_init_upfront_params(struct intel_dp *intel_dp)
> > +{
> > +	if (WARN_ON(intel_dp->upfront_done))
> > +		return;
> > +
> > +	intel_dp->max_link_bw_upfront = intel_dp-
> >dpcd[DP_MAX_LINK_RATE];
> > +	intel_dp->max_lanes_upfront = drm_dp_max_lane_count(intel_dp-
> >dpcd);
> > +}
> > +
> >  static void intel_dp_prepare(struct intel_encoder *encoder)
> >  {
> >  	struct drm_device *dev = encoder->base.dev;
> > @@ -3424,6 +3443,16 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
> >  		intel_dp->num_sink_rates = i;
> >  	}
> >
> > +	/*
> > +	 * The link_bw and lane count vaues are initialized to MAX possible
> > +	 * value for all encoder types i.e DP, eDP, DP-MST, so that the
> > +	 * intel_dp_max_{link_bw/lane_count} APIs do not have to worry
> > +	 * about encoder types. They further cap the max w.r.t the upfront
> > +	 * values also.
> > +	 */
> > +	if (!intel_dp->upfront_done)
> > +		intel_dp_init_upfront_params(intel_dp);
> 
> With all the modeset locks involved there, I have a bad feeling this
> ends up getting called from the hpd pulse work at the wrong time
> perhaps leading to a deadlock.

With some code changes, there is no need for this init_params.
Will remove this in next version.

+ Also making upfront_link_train as a vfunc inside intel_dp,
as Ander pointed out.

Thanks,
Durga

> 
> > +
> >  	intel_dp_print_rates(intel_dp);
> >
> >  	if (!(intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] &
> > @@ -4140,6 +4169,132 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
> >  	intel_dp->has_audio = false;
> >  }
> >
> > +static int intel_dp_upfront_crtc_disable(struct intel_crtc *crtc,
> > +				struct drm_modeset_acquire_ctx *ctx,
> > +				bool enable)
> > +{
> > +	int ret;
> > +	struct drm_atomic_state *state;
> > +	struct intel_crtc_state *crtc_state;
> > +	struct drm_device *dev = crtc->base.dev;
> > +	enum pipe pipe = crtc->pipe;
> > +
> > +	state = drm_atomic_state_alloc(dev);
> > +	if (!state)
> > +		return -ENOMEM;
> > +
> > +	state->acquire_ctx = ctx;
> > +
> > +	crtc_state = intel_atomic_get_crtc_state(state, crtc);
> > +	if (IS_ERR(crtc_state)) {
> > +		ret = PTR_ERR(crtc_state);
> > +		drm_atomic_state_free(state);
> > +		return ret;
> > +	}
> > +
> > +	DRM_DEBUG_KMS("%sabling crtc %c %s upfront link train\n",
> > +			enable ? "En" : "Dis",
> > +			pipe_name(pipe),
> > +			enable ? "after" : "before");
> > +
> > +	crtc_state->base.active = enable;
> > +	ret = drm_atomic_commit(state);
> > +	if (ret)
> > +		drm_atomic_state_free(state);
> > +
> > +	return ret;
> > +}
> > +
> > +static bool __intel_dp_upfront_link_train(struct intel_dp *intel_dp,
> > +				int clock, uint8_t lane_count)
> > +{
> > +	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > +	struct drm_i915_private *dev_priv =
> > +				to_i915(intel_dig_port->base.base.dev);
> > +
> > +	if (IS_BROXTON(dev_priv))
> > +		return intel_bxt_upfront_link_train(intel_dp, clock,
> lane_count);
> > +	/* Other platform calls go here */
> > +
> > +	/* Return true for unsupported platforms */
> > +	return true;
> > +}
> > +static bool intel_dp_upfront_link_train(struct intel_dp *intel_dp)
> > +{
> > +	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_i915_private *dev_priv = to_i915(dev);
> > +	struct drm_mode_config *config = &dev->mode_config;
> > +	struct drm_modeset_acquire_ctx ctx;
> > +	struct intel_crtc *intel_crtc;
> > +	struct drm_crtc *crtc = NULL;
> > +	int ret;
> > +	bool done = false;
> > +	uint8_t lane_count, max_lanes = intel_dp->max_lanes_upfront;
> > +	int common_rates[DP_MAX_SUPPORTED_RATES] = {};
> > +	int clock, common_len;
> > +
> > +	common_len = intel_dp_common_rates(intel_dp, common_rates);
> > +	if (WARN_ON(common_len <= 0))
> > +		return 0;
> > +
> > +	if (!IS_BROXTON(dev))
> > +		return 0;
> > +
> > +	drm_modeset_acquire_init(&ctx, 0);
> > +retry:
> > +	ret = drm_modeset_lock(&config->connection_mutex, &ctx);
> > +	if (ret)
> > +		goto exit_fail;
> > +
> > +	if (intel_encoder->base.crtc) {
> > +		crtc = intel_encoder->base.crtc;
> > +
> > +		ret = drm_modeset_lock(&crtc->mutex, &ctx);
> > +		if (ret)
> > +			goto exit_fail;
> > +
> > +		ret = drm_modeset_lock(&crtc->primary->mutex, &ctx);
> > +		if (ret)
> > +			goto exit_fail;
> > +
> > +		intel_crtc = to_intel_crtc(crtc);
> > +		ret = intel_dp_upfront_crtc_disable(intel_crtc, &ctx, false);
> > +		if (ret)
> > +			goto exit_fail;
> 
> Magically disabling stuff doens't seem right. OTOH I don't have a good
> idea how we'd do this if the user yanks the cable and plugs it back in
> before userspace has had a chance to shut down the pipe(s). Postpone the
> detection until it's all clear, or something? Anyone have good ideas for
> that?
> 
> > +	}
> > +
> > +	mutex_lock(&dev_priv->dpll_lock);
> > +
> > +	for (lane_count = max_lanes; lane_count >= 1 && !done; lane_count
> >>= 1) {
> > +		for (clock = common_len - 1; clock >= 0 && !done; clock--) {
> > +			done = __intel_dp_upfront_link_train(intel_dp,
> > +					common_rates[clock], lane_count);
> > +			if (done) {
> > +				intel_dp->max_lanes_upfront = lane_count;
> > +				intel_dp->max_link_bw_upfront =
> > +
> 	drm_dp_link_rate_to_bw_code(common_rates[clock]);
> > +				break;
> > +			}
> > +		}
> > +	}
> > +
> > +	mutex_unlock(&dev_priv->dpll_lock);
> > +
> > +	if (crtc)
> > +		ret = intel_dp_upfront_crtc_disable(intel_crtc, &ctx, true);
> > +
> > +exit_fail:
> > +	if (ret == -EDEADLK) {
> > +		drm_modeset_backoff(&ctx);
> > +		goto retry;
> > +	}
> > +	drm_modeset_drop_locks(&ctx);
> > +	drm_modeset_acquire_fini(&ctx);
> > +	return done;
> > +}
> > +
> >  static void
> >  intel_dp_long_pulse(struct intel_connector *intel_connector)
> >  {
> > @@ -4150,7 +4305,7 @@ intel_dp_long_pulse(struct intel_connector
> *intel_connector)
> >  	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;
> >
> >  	power_domain =
> intel_display_port_aux_power_domain(intel_encoder);
> > @@ -4235,10 +4390,22 @@ intel_dp_long_pulse(struct intel_connector
> *intel_connector)
> >  			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_dp->upfront_done &&
> > +		intel_encoder->type == INTEL_OUTPUT_DISPLAYPORT &&
> > +		intel_dp->compliance_test_type !=
> DP_TEST_LINK_TRAINING;
> > +
> > +	if (do_upfront_link_train) {
> > +		intel_dp->upfront_done =
> intel_dp_upfront_link_train(intel_dp);
> > +		if (!intel_dp->upfront_done)
> > +			status = connector_status_disconnected;
> > +	}
> > +
> >  out:
> > -	if ((status != connector_status_connected) &&
> > -	    (intel_dp->is_mst == false))
> > +	if (status != connector_status_connected && !intel_dp->is_mst) {
> >  		intel_dp_unset_edid(intel_dp);
> > +		intel_dp->upfront_done = false;
> > +	}
> >
> >  	intel_display_power_put(to_i915(dev), power_domain);
> >  	return;
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> > index bcf570f..060ea9b 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -832,6 +832,12 @@ struct intel_dp {
> >  	enum hdmi_force_audio force_audio;
> >  	bool limited_color_range;
> >  	bool color_range_auto;
> > +
> > +	/* Upfront link train parameters */
> > +	int max_link_bw_upfront;
> > +	uint8_t max_lanes_upfront;
> > +	bool upfront_done;
> > +
> >  	uint8_t dpcd[DP_RECEIVER_CAP_SIZE];
> >  	uint8_t psr_dpcd[EDP_PSR_RECEIVER_CAP_SIZE];
> >  	uint8_t downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
> > @@ -1113,6 +1119,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_bxt_upfront_link_train(struct intel_dp *intel_dp,
> > +				int clock, uint8_t lane_count);
> >
> >  /* 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
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> --
> Ville Syrjälä
> Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCHv6 5/5] drm/i915/dp: Enable Upfront link training for typeC DP support on BXT
  2016-05-26 10:03     ` R, Durgadoss
@ 2016-05-26 11:30       ` Ville Syrjälä
  0 siblings, 0 replies; 16+ messages in thread
From: Ville Syrjälä @ 2016-05-26 11:30 UTC (permalink / raw)
  To: R, Durgadoss; +Cc: Conselvan De Oliveira, Ander, intel-gfx

On Thu, May 26, 2016 at 10:03:22AM +0000, R, Durgadoss wrote:
> 
> > -----Original Message-----
> > From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
> > Sent: Wednesday, May 25, 2016 9:05 PM
> > To: R, Durgadoss <durgadoss.r@intel.com>
> > Cc: intel-gfx@lists.freedesktop.org; Conselvan De Oliveira, Ander
> > <ander.conselvan.de.oliveira@intel.com>
> > Subject: Re: [Intel-gfx] [PATCHv6 5/5] drm/i915/dp: Enable Upfront link
> > training for typeC DP support on BXT
> > 
> > On Fri, May 20, 2016 at 02:29:02PM +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 DP encoder.
> > > * On Connected boot scenarios: When booted with an LFP and a DP,
> > >   sometimes BIOS brings up DP. In these cases, we disable the
> > >   crtc and then do upfront link training; and bring it back up.
> > > * 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.
> > >
> > > Changes since v5:
> > > * Moved retry logic in upfront to intel_dp.c so that it
> > >   can be used for all platforms.
> > > Changes since v4:
> > > * Removed usage of crtc_state in upfront link training;
> > >   Hence no need to find free crtc to do upfront now.
> > > * Re-enable crtc if it was disabled for upfront.
> > > * Use separate variables to track max lane count
> > >   and link rate found by upfront, without modifying
> > >   the original DPCD read from panel.
> > > Changes since v3:
> > > * Fixed a return value on BXT check
> > > * Reworked on top of bxt_ddi_pll_select split from Ander
> > > * Renamed from ddi_upfront to bxt_upfront since the
> > >   upfront logic includes BXT specific functions for now.
> > > Changes since v2:
> > > * Rebased on top of latest dpll_mgr.c code and
> > >   latest HPD related clean ups.
> > > * Corrected return values from upfront (Ander)
> > > * Corrected atomic locking for upfront in intel_dp.c (Ville)
> > > Changes since v1:
> > > *  all pll related functions inside ddi.c
> > >
> > > Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_ddi.c |  45 ++++++++++
> > >  drivers/gpu/drm/i915/intel_dp.c  | 179
> > +++++++++++++++++++++++++++++++++++++--
> > >  drivers/gpu/drm/i915/intel_drv.h |   8 ++
> > >  3 files changed, 226 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> > b/drivers/gpu/drm/i915/intel_ddi.c
> > > index 7e6331a..8d224bf 100644
> > > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > > @@ -2330,6 +2330,51 @@ intel_ddi_init_hdmi_connector(struct
> > intel_digital_port *intel_dig_port)
> > >  	return connector;
> > >  }
> > >
> > > +bool intel_bxt_upfront_link_train(struct intel_dp *intel_dp,
> > > +				int clock, uint8_t lane_count)
> > > +{
> > > +	struct intel_connector *connector = intel_dp->attached_connector;
> > > +	struct intel_encoder *encoder = connector->encoder;
> > > +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > > +	struct intel_shared_dpll *pll;
> > > +	struct intel_shared_dpll_config tmp_pll_config;
> > > +	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> > > +	enum intel_dpll_id dpll_id = (enum intel_dpll_id)dig_port->port;
> > > +
> > > +	/*
> > > +	 * FIXME: Works only for BXT.
> > > +	 * Select the required PLL. This works for platforms where
> > > +	 * there is no shared DPLL.
> > > +	 */
> > > +	pll = &dev_priv->shared_dplls[dpll_id];
> > > +	if (WARN_ON(pll->active_mask)) {
> > > +		DRM_ERROR("Shared DPLL already in use.
> > active_mask:%x\n", pll->active_mask);
> > > +		return false;
> > > +	}
> > > +
> > > +	tmp_pll_config = pll->config;
> > > +
> > > +	if (!bxt_ddi_dp_set_dpll_hw_state(clock, &pll->config.hw_state)) {
> > > +		DRM_ERROR("Could not setup DPLL\n");
> > > +		pll->config = tmp_pll_config;
> > > +		return false;
> > > +	}
> > > +
> > > +	/* Enable PLL followed by port */
> > > +	pll->funcs.enable(dev_priv, pll);
> > > +	intel_ddi_pre_enable_dp(encoder, clock, lane_count, pll);
> > > +
> > > +	DRM_DEBUG_KMS("Upfront link train %s: link_clock:%d
> > lanes:%d\n",
> > > +	intel_dp->train_set_valid ? "Passed" : "Failed", clock, lane_count);
> > > +
> > > +	/* Disable port followed by PLL for next retry/clean up */
> > > +	intel_ddi_post_disable(encoder);
> > > +	pll->funcs.disable(dev_priv, pll);
> > > +
> > > +	pll->config = tmp_pll_config;
> > > +	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 95ba5aa..6ecaa30 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -147,18 +147,28 @@ intel_dp_max_link_bw(struct intel_dp
> > *intel_dp)
> > >  		max_link_bw = DP_LINK_BW_1_62;
> > >  		break;
> > >  	}
> > > -	return max_link_bw;
> > > +	/*
> > > +	 * Limit max link bw w.r.t to the max value found
> > > +	 * using Upfront link training also.
> > > +	 */
> > > +	return min(max_link_bw, intel_dp->max_link_bw_upfront);
> > 
> > This don't feel like the right place for this. I've tried to move
> > us away from the link_bw usage to using proper rate numbers.
> 
> Agreed, While doing these changes, I also felt it would be good to
> use any one of them consistently. So, will make it all  use link_rate.
> 
> > 
> > This should probably be handled somewhere in intel_dp_common_rates()
> > or perhaps just in intel_dp_max_link_rate().
> 
> Ok, moving it to intel_dp_common_rates() in the next version.
> 
> > 
> > >  }
> > >
> > >  static u8 intel_dp_max_lane_count(struct intel_dp *intel_dp)
> > >  {
> > >  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > > -	u8 source_max, sink_max;
> > > +	u8 temp, source_max, sink_max;
> > >
> > >  	source_max = intel_dig_port->max_lanes;
> > >  	sink_max = drm_dp_max_lane_count(intel_dp->dpcd);
> > >
> > > -	return min(source_max, sink_max);
> > > +	temp = min(source_max, sink_max);
> > > +
> > > +	/*
> > > +	 * Limit max lanes w.r.t to the max value found
> > > +	 * using Upfront link training also.
> > > +	 */
> > > +	return min(temp, intel_dp->max_lanes_upfront);
> > >  }
> > >
> > >  /*
> > > @@ -1590,6 +1600,15 @@ void intel_dp_set_link_params(struct intel_dp
> > *intel_dp,
> > >  	intel_dp->lane_count = lane_count;
> > >  }
> > >
> > > +static void intel_dp_init_upfront_params(struct intel_dp *intel_dp)
> > > +{
> > > +	if (WARN_ON(intel_dp->upfront_done))
> > > +		return;
> > > +
> > > +	intel_dp->max_link_bw_upfront = intel_dp-
> > >dpcd[DP_MAX_LINK_RATE];
> > > +	intel_dp->max_lanes_upfront = drm_dp_max_lane_count(intel_dp-
> > >dpcd);
> > > +}
> > > +
> > >  static void intel_dp_prepare(struct intel_encoder *encoder)
> > >  {
> > >  	struct drm_device *dev = encoder->base.dev;
> > > @@ -3424,6 +3443,16 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
> > >  		intel_dp->num_sink_rates = i;
> > >  	}
> > >
> > > +	/*
> > > +	 * The link_bw and lane count vaues are initialized to MAX possible
> > > +	 * value for all encoder types i.e DP, eDP, DP-MST, so that the
> > > +	 * intel_dp_max_{link_bw/lane_count} APIs do not have to worry
> > > +	 * about encoder types. They further cap the max w.r.t the upfront
> > > +	 * values also.
> > > +	 */
> > > +	if (!intel_dp->upfront_done)
> > > +		intel_dp_init_upfront_params(intel_dp);
> > 
> > With all the modeset locks involved there, I have a bad feeling this
> > ends up getting called from the hpd pulse work at the wrong time
> > perhaps leading to a deadlock.
> 
> With some code changes, there is no need for this init_params.
> Will remove this in next version.

I put this comment in the wrong place. I mean the call to
intel_dp_long_pulse() from the hpd_pulse. That looks dangerous to me.

> 
> + Also making upfront_link_train as a vfunc inside intel_dp,
> as Ander pointed out.
> 
> Thanks,
> Durga
> 
> > 
> > > +
> > >  	intel_dp_print_rates(intel_dp);
> > >
> > >  	if (!(intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] &
> > > @@ -4140,6 +4169,132 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
> > >  	intel_dp->has_audio = false;
> > >  }
> > >
> > > +static int intel_dp_upfront_crtc_disable(struct intel_crtc *crtc,
> > > +				struct drm_modeset_acquire_ctx *ctx,
> > > +				bool enable)
> > > +{
> > > +	int ret;
> > > +	struct drm_atomic_state *state;
> > > +	struct intel_crtc_state *crtc_state;
> > > +	struct drm_device *dev = crtc->base.dev;
> > > +	enum pipe pipe = crtc->pipe;
> > > +
> > > +	state = drm_atomic_state_alloc(dev);
> > > +	if (!state)
> > > +		return -ENOMEM;
> > > +
> > > +	state->acquire_ctx = ctx;
> > > +
> > > +	crtc_state = intel_atomic_get_crtc_state(state, crtc);
> > > +	if (IS_ERR(crtc_state)) {
> > > +		ret = PTR_ERR(crtc_state);
> > > +		drm_atomic_state_free(state);
> > > +		return ret;
> > > +	}
> > > +
> > > +	DRM_DEBUG_KMS("%sabling crtc %c %s upfront link train\n",
> > > +			enable ? "En" : "Dis",
> > > +			pipe_name(pipe),
> > > +			enable ? "after" : "before");
> > > +
> > > +	crtc_state->base.active = enable;
> > > +	ret = drm_atomic_commit(state);
> > > +	if (ret)
> > > +		drm_atomic_state_free(state);
> > > +
> > > +	return ret;
> > > +}
> > > +
> > > +static bool __intel_dp_upfront_link_train(struct intel_dp *intel_dp,
> > > +				int clock, uint8_t lane_count)
> > > +{
> > > +	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > > +	struct drm_i915_private *dev_priv =
> > > +				to_i915(intel_dig_port->base.base.dev);
> > > +
> > > +	if (IS_BROXTON(dev_priv))
> > > +		return intel_bxt_upfront_link_train(intel_dp, clock,
> > lane_count);
> > > +	/* Other platform calls go here */
> > > +
> > > +	/* Return true for unsupported platforms */
> > > +	return true;
> > > +}
> > > +static bool intel_dp_upfront_link_train(struct intel_dp *intel_dp)
> > > +{
> > > +	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_i915_private *dev_priv = to_i915(dev);
> > > +	struct drm_mode_config *config = &dev->mode_config;
> > > +	struct drm_modeset_acquire_ctx ctx;
> > > +	struct intel_crtc *intel_crtc;
> > > +	struct drm_crtc *crtc = NULL;
> > > +	int ret;
> > > +	bool done = false;
> > > +	uint8_t lane_count, max_lanes = intel_dp->max_lanes_upfront;
> > > +	int common_rates[DP_MAX_SUPPORTED_RATES] = {};
> > > +	int clock, common_len;
> > > +
> > > +	common_len = intel_dp_common_rates(intel_dp, common_rates);
> > > +	if (WARN_ON(common_len <= 0))
> > > +		return 0;
> > > +
> > > +	if (!IS_BROXTON(dev))
> > > +		return 0;
> > > +
> > > +	drm_modeset_acquire_init(&ctx, 0);
> > > +retry:
> > > +	ret = drm_modeset_lock(&config->connection_mutex, &ctx);
> > > +	if (ret)
> > > +		goto exit_fail;
> > > +
> > > +	if (intel_encoder->base.crtc) {
> > > +		crtc = intel_encoder->base.crtc;
> > > +
> > > +		ret = drm_modeset_lock(&crtc->mutex, &ctx);
> > > +		if (ret)
> > > +			goto exit_fail;
> > > +
> > > +		ret = drm_modeset_lock(&crtc->primary->mutex, &ctx);
> > > +		if (ret)
> > > +			goto exit_fail;
> > > +
> > > +		intel_crtc = to_intel_crtc(crtc);
> > > +		ret = intel_dp_upfront_crtc_disable(intel_crtc, &ctx, false);
> > > +		if (ret)
> > > +			goto exit_fail;
> > 
> > Magically disabling stuff doens't seem right. OTOH I don't have a good
> > idea how we'd do this if the user yanks the cable and plugs it back in
> > before userspace has had a chance to shut down the pipe(s). Postpone the
> > detection until it's all clear, or something? Anyone have good ideas for
> > that?
> > 
> > > +	}
> > > +
> > > +	mutex_lock(&dev_priv->dpll_lock);
> > > +
> > > +	for (lane_count = max_lanes; lane_count >= 1 && !done; lane_count
> > >>= 1) {
> > > +		for (clock = common_len - 1; clock >= 0 && !done; clock--) {
> > > +			done = __intel_dp_upfront_link_train(intel_dp,
> > > +					common_rates[clock], lane_count);
> > > +			if (done) {
> > > +				intel_dp->max_lanes_upfront = lane_count;
> > > +				intel_dp->max_link_bw_upfront =
> > > +
> > 	drm_dp_link_rate_to_bw_code(common_rates[clock]);
> > > +				break;
> > > +			}
> > > +		}
> > > +	}
> > > +
> > > +	mutex_unlock(&dev_priv->dpll_lock);
> > > +
> > > +	if (crtc)
> > > +		ret = intel_dp_upfront_crtc_disable(intel_crtc, &ctx, true);
> > > +
> > > +exit_fail:
> > > +	if (ret == -EDEADLK) {
> > > +		drm_modeset_backoff(&ctx);
> > > +		goto retry;
> > > +	}
> > > +	drm_modeset_drop_locks(&ctx);
> > > +	drm_modeset_acquire_fini(&ctx);
> > > +	return done;
> > > +}
> > > +
> > >  static void
> > >  intel_dp_long_pulse(struct intel_connector *intel_connector)
> > >  {
> > > @@ -4150,7 +4305,7 @@ intel_dp_long_pulse(struct intel_connector
> > *intel_connector)
> > >  	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;
> > >
> > >  	power_domain =
> > intel_display_port_aux_power_domain(intel_encoder);
> > > @@ -4235,10 +4390,22 @@ intel_dp_long_pulse(struct intel_connector
> > *intel_connector)
> > >  			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_dp->upfront_done &&
> > > +		intel_encoder->type == INTEL_OUTPUT_DISPLAYPORT &&
> > > +		intel_dp->compliance_test_type !=
> > DP_TEST_LINK_TRAINING;
> > > +
> > > +	if (do_upfront_link_train) {
> > > +		intel_dp->upfront_done =
> > intel_dp_upfront_link_train(intel_dp);
> > > +		if (!intel_dp->upfront_done)
> > > +			status = connector_status_disconnected;
> > > +	}
> > > +
> > >  out:
> > > -	if ((status != connector_status_connected) &&
> > > -	    (intel_dp->is_mst == false))
> > > +	if (status != connector_status_connected && !intel_dp->is_mst) {
> > >  		intel_dp_unset_edid(intel_dp);
> > > +		intel_dp->upfront_done = false;
> > > +	}
> > >
> > >  	intel_display_power_put(to_i915(dev), power_domain);
> > >  	return;
> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > b/drivers/gpu/drm/i915/intel_drv.h
> > > index bcf570f..060ea9b 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -832,6 +832,12 @@ struct intel_dp {
> > >  	enum hdmi_force_audio force_audio;
> > >  	bool limited_color_range;
> > >  	bool color_range_auto;
> > > +
> > > +	/* Upfront link train parameters */
> > > +	int max_link_bw_upfront;
> > > +	uint8_t max_lanes_upfront;
> > > +	bool upfront_done;
> > > +
> > >  	uint8_t dpcd[DP_RECEIVER_CAP_SIZE];
> > >  	uint8_t psr_dpcd[EDP_PSR_RECEIVER_CAP_SIZE];
> > >  	uint8_t downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
> > > @@ -1113,6 +1119,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_bxt_upfront_link_train(struct intel_dp *intel_dp,
> > > +				int clock, uint8_t lane_count);
> > >
> > >  /* 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
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > --
> > Ville Syrjälä
> > Intel OTC

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

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

* [PATCH 1/5] drm/i915: Don't pass crtc_state to intel_dp_set_link_params()
  2016-07-13 10:45 [PATCHv7 0/5] Add USB typeC based DP support for BXT platform Durgadoss R
@ 2016-07-13 10:45 ` Durgadoss R
  0 siblings, 0 replies; 16+ messages in thread
From: Durgadoss R @ 2016-07-13 10:45 UTC (permalink / raw)
  To: intel-gfx; +Cc: ander.conselvan.de.oliveira, jim.bride

From: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>

Decouple intel_dp_set_link_params() from struct intel_crtc_state. This
will be useful for implementing DP upfront link training.

Reviewed-by: Durgadoss R <durgadoss.r@intel.com>
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c    | 3 ++-
 drivers/gpu/drm/i915/intel_dp.c     | 9 +++++----
 drivers/gpu/drm/i915/intel_dp_mst.c | 4 +++-
 drivers/gpu/drm/i915/intel_drv.h    | 2 +-
 4 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index dd1d6fe..7d78262 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1627,7 +1627,8 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder)
 	if (type == INTEL_OUTPUT_DP || type == INTEL_OUTPUT_EDP) {
 		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
 
-		intel_dp_set_link_params(intel_dp, crtc->config);
+		intel_dp_set_link_params(intel_dp, crtc->config->port_clock,
+					 crtc->config->lane_count);
 
 		intel_ddi_init_dp_buf_reg(intel_encoder);
 
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 0c5ba34..817897c 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1647,10 +1647,10 @@ found:
 }
 
 void intel_dp_set_link_params(struct intel_dp *intel_dp,
-			      const struct intel_crtc_state *pipe_config)
+			      int link_rate, uint8_t lane_count)
 {
-	intel_dp->link_rate = pipe_config->port_clock;
-	intel_dp->lane_count = pipe_config->lane_count;
+	intel_dp->link_rate = link_rate;
+	intel_dp->lane_count = lane_count;
 }
 
 static void intel_dp_prepare(struct intel_encoder *encoder)
@@ -1662,7 +1662,8 @@ static void intel_dp_prepare(struct intel_encoder *encoder)
 	struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
 	const struct drm_display_mode *adjusted_mode = &crtc->config->base.adjusted_mode;
 
-	intel_dp_set_link_params(intel_dp, crtc->config);
+	intel_dp_set_link_params(intel_dp, crtc->config->port_clock,
+				 crtc->config->lane_count);
 
 	/*
 	 * There are four kinds of DP registers:
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index 68a005d..fd8f6a3 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -174,7 +174,9 @@ static void intel_mst_pre_enable_dp(struct intel_encoder *encoder)
 
 		intel_ddi_clk_select(&intel_dig_port->base, intel_crtc->config);
 
-		intel_dp_set_link_params(intel_dp, intel_crtc->config);
+		intel_dp_set_link_params(intel_dp,
+					 intel_crtc->config->port_clock,
+					 intel_crtc->config->lane_count);
 
 		intel_ddi_init_dp_buf_reg(&intel_dig_port->base);
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 6c8485c..be37b98 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1355,7 +1355,7 @@ bool intel_dp_init(struct drm_device *dev, i915_reg_t output_reg, enum port port
 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);
+				int link_rate, uint8_t lane_count);
 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
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [PATCH 1/5] drm/i915: Don't pass crtc_state to intel_dp_set_link_params()
  2016-05-16  6:26 [PATCHv5 0/5] Add USB typeC based DP support for BXT platform Durgadoss R
@ 2016-05-16  6:26 ` Durgadoss R
  0 siblings, 0 replies; 16+ messages in thread
From: Durgadoss R @ 2016-05-16  6:26 UTC (permalink / raw)
  To: intel-gfx; +Cc: ander.conselvan.de.oliveira

From: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>

Decouple intel_dp_set_link_params() from struct intel_crtc_state. This
will be useful for implementing DP upfront link training.

Reviewed-by: Durgadoss R <durgadoss.r@intel.com>
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c    | 3 ++-
 drivers/gpu/drm/i915/intel_dp.c     | 9 +++++----
 drivers/gpu/drm/i915/intel_dp_mst.c | 4 +++-
 drivers/gpu/drm/i915/intel_drv.h    | 2 +-
 4 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index c454744..1cbdd66 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1619,7 +1619,8 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder)
 	if (type == INTEL_OUTPUT_DISPLAYPORT || type == INTEL_OUTPUT_EDP) {
 		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
 
-		intel_dp_set_link_params(intel_dp, crtc->config);
+		intel_dp_set_link_params(intel_dp, crtc->config->port_clock,
+					 crtc->config->lane_count);
 
 		intel_ddi_init_dp_buf_reg(intel_encoder);
 
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 3633002..95ba5aa 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1584,10 +1584,10 @@ found:
 }
 
 void intel_dp_set_link_params(struct intel_dp *intel_dp,
-			      const struct intel_crtc_state *pipe_config)
+			      int link_rate, uint8_t lane_count)
 {
-	intel_dp->link_rate = pipe_config->port_clock;
-	intel_dp->lane_count = pipe_config->lane_count;
+	intel_dp->link_rate = link_rate;
+	intel_dp->lane_count = lane_count;
 }
 
 static void intel_dp_prepare(struct intel_encoder *encoder)
@@ -1599,7 +1599,8 @@ static void intel_dp_prepare(struct intel_encoder *encoder)
 	struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
 	const struct drm_display_mode *adjusted_mode = &crtc->config->base.adjusted_mode;
 
-	intel_dp_set_link_params(intel_dp, crtc->config);
+	intel_dp_set_link_params(intel_dp, crtc->config->port_clock,
+				 crtc->config->lane_count);
 
 	/*
 	 * There are four kinds of DP registers:
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index 7a34090..553a25e 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -175,7 +175,9 @@ static void intel_mst_pre_enable_dp(struct intel_encoder *encoder)
 
 		intel_ddi_clk_select(&intel_dig_port->base, intel_crtc->config);
 
-		intel_dp_set_link_params(intel_dp, intel_crtc->config);
+		intel_dp_set_link_params(intel_dp,
+					 intel_crtc->config->port_clock,
+					 intel_crtc->config->lane_count);
 
 		intel_ddi_init_dp_buf_reg(&intel_dig_port->base);
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 8405ff7..80a1dbd 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1291,7 +1291,7 @@ void intel_dp_init(struct drm_device *dev, i915_reg_t output_reg, enum port port
 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);
+				int link_rate, uint8_t lane_count);
 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
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

end of thread, other threads:[~2016-07-13 10:00 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-20  8:58 [PATCHv6 0/5] Add USB typeC based DP support for BXT platform Durgadoss R
2016-05-20  8:58 ` [PATCH 1/5] drm/i915: Don't pass crtc_state to intel_dp_set_link_params() Durgadoss R
2016-05-20  8:58 ` [PATCH 2/5] drm/i915: Remove ddi_pll_sel from intel_crtc_state Durgadoss R
2016-05-24  7:32   ` [PATCH v2] " Ander Conselvan de Oliveira
2016-05-24  7:51     ` R, Durgadoss
2016-05-20  8:59 ` [PATCH 3/5] drm/i915: Split intel_ddi_pre_enable() into DP and HDMI versions Durgadoss R
2016-05-20  8:59 ` [PATCHv2 4/5] drm/i915: Split bxt_ddi_pll_select() Durgadoss R
2016-05-20  8:59 ` [PATCHv6 5/5] drm/i915/dp: Enable Upfront link training for typeC DP support on BXT Durgadoss R
2016-05-24  7:40   ` Ander Conselvan De Oliveira
2016-05-25 15:35   ` Ville Syrjälä
2016-05-26 10:03     ` R, Durgadoss
2016-05-26 11:30       ` Ville Syrjälä
2016-05-20  9:41 ` ✗ Ro.CI.BAT: warning for Add USB typeC based DP support for BXT platform (rev7) Patchwork
2016-05-24  7:56 ` ✗ Ro.CI.BAT: warning for Add USB typeC based DP support for BXT platform (rev8) Patchwork
  -- strict thread matches above, loose matches on Subject: below --
2016-07-13 10:45 [PATCHv7 0/5] Add USB typeC based DP support for BXT platform Durgadoss R
2016-07-13 10:45 ` [PATCH 1/5] drm/i915: Don't pass crtc_state to intel_dp_set_link_params() Durgadoss R
2016-05-16  6:26 [PATCHv5 0/5] Add USB typeC based DP support for BXT platform Durgadoss R
2016-05-16  6:26 ` [PATCH 1/5] drm/i915: Don't pass crtc_state to intel_dp_set_link_params() Durgadoss R

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.