[PATCH 0/2] drm/i915/scheduler: add gvt force-single-submission and notification for guc

[PATCH 0/2] drm/i915/scheduler: add gvt force-single-submission and notification for guc

[Chuanxiao Dong]

GVT requires force-single-submission and notification when i915
using execlist submit, and these should be extended to GuC when
i915 using GuC submit. Below two patches are used to implement this

Chuanxiao Dong (2):
  drm/i915/scheduler: add gvt force-single-submission for guc
  drm/i915/scheduler: add gvt notification for guc

 drivers/gpu/drm/i915/i915_gem_context.h    | 20 +++++++++++++
 drivers/gpu/drm/i915/i915_guc_submission.c | 10 ++++++-
 drivers/gpu/drm/i915/intel_gvt.h           | 13 +++++++++
 drivers/gpu/drm/i915/intel_lrc.c           | 46 +++++-------------------------
 4 files changed, 49 insertions(+), 40 deletions(-)

-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 1/2] drm/i915/scheduler: add gvt force-single-submission for guc

[Chuanxiao Dong]

GVT needs single submission and cannot allow merge. So when GuC submitting
a GVT request, the next one should be submitted to guc later until the
previous one is completed. This is following the usage when using execlist
mode submission.

Cc: chris@chris-wilson.co.uk
Signed-off-by: Chuanxiao Dong <chuanxiao.dong@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_context.h    | 20 ++++++++++++++++++++
 drivers/gpu/drm/i915/i915_guc_submission.c |  6 +++++-
 drivers/gpu/drm/i915/intel_lrc.c           | 25 ++++---------------------
 3 files changed, 29 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_context.h b/drivers/gpu/drm/i915/i915_gem_context.h
index 4af2ab94..025eba2 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.h
+++ b/drivers/gpu/drm/i915/i915_gem_context.h
@@ -246,6 +246,26 @@ static inline bool i915_gem_context_is_kernel(struct i915_gem_context *ctx)
 	return !ctx->file_priv;
 }
 
+static inline bool
+i915_gem_context_single_port_submit(const struct i915_gem_context *ctx)
+{
+	return (IS_ENABLED(CONFIG_DRM_I915_GVT) &&
+		i915_gem_context_force_single_submission(ctx));
+}
+
+static inline bool
+i915_gem_context_can_merge(const struct i915_gem_context *prev,
+		const struct i915_gem_context *next)
+{
+	if (prev != next)
+		return false;
+
+	if (i915_gem_context_single_port_submit(prev))
+		return false;
+
+	return true;
+}
+
 /* i915_gem_context.c */
 int __must_check i915_gem_context_init(struct drm_i915_private *dev_priv);
 void i915_gem_context_lost(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 991e76e..ad90de1 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -680,10 +680,14 @@ static bool i915_guc_dequeue(struct intel_engine_cs *engine)
 		struct drm_i915_gem_request *rq =
 			rb_entry(rb, typeof(*rq), priotree.node);
 
-		if (last && rq->ctx != last->ctx) {
+		if (last && !i915_gem_context_can_merge(last->ctx, rq->ctx)) {
 			if (port != engine->execlist_port)
 				break;
 
+			if (i915_gem_context_single_port_submit(last->ctx) ||
+				i915_gem_context_single_port_submit(rq->ctx))
+				break;
+
 			i915_gem_request_assign(&port->request, last);
 			nested_enable_signaling(last);
 			port++;
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index dd0e9d587..a49801e 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -377,24 +377,6 @@ static void execlists_submit_ports(struct intel_engine_cs *engine)
 	writel(lower_32_bits(desc[0]), elsp);
 }
 
-static bool ctx_single_port_submission(const struct i915_gem_context *ctx)
-{
-	return (IS_ENABLED(CONFIG_DRM_I915_GVT) &&
-		i915_gem_context_force_single_submission(ctx));
-}
-
-static bool can_merge_ctx(const struct i915_gem_context *prev,
-			  const struct i915_gem_context *next)
-{
-	if (prev != next)
-		return false;
-
-	if (ctx_single_port_submission(prev))
-		return false;
-
-	return true;
-}
-
 static void execlists_dequeue(struct intel_engine_cs *engine)
 {
 	struct drm_i915_gem_request *last;
@@ -462,7 +444,8 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
 		 * request, and so we never need to tell the hardware about
 		 * the first.
 		 */
-		if (last && !can_merge_ctx(cursor->ctx, last->ctx)) {
+		if (last &&
+			!i915_gem_context_can_merge(last->ctx, cursor->ctx)) {
 			/* If we are on the second port and cannot combine
 			 * this request with the last, then we are done.
 			 */
@@ -475,8 +458,8 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
 			 * context (even though a different request) to
 			 * the second port.
 			 */
-			if (ctx_single_port_submission(last->ctx) ||
-			    ctx_single_port_submission(cursor->ctx))
+			if (i915_gem_context_single_port_submit(last->ctx) ||
+			    i915_gem_context_single_port_submit(cursor->ctx))
 				break;
 
 			GEM_BUG_ON(last->ctx == cursor->ctx);
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/2] drm/i915/scheduler: add gvt notification for guc

[Chuanxiao Dong]

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.

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 ad90de1..a1e83e6 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);
@@ -724,6 +726,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 a49801e..dd9bd7a 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]));
@@ -574,7 +559,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

✗ Fi.CI.BAT: warning for drm/i915/scheduler: add gvt force-single-submission and notification for guc

[Patchwork]

== Series Details ==

Series: drm/i915/scheduler: add gvt force-single-submission and notification for guc
URL   : https://patchwork.freedesktop.org/series/21972/
State : warning

== Summary ==

Series 21972v1 drm/i915/scheduler: add gvt force-single-submission and notification for guc
https://patchwork.freedesktop.org/api/1.0/series/21972/revisions/1/mbox/

Test gem_exec_suspend:
        Subgroup basic-s4-devices:
                pass       -> DMESG-WARN (fi-kbl-7560u)

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time: 456s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time: 454s
fi-bsw-n3050     total:278  pass:239  dwarn:0   dfail:0   fail:0   skip:39  time: 591s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time: 537s
fi-bxt-t5700     total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  time: 586s
fi-byt-j1900     total:278  pass:251  dwarn:0   dfail:0   fail:0   skip:27  time: 504s
fi-byt-n2820     total:278  pass:247  dwarn:0   dfail:0   fail:0   skip:31  time: 504s
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: 433s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time: 438s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 521s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 493s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 488s
fi-kbl-7560u     total:278  pass:267  dwarn:1   dfail:0   fail:0   skip:10  time: 597s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 483s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time: 599s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time: 494s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 512s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time: 460s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time: 548s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time: 422s

c3ca5620be419c7e20c39d07f6cf7d1cb7c3bfc8 drm-tip: 2017y-03m-27d-20h-31m-03s UTC integration manifest
0270fc9 drm/i915/scheduler: add gvt notification for guc
dcc0b24 drm/i915/scheduler: add gvt force-single-submission for guc

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4320/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915/scheduler: add gvt force-single-submission for guc

[Chris Wilson]

On Tue, Mar 28, 2017 at 09:49:51AM +0800, Chuanxiao Dong wrote:
> GVT needs single submission and cannot allow merge. So when GuC submitting
> a GVT request, the next one should be submitted to guc later until the
> previous one is completed. This is following the usage when using execlist
> mode submission.
> 
> Cc: chris@chris-wilson.co.uk
> Signed-off-by: Chuanxiao Dong <chuanxiao.dong@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_context.h    | 20 ++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_guc_submission.c |  6 +++++-
>  drivers/gpu/drm/i915/intel_lrc.c           | 25 ++++---------------------
>  3 files changed, 29 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.h b/drivers/gpu/drm/i915/i915_gem_context.h
> index 4af2ab94..025eba2 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.h
> +++ b/drivers/gpu/drm/i915/i915_gem_context.h
> @@ -246,6 +246,26 @@ static inline bool i915_gem_context_is_kernel(struct i915_gem_context *ctx)
>  	return !ctx->file_priv;
>  }
>  
> +static inline bool
> +i915_gem_context_single_port_submit(const struct i915_gem_context *ctx)
> +{
> +	return (IS_ENABLED(CONFIG_DRM_I915_GVT) &&
> +		i915_gem_context_force_single_submission(ctx));
> +}
> +
> +static inline bool
> +i915_gem_context_can_merge(const struct i915_gem_context *prev,
> +		const struct i915_gem_context *next)
> +{
> +	if (prev != next)
> +		return false;
> +
> +	if (i915_gem_context_single_port_submit(prev))
> +		return false;
> +
> +	return true;
> +}
> +
>  /* i915_gem_context.c */
>  int __must_check i915_gem_context_init(struct drm_i915_private *dev_priv);
>  void i915_gem_context_lost(struct drm_i915_private *dev_priv);
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> index 991e76e..ad90de1 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -680,10 +680,14 @@ static bool i915_guc_dequeue(struct intel_engine_cs *engine)
>  		struct drm_i915_gem_request *rq =
>  			rb_entry(rb, typeof(*rq), priotree.node);
>  
> -		if (last && rq->ctx != last->ctx) {
> +		if (last && !i915_gem_context_can_merge(last->ctx, rq->ctx)) {
>  			if (port != engine->execlist_port)
>  				break;
>  
> +			if (i915_gem_context_single_port_submit(last->ctx) ||
> +				i915_gem_context_single_port_submit(rq->ctx))
> +				break;
> +
>  			i915_gem_request_assign(&port->request, last);
>  			nested_enable_signaling(last);
>  			port++;
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index dd0e9d587..a49801e 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -377,24 +377,6 @@ static void execlists_submit_ports(struct intel_engine_cs *engine)
>  	writel(lower_32_bits(desc[0]), elsp);
>  }
>  
> -static bool ctx_single_port_submission(const struct i915_gem_context *ctx)
> -{
> -	return (IS_ENABLED(CONFIG_DRM_I915_GVT) &&
> -		i915_gem_context_force_single_submission(ctx));
> -}
> -
> -static bool can_merge_ctx(const struct i915_gem_context *prev,
> -			  const struct i915_gem_context *next)
> -{
> -	if (prev != next)
> -		return false;
> -
> -	if (ctx_single_port_submission(prev))
> -		return false;
> -
> -	return true;
> -}
> -
>  static void execlists_dequeue(struct intel_engine_cs *engine)
>  {
>  	struct drm_i915_gem_request *last;
> @@ -462,7 +444,8 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
>  		 * request, and so we never need to tell the hardware about
>  		 * the first.
>  		 */
> -		if (last && !can_merge_ctx(cursor->ctx, last->ctx)) {
> +		if (last &&
> +			!i915_gem_context_can_merge(last->ctx, cursor->ctx)) {
>  			/* If we are on the second port and cannot combine
>  			 * this request with the last, then we are done.
>  			 */
> @@ -475,8 +458,8 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
>  			 * context (even though a different request) to
>  			 * the second port.
>  			 */
> -			if (ctx_single_port_submission(last->ctx) ||
> -			    ctx_single_port_submission(cursor->ctx))
> +			if (i915_gem_context_single_port_submit(last->ctx) ||
> +			    i915_gem_context_single_port_submit(cursor->ctx))

Could you please fix gvt before this mess is propagated. There is no way
it should be caring about a non-gvt context in port[0] or port[1].
-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

Re: [PATCH 1/2] drm/i915/scheduler: add gvt force-single-submission for guc

[Dong, Chuanxiao]

> -----Original Message-----
> From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
> Sent: Tuesday, March 28, 2017 4:22 PM
> To: Dong, Chuanxiao <chuanxiao.dong@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; intel-gvt-dev@lists.freedesktop.org
> Subject: Re: [PATCH 1/2] drm/i915/scheduler: add gvt force-single-
> submission for guc
> 
> On Tue, Mar 28, 2017 at 09:49:51AM +0800, Chuanxiao Dong wrote:
> > GVT needs single submission and cannot allow merge. So when GuC
> > submitting a GVT request, the next one should be submitted to guc
> > later until the previous one is completed. This is following the usage
> > when using execlist mode submission.
> >
> > Cc: chris@chris-wilson.co.uk
> > Signed-off-by: Chuanxiao Dong <chuanxiao.dong@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_gem_context.h    | 20 ++++++++++++++++++++
> >  drivers/gpu/drm/i915/i915_guc_submission.c |  6 +++++-
> >  drivers/gpu/drm/i915/intel_lrc.c           | 25 ++++---------------------
> >  3 files changed, 29 insertions(+), 22 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem_context.h
> > b/drivers/gpu/drm/i915/i915_gem_context.h
> > index 4af2ab94..025eba2 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_context.h
> > +++ b/drivers/gpu/drm/i915/i915_gem_context.h
> > @@ -246,6 +246,26 @@ static inline bool
> i915_gem_context_is_kernel(struct i915_gem_context *ctx)
> >  	return !ctx->file_priv;
> >  }
> >
> > +static inline bool
> > +i915_gem_context_single_port_submit(const struct i915_gem_context
> > +*ctx) {
> > +	return (IS_ENABLED(CONFIG_DRM_I915_GVT) &&
> > +		i915_gem_context_force_single_submission(ctx));
> > +}
> > +
> > +static inline bool
> > +i915_gem_context_can_merge(const struct i915_gem_context *prev,
> > +		const struct i915_gem_context *next) {
> > +	if (prev != next)
> > +		return false;
> > +
> > +	if (i915_gem_context_single_port_submit(prev))
> > +		return false;
> > +
> > +	return true;
> > +}
> > +
> >  /* i915_gem_context.c */
> >  int __must_check i915_gem_context_init(struct drm_i915_private
> > *dev_priv);  void i915_gem_context_lost(struct drm_i915_private
> > *dev_priv); diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c
> > b/drivers/gpu/drm/i915/i915_guc_submission.c
> > index 991e76e..ad90de1 100644
> > --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> > +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> > @@ -680,10 +680,14 @@ static bool i915_guc_dequeue(struct
> intel_engine_cs *engine)
> >  		struct drm_i915_gem_request *rq =
> >  			rb_entry(rb, typeof(*rq), priotree.node);
> >
> > -		if (last && rq->ctx != last->ctx) {
> > +		if (last && !i915_gem_context_can_merge(last->ctx, rq->ctx))
> {
> >  			if (port != engine->execlist_port)
> >  				break;
> >
> > +			if (i915_gem_context_single_port_submit(last->ctx)
> ||
> > +				i915_gem_context_single_port_submit(rq-
> >ctx))
> > +				break;
> > +
> >  			i915_gem_request_assign(&port->request, last);
> >  			nested_enable_signaling(last);
> >  			port++;
> > diff --git a/drivers/gpu/drm/i915/intel_lrc.c
> > b/drivers/gpu/drm/i915/intel_lrc.c
> > index dd0e9d587..a49801e 100644
> > --- a/drivers/gpu/drm/i915/intel_lrc.c
> > +++ b/drivers/gpu/drm/i915/intel_lrc.c
> > @@ -377,24 +377,6 @@ static void execlists_submit_ports(struct
> intel_engine_cs *engine)
> >  	writel(lower_32_bits(desc[0]), elsp);  }
> >
> > -static bool ctx_single_port_submission(const struct i915_gem_context
> > *ctx) -{
> > -	return (IS_ENABLED(CONFIG_DRM_I915_GVT) &&
> > -		i915_gem_context_force_single_submission(ctx));
> > -}
> > -
> > -static bool can_merge_ctx(const struct i915_gem_context *prev,
> > -			  const struct i915_gem_context *next)
> > -{
> > -	if (prev != next)
> > -		return false;
> > -
> > -	if (ctx_single_port_submission(prev))
> > -		return false;
> > -
> > -	return true;
> > -}
> > -
> >  static void execlists_dequeue(struct intel_engine_cs *engine)  {
> >  	struct drm_i915_gem_request *last;
> > @@ -462,7 +444,8 @@ static void execlists_dequeue(struct
> intel_engine_cs *engine)
> >  		 * request, and so we never need to tell the hardware about
> >  		 * the first.
> >  		 */
> > -		if (last && !can_merge_ctx(cursor->ctx, last->ctx)) {
> > +		if (last &&
> > +			!i915_gem_context_can_merge(last->ctx, cursor-
> >ctx)) {
> >  			/* If we are on the second port and cannot combine
> >  			 * this request with the last, then we are done.
> >  			 */
> > @@ -475,8 +458,8 @@ static void execlists_dequeue(struct
> intel_engine_cs *engine)
> >  			 * context (even though a different request) to
> >  			 * the second port.
> >  			 */
> > -			if (ctx_single_port_submission(last->ctx) ||
> > -			    ctx_single_port_submission(cursor->ctx))
> > +			if (i915_gem_context_single_port_submit(last->ctx)
> ||
> > +			    i915_gem_context_single_port_submit(cursor-
> >ctx))
> 
> Could you please fix gvt before this mess is propagated. There is no way it
> should be caring about a non-gvt context in port[0] or port[1].

Sure. So the name should use prefix intel_gvt_xxxx, and they should be placed in intel_gvt.h?

> -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

[PATCH v2 0/2] drm/i915/scheduler: add gvt force-single-submission and notification for guc

[Chuanxiao Dong]

GVT requires force-single-submission and notification when i915
using execlist submit, and these should be extended to GuC when
i915 using GuC submit. Below two patches are used to implement this

Chuanxiao Dong (2):
  drm/i915/scheduler: add gvt force-single-submission for guc
  drm/i915/scheduler: add gvt notification for guc

 drivers/gpu/drm/i915/i915_guc_submission.c | 11 ++++++-
 drivers/gpu/drm/i915/intel_gvt.h           | 23 +++++++++++++++
 drivers/gpu/drm/i915/intel_lrc.c           | 46 +++++-------------------------
 3 files changed, 40 insertions(+), 40 deletions(-)

-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 1/2] drm/i915/scheduler: add gvt force-single-submission for guc

[Chuanxiao Dong]

GVT needs single submission and cannot allow merge. So when GuC submitting
a GVT request, the next one should be submitted to guc later until the
previous one is completed. This is following the usage when using execlist
mode submission.

v2: make force-single-submission specific to gvt (Chris)

Cc: chris@chris-wilson.co.uk
Signed-off-by: Chuanxiao Dong <chuanxiao.dong@intel.com>
---
 drivers/gpu/drm/i915/i915_guc_submission.c |  7 ++++++-
 drivers/gpu/drm/i915/intel_gvt.h           | 11 +++++++++++
 drivers/gpu/drm/i915/intel_lrc.c           | 25 ++++---------------------
 3 files changed, 21 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 991e76e..58087630 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -680,10 +680,15 @@ static bool i915_guc_dequeue(struct intel_engine_cs *engine)
 		struct drm_i915_gem_request *rq =
 			rb_entry(rb, typeof(*rq), priotree.node);
 
-		if (last && rq->ctx != last->ctx) {
+		if (last && ((last->ctx != rq->ctx) ||
+			intel_gvt_context_single_port_submit(last->ctx))) {
 			if (port != engine->execlist_port)
 				break;
 
+			if (intel_gvt_context_single_port_submit(last->ctx) ||
+				intel_gvt_context_single_port_submit(rq->ctx))
+				break;
+
 			i915_gem_request_assign(&port->request, last);
 			nested_enable_signaling(last);
 			port++;
diff --git a/drivers/gpu/drm/i915/intel_gvt.h b/drivers/gpu/drm/i915/intel_gvt.h
index 25df2d6..c0dcd66 100644
--- a/drivers/gpu/drm/i915/intel_gvt.h
+++ b/drivers/gpu/drm/i915/intel_gvt.h
@@ -32,6 +32,12 @@ 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 bool
+intel_gvt_context_single_port_submit(const struct i915_gem_context *ctx)
+{
+	return i915_gem_context_force_single_submission(ctx);
+}
 #else
 static inline int intel_gvt_init(struct drm_i915_private *dev_priv)
 {
@@ -40,6 +46,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 bool
+intel_gvt_context_single_port_submit(const struct i915_gem_context *ctx)
+{
+	return false;
+}
 #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..951540f 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -377,24 +377,6 @@ static void execlists_submit_ports(struct intel_engine_cs *engine)
 	writel(lower_32_bits(desc[0]), elsp);
 }
 
-static bool ctx_single_port_submission(const struct i915_gem_context *ctx)
-{
-	return (IS_ENABLED(CONFIG_DRM_I915_GVT) &&
-		i915_gem_context_force_single_submission(ctx));
-}
-
-static bool can_merge_ctx(const struct i915_gem_context *prev,
-			  const struct i915_gem_context *next)
-{
-	if (prev != next)
-		return false;
-
-	if (ctx_single_port_submission(prev))
-		return false;
-
-	return true;
-}
-
 static void execlists_dequeue(struct intel_engine_cs *engine)
 {
 	struct drm_i915_gem_request *last;
@@ -462,7 +444,8 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
 		 * request, and so we never need to tell the hardware about
 		 * the first.
 		 */
-		if (last && !can_merge_ctx(cursor->ctx, last->ctx)) {
+		if (last && ((last->ctx != cursor->ctx) ||
+			intel_gvt_context_single_port_submit(last->ctx))) {
 			/* If we are on the second port and cannot combine
 			 * this request with the last, then we are done.
 			 */
@@ -475,8 +458,8 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
 			 * context (even though a different request) to
 			 * the second port.
 			 */
-			if (ctx_single_port_submission(last->ctx) ||
-			    ctx_single_port_submission(cursor->ctx))
+			if (intel_gvt_context_single_port_submit(last->ctx) ||
+			    intel_gvt_context_single_port_submit(cursor->ctx))
 				break;
 
 			GEM_BUG_ON(last->ctx == cursor->ctx);
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 2/2] drm/i915/scheduler: add gvt notification for guc

[Chuanxiao Dong]

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.

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           | 12 ++++++++++++
 drivers/gpu/drm/i915/intel_lrc.c           | 21 +++------------------
 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 58087630..d8a5942 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);
@@ -725,6 +727,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 c0dcd66..813d0f8 100644
--- a/drivers/gpu/drm/i915/intel_gvt.h
+++ b/drivers/gpu/drm/i915/intel_gvt.h
@@ -38,6 +38,13 @@ intel_gvt_context_single_port_submit(const struct i915_gem_context *ctx)
 {
 	return i915_gem_context_force_single_submission(ctx);
 }
+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)
 {
@@ -51,6 +58,11 @@ intel_gvt_context_single_port_submit(const struct i915_gem_context *ctx)
 {
 	return false;
 }
+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 951540f..2333ffb 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]));
@@ -574,7 +559,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

✓ Fi.CI.BAT: success for series starting with [v2,1/2] drm/i915/scheduler: add gvt force-single-submission for guc

[Patchwork]

== Series Details ==

Series: series starting with [v2,1/2] drm/i915/scheduler: add gvt force-single-submission for guc
URL   : https://patchwork.freedesktop.org/series/21998/
State : success

== Summary ==

Series 21998v1 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/21998/revisions/1/mbox/

Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                pass       -> FAIL       (fi-snb-2600) fdo#100007

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: 463s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time: 462s
fi-bsw-n3050     total:278  pass:239  dwarn:0   dfail:0   fail:0   skip:39  time: 590s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time: 539s
fi-bxt-t5700     total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  time: 588s
fi-byt-j1900     total:278  pass:251  dwarn:0   dfail:0   fail:0   skip:27  time: 516s
fi-byt-n2820     total:278  pass:247  dwarn:0   dfail:0   fail:0   skip:31  time: 508s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 436s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 437s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time: 432s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 520s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 500s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 477s
fi-kbl-7560u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 602s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 481s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time: 604s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time: 483s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 518s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time: 460s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time: 545s
fi-snb-2600      total:278  pass:248  dwarn:0   dfail:0   fail:1   skip:29  time: 417s

2b51c2073441cc5b3b18c7e0e8716cbf3736e873 drm-tip: 2017y-03m-28d-08h-54m-35s UTC integration manifest
98e27fb drm/i915/scheduler: add gvt notification for guc
6690a90 drm/i915/scheduler: add gvt force-single-submission for guc

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4324/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 0/2] drm/i915/scheduler: add gvt force-single-submission and notification for guc

[Dong, Chuanxiao]

Hi, ping for review. Would like to know if this serial patches are OK to be taken or I need to drive more comments here :)

Thanks
Chuanxiao

> -----Original Message-----
> From: Dong, Chuanxiao
> Sent: Tuesday, March 28, 2017 5:39 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: intel-gvt-dev@lists.freedesktop.org; Dong, Chuanxiao
> <chuanxiao.dong@intel.com>
> Subject: [PATCH v2 0/2] drm/i915/scheduler: add gvt force-single-submission
> and notification for guc
> 
> GVT requires force-single-submission and notification when i915 using
> execlist submit, and these should be extended to GuC when
> i915 using GuC submit. Below two patches are used to implement this
> 
> Chuanxiao Dong (2):
>   drm/i915/scheduler: add gvt force-single-submission for guc
>   drm/i915/scheduler: add gvt notification for guc
> 
>  drivers/gpu/drm/i915/i915_guc_submission.c | 11 ++++++-
>  drivers/gpu/drm/i915/intel_gvt.h           | 23 +++++++++++++++
>  drivers/gpu/drm/i915/intel_lrc.c           | 46 +++++-------------------------
>  3 files changed, 40 insertions(+), 40 deletions(-)
> 
> --
> 2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 1/2] drm/i915/scheduler: add gvt force-single-submission for guc

[Chris Wilson]

On Tue, Mar 28, 2017 at 05:38:40PM +0800, Chuanxiao Dong wrote:
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index dd0e9d587..951540f 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -377,24 +377,6 @@ static void execlists_submit_ports(struct intel_engine_cs *engine)
>  	writel(lower_32_bits(desc[0]), elsp);
>  }
>  
> -static bool ctx_single_port_submission(const struct i915_gem_context *ctx)
> -{
> -	return (IS_ENABLED(CONFIG_DRM_I915_GVT) &&
> -		i915_gem_context_force_single_submission(ctx));
> -}
> -
> -static bool can_merge_ctx(const struct i915_gem_context *prev,
> -			  const struct i915_gem_context *next)
> -{
> -	if (prev != next)
> -		return false;
> -
> -	if (ctx_single_port_submission(prev))
> -		return false;
> -
> -	return true;
> -}
> -
>  static void execlists_dequeue(struct intel_engine_cs *engine)
>  {
>  	struct drm_i915_gem_request *last;
> @@ -462,7 +444,8 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
>  		 * request, and so we never need to tell the hardware about
>  		 * the first.
>  		 */
> -		if (last && !can_merge_ctx(cursor->ctx, last->ctx)) {
> +		if (last && ((last->ctx != cursor->ctx) ||
> +			intel_gvt_context_single_port_submit(last->ctx))) {

Which is easier to understand the original code or the replacement?
Bonus points for sticking to coding style.
-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

Re: [PATCH v2 1/2] drm/i915/scheduler: add gvt force-single-submission for guc

[Dong, Chuanxiao]

> -----Original Message-----
> From: intel-gvt-dev [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On
> Behalf Of Chris Wilson
> Sent: Friday, March 31, 2017 10:24 PM
> To: Dong, Chuanxiao <chuanxiao.dong@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; intel-gvt-dev@lists.freedesktop.org
> Subject: Re: [PATCH v2 1/2] drm/i915/scheduler: add gvt force-single-
> submission for guc
> 
> On Tue, Mar 28, 2017 at 05:38:40PM +0800, Chuanxiao Dong wrote:
> > diff --git a/drivers/gpu/drm/i915/intel_lrc.c
> > b/drivers/gpu/drm/i915/intel_lrc.c
> > index dd0e9d587..951540f 100644
> > --- a/drivers/gpu/drm/i915/intel_lrc.c
> > +++ b/drivers/gpu/drm/i915/intel_lrc.c
> > @@ -377,24 +377,6 @@ static void execlists_submit_ports(struct
> intel_engine_cs *engine)
> >  	writel(lower_32_bits(desc[0]), elsp);  }
> >
> > -static bool ctx_single_port_submission(const struct i915_gem_context
> > *ctx) -{
> > -	return (IS_ENABLED(CONFIG_DRM_I915_GVT) &&
> > -		i915_gem_context_force_single_submission(ctx));
> > -}
> > -
> > -static bool can_merge_ctx(const struct i915_gem_context *prev,
> > -			  const struct i915_gem_context *next)
> > -{
> > -	if (prev != next)
> > -		return false;
> > -
> > -	if (ctx_single_port_submission(prev))
> > -		return false;
> > -
> > -	return true;
> > -}
> > -
> >  static void execlists_dequeue(struct intel_engine_cs *engine)  {
> >  	struct drm_i915_gem_request *last;
> > @@ -462,7 +444,8 @@ static void execlists_dequeue(struct
> intel_engine_cs *engine)
> >  		 * request, and so we never need to tell the hardware about
> >  		 * the first.
> >  		 */
> > -		if (last && !can_merge_ctx(cursor->ctx, last->ctx)) {
> > +		if (last && ((last->ctx != cursor->ctx) ||
> > +			intel_gvt_context_single_port_submit(last->ctx))) {
> 
> Which is easier to understand the original code or the replacement?
> Bonus points for sticking to coding style.

I was thinking to use the original code but didn't find a good place to define can_merge_ctx(). The function of can_merge_ctx() is normal to all i915 gem context, seems i915_gem_context.h is a good place. But as can_merge_ctx() will call intel_gvt_context_single_port_submit() which is defined in intel_gvt.h, and intel_gvt_context_single_port_submit() will call the function defined in i915_gem_context.h as well, that will build a dependency between i915_gem_context.h and intel_gvt.h which seems not sense. So just use the above replacement to make it simple. Can we use this replacement? Or keep the original function?

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

[PATCH v3 0/2] drm/i915/scheduler: add gvt force-single-submission and notification for guc

[Chuanxiao Dong]

GVT requires force-single-submission and notification when i915
using execlist submit, and these should be extended to GuC when
i915 using GuC submit. Below two patches are used to implement this

Chuanxiao Dong (2):
  drm/i915/scheduler: add gvt force-single-submission for guc
  drm/i915/scheduler: add gvt notification for guc

 drivers/gpu/drm/i915/i915_gem_context.h    | 13 +++++++++
 drivers/gpu/drm/i915/i915_guc_submission.c | 10 ++++++-
 drivers/gpu/drm/i915/intel_gvt.h           | 23 +++++++++++++++
 drivers/gpu/drm/i915/intel_lrc.c           | 46 +++++-------------------------
 4 files changed, 52 insertions(+), 40 deletions(-)

-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v3 1/2] drm/i915/scheduler: add gvt force-single-submission for guc

[Chuanxiao Dong]

GVT needs single submission and cannot allow merge. So when GuC submitting
a GVT request, the next one should be submitted to guc later until the
previous one is completed. This is following the usage when using execlist
mode submission.

v2: make force-single-submission specific to gvt (Chris)
v3: keep the original code implementation (Chris)

Cc: chris@chris-wilson.co.uk
Signed-off-by: Chuanxiao Dong <chuanxiao.dong@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_context.h    | 13 +++++++++++++
 drivers/gpu/drm/i915/i915_guc_submission.c |  6 +++++-
 drivers/gpu/drm/i915/intel_gvt.h           | 11 +++++++++++
 drivers/gpu/drm/i915/intel_lrc.c           | 25 ++++---------------------
 4 files changed, 33 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_context.h b/drivers/gpu/drm/i915/i915_gem_context.h
index 4af2ab94..2c3afec 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.h
+++ b/drivers/gpu/drm/i915/i915_gem_context.h
@@ -246,6 +246,19 @@ static inline bool i915_gem_context_is_kernel(struct i915_gem_context *ctx)
 	return !ctx->file_priv;
 }
 
+static inline bool
+i915_gem_context_can_merge(const struct i915_gem_context *prev,
+		const struct i915_gem_context *next)
+{
+	if (prev != next)
+		return false;
+
+	if (i915_gem_context_force_single_submission(prev))
+		return false;
+
+	return true;
+}
+
 /* i915_gem_context.c */
 int __must_check i915_gem_context_init(struct drm_i915_private *dev_priv);
 void i915_gem_context_lost(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 1642fff..862f4fd 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -668,10 +668,14 @@ static bool i915_guc_dequeue(struct intel_engine_cs *engine)
 		struct drm_i915_gem_request *rq =
 			rb_entry(rb, typeof(*rq), priotree.node);
 
-		if (last && rq->ctx != last->ctx) {
+		if (last && !i915_gem_context_can_merge(last->ctx, rq->ctx)) {
 			if (port != engine->execlist_port)
 				break;
 
+			if (intel_gvt_context_single_port_submit(last->ctx) ||
+				intel_gvt_context_single_port_submit(rq->ctx))
+				break;
+
 			i915_gem_request_assign(&port->request, last);
 			nested_enable_signaling(last);
 			port++;
diff --git a/drivers/gpu/drm/i915/intel_gvt.h b/drivers/gpu/drm/i915/intel_gvt.h
index 25df2d6..c0dcd66 100644
--- a/drivers/gpu/drm/i915/intel_gvt.h
+++ b/drivers/gpu/drm/i915/intel_gvt.h
@@ -32,6 +32,12 @@ 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 bool
+intel_gvt_context_single_port_submit(const struct i915_gem_context *ctx)
+{
+	return i915_gem_context_force_single_submission(ctx);
+}
 #else
 static inline int intel_gvt_init(struct drm_i915_private *dev_priv)
 {
@@ -40,6 +46,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 bool
+intel_gvt_context_single_port_submit(const struct i915_gem_context *ctx)
+{
+	return false;
+}
 #endif
 
 #endif /* _INTEL_GVT_H_ */
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 0dc1cc4..61291e9 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -377,24 +377,6 @@ static void execlists_submit_ports(struct intel_engine_cs *engine)
 	writel(lower_32_bits(desc[0]), elsp);
 }
 
-static bool ctx_single_port_submission(const struct i915_gem_context *ctx)
-{
-	return (IS_ENABLED(CONFIG_DRM_I915_GVT) &&
-		i915_gem_context_force_single_submission(ctx));
-}
-
-static bool can_merge_ctx(const struct i915_gem_context *prev,
-			  const struct i915_gem_context *next)
-{
-	if (prev != next)
-		return false;
-
-	if (ctx_single_port_submission(prev))
-		return false;
-
-	return true;
-}
-
 static void execlists_dequeue(struct intel_engine_cs *engine)
 {
 	struct drm_i915_gem_request *last;
@@ -450,7 +432,8 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
 		 * request, and so we never need to tell the hardware about
 		 * the first.
 		 */
-		if (last && !can_merge_ctx(cursor->ctx, last->ctx)) {
+		if (last &&
+			!i915_gem_context_can_merge(last->ctx, cursor->ctx)) {
 			/* If we are on the second port and cannot combine
 			 * this request with the last, then we are done.
 			 */
@@ -463,8 +446,8 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
 			 * context (even though a different request) to
 			 * the second port.
 			 */
-			if (ctx_single_port_submission(last->ctx) ||
-			    ctx_single_port_submission(cursor->ctx))
+			if (intel_gvt_context_single_port_submit(last->ctx) ||
+			    intel_gvt_context_single_port_submit(cursor->ctx))
 				break;
 
 			GEM_BUG_ON(last->ctx == cursor->ctx);
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v3 2/2] drm/i915/scheduler: add gvt notification for guc

[Chuanxiao Dong]

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.

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           | 12 ++++++++++++
 drivers/gpu/drm/i915/intel_lrc.c           | 21 +++------------------
 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 862f4fd..2f3bb16 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);
@@ -712,6 +714,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 c0dcd66..813d0f8 100644
--- a/drivers/gpu/drm/i915/intel_gvt.h
+++ b/drivers/gpu/drm/i915/intel_gvt.h
@@ -38,6 +38,13 @@ intel_gvt_context_single_port_submit(const struct i915_gem_context *ctx)
 {
 	return i915_gem_context_force_single_submission(ctx);
 }
+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)
 {
@@ -51,6 +58,11 @@ intel_gvt_context_single_port_submit(const struct i915_gem_context *ctx)
 {
 	return false;
 }
+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 61291e9..81f9a3b 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]));
@@ -560,7 +545,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

✓ Fi.CI.BAT: success for series starting with [v3,1/2] drm/i915/scheduler: add gvt force-single-submission for guc

[Patchwork]

== Series Details ==

Series: series starting with [v3,1/2] drm/i915/scheduler: add gvt force-single-submission for guc
URL   : https://patchwork.freedesktop.org/series/22482/
State : success

== Summary ==

Series 22482v1 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/22482/revisions/1/mbox/

Test gem_exec_fence:
        Subgroup await-hang-default:
                pass       -> INCOMPLETE (fi-ilk-650) fdo#100144

fdo#100144 https://bugs.freedesktop.org/show_bug.cgi?id=100144

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time: 428s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time: 423s
fi-bsw-n3050     total:278  pass:239  dwarn:0   dfail:0   fail:0   skip:39  time: 573s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time: 506s
fi-bxt-t5700     total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  time: 552s
fi-byt-j1900     total:278  pass:251  dwarn:0   dfail:0   fail:0   skip:27  time: 480s
fi-byt-n2820     total:278  pass:247  dwarn:0   dfail:0   fail:0   skip:31  time: 479s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 407s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 404s
fi-ilk-650       total:48   pass:27   dwarn:0   dfail:0   fail:0   skip:20  time: 0s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 499s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 464s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 453s
fi-kbl-7560u     total:278  pass:267  dwarn:1   dfail:0   fail:0   skip:10  time: 569s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 451s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time: 572s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 491s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time: 433s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time: 532s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time: 401s
fi-skl-6700k failed to collect. IGT log at Patchwork_4403/fi-skl-6700k/igt.log

bf30bc2a70b83a77ba63436023f3550083715c56 drm-tip: 2017y-04m-04d-20h-00m-56s UTC integration manifest
bf6e308 drm/i915/scheduler: add gvt notification for guc
e9993ac drm/i915/scheduler: add gvt force-single-submission for guc

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4403/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 0/2] drm/i915/scheduler: add gvt force-single-submission and notification for guc

[Dong, Chuanxiao]

Hello, ping for v3 review.

Thanks
Chuanxiao

> -----Original Message-----
> From: Dong, Chuanxiao
> Sent: Wednesday, April 5, 2017 10:05 AM
> To: intel-gfx@lists.freedesktop.org
> Cc: intel-gvt-dev@lists.freedesktop.org; Dong, Chuanxiao
> <chuanxiao.dong@intel.com>
> Subject: [PATCH v3 0/2] drm/i915/scheduler: add gvt force-single-submission
> and notification for guc
> 
> GVT requires force-single-submission and notification when i915 using
> execlist submit, and these should be extended to GuC when
> i915 using GuC submit. Below two patches are used to implement this
> 
> Chuanxiao Dong (2):
>   drm/i915/scheduler: add gvt force-single-submission for guc
>   drm/i915/scheduler: add gvt notification for guc
> 
>  drivers/gpu/drm/i915/i915_gem_context.h    | 13 +++++++++
>  drivers/gpu/drm/i915/i915_guc_submission.c | 10 ++++++-
>  drivers/gpu/drm/i915/intel_gvt.h           | 23 +++++++++++++++
>  drivers/gpu/drm/i915/intel_lrc.c           | 46 +++++-------------------------
>  4 files changed, 52 insertions(+), 40 deletions(-)
> 
> --
> 2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 2/2] drm/i915/scheduler: add gvt notification for guc

[Chris Wilson]

On Tue, Mar 28, 2017 at 05:38:41PM +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.
> 
> 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           | 12 ++++++++++++
>  drivers/gpu/drm/i915/intel_lrc.c           | 21 +++------------------
>  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 58087630..d8a5942 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);
> @@ -725,6 +727,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);

This is incorrect though. This is no better than just waiting for the
request, which is not enough since the idea is that you need to wait for
the context image to be completely written to memory before you read it.
-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

Re: [PATCH v2 2/2] drm/i915/scheduler: add gvt notification for guc

[Dong, Chuanxiao]

> -----Original Message-----
> From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
> Sent: Thursday, April 6, 2017 9:32 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 v2 2/2] drm/i915/scheduler: add gvt notification for guc
> 
> On Tue, Mar 28, 2017 at 05:38:41PM +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.
> >
> > 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           | 12 ++++++++++++
> >  drivers/gpu/drm/i915/intel_lrc.c           | 21 +++------------------
> >  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 58087630..d8a5942 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);
> > @@ -725,6 +727,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);
> 
> This is incorrect though. This is no better than just waiting for the request,
> which is not enough since the idea is that you need to wait for the context
> image to be completely written to memory before you read it.
> -Chris

The wait for the context image to be completely written will be done in the notification from the GVT, by checking the CSB. If put the wait here will made each i915 request to wait, which seems not necessary.

Thanks
Chuanxiao

> 
> --
> Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 2/2] drm/i915/scheduler: add gvt notification for guc

[Chris Wilson]

On Thu, Apr 06, 2017 at 02:05:15PM +0000, Dong, Chuanxiao wrote:
> 
> 
> > -----Original Message-----
> > From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
> > Sent: Thursday, April 6, 2017 9:32 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 v2 2/2] drm/i915/scheduler: add gvt notification for guc
> > 
> > On Tue, Mar 28, 2017 at 05:38:41PM +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.
> > >
> > > 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           | 12 ++++++++++++
> > >  drivers/gpu/drm/i915/intel_lrc.c           | 21 +++------------------
> > >  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 58087630..d8a5942 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);
> > > @@ -725,6 +727,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);
> > 
> > This is incorrect though. This is no better than just waiting for the request,
> > which is not enough since the idea is that you need to wait for the context
> > image to be completely written to memory before you read it.
> > -Chris
> 
> The wait for the context image to be completely written will be done in the notification from the GVT, by checking the CSB. If put the wait here will made each i915 request to wait, which seems not necessary.

Urm, no. I hope you mean the wait will be on some other thread than
inside this interrupt handler.
-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

Re: [PATCH v2 2/2] drm/i915/scheduler: add gvt notification for guc

[Dong, Chuanxiao]

> -----Original Message-----
> From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
> Sent: Thursday, April 6, 2017 10:19 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 v2 2/2] drm/i915/scheduler: add gvt notification for guc
> 
> On Thu, Apr 06, 2017 at 02:05:15PM +0000, Dong, Chuanxiao wrote:
> >
> >
> > > -----Original Message-----
> > > From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
> > > Sent: Thursday, April 6, 2017 9:32 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 v2 2/2] drm/i915/scheduler: add gvt notification
> > > for guc
> > >
> > > On Tue, Mar 28, 2017 at 05:38:41PM +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.
> > > >
> > > > 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           | 12 ++++++++++++
> > > >  drivers/gpu/drm/i915/intel_lrc.c           | 21 +++------------------
> > > >  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 58087630..d8a5942 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);
> > > > @@ -725,6 +727,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);
> > >
> > > This is incorrect though. This is no better than just waiting for
> > > the request, which is not enough since the idea is that you need to
> > > wait for the context image to be completely written to memory before
> you read it.
> > > -Chris
> >
> > The wait for the context image to be completely written will be done in the
> notification from the GVT, by checking the CSB. If put the wait here will made
> each i915 request to wait, which seems not necessary.
> 
> Urm, no. I hope you mean the wait will be on some other thread than inside
> this interrupt handler.

The SCHEDULE_OUT means to stop GuC to submit another request before the current one is completed by GVT so GVT can manually restore the MMIO. So this irq handler should wait until SCHEDULE_OUT is completed. How it possible to make this irq handler to wait for another thread? From the current software architecture there is no other thread....

To make sure the context image is ready, GVT will poll CSB. If CSB is not in idle, just relax cpu and continue. In the real case, CSB is already idle before GVT to poll.

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

Re: [PATCH v2 2/2] drm/i915/scheduler: add gvt notification for guc

[Chris Wilson]

On Thu, Apr 06, 2017 at 02:49:54PM +0000, Dong, Chuanxiao wrote:
> 
> 
> > -----Original Message-----
> > From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
> > Sent: Thursday, April 6, 2017 10:19 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 v2 2/2] drm/i915/scheduler: add gvt notification for guc
> > 
> > On Thu, Apr 06, 2017 at 02:05:15PM +0000, Dong, Chuanxiao wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
> > > > Sent: Thursday, April 6, 2017 9:32 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 v2 2/2] drm/i915/scheduler: add gvt notification
> > > > for guc
> > > >
> > > > On Tue, Mar 28, 2017 at 05:38:41PM +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.
> > > > >
> > > > > 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           | 12 ++++++++++++
> > > > >  drivers/gpu/drm/i915/intel_lrc.c           | 21 +++------------------
> > > > >  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 58087630..d8a5942 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);
> > > > > @@ -725,6 +727,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);
> > > >
> > > > This is incorrect though. This is no better than just waiting for
> > > > the request, which is not enough since the idea is that you need to
> > > > wait for the context image to be completely written to memory before
> > you read it.
> > > > -Chris
> > >
> > > The wait for the context image to be completely written will be done in the
> > notification from the GVT, by checking the CSB. If put the wait here will made
> > each i915 request to wait, which seems not necessary.
> > 
> > Urm, no. I hope you mean the wait will be on some other thread than inside
> > this interrupt handler.
> 
> The SCHEDULE_OUT means to stop GuC to submit another request before the current one is completed by GVT so GVT can manually restore the MMIO. So this irq handler should wait until SCHEDULE_OUT is completed. How it possible to make this irq handler to wait for another thread? From the current software architecture there is no other thread....

No. It is not acceptable to have any blocking here. Rather you delegate
the polling of CSB to a thread/worker that you kick off from this notify.
-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

Re: [PATCH v2 2/2] drm/i915/scheduler: add gvt notification for guc

[Dong, Chuanxiao]

> -----Original Message-----
> From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
> Sent: Thursday, April 6, 2017 11:07 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; Wang, Zhi A
> Subject: Re: [PATCH v2 2/2] drm/i915/scheduler: add gvt notification for guc
> 
> On Thu, Apr 06, 2017 at 02:49:54PM +0000, Dong, Chuanxiao wrote:
> >
> >
> > > -----Original Message-----
> > > From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
> > > Sent: Thursday, April 6, 2017 10:19 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 v2 2/2] drm/i915/scheduler: add gvt notification
> > > for guc
> > >
> > > On Thu, Apr 06, 2017 at 02:05:15PM +0000, Dong, Chuanxiao wrote:
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
> > > > > Sent: Thursday, April 6, 2017 9:32 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 v2 2/2] drm/i915/scheduler: add gvt
> > > > > notification for guc
> > > > >
> > > > > On Tue, Mar 28, 2017 at 05:38:41PM +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.
> > > > > >
> > > > > > 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           | 12 ++++++++++++
> > > > > >  drivers/gpu/drm/i915/intel_lrc.c           | 21 +++------------------
> > > > > >  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 58087630..d8a5942 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); @@ -725,6
> +727,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);
> > > > >
> > > > > This is incorrect though. This is no better than just waiting
> > > > > for the request, which is not enough since the idea is that you
> > > > > need to wait for the context image to be completely written to
> > > > > memory before
> > > you read it.
> > > > > -Chris
> > > >
> > > > The wait for the context image to be completely written will be
> > > > done in the
> > > notification from the GVT, by checking the CSB. If put the wait here
> > > will made each i915 request to wait, which seems not necessary.
> > >
> > > Urm, no. I hope you mean the wait will be on some other thread than
> > > inside this interrupt handler.
> >
> > The SCHEDULE_OUT means to stop GuC to submit another request before
> the current one is completed by GVT so GVT can manually restore the MMIO.
> So this irq handler should wait until SCHEDULE_OUT is completed. How it
> possible to make this irq handler to wait for another thread? From the
> current software architecture there is no other thread....
> 
> No. It is not acceptable to have any blocking here. Rather you delegate the
> polling of CSB to a thread/worker that you kick off from this notify.
> -Chris

The major issue is that we should wait for the context image to be completely written to memory before GVT read it. I will double check if we are really reading from context image in this SCHEDULE_OUT event and return back later.

Thanks
Chuanxiao

> 
> --
> Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 2/2] drm/i915/scheduler: add gvt notification for guc

[Dong, Chuanxiao]

> -----Original Message-----
> From: intel-gvt-dev [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On
> Behalf Of Dong, Chuanxiao
> Sent: Thursday, April 6, 2017 11:19 PM
> To: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tian, Kevin <kevin.tian@intel.com>; intel-gvt-dev@lists.freedesktop.org;
> intel-gfx@lists.freedesktop.org; joonas.lahtinen@linux.intel.com; Zheng,
> Xiao <xiao.zheng@intel.com>; Wang, Zhi A <zhi.a.wang@intel.com>
> Subject: RE: [PATCH v2 2/2] drm/i915/scheduler: add gvt notification for guc
> 
> 
> 
> > -----Original Message-----
> > From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
> > Sent: Thursday, April 6, 2017 11:07 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; Wang, Zhi A
> > Subject: Re: [PATCH v2 2/2] drm/i915/scheduler: add gvt notification
> > for guc
> >
> > On Thu, Apr 06, 2017 at 02:49:54PM +0000, Dong, Chuanxiao wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
> > > > Sent: Thursday, April 6, 2017 10:19 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 v2 2/2] drm/i915/scheduler: add gvt
> > > > notification for guc
> > > >
> > > > On Thu, Apr 06, 2017 at 02:05:15PM +0000, Dong, Chuanxiao wrote:
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
> > > > > > Sent: Thursday, April 6, 2017 9:32 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 v2 2/2] drm/i915/scheduler: add gvt
> > > > > > notification for guc
> > > > > >
> > > > > > On Tue, Mar 28, 2017 at 05:38:41PM +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.
> > > > > > >
> > > > > > > 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           | 12 ++++++++++++
> > > > > > >  drivers/gpu/drm/i915/intel_lrc.c           | 21 +++------------------
> > > > > > >  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 58087630..d8a5942 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); @@ -725,6
> > +727,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);
> > > > > >
> > > > > > This is incorrect though. This is no better than just waiting
> > > > > > for the request, which is not enough since the idea is that
> > > > > > you need to wait for the context image to be completely
> > > > > > written to memory before
> > > > you read it.
> > > > > > -Chris
> > > > >
> > > > > The wait for the context image to be completely written will be
> > > > > done in the
> > > > notification from the GVT, by checking the CSB. If put the wait
> > > > here will made each i915 request to wait, which seems not necessary.
> > > >
> > > > Urm, no. I hope you mean the wait will be on some other thread
> > > > than inside this interrupt handler.
> > >
> > > The SCHEDULE_OUT means to stop GuC to submit another request
> before
> > the current one is completed by GVT so GVT can manually restore the
> MMIO.
> > So this irq handler should wait until SCHEDULE_OUT is completed. How
> > it possible to make this irq handler to wait for another thread? From
> > the current software architecture there is no other thread....
> >
> > No. It is not acceptable to have any blocking here. Rather you
> > delegate the polling of CSB to a thread/worker that you kick off from this
> notify.
> > -Chris
> 
> The major issue is that we should wait for the context image to be
> completely written to memory before GVT read it. I will double check if we
> are really reading from context image in this SCHEDULE_OUT event and
> return back later.

Hi Chris,

Had a discussion with Zhi, actually the context image is accessed from the workload_thread to update the guest context but not directly from the SCHEDULE_OUT event. So my previous comment is wrong and the CSB waiting should be in workload_thread instead of this IRQ handler.

Thanks
Chuanxiao

> 
> >
> > --
> > 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

Re: [PATCH v2 2/2] drm/i915/scheduler: add gvt notification for guc

[Dong, Chuanxiao]

> -----Original Message-----
> From: Dong, Chuanxiao
> Sent: Monday, April 10, 2017 10:40 AM
> To: Dong, Chuanxiao <chuanxiao.dong@intel.com>; Chris Wilson
> <chris@chris-wilson.co.uk>
> Cc: Tian, Kevin <kevin.tian@intel.com>; intel-gvt-dev@lists.freedesktop.org;
> intel-gfx@lists.freedesktop.org; joonas.lahtinen@linux.intel.com; Zheng,
> Xiao <xiao.zheng@intel.com>; Wang, Zhi A <zhi.a.wang@intel.com>
> Subject: RE: [PATCH v2 2/2] drm/i915/scheduler: add gvt notification for guc
> 
> > -----Original Message-----
> > From: intel-gvt-dev
> > [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On Behalf Of
> > Dong, Chuanxiao
> > Sent: Thursday, April 6, 2017 11:19 PM
> > To: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Tian, Kevin <kevin.tian@intel.com>;
> > intel-gvt-dev@lists.freedesktop.org;
> > intel-gfx@lists.freedesktop.org; joonas.lahtinen@linux.intel.com;
> > Zheng, Xiao <xiao.zheng@intel.com>; Wang, Zhi A <zhi.a.wang@intel.com>
> > Subject: RE: [PATCH v2 2/2] drm/i915/scheduler: add gvt notification
> > for guc
> >
> >
> >
> > > -----Original Message-----
> > > From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
> > > Sent: Thursday, April 6, 2017 11:07 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; Wang, Zhi
> > > A
> > > Subject: Re: [PATCH v2 2/2] drm/i915/scheduler: add gvt notification
> > > for guc
> > >
> > > On Thu, Apr 06, 2017 at 02:49:54PM +0000, Dong, Chuanxiao wrote:
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
> > > > > Sent: Thursday, April 6, 2017 10:19 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 v2 2/2] drm/i915/scheduler: add gvt
> > > > > notification for guc
> > > > >
> > > > > On Thu, Apr 06, 2017 at 02:05:15PM +0000, Dong, Chuanxiao wrote:
> > > > > >
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
> > > > > > > Sent: Thursday, April 6, 2017 9:32 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 v2 2/2] drm/i915/scheduler: add gvt
> > > > > > > notification for guc
> > > > > > >
> > > > > > > On Tue, Mar 28, 2017 at 05:38:41PM +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.
> > > > > > > >
> > > > > > > > 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           | 12 ++++++++++++
> > > > > > > >  drivers/gpu/drm/i915/intel_lrc.c           | 21 +++------------------
> > > > > > > >  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 58087630..d8a5942 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); @@ -725,6
> > > +727,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);
> > > > > > >
> > > > > > > This is incorrect though. This is no better than just
> > > > > > > waiting for the request, which is not enough since the idea
> > > > > > > is that you need to wait for the context image to be
> > > > > > > completely written to memory before
> > > > > you read it.
> > > > > > > -Chris
> > > > > >
> > > > > > The wait for the context image to be completely written will
> > > > > > be done in the
> > > > > notification from the GVT, by checking the CSB. If put the wait
> > > > > here will made each i915 request to wait, which seems not necessary.
> > > > >
> > > > > Urm, no. I hope you mean the wait will be on some other thread
> > > > > than inside this interrupt handler.
> > > >
> > > > The SCHEDULE_OUT means to stop GuC to submit another request
> > before
> > > the current one is completed by GVT so GVT can manually restore the
> > MMIO.
> > > So this irq handler should wait until SCHEDULE_OUT is completed. How
> > > it possible to make this irq handler to wait for another thread?
> > > From the current software architecture there is no other thread....
> > >
> > > No. It is not acceptable to have any blocking here. Rather you
> > > delegate the polling of CSB to a thread/worker that you kick off
> > > from this
> > notify.
> > > -Chris
> >
> > The major issue is that we should wait for the context image to be
> > completely written to memory before GVT read it. I will double check
> > if we are really reading from context image in this SCHEDULE_OUT event
> > and return back later.
> 
> Hi Chris,
> 
> Had a discussion with Zhi, actually the context image is accessed from the
> workload_thread to update the guest context but not directly from the
> SCHEDULE_OUT event. So my previous comment is wrong and the CSB
> waiting should be in workload_thread instead of this IRQ handler.

