drm/i915: Define transcoder timing register bitmasks

Define the contents of the transcoder timing registers using
REG_GENMASK() & co. For ease of maintenance let's just define
the bitmasks with the full 16bit width (also used by the
current hand rolled stuff) even though not all bits are actually
used. None of the unsued bits have ever contained anything.

Jani spotted that the CRT load detection code did use narrower
bitmasks, so that is now going to change. But that is fine
since any garbage in the high bits would have been caught by
the state checker that always used the full 16bit masks.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20230213225258.2127-10-ville.syrjala@linux.intel.comReviewed-by: Jani Nikula <jani.nikula@intel.com>

drivers/gpu/drm/i915/display/icl_dsi.c

@@ -888,7 +888,7 @@ gen11_dsi_set_transcoder_timings(struct intel_encoder *encoder,
 	for_each_dsi_port(port, intel_dsi->ports) {
 		dsi_trans = dsi_port_to_transcoder(port);
 		intel_de_write(dev_priv, TRANS_HTOTAL(dsi_trans),
-			       (hactive - 1) | ((htotal - 1) << 16));
+			       HACTIVE(hactive - 1) | HTOTAL(htotal - 1));
 	}
 
 	/* TRANS_HSYNC register to be programmed only for video mode */
@@ -911,7 +911,7 @@ gen11_dsi_set_transcoder_timings(struct intel_encoder *encoder,
 		for_each_dsi_port(port, intel_dsi->ports) {
 			dsi_trans = dsi_port_to_transcoder(port);
 			intel_de_write(dev_priv, TRANS_HSYNC(dsi_trans),
-				       (hsync_start - 1) | ((hsync_end - 1) << 16));
+				       HSYNC_START(hsync_start - 1) | HSYNC_END(hsync_end - 1));
 		}
 	}
 
@@ -925,7 +925,7 @@ gen11_dsi_set_transcoder_timings(struct intel_encoder *encoder,
 		 * For interlace mode: program required pixel minus 2
 		 */
 		intel_de_write(dev_priv, TRANS_VTOTAL(dsi_trans),
-			       (vactive - 1) | ((vtotal - 1) << 16));
+			       VACTIVE(vactive - 1) | VTOTAL(vtotal - 1));
 	}
 
 	if (vsync_end < vsync_start || vsync_end > vtotal)
@@ -939,7 +939,7 @@ gen11_dsi_set_transcoder_timings(struct intel_encoder *encoder,
 		for_each_dsi_port(port, intel_dsi->ports) {
 			dsi_trans = dsi_port_to_transcoder(port);
 			intel_de_write(dev_priv, TRANS_VSYNC(dsi_trans),
-				       (vsync_start - 1) | ((vsync_end - 1) << 16));
+				       VSYNC_START(vsync_start - 1) | VSYNC_END(vsync_end - 1));
 		}
 	}
 
@@ -962,7 +962,7 @@ gen11_dsi_set_transcoder_timings(struct intel_encoder *encoder,
 		for_each_dsi_port(port, intel_dsi->ports) {
 			dsi_trans = dsi_port_to_transcoder(port);
 			intel_de_write(dev_priv, TRANS_VBLANK(dsi_trans),
-				       (vactive - 1) | ((vtotal - 1) << 16));
+				       VBLANK_START(vactive - 1) | VBLANK_END(vtotal - 1));
 		}
 	}
 }

drivers/gpu/drm/i915/display/intel_crt.c

@@ -698,11 +698,11 @@ intel_crt_load_detect(struct intel_crt *crt, enum pipe pipe)
 	save_vtotal = intel_de_read(dev_priv, TRANS_VTOTAL(cpu_transcoder));
 	vblank = intel_de_read(dev_priv, TRANS_VBLANK(cpu_transcoder));
 
-	vtotal = ((save_vtotal >> 16) & 0xfff) + 1;
-	vactive = (save_vtotal & 0x7ff) + 1;
+	vtotal = REG_FIELD_GET(VTOTAL_MASK, save_vtotal) + 1;
+	vactive = REG_FIELD_GET(VACTIVE_MASK, save_vtotal) + 1;
 
-	vblank_start = (vblank & 0xfff) + 1;
-	vblank_end = ((vblank >> 16) & 0xfff) + 1;
+	vblank_start = REG_FIELD_GET(VBLANK_START_MASK, vblank) + 1;
+	vblank_end = REG_FIELD_GET(VBLANK_END_MASK, vblank) + 1;
 
 	/* Set the border color to purple. */
 	intel_de_write(dev_priv, BCLRPAT(cpu_transcoder), 0x500050);
