All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/12] drm/i915: Eliminate DDI encoder->type frobbery
@ 2016-06-08 10:41 ville.syrjala
  2016-06-08 10:41 ` [PATCH 01/12] drm/i915: Don't mark eDP encoders as MST capable ville.syrjala
                   ` (12 more replies)
  0 siblings, 13 replies; 28+ messages in thread
From: ville.syrjala @ 2016-06-08 10:41 UTC (permalink / raw)
  To: intel-gfx

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

DDI encoders changing encoder->type at random points in time is a
bit of a problem. Earlier I posted a series [1] to split DDI encoders
into separate HDMI and DP encoders, but now that LSPCON is coming
we'll anyway need some way to change the encoder role dynamically.
So I figured I'd take a step back from the split DDI encoers, and
instead try to do the role switching in a more controlled fashion.
I'm still a bit tempted to finish the encoder split for DDI, but
that should at least wait until we figure out LSPCON.

I also briefly considered that we might have two encoders and one
connector for LSPCON, but I suspect that might require some bigger
surgery in the hotplug code since that tries to deal in encoders
for whatever reason.

I came up with the output_types bitmask idea originally just to
eliminate all the silly encoder type related loops in the DPLL
code (as part of my lvds_downclock branch). But recently I realized
that it could help with DDI/LSPCON as well, and so here we are.

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

[1] https://lists.freedesktop.org/archives/intel-gfx/2015-December/082384.html

Ville Syrjälä (12):
  drm/i915: Don't mark eDP encoders as MST capable
  drm/i915: Remove encoder type checks from MST suspend/resume
  drm/i915: Add output_types bitmask into the crtc state
  drm/i915: Unify intel_pipe_has_type() and intel_pipe_will_have_type()
  drm/i915: Replace manual lvds and sdvo/hdmi counting with
    intel_crtc_has_type()
  drm/i915: Kill has_dp_encoder from pipe_config
  drm/i915: Replace some open coded intel_crtc_has_dp_encoder()s
  drm/i915: s/INTEL_OUTPUT_DISPLAYPORT/INTEL_OUTPUT_DP/
  drm/i915: Kill has_dsi_encoder
  drm/i915: Simplify hdmi_12bpc_possible()
  drm/i915: Check for invalid cloning earlier during modeset
  drm/i915: Stop frobbing with DDI encoder->type

 drivers/gpu/drm/i915/i915_debugfs.c   |   8 +-
 drivers/gpu/drm/i915/intel_audio.c    |  15 +-
 drivers/gpu/drm/i915/intel_color.c    |   2 +-
 drivers/gpu/drm/i915/intel_ddi.c      | 207 ++++++++++++++---------
 drivers/gpu/drm/i915/intel_display.c  | 309 ++++++++++++++--------------------
 drivers/gpu/drm/i915/intel_dp.c       |  51 ++----
 drivers/gpu/drm/i915/intel_dp_mst.c   |   3 -
 drivers/gpu/drm/i915/intel_dpll_mgr.c |  21 +--
 drivers/gpu/drm/i915/intel_drv.h      |  20 ++-
 drivers/gpu/drm/i915/intel_dsi.c      |   4 -
 drivers/gpu/drm/i915/intel_hdmi.c     |  30 +---
 drivers/gpu/drm/i915/intel_opregion.c |   4 +-
 12 files changed, 305 insertions(+), 369 deletions(-)

-- 
2.7.4

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

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

* [PATCH 01/12] drm/i915: Don't mark eDP encoders as MST capable
  2016-06-08 10:41 [PATCH 00/12] drm/i915: Eliminate DDI encoder->type frobbery ville.syrjala
@ 2016-06-08 10:41 ` ville.syrjala
  2016-06-16 12:27   ` Sharma, Shashank
  2016-06-16 20:35   ` Dave Airlie
  2016-06-08 10:41 ` [PATCH 02/12] drm/i915: Remove encoder type checks from MST suspend/resume ville.syrjala
                   ` (11 subsequent siblings)
  12 siblings, 2 replies; 28+ messages in thread
From: ville.syrjala @ 2016-06-08 10:41 UTC (permalink / raw)
  To: intel-gfx; +Cc: Dave Airlie

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

If we've determined that the encoder is eDP, we shouldn't try to use MST
on it. Or at least the code doesn't seem to expect that since there are
some type==DP checks in the MST code.

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

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index f97cd5305e4c..0ab4f319f88f 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5528,7 +5528,7 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 		goto fail;
 
 	/* init MST on ports that can support it */
-	if (HAS_DP_MST(dev) &&
+	if (HAS_DP_MST(dev) && !is_edp(intel_dp) &&
 	    (port == PORT_B || port == PORT_C || port == PORT_D))
 		intel_dp_mst_encoder_init(intel_dig_port,
 					  intel_connector->base.base.id);
-- 
2.7.4

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

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

* [PATCH 02/12] drm/i915: Remove encoder type checks from MST suspend/resume
  2016-06-08 10:41 [PATCH 00/12] drm/i915: Eliminate DDI encoder->type frobbery ville.syrjala
  2016-06-08 10:41 ` [PATCH 01/12] drm/i915: Don't mark eDP encoders as MST capable ville.syrjala
@ 2016-06-08 10:41 ` ville.syrjala
  2016-06-16 12:41   ` Sharma, Shashank
  2016-06-08 10:41 ` [PATCH 03/12] drm/i915: Add output_types bitmask into the crtc state ville.syrjala
                   ` (10 subsequent siblings)
  12 siblings, 1 reply; 28+ messages in thread
From: ville.syrjala @ 2016-06-08 10:41 UTC (permalink / raw)
  To: intel-gfx; +Cc: Dave Airlie

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

Now that eDP encoders won't have can_mst==true, we can throw out
the encoder type checks from the MST suspend/resume paths.

Cc: Dave Airlie <airlied@redhat.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 28 ++++++++++------------------
 1 file changed, 10 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 0ab4f319f88f..29fb0d907f7b 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5659,15 +5659,12 @@ void intel_dp_mst_suspend(struct drm_device *dev)
 	/* disable MST */
 	for (i = 0; i < I915_MAX_PORTS; i++) {
 		struct intel_digital_port *intel_dig_port = dev_priv->hotplug.irq_port[i];
-		if (!intel_dig_port)
+
+		if (!intel_dig_port || !intel_dig_port->dp.can_mst)
 			continue;
 
-		if (intel_dig_port->base.type == INTEL_OUTPUT_DISPLAYPORT) {
-			if (!intel_dig_port->dp.can_mst)
-				continue;
-			if (intel_dig_port->dp.is_mst)
-				drm_dp_mst_topology_mgr_suspend(&intel_dig_port->dp.mst_mgr);
-		}
+		if (intel_dig_port->dp.is_mst)
+			drm_dp_mst_topology_mgr_suspend(&intel_dig_port->dp.mst_mgr);
 	}
 }
 
@@ -5678,18 +5675,13 @@ void intel_dp_mst_resume(struct drm_device *dev)
 
 	for (i = 0; i < I915_MAX_PORTS; i++) {
 		struct intel_digital_port *intel_dig_port = dev_priv->hotplug.irq_port[i];
-		if (!intel_dig_port)
-			continue;
-		if (intel_dig_port->base.type == INTEL_OUTPUT_DISPLAYPORT) {
-			int ret;
+		int ret;
 
-			if (!intel_dig_port->dp.can_mst)
-				continue;
+		if (!intel_dig_port || !intel_dig_port->dp.can_mst)
+			continue;
 
-			ret = drm_dp_mst_topology_mgr_resume(&intel_dig_port->dp.mst_mgr);
-			if (ret != 0) {
-				intel_dp_check_mst_status(&intel_dig_port->dp);
-			}
-		}
+		ret = drm_dp_mst_topology_mgr_resume(&intel_dig_port->dp.mst_mgr);
+		if (ret)
+			intel_dp_check_mst_status(&intel_dig_port->dp);
 	}
 }
-- 
2.7.4

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

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

* [PATCH 03/12] drm/i915: Add output_types bitmask into the crtc state
  2016-06-08 10:41 [PATCH 00/12] drm/i915: Eliminate DDI encoder->type frobbery ville.syrjala
  2016-06-08 10:41 ` [PATCH 01/12] drm/i915: Don't mark eDP encoders as MST capable ville.syrjala
  2016-06-08 10:41 ` [PATCH 02/12] drm/i915: Remove encoder type checks from MST suspend/resume ville.syrjala
@ 2016-06-08 10:41 ` ville.syrjala
  2016-06-08 13:25   ` Chris Wilson
  2016-06-13 14:25   ` Daniel Vetter
  2016-06-08 10:41 ` [PATCH 04/12] drm/i915: Unify intel_pipe_has_type() and intel_pipe_will_have_type() ville.syrjala
                   ` (9 subsequent siblings)
  12 siblings, 2 replies; 28+ messages in thread
From: ville.syrjala @ 2016-06-08 10:41 UTC (permalink / raw)
  To: intel-gfx

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

Rather than looping through encoders to see which encoder types
are being driven by the pipe, add an output_types bitmask into
the crtc state and populate it prior to compute_config and during
state readout.

v2: Determine output_types before .compute_config() hooks are called

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 51 +++++++++++++++---------------------
 drivers/gpu/drm/i915/intel_drv.h     |  5 ++++
 2 files changed, 26 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 12ff79594bc1..eabace447b1c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -535,14 +535,7 @@ needs_modeset(struct drm_crtc_state *state)
  */
 bool intel_pipe_has_type(struct intel_crtc *crtc, enum intel_output_type type)
 {
-	struct drm_device *dev = crtc->base.dev;
-	struct intel_encoder *encoder;
-
-	for_each_encoder_on_crtc(dev, &crtc->base, encoder)
-		if (encoder->type == type)
-			return true;
-
-	return false;
+	return crtc->config->output_types & (1 << type);
 }
 
 /**
@@ -552,28 +545,9 @@ bool intel_pipe_has_type(struct intel_crtc *crtc, enum intel_output_type type)
  * encoder->crtc.
  */
 static bool intel_pipe_will_have_type(const struct intel_crtc_state *crtc_state,
-				      int type)
+				      enum intel_output_type type)
 {
-	struct drm_atomic_state *state = crtc_state->base.state;
-	struct drm_connector *connector;
-	struct drm_connector_state *connector_state;
-	struct intel_encoder *encoder;
-	int i, num_connectors = 0;
-
-	for_each_connector_in_state(state, connector, connector_state, i) {
-		if (connector_state->crtc != crtc_state->base.crtc)
-			continue;
-
-		num_connectors++;
-
-		encoder = to_intel_encoder(connector_state->best_encoder);
-		if (encoder->type == type)
-			return true;
-	}
-
-	WARN_ON(num_connectors == 0);
-
-	return false;
+	return crtc_state->output_types & (1 << type);
 }
 
 /*
@@ -12529,6 +12503,19 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
 			       &pipe_config->pipe_src_w,
 			       &pipe_config->pipe_src_h);
 
+	for_each_connector_in_state(state, connector, connector_state, i) {
+		if (connector_state->crtc != crtc)
+			continue;
+
+		encoder = to_intel_encoder(connector_state->best_encoder);
+
+		/*
+		 * 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;
+	}
+
 encoder_retry:
 	/* Ensure the port clock defaults are reset when retrying. */
 	pipe_config->port_clock = 0;
@@ -12826,6 +12813,7 @@ intel_pipe_config_compare(struct drm_device *dev,
 		PIPE_CONF_CHECK_M_N_ALT(dp_m_n, dp_m2_n2);
 
 	PIPE_CONF_CHECK_I(has_dsi_encoder);
+	PIPE_CONF_CHECK_X(output_types);
 
 	PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_hdisplay);
 	PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_htotal);
@@ -13093,8 +13081,10 @@ verify_crtc_state(struct drm_crtc *crtc,
 				"Encoder connected to wrong pipe %c\n",
 				pipe_name(pipe));
 
-		if (active)
+		if (active) {
+			pipe_config->output_types |= 1 << encoder->type;
 			encoder->get_config(encoder, pipe_config);
+		}
 	}
 
 	if (!new_crtc_state->active)
@@ -15985,6 +15975,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
 		if (encoder->get_hw_state(encoder, &pipe)) {
 			crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
 			encoder->base.crtc = &crtc->base;
+			crtc->config->output_types |= 1 << encoder->type;
 			encoder->get_config(encoder, crtc->config);
 		} else {
 			encoder->base.crtc = NULL;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index ebe7b3427e2e..3dae05e6d95b 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -512,6 +512,11 @@ struct intel_crtc_state {
 	/* DSI has special cases */
 	bool has_dsi_encoder;
 
+	/* Bitmask of encoder types (enum intel_output_type)
+	 * driven by the pipe.
+	 */
+	unsigned int output_types;
+
 	/* Whether we should send NULL infoframes. Required for audio. */
 	bool has_hdmi_sink;
 
-- 
2.7.4

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

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

* [PATCH 04/12] drm/i915: Unify intel_pipe_has_type() and intel_pipe_will_have_type()
  2016-06-08 10:41 [PATCH 00/12] drm/i915: Eliminate DDI encoder->type frobbery ville.syrjala
                   ` (2 preceding siblings ...)
  2016-06-08 10:41 ` [PATCH 03/12] drm/i915: Add output_types bitmask into the crtc state ville.syrjala
@ 2016-06-08 10:41 ` ville.syrjala
  2016-06-08 10:41 ` [PATCH 05/12] drm/i915: Replace manual lvds and sdvo/hdmi counting with intel_crtc_has_type() ville.syrjala
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 28+ messages in thread
From: ville.syrjala @ 2016-06-08 10:41 UTC (permalink / raw)
  To: intel-gfx

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

With the introduction of the output_types mask, intel_pipe_has_type()
and intel_pipe_will_have_type() are basically the same thing. Replace
them with a new intel_crtc_has_type() (identical to
intel_pipe_will_have_type() actually).

v2: Rebase

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_audio.c   | 12 +++---
 drivers/gpu/drm/i915/intel_display.c | 84 ++++++++++++++++--------------------
 drivers/gpu/drm/i915/intel_drv.h     |  3 +-
 3 files changed, 44 insertions(+), 55 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index b9329c2a670a..da49d6cead9e 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -154,7 +154,7 @@ static bool audio_rate_need_prog(struct intel_crtc *crtc,
 {
 	if (((mode->clock == TMDS_297M) ||
 		 (mode->clock == TMDS_296M)) &&
-		intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI))
+		intel_crtc_has_type(crtc->config, INTEL_OUTPUT_HDMI))
 		return true;
 	else
 		return false;
@@ -262,7 +262,7 @@ static void hsw_audio_codec_disable(struct intel_encoder *encoder)
 	tmp |= AUD_CONFIG_N_PROG_ENABLE;
 	tmp &= ~AUD_CONFIG_UPPER_N_MASK;
 	tmp &= ~AUD_CONFIG_LOWER_N_MASK;
-	if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DISPLAYPORT))
+	if (intel_crtc_has_type(intel_crtc->config, INTEL_OUTPUT_DISPLAYPORT))
 		tmp |= AUD_CONFIG_N_VALUE_INDEX;
 	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
 
@@ -328,7 +328,7 @@ static void hsw_audio_codec_enable(struct drm_connector *connector,
 	tmp = I915_READ(HSW_AUD_CFG(pipe));
 	tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
 	tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK;
-	if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DISPLAYPORT))
+	if (intel_crtc_has_type(intel_crtc->config, INTEL_OUTPUT_DISPLAYPORT))
 		tmp |= AUD_CONFIG_N_VALUE_INDEX;
 	else
 		tmp |= audio_config_hdmi_pixel_clock(adjusted_mode);
@@ -389,7 +389,7 @@ static void ilk_audio_codec_disable(struct intel_encoder *encoder)
 	tmp |= AUD_CONFIG_N_PROG_ENABLE;
 	tmp &= ~AUD_CONFIG_UPPER_N_MASK;
 	tmp &= ~AUD_CONFIG_LOWER_N_MASK;
-	if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DISPLAYPORT))
+	if (intel_crtc_has_type(intel_crtc->config, INTEL_OUTPUT_DISPLAYPORT))
 		tmp |= AUD_CONFIG_N_VALUE_INDEX;
 	I915_WRITE(aud_config, tmp);
 
@@ -475,7 +475,7 @@ static void ilk_audio_codec_enable(struct drm_connector *connector,
 	tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
 	tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
 	tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK;
-	if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DISPLAYPORT))
+	if (intel_crtc_has_type(intel_crtc->config, INTEL_OUTPUT_DISPLAYPORT))
 		tmp |= AUD_CONFIG_N_VALUE_INDEX;
 	else
 		tmp |= audio_config_hdmi_pixel_clock(adjusted_mode);
@@ -513,7 +513,7 @@ void intel_audio_codec_enable(struct intel_encoder *intel_encoder)
 
 	/* ELD Conn_Type */
 	connector->eld[5] &= ~(3 << 2);
-	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_DISPLAYPORT))
+	if (intel_crtc_has_type(crtc->config, INTEL_OUTPUT_DISPLAYPORT))
 		connector->eld[5] |= (1 << 2);
 
 	connector->eld[6] = drm_av_sync_delay(connector, adjusted_mode) / 2;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index eabace447b1c..20d6c31635b3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -530,22 +530,9 @@ needs_modeset(struct drm_crtc_state *state)
 	return drm_atomic_crtc_needs_modeset(state);
 }
 
-/**
- * Returns whether any output on the specified pipe is of the specified type
- */
-bool intel_pipe_has_type(struct intel_crtc *crtc, enum intel_output_type type)
-{
-	return crtc->config->output_types & (1 << type);
-}
-
-/**
- * Returns whether any output on the specified pipe will have the specified
- * type after a staged modeset is complete, i.e., the same as
- * intel_pipe_has_type() but looking at encoder->new_crtc instead of
- * encoder->crtc.
- */
-static bool intel_pipe_will_have_type(const struct intel_crtc_state *crtc_state,
-				      enum intel_output_type type)
+/* Returns whether any output on the specified pipe is of the specified type */
+bool intel_crtc_has_type(const struct intel_crtc_state *crtc_state,
+			 enum intel_output_type type)
 {
 	return crtc_state->output_types & (1 << type);
 }
@@ -662,7 +649,7 @@ i9xx_select_p2_div(const struct intel_limit *limit,
 {
 	struct drm_device *dev = crtc_state->base.crtc->dev;
 
-	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
+	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_LVDS)) {
 		/*
 		 * For LVDS just rely on its current settings for dual-channel.
 		 * We haven't figured out how to reliably set up different
@@ -1613,9 +1600,10 @@ static int intel_num_dvo_pipes(struct drm_device *dev)
 	struct intel_crtc *crtc;
 	int count = 0;
 
-	for_each_intel_crtc(dev, crtc)
+	for_each_intel_crtc(dev, crtc) {
 		count += crtc->base.state->active &&
-			intel_pipe_has_type(crtc, INTEL_OUTPUT_DVO);
+			intel_crtc_has_type(crtc->config, INTEL_OUTPUT_DVO);
+	}
 
 	return count;
 }
@@ -1700,7 +1688,7 @@ static void i9xx_disable_pll(struct intel_crtc *crtc)
 
 	/* Disable DVO 2x clock on both PLLs if necessary */
 	if (IS_I830(dev) &&
-	    intel_pipe_has_type(crtc, INTEL_OUTPUT_DVO) &&
+	    intel_crtc_has_type(crtc->config, INTEL_OUTPUT_DVO) &&
 	    !intel_num_dvo_pipes(dev)) {
 		I915_WRITE(DPLL(PIPE_B),
 			   I915_READ(DPLL(PIPE_B)) & ~DPLL_DVO_2X_MODE);
@@ -1828,7 +1816,7 @@ static void ironlake_enable_pch_transcoder(struct drm_i915_private *dev_priv,
 		 * here for both 8bpc and 12bpc.
 		 */
 		val &= ~PIPECONF_BPC_MASK;
-		if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_HDMI))
+		if (intel_crtc_has_type(intel_crtc->config, INTEL_OUTPUT_HDMI))
 			val |= PIPECONF_8BPC;
 		else
 			val |= pipeconf_val & PIPECONF_BPC_MASK;
@@ -1837,7 +1825,7 @@ static void ironlake_enable_pch_transcoder(struct drm_i915_private *dev_priv,
 	val &= ~TRANS_INTERLACE_MASK;
 	if ((pipeconf_val & PIPECONF_INTERLACE_MASK) == PIPECONF_INTERLACED_ILK)
 		if (HAS_PCH_IBX(dev_priv) &&
-		    intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_SDVO))
+		    intel_crtc_has_type(intel_crtc->config, INTEL_OUTPUT_SDVO))
 			val |= TRANS_LEGACY_INTERLACED_ILK;
 		else
 			val |= TRANS_INTERLACED;
@@ -6630,7 +6618,7 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
 	 * - LVDS dual channel mode
 	 * - Double wide pipe
 	 */
-	if ((intel_pipe_will_have_type(pipe_config, INTEL_OUTPUT_LVDS) &&
+	if ((intel_crtc_has_type(pipe_config, INTEL_OUTPUT_LVDS) &&
 	     intel_is_dual_link_lvds(dev)) || pipe_config->double_wide)
 		pipe_config->pipe_src_w &= ~1;
 
@@ -7141,7 +7129,7 @@ static void i9xx_update_pll_dividers(struct intel_crtc *crtc,
 	crtc_state->dpll_hw_state.fp0 = fp;
 
 	crtc->lowfreq_avail = false;
-	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
+	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_LVDS) &&
 	    reduced_clock) {
 		crtc_state->dpll_hw_state.fp1 = fp2;
 		crtc->lowfreq_avail = true;
@@ -7347,8 +7335,8 @@ static void vlv_prepare_pll(struct intel_crtc *crtc,
 
 	/* Set HBR and RBR LPF coefficients */
 	if (pipe_config->port_clock == 162000 ||
-	    intel_pipe_has_type(crtc, INTEL_OUTPUT_ANALOG) ||
-	    intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI))
+	    intel_crtc_has_type(crtc->config, INTEL_OUTPUT_ANALOG) ||
+	    intel_crtc_has_type(crtc->config, INTEL_OUTPUT_HDMI))
 		vlv_dpio_write(dev_priv, pipe, VLV_PLL_DW10(pipe),
 				 0x009f0003);
 	else
@@ -7375,8 +7363,8 @@ static void vlv_prepare_pll(struct intel_crtc *crtc,
 
 	coreclk = vlv_dpio_read(dev_priv, pipe, VLV_PLL_DW7(pipe));
 	coreclk = (coreclk & 0x0000ff00) | 0x01c00000;
-	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_DISPLAYPORT) ||
-	    intel_pipe_has_type(crtc, INTEL_OUTPUT_EDP))
+	if (intel_crtc_has_type(crtc->config, INTEL_OUTPUT_DISPLAYPORT) ||
+	    intel_crtc_has_type(crtc->config, INTEL_OUTPUT_EDP))
 		coreclk |= 0x01000000;
 	vlv_dpio_write(dev_priv, pipe, VLV_PLL_DW7(pipe), coreclk);
 
@@ -7557,12 +7545,12 @@ static void i9xx_compute_dpll(struct intel_crtc *crtc,
 
 	i9xx_update_pll_dividers(crtc, crtc_state, reduced_clock);
 
-	is_sdvo = intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_SDVO) ||
-		intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_HDMI);
+	is_sdvo = intel_crtc_has_type(crtc_state, INTEL_OUTPUT_SDVO) ||
+		intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI);
 
 	dpll = DPLL_VGA_MODE_DIS;
 
-	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS))
+	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_LVDS))
 		dpll |= DPLLB_MODE_LVDS;
 	else
 		dpll |= DPLLB_MODE_DAC_SERIAL;
@@ -7605,7 +7593,7 @@ static void i9xx_compute_dpll(struct intel_crtc *crtc,
 
 	if (crtc_state->sdvo_tv_clock)
 		dpll |= PLL_REF_INPUT_TVCLKINBC;
-	else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
+	else if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_LVDS) &&
 		 intel_panel_use_ssc(dev_priv))
 		dpll |= PLLB_REF_INPUT_SPREADSPECTRUMIN;
 	else
@@ -7634,7 +7622,7 @@ static void i8xx_compute_dpll(struct intel_crtc *crtc,
 
 	dpll = DPLL_VGA_MODE_DIS;
 
-	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
+	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_LVDS)) {
 		dpll |= (1 << (clock->p1 - 1)) << DPLL_FPA01_P1_POST_DIV_SHIFT;
 	} else {
 		if (clock->p1 == 2)
@@ -7645,10 +7633,10 @@ static void i8xx_compute_dpll(struct intel_crtc *crtc,
 			dpll |= PLL_P2_DIVIDE_BY_4;
 	}
 
-	if (!IS_I830(dev) && intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_DVO))
+	if (!IS_I830(dev) && intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DVO))
 		dpll |= DPLL_DVO_2X_MODE;
 
-	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
+	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_LVDS) &&
 	    intel_panel_use_ssc(dev_priv))
 		dpll |= PLLB_REF_INPUT_SPREADSPECTRUMIN;
 	else
@@ -7678,7 +7666,7 @@ static void intel_set_pipe_timings(struct intel_crtc *intel_crtc)
 		crtc_vtotal -= 1;
 		crtc_vblank_end -= 1;
 
-		if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_SDVO))
+		if (intel_crtc_has_type(intel_crtc->config, INTEL_OUTPUT_SDVO))
 			vsyncshift = (adjusted_mode->crtc_htotal - 1) / 2;
 		else
 			vsyncshift = adjusted_mode->crtc_hsync_start -
@@ -7857,7 +7845,7 @@ static void i9xx_set_pipeconf(struct intel_crtc *intel_crtc)
 
 	if (intel_crtc->config->base.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE) {
 		if (INTEL_INFO(dev)->gen < 4 ||
-		    intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_SDVO))
+		    intel_crtc_has_type(intel_crtc->config, INTEL_OUTPUT_SDVO))
 			pipeconf |= PIPECONF_INTERLACE_W_FIELD_INDICATION;
 		else
 			pipeconf |= PIPECONF_INTERLACE_W_SYNC_SHIFT;
@@ -7883,14 +7871,14 @@ static int i8xx_crtc_compute_clock(struct intel_crtc *crtc,
 	memset(&crtc_state->dpll_hw_state, 0,
 	       sizeof(crtc_state->dpll_hw_state));
 
-	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
+	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_LVDS)) {
 		if (intel_panel_use_ssc(dev_priv)) {
 			refclk = dev_priv->vbt.lvds_ssc_freq;
 			DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n", refclk);
 		}
 
 		limit = &intel_limits_i8xx_lvds;
-	} else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_DVO)) {
+	} else if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DVO)) {
 		limit = &intel_limits_i8xx_dvo;
 	} else {
 		limit = &intel_limits_i8xx_dac;
@@ -7919,7 +7907,7 @@ static int g4x_crtc_compute_clock(struct intel_crtc *crtc,
 	memset(&crtc_state->dpll_hw_state, 0,
 	       sizeof(crtc_state->dpll_hw_state));
 
-	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
+	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_LVDS)) {
 		if (intel_panel_use_ssc(dev_priv)) {
 			refclk = dev_priv->vbt.lvds_ssc_freq;
 			DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n", refclk);
@@ -7929,10 +7917,10 @@ static int g4x_crtc_compute_clock(struct intel_crtc *crtc,
 			limit = &intel_limits_g4x_dual_channel_lvds;
 		else
 			limit = &intel_limits_g4x_single_channel_lvds;
-	} else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_HDMI) ||
-		   intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_ANALOG)) {
+	} else if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI) ||
+		   intel_crtc_has_type(crtc_state, INTEL_OUTPUT_ANALOG)) {
 		limit = &intel_limits_g4x_hdmi;
-	} else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_SDVO)) {
+	} else if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_SDVO)) {
 		limit = &intel_limits_g4x_sdvo;
 	} else {
 		/* The option is for other outputs */
@@ -7962,7 +7950,7 @@ static int pnv_crtc_compute_clock(struct intel_crtc *crtc,
 	memset(&crtc_state->dpll_hw_state, 0,
 	       sizeof(crtc_state->dpll_hw_state));
 
-	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
+	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_LVDS)) {
 		if (intel_panel_use_ssc(dev_priv)) {
 			refclk = dev_priv->vbt.lvds_ssc_freq;
 			DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n", refclk);
@@ -7996,7 +7984,7 @@ static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
 	memset(&crtc_state->dpll_hw_state, 0,
 	       sizeof(crtc_state->dpll_hw_state));
 
-	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
+	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_LVDS)) {
 		if (intel_panel_use_ssc(dev_priv)) {
 			refclk = dev_priv->vbt.lvds_ssc_freq;
 			DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n", refclk);
@@ -9000,7 +8988,7 @@ static int ironlake_crtc_compute_clock(struct intel_crtc *crtc,
 	if (!crtc_state->has_pch_encoder)
 		return 0;
 
-	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
+	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_LVDS)) {
 		if (intel_panel_use_ssc(dev_priv)) {
 			DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n",
 				      dev_priv->vbt.lvds_ssc_freq);
@@ -9039,7 +9027,7 @@ static int ironlake_crtc_compute_clock(struct intel_crtc *crtc,
 		return -EINVAL;
 	}
 
-	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
+	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_LVDS) &&
 	    has_reduced_clock)
 		crtc->lowfreq_avail = true;
 
@@ -13249,7 +13237,7 @@ static void update_scanline_offset(struct intel_crtc *crtc)
 
 		crtc->scanline_offset = vtotal - 1;
 	} else if (HAS_DDI(dev) &&
-		   intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI)) {
+		   intel_crtc_has_type(crtc->config, INTEL_OUTPUT_HDMI)) {
 		crtc->scanline_offset = 2;
 	} else
 		crtc->scanline_offset = 1;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 3dae05e6d95b..ed5c708858cd 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1167,7 +1167,8 @@ int intel_get_pipe_from_crtc_id(struct drm_device *dev, void *data,
 				struct drm_file *file_priv);
 enum transcoder intel_pipe_to_cpu_transcoder(struct drm_i915_private *dev_priv,
 					     enum pipe pipe);
-bool intel_pipe_has_type(struct intel_crtc *crtc, enum intel_output_type type);
+bool intel_crtc_has_type(const struct intel_crtc_state *crtc_state,
+			 enum intel_output_type type);
 static inline void
 intel_wait_for_vblank(struct drm_device *dev, int pipe)
 {
-- 
2.7.4

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

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

* [PATCH 05/12] drm/i915: Replace manual lvds and sdvo/hdmi counting with intel_crtc_has_type()
  2016-06-08 10:41 [PATCH 00/12] drm/i915: Eliminate DDI encoder->type frobbery ville.syrjala
                   ` (3 preceding siblings ...)
  2016-06-08 10:41 ` [PATCH 04/12] drm/i915: Unify intel_pipe_has_type() and intel_pipe_will_have_type() ville.syrjala
@ 2016-06-08 10:41 ` ville.syrjala
  2016-06-08 10:41 ` [PATCH 06/12] drm/i915: Kill has_dp_encoder from pipe_config ville.syrjala
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 28+ messages in thread
From: ville.syrjala @ 2016-06-08 10:41 UTC (permalink / raw)
  To: intel-gfx

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

Since we now have the output_types bitmaks in the crtc state, there's no
need to iterate through all the encoders to see if an LVDS or SDVO/HDMI
encoder might be present.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 20d6c31635b3..6b92c4a7d50f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7540,14 +7540,10 @@ static void i9xx_compute_dpll(struct intel_crtc *crtc,
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	u32 dpll;
-	bool is_sdvo;
 	struct dpll *clock = &crtc_state->dpll;
 
 	i9xx_update_pll_dividers(crtc, crtc_state, reduced_clock);
 
-	is_sdvo = intel_crtc_has_type(crtc_state, INTEL_OUTPUT_SDVO) ||
-		intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI);
-
 	dpll = DPLL_VGA_MODE_DIS;
 
 	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_LVDS))
@@ -7560,7 +7556,8 @@ static void i9xx_compute_dpll(struct intel_crtc *crtc,
 			<< SDVO_MULTIPLIER_SHIFT_HIRES;
 	}
 
-	if (is_sdvo)
+	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_SDVO) ||
+	    intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
 		dpll |= DPLL_SDVO_HIGH_SPEED;
 
 	if (crtc_state->has_dp_encoder)
@@ -8870,36 +8867,12 @@ static void ironlake_compute_dpll(struct intel_crtc *intel_crtc,
 	struct drm_crtc *crtc = &intel_crtc->base;
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct drm_atomic_state *state = crtc_state->base.state;
-	struct drm_connector *connector;
-	struct drm_connector_state *connector_state;
-	struct intel_encoder *encoder;
 	u32 dpll, fp, fp2;
-	int factor, i;
-	bool is_lvds = false, is_sdvo = false;
-
-	for_each_connector_in_state(state, connector, connector_state, i) {
-		if (connector_state->crtc != crtc_state->base.crtc)
-			continue;
-
-		encoder = to_intel_encoder(connector_state->best_encoder);
-
-		switch (encoder->type) {
-		case INTEL_OUTPUT_LVDS:
-			is_lvds = true;
-			break;
-		case INTEL_OUTPUT_SDVO:
-		case INTEL_OUTPUT_HDMI:
-			is_sdvo = true;
-			break;
-		default:
-			break;
-		}
-	}
+	int factor;
 
 	/* Enable autotuning of the PLL clock (if permissible) */
 	factor = 21;
-	if (is_lvds) {
+	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_LVDS)) {
 		if ((intel_panel_use_ssc(dev_priv) &&
 		     dev_priv->vbt.lvds_ssc_freq == 100000) ||
 		    (HAS_PCH_IBX(dev) && intel_is_dual_link_lvds(dev)))
@@ -8923,7 +8896,7 @@ static void ironlake_compute_dpll(struct intel_crtc *intel_crtc,
 
 	dpll = 0;
 
-	if (is_lvds)
+	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_LVDS))
 		dpll |= DPLLB_MODE_LVDS;
 	else
 		dpll |= DPLLB_MODE_DAC_SERIAL;
@@ -8931,8 +8904,10 @@ static void ironlake_compute_dpll(struct intel_crtc *intel_crtc,
 	dpll |= (crtc_state->pixel_multiplier - 1)
 		<< PLL_REF_SDVO_HDMI_MULTIPLIER_SHIFT;
 
-	if (is_sdvo)
+	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_SDVO) ||
+	    intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
 		dpll |= DPLL_SDVO_HIGH_SPEED;
+
 	if (crtc_state->has_dp_encoder)
 		dpll |= DPLL_SDVO_HIGH_SPEED;
 
@@ -8956,7 +8931,8 @@ static void ironlake_compute_dpll(struct intel_crtc *intel_crtc,
 		break;
 	}
 
-	if (is_lvds && intel_panel_use_ssc(dev_priv))
+	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_LVDS) &&
+	    intel_panel_use_ssc(dev_priv))
 		dpll |= PLLB_REF_INPUT_SPREADSPECTRUMIN;
 	else
 		dpll |= PLL_REF_INPUT_DREFCLK;
-- 
2.7.4

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

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

* [PATCH 06/12] drm/i915: Kill has_dp_encoder from pipe_config
  2016-06-08 10:41 [PATCH 00/12] drm/i915: Eliminate DDI encoder->type frobbery ville.syrjala
                   ` (4 preceding siblings ...)
  2016-06-08 10:41 ` [PATCH 05/12] drm/i915: Replace manual lvds and sdvo/hdmi counting with intel_crtc_has_type() ville.syrjala
@ 2016-06-08 10:41 ` ville.syrjala
  2016-06-08 10:41 ` [PATCH 07/12] drm/i915: Replace some open coded intel_crtc_has_dp_encoder()s ville.syrjala
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 28+ messages in thread
From: ville.syrjala @ 2016-06-08 10:41 UTC (permalink / raw)
  To: intel-gfx

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

Use the new output_types bitmask instead of has_dp_encoder.
To make it less oainlful provide a small helper
(intel_crtc_has_dp_encoder()) to do the bitsy stuff.

v2: Rebase

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c     |  3 +--
 drivers/gpu/drm/i915/intel_display.c | 29 ++++++++++++++++++-----------
 drivers/gpu/drm/i915/intel_dp.c      |  3 ---
 drivers/gpu/drm/i915/intel_dp_mst.c  |  3 ---
 drivers/gpu/drm/i915/intel_drv.h     |  5 +----
 5 files changed, 20 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 022b41d422dc..b7285336fb53 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -834,7 +834,7 @@ static void ddi_dotclock_get(struct intel_crtc_state *pipe_config)
 	if (pipe_config->has_pch_encoder)
 		dotclock = intel_dotclock_calculate(pipe_config->port_clock,
 						    &pipe_config->fdi_m_n);
-	else if (pipe_config->has_dp_encoder)
+	else if (intel_crtc_has_dp_encoder(pipe_config))
 		dotclock = intel_dotclock_calculate(pipe_config->port_clock,
 						    &pipe_config->dp_m_n);
 	else if (pipe_config->has_hdmi_sink && pipe_config->pipe_bpp == 36)
@@ -2200,7 +2200,6 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
 		break;
 	case TRANS_DDI_MODE_SELECT_DP_SST:
 	case TRANS_DDI_MODE_SELECT_DP_MST:
-		pipe_config->has_dp_encoder = true;
 		pipe_config->lane_count =
 			((temp & DDI_PORT_WIDTH_MASK) >> DDI_PORT_WIDTH_SHIFT) + 1;
 		intel_dp_get_m_n(intel_crtc, pipe_config);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 6b92c4a7d50f..2388bfe1b925 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -537,6 +537,14 @@ bool intel_crtc_has_type(const struct intel_crtc_state *crtc_state,
 	return crtc_state->output_types & (1 << type);
 }
 
+bool intel_crtc_has_dp_encoder(const struct intel_crtc_state *crtc_state)
+{
+	return crtc_state->output_types &
+		((1 << INTEL_OUTPUT_DISPLAYPORT) |
+		 (1 << INTEL_OUTPUT_DP_MST) |
+		 (1 << INTEL_OUTPUT_EDP));
+}
+
 /*
  * Platform specific helpers to calculate the port PLL loopback- (clock.m),
  * and post-divider (clock.p) values, pre- (clock.vco) and post-divided fast
@@ -4097,7 +4105,7 @@ static void ironlake_pch_enable(struct drm_crtc *crtc)
 	intel_fdi_normal_train(crtc);
 
 	/* For PCH DP, enable TRANS_DP_CTL */
-	if (HAS_PCH_CPT(dev) && intel_crtc->config->has_dp_encoder) {
+	if (HAS_PCH_CPT(dev) && intel_crtc_has_dp_encoder(intel_crtc->config)) {
 		const struct drm_display_mode *adjusted_mode =
 			&intel_crtc->config->base.adjusted_mode;
 		u32 bpc = (I915_READ(PIPECONF(pipe)) & PIPECONF_BPC_MASK) >> 5;
@@ -4719,7 +4727,7 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
 	if (intel_crtc->config->has_pch_encoder)
 		intel_prepare_shared_dpll(intel_crtc);
 
-	if (intel_crtc->config->has_dp_encoder)
+	if (intel_crtc_has_dp_encoder(intel_crtc->config))
 		intel_dp_set_m_n(intel_crtc, M1_N1);
 
 	intel_set_pipe_timings(intel_crtc);
@@ -4806,7 +4814,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
 	if (intel_crtc->config->shared_dpll)
 		intel_enable_shared_dpll(intel_crtc);
 
-	if (intel_crtc->config->has_dp_encoder)
+	if (intel_crtc_has_dp_encoder(intel_crtc->config))
 		intel_dp_set_m_n(intel_crtc, M1_N1);
 
 	if (!intel_crtc->config->has_dsi_encoder)
@@ -6099,7 +6107,7 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
 	if (WARN_ON(intel_crtc->active))
 		return;
 
-	if (intel_crtc->config->has_dp_encoder)
+	if (intel_crtc_has_dp_encoder(intel_crtc->config))
 		intel_dp_set_m_n(intel_crtc, M1_N1);
 
 	intel_set_pipe_timings(intel_crtc);
@@ -6172,7 +6180,7 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
 
 	i9xx_set_pll_dividers(intel_crtc);
 
-	if (intel_crtc->config->has_dp_encoder)
+	if (intel_crtc_has_dp_encoder(intel_crtc->config))
 		intel_dp_set_m_n(intel_crtc, M1_N1);
 
 	intel_set_pipe_timings(intel_crtc);
@@ -7343,7 +7351,7 @@ static void vlv_prepare_pll(struct intel_crtc *crtc,
 		vlv_dpio_write(dev_priv, pipe, VLV_PLL_DW10(pipe),
 				 0x00d0000f);
 
-	if (pipe_config->has_dp_encoder) {
+	if (intel_crtc_has_dp_encoder(pipe_config)) {
 		/* Use SSC source */
 		if (pipe == PIPE_A)
 			vlv_dpio_write(dev_priv, pipe, VLV_PLL_DW5(pipe),
@@ -7560,7 +7568,7 @@ static void i9xx_compute_dpll(struct intel_crtc *crtc,
 	    intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
 		dpll |= DPLL_SDVO_HIGH_SPEED;
 
-	if (crtc_state->has_dp_encoder)
+	if (intel_crtc_has_dp_encoder(crtc_state))
 		dpll |= DPLL_SDVO_HIGH_SPEED;
 
 	/* compute bitmask from p1 value */
@@ -8908,7 +8916,7 @@ static void ironlake_compute_dpll(struct intel_crtc *intel_crtc,
 	    intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
 		dpll |= DPLL_SDVO_HIGH_SPEED;
 
-	if (crtc_state->has_dp_encoder)
+	if (intel_crtc_has_dp_encoder(crtc_state))
 		dpll |= DPLL_SDVO_HIGH_SPEED;
 
 	/* compute bitmask from p1 value */
@@ -12230,14 +12238,14 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
 		      pipe_config->fdi_m_n.link_m, pipe_config->fdi_m_n.link_n,
 		      pipe_config->fdi_m_n.tu);
 	DRM_DEBUG_KMS("dp: %i, lanes: %i, gmch_m: %u, gmch_n: %u, link_m: %u, link_n: %u, tu: %u\n",
-		      pipe_config->has_dp_encoder,
+		      intel_crtc_has_dp_encoder(pipe_config),
 		      pipe_config->lane_count,
 		      pipe_config->dp_m_n.gmch_m, pipe_config->dp_m_n.gmch_n,
 		      pipe_config->dp_m_n.link_m, pipe_config->dp_m_n.link_n,
 		      pipe_config->dp_m_n.tu);
 
 	DRM_DEBUG_KMS("dp: %i, lanes: %i, gmch_m2: %u, gmch_n2: %u, link_m2: %u, link_n2: %u, tu2: %u\n",
-		      pipe_config->has_dp_encoder,
+		      intel_crtc_has_dp_encoder(pipe_config),
 		      pipe_config->lane_count,
 		      pipe_config->dp_m2_n2.gmch_m,
 		      pipe_config->dp_m2_n2.gmch_n,
@@ -12765,7 +12773,6 @@ intel_pipe_config_compare(struct drm_device *dev,
 	PIPE_CONF_CHECK_I(fdi_lanes);
 	PIPE_CONF_CHECK_M_N(fdi_m_n);
 
-	PIPE_CONF_CHECK_I(has_dp_encoder);
 	PIPE_CONF_CHECK_I(lane_count);
 
 	if (INTEL_INFO(dev)->gen < 8) {
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 29fb0d907f7b..207e82695cfa 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1459,7 +1459,6 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 	if (HAS_PCH_SPLIT(dev) && !HAS_DDI(dev) && port != PORT_A)
 		pipe_config->has_pch_encoder = true;
 
-	pipe_config->has_dp_encoder = true;
 	pipe_config->has_drrs = false;
 	pipe_config->has_audio = intel_dp->has_audio && port != PORT_A;
 
@@ -2395,8 +2394,6 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
 	    !IS_CHERRYVIEW(dev) && tmp & DP_COLOR_RANGE_16_235)
 		pipe_config->limited_color_range = true;
 
-	pipe_config->has_dp_encoder = true;
-
 	pipe_config->lane_count =
 		((tmp & DP_PORT_WIDTH_MASK) >> DP_PORT_WIDTH_SHIFT) + 1;
 
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index f62ca9a126b3..cf94fd460049 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -47,7 +47,6 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder,
 
 	pipe_config->dp_encoder_is_mst = true;
 	pipe_config->has_pch_encoder = false;
-	pipe_config->has_dp_encoder = true;
 	bpp = 24;
 	/*
 	 * for MST we always configure max link bw - the spec doesn't
@@ -243,8 +242,6 @@ 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->has_dp_encoder = true;
-
 	temp = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
 	if (temp & TRANS_DDI_PHSYNC)
 		flags |= DRM_MODE_FLAG_PHSYNC;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index ed5c708858cd..2c70dd677a00 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -505,10 +505,6 @@ struct intel_crtc_state {
 	 */
 	bool limited_color_range;
 
-	/* DP has a bunch of special case unfortunately, so mark the pipe
-	 * accordingly. */
-	bool has_dp_encoder;
-
 	/* DSI has special cases */
 	bool has_dsi_encoder;
 
@@ -1169,6 +1165,7 @@ enum transcoder intel_pipe_to_cpu_transcoder(struct drm_i915_private *dev_priv,
 					     enum pipe pipe);
 bool intel_crtc_has_type(const struct intel_crtc_state *crtc_state,
 			 enum intel_output_type type);
+bool intel_crtc_has_dp_encoder(const struct intel_crtc_state *crtc_state);
 static inline void
 intel_wait_for_vblank(struct drm_device *dev, int pipe)
 {
-- 
2.7.4

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

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

* [PATCH 07/12] drm/i915: Replace some open coded intel_crtc_has_dp_encoder()s
  2016-06-08 10:41 [PATCH 00/12] drm/i915: Eliminate DDI encoder->type frobbery ville.syrjala
                   ` (5 preceding siblings ...)
  2016-06-08 10:41 ` [PATCH 06/12] drm/i915: Kill has_dp_encoder from pipe_config ville.syrjala
@ 2016-06-08 10:41 ` ville.syrjala
  2016-06-08 10:41 ` [PATCH 08/12] drm/i915: s/INTEL_OUTPUT_DISPLAYPORT/INTEL_OUTPUT_DP/ ville.syrjala
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 28+ messages in thread
From: ville.syrjala @ 2016-06-08 10:41 UTC (permalink / raw)
  To: intel-gfx

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

A bunch of places still look for DP encoders manually. Just call
intel_crtc_has_dp_encoder(). Note that many of these places don't
look for EDP or DP_MST, but it's still fine to replace them because
* for audio we don't enable audio on eDP anyway
* the code that lack DP MST check is only for plaforms that
  don't support MST anyway

v2: Rebase

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_audio.c   | 10 +++++-----
 drivers/gpu/drm/i915/intel_display.c |  3 +--
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index da49d6cead9e..0653fb1d9d26 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -262,7 +262,7 @@ static void hsw_audio_codec_disable(struct intel_encoder *encoder)
 	tmp |= AUD_CONFIG_N_PROG_ENABLE;
 	tmp &= ~AUD_CONFIG_UPPER_N_MASK;
 	tmp &= ~AUD_CONFIG_LOWER_N_MASK;
-	if (intel_crtc_has_type(intel_crtc->config, INTEL_OUTPUT_DISPLAYPORT))
+	if (intel_crtc_has_dp_encoder(intel_crtc->config))
 		tmp |= AUD_CONFIG_N_VALUE_INDEX;
 	I915_WRITE(HSW_AUD_CFG(pipe), tmp);
 
@@ -328,7 +328,7 @@ static void hsw_audio_codec_enable(struct drm_connector *connector,
 	tmp = I915_READ(HSW_AUD_CFG(pipe));
 	tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
 	tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK;
-	if (intel_crtc_has_type(intel_crtc->config, INTEL_OUTPUT_DISPLAYPORT))
+	if (intel_crtc_has_dp_encoder(intel_crtc->config))
 		tmp |= AUD_CONFIG_N_VALUE_INDEX;
 	else
 		tmp |= audio_config_hdmi_pixel_clock(adjusted_mode);
@@ -389,7 +389,7 @@ static void ilk_audio_codec_disable(struct intel_encoder *encoder)
 	tmp |= AUD_CONFIG_N_PROG_ENABLE;
 	tmp &= ~AUD_CONFIG_UPPER_N_MASK;
 	tmp &= ~AUD_CONFIG_LOWER_N_MASK;
-	if (intel_crtc_has_type(intel_crtc->config, INTEL_OUTPUT_DISPLAYPORT))
+	if (intel_crtc_has_dp_encoder(intel_crtc->config))
 		tmp |= AUD_CONFIG_N_VALUE_INDEX;
 	I915_WRITE(aud_config, tmp);
 
@@ -475,7 +475,7 @@ static void ilk_audio_codec_enable(struct drm_connector *connector,
 	tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
 	tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
 	tmp &= ~AUD_CONFIG_PIXEL_CLOCK_HDMI_MASK;
-	if (intel_crtc_has_type(intel_crtc->config, INTEL_OUTPUT_DISPLAYPORT))
+	if (intel_crtc_has_dp_encoder(intel_crtc->config))
 		tmp |= AUD_CONFIG_N_VALUE_INDEX;
 	else
 		tmp |= audio_config_hdmi_pixel_clock(adjusted_mode);
@@ -513,7 +513,7 @@ void intel_audio_codec_enable(struct intel_encoder *intel_encoder)
 
 	/* ELD Conn_Type */
 	connector->eld[5] &= ~(3 << 2);
-	if (intel_crtc_has_type(crtc->config, INTEL_OUTPUT_DISPLAYPORT))
+	if (intel_crtc_has_dp_encoder(crtc->config))
 		connector->eld[5] |= (1 << 2);
 
 	connector->eld[6] = drm_av_sync_delay(connector, adjusted_mode) / 2;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 2388bfe1b925..3c78f07a50e1 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7371,8 +7371,7 @@ static void vlv_prepare_pll(struct intel_crtc *crtc,
 
 	coreclk = vlv_dpio_read(dev_priv, pipe, VLV_PLL_DW7(pipe));
 	coreclk = (coreclk & 0x0000ff00) | 0x01c00000;
-	if (intel_crtc_has_type(crtc->config, INTEL_OUTPUT_DISPLAYPORT) ||
-	    intel_crtc_has_type(crtc->config, INTEL_OUTPUT_EDP))
+	if (intel_crtc_has_dp_encoder(crtc->config))
 		coreclk |= 0x01000000;
 	vlv_dpio_write(dev_priv, pipe, VLV_PLL_DW7(pipe), coreclk);
 
-- 
2.7.4

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

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

* [PATCH 08/12] drm/i915: s/INTEL_OUTPUT_DISPLAYPORT/INTEL_OUTPUT_DP/
  2016-06-08 10:41 [PATCH 00/12] drm/i915: Eliminate DDI encoder->type frobbery ville.syrjala
                   ` (6 preceding siblings ...)
  2016-06-08 10:41 ` [PATCH 07/12] drm/i915: Replace some open coded intel_crtc_has_dp_encoder()s ville.syrjala
@ 2016-06-08 10:41 ` ville.syrjala
  2016-06-08 13:05   ` Kahola, Mika
  2016-06-08 10:41 ` [PATCH 09/12] drm/i915: Kill has_dsi_encoder ville.syrjala
                   ` (4 subsequent siblings)
  12 siblings, 1 reply; 28+ messages in thread
From: ville.syrjala @ 2016-06-08 10:41 UTC (permalink / raw)
  To: intel-gfx

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

INTEL_OUTPUT_DISPLAYPORT hsa been bugging me for a long time. It always
looks out of place besides INTEL_OUTPUT_EDP and INTEL_OUTPUT_DP_MST.
Let's just rename it to INTEL_OUTPUT_DP.

v2: Rebase

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c   |  8 ++++----
 drivers/gpu/drm/i915/intel_ddi.c      | 16 ++++++++--------
 drivers/gpu/drm/i915/intel_display.c  | 10 +++++-----
 drivers/gpu/drm/i915/intel_dp.c       | 10 +++++-----
 drivers/gpu/drm/i915/intel_dpll_mgr.c |  6 +++---
 drivers/gpu/drm/i915/intel_drv.h      |  2 +-
 drivers/gpu/drm/i915/intel_opregion.c |  2 +-
 7 files changed, 27 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index e4f2c55d9697..cb69e4b361b5 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2986,7 +2986,7 @@ static void intel_connector_info(struct seq_file *m,
 			   connector->display_info.cea_rev);
 	}
 	if (intel_encoder) {
-		if (intel_encoder->type == INTEL_OUTPUT_DISPLAYPORT ||
+		if (intel_encoder->type == INTEL_OUTPUT_DP ||
 		    intel_encoder->type == INTEL_OUTPUT_EDP)
 			intel_dp_info(m, intel_connector);
 		else if (intel_encoder->type == INTEL_OUTPUT_HDMI)
@@ -3387,7 +3387,7 @@ static void drrs_status_per_crtc(struct seq_file *m,
 		case INTEL_OUTPUT_HDMI:
 			seq_puts(m, "HDMI:\n");
 			break;
-		case INTEL_OUTPUT_DISPLAYPORT:
+		case INTEL_OUTPUT_DP:
 			seq_puts(m, "DP:\n");
 			break;
 		default:
@@ -3492,7 +3492,7 @@ static int i915_dp_mst_info(struct seq_file *m, void *unused)
 	drm_modeset_lock_all(dev);
 	list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) {
 		intel_encoder = to_intel_encoder(encoder);
-		if (intel_encoder->type != INTEL_OUTPUT_DISPLAYPORT)
+		if (intel_encoder->type != INTEL_OUTPUT_DP)
 			continue;
 		intel_dig_port = enc_to_dig_port(encoder);
 		if (!intel_dig_port->dp.can_mst)
@@ -3754,7 +3754,7 @@ static int i9xx_pipe_crc_auto_source(struct drm_device *dev, enum pipe pipe,
 		case INTEL_OUTPUT_TVOUT:
 			*source = INTEL_PIPE_CRC_SOURCE_TV;
 			break;
-		case INTEL_OUTPUT_DISPLAYPORT:
+		case INTEL_OUTPUT_DP:
 		case INTEL_OUTPUT_EDP:
 			dig_port = enc_to_dig_port(&encoder->base);
 			switch (dig_port->port) {
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index b7285336fb53..4f7fc0ed8796 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -318,7 +318,7 @@ static void ddi_get_encoder_port(struct intel_encoder *intel_encoder,
 	default:
 		WARN(1, "Invalid DDI encoder type %d\n", intel_encoder->type);
 		/* fallthrough and treat as unknown */
-	case INTEL_OUTPUT_DISPLAYPORT:
+	case INTEL_OUTPUT_DP:
 	case INTEL_OUTPUT_EDP:
 	case INTEL_OUTPUT_HDMI:
 	case INTEL_OUTPUT_UNKNOWN:
@@ -482,7 +482,7 @@ void intel_prepare_ddi_buffer(struct intel_encoder *encoder)
 		ddi_translations = ddi_translations_edp;
 		size = n_edp_entries;
 		break;
-	case INTEL_OUTPUT_DISPLAYPORT:
+	case INTEL_OUTPUT_DP:
 	case INTEL_OUTPUT_HDMI:
 		ddi_translations = ddi_translations_dp;
 		size = n_dp_entries;
@@ -1068,7 +1068,7 @@ void intel_ddi_set_pipe_settings(struct drm_crtc *crtc)
 	int type = intel_encoder->type;
 	uint32_t temp;
 
-	if (type == INTEL_OUTPUT_DISPLAYPORT || type == INTEL_OUTPUT_EDP || type == INTEL_OUTPUT_DP_MST) {
+	if (type == INTEL_OUTPUT_DP || type == INTEL_OUTPUT_EDP || type == INTEL_OUTPUT_DP_MST) {
 		WARN_ON(transcoder_is_dsi(cpu_transcoder));
 
 		temp = TRANS_MSA_SYNC_CLK;
@@ -1182,7 +1182,7 @@ void intel_ddi_enable_transcoder_func(struct drm_crtc *crtc)
 		temp |= TRANS_DDI_MODE_SELECT_FDI;
 		temp |= (intel_crtc->config->fdi_lanes - 1) << 1;
 
-	} else if (type == INTEL_OUTPUT_DISPLAYPORT ||
+	} else if (type == INTEL_OUTPUT_DP ||
 		   type == INTEL_OUTPUT_EDP) {
 		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
 
@@ -1384,7 +1384,7 @@ static void skl_ddi_set_iboost(struct drm_i915_private *dev_priv,
 	dp_iboost = dev_priv->vbt.ddi_port_info[port].dp_boost_level;
 	hdmi_iboost = dev_priv->vbt.ddi_port_info[port].hdmi_boost_level;
 
-	if (type == INTEL_OUTPUT_DISPLAYPORT) {
+	if (type == INTEL_OUTPUT_DP) {
 		if (dp_iboost) {
 			iboost = dp_iboost;
 		} else {
@@ -1442,7 +1442,7 @@ static void bxt_ddi_vswing_sequence(struct drm_i915_private *dev_priv,
 	if (type == INTEL_OUTPUT_EDP && dev_priv->vbt.edp.low_vswing) {
 		n_entries = ARRAY_SIZE(bxt_ddi_translations_edp);
 		ddi_translations = bxt_ddi_translations_edp;
-	} else if (type == INTEL_OUTPUT_DISPLAYPORT
+	} else if (type == INTEL_OUTPUT_DP
 			|| type == INTEL_OUTPUT_EDP) {
 		n_entries = ARRAY_SIZE(bxt_ddi_translations_dp);
 		ddi_translations = bxt_ddi_translations_dp;
@@ -1616,7 +1616,7 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder)
 
 	intel_ddi_clk_select(intel_encoder, crtc->config);
 
-	if (type == INTEL_OUTPUT_DISPLAYPORT || type == INTEL_OUTPUT_EDP) {
+	if (type == INTEL_OUTPUT_DP || type == INTEL_OUTPUT_EDP) {
 		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
 
 		intel_dp_set_link_params(intel_dp, crtc->config);
@@ -1661,7 +1661,7 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder)
 	if (wait)
 		intel_wait_ddi_buf_idle(dev_priv, port);
 
-	if (type == INTEL_OUTPUT_DISPLAYPORT || type == INTEL_OUTPUT_EDP) {
+	if (type == INTEL_OUTPUT_DP || type == INTEL_OUTPUT_EDP) {
 		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
 		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
 		intel_edp_panel_vdd_on(intel_dp);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3c78f07a50e1..de8e4acf8f76 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -540,7 +540,7 @@ bool intel_crtc_has_type(const struct intel_crtc_state *crtc_state,
 bool intel_crtc_has_dp_encoder(const struct intel_crtc_state *crtc_state)
 {
 	return crtc_state->output_types &
-		((1 << INTEL_OUTPUT_DISPLAYPORT) |
+		((1 << INTEL_OUTPUT_DP) |
 		 (1 << INTEL_OUTPUT_DP_MST) |
 		 (1 << INTEL_OUTPUT_EDP));
 }
@@ -4036,7 +4036,7 @@ intel_trans_dp_port_sel(struct drm_crtc *crtc)
 	struct intel_encoder *encoder;
 
 	for_each_encoder_on_crtc(dev, crtc, encoder) {
-		if (encoder->type == INTEL_OUTPUT_DISPLAYPORT ||
+		if (encoder->type == INTEL_OUTPUT_DP ||
 		    encoder->type == INTEL_OUTPUT_EDP)
 			return enc_to_dig_port(&encoder->base)->port;
 	}
@@ -5116,7 +5116,7 @@ intel_display_port_power_domain(struct intel_encoder *intel_encoder)
 	case INTEL_OUTPUT_UNKNOWN:
 		/* Only DDI platforms should ever use this output type */
 		WARN_ON_ONCE(!HAS_DDI(dev));
-	case INTEL_OUTPUT_DISPLAYPORT:
+	case INTEL_OUTPUT_DP:
 	case INTEL_OUTPUT_HDMI:
 	case INTEL_OUTPUT_EDP:
 		intel_dig_port = enc_to_dig_port(&intel_encoder->base);
@@ -5150,7 +5150,7 @@ intel_display_port_aux_power_domain(struct intel_encoder *intel_encoder)
 		 * run the DP detection too.
 		 */
 		WARN_ON_ONCE(!HAS_DDI(dev));
-	case INTEL_OUTPUT_DISPLAYPORT:
+	case INTEL_OUTPUT_DP:
 	case INTEL_OUTPUT_EDP:
 		intel_dig_port = enc_to_dig_port(&intel_encoder->base);
 		return port_to_aux_power_domain(intel_dig_port->port);
@@ -12377,7 +12377,7 @@ static bool check_digital_port_conflicts(struct drm_atomic_state *state)
 		case INTEL_OUTPUT_UNKNOWN:
 			if (WARN_ON(!HAS_DDI(dev)))
 				break;
-		case INTEL_OUTPUT_DISPLAYPORT:
+		case INTEL_OUTPUT_DP:
 		case INTEL_OUTPUT_HDMI:
 		case INTEL_OUTPUT_EDP:
 			port_mask = 1 << enc_to_dig_port(&encoder->base)->port;
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 207e82695cfa..e34426dc5c1e 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4201,7 +4201,7 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
 	}
 
 	if (intel_encoder->type != INTEL_OUTPUT_EDP)
-		intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
+		intel_encoder->type = INTEL_OUTPUT_DP;
 
 	intel_dp_probe_oui(intel_dp);
 
@@ -4277,7 +4277,7 @@ intel_dp_detect(struct drm_connector *connector, bool force)
 		/* MST devices are disconnected from a monitor POV */
 		intel_dp_unset_edid(intel_dp);
 		if (intel_encoder->type != INTEL_OUTPUT_EDP)
-			intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
+			intel_encoder->type = INTEL_OUTPUT_DP;
 		return connector_status_disconnected;
 	}
 
@@ -4316,7 +4316,7 @@ intel_dp_force(struct drm_connector *connector)
 	intel_display_power_put(dev_priv, power_domain);
 
 	if (intel_encoder->type != INTEL_OUTPUT_EDP)
-		intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
+		intel_encoder->type = INTEL_OUTPUT_DP;
 }
 
 static int intel_dp_get_modes(struct drm_connector *connector)
@@ -4597,7 +4597,7 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
 
 	if (intel_dig_port->base.type != INTEL_OUTPUT_EDP &&
 	    intel_dig_port->base.type != INTEL_OUTPUT_HDMI)
-		intel_dig_port->base.type = INTEL_OUTPUT_DISPLAYPORT;
+		intel_dig_port->base.type = INTEL_OUTPUT_DP;
 
 	if (long_hpd && intel_dig_port->base.type == INTEL_OUTPUT_EDP) {
 		/*
@@ -5620,7 +5620,7 @@ bool intel_dp_init(struct drm_device *dev,
 	intel_dig_port->dp.output_reg = output_reg;
 	intel_dig_port->max_lanes = 4;
 
-	intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
+	intel_encoder->type = INTEL_OUTPUT_DP;
 	if (IS_CHERRYVIEW(dev)) {
 		if (port == PORT_D)
 			intel_encoder->crtc_mask = 1 << 2;
diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index c0eff1571731..5f96466e4e47 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -713,7 +713,7 @@ hsw_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
 		pll = intel_find_shared_dpll(crtc, crtc_state,
 					     DPLL_ID_WRPLL1, DPLL_ID_WRPLL2);
 
-	} else if (encoder->type == INTEL_OUTPUT_DISPLAYPORT ||
+	} else if (encoder->type == INTEL_OUTPUT_DP ||
 		   encoder->type == INTEL_OUTPUT_DP_MST ||
 		   encoder->type == INTEL_OUTPUT_EDP) {
 		enum intel_dpll_id pll_id;
@@ -1222,7 +1222,7 @@ skl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
 			 DPLL_CFGCR2_KDIV(wrpll_params.kdiv) |
 			 DPLL_CFGCR2_PDIV(wrpll_params.pdiv) |
 			 wrpll_params.central_freq;
-	} else if (encoder->type == INTEL_OUTPUT_DISPLAYPORT ||
+	} else if (encoder->type == INTEL_OUTPUT_DP ||
 		   encoder->type == INTEL_OUTPUT_DP_MST ||
 		   encoder->type == INTEL_OUTPUT_EDP) {
 		switch (crtc_state->port_clock / 2) {
@@ -1530,7 +1530,7 @@ bxt_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
 		clk_div.m2_frac_en = clk_div.m2_frac != 0;
 
 		vco = best_clock.vco;
-	} else if (encoder->type == INTEL_OUTPUT_DISPLAYPORT ||
+	} else if (encoder->type == INTEL_OUTPUT_DP ||
 		   encoder->type == INTEL_OUTPUT_EDP) {
 		int i;
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 2c70dd677a00..bd67339357d1 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -135,7 +135,7 @@ enum intel_output_type {
 	INTEL_OUTPUT_LVDS = 4,
 	INTEL_OUTPUT_TVOUT = 5,
 	INTEL_OUTPUT_HDMI = 6,
-	INTEL_OUTPUT_DISPLAYPORT = 7,
+	INTEL_OUTPUT_DP = 7,
 	INTEL_OUTPUT_EDP = 8,
 	INTEL_OUTPUT_DSI = 9,
 	INTEL_OUTPUT_UNKNOWN = 10,
diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
index f6d8a21d2c49..e61317f5a586 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -366,7 +366,7 @@ int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder,
 		type = DISPLAY_TYPE_CRT;
 		break;
 	case INTEL_OUTPUT_UNKNOWN:
-	case INTEL_OUTPUT_DISPLAYPORT:
+	case INTEL_OUTPUT_DP:
 	case INTEL_OUTPUT_HDMI:
 	case INTEL_OUTPUT_DP_MST:
 		type = DISPLAY_TYPE_EXTERNAL_FLAT_PANEL;
-- 
2.7.4

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

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

* [PATCH 09/12] drm/i915: Kill has_dsi_encoder
  2016-06-08 10:41 [PATCH 00/12] drm/i915: Eliminate DDI encoder->type frobbery ville.syrjala
                   ` (7 preceding siblings ...)
  2016-06-08 10:41 ` [PATCH 08/12] drm/i915: s/INTEL_OUTPUT_DISPLAYPORT/INTEL_OUTPUT_DP/ ville.syrjala
@ 2016-06-08 10:41 ` ville.syrjala
  2016-06-08 10:41 ` [PATCH 10/12] drm/i915: Simplify hdmi_12bpc_possible() ville.syrjala
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 28+ messages in thread
From: ville.syrjala @ 2016-06-08 10:41 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula

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

has_dsi_encoder was introduced to indicate that the pipe is driving
a DSI encoder. Now that we have the output_types bitmask that can
tell us the same thing, let's just kill has_dsi_encoder.

v2: Rebase, handle BXT DSI transcoder, rewrote commit message

Cc: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_color.c   |  2 +-
 drivers/gpu/drm/i915/intel_display.c | 47 +++++++++++++++---------------------
 drivers/gpu/drm/i915/intel_drv.h     |  3 ---
 drivers/gpu/drm/i915/intel_dsi.c     |  4 ---
 4 files changed, 20 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 1b3f97449395..ffacd4832170 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -272,7 +272,7 @@ static void i9xx_load_luts_internal(struct drm_crtc *crtc,
 	int i;
 
 	if (HAS_GMCH_DISPLAY(dev)) {
-		if (intel_crtc->config->has_dsi_encoder)
+		if (intel_crtc_has_type(intel_crtc->config, INTEL_OUTPUT_DSI))
 			assert_dsi_pll_enabled(dev_priv);
 		else
 			assert_pll_enabled(dev_priv, pipe);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index de8e4acf8f76..3a0b590c4885 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1955,7 +1955,7 @@ static void intel_enable_pipe(struct intel_crtc *crtc)
 	 * need the check.
 	 */
 	if (HAS_GMCH_DISPLAY(dev_priv))
-		if (crtc->config->has_dsi_encoder)
+		if (intel_crtc_has_type(crtc->config, INTEL_OUTPUT_DSI))
 			assert_dsi_pll_enabled(dev_priv);
 		else
 			assert_pll_enabled(dev_priv, pipe);
@@ -4817,7 +4817,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
 	if (intel_crtc_has_dp_encoder(intel_crtc->config))
 		intel_dp_set_m_n(intel_crtc, M1_N1);
 
-	if (!intel_crtc->config->has_dsi_encoder)
+	if (!transcoder_is_dsi(cpu_transcoder))
 		intel_set_pipe_timings(intel_crtc);
 
 	intel_set_pipe_src_size(intel_crtc);
@@ -4833,7 +4833,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
 				     &intel_crtc->config->fdi_m_n, NULL);
 	}
 
-	if (!intel_crtc->config->has_dsi_encoder)
+	if (!transcoder_is_dsi(cpu_transcoder))
 		haswell_set_pipeconf(crtc);
 
 	haswell_set_pipemisc(crtc);
@@ -4855,7 +4855,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
 	if (intel_crtc->config->has_pch_encoder)
 		dev_priv->display.fdi_link_train(crtc);
 
-	if (!intel_crtc->config->has_dsi_encoder)
+	if (!transcoder_is_dsi(cpu_transcoder))
 		intel_ddi_enable_pipe_clock(intel_crtc);
 
 	if (INTEL_INFO(dev)->gen >= 9)
@@ -4870,7 +4870,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
 	intel_color_load_luts(&pipe_config->base);
 
 	intel_ddi_set_pipe_settings(crtc);
-	if (!intel_crtc->config->has_dsi_encoder)
+	if (!transcoder_is_dsi(cpu_transcoder))
 		intel_ddi_enable_transcoder_func(crtc);
 
 	if (dev_priv->display.initial_watermarks != NULL)
@@ -4879,7 +4879,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
 		intel_update_watermarks(crtc);
 
 	/* XXX: Do the pipe assertions at the right place for BXT DSI. */
-	if (!intel_crtc->config->has_dsi_encoder)
+	if (!transcoder_is_dsi(cpu_transcoder))
 		intel_enable_pipe(intel_crtc);
 
 	if (intel_crtc->config->has_pch_encoder)
@@ -5012,13 +5012,13 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
 	assert_vblank_disabled(crtc);
 
 	/* XXX: Do the pipe assertions at the right place for BXT DSI. */
-	if (!intel_crtc->config->has_dsi_encoder)
+	if (!transcoder_is_dsi(cpu_transcoder))
 		intel_disable_pipe(intel_crtc);
 
 	if (intel_crtc->config->dp_encoder_is_mst)
 		intel_ddi_set_vc_payload_alloc(crtc, false);
 
-	if (!intel_crtc->config->has_dsi_encoder)
+	if (!transcoder_is_dsi(cpu_transcoder))
 		intel_ddi_disable_transcoder_func(dev_priv, cpu_transcoder);
 
 	if (INTEL_INFO(dev)->gen >= 9)
@@ -5026,7 +5026,7 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
 	else
 		ironlake_pfit_disable(intel_crtc, false);
 
-	if (!intel_crtc->config->has_dsi_encoder)
+	if (!transcoder_is_dsi(cpu_transcoder))
 		intel_ddi_disable_pipe_clock(intel_crtc);
 
 	for_each_encoder_on_crtc(dev, crtc, encoder)
@@ -6257,7 +6257,7 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
 		if (encoder->post_disable)
 			encoder->post_disable(encoder);
 
-	if (!intel_crtc->config->has_dsi_encoder) {
+	if (!intel_crtc_has_type(intel_crtc->config, INTEL_OUTPUT_DSI)) {
 		if (IS_CHERRYVIEW(dev))
 			chv_disable_pll(dev_priv, pipe);
 		else if (IS_VALLEYVIEW(dev))
@@ -7256,7 +7256,7 @@ static void vlv_compute_dpll(struct intel_crtc *crtc,
 		pipe_config->dpll_hw_state.dpll |= DPLL_INTEGRATED_CRI_CLK_VLV;
 
 	/* DPLL not used with DSI, but still need the rest set up */
-	if (!pipe_config->has_dsi_encoder)
+	if (!intel_crtc_has_type(pipe_config, INTEL_OUTPUT_DSI))
 		pipe_config->dpll_hw_state.dpll |= DPLL_VCO_ENABLE |
 			DPLL_EXT_BUFFER_ENABLE_VLV;
 
@@ -7273,7 +7273,7 @@ static void chv_compute_dpll(struct intel_crtc *crtc,
 		pipe_config->dpll_hw_state.dpll |= DPLL_INTEGRATED_CRI_CLK_VLV;
 
 	/* DPLL not used with DSI, but still need the rest set up */
-	if (!pipe_config->has_dsi_encoder)
+	if (!intel_crtc_has_type(pipe_config, INTEL_OUTPUT_DSI))
 		pipe_config->dpll_hw_state.dpll |= DPLL_VCO_ENABLE;
 
 	pipe_config->dpll_hw_state.dpll_md =
@@ -9837,10 +9837,7 @@ static void skl_modeset_commit_cdclk(struct drm_atomic_state *old_state)
 static int haswell_crtc_compute_clock(struct intel_crtc *crtc,
 				      struct intel_crtc_state *crtc_state)
 {
-	struct intel_encoder *intel_encoder =
-		intel_ddi_get_crtc_new_encoder(crtc_state);
-
-	if (intel_encoder->type != INTEL_OUTPUT_DSI) {
+	if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI)) {
 		if (!intel_ddi_pll_select(crtc, crtc_state))
 			return -EINVAL;
 	}
@@ -10007,8 +10004,6 @@ static bool bxt_get_dsi_transcoder_state(struct intel_crtc *crtc,
 	enum transcoder cpu_transcoder;
 	u32 tmp;
 
-	pipe_config->has_dsi_encoder = false;
-
 	for_each_port_masked(port, BIT(PORT_A) | BIT(PORT_C)) {
 		if (port == PORT_A)
 			cpu_transcoder = TRANSCODER_DSI_A;
@@ -10040,11 +10035,10 @@ static bool bxt_get_dsi_transcoder_state(struct intel_crtc *crtc,
 			continue;
 
 		pipe_config->cpu_transcoder = cpu_transcoder;
-		pipe_config->has_dsi_encoder = true;
 		break;
 	}
 
-	return pipe_config->has_dsi_encoder;
+	return transcoder_is_dsi(pipe_config->cpu_transcoder);
 }
 
 static void haswell_get_ddi_port_state(struct intel_crtc *crtc,
@@ -10108,18 +10102,16 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 
 	active = hsw_get_transcoder_state(crtc, pipe_config, &power_domain_mask);
 
-	if (IS_BROXTON(dev_priv)) {
-		bxt_get_dsi_transcoder_state(crtc, pipe_config,
-					     &power_domain_mask);
-		WARN_ON(active && pipe_config->has_dsi_encoder);
-		if (pipe_config->has_dsi_encoder)
-			active = true;
+	if (IS_BROXTON(dev_priv) &&
+	    bxt_get_dsi_transcoder_state(crtc, pipe_config, &power_domain_mask)) {
+		WARN_ON(active);
+		active = true;
 	}
 
 	if (!active)
 		goto out;
 
-	if (!pipe_config->has_dsi_encoder) {
+	if (!transcoder_is_dsi(pipe_config->cpu_transcoder)) {
 		haswell_get_ddi_port_state(crtc, pipe_config);
 		intel_get_pipe_timings(crtc, pipe_config);
 	}
@@ -12782,7 +12774,6 @@ intel_pipe_config_compare(struct drm_device *dev,
 	} else
 		PIPE_CONF_CHECK_M_N_ALT(dp_m_n, dp_m2_n2);
 
-	PIPE_CONF_CHECK_I(has_dsi_encoder);
 	PIPE_CONF_CHECK_X(output_types);
 
 	PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_hdisplay);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index bd67339357d1..3cc0d4fcdc36 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -505,9 +505,6 @@ struct intel_crtc_state {
 	 */
 	bool limited_color_range;
 
-	/* DSI has special cases */
-	bool has_dsi_encoder;
-
 	/* Bitmask of encoder types (enum intel_output_type)
 	 * driven by the pipe.
 	 */
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 0f86da048a63..aa3f32506289 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -313,8 +313,6 @@ static bool intel_dsi_compute_config(struct intel_encoder *encoder,
 
 	DRM_DEBUG_KMS("\n");
 
-	pipe_config->has_dsi_encoder = true;
-
 	if (fixed_mode) {
 		intel_fixed_panel_mode(fixed_mode, adjusted_mode);
 
@@ -954,8 +952,6 @@ static void intel_dsi_get_config(struct intel_encoder *encoder,
 	u32 pclk;
 	DRM_DEBUG_KMS("\n");
 
-	pipe_config->has_dsi_encoder = true;
-
 	if (IS_BROXTON(dev))
 		bxt_dsi_get_pipe_config(encoder, pipe_config);
 
-- 
2.7.4

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

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

* [PATCH 10/12] drm/i915: Simplify hdmi_12bpc_possible()
  2016-06-08 10:41 [PATCH 00/12] drm/i915: Eliminate DDI encoder->type frobbery ville.syrjala
                   ` (8 preceding siblings ...)
  2016-06-08 10:41 ` [PATCH 09/12] drm/i915: Kill has_dsi_encoder ville.syrjala
@ 2016-06-08 10:41 ` ville.syrjala
  2016-06-08 10:41 ` [PATCH 11/12] drm/i915: Check for invalid cloning earlier during modeset ville.syrjala
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 28+ messages in thread
From: ville.syrjala @ 2016-06-08 10:41 UTC (permalink / raw)
  To: intel-gfx

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

With the output_types bitmask there's no need to loop through
the encoders anymore when checking for HDMI+non-HDMI cloning.

v2: Use output_types bitmask

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

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index ae153b6f093e..1b91d65ecfb9 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1273,33 +1273,15 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
 static bool hdmi_12bpc_possible(struct intel_crtc_state *crtc_state)
 {
 	struct drm_device *dev = crtc_state->base.crtc->dev;
-	struct drm_atomic_state *state;
-	struct intel_encoder *encoder;
-	struct drm_connector *connector;
-	struct drm_connector_state *connector_state;
-	int count = 0, count_hdmi = 0;
-	int i;
 
 	if (HAS_GMCH_DISPLAY(dev))
 		return false;
 
-	state = crtc_state->base.state;
-
-	for_each_connector_in_state(state, connector, connector_state, i) {
-		if (connector_state->crtc != crtc_state->base.crtc)
-			continue;
-
-		encoder = to_intel_encoder(connector_state->best_encoder);
-
-		count_hdmi += encoder->type == INTEL_OUTPUT_HDMI;
-		count++;
-	}
-
 	/*
 	 * HDMI 12bpc affects the clocks, so it's only possible
 	 * when not cloning with other encoder types.
 	 */
-	return count_hdmi > 0 && count_hdmi == count;
+	return crtc_state->output_types & ~(1 << INTEL_OUTPUT_HDMI);
 }
 
 bool intel_hdmi_compute_config(struct intel_encoder *encoder,
-- 
2.7.4

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

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

* [PATCH 11/12] drm/i915: Check for invalid cloning earlier during modeset
  2016-06-08 10:41 [PATCH 00/12] drm/i915: Eliminate DDI encoder->type frobbery ville.syrjala
                   ` (9 preceding siblings ...)
  2016-06-08 10:41 ` [PATCH 10/12] drm/i915: Simplify hdmi_12bpc_possible() ville.syrjala
@ 2016-06-08 10:41 ` ville.syrjala
  2016-06-08 13:15   ` Chris Wilson
  2016-06-08 10:41 ` [PATCH 12/12] drm/i915: Stop frobbing with DDI encoder->type ville.syrjala
  2016-06-08 11:13 ` ✓ Ro.CI.BAT: success for drm/i915: Eliminate DDI encoder->type frobbery Patchwork
  12 siblings, 1 reply; 28+ messages in thread
From: ville.syrjala @ 2016-06-08 10:41 UTC (permalink / raw)
  To: intel-gfx

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

Move the encoder cloning check to happen earlier in the modeset. The
main benefit will be that the debug output from a failed modeset will
be less confusing as output_types can not indicate an invalid
configuration during the later computation stages.

For instance, what happened to me was kms_setmode was attempting one
of its invalid cloning checks during which it asked for DP+VGA cloning
on HSW. In this case the DP .compute_config() was executed after
the FDI .compute_config() leaving the DP link clock (1.62 in this case)
in port_clock, and then later the FDI BW computation tried to use that
as the FDI link clock (which should always be 2.7). 1.62 x 2 wasn't
enough for the mode it was trying to use, and so it ended up rejecting
the modeset, not because of an invalid cloning configuration, but
because of supposedly running out of FDI bandwidth. Took me a while
to figure out what had actually happened.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3a0b590c4885..442ed6320082 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11997,26 +11997,6 @@ static bool check_single_encoder_cloning(struct drm_atomic_state *state,
 	return true;
 }
 
-static bool check_encoder_cloning(struct drm_atomic_state *state,
-				  struct intel_crtc *crtc)
-{
-	struct intel_encoder *encoder;
-	struct drm_connector *connector;
-	struct drm_connector_state *connector_state;
-	int i;
-
-	for_each_connector_in_state(state, connector, connector_state, i) {
-		if (connector_state->crtc != &crtc->base)
-			continue;
-
-		encoder = to_intel_encoder(connector_state->best_encoder);
-		if (!check_single_encoder_cloning(state, crtc, encoder))
-			return false;
-	}
-
-	return true;
-}
-
 static int intel_crtc_atomic_check(struct drm_crtc *crtc,
 				   struct drm_crtc_state *crtc_state)
 {
@@ -12029,11 +12009,6 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc,
 	int ret;
 	bool mode_changed = needs_modeset(crtc_state);
 
-	if (mode_changed && !check_encoder_cloning(state, intel_crtc)) {
-		DRM_DEBUG_KMS("rejecting invalid cloning configuration\n");
-		return -EINVAL;
-	}
-
 	if (mode_changed && !crtc_state->active)
 		pipe_config->update_wm_post = true;
 
@@ -12472,6 +12447,11 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
 
 		encoder = to_intel_encoder(connector_state->best_encoder);
 
+		if (!check_single_encoder_cloning(state, to_intel_crtc(crtc), encoder)) {
+			DRM_DEBUG_KMS("rejecting invalid cloning configuration\n");
+			goto fail;
+		}
+
 		/*
 		 * Determine output_types before calling the .compute_config()
 		 * hooks so that the hooks can use this information safely.
-- 
2.7.4

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

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

* [PATCH 12/12] drm/i915: Stop frobbing with DDI encoder->type
  2016-06-08 10:41 [PATCH 00/12] drm/i915: Eliminate DDI encoder->type frobbery ville.syrjala
                   ` (10 preceding siblings ...)
  2016-06-08 10:41 ` [PATCH 11/12] drm/i915: Check for invalid cloning earlier during modeset ville.syrjala
@ 2016-06-08 10:41 ` ville.syrjala
  2016-06-08 11:13 ` ✓ Ro.CI.BAT: success for drm/i915: Eliminate DDI encoder->type frobbery Patchwork
  12 siblings, 0 replies; 28+ messages in thread
From: ville.syrjala @ 2016-06-08 10:41 UTC (permalink / raw)
  To: intel-gfx

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

Now that we have the output_types bitmask in the crtc state, we
can use it to indicate in which mode we want to drive the DDI
encoders. For pre-DDI output_types will instead indicate what
kind of cloning is being done, but as DDI platforms don't
support cloning so we don't have to worry about mixing the two
approaches.

From here on out an encoder on DDI platforms can now have a
type of DDI, EDP, ANALOG, or DP_MST. A DDI type encoder can be
driven either in HDMI or DP SST mode. We maintain EDP as a
seaparete type from DDI to keep is_edp() etc. working. ANALOG
(ie. FDI) was already treated quite diffrently, so we'll stick
to that. And MST encoders remain separate beasts as well.

To achieve this neatly we'll add a new optional
.compute_output_type() hook for encoders. Only encoders with
multiple personalities will need to provide one. The hook can
do whatever it needs to figure out what kind of signal should
be output. Additionaly .get_config() will also need to read out
the current type from the hardware somehow and fill that in.
Note that in the name of consistency we'll populate the hook
also for EDP encoders on DDI platforms even though the default
fallback of using encoder->type would work just as well there.

The reason for adding a new hook instead of letting
.compute_config() do the work is to follow the rule that
output_types will already be fully populated by the time
.compute_config() is called for the first encoder (which
simplifies things such as hdmi_12bpc_possible()).

With output_types correctly populated, the biggest churn comes
from changing various encoder->type checks to consult output_types
instead.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_audio.c    |   3 +-
 drivers/gpu/drm/i915/intel_ddi.c      | 204 +++++++++++++++++++++-------------
 drivers/gpu/drm/i915/intel_display.c  |  37 +++---
 drivers/gpu/drm/i915/intel_dp.c       |  16 +--
 drivers/gpu/drm/i915/intel_dpll_mgr.c |  21 ++--
 drivers/gpu/drm/i915/intel_drv.h      |   4 +-
 drivers/gpu/drm/i915/intel_hdmi.c     |  10 +-
 drivers/gpu/drm/i915/intel_opregion.c |   2 +-
 8 files changed, 169 insertions(+), 128 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index 0653fb1d9d26..2815c8c34a66 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -653,7 +653,8 @@ static int i915_audio_component_sync_audio_rate(struct device *dev,
 	intel_encoder = dev_priv->dig_port_map[port];
 	/* intel_encoder might be NULL for DP MST */
 	if (!intel_encoder || !intel_encoder->base.crtc ||
-	    intel_encoder->type != INTEL_OUTPUT_HDMI) {
+	    !intel_crtc_has_type(to_intel_crtc(intel_encoder->base.crtc)->config,
+				 INTEL_OUTPUT_HDMI)) {
 		DRM_DEBUG_KMS("no valid port %c\n", port_name(port));
 		err = -ENODEV;
 		goto unlock;
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 4f7fc0ed8796..a1c5feb0e14c 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -301,8 +301,8 @@ static const struct bxt_ddi_buf_trans bxt_ddi_translations_hdmi[] = {
 	{ 154, 0x9A, 1, 128, true },	/* 9:	1200		0   */
 };
 
-static void bxt_ddi_vswing_sequence(struct drm_i915_private *dev_priv,
-				    u32 level, enum port port, int type);
+static void bxt_ddi_vswing_sequence(struct intel_encoder *encoder,
+				    u32 level);
 
 static void ddi_get_encoder_port(struct intel_encoder *intel_encoder,
 				 struct intel_digital_port **dig_port,
@@ -318,10 +318,8 @@ static void ddi_get_encoder_port(struct intel_encoder *intel_encoder,
 	default:
 		WARN(1, "Invalid DDI encoder type %d\n", intel_encoder->type);
 		/* fallthrough and treat as unknown */
-	case INTEL_OUTPUT_DP:
+	case INTEL_OUTPUT_DDI:
 	case INTEL_OUTPUT_EDP:
-	case INTEL_OUTPUT_HDMI:
-	case INTEL_OUTPUT_UNKNOWN:
 		*dig_port = enc_to_dig_port(encoder);
 		*port = (*dig_port)->port;
 		break;
@@ -398,6 +396,7 @@ skl_get_buf_trans_hdmi(struct drm_i915_private *dev_priv, int *n_entries)
 void intel_prepare_ddi_buffer(struct intel_encoder *encoder)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+	struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
 	u32 iboost_bit = 0;
 	int i, n_hdmi_entries, n_dp_entries, n_edp_entries, hdmi_default_entry,
 	    size;
@@ -413,12 +412,9 @@ void intel_prepare_ddi_buffer(struct intel_encoder *encoder)
 	hdmi_level = dev_priv->vbt.ddi_port_info[port].hdmi_level_shift;
 
 	if (IS_BROXTON(dev_priv)) {
-		if (encoder->type != INTEL_OUTPUT_HDMI)
-			return;
-
 		/* Vswing programming for HDMI */
-		bxt_ddi_vswing_sequence(dev_priv, hdmi_level, port,
-					INTEL_OUTPUT_HDMI);
+		if (intel_crtc_has_type(crtc->config, INTEL_OUTPUT_HDMI))
+			bxt_ddi_vswing_sequence(encoder, hdmi_level);
 		return;
 	}
 
@@ -436,7 +432,7 @@ void intel_prepare_ddi_buffer(struct intel_encoder *encoder)
 		    dev_priv->vbt.ddi_port_info[port].dp_boost_level)
 			iboost_bit = 1<<31;
 
-		if (WARN_ON(encoder->type == INTEL_OUTPUT_EDP &&
+		if (WARN_ON(intel_crtc_has_type(crtc->config, INTEL_OUTPUT_EDP) &&
 			    port != PORT_A && port != PORT_E &&
 			    n_edp_entries > 9))
 			n_edp_entries = 9;
@@ -477,22 +473,20 @@ void intel_prepare_ddi_buffer(struct intel_encoder *encoder)
 		hdmi_default_entry = 7;
 	}
 
-	switch (encoder->type) {
-	case INTEL_OUTPUT_EDP:
+	if (intel_crtc_has_type(crtc->config, INTEL_OUTPUT_EDP)) {
 		ddi_translations = ddi_translations_edp;
 		size = n_edp_entries;
-		break;
-	case INTEL_OUTPUT_DP:
-	case INTEL_OUTPUT_HDMI:
+	} else if (intel_crtc_has_type(crtc->config, INTEL_OUTPUT_DP) ||
+		   intel_crtc_has_type(crtc->config, INTEL_OUTPUT_HDMI)) {
 		ddi_translations = ddi_translations_dp;
 		size = n_dp_entries;
-		break;
-	case INTEL_OUTPUT_ANALOG:
+	} else if (intel_crtc_has_type(crtc->config, INTEL_OUTPUT_ANALOG)) {
 		ddi_translations = ddi_translations_fdi;
 		size = n_dp_entries;
-		break;
-	default:
-		BUG();
+	} else {
+		WARN(1, "Invalid encoder type 0x%x for pipe %c\n",
+		     crtc->config->output_types, pipe_name(crtc->pipe));
+		return;
 	}
 
 	for (i = 0; i < size; i++) {
@@ -502,7 +496,7 @@ void intel_prepare_ddi_buffer(struct intel_encoder *encoder)
 			   ddi_translations[i].trans2);
 	}
 
-	if (encoder->type != INTEL_OUTPUT_HDMI)
+	if (!intel_crtc_has_type(crtc->config, INTEL_OUTPUT_HDMI))
 		return;
 
 	/* Choose a good default if VBT is badly populated */
@@ -549,7 +543,7 @@ void hsw_fdi_link_train(struct drm_crtc *crtc)
 	u32 temp, i, rx_ctl_val;
 
 	for_each_encoder_on_crtc(dev, crtc, encoder) {
-		WARN_ON(encoder->type != INTEL_OUTPUT_ANALOG);
+		WARN_ON(!intel_crtc_has_type(intel_crtc->config, INTEL_OUTPUT_ANALOG));
 		intel_prepare_ddi_buffer(encoder);
 	}
 
@@ -1063,12 +1057,10 @@ void intel_ddi_set_pipe_settings(struct drm_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = crtc->dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	struct intel_encoder *intel_encoder = intel_ddi_get_crtc_encoder(crtc);
 	enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
-	int type = intel_encoder->type;
 	uint32_t temp;
 
-	if (type == INTEL_OUTPUT_DP || type == INTEL_OUTPUT_EDP || type == INTEL_OUTPUT_DP_MST) {
+	if (intel_crtc_has_dp_encoder(intel_crtc->config)) {
 		WARN_ON(transcoder_is_dsi(cpu_transcoder));
 
 		temp = TRANS_MSA_SYNC_CLK;
@@ -1117,7 +1109,6 @@ void intel_ddi_enable_transcoder_func(struct drm_crtc *crtc)
 	enum pipe pipe = intel_crtc->pipe;
 	enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
 	enum port port = intel_ddi_get_encoder_port(intel_encoder);
-	int type = intel_encoder->type;
 	uint32_t temp;
 
 	/* Enable TRANS_DDI_FUNC_CTL for the pipe to work in HDMI mode */
@@ -1172,18 +1163,16 @@ void intel_ddi_enable_transcoder_func(struct drm_crtc *crtc)
 		}
 	}
 
-	if (type == INTEL_OUTPUT_HDMI) {
+	if (intel_crtc_has_type(intel_crtc->config, INTEL_OUTPUT_HDMI)) {
 		if (intel_crtc->config->has_hdmi_sink)
 			temp |= TRANS_DDI_MODE_SELECT_HDMI;
 		else
 			temp |= TRANS_DDI_MODE_SELECT_DVI;
-
-	} else if (type == INTEL_OUTPUT_ANALOG) {
+	} else if (intel_crtc_has_type(intel_crtc->config, INTEL_OUTPUT_ANALOG)) {
 		temp |= TRANS_DDI_MODE_SELECT_FDI;
 		temp |= (intel_crtc->config->fdi_lanes - 1) << 1;
-
-	} else if (type == INTEL_OUTPUT_DP ||
-		   type == INTEL_OUTPUT_EDP) {
+	} else if (intel_crtc_has_type(intel_crtc->config, INTEL_OUTPUT_DP) ||
+		   intel_crtc_has_type(intel_crtc->config, INTEL_OUTPUT_EDP)) {
 		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
 
 		if (intel_dp->is_mst) {
@@ -1192,7 +1181,7 @@ void intel_ddi_enable_transcoder_func(struct drm_crtc *crtc)
 			temp |= TRANS_DDI_MODE_SELECT_DP_SST;
 
 		temp |= DDI_PORT_WIDTH(intel_crtc->config->lane_count);
-	} else if (type == INTEL_OUTPUT_DP_MST) {
+	} else if (intel_crtc_has_type(intel_crtc->config, INTEL_OUTPUT_DP_MST)) {
 		struct intel_dp *intel_dp = &enc_to_mst(encoder)->primary->dp;
 
 		if (intel_dp->is_mst) {
@@ -1202,8 +1191,8 @@ void intel_ddi_enable_transcoder_func(struct drm_crtc *crtc)
 
 		temp |= DDI_PORT_WIDTH(intel_crtc->config->lane_count);
 	} else {
-		WARN(1, "Invalid encoder type %d for pipe %c\n",
-		     intel_encoder->type, pipe_name(pipe));
+		WARN(1, "Invalid encoder type 0x%x for pipe %c\n",
+		     intel_crtc->config->output_types, pipe_name(pipe));
 	}
 
 	I915_WRITE(TRANS_DDI_FUNC_CTL(cpu_transcoder), temp);
@@ -1371,9 +1360,13 @@ void intel_ddi_disable_pipe_clock(struct intel_crtc *intel_crtc)
 			   TRANS_CLK_SEL_DISABLED);
 }
 
-static void skl_ddi_set_iboost(struct drm_i915_private *dev_priv,
-			       u32 level, enum port port, int type)
+static void skl_ddi_set_iboost(struct intel_encoder *encoder,
+			       u32 level)
+
 {
+	struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+	enum port port = enc_to_dig_port(&encoder->base)->port;
 	const struct ddi_buf_trans *ddi_translations;
 	uint8_t iboost;
 	uint8_t dp_iboost, hdmi_iboost;
@@ -1384,14 +1377,14 @@ static void skl_ddi_set_iboost(struct drm_i915_private *dev_priv,
 	dp_iboost = dev_priv->vbt.ddi_port_info[port].dp_boost_level;
 	hdmi_iboost = dev_priv->vbt.ddi_port_info[port].hdmi_boost_level;
 
-	if (type == INTEL_OUTPUT_DP) {
+	if (intel_crtc_has_type(crtc->config, INTEL_OUTPUT_DP)) {
 		if (dp_iboost) {
 			iboost = dp_iboost;
 		} else {
 			ddi_translations = skl_get_buf_trans_dp(dev_priv, &n_entries);
 			iboost = ddi_translations[level].i_boost;
 		}
-	} else if (type == INTEL_OUTPUT_EDP) {
+	} else if (intel_crtc_has_type(crtc->config, INTEL_OUTPUT_EDP)) {
 		if (dp_iboost) {
 			iboost = dp_iboost;
 		} else {
@@ -1403,7 +1396,7 @@ static void skl_ddi_set_iboost(struct drm_i915_private *dev_priv,
 
 			iboost = ddi_translations[level].i_boost;
 		}
-	} else if (type == INTEL_OUTPUT_HDMI) {
+	} else if (intel_crtc_has_type(crtc->config, INTEL_OUTPUT_HDMI)) {
 		if (hdmi_iboost) {
 			iboost = hdmi_iboost;
 		} else {
@@ -1432,32 +1425,38 @@ static void skl_ddi_set_iboost(struct drm_i915_private *dev_priv,
 	I915_WRITE(DISPIO_CR_TX_BMU_CR0, reg);
 }
 
-static void bxt_ddi_vswing_sequence(struct drm_i915_private *dev_priv,
-				    u32 level, enum port port, int type)
+static void bxt_ddi_vswing_sequence(struct intel_encoder *encoder,
+				    u32 level)
 {
+	struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+	enum port port = enc_to_dig_port(&encoder->base)->port;
+
 	const struct bxt_ddi_buf_trans *ddi_translations;
 	u32 n_entries, i;
 	uint32_t val;
 
-	if (type == INTEL_OUTPUT_EDP && dev_priv->vbt.edp.low_vswing) {
+	if (intel_crtc_has_type(crtc->config, INTEL_OUTPUT_EDP) &&
+	    dev_priv->vbt.edp.low_vswing) {
 		n_entries = ARRAY_SIZE(bxt_ddi_translations_edp);
 		ddi_translations = bxt_ddi_translations_edp;
-	} else if (type == INTEL_OUTPUT_DP
-			|| type == INTEL_OUTPUT_EDP) {
+	} else if (intel_crtc_has_type(crtc->config, INTEL_OUTPUT_DP) ||
+		   intel_crtc_has_type(crtc->config, INTEL_OUTPUT_EDP)) {
 		n_entries = ARRAY_SIZE(bxt_ddi_translations_dp);
 		ddi_translations = bxt_ddi_translations_dp;
-	} else if (type == INTEL_OUTPUT_HDMI) {
+	} else if (intel_crtc_has_type(crtc->config, INTEL_OUTPUT_HDMI)) {
 		n_entries = ARRAY_SIZE(bxt_ddi_translations_hdmi);
 		ddi_translations = bxt_ddi_translations_hdmi;
 	} else {
-		DRM_DEBUG_KMS("Vswing programming not done for encoder %d\n",
-				type);
+		DRM_DEBUG_KMS("Vswing programming not done for encoder 0x%x\n",
+			      crtc->config->output_types);
 		return;
 	}
 
 	/* Check if default value has to be used */
 	if (level >= n_entries ||
-	    (type == INTEL_OUTPUT_HDMI && level == HDMI_LEVEL_SHIFT_UNKNOWN)) {
+	    (intel_crtc_has_type(crtc->config, INTEL_OUTPUT_HDMI) &&
+	     level == HDMI_LEVEL_SHIFT_UNKNOWN)) {
 		for (i = 0; i < n_entries; i++) {
 			if (ddi_translations[i].default_index) {
 				level = i;
@@ -1548,21 +1547,19 @@ static uint32_t translate_signal_level(int signal_levels)
 
 uint32_t ddi_signal_levels(struct intel_dp *intel_dp)
 {
-	struct intel_digital_port *dport = dp_to_dig_port(intel_dp);
-	struct drm_i915_private *dev_priv = to_i915(dport->base.base.dev);
-	struct intel_encoder *encoder = &dport->base;
+	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	uint8_t train_set = intel_dp->train_set[0];
 	int signal_levels = train_set & (DP_TRAIN_VOLTAGE_SWING_MASK |
 					 DP_TRAIN_PRE_EMPHASIS_MASK);
-	enum port port = dport->port;
 	uint32_t level;
 
 	level = translate_signal_level(signal_levels);
 
 	if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv))
-		skl_ddi_set_iboost(dev_priv, level, port, encoder->type);
+		skl_ddi_set_iboost(encoder, level);
 	else if (IS_BROXTON(dev_priv))
-		bxt_ddi_vswing_sequence(dev_priv, level, port, encoder->type);
+		bxt_ddi_vswing_sequence(encoder, level);
 
 	return DDI_BUF_TRANS_SELECT(level);
 }
@@ -1599,9 +1596,8 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder)
 	struct drm_i915_private *dev_priv = to_i915(encoder->dev);
 	struct intel_crtc *crtc = to_intel_crtc(encoder->crtc);
 	enum port port = intel_ddi_get_encoder_port(intel_encoder);
-	int type = intel_encoder->type;
 
-	if (type == INTEL_OUTPUT_HDMI) {
+	if (intel_crtc_has_type(crtc->config, INTEL_OUTPUT_HDMI)) {
 		struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
 
 		intel_dp_dual_mode_set_tmds_output(intel_hdmi, true);
@@ -1609,14 +1605,15 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder)
 
 	intel_prepare_ddi_buffer(intel_encoder);
 
-	if (type == INTEL_OUTPUT_EDP) {
+	if (intel_crtc_has_type(crtc->config, INTEL_OUTPUT_EDP)) {
 		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
 		intel_edp_panel_on(intel_dp);
 	}
 
 	intel_ddi_clk_select(intel_encoder, crtc->config);
 
-	if (type == INTEL_OUTPUT_DP || type == INTEL_OUTPUT_EDP) {
+	if (intel_crtc_has_type(crtc->config, INTEL_OUTPUT_DP) ||
+	    intel_crtc_has_type(crtc->config, INTEL_OUTPUT_EDP)) {
 		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
 
 		intel_dp_set_link_params(intel_dp, crtc->config);
@@ -1627,7 +1624,7 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder)
 		intel_dp_start_link_train(intel_dp);
 		if (port != PORT_A || INTEL_INFO(dev_priv)->gen >= 9)
 			intel_dp_stop_link_train(intel_dp);
-	} else if (type == INTEL_OUTPUT_HDMI) {
+	} else if (intel_crtc_has_type(crtc->config, INTEL_OUTPUT_HDMI)) {
 		struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
 
 		intel_hdmi->set_infoframes(encoder,
@@ -1639,10 +1636,10 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder)
 static void intel_ddi_post_disable(struct intel_encoder *intel_encoder)
 {
 	struct drm_encoder *encoder = &intel_encoder->base;
+	struct intel_crtc *crtc = to_intel_crtc(encoder->crtc);
 	struct drm_device *dev = encoder->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	enum port port = intel_ddi_get_encoder_port(intel_encoder);
-	int type = intel_encoder->type;
 	uint32_t val;
 	bool wait = false;
 
@@ -1661,7 +1658,8 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder)
 	if (wait)
 		intel_wait_ddi_buf_idle(dev_priv, port);
 
-	if (type == INTEL_OUTPUT_DP || type == INTEL_OUTPUT_EDP) {
+	if (intel_crtc_has_type(crtc->config, INTEL_OUTPUT_DP) ||
+	    intel_crtc_has_type(crtc->config, INTEL_OUTPUT_EDP)) {
 		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
 		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
 		intel_edp_panel_vdd_on(intel_dp);
@@ -1674,7 +1672,7 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder)
 	else if (INTEL_INFO(dev)->gen < 9)
 		I915_WRITE(PORT_CLK_SEL(port), PORT_CLK_SEL_NONE);
 
-	if (type == INTEL_OUTPUT_HDMI) {
+	if (intel_crtc_has_type(crtc->config, INTEL_OUTPUT_HDMI)) {
 		struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
 
 		intel_dp_dual_mode_set_tmds_output(intel_hdmi, false);
@@ -1689,9 +1687,8 @@ static void intel_enable_ddi(struct intel_encoder *intel_encoder)
 	struct drm_device *dev = encoder->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	enum port port = intel_ddi_get_encoder_port(intel_encoder);
-	int type = intel_encoder->type;
 
-	if (type == INTEL_OUTPUT_HDMI) {
+	if (intel_crtc_has_type(intel_crtc->config, INTEL_OUTPUT_HDMI)) {
 		struct intel_digital_port *intel_dig_port =
 			enc_to_dig_port(encoder);
 
@@ -1702,7 +1699,7 @@ static void intel_enable_ddi(struct intel_encoder *intel_encoder)
 		I915_WRITE(DDI_BUF_CTL(port),
 			   intel_dig_port->saved_port_bits |
 			   DDI_BUF_CTL_ENABLE);
-	} else if (type == INTEL_OUTPUT_EDP) {
+	} else if (intel_crtc_has_type(intel_crtc->config, INTEL_OUTPUT_EDP)) {
 		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
 
 		if (port == PORT_A && INTEL_INFO(dev)->gen < 9)
@@ -1724,7 +1721,6 @@ static void intel_disable_ddi(struct intel_encoder *intel_encoder)
 	struct drm_encoder *encoder = &intel_encoder->base;
 	struct drm_crtc *crtc = encoder->crtc;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	int type = intel_encoder->type;
 	struct drm_device *dev = encoder->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
@@ -1733,7 +1729,7 @@ static void intel_disable_ddi(struct intel_encoder *intel_encoder)
 		intel_display_power_put(dev_priv, POWER_DOMAIN_AUDIO);
 	}
 
-	if (type == INTEL_OUTPUT_EDP) {
+	if (intel_crtc_has_type(intel_crtc->config, INTEL_OUTPUT_EDP)) {
 		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
 
 		intel_edp_drrs_disable(intel_dp);
@@ -2157,6 +2153,26 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
 		return;
 
 	temp = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
+
+	switch (temp & TRANS_DDI_MODE_SELECT_MASK) {
+	case TRANS_DDI_MODE_SELECT_HDMI:
+	case TRANS_DDI_MODE_SELECT_DVI:
+		pipe_config->output_types |= 1 << INTEL_OUTPUT_HDMI;
+		break;
+	case TRANS_DDI_MODE_SELECT_DP_SST:
+		if (encoder->type == INTEL_OUTPUT_EDP)
+			pipe_config->output_types |= 1 << INTEL_OUTPUT_EDP;
+		else
+			pipe_config->output_types |= 1 << INTEL_OUTPUT_DP;
+		break;
+	case TRANS_DDI_MODE_SELECT_FDI:
+		WARN_ON(pipe_config->output_types != 1 << INTEL_OUTPUT_ANALOG);
+		break;
+	default:
+		MISSING_CASE(pipe_config->output_types);
+		break;
+	}
+
 	if (temp & TRANS_DDI_PHSYNC)
 		flags |= DRM_MODE_FLAG_PHSYNC;
 	else
@@ -2214,7 +2230,8 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
 			pipe_config->has_audio = true;
 	}
 
-	if (encoder->type == INTEL_OUTPUT_EDP && dev_priv->vbt.edp.bpp &&
+	if (intel_crtc_has_type(pipe_config, INTEL_OUTPUT_EDP) &&
+	    dev_priv->vbt.edp.bpp &&
 	    pipe_config->pipe_bpp > dev_priv->vbt.edp.bpp) {
 		/*
 		 * This is a big fat ugly hack.
@@ -2237,18 +2254,54 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
 	intel_ddi_clock_get(encoder, pipe_config);
 }
 
+static enum intel_output_type
+intel_ddi_compute_output_type(struct intel_encoder *encoder,
+			      struct intel_crtc_state *crtc_state)
+{
+	struct drm_atomic_state *state = crtc_state->base.state;
+	struct drm_crtc *crtc = crtc_state->base.crtc;
+	struct drm_connector *connector;
+	struct drm_connector_state *connector_state;
+	int i;
+
+	for_each_connector_in_state(state, connector, connector_state, i) {
+		if (connector_state->crtc != crtc)
+			continue;
+
+		if (connector_state->best_encoder != &encoder->base)
+			continue;
+
+		switch (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(connector->connector_type);
+			break;
+		}
+	}
+
+	return INTEL_OUTPUT_UNUSED;
+}
+
 static bool intel_ddi_compute_config(struct intel_encoder *encoder,
 				     struct intel_crtc_state *pipe_config)
 {
-	int type = encoder->type;
-	int port = intel_ddi_get_encoder_port(encoder);
+	enum port port = intel_ddi_get_encoder_port(encoder);
 
-	WARN(type == INTEL_OUTPUT_UNKNOWN, "compute_config() on unknown output!\n");
+	WARN(pipe_config->output_types == 0 ||
+	     pipe_config->output_types & ~(1 << INTEL_OUTPUT_EDP |
+					   1 << INTEL_OUTPUT_DP |
+					   1 << INTEL_OUTPUT_HDMI),
+	     "Bad output_types = 0x%x\n", pipe_config->output_types);
 
 	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))
 		return intel_hdmi_compute_config(encoder, pipe_config);
 	else
 		return intel_dp_compute_config(encoder, pipe_config);
@@ -2348,6 +2401,7 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
 	drm_encoder_init(dev, 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;
 	intel_encoder->pre_enable = intel_ddi_pre_enable;
@@ -2379,7 +2433,7 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
 
 	intel_dig_port->max_lanes = max_lanes;
 
-	intel_encoder->type = INTEL_OUTPUT_UNKNOWN;
+	intel_encoder->type = INTEL_OUTPUT_DDI;
 	intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
 	intel_encoder->cloneable = 0;
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 442ed6320082..a6d723a50ccb 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5113,7 +5113,7 @@ intel_display_port_power_domain(struct intel_encoder *intel_encoder)
 	struct intel_digital_port *intel_dig_port;
 
 	switch (intel_encoder->type) {
-	case INTEL_OUTPUT_UNKNOWN:
+	case INTEL_OUTPUT_DDI:
 		/* Only DDI platforms should ever use this output type */
 		WARN_ON_ONCE(!HAS_DDI(dev));
 	case INTEL_OUTPUT_DP:
@@ -5140,15 +5140,8 @@ intel_display_port_aux_power_domain(struct intel_encoder *intel_encoder)
 	struct intel_digital_port *intel_dig_port;
 
 	switch (intel_encoder->type) {
-	case INTEL_OUTPUT_UNKNOWN:
-	case INTEL_OUTPUT_HDMI:
-		/*
-		 * Only DDI platforms should ever use these output types.
-		 * We can get here after the HDMI detect code has already set
-		 * the type of the shared encoder. Since we can't be sure
-		 * what's the status of the given connectors, play safe and
-		 * run the DP detection too.
-		 */
+	case INTEL_OUTPUT_DDI:
+		/* Only DDI platforms should ever use this output type */
 		WARN_ON_ONCE(!HAS_DDI(dev));
 	case INTEL_OUTPUT_DP:
 	case INTEL_OUTPUT_EDP:
@@ -12341,7 +12334,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(dev)))
 				break;
 		case INTEL_OUTPUT_DP:
@@ -12447,6 +12440,8 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
 
 		encoder = to_intel_encoder(connector_state->best_encoder);
 
+		DRM_DEBUG_KMS("Using encoder %s\n", encoder->base.name);
+
 		if (!check_single_encoder_cloning(state, to_intel_crtc(crtc), encoder)) {
 			DRM_DEBUG_KMS("rejecting invalid cloning configuration\n");
 			goto fail;
@@ -12456,7 +12451,11 @@ 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 |=
+				1 << encoder->compute_output_type(encoder, pipe_config);
+		else
+			pipe_config->output_types |= 1 << encoder->type;
 	}
 
 encoder_retry:
@@ -13023,7 +13022,12 @@ verify_crtc_state(struct drm_crtc *crtc,
 				pipe_name(pipe));
 
 		if (active) {
-			pipe_config->output_types |= 1 << encoder->type;
+			/*
+			 * .get_config() is responsible for this if the
+			 * encoder has multiple personalities
+			 */
+			if (!encoder->compute_output_type)
+				pipe_config->output_types |= 1 << encoder->type;
 			encoder->get_config(encoder, pipe_config);
 		}
 	}
@@ -15916,7 +15920,12 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
 		if (encoder->get_hw_state(encoder, &pipe)) {
 			crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
 			encoder->base.crtc = &crtc->base;
-			crtc->config->output_types |= 1 << encoder->type;
+			/*
+			 * .get_config() is responsible for this if the
+			 * encoder has multiple personalities
+			 */
+			if (!encoder->compute_output_type)
+				crtc->config->output_types |= 1 << encoder->type;
 			encoder->get_config(encoder, crtc->config);
 		} else {
 			encoder->base.crtc = NULL;
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index e34426dc5c1e..5ee7775d71da 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4200,9 +4200,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;
-
 	intel_dp_probe_oui(intel_dp);
 
 	ret = intel_dp_probe_mst(intel_dp);
@@ -4266,8 +4263,6 @@ static enum drm_connector_status
 intel_dp_detect(struct drm_connector *connector, bool force)
 {
 	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 intel_connector *intel_connector = to_intel_connector(connector);
 
 	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
@@ -4276,8 +4271,6 @@ intel_dp_detect(struct drm_connector *connector, bool force)
 	if (intel_dp->is_mst) {
 		/* MST devices are disconnected from a monitor POV */
 		intel_dp_unset_edid(intel_dp);
-		if (intel_encoder->type != INTEL_OUTPUT_EDP)
-			intel_encoder->type = INTEL_OUTPUT_DP;
 		return connector_status_disconnected;
 	}
 
@@ -4314,9 +4307,6 @@ intel_dp_force(struct drm_connector *connector)
 	intel_dp_set_edid(intel_dp);
 
 	intel_display_power_put(dev_priv, 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)
@@ -4595,10 +4585,6 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
 	enum intel_display_power_domain power_domain;
 	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
@@ -5455,7 +5441,7 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 	/*
 	 * For eDP we always set the encoder type to INTEL_OUTPUT_EDP, but
 	 * for DP the encoder type can be set by the caller to
-	 * INTEL_OUTPUT_UNKNOWN for DDI, so don't rewrite it.
+	 * INTEL_OUTPUT_DDI, so don't rewrite it.
 	 */
 	if (type == DRM_MODE_CONNECTOR_eDP)
 		intel_encoder->type = INTEL_OUTPUT_EDP;
diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index 5f96466e4e47..f2e2829ebf51 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -698,7 +698,7 @@ 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)) {
 		uint32_t val;
 		unsigned p, n2, r2;
 
@@ -713,9 +713,7 @@ hsw_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
 		pll = intel_find_shared_dpll(crtc, crtc_state,
 					     DPLL_ID_WRPLL1, DPLL_ID_WRPLL2);
 
-	} 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)) {
 		enum intel_dpll_id pll_id;
 
 		switch (clock / 2) {
@@ -735,7 +733,7 @@ hsw_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
 
 		pll = intel_get_shared_dpll_by_id(dev_priv, pll_id);
 
-	} 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;
 
@@ -1205,7 +1203,7 @@ skl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
 
 	ctrl1 = DPLL_CTRL1_OVERRIDE(0);
 
-	if (encoder->type == INTEL_OUTPUT_HDMI) {
+	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI)) {
 		struct skl_wrpll_params wrpll_params = { 0, };
 
 		ctrl1 |= DPLL_CTRL1_HDMI_MODE(0);
@@ -1222,9 +1220,7 @@ skl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
 			 DPLL_CFGCR2_KDIV(wrpll_params.kdiv) |
 			 DPLL_CFGCR2_PDIV(wrpll_params.pdiv) |
 			 wrpll_params.central_freq;
-	} 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)) {
 		switch (crtc_state->port_clock / 2) {
 		case 81000:
 			ctrl1 |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_810, 0);
@@ -1259,7 +1255,7 @@ skl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
 	crtc_state->dpll_hw_state.cfgcr1 = cfgcr1;
 	crtc_state->dpll_hw_state.cfgcr2 = cfgcr2;
 
-	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);
@@ -1507,7 +1503,7 @@ bxt_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
 	uint32_t lanestagger;
 	int clock = crtc_state->port_clock;
 
-	if (encoder->type == INTEL_OUTPUT_HDMI) {
+	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI)) {
 		struct dpll best_clock;
 
 		/* Calculate HDMI div */
@@ -1530,8 +1526,7 @@ bxt_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
 		clk_div.m2_frac_en = clk_div.m2_frac != 0;
 
 		vco = best_clock.vco;
-	} else if (encoder->type == INTEL_OUTPUT_DP ||
-		   encoder->type == INTEL_OUTPUT_EDP) {
+	} else if (intel_crtc_has_dp_encoder(crtc_state)) {
 		int i;
 
 		clk_div = bxt_dp_clk_val[0];
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 3cc0d4fcdc36..d829d57c9fad 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -138,7 +138,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,
 };
 
@@ -168,6 +168,8 @@ struct intel_encoder {
 	enum intel_output_type type;
 	unsigned int cloneable;
 	void (*hot_plug)(struct intel_encoder *);
+	enum intel_output_type (*compute_output_type)(struct intel_encoder *,
+						      struct intel_crtc_state *);
 	bool (*compute_config)(struct intel_encoder *,
 			       struct intel_crtc_state *);
 	void (*pre_pll_enable)(struct intel_encoder *);
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 1b91d65ecfb9..830968cdc1c7 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1495,12 +1495,9 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
 
 	intel_hdmi_unset_edid(connector);
 
-	if (intel_hdmi_set_edid(connector, live_status)) {
-		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, live_status))
 		status = connector_status_connected;
-	} else
+	else
 		status = connector_status_disconnected;
 
 	intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS);
@@ -1511,8 +1508,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);
 
@@ -1522,7 +1517,6 @@ intel_hdmi_force(struct drm_connector *connector)
 		return;
 
 	intel_hdmi_set_edid(connector, true);
-	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_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
index e61317f5a586..bf9df664cf10 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -365,7 +365,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:
-- 
2.7.4

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

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

* ✓ Ro.CI.BAT: success for drm/i915: Eliminate DDI encoder->type frobbery
  2016-06-08 10:41 [PATCH 00/12] drm/i915: Eliminate DDI encoder->type frobbery ville.syrjala
                   ` (11 preceding siblings ...)
  2016-06-08 10:41 ` [PATCH 12/12] drm/i915: Stop frobbing with DDI encoder->type ville.syrjala
@ 2016-06-08 11:13 ` Patchwork
  12 siblings, 0 replies; 28+ messages in thread
