Re: [PATCH] drm/i915: print full error ring semaphore mboxes and sync.

[Ben Widawsky <ben@bwidawsk.net>]

On Thu, Jul 17, 2014 at 10:58:17AM -0700, Rodrigo Vivi wrote:
> With the increasing number of rings,
> we probably have more information to print than we were printing.
> 
> v2: Loop only over active rings and print info with ring names.
> 
> Cc: Ben Widawsky <benjamin.widawsky@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gpu_error.c | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index 36a7960..b1848e0 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -242,6 +242,10 @@ static void i915_ring_error_state(struct drm_i915_error_state_buf *m,
>  				  struct drm_device *dev,
>  				  struct drm_i915_error_ring *ring)
>  {
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_engine_cs *from, *to;
> +	int i, j;
> +
>  	if (!ring->valid)
>  		return;
>  
> @@ -264,23 +268,19 @@ static void i915_ring_error_state(struct drm_i915_error_state_buf *m,
>  	if (INTEL_INFO(dev)->gen >= 6) {
>  		err_printf(m, "  RC PSMI: 0x%08x\n", ring->rc_psmi);
>  		err_printf(m, "  FAULT_REG: 0x%08x\n", ring->fault_reg);
> -		err_printf(m, "  SYNC_0: 0x%08x [last synced 0x%08x]\n",
> -			   ring->semaphore_mboxes[0],
> -			   ring->semaphore_seqno[0]);
> -		err_printf(m, "  SYNC_1: 0x%08x [last synced 0x%08x]\n",
> -			   ring->semaphore_mboxes[1],
> -			   ring->semaphore_seqno[1]);
> -		if (HAS_VEBOX(dev)) {
> -			err_printf(m, "  SYNC_2: 0x%08x [last synced 0x%08x]\n",
> -				   ring->semaphore_mboxes[2],
> -				   ring->semaphore_seqno[2]);
> +		for_each_ring(from, dev_priv, i) {
> +			for_each_ring(to, dev_priv, j) {
> +				int idx = intel_ring_sync_index(from, to);
If you plan to get the gen8 gaps in the object:

if (i == j && !IS_GEN8())
	continue;

If not, with the existing code:
if (i == j)
	continue;

Currently, I am also in favor of
for(i = 0; i < NUM_RINGS - 1; i++) {
	if (!(INTEL_INFO(dev)->ring_mask & (1<<i)))
		continue;
	for(j = 0; j < NUM_RINGS - 1; j++) {
		if (!(INTEL_INFO(dev)->ring_mask & (j<<i)))
			continue;

		err_printf(...)
	}
}

or use if (ring_is_initialied()) instead of checking ring mask. For
error state, I prefer to use the mask because who knows what has
happened to the rings on reset or something.

Whatever you like. Fix the if (i == j) and lgtm

> +				err_printf(m, "  SYNC[%s -> %s]: 0x%08x [last synced 0x%08x]\n",
> +					   from->name, to->name,
> +					   ring->semaphore_mboxes[idx],
> +					   ring->semaphore_seqno[idx]);
> +			}



>  		}
>  	}
>  	if (USES_PPGTT(dev)) {
>  		err_printf(m, "  GFX_MODE: 0x%08x\n", ring->vm_info.gfx_mode);
> -

Should probably remove this if you do a respin.

>  		if (INTEL_INFO(dev)->gen >= 8) {
> -			int i;
>  			for (i = 0; i < 4; i++)
>  				err_printf(m, "  PDP%d: 0x%016llx\n",
>  					   i, ring->vm_info.pdp[i]);
> -- 
> 1.9.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ben Widawsky, Intel Open Source Technology Center