Hi Chris, may I know if you still have concerns with the above comment? Would like to know if this is acceptable to i915.

Thanks
Chuanxiao

> 
> >
> > >
> > > --
> > > 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

Re: [PATCH v2 2/2] drm/i915/scheduler: add gvt notification for guc

[Chris Wilson]

On Mon, Apr 10, 2017 at 02:40:24AM +0000, Dong, Chuanxiao wrote:
> > -----Original Message-----
> > From: intel-gvt-dev [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On
> > Behalf Of Dong, Chuanxiao
> > Sent: Thursday, April 6, 2017 11:19 PM
> > To: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Tian, Kevin <kevin.tian@intel.com>; intel-gvt-dev@lists.freedesktop.org;
> > intel-gfx@lists.freedesktop.org; joonas.lahtinen@linux.intel.com; Zheng,
> > Xiao <xiao.zheng@intel.com>; Wang, Zhi A <zhi.a.wang@intel.com>
> > Subject: RE: [PATCH v2 2/2] drm/i915/scheduler: add gvt notification for guc
> > 
> > 
> > 
> > > -----Original Message-----
> > > From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
> > > Sent: Thursday, April 6, 2017 11:07 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; Wang, Zhi A
> > > Subject: Re: [PATCH v2 2/2] drm/i915/scheduler: add gvt notification
> > > for guc
> > >
> > > On Thu, Apr 06, 2017 at 02:49:54PM +0000, Dong, Chuanxiao wrote:
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
> > > > > Sent: Thursday, April 6, 2017 10:19 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 v2 2/2] drm/i915/scheduler: add gvt
> > > > > notification for guc
> > > > >
> > > > > On Thu, Apr 06, 2017 at 02:05:15PM +0000, Dong, Chuanxiao wrote:
> > > > > >
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
> > > > > > > Sent: Thursday, April 6, 2017 9:32 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 v2 2/2] drm/i915/scheduler: add gvt
> > > > > > > notification for guc
> > > > > > >
> > > > > > > On Tue, Mar 28, 2017 at 05:38:41PM +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.
> > > > > > > >
> > > > > > > > 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           | 12 ++++++++++++
> > > > > > > >  drivers/gpu/drm/i915/intel_lrc.c           | 21 +++------------------
> > > > > > > >  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 58087630..d8a5942 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); @@ -725,6
> > > +727,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);
> > > > > > >
> > > > > > > This is incorrect though. This is no better than just waiting
> > > > > > > for the request, which is not enough since the idea is that
> > > > > > > you need to wait for the context image to be completely
> > > > > > > written to memory before
> > > > > you read it.
> > > > > > > -Chris
> > > > > >
> > > > > > The wait for the context image to be completely written will be
> > > > > > done in the
> > > > > notification from the GVT, by checking the CSB. If put the wait
> > > > > here will made each i915 request to wait, which seems not necessary.
> > > > >
> > > > > Urm, no. I hope you mean the wait will be on some other thread
> > > > > than inside this interrupt handler.
> > > >
> > > > The SCHEDULE_OUT means to stop GuC to submit another request
> > before
> > > the current one is completed by GVT so GVT can manually restore the
> > MMIO.
> > > So this irq handler should wait until SCHEDULE_OUT is completed. How
> > > it possible to make this irq handler to wait for another thread? From
> > > the current software architecture there is no other thread....
> > >
> > > No. It is not acceptable to have any blocking here. Rather you
> > > delegate the polling of CSB to a thread/worker that you kick off from this
> > notify.
> > > -Chris
> > 
> > The major issue is that we should wait for the context image to be
> > completely written to memory before GVT read it. I will double check if we
> > are really reading from context image in this SCHEDULE_OUT event and
> > return back later.
> 
> Hi Chris,
> 
> Had a discussion with Zhi, actually the context image is accessed from the workload_thread to update the guest context but not directly from the SCHEDULE_OUT event. So my previous comment is wrong and the CSB waiting should be in workload_thread instead of this IRQ handler.

That's better, still a little worried about having to remember to
review these hooks later.

The other concern I express at the start of this was:

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 9e327049b735..5a86abd715df 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -430,16 +430,6 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
                        if (port != engine->execlist_port)
                                break;
 
-                       /* If GVT overrides us we only ever submit port[0],
-                        * leaving port[1] empty. Note that we also have
-                        * to be careful that we don't queue the same
-                        * context (even though a different request) to
-                        * the second port.
-                        */
-                       if (ctx_single_port_submission(last->ctx) ||
-                           ctx_single_port_submission(cursor->ctx))
-                               break;
-


Afaict that requirement is completely bogus (since your workloads do
only active on gvt requests and there will only ever be one scheduled-in
at any time) and burdensome on non-gvt.
-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

Re: [PATCH v2 2/2] drm/i915/scheduler: add gvt notification for guc

[Dong, Chuanxiao]

> -----Original Message-----
> From: intel-gvt-dev [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On
> Behalf Of Chris Wilson
> Sent: Wednesday, April 12, 2017 4:22 PM
> To: Dong, Chuanxiao <chuanxiao.dong@intel.com>
> Cc: Tian, Kevin <kevin.tian@intel.com>; intel-gvt-dev@lists.freedesktop.org;
> intel-gfx@lists.freedesktop.org; joonas.lahtinen@linux.intel.com; Zheng,
> Xiao <xiao.zheng@intel.com>; Wang, Zhi A <zhi.a.wang@intel.com>
> Subject: Re: [PATCH v2 2/2] drm/i915/scheduler: add gvt notification for guc
> 
> On Mon, Apr 10, 2017 at 02:40:24AM +0000, Dong, Chuanxiao wrote:
> > > -----Original Message-----
> > > From: intel-gvt-dev
> > > [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On Behalf Of
> > > Dong, Chuanxiao
> > > Sent: Thursday, April 6, 2017 11:19 PM
> > > To: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: Tian, Kevin <kevin.tian@intel.com>;
> > > intel-gvt-dev@lists.freedesktop.org;
> > > intel-gfx@lists.freedesktop.org; joonas.lahtinen@linux.intel.com;
> > > Zheng, Xiao <xiao.zheng@intel.com>; Wang, Zhi A
> > > <zhi.a.wang@intel.com>
> > > Subject: RE: [PATCH v2 2/2] drm/i915/scheduler: add gvt notification
> > > for guc
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
> > > > Sent: Thursday, April 6, 2017 11:07 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; Wang,
> > > > Zhi A
> > > > Subject: Re: [PATCH v2 2/2] drm/i915/scheduler: add gvt
> > > > notification for guc
> > > >
> > > > On Thu, Apr 06, 2017 at 02:49:54PM +0000, Dong, Chuanxiao wrote:
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
> > > > > > Sent: Thursday, April 6, 2017 10:19 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 v2 2/2] drm/i915/scheduler: add gvt
> > > > > > notification for guc
> > > > > >
> > > > > > On Thu, Apr 06, 2017 at 02:05:15PM +0000, Dong, Chuanxiao wrote:
> > > > > > >
> > > > > > >
> > > > > > > > -----Original Message-----
> > > > > > > > From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
> > > > > > > > Sent: Thursday, April 6, 2017 9:32 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 v2 2/2] drm/i915/scheduler: add gvt
> > > > > > > > notification for guc
> > > > > > > >
> > > > > > > > On Tue, Mar 28, 2017 at 05:38:41PM +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.
> > > > > > > > >
> > > > > > > > > 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           | 12 ++++++++++++
> > > > > > > > >  drivers/gpu/drm/i915/intel_lrc.c           | 21 +++------------------
> > > > > > > > >  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 58087630..d8a5942 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); @@ -725,6
> > > > +727,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);
> > > > > > > >
> > > > > > > > This is incorrect though. This is no better than just
> > > > > > > > waiting for the request, which is not enough since the
> > > > > > > > idea is that you need to wait for the context image to be
> > > > > > > > completely written to memory before
> > > > > > you read it.
> > > > > > > > -Chris
> > > > > > >
> > > > > > > The wait for the context image to be completely written will
> > > > > > > be done in the
> > > > > > notification from the GVT, by checking the CSB. If put the
> > > > > > wait here will made each i915 request to wait, which seems not
> necessary.
> > > > > >
> > > > > > Urm, no. I hope you mean the wait will be on some other thread
> > > > > > than inside this interrupt handler.
> > > > >
> > > > > The SCHEDULE_OUT means to stop GuC to submit another request
> > > before
> > > > the current one is completed by GVT so GVT can manually restore
> > > > the
> > > MMIO.
> > > > So this irq handler should wait until SCHEDULE_OUT is completed.
> > > > How it possible to make this irq handler to wait for another
> > > > thread? From the current software architecture there is no other
> thread....
> > > >
> > > > No. It is not acceptable to have any blocking here. Rather you
> > > > delegate the polling of CSB to a thread/worker that you kick off
> > > > from this
> > > notify.
> > > > -Chris
> > >
> > > The major issue is that we should wait for the context image to be
> > > completely written to memory before GVT read it. I will double check
> > > if we are really reading from context image in this SCHEDULE_OUT
> > > event and return back later.
> >
> > Hi Chris,
> >
> > Had a discussion with Zhi, actually the context image is accessed from the
> workload_thread to update the guest context but not directly from the
> SCHEDULE_OUT event. So my previous comment is wrong and the CSB
> waiting should be in workload_thread instead of this IRQ handler.
> 
> That's better, still a little worried about having to remember to review these
> hooks later.
> 
> The other concern I express at the start of this was:
Sorry, I missed this one.

> 
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c
> b/drivers/gpu/drm/i915/intel_lrc.c
> index 9e327049b735..5a86abd715df 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -430,16 +430,6 @@ static void execlists_dequeue(struct
> intel_engine_cs *engine)
>                         if (port != engine->execlist_port)
>                                 break;
> 
> -                       /* If GVT overrides us we only ever submit port[0],
> -                        * leaving port[1] empty. Note that we also have
> -                        * to be careful that we don't queue the same
> -                        * context (even though a different request) to
> -                        * the second port.
> -                        */
> -                       if (ctx_single_port_submission(last->ctx) ||
> -                           ctx_single_port_submission(cursor->ctx))
> -                               break;
> -
> 
> 
> Afaict that requirement is completely bogus (since your workloads do only
> active on gvt requests and there will only ever be one scheduled-in at any
> time) and burdensome on non-gvt.
> -Chris

My understanding about this is to prevent both port[0] and port[1] submitted if any of the last/cursor is a GVT request. Without this check, 1) last is a GVT request in port[0] and cursor is an i915 request, this i915 request will be submitted to port[1]; 2) last is an i915 request in port[0] and cursor is a GVT request, this GVT request will be submitted to port[1]; 3) Last is a GVT request from vgpu1 in port[0] and cursor is a GVT request from vgpu2, the GVT request from vgpu2 will be submitted to port[1];

Is this ok without the check? Or my understanding is wrong? Please help me to get this. :)

Thanks
Chuanxiao

> 
> --
> 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

Re: [PATCH v2 2/2] drm/i915/scheduler: add gvt notification for guc

[Dong, Chuanxiao]

> -----Original Message-----
> From: intel-gvt-dev [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On
> Behalf Of Dong, Chuanxiao
> Sent: Wednesday, April 12, 2017 5:12 PM
> To: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tian, Kevin <kevin.tian@intel.com>; intel-gvt-dev@lists.freedesktop.org;
> intel-gfx@lists.freedesktop.org; joonas.lahtinen@linux.intel.com; Zheng,
> Xiao <xiao.zheng@intel.com>; Wang, Zhi A <zhi.a.wang@intel.com>
> Subject: RE: [PATCH v2 2/2] drm/i915/scheduler: add gvt notification for guc
> 
> 
> 
> > -----Original Message-----
> > From: intel-gvt-dev
> > [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On Behalf Of
> > Chris Wilson
> > Sent: Wednesday, April 12, 2017 4:22 PM
> > To: Dong, Chuanxiao <chuanxiao.dong@intel.com>
> > Cc: Tian, Kevin <kevin.tian@intel.com>;
> > intel-gvt-dev@lists.freedesktop.org;
> > intel-gfx@lists.freedesktop.org; joonas.lahtinen@linux.intel.com;
> > Zheng, Xiao <xiao.zheng@intel.com>; Wang, Zhi A <zhi.a.wang@intel.com>
> > Subject: Re: [PATCH v2 2/2] drm/i915/scheduler: add gvt notification
> > for guc
> >
> > On Mon, Apr 10, 2017 at 02:40:24AM +0000, Dong, Chuanxiao wrote:
> > > > -----Original Message-----
> > > > From: intel-gvt-dev
> > > > [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On Behalf Of
> > > > Dong, Chuanxiao
> > > > Sent: Thursday, April 6, 2017 11:19 PM
> > > > To: Chris Wilson <chris@chris-wilson.co.uk>
> > > > Cc: Tian, Kevin <kevin.tian@intel.com>;
> > > > intel-gvt-dev@lists.freedesktop.org;
> > > > intel-gfx@lists.freedesktop.org; joonas.lahtinen@linux.intel.com;
> > > > Zheng, Xiao <xiao.zheng@intel.com>; Wang, Zhi A
> > > > <zhi.a.wang@intel.com>
> > > > Subject: RE: [PATCH v2 2/2] drm/i915/scheduler: add gvt
> > > > notification for guc
> > > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
> > > > > Sent: Thursday, April 6, 2017 11:07 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; Wang,
> > > > > Zhi A
> > > > > Subject: Re: [PATCH v2 2/2] drm/i915/scheduler: add gvt
> > > > > notification for guc
> > > > >
> > > > > On Thu, Apr 06, 2017 at 02:49:54PM +0000, Dong, Chuanxiao wrote:
> > > > > >
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
> > > > > > > Sent: Thursday, April 6, 2017 10:19 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 v2 2/2] drm/i915/scheduler: add gvt
> > > > > > > notification for guc
> > > > > > >
> > > > > > > On Thu, Apr 06, 2017 at 02:05:15PM +0000, Dong, Chuanxiao
> wrote:
> > > > > > > >
> > > > > > > >
> > > > > > > > > -----Original Message-----
> > > > > > > > > From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
> > > > > > > > > Sent: Thursday, April 6, 2017 9:32 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 v2 2/2] drm/i915/scheduler: add gvt
> > > > > > > > > notification for guc
> > > > > > > > >
> > > > > > > > > On Tue, Mar 28, 2017 at 05:38:41PM +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.
> > > > > > > > > >
> > > > > > > > > > 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           | 12 ++++++++++++
> > > > > > > > > >  drivers/gpu/drm/i915/intel_lrc.c           | 21 +++----------------
> --
> > > > > > > > > >  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 58087630..d8a5942 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); @@ -
> 725,6
> > > > > +727,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);
> > > > > > > > >
> > > > > > > > > This is incorrect though. This is no better than just
> > > > > > > > > waiting for the request, which is not enough since the
> > > > > > > > > idea is that you need to wait for the context image to
> > > > > > > > > be completely written to memory before
> > > > > > > you read it.
> > > > > > > > > -Chris
> > > > > > > >
> > > > > > > > The wait for the context image to be completely written
> > > > > > > > will be done in the
> > > > > > > notification from the GVT, by checking the CSB. If put the
> > > > > > > wait here will made each i915 request to wait, which seems
> > > > > > > not
> > necessary.
> > > > > > >
> > > > > > > Urm, no. I hope you mean the wait will be on some other
> > > > > > > thread than inside this interrupt handler.
> > > > > >
> > > > > > The SCHEDULE_OUT means to stop GuC to submit another request
> > > > before
> > > > > the current one is completed by GVT so GVT can manually restore
> > > > > the
> > > > MMIO.
> > > > > So this irq handler should wait until SCHEDULE_OUT is completed.
> > > > > How it possible to make this irq handler to wait for another
> > > > > thread? From the current software architecture there is no other
> > thread....
> > > > >
> > > > > No. It is not acceptable to have any blocking here. Rather you
> > > > > delegate the polling of CSB to a thread/worker that you kick off
> > > > > from this
> > > > notify.
> > > > > -Chris
> > > >
> > > > The major issue is that we should wait for the context image to be
> > > > completely written to memory before GVT read it. I will double
> > > > check if we are really reading from context image in this
> > > > SCHEDULE_OUT event and return back later.
> > >
> > > Hi Chris,
> > >
> > > Had a discussion with Zhi, actually the context image is accessed
> > > from the
> > workload_thread to update the guest context but not directly from the
> > SCHEDULE_OUT event. So my previous comment is wrong and the CSB
> > waiting should be in workload_thread instead of this IRQ handler.
> >
> > That's better, still a little worried about having to remember to
> > review these hooks later.
> >
> > The other concern I express at the start of this was:
> Sorry, I missed this one.
> 
> >
> > diff --git a/drivers/gpu/drm/i915/intel_lrc.c
> > b/drivers/gpu/drm/i915/intel_lrc.c
> > index 9e327049b735..5a86abd715df 100644
> > --- a/drivers/gpu/drm/i915/intel_lrc.c
> > +++ b/drivers/gpu/drm/i915/intel_lrc.c
> > @@ -430,16 +430,6 @@ static void execlists_dequeue(struct
> > intel_engine_cs *engine)
> >                         if (port != engine->execlist_port)
> >                                 break;
> >
> > -                       /* If GVT overrides us we only ever submit port[0],
> > -                        * leaving port[1] empty. Note that we also have
> > -                        * to be careful that we don't queue the same
> > -                        * context (even though a different request) to
> > -                        * the second port.
> > -                        */
> > -                       if (ctx_single_port_submission(last->ctx) ||
> > -                           ctx_single_port_submission(cursor->ctx))
> > -                               break;
> > -
> >
> >
> > Afaict that requirement is completely bogus (since your workloads do
> > only active on gvt requests and there will only ever be one
> > scheduled-in at any
> > time) and burdensome on non-gvt.
> > -Chris
> 
> My understanding about this is to prevent both port[0] and port[1] submitted
> if any of the last/cursor is a GVT request. Without this check, 1) last is a GVT
> request in port[0] and cursor is an i915 request, this i915 request will be
> submitted to port[1]; 2) last is an i915 request in port[0] and cursor is a GVT
> request, this GVT request will be submitted to port[1]; 3) Last is a GVT
> request from vgpu1 in port[0] and cursor is a GVT request from vgpu2, the
> GVT request from vgpu2 will be submitted to port[1];
> 
> Is this ok without the check? Or my understanding is wrong? Please help me
> to get this. :)
> 
Hi Chris,

Had a discussed this with Zhi about removing this check, and we can see after removing this check, port[0] and port[1] can be submitted both for the above 3 cases. For GVT request, only one of the port can be used and the other one should be empty, just like the code comment said. The reason is that GVT still needs to do some MMIO restore manually in the SCHEDULE_OUT event before i915 starts to handle another request. If removing the check and have both port[0] and port[1] submitted, i915 will start to process port[1] automatically once port[0] is completed. So this check is meaningful to GVT and cannot be removed. Do you agree to keep this check?

Thanks
Chuanxiao 
> 
> >
> > --
> > 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-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

Re: [PATCH v2 2/2] drm/i915/scheduler: add gvt notification for guc

[Dong, Chuanxiao]

> -----Original Message-----
> From: intel-gvt-dev [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On
> Behalf Of Dong, Chuanxiao
> Sent: Thursday, April 13, 2017 7:03 PM
> To: Chris Wilson <chris@chris-wilson.co.uk>; intel-gfx@lists.freedesktop.org
> Cc: Zheng, Xiao <xiao.zheng@intel.com>; Tian, Kevin
> <kevin.tian@intel.com>; joonas.lahtinen@linux.intel.com; intel-gvt-
> dev@lists.freedesktop.org; Wang, Zhi A <zhi.a.wang@intel.com>
> Subject: RE: [PATCH v2 2/2] drm/i915/scheduler: add gvt notification for guc
> 
> > -----Original Message-----
> > From: intel-gvt-dev
> > [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On Behalf Of
> > Dong, Chuanxiao
> > Sent: Wednesday, April 12, 2017 5:12 PM
> > To: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Tian, Kevin <kevin.tian@intel.com>;
> > intel-gvt-dev@lists.freedesktop.org;
> > intel-gfx@lists.freedesktop.org; joonas.lahtinen@linux.intel.com;
> > Zheng, Xiao <xiao.zheng@intel.com>; Wang, Zhi A <zhi.a.wang@intel.com>
> > Subject: RE: [PATCH v2 2/2] drm/i915/scheduler: add gvt notification
> > for guc
> >
> >
> >
> > > -----Original Message-----
> > > From: intel-gvt-dev
> > > [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On Behalf Of
> > > Chris Wilson
> > > Sent: Wednesday, April 12, 2017 4:22 PM
> > > To: Dong, Chuanxiao <chuanxiao.dong@intel.com>
> > > Cc: Tian, Kevin <kevin.tian@intel.com>;
> > > intel-gvt-dev@lists.freedesktop.org;
> > > intel-gfx@lists.freedesktop.org; joonas.lahtinen@linux.intel.com;
> > > Zheng, Xiao <xiao.zheng@intel.com>; Wang, Zhi A
> > > <zhi.a.wang@intel.com>
> > > Subject: Re: [PATCH v2 2/2] drm/i915/scheduler: add gvt notification
> > > for guc
> > >
> > > On Mon, Apr 10, 2017 at 02:40:24AM +0000, Dong, Chuanxiao wrote:
> > > > > -----Original Message-----
> > > > > From: intel-gvt-dev
> > > > > [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On Behalf
> > > > > Of Dong, Chuanxiao
> > > > > Sent: Thursday, April 6, 2017 11:19 PM
> > > > > To: Chris Wilson <chris@chris-wilson.co.uk>
> > > > > Cc: Tian, Kevin <kevin.tian@intel.com>;
> > > > > intel-gvt-dev@lists.freedesktop.org;
> > > > > intel-gfx@lists.freedesktop.org;
> > > > > joonas.lahtinen@linux.intel.com; Zheng, Xiao
> > > > > <xiao.zheng@intel.com>; Wang, Zhi A <zhi.a.wang@intel.com>
> > > > > Subject: RE: [PATCH v2 2/2] drm/i915/scheduler: add gvt
> > > > > notification for guc
> > > > >
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
> > > > > > Sent: Thursday, April 6, 2017 11:07 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;
> > > > > > Wang, Zhi A
> > > > > > Subject: Re: [PATCH v2 2/2] drm/i915/scheduler: add gvt
> > > > > > notification for guc
> > > > > >
> > > > > > On Thu, Apr 06, 2017 at 02:49:54PM +0000, Dong, Chuanxiao wrote:
> > > > > > >
> > > > > > >
> > > > > > > > -----Original Message-----
> > > > > > > > From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
> > > > > > > > Sent: Thursday, April 6, 2017 10:19 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 v2 2/2] drm/i915/scheduler: add gvt
> > > > > > > > notification for guc
> > > > > > > >
> > > > > > > > On Thu, Apr 06, 2017 at 02:05:15PM +0000, Dong, Chuanxiao
> > wrote:
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > > -----Original Message-----
> > > > > > > > > > From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
> > > > > > > > > > Sent: Thursday, April 6, 2017 9:32 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 v2 2/2] drm/i915/scheduler: add
> > > > > > > > > > gvt notification for guc
> > > > > > > > > >
> > > > > > > > > > On Tue, Mar 28, 2017 at 05:38:41PM +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.
> > > > > > > > > > >
> > > > > > > > > > > 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           | 12 ++++++++++++
> > > > > > > > > > >  drivers/gpu/drm/i915/intel_lrc.c           | 21 +++--------------
> --
> > --
> > > > > > > > > > >  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 58087630..d8a5942 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); @@ -
> > 725,6
> > > > > > +727,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);
> > > > > > > > > >
> > > > > > > > > > This is incorrect though. This is no better than just
> > > > > > > > > > waiting for the request, which is not enough since the
> > > > > > > > > > idea is that you need to wait for the context image to
> > > > > > > > > > be completely written to memory before
> > > > > > > > you read it.
> > > > > > > > > > -Chris
> > > > > > > > >
> > > > > > > > > The wait for the context image to be completely written
> > > > > > > > > will be done in the
> > > > > > > > notification from the GVT, by checking the CSB. If put the
> > > > > > > > wait here will made each i915 request to wait, which seems
> > > > > > > > not
> > > necessary.
> > > > > > > >
> > > > > > > > Urm, no. I hope you mean the wait will be on some other
> > > > > > > > thread than inside this interrupt handler.
> > > > > > >
> > > > > > > The SCHEDULE_OUT means to stop GuC to submit another
> request
> > > > > before
> > > > > > the current one is completed by GVT so GVT can manually
> > > > > > restore the
> > > > > MMIO.
> > > > > > So this irq handler should wait until SCHEDULE_OUT is completed.
> > > > > > How it possible to make this irq handler to wait for another
> > > > > > thread? From the current software architecture there is no
> > > > > > other
> > > thread....
> > > > > >
> > > > > > No. It is not acceptable to have any blocking here. Rather you
> > > > > > delegate the polling of CSB to a thread/worker that you kick
> > > > > > off from this
> > > > > notify.
> > > > > > -Chris
> > > > >
> > > > > The major issue is that we should wait for the context image to
> > > > > be completely written to memory before GVT read it. I will
> > > > > double check if we are really reading from context image in this
> > > > > SCHEDULE_OUT event and return back later.
> > > >
> > > > Hi Chris,
> > > >
> > > > Had a discussion with Zhi, actually the context image is accessed
> > > > from the
> > > workload_thread to update the guest context but not directly from
> > > the SCHEDULE_OUT event. So my previous comment is wrong and the
> CSB
> > > waiting should be in workload_thread instead of this IRQ handler.
> > >
> > > That's better, still a little worried about having to remember to
> > > review these hooks later.
> > >
> > > The other concern I express at the start of this was:
> > Sorry, I missed this one.
> >
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c
> > > b/drivers/gpu/drm/i915/intel_lrc.c
> > > index 9e327049b735..5a86abd715df 100644
> > > --- a/drivers/gpu/drm/i915/intel_lrc.c
> > > +++ b/drivers/gpu/drm/i915/intel_lrc.c
> > > @@ -430,16 +430,6 @@ static void execlists_dequeue(struct
> > > intel_engine_cs *engine)
> > >                         if (port != engine->execlist_port)
> > >                                 break;
> > >
> > > -                       /* If GVT overrides us we only ever submit port[0],
> > > -                        * leaving port[1] empty. Note that we also have
> > > -                        * to be careful that we don't queue the same
> > > -                        * context (even though a different request) to
> > > -                        * the second port.
> > > -                        */
> > > -                       if (ctx_single_port_submission(last->ctx) ||
> > > -                           ctx_single_port_submission(cursor->ctx))
> > > -                               break;
> > > -
> > >
> > >
> > > Afaict that requirement is completely bogus (since your workloads do
> > > only active on gvt requests and there will only ever be one
> > > scheduled-in at any
> > > time) and burdensome on non-gvt.
> > > -Chris
> >
> > My understanding about this is to prevent both port[0] and port[1]
> > submitted if any of the last/cursor is a GVT request. Without this
> > check, 1) last is a GVT request in port[0] and cursor is an i915
> > request, this i915 request will be submitted to port[1]; 2) last is an
> > i915 request in port[0] and cursor is a GVT request, this GVT request
> > will be submitted to port[1]; 3) Last is a GVT request from vgpu1 in
> > port[0] and cursor is a GVT request from vgpu2, the GVT request from
> > vgpu2 will be submitted to port[1];
> >
> > Is this ok without the check? Or my understanding is wrong? Please
> > help me to get this. :)
> >
> Hi Chris,
> 
> Had a discussed this with Zhi about removing this check, and we can see after
> removing this check, port[0] and port[1] can be submitted both for the above
> 3 cases. For GVT request, only one of the port can be used and the other one
> should be empty, just like the code comment said. The reason is that GVT still
> needs to do some MMIO restore manually in the SCHEDULE_OUT event
> before i915 starts to handle another request. If removing the check and have
> both port[0] and port[1] submitted, i915 will start to process port[1]
> automatically once port[0] is completed. So this check is meaningful to GVT
> and cannot be removed. Do you agree to keep this check?
> 

Ping for the review feedback.

> >
> > >
> > > --
> > > 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-gvt-dev mailing list
> > intel-gvt-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
> _______________________________________________
> 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

Re: [PATCH v2 2/2] drm/i915/scheduler: add gvt notification for guc

[Dong, Chuanxiao]

Hi Chris,

Ping for your feedback on my comments. Please help to make it to move forward. :)

Thanks
Chuanxiao

> -----Original Message-----
> From: intel-gvt-dev [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On
> Behalf Of Dong, Chuanxiao
> Sent: Thursday, April 13, 2017 7:03 PM
> To: Chris Wilson; intel-gfx@lists.freedesktop.org
> Cc: Zheng, Xiao; Tian, Kevin; joonas.lahtinen@linux.intel.com; intel-gvt-
> dev@lists.freedesktop.org; Wang, Zhi A
> Subject: RE: [PATCH v2 2/2] drm/i915/scheduler: add gvt notification for guc
> 
> > -----Original Message-----
> > From: intel-gvt-dev
> > [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On Behalf Of
> > Dong, Chuanxiao
> > Sent: Wednesday, April 12, 2017 5:12 PM
> > To: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Tian, Kevin <kevin.tian@intel.com>;
> > intel-gvt-dev@lists.freedesktop.org;
> > intel-gfx@lists.freedesktop.org; joonas.lahtinen@linux.intel.com;
> > Zheng, Xiao <xiao.zheng@intel.com>; Wang, Zhi A <zhi.a.wang@intel.com>
> > Subject: RE: [PATCH v2 2/2] drm/i915/scheduler: add gvt notification
> > for guc
> >
> >
> >
> > > -----Original Message-----
> > > From: intel-gvt-dev
> > > [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On Behalf Of
> > > Chris Wilson
> > > Sent: Wednesday, April 12, 2017 4:22 PM
> > > To: Dong, Chuanxiao <chuanxiao.dong@intel.com>
> > > Cc: Tian, Kevin <kevin.tian@intel.com>;
> > > intel-gvt-dev@lists.freedesktop.org;
> > > intel-gfx@lists.freedesktop.org; joonas.lahtinen@linux.intel.com;
> > > Zheng, Xiao <xiao.zheng@intel.com>; Wang, Zhi A
> > > <zhi.a.wang@intel.com>
> > > Subject: Re: [PATCH v2 2/2] drm/i915/scheduler: add gvt notification
> > > for guc
> > >
> > > On Mon, Apr 10, 2017 at 02:40:24AM +0000, Dong, Chuanxiao wrote:
> > > > > -----Original Message-----
> > > > > From: intel-gvt-dev
> > > > > [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On Behalf
> > > > > Of Dong, Chuanxiao
> > > > > Sent: Thursday, April 6, 2017 11:19 PM
> > > > > To: Chris Wilson <chris@chris-wilson.co.uk>
> > > > > Cc: Tian, Kevin <kevin.tian@intel.com>;
> > > > > intel-gvt-dev@lists.freedesktop.org;
> > > > > intel-gfx@lists.freedesktop.org;
> > > > > joonas.lahtinen@linux.intel.com; Zheng, Xiao
> > > > > <xiao.zheng@intel.com>; Wang, Zhi A <zhi.a.wang@intel.com>
> > > > > Subject: RE: [PATCH v2 2/2] drm/i915/scheduler: add gvt
> > > > > notification for guc
> > > > >
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
> > > > > > Sent: Thursday, April 6, 2017 11:07 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;
> > > > > > Wang, Zhi A
> > > > > > Subject: Re: [PATCH v2 2/2] drm/i915/scheduler: add gvt
> > > > > > notification for guc
> > > > > >
> > > > > > On Thu, Apr 06, 2017 at 02:49:54PM +0000, Dong, Chuanxiao wrote:
> > > > > > >
> > > > > > >
> > > > > > > > -----Original Message-----
> > > > > > > > From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
> > > > > > > > Sent: Thursday, April 6, 2017 10:19 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 v2 2/2] drm/i915/scheduler: add gvt
> > > > > > > > notification for guc
> > > > > > > >
> > > > > > > > On Thu, Apr 06, 2017 at 02:05:15PM +0000, Dong, Chuanxiao
> > wrote:
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > > -----Original Message-----
> > > > > > > > > > From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
> > > > > > > > > > Sent: Thursday, April 6, 2017 9:32 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 v2 2/2] drm/i915/scheduler: add
> > > > > > > > > > gvt notification for guc
> > > > > > > > > >
> > > > > > > > > > On Tue, Mar 28, 2017 at 05:38:41PM +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.
> > > > > > > > > > >
> > > > > > > > > > > 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           | 12
> ++++++++++++
> > > > > > > > > > >  drivers/gpu/drm/i915/intel_lrc.c           | 21 +++---------------
> -
> > --
> > > > > > > > > > >  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 58087630..d8a5942 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); @@ -
> > 725,6
> > > > > > +727,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);
> > > > > > > > > >
> > > > > > > > > > This is incorrect though. This is no better than just
> > > > > > > > > > waiting for the request, which is not enough since the
> > > > > > > > > > idea is that you need to wait for the context image to
> > > > > > > > > > be completely written to memory before
> > > > > > > > you read it.
> > > > > > > > > > -Chris
> > > > > > > > >
> > > > > > > > > The wait for the context image to be completely written
> > > > > > > > > will be done in the
> > > > > > > > notification from the GVT, by checking the CSB. If put the
> > > > > > > > wait here will made each i915 request to wait, which seems
> > > > > > > > not
> > > necessary.
> > > > > > > >
> > > > > > > > Urm, no. I hope you mean the wait will be on some other
> > > > > > > > thread than inside this interrupt handler.
> > > > > > >
> > > > > > > The SCHEDULE_OUT means to stop GuC to submit another
> request
> > > > > before
> > > > > > the current one is completed by GVT so GVT can manually
> > > > > > restore the
> > > > > MMIO.
> > > > > > So this irq handler should wait until SCHEDULE_OUT is completed.
> > > > > > How it possible to make this irq handler to wait for another
> > > > > > thread? From the current software architecture there is no
> > > > > > other
> > > thread....
> > > > > >
> > > > > > No. It is not acceptable to have any blocking here. Rather you
> > > > > > delegate the polling of CSB to a thread/worker that you kick
> > > > > > off from this
> > > > > notify.
> > > > > > -Chris
> > > > >
> > > > > The major issue is that we should wait for the context image to
> > > > > be completely written to memory before GVT read it. I will
> > > > > double check if we are really reading from context image in this
> > > > > SCHEDULE_OUT event and return back later.
> > > >
> > > > Hi Chris,
> > > >
> > > > Had a discussion with Zhi, actually the context image is accessed
> > > > from the
> > > workload_thread to update the guest context but not directly from
> > > the SCHEDULE_OUT event. So my previous comment is wrong and the
> CSB
> > > waiting should be in workload_thread instead of this IRQ handler.
> > >
> > > That's better, still a little worried about having to remember to
> > > review these hooks later.
> > >
> > > The other concern I express at the start of this was:
> > Sorry, I missed this one.
> >
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c
> > > b/drivers/gpu/drm/i915/intel_lrc.c
> > > index 9e327049b735..5a86abd715df 100644
> > > --- a/drivers/gpu/drm/i915/intel_lrc.c
> > > +++ b/drivers/gpu/drm/i915/intel_lrc.c
> > > @@ -430,16 +430,6 @@ static void execlists_dequeue(struct
> > > intel_engine_cs *engine)
> > >                         if (port != engine->execlist_port)
> > >                                 break;
> > >
> > > -                       /* If GVT overrides us we only ever submit port[0],
> > > -                        * leaving port[1] empty. Note that we also have
> > > -                        * to be careful that we don't queue the same
> > > -                        * context (even though a different request) to
> > > -                        * the second port.
> > > -                        */
> > > -                       if (ctx_single_port_submission(last->ctx) ||
> > > -                           ctx_single_port_submission(cursor->ctx))
> > > -                               break;
> > > -
> > >
> > >
> > > Afaict that requirement is completely bogus (since your workloads do
> > > only active on gvt requests and there will only ever be one
> > > scheduled-in at any
> > > time) and burdensome on non-gvt.
> > > -Chris
> >
> > My understanding about this is to prevent both port[0] and port[1]
> > submitted if any of the last/cursor is a GVT request. Without this
> > check, 1) last is a GVT request in port[0] and cursor is an i915
> > request, this i915 request will be submitted to port[1]; 2) last is an
> > i915 request in port[0] and cursor is a GVT request, this GVT request
> > will be submitted to port[1]; 3) Last is a GVT request from vgpu1 in
> > port[0] and cursor is a GVT request from vgpu2, the GVT request from
> > vgpu2 will be submitted to port[1];
> >
> > Is this ok without the check? Or my understanding is wrong? Please
> > help me to get this. :)
> >
> Hi Chris,
> 
> Had a discussed this with Zhi about removing this check, and we can see after
> removing this check, port[0] and port[1] can be submitted both for the above
> 3 cases. For GVT request, only one of the port can be used and the other one
> should be empty, just like the code comment said. The reason is that GVT still
> needs to do some MMIO restore manually in the SCHEDULE_OUT event
> before i915 starts to handle another request. If removing the check and have
> both port[0] and port[1] submitted, i915 will start to process port[1]
> automatically once port[0] is completed. So this check is meaningful to GVT
> and cannot be removed. Do you agree to keep this check?
> 
> Thanks
> Chuanxiao
> >
> > >
> > > --
> > > 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-gvt-dev mailing list
> > intel-gvt-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
> _______________________________________________
> 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

[PATCH 1/4] drm/i915: refactor gvt force-single-submission

[Chuanxiao Dong]

refactor gvt force-single-submission to proper files

v1:
make force-single-submission specific to gvt (Chris)
keep the original code implementation (Chris)

Cc: chris@chris-wilson.co.uk
Signed-off-by: Chuanxiao Dong <chuanxiao.dong@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_context.h | 13 +++++++++++++
 drivers/gpu/drm/i915/intel_gvt.h        | 11 +++++++++++
 drivers/gpu/drm/i915/intel_lrc.c        | 25 ++++---------------------
 3 files changed, 28 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_context.h b/drivers/gpu/drm/i915/i915_gem_context.h
index 4af2ab94..2c3afec 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.h
+++ b/drivers/gpu/drm/i915/i915_gem_context.h
@@ -246,6 +246,19 @@ static inline bool i915_gem_context_is_kernel(struct i915_gem_context *ctx)
 	return !ctx->file_priv;
 }
 
+static inline bool
+i915_gem_context_can_merge(const struct i915_gem_context *prev,
+		const struct i915_gem_context *next)
+{
+	if (prev != next)
+		return false;
+
+	if (i915_gem_context_force_single_submission(prev))
+		return false;
+
+	return true;
+}
+
 /* i915_gem_context.c */
 int __must_check i915_gem_context_init(struct drm_i915_private *dev_priv);
 void i915_gem_context_lost(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_gvt.h b/drivers/gpu/drm/i915/intel_gvt.h
index 25df2d6..c0dcd66 100644
--- a/drivers/gpu/drm/i915/intel_gvt.h
+++ b/drivers/gpu/drm/i915/intel_gvt.h
@@ -32,6 +32,12 @@ 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 bool
+intel_gvt_context_single_port_submit(const struct i915_gem_context *ctx)
+{
+	return i915_gem_context_force_single_submission(ctx);
+}
 #else
 static inline int intel_gvt_init(struct drm_i915_private *dev_priv)
 {
@@ -40,6 +46,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 bool
+intel_gvt_context_single_port_submit(const struct i915_gem_context *ctx)
+{
+	return false;
+}
 #endif
 
 #endif /* _INTEL_GVT_H_ */
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 0dc1cc4..61291e9 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -377,24 +377,6 @@ static void execlists_submit_ports(struct intel_engine_cs *engine)
 	writel(lower_32_bits(desc[0]), elsp);
 }
 
-static bool ctx_single_port_submission(const struct i915_gem_context *ctx)
-{
-	return (IS_ENABLED(CONFIG_DRM_I915_GVT) &&
-		i915_gem_context_force_single_submission(ctx));
-}
-
-static bool can_merge_ctx(const struct i915_gem_context *prev,
-			  const struct i915_gem_context *next)
-{
-	if (prev != next)
-		return false;
-
-	if (ctx_single_port_submission(prev))
-		return false;
-
-	return true;
-}
-
 static void execlists_dequeue(struct intel_engine_cs *engine)
 {
 	struct drm_i915_gem_request *last;
@@ -450,7 +432,8 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
 		 * request, and so we never need to tell the hardware about
 		 * the first.
 		 */
-		if (last && !can_merge_ctx(cursor->ctx, last->ctx)) {
+		if (last &&
+			!i915_gem_context_can_merge(last->ctx, cursor->ctx)) {
 			/* If we are on the second port and cannot combine
 			 * this request with the last, then we are done.
 			 */
@@ -463,8 +446,8 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
 			 * context (even though a different request) to
 			 * the second port.
 			 */
-			if (ctx_single_port_submission(last->ctx) ||
-			    ctx_single_port_submission(cursor->ctx))
+			if (intel_gvt_context_single_port_submit(last->ctx) ||
+			    intel_gvt_context_single_port_submit(cursor->ctx))
 				break;
 
 			GEM_BUG_ON(last->ctx == cursor->ctx);
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/4] drm/i915: refactor gvt context notification

[Chuanxiao Dong]

refactor gvt context notification to proper files

v1:
use context_status_change instead of execlists_context_status_change
for better understanding(ZhengXiao)
remove the comment as it is obvious and not friendly to the caller(Kevin)
fix indent issues(Joonas)
rename the context_status_change to intel_gvt_notify_context_status(Chris)
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/intel_gvt.h | 12 ++++++++++++
 drivers/gpu/drm/i915/intel_lrc.c | 21 +++------------------
 2 files changed, 15 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_gvt.h b/drivers/gpu/drm/i915/intel_gvt.h
index c0dcd66..813d0f8 100644
--- a/drivers/gpu/drm/i915/intel_gvt.h
+++ b/drivers/gpu/drm/i915/intel_gvt.h
@@ -38,6 +38,13 @@ intel_gvt_context_single_port_submit(const struct i915_gem_context *ctx)
 {
 	return i915_gem_context_force_single_submission(ctx);
 }
+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)
 {
@@ -51,6 +58,11 @@ intel_gvt_context_single_port_submit(const struct i915_gem_context *ctx)
 {
 	return false;
 }
+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 61291e9..81f9a3b 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]));
@@ -560,7 +545,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

[PATCH 3/4] drm/i915/scheduler: add gvt force-single-submission for guc

[Chuanxiao Dong]

GVT needs single submission and cannot allow merge. So when GuC submitting
a GVT request, the next one should be submitted to guc later until the
previous one is completed. This is following the usage when using execlist
mode submission.

Cc: chris@chris-wilson.co.uk
Signed-off-by: Chuanxiao Dong <chuanxiao.dong@intel.com>
---
 drivers/gpu/drm/i915/i915_guc_submission.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 1642fff..862f4fd 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -668,10 +668,14 @@ static bool i915_guc_dequeue(struct intel_engine_cs *engine)
 		struct drm_i915_gem_request *rq =
 			rb_entry(rb, typeof(*rq), priotree.node);
 
-		if (last && rq->ctx != last->ctx) {
+		if (last && !i915_gem_context_can_merge(last->ctx, rq->ctx)) {
 			if (port != engine->execlist_port)
 				break;
 
+			if (intel_gvt_context_single_port_submit(last->ctx) ||
+				intel_gvt_context_single_port_submit(rq->ctx))
+				break;
+
 			i915_gem_request_assign(&port->request, last);
 			nested_enable_signaling(last);
 			port++;
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 4/4] drm/i915/scheduler: add gvt notification for guc

[Chuanxiao Dong]

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.

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 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 862f4fd..2f3bb16 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);
@@ -712,6 +714,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;
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for drm/i915/scheduler: add gvt force-single-submission and notification for guc (rev4)

[Patchwork]

== Series Details ==

Series: drm/i915/scheduler: add gvt force-single-submission and notification for guc (rev4)
URL   : https://patchwork.freedesktop.org/series/21972/
State : failure

== Summary ==

Series 21972v4 drm/i915/scheduler: add gvt force-single-submission and notification for guc
https://patchwork.freedesktop.org/api/1.0/series/21972/revisions/4/mbox/

Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                pass       -> FAIL       (fi-snb-2600) fdo#100007
Test gem_exec_suspend:
        Subgroup basic-s4-devices:
                pass       -> DMESG-WARN (fi-kbl-7560u) fdo#100125
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-c:
                pass       -> FAIL       (fi-skl-6700k)
Test pm_rpm:
        Subgroup basic-rte:
                pass       -> INCOMPLETE (fi-bsw-n3050) fdo#100718

fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007
fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125
fdo#100718 https://bugs.freedesktop.org/show_bug.cgi?id=100718

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time:431s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time:426s
fi-bsw-n3050     total:242  pass:207  dwarn:0   dfail:0   fail:0   skip:34 
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:519s
fi-bxt-t5700     total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  time:564s
fi-byt-j1900     total:278  pass:254  dwarn:0   dfail:0   fail:0   skip:24  time:491s
fi-byt-n2820     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:482s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:409s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:407s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time:423s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:500s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:470s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:458s
fi-kbl-7560u     total:278  pass:267  dwarn:1   dfail:0   fail:0   skip:10  time:567s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:451s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time:570s
fi-skl-6700k     total:278  pass:255  dwarn:4   dfail:0   fail:1   skip:18  time:457s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:496s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time:429s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:528s
fi-snb-2600      total:278  pass:248  dwarn:0   dfail:0   fail:1   skip:29  time:402s

1b757084743a73fa672e92b4e5cf00a291667dfc drm-tip: 2017y-04m-19d-12h-50m-15s UTC integration manifest
4f12487 drm/i915: refactor gvt force-single-submission

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4521/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✗ Fi.CI.BAT: failure for drm/i915/scheduler: add gvt force-single-submission and notification for guc (rev4)

[Saarinen, Jani]

Hi, 
> == Series Details ==
> 
> Series: drm/i915/scheduler: add gvt force-single-submission and notification
> for guc (rev4)
> URL   : https://patchwork.freedesktop.org/series/21972/
> State : failure
> 
> == Summary ==
> 
> Series 21972v4 drm/i915/scheduler: add gvt force-single-submission and
> notification for guc
> https://patchwork.freedesktop.org/api/1.0/series/21972/revisions/4/mbox/
> 
> Test gem_exec_flush:
>         Subgroup basic-batch-kernel-default-uc:
>                 pass       -> FAIL       (fi-snb-2600) fdo#100007
> Test gem_exec_suspend:
>         Subgroup basic-s4-devices:
>                 pass       -> DMESG-WARN (fi-kbl-7560u) fdo#100125
> Test kms_pipe_crc_basic:
>         Subgroup suspend-read-crc-pipe-c:
>                 pass       -> FAIL       (fi-skl-6700k)
Seems to match https://bugs.freedesktop.org/show_bug.cgi?id=100367
Re-running and marking known for CI. 

> Test pm_rpm:
>         Subgroup basic-rte:
>                 pass       -> INCOMPLETE (fi-bsw-n3050) fdo#100718
> 
> fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007
> fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125
> fdo#100718 https://bugs.freedesktop.org/show_bug.cgi?id=100718
> 
> fi-skl-6700k     total:278  pass:255  dwarn:4   dfail:0   fail:1   skip:18  time:457s
> 
> 1b757084743a73fa672e92b4e5cf00a291667dfc drm-tip: 2017y-04m-19d-12h-
> 50m-15s UTC integration manifest
> 4f12487 drm/i915: refactor gvt force-single-submission
> 
> == Logs ==
> 
> For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4521/


Jani Saarinen
Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✗ Fi.CI.BAT: failure for drm/i915/scheduler: add gvt force-single-submission and notification for guc (rev4)

[Dong, Chuanxiao]

> -----Original Message-----
> From: Saarinen, Jani
> Sent: Thursday, April 20, 2017 2:30 PM
> To: intel-gfx@lists.freedesktop.org; Dong, Chuanxiao
> <chuanxiao.dong@intel.com>
> Subject: RE: [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/scheduler: add gvt
> force-single-submission and notification for guc (rev4)
> 
> Hi,
> > == Series Details ==
> >
> > Series: drm/i915/scheduler: add gvt force-single-submission and
> > notification for guc (rev4)
> > URL   : https://patchwork.freedesktop.org/series/21972/
> > State : failure
> >
> > == Summary ==
> >
> > Series 21972v4 drm/i915/scheduler: add gvt force-single-submission and
> > notification for guc
> > https://patchwork.freedesktop.org/api/1.0/series/21972/revisions/4/mbo
> > x/
> >
> > Test gem_exec_flush:
> >         Subgroup basic-batch-kernel-default-uc:
> >                 pass       -> FAIL       (fi-snb-2600) fdo#100007
> > Test gem_exec_suspend:
> >         Subgroup basic-s4-devices:
> >                 pass       -> DMESG-WARN (fi-kbl-7560u) fdo#100125
> > Test kms_pipe_crc_basic:
> >         Subgroup suspend-read-crc-pipe-c:
> >                 pass       -> FAIL       (fi-skl-6700k)
> Seems to match https://bugs.freedesktop.org/show_bug.cgi?id=100367
> Re-running and marking known for CI.

I see. Thanks Jani. :)

Thanks
Chuanxiao

> 
> > Test pm_rpm:
> >         Subgroup basic-rte:
> >                 pass       -> INCOMPLETE (fi-bsw-n3050) fdo#100718
> >
> > fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007
> > fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125
> > fdo#100718 https://bugs.freedesktop.org/show_bug.cgi?id=100718
> >
> > fi-skl-6700k     total:278  pass:255  dwarn:4   dfail:0   fail:1   skip:18
> time:457s
> >
> > 1b757084743a73fa672e92b4e5cf00a291667dfc drm-tip: 2017y-04m-19d-
> 12h-
> > 50m-15s UTC integration manifest
> > 4f12487 drm/i915: refactor gvt force-single-submission
> >
> > == Logs ==
> >
> > For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4521/
> 
> 
> Jani Saarinen
> Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
> 

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for drm/i915/scheduler: add gvt force-single-submission and notification for guc (rev4)

[Patchwork]

== Series Details ==

Series: drm/i915/scheduler: add gvt force-single-submission and notification for guc (rev4)
URL   : https://patchwork.freedesktop.org/series/21972/
State : failure

== Summary ==

Series 21972v4 drm/i915/scheduler: add gvt force-single-submission and notification for guc
https://patchwork.freedesktop.org/api/1.0/series/21972/revisions/4/mbox/

Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                pass       -> FAIL       (fi-snb-2600) fdo#100007
Test pm_rpm:
        Subgroup basic-pci-d3-state:
                pass       -> INCOMPLETE (fi-byt-j1900)
        Subgroup basic-rte:
                pass       -> INCOMPLETE (fi-bsw-n3050) fdo#100718

fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007
fdo#100718 https://bugs.freedesktop.org/show_bug.cgi?id=100718

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time:431s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time:426s
fi-bsw-n3050     total:242  pass:207  dwarn:0   dfail:0   fail:0   skip:34 
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:510s
fi-bxt-t5700     total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  time:564s
fi-byt-j1900     total:241  pass:217  dwarn:0   dfail:0   fail:0   skip:23 
fi-byt-n2820     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:481s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:409s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:413s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time:425s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:499s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:462s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:457s
fi-kbl-7560u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:561s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:451s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time:568s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time:465s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:497s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time:441s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:531s
fi-snb-2600      total:278  pass:248  dwarn:0   dfail:0   fail:1   skip:29  time:397s

1b757084743a73fa672e92b4e5cf00a291667dfc drm-tip: 2017y-04m-19d-12h-50m-15s UTC integration manifest
03cb65f drm/i915: refactor gvt force-single-submission

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4522/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✗ Fi.CI.BAT: failure for drm/i915/scheduler: add gvt force-single-submission and notification for guc (rev4)

[Saarinen, Jani]

Hi, 
> == Series Details ==
> 
> Series: drm/i915/scheduler: add gvt force-single-submission and notification
> for guc (rev4)
> URL   : https://patchwork.freedesktop.org/series/21972/
> State : failure
> 
> == Summary ==
> 
> Series 21972v4 drm/i915/scheduler: add gvt force-single-submission and
> notification for guc
> https://patchwork.freedesktop.org/api/1.0/series/21972/revisions/4/mbox/
> 
> Test gem_exec_flush:
>         Subgroup basic-batch-kernel-default-uc:
>                 pass       -> FAIL       (fi-snb-2600) fdo#100007
> Test pm_rpm:
>         Subgroup basic-pci-d3-state:
>                 pass       -> INCOMPLETE (fi-byt-j1900)
Right, now seems to match https://bugs.freedesktop.org/show_bug.cgi?id=99740
Marking. Re-testing. 

>         Subgroup basic-rte:
>                 pass       -> INCOMPLETE (fi-bsw-n3050) fdo#100718
> 
> fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007
> fdo#100718 https://bugs.freedesktop.org/show_bug.cgi?id=100718
> 
> fi-byt-j1900     total:241  pass:217  dwarn:0   dfail:0   fail:0   skip:23
> 
> 1b757084743a73fa672e92b4e5cf00a291667dfc drm-tip: 2017y-04m-19d-12h-
> 50m-15s UTC integration manifest 03cb65f drm/i915: refactor gvt force-
> single-submission
> 
> == Logs ==

Jani Saarinen
Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo



_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915/scheduler: add gvt force-single-submission and notification for guc (rev4)

[Patchwork]

== Series Details ==

Series: drm/i915/scheduler: add gvt force-single-submission and notification for guc (rev4)
URL   : https://patchwork.freedesktop.org/series/21972/
State : success

== Summary ==

Series 21972v4 drm/i915/scheduler: add gvt force-single-submission and notification for guc
https://patchwork.freedesktop.org/api/1.0/series/21972/revisions/4/mbox/

Test gem_exec_suspend:
        Subgroup basic-s4-devices:
                pass       -> DMESG-WARN (fi-kbl-7560u) fdo#100125

fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time:432s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time:429s
fi-bsw-n3050     total:278  pass:242  dwarn:0   dfail:0   fail:0   skip:36  time:576s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:505s
fi-bxt-t5700     total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  time:548s
fi-byt-j1900     total:278  pass:254  dwarn:0   dfail:0   fail:0   skip:24  time:485s
fi-byt-n2820     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:481s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:408s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:405s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time:421s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:483s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:495s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:460s
fi-kbl-7560u     total:278  pass:267  dwarn:1   dfail:0   fail:0   skip:10  time:569s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:449s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time:569s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time:460s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:496s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time:429s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:540s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time:399s

1b757084743a73fa672e92b4e5cf00a291667dfc drm-tip: 2017y-04m-19d-12h-50m-15s UTC integration manifest
9f1c42d drm/i915: refactor gvt force-single-submission

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4523/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx