Hi Eugene,

> >     intel_engine_get_hung_entity(engine, &ce, &rq);
> > -   if (rq && !i915_request_started(rq))
> > -           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);
> > -
> >     if (rq) {
> > +           if (!i915_request_started(rq)) {
> 
> why are you breaking the if here?

Just to be clear, this is not a binding comment as you are
merging to "if (rq)". But I prefer the previous style as the line
of this drm_info() is already too long and with one more level of
indentation is even longer.

In any case:

Reviewed-by: Andi Shyti <[email protected]> 

and now I'm really cc'eing John.

Andi

> > +                   u16 guc_id = ce ? ce->guc_id.id : 0;
> 
> good catch!
> 
> Andi
> 
> > +                   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, guc_id);
> > +           }
> >             capture = intel_engine_coredump_add_request(ee, rq, 
> > ATOMIC_MAYFAIL);
> >             i915_request_put(rq);
> >     } else if (ce) {
> > -- 
> > 2.34.1

Reply via email to