Re: [PATCH 5/5] drm/i915: Implement Link Rate fallback on Link training failure

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 18 Nov 2016, Manasi Navare <manasi.d.navare@xxxxxxxxx> 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.
>
> 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@xxxxxxx>
> Acked-by: Harry Wentland <Harry.wentland@xxxxxxx>
> Cc: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx>
> Cc: Daniel Vetter <daniel.vetter@xxxxxxxxx>
> Cc: Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx>
> Signed-off-by: Manasi Navare <manasi.d.navare@xxxxxxxxx>
> ---
>  drivers/gpu/drm/i915/intel_dp.c               | 63 ++++++++++++++++++++++++++-
>  drivers/gpu/drm/i915/intel_dp_link_training.c | 28 +++++++++++-
>  drivers/gpu/drm/i915/intel_drv.h              |  3 ++
>  3 files changed, 90 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 4fb89e1..a56a34d 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -351,8 +351,14 @@ int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
>  		target_clock = fixed_mode->clock;
>  	}
>  
> -	max_link_clock = intel_dp_max_link_rate(intel_dp);
> -	max_lanes = intel_dp_max_lane_count(intel_dp);
> +	/* Prune the modes using the fallback link rate/lane count */
> +	if (connector->link_status == DRM_MODE_LINK_STATUS_BAD) {
> +		max_link_clock = intel_dp->fallback_link_rate;
> +		max_lanes = intel_dp->fallback_lane_count;
> +	} else {
> +		max_link_clock = intel_dp_max_link_rate(intel_dp);
> +		max_lanes = intel_dp_max_lane_count(intel_dp);
> +	}
>  
>  	max_rate = intel_dp_max_data_rate(max_link_clock, max_lanes);
>  	mode_rate = intel_dp_link_required(target_clock, 18);
> @@ -1588,6 +1594,7 @@ static int intel_dp_compute_bpp(struct intel_dp *intel_dp,
>  	enum port port = dp_to_dig_port(intel_dp)->port;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->base.crtc);
>  	struct intel_connector *intel_connector = intel_dp->attached_connector;
> +	struct drm_connector *connector = &intel_connector->base;
>  	int lane_count, clock;
>  	int min_lane_count = 1;
>  	int max_lane_count = intel_dp_max_lane_count(intel_dp);
> @@ -1635,6 +1642,14 @@ static int intel_dp_compute_bpp(struct intel_dp *intel_dp,
>  	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
>  		return false;
>  
> +	/* Fall back to lower link rate in case of failure in previous modeset */
> +	if (connector->link_status == DRM_MODE_LINK_STATUS_BAD) {
> +		min_lane_count = max_lane_count = intel_dp->fallback_lane_count;
> +		min_clock = max_clock = intel_dp_link_rate_index(intel_dp,
> +								 common_rates,
> +								 intel_dp->fallback_link_rate);
> +	}
> +
>  	DRM_DEBUG_KMS("DP link computation with max lane count %i "
>  		      "max bw %d pixel clock %iKHz\n",
>  		      max_lane_count, common_rates[max_clock],
> @@ -4415,6 +4430,10 @@ static bool intel_digital_port_connected(struct drm_i915_private *dev_priv,
>  		intel_dp->compliance_test_active = 0;
>  		intel_dp->compliance_test_type = 0;
>  		intel_dp->compliance_test_data = 0;
> +		intel_dp->fallback_link_rate = 0;
> +		intel_dp->fallback_lane_count = 0;
> +		drm_mode_connector_set_link_status_property(connector,
> +							    DRM_MODE_LINK_STATUS_GOOD);
>  
>  		if (intel_dp->is_mst) {
>  			DRM_DEBUG_KMS("MST device may have disappeared %d vs %d\n",
> @@ -4506,6 +4525,11 @@ static bool intel_digital_port_connected(struct drm_i915_private *dev_priv,
>  	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
>  		      connector->base.id, connector->name);
>  
> +	/* If this is a retry due to link trianing failure */
> +	if (status == connector_status_connected &&
> +	    connector->link_status == DRM_MODE_LINK_STATUS_BAD)
> +		return status;
> +
>  	/* If full detect is not performed yet, do a full detect */
>  	if (!intel_dp->detect_done)
>  		status = intel_dp_long_pulse(intel_dp->attached_connector);
> @@ -5671,6 +5695,37 @@ 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;
> +	struct drm_display_mode *mode;
> +	bool verbose_prune = true;

Nitpick, totally unnecessary temporary variable.

> +
> +	intel_connector = container_of(work, typeof(*intel_connector),
> +				       modeset_retry_work);
> +	connector = &intel_connector->base;
> +
> +	/* Grab the locks before changing connector property*/
> +	mutex_lock(&connector->dev->mode_config.mutex);
> +	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", connector->base.id,
> +		      connector->name);
> +	list_for_each_entry(mode, &connector->modes, head) {
> +		mode->status = intel_dp_mode_valid(connector,
> +						   mode);
> +	}

The first time we end up here, we'll still have "good" link status, and
the fallback values won't be taken into account in
intel_dp_mode_valid(). If there are modes that no longer fit into the
fallback values, compute config will fail off the bat the next time
around. So I think you should set link status to bad first.

BR,
Jani.

> +	drm_mode_prune_invalid(connector->dev, &connector->modes,
> +			       verbose_prune);
> +	/* 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)
> @@ -5683,6 +5738,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..cc243db 100644
> --- a/drivers/gpu/drm/i915/intel_dp_link_training.c
> +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
> @@ -313,6 +313,30 @@ 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;
> +	struct drm_connector *connector = &intel_connector->base;
> +
> +	if (!intel_dp_link_training_clock_recovery(intel_dp))
> +		goto failure_handling;
> +	if (!intel_dp_link_training_channel_equalization(intel_dp))
> +		goto failure_handling;
> +
> +	/* Reset the Link Train Values */
> +	DRM_DEBUG_KMS("Link Training Passed at Link Rate = %d, Lane count = %d",
> +		      intel_dp->link_rate, intel_dp->lane_count);
> +	intel_dp->fallback_link_rate = 0;
> +	intel_dp->fallback_lane_count = 0;
> +	drm_mode_connector_set_link_status_property(connector,
> +						    DRM_MODE_LINK_STATUS_GOOD);
> +	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 e1c43a9..33be66f 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux