On Tue, Sep 22, 2020 at 07:54:20PM +0300, Ville Syrjälä wrote:
> On Tue, Sep 22, 2020 at 03:51:03PM +0300, Imre Deak wrote:
> > To prepare for a follow-up LTTPR change factor out a helper to disable
> > the training pattern in DPCD. We'll need to do this for each LTTPR
> > (without programming the port to output the idle pattern) when training
> > in LTTPR non-transparent mode.
> >
> > Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx>
> > ---
> > .../drm/i915/display/intel_dp_link_training.c | 28 +++++++++++--------
> > 1 file changed, 16 insertions(+), 12 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 0c3809891bd2..6994a32244dc 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > @@ -102,30 +102,34 @@ void intel_dp_get_adjust_train(struct intel_dp *intel_dp,
> > intel_dp->train_set[lane] = v | p;
> > }
> >
> > +static bool intel_dp_disable_dpcd_training_pattern(struct intel_dp *intel_dp)
> > +{
> > + u8 val = DP_TRAINING_PATTERN_DISABLE;
> > +
> > + return drm_dp_dpcd_write(&intel_dp->aux, DP_TRAINING_PATTERN_SET, &val, 1) == 1;
> > +}
>
>
> > +
> > static bool
> > intel_dp_set_link_train(struct intel_dp *intel_dp,
> > u8 dp_train_pat)
> > {
> > u8 buf[sizeof(intel_dp->train_set) + 1];
> > - int ret, len;
> > + int len;
> >
> > intel_dp_program_link_training_pattern(intel_dp, dp_train_pat);
> >
> > - buf[0] = dp_train_pat;
> > if ((dp_train_pat & ~DP_LINK_SCRAMBLING_DISABLE) ==
> > - DP_TRAINING_PATTERN_DISABLE) {
> > + DP_TRAINING_PATTERN_DISABLE)
> > /* don't write DP_TRAINING_LANEx_SET on disable */
>
> As mentioned in the other patch I think we're doing things in the wrong
> order here. I suspect it'll be cleaner to just stop doing
> intel_dp_set_link_train(DISABLE) entirely and just have a dedicated
> function for disabling link training. We can then trivially do a
> followup to swap the order of operations to match the spec.
intel_dp_disable_dpcd_training_pattern() would be needed after each
LTTPR link training phase, where the port should not output idle
patterns, that's the only reason for this change.
Do you mean to remove intel_dp_stop_link_train() then and do the idle
pattern programming + corresponding DPCD training pattern disable
programming at the end of the link training sequence (and remove the
DP_TRAINING_PATTERN_DISABLE case handling from above)? I agree with
that, but I see that too as a follow-up material (along with changing
the order as you suggested).
>
> > - len = 1;
> > - } else {
> > - /* DP_TRAINING_LANEx_SET follow DP_TRAINING_PATTERN_SET */
> > - memcpy(buf + 1, intel_dp->train_set, intel_dp->lane_count);
> > - len = intel_dp->lane_count + 1;
> > - }
> > + return intel_dp_disable_dpcd_training_pattern(intel_dp);
> >
> > - ret = drm_dp_dpcd_write(&intel_dp->aux, DP_TRAINING_PATTERN_SET,
> > - buf, len);
> > + buf[0] = dp_train_pat;
> > + /* DP_TRAINING_LANEx_SET follow DP_TRAINING_PATTERN_SET */
> > + memcpy(buf + 1, intel_dp->train_set, intel_dp->lane_count);
> > + len = intel_dp->lane_count + 1;
> >
> > - return ret == len;
> > + return drm_dp_dpcd_write(&intel_dp->aux, DP_TRAINING_PATTERN_SET,
> > + buf, len) == len;
> > }
> >
> > static bool
> > --
> > 2.17.1
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Ville Syrjälä
> Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
