All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/5] drm/i915/guc: Remove obsolete comments and remove unused variable
@ 2017-09-14  8:32 Michał Winiarski
  2017-09-14  8:32 ` [PATCH v5 2/5] drm/i915/guc: Submit GuC workitems containing coalesced requests Michał Winiarski
                   ` (7 more replies)
  0 siblings, 8 replies; 21+ messages in thread
From: Michał Winiarski @ 2017-09-14  8:32 UTC (permalink / raw)
  To: intel-gfx

Originally removed in:
c1adab970348 ("drm/i915/guc: Remove failed doorbell stat from debugfs")
f1448a62a103 ("drm/i915/guc: Remove last submission result from debugfs")

Were accidentally restored in:
925344ccc91d ("BackMerge tag 'v4.12-rc5' into drm-next")

We can also remove unused variable and replace it with a WARN.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_guc_submission.c | 3 +--
 drivers/gpu/drm/i915/intel_uc.h            | 4 ----
 2 files changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index b28677e5a4f2..c180ff1423fd 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -601,7 +601,6 @@ static void __i915_guc_submit(struct drm_i915_gem_request *rq)
 	struct intel_guc *guc = &rq->i915->guc;
 	struct i915_guc_client *client = guc->execbuf_client;
 	unsigned long flags;
-	int b_ret;
 
 	/* WA to flush out the pending GMADR writes to ring buffer. */
 	if (i915_vma_is_map_and_fenceable(rq->ring->vma))
@@ -610,7 +609,7 @@ static void __i915_guc_submit(struct drm_i915_gem_request *rq)
 	spin_lock_irqsave(&client->wq_lock, flags);
 
 	guc_wq_item_append(client, rq);
-	b_ret = guc_ring_doorbell(client);
+	WARN_ON(guc_ring_doorbell(client));
 
 	client->submissions[engine_id] += 1;
 
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 22ae52b17b0f..69daf4c01cd0 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -59,10 +59,6 @@ struct drm_i915_gem_request;
  *                available in the work queue (note, the queue is shared,
  *                not per-engine). It is OK for this to be nonzero, but
  *                it should not be huge!
- *   b_fail: failed to ring the doorbell. This should never happen, unless
- *           somehow the hardware misbehaves, or maybe if the GuC firmware
- *           crashes? We probably need to reset the GPU to recover.
- *   retcode: errno from last guc_submit()
  */
 struct i915_guc_client {
 	struct i915_vma *vma;
-- 
2.13.5

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

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

* [PATCH v5 2/5] drm/i915/guc: Submit GuC workitems containing coalesced requests
  2017-09-14  8:32 [PATCH 1/5] drm/i915/guc: Remove obsolete comments and remove unused variable Michał Winiarski
@ 2017-09-14  8:32 ` Michał Winiarski
  2017-09-14  8:32 ` [PATCH v3 3/5] drm/i915/guc: Simplify GuC doorbell logic Michał Winiarski
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 21+ messages in thread
From: Michał Winiarski @ 2017-09-14  8:32 UTC (permalink / raw)
  To: intel-gfx

To create an upper bound on number of GuC workitems, we need to change
the way that requests are being submitted. Rather than submitting each
request as an individual workitem, we can do coalescing in a similar way
we're handlig execlist submission ports. We also need to stop pretending
that we're doing "lite-restore" in GuC submission (we would create a
workitem each time we hit this condition). This allows us to completely
remove the reservation, replacing it with a compile time check.

v2: Also coalesce when replaying on reset (Daniele)
v3: Consistent wq_resv - per-request (Daniele)
v4: Squash removing wq_resv
v5: Reflect i915_guc_submit argument changes in doc

