On Tue, Jun 16, 2020 at 06:22:51PM +0300, Ville Syrjälä wrote:
> On Tue, Jun 16, 2020 at 05:18:51PM +0300, Imre Deak wrote:
> > We calculate the MST available bandwidth using the link's maximum rate
> > and lane count. This bandwidth won't be recalculated in response to a
>
> Thw wording here is a bit ambiguousr as to who "we" is, and what exactly
> "link's maximum rate and lane count". I would try to clarify a bit that
> it's drm_dp_mst_topology.c who is mostly in error here by directly
> interpreting the max data rate/lanes from the DPCD.
>
> Althoguh the i915 code is not wihtout faults, as it lacks any logic
> to modeset all the MST streams for this link when the link params
> change (except on tgl+ where the master/slave stuff should force
> all streams to do a modeset anyway).
>
> > link training error and fallback setting, so modesets following a link
> > training error will calculate incorrect timing parameters (like the
> > transcoder's data M/N params or the number of MST TUs).
> >
> > Prevent the above problem by disabling the link training fallback on MST
> > links for now, until the MST compute config can deal with changing link
> > parameters.
> >
> > The misconfigured timing lead at least to a
> > 'Timed out waiting for DP idle patterns'
> > error.
> >
> > Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > Cc: Manasi Navare <manasi.d.navare@xxxxxxxxx>
> > Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/display/intel_dp.c | 25 ++++++++++++++++++-------
> > 1 file changed, 18 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 42589cae766d..c585b002783a 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -468,6 +468,13 @@ int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
> > struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> > int index;
> >
> > + /*
> > + * TODO: Enable fallback on MST links once MST link compute can handle
> > + * the fallback params.
> > + */
> > + if (intel_dp->is_mst)
> > + return -1;
>
> Should we duplicate the drm_error() from the other path here maybe?
Yes, will add it.
>
> > +
> > index = intel_dp_rate_index(intel_dp->common_rates,
> > intel_dp->num_common_rates,
> > link_rate);
> > @@ -6163,7 +6170,17 @@ intel_dp_detect(struct drm_connector *connector,
> > goto out;
> > }
> >
> > - if (intel_dp->reset_link_params) {
> > + /* Read DP Sink DSC Cap DPCD regs for DP v1.4 */
> > + if (INTEL_GEN(dev_priv) >= 11)
> > + intel_dp_get_dsc_sink_cap(intel_dp);
> > +
> > + intel_dp_configure_mst(intel_dp);
> > +
> > + /*
> > + * TODO: Reset link params when switching to MST mode, until MST
> > + * supports link training fallback params.
> > + */
> > + if (intel_dp->reset_link_params || intel_dp->is_mst) {
>
> /me confused. Why do we need to touch this code?
It's possible to switch to MST mode after the link rate/lane count got
reduced by a link training error in SST mode.
> > /* Initial max link lane count */
> > intel_dp->max_link_lane_count = intel_dp_max_common_lane_count(intel_dp);
> >
> > @@ -6175,12 +6192,6 @@ intel_dp_detect(struct drm_connector *connector,
> >
> > intel_dp_print_rates(intel_dp);
> >
> > - /* Read DP Sink DSC Cap DPCD regs for DP v1.4 */
> > - if (INTEL_GEN(dev_priv) >= 11)
> > - intel_dp_get_dsc_sink_cap(intel_dp);
> > -
> > - intel_dp_configure_mst(intel_dp);
> > -
> > if (intel_dp->is_mst) {
> > /*
> > * If we are in MST mode then this connector
> > --
> > 2.23.1
>
> --
> Ville Syrjälä
> Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
