All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [RFC 06/11] drm/i915/guc: Remove extra arguments from guc_client_alloc
  2017-02-23 19:14 ` [RFC 06/11] drm/i915/guc: Remove extra arguments from guc_client_alloc Michał Winiarski
@ 2017-02-23 16:38   ` Oscar Mateo
  2017-02-24  1:39     ` Daniele Ceraolo Spurio
  0 siblings, 1 reply; 19+ messages in thread
From: Oscar Mateo @ 2017-02-23 16:38 UTC (permalink / raw)
  To: Michał Winiarski, intel-gfx



On 02/23/2017 11:14 AM, Michał Winiarski wrote:
> We're always using all engines and kernel context for guc clients, let's
> remove those arguments from guc_client_alloc.
I am quite new to the GuC but, by the look of it, passing the ctx was 
groundwork for direct submission (which means calling guc_client_alloc 
with contexts different than kernel_context). Why not leave it there, 
since someone was careful to lay the groundwork? I don't see an obvious 
use for the engines, though (clients that only have access to some of 
the engines?).

> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_guc_submission.c | 16 ++++------------
>   1 file changed, 4 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> index 6c64ce1..3080735 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -838,21 +838,15 @@ static void guc_init_doorbell_hw(struct intel_guc *guc)
>   /**
>    * guc_client_alloc() - Allocate an i915_guc_client
>    * @dev_priv:	driver private data structure
> - * @engines:	The set of engines to enable for this client
>    * @priority:	four levels priority _CRITICAL, _HIGH, _NORMAL and _LOW
>    * 		The kernel client to replace ExecList submission is created with
>    * 		NORMAL priority. Priority of a client for scheduler can be HIGH,
>    * 		while a preemption context can use CRITICAL.
> - * @ctx:	the context that owns the client (we use the default render
> - * 		context)
> - *
>    * Return:	An i915_guc_client object if success, else NULL.
>    */
>   static struct i915_guc_client *
>   guc_client_alloc(struct drm_i915_private *dev_priv,
> -		 uint32_t engines,
> -		 uint32_t priority,
> -		 struct i915_gem_context *ctx)
> +		 uint32_t priority)
>   {
>   	struct i915_guc_client *client;
>   	struct intel_guc *guc = &dev_priv->guc;
> @@ -864,9 +858,9 @@ guc_client_alloc(struct drm_i915_private *dev_priv,
>   	if (!client)
>   		return NULL;
>   
> -	client->owner = ctx;
> +	client->owner = dev_priv->kernel_context;
>   	client->guc = guc;
> -	client->engines = engines;
> +	client->engines = INTEL_INFO(dev_priv)->ring_mask;
>   	client->priority = priority;
>   	client->doorbell_id = GUC_INVALID_DOORBELL_ID;
>   
> @@ -1062,9 +1056,7 @@ int i915_guc_submission_init(struct drm_i915_private *dev_priv)
>   	guc_addon_create(guc);
>   
>   	guc->execbuf_client = guc_client_alloc(dev_priv,
> -					       INTEL_INFO(dev_priv)->ring_mask,
> -					       GUC_CTX_PRIORITY_KMD_NORMAL,
> -					       dev_priv->kernel_context);
> +					       GUC_CTX_PRIORITY_KMD_NORMAL);
>   	if (!guc->execbuf_client) {
>   		DRM_ERROR("Failed to create GuC client for execbuf!\n");
>   		goto err;

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [RFC] GuC based preemption
@ 2017-02-23 19:08 Michał Winiarski
  2017-02-23 19:08 ` [RFC 01/11] drm/i915/scheduler: Remember request priority throughout its lifetime Michał Winiarski
                   ` (10 more replies)
  0 siblings, 11 replies; 19+ messages in thread
From: Michał Winiarski @ 2017-02-23 19:08 UTC (permalink / raw)
  To: intel-gfx

Now that we're able to unsubmit requests, let's try to actually preempt.

The series is partially based on "Preemption support for GPU scheduler":
https://lists.freedesktop.org/archives/intel-gfx/2015-December/082817.html

It requires "drm/i915/scheduler: Support user-defined priorities"

It's still not very mature, I'm observing GPU hangs with basic sanity checks
(create low_prio ctx, do work, create high_prio ctx, do work, expect that
high_prio finished before low_prio, repeat) due to incorrect handling of
the preemptive requests sent to GuC.

What I'd like to discuss is the overall approach and userspace interactions.
When considering preemption I've stayed with the "absolute" threshold approach
(we're only considering requests with priority higher than some threshold),
though I'm not sure whether it's the right way of doing things (since userspace
applications won't be able to increase their priority without CAP_SYS_ADMIN).
Perhaps it would be better to track the highest priority of the inflight
requests on each engine and consider preemption relative to that?

There's also the question of whether we want to have an "opt-in" interface for
userspace to explicitly state "I'm ready to handle preemption".
We know that we can safely preempt on the batch buffer boundary, unfortunately
when we try to preempt in the middle of user batches, there are cases where the
default settings are "unsafe" (e.g. require different batch buffer programming
from the userspace), which is why there seems to be a preference towards an
opt-in ABI (either by execbuf flag or context param).
The preemption granularity is being controlled through whitelisted
GEN8_CS_CHICKEN1 register, maybe we could get away with programming a "safe"
default values instead?

Awaiting for feedback!

-Michał

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [RFC 01/11] drm/i915/scheduler: Remember request priority throughout its lifetime
  2017-02-23 19:08 [RFC] GuC based preemption Michał Winiarski
@ 2017-02-23 19:08 ` Michał Winiarski
  2017-02-23 21:29   ` Chris Wilson
  2017-02-23 19:08 ` [RFC 02/11] drm/i915/preempt: Add module parameter for preemption Michał Winiarski
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 19+ messages in thread
From: Michał Winiarski @ 2017-02-23 19:08 UTC (permalink / raw)
  To: intel-gfx

Since request can be cancelled, we need to avoid overriding its priority
during submission to be able to resubmit it.

Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
---
 drivers/gpu/drm/i915/i915_guc_submission.c | 1 -
 drivers/gpu/drm/i915/intel_lrc.c           | 1 -
 2 files changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 89ee0f4..2c76377 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -589,7 +589,6 @@ static bool i915_guc_dequeue(struct intel_engine_cs *engine)
 		rb = rb_next(rb);
 		rb_erase(&cursor->priotree.node, &engine->execlist_queue);
 		RB_CLEAR_NODE(&cursor->priotree.node);
-		cursor->priotree.priority = INT_MAX;
 
 		i915_guc_submit(cursor);
 		last = cursor;
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 6e76d11..d4cfaa1 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -476,7 +476,6 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
 		rb = rb_next(rb);
 		rb_erase(&cursor->priotree.node, &engine->execlist_queue);
 		RB_CLEAR_NODE(&cursor->priotree.node);
-		cursor->priotree.priority = INT_MAX;
 
 		__i915_gem_request_submit(cursor);
 		trace_i915_gem_request_in(cursor, port - engine->execlist_port);
-- 
2.9.3

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC 02/11] drm/i915/preempt: Add module parameter for preemption
  2017-02-23 19:08 [RFC] GuC based preemption Michał Winiarski
  2017-02-23 19:08 ` [RFC 01/11] drm/i915/scheduler: Remember request priority throughout its lifetime Michał Winiarski
@ 2017-02-23 19:08 ` Michał Winiarski
  2017-02-23 19:08 ` [RFC 03/11] drm/i915/preempt: Add information needed to track engine preempt state Michał Winiarski
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Michał Winiarski @ 2017-02-23 19:08 UTC (permalink / raw)
  To: intel-gfx

We're going to support preemption on platforms where GuC submission is
enabled.

Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c            |  6 ++++++
 drivers/gpu/drm/i915/i915_drv.h            |  3 +++
 drivers/gpu/drm/i915/i915_gem.c            | 22 ++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_guc_submission.c |  8 ++++++++
 drivers/gpu/drm/i915/i915_params.c         |  5 +++++
 drivers/gpu/drm/i915/i915_params.h         |  1 +
 drivers/gpu/drm/i915/intel_ringbuffer.h    |  3 +++
 7 files changed, 48 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index b76e8f7..e2c0bec 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -994,6 +994,12 @@ static void intel_sanitize_options(struct drm_i915_private *dev_priv)
 
 	i915.semaphores = intel_sanitize_semaphores(dev_priv, i915.semaphores);
 	DRM_DEBUG_DRIVER("use GPU semaphores? %s\n", yesno(i915.semaphores));
+
+	i915.enable_preemption =
+		intel_sanitize_enable_preemption(dev_priv,
+						 i915.enable_preemption);
+	DRM_DEBUG_DRIVER("preemption enabled? %s\n",
+			 yesno(i915.enable_preemption));
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index eed9ead..f26c42c5 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2963,6 +2963,9 @@ int intel_sanitize_enable_ppgtt(struct drm_i915_private *dev_priv,
 
 bool intel_sanitize_semaphores(struct drm_i915_private *dev_priv, int value);
 
+bool intel_sanitize_enable_preemption(struct drm_i915_private *dev_priv,
+				      int enable_preemption);
+
 /* i915_drv.c */
 void __printf(3, 4)
 __i915_printk(struct drm_i915_private *dev_priv, const char *level,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index a816700..9213425 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4482,6 +4482,28 @@ bool intel_sanitize_semaphores(struct drm_i915_private *dev_priv, int value)
 	return true;
 }
 
+/**
+ * intel_sanitize_enable_preemption() - sanitize i915.enable_preemption
+ * @dev_priv: i915 device private
+ * @enable_preemption: value of i915.enable_preemption module parameter.
+ *
+ * We're currently only supporting preemption when GuC submission is being
+ * used.
+ *
+ * Return: true if preemption is supported and has to be enabled.
+ */
+bool intel_sanitize_enable_preemption(struct drm_i915_private *dev_priv,
+				      int enable_preemption)
+{
+	if (!i915.enable_guc_submission)
+		return false;
+
+	if (enable_preemption >= 0)
+		return enable_preemption;
+
+	return false;
+}
+
 int i915_gem_init(struct drm_i915_private *dev_priv)
 {
 	int ret;
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 2c76377..feccd65 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -489,6 +489,11 @@ static int guc_ring_doorbell(struct i915_guc_client *client)
 	return ret;
 }
 
+static int i915_guc_preempt_noop(struct intel_engine_cs *engine)
+{
+	return 0;
+}
+
 /**
  * __i915_guc_submit() - Submit commands through GuC
  * @rq:		request associated with the commands
@@ -1043,6 +1048,9 @@ int i915_guc_submission_enable(struct drm_i915_private *dev_priv)
 		 */
 		engine->irq_tasklet.func = i915_guc_irq_handler;
 
+		if (i915.enable_preemption)
+			engine->preempt = i915_guc_preempt_noop;
+
 		/* Replay the current set of previously submitted requests */
 		spin_lock_irqsave(&engine->timeline->lock, flags);
 		list_for_each_entry(rq, &engine->timeline->requests, link) {
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 2e9645e..c5b6929 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -63,6 +63,7 @@ struct i915_params i915 __read_mostly = {
 	.inject_load_failure = 0,
 	.enable_dpcd_backlight = false,
 	.enable_gvt = false,
+	.enable_preemption = 0,
 };
 
 module_param_named(modeset, i915.modeset, int, 0400);
@@ -243,3 +244,7 @@ MODULE_PARM_DESC(enable_dpcd_backlight,
 module_param_named(enable_gvt, i915.enable_gvt, bool, 0400);
 MODULE_PARM_DESC(enable_gvt,
 	"Enable support for Intel GVT-g graphics virtualization host support(default:false)");
+
+module_param_named_unsafe(enable_preemption, i915.enable_preemption, int, 0400);
+MODULE_PARM_DESC(enable_preemption,
+	"Enable preemption (0=disabled [default], 1=enabled)");
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index 55d47ee..e0f2f7e 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -49,6 +49,7 @@
 	func(int, use_mmio_flip); \
 	func(int, mmio_debug); \
 	func(int, edp_vswing); \
+	func(int, enable_preemption); \
 	func(unsigned int, inject_load_failure); \
 	/* leave bools at the end to not create holes */ \
 	func(bool, alpha_support); \
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 0f29e07..9c7f36e 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -312,6 +312,9 @@ struct intel_engine_cs {
 	void		(*schedule)(struct drm_i915_gem_request *request,
 				    int priority);
 
+	/* Called to attempt preemption of currently executing workload */
+	int		(*preempt)(struct intel_engine_cs *engine);
+
 	/* Some chipsets are not quite as coherent as advertised and need
 	 * an expensive kick to force a true read of the up-to-date seqno.
 	 * However, the up-to-date seqno is not always required and the last
-- 
2.9.3

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC 03/11] drm/i915/preempt: Add information needed to track engine preempt state
  2017-02-23 19:08 [RFC] GuC based preemption Michał Winiarski
  2017-02-23 19:08 ` [RFC 01/11] drm/i915/scheduler: Remember request priority throughout its lifetime Michał Winiarski
  2017-02-23 19:08 ` [RFC 02/11] drm/i915/preempt: Add module parameter for preemption Michał Winiarski
@ 2017-02-23 19:08 ` Michał Winiarski
  2017-02-23 19:08 ` [RFC 04/11] drm/i915/preempt: Implement null preemption method Michał Winiarski
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Michał Winiarski @ 2017-02-23 19:08 UTC (permalink / raw)
  To: intel-gfx

We're using engine->preempt_requested to mark that preemption has
started, and new hws entry allowing HW to mark that preemption has
finished and the engine is idle, allowing us to do the postprocessing.

Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
---
 drivers/gpu/drm/i915/intel_ringbuffer.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 9c7f36e..637a83c 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -392,6 +392,8 @@ struct intel_engine_cs {
 	struct rb_node *execlist_first;
 	unsigned int fw_domains;
 
+	bool preempt_requested;
+
 	/* Contexts are pinned whilst they are active on the GPU. The last
 	 * context executed remains active whilst the GPU is idle - the
 	 * switch away and write to the context object only occurs on the
@@ -486,9 +488,17 @@ intel_write_status_page(struct intel_engine_cs *engine,
  */
 #define I915_GEM_HWS_INDEX		0x30
 #define I915_GEM_HWS_INDEX_ADDR (I915_GEM_HWS_INDEX << MI_STORE_DWORD_INDEX_SHIFT)
+#define I915_GEM_HWS_PREEMPT_INDEX	0x32
+#define I915_GEM_HWS_PREEMPT_ADDR (I915_GEM_HWS_PREEMPT_INDEX << MI_STORE_DWORD_INDEX_SHIFT)
 #define I915_GEM_HWS_SCRATCH_INDEX	0x40
 #define I915_GEM_HWS_SCRATCH_ADDR (I915_GEM_HWS_SCRATCH_INDEX << MI_STORE_DWORD_INDEX_SHIFT)
 
+static inline bool
+intel_engine_is_preempt_finished(struct intel_engine_cs *engine)
+{
+	return intel_read_status_page(engine, I915_GEM_HWS_PREEMPT_INDEX);
+}
+
 struct intel_ring *
 intel_engine_create_ring(struct intel_engine_cs *engine, int size);
 int intel_ring_pin(struct intel_ring *ring, unsigned int offset_bias);
-- 
2.9.3

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC 04/11] drm/i915/preempt: Implement null preemption method
  2017-02-23 19:08 [RFC] GuC based preemption Michał Winiarski
                   ` (2 preceding siblings ...)
  2017-02-23 19:08 ` [RFC 03/11] drm/i915/preempt: Add information needed to track engine preempt state Michał Winiarski
@ 2017-02-23 19:08 ` Michał Winiarski
  2017-02-23 21:37   ` Chris Wilson
  2017-02-23 19:14 ` [RFC 05/11] drm/i915/preempt: Handle preemption event in guc tasklet Michał Winiarski
                   ` (6 subsequent siblings)
  10 siblings, 1 reply; 19+ messages in thread
From: Michał Winiarski @ 2017-02-23 19:08 UTC (permalink / raw)
  To: intel-gfx

We're only requesting preemption for requests that have high enough
priority (above threshold). Currently we're also ignoring requests that
have dependencies on different engines.

Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
---
 drivers/gpu/drm/i915/i915_guc_submission.c |  3 ++
 drivers/gpu/drm/i915/intel_lrc.c           | 81 +++++++++++++++++++++++++++++-
 2 files changed, 83 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index feccd65..6d9431d 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -491,6 +491,9 @@ static int guc_ring_doorbell(struct i915_guc_client *client)
 
 static int i915_guc_preempt_noop(struct intel_engine_cs *engine)
 {
+	engine->preempt_requested = false;
+	intel_write_status_page(engine, I915_GEM_HWS_PREEMPT_INDEX, 0);
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index d4cfaa1..869b96e 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -674,16 +674,93 @@ pt_lock_engine(struct i915_priotree *pt, struct intel_engine_cs *locked)
 	return engine;
 }
 
+#define EXECLISTS_PREEMPT_THRESHOLD 512
+
+static void __execlists_try_preempt(struct intel_engine_cs *engine,
+				  int prio)
+{
+	struct drm_i915_gem_request *rq;
+	int highest_prio = INT_MIN;
+	int ret;
+
+	spin_lock_irq(&engine->timeline->lock);
+
+	/* Engine is idle */
+	if (execlists_elsp_idle(engine))
+		goto out_unlock;
+
+	if (engine->preempt_requested)
+		goto out_unlock;
+
+	list_for_each_entry_reverse(rq, &engine->timeline->requests, link) {
+		if (!i915_gem_request_completed(rq)) {
+			highest_prio = (rq->priotree.priority > highest_prio) ?
+				 rq->priotree.priority : highest_prio;
+		} else
+			break;
+	}
+
+	/* Bail out if our priority is lower than any of the inflight requests
+	 * (also if there are none requests) */
+	if (highest_prio == INT_MIN || prio <= highest_prio)
+		goto out_unlock;
+
+	engine->preempt_requested = true;
+
+	spin_unlock_irq(&engine->timeline->lock);
+
+	ret = engine->preempt(engine);
+	if (ret) {
+		spin_lock_irq(&engine->timeline->lock);
+		engine->preempt_requested = false;
+		spin_unlock_irq(&engine->timeline->lock);
+	}
+
+	return;
+
+out_unlock:
+	spin_unlock_irq(&engine->timeline->lock);
+}
+
+static void execlists_try_preempt(struct intel_engine_cs *engine,
+				     int prio,
+				     unsigned long *engines_bumped)
+{
+	int num_engines_bumped = bitmap_weight(engines_bumped,
+					       I915_NUM_ENGINES);
+
+	/* Preemption is disabled */
+	if (!engine->preempt)
+		return;
+
+	/* We're not a high priority request */
+	if (prio < EXECLISTS_PREEMPT_THRESHOLD)
+		return;
+
+	/* We have dependencies on many engines */
+	if (num_engines_bumped > 1)
+		return;
+
+	/* We have dependency on a single engine - but it's not our engine */
+	if (num_engines_bumped == 1 && !test_bit(engine->id, engines_bumped))
+		return;
+
+	__execlists_try_preempt(engine, prio);
+}
+
 static void execlists_schedule(struct drm_i915_gem_request *request, int prio)
 {
 	struct intel_engine_cs *engine = NULL;
 	struct i915_dependency *dep, *p;
 	struct i915_dependency stack;
 	LIST_HEAD(dfs);
+	DECLARE_BITMAP(engine_bumped, I915_NUM_ENGINES);
 
 	if (prio <= READ_ONCE(request->priotree.priority))
 		return;
 
+	bitmap_zero(engine_bumped, I915_NUM_ENGINES);
+
 	/* Need BKL in order to use the temporary link inside i915_dependency */
 	lockdep_assert_held(&request->i915->drm.struct_mutex);
 
@@ -719,6 +796,7 @@ static void execlists_schedule(struct drm_i915_gem_request *request, int prio)
 			continue;
 
 		engine = pt_lock_engine(pt, engine);
+		__set_bit(engine->id, engine_bumped);
 
 		/* If it is not already in the rbtree, we can update the
 		 * priority inplace and skip over it (and its dependencies)
@@ -737,6 +815,7 @@ static void execlists_schedule(struct drm_i915_gem_request *request, int prio)
 		INIT_LIST_HEAD(&dep->dfs_link);
 
 		engine = pt_lock_engine(pt, engine);
+		__set_bit(engine->id, engine_bumped);
 
 		if (prio <= pt->priority)
 			continue;
@@ -752,7 +831,7 @@ static void execlists_schedule(struct drm_i915_gem_request *request, int prio)
 	if (engine)
 		spin_unlock_irq(&engine->timeline->lock);
 
-	/* XXX Do we need to preempt to make room for us and our deps? */
+	execlists_try_preempt(engine, prio, engine_bumped);
 }
 
 static int execlists_context_pin(struct intel_engine_cs *engine,
-- 
2.9.3

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC 05/11] drm/i915/preempt: Handle preemption event in guc tasklet
  2017-02-23 19:08 [RFC] GuC based preemption Michał Winiarski
                   ` (3 preceding siblings ...)
  2017-02-23 19:08 ` [RFC 04/11] drm/i915/preempt: Implement null preemption method Michał Winiarski
@ 2017-02-23 19:14 ` Michał Winiarski
  2017-03-01 12:57   ` Chris Wilson
  2017-02-23 19:14 ` [RFC 06/11] drm/i915/guc: Remove extra arguments from guc_client_alloc Michał Winiarski
                   ` (5 subsequent siblings)
  10 siblings, 1 reply; 19+ messages in thread
From: Michał Winiarski @ 2017-02-23 19:14 UTC (permalink / raw)
  To: intel-gfx

We need to avoid sending new work if the preemption is in progress.
Once it finished, we need to identify and unsubmit the preempted
workload, submit new workload (potentially the one responsible for
preemption) and resubmit the preempted workload.

Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_request.h    |  2 +
 drivers/gpu/drm/i915/i915_guc_submission.c | 64 ++++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem_request.h b/drivers/gpu/drm/i915/i915_gem_request.h
index 5f73d8c..a78408dd 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.h
+++ b/drivers/gpu/drm/i915/i915_gem_request.h
@@ -170,6 +170,8 @@ struct drm_i915_gem_request {
 	/** Time at which this request was emitted, in jiffies. */
 	unsigned long emitted_jiffies;
 
+	struct list_head resubmit_link;
+
 	/** engine->request_list entry for this request */
 	struct list_head link;
 
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 6d9431d..6c64ce1 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -580,6 +580,10 @@ static bool i915_guc_dequeue(struct intel_engine_cs *engine)
 	bool submit = false;
 
 	spin_lock_irqsave(&engine->timeline->lock, flags);
+
+	if (engine->preempt_requested)
+		goto out;
+
 	rb = engine->execlist_first;
 	while (rb) {
 		struct drm_i915_gem_request *cursor =
@@ -607,17 +611,71 @@ static bool i915_guc_dequeue(struct intel_engine_cs *engine)
 		nested_enable_signaling(last);
 		engine->execlist_first = rb;
 	}
+out:
 	spin_unlock_irqrestore(&engine->timeline->lock, flags);
 
 	return submit;
 }
 
+static void unsubmit_inflight_requests(struct intel_engine_cs *engine,
+					 struct list_head *resubmit)
+{
+	struct drm_i915_gem_request *rq, *prev;
+
+	assert_spin_locked(&engine->timeline->lock);
+
+	list_for_each_entry_safe_reverse(rq, prev,
+					 &engine->timeline->requests, link) {
+		if (!i915_gem_request_completed(rq)) {
+			i915_gem_request_get(rq);
+			__i915_gem_request_unsubmit(rq);
+			list_add(&rq->resubmit_link, resubmit);
+		}
+	}
+}
+
+static void resubmit_inflight_requests(struct intel_engine_cs *engine,
+					 struct list_head *resubmit)
+{
+	struct drm_i915_gem_request *rq, *prev;
+
+	assert_spin_locked(&engine->timeline->lock);
+
+	list_for_each_entry_safe(rq, prev, resubmit, resubmit_link) {
+		list_del(&rq->resubmit_link);
+		__i915_gem_request_submit(rq);
+		i915_gem_request_put(rq);
+	}
+}
+
 static void i915_guc_irq_handler(unsigned long data)
 {
 	struct intel_engine_cs *engine = (struct intel_engine_cs *)data;
 	struct execlist_port *port = engine->execlist_port;
 	struct drm_i915_gem_request *rq;
+	unsigned long flags;
 	bool submit;
+	LIST_HEAD(resubmit);
+
+	spin_lock_irqsave(&engine->timeline->lock, flags);
+	if (engine->preempt_requested &&
+	    intel_engine_is_preempt_finished(engine)) {
+		unsubmit_inflight_requests(engine, &resubmit);
+		engine->preempt_requested = false;
+		intel_write_status_page(engine, I915_GEM_HWS_PREEMPT_INDEX, 0);
+
+		rq = port[0].request;
+		while(rq && i915_gem_request_completed(rq)) {
+			i915_gem_request_put(rq);
+			if (port != engine->execlist_port)
+				break;
+			port++;
+		}
+		port = engine->execlist_port;
+		port[0].request = NULL;
+		port[1].request = NULL;
+	}
+	spin_unlock_irqrestore(&engine->timeline->lock, flags);
 
 	do {
 		rq = port[0].request;
@@ -632,6 +690,12 @@ static void i915_guc_irq_handler(unsigned long data)
 		if (!port[1].request)
 			submit = i915_guc_dequeue(engine);
 	} while (submit);
+
+	if (!list_empty(&resubmit)) {
+		spin_lock_irqsave(&engine->timeline->lock, flags);
+		resubmit_inflight_requests(engine, &resubmit);
+		spin_unlock_irqrestore(&engine->timeline->lock, flags);
+	}
 }
 
 /*
-- 
2.9.3

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC 06/11] drm/i915/guc: Remove extra arguments from guc_client_alloc
  2017-02-23 19:08 [RFC] GuC based preemption Michał Winiarski
                   ` (4 preceding siblings ...)
  2017-02-23 19:14 ` [RFC 05/11] drm/i915/preempt: Handle preemption event in guc tasklet Michał Winiarski
@ 2017-02-23 19:14 ` Michał Winiarski
  2017-02-23 16:38   ` Oscar Mateo
  2017-02-23 19:14 ` [RFC 07/11] drm/i915/guc: Add a second client, to be used for preemption Michał Winiarski
                   ` (4 subsequent siblings)
  10 siblings, 1 reply; 19+ messages in thread
From: Michał Winiarski @ 2017-02-23 19:14 UTC (permalink / raw)
  To: intel-gfx

We're always using all engines and kernel context for guc clients, let's
remove those arguments from guc_client_alloc.

Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
---
 drivers/gpu/drm/i915/i915_guc_submission.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 6c64ce1..3080735 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -838,21 +838,15 @@ static void guc_init_doorbell_hw(struct intel_guc *guc)
 /**
  * guc_client_alloc() - Allocate an i915_guc_client
  * @dev_priv:	driver private data structure
- * @engines:	The set of engines to enable for this client
  * @priority:	four levels priority _CRITICAL, _HIGH, _NORMAL and _LOW
  * 		The kernel client to replace ExecList submission is created with
  * 		NORMAL priority. Priority of a client for scheduler can be HIGH,
  * 		while a preemption context can use CRITICAL.
- * @ctx:	the context that owns the client (we use the default render
- * 		context)
- *
  * Return:	An i915_guc_client object if success, else NULL.
  */
 static struct i915_guc_client *
 guc_client_alloc(struct drm_i915_private *dev_priv,
-		 uint32_t engines,
-		 uint32_t priority,
-		 struct i915_gem_context *ctx)
+		 uint32_t priority)
 {
 	struct i915_guc_client *client;
 	struct intel_guc *guc = &dev_priv->guc;
@@ -864,9 +858,9 @@ guc_client_alloc(struct drm_i915_private *dev_priv,
 	if (!client)
 		return NULL;
 
-	client->owner = ctx;
+	client->owner = dev_priv->kernel_context;
 	client->guc = guc;
-	client->engines = engines;
+	client->engines = INTEL_INFO(dev_priv)->ring_mask;
 	client->priority = priority;
 	client->doorbell_id = GUC_INVALID_DOORBELL_ID;
 
@@ -1062,9 +1056,7 @@ int i915_guc_submission_init(struct drm_i915_private *dev_priv)
 	guc_addon_create(guc);
 
 	guc->execbuf_client = guc_client_alloc(dev_priv,
-					       INTEL_INFO(dev_priv)->ring_mask,
-					       GUC_CTX_PRIORITY_KMD_NORMAL,
-					       dev_priv->kernel_context);
+					       GUC_CTX_PRIORITY_KMD_NORMAL);
 	if (!guc->execbuf_client) {
 		DRM_ERROR("Failed to create GuC client for execbuf!\n");
 		goto err;
-- 
2.9.3

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC 07/11] drm/i915/guc: Add a second client, to be used for preemption
  2017-02-23 19:08 [RFC] GuC based preemption Michał Winiarski
                   ` (5 preceding siblings ...)
  2017-02-23 19:14 ` [RFC 06/11] drm/i915/guc: Remove extra arguments from guc_client_alloc Michał Winiarski
@ 2017-02-23 19:14 ` Michał Winiarski
  2017-02-23 19:14 ` [RFC 08/11] drm/i915/guc: Add preemption action to GuC firmware interface Michał Winiarski
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Michał Winiarski @ 2017-02-23 19:14 UTC (permalink / raw)
  To: intel-gfx; +Cc: Dave Gordon

From: Dave Gordon <david.s.gordon@intel.com>

This second client is created with priority KMD_HIGH, and marked
as preemptive.

v2: Rebase, cleanup, use an array for the second client.

Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c        |  8 +--
 drivers/gpu/drm/i915/i915_guc_submission.c | 86 ++++++++++++++++++++----------
 drivers/gpu/drm/i915/intel_uc.h            |  9 +++-
 3 files changed, 71 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 1a28b52..10490bf 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2514,7 +2514,7 @@ static int i915_guc_info(struct seq_file *m, void *data)
 	enum intel_engine_id id;
 	u64 total;
 
-	if (!guc->execbuf_client) {
+	if (!guc->client[NORMAL]) {
 		seq_printf(m, "GuC submission %s\n",
 			   HAS_GUC_SCHED(dev_priv) ?
 			   "disabled" :
@@ -2542,8 +2542,10 @@ static int i915_guc_info(struct seq_file *m, void *data)
 	}
 	seq_printf(m, "\t%s: %llu\n", "Total", total);
 
-	seq_printf(m, "\nGuC execbuf client @ %p:\n", guc->execbuf_client);
-	i915_guc_client_info(m, dev_priv, guc->execbuf_client);
+	seq_printf(m, "\nGuC execbuf client @ %p:\n", guc->client[NORMAL]);
+	i915_guc_client_info(m, dev_priv, guc->client[NORMAL]);
+	seq_printf(m, "\nGuC preempt client @ %p:\n", guc->client[PREEMPT]);
+	i915_guc_client_info(m, dev_priv, guc->client[PREEMPT]);
 
 	i915_guc_log_info(m, dev_priv);
 
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 3080735..2a658b9 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -247,6 +247,8 @@ static void guc_ctx_desc_init(struct intel_guc *guc,
 	memset(&desc, 0, sizeof(desc));
 
 	desc.attribute = GUC_CTX_DESC_ATTR_ACTIVE | GUC_CTX_DESC_ATTR_KERNEL;
+	if (client->priority <= GUC_CTX_PRIORITY_HIGH)
+		desc.attribute |= GUC_CTX_DESC_ATTR_PREEMPT;
 	desc.context_id = client->ctx_index;
 	desc.priority = client->priority;
 	desc.db_id = client->doorbell_id;
@@ -344,7 +346,7 @@ static void guc_ctx_desc_fini(struct intel_guc *guc,
 int i915_guc_wq_reserve(struct drm_i915_gem_request *request)
 {
 	const size_t wqi_size = sizeof(struct guc_wq_item);
-	struct i915_guc_client *client = request->i915->guc.execbuf_client;
+	struct i915_guc_client *client = request->i915->guc.client[NORMAL];
 	struct guc_process_desc *desc = client->vaddr +
 					client->proc_desc_offset;
 	u32 freespace;
@@ -368,7 +370,7 @@ int i915_guc_wq_reserve(struct drm_i915_gem_request *request)
 void i915_guc_wq_unreserve(struct drm_i915_gem_request *request)
 {
 	const size_t wqi_size = sizeof(struct guc_wq_item);
-	struct i915_guc_client *client = request->i915->guc.execbuf_client;
+	struct i915_guc_client *client = request->i915->guc.client[NORMAL];
 
 	GEM_BUG_ON(READ_ONCE(client->wq_rsvd) < wqi_size);
 
@@ -518,7 +520,7 @@ static void __i915_guc_submit(struct drm_i915_gem_request *rq)
 	struct intel_engine_cs *engine = rq->engine;
 	unsigned int engine_id = engine->id;
 	struct intel_guc *guc = &rq->i915->guc;
-	struct i915_guc_client *client = guc->execbuf_client;
+	struct i915_guc_client *client = guc->client[NORMAL];
 	int b_ret;
 
 	spin_lock(&client->wq_lock);
@@ -747,8 +749,8 @@ struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size)
 }
 
 static void
-guc_client_free(struct drm_i915_private *dev_priv,
-		struct i915_guc_client *client)
+__guc_client_free(struct drm_i915_private *dev_priv,
+		  struct i915_guc_client *client)
 {
 	struct intel_guc *guc = &dev_priv->guc;
 
@@ -805,7 +807,7 @@ static bool guc_doorbell_check(struct intel_guc *guc, uint16_t db_id)
  */
 static void guc_init_doorbell_hw(struct intel_guc *guc)
 {
-	struct i915_guc_client *client = guc->execbuf_client;
+	struct i915_guc_client *client = guc->client[NORMAL];
 	uint16_t db_id;
 	int i, err;
 
@@ -836,7 +838,7 @@ static void guc_init_doorbell_hw(struct intel_guc *guc)
 }
 
 /**
- * guc_client_alloc() - Allocate an i915_guc_client
+ * __guc_client_alloc() - Allocate an i915_guc_client
  * @dev_priv:	driver private data structure
  * @priority:	four levels priority _CRITICAL, _HIGH, _NORMAL and _LOW
  * 		The kernel client to replace ExecList submission is created with
@@ -845,8 +847,8 @@ static void guc_init_doorbell_hw(struct intel_guc *guc)
  * Return:	An i915_guc_client object if success, else NULL.
  */
 static struct i915_guc_client *
-guc_client_alloc(struct drm_i915_private *dev_priv,
-		 uint32_t priority)
+__guc_client_alloc(struct drm_i915_private *dev_priv,
+		   uint32_t priority)
 {
 	struct i915_guc_client *client;
 	struct intel_guc *guc = &dev_priv->guc;
@@ -910,7 +912,7 @@ guc_client_alloc(struct drm_i915_private *dev_priv,
 	guc_ctx_desc_init(guc, client);
 
 	/* For runtime client allocation we need to enable the doorbell. Not
-	 * required yet for the static execbuf_client as this special kernel
+	 * required yet for the static client as this special kernel
 	 * client is enabled from i915_guc_submission_enable().
 	 *
 	 * guc_update_doorbell_id(guc, client, db_id);
@@ -924,7 +926,7 @@ guc_client_alloc(struct drm_i915_private *dev_priv,
 	return client;
 
 err:
-	guc_client_free(dev_priv, client);
+	__guc_client_free(dev_priv, client);
 	return NULL;
 }
 
@@ -1021,6 +1023,45 @@ static void guc_addon_create(struct intel_guc *guc)
 	kunmap(page);
 }
 
+static void guc_free_clients(struct drm_i915_private *dev_priv)
+{
+	struct intel_guc *guc = &dev_priv->guc;
+	struct i915_guc_client *client;
+
+	client = fetch_and_zero(&guc->client[NORMAL]);
+	if (client)
+		__guc_client_free(dev_priv, client);
+
+	client = fetch_and_zero(&guc->client[PREEMPT]);
+	if (client)
+		__guc_client_free(dev_priv, client);
+}
+
+static int guc_alloc_clients(struct drm_i915_private *dev_priv)
+{
+	struct intel_guc *guc = &dev_priv->guc;
+
+	guc->client[NORMAL] = __guc_client_alloc(dev_priv,
+						 GUC_CTX_PRIORITY_KMD_NORMAL);
+	if (!guc->client[NORMAL]) {
+		DRM_ERROR("Failed to create GuC client for execbuf!\n");
+		goto err;
+	}
+
+	guc->client[PREEMPT] = __guc_client_alloc(dev_priv,
+						  GUC_CTX_PRIORITY_KMD_HIGH);
+	if (!guc->client[PREEMPT]) {
+		DRM_ERROR("Failed to create GuC client for execbuf!\n");
+		goto err;
+	}
+
+	return 0;
+
+err:
+	guc_free_clients(dev_priv);
+	return -ENOMEM;
+}
+
 /*
  * Set up the memory resources to be shared with the GuC.  At this point,
  * we require just one object that can be mapped through the GGTT.
@@ -1055,18 +1096,12 @@ int i915_guc_submission_init(struct drm_i915_private *dev_priv)
 	intel_guc_log_create(guc);
 	guc_addon_create(guc);
 
-	guc->execbuf_client = guc_client_alloc(dev_priv,
-					       GUC_CTX_PRIORITY_KMD_NORMAL);
-	if (!guc->execbuf_client) {
-		DRM_ERROR("Failed to create GuC client for execbuf!\n");
-		goto err;
+	if (guc_alloc_clients(dev_priv) != 0) {
+		i915_guc_submission_fini(dev_priv);
+		return -ENOMEM;
 	}
 
 	return 0;
-
-err:
-	i915_guc_submission_fini(dev_priv);
-	return -ENOMEM;
 }
 
 static void guc_reset_wq(struct i915_guc_client *client)
@@ -1083,7 +1118,7 @@ static void guc_reset_wq(struct i915_guc_client *client)
 int i915_guc_submission_enable(struct drm_i915_private *dev_priv)
 {
 	struct intel_guc *guc = &dev_priv->guc;
-	struct i915_guc_client *client = guc->execbuf_client;
+	struct i915_guc_client *client = guc->client[NORMAL];
 	struct intel_engine_cs *engine;
 	enum intel_engine_id id;
 
@@ -1129,7 +1164,7 @@ void i915_guc_submission_disable(struct drm_i915_private *dev_priv)
 {
 	struct intel_guc *guc = &dev_priv->guc;
 
-	if (!guc->execbuf_client)
+	if (!guc->client[NORMAL])
 		return;
 
 	/* Revert back to manual ELSP submission */
@@ -1139,13 +1174,8 @@ void i915_guc_submission_disable(struct drm_i915_private *dev_priv)
 void i915_guc_submission_fini(struct drm_i915_private *dev_priv)
 {
 	struct intel_guc *guc = &dev_priv->guc;
-	struct i915_guc_client *client;
-
-	client = fetch_and_zero(&guc->execbuf_client);
-	if (!client)
-		return;
 
-	guc_client_free(dev_priv, client);
+	guc_free_clients(dev_priv);
 
 	i915_vma_unpin_and_release(&guc->ads_vma);
 	i915_vma_unpin_and_release(&guc->log.vma);
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index d74f4d3..9dcd8fa 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -146,6 +146,13 @@ struct intel_guc_log {
 	u32 flush_count[GUC_MAX_LOG_BUFFER];
 };
 
+#define I915_GUC_NUM_CLIENTS 2
+
+enum i915_guc_client_id {
+        NORMAL = 0,
+        PREEMPT
+};
+
 struct intel_guc {
 	struct intel_uc_fw fw;
 	struct intel_guc_log log;
@@ -157,7 +164,7 @@ struct intel_guc {
 	struct i915_vma *ctx_pool_vma;
 	struct ida ctx_ids;
 
-	struct i915_guc_client *execbuf_client;
+	struct i915_guc_client *client[I915_GUC_NUM_CLIENTS];
 
 	DECLARE_BITMAP(doorbell_bitmap, GUC_MAX_DOORBELLS);
 	uint32_t db_cacheline;		/* Cyclic counter mod pagesize	*/
-- 
2.9.3

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC 08/11] drm/i915/guc: Add preemption action to GuC firmware interface
  2017-02-23 19:08 [RFC] GuC based preemption Michał Winiarski
                   ` (6 preceding siblings ...)
  2017-02-23 19:14 ` [RFC 07/11] drm/i915/guc: Add a second client, to be used for preemption Michał Winiarski
@ 2017-02-23 19:14 ` Michał Winiarski
  2017-02-23 19:14 ` [RFC 09/11] HACK drm/i915/preempt: Actually send the preemption request Michał Winiarski
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Michał Winiarski @ 2017-02-23 19:14 UTC (permalink / raw)
  To: intel-gfx

Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_fwif.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h b/drivers/gpu/drm/i915/intel_guc_fwif.h
index 25691f0..5b79ce0 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -497,6 +497,7 @@ union guc_log_control {
 /* This Action will be programmed in C180 - SOFT_SCRATCH_O_REG */
 enum intel_guc_action {
 	INTEL_GUC_ACTION_DEFAULT = 0x0,
+	INTEL_GUC_ACTION_REQUEST_PREEMPTION = 0x2,
 	INTEL_GUC_ACTION_SAMPLE_FORCEWAKE = 0x6,
 	INTEL_GUC_ACTION_ALLOCATE_DOORBELL = 0x10,
 	INTEL_GUC_ACTION_DEALLOCATE_DOORBELL = 0x20,
@@ -510,6 +511,12 @@ enum intel_guc_action {
 	INTEL_GUC_ACTION_LIMIT
 };
 
+enum intel_guc_preempt_options {
+	INTEL_GUC_PREEMPT_OPTION_IMMEDIATE = 0x1,
+	INTEL_GUC_PREEMPT_OPTION_DROP_WORK_Q = 0x4,
+	INTEL_GUC_PREEMPT_OPTION_DROP_SUBMIT_Q = 0x8,
+};
+
 /*
  * The GuC sends its response to a command by overwriting the
  * command in SS0. The response is distinguishable from a command
-- 
2.9.3

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC 09/11] HACK drm/i915/preempt: Actually send the preemption request
  2017-02-23 19:08 [RFC] GuC based preemption Michał Winiarski
                   ` (7 preceding siblings ...)
  2017-02-23 19:14 ` [RFC 08/11] drm/i915/guc: Add preemption action to GuC firmware interface Michał Winiarski
@ 2017-02-23 19:14 ` Michał Winiarski
  2017-02-23 19:14 ` [RFC 10/11] drm/i915/preempt: Emit MI_ARB_CHECK before the start of user batch Michał Winiarski
  2017-02-23 19:14 ` [RFC 11/11] drm/i915/preempt: Show engine preempt state in engine_info debugfs Michał Winiarski
  10 siblings, 0 replies; 19+ messages in thread
From: Michał Winiarski @ 2017-02-23 19:14 UTC (permalink / raw)
  To: intel-gfx

Now that we're able to post-process the preemption event, let's actually
send it to GuC. To identify that preemption has finished, we're using a
dummy request sent through kernel_context.

Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
---
 drivers/gpu/drm/i915/i915_guc_submission.c | 90 ++++++++++++++++++++++++++++--
 1 file changed, 86 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 2a658b9..7ccc5b4 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -491,14 +491,96 @@ static int guc_ring_doorbell(struct i915_guc_client *client)
 	return ret;
 }
 
-static int i915_guc_preempt_noop(struct intel_engine_cs *engine)
+static int add_preemption_workitem(struct intel_engine_cs *engine)
 {
-	engine->preempt_requested = false;
-	intel_write_status_page(engine, I915_GEM_HWS_PREEMPT_INDEX, 0);
+	struct drm_i915_private *dev_priv = engine->i915;
+	struct intel_guc *guc = &engine->i915->guc;
+	struct i915_gem_context *ctx = engine->i915->kernel_context;
+	struct i915_guc_client *client = guc->client[PREEMPT];
+	struct drm_i915_gem_request rq;
+	u32 *cs;
+
+	rq.i915 = engine->i915;
+	rq.engine = engine;
+	rq.ctx = ctx;
+	rq.global_seqno = 0;
+	rq.file_priv = NULL;
+	rq.batch = NULL;
+	rq.reserved_space = 32;
+	rq.ring = ctx->engine[engine->id].ring;
+
+	if (engine->id == RCS) {
+		cs = intel_ring_begin(&rq, 8);
+		if (IS_ERR(cs))
+			return PTR_ERR(cs);
+
+		*cs++ = GFX_OP_PIPE_CONTROL(6);
+		*cs++ = PIPE_CONTROL_GLOBAL_GTT_IVB |
+			PIPE_CONTROL_CS_STALL |
+			PIPE_CONTROL_QW_WRITE;
+		*cs++ = engine->status_page.ggtt_offset +
+			I915_GEM_HWS_PREEMPT_ADDR;
+		*cs++ = 0;
+		*cs++ = 1;
+		*cs++ = 0;
+		*cs++ = MI_USER_INTERRUPT;
+		*cs++ = MI_NOOP;
+		intel_ring_advance(&rq, cs);
+	} else {
+		cs = intel_ring_begin(&rq, 6);
+		if (IS_ERR(cs))
+			return PTR_ERR(cs);
+
+		*cs++ = (MI_FLUSH_DW + 1) | MI_FLUSH_DW_OP_STOREDW;
+		*cs++ = (engine->status_page.ggtt_offset + I915_GEM_HWS_PREEMPT_ADDR) |
+		        MI_FLUSH_DW_USE_GTT;
+		*cs++ = 0;
+		*cs++ = 1;
+		*cs++ = MI_USER_INTERRUPT;
+		*cs++ = MI_NOOP;
+		intel_ring_advance(&rq, cs);
+	}
+	rq.tail = intel_ring_offset(&rq, cs);
+
+	spin_lock_irq(&client->wq_lock);
+	client->wq_rsvd += sizeof(struct guc_wq_item);
+	guc_wq_item_append(client, &rq);
+	if (i915_vma_is_map_and_fenceable(rq.ring->vma))
+		POSTING_READ_FW(GUC_STATUS);
+	spin_unlock_irq(&client->wq_lock);
 
 	return 0;
 }
 
