Re: [Intel-gfx] [PATCH 1/3] drm/i915: Try to use fast+narrow link on eDP again and fall back to the old max strategy on failure

[Manasi Navare <manasi.d.navare@intel.com>]

On Thu, Mar 19, 2020 at 06:38:42PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Some new eDP panels don't like to operate at the max parameters, and
> instead we need to go for an optimal confiugration. That unfortunately
> doesn't work with older eDP panels which are generally only guaranteed
> to work at the max parameters.
> 
> To solve these two conflicting requirements let's start with the optimal
> setup, and if that fails we start again with the max parameters. The
> downside is probably an extra modeset when we switch strategies but
> I don't see a good way to avoid that.
> 
> For a bit of history we first tried to go for the fast+narrow in
> commit 7769db588384 ("drm/i915/dp: optimize eDP 1.4+ link config
> fast and narrow"). but that had to be reverted due to regression
> on older panels in commit f11cb1c19ad0 ("drm/i915/dp: revert back
> to max link rate and lane count on eDP"). So now we try to get
> the best of both worlds by using both strategies.
> 
> v2: Deal with output_bpp and uapi vs. hw state split
>     Reword some comments
> 
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Albert Astals Cid <aacid@kde.org> # v5.0 backport
> Cc: Emanuele Panigati <ilpanich@gmail.com> # v5.0 backport
> Cc: Matteo Iervasi <matteoiervasi@gmail.com> # v5.0 backport
> Cc: Timo Aaltonen <tjaalton@ubuntu.com>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=105267
> References: https://bugs.freedesktop.org/show_bug.cgi?id=109959
> References: https://gitlab.freedesktop.org/drm/intel/issues/272
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

This approach looks good to me to fallback to max parameters if
it fails the first time.

> ---
>  .../drm/i915/display/intel_display_types.h    |  1 +
>  drivers/gpu/drm/i915/display/intel_dp.c       | 74 ++++++++++++++++---
>  2 files changed, 66 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 5e00e611f077..ffde0d4af23c 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1258,6 +1258,7 @@ struct intel_dp {
>  	bool link_trained;
>  	bool has_audio;
>  	bool reset_link_params;
> +	bool use_max_params;
>  	u8 dpcd[DP_RECEIVER_CAP_SIZE];
>  	u8 psr_dpcd[EDP_PSR_RECEIVER_CAP_SIZE];
>  	u8 downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index ef2e06e292d5..85abcce492ca 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -465,6 +465,12 @@ int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
>  {
>  	int index;
>  
> +	if (intel_dp_is_edp(intel_dp) && !intel_dp->use_max_params) {
> +		DRM_DEBUG_KMS("Retrying Link training for eDP with max parameters\n");
> +		intel_dp->use_max_params = true;
> +		return 0;
> +	}

We need to remove the current check for intel_dp_can_link_train_fallback_for_edp() right?

Manasi

> +
>  	index = intel_dp_rate_index(intel_dp->common_rates,
>  				    intel_dp->num_common_rates,
>  				    link_rate);
> @@ -2046,6 +2052,44 @@ 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, 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;
> @@ -2261,13 +2305,14 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
>  	limits.min_bpp = intel_dp_min_bpp(pipe_config);
>  	limits.max_bpp = intel_dp_compute_bpp(intel_dp, pipe_config);
>  
> -	if (intel_dp_is_edp(intel_dp)) {
> +	if (intel_dp->use_max_params) {
>  		/*
>  		 * Use the maximum clock and number of lanes the eDP panel
> -		 * advertizes being capable of. The panels are generally
> +		 * advertizes being capable of in case the initial fast
> +		 * optimal params failed us. 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.
> +		 * configuration, and typically on older panels these
> +		 * values correspond to the native resolution of the panel.
>  		 */
>  		limits.min_lane_count = limits.max_lane_count;
>  		limits.min_clock = limits.max_clock;
> @@ -2281,11 +2326,22 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
>  		      intel_dp->common_rates[limits.max_clock],
>  		      limits.max_bpp, adjusted_mode->crtc_clock);
>  
> -	/*
> -	 * 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);
> +	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);
>  
>  	/* enable compression if the mode doesn't fit available BW */
>  	DRM_DEBUG_KMS("Force DSC en = %d\n", intel_dp->force_dsc_en);
> -- 
> 2.24.1
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx