[Intel-gfx] [PATCH 05/13] drm/i915/dsb: Dump the DSB command buffer when DSB fails

[Manna, Animesh]

> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces at lists.freedesktop.org> On Behalf Of Ville
> Syrjala
> Sent: Wednesday, January 18, 2023 10:01 PM
> To: intel-gfx at lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH 05/13] drm/i915/dsb: Dump the DSB command
> buffer when DSB fails
> 
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> Dump the full DSB command buffers and head/tail pointers if the the DSB
> hasn't completed its job in time.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dsb.c | 33 +++++++++++++++++++++---
>  1 file changed, 30 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c
> b/drivers/gpu/drm/i915/display/intel_dsb.c
> index 9e25b1345927..f454329b6901 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsb.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
> @@ -92,6 +92,22 @@ static bool assert_dsb_has_room(struct intel_dsb
> *dsb)
>  			 crtc->base.base.id, crtc->base.name, dsb->id);  }
> 
> +static void intel_dsb_dump(struct intel_dsb *dsb) {
> +	struct intel_crtc *crtc = dsb->crtc;
> +	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
> +	const u32 *buf = dsb->cmd_buf;
> +	int i;
> +
> +	drm_dbg_kms(&i915->drm, "[CRTC:%d:%s] DSB %d commands {\n",
> +		    crtc->base.base.id, crtc->base.name, dsb->id);
> +	for (i = 0; i < ALIGN(dsb->free_pos, 64 / 4); i += 4)
> +		drm_dbg_kms(&i915->drm,
> +			    " 0x%08x: 0x%08x 0x%08x 0x%08x 0x%08x\n",
> +			    i * 4, buf[i], buf[i+1], buf[i+2], buf[i+3]);
> +	drm_dbg_kms(&i915->drm, "}\n");
> +}
> +
>  static bool is_dsb_busy(struct drm_i915_private *i915, enum pipe pipe,
>  			enum dsb_id id)
>  {
> @@ -260,10 +276,21 @@ void intel_dsb_wait(struct intel_dsb *dsb)
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>  	enum pipe pipe = crtc->pipe;
> 
> -	if (wait_for(!is_dsb_busy(dev_priv, pipe, dsb->id), 1))
> +	if (wait_for(!is_dsb_busy(dev_priv, pipe, dsb->id), 1)) {
> +		u32 offset = i915_ggtt_offset(dsb->vma);
> +
> +		intel_de_write(dev_priv, DSB_CTRL(pipe, dsb->id),
> +			       DSB_ENABLE | DSB_HALT);

One doubt - Why DSB_ENABLE bit is set here? Is setting DSB_HALT not sufficient.
Other than above the changes look good to me.

Regards,
Animesh

> +
>  		drm_err(&dev_priv->drm,
> -			"[CRTC:%d:%s] DSB %d timed out waiting for idle\n",
> -			crtc->base.base.id, crtc->base.name, dsb->id);
> +			"[CRTC:%d:%s] DSB %d timed out waiting for idle
> (current head=0x%x, head=0x%x, tail=0x%x)\n",
> +			crtc->base.base.id, crtc->base.name, dsb->id,
> +			intel_de_read(dev_priv, DSB_CURRENT_HEAD(pipe,
> dsb->id)) - offset,
> +			intel_de_read(dev_priv, DSB_HEAD(pipe, dsb->id)) -
> offset,
> +			intel_de_read(dev_priv, DSB_TAIL(pipe, dsb->id)) -
> offset);
> +
> +		intel_dsb_dump(dsb);
> +	}
> 
>  	/* Attempt to reset it */
>  	dsb->free_pos = 0;
> --
> 2.38.2