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

[Dong, Chuanxiao]

> -----Original Message-----
> From: Joonas Lahtinen [mailto:joonas.lahtinen at linux.intel.com]
> Sent: Thursday, March 23, 2017 5:38 PM
> To: Dong, Chuanxiao; intel-gfx at lists.freedesktop.org
> Cc: intel-gvt-dev at lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH v3] drm/i915/scheduler: add gvt notification
> for guc submission
> 
> On ke, 2017-03-22 at 14:34 +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)
> >
> > Cc: xiao.zheng at intel.com
> > Cc: kevin.tian at intel.com
> > Signed-off-by: Chuanxiao Dong <chuanxiao.dong at intel.com>
> 
> <SNIP>
> 
> > @@ -350,7 +335,7 @@ static void execlists_submit_ports(struct
> > intel_engine_cs *engine)
> >
> >  	GEM_BUG_ON(port[0].count > 1);
> >  	if (!port[0].count)
> > -		execlists_context_status_change(port[0].request,
> > +		context_status_change(port[0].request,
> >
> 	INTEL_CONTEXT_SCHEDULE_IN);
> 
> Fix indent.
> 
> >  	desc[0] = execlists_update_context(port[0].request);
> >  	GEM_DEBUG_EXEC(port[0].context_id = upper_32_bits(desc[0]));
> @@
> > -358,7 +343,7 @@ static void execlists_submit_ports(struct
> > intel_engine_cs *engine)
> >
> >  	if (port[1].request) {
> >  		GEM_BUG_ON(port[1].count);
> > -		execlists_context_status_change(port[1].request,
> > +		context_status_change(port[1].request,
> >
> 	INTEL_CONTEXT_SCHEDULE_IN);
> 
> Ditto.
> 
> > @@ -581,7 +566,7 @@ static void intel_lrc_irq_handler(unsigned long data)
> >  			if (--port[0].count == 0) {
> >  				GEM_BUG_ON(status &
> GEN8_CTX_STATUS_PREEMPTED);
> >
> 	GEM_BUG_ON(!i915_gem_request_completed(port[0].request));
> > -
> 	execlists_context_status_change(port[0].request,
> > +				context_status_change(port[0].request,
> >
> 	INTEL_CONTEXT_SCHEDULE_OUT);
> 
> Ditto.
> 
> > @@ -87,5 +87,18 @@ 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
> > +context_status_change(struct drm_i915_gem_request *rq, unsigned long
> > +status)
> 
> This needs intel_lr_ prefix now that it's in .h file.
> 
> With those changes (make sure context_status_change doesn't become
> over character 80 line), this is;

Sure. Will fix in the next version.

Thanks
Chuanxiao

> 
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> 
> Regards, Joonas
> --
> Joonas Lahtinen
> Open Source Technology Center
> Intel Corporation