[PATCH v3 0/7] drm/i915/guc: updates to GuC doorbell handling

[PATCH v3 0/7] drm/i915/guc: updates to GuC doorbell handling

[Dave Gordon]

Various GuC (doorbell) related patches, some trivial. The bulk of the
changes are in [patch 5/7], but it's mostly just reorganisation of
existing code; and [patch 6/7] is just a followup to that, kept as a
separate change for reasons of clarity.  The new functionality is
implemented in a single new function in [patch 7/7].

Background, from v1 of this patchset:

The Linux hibernate/resume sequence involves booting one kernel, and
then replacing(!) its in-memory image with that of the previously
hibernated system.  This can lead to inconsistencies in the state of
the hardware, in particular where a driver does not or cannot reset
it to a well-defined initial state during resume.

For i915, the issue is that the doorbell hardware is not reset when
the GuC is reset; also, the driver *cannot* directly reprogram it:
only the GuC can do that. So this set of patches first reorganises
the doorbell handling, and then (in the last patch of the set)
ensures that the doorbell hardware is fully (re-)initialised when
the GuC is (re-)loaded.

v2:
  Patches reorganised (split), with longer commit message

v3:
  Rebased, patch [6/7] inserted into sequence

Dave Gordon (7):
  drm/i915/guc: add doorbell map to debugfs/i915_guc_info
  drm/i915/guc: prefer __set/clear_bit() to bitmap_set/clear()
  drm/i915/guc: remove writes to GEN8_DRBREG registers
  drm/i915/guc: move guc_ring_doorbell() nearer to callsite
  drm/i915/guc: refactor doorbell management code
  drm/i915/guc: replace assign_doorbell() with
    select_doorbell_register()
  drm/i915/guc: (re)initialise doorbell h/w when enabling GuC submission

 drivers/gpu/drm/i915/i915_debugfs.c        |   4 +
 drivers/gpu/drm/i915/i915_guc_submission.c | 301 +++++++++++++++++------------
 2 files changed, 179 insertions(+), 126 deletions(-)

-- 
1.9.1

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

[PATCH v3 1/7] drm/i915/guc: add doorbell map to debugfs/i915_guc_info

[Dave Gordon]

To properly verify the driver->doorbell->GuC functionality, validation
needs to know how the driver has assigned the doorbell cache lines and
registers, so make them visible through debugfs.

v2: use kernel bitmap-printing format (%pb) rather than %x.

Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index e4f2c55..6efc8b1 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2574,6 +2574,10 @@ static int i915_guc_info(struct seq_file *m, void *data)
 
 	mutex_unlock(&dev->struct_mutex);
 
+	seq_printf(m, "Doorbell map:\n");
+	seq_printf(m, "\t%*pb\n", GUC_MAX_DOORBELLS, guc.doorbell_bitmap);
+	seq_printf(m, "Doorbell next cacheline: 0x%x\n\n", guc.db_cacheline);
+
 	seq_printf(m, "GuC total action count: %llu\n", guc.action_count);
 	seq_printf(m, "GuC action failure count: %u\n", guc.action_fail);
 	seq_printf(m, "GuC last action command: 0x%x\n", guc.action_cmd);
-- 
1.9.1

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

[PATCH v3 2/7] drm/i915/guc: prefer __set/clear_bit() to bitmap_set/clear()

[Dave Gordon]

Bitmap operators are overkill when touching only one bit.

Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_guc_submission.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 65e67f0..21daaa5 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -306,7 +306,7 @@ static uint16_t assign_doorbell(struct intel_guc *guc, uint32_t priority)
 	if (id == end)
 		id = GUC_INVALID_DOORBELL_ID;
 	else
-		bitmap_set(guc->doorbell_bitmap, id, 1);
+		__set_bit(id, guc->doorbell_bitmap);
 
 	DRM_DEBUG_DRIVER("assigned %s priority doorbell id 0x%x\n",
 			hi_pri ? "high" : "normal", id);
@@ -316,7 +316,7 @@ static uint16_t assign_doorbell(struct intel_guc *guc, uint32_t priority)
 
 static void release_doorbell(struct intel_guc *guc, uint16_t id)
 {
-	bitmap_clear(guc->doorbell_bitmap, id, 1);
+	__clear_bit(id, guc->doorbell_bitmap);
 }
 
 /*
-- 
1.9.1

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

[PATCH v3 3/7] drm/i915/guc: remove writes to GEN8_DRBREG registers

[Dave Gordon]

These registers are not actually writable by the CPU; only the GuC can
actually program them. So let's not do writes that have no effect.

Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_guc_submission.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 21daaa5..1589fe9 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -252,14 +252,9 @@ static void guc_disable_doorbell(struct intel_guc *guc,
 
 	doorbell->db_status = GUC_DOORBELL_DISABLED;
 
-	I915_WRITE(drbreg, I915_READ(drbreg) & ~GEN8_DRB_VALID);
-
 	value = I915_READ(drbreg);
 	WARN_ON((value & GEN8_DRB_VALID) != 0);
 
-	I915_WRITE(GEN8_DRBREGU(client->doorbell_id), 0);
-	I915_WRITE(drbreg, 0);
-
 	/* XXX: wait for any interrupts */
 	/* XXX: wait for workqueue to drain */
 }
-- 
1.9.1

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

[PATCH v3 4/7] drm/i915/guc: move guc_ring_doorbell() nearer to callsite

[Dave Gordon]

Just code movement, no actual change to the function. This is in
preparation for the next patch, which will reorganise all the other
doorbell code, but doesn't change this function. So let's shuffle it
down near its caller rather than leaving it mixed in with the setup
code. Unlike the doorbell management code, this function is somewhat
time-critical, so putting it near its caller may even yield a tiny
performance improvement.

Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_guc_submission.c | 110 ++++++++++++++---------------
 1 file changed, 55 insertions(+), 55 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 1589fe9..1c4ff3c 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -185,61 +185,6 @@ static void guc_init_doorbell(struct intel_guc *guc,
 	doorbell->cookie = 0;
 }
 
-static int guc_ring_doorbell(struct i915_guc_client *gc)
-{
-	struct guc_process_desc *desc;
-	union guc_doorbell_qw db_cmp, db_exc, db_ret;
-	union guc_doorbell_qw *db;
-	int attempt = 2, ret = -EAGAIN;
-
-	desc = gc->client_base + gc->proc_desc_offset;
-
-	/* Update the tail so it is visible to GuC */
-	desc->tail = gc->wq_tail;
-
-	/* current cookie */
-	db_cmp.db_status = GUC_DOORBELL_ENABLED;
-	db_cmp.cookie = gc->cookie;
-
-	/* cookie to be updated */
-	db_exc.db_status = GUC_DOORBELL_ENABLED;
-	db_exc.cookie = gc->cookie + 1;
-	if (db_exc.cookie == 0)
-		db_exc.cookie = 1;
-
-	/* pointer of current doorbell cacheline */
-	db = gc->client_base + gc->doorbell_offset;
-
-	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 */
-			gc->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;
-
-		DRM_ERROR("Cookie mismatch. Expected %d, returned %d\n",
-			  db_cmp.cookie, db_ret.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;
-}
-
 static void guc_disable_doorbell(struct intel_guc *guc,
 				 struct i915_guc_client *client)
 {
@@ -538,6 +483,61 @@ static void guc_add_workqueue_item(struct i915_guc_client *gc,
 	kunmap_atomic(base);
 }
 
+static int guc_ring_doorbell(struct i915_guc_client *gc)
+{
+	struct guc_process_desc *desc;
+	union guc_doorbell_qw db_cmp, db_exc, db_ret;
+	union guc_doorbell_qw *db;
+	int attempt = 2, ret = -EAGAIN;
+
+	desc = gc->client_base + gc->proc_desc_offset;
+
+	/* Update the tail so it is visible to GuC */
+	desc->tail = gc->wq_tail;
+
+	/* current cookie */
+	db_cmp.db_status = GUC_DOORBELL_ENABLED;
+	db_cmp.cookie = gc->cookie;
+
+	/* cookie to be updated */
+	db_exc.db_status = GUC_DOORBELL_ENABLED;
+	db_exc.cookie = gc->cookie + 1;
+	if (db_exc.cookie == 0)
+		db_exc.cookie = 1;
+
+	/* pointer of current doorbell cacheline */
+	db = gc->client_base + gc->doorbell_offset;
+
+	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 */
+			gc->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;
+
+		DRM_ERROR("Cookie mismatch. Expected %d, returned %d\n",
+			  db_cmp.cookie, db_ret.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;
+}
+
 /**
  * i915_guc_submit() - Submit commands through GuC
  * @rq:		request associated with the commands
-- 
1.9.1

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

[PATCH v3 5/7] drm/i915/guc: refactor doorbell management code

[Dave Gordon]

This patch refactors the driver's handling and tracking of doorbells, in
preparation for a later one which will resolve a suspend-resume issue.

There are three resources to be managed:
1. Cachelines: a single line within the client-object's page 0
   is snooped by doorbell hardware for writes from the host.
2. Doorbell registers: each defines one cacheline to be snooped.
3. Bitmap: tracks which doorbell registers are in use.

The doorbell setup/teardown protocol starts with:
1. Pick a cacheline: select_doorbell_cacheline()
2. Find an available doorbell register: assign_doorbell()
(These values are passed to the GuC via the shared context
descriptor; this part of the sequence remains unchanged).

3. Update the bitmap to reflect registers-in-use
4. Prepare the cacheline for use by setting its status to ENABLED
5. Ask the GuC to program the doorbell to snoop the cacheline

and of course teardown is very similar:
6. Set the cacheline to DISABLED
7. Ask the GuC to reprogram the doorbell to stop snooping
8. Record that the doorbell is not in use.

Operations 6-8 (guc_disable_doorbell(), host2guc_release_doorbell(), and
release_doorbell()) were called in sequence from guc_client_free(), but
are now moved into the teardown phase of the common function.

Steps 4-5 (guc_init_doorbell() and host2guc_allocate_doorbell()) were
similarly done as sequential steps in guc_client_alloc(), but since it
turns out that we don't need to be able to do them separately they're
now collected into the setup phase of the common function.

The only new code (and new capability) is the block tagged
    /* Update the GuC's idea of the doorbell ID */
i.e. we can now *change* the doorbell register used by an existing
client, whereas previously it was set once for the entire lifetime
of the client. We will use this new feature in the next patch.

v2: Trivial independent fixes pushed ahead as separate patches.
    MUCH longer commit message :) [Tvrtko Ursulin]

Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_guc_submission.c | 94 +++++++++++++++++-------------
 1 file changed, 53 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 1c4ff3c..62bf4bd 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -174,31 +174,59 @@ static int host2guc_sample_forcewake(struct intel_guc *guc,
  * client object which contains the page being used for the doorbell
  */
 
-static void guc_init_doorbell(struct intel_guc *guc,
-			      struct i915_guc_client *client)
+static int guc_update_doorbell_id(struct intel_guc *guc,
+				  struct i915_guc_client *client,
+				  u16 new_id)
 {
+	struct sg_table *sg = guc->ctx_pool_obj->pages;
+	void *doorbell_bitmap = guc->doorbell_bitmap;
 	struct guc_doorbell_info *doorbell;
+	struct guc_context_desc desc;
+	size_t len;
 
 	doorbell = client->client_base + client->doorbell_offset;
 
-	doorbell->db_status = GUC_DOORBELL_ENABLED;
+	if (client->doorbell_id != GUC_INVALID_DOORBELL_ID &&
+	    test_bit(client->doorbell_id, doorbell_bitmap)) {
+		/* Deactivate the old doorbell */
+		doorbell->db_status = GUC_DOORBELL_DISABLED;
+		(void)host2guc_release_doorbell(guc, client);
+		__clear_bit(client->doorbell_id, doorbell_bitmap);
+	}
+
+	/* Update the GuC's idea of the doorbell ID */
+	len = sg_pcopy_to_buffer(sg->sgl, sg->nents, &desc, sizeof(desc),
+			     sizeof(desc) * client->ctx_index);
+	if (len != sizeof(desc))
+		return -EFAULT;
+	desc.db_id = new_id;
+	len = sg_pcopy_from_buffer(sg->sgl, sg->nents, &desc, sizeof(desc),
+			     sizeof(desc) * client->ctx_index);
+	if (len != sizeof(desc))
+		return -EFAULT;
+
+	client->doorbell_id = new_id;
+	if (new_id == GUC_INVALID_DOORBELL_ID)
+		return 0;
+
+	/* Activate the new doorbell */
+	__set_bit(new_id, doorbell_bitmap);
 	doorbell->cookie = 0;
+	doorbell->db_status = GUC_DOORBELL_ENABLED;
+	return host2guc_allocate_doorbell(guc, client);
+}
+
+static int guc_init_doorbell(struct intel_guc *guc,
+			      struct i915_guc_client *client,
+			      uint16_t db_id)
+{
+	return guc_update_doorbell_id(guc, client, db_id);
 }
 
 static void guc_disable_doorbell(struct intel_guc *guc,
 				 struct i915_guc_client *client)
 {
-	struct drm_i915_private *dev_priv = guc_to_i915(guc);
-	struct guc_doorbell_info *doorbell;
-	i915_reg_t drbreg = GEN8_DRBREGL(client->doorbell_id);
-	int value;
-
-	doorbell = client->client_base + client->doorbell_offset;
-
-	doorbell->db_status = GUC_DOORBELL_DISABLED;
-
-	value = I915_READ(drbreg);
-	WARN_ON((value & GEN8_DRB_VALID) != 0);
+	(void)guc_update_doorbell_id(guc, client, GUC_INVALID_DOORBELL_ID);
 
 	/* XXX: wait for any interrupts */
 	/* XXX: wait for workqueue to drain */
@@ -254,11 +282,6 @@ static uint16_t assign_doorbell(struct intel_guc *guc, uint32_t priority)
 	return id;
 }
 
-static void release_doorbell(struct intel_guc *guc, uint16_t id)
-{
-	__clear_bit(id, guc->doorbell_bitmap);
-}
-
 /*
  * Initialise the process descriptor shared with the GuC firmware.
  */
@@ -651,21 +674,11 @@ static void gem_release_guc_obj(struct drm_i915_gem_object *obj)
 	 */
 
 	if (client->client_base) {
-		uint16_t db_id = client->doorbell_id;
-
 		/*
-		 * If we got as far as setting up a doorbell, make sure
-		 * we shut it down before unmapping & deallocating the
-		 * memory. So first disable the doorbell, then tell the
-		 * GuC that we've finished with it, finally deallocate
-		 * it in our bitmap
+		 * If we got as far as setting up a doorbell, make sure we
+		 * shut it down before unmapping & deallocating the memory.
 		 */
-		if (db_id != GUC_INVALID_DOORBELL_ID) {
-			guc_disable_doorbell(guc, client);
-			if (test_bit(db_id, guc->doorbell_bitmap))
-				host2guc_release_doorbell(guc, client);
-			release_doorbell(guc, db_id);
-		}
+		guc_disable_doorbell(guc, client);
 
 		kunmap(kmap_to_page(client->client_base));
 	}
@@ -700,6 +713,7 @@ static void gem_release_guc_obj(struct drm_i915_gem_object *obj)
 	struct i915_guc_client *client;
 	struct intel_guc *guc = &dev_priv->guc;
 	struct drm_i915_gem_object *obj;
+	uint16_t db_id;
 
 	client = kzalloc(sizeof(*client), GFP_KERNEL);
 	if (!client)
@@ -740,22 +754,20 @@ static void gem_release_guc_obj(struct drm_i915_gem_object *obj)
 	else
 		client->proc_desc_offset = (GUC_DB_SIZE / 2);
 
-	client->doorbell_id = assign_doorbell(guc, client->priority);
-	if (client->doorbell_id == GUC_INVALID_DOORBELL_ID)
+	db_id = assign_doorbell(guc, client->priority);
+	if (db_id == GUC_INVALID_DOORBELL_ID)
 		/* XXX: evict a doorbell instead */
 		goto err;
 
 	guc_init_proc_desc(guc, client);
 	guc_init_ctx_desc(guc, client);
-	guc_init_doorbell(guc, client);
-
-	/* XXX: Any cache flushes needed? General domain mgmt calls? */
-
-	if (host2guc_allocate_doorbell(guc, client))
+	if (guc_init_doorbell(guc, client, db_id))
 		goto err;
 
-	DRM_DEBUG_DRIVER("new priority %u client %p: ctx_index %u db_id %u\n",
-		priority, client, client->ctx_index, client->doorbell_id);
+	DRM_DEBUG_DRIVER("new priority %u client %p: ctx_index %u\n",
+		priority, client, client->ctx_index);
+	DRM_DEBUG_DRIVER("doorbell id %u, cacheline offset 0x%x\n",
+		client->doorbell_id, client->doorbell_offset);
 
 	return client;
 
-- 
1.9.1

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

Re: [PATCH v3 5/7] drm/i915/guc: refactor doorbell management code

[Tvrtko Ursulin]

On 13/06/16 17:57, Dave Gordon wrote:
> This patch refactors the driver's handling and tracking of doorbells, in
> preparation for a later one which will resolve a suspend-resume issue.
>
> There are three resources to be managed:
> 1. Cachelines: a single line within the client-object's page 0
>     is snooped by doorbell hardware for writes from the host.
> 2. Doorbell registers: each defines one cacheline to be snooped.
> 3. Bitmap: tracks which doorbell registers are in use.
>
> The doorbell setup/teardown protocol starts with:
> 1. Pick a cacheline: select_doorbell_cacheline()
> 2. Find an available doorbell register: assign_doorbell()
> (These values are passed to the GuC via the shared context
> descriptor; this part of the sequence remains unchanged).
>
> 3. Update the bitmap to reflect registers-in-use
> 4. Prepare the cacheline for use by setting its status to ENABLED
> 5. Ask the GuC to program the doorbell to snoop the cacheline
>
> and of course teardown is very similar:
> 6. Set the cacheline to DISABLED
> 7. Ask the GuC to reprogram the doorbell to stop snooping
> 8. Record that the doorbell is not in use.
>
> Operations 6-8 (guc_disable_doorbell(), host2guc_release_doorbell(), and
> release_doorbell()) were called in sequence from guc_client_free(), but
> are now moved into the teardown phase of the common function.
>
> Steps 4-5 (guc_init_doorbell() and host2guc_allocate_doorbell()) were
> similarly done as sequential steps in guc_client_alloc(), but since it
> turns out that we don't need to be able to do them separately they're
> now collected into the setup phase of the common function.
>
> The only new code (and new capability) is the block tagged
>      /* Update the GuC's idea of the doorbell ID */
> i.e. we can now *change* the doorbell register used by an existing
> client, whereas previously it was set once for the entire lifetime
> of the client. We will use this new feature in the next patch.
>
> v2: Trivial independent fixes pushed ahead as separate patches.
>      MUCH longer commit message :) [Tvrtko Ursulin]
>
> Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_guc_submission.c | 94 +++++++++++++++++-------------
>   1 file changed, 53 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> index 1c4ff3c..62bf4bd 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -174,31 +174,59 @@ static int host2guc_sample_forcewake(struct intel_guc *guc,
>    * client object which contains the page being used for the doorbell
>    */
>
> -static void guc_init_doorbell(struct intel_guc *guc,
> -			      struct i915_guc_client *client)
> +static int guc_update_doorbell_id(struct intel_guc *guc,
> +				  struct i915_guc_client *client,
> +				  u16 new_id)
>   {
> +	struct sg_table *sg = guc->ctx_pool_obj->pages;
> +	void *doorbell_bitmap = guc->doorbell_bitmap;
>   	struct guc_doorbell_info *doorbell;
> +	struct guc_context_desc desc;
> +	size_t len;
>
>   	doorbell = client->client_base + client->doorbell_offset;
>
> -	doorbell->db_status = GUC_DOORBELL_ENABLED;
> +	if (client->doorbell_id != GUC_INVALID_DOORBELL_ID &&
> +	    test_bit(client->doorbell_id, doorbell_bitmap)) {
> +		/* Deactivate the old doorbell */
> +		doorbell->db_status = GUC_DOORBELL_DISABLED;
> +		(void)host2guc_release_doorbell(guc, client);
> +		__clear_bit(client->doorbell_id, doorbell_bitmap);
> +	}
> +
> +	/* Update the GuC's idea of the doorbell ID */
> +	len = sg_pcopy_to_buffer(sg->sgl, sg->nents, &desc, sizeof(desc),
> +			     sizeof(desc) * client->ctx_index);
> +	if (len != sizeof(desc))
> +		return -EFAULT;
> +	desc.db_id = new_id;
> +	len = sg_pcopy_from_buffer(sg->sgl, sg->nents, &desc, sizeof(desc),
> +			     sizeof(desc) * client->ctx_index);
> +	if (len != sizeof(desc))
> +		return -EFAULT;
> +
> +	client->doorbell_id = new_id;
> +	if (new_id == GUC_INVALID_DOORBELL_ID)
> +		return 0;
> +
> +	/* Activate the new doorbell */
> +	__set_bit(new_id, doorbell_bitmap);
>   	doorbell->cookie = 0;
> +	doorbell->db_status = GUC_DOORBELL_ENABLED;
> +	return host2guc_allocate_doorbell(guc, client);
> +}
> +
> +static int guc_init_doorbell(struct intel_guc *guc,
> +			      struct i915_guc_client *client,
> +			      uint16_t db_id)
> +{
> +	return guc_update_doorbell_id(guc, client, db_id);
>   }
>
>   static void guc_disable_doorbell(struct intel_guc *guc,
>   				 struct i915_guc_client *client)
>   {
> -	struct drm_i915_private *dev_priv = guc_to_i915(guc);
> -	struct guc_doorbell_info *doorbell;
> -	i915_reg_t drbreg = GEN8_DRBREGL(client->doorbell_id);
> -	int value;
> -
> -	doorbell = client->client_base + client->doorbell_offset;
> -
> -	doorbell->db_status = GUC_DOORBELL_DISABLED;
> -
> -	value = I915_READ(drbreg);
> -	WARN_ON((value & GEN8_DRB_VALID) != 0);
> +	(void)guc_update_doorbell_id(guc, client, GUC_INVALID_DOORBELL_ID);
>
>   	/* XXX: wait for any interrupts */
>   	/* XXX: wait for workqueue to drain */
> @@ -254,11 +282,6 @@ static uint16_t assign_doorbell(struct intel_guc *guc, uint32_t priority)
>   	return id;
>   }
>
> -static void release_doorbell(struct intel_guc *guc, uint16_t id)
> -{
> -	__clear_bit(id, guc->doorbell_bitmap);
> -}
> -
>   /*
>    * Initialise the process descriptor shared with the GuC firmware.
>    */
> @@ -651,21 +674,11 @@ static void gem_release_guc_obj(struct drm_i915_gem_object *obj)
>   	 */
>
>   	if (client->client_base) {
> -		uint16_t db_id = client->doorbell_id;
> -
>   		/*
> -		 * If we got as far as setting up a doorbell, make sure
> -		 * we shut it down before unmapping & deallocating the
> -		 * memory. So first disable the doorbell, then tell the
> -		 * GuC that we've finished with it, finally deallocate
> -		 * it in our bitmap
> +		 * If we got as far as setting up a doorbell, make sure we
> +		 * shut it down before unmapping & deallocating the memory.
>   		 */
> -		if (db_id != GUC_INVALID_DOORBELL_ID) {
> -			guc_disable_doorbell(guc, client);
> -			if (test_bit(db_id, guc->doorbell_bitmap))
> -				host2guc_release_doorbell(guc, client);
> -			release_doorbell(guc, db_id);
> -		}
> +		guc_disable_doorbell(guc, client);
>
>   		kunmap(kmap_to_page(client->client_base));
>   	}
> @@ -700,6 +713,7 @@ static void gem_release_guc_obj(struct drm_i915_gem_object *obj)
>   	struct i915_guc_client *client;
>   	struct intel_guc *guc = &dev_priv->guc;
>   	struct drm_i915_gem_object *obj;
> +	uint16_t db_id;
>
>   	client = kzalloc(sizeof(*client), GFP_KERNEL);
>   	if (!client)
> @@ -740,22 +754,20 @@ static void gem_release_guc_obj(struct drm_i915_gem_object *obj)
>   	else
>   		client->proc_desc_offset = (GUC_DB_SIZE / 2);
>
> -	client->doorbell_id = assign_doorbell(guc, client->priority);
> -	if (client->doorbell_id == GUC_INVALID_DOORBELL_ID)
> +	db_id = assign_doorbell(guc, client->priority);
> +	if (db_id == GUC_INVALID_DOORBELL_ID)
>   		/* XXX: evict a doorbell instead */
>   		goto err;
>
>   	guc_init_proc_desc(guc, client);
>   	guc_init_ctx_desc(guc, client);
> -	guc_init_doorbell(guc, client);
> -
> -	/* XXX: Any cache flushes needed? General domain mgmt calls? */
> -
> -	if (host2guc_allocate_doorbell(guc, client))
> +	if (guc_init_doorbell(guc, client, db_id))
>   		goto err;
>
> -	DRM_DEBUG_DRIVER("new priority %u client %p: ctx_index %u db_id %u\n",
> -		priority, client, client->ctx_index, client->doorbell_id);
> +	DRM_DEBUG_DRIVER("new priority %u client %p: ctx_index %u\n",
> +		priority, client, client->ctx_index);
> +	DRM_DEBUG_DRIVER("doorbell id %u, cacheline offset 0x%x\n",
> +		client->doorbell_id, client->doorbell_offset);
>
>   	return client;
>
>

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

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

[PATCH v3 6/7] drm/i915/guc: replace assign_doorbell() with select_doorbell_register()

[Dave Gordon]

This version doesn't update the doorbell bitmap, as that will
be done when the selected doorbell is associated with a client.

The call is now slightly earlier, just on the general principle
that potentially-failing operations should be done as early as
possible, to eliminate late failures and simplify recovery.

Suggested-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_guc_submission.c | 62 +++++++++++++++---------------
 1 file changed, 31 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 62bf4bd..a252505 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -232,6 +232,32 @@ static void guc_disable_doorbell(struct intel_guc *guc,
 	/* XXX: wait for workqueue to drain */
 }
 
+static uint16_t
+select_doorbell_register(struct intel_guc *guc, uint32_t priority)
+{
+	/*
+	 * The bitmap tracks which doorbell registers are currently in use.
+	 * It is split into two halves; the first half is used for normal
+	 * priority contexts, the second half for high-priority ones.
+	 * Note that logically higher priorities are numerically less than
+	 * normal ones, so the test below means "is it high-priority?"
+	 */
+	const bool hi_pri = (priority <= GUC_CTX_PRIORITY_HIGH);
+	const uint16_t half = GUC_MAX_DOORBELLS / 2;
+	const uint16_t start = hi_pri ? half : 0;
+	const uint16_t end = start + half;
+	uint16_t id;
+
+	id = find_next_zero_bit(guc->doorbell_bitmap, end, start);
+	if (id == end)
+		id = GUC_INVALID_DOORBELL_ID;
+
+	DRM_DEBUG_DRIVER("assigned %s priority doorbell id 0x%x\n",
+			hi_pri ? "high" : "normal", id);
+
+	return id;
+}
+
 /*
  * Select, assign and relase doorbell cachelines
  *
@@ -256,32 +282,6 @@ static uint32_t select_doorbell_cacheline(struct intel_guc *guc)
 	return offset;
 }
 
-static uint16_t assign_doorbell(struct intel_guc *guc, uint32_t priority)
-{
-	/*
-	 * The bitmap is split into two halves; the first half is used for
-	 * normal priority contexts, the second half for high-priority ones.
-	 * Note that logically higher priorities are numerically less than
-	 * normal ones, so the test below means "is it high-priority?"
-	 */
-	const bool hi_pri = (priority <= GUC_CTX_PRIORITY_HIGH);
-	const uint16_t half = GUC_MAX_DOORBELLS / 2;
-	const uint16_t start = hi_pri ? half : 0;
-	const uint16_t end = start + half;
-	uint16_t id;
-
-	id = find_next_zero_bit(guc->doorbell_bitmap, end, start);
-	if (id == end)
-		id = GUC_INVALID_DOORBELL_ID;
-	else
-		__set_bit(id, guc->doorbell_bitmap);
-
-	DRM_DEBUG_DRIVER("assigned %s priority doorbell id 0x%x\n",
-			hi_pri ? "high" : "normal", id);
-
-	return id;
-}
-
 /*
  * Initialise the process descriptor shared with the GuC firmware.
  */
@@ -742,6 +742,11 @@ static void gem_release_guc_obj(struct drm_i915_gem_object *obj)
 	client->wq_offset = GUC_DB_SIZE;
 	client->wq_size = GUC_WQ_SIZE;
 
+	db_id = select_doorbell_register(guc, client->priority);
+	if (db_id == GUC_INVALID_DOORBELL_ID)
+		/* XXX: evict a doorbell instead? */
+		goto err;
+
 	client->doorbell_offset = select_doorbell_cacheline(guc);
 
 	/*
@@ -754,11 +759,6 @@ static void gem_release_guc_obj(struct drm_i915_gem_object *obj)
 	else
 		client->proc_desc_offset = (GUC_DB_SIZE / 2);
 
-	db_id = assign_doorbell(guc, client->priority);
-	if (db_id == GUC_INVALID_DOORBELL_ID)
-		/* XXX: evict a doorbell instead */
-		goto err;
-
 	guc_init_proc_desc(guc, client);
 	guc_init_ctx_desc(guc, client);
 	if (guc_init_doorbell(guc, client, db_id))
-- 
1.9.1

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

Re: [PATCH v3 6/7] drm/i915/guc: replace assign_doorbell() with select_doorbell_register()

[Tvrtko Ursulin]

On 13/06/16 17:57, Dave Gordon wrote:
> This version doesn't update the doorbell bitmap, as that will
> be done when the selected doorbell is associated with a client.
>
> The call is now slightly earlier, just on the general principle
> that potentially-failing operations should be done as early as
> possible, to eliminate late failures and simplify recovery.
>
> Suggested-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_guc_submission.c | 62 +++++++++++++++---------------
>   1 file changed, 31 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> index 62bf4bd..a252505 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -232,6 +232,32 @@ static void guc_disable_doorbell(struct intel_guc *guc,
>   	/* XXX: wait for workqueue to drain */
>   }
>
> +static uint16_t
> +select_doorbell_register(struct intel_guc *guc, uint32_t priority)
> +{
> +	/*
> +	 * The bitmap tracks which doorbell registers are currently in use.
> +	 * It is split into two halves; the first half is used for normal
> +	 * priority contexts, the second half for high-priority ones.
> +	 * Note that logically higher priorities are numerically less than
> +	 * normal ones, so the test below means "is it high-priority?"
> +	 */
> +	const bool hi_pri = (priority <= GUC_CTX_PRIORITY_HIGH);
> +	const uint16_t half = GUC_MAX_DOORBELLS / 2;
> +	const uint16_t start = hi_pri ? half : 0;
> +	const uint16_t end = start + half;
> +	uint16_t id;
> +
> +	id = find_next_zero_bit(guc->doorbell_bitmap, end, start);
> +	if (id == end)
> +		id = GUC_INVALID_DOORBELL_ID;
> +
> +	DRM_DEBUG_DRIVER("assigned %s priority doorbell id 0x%x\n",
> +			hi_pri ? "high" : "normal", id);
> +
> +	return id;
> +}
> +
>   /*
>    * Select, assign and relase doorbell cachelines
>    *
> @@ -256,32 +282,6 @@ static uint32_t select_doorbell_cacheline(struct intel_guc *guc)
>   	return offset;
>   }
>
> -static uint16_t assign_doorbell(struct intel_guc *guc, uint32_t priority)
> -{
> -	/*
> -	 * The bitmap is split into two halves; the first half is used for
> -	 * normal priority contexts, the second half for high-priority ones.
> -	 * Note that logically higher priorities are numerically less than
> -	 * normal ones, so the test below means "is it high-priority?"
> -	 */
> -	const bool hi_pri = (priority <= GUC_CTX_PRIORITY_HIGH);
> -	const uint16_t half = GUC_MAX_DOORBELLS / 2;
> -	const uint16_t start = hi_pri ? half : 0;
> -	const uint16_t end = start + half;
> -	uint16_t id;
> -
> -	id = find_next_zero_bit(guc->doorbell_bitmap, end, start);
> -	if (id == end)
> -		id = GUC_INVALID_DOORBELL_ID;
> -	else
> -		__set_bit(id, guc->doorbell_bitmap);
> -
> -	DRM_DEBUG_DRIVER("assigned %s priority doorbell id 0x%x\n",
> -			hi_pri ? "high" : "normal", id);
> -
> -	return id;
> -}
> -
>   /*
>    * Initialise the process descriptor shared with the GuC firmware.
>    */
> @@ -742,6 +742,11 @@ static void gem_release_guc_obj(struct drm_i915_gem_object *obj)
>   	client->wq_offset = GUC_DB_SIZE;
>   	client->wq_size = GUC_WQ_SIZE;
>
> +	db_id = select_doorbell_register(guc, client->priority);
> +	if (db_id == GUC_INVALID_DOORBELL_ID)
> +		/* XXX: evict a doorbell instead? */
> +		goto err;
> +
>   	client->doorbell_offset = select_doorbell_cacheline(guc);
>
>   	/*
> @@ -754,11 +759,6 @@ static void gem_release_guc_obj(struct drm_i915_gem_object *obj)
>   	else
>   		client->proc_desc_offset = (GUC_DB_SIZE / 2);
>
> -	db_id = assign_doorbell(guc, client->priority);
> -	if (db_id == GUC_INVALID_DOORBELL_ID)
> -		/* XXX: evict a doorbell instead */
> -		goto err;
> -
>   	guc_init_proc_desc(guc, client);
>   	guc_init_ctx_desc(guc, client);
>   	if (guc_init_doorbell(guc, client, db_id))
>

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

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

[PATCH v3 7/7] drm/i915/guc: (re)initialise doorbell h/w when enabling GuC submission

[Dave Gordon]

During a hibernate/resume cycle, the whole system is reset, including
the GuC and the doorbell hardware. Then the system is booted up, drivers
are loaded, etc -- the GuC firmware may be loaded and set running at
this point. But then, the booted kernel is replaced by the hibernated
image, and this resumed kernel will also try to reload the GuC firmware
(which will fail). To recover, we reset the GuC and try again (which
should work). But this GuC reset doesn't also reset the doorbell
hardware, so it can be left in a state inconsistent with that assumed
by the driver and/or the newly-loaded GuC firmware.

It would be better if the GuC reset also cleared all doorbell state,
but that's not how the hardware currently works; also, the driver cannot
directly reprogram the doorbell hardware (only the GuC can do that).

So this patch cycles through all doorbells, assigning and releasing each
in turn, so that all the doorbell hardware is left in a consistent
state, no matter how it was programmed by the previously-running kernel
and/or GuC firmware.

v2: don't use kmap_atomic() now that client page 0 is kept mapped.

Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_guc_submission.c | 44 +++++++++++++++++++++++++++++-
 1 file changed, 43 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index a252505..22a55ac 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -693,6 +693,48 @@ static void gem_release_guc_obj(struct drm_i915_gem_object *obj)
 	kfree(client);
 }
 
