All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Matt Roper <matthew.d.roper@intel.com>
Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 2/3] drm/i915: Include the AUX CH name in the debug messages
Date: Mon, 27 Jan 2020 18:08:10 +0200	[thread overview]
Message-ID: <20200127160810.GE13686@intel.com> (raw)
In-Reply-To: <20200123171904.GG2244136@mdroper-desk1.amr.corp.intel.com>

On Thu, Jan 23, 2020 at 09:19:04AM -0800, Matt Roper wrote:
> On Thu, Jan 23, 2020 at 05:45:41PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > To make it easier to figure out what caused a particular debug
> > message let's print out aux->name.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> It might be worth adding commas to some of these messages too while
> we're at it (e.g., "not started, status").  But either way, this is a
> really helpful cleanup.

Good idea. The first one of these already uses
"%s: blah (status 0x%x)" as the format, so I went ahead and
converted the rest to follow the same style.

> 
> Reviewed-by: Matt Roper <matthew.d.roper@intel.com>

2-3 pushed to dinq. Thanks for the review.

> 
> 
> > ---
> >  drivers/gpu/drm/i915/display/intel_dp.c | 19 +++++++++++--------
> >  1 file changed, 11 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 1795963e1866..941f0f6d55c1 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -1186,7 +1186,7 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp)
> >  	trace_i915_reg_rw(false, ch_ctl, status, sizeof(status), true);
> >  
> >  	if (!done)
> > -		DRM_ERROR("%s did not complete or timeout within %ums (status 0x%08x)\n",
> > +		DRM_ERROR("%s: did not complete or timeout within %ums (status 0x%08x)\n",
> >  			  intel_dp->aux.name, timeout_ms, status);
> >  #undef C
> >  
> > @@ -1373,8 +1373,8 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
> >  		const u32 status = intel_uncore_read(uncore, ch_ctl);
> >  
> >  		if (status != intel_dp->aux_busy_last_status) {
> > -			WARN(1, "dp_aux_ch not started status 0x%08x\n",
> > -			     status);
> > +			WARN(1, "%s: not started status 0x%08x\n",
> > +			     intel_dp->aux.name, status);
> >  			intel_dp->aux_busy_last_status = status;
> >  		}
> >  
> > @@ -1435,7 +1435,8 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
> >  	}
> >  
> >  	if ((status & DP_AUX_CH_CTL_DONE) == 0) {
> > -		DRM_ERROR("dp_aux_ch not done status 0x%08x\n", status);
> > +		DRM_ERROR("%s: not done status 0x%08x\n",
> > +			  intel_dp->aux.name, status);
> >  		ret = -EBUSY;
> >  		goto out;
> >  	}
> > @@ -1445,7 +1446,8 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
> >  	 * Timeouts occur when the sink is not connected
> >  	 */
> >  	if (status & DP_AUX_CH_CTL_RECEIVE_ERROR) {
> > -		DRM_ERROR("dp_aux_ch receive error status 0x%08x\n", status);
> > +		DRM_ERROR("%s: receive error status 0x%08x\n",
> > +			  intel_dp->aux.name, status);
> >  		ret = -EIO;
> >  		goto out;
> >  	}
> > @@ -1453,7 +1455,8 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
> >  	/* Timeouts occur when the device isn't connected, so they're
> >  	 * "normal" -- don't fill the kernel log with these */
> >  	if (status & DP_AUX_CH_CTL_TIME_OUT_ERROR) {
> > -		DRM_DEBUG_KMS("dp_aux_ch timeout status 0x%08x\n", status);
> > +		DRM_DEBUG_KMS("%s: timeout status 0x%08x\n",
> > +			      intel_dp->aux.name, status);
> >  		ret = -ETIMEDOUT;
> >  		goto out;
> >  	}
> > @@ -1468,8 +1471,8 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
> >  	 * drm layer takes care for the necessary retries.
> >  	 */
> >  	if (recv_bytes == 0 || recv_bytes > 20) {
> > -		DRM_DEBUG_KMS("Forbidden recv_bytes = %d on aux transaction\n",
> > -			      recv_bytes);
> > +		DRM_DEBUG_KMS("%s: Forbidden recv_bytes = %d on aux transaction\n",
> > +			      intel_dp->aux.name, recv_bytes);
> >  		ret = -EBUSY;
> >  		goto out;
> >  	}
> > -- 
> > 2.24.1
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Matt Roper
> Graphics Software Engineer
> VTT-OSGC Platform Enablement
> Intel Corporation
> (916) 356-2795

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

WARNING: multiple messages have this Message-ID (diff)
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Matt Roper <matthew.d.roper@intel.com>
Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 2/3] drm/i915: Include the AUX CH name in the debug messages
Date: Mon, 27 Jan 2020 18:08:10 +0200	[thread overview]
Message-ID: <20200127160810.GE13686@intel.com> (raw)
In-Reply-To: <20200123171904.GG2244136@mdroper-desk1.amr.corp.intel.com>

On Thu, Jan 23, 2020 at 09:19:04AM -0800, Matt Roper wrote:
> On Thu, Jan 23, 2020 at 05:45:41PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > To make it easier to figure out what caused a particular debug
> > message let's print out aux->name.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> It might be worth adding commas to some of these messages too while
> we're at it (e.g., "not started, status").  But either way, this is a
> really helpful cleanup.

