[PATCH v2 1/6] HAX drm/i915: Enable guc submission

[PATCH v2 1/6] HAX drm/i915: Enable guc submission

[Chris Wilson]

---
 drivers/gpu/drm/i915/i915_params.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 0e280fbd52f1..ef1e9921a2af 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 = -1,
+	.enable_guc_submission = -1,
 	.guc_log_level = -1,
 	.enable_dp_mst = true,
 	.inject_load_failure = 0,
-- 
2.10.2

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

[PATCH v2 2/6] drm/i915: Trim i915_guc_info() stack usage

[Chris Wilson]

i915_guc_info() (part of debugfs output) tries to avoid holding
struct_mutex for a long period by copying onto the stack. This causes a
warning that the stack frame is massive, so stop doing that. We can even
forgo holding the struct_mutex here as that doesn't serialise the values
being read (and the lists used exist for the device lifetime).

v2: Skip printing anything if guc->execbuf_client is disabled (avoids
potential NULL dereference).

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 46 ++++++++++++++++---------------------
 1 file changed, 20 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 8eb8c29b7492..a3eaf88f4a42 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2434,47 +2434,41 @@ static void i915_guc_client_info(struct seq_file *m,
 static int i915_guc_info(struct seq_file *m, void *data)
 {
 	struct drm_i915_private *dev_priv = node_to_i915(m->private);
-	struct drm_device *dev = &dev_priv->drm;
-	struct intel_guc guc;
-	struct i915_guc_client client = {};
+	const struct intel_guc *guc = &dev_priv->guc;
 	struct intel_engine_cs *engine;
 	enum intel_engine_id id;
-	u64 total = 0;
-
-	if (!HAS_GUC_SCHED(dev_priv))
-		return 0;
+	u64 total;
 
-	if (mutex_lock_interruptible(&dev->struct_mutex))
+	if (!guc->execbuf_client) {
+		seq_printf(m, "GuC submission %s\n",
+			   HAS_GUC_SCHED(dev_priv) ?
+			   "disabled" :
+			   "not supported");
 		return 0;
-
-	/* Take a local copy of the GuC data, so we can dump it at leisure */
-	guc = dev_priv->guc;
-	if (guc.execbuf_client)
-		client = *guc.execbuf_client;
-
-	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, "\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);
-	seq_printf(m, "GuC last action status: 0x%x\n", guc.action_status);
-	seq_printf(m, "GuC last action error code: %d\n", guc.action_err);
+	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);
+	seq_printf(m, "GuC last action status: 0x%x\n", guc->action_status);
+	seq_printf(m, "GuC last action error code: %d\n", guc->action_err);
 
+	total = 0;
 	seq_printf(m, "\nGuC submissions:\n");
 	for_each_engine(engine, dev_priv, id) {
-		u64 submissions = guc.submissions[id];
+		u64 submissions = guc->submissions[id];
 		total += submissions;
 		seq_printf(m, "\t%-24s: %10llu, last seqno 0x%08x\n",
-			engine->name, submissions, guc.last_seqno[id]);
+			engine->name, submissions, guc->last_seqno[id]);
 	}
 	seq_printf(m, "\t%s: %llu\n", "Total", total);
 
-	seq_printf(m, "\nGuC execbuf client @ %p:\n", guc.execbuf_client);
-	i915_guc_client_info(m, dev_priv, &client);
+	seq_printf(m, "\nGuC execbuf client @ %p:\n", guc->execbuf_client);
+	i915_guc_client_info(m, dev_priv, guc->execbuf_client);
 
 	i915_guc_log_info(m, dev_priv);
 
-- 
2.10.2

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

[PATCH v2 3/6] drm/i915/guc: Rename client->cookie to match use

[Chris Wilson]

The client->cookie is a shadow of the doorbell->cookie value, so rename
it to indicate its association with the doorbell, like the doorbell id
and offset.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c        | 2 +-
 drivers/gpu/drm/i915/i915_guc_submission.c | 6 +++---
 drivers/gpu/drm/i915/intel_uc.h            | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index a3eaf88f4a42..fe400b7b66a8 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2414,7 +2414,7 @@ static void i915_guc_client_info(struct seq_file *m,
 	seq_printf(m, "\tPriority %d, GuC ctx index: %u, PD offset 0x%x\n",
 		client->priority, client->ctx_index, client->proc_desc_offset);
 	seq_printf(m, "\tDoorbell id %d, offset: 0x%x, cookie 0x%x\n",
-		client->doorbell_id, client->doorbell_offset, client->cookie);
+		client->doorbell_id, client->doorbell_offset, client->doorbell_cookie);
 	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 f724a30a3232..aaaf3928394e 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -454,11 +454,11 @@ static int guc_ring_doorbell(struct i915_guc_client *gc)
 
 	/* current cookie */
 	db_cmp.db_status = GUC_DOORBELL_ENABLED;
-	db_cmp.cookie = gc->cookie;
+	db_cmp.cookie = gc->doorbell_cookie;
 
 	/* cookie to be updated */
 	db_exc.db_status = GUC_DOORBELL_ENABLED;
-	db_exc.cookie = gc->cookie + 1;
+	db_exc.cookie = gc->doorbell_cookie + 1;
 	if (db_exc.cookie == 0)
 		db_exc.cookie = 1;
 
@@ -473,7 +473,7 @@ static int guc_ring_doorbell(struct i915_guc_client *gc)
 		/* if the exchange was successfully executed */
 		if (db_ret.value_qw == db_cmp.value_qw) {
 			/* db was successfully rung */
-			gc->cookie = db_exc.cookie;
+			gc->doorbell_cookie = db_exc.cookie;
 			ret = 0;
 			break;
 		}
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index de2b314cb1d7..8507a8f35b72 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -74,7 +74,7 @@ struct i915_guc_client {
 	uint32_t proc_desc_offset;
 
 	uint32_t doorbell_offset;
-	uint32_t cookie;
+	uint32_t doorbell_cookie;
 	uint16_t doorbell_id;
 	uint16_t padding[3];		/* Maintain alignment		*/
 
-- 
2.10.2

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

[PATCH v2 4/6] drm/i915/guc: Initialise doorbell cookie to matching value

[Chris Wilson]

Set the initial value of the doorbell cookie from the client.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_guc_submission.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index aaaf3928394e..dd879f0b077d 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -134,8 +134,8 @@ static int guc_update_doorbell_id(struct intel_guc *guc,
 
 	/* Activate the new doorbell */
 	__set_bit(new_id, doorbell_bitmap);
-	doorbell->cookie = 0;
 	doorbell->db_status = GUC_DOORBELL_ENABLED;
+	doorbell->cookie = client->doorbell_cookie;
 	return guc_allocate_doorbell(guc, client);
 }
 
-- 
2.10.2

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

[PATCH v2 5/6] drm/i915/guc: Keep the execbuf client allocated across reset

[Chris Wilson]

In order to avoid some complexity in trying to reconstruct the
workqueues across reset, remember them instead. The issue comes when we
have to handle a reset between request allocation and submission, the
request has reserved space in the wq, but is not in any list so we fail
to restore the reserved space. By keeping the execbuf client intact
across the reset, we also keep the reservations.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_guc_submission.c | 84 +++++++++++++++++++-----------
 1 file changed, 53 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index dd879f0b077d..b0f858cf2881 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -139,13 +139,6 @@ static int guc_update_doorbell_id(struct intel_guc *guc,
 	return guc_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)
 {
@@ -666,8 +659,7 @@ static void guc_init_doorbell_hw(struct intel_guc *guc)
 	uint16_t db_id;
 	int i, err;
 
-	/* Save client's original doorbell selection */
-	db_id = client->doorbell_id;
+	guc_disable_doorbell(guc, client);
 
 	for (i = 0; i < GUC_MAX_DOORBELLS; ++i) {
 		/* Skip if doorbell is OK */
@@ -680,7 +672,9 @@ static void guc_init_doorbell_hw(struct intel_guc *guc)
 					i, err);
 	}
 
-	/* Restore to original value */
+	db_id = select_doorbell_register(guc, client->priority);
+	WARN_ON(db_id == GUC_INVALID_DOORBELL_ID);
+
 	err = guc_update_doorbell_id(guc, client, db_id);
 	if (err)
 		DRM_WARN("Failed to restore doorbell to %d, err %d\n",
@@ -770,8 +764,13 @@ guc_client_alloc(struct drm_i915_private *dev_priv,
 
 	guc_proc_desc_init(guc, client);
 	guc_ctx_desc_init(guc, client);
-	if (guc_init_doorbell(guc, client, db_id))
-		goto err;
+
+	/* For runtime client allocation we need to enable the doorbell. Not
+	 * required yet for the static execbuf_client as this special kernel
+	 * client is enabled from i915_guc_submission_enable().
+	 *
+	 * guc_update_doorbell_id(guc, client, db_id);
+	 */
 
 	DRM_DEBUG_DRIVER("new priority %u client %p for engine(s) 0x%x: ctx_index %u\n",
 		priority, client, client->engines, client->ctx_index);
@@ -1371,6 +1370,9 @@ int i915_guc_submission_init(struct drm_i915_private *dev_priv)
 	struct intel_guc *guc = &dev_priv->guc;
 	struct i915_vma *vma;
 
+	if (!HAS_GUC_SCHED(dev_priv))
+		return 0;
+
 	/* Wipe bitmap & delete client in case of reinitialisation */
 	bitmap_clear(guc->doorbell_bitmap, 0, GUC_MAX_DOORBELLS);
 	i915_guc_submission_disable(dev_priv);
@@ -1390,42 +1392,58 @@ int i915_guc_submission_init(struct drm_i915_private *dev_priv)
 	guc_log_create(guc);
 	guc_addon_create(guc);
 
+	guc->execbuf_client = guc_client_alloc(dev_priv,
+					       INTEL_INFO(dev_priv)->ring_mask,
+					       GUC_CTX_PRIORITY_KMD_NORMAL,
+					       dev_priv->kernel_context);
+	if (!guc->execbuf_client) {
+		DRM_ERROR("Failed to create GuC client for execbuf!\n");
+		goto err;
+	}
+
 	return 0;
+
+err:
+	i915_guc_submission_fini(dev_priv);
+	return -ENOMEM;
+}
+
+static void guc_reset_wq(struct i915_guc_client *gc)
+{
+	struct guc_process_desc *desc = gc->vaddr + gc->proc_desc_offset;
+
+	desc->head = 0;
+	desc->tail = 0;
+
+	gc->wq_tail = 0;
 }
 
 int i915_guc_submission_enable(struct drm_i915_private *dev_priv)
 {
 	struct intel_guc *guc = &dev_priv->guc;
-	struct drm_i915_gem_request *request;
-	struct i915_guc_client *client;
+	struct i915_guc_client *client = guc->execbuf_client;
 	struct intel_engine_cs *engine;
 	enum intel_engine_id id;
 
-	/* client for execbuf submission */
-	client = guc_client_alloc(dev_priv,
-				  INTEL_INFO(dev_priv)->ring_mask,
-				  GUC_CTX_PRIORITY_KMD_NORMAL,
-				  dev_priv->kernel_context);
-	if (!client) {
-		DRM_ERROR("Failed to create normal GuC client!\n");
-		return -ENOMEM;
-	}
+	if (!client)
+		return -ENODEV;
 
-	guc->execbuf_client = client;
 	intel_guc_sample_forcewake(guc);
+
+	guc_reset_wq(client);
 	guc_init_doorbell_hw(guc);
 
 	/* Take over from manual control of ELSP (execlists) */
 	for_each_engine(engine, dev_priv, id) {
+		struct drm_i915_gem_request *rq;
+
 		engine->submit_request = i915_guc_submit;
 		engine->schedule = NULL;
 
 		/* Replay the current set of previously submitted requests */
-		list_for_each_entry(request,
-				    &engine->timeline->requests, link) {
+		list_for_each_entry(rq, &engine->timeline->requests, link) {
 			client->wq_rsvd += sizeof(struct guc_wq_item);
-			if (i915_sw_fence_done(&request->submit))
-				i915_guc_submit(request);
+			i915_guc_submit(rq);
 		}
 	}
 
@@ -1441,14 +1459,18 @@ void i915_guc_submission_disable(struct drm_i915_private *dev_priv)
 
 	/* Revert back to manual ELSP submission */
 	intel_execlists_enable_submission(dev_priv);
-
-	guc_client_free(dev_priv, guc->execbuf_client);
-	guc->execbuf_client = NULL;
 }
 
 void i915_guc_submission_fini(struct drm_i915_private *dev_priv)
 {
 	struct intel_guc *guc = &dev_priv->guc;
+	struct i915_guc_client *client;
+
+	client = fetch_and_zero(&guc->execbuf_client);
+	if (!client)
+		return;
+
+	guc_client_free(dev_priv, client);
 
 	i915_vma_unpin_and_release(&guc->ads_vma);
 	i915_vma_unpin_and_release(&guc->log.vma);
-- 
2.10.2

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

[PATCH v2 6/6] drm/i915/guc: Split hw submission for replay after GPU reset

[Chris Wilson]

Something I missed before sending off the partial series was that the
non-scheduler guc reset path was broken (in the full series, this is
pushed to the execlists reset handler). The issue is that after a reset,
we have to refill the GuC workqueues, which we do by resubmitting the
requests. However, if we already have submitted them, the fences within
them have already been used and triggering them again is an error.
Instead, just repopulate the guc workqueue.

[  115.858560] [IGT] gem_busy: starting subtest hang-render
[  135.839867] [drm] GPU HANG: ecode 9:0:0xe757fefe, in gem_busy [1716], reason: Hang on render ring, action: reset
[  135.839902] drm/i915: Resetting chip after gpu hang
[  135.839957] [drm] RC6 on
[  135.858351] ------------[ cut here ]------------
[  135.858357] WARNING: CPU: 2 PID: 45 at drivers/gpu/drm/i915/i915_sw_fence.c:108 i915_sw_fence_complete+0x25/0x30
[  135.858357] Modules linked in: rfcomm bnep binfmt_misc nls_iso8859_1 input_leds snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic snd_hda_intel snd_hda_codec snd_hda_core btusb btrtl snd_hwdep snd_pcm 8250_dw snd_seq_midi hid_lenovo snd_seq_midi_event snd_rawmidi iwlwifi x86_pkg_temp_thermal coretemp snd_seq crct10dif_pclmul snd_seq_device hci_uart snd_timer crc32_pclmul ghash_clmulni_intel idma64 aesni_intel virt_dma btbcm snd btqca aes_x86_64 btintel lrw cfg80211 bluetooth gf128mul glue_helper ablk_helper cryptd soundcore intel_lpss_pci intel_pch_thermal intel_lpss_acpi intel_lpss acpi_als mfd_core kfifo_buf acpi_pad industrialio autofs4 hid_plantronics usbhid dm_mirror dm_region_hash dm_log sdhci_pci ahci sdhci libahci i2c_hid hid
[  135.858389] CPU: 2 PID: 45 Comm: kworker/2:1 Tainted: G        W       4.9.0-rc4+ #238
[  135.858389] Hardware name:                  /NUC6i3SYB, BIOS SYSKLi35.86A.0024.2015.1027.2142 10/27/2015
[  135.858392] Workqueue: events_long i915_hangcheck_elapsed
[  135.858394]  ffffc900001bf9b8 ffffffff812bb238 0000000000000000 0000000000000000
[  135.858396]  ffffc900001bf9f8 ffffffff8104f621 0000006c00000000 ffff8808296137f8
[  135.858398]  0000000000000a00 ffff8808457a0000 ffff880845764e60 ffff880845760000
[  135.858399] Call Trace:
[  135.858403]  [<ffffffff812bb238>] dump_stack+0x4d/0x65
[  135.858405]  [<ffffffff8104f621>] __warn+0xc1/0xe0
[  135.858406]  [<ffffffff8104f748>] warn_slowpath_null+0x18/0x20
[  135.858408]  [<ffffffff813f8c15>] i915_sw_fence_complete+0x25/0x30
[  135.858410]  [<ffffffff813f8fad>] i915_sw_fence_commit+0xd/0x30
[  135.858412]  [<ffffffff8142e591>] __i915_gem_request_submit+0xe1/0xf0
[  135.858413]  [<ffffffff8142e5c8>] i915_gem_request_submit+0x28/0x40
[  135.858415]  [<ffffffff814433e7>] i915_guc_submit+0x47/0x210
[  135.858417]  [<ffffffff81443e98>] i915_guc_submission_enable+0x468/0x540
[  135.858419]  [<ffffffff81442495>] intel_guc_setup+0x715/0x810
[  135.858421]  [<ffffffff8142b6b4>] i915_gem_init_hw+0x114/0x2a0
[  135.858423]  [<ffffffff813eeaa8>] i915_reset+0xe8/0x120
[  135.858424]  [<ffffffff813f3937>] i915_reset_and_wakeup+0x157/0x180
[  135.858426]  [<ffffffff813f79db>] i915_handle_error+0x1ab/0x230
[  135.858428]  [<ffffffff812c760d>] ? scnprintf+0x4d/0x90
[  135.858430]  [<ffffffff81435985>] i915_hangcheck_elapsed+0x275/0x3d0
[  135.858432]  [<ffffffff810668cf>] process_one_work+0x12f/0x410
[  135.858433]  [<ffffffff81066bf3>] worker_thread+0x43/0x4d0
[  135.858435]  [<ffffffff81066bb0>] ? process_one_work+0x410/0x410
[  135.858436]  [<ffffffff81066bb0>] ? process_one_work+0x410/0x410
[  135.858438]  [<ffffffff8106bbb4>] kthread+0xd4/0xf0
[  135.858440]  [<ffffffff8106bae0>] ? kthread_park+0x60/0x60

v2: Only resubmit submitted requests
v3: Don't forget the pending requests have reserved space.

Fixes: d55ac5bf97c6 ("drm/i915: Defer transfer onto execution timeline to actual hw submission")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_guc_submission.c | 37 ++++++++++++++++--------------
 1 file changed, 20 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index b0f858cf2881..58413803ba3c 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -489,12 +489,9 @@ static int guc_ring_doorbell(struct i915_guc_client *gc)
 }
 
 /**
- * i915_guc_submit() - Submit commands through GuC
+ * __i915_guc_submit() - Submit commands through GuC
  * @rq:		request associated with the commands
  *
- * Return:	0 on success, otherwise an errno.
- * 		(Note: nonzero really shouldn't happen!)
- *
  * 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.
@@ -506,7 +503,7 @@ static int guc_ring_doorbell(struct i915_guc_client *gc)
  * 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 drm_i915_gem_request *rq)
 {
 	struct drm_i915_private *dev_priv = rq->i915;
 	struct intel_engine_cs *engine = rq->engine;
@@ -515,17 +512,6 @@ static void i915_guc_submit(struct drm_i915_gem_request *rq)
 	struct i915_guc_client *client = guc->execbuf_client;
 	int b_ret;
 
-	/* We keep the previous context alive until we retire the following
-	 * request. This ensures that any the context object is still pinned
-	 * for any residual writes the HW makes into it on the context switch
-	 * into the next object following the breadcrumb. Otherwise, we may
-	 * retire the context too early.
-	 */
-	rq->previous_context = engine->last_context;
-	engine->last_context = rq->ctx;
-
-	i915_gem_request_submit(rq);
-
 	spin_lock(&client->wq_lock);
 	guc_wq_item_append(client, rq);
 
@@ -545,6 +531,23 @@ static void i915_guc_submit(struct drm_i915_gem_request *rq)
 	spin_unlock(&client->wq_lock);
 }
 
+static void i915_guc_submit(struct drm_i915_gem_request *rq)
+{
+	struct intel_engine_cs *engine = rq->engine;
+
+	/* We keep the previous context alive until we retire the following
+	 * request. This ensures that any the context object is still pinned
+	 * for any residual writes the HW makes into it on the context switch
+	 * into the next object following the breadcrumb. Otherwise, we may
+	 * retire the context too early.
+	 */
+	rq->previous_context = engine->last_context;
+	engine->last_context = rq->ctx;
+
+	i915_gem_request_submit(rq);
+	__i915_guc_submit(rq);
+}
+
 /*
  * Everything below here is concerned with setup & teardown, and is
  * therefore not part of the somewhat time-critical batch-submission
@@ -1443,7 +1446,7 @@ int i915_guc_submission_enable(struct drm_i915_private *dev_priv)
 		/* Replay the current set of previously submitted requests */
 		list_for_each_entry(rq, &engine->timeline->requests, link) {
 			client->wq_rsvd += sizeof(struct guc_wq_item);
-			i915_guc_submit(rq);
+			__i915_guc_submit(rq);
 		}
 	}
 
-- 
2.10.2

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

Re: [PATCH v2 2/6] drm/i915: Trim i915_guc_info() stack usage

[Tvrtko Ursulin]

On 29/11/2016 12:10, Chris Wilson wrote:
> i915_guc_info() (part of debugfs output) tries to avoid holding
> struct_mutex for a long period by copying onto the stack. This causes a
> warning that the stack frame is massive, so stop doing that. We can even
> forgo holding the struct_mutex here as that doesn't serialise the values
> being read (and the lists used exist for the device lifetime).
>
> v2: Skip printing anything if guc->execbuf_client is disabled (avoids
> potential NULL dereference).
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 46 ++++++++++++++++---------------------
>  1 file changed, 20 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 8eb8c29b7492..a3eaf88f4a42 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2434,47 +2434,41 @@ static void i915_guc_client_info(struct seq_file *m,
>  static int i915_guc_info(struct seq_file *m, void *data)
>  {
>  	struct drm_i915_private *dev_priv = node_to_i915(m->private);
> -	struct drm_device *dev = &dev_priv->drm;
> -	struct intel_guc guc;
> -	struct i915_guc_client client = {};
> +	const struct intel_guc *guc = &dev_priv->guc;
>  	struct intel_engine_cs *engine;
>  	enum intel_engine_id id;
> -	u64 total = 0;
> -
> -	if (!HAS_GUC_SCHED(dev_priv))
> -		return 0;
> +	u64 total;
>
> -	if (mutex_lock_interruptible(&dev->struct_mutex))
> +	if (!guc->execbuf_client) {
> +		seq_printf(m, "GuC submission %s\n",
> +			   HAS_GUC_SCHED(dev_priv) ?
> +			   "disabled" :
> +			   "not supported");
>  		return 0;
> -
> -	/* Take a local copy of the GuC data, so we can dump it at leisure */
> -	guc = dev_priv->guc;
> -	if (guc.execbuf_client)
> -		client = *guc.execbuf_client;
> -
> -	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, "\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);
> -	seq_printf(m, "GuC last action status: 0x%x\n", guc.action_status);
> -	seq_printf(m, "GuC last action error code: %d\n", guc.action_err);
> +	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);
> +	seq_printf(m, "GuC last action status: 0x%x\n", guc->action_status);
> +	seq_printf(m, "GuC last action error code: %d\n", guc->action_err);
>
> +	total = 0;
>  	seq_printf(m, "\nGuC submissions:\n");
>  	for_each_engine(engine, dev_priv, id) {
> -		u64 submissions = guc.submissions[id];
> +		u64 submissions = guc->submissions[id];
>  		total += submissions;
>  		seq_printf(m, "\t%-24s: %10llu, last seqno 0x%08x\n",
> -			engine->name, submissions, guc.last_seqno[id]);
> +			engine->name, submissions, guc->last_seqno[id]);
>  	}
>  	seq_printf(m, "\t%s: %llu\n", "Total", total);
>
> -	seq_printf(m, "\nGuC execbuf client @ %p:\n", guc.execbuf_client);
> -	i915_guc_client_info(m, dev_priv, &client);
> +	seq_printf(m, "\nGuC execbuf client @ %p:\n", guc->execbuf_client);
> +	i915_guc_client_info(m, dev_priv, guc->execbuf_client);
>
>  	i915_guc_log_info(m, dev_priv);
>
>

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

✓ Fi.CI.BAT: success for series starting with [v2,1/6] HAX drm/i915: Enable guc submission

[Patchwork]

== Series Details ==

Series: series starting with [v2,1/6] HAX drm/i915: Enable guc submission
URL   : https://patchwork.freedesktop.org/series/16096/
State : success

== Summary ==

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


fi-bdw-5557u     total:245  pass:230  dwarn:0   dfail:0   fail:0   skip:15 
fi-bsw-n3050     total:245  pass:205  dwarn:0   dfail:0   fail:0   skip:40 
fi-bxt-t5700     total:245  pass:217  dwarn:0   dfail:0   fail:0   skip:28 
fi-byt-j1900     total:245  pass:217  dwarn:0   dfail:0   fail:0   skip:28 
fi-byt-n2820     total:245  pass:213  dwarn:0   dfail:0   fail:0   skip:32 
fi-hsw-4770      total:245  pass:225  dwarn:0   dfail:0   fail:0   skip:20 
fi-hsw-4770r     total:245  pass:225  dwarn:0   dfail:0   fail:0   skip:20 
fi-ilk-650       total:245  pass:192  dwarn:0   dfail:0   fail:0   skip:53 
fi-ivb-3520m     total:245  pass:223  dwarn:0   dfail:0   fail:0   skip:22 
fi-ivb-3770      total:245  pass:223  dwarn:0   dfail:0   fail:0   skip:22 
fi-kbl-7500u     total:245  pass:223  dwarn:0   dfail:0   fail:0   skip:22 
fi-skl-6260u     total:245  pass:231  dwarn:0   dfail:0   fail:0   skip:14 
fi-skl-6700hq    total:245  pass:224  dwarn:0   dfail:0   fail:0   skip:21 
fi-skl-6700k     total:245  pass:223  dwarn:1   dfail:0   fail:0   skip:21 
fi-skl-6770hq    total:245  pass:231  dwarn:0   dfail:0   fail:0   skip:14 
fi-snb-2520m     total:245  pass:213  dwarn:0   dfail:0   fail:0   skip:32 
fi-snb-2600      total:245  pass:212  dwarn:0   dfail:0   fail:0   skip:33 

676d4ee0b28bd4c5f9b7b3aee61b043c98ab9cc5 drm-tip: 2016y-11m-29d-12h-54m-36s UTC integration manifest
f7b42f2 drm/i915/guc: Split hw submission for replay after GPU reset
481ee6d drm/i915/guc: Keep the execbuf client allocated across reset
03cbf14 drm/i915/guc: Initialise doorbell cookie to matching value
073b2e9 drm/i915/guc: Rename client->cookie to match use
6c97d92 drm/i915: Trim i915_guc_info() stack usage
2771dc9 HAX drm/i915: Enable guc submission

== Logs ==

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

Re: ✓ Fi.CI.BAT: success for series starting with [v2,1/6] HAX drm/i915: Enable guc submission

[Chris Wilson]

On Tue, Nov 29, 2016 at 03:45:23PM -0000, Patchwork wrote:
> == Series Details ==
> 
> Series: series starting with [v2,1/6] HAX drm/i915: Enable guc submission
> URL   : https://patchwork.freedesktop.org/series/16096/
> State : success
> 
> == Summary ==
> 
> Series 16096v1 Series without cover letter
> https://patchwork.freedesktop.org/api/1.0/series/16096/revisions/1/mbox/
> 
> 
> fi-bdw-5557u     total:245  pass:230  dwarn:0   dfail:0   fail:0   skip:15 
> fi-bsw-n3050     total:245  pass:205  dwarn:0   dfail:0   fail:0   skip:40 
> fi-bxt-t5700     total:245  pass:217  dwarn:0   dfail:0   fail:0   skip:28 
> fi-byt-j1900     total:245  pass:217  dwarn:0   dfail:0   fail:0   skip:28 
> fi-byt-n2820     total:245  pass:213  dwarn:0   dfail:0   fail:0   skip:32 
> fi-hsw-4770      total:245  pass:225  dwarn:0   dfail:0   fail:0   skip:20 
> fi-hsw-4770r     total:245  pass:225  dwarn:0   dfail:0   fail:0   skip:20 
> fi-ilk-650       total:245  pass:192  dwarn:0   dfail:0   fail:0   skip:53 
> fi-ivb-3520m     total:245  pass:223  dwarn:0   dfail:0   fail:0   skip:22 
> fi-ivb-3770      total:245  pass:223  dwarn:0   dfail:0   fail:0   skip:22 
> fi-kbl-7500u     total:245  pass:223  dwarn:0   dfail:0   fail:0   skip:22 
> fi-skl-6260u     total:245  pass:231  dwarn:0   dfail:0   fail:0   skip:14 
> fi-skl-6700hq    total:245  pass:224  dwarn:0   dfail:0   fail:0   skip:21 
> fi-skl-6700k     total:245  pass:223  dwarn:1   dfail:0   fail:0   skip:21 
> fi-skl-6770hq    total:245  pass:231  dwarn:0   dfail:0   fail:0   skip:14 
> fi-snb-2520m     total:245  pass:213  dwarn:0   dfail:0   fail:0   skip:32 
> fi-snb-2600      total:245  pass:212  dwarn:0   dfail:0   fail:0   skip:33 
> 
> 676d4ee0b28bd4c5f9b7b3aee61b043c98ab9cc5 drm-tip: 2016y-11m-29d-12h-54m-36s UTC integration manifest
> f7b42f2 drm/i915/guc: Split hw submission for replay after GPU reset
> 481ee6d drm/i915/guc: Keep the execbuf client allocated across reset
> 03cbf14 drm/i915/guc: Initialise doorbell cookie to matching value
> 073b2e9 drm/i915/guc: Rename client->cookie to match use
> 6c97d92 drm/i915: Trim i915_guc_info() stack usage
> 2771dc9 HAX drm/i915: Enable guc submission

Pushed the fixes for reset with GuC submission enabled (but not the
HAX!), thanks for the review.
-Chris

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