On Mon, Mar 13, 2017 at 09:21:58AM +0000, Dong, Chuanxiao wrote: > > > > -----Original Message----- > > From: intel-gvt-dev [mailto:intel-gvt-dev-bounces@xxxxxxxxxxxxxxxxxxxxx] On > > Behalf Of Dong, Chuanxiao > > Sent: Thursday, March 9, 2017 9:28 AM > > To: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>; intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > Cc: intel-gvt-dev@xxxxxxxxxxxxxxxxxxxxx; Wang, Zhi A > > <zhi.a.wang@xxxxxxxxx>; Zhenyu Wang <zhenyuw@xxxxxxxxxxxxxxx> > > Subject: RE: [PATCH] drm/i915/gvt: Remove bogus retry around > > i915_wait_request > > > > > > > > > -----Original Message----- > > > From: Chris Wilson [mailto:chris.ickle.wilson@xxxxxxxxx] On Behalf Of > > > Chris Wilson > > > Sent: Thursday, March 9, 2017 6:08 AM > > > To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > > Cc: intel-gvt-dev@xxxxxxxxxxxxxxxxxxxxx; Chris Wilson <chris@chris- > > > wilson.co.uk>; Dong, Chuanxiao <chuanxiao.dong@xxxxxxxxx>; Zhenyu > > Wang > > > <zhenyuw@xxxxxxxxxxxxxxx>; Wang, Zhi A <zhi.a.wang@xxxxxxxxx> > > > Subject: [PATCH] drm/i915/gvt: Remove bogus retry around > > > i915_wait_request > > > > > > commit 8f1117abb408 ("drm/i915/gvt: handle workload lifecycle > > > properly") includes some nonsense to retry a indefinite wait - > > > i915_wait_request() does not return until the request is completed > > > when used from an uninterruptible context. > > > > > > Fixes: 8f1117abb408 ("drm/i915/gvt: handle workload lifecycle properly" > > > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > > > Cc: Chuanxiao Dong <chuanxiao.dong@xxxxxxxxx> > > > Cc: Zhenyu Wang <zhenyuw@xxxxxxxxxxxxxxx> > > > Cc: Zhi Wang <zhi.a.wang@xxxxxxxxx> > > > --- > > > drivers/gpu/drm/i915/gvt/scheduler.c | 14 +------------- > > > 1 file changed, 1 insertion(+), 13 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c > > > b/drivers/gpu/drm/i915/gvt/scheduler.c > > > index cfd9f7a38c27..8024904c0d58 100644 > > > --- a/drivers/gpu/drm/i915/gvt/scheduler.c > > > +++ b/drivers/gpu/drm/i915/gvt/scheduler.c > > > @@ -465,19 +465,7 @@ static int workload_thread(void *priv) > > > > > > gvt_dbg_sched("ring id %d wait workload %p\n", > > > workload->ring_id, workload); > > > -retry: > > > - i915_wait_request(workload->req, > > > - 0, MAX_SCHEDULE_TIMEOUT); > > > - /* I915 has replay mechanism and a request will be replayed > > > - * if there is i915 reset. So the seqno will be updated anyway. > > > - * If the seqno is not updated yet after waiting, which means > > > - * the replay may still be in progress and we can wait again. > > > - */ > > > - if (!i915_gem_request_completed(workload->req)) { > > > - gvt_dbg_sched("workload %p not completed, wait > > > again\n", > > > - workload); > > > - goto retry; > > > - } > > > + i915_wait_request(workload->req, 0, > > > MAX_SCHEDULE_TIMEOUT); > > > > Thanks Chris for pointing this out. So per the comment, i915_wait_request > > will make sure to return until the request completed, even if a GPU hang > > happened and replay occurred during i915_wait_request(), this function will > > only return after the replay completed, right? > > Hi Chris, ping for the above question. I guess the answer is "yes"? Yes. Given MAX_SCHEDULE_TIMEOUT, no INTERRUPTIBLE flags, it will only return once the request is completed - that is the hw breadcrumb is advanced past the request seqno. That includes waiting across a GPU reset. It is not allowed to return another error. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
