| Message ID | 20240722165503.2084999-15-imre.deak@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/dp_mst: Enable LT fallback for UHBR<->non-UHBR rates | expand |
> -----Original Message----- > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Imre > Deak > Sent: Monday, July 22, 2024 10:25 PM > To: intel-gfx@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@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. Just omit this condition while defining it Regards, Suraj Kandpal > } > > -- > 2.44.2
On Wed, Jul 24, 2024 at 11:52:14AM +0300, Kandpal, Suraj wrote: > > > > -----Original Message----- > > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Imre > > Deak > > Sent: Monday, July 22, 2024 10:25 PM > > To: intel-gfx@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@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. > Just omit this condition while defining it > > Regards, > Suraj Kandpal > > > } > > > > -- > > 2.44.2 >
> -----Original Message----- > From: Deak, Imre <imre.deak@intel.com> > Sent: Wednesday, July 24, 2024 5:03 PM > To: Kandpal, Suraj <suraj.kandpal@intel.com> > Cc: intel-gfx@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@lists.freedesktop.org> On Behalf > > > Of Imre Deak > > > Sent: Monday, July 22, 2024 10:25 PM > > > To: intel-gfx@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@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@intel.com> > > Just omit this condition while defining it > > > > Regards, > > Suraj Kandpal > > > > > } > > > > > > -- > > > 2.44.2 > >
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; - break; }
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@intel.com> --- drivers/gpu/drm/i915/display/intel_dp_link_training.c | 5 ----- 1 file changed, 5 deletions(-)