On Tue, Jun 16, 2020 at 06:39:30PM +0300, Ville Syrjälä wrote:
> 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?
I meant switching the mode for the same sink as it would change its
DP_MST_CAP. I'm not sure if a long HPD is required in that case. Also if
we had a long HPD after a mode change couldn't a modeset run before the
next intel_dp_detect() call could reset the link params?
> >
> > > > /* 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
