[PATCH 00/10] drm/i915/dp_mst: Add support for fractional link bpps

[PATCH 00/10] drm/i915/dp_mst: Add support for fractional link bpps

[Imre Deak]

This patchset adds support for fractional link bpps on MST links and
adds a way to verify this functionality via a connector debugfs entry
forcing the link bpp to a given value.

Imre Deak (10):
  drm/i915/dp: Use the correct connector while computing the link BPP
    limit on MST
  drm/i915/dp: Limit max link bpp properly to a fractional value on SST
  drm/i915/dp_mst: Add support for fractional link bpps on MST
  drm/i915/dp_mst: Enable fractional link bpps on MST
  drm/i915/display: Factor out intel_display_{min,max}_pipe_bpp()
  drm/i915/dp: Export intel_dp_dsc_min_src_compressed_bpp()
  drm/i915/dp: Use an intel_connector pointer everywhere
  drm/i915/hdmi: Use an intel_connector pointer everywhere
  drm/i915: Add support for forcing the link bpp on a connector
  drm/i915: Add force link bpp debugfs entry to connectors

 drivers/gpu/drm/i915/display/intel_crt.c      |  20 +-
 drivers/gpu/drm/i915/display/intel_display.c  |  28 ++-
 drivers/gpu/drm/i915/display/intel_display.h  |   3 +
 .../drm/i915/display/intel_display_device.h   |   1 +
 .../drm/i915/display/intel_display_types.h    |   4 +
 drivers/gpu/drm/i915/display/intel_dp.c       | 180 ++++++++--------
 drivers/gpu/drm/i915/display/intel_dp.h       |   6 +-
 drivers/gpu/drm/i915/display/intel_dp_mst.c   |  33 ++-
 drivers/gpu/drm/i915/display/intel_hdmi.c     | 195 +++++++++--------
 drivers/gpu/drm/i915/display/intel_link_bw.c  | 204 +++++++++++++++++-
 drivers/gpu/drm/i915/display/intel_link_bw.h  |   2 +
 drivers/gpu/drm/i915/display/intel_lvds.c     |  20 +-
 drivers/gpu/drm/i915/display/intel_sdvo.c     |  20 +-
 13 files changed, 508 insertions(+), 208 deletions(-)

-- 
2.44.2

[PATCH 01/10] drm/i915/dp: Use the correct connector while computing the link BPP limit on MST

[Imre Deak]

Atm, on an MST link in DSC mode
intel_dp_compute_config_link_bpp_limits() calculates the maximum link
bpp limit using the MST root connector's DSC capabilities. That's not
correct in general: the decompression could be performed by a branch
device downstream of the root branch device or the sink itself.

Fix the above by passing to intel_dp_compute_config_link_bpp_limits()
the actual connector being modeset, containing the correct DSC
capabilities.

Cc: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Fixes: 1c5b72daff46 ("drm/i915/dp: Set the DSC link limits in intel_dp_compute_config_link_bpp_limits")
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c     | 7 ++++---
 drivers/gpu/drm/i915/display/intel_dp.h     | 1 +
 drivers/gpu/drm/i915/display/intel_dp_mst.c | 5 +++--
 3 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index aeb14a5455fd1..713cc6e65faac 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -2522,6 +2522,7 @@ intel_dp_dsc_compute_pipe_bpp_limits(struct intel_dp *intel_dp,
 
 bool
 intel_dp_compute_config_limits(struct intel_dp *intel_dp,
+			       struct intel_connector *connector,
 			       struct intel_crtc_state *crtc_state,
 			       bool respect_downstream_limits,
 			       bool dsc,
@@ -2575,7 +2576,7 @@ intel_dp_compute_config_limits(struct intel_dp *intel_dp,
 	intel_dp_test_compute_config(intel_dp, crtc_state, limits);
 
 	return intel_dp_compute_config_link_bpp_limits(intel_dp,
-						       intel_dp->attached_connector,
+						       connector,
 						       crtc_state,
 						       dsc,
 						       limits);
@@ -2636,7 +2637,7 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
 	joiner_needs_dsc = intel_dp_joiner_needs_dsc(display, num_joined_pipes);
 
 	dsc_needed = joiner_needs_dsc || intel_dp->force_dsc_en ||
-		     !intel_dp_compute_config_limits(intel_dp, pipe_config,
+		     !intel_dp_compute_config_limits(intel_dp, connector, pipe_config,
 						     respect_downstream_limits,
 						     false,
 						     &limits);
@@ -2670,7 +2671,7 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
 			    str_yes_no(ret), str_yes_no(joiner_needs_dsc),
 			    str_yes_no(intel_dp->force_dsc_en));
 
-		if (!intel_dp_compute_config_limits(intel_dp, pipe_config,
+		if (!intel_dp_compute_config_limits(intel_dp, connector, pipe_config,
 						    respect_downstream_limits,
 						    true,
 						    &limits))
diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
index 9189db4c25946..98f90955fdb1d 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.h
+++ b/drivers/gpu/drm/i915/display/intel_dp.h
@@ -194,6 +194,7 @@ void intel_dp_wait_source_oui(struct intel_dp *intel_dp);
 int intel_dp_output_bpp(enum intel_output_format output_format, int bpp);
 
 bool intel_dp_compute_config_limits(struct intel_dp *intel_dp,
+				    struct intel_connector *connector,
 				    struct intel_crtc_state *crtc_state,
 				    bool respect_downstream_limits,
 				    bool dsc,
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 4c15dcb103aa2..cde662daed5a4 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -619,12 +619,13 @@ adjust_limits_for_dsc_hblank_expansion_quirk(struct intel_dp *intel_dp,
 
 static bool
 mst_stream_compute_config_limits(struct intel_dp *intel_dp,
-				 const struct intel_connector *connector,
+				 struct intel_connector *connector,
 				 struct intel_crtc_state *crtc_state,
 				 bool dsc,
 				 struct link_config_limits *limits)
 {
-	if (!intel_dp_compute_config_limits(intel_dp, crtc_state, false, dsc,
+	if (!intel_dp_compute_config_limits(intel_dp, connector,
+					    crtc_state, false, dsc,
 					    limits))
 		return false;
 
-- 
2.44.2

[PATCH 02/10] drm/i915/dp: Limit max link bpp properly to a fractional value on SST

[Imre Deak]

The maximum link bpp - determined by the link BW for instance - can be
fractional, handle this properly during computing the link bpp on SST.

This keeps the pipe joiner specific maximum link bpp as a rounded-down
integer value still, changing that to a fractional value is left for
later.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 713cc6e65faac..14c3d5e10a430 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -938,6 +938,7 @@ static u32 ultrajoiner_ram_max_bpp(u32 mode_hdisplay)
 	return ultrajoiner_ram_bits() / mode_hdisplay;
 }
 
+/* TODO: return a bpp_x16 value */
 static
 u32 get_max_compressed_bpp_with_joiner(struct intel_display *display,
 				       u32 mode_clock, u32 mode_hdisplay,
@@ -2148,24 +2149,16 @@ static int dsc_compute_compressed_bpp(struct intel_dp *intel_dp,
 	const struct intel_connector *connector = to_intel_connector(conn_state->connector);
 	const struct drm_display_mode *adjusted_mode = &pipe_config->hw.adjusted_mode;
 	int output_bpp;
-	int dsc_min_bpp;
-	int dsc_max_bpp;
-	int min_bpp_x16, max_bpp_x16, bpp_step_x16;
+	int max_bpp_x16, bpp_step_x16;
 	int dsc_joiner_max_bpp;
 	int num_joined_pipes = intel_crtc_num_joined_pipes(pipe_config);
 	int bpp_x16;
 	int ret;
 
-	dsc_min_bpp = fxp_q4_to_int_roundup(limits->link.min_bpp_x16);
-
 	dsc_joiner_max_bpp = get_max_compressed_bpp_with_joiner(display, adjusted_mode->clock,
 								adjusted_mode->hdisplay,
 								num_joined_pipes);
-	dsc_max_bpp = min(dsc_joiner_max_bpp, fxp_q4_to_int(limits->link.max_bpp_x16));
-
-	/* FIXME: remove the round trip via integers */
-	min_bpp_x16 = fxp_q4_from_int(dsc_min_bpp);
-	max_bpp_x16 = fxp_q4_from_int(dsc_max_bpp);
+	max_bpp_x16 = min(fxp_q4_from_int(dsc_joiner_max_bpp), limits->link.max_bpp_x16);
 
 	bpp_step_x16 = intel_dp_dsc_bpp_step_x16(connector);
 
@@ -2173,7 +2166,7 @@ static int dsc_compute_compressed_bpp(struct intel_dp *intel_dp,
 	output_bpp = intel_dp_output_bpp(pipe_config->output_format, pipe_bpp);
 	max_bpp_x16 = min(max_bpp_x16, fxp_q4_from_int(output_bpp) - bpp_step_x16);
 
-	for (bpp_x16 = max_bpp_x16; bpp_x16 >= min_bpp_x16; bpp_x16 -= bpp_step_x16) {
+	for (bpp_x16 = max_bpp_x16; bpp_x16 >= limits->link.min_bpp_x16; bpp_x16 -= bpp_step_x16) {
 		if (!intel_dp_dsc_valid_bpp(intel_dp, bpp_x16))
 			continue;
 
-- 
2.44.2

[PATCH 03/10] drm/i915/dp_mst: Add support for fractional link bpps on MST

[Imre Deak]

Add support for a fractional link bpp on an MST link. Leave enabling a
fractional bpp to a follow-up change.

This keeps the mode validation specific maximum link bpp as a
rounded-down integer value still, changing that to a fractional value is
left for later.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c     | 39 ++++++++++++---------
 drivers/gpu/drm/i915/display/intel_dp.h     |  2 +-
 drivers/gpu/drm/i915/display/intel_dp_mst.c | 23 +++++-------
 3 files changed, 32 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 14c3d5e10a430..2944ae0d53e32 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -846,21 +846,22 @@ small_joiner_ram_size_bits(struct intel_display *display)
 		return 6144 * 8;
 }
 
-u32 intel_dp_dsc_nearest_valid_bpp(struct intel_display *display, u32 bpp, u32 pipe_bpp)
+int intel_dp_dsc_nearest_valid_bpp(struct intel_display *display, int link_bpp_x16, int pipe_bpp)
 {
-	u32 bits_per_pixel = bpp;
 	int i;
 
 	/* Error out if the max bpp is less than smallest allowed valid bpp */
-	if (bits_per_pixel < valid_dsc_bpp[0]) {
-		drm_dbg_kms(display->drm, "Unsupported BPP %u, min %u\n",
-			    bits_per_pixel, valid_dsc_bpp[0]);
+	if (link_bpp_x16 < fxp_q4_from_int(valid_dsc_bpp[0])) {
+		drm_dbg_kms(display->drm,
+			    "Unsupported BPP " FXP_Q4_FMT ", min " FXP_Q4_FMT "\n",
+			    FXP_Q4_ARGS(link_bpp_x16),
+			    FXP_Q4_ARGS(fxp_q4_from_int(valid_dsc_bpp[0])));
 		return 0;
 	}
 
 	/* From XE_LPD onwards we support from bpc upto uncompressed bpp-1 BPPs */
 	if (DISPLAY_VER(display) >= 13) {
-		bits_per_pixel = min(bits_per_pixel, pipe_bpp - 1);
+		link_bpp_x16 = min(link_bpp_x16, fxp_q4_from_int(pipe_bpp - 1));
 
 		/*
 		 * According to BSpec, 27 is the max DSC output bpp,
@@ -870,26 +871,28 @@ u32 intel_dp_dsc_nearest_valid_bpp(struct intel_display *display, u32 bpp, u32 p
 		 * that and probably means we can't fit the required mode, even with
 		 * DSC enabled.
 		 */
-		if (bits_per_pixel < 8) {
+		if (link_bpp_x16 < fxp_q4_from_int(8)) {
 			drm_dbg_kms(display->drm,
-				    "Unsupported BPP %u, min 8\n",
-				    bits_per_pixel);
+				    "Unsupported BPP " FXP_Q4_FMT ", min " FXP_Q4_FMT "\n",
+				    FXP_Q4_ARGS(link_bpp_x16), FXP_Q4_ARGS(fxp_q4_from_int(8)));
 			return 0;
 		}
-		bits_per_pixel = min_t(u32, bits_per_pixel, 27);
+		link_bpp_x16 = min(link_bpp_x16, fxp_q4_from_int(27));
 	} else {
 		/* Find the nearest match in the array of known BPPs from VESA */
 		for (i = 0; i < ARRAY_SIZE(valid_dsc_bpp) - 1; i++) {
-			if (bits_per_pixel < valid_dsc_bpp[i + 1])
+			if (link_bpp_x16 < fxp_q4_from_int(valid_dsc_bpp[i + 1]))
 				break;
 		}
-		drm_dbg_kms(display->drm, "Set dsc bpp from %d to VESA %d\n",
-			    bits_per_pixel, valid_dsc_bpp[i]);
+		drm_dbg_kms(display->drm,
+			    "Set dsc bpp from " FXP_Q4_FMT " to VESA " FXP_Q4_FMT "\n",
+			    FXP_Q4_ARGS(link_bpp_x16),
+			    FXP_Q4_ARGS(fxp_q4_from_int(valid_dsc_bpp[i])));
 
-		bits_per_pixel = valid_dsc_bpp[i];
+		link_bpp_x16 = fxp_q4_from_int(valid_dsc_bpp[i]);
 	}
 
-	return bits_per_pixel;
+	return link_bpp_x16;
 }
 
 static int bigjoiner_interface_bits(struct intel_display *display)
@@ -955,6 +958,7 @@ u32 get_max_compressed_bpp_with_joiner(struct intel_display *display,
 	return max_bpp;
 }
 
+/* TODO: return a bpp_x16 value */
 u16 intel_dp_dsc_get_max_compressed_bpp(struct intel_display *display,
 					u32 link_clock, u32 lane_count,
 					u32 mode_clock, u32 mode_hdisplay,
@@ -1007,9 +1011,10 @@ u16 intel_dp_dsc_get_max_compressed_bpp(struct intel_display *display,
 							    mode_hdisplay, num_joined_pipes);
 	bits_per_pixel = min(bits_per_pixel, joiner_max_bpp);
 
-	bits_per_pixel = intel_dp_dsc_nearest_valid_bpp(display, bits_per_pixel, pipe_bpp);
+	bits_per_pixel = intel_dp_dsc_nearest_valid_bpp(display,
+							fxp_q4_from_int(bits_per_pixel), pipe_bpp);
 
-	return bits_per_pixel;
+	return fxp_q4_to_int(bits_per_pixel);
 }
 
 u8 intel_dp_dsc_get_slice_count(const struct intel_connector *connector,
diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
index 98f90955fdb1d..f3ca6966abe00 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.h
+++ b/drivers/gpu/drm/i915/display/intel_dp.h
@@ -173,7 +173,7 @@ bool intel_dp_supports_dsc(struct intel_dp *intel_dp,
 			   const struct intel_connector *connector,
 			   const struct intel_crtc_state *crtc_state);
 
-u32 intel_dp_dsc_nearest_valid_bpp(struct intel_display *display, u32 bpp, u32 pipe_bpp);
+int intel_dp_dsc_nearest_valid_bpp(struct intel_display *display, int link_bpp_x16, int pipe_bpp);
 
 void intel_ddi_update_pipe(struct intel_atomic_state *state,
 			   struct intel_encoder *encoder,
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index cde662daed5a4..4e607c0853cbd 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -457,7 +457,6 @@ static int mst_stream_dsc_compute_link_config(struct intel_dp *intel_dp,
 	int num_bpc;
 	u8 dsc_bpc[3] = {};
 	int min_bpp, max_bpp, sink_min_bpp, sink_max_bpp;
-	int min_compressed_bpp, max_compressed_bpp;
 
 	max_bpp = limits->pipe.max_bpp;
 	min_bpp = limits->pipe.min_bpp;
@@ -482,24 +481,20 @@ static int mst_stream_dsc_compute_link_config(struct intel_dp *intel_dp,
 
 	crtc_state->pipe_bpp = max_bpp;
 
-	max_compressed_bpp = fxp_q4_to_int(limits->link.max_bpp_x16);
-	min_compressed_bpp = fxp_q4_to_int_roundup(limits->link.min_bpp_x16);
-
-	drm_dbg_kms(display->drm, "DSC Sink supported compressed min bpp %d compressed max bpp %d\n",
-		    min_compressed_bpp, max_compressed_bpp);
-
-	/* Align compressed bpps according to our own constraints */
-	max_compressed_bpp = intel_dp_dsc_nearest_valid_bpp(display, max_compressed_bpp,
-							    crtc_state->pipe_bpp);
-	min_compressed_bpp = intel_dp_dsc_nearest_valid_bpp(display, min_compressed_bpp,
-							    crtc_state->pipe_bpp);
+	drm_dbg_kms(display->drm,
+		    "DSC Sink supported compressed min bpp " FXP_Q4_FMT " compressed max bpp " FXP_Q4_FMT "\n",
+		    FXP_Q4_ARGS(limits->link.min_bpp_x16), FXP_Q4_ARGS(limits->link.max_bpp_x16));
 
 	crtc_state->lane_count = limits->max_lane_count;
 	crtc_state->port_clock = limits->max_rate;
 
 	return intel_dp_mtp_tu_compute_config(intel_dp, crtc_state, conn_state,
-					      fxp_q4_from_int(min_compressed_bpp),
-					      fxp_q4_from_int(max_compressed_bpp),
+					      intel_dp_dsc_nearest_valid_bpp(display,
+									     limits->link.min_bpp_x16,
+									     crtc_state->pipe_bpp),
+					      intel_dp_dsc_nearest_valid_bpp(display,
+									     limits->link.max_bpp_x16,
+									     crtc_state->pipe_bpp),
 					      fxp_q4_from_int(1), true);
 }
 
-- 
2.44.2

[PATCH 04/10] drm/i915/dp_mst: Enable fractional link bpps on MST

[Imre Deak]

Enable using a fractional (compressed) link bpp on an MST link, if this
is supported.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c     | 2 +-
 drivers/gpu/drm/i915/display/intel_dp.h     | 2 ++
 drivers/gpu/drm/i915/display/intel_dp_mst.c | 3 ++-
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 2944ae0d53e32..3cf8d6510bf90 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -2103,7 +2103,7 @@ static int dsc_src_max_compressed_bpp(struct intel_dp *intel_dp)
 /*
  * Note: for pre-13 display you still need to check the validity of each step.
  */
-static int intel_dp_dsc_bpp_step_x16(const struct intel_connector *connector)
+int intel_dp_dsc_bpp_step_x16(const struct intel_connector *connector)
 {
 	struct intel_display *display = to_intel_display(connector);
 	u8 incr = drm_dp_dsc_sink_bpp_incr(connector->dp.dsc_dpcd);
diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
index f3ca6966abe00..c974e4b97338a 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.h
+++ b/drivers/gpu/drm/i915/display/intel_dp.h
@@ -210,4 +210,6 @@ bool intel_dp_has_connector(struct intel_dp *intel_dp,
 int intel_dp_dsc_max_src_input_bpc(struct intel_display *display);
 int intel_dp_dsc_min_src_input_bpc(void);
 
+int intel_dp_dsc_bpp_step_x16(const struct intel_connector *connector);
+
 #endif /* __INTEL_DP_H__ */
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 4e607c0853cbd..35214d9a8c781 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -495,7 +495,8 @@ static int mst_stream_dsc_compute_link_config(struct intel_dp *intel_dp,
 					      intel_dp_dsc_nearest_valid_bpp(display,
 									     limits->link.max_bpp_x16,
 									     crtc_state->pipe_bpp),
-					      fxp_q4_from_int(1), true);
+					      intel_dp_dsc_bpp_step_x16(connector),
+					      true);
 }
 
 static int mst_stream_update_slots(struct intel_dp *intel_dp,
-- 
2.44.2

[PATCH 05/10] drm/i915/display: Factor out intel_display_{min, max}_pipe_bpp()

[Imre Deak]

Factor out helpers that can be used in a follow-up change to query the
minimum and maximum pipe bpp supported by the HW.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 28 +++++++++++++-------
 drivers/gpu/drm/i915/display/intel_display.h |  3 +++
 drivers/gpu/drm/i915/display/intel_dp.c      |  2 +-
 3 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 16fd7c00ba010..621cc581cf51a 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -4317,6 +4317,22 @@ compute_sink_pipe_bpp(const struct drm_connector_state *conn_state,
 	return 0;
 }
 
+int intel_display_min_pipe_bpp(void)
+{
+	return 6 * 3;
+}
+
+int intel_display_max_pipe_bpp(struct intel_display *display)
+{
+	if (display->platform.g4x || display->platform.valleyview ||
+	    display->platform.cherryview)
+		return 10*3;
+	else if (DISPLAY_VER(display) >= 5)
+		return 12*3;
+	else
+		return 8*3;
+}
+
 static int
 compute_baseline_pipe_bpp(struct intel_atomic_state *state,
 			  struct intel_crtc *crtc)
@@ -4326,17 +4342,9 @@ compute_baseline_pipe_bpp(struct intel_atomic_state *state,
 		intel_atomic_get_new_crtc_state(state, crtc);
 	struct drm_connector *connector;
 	struct drm_connector_state *connector_state;
-	int bpp, i;
+	int i;
 
-	if (display->platform.g4x || display->platform.valleyview ||
-	    display->platform.cherryview)
-		bpp = 10*3;
-	else if (DISPLAY_VER(display) >= 5)
-		bpp = 12*3;
-	else
-		bpp = 8*3;
-
-	crtc_state->pipe_bpp = bpp;
+	crtc_state->pipe_bpp = intel_display_max_pipe_bpp(display);
 
 	/* Clamp display bpp to connector max bpp */
 	for_each_new_connector_in_state(&state->base, connector, connector_state, i) {
diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
index 3b54a62c290af..b6610e9175a7a 100644
--- a/drivers/gpu/drm/i915/display/intel_display.h
+++ b/drivers/gpu/drm/i915/display/intel_display.h
@@ -524,6 +524,9 @@ void intel_plane_fixup_bitmasks(struct intel_crtc_state *crtc_state);
 bool intel_crtc_vrr_disabling(struct intel_atomic_state *state,
 			      struct intel_crtc *crtc);
 
+int intel_display_min_pipe_bpp(void);
+int intel_display_max_pipe_bpp(struct intel_display *display);
+
 /* modesetting */
 int intel_modeset_pipes_in_mask_early(struct intel_atomic_state *state,
 				      const char *reason, u8 pipe_mask);
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 3cf8d6510bf90..881852390bb81 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1199,7 +1199,7 @@ intel_dp_output_format(struct intel_connector *connector,
 int intel_dp_min_bpp(enum intel_output_format output_format)
 {
 	if (output_format == INTEL_OUTPUT_FORMAT_RGB)
-		return 6 * 3;
+		return intel_display_min_pipe_bpp();
 	else
 		return 8 * 3;
 }
-- 
2.44.2

[PATCH 06/10] drm/i915/dp: Export intel_dp_dsc_min_src_compressed_bpp()

[Imre Deak]

Export the function that can be used by a follow-up change to query the
minimum compressed link bpp supported by the HW.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 4 ++--
 drivers/gpu/drm/i915/display/intel_dp.h | 1 +
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 881852390bb81..9c15608cc06f7 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -2071,7 +2071,7 @@ int intel_dp_dsc_sink_max_compressed_bpp(const struct intel_connector *connector
 						       pipe_config, bpc) >> 4;
 }
 
-static int dsc_src_min_compressed_bpp(void)
+int intel_dp_dsc_min_src_compressed_bpp(void)
 {
 	/* Min Compressed bpp supported by source is 8 */
 	return 8;
@@ -2475,7 +2475,7 @@ intel_dp_compute_config_link_bpp_limits(struct intel_dp *intel_dp,
 		int dsc_src_min_bpp, dsc_sink_min_bpp, dsc_min_bpp;
 		int dsc_src_max_bpp, dsc_sink_max_bpp, dsc_max_bpp;
 
-		dsc_src_min_bpp = dsc_src_min_compressed_bpp();
+		dsc_src_min_bpp = intel_dp_dsc_min_src_compressed_bpp();
 		dsc_sink_min_bpp = intel_dp_dsc_sink_min_compressed_bpp(crtc_state);
 		dsc_min_bpp = max(dsc_src_min_bpp, dsc_sink_min_bpp);
 		limits->link.min_bpp_x16 = fxp_q4_from_int(dsc_min_bpp);
diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
index c974e4b97338a..01744f9ccd8fc 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.h
+++ b/drivers/gpu/drm/i915/display/intel_dp.h
@@ -209,6 +209,7 @@ bool intel_dp_has_connector(struct intel_dp *intel_dp,
 			    const struct drm_connector_state *conn_state);
 int intel_dp_dsc_max_src_input_bpc(struct intel_display *display);
 int intel_dp_dsc_min_src_input_bpc(void);
+int intel_dp_dsc_min_src_compressed_bpp(void);
 
 int intel_dp_dsc_bpp_step_x16(const struct intel_connector *connector);
 
-- 
2.44.2

[PATCH 07/10] drm/i915/dp: Use an intel_connector pointer everywhere

[Imre Deak]

Following the convention, convert intel_dp.c to use an intel_connector
pointer everywhere, calling this pointer connector. If the intel
connector must be casted from a drm_connector, call this pointer
_connector and use this pointer only for the casting.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 106 +++++++++++++-----------
 1 file changed, 57 insertions(+), 49 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 9c15608cc06f7..8ca33ebedce27 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -5825,20 +5825,21 @@ intel_dp_detect(struct drm_connector *_connector,
 }
 
 static void
-intel_dp_force(struct drm_connector *connector)
+intel_dp_force(struct drm_connector *_connector)
 {
-	struct intel_display *display = to_intel_display(connector->dev);
-	struct intel_dp *intel_dp = intel_attached_dp(to_intel_connector(connector));
+	struct intel_connector *connector = to_intel_connector(_connector);
+	struct intel_display *display = to_intel_display(connector);
+	struct intel_dp *intel_dp = intel_attached_dp(connector);
 
 	drm_dbg_kms(display->drm, "[CONNECTOR:%d:%s]\n",
-		    connector->base.id, connector->name);
+		    connector->base.base.id, connector->base.name);
 
 	if (!intel_display_driver_check_access(display))
 		return;
 
 	intel_dp_unset_edid(intel_dp);
 
-	if (connector->status != connector_status_connected)
+	if (connector->base.status != connector_status_connected)
 		return;
 
 	intel_dp_set_edid(intel_dp);
@@ -5877,24 +5878,25 @@ static int intel_dp_get_modes(struct drm_connector *_connector)
 }
 
 static int
-intel_dp_connector_register(struct drm_connector *connector)
+intel_dp_connector_register(struct drm_connector *_connector)
 {
-	struct intel_display *display = to_intel_display(connector->dev);
-	struct intel_dp *intel_dp = intel_attached_dp(to_intel_connector(connector));
+	struct intel_connector *connector = to_intel_connector(_connector);
+	struct intel_display *display = to_intel_display(connector);
+	struct intel_dp *intel_dp = intel_attached_dp(connector);
 	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
 	int ret;
 
-	ret = intel_connector_register(connector);
+	ret = intel_connector_register(&connector->base);
 	if (ret)
 		return ret;
 
 	drm_dbg_kms(display->drm, "registering %s bus for %s\n",
-		    intel_dp->aux.name, connector->kdev->kobj.name);
+		    intel_dp->aux.name, connector->base.kdev->kobj.name);
 
-	intel_dp->aux.dev = connector->kdev;
+	intel_dp->aux.dev = connector->base.kdev;
 	ret = drm_dp_aux_register(&intel_dp->aux);
 	if (!ret)
-		drm_dp_cec_register_connector(&intel_dp->aux, connector);
+		drm_dp_cec_register_connector(&intel_dp->aux, &connector->base);
 
 	if (!intel_bios_encoder_is_lspcon(dig_port->base.devdata))
 		return ret;
@@ -5905,20 +5907,21 @@ intel_dp_connector_register(struct drm_connector *connector)
 	 */
 	if (intel_lspcon_init(dig_port)) {
 		if (intel_lspcon_detect_hdr_capability(dig_port))
-			drm_connector_attach_hdr_output_metadata_property(connector);
+			drm_connector_attach_hdr_output_metadata_property(&connector->base);
 	}
 
 	return ret;
 }
 
 static void
-intel_dp_connector_unregister(struct drm_connector *connector)
+intel_dp_connector_unregister(struct drm_connector *_connector)
 {
-	struct intel_dp *intel_dp = intel_attached_dp(to_intel_connector(connector));
+	struct intel_connector *connector = to_intel_connector(_connector);
+	struct intel_dp *intel_dp = intel_attached_dp(connector);
 
 	drm_dp_cec_unregister_connector(&intel_dp->aux);
 	drm_dp_aux_unregister(&intel_dp->aux);
-	intel_connector_unregister(connector);
+	intel_connector_unregister(&connector->base);
 }
 
 void intel_dp_connector_sync_state(struct intel_connector *connector,
@@ -5979,21 +5982,22 @@ static int intel_modeset_tile_group(struct intel_atomic_state *state,
 {
 	struct intel_display *display = to_intel_display(state);
 	struct drm_connector_list_iter conn_iter;
-	struct drm_connector *connector;
+	struct drm_connector *_connector;
 	int ret = 0;
 
 	drm_connector_list_iter_begin(display->drm, &conn_iter);
-	drm_for_each_connector_iter(connector, &conn_iter) {
+	drm_for_each_connector_iter(_connector, &conn_iter) {
+		struct intel_connector *connector = to_intel_connector(_connector);
 		struct drm_connector_state *conn_state;
 		struct intel_crtc_state *crtc_state;
 		struct intel_crtc *crtc;
 
-		if (!connector->has_tile ||
-		    connector->tile_group->id != tile_group_id)
+		if (!connector->base.has_tile ||
+		    connector->base.tile_group->id != tile_group_id)
 			continue;
 
 		conn_state = drm_atomic_get_connector_state(&state->base,
-							    connector);
+							    &connector->base);
 		if (IS_ERR(conn_state)) {
 			ret = PTR_ERR(conn_state);
 			break;
@@ -6057,10 +6061,11 @@ static int intel_modeset_affected_transcoders(struct intel_atomic_state *state,
 }
 
 static int intel_modeset_synced_crtcs(struct intel_atomic_state *state,
-				      struct drm_connector *connector)
+				      struct drm_connector *_connector)
 {
+	struct intel_connector *connector = to_intel_connector(_connector);
 	const struct drm_connector_state *old_conn_state =
-		drm_atomic_get_old_connector_state(&state->base, connector);
+		drm_atomic_get_old_connector_state(&state->base, &connector->base);
 	const struct intel_crtc_state *old_crtc_state;
 	struct intel_crtc *crtc;
 	u8 transcoders;
@@ -6082,17 +6087,18 @@ static int intel_modeset_synced_crtcs(struct intel_atomic_state *state,
 						  transcoders);
 }
 
-static int intel_dp_connector_atomic_check(struct drm_connector *conn,
+static int intel_dp_connector_atomic_check(struct drm_connector *_connector,
 					   struct drm_atomic_state *_state)
 {
-	struct intel_display *display = to_intel_display(conn->dev);
+	struct intel_connector *connector = to_intel_connector(_connector);
+	struct intel_display *display = to_intel_display(connector);
 	struct intel_atomic_state *state = to_intel_atomic_state(_state);
-	struct drm_connector_state *conn_state = drm_atomic_get_new_connector_state(_state, conn);
-	struct intel_connector *intel_conn = to_intel_connector(conn);
-	struct intel_dp *intel_dp = enc_to_intel_dp(intel_conn->encoder);
+	struct drm_connector_state *conn_state =
+		drm_atomic_get_new_connector_state(_state, &connector->base);
+	struct intel_dp *intel_dp = enc_to_intel_dp(connector->encoder);
 	int ret;
 
-	ret = intel_digital_connector_atomic_check(conn, &state->base);
+	ret = intel_digital_connector_atomic_check(&connector->base, &state->base);
 	if (ret)
 		return ret;
 
@@ -6102,12 +6108,12 @@ static int intel_dp_connector_atomic_check(struct drm_connector *conn,
 			return ret;
 	}
 
-	if (!intel_connector_needs_modeset(state, conn))
+	if (!intel_connector_needs_modeset(state, &connector->base))
 		return 0;
 
 	ret = intel_dp_tunnel_atomic_check_state(state,
 						 intel_dp,
-						 intel_conn);
+						 connector);
 	if (ret)
 		return ret;
 
@@ -6118,21 +6124,22 @@ static int intel_dp_connector_atomic_check(struct drm_connector *conn,
 	if (DISPLAY_VER(display) < 9)
 		return 0;
 
-	if (conn->has_tile) {
-		ret = intel_modeset_tile_group(state, conn->tile_group->id);
+	if (connector->base.has_tile) {
+		ret = intel_modeset_tile_group(state, connector->base.tile_group->id);
 		if (ret)
 			return ret;
 	}
 
-	return intel_modeset_synced_crtcs(state, conn);
+	return intel_modeset_synced_crtcs(state, &connector->base);
 }
 
-static void intel_dp_oob_hotplug_event(struct drm_connector *connector,
+static void intel_dp_oob_hotplug_event(struct drm_connector *_connector,
 				       enum drm_connector_status hpd_state)
 {
-	struct intel_display *display = to_intel_display(connector->dev);
-	struct intel_encoder *encoder = intel_attached_encoder(to_intel_connector(connector));
-	struct drm_i915_private *i915 = to_i915(connector->dev);
+	struct intel_connector *connector = to_intel_connector(_connector);
+	struct intel_display *display = to_intel_display(connector);
+	struct intel_encoder *encoder = intel_attached_encoder(connector);
+	struct drm_i915_private *i915 = to_i915(display->drm);
 	bool hpd_high = hpd_state == connector_status_connected;
 	unsigned int hpd_pin = encoder->hpd_pin;
 	bool need_work = false;
@@ -6278,36 +6285,37 @@ intel_dp_has_gamut_metadata_dip(struct intel_encoder *encoder)
 }
 
 static void
-intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connector)
+intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *_connector)
 {
+	struct intel_connector *connector = to_intel_connector(_connector);
 	struct intel_display *display = to_intel_display(intel_dp);
 	enum port port = dp_to_dig_port(intel_dp)->base.port;
 
 	if (!intel_dp_is_edp(intel_dp))
-		drm_connector_attach_dp_subconnector_property(connector);
+		drm_connector_attach_dp_subconnector_property(&connector->base);
 
 	if (!display->platform.g4x && port != PORT_A)
-		intel_attach_force_audio_property(connector);
+		intel_attach_force_audio_property(&connector->base);
 
-	intel_attach_broadcast_rgb_property(connector);
+	intel_attach_broadcast_rgb_property(&connector->base);
 	if (HAS_GMCH(display))
-		drm_connector_attach_max_bpc_property(connector, 6, 10);
+		drm_connector_attach_max_bpc_property(&connector->base, 6, 10);
 	else if (DISPLAY_VER(display) >= 5)
-		drm_connector_attach_max_bpc_property(connector, 6, 12);
+		drm_connector_attach_max_bpc_property(&connector->base, 6, 12);
 
 	/* Register HDMI colorspace for case of lspcon */
 	if (intel_bios_encoder_is_lspcon(dp_to_dig_port(intel_dp)->base.devdata)) {
-		drm_connector_attach_content_type_property(connector);
-		intel_attach_hdmi_colorspace_property(connector);
+		drm_connector_attach_content_type_property(&connector->base);
+		intel_attach_hdmi_colorspace_property(&connector->base);
 	} else {
-		intel_attach_dp_colorspace_property(connector);
+		intel_attach_dp_colorspace_property(&connector->base);
 	}
 
 	if (intel_dp_has_gamut_metadata_dip(&dp_to_dig_port(intel_dp)->base))
-		drm_connector_attach_hdr_output_metadata_property(connector);
+		drm_connector_attach_hdr_output_metadata_property(&connector->base);
 
 	if (HAS_VRR(display))
-		drm_connector_attach_vrr_capable_property(connector);
+		drm_connector_attach_vrr_capable_property(&connector->base);
 }
 
 static void
-- 
2.44.2

[PATCH 08/10] drm/i915/hdmi: Use an intel_connector pointer everywhere

[Imre Deak]

Following the convention, convert intel_hdmi.c to use an intel_connector
pointer everywhere, calling this pointer connector. If the intel
connector must be casted from a drm_connector, call this pointer
_connector and use this pointer only for the casting.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_hdmi.c | 187 ++++++++++++----------
 1 file changed, 103 insertions(+), 84 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index f9fa17e1f584b..8f2cef36bdf79 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -715,7 +715,7 @@ intel_hdmi_compute_avi_infoframe(struct intel_encoder *encoder,
 	struct hdmi_avi_infoframe *frame = &crtc_state->infoframes.avi.avi;
 	const struct drm_display_mode *adjusted_mode =
 		&crtc_state->hw.adjusted_mode;
-	struct drm_connector *connector = conn_state->connector;
+	struct intel_connector *connector = to_intel_connector(conn_state->connector);
 	int ret;
 
 	if (!crtc_state->has_infoframe)
@@ -724,7 +724,7 @@ intel_hdmi_compute_avi_infoframe(struct intel_encoder *encoder,
 	crtc_state->infoframes.enable |=
 		intel_hdmi_infoframe_enable(HDMI_INFOFRAME_TYPE_AVI);
 
-	ret = drm_hdmi_avi_infoframe_from_display_mode(frame, connector,
+	ret = drm_hdmi_avi_infoframe_from_display_mode(frame, &connector->base,
 						       adjusted_mode);
 	if (ret)
 		return false;
@@ -743,7 +743,7 @@ intel_hdmi_compute_avi_infoframe(struct intel_encoder *encoder,
 		    crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB);
 
 	if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_RGB) {
-		drm_hdmi_avi_infoframe_quant_range(frame, connector,
+		drm_hdmi_avi_infoframe_quant_range(frame, &connector->base,
 						   adjusted_mode,
 						   crtc_state->limited_color_range ?
 						   HDMI_QUANTIZATION_RANGE_LIMITED :
@@ -1943,11 +1943,12 @@ static bool intel_hdmi_source_bpc_possible(struct intel_display *display, int bp
 	}
 }
 
-static bool intel_hdmi_sink_bpc_possible(struct drm_connector *connector,
+static bool intel_hdmi_sink_bpc_possible(struct drm_connector *_connector,
 					 int bpc, bool has_hdmi_sink,
 					 enum intel_output_format sink_format)
 {
-	const struct drm_display_info *info = &connector->display_info;
+	struct intel_connector *connector = to_intel_connector(_connector);
+	const struct drm_display_info *info = &connector->base.display_info;
 	const struct drm_hdmi_info *hdmi = &info->hdmi;
 
 	switch (bpc) {
@@ -1976,12 +1977,13 @@ static bool intel_hdmi_sink_bpc_possible(struct drm_connector *connector,
 }
 
 static enum drm_mode_status
-intel_hdmi_mode_clock_valid(struct drm_connector *connector, int clock,
+intel_hdmi_mode_clock_valid(struct drm_connector *_connector, int clock,
 			    bool has_hdmi_sink,
 			    enum intel_output_format sink_format)
 {
-	struct intel_display *display = to_intel_display(connector->dev);
-	struct intel_hdmi *hdmi = intel_attached_hdmi(to_intel_connector(connector));
+	struct intel_connector *connector = to_intel_connector(_connector);
+	struct intel_display *display = to_intel_display(connector);
+	struct intel_hdmi *hdmi = intel_attached_hdmi(connector);
 	enum drm_mode_status status = MODE_OK;
 	int bpc;
 
@@ -1996,7 +1998,8 @@ intel_hdmi_mode_clock_valid(struct drm_connector *connector, int clock,
 		if (!intel_hdmi_source_bpc_possible(display, bpc))
 			continue;
 
-		if (!intel_hdmi_sink_bpc_possible(connector, bpc, has_hdmi_sink, sink_format))
+		if (!intel_hdmi_sink_bpc_possible(&connector->base, bpc, has_hdmi_sink,
+						  sink_format))
 			continue;
 
 		status = hdmi_port_clock_valid(hdmi, tmds_clock, true, has_hdmi_sink);
@@ -2011,15 +2014,16 @@ intel_hdmi_mode_clock_valid(struct drm_connector *connector, int clock,
 }
 
 static enum drm_mode_status
-intel_hdmi_mode_valid(struct drm_connector *connector,
+intel_hdmi_mode_valid(struct drm_connector *_connector,
 		      const struct drm_display_mode *mode)
 {
-	struct intel_display *display = to_intel_display(connector->dev);
-	struct intel_hdmi *hdmi = intel_attached_hdmi(to_intel_connector(connector));
+	struct intel_connector *connector = to_intel_connector(_connector);
+	struct intel_display *display = to_intel_display(connector);
+	struct intel_hdmi *hdmi = intel_attached_hdmi(connector);
 	enum drm_mode_status status;
 	int clock = mode->clock;
-	int max_dotclk = to_i915(connector->dev)->display.cdclk.max_dotclk_freq;
-	bool has_hdmi_sink = intel_has_hdmi_sink(hdmi, connector->state);
+	int max_dotclk = display->cdclk.max_dotclk_freq;
+	bool has_hdmi_sink = intel_has_hdmi_sink(hdmi, connector->base.state);
 	bool ycbcr_420_only;
 	enum intel_output_format sink_format;
 
@@ -2048,22 +2052,23 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
 	if (clock > 600000)
 		return MODE_CLOCK_HIGH;
 
-	ycbcr_420_only = drm_mode_is_420_only(&connector->display_info, mode);
+	ycbcr_420_only = drm_mode_is_420_only(&connector->base.display_info, mode);
 
 	if (ycbcr_420_only)
 		sink_format = INTEL_OUTPUT_FORMAT_YCBCR420;
 	else
 		sink_format = INTEL_OUTPUT_FORMAT_RGB;
 
-	status = intel_hdmi_mode_clock_valid(connector, clock, has_hdmi_sink, sink_format);
+	status = intel_hdmi_mode_clock_valid(&connector->base, clock, has_hdmi_sink, sink_format);
 	if (status != MODE_OK) {
 		if (ycbcr_420_only ||
-		    !connector->ycbcr_420_allowed ||
-		    !drm_mode_is_420_also(&connector->display_info, mode))
+		    !connector->base.ycbcr_420_allowed ||
+		    !drm_mode_is_420_also(&connector->base.display_info, mode))
 			return status;
 
 		sink_format = INTEL_OUTPUT_FORMAT_YCBCR420;
-		status = intel_hdmi_mode_clock_valid(connector, clock, has_hdmi_sink, sink_format);
+		status = intel_hdmi_mode_clock_valid(&connector->base, clock, has_hdmi_sink,
+						     sink_format);
 		if (status != MODE_OK)
 			return status;
 	}
@@ -2076,14 +2081,16 @@ bool intel_hdmi_bpc_possible(const struct intel_crtc_state *crtc_state,
 {
 	struct drm_atomic_state *state = crtc_state->uapi.state;
 	struct drm_connector_state *connector_state;
-	struct drm_connector *connector;
+	struct drm_connector *_connector;
 	int i;
 
-	for_each_new_connector_in_state(state, connector, connector_state, i) {
+	for_each_new_connector_in_state(state, _connector, connector_state, i) {
+		struct intel_connector *connector = to_intel_connector(_connector);
+
 		if (connector_state->crtc != crtc_state->uapi.crtc)
 			continue;
 
-		if (!intel_hdmi_sink_bpc_possible(connector, bpc, has_hdmi_sink,
+		if (!intel_hdmi_sink_bpc_possible(&connector->base, bpc, has_hdmi_sink,
 						  crtc_state->sink_format))
 			return false;
 	}
@@ -2211,7 +2218,7 @@ static bool intel_hdmi_has_audio(struct intel_encoder *encoder,
 				 const struct intel_crtc_state *crtc_state,
 				 const struct drm_connector_state *conn_state)
 {
-	struct drm_connector *connector = conn_state->connector;
+	struct intel_connector *connector = to_intel_connector(conn_state->connector);
 	const struct intel_digital_connector_state *intel_conn_state =
 		to_intel_digital_connector_state(conn_state);
 
@@ -2219,7 +2226,7 @@ static bool intel_hdmi_has_audio(struct intel_encoder *encoder,
 		return false;
 
 	if (intel_conn_state->force_audio == HDMI_AUDIO_AUTO)
-		return connector->display_info.has_audio;
+		return connector->base.display_info.has_audio;
 	else
 		return intel_conn_state->force_audio == HDMI_AUDIO_ON;
 }
@@ -2323,14 +2330,14 @@ int intel_hdmi_compute_config(struct intel_encoder *encoder,
 {
 	struct intel_display *display = to_intel_display(encoder);
 	struct drm_display_mode *adjusted_mode = &pipe_config->hw.adjusted_mode;
-	struct drm_connector *connector = conn_state->connector;
-	struct drm_scdc *scdc = &connector->display_info.hdmi.scdc;
+	struct intel_connector *connector = to_intel_connector(conn_state->connector);
+	struct drm_scdc *scdc = &connector->base.display_info.hdmi.scdc;
 	int ret;
 
 	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)
 		return -EINVAL;
 
-	if (!connector->interlace_allowed &&
+	if (!connector->base.interlace_allowed &&
 	    adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE)
 		return -EINVAL;
 
@@ -2425,25 +2432,27 @@ void intel_hdmi_encoder_shutdown(struct intel_encoder *encoder)
 }
 
 static void
-intel_hdmi_unset_edid(struct drm_connector *connector)
+intel_hdmi_unset_edid(struct drm_connector *_connector)
 {
-	struct intel_hdmi *intel_hdmi = intel_attached_hdmi(to_intel_connector(connector));
+	struct intel_connector *connector = to_intel_connector(_connector);
+	struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
 
 	intel_hdmi->dp_dual_mode.type = DRM_DP_DUAL_MODE_NONE;
 	intel_hdmi->dp_dual_mode.max_tmds_clock = 0;
 
-	drm_edid_free(to_intel_connector(connector)->detect_edid);
-	to_intel_connector(connector)->detect_edid = NULL;
+	drm_edid_free(connector->detect_edid);
+	connector->detect_edid = NULL;
 }
 
 static void
-intel_hdmi_dp_dual_mode_detect(struct drm_connector *connector)
+intel_hdmi_dp_dual_mode_detect(struct drm_connector *_connector)
 {
-	struct intel_display *display = to_intel_display(connector->dev);
-	struct drm_i915_private *dev_priv = to_i915(connector->dev);
-	struct intel_hdmi *hdmi = intel_attached_hdmi(to_intel_connector(connector));
+	struct intel_connector *connector = to_intel_connector(_connector);
+	struct intel_display *display = to_intel_display(connector);
+	struct drm_i915_private *dev_priv = to_i915(display->drm);
+	struct intel_hdmi *hdmi = intel_attached_hdmi(connector);
 	struct intel_encoder *encoder = &hdmi_to_dig_port(hdmi)->base;
-	struct i2c_adapter *ddc = connector->ddc;
+	struct i2c_adapter *ddc = connector->base.ddc;
 	enum drm_dp_dual_mode_type type;
 
 	type = drm_dp_dual_mode_detect(display->drm, ddc);
@@ -2458,7 +2467,7 @@ intel_hdmi_dp_dual_mode_detect(struct drm_connector *connector)
 	 * if the port is a dual mode capable DP port.
 	 */
 	if (type == DRM_DP_DUAL_MODE_UNKNOWN) {
-		if (!connector->force &&
+		if (!connector->base.force &&
 		    intel_bios_encoder_supports_dp_dual_mode(encoder->devdata)) {
 			drm_dbg_kms(display->drm,
 				    "Assuming DP dual mode adaptor presence based on VBT\n");
@@ -2490,34 +2499,35 @@ intel_hdmi_dp_dual_mode_detect(struct drm_connector *connector)
 }
 
 static bool
-intel_hdmi_set_edid(struct drm_connector *connector)
+intel_hdmi_set_edid(struct drm_connector *_connector)
 {
-	struct intel_display *display = to_intel_display(connector->dev);
-	struct intel_hdmi *intel_hdmi = intel_attached_hdmi(to_intel_connector(connector));
-	struct i2c_adapter *ddc = connector->ddc;
+	struct intel_connector *connector = to_intel_connector(_connector);
+	struct intel_display *display = to_intel_display(connector);
+	struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
+	struct i2c_adapter *ddc = connector->base.ddc;
 	intel_wakeref_t wakeref;
 	const struct drm_edid *drm_edid;
 	bool connected = false;
 
 	wakeref = intel_display_power_get(display, POWER_DOMAIN_GMBUS);
 
-	drm_edid = drm_edid_read_ddc(connector, ddc);
+	drm_edid = drm_edid_read_ddc(&connector->base, ddc);
 
 	if (!drm_edid && !intel_gmbus_is_forced_bit(ddc)) {
 		drm_dbg_kms(display->drm,
 			    "HDMI GMBUS EDID read failed, retry using GPIO bit-banging\n");
 		intel_gmbus_force_bit(ddc, true);
-		drm_edid = drm_edid_read_ddc(connector, ddc);
+		drm_edid = drm_edid_read_ddc(&connector->base, ddc);
 		intel_gmbus_force_bit(ddc, false);
 	}
 
 	/* Below we depend on display info having been updated */
-	drm_edid_connector_update(connector, drm_edid);
+	drm_edid_connector_update(&connector->base, drm_edid);
 
-	to_intel_connector(connector)->detect_edid = drm_edid;
+	connector->detect_edid = drm_edid;
 
 	if (drm_edid_is_digital(drm_edid)) {
-		intel_hdmi_dp_dual_mode_detect(connector);
+		intel_hdmi_dp_dual_mode_detect(&connector->base);
 
 		connected = true;
 	}
@@ -2525,28 +2535,29 @@ intel_hdmi_set_edid(struct drm_connector *connector)
 	intel_display_power_put(display, POWER_DOMAIN_GMBUS, wakeref);
 
 	cec_notifier_set_phys_addr(intel_hdmi->cec_notifier,
-				   connector->display_info.source_physical_address);
+				   connector->base.display_info.source_physical_address);
 
 	return connected;
 }
 
 static enum drm_connector_status
-intel_hdmi_detect(struct drm_connector *connector, bool force)
+intel_hdmi_detect(struct drm_connector *_connector, bool force)
 {
-	struct intel_display *display = to_intel_display(connector->dev);
+	struct intel_connector *connector = to_intel_connector(_connector);
+	struct intel_display *display = to_intel_display(connector);
 	enum drm_connector_status status = connector_status_disconnected;
-	struct intel_hdmi *intel_hdmi = intel_attached_hdmi(to_intel_connector(connector));
+	struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
 	struct intel_encoder *encoder = &hdmi_to_dig_port(intel_hdmi)->base;
 	intel_wakeref_t wakeref;
 
 	drm_dbg_kms(display->drm, "[CONNECTOR:%d:%s]\n",
-		    connector->base.id, connector->name);
+		    connector->base.base.id, connector->base.name);
 
 	if (!intel_display_device_enabled(display))
 		return connector_status_disconnected;
 
 	if (!intel_display_driver_check_access(display))
-		return connector->status;
+		return connector->base.status;
 
 	wakeref = intel_display_power_get(display, POWER_DOMAIN_GMBUS);
 
@@ -2554,9 +2565,9 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
 	    !intel_digital_port_connected(encoder))
 		goto out;
 
-	intel_hdmi_unset_edid(connector);
+	intel_hdmi_unset_edid(&connector->base);
 
-	if (intel_hdmi_set_edid(connector))
+	if (intel_hdmi_set_edid(&connector->base))
 		status = connector_status_connected;
 
 out:
@@ -2569,49 +2580,54 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
 }
 
 static void
-intel_hdmi_force(struct drm_connector *connector)
+intel_hdmi_force(struct drm_connector *_connector)
 {
-	struct intel_display *display = to_intel_display(connector->dev);
+	struct intel_connector *connector = to_intel_connector(_connector);
+	struct intel_display *display = to_intel_display(connector);
 
 	drm_dbg_kms(display->drm, "[CONNECTOR:%d:%s]\n",
-		    connector->base.id, connector->name);
+		    connector->base.base.id, connector->base.name);
 
 	if (!intel_display_driver_check_access(display))
 		return;
 
-	intel_hdmi_unset_edid(connector);
+	intel_hdmi_unset_edid(&connector->base);
 
-	if (connector->status != connector_status_connected)
+	if (connector->base.status != connector_status_connected)
 		return;
 
-	intel_hdmi_set_edid(connector);
+	intel_hdmi_set_edid(&connector->base);
 }
 
-static int intel_hdmi_get_modes(struct drm_connector *connector)
+static int intel_hdmi_get_modes(struct drm_connector *_connector)
 {
+	struct intel_connector *connector = to_intel_connector(_connector);
+
 	/* drm_edid_connector_update() done in ->detect() or ->force() */
-	return drm_edid_connector_add_modes(connector);
+	return drm_edid_connector_add_modes(&connector->base);
 }
 
 static int
-intel_hdmi_connector_register(struct drm_connector *connector)
+intel_hdmi_connector_register(struct drm_connector *_connector)
 {
+	struct intel_connector *connector = to_intel_connector(_connector);
 	int ret;
 
-	ret = intel_connector_register(connector);
+	ret = intel_connector_register(&connector->base);
 	if (ret)
 		return ret;
 
 	return ret;
 }
 
-static void intel_hdmi_connector_unregister(struct drm_connector *connector)
+static void intel_hdmi_connector_unregister(struct drm_connector *_connector)
 {
-	struct cec_notifier *n = intel_attached_hdmi(to_intel_connector(connector))->cec_notifier;
+	struct intel_connector *connector = to_intel_connector(_connector);
+	struct cec_notifier *n = intel_attached_hdmi(connector)->cec_notifier;
 
 	cec_notifier_conn_unregister(n);
 
-	intel_connector_unregister(connector);
+	intel_connector_unregister(&connector->base);
 }
 
 static const struct drm_connector_funcs intel_hdmi_connector_funcs = {
@@ -2627,15 +2643,16 @@ static const struct drm_connector_funcs intel_hdmi_connector_funcs = {
 	.atomic_duplicate_state = intel_digital_connector_duplicate_state,
 };
 
-static int intel_hdmi_connector_atomic_check(struct drm_connector *connector,
+static int intel_hdmi_connector_atomic_check(struct drm_connector *_connector,
 					     struct drm_atomic_state *state)
 {
-	struct intel_display *display = to_intel_display(connector->dev);
+	struct intel_connector *connector = to_intel_connector(_connector);
+	struct intel_display *display = to_intel_display(connector);
 
 	if (HAS_DDI(display))
-		return intel_digital_connector_atomic_check(connector, state);
+		return intel_digital_connector_atomic_check(&connector->base, state);
 	else
-		return g4x_hdmi_connector_atomic_check(connector, state);
+		return g4x_hdmi_connector_atomic_check(&connector->base, state);
 }
 
 static const struct drm_connector_helper_funcs intel_hdmi_connector_helper_funcs = {
@@ -2645,22 +2662,23 @@ static const struct drm_connector_helper_funcs intel_hdmi_connector_helper_funcs
 };
 
 static void
-intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *connector)
+intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *_connector)
 {
+	struct intel_connector *connector = to_intel_connector(_connector);
 	struct intel_display *display = to_intel_display(intel_hdmi);
 
-	intel_attach_force_audio_property(connector);
-	intel_attach_broadcast_rgb_property(connector);
-	intel_attach_aspect_ratio_property(connector);
+	intel_attach_force_audio_property(&connector->base);
+	intel_attach_broadcast_rgb_property(&connector->base);
+	intel_attach_aspect_ratio_property(&connector->base);
 
-	intel_attach_hdmi_colorspace_property(connector);
-	drm_connector_attach_content_type_property(connector);
+	intel_attach_hdmi_colorspace_property(&connector->base);
+	drm_connector_attach_content_type_property(&connector->base);
 
 	if (DISPLAY_VER(display) >= 10)
-		drm_connector_attach_hdr_output_metadata_property(connector);
+		drm_connector_attach_hdr_output_metadata_property(&connector->base);
 
 	if (!HAS_GMCH(display))
-		drm_connector_attach_max_bpc_property(connector, 8, 12);
+		drm_connector_attach_max_bpc_property(&connector->base, 8, 12);
 }
 
 /*
@@ -2682,25 +2700,26 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *c
  * True on success, false on failure.
  */
 bool intel_hdmi_handle_sink_scrambling(struct intel_encoder *encoder,
-				       struct drm_connector *connector,
+				       struct drm_connector *_connector,
 				       bool high_tmds_clock_ratio,
 				       bool scrambling)
 {
+	struct intel_connector *connector = to_intel_connector(_connector);
 	struct intel_display *display = to_intel_display(encoder);
 	struct drm_scrambling *sink_scrambling =
-		&connector->display_info.hdmi.scdc.scrambling;
+		&connector->base.display_info.hdmi.scdc.scrambling;
 
 	if (!sink_scrambling->supported)
 		return true;
 
 	drm_dbg_kms(display->drm,
 		    "[CONNECTOR:%d:%s] scrambling=%s, TMDS bit clock ratio=1/%d\n",
-		    connector->base.id, connector->name,
+		    connector->base.base.id, connector->base.name,
 		    str_yes_no(scrambling), high_tmds_clock_ratio ? 40 : 10);
 
 	/* Set TMDS bit clock ratio to 1/40 or 1/10, and enable/disable scrambling */
-	return drm_scdc_set_high_tmds_clock_ratio(connector, high_tmds_clock_ratio) &&
-		drm_scdc_set_scrambling(connector, scrambling);
+	return drm_scdc_set_high_tmds_clock_ratio(&connector->base, high_tmds_clock_ratio) &&
+		drm_scdc_set_scrambling(&connector->base, scrambling);
 }
 
 static u8 chv_encoder_to_ddc_pin(struct intel_encoder *encoder)
-- 
2.44.2

[PATCH 09/10] drm/i915: Add support for forcing the link bpp on a connector

[Imre Deak]

Add support for forcing the link bpp on a connector via a connector
debugfs entry. During reducing link bpps due to a link BW limit, keep
bpps close to their forced value.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 .../drm/i915/display/intel_display_types.h    |   4 +
 drivers/gpu/drm/i915/display/intel_link_bw.c  | 204 +++++++++++++++++-
 drivers/gpu/drm/i915/display/intel_link_bw.h  |   2 +
 3 files changed, 203 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 94468a9d2e0d3..6862cb4885b18 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -550,6 +550,10 @@ struct intel_connector {
 		struct intel_dp *dp;
 	} mst;
 
+	struct {
+		int force_bpp_x16;
+	} link;
+
 	/* Work struct to schedule a uevent on link train failure */
 	struct work_struct modeset_retry_work;
 
diff --git a/drivers/gpu/drm/i915/display/intel_link_bw.c b/drivers/gpu/drm/i915/display/intel_link_bw.c
index a10cd39926075..2a05fac53aa78 100644
--- a/drivers/gpu/drm/i915/display/intel_link_bw.c
+++ b/drivers/gpu/drm/i915/display/intel_link_bw.c
@@ -3,6 +3,9 @@
  * Copyright © 2023 Intel Corporation
  */
 
+#include <linux/debugfs.h>
+#include <linux/math.h>
+
 #include <drm/drm_fixed.h>
 #include <drm/drm_print.h>
 
@@ -10,11 +13,35 @@
 #include "intel_crtc.h"
 #include "intel_display_core.h"
 #include "intel_display_types.h"
+#include "intel_dp.h"
 #include "intel_dp_mst.h"
 #include "intel_dp_tunnel.h"
 #include "intel_fdi.h"
 #include "intel_link_bw.h"
 
+static int get_forced_link_bpp_x16(struct intel_atomic_state *state,
+				   const struct intel_crtc *crtc)
+{
+	struct drm_connector_state *conn_state;
+	struct drm_connector *_connector;
+	int force_bpp_x16 = INT_MAX;
+	int i;
+
+	for_each_new_connector_in_state(&state->base, _connector, conn_state, i) {
+		struct intel_connector *connector = to_intel_connector(_connector);
+
+		if (conn_state->crtc != &crtc->base)
+			continue;
+
+		if (!connector->link.force_bpp_x16)
+			continue;
+
+		force_bpp_x16 = min(force_bpp_x16, connector->link.force_bpp_x16);
+	}
+
+	return force_bpp_x16 < INT_MAX ? force_bpp_x16 : 0;
+}
+
 /**
  * intel_link_bw_init_limits - initialize BW limits
  * @state: Atomic state
@@ -31,9 +58,10 @@ void intel_link_bw_init_limits(struct intel_atomic_state *state,
 	limits->force_fec_pipes = 0;
 	limits->bpp_limit_reached_pipes = 0;
 	for_each_pipe(display, pipe) {
+		struct intel_crtc *crtc = intel_crtc_for_pipe(display, pipe);
 		const struct intel_crtc_state *crtc_state =
-			intel_atomic_get_new_crtc_state(state,
-							intel_crtc_for_pipe(display, pipe));
+			intel_atomic_get_new_crtc_state(state, crtc);
+		int forced_bpp_x16 = get_forced_link_bpp_x16(state, crtc);
 
 		if (state->base.duplicated && crtc_state) {
 			limits->max_bpp_x16[pipe] = crtc_state->max_link_bpp_x16;
@@ -42,15 +70,19 @@ void intel_link_bw_init_limits(struct intel_atomic_state *state,
 		} else {
 			limits->max_bpp_x16[pipe] = INT_MAX;
 		}
+
+		if (forced_bpp_x16)
+			limits->max_bpp_x16[pipe] = min(limits->max_bpp_x16[pipe], forced_bpp_x16);
 	}
 }
 
 /**
- * intel_link_bw_reduce_bpp - reduce maximum link bpp for a selected pipe
+ * __intel_link_bw_reduce_bpp - reduce maximum link bpp for a selected pipe
  * @state: atomic state
  * @limits: link BW limits
  * @pipe_mask: mask of pipes to select from
  * @reason: explanation of why bpp reduction is needed
+ * @reduce_forced_bpp: allow reducing bpps below their forced link bpp
  *
  * Select the pipe from @pipe_mask with the biggest link bpp value and set the
  * maximum of link bpp in @limits below this value. Modeset the selected pipe,
@@ -64,10 +96,11 @@ void intel_link_bw_init_limits(struct intel_atomic_state *state,
  *   - %-ENOSPC if no pipe can further reduce its link bpp
  *   - Other negative error, if modesetting the selected pipe failed
  */
-int intel_link_bw_reduce_bpp(struct intel_atomic_state *state,
-			     struct intel_link_bw_limits *limits,
-			     u8 pipe_mask,
-			     const char *reason)
+static int __intel_link_bw_reduce_bpp(struct intel_atomic_state *state,
+				      struct intel_link_bw_limits *limits,
+				      u8 pipe_mask,
+				      const char *reason,
+				      bool reduce_forced_bpp)
 {
 	struct intel_display *display = to_intel_display(state);
 	enum pipe max_bpp_pipe = INVALID_PIPE;
@@ -97,6 +130,10 @@ int intel_link_bw_reduce_bpp(struct intel_atomic_state *state,
 			 */
 			link_bpp_x16 = fxp_q4_from_int(crtc_state->pipe_bpp);
 
+		if (!reduce_forced_bpp &&
+		    link_bpp_x16 <= get_forced_link_bpp_x16(state, crtc))
+			continue;
+
 		if (link_bpp_x16 > max_bpp_x16) {
 			max_bpp_x16 = link_bpp_x16;
 			max_bpp_pipe = crtc->pipe;
@@ -112,6 +149,21 @@ int intel_link_bw_reduce_bpp(struct intel_atomic_state *state,
 						 BIT(max_bpp_pipe));
 }
 
+int intel_link_bw_reduce_bpp(struct intel_atomic_state *state,
+			     struct intel_link_bw_limits *limits,
+			     u8 pipe_mask,
+			     const char *reason)
+{
+	int ret;
+
+	/* Try to keep any forced link BPP. */
+	ret = __intel_link_bw_reduce_bpp(state, limits, pipe_mask, reason, false);
+	if (ret == -ENOSPC)
+		ret = __intel_link_bw_reduce_bpp(state, limits, pipe_mask, reason, true);
+
+	return ret;
+}
+
 /**
  * intel_link_bw_set_bpp_limit_for_pipe - set link bpp limit for a pipe to its minimum
  * @state: atomic state
@@ -245,3 +297,141 @@ int intel_link_bw_atomic_check(struct intel_atomic_state *state,
 
 	return -EAGAIN;
 }
+
+static int force_link_bpp_show(struct seq_file *m, void *data)
+{
+	struct intel_connector *connector = m->private;
+
+	seq_printf(m, FXP_Q4_FMT "\n", FXP_Q4_ARGS(connector->link.force_bpp_x16));
+
+	return 0;
+}
+
+static int str_to_fxp_q4_uint(const char *str, int *val_x16)
+{
+	unsigned int val;
+	int err;
+
+	err = kstrtouint(str, 10, &val);
+	if (err)
+		return err;
+
+	if (val > INT_MAX >> 4)
+		return -ERANGE;
+
+	*val_x16 = fxp_q4_from_int(val);
+
+	return 0;
+}
+
+/* modifies str */
+static int str_to_fxp_q4(char *str, int *val_x16)
+{
+	const char *int_str;
+	char *frac_str;
+	int frac_val;
+	int err;
+
+	int_str = strim(str);
+	frac_str = strchr(int_str, '.');
+
+	if (frac_str)
+		*frac_str++ = '\0';
+
+	err = str_to_fxp_q4_uint(int_str, val_x16);
+	if (err)
+		return err;
+
+	if (!frac_str)
+		return 0;
+
+	if (*frac_str == '+')   /* otherwise valid in front of an unsigned integer */
+		return -EINVAL;
+
+	err = str_to_fxp_q4_uint(frac_str, &frac_val);
+	if (err)
+		return err;
+
+	*val_x16 += DIV_ROUND_CLOSEST(frac_val, int_pow(10, strlen(frac_str)));
+
+	return 0;
+}
+
+static int user_str_to_fxp_q4(const char __user *ubuf, size_t len, int *val_x16)
+{
+	char *kbuf;
+	int err;
+
+	kbuf = memdup_user_nul(ubuf, len);
+	if (IS_ERR(kbuf))
+		return PTR_ERR(kbuf);
+
+	err = str_to_fxp_q4(kbuf, val_x16);
+
+	kfree(kbuf);
+
+	return err;
+}
+
+static bool connector_supports_dsc(struct intel_connector *connector)
+{
+	struct intel_display *display = to_intel_display(connector);
+
+	switch (connector->base.connector_type) {
+	case DRM_MODE_CONNECTOR_eDP:
+		return intel_dp_has_dsc(connector);
+	case DRM_MODE_CONNECTOR_DisplayPort:
+		if (connector->mst.dp)
+			return HAS_DSC_MST(display);
+
+		return HAS_DSC(display);
+	default:
+		return false;
+	}
+}
+
+static ssize_t
+force_link_bpp_write(struct file *file, const char __user *ubuf, size_t len, loff_t *offp)
+{
+	struct seq_file *m = file->private_data;
+	struct intel_connector *connector = m->private;
+	struct intel_display *display = to_intel_display(connector);
+	int min_bpp;
+	int bpp_x16;
+	int err;
+
+	err = user_str_to_fxp_q4(ubuf, len, &bpp_x16);
+	if (err)
+		return err;
+
+	if (connector_supports_dsc(connector))
+		min_bpp = intel_dp_dsc_min_src_compressed_bpp();
+	else
+		min_bpp = intel_display_min_pipe_bpp();
+
+	if (bpp_x16 &&
+	    (bpp_x16 < fxp_q4_from_int(min_bpp) ||
+	     bpp_x16 > fxp_q4_from_int(intel_display_max_pipe_bpp(display))))
+		return -EINVAL;
+
+	err = drm_modeset_lock_single_interruptible(&display->drm->mode_config.connection_mutex);
+	if (err)
+		return err;
+
+	connector->link.force_bpp_x16 = bpp_x16;
+
+	drm_modeset_unlock(&display->drm->mode_config.connection_mutex);
+
+	*offp += len;
+
+	return len;
+}
+DEFINE_SHOW_STORE_ATTRIBUTE(force_link_bpp);
+
+void intel_link_bw_connector_debugfs_add(struct intel_connector *connector)
+{
+	struct dentry *root = connector->base.debugfs_entry;
+
+	debugfs_create_file("i915_force_link_bpp", 0644, root,
+			    connector, &force_link_bpp_fops);
+}
diff --git a/drivers/gpu/drm/i915/display/intel_link_bw.h b/drivers/gpu/drm/i915/display/intel_link_bw.h
index e69049cf178f6..b499042e62b13 100644
--- a/drivers/gpu/drm/i915/display/intel_link_bw.h
+++ b/drivers/gpu/drm/i915/display/intel_link_bw.h
@@ -11,6 +11,7 @@
 #include "intel_display_limits.h"
 
 struct intel_atomic_state;
+struct intel_connector;
 struct intel_crtc_state;
 
 struct intel_link_bw_limits {
@@ -32,5 +33,6 @@ bool intel_link_bw_set_bpp_limit_for_pipe(struct intel_atomic_state *state,
 					  enum pipe pipe);
 int intel_link_bw_atomic_check(struct intel_atomic_state *state,
 			       struct intel_link_bw_limits *new_limits);
+void intel_link_bw_connector_debugfs_add(struct intel_connector *connector);
 
 #endif
-- 
2.44.2

[PATCH 10/10] drm/i915: Add force link bpp debugfs entry to connectors

[Imre Deak]

Add the debugfs entry to force a link bpp to all relevant connectors:
all DP connectors and on an FDI link CRT/SDVO/LVDS/HDMI connectors.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_crt.c      | 20 ++++++++++++++++++-
 .../drm/i915/display/intel_display_device.h   |  1 +
 drivers/gpu/drm/i915/display/intel_dp.c       |  3 +++
 drivers/gpu/drm/i915/display/intel_dp_mst.c   |  2 ++
 drivers/gpu/drm/i915/display/intel_hdmi.c     |  8 +++++++-
 drivers/gpu/drm/i915/display/intel_lvds.c     | 20 ++++++++++++++++++-
 drivers/gpu/drm/i915/display/intel_sdvo.c     | 20 ++++++++++++++++++-
 7 files changed, 70 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_crt.c b/drivers/gpu/drm/i915/display/intel_crt.c
index cca22d2402e88..69831d6f68912 100644
--- a/drivers/gpu/drm/i915/display/intel_crt.c
+++ b/drivers/gpu/drm/i915/display/intel_crt.c
@@ -43,6 +43,7 @@
 #include "intel_ddi.h"
 #include "intel_ddi_buf_trans.h"
 #include "intel_de.h"
+#include "intel_display_device.h"
 #include "intel_display_driver.h"
 #include "intel_display_types.h"
 #include "intel_fdi.h"
@@ -51,6 +52,7 @@
 #include "intel_gmbus.h"
 #include "intel_hotplug.h"
 #include "intel_hotplug_irq.h"
+#include "intel_link_bw.h"
 #include "intel_load_detect.h"
 #include "intel_pch_display.h"
 #include "intel_pch_refclk.h"
@@ -986,13 +988,29 @@ void intel_crt_reset(struct drm_encoder *encoder)
 
 }
 
+static int intel_crt_connector_register(struct drm_connector *_connector)
+{
+	struct intel_connector *connector = to_intel_connector(_connector);
+	struct intel_display *display = to_intel_display(connector);
+	int err;
+
+	err = intel_connector_register(&connector->base);
+	if (err)
+		return err;
+
+	if (HAS_FDI(display))
+		intel_link_bw_connector_debugfs_add(connector);
+
+	return 0;
+}
+
 /*
  * Routines for controlling stuff on the analog port
  */
 
 static const struct drm_connector_funcs intel_crt_connector_funcs = {
 	.fill_modes = drm_helper_probe_single_connector_modes,
-	.late_register = intel_connector_register,
+	.late_register = intel_crt_connector_register,
 	.early_unregister = intel_connector_unregister,
 	.destroy = intel_connector_destroy,
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h b/drivers/gpu/drm/i915/display/intel_display_device.h
index 368b0d3417c26..a84bdc83417f1 100644
--- a/drivers/gpu/drm/i915/display/intel_display_device.h
+++ b/drivers/gpu/drm/i915/display/intel_display_device.h
@@ -171,6 +171,7 @@ struct intel_display_platforms {
 #define HAS_GMBUS_BURST_READ(__display)	(DISPLAY_VER(__display) >= 10 || (__display)->platform.kabylake)
 #define HAS_GMBUS_IRQ(__display)	(DISPLAY_VER(__display) >= 4)
 #define HAS_GMCH(__display)		(DISPLAY_INFO(__display)->has_gmch)
+#define HAS_FDI(__display)		(IS_DISPLAY_VER((__display), 5, 8) && !HAS_GMCH(__display))
 #define HAS_HOTPLUG(__display)		(DISPLAY_INFO(__display)->has_hotplug)
 #define HAS_HW_SAGV_WM(__display)	(DISPLAY_VER(__display) >= 13 && !(__display)->platform.dgfx)
 #define HAS_IPC(__display)		(DISPLAY_INFO(__display)->has_ipc)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 8ca33ebedce27..0b19a9b5adda5 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -80,6 +80,7 @@
 #include "intel_hdmi.h"
 #include "intel_hotplug.h"
 #include "intel_hotplug_irq.h"
+#include "intel_link_bw.h"
 #include "intel_lspcon.h"
 #include "intel_lvds.h"
 #include "intel_modeset_lock.h"
@@ -5890,6 +5891,8 @@ intel_dp_connector_register(struct drm_connector *_connector)
 	if (ret)
 		return ret;
 
+	intel_link_bw_connector_debugfs_add(connector);
+
 	drm_dbg_kms(display->drm, "registering %s bus for %s\n",
 		    intel_dp->aux.name, connector->base.kdev->kobj.name);
 
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 35214d9a8c781..7508aa4e3695f 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -1445,6 +1445,8 @@ mst_connector_late_register(struct drm_connector *_connector)
 	if (ret < 0)
 		drm_dp_mst_connector_early_unregister(&connector->base, connector->mst.port);
 
+	intel_link_bw_connector_debugfs_add(connector);
+
 	return ret;
 }
 
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 8f2cef36bdf79..0747ef8d6c0ca 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -52,6 +52,7 @@
 #include "intel_cx0_phy.h"
 #include "intel_ddi.h"
 #include "intel_de.h"
+#include "intel_display_device.h"
 #include "intel_display_driver.h"
 #include "intel_display_types.h"
 #include "intel_dp.h"
@@ -60,6 +61,7 @@
 #include "intel_hdcp_regs.h"
 #include "intel_hdcp_shim.h"
 #include "intel_hdmi.h"
+#include "intel_link_bw.h"
 #include "intel_lspcon.h"
 #include "intel_panel.h"
 #include "intel_pfit.h"
@@ -2611,13 +2613,17 @@ static int
 intel_hdmi_connector_register(struct drm_connector *_connector)
 {
 	struct intel_connector *connector = to_intel_connector(_connector);
+	struct intel_display *display = to_intel_display(connector);
 	int ret;
 
 	ret = intel_connector_register(&connector->base);
 	if (ret)
 		return ret;
 
-	return ret;
+	if (HAS_FDI(display))
+		intel_link_bw_connector_debugfs_add(connector);
+
+	return 0;
 }
 
 static void intel_hdmi_connector_unregister(struct drm_connector *_connector)
diff --git a/drivers/gpu/drm/i915/display/intel_lvds.c b/drivers/gpu/drm/i915/display/intel_lvds.c
index 89d26913e2539..3ac6aaa025434 100644
--- a/drivers/gpu/drm/i915/display/intel_lvds.c
+++ b/drivers/gpu/drm/i915/display/intel_lvds.c
@@ -45,10 +45,12 @@
 #include "intel_backlight.h"
 #include "intel_connector.h"
 #include "intel_de.h"
+#include "intel_display_device.h"
 #include "intel_display_types.h"
 #include "intel_dpll.h"
 #include "intel_fdi.h"
 #include "intel_gmbus.h"
+#include "intel_link_bw.h"
 #include "intel_lvds.h"
 #include "intel_lvds_regs.h"
 #include "intel_panel.h"
@@ -501,6 +503,22 @@ static int intel_lvds_get_modes(struct drm_connector *_connector)
 	return intel_panel_get_modes(connector);
 }
 
+static int intel_lvds_connector_register(struct drm_connector *_connector)
+{
+	struct intel_connector *connector = to_intel_connector(_connector);
+	struct intel_display *display = to_intel_display(connector);
+	int err;
+
+	err = intel_connector_register(&connector->base);
+	if (err)
+		return err;
+
+	if (HAS_FDI(display))
+		intel_link_bw_connector_debugfs_add(connector);
+
+	return 0;
+}
+
 static const struct drm_connector_helper_funcs intel_lvds_connector_helper_funcs = {
 	.get_modes = intel_lvds_get_modes,
 	.mode_valid = intel_lvds_mode_valid,
@@ -512,7 +530,7 @@ static const struct drm_connector_funcs intel_lvds_connector_funcs = {
 	.fill_modes = drm_helper_probe_single_connector_modes,
 	.atomic_get_property = intel_digital_connector_atomic_get_property,
 	.atomic_set_property = intel_digital_connector_atomic_set_property,
-	.late_register = intel_connector_register,
+	.late_register = intel_lvds_connector_register,
 	.early_unregister = intel_connector_unregister,
 	.destroy = intel_connector_destroy,
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c
index 757b9ce7e3b1c..ab7caaa4f287f 100644
--- a/drivers/gpu/drm/i915/display/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
@@ -45,6 +45,7 @@
 #include "intel_connector.h"
 #include "intel_crtc.h"
 #include "intel_de.h"
+#include "intel_display_device.h"
 #include "intel_display_driver.h"
 #include "intel_display_types.h"
 #include "intel_fdi.h"
@@ -52,6 +53,7 @@
 #include "intel_gmbus.h"
 #include "intel_hdmi.h"
 #include "intel_hotplug.h"
+#include "intel_link_bw.h"
 #include "intel_panel.h"
 #include "intel_sdvo.h"
 #include "intel_sdvo_regs.h"
@@ -2502,12 +2504,28 @@ intel_sdvo_connector_duplicate_state(struct drm_connector *connector)
 	return &state->base.base;
 }
 
+static int intel_sdvo_connector_register(struct drm_connector *_connector)
+{
+	struct intel_connector *connector = to_intel_connector(_connector);
+	struct intel_display *display = to_intel_display(connector);
+	int err;
+
+	err = intel_connector_register(&connector->base);
+	if (err)
+		return err;
+
+	if (HAS_FDI(display))
+		intel_link_bw_connector_debugfs_add(connector);
+
+	return 0;
+}
+
 static const struct drm_connector_funcs intel_sdvo_connector_funcs = {
 	.detect = intel_sdvo_detect,
 	.fill_modes = drm_helper_probe_single_connector_modes,
 	.atomic_get_property = intel_sdvo_connector_atomic_get_property,
 	.atomic_set_property = intel_sdvo_connector_atomic_set_property,
-	.late_register = intel_connector_register,
+	.late_register = intel_sdvo_connector_register,
 	.early_unregister = intel_connector_unregister,
 	.destroy = intel_connector_destroy,
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
-- 
2.44.2

✗ Fi.CI.CHECKPATCH: warning for drm/i915/dp_mst: Add support for fractional link bpps

[Patchwork]

== Series Details ==

Series: drm/i915/dp_mst: Add support for fractional link bpps
URL   : https://patchwork.freedesktop.org/series/147435/
State : warning

== Summary ==

Error: dim checkpatch failed
f08afd36d304 drm/i915/dp: Use the correct connector while computing the link BPP limit on MST
6c7ff9d8593c drm/i915/dp: Limit max link bpp properly to a fractional value on SST
7a2a5566486a drm/i915/dp_mst: Add support for fractional link bpps on MST
-:148: WARNING:LONG_LINE: line length of 111 exceeds 100 columns
#148: FILE: drivers/gpu/drm/i915/display/intel_dp_mst.c:485:
+		    "DSC Sink supported compressed min bpp " FXP_Q4_FMT " compressed max bpp " FXP_Q4_FMT "\n",

-:158: WARNING:LONG_LINE: line length of 102 exceeds 100 columns
#158: FILE: drivers/gpu/drm/i915/display/intel_dp_mst.c:493:
+									     limits->link.min_bpp_x16,

-:161: WARNING:LONG_LINE: line length of 102 exceeds 100 columns
#161: FILE: drivers/gpu/drm/i915/display/intel_dp_mst.c:496:
+									     limits->link.max_bpp_x16,

total: 0 errors, 3 warnings, 0 checks, 132 lines checked
61e1c31de930 drm/i915/dp_mst: Enable fractional link bpps on MST
7ec4ccd5e32a drm/i915/display: Factor out intel_display_{min, max}_pipe_bpp()
-:29: CHECK:SPACING: spaces preferred around that '*' (ctx:VxV)
#29: FILE: drivers/gpu/drm/i915/display/intel_display.c:4329:
+		return 10*3;
 		         ^

-:31: CHECK:SPACING: spaces preferred around that '*' (ctx:VxV)
#31: FILE: drivers/gpu/drm/i915/display/intel_display.c:4331:
+		return 12*3;
 		         ^

-:33: CHECK:SPACING: spaces preferred around that '*' (ctx:VxV)
#33: FILE: drivers/gpu/drm/i915/display/intel_display.c:4333:
+		return 8*3;
 		        ^

total: 0 errors, 0 warnings, 3 checks, 58 lines checked
7d4c5599d79d drm/i915/dp: Export intel_dp_dsc_min_src_compressed_bpp()
230616707a6d drm/i915/dp: Use an intel_connector pointer everywhere
829b702655ca drm/i915/hdmi: Use an intel_connector pointer everywhere
831f98c2f714 drm/i915: Add support for forcing the link bpp on a connector
9ae2115128f0 drm/i915: Add force link bpp debugfs entry to connectors
-:70: CHECK:MACRO_ARG_REUSE: Macro argument reuse '__display' - possible side-effects?
#70: FILE: drivers/gpu/drm/i915/display/intel_display_device.h:174:
+#define HAS_FDI(__display)		(IS_DISPLAY_VER((__display), 5, 8) && !HAS_GMCH(__display))

total: 0 errors, 0 warnings, 1 checks, 191 lines checked

✗ Fi.CI.SPARSE: warning for drm/i915/dp_mst: Add support for fractional link bpps

[Patchwork]

== Series Details ==

Series: drm/i915/dp_mst: Add support for fractional link bpps
URL   : https://patchwork.freedesktop.org/series/147435/
State : warning

== Summary ==

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

✗ i915.CI.BAT: failure for drm/i915/dp_mst: Add support for fractional link bpps

[Patchwork]

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

== Series Details ==

Series: drm/i915/dp_mst: Add support for fractional link bpps
URL   : https://patchwork.freedesktop.org/series/147435/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_16387 -> Patchwork_147435v1
====================================================

Summary
-------

  **FAILURE**

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

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

Participating hosts (44 -> 43)
------------------------------

  Missing    (1): fi-snb-2520m 

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live@gem:
    - bat-arlh-2:         NOTRUN -> [ABORT][1]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_147435v1/bat-arlh-2/igt@i915_selftest@live@gem.html

  
#### Warnings ####

  * igt@i915_selftest@live:
    - bat-arlh-2:         [INCOMPLETE][2] -> [ABORT][3]
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16387/bat-arlh-2/igt@i915_selftest@live.html
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_147435v1/bat-arlh-2/igt@i915_selftest@live.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live:
    - bat-mtlp-8:         [PASS][4] -> [DMESG-FAIL][5] ([i915#12061]) +1 other test dmesg-fail
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16387/bat-mtlp-8/igt@i915_selftest@live.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_147435v1/bat-mtlp-8/igt@i915_selftest@live.html

  * igt@i915_selftest@live@workarounds:
    - bat-arls-5:         [PASS][6] -> [DMESG-FAIL][7] ([i915#12061]) +1 other test dmesg-fail
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16387/bat-arls-5/igt@i915_selftest@live@workarounds.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_147435v1/bat-arls-5/igt@i915_selftest@live@workarounds.html

  
#### Possible fixes ####

  * igt@i915_selftest@live@gt_pm:
    - bat-arlh-2:         [INCOMPLETE][8] -> [PASS][9]
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16387/bat-arlh-2/igt@i915_selftest@live@gt_pm.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_147435v1/bat-arlh-2/igt@i915_selftest@live@gt_pm.html

  * igt@i915_selftest@live@workarounds:
    - bat-arlh-3:         [DMESG-FAIL][10] ([i915#12061]) -> [PASS][11] +1 other test pass
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16387/bat-arlh-3/igt@i915_selftest@live@workarounds.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_147435v1/bat-arlh-3/igt@i915_selftest@live@workarounds.html
    - bat-mtlp-9:         [DMESG-FAIL][12] ([i915#12061]) -> [PASS][13] +1 other test pass
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16387/bat-mtlp-9/igt@i915_selftest@live@workarounds.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_147435v1/bat-mtlp-9/igt@i915_selftest@live@workarounds.html

  
  [i915#12061]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12061


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

  * Linux: CI_DRM_16387 -> Patchwork_147435v1

  CI-20190529: 20190529
  CI_DRM_16387: a49a4787e6bc70296204f4a6e1b0fed3759938cd @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_8311: 851a9c1cb1a690d8c527f26c49c250ec583af65e @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_147435v1: a49a4787e6bc70296204f4a6e1b0fed3759938cd @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

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

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

Re: [PATCH 09/10] drm/i915: Add support for forcing the link bpp on a connector

[kernel test robot]

Hi Imre,

kernel test robot noticed the following build errors:

[auto build test ERROR on next-20250408]
[also build test ERROR on v6.15-rc1]
[cannot apply to linus/master v6.15-rc1 v6.14 v6.14-rc7]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Imre-Deak/drm-i915-dp-Use-the-correct-connector-while-computing-the-link-BPP-limit-on-MST/20250409-055536
base:   next-20250408
patch link:    https://lore.kernel.org/r/20250408214342.1953197-10-imre.deak%40intel.com
patch subject: [PATCH 09/10] drm/i915: Add support for forcing the link bpp on a connector
config: i386-buildonly-randconfig-006-20250409 (https://download.01.org/0day-ci/archive/20250409/202504091033.XYeu9dAP-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250409/202504091033.XYeu9dAP-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202504091033.XYeu9dAP-lkp@intel.com/

All errors (new ones prefixed by >>, old ones prefixed by <<):

>> ERROR: modpost: "__udivdi3" [drivers/gpu/drm/i915/i915.ko] undefined!
ERROR: modpost: "__udivdi3" [drivers/gpu/drm/xe/xe.ko] undefined!

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 09/10] drm/i915: Add support for forcing the link bpp on a connector

[kernel test robot]

Hi Imre,

kernel test robot noticed the following build errors:

[auto build test ERROR on next-20250408]
[also build test ERROR on v6.15-rc1]
[cannot apply to linus/master v6.15-rc1 v6.14 v6.14-rc7]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Imre-Deak/drm-i915-dp-Use-the-correct-connector-while-computing-the-link-BPP-limit-on-MST/20250409-055536
base:   next-20250408
patch link:    https://lore.kernel.org/r/20250408214342.1953197-10-imre.deak%40intel.com
patch subject: [PATCH 09/10] drm/i915: Add support for forcing the link bpp on a connector
config: i386-buildonly-randconfig-001-20250409 (https://download.01.org/0day-ci/archive/20250409/202504091139.9chj3D5y-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250409/202504091139.9chj3D5y-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202504091139.9chj3D5y-lkp@intel.com/

All errors (new ones prefixed by >>):

   ld: drivers/gpu/drm/i915/display/intel_link_bw.o: in function `str_to_fxp_q4':
>> intel_link_bw.c:(.text+0x399): undefined reference to `__udivdi3'

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 10/10] drm/i915: Add force link bpp debugfs entry to connectors

[Jani Nikula]

On Wed, 09 Apr 2025, Imre Deak <imre.deak@intel.com> wrote:
> Add the debugfs entry to force a link bpp to all relevant connectors:
> all DP connectors and on an FDI link CRT/SDVO/LVDS/HDMI connectors.

This deviates from the current approach of intel_connector_register()
calling intel_connector_debugfs_add() which checks for connector types
and other conditions before registering debugfs files.

In many cases intel_connector_debugfs_add() unconditionally calls
feature specific debugfs functions such as
intel_hdcp_connector_debugfs_add() which then check the connector type.

I understand the motivation in this patch, being more object oriented
and all, but it's still a deviation. I prefer the same approach for
all. Currently it's obvious where all connector debugfs files get
registered. After this patch, it's not, and it's no longer clear cut
where connector debugfs files should be created.

Please add the connector type checks in
intel_link_bw_connector_debugfs_add() and call it from
intel_connector_debugfs_add().


BR,
Jani.


>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_crt.c      | 20 ++++++++++++++++++-
>  .../drm/i915/display/intel_display_device.h   |  1 +
>  drivers/gpu/drm/i915/display/intel_dp.c       |  3 +++
>  drivers/gpu/drm/i915/display/intel_dp_mst.c   |  2 ++
>  drivers/gpu/drm/i915/display/intel_hdmi.c     |  8 +++++++-
>  drivers/gpu/drm/i915/display/intel_lvds.c     | 20 ++++++++++++++++++-
>  drivers/gpu/drm/i915/display/intel_sdvo.c     | 20 ++++++++++++++++++-
>  7 files changed, 70 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_crt.c b/drivers/gpu/drm/i915/display/intel_crt.c
> index cca22d2402e88..69831d6f68912 100644
> --- a/drivers/gpu/drm/i915/display/intel_crt.c
> +++ b/drivers/gpu/drm/i915/display/intel_crt.c
> @@ -43,6 +43,7 @@
>  #include "intel_ddi.h"
>  #include "intel_ddi_buf_trans.h"
>  #include "intel_de.h"
> +#include "intel_display_device.h"
>  #include "intel_display_driver.h"
>  #include "intel_display_types.h"
>  #include "intel_fdi.h"
> @@ -51,6 +52,7 @@
>  #include "intel_gmbus.h"
>  #include "intel_hotplug.h"
>  #include "intel_hotplug_irq.h"
> +#include "intel_link_bw.h"
>  #include "intel_load_detect.h"
>  #include "intel_pch_display.h"
>  #include "intel_pch_refclk.h"
> @@ -986,13 +988,29 @@ void intel_crt_reset(struct drm_encoder *encoder)
>  
>  }
>  
> +static int intel_crt_connector_register(struct drm_connector *_connector)
> +{
> +	struct intel_connector *connector = to_intel_connector(_connector);
> +	struct intel_display *display = to_intel_display(connector);
> +	int err;
> +
> +	err = intel_connector_register(&connector->base);
> +	if (err)
> +		return err;
> +
> +	if (HAS_FDI(display))
> +		intel_link_bw_connector_debugfs_add(connector);
> +
> +	return 0;
> +}
> +
>  /*
>   * Routines for controlling stuff on the analog port
>   */
>  
>  static const struct drm_connector_funcs intel_crt_connector_funcs = {
>  	.fill_modes = drm_helper_probe_single_connector_modes,
> -	.late_register = intel_connector_register,
> +	.late_register = intel_crt_connector_register,
>  	.early_unregister = intel_connector_unregister,
>  	.destroy = intel_connector_destroy,
>  	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h b/drivers/gpu/drm/i915/display/intel_display_device.h
> index 368b0d3417c26..a84bdc83417f1 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_device.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_device.h
> @@ -171,6 +171,7 @@ struct intel_display_platforms {
>  #define HAS_GMBUS_BURST_READ(__display)	(DISPLAY_VER(__display) >= 10 || (__display)->platform.kabylake)
>  #define HAS_GMBUS_IRQ(__display)	(DISPLAY_VER(__display) >= 4)
>  #define HAS_GMCH(__display)		(DISPLAY_INFO(__display)->has_gmch)
> +#define HAS_FDI(__display)		(IS_DISPLAY_VER((__display), 5, 8) && !HAS_GMCH(__display))
>  #define HAS_HOTPLUG(__display)		(DISPLAY_INFO(__display)->has_hotplug)
>  #define HAS_HW_SAGV_WM(__display)	(DISPLAY_VER(__display) >= 13 && !(__display)->platform.dgfx)
>  #define HAS_IPC(__display)		(DISPLAY_INFO(__display)->has_ipc)
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 8ca33ebedce27..0b19a9b5adda5 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -80,6 +80,7 @@
>  #include "intel_hdmi.h"
>  #include "intel_hotplug.h"
>  #include "intel_hotplug_irq.h"
> +#include "intel_link_bw.h"
>  #include "intel_lspcon.h"
>  #include "intel_lvds.h"
>  #include "intel_modeset_lock.h"
> @@ -5890,6 +5891,8 @@ intel_dp_connector_register(struct drm_connector *_connector)
>  	if (ret)
>  		return ret;
>  
> +	intel_link_bw_connector_debugfs_add(connector);
> +
>  	drm_dbg_kms(display->drm, "registering %s bus for %s\n",
>  		    intel_dp->aux.name, connector->base.kdev->kobj.name);
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 35214d9a8c781..7508aa4e3695f 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -1445,6 +1445,8 @@ mst_connector_late_register(struct drm_connector *_connector)
>  	if (ret < 0)
>  		drm_dp_mst_connector_early_unregister(&connector->base, connector->mst.port);
>  
> +	intel_link_bw_connector_debugfs_add(connector);
> +
>  	return ret;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index 8f2cef36bdf79..0747ef8d6c0ca 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -52,6 +52,7 @@
>  #include "intel_cx0_phy.h"
>  #include "intel_ddi.h"
>  #include "intel_de.h"
> +#include "intel_display_device.h"
>  #include "intel_display_driver.h"
>  #include "intel_display_types.h"
>  #include "intel_dp.h"
> @@ -60,6 +61,7 @@
>  #include "intel_hdcp_regs.h"
>  #include "intel_hdcp_shim.h"
>  #include "intel_hdmi.h"
> +#include "intel_link_bw.h"
>  #include "intel_lspcon.h"
>  #include "intel_panel.h"
>  #include "intel_pfit.h"
> @@ -2611,13 +2613,17 @@ static int
>  intel_hdmi_connector_register(struct drm_connector *_connector)
>  {
>  	struct intel_connector *connector = to_intel_connector(_connector);
> +	struct intel_display *display = to_intel_display(connector);
>  	int ret;
>  
>  	ret = intel_connector_register(&connector->base);
>  	if (ret)
>  		return ret;
>  
> -	return ret;
> +	if (HAS_FDI(display))
> +		intel_link_bw_connector_debugfs_add(connector);
> +
> +	return 0;
>  }
>  
>  static void intel_hdmi_connector_unregister(struct drm_connector *_connector)
> diff --git a/drivers/gpu/drm/i915/display/intel_lvds.c b/drivers/gpu/drm/i915/display/intel_lvds.c
> index 89d26913e2539..3ac6aaa025434 100644
> --- a/drivers/gpu/drm/i915/display/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/display/intel_lvds.c
> @@ -45,10 +45,12 @@
>  #include "intel_backlight.h"
>  #include "intel_connector.h"
>  #include "intel_de.h"
> +#include "intel_display_device.h"
>  #include "intel_display_types.h"
>  #include "intel_dpll.h"
>  #include "intel_fdi.h"
>  #include "intel_gmbus.h"
> +#include "intel_link_bw.h"
>  #include "intel_lvds.h"
>  #include "intel_lvds_regs.h"
>  #include "intel_panel.h"
> @@ -501,6 +503,22 @@ static int intel_lvds_get_modes(struct drm_connector *_connector)
>  	return intel_panel_get_modes(connector);
>  }
>  
> +static int intel_lvds_connector_register(struct drm_connector *_connector)
> +{
> +	struct intel_connector *connector = to_intel_connector(_connector);
> +	struct intel_display *display = to_intel_display(connector);
> +	int err;
> +
> +	err = intel_connector_register(&connector->base);
> +	if (err)
> +		return err;
> +
> +	if (HAS_FDI(display))
> +		intel_link_bw_connector_debugfs_add(connector);
> +
> +	return 0;
> +}
> +
>  static const struct drm_connector_helper_funcs intel_lvds_connector_helper_funcs = {
>  	.get_modes = intel_lvds_get_modes,
>  	.mode_valid = intel_lvds_mode_valid,
> @@ -512,7 +530,7 @@ static const struct drm_connector_funcs intel_lvds_connector_funcs = {
>  	.fill_modes = drm_helper_probe_single_connector_modes,
>  	.atomic_get_property = intel_digital_connector_atomic_get_property,
>  	.atomic_set_property = intel_digital_connector_atomic_set_property,
> -	.late_register = intel_connector_register,
> +	.late_register = intel_lvds_connector_register,
>  	.early_unregister = intel_connector_unregister,
>  	.destroy = intel_connector_destroy,
>  	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c
> index 757b9ce7e3b1c..ab7caaa4f287f 100644
> --- a/drivers/gpu/drm/i915/display/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
> @@ -45,6 +45,7 @@
>  #include "intel_connector.h"
>  #include "intel_crtc.h"
>  #include "intel_de.h"
> +#include "intel_display_device.h"
>  #include "intel_display_driver.h"
>  #include "intel_display_types.h"
>  #include "intel_fdi.h"
> @@ -52,6 +53,7 @@
>  #include "intel_gmbus.h"
>  #include "intel_hdmi.h"
>  #include "intel_hotplug.h"
> +#include "intel_link_bw.h"
>  #include "intel_panel.h"
>  #include "intel_sdvo.h"
>  #include "intel_sdvo_regs.h"
> @@ -2502,12 +2504,28 @@ intel_sdvo_connector_duplicate_state(struct drm_connector *connector)
>  	return &state->base.base;
>  }
>  
> +static int intel_sdvo_connector_register(struct drm_connector *_connector)
> +{
> +	struct intel_connector *connector = to_intel_connector(_connector);
> +	struct intel_display *display = to_intel_display(connector);
> +	int err;
> +
> +	err = intel_connector_register(&connector->base);
> +	if (err)
> +		return err;
> +
> +	if (HAS_FDI(display))
> +		intel_link_bw_connector_debugfs_add(connector);
> +
> +	return 0;
> +}
> +
>  static const struct drm_connector_funcs intel_sdvo_connector_funcs = {
>  	.detect = intel_sdvo_detect,
>  	.fill_modes = drm_helper_probe_single_connector_modes,
>  	.atomic_get_property = intel_sdvo_connector_atomic_get_property,
>  	.atomic_set_property = intel_sdvo_connector_atomic_set_property,
> -	.late_register = intel_connector_register,
> +	.late_register = intel_sdvo_connector_register,
>  	.early_unregister = intel_connector_unregister,
>  	.destroy = intel_connector_destroy,
>  	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,

-- 
Jani Nikula, Intel

Re: [PATCH 07/10] drm/i915/dp: Use an intel_connector pointer everywhere

[Jani Nikula]

On Wed, 09 Apr 2025, Imre Deak <imre.deak@intel.com> wrote:
> Following the convention, convert intel_dp.c to use an intel_connector
> pointer everywhere, calling this pointer connector. If the intel
> connector must be casted from a drm_connector, call this pointer
> _connector and use this pointer only for the casting.
>
> Signed-off-by: Imre Deak <imre.deak@intel.com>


> @@ -5979,21 +5982,22 @@ static int intel_modeset_tile_group(struct intel_atomic_state *state,
>  {
>  	struct intel_display *display = to_intel_display(state);
>  	struct drm_connector_list_iter conn_iter;
> -	struct drm_connector *connector;
> +	struct drm_connector *_connector;
>  	int ret = 0;
>  
>  	drm_connector_list_iter_begin(display->drm, &conn_iter);
> -	drm_for_each_connector_iter(connector, &conn_iter) {
> +	drm_for_each_connector_iter(_connector, &conn_iter) {

for_each_intel_connector_iter()


-- 
Jani Nikula, Intel

Re: [PATCH 08/10] drm/i915/hdmi: Use an intel_connector pointer everywhere

[Jani Nikula]

On Wed, 09 Apr 2025, Imre Deak <imre.deak@intel.com> wrote:
> Following the convention, convert intel_hdmi.c to use an intel_connector
> pointer everywhere, calling this pointer connector. If the intel
> connector must be casted from a drm_connector, call this pointer
> _connector and use this pointer only for the casting.
>
> Signed-off-by: Imre Deak <imre.deak@intel.com>

> @@ -2076,14 +2081,16 @@ bool intel_hdmi_bpc_possible(const struct intel_crtc_state *crtc_state,
>  {
>  	struct drm_atomic_state *state = crtc_state->uapi.state;
>  	struct drm_connector_state *connector_state;
> -	struct drm_connector *connector;
> +	struct drm_connector *_connector;
>  	int i;
>  
> -	for_each_new_connector_in_state(state, connector, connector_state, i) {
> +	for_each_new_connector_in_state(state, _connector, connector_state, i) {

Can we use for_each_new_intel_connector_in_state() here?

BR,
Jani.


-- 
Jani Nikula, Intel

Re: [PATCH 10/10] drm/i915: Add force link bpp debugfs entry to connectors

[Imre Deak]

On Wed, Apr 09, 2025 at 11:16:58AM +0300, Jani Nikula wrote:
> On Wed, 09 Apr 2025, Imre Deak <imre.deak@intel.com> wrote:
> > Add the debugfs entry to force a link bpp to all relevant connectors:
> > all DP connectors and on an FDI link CRT/SDVO/LVDS/HDMI connectors.
> 
> This deviates from the current approach of intel_connector_register()
> calling intel_connector_debugfs_add() which checks for connector types
> and other conditions before registering debugfs files.
> 
> In many cases intel_connector_debugfs_add() unconditionally calls
> feature specific debugfs functions such as
> intel_hdcp_connector_debugfs_add() which then check the connector type.
> 
> I understand the motivation in this patch, being more object oriented
> and all, but it's still a deviation. I prefer the same approach for
> all. Currently it's obvious where all connector debugfs files get
> registered. After this patch, it's not, and it's no longer clear cut
> where connector debugfs files should be created.

I think the better approach is each connector adding the debugfs entries
relevant to them, even for the existing hdcp, pps, psr etc. entries. That
would avoid all the connector_type checks, replicated now in all the
*_connector_debugfs_add() functions. It's also odd to recheck the
connector type on a code path the connector type is already known to the
caller.

This approach would also match how the connector specific properties are
added.

> Please add the connector type checks in
> intel_link_bw_connector_debugfs_add() and call it from
> intel_connector_debugfs_add().
> 
> 
> BR,
> Jani.
> 
> 
> >
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_crt.c      | 20 ++++++++++++++++++-
> >  .../drm/i915/display/intel_display_device.h   |  1 +
> >  drivers/gpu/drm/i915/display/intel_dp.c       |  3 +++
> >  drivers/gpu/drm/i915/display/intel_dp_mst.c   |  2 ++
> >  drivers/gpu/drm/i915/display/intel_hdmi.c     |  8 +++++++-
> >  drivers/gpu/drm/i915/display/intel_lvds.c     | 20 ++++++++++++++++++-
> >  drivers/gpu/drm/i915/display/intel_sdvo.c     | 20 ++++++++++++++++++-
> >  7 files changed, 70 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_crt.c b/drivers/gpu/drm/i915/display/intel_crt.c
> > index cca22d2402e88..69831d6f68912 100644
> > --- a/drivers/gpu/drm/i915/display/intel_crt.c
> > +++ b/drivers/gpu/drm/i915/display/intel_crt.c
> > @@ -43,6 +43,7 @@
> >  #include "intel_ddi.h"
> >  #include "intel_ddi_buf_trans.h"
> >  #include "intel_de.h"
> > +#include "intel_display_device.h"
> >  #include "intel_display_driver.h"
> >  #include "intel_display_types.h"
> >  #include "intel_fdi.h"
> > @@ -51,6 +52,7 @@
> >  #include "intel_gmbus.h"
> >  #include "intel_hotplug.h"
> >  #include "intel_hotplug_irq.h"
> > +#include "intel_link_bw.h"
> >  #include "intel_load_detect.h"
> >  #include "intel_pch_display.h"
> >  #include "intel_pch_refclk.h"
> > @@ -986,13 +988,29 @@ void intel_crt_reset(struct drm_encoder *encoder)
> >  
> >  }
> >  
> > +static int intel_crt_connector_register(struct drm_connector *_connector)
> > +{
> > +	struct intel_connector *connector = to_intel_connector(_connector);
> > +	struct intel_display *display = to_intel_display(connector);
> > +	int err;
> > +
> > +	err = intel_connector_register(&connector->base);
> > +	if (err)
> > +		return err;
> > +
> > +	if (HAS_FDI(display))
> > +		intel_link_bw_connector_debugfs_add(connector);
> > +
> > +	return 0;
> > +}
> > +
> >  /*
> >   * Routines for controlling stuff on the analog port
> >   */
> >  
> >  static const struct drm_connector_funcs intel_crt_connector_funcs = {
> >  	.fill_modes = drm_helper_probe_single_connector_modes,
> > -	.late_register = intel_connector_register,
> > +	.late_register = intel_crt_connector_register,
> >  	.early_unregister = intel_connector_unregister,
> >  	.destroy = intel_connector_destroy,
> >  	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h b/drivers/gpu/drm/i915/display/intel_display_device.h
> > index 368b0d3417c26..a84bdc83417f1 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_device.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_device.h
> > @@ -171,6 +171,7 @@ struct intel_display_platforms {
> >  #define HAS_GMBUS_BURST_READ(__display)	(DISPLAY_VER(__display) >= 10 || (__display)->platform.kabylake)
> >  #define HAS_GMBUS_IRQ(__display)	(DISPLAY_VER(__display) >= 4)
> >  #define HAS_GMCH(__display)		(DISPLAY_INFO(__display)->has_gmch)
> > +#define HAS_FDI(__display)		(IS_DISPLAY_VER((__display), 5, 8) && !HAS_GMCH(__display))
> >  #define HAS_HOTPLUG(__display)		(DISPLAY_INFO(__display)->has_hotplug)
> >  #define HAS_HW_SAGV_WM(__display)	(DISPLAY_VER(__display) >= 13 && !(__display)->platform.dgfx)
> >  #define HAS_IPC(__display)		(DISPLAY_INFO(__display)->has_ipc)
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 8ca33ebedce27..0b19a9b5adda5 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -80,6 +80,7 @@
> >  #include "intel_hdmi.h"
> >  #include "intel_hotplug.h"
> >  #include "intel_hotplug_irq.h"
> > +#include "intel_link_bw.h"
> >  #include "intel_lspcon.h"
> >  #include "intel_lvds.h"
> >  #include "intel_modeset_lock.h"
> > @@ -5890,6 +5891,8 @@ intel_dp_connector_register(struct drm_connector *_connector)
> >  	if (ret)
> >  		return ret;
> >  
> > +	intel_link_bw_connector_debugfs_add(connector);
> > +
> >  	drm_dbg_kms(display->drm, "registering %s bus for %s\n",
> >  		    intel_dp->aux.name, connector->base.kdev->kobj.name);
> >  
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > index 35214d9a8c781..7508aa4e3695f 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > @@ -1445,6 +1445,8 @@ mst_connector_late_register(struct drm_connector *_connector)
> >  	if (ret < 0)
> >  		drm_dp_mst_connector_early_unregister(&connector->base, connector->mst.port);
> >  
> > +	intel_link_bw_connector_debugfs_add(connector);
> > +
> >  	return ret;
> >  }
> >  
> > diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> > index 8f2cef36bdf79..0747ef8d6c0ca 100644
> > --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> > @@ -52,6 +52,7 @@
> >  #include "intel_cx0_phy.h"
> >  #include "intel_ddi.h"
> >  #include "intel_de.h"
> > +#include "intel_display_device.h"
> >  #include "intel_display_driver.h"
> >  #include "intel_display_types.h"
> >  #include "intel_dp.h"
> > @@ -60,6 +61,7 @@
> >  #include "intel_hdcp_regs.h"
> >  #include "intel_hdcp_shim.h"
> >  #include "intel_hdmi.h"
> > +#include "intel_link_bw.h"
> >  #include "intel_lspcon.h"
> >  #include "intel_panel.h"
> >  #include "intel_pfit.h"
> > @@ -2611,13 +2613,17 @@ static int
> >  intel_hdmi_connector_register(struct drm_connector *_connector)
> >  {
> >  	struct intel_connector *connector = to_intel_connector(_connector);
> > +	struct intel_display *display = to_intel_display(connector);
> >  	int ret;
> >  
> >  	ret = intel_connector_register(&connector->base);
> >  	if (ret)
> >  		return ret;
> >  
> > -	return ret;
> > +	if (HAS_FDI(display))
> > +		intel_link_bw_connector_debugfs_add(connector);
> > +
> > +	return 0;
> >  }
> >  
> >  static void intel_hdmi_connector_unregister(struct drm_connector *_connector)
> > diff --git a/drivers/gpu/drm/i915/display/intel_lvds.c b/drivers/gpu/drm/i915/display/intel_lvds.c
> > index 89d26913e2539..3ac6aaa025434 100644
> > --- a/drivers/gpu/drm/i915/display/intel_lvds.c
> > +++ b/drivers/gpu/drm/i915/display/intel_lvds.c
> > @@ -45,10 +45,12 @@
> >  #include "intel_backlight.h"
> >  #include "intel_connector.h"
> >  #include "intel_de.h"
> > +#include "intel_display_device.h"
> >  #include "intel_display_types.h"
> >  #include "intel_dpll.h"
> >  #include "intel_fdi.h"
> >  #include "intel_gmbus.h"
> > +#include "intel_link_bw.h"
> >  #include "intel_lvds.h"
> >  #include "intel_lvds_regs.h"
> >  #include "intel_panel.h"
> > @@ -501,6 +503,22 @@ static int intel_lvds_get_modes(struct drm_connector *_connector)
> >  	return intel_panel_get_modes(connector);
> >  }
> >  
> > +static int intel_lvds_connector_register(struct drm_connector *_connector)
> > +{
> > +	struct intel_connector *connector = to_intel_connector(_connector);
> > +	struct intel_display *display = to_intel_display(connector);
> > +	int err;
> > +
> > +	err = intel_connector_register(&connector->base);
> > +	if (err)
> > +		return err;
> > +
> > +	if (HAS_FDI(display))
> > +		intel_link_bw_connector_debugfs_add(connector);
> > +
> > +	return 0;
> > +}
> > +
> >  static const struct drm_connector_helper_funcs intel_lvds_connector_helper_funcs = {
> >  	.get_modes = intel_lvds_get_modes,
> >  	.mode_valid = intel_lvds_mode_valid,
> > @@ -512,7 +530,7 @@ static const struct drm_connector_funcs intel_lvds_connector_funcs = {
> >  	.fill_modes = drm_helper_probe_single_connector_modes,
> >  	.atomic_get_property = intel_digital_connector_atomic_get_property,
> >  	.atomic_set_property = intel_digital_connector_atomic_set_property,
> > -	.late_register = intel_connector_register,
> > +	.late_register = intel_lvds_connector_register,
> >  	.early_unregister = intel_connector_unregister,
> >  	.destroy = intel_connector_destroy,
> >  	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> > diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c
> > index 757b9ce7e3b1c..ab7caaa4f287f 100644
> > --- a/drivers/gpu/drm/i915/display/intel_sdvo.c
> > +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
> > @@ -45,6 +45,7 @@
> >  #include "intel_connector.h"
> >  #include "intel_crtc.h"
> >  #include "intel_de.h"
> > +#include "intel_display_device.h"
> >  #include "intel_display_driver.h"
> >  #include "intel_display_types.h"
> >  #include "intel_fdi.h"
> > @@ -52,6 +53,7 @@
> >  #include "intel_gmbus.h"
> >  #include "intel_hdmi.h"
> >  #include "intel_hotplug.h"
> > +#include "intel_link_bw.h"
> >  #include "intel_panel.h"
> >  #include "intel_sdvo.h"
> >  #include "intel_sdvo_regs.h"
> > @@ -2502,12 +2504,28 @@ intel_sdvo_connector_duplicate_state(struct drm_connector *connector)
> >  	return &state->base.base;
> >  }
> >  
> > +static int intel_sdvo_connector_register(struct drm_connector *_connector)
> > +{
> > +	struct intel_connector *connector = to_intel_connector(_connector);
> > +	struct intel_display *display = to_intel_display(connector);
> > +	int err;
> > +
> > +	err = intel_connector_register(&connector->base);
> > +	if (err)
> > +		return err;
> > +
> > +	if (HAS_FDI(display))
> > +		intel_link_bw_connector_debugfs_add(connector);
> > +
> > +	return 0;
> > +}
> > +
> >  static const struct drm_connector_funcs intel_sdvo_connector_funcs = {
> >  	.detect = intel_sdvo_detect,
> >  	.fill_modes = drm_helper_probe_single_connector_modes,
> >  	.atomic_get_property = intel_sdvo_connector_atomic_get_property,
> >  	.atomic_set_property = intel_sdvo_connector_atomic_set_property,
> > -	.late_register = intel_connector_register,
> > +	.late_register = intel_sdvo_connector_register,
> >  	.early_unregister = intel_connector_unregister,
> >  	.destroy = intel_connector_destroy,
> >  	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> 
> -- 
> Jani Nikula, Intel

Re: [PATCH 07/10] drm/i915/dp: Use an intel_connector pointer everywhere

[Imre Deak]

On Wed, Apr 09, 2025 at 11:26:46AM +0300, Jani Nikula wrote:
> On Wed, 09 Apr 2025, Imre Deak <imre.deak@intel.com> wrote:
> > Following the convention, convert intel_dp.c to use an intel_connector
> > pointer everywhere, calling this pointer connector. If the intel
> > connector must be casted from a drm_connector, call this pointer
> > _connector and use this pointer only for the casting.
> >
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> 
> 
> > @@ -5979,21 +5982,22 @@ static int intel_modeset_tile_group(struct intel_atomic_state *state,
> >  {
> >  	struct intel_display *display = to_intel_display(state);
> >  	struct drm_connector_list_iter conn_iter;
> > -	struct drm_connector *connector;
> > +	struct drm_connector *_connector;
> >  	int ret = 0;
> >  
> >  	drm_connector_list_iter_begin(display->drm, &conn_iter);
> > -	drm_for_each_connector_iter(connector, &conn_iter) {
> > +	drm_for_each_connector_iter(_connector, &conn_iter) {
> 
> for_each_intel_connector_iter()

Thanks, will change this.

> 
> 
> -- 
> Jani Nikula, Intel

Re: [PATCH 10/10] drm/i915: Add force link bpp debugfs entry to connectors

[Jani Nikula]

On Wed, 09 Apr 2025, Imre Deak <imre.deak@intel.com> wrote:
> On Wed, Apr 09, 2025 at 11:16:58AM +0300, Jani Nikula wrote:
>> On Wed, 09 Apr 2025, Imre Deak <imre.deak@intel.com> wrote:
>> > Add the debugfs entry to force a link bpp to all relevant connectors:
>> > all DP connectors and on an FDI link CRT/SDVO/LVDS/HDMI connectors.
>> 
>> This deviates from the current approach of intel_connector_register()
>> calling intel_connector_debugfs_add() which checks for connector types
>> and other conditions before registering debugfs files.
>> 
>> In many cases intel_connector_debugfs_add() unconditionally calls
>> feature specific debugfs functions such as
>> intel_hdcp_connector_debugfs_add() which then check the connector type.
>> 
>> I understand the motivation in this patch, being more object oriented
>> and all, but it's still a deviation. I prefer the same approach for
>> all. Currently it's obvious where all connector debugfs files get
>> registered. After this patch, it's not, and it's no longer clear cut
>> where connector debugfs files should be created.
>
> I think the better approach is each connector adding the debugfs entries
> relevant to them, even for the existing hdcp, pps, psr etc. entries. That
> would avoid all the connector_type checks, replicated now in all the
> *_connector_debugfs_add() functions. It's also odd to recheck the
> connector type on a code path the connector type is already known to the
> caller.

I'm not saying that's not a valid argument. And I said I understand the
motivation.

I just don't want this done for a single debugfs entry in a series about
something completely different, essentially leaving behind a mix of two
entirely different approaches.

In the past switching to your proposed approach wasn't really even an
option because everything was still in intel_display_debugfs.c. We've
gradually moved away from that. And arguably the work of moving the
debugfs next to the functionality should be completed first, before
starting another refactoring. intel_connector_debugfs_add() should just
be a function calling intel_*_connector_debugfs_add() functions instead
of having inlined debugfs creation.

BR,
Jani.



>
> This approach would also match how the connector specific properties are
> added.
>
>> Please add the connector type checks in
>> intel_link_bw_connector_debugfs_add() and call it from
>> intel_connector_debugfs_add().
>> 
>> 
>> BR,
>> Jani.
>> 
>> 
>> >
>> > Signed-off-by: Imre Deak <imre.deak@intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/display/intel_crt.c      | 20 ++++++++++++++++++-
>> >  .../drm/i915/display/intel_display_device.h   |  1 +
>> >  drivers/gpu/drm/i915/display/intel_dp.c       |  3 +++
>> >  drivers/gpu/drm/i915/display/intel_dp_mst.c   |  2 ++
>> >  drivers/gpu/drm/i915/display/intel_hdmi.c     |  8 +++++++-
>> >  drivers/gpu/drm/i915/display/intel_lvds.c     | 20 ++++++++++++++++++-
>> >  drivers/gpu/drm/i915/display/intel_sdvo.c     | 20 ++++++++++++++++++-
>> >  7 files changed, 70 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/display/intel_crt.c b/drivers/gpu/drm/i915/display/intel_crt.c
>> > index cca22d2402e88..69831d6f68912 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_crt.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_crt.c
>> > @@ -43,6 +43,7 @@
>> >  #include "intel_ddi.h"
>> >  #include "intel_ddi_buf_trans.h"
>> >  #include "intel_de.h"
>> > +#include "intel_display_device.h"
>> >  #include "intel_display_driver.h"
>> >  #include "intel_display_types.h"
>> >  #include "intel_fdi.h"
>> > @@ -51,6 +52,7 @@
>> >  #include "intel_gmbus.h"
>> >  #include "intel_hotplug.h"
>> >  #include "intel_hotplug_irq.h"
>> > +#include "intel_link_bw.h"
>> >  #include "intel_load_detect.h"
>> >  #include "intel_pch_display.h"
>> >  #include "intel_pch_refclk.h"
>> > @@ -986,13 +988,29 @@ void intel_crt_reset(struct drm_encoder *encoder)
>> >  
>> >  }
>> >  
>> > +static int intel_crt_connector_register(struct drm_connector *_connector)
>> > +{
>> > +	struct intel_connector *connector = to_intel_connector(_connector);
>> > +	struct intel_display *display = to_intel_display(connector);
>> > +	int err;
>> > +
>> > +	err = intel_connector_register(&connector->base);
>> > +	if (err)
>> > +		return err;
>> > +
>> > +	if (HAS_FDI(display))
>> > +		intel_link_bw_connector_debugfs_add(connector);
>> > +
>> > +	return 0;
>> > +}
>> > +
>> >  /*
>> >   * Routines for controlling stuff on the analog port
>> >   */
>> >  
>> >  static const struct drm_connector_funcs intel_crt_connector_funcs = {
>> >  	.fill_modes = drm_helper_probe_single_connector_modes,
>> > -	.late_register = intel_connector_register,
>> > +	.late_register = intel_crt_connector_register,
>> >  	.early_unregister = intel_connector_unregister,
>> >  	.destroy = intel_connector_destroy,
>> >  	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>> > diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h b/drivers/gpu/drm/i915/display/intel_display_device.h
>> > index 368b0d3417c26..a84bdc83417f1 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_display_device.h
>> > +++ b/drivers/gpu/drm/i915/display/intel_display_device.h
>> > @@ -171,6 +171,7 @@ struct intel_display_platforms {
>> >  #define HAS_GMBUS_BURST_READ(__display)	(DISPLAY_VER(__display) >= 10 || (__display)->platform.kabylake)
>> >  #define HAS_GMBUS_IRQ(__display)	(DISPLAY_VER(__display) >= 4)
>> >  #define HAS_GMCH(__display)		(DISPLAY_INFO(__display)->has_gmch)
>> > +#define HAS_FDI(__display)		(IS_DISPLAY_VER((__display), 5, 8) && !HAS_GMCH(__display))
>> >  #define HAS_HOTPLUG(__display)		(DISPLAY_INFO(__display)->has_hotplug)
>> >  #define HAS_HW_SAGV_WM(__display)	(DISPLAY_VER(__display) >= 13 && !(__display)->platform.dgfx)
>> >  #define HAS_IPC(__display)		(DISPLAY_INFO(__display)->has_ipc)
>> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
>> > index 8ca33ebedce27..0b19a9b5adda5 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> > @@ -80,6 +80,7 @@
>> >  #include "intel_hdmi.h"
>> >  #include "intel_hotplug.h"
>> >  #include "intel_hotplug_irq.h"
>> > +#include "intel_link_bw.h"
>> >  #include "intel_lspcon.h"
>> >  #include "intel_lvds.h"
>> >  #include "intel_modeset_lock.h"
>> > @@ -5890,6 +5891,8 @@ intel_dp_connector_register(struct drm_connector *_connector)
>> >  	if (ret)
>> >  		return ret;
>> >  
>> > +	intel_link_bw_connector_debugfs_add(connector);
>> > +
>> >  	drm_dbg_kms(display->drm, "registering %s bus for %s\n",
>> >  		    intel_dp->aux.name, connector->base.kdev->kobj.name);
>> >  
>> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
>> > index 35214d9a8c781..7508aa4e3695f 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
>> > @@ -1445,6 +1445,8 @@ mst_connector_late_register(struct drm_connector *_connector)
>> >  	if (ret < 0)
>> >  		drm_dp_mst_connector_early_unregister(&connector->base, connector->mst.port);
>> >  
>> > +	intel_link_bw_connector_debugfs_add(connector);
>> > +
>> >  	return ret;
>> >  }
>> >  
>> > diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
>> > index 8f2cef36bdf79..0747ef8d6c0ca 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
>> > @@ -52,6 +52,7 @@
>> >  #include "intel_cx0_phy.h"
>> >  #include "intel_ddi.h"
>> >  #include "intel_de.h"
>> > +#include "intel_display_device.h"
>> >  #include "intel_display_driver.h"
>> >  #include "intel_display_types.h"
>> >  #include "intel_dp.h"
>> > @@ -60,6 +61,7 @@
>> >  #include "intel_hdcp_regs.h"
>> >  #include "intel_hdcp_shim.h"
>> >  #include "intel_hdmi.h"
>> > +#include "intel_link_bw.h"
>> >  #include "intel_lspcon.h"
>> >  #include "intel_panel.h"
>> >  #include "intel_pfit.h"
>> > @@ -2611,13 +2613,17 @@ static int
>> >  intel_hdmi_connector_register(struct drm_connector *_connector)
>> >  {
>> >  	struct intel_connector *connector = to_intel_connector(_connector);
>> > +	struct intel_display *display = to_intel_display(connector);
>> >  	int ret;
>> >  
>> >  	ret = intel_connector_register(&connector->base);
>> >  	if (ret)
>> >  		return ret;
>> >  
>> > -	return ret;
>> > +	if (HAS_FDI(display))
>> > +		intel_link_bw_connector_debugfs_add(connector);
>> > +
>> > +	return 0;
>> >  }
>> >  
>> >  static void intel_hdmi_connector_unregister(struct drm_connector *_connector)
>> > diff --git a/drivers/gpu/drm/i915/display/intel_lvds.c b/drivers/gpu/drm/i915/display/intel_lvds.c
>> > index 89d26913e2539..3ac6aaa025434 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_lvds.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_lvds.c
>> > @@ -45,10 +45,12 @@
>> >  #include "intel_backlight.h"
>> >  #include "intel_connector.h"
>> >  #include "intel_de.h"
>> > +#include "intel_display_device.h"
>> >  #include "intel_display_types.h"
>> >  #include "intel_dpll.h"
>> >  #include "intel_fdi.h"
>> >  #include "intel_gmbus.h"
>> > +#include "intel_link_bw.h"
>> >  #include "intel_lvds.h"
>> >  #include "intel_lvds_regs.h"
>> >  #include "intel_panel.h"
>> > @@ -501,6 +503,22 @@ static int intel_lvds_get_modes(struct drm_connector *_connector)
>> >  	return intel_panel_get_modes(connector);
>> >  }
>> >  
>> > +static int intel_lvds_connector_register(struct drm_connector *_connector)
>> > +{
>> > +	struct intel_connector *connector = to_intel_connector(_connector);
>> > +	struct intel_display *display = to_intel_display(connector);
>> > +	int err;
>> > +
>> > +	err = intel_connector_register(&connector->base);
>> > +	if (err)
>> > +		return err;
>> > +
>> > +	if (HAS_FDI(display))
>> > +		intel_link_bw_connector_debugfs_add(connector);
>> > +
>> > +	return 0;
>> > +}
>> > +
>> >  static const struct drm_connector_helper_funcs intel_lvds_connector_helper_funcs = {
>> >  	.get_modes = intel_lvds_get_modes,
>> >  	.mode_valid = intel_lvds_mode_valid,
>> > @@ -512,7 +530,7 @@ static const struct drm_connector_funcs intel_lvds_connector_funcs = {
>> >  	.fill_modes = drm_helper_probe_single_connector_modes,
>> >  	.atomic_get_property = intel_digital_connector_atomic_get_property,
>> >  	.atomic_set_property = intel_digital_connector_atomic_set_property,
>> > -	.late_register = intel_connector_register,
>> > +	.late_register = intel_lvds_connector_register,
>> >  	.early_unregister = intel_connector_unregister,
>> >  	.destroy = intel_connector_destroy,
>> >  	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>> > diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c
>> > index 757b9ce7e3b1c..ab7caaa4f287f 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_sdvo.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
>> > @@ -45,6 +45,7 @@
>> >  #include "intel_connector.h"
>> >  #include "intel_crtc.h"
>> >  #include "intel_de.h"
>> > +#include "intel_display_device.h"
>> >  #include "intel_display_driver.h"
>> >  #include "intel_display_types.h"
>> >  #include "intel_fdi.h"
>> > @@ -52,6 +53,7 @@
>> >  #include "intel_gmbus.h"
>> >  #include "intel_hdmi.h"
>> >  #include "intel_hotplug.h"
>> > +#include "intel_link_bw.h"
>> >  #include "intel_panel.h"
>> >  #include "intel_sdvo.h"
>> >  #include "intel_sdvo_regs.h"
>> > @@ -2502,12 +2504,28 @@ intel_sdvo_connector_duplicate_state(struct drm_connector *connector)
>> >  	return &state->base.base;
>> >  }
>> >  
>> > +static int intel_sdvo_connector_register(struct drm_connector *_connector)
>> > +{
>> > +	struct intel_connector *connector = to_intel_connector(_connector);
>> > +	struct intel_display *display = to_intel_display(connector);
>> > +	int err;
>> > +
>> > +	err = intel_connector_register(&connector->base);
>> > +	if (err)
>> > +		return err;
>> > +
>> > +	if (HAS_FDI(display))
>> > +		intel_link_bw_connector_debugfs_add(connector);
>> > +
>> > +	return 0;
>> > +}
>> > +
>> >  static const struct drm_connector_funcs intel_sdvo_connector_funcs = {
>> >  	.detect = intel_sdvo_detect,
>> >  	.fill_modes = drm_helper_probe_single_connector_modes,
>> >  	.atomic_get_property = intel_sdvo_connector_atomic_get_property,
>> >  	.atomic_set_property = intel_sdvo_connector_atomic_set_property,
>> > -	.late_register = intel_connector_register,
>> > +	.late_register = intel_sdvo_connector_register,
>> >  	.early_unregister = intel_connector_unregister,
>> >  	.destroy = intel_connector_destroy,
>> >  	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>> 
>> -- 
>> Jani Nikula, Intel

-- 
Jani Nikula, Intel

Re: [PATCH 08/10] drm/i915/hdmi: Use an intel_connector pointer everywhere

[Imre Deak]

On Wed, Apr 09, 2025 at 11:29:30AM +0300, Jani Nikula wrote:
> On Wed, 09 Apr 2025, Imre Deak <imre.deak@intel.com> wrote:
> > Following the convention, convert intel_hdmi.c to use an intel_connector
> > pointer everywhere, calling this pointer connector. If the intel
> > connector must be casted from a drm_connector, call this pointer
> > _connector and use this pointer only for the casting.
> >
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> 
> > @@ -2076,14 +2081,16 @@ bool intel_hdmi_bpc_possible(const struct intel_crtc_state *crtc_state,
> >  {
> >  	struct drm_atomic_state *state = crtc_state->uapi.state;
> >  	struct drm_connector_state *connector_state;
> > -	struct drm_connector *connector;
> > +	struct drm_connector *_connector;
> >  	int i;
> >  
> > -	for_each_new_connector_in_state(state, connector, connector_state, i) {
> > +	for_each_new_connector_in_state(state, _connector, connector_state, i) {
> 
> Can we use for_each_new_intel_connector_in_state() here?

Ok, can use it.

It depends on 'base' being the first field in
intel_digital_connector_state, I think that assumption is acceptable
(and used elsewhere already).

> 
> BR,
> Jani.
> 
> 
> -- 
> Jani Nikula, Intel

Re: [PATCH 10/10] drm/i915: Add force link bpp debugfs entry to connectors

[Imre Deak]

On Wed, Apr 09, 2025 at 05:00:18PM +0300, Jani Nikula wrote:
> On Wed, 09 Apr 2025, Imre Deak <imre.deak@intel.com> wrote:
> > On Wed, Apr 09, 2025 at 11:16:58AM +0300, Jani Nikula wrote:
> >> On Wed, 09 Apr 2025, Imre Deak <imre.deak@intel.com> wrote:
> >> > Add the debugfs entry to force a link bpp to all relevant connectors:
> >> > all DP connectors and on an FDI link CRT/SDVO/LVDS/HDMI connectors.
> >> 
> >> This deviates from the current approach of intel_connector_register()
> >> calling intel_connector_debugfs_add() which checks for connector types
> >> and other conditions before registering debugfs files.
> >> 
> >> In many cases intel_connector_debugfs_add() unconditionally calls
> >> feature specific debugfs functions such as
> >> intel_hdcp_connector_debugfs_add() which then check the connector type.
> >> 
> >> I understand the motivation in this patch, being more object oriented
> >> and all, but it's still a deviation. I prefer the same approach for
> >> all. Currently it's obvious where all connector debugfs files get
> >> registered. After this patch, it's not, and it's no longer clear cut
> >> where connector debugfs files should be created.
> >
> > I think the better approach is each connector adding the debugfs entries
> > relevant to them, even for the existing hdcp, pps, psr etc. entries. That
> > would avoid all the connector_type checks, replicated now in all the
> > *_connector_debugfs_add() functions. It's also odd to recheck the
> > connector type on a code path the connector type is already known to the
> > caller.
> 
> I'm not saying that's not a valid argument. And I said I understand the
> motivation.
> 
> I just don't want this done for a single debugfs entry in a series about
> something completely different, essentially leaving behind a mix of two
> entirely different approaches.
> 
> In the past switching to your proposed approach wasn't really even an
> option because everything was still in intel_display_debugfs.c. We've
> gradually moved away from that. And arguably the work of moving the
> debugfs next to the functionality should be completed first, before
> starting another refactoring. intel_connector_debugfs_add() should just
> be a function calling intel_*_connector_debugfs_add() functions instead
> of having inlined debugfs creation.

I don't agree that the new entry added in this patch couldn't be added
in the correct way to begin with, also making it easier to move adding
the rest of the entries the same way as well; but will change it.

> BR,
> Jani.
> 
> 
> 
> >
> > This approach would also match how the connector specific properties are
> > added.
> >
> >> Please add the connector type checks in
> >> intel_link_bw_connector_debugfs_add() and call it from
> >> intel_connector_debugfs_add().
> >> 
> >> 
> >> BR,
> >> Jani.
> >> 
> >> 
> >> >
> >> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> >> > ---
> >> >  drivers/gpu/drm/i915/display/intel_crt.c      | 20 ++++++++++++++++++-
> >> >  .../drm/i915/display/intel_display_device.h   |  1 +
> >> >  drivers/gpu/drm/i915/display/intel_dp.c       |  3 +++
> >> >  drivers/gpu/drm/i915/display/intel_dp_mst.c   |  2 ++
> >> >  drivers/gpu/drm/i915/display/intel_hdmi.c     |  8 +++++++-
> >> >  drivers/gpu/drm/i915/display/intel_lvds.c     | 20 ++++++++++++++++++-
> >> >  drivers/gpu/drm/i915/display/intel_sdvo.c     | 20 ++++++++++++++++++-
> >> >  7 files changed, 70 insertions(+), 4 deletions(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/display/intel_crt.c b/drivers/gpu/drm/i915/display/intel_crt.c
> >> > index cca22d2402e88..69831d6f68912 100644
> >> > --- a/drivers/gpu/drm/i915/display/intel_crt.c
> >> > +++ b/drivers/gpu/drm/i915/display/intel_crt.c
> >> > @@ -43,6 +43,7 @@
> >> >  #include "intel_ddi.h"
> >> >  #include "intel_ddi_buf_trans.h"
> >> >  #include "intel_de.h"
> >> > +#include "intel_display_device.h"
> >> >  #include "intel_display_driver.h"
> >> >  #include "intel_display_types.h"
> >> >  #include "intel_fdi.h"
> >> > @@ -51,6 +52,7 @@
> >> >  #include "intel_gmbus.h"
> >> >  #include "intel_hotplug.h"
> >> >  #include "intel_hotplug_irq.h"
> >> > +#include "intel_link_bw.h"
> >> >  #include "intel_load_detect.h"
> >> >  #include "intel_pch_display.h"
> >> >  #include "intel_pch_refclk.h"
> >> > @@ -986,13 +988,29 @@ void intel_crt_reset(struct drm_encoder *encoder)
> >> >  
> >> >  }
> >> >  
> >> > +static int intel_crt_connector_register(struct drm_connector *_connector)
> >> > +{
> >> > +	struct intel_connector *connector = to_intel_connector(_connector);
> >> > +	struct intel_display *display = to_intel_display(connector);
> >> > +	int err;
> >> > +
> >> > +	err = intel_connector_register(&connector->base);
> >> > +	if (err)
> >> > +		return err;
> >> > +
> >> > +	if (HAS_FDI(display))
> >> > +		intel_link_bw_connector_debugfs_add(connector);
> >> > +
> >> > +	return 0;
> >> > +}
> >> > +
> >> >  /*
> >> >   * Routines for controlling stuff on the analog port
> >> >   */
> >> >  
> >> >  static const struct drm_connector_funcs intel_crt_connector_funcs = {
> >> >  	.fill_modes = drm_helper_probe_single_connector_modes,
> >> > -	.late_register = intel_connector_register,
> >> > +	.late_register = intel_crt_connector_register,
> >> >  	.early_unregister = intel_connector_unregister,
> >> >  	.destroy = intel_connector_destroy,
> >> >  	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> >> > diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h b/drivers/gpu/drm/i915/display/intel_display_device.h
> >> > index 368b0d3417c26..a84bdc83417f1 100644
> >> > --- a/drivers/gpu/drm/i915/display/intel_display_device.h
> >> > +++ b/drivers/gpu/drm/i915/display/intel_display_device.h
> >> > @@ -171,6 +171,7 @@ struct intel_display_platforms {
> >> >  #define HAS_GMBUS_BURST_READ(__display)	(DISPLAY_VER(__display) >= 10 || (__display)->platform.kabylake)
> >> >  #define HAS_GMBUS_IRQ(__display)	(DISPLAY_VER(__display) >= 4)
> >> >  #define HAS_GMCH(__display)		(DISPLAY_INFO(__display)->has_gmch)
> >> > +#define HAS_FDI(__display)		(IS_DISPLAY_VER((__display), 5, 8) && !HAS_GMCH(__display))
> >> >  #define HAS_HOTPLUG(__display)		(DISPLAY_INFO(__display)->has_hotplug)
> >> >  #define HAS_HW_SAGV_WM(__display)	(DISPLAY_VER(__display) >= 13 && !(__display)->platform.dgfx)
> >> >  #define HAS_IPC(__display)		(DISPLAY_INFO(__display)->has_ipc)
> >> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> >> > index 8ca33ebedce27..0b19a9b5adda5 100644
> >> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> >> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> >> > @@ -80,6 +80,7 @@
> >> >  #include "intel_hdmi.h"
> >> >  #include "intel_hotplug.h"
> >> >  #include "intel_hotplug_irq.h"
> >> > +#include "intel_link_bw.h"
> >> >  #include "intel_lspcon.h"
> >> >  #include "intel_lvds.h"
> >> >  #include "intel_modeset_lock.h"
> >> > @@ -5890,6 +5891,8 @@ intel_dp_connector_register(struct drm_connector *_connector)
> >> >  	if (ret)
> >> >  		return ret;
> >> >  
> >> > +	intel_link_bw_connector_debugfs_add(connector);
> >> > +
> >> >  	drm_dbg_kms(display->drm, "registering %s bus for %s\n",
> >> >  		    intel_dp->aux.name, connector->base.kdev->kobj.name);
> >> >  
> >> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> >> > index 35214d9a8c781..7508aa4e3695f 100644
> >> > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> >> > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> >> > @@ -1445,6 +1445,8 @@ mst_connector_late_register(struct drm_connector *_connector)
> >> >  	if (ret < 0)
> >> >  		drm_dp_mst_connector_early_unregister(&connector->base, connector->mst.port);
> >> >  
> >> > +	intel_link_bw_connector_debugfs_add(connector);
> >> > +
> >> >  	return ret;
> >> >  }
> >> >  
> >> > diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> >> > index 8f2cef36bdf79..0747ef8d6c0ca 100644
> >> > --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> >> > +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> >> > @@ -52,6 +52,7 @@
> >> >  #include "intel_cx0_phy.h"
> >> >  #include "intel_ddi.h"
> >> >  #include "intel_de.h"
> >> > +#include "intel_display_device.h"
> >> >  #include "intel_display_driver.h"
> >> >  #include "intel_display_types.h"
> >> >  #include "intel_dp.h"
> >> > @@ -60,6 +61,7 @@
> >> >  #include "intel_hdcp_regs.h"
> >> >  #include "intel_hdcp_shim.h"
> >> >  #include "intel_hdmi.h"
> >> > +#include "intel_link_bw.h"
> >> >  #include "intel_lspcon.h"
> >> >  #include "intel_panel.h"
> >> >  #include "intel_pfit.h"
> >> > @@ -2611,13 +2613,17 @@ static int
> >> >  intel_hdmi_connector_register(struct drm_connector *_connector)
> >> >  {
> >> >  	struct intel_connector *connector = to_intel_connector(_connector);
> >> > +	struct intel_display *display = to_intel_display(connector);
> >> >  	int ret;
> >> >  
> >> >  	ret = intel_connector_register(&connector->base);
> >> >  	if (ret)
> >> >  		return ret;
> >> >  
> >> > -	return ret;
> >> > +	if (HAS_FDI(display))
> >> > +		intel_link_bw_connector_debugfs_add(connector);
> >> > +
> >> > +	return 0;
> >> >  }
> >> >  
> >> >  static void intel_hdmi_connector_unregister(struct drm_connector *_connector)
> >> > diff --git a/drivers/gpu/drm/i915/display/intel_lvds.c b/drivers/gpu/drm/i915/display/intel_lvds.c
> >> > index 89d26913e2539..3ac6aaa025434 100644
> >> > --- a/drivers/gpu/drm/i915/display/intel_lvds.c
> >> > +++ b/drivers/gpu/drm/i915/display/intel_lvds.c
> >> > @@ -45,10 +45,12 @@
> >> >  #include "intel_backlight.h"
> >> >  #include "intel_connector.h"
> >> >  #include "intel_de.h"
> >> > +#include "intel_display_device.h"
> >> >  #include "intel_display_types.h"
> >> >  #include "intel_dpll.h"
> >> >  #include "intel_fdi.h"
> >> >  #include "intel_gmbus.h"
> >> > +#include "intel_link_bw.h"
> >> >  #include "intel_lvds.h"
> >> >  #include "intel_lvds_regs.h"
> >> >  #include "intel_panel.h"
> >> > @@ -501,6 +503,22 @@ static int intel_lvds_get_modes(struct drm_connector *_connector)
> >> >  	return intel_panel_get_modes(connector);
> >> >  }
> >> >  
> >> > +static int intel_lvds_connector_register(struct drm_connector *_connector)
> >> > +{
> >> > +	struct intel_connector *connector = to_intel_connector(_connector);
> >> > +	struct intel_display *display = to_intel_display(connector);
> >> > +	int err;
> >> > +
> >> > +	err = intel_connector_register(&connector->base);
> >> > +	if (err)
> >> > +		return err;
> >> > +
> >> > +	if (HAS_FDI(display))
> >> > +		intel_link_bw_connector_debugfs_add(connector);
> >> > +
> >> > +	return 0;
> >> > +}
> >> > +
> >> >  static const struct drm_connector_helper_funcs intel_lvds_connector_helper_funcs = {
> >> >  	.get_modes = intel_lvds_get_modes,
> >> >  	.mode_valid = intel_lvds_mode_valid,
> >> > @@ -512,7 +530,7 @@ static const struct drm_connector_funcs intel_lvds_connector_funcs = {
> >> >  	.fill_modes = drm_helper_probe_single_connector_modes,
> >> >  	.atomic_get_property = intel_digital_connector_atomic_get_property,
> >> >  	.atomic_set_property = intel_digital_connector_atomic_set_property,
> >> > -	.late_register = intel_connector_register,
> >> > +	.late_register = intel_lvds_connector_register,
> >> >  	.early_unregister = intel_connector_unregister,
> >> >  	.destroy = intel_connector_destroy,
> >> >  	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> >> > diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c
> >> > index 757b9ce7e3b1c..ab7caaa4f287f 100644
> >> > --- a/drivers/gpu/drm/i915/display/intel_sdvo.c
> >> > +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
> >> > @@ -45,6 +45,7 @@
> >> >  #include "intel_connector.h"
> >> >  #include "intel_crtc.h"
> >> >  #include "intel_de.h"
> >> > +#include "intel_display_device.h"
> >> >  #include "intel_display_driver.h"
> >> >  #include "intel_display_types.h"
> >> >  #include "intel_fdi.h"
> >> > @@ -52,6 +53,7 @@
> >> >  #include "intel_gmbus.h"
> >> >  #include "intel_hdmi.h"
> >> >  #include "intel_hotplug.h"
> >> > +#include "intel_link_bw.h"
> >> >  #include "intel_panel.h"
> >> >  #include "intel_sdvo.h"
> >> >  #include "intel_sdvo_regs.h"
> >> > @@ -2502,12 +2504,28 @@ intel_sdvo_connector_duplicate_state(struct drm_connector *connector)
> >> >  	return &state->base.base;
> >> >  }
> >> >  
> >> > +static int intel_sdvo_connector_register(struct drm_connector *_connector)
> >> > +{
> >> > +	struct intel_connector *connector = to_intel_connector(_connector);
> >> > +	struct intel_display *display = to_intel_display(connector);
> >> > +	int err;
> >> > +
> >> > +	err = intel_connector_register(&connector->base);
> >> > +	if (err)
> >> > +		return err;
> >> > +
> >> > +	if (HAS_FDI(display))
> >> > +		intel_link_bw_connector_debugfs_add(connector);
> >> > +
> >> > +	return 0;
> >> > +}
> >> > +
> >> >  static const struct drm_connector_funcs intel_sdvo_connector_funcs = {
> >> >  	.detect = intel_sdvo_detect,
> >> >  	.fill_modes = drm_helper_probe_single_connector_modes,
> >> >  	.atomic_get_property = intel_sdvo_connector_atomic_get_property,
> >> >  	.atomic_set_property = intel_sdvo_connector_atomic_set_property,
> >> > -	.late_register = intel_connector_register,
> >> > +	.late_register = intel_sdvo_connector_register,
> >> >  	.early_unregister = intel_connector_unregister,
> >> >  	.destroy = intel_connector_destroy,
> >> >  	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> >> 
> >> -- 
> >> Jani Nikula, Intel
> 
> -- 
> Jani Nikula, Intel