[Intel-gfx] [PATCH 3/4] drm/dp: Clarify clock recovery and channel equalization failures

[chris at chris-wilson.co.uk]

On Thu, Aug 04, 2016 at 04:50:35PM +0000, Pandiyan, Dhinakaran wrote:
> On Thu, 2016-08-04 at 04:12 +0100, Chris Wilson wrote:
> > On Wed, Aug 03, 2016 at 08:07:40PM -0700, Dhinakaran Pandiyan wrote:
> > > The causes of clock recovery and channel equalization failures are not
> > > explicitly printed in debug messages. Help debugging link training
> > > failures by printing why it failed.
> > > 
> > > Doing this in the driver would mean re-implementing some of the drm static
> > > functions that decode link status. Let's avoid that with these debug
> > > messages in drm.
> > > 
> > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> > > ---
> > >  drivers/gpu/drm/drm_dp_helper.c | 12 +++++++++---
> > >  1 file changed, 9 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> > > index 091053e..d763b57 100644
> > > --- a/drivers/gpu/drm/drm_dp_helper.c
> > > +++ b/drivers/gpu/drm/drm_dp_helper.c
> > > @@ -64,12 +64,16 @@ bool drm_dp_channel_eq_ok(const u8 link_status[DP_LINK_STATUS_SIZE],
> > >  
> > >  	lane_align = dp_link_status(link_status,
> > >  				    DP_LANE_ALIGN_STATUS_UPDATED);
> > > -	if ((lane_align & DP_INTERLANE_ALIGN_DONE) == 0)
> > > +	if ((lane_align & DP_INTERLANE_ALIGN_DONE) == 0) {
> > > +		DRM_DEBUG_KMS("Inter-lane alignment not done\n");
> > >  		return false;
> > > +	}
> > >  	for (lane = 0; lane < lane_count; lane++) {
> > >  		lane_status = dp_get_lane_status(link_status, lane);
> > > -		if ((lane_status & DP_CHANNEL_EQ_BITS) != DP_CHANNEL_EQ_BITS)
> > > +		if ((lane_status & DP_CHANNEL_EQ_BITS) != DP_CHANNEL_EQ_BITS) {
> > > +			DRM_DEBUG_KMS("Channel equalization not done for lane %d\n", lane);
> > >  			return false;
> > > +		}
> > >  	}
> > >  	return true;
> > >  }
> > > @@ -83,8 +87,10 @@ bool drm_dp_clock_recovery_ok(const u8 link_status[DP_LINK_STATUS_SIZE],
> > >  
> > >  	for (lane = 0; lane < lane_count; lane++) {
> > >  		lane_status = dp_get_lane_status(link_status, lane);
> > > -		if ((lane_status & DP_LANE_CR_DONE) == 0)
> > > +		if ((lane_status & DP_LANE_CR_DONE) == 0) {
> > > +			DRM_DEBUG_KMS("Clock recovery not done for lane %d\n", lane);
> > 
> > Please petition, with patch, for DRM_DEBUG_DP.
> > 
> Is that because DRM_DEBUG_KMS in not appropriate or that we have plenty
> of DP related debug messages that it warrants it's own debug macro?

In the case of link failure, we will have plenty of messages from DP.
One debug category just for DP may be overkill, and we may want
something more like DRM_DEBUG_KMS_LOWLEVEL (or DRM_DEBUG_KMS_DRIVER, or
DRM_DEBUG_KMS_HW etc) that suits all similar link training without
cluttering up either the high levels telling what the user selected, and
what the driver picked and the control flow through the modesetting.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre