Re: [PATCH] Revert "drm/i915/edp: Do not do link training fallback or prune modes on EDP"

From: Manasi Navare <manasi.d.navare@intel.com>
To: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>,
	intel-gfx@lists.freedesktop.org, Dave Airlie <airlied@redhat.com>
Subject: Re: [PATCH] Revert "drm/i915/edp: Do not do link training fallback or prune modes on EDP"
Date: Wed, 16 May 2018 14:09:09 -0700	[thread overview]
Message-ID: <20180516210909.GE2730@intel.com> (raw)
In-Reply-To: <20180516172110.19191-1-lucas.demarchi@intel.com>

On Wed, May 16, 2018 at 10:21:10AM -0700, Lucas De Marchi wrote:
> This reverts commit c0cfb10d9e1de490e36d3b9d4228c0ea0ca30677.
> 
> This fails on a Dell XPS13 9350 machine giving me just a black screen.
>  [drm:intel_dp_start_link_train [i915]] [CONNECTOR:59:eDP-1] Link Training Passed at Link Rate = 540000, Lane count = 4
>  [drm:intel_dp_start_link_train [i915]] *ERROR* [CONNECTOR:59:eDP-1] Link Training failed at link rate = 540000, lane count = 4
>  [drm:intel_dp_start_link_train [i915]] [CONNECTOR:59:eDP-1] Link Training Passed at Link Rate = 540000, Lane count = 4
>  [drm:intel_dp_start_link_train [i915]] [CONNECTOR:59:eDP-1] Link Training Passed at Link Rate = 540000, Lane count = 4
>  [drm:intel_dp_start_link_train [i915]] [CONNECTOR:59:eDP-1] Link Training Passed at Link Rate = 540000, Lane count = 4
>  [drm:intel_dp_start_link_train [i915]] *ERROR* [CONNECTOR:59:eDP-1] Link Training failed at link rate = 540000, lane count = 4
>  [drm:intel_dp_start_link_train [i915]] *ERROR* [CONNECTOR:59:eDP-1] Link Training failed at link rate = 540000, lane count = 4
>  [drm:intel_dp_start_link_train [i915]] [CONNECTOR:59:eDP-1] Link Training Passed at Link Rate = 540000, Lane count = 4
>  [drm:intel_dp_start_link_train [i915]] *ERROR* [CONNECTOR:59:eDP-1] Link Training failed at link rate = 540000, lane count = 4
>  [drm:intel_dp_start_link_train [i915]] [CONNECTOR:59:eDP-1] Link Training Passed at Link Rate = 540000, Lane count = 4
>  [drm:intel_dp_start_link_train [i915]] [CONNECTOR:59:eDP-1] Link Training Passed at Link Rate = 540000, Lane count = 4
>  [drm:intel_dp_start_link_train [i915]] *ERROR* [CONNECTOR:59:eDP-1] Link Training failed at link rate = 540000, lane count = 4
> 
> On a working kernel, previous to this commit I have:
>  [drm:intel_dp_start_link_train [i915]] [CONNECTOR:59:eDP-1] Link Training failed at link rate = 540000, lane count = 4
>  [drm:intel_dp_start_link_train [i915]] [CONNECTOR:59:eDP-1] Link Training Passed at Link Rate = 270000, Lane count = 4
> 
> Cc: Clinton Taylor <clinton.a.taylor@intel.com>
> Cc: Jim Bride <jim.bride@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
> 
> I'm sending this for reference and discussion only, but it doesn't seem
> to be the right fix. Not only because it would mean IGT failing on CI,
> but also because we seem to be having several link training running
> concurrently and the last one failing leaves the panel at that state.

I just looked at the full log of the failing case and in that case it first tries training
at the highest link rate, lane count for eDP and passes before enabling pipe A.
But then the panel soon sends a short pulse indicating loss of signal which
causes the driver to call intel_dp_check_link_status() where drm_dp_channel_eq_ok() check
fails and thats why you see multiple link training attempts there which fail and eventually result in a
black screen.

Now in case of passing case, whats interesting is that it starts with same highest link rate 5.4Gbps
but fails the very first time and now the fallback code kicks in and lowers the link rate to 2.7Gbps.
Now in this particular case, the edp panel's native resolution of 3200 x 1800 fits even for the lowered
link rate of 2.7Gbps and it retrains at that and passes so we get a recovered modeset.

So I am thinking because of such cases, we should probably not asusme that the native mode will not
fit the lowered fallback link rate. So I guess we should still try fallback for eDP as long
as its not pruning the native mode on the panel.

Alternatively I think if we start from lowest link rate and lane count like we do
for DP then it will pick the lowest possible link rate required for the native mode and we might
not run into this situation.

Jani, Ville any thoughts on which approach should be chosen?

Regards
Manasi

> 
>  drivers/gpu/drm/i915/intel_dp_link_training.c | 26 +++++++------------
>  1 file changed, 9 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c
> index f59b59bb0a21..78f1fe934da3 100644
> --- a/drivers/gpu/drm/i915/intel_dp_link_training.c
> +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
> @@ -330,22 +330,14 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
>  	return;
>  
>   failure_handling:
> -	/* Dont fallback and prune modes if its eDP */
> -	if (!intel_dp_is_edp(intel_dp)) {
> -		DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Link Training failed at link rate = %d, lane count = %d",
> -			      intel_connector->base.base.id,
> -			      intel_connector->base.name,
> -			      intel_dp->link_rate, intel_dp->lane_count);
> -		if (!intel_dp_get_link_train_fallback_values(intel_dp,
> -							     intel_dp->link_rate,
> -							     intel_dp->lane_count))
> -			/* Schedule a Hotplug Uevent to userspace to start modeset */
> -			schedule_work(&intel_connector->modeset_retry_work);
> -	} else {
> -		DRM_ERROR("[CONNECTOR:%d:%s] Link Training failed at link rate = %d, lane count = %d",
> -			  intel_connector->base.base.id,
> -			  intel_connector->base.name,
> -			  intel_dp->link_rate, intel_dp->lane_count);
> -	}
> +	DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Link Training failed at link rate = %d, lane count = %d",
> +		      intel_connector->base.base.id,
> +		      intel_connector->base.name,
> +		      intel_dp->link_rate, intel_dp->lane_count);
> +	if (!intel_dp_get_link_train_fallback_values(intel_dp,
> +						     intel_dp->link_rate,
> +						     intel_dp->lane_count))
> +		/* Schedule a Hotplug Uevent to userspace to start modeset */
> +		schedule_work(&intel_connector->modeset_retry_work);
>  	return;
>  }
> -- 
> 2.17.0
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx