* Re: [PATCH] drm/i915/scheduler: add gvt notification for guc submission
2017-03-20 1:48 [PATCH] drm/i915/scheduler: add gvt notification for guc submission Chuanxiao Dong
@ 2017-03-20 2:46 ` Zheng, Xiao
2017-03-20 3:19 ` Dong, Chuanxiao
2017-03-21 2:38 ` [PATCH v2] " Chuanxiao Dong
` (5 subsequent siblings)
6 siblings, 1 reply; 27+ messages in thread
From: Zheng, Xiao @ 2017-03-20 2:46 UTC (permalink / raw)
To: Dong, Chuanxiao, intel-gfx; +Cc: intel-gvt-dev
It may consider to change the function name: execlists_context_status_change to context_status_change_notify () instead. Otherwise confusing GUC submission path.
Thanks.
> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf
> Of Chuanxiao Dong
> Sent: Monday, March 20, 2017 9:49 AM
> To: intel-gfx@lists.freedesktop.org
> Cc: intel-gvt-dev@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@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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH] drm/i915/scheduler: add gvt notification for guc submission
2017-03-20 2:46 ` Zheng, Xiao
@ 2017-03-20 3:19 ` Dong, Chuanxiao
2017-03-21 1:38 ` Zheng, Xiao
0 siblings, 1 reply; 27+ messages in thread
From: Dong, Chuanxiao @ 2017-03-20 3:19 UTC (permalink / raw)
To: Zheng, Xiao, intel-gfx; +Cc: intel-gvt-dev
> -----Original Message-----
> From: Zheng, Xiao
> Sent: Monday, March 20, 2017 10:46 AM
> To: Dong, Chuanxiao <chuanxiao.dong@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: intel-gvt-dev@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@lists.freedesktop.org] On
> > Behalf Of Chuanxiao Dong
> > Sent: Monday, March 20, 2017 9:49 AM
> > To: intel-gfx@lists.freedesktop.org
> > Cc: intel-gvt-dev@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@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@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH] drm/i915/scheduler: add gvt notification for guc submission
2017-03-20 3:19 ` Dong, Chuanxiao
@ 2017-03-21 1:38 ` Zheng, Xiao
2017-03-21 1:58 ` Dong, Chuanxiao
0 siblings, 1 reply; 27+ messages in thread
From: Zheng, Xiao @ 2017-03-21 1:38 UTC (permalink / raw)
To: Dong, Chuanxiao, intel-gfx; +Cc: intel-gvt-dev
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.
> -----Original Message-----
> From: Dong, Chuanxiao
> Sent: Monday, March 20, 2017 11:20 AM
> To: Zheng, Xiao <xiao.zheng@intel.com>; intel-gfx@lists.freedesktop.org
> Cc: intel-gvt-dev@lists.freedesktop.org
> 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 <chuanxiao.dong@intel.com>; intel-
> > gfx@lists.freedesktop.org
> > Cc: intel-gvt-dev@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@lists.freedesktop.org] On
> > > Behalf Of Chuanxiao Dong
> > > Sent: Monday, March 20, 2017 9:49 AM
> > > To: intel-gfx@lists.freedesktop.org
> > > Cc: intel-gvt-dev@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@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@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH] drm/i915/scheduler: add gvt notification for guc submission
2017-03-21 1:38 ` Zheng, Xiao
@ 2017-03-21 1:58 ` Dong, Chuanxiao
0 siblings, 0 replies; 27+ messages in thread
From: Dong, Chuanxiao @ 2017-03-21 1:58 UTC (permalink / raw)
To: Zheng, Xiao, intel-gfx; +Cc: intel-gvt-dev
> -----Original Message-----
> From: Zheng, Xiao
> Sent: Tuesday, March 21, 2017 9:39 AM
> To: Dong, Chuanxiao <chuanxiao.dong@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: intel-gvt-dev@lists.freedesktop.org
> 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 <xiao.zheng@intel.com>;
> > intel-gfx@lists.freedesktop.org
> > Cc: intel-gvt-dev@lists.freedesktop.org
> > 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 <chuanxiao.dong@intel.com>; intel-
> > > gfx@lists.freedesktop.org
> > > Cc: intel-gvt-dev@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@lists.freedesktop.org]
> > > > On Behalf Of Chuanxiao Dong
> > > > Sent: Monday, March 20, 2017 9:49 AM
> > > > To: intel-gfx@lists.freedesktop.org
> > > > Cc: intel-gvt-dev@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@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@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v2] drm/i915/scheduler: add gvt notification for guc submission
2017-03-20 1:48 [PATCH] drm/i915/scheduler: add gvt notification for guc submission Chuanxiao Dong
2017-03-20 2:46 ` Zheng, Xiao
@ 2017-03-21 2:38 ` Chuanxiao Dong
2017-03-21 3:07 ` ✓ Fi.CI.BAT: success for " Patchwork
` (4 subsequent siblings)
6 siblings, 0 replies; 27+ messages in thread
From: Chuanxiao Dong @ 2017-03-21 2:38 UTC (permalink / raw)
To: intel-gfx; +Cc: intel-gvt-dev
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)
Cc: xiao.zheng@intel.com
Signed-off-by: Chuanxiao Dong <chuanxiao.dong@intel.com>
---
drivers/gpu/drm/i915/i915_guc_submission.c | 12 ++++++++++++
drivers/gpu/drm/i915/intel_lrc.c | 21 +++------------------
drivers/gpu/drm/i915/intel_lrc.h | 13 +++++++++++++
3 files changed, 28 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index a3636b3..68c1e70 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
+ */
+ 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,12 @@ 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
+ */
+ 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..3a5ed5c 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)
{
@@ -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);
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);
desc[1] = execlists_update_context(port[1].request);
GEM_DEBUG_EXEC(port[1].context_id = upper_32_bits(desc[1]));
@@ -565,7 +550,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);
trace_i915_gem_request_out(port[0].request);
diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index e8015e7..51e1be9 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -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)
+{
+ /*
+ * 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 27+ messages in thread* ✓ Fi.CI.BAT: success for drm/i915/scheduler: add gvt notification for guc submission
2017-03-20 1:48 [PATCH] drm/i915/scheduler: add gvt notification for guc submission Chuanxiao Dong
2017-03-20 2:46 ` Zheng, Xiao
2017-03-21 2:38 ` [PATCH v2] " Chuanxiao Dong
@ 2017-03-21 3:07 ` Patchwork
2017-03-21 11:23 ` ✗ Fi.CI.BAT: failure " Patchwork
` (3 subsequent siblings)
6 siblings, 0 replies; 27+ messages in thread
From: Patchwork @ 2017-03-21 3:07 UTC (permalink / raw)
To: Chuanxiao Dong; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/scheduler: add gvt notification for guc submission
URL : https://patchwork.freedesktop.org/series/21514/
State : success
== Summary ==
Series 21514v1 drm/i915/scheduler: add gvt notification for guc submission
https://patchwork.freedesktop.org/api/1.0/series/21514/revisions/1/mbox/
Test kms_cursor_legacy:
Subgroup basic-flip-before-cursor-atomic:
incomplete -> SKIP (fi-byt-j1900)
Test kms_flip:
Subgroup basic-flip-vs-wf_vblank:
notrun -> INCOMPLETE (fi-byt-j1900)
notrun -> INCOMPLETE (fi-byt-n2820)
Subgroup basic-plain-flip:
incomplete -> PASS (fi-byt-n2820)
Test kms_pipe_crc_basic:
Subgroup hang-read-crc-pipe-b:
incomplete -> PASS (fi-snb-2520m)
none -> INCOMPLETE (fi-skl-6700hq)
notrun -> INCOMPLETE (fi-bxt-j4205)
Subgroup hang-read-crc-pipe-c:
notrun -> INCOMPLETE (fi-ivb-3520m)
incomplete -> PASS (fi-bsw-n3050)
Subgroup read-crc-pipe-b:
incomplete -> PASS (fi-bxt-j4205)
Subgroup read-crc-pipe-c-frame-sequence:
incomplete -> PASS (fi-ivb-3520m)
Test kms_setmode:
Subgroup basic:
notrun -> INCOMPLETE (fi-bsw-n3050)
Test prime_busy:
Subgroup basic-after-default:
notrun -> INCOMPLETE (fi-snb-2520m)
Test prime_vgem:
Subgroup basic-fence-wait-default:
incomplete -> PASS (fi-skl-6770hq)
fi-bdw-5557u total:300 pass:284 dwarn:0 dfail:0 fail:0 skip:16 time: 621s
fi-bsw-n3050 total:300 pass:219 dwarn:0 dfail:0 fail:0 skip:45 time: 0s
fi-bxt-j4205 total:257 pass:234 dwarn:0 dfail:0 fail:3 skip:19 time: 0s
fi-byt-j1900 total:240 pass:212 dwarn:0 dfail:0 fail:0 skip:27 time: 0s
fi-byt-n2820 total:240 pass:208 dwarn:0 dfail:0 fail:0 skip:31 time: 0s
fi-hsw-4770 total:300 pass:279 dwarn:0 dfail:0 fail:1 skip:20 time: 666s
fi-ilk-650 total:300 pass:240 dwarn:0 dfail:0 fail:2 skip:58 time: 538s
fi-ivb-3520m total:263 pass:238 dwarn:0 dfail:0 fail:5 skip:19 time: 0s
fi-kbl-7500u total:300 pass:275 dwarn:1 dfail:0 fail:4 skip:20 time: 626s
fi-skl-6260u total:300 pass:283 dwarn:1 dfail:0 fail:4 skip:12 time: 647s
fi-skl-6700hq total:257 pass:235 dwarn:1 dfail:0 fail:3 skip:17 time: 0s
fi-skl-6700k total:300 pass:271 dwarn:5 dfail:0 fail:4 skip:20 time: 664s
fi-skl-6770hq total:300 pass:283 dwarn:1 dfail:0 fail:4 skip:12 time: 699s
fi-snb-2520m total:300 pass:231 dwarn:0 dfail:0 fail:6 skip:35 time: 0s
fi-snb-2600 total:300 pass:259 dwarn:0 dfail:0 fail:5 skip:36 time: 602s
fi-hsw-4770r failed to connect after reboot
1e78eaf62e1d2fd47cf6fb974876610a5786d352 drm-tip: 2017y-03m-20d-21h-54m-26s UTC integration manifest
e37ece2 drm/i915/scheduler: add gvt notification for guc submission
== Logs ==
For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4237/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread* ✗ Fi.CI.BAT: failure for drm/i915/scheduler: add gvt notification for guc submission
2017-03-20 1:48 [PATCH] drm/i915/scheduler: add gvt notification for guc submission Chuanxiao Dong
` (2 preceding siblings ...)
2017-03-21 3:07 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2017-03-21 11:23 ` Patchwork
2017-03-22 6:34 ` [PATCH v3] " Chuanxiao Dong
` (2 subsequent siblings)
6 siblings, 0 replies; 27+ messages in thread
From: Patchwork @ 2017-03-21 11:23 UTC (permalink / raw)
To: Chuanxiao Dong; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/scheduler: add gvt notification for guc submission
URL : https://patchwork.freedesktop.org/series/21514/
State : failure
== Summary ==
Series 21514v1 drm/i915/scheduler: add gvt notification for guc submission
https://patchwork.freedesktop.org/api/1.0/series/21514/revisions/1/mbox/
Test gem_exec_flush:
Subgroup basic-batch-kernel-default-uc:
fail -> PASS (fi-snb-2600) fdo#100007
Test kms_pipe_crc_basic:
Subgroup nonblocking-crc-pipe-c-frame-sequence:
pass -> FAIL (fi-skl-6700hq)
fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007
fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time: 468s
fi-bsw-n3050 total:278 pass:239 dwarn:0 dfail:0 fail:0 skip:39 time: 581s
fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time: 535s
fi-bxt-t5700 total:278 pass:258 dwarn:0 dfail:0 fail:0 skip:20 time: 544s
fi-byt-j1900 total:278 pass:251 dwarn:0 dfail:0 fail:0 skip:27 time: 507s
fi-byt-n2820 total:278 pass:247 dwarn:0 dfail:0 fail:0 skip:31 time: 502s
fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 440s
fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 432s
fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time: 437s
fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 514s
fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 496s
fi-kbl-7500u total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 489s
fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 493s
fi-skl-6700hq total:278 pass:260 dwarn:0 dfail:0 fail:1 skip:17 time: 606s
fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time: 482s
fi-skl-6770hq total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 527s
fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time: 545s
fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time: 417s
5cde1df3c7da85cb45f45f9844741643f4d96470 drm-tip: 2017y-03m-21d-10h-23m-47s UTC integration manifest
e749e89 drm/i915/scheduler: add gvt notification for guc submission
== Logs ==
For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4239/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread* [PATCH v3] drm/i915/scheduler: add gvt notification for guc submission
2017-03-20 1:48 [PATCH] drm/i915/scheduler: add gvt notification for guc submission Chuanxiao Dong
` (3 preceding siblings ...)
2017-03-21 11:23 ` ✗ Fi.CI.BAT: failure " Patchwork
@ 2017-03-22 6:34 ` Chuanxiao Dong
2017-03-23 5:29 ` Dong, Chuanxiao
` (2 more replies)
2017-03-24 1:49 ` [PATCH v4] " Chuanxiao Dong
2017-03-27 13:32 ` [PATCH v5] " Chuanxiao Dong
6 siblings, 3 replies; 27+ messages in thread
From: Chuanxiao Dong @ 2017-03-22 6:34 UTC (permalink / raw)
To: intel-gfx; +Cc: intel-gvt-dev
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@intel.com
Cc: kevin.tian@intel.com
Signed-off-by: Chuanxiao Dong <chuanxiao.dong@intel.com>
---
drivers/gpu/drm/i915/i915_guc_submission.c | 3 +++
drivers/gpu/drm/i915/intel_lrc.c | 21 +++------------------
drivers/gpu/drm/i915/intel_lrc.h | 13 +++++++++++++
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 055467a..0195547 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -520,6 +520,8 @@ static void __i915_guc_submit(struct drm_i915_gem_request *rq)
unsigned long flags;
int b_ret;
+ 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);
@@ -634,6 +636,7 @@ 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);
+ 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 eec1e71..24c69b5 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)
{
@@ -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);
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);
desc[1] = execlists_update_context(port[1].request);
GEM_DEBUG_EXEC(port[1].context_id = upper_32_bits(desc[1]));
@@ -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);
trace_i915_gem_request_out(port[0].request);
diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index e8015e7..51e1be9 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -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)
+{
+ /*
+ * 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v3] drm/i915/scheduler: add gvt notification for guc submission
2017-03-22 6:34 ` [PATCH v3] " Chuanxiao Dong
@ 2017-03-23 5:29 ` Dong, Chuanxiao
2017-03-23 7:58 ` Tian, Kevin
2017-03-23 9:37 ` Joonas Lahtinen
2017-03-23 9:51 ` Tvrtko Ursulin
2 siblings, 1 reply; 27+ messages in thread
From: Dong, Chuanxiao @ 2017-03-23 5:29 UTC (permalink / raw)
To: intel-gfx; +Cc: intel-gvt-dev
Ping for review. GVT relies on the notification before a request submitted and after a request completed, just like when using execlist mode submission.
> -----Original Message-----
> From: Dong, Chuanxiao
> Sent: Wednesday, March 22, 2017 2:35 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: intel-gvt-dev@lists.freedesktop.org; Dong, Chuanxiao
> <chuanxiao.dong@intel.com>; Zheng, Xiao <xiao.zheng@intel.com>; Tian,
> Kevin <kevin.tian@intel.com>
> Subject: [PATCH v3] 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.
>
> 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@intel.com
> Cc: kevin.tian@intel.com
> Signed-off-by: Chuanxiao Dong <chuanxiao.dong@intel.com>
> ---
> drivers/gpu/drm/i915/i915_guc_submission.c | 3 +++
> drivers/gpu/drm/i915/intel_lrc.c | 21 +++------------------
> drivers/gpu/drm/i915/intel_lrc.h | 13 +++++++++++++
> 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 055467a..0195547 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -520,6 +520,8 @@ static void __i915_guc_submit(struct
> drm_i915_gem_request *rq)
> unsigned long flags;
> int b_ret;
>
> + 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);
> @@ -634,6 +636,7 @@ 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);
> + 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 eec1e71..24c69b5 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) { @@ -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);
> 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);
> desc[1] = execlists_update_context(port[1].request);
> GEM_DEBUG_EXEC(port[1].context_id =
> upper_32_bits(desc[1])); @@ -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);
>
>
> trace_i915_gem_request_out(port[0].request);
> diff --git a/drivers/gpu/drm/i915/intel_lrc.h
> b/drivers/gpu/drm/i915/intel_lrc.h
> index e8015e7..51e1be9 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.h
> +++ b/drivers/gpu/drm/i915/intel_lrc.h
> @@ -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) {
> + /*
> + * 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v3] drm/i915/scheduler: add gvt notification for guc submission
2017-03-23 5:29 ` Dong, Chuanxiao
@ 2017-03-23 7:58 ` Tian, Kevin
0 siblings, 0 replies; 27+ messages in thread
From: Tian, Kevin @ 2017-03-23 7:58 UTC (permalink / raw)
To: Dong, Chuanxiao, intel-gfx; +Cc: intel-gvt-dev
> From: Dong, Chuanxiao
> Sent: Thursday, March 23, 2017 1:29 PM
>
> Ping for review. GVT relies on the notification before a request submitted
> and after a request completed, just like when using execlist mode submission.
>
> > -----Original Message-----
> > From: Dong, Chuanxiao
> > Sent: Wednesday, March 22, 2017 2:35 PM
> > To: intel-gfx@lists.freedesktop.org
> > Cc: intel-gvt-dev@lists.freedesktop.org; Dong, Chuanxiao
> > <chuanxiao.dong@intel.com>; Zheng, Xiao <xiao.zheng@intel.com>; Tian,
> > Kevin <kevin.tian@intel.com>
> > Subject: [PATCH v3] 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.
> >
> > 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@intel.com
> > Cc: kevin.tian@intel.com
> > Signed-off-by: Chuanxiao Dong <chuanxiao.dong@intel.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_guc_submission.c | 3 +++
> > drivers/gpu/drm/i915/intel_lrc.c | 21 +++------------------
> > drivers/gpu/drm/i915/intel_lrc.h | 13 +++++++++++++
> > 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 055467a..0195547 100644
> > --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> > +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> > @@ -520,6 +520,8 @@ static void __i915_guc_submit(struct
> > drm_i915_gem_request *rq)
> > unsigned long flags;
> > int b_ret;
> >
> > + 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);
> > @@ -634,6 +636,7 @@ 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);
> > + 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 eec1e71..24c69b5 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) { @@ -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);
> > 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);
> > desc[1] = execlists_update_context(port[1].request);
> > GEM_DEBUG_EXEC(port[1].context_id =
> upper_32_bits(desc[1])); @@
> > -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);
> >
> >
> > trace_i915_gem_request_out(port[0].request);
> > diff --git a/drivers/gpu/drm/i915/intel_lrc.h
> > b/drivers/gpu/drm/i915/intel_lrc.h
> > index e8015e7..51e1be9 100644
> > --- a/drivers/gpu/drm/i915/intel_lrc.h
> > +++ b/drivers/gpu/drm/i915/intel_lrc.h
> > @@ -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) {
> > + /*
> > + * 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v3] drm/i915/scheduler: add gvt notification for guc submission
2017-03-22 6:34 ` [PATCH v3] " Chuanxiao Dong
2017-03-23 5:29 ` Dong, Chuanxiao
@ 2017-03-23 9:37 ` Joonas Lahtinen
2017-03-23 9:42 ` Chris Wilson
2017-03-23 13:43 ` Dong, Chuanxiao
2017-03-23 9:51 ` Tvrtko Ursulin
2 siblings, 2 replies; 27+ messages in thread
From: Joonas Lahtinen @ 2017-03-23 9:37 UTC (permalink / raw)
To: Chuanxiao Dong, intel-gfx; +Cc: intel-gvt-dev
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@intel.com
> Cc: kevin.tian@intel.com
> Signed-off-by: Chuanxiao Dong <chuanxiao.dong@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;
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v3] drm/i915/scheduler: add gvt notification for guc submission
2017-03-23 9:37 ` Joonas Lahtinen
@ 2017-03-23 9:42 ` Chris Wilson
2017-03-23 13:43 ` Dong, Chuanxiao
2017-03-23 13:43 ` Dong, Chuanxiao
1 sibling, 1 reply; 27+ messages in thread
From: Chris Wilson @ 2017-03-23 9:42 UTC (permalink / raw)
To: Joonas Lahtinen; +Cc: intel-gfx, intel-gvt-dev
On Thu, Mar 23, 2017 at 11:37:32AM +0200, Joonas Lahtinen wrote:
> On ke, 2017-03-22 at 14:34 +0800, Chuanxiao Dong wrote:
> > @@ -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.
But not in this header. The event is not strictly tied to execlists, it
consumes request for starters - but on the other hand it is gvt specific.
I'm tempted to say intel_gvt_notify_context_status().
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v3] drm/i915/scheduler: add gvt notification for guc submission
2017-03-23 9:42 ` Chris Wilson
@ 2017-03-23 13:43 ` Dong, Chuanxiao
0 siblings, 0 replies; 27+ messages in thread
From: Dong, Chuanxiao @ 2017-03-23 13:43 UTC (permalink / raw)
To: Chris Wilson, Joonas Lahtinen; +Cc: intel-gfx, intel-gvt-dev
> -----Original Message-----
> From: intel-gvt-dev [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On
> Behalf Of Chris Wilson
> Sent: Thursday, March 23, 2017 5:43 PM
> To: Joonas Lahtinen
> Cc: intel-gfx@lists.freedesktop.org; intel-gvt-dev@lists.freedesktop.org;
> Dong, Chuanxiao
> Subject: Re: [Intel-gfx] [PATCH v3] drm/i915/scheduler: add gvt notification
> for guc submission
>
> On Thu, Mar 23, 2017 at 11:37:32AM +0200, Joonas Lahtinen wrote:
> > On ke, 2017-03-22 at 14:34 +0800, Chuanxiao Dong wrote:
> > > @@ -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.
>
> But not in this header. The event is not strictly tied to execlists, it consumes
> request for starters - but on the other hand it is gvt specific.
>
> I'm tempted to say intel_gvt_notify_context_status().
It makes sense. Will change the name in the next version.
Thanks
Chuanxiao
> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre
> _______________________________________________
> intel-gvt-dev mailing list
> intel-gvt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v3] drm/i915/scheduler: add gvt notification for guc submission
2017-03-23 9:37 ` Joonas Lahtinen
2017-03-23 9:42 ` Chris Wilson
@ 2017-03-23 13:43 ` Dong, Chuanxiao
1 sibling, 0 replies; 27+ messages in thread
From: Dong, Chuanxiao @ 2017-03-23 13:43 UTC (permalink / raw)
To: Joonas Lahtinen, intel-gfx; +Cc: intel-gvt-dev
> -----Original Message-----
> From: Joonas Lahtinen [mailto:joonas.lahtinen@linux.intel.com]
> Sent: Thursday, March 23, 2017 5:38 PM
> To: Dong, Chuanxiao; intel-gfx@lists.freedesktop.org
> Cc: intel-gvt-dev@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@intel.com
> > Cc: kevin.tian@intel.com
> > Signed-off-by: Chuanxiao Dong <chuanxiao.dong@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@linux.intel.com>
>
> Regards, Joonas
> --
> Joonas Lahtinen
> Open Source Technology Center
> Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v3] drm/i915/scheduler: add gvt notification for guc submission
2017-03-22 6:34 ` [PATCH v3] " Chuanxiao Dong
2017-03-23 5:29 ` Dong, Chuanxiao
2017-03-23 9:37 ` Joonas Lahtinen
@ 2017-03-23 9:51 ` Tvrtko Ursulin
2017-03-23 13:51 ` Dong, Chuanxiao
2 siblings, 1 reply; 27+ messages in thread
From: Tvrtko Ursulin @ 2017-03-23 9:51 UTC (permalink / raw)
To: Chuanxiao Dong, intel-gfx; +Cc: intel-gvt-dev
On 22/03/2017 06:34, 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@intel.com
> Cc: kevin.tian@intel.com
> Signed-off-by: Chuanxiao Dong <chuanxiao.dong@intel.com>
> ---
> drivers/gpu/drm/i915/i915_guc_submission.c | 3 +++
> drivers/gpu/drm/i915/intel_lrc.c | 21 +++------------------
> drivers/gpu/drm/i915/intel_lrc.h | 13 +++++++++++++
> 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 055467a..0195547 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -520,6 +520,8 @@ static void __i915_guc_submit(struct drm_i915_gem_request *rq)
> unsigned long flags;
> int b_ret;
>
> + 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);
> @@ -634,6 +636,7 @@ 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);
> + context_status_change(rq, INTEL_CONTEXT_SCHEDULE_OUT);
Does GVT care that the context will still be active on the GPU for a
small window after this notification? (User interrupt happens before
context complete, which GuC hides from the driver.)
Regards,
Tvrtko
> 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 eec1e71..24c69b5 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)
> {
> @@ -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);
> 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);
> desc[1] = execlists_update_context(port[1].request);
> GEM_DEBUG_EXEC(port[1].context_id = upper_32_bits(desc[1]));
> @@ -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);
>
> trace_i915_gem_request_out(port[0].request);
> diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
> index e8015e7..51e1be9 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.h
> +++ b/drivers/gpu/drm/i915/intel_lrc.h
> @@ -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)
> +{
> + /*
> + * 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_ */
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v3] drm/i915/scheduler: add gvt notification for guc submission
2017-03-23 9:51 ` Tvrtko Ursulin
@ 2017-03-23 13:51 ` Dong, Chuanxiao
0 siblings, 0 replies; 27+ messages in thread
From: Dong, Chuanxiao @ 2017-03-23 13:51 UTC (permalink / raw)
To: Tvrtko Ursulin, intel-gfx; +Cc: intel-gvt-dev
> -----Original Message-----
> From: Tvrtko Ursulin [mailto:tvrtko.ursulin@linux.intel.com]
> Sent: Thursday, March 23, 2017 5:52 PM
> To: Dong, Chuanxiao; intel-gfx@lists.freedesktop.org
> Cc: intel-gvt-dev@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH v3] drm/i915/scheduler: add gvt notification
> for guc submission
>
>
> On 22/03/2017 06:34, 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@intel.com
> > Cc: kevin.tian@intel.com
> > Signed-off-by: Chuanxiao Dong <chuanxiao.dong@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_guc_submission.c | 3 +++
> > drivers/gpu/drm/i915/intel_lrc.c | 21 +++------------------
> > drivers/gpu/drm/i915/intel_lrc.h | 13 +++++++++++++
> > 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 055467a..0195547 100644
> > --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> > +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> > @@ -520,6 +520,8 @@ static void __i915_guc_submit(struct
> drm_i915_gem_request *rq)
> > unsigned long flags;
> > int b_ret;
> >
> > + 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);
> > @@ -634,6 +636,7 @@ 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);
> > + context_status_change(rq,
> INTEL_CONTEXT_SCHEDULE_OUT);
>
> Does GVT care that the context will still be active on the GPU for a small
> window after this notification? (User interrupt happens before context
> complete, which GuC hides from the driver.)
Actually GVT cares. GVT driver will check the context status register to make sure the status is ACTIVE_IDLE in this notification before manually doing the context switch out for the GuC submission case.
Thanks
Chuanxiao
>
> Regards,
>
> Tvrtko
>
> > 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 eec1e71..24c69b5 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) { @@ -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);
> > 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);
> > desc[1] = execlists_update_context(port[1].request);
> > GEM_DEBUG_EXEC(port[1].context_id =
> upper_32_bits(desc[1])); @@
> > -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);
> >
> >
> trace_i915_gem_request_out(port[0].request);
> > diff --git a/drivers/gpu/drm/i915/intel_lrc.h
> > b/drivers/gpu/drm/i915/intel_lrc.h
> > index e8015e7..51e1be9 100644
> > --- a/drivers/gpu/drm/i915/intel_lrc.h
> > +++ b/drivers/gpu/drm/i915/intel_lrc.h
> > @@ -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) {
> > + /*
> > + * 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_ */
> >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v4] drm/i915/scheduler: add gvt notification for guc submission
2017-03-20 1:48 [PATCH] drm/i915/scheduler: add gvt notification for guc submission Chuanxiao Dong
` (4 preceding siblings ...)
2017-03-22 6:34 ` [PATCH v3] " Chuanxiao Dong
@ 2017-03-24 1:49 ` Chuanxiao Dong
2017-03-27 2:35 ` Dong, Chuanxiao
2017-03-27 10:14 ` Joonas Lahtinen
2017-03-27 13:32 ` [PATCH v5] " Chuanxiao Dong
6 siblings, 2 replies; 27+ messages in thread
From: Chuanxiao Dong @ 2017-03-24 1:49 UTC (permalink / raw)
To: intel-gfx; +Cc: intel-gvt-dev
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@intel.com
Cc: kevin.tian@intel.com
Cc: joonas.lahtinen@linux.intel.com
Cc: chris@chris-wilson.co.uk
Signed-off-by: Chuanxiao Dong <chuanxiao.dong@intel.com>
---
drivers/gpu/drm/i915/i915_guc_submission.c | 4 ++++
drivers/gpu/drm/i915/intel_lrc.c | 27 ++++++---------------------
drivers/gpu/drm/i915/intel_lrc.h | 14 ++++++++++++++
3 files changed, 24 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 055467a..91a567d 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -520,6 +520,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);
@@ -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);
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 eec1e71..8b0c937 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)
{
@@ -350,16 +335,16 @@ 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,
- INTEL_CONTEXT_SCHEDULE_IN);
+ intel_gvt_notify_context_status(port[0].request,
+ INTEL_CONTEXT_SCHEDULE_IN);
desc[0] = execlists_update_context(port[0].request);
GEM_DEBUG_EXEC(port[0].context_id = upper_32_bits(desc[0]));
port[0].count++;
if (port[1].request) {
GEM_BUG_ON(port[1].count);
- execlists_context_status_change(port[1].request,
- INTEL_CONTEXT_SCHEDULE_IN);
+ intel_gvt_notify_context_status(port[1].request,
+ INTEL_CONTEXT_SCHEDULE_IN);
desc[1] = execlists_update_context(port[1].request);
GEM_DEBUG_EXEC(port[1].context_id = upper_32_bits(desc[1]));
port[1].count = 1;
@@ -581,8 +566,8 @@ 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,
- INTEL_CONTEXT_SCHEDULE_OUT);
+ intel_gvt_notify_context_status(port[0].request,
+ INTEL_CONTEXT_SCHEDULE_OUT);
trace_i915_gem_request_out(port[0].request);
i915_gem_request_put(port[0].request);
diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index e8015e7..304e9f6 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
+intel_gvt_notify_context_status(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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v4] drm/i915/scheduler: add gvt notification for guc submission
2017-03-24 1:49 ` [PATCH v4] " Chuanxiao Dong
@ 2017-03-27 2:35 ` Dong, Chuanxiao
2017-03-27 10:14 ` Joonas Lahtinen
1 sibling, 0 replies; 27+ messages in thread
From: Dong, Chuanxiao @ 2017-03-27 2:35 UTC (permalink / raw)
To: intel-gfx; +Cc: intel-gvt-dev
Hello, ping for review v4. It fixed the comments from Joonas and Chris. Would like to know if this patch can be applied in i915 to unblock the GVT in GuC mode. :)
Thanks
Chuanxiao
> -----Original Message-----
> From: Dong, Chuanxiao
> Sent: Friday, March 24, 2017 9:49 AM
> To: intel-gfx@lists.freedesktop.org
> Cc: intel-gvt-dev@lists.freedesktop.org; Dong, Chuanxiao
> <chuanxiao.dong@intel.com>; Zheng, Xiao <xiao.zheng@intel.com>; Tian,
> Kevin <kevin.tian@intel.com>; joonas.lahtinen@linux.intel.com; chris@chris-
> wilson.co.uk
> Subject: [PATCH v4] 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.
>
> 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@intel.com
> Cc: kevin.tian@intel.com
> Cc: joonas.lahtinen@linux.intel.com
> Cc: chris@chris-wilson.co.uk
> Signed-off-by: Chuanxiao Dong <chuanxiao.dong@intel.com>
> ---
> drivers/gpu/drm/i915/i915_guc_submission.c | 4 ++++
> drivers/gpu/drm/i915/intel_lrc.c | 27 ++++++---------------------
> drivers/gpu/drm/i915/intel_lrc.h | 14 ++++++++++++++
> 3 files changed, 24 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c
> b/drivers/gpu/drm/i915/i915_guc_submission.c
> index 055467a..91a567d 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -520,6 +520,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);
> @@ -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);
> 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 eec1e71..8b0c937 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) { @@ -350,16 +335,16 @@ 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,
> -
> INTEL_CONTEXT_SCHEDULE_IN);
> + intel_gvt_notify_context_status(port[0].request,
> + INTEL_CONTEXT_SCHEDULE_IN);
> desc[0] = execlists_update_context(port[0].request);
> GEM_DEBUG_EXEC(port[0].context_id = upper_32_bits(desc[0]));
> port[0].count++;
>
> if (port[1].request) {
> GEM_BUG_ON(port[1].count);
> - execlists_context_status_change(port[1].request,
> -
> INTEL_CONTEXT_SCHEDULE_IN);
> + intel_gvt_notify_context_status(port[1].request,
> + INTEL_CONTEXT_SCHEDULE_IN);
> desc[1] = execlists_update_context(port[1].request);
> GEM_DEBUG_EXEC(port[1].context_id =
> upper_32_bits(desc[1]));
> port[1].count = 1;
> @@ -581,8 +566,8 @@ 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,
> -
> INTEL_CONTEXT_SCHEDULE_OUT);
> +
> intel_gvt_notify_context_status(port[0].request,
> +
> INTEL_CONTEXT_SCHEDULE_OUT);
>
>
> trace_i915_gem_request_out(port[0].request);
> i915_gem_request_put(port[0].request);
> diff --git a/drivers/gpu/drm/i915/intel_lrc.h
> b/drivers/gpu/drm/i915/intel_lrc.h
> index e8015e7..304e9f6 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
> +intel_gvt_notify_context_status(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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v4] drm/i915/scheduler: add gvt notification for guc submission
2017-03-24 1:49 ` [PATCH v4] " Chuanxiao Dong
2017-03-27 2:35 ` Dong, Chuanxiao
@ 2017-03-27 10:14 ` Joonas Lahtinen
2017-03-27 10:58 ` Dong, Chuanxiao
1 sibling, 1 reply; 27+ messages in thread
From: Joonas Lahtinen @ 2017-03-27 10:14 UTC (permalink / raw)
To: Chuanxiao Dong, intel-gfx; +Cc: intel-gvt-dev
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@intel.com
> Cc: kevin.tian@intel.com
> Cc: joonas.lahtinen@linux.intel.com
> Cc: chris@chris-wilson.co.uk
> Signed-off-by: Chuanxiao Dong <chuanxiao.dong@intel.com>
<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.
> @@ -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).
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v4] drm/i915/scheduler: add gvt notification for guc submission
2017-03-27 10:14 ` Joonas Lahtinen
@ 2017-03-27 10:58 ` Dong, Chuanxiao
2017-03-27 11:43 ` Joonas Lahtinen
0 siblings, 1 reply; 27+ messages in thread
From: Dong, Chuanxiao @ 2017-03-27 10:58 UTC (permalink / raw)
To: Joonas Lahtinen, intel-gfx; +Cc: intel-gvt-dev
> -----Original Message-----
> From: Joonas Lahtinen [mailto:joonas.lahtinen@linux.intel.com]
> Sent: Monday, March 27, 2017 6:14 PM
> To: Dong, Chuanxiao <chuanxiao.dong@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: intel-gvt-dev@lists.freedesktop.org; Zheng, Xiao
> <xiao.zheng@intel.com>; Tian, Kevin <kevin.tian@intel.com>; 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@intel.com
> > Cc: kevin.tian@intel.com
> > Cc: joonas.lahtinen@linux.intel.com
> > Cc: chris@chris-wilson.co.uk
> > Signed-off-by: Chuanxiao Dong <chuanxiao.dong@intel.com>
>
> <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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v4] drm/i915/scheduler: add gvt notification for guc submission
2017-03-27 10:58 ` Dong, Chuanxiao
@ 2017-03-27 11:43 ` Joonas Lahtinen
0 siblings, 0 replies; 27+ messages in thread
From: Joonas Lahtinen @ 2017-03-27 11:43 UTC (permalink / raw)
To: Dong, Chuanxiao, intel-gfx; +Cc: intel-gvt-dev
On ma, 2017-03-27 at 10:58 +0000, Dong, Chuanxiao wrote:
> >
> > > @@ -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);
>
My bad, I missed one tab.
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v5] drm/i915/scheduler: add gvt notification for guc submission
2017-03-20 1:48 [PATCH] drm/i915/scheduler: add gvt notification for guc submission Chuanxiao Dong
` (5 preceding siblings ...)
2017-03-24 1:49 ` [PATCH v4] " Chuanxiao Dong
@ 2017-03-27 13:32 ` Chuanxiao Dong
2017-03-27 13:49 ` Chris Wilson
6 siblings, 1 reply; 27+ messages in thread
From: Chuanxiao Dong @ 2017-03-27 13:32 UTC (permalink / raw)
To: intel-gfx; +Cc: intel-gvt-dev
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)
v5: move intel_gvt_notify_context_status to intel_gvt.h (Joonas)
Cc: xiao.zheng@intel.com
Cc: kevin.tian@intel.com
Cc: joonas.lahtinen@linux.intel.com
Cc: chris@chris-wilson.co.uk
Signed-off-by: Chuanxiao Dong <chuanxiao.dong@intel.com>
---
drivers/gpu/drm/i915/i915_guc_submission.c | 4 ++++
drivers/gpu/drm/i915/intel_gvt.h | 13 +++++++++++++
drivers/gpu/drm/i915/intel_lrc.c | 21 +++------------------
3 files changed, 20 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 991e76e..1223169 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);
@@ -720,6 +722,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);
i915_gem_request_put(rq);
port[0].request = port[1].request;
port[1].request = NULL;
diff --git a/drivers/gpu/drm/i915/intel_gvt.h b/drivers/gpu/drm/i915/intel_gvt.h
index 25df2d6..9175018 100644
--- a/drivers/gpu/drm/i915/intel_gvt.h
+++ b/drivers/gpu/drm/i915/intel_gvt.h
@@ -32,6 +32,14 @@ void intel_gvt_cleanup(struct drm_i915_private *dev_priv);
int intel_gvt_init_device(struct drm_i915_private *dev_priv);
void intel_gvt_clean_device(struct drm_i915_private *dev_priv);
int intel_gvt_init_host(void);
+
+static inline void
+intel_gvt_notify_context_status(struct drm_i915_gem_request *rq,
+ unsigned long status)
+{
+ atomic_notifier_call_chain(&rq->engine->context_status_notifier,
+ status, rq);
+}
#else
static inline int intel_gvt_init(struct drm_i915_private *dev_priv)
{
@@ -40,6 +48,11 @@ static inline int intel_gvt_init(struct drm_i915_private *dev_priv)
static inline void intel_gvt_cleanup(struct drm_i915_private *dev_priv)
{
}
+static inline void
+intel_gvt_notify_context_status(struct drm_i915_gem_request *rq,
+ unsigned long status)
+{
+}
#endif
#endif /* _INTEL_GVT_H_ */
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index dd0e9d587..8708515 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)
{
@@ -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,
+ intel_gvt_notify_context_status(port[0].request,
INTEL_CONTEXT_SCHEDULE_IN);
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,
+ intel_gvt_notify_context_status(port[1].request,
INTEL_CONTEXT_SCHEDULE_IN);
desc[1] = execlists_update_context(port[1].request);
GEM_DEBUG_EXEC(port[1].context_id = upper_32_bits(desc[1]));
@@ -591,7 +576,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,
+ intel_gvt_notify_context_status(port[0].request,
INTEL_CONTEXT_SCHEDULE_OUT);
trace_i915_gem_request_out(port[0].request);
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v5] drm/i915/scheduler: add gvt notification for guc submission
2017-03-27 13:32 ` [PATCH v5] " Chuanxiao Dong
@ 2017-03-27 13:49 ` Chris Wilson
2017-03-27 14:22 ` Dong, Chuanxiao
0 siblings, 1 reply; 27+ messages in thread
From: Chris Wilson @ 2017-03-27 13:49 UTC (permalink / raw)
To: Chuanxiao Dong; +Cc: intel-gfx, intel-gvt-dev
On Mon, Mar 27, 2017 at 09:32:20PM +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)
> v5: move intel_gvt_notify_context_status to intel_gvt.h (Joonas)
>
> Cc: xiao.zheng@intel.com
> Cc: kevin.tian@intel.com
> Cc: joonas.lahtinen@linux.intel.com
> Cc: chris@chris-wilson.co.uk
> Signed-off-by: Chuanxiao Dong <chuanxiao.dong@intel.com>
> ---
> drivers/gpu/drm/i915/i915_guc_submission.c | 4 ++++
> drivers/gpu/drm/i915/intel_gvt.h | 13 +++++++++++++
> drivers/gpu/drm/i915/intel_lrc.c | 21 +++------------------
> 3 files changed, 20 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> index 991e76e..1223169 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);
So this gets called for every request, rather than at the context switch
boundaries, and we only once signal the SCHEDULE_OUT. Does that matter?
Hmm, shouldn't happen in execlists due to force-single-submission.
Does it matter to gvt that we repeat the SCHEDULE_IN after reset
(happens for execlists as well)?
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v5] drm/i915/scheduler: add gvt notification for guc submission
2017-03-27 13:49 ` Chris Wilson
@ 2017-03-27 14:22 ` Dong, Chuanxiao
2017-03-27 14:33 ` Chris Wilson
0 siblings, 1 reply; 27+ messages in thread
From: Dong, Chuanxiao @ 2017-03-27 14:22 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx, intel-gvt-dev
> -----Original Message-----
> From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
> Sent: Monday, March 27, 2017 9:50 PM
> To: Dong, Chuanxiao
> Cc: intel-gfx@lists.freedesktop.org; intel-gvt-dev@lists.freedesktop.org;
> Zheng, Xiao; Tian, Kevin; joonas.lahtinen@linux.intel.com
> Subject: Re: [PATCH v5] drm/i915/scheduler: add gvt notification for guc
> submission
>
> On Mon, Mar 27, 2017 at 09:32:20PM +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)
> > v5: move intel_gvt_notify_context_status to intel_gvt.h (Joonas)
> >
> > Cc: xiao.zheng@intel.com
> > Cc: kevin.tian@intel.com
> > Cc: joonas.lahtinen@linux.intel.com
> > Cc: chris@chris-wilson.co.uk
> > Signed-off-by: Chuanxiao Dong <chuanxiao.dong@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_guc_submission.c | 4 ++++
> > drivers/gpu/drm/i915/intel_gvt.h | 13 +++++++++++++
> > drivers/gpu/drm/i915/intel_lrc.c | 21 +++------------------
> > 3 files changed, 20 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c
> > b/drivers/gpu/drm/i915/i915_guc_submission.c
> > index 991e76e..1223169 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);
>
> So this gets called for every request, rather than at the context switch
> boundaries, and we only once signal the SCHEDULE_OUT. Does that matter?
Hi Chris, each request submitted by Guc should have a SCHEDULE_OUT in i915_guc_irq_handler to match with this SCHEDULE_IN. Any possible reason for this OUT/IN not mached?
>
> Hmm, shouldn't happen in execlists due to force-single-submission.
GuC should also use force-single-submission for GVT request as execlists does. I have another patch to add this. Do you think if I should combine that patch with this one?
>
> Does it matter to gvt that we repeat the SCHEDULE_IN after reset (happens
> for execlists as well)?
This should be fine.
Thanks
Chuanxiao
> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v5] drm/i915/scheduler: add gvt notification for guc submission
2017-03-27 14:22 ` Dong, Chuanxiao
@ 2017-03-27 14:33 ` Chris Wilson
2017-03-27 14:39 ` Dong, Chuanxiao
0 siblings, 1 reply; 27+ messages in thread
From: Chris Wilson @ 2017-03-27 14:33 UTC (permalink / raw)
To: Dong, Chuanxiao; +Cc: intel-gfx, intel-gvt-dev
On Mon, Mar 27, 2017 at 02:22:10PM +0000, Dong, Chuanxiao wrote:
> > -----Original Message-----
> > From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
> > Sent: Monday, March 27, 2017 9:50 PM
> > To: Dong, Chuanxiao
> > Cc: intel-gfx@lists.freedesktop.org; intel-gvt-dev@lists.freedesktop.org;
> > Zheng, Xiao; Tian, Kevin; joonas.lahtinen@linux.intel.com
> > Subject: Re: [PATCH v5] drm/i915/scheduler: add gvt notification for guc
> > submission
> >
> > On Mon, Mar 27, 2017 at 09:32:20PM +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)
> > > v5: move intel_gvt_notify_context_status to intel_gvt.h (Joonas)
> > >
> > > Cc: xiao.zheng@intel.com
> > > Cc: kevin.tian@intel.com
> > > Cc: joonas.lahtinen@linux.intel.com
> > > Cc: chris@chris-wilson.co.uk
> > > Signed-off-by: Chuanxiao Dong <chuanxiao.dong@intel.com>
> > > ---
> > > drivers/gpu/drm/i915/i915_guc_submission.c | 4 ++++
> > > drivers/gpu/drm/i915/intel_gvt.h | 13 +++++++++++++
> > > drivers/gpu/drm/i915/intel_lrc.c | 21 +++------------------
> > > 3 files changed, 20 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c
> > > b/drivers/gpu/drm/i915/i915_guc_submission.c
> > > index 991e76e..1223169 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);
> >
> > So this gets called for every request, rather than at the context switch
> > boundaries, and we only once signal the SCHEDULE_OUT. Does that matter?
> Hi Chris, each request submitted by Guc should have a SCHEDULE_OUT in i915_guc_irq_handler to match with this SCHEDULE_IN. Any possible reason for this OUT/IN not mached?
>
> >
> > Hmm, shouldn't happen in execlists due to force-single-submission.
> GuC should also use force-single-submission for GVT request as execlists does. I have another patch to add this. Do you think if I should combine that patch with this one?
Currently, each request is emitting SCHEDULE_IN, but then being
amalgamated if in the same context as the earlier. Then we only see
SCHEDULE_OUT from the final request in the context batch. It sounds like
you need to apply the force-single-submission patch first, and then the
context status notifier. Please send them in a single series, in the
order they need to be applied.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v5] drm/i915/scheduler: add gvt notification for guc submission
2017-03-27 14:33 ` Chris Wilson
@ 2017-03-27 14:39 ` Dong, Chuanxiao
0 siblings, 0 replies; 27+ messages in thread
From: Dong, Chuanxiao @ 2017-03-27 14:39 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gvt-dev, intel-gfx
> -----Original Message-----
> From: intel-gvt-dev [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On
> Behalf Of Chris Wilson
> Sent: Monday, March 27, 2017 10:33 PM
> To: Dong, Chuanxiao
> Cc: Zheng, Xiao; intel-gfx@lists.freedesktop.org;
> joonas.lahtinen@linux.intel.com; intel-gvt-dev@lists.freedesktop.org; Tian,
> Kevin
> Subject: Re: [PATCH v5] drm/i915/scheduler: add gvt notification for guc
> submission
>
> On Mon, Mar 27, 2017 at 02:22:10PM +0000, Dong, Chuanxiao wrote:
> > > -----Original Message-----
> > > From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
> > > Sent: Monday, March 27, 2017 9:50 PM
> > > To: Dong, Chuanxiao
> > > Cc: intel-gfx@lists.freedesktop.org;
> > > intel-gvt-dev@lists.freedesktop.org;
> > > Zheng, Xiao; Tian, Kevin; joonas.lahtinen@linux.intel.com
> > > Subject: Re: [PATCH v5] drm/i915/scheduler: add gvt notification for
> > > guc submission
> > >
> > > On Mon, Mar 27, 2017 at 09:32:20PM +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)
> > > > v5: move intel_gvt_notify_context_status to intel_gvt.h (Joonas)
> > > >
> > > > Cc: xiao.zheng@intel.com
> > > > Cc: kevin.tian@intel.com
> > > > Cc: joonas.lahtinen@linux.intel.com
> > > > Cc: chris@chris-wilson.co.uk
> > > > Signed-off-by: Chuanxiao Dong <chuanxiao.dong@intel.com>
> > > > ---
> > > > drivers/gpu/drm/i915/i915_guc_submission.c | 4 ++++
> > > > drivers/gpu/drm/i915/intel_gvt.h | 13 +++++++++++++
> > > > drivers/gpu/drm/i915/intel_lrc.c | 21 +++------------------
> > > > 3 files changed, 20 insertions(+), 18 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c
> > > > b/drivers/gpu/drm/i915/i915_guc_submission.c
> > > > index 991e76e..1223169 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);
> > >
> > > So this gets called for every request, rather than at the context
> > > switch boundaries, and we only once signal the SCHEDULE_OUT. Does
> that matter?
> > Hi Chris, each request submitted by Guc should have a SCHEDULE_OUT in
> i915_guc_irq_handler to match with this SCHEDULE_IN. Any possible reason
> for this OUT/IN not mached?
> >
> > >
> > > Hmm, shouldn't happen in execlists due to force-single-submission.
> > GuC should also use force-single-submission for GVT request as execlists
> does. I have another patch to add this. Do you think if I should combine that
> patch with this one?
>
> Currently, each request is emitting SCHEDULE_IN, but then being
> amalgamated if in the same context as the earlier. Then we only see
> SCHEDULE_OUT from the final request in the context batch. It sounds like
> you need to apply the force-single-submission patch first, and then the
> context status notifier. Please send them in a single series, in the order they
> need to be applied.
I see. Thanks Chris. I will drop this one and send out a new serial patches.
Thanks
Chuanxiao
> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre
> _______________________________________________
> intel-gvt-dev mailing list
> intel-gvt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread