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

[Dong, Chuanxiao]

> -----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?

> 
> Does it matter to gvt that we repeat the SCHEDULE_IN after reset (happens
> for execlists as well)?
This should be fine.

Thanks
Chuanxiao

> -Chris
> 
> --
> Chris Wilson, Intel Open Source Technology Centre