From: Patchwork @ 2016-06-08 11:13 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Eliminate DDI encoder->type frobbery
URL   : https://patchwork.freedesktop.org/series/8439/
State : success

== Summary ==

Series 8439v1 drm/i915: Eliminate DDI encoder->type frobbery
http://patchwork.freedesktop.org/api/1.0/series/8439/revisions/1/mbox


fi-bdw-i7-5557u  total:102  pass:93   dwarn:0   dfail:0   fail:0   skip:8  
fi-skl-i5-6260u  total:209  pass:198  dwarn:0   dfail:0   fail:0   skip:11 
fi-skl-i7-6700k  total:209  pass:184  dwarn:0   dfail:0   fail:0   skip:25 
fi-snb-i7-2600   total:209  pass:170  dwarn:0   dfail:0   fail:0   skip:39 
ro-bdw-i5-5250u  total:183  pass:172  dwarn:0   dfail:0   fail:0   skip:10 
ro-bdw-i7-5600u  total:161  pass:137  dwarn:0   dfail:0   fail:0   skip:23 
ro-bsw-n3050     total:208  pass:166  dwarn:1   dfail:0   fail:2   skip:39 
ro-byt-n2820     total:208  pass:168  dwarn:0   dfail:0   fail:3   skip:37 
ro-hsw-i3-4010u  total:208  pass:185  dwarn:0   dfail:0   fail:0   skip:23 
ro-hsw-i7-4770r  total:183  pass:161  dwarn:0   dfail:0   fail:0   skip:21 
ro-ilk1-i5-650   total:204  pass:146  dwarn:0   dfail:0   fail:1   skip:57 
ro-ivb-i7-3770   total:54   pass:51   dwarn:0   dfail:0   fail:0   skip:3  
ro-ivb2-i7-3770  total:183  pass:158  dwarn:0   dfail:0   fail:0   skip:24 
ro-snb-i7-2620M  total:183  pass:151  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-i7-4770k failed to connect after reboot
ro-bdw-i7-5557U failed to connect after reboot
ro-bdw-i7-5600u failed to connect after reboot
ro-ivb-i7-3770 failed to connect after reboot

Results at /archive/results/CI_IGT_test/RO_Patchwork_1136/

131a54b drm-intel-nightly: 2016y-06m-07d-19h-46m-33s UTC integration manifest
22c492e drm/i915: Stop frobbing with DDI encoder->type
839a38c drm/i915: Check for invalid cloning earlier during modeset
f1341cd drm/i915: Simplify hdmi_12bpc_possible()
a1d7cbf drm/i915: Kill has_dsi_encoder
97669c4 drm/i915: s/INTEL_OUTPUT_DISPLAYPORT/INTEL_OUTPUT_DP/
0babbea drm/i915: Replace some open coded intel_crtc_has_dp_encoder()s
4349ca8e drm/i915: Kill has_dp_encoder from pipe_config
60b1828 drm/i915: Replace manual lvds and sdvo/hdmi counting with intel_crtc_has_type()
6b50b1c drm/i915: Unify intel_pipe_has_type() and intel_pipe_will_have_type()
0ade9c1 drm/i915: Add output_types bitmask into the crtc state
1435be2 drm/i915: Remove encoder type checks from MST suspend/resume
5f65c4a drm/i915: Don't mark eDP encoders as MST capable

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

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

* Re: [PATCH 08/12] drm/i915: s/INTEL_OUTPUT_DISPLAYPORT/INTEL_OUTPUT_DP/
  2016-06-08 10:41 ` [PATCH 08/12] drm/i915: s/INTEL_OUTPUT_DISPLAYPORT/INTEL_OUTPUT_DP/ ville.syrjala
@ 2016-06-08 13:05   ` Kahola, Mika
  0 siblings, 0 replies; 28+ messages in thread
From: Kahola, Mika @ 2016-06-08 13:05 UTC (permalink / raw)
  To: ville.syrjala, intel-gfx

I'll second this idea.

With small typo fixed on commit message this is
Reviewed-by: Mika Kahola <mika.kahola@intel.com>

> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf
> Of ville.syrjala@linux.intel.com
> Sent: Wednesday, June 8, 2016 1:42 PM
> To: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH 08/12] drm/i915:
> s/INTEL_OUTPUT_DISPLAYPORT/INTEL_OUTPUT_DP/
> 
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> INTEL_OUTPUT_DISPLAYPORT hsa been bugging me for a long time. It always
> looks out of place besides INTEL_OUTPUT_EDP and
> INTEL_OUTPUT_DP_MST.
> Let's just rename it to INTEL_OUTPUT_DP.
> 
> v2: Rebase
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c   |  8 ++++----
>  drivers/gpu/drm/i915/intel_ddi.c      | 16 ++++++++--------
>  drivers/gpu/drm/i915/intel_display.c  | 10 +++++-----
>  drivers/gpu/drm/i915/intel_dp.c       | 10 +++++-----
>  drivers/gpu/drm/i915/intel_dpll_mgr.c |  6 +++---
>  drivers/gpu/drm/i915/intel_drv.h      |  2 +-
>  drivers/gpu/drm/i915/intel_opregion.c |  2 +-
>  7 files changed, 27 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index e4f2c55d9697..cb69e4b361b5 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2986,7 +2986,7 @@ static void intel_connector_info(struct seq_file *m,
>  			   connector->display_info.cea_rev);
>  	}
>  	if (intel_encoder) {
> -		if (intel_encoder->type == INTEL_OUTPUT_DISPLAYPORT ||
> +		if (intel_encoder->type == INTEL_OUTPUT_DP ||
>  		    intel_encoder->type == INTEL_OUTPUT_EDP)
>  			intel_dp_info(m, intel_connector);
>  		else if (intel_encoder->type == INTEL_OUTPUT_HDMI) @@ -
> 3387,7 +3387,7 @@ static void drrs_status_per_crtc(struct seq_file *m,
>  		case INTEL_OUTPUT_HDMI:
>  			seq_puts(m, "HDMI:\n");
>  			break;
> -		case INTEL_OUTPUT_DISPLAYPORT:
> +		case INTEL_OUTPUT_DP:
>  			seq_puts(m, "DP:\n");
>  			break;
>  		default:
> @@ -3492,7 +3492,7 @@ static int i915_dp_mst_info(struct seq_file *m, void
> *unused)
>  	drm_modeset_lock_all(dev);
>  	list_for_each_entry(encoder, &dev->mode_config.encoder_list,
> head) {
>  		intel_encoder = to_intel_encoder(encoder);
> -		if (intel_encoder->type != INTEL_OUTPUT_DISPLAYPORT)
> +		if (intel_encoder->type != INTEL_OUTPUT_DP)
>  			continue;
>  		intel_dig_port = enc_to_dig_port(encoder);
>  		if (!intel_dig_port->dp.can_mst)
> @@ -3754,7 +3754,7 @@ static int i9xx_pipe_crc_auto_source(struct
> drm_device *dev, enum pipe pipe,
>  		case INTEL_OUTPUT_TVOUT:
>  			*source = INTEL_PIPE_CRC_SOURCE_TV;
>  			break;
> -		case INTEL_OUTPUT_DISPLAYPORT:
> +		case INTEL_OUTPUT_DP:
>  		case INTEL_OUTPUT_EDP:
>  			dig_port = enc_to_dig_port(&encoder->base);
>  			switch (dig_port->port) {
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> b/drivers/gpu/drm/i915/intel_ddi.c
> index b7285336fb53..4f7fc0ed8796 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -318,7 +318,7 @@ static void ddi_get_encoder_port(struct
> intel_encoder *intel_encoder,
>  	default:
>  		WARN(1, "Invalid DDI encoder type %d\n", intel_encoder-
> >type);
>  		/* fallthrough and treat as unknown */
> -	case INTEL_OUTPUT_DISPLAYPORT:
> +	case INTEL_OUTPUT_DP:
>  	case INTEL_OUTPUT_EDP:
>  	case INTEL_OUTPUT_HDMI:
>  	case INTEL_OUTPUT_UNKNOWN:
> @@ -482,7 +482,7 @@ void intel_prepare_ddi_buffer(struct intel_encoder
> *encoder)
>  		ddi_translations = ddi_translations_edp;
>  		size = n_edp_entries;
>  		break;
> -	case INTEL_OUTPUT_DISPLAYPORT:
> +	case INTEL_OUTPUT_DP:
>  	case INTEL_OUTPUT_HDMI:
>  		ddi_translations = ddi_translations_dp;
>  		size = n_dp_entries;
> @@ -1068,7 +1068,7 @@ void intel_ddi_set_pipe_settings(struct drm_crtc
> *crtc)
>  	int type = intel_encoder->type;
>  	uint32_t temp;
> 
> -	if (type == INTEL_OUTPUT_DISPLAYPORT || type ==
> INTEL_OUTPUT_EDP || type == INTEL_OUTPUT_DP_MST) {
> +	if (type == INTEL_OUTPUT_DP || type == INTEL_OUTPUT_EDP ||
> type ==
> +INTEL_OUTPUT_DP_MST) {
>  		WARN_ON(transcoder_is_dsi(cpu_transcoder));
> 
>  		temp = TRANS_MSA_SYNC_CLK;
> @@ -1182,7 +1182,7 @@ void intel_ddi_enable_transcoder_func(struct
> drm_crtc *crtc)
>  		temp |= TRANS_DDI_MODE_SELECT_FDI;
>  		temp |= (intel_crtc->config->fdi_lanes - 1) << 1;
> 
> -	} else if (type == INTEL_OUTPUT_DISPLAYPORT ||
> +	} else if (type == INTEL_OUTPUT_DP ||
>  		   type == INTEL_OUTPUT_EDP) {
>  		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> 
> @@ -1384,7 +1384,7 @@ static void skl_ddi_set_iboost(struct
> drm_i915_private *dev_priv,
>  	dp_iboost = dev_priv->vbt.ddi_port_info[port].dp_boost_level;
>  	hdmi_iboost = dev_priv->vbt.ddi_port_info[port].hdmi_boost_level;
> 
> -	if (type == INTEL_OUTPUT_DISPLAYPORT) {
> +	if (type == INTEL_OUTPUT_DP) {
>  		if (dp_iboost) {
>  			iboost = dp_iboost;
>  		} else {
> @@ -1442,7 +1442,7 @@ static void bxt_ddi_vswing_sequence(struct
> drm_i915_private *dev_priv,
>  	if (type == INTEL_OUTPUT_EDP && dev_priv->vbt.edp.low_vswing) {
>  		n_entries = ARRAY_SIZE(bxt_ddi_translations_edp);
>  		ddi_translations = bxt_ddi_translations_edp;
> -	} else if (type == INTEL_OUTPUT_DISPLAYPORT
> +	} else if (type == INTEL_OUTPUT_DP
>  			|| type == INTEL_OUTPUT_EDP) {
>  		n_entries = ARRAY_SIZE(bxt_ddi_translations_dp);
>  		ddi_translations = bxt_ddi_translations_dp; @@ -1616,7
> +1616,7 @@ static void intel_ddi_pre_enable(struct intel_encoder
> *intel_encoder)
> 
>  	intel_ddi_clk_select(intel_encoder, crtc->config);
> 
> -	if (type == INTEL_OUTPUT_DISPLAYPORT || type ==
> INTEL_OUTPUT_EDP) {
> +	if (type == INTEL_OUTPUT_DP || type == INTEL_OUTPUT_EDP) {
>  		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> 
>  		intel_dp_set_link_params(intel_dp, crtc->config); @@ -
> 1661,7 +1661,7 @@ static void intel_ddi_post_disable(struct intel_encoder
> *intel_encoder)
>  	if (wait)
>  		intel_wait_ddi_buf_idle(dev_priv, port);
> 
> -	if (type == INTEL_OUTPUT_DISPLAYPORT || type ==
> INTEL_OUTPUT_EDP) {
> +	if (type == INTEL_OUTPUT_DP || type == INTEL_OUTPUT_EDP) {
>  		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
>  		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
>  		intel_edp_panel_vdd_on(intel_dp);
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 3c78f07a50e1..de8e4acf8f76 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -540,7 +540,7 @@ bool intel_crtc_has_type(const struct intel_crtc_state
> *crtc_state,  bool intel_crtc_has_dp_encoder(const struct intel_crtc_state
> *crtc_state)  {
>  	return crtc_state->output_types &
> -		((1 << INTEL_OUTPUT_DISPLAYPORT) |
> +		((1 << INTEL_OUTPUT_DP) |
>  		 (1 << INTEL_OUTPUT_DP_MST) |
>  		 (1 << INTEL_OUTPUT_EDP));
>  }
> @@ -4036,7 +4036,7 @@ intel_trans_dp_port_sel(struct drm_crtc *crtc)
>  	struct intel_encoder *encoder;
> 
>  	for_each_encoder_on_crtc(dev, crtc, encoder) {
> -		if (encoder->type == INTEL_OUTPUT_DISPLAYPORT ||
> +		if (encoder->type == INTEL_OUTPUT_DP ||
>  		    encoder->type == INTEL_OUTPUT_EDP)
>  			return enc_to_dig_port(&encoder->base)->port;
>  	}
> @@ -5116,7 +5116,7 @@ intel_display_port_power_domain(struct
> intel_encoder *intel_encoder)
>  	case INTEL_OUTPUT_UNKNOWN:
>  		/* Only DDI platforms should ever use this output type */
>  		WARN_ON_ONCE(!HAS_DDI(dev));
> -	case INTEL_OUTPUT_DISPLAYPORT:
> +	case INTEL_OUTPUT_DP:
>  	case INTEL_OUTPUT_HDMI:
>  	case INTEL_OUTPUT_EDP:
>  		intel_dig_port = enc_to_dig_port(&intel_encoder->base);
> @@ -5150,7 +5150,7 @@ intel_display_port_aux_power_domain(struct
> intel_encoder *intel_encoder)
>  		 * run the DP detection too.
>  		 */
>  		WARN_ON_ONCE(!HAS_DDI(dev));
> -	case INTEL_OUTPUT_DISPLAYPORT:
> +	case INTEL_OUTPUT_DP:
>  	case INTEL_OUTPUT_EDP:
>  		intel_dig_port = enc_to_dig_port(&intel_encoder->base);
>  		return port_to_aux_power_domain(intel_dig_port->port);
> @@ -12377,7 +12377,7 @@ static bool check_digital_port_conflicts(struct
> drm_atomic_state *state)
>  		case INTEL_OUTPUT_UNKNOWN:
>  			if (WARN_ON(!HAS_DDI(dev)))
>  				break;
> -		case INTEL_OUTPUT_DISPLAYPORT:
> +		case INTEL_OUTPUT_DP:
>  		case INTEL_OUTPUT_HDMI:
>  		case INTEL_OUTPUT_EDP:
>  			port_mask = 1 << enc_to_dig_port(&encoder-
> >base)->port;
> diff --git a/drivers/gpu/drm/i915/intel_dp.c
> b/drivers/gpu/drm/i915/intel_dp.c index 207e82695cfa..e34426dc5c1e
> 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4201,7 +4201,7 @@ intel_dp_long_pulse(struct intel_connector
> *intel_connector)
>  	}
> 
>  	if (intel_encoder->type != INTEL_OUTPUT_EDP)
> -		intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
> +		intel_encoder->type = INTEL_OUTPUT_DP;
> 
>  	intel_dp_probe_oui(intel_dp);
> 
> @@ -4277,7 +4277,7 @@ intel_dp_detect(struct drm_connector *connector,
> bool force)
>  		/* MST devices are disconnected from a monitor POV */
>  		intel_dp_unset_edid(intel_dp);
>  		if (intel_encoder->type != INTEL_OUTPUT_EDP)
> -			intel_encoder->type =
> INTEL_OUTPUT_DISPLAYPORT;
> +			intel_encoder->type = INTEL_OUTPUT_DP;
>  		return connector_status_disconnected;
>  	}
> 
> @@ -4316,7 +4316,7 @@ intel_dp_force(struct drm_connector *connector)
>  	intel_display_power_put(dev_priv, power_domain);
> 
>  	if (intel_encoder->type != INTEL_OUTPUT_EDP)
> -		intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
> +		intel_encoder->type = INTEL_OUTPUT_DP;
>  }
> 
>  static int intel_dp_get_modes(struct drm_connector *connector) @@ -
> 4597,7 +4597,7 @@ intel_dp_hpd_pulse(struct intel_digital_port
> *intel_dig_port, bool long_hpd)
> 
>  	if (intel_dig_port->base.type != INTEL_OUTPUT_EDP &&
>  	    intel_dig_port->base.type != INTEL_OUTPUT_HDMI)
> -		intel_dig_port->base.type = INTEL_OUTPUT_DISPLAYPORT;
> +		intel_dig_port->base.type = INTEL_OUTPUT_DP;
> 
>  	if (long_hpd && intel_dig_port->base.type == INTEL_OUTPUT_EDP) {
>  		/*
> @@ -5620,7 +5620,7 @@ bool intel_dp_init(struct drm_device *dev,
>  	intel_dig_port->dp.output_reg = output_reg;
>  	intel_dig_port->max_lanes = 4;
> 
> -	intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
> +	intel_encoder->type = INTEL_OUTPUT_DP;
>  	if (IS_CHERRYVIEW(dev)) {
>  		if (port == PORT_D)
>  			intel_encoder->crtc_mask = 1 << 2;
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> index c0eff1571731..5f96466e4e47 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> @@ -713,7 +713,7 @@ hsw_get_dpll(struct intel_crtc *crtc, struct
> intel_crtc_state *crtc_state,
>  		pll = intel_find_shared_dpll(crtc, crtc_state,
>  					     DPLL_ID_WRPLL1,
> DPLL_ID_WRPLL2);
> 
> -	} else if (encoder->type == INTEL_OUTPUT_DISPLAYPORT ||
> +	} else if (encoder->type == INTEL_OUTPUT_DP ||
>  		   encoder->type == INTEL_OUTPUT_DP_MST ||
>  		   encoder->type == INTEL_OUTPUT_EDP) {
>  		enum intel_dpll_id pll_id;
> @@ -1222,7 +1222,7 @@ skl_get_dpll(struct intel_crtc *crtc, struct
> intel_crtc_state *crtc_state,
>  			 DPLL_CFGCR2_KDIV(wrpll_params.kdiv) |
>  			 DPLL_CFGCR2_PDIV(wrpll_params.pdiv) |
>  			 wrpll_params.central_freq;
> -	} else if (encoder->type == INTEL_OUTPUT_DISPLAYPORT ||
> +	} else if (encoder->type == INTEL_OUTPUT_DP ||
>  		   encoder->type == INTEL_OUTPUT_DP_MST ||
>  		   encoder->type == INTEL_OUTPUT_EDP) {
>  		switch (crtc_state->port_clock / 2) { @@ -1530,7 +1530,7 @@
> bxt_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
>  		clk_div.m2_frac_en = clk_div.m2_frac != 0;
> 
>  		vco = best_clock.vco;
> -	} else if (encoder->type == INTEL_OUTPUT_DISPLAYPORT ||
> +	} else if (encoder->type == INTEL_OUTPUT_DP ||
>  		   encoder->type == INTEL_OUTPUT_EDP) {
>  		int i;
> 
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 2c70dd677a00..bd67339357d1 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -135,7 +135,7 @@ enum intel_output_type {
>  	INTEL_OUTPUT_LVDS = 4,
>  	INTEL_OUTPUT_TVOUT = 5,
>  	INTEL_OUTPUT_HDMI = 6,
> -	INTEL_OUTPUT_DISPLAYPORT = 7,
> +	INTEL_OUTPUT_DP = 7,
>  	INTEL_OUTPUT_EDP = 8,
>  	INTEL_OUTPUT_DSI = 9,
>  	INTEL_OUTPUT_UNKNOWN = 10,
> diff --git a/drivers/gpu/drm/i915/intel_opregion.c
> b/drivers/gpu/drm/i915/intel_opregion.c
> index f6d8a21d2c49..e61317f5a586 100644
> --- a/drivers/gpu/drm/i915/intel_opregion.c
> +++ b/drivers/gpu/drm/i915/intel_opregion.c
> @@ -366,7 +366,7 @@ int intel_opregion_notify_encoder(struct
> intel_encoder *intel_encoder,
>  		type = DISPLAY_TYPE_CRT;
>  		break;
>  	case INTEL_OUTPUT_UNKNOWN:
> -	case INTEL_OUTPUT_DISPLAYPORT:
> +	case INTEL_OUTPUT_DP:
>  	case INTEL_OUTPUT_HDMI:
>  	case INTEL_OUTPUT_DP_MST:
>  		type = DISPLAY_TYPE_EXTERNAL_FLAT_PANEL;
> --
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 11/12] drm/i915: Check for invalid cloning earlier during modeset
  2016-06-08 10:41 ` [PATCH 11/12] drm/i915: Check for invalid cloning earlier during modeset ville.syrjala
@ 2016-06-08 13:15   ` Chris Wilson
  2016-06-08 13:27     ` Ville Syrjälä
  0 siblings, 1 reply; 28+ messages in thread
From: Chris Wilson @ 2016-06-08 13:15 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

On Wed, Jun 08, 2016 at 01:41:46PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Move the encoder cloning check to happen earlier in the modeset. The
> main benefit will be that the debug output from a failed modeset will
> be less confusing as output_types can not indicate an invalid
> configuration during the later computation stages.
> 
> For instance, what happened to me was kms_setmode was attempting one
> of its invalid cloning checks during which it asked for DP+VGA cloning
> on HSW. In this case the DP .compute_config() was executed after
> the FDI .compute_config() leaving the DP link clock (1.62 in this case)
> in port_clock, and then later the FDI BW computation tried to use that
> as the FDI link clock (which should always be 2.7). 1.62 x 2 wasn't
> enough for the mode it was trying to use, and so it ended up rejecting
> the modeset, not because of an invalid cloning configuration, but
> because of supposedly running out of FDI bandwidth. Took me a while
> to figure out what had actually happened.

Did it reject the 1.62 link clock when you simply removed the
check_encoder_cloning()? Just checking... :)
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 03/12] drm/i915: Add output_types bitmask into the crtc state
  2016-06-08 10:41 ` [PATCH 03/12] drm/i915: Add output_types bitmask into the crtc state ville.syrjala
@ 2016-06-08 13:25   ` Chris Wilson
  2016-06-08 13:33     ` Ville Syrjälä
  2016-06-13 14:25   ` Daniel Vetter
  1 sibling, 1 reply; 28+ messages in thread
From: Chris Wilson @ 2016-06-08 13:25 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

On Wed, Jun 08, 2016 at 01:41:38PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Rather than looping through encoders to see which encoder types
> are being driven by the pipe, add an output_types bitmask into
> the crtc state and populate it prior to compute_config and during
> state readout.
> 
> v2: Determine output_types before .compute_config() hooks are called
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 51 +++++++++++++++---------------------
>  drivers/gpu/drm/i915/intel_drv.h     |  5 ++++
>  2 files changed, 26 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 12ff79594bc1..eabace447b1c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -535,14 +535,7 @@ needs_modeset(struct drm_crtc_state *state)
>   */
>  bool intel_pipe_has_type(struct intel_crtc *crtc, enum intel_output_type type)
>  {
> -	struct drm_device *dev = crtc->base.dev;
> -	struct intel_encoder *encoder;
> -
> -	for_each_encoder_on_crtc(dev, &crtc->base, encoder)
> -		if (encoder->type == type)
> -			return true;
> -
> -	return false;
> +	return crtc->config->output_types & (1 << type);

This could become inline and then all those

	if (intel_pipe_has_type(crtc, VGA) ||
	    intel_pipe_has_type(crtc, LVDS) ||
	    intel_pipe_has_type(crtc, HDMI))

whatnots will just disapear into a single test.

I won't say how long those loops have been upsetting me without doing
anything about them.

> @@ -12529,6 +12503,19 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
>  			       &pipe_config->pipe_src_w,
>  			       &pipe_config->pipe_src_h);
>  
> +	for_each_connector_in_state(state, connector, connector_state, i) {
> +		if (connector_state->crtc != crtc)
> +			continue;
> +
> +		encoder = to_intel_encoder(connector_state->best_encoder);
> +
> +		/*
> +		 * 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;

pipe_config->output_types |= BIT(encoder->type);

Not my personal favourite in obfuscation, but one available to us if we
desire.

The code looks correct, I can't comment on ordering between computation
of mask and use though.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 11/12] drm/i915: Check for invalid cloning earlier during modeset
  2016-06-08 13:15   ` Chris Wilson
@ 2016-06-08 13:27     ` Ville Syrjälä
  2016-06-20 13:54       ` Maarten Lankhorst
  0 siblings, 1 reply; 28+ messages in thread
From: Ville Syrjälä @ 2016-06-08 13:27 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx

On Wed, Jun 08, 2016 at 02:15:25PM +0100, Chris Wilson wrote:
> On Wed, Jun 08, 2016 at 01:41:46PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Move the encoder cloning check to happen earlier in the modeset. The
> > main benefit will be that the debug output from a failed modeset will
> > be less confusing as output_types can not indicate an invalid
> > configuration during the later computation stages.
> > 
> > For instance, what happened to me was kms_setmode was attempting one
> > of its invalid cloning checks during which it asked for DP+VGA cloning
> > on HSW. In this case the DP .compute_config() was executed after
> > the FDI .compute_config() leaving the DP link clock (1.62 in this case)
> > in port_clock, and then later the FDI BW computation tried to use that
> > as the FDI link clock (which should always be 2.7). 1.62 x 2 wasn't
> > enough for the mode it was trying to use, and so it ended up rejecting
> > the modeset, not because of an invalid cloning configuration, but
> > because of supposedly running out of FDI bandwidth. Took me a while
> > to figure out what had actually happened.
> 
> Did it reject the 1.62 link clock when you simply removed the
> check_encoder_cloning()? Just checking... :)

Nope. The rejection only happened due to exceeding the max fdi lane
count. I think I had a warn in there somewhere when I originally
submitted the fdi==port_clock patch, but that apparently didn't
survive into the version that eventually got merged.

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

* Re: [PATCH 03/12] drm/i915: Add output_types bitmask into the crtc state
  2016-06-08 13:25   ` Chris Wilson
@ 2016-06-08 13:33     ` Ville Syrjälä
  0 siblings, 0 replies; 28+ messages in thread
From: Ville Syrjälä @ 2016-06-08 13:33 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx

On Wed, Jun 08, 2016 at 02:25:25PM +0100, Chris Wilson wrote:
> On Wed, Jun 08, 2016 at 01:41:38PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Rather than looping through encoders to see which encoder types
> > are being driven by the pipe, add an output_types bitmask into
> > the crtc state and populate it prior to compute_config and during
> > state readout.
> > 
> > v2: Determine output_types before .compute_config() hooks are called
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 51 +++++++++++++++---------------------
> >  drivers/gpu/drm/i915/intel_drv.h     |  5 ++++
> >  2 files changed, 26 insertions(+), 30 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 12ff79594bc1..eabace447b1c 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -535,14 +535,7 @@ needs_modeset(struct drm_crtc_state *state)
> >   */
> >  bool intel_pipe_has_type(struct intel_crtc *crtc, enum intel_output_type type)
> >  {
> > -	struct drm_device *dev = crtc->base.dev;
> > -	struct intel_encoder *encoder;
> > -
> > -	for_each_encoder_on_crtc(dev, &crtc->base, encoder)
> > -		if (encoder->type == type)
> > -			return true;
> > -
> > -	return false;
> > +	return crtc->config->output_types & (1 << type);
> 
> This could become inline and then all those
> 
> 	if (intel_pipe_has_type(crtc, VGA) ||
> 	    intel_pipe_has_type(crtc, LVDS) ||
> 	    intel_pipe_has_type(crtc, HDMI))
> 
> whatnots will just disapear into a single test.

Sounds sane. Or we could just kill the entire function and just open
code it. Not sure hiding it in a function buys us anything at this
point.

> 
> I won't say how long those loops have been upsetting me without doing
> anything about them.
> 
> > @@ -12529,6 +12503,19 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
> >  			       &pipe_config->pipe_src_w,
> >  			       &pipe_config->pipe_src_h);
> >  
> > +	for_each_connector_in_state(state, connector, connector_state, i) {
> > +		if (connector_state->crtc != crtc)
> > +			continue;
> > +
> > +		encoder = to_intel_encoder(connector_state->best_encoder);
> > +
> > +		/*
> > +		 * 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;
> 
> pipe_config->output_types |= BIT(encoder->type);
> 
> Not my personal favourite in obfuscation, but one available to us if we
> desire.

I've been a bit wary about BIT() on account of the UL in there. But I
suppose it might not make much of a difference in the end.

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

* Re: [PATCH 03/12] drm/i915: Add output_types bitmask into the crtc state
  2016-06-08 10:41 ` [PATCH 03/12] drm/i915: Add output_types bitmask into the crtc state ville.syrjala
  2016-06-08 13:25   ` Chris Wilson
@ 2016-06-13 14:25   ` Daniel Vetter
  2016-06-13 16:24     ` Ville Syrjälä
  1 sibling, 1 reply; 28+ messages in thread
From: Daniel Vetter @ 2016-06-13 14:25 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

On Wed, Jun 08, 2016 at 01:41:38PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Rather than looping through encoders to see which encoder types
> are being driven by the pipe, add an output_types bitmask into
> the crtc state and populate it prior to compute_config and during
> state readout.
> 
> v2: Determine output_types before .compute_config() hooks are called
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Not sure whether this will mesh with LSPCON perfectly, but the atomic way
is to pass connector_state and crtc_state to all encoder functions. That's
at least how the atomic helpers in the core work, and it imo works well.
i915 isn't there yet, but we want that anyway.

Once you have the connector_state, the connector is just a pointer away.
And the functions which care can look up the connector type.

I'd like to go that direction since it would align i915 more with core
atomic. And that misalignment is imo a big reason for why our transition
is so super painful.
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_display.c | 51 +++++++++++++++---------------------
>  drivers/gpu/drm/i915/intel_drv.h     |  5 ++++
>  2 files changed, 26 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 12ff79594bc1..eabace447b1c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -535,14 +535,7 @@ needs_modeset(struct drm_crtc_state *state)
>   */
>  bool intel_pipe_has_type(struct intel_crtc *crtc, enum intel_output_type type)
>  {
> -	struct drm_device *dev = crtc->base.dev;
> -	struct intel_encoder *encoder;
> -
> -	for_each_encoder_on_crtc(dev, &crtc->base, encoder)
> -		if (encoder->type == type)
> -			return true;
> -
> -	return false;
> +	return crtc->config->output_types & (1 << type);
>  }
>  
>  /**
> @@ -552,28 +545,9 @@ bool intel_pipe_has_type(struct intel_crtc *crtc, enum intel_output_type type)
>   * encoder->crtc.
>   */
>  static bool intel_pipe_will_have_type(const struct intel_crtc_state *crtc_state,
> -				      int type)
> +				      enum intel_output_type type)
>  {
> -	struct drm_atomic_state *state = crtc_state->base.state;
> -	struct drm_connector *connector;
> -	struct drm_connector_state *connector_state;
> -	struct intel_encoder *encoder;
> -	int i, num_connectors = 0;
> -
> -	for_each_connector_in_state(state, connector, connector_state, i) {
> -		if (connector_state->crtc != crtc_state->base.crtc)
> -			continue;
> -
> -		num_connectors++;
> -
> -		encoder = to_intel_encoder(connector_state->best_encoder);
> -		if (encoder->type == type)
> -			return true;
> -	}
> -
> -	WARN_ON(num_connectors == 0);
> -
> -	return false;
> +	return crtc_state->output_types & (1 << type);
>  }
>  
>  /*
> @@ -12529,6 +12503,19 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
>  			       &pipe_config->pipe_src_w,
>  			       &pipe_config->pipe_src_h);
>  
> +	for_each_connector_in_state(state, connector, connector_state, i) {
> +		if (connector_state->crtc != crtc)
> +			continue;
> +
> +		encoder = to_intel_encoder(connector_state->best_encoder);
> +
> +		/*
> +		 * 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;
> +	}
> +
>  encoder_retry:
>  	/* Ensure the port clock defaults are reset when retrying. */
>  	pipe_config->port_clock = 0;
> @@ -12826,6 +12813,7 @@ intel_pipe_config_compare(struct drm_device *dev,
>  		PIPE_CONF_CHECK_M_N_ALT(dp_m_n, dp_m2_n2);
>  
>  	PIPE_CONF_CHECK_I(has_dsi_encoder);
> +	PIPE_CONF_CHECK_X(output_types);
>  
>  	PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_hdisplay);
>  	PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_htotal);
> @@ -13093,8 +13081,10 @@ verify_crtc_state(struct drm_crtc *crtc,
>  				"Encoder connected to wrong pipe %c\n",
>  				pipe_name(pipe));
>  
> -		if (active)
> +		if (active) {
> +			pipe_config->output_types |= 1 << encoder->type;
>  			encoder->get_config(encoder, pipe_config);
> +		}
>  	}
>  
>  	if (!new_crtc_state->active)
> @@ -15985,6 +15975,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>  		if (encoder->get_hw_state(encoder, &pipe)) {
>  			crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
>  			encoder->base.crtc = &crtc->base;
> +			crtc->config->output_types |= 1 << encoder->type;
>  			encoder->get_config(encoder, crtc->config);
>  		} else {
>  			encoder->base.crtc = NULL;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index ebe7b3427e2e..3dae05e6d95b 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -512,6 +512,11 @@ struct intel_crtc_state {
>  	/* DSI has special cases */
>  	bool has_dsi_encoder;
>  
> +	/* Bitmask of encoder types (enum intel_output_type)
> +	 * driven by the pipe.
> +	 */
> +	unsigned int output_types;
> +
>  	/* Whether we should send NULL infoframes. Required for audio. */
>  	bool has_hdmi_sink;
>  
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 03/12] drm/i915: Add output_types bitmask into the crtc state
  2016-06-13 14:25   ` Daniel Vetter
@ 2016-06-13 16:24     ` Ville Syrjälä
  0 siblings, 0 replies; 28+ messages in thread
From: Ville Syrjälä @ 2016-06-13 16:24 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

On Mon, Jun 13, 2016 at 04:25:55PM +0200, Daniel Vetter wrote:
> On Wed, Jun 08, 2016 at 01:41:38PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Rather than looping through encoders to see which encoder types
> > are being driven by the pipe, add an output_types bitmask into
> > the crtc state and populate it prior to compute_config and during
> > state readout.
> > 
> > v2: Determine output_types before .compute_config() hooks are called
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Not sure whether this will mesh with LSPCON perfectly, but the atomic way
> is to pass connector_state and crtc_state to all encoder functions. That's
> at least how the atomic helpers in the core work, and it imo works well.
> i915 isn't there yet, but we want that anyway.
> 
> Once you have the connector_state, the connector is just a pointer away.
> And the functions which care can look up the connector type.
> 
> I'd like to go that direction since it would align i915 more with core
> atomic. And that misalignment is imo a big reason for why our transition
> is so super painful.

We want the bitmask anyway since we want to figure things out about
the output type in places where we have no connector state (eg. DPLL
code , or when when we'd have to deal with multiple connector states
(cloning).

> -Daniel
> 
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 51 +++++++++++++++---------------------
> >  drivers/gpu/drm/i915/intel_drv.h     |  5 ++++
> >  2 files changed, 26 insertions(+), 30 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 12ff79594bc1..eabace447b1c 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -535,14 +535,7 @@ needs_modeset(struct drm_crtc_state *state)
> >   */
> >  bool intel_pipe_has_type(struct intel_crtc *crtc, enum intel_output_type type)
> >  {
> > -	struct drm_device *dev = crtc->base.dev;
> > -	struct intel_encoder *encoder;
> > -
> > -	for_each_encoder_on_crtc(dev, &crtc->base, encoder)
> > -		if (encoder->type == type)
> > -			return true;
> > -
> > -	return false;
> > +	return crtc->config->output_types & (1 << type);
> >  }
> >  
> >  /**
> > @@ -552,28 +545,9 @@ bool intel_pipe_has_type(struct intel_crtc *crtc, enum intel_output_type type)
> >   * encoder->crtc.
> >   */
> >  static bool intel_pipe_will_have_type(const struct intel_crtc_state *crtc_state,
> > -				      int type)
> > +				      enum intel_output_type type)
> >  {
> > -	struct drm_atomic_state *state = crtc_state->base.state;
> > -	struct drm_connector *connector;
> > -	struct drm_connector_state *connector_state;
> > -	struct intel_encoder *encoder;
> > -	int i, num_connectors = 0;
> > -
> > -	for_each_connector_in_state(state, connector, connector_state, i) {
> > -		if (connector_state->crtc != crtc_state->base.crtc)
> > -			continue;
> > -
> > -		num_connectors++;
> > -
> > -		encoder = to_intel_encoder(connector_state->best_encoder);
> > -		if (encoder->type == type)
> > -			return true;
> > -	}
> > -
> > -	WARN_ON(num_connectors == 0);
> > -
> > -	return false;
> > +	return crtc_state->output_types & (1 << type);
> >  }
> >  
> >  /*
> > @@ -12529,6 +12503,19 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
> >  			       &pipe_config->pipe_src_w,
> >  			       &pipe_config->pipe_src_h);
> >  
> > +	for_each_connector_in_state(state, connector, connector_state, i) {
> > +		if (connector_state->crtc != crtc)
> > +			continue;
> > +
> > +		encoder = to_intel_encoder(connector_state->best_encoder);
> > +
> > +		/*
> > +		 * 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;
> > +	}
> > +
> >  encoder_retry:
> >  	/* Ensure the port clock defaults are reset when retrying. */
> >  	pipe_config->port_clock = 0;
> > @@ -12826,6 +12813,7 @@ intel_pipe_config_compare(struct drm_device *dev,
> >  		PIPE_CONF_CHECK_M_N_ALT(dp_m_n, dp_m2_n2);
> >  
> >  	PIPE_CONF_CHECK_I(has_dsi_encoder);
> > +	PIPE_CONF_CHECK_X(output_types);
> >  
> >  	PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_hdisplay);
> >  	PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_htotal);
> > @@ -13093,8 +13081,10 @@ verify_crtc_state(struct drm_crtc *crtc,
> >  				"Encoder connected to wrong pipe %c\n",
> >  				pipe_name(pipe));
> >  
> > -		if (active)
> > +		if (active) {
> > +			pipe_config->output_types |= 1 << encoder->type;
> >  			encoder->get_config(encoder, pipe_config);
> > +		}
> >  	}
> >  
> >  	if (!new_crtc_state->active)
> > @@ -15985,6 +15975,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
> >  		if (encoder->get_hw_state(encoder, &pipe)) {
> >  			crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
> >  			encoder->base.crtc = &crtc->base;
> > +			crtc->config->output_types |= 1 << encoder->type;
> >  			encoder->get_config(encoder, crtc->config);
> >  		} else {
> >  			encoder->base.crtc = NULL;
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index ebe7b3427e2e..3dae05e6d95b 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -512,6 +512,11 @@ struct intel_crtc_state {
> >  	/* DSI has special cases */
> >  	bool has_dsi_encoder;
> >  
> > +	/* Bitmask of encoder types (enum intel_output_type)
> > +	 * driven by the pipe.
> > +	 */
> > +	unsigned int output_types;
> > +
> >  	/* Whether we should send NULL infoframes. Required for audio. */
> >  	bool has_hdmi_sink;
> >  
> > -- 
> > 2.7.4
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

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

* Re: [PATCH 01/12] drm/i915: Don't mark eDP encoders as MST capable
  2016-06-08 10:41 ` [PATCH 01/12] drm/i915: Don't mark eDP encoders as MST capable ville.syrjala
@ 2016-06-16 12:27   ` Sharma, Shashank
  2016-06-16 20:35   ` Dave Airlie
  1 sibling, 0 replies; 28+ messages in thread
From: Sharma, Shashank @ 2016-06-16 12:27 UTC (permalink / raw)
  To: ville.syrjala, intel-gfx; +Cc: Dave Airlie

Reviewed-by: Shashank Sharma

Regards
Shashank
On 6/8/2016 4:11 PM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> If we've determined that the encoder is eDP, we shouldn't try to use MST
> on it. Or at least the code doesn't seem to expect that since there are
> some type==DP checks in the MST code.
>
> Cc: Dave Airlie <airlied@redhat.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/intel_dp.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index f97cd5305e4c..0ab4f319f88f 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5528,7 +5528,7 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
>   		goto fail;
>
>   	/* init MST on ports that can support it */
> -	if (HAS_DP_MST(dev) &&
> +	if (HAS_DP_MST(dev) && !is_edp(intel_dp) &&
>   	    (port == PORT_B || port == PORT_C || port == PORT_D))
>   		intel_dp_mst_encoder_init(intel_dig_port,
>   					  intel_connector->base.base.id);
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 02/12] drm/i915: Remove encoder type checks from MST suspend/resume
  2016-06-08 10:41 ` [PATCH 02/12] drm/i915: Remove encoder type checks from MST suspend/resume ville.syrjala
@ 2016-06-16 12:41   ` Sharma, Shashank
  2016-06-16 13:40     ` Ville Syrjälä
  0 siblings, 1 reply; 28+ messages in thread
From: Sharma, Shashank @ 2016-06-16 12:41 UTC (permalink / raw)
  To: ville.syrjala, intel-gfx; +Cc: Dave Airlie

Regards
Shashank
On 6/8/2016 4:11 PM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Now that eDP encoders won't have can_mst==true, we can throw out
> the encoder type checks from the MST suspend/resume paths.
>
> Cc: Dave Airlie <airlied@redhat.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/intel_dp.c | 28 ++++++++++------------------
>   1 file changed, 10 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 0ab4f319f88f..29fb0d907f7b 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5659,15 +5659,12 @@ void intel_dp_mst_suspend(struct drm_device *dev)
>   	/* disable MST */
>   	for (i = 0; i < I915_MAX_PORTS; i++) {
>   		struct intel_digital_port *intel_dig_port = dev_priv->hotplug.irq_port[i];
> -		if (!intel_dig_port)
> +
> +		if (!intel_dig_port || !intel_dig_port->dp.can_mst)
>   			continue;
>
> -		if (intel_dig_port->base.type == INTEL_OUTPUT_DISPLAYPORT) {
> -			if (!intel_dig_port->dp.can_mst)
> -				continue;
> -			if (intel_dig_port->dp.is_mst)
> -				drm_dp_mst_topology_mgr_suspend(&intel_dig_port->dp.mst_mgr);
> -		}
> +		if (intel_dig_port->dp.is_mst)
> +			drm_dp_mst_topology_mgr_suspend(&intel_dig_port->dp.mst_mgr);
>   	}
>   }
>
> @@ -5678,18 +5675,13 @@ void intel_dp_mst_resume(struct drm_device *dev)
>
>   	for (i = 0; i < I915_MAX_PORTS; i++) {
>   		struct intel_digital_port *intel_dig_port = dev_priv->hotplug.irq_port[i];
> -		if (!intel_dig_port)
> -			continue;
> -		if (intel_dig_port->base.type == INTEL_OUTPUT_DISPLAYPORT) {
> -			int ret;
> +		int ret;
>
> -			if (!intel_dig_port->dp.can_mst)
> -				continue;
> +		if (!intel_dig_port || !intel_dig_port->dp.can_mst)
> +			continue;
>
> -			ret = drm_dp_mst_topology_mgr_resume(&intel_dig_port->dp.mst_mgr);
> -			if (ret != 0) {
> -				intel_dp_check_mst_status(&intel_dig_port->dp);
> -			}
> -		}
> +		ret = drm_dp_mst_topology_mgr_resume(&intel_dig_port->dp.mst_mgr);
> +		if (ret)
> +			intel_dp_check_mst_status(&intel_dig_port->dp);
Now when we are modifying this code, can we please check the return 
value of this function 'intel_dp_check_mst_status' which returns an int ?
- Shashank
>   	}
>   }
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 02/12] drm/i915: Remove encoder type checks from MST suspend/resume
  2016-06-16 12:41   ` Sharma, Shashank
@ 2016-06-16 13:40     ` Ville Syrjälä
  2016-06-16 17:48       ` Sharma, Shashank
  0 siblings, 1 reply; 28+ messages in thread
From: Ville Syrjälä @ 2016-06-16 13:40 UTC (permalink / raw)
  To: Sharma, Shashank; +Cc: Dave Airlie, intel-gfx

On Thu, Jun 16, 2016 at 06:11:50PM +0530, Sharma, Shashank wrote:
> Regards
> Shashank
> On 6/8/2016 4:11 PM, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Now that eDP encoders won't have can_mst==true, we can throw out
> > the encoder type checks from the MST suspend/resume paths.
> >
> > Cc: Dave Airlie <airlied@redhat.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >   drivers/gpu/drm/i915/intel_dp.c | 28 ++++++++++------------------
> >   1 file changed, 10 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 0ab4f319f88f..29fb0d907f7b 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -5659,15 +5659,12 @@ void intel_dp_mst_suspend(struct drm_device *dev)
> >   	/* disable MST */
> >   	for (i = 0; i < I915_MAX_PORTS; i++) {
> >   		struct intel_digital_port *intel_dig_port = dev_priv->hotplug.irq_port[i];
> > -		if (!intel_dig_port)
> > +
> > +		if (!intel_dig_port || !intel_dig_port->dp.can_mst)
> >   			continue;
> >
> > -		if (intel_dig_port->base.type == INTEL_OUTPUT_DISPLAYPORT) {
> > -			if (!intel_dig_port->dp.can_mst)
> > -				continue;
> > -			if (intel_dig_port->dp.is_mst)
> > -				drm_dp_mst_topology_mgr_suspend(&intel_dig_port->dp.mst_mgr);
> > -		}
> > +		if (intel_dig_port->dp.is_mst)
> > +			drm_dp_mst_topology_mgr_suspend(&intel_dig_port->dp.mst_mgr);
> >   	}
> >   }
> >
> > @@ -5678,18 +5675,13 @@ void intel_dp_mst_resume(struct drm_device *dev)
> >
> >   	for (i = 0; i < I915_MAX_PORTS; i++) {
> >   		struct intel_digital_port *intel_dig_port = dev_priv->hotplug.irq_port[i];
> > -		if (!intel_dig_port)
> > -			continue;
> > -		if (intel_dig_port->base.type == INTEL_OUTPUT_DISPLAYPORT) {
> > -			int ret;
> > +		int ret;
> >
> > -			if (!intel_dig_port->dp.can_mst)
> > -				continue;
> > +		if (!intel_dig_port || !intel_dig_port->dp.can_mst)
> > +			continue;
> >
> > -			ret = drm_dp_mst_topology_mgr_resume(&intel_dig_port->dp.mst_mgr);
> > -			if (ret != 0) {
> > -				intel_dp_check_mst_status(&intel_dig_port->dp);
> > -			}
> > -		}
> > +		ret = drm_dp_mst_topology_mgr_resume(&intel_dig_port->dp.mst_mgr);
> > +		if (ret)
> > +			intel_dp_check_mst_status(&intel_dig_port->dp);
> Now when we are modifying this code, can we please check the return 
> value of this function 'intel_dp_check_mst_status' which returns an int ?

I'll leave that to someone that cares about MST, and undestands what the
code does.

> - Shashank
> >   	}
> >   }
> >

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

* Re: [PATCH 02/12] drm/i915: Remove encoder type checks from MST suspend/resume
  2016-06-16 13:40     ` Ville Syrjälä
@ 2016-06-16 17:48       ` Sharma, Shashank
  0 siblings, 0 replies; 28+ messages in thread
From: Sharma, Shashank @ 2016-06-16 17:48 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: Dave Airlie, intel-gfx

Regards
Shashank

On 6/16/2016 7:10 PM, Ville Syrjälä wrote:
> On Thu, Jun 16, 2016 at 06:11:50PM +0530, Sharma, Shashank wrote:
>> Regards
>> Shashank
>> On 6/8/2016 4:11 PM, ville.syrjala@linux.intel.com wrote:
>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>
>>> Now that eDP encoders won't have can_mst==true, we can throw out
>>> the encoder type checks from the MST suspend/resume paths.
>>>
>>> Cc: Dave Airlie <airlied@redhat.com>
>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> ---
>>>    drivers/gpu/drm/i915/intel_dp.c | 28 ++++++++++------------------
>>>    1 file changed, 10 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>>> index 0ab4f319f88f..29fb0d907f7b 100644
>>> --- a/drivers/gpu/drm/i915/intel_dp.c
>>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>>> @@ -5659,15 +5659,12 @@ void intel_dp_mst_suspend(struct drm_device *dev)
>>>    	/* disable MST */
>>>    	for (i = 0; i < I915_MAX_PORTS; i++) {
>>>    		struct intel_digital_port *intel_dig_port = dev_priv->hotplug.irq_port[i];
>>> -		if (!intel_dig_port)
>>> +
>>> +		if (!intel_dig_port || !intel_dig_port->dp.can_mst)
>>>    			continue;
>>>
>>> -		if (intel_dig_port->base.type == INTEL_OUTPUT_DISPLAYPORT) {
>>> -			if (!intel_dig_port->dp.can_mst)
>>> -				continue;
>>> -			if (intel_dig_port->dp.is_mst)
>>> -				drm_dp_mst_topology_mgr_suspend(&intel_dig_port->dp.mst_mgr);
>>> -		}
>>> +		if (intel_dig_port->dp.is_mst)
>>> +			drm_dp_mst_topology_mgr_suspend(&intel_dig_port->dp.mst_mgr);
>>>    	}
>>>    }
>>>
>>> @@ -5678,18 +5675,13 @@ void intel_dp_mst_resume(struct drm_device *dev)
>>>
>>>    	for (i = 0; i < I915_MAX_PORTS; i++) {
>>>    		struct intel_digital_port *intel_dig_port = dev_priv->hotplug.irq_port[i];
>>> -		if (!intel_dig_port)
>>> -			continue;
>>> -		if (intel_dig_port->base.type == INTEL_OUTPUT_DISPLAYPORT) {
>>> -			int ret;
>>> +		int ret;
>>>
>>> -			if (!intel_dig_port->dp.can_mst)
>>> -				continue;
>>> +		if (!intel_dig_port || !intel_dig_port->dp.can_mst)
>>> +			continue;
>>>
>>> -			ret = drm_dp_mst_topology_mgr_resume(&intel_dig_port->dp.mst_mgr);
>>> -			if (ret != 0) {
>>> -				intel_dp_check_mst_status(&intel_dig_port->dp);
>>> -			}
>>> -		}
>>> +		ret = drm_dp_mst_topology_mgr_resume(&intel_dig_port->dp.mst_mgr);
>>> +		if (ret)
>>> +			intel_dp_check_mst_status(&intel_dig_port->dp);
>> Now when we are modifying this code, can we please check the return
>> value of this function 'intel_dp_check_mst_status' which returns an int ?
>
> I'll leave that to someone that cares about MST, and undestands what the
> code does.
>
Yes, I guess. And also, I think he would be the more appropriate person 
to review the mess we are making around MST code :).
>> - Shashank
>>>    	}
>>>    }
>>>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 01/12] drm/i915: Don't mark eDP encoders as MST capable
  2016-06-08 10:41 ` [PATCH 01/12] drm/i915: Don't mark eDP encoders as MST capable ville.syrjala
  2016-06-16 12:27   ` Sharma, Shashank
@ 2016-06-16 20:35   ` Dave Airlie
  2016-06-17 14:25     ` Ville Syrjälä
  1 sibling, 1 reply; 28+ messages in thread
From: Dave Airlie @ 2016-06-16 20:35 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: Dave Airlie, intel-gfx

On 8 June 2016 at 20:41,  <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> If we've determined that the encoder is eDP, we shouldn't try to use MST
> on it. Or at least the code doesn't seem to expect that since there are
> some type==DP checks in the MST code.
>
> Cc: Dave Airlie <airlied@redhat.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

How sure are we that MST eDP panels won't show up in the future?

Like the code could be buggy now, but breaking it completely is different.

I've no insight into future eDP panel development.

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

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

* Re: [PATCH 01/12] drm/i915: Don't mark eDP encoders as MST capable
  2016-06-16 20:35   ` Dave Airlie
@ 2016-06-17 14:25     ` Ville Syrjälä
  0 siblings, 0 replies; 28+ messages in thread
From: Ville Syrjälä @ 2016-06-17 14:25 UTC (permalink / raw)
  To: Dave Airlie; +Cc: Dave Airlie, intel-gfx

On Fri, Jun 17, 2016 at 06:35:33AM +1000, Dave Airlie wrote:
> On 8 June 2016 at 20:41,  <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > If we've determined that the encoder is eDP, we shouldn't try to use MST
> > on it. Or at least the code doesn't seem to expect that since there are
> > some type==DP checks in the MST code.
> >
> > Cc: Dave Airlie <airlied@redhat.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> How sure are we that MST eDP panels won't show up in the future?

No clue really. The only reference to MST in the eDP spec is the
explanation of the acronym.

I did spot that there's this thing called multi-SST mode, where you feed
the panel with several SST links. But that's sort of the opposite of
MST.

> 
> Like the code could be buggy now, but breaking it completely is different.

Should be easy for someone familiar with the MST code to fix if eDP+MST
ever makes an appearance; Just revert this patch, and deal with any
fallout.

> 
> I've no insight into future eDP panel development.
> 
> Dave.

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

* Re: [PATCH 11/12] drm/i915: Check for invalid cloning earlier during modeset
  2016-06-08 13:27     ` Ville Syrjälä
@ 2016-06-20 13:54       ` Maarten Lankhorst
  0 siblings, 0 replies; 28+ messages in thread
From: Maarten Lankhorst @ 2016-06-20 13:54 UTC (permalink / raw)
  To: Ville Syrjälä, Chris Wilson, intel-gfx

Op 08-06-16 om 15:27 schreef Ville Syrjälä:
> On Wed, Jun 08, 2016 at 02:15:25PM +0100, Chris Wilson wrote:
>> On Wed, Jun 08, 2016 at 01:41:46PM +0300, ville.syrjala@linux.intel.com wrote:
>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>
>>> Move the encoder cloning check to happen earlier in the modeset. The
>>> main benefit will be that the debug output from a failed modeset will
>>> be less confusing as output_types can not indicate an invalid
>>> configuration during the later computation stages.
>>>
>>> For instance, what happened to me was kms_setmode was attempting one
>>> of its invalid cloning checks during which it asked for DP+VGA cloning
>>> on HSW. In this case the DP .compute_config() was executed after
>>> the FDI .compute_config() leaving the DP link clock (1.62 in this case)
>>> in port_clock, and then later the FDI BW computation tried to use that
>>> as the FDI link clock (which should always be 2.7). 1.62 x 2 wasn't
>>> enough for the mode it was trying to use, and so it ended up rejecting
>>> the modeset, not because of an invalid cloning configuration, but
>>> because of supposedly running out of FDI bandwidth. Took me a while
>>> to figure out what had actually happened.
>> Did it reject the 1.62 link clock when you simply removed the
>> check_encoder_cloning()? Just checking... :)
> Nope. The rejection only happened due to exceeding the max fdi lane
> count. I think I had a warn in there somewhere when I originally
> submitted the fdi==port_clock patch, but that apparently didn't
> survive into the version that eventually got merged.
>
For patch 1-11:

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

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

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

end of thread, other threads:[~2016-06-20 13:54 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-08 10:41 [PATCH 00/12] drm/i915: Eliminate DDI encoder->type frobbery ville.syrjala
2016-06-08 10:41 ` [PATCH 01/12] drm/i915: Don't mark eDP encoders as MST capable ville.syrjala
2016-06-16 12:27   ` Sharma, Shashank
2016-06-16 20:35   ` Dave Airlie
2016-06-17 14:25     ` Ville Syrjälä
2016-06-08 10:41 ` [PATCH 02/12] drm/i915: Remove encoder type checks from MST suspend/resume ville.syrjala
2016-06-16 12:41   ` Sharma, Shashank
2016-06-16 13:40     ` Ville Syrjälä
2016-06-16 17:48       ` Sharma, Shashank
2016-06-08 10:41 ` [PATCH 03/12] drm/i915: Add output_types bitmask into the crtc state ville.syrjala
2016-06-08 13:25   ` Chris Wilson
2016-06-08 13:33     ` Ville Syrjälä
2016-06-13 14:25   ` Daniel Vetter
2016-06-13 16:24     ` Ville Syrjälä
2016-06-08 10:41 ` [PATCH 04/12] drm/i915: Unify intel_pipe_has_type() and intel_pipe_will_have_type() ville.syrjala
2016-06-08 10:41 ` [PATCH 05/12] drm/i915: Replace manual lvds and sdvo/hdmi counting with intel_crtc_has_type() ville.syrjala
2016-06-08 10:41 ` [PATCH 06/12] drm/i915: Kill has_dp_encoder from pipe_config ville.syrjala
2016-06-08 10:41 ` [PATCH 07/12] drm/i915: Replace some open coded intel_crtc_has_dp_encoder()s ville.syrjala
2016-06-08 10:41 ` [PATCH 08/12] drm/i915: s/INTEL_OUTPUT_DISPLAYPORT/INTEL_OUTPUT_DP/ ville.syrjala
2016-06-08 13:05   ` Kahola, Mika
2016-06-08 10:41 ` [PATCH 09/12] drm/i915: Kill has_dsi_encoder ville.syrjala
2016-06-08 10:41 ` [PATCH 10/12] drm/i915: Simplify hdmi_12bpc_possible() ville.syrjala
2016-06-08 10:41 ` [PATCH 11/12] drm/i915: Check for invalid cloning earlier during modeset ville.syrjala
2016-06-08 13:15   ` Chris Wilson
2016-06-08 13:27     ` Ville Syrjälä
2016-06-20 13:54       ` Maarten Lankhorst
2016-06-08 10:41 ` [PATCH 12/12] drm/i915: Stop frobbing with DDI encoder->type ville.syrjala
2016-06-08 11:13 ` ✓ Ro.CI.BAT: success for drm/i915: Eliminate DDI encoder->type frobbery Patchwork

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