[Intel-gfx] [PATCH 5/5] drm/i915/tgl: Select master transcoder in DP MST

Lucas De Marchi lucas.demarchi at intel.com
Wed Oct 30 01:24:48 UTC 2019


From: José Roberto de Souza <jose.souza at intel.com>

On TGL the blending of all the streams have moved from DDI to
transcoder, so now every transcoder working over the same MST port must
send its stream to a master transcoder and master will send to DDI
respecting the time slots.

So here it is picking the lowest pipe/transcoder as it will be
enabled first and disabled last.
BSpec: 50493
BSpec: 49190

v2: Missed set mst_master_trans to TRANSCODER_INVALID when computing HSW
    encoder config. HSW CRT hw state readout calls
    hsw_crt_get_config()->intel_ddi_get_config() that will set
    mst_master_trans to TRANSCODER_INVALID causing the mismatch when
    verifying CRTC state after a modeset. (José)

v3: Add WARN_ON() requested by Jani.
    Add FIXME. From Jani: double check
    PIPE_CONF_CHECK_I(mst_master_trans) - it's now checking for all
    platforms and MST and non-MST alike.  Perhaps in general I'd like
    the approach of only doing the readout when it's relevant, and only
    checking the value when it's relevant.

v4 (Lucas): Revamp previous implementation:
    - Make sure we only compute master once; the compute function will
      be called for each crtc, so we don't need to recompute during check.
      Now the check simply compares the master from before and after.
    - Don't loop on all crtcs and connectors, we can just loop over the
      connectors and find the lowest crtc
    - Move the setting of INVALID_TRANSCODER around to make it similar
      to the places where we got it added for master transport sync.
    - Move all added code to use intel types
    - Use REG_FIELD_GET()

Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
Cc: Manasi Navare <manasi.d.navare at intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
Signed-off-by: José Roberto de Souza <jose.souza at intel.com>
Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
---
 drivers/gpu/drm/i915/display/intel_ddi.c      |  14 +-
 drivers/gpu/drm/i915/display/intel_display.c  |  16 ++
 .../drm/i915/display/intel_display_types.h    |   3 +
 drivers/gpu/drm/i915/display/intel_dp_mst.c   | 143 ++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_dp_mst.h   |   2 +
 5 files changed, 176 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 41b9b9a6772a..2c0da46ac8e2 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -1906,8 +1906,13 @@ intel_ddi_transcoder_func_reg_val_get(const struct intel_crtc_state *crtc_state)
 		temp |= TRANS_DDI_MODE_SELECT_DP_MST;
 		temp |= DDI_PORT_WIDTH(crtc_state->lane_count);
 
-		if (INTEL_GEN(dev_priv) >= 12)
-			temp |= TRANS_DDI_MST_TRANSPORT_SELECT(crtc_state->cpu_transcoder);
+		if (INTEL_GEN(dev_priv) >= 12) {
+			enum transcoder master =
+				crtc_state->mst_master_transcoder;
+
+			WARN_ON(master == INVALID_TRANSCODER);
+			temp |= TRANS_DDI_MST_TRANSPORT_SELECT(master);
+		}
 	} else {
 		temp |= TRANS_DDI_MODE_SELECT_DP_SST;
 		temp |= DDI_PORT_WIDTH(crtc_state->lane_count);
@@ -4304,6 +4309,11 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
 		pipe_config->output_types |= BIT(INTEL_OUTPUT_DP_MST);
 		pipe_config->lane_count =
 			((temp & DDI_PORT_WIDTH_MASK) >> DDI_PORT_WIDTH_SHIFT) + 1;
+
+		if (INTEL_GEN(dev_priv) >= 12)
+			pipe_config->mst_master_transcoder =
+				REG_FIELD_GET(TRANS_DDI_MST_TRANSPORT_SELECT_MASK, temp);
+
 		intel_dp_get_m_n(intel_crtc, pipe_config);
 		break;
 	default:
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index e56a75c07043..4dd1ec1300f1 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -46,6 +46,7 @@
 #include "display/intel_crt.h"
 #include "display/intel_ddi.h"
 #include "display/intel_dp.h"
+#include "display/intel_dp_mst.h"
 #include "display/intel_dsi.h"
 #include "display/intel_dvo.h"
 #include "display/intel_gmbus.h"
@@ -8896,6 +8897,7 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
 	pipe_config->cpu_transcoder = (enum transcoder) crtc->pipe;
 	pipe_config->shared_dpll = NULL;
 	pipe_config->master_transcoder = INVALID_TRANSCODER;
+	pipe_config->mst_master_transcoder = INVALID_TRANSCODER;
 
 	ret = false;
 
@@ -10087,6 +10089,7 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
 	pipe_config->cpu_transcoder = (enum transcoder) crtc->pipe;
 	pipe_config->shared_dpll = NULL;
 	pipe_config->master_transcoder = INVALID_TRANSCODER;
+	pipe_config->mst_master_transcoder = INVALID_TRANSCODER;
 
 	ret = false;
 	tmp = I915_READ(PIPECONF(crtc->pipe));
@@ -10602,6 +10605,7 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 	intel_crtc_init_scalers(crtc, pipe_config);
 
 	pipe_config->master_transcoder = INVALID_TRANSCODER;
+	pipe_config->mst_master_transcoder = INVALID_TRANSCODER;
 
 	power_domain = POWER_DOMAIN_PIPE(crtc->pipe);
 	wf = intel_display_power_get_if_enabled(dev_priv, power_domain);
@@ -12478,6 +12482,11 @@ static void intel_dump_pipe_config(const struct intel_crtc_state *pipe_config,
 			      pipe_config->csc_mode, pipe_config->gamma_mode,
 			      pipe_config->gamma_enable, pipe_config->csc_enable);
 
+	if (INTEL_GEN(dev_priv) >= 12 &&
+	    intel_crtc_has_type(pipe_config, INTEL_OUTPUT_DP_MST))
+		DRM_DEBUG_KMS("MST master transcoder: %s\n",
+			      transcoder_name(pipe_config->mst_master_transcoder));
+
 dump_planes:
 	if (!state)
 		return;
@@ -12694,6 +12703,8 @@ intel_modeset_pipe_config(struct intel_crtc_state *pipe_config)
 		return ret;
 	}
 
+	pipe_config->mst_master_transcoder = INVALID_TRANSCODER;
+
 	/* Pass our mode to the connectors and the CRTC to give them a chance to
 	 * adjust it according to limitations or connector properties, and also
 	 * a chance to reject the mode entirely.
@@ -13215,6 +13226,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
 
 	PIPE_CONF_CHECK_I(sync_mode_slaves_mask);
 	PIPE_CONF_CHECK_I(master_transcoder);
+	PIPE_CONF_CHECK_I(mst_master_transcoder);
 
 #undef PIPE_CONF_CHECK_X
 #undef PIPE_CONF_CHECK_I
@@ -14003,6 +14015,10 @@ static int intel_atomic_check(struct drm_device *dev,
 	if (ret)
 		goto fail;
 
+	ret = intel_dp_mst_atomic_add_affected_crtcs(state);
+	if (ret)
+		return ret;
+
 	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
 					    new_crtc_state, i) {
 		if (!needs_modeset(new_crtc_state))
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index a550abb48b3c..40753d1a29e5 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1002,6 +1002,9 @@ struct intel_crtc_state {
 	/* Forward Error correction State */
 	bool fec_enable;
 
+	/* MST master transcoder for all streams, only used on TGL+ */
+	enum transcoder mst_master_transcoder;
+
 	/* Pointer to master transcoder in case of tiled displays */
 	enum transcoder master_transcoder;
 
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index ad54618f6142..1c040de3a396 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -87,6 +87,50 @@ static int intel_dp_mst_compute_link_config(struct intel_encoder *encoder,
 	return 0;
 }
 
+/*
+ * Iterate over all the CRTCs and return the transcoder of the lowest CRTC that
+ * shares the same MST connector.
+ */
+void
+intel_dp_mst_compute_master_transcoder(struct intel_connector *mst_connector,
+				       struct intel_crtc_state *crtc_state)
+{
+	struct drm_i915_private *i915 = to_i915(crtc_state->base.crtc->dev);
+	struct intel_atomic_state *state =
+		to_intel_atomic_state(crtc_state->base.state);
+	enum transcoder mst_master_transcoder = crtc_state->cpu_transcoder;
+	struct intel_crtc_state *other_crtc_state;
+	struct intel_digital_connector_state *conn_state;
+	struct intel_connector *conn;
+	int i;
+
+	if (INTEL_GEN(i915) < 12)
+		return;
+
+	for_each_new_intel_connector_in_state(state, conn, conn_state, i) {
+		struct intel_crtc *crtc = to_intel_crtc(conn_state->base.crtc);
+
+		if (conn == mst_connector ||
+		    conn->mst_port != mst_connector->mst_port ||
+		    !crtc)
+			continue;
+
+		other_crtc_state = intel_atomic_get_new_crtc_state(state, crtc);
+		if (!other_crtc_state->base.active)
+			continue;
+
+		if (other_crtc_state->cpu_transcoder < mst_master_transcoder)
+			mst_master_transcoder = other_crtc_state->cpu_transcoder;
+	}
+
+	crtc_state->mst_master_transcoder = mst_master_transcoder;
+
+	DRM_DEBUG_KMS("[CRTC:%d:%s] MST master transcoder: %s\n",
+		      crtc_state->base.crtc->base.id,
+		      crtc_state->base.crtc->name,
+		      transcoder_name(mst_master_transcoder));
+}
+
 static int intel_dp_mst_compute_config(struct intel_encoder *encoder,
 				       struct intel_crtc_state *pipe_config,
 				       struct drm_connector_state *conn_state)
@@ -154,6 +198,42 @@ static int intel_dp_mst_compute_config(struct intel_encoder *encoder,
 
 	intel_ddi_compute_min_voltage_level(dev_priv, pipe_config);
 
+	intel_dp_mst_compute_master_transcoder(connector, pipe_config);
+
+	return 0;
+}
+
+static int
+intel_dp_mst_master_transcoder_check(struct intel_connector *connector,
+				     struct intel_atomic_state *state)
+{
+	struct drm_i915_private *i915 = to_i915(connector->base.dev);
+	struct intel_digital_connector_state *new_conn_state =
+		intel_atomic_get_new_connector_state(state, connector);
+	struct intel_digital_connector_state *old_conn_state =
+		intel_atomic_get_old_connector_state(state, connector);
+	struct intel_crtc_state *new_crtc_state, *old_crtc_state;
+	struct intel_crtc *crtc;
+
+	if (INTEL_GEN(i915) < 12)
+		return 0;
+
+	/*
+	 * A modeset will be triggered when checking other affected crtcs if
+	 * this was connected to a master transcoder
+	 */
+	if (!new_conn_state->base.crtc)
+		return 0;
+
+	crtc = to_intel_crtc(new_conn_state->base.crtc);
+	new_crtc_state = intel_atomic_get_new_crtc_state(state, crtc);
+	old_crtc_state = intel_atomic_get_old_crtc_state(state, crtc);
+
+	if (!old_conn_state ||
+	    new_crtc_state->mst_master_transcoder !=
+	    old_crtc_state->mst_master_transcoder)
+		new_crtc_state->base.mode_changed = true;
+
 	return 0;
 }
 
@@ -176,6 +256,11 @@ intel_dp_mst_atomic_check(struct drm_connector *connector,
 	if (ret)
 		return ret;
 
+	ret = intel_dp_mst_master_transcoder_check(intel_connector,
+						   to_intel_atomic_state(state));
+	if (ret)
+		return ret;
+
 	if (!old_conn_state->crtc)
 		return 0;
 
@@ -706,3 +791,61 @@ intel_dp_mst_encoder_cleanup(struct intel_digital_port *intel_dig_port)
 	drm_dp_mst_topology_mgr_destroy(&intel_dp->mst_mgr);
 	/* encoders will get killed by normal cleanup */
 }
+
+/**
+ * intel_dp_mst_atomic_add_affected_crtcs - Add all CRTCs that share the MST
+ * port with the CRTCs in the current atomic state.
+ * @state: state to add CRTCs
+ *
+ * We need to make the CRTCs trigger a call to atomic_check() to every connector
+ * attached to the CRTC in case a new master transcoder is needed.
+ */
+int intel_dp_mst_atomic_add_affected_crtcs(struct intel_atomic_state *state)
+{
+	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
+	struct intel_digital_connector_state *intel_conn_state;
+	struct drm_device *dev = state->base.dev;
+	struct intel_connector *intel_conn;
+	int i;
+
+	if (INTEL_GEN(dev_priv) < 12)
+		return 0;
+
+	for_each_new_intel_connector_in_state(state, intel_conn, intel_conn_state, i) {
+		struct drm_connector_list_iter conn_list_iter;
+		struct drm_connector *conn_iter;
+
+		if (!intel_conn->mst_port)
+			continue;
+
+		drm_connector_list_iter_begin(dev, &conn_list_iter);
+		drm_for_each_connector_iter(conn_iter, &conn_list_iter) {
+			struct drm_connector_state *conn_iter_state;
+			struct intel_connector *intel_conn_iter;
+			struct drm_crtc_state *crtc_state;
+
+			intel_conn_iter = to_intel_connector(conn_iter);
+
+			if (intel_conn_iter->mst_port != intel_conn->mst_port)
+				continue;
+
+			conn_iter_state = drm_atomic_get_connector_state(&state->base, conn_iter);
+			if (IS_ERR(conn_iter_state)) {
+				drm_connector_list_iter_end(&conn_list_iter);
+				return PTR_ERR(conn_iter_state);
+			}
+			if (!conn_iter_state->crtc)
+				continue;
+
+			crtc_state = drm_atomic_get_crtc_state(&state->base,
+							       conn_iter_state->crtc);
+			if (IS_ERR(crtc_state)) {
+				drm_connector_list_iter_end(&conn_list_iter);
+				return PTR_ERR(crtc_state);
+			}
+		}
+		drm_connector_list_iter_end(&conn_list_iter);
+	}
+
+	return 0;
+}
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.h b/drivers/gpu/drm/i915/display/intel_dp_mst.h
index f660ad80db04..9e654f77a2d2 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.h
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.h
@@ -7,9 +7,11 @@
 #define __INTEL_DP_MST_H__
 
 struct intel_digital_port;
+struct intel_atomic_state;
 
 int intel_dp_mst_encoder_init(struct intel_digital_port *intel_dig_port, int conn_id);
 void intel_dp_mst_encoder_cleanup(struct intel_digital_port *intel_dig_port);
 int intel_dp_mst_encoder_active_links(struct intel_digital_port *intel_dig_port);
+int intel_dp_mst_atomic_add_affected_crtcs(struct intel_atomic_state *state);
 
 #endif /* __INTEL_DP_MST_H__ */
-- 
2.23.0



More information about the Intel-gfx mailing list