[Intel-gfx] [PATCH] drm/i915: Don't recheck link status for eDP display.

[Manasi Navare]

On Tue, Oct 17, 2017 at 02:01:56PM -0700, Puthikorn Voravootivat wrote:
> intel_dp_long_pulse() is always checking link status because
> there has been known issues of link loss triggerring long pulse.
> 
> However this is not needed for eDP display since we won't have
> link loss for internal display. Also there are reports that
> screens are flickering during link status check. (repro by
> running modetest command repeatedly)
> 
> Signed-off-by: Puthikorn Voravootivat <puthik at chromium.org>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 4b65cf137f79..75a77ef257e2 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4763,7 +4763,8 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
>  		 */
>  		status = connector_status_disconnected;
>  		goto out;
> -	} else {
> +	} else if (status != connector_status_connected ||
> +		   intel_encoder->type != INTEL_OUTPUT_EDP) {
>  		/*
>  		 * If display is now connected check links status,
>  		 * there has been known issues of link loss triggerring
> @@ -4775,6 +4776,10 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
>  		 * going back up soon after. And once that happens we must
>  		 * retrain the link to get a picture. That's in case no
>  		 * userspace component reacted to intermittent HPD dip.
> +		 *
> +		 * Skip checking links status for connected eDP display.
> +		 * There are known issues of display blinking during checking
> +		 * link status and we don't have link loss for internal display.
>  		 */

Inside intel_dp_check_link_status(), it actually checks if link status is good by
checking both clock recovery and Channel EQ bits in DPCD, only then it will retrain.
So in case of eDP panel, if there is no link loss then it should always return link
status as good and not retrain.
So IMHO I dont think we need to explicitly avoid this for eDP.

When you see the display blinking, do you know for sure that drm_dp_channel_eq_ok() returns false?
Also look at this patch: https://patchwork.freedesktop.org/series/30670/
This makes sure none of the eDP training parameters change unless link is bad. So with this patch, if the
link is good then it should never retrain even in intel_dp_check_link_status() for eDP.

Manasi
>  		intel_dp_check_link_status(intel_dp);
>  	}
> -- 
> 2.15.0.rc0.271.g36b669edcc-goog
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx