Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [Intel-gfx] [PATCH] drm/i915/display/dsi: use intel_de_rmw if possible
@ 2022-12-19 13:08 Andrzej Hajda
  2022-12-20  9:26 ` Jani Nikula
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Andrzej Hajda @ 2022-12-19 13:08 UTC (permalink / raw)
  To: intel-gfx; +Cc: Andrzej Hajda, Rodrigo Vivi

The helper makes the code more compact and readable.

Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com>
---
 drivers/gpu/drm/i915/display/icl_dsi.c | 256 ++++++++-----------------
 1 file changed, 82 insertions(+), 174 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
index ae14c794c4bc09..b02ac9d2b1e4a2 100644
--- a/drivers/gpu/drm/i915/display/icl_dsi.c
+++ b/drivers/gpu/drm/i915/display/icl_dsi.c
@@ -207,7 +207,7 @@ void icl_dsi_frame_update(struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-	u32 tmp, mode_flags;
+	u32 mode_flags;
 	enum port port;
 
 	mode_flags = crtc_state->mode_flags;
@@ -224,9 +224,7 @@ void icl_dsi_frame_update(struct intel_crtc_state *crtc_state)
 	else
 		return;
 
-	tmp = intel_de_read(dev_priv, DSI_CMD_FRMCTL(port));
-	tmp |= DSI_FRAME_UPDATE_REQUEST;
-	intel_de_write(dev_priv, DSI_CMD_FRMCTL(port), tmp);
+	intel_de_rmw(dev_priv, DSI_CMD_FRMCTL(port), 0, DSI_FRAME_UPDATE_REQUEST);
 }
 
 static void dsi_program_swing_and_deemphasis(struct intel_encoder *encoder)
@@ -234,7 +232,7 @@ static void dsi_program_swing_and_deemphasis(struct intel_encoder *encoder)
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
 	enum phy phy;
-	u32 tmp;
+	u32 tmp, mask, val;
 	int lane;
 
 	for_each_dsi_phy(phy, intel_dsi->phys) {
@@ -242,56 +240,35 @@ static void dsi_program_swing_and_deemphasis(struct intel_encoder *encoder)
 		 * Program voltage swing and pre-emphasis level values as per
 		 * table in BSPEC under DDI buffer programing
 		 */
+		mask = SCALING_MODE_SEL_MASK | RTERM_SELECT_MASK;
+		val = SCALING_MODE_SEL(0x2) | TAP2_DISABLE | TAP3_DISABLE |
+		      RTERM_SELECT(0x6);
 		tmp = intel_de_read(dev_priv, ICL_PORT_TX_DW5_LN(0, phy));
-		tmp &= ~(SCALING_MODE_SEL_MASK | RTERM_SELECT_MASK);
-		tmp |= SCALING_MODE_SEL(0x2);
-		tmp |= TAP2_DISABLE | TAP3_DISABLE;
-		tmp |= RTERM_SELECT(0x6);
+		tmp &= ~mask;
+		tmp |= val;
 		intel_de_write(dev_priv, ICL_PORT_TX_DW5_GRP(phy), tmp);
+		intel_de_rmw(dev_priv, ICL_PORT_TX_DW5_AUX(phy), mask, val);
 
-		tmp = intel_de_read(dev_priv, ICL_PORT_TX_DW5_AUX(phy));
-		tmp &= ~(SCALING_MODE_SEL_MASK | RTERM_SELECT_MASK);
-		tmp |= SCALING_MODE_SEL(0x2);
-		tmp |= TAP2_DISABLE | TAP3_DISABLE;
-		tmp |= RTERM_SELECT(0x6);
-		intel_de_write(dev_priv, ICL_PORT_TX_DW5_AUX(phy), tmp);
-
+		mask = SWING_SEL_LOWER_MASK | SWING_SEL_UPPER_MASK |
+		       RCOMP_SCALAR_MASK;
+		val = SWING_SEL_UPPER(0x2) | SWING_SEL_LOWER(0x2) |
+		      RCOMP_SCALAR(0x98);
 		tmp = intel_de_read(dev_priv, ICL_PORT_TX_DW2_LN(0, phy));
-		tmp &= ~(SWING_SEL_LOWER_MASK | SWING_SEL_UPPER_MASK |
-			 RCOMP_SCALAR_MASK);
-		tmp |= SWING_SEL_UPPER(0x2);
-		tmp |= SWING_SEL_LOWER(0x2);
-		tmp |= RCOMP_SCALAR(0x98);
+		tmp &= ~mask;
+		tmp |= val;
 		intel_de_write(dev_priv, ICL_PORT_TX_DW2_GRP(phy), tmp);
+		intel_de_rmw(dev_priv, ICL_PORT_TX_DW2_AUX(phy), mask, val);
 
-		tmp = intel_de_read(dev_priv, ICL_PORT_TX_DW2_AUX(phy));
-		tmp &= ~(SWING_SEL_LOWER_MASK | SWING_SEL_UPPER_MASK |
-			 RCOMP_SCALAR_MASK);
-		tmp |= SWING_SEL_UPPER(0x2);
-		tmp |= SWING_SEL_LOWER(0x2);
-		tmp |= RCOMP_SCALAR(0x98);
-		intel_de_write(dev_priv, ICL_PORT_TX_DW2_AUX(phy), tmp);
-
-		tmp = intel_de_read(dev_priv, ICL_PORT_TX_DW4_AUX(phy));
-		tmp &= ~(POST_CURSOR_1_MASK | POST_CURSOR_2_MASK |
-			 CURSOR_COEFF_MASK);
-		tmp |= POST_CURSOR_1(0x0);
-		tmp |= POST_CURSOR_2(0x0);
-		tmp |= CURSOR_COEFF(0x3f);
-		intel_de_write(dev_priv, ICL_PORT_TX_DW4_AUX(phy), tmp);
-
-		for (lane = 0; lane <= 3; lane++) {
-			/* Bspec: must not use GRP register for write */
-			tmp = intel_de_read(dev_priv,
-					    ICL_PORT_TX_DW4_LN(lane, phy));
-			tmp &= ~(POST_CURSOR_1_MASK | POST_CURSOR_2_MASK |
-				 CURSOR_COEFF_MASK);
-			tmp |= POST_CURSOR_1(0x0);
-			tmp |= POST_CURSOR_2(0x0);
-			tmp |= CURSOR_COEFF(0x3f);
-			intel_de_write(dev_priv,
-				       ICL_PORT_TX_DW4_LN(lane, phy), tmp);
-		}
+		mask = POST_CURSOR_1_MASK | POST_CURSOR_2_MASK |
+		       CURSOR_COEFF_MASK;
+		val = POST_CURSOR_1(0x0) | POST_CURSOR_2(0x0) |
+		      CURSOR_COEFF(0x3f);
+		intel_de_rmw(dev_priv, ICL_PORT_TX_DW4_AUX(phy), mask, val);
+
+		/* Bspec: must not use GRP register for write */
+		for (lane = 0; lane <= 3; lane++)
+			intel_de_rmw(dev_priv, ICL_PORT_TX_DW4_LN(lane, phy),
+				     mask, val);
 	}
 }
 
@@ -310,7 +287,6 @@ static void configure_dual_link_mode(struct intel_encoder *encoder,
 	if (intel_dsi->dual_link == DSI_DUAL_LINK_FRONT_BACK) {
 		const struct drm_display_mode *adjusted_mode =
 					&pipe_config->hw.adjusted_mode;
-		u32 dss_ctl2;
 		u16 hactive = adjusted_mode->crtc_hdisplay;
 		u16 dl_buffer_depth;
 
@@ -323,10 +299,8 @@ static void configure_dual_link_mode(struct intel_encoder *encoder,
 
 		dss_ctl1 &= ~LEFT_DL_BUF_TARGET_DEPTH_MASK;
 		dss_ctl1 |= LEFT_DL_BUF_TARGET_DEPTH(dl_buffer_depth);
-		dss_ctl2 = intel_de_read(dev_priv, DSS_CTL2);
-		dss_ctl2 &= ~RIGHT_DL_BUF_TARGET_DEPTH_MASK;
-		dss_ctl2 |= RIGHT_DL_BUF_TARGET_DEPTH(dl_buffer_depth);
-		intel_de_write(dev_priv, DSS_CTL2, dss_ctl2);
+		intel_de_rmw(dev_priv, DSS_CTL2, RIGHT_DL_BUF_TARGET_DEPTH_MASK,
+			     RIGHT_DL_BUF_TARGET_DEPTH(dl_buffer_depth));
 	} else {
 		/* Interleave */
 		dss_ctl1 |= DUAL_LINK_MODE_INTERLEAVE;
@@ -412,13 +386,10 @@ static void gen11_dsi_enable_io_power(struct intel_encoder *encoder)
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
 	enum port port;
-	u32 tmp;
 
-	for_each_dsi_port(port, intel_dsi->ports) {
-		tmp = intel_de_read(dev_priv, ICL_DSI_IO_MODECTL(port));
-		tmp |= COMBO_PHY_MODE_DSI;
-		intel_de_write(dev_priv, ICL_DSI_IO_MODECTL(port), tmp);
-	}
+	for_each_dsi_port(port, intel_dsi->ports)
+		intel_de_rmw(dev_priv, ICL_DSI_IO_MODECTL(port),
+			     0, COMBO_PHY_MODE_DSI);
 
 	get_dsi_io_power_domains(dev_priv, intel_dsi);
 }
@@ -444,26 +415,16 @@ static void gen11_dsi_config_phy_lanes_sequence(struct intel_encoder *encoder)
 
 	/* Step 4b(i) set loadgen select for transmit and aux lanes */
 	for_each_dsi_phy(phy, intel_dsi->phys) {
-		tmp = intel_de_read(dev_priv, ICL_PORT_TX_DW4_AUX(phy));
-		tmp &= ~LOADGEN_SELECT;
-		intel_de_write(dev_priv, ICL_PORT_TX_DW4_AUX(phy), tmp);
-		for (lane = 0; lane <= 3; lane++) {
-			tmp = intel_de_read(dev_priv,
-					    ICL_PORT_TX_DW4_LN(lane, phy));
-			tmp &= ~LOADGEN_SELECT;
-			if (lane != 2)
-				tmp |= LOADGEN_SELECT;
-			intel_de_write(dev_priv,
-				       ICL_PORT_TX_DW4_LN(lane, phy), tmp);
-		}
+		intel_de_rmw(dev_priv, ICL_PORT_TX_DW4_AUX(phy), LOADGEN_SELECT, 0);
+		for (lane = 0; lane <= 3; lane++)
+			intel_de_rmw(dev_priv, ICL_PORT_TX_DW4_LN(lane, phy),
+				     LOADGEN_SELECT, lane != 2 ? LOADGEN_SELECT : 0);
 	}
 
 	/* Step 4b(ii) set latency optimization for transmit and aux lanes */
 	for_each_dsi_phy(phy, intel_dsi->phys) {
-		tmp = intel_de_read(dev_priv, ICL_PORT_TX_DW2_AUX(phy));
-		tmp &= ~FRC_LATENCY_OPTIM_MASK;
-		tmp |= FRC_LATENCY_OPTIM_VAL(0x5);
-		intel_de_write(dev_priv, ICL_PORT_TX_DW2_AUX(phy), tmp);
+		intel_de_rmw(dev_priv, ICL_PORT_TX_DW2_AUX(phy),
+			     FRC_LATENCY_OPTIM_MASK, FRC_LATENCY_OPTIM_VAL(0x5));
 		tmp = intel_de_read(dev_priv, ICL_PORT_TX_DW2_LN(0, phy));
 		tmp &= ~FRC_LATENCY_OPTIM_MASK;
 		tmp |= FRC_LATENCY_OPTIM_VAL(0x5);
@@ -471,12 +432,8 @@ static void gen11_dsi_config_phy_lanes_sequence(struct intel_encoder *encoder)
 
 		/* For EHL, TGL, set latency optimization for PCS_DW1 lanes */
 		if (IS_JSL_EHL(dev_priv) || (DISPLAY_VER(dev_priv) >= 12)) {
-			tmp = intel_de_read(dev_priv,
-					    ICL_PORT_PCS_DW1_AUX(phy));
-			tmp &= ~LATENCY_OPTIM_MASK;
-			tmp |= LATENCY_OPTIM_VAL(0);
-			intel_de_write(dev_priv, ICL_PORT_PCS_DW1_AUX(phy),
-				       tmp);
+			intel_de_rmw(dev_priv, ICL_PORT_PCS_DW1_AUX(phy),
+				     LATENCY_OPTIM_MASK, LATENCY_OPTIM_VAL(0));
 
 			tmp = intel_de_read(dev_priv,
 					    ICL_PORT_PCS_DW1_LN(0, phy));
@@ -501,9 +458,7 @@ static void gen11_dsi_voltage_swing_program_seq(struct intel_encoder *encoder)
 		tmp = intel_de_read(dev_priv, ICL_PORT_PCS_DW1_LN(0, phy));
 		tmp &= ~COMMON_KEEPER_EN;
 		intel_de_write(dev_priv, ICL_PORT_PCS_DW1_GRP(phy), tmp);
-		tmp = intel_de_read(dev_priv, ICL_PORT_PCS_DW1_AUX(phy));
-		tmp &= ~COMMON_KEEPER_EN;
-		intel_de_write(dev_priv, ICL_PORT_PCS_DW1_AUX(phy), tmp);
+		intel_de_rmw(dev_priv, ICL_PORT_PCS_DW1_AUX(phy), COMMON_KEEPER_EN, 0);
 	}
 
 	/*
@@ -511,20 +466,15 @@ static void gen11_dsi_voltage_swing_program_seq(struct intel_encoder *encoder)
 	 * Note: loadgen select program is done
 	 * as part of lane phy sequence configuration
 	 */
-	for_each_dsi_phy(phy, intel_dsi->phys) {
-		tmp = intel_de_read(dev_priv, ICL_PORT_CL_DW5(phy));
-		tmp |= SUS_CLOCK_CONFIG;
-		intel_de_write(dev_priv, ICL_PORT_CL_DW5(phy), tmp);
-	}
+	for_each_dsi_phy(phy, intel_dsi->phys)
+		intel_de_rmw(dev_priv, ICL_PORT_CL_DW5(phy), 0, SUS_CLOCK_CONFIG);
 
 	/* Clear training enable to change swing values */
 	for_each_dsi_phy(phy, intel_dsi->phys) {
 		tmp = intel_de_read(dev_priv, ICL_PORT_TX_DW5_LN(0, phy));
 		tmp &= ~TX_TRAINING_EN;
 		intel_de_write(dev_priv, ICL_PORT_TX_DW5_GRP(phy), tmp);
-		tmp = intel_de_read(dev_priv, ICL_PORT_TX_DW5_AUX(phy));
-		tmp &= ~TX_TRAINING_EN;
-		intel_de_write(dev_priv, ICL_PORT_TX_DW5_AUX(phy), tmp);
+		intel_de_rmw(dev_priv, ICL_PORT_TX_DW5_AUX(phy), TX_TRAINING_EN, 0);
 	}
 
 	/* Program swing and de-emphasis */
@@ -535,9 +485,7 @@ static void gen11_dsi_voltage_swing_program_seq(struct intel_encoder *encoder)
 		tmp = intel_de_read(dev_priv, ICL_PORT_TX_DW5_LN(0, phy));
 		tmp |= TX_TRAINING_EN;
 		intel_de_write(dev_priv, ICL_PORT_TX_DW5_GRP(phy), tmp);
-		tmp = intel_de_read(dev_priv, ICL_PORT_TX_DW5_AUX(phy));
-		tmp |= TX_TRAINING_EN;
-		intel_de_write(dev_priv, ICL_PORT_TX_DW5_AUX(phy), tmp);
+		intel_de_rmw(dev_priv, ICL_PORT_TX_DW5_AUX(phy), 0, TX_TRAINING_EN);
 	}
 }
 
@@ -545,13 +493,10 @@ static void gen11_dsi_enable_ddi_buffer(struct intel_encoder *encoder)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
-	u32 tmp;
 	enum port port;
 
 	for_each_dsi_port(port, intel_dsi->ports) {
-		tmp = intel_de_read(dev_priv, DDI_BUF_CTL(port));
-		tmp |= DDI_BUF_CTL_ENABLE;
-		intel_de_write(dev_priv, DDI_BUF_CTL(port), tmp);
+		intel_de_rmw(dev_priv, DDI_BUF_CTL(port), 0, DDI_BUF_CTL_ENABLE);
 
 		if (wait_for_us(!(intel_de_read(dev_priv, DDI_BUF_CTL(port)) &
 				  DDI_BUF_IS_IDLE),
@@ -567,17 +512,13 @@ gen11_dsi_setup_dphy_timings(struct intel_encoder *encoder,
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
-	u32 tmp;
 	enum port port;
 	enum phy phy;
 
 	/* Program T-INIT master registers */
-	for_each_dsi_port(port, intel_dsi->ports) {
-		tmp = intel_de_read(dev_priv, ICL_DSI_T_INIT_MASTER(port));
-		tmp &= ~DSI_T_INIT_MASTER_MASK;
-		tmp |= intel_dsi->init_count;
-		intel_de_write(dev_priv, ICL_DSI_T_INIT_MASTER(port), tmp);
-	}
+	for_each_dsi_port(port, intel_dsi->ports)
+		intel_de_rmw(dev_priv, ICL_DSI_T_INIT_MASTER(port),
+			     DSI_T_INIT_MASTER_MASK, intel_dsi->init_count);
 
 	/* Program DPHY clock lanes timings */
 	for_each_dsi_port(port, intel_dsi->ports) {
@@ -608,31 +549,22 @@ gen11_dsi_setup_dphy_timings(struct intel_encoder *encoder,
 	if (DISPLAY_VER(dev_priv) == 11) {
 		if (afe_clk(encoder, crtc_state) <= 800000) {
 			for_each_dsi_port(port, intel_dsi->ports) {
-				tmp = intel_de_read(dev_priv,
-						    DPHY_TA_TIMING_PARAM(port));
-				tmp &= ~TA_SURE_MASK;
-				tmp |= TA_SURE_OVERRIDE | TA_SURE(0);
-				intel_de_write(dev_priv,
-					       DPHY_TA_TIMING_PARAM(port),
-					       tmp);
+				intel_de_rmw(dev_priv, DPHY_TA_TIMING_PARAM(port),
+					     TA_SURE_MASK,
+					     TA_SURE_OVERRIDE | TA_SURE(0));
 
 				/* shadow register inside display core */
-				tmp = intel_de_read(dev_priv,
-						    DSI_TA_TIMING_PARAM(port));
-				tmp &= ~TA_SURE_MASK;
-				tmp |= TA_SURE_OVERRIDE | TA_SURE(0);
-				intel_de_write(dev_priv,
-					       DSI_TA_TIMING_PARAM(port), tmp);
+				intel_de_rmw(dev_priv, DSI_TA_TIMING_PARAM(port),
+					     TA_SURE_MASK,
+					     TA_SURE_OVERRIDE | TA_SURE(0));
 			}
 		}
 	}
 
 	if (IS_JSL_EHL(dev_priv)) {
-		for_each_dsi_phy(phy, intel_dsi->phys) {
-			tmp = intel_de_read(dev_priv, ICL_DPHY_CHKN(phy));
-			tmp |= ICL_DPHY_CHKN_AFE_OVER_PPI_STRAP;
-			intel_de_write(dev_priv, ICL_DPHY_CHKN(phy), tmp);
-		}
+		for_each_dsi_phy(phy, intel_dsi->phys)
+			intel_de_rmw(dev_priv, ICL_DPHY_CHKN(phy),
+				     0, ICL_DPHY_CHKN_AFE_OVER_PPI_STRAP);
 	}
 }
 
@@ -824,11 +756,8 @@ gen11_dsi_configure_transcoder(struct intel_encoder *encoder,
 	if (intel_dsi->dual_link) {
 		for_each_dsi_port(port, intel_dsi->ports) {
 			dsi_trans = dsi_port_to_transcoder(port);
-			tmp = intel_de_read(dev_priv,
-					    TRANS_DDI_FUNC_CTL2(dsi_trans));
-			tmp |= PORT_SYNC_MODE_ENABLE;
-			intel_de_write(dev_priv,
-				       TRANS_DDI_FUNC_CTL2(dsi_trans), tmp);
+			intel_de_rmw(dev_priv, TRANS_DDI_FUNC_CTL2(dsi_trans),
+				     0, PORT_SYNC_MODE_ENABLE);
 		}
 
 		/* configure stream splitting */
@@ -1044,13 +973,10 @@ static void gen11_dsi_enable_transcoder(struct intel_encoder *encoder)
 	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
 	enum port port;
 	enum transcoder dsi_trans;
-	u32 tmp;
 
 	for_each_dsi_port(port, intel_dsi->ports) {
 		dsi_trans = dsi_port_to_transcoder(port);
-		tmp = intel_de_read(dev_priv, PIPECONF(dsi_trans));
-		tmp |= PIPECONF_ENABLE;
-		intel_de_write(dev_priv, PIPECONF(dsi_trans), tmp);
+		intel_de_rmw(dev_priv, PIPECONF(dsi_trans), 0, PIPECONF_ENABLE);
 
 		/* wait for transcoder to be enabled */
 		if (intel_de_wait_for_set(dev_priv, PIPECONF(dsi_trans),
@@ -1067,7 +993,7 @@ static void gen11_dsi_setup_timeouts(struct intel_encoder *encoder,
 	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
 	enum port port;
 	enum transcoder dsi_trans;
-	u32 tmp, hs_tx_timeout, lp_rx_timeout, ta_timeout, divisor, mul;
+	u32 hs_tx_timeout, lp_rx_timeout, ta_timeout, divisor, mul;
 
 	/*
 	 * escape clock count calculation:
@@ -1087,26 +1013,23 @@ static void gen11_dsi_setup_timeouts(struct intel_encoder *encoder,
 		dsi_trans = dsi_port_to_transcoder(port);
 
 		/* program hst_tx_timeout */
-		tmp = intel_de_read(dev_priv, DSI_HSTX_TO(dsi_trans));
-		tmp &= ~HSTX_TIMEOUT_VALUE_MASK;
-		tmp |= HSTX_TIMEOUT_VALUE(hs_tx_timeout);
-		intel_de_write(dev_priv, DSI_HSTX_TO(dsi_trans), tmp);
+		intel_de_rmw(dev_priv, DSI_HSTX_TO(dsi_trans),
+			     HSTX_TIMEOUT_VALUE_MASK,
+			     HSTX_TIMEOUT_VALUE(hs_tx_timeout));
 
 		/* FIXME: DSI_CALIB_TO */
 
 		/* program lp_rx_host timeout */
-		tmp = intel_de_read(dev_priv, DSI_LPRX_HOST_TO(dsi_trans));
-		tmp &= ~LPRX_TIMEOUT_VALUE_MASK;
-		tmp |= LPRX_TIMEOUT_VALUE(lp_rx_timeout);
-		intel_de_write(dev_priv, DSI_LPRX_HOST_TO(dsi_trans), tmp);
+		intel_de_rmw(dev_priv, DSI_LPRX_HOST_TO(dsi_trans),
+			     LPRX_TIMEOUT_VALUE_MASK,
+			     LPRX_TIMEOUT_VALUE(lp_rx_timeout));
 
 		/* FIXME: DSI_PWAIT_TO */
 
 		/* program turn around timeout */
-		tmp = intel_de_read(dev_priv, DSI_TA_TO(dsi_trans));
-		tmp &= ~TA_TIMEOUT_VALUE_MASK;
-		tmp |= TA_TIMEOUT_VALUE(ta_timeout);
-		intel_de_write(dev_priv, DSI_TA_TO(dsi_trans), tmp);
+		intel_de_rmw(dev_priv, DSI_TA_TO(dsi_trans),
+			     TA_TIMEOUT_VALUE_MASK,
+			     TA_TIMEOUT_VALUE(ta_timeout));
 	}
 }
 
@@ -1310,15 +1233,12 @@ static void gen11_dsi_disable_transcoder(struct intel_encoder *encoder)
 	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
 	enum port port;
 	enum transcoder dsi_trans;
-	u32 tmp;
 
 	for_each_dsi_port(port, intel_dsi->ports) {
 		dsi_trans = dsi_port_to_transcoder(port);
 
 		/* disable transcoder */
-		tmp = intel_de_read(dev_priv, PIPECONF(dsi_trans));
-		tmp &= ~PIPECONF_ENABLE;
-		intel_de_write(dev_priv, PIPECONF(dsi_trans), tmp);
+		intel_de_rmw(dev_priv, PIPECONF(dsi_trans), PIPECONF_ENABLE, 0);
 
 		/* wait for transcoder to be disabled */
 		if (intel_de_wait_for_clear(dev_priv, PIPECONF(dsi_trans),
@@ -1350,11 +1270,9 @@ static void gen11_dsi_deconfigure_trancoder(struct intel_encoder *encoder)
 
 	/* disable periodic update mode */
 	if (is_cmd_mode(intel_dsi)) {
-		for_each_dsi_port(port, intel_dsi->ports) {
-			tmp = intel_de_read(dev_priv, DSI_CMD_FRMCTL(port));
-			tmp &= ~DSI_PERIODIC_FRAME_UPDATE_ENABLE;
-			intel_de_write(dev_priv, DSI_CMD_FRMCTL(port), tmp);
-		}
+		for_each_dsi_port(port, intel_dsi->ports)
+			intel_de_rmw(dev_priv, DSI_CMD_FRMCTL(port),
+				     DSI_PERIODIC_FRAME_UPDATE_ENABLE, 0);
 	}
 
 	/* put dsi link in ULPS */
@@ -1374,20 +1292,16 @@ static void gen11_dsi_deconfigure_trancoder(struct intel_encoder *encoder)
 	/* disable ddi function */
 	for_each_dsi_port(port, intel_dsi->ports) {
 		dsi_trans = dsi_port_to_transcoder(port);
-		tmp = intel_de_read(dev_priv, TRANS_DDI_FUNC_CTL(dsi_trans));
-		tmp &= ~TRANS_DDI_FUNC_ENABLE;
-		intel_de_write(dev_priv, TRANS_DDI_FUNC_CTL(dsi_trans), tmp);
+		intel_de_rmw(dev_priv, TRANS_DDI_FUNC_CTL(dsi_trans),
+			     TRANS_DDI_FUNC_ENABLE, 0);
 	}
 
 	/* disable port sync mode if dual link */
 	if (intel_dsi->dual_link) {
 		for_each_dsi_port(port, intel_dsi->ports) {
 			dsi_trans = dsi_port_to_transcoder(port);
-			tmp = intel_de_read(dev_priv,
-					    TRANS_DDI_FUNC_CTL2(dsi_trans));
-			tmp &= ~PORT_SYNC_MODE_ENABLE;
-			intel_de_write(dev_priv,
-				       TRANS_DDI_FUNC_CTL2(dsi_trans), tmp);
+			intel_de_rmw(dev_priv, TRANS_DDI_FUNC_CTL2(dsi_trans),
+				     PORT_SYNC_MODE_ENABLE, 0);
 		}
 	}
 }
@@ -1396,14 +1310,11 @@ static void gen11_dsi_disable_port(struct intel_encoder *encoder)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
-	u32 tmp;
 	enum port port;
 
 	gen11_dsi_ungate_clocks(encoder);
 	for_each_dsi_port(port, intel_dsi->ports) {
-		tmp = intel_de_read(dev_priv, DDI_BUF_CTL(port));
-		tmp &= ~DDI_BUF_CTL_ENABLE;
-		intel_de_write(dev_priv, DDI_BUF_CTL(port), tmp);
+		intel_de_rmw(dev_priv, DDI_BUF_CTL(port), DDI_BUF_CTL_ENABLE, 0);
 
 		if (wait_for_us((intel_de_read(dev_priv, DDI_BUF_CTL(port)) &
 				 DDI_BUF_IS_IDLE),
@@ -1420,7 +1331,6 @@ static void gen11_dsi_disable_io_power(struct intel_encoder *encoder)
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
 	enum port port;
-	u32 tmp;
 
 	for_each_dsi_port(port, intel_dsi->ports) {
 		intel_wakeref_t wakeref;
@@ -1434,11 +1344,9 @@ static void gen11_dsi_disable_io_power(struct intel_encoder *encoder)
 	}
 
 	/* set mode to DDI */
-	for_each_dsi_port(port, intel_dsi->ports) {
-		tmp = intel_de_read(dev_priv, ICL_DSI_IO_MODECTL(port));
-		tmp &= ~COMBO_PHY_MODE_DSI;
-		intel_de_write(dev_priv, ICL_DSI_IO_MODECTL(port), tmp);
-	}
+	for_each_dsi_port(port, intel_dsi->ports)
+		intel_de_rmw(dev_priv, ICL_DSI_IO_MODECTL(port),
+			     COMBO_PHY_MODE_DSI, 0);
 }
 
 static void gen11_dsi_disable(struct intel_atomic_state *state,
-- 
2.34.1


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

* Re: [Intel-gfx] [PATCH] drm/i915/display/dsi: use intel_de_rmw if possible
  2022-12-19 13:08 [Intel-gfx] [PATCH] drm/i915/display/dsi: use intel_de_rmw if possible Andrzej Hajda
@ 2022-12-20  9:26 ` Jani Nikula
  2023-01-30 13:00   ` Jani Nikula
  2022-12-21 14:05 ` [Intel-gfx] ✓ Fi.CI.BAT: success for " Patchwork
  2022-12-21 15:56 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
  2 siblings, 1 reply; 5+ messages in thread
