public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH 0/3] GuC based reset engine
@ 2017-10-30 18:56 Michel Thierry
  2017-10-30 18:56 ` [PATCH 1/3] drm/i915/guc: Rename the function that resets the GuC Michel Thierry
                   ` (6 more replies)
  0 siblings, 7 replies; 25+ messages in thread
From: Michel Thierry @ 2017-10-30 18:56 UTC (permalink / raw)
  To: intel-gfx

We've been supporting reset-engine in execlist submission mode for a
while, but with GuC, the resubmission path had to be different because we
used to re-enable the engines before GuC... so we've been using full gpu
reset when GuC submission is enabled (which reset the fw).

Thanks to Michal Winiarski GuC preemption changes, the firmware is now
enabled before restarting the engines, so the replay part after
resetting an engine is no longer different.

The only change now is that the reset-engine is requested to the
firmware via a H2G command, and GuC is the one in charge of acquiring the
forcewake and idling the engine before reseting it.

(The first patch is a cosmetic change, which I'm sure Tvrtko doesn't
remember he reviewed [1]).

[1] https://lists.freedesktop.org/archives/intel-gfx/2017-April/126813.html

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>

Michal Wajdeczko (1):
  HAX enable GuC submission for CI

Michel Thierry (2):
  drm/i915/guc: Rename the function that resets the GuC
  drm/i915/guc: Add support for reset engine using GuC commands

 drivers/gpu/drm/i915/i915_drv.c       |  9 +++++++--
 drivers/gpu/drm/i915/i915_drv.h       |  3 ++-
 drivers/gpu/drm/i915/i915_gem_gtt.c   |  8 ++------
 drivers/gpu/drm/i915/i915_params.h    |  4 ++--
 drivers/gpu/drm/i915/intel_guc.c      | 24 ++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_guc_fwif.h |  1 +
 drivers/gpu/drm/i915/intel_uc.c       |  4 ++--
 drivers/gpu/drm/i915/intel_uncore.c   |  7 +------
 8 files changed, 41 insertions(+), 19 deletions(-)

-- 
2.14.2

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

^ permalink raw reply	[flat|nested] 25+ messages in thread
* [PATCH v4 1/3] drm/i915/selftests: Add a GuC doorbells selftest
@ 2017-11-15 18:30 Michel Thierry
  2017-11-15 18:30 ` [PATCH 3/3] HAX enable GuC submission for CI Michel Thierry
  0 siblings, 1 reply; 25+ messages in thread
From: Michel Thierry @ 2017-11-15 18:30 UTC (permalink / raw)
  To: intel-gfx

The first test aims to check guc_init_doorbell_hw, changing the existing
guc clients and doorbells state before calling it.

The second test tries to create as many clients as it is currently possible
(currently limited to max number of doorbells) and exercise the doorbell
alloc/dealloc code.

Since our usage mode require very few clients/doorbells, this code has
been exercised very lightly and it's good to have a simple test for it.

As reference, this test already helped identify the bug fixed by
commit 7f1ea2ac3017 ("drm/i915/guc: Fix doorbell id selection").

v2: Extend number of clients; check for client allocation failure when
number of doorbells is exceeded; validate client properties; reuse
guc_init_doorbell_hw (Chris).

v3: guc_init_doorbell_hw test added per Chris suggestion.

v4: Try to explain why guc_init_doorbell_hw exist and comment some
details in the subtest.

Signed-off-by: Michel Thierry <michel.thierry@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_guc_submission.c         |   4 +
 .../gpu/drm/i915/selftests/i915_live_selftests.h   |   1 +
 drivers/gpu/drm/i915/selftests/intel_guc.c         | 362 +++++++++++++++++++++
 3 files changed, 367 insertions(+)
 create mode 100644 drivers/gpu/drm/i915/selftests/intel_guc.c

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 0ba2fc04fe9c..5d6576e01a91 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -1464,3 +1464,7 @@ void i915_guc_submission_disable(struct drm_i915_private *dev_priv)
 
 	guc_clients_destroy(guc);
 }
+
+#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
+#include "selftests/intel_guc.c"
+#endif
diff --git a/drivers/gpu/drm/i915/selftests/i915_live_selftests.h b/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
index d7dd98a6acad..088f45bc6199 100644
--- a/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
+++ b/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
@@ -20,3 +20,4 @@ selftest(evict, i915_gem_evict_live_selftests)
 selftest(hugepages, i915_gem_huge_page_live_selftests)
 selftest(contexts, i915_gem_context_live_selftests)
 selftest(hangcheck, intel_hangcheck_live_selftests)
+selftest(guc, intel_guc_live_selftest)
diff --git a/drivers/gpu/drm/i915/selftests/intel_guc.c b/drivers/gpu/drm/i915/selftests/intel_guc.c
new file mode 100644
index 000000000000..67723a4c82a3
--- /dev/null
+++ b/drivers/gpu/drm/i915/selftests/intel_guc.c
@@ -0,0 +1,362 @@
+/*
+ * Copyright © 2017 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#include "../i915_selftest.h"
+
+/* max doorbell number + negative test for each client type */
+#define ATTEMPTS (GUC_NUM_DOORBELLS + GUC_CLIENT_PRIORITY_NUM)
+
+struct i915_guc_client *clients[ATTEMPTS];
+
+static bool available_dbs(struct intel_guc *guc, u32 priority)
+{
+	unsigned long offset;
+	unsigned long end;
+	u16 id;
+
+	/* first half is used for normal priority, second half for high */
+	offset = 0;
+	end = GUC_NUM_DOORBELLS/2;
+	if (priority <= GUC_CLIENT_PRIORITY_HIGH) {
+		offset = end;
+		end += offset;
+	}
+
+	id = find_next_zero_bit(guc->doorbell_bitmap, end, offset);
+	if (id < end)
+		return true;
+
+	return false;
+}
+
+static int check_all_doorbells(struct intel_guc *guc)
+{
+	u16 db_id;
+
+	pr_info_once("Max number of doorbells: %d", GUC_NUM_DOORBELLS);
+	for (db_id = 0; db_id < GUC_NUM_DOORBELLS; ++db_id) {
+		if (!doorbell_ok(guc, db_id)) {
+			pr_err("doorbell %d, not ok\n", db_id);
+			return -EIO;
+		}
+	}
+
+	return 0;
+}
+
+/*
+ * Basic client sanity check, handy to validate create_clients.
+ */
+static int validate_client(struct i915_guc_client *client,
+			   int client_priority,
+			   bool is_preempt_client)
+{
+	struct drm_i915_private *dev_priv = guc_to_i915(client->guc);
+	struct i915_gem_context *ctx_owner = is_preempt_client ?
+			dev_priv->preempt_context : dev_priv->kernel_context;
+
+	if (client->owner != ctx_owner ||
+	    client->engines != INTEL_INFO(dev_priv)->ring_mask ||
+	    client->priority != client_priority ||
+	    client->doorbell_id == GUC_DOORBELL_INVALID)
+		return -EINVAL;
+	else
+		return 0;
+}
+
+/*
+ * Check that guc_init_doorbell_hw is doing what it should.
+ *
+ * During GuC submission enable, we create GuC clients and their doorbells,
+ * but after resetting the microcontroller (resume & gpu reset), these
+ * GuC clients are still around, but the status of their doorbells may be
+ * incorrect. This is the reason behind validating that the doorbells status
+ * expected by the driver matches what the GuC/HW have.
+ */
+static int igt_guc_init_doorbell_hw(void *args)
+{
+	struct drm_i915_private *dev_priv = args;
+	struct intel_guc *guc;
+	DECLARE_BITMAP(db_bitmap_bk, GUC_NUM_DOORBELLS);
+	int i, err = 0;
+
+	pr_info("GuC init_doorbell_hw selftest\n");
+	GEM_BUG_ON(!HAS_GUC(dev_priv));
+	mutex_lock(&dev_priv->drm.struct_mutex);
+
+	guc = &dev_priv->guc;
+	if (!guc) {
+		pr_err("No guc object!\n");
+		err = -EINVAL;
+		goto unlock;
+	}
+
+	err = check_all_doorbells(guc);
+	if (err)
+		goto unlock;
+
+	/* Get rid of clients created during driver load because the test will
+	 * recreate them.
+	 */
+	guc_clients_destroy(guc);
+	if (guc->execbuf_client || guc->preempt_client) {
+		pr_err("guc_clients_destroy lied!\n");
+		err = -EINVAL;
+		goto unlock;
+	}
+
+	err = guc_clients_create(guc);
+	if (err) {
+		pr_err("Failed to create clients\n");
+		goto unlock;
+	}
+
+	err = validate_client(guc->execbuf_client,
+			      GUC_CLIENT_PRIORITY_KMD_NORMAL, false);
+	if (err) {
+		pr_err("execbug client validation failed\n");
+		goto out;
+	}
+
+	err = validate_client(guc->preempt_client,
+			      GUC_CLIENT_PRIORITY_KMD_HIGH, true);
+	if (err) {
+		pr_err("preempt client validation failed\n");
+		goto out;
+	}
+
+	/* each client should have received a doorbell during alloc */
+	if (!has_doorbell(guc->execbuf_client) ||
+	    !has_doorbell(guc->preempt_client)) {
+		pr_err("guc_clients_create didn't create doorbells\n");
+		err = -EINVAL;
+		goto out;
+	}
+
+	/* Basic test - an attempt to reallocate a valid doorbell to the
+	 * client it is currently assigned should not cause a failure.
+	 */
+	err = guc_init_doorbell_hw(guc);
+	if (err)
+		goto out;
+
+	/* Negative test - a client with no doorbell (invalid db id).
+	 * Each client gets a doorbell when it is created, after destroying
+	 * the doorbell, the db id is changed to GUC_DOORBELL_INVALID and the
+	 * firmware will reject any attempt to allocate a doorbell with an
+	 * invalid id (db has to be reserved before allocation).
+	 */
+	destroy_doorbell(guc->execbuf_client);
+	if (has_doorbell(guc->execbuf_client)) {
+		pr_err("destroy db did not work\n");
+		err = -EINVAL;
+		goto out;
+	}
+
+	err = guc_init_doorbell_hw(guc);
+	if (err != -EIO) {
+		pr_err("unexpected (err = %d)", err);
+		goto out;
+	}
+
+	if (!available_dbs(guc, guc->execbuf_client->priority)) {
+		pr_err("doorbell not available when it should\n");
+		err = -EIO;
+		goto out;
+	}
+
+	/* clean after test */
+	err = create_doorbell(guc->execbuf_client);
+	if (err) {
+		pr_err("recreate doorbell failed\n");
+		goto out;
+	}
+
+	/* Negative test - doorbell_bitmap out of sync, will trigger a few of
+	 * WARN_ON(!doorbell_ok(guc, db_id)) but that's ok as long as the
+	 * doorbells from our clients don't fail.
+	 */
+	bitmap_copy(db_bitmap_bk, guc->doorbell_bitmap, GUC_NUM_DOORBELLS);
+	for (i = 0; i < GUC_NUM_DOORBELLS; i++)
+		if (i % 2)
+			test_and_change_bit(i, guc->doorbell_bitmap);
+
+	err = guc_init_doorbell_hw(guc);
+	if (err) {
+		pr_err("out of sync doorbell caused an error\n");
+		goto out;
+	}
+
+	/* restore 'correct' db bitmap */
+	bitmap_copy(guc->doorbell_bitmap, db_bitmap_bk, GUC_NUM_DOORBELLS);
+	err = guc_init_doorbell_hw(guc);
+	if (err) {
+		pr_err("restored doorbell caused an error\n");
+		goto out;
+	}
+
+out:
+	/* leave clean state for other test, plus the driver always destroy the
+	 * clients during unload.
+	 */
+	guc_clients_destroy(guc);
+	guc_clients_create(guc);
+unlock:
+	mutex_unlock(&dev_priv->drm.struct_mutex);
+	return err;
+}
+
+/*
+ * Create as many clients as number of doorbells. Note that there's already
+ * client(s)/doorbell(s) created during driver load, but this test creates
+ * its own and do not interact with the existing ones.
+ */
+static int igt_guc_doorbells(void *arg)
+{
+	struct drm_i915_private *dev_priv = arg;
+	struct intel_guc *guc;
+	int i, err = 0;
+	u16 db_id;
+
+	pr_info("GuC Doorbells selftest\n");
+	GEM_BUG_ON(!HAS_GUC(dev_priv));
+	mutex_lock(&dev_priv->drm.struct_mutex);
+
+	guc = &dev_priv->guc;
+	if (!guc) {
+		pr_err("No guc object!\n");
+		err = -EINVAL;
+		goto unlock;
+	}
+
+	err = check_all_doorbells(guc);
+	if (err)
+		goto unlock;
+
+	for (i = 0; i < ATTEMPTS; i++) {
+		clients[i] = guc_client_alloc(dev_priv,
+					      INTEL_INFO(dev_priv)->ring_mask,
+					      i % GUC_CLIENT_PRIORITY_NUM,
+					      dev_priv->kernel_context);
+
+		if (!clients[i]) {
+			pr_err("[%d] No guc client\n", i);
+			err = -EINVAL;
+			goto out;
+		}
+
+		if (IS_ERR(clients[i])) {
+			if (PTR_ERR(clients[i]) != -ENOSPC) {
+				pr_err("[%d] unexpected error\n", i);
+				err = PTR_ERR(clients[i]);
+				goto out;
+			}
+
+			if (available_dbs(guc, i % GUC_CLIENT_PRIORITY_NUM)) {
+				pr_err("[%d] non-db related alloc fail\n", i);
+				err = -EINVAL;
+				goto out;
+			}
+
+			/* expected, ran out of dbs for this client type */
+			continue;
+		}
+
+		/* The check below is only valid because we keep a doorbell
+		 * assigned during the whole life of the client.
+		 */
+		if (clients[i]->stage_id >= GUC_NUM_DOORBELLS) {
+			pr_err("[%d] more clients than doorbells (%d >= %d)\n",
+			       i, clients[i]->stage_id, GUC_NUM_DOORBELLS);
+			err = -EINVAL;
+			goto out;
+		}
+
+		err = validate_client(clients[i],
+				      i % GUC_CLIENT_PRIORITY_NUM, false);
+		if (err) {
+			pr_err("[%d] client_alloc sanity check failed!\n", i);
+			err = -EINVAL;
+			goto out;
+		}
+
+		db_id = clients[i]->doorbell_id;
+
+		/* Client alloc gives us a doorbell, but we want to exercise
+		 * this ourselves (this resembles guc_init_doorbell_hw)
+		 */
+		destroy_doorbell(clients[i]);
+		if (clients[i]->doorbell_id != GUC_DOORBELL_INVALID) {
+			pr_err("[%d] destroy db did not work!\n", i);
+			err = -EINVAL;
+			goto out;
+		}
+
+		err = __reserve_doorbell(clients[i]);
+		if (err) {
+			pr_err("[%d] Failed to reserve a doorbell\n", i);
+			goto out;
+		}
+
+		__update_doorbell_desc(clients[i], clients[i]->doorbell_id);
+		err = __create_doorbell(clients[i]);
+		if (err) {
+			pr_err("[%d] Failed to create a doorbell\n", i);
+			goto out;
+		}
+
+		/* doorbell id shouldn't change, we are holding the mutex */
+		if (db_id != clients[i]->doorbell_id) {
+			pr_err("[%d] doorbell id changed (%d != %d)\n",
+			       i, db_id, clients[i]->doorbell_id);
+			err = -EINVAL;
+			goto out;
+		}
+
+		err = check_all_doorbells(guc);
+		if (err)
+			goto out;
+	}
+
+out:
+	for (i = 0; i < ATTEMPTS; i++)
+		if (!IS_ERR_OR_NULL(clients[i]))
+			guc_client_free(clients[i]);
+unlock:
+	mutex_unlock(&dev_priv->drm.struct_mutex);
+	return err;
+}
+
+int intel_guc_live_selftest(struct drm_i915_private *dev_priv)
+{
+	static const struct i915_subtest tests[] = {
+		SUBTEST(igt_guc_init_doorbell_hw),
+		SUBTEST(igt_guc_doorbells),
+	};
+
+	if (!i915_modparams.enable_guc_submission)
+		return 0;
+
+	return i915_subtests(tests, dev_priv);
+}
-- 
2.15.0

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

^ permalink raw reply related	[flat|nested] 25+ messages in thread
* [PATCH 1/3] drm/i915: Add a hook for making the engines idle (parking) and unparking
@ 2017-10-20 21:06 Chris Wilson
  2017-10-20 21:06 ` [PATCH 3/3] HAX Enable GuC Submission for CI Chris Wilson
  0 siblings, 1 reply; 25+ messages in thread
From: Chris Wilson @ 2017-10-20 21:06 UTC (permalink / raw)
  To: intel-gfx

In the next patch, we will want to install a callback when the engines
(GT as a whole) become idle and similarly when they first become busy.
To enable that callback, first rename intel_engines_mark_idle() to the
intel_engines_park() and provide the companion intel_engines_unpark().

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Michał Winiarski <michal.winiarski@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c         |  2 +-
 drivers/gpu/drm/i915/i915_gem_request.c |  2 ++
 drivers/gpu/drm/i915/intel_engine_cs.c  | 33 +++++++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/intel_lrc.c        |  3 +++
 drivers/gpu/drm/i915/intel_ringbuffer.c |  5 ++++-
 drivers/gpu/drm/i915/intel_ringbuffer.h |  6 +++++-
 6 files changed, 46 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 026cb52ece0b..bf350861acf7 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3319,7 +3319,7 @@ i915_gem_idle_work_handler(struct work_struct *work)
 	if (wait_for(intel_engines_are_idle(dev_priv), 10))
 		DRM_ERROR("Timeout waiting for engines to idle\n");
 
-	intel_engines_mark_idle(dev_priv);
+	intel_engines_park(dev_priv);
 	i915_gem_timelines_mark_idle(dev_priv);
 
 	GEM_BUG_ON(!dev_priv->gt.awake);
diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
index d140fcf5c6a3..e0d6221022a8 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/i915_gem_request.c
@@ -259,6 +259,8 @@ static void mark_busy(struct drm_i915_private *i915)
 	if (INTEL_GEN(i915) >= 6)
 		gen6_rps_busy(i915);
 
+	intel_engines_unpark(i915);
+
 	queue_delayed_work(i915->wq,
 			   &i915->gt.retire_work,
 			   round_jiffies_up_relative(HZ));
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index a47a9c6bea52..0213edc237fe 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -1594,19 +1594,48 @@ void intel_engines_reset_default_submission(struct drm_i915_private *i915)
 		engine->set_default_submission(engine);
 }
 
-void intel_engines_mark_idle(struct drm_i915_private *i915)
+/**
+ * intel_engines_park: called when the GT is transitioning from busy->idle
+ * @i915: the i915 device
+ *
+ * The GT is now idle and about to go to sleep (maybe never to wake again?).
+ * Time for us to tidy and put away our toys (release resources back to the
+ * system).
+ */
+void intel_engines_park(struct drm_i915_private *i915)
 {
 	struct intel_engine_cs *engine;
 	enum intel_engine_id id;
 
 	for_each_engine(engine, i915, id) {
+		if (engine->park)
+			engine->park(engine);
+
 		intel_engine_disarm_breadcrumbs(engine);
-		i915_gem_batch_pool_fini(&engine->batch_pool);
 		tasklet_kill(&engine->execlists.irq_tasklet);
+
+		i915_gem_batch_pool_fini(&engine->batch_pool);
 		engine->execlists.no_priolist = false;
 	}
 }
 
+/**
+ * intel_engines_unpark: called when the GT is transitioning from idle->busy
+ * @i915: the i915 device
+ *
+ * The GT was idle and now about to fire up with some new user requests.
+ */
+void intel_engines_unpark(struct drm_i915_private *i915)
+{
+	struct intel_engine_cs *engine;
+	enum intel_engine_id id;
+
+	for_each_engine(engine, i915, id) {
+		if (engine->unpark)
+			engine->unpark(engine);
+	}
+}
+
 bool intel_engine_can_store_dword(struct intel_engine_cs *engine)
 {
 	switch (INTEL_GEN(engine->i915)) {
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 7f45dd7dc3e5..a030ca44a7ae 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1880,6 +1880,9 @@ static void execlists_set_default_submission(struct intel_engine_cs *engine)
 	engine->cancel_requests = execlists_cancel_requests;
 	engine->schedule = execlists_schedule;
 	engine->execlists.irq_tasklet.func = intel_lrc_irq_handler;
+
+	engine->park = NULL;
+	engine->unpark = NULL;
 }
 
 static void
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 8da1bde442dd..05e01446b00b 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2028,12 +2028,15 @@ static void i9xx_set_default_submission(struct intel_engine_cs *engine)
 {
 	engine->submit_request = i9xx_submit_request;
 	engine->cancel_requests = cancel_requests;
+
+	engine->park = NULL;
+	engine->unpark = NULL;
 }
 
 static void gen6_bsd_set_default_submission(struct intel_engine_cs *engine)
 {
+	i9xx_set_default_submission(engine);
 	engine->submit_request = gen6_bsd_submit_request;
-	engine->cancel_requests = cancel_requests;
 }
 
 static void intel_ring_default_vfuncs(struct drm_i915_private *dev_priv,
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 17186f067408..240814b60a65 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -357,6 +357,9 @@ struct intel_engine_cs {
 	void		(*reset_hw)(struct intel_engine_cs *engine,
 				    struct drm_i915_gem_request *req);
 
+	void		(*park)(struct intel_engine_cs *engine);
+	void		(*unpark)(struct intel_engine_cs *engine);
+
 	void		(*set_default_submission)(struct intel_engine_cs *engine);
 
 	struct intel_ring *(*context_pin)(struct intel_engine_cs *engine,
@@ -836,7 +839,8 @@ static inline u32 *gen8_emit_pipe_control(u32 *batch, u32 flags, u32 offset)
 bool intel_engine_is_idle(struct intel_engine_cs *engine);
 bool intel_engines_are_idle(struct drm_i915_private *dev_priv);
 
-void intel_engines_mark_idle(struct drm_i915_private *i915);
+void intel_engines_park(struct drm_i915_private *i915);
+void intel_engines_unpark(struct drm_i915_private *i915);
 void intel_engines_reset_default_submission(struct drm_i915_private *i915);
 
 bool intel_engine_can_store_dword(struct intel_engine_cs *engine);
-- 
2.15.0.rc1

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

^ permalink raw reply related	[flat|nested] 25+ messages in thread
* [PATCH 1/3] drm/i915: Invalidate the guc ggtt TLB upon insertion
@ 2017-01-11 13:13 Chris Wilson
  2017-01-11 13:13 ` [PATCH 3/3] HAX enable guc submission for CI Chris Wilson
  0 siblings, 1 reply; 25+ messages in thread
From: Chris Wilson @ 2017-01-11 13:13 UTC (permalink / raw)
  To: intel-gfx; +Cc: Daniel Vetter

Move the GuC invalidation of its ggtt TLB to where we perform the ggtt
modification rather than proliferate it into all the callers of the
insert (which may or may not in fact have to do the insertion).

v2: Just do the guc invalidate unconditionally, (afaict) it has no impact
without the guc loaded on gen8+
v3: Conditionally invalidate the guc - just in case that register has
not been validated for other modes.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c        | 78 +++++++++++++++++++-----------
 drivers/gpu/drm/i915/i915_gem_gtt.h        |  3 ++
 drivers/gpu/drm/i915/i915_guc_submission.c |  3 --
 drivers/gpu/drm/i915/intel_guc_loader.c    |  7 +--
 drivers/gpu/drm/i915/intel_lrc.c           |  6 ---
 5 files changed, 57 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 0ed99adfd0da..ed120a1e7f93 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -110,6 +110,30 @@ const struct i915_ggtt_view i915_ggtt_view_rotated = {
 	.type = I915_GGTT_VIEW_ROTATED,
 };
 
+static void gen6_ggtt_invalidate(struct drm_i915_private *dev_priv)
+{
+	/* Note that as an uncached mmio write, this should flush the
+	 * WCB of the writes into the GGTT before it triggers the invalidate.
+	 */
+	I915_WRITE(GFX_FLSH_CNTL_GEN6, GFX_FLSH_CNTL_EN);
+}
+
+static void guc_ggtt_invalidate(struct drm_i915_private *dev_priv)
+{
+	gen6_ggtt_invalidate(dev_priv);
+	I915_WRITE(GEN8_GTCR, GEN8_GTCR_INVALIDATE);
+}
+
+static void gmch_ggtt_invalidate(struct drm_i915_private *dev_priv)
+{
+	intel_gtt_chipset_flush();
+}
+
+static inline void i915_ggtt_invalidate(struct drm_i915_private *i915)
+{
+	i915->ggtt.invalidate(i915);
+}
+
 int intel_sanitize_enable_ppgtt(struct drm_i915_private *dev_priv,
 			       	int enable_ppgtt)
 {
@@ -2307,16 +2331,6 @@ void i915_check_and_clear_faults(struct drm_i915_private *dev_priv)
 		POSTING_READ(RING_FAULT_REG(dev_priv->engine[RCS]));
 }
 
-static void i915_ggtt_flush(struct drm_i915_private *dev_priv)
-{
-	if (INTEL_INFO(dev_priv)->gen < 6) {
-		intel_gtt_chipset_flush();
-	} else {
-		I915_WRITE(GFX_FLSH_CNTL_GEN6, GFX_FLSH_CNTL_EN);
-		POSTING_READ(GFX_FLSH_CNTL_GEN6);
-	}
-}
-
 void i915_gem_suspend_gtt_mappings(struct drm_i915_private *dev_priv)
 {
 	struct i915_ggtt *ggtt = &dev_priv->ggtt;
@@ -2331,7 +2345,7 @@ void i915_gem_suspend_gtt_mappings(struct drm_i915_private *dev_priv)
 
 	ggtt->base.clear_range(&ggtt->base, ggtt->base.start, ggtt->base.total);
 
-	i915_ggtt_flush(dev_priv);
+	i915_ggtt_invalidate(dev_priv);
 }
 
 int i915_gem_gtt_prepare_pages(struct drm_i915_gem_object *obj,
@@ -2370,15 +2384,13 @@ static void gen8_ggtt_insert_page(struct i915_address_space *vm,
 				  enum i915_cache_level level,
 				  u32 unused)
 {
-	struct drm_i915_private *dev_priv = vm->i915;
+	struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
 	gen8_pte_t __iomem *pte =
-		(gen8_pte_t __iomem *)dev_priv->ggtt.gsm +
-		(offset >> PAGE_SHIFT);
+		(gen8_pte_t __iomem *)ggtt->gsm + (offset >> PAGE_SHIFT);
 
 	gen8_set_pte(pte, gen8_pte_encode(addr, level));
 
-	I915_WRITE(GFX_FLSH_CNTL_GEN6, GFX_FLSH_CNTL_EN);
-	POSTING_READ(GFX_FLSH_CNTL_GEN6);
+	ggtt->invalidate(vm->i915);
 }
 
 static void gen8_ggtt_insert_entries(struct i915_address_space *vm,
@@ -2386,7 +2398,6 @@ static void gen8_ggtt_insert_entries(struct i915_address_space *vm,
 				     uint64_t start,
 				     enum i915_cache_level level, u32 unused)
 {
-	struct drm_i915_private *dev_priv = vm->i915;
 	struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
 	struct sgt_iter sgt_iter;
 	gen8_pte_t __iomem *gtt_entries;
@@ -2415,8 +2426,7 @@ static void gen8_ggtt_insert_entries(struct i915_address_space *vm,
 	 * want to flush the TLBs only after we're certain all the PTE updates
 	 * have finished.
 	 */
-	I915_WRITE(GFX_FLSH_CNTL_GEN6, GFX_FLSH_CNTL_EN);
-	POSTING_READ(GFX_FLSH_CNTL_GEN6);
+	ggtt->invalidate(vm->i915);
 }
 
 struct insert_entries {
@@ -2451,15 +2461,13 @@ static void gen6_ggtt_insert_page(struct i915_address_space *vm,
 				  enum i915_cache_level level,
 				  u32 flags)
 {
-	struct drm_i915_private *dev_priv = vm->i915;
+	struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
 	gen6_pte_t __iomem *pte =
-		(gen6_pte_t __iomem *)dev_priv->ggtt.gsm +
-		(offset >> PAGE_SHIFT);
+		(gen6_pte_t __iomem *)ggtt->gsm + (offset >> PAGE_SHIFT);
 
 	iowrite32(vm->pte_encode(addr, level, flags), pte);
 
-	I915_WRITE(GFX_FLSH_CNTL_GEN6, GFX_FLSH_CNTL_EN);
-	POSTING_READ(GFX_FLSH_CNTL_GEN6);
+	ggtt->invalidate(vm->i915);
 }
 
 /*
@@ -2473,7 +2481,6 @@ static void gen6_ggtt_insert_entries(struct i915_address_space *vm,
 				     uint64_t start,
 				     enum i915_cache_level level, u32 flags)
 {
-	struct drm_i915_private *dev_priv = vm->i915;
 	struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
 	struct sgt_iter sgt_iter;
 	gen6_pte_t __iomem *gtt_entries;
@@ -2501,8 +2508,7 @@ static void gen6_ggtt_insert_entries(struct i915_address_space *vm,
 	 * want to flush the TLBs only after we're certain all the PTE updates
 	 * have finished.
 	 */
-	I915_WRITE(GFX_FLSH_CNTL_GEN6, GFX_FLSH_CNTL_EN);
-	POSTING_READ(GFX_FLSH_CNTL_GEN6);
+	ggtt->invalidate(vm->i915);
 }
 
 static void nop_clear_range(struct i915_address_space *vm,
@@ -3062,6 +3068,8 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt)
 	if (IS_CHERRYVIEW(dev_priv))
 		ggtt->base.insert_entries = gen8_ggtt_insert_entries__BKL;
 
+	ggtt->invalidate = gen6_ggtt_invalidate;
+
 	return ggtt_probe_common(ggtt, size);
 }
 
@@ -3099,6 +3107,8 @@ static int gen6_gmch_probe(struct i915_ggtt *ggtt)
 	ggtt->base.unbind_vma = ggtt_unbind_vma;
 	ggtt->base.cleanup = gen6_gmch_remove;
 
+	ggtt->invalidate = gen6_ggtt_invalidate;
+
 	if (HAS_EDRAM(dev_priv))
 		ggtt->base.pte_encode = iris_pte_encode;
 	else if (IS_HASWELL(dev_priv))
@@ -3142,6 +3152,8 @@ static int i915_gmch_probe(struct i915_ggtt *ggtt)
 	ggtt->base.unbind_vma = ggtt_unbind_vma;
 	ggtt->base.cleanup = i915_gmch_remove;
 
+	ggtt->invalidate = gmch_ggtt_invalidate;
+
 	if (unlikely(ggtt->do_idle_maps))
 		DRM_INFO("applying Ironlake quirks for intel_iommu\n");
 
@@ -3260,6 +3272,16 @@ int i915_ggtt_enable_hw(struct drm_i915_private *dev_priv)
 	return 0;
 }
 
+void i915_ggtt_enable_guc(struct drm_i915_private *i915)
+{
+	i915->ggtt.invalidate = guc_ggtt_invalidate;
+}
+
+void i915_ggtt_disable_guc(struct drm_i915_private *i915)
+{
+	i915->ggtt.invalidate = gen6_ggtt_invalidate;
+}
+
 void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv)
 {
 	struct i915_ggtt *ggtt = &dev_priv->ggtt;
@@ -3323,7 +3345,7 @@ void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv)
 		}
 	}
 
-	i915_ggtt_flush(dev_priv);
+	i915_ggtt_invalidate(dev_priv);
 }
 
 struct i915_vma *
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 3e031a057f78..6c40088f8cf4 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -336,6 +336,7 @@ struct i915_ggtt {
 
 	/** "Graphics Stolen Memory" holds the global PTEs */
 	void __iomem *gsm;
+	void (*invalidate)(struct drm_i915_private *dev_priv);
 
 	bool do_idle_maps;
 
@@ -504,6 +505,8 @@ i915_vm_to_ggtt(struct i915_address_space *vm)
 int i915_ggtt_probe_hw(struct drm_i915_private *dev_priv);
 int i915_ggtt_init_hw(struct drm_i915_private *dev_priv);
 int i915_ggtt_enable_hw(struct drm_i915_private *dev_priv);
+void i915_ggtt_enable_guc(struct drm_i915_private *i915);
+void i915_ggtt_disable_guc(struct drm_i915_private *i915);
 int i915_gem_init_ggtt(struct drm_i915_private *dev_priv);
 void i915_ggtt_cleanup_hw(struct drm_i915_private *dev_priv);
 
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 710fbb9fc63f..913d87358972 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -579,9 +579,6 @@ static struct i915_vma *guc_allocate_vma(struct intel_guc *guc, u32 size)
 		goto err;
 	}
 
-	/* Invalidate GuC TLB to let GuC take the latest updates to GTT. */
-	I915_WRITE(GEN8_GTCR, GEN8_GTCR_INVALIDATE);
-
 	return vma;
 
 err:
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
index aa2b866474be..4d26965e3f7f 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -366,9 +366,6 @@ static int guc_ucode_xfer(struct drm_i915_private *dev_priv)
 		return PTR_ERR(vma);
 	}
 
-	/* Invalidate GuC TLB to let GuC take the latest updates to GTT. */
-	I915_WRITE(GEN8_GTCR, GEN8_GTCR_INVALIDATE);
-
 	intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
 
 	/* init WOPCM */
@@ -486,6 +483,9 @@ int intel_guc_setup(struct drm_i915_private *dev_priv)
 	guc_interrupts_release(dev_priv);
 	gen9_reset_guc_interrupts(dev_priv);
 
+	/* We need to notify the guc whenever we change the GGTT */
+	i915_ggtt_enable_guc(dev_priv);
+
 	guc_fw->guc_fw_load_status = GUC_FIRMWARE_PENDING;
 
 	DRM_DEBUG_DRIVER("GuC fw status: fetch %s, load %s\n",
@@ -547,6 +547,7 @@ int intel_guc_setup(struct drm_i915_private *dev_priv)
 	guc_interrupts_release(dev_priv);
 	i915_guc_submission_disable(dev_priv);
 	i915_guc_submission_fini(dev_priv);
+	i915_ggtt_disable_guc(dev_priv);
 
 	/*
 	 * We've failed to load the firmware :(
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 81665a9eb43f..d9de455da131 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -811,12 +811,6 @@ static int execlists_context_pin(struct intel_engine_cs *engine,
 
 	ce->state->obj->mm.dirty = true;
 
-	/* Invalidate GuC TLB. */
-	if (i915.enable_guc_submission) {
-		struct drm_i915_private *dev_priv = ctx->i915;
-		I915_WRITE(GEN8_GTCR, GEN8_GTCR_INVALIDATE);
-	}
-
 	i915_gem_context_get(ctx);
 	return 0;
 
-- 
2.11.0

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

^ permalink raw reply related	[flat|nested] 25+ messages in thread
end of thread, other threads:[~2017-11-15 18:30 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-30 18:56 [PATCH 0/3] GuC based reset engine Michel Thierry
2017-10-30 18:56 ` [PATCH 1/3] drm/i915/guc: Rename the function that resets the GuC Michel Thierry
2017-10-30 21:02   ` Chris Wilson
2017-10-30 18:56 ` [PATCH 2/3] drm/i915/guc: Add support for reset engine using GuC commands Michel Thierry
2017-10-30 20:58   ` Chris Wilson
2017-10-30 21:08     ` Michel Thierry
2017-10-30 21:09   ` Chris Wilson
2017-10-31  4:38     ` Michel Thierry
2017-10-31 10:17       ` Chris Wilson
2017-10-31 22:53   ` [PATCH v6] " Michel Thierry
2017-11-01 13:58     ` Chris Wilson
2017-11-01 20:41       ` Jeff McGee
2017-11-02  8:43         ` Chris Wilson
2017-10-30 18:56 ` [PATCH 3/3] HAX enable GuC submission for CI Michel Thierry
2017-10-30 20:05 ` ✓ Fi.CI.BAT: success for GuC based reset engine Patchwork
2017-10-30 21:14   ` Chris Wilson
2017-10-30 23:20 ` ✗ Fi.CI.IGT: warning " Patchwork
2017-10-31 10:20   ` Chris Wilson
2017-10-31 20:56     ` Michel Thierry
2017-10-31 21:31       ` Chris Wilson
2017-10-31 23:50 ` ✓ Fi.CI.BAT: success for GuC based reset engine (rev2) Patchwork
2017-11-01  0:59 ` ✗ Fi.CI.IGT: failure " Patchwork
  -- strict thread matches above, loose matches on Subject: below --
2017-11-15 18:30 [PATCH v4 1/3] drm/i915/selftests: Add a GuC doorbells selftest Michel Thierry
2017-11-15 18:30 ` [PATCH 3/3] HAX enable GuC submission for CI Michel Thierry
2017-10-20 21:06 [PATCH 1/3] drm/i915: Add a hook for making the engines idle (parking) and unparking Chris Wilson
2017-10-20 21:06 ` [PATCH 3/3] HAX Enable GuC Submission for CI Chris Wilson
2017-01-11 13:13 [PATCH 1/3] drm/i915: Invalidate the guc ggtt TLB upon insertion Chris Wilson
2017-01-11 13:13 ` [PATCH 3/3] HAX enable guc submission for CI Chris Wilson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox