[PATCH 4/4] drm/i915: Implement Link Rate fallback on Link training failure
Manasi Navare
manasi.d.navare at intel.com
Thu Dec 8 22:09:10 UTC 2016
On Thu, Dec 08, 2016 at 11:51:30PM +0200, Jani Nikula wrote:
> On Tue, 06 Dec 2016, Manasi Navare <manasi.d.navare at intel.com> wrote:
> > If link training at a link rate optimal for a particular
> > mode fails during modeset's atomic commit phase, then we
> > let the modeset complete and then retry. We save the link rate
> > value at which link training failed, update the link status property
> > to "BAD" and use a lower link rate to prune the modes. It will redo
> > the modeset on the current mode at lower link rate or if the current
> > mode gets pruned due to lower link constraints then, it will send a
> > hotplug uevent for userspace to handle it.
> >
> > This is also required to pass DP CTS tests 4.3.1.3, 4.3.1.4,
> > 4.3.1.6.
> >
> > v9:
> > * Use the trimmed max values of link rate/lane count based on
> > link train fallback (Daniel Vetter)
> > v8:
> > * Set link_status to BAD first and then call mode_valid (Jani Nikula)
> > v7:
> > Remove the redundant variable in previous patch itself
> > v6:
> > * Obtain link rate index from fallback_link_rate using
> > the helper intel_dp_link_rate_index (Jani Nikula)
> > * Include fallback within intel_dp_start_link_train (Jani Nikula)
> > v5:
> > * Move set link status to drm core (Daniel Vetter, Jani Nikula)
> > v4:
> > * Add fallback support for non DDI platforms too
> > * Set connector->link status inside set_link_status function
> > (Jani Nikula)
> > v3:
> > * Set link status property to BAd unconditionally (Jani Nikula)
> > * Dont use two separate variables link_train_failed and link_status
> > to indicate same thing (Jani Nikula)
> > v2:
> > * Squashed a few patches (Jani Nikula)
> >
> > Acked-by: Tony Cheng <tony.cheng at amd.com>
> > Acked-by: Harry Wentland <Harry.wentland at amd.com>
> > Cc: Jani Nikula <jani.nikula at linux.intel.com>
> > Cc: Daniel Vetter <daniel.vetter at intel.com>
> > Cc: Ville Syrjala <ville.syrjala at linux.intel.com>
> > Signed-off-by: Manasi Navare <manasi.d.navare at intel.com>
>
> This is not a journey I'd like to do again, but I like the place where
> we've finally arrived.
>
> Reviewed-by: Jani Nikula <jani.nikula at intel.com>
>
Thanks a lot Jani for all the reviews till now. Its actually
been a great journey for me learning so many things from you. I would def like to do this
again.
Regards
Manasi
> > ---
> > drivers/gpu/drm/i915/intel_dp.c | 27 +++++++++++++++++++++++++++
> > drivers/gpu/drm/i915/intel_dp_link_training.c | 22 ++++++++++++++++++++--
> > drivers/gpu/drm/i915/intel_drv.h | 3 +++
> > 3 files changed, 50 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index b5c7526f..404da32 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -5676,6 +5676,29 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
> > return false;
> > }
> >
> > +static void intel_dp_modeset_retry_work_fn(struct work_struct *work)
> > +{
> > + struct intel_connector *intel_connector;
> > + struct drm_connector *connector;
> > +
> > + intel_connector = container_of(work, typeof(*intel_connector),
> > + modeset_retry_work);
> > + connector = &intel_connector->base;
> > + DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", connector->base.id,
> > + connector->name);
> > +
> > + /* Grab the locks before changing connector property*/
> > + mutex_lock(&connector->dev->mode_config.mutex);
> > + /* Set connector link status to BAD and send a Uevent to notify
> > + * userspace to do a modeset.
> > + */
> > + drm_mode_connector_set_link_status_property(connector,
> > + DRM_MODE_LINK_STATUS_BAD);
> > + mutex_unlock(&connector->dev->mode_config.mutex);
> > + /* Send Hotplug uevent so userspace can reprobe */
> > + drm_kms_helper_hotplug_event(connector->dev);
> > +}
> > +
> > bool
> > intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
> > struct intel_connector *intel_connector)
> > @@ -5688,6 +5711,10 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
> > enum port port = intel_dig_port->port;
> > int type;
> >
> > + /* Initialize the work for modeset in case of link train failure */
> > + INIT_WORK(&intel_connector->modeset_retry_work,
> > + intel_dp_modeset_retry_work_fn);
> > +
> > if (WARN(intel_dig_port->max_lanes < 1,
> > "Not enough lanes (%d) for DP on port %c\n",
> > intel_dig_port->max_lanes, port_name(port)))
> > diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c
> > index 0048b52..955b239 100644
> > --- a/drivers/gpu/drm/i915/intel_dp_link_training.c
> > +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
> > @@ -313,6 +313,24 @@ void intel_dp_stop_link_train(struct intel_dp *intel_dp)
> > void
> > intel_dp_start_link_train(struct intel_dp *intel_dp)
> > {
> > - intel_dp_link_training_clock_recovery(intel_dp);
> > - intel_dp_link_training_channel_equalization(intel_dp);
> > + struct intel_connector *intel_connector = intel_dp->attached_connector;
> > +
> > + if (!intel_dp_link_training_clock_recovery(intel_dp))
> > + goto failure_handling;
> > + if (!intel_dp_link_training_channel_equalization(intel_dp))
> > + goto failure_handling;
> > +
> > + DRM_DEBUG_KMS("Link Training Passed at Link Rate = %d, Lane count = %d",
> > + intel_dp->link_rate, intel_dp->lane_count);
> > + return;
> > +
> > + failure_handling:
> > + DRM_DEBUG_KMS("Link Training failed at link rate = %d, lane count = %d",
> > + intel_dp->link_rate, intel_dp->lane_count);
> > + if (!intel_dp_get_link_train_fallback_values(intel_dp,
> > + intel_dp->link_rate,
> > + intel_dp->lane_count))
> > + /* Schedule a Hotplug Uevent to userspace to start modeset */
> > + schedule_work(&intel_connector->modeset_retry_work);
> > + return;
> > }
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 47e3671..9d6f7ee 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -315,6 +315,9 @@ struct intel_connector {
> > void *port; /* store this opaque as its illegal to dereference it */
> >
> > struct intel_dp *mst_port;
> > +
> > + /* Work struct to schedule a uevent on link train failure */
> > + struct work_struct modeset_retry_work;
> > };
> >
> > struct dpll {
>
> --
> Jani Nikula, Intel Open Source Technology Center
More information about the dri-devel
mailing list