+static int i915_guc_preempt(struct intel_engine_cs *engine)
+{
+	struct intel_guc *guc = &engine->i915->guc;
+	struct i915_guc_client *client = guc->client[PREEMPT];
+	struct guc_process_desc *desc;
+	u32 data[7];
+	int ret;
+
+	ret = add_preemption_workitem(engine);
+	if (ret)
+		return ret;
+
+	desc = client->vaddr + client->proc_desc_offset;
+	desc->tail = client->wq_tail;
+
+	data[0] = INTEL_GUC_ACTION_REQUEST_PREEMPTION;
+	data[1] = client->ctx_index;
+	data[2] = INTEL_GUC_PREEMPT_OPTION_IMMEDIATE |
+		  INTEL_GUC_PREEMPT_OPTION_DROP_WORK_Q |
+		  INTEL_GUC_PREEMPT_OPTION_DROP_SUBMIT_Q;
+	data[3] = engine->guc_id;
+	data[4] = guc->client[NORMAL]->priority;
+	data[5] = guc->client[NORMAL]->ctx_index;
+	data[6] = engine->status_page.ggtt_offset +
+		  (LRC_GUCSHR_PN * PAGE_SIZE);
+
+	return intel_guc_send(guc, data, ARRAY_SIZE(data));
+}
+
 /**
  * __i915_guc_submit() - Submit commands through GuC
  * @rq:		request associated with the commands
@@ -1143,7 +1225,7 @@ int i915_guc_submission_enable(struct drm_i915_private *dev_priv)
 		engine->irq_tasklet.func = i915_guc_irq_handler;
 
 		if (i915.enable_preemption)
-			engine->preempt = i915_guc_preempt_noop;
+			engine->preempt = i915_guc_preempt;
 
 		/* Replay the current set of previously submitted requests */
 		spin_lock_irqsave(&engine->timeline->lock, flags);
-- 
2.9.3

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC 10/11] drm/i915/preempt: Emit MI_ARB_CHECK before the start of user batch
  2017-02-23 19:08 [RFC] GuC based preemption Michał Winiarski
                   ` (8 preceding siblings ...)
  2017-02-23 19:14 ` [RFC 09/11] HACK drm/i915/preempt: Actually send the preemption request Michał Winiarski
@ 2017-02-23 19:14 ` Michał Winiarski
  2017-03-01 12:53   ` Chris Wilson
  2017-02-23 19:14 ` [RFC 11/11] drm/i915/preempt: Show engine preempt state in engine_info debugfs Michał Winiarski
  10 siblings, 1 reply; 19+ messages in thread
From: Michał Winiarski @ 2017-02-23 19:14 UTC (permalink / raw)
  To: intel-gfx

We should probably do this conditionally, based on whether preemption is
actually enabled.

Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
---
 drivers/gpu/drm/i915/intel_lrc.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 869b96e..972f9bd 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1418,10 +1418,13 @@ static int gen8_emit_bb_start(struct drm_i915_gem_request *req,
 		req->ctx->ppgtt->pd_dirty_rings &= ~intel_engine_flag(req->engine);
 	}
 
-	cs = intel_ring_begin(req, 4);
+	cs = intel_ring_begin(req, 6);
 	if (IS_ERR(cs))
 		return PTR_ERR(cs);
 
+	*cs++ = MI_ARB_CHECK;
+	*cs++ = MI_NOOP;
+
 	/* FIXME(BDW): Address space and security selectors. */
 	*cs++ = MI_BATCH_BUFFER_START_GEN8 | (ppgtt << 8) | (dispatch_flags &
 		I915_DISPATCH_RS ? MI_BATCH_RESOURCE_STREAMER : 0);
-- 
2.9.3

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC 11/11] drm/i915/preempt: Show engine preempt state in engine_info debugfs
  2017-02-23 19:08 [RFC] GuC based preemption Michał Winiarski
                   ` (9 preceding siblings ...)
  2017-02-23 19:14 ` [RFC 10/11] drm/i915/preempt: Emit MI_ARB_CHECK before the start of user batch Michał Winiarski
@ 2017-02-23 19:14 ` Michał Winiarski
  10 siblings, 0 replies; 19+ messages in thread
