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

From: Jani Nikula <jani.nikula@linux.intel.com>
To: Manasi Navare <manasi.d.navare@intel.com>,
	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: Thu, 17 May 2018 12:54:45 +0300	[thread overview]
Message-ID: <871seazjx6.fsf@intel.com> (raw)
In-Reply-To: <20180516210909.GE2730@intel.com>

On Wed, 16 May 2018, Manasi Navare <manasi.d.navare@intel.com> wrote:
> 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.

We also have a bug [1] where the link training succeeds after the hpd,
but the display starts to flicker.

[1] https://bugs.freedesktop.org/show_bug.cgi?id=105267

> 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?

See [2].

BR,
Jani.

[2] http://marc.info/?i=87sh6qzqpp.fsf@intel.com

>
> 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
>> 

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx