[PATCH 14/14] drm/i915/dp_mst: Enable LT fallback between UHBR/non-UHBR link rates

[Kandpal, Suraj]

> -----Original Message-----
> From: Deak, Imre <imre.deak at intel.com>
> Sent: Wednesday, July 24, 2024 5:03 PM
> To: Kandpal, Suraj <suraj.kandpal at intel.com>
> Cc: intel-gfx at lists.freedesktop.org
> Subject: Re: [PATCH 14/14] drm/i915/dp_mst: Enable LT fallback between
> UHBR/non-UHBR link rates
> 
> On Wed, Jul 24, 2024 at 11:52:14AM +0300, Kandpal, Suraj wrote:
> >
> >
> > > -----Original Message-----
> > > From: Intel-gfx <intel-gfx-bounces at lists.freedesktop.org> On Behalf
> > > Of Imre Deak
> > > Sent: Monday, July 22, 2024 10:25 PM
> > > To: intel-gfx at lists.freedesktop.org
> > > Subject: [PATCH 14/14] drm/i915/dp_mst: Enable LT fallback between
> > > UHBR/non-UHBR link rates
> > >
> > > Enable switching between UHBR and non-UHBR link rates on MST links
> > > when reducing the link parameters after an LT failure.
> > >
> > > Signed-off-by: Imre Deak <imre.deak at intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_dp_link_training.c | 5 -----
> > >  1 file changed, 5 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 0c8e0d6437b5b..270080b2735f2 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > > @@ -1188,11 +1188,6 @@ static bool
> > > reduce_link_params_in_bw_order(struct intel_dp *intel_dp,
> > >                    intel_dp->link.force_lane_count != lane_count))
> > >                       continue;
> > >
> > > -             /* TODO: Make switching from UHBR to non-UHBR rates
> > > work. */
> > > -             if (drm_dp_is_uhbr_rate(crtc_state->port_clock) !=
> > > -                 drm_dp_is_uhbr_rate(link_rate))
> > > -                     continue;
> > > -
> >
> > Do we need to remove this here, I mean why introduce this piece of
> > todo code to begin with specially in this function as
> > reduce_link_params_in_bw_order is being defined in this series in one
> > of the previous patches.
> 
> That's basically the rule of containing only one change in one patch.
> That rule is for different reasons, one is to help with bisecting an issue. In
> the earlier patch you refer to the change is to switch the fallback logic to
> happen in BW order, but keeping the behavior not to switch between UHBR
> <-> non-UHBR rates as it was before. Here at the end of the patchset is also
> the point to enable this rate switching, after addressing all the
> dependencies for that.
> 

In that case 
LGTM,
Reviewed-by: Suraj Kandpal <suraj.kandpal at intel.com>

> > Just omit this condition while defining it
> >
> > Regards,
> > Suraj Kandpal
> >
> > >       }
> > >
> > > --
> > > 2.44.2
> >