[PATCH 00/10] Add xe3lpd edp enabling

[PATCH 00/10] Add xe3lpd edp enabling

[Matt Atwood]

This series defines the xe3lpd definition, which is functionally
identical to the xe2lpd definition for now. This series then adds
additional requirements mostly for edp output of display through.
Additional patches will be required for display and will follow. 

Clint Taylor (1):
  drm/i915/xe3lpd: reuse xe2lpd definition

Matt Roper (3):
  drm/i915/xe3lpd: Adjust watermark calculations
  drm/i915/xe3lpd: Add new display power wells
  drm/i915/xe3lpd: Update pmdemand programming

Radhakrishna Sripada (1):
  drm/i915/xe3lpd: Add cdclk changes

Suraj Kandpal (5):
  drm/i915/xe3lpd: Add macro to choose HDCP_LINE_REKEY bit
  drm/i915/xe3lpd: Add C20 Phy consolidated programming table
  drm/i915/xe3lpd: Add new bit range of MAX swing setup
  drm/i915/xe3lpd: Add check to see if edp over type c is allowed
  drm/i915/xe3lpd: Add powerdown value of eDP over type c

 drivers/gpu/drm/i915/display/intel_alpm.c     |   2 +-
 drivers/gpu/drm/i915/display/intel_cdclk.c    |  56 +++++++-
 drivers/gpu/drm/i915/display/intel_cx0_phy.c  |  29 +++-
 .../drm/i915/display/intel_display_device.c   |   6 +
 .../drm/i915/display/intel_display_device.h   |   2 +
 .../i915/display/intel_display_power_map.c    | 135 +++++++++++++++++-
 .../drm/i915/display/intel_display_types.h    |   1 +
 drivers/gpu/drm/i915/display/intel_dp.c       |  16 +++
 drivers/gpu/drm/i915/display/intel_hdcp.c     |   5 +-
 drivers/gpu/drm/i915/display/intel_pmdemand.c |  61 +++++---
 drivers/gpu/drm/i915/display/intel_pmdemand.h |   4 +-
 drivers/gpu/drm/i915/display/intel_psr_regs.h |   4 +-
 drivers/gpu/drm/i915/display/skl_watermark.c  |  18 ++-
 drivers/gpu/drm/i915/i915_reg.h               |   6 +-
 include/drm/intel/i915_pciids.h               |  12 ++
 15 files changed, 319 insertions(+), 38 deletions(-)

-- 
2.45.0

[PATCH 01/10] drm/i915/xe3lpd: reuse xe2lpd definition

[Matt Atwood]

From: Clint Taylor <clinton.a.taylor@intel.com>

xe3_lpd display is functionally identical to xe2_lpd for now so reuse
the device description.  A separate xe3 definition will be added in the
future if/when new feature flags are required.

Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display_device.c |  6 ++++++
 drivers/gpu/drm/i915/display/intel_display_device.h |  2 ++
 include/drm/intel/i915_pciids.h                     | 12 ++++++++++++
 3 files changed, 20 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_display_device.c b/drivers/gpu/drm/i915/display/intel_display_device.c
index f33062322c66..aa22189e3853 100644
--- a/drivers/gpu/drm/i915/display/intel_display_device.c
+++ b/drivers/gpu/drm/i915/display/intel_display_device.c
@@ -1252,6 +1252,10 @@ static const struct platform_desc bmg_desc = {
 	PLATFORM(BATTLEMAGE),
 };
 
+static const struct platform_desc ptl_desc = {
+	PLATFORM(PANTHERLAKE),
+};
+
 __diag_pop();
 
 /*
@@ -1322,6 +1326,7 @@ static const struct {
 	INTEL_MTL_IDS(INTEL_DISPLAY_DEVICE, &mtl_desc),
 	INTEL_LNL_IDS(INTEL_DISPLAY_DEVICE, &lnl_desc),
 	INTEL_BMG_IDS(INTEL_DISPLAY_DEVICE, &bmg_desc),
+	INTEL_PTL_IDS(INTEL_DISPLAY_DEVICE, &ptl_desc),
 };
 
 static const struct {
@@ -1332,6 +1337,7 @@ static const struct {
 	{ 14,  0, &xe_lpdp_display },
 	{ 14,  1, &xe2_hpd_display },
 	{ 20,  0, &xe2_lpd_display },
+	{ 30,  0, &xe2_lpd_display },
 };
 
 static const struct intel_display_device_info *
diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h b/drivers/gpu/drm/i915/display/intel_display_device.h
index 3ef537fa551a..071a36b51f79 100644
--- a/drivers/gpu/drm/i915/display/intel_display_device.h
+++ b/drivers/gpu/drm/i915/display/intel_display_device.h
@@ -70,6 +70,8 @@ enum intel_display_platform {
 	INTEL_DISPLAY_LUNARLAKE,
 	/* Display ver 14.1 (based on GMD ID) */
 	INTEL_DISPLAY_BATTLEMAGE,
+	/* Display ver 30 (based on GMD ID) */
+	INTEL_DISPLAY_PANTHERLAKE,
 };
 
 enum intel_display_subplatform {
diff --git a/include/drm/intel/i915_pciids.h b/include/drm/intel/i915_pciids.h
index 02156c6f79b6..6b92f8c3731b 100644
--- a/include/drm/intel/i915_pciids.h
+++ b/include/drm/intel/i915_pciids.h
@@ -794,4 +794,16 @@
 	MACRO__(0xE20D, ## __VA_ARGS__), \
 	MACRO__(0xE212, ## __VA_ARGS__)
 
+/* PTL */
+#define INTEL_PTL_IDS(MACRO__, ...) \
+	MACRO__(0xB080, ## __VA_ARGS__), \
+	MACRO__(0xB081, ## __VA_ARGS__), \
+	MACRO__(0xB082, ## __VA_ARGS__), \
+	MACRO__(0xB090, ## __VA_ARGS__), \
+	MACRO__(0xB091, ## __VA_ARGS__), \
+	MACRO__(0xB092, ## __VA_ARGS__), \
+	MACRO__(0xB0A0, ## __VA_ARGS__), \
+	MACRO__(0xB0A1, ## __VA_ARGS__), \
+	MACRO__(0xB0A2, ## __VA_ARGS__)
+
 #endif /* _I915_PCIIDS_H */
-- 
2.45.0

[PATCH 02/10] drm/i915/xe3lpd: Adjust watermark calculations

[Matt Atwood]

From: Matt Roper <matthew.d.roper@intel.com>

Xe3 makes a couple minor tweaks to the watermark algorithm's block count
calculations.

Bspec: 68985
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
---
 drivers/gpu/drm/i915/display/skl_watermark.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c
index 6e1f04d5ef47..31de33e868df 100644
--- a/drivers/gpu/drm/i915/display/skl_watermark.c
+++ b/drivers/gpu/drm/i915/display/skl_watermark.c
@@ -718,7 +718,7 @@ static int skl_compute_wm_params(const struct intel_crtc_state *crtc_state,
 				 int width, const struct drm_format_info *format,
 				 u64 modifier, unsigned int rotation,
 				 u32 plane_pixel_rate, struct skl_wm_params *wp,
-				 int color_plane);
+				 int color_plane, unsigned int pan_x);
 
 static void skl_compute_plane_wm(const struct intel_crtc_state *crtc_state,
 				 struct intel_plane *plane,
@@ -765,7 +765,7 @@ skl_cursor_allocation(const struct intel_crtc_state *crtc_state,
 				    drm_format_info(DRM_FORMAT_ARGB8888),
 				    DRM_FORMAT_MOD_LINEAR,
 				    DRM_MODE_ROTATE_0,
-				    crtc_state->pixel_rate, &wp, 0);
+				    crtc_state->pixel_rate, &wp, 0, 0);
 	drm_WARN_ON(&i915->drm, ret);
 
 	for (level = 0; level < i915->display.wm.num_levels; level++) {
@@ -1742,7 +1742,7 @@ skl_compute_wm_params(const struct intel_crtc_state *crtc_state,
 		      int width, const struct drm_format_info *format,
 		      u64 modifier, unsigned int rotation,
 		      u32 plane_pixel_rate, struct skl_wm_params *wp,
-		      int color_plane)
+		      int color_plane, unsigned int pan_x)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
@@ -1803,7 +1803,9 @@ skl_compute_wm_params(const struct intel_crtc_state *crtc_state,
 					   wp->y_min_scanlines,
 					   wp->dbuf_block_size);
 
-		if (DISPLAY_VER(i915) >= 10)
+		if (DISPLAY_VER(i915) >= 30)
+			interm_pbpl += (pan_x != 0);
+		else if (DISPLAY_VER(i915) >= 10)
 			interm_pbpl++;
 
 		wp->plane_blocks_per_line = div_fixed16(interm_pbpl,
@@ -1845,7 +1847,8 @@ skl_compute_plane_wm_params(const struct intel_crtc_state *crtc_state,
 				     fb->format, fb->modifier,
 				     plane_state->hw.rotation,
 				     intel_plane_pixel_rate(crtc_state, plane_state),
-				     wp, color_plane);
+				     wp, color_plane,
+				     plane_state->uapi.src.x1);
 }
 
 static bool skl_wm_has_lines(struct drm_i915_private *i915, int level)
@@ -1909,7 +1912,10 @@ static void skl_compute_plane_wm(const struct intel_crtc_state *crtc_state,
 		}
 	}
 
-	blocks = fixed16_to_u32_round_up(selected_result) + 1;
+	blocks = fixed16_to_u32_round_up(selected_result);
+	if (DISPLAY_VER(i915) < 30)
+		blocks++;
+
 	/*
 	 * Lets have blocks at minimum equivalent to plane_blocks_per_line
 	 * as there will be at minimum one line for lines configuration. This
-- 
2.45.0

[PATCH 03/10] drm/i915/xe3lpd: Add new display power wells

[Matt Atwood]

From: Matt Roper <matthew.d.roper@intel.com>

Xe3's power well handling is similar to previous platforms, but there
are a few changes that need to be handled to ensure optimal power
management:
 - PGB now only depends on PG1, not PG2
 - Transcoder B is now in PG1 (was previously in PGB)
 - Transcoders C & D are now in PG2 (were previously in PGC/PGD)
 - DC states now require PG2 to be off (whereas on Xe2 it could remain
   on as a dependency of PGB, although the features inside of it could
   not be used).

Bspec: 72519, 68851
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
---
 .../i915/display/intel_display_power_map.c    | 135 +++++++++++++++++-
 1 file changed, 134 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_power_map.c b/drivers/gpu/drm/i915/display/intel_display_power_map.c
index 10948b3964ee..255b2c09607c 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power_map.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power_map.c
@@ -1586,6 +1586,137 @@ static const struct i915_power_well_desc_list xe2lpd_power_wells[] = {
 	I915_PW_DESCRIPTORS(xe2lpd_power_wells_pica),
 };
 
+/*
+ * Xe3 changes the power well hierarchy slightly from Xe_LPD+; PGB now
+ * depends on PG1 instead of PG2:
+ *
+ *       PG0
+ *        |
+ *     --PG1--
+ *    /   |   \
+ *  PGA  PGB  PG2
+ *           /   \
+ *         PGC   PGD
+ */
+
+#define XE3LPD_PW_C_POWER_DOMAINS \
+	POWER_DOMAIN_PIPE_C, \
+	POWER_DOMAIN_PIPE_PANEL_FITTER_C
+
+#define XE3LPD_PW_D_POWER_DOMAINS \
+	POWER_DOMAIN_PIPE_D, \
+	POWER_DOMAIN_PIPE_PANEL_FITTER_D
+
+#define XE3LPD_PW_2_POWER_DOMAINS \
+	XE3LPD_PW_C_POWER_DOMAINS, \
+	XE3LPD_PW_D_POWER_DOMAINS, \
+	POWER_DOMAIN_TRANSCODER_C, \
+	POWER_DOMAIN_TRANSCODER_D, \
+	POWER_DOMAIN_VGA, \
+	POWER_DOMAIN_PORT_DDI_LANES_TC1, \
+	POWER_DOMAIN_PORT_DDI_LANES_TC2, \
+	POWER_DOMAIN_PORT_DDI_LANES_TC3, \
+	POWER_DOMAIN_PORT_DDI_LANES_TC4
+
+I915_DECL_PW_DOMAINS(xe3lpd_pwdoms_pw_2,
+		     XE3LPD_PW_2_POWER_DOMAINS,
+		     POWER_DOMAIN_INIT);
+
+I915_DECL_PW_DOMAINS(xe3lpd_pwdoms_pw_b,
+		     POWER_DOMAIN_PIPE_B,
+		     POWER_DOMAIN_PIPE_PANEL_FITTER_B,
+		     POWER_DOMAIN_INIT);
+
+I915_DECL_PW_DOMAINS(xe3lpd_pwdoms_pw_c,
+		     XE3LPD_PW_C_POWER_DOMAINS,
+		     POWER_DOMAIN_INIT);
+
+I915_DECL_PW_DOMAINS(xe3lpd_pwdoms_pw_d,
+		     XE3LPD_PW_D_POWER_DOMAINS,
+		     POWER_DOMAIN_INIT);
+
+static const struct i915_power_well_desc xe3lpd_power_wells_main[] = {
+	{
+		.instances = &I915_PW_INSTANCES(
+			I915_PW("PW_2", &xe3lpd_pwdoms_pw_2,
+				.hsw.idx = ICL_PW_CTL_IDX_PW_2,
+				.id = SKL_DISP_PW_2),
+		),
+		.ops = &hsw_power_well_ops,
+		.has_vga = true,
+		.has_fuses = true,
+	}, {
+		.instances = &I915_PW_INSTANCES(
+			I915_PW("PW_A", &xelpd_pwdoms_pw_a,
+				.hsw.idx = XELPD_PW_CTL_IDX_PW_A),
+		),
+		.ops = &hsw_power_well_ops,
+		.irq_pipe_mask = BIT(PIPE_A),
+		.has_fuses = true,
+	}, {
+		.instances = &I915_PW_INSTANCES(
+			I915_PW("PW_B", &xe3lpd_pwdoms_pw_b,
+				.hsw.idx = XELPD_PW_CTL_IDX_PW_B),
+		),
+		.ops = &hsw_power_well_ops,
+		.irq_pipe_mask = BIT(PIPE_B),
+		.has_fuses = true,
+	}, {
+		.instances = &I915_PW_INSTANCES(
+			I915_PW("PW_C", &xe3lpd_pwdoms_pw_c,
+				.hsw.idx = XELPD_PW_CTL_IDX_PW_C),
+		),
+		.ops = &hsw_power_well_ops,
+		.irq_pipe_mask = BIT(PIPE_C),
+		.has_fuses = true,
+	}, {
+		.instances = &I915_PW_INSTANCES(
+			I915_PW("PW_D", &xe3lpd_pwdoms_pw_d,
+				.hsw.idx = XELPD_PW_CTL_IDX_PW_D),
+		),
+		.ops = &hsw_power_well_ops,
+		.irq_pipe_mask = BIT(PIPE_D),
+		.has_fuses = true,
+	}, {
+		.instances = &I915_PW_INSTANCES(
+			I915_PW("AUX_A", &icl_pwdoms_aux_a, .xelpdp.aux_ch = AUX_CH_A),
+			I915_PW("AUX_B", &icl_pwdoms_aux_b, .xelpdp.aux_ch = AUX_CH_B),
+			I915_PW("AUX_TC1", &xelpdp_pwdoms_aux_tc1, .xelpdp.aux_ch = AUX_CH_USBC1),
+			I915_PW("AUX_TC2", &xelpdp_pwdoms_aux_tc2, .xelpdp.aux_ch = AUX_CH_USBC2),
+			I915_PW("AUX_TC3", &xelpdp_pwdoms_aux_tc3, .xelpdp.aux_ch = AUX_CH_USBC3),
+			I915_PW("AUX_TC4", &xelpdp_pwdoms_aux_tc4, .xelpdp.aux_ch = AUX_CH_USBC4),
+		),
+		.ops = &xelpdp_aux_power_well_ops,
+	},
+};
+
+I915_DECL_PW_DOMAINS(xe3lpd_pwdoms_dc_off,
+	POWER_DOMAIN_DC_OFF,
+	XE3LPD_PW_2_POWER_DOMAINS,
+	XE3LPD_PW_C_POWER_DOMAINS,
+	XE3LPD_PW_D_POWER_DOMAINS,
+	POWER_DOMAIN_AUDIO_MMIO,
+	POWER_DOMAIN_INIT);
+
+static const struct i915_power_well_desc xe3lpd_power_wells_dcoff[] = {
+	{
+		.instances = &I915_PW_INSTANCES(
+			I915_PW("DC_off", &xe3lpd_pwdoms_dc_off,
+				.id = SKL_DISP_DC_OFF),
+		),
+		.ops = &gen9_dc_off_power_well_ops,
+	},
+};
+
+
+static const struct i915_power_well_desc_list xe3lpd_power_wells[] = {
+	I915_PW_DESCRIPTORS(i9xx_power_wells_always_on),
+	I915_PW_DESCRIPTORS(icl_power_wells_pw_1),
+	I915_PW_DESCRIPTORS(xe3lpd_power_wells_dcoff),
+	I915_PW_DESCRIPTORS(xe3lpd_power_wells_main),
+	I915_PW_DESCRIPTORS(xe2lpd_power_wells_pica),
+};
+
 static void init_power_well_domains(const struct i915_power_well_instance *inst,
 				    struct i915_power_well *power_well)
 {
@@ -1693,7 +1824,9 @@ int intel_display_power_map_init(struct i915_power_domains *power_domains)
 		return 0;
 	}
 
-	if (DISPLAY_VER(i915) >= 20)
+	if (DISPLAY_VER(i915) >= 30)
+		return set_power_wells(power_domains, xe3lpd_power_wells);
+	else if (DISPLAY_VER(i915) >= 20)
 		return set_power_wells(power_domains, xe2lpd_power_wells);
 	else if (DISPLAY_VER(i915) >= 14)
 		return set_power_wells(power_domains, xelpdp_power_wells);
-- 
2.45.0

[PATCH 04/10] drm/i915/xe3lpd: Update pmdemand programming

[Matt Atwood]

From: Matt Roper <matthew.d.roper@intel.com>

There are some minor changes to pmdemand handling on Xe3:
 - Active scalers are no longer tracked.  We can simply skip the readout
   and programming of this field.
 - Active dbuf slices are no longer tracked.  We should skip the readout
   and programming of this field and also make sure that it stays 0 in
   our software bookkeeping so that we won't erroneously return true
   from intel_pmdemand_needs_update() due to mismatches.
 - Even though there aren't enough pipes to utilize them, the size of
   the 'active pipes' field has expanded to four bits, taking over the
   register bits previously used for dbuf slices.  Since the lower bits
   of the mask have moved, we need to update our reads/writes to handle
   this properly.

Bspec: 68883, 69125
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
---
 drivers/gpu/drm/i915/display/intel_pmdemand.c | 61 +++++++++++++------
 drivers/gpu/drm/i915/display/intel_pmdemand.h |  4 +-
 drivers/gpu/drm/i915/i915_reg.h               |  1 +
 3 files changed, 45 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_pmdemand.c b/drivers/gpu/drm/i915/display/intel_pmdemand.c
index ceaf9e3147da..9af2f83d3a75 100644
--- a/drivers/gpu/drm/i915/display/intel_pmdemand.c
+++ b/drivers/gpu/drm/i915/display/intel_pmdemand.c
@@ -258,6 +258,7 @@ intel_pmdemand_connector_needs_update(struct intel_atomic_state *state)
 
 static bool intel_pmdemand_needs_update(struct intel_atomic_state *state)
 {
+	struct drm_i915_private *i915 = to_i915(state->base.dev);
 	const struct intel_bw_state *new_bw_state, *old_bw_state;
 	const struct intel_cdclk_state *new_cdclk_state, *old_cdclk_state;
 	const struct intel_crtc_state *new_crtc_state, *old_crtc_state;
@@ -274,12 +275,16 @@ static bool intel_pmdemand_needs_update(struct intel_atomic_state *state)
 	new_dbuf_state = intel_atomic_get_new_dbuf_state(state);
 	old_dbuf_state = intel_atomic_get_old_dbuf_state(state);
 	if (new_dbuf_state &&
-	    (new_dbuf_state->active_pipes !=
-	     old_dbuf_state->active_pipes ||
-	     new_dbuf_state->enabled_slices !=
-	     old_dbuf_state->enabled_slices))
+	    new_dbuf_state->active_pipes != old_dbuf_state->active_pipes)
 		return true;
 
+	if (DISPLAY_VER(i915) < 30) {
+		if (new_dbuf_state &&
+		    new_dbuf_state->enabled_slices !=
+		    old_dbuf_state->enabled_slices)
+			return true;
+	}
+
 	new_cdclk_state = intel_atomic_get_new_cdclk_state(state);
 	old_cdclk_state = intel_atomic_get_old_cdclk_state(state);
 	if (new_cdclk_state &&
@@ -329,8 +334,10 @@ int intel_pmdemand_atomic_check(struct intel_atomic_state *state)
 
 	new_pmdemand_state->params.active_pipes =
 		min_t(u8, hweight8(new_dbuf_state->active_pipes), 3);
-	new_pmdemand_state->params.active_dbufs =
-		min_t(u8, hweight8(new_dbuf_state->enabled_slices), 3);
+
+	if (DISPLAY_VER(i915) < 30)
+		new_pmdemand_state->params.active_dbufs =
+			min_t(u8, hweight8(new_dbuf_state->enabled_slices), 3);
 
 	new_cdclk_state = intel_atomic_get_cdclk_state(state);
 	if (IS_ERR(new_cdclk_state))
@@ -395,27 +402,32 @@ intel_pmdemand_init_pmdemand_params(struct drm_i915_private *i915,
 
 	reg2 = intel_de_read(i915, XELPDP_INITIATE_PMDEMAND_REQUEST(1));
 
-	/* Set 1*/
 	pmdemand_state->params.qclk_gv_bw =
 		REG_FIELD_GET(XELPDP_PMDEMAND_QCLK_GV_BW_MASK, reg1);
 	pmdemand_state->params.voltage_index =
 		REG_FIELD_GET(XELPDP_PMDEMAND_VOLTAGE_INDEX_MASK, reg1);
 	pmdemand_state->params.qclk_gv_index =
 		REG_FIELD_GET(XELPDP_PMDEMAND_QCLK_GV_INDEX_MASK, reg1);
-	pmdemand_state->params.active_pipes =
-		REG_FIELD_GET(XELPDP_PMDEMAND_PIPES_MASK, reg1);
-	pmdemand_state->params.active_dbufs =
-		REG_FIELD_GET(XELPDP_PMDEMAND_DBUFS_MASK, reg1);
 	pmdemand_state->params.active_phys =
 		REG_FIELD_GET(XELPDP_PMDEMAND_PHYS_MASK, reg1);
 
-	/* Set 2*/
 	pmdemand_state->params.cdclk_freq_mhz =
 		REG_FIELD_GET(XELPDP_PMDEMAND_CDCLK_FREQ_MASK, reg2);
 	pmdemand_state->params.ddiclk_max =
 		REG_FIELD_GET(XELPDP_PMDEMAND_DDICLK_FREQ_MASK, reg2);
-	pmdemand_state->params.scalers =
-		REG_FIELD_GET(XELPDP_PMDEMAND_SCALERS_MASK, reg2);
+
+	if (DISPLAY_VER(i915) >= 30) {
+		pmdemand_state->params.active_pipes =
+			REG_FIELD_GET(XE3_PMDEMAND_PIPES_MASK, reg1);
+	} else {
+		pmdemand_state->params.active_pipes =
+			REG_FIELD_GET(XELPDP_PMDEMAND_PIPES_MASK, reg1);
+		pmdemand_state->params.active_dbufs =
+			REG_FIELD_GET(XELPDP_PMDEMAND_DBUFS_MASK, reg1);
+
+		pmdemand_state->params.scalers =
+			REG_FIELD_GET(XELPDP_PMDEMAND_SCALERS_MASK, reg2);
+	}
 
 unlock:
 	mutex_unlock(&i915->display.pmdemand.lock);
@@ -442,6 +454,10 @@ void intel_pmdemand_program_dbuf(struct drm_i915_private *i915,
 {
 	u32 dbufs = min_t(u32, hweight8(dbuf_slices), 3);
 
+	/* PM Demand only tracks active dbufs on pre-Xe3 platforms */
+	if (DISPLAY_VER(i915) >= 30)
+		return;
+
 	mutex_lock(&i915->display.pmdemand.lock);
 	if (drm_WARN_ON(&i915->drm,
 			!intel_pmdemand_check_prev_transaction(i915)))
@@ -460,7 +476,8 @@ void intel_pmdemand_program_dbuf(struct drm_i915_private *i915,
 }
 
 static void
-intel_pmdemand_update_params(const struct intel_pmdemand_state *new,
+intel_pmdemand_update_params(struct drm_i915_private *i915,
+			     const struct intel_pmdemand_state *new,
 			     const struct intel_pmdemand_state *old,
 			     u32 *reg1, u32 *reg2, bool serialized)
 {
@@ -495,16 +512,22 @@ intel_pmdemand_update_params(const struct intel_pmdemand_state *new,
 	update_reg(reg1, qclk_gv_bw, XELPDP_PMDEMAND_QCLK_GV_BW_MASK);
 	update_reg(reg1, voltage_index, XELPDP_PMDEMAND_VOLTAGE_INDEX_MASK);
 	update_reg(reg1, qclk_gv_index, XELPDP_PMDEMAND_QCLK_GV_INDEX_MASK);
-	update_reg(reg1, active_pipes, XELPDP_PMDEMAND_PIPES_MASK);
-	update_reg(reg1, active_dbufs, XELPDP_PMDEMAND_DBUFS_MASK);
 	update_reg(reg1, active_phys, XELPDP_PMDEMAND_PHYS_MASK);
 
 	/* Set 2*/
 	update_reg(reg2, cdclk_freq_mhz, XELPDP_PMDEMAND_CDCLK_FREQ_MASK);
 	update_reg(reg2, ddiclk_max, XELPDP_PMDEMAND_DDICLK_FREQ_MASK);
-	update_reg(reg2, scalers, XELPDP_PMDEMAND_SCALERS_MASK);
 	update_reg(reg2, plls, XELPDP_PMDEMAND_PLLS_MASK);
 
+	if (DISPLAY_VER(i915) >= 30) {
+		update_reg(reg1, active_pipes, XE3_PMDEMAND_PIPES_MASK);
+	} else {
+		update_reg(reg1, active_pipes, XELPDP_PMDEMAND_PIPES_MASK);
+		update_reg(reg1, active_dbufs, XELPDP_PMDEMAND_DBUFS_MASK);
+
+		update_reg(reg2, scalers, XELPDP_PMDEMAND_SCALERS_MASK);
+	}
+
 #undef update_reg
 }
 
@@ -529,7 +552,7 @@ intel_pmdemand_program_params(struct drm_i915_private *i915,
 	reg2 = intel_de_read(i915, XELPDP_INITIATE_PMDEMAND_REQUEST(1));
 	mod_reg2 = reg2;
 
-	intel_pmdemand_update_params(new, old, &mod_reg1, &mod_reg2,
+	intel_pmdemand_update_params(i915, new, old, &mod_reg1, &mod_reg2,
 				     serialized);
 
 	if (reg1 != mod_reg1) {
diff --git a/drivers/gpu/drm/i915/display/intel_pmdemand.h b/drivers/gpu/drm/i915/display/intel_pmdemand.h
index 128fd61f8f14..a1c49efdc493 100644
--- a/drivers/gpu/drm/i915/display/intel_pmdemand.h
+++ b/drivers/gpu/drm/i915/display/intel_pmdemand.h
@@ -20,14 +20,14 @@ struct pmdemand_params {
 	u8 voltage_index;
 	u8 qclk_gv_index;
 	u8 active_pipes;
-	u8 active_dbufs;
+	u8 active_dbufs;	/* pre-Xe3 only */
 	/* Total number of non type C active phys from active_phys_mask */
 	u8 active_phys;
 	u8 plls;
 	u16 cdclk_freq_mhz;
 	/* max from ddi_clocks[] */
 	u16 ddiclk_max;
-	u8 scalers;
+	u8 scalers;		/* pre-Xe3 only */
 };
 
 struct intel_pmdemand_state {
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 818142f5a10c..d30459f8d1cb 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2705,6 +2705,7 @@
 #define  XELPDP_PMDEMAND_QCLK_GV_BW_MASK		REG_GENMASK(31, 16)
 #define  XELPDP_PMDEMAND_VOLTAGE_INDEX_MASK		REG_GENMASK(14, 12)
 #define  XELPDP_PMDEMAND_QCLK_GV_INDEX_MASK		REG_GENMASK(11, 8)
+#define  XE3_PMDEMAND_PIPES_MASK			REG_GENMASK(7, 4)
 #define  XELPDP_PMDEMAND_PIPES_MASK			REG_GENMASK(7, 6)
 #define  XELPDP_PMDEMAND_DBUFS_MASK			REG_GENMASK(5, 4)
 #define  XELPDP_PMDEMAND_PHYS_MASK			REG_GENMASK(2, 0)
-- 
2.45.0

[PATCH 05/10] drm/i915/xe3lpd: Add cdclk changes

[Matt Atwood]

From: Radhakrishna Sripada <radhakrishna.sripada@intel.com>

Xe3_LPD has new max cdclk of 691200 which requires reusing the lnl table
and modify/add higher frequencies. Updating the max cdclk supported by
the platform and voltage_level determination is also updated.

There are minor changes in cdclk programming sequence compared to lnl,
where programming cd2x divider needs to be skipped. This is already handled
by the calculations in existing code.

Bspec: 68861, 68863, 68864
Cc: Gustavo Sousa <gustavo.sousa@intel.com>
Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
---
 drivers/gpu/drm/i915/display/intel_cdclk.c | 56 +++++++++++++++++++++-
 1 file changed, 54 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index fa1c2012b10c..6ac7bd6afc36 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -1468,6 +1468,32 @@ static const struct intel_cdclk_vals xe2hpd_cdclk_table[] = {
 	{}
 };
 
+static const struct intel_cdclk_vals xe3lpd_cdclk_table[] = {
+	{ .refclk = 38400, .cdclk = 153600, .ratio = 16, .waveform = 0xaaaa },
+	{ .refclk = 38400, .cdclk = 172800, .ratio = 16, .waveform = 0xad5a },
+	{ .refclk = 38400, .cdclk = 192000, .ratio = 16, .waveform = 0xb6b6 },
+	{ .refclk = 38400, .cdclk = 211200, .ratio = 16, .waveform = 0xdbb6 },
+	{ .refclk = 38400, .cdclk = 230400, .ratio = 16, .waveform = 0xeeee },
+	{ .refclk = 38400, .cdclk = 249600, .ratio = 16, .waveform = 0xf7de },
+	{ .refclk = 38400, .cdclk = 268800, .ratio = 16, .waveform = 0xfefe },
+	{ .refclk = 38400, .cdclk = 288000, .ratio = 16, .waveform = 0xfffe },
+	{ .refclk = 38400, .cdclk = 307200, .ratio = 16, .waveform = 0xffff },
+	{ .refclk = 38400, .cdclk = 330000, .ratio = 25, .waveform = 0xdbb6 },
+	{ .refclk = 38400, .cdclk = 360000, .ratio = 25, .waveform = 0xeeee },
+	{ .refclk = 38400, .cdclk = 390000, .ratio = 25, .waveform = 0xf7de },
+	{ .refclk = 38400, .cdclk = 420000, .ratio = 25, .waveform = 0xfefe },
+	{ .refclk = 38400, .cdclk = 450000, .ratio = 25, .waveform = 0xfffe },
+	{ .refclk = 38400, .cdclk = 480000, .ratio = 25, .waveform = 0xffff },
+	{ .refclk = 38400, .cdclk = 487200, .ratio = 29, .waveform = 0xfefe },
+	{ .refclk = 38400, .cdclk = 522000, .ratio = 29, .waveform = 0xfffe },
+	{ .refclk = 38400, .cdclk = 556800, .ratio = 29, .waveform = 0xffff },
+	{ .refclk = 38400, .cdclk = 561600, .ratio = 36, .waveform = 0xf7de },
+	{ .refclk = 38400, .cdclk = 604800, .ratio = 36, .waveform = 0xfefe },
+	{ .refclk = 38400, .cdclk = 648000, .ratio = 36, .waveform = 0xfffe },
+	{ .refclk = 38400, .cdclk = 691200, .ratio = 36, .waveform = 0xffff },
+	{}
+};
+
 static const int cdclk_squash_len = 16;
 
 static int cdclk_squash_divider(u16 waveform)
@@ -1594,6 +1620,20 @@ static u8 rplu_calc_voltage_level(int cdclk)
 				  rplu_voltage_level_max_cdclk);
 }
 
+static u8 xe3lpd_calc_voltage_level(int cdclk)
+{
+	static const int xe3lpd_voltage_level_max_cdclk[] = {
+		[0] = 307200,
+		[1] = 480000,
+		[2] = 556800,
+		[3] = 691200,
+	};
+
+	return calc_voltage_level(cdclk,
+				  ARRAY_SIZE(xe3lpd_voltage_level_max_cdclk),
+				  xe3lpd_voltage_level_max_cdclk);
+}
+
 static void icl_readout_refclk(struct intel_display *display,
 			       struct intel_cdclk_config *cdclk_config)
 {
@@ -3437,7 +3477,9 @@ void intel_update_max_cdclk(struct intel_display *display)
 {
 	struct drm_i915_private *dev_priv = to_i915(display->drm);
 
-	if (IS_JASPERLAKE(dev_priv) || IS_ELKHARTLAKE(dev_priv)) {
+	if (DISPLAY_VER(display) >= 30) {
+		display->cdclk.max_cdclk_freq = 691200;
+	} else if (IS_JASPERLAKE(dev_priv) || IS_ELKHARTLAKE(dev_priv)) {
 		if (display->cdclk.hw.ref == 24000)
 			display->cdclk.max_cdclk_freq = 552000;
 		else
@@ -3650,6 +3692,13 @@ void intel_cdclk_debugfs_register(struct intel_display *display)
 			    display, &i915_cdclk_info_fops);
 }
 
+static const struct intel_cdclk_funcs xe3lpd_cdclk_funcs = {
+	.get_cdclk = bxt_get_cdclk,
+	.set_cdclk = bxt_set_cdclk,
+	.modeset_calc_cdclk = bxt_modeset_calc_cdclk,
+	.calc_voltage_level = xe3lpd_calc_voltage_level,
+};
+
 static const struct intel_cdclk_funcs rplu_cdclk_funcs = {
 	.get_cdclk = bxt_get_cdclk,
 	.set_cdclk = bxt_set_cdclk,
@@ -3794,7 +3843,10 @@ void intel_init_cdclk_hooks(struct intel_display *display)
 {
 	struct drm_i915_private *dev_priv = to_i915(display->drm);
 
-	if (DISPLAY_VER(display) >= 20) {
+	if (DISPLAY_VER(display) >= 30) {
+		display->funcs.cdclk = &xe3lpd_cdclk_funcs;
+		display->cdclk.table = xe3lpd_cdclk_table;
+	} else if (DISPLAY_VER(display) >= 20) {
 		display->funcs.cdclk = &rplu_cdclk_funcs;
 		display->cdclk.table = xe2lpd_cdclk_table;
 	} else if (DISPLAY_VER_FULL(display) >= IP_VER(14, 1)) {
-- 
2.45.0

[PATCH 06/10] drm/i915/xe3lpd: Add macro to choose HDCP_LINE_REKEY bit

[Matt Atwood]

From: Suraj Kandpal <suraj.kandpal@intel.com>

DISPLAY_VER() >= 30 has the HDCP_LINE_REKEY bit redefined from
bit 12 to bit 14. Create a macro to choose the correct bit based
on DISPLAY_VER().

Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
---
 drivers/gpu/drm/i915/display/intel_hdcp.c | 5 +++--
 drivers/gpu/drm/i915/i915_reg.h           | 2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
index ed6aa87403e2..e9b0414590ce 100644
--- a/drivers/gpu/drm/i915/display/intel_hdcp.c
+++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
@@ -47,10 +47,11 @@ intel_hdcp_disable_hdcp_line_rekeying(struct intel_encoder *encoder,
 			intel_de_rmw(display, MTL_CHICKEN_TRANS(hdcp->cpu_transcoder),
 				     0, HDCP_LINE_REKEY_DISABLE);
 		else if (IS_DISPLAY_VER_STEP(display, IP_VER(14, 1), STEP_B0, STEP_FOREVER) ||
-			 IS_DISPLAY_VER_STEP(display, IP_VER(20, 0), STEP_B0, STEP_FOREVER))
+			 IS_DISPLAY_VER_STEP(display, IP_VER(20, 0), STEP_B0, STEP_FOREVER) ||
+			 DISPLAY_VER(display) >= 30)
 			intel_de_rmw(display,
 				     TRANS_DDI_FUNC_CTL(display, hdcp->cpu_transcoder),
-				     0, TRANS_DDI_HDCP_LINE_REKEY_DISABLE);
+				     0, TRANS_DDI_HDCP_LINE_REKEY_DISABLE(display));
 	}
 }
 
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index d30459f8d1cb..da65500cd0c8 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3832,7 +3832,7 @@ enum skl_power_gate {
 #define  TRANS_DDI_EDP_INPUT_B_ONOFF	(5 << 12)
 #define  TRANS_DDI_EDP_INPUT_C_ONOFF	(6 << 12)
 #define  TRANS_DDI_EDP_INPUT_D_ONOFF	(7 << 12)
-#define  TRANS_DDI_HDCP_LINE_REKEY_DISABLE	REG_BIT(12)
+#define  TRANS_DDI_HDCP_LINE_REKEY_DISABLE(display)	(DISPLAY_VER(display) >= 30 ? REG_BIT(15) : REG_BIT(12))
 #define  TRANS_DDI_MST_TRANSPORT_SELECT_MASK	REG_GENMASK(11, 10)
 #define  TRANS_DDI_MST_TRANSPORT_SELECT(trans)	\
 	REG_FIELD_PREP(TRANS_DDI_MST_TRANSPORT_SELECT_MASK, trans)
-- 
2.45.0

[PATCH 07/10] drm/i915/xe3lpd: Add C20 Phy consolidated programming table

[Matt Atwood]

From: Suraj Kandpal <suraj.kandpal@intel.com>

From DISPLAY_VER() >= 30 C20 PHY consolidated programming table of
DP and eDP been merged and now use the same rates and values. eDP
over TypeC has also been introduced.
Moreover it allows more granular and higher rates. Add new table to
represent this change.

Bspec: 68961
Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
---
 drivers/gpu/drm/i915/display/intel_cx0_phy.c | 24 ++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
index 4a6c3040ca15..0d6f75ae35f5 100644
--- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
+++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
@@ -1122,6 +1122,22 @@ static const struct intel_c20pll_state * const xe2hpd_c20_dp_tables[] = {
 	NULL,
 };
 
+static const struct intel_c20pll_state * const xe3lpd_c20_dp_edp_tables[] = {
+	&mtl_c20_dp_rbr,
+	&xe2hpd_c20_edp_r216,
+	&xe2hpd_c20_edp_r243,
+	&mtl_c20_dp_hbr1,
+	&xe2hpd_c20_edp_r324,
+	&xe2hpd_c20_edp_r432,
+	&mtl_c20_dp_hbr2,
+	&xe2hpd_c20_edp_r675,
+	&mtl_c20_dp_hbr3,
+	&mtl_c20_dp_uhbr10,
+	&xe2hpd_c20_dp_uhbr13_5,
+	&mtl_c20_dp_uhbr20,
+	NULL,
+};
+
 /*
  * HDMI link rates with 38.4 MHz reference clock.
  */
@@ -2242,11 +2258,15 @@ intel_c20_pll_tables_get(struct intel_crtc_state *crtc_state,
 	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
 
 	if (intel_crtc_has_dp_encoder(crtc_state)) {
-		if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_EDP))
-			return xe2hpd_c20_edp_tables;
+		if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_EDP)) {
+			if (DISPLAY_VER_FULL(i915) == IP_VER(14, 1))
+				return xe2hpd_c20_edp_tables;
+		}
 
 		if (DISPLAY_VER_FULL(i915) == IP_VER(14, 1))
 			return xe2hpd_c20_dp_tables;
+		else if (DISPLAY_VER(i915) >= 30)
+			return xe3lpd_c20_dp_edp_tables;
 		else
 			return mtl_c20_dp_tables;
 
-- 
2.45.0

[PATCH 08/10] drm/i915/xe3lpd: Add new bit range of MAX swing setup

[Matt Atwood]

From: Suraj Kandpal <suraj.kandpal@intel.com>

Add new bit range for Max PHY Swing Setup in PORT_ALPM_CTL
register for DISPLAY_VER >= 30.

Bspec: 70277
Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
---
 drivers/gpu/drm/i915/display/intel_alpm.c     | 2 +-
 drivers/gpu/drm/i915/display/intel_psr_regs.h | 4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c b/drivers/gpu/drm/i915/display/intel_alpm.c
index 55f3ae1e68c9..100ce776a203 100644
--- a/drivers/gpu/drm/i915/display/intel_alpm.c
+++ b/drivers/gpu/drm/i915/display/intel_alpm.c
@@ -334,7 +334,7 @@ static void lnl_alpm_configure(struct intel_dp *intel_dp,
 		intel_de_write(display,
 			       PORT_ALPM_CTL(port),
 			       PORT_ALPM_CTL_ALPM_AUX_LESS_ENABLE |
-			       PORT_ALPM_CTL_MAX_PHY_SWING_SETUP(15) |
+			       PORT_ALPM_CTL_MAX_PHY_SWING_SETUP(display, 15) |
 			       PORT_ALPM_CTL_MAX_PHY_SWING_HOLD(0) |
 			       PORT_ALPM_CTL_SILENCE_PERIOD(
 				       intel_dp->alpm_parameters.silence_period_sym_clocks));
diff --git a/drivers/gpu/drm/i915/display/intel_psr_regs.h b/drivers/gpu/drm/i915/display/intel_psr_regs.h
index 0841242543ca..046e400704e8 100644
--- a/drivers/gpu/drm/i915/display/intel_psr_regs.h
+++ b/drivers/gpu/drm/i915/display/intel_psr_regs.h
@@ -299,7 +299,9 @@
 #define PORT_ALPM_CTL(port)			_MMIO_PORT(port, _PORT_ALPM_CTL_A, _PORT_ALPM_CTL_B)
 #define  PORT_ALPM_CTL_ALPM_AUX_LESS_ENABLE	REG_BIT(31)
 #define  PORT_ALPM_CTL_MAX_PHY_SWING_SETUP_MASK	REG_GENMASK(23, 20)
-#define  PORT_ALPM_CTL_MAX_PHY_SWING_SETUP(val)	REG_FIELD_PREP(PORT_ALPM_CTL_MAX_PHY_SWING_SETUP_MASK, val)
+#define  PTL_PORT_ALPM_CTL_MAX_PHY_SWING_SETUP_MASK	REG_GENMASK(25, 20)
+#define  PORT_ALPM_CTL_MAX_PHY_SWING_SETUP(display, val)	(DISPLAY_VER(display) >= 30 ? REG_FIELD_PREP(PTL_PORT_ALPM_CTL_MAX_PHY_SWING_SETUP_MASK, val) :\
+							 REG_FIELD_PREP(PORT_ALPM_CTL_MAX_PHY_SWING_SETUP_MASK, val))
 #define  PORT_ALPM_CTL_MAX_PHY_SWING_HOLD_MASK	REG_GENMASK(19, 16)
 #define  PORT_ALPM_CTL_MAX_PHY_SWING_HOLD(val)	REG_FIELD_PREP(PORT_ALPM_CTL_MAX_PHY_SWING_HOLD_MASK, val)
 #define  PORT_ALPM_CTL_SILENCE_PERIOD_MASK	REG_GENMASK(7, 0)
-- 
2.45.0

[PATCH 09/10] drm/i915/xe3lpd: Add check to see if edp over type c is allowed

[Matt Atwood]

From: Suraj Kandpal <suraj.kandpal@intel.com>

Read PICA register to see if edp over type C is possible and then
add the appropriate tables for it.

Bspec: 68846
Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
---
 drivers/gpu/drm/i915/display/intel_cx0_phy.c     |  2 ++
 .../gpu/drm/i915/display/intel_display_types.h   |  1 +
 drivers/gpu/drm/i915/display/intel_dp.c          | 16 ++++++++++++++++
 drivers/gpu/drm/i915/i915_reg.h                  |  3 +++
 4 files changed, 22 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
index 0d6f75ae35f5..1c8c2a2b05e1 100644
--- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
+++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
@@ -2261,6 +2261,8 @@ intel_c20_pll_tables_get(struct intel_crtc_state *crtc_state,
 		if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_EDP)) {
 			if (DISPLAY_VER_FULL(i915) == IP_VER(14, 1))
 				return xe2hpd_c20_edp_tables;
+			if (DISPLAY_VER(i915) >= 30 && encoder->typec_supp)
+				return xe3lpd_c20_dp_edp_tables;
 		}
 
 		if (DISPLAY_VER_FULL(i915) == IP_VER(14, 1))
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 2bb1fa64da2f..e9dc7707fbcd 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -158,6 +158,7 @@ struct intel_encoder {
 	enum port port;
 	u16 cloneable;
 	u8 pipe_mask;
+	bool typec_supp;
 
 	/* Check and recover a bad link state. */
 	struct delayed_work link_check_work;
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index fbb096be02ad..917a503cc43b 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -5570,6 +5570,20 @@ intel_dp_detect_sdp_caps(struct intel_dp *intel_dp)
 		drm_dp_as_sdp_supported(&intel_dp->aux, intel_dp->dpcd);
 }
 
+static void
+intel_dp_check_edp_typec_supp(struct intel_encoder *encoder)
+{
+	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
+	bool is_tc_port = intel_encoder_is_tc(encoder);
+	u32 ret = 0;
+
+	if (encoder->type != INTEL_OUTPUT_EDP || !is_tc_port)
+		return;
+
+	ret = intel_de_read(i915, PICA_PHY_CONFIG_CONTROL);
+	encoder->typec_supp = ret & EDP_ON_TYPEC;
+}
+
 static int
 intel_dp_detect(struct drm_connector *connector,
 		struct drm_modeset_acquire_ctx *ctx,
@@ -5595,6 +5609,8 @@ intel_dp_detect(struct drm_connector *connector,
 	if (!intel_display_driver_check_access(dev_priv))
 		return connector->status;
 
+	intel_dp_check_edp_typec_supp(encoder);
+
 	/* Can't disconnect eDP */
 	if (intel_dp_is_edp(intel_dp))
 		status = edp_detect(intel_dp);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index da65500cd0c8..5f5a6ade5f8c 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4583,4 +4583,7 @@ enum skl_power_gate {
 
 #define MTL_MEDIA_GSI_BASE		0x380000
 
+#define PICA_PHY_CONFIG_CONTROL 	_MMIO(0x16FE68)
+#define   EDP_ON_TYPEC			REG_BIT(31)
+
 #endif /* _I915_REG_H_ */
-- 
2.45.0

[PATCH 10/10] drm/i915/xe3lpd: Add powerdown value of eDP over type c

[Matt Atwood]

From: Suraj Kandpal <suraj.kandpal@intel.com>

Add condition for P2.PG power down value.

Bspec: 74494
Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
---
 drivers/gpu/drm/i915/display/intel_cx0_phy.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
index 1c8c2a2b05e1..3d95ee65a9f1 100644
--- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
+++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
@@ -3144,7 +3144,8 @@ static u8 cx0_power_control_disable_val(struct intel_encoder *encoder)
 	if (intel_encoder_is_c10phy(encoder))
 		return CX0_P2PG_STATE_DISABLE;
 
-	if (IS_BATTLEMAGE(i915) && encoder->port == PORT_A)
+	if ((IS_BATTLEMAGE(i915) && encoder->port == PORT_A) ||
+	    (DISPLAY_VER(i915) >= 30 && encoder->type == INTEL_OUTPUT_EDP))
 		return CX0_P2PG_STATE_DISABLE;
 
 	return CX0_P4PG_STATE_DISABLE;
-- 
2.45.0

Re: [PATCH 01/10] drm/i915/xe3lpd: reuse xe2lpd definition

[Matt Roper]

On Tue, Oct 08, 2024 at 03:37:32PM -0700, Matt Atwood wrote:
> From: Clint Taylor <clinton.a.taylor@intel.com>
> 
> xe3_lpd display is functionally identical to xe2_lpd for now so reuse
> the device description.  A separate xe3 definition will be added in the
> future if/when new feature flags are required.
> 
> Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
> Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>

Reviewed-by: Matt Roper <matthew.d.roper@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_display_device.c |  6 ++++++
>  drivers/gpu/drm/i915/display/intel_display_device.h |  2 ++
>  include/drm/intel/i915_pciids.h                     | 12 ++++++++++++
>  3 files changed, 20 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_device.c b/drivers/gpu/drm/i915/display/intel_display_device.c
> index f33062322c66..aa22189e3853 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_device.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_device.c
> @@ -1252,6 +1252,10 @@ static const struct platform_desc bmg_desc = {
>  	PLATFORM(BATTLEMAGE),
>  };
>  
> +static const struct platform_desc ptl_desc = {
> +	PLATFORM(PANTHERLAKE),
> +};
> +
>  __diag_pop();
>  
>  /*
> @@ -1322,6 +1326,7 @@ static const struct {
>  	INTEL_MTL_IDS(INTEL_DISPLAY_DEVICE, &mtl_desc),
>  	INTEL_LNL_IDS(INTEL_DISPLAY_DEVICE, &lnl_desc),
>  	INTEL_BMG_IDS(INTEL_DISPLAY_DEVICE, &bmg_desc),
> +	INTEL_PTL_IDS(INTEL_DISPLAY_DEVICE, &ptl_desc),
>  };
>  
>  static const struct {
> @@ -1332,6 +1337,7 @@ static const struct {
>  	{ 14,  0, &xe_lpdp_display },
>  	{ 14,  1, &xe2_hpd_display },
>  	{ 20,  0, &xe2_lpd_display },
> +	{ 30,  0, &xe2_lpd_display },
>  };
>  
>  static const struct intel_display_device_info *
> diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h b/drivers/gpu/drm/i915/display/intel_display_device.h
> index 3ef537fa551a..071a36b51f79 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_device.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_device.h
> @@ -70,6 +70,8 @@ enum intel_display_platform {
>  	INTEL_DISPLAY_LUNARLAKE,
>  	/* Display ver 14.1 (based on GMD ID) */
>  	INTEL_DISPLAY_BATTLEMAGE,
> +	/* Display ver 30 (based on GMD ID) */
> +	INTEL_DISPLAY_PANTHERLAKE,
>  };
>  
>  enum intel_display_subplatform {
> diff --git a/include/drm/intel/i915_pciids.h b/include/drm/intel/i915_pciids.h
> index 02156c6f79b6..6b92f8c3731b 100644
> --- a/include/drm/intel/i915_pciids.h
> +++ b/include/drm/intel/i915_pciids.h
> @@ -794,4 +794,16 @@
>  	MACRO__(0xE20D, ## __VA_ARGS__), \
>  	MACRO__(0xE212, ## __VA_ARGS__)
>  
> +/* PTL */
> +#define INTEL_PTL_IDS(MACRO__, ...) \
> +	MACRO__(0xB080, ## __VA_ARGS__), \
> +	MACRO__(0xB081, ## __VA_ARGS__), \
> +	MACRO__(0xB082, ## __VA_ARGS__), \
> +	MACRO__(0xB090, ## __VA_ARGS__), \
> +	MACRO__(0xB091, ## __VA_ARGS__), \
> +	MACRO__(0xB092, ## __VA_ARGS__), \
> +	MACRO__(0xB0A0, ## __VA_ARGS__), \
> +	MACRO__(0xB0A1, ## __VA_ARGS__), \
> +	MACRO__(0xB0A2, ## __VA_ARGS__)
> +
>  #endif /* _I915_PCIIDS_H */
> -- 
> 2.45.0
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation

Re: [PATCH 05/10] drm/i915/xe3lpd: Add cdclk changes

[Matt Roper]

On Tue, Oct 08, 2024 at 03:37:36PM -0700, Matt Atwood wrote:
> From: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> 
> Xe3_LPD has new max cdclk of 691200 which requires reusing the lnl table
> and modify/add higher frequencies. Updating the max cdclk supported by
> the platform and voltage_level determination is also updated.
> 
> There are minor changes in cdclk programming sequence compared to lnl,
> where programming cd2x divider needs to be skipped. This is already handled
> by the calculations in existing code.
> 
> Bspec: 68861, 68863, 68864
> Cc: Gustavo Sousa <gustavo.sousa@intel.com>
> Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_cdclk.c | 56 +++++++++++++++++++++-
>  1 file changed, 54 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index fa1c2012b10c..6ac7bd6afc36 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -1468,6 +1468,32 @@ static const struct intel_cdclk_vals xe2hpd_cdclk_table[] = {
>  	{}
>  };
>  
> +static const struct intel_cdclk_vals xe3lpd_cdclk_table[] = {
> +	{ .refclk = 38400, .cdclk = 153600, .ratio = 16, .waveform = 0xaaaa },
> +	{ .refclk = 38400, .cdclk = 172800, .ratio = 16, .waveform = 0xad5a },
> +	{ .refclk = 38400, .cdclk = 192000, .ratio = 16, .waveform = 0xb6b6 },
> +	{ .refclk = 38400, .cdclk = 211200, .ratio = 16, .waveform = 0xdbb6 },
> +	{ .refclk = 38400, .cdclk = 230400, .ratio = 16, .waveform = 0xeeee },
> +	{ .refclk = 38400, .cdclk = 249600, .ratio = 16, .waveform = 0xf7de },
> +	{ .refclk = 38400, .cdclk = 268800, .ratio = 16, .waveform = 0xfefe },
> +	{ .refclk = 38400, .cdclk = 288000, .ratio = 16, .waveform = 0xfffe },
> +	{ .refclk = 38400, .cdclk = 307200, .ratio = 16, .waveform = 0xffff },

Up to this point the table seems to match what I see in the spec.  But
after this point most of the entries below don't show up in the bspec,
and the ones in the bspec aren't included here.  Looks like things have
changed since this patch was originally written.

> +	{ .refclk = 38400, .cdclk = 330000, .ratio = 25, .waveform = 0xdbb6 },
> +	{ .refclk = 38400, .cdclk = 360000, .ratio = 25, .waveform = 0xeeee },
> +	{ .refclk = 38400, .cdclk = 390000, .ratio = 25, .waveform = 0xf7de },
> +	{ .refclk = 38400, .cdclk = 420000, .ratio = 25, .waveform = 0xfefe },
> +	{ .refclk = 38400, .cdclk = 450000, .ratio = 25, .waveform = 0xfffe },
> +	{ .refclk = 38400, .cdclk = 480000, .ratio = 25, .waveform = 0xffff },
> +	{ .refclk = 38400, .cdclk = 487200, .ratio = 29, .waveform = 0xfefe },
> +	{ .refclk = 38400, .cdclk = 522000, .ratio = 29, .waveform = 0xfffe },
> +	{ .refclk = 38400, .cdclk = 556800, .ratio = 29, .waveform = 0xffff },
> +	{ .refclk = 38400, .cdclk = 561600, .ratio = 36, .waveform = 0xf7de },
> +	{ .refclk = 38400, .cdclk = 604800, .ratio = 36, .waveform = 0xfefe },
> +	{ .refclk = 38400, .cdclk = 648000, .ratio = 36, .waveform = 0xfffe },
> +	{ .refclk = 38400, .cdclk = 691200, .ratio = 36, .waveform = 0xffff },
> +	{}
> +};
> +
>  static const int cdclk_squash_len = 16;
>  
>  static int cdclk_squash_divider(u16 waveform)
> @@ -1594,6 +1620,20 @@ static u8 rplu_calc_voltage_level(int cdclk)
>  				  rplu_voltage_level_max_cdclk);
>  }
>  
> +static u8 xe3lpd_calc_voltage_level(int cdclk)
> +{
> +	static const int xe3lpd_voltage_level_max_cdclk[] = {
> +		[0] = 307200,
> +		[1] = 480000,
> +		[2] = 556800,
> +		[3] = 691200,
> +	};

This doesn't seem to match the spec either.  I only see two levels (vmin
and elevated) rather than four.


Matt

> +
> +	return calc_voltage_level(cdclk,
> +				  ARRAY_SIZE(xe3lpd_voltage_level_max_cdclk),
> +				  xe3lpd_voltage_level_max_cdclk);
> +}
> +
>  static void icl_readout_refclk(struct intel_display *display,
>  			       struct intel_cdclk_config *cdclk_config)
>  {
> @@ -3437,7 +3477,9 @@ void intel_update_max_cdclk(struct intel_display *display)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(display->drm);
>  
> -	if (IS_JASPERLAKE(dev_priv) || IS_ELKHARTLAKE(dev_priv)) {
> +	if (DISPLAY_VER(display) >= 30) {
> +		display->cdclk.max_cdclk_freq = 691200;
> +	} else if (IS_JASPERLAKE(dev_priv) || IS_ELKHARTLAKE(dev_priv)) {
>  		if (display->cdclk.hw.ref == 24000)
>  			display->cdclk.max_cdclk_freq = 552000;
>  		else
> @@ -3650,6 +3692,13 @@ void intel_cdclk_debugfs_register(struct intel_display *display)
>  			    display, &i915_cdclk_info_fops);
>  }
>  
> +static const struct intel_cdclk_funcs xe3lpd_cdclk_funcs = {
> +	.get_cdclk = bxt_get_cdclk,
> +	.set_cdclk = bxt_set_cdclk,
> +	.modeset_calc_cdclk = bxt_modeset_calc_cdclk,
> +	.calc_voltage_level = xe3lpd_calc_voltage_level,
> +};
> +
>  static const struct intel_cdclk_funcs rplu_cdclk_funcs = {
>  	.get_cdclk = bxt_get_cdclk,
>  	.set_cdclk = bxt_set_cdclk,
> @@ -3794,7 +3843,10 @@ void intel_init_cdclk_hooks(struct intel_display *display)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(display->drm);
>  
> -	if (DISPLAY_VER(display) >= 20) {
> +	if (DISPLAY_VER(display) >= 30) {
> +		display->funcs.cdclk = &xe3lpd_cdclk_funcs;
> +		display->cdclk.table = xe3lpd_cdclk_table;
> +	} else if (DISPLAY_VER(display) >= 20) {
>  		display->funcs.cdclk = &rplu_cdclk_funcs;
>  		display->cdclk.table = xe2lpd_cdclk_table;
>  	} else if (DISPLAY_VER_FULL(display) >= IP_VER(14, 1)) {
> -- 
> 2.45.0
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation

Re: [PATCH 06/10] drm/i915/xe3lpd: Add macro to choose HDCP_LINE_REKEY bit

[Matt Roper]

On Tue, Oct 08, 2024 at 03:37:37PM -0700, Matt Atwood wrote:
> From: Suraj Kandpal <suraj.kandpal@intel.com>
> 
> DISPLAY_VER() >= 30 has the HDCP_LINE_REKEY bit redefined from
> bit 12 to bit 14. Create a macro to choose the correct bit based

Typo?  The actual bit moved to 15, not 14.

> on DISPLAY_VER().
> 
> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_hdcp.c | 5 +++--
>  drivers/gpu/drm/i915/i915_reg.h           | 2 +-
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
> index ed6aa87403e2..e9b0414590ce 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> @@ -47,10 +47,11 @@ intel_hdcp_disable_hdcp_line_rekeying(struct intel_encoder *encoder,
>  			intel_de_rmw(display, MTL_CHICKEN_TRANS(hdcp->cpu_transcoder),
>  				     0, HDCP_LINE_REKEY_DISABLE);
>  		else if (IS_DISPLAY_VER_STEP(display, IP_VER(14, 1), STEP_B0, STEP_FOREVER) ||
> -			 IS_DISPLAY_VER_STEP(display, IP_VER(20, 0), STEP_B0, STEP_FOREVER))
> +			 IS_DISPLAY_VER_STEP(display, IP_VER(20, 0), STEP_B0, STEP_FOREVER) ||
> +			 DISPLAY_VER(display) >= 30)

Is this correct?  The programming here is to account for Wa_16021352814
which applies to MTL, LNL, and BMG (but gives different direction for
different steppings of each platform).  The workaround does not apply to
PTL, so is there something else in the spec indicating that we need to
disable the rekeying?

Also, the commit message doesn't say anything about this change, only
about the disable bit switching spots in the register.


Matt

>  			intel_de_rmw(display,
>  				     TRANS_DDI_FUNC_CTL(display, hdcp->cpu_transcoder),
> -				     0, TRANS_DDI_HDCP_LINE_REKEY_DISABLE);
> +				     0, TRANS_DDI_HDCP_LINE_REKEY_DISABLE(display));
>  	}
>  }
>  
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index d30459f8d1cb..da65500cd0c8 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3832,7 +3832,7 @@ enum skl_power_gate {
>  #define  TRANS_DDI_EDP_INPUT_B_ONOFF	(5 << 12)
>  #define  TRANS_DDI_EDP_INPUT_C_ONOFF	(6 << 12)
>  #define  TRANS_DDI_EDP_INPUT_D_ONOFF	(7 << 12)
> -#define  TRANS_DDI_HDCP_LINE_REKEY_DISABLE	REG_BIT(12)
> +#define  TRANS_DDI_HDCP_LINE_REKEY_DISABLE(display)	(DISPLAY_VER(display) >= 30 ? REG_BIT(15) : REG_BIT(12))
>  #define  TRANS_DDI_MST_TRANSPORT_SELECT_MASK	REG_GENMASK(11, 10)
>  #define  TRANS_DDI_MST_TRANSPORT_SELECT(trans)	\
>  	REG_FIELD_PREP(TRANS_DDI_MST_TRANSPORT_SELECT_MASK, trans)
> -- 
> 2.45.0
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation

✗ Fi.CI.CHECKPATCH: warning for Add xe3lpd edp enabling

[Patchwork]

== Series Details ==

Series: Add xe3lpd edp enabling
URL   : https://patchwork.freedesktop.org/series/139731/
State : warning

== Summary ==

Error: dim checkpatch failed
b37e76d7e831 drm/i915/xe3lpd: reuse xe2lpd definition
-:67: ERROR:COMPLEX_MACRO: Macros with complex values should be enclosed in parentheses
#67: FILE: include/drm/intel/i915_pciids.h:798:
+#define INTEL_PTL_IDS(MACRO__, ...) \
+	MACRO__(0xB080, ## __VA_ARGS__), \
+	MACRO__(0xB081, ## __VA_ARGS__), \
+	MACRO__(0xB082, ## __VA_ARGS__), \
+	MACRO__(0xB090, ## __VA_ARGS__), \
+	MACRO__(0xB091, ## __VA_ARGS__), \
+	MACRO__(0xB092, ## __VA_ARGS__), \
+	MACRO__(0xB0A0, ## __VA_ARGS__), \
+	MACRO__(0xB0A1, ## __VA_ARGS__), \
+	MACRO__(0xB0A2, ## __VA_ARGS__)

-:67: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'MACRO__' - possible side-effects?
#67: FILE: include/drm/intel/i915_pciids.h:798:
+#define INTEL_PTL_IDS(MACRO__, ...) \
+	MACRO__(0xB080, ## __VA_ARGS__), \
+	MACRO__(0xB081, ## __VA_ARGS__), \
+	MACRO__(0xB082, ## __VA_ARGS__), \
+	MACRO__(0xB090, ## __VA_ARGS__), \
+	MACRO__(0xB091, ## __VA_ARGS__), \
+	MACRO__(0xB092, ## __VA_ARGS__), \
+	MACRO__(0xB0A0, ## __VA_ARGS__), \
+	MACRO__(0xB0A1, ## __VA_ARGS__), \
+	MACRO__(0xB0A2, ## __VA_ARGS__)

total: 1 errors, 0 warnings, 1 checks, 48 lines checked
a637248b3bb8 drm/i915/xe3lpd: Adjust watermark calculations
e520e7565407 drm/i915/xe3lpd: Add new display power wells
-:41: ERROR:COMPLEX_MACRO: Macros with complex values should be enclosed in parentheses
#41: FILE: drivers/gpu/drm/i915/display/intel_display_power_map.c:1602:
+#define XE3LPD_PW_C_POWER_DOMAINS \
+	POWER_DOMAIN_PIPE_C, \
+	POWER_DOMAIN_PIPE_PANEL_FITTER_C

-:45: ERROR:COMPLEX_MACRO: Macros with complex values should be enclosed in parentheses
#45: FILE: drivers/gpu/drm/i915/display/intel_display_power_map.c:1606:
+#define XE3LPD_PW_D_POWER_DOMAINS \
+	POWER_DOMAIN_PIPE_D, \
+	POWER_DOMAIN_PIPE_PANEL_FITTER_D

-:49: ERROR:COMPLEX_MACRO: Macros with complex values should be enclosed in parentheses
#49: FILE: drivers/gpu/drm/i915/display/intel_display_power_map.c:1610:
+#define XE3LPD_PW_2_POWER_DOMAINS \
+	XE3LPD_PW_C_POWER_DOMAINS, \
+	XE3LPD_PW_D_POWER_DOMAINS, \
+	POWER_DOMAIN_TRANSCODER_C, \
+	POWER_DOMAIN_TRANSCODER_D, \
+	POWER_DOMAIN_VGA, \
+	POWER_DOMAIN_PORT_DDI_LANES_TC1, \
+	POWER_DOMAIN_PORT_DDI_LANES_TC2, \
+	POWER_DOMAIN_PORT_DDI_LANES_TC3, \
+	POWER_DOMAIN_PORT_DDI_LANES_TC4

-:79: CHECK:OPEN_ENDED_LINE: Lines should not end with a '('
#79: FILE: drivers/gpu/drm/i915/display/intel_display_power_map.c:1640:
+		.instances = &I915_PW_INSTANCES(

-:88: CHECK:OPEN_ENDED_LINE: Lines should not end with a '('
#88: FILE: drivers/gpu/drm/i915/display/intel_display_power_map.c:1649:
+		.instances = &I915_PW_INSTANCES(

-:96: CHECK:OPEN_ENDED_LINE: Lines should not end with a '('
#96: FILE: drivers/gpu/drm/i915/display/intel_display_power_map.c:1657:
+		.instances = &I915_PW_INSTANCES(

-:104: CHECK:OPEN_ENDED_LINE: Lines should not end with a '('
#104: FILE: drivers/gpu/drm/i915/display/intel_display_power_map.c:1665:
+		.instances = &I915_PW_INSTANCES(

-:112: CHECK:OPEN_ENDED_LINE: Lines should not end with a '('
#112: FILE: drivers/gpu/drm/i915/display/intel_display_power_map.c:1673:
+		.instances = &I915_PW_INSTANCES(

-:120: CHECK:OPEN_ENDED_LINE: Lines should not end with a '('
#120: FILE: drivers/gpu/drm/i915/display/intel_display_power_map.c:1681:
+		.instances = &I915_PW_INSTANCES(

-:133: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#133: FILE: drivers/gpu/drm/i915/display/intel_display_power_map.c:1694:
+I915_DECL_PW_DOMAINS(xe3lpd_pwdoms_dc_off,
+	POWER_DOMAIN_DC_OFF,

-:142: CHECK:OPEN_ENDED_LINE: Lines should not end with a '('
#142: FILE: drivers/gpu/drm/i915/display/intel_display_power_map.c:1703:
+		.instances = &I915_PW_INSTANCES(

-:150: CHECK:LINE_SPACING: Please don't use multiple blank lines
#150: FILE: drivers/gpu/drm/i915/display/intel_display_power_map.c:1711:
+
+

total: 3 errors, 0 warnings, 9 checks, 147 lines checked
da2a35cfc7ac drm/i915/xe3lpd: Update pmdemand programming
d8f572374294 drm/i915/xe3lpd: Add cdclk changes
9055734b8617 drm/i915/xe3lpd: Add macro to choose HDCP_LINE_REKEY bit
-:40: WARNING:LONG_LINE: line length of 112 exceeds 100 columns
#40: FILE: drivers/gpu/drm/i915/i915_reg.h:3835:
+#define  TRANS_DDI_HDCP_LINE_REKEY_DISABLE(display)	(DISPLAY_VER(display) >= 30 ? REG_BIT(15) : REG_BIT(12))

total: 0 errors, 1 warnings, 0 checks, 21 lines checked
8ff4cc5b3ef2 drm/i915/xe3lpd: Add C20 Phy consolidated programming table
b03a021c1a52 drm/i915/xe3lpd: Add new bit range of MAX swing setup
-:36: WARNING:LONG_LINE: line length of 160 exceeds 100 columns
#36: FILE: drivers/gpu/drm/i915/display/intel_psr_regs.h:303:
+#define  PORT_ALPM_CTL_MAX_PHY_SWING_SETUP(display, val)	(DISPLAY_VER(display) >= 30 ? REG_FIELD_PREP(PTL_PORT_ALPM_CTL_MAX_PHY_SWING_SETUP_MASK, val) :\

-:36: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'val' - possible side-effects?
#36: FILE: drivers/gpu/drm/i915/display/intel_psr_regs.h:303:
+#define  PORT_ALPM_CTL_MAX_PHY_SWING_SETUP(display, val)	(DISPLAY_VER(display) >= 30 ? REG_FIELD_PREP(PTL_PORT_ALPM_CTL_MAX_PHY_SWING_SETUP_MASK, val) :\
+							 REG_FIELD_PREP(PORT_ALPM_CTL_MAX_PHY_SWING_SETUP_MASK, val))

-:37: WARNING:LONG_LINE: line length of 117 exceeds 100 columns
#37: FILE: drivers/gpu/drm/i915/display/intel_psr_regs.h:304:
+							 REG_FIELD_PREP(PORT_ALPM_CTL_MAX_PHY_SWING_SETUP_MASK, val))

total: 0 errors, 2 warnings, 1 checks, 18 lines checked
4dd2162af137 drm/i915/xe3lpd: Add check to see if edp over type c is allowed
-:81: WARNING:SPACE_BEFORE_TAB: please, no space before tabs
#81: FILE: drivers/gpu/drm/i915/i915_reg.h:4586:
+#define PICA_PHY_CONFIG_CONTROL ^I_MMIO(0x16FE68)$

total: 0 errors, 1 warnings, 0 checks, 50 lines checked
dc2b39126319 drm/i915/xe3lpd: Add powerdown value of eDP over type c

✗ Fi.CI.SPARSE: warning for Add xe3lpd edp enabling

[Patchwork]

== Series Details ==

Series: Add xe3lpd edp enabling
URL   : https://patchwork.freedesktop.org/series/139731/
State : warning

== Summary ==

Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.

✓ Fi.CI.BAT: success for Add xe3lpd edp enabling

[Patchwork]

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

== Series Details ==

Series: Add xe3lpd edp enabling
URL   : https://patchwork.freedesktop.org/series/139731/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_15495 -> Patchwork_139731v1
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

Participating hosts (43 -> 41)
------------------------------

  Missing    (2): bat-rpls-4 fi-snb-2520m 

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

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_module_load@reload:
    - fi-kbl-7567u:       [PASS][1] -> [DMESG-WARN][2] ([i915#11621] / [i915#180] / [i915#1982])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15495/fi-kbl-7567u/igt@i915_module_load@reload.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139731v1/fi-kbl-7567u/igt@i915_module_load@reload.html

  * igt@i915_selftest@live:
    - bat-twl-1:          [PASS][3] -> [INCOMPLETE][4] ([i915#12133] / [i915#9413])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15495/bat-twl-1/igt@i915_selftest@live.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139731v1/bat-twl-1/igt@i915_selftest@live.html
    - bat-twl-2:          [PASS][5] -> [INCOMPLETE][6] ([i915#12133] / [i915#9413])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15495/bat-twl-2/igt@i915_selftest@live.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139731v1/bat-twl-2/igt@i915_selftest@live.html

  * igt@i915_selftest@live@gt_lrc:
    - bat-twl-2:          [PASS][7] -> [INCOMPLETE][8] ([i915#9413])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15495/bat-twl-2/igt@i915_selftest@live@gt_lrc.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139731v1/bat-twl-2/igt@i915_selftest@live@gt_lrc.html
    - bat-twl-1:          [PASS][9] -> [INCOMPLETE][10] ([i915#10886] / [i915#9413])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15495/bat-twl-1/igt@i915_selftest@live@gt_lrc.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139731v1/bat-twl-1/igt@i915_selftest@live@gt_lrc.html

  * igt@i915_selftest@live@sanitycheck:
    - fi-kbl-7567u:       [PASS][11] -> [DMESG-WARN][12] ([i915#11621]) +49 other tests dmesg-warn
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15495/fi-kbl-7567u/igt@i915_selftest@live@sanitycheck.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139731v1/fi-kbl-7567u/igt@i915_selftest@live@sanitycheck.html

  * igt@i915_selftest@live@workarounds:
    - bat-adlp-9:         [PASS][13] -> [INCOMPLETE][14] ([i915#9413]) +1 other test incomplete
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15495/bat-adlp-9/igt@i915_selftest@live@workarounds.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139731v1/bat-adlp-9/igt@i915_selftest@live@workarounds.html

  * igt@kms_busy@basic@flip:
    - fi-kbl-7567u:       [PASS][15] -> [DMESG-WARN][16] ([i915#180])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15495/fi-kbl-7567u/igt@kms_busy@basic@flip.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139731v1/fi-kbl-7567u/igt@kms_busy@basic@flip.html

  * igt@kms_pm_rpm@basic-pci-d3-state:
    - fi-kbl-7567u:       [PASS][17] -> [DMESG-WARN][18] ([i915#11621] / [i915#180]) +50 other tests dmesg-warn
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15495/fi-kbl-7567u/igt@kms_pm_rpm@basic-pci-d3-state.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139731v1/fi-kbl-7567u/igt@kms_pm_rpm@basic-pci-d3-state.html

  
#### Possible fixes ####

  * igt@i915_selftest@live:
    - bat-mtlp-8:         [ABORT][19] ([i915#12216]) -> [PASS][20] +1 other test pass
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15495/bat-mtlp-8/igt@i915_selftest@live.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139731v1/bat-mtlp-8/igt@i915_selftest@live.html

  
#### Warnings ####

  * igt@i915_pm_rpm@module-reload:
    - fi-kbl-7567u:       [DMESG-WARN][21] ([i915#11621] / [i915#1982]) -> [DMESG-WARN][22] ([i915#11621] / [i915#180])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15495/fi-kbl-7567u/igt@i915_pm_rpm@module-reload.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_139731v1/fi-kbl-7567u/igt@i915_pm_rpm@module-reload.html

  
  [i915#10886]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/10886
  [i915#11621]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11621
  [i915#12133]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12133
  [i915#12216]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12216
  [i915#180]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/180
  [i915#1982]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/1982
  [i915#9413]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/9413


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

  * Linux: CI_DRM_15495 -> Patchwork_139731v1

  CI-20190529: 20190529
  CI_DRM_15495: c8e01f4159a790812aa3c38cf659d6480fc7d029 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_8061: 8061
  Patchwork_139731v1: c8e01f4159a790812aa3c38cf659d6480fc7d029 @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

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

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

Re: [PATCH 10/10] drm/i915/xe3lpd: Add powerdown value of eDP over type c

[Chauhan, Shekhar]

On 10/9/2024 4:07, Matt Atwood wrote:
> From: Suraj Kandpal <suraj.kandpal@intel.com>
>
> Add condition for P2.PG power down value.
>
> Bspec: 74494
> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
Reviewed-by: Shekhar Chauhan <shekhar.chauhan@intel.com>
> ---
>   drivers/gpu/drm/i915/display/intel_cx0_phy.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> index 1c8c2a2b05e1..3d95ee65a9f1 100644
> --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> @@ -3144,7 +3144,8 @@ static u8 cx0_power_control_disable_val(struct intel_encoder *encoder)
>   	if (intel_encoder_is_c10phy(encoder))
>   		return CX0_P2PG_STATE_DISABLE;
>   
> -	if (IS_BATTLEMAGE(i915) && encoder->port == PORT_A)
> +	if ((IS_BATTLEMAGE(i915) && encoder->port == PORT_A) ||
> +	    (DISPLAY_VER(i915) >= 30 && encoder->type == INTEL_OUTPUT_EDP))
>   		return CX0_P2PG_STATE_DISABLE;
>   
>   	return CX0_P4PG_STATE_DISABLE;

-- 
-shekhar

Re: [PATCH 08/10] drm/i915/xe3lpd: Add new bit range of MAX swing setup

[Chauhan, Shekhar]

On 10/9/2024 4:07, Matt Atwood wrote:
> From: Suraj Kandpal <suraj.kandpal@intel.com>
>
> Add new bit range for Max PHY Swing Setup in PORT_ALPM_CTL
> register for DISPLAY_VER >= 30.
>
> Bspec: 70277
> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> ---
Reviewed-by: Shekhar Chauhan <shekhar.chauhan@intel.com>
>   drivers/gpu/drm/i915/display/intel_alpm.c     | 2 +-
>   drivers/gpu/drm/i915/display/intel_psr_regs.h | 4 +++-
>   2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c b/drivers/gpu/drm/i915/display/intel_alpm.c
> index 55f3ae1e68c9..100ce776a203 100644
> --- a/drivers/gpu/drm/i915/display/intel_alpm.c
> +++ b/drivers/gpu/drm/i915/display/intel_alpm.c
> @@ -334,7 +334,7 @@ static void lnl_alpm_configure(struct intel_dp *intel_dp,
>   		intel_de_write(display,
>   			       PORT_ALPM_CTL(port),
>   			       PORT_ALPM_CTL_ALPM_AUX_LESS_ENABLE |
> -			       PORT_ALPM_CTL_MAX_PHY_SWING_SETUP(15) |
> +			       PORT_ALPM_CTL_MAX_PHY_SWING_SETUP(display, 15) |
>   			       PORT_ALPM_CTL_MAX_PHY_SWING_HOLD(0) |
>   			       PORT_ALPM_CTL_SILENCE_PERIOD(
>   				       intel_dp->alpm_parameters.silence_period_sym_clocks));
> diff --git a/drivers/gpu/drm/i915/display/intel_psr_regs.h b/drivers/gpu/drm/i915/display/intel_psr_regs.h
> index 0841242543ca..046e400704e8 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr_regs.h
> +++ b/drivers/gpu/drm/i915/display/intel_psr_regs.h
> @@ -299,7 +299,9 @@
>   #define PORT_ALPM_CTL(port)			_MMIO_PORT(port, _PORT_ALPM_CTL_A, _PORT_ALPM_CTL_B)
>   #define  PORT_ALPM_CTL_ALPM_AUX_LESS_ENABLE	REG_BIT(31)
>   #define  PORT_ALPM_CTL_MAX_PHY_SWING_SETUP_MASK	REG_GENMASK(23, 20)
> -#define  PORT_ALPM_CTL_MAX_PHY_SWING_SETUP(val)	REG_FIELD_PREP(PORT_ALPM_CTL_MAX_PHY_SWING_SETUP_MASK, val)
> +#define  PTL_PORT_ALPM_CTL_MAX_PHY_SWING_SETUP_MASK	REG_GENMASK(25, 20)
> +#define  PORT_ALPM_CTL_MAX_PHY_SWING_SETUP(display, val)	(DISPLAY_VER(display) >= 30 ? REG_FIELD_PREP(PTL_PORT_ALPM_CTL_MAX_PHY_SWING_SETUP_MASK, val) :\
> +							 REG_FIELD_PREP(PORT_ALPM_CTL_MAX_PHY_SWING_SETUP_MASK, val))
>   #define  PORT_ALPM_CTL_MAX_PHY_SWING_HOLD_MASK	REG_GENMASK(19, 16)
>   #define  PORT_ALPM_CTL_MAX_PHY_SWING_HOLD(val)	REG_FIELD_PREP(PORT_ALPM_CTL_MAX_PHY_SWING_HOLD_MASK, val)
>   #define  PORT_ALPM_CTL_SILENCE_PERIOD_MASK	REG_GENMASK(7, 0)

-- 
-shekhar

Re: [PATCH 06/10] drm/i915/xe3lpd: Add macro to choose HDCP_LINE_REKEY bit

[Jani Nikula]

On Tue, 08 Oct 2024, Matt Atwood <matthew.s.atwood@intel.com> wrote:
> From: Suraj Kandpal <suraj.kandpal@intel.com>
>
> DISPLAY_VER() >= 30 has the HDCP_LINE_REKEY bit redefined from
> bit 12 to bit 14. Create a macro to choose the correct bit based
> on DISPLAY_VER().
>
> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_hdcp.c | 5 +++--
>  drivers/gpu/drm/i915/i915_reg.h           | 2 +-
>  2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c
> index ed6aa87403e2..e9b0414590ce 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> @@ -47,10 +47,11 @@ intel_hdcp_disable_hdcp_line_rekeying(struct intel_encoder *encoder,
>  			intel_de_rmw(display, MTL_CHICKEN_TRANS(hdcp->cpu_transcoder),
>  				     0, HDCP_LINE_REKEY_DISABLE);
>  		else if (IS_DISPLAY_VER_STEP(display, IP_VER(14, 1), STEP_B0, STEP_FOREVER) ||
> -			 IS_DISPLAY_VER_STEP(display, IP_VER(20, 0), STEP_B0, STEP_FOREVER))
> +			 IS_DISPLAY_VER_STEP(display, IP_VER(20, 0), STEP_B0, STEP_FOREVER) ||
> +			 DISPLAY_VER(display) >= 30)
>  			intel_de_rmw(display,
>  				     TRANS_DDI_FUNC_CTL(display, hdcp->cpu_transcoder),
> -				     0, TRANS_DDI_HDCP_LINE_REKEY_DISABLE);
> +				     0, TRANS_DDI_HDCP_LINE_REKEY_DISABLE(display));
>  	}
>  }
>  
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index d30459f8d1cb..da65500cd0c8 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3832,7 +3832,7 @@ enum skl_power_gate {
>  #define  TRANS_DDI_EDP_INPUT_B_ONOFF	(5 << 12)
>  #define  TRANS_DDI_EDP_INPUT_C_ONOFF	(6 << 12)
>  #define  TRANS_DDI_EDP_INPUT_D_ONOFF	(7 << 12)
> -#define  TRANS_DDI_HDCP_LINE_REKEY_DISABLE	REG_BIT(12)
> +#define  TRANS_DDI_HDCP_LINE_REKEY_DISABLE(display)	(DISPLAY_VER(display) >= 30 ? REG_BIT(15) : REG_BIT(12))

Do we really want to extend this style to individual bits?

BR,
Jani.

>  #define  TRANS_DDI_MST_TRANSPORT_SELECT_MASK	REG_GENMASK(11, 10)
>  #define  TRANS_DDI_MST_TRANSPORT_SELECT(trans)	\
>  	REG_FIELD_PREP(TRANS_DDI_MST_TRANSPORT_SELECT_MASK, trans)

-- 
Jani Nikula, Intel

Re: [PATCH 08/10] drm/i915/xe3lpd: Add new bit range of MAX swing setup

[Jani Nikula]

On Tue, 08 Oct 2024, Matt Atwood <matthew.s.atwood@intel.com> wrote:
> From: Suraj Kandpal <suraj.kandpal@intel.com>
>
> Add new bit range for Max PHY Swing Setup in PORT_ALPM_CTL
> register for DISPLAY_VER >= 30.
>
> Bspec: 70277
> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_alpm.c     | 2 +-
>  drivers/gpu/drm/i915/display/intel_psr_regs.h | 4 +++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c b/drivers/gpu/drm/i915/display/intel_alpm.c
> index 55f3ae1e68c9..100ce776a203 100644
> --- a/drivers/gpu/drm/i915/display/intel_alpm.c
> +++ b/drivers/gpu/drm/i915/display/intel_alpm.c
> @@ -334,7 +334,7 @@ static void lnl_alpm_configure(struct intel_dp *intel_dp,
>  		intel_de_write(display,
>  			       PORT_ALPM_CTL(port),
>  			       PORT_ALPM_CTL_ALPM_AUX_LESS_ENABLE |
> -			       PORT_ALPM_CTL_MAX_PHY_SWING_SETUP(15) |
> +			       PORT_ALPM_CTL_MAX_PHY_SWING_SETUP(display, 15) |
>  			       PORT_ALPM_CTL_MAX_PHY_SWING_HOLD(0) |
>  			       PORT_ALPM_CTL_SILENCE_PERIOD(
>  				       intel_dp->alpm_parameters.silence_period_sym_clocks));
> diff --git a/drivers/gpu/drm/i915/display/intel_psr_regs.h b/drivers/gpu/drm/i915/display/intel_psr_regs.h
> index 0841242543ca..046e400704e8 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr_regs.h
> +++ b/drivers/gpu/drm/i915/display/intel_psr_regs.h
> @@ -299,7 +299,9 @@
>  #define PORT_ALPM_CTL(port)			_MMIO_PORT(port, _PORT_ALPM_CTL_A, _PORT_ALPM_CTL_B)
>  #define  PORT_ALPM_CTL_ALPM_AUX_LESS_ENABLE	REG_BIT(31)
>  #define  PORT_ALPM_CTL_MAX_PHY_SWING_SETUP_MASK	REG_GENMASK(23, 20)
> -#define  PORT_ALPM_CTL_MAX_PHY_SWING_SETUP(val)	REG_FIELD_PREP(PORT_ALPM_CTL_MAX_PHY_SWING_SETUP_MASK, val)
> +#define  PTL_PORT_ALPM_CTL_MAX_PHY_SWING_SETUP_MASK	REG_GENMASK(25, 20)
> +#define  PORT_ALPM_CTL_MAX_PHY_SWING_SETUP(display, val)	(DISPLAY_VER(display) >= 30 ? REG_FIELD_PREP(PTL_PORT_ALPM_CTL_MAX_PHY_SWING_SETUP_MASK, val) :\
> +							 REG_FIELD_PREP(PORT_ALPM_CTL_MAX_PHY_SWING_SETUP_MASK, val))

I'm inclined to think this is not a good direction. Please define
separate macros for different platforms.

BR,
Jani.


>  #define  PORT_ALPM_CTL_MAX_PHY_SWING_HOLD_MASK	REG_GENMASK(19, 16)
>  #define  PORT_ALPM_CTL_MAX_PHY_SWING_HOLD(val)	REG_FIELD_PREP(PORT_ALPM_CTL_MAX_PHY_SWING_HOLD_MASK, val)
>  #define  PORT_ALPM_CTL_SILENCE_PERIOD_MASK	REG_GENMASK(7, 0)

-- 
Jani Nikula, Intel

Re: [PATCH 09/10] drm/i915/xe3lpd: Add check to see if edp over type c is allowed

[Jani Nikula]

On Tue, 08 Oct 2024, Matt Atwood <matthew.s.atwood@intel.com> wrote:
> From: Suraj Kandpal <suraj.kandpal@intel.com>
>
> Read PICA register to see if edp over type C is possible and then
> add the appropriate tables for it.

There's clearly more to be done for the feature than this.

>
> Bspec: 68846
> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_cx0_phy.c     |  2 ++
>  .../gpu/drm/i915/display/intel_display_types.h   |  1 +
>  drivers/gpu/drm/i915/display/intel_dp.c          | 16 ++++++++++++++++
>  drivers/gpu/drm/i915/i915_reg.h                  |  3 +++
>  4 files changed, 22 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> index 0d6f75ae35f5..1c8c2a2b05e1 100644
> --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> @@ -2261,6 +2261,8 @@ intel_c20_pll_tables_get(struct intel_crtc_state *crtc_state,
>  		if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_EDP)) {
>  			if (DISPLAY_VER_FULL(i915) == IP_VER(14, 1))
>  				return xe2hpd_c20_edp_tables;
> +			if (DISPLAY_VER(i915) >= 30 && encoder->typec_supp)
> +				return xe3lpd_c20_dp_edp_tables;
>  		}
>  
>  		if (DISPLAY_VER_FULL(i915) == IP_VER(14, 1))
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 2bb1fa64da2f..e9dc7707fbcd 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -158,6 +158,7 @@ struct intel_encoder {
>  	enum port port;
>  	u16 cloneable;
>  	u8 pipe_mask;
> +	bool typec_supp;

The register is global, why do we store this per encoder?

Side not, please let's not abbreviate stuff like _supp for the sake of
abbreviating stuff.

>  
>  	/* Check and recover a bad link state. */
>  	struct delayed_work link_check_work;
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index fbb096be02ad..917a503cc43b 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -5570,6 +5570,20 @@ intel_dp_detect_sdp_caps(struct intel_dp *intel_dp)
>  		drm_dp_as_sdp_supported(&intel_dp->aux, intel_dp->dpcd);
>  }
>  
> +static void
> +intel_dp_check_edp_typec_supp(struct intel_encoder *encoder)

It's not about checking anything, it's about reading, right?

> +{
> +	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> +	bool is_tc_port = intel_encoder_is_tc(encoder);
> +	u32 ret = 0;
> +
> +	if (encoder->type != INTEL_OUTPUT_EDP || !is_tc_port)

Currently we warn at connector init for eDP type-C combo.

> +		return;
> +
> +	ret = intel_de_read(i915, PICA_PHY_CONFIG_CONTROL);
> +	encoder->typec_supp = ret & EDP_ON_TYPEC;
> +}
> +
>  static int
>  intel_dp_detect(struct drm_connector *connector,
>  		struct drm_modeset_acquire_ctx *ctx,
> @@ -5595,6 +5609,8 @@ intel_dp_detect(struct drm_connector *connector,
>  	if (!intel_display_driver_check_access(dev_priv))
>  		return connector->status;
>  
> +	intel_dp_check_edp_typec_supp(encoder);
> +

Isn't this something that should be determined at intel_ddi_init() time?

BR,
Jani.


>  	/* Can't disconnect eDP */
>  	if (intel_dp_is_edp(intel_dp))
>  		status = edp_detect(intel_dp);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index da65500cd0c8..5f5a6ade5f8c 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4583,4 +4583,7 @@ enum skl_power_gate {
>  
>  #define MTL_MEDIA_GSI_BASE		0x380000
>  
> +#define PICA_PHY_CONFIG_CONTROL 	_MMIO(0x16FE68)
> +#define   EDP_ON_TYPEC			REG_BIT(31)
> +
>  #endif /* _I915_REG_H_ */

-- 
Jani Nikula, Intel

Re: [PATCH 10/10] drm/i915/xe3lpd: Add powerdown value of eDP over type c

[Jani Nikula]

On Tue, 08 Oct 2024, Matt Atwood <matthew.s.atwood@intel.com> wrote:
> From: Suraj Kandpal <suraj.kandpal@intel.com>
>
> Add condition for P2.PG power down value.
>
> Bspec: 74494
> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_cx0_phy.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> index 1c8c2a2b05e1..3d95ee65a9f1 100644
> --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> @@ -3144,7 +3144,8 @@ static u8 cx0_power_control_disable_val(struct intel_encoder *encoder)
>  	if (intel_encoder_is_c10phy(encoder))
>  		return CX0_P2PG_STATE_DISABLE;
>  
> -	if (IS_BATTLEMAGE(i915) && encoder->port == PORT_A)
> +	if ((IS_BATTLEMAGE(i915) && encoder->port == PORT_A) ||
> +	    (DISPLAY_VER(i915) >= 30 && encoder->type == INTEL_OUTPUT_EDP))
>  		return CX0_P2PG_STATE_DISABLE;

Does this match what the subject says?

BR,
Jani.

>  
>  	return CX0_P4PG_STATE_DISABLE;

-- 
Jani Nikula, Intel

Re: [PATCH 03/10] drm/i915/xe3lpd: Add new display power wells

[Luca Coelho]

On Tue, 2024-10-08 at 15:37 -0700, Matt Atwood wrote:
> From: Matt Roper <matthew.d.roper@intel.com>
> 
> Xe3's power well handling is similar to previous platforms, but there
> are a few changes that need to be handled to ensure optimal power
> management:
>  - PGB now only depends on PG1, not PG2
>  - Transcoder B is now in PG1 (was previously in PGB)
>  - Transcoders C & D are now in PG2 (were previously in PGC/PGD)
>  - DC states now require PG2 to be off (whereas on Xe2 it could remain
>    on as a dependency of PGB, although the features inside of it could
>    not be used).
> 
> Bspec: 72519, 68851
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> ---

Reviewed-by: Luca Coelho <luciano.coelho@intel.com>

--
Cheers,
Luca.

Re: [PATCH 02/10] drm/i915/xe3lpd: Adjust watermark calculations

[Govindapillai, Vinod]

On Tue, 2024-10-08 at 15:37 -0700, Matt Atwood wrote:
> From: Matt Roper <matthew.d.roper@intel.com>
> 
> Xe3 makes a couple minor tweaks to the watermark algorithm's block count
> calculations.
> 
> Bspec: 68985
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> ---
>  drivers/gpu/drm/i915/display/skl_watermark.c | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
> 

Looks good to me as per Bspec 68985

Reviewed-by: Vinod Govindapillai <vinod.govindapillai@intel.com>

> diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c
> b/drivers/gpu/drm/i915/display/skl_watermark.c
> index 6e1f04d5ef47..31de33e868df 100644
> --- a/drivers/gpu/drm/i915/display/skl_watermark.c
> +++ b/drivers/gpu/drm/i915/display/skl_watermark.c
> @@ -718,7 +718,7 @@ static int skl_compute_wm_params(const struct intel_crtc_state *crtc_state,
>                                  int width, const struct drm_format_info *format,
>                                  u64 modifier, unsigned int rotation,
>                                  u32 plane_pixel_rate, struct skl_wm_params *wp,
> -                                int color_plane);
> +                                int color_plane, unsigned int pan_x);
>  
>  static void skl_compute_plane_wm(const struct intel_crtc_state *crtc_state,
>                                  struct intel_plane *plane,
> @@ -765,7 +765,7 @@ skl_cursor_allocation(const struct intel_crtc_state *crtc_state,
>                                     drm_format_info(DRM_FORMAT_ARGB8888),
>                                     DRM_FORMAT_MOD_LINEAR,
>                                     DRM_MODE_ROTATE_0,
> -                                   crtc_state->pixel_rate, &wp, 0);
> +                                   crtc_state->pixel_rate, &wp, 0, 0);
>         drm_WARN_ON(&i915->drm, ret);
>  
>         for (level = 0; level < i915->display.wm.num_levels; level++) {
> @@ -1742,7 +1742,7 @@ skl_compute_wm_params(const struct intel_crtc_state *crtc_state,
>                       int width, const struct drm_format_info *format,
>                       u64 modifier, unsigned int rotation,
>                       u32 plane_pixel_rate, struct skl_wm_params *wp,
> -                     int color_plane)
> +                     int color_plane, unsigned int pan_x)
>  {
>         struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
>         struct drm_i915_private *i915 = to_i915(crtc->base.dev);
> @@ -1803,7 +1803,9 @@ skl_compute_wm_params(const struct intel_crtc_state *crtc_state,
>                                            wp->y_min_scanlines,
>                                            wp->dbuf_block_size);
>  
> -               if (DISPLAY_VER(i915) >= 10)
> +               if (DISPLAY_VER(i915) >= 30)
> +                       interm_pbpl += (pan_x != 0);
> +               else if (DISPLAY_VER(i915) >= 10)
>                         interm_pbpl++;
>  
>                 wp->plane_blocks_per_line = div_fixed16(interm_pbpl,
> @@ -1845,7 +1847,8 @@ skl_compute_plane_wm_params(const struct intel_crtc_state *crtc_state,
>                                      fb->format, fb->modifier,
>                                      plane_state->hw.rotation,
>                                      intel_plane_pixel_rate(crtc_state, plane_state),
> -                                    wp, color_plane);
> +                                    wp, color_plane,
> +                                    plane_state->uapi.src.x1);
>  }
>  
>  static bool skl_wm_has_lines(struct drm_i915_private *i915, int level)
> @@ -1909,7 +1912,10 @@ static void skl_compute_plane_wm(const struct intel_crtc_state *crtc_state,
>                 }
>         }
>  
> -       blocks = fixed16_to_u32_round_up(selected_result) + 1;
> +       blocks = fixed16_to_u32_round_up(selected_result);
> +       if (DISPLAY_VER(i915) < 30)
> +               blocks++;
> +
>         /*
>          * Lets have blocks at minimum equivalent to plane_blocks_per_line
>          * as there will be at minimum one line for lines configuration. This

Re: [PATCH 04/10] drm/i915/xe3lpd: Update pmdemand programming

[Govindapillai, Vinod]

Hi Matt,

Probably you missed one change...

On Tue, 2024-10-08 at 15:37 -0700, Matt Atwood wrote:
> From: Matt Roper <matthew.d.roper@intel.com>
> 
> There are some minor changes to pmdemand handling on Xe3:
>  - Active scalers are no longer tracked.  We can simply skip the readout
>    and programming of this field.
>  - Active dbuf slices are no longer tracked.  We should skip the readout
>    and programming of this field and also make sure that it stays 0 in
>    our software bookkeeping so that we won't erroneously return true
>    from intel_pmdemand_needs_update() due to mismatches.
>  - Even though there aren't enough pipes to utilize them, the size of
>    the 'active pipes' field has expanded to four bits, taking over the
>    register bits previously used for dbuf slices.  Since the lower bits
>    of the mask have moved, we need to update our reads/writes to handle
>    this properly.
> 
> Bspec: 68883, 69125
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_pmdemand.c | 61 +++++++++++++------
>  drivers/gpu/drm/i915/display/intel_pmdemand.h |  4 +-
>  drivers/gpu/drm/i915/i915_reg.h               |  1 +
>  3 files changed, 45 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_pmdemand.c
> b/drivers/gpu/drm/i915/display/intel_pmdemand.c
> index ceaf9e3147da..9af2f83d3a75 100644
> --- a/drivers/gpu/drm/i915/display/intel_pmdemand.c
> +++ b/drivers/gpu/drm/i915/display/intel_pmdemand.c
> @@ -258,6 +258,7 @@ intel_pmdemand_connector_needs_update(struct intel_atomic_state *state)
>  
>  static bool intel_pmdemand_needs_update(struct intel_atomic_state *state)
>  {
> +       struct drm_i915_private *i915 = to_i915(state->base.dev);
>         const struct intel_bw_state *new_bw_state, *old_bw_state;
>         const struct intel_cdclk_state *new_cdclk_state, *old_cdclk_state;
>         const struct intel_crtc_state *new_crtc_state, *old_crtc_state;
> @@ -274,12 +275,16 @@ static bool intel_pmdemand_needs_update(struct intel_atomic_state *state)
>         new_dbuf_state = intel_atomic_get_new_dbuf_state(state);
>         old_dbuf_state = intel_atomic_get_old_dbuf_state(state);
>         if (new_dbuf_state &&
> -           (new_dbuf_state->active_pipes !=
> -            old_dbuf_state->active_pipes ||
> -            new_dbuf_state->enabled_slices !=
> -            old_dbuf_state->enabled_slices))
> +           new_dbuf_state->active_pipes != old_dbuf_state->active_pipes)
>                 return true;
>  
> +       if (DISPLAY_VER(i915) < 30) {
> +               if (new_dbuf_state &&
> +                   new_dbuf_state->enabled_slices !=
> +                   old_dbuf_state->enabled_slices)
> +                       return true;
> +       }
> +
>         new_cdclk_state = intel_atomic_get_new_cdclk_state(state);
>         old_cdclk_state = intel_atomic_get_old_cdclk_state(state);
>         if (new_cdclk_state &&
> @@ -329,8 +334,10 @@ int intel_pmdemand_atomic_check(struct intel_atomic_state *state)
>  
>         new_pmdemand_state->params.active_pipes =
>                 min_t(u8, hweight8(new_dbuf_state->active_pipes), 3);
In xe3, min could be 4  (11b for 3 pipes and 100b for 4 pipes.)

BR
vinod

> -       new_pmdemand_state->params.active_dbufs =
> -               min_t(u8, hweight8(new_dbuf_state->enabled_slices), 3);
> +
> +       if (DISPLAY_VER(i915) < 30)
> +               new_pmdemand_state->params.active_dbufs =
> +                       min_t(u8, hweight8(new_dbuf_state->enabled_slices), 3);
>  
>         new_cdclk_state = intel_atomic_get_cdclk_state(state);
>         if (IS_ERR(new_cdclk_state))
> @@ -395,27 +402,32 @@ intel_pmdemand_init_pmdemand_params(struct drm_i915_private *i915,
>  
>         reg2 = intel_de_read(i915, XELPDP_INITIATE_PMDEMAND_REQUEST(1));
>  
> -       /* Set 1*/
>         pmdemand_state->params.qclk_gv_bw =
>                 REG_FIELD_GET(XELPDP_PMDEMAND_QCLK_GV_BW_MASK, reg1);
>         pmdemand_state->params.voltage_index =
>                 REG_FIELD_GET(XELPDP_PMDEMAND_VOLTAGE_INDEX_MASK, reg1);
>         pmdemand_state->params.qclk_gv_index =
>                 REG_FIELD_GET(XELPDP_PMDEMAND_QCLK_GV_INDEX_MASK, reg1);
> -       pmdemand_state->params.active_pipes =
> -               REG_FIELD_GET(XELPDP_PMDEMAND_PIPES_MASK, reg1);
> -       pmdemand_state->params.active_dbufs =
> -               REG_FIELD_GET(XELPDP_PMDEMAND_DBUFS_MASK, reg1);
>         pmdemand_state->params.active_phys =
>                 REG_FIELD_GET(XELPDP_PMDEMAND_PHYS_MASK, reg1);
>  
> -       /* Set 2*/
>         pmdemand_state->params.cdclk_freq_mhz =
>                 REG_FIELD_GET(XELPDP_PMDEMAND_CDCLK_FREQ_MASK, reg2);
>         pmdemand_state->params.ddiclk_max =
>                 REG_FIELD_GET(XELPDP_PMDEMAND_DDICLK_FREQ_MASK, reg2);
> -       pmdemand_state->params.scalers =
> -               REG_FIELD_GET(XELPDP_PMDEMAND_SCALERS_MASK, reg2);
> +
> +       if (DISPLAY_VER(i915) >= 30) {
> +               pmdemand_state->params.active_pipes =
> +                       REG_FIELD_GET(XE3_PMDEMAND_PIPES_MASK, reg1);
> +       } else {
> +               pmdemand_state->params.active_pipes =
> +                       REG_FIELD_GET(XELPDP_PMDEMAND_PIPES_MASK, reg1);
> +               pmdemand_state->params.active_dbufs =
> +                       REG_FIELD_GET(XELPDP_PMDEMAND_DBUFS_MASK, reg1);
> +
> +               pmdemand_state->params.scalers =
> +                       REG_FIELD_GET(XELPDP_PMDEMAND_SCALERS_MASK, reg2);
> +       }
>  
>  unlock:
>         mutex_unlock(&i915->display.pmdemand.lock);
> @@ -442,6 +454,10 @@ void intel_pmdemand_program_dbuf(struct drm_i915_private *i915,
>  {
>         u32 dbufs = min_t(u32, hweight8(dbuf_slices), 3);
>  
> +       /* PM Demand only tracks active dbufs on pre-Xe3 platforms */
> +       if (DISPLAY_VER(i915) >= 30)
> +               return;
> +
>         mutex_lock(&i915->display.pmdemand.lock);
>         if (drm_WARN_ON(&i915->drm,
>                         !intel_pmdemand_check_prev_transaction(i915)))
> @@ -460,7 +476,8 @@ void intel_pmdemand_program_dbuf(struct drm_i915_private *i915,
>  }
>  
>  static void
> -intel_pmdemand_update_params(const struct intel_pmdemand_state *new,
> +intel_pmdemand_update_params(struct drm_i915_private *i915,
> +                            const struct intel_pmdemand_state *new,
>                              const struct intel_pmdemand_state *old,
>                              u32 *reg1, u32 *reg2, bool serialized)
>  {
> @@ -495,16 +512,22 @@ intel_pmdemand_update_params(const struct intel_pmdemand_state *new,
>         update_reg(reg1, qclk_gv_bw, XELPDP_PMDEMAND_QCLK_GV_BW_MASK);
>         update_reg(reg1, voltage_index, XELPDP_PMDEMAND_VOLTAGE_INDEX_MASK);
>         update_reg(reg1, qclk_gv_index, XELPDP_PMDEMAND_QCLK_GV_INDEX_MASK);
> -       update_reg(reg1, active_pipes, XELPDP_PMDEMAND_PIPES_MASK);
> -       update_reg(reg1, active_dbufs, XELPDP_PMDEMAND_DBUFS_MASK);
>         update_reg(reg1, active_phys, XELPDP_PMDEMAND_PHYS_MASK);
>  
>         /* Set 2*/
>         update_reg(reg2, cdclk_freq_mhz, XELPDP_PMDEMAND_CDCLK_FREQ_MASK);
>         update_reg(reg2, ddiclk_max, XELPDP_PMDEMAND_DDICLK_FREQ_MASK);
> -       update_reg(reg2, scalers, XELPDP_PMDEMAND_SCALERS_MASK);
>         update_reg(reg2, plls, XELPDP_PMDEMAND_PLLS_MASK);
>  
> +       if (DISPLAY_VER(i915) >= 30) {
> +               update_reg(reg1, active_pipes, XE3_PMDEMAND_PIPES_MASK);
> +       } else {
> +               update_reg(reg1, active_pipes, XELPDP_PMDEMAND_PIPES_MASK);
> +               update_reg(reg1, active_dbufs, XELPDP_PMDEMAND_DBUFS_MASK);
> +
> +               update_reg(reg2, scalers, XELPDP_PMDEMAND_SCALERS_MASK);
> +       }
> +
>  #undef update_reg
>  }
>  
> @@ -529,7 +552,7 @@ intel_pmdemand_program_params(struct drm_i915_private *i915,
>         reg2 = intel_de_read(i915, XELPDP_INITIATE_PMDEMAND_REQUEST(1));
>         mod_reg2 = reg2;
>  
> -       intel_pmdemand_update_params(new, old, &mod_reg1, &mod_reg2,
> +       intel_pmdemand_update_params(i915, new, old, &mod_reg1, &mod_reg2,
>                                      serialized);
>  
>         if (reg1 != mod_reg1) {
> diff --git a/drivers/gpu/drm/i915/display/intel_pmdemand.h
> b/drivers/gpu/drm/i915/display/intel_pmdemand.h
> index 128fd61f8f14..a1c49efdc493 100644
> --- a/drivers/gpu/drm/i915/display/intel_pmdemand.h
> +++ b/drivers/gpu/drm/i915/display/intel_pmdemand.h
> @@ -20,14 +20,14 @@ struct pmdemand_params {
>         u8 voltage_index;
>         u8 qclk_gv_index;
>         u8 active_pipes;
> -       u8 active_dbufs;
> +       u8 active_dbufs;        /* pre-Xe3 only */
>         /* Total number of non type C active phys from active_phys_mask */
>         u8 active_phys;
>         u8 plls;
>         u16 cdclk_freq_mhz;
>         /* max from ddi_clocks[] */
>         u16 ddiclk_max;
> -       u8 scalers;
> +       u8 scalers;             /* pre-Xe3 only */
>  };
>  
>  struct intel_pmdemand_state {
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 818142f5a10c..d30459f8d1cb 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2705,6 +2705,7 @@
>  #define  XELPDP_PMDEMAND_QCLK_GV_BW_MASK               REG_GENMASK(31, 16)
>  #define  XELPDP_PMDEMAND_VOLTAGE_INDEX_MASK            REG_GENMASK(14, 12)
>  #define  XELPDP_PMDEMAND_QCLK_GV_INDEX_MASK            REG_GENMASK(11, 8)
> +#define  XE3_PMDEMAND_PIPES_MASK                       REG_GENMASK(7, 4)
>  #define  XELPDP_PMDEMAND_PIPES_MASK                    REG_GENMASK(7, 6)
>  #define  XELPDP_PMDEMAND_DBUFS_MASK                    REG_GENMASK(5, 4)
>  #define  XELPDP_PMDEMAND_PHYS_MASK                     REG_GENMASK(2, 0)

Re: [PATCH 04/10] drm/i915/xe3lpd: Update pmdemand programming

[Gustavo Sousa]

Quoting Govindapillai, Vinod (2024-10-09 10:09:45-03:00)
>Hi Matt,
>
>Probably you missed one change...
>
>On Tue, 2024-10-08 at 15:37 -0700, Matt Atwood wrote:
>> From: Matt Roper <matthew.d.roper@intel.com>
>> 
>> There are some minor changes to pmdemand handling on Xe3:
>>  - Active scalers are no longer tracked.  We can simply skip the readout
>>    and programming of this field.
>>  - Active dbuf slices are no longer tracked.  We should skip the readout
>>    and programming of this field and also make sure that it stays 0 in
>>    our software bookkeeping so that we won't erroneously return true
>>    from intel_pmdemand_needs_update() due to mismatches.
>>  - Even though there aren't enough pipes to utilize them, the size of
>>    the 'active pipes' field has expanded to four bits, taking over the
>>    register bits previously used for dbuf slices.  Since the lower bits
>>    of the mask have moved, we need to update our reads/writes to handle
>>    this properly.
>> 
>> Bspec: 68883, 69125
>> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
>> Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_pmdemand.c | 61 +++++++++++++------
>>  drivers/gpu/drm/i915/display/intel_pmdemand.h |  4 +-
>>  drivers/gpu/drm/i915/i915_reg.h               |  1 +
>>  3 files changed, 45 insertions(+), 21 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_pmdemand.c
>> b/drivers/gpu/drm/i915/display/intel_pmdemand.c
>> index ceaf9e3147da..9af2f83d3a75 100644
>> --- a/drivers/gpu/drm/i915/display/intel_pmdemand.c
>> +++ b/drivers/gpu/drm/i915/display/intel_pmdemand.c
>> @@ -258,6 +258,7 @@ intel_pmdemand_connector_needs_update(struct intel_atomic_state *state)
>>  
>>  static bool intel_pmdemand_needs_update(struct intel_atomic_state *state)
>>  {
>> +       struct drm_i915_private *i915 = to_i915(state->base.dev);
>>         const struct intel_bw_state *new_bw_state, *old_bw_state;
>>         const struct intel_cdclk_state *new_cdclk_state, *old_cdclk_state;
>>         const struct intel_crtc_state *new_crtc_state, *old_crtc_state;
>> @@ -274,12 +275,16 @@ static bool intel_pmdemand_needs_update(struct intel_atomic_state *state)
>>         new_dbuf_state = intel_atomic_get_new_dbuf_state(state);
>>         old_dbuf_state = intel_atomic_get_old_dbuf_state(state);
>>         if (new_dbuf_state &&
>> -           (new_dbuf_state->active_pipes !=
>> -            old_dbuf_state->active_pipes ||
>> -            new_dbuf_state->enabled_slices !=
>> -            old_dbuf_state->enabled_slices))
>> +           new_dbuf_state->active_pipes != old_dbuf_state->active_pipes)
>>                 return true;
>>  
>> +       if (DISPLAY_VER(i915) < 30) {
>> +               if (new_dbuf_state &&
>> +                   new_dbuf_state->enabled_slices !=
>> +                   old_dbuf_state->enabled_slices)
>> +                       return true;
>> +       }
>> +
>>         new_cdclk_state = intel_atomic_get_new_cdclk_state(state);
>>         old_cdclk_state = intel_atomic_get_old_cdclk_state(state);
>>         if (new_cdclk_state &&
>> @@ -329,8 +334,10 @@ int intel_pmdemand_atomic_check(struct intel_atomic_state *state)
>>  
>>         new_pmdemand_state->params.active_pipes =
>>                 min_t(u8, hweight8(new_dbuf_state->active_pipes), 3);
>In xe3, min could be 4  (11b for 3 pipes and 100b for 4 pipes.)

Good catch!

In this case, we could either:

- Apply min_t() only for pre-xe3 and just use the value directly for
  xe3. Bspec mentions that Pcode should clamp to the maximum defined
  number of pipes.
- Define a local max_active_pipes variable, using 3 for pre-xe3 and the
  number of possible pipes for xe3. Then use that variable in min_t().

I would prefer the latter, which does what Pcode is also supposed to do.
This little redundancy doesn't hurt and make things safer.

--
Gustavo Sousa

>
>BR
>vinod
>
>> -       new_pmdemand_state->params.active_dbufs =
>> -               min_t(u8, hweight8(new_dbuf_state->enabled_slices), 3);
>> +
>> +       if (DISPLAY_VER(i915) < 30)
>> +               new_pmdemand_state->params.active_dbufs =
>> +                       min_t(u8, hweight8(new_dbuf_state->enabled_slices), 3);
>>  
>>         new_cdclk_state = intel_atomic_get_cdclk_state(state);
>>         if (IS_ERR(new_cdclk_state))
>> @@ -395,27 +402,32 @@ intel_pmdemand_init_pmdemand_params(struct drm_i915_private *i915,
>>  
>>         reg2 = intel_de_read(i915, XELPDP_INITIATE_PMDEMAND_REQUEST(1));
>>  
>> -       /* Set 1*/
>>         pmdemand_state->params.qclk_gv_bw =
>>                 REG_FIELD_GET(XELPDP_PMDEMAND_QCLK_GV_BW_MASK, reg1);
>>         pmdemand_state->params.voltage_index =
>>                 REG_FIELD_GET(XELPDP_PMDEMAND_VOLTAGE_INDEX_MASK, reg1);
>>         pmdemand_state->params.qclk_gv_index =
>>                 REG_FIELD_GET(XELPDP_PMDEMAND_QCLK_GV_INDEX_MASK, reg1);
>> -       pmdemand_state->params.active_pipes =
>> -               REG_FIELD_GET(XELPDP_PMDEMAND_PIPES_MASK, reg1);
>> -       pmdemand_state->params.active_dbufs =
>> -               REG_FIELD_GET(XELPDP_PMDEMAND_DBUFS_MASK, reg1);
>>         pmdemand_state->params.active_phys =
>>                 REG_FIELD_GET(XELPDP_PMDEMAND_PHYS_MASK, reg1);
>>  
>> -       /* Set 2*/
>>         pmdemand_state->params.cdclk_freq_mhz =
>>                 REG_FIELD_GET(XELPDP_PMDEMAND_CDCLK_FREQ_MASK, reg2);
>>         pmdemand_state->params.ddiclk_max =
>>                 REG_FIELD_GET(XELPDP_PMDEMAND_DDICLK_FREQ_MASK, reg2);
>> -       pmdemand_state->params.scalers =
>> -               REG_FIELD_GET(XELPDP_PMDEMAND_SCALERS_MASK, reg2);
>> +
>> +       if (DISPLAY_VER(i915) >= 30) {
>> +               pmdemand_state->params.active_pipes =
>> +                       REG_FIELD_GET(XE3_PMDEMAND_PIPES_MASK, reg1);
>> +       } else {
>> +               pmdemand_state->params.active_pipes =
>> +                       REG_FIELD_GET(XELPDP_PMDEMAND_PIPES_MASK, reg1);
>> +               pmdemand_state->params.active_dbufs =
>> +                       REG_FIELD_GET(XELPDP_PMDEMAND_DBUFS_MASK, reg1);
>> +
>> +               pmdemand_state->params.scalers =
>> +                       REG_FIELD_GET(XELPDP_PMDEMAND_SCALERS_MASK, reg2);
>> +       }
>>  
>>  unlock:
>>         mutex_unlock(&i915->display.pmdemand.lock);
>> @@ -442,6 +454,10 @@ void intel_pmdemand_program_dbuf(struct drm_i915_private *i915,
>>  {
>>         u32 dbufs = min_t(u32, hweight8(dbuf_slices), 3);
>>  
>> +       /* PM Demand only tracks active dbufs on pre-Xe3 platforms */
>> +       if (DISPLAY_VER(i915) >= 30)
>> +               return;
>> +
>>         mutex_lock(&i915->display.pmdemand.lock);
>>         if (drm_WARN_ON(&i915->drm,
>>                         !intel_pmdemand_check_prev_transaction(i915)))
>> @@ -460,7 +476,8 @@ void intel_pmdemand_program_dbuf(struct drm_i915_private *i915,
>>  }
>>  
>>  static void
>> -intel_pmdemand_update_params(const struct intel_pmdemand_state *new,
>> +intel_pmdemand_update_params(struct drm_i915_private *i915,
>> +                            const struct intel_pmdemand_state *new,
>>                              const struct intel_pmdemand_state *old,
>>                              u32 *reg1, u32 *reg2, bool serialized)
>>  {
>> @@ -495,16 +512,22 @@ intel_pmdemand_update_params(const struct intel_pmdemand_state *new,
>>         update_reg(reg1, qclk_gv_bw, XELPDP_PMDEMAND_QCLK_GV_BW_MASK);
>>         update_reg(reg1, voltage_index, XELPDP_PMDEMAND_VOLTAGE_INDEX_MASK);
>>         update_reg(reg1, qclk_gv_index, XELPDP_PMDEMAND_QCLK_GV_INDEX_MASK);
>> -       update_reg(reg1, active_pipes, XELPDP_PMDEMAND_PIPES_MASK);
>> -       update_reg(reg1, active_dbufs, XELPDP_PMDEMAND_DBUFS_MASK);
>>         update_reg(reg1, active_phys, XELPDP_PMDEMAND_PHYS_MASK);
>>  
>>         /* Set 2*/
>>         update_reg(reg2, cdclk_freq_mhz, XELPDP_PMDEMAND_CDCLK_FREQ_MASK);
>>         update_reg(reg2, ddiclk_max, XELPDP_PMDEMAND_DDICLK_FREQ_MASK);
>> -       update_reg(reg2, scalers, XELPDP_PMDEMAND_SCALERS_MASK);
>>         update_reg(reg2, plls, XELPDP_PMDEMAND_PLLS_MASK);
>>  
>> +       if (DISPLAY_VER(i915) >= 30) {
>> +               update_reg(reg1, active_pipes, XE3_PMDEMAND_PIPES_MASK);
>> +       } else {
>> +               update_reg(reg1, active_pipes, XELPDP_PMDEMAND_PIPES_MASK);
>> +               update_reg(reg1, active_dbufs, XELPDP_PMDEMAND_DBUFS_MASK);
>> +
>> +               update_reg(reg2, scalers, XELPDP_PMDEMAND_SCALERS_MASK);
>> +       }
>> +
>>  #undef update_reg
>>  }
>>  
>> @@ -529,7 +552,7 @@ intel_pmdemand_program_params(struct drm_i915_private *i915,
>>         reg2 = intel_de_read(i915, XELPDP_INITIATE_PMDEMAND_REQUEST(1));
>>         mod_reg2 = reg2;
>>  
>> -       intel_pmdemand_update_params(new, old, &mod_reg1, &mod_reg2,
>> +       intel_pmdemand_update_params(i915, new, old, &mod_reg1, &mod_reg2,
>>                                      serialized);
>>  
>>         if (reg1 != mod_reg1) {
>> diff --git a/drivers/gpu/drm/i915/display/intel_pmdemand.h
>> b/drivers/gpu/drm/i915/display/intel_pmdemand.h
>> index 128fd61f8f14..a1c49efdc493 100644
>> --- a/drivers/gpu/drm/i915/display/intel_pmdemand.h
>> +++ b/drivers/gpu/drm/i915/display/intel_pmdemand.h
>> @@ -20,14 +20,14 @@ struct pmdemand_params {
>>         u8 voltage_index;
>>         u8 qclk_gv_index;
>>         u8 active_pipes;
>> -       u8 active_dbufs;
>> +       u8 active_dbufs;        /* pre-Xe3 only */
>>         /* Total number of non type C active phys from active_phys_mask */
>>         u8 active_phys;
>>         u8 plls;
>>         u16 cdclk_freq_mhz;
>>         /* max from ddi_clocks[] */
>>         u16 ddiclk_max;
>> -       u8 scalers;
>> +       u8 scalers;             /* pre-Xe3 only */
>>  };
>>  
>>  struct intel_pmdemand_state {
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 818142f5a10c..d30459f8d1cb 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -2705,6 +2705,7 @@
>>  #define  XELPDP_PMDEMAND_QCLK_GV_BW_MASK               REG_GENMASK(31, 16)
>>  #define  XELPDP_PMDEMAND_VOLTAGE_INDEX_MASK            REG_GENMASK(14, 12)
>>  #define  XELPDP_PMDEMAND_QCLK_GV_INDEX_MASK            REG_GENMASK(11, 8)
>> +#define  XE3_PMDEMAND_PIPES_MASK                       REG_GENMASK(7, 4)
>>  #define  XELPDP_PMDEMAND_PIPES_MASK                    REG_GENMASK(7, 6)
>>  #define  XELPDP_PMDEMAND_DBUFS_MASK                    REG_GENMASK(5, 4)
>>  #define  XELPDP_PMDEMAND_PHYS_MASK                     REG_GENMASK(2, 0)
>

Re: [PATCH 07/10] drm/i915/xe3lpd: Add C20 Phy consolidated programming table

[Taylor, Clinton A]

Reviewed-by: Clint Taylor <Clinton.A.Taylor@intel.com> 

-Clint

On Tue, 2024-10-08 at 15:37 -0700, Matt Atwood wrote:
> From: Suraj Kandpal <suraj.kandpal@intel.com>
> 
> From DISPLAY_VER() >= 30 C20 PHY consolidated programming table of
> DP and eDP been merged and now use the same rates and values. eDP
> over TypeC has also been introduced.
> Moreover it allows more granular and higher rates. Add new table to
> represent this change.
> 
> Bspec: 68961
> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_cx0_phy.c | 24 ++++++++++++++++++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> index 4a6c3040ca15..0d6f75ae35f5 100644
> --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> @@ -1122,6 +1122,22 @@ static const struct intel_c20pll_state * const
> xe2hpd_c20_dp_tables[] = {
>  	NULL,
>  };
>  
> +static const struct intel_c20pll_state * const xe3lpd_c20_dp_edp_tables[] = {
> +	&mtl_c20_dp_rbr,
> +	&xe2hpd_c20_edp_r216,
> +	&xe2hpd_c20_edp_r243,
> +	&mtl_c20_dp_hbr1,
> +	&xe2hpd_c20_edp_r324,
> +	&xe2hpd_c20_edp_r432,
> +	&mtl_c20_dp_hbr2,
> +	&xe2hpd_c20_edp_r675,
> +	&mtl_c20_dp_hbr3,
> +	&mtl_c20_dp_uhbr10,
> +	&xe2hpd_c20_dp_uhbr13_5,
> +	&mtl_c20_dp_uhbr20,
> +	NULL,
> +};
> +
>  /*
>   * HDMI link rates with 38.4 MHz reference clock.
>   */
> @@ -2242,11 +2258,15 @@ intel_c20_pll_tables_get(struct intel_crtc_state *crtc_state,
>  	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
>  
>  	if (intel_crtc_has_dp_encoder(crtc_state)) {
> -		if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_EDP))
> -			return xe2hpd_c20_edp_tables;
> +		if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_EDP)) {
> +			if (DISPLAY_VER_FULL(i915) == IP_VER(14, 1))
> +				return xe2hpd_c20_edp_tables;
> +		}
>  
>  		if (DISPLAY_VER_FULL(i915) == IP_VER(14, 1))
>  			return xe2hpd_c20_dp_tables;
> +		else if (DISPLAY_VER(i915) >= 30)
> +			return xe3lpd_c20_dp_edp_tables;
>  		else
>  			return mtl_c20_dp_tables;
>  

Re: [PATCH 10/10] drm/i915/xe3lpd: Add powerdown value of eDP over type c

[Matt Atwood]

On Wed, Oct 09, 2024 at 10:57:03AM +0300, Jani Nikula wrote:
> On Tue, 08 Oct 2024, Matt Atwood <matthew.s.atwood@intel.com> wrote:
> > From: Suraj Kandpal <suraj.kandpal@intel.com>
> >
> > Add condition for P2.PG power down value.
> >
> > Bspec: 74494
> > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_cx0_phy.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > index 1c8c2a2b05e1..3d95ee65a9f1 100644
> > --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > @@ -3144,7 +3144,8 @@ static u8 cx0_power_control_disable_val(struct intel_encoder *encoder)
> >  	if (intel_encoder_is_c10phy(encoder))
> >  		return CX0_P2PG_STATE_DISABLE;
> >  
> > -	if (IS_BATTLEMAGE(i915) && encoder->port == PORT_A)
> > +	if ((IS_BATTLEMAGE(i915) && encoder->port == PORT_A) ||
> > +	    (DISPLAY_VER(i915) >= 30 && encoder->type == INTEL_OUTPUT_EDP))
> >  		return CX0_P2PG_STATE_DISABLE;
> 
> Does this match what the subject says?
Please address Jani's comments
> 
> BR,
> Jani.
> 
> >  
> >  	return CX0_P4PG_STATE_DISABLE;
> 
> -- 
> Jani Nikula, Intel

Re: [PATCH 09/10] drm/i915/xe3lpd: Add check to see if edp over type c is allowed

[Matt Atwood]

On Wed, Oct 09, 2024 at 10:53:56AM +0300, Jani Nikula wrote:
> On Tue, 08 Oct 2024, Matt Atwood <matthew.s.atwood@intel.com> wrote:
> > From: Suraj Kandpal <suraj.kandpal@intel.com>
> >
> > Read PICA register to see if edp over type C is possible and then
> > add the appropriate tables for it.
> 
> There's clearly more to be done for the feature than this.
> 
> >
> > Bspec: 68846
> > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_cx0_phy.c     |  2 ++
> >  .../gpu/drm/i915/display/intel_display_types.h   |  1 +
> >  drivers/gpu/drm/i915/display/intel_dp.c          | 16 ++++++++++++++++
> >  drivers/gpu/drm/i915/i915_reg.h                  |  3 +++
> >  4 files changed, 22 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > index 0d6f75ae35f5..1c8c2a2b05e1 100644
> > --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > @@ -2261,6 +2261,8 @@ intel_c20_pll_tables_get(struct intel_crtc_state *crtc_state,
> >  		if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_EDP)) {
> >  			if (DISPLAY_VER_FULL(i915) == IP_VER(14, 1))
> >  				return xe2hpd_c20_edp_tables;
> > +			if (DISPLAY_VER(i915) >= 30 && encoder->typec_supp)
> > +				return xe3lpd_c20_dp_edp_tables;
> >  		}
> >  
> >  		if (DISPLAY_VER_FULL(i915) == IP_VER(14, 1))
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index 2bb1fa64da2f..e9dc7707fbcd 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -158,6 +158,7 @@ struct intel_encoder {
> >  	enum port port;
> >  	u16 cloneable;
> >  	u8 pipe_mask;
> > +	bool typec_supp;
> 
> The register is global, why do we store this per encoder?
> 
> Side not, please let's not abbreviate stuff like _supp for the sake of
> abbreviating stuff.
> 
> >  
> >  	/* Check and recover a bad link state. */
> >  	struct delayed_work link_check_work;
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > index fbb096be02ad..917a503cc43b 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -5570,6 +5570,20 @@ intel_dp_detect_sdp_caps(struct intel_dp *intel_dp)
> >  		drm_dp_as_sdp_supported(&intel_dp->aux, intel_dp->dpcd);
> >  }
> >  
> > +static void
> > +intel_dp_check_edp_typec_supp(struct intel_encoder *encoder)
> 
> It's not about checking anything, it's about reading, right?
> 
> > +{
> > +	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> > +	bool is_tc_port = intel_encoder_is_tc(encoder);
> > +	u32 ret = 0;
> > +
> > +	if (encoder->type != INTEL_OUTPUT_EDP || !is_tc_port)
> 
> Currently we warn at connector init for eDP type-C combo.
> 
> > +		return;
> > +
> > +	ret = intel_de_read(i915, PICA_PHY_CONFIG_CONTROL);
> > +	encoder->typec_supp = ret & EDP_ON_TYPEC;
> > +}
> > +
> >  static int
> >  intel_dp_detect(struct drm_connector *connector,
> >  		struct drm_modeset_acquire_ctx *ctx,
> > @@ -5595,6 +5609,8 @@ intel_dp_detect(struct drm_connector *connector,
> >  	if (!intel_display_driver_check_access(dev_priv))
> >  		return connector->status;
> >  
> > +	intel_dp_check_edp_typec_supp(encoder);
> > +
> 
> Isn't this something that should be determined at intel_ddi_init() time?
> 
> BR,
> Jani.
Please respond to Jani's comments
MattA
> 
> 
> >  	/* Can't disconnect eDP */
> >  	if (intel_dp_is_edp(intel_dp))
> >  		status = edp_detect(intel_dp);
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index da65500cd0c8..5f5a6ade5f8c 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -4583,4 +4583,7 @@ enum skl_power_gate {
> >  
> >  #define MTL_MEDIA_GSI_BASE		0x380000
> >  
> > +#define PICA_PHY_CONFIG_CONTROL 	_MMIO(0x16FE68)
> > +#define   EDP_ON_TYPEC			REG_BIT(31)
> > +
> >  #endif /* _I915_REG_H_ */
> 
> -- 
> Jani Nikula, Intel

RE: [PATCH 10/10] drm/i915/xe3lpd: Add powerdown value of eDP over type c

[Kandpal, Suraj]

> -----Original Message-----
> From: Atwood, Matthew S <matthew.s.atwood@intel.com>
> Sent: Thursday, October 10, 2024 4:35 AM
> To: Kandpal, Suraj <suraj.kandpal@intel.com>
> Cc: intel-xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; Kandpal,
> Suraj <suraj.kandpal@intel.com>
> Subject: Re: [PATCH 10/10] drm/i915/xe3lpd: Add powerdown value of eDP
> over type c
> 
> On Wed, Oct 09, 2024 at 10:57:03AM +0300, Jani Nikula wrote:
> > On Tue, 08 Oct 2024, Matt Atwood <matthew.s.atwood@intel.com> wrote:
> > > From: Suraj Kandpal <suraj.kandpal@intel.com>
> > >
> > > Add condition for P2.PG power down value.
> > >
> > > Bspec: 74494
> > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> > > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_cx0_phy.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > > index 1c8c2a2b05e1..3d95ee65a9f1 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > > @@ -3144,7 +3144,8 @@ static u8 cx0_power_control_disable_val(struct
> intel_encoder *encoder)
> > >  	if (intel_encoder_is_c10phy(encoder))
> > >  		return CX0_P2PG_STATE_DISABLE;
> > >
> > > -	if (IS_BATTLEMAGE(i915) && encoder->port == PORT_A)
> > > +	if ((IS_BATTLEMAGE(i915) && encoder->port == PORT_A) ||
> > > +	    (DISPLAY_VER(i915) >= 30 && encoder->type ==
> INTEL_OUTPUT_EDP))
> > >  		return CX0_P2PG_STATE_DISABLE;
> >
> > Does this match what the subject says?

True it should have been Add condition for EDP to powerdown P2.PG

Regards,
Suraj Kandpal
> Please address Jani's comments
> >
> > BR,
> > Jani.
> >
> > >
> > >  	return CX0_P4PG_STATE_DISABLE;
> >
> > --
> > Jani Nikula, Intel

RE: [PATCH 06/10] drm/i915/xe3lpd: Add macro to choose HDCP_LINE_REKEY bit

[Kandpal, Suraj]

> -----Original Message-----
> From: Roper, Matthew D <matthew.d.roper@intel.com>
> Sent: Wednesday, October 9, 2024 5:07 AM
> To: Atwood, Matthew S <matthew.s.atwood@intel.com>
> Cc: intel-xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; Kandpal,
> Suraj <suraj.kandpal@intel.com>
> Subject: Re: [PATCH 06/10] drm/i915/xe3lpd: Add macro to choose
> HDCP_LINE_REKEY bit
> 
> On Tue, Oct 08, 2024 at 03:37:37PM -0700, Matt Atwood wrote:
> > From: Suraj Kandpal <suraj.kandpal@intel.com>
> >
> > DISPLAY_VER() >= 30 has the HDCP_LINE_REKEY bit redefined from bit 12
> > to bit 14. Create a macro to choose the correct bit based
> 
> Typo?  The actual bit moved to 15, not 14.

Yes that is a typo it should have been bit 15

> 
> > on DISPLAY_VER().
> >
> > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_hdcp.c | 5 +++--
> >  drivers/gpu/drm/i915/i915_reg.h           | 2 +-
> >  2 files changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c
> > b/drivers/gpu/drm/i915/display/intel_hdcp.c
> > index ed6aa87403e2..e9b0414590ce 100644
> > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> > @@ -47,10 +47,11 @@ intel_hdcp_disable_hdcp_line_rekeying(struct
> intel_encoder *encoder,
> >  			intel_de_rmw(display, MTL_CHICKEN_TRANS(hdcp-
> >cpu_transcoder),
> >  				     0, HDCP_LINE_REKEY_DISABLE);
> >  		else if (IS_DISPLAY_VER_STEP(display, IP_VER(14, 1), STEP_B0,
> STEP_FOREVER) ||
> > -			 IS_DISPLAY_VER_STEP(display, IP_VER(20, 0),
> STEP_B0, STEP_FOREVER))
> > +			 IS_DISPLAY_VER_STEP(display, IP_VER(20, 0),
> STEP_B0, STEP_FOREVER) ||
> > +			 DISPLAY_VER(display) >= 30)
> 
> Is this correct?  The programming here is to account for Wa_16021352814
> which applies to MTL, LNL, and BMG (but gives different direction for
> different steppings of each platform).  The workaround does not apply to
> PTL, so is there something else in the spec indicating that we need to
> disable the rekeying?
> 
> Also, the commit message doesn't say anything about this change, only
> about the disable bit switching spots in the register.

Right that is the hblank restriction that comes for the following platoforms
MTL >= D0
BMG >=B0
LNL >= B0
And for all platforms of PTL
So the line rekeying needs to be done for the following and below that the hblank restriction needs to be done
So this code just ends up adding code to do line rekeying for the above platforms.
We can add the part that we need to add ptl to the platforms for which line rekeying needs to be done

Regards,
Suraj Kandpal
> 
> 
> Matt
> 
> >  			intel_de_rmw(display,
> >  				     TRANS_DDI_FUNC_CTL(display, hdcp-
> >cpu_transcoder),
> > -				     0,
> TRANS_DDI_HDCP_LINE_REKEY_DISABLE);
> > +				     0,
> TRANS_DDI_HDCP_LINE_REKEY_DISABLE(display));
> >  	}
> >  }
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h index d30459f8d1cb..da65500cd0c8
> > 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -3832,7 +3832,7 @@ enum skl_power_gate {
> >  #define  TRANS_DDI_EDP_INPUT_B_ONOFF	(5 << 12)
> >  #define  TRANS_DDI_EDP_INPUT_C_ONOFF	(6 << 12)
> >  #define  TRANS_DDI_EDP_INPUT_D_ONOFF	(7 << 12)
> > -#define  TRANS_DDI_HDCP_LINE_REKEY_DISABLE	REG_BIT(12)
> > +#define  TRANS_DDI_HDCP_LINE_REKEY_DISABLE(display)
> 	(DISPLAY_VER(display) >= 30 ? REG_BIT(15) : REG_BIT(12))
> >  #define  TRANS_DDI_MST_TRANSPORT_SELECT_MASK
> 	REG_GENMASK(11, 10)
> >  #define  TRANS_DDI_MST_TRANSPORT_SELECT(trans)	\
> >  	REG_FIELD_PREP(TRANS_DDI_MST_TRANSPORT_SELECT_MASK,
> trans)
> > --
> > 2.45.0
> >
> 
> --
> Matt Roper
> Graphics Software Engineer
> Linux GPU Platform Enablement
> Intel Corporation

RE: [PATCH 06/10] drm/i915/xe3lpd: Add macro to choose HDCP_LINE_REKEY bit

[Kandpal, Suraj]

> -----Original Message-----
> From: Jani Nikula <jani.nikula@linux.intel.com>
> Sent: Wednesday, October 9, 2024 1:09 PM
> To: Atwood, Matthew S <matthew.s.atwood@intel.com>; intel-
> xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org
> Cc: Kandpal, Suraj <suraj.kandpal@intel.com>; Atwood, Matthew S
> <matthew.s.atwood@intel.com>
> Subject: Re: [PATCH 06/10] drm/i915/xe3lpd: Add macro to choose
> HDCP_LINE_REKEY bit
> 
> On Tue, 08 Oct 2024, Matt Atwood <matthew.s.atwood@intel.com> wrote:
> > From: Suraj Kandpal <suraj.kandpal@intel.com>
> >
> > DISPLAY_VER() >= 30 has the HDCP_LINE_REKEY bit redefined from bit 12
> > to bit 14. Create a macro to choose the correct bit based on
> > DISPLAY_VER().
> >
> > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_hdcp.c | 5 +++--
> >  drivers/gpu/drm/i915/i915_reg.h           | 2 +-
> >  2 files changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c
> > b/drivers/gpu/drm/i915/display/intel_hdcp.c
> > index ed6aa87403e2..e9b0414590ce 100644
> > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
> > @@ -47,10 +47,11 @@ intel_hdcp_disable_hdcp_line_rekeying(struct
> intel_encoder *encoder,
> >  			intel_de_rmw(display, MTL_CHICKEN_TRANS(hdcp-
> >cpu_transcoder),
> >  				     0, HDCP_LINE_REKEY_DISABLE);
> >  		else if (IS_DISPLAY_VER_STEP(display, IP_VER(14, 1), STEP_B0,
> STEP_FOREVER) ||
> > -			 IS_DISPLAY_VER_STEP(display, IP_VER(20, 0),
> STEP_B0, STEP_FOREVER))
> > +			 IS_DISPLAY_VER_STEP(display, IP_VER(20, 0),
> STEP_B0, STEP_FOREVER) ||
> > +			 DISPLAY_VER(display) >= 30)
> >  			intel_de_rmw(display,
> >  				     TRANS_DDI_FUNC_CTL(display, hdcp-
> >cpu_transcoder),
> > -				     0,
> TRANS_DDI_HDCP_LINE_REKEY_DISABLE);
> > +				     0,
> TRANS_DDI_HDCP_LINE_REKEY_DISABLE(display));
> >  	}
> >  }
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h index d30459f8d1cb..da65500cd0c8
> > 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -3832,7 +3832,7 @@ enum skl_power_gate {
> >  #define  TRANS_DDI_EDP_INPUT_B_ONOFF	(5 << 12)
> >  #define  TRANS_DDI_EDP_INPUT_C_ONOFF	(6 << 12)
> >  #define  TRANS_DDI_EDP_INPUT_D_ONOFF	(7 << 12)
> > -#define  TRANS_DDI_HDCP_LINE_REKEY_DISABLE	REG_BIT(12)
> > +#define  TRANS_DDI_HDCP_LINE_REKEY_DISABLE(display)
> 	(DISPLAY_VER(display) >= 30 ? REG_BIT(15) : REG_BIT(12))
> 
> Do we really want to extend this style to individual bits?

I just thought this might be cleaner should we use 
TRANS_DDI_HDCP_LINE_REKEY_DISABLE
And 
XE3_TRANS_DDI_HDCP_LINE_REKEY_DISABLE instead then?

Regards,
Suraj Kandpal

> 
> BR,
> Jani.
> 
> >  #define  TRANS_DDI_MST_TRANSPORT_SELECT_MASK
> 	REG_GENMASK(11, 10)
> >  #define  TRANS_DDI_MST_TRANSPORT_SELECT(trans)	\
> >  	REG_FIELD_PREP(TRANS_DDI_MST_TRANSPORT_SELECT_MASK,
> trans)
> 
> --
> Jani Nikula, Intel

RE: [PATCH 09/10] drm/i915/xe3lpd: Add check to see if edp over type c is allowed

[Kandpal, Suraj]

> -----Original Message-----
> From: Atwood, Matthew S <matthew.s.atwood@intel.com>
> Sent: Thursday, October 10, 2024 4:36 AM
> To: Kandpal, Suraj <suraj.kandpal@intel.com>
> Cc: intel-xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; Kandpal,
> Suraj <suraj.kandpal@intel.com>
> Subject: Re: [PATCH 09/10] drm/i915/xe3lpd: Add check to see if edp over
> type c is allowed
> 
> On Wed, Oct 09, 2024 at 10:53:56AM +0300, Jani Nikula wrote:
> > On Tue, 08 Oct 2024, Matt Atwood <matthew.s.atwood@intel.com> wrote:
> > > From: Suraj Kandpal <suraj.kandpal@intel.com>
> > >
> > > Read PICA register to see if edp over type C is possible and then
> > > add the appropriate tables for it.
> >
> > There's clearly more to be done for the feature than this.

From what I could see in the spec we just need to read this the rest of the framework
Already seemed to be in place and removing the checks where we didn't allow edp to go ahead when
It was type c

> >
> > >
> > > Bspec: 68846
> > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> > > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_cx0_phy.c     |  2 ++
> > >  .../gpu/drm/i915/display/intel_display_types.h   |  1 +
> > >  drivers/gpu/drm/i915/display/intel_dp.c          | 16 ++++++++++++++++
> > >  drivers/gpu/drm/i915/i915_reg.h                  |  3 +++
> > >  4 files changed, 22 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > > b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > > index 0d6f75ae35f5..1c8c2a2b05e1 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > > @@ -2261,6 +2261,8 @@ intel_c20_pll_tables_get(struct intel_crtc_state
> *crtc_state,
> > >  		if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_EDP)) {
> > >  			if (DISPLAY_VER_FULL(i915) == IP_VER(14, 1))
> > >  				return xe2hpd_c20_edp_tables;
> > > +			if (DISPLAY_VER(i915) >= 30 && encoder-
> >typec_supp)
> > > +				return xe3lpd_c20_dp_edp_tables;
> > >  		}
> > >
> > >  		if (DISPLAY_VER_FULL(i915) == IP_VER(14, 1)) diff --git
> > > a/drivers/gpu/drm/i915/display/intel_display_types.h
> > > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > > index 2bb1fa64da2f..e9dc7707fbcd 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > > @@ -158,6 +158,7 @@ struct intel_encoder {
> > >  	enum port port;
> > >  	u16 cloneable;
> > >  	u8 pipe_mask;
> > > +	bool typec_supp;
> >
> > The register is global, why do we store this per encoder?

Do you think having this in drm_i915_private makes sense wanted to put it there originally

> >
> > Side not, please let's not abbreviate stuff like _supp for the sake of
> > abbreviating stuff.

Sure will fix the naming
Also quick question what would be the rule when abbreviating variables or
When would we want to abbreviate the a variable if we want to

> >
> > >
> > >  	/* Check and recover a bad link state. */
> > >  	struct delayed_work link_check_work; diff --git
> > > a/drivers/gpu/drm/i915/display/intel_dp.c
> > > b/drivers/gpu/drm/i915/display/intel_dp.c
> > > index fbb096be02ad..917a503cc43b 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > > @@ -5570,6 +5570,20 @@ intel_dp_detect_sdp_caps(struct intel_dp
> *intel_dp)
> > >  		drm_dp_as_sdp_supported(&intel_dp->aux, intel_dp-
> >dpcd);  }
> > >
> > > +static void
> > > +intel_dp_check_edp_typec_supp(struct intel_encoder *encoder)
> >
> > It's not about checking anything, it's about reading, right?

Yes will rename this to intel_dp_read_edp_typec_support

> >
> > > +{
> > > +	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> > > +	bool is_tc_port = intel_encoder_is_tc(encoder);
> > > +	u32 ret = 0;
> > > +
> > > +	if (encoder->type != INTEL_OUTPUT_EDP || !is_tc_port)
> >
> > Currently we warn at connector init for eDP type-C combo.

That's true we will need to remove that check for DISPLAY_VER > 20
Thanks will add that in this patch

> >
> > > +		return;
> > > +
> > > +	ret = intel_de_read(i915, PICA_PHY_CONFIG_CONTROL);
> > > +	encoder->typec_supp = ret & EDP_ON_TYPEC; }
> > > +
> > >  static int
> > >  intel_dp_detect(struct drm_connector *connector,
> > >  		struct drm_modeset_acquire_ctx *ctx, @@ -5595,6 +5609,8
> @@
> > > intel_dp_detect(struct drm_connector *connector,
> > >  	if (!intel_display_driver_check_access(dev_priv))
> > >  		return connector->status;
> > >
> > > +	intel_dp_check_edp_typec_supp(encoder);
> > > +
> >
> > Isn't this something that should be determined at intel_ddi_init() time?

Or intel_dp_connector_init can add it there what do you think ?

Regards,
Suraj Kandpal
> >
> > BR,
> > Jani.
> Please respond to Jani's comments
> MattA
> >
> >
> > >  	/* Can't disconnect eDP */
> > >  	if (intel_dp_is_edp(intel_dp))
> > >  		status = edp_detect(intel_dp);
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > b/drivers/gpu/drm/i915/i915_reg.h index da65500cd0c8..5f5a6ade5f8c
> > > 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -4583,4 +4583,7 @@ enum skl_power_gate {
> > >
> > >  #define MTL_MEDIA_GSI_BASE		0x380000
> > >
> > > +#define PICA_PHY_CONFIG_CONTROL 	_MMIO(0x16FE68)
> > > +#define   EDP_ON_TYPEC			REG_BIT(31)
> > > +
> > >  #endif /* _I915_REG_H_ */
> >
> > --
> > Jani Nikula, Intel

RE: [PATCH 06/10] drm/i915/xe3lpd: Add macro to choose HDCP_LINE_REKEY bit

[Jani Nikula]

On Thu, 10 Oct 2024, "Kandpal, Suraj" <suraj.kandpal@intel.com> wrote:
>> -----Original Message-----
>> From: Jani Nikula <jani.nikula@linux.intel.com>
>> Sent: Wednesday, October 9, 2024 1:09 PM
>> To: Atwood, Matthew S <matthew.s.atwood@intel.com>; intel-
>> xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org
>> Cc: Kandpal, Suraj <suraj.kandpal@intel.com>; Atwood, Matthew S
>> <matthew.s.atwood@intel.com>
>> Subject: Re: [PATCH 06/10] drm/i915/xe3lpd: Add macro to choose
>> HDCP_LINE_REKEY bit
>> 
>> On Tue, 08 Oct 2024, Matt Atwood <matthew.s.atwood@intel.com> wrote:
>> > From: Suraj Kandpal <suraj.kandpal@intel.com>
>> >
>> > DISPLAY_VER() >= 30 has the HDCP_LINE_REKEY bit redefined from bit 12
>> > to bit 14. Create a macro to choose the correct bit based on
>> > DISPLAY_VER().
>> >
>> > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
>> > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/display/intel_hdcp.c | 5 +++--
>> >  drivers/gpu/drm/i915/i915_reg.h           | 2 +-
>> >  2 files changed, 4 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c
>> > b/drivers/gpu/drm/i915/display/intel_hdcp.c
>> > index ed6aa87403e2..e9b0414590ce 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c
>> > @@ -47,10 +47,11 @@ intel_hdcp_disable_hdcp_line_rekeying(struct
>> intel_encoder *encoder,
>> >  			intel_de_rmw(display, MTL_CHICKEN_TRANS(hdcp-
>> >cpu_transcoder),
>> >  				     0, HDCP_LINE_REKEY_DISABLE);
>> >  		else if (IS_DISPLAY_VER_STEP(display, IP_VER(14, 1), STEP_B0,
>> STEP_FOREVER) ||
>> > -			 IS_DISPLAY_VER_STEP(display, IP_VER(20, 0),
>> STEP_B0, STEP_FOREVER))
>> > +			 IS_DISPLAY_VER_STEP(display, IP_VER(20, 0),
>> STEP_B0, STEP_FOREVER) ||
>> > +			 DISPLAY_VER(display) >= 30)
>> >  			intel_de_rmw(display,
>> >  				     TRANS_DDI_FUNC_CTL(display, hdcp-
>> >cpu_transcoder),
>> > -				     0,
>> TRANS_DDI_HDCP_LINE_REKEY_DISABLE);
>> > +				     0,
>> TRANS_DDI_HDCP_LINE_REKEY_DISABLE(display));
>> >  	}
>> >  }
>> >
>> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
>> > b/drivers/gpu/drm/i915/i915_reg.h index d30459f8d1cb..da65500cd0c8
>> > 100644
>> > --- a/drivers/gpu/drm/i915/i915_reg.h
>> > +++ b/drivers/gpu/drm/i915/i915_reg.h
>> > @@ -3832,7 +3832,7 @@ enum skl_power_gate {
>> >  #define  TRANS_DDI_EDP_INPUT_B_ONOFF	(5 << 12)
>> >  #define  TRANS_DDI_EDP_INPUT_C_ONOFF	(6 << 12)
>> >  #define  TRANS_DDI_EDP_INPUT_D_ONOFF	(7 << 12)
>> > -#define  TRANS_DDI_HDCP_LINE_REKEY_DISABLE	REG_BIT(12)
>> > +#define  TRANS_DDI_HDCP_LINE_REKEY_DISABLE(display)
>> 	(DISPLAY_VER(display) >= 30 ? REG_BIT(15) : REG_BIT(12))
>> 
>> Do we really want to extend this style to individual bits?
>
> I just thought this might be cleaner should we use 
> TRANS_DDI_HDCP_LINE_REKEY_DISABLE
> And 
> XE3_TRANS_DDI_HDCP_LINE_REKEY_DISABLE instead then?

I think so yes.

If it becomes too ugly in code, at the very least define the bits
separately instead of inline in the ternary operator.

BR,
Jani.


>
> Regards,
> Suraj Kandpal
>
>> 
>> BR,
>> Jani.
>> 
>> >  #define  TRANS_DDI_MST_TRANSPORT_SELECT_MASK
>> 	REG_GENMASK(11, 10)
>> >  #define  TRANS_DDI_MST_TRANSPORT_SELECT(trans)	\
>> >  	REG_FIELD_PREP(TRANS_DDI_MST_TRANSPORT_SELECT_MASK,
>> trans)
>> 
>> --
>> Jani Nikula, Intel

-- 
Jani Nikula, Intel

RE: [PATCH 09/10] drm/i915/xe3lpd: Add check to see if edp over type c is allowed

[Jani Nikula]

On Thu, 10 Oct 2024, "Kandpal, Suraj" <suraj.kandpal@intel.com> wrote:
>> -----Original Message-----
>> From: Atwood, Matthew S <matthew.s.atwood@intel.com>
>> Sent: Thursday, October 10, 2024 4:36 AM
>> To: Kandpal, Suraj <suraj.kandpal@intel.com>
>> Cc: intel-xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; Kandpal,
>> Suraj <suraj.kandpal@intel.com>
>> Subject: Re: [PATCH 09/10] drm/i915/xe3lpd: Add check to see if edp over
>> type c is allowed
>> 
>> On Wed, Oct 09, 2024 at 10:53:56AM +0300, Jani Nikula wrote:
>> > On Tue, 08 Oct 2024, Matt Atwood <matthew.s.atwood@intel.com> wrote:
>> > > From: Suraj Kandpal <suraj.kandpal@intel.com>
>> > >
>> > > Read PICA register to see if edp over type C is possible and then
>> > > add the appropriate tables for it.
>> >
>> > There's clearly more to be done for the feature than this.
>
> From what I could see in the spec we just need to read this the rest of the framework
> Already seemed to be in place and removing the checks where we didn't allow edp to go ahead when
> It was type c

Is it driven by VBT?

>
>> >
>> > >
>> > > Bspec: 68846
>> > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
>> > > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
>> > > ---
>> > >  drivers/gpu/drm/i915/display/intel_cx0_phy.c     |  2 ++
>> > >  .../gpu/drm/i915/display/intel_display_types.h   |  1 +
>> > >  drivers/gpu/drm/i915/display/intel_dp.c          | 16 ++++++++++++++++
>> > >  drivers/gpu/drm/i915/i915_reg.h                  |  3 +++
>> > >  4 files changed, 22 insertions(+)
>> > >
>> > > diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
>> > > b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
>> > > index 0d6f75ae35f5..1c8c2a2b05e1 100644
>> > > --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
>> > > +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
>> > > @@ -2261,6 +2261,8 @@ intel_c20_pll_tables_get(struct intel_crtc_state
>> *crtc_state,
>> > >  		if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_EDP)) {
>> > >  			if (DISPLAY_VER_FULL(i915) == IP_VER(14, 1))
>> > >  				return xe2hpd_c20_edp_tables;
>> > > +			if (DISPLAY_VER(i915) >= 30 && encoder-
>> >typec_supp)
>> > > +				return xe3lpd_c20_dp_edp_tables;
>> > >  		}
>> > >
>> > >  		if (DISPLAY_VER_FULL(i915) == IP_VER(14, 1)) diff --git
>> > > a/drivers/gpu/drm/i915/display/intel_display_types.h
>> > > b/drivers/gpu/drm/i915/display/intel_display_types.h
>> > > index 2bb1fa64da2f..e9dc7707fbcd 100644
>> > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
>> > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
>> > > @@ -158,6 +158,7 @@ struct intel_encoder {
>> > >  	enum port port;
>> > >  	u16 cloneable;
>> > >  	u8 pipe_mask;
>> > > +	bool typec_supp;
>> >
>> > The register is global, why do we store this per encoder?
>
> Do you think having this in drm_i915_private makes sense wanted to put it there originally

Ugh no. We've stopped putting *anything* in drm_i915_private.

I couldn't find much detail about how the register behaves, but it looks
like a strap. I think I'd put the info in struct
intel_display_runtime_info and initialize in
__intel_display_device_info_runtime_init() based on the register,
similar to a ton of other things there.

>
>> >
>> > Side not, please let's not abbreviate stuff like _supp for the sake of
>> > abbreviating stuff.
>
> Sure will fix the naming
> Also quick question what would be the rule when abbreviating variables or
> When would we want to abbreviate the a variable if we want to

It's all about conventions, scope, and context. Certain things always
have the same name. Shorter names are fine in tight scope. Context can
allow you to shorten the name if parts of it are obvious from context.

>
>> >
>> > >
>> > >  	/* Check and recover a bad link state. */
>> > >  	struct delayed_work link_check_work; diff --git
>> > > a/drivers/gpu/drm/i915/display/intel_dp.c
>> > > b/drivers/gpu/drm/i915/display/intel_dp.c
>> > > index fbb096be02ad..917a503cc43b 100644
>> > > --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> > > @@ -5570,6 +5570,20 @@ intel_dp_detect_sdp_caps(struct intel_dp
>> *intel_dp)
>> > >  		drm_dp_as_sdp_supported(&intel_dp->aux, intel_dp-
>> >dpcd);  }
>> > >
>> > > +static void
>> > > +intel_dp_check_edp_typec_supp(struct intel_encoder *encoder)
>> >
>> > It's not about checking anything, it's about reading, right?
>
> Yes will rename this to intel_dp_read_edp_typec_support

If we move the check to runtime info, the function shouldn't be needed.

>
>> >
>> > > +{
>> > > +	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
>> > > +	bool is_tc_port = intel_encoder_is_tc(encoder);
>> > > +	u32 ret = 0;
>> > > +
>> > > +	if (encoder->type != INTEL_OUTPUT_EDP || !is_tc_port)
>> >
>> > Currently we warn at connector init for eDP type-C combo.
>
> That's true we will need to remove that check for DISPLAY_VER > 20
> Thanks will add that in this patch

That check should be amended with the runtime info check.

>
>> >
>> > > +		return;
>> > > +
>> > > +	ret = intel_de_read(i915, PICA_PHY_CONFIG_CONTROL);
>> > > +	encoder->typec_supp = ret & EDP_ON_TYPEC; }
>> > > +
>> > >  static int
>> > >  intel_dp_detect(struct drm_connector *connector,
>> > >  		struct drm_modeset_acquire_ctx *ctx, @@ -5595,6 +5609,8
>> @@
>> > > intel_dp_detect(struct drm_connector *connector,
>> > >  	if (!intel_display_driver_check_access(dev_priv))
>> > >  		return connector->status;
>> > >
>> > > +	intel_dp_check_edp_typec_supp(encoder);
>> > > +
>> >
>> > Isn't this something that should be determined at intel_ddi_init() time?
>
> Or intel_dp_connector_init can add it there what do you think ?

Yes, that's where we check the type-C/eDP combo currently.

BR,
Jani.

>
> Regards,
> Suraj Kandpal
>> >
>> > BR,
>> > Jani.
>> Please respond to Jani's comments
>> MattA
>> >
>> >
>> > >  	/* Can't disconnect eDP */
>> > >  	if (intel_dp_is_edp(intel_dp))
>> > >  		status = edp_detect(intel_dp);
>> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
>> > > b/drivers/gpu/drm/i915/i915_reg.h index da65500cd0c8..5f5a6ade5f8c
>> > > 100644
>> > > --- a/drivers/gpu/drm/i915/i915_reg.h
>> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
>> > > @@ -4583,4 +4583,7 @@ enum skl_power_gate {
>> > >
>> > >  #define MTL_MEDIA_GSI_BASE		0x380000
>> > >
>> > > +#define PICA_PHY_CONFIG_CONTROL 	_MMIO(0x16FE68)
>> > > +#define   EDP_ON_TYPEC			REG_BIT(31)
>> > > +
>> > >  #endif /* _I915_REG_H_ */
>> >
>> > --
>> > Jani Nikula, Intel

-- 
Jani Nikula, Intel