References: https://bugs.freedesktop.org/show_bug.cgi?id=101873
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Jeff McGee <jeff.mcgee@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_debugfs.c        |   2 -
 drivers/gpu/drm/i915/i915_guc_submission.c | 181 ++++++++++-------------------
 drivers/gpu/drm/i915/intel_lrc.c           |  25 +---
 drivers/gpu/drm/i915/intel_uc.h            |  11 --
 4 files changed, 63 insertions(+), 156 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 12381045ed6a..a4c2d0718a70 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2450,8 +2450,6 @@ static void i915_guc_client_info(struct seq_file *m,
 	seq_printf(m, "\tWQ size %d, offset: 0x%x, tail %d\n",
 		client->wq_size, client->wq_offset, client->wq_tail);
 
-	seq_printf(m, "\tWork queue full: %u\n", client->no_wq_space);
-
 	for_each_engine(engine, dev_priv, id) {
 		u64 submissions = client->submissions[id];
 		tot += submissions;
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index c180ff1423fd..84beedae3708 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -406,63 +406,6 @@ static void guc_stage_desc_fini(struct intel_guc *guc,
 	memset(desc, 0, sizeof(*desc));
 }
 
-/**
- * i915_guc_wq_reserve() - reserve space in the GuC's workqueue
- * @request:	request associated with the commands
- *
- * Return:	0 if space is available
- *		-EAGAIN if space is not currently available
- *
- * This function must be called (and must return 0) before a request
- * is submitted to the GuC via i915_guc_submit() below. Once a result
- * of 0 has been returned, it must be balanced by a corresponding
- * call to submit().
- *
- * Reservation allows the caller to determine in advance that space
- * will be available for the next submission before committing resources
- * to it, and helps avoid late failures with complicated recovery paths.
- */
-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 guc_process_desc *desc = __get_process_desc(client);
-	u32 freespace;
-	int ret;
-
-	spin_lock_irq(&client->wq_lock);
-	freespace = CIRC_SPACE(client->wq_tail, desc->head, client->wq_size);
-	freespace -= client->wq_rsvd;
-	if (likely(freespace >= wqi_size)) {
-		client->wq_rsvd += wqi_size;
-		ret = 0;
-	} else {
-		client->no_wq_space++;
-		ret = -EAGAIN;
-	}
-	spin_unlock_irq(&client->wq_lock);
-
-	return ret;
-}
-
-static void guc_client_update_wq_rsvd(struct i915_guc_client *client, int size)
-{
-	unsigned long flags;
-
-	spin_lock_irqsave(&client->wq_lock, flags);
-	client->wq_rsvd += size;
-	spin_unlock_irqrestore(&client->wq_lock, flags);
-}
-
-void i915_guc_wq_unreserve(struct drm_i915_gem_request *request)
-{
-	const int wqi_size = sizeof(struct guc_wq_item);
-	struct i915_guc_client *client = request->i915->guc.execbuf_client;
-
-	GEM_BUG_ON(READ_ONCE(client->wq_rsvd) < wqi_size);
-	guc_client_update_wq_rsvd(client, -wqi_size);
-}
-
 /* Construct a Work Item and append it to the GuC's Work Queue */
 static void guc_wq_item_append(struct i915_guc_client *client,
 			       struct drm_i915_gem_request *rq)
@@ -476,7 +419,7 @@ static void guc_wq_item_append(struct i915_guc_client *client,
 	struct guc_wq_item *wqi;
 	u32 freespace, tail, wq_off;
 
-	/* Free space is guaranteed, see i915_guc_wq_reserve() above */
+	/* Free space is guaranteed */
 	freespace = CIRC_SPACE(client->wq_tail, desc->head, client->wq_size);
 	GEM_BUG_ON(freespace < wqi_size);
 
@@ -491,14 +434,12 @@ static void guc_wq_item_append(struct i915_guc_client *client,
 	 * workqueue buffer dw by dw.
 	 */
 	BUILD_BUG_ON(wqi_size != 16);
-	GEM_BUG_ON(client->wq_rsvd < wqi_size);
 
 	/* postincrement WQ tail for next time */
 	wq_off = client->wq_tail;
 	GEM_BUG_ON(wq_off & (wqi_size - 1));
 	client->wq_tail += wqi_size;
 	client->wq_tail &= client->wq_size - 1;
-	client->wq_rsvd -= wqi_size;
 
 	/* WQ starts from the page after doorbell / process_desc */
 	wqi = client->vaddr + wq_off + GUC_DB_SIZE;
@@ -579,47 +520,43 @@ static int guc_ring_doorbell(struct i915_guc_client *client)
 }
 
 /**
- * __i915_guc_submit() - Submit commands through GuC
- * @rq:		request associated with the commands
- *
- * The caller must have already called i915_guc_wq_reserve() above with
- * a result of 0 (success), guaranteeing that there is space in the work
- * queue for the new request, so enqueuing the item cannot fail.
- *
- * Bad Things Will Happen if the caller violates this protocol e.g. calls
- * submit() when _reserve() says there's no space, or calls _submit()
- * a different number of times from (successful) calls to _reserve().
+ * i915_guc_submit() - Submit commands through GuC
+ * @engine: engine associated with the commands
  *
  * The only error here arises if the doorbell hardware isn't functioning
  * as expected, which really shouln't happen.
  */
-static void __i915_guc_submit(struct drm_i915_gem_request *rq)
+static void i915_guc_submit(struct intel_engine_cs *engine)
 {
-	struct drm_i915_private *dev_priv = rq->i915;
-	struct intel_engine_cs *engine = rq->engine;
-	unsigned int engine_id = engine->id;
-	struct intel_guc *guc = &rq->i915->guc;
+	struct drm_i915_private *dev_priv = engine->i915;
+	struct intel_guc *guc = &dev_priv->guc;
 	struct i915_guc_client *client = guc->execbuf_client;
+	struct execlist_port *port = engine->execlist_port;
+	unsigned int engine_id = engine->id;
+	unsigned int n;
 	unsigned long flags;
 
-	/* 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);
+	for (n = 0; n < ARRAY_SIZE(engine->execlist_port); n++) {
+		struct drm_i915_gem_request *rq;
+		unsigned int count;
 
-	spin_lock_irqsave(&client->wq_lock, flags);
+		rq = port_unpack(&port[n], &count);
+		if (rq && count == 0) {
+			port_set(&port[n], port_pack(rq, ++count));
 
-	guc_wq_item_append(client, rq);
-	WARN_ON(guc_ring_doorbell(client));
+			if (i915_vma_is_map_and_fenceable(rq->ring->vma))
+				POSTING_READ_FW(GUC_STATUS);
 
-	client->submissions[engine_id] += 1;
+			spin_lock_irqsave(&client->wq_lock, flags);
 
-	spin_unlock_irqrestore(&client->wq_lock, flags);
-}
+			guc_wq_item_append(client, rq);
+			WARN_ON(guc_ring_doorbell(client));
 
-static void i915_guc_submit(struct drm_i915_gem_request *rq)
-{
-	__i915_gem_request_submit(rq);
-	__i915_guc_submit(rq);
+			client->submissions[engine_id] += 1;
+
+			spin_unlock_irqrestore(&client->wq_lock, flags);
+		}
+	}
 }
 
 static void nested_enable_signaling(struct drm_i915_gem_request *rq)
@@ -653,16 +590,19 @@ static void port_assign(struct execlist_port *port,
 	if (port_isset(port))
 		i915_gem_request_put(port_request(port));
 
-	port_set(port, i915_gem_request_get(rq));
+	port_set(port, port_pack(i915_gem_request_get(rq), port_count(port)));
 	nested_enable_signaling(rq);
 }
 
-static bool i915_guc_dequeue(struct intel_engine_cs *engine)
+static void i915_guc_dequeue(struct intel_engine_cs *engine)
 {
 	struct execlist_port *port = engine->execlist_port;
-	struct drm_i915_gem_request *last = port_request(port);
-	struct rb_node *rb;
+	struct drm_i915_gem_request *last = NULL;
 	bool submit = false;
+	struct rb_node *rb;
+
+	if (port_isset(port))
+		port++;
 
 	spin_lock_irq(&engine->timeline->lock);
 	rb = engine->execlist_first;
@@ -687,7 +627,7 @@ static bool i915_guc_dequeue(struct intel_engine_cs *engine)
 			INIT_LIST_HEAD(&rq->priotree.link);
 			rq->priotree.priority = INT_MAX;
 
-			i915_guc_submit(rq);
+			__i915_gem_request_submit(rq);
 			trace_i915_gem_request_in(rq, port_index(port, engine));
 			last = rq;
 			submit = true;
@@ -701,11 +641,11 @@ static bool i915_guc_dequeue(struct intel_engine_cs *engine)
 	}
 done:
 	engine->execlist_first = rb;
-	if (submit)
+	if (submit) {
 		port_assign(port, last);
+		i915_guc_submit(engine);
+	}
 	spin_unlock_irq(&engine->timeline->lock);
-
-	return submit;
 }
 
 static void i915_guc_irq_handler(unsigned long data)
@@ -713,24 +653,20 @@ 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;
-	bool submit;
 
-	do {
-		rq = port_request(&port[0]);
-		while (rq && i915_gem_request_completed(rq)) {
-			trace_i915_gem_request_out(rq);
-			i915_gem_request_put(rq);
+	rq = port_request(&port[0]);
+	while (rq && i915_gem_request_completed(rq)) {
+		trace_i915_gem_request_out(rq);
+		i915_gem_request_put(rq);
 
-			port[0] = port[1];
-			memset(&port[1], 0, sizeof(port[1]));
+		port[0] = port[1];
+		memset(&port[1], 0, sizeof(port[1]));
 
-			rq = port_request(&port[0]);
-		}
+		rq = port_request(&port[0]);
+	}
 
-		submit = false;
-		if (!port_count(&port[1]))
-			submit = i915_guc_dequeue(engine);
-	} while (submit);
+	if (!port_isset(&port[1]))
+		i915_guc_dequeue(engine);
 }
 
 /*
@@ -1239,6 +1175,19 @@ int i915_guc_submission_enable(struct drm_i915_private *dev_priv)
 	enum intel_engine_id id;
 	int err;
 
+	/*
+	 * We're using GuC work items for submitting work through GuC. Since
+	 * we're coalescing multiple requests from a single context into a
+	 * single work item prior to assigning it to execlist_port, we can
+	 * never have more work items than the total number of ports (for all
+	 * engines). The GuC firmware is controlling the HEAD of work queue,
+	 * and it is guaranteed that it will remove the work item from the
+	 * queue before our request is completed.
+	 */
+	BUILD_BUG_ON(ARRAY_SIZE(engine->execlist_port) *
+		     sizeof(struct guc_wq_item) *
+		     I915_NUM_ENGINES > GUC_WQ_SIZE);
+
 	if (!client) {
 		client = guc_client_alloc(dev_priv,
 					  INTEL_INFO(dev_priv)->ring_mask,
@@ -1266,9 +1215,6 @@ int i915_guc_submission_enable(struct drm_i915_private *dev_priv)
 	guc_interrupts_capture(dev_priv);
 
 	for_each_engine(engine, dev_priv, id) {
-		const int wqi_size = sizeof(struct guc_wq_item);
-		struct drm_i915_gem_request *rq;
-
 		/* The tasklet was initialised by execlists, and may be in
 		 * a state of flux (across a reset) and so we just want to
 		 * take over the callback without changing any other state
@@ -1276,14 +1222,7 @@ int i915_guc_submission_enable(struct drm_i915_private *dev_priv)
 		 */
 		engine->irq_tasklet.func = i915_guc_irq_handler;
 		clear_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted);
-
-		/* Replay the current set of previously submitted requests */
-		spin_lock_irq(&engine->timeline->lock);
-		list_for_each_entry(rq, &engine->timeline->requests, link) {
-			guc_client_update_wq_rsvd(client, wqi_size);
-			__i915_guc_submit(rq);
-		}
-		spin_unlock_irq(&engine->timeline->lock);
+		i915_guc_submit(engine);
 	}
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 1960ba5ff9e4..759172dc0d49 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -926,27 +926,14 @@ static int execlists_request_alloc(struct drm_i915_gem_request *request)
 	 */
 	request->reserved_space += EXECLISTS_REQUEST_SIZE;
 
-	if (i915.enable_guc_submission) {
-		/*
-		 * Check that the GuC has space for the request before
-		 * going any further, as the i915_add_request() call
-		 * later on mustn't fail ...
-		 */
-		ret = i915_guc_wq_reserve(request);
-		if (ret)
-			goto err;
-	}
-
 	cs = intel_ring_begin(request, 0);
-	if (IS_ERR(cs)) {
-		ret = PTR_ERR(cs);
-		goto err_unreserve;
-	}
+	if (IS_ERR(cs))
+		return PTR_ERR(cs);
 
 	if (!ce->initialised) {
 		ret = engine->init_context(request);
 		if (ret)
-			goto err_unreserve;
+			return ret;
 
 		ce->initialised = true;
 	}
@@ -960,12 +947,6 @@ static int execlists_request_alloc(struct drm_i915_gem_request *request)
 
 	request->reserved_space -= EXECLISTS_REQUEST_SIZE;
 	return 0;
-
-err_unreserve:
-	if (i915.enable_guc_submission)
-		i915_guc_wq_unreserve(request);
-err:
-	return ret;
 }
 
 /*
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 69daf4c01cd0..d41051688221 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -52,13 +52,6 @@ struct drm_i915_gem_request;
  * GuC). The subsequent  pages of the client object constitute the work
  * queue (a circular array of work items), again described in the process
  * descriptor. Work queue pages are mapped momentarily as required.
- *
- * We also keep a few statistics on failures. Ideally, these should all
- * be zero!
- *   no_wq_space: times that the submission pre-check found no space was
- *                available in the work queue (note, the queue is shared,
- *                not per-engine). It is OK for this to be nonzero, but
- *                it should not be huge!
  */
 struct i915_guc_client {
 	struct i915_vma *vma;
@@ -79,8 +72,6 @@ struct i915_guc_client {
 	uint32_t wq_offset;
 	uint32_t wq_size;
 	uint32_t wq_tail;
-	uint32_t wq_rsvd;
-	uint32_t no_wq_space;
 
 	/* Per-engine counts of GuC submissions */
 	uint64_t submissions[I915_NUM_ENGINES];
@@ -246,8 +237,6 @@ u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv);
 /* i915_guc_submission.c */
 int i915_guc_submission_init(struct drm_i915_private *dev_priv);
 int i915_guc_submission_enable(struct drm_i915_private *dev_priv);
-int i915_guc_wq_reserve(struct drm_i915_gem_request *rq);
-void i915_guc_wq_unreserve(struct drm_i915_gem_request *request);
 void i915_guc_submission_disable(struct drm_i915_private *dev_priv);
 void i915_guc_submission_fini(struct drm_i915_private *dev_priv);
 struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size);
-- 
2.13.5

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

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

* [PATCH v3 3/5] drm/i915/guc: Simplify GuC doorbell logic
  2017-09-14  8:32 [PATCH 1/5] drm/i915/guc: Remove obsolete comments and remove unused variable Michał Winiarski
  2017-09-14  8:32 ` [PATCH v5 2/5] drm/i915/guc: Submit GuC workitems containing coalesced requests Michał Winiarski
@ 2017-09-14  8:32 ` Michał Winiarski
  2017-09-14 10:37   ` Chris Wilson
  2017-09-14 10:51   ` [PATCH v4 " Michał Winiarski
  2017-09-14  8:32 ` [PATCH 4/5] drm/i915/guc: Cleanup adding GuC work items Michał Winiarski
                   ` (5 subsequent siblings)
  7 siblings, 2 replies; 21+ messages in thread
From: Michał Winiarski @ 2017-09-14  8:32 UTC (permalink / raw)
  To: intel-gfx

All we're really doing is incrementing a simple counter in a
doorbell_info struct. We can do without extra variables and a separate
counter kept in guc_client. Since it's gone, we're also removing its
debugfs.
The only functional change here, is that we're no longer treating 0 as a
special value. GuC doesn't seem to care, why should we?

v2: Restore desc->tail update.
v3: Drop the retry loop, assert that doorbell cookie doesn't change
behind our back.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c        |  4 +-
 drivers/gpu/drm/i915/i915_guc_submission.c | 61 +++++++-----------------------
 drivers/gpu/drm/i915/intel_uc.h            |  1 -
 3 files changed, 16 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index a4c2d0718a70..46ac6091772e 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2445,8 +2445,8 @@ static void i915_guc_client_info(struct seq_file *m,
 
 	seq_printf(m, "\tPriority %d, GuC stage index: %u, PD offset 0x%x\n",
 		client->priority, client->stage_id, client->proc_desc_offset);
-	seq_printf(m, "\tDoorbell id %d, offset: 0x%lx, cookie 0x%x\n",
-		client->doorbell_id, client->doorbell_offset, client->doorbell_cookie);
+	seq_printf(m, "\tDoorbell id %d, offset: 0x%lx\n",
+		client->doorbell_id, client->doorbell_offset);
 	seq_printf(m, "\tWQ size %d, offset: 0x%x, tail %d\n",
 		client->wq_size, client->wq_offset, client->wq_tail);
 
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 84beedae3708..360be80613ba 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -192,13 +192,12 @@ static int __create_doorbell(struct i915_guc_client *client)
 
 	doorbell = __get_doorbell(client);
 	doorbell->db_status = GUC_DOORBELL_ENABLED;
-	doorbell->cookie = client->doorbell_cookie;
+	doorbell->cookie = 0;
 
 	err = __guc_allocate_doorbell(client->guc, client->stage_id);
-	if (err) {
+	if (err)
 		doorbell->db_status = GUC_DOORBELL_DISABLED;
-		doorbell->cookie = 0;
-	}
+
 	return err;
 }
 
@@ -466,57 +465,25 @@ static void guc_reset_wq(struct i915_guc_client *client)
 	client->wq_tail = 0;
 }
 
-static int guc_ring_doorbell(struct i915_guc_client *client)
+static void guc_ring_doorbell(struct i915_guc_client *client)
 {
 	struct guc_process_desc *desc = __get_process_desc(client);
-	union guc_doorbell_qw db_cmp, db_exc, db_ret;
-	union guc_doorbell_qw *db;
-	int attempt = 2, ret = -EAGAIN;
+	struct guc_doorbell_info *db;
+	u32 cookie;
 
 	/* Update the tail so it is visible to GuC */
 	desc->tail = client->wq_tail;
 
-	/* current cookie */
-	db_cmp.db_status = GUC_DOORBELL_ENABLED;
-	db_cmp.cookie = client->doorbell_cookie;
-
-	/* cookie to be updated */
-	db_exc.db_status = GUC_DOORBELL_ENABLED;
-	db_exc.cookie = client->doorbell_cookie + 1;
-	if (db_exc.cookie == 0)
-		db_exc.cookie = 1;
-
 	/* pointer of current doorbell cacheline */
-	db = (union guc_doorbell_qw *)__get_doorbell(client);
-
-	while (attempt--) {
-		/* lets ring the doorbell */
-		db_ret.value_qw = atomic64_cmpxchg((atomic64_t *)db,
-			db_cmp.value_qw, db_exc.value_qw);
-
-		/* if the exchange was successfully executed */
-		if (db_ret.value_qw == db_cmp.value_qw) {
-			/* db was successfully rung */
-			client->doorbell_cookie = db_exc.cookie;
-			ret = 0;
-			break;
-		}
-
-		/* XXX: doorbell was lost and need to acquire it again */
-		if (db_ret.db_status == GUC_DOORBELL_DISABLED)
-			break;
+	db = __get_doorbell(client);
 
-		DRM_WARN("Cookie mismatch. Expected %d, found %d\n",
-			 db_cmp.cookie, db_ret.cookie);
+	/* we're not expecting the doorbell cookie to change behind our back */
+	cookie = READ_ONCE(db->cookie);
+	if (cmpxchg(&db->cookie, cookie, cookie + 1) != cookie)
+		BUG();
 
-		/* update the cookie to newly read cookie from GuC */
-		db_cmp.cookie = db_ret.cookie;
-		db_exc.cookie = db_ret.cookie + 1;
-		if (db_exc.cookie == 0)
-			db_exc.cookie = 1;
-	}
-
-	return ret;
+	/* XXX: doorbell was lost and need to acquire it again */
+	GEM_BUG_ON(db->db_status != GUC_DOORBELL_ENABLED);
 }
 
 /**
@@ -550,7 +517,7 @@ static void i915_guc_submit(struct intel_engine_cs *engine)
 			spin_lock_irqsave(&client->wq_lock, flags);
 
 			guc_wq_item_append(client, rq);
-			WARN_ON(guc_ring_doorbell(client));
+			guc_ring_doorbell(client);
 
 			client->submissions[engine_id] += 1;
 
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index d41051688221..f505dcccd613 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -66,7 +66,6 @@ struct i915_guc_client {
 
 	u16 doorbell_id;
 	unsigned long doorbell_offset;
-	u32 doorbell_cookie;
 
 	spinlock_t wq_lock;
 	uint32_t wq_offset;
-- 
2.13.5

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

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

* [PATCH 4/5] drm/i915/guc: Cleanup adding GuC work items
  2017-09-14  8:32 [PATCH 1/5] drm/i915/guc: Remove obsolete comments and remove unused variable Michał Winiarski
  2017-09-14  8:32 ` [PATCH v5 2/5] drm/i915/guc: Submit GuC workitems containing coalesced requests Michał Winiarski
  2017-09-14  8:32 ` [PATCH v3 3/5] drm/i915/guc: Simplify GuC doorbell logic Michał Winiarski
@ 2017-09-14  8:32 ` Michał Winiarski
  2017-09-14 10:38   ` Chris Wilson
                     ` (2 more replies)
  2017-09-14  8:32 ` [PATCH 5/5] HAX Enable GuC Submission for CI Michał Winiarski
                   ` (4 subsequent siblings)
  7 siblings, 3 replies; 21+ messages in thread
From: Michał Winiarski @ 2017-09-14  8:32 UTC (permalink / raw)
  To: intel-gfx

We can just operate on the wq_tail directly (in the process descriptor).
This allows us to remove the duplicated tail from the client. While I'm
here let's also remove the constants kept in the client and document our
locking requirements. This causes a small change in one of GuC debugfs
files. We're no longer reporting constant values (which I don't think
is a problem), but we're also no longer reporting the tail (does anyone
care?).

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c        |  2 --
 drivers/gpu/drm/i915/i915_guc_submission.c | 35 ++++++++++++------------------
 drivers/gpu/drm/i915/intel_uc.h            |  4 ----
 3 files changed, 14 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 46ac6091772e..2518bdf95eef 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2447,8 +2447,6 @@ static void i915_guc_client_info(struct seq_file *m,
 		client->priority, client->stage_id, client->proc_desc_offset);
 	seq_printf(m, "\tDoorbell id %d, offset: 0x%lx\n",
 		client->doorbell_id, client->doorbell_offset);
-	seq_printf(m, "\tWQ size %d, offset: 0x%x, tail %d\n",
-		client->wq_size, client->wq_offset, client->wq_tail);
 
 	for_each_engine(engine, dev_priv, id) {
 		u64 submissions = client->submissions[id];
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 360be80613ba..3f9d227154a5 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -305,7 +305,7 @@ static void guc_proc_desc_init(struct intel_guc *guc,
 	desc->db_base_addr = 0;
 
 	desc->stage_id = client->stage_id;
-	desc->wq_size_bytes = client->wq_size;
+	desc->wq_size_bytes = GUC_WQ_SIZE;
 	desc->wq_status = WQ_STATUS_ACTIVE;
 	desc->priority = client->priority;
 }
@@ -390,8 +390,8 @@ static void guc_stage_desc_init(struct intel_guc *guc,
 	desc->db_trigger_cpu = (uintptr_t)__get_doorbell(client);
 	desc->db_trigger_uk = gfx_addr + client->doorbell_offset;
 	desc->process_desc = gfx_addr + client->proc_desc_offset;
-	desc->wq_addr = gfx_addr + client->wq_offset;
-	desc->wq_size = client->wq_size;
+	desc->wq_addr = gfx_addr + GUC_DB_SIZE;
+	desc->wq_size = GUC_WQ_SIZE;
 
 	desc->desc_private = (uintptr_t)client;
 }
@@ -416,14 +416,12 @@ static void guc_wq_item_append(struct i915_guc_client *client,
 	struct i915_gem_context *ctx = rq->ctx;
 	struct guc_process_desc *desc = __get_process_desc(client);
 	struct guc_wq_item *wqi;
-	u32 freespace, tail, wq_off;
+	u32 ring_tail, wq_off;
 
-	/* Free space is guaranteed */
-	freespace = CIRC_SPACE(client->wq_tail, desc->head, client->wq_size);
-	GEM_BUG_ON(freespace < wqi_size);
+	lockdep_assert_held(&client->wq_lock);
 
-	tail = intel_ring_set_tail(rq->ring, rq->tail) / sizeof(u64);
-	GEM_BUG_ON(tail > WQ_RING_TAIL_MAX);
+	ring_tail = intel_ring_set_tail(rq->ring, rq->tail) / sizeof(u64);
+	GEM_BUG_ON(ring_tail > WQ_RING_TAIL_MAX);
 
 	/* For now workqueue item is 4 DWs; workqueue buffer is 2 pages. So we
 	 * should not have the case where structure wqi is across page, neither
@@ -434,11 +432,12 @@ static void guc_wq_item_append(struct i915_guc_client *client,
 	 */
 	BUILD_BUG_ON(wqi_size != 16);
 
-	/* postincrement WQ tail for next time */
-	wq_off = client->wq_tail;
+	/* Postincrement WQ tail for next time, free space is guaranteed. */
+	wq_off = READ_ONCE(desc->tail);
+	GEM_BUG_ON(CIRC_SPACE(wq_off, READ_ONCE(desc->head),
+			      GUC_WQ_SIZE) < wqi_size);
+	WRITE_ONCE(desc->tail, (wq_off + wqi_size) & (GUC_WQ_SIZE - 1));
 	GEM_BUG_ON(wq_off & (wqi_size - 1));
-	client->wq_tail += wqi_size;
-	client->wq_tail &= client->wq_size - 1;
 
 	/* WQ starts from the page after doorbell / process_desc */
 	wqi = client->vaddr + wq_off + GUC_DB_SIZE;
@@ -451,7 +450,7 @@ static void guc_wq_item_append(struct i915_guc_client *client,
 
 	wqi->context_desc = lower_32_bits(intel_lr_context_descriptor(ctx, engine));
 
-	wqi->submit_element_info = tail << WQ_RING_TAIL_SHIFT;
+	wqi->submit_element_info = ring_tail << WQ_RING_TAIL_SHIFT;
 	wqi->fence_id = rq->global_seqno;
 }
 
@@ -461,18 +460,14 @@ static void guc_reset_wq(struct i915_guc_client *client)
 
 	desc->head = 0;
 	desc->tail = 0;
-
-	client->wq_tail = 0;
 }
 
 static void guc_ring_doorbell(struct i915_guc_client *client)
 {
-	struct guc_process_desc *desc = __get_process_desc(client);
 	struct guc_doorbell_info *db;
 	u32 cookie;
 
-	/* Update the tail so it is visible to GuC */
-	desc->tail = client->wq_tail;
+	lockdep_assert_held(&client->wq_lock);
 
 	/* pointer of current doorbell cacheline */
 	db = __get_doorbell(client);
@@ -814,8 +809,6 @@ guc_client_alloc(struct drm_i915_private *dev_priv,
 	client->engines = engines;
 	client->priority = priority;
 	client->doorbell_id = GUC_DOORBELL_INVALID;
-	client->wq_offset = GUC_DB_SIZE;
-	client->wq_size = GUC_WQ_SIZE;
 	spin_lock_init(&client->wq_lock);
 
 	ret = ida_simple_get(&guc->stage_ids, 0, GUC_MAX_STAGE_DESCRIPTORS,
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index f505dcccd613..7703c9ad6511 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -68,10 +68,6 @@ struct i915_guc_client {
 	unsigned long doorbell_offset;
 
 	spinlock_t wq_lock;
-	uint32_t wq_offset;
-	uint32_t wq_size;
-	uint32_t wq_tail;
-
 	/* Per-engine counts of GuC submissions */
 	uint64_t submissions[I915_NUM_ENGINES];
 };
-- 
2.13.5

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

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

* [PATCH 5/5] HAX Enable GuC Submission for CI
  2017-09-14  8:32 [PATCH 1/5] drm/i915/guc: Remove obsolete comments and remove unused variable Michał Winiarski
                   ` (2 preceding siblings ...)
  2017-09-14  8:32 ` [PATCH 4/5] drm/i915/guc: Cleanup adding GuC work items Michał Winiarski
@ 2017-09-14  8:32 ` Michał Winiarski
  2017-09-14  9:04 ` ✗ Fi.CI.BAT: failure for series starting with [1/5] drm/i915/guc: Remove obsolete comments and remove unused variable Patchwork
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 21+ messages in thread
From: Michał Winiarski @ 2017-09-14  8:32 UTC (permalink / raw)
  To: intel-gfx

Also:
Revert "drm/i915/guc: Assert that we switch between known ggtt->invalidate functions"

This reverts commit 04f7b24eccdfae680a36e9825fe0d61dcd5ed528.
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 8 ++------
 drivers/gpu/drm/i915/i915_params.c  | 4 ++--
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 09e524dbc090..478a8d42aeb0 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3189,17 +3189,13 @@ int i915_ggtt_enable_hw(struct drm_i915_private *dev_priv)
 
 void i915_ggtt_enable_guc(struct drm_i915_private *i915)
 {
-	GEM_BUG_ON(i915->ggtt.invalidate != gen6_ggtt_invalidate);
-
 	i915->ggtt.invalidate = guc_ggtt_invalidate;
 }
 
 void i915_ggtt_disable_guc(struct drm_i915_private *i915)
 {
-	/* We should only be called after i915_ggtt_enable_guc() */
-	GEM_BUG_ON(i915->ggtt.invalidate != guc_ggtt_invalidate);
-
-	i915->ggtt.invalidate = gen6_ggtt_invalidate;
+	if (i915->ggtt.invalidate == guc_ggtt_invalidate)
+		i915->ggtt.invalidate = gen6_ggtt_invalidate;
 }
 
 void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 8ab003dca113..c9d72f1b8383 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -56,8 +56,8 @@ struct i915_params i915 __read_mostly = {
 	.verbose_state_checks = 1,
 	.nuclear_pageflip = 0,
 	.edp_vswing = 0,
-	.enable_guc_loading = 0,
-	.enable_guc_submission = 0,
+	.enable_guc_loading = 2,
+	.enable_guc_submission = 2,
 	.guc_log_level = -1,
 	.guc_firmware_path = NULL,
 	.huc_firmware_path = NULL,
-- 
2.13.5

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

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

* ✗ Fi.CI.BAT: failure for series starting with [1/5] drm/i915/guc: Remove obsolete comments and remove unused variable
  2017-09-14  8:32 [PATCH 1/5] drm/i915/guc: Remove obsolete comments and remove unused variable Michał Winiarski
                   ` (3 preceding siblings ...)
  2017-09-14  8:32 ` [PATCH 5/5] HAX Enable GuC Submission for CI Michał Winiarski
@ 2017-09-14  9:04 ` Patchwork
  2017-09-14 10:28   ` Michał Winiarski
  2017-09-14 11:12 ` ✗ Fi.CI.BAT: failure for series starting with [1/5] drm/i915/guc: Remove obsolete comments and remove unused variable (rev2) Patchwork
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 21+ messages in thread
From: Patchwork @ 2017-09-14  9:04 UTC (permalink / raw)
  To: Michał Winiarski; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/5] drm/i915/guc: Remove obsolete comments and remove unused variable
URL   : https://patchwork.freedesktop.org/series/30345/
State : failure

== Summary ==

Series 30345v1 series starting with [1/5] drm/i915/guc: Remove obsolete comments and remove unused variable
https://patchwork.freedesktop.org/api/1.0/series/30345/revisions/1/mbox/

Test chamelium:
        Subgroup dp-crc-fast:
                fail       -> PASS       (fi-kbl-7500u) fdo#102514
Test debugfs_test:
        Subgroup read_all_entries:
                pass       -> SKIP       (fi-glk-2a)
Test drv_hangman:
        Subgroup error-state-basic:
                pass       -> SKIP       (fi-glk-2a)
Test gem_busy:
        Subgroup basic-busy-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-hang-default:
                pass       -> SKIP       (fi-glk-2a)
Test gem_close_race:
        Subgroup basic-process:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-threads:
                pass       -> SKIP       (fi-glk-2a)
Test gem_cpu_reloc:
        Subgroup basic:
                pass       -> SKIP       (fi-glk-2a)
Test gem_cs_tlb:
        Subgroup basic-default:
                pass       -> SKIP       (fi-glk-2a)
Test gem_ctx_create:
        Subgroup basic:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-files:
                pass       -> SKIP       (fi-glk-2a)
Test gem_ctx_exec:
        Subgroup basic:
                pass       -> SKIP       (fi-glk-2a)
Test gem_ctx_switch:
        Subgroup basic-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-default-heavy:
                pass       -> SKIP       (fi-glk-2a)
Test gem_exec_basic:
        Subgroup basic-blt:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-bsd:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-render:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-vebox:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup gtt-blt:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup gtt-bsd:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup gtt-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup gtt-render:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup gtt-vebox:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup readonly-blt:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup readonly-bsd:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup readonly-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup readonly-render:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup readonly-vebox:
                pass       -> SKIP       (fi-glk-2a)
Test gem_exec_create:
        Subgroup basic:
                pass       -> SKIP       (fi-glk-2a)
Test gem_exec_fence:
        Subgroup basic-busy-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-wait-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-await-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup await-hang-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup nb-await-default:
                pass       -> SKIP       (fi-glk-2a)
Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-batch-kernel-default-wb:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-uc-pro-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-uc-prw-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-uc-ro-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-uc-rw-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-uc-set-default:
WARNING: Long output truncated
fi-pnv-d510 failed to connect after reboot

b21142588a9a16f74988f6b524953d4382be4a81 drm-tip: 2017y-09m-14d-08h-23m-57s UTC integration manifest
04fa653656cf HAX Enable GuC Submission for CI
8473b76dbe42 drm/i915/guc: Cleanup adding GuC work items
6800bd8067ca drm/i915/guc: Simplify GuC doorbell logic
158ad955fb26 drm/i915/guc: Submit GuC workitems containing coalesced requests
ef09cda09694 drm/i915/guc: Remove obsolete comments and remove unused variable

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5693/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: ✗ Fi.CI.BAT: failure for series starting with [1/5] drm/i915/guc: Remove obsolete comments and remove unused variable
  2017-09-14  9:04 ` ✗ Fi.CI.BAT: failure for series starting with [1/5] drm/i915/guc: Remove obsolete comments and remove unused variable Patchwork
@ 2017-09-14 10:28   ` Michał Winiarski
  2017-09-14 10:40     ` Chris Wilson
  0 siblings, 1 reply; 21+ messages in thread
From: Michał Winiarski @ 2017-09-14 10:28 UTC (permalink / raw)
  To: intel-gfx

On Thu, Sep 14, 2017 at 09:04:57AM +0000, Patchwork wrote:
> == Series Details ==
> 
> Series: series starting with [1/5] drm/i915/guc: Remove obsolete comments and remove unused variable
> URL   : https://patchwork.freedesktop.org/series/30345/
> State : failure
> 
> == Summary ==
> 
> Series 30345v1 series starting with [1/5] drm/i915/guc: Remove obsolete comments and remove unused variable
> https://patchwork.freedesktop.org/api/1.0/series/30345/revisions/1/mbox/
> 
> Test chamelium:
>         Subgroup dp-crc-fast:
>                 fail       -> PASS       (fi-kbl-7500u) fdo#102514
> Test debugfs_test:
>         Subgroup read_all_entries:
>                 pass       -> SKIP       (fi-glk-2a)
> Test drv_hangman:
>         Subgroup error-state-basic:
>                 pass       -> SKIP       (fi-glk-2a)
> Test gem_busy:
>         Subgroup basic-busy-default:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup basic-hang-default:
>                 pass       -> SKIP       (fi-glk-2a)
> Test gem_close_race:
>         Subgroup basic-process:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup basic-threads:
>                 pass       -> SKIP       (fi-glk-2a)
> Test gem_cpu_reloc:
>         Subgroup basic:
>                 pass       -> SKIP       (fi-glk-2a)
> Test gem_cs_tlb:
>         Subgroup basic-default:
>                 pass       -> SKIP       (fi-glk-2a)
> Test gem_ctx_create:
>         Subgroup basic:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup basic-files:
>                 pass       -> SKIP       (fi-glk-2a)
> Test gem_ctx_exec:
>         Subgroup basic:
>                 pass       -> SKIP       (fi-glk-2a)
> Test gem_ctx_switch:
>         Subgroup basic-default:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup basic-default-heavy:
>                 pass       -> SKIP       (fi-glk-2a)
> Test gem_exec_basic:
>         Subgroup basic-blt:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup basic-bsd:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup basic-default:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup basic-render:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup basic-vebox:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup gtt-blt:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup gtt-bsd:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup gtt-default:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup gtt-render:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup gtt-vebox:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup readonly-blt:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup readonly-bsd:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup readonly-default:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup readonly-render:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup readonly-vebox:
>                 pass       -> SKIP       (fi-glk-2a)
> Test gem_exec_create:
>         Subgroup basic:
>                 pass       -> SKIP       (fi-glk-2a)
> Test gem_exec_fence:
>         Subgroup basic-busy-default:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup basic-wait-default:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup basic-await-default:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup await-hang-default:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup nb-await-default:
>                 pass       -> SKIP       (fi-glk-2a)
> Test gem_exec_flush:
>         Subgroup basic-batch-kernel-default-uc:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup basic-batch-kernel-default-wb:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup basic-uc-pro-default:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup basic-uc-prw-default:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup basic-uc-ro-default:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup basic-uc-rw-default:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup basic-uc-set-default:
> WARNING: Long output truncated
> fi-pnv-d510 failed to connect after reboot
> 
> b21142588a9a16f74988f6b524953d4382be4a81 drm-tip: 2017y-09m-14d-08h-23m-57s UTC integration manifest
> 04fa653656cf HAX Enable GuC Submission for CI
> 8473b76dbe42 drm/i915/guc: Cleanup adding GuC work items
> 6800bd8067ca drm/i915/guc: Simplify GuC doorbell logic
> 158ad955fb26 drm/i915/guc: Submit GuC workitems containing coalesced requests
> ef09cda09694 drm/i915/guc: Remove obsolete comments and remove unused variable

I think the only suspicious thing in the results, is the GPU hang in gem_sync.
However, I'm also able to reproduce it locally without the series (with GuC
submission enabled).

-Michał

> 
> == Logs ==
> 
> For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5693/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v3 3/5] drm/i915/guc: Simplify GuC doorbell logic
  2017-09-14  8:32 ` [PATCH v3 3/5] drm/i915/guc: Simplify GuC doorbell logic Michał Winiarski
@ 2017-09-14 10:37   ` Chris Wilson
  2017-09-14 10:51   ` [PATCH v4 " Michał Winiarski
  1 sibling, 0 replies; 21+ messages in thread
From: Chris Wilson @ 2017-09-14 10:37 UTC (permalink / raw)
  To: Michał Winiarski, intel-gfx

Quoting Michał Winiarski (2017-09-14 09:32:14)
> +       /* we're not expecting the doorbell cookie to change behind our back */
> +       cookie = READ_ONCE(db->cookie);
> +       if (cmpxchg(&db->cookie, cookie, cookie + 1) != cookie)
> +               BUG();

Or just
WARN_ON_ONCE(xchg(&db->cookie, cookie + 1) != cookie);
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 4/5] drm/i915/guc: Cleanup adding GuC work items
  2017-09-14  8:32 ` [PATCH 4/5] drm/i915/guc: Cleanup adding GuC work items Michał Winiarski
@ 2017-09-14 10:38   ` Chris Wilson
  2017-09-17 12:47   ` Chris Wilson
  2017-09-18  8:30   ` [PATCH v2 " Michał Winiarski
  2 siblings, 0 replies; 21+ messages in thread
From: Chris Wilson @ 2017-09-14 10:38 UTC (permalink / raw)
  To: Michał Winiarski, intel-gfx

Quoting Michał Winiarski (2017-09-14 09:32:15)
> We can just operate on the wq_tail directly (in the process descriptor).
> This allows us to remove the duplicated tail from the client. While I'm
> here let's also remove the constants kept in the client and document our
> locking requirements. This causes a small change in one of GuC debugfs
> files. We're no longer reporting constant values (which I don't think
> is a problem), but we're also no longer reporting the tail (does anyone
> care?).
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Oscar Mateo <oscar.mateo@intel.com>
> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: ✗ Fi.CI.BAT: failure for series starting with [1/5] drm/i915/guc: Remove obsolete comments and remove unused variable
  2017-09-14 10:28   ` Michał Winiarski
@ 2017-09-14 10:40     ` Chris Wilson
  0 siblings, 0 replies; 21+ messages in thread
From: Chris Wilson @ 2017-09-14 10:40 UTC (permalink / raw)
  To: Michał Winiarski, intel-gfx

Quoting Michał Winiarski (2017-09-14 11:28:11)
> On Thu, Sep 14, 2017 at 09:04:57AM +0000, Patchwork wrote:
> > == Series Details ==
> > 
> > Series: series starting with [1/5] drm/i915/guc: Remove obsolete comments and remove unused variable
> > URL   : https://patchwork.freedesktop.org/series/30345/
> > State : failure
> > 
> > == Summary ==
> > 
> > Series 30345v1 series starting with [1/5] drm/i915/guc: Remove obsolete comments and remove unused variable
> > https://patchwork.freedesktop.org/api/1.0/series/30345/revisions/1/mbox/
> > 
> > Test chamelium:
> >         Subgroup dp-crc-fast:
> >                 fail       -> PASS       (fi-kbl-7500u) fdo#102514
> > Test debugfs_test:
> >         Subgroup read_all_entries:
> >                 pass       -> SKIP       (fi-glk-2a)
> > Test drv_hangman:
> >         Subgroup error-state-basic:
> >                 pass       -> SKIP       (fi-glk-2a)
> > Test gem_busy:
> >         Subgroup basic-busy-default:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup basic-hang-default:
> >                 pass       -> SKIP       (fi-glk-2a)
> > Test gem_close_race:
> >         Subgroup basic-process:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup basic-threads:
> >                 pass       -> SKIP       (fi-glk-2a)
> > Test gem_cpu_reloc:
> >         Subgroup basic:
> >                 pass       -> SKIP       (fi-glk-2a)
> > Test gem_cs_tlb:
> >         Subgroup basic-default:
> >                 pass       -> SKIP       (fi-glk-2a)
> > Test gem_ctx_create:
> >         Subgroup basic:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup basic-files:
> >                 pass       -> SKIP       (fi-glk-2a)
> > Test gem_ctx_exec:
> >         Subgroup basic:
> >                 pass       -> SKIP       (fi-glk-2a)
> > Test gem_ctx_switch:
> >         Subgroup basic-default:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup basic-default-heavy:
> >                 pass       -> SKIP       (fi-glk-2a)
> > Test gem_exec_basic:
> >         Subgroup basic-blt:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup basic-bsd:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup basic-default:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup basic-render:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup basic-vebox:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup gtt-blt:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup gtt-bsd:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup gtt-default:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup gtt-render:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup gtt-vebox:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup readonly-blt:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup readonly-bsd:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup readonly-default:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup readonly-render:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup readonly-vebox:
> >                 pass       -> SKIP       (fi-glk-2a)
> > Test gem_exec_create:
> >         Subgroup basic:
> >                 pass       -> SKIP       (fi-glk-2a)
> > Test gem_exec_fence:
> >         Subgroup basic-busy-default:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup basic-wait-default:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup basic-await-default:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup await-hang-default:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup nb-await-default:
> >                 pass       -> SKIP       (fi-glk-2a)
> > Test gem_exec_flush:
> >         Subgroup basic-batch-kernel-default-uc:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup basic-batch-kernel-default-wb:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup basic-uc-pro-default:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup basic-uc-prw-default:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup basic-uc-ro-default:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup basic-uc-rw-default:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup basic-uc-set-default:
> > WARNING: Long output truncated
> > fi-pnv-d510 failed to connect after reboot
> > 
> > b21142588a9a16f74988f6b524953d4382be4a81 drm-tip: 2017y-09m-14d-08h-23m-57s UTC integration manifest
> > 04fa653656cf HAX Enable GuC Submission for CI
> > 8473b76dbe42 drm/i915/guc: Cleanup adding GuC work items
> > 6800bd8067ca drm/i915/guc: Simplify GuC doorbell logic
> > 158ad955fb26 drm/i915/guc: Submit GuC workitems containing coalesced requests
> > ef09cda09694 drm/i915/guc: Remove obsolete comments and remove unused variable
> 
> I think the only suspicious thing in the results, is the GPU hang in gem_sync.
> However, I'm also able to reproduce it locally without the series (with GuC
> submission enabled).

And it's been fluctuating between different versions of this series as
well (without an obvious correlation to the series/versions).
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [PATCH v4 3/5] drm/i915/guc: Simplify GuC doorbell logic
  2017-09-14  8:32 ` [PATCH v3 3/5] drm/i915/guc: Simplify GuC doorbell logic Michał Winiarski
  2017-09-14 10:37   ` Chris Wilson
@ 2017-09-14 10:51   ` Michał Winiarski
  1 sibling, 0 replies; 21+ messages in thread
From: Michał Winiarski @ 2017-09-14 10:51 UTC (permalink / raw)
  To: intel-gfx

All we're really doing is incrementing a simple counter in a
doorbell_info struct. We can do without extra variables and a separate
counter kept in guc_client. Since it's gone, we're also removing its
debugfs.
The only functional change here, is that we're no longer treating 0 as a
special value. GuC doesn't seem to care, why should we?

v2: Restore desc->tail update.
v3: Drop the retry loop, assert that doorbell cookie doesn't change
behind our back.
v4: WARN rather than BUG, use xchg. (Chris)

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c        |  4 +-
 drivers/gpu/drm/i915/i915_guc_submission.c | 60 +++++++-----------------------
 drivers/gpu/drm/i915/intel_uc.h            |  1 -
 3 files changed, 15 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index a4c2d0718a70..46ac6091772e 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2445,8 +2445,8 @@ static void i915_guc_client_info(struct seq_file *m,
 
 	seq_printf(m, "\tPriority %d, GuC stage index: %u, PD offset 0x%x\n",
 		client->priority, client->stage_id, client->proc_desc_offset);
-	seq_printf(m, "\tDoorbell id %d, offset: 0x%lx, cookie 0x%x\n",
-		client->doorbell_id, client->doorbell_offset, client->doorbell_cookie);
+	seq_printf(m, "\tDoorbell id %d, offset: 0x%lx\n",
+		client->doorbell_id, client->doorbell_offset);
 	seq_printf(m, "\tWQ size %d, offset: 0x%x, tail %d\n",
 		client->wq_size, client->wq_offset, client->wq_tail);
 
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 84beedae3708..16ce570a0b74 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -192,13 +192,12 @@ static int __create_doorbell(struct i915_guc_client *client)
 
 	doorbell = __get_doorbell(client);
 	doorbell->db_status = GUC_DOORBELL_ENABLED;
-	doorbell->cookie = client->doorbell_cookie;
+	doorbell->cookie = 0;
 
 	err = __guc_allocate_doorbell(client->guc, client->stage_id);
-	if (err) {
+	if (err)
 		doorbell->db_status = GUC_DOORBELL_DISABLED;
-		doorbell->cookie = 0;
-	}
+
 	return err;
 }
 
@@ -466,57 +465,24 @@ static void guc_reset_wq(struct i915_guc_client *client)
 	client->wq_tail = 0;
 }
 
-static int guc_ring_doorbell(struct i915_guc_client *client)
+static void guc_ring_doorbell(struct i915_guc_client *client)
 {
 	struct guc_process_desc *desc = __get_process_desc(client);
-	union guc_doorbell_qw db_cmp, db_exc, db_ret;
-	union guc_doorbell_qw *db;
-	int attempt = 2, ret = -EAGAIN;
+	struct guc_doorbell_info *db;
+	u32 cookie;
 
 	/* Update the tail so it is visible to GuC */
 	desc->tail = client->wq_tail;
 
-	/* current cookie */
-	db_cmp.db_status = GUC_DOORBELL_ENABLED;
-	db_cmp.cookie = client->doorbell_cookie;
-
-	/* cookie to be updated */
-	db_exc.db_status = GUC_DOORBELL_ENABLED;
-	db_exc.cookie = client->doorbell_cookie + 1;
-	if (db_exc.cookie == 0)
-		db_exc.cookie = 1;
-
 	/* pointer of current doorbell cacheline */
-	db = (union guc_doorbell_qw *)__get_doorbell(client);
-
-	while (attempt--) {
-		/* lets ring the doorbell */
-		db_ret.value_qw = atomic64_cmpxchg((atomic64_t *)db,
-			db_cmp.value_qw, db_exc.value_qw);
-
-		/* if the exchange was successfully executed */
-		if (db_ret.value_qw == db_cmp.value_qw) {
-			/* db was successfully rung */
-			client->doorbell_cookie = db_exc.cookie;
-			ret = 0;
-			break;
-		}
-
-		/* XXX: doorbell was lost and need to acquire it again */
-		if (db_ret.db_status == GUC_DOORBELL_DISABLED)
-			break;
+	db = __get_doorbell(client);
 
-		DRM_WARN("Cookie mismatch. Expected %d, found %d\n",
-			 db_cmp.cookie, db_ret.cookie);
+	/* we're not expecting the doorbell cookie to change behind our back */
+	cookie = READ_ONCE(db->cookie);
+	WARN_ON_ONCE(xchg(&db->cookie, cookie + 1) != cookie);
 
-		/* update the cookie to newly read cookie from GuC */
-		db_cmp.cookie = db_ret.cookie;
-		db_exc.cookie = db_ret.cookie + 1;
-		if (db_exc.cookie == 0)
-			db_exc.cookie = 1;
-	}
-
-	return ret;
+	/* XXX: doorbell was lost and need to acquire it again */
+	GEM_BUG_ON(db->db_status != GUC_DOORBELL_ENABLED);
 }
 
 /**
@@ -550,7 +516,7 @@ static void i915_guc_submit(struct intel_engine_cs *engine)
 			spin_lock_irqsave(&client->wq_lock, flags);
 
 			guc_wq_item_append(client, rq);
-			WARN_ON(guc_ring_doorbell(client));
+			guc_ring_doorbell(client);
 
 			client->submissions[engine_id] += 1;
 
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index d41051688221..f505dcccd613 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -66,7 +66,6 @@ struct i915_guc_client {
 
 	u16 doorbell_id;
 	unsigned long doorbell_offset;
-	u32 doorbell_cookie;
 
 	spinlock_t wq_lock;
 	uint32_t wq_offset;
-- 
2.13.5

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

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

* ✗ Fi.CI.BAT: failure for series starting with [1/5] drm/i915/guc: Remove obsolete comments and remove unused variable (rev2)
  2017-09-14  8:32 [PATCH 1/5] drm/i915/guc: Remove obsolete comments and remove unused variable Michał Winiarski
                   ` (4 preceding siblings ...)
  2017-09-14  9:04 ` ✗ Fi.CI.BAT: failure for series starting with [1/5] drm/i915/guc: Remove obsolete comments and remove unused variable Patchwork
@ 2017-09-14 11:12 ` Patchwork
  2017-09-18  8:56 ` ✗ Fi.CI.BAT: failure for series starting with [1/5] drm/i915/guc: Remove obsolete comments and remove unused variable (rev3) Patchwork
  2017-09-18 10:03 ` ✗ Fi.CI.BAT: failure for series starting with [1/5] drm/i915/guc: Remove obsolete comments and remove unused variable (rev4) Patchwork
  7 siblings, 0 replies; 21+ messages in thread
From: Patchwork @ 2017-09-14 11:12 UTC (permalink / raw)
  To: Michał Winiarski; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/5] drm/i915/guc: Remove obsolete comments and remove unused variable (rev2)
URL   : https://patchwork.freedesktop.org/series/30345/
State : failure

== Summary ==

Series 30345v2 series starting with [1/5] drm/i915/guc: Remove obsolete comments and remove unused variable
https://patchwork.freedesktop.org/api/1.0/series/30345/revisions/2/mbox/

Test debugfs_test:
        Subgroup read_all_entries:
                pass       -> SKIP       (fi-glk-2a)
Test drv_hangman:
        Subgroup error-state-basic:
                pass       -> SKIP       (fi-glk-2a)
Test gem_busy:
        Subgroup basic-busy-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-hang-default:
                pass       -> SKIP       (fi-glk-2a)
Test gem_close_race:
        Subgroup basic-process:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-threads:
                pass       -> SKIP       (fi-glk-2a)
Test gem_cpu_reloc:
        Subgroup basic:
                pass       -> SKIP       (fi-glk-2a)
Test gem_cs_tlb:
        Subgroup basic-default:
                pass       -> SKIP       (fi-glk-2a)
Test gem_ctx_create:
        Subgroup basic:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-files:
                pass       -> SKIP       (fi-glk-2a)
Test gem_ctx_exec:
        Subgroup basic:
                pass       -> SKIP       (fi-glk-2a)
Test gem_ctx_switch:
        Subgroup basic-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-default-heavy:
                pass       -> SKIP       (fi-glk-2a)
Test gem_exec_basic:
        Subgroup basic-blt:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-bsd:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-render:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-vebox:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup gtt-blt:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup gtt-bsd:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup gtt-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup gtt-render:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup gtt-vebox:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup readonly-blt:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup readonly-bsd:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup readonly-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup readonly-render:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup readonly-vebox:
                pass       -> SKIP       (fi-glk-2a)
Test gem_exec_create:
        Subgroup basic:
                pass       -> SKIP       (fi-glk-2a)
Test gem_exec_fence:
        Subgroup basic-busy-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-wait-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-await-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup await-hang-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup nb-await-default:
                pass       -> SKIP       (fi-glk-2a)
Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-batch-kernel-default-wb:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-uc-pro-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-uc-prw-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-uc-ro-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-uc-rw-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-uc-set-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-wb-pro-default:
                pass       -> SKIP       (fi-glk-2a)
WARNING: Long output truncated
fi-bdw-gvtdvm failed to connect after reboot
fi-byt-n2820 failed to connect after reboot

b21142588a9a16f74988f6b524953d4382be4a81 drm-tip: 2017y-09m-14d-08h-23m-57s UTC integration manifest
fed6c1df3c56 HAX Enable GuC Submission for CI
0f534b424a3a drm/i915/guc: Cleanup adding GuC work items
7046415e64d7 drm/i915/guc: Simplify GuC doorbell logic
d83e307a26a5 drm/i915/guc: Submit GuC workitems containing coalesced requests
e9e378a890de drm/i915/guc: Remove obsolete comments and remove unused variable

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5696/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 4/5] drm/i915/guc: Cleanup adding GuC work items
  2017-09-14  8:32 ` [PATCH 4/5] drm/i915/guc: Cleanup adding GuC work items Michał Winiarski
  2017-09-14 10:38   ` Chris Wilson
@ 2017-09-17 12:47   ` Chris Wilson
  2017-09-18  8:30   ` [PATCH v2 " Michał Winiarski
  2 siblings, 0 replies; 21+ messages in thread
From: Chris Wilson @ 2017-09-17 12:47 UTC (permalink / raw)
  To: Michał Winiarski, intel-gfx

Quoting Michał Winiarski (2017-09-14 09:32:15)
> @@ -434,11 +432,12 @@ static void guc_wq_item_append(struct i915_guc_client *client,
>          */
>         BUILD_BUG_ON(wqi_size != 16);
>  
> -       /* postincrement WQ tail for next time */
> -       wq_off = client->wq_tail;
> +       /* Postincrement WQ tail for next time, free space is guaranteed. */
> +       wq_off = READ_ONCE(desc->tail);
> +       GEM_BUG_ON(CIRC_SPACE(wq_off, READ_ONCE(desc->head),
> +                             GUC_WQ_SIZE) < wqi_size);
> +       WRITE_ONCE(desc->tail, (wq_off + wqi_size) & (GUC_WQ_SIZE - 1));
>         GEM_BUG_ON(wq_off & (wqi_size - 1));
> -       client->wq_tail += wqi_size;
> -       client->wq_tail &= client->wq_size - 1;
>  
>         /* WQ starts from the page after doorbell / process_desc */
>         wqi = client->vaddr + wq_off + GUC_DB_SIZE;
> @@ -451,7 +450,7 @@ static void guc_wq_item_append(struct i915_guc_client *client,
>  
>         wqi->context_desc = lower_32_bits(intel_lr_context_descriptor(ctx, engine));
>  
> -       wqi->submit_element_info = tail << WQ_RING_TAIL_SHIFT;
> +       wqi->submit_element_info = ring_tail << WQ_RING_TAIL_SHIFT;
>         wqi->fence_id = rq->global_seqno;

Ah, you reminded me that we should set the wq element before updating
the tail, i.e. move the
	WRITE_ONCE(desc->tail, (wq_off + wqi_size) & (GUC_WQ_SIZE - 1));
to here.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [PATCH v2 4/5] drm/i915/guc: Cleanup adding GuC work items
  2017-09-14  8:32 ` [PATCH 4/5] drm/i915/guc: Cleanup adding GuC work items Michał Winiarski
  2017-09-14 10:38   ` Chris Wilson
  2017-09-17 12:47   ` Chris Wilson
@ 2017-09-18  8:30   ` Michał Winiarski
  2017-09-18  9:25     ` [PATCH v3 " Michał Winiarski
  2 siblings, 1 reply; 21+ messages in thread
From: Michał Winiarski @ 2017-09-18  8:30 UTC (permalink / raw)
  To: intel-gfx

We can just operate on the wq_tail directly (in the process descriptor).
This allows us to remove the duplicated tail from the client. While I'm
here let's also remove the constants kept in the client and document our
locking requirements. This causes a small change in one of GuC debugfs
files. We're no longer reporting constant values (which I don't think
is a problem), but we're also no longer reporting the tail (does anyone
care?).

v2: Update tail after wqi contents. (Chris)

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_debugfs.c        |  2 --
 drivers/gpu/drm/i915/i915_guc_submission.c | 35 ++++++++++++------------------
 drivers/gpu/drm/i915/intel_uc.h            |  4 ----
 3 files changed, 14 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 46ac6091772e..2518bdf95eef 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2447,8 +2447,6 @@ static void i915_guc_client_info(struct seq_file *m,
 		client->priority, client->stage_id, client->proc_desc_offset);
 	seq_printf(m, "\tDoorbell id %d, offset: 0x%lx\n",
 		client->doorbell_id, client->doorbell_offset);
-	seq_printf(m, "\tWQ size %d, offset: 0x%x, tail %d\n",
-		client->wq_size, client->wq_offset, client->wq_tail);
 
 	for_each_engine(engine, dev_priv, id) {
 		u64 submissions = client->submissions[id];
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 16ce570a0b74..954bd183591b 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -305,7 +305,7 @@ static void guc_proc_desc_init(struct intel_guc *guc,
 	desc->db_base_addr = 0;
 
 	desc->stage_id = client->stage_id;
-	desc->wq_size_bytes = client->wq_size;
+	desc->wq_size_bytes = GUC_WQ_SIZE;
 	desc->wq_status = WQ_STATUS_ACTIVE;
 	desc->priority = client->priority;
 }
@@ -390,8 +390,8 @@ static void guc_stage_desc_init(struct intel_guc *guc,
 	desc->db_trigger_cpu = (uintptr_t)__get_doorbell(client);
 	desc->db_trigger_uk = gfx_addr + client->doorbell_offset;
 	desc->process_desc = gfx_addr + client->proc_desc_offset;
-	desc->wq_addr = gfx_addr + client->wq_offset;
-	desc->wq_size = client->wq_size;
+	desc->wq_addr = gfx_addr + GUC_DB_SIZE;
+	desc->wq_size = GUC_WQ_SIZE;
 
 	desc->desc_private = (uintptr_t)client;
 }
@@ -416,14 +416,12 @@ static void guc_wq_item_append(struct i915_guc_client *client,
 	struct i915_gem_context *ctx = rq->ctx;
 	struct guc_process_desc *desc = __get_process_desc(client);
 	struct guc_wq_item *wqi;
-	u32 freespace, tail, wq_off;
+	u32 ring_tail, wq_off;
 
-	/* Free space is guaranteed */
-	freespace = CIRC_SPACE(client->wq_tail, desc->head, client->wq_size);
-	GEM_BUG_ON(freespace < wqi_size);
+	lockdep_assert_held(&client->wq_lock);
 
-	tail = intel_ring_set_tail(rq->ring, rq->tail) / sizeof(u64);
-	GEM_BUG_ON(tail > WQ_RING_TAIL_MAX);
+	ring_tail = intel_ring_set_tail(rq->ring, rq->tail) / sizeof(u64);
+	GEM_BUG_ON(ring_tail > WQ_RING_TAIL_MAX);
 
 	/* For now workqueue item is 4 DWs; workqueue buffer is 2 pages. So we
 	 * should not have the case where structure wqi is across page, neither
@@ -434,11 +432,12 @@ static void guc_wq_item_append(struct i915_guc_client *client,
 	 */
 	BUILD_BUG_ON(wqi_size != 16);
 
-	/* postincrement WQ tail for next time */
-	wq_off = client->wq_tail;
+	/* Postincrement WQ tail for next time, free space is guaranteed. */
+	wq_off = READ_ONCE(desc->tail);
+	GEM_BUG_ON(CIRC_SPACE(wq_off, READ_ONCE(desc->head),
+			      GUC_WQ_SIZE) < wqi_size);
+	WRITE_ONCE(desc->tail, (wq_off + wqi_size) & (GUC_WQ_SIZE - 1));
 	GEM_BUG_ON(wq_off & (wqi_size - 1));
-	client->wq_tail += wqi_size;
-	client->wq_tail &= client->wq_size - 1;
 
 	/* WQ starts from the page after doorbell / process_desc */
 	wqi = client->vaddr + wq_off + GUC_DB_SIZE;
@@ -451,7 +450,7 @@ static void guc_wq_item_append(struct i915_guc_client *client,
 
 	wqi->context_desc = lower_32_bits(intel_lr_context_descriptor(ctx, engine));
 
-	wqi->submit_element_info = tail << WQ_RING_TAIL_SHIFT;
+	wqi->submit_element_info = ring_tail << WQ_RING_TAIL_SHIFT;
 	wqi->fence_id = rq->global_seqno;
 }
 
@@ -461,18 +460,14 @@ static void guc_reset_wq(struct i915_guc_client *client)
 
 	desc->head = 0;
 	desc->tail = 0;
-
-	client->wq_tail = 0;
 }
 
 static void guc_ring_doorbell(struct i915_guc_client *client)
 {
-	struct guc_process_desc *desc = __get_process_desc(client);
 	struct guc_doorbell_info *db;
 	u32 cookie;
 
-	/* Update the tail so it is visible to GuC */
-	desc->tail = client->wq_tail;
+	lockdep_assert_held(&client->wq_lock);
 
 	/* pointer of current doorbell cacheline */
 	db = __get_doorbell(client);
@@ -813,8 +808,6 @@ guc_client_alloc(struct drm_i915_private *dev_priv,
 	client->engines = engines;
 	client->priority = priority;
 	client->doorbell_id = GUC_DOORBELL_INVALID;
-	client->wq_offset = GUC_DB_SIZE;
-	client->wq_size = GUC_WQ_SIZE;
 	spin_lock_init(&client->wq_lock);
 
 	ret = ida_simple_get(&guc->stage_ids, 0, GUC_MAX_STAGE_DESCRIPTORS,
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index f505dcccd613..7703c9ad6511 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -68,10 +68,6 @@ struct i915_guc_client {
 	unsigned long doorbell_offset;
 
 	spinlock_t wq_lock;
-	uint32_t wq_offset;
-	uint32_t wq_size;
-	uint32_t wq_tail;
-
 	/* Per-engine counts of GuC submissions */
 	uint64_t submissions[I915_NUM_ENGINES];
 };
-- 
2.13.5

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

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

* ✗ Fi.CI.BAT: failure for series starting with [1/5] drm/i915/guc: Remove obsolete comments and remove unused variable (rev3)
  2017-09-14  8:32 [PATCH 1/5] drm/i915/guc: Remove obsolete comments and remove unused variable Michał Winiarski
                   ` (5 preceding siblings ...)
  2017-09-14 11:12 ` ✗ Fi.CI.BAT: failure for series starting with [1/5] drm/i915/guc: Remove obsolete comments and remove unused variable (rev2) Patchwork
@ 2017-09-18  8:56 ` Patchwork
  2017-09-18 10:03 ` ✗ Fi.CI.BAT: failure for series starting with [1/5] drm/i915/guc: Remove obsolete comments and remove unused variable (rev4) Patchwork
  7 siblings, 0 replies; 21+ messages in thread
From: Patchwork @ 2017-09-18  8:56 UTC (permalink / raw)
  To: Michał Winiarski; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/5] drm/i915/guc: Remove obsolete comments and remove unused variable (rev3)
URL   : https://patchwork.freedesktop.org/series/30345/
State : failure

== Summary ==

Series 30345v3 series starting with [1/5] drm/i915/guc: Remove obsolete comments and remove unused variable
https://patchwork.freedesktop.org/api/1.0/series/30345/revisions/3/mbox/

Test debugfs_test:
        Subgroup read_all_entries:
                pass       -> SKIP       (fi-glk-2a)
Test drv_hangman:
        Subgroup error-state-basic:
                pass       -> SKIP       (fi-glk-2a)
Test gem_busy:
        Subgroup basic-busy-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-hang-default:
                pass       -> SKIP       (fi-glk-2a)
Test gem_close_race:
        Subgroup basic-process:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-threads:
                pass       -> SKIP       (fi-glk-2a)
Test gem_cpu_reloc:
        Subgroup basic:
                pass       -> SKIP       (fi-glk-2a)
Test gem_cs_tlb:
        Subgroup basic-default:
                pass       -> SKIP       (fi-glk-2a)
Test gem_ctx_create:
        Subgroup basic:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-files:
                pass       -> SKIP       (fi-glk-2a)
Test gem_ctx_exec:
        Subgroup basic:
                pass       -> SKIP       (fi-glk-2a)
Test gem_ctx_switch:
        Subgroup basic-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-default-heavy:
                pass       -> SKIP       (fi-glk-2a)
Test gem_exec_basic:
        Subgroup basic-blt:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-bsd:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-render:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-vebox:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup gtt-blt:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup gtt-bsd:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup gtt-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup gtt-render:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup gtt-vebox:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup readonly-blt:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup readonly-bsd:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup readonly-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup readonly-render:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup readonly-vebox:
                pass       -> SKIP       (fi-glk-2a)
Test gem_exec_create:
        Subgroup basic:
                pass       -> SKIP       (fi-glk-2a)
Test gem_exec_fence:
        Subgroup basic-busy-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-wait-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-await-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup await-hang-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup nb-await-default:
                pass       -> SKIP       (fi-glk-2a)
Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-batch-kernel-default-wb:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-uc-pro-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-uc-prw-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-uc-ro-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-uc-rw-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-uc-set-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-wb-pro-default:
                pass       -> SKIP       (fi-glk-2a)
WARNING: Long output truncated

7fb202bddcf81e23028f9aadcf82b732699bd527 drm-tip: 2017y-09m-16d-10h-04m-01s UTC integration manifest
08d95e829f52 HAX Enable GuC Submission for CI
e2b3b402316a drm/i915/guc: Cleanup adding GuC work items
17825246e8e0 drm/i915/guc: Simplify GuC doorbell logic
ee856fbc1bdd drm/i915/guc: Submit GuC workitems containing coalesced requests
a65869bb678b drm/i915/guc: Remove obsolete comments and remove unused variable

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5722/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [PATCH v3 4/5] drm/i915/guc: Cleanup adding GuC work items
  2017-09-18  8:30   ` [PATCH v2 " Michał Winiarski
@ 2017-09-18  9:25     ` Michał Winiarski
  2017-09-18 16:10       ` Daniele Ceraolo Spurio
  0 siblings, 1 reply; 21+ messages in thread
From: Michał Winiarski @ 2017-09-18  9:25 UTC (permalink / raw)
  To: intel-gfx

We can just operate on the wq_tail directly (in the process descriptor).
This allows us to remove the duplicated tail from the client. While I'm
here let's also remove the constants kept in the client and document our
locking requirements. This causes a small change in one of GuC debugfs
files. We're no longer reporting constant values (which I don't think
is a problem), but we're also no longer reporting the tail (does anyone
care?).

v2: Update tail after wqi contents. (Chris)
v3: Really update tail after wqi contents.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_debugfs.c        |  2 --
 drivers/gpu/drm/i915/i915_guc_submission.c | 37 +++++++++++++-----------------
 drivers/gpu/drm/i915/intel_uc.h            |  4 ----
 3 files changed, 16 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 46ac6091772e..2518bdf95eef 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2447,8 +2447,6 @@ static void i915_guc_client_info(struct seq_file *m,
 		client->priority, client->stage_id, client->proc_desc_offset);
 	seq_printf(m, "\tDoorbell id %d, offset: 0x%lx\n",
 		client->doorbell_id, client->doorbell_offset);
-	seq_printf(m, "\tWQ size %d, offset: 0x%x, tail %d\n",
-		client->wq_size, client->wq_offset, client->wq_tail);
 
 	for_each_engine(engine, dev_priv, id) {
 		u64 submissions = client->submissions[id];
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 16ce570a0b74..18fefc11b8d7 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -305,7 +305,7 @@ static void guc_proc_desc_init(struct intel_guc *guc,
 	desc->db_base_addr = 0;
 
 	desc->stage_id = client->stage_id;
-	desc->wq_size_bytes = client->wq_size;
+	desc->wq_size_bytes = GUC_WQ_SIZE;
 	desc->wq_status = WQ_STATUS_ACTIVE;
 	desc->priority = client->priority;
 }
@@ -390,8 +390,8 @@ static void guc_stage_desc_init(struct intel_guc *guc,
 	desc->db_trigger_cpu = (uintptr_t)__get_doorbell(client);
 	desc->db_trigger_uk = gfx_addr + client->doorbell_offset;
 	desc->process_desc = gfx_addr + client->proc_desc_offset;
-	desc->wq_addr = gfx_addr + client->wq_offset;
-	desc->wq_size = client->wq_size;
+	desc->wq_addr = gfx_addr + GUC_DB_SIZE;
+	desc->wq_size = GUC_WQ_SIZE;
 
 	desc->desc_private = (uintptr_t)client;
 }
@@ -416,14 +416,12 @@ static void guc_wq_item_append(struct i915_guc_client *client,
 	struct i915_gem_context *ctx = rq->ctx;
 	struct guc_process_desc *desc = __get_process_desc(client);
 	struct guc_wq_item *wqi;
-	u32 freespace, tail, wq_off;
+	u32 ring_tail, wq_off;
 
-	/* Free space is guaranteed */
-	freespace = CIRC_SPACE(client->wq_tail, desc->head, client->wq_size);
-	GEM_BUG_ON(freespace < wqi_size);
+	lockdep_assert_held(&client->wq_lock);
 
-	tail = intel_ring_set_tail(rq->ring, rq->tail) / sizeof(u64);
-	GEM_BUG_ON(tail > WQ_RING_TAIL_MAX);
+	ring_tail = intel_ring_set_tail(rq->ring, rq->tail) / sizeof(u64);
+	GEM_BUG_ON(ring_tail > WQ_RING_TAIL_MAX);
 
 	/* For now workqueue item is 4 DWs; workqueue buffer is 2 pages. So we
 	 * should not have the case where structure wqi is across page, neither
@@ -434,11 +432,11 @@ static void guc_wq_item_append(struct i915_guc_client *client,
 	 */
 	BUILD_BUG_ON(wqi_size != 16);
 
-	/* postincrement WQ tail for next time */
-	wq_off = client->wq_tail;
+	/* Free space is guaranteed. */
+	wq_off = READ_ONCE(desc->tail);
+	GEM_BUG_ON(CIRC_SPACE(wq_off, READ_ONCE(desc->head),
+			      GUC_WQ_SIZE) < wqi_size);
 	GEM_BUG_ON(wq_off & (wqi_size - 1));
-	client->wq_tail += wqi_size;
-	client->wq_tail &= client->wq_size - 1;
 
 	/* WQ starts from the page after doorbell / process_desc */
 	wqi = client->vaddr + wq_off + GUC_DB_SIZE;
@@ -451,8 +449,11 @@ static void guc_wq_item_append(struct i915_guc_client *client,
 
 	wqi->context_desc = lower_32_bits(intel_lr_context_descriptor(ctx, engine));
 
-	wqi->submit_element_info = tail << WQ_RING_TAIL_SHIFT;
+	wqi->submit_element_info = ring_tail << WQ_RING_TAIL_SHIFT;
 	wqi->fence_id = rq->global_seqno;
+
+	/* Postincrement WQ tail for next time. */
+	WRITE_ONCE(desc->tail, (wq_off + wqi_size) & (GUC_WQ_SIZE - 1));
 }
 
 static void guc_reset_wq(struct i915_guc_client *client)
@@ -461,18 +462,14 @@ static void guc_reset_wq(struct i915_guc_client *client)
 
 	desc->head = 0;
 	desc->tail = 0;
-
-	client->wq_tail = 0;
 }
 
 static void guc_ring_doorbell(struct i915_guc_client *client)
 {
-	struct guc_process_desc *desc = __get_process_desc(client);
 	struct guc_doorbell_info *db;
 	u32 cookie;
 
-	/* Update the tail so it is visible to GuC */
-	desc->tail = client->wq_tail;
+	lockdep_assert_held(&client->wq_lock);
 
 	/* pointer of current doorbell cacheline */
 	db = __get_doorbell(client);
@@ -813,8 +810,6 @@ guc_client_alloc(struct drm_i915_private *dev_priv,
 	client->engines = engines;
 	client->priority = priority;
 	client->doorbell_id = GUC_DOORBELL_INVALID;
-	client->wq_offset = GUC_DB_SIZE;
-	client->wq_size = GUC_WQ_SIZE;
 	spin_lock_init(&client->wq_lock);
 
 	ret = ida_simple_get(&guc->stage_ids, 0, GUC_MAX_STAGE_DESCRIPTORS,
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index f505dcccd613..7703c9ad6511 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -68,10 +68,6 @@ struct i915_guc_client {
 	unsigned long doorbell_offset;
 
 	spinlock_t wq_lock;
-	uint32_t wq_offset;
-	uint32_t wq_size;
-	uint32_t wq_tail;
-
 	/* Per-engine counts of GuC submissions */
 	uint64_t submissions[I915_NUM_ENGINES];
 };
-- 
2.13.5

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

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

* ✗ Fi.CI.BAT: failure for series starting with [1/5] drm/i915/guc: Remove obsolete comments and remove unused variable (rev4)
  2017-09-14  8:32 [PATCH 1/5] drm/i915/guc: Remove obsolete comments and remove unused variable Michał Winiarski
                   ` (6 preceding siblings ...)
  2017-09-18  8:56 ` ✗ Fi.CI.BAT: failure for series starting with [1/5] drm/i915/guc: Remove obsolete comments and remove unused variable (rev3) Patchwork
@ 2017-09-18 10:03 ` Patchwork
  7 siblings, 0 replies; 21+ messages in thread
From: Patchwork @ 2017-09-18 10:03 UTC (permalink / raw)
  To: Michał Winiarski; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/5] drm/i915/guc: Remove obsolete comments and remove unused variable (rev4)
URL   : https://patchwork.freedesktop.org/series/30345/
State : failure

== Summary ==

Series 30345v4 series starting with [1/5] drm/i915/guc: Remove obsolete comments and remove unused variable
https://patchwork.freedesktop.org/api/1.0/series/30345/revisions/4/mbox/

Test debugfs_test:
        Subgroup read_all_entries:
                pass       -> SKIP       (fi-glk-2a)
Test drv_hangman:
        Subgroup error-state-basic:
                pass       -> SKIP       (fi-glk-2a)
Test gem_busy:
        Subgroup basic-busy-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-hang-default:
                pass       -> SKIP       (fi-glk-2a)
Test gem_close_race:
        Subgroup basic-process:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-threads:
                pass       -> SKIP       (fi-glk-2a)
Test gem_cpu_reloc:
        Subgroup basic:
                pass       -> SKIP       (fi-glk-2a)
Test gem_cs_tlb:
        Subgroup basic-default:
                pass       -> SKIP       (fi-glk-2a)
Test gem_ctx_create:
        Subgroup basic:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-files:
                pass       -> SKIP       (fi-glk-2a)
Test gem_ctx_exec:
        Subgroup basic:
                pass       -> SKIP       (fi-glk-2a)
Test gem_ctx_switch:
        Subgroup basic-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-default-heavy:
                pass       -> SKIP       (fi-glk-2a)
Test gem_exec_basic:
        Subgroup basic-blt:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-bsd:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-render:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-vebox:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup gtt-blt:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup gtt-bsd:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup gtt-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup gtt-render:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup gtt-vebox:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup readonly-blt:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup readonly-bsd:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup readonly-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup readonly-render:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup readonly-vebox:
                pass       -> SKIP       (fi-glk-2a)
Test gem_exec_create:
        Subgroup basic:
                pass       -> SKIP       (fi-glk-2a)
Test gem_exec_fence:
        Subgroup basic-busy-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-wait-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-await-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup await-hang-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup nb-await-default:
                pass       -> SKIP       (fi-glk-2a)
Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-batch-kernel-default-wb:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-uc-pro-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-uc-prw-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-uc-ro-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-uc-rw-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-uc-set-default:
                pass       -> SKIP       (fi-glk-2a)
        Subgroup basic-wb-pro-default:
                pass       -> SKIP       (fi-glk-2a)
WARNING: Long output truncated

5299e24e48f3c24e612bcdb997d0dc477cdde0d0 drm-tip: 2017y-09m-18d-08h-44m-15s UTC integration manifest
64c3ff54acc4 HAX Enable GuC Submission for CI
014245b3bc97 drm/i915/guc: Cleanup adding GuC work items
081c491cce12 drm/i915/guc: Simplify GuC doorbell logic
dcc489641106 drm/i915/guc: Submit GuC workitems containing coalesced requests
acde6ebc373d drm/i915/guc: Remove obsolete comments and remove unused variable

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5723/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v3 4/5] drm/i915/guc: Cleanup adding GuC work items
  2017-09-18  9:25     ` [PATCH v3 " Michał Winiarski
@ 2017-09-18 16:10       ` Daniele Ceraolo Spurio
  0 siblings, 0 replies; 21+ messages in thread
From: Daniele Ceraolo Spurio @ 2017-09-18 16:10 UTC (permalink / raw)
  To: Michał Winiarski, intel-gfx



On 18/09/17 02:25, Michał Winiarski wrote:
> We can just operate on the wq_tail directly (in the process descriptor).
> This allows us to remove the duplicated tail from the client. While I'm
> here let's also remove the constants kept in the client and document our
> locking requirements. This causes a small change in one of GuC debugfs
> files. We're no longer reporting constant values (which I don't think
> is a problem), but we're also no longer reporting the tail (does anyone
> care?).
> 
> v2: Update tail after wqi contents. (Chris)
> v3: Really update tail after wqi contents.
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Oscar Mateo <oscar.mateo@intel.com>
> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/i915_debugfs.c        |  2 --
>   drivers/gpu/drm/i915/i915_guc_submission.c | 37 +++++++++++++-----------------
>   drivers/gpu/drm/i915/intel_uc.h            |  4 ----
>   3 files changed, 16 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 46ac6091772e..2518bdf95eef 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2447,8 +2447,6 @@ static void i915_guc_client_info(struct seq_file *m,
>   		client->priority, client->stage_id, client->proc_desc_offset);
>   	seq_printf(m, "\tDoorbell id %d, offset: 0x%lx\n",
>   		client->doorbell_id, client->doorbell_offset);
> -	seq_printf(m, "\tWQ size %d, offset: 0x%x, tail %d\n",
> -		client->wq_size, client->wq_offset, client->wq_tail);
>   
>   	for_each_engine(engine, dev_priv, id) {
>   		u64 submissions = client->submissions[id];
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> index 16ce570a0b74..18fefc11b8d7 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -305,7 +305,7 @@ static void guc_proc_desc_init(struct intel_guc *guc,
>   	desc->db_base_addr = 0;
>   
>   	desc->stage_id = client->stage_id;
> -	desc->wq_size_bytes = client->wq_size;
> +	desc->wq_size_bytes = GUC_WQ_SIZE;
>   	desc->wq_status = WQ_STATUS_ACTIVE;
>   	desc->priority = client->priority;
>   }
> @@ -390,8 +390,8 @@ static void guc_stage_desc_init(struct intel_guc *guc,
>   	desc->db_trigger_cpu = (uintptr_t)__get_doorbell(client);
>   	desc->db_trigger_uk = gfx_addr + client->doorbell_offset;
>   	desc->process_desc = gfx_addr + client->proc_desc_offset;
> -	desc->wq_addr = gfx_addr + client->wq_offset;
> -	desc->wq_size = client->wq_size;
> +	desc->wq_addr = gfx_addr + GUC_DB_SIZE;
> +	desc->wq_size = GUC_WQ_SIZE;
>   
>   	desc->desc_private = (uintptr_t)client;
>   }
> @@ -416,14 +416,12 @@ static void guc_wq_item_append(struct i915_guc_client *client,
>   	struct i915_gem_context *ctx = rq->ctx;
>   	struct guc_process_desc *desc = __get_process_desc(client);
>   	struct guc_wq_item *wqi;
> -	u32 freespace, tail, wq_off;
> +	u32 ring_tail, wq_off;
>   
> -	/* Free space is guaranteed */
> -	freespace = CIRC_SPACE(client->wq_tail, desc->head, client->wq_size);
> -	GEM_BUG_ON(freespace < wqi_size);
> +	lockdep_assert_held(&client->wq_lock);
>   
> -	tail = intel_ring_set_tail(rq->ring, rq->tail) / sizeof(u64);
> -	GEM_BUG_ON(tail > WQ_RING_TAIL_MAX);
> +	ring_tail = intel_ring_set_tail(rq->ring, rq->tail) / sizeof(u64);
> +	GEM_BUG_ON(ring_tail > WQ_RING_TAIL_MAX);
>   
>   	/* For now workqueue item is 4 DWs; workqueue buffer is 2 pages. So we
>   	 * should not have the case where structure wqi is across page, neither
> @@ -434,11 +432,11 @@ static void guc_wq_item_append(struct i915_guc_client *client,
>   	 */
>   	BUILD_BUG_ON(wqi_size != 16);
>   
> -	/* postincrement WQ tail for next time */
> -	wq_off = client->wq_tail;
> +	/* Free space is guaranteed. */
> +	wq_off = READ_ONCE(desc->tail);
> +	GEM_BUG_ON(CIRC_SPACE(wq_off, READ_ONCE(desc->head),
> +			      GUC_WQ_SIZE) < wqi_size);
>   	GEM_BUG_ON(wq_off & (wqi_size - 1));
> -	client->wq_tail += wqi_size;
> -	client->wq_tail &= client->wq_size - 1;
>   
>   	/* WQ starts from the page after doorbell / process_desc */
>   	wqi = client->vaddr + wq_off + GUC_DB_SIZE;
> @@ -451,8 +449,11 @@ static void guc_wq_item_append(struct i915_guc_client *client,
>   
>   	wqi->context_desc = lower_32_bits(intel_lr_context_descriptor(ctx, engine));
>   
> -	wqi->submit_element_info = tail << WQ_RING_TAIL_SHIFT;
> +	wqi->submit_element_info = ring_tail << WQ_RING_TAIL_SHIFT;
>   	wqi->fence_id = rq->global_seqno;
> +
> +	/* Postincrement WQ tail for next time. */

Apologies for the late comment, just a nitpick: by updating desc->tail 
here we're making the update visible to GuC, we're not doing something 
for next SW iteration. I believe the comment was used to refer to the SW 
tracker (client->wq_tail) that is now gone so we could reword/drop it.

Daniele

> +	WRITE_ONCE(desc->tail, (wq_off + wqi_size) & (GUC_WQ_SIZE - 1));
>   }
>   
>   static void guc_reset_wq(struct i915_guc_client *client)
> @@ -461,18 +462,14 @@ static void guc_reset_wq(struct i915_guc_client *client)
>   
>   	desc->head = 0;
>   	desc->tail = 0;
> -
> -	client->wq_tail = 0;
>   }
>   
>   static void guc_ring_doorbell(struct i915_guc_client *client)
>   {
> -	struct guc_process_desc *desc = __get_process_desc(client);
>   	struct guc_doorbell_info *db;
>   	u32 cookie;
>   
> -	/* Update the tail so it is visible to GuC */
> -	desc->tail = client->wq_tail;
> +	lockdep_assert_held(&client->wq_lock);
>   
>   	/* pointer of current doorbell cacheline */
>   	db = __get_doorbell(client);
> @@ -813,8 +810,6 @@ guc_client_alloc(struct drm_i915_private *dev_priv,
>   	client->engines = engines;
>   	client->priority = priority;
>   	client->doorbell_id = GUC_DOORBELL_INVALID;
> -	client->wq_offset = GUC_DB_SIZE;
> -	client->wq_size = GUC_WQ_SIZE;
>   	spin_lock_init(&client->wq_lock);
>   
>   	ret = ida_simple_get(&guc->stage_ids, 0, GUC_MAX_STAGE_DESCRIPTORS,
> diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
> index f505dcccd613..7703c9ad6511 100644
> --- a/drivers/gpu/drm/i915/intel_uc.h
> +++ b/drivers/gpu/drm/i915/intel_uc.h
> @@ -68,10 +68,6 @@ struct i915_guc_client {
>   	unsigned long doorbell_offset;
>   
>   	spinlock_t wq_lock;
> -	uint32_t wq_offset;
> -	uint32_t wq_size;
> -	uint32_t wq_tail;
> -
>   	/* Per-engine counts of GuC submissions */
>   	uint64_t submissions[I915_NUM_ENGINES];
>   };
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [PATCH 5/5] HAX enable GuC submission for CI
  2017-11-05 13:39 [PATCH 1/5] drm/i915/guc: Separate GuC doorbell destroy function into doorbell unit and GuC task Sagar Arun Kamble
@ 2017-11-05 13:39 ` Sagar Arun Kamble
  0 siblings, 0 replies; 21+ messages in thread
From: Sagar Arun Kamble @ 2017-11-05 13:39 UTC (permalink / raw)
  To: intel-gfx

Also revert ("drm/i915/guc: Assert that we switch between
known ggtt->invalidate functions")
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 8 ++------
 drivers/gpu/drm/i915/i915_params.h  | 4 ++--
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 0684d5d..a351ddf 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3562,17 +3562,13 @@ int i915_ggtt_enable_hw(struct drm_i915_private *dev_priv)
 
 void i915_ggtt_enable_guc(struct drm_i915_private *i915)
 {
-	GEM_BUG_ON(i915->ggtt.invalidate != gen6_ggtt_invalidate);
-
 	i915->ggtt.invalidate = guc_ggtt_invalidate;
 }
 
 void i915_ggtt_disable_guc(struct drm_i915_private *i915)
 {
-	/* We should only be called after i915_ggtt_enable_guc() */
-	GEM_BUG_ON(i915->ggtt.invalidate != guc_ggtt_invalidate);
-
-	i915->ggtt.invalidate = gen6_ggtt_invalidate;
+	if (i915->ggtt.invalidate == guc_ggtt_invalidate)
+		i915->ggtt.invalidate = gen6_ggtt_invalidate;
 }
 
 void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index c729226..c38cef0 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -44,8 +44,8 @@
 	param(int, disable_power_well, -1) \
 	param(int, enable_ips, 1) \
 	param(int, invert_brightness, 0) \
-	param(int, enable_guc_loading, 0) \
-	param(int, enable_guc_submission, 0) \
+	param(int, enable_guc_loading, 1) \
+	param(int, enable_guc_submission, 1) \
 	param(int, guc_log_level, -1) \
 	param(char *, guc_firmware_path, NULL) \
 	param(char *, huc_firmware_path, NULL) \
-- 
1.9.1

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

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

* [PATCH 5/5] HAX Enable GuC Submission for CI
  2017-09-12 17:40 [PATCH 1/5] drm/i915/guc: Remove obsolete comments and remove unused variable Michał Winiarski
@ 2017-09-12 17:40 ` Michał Winiarski
  0 siblings, 0 replies; 21+ messages in thread
From: Michał Winiarski @ 2017-09-12 17:40 UTC (permalink / raw)
  To: intel-gfx

Also:
Revert "drm/i915/guc: Assert that we switch between known ggtt->invalidate functions"

This reverts commit 04f7b24eccdfae680a36e9825fe0d61dcd5ed528.
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 8 ++------
 drivers/gpu/drm/i915/i915_params.c  | 4 ++--
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 09e524dbc090..478a8d42aeb0 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3189,17 +3189,13 @@ int i915_ggtt_enable_hw(struct drm_i915_private *dev_priv)
 
 void i915_ggtt_enable_guc(struct drm_i915_private *i915)
 {
-	GEM_BUG_ON(i915->ggtt.invalidate != gen6_ggtt_invalidate);
-
 	i915->ggtt.invalidate = guc_ggtt_invalidate;
 }
 
 void i915_ggtt_disable_guc(struct drm_i915_private *i915)
 {
-	/* We should only be called after i915_ggtt_enable_guc() */
-	GEM_BUG_ON(i915->ggtt.invalidate != guc_ggtt_invalidate);
-
-	i915->ggtt.invalidate = gen6_ggtt_invalidate;
+	if (i915->ggtt.invalidate == guc_ggtt_invalidate)
+		i915->ggtt.invalidate = gen6_ggtt_invalidate;
 }
 
 void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 8ab003dca113..c9d72f1b8383 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -56,8 +56,8 @@ struct i915_params i915 __read_mostly = {
 	.verbose_state_checks = 1,
 	.nuclear_pageflip = 0,
 	.edp_vswing = 0,
-	.enable_guc_loading = 0,
-	.enable_guc_submission = 0,
+	.enable_guc_loading = 2,
+	.enable_guc_submission = 2,
 	.guc_log_level = -1,
 	.guc_firmware_path = NULL,
 	.huc_firmware_path = NULL,
-- 
2.13.5

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

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

* [PATCH 5/5] HAX Enable GuC Submission for CI
  2017-09-12 15:37 [PATCH 1/5] drm/i915/guc: Remove obsolete comments and remove unused variable Chris Wilson
@ 2017-09-12 15:37 ` Chris Wilson
  0 siblings, 0 replies; 21+ messages in thread
From: Chris Wilson @ 2017-09-12 15:37 UTC (permalink / raw)
  To: intel-gfx

From: Michał Winiarski <michal.winiarski@intel.com>

Also:
Revert "drm/i915/guc: Assert that we switch between known ggtt->invalidate functions"

This reverts commit 04f7b24eccdfae680a36e9825fe0d61dcd5ed528.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Link: https://patchwork.freedesktop.org/patch/msgid/20170912132226.25629-1-michal.winiarski@intel.com
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 8 ++------
 drivers/gpu/drm/i915/i915_params.c  | 4 ++--
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 09e524dbc090..478a8d42aeb0 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3189,17 +3189,13 @@ int i915_ggtt_enable_hw(struct drm_i915_private *dev_priv)
 
 void i915_ggtt_enable_guc(struct drm_i915_private *i915)
 {
-	GEM_BUG_ON(i915->ggtt.invalidate != gen6_ggtt_invalidate);
-
 	i915->ggtt.invalidate = guc_ggtt_invalidate;
 }
 
 void i915_ggtt_disable_guc(struct drm_i915_private *i915)
 {
-	/* We should only be called after i915_ggtt_enable_guc() */
-	GEM_BUG_ON(i915->ggtt.invalidate != guc_ggtt_invalidate);
-
-	i915->ggtt.invalidate = gen6_ggtt_invalidate;
+	if (i915->ggtt.invalidate == guc_ggtt_invalidate)
+		i915->ggtt.invalidate = gen6_ggtt_invalidate;
 }
 
 void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 8ab003dca113..c9d72f1b8383 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -56,8 +56,8 @@ struct i915_params i915 __read_mostly = {
 	.verbose_state_checks = 1,
 	.nuclear_pageflip = 0,
 	.edp_vswing = 0,
-	.enable_guc_loading = 0,
-	.enable_guc_submission = 0,
+	.enable_guc_loading = 2,
+	.enable_guc_submission = 2,
 	.guc_log_level = -1,
 	.guc_firmware_path = NULL,
 	.huc_firmware_path = NULL,
-- 
2.14.1

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

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

end of thread, other threads:[~2017-11-05 13:36 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-14  8:32 [PATCH 1/5] drm/i915/guc: Remove obsolete comments and remove unused variable Michał Winiarski
2017-09-14  8:32 ` [PATCH v5 2/5] drm/i915/guc: Submit GuC workitems containing coalesced requests Michał Winiarski
2017-09-14  8:32 ` [PATCH v3 3/5] drm/i915/guc: Simplify GuC doorbell logic Michał Winiarski
2017-09-14 10:37   ` Chris Wilson
2017-09-14 10:51   ` [PATCH v4 " Michał Winiarski
2017-09-14  8:32 ` [PATCH 4/5] drm/i915/guc: Cleanup adding GuC work items Michał Winiarski
2017-09-14 10:38   ` Chris Wilson
2017-09-17 12:47   ` Chris Wilson
2017-09-18  8:30   ` [PATCH v2 " Michał Winiarski
2017-09-18  9:25     ` [PATCH v3 " Michał Winiarski
2017-09-18 16:10       ` Daniele Ceraolo Spurio
2017-09-14  8:32 ` [PATCH 5/5] HAX Enable GuC Submission for CI Michał Winiarski
2017-09-14  9:04 ` ✗ Fi.CI.BAT: failure for series starting with [1/5] drm/i915/guc: Remove obsolete comments and remove unused variable Patchwork
2017-09-14 10:28   ` Michał Winiarski
2017-09-14 10:40     ` Chris Wilson
2017-09-14 11:12 ` ✗ Fi.CI.BAT: failure for series starting with [1/5] drm/i915/guc: Remove obsolete comments and remove unused variable (rev2) Patchwork
2017-09-18  8:56 ` ✗ Fi.CI.BAT: failure for series starting with [1/5] drm/i915/guc: Remove obsolete comments and remove unused variable (rev3) Patchwork
2017-09-18 10:03 ` ✗ Fi.CI.BAT: failure for series starting with [1/5] drm/i915/guc: Remove obsolete comments and remove unused variable (rev4) Patchwork
  -- strict thread matches above, loose matches on Subject: below --
2017-11-05 13:39 [PATCH 1/5] drm/i915/guc: Separate GuC doorbell destroy function into doorbell unit and GuC task Sagar Arun Kamble
2017-11-05 13:39 ` [PATCH 5/5] HAX enable GuC submission for CI Sagar Arun Kamble
2017-09-12 17:40 [PATCH 1/5] drm/i915/guc: Remove obsolete comments and remove unused variable Michał Winiarski
2017-09-12 17:40 ` [PATCH 5/5] HAX Enable GuC Submission for CI Michał Winiarski
2017-09-12 15:37 [PATCH 1/5] drm/i915/guc: Remove obsolete comments and remove unused variable Chris Wilson
2017-09-12 15:37 ` [PATCH 5/5] HAX Enable GuC Submission for CI Chris Wilson

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.