[Intel-gfx] [PATCH 4/5] drm/i915: Find fallback link rate/lane count

[Jani Nikula]

On Tue, 15 Nov 2016, Manasi Navare <manasi.d.navare at intel.com> wrote:
> If link training fails, then we need to fallback to lower
> link rate first and if link training fails at RBR, then
> fallback to lower lane count.
> This function finds the next lower link rate/lane count
> value after link training failure.
>
> v2:
> Squash the patch that returns the link rate index (Jani Nikula)
>
> Acked-by: Tony Cheng <tony.cheng at amd.com>
> Acked-by: Harry Wentland <harry.wentland at amd.com>
> Cc: Ville Syrjala <ville.syrjala at linux.intel.com>
> Cc: Jani Nikula <jani.nikula at linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter at intel.com>
> Signed-off-by: Manasi Navare <manasi.d.navare at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c  | 42 ++++++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h |  6 ++++++
>  2 files changed, 48 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index a1b0181..e87b451 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -288,6 +288,48 @@ static int intel_dp_common_rates(struct intel_dp *intel_dp,
>  			       common_rates);
>  }
>  
> +static 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 = 0; index < common_len; index++) {
> +		if (link_rate == common_rates[common_len - index - 1])
> +			return common_len - index - 1;
> +	}
> +
> +	return -1;
> +}
> +
> +int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
> +					    int link_rate, uint8_t lane_count)
> +{
> +	int common_rates[DP_MAX_SUPPORTED_RATES] = {};
> +	int common_len;
> +	int link_rate_index = -1;
> +
> +	common_len = intel_dp_common_rates(intel_dp, common_rates);
> +	link_rate_index = intel_dp_link_rate_index(intel_dp,
> +						   common_rates,
> +						   link_rate);
> +	if (link_rate_index > 0) {
> +		intel_dp->fallback_link_rate_index = link_rate_index - 1;
> +		intel_dp->fallback_link_rate = common_rates[intel_dp->fallback_link_rate_index];

fallback_link_rate_index and fallback_link_rate are two ways to express
the same thing, and you can derive one from the other. When you have
two, you run the risk of having them out of sync. I thought I'd
mentioned this in earlier review.

Please do not store fallback_link_rate_index in intel_dp. Only store
fallback_link_rate. You can use your intel_dp_link_rate_index() function
when you need to convert rate to index.

Moreover, resetting the state becomes more clear when all can be set to
0.

BR,
Jani.

> +		intel_dp->fallback_lane_count = intel_dp_max_lane_count(intel_dp);
> +	} else if (lane_count > 1) {
> +		intel_dp->fallback_link_rate_index = common_len - 1;
> +		intel_dp->fallback_link_rate = common_rates[intel_dp->fallback_link_rate_index];
> +		intel_dp->fallback_lane_count = lane_count >> 1;
> +	} else {
> +		DRM_ERROR("Link Training Unsuccessful\n");
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
>  static enum drm_mode_status
>  intel_dp_mode_valid(struct drm_connector *connector,
>  		    struct drm_display_mode *mode)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 75252ec..55ceb44 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -887,6 +887,10 @@ struct intel_dp {
>  	uint32_t DP;
>  	int link_rate;
>  	uint8_t lane_count;
> +	int fallback_link_rate;
> +	uint8_t fallback_lane_count;
> +	int fallback_link_rate_index;
> +	bool link_train_failed;
>  	uint8_t sink_count;
>  	bool link_mst;
>  	bool has_audio;
> @@ -1383,6 +1387,8 @@ 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);
> +int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
> +					    int link_rate, uint8_t lane_count);
>  void 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);

-- 
Jani Nikula, Intel Open Source Technology Center