@@ -732,11 +732,12 @@ intel_crt_load_detect(struct intel_crt *crt, enum pipe pipe)
 		*/
 		if (vblank_start <= vactive && vblank_end >= vtotal) {
 			u32 vsync = intel_de_read(dev_priv, TRANS_VSYNC(cpu_transcoder));
-			u32 vsync_start = (vsync & 0xffff) + 1;
+			u32 vsync_start = REG_FIELD_GET(VSYNC_START_MASK, vsync) + 1;
 
 			vblank_start = vsync_start;
 			intel_de_write(dev_priv, TRANS_VBLANK(cpu_transcoder),
-				       (vblank_start - 1) | ((vblank_end - 1) << 16));
+				       VBLANK_START(vblank_start - 1) |
+				       VBLANK_END(vblank_end - 1));
 			restore_vblank = true;
 		}
 		/* sample in the vertical border, selecting the larger one */

drivers/gpu/drm/i915/display/intel_display.c

@@ -2751,18 +2751,24 @@ static void intel_set_transcoder_timings(const struct intel_crtc_state *crtc_sta
 			       vsyncshift);
 
 	intel_de_write(dev_priv, TRANS_HTOTAL(cpu_transcoder),
-		       (adjusted_mode->crtc_hdisplay - 1) | ((adjusted_mode->crtc_htotal - 1) << 16));
+		       HACTIVE(adjusted_mode->crtc_hdisplay - 1) |
+		       HTOTAL(adjusted_mode->crtc_htotal - 1));
 	intel_de_write(dev_priv, TRANS_HBLANK(cpu_transcoder),
-		       (adjusted_mode->crtc_hblank_start - 1) | ((adjusted_mode->crtc_hblank_end - 1) << 16));
+		       HBLANK_START(adjusted_mode->crtc_hblank_start - 1) |
+		       HBLANK_END(adjusted_mode->crtc_hblank_end - 1));
 	intel_de_write(dev_priv, TRANS_HSYNC(cpu_transcoder),
-		       (adjusted_mode->crtc_hsync_start - 1) | ((adjusted_mode->crtc_hsync_end - 1) << 16));
+		       HSYNC_START(adjusted_mode->crtc_hsync_start - 1) |
+		       HSYNC_END(adjusted_mode->crtc_hsync_end - 1));
 
 	intel_de_write(dev_priv, TRANS_VTOTAL(cpu_transcoder),
-		       (adjusted_mode->crtc_vdisplay - 1) | ((crtc_vtotal - 1) << 16));
+		       VACTIVE(adjusted_mode->crtc_vdisplay - 1) |
+		       VTOTAL(crtc_vtotal - 1));
 	intel_de_write(dev_priv, TRANS_VBLANK(cpu_transcoder),
-		       (adjusted_mode->crtc_vblank_start - 1) | ((crtc_vblank_end - 1) << 16));
+		       VBLANK_START(adjusted_mode->crtc_vblank_start - 1) |
+		       VBLANK_END(crtc_vblank_end - 1));
 	intel_de_write(dev_priv, TRANS_VSYNC(cpu_transcoder),
-		       (adjusted_mode->crtc_vsync_start - 1) | ((adjusted_mode->crtc_vsync_end - 1) << 16));
+		       VSYNC_START(adjusted_mode->crtc_vsync_start - 1) |
+		       VSYNC_END(adjusted_mode->crtc_vsync_end - 1));
 
 	/* Workaround: when the EDP input selection is B, the VTOTAL_B must be
 	 * programmed with the VTOTAL_EDP value. Same for VTOTAL_C. This is
@@ -2815,30 +2821,31 @@ static void intel_get_transcoder_timings(struct intel_crtc *crtc,
 	u32 tmp;
 
 	tmp = intel_de_read(dev_priv, TRANS_HTOTAL(cpu_transcoder));
-	adjusted_mode->crtc_hdisplay = (tmp & 0xffff) + 1;
-	adjusted_mode->crtc_htotal = ((tmp >> 16) & 0xffff) + 1;
+	adjusted_mode->crtc_hdisplay = REG_FIELD_GET(HACTIVE_MASK, tmp) + 1;
+	adjusted_mode->crtc_htotal = REG_FIELD_GET(HTOTAL_MASK, tmp) + 1;
 
 	if (!transcoder_is_dsi(cpu_transcoder)) {
 		tmp = intel_de_read(dev_priv, TRANS_HBLANK(cpu_transcoder));
-		adjusted_mode->crtc_hblank_start = (tmp & 0xffff) + 1;
-		adjusted_mode->crtc_hblank_end = ((tmp >> 16) & 0xffff) + 1;
+		adjusted_mode->crtc_hblank_start = REG_FIELD_GET(HBLANK_START_MASK, tmp) + 1;
+		adjusted_mode->crtc_hblank_end = REG_FIELD_GET(HBLANK_END_MASK, tmp) + 1;
 	}
+
 	tmp = intel_de_read(dev_priv, TRANS_HSYNC(cpu_transcoder));
-	adjusted_mode->crtc_hsync_start = (tmp & 0xffff) + 1;
-	adjusted_mode->crtc_hsync_end = ((tmp >> 16) & 0xffff) + 1;
+	adjusted_mode->crtc_hsync_start = REG_FIELD_GET(HSYNC_START_MASK, tmp) + 1;
+	adjusted_mode->crtc_hsync_end = REG_FIELD_GET(HSYNC_END_MASK, tmp) + 1;
 
 	tmp = intel_de_read(dev_priv, TRANS_VTOTAL(cpu_transcoder));
-	adjusted_mode->crtc_vdisplay = (tmp & 0xffff) + 1;
-	adjusted_mode->crtc_vtotal = ((tmp >> 16) & 0xffff) + 1;
+	adjusted_mode->crtc_vdisplay = REG_FIELD_GET(VACTIVE_MASK, tmp) + 1;
+	adjusted_mode->crtc_vtotal = REG_FIELD_GET(VTOTAL_MASK, tmp) + 1;
 
 	if (!transcoder_is_dsi(cpu_transcoder)) {
 		tmp = intel_de_read(dev_priv, TRANS_VBLANK(cpu_transcoder));
-		adjusted_mode->crtc_vblank_start = (tmp & 0xffff) + 1;
-		adjusted_mode->crtc_vblank_end = ((tmp >> 16) & 0xffff) + 1;
+		adjusted_mode->crtc_vblank_start = REG_FIELD_GET(VBLANK_START_MASK, tmp) + 1;
+		adjusted_mode->crtc_vblank_end = REG_FIELD_GET(VBLANK_END_MASK, tmp) + 1;
 	}
 	tmp = intel_de_read(dev_priv, TRANS_VSYNC(cpu_transcoder));
-	adjusted_mode->crtc_vsync_start = (tmp & 0xffff) + 1;
-	adjusted_mode->crtc_vsync_end = ((tmp >> 16) & 0xffff) + 1;
+	adjusted_mode->crtc_vsync_start = REG_FIELD_GET(VSYNC_START_MASK, tmp) + 1;
+	adjusted_mode->crtc_vsync_end = REG_FIELD_GET(VSYNC_END_MASK, tmp) + 1;
 
 	if (intel_pipe_is_interlaced(pipe_config)) {
 		adjusted_mode->flags |= DRM_MODE_FLAG_INTERLACE;
@@ -8626,13 +8633,20 @@ void i830_enable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe)
 		PLL_REF_INPUT_DREFCLK |
 		DPLL_VCO_ENABLE;
 
-	intel_de_write(dev_priv, TRANS_HTOTAL(cpu_transcoder), (640 - 1) | ((800 - 1) << 16));
-	intel_de_write(dev_priv, TRANS_HBLANK(cpu_transcoder), (640 - 1) | ((800 - 1) << 16));
-	intel_de_write(dev_priv, TRANS_HSYNC(cpu_transcoder), (656 - 1) | ((752 - 1) << 16));
-	intel_de_write(dev_priv, TRANS_VTOTAL(cpu_transcoder), (480 - 1) | ((525 - 1) << 16));
-	intel_de_write(dev_priv, TRANS_VBLANK(cpu_transcoder), (480 - 1) | ((525 - 1) << 16));
-	intel_de_write(dev_priv, TRANS_VSYNC(cpu_transcoder), (490 - 1) | ((492 - 1) << 16));
-	intel_de_write(dev_priv, PIPESRC(pipe), ((640 - 1) << 16) | (480 - 1));
+	intel_de_write(dev_priv, TRANS_HTOTAL(cpu_transcoder),
+		       HACTIVE(640 - 1) | HTOTAL(800 - 1));
+	intel_de_write(dev_priv, TRANS_HBLANK(cpu_transcoder),
+		       HBLANK_START(640 - 1) | HBLANK_END(800 - 1));
+	intel_de_write(dev_priv, TRANS_HSYNC(cpu_transcoder),
+		       HSYNC_START(656 - 1) | HSYNC_END(752 - 1));
+	intel_de_write(dev_priv, TRANS_VTOTAL(cpu_transcoder),
+		       VACTIVE(480 - 1) | VTOTAL(525 - 1));
+	intel_de_write(dev_priv, TRANS_VBLANK(cpu_transcoder),
+		       VBLANK_START(480 - 1) | VBLANK_END(525 - 1));
+	intel_de_write(dev_priv, TRANS_VSYNC(cpu_transcoder),
+		       VSYNC_START(490 - 1) | VSYNC_END(492 - 1));
+	intel_de_write(dev_priv, PIPESRC(pipe),
+		       PIPESRC_WIDTH(640 - 1) | PIPESRC_HEIGHT(480 - 1));
 
 	intel_de_write(dev_priv, FP0(pipe), fp);
 	intel_de_write(dev_priv, FP1(pipe), fp);

drivers/gpu/drm/i915/i915_reg.h

@@ -1916,11 +1916,35 @@
 
 /* Pipe/transcoder A timing regs */
 #define _TRANS_HTOTAL_A		0x60000
