[PATCH 0/8] drm/i915: Bigjoiner stuff

[PATCH 0/8] drm/i915: Bigjoiner stuff

[Ville Syrjala]

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

Here's some prep work I cooked up while thinking how to handle
the bigjoiner stuff. I coverted the .crtc_disable() side of things
to follow the new approach of iterating the joined pipes at a lower
level. Looks pretty reasonable to me in the end. The enable side
is somewhat more messy so I left that as an excercise for the reader.

TODO: need CI to actually check that things work

Ville Syrjälä (8):
  drm/i915: Rename the crtc/crtc_states in the top level DDI hooks/etc
  drm/i915: Introduce intel_crtc_joined_pipe_mask()
  drm/i915: Extract intel_ddi_post_disable_hdmi_or_sst()
  drm/i915: Utilize intel_crtc_joined_pipe_mask() more
  drm/i915: Precompute disable_pipes bitmask in
    intel_commit_modeset_disables()
  drm/i915: Disable planes more atomically during modesets
  drm/i915: Simplify intel_old_crtc_state_disables() calling convention
  drm/i915: Handle joined pipes inside hsw_crtc_disable()

 drivers/gpu/drm/i915/display/intel_ddi.c     | 357 ++++++++++---------
 drivers/gpu/drm/i915/display/intel_display.c | 192 +++++-----
 drivers/gpu/drm/i915/display/intel_display.h |   1 +
 drivers/gpu/drm/i915/display/intel_dp_mst.c  |  91 +++--
 4 files changed, 336 insertions(+), 305 deletions(-)

-- 
2.43.0

[PATCH 1/8] drm/i915: Rename the crtc/crtc_states in the top level DDI hooks/etc

[Ville Syrjala]

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

In preparation for doing a more sensible pipe vs. transcoder
handling for bigjoiner let's rename the crtc/crtc_state in the
top level crtc_enable/disable and the DDI encoder hooks to
include "master" in the name. This way they won't collide with
the per-pipe stuff.

Note that at this point this is (at least partially) telling
lies as we still run through some of these for slave pipes as
well. But I wanted to get the huge rename out of the way so
it won't clutter the functional patches so much.

TODO: or perhaps use some other names for the per-pipe stuff instead?

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_ddi.c     | 326 +++++++++----------
 drivers/gpu/drm/i915/display/intel_display.c | 100 +++---
 drivers/gpu/drm/i915/display/intel_dp_mst.c  |  91 +++---
 3 files changed, 258 insertions(+), 259 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index c587a8efeafc..6287629f9e77 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -2490,21 +2490,21 @@ static void mtl_port_buf_ctl_io_selection(struct intel_encoder *encoder)
 
 static void mtl_ddi_pre_enable_dp(struct intel_atomic_state *state,
 				  struct intel_encoder *encoder,
-				  const struct intel_crtc_state *crtc_state,
+				  const struct intel_crtc_state *master_crtc_state,
 				  const struct drm_connector_state *conn_state)
 {
 	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
-	bool is_mst = intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST);
+	bool is_mst = intel_crtc_has_type(master_crtc_state, INTEL_OUTPUT_DP_MST);
 
 	intel_dp_set_link_params(intel_dp,
-				 crtc_state->port_clock,
-				 crtc_state->lane_count);
+				 master_crtc_state->port_clock,
+				 master_crtc_state->lane_count);
 
 	/*
 	 * We only configure what the register value will be here.  Actual
 	 * enabling happens during link training farther down.
 	 */
-	intel_ddi_init_dp_buf_reg(encoder, crtc_state);
+	intel_ddi_init_dp_buf_reg(encoder, master_crtc_state);
 
 	/*
 	 * 1. Enable Power Wells
@@ -2522,48 +2522,48 @@ static void mtl_ddi_pre_enable_dp(struct intel_atomic_state *state,
 	intel_pps_on(intel_dp);
 
 	/* 5. Enable the port PLL */
-	intel_ddi_enable_clock(encoder, crtc_state);
+	intel_ddi_enable_clock(encoder, master_crtc_state);
 
 	/*
 	 * 6.a Configure Transcoder Clock Select to direct the Port clock to the
 	 * Transcoder.
 	 */
-	intel_ddi_enable_transcoder_clock(encoder, crtc_state);
+	intel_ddi_enable_transcoder_clock(encoder, master_crtc_state);
 
 	/*
 	 * 6.b If DP v2.0/128b mode - Configure TRANS_DP2_CTL register settings.
 	 */
-	intel_ddi_config_transcoder_dp2(encoder, crtc_state);
+	intel_ddi_config_transcoder_dp2(encoder, master_crtc_state);
 
 	/*
 	 * 6.c Configure TRANS_DDI_FUNC_CTL DDI Select, DDI Mode Select & MST
 	 * Transport Select
 	 */
-	intel_ddi_config_transcoder_func(encoder, crtc_state);
+	intel_ddi_config_transcoder_func(encoder, master_crtc_state);
 
 	/*
 	 * 6.e Program CoG/MSO configuration bits in DSS_CTL1 if selected.
 	 */
-	intel_ddi_mso_configure(crtc_state);
+	intel_ddi_mso_configure(master_crtc_state);
 
 	if (!is_mst)
 		intel_dp_set_power(intel_dp, DP_SET_POWER_D0);
 
-	intel_dp_configure_protocol_converter(intel_dp, crtc_state);
+	intel_dp_configure_protocol_converter(intel_dp, master_crtc_state);
 	if (!is_mst)
 		intel_dp_sink_enable_decompression(state,
 						   to_intel_connector(conn_state->connector),
-						   crtc_state);
+						   master_crtc_state);
 
 	/*
 	 * DDI FEC: "anticipates enabling FEC encoding sets the FEC_READY bit
 	 * in the FEC_CONFIGURATION register to 1 before initiating link
 	 * training
 	 */
-	intel_dp_sink_set_fec_ready(intel_dp, crtc_state, true);
+	intel_dp_sink_set_fec_ready(intel_dp, master_crtc_state, true);
 
 	intel_dp_check_frl_training(intel_dp);
-	intel_dp_pcon_dsc_configure(intel_dp, crtc_state);
+	intel_dp_pcon_dsc_configure(intel_dp, master_crtc_state);
 
 	/*
 	 * 6. The rest of the below are substeps under the bspec's "Enable and
@@ -2584,38 +2584,38 @@ static void mtl_ddi_pre_enable_dp(struct intel_atomic_state *state,
 	 *     Pattern, wait for 5 idle patterns (DP_TP_STATUS Min_Idles_Sent)
 	 *     (timeout after 800 us)
 	 */
-	intel_dp_start_link_train(intel_dp, crtc_state);
+	intel_dp_start_link_train(intel_dp, master_crtc_state);
 
 	/* 6.n Set DP_TP_CTL link training to Normal */
-	if (!is_trans_port_sync_mode(crtc_state))
-		intel_dp_stop_link_train(intel_dp, crtc_state);
+	if (!is_trans_port_sync_mode(master_crtc_state))
+		intel_dp_stop_link_train(intel_dp, master_crtc_state);
 
 	/* 6.o Configure and enable FEC if needed */
-	intel_ddi_enable_fec(encoder, crtc_state);
+	intel_ddi_enable_fec(encoder, master_crtc_state);
 
 	if (!is_mst)
-		intel_dsc_dp_pps_write(encoder, crtc_state);
+		intel_dsc_dp_pps_write(encoder, master_crtc_state);
 }
 
 static void tgl_ddi_pre_enable_dp(struct intel_atomic_state *state,
 				  struct intel_encoder *encoder,
-				  const struct intel_crtc_state *crtc_state,
+				  const struct intel_crtc_state *master_crtc_state,
 				  const struct drm_connector_state *conn_state)
 {
 	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
-	bool is_mst = intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST);
+	bool is_mst = intel_crtc_has_type(master_crtc_state, INTEL_OUTPUT_DP_MST);
 
 	intel_dp_set_link_params(intel_dp,
-				 crtc_state->port_clock,
-				 crtc_state->lane_count);
+				 master_crtc_state->port_clock,
+				 master_crtc_state->lane_count);
 
 	/*
 	 * We only configure what the register value will be here.  Actual
 	 * enabling happens during link training farther down.
 	 */
-	intel_ddi_init_dp_buf_reg(encoder, crtc_state);
+	intel_ddi_init_dp_buf_reg(encoder, master_crtc_state);
 
 	/*
 	 * 1. Enable Power Wells
@@ -2642,7 +2642,7 @@ static void tgl_ddi_pre_enable_dp(struct intel_atomic_state *state,
 	 * hsw_crtc_enable()->intel_enable_shared_dpll().  We need only
 	 * configure the PLL to port mapping here.
 	 */
-	intel_ddi_enable_clock(encoder, crtc_state);
+	intel_ddi_enable_clock(encoder, master_crtc_state);
 
 	/* 5. If IO power is controlled through PWR_WELL_CTL, Enable IO Power */
 	if (!intel_tc_port_in_tbt_alt_mode(dig_port)) {
@@ -2652,7 +2652,7 @@ static void tgl_ddi_pre_enable_dp(struct intel_atomic_state *state,
 	}
 
 	/* 6. Program DP_MODE */
-	icl_program_mg_dp_mode(dig_port, crtc_state);
+	icl_program_mg_dp_mode(dig_port, master_crtc_state);
 
 	/*
 	 * 7. The rest of the below are substeps under the bspec's "Enable and
@@ -2668,16 +2668,16 @@ static void tgl_ddi_pre_enable_dp(struct intel_atomic_state *state,
 	 * 7.a Configure Transcoder Clock Select to direct the Port clock to the
 	 * Transcoder.
 	 */
-	intel_ddi_enable_transcoder_clock(encoder, crtc_state);
+	intel_ddi_enable_transcoder_clock(encoder, master_crtc_state);
 
 	if (HAS_DP20(dev_priv))
-		intel_ddi_config_transcoder_dp2(encoder, crtc_state);
+		intel_ddi_config_transcoder_dp2(encoder, master_crtc_state);
 
 	/*
 	 * 7.b Configure TRANS_DDI_FUNC_CTL DDI Select, DDI Mode Select & MST
 	 * Transport Select
 	 */
-	intel_ddi_config_transcoder_func(encoder, crtc_state);
+	intel_ddi_config_transcoder_func(encoder, master_crtc_state);
 
 	/*
 	 * 7.c Configure & enable DP_TP_CTL with link training pattern 1
@@ -2688,36 +2688,36 @@ static void tgl_ddi_pre_enable_dp(struct intel_atomic_state *state,
 	 */
 
 	/* 7.e Configure voltage swing and related IO settings */
-	encoder->set_signal_levels(encoder, crtc_state);
+	encoder->set_signal_levels(encoder, master_crtc_state);
 
 	/*
 	 * 7.f Combo PHY: Configure PORT_CL_DW10 Static Power Down to power up
 	 * the used lanes of the DDI.
 	 */
-	intel_ddi_power_up_lanes(encoder, crtc_state);
+	intel_ddi_power_up_lanes(encoder, master_crtc_state);
 
 	/*
 	 * 7.g Program CoG/MSO configuration bits in DSS_CTL1 if selected.
 	 */
-	intel_ddi_mso_configure(crtc_state);
+	intel_ddi_mso_configure(master_crtc_state);
 
 	if (!is_mst)
 		intel_dp_set_power(intel_dp, DP_SET_POWER_D0);
 
-	intel_dp_configure_protocol_converter(intel_dp, crtc_state);
+	intel_dp_configure_protocol_converter(intel_dp, master_crtc_state);
 	if (!is_mst)
 		intel_dp_sink_enable_decompression(state,
 						   to_intel_connector(conn_state->connector),
-						   crtc_state);
+						   master_crtc_state);
 	/*
 	 * DDI FEC: "anticipates enabling FEC encoding sets the FEC_READY bit
 	 * in the FEC_CONFIGURATION register to 1 before initiating link
 	 * training
 	 */
-	intel_dp_sink_set_fec_ready(intel_dp, crtc_state, true);
+	intel_dp_sink_set_fec_ready(intel_dp, master_crtc_state, true);
 
 	intel_dp_check_frl_training(intel_dp);
-	intel_dp_pcon_dsc_configure(intel_dp, crtc_state);
+	intel_dp_pcon_dsc_configure(intel_dp, master_crtc_state);
 
 	/*
 	 * 7.i Follow DisplayPort specification training sequence (see notes for
@@ -2726,29 +2726,29 @@ static void tgl_ddi_pre_enable_dp(struct intel_atomic_state *state,
 	 *     Pattern, wait for 5 idle patterns (DP_TP_STATUS Min_Idles_Sent)
 	 *     (timeout after 800 us)
 	 */
-	intel_dp_start_link_train(intel_dp, crtc_state);
+	intel_dp_start_link_train(intel_dp, master_crtc_state);
 
 	/* 7.k Set DP_TP_CTL link training to Normal */
-	if (!is_trans_port_sync_mode(crtc_state))
-		intel_dp_stop_link_train(intel_dp, crtc_state);
+	if (!is_trans_port_sync_mode(master_crtc_state))
+		intel_dp_stop_link_train(intel_dp, master_crtc_state);
 
 	/* 7.l Configure and enable FEC if needed */
-	intel_ddi_enable_fec(encoder, crtc_state);
+	intel_ddi_enable_fec(encoder, master_crtc_state);
 
 	if (!is_mst)
-		intel_dsc_dp_pps_write(encoder, crtc_state);
+		intel_dsc_dp_pps_write(encoder, master_crtc_state);
 }
 
 static void hsw_ddi_pre_enable_dp(struct intel_atomic_state *state,
 				  struct intel_encoder *encoder,
-				  const struct intel_crtc_state *crtc_state,
+				  const struct intel_crtc_state *master_crtc_state,
 				  const struct drm_connector_state *conn_state)
 {
 	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	enum port port = encoder->port;
 	struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
-	bool is_mst = intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST);
+	bool is_mst = intel_crtc_has_type(master_crtc_state, INTEL_OUTPUT_DP_MST);
 
 	if (DISPLAY_VER(dev_priv) < 11)
 		drm_WARN_ON(&dev_priv->drm,
@@ -2757,18 +2757,18 @@ static void hsw_ddi_pre_enable_dp(struct intel_atomic_state *state,
 		drm_WARN_ON(&dev_priv->drm, is_mst && port == PORT_A);
 
 	intel_dp_set_link_params(intel_dp,
-				 crtc_state->port_clock,
-				 crtc_state->lane_count);
+				 master_crtc_state->port_clock,
+				 master_crtc_state->lane_count);
 
 	/*
 	 * We only configure what the register value will be here.  Actual
 	 * enabling happens during link training farther down.
 	 */
-	intel_ddi_init_dp_buf_reg(encoder, crtc_state);
+	intel_ddi_init_dp_buf_reg(encoder, master_crtc_state);
 
 	intel_pps_on(intel_dp);
 
-	intel_ddi_enable_clock(encoder, crtc_state);
+	intel_ddi_enable_clock(encoder, master_crtc_state);
 
 	if (!intel_tc_port_in_tbt_alt_mode(dig_port)) {
 		drm_WARN_ON(&dev_priv->drm, dig_port->ddi_io_wakeref);
@@ -2776,39 +2776,39 @@ static void hsw_ddi_pre_enable_dp(struct intel_atomic_state *state,
 								   dig_port->ddi_io_power_domain);
 	}
 
-	icl_program_mg_dp_mode(dig_port, crtc_state);
+	icl_program_mg_dp_mode(dig_port, master_crtc_state);
 
 	if (has_buf_trans_select(dev_priv))
-		hsw_prepare_dp_ddi_buffers(encoder, crtc_state);
+		hsw_prepare_dp_ddi_buffers(encoder, master_crtc_state);
 
-	encoder->set_signal_levels(encoder, crtc_state);
+	encoder->set_signal_levels(encoder, master_crtc_state);
 
-	intel_ddi_power_up_lanes(encoder, crtc_state);
+	intel_ddi_power_up_lanes(encoder, master_crtc_state);
 
 	if (!is_mst)
 		intel_dp_set_power(intel_dp, DP_SET_POWER_D0);
-	intel_dp_configure_protocol_converter(intel_dp, crtc_state);
+	intel_dp_configure_protocol_converter(intel_dp, master_crtc_state);
 	if (!is_mst)
 		intel_dp_sink_enable_decompression(state,
 						   to_intel_connector(conn_state->connector),
-						   crtc_state);
-	intel_dp_sink_set_fec_ready(intel_dp, crtc_state, true);
-	intel_dp_start_link_train(intel_dp, crtc_state);
+						   master_crtc_state);
+	intel_dp_sink_set_fec_ready(intel_dp, master_crtc_state, true);
+	intel_dp_start_link_train(intel_dp, master_crtc_state);
 	if ((port != PORT_A || DISPLAY_VER(dev_priv) >= 9) &&
-	    !is_trans_port_sync_mode(crtc_state))
-		intel_dp_stop_link_train(intel_dp, crtc_state);
+	    !is_trans_port_sync_mode(master_crtc_state))
+		intel_dp_stop_link_train(intel_dp, master_crtc_state);
 
-	intel_ddi_enable_fec(encoder, crtc_state);
+	intel_ddi_enable_fec(encoder, master_crtc_state);
 
 	if (!is_mst) {
-		intel_ddi_enable_transcoder_clock(encoder, crtc_state);
-		intel_dsc_dp_pps_write(encoder, crtc_state);
+		intel_ddi_enable_transcoder_clock(encoder, master_crtc_state);
+		intel_dsc_dp_pps_write(encoder, master_crtc_state);
 	}
 }
 
 static void intel_ddi_pre_enable_dp(struct intel_atomic_state *state,
 				    struct intel_encoder *encoder,
-				    const struct intel_crtc_state *crtc_state,
+				    const struct intel_crtc_state *master_crtc_state,
 				    const struct drm_connector_state *conn_state)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
@@ -2816,29 +2816,29 @@ static void intel_ddi_pre_enable_dp(struct intel_atomic_state *state,
 
 	if (HAS_DP20(dev_priv)) {
 		intel_dp_128b132b_sdp_crc16(enc_to_intel_dp(encoder),
-					    crtc_state);
-		if (crtc_state->has_panel_replay)
+					    master_crtc_state);
+		if (master_crtc_state->has_panel_replay)
 			drm_dp_dpcd_writeb(&intel_dp->aux, PANEL_REPLAY_CONFIG,
 					   DP_PANEL_REPLAY_ENABLE);
 	}
 
 	if (DISPLAY_VER(dev_priv) >= 14)
-		mtl_ddi_pre_enable_dp(state, encoder, crtc_state, conn_state);
+		mtl_ddi_pre_enable_dp(state, encoder, master_crtc_state, conn_state);
 	else if (DISPLAY_VER(dev_priv) >= 12)
-		tgl_ddi_pre_enable_dp(state, encoder, crtc_state, conn_state);
+		tgl_ddi_pre_enable_dp(state, encoder, master_crtc_state, conn_state);
 	else
-		hsw_ddi_pre_enable_dp(state, encoder, crtc_state, conn_state);
+		hsw_ddi_pre_enable_dp(state, encoder, master_crtc_state, conn_state);
 
 	/* MST will call a setting of MSA after an allocating of Virtual Channel
 	 * from MST encoder pre_enable callback.
 	 */
