[Intel-gfx] [PATCH] drm/i915/display/dp: Add fallback on LT failure for DP2.0

[Murthy, Arun R]

> -----Original Message-----
> From: Jani Nikula <jani.nikula at linux.intel.com>
> Sent: Tuesday, June 20, 2023 7:26 PM
> To: Murthy, Arun R <arun.r.murthy at intel.com>; intel-
> gfx at lists.freedesktop.org
> Cc: ville.syrjala at linux.intel.com
> Subject: Re: [Intel-gfx] [PATCH] drm/i915/display/dp: Add fallback on LT
> failure for DP2.0
> 
> On Tue, 20 Jun 2023, Arun R Murthy <arun.r.murthy at intel.com> wrote:
> > For DP2.0 as per the Spec on LT failure we need to reduce the lane
> > count if the lane count is not equal to 1. If lane count is 1 then
> > need to retry with reducing the link rate.
> >
> > Signed-off-by: Arun R Murthy <arun.r.murthy at intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_dp.c | 20 ++++++++++++++++++--
> >  1 file changed, 18 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> > b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 54ddc953e5bc..2b12ca45596d 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -657,8 +657,20 @@ int intel_dp_get_link_train_fallback_values(struct
> intel_dp *intel_dp,
> >  				    "Retrying Link training for eDP with same
> parameters\n");
> >  			return 0;
> >  		}
> > -		intel_dp->max_link_rate = intel_dp_common_rate(intel_dp,
> index - 1);
> > -		intel_dp->max_link_lane_count = lane_count;
> > +		if (intel_dp->dpcd[DP_MAIN_LINK_CHANNEL_CODING] &
> > +DP_CAP_ANSI_128B132B) {
> 
> This doesn't mean we're actually using UHBR. And we don't get here for
> UHBR because it's always mst. And this doesn't handle mst (it should). There
> are a lot of gaps here.

Agree, this patch is not trying to fill the entire gaps and instead trying to have a fallback within the UHBR range.

> 
> The whole thing has been discussed before, and my previous idea was that
> we'd generate a list of {link rate, link lane count} pairs up front, in the correct
> order, and just have one an index to that list for the fallback. Because the
> logic here is complicated to follow.
> 
> Eventually we'd need to be able to switch between UHBR and non-UHBR in
> the fallback too, which we aren't capable of doing at the moment.
> 
Yes agree on this.
Will drop this patch over here for now!

Thanks and Regards,
Arun R Murthy
--------------------
> 
> BR,
> Jani.
> 
> 
> > +			if (lane_count > 1) {
> > +				/* Reduce the lane count */
> > +				intel_dp->max_link_lane_count = lane_count
> >> 1;
> > +				intel_dp->max_link_rate =
> intel_dp_common_rate(intel_dp, index);
> > +			} else {
> > +				/* Reduce the link rate */
> > +				intel_dp->max_link_rate =
> intel_dp_common_rate(intel_dp, index - 1);
> > +				intel_dp->max_link_lane_count =
> intel_dp_max_common_lane_count(intel_dp);
> > +			}
> > +		} else {
> > +			intel_dp->max_link_rate =
> intel_dp_common_rate(intel_dp, index - 1);
> > +			intel_dp->max_link_lane_count = lane_count;
> > +		}
> >  	} else if (lane_count > 1) {
> >  		if (intel_dp_is_edp(intel_dp) &&
> >  		    !intel_dp_can_link_train_fallback_for_edp(intel_dp,
> > @@ -675,6 +687,10 @@ int intel_dp_get_link_train_fallback_values(struct
> intel_dp *intel_dp,
> >  		return -1;
> >  	}
> >
> > +	drm_dbg_kms(&i915->drm,
> > +		    "Retrying Link training with link rate %d and lane count
> %d\n",
> > +		    intel_dp->max_link_rate, intel_dp->max_link_lane_count);
> > +
> >  	return 0;
> >  }
> 
> --
> Jani Nikula, Intel Open Source Graphics Center