drm/i915/dp: link config compute refactoring

We haven't used the DP bw constants here for a while. No functional changes.

Signed-off-by Jani Nikula <[email protected]>

---
 drivers/gpu/drm/i915/intel_dp.c | 1 -
 1 file changed, 1 deletion(-)

drivers/gpu/drm/i915/intel_dp.c +0 -1

index 62f82c4298ac..5f4b30faf6a2 100644
@@ -1701,7 +1701,6 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 	int lane_count, clock;
 	int min_lane_count = 1;
 	int max_lane_count = intel_dp_max_lane_count(intel_dp);
-	/* Conveniently, the link BW constants become indices with a shift...*/
 	int min_clock = 0;
 	int max_clock;
 	int bpp, mode_rate;

Signature · 1 line

2.11.0

The debug prints make more sense where the results are actually used, and this cleans up extra clutter from the already overcrowded intel_dp_compute_config().

Signed-off-by Jani Nikula <[email protected]>

---
 drivers/gpu/drm/i915/intel_dp.c               | 9 ++-------
 drivers/gpu/drm/i915/intel_dp_link_training.c | 5 +++++
 2 files changed, 7 insertions(+), 7 deletions(-)

drivers/gpu/drm/i915/intel_dp.c +0 -1

index 5f4b30faf6a2..81cf363e71af 100644
@@ -1706,7 +1706,6 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 	int bpp, mode_rate;
 	int link_avail, link_clock;
 	int common_len;
-	uint8_t link_bw, rate_select;
 	bool reduce_m_n = drm_dp_has_quirk(&intel_dp->desc,
 					   DP_DPCD_QUIRK_LIMITED_M_N);
 

drivers/gpu/drm/i915/intel_dp.c +2 -4

-
-	DRM_DEBUG_KMS("DP link bw %02x rate select %02x lane count %d clock %d bpp %d\n",
-		      link_bw, rate_select, pipe_config->lane_count,
-		      pipe_config->port_clock, bpp);
+	DRM_DEBUG_KMS("DP lane count %d clock %d bpp %d\n",
+		      pipe_config->lane_count, pipe_config->port_clock, bpp);
 	DRM_DEBUG_KMS("DP link bw required %i available %i\n",
 		      mode_rate, link_avail);
 

drivers/gpu/drm/i915/intel_dp_link_training.c +5 -0

index f59b59bb0a21..3fcaa98b9055 100644
@@ -139,6 +139,11 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp)
 	intel_dp_compute_rate(intel_dp, intel_dp->link_rate,
 			      &link_bw, &rate_select);
 
+	if (link_bw)
+		DRM_DEBUG_KMS("Using LINK_BW_SET value %02x\n", link_bw);
+	else
+		DRM_DEBUG_KMS("Using LINK_RATE_SET value %02x\n", rate_select);
+
 	/* Write the link configuration data */
 	link_config[0] = link_bw;
 	link_config[1] = intel_dp->lane_count;

Signature · 1 line

2.11.0

We haven't used the DP bw constants here for a while. No functional changes.

Signed-off-by Jani Nikula <[email protected]>

---
 drivers/gpu/drm/i915/intel_dp.c | 1 -
 1 file changed, 1 deletion(-)

drivers/gpu/drm/i915/intel_dp.c +0 -1

index 62f82c4298ac..5f4b30faf6a2 100644
@@ -1701,7 +1701,6 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 	int lane_count, clock;
 	int min_lane_count = 1;
 	int max_lane_count = intel_dp_max_lane_count(intel_dp);
-	/* Conveniently, the link BW constants become indices with a shift...*/
 	int min_clock = 0;
 	int max_clock;
 	int bpp, mode_rate;

Signature · 1 line

2.11.0

Quoted 26 lines · Jani Nikula

The debug prints make more sense where the results are actually used, and this cleans up extra clutter from the already overcrowded intel_dp_compute_config().

Signed-off-by Jani Nikula <[email protected]>

---
 drivers/gpu/drm/i915/intel_dp.c               | 9 ++-------
 drivers/gpu/drm/i915/intel_dp_link_training.c | 5 +++++
 2 files changed, 7 insertions(+), 7 deletions(-)

drivers/gpu/drm/i915/intel_dp.c +0 -1

index 5f4b30faf6a2..81cf363e71af 100644
@@ -1706,7 +1706,6 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 	int bpp, mode_rate;
 	int link_avail, link_clock;
 	int common_len;
-	uint8_t link_bw, rate_select;
 	bool reduce_m_n = drm_dp_has_quirk(&intel_dp->desc,
 					   DP_DPCD_QUIRK_LIMITED_M_N);
 

@@ -1852,12 +1851,8 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 	pipe_config->pipe_bpp = bpp;
 	pipe_config->port_clock = intel_dp->common_rates[clock];
 

drivers/gpu/drm/i915/intel_dp.c intel_dp_compute_config(struct intel_encoder *encoder, · line 1856

Quoted 2 lines

drivers/gpu/drm/i915/intel_dp.c +0 -2

-	intel_dp_compute_rate(intel_dp, pipe_config->port_clock,
-			      &link_bw, &rate_select);

the commit message state more about the debug message itself, but by removing this call here it seems that you change the behavior of eDP. So I couldn't follow the changes you are actually aiming to do here.

Quoted 17 lines

drivers/gpu/drm/i915/intel_dp.c +2 -4

-
-	DRM_DEBUG_KMS("DP link bw %02x rate select %02x lane count %d clock %d bpp %d\n",
-		      link_bw, rate_select, pipe_config->lane_count,
-		      pipe_config->port_clock, bpp);
+	DRM_DEBUG_KMS("DP lane count %d clock %d bpp %d\n",
+		      pipe_config->lane_count, pipe_config->port_clock, bpp);
 	DRM_DEBUG_KMS("DP link bw required %i available %i\n",
 		      mode_rate, link_avail);
 

drivers/gpu/drm/i915/intel_dp_link_training.c +0 -0

index f59b59bb0a21..3fcaa98b9055 100644
@@ -139,6 +139,11 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp)
 	intel_dp_compute_rate(intel_dp, intel_dp->link_rate,
 			      &link_bw, &rate_select);
 

Quoted 11 lines

drivers/gpu/drm/i915/intel_dp_link_training.c +5 -0

+	if (link_bw)
+		DRM_DEBUG_KMS("Using LINK_BW_SET value %02x\n", link_bw);
+	else
+		DRM_DEBUG_KMS("Using LINK_RATE_SET value %02x\n", rate_select);
+
 	/* Write the link configuration data */
 	link_config[0] = link_bw;
 	link_config[1] = intel_dp->lane_count;

Signature · 1 line

2.11.0

Keep related things together. No functional changes.

Signed-off-by Jani Nikula <[email protected]>

---

 drivers/gpu/drm/i915/intel_dp.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

drivers/gpu/drm/i915/intel_dp.c +13 -11

index 19fe5eb8d32a..dd42e0422af6 100644
@@ -1650,6 +1650,8 @@ void intel_dp_compute_rate(struct intel_dp *intel_dp, int port_clock,
 static int intel_dp_compute_bpp(struct intel_dp *intel_dp,
 				struct intel_crtc_state *pipe_config)
 {
+	struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
+	struct intel_connector *intel_connector = intel_dp->attached_connector;
 	int bpp, bpc;
 
 	bpp = pipe_config->pipe_bpp;

@@ -1665,6 +1667,17 @@ static int intel_dp_compute_bpp(struct intel_dp *intel_dp,
 		DRM_DEBUG_KMS("Setting pipe_bpp to %d\n",
 			      pipe_config->pipe_bpp);
 	}
+
+	if (intel_dp_is_edp(intel_dp)) {
+		/* Get bpp from vbt only for panels that dont have bpp in edid */
+		if (intel_connector->base.display_info.bpc == 0 &&
+			(dev_priv->vbt.edp.bpp && dev_priv->vbt.edp.bpp < bpp)) {
+			DRM_DEBUG_KMS("clamping bpp for eDP panel to BIOS-provided %i\n",
+				      dev_priv->vbt.edp.bpp);
+			bpp = dev_priv->vbt.edp.bpp;
+		}
+	}
+
 	return bpp;
 }
 

@@ -1689,10 +1702,8 @@ static bool
 intel_dp_compute_link_config(struct intel_encoder *encoder,
 			     struct intel_crtc_state *pipe_config)
 {
-	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
 	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
-	struct intel_connector *intel_connector = intel_dp->attached_connector;
 	int lane_count, clock;
 	int min_lane_count = 1;
 	int max_lane_count = intel_dp_max_lane_count(intel_dp);

@@ -1735,15 +1746,6 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
 	 * bpc in between. */
 	bpp = intel_dp_compute_bpp(intel_dp, pipe_config);
 	if (intel_dp_is_edp(intel_dp)) {
-
-		/* Get bpp from vbt only for panels that dont have bpp in edid */
-		if (intel_connector->base.display_info.bpc == 0 &&
-			(dev_priv->vbt.edp.bpp && dev_priv->vbt.edp.bpp < bpp)) {
-			DRM_DEBUG_KMS("clamping bpp for eDP panel to BIOS-provided %i\n",
-				      dev_priv->vbt.edp.bpp);
-			bpp = dev_priv->vbt.edp.bpp;
-		}
-
 		/*
 		 * Use the maximum clock and number of lanes the eDP panel
 		 * advertizes being capable of. The panels are generally

Signature · 1 line

2.11.0

For now, there's just the one link config selection, optimizing for slow and wide link. No functional changes.

Signed-off-by Jani Nikula <[email protected]>

---
 drivers/gpu/drm/i915/intel_dp.c | 81 +++++++++++++++++++++++++----------------
 1 file changed, 50 insertions(+), 31 deletions(-)

drivers/gpu/drm/i915/intel_dp.c +42 -29

index 3c5fbdf42b9b..c98626b3af65 100644
@@ -1704,6 +1704,42 @@ static bool intel_edp_compare_alt_mode(struct drm_display_mode *m1,
 	return bres;
 }
 
+/* Optimize link config in order: max bpp, min clock, min lanes */
+static bool
+intel_dp_compute_link_config_wide(struct intel_dp *intel_dp,
+				  struct intel_crtc_state *pipe_config,
+				  const struct link_config_limits *limits)
+{
+	struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
+	int bpp, clock, lane_count;
+	int mode_rate, link_clock, link_avail;
+
+	for (bpp = limits->max_bpp; bpp >= limits->min_bpp; bpp -= 2 * 3) {
+		mode_rate = intel_dp_link_required(adjusted_mode->crtc_clock,
+						   bpp);
+
+		for (clock = limits->min_clock; clock <= limits->max_clock; clock++) {
+			for (lane_count = limits->min_lane_count;
+			     lane_count <= limits->max_lane_count;
+			     lane_count <<= 1) {
+				link_clock = intel_dp->common_rates[clock];
+				link_avail = intel_dp_max_data_rate(link_clock,
+								    lane_count);
+
+				if (mode_rate <= link_avail) {
+					pipe_config->lane_count = lane_count;
+					pipe_config->pipe_bpp = bpp;
+					pipe_config->port_clock = link_clock;
+
+					return true;
+				}
+			}
+		}
+	}
+
+	return false;
+}
+
 static bool
 intel_dp_compute_link_config(struct intel_encoder *encoder,
 			     struct intel_crtc_state *pipe_config)

@@ -1711,8 +1747,6 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
 	struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
 	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
 	struct link_config_limits limits;
-	int bpp, clock, lane_count;
-	int mode_rate, link_avail, link_clock;
 	int common_len;
 
 	common_len = intel_dp_common_len_rate_limit(intel_dp,

@@ -1766,37 +1800,22 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
 		      intel_dp->common_rates[limits.max_clock],
 		      limits.max_bpp, adjusted_mode->crtc_clock);
 
-	for (bpp = limits.max_bpp; bpp >= limits.min_bpp; bpp -= 2 * 3) {
-		mode_rate = intel_dp_link_required(adjusted_mode->crtc_clock,
-						   bpp);
-
-		for (clock = limits.min_clock; clock <= limits.max_clock; clock++) {
-			for (lane_count = limits.min_lane_count;
-			     lane_count <= limits.max_lane_count;
-			     lane_count <<= 1) {
-
-				link_clock = intel_dp->common_rates[clock];
-				link_avail = intel_dp_max_data_rate(link_clock,
-								    lane_count);
-
-				if (mode_rate <= link_avail) {
-					goto found;
-				}
-			}
-		}
-	}
-
-	return false;
-
-found:
-	pipe_config->lane_count = lane_count;
-	pipe_config->pipe_bpp = bpp;
-	pipe_config->port_clock = intel_dp->common_rates[clock];
+	/*
+	 * Optimize for slow and wide. This is the place to add alternative
+	 * optimization policy.
+	 */
+	if (!intel_dp_compute_link_config_wide(intel_dp, pipe_config, &limits))
+		return false;
 
 	DRM_DEBUG_KMS("DP lane count %d clock %d bpp %d\n",
-		      pipe_config->lane_count, pipe_config->port_clock, bpp);

drivers/gpu/drm/i915/intel_dp.c +0 -0

 	return true;
 }

Signature · 1 line

2.11.0

Abstract compliance test adjustments to a single function. Also make the bpc adjustments affect the limits, actually forcing the bpc. Seems like directly changing the pipe_bpp in the past could not have been effective.

Signed-off-by Jani Nikula <[email protected]>

---
 drivers/gpu/drm/i915/intel_dp.c | 64 ++++++++++++++++++++++++-----------------
 1 file changed, 38 insertions(+), 26 deletions(-)

drivers/gpu/drm/i915/intel_dp.c +38 -26

index c98626b3af65..4ddb9dc61f46 100644
@@ -1666,14 +1666,6 @@ static int intel_dp_compute_bpp(struct intel_dp *intel_dp,
 	if (bpc > 0)
 		bpp = min(bpp, 3*bpc);
 
-	/* For DP Compliance we override the computed bpp for the pipe */
-	if (intel_dp->compliance.test_data.bpc != 0) {
-		pipe_config->pipe_bpp =	3*intel_dp->compliance.test_data.bpc;
-		pipe_config->dither_force_disable = pipe_config->pipe_bpp == 6*3;
-		DRM_DEBUG_KMS("Setting pipe_bpp to %d\n",
-			      pipe_config->pipe_bpp);
-	}
-
 	if (intel_dp_is_edp(intel_dp)) {
 		/* Get bpp from vbt only for panels that dont have bpp in edid */
 		if (intel_connector->base.display_info.bpc == 0 &&

@@ -1704,6 +1696,42 @@ static bool intel_edp_compare_alt_mode(struct drm_display_mode *m1,
 	return bres;
 }
 
+/* Adjust link config limits based on compliance test requests. */
+static void
+intel_dp_adjust_compliance_config(struct intel_dp *intel_dp,
+				  struct intel_crtc_state *pipe_config,
+				  struct link_config_limits *limits)
+{
+	/* For DP Compliance we override the computed bpp for the pipe */
+	if (intel_dp->compliance.test_data.bpc != 0) {
+		int bpp = 3 * intel_dp->compliance.test_data.bpc;
+
+		limits->min_bpp = limits->max_bpp = bpp;
+		pipe_config->dither_force_disable = bpp == 6 * 3;
+
+		DRM_DEBUG_KMS("Setting pipe_bpp to %d\n", bpp);
+	}
+
+	/* Use values requested by Compliance Test Request */
+	if (intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING) {
+		int index;
+
+		/* Validate the compliance test data since max values
+		 * might have changed due to link train fallback.
+		 */
+		if (intel_dp_link_params_valid(intel_dp, intel_dp->compliance.test_link_rate,
+					       intel_dp->compliance.test_lane_count)) {
+			index = intel_dp_rate_index(intel_dp->common_rates,
+						    intel_dp->num_common_rates,
+						    intel_dp->compliance.test_link_rate);
+			if (index >= 0)
+				limits->min_clock = limits->max_clock = index;
+			limits->min_lane_count = limits->max_lane_count =
+				intel_dp->compliance.test_lane_count;
+		}
+	}
+}
+
 /* Optimize link config in order: max bpp, min clock, min lanes */
 static bool
 intel_dp_compute_link_config_wide(struct intel_dp *intel_dp,

@@ -1764,24 +1792,6 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
 	limits.min_bpp = 6 * 3;
 	limits.max_bpp = intel_dp_compute_bpp(intel_dp, pipe_config);
 
-	/* Use values requested by Compliance Test Request */
-	if (intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING) {
-		int index;
-
-		/* Validate the compliance test data since max values
-		 * might have changed due to link train fallback.
-		 */
-		if (intel_dp_link_params_valid(intel_dp, intel_dp->compliance.test_link_rate,
-					       intel_dp->compliance.test_lane_count)) {
-			index = intel_dp_rate_index(intel_dp->common_rates,
-						    intel_dp->num_common_rates,
-						    intel_dp->compliance.test_link_rate);
-			if (index >= 0)
-				limits.min_clock = limits.max_clock = index;
-			limits.min_lane_count = limits.max_lane_count = intel_dp->compliance.test_lane_count;
-		}
-	}
-
 	if (intel_dp_is_edp(intel_dp)) {
 		/*
 		 * Use the maximum clock and number of lanes the eDP panel

@@ -1794,6 +1804,8 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
 		limits.min_clock = limits.max_clock;
 	}
 
+	intel_dp_adjust_compliance_config(intel_dp, pipe_config, &limits);
+
 	DRM_DEBUG_KMS("DP link computation with max lane count %i "
 		      "max rate %d max bpp %d pixel clock %iKHz\n",
 		      limits.max_lane_count,

Signature · 1 line

2.11.0

Quoted 110 lines · Jani Nikula

For now, there's just the one link config selection, optimizing for slow and wide link. No functional changes.

Signed-off-by Jani Nikula <[email protected]>

---
 drivers/gpu/drm/i915/intel_dp.c | 81 +++++++++++++++++++++++++----------------
 1 file changed, 50 insertions(+), 31 deletions(-)

drivers/gpu/drm/i915/intel_dp.c +47 -31

index 3c5fbdf42b9b..c98626b3af65 100644
@@ -1704,6 +1704,42 @@ static bool intel_edp_compare_alt_mode(struct drm_display_mode *m1,
 	return bres;
 }
 
+/* Optimize link config in order: max bpp, min clock, min lanes */
+static bool
+intel_dp_compute_link_config_wide(struct intel_dp *intel_dp,
+				  struct intel_crtc_state *pipe_config,
+				  const struct link_config_limits *limits)
+{
+	struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
+	int bpp, clock, lane_count;
+	int mode_rate, link_clock, link_avail;
+
+	for (bpp = limits->max_bpp; bpp >= limits->min_bpp; bpp -= 2 * 3) {
+		mode_rate = intel_dp_link_required(adjusted_mode->crtc_clock,
+						   bpp);
+
+		for (clock = limits->min_clock; clock <= limits->max_clock; clock++) {
+			for (lane_count = limits->min_lane_count;
+			     lane_count <= limits->max_lane_count;
+			     lane_count <<= 1) {
+				link_clock = intel_dp->common_rates[clock];
+				link_avail = intel_dp_max_data_rate(link_clock,
+								    lane_count);
+
+				if (mode_rate <= link_avail) {
+					pipe_config->lane_count = lane_count;
+					pipe_config->pipe_bpp = bpp;
+					pipe_config->port_clock = link_clock;
+
+					return true;
+				}
+			}
+		}
+	}
+
+	return false;
+}
+
 static bool
 intel_dp_compute_link_config(struct intel_encoder *encoder,
 			     struct intel_crtc_state *pipe_config)

@@ -1711,8 +1747,6 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
 	struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
 	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
 	struct link_config_limits limits;
-	int bpp, clock, lane_count;
-	int mode_rate, link_avail, link_clock;
 	int common_len;
 
 	common_len = intel_dp_common_len_rate_limit(intel_dp,

@@ -1766,37 +1800,22 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
 		      intel_dp->common_rates[limits.max_clock],
 		      limits.max_bpp, adjusted_mode->crtc_clock);
 
-	for (bpp = limits.max_bpp; bpp >= limits.min_bpp; bpp -= 2 * 3) {
-		mode_rate = intel_dp_link_required(adjusted_mode->crtc_clock,
-						   bpp);
-
-		for (clock = limits.min_clock; clock <= limits.max_clock; clock++) {
-			for (lane_count = limits.min_lane_count;
-			     lane_count <= limits.max_lane_count;
-			     lane_count <<= 1) {
-
-				link_clock = intel_dp->common_rates[clock];
-				link_avail = intel_dp_max_data_rate(link_clock,
-								    lane_count);
-
-				if (mode_rate <= link_avail) {
-					goto found;
-				}
-			}
-		}
-	}
-
-	return false;
-
-found:
-	pipe_config->lane_count = lane_count;
-	pipe_config->pipe_bpp = bpp;
-	pipe_config->port_clock = intel_dp->common_rates[clock];
+	/*
+	 * Optimize for slow and wide. This is the place to add alternative
+	 * optimization policy.
+	 */
+	if (!intel_dp_compute_link_config_wide(intel_dp, pipe_config, &limits))
+		return false;
 
 	DRM_DEBUG_KMS("DP lane count %d clock %d bpp %d\n",
-		      pipe_config->lane_count, pipe_config->port_clock, bpp);
-	DRM_DEBUG_KMS("DP link bw required %i available %i\n",
-		      mode_rate, link_avail);
+		      pipe_config->lane_count, pipe_config->port_clock,
+		      pipe_config->pipe_bpp);
+
+	DRM_DEBUG_KMS("DP link rate required %i available %i\n",
+		      intel_dp_link_required(adjusted_mode->crtc_clock,

drivers/gpu/drm/i915/intel_dp.c intel_dp_compute_link_config(struct intel_encoder *encoder, · line 1818

Quoted 3 lines

drivers/gpu/drm/i915/intel_dp.c +3 -0

+					     pipe_config->pipe_bpp),
+		      intel_dp_max_data_rate(pipe_config->port_clock,
+					     pipe_config->lane_count));

Wouldnt it be better if we move this Debug message about Available and required link rate in the other function right after the condition if (mode_rate <= link_avail) is true, before returning true from that function. I think it will be just more intuitive there.

Everything else looks good. So

Reviewed-by Manasi Navare <[email protected]>

Manasi

Quoted 6 lines

drivers/gpu/drm/i915/intel_dp.c +0 -0

 	return true;
 }

Signature · 1 line

2.11.0

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Quoted 111 lines · Manasi Navare

Quoted 110 lines · Jani Nikula

For now, there's just the one link config selection, optimizing for slow and wide link. No functional changes.

Signed-off-by Jani Nikula <[email protected]>

---
 drivers/gpu/drm/i915/intel_dp.c | 81 +++++++++++++++++++++++++----------------
 1 file changed, 50 insertions(+), 31 deletions(-)

drivers/gpu/drm/i915/intel_dp.c +47 -31

index 3c5fbdf42b9b..c98626b3af65 100644
@@ -1704,6 +1704,42 @@ static bool intel_edp_compare_alt_mode(struct drm_display_mode *m1,
 	return bres;
 }
 
+/* Optimize link config in order: max bpp, min clock, min lanes */
+static bool
+intel_dp_compute_link_config_wide(struct intel_dp *intel_dp,
+				  struct intel_crtc_state *pipe_config,
+				  const struct link_config_limits *limits)
+{
+	struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
+	int bpp, clock, lane_count;
+	int mode_rate, link_clock, link_avail;
+
+	for (bpp = limits->max_bpp; bpp >= limits->min_bpp; bpp -= 2 * 3) {
+		mode_rate = intel_dp_link_required(adjusted_mode->crtc_clock,
+						   bpp);
+
+		for (clock = limits->min_clock; clock <= limits->max_clock; clock++) {
+			for (lane_count = limits->min_lane_count;
+			     lane_count <= limits->max_lane_count;
+			     lane_count <<= 1) {
+				link_clock = intel_dp->common_rates[clock];
+				link_avail = intel_dp_max_data_rate(link_clock,
+								    lane_count);
+
+				if (mode_rate <= link_avail) {
+					pipe_config->lane_count = lane_count;
+					pipe_config->pipe_bpp = bpp;
+					pipe_config->port_clock = link_clock;
+
+					return true;
+				}
+			}
+		}
+	}
+
+	return false;
+}
+
 static bool
 intel_dp_compute_link_config(struct intel_encoder *encoder,
 			     struct intel_crtc_state *pipe_config)

@@ -1711,8 +1747,6 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
 	struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
 	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
 	struct link_config_limits limits;
-	int bpp, clock, lane_count;
-	int mode_rate, link_avail, link_clock;
 	int common_len;
 
 	common_len = intel_dp_common_len_rate_limit(intel_dp,

@@ -1766,37 +1800,22 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
 		      intel_dp->common_rates[limits.max_clock],
 		      limits.max_bpp, adjusted_mode->crtc_clock);
 
-	for (bpp = limits.max_bpp; bpp >= limits.min_bpp; bpp -= 2 * 3) {
-		mode_rate = intel_dp_link_required(adjusted_mode->crtc_clock,
-						   bpp);
-
-		for (clock = limits.min_clock; clock <= limits.max_clock; clock++) {
-			for (lane_count = limits.min_lane_count;
-			     lane_count <= limits.max_lane_count;
-			     lane_count <<= 1) {
-
-				link_clock = intel_dp->common_rates[clock];
-				link_avail = intel_dp_max_data_rate(link_clock,
-								    lane_count);
-
-				if (mode_rate <= link_avail) {
-					goto found;
-				}
-			}
-		}
-	}
-
-	return false;
-
-found:
-	pipe_config->lane_count = lane_count;
-	pipe_config->pipe_bpp = bpp;
-	pipe_config->port_clock = intel_dp->common_rates[clock];
+	/*
+	 * Optimize for slow and wide. This is the place to add alternative
+	 * optimization policy.
+	 */
+	if (!intel_dp_compute_link_config_wide(intel_dp, pipe_config, &limits))
+		return false;
 
 	DRM_DEBUG_KMS("DP lane count %d clock %d bpp %d\n",
-		      pipe_config->lane_count, pipe_config->port_clock, bpp);
-	DRM_DEBUG_KMS("DP link bw required %i available %i\n",
-		      mode_rate, link_avail);
+		      pipe_config->lane_count, pipe_config->port_clock,
+		      pipe_config->pipe_bpp);
+
+	DRM_DEBUG_KMS("DP link rate required %i available %i\n",
+		      intel_dp_link_required(adjusted_mode->crtc_clock,

Quoted 8 lines

drivers/gpu/drm/i915/intel_dp.c intel_dp_compute_link_config(struct intel_encoder *encoder, · line 1818

Quoted 3 lines

drivers/gpu/drm/i915/intel_dp.c +3 -0

+					     pipe_config->pipe_bpp),
+		      intel_dp_max_data_rate(pipe_config->port_clock,
+					     pipe_config->lane_count));

Wouldnt it be better if we move this Debug message about Available and required link rate in the other function right after the condition if (mode_rate <= link_avail) is true, before returning true from that function. I think it will be just more intuitive there.

That's what I thought too at first, but then realized if we're going to add an alternative call to an alternative approach, and perhaps yet another for DSC, the debugging gets duplicated in all of them.

BR,
Jani.

Quoted 14 lines

Everything else looks good. So

Reviewed-by Manasi Navare <[email protected]>

Manasi

Quoted 6 lines

drivers/gpu/drm/i915/intel_dp.c +0 -0

 	return true;
 }

Signature · 1 line

2.11.0

Quoted 4 lines

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Signature · 1 line

Jani Nikula, Intel Open Source Technology Center

Abstract a new intel_dp_compute_link_config() from intel_dp_compute_config(), with the parts related to link configuration, i.e. bpp, link rate, and lane count selection. No functional changes.

Signed-off-by Jani Nikula <[email protected]>

---
 drivers/gpu/drm/i915/intel_dp.c | 160 ++++++++++++++++++++++------------------
 1 file changed, 87 insertions(+), 73 deletions(-)

drivers/gpu/drm/i915/intel_dp.c +87 -73

index 81cf363e71af..19fe5eb8d32a 100644
@@ -1685,19 +1685,14 @@ static bool intel_edp_compare_alt_mode(struct drm_display_mode *m1,
 	return bres;
 }
 
-bool
-intel_dp_compute_config(struct intel_encoder *encoder,
-			struct intel_crtc_state *pipe_config,
-			struct drm_connector_state *conn_state)
+static bool
+intel_dp_compute_link_config(struct intel_encoder *encoder,
+			     struct intel_crtc_state *pipe_config)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
 	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
-	enum port port = encoder->port;
-	struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->base.crtc);
 	struct intel_connector *intel_connector = intel_dp->attached_connector;
-	struct intel_digital_connector_state *intel_conn_state =
-		to_intel_digital_connector_state(conn_state);
 	int lane_count, clock;
 	int min_lane_count = 1;
 	int max_lane_count = intel_dp_max_lane_count(intel_dp);

@@ -1706,9 +1701,6 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 	int bpp, mode_rate;
 	int link_avail, link_clock;
 	int common_len;
-	bool reduce_m_n = drm_dp_has_quirk(&intel_dp->desc,
-					   DP_DPCD_QUIRK_LIMITED_M_N);
-
 	common_len = intel_dp_common_len_rate_limit(intel_dp,
 						    intel_dp->max_link_rate);
 

@@ -1717,51 +1709,6 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 
 	max_clock = common_len - 1;
 
-	if (HAS_PCH_SPLIT(dev_priv) && !HAS_DDI(dev_priv) && port != PORT_A)
-		pipe_config->has_pch_encoder = true;
-
-	pipe_config->has_drrs = false;
-	if (IS_G4X(dev_priv) || port == PORT_A)
-		pipe_config->has_audio = false;
-	else if (intel_conn_state->force_audio == HDMI_AUDIO_AUTO)
-		pipe_config->has_audio = intel_dp->has_audio;
-	else
-		pipe_config->has_audio = intel_conn_state->force_audio == HDMI_AUDIO_ON;
-
-	if (intel_dp_is_edp(intel_dp) && intel_connector->panel.fixed_mode) {
-		struct drm_display_mode *panel_mode =
-			intel_connector->panel.alt_fixed_mode;
-		struct drm_display_mode *req_mode = &pipe_config->base.mode;
-
-		if (!intel_edp_compare_alt_mode(req_mode, panel_mode))
-			panel_mode = intel_connector->panel.fixed_mode;
-
-		drm_mode_debug_printmodeline(panel_mode);
-
-		intel_fixed_panel_mode(panel_mode, adjusted_mode);
-
-		if (INTEL_GEN(dev_priv) >= 9) {
-			int ret;
-			ret = skl_update_scaler_crtc(pipe_config);
-			if (ret)
-				return ret;
-		}
-
-		if (HAS_GMCH_DISPLAY(dev_priv))
-			intel_gmch_panel_fitting(intel_crtc, pipe_config,
-						 conn_state->scaling_mode);
-		else
-			intel_pch_panel_fitting(intel_crtc, pipe_config,
-						conn_state->scaling_mode);
-	}
-
-	if ((IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) &&
-	    adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE)
-		return false;
-
-	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
-		return false;
-
 	/* Use values requested by Compliance Test Request */
 	if (intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING) {
 		int index;

@@ -1831,6 +1778,82 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 	return false;
 
 found:
+	pipe_config->lane_count = lane_count;
+	pipe_config->pipe_bpp = bpp;
+	pipe_config->port_clock = intel_dp->common_rates[clock];
+
+	DRM_DEBUG_KMS("DP lane count %d clock %d bpp %d\n",
+		      pipe_config->lane_count, pipe_config->port_clock, bpp);
+	DRM_DEBUG_KMS("DP link bw required %i available %i\n",
+		      mode_rate, link_avail);
+
+	return true;
+}
+
+bool
+intel_dp_compute_config(struct intel_encoder *encoder,
+			struct intel_crtc_state *pipe_config,
+			struct drm_connector_state *conn_state)
+{
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+	struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
+	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
+	enum port port = encoder->port;
+	struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->base.crtc);
+	struct intel_connector *intel_connector = intel_dp->attached_connector;
+	struct intel_digital_connector_state *intel_conn_state =
+		to_intel_digital_connector_state(conn_state);
+	bool reduce_m_n = drm_dp_has_quirk(&intel_dp->desc,
+					   DP_DPCD_QUIRK_LIMITED_M_N);
+
+	if (HAS_PCH_SPLIT(dev_priv) && !HAS_DDI(dev_priv) && port != PORT_A)
+		pipe_config->has_pch_encoder = true;
+
+	pipe_config->has_drrs = false;
+	if (IS_G4X(dev_priv) || port == PORT_A)
+		pipe_config->has_audio = false;
+	else if (intel_conn_state->force_audio == HDMI_AUDIO_AUTO)
+		pipe_config->has_audio = intel_dp->has_audio;
+	else
+		pipe_config->has_audio = intel_conn_state->force_audio == HDMI_AUDIO_ON;
+
+	if (intel_dp_is_edp(intel_dp) && intel_connector->panel.fixed_mode) {
+		struct drm_display_mode *panel_mode =
+			intel_connector->panel.alt_fixed_mode;
+		struct drm_display_mode *req_mode = &pipe_config->base.mode;
+
+		if (!intel_edp_compare_alt_mode(req_mode, panel_mode))
+			panel_mode = intel_connector->panel.fixed_mode;
+
+		drm_mode_debug_printmodeline(panel_mode);
+
+		intel_fixed_panel_mode(panel_mode, adjusted_mode);
+
+		if (INTEL_GEN(dev_priv) >= 9) {
+			int ret;
+			ret = skl_update_scaler_crtc(pipe_config);
+			if (ret)
+				return ret;
+		}
+
+		if (HAS_GMCH_DISPLAY(dev_priv))
+			intel_gmch_panel_fitting(intel_crtc, pipe_config,
+						 conn_state->scaling_mode);
+		else
+			intel_pch_panel_fitting(intel_crtc, pipe_config,
+						conn_state->scaling_mode);
+	}
+
+	if ((IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) &&
+	    adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE)
+		return false;
+
+	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
+		return false;
+
+	if (!intel_dp_compute_link_config(encoder, pipe_config))
+		return false;
+
 	if (intel_conn_state->broadcast_rgb == INTEL_BROADCAST_RGB_AUTO) {
 		/*
 		 * See:

@@ -1838,7 +1861,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 		 * VESA DisplayPort Ver.1.2a - 5.1.1.1 Video Colorimetry
 		 */
 		pipe_config->limited_color_range =
-			bpp != 18 &&
+			pipe_config->pipe_bpp != 18 &&
 			drm_default_rgb_quant_range(adjusted_mode) ==
 			HDMI_QUANTIZATION_RANGE_LIMITED;
 	} else {

@@ -1846,17 +1869,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 			intel_conn_state->broadcast_rgb == INTEL_BROADCAST_RGB_LIMITED;
 	}
 
-	pipe_config->lane_count = lane_count;
-
-	pipe_config->pipe_bpp = bpp;
-	pipe_config->port_clock = intel_dp->common_rates[clock];
-
-	DRM_DEBUG_KMS("DP lane count %d clock %d bpp %d\n",
-		      pipe_config->lane_count, pipe_config->port_clock, bpp);
-	DRM_DEBUG_KMS("DP link bw required %i available %i\n",
-		      mode_rate, link_avail);
-
-	intel_link_compute_m_n(bpp, lane_count,
+	intel_link_compute_m_n(pipe_config->pipe_bpp, pipe_config->lane_count,
 			       adjusted_mode->crtc_clock,
 			       pipe_config->port_clock,
 			       &pipe_config->dp_m_n,

@@ -1865,11 +1878,12 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 	if (intel_connector->panel.downclock_mode != NULL &&
 		dev_priv->drrs.type == SEAMLESS_DRRS_SUPPORT) {
 			pipe_config->has_drrs = true;
-			intel_link_compute_m_n(bpp, lane_count,
-				intel_connector->panel.downclock_mode->clock,
-				pipe_config->port_clock,
-				&pipe_config->dp_m2_n2,
-				reduce_m_n);
+			intel_link_compute_m_n(pipe_config->pipe_bpp,
+					       pipe_config->lane_count,
+					       intel_connector->panel.downclock_mode->clock,
+					       pipe_config->port_clock,
+					       &pipe_config->dp_m2_n2,
+					       reduce_m_n);
 	}
 
 	/*

Signature · 1 line

2.11.0

Also use same min/max model for bpp, and adjust debug logging while at it.

Signed-off-by Jani Nikula <[email protected]>

---
 drivers/gpu/drm/i915/intel_dp.c | 57 ++++++++++++++++++++++++-----------------
 1 file changed, 33 insertions(+), 24 deletions(-)

drivers/gpu/drm/i915/intel_dp.c +33 -24

index dd42e0422af6..3c5fbdf42b9b 100644
@@ -1647,6 +1647,12 @@ void intel_dp_compute_rate(struct intel_dp *intel_dp, int port_clock,
 	}
 }
 
+struct link_config_limits {
+	int min_clock, max_clock;
+	int min_lane_count, max_lane_count;
+	int min_bpp, max_bpp;
+};
+
 static int intel_dp_compute_bpp(struct intel_dp *intel_dp,
 				struct intel_crtc_state *pipe_config)
 {

@@ -1704,21 +1710,25 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
 {
 	struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
 	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
-	int lane_count, clock;
-	int min_lane_count = 1;
-	int max_lane_count = intel_dp_max_lane_count(intel_dp);
-	int min_clock = 0;
-	int max_clock;
-	int bpp, mode_rate;
-	int link_avail, link_clock;
+	struct link_config_limits limits;
+	int bpp, clock, lane_count;
+	int mode_rate, link_avail, link_clock;
 	int common_len;
+
 	common_len = intel_dp_common_len_rate_limit(intel_dp,
 						    intel_dp->max_link_rate);
 
 	/* No common link rates between source and sink */
 	WARN_ON(common_len <= 0);
 
-	max_clock = common_len - 1;
+	limits.min_clock = 0;
+	limits.max_clock = common_len - 1;
+
+	limits.min_lane_count = 1;
+	limits.max_lane_count = intel_dp_max_lane_count(intel_dp);
+
+	limits.min_bpp = 6 * 3;
+	limits.max_bpp = intel_dp_compute_bpp(intel_dp, pipe_config);
 
 	/* Use values requested by Compliance Test Request */
 	if (intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING) {

@@ -1733,18 +1743,11 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
 						    intel_dp->num_common_rates,
 						    intel_dp->compliance.test_link_rate);
 			if (index >= 0)
-				min_clock = max_clock = index;
-			min_lane_count = max_lane_count = intel_dp->compliance.test_lane_count;
+				limits.min_clock = limits.max_clock = index;
+			limits.min_lane_count = limits.max_lane_count = intel_dp->compliance.test_lane_count;
 		}
 	}
-	DRM_DEBUG_KMS("DP link computation with max lane count %i "
-		      "max bw %d pixel clock %iKHz\n",
-		      max_lane_count, intel_dp->common_rates[max_clock],
-		      adjusted_mode->crtc_clock);
 
-	/* Walk through all bpp values. Luckily they're all nicely spaced with 2
-	 * bpc in between. */
-	bpp = intel_dp_compute_bpp(intel_dp, pipe_config);
 	if (intel_dp_is_edp(intel_dp)) {
 		/*
 		 * Use the maximum clock and number of lanes the eDP panel

@@ -1753,18 +1756,24 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
 		 * configuration, and typically these values correspond to the
 		 * native resolution of the panel.
 		 */
-		min_lane_count = max_lane_count;
-		min_clock = max_clock;
+		limits.min_lane_count = limits.max_lane_count;
+		limits.min_clock = limits.max_clock;
 	}
 
-	for (; bpp >= 6*3; bpp -= 2*3) {
+	DRM_DEBUG_KMS("DP link computation with max lane count %i "
+		      "max rate %d max bpp %d pixel clock %iKHz\n",
+		      limits.max_lane_count,
+		      intel_dp->common_rates[limits.max_clock],
+		      limits.max_bpp, adjusted_mode->crtc_clock);
+
+	for (bpp = limits.max_bpp; bpp >= limits.min_bpp; bpp -= 2 * 3) {
 		mode_rate = intel_dp_link_required(adjusted_mode->crtc_clock,
 						   bpp);
 
-		for (clock = min_clock; clock <= max_clock; clock++) {
-			for (lane_count = min_lane_count;
-				lane_count <= max_lane_count;
-				lane_count <<= 1) {
+		for (clock = limits.min_clock; clock <= limits.max_clock; clock++) {
+			for (lane_count = limits.min_lane_count;
+			     lane_count <= limits.max_lane_count;
+			     lane_count <<= 1) {
 
 				link_clock = intel_dp->common_rates[clock];
 				link_avail = intel_dp_max_data_rate(link_clock,

Signature · 1 line

2.11.0

Quoted 111 lines · Manasi Navare

Quoted 110 lines · Jani Nikula

For now, there's just the one link config selection, optimizing for slow and wide link. No functional changes.

Signed-off-by Jani Nikula <[email protected]>

---
 drivers/gpu/drm/i915/intel_dp.c | 81 +++++++++++++++++++++++++----------------
 1 file changed, 50 insertions(+), 31 deletions(-)

drivers/gpu/drm/i915/intel_dp.c +47 -31

index 3c5fbdf42b9b..c98626b3af65 100644
@@ -1704,6 +1704,42 @@ static bool intel_edp_compare_alt_mode(struct drm_display_mode *m1,
 	return bres;
 }
 
+/* Optimize link config in order: max bpp, min clock, min lanes */
+static bool
+intel_dp_compute_link_config_wide(struct intel_dp *intel_dp,
+				  struct intel_crtc_state *pipe_config,
+				  const struct link_config_limits *limits)
+{
+	struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
+	int bpp, clock, lane_count;
+	int mode_rate, link_clock, link_avail;
+
+	for (bpp = limits->max_bpp; bpp >= limits->min_bpp; bpp -= 2 * 3) {
+		mode_rate = intel_dp_link_required(adjusted_mode->crtc_clock,
+						   bpp);
+
+		for (clock = limits->min_clock; clock <= limits->max_clock; clock++) {
+			for (lane_count = limits->min_lane_count;
+			     lane_count <= limits->max_lane_count;
+			     lane_count <<= 1) {
+				link_clock = intel_dp->common_rates[clock];
+				link_avail = intel_dp_max_data_rate(link_clock,
+								    lane_count);
+
+				if (mode_rate <= link_avail) {
+					pipe_config->lane_count = lane_count;
+					pipe_config->pipe_bpp = bpp;
+					pipe_config->port_clock = link_clock;
+
+					return true;
+				}
+			}
+		}
+	}
+
+	return false;
+}
+
 static bool
 intel_dp_compute_link_config(struct intel_encoder *encoder,
 			     struct intel_crtc_state *pipe_config)

@@ -1711,8 +1747,6 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
 	struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
 	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
 	struct link_config_limits limits;
-	int bpp, clock, lane_count;
-	int mode_rate, link_avail, link_clock;
 	int common_len;
 
 	common_len = intel_dp_common_len_rate_limit(intel_dp,

@@ -1766,37 +1800,22 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
 		      intel_dp->common_rates[limits.max_clock],
 		      limits.max_bpp, adjusted_mode->crtc_clock);
 
-	for (bpp = limits.max_bpp; bpp >= limits.min_bpp; bpp -= 2 * 3) {
-		mode_rate = intel_dp_link_required(adjusted_mode->crtc_clock,
-						   bpp);
-
-		for (clock = limits.min_clock; clock <= limits.max_clock; clock++) {
-			for (lane_count = limits.min_lane_count;
-			     lane_count <= limits.max_lane_count;
-			     lane_count <<= 1) {
-
-				link_clock = intel_dp->common_rates[clock];
-				link_avail = intel_dp_max_data_rate(link_clock,
-								    lane_count);
-
-				if (mode_rate <= link_avail) {
-					goto found;
-				}
-			}
-		}
-	}
-
-	return false;
-
-found:
-	pipe_config->lane_count = lane_count;
-	pipe_config->pipe_bpp = bpp;
-	pipe_config->port_clock = intel_dp->common_rates[clock];
+	/*
+	 * Optimize for slow and wide. This is the place to add alternative
+	 * optimization policy.
+	 */
+	if (!intel_dp_compute_link_config_wide(intel_dp, pipe_config, &limits))
+		return false;
 
 	DRM_DEBUG_KMS("DP lane count %d clock %d bpp %d\n",
-		      pipe_config->lane_count, pipe_config->port_clock, bpp);
-	DRM_DEBUG_KMS("DP link bw required %i available %i\n",
-		      mode_rate, link_avail);
+		      pipe_config->lane_count, pipe_config->port_clock,
+		      pipe_config->pipe_bpp);
+
+	DRM_DEBUG_KMS("DP link rate required %i available %i\n",
+		      intel_dp_link_required(adjusted_mode->crtc_clock,

Quoted 8 lines

drivers/gpu/drm/i915/intel_dp.c intel_dp_compute_link_config(struct intel_encoder *encoder, · line 1818

Quoted 3 lines

drivers/gpu/drm/i915/intel_dp.c +3 -0

+					     pipe_config->pipe_bpp),
+		      intel_dp_max_data_rate(pipe_config->port_clock,
+					     pipe_config->lane_count));

Wouldnt it be better if we move this Debug message about Available and required link rate in the other function right after the condition if (mode_rate <= link_avail) is true, before returning true from that function. I think it will be just more intuitive there.

That's what I thought too at first, but then realized if we're going to add an alternative call to an alternative approach, and perhaps yet another for DSC, the debugging gets duplicated in all of them.

BR,
Jani.

Quoted 15 lines

Everything else looks good. So

Reviewed-by Manasi Navare <[email protected]>

Manasi

Quoted 6 lines

drivers/gpu/drm/i915/intel_dp.c +0 -0

 	return true;
 }

Signature · 1 line

2.11.0

Quoted 4 lines

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Signature · 1 line

Jani Nikula, Intel Open Source Technology Center