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

2016-09-06 Thread Manasi Navare 
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 
> > ---
> >  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(>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(>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)
> >  

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

2016-09-06 Thread Manasi Navare 
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 
> > ---
> >  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(>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(>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,
> >

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

2016-09-06 Thread Manasi Navare 
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 
> > ---
> >  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(>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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

2016-09-02 Thread Jim Bride 
On Fri, Sep 02, 2016 at 07:52:53PM +, 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 
> > ---
> >  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(>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(>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,
> >

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

2016-09-02 Thread Pandiyan, Dhinakaran 
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 
> ---
>  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(>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(>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,
> >config->base.adjusted_mode,
> crtc->config->shared_dpll);
> - }
> +
>  }
>  
>  static void

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

2016-09-02 Thread Mika Kahola 
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 
> ---
>  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(>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(>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,
>  

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

2016-09-02 Thread 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 
> ---
>  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(>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(>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,
> >config->base.adjusted_mode,
> crtc->config->shared_dpll);
> - }
> +
>  }
>  
>  static void

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

2016-09-02 Thread 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 
> ---
>  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(>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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

2016-09-01 Thread Manasi Navare 
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 
---
 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(>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(>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,
  >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