[Intel-gfx] [PATCH v5] drm/i915/scheduler: add gvt notification for guc submission

[Chris Wilson]

On Mon, Mar 27, 2017 at 02:22:10PM +0000, Dong, Chuanxiao wrote:
> > -----Original Message-----
> > From: Chris Wilson [mailto:chris at chris-wilson.co.uk]
> > Sent: Monday, March 27, 2017 9:50 PM
> > To: Dong, Chuanxiao
> > Cc: intel-gfx at lists.freedesktop.org; intel-gvt-dev at lists.freedesktop.org;
> > Zheng, Xiao; Tian, Kevin; joonas.lahtinen at linux.intel.com
> > Subject: Re: [PATCH v5] drm/i915/scheduler: add gvt notification for guc
> > submission
> > 
> > On Mon, Mar 27, 2017 at 09:32:20PM +0800, Chuanxiao Dong wrote:
> > > GVT request needs a manual mmio load/restore. Before GuC submit a
> > > request, send notification to gvt for mmio loading. And after the GuC
> > > finished this GVT request, notify gvt again for mmio restore. This
> > > follows the usage when using execlists submission.
> > >
> > > v2: use context_status_change instead of
> > execlists_context_status_change
> > >     for better understanding (ZhengXiao)
> > > v3: remove the comment as it is obvious and not friendly to
> > >     the caller (Kevin)
> > > v4: fix indent issues (Joonas)
> > >     rename the context_status_change to
> > > intel_gvt_notify_context_status (Chris)
> > > v5: move intel_gvt_notify_context_status to intel_gvt.h (Joonas)
> > >
> > > Cc: xiao.zheng at intel.com
> > > Cc: kevin.tian at intel.com
> > > Cc: joonas.lahtinen at linux.intel.com
> > > Cc: chris at chris-wilson.co.uk
> > > Signed-off-by: Chuanxiao Dong <chuanxiao.dong at intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_guc_submission.c |  4 ++++
> > >  drivers/gpu/drm/i915/intel_gvt.h           | 13 +++++++++++++
> > >  drivers/gpu/drm/i915/intel_lrc.c           | 21 +++------------------
> > >  3 files changed, 20 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c
> > > b/drivers/gpu/drm/i915/i915_guc_submission.c
> > > index 991e76e..1223169 100644
> > > --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> > > +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> > > @@ -606,6 +606,8 @@ static void __i915_guc_submit(struct
> > drm_i915_gem_request *rq)
> > >  	unsigned long flags;
> > >  	int b_ret;
> > >
> > > +	intel_gvt_notify_context_status(rq, INTEL_CONTEXT_SCHEDULE_IN);
> > 
> > So this gets called for every request, rather than at the context switch
> > boundaries, and we only once signal the SCHEDULE_OUT. Does that matter?
> Hi Chris, each request submitted by Guc should have a SCHEDULE_OUT in i915_guc_irq_handler to match with this SCHEDULE_IN. Any possible reason for this OUT/IN not mached?
> 
> > 
> > Hmm, shouldn't happen in execlists due to force-single-submission.
> GuC should also use force-single-submission for GVT request as execlists does. I have another patch to add this. Do you think if I should combine that patch with this one?

Currently, each request is emitting SCHEDULE_IN, but then being
amalgamated if in the same context as the earlier. Then we only see
SCHEDULE_OUT from the final request in the context batch. It sounds like
you need to apply the force-single-submission patch first, and then the
context status notifier. Please send them in a single series, in the
order they need to be applied.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre