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
>
