Re: [PATCH] drm/i915: Tune down link train error messages due to IO failure

[Manasi Navare <manasi.d.navare@intel.com>]

On Fri, May 24, 2019 at 07:49:07PM +0300, Imre Deak wrote:
> On Fri, May 24, 2019 at 07:27:54PM +0300, Martin Peres wrote:
> > On 23/05/2019 16:08, Imre Deak wrote:
> > > An IO failure happens if the sink is unplugged. This scenario shouldn't
> > > be logged with error level, since it is not a user visible problem.
> > > Converting the corresponding error messages to be debug messages also
> > > ensures that we don't fail some CI test incorrectly that tests the
> > > unplugging while a mode is still active on the given connector (like the
> > > kms_chamelium tests imititating such an unplug during being suspended).
> > > 
> > > Leave those link training error messages that are due to an actual
> > > transfer rate error (and not an IO error) as-is, as that kind of problem
> > > is related to the user's configuration (cable, mode, monitor) and so it
> > > is really user visible.
> > 
> > Are there some scenarios that would be warranting a DRM_ERROR? If so,
> > then we should decide on the verbosity based on wether the connector is
> > connected or disconnected, don't you think?
> 
> I don't think we should depend on the correctness of setting the message
> level correctly based on the live state, since the live state itself is
> flaky/unreliable based on the timing of the unplug vs. the settling of
> the live state flag. For instance we could see the AUX IO failing
> already but live state still indicating that the sink is plugged, so
> we'd print again an error instead of the correct tuned-down note.
>

But in case of AUX failures which we see a lot when AUX power
is accidently off or eDP is still not ON or sink is unplugged while
the link training was in process, doesnt it make sense to
call it as a DRM_ERROR and not just a DEBUG_KMS.
For Eg: the CI tests failed due to AUX timeouts should be the actual failures
which would be filtered now if we tune it down to DEBUG_KMS.

Manasi
 
> > 
> > Martin
> > > 
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110390
> > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_dp_link_training.c | 12 ++++++------
> > >  1 file changed, 6 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c
> > > index 9b1fccea966b..392ebeb89ce8 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp_link_training.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
> > > @@ -169,7 +169,7 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp)
> > >  	if (!intel_dp_reset_link_train(intel_dp,
> > >  				       DP_TRAINING_PATTERN_1 |
> > >  				       DP_LINK_SCRAMBLING_DISABLE)) {
> > > -		DRM_ERROR("failed to enable link training\n");
> > > +		DRM_DEBUG_KMS("failed to enable link training\n");
> > >  		return false;
> > >  	}
> > >  
> > > @@ -193,7 +193,7 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp)
> > >  		drm_dp_link_train_clock_recovery_delay(intel_dp->dpcd);
> > >  
> > >  		if (!intel_dp_get_link_status(intel_dp, link_status)) {
> > > -			DRM_ERROR("failed to get link status\n");
> > > +			DRM_DEBUG_KMS("failed to get link status\n");
> > >  			return false;
> > >  		}
> > >  
> > > @@ -217,7 +217,7 @@ intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp)
> > >  		/* Update training set as requested by target */
> > >  		intel_get_adjust_train(intel_dp, link_status);
> > >  		if (!intel_dp_update_link_train(intel_dp)) {
> > > -			DRM_ERROR("failed to update link training\n");
> > > +			DRM_DEBUG_KMS("failed to update link training\n");
> > >  			return false;
> > >  		}
> > >  
> > > @@ -295,7 +295,7 @@ intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp)
> > >  	/* channel equalization */
> > >  	if (!intel_dp_set_link_train(intel_dp,
> > >  				     training_pattern)) {
> > > -		DRM_ERROR("failed to start channel equalization\n");
> > > +		DRM_DEBUG_KMS("failed to start channel equalization\n");
> > >  		return false;
> > >  	}
> > >  
> > > @@ -303,7 +303,7 @@ intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp)
> > >  
> > >  		drm_dp_link_train_channel_eq_delay(intel_dp->dpcd);
> > >  		if (!intel_dp_get_link_status(intel_dp, link_status)) {
> > > -			DRM_ERROR("failed to get link status\n");
> > > +			DRM_DEBUG_KMS("failed to get link status\n");
> > >  			break;
> > >  		}
> > >  
> > > @@ -327,7 +327,7 @@ intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp)
> > >  		/* Update training set as requested by target */
> > >  		intel_get_adjust_train(intel_dp, link_status);
> > >  		if (!intel_dp_update_link_train(intel_dp)) {
> > > -			DRM_ERROR("failed to update link training\n");
> > > +			DRM_DEBUG_KMS("failed to update link training\n");
> > >  			break;
> > >  		}
> > >  	}
> > > 
> > 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx