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

[Dong, Chuanxiao]

> -----Original Message-----
> From: Zheng, Xiao
> Sent: Monday, March 20, 2017 10:46 AM
> To: Dong, Chuanxiao <chuanxiao.dong at intel.com>; intel-
> gfx at lists.freedesktop.org
> Cc: intel-gvt-dev at lists.freedesktop.org
> Subject: RE: [Intel-gfx] [PATCH] drm/i915/scheduler: add gvt notification for
> guc submission
> 
> It may consider to change the function name:
> execlists_context_status_change to context_status_change_notify () instead.
> Otherwise confusing GUC submission path.
> Thanks.

Hi Xiao, I was considering to use the name of context_status_change_notify, but considering the guc submission is actually has an emulation of execlists on top, so back to use the original function name to align with execlists submission, as well as making the code change less. Is this explanation clear enough to use the original function name?

Thanks
Chuanxiao

> 
> > -----Original Message-----
> > From: Intel-gfx [mailto:intel-gfx-bounces at lists.freedesktop.org] On
> > Behalf Of Chuanxiao Dong
> > Sent: Monday, March 20, 2017 9:49 AM
> > To: intel-gfx at lists.freedesktop.org
> > Cc: intel-gvt-dev at lists.freedesktop.org
> > Subject: [Intel-gfx] [PATCH] drm/i915/scheduler: add gvt notification
> > for guc submission
> >
> > 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.
> >
> > Signed-off-by: Chuanxiao Dong <chuanxiao.dong at intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_guc_submission.c | 13 +++++++++++++
> >  drivers/gpu/drm/i915/intel_lrc.c           | 15 ---------------
> >  drivers/gpu/drm/i915/intel_lrc.h           | 14 ++++++++++++++
> >  3 files changed, 27 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c
> > b/drivers/gpu/drm/i915/i915_guc_submission.c
> > index a3636b3..328b11c 100644
> > --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> > +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> > @@ -520,6 +520,12 @@ static void __i915_guc_submit(struct
> > drm_i915_gem_request *rq)
> >  	unsigned long flags;
> >  	int b_ret;
> >
> > +	/* Notify for the context status change schedule in
> > +	 * Currently only GVT care this notification for manually
> > +	 * context switch, like when using execlist mode submission
> > +	 */
> > +	execlists_context_status_change(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);
> > @@ -623,6 +629,13 @@ 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);
> > +			/* Notify for the context status change schedule
> > +			 * out. Currently only GVT care this notification
> > +			 * for manually context switch, like when using
> > +			 * execlist mode submission
> > +			 */
> > +			execlists_context_status_change(rq,
> > +
> > 	INTEL_CONTEXT_SCHEDULE_OUT);
> >  			i915_gem_request_put(rq);
> >  			port[0].request = port[1].request;
> >  			port[1].request = NULL;
> > diff --git a/drivers/gpu/drm/i915/intel_lrc.c
> > b/drivers/gpu/drm/i915/intel_lrc.c
> > index becde55..4f5906b 100644
> > --- a/drivers/gpu/drm/i915/intel_lrc.c
> > +++ b/drivers/gpu/drm/i915/intel_lrc.c
> > @@ -295,21 +295,6 @@ uint64_t intel_lr_context_descriptor(struct
> > i915_gem_context *ctx,
> >  	return ctx->engine[engine->id].lrc_desc;  }
> >
> > -static inline void
> > -execlists_context_status_change(struct drm_i915_gem_request *rq,
> > -				unsigned long status)
> > -{
> > -	/*
> > -	 * Only used when GVT-g is enabled now. When GVT-g is disabled,
> > -	 * The compiler should eliminate this function as dead-code.
> > -	 */
> > -	if (!IS_ENABLED(CONFIG_DRM_I915_GVT))
> > -		return;
> > -
> > -	atomic_notifier_call_chain(&rq->engine->context_status_notifier,
> > -				   status, rq);
> > -}
> > -
> >  static void
> >  execlists_update_context_pdps(struct i915_hw_ppgtt *ppgtt, u32
> > *reg_state)  { diff --git a/drivers/gpu/drm/i915/intel_lrc.h
> > b/drivers/gpu/drm/i915/intel_lrc.h
> > index e8015e7..d3aa108 100644
> > --- a/drivers/gpu/drm/i915/intel_lrc.h
> > +++ b/drivers/gpu/drm/i915/intel_lrc.h
> > @@ -87,5 +87,19 @@ uint64_t intel_lr_context_descriptor(struct
> > i915_gem_context *ctx,
> >  /* Execlists */
> >  int intel_sanitize_enable_execlists(struct drm_i915_private *dev_priv,
> >  				    int enable_execlists);
> > +static inline void
> > +execlists_context_status_change(struct drm_i915_gem_request *rq,
> > +				unsigned long status)
> > +{
> > +	/*
> > +	 * Only used when GVT-g is enabled now. When GVT-g is disabled,
> > +	 * The compiler should eliminate this function as dead-code.
> > +	 */
> > +	if (!IS_ENABLED(CONFIG_DRM_I915_GVT))
> > +		return;
> > +
> > +	atomic_notifier_call_chain(&rq->engine->context_status_notifier,
> > +				   status, rq);
> > +}
> >
> >  #endif /* _INTEL_LRC_H_ */
> > --
> > 2.7.4
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx