Hi Michal,
> > + if (rq && !i915_request_started(rq)) {
> > + /*
> > + * We want to know also what is the gcu_id of the context,
>
> typo: guc_id
>
> > + * but if we don't have the context reference, then skip
> > + * printing it.
> > + */
>
> but IMO this comment is redundant as it's quite obvious that without
> context pointer you can't print guc_id member
I recommended to add a comment because there is some code
redundancy that, I think, needs some explanation; someone might
not spot immediately the need for ce, unless goes a the end of
the drm_info parameter's list.
> > + if (ce)
> > + drm_info(&engine->gt->i915->drm,
> > + "Got hung context on %s with active request
> > %lld:%lld [0x%04X] not yet started\n",
> > + engine->name, rq->fence.context,
> > rq->fence.seqno, ce->guc_id.id);
> > + else
> > + drm_info(&engine->gt->i915->drm,
> > + "Got hung context on %s with active request
> > %lld:%lld not yet started\n",
> > + engine->name, rq->fence.context,
> > rq->fence.seqno);
>
> since you are touching drm_info() where we use engine->gt then maybe
> it's good time to switch to gt_info() to get better per-GT message?
I think the original reason for using drm_info is because we are
outside the GT. But, because the engine belongs to the GT, it
makes also sense to use gt_info(), I don't oppose.
It would make more sense to move this function completely into
gt/.
Thanks,
Andi
