On Wed, Jan 17, 2018 at 09:21:49PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> LSPCON likes to throw short HPDs during the enable seqeunce prior to the
> link being trained. These obviously result in the channel CR/EQ check
> failing and thus we schedule a pointless hotplug work to retrain the
> link. Avoid that by ignoring the bad CR/EQ status until we've actually
> initially trained the link.
>
> I've not actually investigated to see what LSPCON is trying to signal
> with the short pulse. But as long as it signals anything I think we're
> supposed to check the link status anyway, so I don't really see other
> good ways to solve this. I've not seen these short pulses being
> generated by normal DP sinks.
>
I agree with avoiding the retraining of the link through these HPDs
when its not trained even for the first time.
The only concern I have here is that we probably shouldnt set the link_trained to true
unless it has been sucessfully trained. So move it to the intel_dp_start_link_train
before failure handling. This will also avoid the case where we are in the retraining
and we get these short HPDs.
Also the link_trained should be set to false in failure_handling in intel_dp_start_link_train()
before scheduling the retry modeset work function.
Thoughts?
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_ddi.c | 2 ++
> drivers/gpu/drm/i915/intel_dp.c | 10 +++++++---
> drivers/gpu/drm/i915/intel_dp_link_training.c | 2 ++
> drivers/gpu/drm/i915/intel_drv.h | 1 +
> 4 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 5f3d58f1ae6e..7a4c5a2d36ed 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2466,6 +2466,8 @@ static void intel_disable_ddi_dp(struct intel_encoder *encoder,
> {
> struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
>
> + intel_dp->link_trained = false;
> +
> if (old_crtc_state->has_audio)
> intel_audio_codec_disable(encoder,
> old_crtc_state, old_conn_state);
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 152016e09a11..0cf92aa60f3e 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1854,6 +1854,7 @@ void intel_dp_set_link_params(struct intel_dp *intel_dp,
> int link_rate, uint8_t lane_count,
> bool link_mst)
> {
> + intel_dp->link_trained = false;
> intel_dp->link_rate = link_rate;
> intel_dp->lane_count = lane_count;
> intel_dp->link_mst = link_mst;
> @@ -2702,6 +2703,8 @@ static void intel_disable_dp(struct intel_encoder *encoder,
> {
> struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
>
> + intel_dp->link_trained = false;
> +
> if (old_crtc_state->has_audio)
> intel_audio_codec_disable(encoder,
> old_crtc_state, old_conn_state);
> @@ -4280,10 +4283,11 @@ intel_dp_needs_link_retrain(struct intel_dp *intel_dp)
> {
> u8 link_status[DP_LINK_STATUS_SIZE];
>
> - if (!intel_dp_get_link_status(intel_dp, link_status)) {
> - DRM_ERROR("Failed to get link status\n");
> + if (!intel_dp->link_trained)
> + return false;
> +
> + if (!intel_dp_get_link_status(intel_dp, link_status))
> return false;
> - }
>
> /*
> * Validate the cached values of intel_dp->link_rate and
> diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c
> index 1314f5d87d7d..78f1fe934da3 100644
> --- a/drivers/gpu/drm/i915/intel_dp_link_training.c
> +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
> @@ -307,6 +307,8 @@ intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp)
>
> void intel_dp_stop_link_train(struct intel_dp *intel_dp)
> {
> + intel_dp->link_trained = true;
> +
Move to intel_dp_start_link_train() on successful link train?
Manasi
> intel_dp_set_link_train(intel_dp,
> DP_TRAINING_PATTERN_DISABLE);
> }
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 1d018869ad02..7a45ffb9e524 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1043,6 +1043,7 @@ struct intel_dp {
> uint8_t lane_count;
> uint8_t sink_count;
> bool link_mst;
> + bool link_trained;
> bool has_audio;
> bool detect_done;
> bool reset_link_params;
> --
> 2.13.6
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
