All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915/dp: revert back to max link rate and lane count on eDP
@ 2019-04-05  7:24 Jani Nikula
  2019-04-05  7:50 ` Jani Nikula
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Jani Nikula @ 2019-04-05  7:24 UTC (permalink / raw)
  To: intel-gfx
  Cc: jani.nikula, Ville Syrjälä,
	Manasi Navare, Rodrigo Vivi, Matt Atwood, Lee, Shawn C,
	Dave Airlie, stable

Commit 7769db588384 ("drm/i915/dp: optimize eDP 1.4+ link config fast
and narrow") started to optize the eDP 1.4+ link config, both per spec
and as preparation for display stream compression support.

Sadly, we again face panels that flat out fail with parameters they
claim to support. Revert, and go back to the drawing board.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109959
Fixes: 7769db588384 ("drm/i915/dp: optimize eDP 1.4+ link config fast and narrow")
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Matt Atwood <matthew.s.atwood@intel.com>
Cc: "Lee, Shawn C" <shawn.c.lee@intel.com>
Cc: Dave Airlie <airlied@gmail.com>
Cc: intel-gfx@lists.freedesktop.org
Cc: <stable@vger.kernel.org> # v5.0+
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 67 +++++----------------------------
 1 file changed, 9 insertions(+), 58 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 72c490..fd0f53 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1856,42 +1856,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)
-{
-	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 (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;
@@ -2031,12 +1995,10 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
 	if (intel_dp_is_edp(intel_dp) && intel_dp->edp_dpcd[0] < DP_EDP_14) {
 		/*
 		 * Use the maximum clock and number of lanes the eDP panel
-		 * advertizes being capable of. The eDP 1.3 and earlier panels
-		 * are generally designed to support only a single clock and
-		 * lane configuration, and typically these values correspond to
-		 * the native resolution of the panel. With eDP 1.4 rate select
-		 * and DSC, this is decreasingly the case, and we need to be
-		 * able to select less than maximum link config.
+		 * advertizes being capable of. The panels are generally
+		 * designed to support only a single clock and lane
+		 * configuration, and typically these values correspond to the
+		 * native resolution of the panel.
 		 */
 		limits.min_lane_count = limits.max_lane_count;
 		limits.min_clock = limits.max_clock;
@@ -2050,22 +2012,11 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
 		      intel_dp->common_rates[limits.max_clock],
 		      limits.max_bpp, adjusted_mode->crtc_clock);
 
-	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 use the max clock and lane count for eDP 1.3 and
-		 * earlier, and fast vs. wide is irrelevant.
-		 */
-		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. This is the place to add alternative
+	 * optimization policy.
+	 */
+	ret = intel_dp_compute_link_config_wide(intel_dp, pipe_config, &limits);
 
 	/* enable compression if the mode doesn't fit available BW */
 	DRM_DEBUG_KMS("Force DSC en = %d\n", intel_dp->force_dsc_en);
-- 
2.20.1


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

* Re: [PATCH] drm/i915/dp: revert back to max link rate and lane count on eDP
  2019-04-05  7:24 [PATCH] drm/i915/dp: revert back to max link rate and lane count on eDP Jani Nikula
@ 2019-04-05  7:50 ` Jani Nikula
  2019-04-05  7:52 ` [PATCH v2] " Jani Nikula
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 13+ messages in thread
From: Jani Nikula @ 2019-04-05  7:50 UTC (permalink / raw)
  To: intel-gfx
  Cc: Ville Syrjälä,
	Manasi Navare, Rodrigo Vivi, Matt Atwood, Lee, Shawn C,
	Dave Airlie, stable

On Fri, 05 Apr 2019, Jani Nikula <jani.nikula@intel.com> wrote:
> Commit 7769db588384 ("drm/i915/dp: optimize eDP 1.4+ link config fast
> and narrow") started to optize the eDP 1.4+ link config, both per spec
> and as preparation for display stream compression support.
>
> Sadly, we again face panels that flat out fail with parameters they
> claim to support. Revert, and go back to the drawing board.

Fail, this one actually reverts back to wide-and-slow, while retaining
the use of optimal parameters rather than maxing them out.

It's a data point, but we'll want the full revert now.

BR,
Jani.

>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109959
> Fixes: 7769db588384 ("drm/i915/dp: optimize eDP 1.4+ link config fast and narrow")
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Matt Atwood <matthew.s.atwood@intel.com>
> Cc: "Lee, Shawn C" <shawn.c.lee@intel.com>
> Cc: Dave Airlie <airlied@gmail.com>
> Cc: intel-gfx@lists.freedesktop.org
> Cc: <stable@vger.kernel.org> # v5.0+
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 67 +++++----------------------------
>  1 file changed, 9 insertions(+), 58 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 72c490..fd0f53 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1856,42 +1856,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)
> -{
> -	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 (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;
> @@ -2031,12 +1995,10 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
>  	if (intel_dp_is_edp(intel_dp) && intel_dp->edp_dpcd[0] < DP_EDP_14) {
>  		/*
>  		 * Use the maximum clock and number of lanes the eDP panel
> -		 * advertizes being capable of. The eDP 1.3 and earlier panels
> -		 * are generally designed to support only a single clock and
> -		 * lane configuration, and typically these values correspond to
> -		 * the native resolution of the panel. With eDP 1.4 rate select
> -		 * and DSC, this is decreasingly the case, and we need to be
> -		 * able to select less than maximum link config.
> +		 * advertizes being capable of. The panels are generally
> +		 * designed to support only a single clock and lane
> +		 * configuration, and typically these values correspond to the
> +		 * native resolution of the panel.
>  		 */
>  		limits.min_lane_count = limits.max_lane_count;
>  		limits.min_clock = limits.max_clock;
> @@ -2050,22 +2012,11 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
>  		      intel_dp->common_rates[limits.max_clock],
>  		      limits.max_bpp, adjusted_mode->crtc_clock);
>  
> -	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 use the max clock and lane count for eDP 1.3 and
> -		 * earlier, and fast vs. wide is irrelevant.
> -		 */
> -		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. This is the place to add alternative
> +	 * optimization policy.
> +	 */
> +	ret = intel_dp_compute_link_config_wide(intel_dp, pipe_config, &limits);
>  
>  	/* enable compression if the mode doesn't fit available BW */
>  	DRM_DEBUG_KMS("Force DSC en = %d\n", intel_dp->force_dsc_en);

-- 
Jani Nikula, Intel Open Source Graphics Center

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

* [PATCH v2] drm/i915/dp: revert back to max link rate and lane count on eDP
  2019-04-05  7:24 [PATCH] drm/i915/dp: revert back to max link rate and lane count on eDP Jani Nikula
  2019-04-05  7:50 ` Jani Nikula
@ 2019-04-05  7:52 ` Jani Nikula
  2019-04-05 17:23   ` Rodrigo Vivi
                     ` (2 more replies)
  2019-04-05 11:24 ` ✓ Fi.CI.BAT: success for drm/i915/dp: revert back to max link rate and lane count on eDP (rev2) Patchwork
  2019-04-06  6:15 ` ✗ Fi.CI.IGT: failure " Patchwork
  3 siblings, 3 replies; 13+ messages in thread
From: Jani Nikula @ 2019-04-05  7:52 UTC (permalink / raw)
  To: Jani Nikula, intel-gfx
  Cc: Ville Syrjälä,
	Manasi Navare, Rodrigo Vivi, Matt Atwood, Lee, Shawn C,
	Dave Airlie, stable

Commit 7769db588384 ("drm/i915/dp: optimize eDP 1.4+ link config fast
and narrow") started to optize the eDP 1.4+ link config, both per spec
and as preparation for display stream compression support.

Sadly, we again face panels that flat out fail with parameters they
claim to support. Revert, and go back to the drawing board.

v2: Actually revert to max params instead of just wide-and-slow.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109959
Fixes: 7769db588384 ("drm/i915/dp: optimize eDP 1.4+ link config fast and narrow")
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Matt Atwood <matthew.s.atwood@intel.com>
Cc: "Lee, Shawn C" <shawn.c.lee@intel.com>
Cc: Dave Airlie <airlied@gmail.com>
Cc: intel-gfx@lists.freedesktop.org
Cc: <stable@vger.kernel.org> # v5.0+
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 69 +++++----------------------------
 1 file changed, 10 insertions(+), 59 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 72c490..dfa770 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1856,42 +1856,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)
-{
-	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 (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;
@@ -2028,15 +1992,13 @@ 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);
 
-	if (intel_dp_is_edp(intel_dp) && intel_dp->edp_dpcd[0] < DP_EDP_14) {
+	if (intel_dp_is_edp(intel_dp)) {
 		/*
 		 * Use the maximum clock and number of lanes the eDP panel
-		 * advertizes being capable of. The eDP 1.3 and earlier panels
-		 * are generally designed to support only a single clock and
-		 * lane configuration, and typically these values correspond to
-		 * the native resolution of the panel. With eDP 1.4 rate select
-		 * and DSC, this is decreasingly the case, and we need to be
-		 * able to select less than maximum link config.
+		 * advertizes being capable of. The panels are generally
+		 * designed to support only a single clock and lane
+		 * configuration, and typically these values correspond to the
+		 * native resolution of the panel.
 		 */
 		limits.min_lane_count = limits.max_lane_count;
 		limits.min_clock = limits.max_clock;
@@ -2050,22 +2012,11 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
 		      intel_dp->common_rates[limits.max_clock],
 		      limits.max_bpp, adjusted_mode->crtc_clock);
 
-	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 use the max clock and lane count for eDP 1.3 and
-		 * earlier, and fast vs. wide is irrelevant.
-		 */
-		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. This is the place to add alternative
+	 * optimization policy.
+	 */
+	ret = intel_dp_compute_link_config_wide(intel_dp, pipe_config, &limits);
 
 	/* enable compression if the mode doesn't fit available BW */
 	DRM_DEBUG_KMS("Force DSC en = %d\n", intel_dp->force_dsc_en);
-- 
2.20.1


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

* ✓ Fi.CI.BAT: success for drm/i915/dp: revert back to max link rate and lane count on eDP (rev2)
  2019-04-05  7:24 [PATCH] drm/i915/dp: revert back to max link rate and lane count on eDP Jani Nikula
  2019-04-05  7:50 ` Jani Nikula
  2019-04-05  7:52 ` [PATCH v2] " Jani Nikula
@ 2019-04-05 11:24 ` Patchwork
  2019-04-06  6:15 ` ✗ Fi.CI.IGT: failure " Patchwork
  3 siblings, 0 replies; 13+ messages in thread
From: Patchwork @ 2019-04-05 11:24 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/dp: revert back to max link rate and lane count on eDP (rev2)
URL   : https://patchwork.freedesktop.org/series/59039/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5878 -> Patchwork_12692
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/59039/revisions/2/mbox/

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_flink_basic@double-flink:
    - fi-icl-y:           PASS -> DMESG-WARN [fdo#109638]

  * igt@i915_module_load@reload:
    - fi-blb-e6850:       PASS -> INCOMPLETE [fdo#107718]

  * igt@i915_selftest@live_contexts:
    - fi-skl-gvtdvm:      PASS -> DMESG-FAIL [fdo#110235 ]

  * igt@i915_selftest@live_uncore:
    - fi-kbl-8809g:       PASS -> DMESG-FAIL [fdo#110210]

  * igt@kms_busy@basic-flip-a:
    - fi-bsw-n3050:       NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +1

  * igt@kms_chamelium@hdmi-crc-fast:
    - fi-bsw-n3050:       NOTRUN -> SKIP [fdo#109271] +62

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
    - fi-byt-clapper:     PASS -> FAIL [fdo#103191] / [fdo#107362] +1

  
#### Possible fixes ####

  * igt@gem_mmap@basic-small-bo:
    - {fi-icl-u3}:        DMESG-WARN [fdo#107724] -> PASS

  * igt@i915_selftest@live_execlists:
    - fi-apl-guc:         INCOMPLETE [fdo#103927] / [fdo#109720] -> PASS

  * igt@i915_selftest@live_uncore:
    - fi-ivb-3770:        DMESG-FAIL [fdo#110210] -> PASS

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109638]: https://bugs.freedesktop.org/show_bug.cgi?id=109638
  [fdo#109720]: https://bugs.freedesktop.org/show_bug.cgi?id=109720
  [fdo#110210]: https://bugs.freedesktop.org/show_bug.cgi?id=110210
  [fdo#110235 ]: https://bugs.freedesktop.org/show_bug.cgi?id=110235 


Participating hosts (46 -> 43)
------------------------------

  Additional (1): fi-bsw-n3050 
  Missing    (4): fi-kbl-soraka fi-ilk-m540 fi-byt-squawks fi-bsw-cyan 


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

    * Linux: CI_DRM_5878 -> Patchwork_12692

  CI_DRM_5878: ae3367460fdf5e6f5a47038c1281b502817184e1 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4931: 019f892e5d1a0a9643cb726c47ce2d99c14b444f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12692: a8c54be839b389e42731eb174f7a83d7682db411 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

a8c54be839b3 drm/i915/dp: revert back to max link rate and lane count on eDP

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12692/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v2] drm/i915/dp: revert back to max link rate and lane count on eDP
  2019-04-05  7:52 ` [PATCH v2] " Jani Nikula
@ 2019-04-05 17:23   ` Rodrigo Vivi
  2019-04-10 12:54     ` Jani Nikula
  2019-04-05 18:18   ` Manasi Navare
  2019-04-08 14:53   ` Sasha Levin
  2 siblings, 1 reply; 13+ messages in thread
From: Rodrigo Vivi @ 2019-04-05 17:23 UTC (permalink / raw)
  To: Jani Nikula
  Cc: intel-gfx, Ville Syrjälä,
	Manasi Navare, Matt Atwood, Lee, Shawn C, Dave Airlie, stable

On Fri, Apr 05, 2019 at 10:52:20AM +0300, Jani Nikula wrote:
> Commit 7769db588384 ("drm/i915/dp: optimize eDP 1.4+ link config fast
> and narrow") started to optize the eDP 1.4+ link config, both per spec
> and as preparation for display stream compression support.
> 
> Sadly, we again face panels that flat out fail with parameters they
> claim to support. Revert, and go back to the drawing board.
> 
> v2: Actually revert to max params instead of just wide-and-slow.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109959
> Fixes: 7769db588384 ("drm/i915/dp: optimize eDP 1.4+ link config fast and narrow")
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Matt Atwood <matthew.s.atwood@intel.com>
> Cc: "Lee, Shawn C" <shawn.c.lee@intel.com>
> Cc: Dave Airlie <airlied@gmail.com>
> Cc: intel-gfx@lists.freedesktop.org
> Cc: <stable@vger.kernel.org> # v5.0+
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

I can hear from here your "I told you so"... Sorry!

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_dp.c | 69 +++++----------------------------
>  1 file changed, 10 insertions(+), 59 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 72c490..dfa770 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1856,42 +1856,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)
> -{
> -	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 (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;
> @@ -2028,15 +1992,13 @@ 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);
>  
> -	if (intel_dp_is_edp(intel_dp) && intel_dp->edp_dpcd[0] < DP_EDP_14) {
> +	if (intel_dp_is_edp(intel_dp)) {
>  		/*
>  		 * Use the maximum clock and number of lanes the eDP panel
> -		 * advertizes being capable of. The eDP 1.3 and earlier panels
> -		 * are generally designed to support only a single clock and
> -		 * lane configuration, and typically these values correspond to
> -		 * the native resolution of the panel. With eDP 1.4 rate select
> -		 * and DSC, this is decreasingly the case, and we need to be
> -		 * able to select less than maximum link config.
> +		 * advertizes being capable of. The panels are generally
> +		 * designed to support only a single clock and lane
> +		 * configuration, and typically these values correspond to the
> +		 * native resolution of the panel.
>  		 */
>  		limits.min_lane_count = limits.max_lane_count;
>  		limits.min_clock = limits.max_clock;
> @@ -2050,22 +2012,11 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
>  		      intel_dp->common_rates[limits.max_clock],
>  		      limits.max_bpp, adjusted_mode->crtc_clock);
>  
> -	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 use the max clock and lane count for eDP 1.3 and
> -		 * earlier, and fast vs. wide is irrelevant.
> -		 */
> -		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. This is the place to add alternative
> +	 * optimization policy.
> +	 */
> +	ret = intel_dp_compute_link_config_wide(intel_dp, pipe_config, &limits);
>  
>  	/* enable compression if the mode doesn't fit available BW */
>  	DRM_DEBUG_KMS("Force DSC en = %d\n", intel_dp->force_dsc_en);
> -- 
> 2.20.1
> 

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

* Re: [PATCH v2] drm/i915/dp: revert back to max link rate and lane count on eDP
  2019-04-05  7:52 ` [PATCH v2] " Jani Nikula
  2019-04-05 17:23   ` Rodrigo Vivi
@ 2019-04-05 18:18   ` Manasi Navare
  2019-04-05 20:13     ` Rodrigo Vivi
  2019-04-08 14:53   ` Sasha Levin
  2 siblings, 1 reply; 13+ messages in thread
From: Manasi Navare @ 2019-04-05 18:18 UTC (permalink / raw)
  To: Jani Nikula
  Cc: intel-gfx, Ville Syrjälä,
	Rodrigo Vivi, Matt Atwood, Lee, Shawn C, Dave Airlie, stable

On Fri, Apr 05, 2019 at 10:52:20AM +0300, Jani Nikula wrote:
> Commit 7769db588384 ("drm/i915/dp: optimize eDP 1.4+ link config fast
> and narrow") started to optize the eDP 1.4+ link config, both per spec
> and as preparation for display stream compression support.
> 
> Sadly, we again face panels that flat out fail with parameters they
> claim to support. Revert, and go back to the drawing board.

Yup, already multiple users facing this issue with eDP 1.4 panels that require
max parameters to pass link train.

I hear you now :)

Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>

> 
> v2: Actually revert to max params instead of just wide-and-slow.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109959
> Fixes: 7769db588384 ("drm/i915/dp: optimize eDP 1.4+ link config fast and narrow")
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Matt Atwood <matthew.s.atwood@intel.com>
> Cc: "Lee, Shawn C" <shawn.c.lee@intel.com>
> Cc: Dave Airlie <airlied@gmail.com>
> Cc: intel-gfx@lists.freedesktop.org
> Cc: <stable@vger.kernel.org> # v5.0+
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 69 +++++----------------------------
>  1 file changed, 10 insertions(+), 59 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 72c490..dfa770 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1856,42 +1856,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)
> -{
> -	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 (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;
> @@ -2028,15 +1992,13 @@ 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);
>  
> -	if (intel_dp_is_edp(intel_dp) && intel_dp->edp_dpcd[0] < DP_EDP_14) {
> +	if (intel_dp_is_edp(intel_dp)) {
>  		/*
>  		 * Use the maximum clock and number of lanes the eDP panel
> -		 * advertizes being capable of. The eDP 1.3 and earlier panels
> -		 * are generally designed to support only a single clock and
> -		 * lane configuration, and typically these values correspond to
> -		 * the native resolution of the panel. With eDP 1.4 rate select
> -		 * and DSC, this is decreasingly the case, and we need to be
> -		 * able to select less than maximum link config.
> +		 * advertizes being capable of. The panels are generally
> +		 * designed to support only a single clock and lane
> +		 * configuration, and typically these values correspond to the
> +		 * native resolution of the panel.
>  		 */
>  		limits.min_lane_count = limits.max_lane_count;
>  		limits.min_clock = limits.max_clock;
> @@ -2050,22 +2012,11 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
>  		      intel_dp->common_rates[limits.max_clock],
>  		      limits.max_bpp, adjusted_mode->crtc_clock);
>  
> -	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 use the max clock and lane count for eDP 1.3 and
> -		 * earlier, and fast vs. wide is irrelevant.
> -		 */
> -		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. This is the place to add alternative
> +	 * optimization policy.
> +	 */
> +	ret = intel_dp_compute_link_config_wide(intel_dp, pipe_config, &limits);
>  
>  	/* enable compression if the mode doesn't fit available BW */
>  	DRM_DEBUG_KMS("Force DSC en = %d\n", intel_dp->force_dsc_en);
> -- 
> 2.20.1
> 

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

* Re: [PATCH v2] drm/i915/dp: revert back to max link rate and lane count on eDP
  2019-04-05 18:18   ` Manasi Navare
@ 2019-04-05 20:13     ` Rodrigo Vivi
  2019-04-05 20:20       ` Manasi Navare
  0 siblings, 1 reply; 13+ messages in thread
From: Rodrigo Vivi @ 2019-04-05 20:13 UTC (permalink / raw)
  To: Manasi Navare
  Cc: Jani Nikula, intel-gfx, Ville Syrjälä,
	Matt Atwood, Lee, Shawn C, Dave Airlie, stable

On Fri, Apr 05, 2019 at 11:18:25AM -0700, Manasi Navare wrote:
> On Fri, Apr 05, 2019 at 10:52:20AM +0300, Jani Nikula wrote:
> > Commit 7769db588384 ("drm/i915/dp: optimize eDP 1.4+ link config fast
> > and narrow") started to optize the eDP 1.4+ link config, both per spec
> > and as preparation for display stream compression support.
> > 
> > Sadly, we again face panels that flat out fail with parameters they
> > claim to support. Revert, and go back to the drawing board.
> 
> Yup, already multiple users facing this issue with eDP 1.4 panels that require
> max parameters to pass link train.

I was wondering if we should blacklist the panel/platform, but if
there are multiple cases it is better to revert...

Unless there's a way to try and fallback quickly?!

> 
> I hear you now :)
> 
> Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>
> 
> > 
> > v2: Actually revert to max params instead of just wide-and-slow.
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109959
> > Fixes: 7769db588384 ("drm/i915/dp: optimize eDP 1.4+ link config fast and narrow")
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Manasi Navare <manasi.d.navare@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Matt Atwood <matthew.s.atwood@intel.com>
> > Cc: "Lee, Shawn C" <shawn.c.lee@intel.com>
> > Cc: Dave Airlie <airlied@gmail.com>
> > Cc: intel-gfx@lists.freedesktop.org
> > Cc: <stable@vger.kernel.org> # v5.0+
> > Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 69 +++++----------------------------
> >  1 file changed, 10 insertions(+), 59 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 72c490..dfa770 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -1856,42 +1856,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)
> > -{
> > -	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 (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;
> > @@ -2028,15 +1992,13 @@ 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);
> >  
> > -	if (intel_dp_is_edp(intel_dp) && intel_dp->edp_dpcd[0] < DP_EDP_14) {
> > +	if (intel_dp_is_edp(intel_dp)) {
> >  		/*
> >  		 * Use the maximum clock and number of lanes the eDP panel
> > -		 * advertizes being capable of. The eDP 1.3 and earlier panels
> > -		 * are generally designed to support only a single clock and
> > -		 * lane configuration, and typically these values correspond to
> > -		 * the native resolution of the panel. With eDP 1.4 rate select
> > -		 * and DSC, this is decreasingly the case, and we need to be
> > -		 * able to select less than maximum link config.
> > +		 * advertizes being capable of. The panels are generally
> > +		 * designed to support only a single clock and lane
> > +		 * configuration, and typically these values correspond to the
> > +		 * native resolution of the panel.
> >  		 */
> >  		limits.min_lane_count = limits.max_lane_count;
> >  		limits.min_clock = limits.max_clock;
> > @@ -2050,22 +2012,11 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
> >  		      intel_dp->common_rates[limits.max_clock],
> >  		      limits.max_bpp, adjusted_mode->crtc_clock);
> >  
> > -	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 use the max clock and lane count for eDP 1.3 and
> > -		 * earlier, and fast vs. wide is irrelevant.
> > -		 */
> > -		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. This is the place to add alternative
> > +	 * optimization policy.
> > +	 */
> > +	ret = intel_dp_compute_link_config_wide(intel_dp, pipe_config, &limits);
> >  
> >  	/* enable compression if the mode doesn't fit available BW */
> >  	DRM_DEBUG_KMS("Force DSC en = %d\n", intel_dp->force_dsc_en);
> > -- 
> > 2.20.1
> > 

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

* Re: [PATCH v2] drm/i915/dp: revert back to max link rate and lane count on eDP
  2019-04-05 20:13     ` Rodrigo Vivi
@ 2019-04-05 20:20       ` Manasi Navare
  0 siblings, 0 replies; 13+ messages in thread
From: Manasi Navare @ 2019-04-05 20:20 UTC (permalink / raw)
  To: Rodrigo Vivi
  Cc: Jani Nikula, intel-gfx, Ville Syrjälä,
	Matt Atwood, Lee, Shawn C, Dave Airlie, stable

On Fri, Apr 05, 2019 at 01:13:11PM -0700, Rodrigo Vivi wrote:
> On Fri, Apr 05, 2019 at 11:18:25AM -0700, Manasi Navare wrote:
> > On Fri, Apr 05, 2019 at 10:52:20AM +0300, Jani Nikula wrote:
> > > Commit 7769db588384 ("drm/i915/dp: optimize eDP 1.4+ link config fast
> > > and narrow") started to optize the eDP 1.4+ link config, both per spec
> > > and as preparation for display stream compression support.
> > > 
> > > Sadly, we again face panels that flat out fail with parameters they
> > > claim to support. Revert, and go back to the drawing board.
> > 
> > Yup, already multiple users facing this issue with eDP 1.4 panels that require
> > max parameters to pass link train.
> 
> I was wondering if we should blacklist the panel/platform, but if
> there are multiple cases it is better to revert...

Yes the bug has been reported and seen by atleast 4 users on different eDP panel product IDs

> 
> Unless there's a way to try and fallback quickly?!

I submitted a patch earlier to fallback with max parameters if optimized ones dont work:
https://patchwork.freedesktop.org/patch/296273/?series=58975&rev=2

But airlied suugested a revert instead.

Manasi

> 
> > 
> > I hear you now :)
> > 
> > Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>
> > 
> > > 
> > > v2: Actually revert to max params instead of just wide-and-slow.
> > > 
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109959
> > > Fixes: 7769db588384 ("drm/i915/dp: optimize eDP 1.4+ link config fast and narrow")
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Cc: Manasi Navare <manasi.d.navare@intel.com>
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Cc: Matt Atwood <matthew.s.atwood@intel.com>
> > > Cc: "Lee, Shawn C" <shawn.c.lee@intel.com>
> > > Cc: Dave Airlie <airlied@gmail.com>
> > > Cc: intel-gfx@lists.freedesktop.org
> > > Cc: <stable@vger.kernel.org> # v5.0+
> > > Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_dp.c | 69 +++++----------------------------
> > >  1 file changed, 10 insertions(+), 59 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > index 72c490..dfa770 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -1856,42 +1856,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)
> > > -{
> > > -	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 (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;
> > > @@ -2028,15 +1992,13 @@ 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);
> > >  
> > > -	if (intel_dp_is_edp(intel_dp) && intel_dp->edp_dpcd[0] < DP_EDP_14) {
> > > +	if (intel_dp_is_edp(intel_dp)) {
> > >  		/*
> > >  		 * Use the maximum clock and number of lanes the eDP panel
> > > -		 * advertizes being capable of. The eDP 1.3 and earlier panels
> > > -		 * are generally designed to support only a single clock and
> > > -		 * lane configuration, and typically these values correspond to
> > > -		 * the native resolution of the panel. With eDP 1.4 rate select
> > > -		 * and DSC, this is decreasingly the case, and we need to be
> > > -		 * able to select less than maximum link config.
> > > +		 * advertizes being capable of. The panels are generally
> > > +		 * designed to support only a single clock and lane
> > > +		 * configuration, and typically these values correspond to the
> > > +		 * native resolution of the panel.
> > >  		 */
> > >  		limits.min_lane_count = limits.max_lane_count;
> > >  		limits.min_clock = limits.max_clock;
> > > @@ -2050,22 +2012,11 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
> > >  		      intel_dp->common_rates[limits.max_clock],
> > >  		      limits.max_bpp, adjusted_mode->crtc_clock);
> > >  
> > > -	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 use the max clock and lane count for eDP 1.3 and
> > > -		 * earlier, and fast vs. wide is irrelevant.
> > > -		 */
> > > -		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. This is the place to add alternative
> > > +	 * optimization policy.
> > > +	 */
> > > +	ret = intel_dp_compute_link_config_wide(intel_dp, pipe_config, &limits);
> > >  
> > >  	/* enable compression if the mode doesn't fit available BW */
> > >  	DRM_DEBUG_KMS("Force DSC en = %d\n", intel_dp->force_dsc_en);
> > > -- 
> > > 2.20.1
> > > 

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

* ✗ Fi.CI.IGT: failure for drm/i915/dp: revert back to max link rate and lane count on eDP (rev2)
  2019-04-05  7:24 [PATCH] drm/i915/dp: revert back to max link rate and lane count on eDP Jani Nikula
                   ` (2 preceding siblings ...)
  2019-04-05 11:24 ` ✓ Fi.CI.BAT: success for drm/i915/dp: revert back to max link rate and lane count on eDP (rev2) Patchwork
@ 2019-04-06  6:15 ` Patchwork
  3 siblings, 0 replies; 13+ messages in thread
From: Patchwork @ 2019-04-06  6:15 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/dp: revert back to max link rate and lane count on eDP (rev2)
URL   : https://patchwork.freedesktop.org/series/59039/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_5878_full -> Patchwork_12692_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_12692_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_12692_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@sw_sync@sync_busy_fork:
    - shard-iclb:         PASS -> INCOMPLETE

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_create@create-clear:
    - shard-iclb:         PASS -> INCOMPLETE [fdo#109100] +1

  * igt@gem_eio@unwedge-stress:
    - shard-snb:          PASS -> FAIL [fdo#109661]

  * igt@gem_ppgtt@blt-vs-render-ctxn:
    - shard-iclb:         PASS -> INCOMPLETE [fdo#109801]

  * igt@gem_pwrite@huge-cpu-fbr:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109290]

  * igt@gem_tiled_swapping@non-threaded:
    - shard-iclb:         PASS -> INCOMPLETE [fdo#108686]

  * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-a:
    - shard-iclb:         PASS -> DMESG-WARN [fdo#110222]

  * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-c:
    - shard-skl:          NOTRUN -> DMESG-WARN [fdo#110222]

  * igt@kms_cursor_crc@cursor-128x42-onscreen:
    - shard-apl:          PASS -> FAIL [fdo#103232]

  * igt@kms_cursor_legacy@cursor-vs-flip-legacy:
    - shard-iclb:         PASS -> FAIL [fdo#103355]

  * igt@kms_flip@dpms-vs-vblank-race-interruptible:
    - shard-glk:          PASS -> FAIL [fdo#103060]

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-skl:          NOTRUN -> FAIL [fdo#105363]

  * igt@kms_flip_tiling@flip-x-tiled:
    - shard-iclb:         PASS -> FAIL [fdo#108303]

  * igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite:
    - shard-iclb:         PASS -> FAIL [fdo#103167] +4

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-pri-shrfb-draw-mmap-cpu:
    - shard-apl:          NOTRUN -> SKIP [fdo#109271] +22

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-shrfb-msflip-blt:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] +40

  * igt@kms_frontbuffer_tracking@fbcpsr-2p-scndscrn-pri-shrfb-draw-render:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] +19

  * igt@kms_frontbuffer_tracking@fbcpsr-rgb101010-draw-render:
    - shard-iclb:         PASS -> FAIL [fdo#105682] / [fdo#109247]

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-fullscreen:
    - shard-iclb:         PASS -> FAIL [fdo#109247] +11

  * igt@kms_pipe_crc_basic@hang-read-crc-pipe-d:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +1

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b-frame-sequence:
    - shard-skl:          PASS -> FAIL [fdo#103191] / [fdo#107362]

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-d:
    - shard-skl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +16

  * igt@kms_plane@pixel-format-pipe-b-planes-source-clamping:
    - shard-glk:          PASS -> SKIP [fdo#109271]

  * igt@kms_plane_alpha_blend@pipe-a-alpha-basic:
    - shard-skl:          NOTRUN -> FAIL [fdo#107815] / [fdo#108145]

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-max:
    - shard-skl:          NOTRUN -> FAIL [fdo#108145] +1

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          NOTRUN -> FAIL [fdo#107815]
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +2

  * igt@kms_plane_scaling@pipe-c-scaler-with-clipping-clamping:
    - shard-apl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +2

  * igt@kms_psr@cursor_plane_onoff:
    - shard-iclb:         PASS -> FAIL [fdo#107383] / [fdo#110215] +1

  * igt@kms_psr@no_drrs:
    - shard-iclb:         PASS -> FAIL [fdo#108341]

  * igt@kms_psr@psr2_cursor_plane_onoff:
    - shard-iclb:         PASS -> SKIP [fdo#109441] +3

  * igt@kms_psr@suspend:
    - shard-skl:          PASS -> INCOMPLETE [fdo#107773]

  * igt@kms_setmode@basic:
    - shard-apl:          PASS -> FAIL [fdo#99912]

  * igt@perf_pmu@busy-accuracy-50-vcs1:
    - shard-skl:          NOTRUN -> SKIP [fdo#109271] +159

  * igt@prime_busy@wait-hang-bsd2:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109276]

  
#### Possible fixes ####

  * igt@gem_linear_blits@normal:
    - shard-iclb:         TIMEOUT [fdo#109673] -> PASS

  * igt@i915_pm_rpm@i2c:
    - shard-iclb:         DMESG-WARN [fdo#109982] -> PASS

  * igt@kms_atomic_transition@1x-modeset-transitions-nonblocking-fencing:
    - shard-apl:          FAIL [fdo#109660] -> PASS

  * igt@kms_atomic_transition@plane-toggle-modeset-transition:
    - shard-apl:          INCOMPLETE [fdo#103927] -> PASS

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-c:
    - shard-glk:          DMESG-WARN [fdo#110222] -> PASS

  * igt@kms_cursor_crc@cursor-256x256-suspend:
    - shard-skl:          INCOMPLETE [fdo#104108] -> PASS

  * igt@kms_flip@modeset-vs-vblank-race:
    - shard-kbl:          FAIL [fdo#103060] -> PASS

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-indfb-draw-blt:
    - shard-iclb:         FAIL [fdo#109247] -> PASS +19

  * igt@kms_frontbuffer_tracking@fbcpsr-rgb101010-draw-pwrite:
    - shard-iclb:         FAIL [fdo#105682] / [fdo#109247] -> PASS +1

  * igt@kms_frontbuffer_tracking@fbcpsr-rgb565-draw-blt:
    - shard-iclb:         FAIL [fdo#103167] -> PASS

  * igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min:
    - shard-skl:          FAIL [fdo#108145] -> PASS

  * igt@kms_plane_scaling@pipe-a-scaler-with-pixel-format:
    - shard-glk:          SKIP [fdo#109271] / [fdo#109278] -> PASS

  * igt@kms_psr2_su@page_flip:
    - shard-iclb:         SKIP [fdo#109642] -> PASS

  * igt@kms_psr@psr2_cursor_blt:
    - shard-iclb:         SKIP [fdo#109441] -> PASS

  * igt@kms_psr@sprite_mmap_gtt:
    - shard-iclb:         FAIL [fdo#107383] / [fdo#110215] -> PASS +2

  * igt@kms_rotation_crc@multiplane-rotation:
    - shard-kbl:          INCOMPLETE [fdo#103665] -> PASS

  * igt@kms_vblank@pipe-a-ts-continuation-modeset-rpm:
    - shard-apl:          FAIL [fdo#104894] -> PASS

  * igt@perf@polling:
    - shard-iclb:         FAIL [fdo#108587] -> PASS

  
  [fdo#103060]: https://bugs.freedesktop.org/show_bug.cgi?id=103060
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103355]: https://bugs.freedesktop.org/show_bug.cgi?id=103355
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#104894]: https://bugs.freedesktop.org/show_bug.cgi?id=104894
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105682]: https://bugs.freedesktop.org/show_bug.cgi?id=105682
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#107383]: https://bugs.freedesktop.org/show_bug.cgi?id=107383
  [fdo#107773]: https://bugs.freedesktop.org/show_bug.cgi?id=107773
  [fdo#107815]: https://bugs.freedesktop.org/show_bug.cgi?id=107815
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108303]: https://bugs.freedesktop.org/show_bug.cgi?id=108303
  [fdo#108341]: https://bugs.freedesktop.org/show_bug.cgi?id=108341
  [fdo#108587]: https://bugs.freedesktop.org/show_bug.cgi?id=108587
  [fdo#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
  [fdo#109100]: https://bugs.freedesktop.org/show_bug.cgi?id=109100
  [fdo#109247]: https://bugs.freedesktop.org/show_bug.cgi?id=109247
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109290]: https://bugs.freedesktop.org/show_bug.cgi?id=109290
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#109660]: https://bugs.freedesktop.org/show_bug.cgi?id=109660
  [fdo#109661]: https://bugs.freedesktop.org/show_bug.cgi?id=109661
  [fdo#109673]: https://bugs.freedesktop.org/show_bug.cgi?id=109673
  [fdo#109801]: https://bugs.freedesktop.org/show_bug.cgi?id=109801
  [fdo#109982]: https://bugs.freedesktop.org/show_bug.cgi?id=109982
  [fdo#110215]: https://bugs.freedesktop.org/show_bug.cgi?id=110215
  [fdo#110222]: https://bugs.freedesktop.org/show_bug.cgi?id=110222
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (10 -> 9)
------------------------------

  Missing    (1): shard-hsw 


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

    * Linux: CI_DRM_5878 -> Patchwork_12692

  CI_DRM_5878: ae3367460fdf5e6f5a47038c1281b502817184e1 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4931: 019f892e5d1a0a9643cb726c47ce2d99c14b444f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12692: a8c54be839b389e42731eb174f7a83d7682db411 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12692/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v2] drm/i915/dp: revert back to max link rate and lane count on eDP
  2019-04-05  7:52 ` [PATCH v2] " Jani Nikula
  2019-04-05 17:23   ` Rodrigo Vivi
  2019-04-05 18:18   ` Manasi Navare
@ 2019-04-08 14:53   ` Sasha Levin
  2 siblings, 0 replies; 13+ messages in thread
From: Sasha Levin @ 2019-04-08 14:53 UTC (permalink / raw)
  To: Sasha Levin, Jani Nikula; +Cc: intel-gfx, stable

Hi,

[This is an automated email]

This commit has been processed because it contains a "Fixes:" tag,
fixing commit: 7769db588384 drm/i915/dp: optimize eDP 1.4+ link config fast and narrow.

The bot has tested the following trees: v5.0.7.

v5.0.7: Failed to apply! Possible dependencies:
    204474a6b859 ("drm/i915: Pass down rc in intel_encoder->compute_config()")
    e845f099f1c6 ("drm/i915/dsc: Add Per connector debugfs node for DSC support/enable")
    f6bff60e927b ("drm/i915/icl: Fix HPD handling for TypeC legacy ports")


How should we proceed with this patch?

--
Thanks,
Sasha
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v2] drm/i915/dp: revert back to max link rate and lane count on eDP
  2019-04-05 17:23   ` Rodrigo Vivi
@ 2019-04-10 12:54     ` Jani Nikula
  2019-04-10 18:00         ` Rodrigo Vivi
  0 siblings, 1 reply; 13+ messages in thread
From: Jani Nikula @ 2019-04-10 12:54 UTC (permalink / raw)
  To: Rodrigo Vivi
  Cc: intel-gfx, Ville Syrjälä,
	Manasi Navare, Matt Atwood, Lee, Shawn C, Dave Airlie, stable

On Fri, 05 Apr 2019, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> On Fri, Apr 05, 2019 at 10:52:20AM +0300, Jani Nikula wrote:
>> Commit 7769db588384 ("drm/i915/dp: optimize eDP 1.4+ link config fast
>> and narrow") started to optize the eDP 1.4+ link config, both per spec
>> and as preparation for display stream compression support.
>> 
>> Sadly, we again face panels that flat out fail with parameters they
>> claim to support. Revert, and go back to the drawing board.
>> 
>> v2: Actually revert to max params instead of just wide-and-slow.
>> 
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109959
>> Fixes: 7769db588384 ("drm/i915/dp: optimize eDP 1.4+ link config fast and narrow")
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Cc: Manasi Navare <manasi.d.navare@intel.com>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Cc: Matt Atwood <matthew.s.atwood@intel.com>
>> Cc: "Lee, Shawn C" <shawn.c.lee@intel.com>
>> Cc: Dave Airlie <airlied@gmail.com>
>> Cc: intel-gfx@lists.freedesktop.org
>> Cc: <stable@vger.kernel.org> # v5.0+
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>
> I can hear from here your "I told you so"... Sorry!
>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Thanks, pushed, please cherry-pick this for drm-intel-fixes.

BR,
Jani.



>
>> ---
>>  drivers/gpu/drm/i915/intel_dp.c | 69 +++++----------------------------
>>  1 file changed, 10 insertions(+), 59 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index 72c490..dfa770 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -1856,42 +1856,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)
>> -{
>> -	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 (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;
>> @@ -2028,15 +1992,13 @@ 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);
>>  
>> -	if (intel_dp_is_edp(intel_dp) && intel_dp->edp_dpcd[0] < DP_EDP_14) {
>> +	if (intel_dp_is_edp(intel_dp)) {
>>  		/*
>>  		 * Use the maximum clock and number of lanes the eDP panel
>> -		 * advertizes being capable of. The eDP 1.3 and earlier panels
>> -		 * are generally designed to support only a single clock and
>> -		 * lane configuration, and typically these values correspond to
>> -		 * the native resolution of the panel. With eDP 1.4 rate select
>> -		 * and DSC, this is decreasingly the case, and we need to be
>> -		 * able to select less than maximum link config.
>> +		 * advertizes being capable of. The panels are generally
>> +		 * designed to support only a single clock and lane
>> +		 * configuration, and typically these values correspond to the
>> +		 * native resolution of the panel.
>>  		 */
>>  		limits.min_lane_count = limits.max_lane_count;
>>  		limits.min_clock = limits.max_clock;
>> @@ -2050,22 +2012,11 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
>>  		      intel_dp->common_rates[limits.max_clock],
>>  		      limits.max_bpp, adjusted_mode->crtc_clock);
>>  
>> -	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 use the max clock and lane count for eDP 1.3 and
>> -		 * earlier, and fast vs. wide is irrelevant.
>> -		 */
>> -		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. This is the place to add alternative
>> +	 * optimization policy.
>> +	 */
>> +	ret = intel_dp_compute_link_config_wide(intel_dp, pipe_config, &limits);
>>  
>>  	/* enable compression if the mode doesn't fit available BW */
>>  	DRM_DEBUG_KMS("Force DSC en = %d\n", intel_dp->force_dsc_en);
>> -- 
>> 2.20.1
>> 

-- 
Jani Nikula, Intel Open Source Graphics Center

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

* Re: [Intel-gfx] [PATCH v2] drm/i915/dp: revert back to max link rate and lane count on eDP
  2019-04-10 12:54     ` Jani Nikula
@ 2019-04-10 18:00         ` Rodrigo Vivi
  0 siblings, 0 replies; 13+ messages in thread
From: Rodrigo Vivi @ 2019-04-10 18:00 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx, stable

On Wed, Apr 10, 2019 at 03:54:23PM +0300, Jani Nikula wrote:
> On Fri, 05 Apr 2019, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> > On Fri, Apr 05, 2019 at 10:52:20AM +0300, Jani Nikula wrote:
> >> Commit 7769db588384 ("drm/i915/dp: optimize eDP 1.4+ link config fast
> >> and narrow") started to optize the eDP 1.4+ link config, both per spec
> >> and as preparation for display stream compression support.
> >> 
> >> Sadly, we again face panels that flat out fail with parameters they
> >> claim to support. Revert, and go back to the drawing board.
> >> 
> >> v2: Actually revert to max params instead of just wide-and-slow.
> >> 
> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109959
> >> Fixes: 7769db588384 ("drm/i915/dp: optimize eDP 1.4+ link config fast and narrow")
> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> Cc: Manasi Navare <manasi.d.navare@intel.com>
> >> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> >> Cc: Matt Atwood <matthew.s.atwood@intel.com>
> >> Cc: "Lee, Shawn C" <shawn.c.lee@intel.com>
> >> Cc: Dave Airlie <airlied@gmail.com>
> >> Cc: intel-gfx@lists.freedesktop.org
> >> Cc: <stable@vger.kernel.org> # v5.0+
> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> >
> > I can hear from here your "I told you so"... Sorry!
> >
> > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> 
> Thanks, pushed, please cherry-pick this for drm-intel-fixes.

done. going on tomorrow's pull request...

> 
> BR,
> Jani.
> 
> 
> 
> >
> >> ---
> >>  drivers/gpu/drm/i915/intel_dp.c | 69 +++++----------------------------
> >>  1 file changed, 10 insertions(+), 59 deletions(-)
> >> 
> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> >> index 72c490..dfa770 100644
> >> --- a/drivers/gpu/drm/i915/intel_dp.c
> >> +++ b/drivers/gpu/drm/i915/intel_dp.c
> >> @@ -1856,42 +1856,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)
> >> -{
> >> -	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 (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;
> >> @@ -2028,15 +1992,13 @@ 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);
> >>  
> >> -	if (intel_dp_is_edp(intel_dp) && intel_dp->edp_dpcd[0] < DP_EDP_14) {
> >> +	if (intel_dp_is_edp(intel_dp)) {
> >>  		/*
> >>  		 * Use the maximum clock and number of lanes the eDP panel
> >> -		 * advertizes being capable of. The eDP 1.3 and earlier panels
> >> -		 * are generally designed to support only a single clock and
> >> -		 * lane configuration, and typically these values correspond to
> >> -		 * the native resolution of the panel. With eDP 1.4 rate select
> >> -		 * and DSC, this is decreasingly the case, and we need to be
> >> -		 * able to select less than maximum link config.
> >> +		 * advertizes being capable of. The panels are generally
> >> +		 * designed to support only a single clock and lane
> >> +		 * configuration, and typically these values correspond to the
> >> +		 * native resolution of the panel.
> >>  		 */
> >>  		limits.min_lane_count = limits.max_lane_count;
> >>  		limits.min_clock = limits.max_clock;
> >> @@ -2050,22 +2012,11 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
> >>  		      intel_dp->common_rates[limits.max_clock],
> >>  		      limits.max_bpp, adjusted_mode->crtc_clock);
> >>  
> >> -	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 use the max clock and lane count for eDP 1.3 and
> >> -		 * earlier, and fast vs. wide is irrelevant.
> >> -		 */
> >> -		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. This is the place to add alternative
> >> +	 * optimization policy.
> >> +	 */
> >> +	ret = intel_dp_compute_link_config_wide(intel_dp, pipe_config, &limits);
> >>  
> >>  	/* enable compression if the mode doesn't fit available BW */
> >>  	DRM_DEBUG_KMS("Force DSC en = %d\n", intel_dp->force_dsc_en);
> >> -- 
> >> 2.20.1
> >> 
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v2] drm/i915/dp: revert back to max link rate and lane count on eDP
@ 2019-04-10 18:00         ` Rodrigo Vivi
  0 siblings, 0 replies; 13+ messages in thread
From: Rodrigo Vivi @ 2019-04-10 18:00 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx, stable

On Wed, Apr 10, 2019 at 03:54:23PM +0300, Jani Nikula wrote:
> On Fri, 05 Apr 2019, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> > On Fri, Apr 05, 2019 at 10:52:20AM +0300, Jani Nikula wrote:
> >> Commit 7769db588384 ("drm/i915/dp: optimize eDP 1.4+ link config fast
> >> and narrow") started to optize the eDP 1.4+ link config, both per spec
> >> and as preparation for display stream compression support.
> >> 
> >> Sadly, we again face panels that flat out fail with parameters they
> >> claim to support. Revert, and go back to the drawing board.
> >> 
> >> v2: Actually revert to max params instead of just wide-and-slow.
> >> 
> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109959
> >> Fixes: 7769db588384 ("drm/i915/dp: optimize eDP 1.4+ link config fast and narrow")
> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> Cc: Manasi Navare <manasi.d.navare@intel.com>
> >> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> >> Cc: Matt Atwood <matthew.s.atwood@intel.com>
> >> Cc: "Lee, Shawn C" <shawn.c.lee@intel.com>
> >> Cc: Dave Airlie <airlied@gmail.com>
> >> Cc: intel-gfx@lists.freedesktop.org
> >> Cc: <stable@vger.kernel.org> # v5.0+
> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> >
> > I can hear from here your "I told you so"... Sorry!
> >
> > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> 
> Thanks, pushed, please cherry-pick this for drm-intel-fixes.

done. going on tomorrow's pull request...

> 
> BR,
> Jani.
> 
> 
> 
> >
> >> ---
> >>  drivers/gpu/drm/i915/intel_dp.c | 69 +++++----------------------------
> >>  1 file changed, 10 insertions(+), 59 deletions(-)
> >> 
> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> >> index 72c490..dfa770 100644
> >> --- a/drivers/gpu/drm/i915/intel_dp.c
> >> +++ b/drivers/gpu/drm/i915/intel_dp.c
> >> @@ -1856,42 +1856,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)
> >> -{
> >> -	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 (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;
> >> @@ -2028,15 +1992,13 @@ 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);
> >>  
> >> -	if (intel_dp_is_edp(intel_dp) && intel_dp->edp_dpcd[0] < DP_EDP_14) {
> >> +	if (intel_dp_is_edp(intel_dp)) {
> >>  		/*
> >>  		 * Use the maximum clock and number of lanes the eDP panel
> >> -		 * advertizes being capable of. The eDP 1.3 and earlier panels
> >> -		 * are generally designed to support only a single clock and
> >> -		 * lane configuration, and typically these values correspond to
> >> -		 * the native resolution of the panel. With eDP 1.4 rate select
> >> -		 * and DSC, this is decreasingly the case, and we need to be
> >> -		 * able to select less than maximum link config.
> >> +		 * advertizes being capable of. The panels are generally
> >> +		 * designed to support only a single clock and lane
> >> +		 * configuration, and typically these values correspond to the
> >> +		 * native resolution of the panel.
> >>  		 */
> >>  		limits.min_lane_count = limits.max_lane_count;
> >>  		limits.min_clock = limits.max_clock;
> >> @@ -2050,22 +2012,11 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
> >>  		      intel_dp->common_rates[limits.max_clock],
> >>  		      limits.max_bpp, adjusted_mode->crtc_clock);
> >>  
> >> -	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 use the max clock and lane count for eDP 1.3 and
> >> -		 * earlier, and fast vs. wide is irrelevant.
> >> -		 */
> >> -		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. This is the place to add alternative
> >> +	 * optimization policy.
> >> +	 */
> >> +	ret = intel_dp_compute_link_config_wide(intel_dp, pipe_config, &limits);
> >>  
> >>  	/* enable compression if the mode doesn't fit available BW */
> >>  	DRM_DEBUG_KMS("Force DSC en = %d\n", intel_dp->force_dsc_en);
> >> -- 
> >> 2.20.1
> >> 
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

end of thread, other threads:[~2019-04-10 18:00 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-05  7:24 [PATCH] drm/i915/dp: revert back to max link rate and lane count on eDP Jani Nikula
2019-04-05  7:50 ` Jani Nikula
2019-04-05  7:52 ` [PATCH v2] " Jani Nikula
2019-04-05 17:23   ` Rodrigo Vivi
2019-04-10 12:54     ` Jani Nikula
2019-04-10 18:00       ` [Intel-gfx] " Rodrigo Vivi
2019-04-10 18:00         ` Rodrigo Vivi
2019-04-05 18:18   ` Manasi Navare
2019-04-05 20:13     ` Rodrigo Vivi
2019-04-05 20:20       ` Manasi Navare
2019-04-08 14:53   ` Sasha Levin
2019-04-05 11:24 ` ✓ Fi.CI.BAT: success for drm/i915/dp: revert back to max link rate and lane count on eDP (rev2) Patchwork
2019-04-06  6:15 ` ✗ Fi.CI.IGT: failure " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.