diff mbox series

drm/i915/dp: Use slow and wide link training for everything

Message ID 20210421052054.1434718-1-kai.heng.feng@canonical.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/dp: Use slow and wide link training for everything | expand

Commit Message

Kai-Heng Feng April 21, 2021, 5:20 a.m. UTC
Screen flickers on Innolux eDP 1.3 panel when clock rate 540000 is in use.

According to the panel vendor, though clock rate 540000 is advertised,
but the max clock rate it really supports is 270000.

Ville Syrjälä mentioned that fast and narrow also breaks some eDP 1.4
panel, so use slow and wide training for all panels to resolve the
issue.

User also confirmed that the new strategy doesn't introduce any
regression on XPS 9380.

v2:
 - Use slow and wide for everything.

Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/3384
References: https://gitlab.freedesktop.org/drm/intel/-/issues/272
Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 59 +++----------------------
 1 file changed, 5 insertions(+), 54 deletions(-)

Comments

Ville Syrjälä April 21, 2021, 6:39 p.m. UTC | #1
On Wed, Apr 21, 2021 at 01:20:31PM +0800, Kai-Heng Feng wrote:
> Screen flickers on Innolux eDP 1.3 panel when clock rate 540000 is in use.
> 
> According to the panel vendor, though clock rate 540000 is advertised,
> but the max clock rate it really supports is 270000.
> 
> Ville Syrjälä mentioned that fast and narrow also breaks some eDP 1.4
> panel, so use slow and wide training for all panels to resolve the
> issue.
> 
> User also confirmed that the new strategy doesn't introduce any
> regression on XPS 9380.
> 
> v2:
>  - Use slow and wide for everything.
> 
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/3384
> References: https://gitlab.freedesktop.org/drm/intel/-/issues/272
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>

Thanks. Pushed to drm-intel-next.

I did a quick scan of a few CI logs and noticed that at least cml-u2
changed behaviour:
- [CONNECTOR:95:eDP-1] Link Training passed at link rate = 432000, lane count = 1, at DPRX
+ [CONNECTOR:95:eDP-1] Link Training passed at link rate = 216000, lane count = 2, at DPRX

But it still appears to work, and 2.16Gbps is also the link rate chosen
by the BIOS, which is reassuring.

> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -1095,44 +1095,6 @@ intel_dp_compute_link_config_wide(struct intel_dp *intel_dp,
>  	return -EINVAL;
>  }
>  
> -/* Optimize link config in order: max bpp, min lanes, min clock */
> -static int
> -intel_dp_compute_link_config_fast(struct intel_dp *intel_dp,
> -				  struct intel_crtc_state *pipe_config,
> -				  const struct link_config_limits *limits)
> -{
> -	const struct drm_display_mode *adjusted_mode = &pipe_config->hw.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) {
> -		int output_bpp = intel_dp_output_bpp(pipe_config->output_format, bpp);
> -
> -		mode_rate = intel_dp_link_required(adjusted_mode->crtc_clock,
> -						   output_bpp);
> -
> -		for (lane_count = limits->min_lane_count;
> -		     lane_count <= limits->max_lane_count;
> -		     lane_count <<= 1) {
> -			for (clock = limits->min_clock; clock <= limits->max_clock; clock++) {
> -				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 0;
> -				}
> -			}
> -		}
> -	}
> -
> -	return -EINVAL;
> -}
> -
>  static int intel_dp_dsc_compute_bpp(struct intel_dp *intel_dp, u8 dsc_max_bpc)
>  {
>  	int i, num_bpc;
> @@ -1382,22 +1344,11 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
>  	    intel_dp_can_bigjoiner(intel_dp))
>  		pipe_config->bigjoiner = true;
>  
> -	if (intel_dp_is_edp(intel_dp))
> -		/*
> -		 * Optimize for fast and narrow. eDP 1.3 section 3.3 and eDP 1.4
> -		 * section A.1: "It is recommended that the minimum number of
> -		 * lanes be used, using the minimum link rate allowed for that
> -		 * lane configuration."
> -		 *
> -		 * Note that we fall back to the max clock and lane count for eDP
> -		 * panels that fail with the fast optimal settings (see
> -		 * intel_dp->use_max_params), in which case the fast vs. wide
> -		 * choice doesn't matter.
> -		 */
> -		ret = intel_dp_compute_link_config_fast(intel_dp, pipe_config, &limits);
> -	else
> -		/* Optimize for slow and wide. */
> -		ret = intel_dp_compute_link_config_wide(intel_dp, pipe_config, &limits);
> +	/*
> +	 * Optimize for slow and wide for everything, because there are some
> +	 * eDP 1.3 and 1.4 panels don't work well with fast and narrow.
> +	 */
> +	ret = intel_dp_compute_link_config_wide(intel_dp, pipe_config, &limits);
>  
>  	/* enable compression if the mode doesn't fit available BW */
>  	drm_dbg_kms(&i915->drm, "Force DSC en = %d\n", intel_dp->force_dsc_en);
> -- 
> 2.30.2
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 52ea09fc5e70..4ad12dde5938 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1095,44 +1095,6 @@  intel_dp_compute_link_config_wide(struct intel_dp *intel_dp,
 	return -EINVAL;
 }
 
-/* Optimize link config in order: max bpp, min lanes, min clock */
-static int
-intel_dp_compute_link_config_fast(struct intel_dp *intel_dp,
-				  struct intel_crtc_state *pipe_config,
-				  const struct link_config_limits *limits)
-{
-	const struct drm_display_mode *adjusted_mode = &pipe_config->hw.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) {
-		int output_bpp = intel_dp_output_bpp(pipe_config->output_format, bpp);
-
-		mode_rate = intel_dp_link_required(adjusted_mode->crtc_clock,
-						   output_bpp);
-
-		for (lane_count = limits->min_lane_count;
-		     lane_count <= limits->max_lane_count;
-		     lane_count <<= 1) {
-			for (clock = limits->min_clock; clock <= limits->max_clock; clock++) {
-				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 0;
-				}
-			}
-		}
-	}
-
-	return -EINVAL;
-}
-
 static int intel_dp_dsc_compute_bpp(struct intel_dp *intel_dp, u8 dsc_max_bpc)
 {
 	int i, num_bpc;
@@ -1382,22 +1344,11 @@  intel_dp_compute_link_config(struct intel_encoder *encoder,
 	    intel_dp_can_bigjoiner(intel_dp))
 		pipe_config->bigjoiner = true;
 
-	if (intel_dp_is_edp(intel_dp))
-		/*
-		 * Optimize for fast and narrow. eDP 1.3 section 3.3 and eDP 1.4
-		 * section A.1: "It is recommended that the minimum number of
-		 * lanes be used, using the minimum link rate allowed for that
-		 * lane configuration."
-		 *
-		 * Note that we fall back to the max clock and lane count for eDP
-		 * panels that fail with the fast optimal settings (see
-		 * intel_dp->use_max_params), in which case the fast vs. wide
-		 * choice doesn't matter.
-		 */
-		ret = intel_dp_compute_link_config_fast(intel_dp, pipe_config, &limits);
-	else
-		/* Optimize for slow and wide. */
-		ret = intel_dp_compute_link_config_wide(intel_dp, pipe_config, &limits);
+	/*
+	 * Optimize for slow and wide for everything, because there are some
+	 * eDP 1.3 and 1.4 panels don't work well with fast and narrow.
+	 */
+	ret = intel_dp_compute_link_config_wide(intel_dp, pipe_config, &limits);
 
 	/* enable compression if the mode doesn't fit available BW */
 	drm_dbg_kms(&i915->drm, "Force DSC en = %d\n", intel_dp->force_dsc_en);