On Wed, Sep 7, 2016 at 5:30 PM, Manasi Navare <manasi.d.navare@xxxxxxxxx> 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.
>
> v4:
> * Fixed the link rate fallback loop (Manasi Navare)
> v3:
> * Fixed some rebase issues (Mika Kahola)
> 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@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_ddi.c | 109 +++++++++++++++++++++++---
> 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 | 6 +-
> 4 files changed, 128 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 25e7973..1278daa 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,28 @@ 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)))
> + 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 +1711,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_OUTPUT_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,
> @@ -2432,6 +2459,68 @@ 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)
> +{
> + 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_index >= 0; link_rate_index --) {
ERROR: space prohibited before that '--' (ctx:WxB)
#148: FILE: drivers/gpu/drm/i915/intel_ddi.c:2483:
+ for (; link_rate_index >= 0; link_rate_index --) {
^
> + link_rate = common_rates[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");
The most common way in our code is to not split the comment and go over 80 cols.
checkpatch complains either ways anyway:
WARNING: quoted string split across lines
#153: FILE: drivers/gpu/drm/i915/intel_ddi.c:2488:
+ DRM_ERROR("Could not find DPLL for link "
+ "training.\n");
Not sure what our maintainer prefer. Jani?
> + 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 */
And here it goes over 80 cols the most common is to
/*
* Disable...
* ...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);
also here and in 2 other places in
drm-i915-Make-DP-link-training-channel-equalization-.patch
Thanks,
Rodrigo.
> + break;
> + }
> + }
> + intel_dp_stop_link_train(intel_dp);
> +
> + 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 75ac62f..1378116 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 c438b02..f1e08f0 100644
> --- a/drivers/gpu/drm/i915/intel_dp_link_training.c
> +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
> @@ -313,9 +313,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 ca51e1a..5b97a7d4 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1160,6 +1160,8 @@ 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);
> 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 +1383,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 +1405,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);
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
