[Intel-gfx] [PATCH v2 11/14] drm/i915: Fallback to lower link rate and lane count during link training

Mika Kahola mika.kahola at intel.com
Wed Sep 7 09:47:49 UTC 2016


On Tue, 2016-09-06 at 17:13 -0700, Manasi Navare wrote:
> According to the DisplayPort Spec, in case of Clock Recovery failure
> the link training sequence should fall back to the lower link rate
> followed by lower lane count until CR succeeds.
> On CR success, the sequence proceeds with Channel EQ.
> In case of Channel EQ failures, it should fallback to
> lower link rate and lane count and start the CR phase again.
> 
> v2:
> * Add a helper function to return index of requested link rate
> into common_rates array
> * Changed the link rate fallback loop to make use
> of common_rates array (Mika Kahola)
> * Changed INTEL_INFO to INTEL_GEN (David Weinehall)
> 
> Signed-off-by: Manasi Navare <manasi.d.navare at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c              | 125
> +++++++++++++++++++++++---
>  drivers/gpu/drm/i915/intel_dp.c               |  15 ++++
>  drivers/gpu/drm/i915/intel_dp_link_training.c |  12 ++-
>  drivers/gpu/drm/i915/intel_drv.h              |   7 +-
>  4 files changed, 145 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> b/drivers/gpu/drm/i915/intel_ddi.c
> index 67a6a0b..e38bf4b 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1634,19 +1634,18 @@ void intel_ddi_clk_select(struct
> intel_encoder *encoder,
>  	}
>  }
>  
> -static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
> +static void intel_ddi_pre_enable_edp(struct intel_encoder *encoder,
>  				    int link_rate, uint32_t
> lane_count,
> -				    struct intel_shared_dpll *pll,
> -				    bool link_mst)
> +				    struct intel_shared_dpll *pll)
>  {
>  	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
>  	struct drm_i915_private *dev_priv = to_i915(encoder-
> >base.dev);
>  	enum port port = intel_ddi_get_encoder_port(encoder);
>  
>  	intel_dp_set_link_params(intel_dp, link_rate, lane_count,
> -				 link_mst);
> -	if (encoder->type == INTEL_OUTPUT_EDP)
> -		intel_edp_panel_on(intel_dp);
> +				 false);
> +
> +	intel_edp_panel_on(intel_dp);
>  
>  	intel_ddi_clk_select(encoder, pll);
>  	intel_prepare_dp_ddi_buffers(encoder);
> @@ -1657,6 +1656,29 @@ static void intel_ddi_pre_enable_dp(struct
> intel_encoder *encoder,
>  		intel_dp_stop_link_train(intel_dp);
>  }
>  
> +static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
> +				    int link_rate, uint32_t
> lane_count,
> +				    struct intel_shared_dpll *pll,
> +				    bool link_mst)
> +{
> +	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> +	struct drm_i915_private *dev_priv = to_i915(encoder-
> >base.dev);
> +	struct intel_shared_dpll_config tmp_pll_config;
> +
> +	/* Disable the PLL and obtain the PLL for Link Training
> +	 * that starts with highest link rate and lane count.
> +	 */
> +	tmp_pll_config = pll->config;
> +	pll->funcs.disable(dev_priv, pll);
> +	pll->config.crtc_mask = 0;
> +
> +	/* If Link Training fails, send a uevent to generate a
> hotplug */
> +	if (!(intel_ddi_link_train(intel_dp, link_rate, lane_count,
> link_mst,
> +				   false)))
> +		drm_kms_helper_hotplug_event(encoder->base.dev);
> +	pll->config = tmp_pll_config;
> +}
> +
>  static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
>  				      bool has_hdmi_sink,
>  				      struct drm_display_mode
> *adjusted_mode,
> @@ -1690,20 +1712,26 @@ static void intel_ddi_pre_enable(struct
> intel_encoder *intel_encoder,
>  	struct intel_crtc *crtc = to_intel_crtc(encoder->crtc);
>  	int type = intel_encoder->type;
>  
> -	if (type == INTEL_OUTPUT_DP || type == INTEL_OUTPUT_EDP) {
> +	if (type == INTEL_OUTPUT_EDP)
> +		intel_ddi_pre_enable_edp(intel_encoder,
> +					crtc->config->port_clock,
> +					crtc->config->lane_count,
> +					crtc->config->shared_dpll);
> +
> +	if (type == INTEL_OUTPUT_DP)
>  		intel_ddi_pre_enable_dp(intel_encoder,
>  					crtc->config->port_clock,
>  					crtc->config->lane_count,
>  					crtc->config->shared_dpll,
>  					intel_crtc_has_type(crtc-
> >config,
>  							    INTEL_OU
> TPUT_DP_MST));
> -	}
> -	if (type == INTEL_OUTPUT_HDMI) {
> +
> +	if (type == INTEL_OUTPUT_HDMI)
>  		intel_ddi_pre_enable_hdmi(intel_encoder,
>  					  crtc->config-
> >has_hdmi_sink,
>  					  &crtc->config-
> >base.adjusted_mode,
>  					  crtc->config-
> >shared_dpll);
> -	}
> +
>  }
>  
>  static void intel_ddi_post_disable(struct intel_encoder
> *intel_encoder,
> @@ -2431,6 +2459,83 @@ intel_ddi_get_link_dpll(struct intel_dp
> *intel_dp, int clock)
>  	return pll;
>  }
>  
> +bool
> +intel_ddi_link_train(struct intel_dp *intel_dp, int max_link_rate,
> +		     uint8_t max_lane_count, bool link_mst, bool
> is_upfront)
> +{
> +	struct intel_connector *connector = intel_dp-
> >attached_connector;
> +	struct intel_encoder *encoder = connector->encoder;
> +	struct drm_i915_private *dev_priv = to_i915(encoder-
> >base.dev);
> +	struct intel_shared_dpll *pll;
> +	struct intel_shared_dpll_config tmp_pll_config;
> +	int link_rate, link_rate_index;
> +	uint8_t lane_count;
> +	int common_rates[DP_MAX_SUPPORTED_RATES] = {};
> +	bool ret = false;
> +
> +	link_rate_index = intel_dp_link_rate_index(intel_dp,
> common_rates,
> +						   max_link_rate);
> +	if (link_rate_index < 0) {
> +		DRM_ERROR("Invalid Link Rate\n");
> +		return false;
> +	}
> +	for (lane_count = max_lane_count; lane_count > 0; lane_count
> >>= 1) {
> +		for (link_rate = common_rates[link_rate_index];
> +		     link_rate_index >= 0; link_rate_index --) {
> +			pll = intel_ddi_get_link_dpll(intel_dp,
> link_rate);
> +			if (pll == NULL) {
> +				DRM_ERROR("Could not find DPLL for
> link "
> +					  "training.\n");
> +				return false;
> +			}
> +			tmp_pll_config = pll->config;
> +			pll->funcs.enable(dev_priv, pll);
> +
> +			intel_dp_set_link_params(intel_dp,
> link_rate,
> +						 lane_count,
> link_mst);
> +
> +			intel_ddi_clk_select(encoder, pll);
> +			intel_prepare_dp_ddi_buffers(encoder);
> +			intel_ddi_init_dp_buf_reg(encoder);
> +			intel_dp_sink_dpms(intel_dp,
> DRM_MODE_DPMS_ON);
> +			ret = intel_dp_start_link_train(intel_dp);
> +			if (ret)
> +				break;
> +
> +			/* Disable port followed by PLL for next
> retry/clean up */
> +			intel_ddi_post_disable(encoder, NULL, NULL);
> +			pll->funcs.disable(dev_priv, pll);
> +			pll->config = tmp_pll_config;
> +		}
> +		if (ret) {
> +			DRM_DEBUG_KMS("Link Training successful at
> link rate: "
> +				      "%d lane:%d\n", link_rate,
> lane_count);
> +			break;
> +		}
> +	}
> +	intel_dp_stop_link_train(intel_dp);
> +
> +	if (is_upfront) {
> +		DRM_DEBUG_KMS("Upfront link train %s: link_clock:%d
> lanes:%d\n",
> +			      ret ? "Passed" : "Failed",
> +			      link_rate, lane_count);
> +		/* Disable port followed by PLL for next retry/clean
> up */
> +		intel_ddi_post_disable(encoder, NULL, NULL);
> +		pll->funcs.disable(dev_priv, pll);
> +		pll->config = tmp_pll_config;
In a case where link training was successful, I guess we could leave
the port on. Is there a reason why we should do otherwise?


> +		if (ret) {
> +			/* Save the upfront values */
> +			intel_dp->max_lanes_upfront = lane_count;
> +			intel_dp->max_link_rate_upfront = link_rate;
My compiler wasn't really a big fan of these new members in a intel_dp
struct so I these needs to be defined in intel_drv.h

drivers/gpu/drm/i915/intel_ddi.c: In function ‘intel_ddi_link_train’:
drivers/gpu/drm/i915/intel_ddi.c:2528:12: error: ‘struct intel_dp’ has
no member named ‘max_lanes_upfront’
    intel_dp->max_lanes_upfront = lane_count;
            ^
drivers/gpu/drm/i915/intel_ddi.c:2529:12: error: ‘struct intel_dp’ has
no member named ‘max_link_rate_upfront’

    intel_dp->max_link_rate_upfront = link_rate;
> +		}
> +	}
> +
> +	if (!lane_count)
> +		DRM_ERROR("Link Training Failed\n");
> +
> +	return ret;
> +}
> +
>  void intel_ddi_init(struct drm_device *dev, enum port port)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(dev);
> diff --git a/drivers/gpu/drm/i915/intel_dp.c
> b/drivers/gpu/drm/i915/intel_dp.c
> index dfdbe65..391e384 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1443,6 +1443,21 @@ intel_dp_max_link_rate(struct intel_dp
> *intel_dp)
>  	return rates[len - 1];
>  }
>  
> +int intel_dp_link_rate_index(struct intel_dp *intel_dp, int
> *common_rates,
> +			     int link_rate)
> +{
> +	int common_len;
> +	int index;
> +
> +	common_len = intel_dp_common_rates(intel_dp, common_rates);
> +	for (index = common_len - 1; index >= 0; index--) {
> +		if (link_rate == common_rates[index])
> +			return index;
> +	}
> +
> +	return -1;
> +}
> +
>  int intel_dp_rate_select(struct intel_dp *intel_dp, int rate)
>  {
>  	return rate_to_index(rate, intel_dp->sink_rates);
> diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c
> b/drivers/gpu/drm/i915/intel_dp_link_training.c
> index 0fb845d..705dbd1 100644
> --- a/drivers/gpu/drm/i915/intel_dp_link_training.c
> +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
> @@ -309,9 +309,15 @@ void intel_dp_stop_link_train(struct intel_dp
> *intel_dp)
>  				DP_TRAINING_PATTERN_DISABLE);
>  }
>  
> -void
> +bool
>  intel_dp_start_link_train(struct intel_dp *intel_dp)
>  {
> -	intel_dp_link_training_clock_recovery(intel_dp);
> -	intel_dp_link_training_channel_equalization(intel_dp);
> +	bool ret;
> +
> +	if (intel_dp_link_training_clock_recovery(intel_dp)) {
> +		ret =
> intel_dp_link_training_channel_equalization(intel_dp);
> +		if (ret)
> +			return true;
> +	}
> +	return false;
>  }
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index e5bc976..90e7b15 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1160,6 +1160,9 @@ void intel_ddi_clock_get(struct intel_encoder
> *encoder,
>  			 struct intel_crtc_state *pipe_config);
>  void intel_ddi_set_vc_payload_alloc(struct drm_crtc *crtc, bool
> state);
>  uint32_t ddi_signal_levels(struct intel_dp *intel_dp);
> +bool intel_ddi_link_train(struct intel_dp *intel_dp, int
> max_link_rate,
> +			  uint8_t max_lane_count, bool link_mst,
> +			  bool is_upfront);
>  struct intel_shared_dpll *intel_ddi_get_link_dpll(struct intel_dp
> *intel_dp,
>  						  int clock);
>  unsigned int intel_fb_align_height(struct drm_device *dev,
> @@ -1381,7 +1384,7 @@ bool intel_dp_init_connector(struct
> intel_digital_port *intel_dig_port,
>  void intel_dp_set_link_params(struct intel_dp *intel_dp,
>  			      int link_rate, uint8_t lane_count,
>  			      bool link_mst);
> -void intel_dp_start_link_train(struct intel_dp *intel_dp);
> +bool intel_dp_start_link_train(struct intel_dp *intel_dp);
>  void intel_dp_stop_link_train(struct intel_dp *intel_dp);
>  void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode);
>  void intel_dp_encoder_reset(struct drm_encoder *encoder);
> @@ -1403,6 +1406,8 @@ void intel_dp_add_properties(struct intel_dp
> *intel_dp, struct drm_connector *co
>  void intel_dp_mst_suspend(struct drm_device *dev);
>  void intel_dp_mst_resume(struct drm_device *dev);
>  int intel_dp_max_link_rate(struct intel_dp *intel_dp);
> +int intel_dp_link_rate_index(struct intel_dp *intel_dp, int
> *common_rates,
> +			     int link_rate);
>  int intel_dp_rate_select(struct intel_dp *intel_dp, int rate);
>  void intel_dp_hot_plug(struct intel_encoder *intel_encoder);
>  void intel_power_sequencer_reset(struct drm_i915_private *dev_priv);
-- 
Mika Kahola - Intel OTC



More information about the Intel-gfx mailing list