> -----Original Message-----
> From: Zheng, Xiao
> Sent: Tuesday, March 21, 2017 9:39 AM
> To: Dong, Chuanxiao <[email protected]>; intel-
> [email protected]
> Cc: [email protected]
> Subject: RE: [Intel-gfx] [PATCH] drm/i915/scheduler: add gvt notification for
> guc submission
> 
> The new guc submission scheduler is not to emulation Execlist mode in driver.
> You can keep existing execlist_xx function name if insisted it is better.

ok got your point. The new guc submission emulate the execlists submission way, 
indeed it is not execlist mode. Will use a clear name in the next version.

> 
> > -----Original Message-----
> > From: Dong, Chuanxiao
> > Sent: Monday, March 20, 2017 11:20 AM
> > To: Zheng, Xiao <[email protected]>;
> > [email protected]
> > Cc: [email protected]
> > Subject: RE: [Intel-gfx] [PATCH] drm/i915/scheduler: add gvt
> > notification for guc submission
> >
> >
> >
> > > -----Original Message-----
> > > From: Zheng, Xiao
> > > Sent: Monday, March 20, 2017 10:46 AM
> > > To: Dong, Chuanxiao <[email protected]>; intel-
> > > [email protected]
> > > Cc: [email protected]
> > > 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:[email protected]]
> > > > On Behalf Of Chuanxiao Dong
> > > > Sent: Monday, March 20, 2017 9:49 AM
> > > > To: [email protected]
> > > > Cc: [email protected]
> > > > 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 <[email protected]>
> > > > ---
> > > >  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
> > > > [email protected]
> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to