Re: [PATCH 6/6] drm/i915: collect more per ring error state

[Chris Wilson <chris@chris-wilson.co.uk>]

On Tue, 11 Oct 2011 16:39:14 +0200, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Based on a patch by Ben Widawsky, but with different colors
> for the bikeshed.
> 
> In contrast to Ben's patch this one doesn't add the fault regs.
> Afaics they're for the optional page fault support which
> - we're not enabling
> - and which seems to be unsupported by the hw team. Recent bspec
>   lacks tons of information about this that the public docs released
>   half a year back still contain.
> 
> Also dump ring HEAD/TAIL registers - I've recently seen a few
> error_state where just guessing these is not good enough.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> dump head/tail into error_state
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c |    5 ++++-
>  drivers/gpu/drm/i915/i915_drv.h     |    5 ++++-
>  drivers/gpu/drm/i915/i915_irq.c     |    7 ++++++-
>  drivers/gpu/drm/i915/i915_reg.h     |    2 ++
>  4 files changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index ea7237d..5d73ae5 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -733,17 +733,20 @@ static void i915_ring_error_state(struct seq_file *m,
>  				  unsigned ring)
>  {
>  	seq_printf(m, "%s command stream:\n", ring_str(ring));
> +	seq_printf(m, "  HEAD: 0x%08x\n", error->head[ring]);
> +	seq_printf(m, "  TAIL: 0x%08x\n", error->tail[ring]);
>  	seq_printf(m, "  ACTHD: 0x%08x\n", error->acthd[ring]);
>  	seq_printf(m, "  IPEIR: 0x%08x\n", error->ipeir[ring]);
>  	seq_printf(m, "  IPEHR: 0x%08x\n", error->ipehr[ring]);
>  	seq_printf(m, "  INSTDONE: 0x%08x\n", error->instdone[ring]);
> +	seq_printf(m, "  INSTPS: 0x%08x\n", error->instps[ring]);
>  	if (ring == RCS) {
>  		if (INTEL_INFO(dev)->gen >= 4) {
>  			seq_printf(m, "  INSTDONE1: 0x%08x\n", error->instdone1);
> -			seq_printf(m, "  INSTPS: 0x%08x\n", error->instps);
>  		}
>  		seq_printf(m, "  INSTPM: 0x%08x\n", error->instpm);

Hmm, isn't INSTPM also per-ring?

Can you also add the DMA fetch addresses whilst you are here? I've never
found an instance where they were more informative than the ACTHD, but
that was only for a very small sampling of local errors.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre