[PATCH 00/10] drm/i915: Last part of DDI encoder->type cleanup

public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed

* [PATCH 00/10] drm/i915: Last part of DDI encoder->type cleanup
@ 2017-10-19 13:37 Ville Syrjala
  2017-10-19 13:37 ` [PATCH 01/10] drm/i915: Don't use encoder->type in intel_ddi_set_pipe_settings() Ville Syrjala
                   ` (12 more replies)
  0 siblings, 13 replies; 27+ messages in thread
From: Ville Syrjala @ 2017-10-19 13:37 UTC (permalink / raw)
  To: intel-gfx

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

Here's the last chunk of my DDI encoder->type frobbery elimnination. Afterwards
DP/HDMI DDI encoders will have "DDI" as their type permanently, and instead one
needs to consult the crtc state output_types to determine in which mode we're
driving the port. For eDP DDI encoder we leave the type as "EDP" permently
since there's quite a bit of code that relies on that being the case. It also
allows us to tell DP vs. eDP apart in the DP link training code where we don't
currently have the crtc state around as such.

I also tossed in a bit of semi-related MST polish on top.

Entire series available here:
git://github.com/vsyrjala/linux.git ddi_output_types_4

Ville Syrjälä (10):
  drm/i915: Don't use encoder->type in intel_ddi_set_pipe_settings()
  drm/i915: Pass crtc state to intel_prepare_dp_ddi_buffers()
  drm/i915: Start using output_types for DPLL selection
  drm/i915: Stop using encoder->type in
    intel_ddi_enable_transcoder_func()
  drm/i915: Stop frobbing with DDI encoder->type
  drm/i915: Nuke intel_ddi_get_encoder_port()
  drm/i915: Eliminate pll->state usage from bxt_calc_pll_link()
  drm/i915: Pass a crtc state to ddi post_disable from MST code
  drm/i915: Use intel_ddi_get_config() for MST
  drm/i915: Use intel_crtc_has_dp_encoder() for LPE audio

 drivers/gpu/drm/i915/i915_debugfs.c   |   2 +-
 drivers/gpu/drm/i915/intel_audio.c    |   2 +-
 drivers/gpu/drm/i915/intel_crt.c      |   2 +
 drivers/gpu/drm/i915/intel_ddi.c      | 219 ++++++++++++++++------------------
 drivers/gpu/drm/i915/intel_display.c  |  16 +--
 drivers/gpu/drm/i915/intel_dp.c       |  24 ++--
 drivers/gpu/drm/i915/intel_dp_mst.c   |  48 +-------
 drivers/gpu/drm/i915/intel_dpll_mgr.c |  36 ++----
 drivers/gpu/drm/i915/intel_drv.h      |   8 +-
 drivers/gpu/drm/i915/intel_dsi.c      |   2 +
 drivers/gpu/drm/i915/intel_dvo.c      |   2 +
 drivers/gpu/drm/i915/intel_hdmi.c     |  12 +-
 drivers/gpu/drm/i915/intel_lvds.c     |   2 +
 drivers/gpu/drm/i915/intel_opregion.c |   4 +-
 drivers/gpu/drm/i915/intel_sdvo.c     |   2 +
 drivers/gpu/drm/i915/intel_tv.c       |   2 +
 16 files changed, 167 insertions(+), 216 deletions(-)

-- 
2.13.6

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

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

* [PATCH 01/10] drm/i915: Don't use encoder->type in intel_ddi_set_pipe_settings()
  2017-10-19 13:37 [PATCH 00/10] drm/i915: Last part of DDI encoder->type cleanup Ville Syrjala
@ 2017-10-19 13:37 ` Ville Syrjala
  2017-10-19 13:37 ` [PATCH 02/10] drm/i915: Pass crtc state to intel_prepare_dp_ddi_buffers() Ville Syrjala
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 27+ messages in thread
From: Ville Syrjala @ 2017-10-19 13:37 UTC (permalink / raw)
  To: intel-gfx

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

