| Message ID | 20230620074400.1054530-1-arun.r.murthy@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/display/dp: Add fallback on LT failure for DP2.0 | expand |
> 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. Maybe adding the section where one can refer this in DP 2.0 spec can help. > > Signed-off-by: Arun R Murthy <arun.r.murthy@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; NIT: Can we add a small comment here on what is being done. > + if (intel_dp->dpcd[DP_MAIN_LINK_CHANNEL_CODING] & > DP_CAP_ANSI_128B132B) { > + 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; > + } Also I see a little issue here so previously what used to happen was we reduced the Link rate first and once we were at the lowest only then proceed to reduce the lane count so has this been reversed in DP2.0 spec. Regards, Suraj Kandpal > } 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; > } > > -- > 2.25.1
> -----Original Message----- > From: Kandpal, Suraj <suraj.kandpal@intel.com> > Sent: Tuesday, June 20, 2023 4:35 PM > To: Murthy, Arun R <arun.r.murthy@intel.com>; intel- > gfx@lists.freedesktop.org > Subject: RE: [Intel-gfx] [PATCH] drm/i915/display/dp: Add fallback on LT > failure for DP2.0 > > > 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. > > Maybe adding the section where one can refer this in DP 2.0 spec can help. > Ok Sure! > > > > Signed-off-by: Arun R Murthy <arun.r.murthy@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; > > NIT: Can we add a small comment here on what is being done. > Comments are added below as to what each if condition is doing or is it something different that you are referring to? > > + if (intel_dp->dpcd[DP_MAIN_LINK_CHANNEL_CODING] & > > DP_CAP_ANSI_128B132B) { > > + 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; > > + } > > Also I see a little issue here so previously what used to happen was we > reduced the Link rate first and once we were at the lowest only then proceed > to reduce the lane count so has this been reversed in DP2.0 spec. > Yes here we reduce the lane count until 1 and then reduce the link rate. Ref: Table 3-31 of DP2.1 Spec Thanks and Regards, Arun R Murthy -------------------- > Regards, > Suraj Kandpal > > > } 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; > > } > > > > -- > > 2.25.1
On Tue, 20 Jun 2023, Arun R Murthy <arun.r.murthy@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@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. 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. 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; > }
> -----Original Message----- > From: Jani Nikula <jani.nikula@linux.intel.com> > Sent: Tuesday, June 20, 2023 7:26 PM > To: Murthy, Arun R <arun.r.murthy@intel.com>; intel- > gfx@lists.freedesktop.org > Cc: ville.syrjala@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@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@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
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) { + 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; }
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@intel.com> --- drivers/gpu/drm/i915/display/intel_dp.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-)