[Intel-gfx] [PATCH v2 0/7] drm/i915: Init DDI ports based on port_mask

[Intel-gfx] [PATCH v2 0/7] drm/i915: Init DDI ports based on port_mask

[Ville Syrjala]

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

Introduce port_mask into the device info and utilize it
it initalize DDI ports instead of hand rolling each
intel_ddi_init() call per platform+port.

This is an intermediate step towards initializing
DDI/DP/HDMI/DSI ports purely based on VBT information.

v2: rebased due to intel_display_device.c

Ville Syrjälä (7):
  drm/i915: Remove bogus DDI-F from hsw/bdw output init
  drm/i915: Introduce device info port_mask
  drm/i915: Assert that device info bitmasks have enough bits
  drm/i915: Assert that the port being initialized is valid
  drm/i915: Beef up SDVO/HDMI port checks
  drm/i915: Init DDI outputs based on port_mask on skl+
  drm/i915: Convert HSW/BDW to use port_mask for DDI probe

 drivers/gpu/drm/i915/display/g4x_dp.c         |   3 +
 drivers/gpu/drm/i915/display/g4x_hdmi.c       |  20 +++
 drivers/gpu/drm/i915/display/intel_crt.c      |   2 +
 drivers/gpu/drm/i915/display/intel_ddi.c      |  32 ++++
 drivers/gpu/drm/i915/display/intel_display.c  |  99 ++---------
 drivers/gpu/drm/i915/display/intel_display.h  |   2 +
 .../drm/i915/display/intel_display_device.c   | 155 ++++++++++++------
 .../drm/i915/display/intel_display_device.h   |   1 +
 drivers/gpu/drm/i915/display/intel_dvo.c      |   2 +
 drivers/gpu/drm/i915/display/intel_sdvo.c     |  20 ++-
 drivers/gpu/drm/i915/intel_device_info.c      |   9 +
 11 files changed, 208 insertions(+), 137 deletions(-)

-- 
2.39.3

[Intel-gfx] [PATCH v2 1/7] drm/i915: Remove bogus DDI-F from hsw/bdw output init

[Ville Syrjala]

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

HSW/BDW don't have DDI-F so don't go looking for one.

Seems to have been accidentally left behind when the
skl+ stuff got split out in commit 097d9e902068
("drm/i915/display: remove strap checks from gen 9").

Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index f51a55f4e9d0..4f158c428937 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -7466,8 +7466,6 @@ void intel_setup_outputs(struct drm_i915_private *dev_priv)
 			intel_ddi_init(dev_priv, PORT_C);
 		if (found & SFUSE_STRAP_DDID_DETECTED)
 			intel_ddi_init(dev_priv, PORT_D);
-		if (found & SFUSE_STRAP_DDIF_DETECTED)
-			intel_ddi_init(dev_priv, PORT_F);
 	} else if (HAS_PCH_SPLIT(dev_priv)) {
 		int found;
 
-- 
2.39.3

[Intel-gfx] [PATCH v2 2/7] drm/i915: Introduce device info port_mask

[Ville Syrjala]

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

Declare the available DVO/SDVO/HDMI/DP/DDI ports in the
device info. The other outputs (LVDS/TV/DSI/VGA) are left
out since for most of them we don't consider them as "ports".

DSI we should probably perhaps include somehow in the device
info. Just not sure how. Or we just introduce a HAS_DSI() and
call it a day?

TODO: figure out what to do about the subplatform stuff. Would
      it be better to declare those directly with a different
      device info or not? Also not sure the icl port-f stuff
      matters even. Bspec claims there are icl SKUs with far
      less ports than that and we don't seem to check for those
      either?

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 .../drm/i915/display/intel_display_device.c   | 155 ++++++++++++------
 .../drm/i915/display/intel_display_device.h   |   1 +
 drivers/gpu/drm/i915/intel_device_info.c      |   5 +
 3 files changed, 111 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_device.c b/drivers/gpu/drm/i915/display/intel_display_device.c
index a513ac8f71a3..3e0c768aaac9 100644
--- a/drivers/gpu/drm/i915/display/intel_display_device.c
+++ b/drivers/gpu/drm/i915/display/intel_display_device.c
@@ -182,10 +182,6 @@ static const struct intel_display_device_info no_display = {};
 	.__runtime_defaults.cpu_transcoder_mask = \
 		BIT(TRANSCODER_A) | BIT(TRANSCODER_B)
 
-static const struct intel_display_device_info i830_display = {
-	I830_DISPLAY,
-};
-
 #define I845_DISPLAY \
 	.has_overlay = 1, \
 	.overlay_needs_physical = 1, \
@@ -198,19 +194,29 @@ static const struct intel_display_device_info i830_display = {
 	.__runtime_defaults.pipe_mask = BIT(PIPE_A), \
 	.__runtime_defaults.cpu_transcoder_mask = BIT(TRANSCODER_A)
 
+static const struct intel_display_device_info i830_display = {
+	I830_DISPLAY,
+
+	.__runtime_defaults.port_mask = BIT(PORT_A) | BIT(PORT_B) | BIT(PORT_C), /* DVO A/B/C */
+};
+
 static const struct intel_display_device_info i845_display = {
 	I845_DISPLAY,
+
+	.__runtime_defaults.port_mask = BIT(PORT_B) | BIT(PORT_C), /* DVO B/C */
 };
 
 static const struct intel_display_device_info i85x_display = {
 	I830_DISPLAY,
 
+	.__runtime_defaults.port_mask = BIT(PORT_B) | BIT(PORT_C), /* DVO B/C */
 	.__runtime_defaults.fbc_mask = BIT(INTEL_FBC_A),
 };
 
 static const struct intel_display_device_info i865g_display = {
 	I845_DISPLAY,
 
+	.__runtime_defaults.port_mask = BIT(PORT_B) | BIT(PORT_C), /* DVO B/C */
 	.__runtime_defaults.fbc_mask = BIT(INTEL_FBC_A),
 };
 
@@ -224,7 +230,8 @@ static const struct intel_display_device_info i865g_display = {
 	.__runtime_defaults.ip.ver = 3, \
 	.__runtime_defaults.pipe_mask = BIT(PIPE_A) | BIT(PIPE_B), \
 	.__runtime_defaults.cpu_transcoder_mask = \
-		BIT(TRANSCODER_A) | BIT(TRANSCODER_B)
+		BIT(TRANSCODER_A) | BIT(TRANSCODER_B), \
+	.__runtime_defaults.port_mask = BIT(PORT_B) | BIT(PORT_C) /* SDVO B/C */
 
 static const struct intel_display_device_info i915g_display = {
 	GEN3_DISPLAY,
@@ -278,6 +285,8 @@ static const struct intel_display_device_info g33_display = {
 static const struct intel_display_device_info i965g_display = {
 	GEN4_DISPLAY,
 	.has_overlay = 1,
+
+	.__runtime_defaults.port_mask = BIT(PORT_B) | BIT(PORT_C), /* SDVO B/C */
 };
 
 static const struct intel_display_device_info i965gm_display = {
@@ -285,17 +294,21 @@ static const struct intel_display_device_info i965gm_display = {
 	.has_overlay = 1,
 	.supports_tv = 1,
 
+	.__runtime_defaults.port_mask = BIT(PORT_B) | BIT(PORT_C), /* SDVO B/C */
 	.__runtime_defaults.fbc_mask = BIT(INTEL_FBC_A),
 };
 
 static const struct intel_display_device_info g45_display = {
 	GEN4_DISPLAY,
+
+	.__runtime_defaults.port_mask = BIT(PORT_B) | BIT(PORT_C) | BIT(PORT_D), /* SDVO/HDMI/DP B/C, DP D */
 };
 
 static const struct intel_display_device_info gm45_display = {
 	GEN4_DISPLAY,
 	.supports_tv = 1,
 
+	.__runtime_defaults.port_mask = BIT(PORT_B) | BIT(PORT_C) | BIT(PORT_D), /* SDVO/HDMI/DP B/C, DP D */
 	.__runtime_defaults.fbc_mask = BIT(INTEL_FBC_A),
 };
 
@@ -308,7 +321,8 @@ static const struct intel_display_device_info gm45_display = {
 	.__runtime_defaults.ip.ver = 5, \
 	.__runtime_defaults.pipe_mask = BIT(PIPE_A) | BIT(PIPE_B), \
 	.__runtime_defaults.cpu_transcoder_mask = \
-		BIT(TRANSCODER_A) | BIT(TRANSCODER_B)
+		BIT(TRANSCODER_A) | BIT(TRANSCODER_B), \
+	.__runtime_defaults.port_mask = BIT(PORT_A) | BIT(PORT_B) | BIT(PORT_C) | BIT(PORT_D) /* DP A, SDVO/HDMI/DP B, HDMI/DP C/D */
 
 static const struct intel_display_device_info ilk_d_display = {
 	ILK_DISPLAY,
@@ -330,6 +344,7 @@ static const struct intel_display_device_info snb_display = {
 	.__runtime_defaults.pipe_mask = BIT(PIPE_A) | BIT(PIPE_B),
 	.__runtime_defaults.cpu_transcoder_mask =
 		BIT(TRANSCODER_A) | BIT(TRANSCODER_B),
+	.__runtime_defaults.port_mask = BIT(PORT_A) | BIT(PORT_B) | BIT(PORT_C) | BIT(PORT_D), /* DP A, SDVO/HDMI/DP B, HDMI/DP C/D */
 	.__runtime_defaults.fbc_mask = BIT(INTEL_FBC_A),
 };
 
@@ -343,6 +358,7 @@ static const struct intel_display_device_info ivb_display = {
 	.__runtime_defaults.pipe_mask = BIT(PIPE_A) | BIT(PIPE_B) | BIT(PIPE_C),
 	.__runtime_defaults.cpu_transcoder_mask =
 		BIT(TRANSCODER_A) | BIT(TRANSCODER_B) | BIT(TRANSCODER_C),
+	.__runtime_defaults.port_mask = BIT(PORT_A) | BIT(PORT_B) | BIT(PORT_C) | BIT(PORT_D), /* DP A, SDVO/HDMI/DP B, HDMI/DP C/D */
 	.__runtime_defaults.fbc_mask = BIT(INTEL_FBC_A),
 };
 
@@ -358,6 +374,7 @@ static const struct intel_display_device_info vlv_display = {
 	.__runtime_defaults.pipe_mask = BIT(PIPE_A) | BIT(PIPE_B),
 	.__runtime_defaults.cpu_transcoder_mask =
 		BIT(TRANSCODER_A) | BIT(TRANSCODER_B),
+	.__runtime_defaults.port_mask = BIT(PORT_B) | BIT(PORT_C), /* HDMI/DP B/C */
 };
 
 static const struct intel_display_device_info hsw_display = {
@@ -374,6 +391,7 @@ static const struct intel_display_device_info hsw_display = {
 	.__runtime_defaults.cpu_transcoder_mask =
 		BIT(TRANSCODER_A) | BIT(TRANSCODER_B) |
 		BIT(TRANSCODER_C) | BIT(TRANSCODER_EDP),
+	.__runtime_defaults.port_mask = BIT(PORT_A) | BIT(PORT_B) | BIT(PORT_C) | BIT(PORT_D) | BIT(PORT_E),
 	.__runtime_defaults.fbc_mask = BIT(INTEL_FBC_A),
 };
 
@@ -391,6 +409,7 @@ static const struct intel_display_device_info bdw_display = {
 	.__runtime_defaults.cpu_transcoder_mask =
 		BIT(TRANSCODER_A) | BIT(TRANSCODER_B) |
 		BIT(TRANSCODER_C) | BIT(TRANSCODER_EDP),
+	.__runtime_defaults.port_mask = BIT(PORT_A) | BIT(PORT_B) | BIT(PORT_C) | BIT(PORT_D) | BIT(PORT_E),
 	.__runtime_defaults.fbc_mask = BIT(INTEL_FBC_A),
 };
 
@@ -406,6 +425,7 @@ static const struct intel_display_device_info chv_display = {
 	.__runtime_defaults.pipe_mask = BIT(PIPE_A) | BIT(PIPE_B) | BIT(PIPE_C),
 	.__runtime_defaults.cpu_transcoder_mask =
 		BIT(TRANSCODER_A) | BIT(TRANSCODER_B) | BIT(TRANSCODER_C),
+	.__runtime_defaults.port_mask = BIT(PORT_B) | BIT(PORT_C) | BIT(PORT_D), /* HDMI/DP B/C/D */
 };
 
 static const struct intel_display_device_info skl_display = {
@@ -429,6 +449,7 @@ static const struct intel_display_device_info skl_display = {
 	.__runtime_defaults.cpu_transcoder_mask =
 		BIT(TRANSCODER_A) | BIT(TRANSCODER_B) |
 		BIT(TRANSCODER_C) | BIT(TRANSCODER_EDP),
+	.__runtime_defaults.port_mask = BIT(PORT_A) | BIT(PORT_B) | BIT(PORT_C) | BIT(PORT_D) | BIT(PORT_E),
 	.__runtime_defaults.fbc_mask = BIT(INTEL_FBC_A),
 };
 
@@ -452,7 +473,8 @@ static const struct intel_display_device_info skl_display = {
 	.__runtime_defaults.cpu_transcoder_mask = \
 		BIT(TRANSCODER_A) | BIT(TRANSCODER_B) | \
 		BIT(TRANSCODER_C) | BIT(TRANSCODER_EDP) | \
-		BIT(TRANSCODER_DSI_A) | BIT(TRANSCODER_DSI_C)
+		BIT(TRANSCODER_DSI_A) | BIT(TRANSCODER_DSI_C), \
+	.__runtime_defaults.port_mask = BIT(PORT_A) | BIT(PORT_B) | BIT(PORT_C)
 
 static const struct intel_display_device_info bxt_display = {
 	GEN9_LP_DISPLAY,
@@ -469,46 +491,58 @@ static const struct intel_display_device_info glk_display = {
 	.__runtime_defaults.ip.ver = 10,
 };
 
-static const struct intel_display_device_info gen11_display = {
-	.abox_mask = BIT(0),
-	.dbuf.size = 2048,
-	.dbuf.slice_mask = BIT(DBUF_S1) | BIT(DBUF_S2),
-	.has_ddi = 1,
-	.has_dp_mst = 1,
-	.has_fpga_dbg = 1,
-	.has_hotplug = 1,
-	.has_ipc = 1,
-	.has_psr = 1,
-	.has_psr_hw_tracking = 1,
-	.pipe_offsets = {
-		[TRANSCODER_A] = PIPE_A_OFFSET,
-		[TRANSCODER_B] = PIPE_B_OFFSET,
-		[TRANSCODER_C] = PIPE_C_OFFSET,
-		[TRANSCODER_EDP] = PIPE_EDP_OFFSET,
-		[TRANSCODER_DSI_0] = PIPE_DSI0_OFFSET,
-		[TRANSCODER_DSI_1] = PIPE_DSI1_OFFSET,
-	},
-	.trans_offsets = {
-		[TRANSCODER_A] = TRANSCODER_A_OFFSET,
-		[TRANSCODER_B] = TRANSCODER_B_OFFSET,
-		[TRANSCODER_C] = TRANSCODER_C_OFFSET,
-		[TRANSCODER_EDP] = TRANSCODER_EDP_OFFSET,
-		[TRANSCODER_DSI_0] = TRANSCODER_DSI0_OFFSET,
-		[TRANSCODER_DSI_1] = TRANSCODER_DSI1_OFFSET,
-	},
-	IVB_CURSOR_OFFSETS,
-	ICL_COLORS,
+#define ICL_DISPLAY \
+	.abox_mask = BIT(0), \
+	.dbuf.size = 2048, \
+	.dbuf.slice_mask = BIT(DBUF_S1) | BIT(DBUF_S2), \
+	.has_ddi = 1, \
+	.has_dp_mst = 1, \
+	.has_fpga_dbg = 1, \
+	.has_hotplug = 1, \
+	.has_ipc = 1, \
+	.has_psr = 1, \
+	.has_psr_hw_tracking = 1, \
+	.pipe_offsets = { \
+		[TRANSCODER_A] = PIPE_A_OFFSET, \
+		[TRANSCODER_B] = PIPE_B_OFFSET, \
+		[TRANSCODER_C] = PIPE_C_OFFSET, \
+		[TRANSCODER_EDP] = PIPE_EDP_OFFSET, \
+		[TRANSCODER_DSI_0] = PIPE_DSI0_OFFSET, \
+		[TRANSCODER_DSI_1] = PIPE_DSI1_OFFSET, \
+	}, \
+	.trans_offsets = { \
+		[TRANSCODER_A] = TRANSCODER_A_OFFSET, \
+		[TRANSCODER_B] = TRANSCODER_B_OFFSET, \
+		[TRANSCODER_C] = TRANSCODER_C_OFFSET, \
+		[TRANSCODER_EDP] = TRANSCODER_EDP_OFFSET, \
+		[TRANSCODER_DSI_0] = TRANSCODER_DSI0_OFFSET, \
+		[TRANSCODER_DSI_1] = TRANSCODER_DSI1_OFFSET, \
+	}, \
+	IVB_CURSOR_OFFSETS, \
+	ICL_COLORS, \
+	\
+	.__runtime_defaults.ip.ver = 11, \
+	.__runtime_defaults.has_dmc = 1, \
+	.__runtime_defaults.has_dsc = 1, \
+	.__runtime_defaults.has_hdcp = 1, \
+	.__runtime_defaults.pipe_mask = BIT(PIPE_A) | BIT(PIPE_B) | BIT(PIPE_C), \
+	.__runtime_defaults.cpu_transcoder_mask = \
+		BIT(TRANSCODER_A) | BIT(TRANSCODER_B) | \
+		BIT(TRANSCODER_C) | BIT(TRANSCODER_EDP) | \
+		BIT(TRANSCODER_DSI_0) | BIT(TRANSCODER_DSI_1), \
+	.__runtime_defaults.fbc_mask = BIT(INTEL_FBC_A)
 
-	.__runtime_defaults.ip.ver = 11,
-	.__runtime_defaults.has_dmc = 1,
-	.__runtime_defaults.has_dsc = 1,
-	.__runtime_defaults.has_hdcp = 1,
-	.__runtime_defaults.pipe_mask = BIT(PIPE_A) | BIT(PIPE_B) | BIT(PIPE_C),
-	.__runtime_defaults.cpu_transcoder_mask =
-		BIT(TRANSCODER_A) | BIT(TRANSCODER_B) |
-		BIT(TRANSCODER_C) | BIT(TRANSCODER_EDP) |
-		BIT(TRANSCODER_DSI_0) | BIT(TRANSCODER_DSI_1),
-	.__runtime_defaults.fbc_mask = BIT(INTEL_FBC_A),
+
+static const struct intel_display_device_info icl_display = {
+	ICL_DISPLAY,
+
+	.__runtime_defaults.port_mask = BIT(PORT_A) | BIT(PORT_B) | BIT(PORT_C) | BIT(PORT_D) | BIT(PORT_E),
+};
+
+static const struct intel_display_device_info jsl_ehl_display = {
+	ICL_DISPLAY,
+
+	.__runtime_defaults.port_mask = BIT(PORT_A) | BIT(PORT_B) | BIT(PORT_C) | BIT(PORT_D),
 };
 
 #define XE_D_DISPLAY \
@@ -556,6 +590,16 @@ static const struct intel_display_device_info gen11_display = {
 
 static const struct intel_display_device_info tgl_display = {
 	XE_D_DISPLAY,
+
+	.__runtime_defaults.port_mask = BIT(PORT_A) | BIT(PORT_B) | BIT(PORT_C) |
+		BIT(PORT_TC1) | BIT(PORT_TC2) | BIT(PORT_TC3) | BIT(PORT_TC4) | BIT(PORT_TC5) | BIT(PORT_TC5),
+};
+
+static const struct intel_display_device_info dg1_display = {
+	XE_D_DISPLAY,
+
+	.__runtime_defaults.port_mask = BIT(PORT_A) | BIT(PORT_B) |
+		BIT(PORT_TC1) | BIT(PORT_TC2),
 };
 
 static const struct intel_display_device_info rkl_display = {
@@ -567,12 +611,17 @@ static const struct intel_display_device_info rkl_display = {
 	.__runtime_defaults.pipe_mask = BIT(PIPE_A) | BIT(PIPE_B) | BIT(PIPE_C),
 	.__runtime_defaults.cpu_transcoder_mask =
 		BIT(TRANSCODER_A) | BIT(TRANSCODER_B) | BIT(TRANSCODER_C),
+	.__runtime_defaults.port_mask = BIT(PORT_A) | BIT(PORT_B) |
+		BIT(PORT_TC1) | BIT(PORT_TC2),
 };
 
 static const struct intel_display_device_info adl_s_display = {
 	XE_D_DISPLAY,
 	.has_hti = 1,
 	.has_psr_hw_tracking = 0,
+
+	.__runtime_defaults.port_mask = BIT(PORT_A) |
+		BIT(PORT_TC1) | BIT(PORT_TC2) | BIT(PORT_TC3) | BIT(PORT_TC4),
 };
 
 #define XE_LPD_FEATURES \
@@ -627,6 +676,8 @@ static const struct intel_display_device_info xe_lpd_display = {
 		BIT(TRANSCODER_A) | BIT(TRANSCODER_B) |
 		BIT(TRANSCODER_C) | BIT(TRANSCODER_D) |
 		BIT(TRANSCODER_DSI_0) | BIT(TRANSCODER_DSI_1),
+	.__runtime_defaults.port_mask = BIT(PORT_A) | BIT(PORT_B) |
+		BIT(PORT_TC1) | BIT(PORT_TC2) | BIT(PORT_TC3) | BIT(PORT_TC4),
 };
 
 static const struct intel_display_device_info xe_hpd_display = {
@@ -636,6 +687,8 @@ static const struct intel_display_device_info xe_hpd_display = {
 	.__runtime_defaults.cpu_transcoder_mask =
 		BIT(TRANSCODER_A) | BIT(TRANSCODER_B) |
 		BIT(TRANSCODER_C) | BIT(TRANSCODER_D),
+	.__runtime_defaults.port_mask = BIT(PORT_A) | BIT(PORT_B) | BIT(PORT_C) | BIT(PORT_D_XELPD) |
+		BIT(PORT_TC1),
 };
 
 static const struct intel_display_device_info xe_lpdp_display = {
@@ -648,6 +701,8 @@ static const struct intel_display_device_info xe_lpdp_display = {
 	.__runtime_defaults.cpu_transcoder_mask =
 		BIT(TRANSCODER_A) | BIT(TRANSCODER_B) |
 		BIT(TRANSCODER_C) | BIT(TRANSCODER_D),
+	.__runtime_defaults.port_mask = BIT(PORT_A) | BIT(PORT_B) |
+		BIT(PORT_TC1) | BIT(PORT_TC2) | BIT(PORT_TC3) | BIT(PORT_TC4),
 };
 
 #undef INTEL_VGA_DEVICE
@@ -690,11 +745,11 @@ static const struct {
 	INTEL_GLK_IDS(&glk_display),
 	INTEL_KBL_IDS(&skl_display),
 	INTEL_CFL_IDS(&skl_display),
-	INTEL_ICL_11_IDS(&gen11_display),
-	INTEL_EHL_IDS(&gen11_display),
-	INTEL_JSL_IDS(&gen11_display),
+	INTEL_ICL_11_IDS(&icl_display),
+	INTEL_EHL_IDS(&jsl_ehl_display),
+	INTEL_JSL_IDS(&jsl_ehl_display),
 	INTEL_TGL_12_IDS(&tgl_display),
-	INTEL_DG1_IDS(&tgl_display),
+	INTEL_DG1_IDS(&dg1_display),
 	INTEL_RKL_IDS(&rkl_display),
 	INTEL_ADLS_IDS(&adl_s_display),
 	INTEL_RPLS_IDS(&adl_s_display),
diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h b/drivers/gpu/drm/i915/display/intel_display_device.h
index 2aa82cbdf1c5..23a4c01d500f 100644
--- a/drivers/gpu/drm/i915/display/intel_display_device.h
+++ b/drivers/gpu/drm/i915/display/intel_display_device.h
@@ -79,6 +79,7 @@ struct intel_display_runtime_info {
 
 	u8 pipe_mask;
 	u8 cpu_transcoder_mask;
+	u16 port_mask;
 
 	u8 num_sprites[I915_MAX_PIPES];
 	u8 num_scalers[I915_MAX_PIPES];
diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
index 2f79d232b04a..f79142983f28 100644
--- a/drivers/gpu/drm/i915/intel_device_info.c
+++ b/drivers/gpu/drm/i915/intel_device_info.c
@@ -262,15 +262,19 @@ static void intel_device_info_subplatform_init(struct drm_i915_private *i915)
 	if (find_devid(devid, subplatform_ult_ids,
 		       ARRAY_SIZE(subplatform_ult_ids))) {
 		mask = BIT(INTEL_SUBPLATFORM_ULT);
+		if (IS_HASWELL(i915) || IS_BROADWELL(i915))
+			DISPLAY_RUNTIME_INFO(i915)->port_mask &= ~BIT(PORT_D);
 	} else if (find_devid(devid, subplatform_ulx_ids,
 			      ARRAY_SIZE(subplatform_ulx_ids))) {
 		mask = BIT(INTEL_SUBPLATFORM_ULX);
 		if (IS_HASWELL(i915) || IS_BROADWELL(i915)) {
 			/* ULX machines are also considered ULT. */
 			mask |= BIT(INTEL_SUBPLATFORM_ULT);
+			DISPLAY_RUNTIME_INFO(i915)->port_mask &= ~BIT(PORT_D);
 		}
 	} else if (find_devid(devid, subplatform_portf_ids,
 			      ARRAY_SIZE(subplatform_portf_ids))) {
+		DISPLAY_RUNTIME_INFO(i915)->port_mask |= BIT(PORT_F);
 		mask = BIT(INTEL_SUBPLATFORM_PORTF);
 	} else if (find_devid(devid, subplatform_uy_ids,
 			   ARRAY_SIZE(subplatform_uy_ids))) {
@@ -546,6 +550,7 @@ void intel_device_info_runtime_init(struct drm_i915_private *dev_priv)
 		info->display = &no_display;
 
 		display_runtime->cpu_transcoder_mask = 0;
+		display_runtime->port_mask = 0;
 		memset(display_runtime->num_sprites, 0, sizeof(display_runtime->num_sprites));
 		memset(display_runtime->num_scalers, 0, sizeof(display_runtime->num_scalers));
 		display_runtime->fbc_mask = 0;
-- 
2.39.3

Re: [Intel-gfx] [PATCH v2 2/7] drm/i915: Introduce device info port_mask

[Jani Nikula]

On Wed, 31 May 2023, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> @@ -556,6 +590,16 @@ static const struct intel_display_device_info gen11_display = {
>  
>  static const struct intel_display_device_info tgl_display = {
>  	XE_D_DISPLAY,
> +
> +	.__runtime_defaults.port_mask = BIT(PORT_A) | BIT(PORT_B) | BIT(PORT_C) |

Where does port C come from? It's not in intel_setup_outputs()
currently.

> +		BIT(PORT_TC1) | BIT(PORT_TC2) | BIT(PORT_TC3) | BIT(PORT_TC4) | BIT(PORT_TC5) | BIT(PORT_TC5),

TC5 duplicated, TC6 missing.

BR,
Jani.

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [Intel-gfx] [PATCH v2 2/7] drm/i915: Introduce device info port_mask

[Ville Syrjälä]

On Fri, Jun 02, 2023 at 05:11:45PM +0300, Jani Nikula wrote:
> On Wed, 31 May 2023, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> > @@ -556,6 +590,16 @@ static const struct intel_display_device_info gen11_display = {
> >  
> >  static const struct intel_display_device_info tgl_display = {
> >  	XE_D_DISPLAY,
> > +
> > +	.__runtime_defaults.port_mask = BIT(PORT_A) | BIT(PORT_B) | BIT(PORT_C) |
> 
> Where does port C come from?

From the spec.

> It's not in intel_setup_outputs()
> currently.

Hmm. Based on the history there seems to be some kind of
screwup in the combo PHY code wrt. ports that are not
actually present in the SKU. The spec says we should rely
on hpd to figure out what ports are actually present. So
looks like the combo PHY code needs quite a bit of redesign :(
I guess I'll leave this out until then.

> 
> > +		BIT(PORT_TC1) | BIT(PORT_TC2) | BIT(PORT_TC3) | BIT(PORT_TC4) | BIT(PORT_TC5) | BIT(PORT_TC5),
> 
> TC5 duplicated, TC6 missing.
> 
> BR,
> Jani.
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center

-- 
Ville Syrjälä
Intel

Re: [Intel-gfx] [PATCH v2 2/7] drm/i915: Introduce device info port_mask

[Shankar, Uma]

> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Ville Syrjälä
> Sent: Friday, June 9, 2023 1:06 AM
> To: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH v2 2/7] drm/i915: Introduce device info port_mask
> 
> On Fri, Jun 02, 2023 at 05:11:45PM +0300, Jani Nikula wrote:
> > On Wed, 31 May 2023, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> > > @@ -556,6 +590,16 @@ static const struct intel_display_device_info
> > > gen11_display = {
> > >
> > >  static const struct intel_display_device_info tgl_display = {
> > >  	XE_D_DISPLAY,
> > > +
> > > +	.__runtime_defaults.port_mask = BIT(PORT_A) | BIT(PORT_B) |
> > > +BIT(PORT_C) |
> >
> > Where does port C come from?
> 
> From the spec.
> 
> > It's not in intel_setup_outputs()
> > currently.
> 
> Hmm. Based on the history there seems to be some kind of screwup in the combo
> PHY code wrt. ports that are not actually present in the SKU. The spec says we
> should rely on hpd to figure out what ports are actually present. So looks like the
> combo PHY code needs quite a bit of redesign :( I guess I'll leave this out until then.
> 

This info on what ports are physically present is advertised via VBT currently.
But based on spec, we can declare the capability at setup and intel_ddi_init should
check VBT and proceed.

> > > +		BIT(PORT_TC1) | BIT(PORT_TC2) | BIT(PORT_TC3) | BIT(PORT_TC4)
> |
> > > +BIT(PORT_TC5) | BIT(PORT_TC5),
> >
> > TC5 duplicated, TC6 missing.

Yeah some typo here.

With the above addressed, this change looks good to me.
Reviewed-by: Uma Shankar <uma.shankar@intel.com>

> >
> > BR,
> > Jani.
> >
> > --
> > Jani Nikula, Intel Open Source Graphics Center
> 
> --
> Ville Syrjälä
> Intel

[Intel-gfx] [PATCH v2 3/7] drm/i915: Assert that device info bitmasks have enough bits

[Ville Syrjala]

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

Sprinkle in some BUILD_BUG_ON()s to make sure some of
the bitmasks used in the device info have enough bits.

Do we have a better place for this sort of stuff?

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

diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
index f79142983f28..8a35005c46c0 100644
--- a/drivers/gpu/drm/i915/intel_device_info.c
+++ b/drivers/gpu/drm/i915/intel_device_info.c
@@ -419,6 +419,10 @@ void intel_device_info_runtime_init(struct drm_i915_private *dev_priv)
 		DISPLAY_RUNTIME_INFO(dev_priv);
 	enum pipe pipe;
 
+	BUILD_BUG_ON(BITS_PER_TYPE(display_runtime->pipe_mask) < I915_MAX_PIPES);
+	BUILD_BUG_ON(BITS_PER_TYPE(display_runtime->cpu_transcoder_mask) < I915_MAX_TRANSCODERS);
+	BUILD_BUG_ON(BITS_PER_TYPE(display_runtime->port_mask) < I915_MAX_PORTS);
+
 	/* Wa_14011765242: adl-s A0,A1 */
 	if (IS_ADLS_DISPLAY_STEP(dev_priv, STEP_A0, STEP_A2))
 		for_each_pipe(dev_priv, pipe)
-- 
2.39.3

Re: [Intel-gfx] [PATCH v2 3/7] drm/i915: Assert that device info bitmasks have enough bits

[Jani Nikula]

On Wed, 31 May 2023, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Sprinkle in some BUILD_BUG_ON()s to make sure some of
> the bitmasks used in the device info have enough bits.
>
> Do we have a better place for this sort of stuff?

*grin* intel_display_device_info_runtime_init()

https://patchwork.freedesktop.org/patch/msgid/20230601212535.675751-1-matthew.d.roper@intel.com

It'll conflict, again. :/

>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_device_info.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
> index f79142983f28..8a35005c46c0 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.c
> +++ b/drivers/gpu/drm/i915/intel_device_info.c
> @@ -419,6 +419,10 @@ void intel_device_info_runtime_init(struct drm_i915_private *dev_priv)
>  		DISPLAY_RUNTIME_INFO(dev_priv);
>  	enum pipe pipe;
>  
> +	BUILD_BUG_ON(BITS_PER_TYPE(display_runtime->pipe_mask) < I915_MAX_PIPES);
> +	BUILD_BUG_ON(BITS_PER_TYPE(display_runtime->cpu_transcoder_mask) < I915_MAX_TRANSCODERS);
> +	BUILD_BUG_ON(BITS_PER_TYPE(display_runtime->port_mask) < I915_MAX_PORTS);
> +
>  	/* Wa_14011765242: adl-s A0,A1 */
>  	if (IS_ADLS_DISPLAY_STEP(dev_priv, STEP_A0, STEP_A2))
>  		for_each_pipe(dev_priv, pipe)

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [Intel-gfx] [PATCH v2 3/7] drm/i915: Assert that device info bitmasks have enough bits

[Jani Nikula]

On Fri, 02 Jun 2023, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> On Wed, 31 May 2023, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>
>> Sprinkle in some BUILD_BUG_ON()s to make sure some of
>> the bitmasks used in the device info have enough bits.
>>
>> Do we have a better place for this sort of stuff?
>
> *grin* intel_display_device_info_runtime_init()
>
> https://patchwork.freedesktop.org/patch/msgid/20230601212535.675751-1-matthew.d.roper@intel.com
>
> It'll conflict, again. :/

static_assert in the global scope is an option.

Also,

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

>
>>
>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_device_info.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
>> index f79142983f28..8a35005c46c0 100644
>> --- a/drivers/gpu/drm/i915/intel_device_info.c
>> +++ b/drivers/gpu/drm/i915/intel_device_info.c
>> @@ -419,6 +419,10 @@ void intel_device_info_runtime_init(struct drm_i915_private *dev_priv)
>>  		DISPLAY_RUNTIME_INFO(dev_priv);
>>  	enum pipe pipe;
>>  
>> +	BUILD_BUG_ON(BITS_PER_TYPE(display_runtime->pipe_mask) < I915_MAX_PIPES);
>> +	BUILD_BUG_ON(BITS_PER_TYPE(display_runtime->cpu_transcoder_mask) < I915_MAX_TRANSCODERS);
>> +	BUILD_BUG_ON(BITS_PER_TYPE(display_runtime->port_mask) < I915_MAX_PORTS);
>> +
>>  	/* Wa_14011765242: adl-s A0,A1 */
>>  	if (IS_ADLS_DISPLAY_STEP(dev_priv, STEP_A0, STEP_A2))
>>  		for_each_pipe(dev_priv, pipe)

-- 
Jani Nikula, Intel Open Source Graphics Center

[Intel-gfx] [PATCH v2 4/7] drm/i915: Assert that the port being initialized is valid

[Ville Syrjala]

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

Sprinkle some asserts to catch any mishaps in the port_mask
vs. output init.

For DDI/DP/HDMI/SDVO I decided that we want to bail out for
an invalid port since those are the encoder types where
we might want consider driving the whole thing from the VBT
child device list, and bogus VBTs could be a real issue
(if for no other reason than the i915.vbt_firmware).

For DVO and HSW/BDW CRT port I just threw the assert in
there for good measure.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/g4x_dp.c        | 3 +++
 drivers/gpu/drm/i915/display/g4x_hdmi.c      | 3 +++
 drivers/gpu/drm/i915/display/intel_crt.c     | 2 ++
 drivers/gpu/drm/i915/display/intel_ddi.c     | 3 +++
 drivers/gpu/drm/i915/display/intel_display.c | 6 ++++++
 drivers/gpu/drm/i915/display/intel_display.h | 2 ++
 drivers/gpu/drm/i915/display/intel_dvo.c     | 2 ++
 drivers/gpu/drm/i915/display/intel_sdvo.c    | 3 +++
 8 files changed, 24 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/g4x_dp.c b/drivers/gpu/drm/i915/display/g4x_dp.c
index 112d91d81fdc..c58a3f249a01 100644
--- a/drivers/gpu/drm/i915/display/g4x_dp.c
+++ b/drivers/gpu/drm/i915/display/g4x_dp.c
@@ -1259,6 +1259,9 @@ bool g4x_dp_init(struct drm_i915_private *dev_priv,
 	struct drm_encoder *encoder;
 	struct intel_connector *intel_connector;
 
+	if (!assert_port_valid(dev_priv, port))
+		return false;
+
 	devdata = intel_bios_encoder_data_lookup(dev_priv, port);
 
 	/* FIXME bail? */
diff --git a/drivers/gpu/drm/i915/display/g4x_hdmi.c b/drivers/gpu/drm/i915/display/g4x_hdmi.c
index 5c187e6e0472..59704939c111 100644
--- a/drivers/gpu/drm/i915/display/g4x_hdmi.c
+++ b/drivers/gpu/drm/i915/display/g4x_hdmi.c
@@ -667,6 +667,9 @@ void g4x_hdmi_init(struct drm_i915_private *dev_priv,
 	struct intel_encoder *intel_encoder;
 	struct intel_connector *intel_connector;
 
+	if (!assert_port_valid(dev_priv, port))
+		return;
+
 	devdata = intel_bios_encoder_data_lookup(dev_priv, port);
 
 	/* FIXME bail? */
diff --git a/drivers/gpu/drm/i915/display/intel_crt.c b/drivers/gpu/drm/i915/display/intel_crt.c
index 673c03646696..643acf90e6d6 100644
--- a/drivers/gpu/drm/i915/display/intel_crt.c
+++ b/drivers/gpu/drm/i915/display/intel_crt.c
@@ -1062,6 +1062,8 @@ void intel_crt_init(struct drm_i915_private *dev_priv)
 	}
 
 	if (HAS_DDI(dev_priv)) {
+		assert_port_valid(dev_priv, PORT_E);
+
 		crt->base.port = PORT_E;
 		crt->base.get_config = hsw_crt_get_config;
 		crt->base.get_hw_state = intel_ddi_get_hw_state;
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index d1a9a3cf94b5..31001b9a29b0 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -4665,6 +4665,9 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
 	bool init_hdmi, init_dp;
 	enum phy phy = intel_port_to_phy(dev_priv, port);
 
+	if (!assert_port_valid(dev_priv, port))
+		return;
+
 	/*
 	 * On platforms with HTI (aka HDPORT), if it's enabled at boot it may
 	 * have taken over some of the PHYs and made them unavailable to the
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 4f158c428937..d3fc498c82c1 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -7371,6 +7371,12 @@ static bool intel_ddi_crt_present(struct drm_i915_private *dev_priv)
 	return true;
 }
 
+bool assert_port_valid(struct drm_i915_private *i915, enum port port)
+{
+	return !drm_WARN(&i915->drm, !(DISPLAY_RUNTIME_INFO(i915)->port_mask & BIT(port)),
+			 "Platform does not support port %c\n", port_name(port));
+}
+
 void intel_setup_outputs(struct drm_i915_private *dev_priv)
 {
 	struct intel_encoder *encoder;
diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
index c744c021af23..53ca0e4e2357 100644
--- a/drivers/gpu/drm/i915/display/intel_display.h
+++ b/drivers/gpu/drm/i915/display/intel_display.h
@@ -539,6 +539,8 @@ void assert_transcoder(struct drm_i915_private *dev_priv,
 #define assert_transcoder_enabled(d, t) assert_transcoder(d, t, true)
 #define assert_transcoder_disabled(d, t) assert_transcoder(d, t, false)
 
+bool assert_port_valid(struct drm_i915_private *i915, enum port port);
+
 /*
  * Use I915_STATE_WARN(x) (rather than WARN() and WARN_ON()) for hw state sanity
  * checks to check for unexpected conditions which may not necessarily be a user
diff --git a/drivers/gpu/drm/i915/display/intel_dvo.c b/drivers/gpu/drm/i915/display/intel_dvo.c
index 9884678743b6..b386894c3a6d 100644
--- a/drivers/gpu/drm/i915/display/intel_dvo.c
+++ b/drivers/gpu/drm/i915/display/intel_dvo.c
@@ -509,6 +509,8 @@ void intel_dvo_init(struct drm_i915_private *i915)
 		return;
 	}
 
+	assert_port_valid(i915, intel_dvo->dev.port);
+
 	encoder->type = INTEL_OUTPUT_DVO;
 	encoder->power_domain = POWER_DOMAIN_PORT_OTHER;
 	encoder->port = intel_dvo->dev.port;
diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c
index 34ee9dd82a78..6ed613558cf8 100644
--- a/drivers/gpu/drm/i915/display/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
@@ -3330,6 +3330,9 @@ bool intel_sdvo_init(struct drm_i915_private *dev_priv,
 	struct intel_sdvo *intel_sdvo;
 	int i;
 
+	if (!assert_port_valid(dev_priv, port))
+		return false;
+
 	assert_sdvo_port_valid(dev_priv, port);
 
 	intel_sdvo = kzalloc(sizeof(*intel_sdvo), GFP_KERNEL);
-- 
2.39.3

Re: [Intel-gfx] [PATCH v2 4/7] drm/i915: Assert that the port being initialized is valid

[Jani Nikula]

On Wed, 31 May 2023, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Sprinkle some asserts to catch any mishaps in the port_mask
> vs. output init.
>
> For DDI/DP/HDMI/SDVO I decided that we want to bail out for
> an invalid port since those are the encoder types where
> we might want consider driving the whole thing from the VBT
> child device list, and bogus VBTs could be a real issue
> (if for no other reason than the i915.vbt_firmware).
>
> For DVO and HSW/BDW CRT port I just threw the assert in
> there for good measure.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

I've got increasingly mixed feelings about the assert_* naming,
including in the existing functions, because "assert" feels like it's
only there for debug builds.

Regardless, they should be all renamed in one go instead of forking
another naming lineage right here.

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


> ---
>  drivers/gpu/drm/i915/display/g4x_dp.c        | 3 +++
>  drivers/gpu/drm/i915/display/g4x_hdmi.c      | 3 +++
>  drivers/gpu/drm/i915/display/intel_crt.c     | 2 ++
>  drivers/gpu/drm/i915/display/intel_ddi.c     | 3 +++
>  drivers/gpu/drm/i915/display/intel_display.c | 6 ++++++
>  drivers/gpu/drm/i915/display/intel_display.h | 2 ++
>  drivers/gpu/drm/i915/display/intel_dvo.c     | 2 ++
>  drivers/gpu/drm/i915/display/intel_sdvo.c    | 3 +++
>  8 files changed, 24 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/g4x_dp.c b/drivers/gpu/drm/i915/display/g4x_dp.c
> index 112d91d81fdc..c58a3f249a01 100644
> --- a/drivers/gpu/drm/i915/display/g4x_dp.c
> +++ b/drivers/gpu/drm/i915/display/g4x_dp.c
> @@ -1259,6 +1259,9 @@ bool g4x_dp_init(struct drm_i915_private *dev_priv,
>  	struct drm_encoder *encoder;
>  	struct intel_connector *intel_connector;
>  
> +	if (!assert_port_valid(dev_priv, port))
> +		return false;
> +
>  	devdata = intel_bios_encoder_data_lookup(dev_priv, port);
>  
>  	/* FIXME bail? */
> diff --git a/drivers/gpu/drm/i915/display/g4x_hdmi.c b/drivers/gpu/drm/i915/display/g4x_hdmi.c
> index 5c187e6e0472..59704939c111 100644
> --- a/drivers/gpu/drm/i915/display/g4x_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/g4x_hdmi.c
> @@ -667,6 +667,9 @@ void g4x_hdmi_init(struct drm_i915_private *dev_priv,
>  	struct intel_encoder *intel_encoder;
>  	struct intel_connector *intel_connector;
>  
> +	if (!assert_port_valid(dev_priv, port))
> +		return;
> +
>  	devdata = intel_bios_encoder_data_lookup(dev_priv, port);
>  
>  	/* FIXME bail? */
> diff --git a/drivers/gpu/drm/i915/display/intel_crt.c b/drivers/gpu/drm/i915/display/intel_crt.c
> index 673c03646696..643acf90e6d6 100644
> --- a/drivers/gpu/drm/i915/display/intel_crt.c
> +++ b/drivers/gpu/drm/i915/display/intel_crt.c
> @@ -1062,6 +1062,8 @@ void intel_crt_init(struct drm_i915_private *dev_priv)
>  	}
>  
>  	if (HAS_DDI(dev_priv)) {
> +		assert_port_valid(dev_priv, PORT_E);
> +
>  		crt->base.port = PORT_E;
>  		crt->base.get_config = hsw_crt_get_config;
>  		crt->base.get_hw_state = intel_ddi_get_hw_state;
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index d1a9a3cf94b5..31001b9a29b0 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -4665,6 +4665,9 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>  	bool init_hdmi, init_dp;
>  	enum phy phy = intel_port_to_phy(dev_priv, port);
>  
> +	if (!assert_port_valid(dev_priv, port))
> +		return;
> +
>  	/*
>  	 * On platforms with HTI (aka HDPORT), if it's enabled at boot it may
>  	 * have taken over some of the PHYs and made them unavailable to the
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 4f158c428937..d3fc498c82c1 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -7371,6 +7371,12 @@ static bool intel_ddi_crt_present(struct drm_i915_private *dev_priv)
>  	return true;
>  }
>  
> +bool assert_port_valid(struct drm_i915_private *i915, enum port port)
> +{
> +	return !drm_WARN(&i915->drm, !(DISPLAY_RUNTIME_INFO(i915)->port_mask & BIT(port)),
> +			 "Platform does not support port %c\n", port_name(port));
> +}
> +
>  void intel_setup_outputs(struct drm_i915_private *dev_priv)
>  {
>  	struct intel_encoder *encoder;
> diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
> index c744c021af23..53ca0e4e2357 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.h
> +++ b/drivers/gpu/drm/i915/display/intel_display.h
> @@ -539,6 +539,8 @@ void assert_transcoder(struct drm_i915_private *dev_priv,
>  #define assert_transcoder_enabled(d, t) assert_transcoder(d, t, true)
>  #define assert_transcoder_disabled(d, t) assert_transcoder(d, t, false)
>  
> +bool assert_port_valid(struct drm_i915_private *i915, enum port port);
> +
>  /*
>   * Use I915_STATE_WARN(x) (rather than WARN() and WARN_ON()) for hw state sanity
>   * checks to check for unexpected conditions which may not necessarily be a user
> diff --git a/drivers/gpu/drm/i915/display/intel_dvo.c b/drivers/gpu/drm/i915/display/intel_dvo.c
> index 9884678743b6..b386894c3a6d 100644
> --- a/drivers/gpu/drm/i915/display/intel_dvo.c
> +++ b/drivers/gpu/drm/i915/display/intel_dvo.c
> @@ -509,6 +509,8 @@ void intel_dvo_init(struct drm_i915_private *i915)
>  		return;
>  	}
>  
> +	assert_port_valid(i915, intel_dvo->dev.port);
> +
>  	encoder->type = INTEL_OUTPUT_DVO;
>  	encoder->power_domain = POWER_DOMAIN_PORT_OTHER;
>  	encoder->port = intel_dvo->dev.port;
> diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c
> index 34ee9dd82a78..6ed613558cf8 100644
> --- a/drivers/gpu/drm/i915/display/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
> @@ -3330,6 +3330,9 @@ bool intel_sdvo_init(struct drm_i915_private *dev_priv,
>  	struct intel_sdvo *intel_sdvo;
>  	int i;
>  
> +	if (!assert_port_valid(dev_priv, port))
> +		return false;
> +
>  	assert_sdvo_port_valid(dev_priv, port);
>  
>  	intel_sdvo = kzalloc(sizeof(*intel_sdvo), GFP_KERNEL);

-- 
Jani Nikula, Intel Open Source Graphics Center

[Intel-gfx] [PATCH v2 5/7] drm/i915: Beef up SDVO/HDMI port checks

[Ville Syrjala]

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

The SDVO code already warns when the port in question doesn't
actually support SDVO. Let's make that also bail the encoder
registration like the generic assert_port_valid() we added.

And add a similar thing for g4x HDMI, mainly because on g4x
itsefl port D only supports DP but not SDVO/HDMI. For the
other platforms the generic port_mask check should actually
be sufficient, but since we're here might as well list the
ports.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/g4x_hdmi.c   | 17 +++++++++++++++++
 drivers/gpu/drm/i915/display/intel_sdvo.c | 17 ++++++++++++-----
 2 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/g4x_hdmi.c b/drivers/gpu/drm/i915/display/g4x_hdmi.c
index 59704939c111..8c71e3ede680 100644
--- a/drivers/gpu/drm/i915/display/g4x_hdmi.c
+++ b/drivers/gpu/drm/i915/display/g4x_hdmi.c
@@ -659,6 +659,20 @@ int g4x_hdmi_connector_atomic_check(struct drm_connector *connector,
 	return ret;
 }
 
+static bool is_hdmi_port_valid(struct drm_i915_private *i915, enum port port)
+{
+	if (IS_G4X(i915) || IS_VALLEYVIEW(i915))
+		return port == PORT_B || port == PORT_C;
+	else
+		return port == PORT_B || port == PORT_C || port == PORT_D;
+}
+
+static bool assert_hdmi_port_valid(struct drm_i915_private *i915, enum port port)
+{
+	return !drm_WARN(&i915->drm, !is_hdmi_port_valid(i915, port),
+			 "Platform does not support HDMI %c\n", port_name(port));
+}
+
 void g4x_hdmi_init(struct drm_i915_private *dev_priv,
 		   i915_reg_t hdmi_reg, enum port port)
 {
@@ -670,6 +684,9 @@ void g4x_hdmi_init(struct drm_i915_private *dev_priv,
 	if (!assert_port_valid(dev_priv, port))
 		return;
 
+	if (!assert_hdmi_port_valid(dev_priv, port))
+		return;
+
 	devdata = intel_bios_encoder_data_lookup(dev_priv, port);
 
 	/* FIXME bail? */
diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c
index 6ed613558cf8..e6b140b073c3 100644
--- a/drivers/gpu/drm/i915/display/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
@@ -3314,13 +3314,19 @@ intel_sdvo_init_ddc_proxy(struct intel_sdvo *sdvo,
 	return i2c_add_adapter(&sdvo->ddc) == 0;
 }
 
-static void assert_sdvo_port_valid(const struct drm_i915_private *dev_priv,
-				   enum port port)
+static bool is_sdvo_port_valid(struct drm_i915_private *dev_priv, enum port port)
 {
 	if (HAS_PCH_SPLIT(dev_priv))
-		drm_WARN_ON(&dev_priv->drm, port != PORT_B);
+		return port == PORT_B;
 	else
-		drm_WARN_ON(&dev_priv->drm, port != PORT_B && port != PORT_C);
+		return port == PORT_B || port == PORT_C;
+}
+
+static bool assert_sdvo_port_valid(struct drm_i915_private *dev_priv,
+				   enum port port)
+{
+	return !drm_WARN(&dev_priv->drm, !is_sdvo_port_valid(dev_priv, port),
+			 "Platform does not support SDVO %c\n", port_name(port));
 }
 
 bool intel_sdvo_init(struct drm_i915_private *dev_priv,
@@ -3333,7 +3339,8 @@ bool intel_sdvo_init(struct drm_i915_private *dev_priv,
 	if (!assert_port_valid(dev_priv, port))
 		return false;
 
-	assert_sdvo_port_valid(dev_priv, port);
+	if (!assert_sdvo_port_valid(dev_priv, port))
+		return false;
 
 	intel_sdvo = kzalloc(sizeof(*intel_sdvo), GFP_KERNEL);
 	if (!intel_sdvo)
-- 
2.39.3

Re: [Intel-gfx] [PATCH v2 5/7] drm/i915: Beef up SDVO/HDMI port checks

[Jani Nikula]

On Wed, 31 May 2023, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> The SDVO code already warns when the port in question doesn't
> actually support SDVO. Let's make that also bail the encoder
> registration like the generic assert_port_valid() we added.
>
> And add a similar thing for g4x HDMI, mainly because on g4x
> itsefl port D only supports DP but not SDVO/HDMI. For the
> other platforms the generic port_mask check should actually
> be sufficient, but since we're here might as well list the
> ports.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

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

> ---
>  drivers/gpu/drm/i915/display/g4x_hdmi.c   | 17 +++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_sdvo.c | 17 ++++++++++++-----
>  2 files changed, 29 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/g4x_hdmi.c b/drivers/gpu/drm/i915/display/g4x_hdmi.c
> index 59704939c111..8c71e3ede680 100644
> --- a/drivers/gpu/drm/i915/display/g4x_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/g4x_hdmi.c
> @@ -659,6 +659,20 @@ int g4x_hdmi_connector_atomic_check(struct drm_connector *connector,
>  	return ret;
>  }
>  
> +static bool is_hdmi_port_valid(struct drm_i915_private *i915, enum port port)
> +{
> +	if (IS_G4X(i915) || IS_VALLEYVIEW(i915))
> +		return port == PORT_B || port == PORT_C;
> +	else
> +		return port == PORT_B || port == PORT_C || port == PORT_D;
> +}
> +
> +static bool assert_hdmi_port_valid(struct drm_i915_private *i915, enum port port)
> +{
> +	return !drm_WARN(&i915->drm, !is_hdmi_port_valid(i915, port),
> +			 "Platform does not support HDMI %c\n", port_name(port));
> +}
> +
>  void g4x_hdmi_init(struct drm_i915_private *dev_priv,
>  		   i915_reg_t hdmi_reg, enum port port)
>  {
> @@ -670,6 +684,9 @@ void g4x_hdmi_init(struct drm_i915_private *dev_priv,
>  	if (!assert_port_valid(dev_priv, port))
>  		return;
>  
> +	if (!assert_hdmi_port_valid(dev_priv, port))
> +		return;
> +
>  	devdata = intel_bios_encoder_data_lookup(dev_priv, port);
>  
>  	/* FIXME bail? */
> diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c
> index 6ed613558cf8..e6b140b073c3 100644
> --- a/drivers/gpu/drm/i915/display/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
> @@ -3314,13 +3314,19 @@ intel_sdvo_init_ddc_proxy(struct intel_sdvo *sdvo,
>  	return i2c_add_adapter(&sdvo->ddc) == 0;
>  }
>  
> -static void assert_sdvo_port_valid(const struct drm_i915_private *dev_priv,
> -				   enum port port)
> +static bool is_sdvo_port_valid(struct drm_i915_private *dev_priv, enum port port)
>  {
>  	if (HAS_PCH_SPLIT(dev_priv))
> -		drm_WARN_ON(&dev_priv->drm, port != PORT_B);
> +		return port == PORT_B;
>  	else
> -		drm_WARN_ON(&dev_priv->drm, port != PORT_B && port != PORT_C);
> +		return port == PORT_B || port == PORT_C;
> +}
> +
> +static bool assert_sdvo_port_valid(struct drm_i915_private *dev_priv,
> +				   enum port port)
> +{
> +	return !drm_WARN(&dev_priv->drm, !is_sdvo_port_valid(dev_priv, port),
> +			 "Platform does not support SDVO %c\n", port_name(port));
>  }
>  
>  bool intel_sdvo_init(struct drm_i915_private *dev_priv,
> @@ -3333,7 +3339,8 @@ bool intel_sdvo_init(struct drm_i915_private *dev_priv,
>  	if (!assert_port_valid(dev_priv, port))
>  		return false;
>  
> -	assert_sdvo_port_valid(dev_priv, port);
> +	if (!assert_sdvo_port_valid(dev_priv, port))
> +		return false;
>  
>  	intel_sdvo = kzalloc(sizeof(*intel_sdvo), GFP_KERNEL);
>  	if (!intel_sdvo)

-- 
Jani Nikula, Intel Open Source Graphics Center

[Intel-gfx] [PATCH v2 6/7] drm/i915: Init DDI outputs based on port_mask on skl+

[Ville Syrjala]

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

Instead of listing every platform's possible DDI outputs
in intel_setup_outputs() just loop over the new port_mask
to achieve the same thing.

HSW/BDW were left as is since they still look at the straps
as well.

DSI is still a mess. For now just check for the relevant
platforms explicitly.

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

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index d3fc498c82c1..12f2e3897595 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -7387,73 +7387,19 @@ void intel_setup_outputs(struct drm_i915_private *dev_priv)
 	if (!HAS_DISPLAY(dev_priv))
 		return;
 
-	if (IS_METEORLAKE(dev_priv)) {
-		intel_ddi_init(dev_priv, PORT_A);
-		intel_ddi_init(dev_priv, PORT_B);
-		intel_ddi_init(dev_priv, PORT_TC1);
-		intel_ddi_init(dev_priv, PORT_TC2);
-		intel_ddi_init(dev_priv, PORT_TC3);
-		intel_ddi_init(dev_priv, PORT_TC4);
-	} else if (IS_DG2(dev_priv)) {
-		intel_ddi_init(dev_priv, PORT_A);
-		intel_ddi_init(dev_priv, PORT_B);
-		intel_ddi_init(dev_priv, PORT_C);
-		intel_ddi_init(dev_priv, PORT_D_XELPD);
-		intel_ddi_init(dev_priv, PORT_TC1);
-	} else if (IS_ALDERLAKE_P(dev_priv)) {
-		intel_ddi_init(dev_priv, PORT_A);
-		intel_ddi_init(dev_priv, PORT_B);
-		intel_ddi_init(dev_priv, PORT_TC1);
-		intel_ddi_init(dev_priv, PORT_TC2);
-		intel_ddi_init(dev_priv, PORT_TC3);
-		intel_ddi_init(dev_priv, PORT_TC4);
-		icl_dsi_init(dev_priv);
-	} else if (IS_ALDERLAKE_S(dev_priv)) {
-		intel_ddi_init(dev_priv, PORT_A);
-		intel_ddi_init(dev_priv, PORT_TC1);
-		intel_ddi_init(dev_priv, PORT_TC2);
-		intel_ddi_init(dev_priv, PORT_TC3);
-		intel_ddi_init(dev_priv, PORT_TC4);
-	} else if (IS_DG1(dev_priv) || IS_ROCKETLAKE(dev_priv)) {
-		intel_ddi_init(dev_priv, PORT_A);
-		intel_ddi_init(dev_priv, PORT_B);
-		intel_ddi_init(dev_priv, PORT_TC1);
-		intel_ddi_init(dev_priv, PORT_TC2);
-	} else if (DISPLAY_VER(dev_priv) >= 12) {
-		intel_ddi_init(dev_priv, PORT_A);
-		intel_ddi_init(dev_priv, PORT_B);
-		intel_ddi_init(dev_priv, PORT_TC1);
-		intel_ddi_init(dev_priv, PORT_TC2);
-		intel_ddi_init(dev_priv, PORT_TC3);
-		intel_ddi_init(dev_priv, PORT_TC4);
-		intel_ddi_init(dev_priv, PORT_TC5);
-		intel_ddi_init(dev_priv, PORT_TC6);
-		icl_dsi_init(dev_priv);
-	} else if (IS_JSL_EHL(dev_priv)) {
-		intel_ddi_init(dev_priv, PORT_A);
-		intel_ddi_init(dev_priv, PORT_B);
-		intel_ddi_init(dev_priv, PORT_C);
-		intel_ddi_init(dev_priv, PORT_D);
-		icl_dsi_init(dev_priv);
-	} else if (DISPLAY_VER(dev_priv) == 11) {
-		intel_ddi_init(dev_priv, PORT_A);
-		intel_ddi_init(dev_priv, PORT_B);
-		intel_ddi_init(dev_priv, PORT_C);
-		intel_ddi_init(dev_priv, PORT_D);
-		intel_ddi_init(dev_priv, PORT_E);
-		intel_ddi_init(dev_priv, PORT_F);
-		icl_dsi_init(dev_priv);
-	} else if (IS_GEMINILAKE(dev_priv) || IS_BROXTON(dev_priv)) {
-		intel_ddi_init(dev_priv, PORT_A);
-		intel_ddi_init(dev_priv, PORT_B);
-		intel_ddi_init(dev_priv, PORT_C);
-		vlv_dsi_init(dev_priv);
-	} else if (DISPLAY_VER(dev_priv) >= 9) {
-		intel_ddi_init(dev_priv, PORT_A);
-		intel_ddi_init(dev_priv, PORT_B);
-		intel_ddi_init(dev_priv, PORT_C);
-		intel_ddi_init(dev_priv, PORT_D);
-		intel_ddi_init(dev_priv, PORT_E);
+	if (DISPLAY_VER(dev_priv) >= 9) {
+		enum port port;
+
+		for_each_port_masked(port, DISPLAY_RUNTIME_INFO(dev_priv)->port_mask)
+			intel_ddi_init(dev_priv, port);
+
+		/* FIXME do something about DSI */
+		if (IS_ALDERLAKE_P(dev_priv) || IS_TIGERLAKE(dev_priv) ||
+		    DISPLAY_VER(dev_priv) == 11)
+			icl_dsi_init(dev_priv);
+
+		if (IS_GEMINILAKE(dev_priv) || IS_BROXTON(dev_priv))
+			vlv_dsi_init(dev_priv);
 	} else if (HAS_DDI(dev_priv)) {
 		u32 found;
 
-- 
2.39.3

Re: [Intel-gfx] [PATCH v2 6/7] drm/i915: Init DDI outputs based on port_mask on skl+

[Jani Nikula]

On Wed, 31 May 2023, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Instead of listing every platform's possible DDI outputs
> in intel_setup_outputs() just loop over the new port_mask
> to achieve the same thing.

For posterity, I think I rejected a patch from Lucas generalizing the
initialization in the past. I think that used the VBT child device list
directly, and I wanted to preserve a clear way to check what the
supported ports for a platform were. I think having the ports in runtime
info now covers that concern. And with this, I'm open to using the child
device list as it can now be cross-checked against the runtime info.

>
> HSW/BDW were left as is since they still look at the straps
> as well.
>
> DSI is still a mess. For now just check for the relevant
> platforms explicitly.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 80 ++++----------------
>  1 file changed, 13 insertions(+), 67 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index d3fc498c82c1..12f2e3897595 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -7387,73 +7387,19 @@ void intel_setup_outputs(struct drm_i915_private *dev_priv)
>  	if (!HAS_DISPLAY(dev_priv))
>  		return;
>  
> -	if (IS_METEORLAKE(dev_priv)) {
> -		intel_ddi_init(dev_priv, PORT_A);
> -		intel_ddi_init(dev_priv, PORT_B);
> -		intel_ddi_init(dev_priv, PORT_TC1);
> -		intel_ddi_init(dev_priv, PORT_TC2);
> -		intel_ddi_init(dev_priv, PORT_TC3);
> -		intel_ddi_init(dev_priv, PORT_TC4);
> -	} else if (IS_DG2(dev_priv)) {
> -		intel_ddi_init(dev_priv, PORT_A);
> -		intel_ddi_init(dev_priv, PORT_B);
> -		intel_ddi_init(dev_priv, PORT_C);
> -		intel_ddi_init(dev_priv, PORT_D_XELPD);
> -		intel_ddi_init(dev_priv, PORT_TC1);
> -	} else if (IS_ALDERLAKE_P(dev_priv)) {
> -		intel_ddi_init(dev_priv, PORT_A);
> -		intel_ddi_init(dev_priv, PORT_B);
> -		intel_ddi_init(dev_priv, PORT_TC1);
> -		intel_ddi_init(dev_priv, PORT_TC2);
> -		intel_ddi_init(dev_priv, PORT_TC3);
> -		intel_ddi_init(dev_priv, PORT_TC4);
> -		icl_dsi_init(dev_priv);
> -	} else if (IS_ALDERLAKE_S(dev_priv)) {
> -		intel_ddi_init(dev_priv, PORT_A);
> -		intel_ddi_init(dev_priv, PORT_TC1);
> -		intel_ddi_init(dev_priv, PORT_TC2);
> -		intel_ddi_init(dev_priv, PORT_TC3);
> -		intel_ddi_init(dev_priv, PORT_TC4);
> -	} else if (IS_DG1(dev_priv) || IS_ROCKETLAKE(dev_priv)) {
> -		intel_ddi_init(dev_priv, PORT_A);
> -		intel_ddi_init(dev_priv, PORT_B);
> -		intel_ddi_init(dev_priv, PORT_TC1);
> -		intel_ddi_init(dev_priv, PORT_TC2);
> -	} else if (DISPLAY_VER(dev_priv) >= 12) {
> -		intel_ddi_init(dev_priv, PORT_A);
> -		intel_ddi_init(dev_priv, PORT_B);
> -		intel_ddi_init(dev_priv, PORT_TC1);
> -		intel_ddi_init(dev_priv, PORT_TC2);
> -		intel_ddi_init(dev_priv, PORT_TC3);
> -		intel_ddi_init(dev_priv, PORT_TC4);
> -		intel_ddi_init(dev_priv, PORT_TC5);
> -		intel_ddi_init(dev_priv, PORT_TC6);
> -		icl_dsi_init(dev_priv);
> -	} else if (IS_JSL_EHL(dev_priv)) {
> -		intel_ddi_init(dev_priv, PORT_A);
> -		intel_ddi_init(dev_priv, PORT_B);
> -		intel_ddi_init(dev_priv, PORT_C);
> -		intel_ddi_init(dev_priv, PORT_D);
> -		icl_dsi_init(dev_priv);
> -	} else if (DISPLAY_VER(dev_priv) == 11) {
> -		intel_ddi_init(dev_priv, PORT_A);
> -		intel_ddi_init(dev_priv, PORT_B);
> -		intel_ddi_init(dev_priv, PORT_C);
> -		intel_ddi_init(dev_priv, PORT_D);
> -		intel_ddi_init(dev_priv, PORT_E);
> -		intel_ddi_init(dev_priv, PORT_F);
> -		icl_dsi_init(dev_priv);
> -	} else if (IS_GEMINILAKE(dev_priv) || IS_BROXTON(dev_priv)) {
> -		intel_ddi_init(dev_priv, PORT_A);
> -		intel_ddi_init(dev_priv, PORT_B);
> -		intel_ddi_init(dev_priv, PORT_C);
> -		vlv_dsi_init(dev_priv);
> -	} else if (DISPLAY_VER(dev_priv) >= 9) {
> -		intel_ddi_init(dev_priv, PORT_A);
> -		intel_ddi_init(dev_priv, PORT_B);
> -		intel_ddi_init(dev_priv, PORT_C);
> -		intel_ddi_init(dev_priv, PORT_D);
> -		intel_ddi_init(dev_priv, PORT_E);
> +	if (DISPLAY_VER(dev_priv) >= 9) {
> +		enum port port;
> +
> +		for_each_port_masked(port, DISPLAY_RUNTIME_INFO(dev_priv)->port_mask)
> +			intel_ddi_init(dev_priv, port);
> +
> +		/* FIXME do something about DSI */
> +		if (IS_ALDERLAKE_P(dev_priv) || IS_TIGERLAKE(dev_priv) ||
> +		    DISPLAY_VER(dev_priv) == 11)
> +			icl_dsi_init(dev_priv);

This reflects current code, but apparently commit e341c618acde
("drm/i915/adl_s: Initialize display for ADL-S") stopped initializing
DSI for ADL-S. It does support DSI.

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


> +
> +		if (IS_GEMINILAKE(dev_priv) || IS_BROXTON(dev_priv))
> +			vlv_dsi_init(dev_priv);
>  	} else if (HAS_DDI(dev_priv)) {
>  		u32 found;

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [Intel-gfx] [PATCH v2 6/7] drm/i915: Init DDI outputs based on port_mask on skl+

[Ville Syrjälä]

On Fri, Jun 02, 2023 at 05:41:50PM +0300, Jani Nikula wrote:
> On Wed, 31 May 2023, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Instead of listing every platform's possible DDI outputs
> > in intel_setup_outputs() just loop over the new port_mask
> > to achieve the same thing.
> 
> For posterity, I think I rejected a patch from Lucas generalizing the
> initialization in the past. I think that used the VBT child device list
> directly, and I wanted to preserve a clear way to check what the
> supported ports for a platform were. I think having the ports in runtime
> info now covers that concern. And with this, I'm open to using the child
> device list as it can now be cross-checked against the runtime info.
> 
> >
> > HSW/BDW were left as is since they still look at the straps
> > as well.
> >
> > DSI is still a mess. For now just check for the relevant
> > platforms explicitly.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_display.c | 80 ++++----------------
> >  1 file changed, 13 insertions(+), 67 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index d3fc498c82c1..12f2e3897595 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -7387,73 +7387,19 @@ void intel_setup_outputs(struct drm_i915_private *dev_priv)
> >  	if (!HAS_DISPLAY(dev_priv))
> >  		return;
> >  
> > -	if (IS_METEORLAKE(dev_priv)) {
> > -		intel_ddi_init(dev_priv, PORT_A);
> > -		intel_ddi_init(dev_priv, PORT_B);
> > -		intel_ddi_init(dev_priv, PORT_TC1);
> > -		intel_ddi_init(dev_priv, PORT_TC2);
> > -		intel_ddi_init(dev_priv, PORT_TC3);
> > -		intel_ddi_init(dev_priv, PORT_TC4);
> > -	} else if (IS_DG2(dev_priv)) {
> > -		intel_ddi_init(dev_priv, PORT_A);
> > -		intel_ddi_init(dev_priv, PORT_B);
> > -		intel_ddi_init(dev_priv, PORT_C);
> > -		intel_ddi_init(dev_priv, PORT_D_XELPD);
> > -		intel_ddi_init(dev_priv, PORT_TC1);
> > -	} else if (IS_ALDERLAKE_P(dev_priv)) {
> > -		intel_ddi_init(dev_priv, PORT_A);
> > -		intel_ddi_init(dev_priv, PORT_B);
> > -		intel_ddi_init(dev_priv, PORT_TC1);
> > -		intel_ddi_init(dev_priv, PORT_TC2);
> > -		intel_ddi_init(dev_priv, PORT_TC3);
> > -		intel_ddi_init(dev_priv, PORT_TC4);
> > -		icl_dsi_init(dev_priv);
> > -	} else if (IS_ALDERLAKE_S(dev_priv)) {
> > -		intel_ddi_init(dev_priv, PORT_A);
> > -		intel_ddi_init(dev_priv, PORT_TC1);
> > -		intel_ddi_init(dev_priv, PORT_TC2);
> > -		intel_ddi_init(dev_priv, PORT_TC3);
> > -		intel_ddi_init(dev_priv, PORT_TC4);
> > -	} else if (IS_DG1(dev_priv) || IS_ROCKETLAKE(dev_priv)) {
> > -		intel_ddi_init(dev_priv, PORT_A);
> > -		intel_ddi_init(dev_priv, PORT_B);
> > -		intel_ddi_init(dev_priv, PORT_TC1);
> > -		intel_ddi_init(dev_priv, PORT_TC2);
> > -	} else if (DISPLAY_VER(dev_priv) >= 12) {
> > -		intel_ddi_init(dev_priv, PORT_A);
> > -		intel_ddi_init(dev_priv, PORT_B);
> > -		intel_ddi_init(dev_priv, PORT_TC1);
> > -		intel_ddi_init(dev_priv, PORT_TC2);
> > -		intel_ddi_init(dev_priv, PORT_TC3);
> > -		intel_ddi_init(dev_priv, PORT_TC4);
> > -		intel_ddi_init(dev_priv, PORT_TC5);
> > -		intel_ddi_init(dev_priv, PORT_TC6);
> > -		icl_dsi_init(dev_priv);
> > -	} else if (IS_JSL_EHL(dev_priv)) {
> > -		intel_ddi_init(dev_priv, PORT_A);
> > -		intel_ddi_init(dev_priv, PORT_B);
> > -		intel_ddi_init(dev_priv, PORT_C);
> > -		intel_ddi_init(dev_priv, PORT_D);
> > -		icl_dsi_init(dev_priv);
> > -	} else if (DISPLAY_VER(dev_priv) == 11) {
> > -		intel_ddi_init(dev_priv, PORT_A);
> > -		intel_ddi_init(dev_priv, PORT_B);
> > -		intel_ddi_init(dev_priv, PORT_C);
> > -		intel_ddi_init(dev_priv, PORT_D);
> > -		intel_ddi_init(dev_priv, PORT_E);
> > -		intel_ddi_init(dev_priv, PORT_F);
> > -		icl_dsi_init(dev_priv);
> > -	} else if (IS_GEMINILAKE(dev_priv) || IS_BROXTON(dev_priv)) {
> > -		intel_ddi_init(dev_priv, PORT_A);
> > -		intel_ddi_init(dev_priv, PORT_B);
> > -		intel_ddi_init(dev_priv, PORT_C);
> > -		vlv_dsi_init(dev_priv);
> > -	} else if (DISPLAY_VER(dev_priv) >= 9) {
> > -		intel_ddi_init(dev_priv, PORT_A);
> > -		intel_ddi_init(dev_priv, PORT_B);
> > -		intel_ddi_init(dev_priv, PORT_C);
> > -		intel_ddi_init(dev_priv, PORT_D);
> > -		intel_ddi_init(dev_priv, PORT_E);
> > +	if (DISPLAY_VER(dev_priv) >= 9) {
> > +		enum port port;
> > +
> > +		for_each_port_masked(port, DISPLAY_RUNTIME_INFO(dev_priv)->port_mask)
> > +			intel_ddi_init(dev_priv, port);
> > +
> > +		/* FIXME do something about DSI */
> > +		if (IS_ALDERLAKE_P(dev_priv) || IS_TIGERLAKE(dev_priv) ||
> > +		    DISPLAY_VER(dev_priv) == 11)
> > +			icl_dsi_init(dev_priv);
> 
> This reflects current code, but apparently commit e341c618acde
> ("drm/i915/adl_s: Initialize display for ADL-S") stopped initializing
> DSI for ADL-S. It does support DSI.

Not according to bspec. The diagram does still show the
DSI transcoders being present but the PHY is missing.

> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> 
> 
> > +
> > +		if (IS_GEMINILAKE(dev_priv) || IS_BROXTON(dev_priv))
> > +			vlv_dsi_init(dev_priv);
> >  	} else if (HAS_DDI(dev_priv)) {
> >  		u32 found;
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center

-- 
Ville Syrjälä
Intel

Re: [Intel-gfx] [PATCH v2 6/7] drm/i915: Init DDI outputs based on port_mask on skl+

[Jani Nikula]

On Thu, 08 Jun 2023, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Fri, Jun 02, 2023 at 05:41:50PM +0300, Jani Nikula wrote:
>> This reflects current code, but apparently commit e341c618acde
>> ("drm/i915/adl_s: Initialize display for ADL-S") stopped initializing
>> DSI for ADL-S. It does support DSI.
>
> Not according to bspec. The diagram does still show the
> DSI transcoders being present but the PHY is missing.

My bad. R-b holds.

>
>> 
>> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
>> 
>> 
>> > +
>> > +		if (IS_GEMINILAKE(dev_priv) || IS_BROXTON(dev_priv))
>> > +			vlv_dsi_init(dev_priv);
>> >  	} else if (HAS_DDI(dev_priv)) {
>> >  		u32 found;
>> 
>> -- 
>> Jani Nikula, Intel Open Source Graphics Center

-- 
Jani Nikula, Intel Open Source Graphics Center

[Intel-gfx] [PATCH v2 7/7] drm/i915: Convert HSW/BDW to use port_mask for DDI probe

[Ville Syrjala]

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

Make HSW/BDW use port_mask for output probing as well.
To achieve that the strap checks are moved into
intel_ddi_init() itself. Or should we move them to the
runtime port_mask init instead? Maybe not since the hardware
is still there, just not connected to anything.

v2: Account for DDI-E in strap detection
    Keep to the old CRT->DDI init order

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_ddi.c     | 29 ++++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_display.c | 23 +++-------------
 2 files changed, 33 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 31001b9a29b0..d89a9b85a780 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -4657,6 +4657,29 @@ static void intel_ddi_tc_encoder_shutdown_complete(struct intel_encoder *encoder
 #define port_tc_name(port) ((port) - PORT_TC1 + '1')
 #define tc_port_name(tc_port) ((tc_port) - TC_PORT_1 + '1')
 
+static bool port_strap_detected(struct drm_i915_private *i915, enum port port)
+{
+	/* straps not used on skl+ */
+	if (DISPLAY_VER(i915) >= 9)
+		return true;
+
+	switch (port) {
+	case PORT_A:
+		return intel_de_read(i915, DDI_BUF_CTL(PORT_A)) & DDI_INIT_DISPLAY_DETECTED;
+	case PORT_B:
+		return intel_de_read(i915, SFUSE_STRAP) & SFUSE_STRAP_DDIB_DETECTED;
+	case PORT_C:
+		return intel_de_read(i915, SFUSE_STRAP) & SFUSE_STRAP_DDIC_DETECTED;
+	case PORT_D:
+		return intel_de_read(i915, SFUSE_STRAP) & SFUSE_STRAP_DDID_DETECTED;
+	case PORT_E:
+		return true; /* no strap for DDI-E */
+	default:
+		MISSING_CASE(port);
+		return false;
+	}
+}
+
 void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
 {
 	struct intel_digital_port *dig_port;
@@ -4665,6 +4688,12 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
 	bool init_hdmi, init_dp;
 	enum phy phy = intel_port_to_phy(dev_priv, port);
 
+	if (!port_strap_detected(dev_priv, port)) {
+		drm_dbg_kms(&dev_priv->drm,
+			    "Port %c strap not detected\n", port_name(port));
+		return;
+	}
+
 	if (!assert_port_valid(dev_priv, port))
 		return;
 
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 12f2e3897595..1ae4854b275e 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -7387,9 +7387,12 @@ void intel_setup_outputs(struct drm_i915_private *dev_priv)
 	if (!HAS_DISPLAY(dev_priv))
 		return;
 
-	if (DISPLAY_VER(dev_priv) >= 9) {
+	if (HAS_DDI(dev_priv)) {
 		enum port port;
 
+		if (intel_ddi_crt_present(dev_priv))
+			intel_crt_init(dev_priv);
+
 		for_each_port_masked(port, DISPLAY_RUNTIME_INFO(dev_priv)->port_mask)
 			intel_ddi_init(dev_priv, port);
 
@@ -7400,24 +7403,6 @@ void intel_setup_outputs(struct drm_i915_private *dev_priv)
 
 		if (IS_GEMINILAKE(dev_priv) || IS_BROXTON(dev_priv))
 			vlv_dsi_init(dev_priv);
-	} else if (HAS_DDI(dev_priv)) {
-		u32 found;
-
-		if (intel_ddi_crt_present(dev_priv))
-			intel_crt_init(dev_priv);
-
-		/* Haswell uses DDI functions to detect digital outputs. */
-		found = intel_de_read(dev_priv, DDI_BUF_CTL(PORT_A)) & DDI_INIT_DISPLAY_DETECTED;
-		if (found)
-			intel_ddi_init(dev_priv, PORT_A);
-
-		found = intel_de_read(dev_priv, SFUSE_STRAP);
-		if (found & SFUSE_STRAP_DDIB_DETECTED)
-			intel_ddi_init(dev_priv, PORT_B);
-		if (found & SFUSE_STRAP_DDIC_DETECTED)
-			intel_ddi_init(dev_priv, PORT_C);
-		if (found & SFUSE_STRAP_DDID_DETECTED)
-			intel_ddi_init(dev_priv, PORT_D);
 	} else if (HAS_PCH_SPLIT(dev_priv)) {
 		int found;
 
-- 
2.39.3

Re: [Intel-gfx] [PATCH v2 7/7] drm/i915: Convert HSW/BDW to use port_mask for DDI probe

[Jani Nikula]

On Wed, 31 May 2023, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Make HSW/BDW use port_mask for output probing as well.
> To achieve that the strap checks are moved into
> intel_ddi_init() itself. Or should we move them to the
> runtime port_mask init instead? Maybe not since the hardware
> is still there, just not connected to anything.
>
> v2: Account for DDI-E in strap detection
>     Keep to the old CRT->DDI init order
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c     | 29 ++++++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_display.c | 23 +++-------------
>  2 files changed, 33 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 31001b9a29b0..d89a9b85a780 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -4657,6 +4657,29 @@ static void intel_ddi_tc_encoder_shutdown_complete(struct intel_encoder *encoder
>  #define port_tc_name(port) ((port) - PORT_TC1 + '1')
>  #define tc_port_name(tc_port) ((tc_port) - TC_PORT_1 + '1')
>  
> +static bool port_strap_detected(struct drm_i915_private *i915, enum port port)
> +{
> +	/* straps not used on skl+ */
> +	if (DISPLAY_VER(i915) >= 9)
> +		return true;
> +
> +	switch (port) {
> +	case PORT_A:
> +		return intel_de_read(i915, DDI_BUF_CTL(PORT_A)) & DDI_INIT_DISPLAY_DETECTED;
> +	case PORT_B:
> +		return intel_de_read(i915, SFUSE_STRAP) & SFUSE_STRAP_DDIB_DETECTED;
> +	case PORT_C:
> +		return intel_de_read(i915, SFUSE_STRAP) & SFUSE_STRAP_DDIC_DETECTED;
> +	case PORT_D:
> +		return intel_de_read(i915, SFUSE_STRAP) & SFUSE_STRAP_DDID_DETECTED;
> +	case PORT_E:
> +		return true; /* no strap for DDI-E */
> +	default:
> +		MISSING_CASE(port);
> +		return false;
> +	}
> +}
> +
>  void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>  {
>  	struct intel_digital_port *dig_port;
> @@ -4665,6 +4688,12 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>  	bool init_hdmi, init_dp;
>  	enum phy phy = intel_port_to_phy(dev_priv, port);
>  
> +	if (!port_strap_detected(dev_priv, port)) {
> +		drm_dbg_kms(&dev_priv->drm,
> +			    "Port %c strap not detected\n", port_name(port));
> +		return;
> +	}
> +
>  	if (!assert_port_valid(dev_priv, port))
>  		return;

I might have put this check first before the straps, so we get the
invalid port message for bogus ports instead of MISSING_CASE().

Either way,

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

>  
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 12f2e3897595..1ae4854b275e 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -7387,9 +7387,12 @@ void intel_setup_outputs(struct drm_i915_private *dev_priv)
>  	if (!HAS_DISPLAY(dev_priv))
>  		return;
>  
> -	if (DISPLAY_VER(dev_priv) >= 9) {
> +	if (HAS_DDI(dev_priv)) {
>  		enum port port;
>  
> +		if (intel_ddi_crt_present(dev_priv))
> +			intel_crt_init(dev_priv);
> +
>  		for_each_port_masked(port, DISPLAY_RUNTIME_INFO(dev_priv)->port_mask)
>  			intel_ddi_init(dev_priv, port);
>  
> @@ -7400,24 +7403,6 @@ void intel_setup_outputs(struct drm_i915_private *dev_priv)
>  
>  		if (IS_GEMINILAKE(dev_priv) || IS_BROXTON(dev_priv))
>  			vlv_dsi_init(dev_priv);
> -	} else if (HAS_DDI(dev_priv)) {
> -		u32 found;
> -
> -		if (intel_ddi_crt_present(dev_priv))
> -			intel_crt_init(dev_priv);
> -
> -		/* Haswell uses DDI functions to detect digital outputs. */
> -		found = intel_de_read(dev_priv, DDI_BUF_CTL(PORT_A)) & DDI_INIT_DISPLAY_DETECTED;
> -		if (found)
> -			intel_ddi_init(dev_priv, PORT_A);
> -
> -		found = intel_de_read(dev_priv, SFUSE_STRAP);
> -		if (found & SFUSE_STRAP_DDIB_DETECTED)
> -			intel_ddi_init(dev_priv, PORT_B);
> -		if (found & SFUSE_STRAP_DDIC_DETECTED)
> -			intel_ddi_init(dev_priv, PORT_C);
> -		if (found & SFUSE_STRAP_DDID_DETECTED)
> -			intel_ddi_init(dev_priv, PORT_D);
>  	} else if (HAS_PCH_SPLIT(dev_priv)) {
>  		int found;

-- 
Jani Nikula, Intel Open Source Graphics Center

Re: [Intel-gfx] [PATCH v2 7/7] drm/i915: Convert HSW/BDW to use port_mask for DDI probe

[Ville Syrjälä]

On Fri, Jun 02, 2023 at 05:51:19PM +0300, Jani Nikula wrote:
> On Wed, 31 May 2023, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Make HSW/BDW use port_mask for output probing as well.
> > To achieve that the strap checks are moved into
> > intel_ddi_init() itself. Or should we move them to the
> > runtime port_mask init instead? Maybe not since the hardware
> > is still there, just not connected to anything.
> >
> > v2: Account for DDI-E in strap detection
> >     Keep to the old CRT->DDI init order
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_ddi.c     | 29 ++++++++++++++++++++
> >  drivers/gpu/drm/i915/display/intel_display.c | 23 +++-------------
> >  2 files changed, 33 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index 31001b9a29b0..d89a9b85a780 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -4657,6 +4657,29 @@ static void intel_ddi_tc_encoder_shutdown_complete(struct intel_encoder *encoder
> >  #define port_tc_name(port) ((port) - PORT_TC1 + '1')
> >  #define tc_port_name(tc_port) ((tc_port) - TC_PORT_1 + '1')
> >  
> > +static bool port_strap_detected(struct drm_i915_private *i915, enum port port)
> > +{
> > +	/* straps not used on skl+ */
> > +	if (DISPLAY_VER(i915) >= 9)
> > +		return true;
> > +
> > +	switch (port) {
> > +	case PORT_A:
> > +		return intel_de_read(i915, DDI_BUF_CTL(PORT_A)) & DDI_INIT_DISPLAY_DETECTED;
> > +	case PORT_B:
> > +		return intel_de_read(i915, SFUSE_STRAP) & SFUSE_STRAP_DDIB_DETECTED;
> > +	case PORT_C:
> > +		return intel_de_read(i915, SFUSE_STRAP) & SFUSE_STRAP_DDIC_DETECTED;
> > +	case PORT_D:
> > +		return intel_de_read(i915, SFUSE_STRAP) & SFUSE_STRAP_DDID_DETECTED;
> > +	case PORT_E:
> > +		return true; /* no strap for DDI-E */
> > +	default:
> > +		MISSING_CASE(port);
> > +		return false;
> > +	}
> > +}
> > +
> >  void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
> >  {
> >  	struct intel_digital_port *dig_port;
> > @@ -4665,6 +4688,12 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
> >  	bool init_hdmi, init_dp;
> >  	enum phy phy = intel_port_to_phy(dev_priv, port);
> >  
> > +	if (!port_strap_detected(dev_priv, port)) {
> > +		drm_dbg_kms(&dev_priv->drm,
> > +			    "Port %c strap not detected\n", port_name(port));
> > +		return;
> > +	}
> > +
> >  	if (!assert_port_valid(dev_priv, port))
> >  		return;
> 
> I might have put this check first before the straps, so we get the
> invalid port message for bogus ports instead of MISSING_CASE().

That would trip assert_port_valid() when we try to init DDI D on
ULT/ULX machines.

Sadly I also have a one HSW ULT machine here where the VBT
(arguably incorrectly) declares that DDI D is present. So
even when we switch to VBT based init we need to do the strap
check first. I guess one option would be to remove the
MISSING_CASE() and just return true for all DDI E+.

> 
> Either way,
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> 
> >  
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index 12f2e3897595..1ae4854b275e 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -7387,9 +7387,12 @@ void intel_setup_outputs(struct drm_i915_private *dev_priv)
> >  	if (!HAS_DISPLAY(dev_priv))
> >  		return;
> >  
> > -	if (DISPLAY_VER(dev_priv) >= 9) {
> > +	if (HAS_DDI(dev_priv)) {
> >  		enum port port;
> >  
> > +		if (intel_ddi_crt_present(dev_priv))
> > +			intel_crt_init(dev_priv);
> > +
> >  		for_each_port_masked(port, DISPLAY_RUNTIME_INFO(dev_priv)->port_mask)
> >  			intel_ddi_init(dev_priv, port);
> >  
> > @@ -7400,24 +7403,6 @@ void intel_setup_outputs(struct drm_i915_private *dev_priv)
> >  
> >  		if (IS_GEMINILAKE(dev_priv) || IS_BROXTON(dev_priv))
> >  			vlv_dsi_init(dev_priv);
> > -	} else if (HAS_DDI(dev_priv)) {
> > -		u32 found;
> > -
> > -		if (intel_ddi_crt_present(dev_priv))
> > -			intel_crt_init(dev_priv);
> > -
> > -		/* Haswell uses DDI functions to detect digital outputs. */
> > -		found = intel_de_read(dev_priv, DDI_BUF_CTL(PORT_A)) & DDI_INIT_DISPLAY_DETECTED;
> > -		if (found)
> > -			intel_ddi_init(dev_priv, PORT_A);
> > -
> > -		found = intel_de_read(dev_priv, SFUSE_STRAP);
> > -		if (found & SFUSE_STRAP_DDIB_DETECTED)
> > -			intel_ddi_init(dev_priv, PORT_B);
> > -		if (found & SFUSE_STRAP_DDIC_DETECTED)
> > -			intel_ddi_init(dev_priv, PORT_C);
> > -		if (found & SFUSE_STRAP_DDID_DETECTED)
> > -			intel_ddi_init(dev_priv, PORT_D);
> >  	} else if (HAS_PCH_SPLIT(dev_priv)) {
> >  		int found;
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center

-- 
Ville Syrjälä
Intel

[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915: Init DDI ports based on port_mask (rev3)

[Patchwork]

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

== Series Details ==

Series: drm/i915: Init DDI ports based on port_mask (rev3)
URL   : https://patchwork.freedesktop.org/series/117641/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_13208_full -> Patchwork_117641v3_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Participating hosts (8 -> 7)
------------------------------

  Missing    (1): shard-rkl0 

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_lmem_swapping@parallel-random:
    - shard-glk:          NOTRUN -> [SKIP][1] ([fdo#109271] / [i915#4613])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117641v3/shard-glk3/igt@gem_lmem_swapping@parallel-random.html

  * igt@kms_ccs@pipe-b-crc-primary-basic-y_tiled_gen12_rc_ccs_cc:
    - shard-glk:          NOTRUN -> [SKIP][2] ([fdo#109271] / [i915#3886])
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117641v3/shard-glk3/igt@kms_ccs@pipe-b-crc-primary-basic-y_tiled_gen12_rc_ccs_cc.html

  * igt@kms_content_protection@srm:
    - shard-glk:          NOTRUN -> [SKIP][3] ([fdo#109271] / [i915#4579]) +1 similar issue
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117641v3/shard-glk3/igt@kms_content_protection@srm.html

  * igt@kms_plane_scaling@invalid-num-scalers@pipe-a-hdmi-a-1-invalid-num-scalers:
    - shard-snb:          NOTRUN -> [SKIP][4] ([fdo#109271]) +14 similar issues
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117641v3/shard-snb1/igt@kms_plane_scaling@invalid-num-scalers@pipe-a-hdmi-a-1-invalid-num-scalers.html

  * igt@kms_plane_scaling@plane-upscale-with-modifiers-20x20@pipe-b-vga-1:
    - shard-snb:          NOTRUN -> [SKIP][5] ([fdo#109271] / [i915#4579]) +8 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117641v3/shard-snb4/igt@kms_plane_scaling@plane-upscale-with-modifiers-20x20@pipe-b-vga-1.html

  * igt@kms_psr2_sf@primary-plane-update-sf-dmg-area-big-fb:
    - shard-glk:          NOTRUN -> [SKIP][6] ([fdo#109271] / [i915#658])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117641v3/shard-glk3/igt@kms_psr2_sf@primary-plane-update-sf-dmg-area-big-fb.html

  * igt@kms_psr@psr2_sprite_blt:
    - shard-glk:          NOTRUN -> [SKIP][7] ([fdo#109271]) +51 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117641v3/shard-glk3/igt@kms_psr@psr2_sprite_blt.html

  
#### Possible fixes ####

  * igt@drm_fdinfo@most-busy-check-all@rcs0:
    - {shard-rkl}:        [FAIL][8] ([i915#7742]) -> [PASS][9]
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13208/shard-rkl-2/igt@drm_fdinfo@most-busy-check-all@rcs0.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117641v3/shard-rkl-4/igt@drm_fdinfo@most-busy-check-all@rcs0.html

  * igt@gem_eio@hibernate:
    - {shard-tglu}:       [ABORT][10] ([i915#7975] / [i915#8213] / [i915#8398]) -> [PASS][11]
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13208/shard-tglu-10/igt@gem_eio@hibernate.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117641v3/shard-tglu-8/igt@gem_eio@hibernate.html

  * igt@gem_exec_fair@basic-deadline:
    - {shard-rkl}:        [FAIL][12] ([i915#2846]) -> [PASS][13]
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13208/shard-rkl-2/igt@gem_exec_fair@basic-deadline.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117641v3/shard-rkl-4/igt@gem_exec_fair@basic-deadline.html

  * igt@gem_exec_fair@basic-none@bcs0:
    - {shard-rkl}:        [FAIL][14] ([i915#2842]) -> [PASS][15] +2 similar issues
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13208/shard-rkl-4/igt@gem_exec_fair@basic-none@bcs0.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117641v3/shard-rkl-3/igt@gem_exec_fair@basic-none@bcs0.html

  * igt@gem_exec_fair@basic-pace-share@rcs0:
    - shard-glk:          [FAIL][16] ([i915#2842]) -> [PASS][17]
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13208/shard-glk8/igt@gem_exec_fair@basic-pace-share@rcs0.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117641v3/shard-glk3/igt@gem_exec_fair@basic-pace-share@rcs0.html

  * igt@i915_pm_dc@dc6-dpms:
    - {shard-tglu}:       [FAIL][18] ([i915#3989] / [i915#454]) -> [PASS][19]
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13208/shard-tglu-3/igt@i915_pm_dc@dc6-dpms.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117641v3/shard-tglu-4/igt@i915_pm_dc@dc6-dpms.html

  * igt@i915_pm_rc6_residency@rc6-idle@rcs0:
    - {shard-dg1}:        [FAIL][20] ([i915#3591]) -> [PASS][21]
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13208/shard-dg1-14/igt@i915_pm_rc6_residency@rc6-idle@rcs0.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117641v3/shard-dg1-14/igt@i915_pm_rc6_residency@rc6-idle@rcs0.html

  * igt@i915_pm_rpm@dpms-non-lpsp:
    - {shard-dg1}:        [SKIP][22] ([i915#1397]) -> [PASS][23] +1 similar issue
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13208/shard-dg1-19/igt@i915_pm_rpm@dpms-non-lpsp.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117641v3/shard-dg1-14/igt@i915_pm_rpm@dpms-non-lpsp.html

  * igt@i915_pm_rpm@modeset-non-lpsp-stress:
    - {shard-rkl}:        [SKIP][24] ([i915#1397]) -> [PASS][25]
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13208/shard-rkl-7/igt@i915_pm_rpm@modeset-non-lpsp-stress.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117641v3/shard-rkl-4/igt@i915_pm_rpm@modeset-non-lpsp-stress.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions:
    - shard-glk:          [FAIL][26] ([i915#2346]) -> [PASS][27]
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13208/shard-glk3/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117641v3/shard-glk9/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html

  * igt@kms_cursor_legacy@single-move@pipe-b:
    - {shard-dg1}:        [INCOMPLETE][28] ([i915#8011] / [i915#8347]) -> [PASS][29]
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13208/shard-dg1-19/igt@kms_cursor_legacy@single-move@pipe-b.html
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117641v3/shard-dg1-16/igt@kms_cursor_legacy@single-move@pipe-b.html

  * igt@kms_flip@2x-flip-vs-expired-vblank@ac-hdmi-a1-hdmi-a2:
    - shard-glk:          [FAIL][30] ([i915#79]) -> [PASS][31] +2 similar issues
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13208/shard-glk7/igt@kms_flip@2x-flip-vs-expired-vblank@ac-hdmi-a1-hdmi-a2.html
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117641v3/shard-glk6/igt@kms_flip@2x-flip-vs-expired-vblank@ac-hdmi-a1-hdmi-a2.html

  * igt@kms_frontbuffer_tracking@fbc-suspend:
    - {shard-dg1}:        [FAIL][32] ([fdo#103375]) -> [PASS][33]
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13208/shard-dg1-18/igt@kms_frontbuffer_tracking@fbc-suspend.html
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117641v3/shard-dg1-15/igt@kms_frontbuffer_tracking@fbc-suspend.html

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

  [fdo#103375]: https://bugs.freedesktop.org/show_bug.cgi?id=103375
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109302]: https://bugs.freedesktop.org/show_bug.cgi?id=109302
  [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
  [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#111825]: https://bugs.freedesktop.org/show_bug.cgi?id=111825
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [fdo#112283]: https://bugs.freedesktop.org/show_bug.cgi?id=112283
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#1397]: https://gitlab.freedesktop.org/drm/intel/issues/1397
  [i915#1825]: https://gitlab.freedesktop.org/drm/intel/issues/1825
  [i915#1839]: https://gitlab.freedesktop.org/drm/intel/issues/1839
  [i915#2346]: https://gitlab.freedesktop.org/drm/intel/issues/2346
  [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#2587]: https://gitlab.freedesktop.org/drm/intel/issues/2587
  [i915#2672]: https://gitlab.freedesktop.org/drm/intel/issues/2672
  [i915#2681]: https://gitlab.freedesktop.org/drm/intel/issues/2681
  [i915#2842]: https://gitlab.freedesktop.org/drm/intel/issues/2842
  [i915#2846]: https://gitlab.freedesktop.org/drm/intel/issues/2846
  [i915#3023]: https://gitlab.freedesktop.org/drm/intel/issues/3023
  [i915#3281]: https://gitlab.freedesktop.org/drm/intel/issues/3281
  [i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
  [i915#3359]: https://gitlab.freedesktop.org/drm/intel/issues/3359
  [i915#3458]: https://gitlab.freedesktop.org/drm/intel/issues/3458
  [i915#3469]: https://gitlab.freedesktop.org/drm/intel/issues/3469
  [i915#3539]: https://gitlab.freedesktop.org/drm/intel/issues/3539
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3591]: https://gitlab.freedesktop.org/drm/intel/issues/3591
  [i915#3638]: https://gitlab.freedesktop.org/drm/intel/issues/3638
  [i915#3689]: https://gitlab.freedesktop.org/drm/intel/issues/3689
  [i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
  [i915#3734]: https://gitlab.freedesktop.org/drm/intel/issues/3734
  [i915#3886]: https://gitlab.freedesktop.org/drm/intel/issues/3886
  [i915#3955]: https://gitlab.freedesktop.org/drm/intel/issues/3955
  [i915#3989]: https://gitlab.freedesktop.org/drm/intel/issues/3989
  [i915#4070]: https://gitlab.freedesktop.org/drm/intel/issues/4070
  [i915#4077]: https://gitlab.freedesktop.org/drm/intel/issues/4077
  [i915#4078]: https://gitlab.freedesktop.org/drm/intel/issues/4078
  [i915#4083]: https://gitlab.freedesktop.org/drm/intel/issues/4083
  [i915#4212]: https://gitlab.freedesktop.org/drm/intel/issues/4212
  [i915#4270]: https://gitlab.freedesktop.org/drm/intel/issues/4270
  [i915#4538]: https://gitlab.freedesktop.org/drm/intel/issues/4538
  [i915#454]: https://gitlab.freedesktop.org/drm/intel/issues/454
  [i915#4565]: https://gitlab.freedesktop.org/drm/intel/issues/4565
  [i915#4579]: https://gitlab.freedesktop.org/drm/intel/issues/4579
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4771]: https://gitlab.freedesktop.org/drm/intel/issues/4771
  [i915#4833]: https://gitlab.freedesktop.org/drm/intel/issues/4833
  [i915#4852]: https://gitlab.freedesktop.org/drm/intel/issues/4852
  [i915#4860]: https://gitlab.freedesktop.org/drm/intel/issues/4860
  [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#5289]: https://gitlab.freedesktop.org/drm/intel/issues/5289
  [i915#5325]: https://gitlab.freedesktop.org/drm/intel/issues/5325
  [i915#533]: https://gitlab.freedesktop.org/drm/intel/issues/533
  [i915#5354]: https://gitlab.freedesktop.org/drm/intel/issues/5354
  [i915#5493]: https://gitlab.freedesktop.org/drm/intel/issues/5493
  [i915#6095]: https://gitlab.freedesktop.org/drm/intel/issues/6095
  [i915#6268]: https://gitlab.freedesktop.org/drm/intel/issues/6268
  [i915#6334]: https://gitlab.freedesktop.org/drm/intel/issues/6334
  [i915#6433]: https://gitlab.freedesktop.org/drm/intel/issues/6433
  [i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658
  [i915#6768]: https://gitlab.freedesktop.org/drm/intel/issues/6768
  [i915#6806]: https://gitlab.freedesktop.org/drm/intel/issues/6806
  [i915#7178]: https://gitlab.freedesktop.org/drm/intel/issues/7178
  [i915#7697]: https://gitlab.freedesktop.org/drm/intel/issues/7697
  [i915#7701]: https://gitlab.freedesktop.org/drm/intel/issues/7701
  [i915#7711]: https://gitlab.freedesktop.org/drm/intel/issues/7711
  [i915#7742]: https://gitlab.freedesktop.org/drm/intel/issues/7742
  [i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
  [i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
  [i915#7975]: https://gitlab.freedesktop.org/drm/intel/issues/7975
  [i915#8011]: https://gitlab.freedesktop.org/drm/intel/issues/8011
  [i915#8213]: https://gitlab.freedesktop.org/drm/intel/issues/8213
  [i915#8247]: https://gitlab.freedesktop.org/drm/intel/issues/8247
  [i915#8292]: https://gitlab.freedesktop.org/drm/intel/issues/8292
  [i915#8347]: https://gitlab.freedesktop.org/drm/intel/issues/8347
  [i915#8381]: https://gitlab.freedesktop.org/drm/intel/issues/8381
  [i915#8398]: https://gitlab.freedesktop.org/drm/intel/issues/8398
  [i915#8414]: https://gitlab.freedesktop.org/drm/intel/issues/8414
  [i915#8555]: https://gitlab.freedesktop.org/drm/intel/issues/8555


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

  * Linux: CI_DRM_13208 -> Patchwork_117641v3

  CI-20190529: 20190529
  CI_DRM_13208: fa006f7737d7eebd38030e06310ba773d95a5960 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7314: ab70dfcdecf93a17fcaddb774855f726325fa0dd @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_117641v3: fa006f7737d7eebd38030e06310ba773d95a5960 @ 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_117641v3/index.html

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