[Intel-gfx] [PATCH] Revert "drm/i915: Implement Link Rate fallback on Link training failure"

[Manasi Navare]

Hi Daniel,

I investigated the CI failures because of this patch. These
were essentially the failures which were always there but hidden because
they used be DEBUG_KMS messages for link failures so never got caught by CI.
But now my patch actually throws DRM_ERROR if the link training fails at RBR and 1 lane.
So it caught these errors. 

So going back to failures, there were two failures:
1. On SKL 6500 this was because the machine in CI lab is a SKL desktop without eDP
on Port A. But our VBT initialization code in the driver writes VBT defaults in a way
that it always sets DP flag on Port A and this does not get cleared after parsing the
VBT outputs. So now there is a fix for this by Jani Nikula in the driver which is merged.
"drm/i915/vbt: defaults handling for no VBT case"

2. On ILK desktop - This was happening because of a bad monitor desktop combination.
I switched the monitor in the CI lab and that helped get rid of the link failures
on ILK system.

So now looks like the fixes have gone in to fix the link failures thrown by this patch.
Should I resend this patch now?

Regards
Manasi

On Thu, Mar 02, 2017 at 09:17:47AM +0100, Daniel Vetter wrote:
> On Wed, Mar 01, 2017 at 06:17:49PM +0100, Daniel Vetter wrote:
> > This reverts commit 233ce881dd91fb13eb6b09deefae33168e6ead4c.
> > 
> > I assumed it's ok, but really should have double-checked - CI caught
> > tons of fail :(
> > 
> > Cc: Jani Nikula <jani.nikula at intel.com>
> > Cc: Manasi Navare <manasi.d.navare at intel.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
> 
> Pushed with Manasi' ack on irc.
> -Daniel
> 
> > ---
> >  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, 2 insertions(+), 50 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index af07a830fa95..d1670b8afbf5 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -5790,29 +5790,6 @@ 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)
> > @@ -5825,10 +5802,6 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
> >  	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 955b239e7c2d..0048b520baf7 100644
> > --- a/drivers/gpu/drm/i915/intel_dp_link_training.c
> > +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
> > @@ -313,24 +313,6 @@ void intel_dp_stop_link_train(struct intel_dp *intel_dp)
> >  void
> >  intel_dp_start_link_train(struct intel_dp *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;
> > +	intel_dp_link_training_clock_recovery(intel_dp);
> > +	intel_dp_link_training_channel_equalization(intel_dp);
> >  }
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 6c6abc932726..3b797909f631 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -316,9 +316,6 @@ 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 {
> > -- 
> > 2.11.0
> > 
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch