[Intel-gfx] [PATCH v1] drm/i915: Minor link training logic fixes for dp_mst

[Lisovskiy, Stanislav]

On Wed, May 27, 2020 at 01:54:27PM -0700, Manasi Navare wrote:
> On Wed, May 27, 2020 at 11:00:22PM +0300, Stanislav Lisovskiy wrote:
> > First of all *_needs_link_retraining function should return
> > false is link_train is set to true but not false.
> > 
> > Also if we detect channel eq problem when checking mst status
> > we simply bail out, without setting link_train to false again,
> > which might end up in a situation that we don't do link retraining
> > when needed.
> > 
> > There were some issues, when we had several problems with dp mst
> > and at the same time the log was floode by messages about
> > "channel eq not ok, need retraining" however the actual training
> > seems to be never done.
> > 
> > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy at intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_dp.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 1768731678a1..9288dc1f8914 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -5627,6 +5627,7 @@ intel_dp_check_mst_status(struct intel_dp *intel_dp)
> >  			drm_dbg_kms(&i915->drm,
> >  				    "channel EQ not ok, retraining\n");
> >  			need_retrain = true;
> > +			intel_dp->link_trained = false;
> >  		}
> >  
> >  		drm_dbg_kms(&i915->drm, "got esi %3ph\n", esi);
> > @@ -5654,7 +5655,7 @@ intel_dp_needs_link_retrain(struct intel_dp *intel_dp)
> >  {
> >  	u8 link_status[DP_LINK_STATUS_SIZE];
> >  
> > -	if (!intel_dp->link_trained)
> > +	if (intel_dp->link_trained)
> 
> This is not correct. Since link_trained is set when link training is completed as part of a
> complete modeset. If link training is not done, like at hotplug, then in that case we should
> not retrain since the pipe has not been configured for this new hotplug and link training
> has not been done.
>
 
Ok, I was confusing the link training and retraining - assuming those are same procedure.
So we can't retrain until the training has been completed.
Thanks for clarification - will dig further then.

> Retraining is expected to happen only in cases where there is a short pulse or a spurious long pulse
> when link training through modeset is already complete and hence the old logic of returnin a false
> when !intel_dp->link_trained is correct.

Yes, that is the scenario which I was suspecting to be happening in our recent MST failures.
We sometimes get a lot of short pulses requiring rettraining however it doesn't happen in practice.


Stan

> 
> Regards
> Manasi
> 
> >  		return false;
> >  
> >  	/*
> > -- 
> > 2.24.1.485.gad05a3d8e5
> >