Re: [PATCH v4] drm/i915/scheduler: add gvt notification for guc submission

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> -----Original Message-----
> From: Joonas Lahtinen [mailto:joonas.lahtinen@xxxxxxxxxxxxxxx]
> Sent: Monday, March 27, 2017 6:14 PM
> To: Dong, Chuanxiao <chuanxiao.dong@xxxxxxxxx>; intel-
> gfx@xxxxxxxxxxxxxxxxxxxxx
> Cc: intel-gvt-dev@xxxxxxxxxxxxxxxxxxxxx; Zheng, Xiao
> <xiao.zheng@xxxxxxxxx>; Tian, Kevin <kevin.tian@xxxxxxxxx>; chris@chris-
> wilson.co.uk
> Subject: Re: [PATCH v4] drm/i915/scheduler: add gvt notification for guc
> submission
> 
> On pe, 2017-03-24 at 09:49 +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)
> >
> > Cc: xiao.zheng@xxxxxxxxx
> > Cc: kevin.tian@xxxxxxxxx
> > Cc: joonas.lahtinen@xxxxxxxxxxxxxxx
> > Cc: chris@xxxxxxxxxxxxxxxxxx
> > Signed-off-by: Chuanxiao Dong <chuanxiao.dong@xxxxxxxxx>
> 
> <SNIP>
> 
> > @@ -634,6 +636,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);
> 
> Code indent is still broken.
Hi Joonas, I am sorry for not getting this code indent issue. The above code is just split by typing an enter due to longer than 80 characters. Are you expecting to see the code like below? The 2nd line will be longer than 80 characters in below case. If this is fine then I will change this in the next version.
			intel_gvt_notify_context_status(rq,
							INTEL_CONTEXT_SCHEDULE_OUT);

> 
> > @@ -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
> > +intel_gvt_notify_context_status(struct drm_i915_gem_request *rq,
> > +				unsigned long status)
> 
> With that prefix, this needs to go to intel_gvt.h, where you can take
> advantage of the existing #ifdef block (which should really be #if
> IS_ENABLED() too).

Sure. Will move to intel_gvt.h in the next version.

Thanks
Chuanxiao

> 
> Regards, Joonas
> --
> Joonas Lahtinen
> Open Source Technology Center
> Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux