| Message ID | 1454007684-16777-4-git-send-email-arun.siluvery@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Arun Siluvery <arun.siluvery@linux.intel.com> writes: > From: Dave Gordon <david.s.gordon@intel.com> > > 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? -Mika > For: VIZ-2021 > Signed-off-by: Dave Gordon <david.s.gordon@intel.com> > --- > 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
On 29/01/2016 08:17, Mika Kuoppala wrote: > Arun Siluvery <arun.siluvery@linux.intel.com> writes: > >> From: Dave Gordon <david.s.gordon@intel.com> >> >> 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@intel.com> >> --- >> 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 >
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; } }