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

[Jani Nikula <jani.nikula@linux.intel.com>]

On Mon, 22 Jan 2018, Imre Deak <imre.deak@intel.com> wrote:
> On Fri, Jan 19, 2018 at 05:45:16PM +0200, Imre Deak wrote:
>> On Thu, Oct 12, 2017 at 12:13:38PM -0700, Manasi Navare wrote:
>> > In case of eDP because the panel has a fixed mode, the link rate
>> > and lane count at which it is trained corresponds to the link BW
>> > required to support the native resolution of the panel. In case of
>> > panles with lower resolutions where fewer lanes are hooked up internally,
>> > that number is reflected in the MAX_LANE_COUNT DPCD register of the panel.
>> > So it is pointless to fallback to lower link rate/lane count in case
>> > of link training failure on eDP connector since the lower link BW
>> > will not support the native resolution of the panel and we cannot
>> > prune the preferred mode on the eDP connector.
>> > 
>> > In case of Link training failure on the eDP panel, something is wrong
>> > in the HW internally and hence driver errors out with a loud
>> > and clear DRM_ERROR message.
>> > 
>> > v2:
>> > * Fix the DEBUG_ERROR and add {} in else (Ville Syrjala)
>> > 
>> > 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>
>> > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
>> > Reviewed-by: Ville Syrjala <ville.syrjala@linux.intel.com>
>> 
>> This fell through the cracks, looks like it partially fixes
>> https://bugs.freedesktop.org/show_bug.cgi?id=103369
>> 
>> Why link training fails there is not clear.
>
> Ok, the link training fail turned out to be a race between a modeset
> link training and a link retraining called from
> runtime_resume->intel_hpd_init->dp_detect. As Ville pointed out that
> one was fixed meanwhile by
>
> commit 42e5e65765265485ecf2a480c244d76c2c624449
> Author:     Daniel Vetter <daniel.vetter@ffwll.ch>
> AuthorDate: Mon Nov 13 17:01:40 2017 +0100
> Commit:     Daniel Vetter <daniel.vetter@ffwll.ch>
> CommitDate: Thu Nov 23 14:59:07 2017 +0100
>
>     drm/i915: sync dp link status checks against atomic commmits
>
> I merged now this fix to address the other issue, adding the above bug
> as reference. Thanks for the patch and the review.

Thanks for the follow-up... but should we have added a Fixes: or cc:
stable tag here?

BR,
Jani.


>
> --Imre
>
>> 
>> > ---
>> >  drivers/gpu/drm/i915/intel_dp_link_training.c | 26 +++++++++++++++++---------
>> >  1 file changed, 17 insertions(+), 9 deletions(-)
>> > 
>> > diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c
>> > index 05907fa..cf8fef8 100644
>> > --- a/drivers/gpu/drm/i915/intel_dp_link_training.c
>> > +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
>> > @@ -328,14 +328,22 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
>> >  	return;
>> >  
>> >   failure_handling:
>> > -	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);
>> > +	/* 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);
>> > +	}
>> >  	return;
>> >  }
>> > -- 
>> > 2.1.4
>> > 
>> > _______________________________________________
>> > Intel-gfx mailing list
>> > Intel-gfx@lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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