-	if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST))
-		intel_ddi_set_dp_msa(crtc_state, conn_state);
+	if (!intel_crtc_has_type(master_crtc_state, INTEL_OUTPUT_DP_MST))
+		intel_ddi_set_dp_msa(master_crtc_state, conn_state);
 }
 
 static void intel_ddi_pre_enable_hdmi(struct intel_atomic_state *state,
 				      struct intel_encoder *encoder,
-				      const struct intel_crtc_state *crtc_state,
+				      const struct intel_crtc_state *master_crtc_state,
 				      const struct drm_connector_state *conn_state)
 {
 	struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
@@ -2846,27 +2846,27 @@ static void intel_ddi_pre_enable_hdmi(struct intel_atomic_state *state,
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 
 	intel_dp_dual_mode_set_tmds_output(intel_hdmi, true);
-	intel_ddi_enable_clock(encoder, crtc_state);
+	intel_ddi_enable_clock(encoder, master_crtc_state);
 
 	drm_WARN_ON(&dev_priv->drm, dig_port->ddi_io_wakeref);
 	dig_port->ddi_io_wakeref = intel_display_power_get(dev_priv,
 							   dig_port->ddi_io_power_domain);
 
-	icl_program_mg_dp_mode(dig_port, crtc_state);
+	icl_program_mg_dp_mode(dig_port, master_crtc_state);
 
-	intel_ddi_enable_transcoder_clock(encoder, crtc_state);
+	intel_ddi_enable_transcoder_clock(encoder, master_crtc_state);
 
 	dig_port->set_infoframes(encoder,
-				 crtc_state->has_infoframe,
-				 crtc_state, conn_state);
+				 master_crtc_state->has_infoframe,
+				 master_crtc_state, conn_state);
 }
 
 static void intel_ddi_pre_enable(struct intel_atomic_state *state,
 				 struct intel_encoder *encoder,
-				 const struct intel_crtc_state *crtc_state,
+				 const struct intel_crtc_state *master_crtc_state,
 				 const struct drm_connector_state *conn_state)
 {
-	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+	struct intel_crtc *crtc = to_intel_crtc(master_crtc_state->uapi.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	enum pipe pipe = crtc->pipe;
 
@@ -2876,32 +2876,32 @@ static void intel_ddi_pre_enable(struct intel_atomic_state *state,
 	 * - encoder will be the main encoder (ie. mst->primary)
 	 * - the main connector associated with this port
 	 *   won't be active or linked to a crtc
-	 * - crtc_state will be the state of the first stream to
+	 * - master_crtc_state will be the state of the first stream to
 	 *   be activated on this port, and it may not be the same
 	 *   stream that will be deactivated last, but each stream
 	 *   should have a state that is identical when it comes to
 	 *   the DP link parameteres
 	 */
 
-	drm_WARN_ON(&dev_priv->drm, crtc_state->has_pch_encoder);
+	drm_WARN_ON(&dev_priv->drm, master_crtc_state->has_pch_encoder);
 
 	intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
 
-	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI)) {
-		intel_ddi_pre_enable_hdmi(state, encoder, crtc_state,
+	if (intel_crtc_has_type(master_crtc_state, INTEL_OUTPUT_HDMI)) {
+		intel_ddi_pre_enable_hdmi(state, encoder, master_crtc_state,
 					  conn_state);
 	} else {
 		struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
 
-		intel_ddi_pre_enable_dp(state, encoder, crtc_state,
+		intel_ddi_pre_enable_dp(state, encoder, master_crtc_state,
 					conn_state);
 
 		/* FIXME precompute everything properly */
 		/* FIXME how do we turn infoframes off again? */
 		if (dig_port->lspcon.active && intel_dp_has_hdmi_sink(&dig_port->dp))
 			dig_port->set_infoframes(encoder,
-						 crtc_state->has_infoframe,
-						 crtc_state, conn_state);
+						 master_crtc_state->has_infoframe,
+						 master_crtc_state, conn_state);
 	}
 }
 
@@ -3000,19 +3000,19 @@ static void intel_disable_ddi_buf(struct intel_encoder *encoder,
 
 static void intel_ddi_post_disable_dp(struct intel_atomic_state *state,
 				      struct intel_encoder *encoder,
-				      const struct intel_crtc_state *old_crtc_state,
+				      const struct intel_crtc_state *old_master_crtc_state,
 				      const struct drm_connector_state *old_conn_state)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
 	struct intel_dp *intel_dp = &dig_port->dp;
 	intel_wakeref_t wakeref;
-	bool is_mst = intel_crtc_has_type(old_crtc_state,
+	bool is_mst = intel_crtc_has_type(old_master_crtc_state,
 					  INTEL_OUTPUT_DP_MST);
 
 	if (!is_mst)
 		intel_dp_set_infoframes(encoder, false,
-					old_crtc_state, old_conn_state);
+					old_master_crtc_state, old_conn_state);
 
 	/*
 	 * Power down sink before disabling the port, otherwise we end
@@ -3022,7 +3022,7 @@ static void intel_ddi_post_disable_dp(struct intel_atomic_state *state,
 
 	if (DISPLAY_VER(dev_priv) >= 12) {
 		if (is_mst) {
-			enum transcoder cpu_transcoder = old_crtc_state->cpu_transcoder;
+			enum transcoder cpu_transcoder = old_master_crtc_state->cpu_transcoder;
 
 			intel_de_rmw(dev_priv, TRANS_DDI_FUNC_CTL(cpu_transcoder),
 				     TGL_TRANS_DDI_PORT_MASK | TRANS_DDI_MODE_SELECT_MASK,
@@ -3030,12 +3030,12 @@ static void intel_ddi_post_disable_dp(struct intel_atomic_state *state,
 		}
 	} else {
 		if (!is_mst)
-			intel_ddi_disable_transcoder_clock(old_crtc_state);
+			intel_ddi_disable_transcoder_clock(old_master_crtc_state);
 	}
 
-	intel_disable_ddi_buf(encoder, old_crtc_state);
+	intel_disable_ddi_buf(encoder, old_master_crtc_state);
 
-	intel_dp_sink_set_fec_ready(intel_dp, old_crtc_state, false);
+	intel_dp_sink_set_fec_ready(intel_dp, old_master_crtc_state, false);
 
 	/*
 	 * From TGL spec: "If single stream or multi-stream master transcoder:
@@ -3043,7 +3043,7 @@ static void intel_ddi_post_disable_dp(struct intel_atomic_state *state,
 	 * transcoder"
 	 */
 	if (DISPLAY_VER(dev_priv) >= 12)
-		intel_ddi_disable_transcoder_clock(old_crtc_state);
+		intel_ddi_disable_transcoder_clock(old_master_crtc_state);
 
 	intel_pps_vdd_on(intel_dp);
 	intel_pps_off(intel_dp);
@@ -3065,7 +3065,7 @@ static void intel_ddi_post_disable_dp(struct intel_atomic_state *state,
 
 static void intel_ddi_post_disable_hdmi(struct intel_atomic_state *state,
 					struct intel_encoder *encoder,
-					const struct intel_crtc_state *old_crtc_state,
+					const struct intel_crtc_state *old_master_crtc_state,
 					const struct drm_connector_state *old_conn_state)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
@@ -3074,15 +3074,15 @@ static void intel_ddi_post_disable_hdmi(struct intel_atomic_state *state,
 	intel_wakeref_t wakeref;
 
 	dig_port->set_infoframes(encoder, false,
-				 old_crtc_state, old_conn_state);
+				 old_master_crtc_state, old_conn_state);
 
 	if (DISPLAY_VER(dev_priv) < 12)
-		intel_ddi_disable_transcoder_clock(old_crtc_state);
+		intel_ddi_disable_transcoder_clock(old_master_crtc_state);
 
-	intel_disable_ddi_buf(encoder, old_crtc_state);
+	intel_disable_ddi_buf(encoder, old_master_crtc_state);
 
 	if (DISPLAY_VER(dev_priv) >= 12)
-		intel_ddi_disable_transcoder_clock(old_crtc_state);
+		intel_ddi_disable_transcoder_clock(old_master_crtc_state);
 
 	wakeref = fetch_and_zero(&dig_port->ddi_io_wakeref);
 	if (wakeref)
@@ -3097,29 +3097,29 @@ static void intel_ddi_post_disable_hdmi(struct intel_atomic_state *state,
 
 static void intel_ddi_post_disable(struct intel_atomic_state *state,
 				   struct intel_encoder *encoder,
-				   const struct intel_crtc_state *old_crtc_state,
+				   const struct intel_crtc_state *old_master_crtc_state,
 				   const struct drm_connector_state *old_conn_state)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	struct intel_crtc *slave_crtc;
 
-	if (!intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_DP_MST)) {
-		intel_crtc_vblank_off(old_crtc_state);
+	if (!intel_crtc_has_type(old_master_crtc_state, INTEL_OUTPUT_DP_MST)) {
+		intel_crtc_vblank_off(old_master_crtc_state);
 
-		intel_disable_transcoder(old_crtc_state);
+		intel_disable_transcoder(old_master_crtc_state);
 
-		intel_ddi_disable_transcoder_func(old_crtc_state);
+		intel_ddi_disable_transcoder_func(old_master_crtc_state);
 
-		intel_dsc_disable(old_crtc_state);
+		intel_dsc_disable(old_master_crtc_state);
 
 		if (DISPLAY_VER(dev_priv) >= 9)
-			skl_scaler_disable(old_crtc_state);
+			skl_scaler_disable(old_master_crtc_state);
 		else
-			ilk_pfit_disable(old_crtc_state);
+			ilk_pfit_disable(old_master_crtc_state);
 	}
 
 	for_each_intel_crtc_in_pipe_mask(&dev_priv->drm, slave_crtc,
-					 intel_crtc_bigjoiner_slave_pipes(old_crtc_state)) {
+					 intel_crtc_bigjoiner_slave_pipes(old_master_crtc_state)) {
 		const struct intel_crtc_state *old_slave_crtc_state =
 			intel_atomic_get_old_crtc_state(state, slave_crtc);
 
@@ -3142,17 +3142,17 @@ static void intel_ddi_post_disable(struct intel_atomic_state *state,
 	 *   the DP link parameteres
 	 */
 
-	if (intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_HDMI))
-		intel_ddi_post_disable_hdmi(state, encoder, old_crtc_state,
+	if (intel_crtc_has_type(old_master_crtc_state, INTEL_OUTPUT_HDMI))
+		intel_ddi_post_disable_hdmi(state, encoder, old_master_crtc_state,
 					    old_conn_state);
 	else
-		intel_ddi_post_disable_dp(state, encoder, old_crtc_state,
+		intel_ddi_post_disable_dp(state, encoder, old_master_crtc_state,
 					  old_conn_state);
 }
 
 static void intel_ddi_post_pll_disable(struct intel_atomic_state *state,
 				       struct intel_encoder *encoder,
-				       const struct intel_crtc_state *old_crtc_state,
+				       const struct intel_crtc_state *old_master_crtc_state,
 				       const struct drm_connector_state *old_conn_state)
 {
 	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
@@ -3160,7 +3160,7 @@ static void intel_ddi_post_pll_disable(struct intel_atomic_state *state,
 	enum phy phy = intel_port_to_phy(i915, encoder->port);
 	bool is_tc_port = intel_phy_is_tc(i915, phy);
 
-	main_link_aux_power_domain_put(dig_port, old_crtc_state);
+	main_link_aux_power_domain_put(dig_port, old_master_crtc_state);
 
 	if (is_tc_port)
 		intel_tc_port_put_link(dig_port);
@@ -3205,7 +3205,7 @@ static void trans_port_sync_stop_link_train(struct intel_atomic_state *state,
 
 static void intel_enable_ddi_dp(struct intel_atomic_state *state,
 				struct intel_encoder *encoder,
-				const struct intel_crtc_state *crtc_state,
+				const struct intel_crtc_state *master_crtc_state,
 				const struct drm_connector_state *conn_state)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
@@ -3214,15 +3214,15 @@ static void intel_enable_ddi_dp(struct intel_atomic_state *state,
 	enum port port = encoder->port;
 
 	if (port == PORT_A && DISPLAY_VER(dev_priv) < 9)
-		intel_dp_stop_link_train(intel_dp, crtc_state);
+		intel_dp_stop_link_train(intel_dp, master_crtc_state);
 
 	drm_connector_update_privacy_screen(conn_state);
-	intel_edp_backlight_on(crtc_state, conn_state);
+	intel_edp_backlight_on(master_crtc_state, conn_state);
 
 	if (!dig_port->lspcon.active || intel_dp_has_hdmi_sink(&dig_port->dp))
-		intel_dp_set_infoframes(encoder, true, crtc_state, conn_state);
+		intel_dp_set_infoframes(encoder, true, master_crtc_state, conn_state);
 
-	trans_port_sync_stop_link_train(state, encoder, crtc_state);
+	trans_port_sync_stop_link_train(state, encoder, master_crtc_state);
 }
 
 /* FIXME bad home for this function */
@@ -3256,7 +3256,7 @@ gen9_chicken_trans_reg_by_port(struct drm_i915_private *dev_priv,
 
 static void intel_enable_ddi_hdmi(struct intel_atomic_state *state,
 				  struct intel_encoder *encoder,
-				  const struct intel_crtc_state *crtc_state,
+				  const struct intel_crtc_state *master_crtc_state,
 				  const struct drm_connector_state *conn_state)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
@@ -3267,20 +3267,20 @@ static void intel_enable_ddi_hdmi(struct intel_atomic_state *state,
 	u32 buf_ctl;
 
 	if (!intel_hdmi_handle_sink_scrambling(encoder, connector,
-					       crtc_state->hdmi_high_tmds_clock_ratio,
-					       crtc_state->hdmi_scrambling))
+					       master_crtc_state->hdmi_high_tmds_clock_ratio,
+					       master_crtc_state->hdmi_scrambling))
 		drm_dbg_kms(&dev_priv->drm,
 			    "[CONNECTOR:%d:%s] Failed to configure sink scrambling/TMDS bit clock ratio\n",
 			    connector->base.id, connector->name);
 
 	if (has_buf_trans_select(dev_priv))
-		hsw_prepare_hdmi_ddi_buffers(encoder, crtc_state);
+		hsw_prepare_hdmi_ddi_buffers(encoder, master_crtc_state);
 
 	/* e. Enable D2D Link for C10/C20 Phy */
 	if (DISPLAY_VER(dev_priv) >= 14)
 		mtl_ddi_enable_d2d(encoder);
 
-	encoder->set_signal_levels(encoder, crtc_state);
+	encoder->set_signal_levels(encoder, master_crtc_state);
 
 	/* Display WA #1143: skl,kbl,cfl */
 	if (DISPLAY_VER(dev_priv) == 9 && !IS_BROXTON(dev_priv)) {
@@ -3317,7 +3317,7 @@ static void intel_enable_ddi_hdmi(struct intel_atomic_state *state,
 		intel_de_write(dev_priv, reg, val);
 	}
 
-	intel_ddi_power_up_lanes(encoder, crtc_state);
+	intel_ddi_power_up_lanes(encoder, master_crtc_state);
 
 	/* In HDMI/DVI mode, the port width, and swing/emphasis values
 	 * are ignored so nothing special needs to be done besides
@@ -3327,12 +3327,12 @@ static void intel_enable_ddi_hdmi(struct intel_atomic_state *state,
 	 * these are both 0 for HDMI.
 	 *
 	 * But MTL onwards HDMI2.1 is supported and in TMDS mode this
-	 * is filled with lane count, already set in the crtc_state.
+	 * is filled with lane count, already set in the master_crtc_state.
 	 * The same is required to be filled in PORT_BUF_CTL for C10/20 Phy.
 	 */
 	buf_ctl = dig_port->saved_port_bits | DDI_BUF_CTL_ENABLE;
 	if (DISPLAY_VER(dev_priv) >= 14) {
-		u8  lane_count = mtl_get_port_width(crtc_state->lane_count);
+		u8  lane_count = mtl_get_port_width(master_crtc_state->lane_count);
 		u32 port_buf = 0;
 
 		port_buf |= XELPDP_PORT_WIDTH(lane_count);
@@ -3359,35 +3359,35 @@ static void intel_enable_ddi_hdmi(struct intel_atomic_state *state,
 
 static void intel_enable_ddi(struct intel_atomic_state *state,
 			     struct intel_encoder *encoder,
-			     const struct intel_crtc_state *crtc_state,
+			     const struct intel_crtc_state *master_crtc_state,
 			     const struct drm_connector_state *conn_state)
 {
-	drm_WARN_ON(state->base.dev, crtc_state->has_pch_encoder);
+	drm_WARN_ON(state->base.dev, master_crtc_state->has_pch_encoder);
 
-	if (!intel_crtc_is_bigjoiner_slave(crtc_state))
-		intel_ddi_enable_transcoder_func(encoder, crtc_state);
+	if (!intel_crtc_is_bigjoiner_slave(master_crtc_state))
+		intel_ddi_enable_transcoder_func(encoder, master_crtc_state);
 
 	/* Enable/Disable DP2.0 SDP split config before transcoder */
-	intel_audio_sdp_split_update(crtc_state);
+	intel_audio_sdp_split_update(master_crtc_state);
 
-	intel_enable_transcoder(crtc_state);
+	intel_enable_transcoder(master_crtc_state);
 
-	intel_ddi_wait_for_fec_status(encoder, crtc_state, true);
+	intel_ddi_wait_for_fec_status(encoder, master_crtc_state, true);
 
-	intel_crtc_vblank_on(crtc_state);
+	intel_crtc_vblank_on(master_crtc_state);
 
-	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
-		intel_enable_ddi_hdmi(state, encoder, crtc_state, conn_state);
+	if (intel_crtc_has_type(master_crtc_state, INTEL_OUTPUT_HDMI))
+		intel_enable_ddi_hdmi(state, encoder, master_crtc_state, conn_state);
 	else
-		intel_enable_ddi_dp(state, encoder, crtc_state, conn_state);
+		intel_enable_ddi_dp(state, encoder, master_crtc_state, conn_state);
 
-	intel_hdcp_enable(state, encoder, crtc_state, conn_state);
+	intel_hdcp_enable(state, encoder, master_crtc_state, conn_state);
 
 }
 
 static void intel_disable_ddi_dp(struct intel_atomic_state *state,
 				 struct intel_encoder *encoder,
-				 const struct intel_crtc_state *old_crtc_state,
+				 const struct intel_crtc_state *old_master_crtc_state,
 				 const struct drm_connector_state *old_conn_state)
 {
 	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
@@ -3396,19 +3396,19 @@ static void intel_disable_ddi_dp(struct intel_atomic_state *state,
 
 	intel_dp->link_trained = false;
 
-	intel_psr_disable(intel_dp, old_crtc_state);
+	intel_psr_disable(intel_dp, old_master_crtc_state);
 	intel_edp_backlight_off(old_conn_state);
 	/* Disable the decompression in DP Sink */
 	intel_dp_sink_disable_decompression(state,
-					    connector, old_crtc_state);
+					    connector, old_master_crtc_state);
 	/* Disable Ignore_MSA bit in DP Sink */
-	intel_dp_sink_set_msa_timing_par_ignore_state(intel_dp, old_crtc_state,
+	intel_dp_sink_set_msa_timing_par_ignore_state(intel_dp, old_master_crtc_state,
 						      false);
 }
 
 static void intel_disable_ddi_hdmi(struct intel_atomic_state *state,
 				   struct intel_encoder *encoder,
-				   const struct intel_crtc_state *old_crtc_state,
+				   const struct intel_crtc_state *old_master_crtc_state,
 				   const struct drm_connector_state *old_conn_state)
 {
 	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
@@ -3423,55 +3423,55 @@ static void intel_disable_ddi_hdmi(struct intel_atomic_state *state,
 
 static void intel_disable_ddi(struct intel_atomic_state *state,
 			      struct intel_encoder *encoder,
-			      const struct intel_crtc_state *old_crtc_state,
+			      const struct intel_crtc_state *old_master_crtc_state,
 			      const struct drm_connector_state *old_conn_state)
 {
 	intel_tc_port_link_cancel_reset_work(enc_to_dig_port(encoder));
 
 	intel_hdcp_disable(to_intel_connector(old_conn_state->connector));
 
-	if (intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_HDMI))
-		intel_disable_ddi_hdmi(state, encoder, old_crtc_state,
+	if (intel_crtc_has_type(old_master_crtc_state, INTEL_OUTPUT_HDMI))
+		intel_disable_ddi_hdmi(state, encoder, old_master_crtc_state,
 				       old_conn_state);
 	else
-		intel_disable_ddi_dp(state, encoder, old_crtc_state,
+		intel_disable_ddi_dp(state, encoder, old_master_crtc_state,
 				     old_conn_state);
 }
 
 static void intel_ddi_update_pipe_dp(struct intel_atomic_state *state,
 				     struct intel_encoder *encoder,
-				     const struct intel_crtc_state *crtc_state,
+				     const struct intel_crtc_state *master_crtc_state,
 				     const struct drm_connector_state *conn_state)
 {
-	intel_ddi_set_dp_msa(crtc_state, conn_state);
+	intel_ddi_set_dp_msa(master_crtc_state, conn_state);
 
-	intel_dp_set_infoframes(encoder, true, crtc_state, conn_state);
+	intel_dp_set_infoframes(encoder, true, master_crtc_state, conn_state);
 
-	intel_backlight_update(state, encoder, crtc_state, conn_state);
+	intel_backlight_update(state, encoder, master_crtc_state, conn_state);
 	drm_connector_update_privacy_screen(conn_state);
 }
 
 void intel_ddi_update_pipe(struct intel_atomic_state *state,
 			   struct intel_encoder *encoder,
-			   const struct intel_crtc_state *crtc_state,
+			   const struct intel_crtc_state *master_crtc_state,
 			   const struct drm_connector_state *conn_state)
 {
 
-	if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI) &&
+	if (!intel_crtc_has_type(master_crtc_state, INTEL_OUTPUT_HDMI) &&
 	    !intel_encoder_is_mst(encoder))
-		intel_ddi_update_pipe_dp(state, encoder, crtc_state,
+		intel_ddi_update_pipe_dp(state, encoder, master_crtc_state,
 					 conn_state);
 
-	intel_hdcp_update_pipe(state, encoder, crtc_state, conn_state);
+	intel_hdcp_update_pipe(state, encoder, master_crtc_state, conn_state);
 }
 
 void intel_ddi_update_active_dpll(struct intel_atomic_state *state,
 				  struct intel_encoder *encoder,
-				  struct intel_crtc *crtc)
+				  struct intel_crtc *master_crtc)
 {
 	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
-	struct intel_crtc_state *crtc_state =
-		intel_atomic_get_new_crtc_state(state, crtc);
+	struct intel_crtc_state *master_crtc_state =
+		intel_atomic_get_new_crtc_state(state, master_crtc);
 	struct intel_crtc *slave_crtc;
 	enum phy phy = intel_port_to_phy(i915, encoder->port);
 
@@ -3479,16 +3479,16 @@ void intel_ddi_update_active_dpll(struct intel_atomic_state *state,
 	if (DISPLAY_VER(i915) >= 14 || !intel_phy_is_tc(i915, phy))
 		return;
 
-	intel_update_active_dpll(state, crtc, encoder);
+	intel_update_active_dpll(state, master_crtc, encoder);
 	for_each_intel_crtc_in_pipe_mask(&i915->drm, slave_crtc,
-					 intel_crtc_bigjoiner_slave_pipes(crtc_state))
+					 intel_crtc_bigjoiner_slave_pipes(master_crtc_state))
 		intel_update_active_dpll(state, slave_crtc, encoder);
 }
 
 static void
 intel_ddi_pre_pll_enable(struct intel_atomic_state *state,
 			 struct intel_encoder *encoder,
-			 const struct intel_crtc_state *crtc_state,
+			 const struct intel_crtc_state *master_crtc_state,
 			 const struct drm_connector_state *conn_state)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
@@ -3498,23 +3498,23 @@ intel_ddi_pre_pll_enable(struct intel_atomic_state *state,
 
 	if (is_tc_port) {
 		struct intel_crtc *master_crtc =
-			to_intel_crtc(crtc_state->uapi.crtc);
+			to_intel_crtc(master_crtc_state->uapi.crtc);
 
-		intel_tc_port_get_link(dig_port, crtc_state->lane_count);
+		intel_tc_port_get_link(dig_port, master_crtc_state->lane_count);
 		intel_ddi_update_active_dpll(state, encoder, master_crtc);
 	}
 
-	main_link_aux_power_domain_get(dig_port, crtc_state);
+	main_link_aux_power_domain_get(dig_port, master_crtc_state);
 
 	if (is_tc_port && !intel_tc_port_in_tbt_alt_mode(dig_port))
 		/*
 		 * Program the lane count for static/dynamic connections on
 		 * Type-C ports.  Skip this step for TBT.
 		 */
-		intel_tc_port_set_fia_lane_count(dig_port, crtc_state->lane_count);
+		intel_tc_port_set_fia_lane_count(dig_port, master_crtc_state->lane_count);
 	else if (IS_GEMINILAKE(dev_priv) || IS_BROXTON(dev_priv))
 		bxt_ddi_phy_set_lane_optim_mask(encoder,
-						crtc_state->lane_lat_optim_mask);
+						master_crtc_state->lane_lat_optim_mask);
 }
 
 static void adlp_tbt_to_dp_alt_switch_wa(struct intel_encoder *encoder)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index ab2f52d21bad..f29d38a8035e 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -1634,88 +1634,88 @@ static void hsw_configure_cpu_transcoder(const struct intel_crtc_state *crtc_sta
 }
 
 static void hsw_crtc_enable(struct intel_atomic_state *state,
-			    struct intel_crtc *crtc)
+			    struct intel_crtc *master_crtc)
 {
-	const struct intel_crtc_state *new_crtc_state =
-		intel_atomic_get_new_crtc_state(state, crtc);
-	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-	enum pipe pipe = crtc->pipe, hsw_workaround_pipe;
-	enum transcoder cpu_transcoder = new_crtc_state->cpu_transcoder;
+	const struct intel_crtc_state *new_master_crtc_state =
+		intel_atomic_get_new_crtc_state(state, master_crtc);
+	struct drm_i915_private *dev_priv = to_i915(master_crtc->base.dev);
+	enum transcoder cpu_transcoder = new_master_crtc_state->cpu_transcoder;
+	enum pipe hsw_workaround_pipe;
 	bool psl_clkgate_wa;
 
-	if (drm_WARN_ON(&dev_priv->drm, crtc->active))
+	if (drm_WARN_ON(&dev_priv->drm, master_crtc->active))
 		return;
 
-	intel_dmc_enable_pipe(dev_priv, crtc->pipe);
+	intel_dmc_enable_pipe(dev_priv, master_crtc->pipe);
 
-	if (!new_crtc_state->bigjoiner_pipes) {
-		intel_encoders_pre_pll_enable(state, crtc);
+	if (!new_master_crtc_state->bigjoiner_pipes) {
+		intel_encoders_pre_pll_enable(state, master_crtc);
 
-		if (new_crtc_state->shared_dpll)
-			intel_enable_shared_dpll(new_crtc_state);
+		if (new_master_crtc_state->shared_dpll)
+			intel_enable_shared_dpll(new_master_crtc_state);
 
-		intel_encoders_pre_enable(state, crtc);
+		intel_encoders_pre_enable(state, master_crtc);
 	} else {
-		icl_ddi_bigjoiner_pre_enable(state, new_crtc_state);
+		icl_ddi_bigjoiner_pre_enable(state, new_master_crtc_state);
 	}
 
-	intel_dsc_enable(new_crtc_state);
+	intel_dsc_enable(new_master_crtc_state);
 
 	if (DISPLAY_VER(dev_priv) >= 13)
-		intel_uncompressed_joiner_enable(new_crtc_state);
+		intel_uncompressed_joiner_enable(new_master_crtc_state);
 
-	intel_set_pipe_src_size(new_crtc_state);
+	intel_set_pipe_src_size(new_master_crtc_state);
 	if (DISPLAY_VER(dev_priv) >= 9 || IS_BROADWELL(dev_priv))
-		bdw_set_pipe_misc(new_crtc_state);
+		bdw_set_pipe_misc(new_master_crtc_state);
 
-	if (!intel_crtc_is_bigjoiner_slave(new_crtc_state) &&
+	if (!intel_crtc_is_bigjoiner_slave(new_master_crtc_state) &&
 	    !transcoder_is_dsi(cpu_transcoder))
-		hsw_configure_cpu_transcoder(new_crtc_state);
+		hsw_configure_cpu_transcoder(new_master_crtc_state);
 
-	crtc->active = true;
+	master_crtc->active = true;
 
 	/* Display WA #1180: WaDisableScalarClockGating: glk */
 	psl_clkgate_wa = DISPLAY_VER(dev_priv) == 10 &&
-		new_crtc_state->pch_pfit.enabled;
+		new_master_crtc_state->pch_pfit.enabled;
 	if (psl_clkgate_wa)
-		glk_pipe_scaler_clock_gating_wa(dev_priv, pipe, true);
+		glk_pipe_scaler_clock_gating_wa(dev_priv, master_crtc->pipe, true);
 
 	if (DISPLAY_VER(dev_priv) >= 9)
-		skl_pfit_enable(new_crtc_state);
+		skl_pfit_enable(new_master_crtc_state);
 	else
-		ilk_pfit_enable(new_crtc_state);
+		ilk_pfit_enable(new_master_crtc_state);
 
 	/*
 	 * On ILK+ LUT must be loaded before the pipe is running but with
 	 * clocks enabled
 	 */
-	intel_color_load_luts(new_crtc_state);
-	intel_color_commit_noarm(new_crtc_state);
-	intel_color_commit_arm(new_crtc_state);
+	intel_color_load_luts(new_master_crtc_state);
+	intel_color_commit_noarm(new_master_crtc_state);
+	intel_color_commit_arm(new_master_crtc_state);
 	/* update DSPCNTR to configure gamma/csc for pipe bottom color */
 	if (DISPLAY_VER(dev_priv) < 9)
-		intel_disable_primary_plane(new_crtc_state);
+		intel_disable_primary_plane(new_master_crtc_state);
 
-	hsw_set_linetime_wm(new_crtc_state);
+	hsw_set_linetime_wm(new_master_crtc_state);
 
 	if (DISPLAY_VER(dev_priv) >= 11)
-		icl_set_pipe_chicken(new_crtc_state);
+		icl_set_pipe_chicken(new_master_crtc_state);
 
-	intel_initial_watermarks(state, crtc);
+	intel_initial_watermarks(state, master_crtc);
 
-	if (intel_crtc_is_bigjoiner_slave(new_crtc_state))
-		intel_crtc_vblank_on(new_crtc_state);
+	if (intel_crtc_is_bigjoiner_slave(new_master_crtc_state))
+		intel_crtc_vblank_on(new_master_crtc_state);
 
-	intel_encoders_enable(state, crtc);
+	intel_encoders_enable(state, master_crtc);
 
 	if (psl_clkgate_wa) {
-		intel_crtc_wait_for_next_vblank(crtc);
-		glk_pipe_scaler_clock_gating_wa(dev_priv, pipe, false);
+		intel_crtc_wait_for_next_vblank(master_crtc);
+		glk_pipe_scaler_clock_gating_wa(dev_priv, master_crtc->pipe, false);
 	}
 
 	/* If we change the relative order between pipe/planes enabling, we need
 	 * to change the workaround. */
-	hsw_workaround_pipe = new_crtc_state->hsw_workaround_pipe;
+	hsw_workaround_pipe = new_master_crtc_state->hsw_workaround_pipe;
 	if (IS_HASWELL(dev_priv) && hsw_workaround_pipe != INVALID_PIPE) {
 		struct intel_crtc *wa_crtc;
 
@@ -1781,32 +1781,32 @@ static void ilk_crtc_disable(struct intel_atomic_state *state,
 }
 
 static void hsw_crtc_disable(struct intel_atomic_state *state,
-			     struct intel_crtc *crtc)
+			     struct intel_crtc *master_crtc)
 {
-	const struct intel_crtc_state *old_crtc_state =
-		intel_atomic_get_old_crtc_state(state, crtc);
-	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
+	const struct intel_crtc_state *old_master_crtc_state =
+		intel_atomic_get_old_crtc_state(state, master_crtc);
+	struct drm_i915_private *i915 = to_i915(master_crtc->base.dev);
 
 	/*
 	 * FIXME collapse everything to one hook.
 	 * Need care with mst->ddi interactions.
 	 */
-	if (!intel_crtc_is_bigjoiner_slave(old_crtc_state)) {
-		intel_encoders_disable(state, crtc);
-		intel_encoders_post_disable(state, crtc);
+	if (!intel_crtc_is_bigjoiner_slave(old_master_crtc_state)) {
+		intel_encoders_disable(state, master_crtc);
+		intel_encoders_post_disable(state, master_crtc);
 	}
 
-	intel_disable_shared_dpll(old_crtc_state);
+	intel_disable_shared_dpll(old_master_crtc_state);
 
-	if (!intel_crtc_is_bigjoiner_slave(old_crtc_state)) {
+	if (!intel_crtc_is_bigjoiner_slave(old_master_crtc_state)) {
 		struct intel_crtc *slave_crtc;
 
-		intel_encoders_post_pll_disable(state, crtc);
+		intel_encoders_post_pll_disable(state, master_crtc);
 
-		intel_dmc_disable_pipe(i915, crtc->pipe);
+		intel_dmc_disable_pipe(i915, master_crtc->pipe);
 
 		for_each_intel_crtc_in_pipe_mask(&i915->drm, slave_crtc,
-						 intel_crtc_bigjoiner_slave_pipes(old_crtc_state))
+						 intel_crtc_bigjoiner_slave_pipes(old_master_crtc_state))
 			intel_dmc_disable_pipe(i915, slave_crtc->pipe);
 	}
 }
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index db1254b036f1..e4e71e4ae0e1 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -917,7 +917,7 @@ static void wait_for_act_sent(struct intel_encoder *encoder,
 
 static void intel_mst_disable_dp(struct intel_atomic_state *state,
 				 struct intel_encoder *encoder,
-				 const struct intel_crtc_state *old_crtc_state,
+				 const struct intel_crtc_state *old_master_crtc_state,
 				 const struct drm_connector_state *old_conn_state)
 {
 	struct intel_dp_mst_encoder *intel_mst = enc_to_mst(encoder);
@@ -932,12 +932,12 @@ static void intel_mst_disable_dp(struct intel_atomic_state *state,
 
 	intel_hdcp_disable(intel_mst->connector);
 
-	intel_dp_sink_disable_decompression(state, connector, old_crtc_state);
+	intel_dp_sink_disable_decompression(state, connector, old_master_crtc_state);
 }
 
 static void intel_mst_post_disable_dp(struct intel_atomic_state *state,
 				      struct intel_encoder *encoder,
-				      const struct intel_crtc_state *old_crtc_state,
+				      const struct intel_crtc_state *old_master_crtc_state,
 				      const struct drm_connector_state *old_conn_state)
 {
 	struct intel_dp_mst_encoder *intel_mst = enc_to_mst(encoder);
@@ -960,32 +960,32 @@ static void intel_mst_post_disable_dp(struct intel_atomic_state *state,
 	last_mst_stream = intel_dp->active_mst_links == 0;
 	drm_WARN_ON(&dev_priv->drm,
 		    DISPLAY_VER(dev_priv) >= 12 && last_mst_stream &&
-		    !intel_dp_mst_is_master_trans(old_crtc_state));
+		    !intel_dp_mst_is_master_trans(old_master_crtc_state));
 
-	intel_crtc_vblank_off(old_crtc_state);
+	intel_crtc_vblank_off(old_master_crtc_state);
 
-	intel_disable_transcoder(old_crtc_state);
+	intel_disable_transcoder(old_master_crtc_state);
 
 	drm_dp_remove_payload_part1(&intel_dp->mst_mgr, new_mst_state, new_payload);
 
-	clear_act_sent(encoder, old_crtc_state);
+	clear_act_sent(encoder, old_master_crtc_state);
 
-	intel_de_rmw(dev_priv, TRANS_DDI_FUNC_CTL(old_crtc_state->cpu_transcoder),
+	intel_de_rmw(dev_priv, TRANS_DDI_FUNC_CTL(old_master_crtc_state->cpu_transcoder),
 		     TRANS_DDI_DP_VC_PAYLOAD_ALLOC, 0);
 
-	wait_for_act_sent(encoder, old_crtc_state);
+	wait_for_act_sent(encoder, old_master_crtc_state);
 
 	drm_dp_remove_payload_part2(&intel_dp->mst_mgr, new_mst_state,
 				    old_payload, new_payload);
 
-	intel_ddi_disable_transcoder_func(old_crtc_state);
+	intel_ddi_disable_transcoder_func(old_master_crtc_state);
 
-	intel_dsc_disable(old_crtc_state);
+	intel_dsc_disable(old_master_crtc_state);
 
 	if (DISPLAY_VER(dev_priv) >= 9)
-		skl_scaler_disable(old_crtc_state);
+		skl_scaler_disable(old_master_crtc_state);
 	else
-		ilk_pfit_disable(old_crtc_state);
+		ilk_pfit_disable(old_master_crtc_state);
 
 	/*
 	 * Power down mst path before disabling the port, otherwise we end
@@ -999,7 +999,7 @@ static void intel_mst_post_disable_dp(struct intel_atomic_state *state,
 	 * the transcoder clock select is set to none.
 	 */
 	intel_dp_set_infoframes(&dig_port->base, false,
-				old_crtc_state, NULL);
+				old_master_crtc_state, NULL);
 	/*
 	 * From TGL spec: "If multi-stream slave transcoder: Configure
 	 * Transcoder Clock Select to direct no clock to the transcoder"
@@ -1008,13 +1008,12 @@ static void intel_mst_post_disable_dp(struct intel_atomic_state *state,
 	 * no clock to the transcoder"
 	 */
 	if (DISPLAY_VER(dev_priv) < 12 || !last_mst_stream)
-		intel_ddi_disable_transcoder_clock(old_crtc_state);
-
+		intel_ddi_disable_transcoder_clock(old_master_crtc_state);
 
 	intel_mst->connector = NULL;
 	if (last_mst_stream)
 		dig_port->base.post_disable(state, &dig_port->base,
-						  old_crtc_state, NULL);
+						  old_master_crtc_state, NULL);
 
 	drm_dbg_kms(&dev_priv->drm, "active links %d\n",
 		    intel_dp->active_mst_links);
@@ -1022,7 +1021,7 @@ static void intel_mst_post_disable_dp(struct intel_atomic_state *state,
 
 static void intel_mst_post_pll_disable_dp(struct intel_atomic_state *state,
 					  struct intel_encoder *encoder,
-					  const struct intel_crtc_state *old_crtc_state,
+					  const struct intel_crtc_state *old_master_crtc_state,
 					  const struct drm_connector_state *old_conn_state)
 {
 	struct intel_dp_mst_encoder *intel_mst = enc_to_mst(encoder);
@@ -1031,33 +1030,33 @@ static void intel_mst_post_pll_disable_dp(struct intel_atomic_state *state,
 
 	if (intel_dp->active_mst_links == 0 &&
 	    dig_port->base.post_pll_disable)
-		dig_port->base.post_pll_disable(state, encoder, old_crtc_state, old_conn_state);
+		dig_port->base.post_pll_disable(state, encoder, old_master_crtc_state, old_conn_state);
 }
 
 static void intel_mst_pre_pll_enable_dp(struct intel_atomic_state *state,
 					struct intel_encoder *encoder,
-					const struct intel_crtc_state *pipe_config,
+					const struct intel_crtc_state *master_crtc_state,
 					const struct drm_connector_state *conn_state)
 {
+	struct intel_crtc *master_crtc = to_intel_crtc(master_crtc_state->uapi.crtc);
 	struct intel_dp_mst_encoder *intel_mst = enc_to_mst(encoder);
 	struct intel_digital_port *dig_port = intel_mst->primary;
 	struct intel_dp *intel_dp = &dig_port->dp;
 
 	if (intel_dp->active_mst_links == 0)
 		dig_port->base.pre_pll_enable(state, &dig_port->base,
-						    pipe_config, NULL);
+					      master_crtc_state, NULL);
 	else
 		/*
 		 * The port PLL state needs to get updated for secondary
 		 * streams as for the primary stream.
 		 */
-		intel_ddi_update_active_dpll(state, &dig_port->base,
-					     to_intel_crtc(pipe_config->uapi.crtc));
+		intel_ddi_update_active_dpll(state, &dig_port->base, master_crtc);
 }
 
 static void intel_mst_pre_enable_dp(struct intel_atomic_state *state,
 				    struct intel_encoder *encoder,
-				    const struct intel_crtc_state *pipe_config,
+				    const struct intel_crtc_state *master_crtc_state,
 				    const struct drm_connector_state *conn_state)
 {
 	struct intel_dp_mst_encoder *intel_mst = enc_to_mst(encoder);
@@ -1079,7 +1078,7 @@ static void intel_mst_pre_enable_dp(struct intel_atomic_state *state,
 	first_mst_stream = intel_dp->active_mst_links == 0;
 	drm_WARN_ON(&dev_priv->drm,
 		    DISPLAY_VER(dev_priv) >= 12 && first_mst_stream &&
-		    !intel_dp_mst_is_master_trans(pipe_config));
+		    !intel_dp_mst_is_master_trans(master_crtc_state));
 
 	drm_dbg_kms(&dev_priv->drm, "active links %d\n",
 		    intel_dp->active_mst_links);
@@ -1089,11 +1088,11 @@ static void intel_mst_pre_enable_dp(struct intel_atomic_state *state,
 
 	drm_dp_send_power_updown_phy(&intel_dp->mst_mgr, connector->port, true);
 
-	intel_dp_sink_enable_decompression(state, connector, pipe_config);
+	intel_dp_sink_enable_decompression(state, connector, master_crtc_state);
 
 	if (first_mst_stream)
 		dig_port->base.pre_enable(state, &dig_port->base,
-						pipe_config, NULL);
+						master_crtc_state, NULL);
 
 	intel_dp->active_mst_links++;
 
@@ -1111,15 +1110,15 @@ static void intel_mst_pre_enable_dp(struct intel_atomic_state *state,
 	 * here for the following ones.
 	 */
 	if (DISPLAY_VER(dev_priv) < 12 || !first_mst_stream)
-		intel_ddi_enable_transcoder_clock(encoder, pipe_config);
+		intel_ddi_enable_transcoder_clock(encoder, master_crtc_state);
 
-	intel_dsc_dp_pps_write(&dig_port->base, pipe_config);
-	intel_ddi_set_dp_msa(pipe_config, conn_state);
+	intel_dsc_dp_pps_write(&dig_port->base, master_crtc_state);
+	intel_ddi_set_dp_msa(master_crtc_state, conn_state);
 }
 
 static void intel_mst_enable_dp(struct intel_atomic_state *state,
 				struct intel_encoder *encoder,
-				const struct intel_crtc_state *pipe_config,
+				const struct intel_crtc_state *master_crtc_state,
 				const struct drm_connector_state *conn_state)
 {
 	struct intel_dp_mst_encoder *intel_mst = enc_to_mst(encoder);
@@ -1129,25 +1128,25 @@ static void intel_mst_enable_dp(struct intel_atomic_state *state,
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	struct drm_dp_mst_topology_state *mst_state =
 		drm_atomic_get_new_mst_topology_state(&state->base, &intel_dp->mst_mgr);
-	enum transcoder trans = pipe_config->cpu_transcoder;
+	enum transcoder trans = master_crtc_state->cpu_transcoder;
 	bool first_mst_stream = intel_dp->active_mst_links == 1;
 
-	drm_WARN_ON(&dev_priv->drm, pipe_config->has_pch_encoder);
+	drm_WARN_ON(&dev_priv->drm, master_crtc_state->has_pch_encoder);
 
-	if (intel_dp_is_uhbr(pipe_config)) {
+	if (intel_dp_is_uhbr(master_crtc_state)) {
 		const struct drm_display_mode *adjusted_mode =
-			&pipe_config->hw.adjusted_mode;
+			&master_crtc_state->hw.adjusted_mode;
 		u64 crtc_clock_hz = KHz(adjusted_mode->crtc_clock);
 
-		intel_de_write(dev_priv, TRANS_DP2_VFREQHIGH(pipe_config->cpu_transcoder),
+		intel_de_write(dev_priv, TRANS_DP2_VFREQHIGH(master_crtc_state->cpu_transcoder),
 			       TRANS_DP2_VFREQ_PIXEL_CLOCK(crtc_clock_hz >> 24));
-		intel_de_write(dev_priv, TRANS_DP2_VFREQLOW(pipe_config->cpu_transcoder),
+		intel_de_write(dev_priv, TRANS_DP2_VFREQLOW(master_crtc_state->cpu_transcoder),
 			       TRANS_DP2_VFREQ_PIXEL_CLOCK(crtc_clock_hz & 0xffffff));
 	}
 
-	intel_ddi_enable_transcoder_func(encoder, pipe_config);
+	intel_ddi_enable_transcoder_func(encoder, master_crtc_state);
 
-	clear_act_sent(encoder, pipe_config);
+	clear_act_sent(encoder, master_crtc_state);
 
 	intel_de_rmw(dev_priv, TRANS_DDI_FUNC_CTL(trans), 0,
 		     TRANS_DDI_DP_VC_PAYLOAD_ALLOC);
@@ -1155,10 +1154,10 @@ static void intel_mst_enable_dp(struct intel_atomic_state *state,
 	drm_dbg_kms(&dev_priv->drm, "active links %d\n",
 		    intel_dp->active_mst_links);
 
-	wait_for_act_sent(encoder, pipe_config);
+	wait_for_act_sent(encoder, master_crtc_state);
 
 	if (first_mst_stream)
-		intel_ddi_wait_for_fec_status(encoder, pipe_config, true);
+		intel_ddi_wait_for_fec_status(encoder, master_crtc_state, true);
 
 	drm_dp_add_payload_part2(&intel_dp->mst_mgr, &state->base,
 				 drm_atomic_get_mst_payload_state(mst_state, connector->port));
@@ -1166,15 +1165,15 @@ static void intel_mst_enable_dp(struct intel_atomic_state *state,
 	if (DISPLAY_VER(dev_priv) >= 12)
 		intel_de_rmw(dev_priv, hsw_chicken_trans_reg(dev_priv, trans),
 			     FECSTALL_DIS_DPTSTREAM_DPTTG,
-			     pipe_config->fec_enable ? FECSTALL_DIS_DPTSTREAM_DPTTG : 0);
+			     master_crtc_state->fec_enable ? FECSTALL_DIS_DPTSTREAM_DPTTG : 0);
 
-	intel_audio_sdp_split_update(pipe_config);
+	intel_audio_sdp_split_update(master_crtc_state);
 
-	intel_enable_transcoder(pipe_config);
+	intel_enable_transcoder(master_crtc_state);
 
-	intel_crtc_vblank_on(pipe_config);
+	intel_crtc_vblank_on(master_crtc_state);
 
-	intel_hdcp_enable(state, encoder, pipe_config, conn_state);
+	intel_hdcp_enable(state, encoder, master_crtc_state, conn_state);
 }
 
 static bool intel_dp_mst_enc_get_hw_state(struct intel_encoder *encoder,
-- 
2.43.0

Re: [PATCH 1/8] drm/i915: Rename the crtc/crtc_states in the top level DDI hooks/etc

[Lisovskiy, Stanislav]

On Fri, Mar 01, 2024 at 04:35:53PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> In preparation for doing a more sensible pipe vs. transcoder
> handling for bigjoiner let's rename the crtc/crtc_state in the
> top level crtc_enable/disable and the DDI encoder hooks to
> include "master" in the name. This way they won't collide with
> the per-pipe stuff.
> 
> Note that at this point this is (at least partially) telling
> lies as we still run through some of these for slave pipes as
> well. But I wanted to get the huge rename out of the way so
> it won't clutter the functional patches so much.
> 
> TODO: or perhaps use some other names for the per-pipe stuff instead?
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

I will then review now the patches which you could merge before the bigjoiner
stuff could be finished.
Checked this patch I guess, you were also talking that this renaming might
be not the best idea.
I also wonder whether should we really emphasize things like "master"/"slave"
in function names. I thought that one idea in our refactoring was to unify
joined pipes handling so that there are no(or at least almost no) explicit code
paths/function names for masters/slaves.

Stan

> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c     | 326 +++++++++----------
>  drivers/gpu/drm/i915/display/intel_display.c | 100 +++---
>  drivers/gpu/drm/i915/display/intel_dp_mst.c  |  91 +++---
>  3 files changed, 258 insertions(+), 259 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index c587a8efeafc..6287629f9e77 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -2490,21 +2490,21 @@ static void mtl_port_buf_ctl_io_selection(struct intel_encoder *encoder)
>  
>  static void mtl_ddi_pre_enable_dp(struct intel_atomic_state *state,
>  				  struct intel_encoder *encoder,
> -				  const struct intel_crtc_state *crtc_state,
> +				  const struct intel_crtc_state *master_crtc_state,
>  				  const struct drm_connector_state *conn_state)
>  {
>  	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> -	bool is_mst = intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST);
> +	bool is_mst = intel_crtc_has_type(master_crtc_state, INTEL_OUTPUT_DP_MST);
>  
>  	intel_dp_set_link_params(intel_dp,
> -				 crtc_state->port_clock,
> -				 crtc_state->lane_count);
> +				 master_crtc_state->port_clock,
> +				 master_crtc_state->lane_count);
>  
>  	/*
>  	 * We only configure what the register value will be here.  Actual
>  	 * enabling happens during link training farther down.
>  	 */
> -	intel_ddi_init_dp_buf_reg(encoder, crtc_state);
> +	intel_ddi_init_dp_buf_reg(encoder, master_crtc_state);
>  
>  	/*
>  	 * 1. Enable Power Wells
> @@ -2522,48 +2522,48 @@ static void mtl_ddi_pre_enable_dp(struct intel_atomic_state *state,
>  	intel_pps_on(intel_dp);
>  
>  	/* 5. Enable the port PLL */
> -	intel_ddi_enable_clock(encoder, crtc_state);
> +	intel_ddi_enable_clock(encoder, master_crtc_state);
>  
>  	/*
>  	 * 6.a Configure Transcoder Clock Select to direct the Port clock to the
>  	 * Transcoder.
>  	 */
> -	intel_ddi_enable_transcoder_clock(encoder, crtc_state);
> +	intel_ddi_enable_transcoder_clock(encoder, master_crtc_state);
>  
>  	/*
>  	 * 6.b If DP v2.0/128b mode - Configure TRANS_DP2_CTL register settings.
>  	 */
> -	intel_ddi_config_transcoder_dp2(encoder, crtc_state);
> +	intel_ddi_config_transcoder_dp2(encoder, master_crtc_state);
>  
>  	/*
>  	 * 6.c Configure TRANS_DDI_FUNC_CTL DDI Select, DDI Mode Select & MST
>  	 * Transport Select
>  	 */
> -	intel_ddi_config_transcoder_func(encoder, crtc_state);
> +	intel_ddi_config_transcoder_func(encoder, master_crtc_state);
>  
>  	/*
>  	 * 6.e Program CoG/MSO configuration bits in DSS_CTL1 if selected.
>  	 */
> -	intel_ddi_mso_configure(crtc_state);
> +	intel_ddi_mso_configure(master_crtc_state);
>  
>  	if (!is_mst)
>  		intel_dp_set_power(intel_dp, DP_SET_POWER_D0);
>  
> -	intel_dp_configure_protocol_converter(intel_dp, crtc_state);
> +	intel_dp_configure_protocol_converter(intel_dp, master_crtc_state);
>  	if (!is_mst)
>  		intel_dp_sink_enable_decompression(state,
>  						   to_intel_connector(conn_state->connector),
> -						   crtc_state);
> +						   master_crtc_state);
>  
>  	/*
>  	 * DDI FEC: "anticipates enabling FEC encoding sets the FEC_READY bit
>  	 * in the FEC_CONFIGURATION register to 1 before initiating link
>  	 * training
>  	 */
> -	intel_dp_sink_set_fec_ready(intel_dp, crtc_state, true);
> +	intel_dp_sink_set_fec_ready(intel_dp, master_crtc_state, true);
>  
>  	intel_dp_check_frl_training(intel_dp);
> -	intel_dp_pcon_dsc_configure(intel_dp, crtc_state);
> +	intel_dp_pcon_dsc_configure(intel_dp, master_crtc_state);
>  
>  	/*
>  	 * 6. The rest of the below are substeps under the bspec's "Enable and
> @@ -2584,38 +2584,38 @@ static void mtl_ddi_pre_enable_dp(struct intel_atomic_state *state,
>  	 *     Pattern, wait for 5 idle patterns (DP_TP_STATUS Min_Idles_Sent)
>  	 *     (timeout after 800 us)
>  	 */
> -	intel_dp_start_link_train(intel_dp, crtc_state);
> +	intel_dp_start_link_train(intel_dp, master_crtc_state);
>  
>  	/* 6.n Set DP_TP_CTL link training to Normal */
> -	if (!is_trans_port_sync_mode(crtc_state))
> -		intel_dp_stop_link_train(intel_dp, crtc_state);
> +	if (!is_trans_port_sync_mode(master_crtc_state))
> +		intel_dp_stop_link_train(intel_dp, master_crtc_state);
>  
>  	/* 6.o Configure and enable FEC if needed */
> -	intel_ddi_enable_fec(encoder, crtc_state);
> +	intel_ddi_enable_fec(encoder, master_crtc_state);
>  
>  	if (!is_mst)
> -		intel_dsc_dp_pps_write(encoder, crtc_state);
> +		intel_dsc_dp_pps_write(encoder, master_crtc_state);
>  }
>  
>  static void tgl_ddi_pre_enable_dp(struct intel_atomic_state *state,
>  				  struct intel_encoder *encoder,
> -				  const struct intel_crtc_state *crtc_state,
> +				  const struct intel_crtc_state *master_crtc_state,
>  				  const struct drm_connector_state *conn_state)
>  {
>  	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
> -	bool is_mst = intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST);
> +	bool is_mst = intel_crtc_has_type(master_crtc_state, INTEL_OUTPUT_DP_MST);
>  
>  	intel_dp_set_link_params(intel_dp,
> -				 crtc_state->port_clock,
> -				 crtc_state->lane_count);
> +				 master_crtc_state->port_clock,
> +				 master_crtc_state->lane_count);
>  
>  	/*
>  	 * We only configure what the register value will be here.  Actual
>  	 * enabling happens during link training farther down.
>  	 */
> -	intel_ddi_init_dp_buf_reg(encoder, crtc_state);
> +	intel_ddi_init_dp_buf_reg(encoder, master_crtc_state);
>  
>  	/*
>  	 * 1. Enable Power Wells
> @@ -2642,7 +2642,7 @@ static void tgl_ddi_pre_enable_dp(struct intel_atomic_state *state,
>  	 * hsw_crtc_enable()->intel_enable_shared_dpll().  We need only
>  	 * configure the PLL to port mapping here.
>  	 */
> -	intel_ddi_enable_clock(encoder, crtc_state);
> +	intel_ddi_enable_clock(encoder, master_crtc_state);
>  
>  	/* 5. If IO power is controlled through PWR_WELL_CTL, Enable IO Power */
>  	if (!intel_tc_port_in_tbt_alt_mode(dig_port)) {
> @@ -2652,7 +2652,7 @@ static void tgl_ddi_pre_enable_dp(struct intel_atomic_state *state,
>  	}
>  
>  	/* 6. Program DP_MODE */
> -	icl_program_mg_dp_mode(dig_port, crtc_state);
> +	icl_program_mg_dp_mode(dig_port, master_crtc_state);
>  
>  	/*
>  	 * 7. The rest of the below are substeps under the bspec's "Enable and
> @@ -2668,16 +2668,16 @@ static void tgl_ddi_pre_enable_dp(struct intel_atomic_state *state,
>  	 * 7.a Configure Transcoder Clock Select to direct the Port clock to the
>  	 * Transcoder.
>  	 */
> -	intel_ddi_enable_transcoder_clock(encoder, crtc_state);
> +	intel_ddi_enable_transcoder_clock(encoder, master_crtc_state);
>  
>  	if (HAS_DP20(dev_priv))
> -		intel_ddi_config_transcoder_dp2(encoder, crtc_state);
> +		intel_ddi_config_transcoder_dp2(encoder, master_crtc_state);
>  
>  	/*
>  	 * 7.b Configure TRANS_DDI_FUNC_CTL DDI Select, DDI Mode Select & MST
>  	 * Transport Select
>  	 */
> -	intel_ddi_config_transcoder_func(encoder, crtc_state);
> +	intel_ddi_config_transcoder_func(encoder, master_crtc_state);
>  
>  	/*
>  	 * 7.c Configure & enable DP_TP_CTL with link training pattern 1
> @@ -2688,36 +2688,36 @@ static void tgl_ddi_pre_enable_dp(struct intel_atomic_state *state,
>  	 */
>  
>  	/* 7.e Configure voltage swing and related IO settings */
> -	encoder->set_signal_levels(encoder, crtc_state);
> +	encoder->set_signal_levels(encoder, master_crtc_state);
>  
>  	/*
>  	 * 7.f Combo PHY: Configure PORT_CL_DW10 Static Power Down to power up
>  	 * the used lanes of the DDI.
>  	 */
> -	intel_ddi_power_up_lanes(encoder, crtc_state);
> +	intel_ddi_power_up_lanes(encoder, master_crtc_state);
>  
>  	/*
>  	 * 7.g Program CoG/MSO configuration bits in DSS_CTL1 if selected.
>  	 */
> -	intel_ddi_mso_configure(crtc_state);
> +	intel_ddi_mso_configure(master_crtc_state);
>  
>  	if (!is_mst)
>  		intel_dp_set_power(intel_dp, DP_SET_POWER_D0);
>  
> -	intel_dp_configure_protocol_converter(intel_dp, crtc_state);
> +	intel_dp_configure_protocol_converter(intel_dp, master_crtc_state);
>  	if (!is_mst)
>  		intel_dp_sink_enable_decompression(state,
>  						   to_intel_connector(conn_state->connector),
> -						   crtc_state);
> +						   master_crtc_state);
>  	/*
>  	 * DDI FEC: "anticipates enabling FEC encoding sets the FEC_READY bit
>  	 * in the FEC_CONFIGURATION register to 1 before initiating link
>  	 * training
>  	 */
> -	intel_dp_sink_set_fec_ready(intel_dp, crtc_state, true);
> +	intel_dp_sink_set_fec_ready(intel_dp, master_crtc_state, true);
>  
>  	intel_dp_check_frl_training(intel_dp);
> -	intel_dp_pcon_dsc_configure(intel_dp, crtc_state);
> +	intel_dp_pcon_dsc_configure(intel_dp, master_crtc_state);
>  
>  	/*
>  	 * 7.i Follow DisplayPort specification training sequence (see notes for
> @@ -2726,29 +2726,29 @@ static void tgl_ddi_pre_enable_dp(struct intel_atomic_state *state,
>  	 *     Pattern, wait for 5 idle patterns (DP_TP_STATUS Min_Idles_Sent)
>  	 *     (timeout after 800 us)
>  	 */
> -	intel_dp_start_link_train(intel_dp, crtc_state);
> +	intel_dp_start_link_train(intel_dp, master_crtc_state);
>  
>  	/* 7.k Set DP_TP_CTL link training to Normal */
> -	if (!is_trans_port_sync_mode(crtc_state))
> -		intel_dp_stop_link_train(intel_dp, crtc_state);
> +	if (!is_trans_port_sync_mode(master_crtc_state))
> +		intel_dp_stop_link_train(intel_dp, master_crtc_state);
>  
>  	/* 7.l Configure and enable FEC if needed */
> -	intel_ddi_enable_fec(encoder, crtc_state);
> +	intel_ddi_enable_fec(encoder, master_crtc_state);
>  
>  	if (!is_mst)
> -		intel_dsc_dp_pps_write(encoder, crtc_state);
> +		intel_dsc_dp_pps_write(encoder, master_crtc_state);
>  }
>  
>  static void hsw_ddi_pre_enable_dp(struct intel_atomic_state *state,
>  				  struct intel_encoder *encoder,
> -				  const struct intel_crtc_state *crtc_state,
> +				  const struct intel_crtc_state *master_crtc_state,
>  				  const struct drm_connector_state *conn_state)
>  {
>  	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	enum port port = encoder->port;
>  	struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
> -	bool is_mst = intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST);
> +	bool is_mst = intel_crtc_has_type(master_crtc_state, INTEL_OUTPUT_DP_MST);
>  
>  	if (DISPLAY_VER(dev_priv) < 11)
>  		drm_WARN_ON(&dev_priv->drm,
> @@ -2757,18 +2757,18 @@ static void hsw_ddi_pre_enable_dp(struct intel_atomic_state *state,
>  		drm_WARN_ON(&dev_priv->drm, is_mst && port == PORT_A);
>  
>  	intel_dp_set_link_params(intel_dp,
> -				 crtc_state->port_clock,
> -				 crtc_state->lane_count);
> +				 master_crtc_state->port_clock,
> +				 master_crtc_state->lane_count);
>  
>  	/*
>  	 * We only configure what the register value will be here.  Actual
>  	 * enabling happens during link training farther down.
>  	 */
> -	intel_ddi_init_dp_buf_reg(encoder, crtc_state);
> +	intel_ddi_init_dp_buf_reg(encoder, master_crtc_state);
>  
>  	intel_pps_on(intel_dp);
>  
> -	intel_ddi_enable_clock(encoder, crtc_state);
> +	intel_ddi_enable_clock(encoder, master_crtc_state);
>  
>  	if (!intel_tc_port_in_tbt_alt_mode(dig_port)) {
>  		drm_WARN_ON(&dev_priv->drm, dig_port->ddi_io_wakeref);
> @@ -2776,39 +2776,39 @@ static void hsw_ddi_pre_enable_dp(struct intel_atomic_state *state,
>  								   dig_port->ddi_io_power_domain);
>  	}
>  
> -	icl_program_mg_dp_mode(dig_port, crtc_state);
> +	icl_program_mg_dp_mode(dig_port, master_crtc_state);
>  
>  	if (has_buf_trans_select(dev_priv))
> -		hsw_prepare_dp_ddi_buffers(encoder, crtc_state);
> +		hsw_prepare_dp_ddi_buffers(encoder, master_crtc_state);
>  
> -	encoder->set_signal_levels(encoder, crtc_state);
> +	encoder->set_signal_levels(encoder, master_crtc_state);
>  
> -	intel_ddi_power_up_lanes(encoder, crtc_state);
> +	intel_ddi_power_up_lanes(encoder, master_crtc_state);
>  
>  	if (!is_mst)
>  		intel_dp_set_power(intel_dp, DP_SET_POWER_D0);
> -	intel_dp_configure_protocol_converter(intel_dp, crtc_state);
> +	intel_dp_configure_protocol_converter(intel_dp, master_crtc_state);
>  	if (!is_mst)
>  		intel_dp_sink_enable_decompression(state,
>  						   to_intel_connector(conn_state->connector),
> -						   crtc_state);
> -	intel_dp_sink_set_fec_ready(intel_dp, crtc_state, true);
> -	intel_dp_start_link_train(intel_dp, crtc_state);
> +						   master_crtc_state);
> +	intel_dp_sink_set_fec_ready(intel_dp, master_crtc_state, true);
> +	intel_dp_start_link_train(intel_dp, master_crtc_state);
>  	if ((port != PORT_A || DISPLAY_VER(dev_priv) >= 9) &&
> -	    !is_trans_port_sync_mode(crtc_state))
> -		intel_dp_stop_link_train(intel_dp, crtc_state);
> +	    !is_trans_port_sync_mode(master_crtc_state))
> +		intel_dp_stop_link_train(intel_dp, master_crtc_state);
>  
> -	intel_ddi_enable_fec(encoder, crtc_state);
> +	intel_ddi_enable_fec(encoder, master_crtc_state);
>  
>  	if (!is_mst) {
> -		intel_ddi_enable_transcoder_clock(encoder, crtc_state);
> -		intel_dsc_dp_pps_write(encoder, crtc_state);
> +		intel_ddi_enable_transcoder_clock(encoder, master_crtc_state);
> +		intel_dsc_dp_pps_write(encoder, master_crtc_state);
>  	}
>  }
>  
>  static void intel_ddi_pre_enable_dp(struct intel_atomic_state *state,
>  				    struct intel_encoder *encoder,
> -				    const struct intel_crtc_state *crtc_state,
> +				    const struct intel_crtc_state *master_crtc_state,
>  				    const struct drm_connector_state *conn_state)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> @@ -2816,29 +2816,29 @@ static void intel_ddi_pre_enable_dp(struct intel_atomic_state *state,
>  
>  	if (HAS_DP20(dev_priv)) {
>  		intel_dp_128b132b_sdp_crc16(enc_to_intel_dp(encoder),
> -					    crtc_state);
> -		if (crtc_state->has_panel_replay)
> +					    master_crtc_state);
> +		if (master_crtc_state->has_panel_replay)
>  			drm_dp_dpcd_writeb(&intel_dp->aux, PANEL_REPLAY_CONFIG,
>  					   DP_PANEL_REPLAY_ENABLE);
>  	}
>  
>  	if (DISPLAY_VER(dev_priv) >= 14)
> -		mtl_ddi_pre_enable_dp(state, encoder, crtc_state, conn_state);
> +		mtl_ddi_pre_enable_dp(state, encoder, master_crtc_state, conn_state);
>  	else if (DISPLAY_VER(dev_priv) >= 12)
> -		tgl_ddi_pre_enable_dp(state, encoder, crtc_state, conn_state);
> +		tgl_ddi_pre_enable_dp(state, encoder, master_crtc_state, conn_state);
>  	else
> -		hsw_ddi_pre_enable_dp(state, encoder, crtc_state, conn_state);
> +		hsw_ddi_pre_enable_dp(state, encoder, master_crtc_state, conn_state);
>  
>  	/* MST will call a setting of MSA after an allocating of Virtual Channel
>  	 * from MST encoder pre_enable callback.
>  	 */
> -	if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST))
> -		intel_ddi_set_dp_msa(crtc_state, conn_state);
> +	if (!intel_crtc_has_type(master_crtc_state, INTEL_OUTPUT_DP_MST))
> +		intel_ddi_set_dp_msa(master_crtc_state, conn_state);
>  }
>  
>  static void intel_ddi_pre_enable_hdmi(struct intel_atomic_state *state,
>  				      struct intel_encoder *encoder,
> -				      const struct intel_crtc_state *crtc_state,
> +				      const struct intel_crtc_state *master_crtc_state,
>  				      const struct drm_connector_state *conn_state)
>  {
>  	struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
> @@ -2846,27 +2846,27 @@ static void intel_ddi_pre_enable_hdmi(struct intel_atomic_state *state,
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  
>  	intel_dp_dual_mode_set_tmds_output(intel_hdmi, true);
> -	intel_ddi_enable_clock(encoder, crtc_state);
> +	intel_ddi_enable_clock(encoder, master_crtc_state);
>  
>  	drm_WARN_ON(&dev_priv->drm, dig_port->ddi_io_wakeref);
>  	dig_port->ddi_io_wakeref = intel_display_power_get(dev_priv,
>  							   dig_port->ddi_io_power_domain);
>  
> -	icl_program_mg_dp_mode(dig_port, crtc_state);
> +	icl_program_mg_dp_mode(dig_port, master_crtc_state);
>  
> -	intel_ddi_enable_transcoder_clock(encoder, crtc_state);
> +	intel_ddi_enable_transcoder_clock(encoder, master_crtc_state);
>  
>  	dig_port->set_infoframes(encoder,
> -				 crtc_state->has_infoframe,
> -				 crtc_state, conn_state);
> +				 master_crtc_state->has_infoframe,
> +				 master_crtc_state, conn_state);
>  }
>  
>  static void intel_ddi_pre_enable(struct intel_atomic_state *state,
>  				 struct intel_encoder *encoder,
> -				 const struct intel_crtc_state *crtc_state,
> +				 const struct intel_crtc_state *master_crtc_state,
>  				 const struct drm_connector_state *conn_state)
>  {
> -	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> +	struct intel_crtc *crtc = to_intel_crtc(master_crtc_state->uapi.crtc);
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>  	enum pipe pipe = crtc->pipe;
>  
> @@ -2876,32 +2876,32 @@ static void intel_ddi_pre_enable(struct intel_atomic_state *state,
>  	 * - encoder will be the main encoder (ie. mst->primary)
>  	 * - the main connector associated with this port
>  	 *   won't be active or linked to a crtc
> -	 * - crtc_state will be the state of the first stream to
> +	 * - master_crtc_state will be the state of the first stream to
>  	 *   be activated on this port, and it may not be the same
>  	 *   stream that will be deactivated last, but each stream
>  	 *   should have a state that is identical when it comes to
>  	 *   the DP link parameteres
>  	 */
>  
> -	drm_WARN_ON(&dev_priv->drm, crtc_state->has_pch_encoder);
> +	drm_WARN_ON(&dev_priv->drm, master_crtc_state->has_pch_encoder);
>  
>  	intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
>  
> -	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI)) {
> -		intel_ddi_pre_enable_hdmi(state, encoder, crtc_state,
> +	if (intel_crtc_has_type(master_crtc_state, INTEL_OUTPUT_HDMI)) {
> +		intel_ddi_pre_enable_hdmi(state, encoder, master_crtc_state,
>  					  conn_state);
>  	} else {
>  		struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
>  
> -		intel_ddi_pre_enable_dp(state, encoder, crtc_state,
> +		intel_ddi_pre_enable_dp(state, encoder, master_crtc_state,
>  					conn_state);
>  
>  		/* FIXME precompute everything properly */
>  		/* FIXME how do we turn infoframes off again? */
>  		if (dig_port->lspcon.active && intel_dp_has_hdmi_sink(&dig_port->dp))
>  			dig_port->set_infoframes(encoder,
> -						 crtc_state->has_infoframe,
> -						 crtc_state, conn_state);
> +						 master_crtc_state->has_infoframe,
> +						 master_crtc_state, conn_state);
>  	}
>  }
>  
> @@ -3000,19 +3000,19 @@ static void intel_disable_ddi_buf(struct intel_encoder *encoder,
>  
>  static void intel_ddi_post_disable_dp(struct intel_atomic_state *state,
>  				      struct intel_encoder *encoder,
> -				      const struct intel_crtc_state *old_crtc_state,
> +				      const struct intel_crtc_state *old_master_crtc_state,
>  				      const struct drm_connector_state *old_conn_state)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
>  	struct intel_dp *intel_dp = &dig_port->dp;
>  	intel_wakeref_t wakeref;
> -	bool is_mst = intel_crtc_has_type(old_crtc_state,
> +	bool is_mst = intel_crtc_has_type(old_master_crtc_state,
>  					  INTEL_OUTPUT_DP_MST);
>  
>  	if (!is_mst)
>  		intel_dp_set_infoframes(encoder, false,
> -					old_crtc_state, old_conn_state);
> +					old_master_crtc_state, old_conn_state);
>  
>  	/*
>  	 * Power down sink before disabling the port, otherwise we end
> @@ -3022,7 +3022,7 @@ static void intel_ddi_post_disable_dp(struct intel_atomic_state *state,
>  
>  	if (DISPLAY_VER(dev_priv) >= 12) {
>  		if (is_mst) {
> -			enum transcoder cpu_transcoder = old_crtc_state->cpu_transcoder;
> +			enum transcoder cpu_transcoder = old_master_crtc_state->cpu_transcoder;
>  
>  			intel_de_rmw(dev_priv, TRANS_DDI_FUNC_CTL(cpu_transcoder),
>  				     TGL_TRANS_DDI_PORT_MASK | TRANS_DDI_MODE_SELECT_MASK,
> @@ -3030,12 +3030,12 @@ static void intel_ddi_post_disable_dp(struct intel_atomic_state *state,
>  		}
>  	} else {
>  		if (!is_mst)
> -			intel_ddi_disable_transcoder_clock(old_crtc_state);
> +			intel_ddi_disable_transcoder_clock(old_master_crtc_state);
>  	}
>  
> -	intel_disable_ddi_buf(encoder, old_crtc_state);
> +	intel_disable_ddi_buf(encoder, old_master_crtc_state);
>  
> -	intel_dp_sink_set_fec_ready(intel_dp, old_crtc_state, false);
> +	intel_dp_sink_set_fec_ready(intel_dp, old_master_crtc_state, false);
>  
>  	/*
>  	 * From TGL spec: "If single stream or multi-stream master transcoder:
> @@ -3043,7 +3043,7 @@ static void intel_ddi_post_disable_dp(struct intel_atomic_state *state,
>  	 * transcoder"
>  	 */
>  	if (DISPLAY_VER(dev_priv) >= 12)
> -		intel_ddi_disable_transcoder_clock(old_crtc_state);
> +		intel_ddi_disable_transcoder_clock(old_master_crtc_state);
>  
>  	intel_pps_vdd_on(intel_dp);
>  	intel_pps_off(intel_dp);
> @@ -3065,7 +3065,7 @@ static void intel_ddi_post_disable_dp(struct intel_atomic_state *state,
>  
>  static void intel_ddi_post_disable_hdmi(struct intel_atomic_state *state,
>  					struct intel_encoder *encoder,
> -					const struct intel_crtc_state *old_crtc_state,
> +					const struct intel_crtc_state *old_master_crtc_state,
>  					const struct drm_connector_state *old_conn_state)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> @@ -3074,15 +3074,15 @@ static void intel_ddi_post_disable_hdmi(struct intel_atomic_state *state,
>  	intel_wakeref_t wakeref;
>  
>  	dig_port->set_infoframes(encoder, false,
> -				 old_crtc_state, old_conn_state);
> +				 old_master_crtc_state, old_conn_state);
>  
>  	if (DISPLAY_VER(dev_priv) < 12)
> -		intel_ddi_disable_transcoder_clock(old_crtc_state);
> +		intel_ddi_disable_transcoder_clock(old_master_crtc_state);
>  
> -	intel_disable_ddi_buf(encoder, old_crtc_state);
> +	intel_disable_ddi_buf(encoder, old_master_crtc_state);
>  
>  	if (DISPLAY_VER(dev_priv) >= 12)
> -		intel_ddi_disable_transcoder_clock(old_crtc_state);
> +		intel_ddi_disable_transcoder_clock(old_master_crtc_state);
>  
>  	wakeref = fetch_and_zero(&dig_port->ddi_io_wakeref);
>  	if (wakeref)
> @@ -3097,29 +3097,29 @@ static void intel_ddi_post_disable_hdmi(struct intel_atomic_state *state,
>  
>  static void intel_ddi_post_disable(struct intel_atomic_state *state,
>  				   struct intel_encoder *encoder,
> -				   const struct intel_crtc_state *old_crtc_state,
> +				   const struct intel_crtc_state *old_master_crtc_state,
>  				   const struct drm_connector_state *old_conn_state)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	struct intel_crtc *slave_crtc;
>  
> -	if (!intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_DP_MST)) {
> -		intel_crtc_vblank_off(old_crtc_state);
> +	if (!intel_crtc_has_type(old_master_crtc_state, INTEL_OUTPUT_DP_MST)) {
> +		intel_crtc_vblank_off(old_master_crtc_state);
>  
> -		intel_disable_transcoder(old_crtc_state);
> +		intel_disable_transcoder(old_master_crtc_state);
>  
> -		intel_ddi_disable_transcoder_func(old_crtc_state);
> +		intel_ddi_disable_transcoder_func(old_master_crtc_state);
>  
> -		intel_dsc_disable(old_crtc_state);
> +		intel_dsc_disable(old_master_crtc_state);
>  
>  		if (DISPLAY_VER(dev_priv) >= 9)
> -			skl_scaler_disable(old_crtc_state);
> +			skl_scaler_disable(old_master_crtc_state);
>  		else
> -			ilk_pfit_disable(old_crtc_state);
> +			ilk_pfit_disable(old_master_crtc_state);
>  	}
>  
>  	for_each_intel_crtc_in_pipe_mask(&dev_priv->drm, slave_crtc,
> -					 intel_crtc_bigjoiner_slave_pipes(old_crtc_state)) {
> +					 intel_crtc_bigjoiner_slave_pipes(old_master_crtc_state)) {
>  		const struct intel_crtc_state *old_slave_crtc_state =
>  			intel_atomic_get_old_crtc_state(state, slave_crtc);
>  
> @@ -3142,17 +3142,17 @@ static void intel_ddi_post_disable(struct intel_atomic_state *state,
>  	 *   the DP link parameteres
>  	 */
>  
> -	if (intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_HDMI))
> -		intel_ddi_post_disable_hdmi(state, encoder, old_crtc_state,
> +	if (intel_crtc_has_type(old_master_crtc_state, INTEL_OUTPUT_HDMI))
> +		intel_ddi_post_disable_hdmi(state, encoder, old_master_crtc_state,
>  					    old_conn_state);
>  	else
> -		intel_ddi_post_disable_dp(state, encoder, old_crtc_state,
> +		intel_ddi_post_disable_dp(state, encoder, old_master_crtc_state,
>  					  old_conn_state);
>  }
>  
>  static void intel_ddi_post_pll_disable(struct intel_atomic_state *state,
>  				       struct intel_encoder *encoder,
> -				       const struct intel_crtc_state *old_crtc_state,
> +				       const struct intel_crtc_state *old_master_crtc_state,
>  				       const struct drm_connector_state *old_conn_state)
>  {
>  	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> @@ -3160,7 +3160,7 @@ static void intel_ddi_post_pll_disable(struct intel_atomic_state *state,
>  	enum phy phy = intel_port_to_phy(i915, encoder->port);
>  	bool is_tc_port = intel_phy_is_tc(i915, phy);
>  
> -	main_link_aux_power_domain_put(dig_port, old_crtc_state);
> +	main_link_aux_power_domain_put(dig_port, old_master_crtc_state);
>  
>  	if (is_tc_port)
>  		intel_tc_port_put_link(dig_port);
> @@ -3205,7 +3205,7 @@ static void trans_port_sync_stop_link_train(struct intel_atomic_state *state,
>  
>  static void intel_enable_ddi_dp(struct intel_atomic_state *state,
>  				struct intel_encoder *encoder,
> -				const struct intel_crtc_state *crtc_state,
> +				const struct intel_crtc_state *master_crtc_state,
>  				const struct drm_connector_state *conn_state)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> @@ -3214,15 +3214,15 @@ static void intel_enable_ddi_dp(struct intel_atomic_state *state,
>  	enum port port = encoder->port;
>  
>  	if (port == PORT_A && DISPLAY_VER(dev_priv) < 9)
> -		intel_dp_stop_link_train(intel_dp, crtc_state);
> +		intel_dp_stop_link_train(intel_dp, master_crtc_state);
>  
>  	drm_connector_update_privacy_screen(conn_state);
> -	intel_edp_backlight_on(crtc_state, conn_state);
> +	intel_edp_backlight_on(master_crtc_state, conn_state);
>  
>  	if (!dig_port->lspcon.active || intel_dp_has_hdmi_sink(&dig_port->dp))
> -		intel_dp_set_infoframes(encoder, true, crtc_state, conn_state);
> +		intel_dp_set_infoframes(encoder, true, master_crtc_state, conn_state);
>  
> -	trans_port_sync_stop_link_train(state, encoder, crtc_state);
> +	trans_port_sync_stop_link_train(state, encoder, master_crtc_state);
>  }
>  
>  /* FIXME bad home for this function */
> @@ -3256,7 +3256,7 @@ gen9_chicken_trans_reg_by_port(struct drm_i915_private *dev_priv,
>  
>  static void intel_enable_ddi_hdmi(struct intel_atomic_state *state,
>  				  struct intel_encoder *encoder,
> -				  const struct intel_crtc_state *crtc_state,
> +				  const struct intel_crtc_state *master_crtc_state,
>  				  const struct drm_connector_state *conn_state)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> @@ -3267,20 +3267,20 @@ static void intel_enable_ddi_hdmi(struct intel_atomic_state *state,
>  	u32 buf_ctl;
>  
>  	if (!intel_hdmi_handle_sink_scrambling(encoder, connector,
> -					       crtc_state->hdmi_high_tmds_clock_ratio,
> -					       crtc_state->hdmi_scrambling))
> +					       master_crtc_state->hdmi_high_tmds_clock_ratio,
> +					       master_crtc_state->hdmi_scrambling))
>  		drm_dbg_kms(&dev_priv->drm,
>  			    "[CONNECTOR:%d:%s] Failed to configure sink scrambling/TMDS bit clock ratio\n",
>  			    connector->base.id, connector->name);
>  
>  	if (has_buf_trans_select(dev_priv))
> -		hsw_prepare_hdmi_ddi_buffers(encoder, crtc_state);
> +		hsw_prepare_hdmi_ddi_buffers(encoder, master_crtc_state);
>  
>  	/* e. Enable D2D Link for C10/C20 Phy */
>  	if (DISPLAY_VER(dev_priv) >= 14)
>  		mtl_ddi_enable_d2d(encoder);
>  
> -	encoder->set_signal_levels(encoder, crtc_state);
> +	encoder->set_signal_levels(encoder, master_crtc_state);
>  
>  	/* Display WA #1143: skl,kbl,cfl */
>  	if (DISPLAY_VER(dev_priv) == 9 && !IS_BROXTON(dev_priv)) {
> @@ -3317,7 +3317,7 @@ static void intel_enable_ddi_hdmi(struct intel_atomic_state *state,
>  		intel_de_write(dev_priv, reg, val);
>  	}
>  
> -	intel_ddi_power_up_lanes(encoder, crtc_state);
> +	intel_ddi_power_up_lanes(encoder, master_crtc_state);
>  
>  	/* In HDMI/DVI mode, the port width, and swing/emphasis values
>  	 * are ignored so nothing special needs to be done besides
> @@ -3327,12 +3327,12 @@ static void intel_enable_ddi_hdmi(struct intel_atomic_state *state,
>  	 * these are both 0 for HDMI.
>  	 *
>  	 * But MTL onwards HDMI2.1 is supported and in TMDS mode this
> -	 * is filled with lane count, already set in the crtc_state.
> +	 * is filled with lane count, already set in the master_crtc_state.
>  	 * The same is required to be filled in PORT_BUF_CTL for C10/20 Phy.
>  	 */
>  	buf_ctl = dig_port->saved_port_bits | DDI_BUF_CTL_ENABLE;
>  	if (DISPLAY_VER(dev_priv) >= 14) {
> -		u8  lane_count = mtl_get_port_width(crtc_state->lane_count);
> +		u8  lane_count = mtl_get_port_width(master_crtc_state->lane_count);
>  		u32 port_buf = 0;
>  
>  		port_buf |= XELPDP_PORT_WIDTH(lane_count);
> @@ -3359,35 +3359,35 @@ static void intel_enable_ddi_hdmi(struct intel_atomic_state *state,
>  
>  static void intel_enable_ddi(struct intel_atomic_state *state,
>  			     struct intel_encoder *encoder,
> -			     const struct intel_crtc_state *crtc_state,
> +			     const struct intel_crtc_state *master_crtc_state,
>  			     const struct drm_connector_state *conn_state)
>  {
> -	drm_WARN_ON(state->base.dev, crtc_state->has_pch_encoder);
> +	drm_WARN_ON(state->base.dev, master_crtc_state->has_pch_encoder);
>  
> -	if (!intel_crtc_is_bigjoiner_slave(crtc_state))
> -		intel_ddi_enable_transcoder_func(encoder, crtc_state);
> +	if (!intel_crtc_is_bigjoiner_slave(master_crtc_state))
> +		intel_ddi_enable_transcoder_func(encoder, master_crtc_state);
>  
>  	/* Enable/Disable DP2.0 SDP split config before transcoder */
> -	intel_audio_sdp_split_update(crtc_state);
> +	intel_audio_sdp_split_update(master_crtc_state);
>  
> -	intel_enable_transcoder(crtc_state);
> +	intel_enable_transcoder(master_crtc_state);
>  
> -	intel_ddi_wait_for_fec_status(encoder, crtc_state, true);
> +	intel_ddi_wait_for_fec_status(encoder, master_crtc_state, true);
>  
> -	intel_crtc_vblank_on(crtc_state);
> +	intel_crtc_vblank_on(master_crtc_state);
>  
> -	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
> -		intel_enable_ddi_hdmi(state, encoder, crtc_state, conn_state);
> +	if (intel_crtc_has_type(master_crtc_state, INTEL_OUTPUT_HDMI))
> +		intel_enable_ddi_hdmi(state, encoder, master_crtc_state, conn_state);
>  	else
> -		intel_enable_ddi_dp(state, encoder, crtc_state, conn_state);
> +		intel_enable_ddi_dp(state, encoder, master_crtc_state, conn_state);
>  
> -	intel_hdcp_enable(state, encoder, crtc_state, conn_state);
> +	intel_hdcp_enable(state, encoder, master_crtc_state, conn_state);
>  
>  }
>  
>  static void intel_disable_ddi_dp(struct intel_atomic_state *state,
>  				 struct intel_encoder *encoder,
> -				 const struct intel_crtc_state *old_crtc_state,
> +				 const struct intel_crtc_state *old_master_crtc_state,
>  				 const struct drm_connector_state *old_conn_state)
>  {
>  	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> @@ -3396,19 +3396,19 @@ static void intel_disable_ddi_dp(struct intel_atomic_state *state,
>  
>  	intel_dp->link_trained = false;
>  
> -	intel_psr_disable(intel_dp, old_crtc_state);
> +	intel_psr_disable(intel_dp, old_master_crtc_state);
>  	intel_edp_backlight_off(old_conn_state);
>  	/* Disable the decompression in DP Sink */
>  	intel_dp_sink_disable_decompression(state,
> -					    connector, old_crtc_state);
> +					    connector, old_master_crtc_state);
>  	/* Disable Ignore_MSA bit in DP Sink */
> -	intel_dp_sink_set_msa_timing_par_ignore_state(intel_dp, old_crtc_state,
> +	intel_dp_sink_set_msa_timing_par_ignore_state(intel_dp, old_master_crtc_state,
>  						      false);
>  }
>  
>  static void intel_disable_ddi_hdmi(struct intel_atomic_state *state,
>  				   struct intel_encoder *encoder,
> -				   const struct intel_crtc_state *old_crtc_state,
> +				   const struct intel_crtc_state *old_master_crtc_state,
>  				   const struct drm_connector_state *old_conn_state)
>  {
>  	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> @@ -3423,55 +3423,55 @@ static void intel_disable_ddi_hdmi(struct intel_atomic_state *state,
>  
>  static void intel_disable_ddi(struct intel_atomic_state *state,
>  			      struct intel_encoder *encoder,
> -			      const struct intel_crtc_state *old_crtc_state,
> +			      const struct intel_crtc_state *old_master_crtc_state,
>  			      const struct drm_connector_state *old_conn_state)
>  {
>  	intel_tc_port_link_cancel_reset_work(enc_to_dig_port(encoder));
>  
>  	intel_hdcp_disable(to_intel_connector(old_conn_state->connector));
>  
> -	if (intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_HDMI))
> -		intel_disable_ddi_hdmi(state, encoder, old_crtc_state,
> +	if (intel_crtc_has_type(old_master_crtc_state, INTEL_OUTPUT_HDMI))
> +		intel_disable_ddi_hdmi(state, encoder, old_master_crtc_state,
>  				       old_conn_state);
>  	else
> -		intel_disable_ddi_dp(state, encoder, old_crtc_state,
> +		intel_disable_ddi_dp(state, encoder, old_master_crtc_state,
>  				     old_conn_state);
>  }
>  
>  static void intel_ddi_update_pipe_dp(struct intel_atomic_state *state,
>  				     struct intel_encoder *encoder,
> -				     const struct intel_crtc_state *crtc_state,
> +				     const struct intel_crtc_state *master_crtc_state,
>  				     const struct drm_connector_state *conn_state)
>  {
> -	intel_ddi_set_dp_msa(crtc_state, conn_state);
> +	intel_ddi_set_dp_msa(master_crtc_state, conn_state);
>  
> -	intel_dp_set_infoframes(encoder, true, crtc_state, conn_state);
> +	intel_dp_set_infoframes(encoder, true, master_crtc_state, conn_state);
>  
> -	intel_backlight_update(state, encoder, crtc_state, conn_state);
> +	intel_backlight_update(state, encoder, master_crtc_state, conn_state);
>  	drm_connector_update_privacy_screen(conn_state);
>  }
>  
>  void intel_ddi_update_pipe(struct intel_atomic_state *state,
>  			   struct intel_encoder *encoder,
> -			   const struct intel_crtc_state *crtc_state,
> +			   const struct intel_crtc_state *master_crtc_state,
>  			   const struct drm_connector_state *conn_state)
>  {
>  
> -	if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI) &&
> +	if (!intel_crtc_has_type(master_crtc_state, INTEL_OUTPUT_HDMI) &&
>  	    !intel_encoder_is_mst(encoder))
> -		intel_ddi_update_pipe_dp(state, encoder, crtc_state,
> +		intel_ddi_update_pipe_dp(state, encoder, master_crtc_state,
>  					 conn_state);
>  
> -	intel_hdcp_update_pipe(state, encoder, crtc_state, conn_state);
> +	intel_hdcp_update_pipe(state, encoder, master_crtc_state, conn_state);
>  }
>  
>  void intel_ddi_update_active_dpll(struct intel_atomic_state *state,
>  				  struct intel_encoder *encoder,
> -				  struct intel_crtc *crtc)
> +				  struct intel_crtc *master_crtc)
>  {
>  	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> -	struct intel_crtc_state *crtc_state =
> -		intel_atomic_get_new_crtc_state(state, crtc);
> +	struct intel_crtc_state *master_crtc_state =
> +		intel_atomic_get_new_crtc_state(state, master_crtc);
>  	struct intel_crtc *slave_crtc;
>  	enum phy phy = intel_port_to_phy(i915, encoder->port);
>  
> @@ -3479,16 +3479,16 @@ void intel_ddi_update_active_dpll(struct intel_atomic_state *state,
>  	if (DISPLAY_VER(i915) >= 14 || !intel_phy_is_tc(i915, phy))
>  		return;
>  
> -	intel_update_active_dpll(state, crtc, encoder);
> +	intel_update_active_dpll(state, master_crtc, encoder);
>  	for_each_intel_crtc_in_pipe_mask(&i915->drm, slave_crtc,
> -					 intel_crtc_bigjoiner_slave_pipes(crtc_state))
> +					 intel_crtc_bigjoiner_slave_pipes(master_crtc_state))
>  		intel_update_active_dpll(state, slave_crtc, encoder);
>  }
>  
>  static void
>  intel_ddi_pre_pll_enable(struct intel_atomic_state *state,
>  			 struct intel_encoder *encoder,
> -			 const struct intel_crtc_state *crtc_state,
> +			 const struct intel_crtc_state *master_crtc_state,
>  			 const struct drm_connector_state *conn_state)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> @@ -3498,23 +3498,23 @@ intel_ddi_pre_pll_enable(struct intel_atomic_state *state,
>  
>  	if (is_tc_port) {
>  		struct intel_crtc *master_crtc =
> -			to_intel_crtc(crtc_state->uapi.crtc);
> +			to_intel_crtc(master_crtc_state->uapi.crtc);
>  
> -		intel_tc_port_get_link(dig_port, crtc_state->lane_count);
> +		intel_tc_port_get_link(dig_port, master_crtc_state->lane_count);
>  		intel_ddi_update_active_dpll(state, encoder, master_crtc);
>  	}
>  
> -	main_link_aux_power_domain_get(dig_port, crtc_state);
> +	main_link_aux_power_domain_get(dig_port, master_crtc_state);
>  
>  	if (is_tc_port && !intel_tc_port_in_tbt_alt_mode(dig_port))
>  		/*
>  		 * Program the lane count for static/dynamic connections on
>  		 * Type-C ports.  Skip this step for TBT.
>  		 */
> -		intel_tc_port_set_fia_lane_count(dig_port, crtc_state->lane_count);
> +		intel_tc_port_set_fia_lane_count(dig_port, master_crtc_state->lane_count);
>  	else if (IS_GEMINILAKE(dev_priv) || IS_BROXTON(dev_priv))
>  		bxt_ddi_phy_set_lane_optim_mask(encoder,
> -						crtc_state->lane_lat_optim_mask);
> +						master_crtc_state->lane_lat_optim_mask);
>  }
>  
>  static void adlp_tbt_to_dp_alt_switch_wa(struct intel_encoder *encoder)
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index ab2f52d21bad..f29d38a8035e 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -1634,88 +1634,88 @@ static void hsw_configure_cpu_transcoder(const struct intel_crtc_state *crtc_sta
>  }
>  
>  static void hsw_crtc_enable(struct intel_atomic_state *state,
> -			    struct intel_crtc *crtc)
> +			    struct intel_crtc *master_crtc)
>  {
> -	const struct intel_crtc_state *new_crtc_state =
> -		intel_atomic_get_new_crtc_state(state, crtc);
> -	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> -	enum pipe pipe = crtc->pipe, hsw_workaround_pipe;
> -	enum transcoder cpu_transcoder = new_crtc_state->cpu_transcoder;
> +	const struct intel_crtc_state *new_master_crtc_state =
> +		intel_atomic_get_new_crtc_state(state, master_crtc);
> +	struct drm_i915_private *dev_priv = to_i915(master_crtc->base.dev);
> +	enum transcoder cpu_transcoder = new_master_crtc_state->cpu_transcoder;
> +	enum pipe hsw_workaround_pipe;
>  	bool psl_clkgate_wa;
>  
> -	if (drm_WARN_ON(&dev_priv->drm, crtc->active))
> +	if (drm_WARN_ON(&dev_priv->drm, master_crtc->active))
>  		return;
>  
> -	intel_dmc_enable_pipe(dev_priv, crtc->pipe);
> +	intel_dmc_enable_pipe(dev_priv, master_crtc->pipe);
>  
> -	if (!new_crtc_state->bigjoiner_pipes) {
> -		intel_encoders_pre_pll_enable(state, crtc);
> +	if (!new_master_crtc_state->bigjoiner_pipes) {
> +		intel_encoders_pre_pll_enable(state, master_crtc);
>  
> -		if (new_crtc_state->shared_dpll)
> -			intel_enable_shared_dpll(new_crtc_state);
> +		if (new_master_crtc_state->shared_dpll)
> +			intel_enable_shared_dpll(new_master_crtc_state);
>  
> -		intel_encoders_pre_enable(state, crtc);
> +		intel_encoders_pre_enable(state, master_crtc);
>  	} else {
> -		icl_ddi_bigjoiner_pre_enable(state, new_crtc_state);
> +		icl_ddi_bigjoiner_pre_enable(state, new_master_crtc_state);
>  	}
>  
> -	intel_dsc_enable(new_crtc_state);
> +	intel_dsc_enable(new_master_crtc_state);
>  
>  	if (DISPLAY_VER(dev_priv) >= 13)
> -		intel_uncompressed_joiner_enable(new_crtc_state);
> +		intel_uncompressed_joiner_enable(new_master_crtc_state);
>  
> -	intel_set_pipe_src_size(new_crtc_state);
> +	intel_set_pipe_src_size(new_master_crtc_state);
>  	if (DISPLAY_VER(dev_priv) >= 9 || IS_BROADWELL(dev_priv))
> -		bdw_set_pipe_misc(new_crtc_state);
> +		bdw_set_pipe_misc(new_master_crtc_state);
>  
> -	if (!intel_crtc_is_bigjoiner_slave(new_crtc_state) &&
> +	if (!intel_crtc_is_bigjoiner_slave(new_master_crtc_state) &&
>  	    !transcoder_is_dsi(cpu_transcoder))
> -		hsw_configure_cpu_transcoder(new_crtc_state);
> +		hsw_configure_cpu_transcoder(new_master_crtc_state);
>  
> -	crtc->active = true;
> +	master_crtc->active = true;
>  
>  	/* Display WA #1180: WaDisableScalarClockGating: glk */
>  	psl_clkgate_wa = DISPLAY_VER(dev_priv) == 10 &&
> -		new_crtc_state->pch_pfit.enabled;
> +		new_master_crtc_state->pch_pfit.enabled;
>  	if (psl_clkgate_wa)
> -		glk_pipe_scaler_clock_gating_wa(dev_priv, pipe, true);
> +		glk_pipe_scaler_clock_gating_wa(dev_priv, master_crtc->pipe, true);
>  
>  	if (DISPLAY_VER(dev_priv) >= 9)
> -		skl_pfit_enable(new_crtc_state);
> +		skl_pfit_enable(new_master_crtc_state);
>  	else
> -		ilk_pfit_enable(new_crtc_state);
> +		ilk_pfit_enable(new_master_crtc_state);
>  
>  	/*
>  	 * On ILK+ LUT must be loaded before the pipe is running but with
>  	 * clocks enabled
>  	 */
> -	intel_color_load_luts(new_crtc_state);
> -	intel_color_commit_noarm(new_crtc_state);
> -	intel_color_commit_arm(new_crtc_state);
> +	intel_color_load_luts(new_master_crtc_state);
> +	intel_color_commit_noarm(new_master_crtc_state);
> +	intel_color_commit_arm(new_master_crtc_state);
>  	/* update DSPCNTR to configure gamma/csc for pipe bottom color */
>  	if (DISPLAY_VER(dev_priv) < 9)
> -		intel_disable_primary_plane(new_crtc_state);
> +		intel_disable_primary_plane(new_master_crtc_state);
>  
> -	hsw_set_linetime_wm(new_crtc_state);
> +	hsw_set_linetime_wm(new_master_crtc_state);
>  
>  	if (DISPLAY_VER(dev_priv) >= 11)
> -		icl_set_pipe_chicken(new_crtc_state);
> +		icl_set_pipe_chicken(new_master_crtc_state);
>  
> -	intel_initial_watermarks(state, crtc);
> +	intel_initial_watermarks(state, master_crtc);
>  
> -	if (intel_crtc_is_bigjoiner_slave(new_crtc_state))
> -		intel_crtc_vblank_on(new_crtc_state);
> +	if (intel_crtc_is_bigjoiner_slave(new_master_crtc_state))
> +		intel_crtc_vblank_on(new_master_crtc_state);
>  
> -	intel_encoders_enable(state, crtc);
> +	intel_encoders_enable(state, master_crtc);
>  
>  	if (psl_clkgate_wa) {
> -		intel_crtc_wait_for_next_vblank(crtc);
> -		glk_pipe_scaler_clock_gating_wa(dev_priv, pipe, false);
> +		intel_crtc_wait_for_next_vblank(master_crtc);
> +		glk_pipe_scaler_clock_gating_wa(dev_priv, master_crtc->pipe, false);
>  	}
>  
>  	/* If we change the relative order between pipe/planes enabling, we need
>  	 * to change the workaround. */
> -	hsw_workaround_pipe = new_crtc_state->hsw_workaround_pipe;
> +	hsw_workaround_pipe = new_master_crtc_state->hsw_workaround_pipe;
>  	if (IS_HASWELL(dev_priv) && hsw_workaround_pipe != INVALID_PIPE) {
>  		struct intel_crtc *wa_crtc;
>  
> @@ -1781,32 +1781,32 @@ static void ilk_crtc_disable(struct intel_atomic_state *state,
>  }
>  
>  static void hsw_crtc_disable(struct intel_atomic_state *state,
> -			     struct intel_crtc *crtc)
> +			     struct intel_crtc *master_crtc)
>  {
> -	const struct intel_crtc_state *old_crtc_state =
> -		intel_atomic_get_old_crtc_state(state, crtc);
> -	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
> +	const struct intel_crtc_state *old_master_crtc_state =
> +		intel_atomic_get_old_crtc_state(state, master_crtc);
> +	struct drm_i915_private *i915 = to_i915(master_crtc->base.dev);
>  
>  	/*
>  	 * FIXME collapse everything to one hook.
>  	 * Need care with mst->ddi interactions.
>  	 */
> -	if (!intel_crtc_is_bigjoiner_slave(old_crtc_state)) {
> -		intel_encoders_disable(state, crtc);
> -		intel_encoders_post_disable(state, crtc);
> +	if (!intel_crtc_is_bigjoiner_slave(old_master_crtc_state)) {
> +		intel_encoders_disable(state, master_crtc);
> +		intel_encoders_post_disable(state, master_crtc);
>  	}
>  
> -	intel_disable_shared_dpll(old_crtc_state);
> +	intel_disable_shared_dpll(old_master_crtc_state);
>  
> -	if (!intel_crtc_is_bigjoiner_slave(old_crtc_state)) {
> +	if (!intel_crtc_is_bigjoiner_slave(old_master_crtc_state)) {
>  		struct intel_crtc *slave_crtc;
>  
> -		intel_encoders_post_pll_disable(state, crtc);
> +		intel_encoders_post_pll_disable(state, master_crtc);
>  
> -		intel_dmc_disable_pipe(i915, crtc->pipe);
> +		intel_dmc_disable_pipe(i915, master_crtc->pipe);
>  
>  		for_each_intel_crtc_in_pipe_mask(&i915->drm, slave_crtc,
> -						 intel_crtc_bigjoiner_slave_pipes(old_crtc_state))
> +						 intel_crtc_bigjoiner_slave_pipes(old_master_crtc_state))
>  			intel_dmc_disable_pipe(i915, slave_crtc->pipe);
>  	}
>  }
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index db1254b036f1..e4e71e4ae0e1 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -917,7 +917,7 @@ static void wait_for_act_sent(struct intel_encoder *encoder,
>  
>  static void intel_mst_disable_dp(struct intel_atomic_state *state,
>  				 struct intel_encoder *encoder,
> -				 const struct intel_crtc_state *old_crtc_state,
> +				 const struct intel_crtc_state *old_master_crtc_state,
>  				 const struct drm_connector_state *old_conn_state)
>  {
>  	struct intel_dp_mst_encoder *intel_mst = enc_to_mst(encoder);
> @@ -932,12 +932,12 @@ static void intel_mst_disable_dp(struct intel_atomic_state *state,
>  
>  	intel_hdcp_disable(intel_mst->connector);
>  
> -	intel_dp_sink_disable_decompression(state, connector, old_crtc_state);
> +	intel_dp_sink_disable_decompression(state, connector, old_master_crtc_state);
>  }
>  
>  static void intel_mst_post_disable_dp(struct intel_atomic_state *state,
>  				      struct intel_encoder *encoder,
> -				      const struct intel_crtc_state *old_crtc_state,
> +				      const struct intel_crtc_state *old_master_crtc_state,
>  				      const struct drm_connector_state *old_conn_state)
>  {
>  	struct intel_dp_mst_encoder *intel_mst = enc_to_mst(encoder);
> @@ -960,32 +960,32 @@ static void intel_mst_post_disable_dp(struct intel_atomic_state *state,
>  	last_mst_stream = intel_dp->active_mst_links == 0;
>  	drm_WARN_ON(&dev_priv->drm,
>  		    DISPLAY_VER(dev_priv) >= 12 && last_mst_stream &&
> -		    !intel_dp_mst_is_master_trans(old_crtc_state));
> +		    !intel_dp_mst_is_master_trans(old_master_crtc_state));
>  
> -	intel_crtc_vblank_off(old_crtc_state);
> +	intel_crtc_vblank_off(old_master_crtc_state);
>  
> -	intel_disable_transcoder(old_crtc_state);
> +	intel_disable_transcoder(old_master_crtc_state);
>  
>  	drm_dp_remove_payload_part1(&intel_dp->mst_mgr, new_mst_state, new_payload);
>  
> -	clear_act_sent(encoder, old_crtc_state);
> +	clear_act_sent(encoder, old_master_crtc_state);
>  
> -	intel_de_rmw(dev_priv, TRANS_DDI_FUNC_CTL(old_crtc_state->cpu_transcoder),
> +	intel_de_rmw(dev_priv, TRANS_DDI_FUNC_CTL(old_master_crtc_state->cpu_transcoder),
>  		     TRANS_DDI_DP_VC_PAYLOAD_ALLOC, 0);
>  
> -	wait_for_act_sent(encoder, old_crtc_state);
> +	wait_for_act_sent(encoder, old_master_crtc_state);
>  
>  	drm_dp_remove_payload_part2(&intel_dp->mst_mgr, new_mst_state,
>  				    old_payload, new_payload);
>  
> -	intel_ddi_disable_transcoder_func(old_crtc_state);
> +	intel_ddi_disable_transcoder_func(old_master_crtc_state);
>  
> -	intel_dsc_disable(old_crtc_state);
> +	intel_dsc_disable(old_master_crtc_state);
>  
>  	if (DISPLAY_VER(dev_priv) >= 9)
> -		skl_scaler_disable(old_crtc_state);
> +		skl_scaler_disable(old_master_crtc_state);
>  	else
> -		ilk_pfit_disable(old_crtc_state);
> +		ilk_pfit_disable(old_master_crtc_state);
>  
>  	/*
>  	 * Power down mst path before disabling the port, otherwise we end
> @@ -999,7 +999,7 @@ static void intel_mst_post_disable_dp(struct intel_atomic_state *state,
>  	 * the transcoder clock select is set to none.
>  	 */
>  	intel_dp_set_infoframes(&dig_port->base, false,
> -				old_crtc_state, NULL);
> +				old_master_crtc_state, NULL);
>  	/*
>  	 * From TGL spec: "If multi-stream slave transcoder: Configure
>  	 * Transcoder Clock Select to direct no clock to the transcoder"
> @@ -1008,13 +1008,12 @@ static void intel_mst_post_disable_dp(struct intel_atomic_state *state,
>  	 * no clock to the transcoder"
>  	 */
>  	if (DISPLAY_VER(dev_priv) < 12 || !last_mst_stream)
> -		intel_ddi_disable_transcoder_clock(old_crtc_state);
> -
> +		intel_ddi_disable_transcoder_clock(old_master_crtc_state);
>  
>  	intel_mst->connector = NULL;
>  	if (last_mst_stream)
>  		dig_port->base.post_disable(state, &dig_port->base,
> -						  old_crtc_state, NULL);
> +						  old_master_crtc_state, NULL);
>  
>  	drm_dbg_kms(&dev_priv->drm, "active links %d\n",
>  		    intel_dp->active_mst_links);
> @@ -1022,7 +1021,7 @@ static void intel_mst_post_disable_dp(struct intel_atomic_state *state,
>  
>  static void intel_mst_post_pll_disable_dp(struct intel_atomic_state *state,
>  					  struct intel_encoder *encoder,
> -					  const struct intel_crtc_state *old_crtc_state,
> +					  const struct intel_crtc_state *old_master_crtc_state,
>  					  const struct drm_connector_state *old_conn_state)
>  {
>  	struct intel_dp_mst_encoder *intel_mst = enc_to_mst(encoder);
> @@ -1031,33 +1030,33 @@ static void intel_mst_post_pll_disable_dp(struct intel_atomic_state *state,
>  
>  	if (intel_dp->active_mst_links == 0 &&
>  	    dig_port->base.post_pll_disable)
> -		dig_port->base.post_pll_disable(state, encoder, old_crtc_state, old_conn_state);
> +		dig_port->base.post_pll_disable(state, encoder, old_master_crtc_state, old_conn_state);
>  }
>  
>  static void intel_mst_pre_pll_enable_dp(struct intel_atomic_state *state,
>  					struct intel_encoder *encoder,
> -					const struct intel_crtc_state *pipe_config,
> +					const struct intel_crtc_state *master_crtc_state,
>  					const struct drm_connector_state *conn_state)
>  {
> +	struct intel_crtc *master_crtc = to_intel_crtc(master_crtc_state->uapi.crtc);
>  	struct intel_dp_mst_encoder *intel_mst = enc_to_mst(encoder);
>  	struct intel_digital_port *dig_port = intel_mst->primary;
>  	struct intel_dp *intel_dp = &dig_port->dp;
>  
>  	if (intel_dp->active_mst_links == 0)
>  		dig_port->base.pre_pll_enable(state, &dig_port->base,
> -						    pipe_config, NULL);
> +					      master_crtc_state, NULL);
>  	else
>  		/*
>  		 * The port PLL state needs to get updated for secondary
>  		 * streams as for the primary stream.
>  		 */
> -		intel_ddi_update_active_dpll(state, &dig_port->base,
> -					     to_intel_crtc(pipe_config->uapi.crtc));
> +		intel_ddi_update_active_dpll(state, &dig_port->base, master_crtc);
>  }
>  
>  static void intel_mst_pre_enable_dp(struct intel_atomic_state *state,
>  				    struct intel_encoder *encoder,
> -				    const struct intel_crtc_state *pipe_config,
> +				    const struct intel_crtc_state *master_crtc_state,
>  				    const struct drm_connector_state *conn_state)
>  {
>  	struct intel_dp_mst_encoder *intel_mst = enc_to_mst(encoder);
> @@ -1079,7 +1078,7 @@ static void intel_mst_pre_enable_dp(struct intel_atomic_state *state,
>  	first_mst_stream = intel_dp->active_mst_links == 0;
>  	drm_WARN_ON(&dev_priv->drm,
>  		    DISPLAY_VER(dev_priv) >= 12 && first_mst_stream &&
> -		    !intel_dp_mst_is_master_trans(pipe_config));
> +		    !intel_dp_mst_is_master_trans(master_crtc_state));
>  
>  	drm_dbg_kms(&dev_priv->drm, "active links %d\n",
>  		    intel_dp->active_mst_links);
> @@ -1089,11 +1088,11 @@ static void intel_mst_pre_enable_dp(struct intel_atomic_state *state,
>  
>  	drm_dp_send_power_updown_phy(&intel_dp->mst_mgr, connector->port, true);
>  
> -	intel_dp_sink_enable_decompression(state, connector, pipe_config);
> +	intel_dp_sink_enable_decompression(state, connector, master_crtc_state);
>  
>  	if (first_mst_stream)
>  		dig_port->base.pre_enable(state, &dig_port->base,
> -						pipe_config, NULL);
> +						master_crtc_state, NULL);
>  
>  	intel_dp->active_mst_links++;
>  
> @@ -1111,15 +1110,15 @@ static void intel_mst_pre_enable_dp(struct intel_atomic_state *state,
>  	 * here for the following ones.
>  	 */
>  	if (DISPLAY_VER(dev_priv) < 12 || !first_mst_stream)
> -		intel_ddi_enable_transcoder_clock(encoder, pipe_config);
> +		intel_ddi_enable_transcoder_clock(encoder, master_crtc_state);
>  
> -	intel_dsc_dp_pps_write(&dig_port->base, pipe_config);
> -	intel_ddi_set_dp_msa(pipe_config, conn_state);
> +	intel_dsc_dp_pps_write(&dig_port->base, master_crtc_state);
> +	intel_ddi_set_dp_msa(master_crtc_state, conn_state);
>  }
>  
>  static void intel_mst_enable_dp(struct intel_atomic_state *state,
>  				struct intel_encoder *encoder,
> -				const struct intel_crtc_state *pipe_config,
> +				const struct intel_crtc_state *master_crtc_state,
>  				const struct drm_connector_state *conn_state)
>  {
>  	struct intel_dp_mst_encoder *intel_mst = enc_to_mst(encoder);
> @@ -1129,25 +1128,25 @@ static void intel_mst_enable_dp(struct intel_atomic_state *state,
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	struct drm_dp_mst_topology_state *mst_state =
>  		drm_atomic_get_new_mst_topology_state(&state->base, &intel_dp->mst_mgr);
> -	enum transcoder trans = pipe_config->cpu_transcoder;
> +	enum transcoder trans = master_crtc_state->cpu_transcoder;
>  	bool first_mst_stream = intel_dp->active_mst_links == 1;
>  
> -	drm_WARN_ON(&dev_priv->drm, pipe_config->has_pch_encoder);
> +	drm_WARN_ON(&dev_priv->drm, master_crtc_state->has_pch_encoder);
>  
> -	if (intel_dp_is_uhbr(pipe_config)) {
> +	if (intel_dp_is_uhbr(master_crtc_state)) {
>  		const struct drm_display_mode *adjusted_mode =
> -			&pipe_config->hw.adjusted_mode;
> +			&master_crtc_state->hw.adjusted_mode;
>  		u64 crtc_clock_hz = KHz(adjusted_mode->crtc_clock);
>  
> -		intel_de_write(dev_priv, TRANS_DP2_VFREQHIGH(pipe_config->cpu_transcoder),
> +		intel_de_write(dev_priv, TRANS_DP2_VFREQHIGH(master_crtc_state->cpu_transcoder),
>  			       TRANS_DP2_VFREQ_PIXEL_CLOCK(crtc_clock_hz >> 24));
> -		intel_de_write(dev_priv, TRANS_DP2_VFREQLOW(pipe_config->cpu_transcoder),
> +		intel_de_write(dev_priv, TRANS_DP2_VFREQLOW(master_crtc_state->cpu_transcoder),
>  			       TRANS_DP2_VFREQ_PIXEL_CLOCK(crtc_clock_hz & 0xffffff));
>  	}
>  
> -	intel_ddi_enable_transcoder_func(encoder, pipe_config);
> +	intel_ddi_enable_transcoder_func(encoder, master_crtc_state);
>  
> -	clear_act_sent(encoder, pipe_config);
> +	clear_act_sent(encoder, master_crtc_state);
>  
>  	intel_de_rmw(dev_priv, TRANS_DDI_FUNC_CTL(trans), 0,
>  		     TRANS_DDI_DP_VC_PAYLOAD_ALLOC);
> @@ -1155,10 +1154,10 @@ static void intel_mst_enable_dp(struct intel_atomic_state *state,
>  	drm_dbg_kms(&dev_priv->drm, "active links %d\n",
>  		    intel_dp->active_mst_links);
>  
> -	wait_for_act_sent(encoder, pipe_config);
> +	wait_for_act_sent(encoder, master_crtc_state);
>  
>  	if (first_mst_stream)
> -		intel_ddi_wait_for_fec_status(encoder, pipe_config, true);
> +		intel_ddi_wait_for_fec_status(encoder, master_crtc_state, true);
>  
>  	drm_dp_add_payload_part2(&intel_dp->mst_mgr, &state->base,
>  				 drm_atomic_get_mst_payload_state(mst_state, connector->port));
> @@ -1166,15 +1165,15 @@ static void intel_mst_enable_dp(struct intel_atomic_state *state,
>  	if (DISPLAY_VER(dev_priv) >= 12)
>  		intel_de_rmw(dev_priv, hsw_chicken_trans_reg(dev_priv, trans),
>  			     FECSTALL_DIS_DPTSTREAM_DPTTG,
> -			     pipe_config->fec_enable ? FECSTALL_DIS_DPTSTREAM_DPTTG : 0);
> +			     master_crtc_state->fec_enable ? FECSTALL_DIS_DPTSTREAM_DPTTG : 0);
>  
> -	intel_audio_sdp_split_update(pipe_config);
> +	intel_audio_sdp_split_update(master_crtc_state);
>  
> -	intel_enable_transcoder(pipe_config);
> +	intel_enable_transcoder(master_crtc_state);
>  
> -	intel_crtc_vblank_on(pipe_config);
> +	intel_crtc_vblank_on(master_crtc_state);
>  
> -	intel_hdcp_enable(state, encoder, pipe_config, conn_state);
> +	intel_hdcp_enable(state, encoder, master_crtc_state, conn_state);
>  }
>  
>  static bool intel_dp_mst_enc_get_hw_state(struct intel_encoder *encoder,
> -- 
> 2.43.0
> 

Re: [PATCH 1/8] drm/i915: Rename the crtc/crtc_states in the top level DDI hooks/etc

[Ville Syrjälä]

On Tue, Mar 05, 2024 at 10:41:49AM +0200, Lisovskiy, Stanislav wrote:
> On Fri, Mar 01, 2024 at 04:35:53PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > In preparation for doing a more sensible pipe vs. transcoder
> > handling for bigjoiner let's rename the crtc/crtc_state in the
> > top level crtc_enable/disable and the DDI encoder hooks to
> > include "master" in the name. This way they won't collide with
> > the per-pipe stuff.
> > 
> > Note that at this point this is (at least partially) telling
> > lies as we still run through some of these for slave pipes as
> > well. But I wanted to get the huge rename out of the way so
> > it won't clutter the functional patches so much.
> > 
> > TODO: or perhaps use some other names for the per-pipe stuff instead?
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> I will then review now the patches which you could merge before the bigjoiner
> stuff could be finished.

I just sent a separate series with the disable_pipes bitmask
stuff.

> Checked this patch I guess, you were also talking that this renaming might
> be not the best idea.
> I also wonder whether should we really emphasize things like "master"/"slave"
> in function names. I thought that one idea in our refactoring was to unify
> joined pipes handling so that there are no(or at least almost no) explicit code
> paths/function names for masters/slaves.

There are no master vs. slave functions. The split is going to be
transcoder/port vs. pipe.

-- 
Ville Syrjälä
Intel

Re: [PATCH 1/8] drm/i915: Rename the crtc/crtc_states in the top level DDI hooks/etc

[Lisovskiy, Stanislav]

On Tue, Mar 05, 2024 at 10:50:01AM +0200, Ville Syrjälä wrote:
> On Tue, Mar 05, 2024 at 10:41:49AM +0200, Lisovskiy, Stanislav wrote:
> > On Fri, Mar 01, 2024 at 04:35:53PM +0200, Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > In preparation for doing a more sensible pipe vs. transcoder
> > > handling for bigjoiner let's rename the crtc/crtc_state in the
> > > top level crtc_enable/disable and the DDI encoder hooks to
> > > include "master" in the name. This way they won't collide with
> > > the per-pipe stuff.
> > > 
> > > Note that at this point this is (at least partially) telling
> > > lies as we still run through some of these for slave pipes as
> > > well. But I wanted to get the huge rename out of the way so
> > > it won't clutter the functional patches so much.
> > > 
> > > TODO: or perhaps use some other names for the per-pipe stuff instead?
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > I will then review now the patches which you could merge before the bigjoiner
> > stuff could be finished.
> 
> I just sent a separate series with the disable_pipes bitmask
> stuff.

I already reviewed all the patches, including that one, if there were
no changes, I guess you can apply that r-b there as well.

> 
> > Checked this patch I guess, you were also talking that this renaming might
> > be not the best idea.
> > I also wonder whether should we really emphasize things like "master"/"slave"
> > in function names. I thought that one idea in our refactoring was to unify
> > joined pipes handling so that there are no(or at least almost no) explicit code
> > paths/function names for masters/slaves.
> 
> There are no master vs. slave functions. The split is going to be
> transcoder/port vs. pipe.

In practice thats what you want to achieve, the functions which also include encoder
programming and/or handling joined pipes you wanted to add master in the name.

I think we should try to mention master/slave explicitly as less as possible.

Stan

> 
> -- 
> Ville Syrjälä
> Intel

Re: [PATCH 1/8] drm/i915: Rename the crtc/crtc_states in the top level DDI hooks/etc

[Ville Syrjälä]

On Tue, Mar 05, 2024 at 11:08:56AM +0200, Lisovskiy, Stanislav wrote:
> On Tue, Mar 05, 2024 at 10:50:01AM +0200, Ville Syrjälä wrote:
> > On Tue, Mar 05, 2024 at 10:41:49AM +0200, Lisovskiy, Stanislav wrote:
> > > On Fri, Mar 01, 2024 at 04:35:53PM +0200, Ville Syrjala wrote:
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > 
> > > > In preparation for doing a more sensible pipe vs. transcoder
> > > > handling for bigjoiner let's rename the crtc/crtc_state in the
> > > > top level crtc_enable/disable and the DDI encoder hooks to
> > > > include "master" in the name. This way they won't collide with
> > > > the per-pipe stuff.
> > > > 
> > > > Note that at this point this is (at least partially) telling
> > > > lies as we still run through some of these for slave pipes as
> > > > well. But I wanted to get the huge rename out of the way so
> > > > it won't clutter the functional patches so much.
> > > > 
> > > > TODO: or perhaps use some other names for the per-pipe stuff instead?
> > > > 
> > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > I will then review now the patches which you could merge before the bigjoiner
> > > stuff could be finished.
> > 
> > I just sent a separate series with the disable_pipes bitmask
> > stuff.
> 
> I already reviewed all the patches, including that one, if there were
> no changes, I guess you can apply that r-b there as well.

Sure. Thanks.

> 
> > 
> > > Checked this patch I guess, you were also talking that this renaming might
> > > be not the best idea.
> > > I also wonder whether should we really emphasize things like "master"/"slave"
> > > in function names. I thought that one idea in our refactoring was to unify
> > > joined pipes handling so that there are no(or at least almost no) explicit code
> > > paths/function names for masters/slaves.
> > 
> > There are no master vs. slave functions. The split is going to be
> > transcoder/port vs. pipe.
> 
> In practice thats what you want to achieve, the functions which also include encoder
> programming and/or handling joined pipes you wanted to add master in the name.

I wanted clarity which crtc state is for which purpose. But I think we
achieve that by naming the per-pipe variables a bit differently instead
(eg. pipe_crtc + pipe_crtc_state).

-- 
Ville Syrjälä
Intel

Re: [PATCH 1/8] drm/i915: Rename the crtc/crtc_states in the top level DDI hooks/etc

[Jani Nikula]

On Tue, 05 Mar 2024, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Tue, Mar 05, 2024 at 10:41:49AM +0200, Lisovskiy, Stanislav wrote:
>> I also wonder whether should we really emphasize things like "master"/"slave"
>> in function names. I thought that one idea in our refactoring was to unify
>> joined pipes handling so that there are no(or at least almost no) explicit code
>> paths/function names for masters/slaves.
>
> There are no master vs. slave functions. The split is going to be
> transcoder/port vs. pipe.

Besides, for modern platforms the spec has already been changed to use
primary/secondary terminology. When renaming or refactoring stuff,
please switch to them instead of sticking with master/slave.

BR,
Jani.

-- 
Jani Nikula, Intel

Re: [PATCH 1/8] drm/i915: Rename the crtc/crtc_states in the top level DDI hooks/etc

[Ville Syrjälä]

On Tue, Mar 05, 2024 at 04:07:45PM +0200, Jani Nikula wrote:
> On Tue, 05 Mar 2024, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > On Tue, Mar 05, 2024 at 10:41:49AM +0200, Lisovskiy, Stanislav wrote:
> >> I also wonder whether should we really emphasize things like "master"/"slave"
> >> in function names. I thought that one idea in our refactoring was to unify
> >> joined pipes handling so that there are no(or at least almost no) explicit code
> >> paths/function names for masters/slaves.
> >
> > There are no master vs. slave functions. The split is going to be
> > transcoder/port vs. pipe.
> 
> Besides, for modern platforms the spec has already been changed to use
> primary/secondary terminology. When renaming or refactoring stuff,
> please switch to them instead of sticking with master/slave.

If the spec got updated then we should probably just do a full rename
pass over the whole codebase instead of confusing things more by
mixing up the terminology.

Also we should probably s/bigjoiner/joiner/ to make it clear that
all of it also applies to uncompressed joiner as well.

-- 
Ville Syrjälä
Intel

Re: [PATCH 1/8] drm/i915: Rename the crtc/crtc_states in the top level DDI hooks/etc

[Jani Nikula]

On Tue, 05 Mar 2024, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> If the spec got updated then we should probably just do a full rename
> pass over the whole codebase instead of confusing things more by
> mixing up the terminology.

For example:

Bspec 54142 for display version 12+ has some mixed old/new terminology.

Bspec 68847 for xe2lpd+ only has new terminology.

> Also we should probably s/bigjoiner/joiner/ to make it clear that
> all of it also applies to uncompressed joiner as well.

Ack.

BR,
Jani.

-- 
Jani Nikula, Intel

[PATCH 2/8] drm/i915: Introduce intel_crtc_joined_pipe_mask()

[Ville Syrjala]

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

Add a small helper to compute the set of pipes that the current
crtc is using.

And we have at least one trivial place in
intel_ddi_update_active_dpll() where we can use it
immediately, so let's do that.

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

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 6287629f9e77..0e5834f8af6e 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -3472,17 +3472,16 @@ void intel_ddi_update_active_dpll(struct intel_atomic_state *state,
 	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
 	struct intel_crtc_state *master_crtc_state =
 		intel_atomic_get_new_crtc_state(state, master_crtc);
-	struct intel_crtc *slave_crtc;
+	u8 pipe_mask = intel_crtc_joined_pipe_mask(master_crtc_state);
 	enum phy phy = intel_port_to_phy(i915, encoder->port);
+	struct intel_crtc *crtc;
 
 	/* FIXME: Add MTL pll_mgr */
 	if (DISPLAY_VER(i915) >= 14 || !intel_phy_is_tc(i915, phy))
 		return;
 
-	intel_update_active_dpll(state, master_crtc, encoder);
-	for_each_intel_crtc_in_pipe_mask(&i915->drm, slave_crtc,
-					 intel_crtc_bigjoiner_slave_pipes(master_crtc_state))
-		intel_update_active_dpll(state, slave_crtc, encoder);
+	for_each_intel_crtc_in_pipe_mask(&i915->drm, crtc, pipe_mask)
+		intel_update_active_dpll(state, crtc, encoder);
 }
 
 static void
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index f29d38a8035e..8cc5de31c1dd 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -275,6 +275,13 @@ static int intel_bigjoiner_num_pipes(const struct intel_crtc_state *crtc_state)
 	return hweight8(crtc_state->bigjoiner_pipes);
 }
 
+u8 intel_crtc_joined_pipe_mask(const struct intel_crtc_state *crtc_state)
+{
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+
+	return BIT(crtc->pipe) | crtc_state->bigjoiner_pipes;
+}
+
 struct intel_crtc *intel_master_crtc(const struct intel_crtc_state *crtc_state)
 {
 	struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
index f4a0773f0fca..6df84098ff8c 100644
--- a/drivers/gpu/drm/i915/display/intel_display.h
+++ b/drivers/gpu/drm/i915/display/intel_display.h
@@ -408,6 +408,7 @@ intel_cpu_transcoder_mode_valid(struct drm_i915_private *i915,
 enum phy intel_port_to_phy(struct drm_i915_private *i915, enum port port);
 bool is_trans_port_sync_mode(const struct intel_crtc_state *state);
 bool is_trans_port_sync_master(const struct intel_crtc_state *state);
+u8 intel_crtc_joined_pipe_mask(const struct intel_crtc_state *crtc_state);
 bool intel_crtc_is_bigjoiner_slave(const struct intel_crtc_state *crtc_state);
 bool intel_crtc_is_bigjoiner_master(const struct intel_crtc_state *crtc_state);
 u8 intel_crtc_bigjoiner_slave_pipes(const struct intel_crtc_state *crtc_state);
-- 
2.43.0

[PATCH 3/8] drm/i915: Extract intel_ddi_post_disable_hdmi_or_sst()

[Ville Syrjala]

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

Extract the "not-MST" stuff from intel_ddi_post_disable() so that
the whole thing isn't so cluttered.

The bigjoiner slave handling was outside of the !MST check,
but it really should have been inside it as its the counterpart
to the master handling inside the check. So we pull that
in as well. There is no functional change here as we don't
currently support bigjoiner+MST anyway.

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

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 0e5834f8af6e..f3f8ecf1a87e 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -3095,28 +3095,26 @@ static void intel_ddi_post_disable_hdmi(struct intel_atomic_state *state,
 	intel_dp_dual_mode_set_tmds_output(intel_hdmi, false);
 }
 
-static void intel_ddi_post_disable(struct intel_atomic_state *state,
-				   struct intel_encoder *encoder,
-				   const struct intel_crtc_state *old_master_crtc_state,
-				   const struct drm_connector_state *old_conn_state)
+static void intel_ddi_post_disable_hdmi_or_sst(struct intel_atomic_state *state,
+					       struct intel_encoder *encoder,
+					       const struct intel_crtc_state *old_master_crtc_state,
+					       const struct drm_connector_state *old_conn_state)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	struct intel_crtc *slave_crtc;
 
-	if (!intel_crtc_has_type(old_master_crtc_state, INTEL_OUTPUT_DP_MST)) {
-		intel_crtc_vblank_off(old_master_crtc_state);
+	intel_crtc_vblank_off(old_master_crtc_state);
 
-		intel_disable_transcoder(old_master_crtc_state);
+	intel_disable_transcoder(old_master_crtc_state);
 
-		intel_ddi_disable_transcoder_func(old_master_crtc_state);
+	intel_ddi_disable_transcoder_func(old_master_crtc_state);
 
-		intel_dsc_disable(old_master_crtc_state);
+	intel_dsc_disable(old_master_crtc_state);
 
-		if (DISPLAY_VER(dev_priv) >= 9)
-			skl_scaler_disable(old_master_crtc_state);
-		else
-			ilk_pfit_disable(old_master_crtc_state);
-	}
+	if (DISPLAY_VER(dev_priv) >= 9)
+		skl_scaler_disable(old_master_crtc_state);
+	else
+		ilk_pfit_disable(old_master_crtc_state);
 
 	for_each_intel_crtc_in_pipe_mask(&dev_priv->drm, slave_crtc,
 					 intel_crtc_bigjoiner_slave_pipes(old_master_crtc_state)) {
@@ -3128,6 +3126,16 @@ static void intel_ddi_post_disable(struct intel_atomic_state *state,
 		intel_dsc_disable(old_slave_crtc_state);
 		skl_scaler_disable(old_slave_crtc_state);
 	}
+}
+
+static void intel_ddi_post_disable(struct intel_atomic_state *state,
+				   struct intel_encoder *encoder,
+				   const struct intel_crtc_state *old_master_crtc_state,
+				   const struct drm_connector_state *old_conn_state)
+{
+	if (!intel_crtc_has_type(old_master_crtc_state, INTEL_OUTPUT_DP_MST))
+		intel_ddi_post_disable_hdmi_or_sst(state, encoder,
+						   old_master_crtc_state, old_conn_state);
 
 	/*
 	 * When called from DP MST code:
-- 
2.43.0

Re: [PATCH 3/8] drm/i915: Extract intel_ddi_post_disable_hdmi_or_sst()

[Lisovskiy, Stanislav]

On Fri, Mar 01, 2024 at 04:35:55PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Extract the "not-MST" stuff from intel_ddi_post_disable() so that
> the whole thing isn't so cluttered.
> 
> The bigjoiner slave handling was outside of the !MST check,
> but it really should have been inside it as its the counterpart
> to the master handling inside the check. So we pull that
> in as well. There is no functional change here as we don't
> currently support bigjoiner+MST anyway.

Had same idea. Was really wondering why we handle it that way here.
But I guess for MST case, we need to add similar handling then?

Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>

> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c | 36 +++++++++++++++---------
>  1 file changed, 22 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 0e5834f8af6e..f3f8ecf1a87e 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -3095,28 +3095,26 @@ static void intel_ddi_post_disable_hdmi(struct intel_atomic_state *state,
>  	intel_dp_dual_mode_set_tmds_output(intel_hdmi, false);
>  }
>  
> -static void intel_ddi_post_disable(struct intel_atomic_state *state,
> -				   struct intel_encoder *encoder,
> -				   const struct intel_crtc_state *old_master_crtc_state,
> -				   const struct drm_connector_state *old_conn_state)
> +static void intel_ddi_post_disable_hdmi_or_sst(struct intel_atomic_state *state,
> +					       struct intel_encoder *encoder,
> +					       const struct intel_crtc_state *old_master_crtc_state,
> +					       const struct drm_connector_state *old_conn_state)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	struct intel_crtc *slave_crtc;
>  
> -	if (!intel_crtc_has_type(old_master_crtc_state, INTEL_OUTPUT_DP_MST)) {
> -		intel_crtc_vblank_off(old_master_crtc_state);
> +	intel_crtc_vblank_off(old_master_crtc_state);
>  
> -		intel_disable_transcoder(old_master_crtc_state);
> +	intel_disable_transcoder(old_master_crtc_state);
>  
> -		intel_ddi_disable_transcoder_func(old_master_crtc_state);
> +	intel_ddi_disable_transcoder_func(old_master_crtc_state);
>  
> -		intel_dsc_disable(old_master_crtc_state);
> +	intel_dsc_disable(old_master_crtc_state);
>  
> -		if (DISPLAY_VER(dev_priv) >= 9)
> -			skl_scaler_disable(old_master_crtc_state);
> -		else
> -			ilk_pfit_disable(old_master_crtc_state);
> -	}
> +	if (DISPLAY_VER(dev_priv) >= 9)
> +		skl_scaler_disable(old_master_crtc_state);
> +	else
> +		ilk_pfit_disable(old_master_crtc_state);
>  
>  	for_each_intel_crtc_in_pipe_mask(&dev_priv->drm, slave_crtc,
>  					 intel_crtc_bigjoiner_slave_pipes(old_master_crtc_state)) {
> @@ -3128,6 +3126,16 @@ static void intel_ddi_post_disable(struct intel_atomic_state *state,
>  		intel_dsc_disable(old_slave_crtc_state);
>  		skl_scaler_disable(old_slave_crtc_state);
>  	}
> +}
> +
> +static void intel_ddi_post_disable(struct intel_atomic_state *state,
> +				   struct intel_encoder *encoder,
> +				   const struct intel_crtc_state *old_master_crtc_state,
> +				   const struct drm_connector_state *old_conn_state)
> +{
> +	if (!intel_crtc_has_type(old_master_crtc_state, INTEL_OUTPUT_DP_MST))
> +		intel_ddi_post_disable_hdmi_or_sst(state, encoder,
> +						   old_master_crtc_state, old_conn_state);
>  
>  	/*
>  	 * When called from DP MST code:
> -- 
> 2.43.0
> 

[PATCH 4/8] drm/i915: Utilize intel_crtc_joined_pipe_mask() more

[Ville Syrjala]

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

Unify the master vs. slave handling in
intel_ddi_post_disable_hdmi_or_sst() by looping over all the
pipes in one go.

This also lets us move the intel_crtc_vblank_off() calls to
happen in a consistent place vs. the transcoder disable.
Previously we did the master vs. slaves on different sides
of that.

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

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index f3f8ecf1a87e..1e06d7599c2a 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -3101,30 +3101,30 @@ static void intel_ddi_post_disable_hdmi_or_sst(struct intel_atomic_state *state,
 					       const struct drm_connector_state *old_conn_state)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-	struct intel_crtc *slave_crtc;
+	u8 pipe_mask = intel_crtc_joined_pipe_mask(old_master_crtc_state);
+	struct intel_crtc *crtc;
 
-	intel_crtc_vblank_off(old_master_crtc_state);
+	for_each_intel_crtc_in_pipe_mask(&dev_priv->drm, crtc, pipe_mask) {
+		const struct intel_crtc_state *old_crtc_state =
+			intel_atomic_get_old_crtc_state(state, crtc);
+
+		intel_crtc_vblank_off(old_crtc_state);
+	}
 
 	intel_disable_transcoder(old_master_crtc_state);
 
 	intel_ddi_disable_transcoder_func(old_master_crtc_state);
 
-	intel_dsc_disable(old_master_crtc_state);
+	for_each_intel_crtc_in_pipe_mask(&dev_priv->drm, crtc, pipe_mask) {
+		const struct intel_crtc_state *old_crtc_state =
+			intel_atomic_get_old_crtc_state(state, crtc);
 
-	if (DISPLAY_VER(dev_priv) >= 9)
-		skl_scaler_disable(old_master_crtc_state);
-	else
-		ilk_pfit_disable(old_master_crtc_state);
+		intel_dsc_disable(old_crtc_state);
 
-	for_each_intel_crtc_in_pipe_mask(&dev_priv->drm, slave_crtc,
-					 intel_crtc_bigjoiner_slave_pipes(old_master_crtc_state)) {
-		const struct intel_crtc_state *old_slave_crtc_state =
-			intel_atomic_get_old_crtc_state(state, slave_crtc);
-
-		intel_crtc_vblank_off(old_slave_crtc_state);
-
-		intel_dsc_disable(old_slave_crtc_state);
-		skl_scaler_disable(old_slave_crtc_state);
+		if (DISPLAY_VER(dev_priv) >= 9)
+			skl_scaler_disable(old_crtc_state);
+		else
+			ilk_pfit_disable(old_crtc_state);
 	}
 }
 
-- 
2.43.0

[PATCH 5/8] drm/i915: Precompute disable_pipes bitmask in intel_commit_modeset_disables()

[Ville Syrjala]

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

Copy the pipe bitmask based approach skl_commit_modeset_enables()
into intel_commit_modeset_disables(). This avoids doing so many
duplicated checks in all the loops, and also let's us WARN at the
end if we screwed up somewhere and forgot to disable some pipe.

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

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 8cc5de31c1dd..2351ee52d16e 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -6775,9 +6775,10 @@ static void intel_old_crtc_state_disables(struct intel_atomic_state *state,
 
 static void intel_commit_modeset_disables(struct intel_atomic_state *state)
 {
+	struct drm_i915_private *i915 = to_i915(state->base.dev);
 	struct intel_crtc_state *new_crtc_state, *old_crtc_state;
 	struct intel_crtc *crtc;
-	u32 handled = 0;
+	u8 disable_pipes = 0;
 	int i;
 
 	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
@@ -6785,21 +6786,24 @@ static void intel_commit_modeset_disables(struct intel_atomic_state *state)
 		if (!intel_crtc_needs_modeset(new_crtc_state))
 			continue;
 
+		/*
+		 * Needs to be done even for pipes
+		 * that weren't enabled previously.
+		 */
 		intel_pre_plane_update(state, crtc);
 
 		if (!old_crtc_state->hw.active)
 			continue;
 
 		intel_crtc_disable_planes(state, crtc);
+
+		disable_pipes |= BIT(crtc->pipe);
 	}
 
 	/* Only disable port sync and MST slaves */
 	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
 					    new_crtc_state, i) {
-		if (!intel_crtc_needs_modeset(new_crtc_state))
-			continue;
-
-		if (!old_crtc_state->hw.active)
+		if ((disable_pipes & BIT(crtc->pipe)) == 0)
 			continue;
 
 		/* In case of Transcoder port Sync master slave CRTCs can be
@@ -6814,22 +6818,23 @@ static void intel_commit_modeset_disables(struct intel_atomic_state *state)
 
 		intel_old_crtc_state_disables(state, old_crtc_state,
 					      new_crtc_state, crtc);
-		handled |= BIT(crtc->pipe);
+
+		disable_pipes &= ~BIT(crtc->pipe);
 	}
 
 	/* Disable everything else left on */
 	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
 					    new_crtc_state, i) {
-		if (!intel_crtc_needs_modeset(new_crtc_state) ||
-		    (handled & BIT(crtc->pipe)))
-			continue;
-
-		if (!old_crtc_state->hw.active)
+		if ((disable_pipes & BIT(crtc->pipe)) == 0)
 			continue;
 
 		intel_old_crtc_state_disables(state, old_crtc_state,
 					      new_crtc_state, crtc);
+
+		disable_pipes &= ~BIT(crtc->pipe);
 	}
+
+	drm_WARN_ON(&i915->drm, disable_pipes);
 }
 
 static void intel_commit_modeset_enables(struct intel_atomic_state *state)
-- 
2.43.0

Re: [PATCH 5/8] drm/i915: Precompute disable_pipes bitmask in intel_commit_modeset_disables()

[Lisovskiy, Stanislav]

On Fri, Mar 01, 2024 at 04:35:57PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Copy the pipe bitmask based approach skl_commit_modeset_enables()
> into intel_commit_modeset_disables(). This avoids doing so many
> duplicated checks in all the loops, and also let's us WARN at the
> end if we screwed up somewhere and forgot to disable some pipe.

Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>

> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 27 ++++++++++++--------
>  1 file changed, 16 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 8cc5de31c1dd..2351ee52d16e 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -6775,9 +6775,10 @@ static void intel_old_crtc_state_disables(struct intel_atomic_state *state,
>  
>  static void intel_commit_modeset_disables(struct intel_atomic_state *state)
>  {
> +	struct drm_i915_private *i915 = to_i915(state->base.dev);
>  	struct intel_crtc_state *new_crtc_state, *old_crtc_state;
>  	struct intel_crtc *crtc;
> -	u32 handled = 0;
> +	u8 disable_pipes = 0;
>  	int i;
>  
>  	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
> @@ -6785,21 +6786,24 @@ static void intel_commit_modeset_disables(struct intel_atomic_state *state)
>  		if (!intel_crtc_needs_modeset(new_crtc_state))
>  			continue;
>  
> +		/*
> +		 * Needs to be done even for pipes
> +		 * that weren't enabled previously.
> +		 */
>  		intel_pre_plane_update(state, crtc);
>  
>  		if (!old_crtc_state->hw.active)
>  			continue;
>  
>  		intel_crtc_disable_planes(state, crtc);
> +
> +		disable_pipes |= BIT(crtc->pipe);
>  	}
>  
>  	/* Only disable port sync and MST slaves */
>  	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
>  					    new_crtc_state, i) {
> -		if (!intel_crtc_needs_modeset(new_crtc_state))
> -			continue;
> -
> -		if (!old_crtc_state->hw.active)
> +		if ((disable_pipes & BIT(crtc->pipe)) == 0)
>  			continue;
>  
>  		/* In case of Transcoder port Sync master slave CRTCs can be
> @@ -6814,22 +6818,23 @@ static void intel_commit_modeset_disables(struct intel_atomic_state *state)
>  
>  		intel_old_crtc_state_disables(state, old_crtc_state,
>  					      new_crtc_state, crtc);
> -		handled |= BIT(crtc->pipe);
> +
> +		disable_pipes &= ~BIT(crtc->pipe);
>  	}
>  
>  	/* Disable everything else left on */
>  	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
>  					    new_crtc_state, i) {
> -		if (!intel_crtc_needs_modeset(new_crtc_state) ||
> -		    (handled & BIT(crtc->pipe)))
> -			continue;
> -
> -		if (!old_crtc_state->hw.active)
> +		if ((disable_pipes & BIT(crtc->pipe)) == 0)
>  			continue;
>  
>  		intel_old_crtc_state_disables(state, old_crtc_state,
>  					      new_crtc_state, crtc);
> +
> +		disable_pipes &= ~BIT(crtc->pipe);
>  	}
> +
> +	drm_WARN_ON(&i915->drm, disable_pipes);
>  }
>  
>  static void intel_commit_modeset_enables(struct intel_atomic_state *state)
> -- 
> 2.43.0
> 

[PATCH 6/8] drm/i915: Disable planes more atomically during modesets

[Ville Syrjala]

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

Follow in the footsteps of commit c610e841f19d ("drm/i915:
Do plane/etc. updates more atomically across pipes") and
do the plane disables back to back for all pipes also when
we are disabling pipes.

This should provide for a potentially more atomic user
experience, which might be especially nice when using
joiner or tiled displays.

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

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 2351ee52d16e..01d7e91cb1bc 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -6795,11 +6795,16 @@ static void intel_commit_modeset_disables(struct intel_atomic_state *state)
 		if (!old_crtc_state->hw.active)
 			continue;
 
-		intel_crtc_disable_planes(state, crtc);
-
 		disable_pipes |= BIT(crtc->pipe);
 	}
 
+	for_each_old_intel_crtc_in_state(state, crtc, old_crtc_state, i) {
+		if ((disable_pipes & BIT(crtc->pipe)) == 0)
+			continue;
+
+		intel_crtc_disable_planes(state, crtc);
+	}
+
 	/* Only disable port sync and MST slaves */
 	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
 					    new_crtc_state, i) {
-- 
2.43.0

Re: [PATCH 6/8] drm/i915: Disable planes more atomically during modesets

[Lisovskiy, Stanislav]

On Fri, Mar 01, 2024 at 04:35:58PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Follow in the footsteps of commit c610e841f19d ("drm/i915:
> Do plane/etc. updates more atomically across pipes") and
> do the plane disables back to back for all pipes also when
> we are disabling pipes.
> 
> This should provide for a potentially more atomic user
> experience, which might be especially nice when using
> joiner or tiled displays.

Okay, so the difference is that previously we did call 
intel_pre_plane_update/intel_crtc_disable_planes per crtc, but
now we first call first intel_pre_plane_update for each crtc
and then call intel_crtc_disable_planes per crtc.

So in cases when there are some cross pipe dependencies, that might be
more proper way. Just to clarify that I understood this right.

Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>

> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 2351ee52d16e..01d7e91cb1bc 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -6795,11 +6795,16 @@ static void intel_commit_modeset_disables(struct intel_atomic_state *state)
>  		if (!old_crtc_state->hw.active)
>  			continue;
>  
> -		intel_crtc_disable_planes(state, crtc);
> -
>  		disable_pipes |= BIT(crtc->pipe);
>  	}
>  
> +	for_each_old_intel_crtc_in_state(state, crtc, old_crtc_state, i) {
> +		if ((disable_pipes & BIT(crtc->pipe)) == 0)
> +			continue;
> +
> +		intel_crtc_disable_planes(state, crtc);
> +	}
> +
>  	/* Only disable port sync and MST slaves */
>  	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
>  					    new_crtc_state, i) {
> -- 
> 2.43.0
> 

[PATCH 7/8] drm/i915: Simplify intel_old_crtc_state_disables() calling convention

[Ville Syrjala]

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

Stop passing in so much redundant stuff to
intel_old_crtc_state_disables(). Top level atomic state + crtc
is all we need.

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

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 01d7e91cb1bc..1df3923cc30d 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -6753,11 +6753,11 @@ static void intel_update_crtc(struct intel_atomic_state *state,
 }
 
 static void intel_old_crtc_state_disables(struct intel_atomic_state *state,
-					  struct intel_crtc_state *old_crtc_state,
-					  struct intel_crtc_state *new_crtc_state,
 					  struct intel_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
+	const struct intel_crtc_state *new_crtc_state =
+		intel_atomic_get_new_crtc_state(state, crtc);
 
 	/*
 	 * We need to disable pipe CRC before disabling the pipe,
@@ -6776,7 +6776,7 @@ static void intel_old_crtc_state_disables(struct intel_atomic_state *state,
 static void intel_commit_modeset_disables(struct intel_atomic_state *state)
 {
 	struct drm_i915_private *i915 = to_i915(state->base.dev);
-	struct intel_crtc_state *new_crtc_state, *old_crtc_state;
+	const struct intel_crtc_state *new_crtc_state, *old_crtc_state;
 	struct intel_crtc *crtc;
 	u8 disable_pipes = 0;
 	int i;
@@ -6806,8 +6806,7 @@ static void intel_commit_modeset_disables(struct intel_atomic_state *state)
 	}
 
 	/* Only disable port sync and MST slaves */
-	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
-					    new_crtc_state, i) {
+	for_each_old_intel_crtc_in_state(state, crtc, old_crtc_state, i) {
 		if ((disable_pipes & BIT(crtc->pipe)) == 0)
 			continue;
 
@@ -6821,20 +6820,17 @@ static void intel_commit_modeset_disables(struct intel_atomic_state *state)
 		    !intel_crtc_is_bigjoiner_slave(old_crtc_state))
 			continue;
 
-		intel_old_crtc_state_disables(state, old_crtc_state,
-					      new_crtc_state, crtc);
+		intel_old_crtc_state_disables(state, crtc);
 
 		disable_pipes &= ~BIT(crtc->pipe);
 	}
 
 	/* Disable everything else left on */
-	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
-					    new_crtc_state, i) {
+	for_each_old_intel_crtc_in_state(state, crtc, old_crtc_state, i) {
 		if ((disable_pipes & BIT(crtc->pipe)) == 0)
 			continue;
 
-		intel_old_crtc_state_disables(state, old_crtc_state,
-					      new_crtc_state, crtc);
+		intel_old_crtc_state_disables(state, crtc);
 
 		disable_pipes &= ~BIT(crtc->pipe);
 	}
-- 
2.43.0

Re: [PATCH 7/8] drm/i915: Simplify intel_old_crtc_state_disables() calling convention

[Lisovskiy, Stanislav]

On Fri, Mar 01, 2024 at 04:35:59PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Stop passing in so much redundant stuff to
> intel_old_crtc_state_disables(). Top level atomic state + crtc
> is all we need.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 18 +++++++-----------
>  1 file changed, 7 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 01d7e91cb1bc..1df3923cc30d 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -6753,11 +6753,11 @@ static void intel_update_crtc(struct intel_atomic_state *state,
>  }
>  
>  static void intel_old_crtc_state_disables(struct intel_atomic_state *state,
> -					  struct intel_crtc_state *old_crtc_state,
> -					  struct intel_crtc_state *new_crtc_state,
>  					  struct intel_crtc *crtc)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> +	const struct intel_crtc_state *new_crtc_state =
> +		intel_atomic_get_new_crtc_state(state, crtc);
>  
>  	/*
>  	 * We need to disable pipe CRC before disabling the pipe,
> @@ -6776,7 +6776,7 @@ static void intel_old_crtc_state_disables(struct intel_atomic_state *state,
>  static void intel_commit_modeset_disables(struct intel_atomic_state *state)
>  {
>  	struct drm_i915_private *i915 = to_i915(state->base.dev);
> -	struct intel_crtc_state *new_crtc_state, *old_crtc_state;
> +	const struct intel_crtc_state *new_crtc_state, *old_crtc_state;
>  	struct intel_crtc *crtc;
>  	u8 disable_pipes = 0;
>  	int i;
> @@ -6806,8 +6806,7 @@ static void intel_commit_modeset_disables(struct intel_atomic_state *state)
>  	}
>  
>  	/* Only disable port sync and MST slaves */
> -	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
> -					    new_crtc_state, i) {
> +	for_each_old_intel_crtc_in_state(state, crtc, old_crtc_state, i) {
>  		if ((disable_pipes & BIT(crtc->pipe)) == 0)
>  			continue;
>  
> @@ -6821,20 +6820,17 @@ static void intel_commit_modeset_disables(struct intel_atomic_state *state)
>  		    !intel_crtc_is_bigjoiner_slave(old_crtc_state))
>  			continue;
>  
> -		intel_old_crtc_state_disables(state, old_crtc_state,
> -					      new_crtc_state, crtc);
> +		intel_old_crtc_state_disables(state, crtc);
>  
>  		disable_pipes &= ~BIT(crtc->pipe);
>  	}
>  
>  	/* Disable everything else left on */
> -	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
> -					    new_crtc_state, i) {
> +	for_each_old_intel_crtc_in_state(state, crtc, old_crtc_state, i) {
>  		if ((disable_pipes & BIT(crtc->pipe)) == 0)
>  			continue;
>  
> -		intel_old_crtc_state_disables(state, old_crtc_state,
> -					      new_crtc_state, crtc);
> +		intel_old_crtc_state_disables(state, crtc);
>  
>  		disable_pipes &= ~BIT(crtc->pipe);
>  	}
> -- 
> 2.43.0
> 

[PATCH 8/8] drm/i915: Handle joined pipes inside hsw_crtc_disable()

[Ville Syrjala]

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

Reorganize the crtc disable path to only deal with the
master pipes/transcoders in intel_old_crtc_state_disables()
and offload the handling of joined pipes to hsw_crtc_disable().
This makes the whole thing much more sensible since we can
actually control the order in which we do the per-pipe vs.
per-transcoder modeset steps.

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

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 1df3923cc30d..07239c1ce9df 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -1793,29 +1793,27 @@ static void hsw_crtc_disable(struct intel_atomic_state *state,
 	const struct intel_crtc_state *old_master_crtc_state =
 		intel_atomic_get_old_crtc_state(state, master_crtc);
 	struct drm_i915_private *i915 = to_i915(master_crtc->base.dev);
+	u8 pipe_mask = intel_crtc_joined_pipe_mask(old_master_crtc_state);
+	struct intel_crtc *crtc;
 
 	/*
 	 * FIXME collapse everything to one hook.
 	 * Need care with mst->ddi interactions.
 	 */
-	if (!intel_crtc_is_bigjoiner_slave(old_master_crtc_state)) {
-		intel_encoders_disable(state, master_crtc);
-		intel_encoders_post_disable(state, master_crtc);
-	}
-
-	intel_disable_shared_dpll(old_master_crtc_state);
+	intel_encoders_disable(state, master_crtc);
+	intel_encoders_post_disable(state, master_crtc);
 
-	if (!intel_crtc_is_bigjoiner_slave(old_master_crtc_state)) {
-		struct intel_crtc *slave_crtc;
+	for_each_intel_crtc_in_pipe_mask(&i915->drm, crtc, pipe_mask) {
+		const struct intel_crtc_state *old_crtc_state =
+			intel_atomic_get_old_crtc_state(state, crtc);
 
-		intel_encoders_post_pll_disable(state, master_crtc);
+		intel_disable_shared_dpll(old_crtc_state);
+	}
 
-		intel_dmc_disable_pipe(i915, master_crtc->pipe);
+	intel_encoders_post_pll_disable(state, master_crtc);
 
-		for_each_intel_crtc_in_pipe_mask(&i915->drm, slave_crtc,
-						 intel_crtc_bigjoiner_slave_pipes(old_master_crtc_state))
-			intel_dmc_disable_pipe(i915, slave_crtc->pipe);
-	}
+	for_each_intel_crtc_in_pipe_mask(&i915->drm, crtc, pipe_mask)
+		intel_dmc_disable_pipe(i915, crtc->pipe);
 }
 
 static void i9xx_pfit_enable(const struct intel_crtc_state *crtc_state)
@@ -6753,24 +6751,33 @@ static void intel_update_crtc(struct intel_atomic_state *state,
 }
 
 static void intel_old_crtc_state_disables(struct intel_atomic_state *state,
-					  struct intel_crtc *crtc)
+					  struct intel_crtc *master_crtc)
 {
 	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
-	const struct intel_crtc_state *new_crtc_state =
-		intel_atomic_get_new_crtc_state(state, crtc);
+	const struct intel_crtc_state *old_master_crtc_state =
+		intel_atomic_get_old_crtc_state(state, master_crtc);
+	u8 pipe_mask = intel_crtc_joined_pipe_mask(old_master_crtc_state);
+	struct intel_crtc *crtc;
 
 	/*
 	 * We need to disable pipe CRC before disabling the pipe,
 	 * or we race against vblank off.
 	 */
-	intel_crtc_disable_pipe_crc(crtc);
+	for_each_intel_crtc_in_pipe_mask(&dev_priv->drm, crtc, pipe_mask)
+		intel_crtc_disable_pipe_crc(crtc);
 
 	dev_priv->display.funcs.display->crtc_disable(state, crtc);
-	crtc->active = false;
-	intel_fbc_disable(crtc);
 
-	if (!new_crtc_state->hw.active)
-		intel_initial_watermarks(state, crtc);
+	for_each_intel_crtc_in_pipe_mask(&dev_priv->drm, crtc, pipe_mask) {
+		const struct intel_crtc_state *new_crtc_state =
+			intel_atomic_get_new_crtc_state(state, crtc);
+
+		crtc->active = false;
+		intel_fbc_disable(crtc);
+
+		if (!new_crtc_state->hw.active)
+			intel_initial_watermarks(state, crtc);
+	}
 }
 
 static void intel_commit_modeset_disables(struct intel_atomic_state *state)
@@ -6810,19 +6817,21 @@ static void intel_commit_modeset_disables(struct intel_atomic_state *state)
 		if ((disable_pipes & BIT(crtc->pipe)) == 0)
 			continue;
 
+		if (intel_crtc_is_bigjoiner_slave(old_crtc_state))
+			continue;
+
 		/* In case of Transcoder port Sync master slave CRTCs can be
 		 * assigned in any order and we need to make sure that
 		 * slave CRTCs are disabled first and then master CRTC since
 		 * Slave vblanks are masked till Master Vblanks.
 		 */
 		if (!is_trans_port_sync_slave(old_crtc_state) &&
-		    !intel_dp_mst_is_slave_trans(old_crtc_state) &&
-		    !intel_crtc_is_bigjoiner_slave(old_crtc_state))
+		    !intel_dp_mst_is_slave_trans(old_crtc_state))
 			continue;
 
 		intel_old_crtc_state_disables(state, crtc);
 
-		disable_pipes &= ~BIT(crtc->pipe);
+		disable_pipes &= ~intel_crtc_joined_pipe_mask(old_crtc_state);
 	}
 
 	/* Disable everything else left on */
@@ -6830,9 +6839,12 @@ static void intel_commit_modeset_disables(struct intel_atomic_state *state)
 		if ((disable_pipes & BIT(crtc->pipe)) == 0)
 			continue;
 
+		if (intel_crtc_is_bigjoiner_slave(old_crtc_state))
+			continue;
+
 		intel_old_crtc_state_disables(state, crtc);
 
-		disable_pipes &= ~BIT(crtc->pipe);
+		disable_pipes &= ~intel_crtc_joined_pipe_mask(old_crtc_state);
 	}
 
 	drm_WARN_ON(&i915->drm, disable_pipes);
-- 
2.43.0

Re: [PATCH 8/8] drm/i915: Handle joined pipes inside hsw_crtc_disable()

[Lisovskiy, Stanislav]

On Fri, Mar 01, 2024 at 04:36:00PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Reorganize the crtc disable path to only deal with the
> master pipes/transcoders in intel_old_crtc_state_disables()
> and offload the handling of joined pipes to hsw_crtc_disable().
> This makes the whole thing much more sensible since we can
> actually control the order in which we do the per-pipe vs.
> per-transcoder modeset steps.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 64 ++++++++++++--------
>  1 file changed, 38 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 1df3923cc30d..07239c1ce9df 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -1793,29 +1793,27 @@ static void hsw_crtc_disable(struct intel_atomic_state *state,
>  	const struct intel_crtc_state *old_master_crtc_state =
>  		intel_atomic_get_old_crtc_state(state, master_crtc);
>  	struct drm_i915_private *i915 = to_i915(master_crtc->base.dev);
> +	u8 pipe_mask = intel_crtc_joined_pipe_mask(old_master_crtc_state);
> +	struct intel_crtc *crtc;
>  
>  	/*
>  	 * FIXME collapse everything to one hook.
>  	 * Need care with mst->ddi interactions.
>  	 */
> -	if (!intel_crtc_is_bigjoiner_slave(old_master_crtc_state)) {
> -		intel_encoders_disable(state, master_crtc);
> -		intel_encoders_post_disable(state, master_crtc);
> -	}
> -
> -	intel_disable_shared_dpll(old_master_crtc_state);
> +	intel_encoders_disable(state, master_crtc);
> +	intel_encoders_post_disable(state, master_crtc);
>  
> -	if (!intel_crtc_is_bigjoiner_slave(old_master_crtc_state)) {
> -		struct intel_crtc *slave_crtc;
> +	for_each_intel_crtc_in_pipe_mask(&i915->drm, crtc, pipe_mask) {
> +		const struct intel_crtc_state *old_crtc_state =
> +			intel_atomic_get_old_crtc_state(state, crtc);
>  
> -		intel_encoders_post_pll_disable(state, master_crtc);
> +		intel_disable_shared_dpll(old_crtc_state);
> +	}
>  
> -		intel_dmc_disable_pipe(i915, master_crtc->pipe);
> +	intel_encoders_post_pll_disable(state, master_crtc);
>  
> -		for_each_intel_crtc_in_pipe_mask(&i915->drm, slave_crtc,
> -						 intel_crtc_bigjoiner_slave_pipes(old_master_crtc_state))
> -			intel_dmc_disable_pipe(i915, slave_crtc->pipe);
> -	}
> +	for_each_intel_crtc_in_pipe_mask(&i915->drm, crtc, pipe_mask)
> +		intel_dmc_disable_pipe(i915, crtc->pipe);
>  }

Okay the only difference from hsw_crtc_disable part from my patch is that
I don't have intel_crtc_joined_pipe_mask and encoder calls are outside the pipe
loop. Ok. You could of course just communicate this to me, it is quite a small
thing to change.

And still there is a question about how to handle the crtc enable side, since
extracting transcoder programming from the pipe loop, will break the sequence,
as I described. Either it is ok that we will partly program slave/master pipe, then
program transcoder then again program slave/master pipes or it has to be
in a pipe loop.

Stan

>  
>  static void i9xx_pfit_enable(const struct intel_crtc_state *crtc_state)
> @@ -6753,24 +6751,33 @@ static void intel_update_crtc(struct intel_atomic_state *state,
>  }
>  
>  static void intel_old_crtc_state_disables(struct intel_atomic_state *state,
> -					  struct intel_crtc *crtc)
> +					  struct intel_crtc *master_crtc)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> -	const struct intel_crtc_state *new_crtc_state =
> -		intel_atomic_get_new_crtc_state(state, crtc);
> +	const struct intel_crtc_state *old_master_crtc_state =
> +		intel_atomic_get_old_crtc_state(state, master_crtc);
> +	u8 pipe_mask = intel_crtc_joined_pipe_mask(old_master_crtc_state);
> +	struct intel_crtc *crtc;
>  
>  	/*
>  	 * We need to disable pipe CRC before disabling the pipe,
>  	 * or we race against vblank off.
>  	 */
> -	intel_crtc_disable_pipe_crc(crtc);
> +	for_each_intel_crtc_in_pipe_mask(&dev_priv->drm, crtc, pipe_mask)
> +		intel_crtc_disable_pipe_crc(crtc);
>  
>  	dev_priv->display.funcs.display->crtc_disable(state, crtc);
> -	crtc->active = false;
> -	intel_fbc_disable(crtc);
>  
> -	if (!new_crtc_state->hw.active)
> -		intel_initial_watermarks(state, crtc);
> +	for_each_intel_crtc_in_pipe_mask(&dev_priv->drm, crtc, pipe_mask) {
> +		const struct intel_crtc_state *new_crtc_state =
> +			intel_atomic_get_new_crtc_state(state, crtc);
> +
> +		crtc->active = false;
> +		intel_fbc_disable(crtc);
> +
> +		if (!new_crtc_state->hw.active)
> +			intel_initial_watermarks(state, crtc);
> +	}
>  }
>  
>  static void intel_commit_modeset_disables(struct intel_atomic_state *state)
> @@ -6810,19 +6817,21 @@ static void intel_commit_modeset_disables(struct intel_atomic_state *state)
>  		if ((disable_pipes & BIT(crtc->pipe)) == 0)
>  			continue;
>  
> +		if (intel_crtc_is_bigjoiner_slave(old_crtc_state))
> +			continue;
> +
>  		/* In case of Transcoder port Sync master slave CRTCs can be
>  		 * assigned in any order and we need to make sure that
>  		 * slave CRTCs are disabled first and then master CRTC since
>  		 * Slave vblanks are masked till Master Vblanks.
>  		 */
>  		if (!is_trans_port_sync_slave(old_crtc_state) &&
> -		    !intel_dp_mst_is_slave_trans(old_crtc_state) &&
> -		    !intel_crtc_is_bigjoiner_slave(old_crtc_state))
> +		    !intel_dp_mst_is_slave_trans(old_crtc_state))
>  			continue;
>  
>  		intel_old_crtc_state_disables(state, crtc);
>  
> -		disable_pipes &= ~BIT(crtc->pipe);
> +		disable_pipes &= ~intel_crtc_joined_pipe_mask(old_crtc_state);
>  	}
>  
>  	/* Disable everything else left on */
> @@ -6830,9 +6839,12 @@ static void intel_commit_modeset_disables(struct intel_atomic_state *state)
>  		if ((disable_pipes & BIT(crtc->pipe)) == 0)
>  			continue;
>  
> +		if (intel_crtc_is_bigjoiner_slave(old_crtc_state))
> +			continue;
> +
>  		intel_old_crtc_state_disables(state, crtc);
>  
> -		disable_pipes &= ~BIT(crtc->pipe);
> +		disable_pipes &= ~intel_crtc_joined_pipe_mask(old_crtc_state);
>  	}
>  
>  	drm_WARN_ON(&i915->drm, disable_pipes);
> -- 
> 2.43.0
> 

Re: [PATCH 8/8] drm/i915: Handle joined pipes inside hsw_crtc_disable()

[Ville Syrjälä]

On Fri, Mar 01, 2024 at 06:04:27PM +0200, Lisovskiy, Stanislav wrote:
> On Fri, Mar 01, 2024 at 04:36:00PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Reorganize the crtc disable path to only deal with the
> > master pipes/transcoders in intel_old_crtc_state_disables()
> > and offload the handling of joined pipes to hsw_crtc_disable().
> > This makes the whole thing much more sensible since we can
> > actually control the order in which we do the per-pipe vs.
> > per-transcoder modeset steps.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_display.c | 64 ++++++++++++--------
> >  1 file changed, 38 insertions(+), 26 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index 1df3923cc30d..07239c1ce9df 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -1793,29 +1793,27 @@ static void hsw_crtc_disable(struct intel_atomic_state *state,
> >  	const struct intel_crtc_state *old_master_crtc_state =
> >  		intel_atomic_get_old_crtc_state(state, master_crtc);
> >  	struct drm_i915_private *i915 = to_i915(master_crtc->base.dev);
> > +	u8 pipe_mask = intel_crtc_joined_pipe_mask(old_master_crtc_state);
> > +	struct intel_crtc *crtc;
> >  
> >  	/*
> >  	 * FIXME collapse everything to one hook.
> >  	 * Need care with mst->ddi interactions.
> >  	 */
> > -	if (!intel_crtc_is_bigjoiner_slave(old_master_crtc_state)) {
> > -		intel_encoders_disable(state, master_crtc);
> > -		intel_encoders_post_disable(state, master_crtc);
> > -	}
> > -
> > -	intel_disable_shared_dpll(old_master_crtc_state);
> > +	intel_encoders_disable(state, master_crtc);
> > +	intel_encoders_post_disable(state, master_crtc);
> >  
> > -	if (!intel_crtc_is_bigjoiner_slave(old_master_crtc_state)) {
> > -		struct intel_crtc *slave_crtc;
> > +	for_each_intel_crtc_in_pipe_mask(&i915->drm, crtc, pipe_mask) {
> > +		const struct intel_crtc_state *old_crtc_state =
> > +			intel_atomic_get_old_crtc_state(state, crtc);
> >  
> > -		intel_encoders_post_pll_disable(state, master_crtc);
> > +		intel_disable_shared_dpll(old_crtc_state);
> > +	}
> >  
> > -		intel_dmc_disable_pipe(i915, master_crtc->pipe);
> > +	intel_encoders_post_pll_disable(state, master_crtc);
> >  
> > -		for_each_intel_crtc_in_pipe_mask(&i915->drm, slave_crtc,
> > -						 intel_crtc_bigjoiner_slave_pipes(old_master_crtc_state))
> > -			intel_dmc_disable_pipe(i915, slave_crtc->pipe);
> > -	}
> > +	for_each_intel_crtc_in_pipe_mask(&i915->drm, crtc, pipe_mask)
> > +		intel_dmc_disable_pipe(i915, crtc->pipe);
> >  }
> 
> Okay the only difference from hsw_crtc_disable part from my patch is that
> I don't have intel_crtc_joined_pipe_mask and encoder calls are outside the pipe
> loop. Ok. You could of course just communicate this to me, it is quite a small
> thing to change.
> 
> And still there is a question about how to handle the crtc enable side, since
> extracting transcoder programming from the pipe loop, will break the sequence,
> as I described. Either it is ok that we will partly program slave/master pipe, then
> program transcoder then again program slave/master pipes or it has to be
> in a pipe loop.

Transcoder stuff shouldn't be in pipe loops. That's what
I've been saying all along.

-- 
Ville Syrjälä
Intel

Re: [PATCH 8/8] drm/i915: Handle joined pipes inside hsw_crtc_disable()

[Lisovskiy, Stanislav]

On Fri, Mar 01, 2024 at 06:10:25PM +0200, Ville Syrjälä wrote:
> On Fri, Mar 01, 2024 at 06:04:27PM +0200, Lisovskiy, Stanislav wrote:
> > On Fri, Mar 01, 2024 at 04:36:00PM +0200, Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Reorganize the crtc disable path to only deal with the
> > > master pipes/transcoders in intel_old_crtc_state_disables()
> > > and offload the handling of joined pipes to hsw_crtc_disable().
> > > This makes the whole thing much more sensible since we can
> > > actually control the order in which we do the per-pipe vs.
> > > per-transcoder modeset steps.
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_display.c | 64 ++++++++++++--------
> > >  1 file changed, 38 insertions(+), 26 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > > index 1df3923cc30d..07239c1ce9df 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > @@ -1793,29 +1793,27 @@ static void hsw_crtc_disable(struct intel_atomic_state *state,
> > >  	const struct intel_crtc_state *old_master_crtc_state =
> > >  		intel_atomic_get_old_crtc_state(state, master_crtc);
> > >  	struct drm_i915_private *i915 = to_i915(master_crtc->base.dev);
> > > +	u8 pipe_mask = intel_crtc_joined_pipe_mask(old_master_crtc_state);
> > > +	struct intel_crtc *crtc;
> > >  
> > >  	/*
> > >  	 * FIXME collapse everything to one hook.
> > >  	 * Need care with mst->ddi interactions.
> > >  	 */
> > > -	if (!intel_crtc_is_bigjoiner_slave(old_master_crtc_state)) {
> > > -		intel_encoders_disable(state, master_crtc);
> > > -		intel_encoders_post_disable(state, master_crtc);
> > > -	}
> > > -
> > > -	intel_disable_shared_dpll(old_master_crtc_state);
> > > +	intel_encoders_disable(state, master_crtc);
> > > +	intel_encoders_post_disable(state, master_crtc);
> > >  
> > > -	if (!intel_crtc_is_bigjoiner_slave(old_master_crtc_state)) {
> > > -		struct intel_crtc *slave_crtc;
> > > +	for_each_intel_crtc_in_pipe_mask(&i915->drm, crtc, pipe_mask) {
> > > +		const struct intel_crtc_state *old_crtc_state =
> > > +			intel_atomic_get_old_crtc_state(state, crtc);
> > >  
> > > -		intel_encoders_post_pll_disable(state, master_crtc);
> > > +		intel_disable_shared_dpll(old_crtc_state);
> > > +	}
> > >  
> > > -		intel_dmc_disable_pipe(i915, master_crtc->pipe);
> > > +	intel_encoders_post_pll_disable(state, master_crtc);
> > >  
> > > -		for_each_intel_crtc_in_pipe_mask(&i915->drm, slave_crtc,
> > > -						 intel_crtc_bigjoiner_slave_pipes(old_master_crtc_state))
> > > -			intel_dmc_disable_pipe(i915, slave_crtc->pipe);
> > > -	}
> > > +	for_each_intel_crtc_in_pipe_mask(&i915->drm, crtc, pipe_mask)
> > > +		intel_dmc_disable_pipe(i915, crtc->pipe);
> > >  }
> > 
> > Okay the only difference from hsw_crtc_disable part from my patch is that
> > I don't have intel_crtc_joined_pipe_mask and encoder calls are outside the pipe
> > loop. Ok. You could of course just communicate this to me, it is quite a small
> > thing to change.
> > 
> > And still there is a question about how to handle the crtc enable side, since
> > extracting transcoder programming from the pipe loop, will break the sequence,
> > as I described. Either it is ok that we will partly program slave/master pipe, then
> > program transcoder then again program slave/master pipes or it has to be
> > in a pipe loop.
> 
> Transcoder stuff shouldn't be in pipe loops. That's what
> I've been saying all along.

Yep, I realize you kept saying this and I described you the problem what happens if 
we extract it from there.
Either it is ok to have 2 loops and have transcoder programming in between or you
first program pipes then program the transcoder - in both cases that would change
the sequence of how it is done now.
My question was if this is ok or not.

Stan

> 
> -- 
> Ville Syrjälä
> Intel

Re: [PATCH 8/8] drm/i915: Handle joined pipes inside hsw_crtc_disable()

[Ville Syrjälä]

On Fri, Mar 01, 2024 at 06:22:19PM +0200, Lisovskiy, Stanislav wrote:
> On Fri, Mar 01, 2024 at 06:10:25PM +0200, Ville Syrjälä wrote:
> > On Fri, Mar 01, 2024 at 06:04:27PM +0200, Lisovskiy, Stanislav wrote:
> > > On Fri, Mar 01, 2024 at 04:36:00PM +0200, Ville Syrjala wrote:
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > 
> > > > Reorganize the crtc disable path to only deal with the
> > > > master pipes/transcoders in intel_old_crtc_state_disables()
> > > > and offload the handling of joined pipes to hsw_crtc_disable().
> > > > This makes the whole thing much more sensible since we can
> > > > actually control the order in which we do the per-pipe vs.
> > > > per-transcoder modeset steps.
> > > > 
> > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/display/intel_display.c | 64 ++++++++++++--------
> > > >  1 file changed, 38 insertions(+), 26 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > > > index 1df3923cc30d..07239c1ce9df 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > > @@ -1793,29 +1793,27 @@ static void hsw_crtc_disable(struct intel_atomic_state *state,
> > > >  	const struct intel_crtc_state *old_master_crtc_state =
> > > >  		intel_atomic_get_old_crtc_state(state, master_crtc);
> > > >  	struct drm_i915_private *i915 = to_i915(master_crtc->base.dev);
> > > > +	u8 pipe_mask = intel_crtc_joined_pipe_mask(old_master_crtc_state);
> > > > +	struct intel_crtc *crtc;
> > > >  
> > > >  	/*
> > > >  	 * FIXME collapse everything to one hook.
> > > >  	 * Need care with mst->ddi interactions.
> > > >  	 */
> > > > -	if (!intel_crtc_is_bigjoiner_slave(old_master_crtc_state)) {
> > > > -		intel_encoders_disable(state, master_crtc);
> > > > -		intel_encoders_post_disable(state, master_crtc);
> > > > -	}
> > > > -
> > > > -	intel_disable_shared_dpll(old_master_crtc_state);
> > > > +	intel_encoders_disable(state, master_crtc);
> > > > +	intel_encoders_post_disable(state, master_crtc);
> > > >  
> > > > -	if (!intel_crtc_is_bigjoiner_slave(old_master_crtc_state)) {
> > > > -		struct intel_crtc *slave_crtc;
> > > > +	for_each_intel_crtc_in_pipe_mask(&i915->drm, crtc, pipe_mask) {
> > > > +		const struct intel_crtc_state *old_crtc_state =
> > > > +			intel_atomic_get_old_crtc_state(state, crtc);
> > > >  
> > > > -		intel_encoders_post_pll_disable(state, master_crtc);
> > > > +		intel_disable_shared_dpll(old_crtc_state);
> > > > +	}
> > > >  
> > > > -		intel_dmc_disable_pipe(i915, master_crtc->pipe);
> > > > +	intel_encoders_post_pll_disable(state, master_crtc);
> > > >  
> > > > -		for_each_intel_crtc_in_pipe_mask(&i915->drm, slave_crtc,
> > > > -						 intel_crtc_bigjoiner_slave_pipes(old_master_crtc_state))
> > > > -			intel_dmc_disable_pipe(i915, slave_crtc->pipe);
> > > > -	}
> > > > +	for_each_intel_crtc_in_pipe_mask(&i915->drm, crtc, pipe_mask)
> > > > +		intel_dmc_disable_pipe(i915, crtc->pipe);
> > > >  }
> > > 
> > > Okay the only difference from hsw_crtc_disable part from my patch is that
> > > I don't have intel_crtc_joined_pipe_mask and encoder calls are outside the pipe
> > > loop. Ok. You could of course just communicate this to me, it is quite a small
> > > thing to change.
> > > 
> > > And still there is a question about how to handle the crtc enable side, since
> > > extracting transcoder programming from the pipe loop, will break the sequence,
> > > as I described. Either it is ok that we will partly program slave/master pipe, then
> > > program transcoder then again program slave/master pipes or it has to be
> > > in a pipe loop.
> > 
> > Transcoder stuff shouldn't be in pipe loops. That's what
> > I've been saying all along.
> 
> Yep, I realize you kept saying this and I described you the problem what happens if 
> we extract it from there.
> Either it is ok to have 2 loops and have transcoder programming in between or you
> first program pipes then program the transcoder - in both cases that would change
> the sequence of how it is done now.
> My question was if this is ok or not.

Well, that's pretty much it's supposed to be done. As mentioned
I think the current code kinda works more by luck.

I suppose the only reason it works at all is that we do try to order
at least some of the steps via the tricks in
icl_ddi_bigjoiner_pre_enable() and the specific ordering of the crtcs
from the modeset_enables/disables(). But I'm pretty sure there are 
some steps that currently get done in different places for
the master and slace pipes. And that's not by design.

In general it's pretty hard to actually figure out what steps are
being done in which order in the current code.

The "is it OK?" question I think is best answered by asking the
real hardware. If there is some specific ordering requirement
that the current code accidentally gets right but the obvious
code would somehow get wrong, the hardware should be able to
tell us pretty quickly.

-- 
Ville Syrjälä
Intel

Re: [PATCH 8/8] drm/i915: Handle joined pipes inside hsw_crtc_disable()

[Lisovskiy, Stanislav]

On Fri, Mar 01, 2024 at 06:47:54PM +0200, Ville Syrjälä wrote:
> On Fri, Mar 01, 2024 at 06:22:19PM +0200, Lisovskiy, Stanislav wrote:
> > On Fri, Mar 01, 2024 at 06:10:25PM +0200, Ville Syrjälä wrote:
> > > On Fri, Mar 01, 2024 at 06:04:27PM +0200, Lisovskiy, Stanislav wrote:
> > > > On Fri, Mar 01, 2024 at 04:36:00PM +0200, Ville Syrjala wrote:
> > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > 
> > > > > Reorganize the crtc disable path to only deal with the
> > > > > master pipes/transcoders in intel_old_crtc_state_disables()
> > > > > and offload the handling of joined pipes to hsw_crtc_disable().
> > > > > This makes the whole thing much more sensible since we can
> > > > > actually control the order in which we do the per-pipe vs.
> > > > > per-transcoder modeset steps.
> > > > > 
> > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > ---
> > > > >  drivers/gpu/drm/i915/display/intel_display.c | 64 ++++++++++++--------
> > > > >  1 file changed, 38 insertions(+), 26 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > > > > index 1df3923cc30d..07239c1ce9df 100644
> > > > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > > > @@ -1793,29 +1793,27 @@ static void hsw_crtc_disable(struct intel_atomic_state *state,
> > > > >  	const struct intel_crtc_state *old_master_crtc_state =
> > > > >  		intel_atomic_get_old_crtc_state(state, master_crtc);
> > > > >  	struct drm_i915_private *i915 = to_i915(master_crtc->base.dev);
> > > > > +	u8 pipe_mask = intel_crtc_joined_pipe_mask(old_master_crtc_state);
> > > > > +	struct intel_crtc *crtc;
> > > > >  
> > > > >  	/*
> > > > >  	 * FIXME collapse everything to one hook.
> > > > >  	 * Need care with mst->ddi interactions.
> > > > >  	 */
> > > > > -	if (!intel_crtc_is_bigjoiner_slave(old_master_crtc_state)) {
> > > > > -		intel_encoders_disable(state, master_crtc);
> > > > > -		intel_encoders_post_disable(state, master_crtc);
> > > > > -	}
> > > > > -
> > > > > -	intel_disable_shared_dpll(old_master_crtc_state);
> > > > > +	intel_encoders_disable(state, master_crtc);
> > > > > +	intel_encoders_post_disable(state, master_crtc);
> > > > >  
> > > > > -	if (!intel_crtc_is_bigjoiner_slave(old_master_crtc_state)) {
> > > > > -		struct intel_crtc *slave_crtc;
> > > > > +	for_each_intel_crtc_in_pipe_mask(&i915->drm, crtc, pipe_mask) {
> > > > > +		const struct intel_crtc_state *old_crtc_state =
> > > > > +			intel_atomic_get_old_crtc_state(state, crtc);
> > > > >  
> > > > > -		intel_encoders_post_pll_disable(state, master_crtc);
> > > > > +		intel_disable_shared_dpll(old_crtc_state);
> > > > > +	}
> > > > >  
> > > > > -		intel_dmc_disable_pipe(i915, master_crtc->pipe);
> > > > > +	intel_encoders_post_pll_disable(state, master_crtc);
> > > > >  
> > > > > -		for_each_intel_crtc_in_pipe_mask(&i915->drm, slave_crtc,
> > > > > -						 intel_crtc_bigjoiner_slave_pipes(old_master_crtc_state))
> > > > > -			intel_dmc_disable_pipe(i915, slave_crtc->pipe);
> > > > > -	}
> > > > > +	for_each_intel_crtc_in_pipe_mask(&i915->drm, crtc, pipe_mask)
> > > > > +		intel_dmc_disable_pipe(i915, crtc->pipe);
> > > > >  }
> > > > 
> > > > Okay the only difference from hsw_crtc_disable part from my patch is that
> > > > I don't have intel_crtc_joined_pipe_mask and encoder calls are outside the pipe
> > > > loop. Ok. You could of course just communicate this to me, it is quite a small
> > > > thing to change.
> > > > 
> > > > And still there is a question about how to handle the crtc enable side, since
> > > > extracting transcoder programming from the pipe loop, will break the sequence,
> > > > as I described. Either it is ok that we will partly program slave/master pipe, then
> > > > program transcoder then again program slave/master pipes or it has to be
> > > > in a pipe loop.
> > > 
> > > Transcoder stuff shouldn't be in pipe loops. That's what
> > > I've been saying all along.
> > 
> > Yep, I realize you kept saying this and I described you the problem what happens if 
> > we extract it from there.
> > Either it is ok to have 2 loops and have transcoder programming in between or you
> > first program pipes then program the transcoder - in both cases that would change
> > the sequence of how it is done now.
> > My question was if this is ok or not.
> 
> Well, that's pretty much it's supposed to be done. As mentioned
> I think the current code kinda works more by luck.
> 
> I suppose the only reason it works at all is that we do try to order
> at least some of the steps via the tricks in
> icl_ddi_bigjoiner_pre_enable() and the specific ordering of the crtcs
> from the modeset_enables/disables(). But I'm pretty sure there are 
> some steps that currently get done in different places for
> the master and slace pipes. And that's not by design.
> 
> In general it's pretty hard to actually figure out what steps are
> being done in which order in the current code.
> 
> The "is it OK?" question I think is best answered by asking the
> real hardware. If there is some specific ordering requirement
> that the current code accidentally gets right but the obvious
> code would somehow get wrong, the hardware should be able to
> tell us pretty quickly.

I think this has to be spec to follow or somekind of agreement, 
trial and error method doesn't sound like right approach, even if
hardware works someway, doesn't mean it is supposed to be like that.
That is why I didn't want to change the original sequence of calls,
assuming there was some reason behind it.

Stan

> 
> -- 
> Ville Syrjälä
> Intel

Re: [PATCH 8/8] drm/i915: Handle joined pipes inside hsw_crtc_disable()

[Lisovskiy, Stanislav]

On Fri, Mar 01, 2024 at 04:36:00PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Reorganize the crtc disable path to only deal with the
> master pipes/transcoders in intel_old_crtc_state_disables()
> and offload the handling of joined pipes to hsw_crtc_disable().
> This makes the whole thing much more sensible since we can
> actually control the order in which we do the per-pipe vs.
> per-transcoder modeset steps.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 64 ++++++++++++--------
>  1 file changed, 38 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 1df3923cc30d..07239c1ce9df 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -1793,29 +1793,27 @@ static void hsw_crtc_disable(struct intel_atomic_state *state,
>  	const struct intel_crtc_state *old_master_crtc_state =
>  		intel_atomic_get_old_crtc_state(state, master_crtc);
>  	struct drm_i915_private *i915 = to_i915(master_crtc->base.dev);
> +	u8 pipe_mask = intel_crtc_joined_pipe_mask(old_master_crtc_state);
> +	struct intel_crtc *crtc;
>  
>  	/*
>  	 * FIXME collapse everything to one hook.
>  	 * Need care with mst->ddi interactions.
>  	 */
> -	if (!intel_crtc_is_bigjoiner_slave(old_master_crtc_state)) {
> -		intel_encoders_disable(state, master_crtc);
> -		intel_encoders_post_disable(state, master_crtc);
> -	}
> -
> -	intel_disable_shared_dpll(old_master_crtc_state);
> +	intel_encoders_disable(state, master_crtc);
> +	intel_encoders_post_disable(state, master_crtc);
>  
> -	if (!intel_crtc_is_bigjoiner_slave(old_master_crtc_state)) {
> -		struct intel_crtc *slave_crtc;
> +	for_each_intel_crtc_in_pipe_mask(&i915->drm, crtc, pipe_mask) {
> +		const struct intel_crtc_state *old_crtc_state =
> +			intel_atomic_get_old_crtc_state(state, crtc);
>  
> -		intel_encoders_post_pll_disable(state, master_crtc);
> +		intel_disable_shared_dpll(old_crtc_state);
> +	}
>  
> -		intel_dmc_disable_pipe(i915, master_crtc->pipe);
> +	intel_encoders_post_pll_disable(state, master_crtc);
>  
> -		for_each_intel_crtc_in_pipe_mask(&i915->drm, slave_crtc,
> -						 intel_crtc_bigjoiner_slave_pipes(old_master_crtc_state))
> -			intel_dmc_disable_pipe(i915, slave_crtc->pipe);
> -	}
> +	for_each_intel_crtc_in_pipe_mask(&i915->drm, crtc, pipe_mask)
> +		intel_dmc_disable_pipe(i915, crtc->pipe);
>  }
>  
>  static void i9xx_pfit_enable(const struct intel_crtc_state *crtc_state)
> @@ -6753,24 +6751,33 @@ static void intel_update_crtc(struct intel_atomic_state *state,
>  }
>  
>  static void intel_old_crtc_state_disables(struct intel_atomic_state *state,
> -					  struct intel_crtc *crtc)
> +					  struct intel_crtc *master_crtc)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> -	const struct intel_crtc_state *new_crtc_state =
> -		intel_atomic_get_new_crtc_state(state, crtc);
> +	const struct intel_crtc_state *old_master_crtc_state =
> +		intel_atomic_get_old_crtc_state(state, master_crtc);
> +	u8 pipe_mask = intel_crtc_joined_pipe_mask(old_master_crtc_state);
> +	struct intel_crtc *crtc;
>  
>  	/*
>  	 * We need to disable pipe CRC before disabling the pipe,
>  	 * or we race against vblank off.
>  	 */
> -	intel_crtc_disable_pipe_crc(crtc);
> +	for_each_intel_crtc_in_pipe_mask(&dev_priv->drm, crtc, pipe_mask)
> +		intel_crtc_disable_pipe_crc(crtc);
>  
>  	dev_priv->display.funcs.display->crtc_disable(state, crtc);
> -	crtc->active = false;
> -	intel_fbc_disable(crtc);
>  
> -	if (!new_crtc_state->hw.active)
> -		intel_initial_watermarks(state, crtc);
> +	for_each_intel_crtc_in_pipe_mask(&dev_priv->drm, crtc, pipe_mask) {
> +		const struct intel_crtc_state *new_crtc_state =
> +			intel_atomic_get_new_crtc_state(state, crtc);
> +
> +		crtc->active = false;
> +		intel_fbc_disable(crtc);
> +
> +		if (!new_crtc_state->hw.active)
> +			intel_initial_watermarks(state, crtc);
> +	}
>  }
>  
>  static void intel_commit_modeset_disables(struct intel_atomic_state *state)
> @@ -6810,19 +6817,21 @@ static void intel_commit_modeset_disables(struct intel_atomic_state *state)
>  		if ((disable_pipes & BIT(crtc->pipe)) == 0)
>  			continue;
>  
> +		if (intel_crtc_is_bigjoiner_slave(old_crtc_state))
> +			continue;
> +
>  		/* In case of Transcoder port Sync master slave CRTCs can be
>  		 * assigned in any order and we need to make sure that
>  		 * slave CRTCs are disabled first and then master CRTC since
>  		 * Slave vblanks are masked till Master Vblanks.
>  		 */
>  		if (!is_trans_port_sync_slave(old_crtc_state) &&
> -		    !intel_dp_mst_is_slave_trans(old_crtc_state) &&
> -		    !intel_crtc_is_bigjoiner_slave(old_crtc_state))
> +		    !intel_dp_mst_is_slave_trans(old_crtc_state))
>  			continue;
>  
>  		intel_old_crtc_state_disables(state, crtc);
>  
> -		disable_pipes &= ~BIT(crtc->pipe);
> +		disable_pipes &= ~intel_crtc_joined_pipe_mask(old_crtc_state);
>  	}
>  
>  	/* Disable everything else left on */
> @@ -6830,9 +6839,12 @@ static void intel_commit_modeset_disables(struct intel_atomic_state *state)
>  		if ((disable_pipes & BIT(crtc->pipe)) == 0)
>  			continue;
>  
> +		if (intel_crtc_is_bigjoiner_slave(old_crtc_state))
> +			continue;
> +
>  		intel_old_crtc_state_disables(state, crtc);
>  
> -		disable_pipes &= ~BIT(crtc->pipe);
> +		disable_pipes &= ~intel_crtc_joined_pipe_mask(old_crtc_state);

Yep btw that one I've also fixed today, based on your comment, also for enable side..


Stan

>  	}
>  
>  	drm_WARN_ON(&i915->drm, disable_pipes);
> -- 
> 2.43.0
> 

Re: [PATCH 8/8] drm/i915: Handle joined pipes inside hsw_crtc_disable()

[Ville Syrjälä]

On Fri, Mar 01, 2024 at 04:36:00PM +0200, Ville Syrjala wrote:
>  	/*
>  	 * We need to disable pipe CRC before disabling the pipe,
>  	 * or we race against vblank off.
>  	 */
> -	intel_crtc_disable_pipe_crc(crtc);
> +	for_each_intel_crtc_in_pipe_mask(&dev_priv->drm, crtc, pipe_mask)
> +		intel_crtc_disable_pipe_crc(crtc);

Oh, and the pipe crc stuff is one thing we probably should spend some
brain cells on. The pfit/plane CRCs are per-pipe, so we should either
handle it in igt somehow, or we need come up with some kind of scheme
to combine the CRCs from all the joined pipes in the kernel so that
userspace doesn't have to deal with them.

-- 
Ville Syrjälä
Intel

[PATCH v2 8/8] drm/i915: Handle joined pipes inside hsw_crtc_disable()

[Ville Syrjala]

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

Reorganize the crtc disable path to only deal with the
master pipes/transcoders in intel_old_crtc_state_disables()
and offload the handling of joined pipes to hsw_crtc_disable().
This makes the whole thing much more sensible since we can
actually control the order in which we do the per-pipe vs.
per-transcoder modeset steps.

v2: Pass the correct crtc pointer to .crtc_disable()

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

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 1df3923cc30d..e01536983303 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -1793,29 +1793,27 @@ static void hsw_crtc_disable(struct intel_atomic_state *state,
 	const struct intel_crtc_state *old_master_crtc_state =
 		intel_atomic_get_old_crtc_state(state, master_crtc);
 	struct drm_i915_private *i915 = to_i915(master_crtc->base.dev);
+	u8 pipe_mask = intel_crtc_joined_pipe_mask(old_master_crtc_state);
+	struct intel_crtc *crtc;
 
 	/*
 	 * FIXME collapse everything to one hook.
 	 * Need care with mst->ddi interactions.
 	 */
-	if (!intel_crtc_is_bigjoiner_slave(old_master_crtc_state)) {
-		intel_encoders_disable(state, master_crtc);
-		intel_encoders_post_disable(state, master_crtc);
-	}
-
-	intel_disable_shared_dpll(old_master_crtc_state);
+	intel_encoders_disable(state, master_crtc);
+	intel_encoders_post_disable(state, master_crtc);
 
-	if (!intel_crtc_is_bigjoiner_slave(old_master_crtc_state)) {
-		struct intel_crtc *slave_crtc;
+	for_each_intel_crtc_in_pipe_mask(&i915->drm, crtc, pipe_mask) {
+		const struct intel_crtc_state *old_crtc_state =
+			intel_atomic_get_old_crtc_state(state, crtc);
 
-		intel_encoders_post_pll_disable(state, master_crtc);
+		intel_disable_shared_dpll(old_crtc_state);
+	}
 
-		intel_dmc_disable_pipe(i915, master_crtc->pipe);
+	intel_encoders_post_pll_disable(state, master_crtc);
 
-		for_each_intel_crtc_in_pipe_mask(&i915->drm, slave_crtc,
-						 intel_crtc_bigjoiner_slave_pipes(old_master_crtc_state))
-			intel_dmc_disable_pipe(i915, slave_crtc->pipe);
-	}
+	for_each_intel_crtc_in_pipe_mask(&i915->drm, crtc, pipe_mask)
+		intel_dmc_disable_pipe(i915, crtc->pipe);
 }
 
 static void i9xx_pfit_enable(const struct intel_crtc_state *crtc_state)
@@ -6753,24 +6751,33 @@ static void intel_update_crtc(struct intel_atomic_state *state,
 }
 
 static void intel_old_crtc_state_disables(struct intel_atomic_state *state,
-					  struct intel_crtc *crtc)
+					  struct intel_crtc *master_crtc)
 {
 	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
-	const struct intel_crtc_state *new_crtc_state =
-		intel_atomic_get_new_crtc_state(state, crtc);
+	const struct intel_crtc_state *old_master_crtc_state =
+		intel_atomic_get_old_crtc_state(state, master_crtc);
+	u8 pipe_mask = intel_crtc_joined_pipe_mask(old_master_crtc_state);
+	struct intel_crtc *crtc;
 
 	/*
 	 * We need to disable pipe CRC before disabling the pipe,
 	 * or we race against vblank off.
 	 */
-	intel_crtc_disable_pipe_crc(crtc);
+	for_each_intel_crtc_in_pipe_mask(&dev_priv->drm, crtc, pipe_mask)
+		intel_crtc_disable_pipe_crc(crtc);
 
-	dev_priv->display.funcs.display->crtc_disable(state, crtc);
-	crtc->active = false;
-	intel_fbc_disable(crtc);
+	dev_priv->display.funcs.display->crtc_disable(state, master_crtc);
 
-	if (!new_crtc_state->hw.active)
-		intel_initial_watermarks(state, crtc);
+	for_each_intel_crtc_in_pipe_mask(&dev_priv->drm, crtc, pipe_mask) {
+		const struct intel_crtc_state *new_crtc_state =
+			intel_atomic_get_new_crtc_state(state, crtc);
+
+		crtc->active = false;
+		intel_fbc_disable(crtc);
+
+		if (!new_crtc_state->hw.active)
+			intel_initial_watermarks(state, crtc);
+	}
 }
 
 static void intel_commit_modeset_disables(struct intel_atomic_state *state)
@@ -6810,19 +6817,21 @@ static void intel_commit_modeset_disables(struct intel_atomic_state *state)
 		if ((disable_pipes & BIT(crtc->pipe)) == 0)
 			continue;
 
+		if (intel_crtc_is_bigjoiner_slave(old_crtc_state))
+			continue;
+
 		/* In case of Transcoder port Sync master slave CRTCs can be
 		 * assigned in any order and we need to make sure that
 		 * slave CRTCs are disabled first and then master CRTC since
 		 * Slave vblanks are masked till Master Vblanks.
 		 */
 		if (!is_trans_port_sync_slave(old_crtc_state) &&
-		    !intel_dp_mst_is_slave_trans(old_crtc_state) &&
-		    !intel_crtc_is_bigjoiner_slave(old_crtc_state))
+		    !intel_dp_mst_is_slave_trans(old_crtc_state))
 			continue;
 
 		intel_old_crtc_state_disables(state, crtc);
 
-		disable_pipes &= ~BIT(crtc->pipe);
+		disable_pipes &= ~intel_crtc_joined_pipe_mask(old_crtc_state);
 	}
 
 	/* Disable everything else left on */
@@ -6830,9 +6839,12 @@ static void intel_commit_modeset_disables(struct intel_atomic_state *state)
 		if ((disable_pipes & BIT(crtc->pipe)) == 0)
 			continue;
 
+		if (intel_crtc_is_bigjoiner_slave(old_crtc_state))
+			continue;
+
 		intel_old_crtc_state_disables(state, crtc);
 
-		disable_pipes &= ~BIT(crtc->pipe);
+		disable_pipes &= ~intel_crtc_joined_pipe_mask(old_crtc_state);
 	}
 
 	drm_WARN_ON(&i915->drm, disable_pipes);
-- 
2.43.0

RE: [PATCH v2 8/8] drm/i915: Handle joined pipes inside hsw_crtc_disable()

[Srinivas, Vidya]

Thank you very much Ville and Stan.
With https://patchwork.freedesktop.org/series/130619/ and https://patchwork.freedesktop.org/series/130449/ tested that 6K works
Tested-by: Vidya Srinivas <vidya.srinivas@intel.com>

> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Ville
> Syrjala
> Sent: Friday, March 1, 2024 10:54 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: Lisovskiy, Stanislav <stanislav.lisovskiy@intel.com>
> Subject: [PATCH v2 8/8] drm/i915: Handle joined pipes inside
> hsw_crtc_disable()
> 
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Reorganize the crtc disable path to only deal with the master
> pipes/transcoders in intel_old_crtc_state_disables() and offload the handling
> of joined pipes to hsw_crtc_disable().
> This makes the whole thing much more sensible since we can actually control
> the order in which we do the per-pipe vs.
> per-transcoder modeset steps.
> 
> v2: Pass the correct crtc pointer to .crtc_disable()
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 66 ++++++++++++--------
>  1 file changed, 39 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 1df3923cc30d..e01536983303 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -1793,29 +1793,27 @@ static void hsw_crtc_disable(struct
> intel_atomic_state *state,
>  	const struct intel_crtc_state *old_master_crtc_state =
>  		intel_atomic_get_old_crtc_state(state, master_crtc);
>  	struct drm_i915_private *i915 = to_i915(master_crtc->base.dev);
> +	u8 pipe_mask = intel_crtc_joined_pipe_mask(old_master_crtc_state);
> +	struct intel_crtc *crtc;
> 
>  	/*
>  	 * FIXME collapse everything to one hook.
>  	 * Need care with mst->ddi interactions.
>  	 */
> -	if (!intel_crtc_is_bigjoiner_slave(old_master_crtc_state)) {
> -		intel_encoders_disable(state, master_crtc);
> -		intel_encoders_post_disable(state, master_crtc);
> -	}
> -
> -	intel_disable_shared_dpll(old_master_crtc_state);
> +	intel_encoders_disable(state, master_crtc);
> +	intel_encoders_post_disable(state, master_crtc);
> 
> -	if (!intel_crtc_is_bigjoiner_slave(old_master_crtc_state)) {
> -		struct intel_crtc *slave_crtc;
> +	for_each_intel_crtc_in_pipe_mask(&i915->drm, crtc, pipe_mask) {
> +		const struct intel_crtc_state *old_crtc_state =
> +			intel_atomic_get_old_crtc_state(state, crtc);
> 
> -		intel_encoders_post_pll_disable(state, master_crtc);
> +		intel_disable_shared_dpll(old_crtc_state);
> +	}
> 
> -		intel_dmc_disable_pipe(i915, master_crtc->pipe);
> +	intel_encoders_post_pll_disable(state, master_crtc);
> 
> -		for_each_intel_crtc_in_pipe_mask(&i915->drm, slave_crtc,
> -
> intel_crtc_bigjoiner_slave_pipes(old_master_crtc_state))
> -			intel_dmc_disable_pipe(i915, slave_crtc->pipe);
> -	}
> +	for_each_intel_crtc_in_pipe_mask(&i915->drm, crtc, pipe_mask)
> +		intel_dmc_disable_pipe(i915, crtc->pipe);
>  }
> 
>  static void i9xx_pfit_enable(const struct intel_crtc_state *crtc_state) @@ -
> 6753,24 +6751,33 @@ static void intel_update_crtc(struct intel_atomic_state
> *state,  }
> 
>  static void intel_old_crtc_state_disables(struct intel_atomic_state *state,
> -					  struct intel_crtc *crtc)
> +					  struct intel_crtc *master_crtc)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> -	const struct intel_crtc_state *new_crtc_state =
> -		intel_atomic_get_new_crtc_state(state, crtc);
> +	const struct intel_crtc_state *old_master_crtc_state =
> +		intel_atomic_get_old_crtc_state(state, master_crtc);
> +	u8 pipe_mask = intel_crtc_joined_pipe_mask(old_master_crtc_state);
> +	struct intel_crtc *crtc;
> 
>  	/*
>  	 * We need to disable pipe CRC before disabling the pipe,
>  	 * or we race against vblank off.
>  	 */
> -	intel_crtc_disable_pipe_crc(crtc);
> +	for_each_intel_crtc_in_pipe_mask(&dev_priv->drm, crtc, pipe_mask)
> +		intel_crtc_disable_pipe_crc(crtc);
> 
> -	dev_priv->display.funcs.display->crtc_disable(state, crtc);
> -	crtc->active = false;
> -	intel_fbc_disable(crtc);
> +	dev_priv->display.funcs.display->crtc_disable(state, master_crtc);
> 
> -	if (!new_crtc_state->hw.active)
> -		intel_initial_watermarks(state, crtc);
> +	for_each_intel_crtc_in_pipe_mask(&dev_priv->drm, crtc, pipe_mask)
> {
> +		const struct intel_crtc_state *new_crtc_state =
> +			intel_atomic_get_new_crtc_state(state, crtc);
> +
> +		crtc->active = false;
> +		intel_fbc_disable(crtc);
> +
> +		if (!new_crtc_state->hw.active)
> +			intel_initial_watermarks(state, crtc);
> +	}
>  }
> 
>  static void intel_commit_modeset_disables(struct intel_atomic_state *state)
> @@ -6810,19 +6817,21 @@ static void
> intel_commit_modeset_disables(struct intel_atomic_state *state)
>  		if ((disable_pipes & BIT(crtc->pipe)) == 0)
>  			continue;
> 
> +		if (intel_crtc_is_bigjoiner_slave(old_crtc_state))
> +			continue;
> +
>  		/* In case of Transcoder port Sync master slave CRTCs can be
>  		 * assigned in any order and we need to make sure that
>  		 * slave CRTCs are disabled first and then master CRTC since
>  		 * Slave vblanks are masked till Master Vblanks.
>  		 */
>  		if (!is_trans_port_sync_slave(old_crtc_state) &&
> -		    !intel_dp_mst_is_slave_trans(old_crtc_state) &&
> -		    !intel_crtc_is_bigjoiner_slave(old_crtc_state))
> +		    !intel_dp_mst_is_slave_trans(old_crtc_state))
>  			continue;
> 
>  		intel_old_crtc_state_disables(state, crtc);
> 
> -		disable_pipes &= ~BIT(crtc->pipe);
> +		disable_pipes &=
> ~intel_crtc_joined_pipe_mask(old_crtc_state);
>  	}
> 
>  	/* Disable everything else left on */
> @@ -6830,9 +6839,12 @@ static void intel_commit_modeset_disables(struct
> intel_atomic_state *state)
>  		if ((disable_pipes & BIT(crtc->pipe)) == 0)
>  			continue;
> 
> +		if (intel_crtc_is_bigjoiner_slave(old_crtc_state))
> +			continue;
> +
>  		intel_old_crtc_state_disables(state, crtc);
> 
> -		disable_pipes &= ~BIT(crtc->pipe);
> +		disable_pipes &=
> ~intel_crtc_joined_pipe_mask(old_crtc_state);
>  	}
> 
>  	drm_WARN_ON(&i915->drm, disable_pipes);
> --
> 2.43.0

Re: [PATCH v2 8/8] drm/i915: Handle joined pipes inside hsw_crtc_disable()

[Lisovskiy, Stanislav]

On Mon, Mar 04, 2024 at 08:44:35AM +0200, Srinivas, Vidya wrote:
> Thank you very much Ville and Stan.
> With https://patchwork.freedesktop.org/series/130619/ and https://patchwork.freedesktop.org/series/130449/ tested that 6K works
> Tested-by: Vidya Srinivas <vidya.srinivas@intel.com>

The thing is that we still don't handle crtc enable(i.e actual enabling of displays)
here at all, only disabling part. So fact that it works could be also related to this.

Ville, should I use your series, plus the things we had discussed in my series to implement
hsw_crtc_enable on top of your series?
Of course things related to transcoder have to be clarified still. 
Or do you plan to do it yourself?

Stan

> 
> > -----Original Message-----
> > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Ville
> > Syrjala
> > Sent: Friday, March 1, 2024 10:54 PM
> > To: intel-gfx@lists.freedesktop.org
> > Cc: Lisovskiy, Stanislav <stanislav.lisovskiy@intel.com>
> > Subject: [PATCH v2 8/8] drm/i915: Handle joined pipes inside
> > hsw_crtc_disable()
> > 
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Reorganize the crtc disable path to only deal with the master
> > pipes/transcoders in intel_old_crtc_state_disables() and offload the handling
> > of joined pipes to hsw_crtc_disable().
> > This makes the whole thing much more sensible since we can actually control
> > the order in which we do the per-pipe vs.
> > per-transcoder modeset steps.
> > 
> > v2: Pass the correct crtc pointer to .crtc_disable()
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_display.c | 66 ++++++++++++--------
> >  1 file changed, 39 insertions(+), 27 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > b/drivers/gpu/drm/i915/display/intel_display.c
> > index 1df3923cc30d..e01536983303 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -1793,29 +1793,27 @@ static void hsw_crtc_disable(struct
> > intel_atomic_state *state,
> >  	const struct intel_crtc_state *old_master_crtc_state =
> >  		intel_atomic_get_old_crtc_state(state, master_crtc);
> >  	struct drm_i915_private *i915 = to_i915(master_crtc->base.dev);
> > +	u8 pipe_mask = intel_crtc_joined_pipe_mask(old_master_crtc_state);
> > +	struct intel_crtc *crtc;
> > 
> >  	/*
> >  	 * FIXME collapse everything to one hook.
> >  	 * Need care with mst->ddi interactions.
> >  	 */
> > -	if (!intel_crtc_is_bigjoiner_slave(old_master_crtc_state)) {
> > -		intel_encoders_disable(state, master_crtc);
> > -		intel_encoders_post_disable(state, master_crtc);
> > -	}
> > -
> > -	intel_disable_shared_dpll(old_master_crtc_state);
> > +	intel_encoders_disable(state, master_crtc);
> > +	intel_encoders_post_disable(state, master_crtc);
> > 
> > -	if (!intel_crtc_is_bigjoiner_slave(old_master_crtc_state)) {
> > -		struct intel_crtc *slave_crtc;
> > +	for_each_intel_crtc_in_pipe_mask(&i915->drm, crtc, pipe_mask) {
> > +		const struct intel_crtc_state *old_crtc_state =
> > +			intel_atomic_get_old_crtc_state(state, crtc);
> > 
> > -		intel_encoders_post_pll_disable(state, master_crtc);
> > +		intel_disable_shared_dpll(old_crtc_state);
> > +	}
> > 
> > -		intel_dmc_disable_pipe(i915, master_crtc->pipe);
> > +	intel_encoders_post_pll_disable(state, master_crtc);
> > 
> > -		for_each_intel_crtc_in_pipe_mask(&i915->drm, slave_crtc,
> > -
> > intel_crtc_bigjoiner_slave_pipes(old_master_crtc_state))
> > -			intel_dmc_disable_pipe(i915, slave_crtc->pipe);
> > -	}
> > +	for_each_intel_crtc_in_pipe_mask(&i915->drm, crtc, pipe_mask)
> > +		intel_dmc_disable_pipe(i915, crtc->pipe);
> >  }
> > 
> >  static void i9xx_pfit_enable(const struct intel_crtc_state *crtc_state) @@ -
> > 6753,24 +6751,33 @@ static void intel_update_crtc(struct intel_atomic_state
> > *state,  }
> > 
> >  static void intel_old_crtc_state_disables(struct intel_atomic_state *state,
> > -					  struct intel_crtc *crtc)
> > +					  struct intel_crtc *master_crtc)
> >  {
> >  	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> > -	const struct intel_crtc_state *new_crtc_state =
> > -		intel_atomic_get_new_crtc_state(state, crtc);
> > +	const struct intel_crtc_state *old_master_crtc_state =
> > +		intel_atomic_get_old_crtc_state(state, master_crtc);
> > +	u8 pipe_mask = intel_crtc_joined_pipe_mask(old_master_crtc_state);
> > +	struct intel_crtc *crtc;
> > 
> >  	/*
> >  	 * We need to disable pipe CRC before disabling the pipe,
> >  	 * or we race against vblank off.
> >  	 */
> > -	intel_crtc_disable_pipe_crc(crtc);
> > +	for_each_intel_crtc_in_pipe_mask(&dev_priv->drm, crtc, pipe_mask)
> > +		intel_crtc_disable_pipe_crc(crtc);
> > 
> > -	dev_priv->display.funcs.display->crtc_disable(state, crtc);
> > -	crtc->active = false;
> > -	intel_fbc_disable(crtc);
> > +	dev_priv->display.funcs.display->crtc_disable(state, master_crtc);
> > 
> > -	if (!new_crtc_state->hw.active)
> > -		intel_initial_watermarks(state, crtc);
> > +	for_each_intel_crtc_in_pipe_mask(&dev_priv->drm, crtc, pipe_mask)
> > {
> > +		const struct intel_crtc_state *new_crtc_state =
> > +			intel_atomic_get_new_crtc_state(state, crtc);
> > +
> > +		crtc->active = false;
> > +		intel_fbc_disable(crtc);
> > +
> > +		if (!new_crtc_state->hw.active)
> > +			intel_initial_watermarks(state, crtc);
> > +	}
> >  }
> > 
> >  static void intel_commit_modeset_disables(struct intel_atomic_state *state)
> > @@ -6810,19 +6817,21 @@ static void
> > intel_commit_modeset_disables(struct intel_atomic_state *state)
> >  		if ((disable_pipes & BIT(crtc->pipe)) == 0)
> >  			continue;
> > 
> > +		if (intel_crtc_is_bigjoiner_slave(old_crtc_state))
> > +			continue;
> > +
> >  		/* In case of Transcoder port Sync master slave CRTCs can be
> >  		 * assigned in any order and we need to make sure that
> >  		 * slave CRTCs are disabled first and then master CRTC since
> >  		 * Slave vblanks are masked till Master Vblanks.
> >  		 */
> >  		if (!is_trans_port_sync_slave(old_crtc_state) &&
> > -		    !intel_dp_mst_is_slave_trans(old_crtc_state) &&
> > -		    !intel_crtc_is_bigjoiner_slave(old_crtc_state))
> > +		    !intel_dp_mst_is_slave_trans(old_crtc_state))
> >  			continue;
> > 
> >  		intel_old_crtc_state_disables(state, crtc);
> > 
> > -		disable_pipes &= ~BIT(crtc->pipe);
> > +		disable_pipes &=
> > ~intel_crtc_joined_pipe_mask(old_crtc_state);
> >  	}
> > 
> >  	/* Disable everything else left on */
> > @@ -6830,9 +6839,12 @@ static void intel_commit_modeset_disables(struct
> > intel_atomic_state *state)
> >  		if ((disable_pipes & BIT(crtc->pipe)) == 0)
> >  			continue;
> > 
> > +		if (intel_crtc_is_bigjoiner_slave(old_crtc_state))
> > +			continue;
> > +
> >  		intel_old_crtc_state_disables(state, crtc);
> > 
> > -		disable_pipes &= ~BIT(crtc->pipe);
> > +		disable_pipes &=
> > ~intel_crtc_joined_pipe_mask(old_crtc_state);
> >  	}
> > 
> >  	drm_WARN_ON(&i915->drm, disable_pipes);
> > --
> > 2.43.0
> 

✗ Fi.CI.CHECKPATCH: warning for drm/i915: Bigjoiner stuff (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: Bigjoiner stuff (rev2)
URL   : https://patchwork.freedesktop.org/series/130619/
State : warning

== Summary ==

Error: dim checkpatch failed
8726aac5fa9c drm/i915: Rename the crtc/crtc_states in the top level DDI hooks/etc
-:1080: WARNING:LONG_LINE: line length of 105 exceeds 100 columns
#1080: FILE: drivers/gpu/drm/i915/display/intel_display.c:1809:
+						 intel_crtc_bigjoiner_slave_pipes(old_master_crtc_state))

-:1194: WARNING:LONG_LINE: line length of 103 exceeds 100 columns
#1194: FILE: drivers/gpu/drm/i915/display/intel_dp_mst.c:1033:
+		dig_port->base.post_pll_disable(state, encoder, old_master_crtc_state, old_conn_state);

total: 0 errors, 2 warnings, 0 checks, 1255 lines checked
7cd79f1e6a16 drm/i915: Introduce intel_crtc_joined_pipe_mask()
82085540bb4f drm/i915: Extract intel_ddi_post_disable_hdmi_or_sst()
da906396f09f drm/i915: Utilize intel_crtc_joined_pipe_mask() more
500ddd32f586 drm/i915: Precompute disable_pipes bitmask in intel_commit_modeset_disables()
1f61f27d3478 drm/i915: Disable planes more atomically during modesets
05378710d5c8 drm/i915: Simplify intel_old_crtc_state_disables() calling convention
64b650019ae8 drm/i915: Handle joined pipes inside hsw_crtc_disable()

✗ Fi.CI.BAT: failure for drm/i915: Bigjoiner stuff (rev2)

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 8324 bytes --]

== Series Details ==

Series: drm/i915: Bigjoiner stuff (rev2)
URL   : https://patchwork.freedesktop.org/series/130619/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_14378 -> Patchwork_130619v2
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_130619v2 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_130619v2, please notify your bug team (I915-ci-infra@lists.freedesktop.org) to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130619v2/index.html

Participating hosts (41 -> 39)
------------------------------

  Missing    (2): bat-kbl-2 fi-snb-2520m 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_130619v2:

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live@gem_contexts:
    - bat-arls-2:         [PASS][1] -> [ABORT][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14378/bat-arls-2/igt@i915_selftest@live@gem_contexts.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130619v2/bat-arls-2/igt@i915_selftest@live@gem_contexts.html

  
Known issues
------------

  Here are the changes found in Patchwork_130619v2 that come from known issues:

### CI changes ###

#### Possible fixes ####

  * boot:
    - bat-jsl-1:          [FAIL][3] ([i915#8293]) -> [PASS][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14378/bat-jsl-1/boot.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130619v2/bat-jsl-1/boot.html

  

### IGT changes ###

#### Issues hit ####

  * igt@debugfs_test@basic-hwmon:
    - bat-jsl-1:          NOTRUN -> [SKIP][5] ([i915#9318])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130619v2/bat-jsl-1/igt@debugfs_test@basic-hwmon.html

  * igt@gem_huc_copy@huc-copy:
    - bat-jsl-1:          NOTRUN -> [SKIP][6] ([i915#2190])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130619v2/bat-jsl-1/igt@gem_huc_copy@huc-copy.html

  * igt@gem_lmem_swapping@verify-random:
    - bat-jsl-1:          NOTRUN -> [SKIP][7] ([i915#4613]) +3 other tests skip
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130619v2/bat-jsl-1/igt@gem_lmem_swapping@verify-random.html

  * igt@i915_pm_rpm@module-reload:
    - fi-kbl-7567u:       [PASS][8] -> [CRASH][9] ([i915#9947])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14378/fi-kbl-7567u/igt@i915_pm_rpm@module-reload.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130619v2/fi-kbl-7567u/igt@i915_pm_rpm@module-reload.html

  * igt@i915_pm_rps@basic-api:
    - bat-dg2-9:          NOTRUN -> [SKIP][10] ([i915#6621])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130619v2/bat-dg2-9/igt@i915_pm_rps@basic-api.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
    - bat-jsl-1:          NOTRUN -> [SKIP][11] ([i915#4103]) +1 other test skip
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130619v2/bat-jsl-1/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html

  * igt@kms_dsc@dsc-basic:
    - bat-jsl-1:          NOTRUN -> [SKIP][12] ([i915#3555] / [i915#9886])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130619v2/bat-jsl-1/igt@kms_dsc@dsc-basic.html

  * igt@kms_force_connector_basic@force-load-detect:
    - bat-jsl-1:          NOTRUN -> [SKIP][13] ([fdo#109285])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130619v2/bat-jsl-1/igt@kms_force_connector_basic@force-load-detect.html

  * igt@kms_psr@psr-sprite-plane-onoff:
    - bat-dg2-9:          NOTRUN -> [SKIP][14] ([i915#9673] / [i915#9732]) +3 other tests skip
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130619v2/bat-dg2-9/igt@kms_psr@psr-sprite-plane-onoff.html

  * igt@kms_setmode@basic-clone-single-crtc:
    - bat-dg2-9:          NOTRUN -> [SKIP][15] ([i915#3555])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130619v2/bat-dg2-9/igt@kms_setmode@basic-clone-single-crtc.html
    - bat-jsl-1:          NOTRUN -> [SKIP][16] ([i915#3555])
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130619v2/bat-jsl-1/igt@kms_setmode@basic-clone-single-crtc.html

  * igt@prime_vgem@basic-fence-flip:
    - bat-dg2-9:          NOTRUN -> [SKIP][17] ([i915#3708])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130619v2/bat-dg2-9/igt@prime_vgem@basic-fence-flip.html

  * igt@prime_vgem@basic-fence-mmap:
    - bat-dg2-9:          NOTRUN -> [SKIP][18] ([i915#3708] / [i915#4077]) +1 other test skip
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130619v2/bat-dg2-9/igt@prime_vgem@basic-fence-mmap.html

  * igt@prime_vgem@basic-write:
    - bat-dg2-9:          NOTRUN -> [SKIP][19] ([i915#3291] / [i915#3708]) +2 other tests skip
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130619v2/bat-dg2-9/igt@prime_vgem@basic-write.html

  
#### Possible fixes ####

  * igt@i915_selftest@live@gt_lrc:
    - bat-adln-1:         [INCOMPLETE][20] ([i915#10241] / [i915#9413]) -> [PASS][21]
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14378/bat-adln-1/igt@i915_selftest@live@gt_lrc.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130619v2/bat-adln-1/igt@i915_selftest@live@gt_lrc.html

  * igt@i915_selftest@live@perf:
    - bat-dg2-14:         [ABORT][22] -> [PASS][23]
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14378/bat-dg2-14/igt@i915_selftest@live@perf.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130619v2/bat-dg2-14/igt@i915_selftest@live@perf.html

  * igt@kms_pm_rpm@basic-pci-d3-state:
    - bat-dg2-9:          [ABORT][24] -> [PASS][25]
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14378/bat-dg2-9/igt@kms_pm_rpm@basic-pci-d3-state.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130619v2/bat-dg2-9/igt@kms_pm_rpm@basic-pci-d3-state.html

  
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [i915#10241]: https://gitlab.freedesktop.org/drm/intel/issues/10241
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#3291]: https://gitlab.freedesktop.org/drm/intel/issues/3291
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
  [i915#4077]: https://gitlab.freedesktop.org/drm/intel/issues/4077
  [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#6621]: https://gitlab.freedesktop.org/drm/intel/issues/6621
  [i915#8293]: https://gitlab.freedesktop.org/drm/intel/issues/8293
  [i915#9318]: https://gitlab.freedesktop.org/drm/intel/issues/9318
  [i915#9413]: https://gitlab.freedesktop.org/drm/intel/issues/9413
  [i915#9673]: https://gitlab.freedesktop.org/drm/intel/issues/9673
  [i915#9732]: https://gitlab.freedesktop.org/drm/intel/issues/9732
  [i915#9886]: https://gitlab.freedesktop.org/drm/intel/issues/9886
  [i915#9947]: https://gitlab.freedesktop.org/drm/intel/issues/9947


Build changes
-------------

  * Linux: CI_DRM_14378 -> Patchwork_130619v2

  CI-20190529: 20190529
  CI_DRM_14378: 5f60548dd58e2ad93005c933fd2633d98db60de4 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7742: 7742
  Patchwork_130619v2: 5f60548dd58e2ad93005c933fd2633d98db60de4 @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

5c34e43a4e78 drm/i915: Handle joined pipes inside hsw_crtc_disable()
e655dcbab735 drm/i915: Simplify intel_old_crtc_state_disables() calling convention
3270e344562f drm/i915: Disable planes more atomically during modesets
b72068ae5537 drm/i915: Precompute disable_pipes bitmask in intel_commit_modeset_disables()
4120574ae215 drm/i915: Utilize intel_crtc_joined_pipe_mask() more
fd7ba7831e9f drm/i915: Extract intel_ddi_post_disable_hdmi_or_sst()
d3a80ec53a6b drm/i915: Introduce intel_crtc_joined_pipe_mask()
03006338af02 drm/i915: Rename the crtc/crtc_states in the top level DDI hooks/etc

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130619v2/index.html

[-- Attachment #2: Type: text/html, Size: 9698 bytes --]