From: Michał Winiarski @ 2017-02-23 19:14 UTC (permalink / raw)
  To: intel-gfx

Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 10490bf..b71420c 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3268,6 +3268,10 @@ static int i915_engine_info(struct seq_file *m, void *unused)
 			   engine->hangcheck.seqno,
 			   jiffies_to_msecs(jiffies - engine->hangcheck.action_timestamp));
 
+		seq_printf(m, "\tpreempt [requested %d, finished %d]\n",
+			   engine->preempt_requested,
+			   intel_engine_is_preempt_finished(engine));
+
 		rcu_read_lock();
 
 		seq_printf(m, "\tRequests:\n");
-- 
2.9.3

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [RFC 01/11] drm/i915/scheduler: Remember request priority throughout its lifetime
  2017-02-23 19:08 ` [RFC 01/11] drm/i915/scheduler: Remember request priority throughout its lifetime Michał Winiarski
@ 2017-02-23 21:29   ` Chris Wilson
  0 siblings, 0 replies; 19+ messages in thread
From: Chris Wilson @ 2017-02-23 21:29 UTC (permalink / raw)
  To: Michał Winiarski; +Cc: intel-gfx

On Thu, Feb 23, 2017 at 08:08:23PM +0100, Michał Winiarski wrote:
> Since request can be cancelled, we need to avoid overriding its priority
> during submission to be able to resubmit it.