encoder->type isn't reliable for DP/HDMI so instead extract the correct
type from the crtc state in intel_ddi_set_pipe_settings().

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

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index b8925bc82f30..c69963d57a9d 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1502,33 +1502,34 @@ void intel_ddi_set_pipe_settings(const struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-	struct intel_encoder *encoder = intel_ddi_get_crtc_encoder(crtc);
 	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
-	int type = encoder->type;
-	uint32_t temp;
+	u32 temp;
 
-	if (type == INTEL_OUTPUT_DP || type == INTEL_OUTPUT_EDP || type == INTEL_OUTPUT_DP_MST) {
-		WARN_ON(transcoder_is_dsi(cpu_transcoder));
+	if (!intel_crtc_has_dp_encoder(crtc_state))
+		return;
 
-		temp = TRANS_MSA_SYNC_CLK;
-		switch (crtc_state->pipe_bpp) {
-		case 18:
-			temp |= TRANS_MSA_6_BPC;
-			break;
-		case 24:
-			temp |= TRANS_MSA_8_BPC;
-			break;
-		case 30:
-			temp |= TRANS_MSA_10_BPC;
-			break;
-		case 36:
-			temp |= TRANS_MSA_12_BPC;
-			break;
-		default:
-			BUG();
-		}
-		I915_WRITE(TRANS_MSA_MISC(cpu_transcoder), temp);
+	WARN_ON(transcoder_is_dsi(cpu_transcoder));
+
+	temp = TRANS_MSA_SYNC_CLK;
+	switch (crtc_state->pipe_bpp) {
+	case 18:
+		temp |= TRANS_MSA_6_BPC;
+		break;
+	case 24:
+		temp |= TRANS_MSA_8_BPC;
+		break;
+	case 30:
+		temp |= TRANS_MSA_10_BPC;
+		break;
+	case 36:
+		temp |= TRANS_MSA_12_BPC;
+		break;
+	default:
+		MISSING_CASE(crtc_state->pipe_bpp);
+		break;
 	}
+
+	I915_WRITE(TRANS_MSA_MISC(cpu_transcoder), temp);
 }
 
 void intel_ddi_set_vc_payload_alloc(const struct intel_crtc_state *crtc_state,
-- 
2.13.6

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

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

* [PATCH 02/10] drm/i915: Pass crtc state to intel_prepare_dp_ddi_buffers()
  2017-10-19 13:37 [PATCH 00/10] drm/i915: Last part of DDI encoder->type cleanup Ville Syrjala
  2017-10-19 13:37 ` [PATCH 01/10] drm/i915: Don't use encoder->type in intel_ddi_set_pipe_settings() Ville Syrjala
@ 2017-10-19 13:37 ` Ville Syrjala
  2017-10-19 13:37 ` [PATCH 03/10] drm/i915: Start using output_types for DPLL selection Ville Syrjala
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 27+ messages in thread
From: Ville Syrjala @ 2017-10-19 13:37 UTC (permalink / raw)
  To: intel-gfx

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

Eliminate intel_prepare_dp_ddi_buffers()'s reliance on the encoder->type
by passing in the crtc state.

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

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index c69963d57a9d..9f154f6fd4f3 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -811,7 +811,8 @@ static int intel_ddi_hdmi_level(struct drm_i915_private *dev_priv, enum port por
  * values in advance. This function programs the correct values for
  * DP/eDP/FDI use cases.
  */
-static void intel_prepare_dp_ddi_buffers(struct intel_encoder *encoder)
+static void intel_prepare_dp_ddi_buffers(struct intel_encoder *encoder,
+					 const struct intel_crtc_state *crtc_state)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	u32 iboost_bit = 0;
@@ -819,23 +820,15 @@ static void intel_prepare_dp_ddi_buffers(struct intel_encoder *encoder)
 	enum port port = intel_ddi_get_encoder_port(encoder);
 	const struct ddi_buf_trans *ddi_translations;
 
-	switch (encoder->type) {
-	case INTEL_OUTPUT_EDP:
+	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_ANALOG))
+		ddi_translations = intel_ddi_get_buf_trans_fdi(dev_priv,
+							       &n_entries);
+	else if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_EDP))
 		ddi_translations = intel_ddi_get_buf_trans_edp(dev_priv, port,
 							       &n_entries);
-		break;
-	case INTEL_OUTPUT_DP:
+	else
 		ddi_translations = intel_ddi_get_buf_trans_dp(dev_priv, port,
 							      &n_entries);
-		break;
-	case INTEL_OUTPUT_ANALOG:
-		ddi_translations = intel_ddi_get_buf_trans_fdi(dev_priv,
-							       &n_entries);
-		break;
-	default:
-		MISSING_CASE(encoder->type);
-		return;
-	}
 
 	/* If we're boosting the current, set bit 31 of trans1 */
 	if (IS_GEN9_BC(dev_priv) &&
@@ -937,7 +930,7 @@ void hsw_fdi_link_train(struct intel_crtc *crtc,
 
 	for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
 		WARN_ON(encoder->type != INTEL_OUTPUT_ANALOG);
-		intel_prepare_dp_ddi_buffers(encoder);
+		intel_prepare_dp_ddi_buffers(encoder, crtc_state);
 	}
 
 	/* Set the FDI_RX_MISC pwrdn lanes and the 2 workarounds listed at the
@@ -2199,7 +2192,7 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
 	else if (IS_GEN9_LP(dev_priv))
 		bxt_ddi_vswing_sequence(encoder, level, encoder->type);
 	else
-		intel_prepare_dp_ddi_buffers(encoder);
+		intel_prepare_dp_ddi_buffers(encoder, crtc_state);
 
 	intel_ddi_init_dp_buf_reg(encoder);
 	if (!is_mst)
-- 
2.13.6

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

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

* [PATCH 03/10] drm/i915: Start using output_types for DPLL selection
  2017-10-19 13:37 [PATCH 00/10] drm/i915: Last part of DDI encoder->type cleanup Ville Syrjala
  2017-10-19 13:37 ` [PATCH 01/10] drm/i915: Don't use encoder->type in intel_ddi_set_pipe_settings() Ville Syrjala
  2017-10-19 13:37 ` [PATCH 02/10] drm/i915: Pass crtc state to intel_prepare_dp_ddi_buffers() Ville Syrjala
@ 2017-10-19 13:37 ` Ville Syrjala
  2017-10-19 13:37 ` [PATCH 04/10] drm/i915: Stop using encoder->type in intel_ddi_enable_transcoder_func() Ville Syrjala
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 27+ messages in thread
From: Ville Syrjala @ 2017-10-19 13:37 UTC (permalink / raw)
  To: intel-gfx

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

encoder->type is not realiable for DP/HDMI so let's switch the DPLL
selection over to using output_types.

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

diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index df808a94c511..5d88a5bae19b 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -813,15 +813,11 @@ hsw_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
 	memset(&crtc_state->dpll_hw_state, 0,
 	       sizeof(crtc_state->dpll_hw_state));
 
-	if (encoder->type == INTEL_OUTPUT_HDMI) {
+	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI)) {
 		pll = hsw_ddi_hdmi_get_dpll(clock, crtc, crtc_state);
-
-	} else if (encoder->type == INTEL_OUTPUT_DP ||
-		   encoder->type == INTEL_OUTPUT_DP_MST ||
-		   encoder->type == INTEL_OUTPUT_EDP) {
+	} else if (intel_crtc_has_dp_encoder(crtc_state)) {
 		pll = hsw_ddi_dp_get_dpll(encoder, clock);
-
-	} else if (encoder->type == INTEL_OUTPUT_ANALOG) {
+	} else if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_ANALOG)) {
 		if (WARN_ON(crtc_state->port_clock / 2 != 135000))
 			return NULL;
 
@@ -1369,15 +1365,13 @@ skl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
 
 	memset(&dpll_hw_state, 0, sizeof(dpll_hw_state));
 
-	if (encoder->type == INTEL_OUTPUT_HDMI) {
+	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI)) {
 		bret = skl_ddi_hdmi_pll_dividers(crtc, crtc_state, clock);
 		if (!bret) {
 			DRM_DEBUG_KMS("Could not get HDMI pll dividers.\n");
 			return NULL;
 		}
-	} else if (encoder->type == INTEL_OUTPUT_DP ||
-		   encoder->type == INTEL_OUTPUT_DP_MST ||
-		   encoder->type == INTEL_OUTPUT_EDP) {
+	} else if (intel_crtc_has_dp_encoder(crtc_state)) {
 		bret = skl_ddi_dp_set_dpll_hw_state(clock, &dpll_hw_state);
 		if (!bret) {
 			DRM_DEBUG_KMS("Could not set DP dpll HW state.\n");
@@ -1388,7 +1382,7 @@ skl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
 		return NULL;
 	}
 
-	if (encoder->type == INTEL_OUTPUT_EDP)
+	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_EDP))
 		pll = intel_find_shared_dpll(crtc, crtc_state,
 					     DPLL_ID_SKL_DPLL0,
 					     DPLL_ID_SKL_DPLL0);
@@ -1812,14 +1806,12 @@ bxt_get_dpll(struct intel_crtc *crtc,
 	struct intel_shared_dpll *pll;
 	int i, clock = crtc_state->port_clock;
 
-	if (encoder->type == INTEL_OUTPUT_HDMI &&
+	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI) &&
 	    !bxt_ddi_hdmi_set_dpll_hw_state(crtc, crtc_state, clock,
 					    &dpll_hw_state))
 		return NULL;
 
-	if ((encoder->type == INTEL_OUTPUT_DP ||
-	     encoder->type == INTEL_OUTPUT_EDP ||
-	     encoder->type == INTEL_OUTPUT_DP_MST) &&
+	if (intel_crtc_has_dp_encoder(crtc_state) &&
 	    !bxt_ddi_dp_set_dpll_hw_state(clock, &dpll_hw_state))
 		return NULL;
 
@@ -1828,7 +1820,7 @@ bxt_get_dpll(struct intel_crtc *crtc,
 
 	crtc_state->dpll_hw_state = dpll_hw_state;
 
-	if (encoder->type == INTEL_OUTPUT_DP_MST) {
+	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST)) {
 		struct intel_dp_mst_encoder *intel_mst = enc_to_mst(&encoder->base);
 
 		intel_dig_port = intel_mst->primary;
@@ -2345,15 +2337,13 @@ cnl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
 
 	memset(&dpll_hw_state, 0, sizeof(dpll_hw_state));
 
-	if (encoder->type == INTEL_OUTPUT_HDMI) {
+	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI)) {
 		bret = cnl_ddi_hdmi_pll_dividers(crtc, crtc_state, clock);
 		if (!bret) {
 			DRM_DEBUG_KMS("Could not get HDMI pll dividers.\n");
 			return NULL;
 		}
-	} else if (encoder->type == INTEL_OUTPUT_DP ||
-		   encoder->type == INTEL_OUTPUT_DP_MST ||
-		   encoder->type == INTEL_OUTPUT_EDP) {
+	} else if (intel_crtc_has_dp_encoder(crtc_state)) {
 		bret = cnl_ddi_dp_set_dpll_hw_state(clock, &dpll_hw_state);
 		if (!bret) {
 			DRM_DEBUG_KMS("Could not set DP dpll HW state.\n");
@@ -2361,8 +2351,8 @@ cnl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
 		}
 		crtc_state->dpll_hw_state = dpll_hw_state;
 	} else {
-		DRM_DEBUG_KMS("Skip DPLL setup for encoder %d\n",
-			      encoder->type);
+		DRM_DEBUG_KMS("Skip DPLL setup for output_types 0x%x\n",
+			      crtc_state->output_types);
 		return NULL;
 	}
 
-- 
2.13.6

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

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

* [PATCH 04/10] drm/i915: Stop using encoder->type in intel_ddi_enable_transcoder_func()
  2017-10-19 13:37 [PATCH 00/10] drm/i915: Last part of DDI encoder->type cleanup Ville Syrjala
                   ` (2 preceding siblings ...)
  2017-10-19 13:37 ` [PATCH 03/10] drm/i915: Start using output_types for DPLL selection Ville Syrjala
@ 2017-10-19 13:37 ` Ville Syrjala
  2017-10-19 13:37 ` [PATCH v5 05/10] drm/i915: Stop frobbing with DDI encoder->type Ville Syrjala
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 27+ messages in thread
From: Ville Syrjala @ 2017-10-19 13:37 UTC (permalink / raw)
  To: intel-gfx

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

intel_ddi_enable_transcoder_func() already has the crtc state so we can
use that instead of the untrustworthy encoder->type.

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

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 9f154f6fd4f3..c8790bb74839 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1548,7 +1548,6 @@ void intel_ddi_enable_transcoder_func(const struct intel_crtc_state *crtc_state)
 	enum pipe pipe = crtc->pipe;
 	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
 	enum port port = intel_ddi_get_encoder_port(encoder);
-	int type = encoder->type;
 	uint32_t temp;
 
 	/* Enable TRANS_DDI_FUNC_CTL for the pipe to work in HDMI mode */
@@ -1603,7 +1602,7 @@ void intel_ddi_enable_transcoder_func(const struct intel_crtc_state *crtc_state)
 		}
 	}
 
-	if (type == INTEL_OUTPUT_HDMI) {
+	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI)) {
 		if (crtc_state->has_hdmi_sink)
 			temp |= TRANS_DDI_MODE_SELECT_HDMI;
 		else
@@ -1613,19 +1612,15 @@ void intel_ddi_enable_transcoder_func(const struct intel_crtc_state *crtc_state)
 			temp |= TRANS_DDI_HDMI_SCRAMBLING_MASK;
 		if (crtc_state->hdmi_high_tmds_clock_ratio)
 			temp |= TRANS_DDI_HIGH_TMDS_CHAR_RATE;
-	} else if (type == INTEL_OUTPUT_ANALOG) {
+	} else if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_ANALOG)) {
 		temp |= TRANS_DDI_MODE_SELECT_FDI;
 		temp |= (crtc_state->fdi_lanes - 1) << 1;
-	} else if (type == INTEL_OUTPUT_DP ||
-		   type == INTEL_OUTPUT_EDP) {
-		temp |= TRANS_DDI_MODE_SELECT_DP_SST;
-		temp |= DDI_PORT_WIDTH(crtc_state->lane_count);
-	} else if (type == INTEL_OUTPUT_DP_MST) {
+	} else if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST)) {
 		temp |= TRANS_DDI_MODE_SELECT_DP_MST;
 		temp |= DDI_PORT_WIDTH(crtc_state->lane_count);
 	} else {
-		WARN(1, "Invalid encoder type %d for pipe %c\n",
-		     encoder->type, pipe_name(pipe));
+		temp |= TRANS_DDI_MODE_SELECT_DP_SST;
+		temp |= DDI_PORT_WIDTH(crtc_state->lane_count);
 	}
 
 	I915_WRITE(TRANS_DDI_FUNC_CTL(cpu_transcoder), temp);
-- 
2.13.6

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

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

* [PATCH v5 05/10] drm/i915: Stop frobbing with DDI encoder->type
  2017-10-19 13:37 [PATCH 00/10] drm/i915: Last part of DDI encoder->type cleanup Ville Syrjala
                   ` (3 preceding siblings ...)
  2017-10-19 13:37 ` [PATCH 04/10] drm/i915: Stop using encoder->type in intel_ddi_enable_transcoder_func() Ville Syrjala
@ 2017-10-19 13:37 ` Ville Syrjala
  2017-10-27 12:05   ` Maarten Lankhorst
  2017-10-19 13:37 ` [PATCH 06/10] drm/i915: Nuke intel_ddi_get_encoder_port() Ville Syrjala
                   ` (7 subsequent siblings)
  12 siblings, 1 reply; 27+ messages in thread
From: Ville Syrjala @ 2017-10-19 13:37 UTC (permalink / raw)
  To: intel-gfx

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

Currently the DDI encoder->type will change at runtime depending on
what kind of hotplugs we've processed. That's quite bad since we can't
really trust that that current value of encoder->type actually matches
the type of signal we're trying to drive through it.

Let's eliminate that problem by declaring that non-eDP DDI port will
always have the encoder type as INTEL_OUTPUT_DDI. This means the code
can no longer try to distinguish DP vs. HDMI based on encoder->type.
We'll leave eDP as INTEL_OUTPUT_EDP, since it'll never change and
there's a bunch of code that relies on that value to identify eDP
encoders.

We'll introduce a new encoder .compute_output_type() hook. This allows
us to compute the full output_types before any encoder .compute_config()
hooks get called, thus those hooks can rely on output_types being
correct, which is useful for cloning on oldr platforms. For now we'll
just look at the connector type and pick the correct mode based on that.
In the future the new hook could be used to implement dynamic switching
between LS and PCON modes for LSPCON.

TODO: maybe make .get_config() populate output_types rather than doing
the default encoder->type thing in caller and then undoing it for
DDI in .get_config().

v2: Fix BXT/GLK PPS explosion with DSI/MST encoders
v3: Avoid the PPS warn on pure HDMI/DVI DDI encoders by checking dp.output_reg
v4: Rebase
v5: Populate output_types in .get_config() rather than in the caller

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c   |  2 +-
 drivers/gpu/drm/i915/intel_crt.c      |  2 ++
 drivers/gpu/drm/i915/intel_ddi.c      | 43 ++++++++++++++++++++++++++++-------
 drivers/gpu/drm/i915/intel_display.c  | 16 +++++++------
 drivers/gpu/drm/i915/intel_dp.c       | 24 +++++++++----------
 drivers/gpu/drm/i915/intel_dp_mst.c   |  2 ++
 drivers/gpu/drm/i915/intel_drv.h      |  7 ++++--
 drivers/gpu/drm/i915/intel_dsi.c      |  2 ++
 drivers/gpu/drm/i915/intel_dvo.c      |  2 ++
 drivers/gpu/drm/i915/intel_hdmi.c     | 12 ++++------
 drivers/gpu/drm/i915/intel_lvds.c     |  2 ++
 drivers/gpu/drm/i915/intel_opregion.c |  2 +-
 drivers/gpu/drm/i915/intel_sdvo.c     |  2 ++
 drivers/gpu/drm/i915/intel_tv.c       |  2 ++
 14 files changed, 80 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index c65e381b85f3..e5333bfdf32d 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3049,7 +3049,7 @@ static void intel_connector_info(struct seq_file *m,
 		break;
 	case DRM_MODE_CONNECTOR_HDMIA:
 		if (intel_encoder->type == INTEL_OUTPUT_HDMI ||
-		    intel_encoder->type == INTEL_OUTPUT_UNKNOWN)
+		    intel_encoder->type == INTEL_OUTPUT_DDI)
 			intel_hdmi_info(m, intel_connector);
 		break;
 	default:
diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 668e8c3e791d..a61e61efe9aa 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -119,6 +119,8 @@ static unsigned int intel_crt_get_flags(struct intel_encoder *encoder)
 static void intel_crt_get_config(struct intel_encoder *encoder,
 				 struct intel_crtc_state *pipe_config)
 {
+	pipe_config->output_types |= BIT(INTEL_OUTPUT_ANALOG);
+
 	pipe_config->base.adjusted_mode.flags |= intel_crt_get_flags(encoder);
 
 	pipe_config->base.adjusted_mode.crtc_clock = pipe_config->port_clock;
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index c8790bb74839..630609575db4 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -497,10 +497,8 @@ enum port intel_ddi_get_encoder_port(struct intel_encoder *encoder)
 	switch (encoder->type) {
 	case INTEL_OUTPUT_DP_MST:
 		return enc_to_mst(&encoder->base)->primary->port;
-	case INTEL_OUTPUT_DP:
 	case INTEL_OUTPUT_EDP:
-	case INTEL_OUTPUT_HDMI:
-	case INTEL_OUTPUT_UNKNOWN:
+	case INTEL_OUTPUT_DDI:
 		return enc_to_dig_port(&encoder->base)->port;
 	case INTEL_OUTPUT_ANALOG:
 		return PORT_E;
@@ -1532,6 +1530,7 @@ void intel_ddi_set_vc_payload_alloc(const struct intel_crtc_state *crtc_state,
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
 	uint32_t temp;
+
 	temp = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
 	if (state == true)
 		temp |= TRANS_DDI_DP_VC_PAYLOAD_ALLOC;
@@ -2586,12 +2585,23 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
 			pipe_config->hdmi_high_tmds_clock_ratio = true;
 		/* fall through */
 	case TRANS_DDI_MODE_SELECT_DVI:
+		pipe_config->output_types |= BIT(INTEL_OUTPUT_HDMI);
 		pipe_config->lane_count = 4;
 		break;
 	case TRANS_DDI_MODE_SELECT_FDI:
+		pipe_config->output_types |= BIT(INTEL_OUTPUT_ANALOG);
 		break;
 	case TRANS_DDI_MODE_SELECT_DP_SST:
+		if (encoder->type == INTEL_OUTPUT_EDP)
+			pipe_config->output_types |= BIT(INTEL_OUTPUT_EDP);
+		else
+			pipe_config->output_types |= BIT(INTEL_OUTPUT_DP);
+		pipe_config->lane_count =
+			((temp & DDI_PORT_WIDTH_MASK) >> DDI_PORT_WIDTH_SHIFT) + 1;
+		intel_dp_get_m_n(intel_crtc, pipe_config);
+		break;
 	case TRANS_DDI_MODE_SELECT_DP_MST:
+		pipe_config->output_types |= BIT(INTEL_OUTPUT_DP_MST);
 		pipe_config->lane_count =
 			((temp & DDI_PORT_WIDTH_MASK) >> DDI_PORT_WIDTH_SHIFT) + 1;
 		intel_dp_get_m_n(intel_crtc, pipe_config);
@@ -2630,21 +2640,36 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
 			bxt_ddi_phy_get_lane_lat_optim_mask(encoder);
 }
 
+static enum intel_output_type
+intel_ddi_compute_output_type(struct intel_encoder *encoder,
+			      struct intel_crtc_state *crtc_state,
+			      struct drm_connector_state *conn_state)
+{
+	switch (conn_state->connector->connector_type) {
+	case DRM_MODE_CONNECTOR_HDMIA:
+		return INTEL_OUTPUT_HDMI;
+	case DRM_MODE_CONNECTOR_eDP:
+		return INTEL_OUTPUT_EDP;
+	case DRM_MODE_CONNECTOR_DisplayPort:
+		return INTEL_OUTPUT_DP;
+	default:
+		MISSING_CASE(conn_state->connector->connector_type);
+		return INTEL_OUTPUT_UNUSED;
+	}
+}
+
 static bool intel_ddi_compute_config(struct intel_encoder *encoder,
 				     struct intel_crtc_state *pipe_config,
 				     struct drm_connector_state *conn_state)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-	int type = encoder->type;
 	int port = intel_ddi_get_encoder_port(encoder);
 	int ret;
 
-	WARN(type == INTEL_OUTPUT_UNKNOWN, "compute_config() on unknown output!\n");
-
 	if (port == PORT_A)
 		pipe_config->cpu_transcoder = TRANSCODER_EDP;
 
-	if (type == INTEL_OUTPUT_HDMI)
+	if (intel_crtc_has_type(pipe_config, INTEL_OUTPUT_HDMI))
 		ret = intel_hdmi_compute_config(encoder, pipe_config, conn_state);
 	else
 		ret = intel_dp_compute_config(encoder, pipe_config, conn_state);
@@ -2764,6 +2789,7 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
 	drm_encoder_init(&dev_priv->drm, encoder, &intel_ddi_funcs,
 			 DRM_MODE_ENCODER_TMDS, "DDI %c", port_name(port));
 
+	intel_encoder->compute_output_type = intel_ddi_compute_output_type;
 	intel_encoder->compute_config = intel_ddi_compute_config;
 	intel_encoder->enable = intel_enable_ddi;
 	if (IS_GEN9_LP(dev_priv))
@@ -2821,9 +2847,10 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
 		}
 	}
 
+	intel_dig_port->dp.output_reg = INVALID_MMIO_REG;
 	intel_dig_port->max_lanes = max_lanes;
 
-	intel_encoder->type = INTEL_OUTPUT_UNKNOWN;
+	intel_encoder->type = INTEL_OUTPUT_DDI;
 	intel_encoder->power_domain = intel_port_to_power_domain(port);
 	intel_encoder->port = port;
 	intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index bd62c0a65bcd..a7e02ae33583 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10591,7 +10591,7 @@ static const char * const output_type_str[] = {
 	OUTPUT_TYPE(DP),
 	OUTPUT_TYPE(EDP),
 	OUTPUT_TYPE(DSI),
-	OUTPUT_TYPE(UNKNOWN),
+	OUTPUT_TYPE(DDI),
 	OUTPUT_TYPE(DP_MST),
 };
 
@@ -10762,7 +10762,7 @@ static bool check_digital_port_conflicts(struct drm_atomic_state *state)
 
 		switch (encoder->type) {
 			unsigned int port_mask;
-		case INTEL_OUTPUT_UNKNOWN:
+		case INTEL_OUTPUT_DDI:
 			if (WARN_ON(!HAS_DDI(to_i915(dev))))
 				break;
 		case INTEL_OUTPUT_DP:
@@ -10895,7 +10895,12 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
 		 * Determine output_types before calling the .compute_config()
 		 * hooks so that the hooks can use this information safely.
 		 */
-		pipe_config->output_types |= 1 << encoder->type;
+		if (encoder->compute_output_type)
+			pipe_config->output_types |=
+				BIT(encoder->compute_output_type(encoder, pipe_config,
+								 connector_state));
+		else
+			pipe_config->output_types |= BIT(encoder->type);
 	}
 
 encoder_retry:
@@ -11572,10 +11577,8 @@ verify_crtc_state(struct drm_crtc *crtc,
 				"Encoder connected to wrong pipe %c\n",
 				pipe_name(pipe));
 
-		if (active) {
-			pipe_config->output_types |= 1 << encoder->type;
+		if (active)
 			encoder->get_config(encoder, pipe_config);
-		}
 	}
 
 	intel_crtc_compute_pixel_rate(pipe_config);
@@ -14963,7 +14966,6 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
 			crtc_state = to_intel_crtc_state(crtc->base.state);
 
 			encoder->base.crtc = &crtc->base;
-			crtc_state->output_types |= 1 << encoder->type;
 			encoder->get_config(encoder, crtc_state);
 		} else {
 			encoder->base.crtc = NULL;
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index aa75f55eeb61..2b482d12000e 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -758,11 +758,16 @@ void intel_power_sequencer_reset(struct drm_i915_private *dev_priv)
 		struct intel_dp *intel_dp;
 
 		if (encoder->type != INTEL_OUTPUT_DP &&
-		    encoder->type != INTEL_OUTPUT_EDP)
+		    encoder->type != INTEL_OUTPUT_EDP &&
+		    encoder->type != INTEL_OUTPUT_DDI)
 			continue;
 
 		intel_dp = enc_to_intel_dp(&encoder->base);
 
+		/* Skip pure DVI/HDMI DDI encoders */
+		if (!i915_mmio_reg_valid(intel_dp->output_reg))
+			continue;
+
 		WARN_ON(intel_dp->active_pipe != INVALID_PIPE);
 
 		if (encoder->type != INTEL_OUTPUT_EDP)
@@ -2603,6 +2608,11 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
 	enum port port = dp_to_dig_port(intel_dp)->port;
 	struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
 
+	if (encoder->type == INTEL_OUTPUT_EDP)
+		pipe_config->output_types |= BIT(INTEL_OUTPUT_EDP);
+	else
+		pipe_config->output_types |= BIT(INTEL_OUTPUT_DP);
+
 	tmp = I915_READ(intel_dp->output_reg);
 
 	pipe_config->has_audio = tmp & DP_AUDIO_OUTPUT_ENABLE && port != PORT_A;
@@ -4699,8 +4709,6 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
 {
 	struct drm_connector *connector = &intel_connector->base;
 	struct intel_dp *intel_dp = intel_attached_dp(connector);
-	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
-	struct intel_encoder *intel_encoder = &intel_dig_port->base;
 	struct drm_device *dev = connector->dev;
 	enum drm_connector_status status;
 	u8 sink_irq_vector = 0;
@@ -4733,9 +4741,6 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
 		goto out;
 	}
 
-	if (intel_encoder->type != INTEL_OUTPUT_EDP)
-		intel_encoder->type = INTEL_OUTPUT_DP;
-
 	if (intel_dp->reset_link_params) {
 		/* Initial max link lane count */
 		intel_dp->max_link_lane_count = intel_dp_max_common_lane_count(intel_dp);
@@ -4852,9 +4857,6 @@ intel_dp_force(struct drm_connector *connector)
 	intel_dp_set_edid(intel_dp);
 
 	intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
-
-	if (intel_encoder->type != INTEL_OUTPUT_EDP)
-		intel_encoder->type = INTEL_OUTPUT_DP;
 }
 
 static int intel_dp_get_modes(struct drm_connector *connector)
@@ -5073,10 +5075,6 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	enum irqreturn ret = IRQ_NONE;
 
-	if (intel_dig_port->base.type != INTEL_OUTPUT_EDP &&
-	    intel_dig_port->base.type != INTEL_OUTPUT_HDMI)
-		intel_dig_port->base.type = INTEL_OUTPUT_DP;
-
 	if (long_hpd && intel_dig_port->base.type == INTEL_OUTPUT_EDP) {
 		/*
 		 * vdd off can generate a long pulse on eDP which
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index 772521440a9f..9be6f32bb100 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -270,6 +270,8 @@ static void intel_dp_mst_enc_get_config(struct intel_encoder *encoder,
 	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
 	u32 temp, flags = 0;
 
+	pipe_config->output_types |= BIT(INTEL_OUTPUT_DP_MST);
+
 	pipe_config->has_audio =
 		intel_ddi_is_audio_enabled(dev_priv, crtc);
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a05ab3a1ab27..650129980ce2 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -173,7 +173,7 @@ enum intel_output_type {
 	INTEL_OUTPUT_DP = 7,
 	INTEL_OUTPUT_EDP = 8,
 	INTEL_OUTPUT_DSI = 9,
-	INTEL_OUTPUT_UNKNOWN = 10,
+	INTEL_OUTPUT_DDI = 10,
 	INTEL_OUTPUT_DP_MST = 11,
 };
 
@@ -216,6 +216,9 @@ struct intel_encoder {
 	enum port port;
 	unsigned int cloneable;
 	void (*hot_plug)(struct intel_encoder *);
+	enum intel_output_type (*compute_output_type)(struct intel_encoder *,
+						      struct intel_crtc_state *,
+						      struct drm_connector_state *);
 	bool (*compute_config)(struct intel_encoder *,
 			       struct intel_crtc_state *,
 			       struct drm_connector_state *);
@@ -1149,7 +1152,7 @@ enc_to_dig_port(struct drm_encoder *encoder)
 	struct intel_encoder *intel_encoder = to_intel_encoder(encoder);
 
 	switch (intel_encoder->type) {
-	case INTEL_OUTPUT_UNKNOWN:
+	case INTEL_OUTPUT_DDI:
 		WARN_ON(!HAS_DDI(to_i915(encoder->dev)));
 	case INTEL_OUTPUT_DP:
 	case INTEL_OUTPUT_EDP:
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 66bbedc5fa01..2ae3eea19317 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -1243,6 +1243,8 @@ static void intel_dsi_get_config(struct intel_encoder *encoder,
 	u32 pclk;
 	DRM_DEBUG_KMS("\n");
 
+	pipe_config->output_types |= BIT(INTEL_OUTPUT_DSI);
+
 	if (IS_GEN9_LP(dev_priv))
 		bxt_dsi_get_pipe_config(encoder, pipe_config);
 
diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
index 53c9b763f4ce..754baa00bea9 100644
--- a/drivers/gpu/drm/i915/intel_dvo.c
+++ b/drivers/gpu/drm/i915/intel_dvo.c
@@ -159,6 +159,8 @@ static void intel_dvo_get_config(struct intel_encoder *encoder,
 	struct intel_dvo *intel_dvo = enc_to_dvo(encoder);
 	u32 tmp, flags = 0;
 
+	pipe_config->output_types |= BIT(INTEL_OUTPUT_DVO);
+
 	tmp = I915_READ(intel_dvo->dev.dvo_reg);
 	if (tmp & DVO_HSYNC_ACTIVE_HIGH)
 		flags |= DRM_MODE_FLAG_PHSYNC;
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 5132dc814788..1ff448a67b99 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -957,6 +957,8 @@ static void intel_hdmi_get_config(struct intel_encoder *encoder,
 	u32 tmp, flags = 0;
 	int dotclock;
 
+	pipe_config->output_types |= BIT(INTEL_OUTPUT_HDMI);
+
 	tmp = I915_READ(intel_hdmi->hdmi_reg);
 
 	if (tmp & SDVO_HSYNC_ACTIVE_HIGH)
@@ -1610,12 +1612,9 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
 
 	intel_hdmi_unset_edid(connector);
 
-	if (intel_hdmi_set_edid(connector)) {
-		struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
-
-		hdmi_to_dig_port(intel_hdmi)->base.type = INTEL_OUTPUT_HDMI;
+	if (intel_hdmi_set_edid(connector))
 		status = connector_status_connected;
-	} else
+	else
 		status = connector_status_disconnected;
 
 	intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS);
@@ -1626,8 +1625,6 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
 static void
 intel_hdmi_force(struct drm_connector *connector)
 {
-	struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
-
 	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
 		      connector->base.id, connector->name);
 
@@ -1637,7 +1634,6 @@ intel_hdmi_force(struct drm_connector *connector)
 		return;
 
 	intel_hdmi_set_edid(connector);
-	hdmi_to_dig_port(intel_hdmi)->base.type = INTEL_OUTPUT_HDMI;
 }
 
 static int intel_hdmi_get_modes(struct drm_connector *connector)
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 38572d65e46e..ef80499113ee 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -125,6 +125,8 @@ static void intel_lvds_get_config(struct intel_encoder *encoder,
 	struct intel_lvds_encoder *lvds_encoder = to_lvds_encoder(&encoder->base);
 	u32 tmp, flags = 0;
 
+	pipe_config->output_types |= BIT(INTEL_OUTPUT_LVDS);
+
 	tmp = I915_READ(lvds_encoder->reg);
 	if (tmp & LVDS_HSYNC_POLARITY)
 		flags |= DRM_MODE_FLAG_NHSYNC;
diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
index 1d946240e55f..39714be3eb6b 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -383,7 +383,7 @@ int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder,
 	case INTEL_OUTPUT_ANALOG:
 		type = DISPLAY_TYPE_CRT;
 		break;
-	case INTEL_OUTPUT_UNKNOWN:
+	case INTEL_OUTPUT_DDI:
 	case INTEL_OUTPUT_DP:
 	case INTEL_OUTPUT_HDMI:
 	case INTEL_OUTPUT_DP_MST:
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 7437944b388f..42ec2d1f7a61 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -1429,6 +1429,8 @@ static void intel_sdvo_get_config(struct intel_encoder *encoder,
 	u8 val;
 	bool ret;
 
+	pipe_config->output_types |= BIT(INTEL_OUTPUT_SDVO);
+
 	sdvox = I915_READ(intel_sdvo->sdvo_reg);
 
 	ret = intel_sdvo_get_input_timing(intel_sdvo, &dtd);
diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index a79a7591b2cf..b18609cebe03 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -868,6 +868,8 @@ static void
 intel_tv_get_config(struct intel_encoder *encoder,
 		    struct intel_crtc_state *pipe_config)
 {
+	pipe_config->output_types |= BIT(INTEL_OUTPUT_TVOUT);
+
 	pipe_config->base.adjusted_mode.crtc_clock = pipe_config->port_clock;
 }
 
-- 
2.13.6

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

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

* [PATCH 06/10] drm/i915: Nuke intel_ddi_get_encoder_port()
  2017-10-19 13:37 [PATCH 00/10] drm/i915: Last part of DDI encoder->type cleanup Ville Syrjala
                   ` (4 preceding siblings ...)
  2017-10-19 13:37 ` [PATCH v5 05/10] drm/i915: Stop frobbing with DDI encoder->type Ville Syrjala
@ 2017-10-19 13:37 ` Ville Syrjala
  2017-10-19 13:37 ` [PATCH 07/10] drm/i915: Eliminate pll->state usage from bxt_calc_pll_link() Ville Syrjala
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 27+ messages in thread
From: Ville Syrjala @ 2017-10-19 13:37 UTC (permalink / raw)
  To: intel-gfx

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

encoder->port works for FDI, and it also works for MST (regardless of
whether we're dealing with the "fake" MST encoder, or mst->primary).
So let's eliminate intel_ddi_get_encoder_port().

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c      | 50 ++++++++++++-----------------------
 drivers/gpu/drm/i915/intel_drv.h      |  1 -
 drivers/gpu/drm/i915/intel_opregion.c |  2 +-
 3 files changed, 18 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 630609575db4..1747dcb5272e 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -492,22 +492,6 @@ static const struct cnl_ddi_buf_trans cnl_ddi_translations_edp_1_05V[] = {
 	{ 0x2, 0x7F, 0x3F, 0x00, 0x00 },	/* 400   400      0.0   */
 };
 
-enum port intel_ddi_get_encoder_port(struct intel_encoder *encoder)
-{
-	switch (encoder->type) {
-	case INTEL_OUTPUT_DP_MST:
-		return enc_to_mst(&encoder->base)->primary->port;
-	case INTEL_OUTPUT_EDP:
-	case INTEL_OUTPUT_DDI:
-		return enc_to_dig_port(&encoder->base)->port;
-	case INTEL_OUTPUT_ANALOG:
-		return PORT_E;
-	default:
-		MISSING_CASE(encoder->type);
-		return PORT_A;
-	}
-}
-
 static const struct ddi_buf_trans *
 bdw_get_buf_trans_edp(struct drm_i915_private *dev_priv, int *n_entries)
 {
@@ -815,7 +799,7 @@ static void intel_prepare_dp_ddi_buffers(struct intel_encoder *encoder,
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	u32 iboost_bit = 0;
 	int i, n_entries;
-	enum port port = intel_ddi_get_encoder_port(encoder);
+	enum port port = encoder->port;
 	const struct ddi_buf_trans *ddi_translations;
 
 	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_ANALOG))
@@ -852,7 +836,7 @@ static void intel_prepare_hdmi_ddi_buffers(struct intel_encoder *encoder,
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	u32 iboost_bit = 0;
 	int n_entries;
-	enum port port = intel_ddi_get_encoder_port(encoder);
+	enum port port = encoder->port;
 	const struct ddi_buf_trans *ddi_translations;
 
 	ddi_translations = intel_ddi_get_buf_trans_hdmi(dev_priv, &n_entries);
@@ -1466,7 +1450,7 @@ static void bxt_ddi_clock_get(struct intel_encoder *encoder,
 				struct intel_crtc_state *pipe_config)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-	enum port port = intel_ddi_get_encoder_port(encoder);
+	enum port port = encoder->port;
 	uint32_t dpll = port;
 
 	pipe_config->port_clock = bxt_calc_pll_link(dev_priv, dpll);
@@ -1546,7 +1530,7 @@ void intel_ddi_enable_transcoder_func(const struct intel_crtc_state *crtc_state)
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	enum pipe pipe = crtc->pipe;
 	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
-	enum port port = intel_ddi_get_encoder_port(encoder);
+	enum port port = encoder->port;
 	uint32_t temp;
 
 	/* Enable TRANS_DDI_FUNC_CTL for the pipe to work in HDMI mode */
@@ -1642,7 +1626,7 @@ bool intel_ddi_connector_get_hw_state(struct intel_connector *intel_connector)
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_encoder *encoder = intel_connector->encoder;
 	int type = intel_connector->base.connector_type;
-	enum port port = intel_ddi_get_encoder_port(encoder);
+	enum port port = encoder->port;
 	enum pipe pipe = 0;
 	enum transcoder cpu_transcoder;
 	uint32_t tmp;
@@ -1701,7 +1685,7 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
 {
 	struct drm_device *dev = encoder->base.dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
-	enum port port = intel_ddi_get_encoder_port(encoder);
+	enum port port = encoder->port;
 	u32 tmp;
 	int i;
 	bool ret;
@@ -1786,7 +1770,7 @@ void intel_ddi_enable_pipe_clock(const struct intel_crtc_state *crtc_state)
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	struct intel_encoder *encoder = intel_ddi_get_crtc_encoder(crtc);
-	enum port port = intel_ddi_get_encoder_port(encoder);
+	enum port port = encoder->port;
 	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
 
 	if (cpu_transcoder != TRANSCODER_EDP)
@@ -1925,8 +1909,8 @@ static void cnl_ddi_vswing_program(struct intel_encoder *encoder,
 				   int level, enum intel_output_type type)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-	enum port port = intel_ddi_get_encoder_port(encoder);
 	const struct cnl_ddi_buf_trans *ddi_translations;
+	enum port port = encoder->port;
 	int n_entries, ln;
 	u32 val;
 
@@ -1989,7 +1973,7 @@ static void cnl_ddi_vswing_sequence(struct intel_encoder *encoder,
 				    int level, enum intel_output_type type)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-	enum port port = intel_ddi_get_encoder_port(encoder);
+	enum port port = encoder->port;
 	int width, rate, ln;
 	u32 val;
 
@@ -2108,7 +2092,7 @@ static void intel_ddi_clk_select(struct intel_encoder *encoder,
 				 const 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);
+	enum port port = encoder->port;
 	uint32_t val;
 
 	if (WARN_ON(!pll))
@@ -2147,7 +2131,7 @@ static void intel_ddi_clk_select(struct intel_encoder *encoder,
 static void intel_ddi_clk_disable(struct intel_encoder *encoder)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-	enum port port = intel_ddi_get_encoder_port(encoder);
+	enum port port = encoder->port;
 
 	if (IS_CANNONLAKE(dev_priv))
 		I915_WRITE(DPCLKA_CFGCR0, I915_READ(DPCLKA_CFGCR0) |
@@ -2165,7 +2149,7 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
 {
 	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);
+	enum port port = encoder->port;
 	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
 	bool is_mst = intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST);
 	int level = intel_ddi_dp_level(intel_dp);
@@ -2203,7 +2187,7 @@ static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
 	struct intel_digital_port *intel_dig_port = enc_to_dig_port(&encoder->base);
 	struct intel_hdmi *intel_hdmi = &intel_dig_port->hdmi;
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-	enum port port = intel_ddi_get_encoder_port(encoder);
+	enum port port = encoder->port;
 	int level = intel_ddi_hdmi_level(dev_priv, port);
 	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
 
@@ -2248,7 +2232,7 @@ static void intel_ddi_pre_enable(struct intel_encoder *encoder,
 static void intel_disable_ddi_buf(struct intel_encoder *encoder)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-	enum port port = intel_ddi_get_encoder_port(encoder);
+	enum port port = encoder->port;
 	bool wait = false;
 	u32 val;
 
@@ -2377,7 +2361,7 @@ static void intel_enable_ddi_dp(struct intel_encoder *encoder,
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
-	enum port port = intel_ddi_get_encoder_port(encoder);
+	enum port port = encoder->port;
 
 	if (port == PORT_A && INTEL_GEN(dev_priv) < 9)
 		intel_dp_stop_link_train(intel_dp);
@@ -2396,7 +2380,7 @@ static void intel_enable_ddi_hdmi(struct intel_encoder *encoder,
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
-	enum port port = intel_ddi_get_encoder_port(encoder);
+	enum port port = encoder->port;
 
 	intel_hdmi_handle_sink_scrambling(encoder,
 					  conn_state->connector,
@@ -2663,7 +2647,7 @@ static bool intel_ddi_compute_config(struct intel_encoder *encoder,
 				     struct drm_connector_state *conn_state)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-	int port = intel_ddi_get_encoder_port(encoder);
+	enum port port = encoder->port;
 	int ret;
 
 	if (port == PORT_A)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 650129980ce2..492e2bf720c5 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1276,7 +1276,6 @@ void intel_ddi_fdi_post_disable(struct intel_encoder *intel_encoder,
 void hsw_fdi_link_train(struct intel_crtc *crtc,
 			const struct intel_crtc_state *crtc_state);
 void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port);
-enum port intel_ddi_get_encoder_port(struct intel_encoder *intel_encoder);
 bool intel_ddi_get_hw_state(struct intel_encoder *encoder, enum pipe *pipe);
 void intel_ddi_enable_transcoder_func(const struct intel_crtc_state *crtc_state);
 void intel_ddi_disable_transcoder_func(struct drm_i915_private *dev_priv,
diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
index 39714be3eb6b..fc65f5e451dd 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -367,7 +367,7 @@ int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder,
 	if (intel_encoder->type == INTEL_OUTPUT_DSI)
 		port = 0;
 	else
-		port = intel_ddi_get_encoder_port(intel_encoder);
+		port = intel_encoder->port;
 
 	if (port == PORT_E)  {
 		port = 0;
-- 
2.13.6

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

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

* [PATCH 07/10] drm/i915: Eliminate pll->state usage from bxt_calc_pll_link()
  2017-10-19 13:37 [PATCH 00/10] drm/i915: Last part of DDI encoder->type cleanup Ville Syrjala
                   ` (5 preceding siblings ...)
  2017-10-19 13:37 ` [PATCH 06/10] drm/i915: Nuke intel_ddi_get_encoder_port() Ville Syrjala
@ 2017-10-19 13:37 ` Ville Syrjala
  2017-10-19 13:37 ` [PATCH 08/10] drm/i915: Pass a crtc state to ddi post_disable from MST code Ville Syrjala
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 27+ messages in thread
From: Ville Syrjala @ 2017-10-19 13:37 UTC (permalink / raw)
  To: intel-gfx

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

We should be using the DPLL hw state we got from the current crtc state
to determine the corresponding port clock frequency rather than getting
it via the current state programmed into the DPLL.

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

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 1747dcb5272e..a8ae95e4c6f2 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1421,19 +1421,16 @@ static void hsw_ddi_clock_get(struct intel_encoder *encoder,
 	ddi_dotclock_get(pipe_config);
 }
 
-static int bxt_calc_pll_link(struct drm_i915_private *dev_priv,
-				enum intel_dpll_id dpll)
+static int bxt_calc_pll_link(struct intel_crtc_state *crtc_state)
 {
-	struct intel_shared_dpll *pll;
 	struct intel_dpll_hw_state *state;
 	struct dpll clock;
 
 	/* For DDI ports we always use a shared PLL. */
-	if (WARN_ON(dpll == DPLL_ID_PRIVATE))
+	if (WARN_ON(!crtc_state->shared_dpll))
 		return 0;
 
-	pll = &dev_priv->shared_dplls[dpll];
-	state = &pll->state.hw_state;
+	state = &crtc_state->dpll_hw_state;
 
 	clock.m1 = 2;
 	clock.m2 = (state->pll0 & PORT_PLL_M2_MASK) << 22;
@@ -1447,13 +1444,9 @@ static int bxt_calc_pll_link(struct drm_i915_private *dev_priv,
 }
 
 static void bxt_ddi_clock_get(struct intel_encoder *encoder,
-				struct intel_crtc_state *pipe_config)
+			      struct intel_crtc_state *pipe_config)
 {
-	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-	enum port port = encoder->port;
-	uint32_t dpll = port;
-
-	pipe_config->port_clock = bxt_calc_pll_link(dev_priv, dpll);
+	pipe_config->port_clock = bxt_calc_pll_link(pipe_config);
 
 	ddi_dotclock_get(pipe_config);
 }
-- 
2.13.6

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

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

* [PATCH 08/10] drm/i915: Pass a crtc state to ddi post_disable from MST code
  2017-10-19 13:37 [PATCH 00/10] drm/i915: Last part of DDI encoder->type cleanup Ville Syrjala
                   ` (6 preceding siblings ...)
  2017-10-19 13:37 ` [PATCH 07/10] drm/i915: Eliminate pll->state usage from bxt_calc_pll_link() Ville Syrjala
@ 2017-10-19 13:37 ` Ville Syrjala
  2017-10-27 11:39   ` Maarten Lankhorst
  2017-10-19 13:37 ` [PATCH 09/10] drm/i915: Use intel_ddi_get_config() for MST Ville Syrjala
                   ` (4 subsequent siblings)
  12 siblings, 1 reply; 27+ messages in thread
From: Ville Syrjala @ 2017-10-19 13:37 UTC (permalink / raw)
  To: intel-gfx

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

Pass an old crtc state to intel_ddi_post_disable() from the MST code.

Note that this crtc state won't necessaitly match the one that was
passed to intel_ddi_pre_enable() if the first stream to be enabled isn't
the last stream to be disabled. But this is fine since the states should
be identical in every important way. This does mean people frobbing
the DDI pre_enable/post_disable hooks have to pay attention in what
parts of the state they consult.

The alternative would be to inline the relevant code into the MST code.
That is actually what we used to do for pre_enable before
commit e081c8463ac9 ("drm/i915: Remove duplicate DDI enabling logic
from MST path"). For post_disable we've always called the DDI hook.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c    | 24 +++++++++++-------------
 drivers/gpu/drm/i915/intel_dp_mst.c |  6 +++---
 2 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index a8ae95e4c6f2..0123c4753cd4 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2210,11 +2210,15 @@ static void intel_ddi_pre_enable(struct intel_encoder *encoder,
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-	enum pipe pipe = crtc->pipe;
+
+	/*
+	 * conn_state is NULL when called from DP_MST. The main connector
+	 * associated with this port is never bound to a crtc for MST.
+	 */
 
 	WARN_ON(crtc_state->has_pch_encoder);
 
-	intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
+	intel_set_cpu_fifo_underrun_reporting(dev_priv, crtc->pipe, true);
 
 	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
 		intel_ddi_pre_enable_hdmi(encoder, crtc_state, conn_state);
@@ -2252,12 +2256,7 @@ static void intel_ddi_post_disable_dp(struct intel_encoder *encoder,
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
 	struct intel_dp *intel_dp = &dig_port->dp;
-	/*
-	 * old_crtc_state and old_conn_state are NULL when called from
-	 * DP_MST. The main connector associated with this port is never
-	 * bound to a crtc for MST.
-	 */
-	bool is_mst = !old_crtc_state;
+	bool is_mst = intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_DP_MST);
 
 	/*
 	 * Power down sink before disabling the port, otherwise we end
@@ -2301,12 +2300,11 @@ static void intel_ddi_post_disable(struct intel_encoder *encoder,
 				   const struct drm_connector_state *old_conn_state)
 {
 	/*
-	 * old_crtc_state and old_conn_state are NULL when called from
-	 * DP_MST. The main connector associated with this port is never
-	 * bound to a crtc for MST.
+	 * old_conn_state is NULL when called from DP_MST. The main connector
+	 * associated with this port is never bound to a crtc for MST.
 	 */
-	if (old_crtc_state &&
-	    intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_HDMI))
+
+	if (intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_HDMI))
 		intel_ddi_post_disable_hdmi(encoder,
 					    old_crtc_state, old_conn_state);
 	else
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index 9be6f32bb100..cdffe7e93a71 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -172,10 +172,10 @@ static void intel_mst_post_disable_dp(struct intel_encoder *encoder,
 	intel_dp->active_mst_links--;
 
 	intel_mst->connector = NULL;
-	if (intel_dp->active_mst_links == 0) {
+	if (intel_dp->active_mst_links == 0)
 		intel_dig_port->base.post_disable(&intel_dig_port->base,
-						  NULL, NULL);
-	}
+						  old_crtc_state, NULL);
+
 	DRM_DEBUG_KMS("active links %d\n", intel_dp->active_mst_links);
 }
 
-- 
2.13.6

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

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

* [PATCH 09/10] drm/i915: Use intel_ddi_get_config() for MST
  2017-10-19 13:37 [PATCH 00/10] drm/i915: Last part of DDI encoder->type cleanup Ville Syrjala
                   ` (7 preceding siblings ...)
  2017-10-19 13:37 ` [PATCH 08/10] drm/i915: Pass a crtc state to ddi post_disable from MST code Ville Syrjala
@ 2017-10-19 13:37 ` Ville Syrjala
  2017-10-27 11:43   ` Maarten Lankhorst
  2017-10-19 13:37 ` [PATCH 10/10] drm/i915: Use intel_crtc_has_dp_encoder() for LPE audio Ville Syrjala
                   ` (3 subsequent siblings)
  12 siblings, 1 reply; 27+ messages in thread
From: Ville Syrjala @ 2017-10-19 13:37 UTC (permalink / raw)
  To: intel-gfx

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

Eliminate the partially duplicated DDI readout code from MST, and
instead just call intel_ddi_get_config(). As a nice bonus we get
more cross checking as intel_ddi_get_config() will populate
output_types based on the actual mode of the DDI port.

Additonally intel_ddi_get_config() must be changed to get the crtc
from the passed in crtc state rather than from the encoder->crtc link.
encoder->crtc really shouldn't be used anyway.

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

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 0123c4753cd4..e96b458d6592 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2507,7 +2507,7 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
 			  struct intel_crtc_state *pipe_config)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
+	struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->base.crtc);
 	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
 	struct intel_digital_port *intel_dig_port;
 	u32 temp, flags = 0;
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index cdffe7e93a71..e9584294dbbe 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -265,50 +265,8 @@ static void intel_dp_mst_enc_get_config(struct intel_encoder *encoder,
 {
 	struct intel_dp_mst_encoder *intel_mst = enc_to_mst(&encoder->base);
 	struct intel_digital_port *intel_dig_port = intel_mst->primary;
-	struct intel_crtc *crtc = to_intel_crtc(pipe_config->base.crtc);
-	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
-	u32 temp, flags = 0;
 
-	pipe_config->output_types |= BIT(INTEL_OUTPUT_DP_MST);
-
-	pipe_config->has_audio =
-		intel_ddi_is_audio_enabled(dev_priv, crtc);
-
-	temp = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
-	if (temp & TRANS_DDI_PHSYNC)
-		flags |= DRM_MODE_FLAG_PHSYNC;
-	else
-		flags |= DRM_MODE_FLAG_NHSYNC;
-	if (temp & TRANS_DDI_PVSYNC)
-		flags |= DRM_MODE_FLAG_PVSYNC;
-	else
-		flags |= DRM_MODE_FLAG_NVSYNC;
-
-	switch (temp & TRANS_DDI_BPC_MASK) {
-	case TRANS_DDI_BPC_6:
-		pipe_config->pipe_bpp = 18;
-		break;
-	case TRANS_DDI_BPC_8:
-		pipe_config->pipe_bpp = 24;
-		break;
-	case TRANS_DDI_BPC_10:
-		pipe_config->pipe_bpp = 30;
-		break;
-	case TRANS_DDI_BPC_12:
-		pipe_config->pipe_bpp = 36;
-		break;
-	default:
-		break;
-	}
-	pipe_config->base.adjusted_mode.flags |= flags;
-
-	pipe_config->lane_count =
-		((temp & DDI_PORT_WIDTH_MASK) >> DDI_PORT_WIDTH_SHIFT) + 1;
-
-	intel_dp_get_m_n(crtc, pipe_config);
-
-	intel_ddi_clock_get(&intel_dig_port->base, pipe_config);
+	intel_ddi_get_config(&intel_dig_port->base, pipe_config);
 }
 
 static int intel_dp_mst_get_ddc_modes(struct drm_connector *connector)
-- 
2.13.6

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

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

* [PATCH 10/10] drm/i915: Use intel_crtc_has_dp_encoder() for LPE audio
  2017-10-19 13:37 [PATCH 00/10] drm/i915: Last part of DDI encoder->type cleanup Ville Syrjala
                   ` (8 preceding siblings ...)
  2017-10-19 13:37 ` [PATCH 09/10] drm/i915: Use intel_ddi_get_config() for MST Ville Syrjala
@ 2017-10-19 13:37 ` Ville Syrjala
  2017-10-27 11:43   ` Maarten Lankhorst
  2017-10-19 14:00 ` ✓ Fi.CI.BAT: success for drm/i915: Last part of DDI encoder->type cleanup Patchwork
                   ` (2 subsequent siblings)
  12 siblings, 1 reply; 27+ messages in thread
From: Ville Syrjala @ 2017-10-19 13:37 UTC (permalink / raw)
  To: intel-gfx

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

Reduce our general reliance on encoder->type and instead use
output_types from the crtc state when enabling LPE audio.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_audio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index 0ddba16fde1b..1e59efbfe8a3 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -629,7 +629,7 @@ void intel_audio_codec_enable(struct intel_encoder *intel_encoder,
 
 	intel_lpe_audio_notify(dev_priv, pipe, port, connector->eld,
 			       crtc_state->port_clock,
-			       intel_encoder->type == INTEL_OUTPUT_DP);
+			       intel_crtc_has_dp_encoder(crtc_state));
 }
 
 /**
-- 
2.13.6

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

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

* ✓ Fi.CI.BAT: success for drm/i915: Last part of DDI encoder->type cleanup
  2017-10-19 13:37 [PATCH 00/10] drm/i915: Last part of DDI encoder->type cleanup Ville Syrjala
                   ` (9 preceding siblings ...)
  2017-10-19 13:37 ` [PATCH 10/10] drm/i915: Use intel_crtc_has_dp_encoder() for LPE audio Ville Syrjala
@ 2017-10-19 14:00 ` Patchwork
  2017-10-19 14:59 ` ✓ Fi.CI.IGT: " Patchwork
  2017-10-27 16:39 ` ✗ Fi.CI.BAT: failure " Patchwork
  12 siblings, 0 replies; 27+ messages in thread
From: Patchwork @ 2017-10-19 14:00 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Last part of DDI encoder->type cleanup
URL   : https://patchwork.freedesktop.org/series/32298/
State : success

== Summary ==

Series 32298v1 drm/i915: Last part of DDI encoder->type cleanup
https://patchwork.freedesktop.org/api/1.0/series/32298/revisions/1/mbox/

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:443s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:449s
fi-blb-e6850     total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:372s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:530s
fi-bwr-2160      total:289  pass:183  dwarn:0   dfail:0   fail:0   skip:106 time:264s
fi-bxt-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:501s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:497s
fi-byt-j1900     total:289  pass:253  dwarn:1   dfail:0   fail:0   skip:35  time:496s
fi-byt-n2820     total:289  pass:249  dwarn:1   dfail:0   fail:0   skip:39  time:482s
fi-cfl-s         total:289  pass:253  dwarn:4   dfail:0   fail:0   skip:32  time:557s
fi-elk-e7500     total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:419s
fi-gdg-551       total:289  pass:178  dwarn:1   dfail:0   fail:1   skip:109 time:250s
fi-glk-1         total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:583s
fi-hsw-4770      total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:451s
fi-hsw-4770r     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:428s
fi-ilk-650       total:289  pass:228  dwarn:0   dfail:0   fail:0   skip:61  time:441s
fi-ivb-3520m     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:495s
fi-ivb-3770      total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:470s
fi-kbl-7500u     total:289  pass:264  dwarn:1   dfail:0   fail:0   skip:24  time:490s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:574s
fi-kbl-7567u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:477s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:587s
fi-pnv-d510      total:289  pass:222  dwarn:1   dfail:0   fail:0   skip:66  time:549s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:453s
fi-skl-6700hq    total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:655s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:522s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:500s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:460s
fi-snb-2520m     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:567s
fi-snb-2600      total:289  pass:249  dwarn:0   dfail:0   fail:0   skip:40  time:427s

93f001963c9915c52b5ad84500075d231e008ced drm-tip: 2017y-10m-19d-10h-52m-17s UTC integration manifest
9ef83e01207e drm/i915: Don't use encoder->type in intel_ddi_set_pipe_settings()

== Logs ==

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

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

* ✓ Fi.CI.IGT: success for drm/i915: Last part of DDI encoder->type cleanup
  2017-10-19 13:37 [PATCH 00/10] drm/i915: Last part of DDI encoder->type cleanup Ville Syrjala
                   ` (10 preceding siblings ...)
  2017-10-19 14:00 ` ✓ Fi.CI.BAT: success for drm/i915: Last part of DDI encoder->type cleanup Patchwork
@ 2017-10-19 14:59 ` Patchwork
  2017-10-27 16:39 ` ✗ Fi.CI.BAT: failure " Patchwork
  12 siblings, 0 replies; 27+ messages in thread
From: Patchwork @ 2017-10-19 14:59 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Last part of DDI encoder->type cleanup
URL   : https://patchwork.freedesktop.org/series/32298/
State : success

== Summary ==

Test drv_module_reload:
        Subgroup basic-reload-inject:
                pass       -> DMESG-WARN (shard-hsw) fdo#102707
Test kms_setmode:
        Subgroup basic:
                fail       -> PASS       (shard-hsw) fdo#99912
Test kms_busy:
        Subgroup extended-modeset-hang-oldfb-with-reset-render-B:
                pass       -> DMESG-WARN (shard-hsw) fdo#102249 +1

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

shard-hsw        total:2540 pass:1428 dwarn:3   dfail:0   fail:8   skip:1101 time:9199s

== Logs ==

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

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

* Re: [PATCH 08/10] drm/i915: Pass a crtc state to ddi post_disable from MST code
  2017-10-19 13:37 ` [PATCH 08/10] drm/i915: Pass a crtc state to ddi post_disable from MST code Ville Syrjala
@ 2017-10-27 11:39   ` Maarten Lankhorst
  2017-10-27 11:49     ` Ville Syrjälä
  0 siblings, 1 reply; 27+ messages in thread
From: Maarten Lankhorst @ 2017-10-27 11:39 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx

Op 19-10-17 om 15:37 schreef Ville Syrjala:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Pass an old crtc state to intel_ddi_post_disable() from the MST code.
>
> Note that this crtc state won't necessaitly match the one that was
> passed to intel_ddi_pre_enable() if the first stream to be enabled isn't
> the last stream to be disabled. But this is fine since the states should
> be identical in every important way. This does mean people frobbing
> the DDI pre_enable/post_disable hooks have to pay attention in what
> parts of the state they consult.
>
> The alternative would be to inline the relevant code into the MST code.
> That is actually what we used to do for pre_enable before
> commit e081c8463ac9 ("drm/i915: Remove duplicate DDI enabling logic
> from MST path"). For post_disable we've always called the DDI hook.

I woudl rather have it NULL throughout the entire disable sequence, it's going to be
dangerous anyway, and something dangerous shouldn't be easy. The fact that you have
to think

Also there's an unrelated hunk in intel_ddi_pre_enable?

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

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

* Re: [PATCH 09/10] drm/i915: Use intel_ddi_get_config() for MST
  2017-10-19 13:37 ` [PATCH 09/10] drm/i915: Use intel_ddi_get_config() for MST Ville Syrjala
@ 2017-10-27 11:43   ` Maarten Lankhorst
  2017-10-27 11:55     ` Ville Syrjälä
  0 siblings, 1 reply; 27+ messages in thread
From: Maarten Lankhorst @ 2017-10-27 11:43 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx

Op 19-10-17 om 15:37 schreef Ville Syrjala:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Eliminate the partially duplicated DDI readout code from MST, and
> instead just call intel_ddi_get_config(). As a nice bonus we get
> more cross checking as intel_ddi_get_config() will populate
> output_types based on the actual mode of the DDI port.
>
> Additonally intel_ddi_get_config() must be changed to get the crtc
> from the passed in crtc state rather than from the encoder->crtc link.
> encoder->crtc really shouldn't be used anyway.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
I looked and noticed 2 small differences, one is the GEN9_LP part, the other is
min_voltage_level. Is this also a bugfix because of this?

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

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

* Re: [PATCH 10/10] drm/i915: Use intel_crtc_has_dp_encoder() for LPE audio
  2017-10-19 13:37 ` [PATCH 10/10] drm/i915: Use intel_crtc_has_dp_encoder() for LPE audio Ville Syrjala
@ 2017-10-27 11:43   ` Maarten Lankhorst
  2017-10-27 11:53     ` Ville Syrjälä
  0 siblings, 1 reply; 27+ messages in thread
From: Maarten Lankhorst @ 2017-10-27 11:43 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx

Op 19-10-17 om 15:37 schreef Ville Syrjala:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Reduce our general reliance on encoder->type and instead use
> output_types from the crtc state when enabling LPE audio.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_audio.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> index 0ddba16fde1b..1e59efbfe8a3 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -629,7 +629,7 @@ void intel_audio_codec_enable(struct intel_encoder *intel_encoder,
>  
>  	intel_lpe_audio_notify(dev_priv, pipe, port, connector->eld,
>  			       crtc_state->port_clock,
> -			       intel_encoder->type == INTEL_OUTPUT_DP);
> +			       intel_crtc_has_dp_encoder(crtc_state));
>  }
>  
>  /**

Should this patch be moved to before 05/10?

Looks sane otherwise.

~Maarten

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

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

* Re: [PATCH 08/10] drm/i915: Pass a crtc state to ddi post_disable from MST code
  2017-10-27 11:39   ` Maarten Lankhorst
@ 2017-10-27 11:49     ` Ville Syrjälä
  2017-10-27 14:31       ` Maarten Lankhorst
  0 siblings, 1 reply; 27+ messages in thread
From: Ville Syrjälä @ 2017-10-27 11:49 UTC (permalink / raw)
  To: Maarten Lankhorst; +Cc: intel-gfx

On Fri, Oct 27, 2017 at 01:39:00PM +0200, Maarten Lankhorst wrote:
> Op 19-10-17 om 15:37 schreef Ville Syrjala:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Pass an old crtc state to intel_ddi_post_disable() from the MST code.
> >
> > Note that this crtc state won't necessaitly match the one that was
> > passed to intel_ddi_pre_enable() if the first stream to be enabled isn't
> > the last stream to be disabled. But this is fine since the states should
> > be identical in every important way. This does mean people frobbing
> > the DDI pre_enable/post_disable hooks have to pay attention in what
> > parts of the state they consult.
> >
> > The alternative would be to inline the relevant code into the MST code.
> > That is actually what we used to do for pre_enable before
> > commit e081c8463ac9 ("drm/i915: Remove duplicate DDI enabling logic
> > from MST path"). For post_disable we've always called the DDI hook.
> 
> I woudl rather have it NULL throughout the entire disable sequence, it's going to be
> dangerous anyway, and something dangerous shouldn't be easy. The fact that you have
> to think

It makes for really ugly code as the enable and disable don't look at
all like each other when one has the NULL and the other doesn't. If it's
dangerous for disable, then it's equally dangerous for enable. So I'd
just go with whatever makes the code suck the least, which IMO is
passing a crtc state to both.

Probably the proper fix would be some kind of encoder state which would
be assigned to mst->primary. But that's going to require actual design
work.

> 
> Also there's an unrelated hunk in intel_ddi_pre_enable?

Apparently I wanted to get rid of the local pipe variable.

-- 
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] 27+ messages in thread

* Re: [PATCH 10/10] drm/i915: Use intel_crtc_has_dp_encoder() for LPE audio
  2017-10-27 11:43   ` Maarten Lankhorst
@ 2017-10-27 11:53     ` Ville Syrjälä
  0 siblings, 0 replies; 27+ messages in thread
From: Ville Syrjälä @ 2017-10-27 11:53 UTC (permalink / raw)
  To: Maarten Lankhorst; +Cc: intel-gfx

On Fri, Oct 27, 2017 at 01:43:54PM +0200, Maarten Lankhorst wrote:
> Op 19-10-17 om 15:37 schreef Ville Syrjala:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Reduce our general reliance on encoder->type and instead use
> > output_types from the crtc state when enabling LPE audio.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_audio.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> > index 0ddba16fde1b..1e59efbfe8a3 100644
> > --- a/drivers/gpu/drm/i915/intel_audio.c
> > +++ b/drivers/gpu/drm/i915/intel_audio.c
> > @@ -629,7 +629,7 @@ void intel_audio_codec_enable(struct intel_encoder *intel_encoder,
> >  
> >  	intel_lpe_audio_notify(dev_priv, pipe, port, connector->eld,
> >  			       crtc_state->port_clock,
> > -			       intel_encoder->type == INTEL_OUTPUT_DP);
> > +			       intel_crtc_has_dp_encoder(crtc_state));
> >  }
> >  
> >  /**
> 
> Should this patch be moved to before 05/10?

Actually I'd just drop this for now. Jani pointed out that there are a
few other cases in the audio code as well. And after I had a look at it
I came up with a more thorough patch that plumbs the crtc/connector
states all the way down in the audio code. I'll post that as part of yet
another cleanup series.

-- 
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] 27+ messages in thread

* Re: [PATCH 09/10] drm/i915: Use intel_ddi_get_config() for MST
  2017-10-27 11:43   ` Maarten Lankhorst
@ 2017-10-27 11:55     ` Ville Syrjälä
  0 siblings, 0 replies; 27+ messages in thread
From: Ville Syrjälä @ 2017-10-27 11:55 UTC (permalink / raw)
  To: Maarten Lankhorst; +Cc: intel-gfx

On Fri, Oct 27, 2017 at 01:43:03PM +0200, Maarten Lankhorst wrote:
> Op 19-10-17 om 15:37 schreef Ville Syrjala:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Eliminate the partially duplicated DDI readout code from MST, and
> > instead just call intel_ddi_get_config(). As a nice bonus we get
> > more cross checking as intel_ddi_get_config() will populate
> > output_types based on the actual mode of the DDI port.
> >
> > Additonally intel_ddi_get_config() must be changed to get the crtc
> > from the passed in crtc state rather than from the encoder->crtc link.
> > encoder->crtc really shouldn't be used anyway.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> I looked and noticed 2 small differences, one is the GEN9_LP part, the other is
> min_voltage_level. Is this also a bugfix because of this?

The min_voltage thing should be fine. We should have it in both DDI and
MST code (unless I made a mistake when adding it, but doesn't look like
I did).

The bxt lat_optim thing is a real issue. Looks like I need to add that to
the MST compute_config. Though as a whole it feels a bit overkill to
track that particular piece of state specifically. We don't do that on
CHV either, which has the exact same PHY.

-- 
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] 27+ messages in thread

* Re: [PATCH v5 05/10] drm/i915: Stop frobbing with DDI encoder->type
  2017-10-19 13:37 ` [PATCH v5 05/10] drm/i915: Stop frobbing with DDI encoder->type Ville Syrjala
@ 2017-10-27 12:05   ` Maarten Lankhorst
  2017-10-27 12:44     ` Ville Syrjälä
  2017-10-27 19:32     ` Ville Syrjälä
  0 siblings, 2 replies; 27+ messages in thread
From: Maarten Lankhorst @ 2017-10-27 12:05 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx

Op 19-10-17 om 15:37 schreef Ville Syrjala:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Currently the DDI encoder->type will change at runtime depending on
> what kind of hotplugs we've processed. That's quite bad since we can't
> really trust that that current value of encoder->type actually matches
> the type of signal we're trying to drive through it.
>
> Let's eliminate that problem by declaring that non-eDP DDI port will
> always have the encoder type as INTEL_OUTPUT_DDI. This means the code
> can no longer try to distinguish DP vs. HDMI based on encoder->type.
> We'll leave eDP as INTEL_OUTPUT_EDP, since it'll never change and
> there's a bunch of code that relies on that value to identify eDP
> encoders.
>
> We'll introduce a new encoder .compute_output_type() hook. This allows
> us to compute the full output_types before any encoder .compute_config()
> hooks get called, thus those hooks can rely on output_types being
> correct, which is useful for cloning on oldr platforms. For now we'll
> just look at the connector type and pick the correct mode based on that.
> In the future the new hook could be used to implement dynamic switching
> between LS and PCON modes for LSPCON.
>
> TODO: maybe make .get_config() populate output_types rather than doing
> the default encoder->type thing in caller and then undoing it for
> DDI in .get_config().
>
> v2: Fix BXT/GLK PPS explosion with DSI/MST encoders
> v3: Avoid the PPS warn on pure HDMI/DVI DDI encoders by checking dp.output_reg
> v4: Rebase
> v5: Populate output_types in .get_config() rather than in the caller

Could we get rid of compute_output_type in this patch? Perhaps do it as preliminary
patch towards removing the use?

Rest of the series except patch 8 looks good. For patch 1-4, 6, 7, 9 (if the delta
between ddi_get_config and old mst get_config is harmless or beneficial) and 10:

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

Thanks for the work towards cleaning this mess up.

~Maarten
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c   |  2 +-
>  drivers/gpu/drm/i915/intel_crt.c      |  2 ++
>  drivers/gpu/drm/i915/intel_ddi.c      | 43 ++++++++++++++++++++++++++++-------
>  drivers/gpu/drm/i915/intel_display.c  | 16 +++++++------
>  drivers/gpu/drm/i915/intel_dp.c       | 24 +++++++++----------
>  drivers/gpu/drm/i915/intel_dp_mst.c   |  2 ++
>  drivers/gpu/drm/i915/intel_drv.h      |  7 ++++--
>  drivers/gpu/drm/i915/intel_dsi.c      |  2 ++
>  drivers/gpu/drm/i915/intel_dvo.c      |  2 ++
>  drivers/gpu/drm/i915/intel_hdmi.c     | 12 ++++------
>  drivers/gpu/drm/i915/intel_lvds.c     |  2 ++
>  drivers/gpu/drm/i915/intel_opregion.c |  2 +-
>  drivers/gpu/drm/i915/intel_sdvo.c     |  2 ++
>  drivers/gpu/drm/i915/intel_tv.c       |  2 ++
>  14 files changed, 80 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index c65e381b85f3..e5333bfdf32d 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -3049,7 +3049,7 @@ static void intel_connector_info(struct seq_file *m,
>  		break;
>  	case DRM_MODE_CONNECTOR_HDMIA:
>  		if (intel_encoder->type == INTEL_OUTPUT_HDMI ||
> -		    intel_encoder->type == INTEL_OUTPUT_UNKNOWN)
> +		    intel_encoder->type == INTEL_OUTPUT_DDI)
>  			intel_hdmi_info(m, intel_connector);
>  		break;
>  	default:
> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> index 668e8c3e791d..a61e61efe9aa 100644
> --- a/drivers/gpu/drm/i915/intel_crt.c
> +++ b/drivers/gpu/drm/i915/intel_crt.c
> @@ -119,6 +119,8 @@ static unsigned int intel_crt_get_flags(struct intel_encoder *encoder)
>  static void intel_crt_get_config(struct intel_encoder *encoder,
>  				 struct intel_crtc_state *pipe_config)
>  {
> +	pipe_config->output_types |= BIT(INTEL_OUTPUT_ANALOG);
> +
>  	pipe_config->base.adjusted_mode.flags |= intel_crt_get_flags(encoder);
>  
>  	pipe_config->base.adjusted_mode.crtc_clock = pipe_config->port_clock;
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index c8790bb74839..630609575db4 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -497,10 +497,8 @@ enum port intel_ddi_get_encoder_port(struct intel_encoder *encoder)
>  	switch (encoder->type) {
>  	case INTEL_OUTPUT_DP_MST:
>  		return enc_to_mst(&encoder->base)->primary->port;
> -	case INTEL_OUTPUT_DP:
>  	case INTEL_OUTPUT_EDP:
> -	case INTEL_OUTPUT_HDMI:
> -	case INTEL_OUTPUT_UNKNOWN:
> +	case INTEL_OUTPUT_DDI:
>  		return enc_to_dig_port(&encoder->base)->port;
>  	case INTEL_OUTPUT_ANALOG:
>  		return PORT_E;
> @@ -1532,6 +1530,7 @@ void intel_ddi_set_vc_payload_alloc(const struct intel_crtc_state *crtc_state,
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>  	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>  	uint32_t temp;
> +
>  	temp = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
>  	if (state == true)
>  		temp |= TRANS_DDI_DP_VC_PAYLOAD_ALLOC;
> @@ -2586,12 +2585,23 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
>  			pipe_config->hdmi_high_tmds_clock_ratio = true;
>  		/* fall through */
>  	case TRANS_DDI_MODE_SELECT_DVI:
> +		pipe_config->output_types |= BIT(INTEL_OUTPUT_HDMI);
>  		pipe_config->lane_count = 4;
>  		break;
>  	case TRANS_DDI_MODE_SELECT_FDI:
> +		pipe_config->output_types |= BIT(INTEL_OUTPUT_ANALOG);
>  		break;
>  	case TRANS_DDI_MODE_SELECT_DP_SST:
> +		if (encoder->type == INTEL_OUTPUT_EDP)
> +			pipe_config->output_types |= BIT(INTEL_OUTPUT_EDP);
> +		else
> +			pipe_config->output_types |= BIT(INTEL_OUTPUT_DP);
> +		pipe_config->lane_count =
> +			((temp & DDI_PORT_WIDTH_MASK) >> DDI_PORT_WIDTH_SHIFT) + 1;
> +		intel_dp_get_m_n(intel_crtc, pipe_config);
> +		break;
>  	case TRANS_DDI_MODE_SELECT_DP_MST:
> +		pipe_config->output_types |= BIT(INTEL_OUTPUT_DP_MST);
>  		pipe_config->lane_count =
>  			((temp & DDI_PORT_WIDTH_MASK) >> DDI_PORT_WIDTH_SHIFT) + 1;
>  		intel_dp_get_m_n(intel_crtc, pipe_config);
> @@ -2630,21 +2640,36 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
>  			bxt_ddi_phy_get_lane_lat_optim_mask(encoder);
>  }
>  
> +static enum intel_output_type
> +intel_ddi_compute_output_type(struct intel_encoder *encoder,
> +			      struct intel_crtc_state *crtc_state,
> +			      struct drm_connector_state *conn_state)
> +{
> +	switch (conn_state->connector->connector_type) {
> +	case DRM_MODE_CONNECTOR_HDMIA:
> +		return INTEL_OUTPUT_HDMI;
> +	case DRM_MODE_CONNECTOR_eDP:
> +		return INTEL_OUTPUT_EDP;
> +	case DRM_MODE_CONNECTOR_DisplayPort:
> +		return INTEL_OUTPUT_DP;
> +	default:
> +		MISSING_CASE(conn_state->connector->connector_type);
> +		return INTEL_OUTPUT_UNUSED;
> +	}
> +}
> +
>  static bool intel_ddi_compute_config(struct intel_encoder *encoder,
>  				     struct intel_crtc_state *pipe_config,
>  				     struct drm_connector_state *conn_state)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> -	int type = encoder->type;
>  	int port = intel_ddi_get_encoder_port(encoder);
>  	int ret;
>  
> -	WARN(type == INTEL_OUTPUT_UNKNOWN, "compute_config() on unknown output!\n");
> -
>  	if (port == PORT_A)
>  		pipe_config->cpu_transcoder = TRANSCODER_EDP;
>  
> -	if (type == INTEL_OUTPUT_HDMI)
> +	if (intel_crtc_has_type(pipe_config, INTEL_OUTPUT_HDMI))
>  		ret = intel_hdmi_compute_config(encoder, pipe_config, conn_state);
>  	else
>  		ret = intel_dp_compute_config(encoder, pipe_config, conn_state);
> @@ -2764,6 +2789,7 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>  	drm_encoder_init(&dev_priv->drm, encoder, &intel_ddi_funcs,
>  			 DRM_MODE_ENCODER_TMDS, "DDI %c", port_name(port));
>  
> +	intel_encoder->compute_output_type = intel_ddi_compute_output_type;
>  	intel_encoder->compute_config = intel_ddi_compute_config;
>  	intel_encoder->enable = intel_enable_ddi;
>  	if (IS_GEN9_LP(dev_priv))
> @@ -2821,9 +2847,10 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>  		}
>  	}
>  
> +	intel_dig_port->dp.output_reg = INVALID_MMIO_REG;
>  	intel_dig_port->max_lanes = max_lanes;
>  
> -	intel_encoder->type = INTEL_OUTPUT_UNKNOWN;
> +	intel_encoder->type = INTEL_OUTPUT_DDI;
>  	intel_encoder->power_domain = intel_port_to_power_domain(port);
>  	intel_encoder->port = port;
>  	intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index bd62c0a65bcd..a7e02ae33583 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -10591,7 +10591,7 @@ static const char * const output_type_str[] = {
>  	OUTPUT_TYPE(DP),
>  	OUTPUT_TYPE(EDP),
>  	OUTPUT_TYPE(DSI),
> -	OUTPUT_TYPE(UNKNOWN),
> +	OUTPUT_TYPE(DDI),
>  	OUTPUT_TYPE(DP_MST),
>  };
>  
> @@ -10762,7 +10762,7 @@ static bool check_digital_port_conflicts(struct drm_atomic_state *state)
>  
>  		switch (encoder->type) {
>  			unsigned int port_mask;
> -		case INTEL_OUTPUT_UNKNOWN:
> +		case INTEL_OUTPUT_DDI:
>  			if (WARN_ON(!HAS_DDI(to_i915(dev))))
>  				break;
>  		case INTEL_OUTPUT_DP:
> @@ -10895,7 +10895,12 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
>  		 * Determine output_types before calling the .compute_config()
>  		 * hooks so that the hooks can use this information safely.
>  		 */
> -		pipe_config->output_types |= 1 << encoder->type;
> +		if (encoder->compute_output_type)
> +			pipe_config->output_types |=
> +				BIT(encoder->compute_output_type(encoder, pipe_config,
> +								 connector_state));
> +		else
> +			pipe_config->output_types |= BIT(encoder->type);
>  	}
>  
>  encoder_retry:
> @@ -11572,10 +11577,8 @@ verify_crtc_state(struct drm_crtc *crtc,
>  				"Encoder connected to wrong pipe %c\n",
>  				pipe_name(pipe));
>  
> -		if (active) {
> -			pipe_config->output_types |= 1 << encoder->type;
> +		if (active)
>  			encoder->get_config(encoder, pipe_config);
> -		}
>  	}
>  
>  	intel_crtc_compute_pixel_rate(pipe_config);
> @@ -14963,7 +14966,6 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>  			crtc_state = to_intel_crtc_state(crtc->base.state);
>  
>  			encoder->base.crtc = &crtc->base;
> -			crtc_state->output_types |= 1 << encoder->type;
>  			encoder->get_config(encoder, crtc_state);
>  		} else {
>  			encoder->base.crtc = NULL;
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index aa75f55eeb61..2b482d12000e 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -758,11 +758,16 @@ void intel_power_sequencer_reset(struct drm_i915_private *dev_priv)
>  		struct intel_dp *intel_dp;
>  
>  		if (encoder->type != INTEL_OUTPUT_DP &&
> -		    encoder->type != INTEL_OUTPUT_EDP)
> +		    encoder->type != INTEL_OUTPUT_EDP &&
> +		    encoder->type != INTEL_OUTPUT_DDI)
>  			continue;
>  
>  		intel_dp = enc_to_intel_dp(&encoder->base);
>  
> +		/* Skip pure DVI/HDMI DDI encoders */
> +		if (!i915_mmio_reg_valid(intel_dp->output_reg))
> +			continue;
> +
>  		WARN_ON(intel_dp->active_pipe != INVALID_PIPE);
>  
>  		if (encoder->type != INTEL_OUTPUT_EDP)
> @@ -2603,6 +2608,11 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
>  	enum port port = dp_to_dig_port(intel_dp)->port;
>  	struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
>  
> +	if (encoder->type == INTEL_OUTPUT_EDP)
> +		pipe_config->output_types |= BIT(INTEL_OUTPUT_EDP);
> +	else
> +		pipe_config->output_types |= BIT(INTEL_OUTPUT_DP);
> +
>  	tmp = I915_READ(intel_dp->output_reg);
>  
>  	pipe_config->has_audio = tmp & DP_AUDIO_OUTPUT_ENABLE && port != PORT_A;
> @@ -4699,8 +4709,6 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
>  {
>  	struct drm_connector *connector = &intel_connector->base;
>  	struct intel_dp *intel_dp = intel_attached_dp(connector);
> -	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> -	struct intel_encoder *intel_encoder = &intel_dig_port->base;
>  	struct drm_device *dev = connector->dev;
>  	enum drm_connector_status status;
>  	u8 sink_irq_vector = 0;
> @@ -4733,9 +4741,6 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
>  		goto out;
>  	}
>  
> -	if (intel_encoder->type != INTEL_OUTPUT_EDP)
> -		intel_encoder->type = INTEL_OUTPUT_DP;
> -
>  	if (intel_dp->reset_link_params) {
>  		/* Initial max link lane count */
>  		intel_dp->max_link_lane_count = intel_dp_max_common_lane_count(intel_dp);
> @@ -4852,9 +4857,6 @@ intel_dp_force(struct drm_connector *connector)
>  	intel_dp_set_edid(intel_dp);
>  
>  	intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
> -
> -	if (intel_encoder->type != INTEL_OUTPUT_EDP)
> -		intel_encoder->type = INTEL_OUTPUT_DP;
>  }
>  
>  static int intel_dp_get_modes(struct drm_connector *connector)
> @@ -5073,10 +5075,6 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	enum irqreturn ret = IRQ_NONE;
>  
> -	if (intel_dig_port->base.type != INTEL_OUTPUT_EDP &&
> -	    intel_dig_port->base.type != INTEL_OUTPUT_HDMI)
> -		intel_dig_port->base.type = INTEL_OUTPUT_DP;
> -
>  	if (long_hpd && intel_dig_port->base.type == INTEL_OUTPUT_EDP) {
>  		/*
>  		 * vdd off can generate a long pulse on eDP which
> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
> index 772521440a9f..9be6f32bb100 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -270,6 +270,8 @@ static void intel_dp_mst_enc_get_config(struct intel_encoder *encoder,
>  	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
>  	u32 temp, flags = 0;
>  
> +	pipe_config->output_types |= BIT(INTEL_OUTPUT_DP_MST);
> +
>  	pipe_config->has_audio =
>  		intel_ddi_is_audio_enabled(dev_priv, crtc);
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index a05ab3a1ab27..650129980ce2 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -173,7 +173,7 @@ enum intel_output_type {
>  	INTEL_OUTPUT_DP = 7,
>  	INTEL_OUTPUT_EDP = 8,
>  	INTEL_OUTPUT_DSI = 9,
> -	INTEL_OUTPUT_UNKNOWN = 10,
> +	INTEL_OUTPUT_DDI = 10,
>  	INTEL_OUTPUT_DP_MST = 11,
>  };
>  
> @@ -216,6 +216,9 @@ struct intel_encoder {
>  	enum port port;
>  	unsigned int cloneable;
>  	void (*hot_plug)(struct intel_encoder *);
> +	enum intel_output_type (*compute_output_type)(struct intel_encoder *,
> +						      struct intel_crtc_state *,
> +						      struct drm_connector_state *);
>  	bool (*compute_config)(struct intel_encoder *,
>  			       struct intel_crtc_state *,
>  			       struct drm_connector_state *);
> @@ -1149,7 +1152,7 @@ enc_to_dig_port(struct drm_encoder *encoder)
>  	struct intel_encoder *intel_encoder = to_intel_encoder(encoder);
>  
>  	switch (intel_encoder->type) {
> -	case INTEL_OUTPUT_UNKNOWN:
> +	case INTEL_OUTPUT_DDI:
>  		WARN_ON(!HAS_DDI(to_i915(encoder->dev)));
>  	case INTEL_OUTPUT_DP:
>  	case INTEL_OUTPUT_EDP:
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 66bbedc5fa01..2ae3eea19317 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -1243,6 +1243,8 @@ static void intel_dsi_get_config(struct intel_encoder *encoder,
>  	u32 pclk;
>  	DRM_DEBUG_KMS("\n");
>  
> +	pipe_config->output_types |= BIT(INTEL_OUTPUT_DSI);
> +
>  	if (IS_GEN9_LP(dev_priv))
>  		bxt_dsi_get_pipe_config(encoder, pipe_config);
>  
> diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
> index 53c9b763f4ce..754baa00bea9 100644
> --- a/drivers/gpu/drm/i915/intel_dvo.c
> +++ b/drivers/gpu/drm/i915/intel_dvo.c
> @@ -159,6 +159,8 @@ static void intel_dvo_get_config(struct intel_encoder *encoder,
>  	struct intel_dvo *intel_dvo = enc_to_dvo(encoder);
>  	u32 tmp, flags = 0;
>  
> +	pipe_config->output_types |= BIT(INTEL_OUTPUT_DVO);
> +
>  	tmp = I915_READ(intel_dvo->dev.dvo_reg);
>  	if (tmp & DVO_HSYNC_ACTIVE_HIGH)
>  		flags |= DRM_MODE_FLAG_PHSYNC;
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 5132dc814788..1ff448a67b99 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -957,6 +957,8 @@ static void intel_hdmi_get_config(struct intel_encoder *encoder,
>  	u32 tmp, flags = 0;
>  	int dotclock;
>  
> +	pipe_config->output_types |= BIT(INTEL_OUTPUT_HDMI);
> +
>  	tmp = I915_READ(intel_hdmi->hdmi_reg);
>  
>  	if (tmp & SDVO_HSYNC_ACTIVE_HIGH)
> @@ -1610,12 +1612,9 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
>  
>  	intel_hdmi_unset_edid(connector);
>  
> -	if (intel_hdmi_set_edid(connector)) {
> -		struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
> -
> -		hdmi_to_dig_port(intel_hdmi)->base.type = INTEL_OUTPUT_HDMI;
> +	if (intel_hdmi_set_edid(connector))
>  		status = connector_status_connected;
> -	} else
> +	else
>  		status = connector_status_disconnected;
>  
>  	intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS);
> @@ -1626,8 +1625,6 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
>  static void
>  intel_hdmi_force(struct drm_connector *connector)
>  {
> -	struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
> -
>  	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
>  		      connector->base.id, connector->name);
>  
> @@ -1637,7 +1634,6 @@ intel_hdmi_force(struct drm_connector *connector)
>  		return;
>  
>  	intel_hdmi_set_edid(connector);
> -	hdmi_to_dig_port(intel_hdmi)->base.type = INTEL_OUTPUT_HDMI;
>  }
>  
>  static int intel_hdmi_get_modes(struct drm_connector *connector)
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index 38572d65e46e..ef80499113ee 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -125,6 +125,8 @@ static void intel_lvds_get_config(struct intel_encoder *encoder,
>  	struct intel_lvds_encoder *lvds_encoder = to_lvds_encoder(&encoder->base);
>  	u32 tmp, flags = 0;
>  
> +	pipe_config->output_types |= BIT(INTEL_OUTPUT_LVDS);
> +
>  	tmp = I915_READ(lvds_encoder->reg);
>  	if (tmp & LVDS_HSYNC_POLARITY)
>  		flags |= DRM_MODE_FLAG_NHSYNC;
> diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
> index 1d946240e55f..39714be3eb6b 100644
> --- a/drivers/gpu/drm/i915/intel_opregion.c
> +++ b/drivers/gpu/drm/i915/intel_opregion.c
> @@ -383,7 +383,7 @@ int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder,
>  	case INTEL_OUTPUT_ANALOG:
>  		type = DISPLAY_TYPE_CRT;
>  		break;
> -	case INTEL_OUTPUT_UNKNOWN:
> +	case INTEL_OUTPUT_DDI:
>  	case INTEL_OUTPUT_DP:
>  	case INTEL_OUTPUT_HDMI:
>  	case INTEL_OUTPUT_DP_MST:
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index 7437944b388f..42ec2d1f7a61 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -1429,6 +1429,8 @@ static void intel_sdvo_get_config(struct intel_encoder *encoder,
>  	u8 val;
>  	bool ret;
>  
> +	pipe_config->output_types |= BIT(INTEL_OUTPUT_SDVO);
> +
>  	sdvox = I915_READ(intel_sdvo->sdvo_reg);
>  
>  	ret = intel_sdvo_get_input_timing(intel_sdvo, &dtd);
> diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
> index a79a7591b2cf..b18609cebe03 100644
> --- a/drivers/gpu/drm/i915/intel_tv.c
> +++ b/drivers/gpu/drm/i915/intel_tv.c
> @@ -868,6 +868,8 @@ static void
>  intel_tv_get_config(struct intel_encoder *encoder,
>  		    struct intel_crtc_state *pipe_config)
>  {
> +	pipe_config->output_types |= BIT(INTEL_OUTPUT_TVOUT);
> +
>  	pipe_config->base.adjusted_mode.crtc_clock = pipe_config->port_clock;
>  }
>  


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

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

* Re: [PATCH v5 05/10] drm/i915: Stop frobbing with DDI encoder->type
  2017-10-27 12:05   ` Maarten Lankhorst
@ 2017-10-27 12:44     ` Ville Syrjälä
  2017-10-27 13:53       ` Maarten Lankhorst
  2017-10-27 19:32     ` Ville Syrjälä
  1 sibling, 1 reply; 27+ messages in thread
From: Ville Syrjälä @ 2017-10-27 12:44 UTC (permalink / raw)
  To: Maarten Lankhorst; +Cc: intel-gfx

On Fri, Oct 27, 2017 at 02:05:38PM +0200, Maarten Lankhorst wrote:
> Op 19-10-17 om 15:37 schreef Ville Syrjala:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Currently the DDI encoder->type will change at runtime depending on
> > what kind of hotplugs we've processed. That's quite bad since we can't
> > really trust that that current value of encoder->type actually matches
> > the type of signal we're trying to drive through it.
> >
> > Let's eliminate that problem by declaring that non-eDP DDI port will
> > always have the encoder type as INTEL_OUTPUT_DDI. This means the code
> > can no longer try to distinguish DP vs. HDMI based on encoder->type.
> > We'll leave eDP as INTEL_OUTPUT_EDP, since it'll never change and
> > there's a bunch of code that relies on that value to identify eDP
> > encoders.
> >
> > We'll introduce a new encoder .compute_output_type() hook. This allows
> > us to compute the full output_types before any encoder .compute_config()
> > hooks get called, thus those hooks can rely on output_types being
> > correct, which is useful for cloning on oldr platforms. For now we'll
> > just look at the connector type and pick the correct mode based on that.
> > In the future the new hook could be used to implement dynamic switching
> > between LS and PCON modes for LSPCON.
> >
> > TODO: maybe make .get_config() populate output_types rather than doing
> > the default encoder->type thing in caller and then undoing it for
> > DDI in .get_config().
> >
> > v2: Fix BXT/GLK PPS explosion with DSI/MST encoders
> > v3: Avoid the PPS warn on pure HDMI/DVI DDI encoders by checking dp.output_reg
> > v4: Rebase
> > v5: Populate output_types in .get_config() rather than in the caller
> 
> Could we get rid of compute_output_type in this patch? Perhaps do it as preliminary
> patch towards removing the use?

We don't want to remove it.

> 
> Rest of the series except patch 8 looks good. For patch 1-4, 6, 7, 9 (if the delta
> between ddi_get_config and old mst get_config is harmless or beneficial) and 10:
> 
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> 
> Thanks for the work towards cleaning this mess up.
> 
> ~Maarten
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_debugfs.c   |  2 +-
> >  drivers/gpu/drm/i915/intel_crt.c      |  2 ++
> >  drivers/gpu/drm/i915/intel_ddi.c      | 43 ++++++++++++++++++++++++++++-------
> >  drivers/gpu/drm/i915/intel_display.c  | 16 +++++++------
> >  drivers/gpu/drm/i915/intel_dp.c       | 24 +++++++++----------
> >  drivers/gpu/drm/i915/intel_dp_mst.c   |  2 ++
> >  drivers/gpu/drm/i915/intel_drv.h      |  7 ++++--
> >  drivers/gpu/drm/i915/intel_dsi.c      |  2 ++
> >  drivers/gpu/drm/i915/intel_dvo.c      |  2 ++
> >  drivers/gpu/drm/i915/intel_hdmi.c     | 12 ++++------
> >  drivers/gpu/drm/i915/intel_lvds.c     |  2 ++
> >  drivers/gpu/drm/i915/intel_opregion.c |  2 +-
> >  drivers/gpu/drm/i915/intel_sdvo.c     |  2 ++
> >  drivers/gpu/drm/i915/intel_tv.c       |  2 ++
> >  14 files changed, 80 insertions(+), 40 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> > index c65e381b85f3..e5333bfdf32d 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -3049,7 +3049,7 @@ static void intel_connector_info(struct seq_file *m,
> >  		break;
> >  	case DRM_MODE_CONNECTOR_HDMIA:
> >  		if (intel_encoder->type == INTEL_OUTPUT_HDMI ||
> > -		    intel_encoder->type == INTEL_OUTPUT_UNKNOWN)
> > +		    intel_encoder->type == INTEL_OUTPUT_DDI)
> >  			intel_hdmi_info(m, intel_connector);
> >  		break;
> >  	default:
> > diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> > index 668e8c3e791d..a61e61efe9aa 100644
> > --- a/drivers/gpu/drm/i915/intel_crt.c
> > +++ b/drivers/gpu/drm/i915/intel_crt.c
> > @@ -119,6 +119,8 @@ static unsigned int intel_crt_get_flags(struct intel_encoder *encoder)
> >  static void intel_crt_get_config(struct intel_encoder *encoder,
> >  				 struct intel_crtc_state *pipe_config)
> >  {
> > +	pipe_config->output_types |= BIT(INTEL_OUTPUT_ANALOG);
> > +
> >  	pipe_config->base.adjusted_mode.flags |= intel_crt_get_flags(encoder);
> >  
> >  	pipe_config->base.adjusted_mode.crtc_clock = pipe_config->port_clock;
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > index c8790bb74839..630609575db4 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -497,10 +497,8 @@ enum port intel_ddi_get_encoder_port(struct intel_encoder *encoder)
> >  	switch (encoder->type) {
> >  	case INTEL_OUTPUT_DP_MST:
> >  		return enc_to_mst(&encoder->base)->primary->port;
> > -	case INTEL_OUTPUT_DP:
> >  	case INTEL_OUTPUT_EDP:
> > -	case INTEL_OUTPUT_HDMI:
> > -	case INTEL_OUTPUT_UNKNOWN:
> > +	case INTEL_OUTPUT_DDI:
> >  		return enc_to_dig_port(&encoder->base)->port;
> >  	case INTEL_OUTPUT_ANALOG:
> >  		return PORT_E;
> > @@ -1532,6 +1530,7 @@ void intel_ddi_set_vc_payload_alloc(const struct intel_crtc_state *crtc_state,
> >  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> >  	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> >  	uint32_t temp;
> > +
> >  	temp = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
> >  	if (state == true)
> >  		temp |= TRANS_DDI_DP_VC_PAYLOAD_ALLOC;
> > @@ -2586,12 +2585,23 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
> >  			pipe_config->hdmi_high_tmds_clock_ratio = true;
> >  		/* fall through */
> >  	case TRANS_DDI_MODE_SELECT_DVI:
> > +		pipe_config->output_types |= BIT(INTEL_OUTPUT_HDMI);
> >  		pipe_config->lane_count = 4;
> >  		break;
> >  	case TRANS_DDI_MODE_SELECT_FDI:
> > +		pipe_config->output_types |= BIT(INTEL_OUTPUT_ANALOG);
> >  		break;
> >  	case TRANS_DDI_MODE_SELECT_DP_SST:
> > +		if (encoder->type == INTEL_OUTPUT_EDP)
> > +			pipe_config->output_types |= BIT(INTEL_OUTPUT_EDP);
> > +		else
> > +			pipe_config->output_types |= BIT(INTEL_OUTPUT_DP);
> > +		pipe_config->lane_count =
> > +			((temp & DDI_PORT_WIDTH_MASK) >> DDI_PORT_WIDTH_SHIFT) + 1;
> > +		intel_dp_get_m_n(intel_crtc, pipe_config);
> > +		break;
> >  	case TRANS_DDI_MODE_SELECT_DP_MST:
> > +		pipe_config->output_types |= BIT(INTEL_OUTPUT_DP_MST);
> >  		pipe_config->lane_count =
> >  			((temp & DDI_PORT_WIDTH_MASK) >> DDI_PORT_WIDTH_SHIFT) + 1;
> >  		intel_dp_get_m_n(intel_crtc, pipe_config);
> > @@ -2630,21 +2640,36 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
> >  			bxt_ddi_phy_get_lane_lat_optim_mask(encoder);
> >  }
> >  
> > +static enum intel_output_type
> > +intel_ddi_compute_output_type(struct intel_encoder *encoder,
> > +			      struct intel_crtc_state *crtc_state,
> > +			      struct drm_connector_state *conn_state)
> > +{
> > +	switch (conn_state->connector->connector_type) {
> > +	case DRM_MODE_CONNECTOR_HDMIA:
> > +		return INTEL_OUTPUT_HDMI;
> > +	case DRM_MODE_CONNECTOR_eDP:
> > +		return INTEL_OUTPUT_EDP;
> > +	case DRM_MODE_CONNECTOR_DisplayPort:
> > +		return INTEL_OUTPUT_DP;
> > +	default:
> > +		MISSING_CASE(conn_state->connector->connector_type);
> > +		return INTEL_OUTPUT_UNUSED;
> > +	}
> > +}
> > +
> >  static bool intel_ddi_compute_config(struct intel_encoder *encoder,
> >  				     struct intel_crtc_state *pipe_config,
> >  				     struct drm_connector_state *conn_state)
> >  {
> >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > -	int type = encoder->type;
> >  	int port = intel_ddi_get_encoder_port(encoder);
> >  	int ret;
> >  
> > -	WARN(type == INTEL_OUTPUT_UNKNOWN, "compute_config() on unknown output!\n");
> > -
> >  	if (port == PORT_A)
> >  		pipe_config->cpu_transcoder = TRANSCODER_EDP;
> >  
> > -	if (type == INTEL_OUTPUT_HDMI)
> > +	if (intel_crtc_has_type(pipe_config, INTEL_OUTPUT_HDMI))
> >  		ret = intel_hdmi_compute_config(encoder, pipe_config, conn_state);
> >  	else
> >  		ret = intel_dp_compute_config(encoder, pipe_config, conn_state);
> > @@ -2764,6 +2789,7 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
> >  	drm_encoder_init(&dev_priv->drm, encoder, &intel_ddi_funcs,
> >  			 DRM_MODE_ENCODER_TMDS, "DDI %c", port_name(port));
> >  
> > +	intel_encoder->compute_output_type = intel_ddi_compute_output_type;
> >  	intel_encoder->compute_config = intel_ddi_compute_config;
> >  	intel_encoder->enable = intel_enable_ddi;
> >  	if (IS_GEN9_LP(dev_priv))
> > @@ -2821,9 +2847,10 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
> >  		}
> >  	}
> >  
> > +	intel_dig_port->dp.output_reg = INVALID_MMIO_REG;
> >  	intel_dig_port->max_lanes = max_lanes;
> >  
> > -	intel_encoder->type = INTEL_OUTPUT_UNKNOWN;
> > +	intel_encoder->type = INTEL_OUTPUT_DDI;
> >  	intel_encoder->power_domain = intel_port_to_power_domain(port);
> >  	intel_encoder->port = port;
> >  	intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index bd62c0a65bcd..a7e02ae33583 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -10591,7 +10591,7 @@ static const char * const output_type_str[] = {
> >  	OUTPUT_TYPE(DP),
> >  	OUTPUT_TYPE(EDP),
> >  	OUTPUT_TYPE(DSI),
> > -	OUTPUT_TYPE(UNKNOWN),
> > +	OUTPUT_TYPE(DDI),
> >  	OUTPUT_TYPE(DP_MST),
> >  };
> >  
> > @@ -10762,7 +10762,7 @@ static bool check_digital_port_conflicts(struct drm_atomic_state *state)
> >  
> >  		switch (encoder->type) {
> >  			unsigned int port_mask;
> > -		case INTEL_OUTPUT_UNKNOWN:
> > +		case INTEL_OUTPUT_DDI:
> >  			if (WARN_ON(!HAS_DDI(to_i915(dev))))
> >  				break;
> >  		case INTEL_OUTPUT_DP:
> > @@ -10895,7 +10895,12 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
> >  		 * Determine output_types before calling the .compute_config()
> >  		 * hooks so that the hooks can use this information safely.
> >  		 */
> > -		pipe_config->output_types |= 1 << encoder->type;
> > +		if (encoder->compute_output_type)
> > +			pipe_config->output_types |=
> > +				BIT(encoder->compute_output_type(encoder, pipe_config,
> > +								 connector_state));
> > +		else
> > +			pipe_config->output_types |= BIT(encoder->type);
> >  	}
> >  
> >  encoder_retry:
> > @@ -11572,10 +11577,8 @@ verify_crtc_state(struct drm_crtc *crtc,
> >  				"Encoder connected to wrong pipe %c\n",
> >  				pipe_name(pipe));
> >  
> > -		if (active) {
> > -			pipe_config->output_types |= 1 << encoder->type;
> > +		if (active)
> >  			encoder->get_config(encoder, pipe_config);
> > -		}
> >  	}
> >  
> >  	intel_crtc_compute_pixel_rate(pipe_config);
> > @@ -14963,7 +14966,6 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
> >  			crtc_state = to_intel_crtc_state(crtc->base.state);
> >  
> >  			encoder->base.crtc = &crtc->base;
> > -			crtc_state->output_types |= 1 << encoder->type;
> >  			encoder->get_config(encoder, crtc_state);
> >  		} else {
> >  			encoder->base.crtc = NULL;
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index aa75f55eeb61..2b482d12000e 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -758,11 +758,16 @@ void intel_power_sequencer_reset(struct drm_i915_private *dev_priv)
> >  		struct intel_dp *intel_dp;
> >  
> >  		if (encoder->type != INTEL_OUTPUT_DP &&
> > -		    encoder->type != INTEL_OUTPUT_EDP)
> > +		    encoder->type != INTEL_OUTPUT_EDP &&
> > +		    encoder->type != INTEL_OUTPUT_DDI)
> >  			continue;
> >  
> >  		intel_dp = enc_to_intel_dp(&encoder->base);
> >  
> > +		/* Skip pure DVI/HDMI DDI encoders */
> > +		if (!i915_mmio_reg_valid(intel_dp->output_reg))
> > +			continue;
> > +
> >  		WARN_ON(intel_dp->active_pipe != INVALID_PIPE);
> >  
> >  		if (encoder->type != INTEL_OUTPUT_EDP)
> > @@ -2603,6 +2608,11 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
> >  	enum port port = dp_to_dig_port(intel_dp)->port;
> >  	struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
> >  
> > +	if (encoder->type == INTEL_OUTPUT_EDP)
> > +		pipe_config->output_types |= BIT(INTEL_OUTPUT_EDP);
> > +	else
> > +		pipe_config->output_types |= BIT(INTEL_OUTPUT_DP);
> > +
> >  	tmp = I915_READ(intel_dp->output_reg);
> >  
> >  	pipe_config->has_audio = tmp & DP_AUDIO_OUTPUT_ENABLE && port != PORT_A;
> > @@ -4699,8 +4709,6 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
> >  {
> >  	struct drm_connector *connector = &intel_connector->base;
> >  	struct intel_dp *intel_dp = intel_attached_dp(connector);
> > -	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > -	struct intel_encoder *intel_encoder = &intel_dig_port->base;
> >  	struct drm_device *dev = connector->dev;
> >  	enum drm_connector_status status;
> >  	u8 sink_irq_vector = 0;
> > @@ -4733,9 +4741,6 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
> >  		goto out;
> >  	}
> >  
> > -	if (intel_encoder->type != INTEL_OUTPUT_EDP)
> > -		intel_encoder->type = INTEL_OUTPUT_DP;
> > -
> >  	if (intel_dp->reset_link_params) {
> >  		/* Initial max link lane count */
> >  		intel_dp->max_link_lane_count = intel_dp_max_common_lane_count(intel_dp);
> > @@ -4852,9 +4857,6 @@ intel_dp_force(struct drm_connector *connector)
> >  	intel_dp_set_edid(intel_dp);
> >  
> >  	intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
> > -
> > -	if (intel_encoder->type != INTEL_OUTPUT_EDP)
> > -		intel_encoder->type = INTEL_OUTPUT_DP;
> >  }
> >  
> >  static int intel_dp_get_modes(struct drm_connector *connector)
> > @@ -5073,10 +5075,6 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
> >  	struct drm_i915_private *dev_priv = to_i915(dev);
> >  	enum irqreturn ret = IRQ_NONE;
> >  
> > -	if (intel_dig_port->base.type != INTEL_OUTPUT_EDP &&
> > -	    intel_dig_port->base.type != INTEL_OUTPUT_HDMI)
> > -		intel_dig_port->base.type = INTEL_OUTPUT_DP;
> > -
> >  	if (long_hpd && intel_dig_port->base.type == INTEL_OUTPUT_EDP) {
> >  		/*
> >  		 * vdd off can generate a long pulse on eDP which
> > diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
> > index 772521440a9f..9be6f32bb100 100644
> > --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> > +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> > @@ -270,6 +270,8 @@ static void intel_dp_mst_enc_get_config(struct intel_encoder *encoder,
> >  	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
> >  	u32 temp, flags = 0;
> >  
> > +	pipe_config->output_types |= BIT(INTEL_OUTPUT_DP_MST);
> > +
> >  	pipe_config->has_audio =
> >  		intel_ddi_is_audio_enabled(dev_priv, crtc);
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index a05ab3a1ab27..650129980ce2 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -173,7 +173,7 @@ enum intel_output_type {
> >  	INTEL_OUTPUT_DP = 7,
> >  	INTEL_OUTPUT_EDP = 8,
> >  	INTEL_OUTPUT_DSI = 9,
> > -	INTEL_OUTPUT_UNKNOWN = 10,
> > +	INTEL_OUTPUT_DDI = 10,
> >  	INTEL_OUTPUT_DP_MST = 11,
> >  };
> >  
> > @@ -216,6 +216,9 @@ struct intel_encoder {
> >  	enum port port;
> >  	unsigned int cloneable;
> >  	void (*hot_plug)(struct intel_encoder *);
> > +	enum intel_output_type (*compute_output_type)(struct intel_encoder *,
> > +						      struct intel_crtc_state *,
> > +						      struct drm_connector_state *);
> >  	bool (*compute_config)(struct intel_encoder *,
> >  			       struct intel_crtc_state *,
> >  			       struct drm_connector_state *);
> > @@ -1149,7 +1152,7 @@ enc_to_dig_port(struct drm_encoder *encoder)
> >  	struct intel_encoder *intel_encoder = to_intel_encoder(encoder);
> >  
> >  	switch (intel_encoder->type) {
> > -	case INTEL_OUTPUT_UNKNOWN:
> > +	case INTEL_OUTPUT_DDI:
> >  		WARN_ON(!HAS_DDI(to_i915(encoder->dev)));
> >  	case INTEL_OUTPUT_DP:
> >  	case INTEL_OUTPUT_EDP:
> > diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> > index 66bbedc5fa01..2ae3eea19317 100644
> > --- a/drivers/gpu/drm/i915/intel_dsi.c
> > +++ b/drivers/gpu/drm/i915/intel_dsi.c
> > @@ -1243,6 +1243,8 @@ static void intel_dsi_get_config(struct intel_encoder *encoder,
> >  	u32 pclk;
> >  	DRM_DEBUG_KMS("\n");
> >  
> > +	pipe_config->output_types |= BIT(INTEL_OUTPUT_DSI);
> > +
> >  	if (IS_GEN9_LP(dev_priv))
> >  		bxt_dsi_get_pipe_config(encoder, pipe_config);
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
> > index 53c9b763f4ce..754baa00bea9 100644
> > --- a/drivers/gpu/drm/i915/intel_dvo.c
> > +++ b/drivers/gpu/drm/i915/intel_dvo.c
> > @@ -159,6 +159,8 @@ static void intel_dvo_get_config(struct intel_encoder *encoder,
> >  	struct intel_dvo *intel_dvo = enc_to_dvo(encoder);
> >  	u32 tmp, flags = 0;
> >  
> > +	pipe_config->output_types |= BIT(INTEL_OUTPUT_DVO);
> > +
> >  	tmp = I915_READ(intel_dvo->dev.dvo_reg);
> >  	if (tmp & DVO_HSYNC_ACTIVE_HIGH)
> >  		flags |= DRM_MODE_FLAG_PHSYNC;
> > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> > index 5132dc814788..1ff448a67b99 100644
> > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > @@ -957,6 +957,8 @@ static void intel_hdmi_get_config(struct intel_encoder *encoder,
> >  	u32 tmp, flags = 0;
> >  	int dotclock;
> >  
> > +	pipe_config->output_types |= BIT(INTEL_OUTPUT_HDMI);
> > +
> >  	tmp = I915_READ(intel_hdmi->hdmi_reg);
> >  
> >  	if (tmp & SDVO_HSYNC_ACTIVE_HIGH)
> > @@ -1610,12 +1612,9 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
> >  
> >  	intel_hdmi_unset_edid(connector);
> >  
> > -	if (intel_hdmi_set_edid(connector)) {
> > -		struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
> > -
> > -		hdmi_to_dig_port(intel_hdmi)->base.type = INTEL_OUTPUT_HDMI;
> > +	if (intel_hdmi_set_edid(connector))
> >  		status = connector_status_connected;
> > -	} else
> > +	else
> >  		status = connector_status_disconnected;
> >  
> >  	intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS);
> > @@ -1626,8 +1625,6 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
> >  static void
> >  intel_hdmi_force(struct drm_connector *connector)
> >  {
> > -	struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
> > -
> >  	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
> >  		      connector->base.id, connector->name);
> >  
> > @@ -1637,7 +1634,6 @@ intel_hdmi_force(struct drm_connector *connector)
> >  		return;
> >  
> >  	intel_hdmi_set_edid(connector);
> > -	hdmi_to_dig_port(intel_hdmi)->base.type = INTEL_OUTPUT_HDMI;
> >  }
> >  
> >  static int intel_hdmi_get_modes(struct drm_connector *connector)
> > diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> > index 38572d65e46e..ef80499113ee 100644
> > --- a/drivers/gpu/drm/i915/intel_lvds.c
> > +++ b/drivers/gpu/drm/i915/intel_lvds.c
> > @@ -125,6 +125,8 @@ static void intel_lvds_get_config(struct intel_encoder *encoder,
> >  	struct intel_lvds_encoder *lvds_encoder = to_lvds_encoder(&encoder->base);
> >  	u32 tmp, flags = 0;
> >  
> > +	pipe_config->output_types |= BIT(INTEL_OUTPUT_LVDS);
> > +
> >  	tmp = I915_READ(lvds_encoder->reg);
> >  	if (tmp & LVDS_HSYNC_POLARITY)
> >  		flags |= DRM_MODE_FLAG_NHSYNC;
> > diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
> > index 1d946240e55f..39714be3eb6b 100644
> > --- a/drivers/gpu/drm/i915/intel_opregion.c
> > +++ b/drivers/gpu/drm/i915/intel_opregion.c
> > @@ -383,7 +383,7 @@ int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder,
> >  	case INTEL_OUTPUT_ANALOG:
> >  		type = DISPLAY_TYPE_CRT;
> >  		break;
> > -	case INTEL_OUTPUT_UNKNOWN:
> > +	case INTEL_OUTPUT_DDI:
> >  	case INTEL_OUTPUT_DP:
> >  	case INTEL_OUTPUT_HDMI:
> >  	case INTEL_OUTPUT_DP_MST:
> > diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> > index 7437944b388f..42ec2d1f7a61 100644
> > --- a/drivers/gpu/drm/i915/intel_sdvo.c
> > +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> > @@ -1429,6 +1429,8 @@ static void intel_sdvo_get_config(struct intel_encoder *encoder,
> >  	u8 val;
> >  	bool ret;
> >  
> > +	pipe_config->output_types |= BIT(INTEL_OUTPUT_SDVO);
> > +
> >  	sdvox = I915_READ(intel_sdvo->sdvo_reg);
> >  
> >  	ret = intel_sdvo_get_input_timing(intel_sdvo, &dtd);
> > diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
> > index a79a7591b2cf..b18609cebe03 100644
> > --- a/drivers/gpu/drm/i915/intel_tv.c
> > +++ b/drivers/gpu/drm/i915/intel_tv.c
> > @@ -868,6 +868,8 @@ static void
> >  intel_tv_get_config(struct intel_encoder *encoder,
> >  		    struct intel_crtc_state *pipe_config)
> >  {
> > +	pipe_config->output_types |= BIT(INTEL_OUTPUT_TVOUT);
> > +
> >  	pipe_config->base.adjusted_mode.crtc_clock = pipe_config->port_clock;
> >  }
> >  
> 

-- 
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] 27+ messages in thread

* Re: [PATCH v5 05/10] drm/i915: Stop frobbing with DDI encoder->type
  2017-10-27 12:44     ` Ville Syrjälä
@ 2017-10-27 13:53       ` Maarten Lankhorst
  2017-10-27 14:03         ` Ville Syrjälä
  0 siblings, 1 reply; 27+ messages in thread
From: Maarten Lankhorst @ 2017-10-27 13:53 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

Op 27-10-17 om 14:44 schreef Ville Syrjälä:
> On Fri, Oct 27, 2017 at 02:05:38PM +0200, Maarten Lankhorst wrote:
>> Op 19-10-17 om 15:37 schreef Ville Syrjala:
>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>
>>> Currently the DDI encoder->type will change at runtime depending on
>>> what kind of hotplugs we've processed. That's quite bad since we can't
>>> really trust that that current value of encoder->type actually matches
>>> the type of signal we're trying to drive through it.
>>>
>>> Let's eliminate that problem by declaring that non-eDP DDI port will
>>> always have the encoder type as INTEL_OUTPUT_DDI. This means the code
>>> can no longer try to distinguish DP vs. HDMI based on encoder->type.
>>> We'll leave eDP as INTEL_OUTPUT_EDP, since it'll never change and
>>> there's a bunch of code that relies on that value to identify eDP
>>> encoders.
>>>
>>> We'll introduce a new encoder .compute_output_type() hook. This allows
>>> us to compute the full output_types before any encoder .compute_config()
>>> hooks get called, thus those hooks can rely on output_types being
>>> correct, which is useful for cloning on oldr platforms. For now we'll
>>> just look at the connector type and pick the correct mode based on that.
>>> In the future the new hook could be used to implement dynamic switching
>>> between LS and PCON modes for LSPCON.
>>>
>>> TODO: maybe make .get_config() populate output_types rather than doing
>>> the default encoder->type thing in caller and then undoing it for
>>> DDI in .get_config().
>>>
>>> v2: Fix BXT/GLK PPS explosion with DSI/MST encoders
>>> v3: Avoid the PPS warn on pure HDMI/DVI DDI encoders by checking dp.output_reg
>>> v4: Rebase
>>> v5: Populate output_types in .get_config() rather than in the caller
>> Could we get rid of compute_output_type in this patch? Perhaps do it as preliminary
>> patch towards removing the use?
> We don't want to remove it.
Ok, but could that still be a separate preparation patch? That would make it easier to review. :)
>> Rest of the series except patch 8 looks good. For patch 1-4, 6, 7, 9 (if the delta
>> between ddi_get_config and old mst get_config is harmless or beneficial) and 10:
>>
>> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>
>> Thanks for the work towards cleaning this mess up.
>>
>> ~Maarten
>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> ---
>>>  drivers/gpu/drm/i915/i915_debugfs.c   |  2 +-
>>>  drivers/gpu/drm/i915/intel_crt.c      |  2 ++
>>>  drivers/gpu/drm/i915/intel_ddi.c      | 43 ++++++++++++++++++++++++++++-------
>>>  drivers/gpu/drm/i915/intel_display.c  | 16 +++++++------
>>>  drivers/gpu/drm/i915/intel_dp.c       | 24 +++++++++----------
>>>  drivers/gpu/drm/i915/intel_dp_mst.c   |  2 ++
>>>  drivers/gpu/drm/i915/intel_drv.h      |  7 ++++--
>>>  drivers/gpu/drm/i915/intel_dsi.c      |  2 ++
>>>  drivers/gpu/drm/i915/intel_dvo.c      |  2 ++
>>>  drivers/gpu/drm/i915/intel_hdmi.c     | 12 ++++------
>>>  drivers/gpu/drm/i915/intel_lvds.c     |  2 ++
>>>  drivers/gpu/drm/i915/intel_opregion.c |  2 +-
>>>  drivers/gpu/drm/i915/intel_sdvo.c     |  2 ++
>>>  drivers/gpu/drm/i915/intel_tv.c       |  2 ++
>>>  14 files changed, 80 insertions(+), 40 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
>>> index c65e381b85f3..e5333bfdf32d 100644
>>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>>> @@ -3049,7 +3049,7 @@ static void intel_connector_info(struct seq_file *m,
>>>  		break;
>>>  	case DRM_MODE_CONNECTOR_HDMIA:
>>>  		if (intel_encoder->type == INTEL_OUTPUT_HDMI ||
>>> -		    intel_encoder->type == INTEL_OUTPUT_UNKNOWN)
>>> +		    intel_encoder->type == INTEL_OUTPUT_DDI)
>>>  			intel_hdmi_info(m, intel_connector);
>>>  		break;
>>>  	default:
>>> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
>>> index 668e8c3e791d..a61e61efe9aa 100644
>>> --- a/drivers/gpu/drm/i915/intel_crt.c
>>> +++ b/drivers/gpu/drm/i915/intel_crt.c
>>> @@ -119,6 +119,8 @@ static unsigned int intel_crt_get_flags(struct intel_encoder *encoder)
>>>  static void intel_crt_get_config(struct intel_encoder *encoder,
>>>  				 struct intel_crtc_state *pipe_config)
>>>  {
>>> +	pipe_config->output_types |= BIT(INTEL_OUTPUT_ANALOG);
>>> +
>>>  	pipe_config->base.adjusted_mode.flags |= intel_crt_get_flags(encoder);
>>>  
>>>  	pipe_config->base.adjusted_mode.crtc_clock = pipe_config->port_clock;
>>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
>>> index c8790bb74839..630609575db4 100644
>>> --- a/drivers/gpu/drm/i915/intel_ddi.c
>>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
>>> @@ -497,10 +497,8 @@ enum port intel_ddi_get_encoder_port(struct intel_encoder *encoder)
>>>  	switch (encoder->type) {
>>>  	case INTEL_OUTPUT_DP_MST:
>>>  		return enc_to_mst(&encoder->base)->primary->port;
>>> -	case INTEL_OUTPUT_DP:
>>>  	case INTEL_OUTPUT_EDP:
>>> -	case INTEL_OUTPUT_HDMI:
>>> -	case INTEL_OUTPUT_UNKNOWN:
>>> +	case INTEL_OUTPUT_DDI:
>>>  		return enc_to_dig_port(&encoder->base)->port;
>>>  	case INTEL_OUTPUT_ANALOG:
>>>  		return PORT_E;
>>> @@ -1532,6 +1530,7 @@ void intel_ddi_set_vc_payload_alloc(const struct intel_crtc_state *crtc_state,
>>>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>>>  	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>>>  	uint32_t temp;
>>> +
>>>  	temp = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
>>>  	if (state == true)
>>>  		temp |= TRANS_DDI_DP_VC_PAYLOAD_ALLOC;
>>> @@ -2586,12 +2585,23 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
>>>  			pipe_config->hdmi_high_tmds_clock_ratio = true;
>>>  		/* fall through */
>>>  	case TRANS_DDI_MODE_SELECT_DVI:
>>> +		pipe_config->output_types |= BIT(INTEL_OUTPUT_HDMI);
>>>  		pipe_config->lane_count = 4;
>>>  		break;
>>>  	case TRANS_DDI_MODE_SELECT_FDI:
>>> +		pipe_config->output_types |= BIT(INTEL_OUTPUT_ANALOG);
>>>  		break;
>>>  	case TRANS_DDI_MODE_SELECT_DP_SST:
>>> +		if (encoder->type == INTEL_OUTPUT_EDP)
>>> +			pipe_config->output_types |= BIT(INTEL_OUTPUT_EDP);
>>> +		else
>>> +			pipe_config->output_types |= BIT(INTEL_OUTPUT_DP);
>>> +		pipe_config->lane_count =
>>> +			((temp & DDI_PORT_WIDTH_MASK) >> DDI_PORT_WIDTH_SHIFT) + 1;
>>> +		intel_dp_get_m_n(intel_crtc, pipe_config);
>>> +		break;
>>>  	case TRANS_DDI_MODE_SELECT_DP_MST:
>>> +		pipe_config->output_types |= BIT(INTEL_OUTPUT_DP_MST);
>>>  		pipe_config->lane_count =
>>>  			((temp & DDI_PORT_WIDTH_MASK) >> DDI_PORT_WIDTH_SHIFT) + 1;
>>>  		intel_dp_get_m_n(intel_crtc, pipe_config);
>>> @@ -2630,21 +2640,36 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
>>>  			bxt_ddi_phy_get_lane_lat_optim_mask(encoder);
>>>  }
>>>  
>>> +static enum intel_output_type
>>> +intel_ddi_compute_output_type(struct intel_encoder *encoder,
>>> +			      struct intel_crtc_state *crtc_state,
>>> +			      struct drm_connector_state *conn_state)
>>> +{
>>> +	switch (conn_state->connector->connector_type) {
>>> +	case DRM_MODE_CONNECTOR_HDMIA:
>>> +		return INTEL_OUTPUT_HDMI;
>>> +	case DRM_MODE_CONNECTOR_eDP:
>>> +		return INTEL_OUTPUT_EDP;
>>> +	case DRM_MODE_CONNECTOR_DisplayPort:
>>> +		return INTEL_OUTPUT_DP;
>>> +	default:
>>> +		MISSING_CASE(conn_state->connector->connector_type);
>>> +		return INTEL_OUTPUT_UNUSED;
>>> +	}
>>> +}
>>> +
>>>  static bool intel_ddi_compute_config(struct intel_encoder *encoder,
>>>  				     struct intel_crtc_state *pipe_config,
>>>  				     struct drm_connector_state *conn_state)
>>>  {
>>>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>>> -	int type = encoder->type;
>>>  	int port = intel_ddi_get_encoder_port(encoder);
>>>  	int ret;
>>>  
>>> -	WARN(type == INTEL_OUTPUT_UNKNOWN, "compute_config() on unknown output!\n");
>>> -
>>>  	if (port == PORT_A)
>>>  		pipe_config->cpu_transcoder = TRANSCODER_EDP;
>>>  
>>> -	if (type == INTEL_OUTPUT_HDMI)
>>> +	if (intel_crtc_has_type(pipe_config, INTEL_OUTPUT_HDMI))
>>>  		ret = intel_hdmi_compute_config(encoder, pipe_config, conn_state);
>>>  	else
>>>  		ret = intel_dp_compute_config(encoder, pipe_config, conn_state);
>>> @@ -2764,6 +2789,7 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>>>  	drm_encoder_init(&dev_priv->drm, encoder, &intel_ddi_funcs,
>>>  			 DRM_MODE_ENCODER_TMDS, "DDI %c", port_name(port));
>>>  
>>> +	intel_encoder->compute_output_type = intel_ddi_compute_output_type;
>>>  	intel_encoder->compute_config = intel_ddi_compute_config;
>>>  	intel_encoder->enable = intel_enable_ddi;
>>>  	if (IS_GEN9_LP(dev_priv))
>>> @@ -2821,9 +2847,10 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>>>  		}
>>>  	}
>>>  
>>> +	intel_dig_port->dp.output_reg = INVALID_MMIO_REG;
>>>  	intel_dig_port->max_lanes = max_lanes;
>>>  
>>> -	intel_encoder->type = INTEL_OUTPUT_UNKNOWN;
>>> +	intel_encoder->type = INTEL_OUTPUT_DDI;
>>>  	intel_encoder->power_domain = intel_port_to_power_domain(port);
>>>  	intel_encoder->port = port;
>>>  	intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
>>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>>> index bd62c0a65bcd..a7e02ae33583 100644
>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>> @@ -10591,7 +10591,7 @@ static const char * const output_type_str[] = {
>>>  	OUTPUT_TYPE(DP),
>>>  	OUTPUT_TYPE(EDP),
>>>  	OUTPUT_TYPE(DSI),
>>> -	OUTPUT_TYPE(UNKNOWN),
>>> +	OUTPUT_TYPE(DDI),
>>>  	OUTPUT_TYPE(DP_MST),
>>>  };
>>>  
>>> @@ -10762,7 +10762,7 @@ static bool check_digital_port_conflicts(struct drm_atomic_state *state)
>>>  
>>>  		switch (encoder->type) {
>>>  			unsigned int port_mask;
>>> -		case INTEL_OUTPUT_UNKNOWN:
>>> +		case INTEL_OUTPUT_DDI:
>>>  			if (WARN_ON(!HAS_DDI(to_i915(dev))))
>>>  				break;
>>>  		case INTEL_OUTPUT_DP:
>>> @@ -10895,7 +10895,12 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
>>>  		 * Determine output_types before calling the .compute_config()
>>>  		 * hooks so that the hooks can use this information safely.
>>>  		 */
>>> -		pipe_config->output_types |= 1 << encoder->type;
>>> +		if (encoder->compute_output_type)
>>> +			pipe_config->output_types |=
>>> +				BIT(encoder->compute_output_type(encoder, pipe_config,
>>> +								 connector_state));
>>> +		else
>>> +			pipe_config->output_types |= BIT(encoder->type);
>>>  	}
>>>  
>>>  encoder_retry:
>>> @@ -11572,10 +11577,8 @@ verify_crtc_state(struct drm_crtc *crtc,
>>>  				"Encoder connected to wrong pipe %c\n",
>>>  				pipe_name(pipe));
>>>  
>>> -		if (active) {
>>> -			pipe_config->output_types |= 1 << encoder->type;
>>> +		if (active)
>>>  			encoder->get_config(encoder, pipe_config);
>>> -		}
>>>  	}
>>>  
>>>  	intel_crtc_compute_pixel_rate(pipe_config);
>>> @@ -14963,7 +14966,6 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>>>  			crtc_state = to_intel_crtc_state(crtc->base.state);
>>>  
>>>  			encoder->base.crtc = &crtc->base;
>>> -			crtc_state->output_types |= 1 << encoder->type;
>>>  			encoder->get_config(encoder, crtc_state);
>>>  		} else {
>>>  			encoder->base.crtc = NULL;
>>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>>> index aa75f55eeb61..2b482d12000e 100644
>>> --- a/drivers/gpu/drm/i915/intel_dp.c
>>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>>> @@ -758,11 +758,16 @@ void intel_power_sequencer_reset(struct drm_i915_private *dev_priv)
>>>  		struct intel_dp *intel_dp;
>>>  
>>>  		if (encoder->type != INTEL_OUTPUT_DP &&
>>> -		    encoder->type != INTEL_OUTPUT_EDP)
>>> +		    encoder->type != INTEL_OUTPUT_EDP &&
>>> +		    encoder->type != INTEL_OUTPUT_DDI)
>>>  			continue;
>>>  
>>>  		intel_dp = enc_to_intel_dp(&encoder->base);
>>>  
>>> +		/* Skip pure DVI/HDMI DDI encoders */
>>> +		if (!i915_mmio_reg_valid(intel_dp->output_reg))
>>> +			continue;
>>> +
>>>  		WARN_ON(intel_dp->active_pipe != INVALID_PIPE);
>>>  
>>>  		if (encoder->type != INTEL_OUTPUT_EDP)
>>> @@ -2603,6 +2608,11 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
>>>  	enum port port = dp_to_dig_port(intel_dp)->port;
>>>  	struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
>>>  
>>> +	if (encoder->type == INTEL_OUTPUT_EDP)
>>> +		pipe_config->output_types |= BIT(INTEL_OUTPUT_EDP);
>>> +	else
>>> +		pipe_config->output_types |= BIT(INTEL_OUTPUT_DP);
>>> +
>>>  	tmp = I915_READ(intel_dp->output_reg);
>>>  
>>>  	pipe_config->has_audio = tmp & DP_AUDIO_OUTPUT_ENABLE && port != PORT_A;
>>> @@ -4699,8 +4709,6 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
>>>  {
>>>  	struct drm_connector *connector = &intel_connector->base;
>>>  	struct intel_dp *intel_dp = intel_attached_dp(connector);
>>> -	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>>> -	struct intel_encoder *intel_encoder = &intel_dig_port->base;
>>>  	struct drm_device *dev = connector->dev;
>>>  	enum drm_connector_status status;
>>>  	u8 sink_irq_vector = 0;
>>> @@ -4733,9 +4741,6 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
>>>  		goto out;
>>>  	}
>>>  
>>> -	if (intel_encoder->type != INTEL_OUTPUT_EDP)
>>> -		intel_encoder->type = INTEL_OUTPUT_DP;
>>> -
>>>  	if (intel_dp->reset_link_params) {
>>>  		/* Initial max link lane count */
>>>  		intel_dp->max_link_lane_count = intel_dp_max_common_lane_count(intel_dp);
>>> @@ -4852,9 +4857,6 @@ intel_dp_force(struct drm_connector *connector)
>>>  	intel_dp_set_edid(intel_dp);
>>>  
>>>  	intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
>>> -
>>> -	if (intel_encoder->type != INTEL_OUTPUT_EDP)
>>> -		intel_encoder->type = INTEL_OUTPUT_DP;
>>>  }
>>>  
>>>  static int intel_dp_get_modes(struct drm_connector *connector)
>>> @@ -5073,10 +5075,6 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
>>>  	struct drm_i915_private *dev_priv = to_i915(dev);
>>>  	enum irqreturn ret = IRQ_NONE;
>>>  
>>> -	if (intel_dig_port->base.type != INTEL_OUTPUT_EDP &&
>>> -	    intel_dig_port->base.type != INTEL_OUTPUT_HDMI)
>>> -		intel_dig_port->base.type = INTEL_OUTPUT_DP;
>>> -
>>>  	if (long_hpd && intel_dig_port->base.type == INTEL_OUTPUT_EDP) {
>>>  		/*
>>>  		 * vdd off can generate a long pulse on eDP which
>>> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
>>> index 772521440a9f..9be6f32bb100 100644
>>> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
>>> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
>>> @@ -270,6 +270,8 @@ static void intel_dp_mst_enc_get_config(struct intel_encoder *encoder,
>>>  	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
>>>  	u32 temp, flags = 0;
>>>  
>>> +	pipe_config->output_types |= BIT(INTEL_OUTPUT_DP_MST);
>>> +
>>>  	pipe_config->has_audio =
>>>  		intel_ddi_is_audio_enabled(dev_priv, crtc);
>>>  
>>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>>> index a05ab3a1ab27..650129980ce2 100644
>>> --- a/drivers/gpu/drm/i915/intel_drv.h
>>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>>> @@ -173,7 +173,7 @@ enum intel_output_type {
>>>  	INTEL_OUTPUT_DP = 7,
>>>  	INTEL_OUTPUT_EDP = 8,
>>>  	INTEL_OUTPUT_DSI = 9,
>>> -	INTEL_OUTPUT_UNKNOWN = 10,
>>> +	INTEL_OUTPUT_DDI = 10,
>>>  	INTEL_OUTPUT_DP_MST = 11,
>>>  };
>>>  
>>> @@ -216,6 +216,9 @@ struct intel_encoder {
>>>  	enum port port;
>>>  	unsigned int cloneable;
>>>  	void (*hot_plug)(struct intel_encoder *);
>>> +	enum intel_output_type (*compute_output_type)(struct intel_encoder *,
>>> +						      struct intel_crtc_state *,
>>> +						      struct drm_connector_state *);
>>>  	bool (*compute_config)(struct intel_encoder *,
>>>  			       struct intel_crtc_state *,
>>>  			       struct drm_connector_state *);
>>> @@ -1149,7 +1152,7 @@ enc_to_dig_port(struct drm_encoder *encoder)
>>>  	struct intel_encoder *intel_encoder = to_intel_encoder(encoder);
>>>  
>>>  	switch (intel_encoder->type) {
>>> -	case INTEL_OUTPUT_UNKNOWN:
>>> +	case INTEL_OUTPUT_DDI:
>>>  		WARN_ON(!HAS_DDI(to_i915(encoder->dev)));
>>>  	case INTEL_OUTPUT_DP:
>>>  	case INTEL_OUTPUT_EDP:
>>> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
>>> index 66bbedc5fa01..2ae3eea19317 100644
>>> --- a/drivers/gpu/drm/i915/intel_dsi.c
>>> +++ b/drivers/gpu/drm/i915/intel_dsi.c
>>> @@ -1243,6 +1243,8 @@ static void intel_dsi_get_config(struct intel_encoder *encoder,
>>>  	u32 pclk;
>>>  	DRM_DEBUG_KMS("\n");
>>>  
>>> +	pipe_config->output_types |= BIT(INTEL_OUTPUT_DSI);
>>> +
>>>  	if (IS_GEN9_LP(dev_priv))
>>>  		bxt_dsi_get_pipe_config(encoder, pipe_config);
>>>  
>>> diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
>>> index 53c9b763f4ce..754baa00bea9 100644
>>> --- a/drivers/gpu/drm/i915/intel_dvo.c
>>> +++ b/drivers/gpu/drm/i915/intel_dvo.c
>>> @@ -159,6 +159,8 @@ static void intel_dvo_get_config(struct intel_encoder *encoder,
>>>  	struct intel_dvo *intel_dvo = enc_to_dvo(encoder);
>>>  	u32 tmp, flags = 0;
>>>  
>>> +	pipe_config->output_types |= BIT(INTEL_OUTPUT_DVO);
>>> +
>>>  	tmp = I915_READ(intel_dvo->dev.dvo_reg);
>>>  	if (tmp & DVO_HSYNC_ACTIVE_HIGH)
>>>  		flags |= DRM_MODE_FLAG_PHSYNC;
>>> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
>>> index 5132dc814788..1ff448a67b99 100644
>>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
>>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
>>> @@ -957,6 +957,8 @@ static void intel_hdmi_get_config(struct intel_encoder *encoder,
>>>  	u32 tmp, flags = 0;
>>>  	int dotclock;
>>>  
>>> +	pipe_config->output_types |= BIT(INTEL_OUTPUT_HDMI);
>>> +
>>>  	tmp = I915_READ(intel_hdmi->hdmi_reg);
>>>  
>>>  	if (tmp & SDVO_HSYNC_ACTIVE_HIGH)
>>> @@ -1610,12 +1612,9 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
>>>  
>>>  	intel_hdmi_unset_edid(connector);
>>>  
>>> -	if (intel_hdmi_set_edid(connector)) {
>>> -		struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
>>> -
>>> -		hdmi_to_dig_port(intel_hdmi)->base.type = INTEL_OUTPUT_HDMI;
>>> +	if (intel_hdmi_set_edid(connector))
>>>  		status = connector_status_connected;
>>> -	} else
>>> +	else
>>>  		status = connector_status_disconnected;
>>>  
>>>  	intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS);
>>> @@ -1626,8 +1625,6 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
>>>  static void
>>>  intel_hdmi_force(struct drm_connector *connector)
>>>  {
>>> -	struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
>>> -
>>>  	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
>>>  		      connector->base.id, connector->name);
>>>  
>>> @@ -1637,7 +1634,6 @@ intel_hdmi_force(struct drm_connector *connector)
>>>  		return;
>>>  
>>>  	intel_hdmi_set_edid(connector);
>>> -	hdmi_to_dig_port(intel_hdmi)->base.type = INTEL_OUTPUT_HDMI;
>>>  }
>>>  
>>>  static int intel_hdmi_get_modes(struct drm_connector *connector)
>>> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
>>> index 38572d65e46e..ef80499113ee 100644
>>> --- a/drivers/gpu/drm/i915/intel_lvds.c
>>> +++ b/drivers/gpu/drm/i915/intel_lvds.c
>>> @@ -125,6 +125,8 @@ static void intel_lvds_get_config(struct intel_encoder *encoder,
>>>  	struct intel_lvds_encoder *lvds_encoder = to_lvds_encoder(&encoder->base);
>>>  	u32 tmp, flags = 0;
>>>  
>>> +	pipe_config->output_types |= BIT(INTEL_OUTPUT_LVDS);
>>> +
>>>  	tmp = I915_READ(lvds_encoder->reg);
>>>  	if (tmp & LVDS_HSYNC_POLARITY)
>>>  		flags |= DRM_MODE_FLAG_NHSYNC;
>>> diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
>>> index 1d946240e55f..39714be3eb6b 100644
>>> --- a/drivers/gpu/drm/i915/intel_opregion.c
>>> +++ b/drivers/gpu/drm/i915/intel_opregion.c
>>> @@ -383,7 +383,7 @@ int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder,
>>>  	case INTEL_OUTPUT_ANALOG:
>>>  		type = DISPLAY_TYPE_CRT;
>>>  		break;
>>> -	case INTEL_OUTPUT_UNKNOWN:
>>> +	case INTEL_OUTPUT_DDI:
>>>  	case INTEL_OUTPUT_DP:
>>>  	case INTEL_OUTPUT_HDMI:
>>>  	case INTEL_OUTPUT_DP_MST:
>>> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
>>> index 7437944b388f..42ec2d1f7a61 100644
>>> --- a/drivers/gpu/drm/i915/intel_sdvo.c
>>> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
>>> @@ -1429,6 +1429,8 @@ static void intel_sdvo_get_config(struct intel_encoder *encoder,
>>>  	u8 val;
>>>  	bool ret;
>>>  
>>> +	pipe_config->output_types |= BIT(INTEL_OUTPUT_SDVO);
>>> +
>>>  	sdvox = I915_READ(intel_sdvo->sdvo_reg);
>>>  
>>>  	ret = intel_sdvo_get_input_timing(intel_sdvo, &dtd);
>>> diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
>>> index a79a7591b2cf..b18609cebe03 100644
>>> --- a/drivers/gpu/drm/i915/intel_tv.c
>>> +++ b/drivers/gpu/drm/i915/intel_tv.c
>>> @@ -868,6 +868,8 @@ static void
>>>  intel_tv_get_config(struct intel_encoder *encoder,
>>>  		    struct intel_crtc_state *pipe_config)
>>>  {
>>> +	pipe_config->output_types |= BIT(INTEL_OUTPUT_TVOUT);
>>> +
>>>  	pipe_config->base.adjusted_mode.crtc_clock = pipe_config->port_clock;
>>>  }
>>>  


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

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

* Re: [PATCH v5 05/10] drm/i915: Stop frobbing with DDI encoder->type
  2017-10-27 13:53       ` Maarten Lankhorst
@ 2017-10-27 14:03         ` Ville Syrjälä
  2017-10-27 14:05           ` Maarten Lankhorst
  0 siblings, 1 reply; 27+ messages in thread
From: Ville Syrjälä @ 2017-10-27 14:03 UTC (permalink / raw)
  To: Maarten Lankhorst; +Cc: intel-gfx

On Fri, Oct 27, 2017 at 03:53:34PM +0200, Maarten Lankhorst wrote:
> Op 27-10-17 om 14:44 schreef Ville Syrjälä:
> > On Fri, Oct 27, 2017 at 02:05:38PM +0200, Maarten Lankhorst wrote:
> >> Op 19-10-17 om 15:37 schreef Ville Syrjala:
> >>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>>
> >>> Currently the DDI encoder->type will change at runtime depending on
> >>> what kind of hotplugs we've processed. That's quite bad since we can't
> >>> really trust that that current value of encoder->type actually matches
> >>> the type of signal we're trying to drive through it.
> >>>
> >>> Let's eliminate that problem by declaring that non-eDP DDI port will
> >>> always have the encoder type as INTEL_OUTPUT_DDI. This means the code
> >>> can no longer try to distinguish DP vs. HDMI based on encoder->type.
> >>> We'll leave eDP as INTEL_OUTPUT_EDP, since it'll never change and
> >>> there's a bunch of code that relies on that value to identify eDP
> >>> encoders.
> >>>
> >>> We'll introduce a new encoder .compute_output_type() hook. This allows
> >>> us to compute the full output_types before any encoder .compute_config()
> >>> hooks get called, thus those hooks can rely on output_types being
> >>> correct, which is useful for cloning on oldr platforms. For now we'll
> >>> just look at the connector type and pick the correct mode based on that.
> >>> In the future the new hook could be used to implement dynamic switching
> >>> between LS and PCON modes for LSPCON.
> >>>
> >>> TODO: maybe make .get_config() populate output_types rather than doing
> >>> the default encoder->type thing in caller and then undoing it for
> >>> DDI in .get_config().
> >>>
> >>> v2: Fix BXT/GLK PPS explosion with DSI/MST encoders
> >>> v3: Avoid the PPS warn on pure HDMI/DVI DDI encoders by checking dp.output_reg
> >>> v4: Rebase
> >>> v5: Populate output_types in .get_config() rather than in the caller
> >> Could we get rid of compute_output_type in this patch? Perhaps do it as preliminary
> >> patch towards removing the use?
> > We don't want to remove it.
> Ok, but could that still be a separate preparation patch? That would make it easier to review. :)

Not sure which part you mean here. Moving the 'output_types |= whatever'
into .get_config()? Hmm. Yeah that should be easy to split out. I guess
it won't even make the DDI case any more broken that it already is since
even currently encoder->type might change between the modeset compute
phase and .get_config(). And it's going to be followed closely by the
actual fix anyway.

> >> Rest of the series except patch 8 looks good. For patch 1-4, 6, 7, 9 (if the delta
> >> between ddi_get_config and old mst get_config is harmless or beneficial) and 10:
> >>
> >> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >>
> >> Thanks for the work towards cleaning this mess up.
> >>
> >> ~Maarten
> >>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>> ---
> >>>  drivers/gpu/drm/i915/i915_debugfs.c   |  2 +-
> >>>  drivers/gpu/drm/i915/intel_crt.c      |  2 ++
> >>>  drivers/gpu/drm/i915/intel_ddi.c      | 43 ++++++++++++++++++++++++++++-------
> >>>  drivers/gpu/drm/i915/intel_display.c  | 16 +++++++------
> >>>  drivers/gpu/drm/i915/intel_dp.c       | 24 +++++++++----------
> >>>  drivers/gpu/drm/i915/intel_dp_mst.c   |  2 ++
> >>>  drivers/gpu/drm/i915/intel_drv.h      |  7 ++++--
> >>>  drivers/gpu/drm/i915/intel_dsi.c      |  2 ++
> >>>  drivers/gpu/drm/i915/intel_dvo.c      |  2 ++
> >>>  drivers/gpu/drm/i915/intel_hdmi.c     | 12 ++++------
> >>>  drivers/gpu/drm/i915/intel_lvds.c     |  2 ++
> >>>  drivers/gpu/drm/i915/intel_opregion.c |  2 +-
> >>>  drivers/gpu/drm/i915/intel_sdvo.c     |  2 ++
> >>>  drivers/gpu/drm/i915/intel_tv.c       |  2 ++
> >>>  14 files changed, 80 insertions(+), 40 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> >>> index c65e381b85f3..e5333bfdf32d 100644
> >>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> >>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> >>> @@ -3049,7 +3049,7 @@ static void intel_connector_info(struct seq_file *m,
> >>>  		break;
> >>>  	case DRM_MODE_CONNECTOR_HDMIA:
> >>>  		if (intel_encoder->type == INTEL_OUTPUT_HDMI ||
> >>> -		    intel_encoder->type == INTEL_OUTPUT_UNKNOWN)
> >>> +		    intel_encoder->type == INTEL_OUTPUT_DDI)
> >>>  			intel_hdmi_info(m, intel_connector);
> >>>  		break;
> >>>  	default:
> >>> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> >>> index 668e8c3e791d..a61e61efe9aa 100644
> >>> --- a/drivers/gpu/drm/i915/intel_crt.c
> >>> +++ b/drivers/gpu/drm/i915/intel_crt.c
> >>> @@ -119,6 +119,8 @@ static unsigned int intel_crt_get_flags(struct intel_encoder *encoder)
> >>>  static void intel_crt_get_config(struct intel_encoder *encoder,
> >>>  				 struct intel_crtc_state *pipe_config)
> >>>  {
> >>> +	pipe_config->output_types |= BIT(INTEL_OUTPUT_ANALOG);
> >>> +
> >>>  	pipe_config->base.adjusted_mode.flags |= intel_crt_get_flags(encoder);
> >>>  
> >>>  	pipe_config->base.adjusted_mode.crtc_clock = pipe_config->port_clock;
> >>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> >>> index c8790bb74839..630609575db4 100644
> >>> --- a/drivers/gpu/drm/i915/intel_ddi.c
> >>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> >>> @@ -497,10 +497,8 @@ enum port intel_ddi_get_encoder_port(struct intel_encoder *encoder)
> >>>  	switch (encoder->type) {
> >>>  	case INTEL_OUTPUT_DP_MST:
> >>>  		return enc_to_mst(&encoder->base)->primary->port;
> >>> -	case INTEL_OUTPUT_DP:
> >>>  	case INTEL_OUTPUT_EDP:
> >>> -	case INTEL_OUTPUT_HDMI:
> >>> -	case INTEL_OUTPUT_UNKNOWN:
> >>> +	case INTEL_OUTPUT_DDI:
> >>>  		return enc_to_dig_port(&encoder->base)->port;
> >>>  	case INTEL_OUTPUT_ANALOG:
> >>>  		return PORT_E;
> >>> @@ -1532,6 +1530,7 @@ void intel_ddi_set_vc_payload_alloc(const struct intel_crtc_state *crtc_state,
> >>>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> >>>  	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> >>>  	uint32_t temp;
> >>> +
> >>>  	temp = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
> >>>  	if (state == true)
> >>>  		temp |= TRANS_DDI_DP_VC_PAYLOAD_ALLOC;
> >>> @@ -2586,12 +2585,23 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
> >>>  			pipe_config->hdmi_high_tmds_clock_ratio = true;
> >>>  		/* fall through */
> >>>  	case TRANS_DDI_MODE_SELECT_DVI:
> >>> +		pipe_config->output_types |= BIT(INTEL_OUTPUT_HDMI);
> >>>  		pipe_config->lane_count = 4;
> >>>  		break;
> >>>  	case TRANS_DDI_MODE_SELECT_FDI:
> >>> +		pipe_config->output_types |= BIT(INTEL_OUTPUT_ANALOG);
> >>>  		break;
> >>>  	case TRANS_DDI_MODE_SELECT_DP_SST:
> >>> +		if (encoder->type == INTEL_OUTPUT_EDP)
> >>> +			pipe_config->output_types |= BIT(INTEL_OUTPUT_EDP);
> >>> +		else
> >>> +			pipe_config->output_types |= BIT(INTEL_OUTPUT_DP);
> >>> +		pipe_config->lane_count =
> >>> +			((temp & DDI_PORT_WIDTH_MASK) >> DDI_PORT_WIDTH_SHIFT) + 1;
> >>> +		intel_dp_get_m_n(intel_crtc, pipe_config);
> >>> +		break;
> >>>  	case TRANS_DDI_MODE_SELECT_DP_MST:
> >>> +		pipe_config->output_types |= BIT(INTEL_OUTPUT_DP_MST);
> >>>  		pipe_config->lane_count =
> >>>  			((temp & DDI_PORT_WIDTH_MASK) >> DDI_PORT_WIDTH_SHIFT) + 1;
> >>>  		intel_dp_get_m_n(intel_crtc, pipe_config);
> >>> @@ -2630,21 +2640,36 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
> >>>  			bxt_ddi_phy_get_lane_lat_optim_mask(encoder);
> >>>  }
> >>>  
> >>> +static enum intel_output_type
> >>> +intel_ddi_compute_output_type(struct intel_encoder *encoder,
> >>> +			      struct intel_crtc_state *crtc_state,
> >>> +			      struct drm_connector_state *conn_state)
> >>> +{
> >>> +	switch (conn_state->connector->connector_type) {
> >>> +	case DRM_MODE_CONNECTOR_HDMIA:
> >>> +		return INTEL_OUTPUT_HDMI;
> >>> +	case DRM_MODE_CONNECTOR_eDP:
> >>> +		return INTEL_OUTPUT_EDP;
> >>> +	case DRM_MODE_CONNECTOR_DisplayPort:
> >>> +		return INTEL_OUTPUT_DP;
> >>> +	default:
> >>> +		MISSING_CASE(conn_state->connector->connector_type);
> >>> +		return INTEL_OUTPUT_UNUSED;
> >>> +	}
> >>> +}
> >>> +
> >>>  static bool intel_ddi_compute_config(struct intel_encoder *encoder,
> >>>  				     struct intel_crtc_state *pipe_config,
> >>>  				     struct drm_connector_state *conn_state)
> >>>  {
> >>>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> >>> -	int type = encoder->type;
> >>>  	int port = intel_ddi_get_encoder_port(encoder);
> >>>  	int ret;
> >>>  
> >>> -	WARN(type == INTEL_OUTPUT_UNKNOWN, "compute_config() on unknown output!\n");
> >>> -
> >>>  	if (port == PORT_A)
> >>>  		pipe_config->cpu_transcoder = TRANSCODER_EDP;
> >>>  
> >>> -	if (type == INTEL_OUTPUT_HDMI)
> >>> +	if (intel_crtc_has_type(pipe_config, INTEL_OUTPUT_HDMI))
> >>>  		ret = intel_hdmi_compute_config(encoder, pipe_config, conn_state);
> >>>  	else
> >>>  		ret = intel_dp_compute_config(encoder, pipe_config, conn_state);
> >>> @@ -2764,6 +2789,7 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
> >>>  	drm_encoder_init(&dev_priv->drm, encoder, &intel_ddi_funcs,
> >>>  			 DRM_MODE_ENCODER_TMDS, "DDI %c", port_name(port));
> >>>  
> >>> +	intel_encoder->compute_output_type = intel_ddi_compute_output_type;
> >>>  	intel_encoder->compute_config = intel_ddi_compute_config;
> >>>  	intel_encoder->enable = intel_enable_ddi;
> >>>  	if (IS_GEN9_LP(dev_priv))
> >>> @@ -2821,9 +2847,10 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
> >>>  		}
> >>>  	}
> >>>  
> >>> +	intel_dig_port->dp.output_reg = INVALID_MMIO_REG;
> >>>  	intel_dig_port->max_lanes = max_lanes;
> >>>  
> >>> -	intel_encoder->type = INTEL_OUTPUT_UNKNOWN;
> >>> +	intel_encoder->type = INTEL_OUTPUT_DDI;
> >>>  	intel_encoder->power_domain = intel_port_to_power_domain(port);
> >>>  	intel_encoder->port = port;
> >>>  	intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
> >>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >>> index bd62c0a65bcd..a7e02ae33583 100644
> >>> --- a/drivers/gpu/drm/i915/intel_display.c
> >>> +++ b/drivers/gpu/drm/i915/intel_display.c
> >>> @@ -10591,7 +10591,7 @@ static const char * const output_type_str[] = {
> >>>  	OUTPUT_TYPE(DP),
> >>>  	OUTPUT_TYPE(EDP),
> >>>  	OUTPUT_TYPE(DSI),
> >>> -	OUTPUT_TYPE(UNKNOWN),
> >>> +	OUTPUT_TYPE(DDI),
> >>>  	OUTPUT_TYPE(DP_MST),
> >>>  };
> >>>  
> >>> @@ -10762,7 +10762,7 @@ static bool check_digital_port_conflicts(struct drm_atomic_state *state)
> >>>  
> >>>  		switch (encoder->type) {
> >>>  			unsigned int port_mask;
> >>> -		case INTEL_OUTPUT_UNKNOWN:
> >>> +		case INTEL_OUTPUT_DDI:
> >>>  			if (WARN_ON(!HAS_DDI(to_i915(dev))))
> >>>  				break;
> >>>  		case INTEL_OUTPUT_DP:
> >>> @@ -10895,7 +10895,12 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
> >>>  		 * Determine output_types before calling the .compute_config()
> >>>  		 * hooks so that the hooks can use this information safely.
> >>>  		 */
> >>> -		pipe_config->output_types |= 1 << encoder->type;
> >>> +		if (encoder->compute_output_type)
> >>> +			pipe_config->output_types |=
> >>> +				BIT(encoder->compute_output_type(encoder, pipe_config,
> >>> +								 connector_state));
> >>> +		else
> >>> +			pipe_config->output_types |= BIT(encoder->type);
> >>>  	}
> >>>  
> >>>  encoder_retry:
> >>> @@ -11572,10 +11577,8 @@ verify_crtc_state(struct drm_crtc *crtc,
> >>>  				"Encoder connected to wrong pipe %c\n",
> >>>  				pipe_name(pipe));
> >>>  
> >>> -		if (active) {
> >>> -			pipe_config->output_types |= 1 << encoder->type;
> >>> +		if (active)
> >>>  			encoder->get_config(encoder, pipe_config);
> >>> -		}
> >>>  	}
> >>>  
> >>>  	intel_crtc_compute_pixel_rate(pipe_config);
> >>> @@ -14963,7 +14966,6 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
> >>>  			crtc_state = to_intel_crtc_state(crtc->base.state);
> >>>  
> >>>  			encoder->base.crtc = &crtc->base;
> >>> -			crtc_state->output_types |= 1 << encoder->type;
> >>>  			encoder->get_config(encoder, crtc_state);
> >>>  		} else {
> >>>  			encoder->base.crtc = NULL;
> >>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> >>> index aa75f55eeb61..2b482d12000e 100644
> >>> --- a/drivers/gpu/drm/i915/intel_dp.c
> >>> +++ b/drivers/gpu/drm/i915/intel_dp.c
> >>> @@ -758,11 +758,16 @@ void intel_power_sequencer_reset(struct drm_i915_private *dev_priv)
> >>>  		struct intel_dp *intel_dp;
> >>>  
> >>>  		if (encoder->type != INTEL_OUTPUT_DP &&
> >>> -		    encoder->type != INTEL_OUTPUT_EDP)
> >>> +		    encoder->type != INTEL_OUTPUT_EDP &&
> >>> +		    encoder->type != INTEL_OUTPUT_DDI)
> >>>  			continue;
> >>>  
> >>>  		intel_dp = enc_to_intel_dp(&encoder->base);
> >>>  
> >>> +		/* Skip pure DVI/HDMI DDI encoders */
> >>> +		if (!i915_mmio_reg_valid(intel_dp->output_reg))
> >>> +			continue;
> >>> +
> >>>  		WARN_ON(intel_dp->active_pipe != INVALID_PIPE);
> >>>  
> >>>  		if (encoder->type != INTEL_OUTPUT_EDP)
> >>> @@ -2603,6 +2608,11 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
> >>>  	enum port port = dp_to_dig_port(intel_dp)->port;
> >>>  	struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
> >>>  
> >>> +	if (encoder->type == INTEL_OUTPUT_EDP)
> >>> +		pipe_config->output_types |= BIT(INTEL_OUTPUT_EDP);
> >>> +	else
> >>> +		pipe_config->output_types |= BIT(INTEL_OUTPUT_DP);
> >>> +
> >>>  	tmp = I915_READ(intel_dp->output_reg);
> >>>  
> >>>  	pipe_config->has_audio = tmp & DP_AUDIO_OUTPUT_ENABLE && port != PORT_A;
> >>> @@ -4699,8 +4709,6 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
> >>>  {
> >>>  	struct drm_connector *connector = &intel_connector->base;
> >>>  	struct intel_dp *intel_dp = intel_attached_dp(connector);
> >>> -	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> >>> -	struct intel_encoder *intel_encoder = &intel_dig_port->base;
> >>>  	struct drm_device *dev = connector->dev;
> >>>  	enum drm_connector_status status;
> >>>  	u8 sink_irq_vector = 0;
> >>> @@ -4733,9 +4741,6 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
> >>>  		goto out;
> >>>  	}
> >>>  
> >>> -	if (intel_encoder->type != INTEL_OUTPUT_EDP)
> >>> -		intel_encoder->type = INTEL_OUTPUT_DP;
> >>> -
> >>>  	if (intel_dp->reset_link_params) {
> >>>  		/* Initial max link lane count */
> >>>  		intel_dp->max_link_lane_count = intel_dp_max_common_lane_count(intel_dp);
> >>> @@ -4852,9 +4857,6 @@ intel_dp_force(struct drm_connector *connector)
> >>>  	intel_dp_set_edid(intel_dp);
> >>>  
> >>>  	intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
> >>> -
> >>> -	if (intel_encoder->type != INTEL_OUTPUT_EDP)
> >>> -		intel_encoder->type = INTEL_OUTPUT_DP;
> >>>  }
> >>>  
> >>>  static int intel_dp_get_modes(struct drm_connector *connector)
> >>> @@ -5073,10 +5075,6 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
> >>>  	struct drm_i915_private *dev_priv = to_i915(dev);
> >>>  	enum irqreturn ret = IRQ_NONE;
> >>>  
> >>> -	if (intel_dig_port->base.type != INTEL_OUTPUT_EDP &&
> >>> -	    intel_dig_port->base.type != INTEL_OUTPUT_HDMI)
> >>> -		intel_dig_port->base.type = INTEL_OUTPUT_DP;
> >>> -
> >>>  	if (long_hpd && intel_dig_port->base.type == INTEL_OUTPUT_EDP) {
> >>>  		/*
> >>>  		 * vdd off can generate a long pulse on eDP which
> >>> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
> >>> index 772521440a9f..9be6f32bb100 100644
> >>> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> >>> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> >>> @@ -270,6 +270,8 @@ static void intel_dp_mst_enc_get_config(struct intel_encoder *encoder,
> >>>  	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
> >>>  	u32 temp, flags = 0;
> >>>  
> >>> +	pipe_config->output_types |= BIT(INTEL_OUTPUT_DP_MST);
> >>> +
> >>>  	pipe_config->has_audio =
> >>>  		intel_ddi_is_audio_enabled(dev_priv, crtc);
> >>>  
> >>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> >>> index a05ab3a1ab27..650129980ce2 100644
> >>> --- a/drivers/gpu/drm/i915/intel_drv.h
> >>> +++ b/drivers/gpu/drm/i915/intel_drv.h
> >>> @@ -173,7 +173,7 @@ enum intel_output_type {
> >>>  	INTEL_OUTPUT_DP = 7,
> >>>  	INTEL_OUTPUT_EDP = 8,
> >>>  	INTEL_OUTPUT_DSI = 9,
> >>> -	INTEL_OUTPUT_UNKNOWN = 10,
> >>> +	INTEL_OUTPUT_DDI = 10,
> >>>  	INTEL_OUTPUT_DP_MST = 11,
> >>>  };
> >>>  
> >>> @@ -216,6 +216,9 @@ struct intel_encoder {
> >>>  	enum port port;
> >>>  	unsigned int cloneable;
> >>>  	void (*hot_plug)(struct intel_encoder *);
> >>> +	enum intel_output_type (*compute_output_type)(struct intel_encoder *,
> >>> +						      struct intel_crtc_state *,
> >>> +						      struct drm_connector_state *);
> >>>  	bool (*compute_config)(struct intel_encoder *,
> >>>  			       struct intel_crtc_state *,
> >>>  			       struct drm_connector_state *);
> >>> @@ -1149,7 +1152,7 @@ enc_to_dig_port(struct drm_encoder *encoder)
> >>>  	struct intel_encoder *intel_encoder = to_intel_encoder(encoder);
> >>>  
> >>>  	switch (intel_encoder->type) {
> >>> -	case INTEL_OUTPUT_UNKNOWN:
> >>> +	case INTEL_OUTPUT_DDI:
> >>>  		WARN_ON(!HAS_DDI(to_i915(encoder->dev)));
> >>>  	case INTEL_OUTPUT_DP:
> >>>  	case INTEL_OUTPUT_EDP:
> >>> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> >>> index 66bbedc5fa01..2ae3eea19317 100644
> >>> --- a/drivers/gpu/drm/i915/intel_dsi.c
> >>> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> >>> @@ -1243,6 +1243,8 @@ static void intel_dsi_get_config(struct intel_encoder *encoder,
> >>>  	u32 pclk;
> >>>  	DRM_DEBUG_KMS("\n");
> >>>  
> >>> +	pipe_config->output_types |= BIT(INTEL_OUTPUT_DSI);
> >>> +
> >>>  	if (IS_GEN9_LP(dev_priv))
> >>>  		bxt_dsi_get_pipe_config(encoder, pipe_config);
> >>>  
> >>> diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
> >>> index 53c9b763f4ce..754baa00bea9 100644
> >>> --- a/drivers/gpu/drm/i915/intel_dvo.c
> >>> +++ b/drivers/gpu/drm/i915/intel_dvo.c
> >>> @@ -159,6 +159,8 @@ static void intel_dvo_get_config(struct intel_encoder *encoder,
> >>>  	struct intel_dvo *intel_dvo = enc_to_dvo(encoder);
> >>>  	u32 tmp, flags = 0;
> >>>  
> >>> +	pipe_config->output_types |= BIT(INTEL_OUTPUT_DVO);
> >>> +
> >>>  	tmp = I915_READ(intel_dvo->dev.dvo_reg);
> >>>  	if (tmp & DVO_HSYNC_ACTIVE_HIGH)
> >>>  		flags |= DRM_MODE_FLAG_PHSYNC;
> >>> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> >>> index 5132dc814788..1ff448a67b99 100644
> >>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> >>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> >>> @@ -957,6 +957,8 @@ static void intel_hdmi_get_config(struct intel_encoder *encoder,
> >>>  	u32 tmp, flags = 0;
> >>>  	int dotclock;
> >>>  
> >>> +	pipe_config->output_types |= BIT(INTEL_OUTPUT_HDMI);
> >>> +
> >>>  	tmp = I915_READ(intel_hdmi->hdmi_reg);
> >>>  
> >>>  	if (tmp & SDVO_HSYNC_ACTIVE_HIGH)
> >>> @@ -1610,12 +1612,9 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
> >>>  
> >>>  	intel_hdmi_unset_edid(connector);
> >>>  
> >>> -	if (intel_hdmi_set_edid(connector)) {
> >>> -		struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
> >>> -
> >>> -		hdmi_to_dig_port(intel_hdmi)->base.type = INTEL_OUTPUT_HDMI;
> >>> +	if (intel_hdmi_set_edid(connector))
> >>>  		status = connector_status_connected;
> >>> -	} else
> >>> +	else
> >>>  		status = connector_status_disconnected;
> >>>  
> >>>  	intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS);
> >>> @@ -1626,8 +1625,6 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
> >>>  static void
> >>>  intel_hdmi_force(struct drm_connector *connector)
> >>>  {
> >>> -	struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
> >>> -
> >>>  	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
> >>>  		      connector->base.id, connector->name);
> >>>  
> >>> @@ -1637,7 +1634,6 @@ intel_hdmi_force(struct drm_connector *connector)
> >>>  		return;
> >>>  
> >>>  	intel_hdmi_set_edid(connector);
> >>> -	hdmi_to_dig_port(intel_hdmi)->base.type = INTEL_OUTPUT_HDMI;
> >>>  }
> >>>  
> >>>  static int intel_hdmi_get_modes(struct drm_connector *connector)
> >>> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> >>> index 38572d65e46e..ef80499113ee 100644
> >>> --- a/drivers/gpu/drm/i915/intel_lvds.c
> >>> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> >>> @@ -125,6 +125,8 @@ static void intel_lvds_get_config(struct intel_encoder *encoder,
> >>>  	struct intel_lvds_encoder *lvds_encoder = to_lvds_encoder(&encoder->base);
> >>>  	u32 tmp, flags = 0;
> >>>  
> >>> +	pipe_config->output_types |= BIT(INTEL_OUTPUT_LVDS);
> >>> +
> >>>  	tmp = I915_READ(lvds_encoder->reg);
> >>>  	if (tmp & LVDS_HSYNC_POLARITY)
> >>>  		flags |= DRM_MODE_FLAG_NHSYNC;
> >>> diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
> >>> index 1d946240e55f..39714be3eb6b 100644
> >>> --- a/drivers/gpu/drm/i915/intel_opregion.c
> >>> +++ b/drivers/gpu/drm/i915/intel_opregion.c
> >>> @@ -383,7 +383,7 @@ int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder,
> >>>  	case INTEL_OUTPUT_ANALOG:
> >>>  		type = DISPLAY_TYPE_CRT;
> >>>  		break;
> >>> -	case INTEL_OUTPUT_UNKNOWN:
> >>> +	case INTEL_OUTPUT_DDI:
> >>>  	case INTEL_OUTPUT_DP:
> >>>  	case INTEL_OUTPUT_HDMI:
> >>>  	case INTEL_OUTPUT_DP_MST:
> >>> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> >>> index 7437944b388f..42ec2d1f7a61 100644
> >>> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> >>> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> >>> @@ -1429,6 +1429,8 @@ static void intel_sdvo_get_config(struct intel_encoder *encoder,
> >>>  	u8 val;
> >>>  	bool ret;
> >>>  
> >>> +	pipe_config->output_types |= BIT(INTEL_OUTPUT_SDVO);
> >>> +
> >>>  	sdvox = I915_READ(intel_sdvo->sdvo_reg);
> >>>  
> >>>  	ret = intel_sdvo_get_input_timing(intel_sdvo, &dtd);
> >>> diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
> >>> index a79a7591b2cf..b18609cebe03 100644
> >>> --- a/drivers/gpu/drm/i915/intel_tv.c
> >>> +++ b/drivers/gpu/drm/i915/intel_tv.c
> >>> @@ -868,6 +868,8 @@ static void
> >>>  intel_tv_get_config(struct intel_encoder *encoder,
> >>>  		    struct intel_crtc_state *pipe_config)
> >>>  {
> >>> +	pipe_config->output_types |= BIT(INTEL_OUTPUT_TVOUT);
> >>> +
> >>>  	pipe_config->base.adjusted_mode.crtc_clock = pipe_config->port_clock;
> >>>  }
> >>>  
> 

-- 
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] 27+ messages in thread

* Re: [PATCH v5 05/10] drm/i915: Stop frobbing with DDI encoder->type
  2017-10-27 14:03         ` Ville Syrjälä
@ 2017-10-27 14:05           ` Maarten Lankhorst
  0 siblings, 0 replies; 27+ messages in thread
From: Maarten Lankhorst @ 2017-10-27 14:05 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

Op 27-10-17 om 16:03 schreef Ville Syrjälä:
> On Fri, Oct 27, 2017 at 03:53:34PM +0200, Maarten Lankhorst wrote:
>> Op 27-10-17 om 14:44 schreef Ville Syrjälä:
>>> On Fri, Oct 27, 2017 at 02:05:38PM +0200, Maarten Lankhorst wrote:
>>>> Op 19-10-17 om 15:37 schreef Ville Syrjala:
>>>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>>>
>>>>> Currently the DDI encoder->type will change at runtime depending on
>>>>> what kind of hotplugs we've processed. That's quite bad since we can't
>>>>> really trust that that current value of encoder->type actually matches
>>>>> the type of signal we're trying to drive through it.
>>>>>
>>>>> Let's eliminate that problem by declaring that non-eDP DDI port will
>>>>> always have the encoder type as INTEL_OUTPUT_DDI. This means the code
>>>>> can no longer try to distinguish DP vs. HDMI based on encoder->type.
>>>>> We'll leave eDP as INTEL_OUTPUT_EDP, since it'll never change and
>>>>> there's a bunch of code that relies on that value to identify eDP
>>>>> encoders.
>>>>>
>>>>> We'll introduce a new encoder .compute_output_type() hook. This allows
>>>>> us to compute the full output_types before any encoder .compute_config()
>>>>> hooks get called, thus those hooks can rely on output_types being
>>>>> correct, which is useful for cloning on oldr platforms. For now we'll
>>>>> just look at the connector type and pick the correct mode based on that.
>>>>> In the future the new hook could be used to implement dynamic switching
>>>>> between LS and PCON modes for LSPCON.
>>>>>
>>>>> TODO: maybe make .get_config() populate output_types rather than doing
>>>>> the default encoder->type thing in caller and then undoing it for
>>>>> DDI in .get_config().
>>>>>
>>>>> v2: Fix BXT/GLK PPS explosion with DSI/MST encoders
>>>>> v3: Avoid the PPS warn on pure HDMI/DVI DDI encoders by checking dp.output_reg
>>>>> v4: Rebase
>>>>> v5: Populate output_types in .get_config() rather than in the caller
>>>> Could we get rid of compute_output_type in this patch? Perhaps do it as preliminary
>>>> patch towards removing the use?
>>> We don't want to remove it.
>> Ok, but could that still be a separate preparation patch? That would make it easier to review. :)
> Not sure which part you mean here. Moving the 'output_types |= whatever'
> into .get_config()? Hmm. Yeah that should be easy to split out. I guess
> it won't even make the DDI case any more broken that it already is since
> even currently encoder->type might change between the modeset compute
> phase and .get_config(). And it's going to be followed closely by the
> actual fix anyway.
>
Yes, that part. It should work even without the UNKNOWN->DDI change afaics.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 08/10] drm/i915: Pass a crtc state to ddi post_disable from MST code
  2017-10-27 11:49     ` Ville Syrjälä
@ 2017-10-27 14:31       ` Maarten Lankhorst
  0 siblings, 0 replies; 27+ messages in thread
From: Maarten Lankhorst @ 2017-10-27 14:31 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

Op 27-10-17 om 13:49 schreef Ville Syrjälä:
> On Fri, Oct 27, 2017 at 01:39:00PM +0200, Maarten Lankhorst wrote:
>> Op 19-10-17 om 15:37 schreef Ville Syrjala:
>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>
>>> Pass an old crtc state to intel_ddi_post_disable() from the MST code.
>>>
>>> Note that this crtc state won't necessaitly match the one that was
>>> passed to intel_ddi_pre_enable() if the first stream to be enabled isn't
>>> the last stream to be disabled. But this is fine since the states should
>>> be identical in every important way. This does mean people frobbing
>>> the DDI pre_enable/post_disable hooks have to pay attention in what
>>> parts of the state they consult.
>>>
>>> The alternative would be to inline the relevant code into the MST code.
>>> That is actually what we used to do for pre_enable before
>>> commit e081c8463ac9 ("drm/i915: Remove duplicate DDI enabling logic
>>> from MST path"). For post_disable we've always called the DDI hook.
>> I woudl rather have it NULL throughout the entire disable sequence, it's going to be
>> dangerous anyway, and something dangerous shouldn't be easy. The fact that you have
>> to think
> It makes for really ugly code as the enable and disable don't look at
> all like each other when one has the NULL and the other doesn't. If it's
> dangerous for disable, then it's equally dangerous for enable. So I'd
> just go with whatever makes the code suck the least, which IMO is
> passing a crtc state to both.
>
> Probably the proper fix would be some kind of encoder state which would
> be assigned to mst->primary. But that's going to require actual design
> work.
Yeah, I just think it's dangerous regardless, I wish we could do something more safe about it, but this will have to do for now.

Theoretically we could add encoder state, but that probably requires a massive split in crtc_state.

Not sure what's wisdom here. :/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* ✗ Fi.CI.BAT: failure for drm/i915: Last part of DDI encoder->type cleanup
  2017-10-19 13:37 [PATCH 00/10] drm/i915: Last part of DDI encoder->type cleanup Ville Syrjala
                   ` (11 preceding siblings ...)
  2017-10-19 14:59 ` ✓ Fi.CI.IGT: " Patchwork
@ 2017-10-27 16:39 ` Patchwork
  12 siblings, 0 replies; 27+ messages in thread
From: Patchwork @ 2017-10-27 16:39 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Last part of DDI encoder->type cleanup
URL   : https://patchwork.freedesktop.org/series/32298/
State : failure

== Summary ==

Series 32298v1 drm/i915: Last part of DDI encoder->type cleanup
https://patchwork.freedesktop.org/api/1.0/series/32298/revisions/1/mbox/

Test chamelium:
        Subgroup dp-crc-fast:
                pass       -> FAIL       (fi-kbl-7500u) fdo#102514
Test gem_exec_suspend:
        Subgroup basic-s3:
                pass       -> DMESG-WARN (fi-bxt-j4205)
        Subgroup basic-s4-devices:
                pass       -> DMESG-WARN (fi-bxt-j4205)
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                pass       -> DMESG-WARN (fi-bxt-j4205)
        Subgroup suspend-read-crc-pipe-b:
                pass       -> DMESG-WARN (fi-bxt-j4205)
        Subgroup suspend-read-crc-pipe-c:
                pass       -> DMESG-WARN (fi-bxt-j4205)
Test kms_psr_sink_crc:
        Subgroup psr_basic:
                pass       -> INCOMPLETE (fi-cnl-y)
Test drv_module_reload:
        Subgroup basic-reload:
                pass       -> DMESG-WARN (fi-bxt-j4205)

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

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:443s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:451s
fi-blb-e6850     total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:375s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:524s
fi-bwr-2160      total:289  pass:183  dwarn:0   dfail:0   fail:0   skip:106 time:266s
fi-bxt-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:494s
fi-bxt-j4205     total:289  pass:254  dwarn:6   dfail:0   fail:0   skip:29  time:503s
fi-byt-j1900     total:289  pass:253  dwarn:1   dfail:0   fail:0   skip:35  time:492s
fi-byt-n2820     total:289  pass:249  dwarn:1   dfail:0   fail:0   skip:39  time:470s
fi-cnl-y         total:289  pass:223  dwarn:0   dfail:0   fail:0   skip:24 
fi-elk-e7500     total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:419s
fi-gdg-551       total:289  pass:178  dwarn:1   dfail:0   fail:1   skip:109 time:249s
fi-glk-1         total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:576s
fi-glk-dsi       total:289  pass:258  dwarn:0   dfail:0   fail:1   skip:30  time:490s
fi-hsw-4770      total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:428s
fi-hsw-4770r     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:418s
fi-ilk-650       total:289  pass:228  dwarn:0   dfail:0   fail:0   skip:61  time:426s
fi-ivb-3520m     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:486s
fi-ivb-3770      total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:465s
fi-kbl-7500u     total:289  pass:263  dwarn:1   dfail:0   fail:1   skip:24  time:483s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:574s
fi-kbl-7567u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:482s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:584s
fi-pnv-d510      total:289  pass:222  dwarn:1   dfail:0   fail:0   skip:66  time:543s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:451s
fi-skl-6600u     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:584s
fi-skl-6700hq    total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:639s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:515s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:498s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:455s
fi-snb-2520m     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:564s
fi-snb-2600      total:289  pass:249  dwarn:0   dfail:0   fail:0   skip:40  time:416s

4efa630855362c267af94785e50948bb60615bfe drm-tip: 2017y-10m-27d-15h-25m-04s UTC integration manifest
019a84602963 drm/i915: Stop using encoder->type in intel_ddi_enable_transcoder_func()
ce949d4af951 drm/i915: Start using output_types for DPLL selection
325d38842030 drm/i915: Pass crtc state to intel_prepare_dp_ddi_buffers()
04f006f37c09 drm/i915: Don't use encoder->type in intel_ddi_set_pipe_settings()

== Logs ==

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

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

* Re: [PATCH v5 05/10] drm/i915: Stop frobbing with DDI encoder->type
  2017-10-27 12:05   ` Maarten Lankhorst
  2017-10-27 12:44     ` Ville Syrjälä
@ 2017-10-27 19:32     ` Ville Syrjälä
  1 sibling, 0 replies; 27+ messages in thread
From: Ville Syrjälä @ 2017-10-27 19:32 UTC (permalink / raw)
  To: Maarten Lankhorst; +Cc: intel-gfx

On Fri, Oct 27, 2017 at 02:05:38PM +0200, Maarten Lankhorst wrote:
> Op 19-10-17 om 15:37 schreef Ville Syrjala:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Currently the DDI encoder->type will change at runtime depending on
> > what kind of hotplugs we've processed. That's quite bad since we can't
> > really trust that that current value of encoder->type actually matches
> > the type of signal we're trying to drive through it.
> >
> > Let's eliminate that problem by declaring that non-eDP DDI port will
> > always have the encoder type as INTEL_OUTPUT_DDI. This means the code
> > can no longer try to distinguish DP vs. HDMI based on encoder->type.
> > We'll leave eDP as INTEL_OUTPUT_EDP, since it'll never change and
> > there's a bunch of code that relies on that value to identify eDP
> > encoders.
> >
> > We'll introduce a new encoder .compute_output_type() hook. This allows
> > us to compute the full output_types before any encoder .compute_config()
> > hooks get called, thus those hooks can rely on output_types being
> > correct, which is useful for cloning on oldr platforms. For now we'll
> > just look at the connector type and pick the correct mode based on that.
> > In the future the new hook could be used to implement dynamic switching
> > between LS and PCON modes for LSPCON.
> >
> > TODO: maybe make .get_config() populate output_types rather than doing
> > the default encoder->type thing in caller and then undoing it for
> > DDI in .get_config().
> >
> > v2: Fix BXT/GLK PPS explosion with DSI/MST encoders
> > v3: Avoid the PPS warn on pure HDMI/DVI DDI encoders by checking dp.output_reg
> > v4: Rebase
> > v5: Populate output_types in .get_config() rather than in the caller
> 
> Could we get rid of compute_output_type in this patch? Perhaps do it as preliminary
> patch towards removing the use?
> 
> Rest of the series except patch 8 looks good. For patch 1-4, 6, 7, 9 (if the delta
> between ddi_get_config and old mst get_config is harmless or beneficial) and 10:
> 
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

Thanks. First four patches pushed to dinq. The rest reposted as a new
series, with this patch split up.

> 
> Thanks for the work towards cleaning this mess up.
> 
> ~Maarten
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_debugfs.c   |  2 +-
> >  drivers/gpu/drm/i915/intel_crt.c      |  2 ++
> >  drivers/gpu/drm/i915/intel_ddi.c      | 43 ++++++++++++++++++++++++++++-------
> >  drivers/gpu/drm/i915/intel_display.c  | 16 +++++++------
> >  drivers/gpu/drm/i915/intel_dp.c       | 24 +++++++++----------
> >  drivers/gpu/drm/i915/intel_dp_mst.c   |  2 ++
> >  drivers/gpu/drm/i915/intel_drv.h      |  7 ++++--
> >  drivers/gpu/drm/i915/intel_dsi.c      |  2 ++
> >  drivers/gpu/drm/i915/intel_dvo.c      |  2 ++
> >  drivers/gpu/drm/i915/intel_hdmi.c     | 12 ++++------
> >  drivers/gpu/drm/i915/intel_lvds.c     |  2 ++
> >  drivers/gpu/drm/i915/intel_opregion.c |  2 +-
> >  drivers/gpu/drm/i915/intel_sdvo.c     |  2 ++
> >  drivers/gpu/drm/i915/intel_tv.c       |  2 ++
> >  14 files changed, 80 insertions(+), 40 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> > index c65e381b85f3..e5333bfdf32d 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -3049,7 +3049,7 @@ static void intel_connector_info(struct seq_file *m,
> >  		break;
> >  	case DRM_MODE_CONNECTOR_HDMIA:
> >  		if (intel_encoder->type == INTEL_OUTPUT_HDMI ||
> > -		    intel_encoder->type == INTEL_OUTPUT_UNKNOWN)
> > +		    intel_encoder->type == INTEL_OUTPUT_DDI)
> >  			intel_hdmi_info(m, intel_connector);
> >  		break;
> >  	default:
> > diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> > index 668e8c3e791d..a61e61efe9aa 100644
> > --- a/drivers/gpu/drm/i915/intel_crt.c
> > +++ b/drivers/gpu/drm/i915/intel_crt.c
> > @@ -119,6 +119,8 @@ static unsigned int intel_crt_get_flags(struct intel_encoder *encoder)
> >  static void intel_crt_get_config(struct intel_encoder *encoder,
> >  				 struct intel_crtc_state *pipe_config)
> >  {
> > +	pipe_config->output_types |= BIT(INTEL_OUTPUT_ANALOG);
> > +
> >  	pipe_config->base.adjusted_mode.flags |= intel_crt_get_flags(encoder);
> >  
> >  	pipe_config->base.adjusted_mode.crtc_clock = pipe_config->port_clock;
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > index c8790bb74839..630609575db4 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -497,10 +497,8 @@ enum port intel_ddi_get_encoder_port(struct intel_encoder *encoder)
> >  	switch (encoder->type) {
> >  	case INTEL_OUTPUT_DP_MST:
> >  		return enc_to_mst(&encoder->base)->primary->port;
> > -	case INTEL_OUTPUT_DP:
> >  	case INTEL_OUTPUT_EDP:
> > -	case INTEL_OUTPUT_HDMI:
> > -	case INTEL_OUTPUT_UNKNOWN:
> > +	case INTEL_OUTPUT_DDI:
> >  		return enc_to_dig_port(&encoder->base)->port;
> >  	case INTEL_OUTPUT_ANALOG:
> >  		return PORT_E;
> > @@ -1532,6 +1530,7 @@ void intel_ddi_set_vc_payload_alloc(const struct intel_crtc_state *crtc_state,
> >  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> >  	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> >  	uint32_t temp;
> > +
> >  	temp = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
> >  	if (state == true)
> >  		temp |= TRANS_DDI_DP_VC_PAYLOAD_ALLOC;
> > @@ -2586,12 +2585,23 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
> >  			pipe_config->hdmi_high_tmds_clock_ratio = true;
> >  		/* fall through */
> >  	case TRANS_DDI_MODE_SELECT_DVI:
> > +		pipe_config->output_types |= BIT(INTEL_OUTPUT_HDMI);
> >  		pipe_config->lane_count = 4;
> >  		break;
> >  	case TRANS_DDI_MODE_SELECT_FDI:
> > +		pipe_config->output_types |= BIT(INTEL_OUTPUT_ANALOG);
> >  		break;
> >  	case TRANS_DDI_MODE_SELECT_DP_SST:
> > +		if (encoder->type == INTEL_OUTPUT_EDP)
> > +			pipe_config->output_types |= BIT(INTEL_OUTPUT_EDP);
> > +		else
> > +			pipe_config->output_types |= BIT(INTEL_OUTPUT_DP);
> > +		pipe_config->lane_count =
> > +			((temp & DDI_PORT_WIDTH_MASK) >> DDI_PORT_WIDTH_SHIFT) + 1;
> > +		intel_dp_get_m_n(intel_crtc, pipe_config);
> > +		break;
> >  	case TRANS_DDI_MODE_SELECT_DP_MST:
> > +		pipe_config->output_types |= BIT(INTEL_OUTPUT_DP_MST);
> >  		pipe_config->lane_count =
> >  			((temp & DDI_PORT_WIDTH_MASK) >> DDI_PORT_WIDTH_SHIFT) + 1;
> >  		intel_dp_get_m_n(intel_crtc, pipe_config);
> > @@ -2630,21 +2640,36 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
> >  			bxt_ddi_phy_get_lane_lat_optim_mask(encoder);
> >  }
> >  
> > +static enum intel_output_type
> > +intel_ddi_compute_output_type(struct intel_encoder *encoder,
> > +			      struct intel_crtc_state *crtc_state,
> > +			      struct drm_connector_state *conn_state)
> > +{
> > +	switch (conn_state->connector->connector_type) {
> > +	case DRM_MODE_CONNECTOR_HDMIA:
> > +		return INTEL_OUTPUT_HDMI;
> > +	case DRM_MODE_CONNECTOR_eDP:
> > +		return INTEL_OUTPUT_EDP;
> > +	case DRM_MODE_CONNECTOR_DisplayPort:
> > +		return INTEL_OUTPUT_DP;
> > +	default:
> > +		MISSING_CASE(conn_state->connector->connector_type);
> > +		return INTEL_OUTPUT_UNUSED;
> > +	}
> > +}
> > +
> >  static bool intel_ddi_compute_config(struct intel_encoder *encoder,
> >  				     struct intel_crtc_state *pipe_config,
> >  				     struct drm_connector_state *conn_state)
> >  {
> >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > -	int type = encoder->type;
> >  	int port = intel_ddi_get_encoder_port(encoder);
> >  	int ret;
> >  
> > -	WARN(type == INTEL_OUTPUT_UNKNOWN, "compute_config() on unknown output!\n");
> > -
> >  	if (port == PORT_A)
> >  		pipe_config->cpu_transcoder = TRANSCODER_EDP;
> >  
> > -	if (type == INTEL_OUTPUT_HDMI)
> > +	if (intel_crtc_has_type(pipe_config, INTEL_OUTPUT_HDMI))
> >  		ret = intel_hdmi_compute_config(encoder, pipe_config, conn_state);
> >  	else
> >  		ret = intel_dp_compute_config(encoder, pipe_config, conn_state);
> > @@ -2764,6 +2789,7 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
> >  	drm_encoder_init(&dev_priv->drm, encoder, &intel_ddi_funcs,
> >  			 DRM_MODE_ENCODER_TMDS, "DDI %c", port_name(port));
> >  
> > +	intel_encoder->compute_output_type = intel_ddi_compute_output_type;
> >  	intel_encoder->compute_config = intel_ddi_compute_config;
> >  	intel_encoder->enable = intel_enable_ddi;
> >  	if (IS_GEN9_LP(dev_priv))
> > @@ -2821,9 +2847,10 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
> >  		}
> >  	}
> >  
> > +	intel_dig_port->dp.output_reg = INVALID_MMIO_REG;
> >  	intel_dig_port->max_lanes = max_lanes;
> >  
> > -	intel_encoder->type = INTEL_OUTPUT_UNKNOWN;
> > +	intel_encoder->type = INTEL_OUTPUT_DDI;
> >  	intel_encoder->power_domain = intel_port_to_power_domain(port);
> >  	intel_encoder->port = port;
> >  	intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index bd62c0a65bcd..a7e02ae33583 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -10591,7 +10591,7 @@ static const char * const output_type_str[] = {
> >  	OUTPUT_TYPE(DP),
> >  	OUTPUT_TYPE(EDP),
> >  	OUTPUT_TYPE(DSI),
> > -	OUTPUT_TYPE(UNKNOWN),
> > +	OUTPUT_TYPE(DDI),
> >  	OUTPUT_TYPE(DP_MST),
> >  };
> >  
> > @@ -10762,7 +10762,7 @@ static bool check_digital_port_conflicts(struct drm_atomic_state *state)
> >  
> >  		switch (encoder->type) {
> >  			unsigned int port_mask;
> > -		case INTEL_OUTPUT_UNKNOWN:
> > +		case INTEL_OUTPUT_DDI:
> >  			if (WARN_ON(!HAS_DDI(to_i915(dev))))
> >  				break;
> >  		case INTEL_OUTPUT_DP:
> > @@ -10895,7 +10895,12 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
> >  		 * Determine output_types before calling the .compute_config()
> >  		 * hooks so that the hooks can use this information safely.
> >  		 */
> > -		pipe_config->output_types |= 1 << encoder->type;
> > +		if (encoder->compute_output_type)
> > +			pipe_config->output_types |=
> > +				BIT(encoder->compute_output_type(encoder, pipe_config,
> > +								 connector_state));
> > +		else
> > +			pipe_config->output_types |= BIT(encoder->type);
> >  	}
> >  
> >  encoder_retry:
> > @@ -11572,10 +11577,8 @@ verify_crtc_state(struct drm_crtc *crtc,
> >  				"Encoder connected to wrong pipe %c\n",
> >  				pipe_name(pipe));
> >  
> > -		if (active) {
> > -			pipe_config->output_types |= 1 << encoder->type;
> > +		if (active)
> >  			encoder->get_config(encoder, pipe_config);
> > -		}
> >  	}
> >  
> >  	intel_crtc_compute_pixel_rate(pipe_config);
> > @@ -14963,7 +14966,6 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
> >  			crtc_state = to_intel_crtc_state(crtc->base.state);
> >  
> >  			encoder->base.crtc = &crtc->base;
> > -			crtc_state->output_types |= 1 << encoder->type;
> >  			encoder->get_config(encoder, crtc_state);
> >  		} else {
> >  			encoder->base.crtc = NULL;
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index aa75f55eeb61..2b482d12000e 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -758,11 +758,16 @@ void intel_power_sequencer_reset(struct drm_i915_private *dev_priv)
> >  		struct intel_dp *intel_dp;
> >  
> >  		if (encoder->type != INTEL_OUTPUT_DP &&
> > -		    encoder->type != INTEL_OUTPUT_EDP)
> > +		    encoder->type != INTEL_OUTPUT_EDP &&
> > +		    encoder->type != INTEL_OUTPUT_DDI)
> >  			continue;
> >  
> >  		intel_dp = enc_to_intel_dp(&encoder->base);
> >  
> > +		/* Skip pure DVI/HDMI DDI encoders */
> > +		if (!i915_mmio_reg_valid(intel_dp->output_reg))
> > +			continue;
> > +
> >  		WARN_ON(intel_dp->active_pipe != INVALID_PIPE);
> >  
> >  		if (encoder->type != INTEL_OUTPUT_EDP)
> > @@ -2603,6 +2608,11 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
> >  	enum port port = dp_to_dig_port(intel_dp)->port;
> >  	struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
> >  
> > +	if (encoder->type == INTEL_OUTPUT_EDP)
> > +		pipe_config->output_types |= BIT(INTEL_OUTPUT_EDP);
> > +	else
> > +		pipe_config->output_types |= BIT(INTEL_OUTPUT_DP);
> > +
> >  	tmp = I915_READ(intel_dp->output_reg);
> >  
> >  	pipe_config->has_audio = tmp & DP_AUDIO_OUTPUT_ENABLE && port != PORT_A;
> > @@ -4699,8 +4709,6 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
> >  {
> >  	struct drm_connector *connector = &intel_connector->base;
> >  	struct intel_dp *intel_dp = intel_attached_dp(connector);
> > -	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > -	struct intel_encoder *intel_encoder = &intel_dig_port->base;
> >  	struct drm_device *dev = connector->dev;
> >  	enum drm_connector_status status;
> >  	u8 sink_irq_vector = 0;
> > @@ -4733,9 +4741,6 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
> >  		goto out;
> >  	}
> >  
> > -	if (intel_encoder->type != INTEL_OUTPUT_EDP)
> > -		intel_encoder->type = INTEL_OUTPUT_DP;
> > -
> >  	if (intel_dp->reset_link_params) {
> >  		/* Initial max link lane count */
> >  		intel_dp->max_link_lane_count = intel_dp_max_common_lane_count(intel_dp);
> > @@ -4852,9 +4857,6 @@ intel_dp_force(struct drm_connector *connector)
> >  	intel_dp_set_edid(intel_dp);
> >  
> >  	intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
> > -
> > -	if (intel_encoder->type != INTEL_OUTPUT_EDP)
> > -		intel_encoder->type = INTEL_OUTPUT_DP;
> >  }
> >  
> >  static int intel_dp_get_modes(struct drm_connector *connector)
> > @@ -5073,10 +5075,6 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
> >  	struct drm_i915_private *dev_priv = to_i915(dev);
> >  	enum irqreturn ret = IRQ_NONE;
> >  
> > -	if (intel_dig_port->base.type != INTEL_OUTPUT_EDP &&
> > -	    intel_dig_port->base.type != INTEL_OUTPUT_HDMI)
> > -		intel_dig_port->base.type = INTEL_OUTPUT_DP;
> > -
> >  	if (long_hpd && intel_dig_port->base.type == INTEL_OUTPUT_EDP) {
> >  		/*
> >  		 * vdd off can generate a long pulse on eDP which
> > diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
> > index 772521440a9f..9be6f32bb100 100644
> > --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> > +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> > @@ -270,6 +270,8 @@ static void intel_dp_mst_enc_get_config(struct intel_encoder *encoder,
> >  	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
> >  	u32 temp, flags = 0;
> >  
> > +	pipe_config->output_types |= BIT(INTEL_OUTPUT_DP_MST);
> > +
> >  	pipe_config->has_audio =
> >  		intel_ddi_is_audio_enabled(dev_priv, crtc);
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index a05ab3a1ab27..650129980ce2 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -173,7 +173,7 @@ enum intel_output_type {
> >  	INTEL_OUTPUT_DP = 7,
> >  	INTEL_OUTPUT_EDP = 8,
> >  	INTEL_OUTPUT_DSI = 9,
> > -	INTEL_OUTPUT_UNKNOWN = 10,
> > +	INTEL_OUTPUT_DDI = 10,
> >  	INTEL_OUTPUT_DP_MST = 11,
> >  };
> >  
> > @@ -216,6 +216,9 @@ struct intel_encoder {
> >  	enum port port;
> >  	unsigned int cloneable;
> >  	void (*hot_plug)(struct intel_encoder *);
> > +	enum intel_output_type (*compute_output_type)(struct intel_encoder *,
> > +						      struct intel_crtc_state *,
> > +						      struct drm_connector_state *);
> >  	bool (*compute_config)(struct intel_encoder *,
> >  			       struct intel_crtc_state *,
> >  			       struct drm_connector_state *);
> > @@ -1149,7 +1152,7 @@ enc_to_dig_port(struct drm_encoder *encoder)
> >  	struct intel_encoder *intel_encoder = to_intel_encoder(encoder);
> >  
> >  	switch (intel_encoder->type) {
> > -	case INTEL_OUTPUT_UNKNOWN:
> > +	case INTEL_OUTPUT_DDI:
> >  		WARN_ON(!HAS_DDI(to_i915(encoder->dev)));
> >  	case INTEL_OUTPUT_DP:
> >  	case INTEL_OUTPUT_EDP:
> > diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> > index 66bbedc5fa01..2ae3eea19317 100644
> > --- a/drivers/gpu/drm/i915/intel_dsi.c
> > +++ b/drivers/gpu/drm/i915/intel_dsi.c
> > @@ -1243,6 +1243,8 @@ static void intel_dsi_get_config(struct intel_encoder *encoder,
> >  	u32 pclk;
> >  	DRM_DEBUG_KMS("\n");
> >  
> > +	pipe_config->output_types |= BIT(INTEL_OUTPUT_DSI);
> > +
> >  	if (IS_GEN9_LP(dev_priv))
> >  		bxt_dsi_get_pipe_config(encoder, pipe_config);
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
> > index 53c9b763f4ce..754baa00bea9 100644
> > --- a/drivers/gpu/drm/i915/intel_dvo.c
> > +++ b/drivers/gpu/drm/i915/intel_dvo.c
> > @@ -159,6 +159,8 @@ static void intel_dvo_get_config(struct intel_encoder *encoder,
> >  	struct intel_dvo *intel_dvo = enc_to_dvo(encoder);
> >  	u32 tmp, flags = 0;
> >  
> > +	pipe_config->output_types |= BIT(INTEL_OUTPUT_DVO);
> > +
> >  	tmp = I915_READ(intel_dvo->dev.dvo_reg);
> >  	if (tmp & DVO_HSYNC_ACTIVE_HIGH)
> >  		flags |= DRM_MODE_FLAG_PHSYNC;
> > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> > index 5132dc814788..1ff448a67b99 100644
> > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > @@ -957,6 +957,8 @@ static void intel_hdmi_get_config(struct intel_encoder *encoder,
> >  	u32 tmp, flags = 0;
> >  	int dotclock;
> >  
> > +	pipe_config->output_types |= BIT(INTEL_OUTPUT_HDMI);
> > +
> >  	tmp = I915_READ(intel_hdmi->hdmi_reg);
> >  
> >  	if (tmp & SDVO_HSYNC_ACTIVE_HIGH)
> > @@ -1610,12 +1612,9 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
> >  
> >  	intel_hdmi_unset_edid(connector);
> >  
> > -	if (intel_hdmi_set_edid(connector)) {
> > -		struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
> > -
> > -		hdmi_to_dig_port(intel_hdmi)->base.type = INTEL_OUTPUT_HDMI;
> > +	if (intel_hdmi_set_edid(connector))
> >  		status = connector_status_connected;
> > -	} else
> > +	else
> >  		status = connector_status_disconnected;
> >  
> >  	intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS);
> > @@ -1626,8 +1625,6 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
> >  static void
> >  intel_hdmi_force(struct drm_connector *connector)
> >  {
> > -	struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
> > -
> >  	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
> >  		      connector->base.id, connector->name);
> >  
> > @@ -1637,7 +1634,6 @@ intel_hdmi_force(struct drm_connector *connector)
> >  		return;
> >  
> >  	intel_hdmi_set_edid(connector);
> > -	hdmi_to_dig_port(intel_hdmi)->base.type = INTEL_OUTPUT_HDMI;
> >  }
> >  
> >  static int intel_hdmi_get_modes(struct drm_connector *connector)
> > diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> > index 38572d65e46e..ef80499113ee 100644
> > --- a/drivers/gpu/drm/i915/intel_lvds.c
> > +++ b/drivers/gpu/drm/i915/intel_lvds.c
> > @@ -125,6 +125,8 @@ static void intel_lvds_get_config(struct intel_encoder *encoder,
> >  	struct intel_lvds_encoder *lvds_encoder = to_lvds_encoder(&encoder->base);
> >  	u32 tmp, flags = 0;
> >  
> > +	pipe_config->output_types |= BIT(INTEL_OUTPUT_LVDS);
> > +
> >  	tmp = I915_READ(lvds_encoder->reg);
> >  	if (tmp & LVDS_HSYNC_POLARITY)
> >  		flags |= DRM_MODE_FLAG_NHSYNC;
> > diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
> > index 1d946240e55f..39714be3eb6b 100644
> > --- a/drivers/gpu/drm/i915/intel_opregion.c
> > +++ b/drivers/gpu/drm/i915/intel_opregion.c
> > @@ -383,7 +383,7 @@ int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder,
> >  	case INTEL_OUTPUT_ANALOG:
> >  		type = DISPLAY_TYPE_CRT;
> >  		break;
> > -	case INTEL_OUTPUT_UNKNOWN:
> > +	case INTEL_OUTPUT_DDI:
> >  	case INTEL_OUTPUT_DP:
> >  	case INTEL_OUTPUT_HDMI:
> >  	case INTEL_OUTPUT_DP_MST:
> > diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> > index 7437944b388f..42ec2d1f7a61 100644
> > --- a/drivers/gpu/drm/i915/intel_sdvo.c
> > +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> > @@ -1429,6 +1429,8 @@ static void intel_sdvo_get_config(struct intel_encoder *encoder,
> >  	u8 val;
> >  	bool ret;
> >  
> > +	pipe_config->output_types |= BIT(INTEL_OUTPUT_SDVO);
> > +
> >  	sdvox = I915_READ(intel_sdvo->sdvo_reg);
> >  
> >  	ret = intel_sdvo_get_input_timing(intel_sdvo, &dtd);
> > diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
> > index a79a7591b2cf..b18609cebe03 100644
> > --- a/drivers/gpu/drm/i915/intel_tv.c
> > +++ b/drivers/gpu/drm/i915/intel_tv.c
> > @@ -868,6 +868,8 @@ static void
> >  intel_tv_get_config(struct intel_encoder *encoder,
> >  		    struct intel_crtc_state *pipe_config)
> >  {
> > +	pipe_config->output_types |= BIT(INTEL_OUTPUT_TVOUT);
> > +
> >  	pipe_config->base.adjusted_mode.crtc_clock = pipe_config->port_clock;
> >  }
> >  
> 

-- 
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] 27+ messages in thread

end of thread, other threads:[~2017-10-27 19:32 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-19 13:37 [PATCH 00/10] drm/i915: Last part of DDI encoder->type cleanup Ville Syrjala
2017-10-19 13:37 ` [PATCH 01/10] drm/i915: Don't use encoder->type in intel_ddi_set_pipe_settings() Ville Syrjala
2017-10-19 13:37 ` [PATCH 02/10] drm/i915: Pass crtc state to intel_prepare_dp_ddi_buffers() Ville Syrjala
2017-10-19 13:37 ` [PATCH 03/10] drm/i915: Start using output_types for DPLL selection Ville Syrjala
2017-10-19 13:37 ` [PATCH 04/10] drm/i915: Stop using encoder->type in intel_ddi_enable_transcoder_func() Ville Syrjala
2017-10-19 13:37 ` [PATCH v5 05/10] drm/i915: Stop frobbing with DDI encoder->type Ville Syrjala
2017-10-27 12:05   ` Maarten Lankhorst
2017-10-27 12:44     ` Ville Syrjälä
2017-10-27 13:53       ` Maarten Lankhorst
2017-10-27 14:03         ` Ville Syrjälä
2017-10-27 14:05           ` Maarten Lankhorst
2017-10-27 19:32     ` Ville Syrjälä
2017-10-19 13:37 ` [PATCH 06/10] drm/i915: Nuke intel_ddi_get_encoder_port() Ville Syrjala
2017-10-19 13:37 ` [PATCH 07/10] drm/i915: Eliminate pll->state usage from bxt_calc_pll_link() Ville Syrjala
2017-10-19 13:37 ` [PATCH 08/10] drm/i915: Pass a crtc state to ddi post_disable from MST code Ville Syrjala
2017-10-27 11:39   ` Maarten Lankhorst
2017-10-27 11:49     ` Ville Syrjälä
2017-10-27 14:31       ` Maarten Lankhorst
2017-10-19 13:37 ` [PATCH 09/10] drm/i915: Use intel_ddi_get_config() for MST Ville Syrjala
2017-10-27 11:43   ` Maarten Lankhorst
2017-10-27 11:55     ` Ville Syrjälä
2017-10-19 13:37 ` [PATCH 10/10] drm/i915: Use intel_crtc_has_dp_encoder() for LPE audio Ville Syrjala
2017-10-27 11:43   ` Maarten Lankhorst
2017-10-27 11:53     ` Ville Syrjälä
2017-10-19 14:00 ` ✓ Fi.CI.BAT: success for drm/i915: Last part of DDI encoder->type cleanup Patchwork
2017-10-19 14:59 ` ✓ Fi.CI.IGT: " Patchwork
2017-10-27 16:39 ` ✗ Fi.CI.BAT: failure " Patchwork

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox