On Mon, Oct 04, 2021 at 08:05:31PM +0300, Ville Syrjala wrote: > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > Currently we consider the max vswing reached when we transmit a > the max voltage level, but we don't consider pre-emphasis at all. > This kinda matches older DP specs that only had some vague text > about transmitting the maximum voltage swing. Latest versions > now say something vague about consider the sum of the vswing > and pre-emphasis fields in the ADJUST_REQUEST_LANE registers. > Very vague, and super confusing especially the fact that it > talks about transmitted voltgage swing in the same sentence > as it say to look at the requested values. > > Also glanced at the link CTS spec, and that one seems to have > tests that assume contradicting behaviour. Some say to consider > just the vswing level we transmit, others say to check for > sum of transmitted vswing+preemph being 3. > > So let's try to take some kind of sane middle ground here. > I think what could make sense is only consider max vswing > reached if MAX_SWING_REACHED==1 _and_ vswing+preemph==3. > That will allow things to go all the way up to vswing 3 + > pre-emph 0 or vswing 2 + pre-emph 1, depending on what > the maximum supported vswing is. Only considering the sum > of vswing+pre-emph doesn't make much sense to me since > we could terminate too early if the sink requests eg. > vswing 0 + pre-emph 3. And if we'd stick to the current > code we could terminate too early of the sink asks for > vswing 2 + pre-emph 0 when vswing level 3 is not supported. > > Side note: I don't really understand why any of this stuff is > "specified" at all. There is already a limit of 5 attempts at > the same vswing+pre-emph level, and a total limit of 10 > attempts. So might as well stick to the same max 5 attempts > across the board IMO. > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> Reviewed-by: Imre Deak <imre.deak@xxxxxxxxx> > --- > .../drm/i915/display/intel_dp_link_training.c | 22 ++++++++++++++++--- > 1 file changed, 19 insertions(+), 3 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 c052ce14894d..a45569b8c959 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c > +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c > @@ -492,11 +492,27 @@ static bool intel_dp_link_max_vswing_reached(struct intel_dp *intel_dp, > { > int lane; > > - for (lane = 0; lane < crtc_state->lane_count; lane++) > - if ((intel_dp->train_set[lane] & > - DP_TRAIN_MAX_SWING_REACHED) == 0) > + /* > + * FIXME: The DP spec is very confusing here, also the Link CTS > + * spec seems to have self contradicting tests around this area. > + * > + * In lieu of better ideas let's just stop when we've reached the > + * max supported vswing with its max pre-emphasis, which is either > + * 2+1 or 3+0 depending on whether vswing level 3 is supported or not. > + */ > + for (lane = 0; lane < crtc_state->lane_count; lane++) { > + u8 v = (intel_dp->train_set[lane] & DP_TRAIN_VOLTAGE_SWING_MASK) >> > + DP_TRAIN_VOLTAGE_SWING_SHIFT; > + u8 p = (intel_dp->train_set[lane] & DP_TRAIN_PRE_EMPHASIS_MASK) >> > + DP_TRAIN_PRE_EMPHASIS_SHIFT; > + > + if ((intel_dp->train_set[lane] & DP_TRAIN_MAX_SWING_REACHED) == 0) > return false; > > + if (v + p != 3) > + return false; > + } > + > return true; > } > > -- > 2.32.0 >