Re: [PATCH v6] drm/i915: Fix NULL pointer dereference in capture_engine

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