+#define   HTOTAL_MASK			REG_GENMASK(31, 16)
+#define   HTOTAL(htotal)		REG_FIELD_PREP(HTOTAL_MASK, (htotal))
+#define   HACTIVE_MASK			REG_GENMASK(15, 0)
+#define   HACTIVE(hdisplay)		REG_FIELD_PREP(HACTIVE_MASK, (hdisplay))
 #define _TRANS_HBLANK_A		0x60004
+#define   HBLANK_END_MASK		REG_GENMASK(31, 16)
+#define   HBLANK_END(hblank_end)	REG_FIELD_PREP(HBLANK_END_MASK, (hblank_end))
+#define   HBLANK_START_MASK		REG_GENMASK(15, 0)
+#define   HBLANK_START(hblank_start)	REG_FIELD_PREP(HBLANK_START_MASK, (hblank_start))
 #define _TRANS_HSYNC_A		0x60008
+#define   HSYNC_END_MASK		REG_GENMASK(31, 16)
+#define   HSYNC_END(hsync_end)		REG_FIELD_PREP(HSYNC_END_MASK, (hsync_end))
+#define   HSYNC_START_MASK		REG_GENMASK(15, 0)
+#define   HSYNC_START(hsync_start)	REG_FIELD_PREP(HSYNC_START_MASK, (hsync_start))
 #define _TRANS_VTOTAL_A		0x6000c
+#define   VTOTAL_MASK			REG_GENMASK(31, 16)
+#define   VTOTAL(vtotal)		REG_FIELD_PREP(VTOTAL_MASK, (vtotal))
+#define   VACTIVE_MASK			REG_GENMASK(15, 0)
+#define   VACTIVE(vdisplay)		REG_FIELD_PREP(VACTIVE_MASK, (vdisplay))
 #define _TRANS_VBLANK_A		0x60010
+#define   VBLANK_END_MASK		REG_GENMASK(31, 16)
+#define   VBLANK_END(vblank_end)	REG_FIELD_PREP(VBLANK_END_MASK, (vblank_end))
+#define   VBLANK_START_MASK		REG_GENMASK(15, 0)
+#define   VBLANK_START(vblank_start)	REG_FIELD_PREP(VBLANK_START_MASK, (vblank_start))
 #define _TRANS_VSYNC_A		0x60014
+#define   VSYNC_END_MASK		REG_GENMASK(31, 16)
+#define   VSYNC_END(vsync_end)		REG_FIELD_PREP(VSYNC_END_MASK, (vsync_end))
+#define   VSYNC_START_MASK		REG_GENMASK(15, 0)
+#define   VSYNC_START(vsync_start)	REG_FIELD_PREP(VSYNC_START_MASK, (vsync_start))
 #define _TRANS_EXITLINE_A	0x60018
 #define _PIPEASRC		0x6001c
 #define   PIPESRC_WIDTH_MASK	REG_GENMASK(31, 16)