Quoting Tvrtko Ursulin (2019-01-23 13:18:43)
>
> On 23/01/2019 12:54, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2019-01-22 15:50:27)
> >>
> >> On 21/01/2019 22:21, Chris Wilson wrote:
> >>> The global seqno is defunct and so we have no meaningful indicator of
> >>> forward progress for an engine. You need to listen to the request
> >>> signaling tracepoints instead.
> >>>
> >>> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> >>> ---
> >>> drivers/gpu/drm/i915/i915_irq.c | 2 --
> >>> drivers/gpu/drm/i915/i915_trace.h | 25 -------------------------
> >>> 2 files changed, 27 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> >>> index 5fd5080c4ccb..71d11dc2c235 100644
> >>> --- a/drivers/gpu/drm/i915/i915_irq.c
> >>> +++ b/drivers/gpu/drm/i915/i915_irq.c
> >>> @@ -1209,8 +1209,6 @@ static void notify_ring(struct intel_engine_cs *engine)
> >>> wake_up_process(tsk);
> >>>
> >>> rcu_read_unlock();
> >>> -
> >>> - trace_intel_engine_notify(engine, wait);
> >>> }
> >>>
> >>> static void vlv_c0_read(struct drm_i915_private *dev_priv,
> >>> diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
> >>> index 33d90eca9cdd..cb5bc65d575d 100644
> >>> --- a/drivers/gpu/drm/i915/i915_trace.h
> >>> +++ b/drivers/gpu/drm/i915/i915_trace.h
> >>> @@ -750,31 +750,6 @@ trace_i915_request_out(struct i915_request *rq)
> >>> #endif
> >>> #endif
> >>>
> >>> -TRACE_EVENT(intel_engine_notify,
> >>> - TP_PROTO(struct intel_engine_cs *engine, bool waiters),
> >>> - TP_ARGS(engine, waiters),
> >>> -
> >>> - TP_STRUCT__entry(
> >>> - __field(u32, dev)
> >>> - __field(u16, class)
> >>> - __field(u16, instance)
> >>> - __field(u32, seqno)
> >>> - __field(bool, waiters)
> >>> - ),
> >>> -
> >>> - TP_fast_assign(
> >>> - __entry->dev = engine->i915->drm.primary->index;
> >>> - __entry->class = engine->uabi_class;
> >>> - __entry->instance = engine->instance;
> >>> - __entry->seqno = intel_engine_get_seqno(engine);
> >>> - __entry->waiters = waiters;
> >>> - ),
> >>> -
> >>> - TP_printk("dev=%u, engine=%u:%u, seqno=%u, waiters=%u",
> >>> - __entry->dev, __entry->class, __entry->instance,
> >>> - __entry->seqno, __entry->waiters)
> >>> -);
> >>> -
> >>> DEFINE_EVENT(i915_request, i915_request_retire,
> >>> TP_PROTO(struct i915_request *rq),
> >>> TP_ARGS(rq)
> >>>
> >>
> >> I cannot decide if keeping what we can would make it useful. Certainly
> >> not for debugging intel_engine_breadcrumbs_irq.. a sequence of
> >> intel_engine_notify(dev, class, instance) -> dma_fence_signaled would be
> >> a very unreliable trace of what engine actually executed something. What
> >> do you think?
> >
> > All we get is a tracepoint to say an user-interrupt occurred, but nothing to
> > tie it to any request. We are debugging interrupt generation at that
> > point, and I feel a tracepoint ill-suited. We want something geared
> > towards CI instead, so a bunch of selftests... That would be sensible!
>
> We get the engine as well, so could look at sequence of dma fence
> signaling happening following that and imply something, sometimes. Like
> which physical engine executed what. Since the signaling is done
> directly from the interrupt handler and engines are handled in serial
> fashion.
Oh, but fence signaling so rarely happens from an irq handler ;)
Or rather we so often signal fences as we do the next execbuf (or
otherwise) that the irq handler has nothing to do.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
