[Intel-gfx] [PATCH 3/7] drm/i915: Simplify the link training functions
Imre Deak
imre.deak at intel.com
Tue Sep 22 15:30:35 UTC 2020
On Tue, Sep 22, 2020 at 04:27:05PM +0300, Ville Syrjälä wrote:
> On Tue, Sep 22, 2020 at 03:51:02PM +0300, Imre Deak wrote:
> > Split the prepare, link training, fallback-handling steps into their own
> > functions for clarity and as a preparation for the upcoming LTTPR changes.
> >
> > While at it also add some documentation to exported functions.
> >
> > Signed-off-by: Imre Deak <imre.deak at intel.com>
> > ---
> > .../drm/i915/display/intel_dp_link_training.c | 80 ++++++++++++++-----
> > 1 file changed, 62 insertions(+), 18 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 6d13d00db5e6..0c3809891bd2 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> > @@ -162,14 +162,13 @@ static bool intel_dp_link_max_vswing_reached(struct intel_dp *intel_dp)
> > return true;
> > }
> >
> > -/* Enable corresponding port and start training pattern 1 */
> > -static bool
> > -intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp)
> > +/*
> > + * Prepare link training by configuring the link parameters and enabling the
> > + * corresponding port.
> > + */
> > +static void intel_dp_prepare_link_train(struct intel_dp *intel_dp)
> > {
> > struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> > - u8 voltage;
> > - int voltage_tries, cr_tries, max_cr_tries;
> > - bool max_vswing_reached = false;
> > u8 link_config[2];
> > u8 link_bw, rate_select;
> >
> > @@ -203,6 +202,16 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp)
> > drm_dp_dpcd_write(&intel_dp->aux, DP_DOWNSPREAD_CTRL, link_config, 2);
> >
> > intel_dp->DP |= DP_PORT_EN;
>
> I guess we no longer actually enable the port here? The comment ^ still says
> we do.
>
> Hmm. Seems we do enable the port on ddi platforms, but not on older
> platforms. I guess the docs could still use a tweak to reflect
> reality a bit better.
Yes, missed the old platform part, will update the comment.
>
> > +}
> > +
> > +/* Perform the link training clock recovery phase using training pattern 1. */
> > +static bool
> > +intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp)
> > +{
> > + struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> > + u8 voltage;
> > + int voltage_tries, cr_tries, max_cr_tries;
> > + bool max_vswing_reached = false;
> >
> > /* clock recovery */
> > if (!intel_dp_reset_link_train(intel_dp,
> > @@ -325,6 +334,10 @@ static u32 intel_dp_training_pattern(struct intel_dp *intel_dp)
> > return DP_TRAINING_PATTERN_2;
> > }
> >
> > +/*
> > + * Perform the link training channel equalization phase using one of training
> > + * pattern 2, 3 or 4 depending on the the source and sink capabilities.
> > + */
> > static bool
> > intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp)
> > {
> > @@ -395,6 +408,15 @@ intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp)
> >
> > }
> >
> > +/**
> > + * intel_dp_stop_link_train - stop link training
> > + * @intel_dp: DP struct
> > + *
> > + * Stop the link training of the @intel_dp port, programming the port to
> > + * output an idle pattern
>
> I don't think we use the idle pattern on all platforms.
Yes, just DDI, this also needs a doc update.
> BTW intel_dp_set_idle_link_train() looks pretty pointless. Could just
> inline it into its only caller, or at least move it into
> intel_dp_link_training.c.
Ok, can unexport/inline it. Btw, this part made me wonder what's the
exact reason for keeping the idle pattern output and corresponding DPCD
programming separate, that is why can't we disable the training pattern
in DPCD after intel_dp_set_idle_link_train()? That would make things
more uniform on all platforms.
>
> Reviewed-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
>
> > on the link and disabling the training pattern in
> > + * the sink's DPCD.
> > + * This function must be called after intel_dp_start_link_train().
> > + */
> > void intel_dp_stop_link_train(struct intel_dp *intel_dp)
> > {
> > intel_dp->link_trained = true;
> > @@ -403,30 +425,37 @@ void intel_dp_stop_link_train(struct intel_dp *intel_dp)
> > DP_TRAINING_PATTERN_DISABLE);
> > }
> >
> > -void
> > -intel_dp_start_link_train(struct intel_dp *intel_dp)
> > +static bool
> > +intel_dp_link_train(struct intel_dp *intel_dp)
> > {
> > struct intel_connector *intel_connector = intel_dp->attached_connector;
> > + bool ret = false;
> > +
> > + intel_dp_prepare_link_train(intel_dp);
> >
> > if (!intel_dp_link_training_clock_recovery(intel_dp))
> > - goto failure_handling;
> > + goto out;
> > +
> > if (!intel_dp_link_training_channel_equalization(intel_dp))
> > - goto failure_handling;
> > + goto out;
> >
> > - drm_dbg_kms(&dp_to_i915(intel_dp)->drm,
> > - "[CONNECTOR:%d:%s] Link Training Passed at Link Rate = %d, Lane count = %d",
> > - intel_connector->base.base.id,
> > - intel_connector->base.name,
> > - intel_dp->link_rate, intel_dp->lane_count);
> > - return;
> > + ret = true;
> >
> > - failure_handling:
> > +out:
> > drm_dbg_kms(&dp_to_i915(intel_dp)->drm,
> > - "[CONNECTOR:%d:%s] Link Training failed at link rate = %d, lane count = %d",
> > + "[CONNECTOR:%d:%s] Link Training %s at Link Rate = %d, Lane count = %d",
> > intel_connector->base.base.id,
> > intel_connector->base.name,
> > + ret ? "passed" : "failed",
> > intel_dp->link_rate, intel_dp->lane_count);
> >
> > + return ret;
> > +}
> > +
> > +static void intel_dp_schedule_fallback_link_training(struct intel_dp *intel_dp)
> > +{
> > + struct intel_connector *intel_connector = intel_dp->attached_connector;
> > +
> > if (intel_dp->hobl_active) {
> > drm_dbg_kms(&dp_to_i915(intel_dp)->drm,
> > "Link Training failed with HOBL active, not enabling it from now on");
> > @@ -440,3 +469,18 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
> > /* Schedule a Hotplug Uevent to userspace to start modeset */
> > schedule_work(&intel_connector->modeset_retry_work);
> > }
> > +
> > +/**
> > + * intel_dp_start_link_train - start link training
> > + * @intel_dp: DP struct
> > + *
> > + * Start the link training of the @intel_dp port, scheduling a fallback
> > + * retraining with reduced link rate/lane parameters if the link training
> > + * fails.
> > + * After calling this function intel_dp_stop_link_train() must be called.
> > + */
> > +void intel_dp_start_link_train(struct intel_dp *intel_dp)
> > +{
> > + if (!intel_dp_link_train(intel_dp))
> > + intel_dp_schedule_fallback_link_training(intel_dp);
> > +}
> > --
> > 2.17.1
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Ville Syrjälä
> Intel
More information about the Intel-gfx
mailing list