And this breaks rescheduling.
-Chris

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC 04/11] drm/i915/preempt: Implement null preemption method
  2017-02-23 19:08 ` [RFC 04/11] drm/i915/preempt: Implement null preemption method Michał Winiarski
@ 2017-02-23 21:37   ` Chris Wilson
  0 siblings, 0 replies; 19+ messages in thread
From: Chris Wilson @ 2017-02-23 21:37 UTC (permalink / raw)
  To: Michał Winiarski; +Cc: intel-gfx

On Thu, Feb 23, 2017 at 08:08:26PM +0100, Michał Winiarski wrote:
> +static void __execlists_try_preempt(struct intel_engine_cs *engine,
> +				  int prio)
> +{
> +	struct drm_i915_gem_request *rq;
> +	int highest_prio = INT_MIN;
> +	int ret;
> +
> +	spin_lock_irq(&engine->timeline->lock);
> +
> +	/* Engine is idle */
> +	if (execlists_elsp_idle(engine))
> +		goto out_unlock;
> +
> +	if (engine->preempt_requested)
> +		goto out_unlock;
> +
> +	list_for_each_entry_reverse(rq, &engine->timeline->requests, link) {
> +		if (!i915_gem_request_completed(rq)) {
> +			highest_prio = (rq->priotree.priority > highest_prio) ?
> +				 rq->priotree.priority : highest_prio;
> +		} else
> +			break;
> +	}
> +
> +	/* Bail out if our priority is lower than any of the inflight requests
> +	 * (also if there are none requests) */
> +	if (highest_prio == INT_MIN || prio <= highest_prio)
> +		goto out_unlock;
> +
> +	engine->preempt_requested = true;

Here you are meant to unwind the already submitted requests and put them
back onto their rq->timelines.
-Chris

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC 06/11] drm/i915/guc: Remove extra arguments from guc_client_alloc
  2017-02-23 16:38   ` Oscar Mateo
@ 2017-02-24  1:39     ` Daniele Ceraolo Spurio
  0 siblings, 0 replies; 19+ messages in thread
From: Daniele Ceraolo Spurio @ 2017-02-24  1:39 UTC (permalink / raw)
  To: Oscar Mateo, Michał Winiarski, intel-gfx



On 23/02/17 08:38, Oscar Mateo wrote:
>
>
> On 02/23/2017 11:14 AM, Michał Winiarski wrote:
>> We're always using all engines and kernel context for guc clients, let's
>> remove those arguments from guc_client_alloc.
> I am quite new to the GuC but, by the look of it, passing the ctx was
> groundwork for direct submission (which means calling guc_client_alloc
> with contexts different than kernel_context). Why not leave it there,
> since someone was careful to lay the groundwork? I don't see an obvious
> use for the engines, though (clients that only have access to some of
> the engines?).
>

If I recall correctly the idea behind having the engine flag was for 
possibly having dedicated clients for single engines and/or engines 
groups if 1 work-queue + doorbell pair turned out to not be enough for 
our needs. There might also be some little performance benefit in that 
scenario because we can update different work-queues in parallel, but 
we're still capped by how fast GuC can process them. Not sure if it 
still makes sense though.

Daniele

>> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_guc_submission.c | 16 ++++------------
>>   1 file changed, 4 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c
>> b/drivers/gpu/drm/i915/i915_guc_submission.c
>> index 6c64ce1..3080735 100644
>> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
>> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
>> @@ -838,21 +838,15 @@ static void guc_init_doorbell_hw(struct
>> intel_guc *guc)
>>   /**
>>    * guc_client_alloc() - Allocate an i915_guc_client
>>    * @dev_priv:    driver private data structure
>> - * @engines:    The set of engines to enable for this client
>>    * @priority:    four levels priority _CRITICAL, _HIGH, _NORMAL and
>> _LOW
>>    *         The kernel client to replace ExecList submission is
>> created with
>>    *         NORMAL priority. Priority of a client for scheduler can
>> be HIGH,
>>    *         while a preemption context can use CRITICAL.
>> - * @ctx:    the context that owns the client (we use the default render
>> - *         context)
>> - *
>>    * Return:    An i915_guc_client object if success, else NULL.
>>    */
>>   static struct i915_guc_client *
>>   guc_client_alloc(struct drm_i915_private *dev_priv,
>> -         uint32_t engines,
>> -         uint32_t priority,
>> -         struct i915_gem_context *ctx)
>> +         uint32_t priority)
>>   {
>>       struct i915_guc_client *client;
>>       struct intel_guc *guc = &dev_priv->guc;
>> @@ -864,9 +858,9 @@ guc_client_alloc(struct drm_i915_private *dev_priv,
>>       if (!client)
>>           return NULL;
>>   -    client->owner = ctx;
>> +    client->owner = dev_priv->kernel_context;
>>       client->guc = guc;
>> -    client->engines = engines;
>> +    client->engines = INTEL_INFO(dev_priv)->ring_mask;
>>       client->priority = priority;
>>       client->doorbell_id = GUC_INVALID_DOORBELL_ID;
>>   @@ -1062,9 +1056,7 @@ int i915_guc_submission_init(struct
>> drm_i915_private *dev_priv)
>>       guc_addon_create(guc);
>>         guc->execbuf_client = guc_client_alloc(dev_priv,
>> -                           INTEL_INFO(dev_priv)->ring_mask,
>> -                           GUC_CTX_PRIORITY_KMD_NORMAL,
>> -                           dev_priv->kernel_context);
>> +                           GUC_CTX_PRIORITY_KMD_NORMAL);
>>       if (!guc->execbuf_client) {
>>           DRM_ERROR("Failed to create GuC client for execbuf!\n");
>>           goto err;
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC 10/11] drm/i915/preempt: Emit MI_ARB_CHECK before the start of user batch
  2017-02-23 19:14 ` [RFC 10/11] drm/i915/preempt: Emit MI_ARB_CHECK before the start of user batch Michał Winiarski
@ 2017-03-01 12:53   ` Chris Wilson
  0 siblings, 0 replies; 19+ messages in thread
From: Chris Wilson @ 2017-03-01 12:53 UTC (permalink / raw)
  To: Michał Winiarski; +Cc: intel-gfx

On Thu, Feb 23, 2017 at 08:14:20PM +0100, Michał Winiarski wrote:
> We should probably do this conditionally, based on whether preemption is
> actually enabled.
> 
> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_lrc.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 869b96e..972f9bd 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1418,10 +1418,13 @@ static int gen8_emit_bb_start(struct drm_i915_gem_request *req,
>  		req->ctx->ppgtt->pd_dirty_rings &= ~intel_engine_flag(req->engine);
>  	}
>  
> -	cs = intel_ring_begin(req, 4);
> +	cs = intel_ring_begin(req, 6);
>  	if (IS_ERR(cs))
>  		return PTR_ERR(cs);
>  
> +	*cs++ = MI_ARB_CHECK;
> +	*cs++ = MI_NOOP;
> +
>  	/* FIXME(BDW): Address space and security selectors. */
>  	*cs++ = MI_BATCH_BUFFER_START_GEN8 | (ppgtt << 8) | (dispatch_flags &
>  		I915_DISPATCH_RS ? MI_BATCH_RESOURCE_STREAMER : 0);

Shouldn't we only enable arbitration across the batch, i.e. add the
disable here? The flush and breadcrumb are not worth interrupting and
saves us some work if they are not.
-Chris

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC 05/11] drm/i915/preempt: Handle preemption event in guc tasklet
  2017-02-23 19:14 ` [RFC 05/11] drm/i915/preempt: Handle preemption event in guc tasklet Michał Winiarski
@ 2017-03-01 12:57   ` Chris Wilson
  2017-03-06 10:53     ` Chris Wilson
  0 siblings, 1 reply; 19+ messages in thread
From: Chris Wilson @ 2017-03-01 12:57 UTC (permalink / raw)
  To: Michał Winiarski; +Cc: intel-gfx

On Thu, Feb 23, 2017 at 08:14:15PM +0100, Michał Winiarski wrote:
> +static void unsubmit_inflight_requests(struct intel_engine_cs *engine,
> +					 struct list_head *resubmit)
> +{
> +	struct drm_i915_gem_request *rq, *prev;
> +
> +	assert_spin_locked(&engine->timeline->lock);
> +
> +	list_for_each_entry_safe_reverse(rq, prev,
> +					 &engine->timeline->requests, link) {
> +		if (!i915_gem_request_completed(rq)) {
> +			i915_gem_request_get(rq);
> +			__i915_gem_request_unsubmit(rq);
> +			list_add(&rq->resubmit_link, resubmit);

I was thinking it would be going back to the execlist queue, where it
would be priority sorted against later requests.

Pro/cons?
-Chris

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC 05/11] drm/i915/preempt: Handle preemption event in guc tasklet
  2017-03-01 12:57   ` Chris Wilson
@ 2017-03-06 10:53     ` Chris Wilson
  0 siblings, 0 replies; 19+ messages in thread
From: Chris Wilson @ 2017-03-06 10:53 UTC (permalink / raw)
  To: Michał Winiarski, intel-gfx, Tvrtko Ursulin,
	Joonas Lahtinen, John C Harrison, Jeff Mcgee

On Wed, Mar 01, 2017 at 12:57:15PM +0000, Chris Wilson wrote:
> On Thu, Feb 23, 2017 at 08:14:15PM +0100, Michał Winiarski wrote:
> > +static void unsubmit_inflight_requests(struct intel_engine_cs *engine,
> > +					 struct list_head *resubmit)
> > +{
> > +	struct drm_i915_gem_request *rq, *prev;
> > +
> > +	assert_spin_locked(&engine->timeline->lock);
> > +
> > +	list_for_each_entry_safe_reverse(rq, prev,
> > +					 &engine->timeline->requests, link) {
> > +		if (!i915_gem_request_completed(rq)) {
> > +			i915_gem_request_get(rq);
> > +			__i915_gem_request_unsubmit(rq);
> > +			list_add(&rq->resubmit_link, resubmit);
> 
> I was thinking it would be going back to the execlist queue, where it
> would be priority sorted against later requests.

Hmm. The execlists rbtree is setup to keep fifo ordering. We would end
up reversing the inflight requests if we are not careful. :|
-Chris

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2017-03-06 10:53 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-23 19:08 [RFC] GuC based preemption Michał Winiarski
2017-02-23 19:08 ` [RFC 01/11] drm/i915/scheduler: Remember request priority throughout its lifetime Michał Winiarski
2017-02-23 21:29   ` Chris Wilson
2017-02-23 19:08 ` [RFC 02/11] drm/i915/preempt: Add module parameter for preemption Michał Winiarski
2017-02-23 19:08 ` [RFC 03/11] drm/i915/preempt: Add information needed to track engine preempt state Michał Winiarski
2017-02-23 19:08 ` [RFC 04/11] drm/i915/preempt: Implement null preemption method Michał Winiarski
2017-02-23 21:37   ` Chris Wilson
2017-02-23 19:14 ` [RFC 05/11] drm/i915/preempt: Handle preemption event in guc tasklet Michał Winiarski
2017-03-01 12:57   ` Chris Wilson
2017-03-06 10:53     ` Chris Wilson
2017-02-23 19:14 ` [RFC 06/11] drm/i915/guc: Remove extra arguments from guc_client_alloc Michał Winiarski
2017-02-23 16:38   ` Oscar Mateo
2017-02-24  1:39     ` Daniele Ceraolo Spurio
2017-02-23 19:14 ` [RFC 07/11] drm/i915/guc: Add a second client, to be used for preemption Michał Winiarski
2017-02-23 19:14 ` [RFC 08/11] drm/i915/guc: Add preemption action to GuC firmware interface Michał Winiarski
2017-02-23 19:14 ` [RFC 09/11] HACK drm/i915/preempt: Actually send the preemption request Michał Winiarski
2017-02-23 19:14 ` [RFC 10/11] drm/i915/preempt: Emit MI_ARB_CHECK before the start of user batch Michał Winiarski
2017-03-01 12:53   ` Chris Wilson
2017-02-23 19:14 ` [RFC 11/11] drm/i915/preempt: Show engine preempt state in engine_info debugfs Michał Winiarski

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.