+/*
+ * Borrow the first client to set up & tear down every doorbell
+ * in turn, to ensure that all doorbell h/w is (re)initialised.
+ */
+static void guc_init_doorbell_hw(struct intel_guc *guc)
+{
+	struct drm_i915_private *dev_priv = guc_to_i915(guc);
+	struct i915_guc_client *client = guc->execbuf_client;
+	uint16_t db_id, i;
+	int err;
+
+	db_id = client->doorbell_id;
+
+	for (i = 0; i < GUC_MAX_DOORBELLS; ++i) {
+		i915_reg_t drbreg = GEN8_DRBREGL(i);
+		u32 value = I915_READ(drbreg);
+
+		err = guc_update_doorbell_id(guc, client, i);
+
+		/* Report update failure or unexpectedly active doorbell */
+		if (err || (i != db_id && (value & GUC_DOORBELL_ENABLED)))
+			DRM_DEBUG_DRIVER("Doorbell %d (reg 0x%x) was 0x%x, err %d\n",
+					  i, drbreg.reg, value, err);
+	}
+
+	/* Restore to original value */
+	err = guc_update_doorbell_id(guc, client, db_id);
+	if (err)
+		DRM_ERROR("Failed to restore doorbell to %d, err %d\n",
+			db_id, err);
+
+	for (i = 0; i < GUC_MAX_DOORBELLS; ++i) {
+		i915_reg_t drbreg = GEN8_DRBREGL(i);
+		u32 value = I915_READ(drbreg);
+
+		if (i != db_id && (value & GUC_DOORBELL_ENABLED))
+			DRM_DEBUG_DRIVER("Doorbell %d (reg 0x%x) finally 0x%x\n",
+					  i, drbreg.reg, value);
+
+	}
+}
+
 /**
  * guc_client_alloc() - Allocate an i915_guc_client
  * @dev_priv:	driver private data structure
@@ -956,8 +998,8 @@ int i915_guc_submission_enable(struct drm_i915_private *dev_priv)
 	}
 
 	guc->execbuf_client = client;
-
 	host2guc_sample_forcewake(guc, client);
+	guc_init_doorbell_hw(guc);
 
 	return 0;
 }
-- 
1.9.1

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

✗ Ro.CI.BAT: failure for drm/i915/guc: updates to GuC doorbell handling (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/guc: updates to GuC doorbell handling (rev2)
URL   : https://patchwork.freedesktop.org/series/8553/
State : failure

== Summary ==

Series 8553v2 drm/i915/guc: updates to GuC doorbell handling
http://patchwork.freedesktop.org/api/1.0/series/8553/revisions/2/mbox

Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-cmd:
                fail       -> PASS       (ro-byt-n2820)
Test kms_flip:
        Subgroup basic-flip-vs-wf_vblank:
                pass       -> FAIL       (ro-bdw-i7-5600u)

fi-bdw-i7-5557u  total:213  pass:201  dwarn:0   dfail:0   fail:0   skip:12 
fi-skl-i7-6700k  total:213  pass:188  dwarn:0   dfail:0   fail:0   skip:25 
fi-snb-i7-2600   total:213  pass:174  dwarn:0   dfail:0   fail:0   skip:39 
ro-bdw-i7-5600u  total:213  pass:184  dwarn:0   dfail:0   fail:1   skip:28 
ro-bsw-n3050     total:213  pass:172  dwarn:0   dfail:0   fail:2   skip:39 
ro-byt-n2820     total:213  pass:174  dwarn:0   dfail:0   fail:2   skip:37 
ro-hsw-i3-4010u  total:213  pass:190  dwarn:0   dfail:0   fail:0   skip:23 
ro-hsw-i7-4770r  total:213  pass:190  dwarn:0   dfail:0   fail:0   skip:23 
ro-ilk-i7-620lm  total:213  pass:150  dwarn:0   dfail:0   fail:1   skip:62 
ro-ilk1-i5-650   total:208  pass:150  dwarn:0   dfail:0   fail:1   skip:57 
ro-ivb-i7-3770   total:213  pass:181  dwarn:0   dfail:0   fail:0   skip:32 
ro-ivb2-i7-3770  total:213  pass:185  dwarn:0   dfail:0   fail:0   skip:28 
ro-skl3-i5-6260u total:213  pass:201  dwarn:1   dfail:0   fail:0   skip:11 
ro-snb-i7-2620M  total:213  pass:174  dwarn:0   dfail:0   fail:1   skip:38 
fi-hsw-i7-4770k failed to connect after reboot
ro-bdw-i5-5250u failed to connect after reboot
ro-bdw-i7-5557U failed to connect after reboot

Results at /archive/results/CI_IGT_test/RO_Patchwork_1178/

b7936d4 drm-intel-nightly: 2016y-06m-13d-16h-41m-03s UTC integration manifest
58358c2 drm/i915/guc: (re)initialise doorbell h/w when enabling GuC submission
6dcd14b drm/i915/guc: replace assign_doorbell() with select_doorbell_register()
0015c92 drm/i915/guc: refactor doorbell management code
1c6d522 drm/i915/guc: move guc_ring_doorbell() nearer to callsite
e42a6c9 drm/i915/guc: remove writes to GEN8_DRBREG registers
046480b drm/i915/guc: prefer __set/clear_bit() to bitmap_set/clear()
574f560 drm/i915/guc: add doorbell map to debugfs/i915_guc_info

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

Re: ✗ Ro.CI.BAT: failure for drm/i915/guc: updates to GuC doorbell handling (rev2)

[Dave Gordon]

On 14/06/16 06:27, Patchwork wrote:
> == Series Details ==
>
> Series: drm/i915/guc: updates to GuC doorbell handling (rev2)
> URL   : https://patchwork.freedesktop.org/series/8553/
> State : failure
>
> == Summary ==
>
> Series 8553v2 drm/i915/guc: updates to GuC doorbell handling
> http://patchwork.freedesktop.org/api/1.0/series/8553/revisions/2/mbox
>
> Test gem_exec_flush:
>          Subgroup basic-batch-kernel-default-cmd:
>                  fail       -> PASS       (ro-byt-n2820)
> Test kms_flip:
>          Subgroup basic-flip-vs-wf_vblank:
>                  pass       -> FAIL       (ro-bdw-i7-5600u)

Bug 95236 - [BAT various] kms_flip fails with "inter-flip ts jitter: 0s, 
183334usec" or similar time around 180msec.

.Dave.

> fi-bdw-i7-5557u  total:213  pass:201  dwarn:0   dfail:0   fail:0   skip:12
> fi-skl-i7-6700k  total:213  pass:188  dwarn:0   dfail:0   fail:0   skip:25
> fi-snb-i7-2600   total:213  pass:174  dwarn:0   dfail:0   fail:0   skip:39
> ro-bdw-i7-5600u  total:213  pass:184  dwarn:0   dfail:0   fail:1   skip:28
> ro-bsw-n3050     total:213  pass:172  dwarn:0   dfail:0   fail:2   skip:39
> ro-byt-n2820     total:213  pass:174  dwarn:0   dfail:0   fail:2   skip:37
> ro-hsw-i3-4010u  total:213  pass:190  dwarn:0   dfail:0   fail:0   skip:23
> ro-hsw-i7-4770r  total:213  pass:190  dwarn:0   dfail:0   fail:0   skip:23
> ro-ilk-i7-620lm  total:213  pass:150  dwarn:0   dfail:0   fail:1   skip:62
> ro-ilk1-i5-650   total:208  pass:150  dwarn:0   dfail:0   fail:1   skip:57
> ro-ivb-i7-3770   total:213  pass:181  dwarn:0   dfail:0   fail:0   skip:32
> ro-ivb2-i7-3770  total:213  pass:185  dwarn:0   dfail:0   fail:0   skip:28
> ro-skl3-i5-6260u total:213  pass:201  dwarn:1   dfail:0   fail:0   skip:11
> ro-snb-i7-2620M  total:213  pass:174  dwarn:0   dfail:0   fail:1   skip:38
> fi-hsw-i7-4770k failed to connect after reboot
> ro-bdw-i5-5250u failed to connect after reboot
> ro-bdw-i7-5557U failed to connect after reboot
>
> Results at /archive/results/CI_IGT_test/RO_Patchwork_1178/
>
> b7936d4 drm-intel-nightly: 2016y-06m-13d-16h-41m-03s UTC integration manifest
> 58358c2 drm/i915/guc: (re)initialise doorbell h/w when enabling GuC submission
> 6dcd14b drm/i915/guc: replace assign_doorbell() with select_doorbell_register()
> 0015c92 drm/i915/guc: refactor doorbell management code
> 1c6d522 drm/i915/guc: move guc_ring_doorbell() nearer to callsite
> e42a6c9 drm/i915/guc: remove writes to GEN8_DRBREG registers
> 046480b drm/i915/guc: prefer __set/clear_bit() to bitmap_set/clear()
> 574f560 drm/i915/guc: add doorbell map to debugfs/i915_guc_info


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

Re: ✗ Ro.CI.BAT: failure for drm/i915/guc: updates to GuC doorbell handling (rev2)

[Tvrtko Ursulin]

On 14/06/16 14:42, Dave Gordon wrote:
> On 14/06/16 06:27, Patchwork wrote:
>> == Series Details ==
>>
>> Series: drm/i915/guc: updates to GuC doorbell handling (rev2)
>> URL   : https://patchwork.freedesktop.org/series/8553/
>> State : failure
>>
>> == Summary ==
>>
>> Series 8553v2 drm/i915/guc: updates to GuC doorbell handling
>> http://patchwork.freedesktop.org/api/1.0/series/8553/revisions/2/mbox
>>
>> Test gem_exec_flush:
>>          Subgroup basic-batch-kernel-default-cmd:
>>                  fail       -> PASS       (ro-byt-n2820)
>> Test kms_flip:
>>          Subgroup basic-flip-vs-wf_vblank:
>>                  pass       -> FAIL       (ro-bdw-i7-5600u)
>
> Bug 95236 - [BAT various] kms_flip fails with "inter-flip ts jitter: 0s,
> 183334usec" or similar time around 180msec.
>
> .Dave.
>
>> fi-bdw-i7-5557u  total:213  pass:201  dwarn:0   dfail:0   fail:0
>> skip:12
>> fi-skl-i7-6700k  total:213  pass:188  dwarn:0   dfail:0   fail:0
>> skip:25
>> fi-snb-i7-2600   total:213  pass:174  dwarn:0   dfail:0   fail:0
>> skip:39
>> ro-bdw-i7-5600u  total:213  pass:184  dwarn:0   dfail:0   fail:1
>> skip:28
>> ro-bsw-n3050     total:213  pass:172  dwarn:0   dfail:0   fail:2
>> skip:39
>> ro-byt-n2820     total:213  pass:174  dwarn:0   dfail:0   fail:2
>> skip:37
>> ro-hsw-i3-4010u  total:213  pass:190  dwarn:0   dfail:0   fail:0
>> skip:23
>> ro-hsw-i7-4770r  total:213  pass:190  dwarn:0   dfail:0   fail:0
>> skip:23
>> ro-ilk-i7-620lm  total:213  pass:150  dwarn:0   dfail:0   fail:1
>> skip:62
>> ro-ilk1-i5-650   total:208  pass:150  dwarn:0   dfail:0   fail:1
>> skip:57
>> ro-ivb-i7-3770   total:213  pass:181  dwarn:0   dfail:0   fail:0
>> skip:32
>> ro-ivb2-i7-3770  total:213  pass:185  dwarn:0   dfail:0   fail:0
>> skip:28
>> ro-skl3-i5-6260u total:213  pass:201  dwarn:1   dfail:0   fail:0
>> skip:11
>> ro-snb-i7-2620M  total:213  pass:174  dwarn:0   dfail:0   fail:1
>> skip:38
>> fi-hsw-i7-4770k failed to connect after reboot
>> ro-bdw-i5-5250u failed to connect after reboot
>> ro-bdw-i7-5557U failed to connect after reboot
>>
>> Results at /archive/results/CI_IGT_test/RO_Patchwork_1178/
>>
>> b7936d4 drm-intel-nightly: 2016y-06m-13d-16h-41m-03s UTC integration
>> manifest
>> 58358c2 drm/i915/guc: (re)initialise doorbell h/w when enabling GuC
>> submission
>> 6dcd14b drm/i915/guc: replace assign_doorbell() with
>> select_doorbell_register()
>> 0015c92 drm/i915/guc: refactor doorbell management code
>> 1c6d522 drm/i915/guc: move guc_ring_doorbell() nearer to callsite
>> e42a6c9 drm/i915/guc: remove writes to GEN8_DRBREG registers
>> 046480b drm/i915/guc: prefer __set/clear_bit() to bitmap_set/clear()
>> 574f560 drm/i915/guc: add doorbell map to debugfs/i915_guc_info

Merged to dinq, thanks for the patches and review. :)

Regards,

Tvrtko

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