[Intel-gfx] [PATCH 3/5] drm/i915: Fixup preempt-to-busy vs resubmission of a virtual request

[Chris Wilson]

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