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

[Chris Wilson]

On Thu, Apr 06, 2017 at 02:05:15PM +0000, Dong, Chuanxiao wrote:
> 
> 
> > -----Original Message-----
> > From: Chris Wilson [mailto:chris at chris-wilson.co.uk]
> > Sent: Thursday, April 6, 2017 9:32 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 v2 2/2] drm/i915/scheduler: add gvt notification for guc
> > 
> > On Tue, Mar 28, 2017 at 05:38:41PM +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.
> > >
> > > 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           | 12 ++++++++++++
> > >  drivers/gpu/drm/i915/intel_lrc.c           | 21 +++------------------
> > >  3 files changed, 19 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c
> > > b/drivers/gpu/drm/i915/i915_guc_submission.c
> > > index 58087630..d8a5942 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);
> > > +
> > >  	/* WA to flush out the pending GMADR writes to ring buffer. */
> > >  	if (i915_vma_is_map_and_fenceable(rq->ring->vma))
> > >  		POSTING_READ_FW(GUC_STATUS);
> > > @@ -725,6 +727,8 @@ static void i915_guc_irq_handler(unsigned long data)
> > >  		rq = port[0].request;
> > >  		while (rq && i915_gem_request_completed(rq)) {
> > >  			trace_i915_gem_request_out(rq);
> > > +			intel_gvt_notify_context_status(rq,
> > > +					INTEL_CONTEXT_SCHEDULE_OUT);
> > 
> > This is incorrect though. This is no better than just waiting for the request,
> > which is not enough since the idea is that you need to wait for the context
> > image to be completely written to memory before you read it.
> > -Chris
> 
> The wait for the context image to be completely written will be done in the notification from the GVT, by checking the CSB. If put the wait here will made each i915 request to wait, which seems not necessary.

Urm, no. I hope you mean the wait will be on some other thread than
inside this interrupt handler.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre