dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] drm/i915/dp: Use slow and wide link training for everything
@ 2021-04-21  5:20 Kai-Heng Feng
  2021-04-21 18:39 ` Ville Syrjälä
  0 siblings, 1 reply; 2+ messages in thread
From: Kai-Heng Feng @ 2021-04-21  5:20 UTC (permalink / raw)
  To: jani.nikula, joonas.lahtinen, rodrigo.vivi, ville.syrjala
  Cc: David Airlie, Lucas De Marchi, open list, open list:DRM DRIVERS,
	Gwan-gyeong Mun, Manasi Navare, Kai-Heng Feng, Sean Paul,
	Ankit Nautiyal, intel-gfx, Uma Shankar

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(-)

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);
-- 
2.30.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] drm/i915/dp: Use slow and wide link training for everything
  2021-04-21  5:20 [PATCH] drm/i915/dp: Use slow and wide link training for everything Kai-Heng Feng
@ 2021-04-21 18:39 ` Ville Syrjälä
  0 siblings, 0 replies; 2+ messages in thread
From: Ville Syrjälä @ 2021-04-21 18:39 UTC (permalink / raw)
  To: Kai-Heng Feng
  Cc: David Airlie, Lucas De Marchi, open list, Gwan-gyeong Mun,
	Manasi Navare, Uma Shankar, Sean Paul, open list:DRM DRIVERS,
	rodrigo.vivi, Ankit Nautiyal, intel-gfx

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

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2021-04-21 18:39 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-21  5:20 [PATCH] drm/i915/dp: Use slow and wide link training for everything Kai-Heng Feng
2021-04-21 18:39 ` Ville Syrjälä

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).