From: Jani Nikula @ 2022-12-20  9:26 UTC (permalink / raw)
  To: Andrzej Hajda, intel-gfx; +Cc: Andrzej Hajda, Rodrigo Vivi

On Mon, 19 Dec 2022, Andrzej Hajda <andrzej.hajda@intel.com> wrote:
> The helper makes the code more compact and readable.
>
> Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  drivers/gpu/drm/i915/display/icl_dsi.c | 256 ++++++++-----------------
>  1 file changed, 82 insertions(+), 174 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
> index ae14c794c4bc09..b02ac9d2b1e4a2 100644
> --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> @@ -207,7 +207,7 @@ void icl_dsi_frame_update(struct intel_crtc_state *crtc_state)
>  {
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> -	u32 tmp, mode_flags;
> +	u32 mode_flags;
>  	enum port port;
>  
>  	mode_flags = crtc_state->mode_flags;
> @@ -224,9 +224,7 @@ void icl_dsi_frame_update(struct intel_crtc_state *crtc_state)
>  	else
>  		return;
>  
> -	tmp = intel_de_read(dev_priv, DSI_CMD_FRMCTL(port));
> -	tmp |= DSI_FRAME_UPDATE_REQUEST;
> -	intel_de_write(dev_priv, DSI_CMD_FRMCTL(port), tmp);
> +	intel_de_rmw(dev_priv, DSI_CMD_FRMCTL(port), 0, DSI_FRAME_UPDATE_REQUEST);
>  }
>  
>  static void dsi_program_swing_and_deemphasis(struct intel_encoder *encoder)
> @@ -234,7 +232,7 @@ static void dsi_program_swing_and_deemphasis(struct intel_encoder *encoder)
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
>  	enum phy phy;
> -	u32 tmp;
> +	u32 tmp, mask, val;
>  	int lane;
>  
>  	for_each_dsi_phy(phy, intel_dsi->phys) {
> @@ -242,56 +240,35 @@ static void dsi_program_swing_and_deemphasis(struct intel_encoder *encoder)
>  		 * Program voltage swing and pre-emphasis level values as per
>  		 * table in BSPEC under DDI buffer programing
>  		 */
> +		mask = SCALING_MODE_SEL_MASK | RTERM_SELECT_MASK;
> +		val = SCALING_MODE_SEL(0x2) | TAP2_DISABLE | TAP3_DISABLE |
> +		      RTERM_SELECT(0x6);
>  		tmp = intel_de_read(dev_priv, ICL_PORT_TX_DW5_LN(0, phy));
> -		tmp &= ~(SCALING_MODE_SEL_MASK | RTERM_SELECT_MASK);
> -		tmp |= SCALING_MODE_SEL(0x2);
> -		tmp |= TAP2_DISABLE | TAP3_DISABLE;
> -		tmp |= RTERM_SELECT(0x6);
> +		tmp &= ~mask;
> +		tmp |= val;
>  		intel_de_write(dev_priv, ICL_PORT_TX_DW5_GRP(phy), tmp);
> +		intel_de_rmw(dev_priv, ICL_PORT_TX_DW5_AUX(phy), mask, val);
>  
> -		tmp = intel_de_read(dev_priv, ICL_PORT_TX_DW5_AUX(phy));
> -		tmp &= ~(SCALING_MODE_SEL_MASK | RTERM_SELECT_MASK);
> -		tmp |= SCALING_MODE_SEL(0x2);
> -		tmp |= TAP2_DISABLE | TAP3_DISABLE;
> -		tmp |= RTERM_SELECT(0x6);
> -		intel_de_write(dev_priv, ICL_PORT_TX_DW5_AUX(phy), tmp);
> -
> +		mask = SWING_SEL_LOWER_MASK | SWING_SEL_UPPER_MASK |
> +		       RCOMP_SCALAR_MASK;
> +		val = SWING_SEL_UPPER(0x2) | SWING_SEL_LOWER(0x2) |
> +		      RCOMP_SCALAR(0x98);
>  		tmp = intel_de_read(dev_priv, ICL_PORT_TX_DW2_LN(0, phy));
> -		tmp &= ~(SWING_SEL_LOWER_MASK | SWING_SEL_UPPER_MASK |
> -			 RCOMP_SCALAR_MASK);
> -		tmp |= SWING_SEL_UPPER(0x2);
> -		tmp |= SWING_SEL_LOWER(0x2);
> -		tmp |= RCOMP_SCALAR(0x98);
> +		tmp &= ~mask;
> +		tmp |= val;
>  		intel_de_write(dev_priv, ICL_PORT_TX_DW2_GRP(phy), tmp);
> +		intel_de_rmw(dev_priv, ICL_PORT_TX_DW2_AUX(phy), mask, val);
>  
> -		tmp = intel_de_read(dev_priv, ICL_PORT_TX_DW2_AUX(phy));
> -		tmp &= ~(SWING_SEL_LOWER_MASK | SWING_SEL_UPPER_MASK |
> -			 RCOMP_SCALAR_MASK);
> -		tmp |= SWING_SEL_UPPER(0x2);
> -		tmp |= SWING_SEL_LOWER(0x2);
> -		tmp |= RCOMP_SCALAR(0x98);
> -		intel_de_write(dev_priv, ICL_PORT_TX_DW2_AUX(phy), tmp);
> -
> -		tmp = intel_de_read(dev_priv, ICL_PORT_TX_DW4_AUX(phy));
> -		tmp &= ~(POST_CURSOR_1_MASK | POST_CURSOR_2_MASK |
> -			 CURSOR_COEFF_MASK);
> -		tmp |= POST_CURSOR_1(0x0);
> -		tmp |= POST_CURSOR_2(0x0);
> -		tmp |= CURSOR_COEFF(0x3f);
> -		intel_de_write(dev_priv, ICL_PORT_TX_DW4_AUX(phy), tmp);
> -
> -		for (lane = 0; lane <= 3; lane++) {
> -			/* Bspec: must not use GRP register for write */
> -			tmp = intel_de_read(dev_priv,
> -					    ICL_PORT_TX_DW4_LN(lane, phy));
> -			tmp &= ~(POST_CURSOR_1_MASK | POST_CURSOR_2_MASK |
> -				 CURSOR_COEFF_MASK);
> -			tmp |= POST_CURSOR_1(0x0);
> -			tmp |= POST_CURSOR_2(0x0);
> -			tmp |= CURSOR_COEFF(0x3f);
> -			intel_de_write(dev_priv,
> -				       ICL_PORT_TX_DW4_LN(lane, phy), tmp);
> -		}
> +		mask = POST_CURSOR_1_MASK | POST_CURSOR_2_MASK |
> +		       CURSOR_COEFF_MASK;
> +		val = POST_CURSOR_1(0x0) | POST_CURSOR_2(0x0) |
> +		      CURSOR_COEFF(0x3f);
> +		intel_de_rmw(dev_priv, ICL_PORT_TX_DW4_AUX(phy), mask, val);
> +
> +		/* Bspec: must not use GRP register for write */
> +		for (lane = 0; lane <= 3; lane++)
> +			intel_de_rmw(dev_priv, ICL_PORT_TX_DW4_LN(lane, phy),
> +				     mask, val);
>  	}
>  }
>  
> @@ -310,7 +287,6 @@ static void configure_dual_link_mode(struct intel_encoder *encoder,
>  	if (intel_dsi->dual_link == DSI_DUAL_LINK_FRONT_BACK) {
>  		const struct drm_display_mode *adjusted_mode =
>  					&pipe_config->hw.adjusted_mode;
> -		u32 dss_ctl2;
>  		u16 hactive = adjusted_mode->crtc_hdisplay;
>  		u16 dl_buffer_depth;
>  
> @@ -323,10 +299,8 @@ static void configure_dual_link_mode(struct intel_encoder *encoder,
>  
>  		dss_ctl1 &= ~LEFT_DL_BUF_TARGET_DEPTH_MASK;
>  		dss_ctl1 |= LEFT_DL_BUF_TARGET_DEPTH(dl_buffer_depth);
> -		dss_ctl2 = intel_de_read(dev_priv, DSS_CTL2);
> -		dss_ctl2 &= ~RIGHT_DL_BUF_TARGET_DEPTH_MASK;
> -		dss_ctl2 |= RIGHT_DL_BUF_TARGET_DEPTH(dl_buffer_depth);
> -		intel_de_write(dev_priv, DSS_CTL2, dss_ctl2);
> +		intel_de_rmw(dev_priv, DSS_CTL2, RIGHT_DL_BUF_TARGET_DEPTH_MASK,
> +			     RIGHT_DL_BUF_TARGET_DEPTH(dl_buffer_depth));
>  	} else {
>  		/* Interleave */
>  		dss_ctl1 |= DUAL_LINK_MODE_INTERLEAVE;
> @@ -412,13 +386,10 @@ static void gen11_dsi_enable_io_power(struct intel_encoder *encoder)
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
>  	enum port port;
> -	u32 tmp;
>  
> -	for_each_dsi_port(port, intel_dsi->ports) {
> -		tmp = intel_de_read(dev_priv, ICL_DSI_IO_MODECTL(port));
> -		tmp |= COMBO_PHY_MODE_DSI;
> -		intel_de_write(dev_priv, ICL_DSI_IO_MODECTL(port), tmp);
> -	}
> +	for_each_dsi_port(port, intel_dsi->ports)
> +		intel_de_rmw(dev_priv, ICL_DSI_IO_MODECTL(port),
> +			     0, COMBO_PHY_MODE_DSI);
>  
>  	get_dsi_io_power_domains(dev_priv, intel_dsi);
>  }
> @@ -444,26 +415,16 @@ static void gen11_dsi_config_phy_lanes_sequence(struct intel_encoder *encoder)
>  
>  	/* Step 4b(i) set loadgen select for transmit and aux lanes */
>  	for_each_dsi_phy(phy, intel_dsi->phys) {
> -		tmp = intel_de_read(dev_priv, ICL_PORT_TX_DW4_AUX(phy));
> -		tmp &= ~LOADGEN_SELECT;
> -		intel_de_write(dev_priv, ICL_PORT_TX_DW4_AUX(phy), tmp);
> -		for (lane = 0; lane <= 3; lane++) {
> -			tmp = intel_de_read(dev_priv,
> -					    ICL_PORT_TX_DW4_LN(lane, phy));
> -			tmp &= ~LOADGEN_SELECT;
> -			if (lane != 2)
> -				tmp |= LOADGEN_SELECT;
> -			intel_de_write(dev_priv,
> -				       ICL_PORT_TX_DW4_LN(lane, phy), tmp);
> -		}
> +		intel_de_rmw(dev_priv, ICL_PORT_TX_DW4_AUX(phy), LOADGEN_SELECT, 0);
> +		for (lane = 0; lane <= 3; lane++)
> +			intel_de_rmw(dev_priv, ICL_PORT_TX_DW4_LN(lane, phy),
> +				     LOADGEN_SELECT, lane != 2 ? LOADGEN_SELECT : 0);
>  	}
>  
>  	/* Step 4b(ii) set latency optimization for transmit and aux lanes */
>  	for_each_dsi_phy(phy, intel_dsi->phys) {
> -		tmp = intel_de_read(dev_priv, ICL_PORT_TX_DW2_AUX(phy));
> -		tmp &= ~FRC_LATENCY_OPTIM_MASK;
> -		tmp |= FRC_LATENCY_OPTIM_VAL(0x5);
> -		intel_de_write(dev_priv, ICL_PORT_TX_DW2_AUX(phy), tmp);
> +		intel_de_rmw(dev_priv, ICL_PORT_TX_DW2_AUX(phy),
> +			     FRC_LATENCY_OPTIM_MASK, FRC_LATENCY_OPTIM_VAL(0x5));
>  		tmp = intel_de_read(dev_priv, ICL_PORT_TX_DW2_LN(0, phy));
>  		tmp &= ~FRC_LATENCY_OPTIM_MASK;
>  		tmp |= FRC_LATENCY_OPTIM_VAL(0x5);
> @@ -471,12 +432,8 @@ static void gen11_dsi_config_phy_lanes_sequence(struct intel_encoder *encoder)
>  
>  		/* For EHL, TGL, set latency optimization for PCS_DW1 lanes */
>  		if (IS_JSL_EHL(dev_priv) || (DISPLAY_VER(dev_priv) >= 12)) {
> -			tmp = intel_de_read(dev_priv,
> -					    ICL_PORT_PCS_DW1_AUX(phy));
> -			tmp &= ~LATENCY_OPTIM_MASK;
> -			tmp |= LATENCY_OPTIM_VAL(0);
> -			intel_de_write(dev_priv, ICL_PORT_PCS_DW1_AUX(phy),
> -				       tmp);
> +			intel_de_rmw(dev_priv, ICL_PORT_PCS_DW1_AUX(phy),
> +				     LATENCY_OPTIM_MASK, LATENCY_OPTIM_VAL(0));
>  
>  			tmp = intel_de_read(dev_priv,
>  					    ICL_PORT_PCS_DW1_LN(0, phy));
> @@ -501,9 +458,7 @@ static void gen11_dsi_voltage_swing_program_seq(struct intel_encoder *encoder)
>  		tmp = intel_de_read(dev_priv, ICL_PORT_PCS_DW1_LN(0, phy));
>  		tmp &= ~COMMON_KEEPER_EN;
>  		intel_de_write(dev_priv, ICL_PORT_PCS_DW1_GRP(phy), tmp);
> -		tmp = intel_de_read(dev_priv, ICL_PORT_PCS_DW1_AUX(phy));
> -		tmp &= ~COMMON_KEEPER_EN;
> -		intel_de_write(dev_priv, ICL_PORT_PCS_DW1_AUX(phy), tmp);
> +		intel_de_rmw(dev_priv, ICL_PORT_PCS_DW1_AUX(phy), COMMON_KEEPER_EN, 0);
>  	}
>  
>  	/*
> @@ -511,20 +466,15 @@ static void gen11_dsi_voltage_swing_program_seq(struct intel_encoder *encoder)
>  	 * Note: loadgen select program is done
>  	 * as part of lane phy sequence configuration
>  	 */
> -	for_each_dsi_phy(phy, intel_dsi->phys) {
> -		tmp = intel_de_read(dev_priv, ICL_PORT_CL_DW5(phy));
> -		tmp |= SUS_CLOCK_CONFIG;
> -		intel_de_write(dev_priv, ICL_PORT_CL_DW5(phy), tmp);
> -	}
> +	for_each_dsi_phy(phy, intel_dsi->phys)
> +		intel_de_rmw(dev_priv, ICL_PORT_CL_DW5(phy), 0, SUS_CLOCK_CONFIG);
>  
>  	/* Clear training enable to change swing values */
>  	for_each_dsi_phy(phy, intel_dsi->phys) {
>  		tmp = intel_de_read(dev_priv, ICL_PORT_TX_DW5_LN(0, phy));
>  		tmp &= ~TX_TRAINING_EN;
>  		intel_de_write(dev_priv, ICL_PORT_TX_DW5_GRP(phy), tmp);
> -		tmp = intel_de_read(dev_priv, ICL_PORT_TX_DW5_AUX(phy));
> -		tmp &= ~TX_TRAINING_EN;
> -		intel_de_write(dev_priv, ICL_PORT_TX_DW5_AUX(phy), tmp);
> +		intel_de_rmw(dev_priv, ICL_PORT_TX_DW5_AUX(phy), TX_TRAINING_EN, 0);
>  	}
>  
>  	/* Program swing and de-emphasis */
> @@ -535,9 +485,7 @@ static void gen11_dsi_voltage_swing_program_seq(struct intel_encoder *encoder)
>  		tmp = intel_de_read(dev_priv, ICL_PORT_TX_DW5_LN(0, phy));
>  		tmp |= TX_TRAINING_EN;
>  		intel_de_write(dev_priv, ICL_PORT_TX_DW5_GRP(phy), tmp);
> -		tmp = intel_de_read(dev_priv, ICL_PORT_TX_DW5_AUX(phy));
> -		tmp |= TX_TRAINING_EN;
> -		intel_de_write(dev_priv, ICL_PORT_TX_DW5_AUX(phy), tmp);
> +		intel_de_rmw(dev_priv, ICL_PORT_TX_DW5_AUX(phy), 0, TX_TRAINING_EN);
>  	}
>  }
>  
> @@ -545,13 +493,10 @@ static void gen11_dsi_enable_ddi_buffer(struct intel_encoder *encoder)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
> -	u32 tmp;
>  	enum port port;
>  
>  	for_each_dsi_port(port, intel_dsi->ports) {
> -		tmp = intel_de_read(dev_priv, DDI_BUF_CTL(port));
> -		tmp |= DDI_BUF_CTL_ENABLE;
> -		intel_de_write(dev_priv, DDI_BUF_CTL(port), tmp);
> +		intel_de_rmw(dev_priv, DDI_BUF_CTL(port), 0, DDI_BUF_CTL_ENABLE);
>  
>  		if (wait_for_us(!(intel_de_read(dev_priv, DDI_BUF_CTL(port)) &
>  				  DDI_BUF_IS_IDLE),
> @@ -567,17 +512,13 @@ gen11_dsi_setup_dphy_timings(struct intel_encoder *encoder,
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
> -	u32 tmp;
>  	enum port port;
>  	enum phy phy;
>  
>  	/* Program T-INIT master registers */
> -	for_each_dsi_port(port, intel_dsi->ports) {
> -		tmp = intel_de_read(dev_priv, ICL_DSI_T_INIT_MASTER(port));
> -		tmp &= ~DSI_T_INIT_MASTER_MASK;
> -		tmp |= intel_dsi->init_count;
> -		intel_de_write(dev_priv, ICL_DSI_T_INIT_MASTER(port), tmp);
> -	}
> +	for_each_dsi_port(port, intel_dsi->ports)
> +		intel_de_rmw(dev_priv, ICL_DSI_T_INIT_MASTER(port),
> +			     DSI_T_INIT_MASTER_MASK, intel_dsi->init_count);
>  
>  	/* Program DPHY clock lanes timings */
>  	for_each_dsi_port(port, intel_dsi->ports) {
> @@ -608,31 +549,22 @@ gen11_dsi_setup_dphy_timings(struct intel_encoder *encoder,
>  	if (DISPLAY_VER(dev_priv) == 11) {
>  		if (afe_clk(encoder, crtc_state) <= 800000) {
>  			for_each_dsi_port(port, intel_dsi->ports) {
> -				tmp = intel_de_read(dev_priv,
> -						    DPHY_TA_TIMING_PARAM(port));
> -				tmp &= ~TA_SURE_MASK;
> -				tmp |= TA_SURE_OVERRIDE | TA_SURE(0);
> -				intel_de_write(dev_priv,
> -					       DPHY_TA_TIMING_PARAM(port),
> -					       tmp);
> +				intel_de_rmw(dev_priv, DPHY_TA_TIMING_PARAM(port),
> +					     TA_SURE_MASK,
> +					     TA_SURE_OVERRIDE | TA_SURE(0));
>  
>  				/* shadow register inside display core */
> -				tmp = intel_de_read(dev_priv,
> -						    DSI_TA_TIMING_PARAM(port));
> -				tmp &= ~TA_SURE_MASK;
> -				tmp |= TA_SURE_OVERRIDE | TA_SURE(0);
> -				intel_de_write(dev_priv,
> -					       DSI_TA_TIMING_PARAM(port), tmp);
> +				intel_de_rmw(dev_priv, DSI_TA_TIMING_PARAM(port),
> +					     TA_SURE_MASK,
> +					     TA_SURE_OVERRIDE | TA_SURE(0));
>  			}
>  		}
>  	}
>  
>  	if (IS_JSL_EHL(dev_priv)) {
> -		for_each_dsi_phy(phy, intel_dsi->phys) {
> -			tmp = intel_de_read(dev_priv, ICL_DPHY_CHKN(phy));
> -			tmp |= ICL_DPHY_CHKN_AFE_OVER_PPI_STRAP;
> -			intel_de_write(dev_priv, ICL_DPHY_CHKN(phy), tmp);
> -		}
> +		for_each_dsi_phy(phy, intel_dsi->phys)
> +			intel_de_rmw(dev_priv, ICL_DPHY_CHKN(phy),
> +				     0, ICL_DPHY_CHKN_AFE_OVER_PPI_STRAP);
>  	}
>  }
>  
> @@ -824,11 +756,8 @@ gen11_dsi_configure_transcoder(struct intel_encoder *encoder,
>  	if (intel_dsi->dual_link) {
>  		for_each_dsi_port(port, intel_dsi->ports) {
>  			dsi_trans = dsi_port_to_transcoder(port);
> -			tmp = intel_de_read(dev_priv,
> -					    TRANS_DDI_FUNC_CTL2(dsi_trans));
> -			tmp |= PORT_SYNC_MODE_ENABLE;
> -			intel_de_write(dev_priv,
> -				       TRANS_DDI_FUNC_CTL2(dsi_trans), tmp);
> +			intel_de_rmw(dev_priv, TRANS_DDI_FUNC_CTL2(dsi_trans),
> +				     0, PORT_SYNC_MODE_ENABLE);
>  		}
>  
>  		/* configure stream splitting */
> @@ -1044,13 +973,10 @@ static void gen11_dsi_enable_transcoder(struct intel_encoder *encoder)
>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
>  	enum port port;
>  	enum transcoder dsi_trans;
> -	u32 tmp;
>  
>  	for_each_dsi_port(port, intel_dsi->ports) {
>  		dsi_trans = dsi_port_to_transcoder(port);
> -		tmp = intel_de_read(dev_priv, PIPECONF(dsi_trans));
> -		tmp |= PIPECONF_ENABLE;
> -		intel_de_write(dev_priv, PIPECONF(dsi_trans), tmp);
> +		intel_de_rmw(dev_priv, PIPECONF(dsi_trans), 0, PIPECONF_ENABLE);
>  
>  		/* wait for transcoder to be enabled */
>  		if (intel_de_wait_for_set(dev_priv, PIPECONF(dsi_trans),
> @@ -1067,7 +993,7 @@ static void gen11_dsi_setup_timeouts(struct intel_encoder *encoder,
>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
>  	enum port port;
>  	enum transcoder dsi_trans;
> -	u32 tmp, hs_tx_timeout, lp_rx_timeout, ta_timeout, divisor, mul;
> +	u32 hs_tx_timeout, lp_rx_timeout, ta_timeout, divisor, mul;
>  
>  	/*
>  	 * escape clock count calculation:
> @@ -1087,26 +1013,23 @@ static void gen11_dsi_setup_timeouts(struct intel_encoder *encoder,
>  		dsi_trans = dsi_port_to_transcoder(port);
>  
>  		/* program hst_tx_timeout */
> -		tmp = intel_de_read(dev_priv, DSI_HSTX_TO(dsi_trans));
> -		tmp &= ~HSTX_TIMEOUT_VALUE_MASK;
> -		tmp |= HSTX_TIMEOUT_VALUE(hs_tx_timeout);
> -		intel_de_write(dev_priv, DSI_HSTX_TO(dsi_trans), tmp);
> +		intel_de_rmw(dev_priv, DSI_HSTX_TO(dsi_trans),
> +			     HSTX_TIMEOUT_VALUE_MASK,
> +			     HSTX_TIMEOUT_VALUE(hs_tx_timeout));
>  
>  		/* FIXME: DSI_CALIB_TO */
>  
>  		/* program lp_rx_host timeout */
> -		tmp = intel_de_read(dev_priv, DSI_LPRX_HOST_TO(dsi_trans));
> -		tmp &= ~LPRX_TIMEOUT_VALUE_MASK;
> -		tmp |= LPRX_TIMEOUT_VALUE(lp_rx_timeout);
> -		intel_de_write(dev_priv, DSI_LPRX_HOST_TO(dsi_trans), tmp);
> +		intel_de_rmw(dev_priv, DSI_LPRX_HOST_TO(dsi_trans),
> +			     LPRX_TIMEOUT_VALUE_MASK,
> +			     LPRX_TIMEOUT_VALUE(lp_rx_timeout));
>  
>  		/* FIXME: DSI_PWAIT_TO */
>  
>  		/* program turn around timeout */
> -		tmp = intel_de_read(dev_priv, DSI_TA_TO(dsi_trans));
> -		tmp &= ~TA_TIMEOUT_VALUE_MASK;
> -		tmp |= TA_TIMEOUT_VALUE(ta_timeout);
> -		intel_de_write(dev_priv, DSI_TA_TO(dsi_trans), tmp);
> +		intel_de_rmw(dev_priv, DSI_TA_TO(dsi_trans),
> +			     TA_TIMEOUT_VALUE_MASK,
> +			     TA_TIMEOUT_VALUE(ta_timeout));
>  	}
>  }
>  
> @@ -1310,15 +1233,12 @@ static void gen11_dsi_disable_transcoder(struct intel_encoder *encoder)
>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
>  	enum port port;
>  	enum transcoder dsi_trans;
> -	u32 tmp;
>  
>  	for_each_dsi_port(port, intel_dsi->ports) {
>  		dsi_trans = dsi_port_to_transcoder(port);
>  
>  		/* disable transcoder */
> -		tmp = intel_de_read(dev_priv, PIPECONF(dsi_trans));
> -		tmp &= ~PIPECONF_ENABLE;
> -		intel_de_write(dev_priv, PIPECONF(dsi_trans), tmp);
> +		intel_de_rmw(dev_priv, PIPECONF(dsi_trans), PIPECONF_ENABLE, 0);
>  
>  		/* wait for transcoder to be disabled */
>  		if (intel_de_wait_for_clear(dev_priv, PIPECONF(dsi_trans),
> @@ -1350,11 +1270,9 @@ static void gen11_dsi_deconfigure_trancoder(struct intel_encoder *encoder)
>  
>  	/* disable periodic update mode */
>  	if (is_cmd_mode(intel_dsi)) {
> -		for_each_dsi_port(port, intel_dsi->ports) {
> -			tmp = intel_de_read(dev_priv, DSI_CMD_FRMCTL(port));
> -			tmp &= ~DSI_PERIODIC_FRAME_UPDATE_ENABLE;
> -			intel_de_write(dev_priv, DSI_CMD_FRMCTL(port), tmp);
> -		}
> +		for_each_dsi_port(port, intel_dsi->ports)
> +			intel_de_rmw(dev_priv, DSI_CMD_FRMCTL(port),
> +				     DSI_PERIODIC_FRAME_UPDATE_ENABLE, 0);
>  	}
>  
>  	/* put dsi link in ULPS */
> @@ -1374,20 +1292,16 @@ static void gen11_dsi_deconfigure_trancoder(struct intel_encoder *encoder)
>  	/* disable ddi function */
>  	for_each_dsi_port(port, intel_dsi->ports) {
>  		dsi_trans = dsi_port_to_transcoder(port);
> -		tmp = intel_de_read(dev_priv, TRANS_DDI_FUNC_CTL(dsi_trans));
> -		tmp &= ~TRANS_DDI_FUNC_ENABLE;
> -		intel_de_write(dev_priv, TRANS_DDI_FUNC_CTL(dsi_trans), tmp);
> +		intel_de_rmw(dev_priv, TRANS_DDI_FUNC_CTL(dsi_trans),
> +			     TRANS_DDI_FUNC_ENABLE, 0);
>  	}
>  
>  	/* disable port sync mode if dual link */
>  	if (intel_dsi->dual_link) {
>  		for_each_dsi_port(port, intel_dsi->ports) {
>  			dsi_trans = dsi_port_to_transcoder(port);
> -			tmp = intel_de_read(dev_priv,
> -					    TRANS_DDI_FUNC_CTL2(dsi_trans));
> -			tmp &= ~PORT_SYNC_MODE_ENABLE;
> -			intel_de_write(dev_priv,
> -				       TRANS_DDI_FUNC_CTL2(dsi_trans), tmp);
> +			intel_de_rmw(dev_priv, TRANS_DDI_FUNC_CTL2(dsi_trans),
> +				     PORT_SYNC_MODE_ENABLE, 0);
>  		}
>  	}
>  }
> @@ -1396,14 +1310,11 @@ static void gen11_dsi_disable_port(struct intel_encoder *encoder)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
> -	u32 tmp;
>  	enum port port;
>  
>  	gen11_dsi_ungate_clocks(encoder);
>  	for_each_dsi_port(port, intel_dsi->ports) {
> -		tmp = intel_de_read(dev_priv, DDI_BUF_CTL(port));
> -		tmp &= ~DDI_BUF_CTL_ENABLE;
> -		intel_de_write(dev_priv, DDI_BUF_CTL(port), tmp);
> +		intel_de_rmw(dev_priv, DDI_BUF_CTL(port), DDI_BUF_CTL_ENABLE, 0);
>  
>  		if (wait_for_us((intel_de_read(dev_priv, DDI_BUF_CTL(port)) &
>  				 DDI_BUF_IS_IDLE),
> @@ -1420,7 +1331,6 @@ static void gen11_dsi_disable_io_power(struct intel_encoder *encoder)
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
>  	enum port port;
> -	u32 tmp;
>  
>  	for_each_dsi_port(port, intel_dsi->ports) {
>  		intel_wakeref_t wakeref;
> @@ -1434,11 +1344,9 @@ static void gen11_dsi_disable_io_power(struct intel_encoder *encoder)
>  	}
>  
>  	/* set mode to DDI */
> -	for_each_dsi_port(port, intel_dsi->ports) {
> -		tmp = intel_de_read(dev_priv, ICL_DSI_IO_MODECTL(port));
> -		tmp &= ~COMBO_PHY_MODE_DSI;
> -		intel_de_write(dev_priv, ICL_DSI_IO_MODECTL(port), tmp);
> -	}
> +	for_each_dsi_port(port, intel_dsi->ports)
> +		intel_de_rmw(dev_priv, ICL_DSI_IO_MODECTL(port),
> +			     COMBO_PHY_MODE_DSI, 0);
>  }
>  
>  static void gen11_dsi_disable(struct intel_atomic_state *state,

-- 
Jani Nikula, Intel Open Source Graphics Center

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

* [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/display/dsi: use intel_de_rmw if possible
  2022-12-19 13:08 [Intel-gfx] [PATCH] drm/i915/display/dsi: use intel_de_rmw if possible Andrzej Hajda
  2022-12-20  9:26 ` Jani Nikula
@ 2022-12-21 14:05 ` Patchwork
  2022-12-21 15:56 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
  2 siblings, 0 replies; 5+ messages in thread
From: Patchwork @ 2022-12-21 14:05 UTC (permalink / raw)
  To: Andrzej Hajda; +Cc: intel-gfx

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

== Series Details ==

Series: drm/i915/display/dsi: use intel_de_rmw if possible
URL   : https://patchwork.freedesktop.org/series/112062/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_12517 -> Patchwork_112062v1
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

Participating hosts (41 -> 46)
------------------------------

  Additional (5): bat-dg1-7 bat-dg1-6 bat-dg2-oem1 bat-adln-1 bat-atsm-1 

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

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

### IGT changes ###

#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@gem_exec_suspend@basic-s3@smem:
    - {bat-dg2-oem1}:     NOTRUN -> [DMESG-WARN][1] +3 similar issues
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112062v1/bat-dg2-oem1/igt@gem_exec_suspend@basic-s3@smem.html

  * igt@i915_pm_rpm@module-reload:
    - {bat-dg2-oem1}:     NOTRUN -> [SKIP][2] +2 similar issues
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112062v1/bat-dg2-oem1/igt@i915_pm_rpm@module-reload.html

  * igt@i915_selftest@live@hangcheck:
    - {bat-atsm-1}:       NOTRUN -> [FAIL][3] +37 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112062v1/bat-atsm-1/igt@i915_selftest@live@hangcheck.html

  * igt@kms_pipe_crc_basic@read-crc-frame-sequence:
    - {bat-atsm-1}:       NOTRUN -> [SKIP][4] +7 similar issues
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112062v1/bat-atsm-1/igt@kms_pipe_crc_basic@read-crc-frame-sequence.html

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

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

### CI changes ###

#### Possible fixes ####

  * boot:
    - fi-ilk-650:         [FAIL][5] ([i915#7350]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12517/fi-ilk-650/boot.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112062v1/fi-ilk-650/boot.html

  

### IGT changes ###

#### Issues hit ####

  * igt@gem_mmap@basic:
    - bat-dg1-6:          NOTRUN -> [SKIP][7] ([i915#4083])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112062v1/bat-dg1-6/igt@gem_mmap@basic.html

  * igt@gem_render_tiled_blits@basic:
    - bat-dg1-6:          NOTRUN -> [SKIP][8] ([i915#4079]) +1 similar issue
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112062v1/bat-dg1-6/igt@gem_render_tiled_blits@basic.html

  * igt@gem_tiled_fence_blits@basic:
    - bat-dg1-6:          NOTRUN -> [SKIP][9] ([i915#4077]) +2 similar issues
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112062v1/bat-dg1-6/igt@gem_tiled_fence_blits@basic.html

  * igt@i915_pm_backlight@basic-brightness:
    - bat-dg1-6:          NOTRUN -> [SKIP][10] ([i915#7561])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112062v1/bat-dg1-6/igt@i915_pm_backlight@basic-brightness.html

  * igt@i915_pm_rpm@basic-pci-d3-state:
    - fi-ilk-650:         NOTRUN -> [SKIP][11] ([fdo#109271]) +20 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112062v1/fi-ilk-650/igt@i915_pm_rpm@basic-pci-d3-state.html

  * igt@i915_pm_rps@basic-api:
    - bat-dg1-6:          NOTRUN -> [SKIP][12] ([i915#6621])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112062v1/bat-dg1-6/igt@i915_pm_rps@basic-api.html

  * igt@i915_selftest@live@gt_heartbeat:
    - fi-skl-6700k2:      [PASS][13] -> [DMESG-FAIL][14] ([i915#5334])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12517/fi-skl-6700k2/igt@i915_selftest@live@gt_heartbeat.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112062v1/fi-skl-6700k2/igt@i915_selftest@live@gt_heartbeat.html

  * igt@kms_addfb_basic@basic-y-tiled-legacy:
    - bat-dg1-6:          NOTRUN -> [SKIP][15] ([i915#4215])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112062v1/bat-dg1-6/igt@kms_addfb_basic@basic-y-tiled-legacy.html

  * igt@kms_addfb_basic@tile-pitch-mismatch:
    - bat-dg1-6:          NOTRUN -> [SKIP][16] ([i915#4212]) +7 similar issues
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112062v1/bat-dg1-6/igt@kms_addfb_basic@tile-pitch-mismatch.html

  * igt@kms_chamelium@common-hpd-after-suspend:
    - bat-dg1-5:          NOTRUN -> [SKIP][17] ([fdo#111827])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112062v1/bat-dg1-5/igt@kms_chamelium@common-hpd-after-suspend.html

  * igt@kms_chamelium@dp-hpd-fast:
    - fi-ilk-650:         NOTRUN -> [SKIP][18] ([fdo#109271] / [fdo#111827]) +8 similar issues
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112062v1/fi-ilk-650/igt@kms_chamelium@dp-hpd-fast.html

  * igt@kms_chamelium@hdmi-crc-fast:
    - bat-dg1-6:          NOTRUN -> [SKIP][19] ([fdo#111827]) +8 similar issues
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112062v1/bat-dg1-6/igt@kms_chamelium@hdmi-crc-fast.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor:
    - bat-dg1-6:          NOTRUN -> [SKIP][20] ([i915#4103] / [i915#4213])
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112062v1/bat-dg1-6/igt@kms_cursor_legacy@basic-busy-flip-before-cursor.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions-varying-size:
    - fi-bsw-kefka:       [PASS][21] -> [FAIL][22] ([i915#6298])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12517/fi-bsw-kefka/igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions-varying-size.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112062v1/fi-bsw-kefka/igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions-varying-size.html

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

  * igt@kms_psr@sprite_plane_onoff:
    - bat-dg1-6:          NOTRUN -> [SKIP][24] ([i915#1072] / [i915#4078]) +3 similar issues
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112062v1/bat-dg1-6/igt@kms_psr@sprite_plane_onoff.html

  * igt@kms_setmode@basic-clone-single-crtc:
    - bat-dg1-6:          NOTRUN -> [SKIP][25] ([i915#3555])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112062v1/bat-dg1-6/igt@kms_setmode@basic-clone-single-crtc.html

  * igt@prime_vgem@basic-gtt:
    - bat-dg1-6:          NOTRUN -> [SKIP][26] ([i915#3708] / [i915#4077]) +1 similar issue
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112062v1/bat-dg1-6/igt@prime_vgem@basic-gtt.html

  * igt@prime_vgem@basic-userptr:
    - bat-dg1-6:          NOTRUN -> [SKIP][27] ([i915#3708] / [i915#4873])
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112062v1/bat-dg1-6/igt@prime_vgem@basic-userptr.html

  * igt@prime_vgem@basic-write:
    - bat-dg1-6:          NOTRUN -> [SKIP][28] ([i915#3708]) +3 similar issues
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112062v1/bat-dg1-6/igt@prime_vgem@basic-write.html

  
#### Possible fixes ####

  * igt@fbdev@write:
    - fi-blb-e6850:       [SKIP][29] ([fdo#109271]) -> [PASS][30] +4 similar issues
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12517/fi-blb-e6850/igt@fbdev@write.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112062v1/fi-blb-e6850/igt@fbdev@write.html

  * igt@i915_selftest@live@migrate:
    - {bat-adlp-6}:       [DMESG-FAIL][31] ([i915#7699]) -> [PASS][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12517/bat-adlp-6/igt@i915_selftest@live@migrate.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112062v1/bat-adlp-6/igt@i915_selftest@live@migrate.html

  * igt@i915_selftest@live@mman:
    - fi-rkl-guc:         [TIMEOUT][33] ([i915#6794]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12517/fi-rkl-guc/igt@i915_selftest@live@mman.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112062v1/fi-rkl-guc/igt@i915_selftest@live@mman.html

  * igt@i915_selftest@live@workarounds:
    - bat-dg1-5:          [INCOMPLETE][35] ([i915#4983]) -> [PASS][36]
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12517/bat-dg1-5/igt@i915_selftest@live@workarounds.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_112062v1/bat-dg1-5/igt@i915_selftest@live@workarounds.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109295]: https://bugs.freedesktop.org/show_bug.cgi?id=109295
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845
  [i915#2575]: https://gitlab.freedesktop.org/drm/intel/issues/2575
  [i915#2582]: https://gitlab.freedesktop.org/drm/intel/issues/2582
  [i915#2867]: https://gitlab.freedesktop.org/drm/intel/issues/2867
  [i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
  [i915#3291]: https://gitlab.freedesktop.org/drm/intel/issues/3291
  [i915#3301]: https://gitlab.freedesktop.org/drm/intel/issues/3301
  [i915#3546]: https://gitlab.freedesktop.org/drm/intel/issues/3546
  [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#4078]: https://gitlab.freedesktop.org/drm/intel/issues/4078
  [i915#4079]: https://gitlab.freedesktop.org/drm/intel/issues/4079
  [i915#4083]: https://gitlab.freedesktop.org/drm/intel/issues/4083
  [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
  [i915#4212]: https://gitlab.freedesktop.org/drm/intel/issues/4212
  [i915#4213]: https://gitlab.freedesktop.org/drm/intel/issues/4213
  [i915#4215]: https://gitlab.freedesktop.org/drm/intel/issues/4215
  [i915#4303]: https://gitlab.freedesktop.org/drm/intel/issues/4303
  [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
  [i915#4579]: https://gitlab.freedesktop.org/drm/intel/issues/4579
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4873]: https://gitlab.freedesktop.org/drm/intel/issues/4873
  [i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
  [i915#5174]: https://gitlab.freedesktop.org/drm/intel/issues/5174
  [i915#5190]: https://gitlab.freedesktop.org/drm/intel/issues/5190
  [i915#5274]: https://gitlab.freedesktop.org/drm/intel/issues/5274
  [i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334
  [i915#5354]: https://gitlab.freedesktop.org/drm/intel/issues/5354
  [i915#5775]: https://gitlab.freedesktop.org/drm/intel/issues/5775
  [i915#6298]: https://gitlab.freedesktop.org/drm/intel/issues/6298
  [i915#6621]: https://gitlab.freedesktop.org/drm/intel/issues/6621
  [i915#6645]: https://gitlab.freedesktop.org/drm/intel/issues/6645
  [i915#6794]: https://gitlab.freedesktop.org/drm/intel/issues/6794
  [i915#6997]: https://gitlab.freedesktop.org/drm/intel/issues/6997
  [i915#7350]: https://gitlab.freedesktop.org/drm/intel/issues/7350
  [i915#7456]: https://gitlab.freedesktop.org/drm/intel/issues/7456
  [i915#7561]: https://gitlab.freedesktop.org/drm/intel/issues/7561
  [i915#7699]: https://gitlab.freedesktop.org/drm/intel/issues/7699


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

  * Linux: CI_DRM_12517 -> Patchwork_112062v1

  CI-20190529: 20190529
  CI_DRM_12517: 0ccc7ebce461c057c63b5879b8ace1a34ea8423a @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7100: 04466b02a9b5356d266a899daa5183c1f6b0e20f @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_112062v1: 0ccc7ebce461c057c63b5879b8ace1a34ea8423a @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

b42285f98840 drm/i915/display/dsi: use intel_de_rmw if possible

== Logs ==

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

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

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

* [Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915/display/dsi: use intel_de_rmw if possible
  2022-12-19 13:08 [Intel-gfx] [PATCH] drm/i915/display/dsi: use intel_de_rmw if possible Andrzej Hajda
  2022-12-20  9:26 ` Jani Nikula
  2022-12-21 14:05 ` [Intel-gfx] ✓ Fi.CI.BAT: success for " Patchwork
@ 2022-12-21 15:56 ` Patchwork
  2 siblings, 0 replies; 5+ messages in thread
From: Patchwork @ 2022-12-21 15:56 UTC (permalink / raw)
  To: Andrzej Hajda; +Cc: intel-gfx

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

== Series Details ==

Series: drm/i915/display/dsi: use intel_de_rmw if possible
URL   : https://patchwork.freedesktop.org/series/112062/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_12517_full -> Patchwork_112062v1_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

Participating hosts (14 -> 9)
------------------------------

  Missing    (5): pig-kbl-iris shard-tglu-9 pig-glk-j5005 pig-skl-6260u shard-rkl 

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

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

### IGT changes ###

  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109274]: https://bugs.freedesktop.org/show_bug.cgi?id=109274
  [fdo#109279]: https://bugs.freedesktop.org/show_bug.cgi?id=109279
  [fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109295]: https://bugs.freedesktop.org/show_bug.cgi?id=109295
  [fdo#109302]: https://bugs.freedesktop.org/show_bug.cgi?id=109302
  [fdo#109307]: https://bugs.freedesktop.org/show_bug.cgi?id=109307
  [fdo#109309]: https://bugs.freedesktop.org/show_bug.cgi?id=109309
  [fdo#109313]: https://bugs.freedesktop.org/show_bug.cgi?id=109313
  [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
  [fdo#109506]: https://bugs.freedesktop.org/show_bug.cgi?id=109506
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#110189]: https://bugs.freedesktop.org/show_bug.cgi?id=110189
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
  [fdo#111614]: https://bugs.freedesktop.org/show_bug.cgi?id=111614
  [fdo#111615]: https://bugs.freedesktop.org/show_bug.cgi?id=111615
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [fdo#112054]: https://bugs.freedesktop.org/show_bug.cgi?id=112054
  [i915#1769]: https://gitlab.freedesktop.org/drm/intel/issues/1769
  [i915#1839]: https://gitlab.freedesktop.org/drm/intel/issues/1839
  [i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845
  [i915#1849]: https://gitlab.freedesktop.org/drm/intel/issues/1849
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#2437]: https://gitlab.freedesktop.org/drm/intel/issues/2437
  [i915#2527]: https://gitlab.freedesktop.org/drm/intel/issues/2527
  [i915#2575]: https://gitlab.freedesktop.org/drm/intel/issues/2575
  [i915#2582]: https://gitlab.freedesktop.org/drm/intel/issues/2582
  [i915#2587]: https://gitlab.freedesktop.org/drm/intel/issues/2587
  [i915#2658]: https://gitlab.freedesktop.org/drm/intel/issues/2658
  [i915#2672]: https://gitlab.freedesktop.org/drm/intel/issues/2672
  [i915#2681]: https://gitlab.freedesktop.org/drm/intel/issues/2681
  [i915#2705]: https://gitlab.freedesktop.org/drm/intel/issues/2705
  [i915#2842]: https://gitlab.freedesktop.org/drm/intel/issues/2842
  [i915#2856]: https://gitlab.freedesktop.org/drm/intel/issues/2856
  [i915#2994]: https://gitlab.freedesktop.org/drm/intel/issues/2994
  [i915#3116]: https://gitlab.freedesktop.org/drm/intel/issues/3116
  [i915#3297]: https://gitlab.freedesktop.org/drm/intel/issues/3297
  [i915#3299]: https://gitlab.freedesktop.org/drm/intel/issues/3299
  [i915#3301]: https://gitlab.freedesktop.org/drm/intel/issues/3301
  [i915#3323]: https://gitlab.freedesktop.org/drm/intel/issues/3323
  [i915#3359]: https://gitlab.freedesktop.org/drm/intel/issues/3359
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3637]: https://gitlab.freedesktop.org/drm/intel/issues/3637
  [i915#3689]: https://gitlab.freedesktop.org/drm/intel/issues/3689
  [i915#3742]: https://gitlab.freedesktop.org/drm/intel/issues/3742
  [i915#3804]: https://gitlab.freedesktop.org/drm/intel/issues/3804
  [i915#3886]: https://gitlab.freedesktop.org/drm/intel/issues/3886
  [i915#404]: https://gitlab.freedesktop.org/drm/intel/issues/404
  [i915#4270]: https://gitlab.freedesktop.org/drm/intel/issues/4270
  [i915#4281]: https://gitlab.freedesktop.org/drm/intel/issues/4281
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4767]: https://gitlab.freedesktop.org/drm/intel/issues/4767
  [i915#5176]: https://gitlab.freedesktop.org/drm/intel/issues/5176
  [i915#5235]: https://gitlab.freedesktop.org/drm/intel/issues/5235
  [i915#5286]: https://gitlab.freedesktop.org/drm/intel/issues/5286
  [i915#5288]: https://gitlab.freedesktop.org/drm/intel/issues/5288
  [i915#5325]: https://gitlab.freedesktop.org/drm/intel/issues/5325
  [i915#5439]: https://gitlab.freedesktop.org/drm/intel/issues/5439
  [i915#5461]: https://gitlab.freedesktop.org/drm/intel/issues/5461
  [i915#6095]: https://gitlab.freedesktop.org/drm/intel/issues/6095
  [i915#6230]: https://gitlab.freedesktop.org/drm/intel/issues/6230
  [i915#6335]: https://gitlab.freedesktop.org/drm/intel/issues/6335
  [i915#6524]: https://gitlab.freedesktop.org/drm/intel/issues/6524
  [i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658
  [i915#7116]: https://gitlab.freedesktop.org/drm/intel/issues/7116
  [i915#7118]: https://gitlab.freedesktop.org/drm/intel/issues/7118
  [i915#7561]: https://gitlab.freedesktop.org/drm/intel/issues/7561
  [i915#7582]: https://gitlab.freedesktop.org/drm/intel/issues/7582
  [i915#7651]: https://gitlab.freedesktop.org/drm/intel/issues/7651
  [i915#7697]: https://gitlab.freedesktop.org/drm/intel/issues/7697


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

  * Linux: CI_DRM_12517 -> Patchwork_112062v1
  * Piglit: piglit_4509 -> None

  CI-20190529: 20190529
  CI_DRM_12517: 0ccc7ebce461c057c63b5879b8ace1a34ea8423a @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7100: 04466b02a9b5356d266a899daa5183c1f6b0e20f @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_112062v1: 0ccc7ebce461c057c63b5879b8ace1a34ea8423a @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

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

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

* Re: [Intel-gfx] [PATCH] drm/i915/display/dsi: use intel_de_rmw if possible
  2022-12-20  9:26 ` Jani Nikula
@ 2023-01-30 13:00   ` Jani Nikula
  0 siblings, 0 replies; 5+ messages in thread
From: Jani Nikula @ 2023-01-30 13:00 UTC (permalink / raw)
  To: Andrzej Hajda, intel-gfx; +Cc: Andrzej Hajda, Rodrigo Vivi

On Tue, 20 Dec 2022, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> On Mon, 19 Dec 2022, Andrzej Hajda <andrzej.hajda@intel.com> wrote:
>> The helper makes the code more compact and readable.
>>
>> Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com>
>
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>

And pushed to din, thanks for the patch.

BR,
Jani.


>
>> ---
>>  drivers/gpu/drm/i915/display/icl_dsi.c | 256 ++++++++-----------------
>>  1 file changed, 82 insertions(+), 174 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
>> index ae14c794c4bc09..b02ac9d2b1e4a2 100644
>> --- a/drivers/gpu/drm/i915/display/icl_dsi.c
>> +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
>> @@ -207,7 +207,7 @@ void icl_dsi_frame_update(struct intel_crtc_state *crtc_state)
>>  {
>>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
>>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>> -	u32 tmp, mode_flags;
>> +	u32 mode_flags;
>>  	enum port port;
>>  
>>  	mode_flags = crtc_state->mode_flags;
>> @@ -224,9 +224,7 @@ void icl_dsi_frame_update(struct intel_crtc_state *crtc_state)
>>  	else
>>  		return;
>>  
>> -	tmp = intel_de_read(dev_priv, DSI_CMD_FRMCTL(port));
>> -	tmp |= DSI_FRAME_UPDATE_REQUEST;
>> -	intel_de_write(dev_priv, DSI_CMD_FRMCTL(port), tmp);
>> +	intel_de_rmw(dev_priv, DSI_CMD_FRMCTL(port), 0, DSI_FRAME_UPDATE_REQUEST);
>>  }
>>  
>>  static void dsi_program_swing_and_deemphasis(struct intel_encoder *encoder)
>> @@ -234,7 +232,7 @@ static void dsi_program_swing_and_deemphasis(struct intel_encoder *encoder)
>>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
>>  	enum phy phy;
>> -	u32 tmp;
>> +	u32 tmp, mask, val;
>>  	int lane;
>>  
>>  	for_each_dsi_phy(phy, intel_dsi->phys) {
>> @@ -242,56 +240,35 @@ static void dsi_program_swing_and_deemphasis(struct intel_encoder *encoder)
>>  		 * Program voltage swing and pre-emphasis level values as per
>>  		 * table in BSPEC under DDI buffer programing
>>  		 */
>> +		mask = SCALING_MODE_SEL_MASK | RTERM_SELECT_MASK;
>> +		val = SCALING_MODE_SEL(0x2) | TAP2_DISABLE | TAP3_DISABLE |
>> +		      RTERM_SELECT(0x6);
>>  		tmp = intel_de_read(dev_priv, ICL_PORT_TX_DW5_LN(0, phy));
>> -		tmp &= ~(SCALING_MODE_SEL_MASK | RTERM_SELECT_MASK);
>> -		tmp |= SCALING_MODE_SEL(0x2);
>> -		tmp |= TAP2_DISABLE | TAP3_DISABLE;
>> -		tmp |= RTERM_SELECT(0x6);
>> +		tmp &= ~mask;
>> +		tmp |= val;
>>  		intel_de_write(dev_priv, ICL_PORT_TX_DW5_GRP(phy), tmp);
>> +		intel_de_rmw(dev_priv, ICL_PORT_TX_DW5_AUX(phy), mask, val);
>>  
>> -		tmp = intel_de_read(dev_priv, ICL_PORT_TX_DW5_AUX(phy));
>> -		tmp &= ~(SCALING_MODE_SEL_MASK | RTERM_SELECT_MASK);
>> -		tmp |= SCALING_MODE_SEL(0x2);
>> -		tmp |= TAP2_DISABLE | TAP3_DISABLE;
>> -		tmp |= RTERM_SELECT(0x6);
>> -		intel_de_write(dev_priv, ICL_PORT_TX_DW5_AUX(phy), tmp);
>> -
>> +		mask = SWING_SEL_LOWER_MASK | SWING_SEL_UPPER_MASK |
>> +		       RCOMP_SCALAR_MASK;
>> +		val = SWING_SEL_UPPER(0x2) | SWING_SEL_LOWER(0x2) |
>> +		      RCOMP_SCALAR(0x98);
>>  		tmp = intel_de_read(dev_priv, ICL_PORT_TX_DW2_LN(0, phy));
>> -		tmp &= ~(SWING_SEL_LOWER_MASK | SWING_SEL_UPPER_MASK |
>> -			 RCOMP_SCALAR_MASK);
>> -		tmp |= SWING_SEL_UPPER(0x2);
>> -		tmp |= SWING_SEL_LOWER(0x2);
>> -		tmp |= RCOMP_SCALAR(0x98);
>> +		tmp &= ~mask;
>> +		tmp |= val;
>>  		intel_de_write(dev_priv, ICL_PORT_TX_DW2_GRP(phy), tmp);
>> +		intel_de_rmw(dev_priv, ICL_PORT_TX_DW2_AUX(phy), mask, val);
>>  
>> -		tmp = intel_de_read(dev_priv, ICL_PORT_TX_DW2_AUX(phy));
>> -		tmp &= ~(SWING_SEL_LOWER_MASK | SWING_SEL_UPPER_MASK |
>> -			 RCOMP_SCALAR_MASK);
>> -		tmp |= SWING_SEL_UPPER(0x2);
>> -		tmp |= SWING_SEL_LOWER(0x2);
>> -		tmp |= RCOMP_SCALAR(0x98);
>> -		intel_de_write(dev_priv, ICL_PORT_TX_DW2_AUX(phy), tmp);
>> -
>> -		tmp = intel_de_read(dev_priv, ICL_PORT_TX_DW4_AUX(phy));
>> -		tmp &= ~(POST_CURSOR_1_MASK | POST_CURSOR_2_MASK |
>> -			 CURSOR_COEFF_MASK);
>> -		tmp |= POST_CURSOR_1(0x0);
>> -		tmp |= POST_CURSOR_2(0x0);
>> -		tmp |= CURSOR_COEFF(0x3f);
>> -		intel_de_write(dev_priv, ICL_PORT_TX_DW4_AUX(phy), tmp);
>> -
>> -		for (lane = 0; lane <= 3; lane++) {
>> -			/* Bspec: must not use GRP register for write */
>> -			tmp = intel_de_read(dev_priv,
>> -					    ICL_PORT_TX_DW4_LN(lane, phy));
>> -			tmp &= ~(POST_CURSOR_1_MASK | POST_CURSOR_2_MASK |
>> -				 CURSOR_COEFF_MASK);
>> -			tmp |= POST_CURSOR_1(0x0);
>> -			tmp |= POST_CURSOR_2(0x0);
>> -			tmp |= CURSOR_COEFF(0x3f);
>> -			intel_de_write(dev_priv,
>> -				       ICL_PORT_TX_DW4_LN(lane, phy), tmp);
>> -		}
>> +		mask = POST_CURSOR_1_MASK | POST_CURSOR_2_MASK |
>> +		       CURSOR_COEFF_MASK;
>> +		val = POST_CURSOR_1(0x0) | POST_CURSOR_2(0x0) |
>> +		      CURSOR_COEFF(0x3f);
>> +		intel_de_rmw(dev_priv, ICL_PORT_TX_DW4_AUX(phy), mask, val);
>> +
>> +		/* Bspec: must not use GRP register for write */
>> +		for (lane = 0; lane <= 3; lane++)
>> +			intel_de_rmw(dev_priv, ICL_PORT_TX_DW4_LN(lane, phy),
>> +				     mask, val);
>>  	}
>>  }
>>  
>> @@ -310,7 +287,6 @@ static void configure_dual_link_mode(struct intel_encoder *encoder,
>>  	if (intel_dsi->dual_link == DSI_DUAL_LINK_FRONT_BACK) {
>>  		const struct drm_display_mode *adjusted_mode =
>>  					&pipe_config->hw.adjusted_mode;
>> -		u32 dss_ctl2;
>>  		u16 hactive = adjusted_mode->crtc_hdisplay;
>>  		u16 dl_buffer_depth;
>>  
>> @@ -323,10 +299,8 @@ static void configure_dual_link_mode(struct intel_encoder *encoder,
>>  
>>  		dss_ctl1 &= ~LEFT_DL_BUF_TARGET_DEPTH_MASK;
>>  		dss_ctl1 |= LEFT_DL_BUF_TARGET_DEPTH(dl_buffer_depth);
>> -		dss_ctl2 = intel_de_read(dev_priv, DSS_CTL2);
>> -		dss_ctl2 &= ~RIGHT_DL_BUF_TARGET_DEPTH_MASK;
>> -		dss_ctl2 |= RIGHT_DL_BUF_TARGET_DEPTH(dl_buffer_depth);
>> -		intel_de_write(dev_priv, DSS_CTL2, dss_ctl2);
>> +		intel_de_rmw(dev_priv, DSS_CTL2, RIGHT_DL_BUF_TARGET_DEPTH_MASK,
>> +			     RIGHT_DL_BUF_TARGET_DEPTH(dl_buffer_depth));
>>  	} else {
>>  		/* Interleave */
>>  		dss_ctl1 |= DUAL_LINK_MODE_INTERLEAVE;
>> @@ -412,13 +386,10 @@ static void gen11_dsi_enable_io_power(struct intel_encoder *encoder)
>>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
>>  	enum port port;
>> -	u32 tmp;
>>  
>> -	for_each_dsi_port(port, intel_dsi->ports) {
>> -		tmp = intel_de_read(dev_priv, ICL_DSI_IO_MODECTL(port));
>> -		tmp |= COMBO_PHY_MODE_DSI;
>> -		intel_de_write(dev_priv, ICL_DSI_IO_MODECTL(port), tmp);
>> -	}
>> +	for_each_dsi_port(port, intel_dsi->ports)
>> +		intel_de_rmw(dev_priv, ICL_DSI_IO_MODECTL(port),
>> +			     0, COMBO_PHY_MODE_DSI);
>>  
>>  	get_dsi_io_power_domains(dev_priv, intel_dsi);
>>  }
>> @@ -444,26 +415,16 @@ static void gen11_dsi_config_phy_lanes_sequence(struct intel_encoder *encoder)
>>  
>>  	/* Step 4b(i) set loadgen select for transmit and aux lanes */
>>  	for_each_dsi_phy(phy, intel_dsi->phys) {
>> -		tmp = intel_de_read(dev_priv, ICL_PORT_TX_DW4_AUX(phy));
>> -		tmp &= ~LOADGEN_SELECT;
>> -		intel_de_write(dev_priv, ICL_PORT_TX_DW4_AUX(phy), tmp);
>> -		for (lane = 0; lane <= 3; lane++) {
>> -			tmp = intel_de_read(dev_priv,
>> -					    ICL_PORT_TX_DW4_LN(lane, phy));
>> -			tmp &= ~LOADGEN_SELECT;
>> -			if (lane != 2)
>> -				tmp |= LOADGEN_SELECT;
>> -			intel_de_write(dev_priv,
>> -				       ICL_PORT_TX_DW4_LN(lane, phy), tmp);
>> -		}
>> +		intel_de_rmw(dev_priv, ICL_PORT_TX_DW4_AUX(phy), LOADGEN_SELECT, 0);
>> +		for (lane = 0; lane <= 3; lane++)
>> +			intel_de_rmw(dev_priv, ICL_PORT_TX_DW4_LN(lane, phy),
>> +				     LOADGEN_SELECT, lane != 2 ? LOADGEN_SELECT : 0);
>>  	}
>>  
>>  	/* Step 4b(ii) set latency optimization for transmit and aux lanes */
>>  	for_each_dsi_phy(phy, intel_dsi->phys) {
>> -		tmp = intel_de_read(dev_priv, ICL_PORT_TX_DW2_AUX(phy));
>> -		tmp &= ~FRC_LATENCY_OPTIM_MASK;
>> -		tmp |= FRC_LATENCY_OPTIM_VAL(0x5);
>> -		intel_de_write(dev_priv, ICL_PORT_TX_DW2_AUX(phy), tmp);
>> +		intel_de_rmw(dev_priv, ICL_PORT_TX_DW2_AUX(phy),
>> +			     FRC_LATENCY_OPTIM_MASK, FRC_LATENCY_OPTIM_VAL(0x5));
>>  		tmp = intel_de_read(dev_priv, ICL_PORT_TX_DW2_LN(0, phy));
>>  		tmp &= ~FRC_LATENCY_OPTIM_MASK;
>>  		tmp |= FRC_LATENCY_OPTIM_VAL(0x5);
>> @@ -471,12 +432,8 @@ static void gen11_dsi_config_phy_lanes_sequence(struct intel_encoder *encoder)
>>  
>>  		/* For EHL, TGL, set latency optimization for PCS_DW1 lanes */
>>  		if (IS_JSL_EHL(dev_priv) || (DISPLAY_VER(dev_priv) >= 12)) {
>> -			tmp = intel_de_read(dev_priv,
>> -					    ICL_PORT_PCS_DW1_AUX(phy));
>> -			tmp &= ~LATENCY_OPTIM_MASK;
>> -			tmp |= LATENCY_OPTIM_VAL(0);
>> -			intel_de_write(dev_priv, ICL_PORT_PCS_DW1_AUX(phy),
>> -				       tmp);
>> +			intel_de_rmw(dev_priv, ICL_PORT_PCS_DW1_AUX(phy),
>> +				     LATENCY_OPTIM_MASK, LATENCY_OPTIM_VAL(0));
>>  
>>  			tmp = intel_de_read(dev_priv,
>>  					    ICL_PORT_PCS_DW1_LN(0, phy));
>> @@ -501,9 +458,7 @@ static void gen11_dsi_voltage_swing_program_seq(struct intel_encoder *encoder)
>>  		tmp = intel_de_read(dev_priv, ICL_PORT_PCS_DW1_LN(0, phy));
>>  		tmp &= ~COMMON_KEEPER_EN;
>>  		intel_de_write(dev_priv, ICL_PORT_PCS_DW1_GRP(phy), tmp);
>> -		tmp = intel_de_read(dev_priv, ICL_PORT_PCS_DW1_AUX(phy));
>> -		tmp &= ~COMMON_KEEPER_EN;
>> -		intel_de_write(dev_priv, ICL_PORT_PCS_DW1_AUX(phy), tmp);
>> +		intel_de_rmw(dev_priv, ICL_PORT_PCS_DW1_AUX(phy), COMMON_KEEPER_EN, 0);
>>  	}
>>  
>>  	/*
>> @@ -511,20 +466,15 @@ static void gen11_dsi_voltage_swing_program_seq(struct intel_encoder *encoder)
>>  	 * Note: loadgen select program is done
>>  	 * as part of lane phy sequence configuration
>>  	 */
>> -	for_each_dsi_phy(phy, intel_dsi->phys) {
>> -		tmp = intel_de_read(dev_priv, ICL_PORT_CL_DW5(phy));
>> -		tmp |= SUS_CLOCK_CONFIG;
>> -		intel_de_write(dev_priv, ICL_PORT_CL_DW5(phy), tmp);
>> -	}
>> +	for_each_dsi_phy(phy, intel_dsi->phys)
>> +		intel_de_rmw(dev_priv, ICL_PORT_CL_DW5(phy), 0, SUS_CLOCK_CONFIG);
>>  
>>  	/* Clear training enable to change swing values */
>>  	for_each_dsi_phy(phy, intel_dsi->phys) {
>>  		tmp = intel_de_read(dev_priv, ICL_PORT_TX_DW5_LN(0, phy));
>>  		tmp &= ~TX_TRAINING_EN;
>>  		intel_de_write(dev_priv, ICL_PORT_TX_DW5_GRP(phy), tmp);
>> -		tmp = intel_de_read(dev_priv, ICL_PORT_TX_DW5_AUX(phy));
>> -		tmp &= ~TX_TRAINING_EN;
>> -		intel_de_write(dev_priv, ICL_PORT_TX_DW5_AUX(phy), tmp);
>> +		intel_de_rmw(dev_priv, ICL_PORT_TX_DW5_AUX(phy), TX_TRAINING_EN, 0);
>>  	}
>>  
>>  	/* Program swing and de-emphasis */
>> @@ -535,9 +485,7 @@ static void gen11_dsi_voltage_swing_program_seq(struct intel_encoder *encoder)
>>  		tmp = intel_de_read(dev_priv, ICL_PORT_TX_DW5_LN(0, phy));
>>  		tmp |= TX_TRAINING_EN;
>>  		intel_de_write(dev_priv, ICL_PORT_TX_DW5_GRP(phy), tmp);
>> -		tmp = intel_de_read(dev_priv, ICL_PORT_TX_DW5_AUX(phy));
>> -		tmp |= TX_TRAINING_EN;
>> -		intel_de_write(dev_priv, ICL_PORT_TX_DW5_AUX(phy), tmp);
>> +		intel_de_rmw(dev_priv, ICL_PORT_TX_DW5_AUX(phy), 0, TX_TRAINING_EN);
>>  	}
>>  }
>>  
>> @@ -545,13 +493,10 @@ static void gen11_dsi_enable_ddi_buffer(struct intel_encoder *encoder)
>>  {
>>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
>> -	u32 tmp;
>>  	enum port port;
>>  
>>  	for_each_dsi_port(port, intel_dsi->ports) {
>> -		tmp = intel_de_read(dev_priv, DDI_BUF_CTL(port));
>> -		tmp |= DDI_BUF_CTL_ENABLE;
>> -		intel_de_write(dev_priv, DDI_BUF_CTL(port), tmp);
>> +		intel_de_rmw(dev_priv, DDI_BUF_CTL(port), 0, DDI_BUF_CTL_ENABLE);
>>  
>>  		if (wait_for_us(!(intel_de_read(dev_priv, DDI_BUF_CTL(port)) &
>>  				  DDI_BUF_IS_IDLE),
>> @@ -567,17 +512,13 @@ gen11_dsi_setup_dphy_timings(struct intel_encoder *encoder,
>>  {
>>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
>> -	u32 tmp;
>>  	enum port port;
>>  	enum phy phy;
>>  
>>  	/* Program T-INIT master registers */
>> -	for_each_dsi_port(port, intel_dsi->ports) {
>> -		tmp = intel_de_read(dev_priv, ICL_DSI_T_INIT_MASTER(port));
>> -		tmp &= ~DSI_T_INIT_MASTER_MASK;
>> -		tmp |= intel_dsi->init_count;
>> -		intel_de_write(dev_priv, ICL_DSI_T_INIT_MASTER(port), tmp);
>> -	}
>> +	for_each_dsi_port(port, intel_dsi->ports)
>> +		intel_de_rmw(dev_priv, ICL_DSI_T_INIT_MASTER(port),
>> +			     DSI_T_INIT_MASTER_MASK, intel_dsi->init_count);
>>  
>>  	/* Program DPHY clock lanes timings */
>>  	for_each_dsi_port(port, intel_dsi->ports) {
>> @@ -608,31 +549,22 @@ gen11_dsi_setup_dphy_timings(struct intel_encoder *encoder,
>>  	if (DISPLAY_VER(dev_priv) == 11) {
>>  		if (afe_clk(encoder, crtc_state) <= 800000) {
>>  			for_each_dsi_port(port, intel_dsi->ports) {
>> -				tmp = intel_de_read(dev_priv,
>> -						    DPHY_TA_TIMING_PARAM(port));
>> -				tmp &= ~TA_SURE_MASK;
>> -				tmp |= TA_SURE_OVERRIDE | TA_SURE(0);
>> -				intel_de_write(dev_priv,
>> -					       DPHY_TA_TIMING_PARAM(port),
>> -					       tmp);
>> +				intel_de_rmw(dev_priv, DPHY_TA_TIMING_PARAM(port),
>> +					     TA_SURE_MASK,
>> +					     TA_SURE_OVERRIDE | TA_SURE(0));
>>  
>>  				/* shadow register inside display core */
>> -				tmp = intel_de_read(dev_priv,
>> -						    DSI_TA_TIMING_PARAM(port));
>> -				tmp &= ~TA_SURE_MASK;
>> -				tmp |= TA_SURE_OVERRIDE | TA_SURE(0);
>> -				intel_de_write(dev_priv,
>> -					       DSI_TA_TIMING_PARAM(port), tmp);
>> +				intel_de_rmw(dev_priv, DSI_TA_TIMING_PARAM(port),
>> +					     TA_SURE_MASK,
>> +					     TA_SURE_OVERRIDE | TA_SURE(0));
>>  			}
>>  		}
>>  	}
>>  
>>  	if (IS_JSL_EHL(dev_priv)) {
>> -		for_each_dsi_phy(phy, intel_dsi->phys) {
>> -			tmp = intel_de_read(dev_priv, ICL_DPHY_CHKN(phy));
>> -			tmp |= ICL_DPHY_CHKN_AFE_OVER_PPI_STRAP;
>> -			intel_de_write(dev_priv, ICL_DPHY_CHKN(phy), tmp);
>> -		}
>> +		for_each_dsi_phy(phy, intel_dsi->phys)
>> +			intel_de_rmw(dev_priv, ICL_DPHY_CHKN(phy),
>> +				     0, ICL_DPHY_CHKN_AFE_OVER_PPI_STRAP);
>>  	}
>>  }
>>  
>> @@ -824,11 +756,8 @@ gen11_dsi_configure_transcoder(struct intel_encoder *encoder,
>>  	if (intel_dsi->dual_link) {
>>  		for_each_dsi_port(port, intel_dsi->ports) {
>>  			dsi_trans = dsi_port_to_transcoder(port);
>> -			tmp = intel_de_read(dev_priv,
>> -					    TRANS_DDI_FUNC_CTL2(dsi_trans));
>> -			tmp |= PORT_SYNC_MODE_ENABLE;
>> -			intel_de_write(dev_priv,
>> -				       TRANS_DDI_FUNC_CTL2(dsi_trans), tmp);
>> +			intel_de_rmw(dev_priv, TRANS_DDI_FUNC_CTL2(dsi_trans),
>> +				     0, PORT_SYNC_MODE_ENABLE);
>>  		}
>>  
>>  		/* configure stream splitting */
>> @@ -1044,13 +973,10 @@ static void gen11_dsi_enable_transcoder(struct intel_encoder *encoder)
>>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
>>  	enum port port;
>>  	enum transcoder dsi_trans;
>> -	u32 tmp;
>>  
>>  	for_each_dsi_port(port, intel_dsi->ports) {
>>  		dsi_trans = dsi_port_to_transcoder(port);
>> -		tmp = intel_de_read(dev_priv, PIPECONF(dsi_trans));
>> -		tmp |= PIPECONF_ENABLE;
>> -		intel_de_write(dev_priv, PIPECONF(dsi_trans), tmp);
>> +		intel_de_rmw(dev_priv, PIPECONF(dsi_trans), 0, PIPECONF_ENABLE);
>>  
>>  		/* wait for transcoder to be enabled */
>>  		if (intel_de_wait_for_set(dev_priv, PIPECONF(dsi_trans),
>> @@ -1067,7 +993,7 @@ static void gen11_dsi_setup_timeouts(struct intel_encoder *encoder,
>>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
>>  	enum port port;
>>  	enum transcoder dsi_trans;
>> -	u32 tmp, hs_tx_timeout, lp_rx_timeout, ta_timeout, divisor, mul;
>> +	u32 hs_tx_timeout, lp_rx_timeout, ta_timeout, divisor, mul;
>>  
>>  	/*
>>  	 * escape clock count calculation:
>> @@ -1087,26 +1013,23 @@ static void gen11_dsi_setup_timeouts(struct intel_encoder *encoder,
>>  		dsi_trans = dsi_port_to_transcoder(port);
>>  
>>  		/* program hst_tx_timeout */
>> -		tmp = intel_de_read(dev_priv, DSI_HSTX_TO(dsi_trans));
>> -		tmp &= ~HSTX_TIMEOUT_VALUE_MASK;
>> -		tmp |= HSTX_TIMEOUT_VALUE(hs_tx_timeout);
>> -		intel_de_write(dev_priv, DSI_HSTX_TO(dsi_trans), tmp);
>> +		intel_de_rmw(dev_priv, DSI_HSTX_TO(dsi_trans),
>> +			     HSTX_TIMEOUT_VALUE_MASK,
>> +			     HSTX_TIMEOUT_VALUE(hs_tx_timeout));
>>  
>>  		/* FIXME: DSI_CALIB_TO */
>>  
>>  		/* program lp_rx_host timeout */
>> -		tmp = intel_de_read(dev_priv, DSI_LPRX_HOST_TO(dsi_trans));
>> -		tmp &= ~LPRX_TIMEOUT_VALUE_MASK;
>> -		tmp |= LPRX_TIMEOUT_VALUE(lp_rx_timeout);
>> -		intel_de_write(dev_priv, DSI_LPRX_HOST_TO(dsi_trans), tmp);
>> +		intel_de_rmw(dev_priv, DSI_LPRX_HOST_TO(dsi_trans),
>> +			     LPRX_TIMEOUT_VALUE_MASK,
>> +			     LPRX_TIMEOUT_VALUE(lp_rx_timeout));
>>  
>>  		/* FIXME: DSI_PWAIT_TO */
>>  
>>  		/* program turn around timeout */
>> -		tmp = intel_de_read(dev_priv, DSI_TA_TO(dsi_trans));
>> -		tmp &= ~TA_TIMEOUT_VALUE_MASK;
>> -		tmp |= TA_TIMEOUT_VALUE(ta_timeout);
>> -		intel_de_write(dev_priv, DSI_TA_TO(dsi_trans), tmp);
>> +		intel_de_rmw(dev_priv, DSI_TA_TO(dsi_trans),
>> +			     TA_TIMEOUT_VALUE_MASK,
>> +			     TA_TIMEOUT_VALUE(ta_timeout));
>>  	}
>>  }
>>  
>> @@ -1310,15 +1233,12 @@ static void gen11_dsi_disable_transcoder(struct intel_encoder *encoder)
>>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
>>  	enum port port;
>>  	enum transcoder dsi_trans;
>> -	u32 tmp;
>>  
>>  	for_each_dsi_port(port, intel_dsi->ports) {
>>  		dsi_trans = dsi_port_to_transcoder(port);
>>  
>>  		/* disable transcoder */
>> -		tmp = intel_de_read(dev_priv, PIPECONF(dsi_trans));
>> -		tmp &= ~PIPECONF_ENABLE;
>> -		intel_de_write(dev_priv, PIPECONF(dsi_trans), tmp);
>> +		intel_de_rmw(dev_priv, PIPECONF(dsi_trans), PIPECONF_ENABLE, 0);
>>  
>>  		/* wait for transcoder to be disabled */
>>  		if (intel_de_wait_for_clear(dev_priv, PIPECONF(dsi_trans),
>> @@ -1350,11 +1270,9 @@ static void gen11_dsi_deconfigure_trancoder(struct intel_encoder *encoder)
>>  
>>  	/* disable periodic update mode */
>>  	if (is_cmd_mode(intel_dsi)) {
>> -		for_each_dsi_port(port, intel_dsi->ports) {
>> -			tmp = intel_de_read(dev_priv, DSI_CMD_FRMCTL(port));
>> -			tmp &= ~DSI_PERIODIC_FRAME_UPDATE_ENABLE;
>> -			intel_de_write(dev_priv, DSI_CMD_FRMCTL(port), tmp);
>> -		}
>> +		for_each_dsi_port(port, intel_dsi->ports)
>> +			intel_de_rmw(dev_priv, DSI_CMD_FRMCTL(port),
>> +				     DSI_PERIODIC_FRAME_UPDATE_ENABLE, 0);
>>  	}
>>  
>>  	/* put dsi link in ULPS */
>> @@ -1374,20 +1292,16 @@ static void gen11_dsi_deconfigure_trancoder(struct intel_encoder *encoder)
>>  	/* disable ddi function */
>>  	for_each_dsi_port(port, intel_dsi->ports) {
>>  		dsi_trans = dsi_port_to_transcoder(port);
>> -		tmp = intel_de_read(dev_priv, TRANS_DDI_FUNC_CTL(dsi_trans));
>> -		tmp &= ~TRANS_DDI_FUNC_ENABLE;
>> -		intel_de_write(dev_priv, TRANS_DDI_FUNC_CTL(dsi_trans), tmp);
>> +		intel_de_rmw(dev_priv, TRANS_DDI_FUNC_CTL(dsi_trans),
>> +			     TRANS_DDI_FUNC_ENABLE, 0);
>>  	}
>>  
>>  	/* disable port sync mode if dual link */
>>  	if (intel_dsi->dual_link) {
>>  		for_each_dsi_port(port, intel_dsi->ports) {
>>  			dsi_trans = dsi_port_to_transcoder(port);
>> -			tmp = intel_de_read(dev_priv,
>> -					    TRANS_DDI_FUNC_CTL2(dsi_trans));
>> -			tmp &= ~PORT_SYNC_MODE_ENABLE;
>> -			intel_de_write(dev_priv,
>> -				       TRANS_DDI_FUNC_CTL2(dsi_trans), tmp);
>> +			intel_de_rmw(dev_priv, TRANS_DDI_FUNC_CTL2(dsi_trans),
>> +				     PORT_SYNC_MODE_ENABLE, 0);
>>  		}
>>  	}
>>  }
>> @@ -1396,14 +1310,11 @@ static void gen11_dsi_disable_port(struct intel_encoder *encoder)
>>  {
>>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
>> -	u32 tmp;
>>  	enum port port;
>>  
>>  	gen11_dsi_ungate_clocks(encoder);
>>  	for_each_dsi_port(port, intel_dsi->ports) {
>> -		tmp = intel_de_read(dev_priv, DDI_BUF_CTL(port));
>> -		tmp &= ~DDI_BUF_CTL_ENABLE;
>> -		intel_de_write(dev_priv, DDI_BUF_CTL(port), tmp);
>> +		intel_de_rmw(dev_priv, DDI_BUF_CTL(port), DDI_BUF_CTL_ENABLE, 0);
>>  
>>  		if (wait_for_us((intel_de_read(dev_priv, DDI_BUF_CTL(port)) &
>>  				 DDI_BUF_IS_IDLE),
>> @@ -1420,7 +1331,6 @@ static void gen11_dsi_disable_io_power(struct intel_encoder *encoder)
>>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
>>  	enum port port;
>> -	u32 tmp;
>>  
>>  	for_each_dsi_port(port, intel_dsi->ports) {
>>  		intel_wakeref_t wakeref;
>> @@ -1434,11 +1344,9 @@ static void gen11_dsi_disable_io_power(struct intel_encoder *encoder)
>>  	}
>>  
>>  	/* set mode to DDI */
>> -	for_each_dsi_port(port, intel_dsi->ports) {
>> -		tmp = intel_de_read(dev_priv, ICL_DSI_IO_MODECTL(port));
>> -		tmp &= ~COMBO_PHY_MODE_DSI;
>> -		intel_de_write(dev_priv, ICL_DSI_IO_MODECTL(port), tmp);
>> -	}
>> +	for_each_dsi_port(port, intel_dsi->ports)
>> +		intel_de_rmw(dev_priv, ICL_DSI_IO_MODECTL(port),
>> +			     COMBO_PHY_MODE_DSI, 0);
>>  }
>>  
>>  static void gen11_dsi_disable(struct intel_atomic_state *state,

-- 
Jani Nikula, Intel Open Source Graphics Center

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

end of thread, other threads:[~2023-01-30 13:00 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-12-19 13:08 [Intel-gfx] [PATCH] drm/i915/display/dsi: use intel_de_rmw if possible Andrzej Hajda
2022-12-20  9:26 ` Jani Nikula
2023-01-30 13:00   ` Jani Nikula
2022-12-21 14:05 ` [Intel-gfx] ✓ Fi.CI.BAT: success for " Patchwork
2022-12-21 15:56 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork

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