Good idea. The first one of these already uses
"%s: blah (status 0x%x)" as the format, so I went ahead and
converted the rest to follow the same style.

> 
> Reviewed-by: Matt Roper <matthew.d.roper@intel.com>

2-3 pushed to dinq. Thanks for the review.

> 
> 
> > ---
> >  drivers/gpu/drm/i915/display/intel_dp.c | 19 +++++++++++--------
> >  1 file changed, 11 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 1795963e1866..941f0f6d55c1 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -1186,7 +1186,7 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp)
> >  	trace_i915_reg_rw(false, ch_ctl, status, sizeof(status), true);
> >  
> >  	if (!done)
> > -		DRM_ERROR("%s did not complete or timeout within %ums (status 0x%08x)\n",
> > +		DRM_ERROR("%s: did not complete or timeout within %ums (status 0x%08x)\n",
> >  			  intel_dp->aux.name, timeout_ms, status);
> >  #undef C
> >  
> > @@ -1373,8 +1373,8 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
> >  		const u32 status = intel_uncore_read(uncore, ch_ctl);
> >  
> >  		if (status != intel_dp->aux_busy_last_status) {
> > -			WARN(1, "dp_aux_ch not started status 0x%08x\n",
> > -			     status);
> > +			WARN(1, "%s: not started status 0x%08x\n",
> > +			     intel_dp->aux.name, status);
> >  			intel_dp->aux_busy_last_status = status;
> >  		}
> >  
> > @@ -1435,7 +1435,8 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
> >  	}
> >  
> >  	if ((status & DP_AUX_CH_CTL_DONE) == 0) {
> > -		DRM_ERROR("dp_aux_ch not done status 0x%08x\n", status);
> > +		DRM_ERROR("%s: not done status 0x%08x\n",
> > +			  intel_dp->aux.name, status);
> >  		ret = -EBUSY;
> >  		goto out;
> >  	}
> > @@ -1445,7 +1446,8 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
> >  	 * Timeouts occur when the sink is not connected
> >  	 */
> >  	if (status & DP_AUX_CH_CTL_RECEIVE_ERROR) {
> > -		DRM_ERROR("dp_aux_ch receive error status 0x%08x\n", status);
> > +		DRM_ERROR("%s: receive error status 0x%08x\n",
> > +			  intel_dp->aux.name, status);
> >  		ret = -EIO;
> >  		goto out;
> >  	}
> > @@ -1453,7 +1455,8 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
> >  	/* Timeouts occur when the device isn't connected, so they're
> >  	 * "normal" -- don't fill the kernel log with these */
> >  	if (status & DP_AUX_CH_CTL_TIME_OUT_ERROR) {
> > -		DRM_DEBUG_KMS("dp_aux_ch timeout status 0x%08x\n", status);
> > +		DRM_DEBUG_KMS("%s: timeout status 0x%08x\n",
> > +			      intel_dp->aux.name, status);
> >  		ret = -ETIMEDOUT;
> >  		goto out;
> >  	}
> > @@ -1468,8 +1471,8 @@ intel_dp_aux_xfer(struct intel_dp *intel_dp,
> >  	 * drm layer takes care for the necessary retries.
> >  	 */
> >  	if (recv_bytes == 0 || recv_bytes > 20) {
> > -		DRM_DEBUG_KMS("Forbidden recv_bytes = %d on aux transaction\n",
> > -			      recv_bytes);
> > +		DRM_DEBUG_KMS("%s: Forbidden recv_bytes = %d on aux transaction\n",
> > +			      intel_dp->aux.name, recv_bytes);
> >  		ret = -EBUSY;
> >  		goto out;
> >  	}
> > -- 
> > 2.24.1
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Matt Roper
> Graphics Software Engineer
> VTT-OSGC Platform Enablement
> Intel Corporation
> (916) 356-2795

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2020-01-27 16:08 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-23 15:45 [PATCH 1/3] drm/dp: Include the AUX CH name in the debug messages Ville Syrjala
2020-01-23 15:45 ` [Intel-gfx] " Ville Syrjala
2020-01-23 15:45 ` [PATCH 2/3] drm/i915: " Ville Syrjala
2020-01-23 15:45   ` [Intel-gfx] " Ville Syrjala
2020-01-23 17:19   ` Matt Roper
2020-01-23 17:19     ` [Intel-gfx] " Matt Roper
2020-01-27 16:08     ` Ville Syrjälä [this message]
2020-01-27 16:08       ` Ville Syrjälä
2020-01-23 15:45 ` [PATCH 3/3] drm/i915: Give aux channels a better name Ville Syrjala
2020-01-23 15:45   ` [Intel-gfx] " Ville Syrjala
2020-01-23 17:22   ` Matt Roper
2020-01-23 17:22     ` Matt Roper
2020-01-24  2:56 ` [Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/3] drm/dp: Include the AUX CH name in the debug messages Patchwork
2020-01-25 20:19 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200127160810.GE13686@intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=matthew.d.roper@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.