Re: [PATCH 3/5] drm/i915/error: report ctx id & desc for each request in the queue

[Arun Siluvery <arun.siluvery@xxxxxxxxxxxxxxx>]

On 29/01/2016 08:17, Mika Kuoppala wrote:
> Arun Siluvery <arun.siluvery@xxxxxxxxxxxxxxx> writes:
>
> > From: Dave Gordon <david.s.gordon@xxxxxxxxx>
> >
> > Also decode and output CSB entries, in time order
>
> Traditionally we have had the decoding burden in
> igt/tools/intel_error_decode.
>
> Is there reason not to follow that pattern?

I have not use error_decode much, most of the time it is easier to just 
have all the details in the error state itself instead of running 
another tool to decode it. Last when I used it still skips most of the 
unknown commands and it will be the case unless it is completely 
integrated with spec. We can extend the tool with these changes also but 
I think it helps to have this decoded info in error state.

regards
Arun

> -Mika
>
>
> > For: VIZ-2021
> > Signed-off-by: Dave Gordon <david.s.gordon@xxxxxxxxx>
> > ---
> >   drivers/gpu/drm/i915/i915_drv.h       |  1 +
> >   drivers/gpu/drm/i915/i915_gpu_error.c | 37 +++++++++++++++++++++++++++--------
> >   2 files changed, 30 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 8b510fb..239aaed 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -568,6 +568,7 @@ struct drm_i915_error_state {
> >   		} *req_ringbuffer, *hw_ringbuffer, *batchbuffer, *wa_batchbuffer, *ctx, *hws_page;
> >
> >   		struct drm_i915_error_request {
> > +			uint64_t ctx_desc;
> >   			long jiffies;
> >   			u32 seqno;
> >   			u32 tail;
> > diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> > index 5c8ec63..a88160c 100644
> > --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> > +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> > @@ -311,9 +311,25 @@ static void i915_ring_error_state(struct drm_i915_error_state_buf *m,
> >   	err_printf(m, "  EXECLIST_CSB_WR: 0x%08x\n", ring->execlist_csb_write_pointer);
> >   	err_printf(m, "  EXECLIST_CSB_RD: 0x%08x\n", ring->execlist_csb_read_pointer);
> >
> > -	for (i = 0; i < 6; i++) {
> > -		err_printf(m, "  EXECLIST_CSB[%d]: 0x%08x\n", i, ring->execlist_csb[i]);
> > -		err_printf(m, "  EXECLIST_CTX[%d]: 0x%08x\n", i, ring->execlist_ctx[i]);
> > +#define GEN8_CTX_STATUS_IDLE_ACTIVE	(1 << 0)
> > +#define GEN8_CTX_STATUS_PREEMPTED	(1 << 1)
> > +#define GEN8_CTX_STATUS_ELEMENT_SWITCH	(1 << 2)
> > +#define GEN8_CTX_STATUS_ACTIVE_IDLE	(1 << 3)
> > +#define GEN8_CTX_STATUS_COMPLETE	(1 << 4)
> > +#define GEN8_CTX_STATUS_LITE_RESTORE	(1 << 15)
> > +
> > +	for (i = 1; i <= 6; ++i) {
> > +		int n = (ring->execlist_csb_write_pointer + i) % 6;
> > +		u32 csb = ring->execlist_csb[n];
> > +		err_printf(m, "  EXECLIST_CTX/CSB[%d]:  0x%08x  0x%08x  ",
> > +			n, ring->execlist_ctx[n], csb);
> > +		err_printf(m, "%s %s %s %s %s %s\n",
> > +			csb & GEN8_CTX_STATUS_IDLE_ACTIVE	? "I->A" : "    ",
> > +			csb & GEN8_CTX_STATUS_PREEMPTED		? "PRMT" : "    ",
> > +			csb & GEN8_CTX_STATUS_ELEMENT_SWITCH	? "ELSW" : "    ",
> > +			csb & GEN8_CTX_STATUS_ACTIVE_IDLE	? "A->I" : "    ",
> > +			csb & GEN8_CTX_STATUS_COMPLETE		? "DONE" : "    ",
> > +			csb & GEN8_CTX_STATUS_LITE_RESTORE	? "LITE" : "    ");
> >   	}
> >   }
> >
> > @@ -468,10 +484,13 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
> >   				   dev_priv->ring[i].name,
> >   				   error->ring[i].num_requests);
> >   			for (j = 0; j < error->ring[i].num_requests; j++) {
> > -				err_printf(m, "  seqno 0x%08x, emitted %ld, tail 0x%08x\n",
> > -					   error->ring[i].requests[j].seqno,
> > -					   error->ring[i].requests[j].jiffies,
> > -					   error->ring[i].requests[j].tail);
> > +				struct drm_i915_error_request *erq;
> > +				erq = &error->ring[i].requests[j];
> > +				err_printf(m, "  seqno 0x%08x, tail 0x%08x, "
> > +					"emitted %ld, ctx_desc 0x%08x_%08x\n",
> > +					erq->seqno, erq->tail, erq->jiffies,
> > +					upper_32_bits(erq->ctx_desc),
> > +					lower_32_bits(erq->ctx_desc));
> >   			}
> >   		}
> >
> > @@ -1130,6 +1149,7 @@ static void i915_gem_record_rings(struct drm_device *dev,
> >
> >   		count = 0;
> >   		list_for_each_entry(request, &ring->request_list, list) {
> > +			struct intel_context *ctx = request->ctx;
> >   			struct drm_i915_error_request *erq;
> >
> >   			if (count >= error->ring[i].num_requests) {
> > @@ -1152,8 +1172,9 @@ static void i915_gem_record_rings(struct drm_device *dev,
> >   			}
> >
> >   			erq = &error->ring[i].requests[count++];
> > -			erq->seqno = request->seqno;
> > +			erq->ctx_desc = intel_lr_context_descriptor(ctx, ring);
> >   			erq->jiffies = request->emitted_jiffies;
> > +			erq->seqno = request->seqno;
> >   			erq->tail = request->postfix;
> >   		}
> >   	}
> > --
> > 1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx