Quoting Chris Wilson (2019-09-23 14:39:20)
> Quoting Tvrtko Ursulin (2019-09-23 14:32:23)
> >
> > On 21/09/2019 10:55, Chris Wilson wrote:
> > > + if (i915_request_completed(rq)) {
> > > + __i915_request_submit(rq);
> > > + ve->request = NULL;
> > > + } else {
> > > + ve->base.execlists.queue_priority_hint = rq_prio(rq);
> > > + ve->request = i915_request_get(rq);
> > > + rq->engine = &ve->base; /* fixup from unwind */
> >
> > The last line has me confused. Isn't this the normal veng rq submission
> > path?
>
> It is also on the normal submission path.
>
> > In which case rq->engine will already be set to veng.
>
> Yes
>
> > But on the
> > unwind path you have removed reset-back of rq->engine to owner. Ah.. the
> > unwind calls veng->submit_request on it, and then we end up in here..
> > Okay, this is outside the normal path, I mean the else block has two
> > functions/paths, and this should be explained in a comment.
>
> That was the intent of "fixup from unwind?"
>
> I can squeeze in /* fixup __unwind_incomplete_requests */ is that more
> clueful? Or do you think it needs more?
Also it doesn't strictly have to be moved. And certainly there's not
good reason why it needs to be done in this patch -- initially I was
moving it to avoid dereferencing incomplete virtual engine state inside
__i915_request_submit(), but that's better handled by the earlier
patches.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
