On Wed, Jul 24, 2024 at 07:55:03AM +0300, Murthy, Arun R wrote:
> > -----Original Message-----
> > From: Intel-gfx <intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of Imre
> > Deak
> > Sent: Monday, July 22, 2024 10:25 PM
> > To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > Subject: [PATCH 07/14] drm/i915/dp: Add a separate function to reduce the link
> > parameters
> >
> > A follow-up patch will add an alternative way to reduce the link parameters in
> > BW order on MST links, prepare for that here.
> >
> > Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx>
> > ---
> > .../drm/i915/display/intel_dp_link_training.c | 39 +++++++++++++++----
> > 1 file changed, 31 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > index 58dea87a9fa28..57536ae200b77 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > @@ -1193,6 +1193,36 @@ static int reduce_lane_count(struct intel_dp
> > *intel_dp, int current_lane_count)
> > return current_lane_count >> 1;
> > }
> >
> > +static bool reduce_link_params_in_rate_lane_order(struct intel_dp *intel_dp,
> > + const struct intel_crtc_state
> > *crtc_state,
> > + int *new_link_rate, int
> > *new_lane_count) {
> > + int link_rate;
> > + int lane_count;
> > +
> > + lane_count = crtc_state->lane_count;
> > + link_rate = reduce_link_rate(intel_dp, crtc_state->port_clock);
> > + if (link_rate < 0) {
> > + lane_count = reduce_lane_count(intel_dp, crtc_state-
> > >lane_count);
> > + link_rate = intel_dp_max_common_rate(intel_dp);
> > + }
> > +
>
> On link training failure reducing link rate or lane count is not
> linear. Sometime we fall from uhbr to hbr and then again with uhbr
> with lane reduction. So would it be better to have a table/list for
> the fallback link rate/lane count.
This patch is meant to to keep the current way of reducing the rate and
lane count, just preparing for a follow-up change that adds the
alternetive BW order fallback logic for MST. I think later SST would
need to be switched to the logic as well, for now I didn't want to
change this.
>
> Thanks and Regards,
> Arun R Murthy
> --------------------
> > + if (lane_count < 0)
> > + return false;
> > +
> > + *new_link_rate = link_rate;
> > + *new_lane_count = lane_count;
> > +
> > + return true;
> > +}
> > +
> > +static bool reduce_link_params(struct intel_dp *intel_dp, const struct
> > intel_crtc_state *crtc_state,
> > + int *new_link_rate, int *new_lane_count) {
> > + return reduce_link_params_in_rate_lane_order(intel_dp, crtc_state,
> > + new_link_rate,
> > new_lane_count); }
> > +
> > static int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
> > const struct intel_crtc_state
> > *crtc_state) { @@ -1206,14 +1236,7 @@ static int
> > intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
> > return 0;
> > }
> >
> > - new_lane_count = crtc_state->lane_count;
> > - new_link_rate = reduce_link_rate(intel_dp, crtc_state->port_clock);
> > - if (new_link_rate < 0) {
> > - new_lane_count = reduce_lane_count(intel_dp, crtc_state-
> > >lane_count);
> > - new_link_rate = intel_dp_max_common_rate(intel_dp);
> > - }
> > -
> > - if (new_lane_count < 0)
> > + if (!reduce_link_params(intel_dp, crtc_state, &new_link_rate,
> > +&new_lane_count))
> > return -1;
> >
> > if (intel_dp_is_edp(intel_dp) &&
> > --
> > 2.44.2
>
