On Tue, Jun 16, 2020 at 06:30:55PM +0300, Imre Deak wrote:
> 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.
But then we should have a long hpd and reset_link_params should be set
anyway no?
>
> > > /* 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
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
