| Message ID | 1367249118-3212-1-git-send-email-imre.deak@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi 2013/4/29 Imre Deak <imre.deak@intel.com>: > According to BSpec the link training sequence for eDP on HSW port-A > should be as follows: > > 1. link training: clock recovery > 2. link training: equalization > 3. link training: set idle transmission mode > 4. display pipe enable > 5. link training: disable (set normal mode) > > Contrary to this at the moment we don't do step 3. and we do step 5. > before step 4. Fix this by setting idle transmission mode for eDP at > the end of intel_dp_complete_link_train and adding a new > intel_dp_stop_link_training function to disable link training. With > these changes we'll end up with the following functions corresponding > to the above steps: > > intel_dp_start_link_train -> step 1. > intel_dp_complete_link_train -> step 2., step 3. > intel_dp_stop_link_train -> step 5. > > For port-A we'll call intel_dp_stop_link_train only after enabling the > pipe, for everything else we'll call it right after > intel_dp_complete_link_train to preserve the current behavior. > > Tested on HSW/HSW-ULT. > > In v2: > - Due to a HW issue we must set idle transmission mode for port-A too > before enabling the pipe. Thanks for Arthur Runyan for explaining > this. > - Update the patch subject to make it clear that it's an eDP fix, DP is > not affected. > > Signed-off-by: Imre Deak <imre.deak@intel.com> > --- > drivers/gpu/drm/i915/intel_ddi.c | 5 ++++ > drivers/gpu/drm/i915/intel_dp.c | 59 ++++++++++++++++++++++++++++---------- > drivers/gpu/drm/i915/intel_drv.h | 1 + > 3 files changed, 50 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > index 0d7cf31..f3d0f21 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -1265,6 +1265,8 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder) > intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); > intel_dp_start_link_train(intel_dp); > intel_dp_complete_link_train(intel_dp); > + if (port != PORT_A) > + intel_dp_stop_link_train(intel_dp); > } > } > > @@ -1326,6 +1328,9 @@ static void intel_enable_ddi(struct intel_encoder *intel_encoder) > } else if (type == INTEL_OUTPUT_EDP) { > struct intel_dp *intel_dp = enc_to_intel_dp(encoder); > > + if (port == PORT_A) > + intel_dp_stop_link_train(intel_dp); > + > ironlake_edp_backlight_on(intel_dp); > } > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 13d2fd6..d4ddeb9 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -1437,6 +1437,7 @@ static void intel_enable_dp(struct intel_encoder *encoder) > ironlake_edp_panel_on(intel_dp); > ironlake_edp_panel_vdd_off(intel_dp, true); > intel_dp_complete_link_train(intel_dp); > + intel_dp_stop_link_train(intel_dp); > ironlake_edp_backlight_on(intel_dp); > > if (IS_VALLEYVIEW(dev)) { > @@ -1935,10 +1936,9 @@ intel_dp_set_link_train(struct intel_dp *intel_dp, > struct drm_i915_private *dev_priv = dev->dev_private; > enum port port = intel_dig_port->port; > int ret; > - uint32_t temp; > > if (HAS_DDI(dev)) { > - temp = I915_READ(DP_TP_CTL(port)); > + uint32_t temp = I915_READ(DP_TP_CTL(port)); > > if (dp_train_pat & DP_LINK_SCRAMBLING_DISABLE) > temp |= DP_TP_CTL_SCRAMBLE_DISABLE; > @@ -1948,18 +1948,6 @@ intel_dp_set_link_train(struct intel_dp *intel_dp, > temp &= ~DP_TP_CTL_LINK_TRAIN_MASK; > switch (dp_train_pat & DP_TRAINING_PATTERN_MASK) { > case DP_TRAINING_PATTERN_DISABLE: > - > - if (port != PORT_A) { > - temp |= DP_TP_CTL_LINK_TRAIN_IDLE; > - I915_WRITE(DP_TP_CTL(port), temp); > - > - if (wait_for((I915_READ(DP_TP_STATUS(port)) & > - DP_TP_STATUS_IDLE_DONE), 1)) > - DRM_ERROR("Timed out waiting for DP idle patterns\n"); > - > - temp &= ~DP_TP_CTL_LINK_TRAIN_MASK; > - } > - > temp |= DP_TP_CTL_LINK_TRAIN_NORMAL; > > break; > @@ -2035,6 +2023,37 @@ intel_dp_set_link_train(struct intel_dp *intel_dp, > return true; > } > > +static void intel_dp_idle_link_train(struct intel_dp *intel_dp) Maybe intel_dp_set_idle_link_train? > +{ > + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); > + struct drm_device *dev = intel_dig_port->base.base.dev; > + struct drm_i915_private *dev_priv = dev->dev_private; > + enum port port = intel_dig_port->port; > + uint32_t l; We usually use "val", "tmp" or "temp". L is unusual. > + > + if (!HAS_DDI(dev)) > + return; > + > + l = I915_READ(DP_TP_CTL(port)); > + l &= ~DP_TP_CTL_LINK_TRAIN_MASK; > + l |= DP_TP_CTL_LINK_TRAIN_IDLE; > + I915_WRITE(DP_TP_CTL(port), l); > + > + /* > + * On PORT_A we can have only eDP in SST mode. There the only reason > + * we need to set idle transmission mode is to work around a HW issue > + * where we enable the pipe while not in idle link-training mode. > + * In this case there is requirement to wait for a minimum number of > + * idle patterns to be sent. > + */ > + if (port == PORT_A) > + return; > + > + if (wait_for((I915_READ(DP_TP_STATUS(port)) & DP_TP_STATUS_IDLE_DONE), > + 1)) > + DRM_ERROR("Timed out waiting for DP idle patterns\n"); > +} > + > /* Enable corresponding port and start training pattern 1 */ > void > intel_dp_start_link_train(struct intel_dp *intel_dp) > @@ -2177,10 +2196,19 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp) > ++tries; > } > > + intel_dp_idle_link_train(intel_dp); > + > + intel_dp->DP = DP; I know this is not your fault, but I kinda _really_ don't like the fact that we have 3 places to store the contents of the same register: (i) the register itself, (ii) intel_dp->DP and (iii) these temporary DP variables. Whoever writes a patch to improve that without adding any regressions will get a free $favorite_beverage. I guess the first step is to try to remove the temporary variables only. So I've also booted your patch and both DP and eDP still work for me. With or without the changes I proposed above: Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> Tested-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > + > if (channel_eq) > DRM_DEBUG_KMS("Channel EQ done. DP Training successfull\n"); > > - intel_dp_set_link_train(intel_dp, DP, DP_TRAINING_PATTERN_DISABLE); > +} > + > +void intel_dp_stop_link_train(struct intel_dp *intel_dp) > +{ > + intel_dp_set_link_train(intel_dp, intel_dp->DP, > + DP_TRAINING_PATTERN_DISABLE); > } > > static void > @@ -2388,6 +2416,7 @@ intel_dp_check_link_status(struct intel_dp *intel_dp) > drm_get_encoder_name(&intel_encoder->base)); > intel_dp_start_link_train(intel_dp); > intel_dp_complete_link_train(intel_dp); > + intel_dp_stop_link_train(intel_dp); > } > } > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index a8c6960..6d1e63a 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -542,6 +542,7 @@ extern void intel_dp_init_connector(struct intel_digital_port *intel_dig_port, > extern void intel_dp_init_link_config(struct intel_dp *intel_dp); > extern void intel_dp_start_link_train(struct intel_dp *intel_dp); > extern void intel_dp_complete_link_train(struct intel_dp *intel_dp); > +extern void intel_dp_stop_link_train(struct intel_dp *intel_dp); > extern void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode); > extern void intel_dp_encoder_destroy(struct drm_encoder *encoder); > extern void intel_dp_check_link_status(struct intel_dp *intel_dp); > -- > 1.7.10.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, 2013-05-02 at 14:45 -0300, Paulo Zanoni wrote: > Hi > > 2013/4/29 Imre Deak <imre.deak@intel.com>: > > According to BSpec the link training sequence for eDP on HSW port-A > > should be as follows: > > > > 1. link training: clock recovery > > 2. link training: equalization > > 3. link training: set idle transmission mode > > 4. display pipe enable > > 5. link training: disable (set normal mode) > > > > Contrary to this at the moment we don't do step 3. and we do step 5. > > before step 4. Fix this by setting idle transmission mode for eDP at > > the end of intel_dp_complete_link_train and adding a new > > intel_dp_stop_link_training function to disable link training. With > > these changes we'll end up with the following functions corresponding > > to the above steps: > > > > intel_dp_start_link_train -> step 1. > > intel_dp_complete_link_train -> step 2., step 3. > > intel_dp_stop_link_train -> step 5. > > > > For port-A we'll call intel_dp_stop_link_train only after enabling the > > pipe, for everything else we'll call it right after > > intel_dp_complete_link_train to preserve the current behavior. > > > > Tested on HSW/HSW-ULT. > > > > In v2: > > - Due to a HW issue we must set idle transmission mode for port-A too > > before enabling the pipe. Thanks for Arthur Runyan for explaining > > this. > > - Update the patch subject to make it clear that it's an eDP fix, DP is > > not affected. > > > > Signed-off-by: Imre Deak <imre.deak@intel.com> > > --- > > drivers/gpu/drm/i915/intel_ddi.c | 5 ++++ > > drivers/gpu/drm/i915/intel_dp.c | 59 ++++++++++++++++++++++++++++---------- > > drivers/gpu/drm/i915/intel_drv.h | 1 + > > 3 files changed, 50 insertions(+), 15 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > > index 0d7cf31..f3d0f21 100644 > > --- a/drivers/gpu/drm/i915/intel_ddi.c > > +++ b/drivers/gpu/drm/i915/intel_ddi.c > > @@ -1265,6 +1265,8 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder) > > intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); > > intel_dp_start_link_train(intel_dp); > > intel_dp_complete_link_train(intel_dp); > > + if (port != PORT_A) > > + intel_dp_stop_link_train(intel_dp); > > } > > } > > > > @@ -1326,6 +1328,9 @@ static void intel_enable_ddi(struct intel_encoder *intel_encoder) > > } else if (type == INTEL_OUTPUT_EDP) { > > struct intel_dp *intel_dp = enc_to_intel_dp(encoder); > > > > + if (port == PORT_A) > > + intel_dp_stop_link_train(intel_dp); > > + > > ironlake_edp_backlight_on(intel_dp); > > } > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > > index 13d2fd6..d4ddeb9 100644 > > --- a/drivers/gpu/drm/i915/intel_dp.c > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > @@ -1437,6 +1437,7 @@ static void intel_enable_dp(struct intel_encoder *encoder) > > ironlake_edp_panel_on(intel_dp); > > ironlake_edp_panel_vdd_off(intel_dp, true); > > intel_dp_complete_link_train(intel_dp); > > + intel_dp_stop_link_train(intel_dp); > > ironlake_edp_backlight_on(intel_dp); > > > > if (IS_VALLEYVIEW(dev)) { > > @@ -1935,10 +1936,9 @@ intel_dp_set_link_train(struct intel_dp *intel_dp, > > struct drm_i915_private *dev_priv = dev->dev_private; > > enum port port = intel_dig_port->port; > > int ret; > > - uint32_t temp; > > > > if (HAS_DDI(dev)) { > > - temp = I915_READ(DP_TP_CTL(port)); > > + uint32_t temp = I915_READ(DP_TP_CTL(port)); > > > > if (dp_train_pat & DP_LINK_SCRAMBLING_DISABLE) > > temp |= DP_TP_CTL_SCRAMBLE_DISABLE; > > @@ -1948,18 +1948,6 @@ intel_dp_set_link_train(struct intel_dp *intel_dp, > > temp &= ~DP_TP_CTL_LINK_TRAIN_MASK; > > switch (dp_train_pat & DP_TRAINING_PATTERN_MASK) { > > case DP_TRAINING_PATTERN_DISABLE: > > - > > - if (port != PORT_A) { > > - temp |= DP_TP_CTL_LINK_TRAIN_IDLE; > > - I915_WRITE(DP_TP_CTL(port), temp); > > - > > - if (wait_for((I915_READ(DP_TP_STATUS(port)) & > > - DP_TP_STATUS_IDLE_DONE), 1)) > > - DRM_ERROR("Timed out waiting for DP idle patterns\n"); > > - > > - temp &= ~DP_TP_CTL_LINK_TRAIN_MASK; > > - } > > - > > temp |= DP_TP_CTL_LINK_TRAIN_NORMAL; > > > > break; > > @@ -2035,6 +2023,37 @@ intel_dp_set_link_train(struct intel_dp *intel_dp, > > return true; > > } > > > > +static void intel_dp_idle_link_train(struct intel_dp *intel_dp) > > Maybe intel_dp_set_idle_link_train? Yea, that's more like the other link_train functions. Will change it. > > > > +{ > > + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); > > + struct drm_device *dev = intel_dig_port->base.base.dev; > > + struct drm_i915_private *dev_priv = dev->dev_private; > > + enum port port = intel_dig_port->port; > > + uint32_t l; > > We usually use "val", "tmp" or "temp". L is unusual. Ok, will change. > > > > + > > + if (!HAS_DDI(dev)) > > + return; > > + > > + l = I915_READ(DP_TP_CTL(port)); > > + l &= ~DP_TP_CTL_LINK_TRAIN_MASK; > > + l |= DP_TP_CTL_LINK_TRAIN_IDLE; > > + I915_WRITE(DP_TP_CTL(port), l); > > + > > + /* > > + * On PORT_A we can have only eDP in SST mode. There the only reason > > + * we need to set idle transmission mode is to work around a HW issue > > + * where we enable the pipe while not in idle link-training mode. > > + * In this case there is requirement to wait for a minimum number of > > + * idle patterns to be sent. > > + */ > > + if (port == PORT_A) > > + return; > > + > > + if (wait_for((I915_READ(DP_TP_STATUS(port)) & DP_TP_STATUS_IDLE_DONE), > > + 1)) > > + DRM_ERROR("Timed out waiting for DP idle patterns\n"); > > +} > > + > > /* Enable corresponding port and start training pattern 1 */ > > void > > intel_dp_start_link_train(struct intel_dp *intel_dp) > > @@ -2177,10 +2196,19 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp) > > ++tries; > > } > > > > + intel_dp_idle_link_train(intel_dp); > > + > > + intel_dp->DP = DP; > > I know this is not your fault, but I kinda _really_ don't like the > fact that we have 3 places to store the contents of the same register: > (i) the register itself, (ii) intel_dp->DP and (iii) these temporary > DP variables. Whoever writes a patch to improve that without adding > any regressions will get a free $favorite_beverage. I guess the first > step is to try to remove the temporary variables only. Yea, I agree we should improve on this. You didn't insist, so I suggest that we do it after this patch gets merged. If there's no one else I can also volunteer to do this a bit later. > So I've also booted your patch and both DP and eDP still work for me. > With or without the changes I proposed above: > Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > Tested-by: Paulo Zanoni <paulo.r.zanoni@intel.com> Thanks, Imre > > > + > > if (channel_eq) > > DRM_DEBUG_KMS("Channel EQ done. DP Training successfull\n"); > > > > - intel_dp_set_link_train(intel_dp, DP, DP_TRAINING_PATTERN_DISABLE); > > +} > > + > > +void intel_dp_stop_link_train(struct intel_dp *intel_dp) > > +{ > > + intel_dp_set_link_train(intel_dp, intel_dp->DP, > > + DP_TRAINING_PATTERN_DISABLE); > > } > > > > static void > > @@ -2388,6 +2416,7 @@ intel_dp_check_link_status(struct intel_dp *intel_dp) > > drm_get_encoder_name(&intel_encoder->base)); > > intel_dp_start_link_train(intel_dp); > > intel_dp_complete_link_train(intel_dp); > > + intel_dp_stop_link_train(intel_dp); > > } > > } > > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > > index a8c6960..6d1e63a 100644 > > --- a/drivers/gpu/drm/i915/intel_drv.h > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > @@ -542,6 +542,7 @@ extern void intel_dp_init_connector(struct intel_digital_port *intel_dig_port, > > extern void intel_dp_init_link_config(struct intel_dp *intel_dp); > > extern void intel_dp_start_link_train(struct intel_dp *intel_dp); > > extern void intel_dp_complete_link_train(struct intel_dp *intel_dp); > > +extern void intel_dp_stop_link_train(struct intel_dp *intel_dp); > > extern void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode); > > extern void intel_dp_encoder_destroy(struct drm_encoder *encoder); > > extern void intel_dp_check_link_status(struct intel_dp *intel_dp); > > -- > > 1.7.10.4 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx >
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index 0d7cf31..f3d0f21 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -1265,6 +1265,8 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder) intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); intel_dp_start_link_train(intel_dp); intel_dp_complete_link_train(intel_dp); + if (port != PORT_A) + intel_dp_stop_link_train(intel_dp); } } @@ -1326,6 +1328,9 @@ static void intel_enable_ddi(struct intel_encoder *intel_encoder) } else if (type == INTEL_OUTPUT_EDP) { struct intel_dp *intel_dp = enc_to_intel_dp(encoder); + if (port == PORT_A) + intel_dp_stop_link_train(intel_dp); + ironlake_edp_backlight_on(intel_dp); } diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 13d2fd6..d4ddeb9 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1437,6 +1437,7 @@ static void intel_enable_dp(struct intel_encoder *encoder) ironlake_edp_panel_on(intel_dp); ironlake_edp_panel_vdd_off(intel_dp, true); intel_dp_complete_link_train(intel_dp); + intel_dp_stop_link_train(intel_dp); ironlake_edp_backlight_on(intel_dp); if (IS_VALLEYVIEW(dev)) { @@ -1935,10 +1936,9 @@ intel_dp_set_link_train(struct intel_dp *intel_dp, struct drm_i915_private *dev_priv = dev->dev_private; enum port port = intel_dig_port->port; int ret; - uint32_t temp; if (HAS_DDI(dev)) { - temp = I915_READ(DP_TP_CTL(port)); + uint32_t temp = I915_READ(DP_TP_CTL(port)); if (dp_train_pat & DP_LINK_SCRAMBLING_DISABLE) temp |= DP_TP_CTL_SCRAMBLE_DISABLE; @@ -1948,18 +1948,6 @@ intel_dp_set_link_train(struct intel_dp *intel_dp, temp &= ~DP_TP_CTL_LINK_TRAIN_MASK; switch (dp_train_pat & DP_TRAINING_PATTERN_MASK) { case DP_TRAINING_PATTERN_DISABLE: - - if (port != PORT_A) { - temp |= DP_TP_CTL_LINK_TRAIN_IDLE; - I915_WRITE(DP_TP_CTL(port), temp); - - if (wait_for((I915_READ(DP_TP_STATUS(port)) & - DP_TP_STATUS_IDLE_DONE), 1)) - DRM_ERROR("Timed out waiting for DP idle patterns\n"); - - temp &= ~DP_TP_CTL_LINK_TRAIN_MASK; - } - temp |= DP_TP_CTL_LINK_TRAIN_NORMAL; break; @@ -2035,6 +2023,37 @@ intel_dp_set_link_train(struct intel_dp *intel_dp, return true; } +static void intel_dp_idle_link_train(struct intel_dp *intel_dp) +{ + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); + struct drm_device *dev = intel_dig_port->base.base.dev; + struct drm_i915_private *dev_priv = dev->dev_private; + enum port port = intel_dig_port->port; + uint32_t l; + + if (!HAS_DDI(dev)) + return; + + l = I915_READ(DP_TP_CTL(port)); + l &= ~DP_TP_CTL_LINK_TRAIN_MASK; + l |= DP_TP_CTL_LINK_TRAIN_IDLE; + I915_WRITE(DP_TP_CTL(port), l); + + /* + * On PORT_A we can have only eDP in SST mode. There the only reason + * we need to set idle transmission mode is to work around a HW issue + * where we enable the pipe while not in idle link-training mode. + * In this case there is requirement to wait for a minimum number of + * idle patterns to be sent. + */ + if (port == PORT_A) + return; + + if (wait_for((I915_READ(DP_TP_STATUS(port)) & DP_TP_STATUS_IDLE_DONE), + 1)) + DRM_ERROR("Timed out waiting for DP idle patterns\n"); +} + /* Enable corresponding port and start training pattern 1 */ void intel_dp_start_link_train(struct intel_dp *intel_dp) @@ -2177,10 +2196,19 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp) ++tries; } + intel_dp_idle_link_train(intel_dp); + + intel_dp->DP = DP; + if (channel_eq) DRM_DEBUG_KMS("Channel EQ done. DP Training successfull\n"); - intel_dp_set_link_train(intel_dp, DP, DP_TRAINING_PATTERN_DISABLE); +} + +void intel_dp_stop_link_train(struct intel_dp *intel_dp) +{ + intel_dp_set_link_train(intel_dp, intel_dp->DP, + DP_TRAINING_PATTERN_DISABLE); } static void @@ -2388,6 +2416,7 @@ intel_dp_check_link_status(struct intel_dp *intel_dp) drm_get_encoder_name(&intel_encoder->base)); intel_dp_start_link_train(intel_dp); intel_dp_complete_link_train(intel_dp); + intel_dp_stop_link_train(intel_dp); } } diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index a8c6960..6d1e63a 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -542,6 +542,7 @@ extern void intel_dp_init_connector(struct intel_digital_port *intel_dig_port, extern void intel_dp_init_link_config(struct intel_dp *intel_dp); extern void intel_dp_start_link_train(struct intel_dp *intel_dp); extern void intel_dp_complete_link_train(struct intel_dp *intel_dp); +extern void intel_dp_stop_link_train(struct intel_dp *intel_dp); extern void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode); extern void intel_dp_encoder_destroy(struct drm_encoder *encoder); extern void intel_dp_check_link_status(struct intel_dp *intel_dp);
According to BSpec the link training sequence for eDP on HSW port-A should be as follows: 1. link training: clock recovery 2. link training: equalization 3. link training: set idle transmission mode 4. display pipe enable 5. link training: disable (set normal mode) Contrary to this at the moment we don't do step 3. and we do step 5. before step 4. Fix this by setting idle transmission mode for eDP at the end of intel_dp_complete_link_train and adding a new intel_dp_stop_link_training function to disable link training. With these changes we'll end up with the following functions corresponding to the above steps: intel_dp_start_link_train -> step 1. intel_dp_complete_link_train -> step 2., step 3. intel_dp_stop_link_train -> step 5. For port-A we'll call intel_dp_stop_link_train only after enabling the pipe, for everything else we'll call it right after intel_dp_complete_link_train to preserve the current behavior. Tested on HSW/HSW-ULT. In v2: - Due to a HW issue we must set idle transmission mode for port-A too before enabling the pipe. Thanks for Arthur Runyan for explaining this. - Update the patch subject to make it clear that it's an eDP fix, DP is not affected. Signed-off-by: Imre Deak <imre.deak@intel.com> --- drivers/gpu/drm/i915/intel_ddi.c | 5 ++++ drivers/gpu/drm/i915/intel_dp.c | 59 ++++++++++++++++++++++++++++---------- drivers/gpu/drm/i915/intel_drv.h | 1 + 3 files changed, 50 insertions(+), 15 deletions(-)