| Message ID | 1472767699-31211-12-git-send-email-manasi.d.navare@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Sep 01, 2016 at 03:08:16PM -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. > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > --- > drivers/gpu/drm/i915/intel_ddi.c | 109 +++++++++++++++++++++++--- > drivers/gpu/drm/i915/intel_dp_link_training.c | 12 ++- > drivers/gpu/drm/i915/intel_drv.h | 4 +- > 3 files changed, 110 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > index 67a6a0b..78d6687 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -1634,29 +1634,50 @@ 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); > intel_ddi_init_dp_buf_reg(encoder); > intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); > intel_dp_start_link_train(intel_dp); > - if (port != PORT_A || INTEL_GEN(dev_priv) >= 9) > + if (port != PORT_A || INTEL_INFO(dev_priv)->gen >= 9) I think you got this backwards. We *want* to use INTEL_GEN(). [snip] Kind regards, David Weinehall
On Thu, Sep 01, 2016 at 03:08:16PM -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. > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > --- > drivers/gpu/drm/i915/intel_ddi.c | 109 +++++++++++++++++++++++--- > drivers/gpu/drm/i915/intel_dp_link_training.c | 12 ++- > drivers/gpu/drm/i915/intel_drv.h | 4 +- > 3 files changed, 110 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > index 67a6a0b..78d6687 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -1634,29 +1634,50 @@ 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); > intel_ddi_init_dp_buf_reg(encoder); > intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); > intel_dp_start_link_train(intel_dp); > - if (port != PORT_A || INTEL_GEN(dev_priv) >= 9) > + if (port != PORT_A || INTEL_INFO(dev_priv)->gen >= 9) > 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, > @@ -2431,6 +2458,66 @@ 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; > + uint8_t lane_count; > + bool link_rate_not_found = true; > + bool ret = false; > + > + for (lane_count = max_lane_count; lane_count > 0; lane_count >>= 1) { > + > + link_rate = max_link_rate; > + while (link_rate_not_found) { > + 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); Do you really need to run these two for every link rate? > + intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); > + ret = intel_dp_start_link_train(intel_dp); > + if (ret) > + break; Where do you stop link training on failure? Perhaps: ret = intel_dp_start_link_train(intel_dp); intel_dp_stop_link_train(intel_dp); if (ret) break; and remove the stop on the success path? > + > + /* Disable port followed by PLL for next retry/clean up */ > + intel_ddi_post_disable(encoder, NULL, NULL); > + pll->funcs.disable(dev_priv, pll); > + if (link_rate == 540000) > + link_rate = 270000; > + else if (link_rate == 270000) > + link_rate = 162000; > + else > + link_rate_not_found = false; > + } > + if (ret) { > + DRM_DEBUG_KMS("Link Training successful\n"); > + intel_dp_stop_link_train(intel_dp); > + break; > + } > + } > + 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_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c > index 07f0159..0df49e8 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..342a2d5 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); > -- > 1.9.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, 2016-09-01 at 15:08 -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. > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > --- > drivers/gpu/drm/i915/intel_ddi.c | 109 > +++++++++++++++++++++++--- > drivers/gpu/drm/i915/intel_dp_link_training.c | 12 ++- > drivers/gpu/drm/i915/intel_drv.h | 4 +- > 3 files changed, 110 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > b/drivers/gpu/drm/i915/intel_ddi.c > index 67a6a0b..78d6687 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -1634,29 +1634,50 @@ 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); > intel_ddi_init_dp_buf_reg(encoder); > intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); > intel_dp_start_link_train(intel_dp); > - if (port != PORT_A || INTEL_GEN(dev_priv) >= 9) > + if (port != PORT_A || INTEL_INFO(dev_priv)->gen >= 9) > 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); At first glance, this seems that hotplug events are generated every time when link training fails. Is there a way to limit the number of link training tries so that we don't end up generating hotplug events over and over again? > + 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_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 +2458,66 @@ 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; > + uint8_t lane_count; > + bool link_rate_not_found = true; > + bool ret = false; > + > + for (lane_count = max_lane_count; lane_count > 0; lane_count > >>= 1) { > + > + link_rate = max_link_rate; > + while (link_rate_not_found) { At first glance, I read this as we haven't found the link rate yet and start from 0. This is not the case here as we have found the link rate which is the maximum link rate available. As we iterate link rates from max to min should we switch the logic here too? I mean something like this bool link_rate_not_found = false; ... while (!link_rate_not_found) { ... } The other way of doing this is not to use the boolean link_rate_not_found parameter at all but utilize the link_rate parameter only and iterate in a loop as long as the link_rate exceeds zero value. Anyway, this was just a thought and is more related on personal preference than the functionality. > + 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); It should be safe to move this ddi buffer initialization outside the loop. > + 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); > + if (link_rate == 540000) > + link_rate = 270000; > + else if (link_rate == 270000) > + link_rate = 162000; > + else > + link_rate_not_found = false; > + } > + if (ret) { > + DRM_DEBUG_KMS("Link Training successful\n"); > + intel_dp_stop_link_train(intel_dp); > + break; > + } > + } > + if (!lane_count) > + DRM_ERROR("Link Training Failed\n"); In case of failure, I think we should still stop the link training and call 'intel_dp_stop_link_train()' > + > + 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_link_training.c > b/drivers/gpu/drm/i915/intel_dp_link_training.c > index 07f0159..0df49e8 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; yep, this is what I had in mind when I reviewed the previous patch. > } > diff --git a/drivers/gpu/drm/i915/intel_drv.h > b/drivers/gpu/drm/i915/intel_drv.h > index e5bc976..342a2d5 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);
On Thu, 2016-09-01 at 15:08 -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. > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > --- > drivers/gpu/drm/i915/intel_ddi.c | 109 +++++++++++++++++++++++--- > drivers/gpu/drm/i915/intel_dp_link_training.c | 12 ++- > drivers/gpu/drm/i915/intel_drv.h | 4 +- > 3 files changed, 110 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > index 67a6a0b..78d6687 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -1634,29 +1634,50 @@ 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); > intel_ddi_init_dp_buf_reg(encoder); > intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); > intel_dp_start_link_train(intel_dp); > - if (port != PORT_A || INTEL_GEN(dev_priv) >= 9) > + if (port != PORT_A || INTEL_INFO(dev_priv)->gen >= 9) > 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, > @@ -2431,6 +2458,66 @@ 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; > + uint8_t lane_count; > + bool link_rate_not_found = true; > + bool ret = false; > + > + for (lane_count = max_lane_count; lane_count > 0; lane_count >>= 1) { > + > + link_rate = max_link_rate; > + while (link_rate_not_found) { > + 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); > + if (link_rate == 540000) > + link_rate = 270000; > + else if (link_rate == 270000) > + link_rate = 162000; > + else > + link_rate_not_found = false; 1. Aren't the supported rates platform dependent? I see these definitions in intel_dp.c static const int bxt_rates[] = { 162000, 216000, 243000, 270000, 324000, 432000, 540000 }; static const int skl_rates[] = { 162000, 216000, 270000, 324000, 432000, 540000 }; 2. Also, it is good to have the standard rates defined in a array within a header instead of having literals here. Or even better, re-use what's available in intel_dp.c static const int default_rates[] = { 162000, 270000, 540000 }; > + } > + if (ret) { > + DRM_DEBUG_KMS("Link Training successful\n"); > + intel_dp_stop_link_train(intel_dp); > + break; > + } > + } > + 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_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c > index 07f0159..0df49e8 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..342a2d5 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);
On Fri, Sep 02, 2016 at 07:52:53PM +0000, Pandiyan, Dhinakaran wrote: > On Thu, 2016-09-01 at 15:08 -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. > > > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > --- > > drivers/gpu/drm/i915/intel_ddi.c | 109 +++++++++++++++++++++++--- > > drivers/gpu/drm/i915/intel_dp_link_training.c | 12 ++- > > drivers/gpu/drm/i915/intel_drv.h | 4 +- > > 3 files changed, 110 insertions(+), 15 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > > index 67a6a0b..78d6687 100644 > > --- a/drivers/gpu/drm/i915/intel_ddi.c > > +++ b/drivers/gpu/drm/i915/intel_ddi.c > > @@ -1634,29 +1634,50 @@ 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); > > intel_ddi_init_dp_buf_reg(encoder); > > intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); > > intel_dp_start_link_train(intel_dp); > > - if (port != PORT_A || INTEL_GEN(dev_priv) >= 9) > > + if (port != PORT_A || INTEL_INFO(dev_priv)->gen >= 9) > > 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, > > @@ -2431,6 +2458,66 @@ 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; > > + uint8_t lane_count; > > + bool link_rate_not_found = true; > > + bool ret = false; > > + > > + for (lane_count = max_lane_count; lane_count > 0; lane_count >>= 1) { > > + > > + link_rate = max_link_rate; > > + while (link_rate_not_found) { > > + 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); > > + if (link_rate == 540000) > > + link_rate = 270000; > > + else if (link_rate == 270000) > > + link_rate = 162000; > > + else > > + link_rate_not_found = false; > > 1. Aren't the supported rates platform dependent? I see these > definitions in intel_dp.c > > static const int bxt_rates[] = { 162000, 216000, 243000, 270000, > 324000, 432000, 540000 }; > static const int skl_rates[] = { 162000, 216000, 270000, > 324000, 432000, 540000 }; > > > 2. Also, it is good to have the standard rates defined in a array within > a header instead of having literals here. > > Or even better, re-use what's available in intel_dp.c > static const int default_rates[] = { 162000, 270000, 540000 }; Actually, we should be fetching and using the common rates here. There's already code to take the union of what the source and sink support. Jim > > > + } > > + if (ret) { > > + DRM_DEBUG_KMS("Link Training successful\n"); > > + intel_dp_stop_link_train(intel_dp); > > + break; > > + } > > + } > > + 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_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c > > index 07f0159..0df49e8 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..342a2d5 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); > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Fri, Sep 02, 2016 at 03:03:22PM +0300, David Weinehall wrote: > On Thu, Sep 01, 2016 at 03:08:16PM -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. > > > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > --- > > drivers/gpu/drm/i915/intel_ddi.c | 109 +++++++++++++++++++++++--- > > drivers/gpu/drm/i915/intel_dp_link_training.c | 12 ++- > > drivers/gpu/drm/i915/intel_drv.h | 4 +- > > 3 files changed, 110 insertions(+), 15 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > > index 67a6a0b..78d6687 100644 > > --- a/drivers/gpu/drm/i915/intel_ddi.c > > +++ b/drivers/gpu/drm/i915/intel_ddi.c > > @@ -1634,29 +1634,50 @@ 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); > > intel_ddi_init_dp_buf_reg(encoder); > > intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); > > intel_dp_start_link_train(intel_dp); > > - if (port != PORT_A || INTEL_GEN(dev_priv) >= 9) > > + if (port != PORT_A || INTEL_INFO(dev_priv)->gen >= 9) > > I think you got this backwards. We *want* to use INTEL_GEN(). Thanks for your review. I think it got reverted due to rebasing. I will correct it and resend the patch. Manasi > > [snip] > > > Kind regards, David Weinehall
On Fri, Sep 02, 2016 at 03:49:02PM +0300, David Weinehall wrote: > On Thu, Sep 01, 2016 at 03:08:16PM -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. > > > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > --- > > drivers/gpu/drm/i915/intel_ddi.c | 109 +++++++++++++++++++++++--- > > drivers/gpu/drm/i915/intel_dp_link_training.c | 12 ++- > > drivers/gpu/drm/i915/intel_drv.h | 4 +- > > 3 files changed, 110 insertions(+), 15 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > > index 67a6a0b..78d6687 100644 > > --- a/drivers/gpu/drm/i915/intel_ddi.c > > +++ b/drivers/gpu/drm/i915/intel_ddi.c > > @@ -1634,29 +1634,50 @@ 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); > > intel_ddi_init_dp_buf_reg(encoder); > > intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); > > intel_dp_start_link_train(intel_dp); > > - if (port != PORT_A || INTEL_GEN(dev_priv) >= 9) > > + if (port != PORT_A || INTEL_INFO(dev_priv)->gen >= 9) > > 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, > > @@ -2431,6 +2458,66 @@ 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; > > + uint8_t lane_count; > > + bool link_rate_not_found = true; > > + bool ret = false; > > + > > + for (lane_count = max_lane_count; lane_count > 0; lane_count >>= 1) { > > + > > + link_rate = max_link_rate; > > + while (link_rate_not_found) { > > + 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); > > Do you really need to run these two for every link rate? According to Bspec on link training failure when we change the frequency, it should follow the enable sequence steps to enable PLL and port which include configuring DDI_BUF_TRANS and DDI_BUF_CTL. > > > + intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); > > + ret = intel_dp_start_link_train(intel_dp); > > + if (ret) > > + break; > > Where do you stop link training on failure? Perhaps: > > ret = intel_dp_start_link_train(intel_dp); > intel_dp_stop_link_train(intel_dp); > if (ret) > break; > > and remove the stop on the success path? > I will make this change and resubmit. > > + > > + /* Disable port followed by PLL for next retry/clean up */ > > + intel_ddi_post_disable(encoder, NULL, NULL); > > + pll->funcs.disable(dev_priv, pll); > > + if (link_rate == 540000) > > + link_rate = 270000; > > + else if (link_rate == 270000) > > + link_rate = 162000; > > + else > > + link_rate_not_found = false; > > + } > > + if (ret) { > > + DRM_DEBUG_KMS("Link Training successful\n"); > > + intel_dp_stop_link_train(intel_dp); > > + break; > > + } > > + } > > + 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_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c > > index 07f0159..0df49e8 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..342a2d5 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); > > -- > > 1.9.1 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Fri, Sep 02, 2016 at 04:00:20PM +0300, Mika Kahola wrote: > On Thu, 2016-09-01 at 15:08 -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. > > > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > --- > > drivers/gpu/drm/i915/intel_ddi.c | 109 > > +++++++++++++++++++++++--- > > drivers/gpu/drm/i915/intel_dp_link_training.c | 12 ++- > > drivers/gpu/drm/i915/intel_drv.h | 4 +- > > 3 files changed, 110 insertions(+), 15 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > > b/drivers/gpu/drm/i915/intel_ddi.c > > index 67a6a0b..78d6687 100644 > > --- a/drivers/gpu/drm/i915/intel_ddi.c > > +++ b/drivers/gpu/drm/i915/intel_ddi.c > > @@ -1634,29 +1634,50 @@ 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); > > intel_ddi_init_dp_buf_reg(encoder); > > intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); > > intel_dp_start_link_train(intel_dp); > > - if (port != PORT_A || INTEL_GEN(dev_priv) >= 9) > > + if (port != PORT_A || INTEL_INFO(dev_priv)->gen >= 9) > > 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); > At first glance, this seems that hotplug events are generated every > time when link training fails. Is there a way to limit the number of > link training tries so that we don't end up generating hotplug events > over and over again? This uevent will be sent only after retrying all the volatge swings/pre emphasis and trying at all all possible lane counts and link rates. Also since we are doing upfront link training, link training failing at this point and generating a hotplug event is very rare. I could possibly add a ERROR message to indicate that link training has failed after all retries before it generates a hotplug event. Manasi > > > + 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_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 +2458,66 @@ 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; > > + uint8_t lane_count; > > + bool link_rate_not_found = true; > > + bool ret = false; > > + > > + for (lane_count = max_lane_count; lane_count > 0; lane_count > > >>= 1) { > > + > > + link_rate = max_link_rate; > > + while (link_rate_not_found) { > At first glance, I read this as we haven't found the link rate yet and > start from 0. This is not the case here as we have found the link rate > which is the maximum link rate available. As we iterate link rates from > max to min should we switch the logic here too? I mean something like > this > > bool link_rate_not_found = false; > ... > while (!link_rate_not_found) { > ... > } > > The other way of doing this is not to use the boolean > link_rate_not_found parameter at all but utilize the link_rate > parameter only and iterate in a loop as long as the link_rate exceeds > zero value. Anyway, this was just a thought and is more related on > personal preference than the functionality. Thanks for the review comment. I will rework this logic to use the link rates/common rates array to iterate through the link rates from max to min. Manasi > > > + 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); > It should be safe to move this ddi buffer initialization outside the > loop. > > > + 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); > > + if (link_rate == 540000) > > + link_rate = 270000; > > + else if (link_rate == 270000) > > + link_rate = 162000; > > + else > > + link_rate_not_found = false; > > + } > > + if (ret) { > > + DRM_DEBUG_KMS("Link Training successful\n"); > > + intel_dp_stop_link_train(intel_dp); > > + break; > > + } > > + } > > + if (!lane_count) > > + DRM_ERROR("Link Training Failed\n"); > In case of failure, I think we should still stop the link training and > call 'intel_dp_stop_link_train()' > > > + > > + 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_link_training.c > > b/drivers/gpu/drm/i915/intel_dp_link_training.c > > index 07f0159..0df49e8 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; > yep, this is what I had in mind when I reviewed the previous patch. > > > } > > diff --git a/drivers/gpu/drm/i915/intel_drv.h > > b/drivers/gpu/drm/i915/intel_drv.h > > index e5bc976..342a2d5 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); > -- > Mika Kahola - Intel OTC >
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index 67a6a0b..78d6687 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -1634,29 +1634,50 @@ 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); intel_ddi_init_dp_buf_reg(encoder); intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); intel_dp_start_link_train(intel_dp); - if (port != PORT_A || INTEL_GEN(dev_priv) >= 9) + if (port != PORT_A || INTEL_INFO(dev_priv)->gen >= 9) 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, @@ -2431,6 +2458,66 @@ 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; + uint8_t lane_count; + bool link_rate_not_found = true; + bool ret = false; + + for (lane_count = max_lane_count; lane_count > 0; lane_count >>= 1) { + + link_rate = max_link_rate; + while (link_rate_not_found) { + 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); + if (link_rate == 540000) + link_rate = 270000; + else if (link_rate == 270000) + link_rate = 162000; + else + link_rate_not_found = false; + } + if (ret) { + DRM_DEBUG_KMS("Link Training successful\n"); + intel_dp_stop_link_train(intel_dp); + break; + } + } + 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_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c index 07f0159..0df49e8 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..342a2d5 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);
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. Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> --- drivers/gpu/drm/i915/intel_ddi.c | 109 +++++++++++++++++++++++--- drivers/gpu/drm/i915/intel_dp_link_training.c | 12 ++- drivers/gpu/drm/i915/intel_drv.h | 4 +- 3 files changed, 110 insertions(+), 15 deletions(-)