[PATCH v6 01/12] drm/i915: Correctly handle error path in i915_gem_init_hw

[PATCH v6 01/12] drm/i915: Correctly handle error path in i915_gem_init_hw

[Michal Wajdeczko]

In function gem_init_hw() we are calling uc_init_hw() but in case
of error later in function, we missed to call matching uc_fini_hw()

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 9650a7b..65ba104 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5171,9 +5171,15 @@ int i915_gem_init_hw(struct drm_i915_private *dev_priv)
 
 	/* Only when the HW is re-initialised, can we replay the requests */
 	ret = __i915_gem_restart_engines(dev_priv);
+	if (ret)
+		goto cleanup_uc;
 out:
 	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
 	return ret;
+
+cleanup_uc:
+	intel_uc_fini_hw(dev_priv);
+	goto out;
 }
 
 static int __intel_engines_record_defaults(struct drm_i915_private *i915)
-- 
1.9.1

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

[PATCH v6 02/12] drm/i915: Move i915_gem_fini to i915_gem.c

[Michal Wajdeczko]

We should keep i915_gem_init/fini functions together for easier
tracking of their symmetry.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.c | 20 --------------------
 drivers/gpu/drm/i915/i915_drv.h |  1 +
 drivers/gpu/drm/i915/i915_gem.c | 20 ++++++++++++++++++++
 3 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index d354627..07e212a 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -625,26 +625,6 @@ static bool i915_switcheroo_can_switch(struct pci_dev *pdev)
 	.can_switch = i915_switcheroo_can_switch,
 };
 
-static void i915_gem_fini(struct drm_i915_private *dev_priv)
-{
-	/* Flush any outstanding unpin_work. */
-	i915_gem_drain_workqueue(dev_priv);
-
-	mutex_lock(&dev_priv->drm.struct_mutex);
-	intel_uc_fini_hw(dev_priv);
-	intel_uc_fini(dev_priv);
-	i915_gem_cleanup_engines(dev_priv);
-	i915_gem_contexts_fini(dev_priv);
-	mutex_unlock(&dev_priv->drm.struct_mutex);
-
-	intel_uc_fini_misc(dev_priv);
-	i915_gem_cleanup_userptr(dev_priv);
-
-	i915_gem_drain_freed_objects(dev_priv);
-
-	WARN_ON(!list_empty(&dev_priv->contexts.list));
-}
-
 static int i915_load_modeset_init(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5373b17..19ed3d4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3138,6 +3138,7 @@ void i915_gem_reset_engine(struct intel_engine_cs *engine,
 int __must_check i915_gem_init(struct drm_i915_private *dev_priv);
 int __must_check i915_gem_init_hw(struct drm_i915_private *dev_priv);
 void i915_gem_init_swizzling(struct drm_i915_private *dev_priv);
+void i915_gem_fini(struct drm_i915_private *dev_priv);
 void i915_gem_cleanup_engines(struct drm_i915_private *dev_priv);
 int i915_gem_wait_for_idle(struct drm_i915_private *dev_priv,
 			   unsigned int flags);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 65ba104..19d493b 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5445,6 +5445,26 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
 	return ret;
 }
 
+void i915_gem_fini(struct drm_i915_private *dev_priv)
+{
+	/* Flush any outstanding unpin_work. */
+	i915_gem_drain_workqueue(dev_priv);
+
+	mutex_lock(&dev_priv->drm.struct_mutex);
+	intel_uc_fini_hw(dev_priv);
+	intel_uc_fini(dev_priv);
+	i915_gem_cleanup_engines(dev_priv);
+	i915_gem_contexts_fini(dev_priv);
+	mutex_unlock(&dev_priv->drm.struct_mutex);
+
+	intel_uc_fini_misc(dev_priv);
+	i915_gem_cleanup_userptr(dev_priv);
+
+	i915_gem_drain_freed_objects(dev_priv);
+
+	WARN_ON(!list_empty(&dev_priv->contexts.list));
+}
+
 void i915_gem_init_mmio(struct drm_i915_private *i915)
 {
 	i915_gem_sanitize(i915);
-- 
1.9.1

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

[PATCH v6 03/12] drm/i915: Introduce i915_gem_fini_hw for symmetry with i915_gem_init_hw

[Michal Wajdeczko]

We have i915_gem_init_hw function that on failure requires some
cleanup in uC and then in other places we were trying to do
such cleanup directly. Let's fix that by adding i915_gem_fini_hw
for nice symmetry with init_hw and call it on cleanup paths.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.h |  1 +
 drivers/gpu/drm/i915/i915_gem.c | 13 +++++++++++--
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 19ed3d4..97f04fe 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3139,6 +3139,7 @@ void i915_gem_reset_engine(struct intel_engine_cs *engine,
 int __must_check i915_gem_init_hw(struct drm_i915_private *dev_priv);
 void i915_gem_init_swizzling(struct drm_i915_private *dev_priv);
 void i915_gem_fini(struct drm_i915_private *dev_priv);
+void i915_gem_fini_hw(struct drm_i915_private *dev_priv);
 void i915_gem_cleanup_engines(struct drm_i915_private *dev_priv);
 int i915_gem_wait_for_idle(struct drm_i915_private *dev_priv,
 			   unsigned int flags);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 19d493b..701f137 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5182,6 +5182,15 @@ int i915_gem_init_hw(struct drm_i915_private *dev_priv)
 	goto out;
 }
 
+void i915_gem_fini_hw(struct drm_i915_private *dev_priv)
+{
+	intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
+
+	intel_uc_fini_hw(dev_priv);
+
+	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
+}
+
 static int __intel_engines_record_defaults(struct drm_i915_private *i915)
 {
 	struct i915_gem_context *ctx;
@@ -5407,7 +5416,7 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
 err_init_hw:
 	i915_gem_wait_for_idle(dev_priv, I915_WAIT_LOCKED);
 	i915_gem_contexts_lost(dev_priv);
-	intel_uc_fini_hw(dev_priv);
+	i915_gem_fini_hw(dev_priv);
 err_uc_init:
 	intel_uc_fini(dev_priv);
 err_pm:
@@ -5451,7 +5460,7 @@ void i915_gem_fini(struct drm_i915_private *dev_priv)
 	i915_gem_drain_workqueue(dev_priv);
 
 	mutex_lock(&dev_priv->drm.struct_mutex);
-	intel_uc_fini_hw(dev_priv);
+	i915_gem_fini_hw(dev_priv);
 	intel_uc_fini(dev_priv);
 	i915_gem_cleanup_engines(dev_priv);
 	i915_gem_contexts_fini(dev_priv);
-- 
1.9.1

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

[PATCH v6 04/12] drm/i915: Add i915_gem_fini_hw to i915_gem_suspend

[Michal Wajdeczko]

By calling i915_gem_init_hw in i915_gem_resume and not calling
i915_gem_fini_hw in i915_gem_suspend we introduced asymmetry
in init_hw/fini_hw calls. Let's fix that.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 701f137..f5ef02d 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4986,6 +4986,7 @@ int i915_gem_suspend(struct drm_i915_private *dev_priv)
 	 * machines is a good idea, we don't - just in case it leaves the
 	 * machine in an unusable condition.
 	 */
+	i915_gem_fini_hw(dev_priv);
 	intel_uc_sanitize(dev_priv);
 	i915_gem_sanitize(dev_priv);
 
-- 
1.9.1

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

[PATCH v6 05/12] drm/i915: Add i915_gem_fini_hw to i915_reset

[Michal Wajdeczko]

By calling in i915_reset only i915_gem_init_hw without previous
i915_gem_fini_hw we introduced asymmetry. Let's fix that.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 07e212a..9977adf 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1897,6 +1897,8 @@ void i915_reset(struct drm_i915_private *i915)
 		goto error;
 	}
 
+	i915_gem_fini_hw(i915);
+
 	for (i = 0; i < 3; i++) {
 		ret = intel_gpu_reset(i915, ALL_ENGINES);
 		if (ret == 0)
-- 
1.9.1

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

[PATCH v6 06/12] drm/i915/guc: Ignore dev_priv->gt.awake while disabling submission

[Michal Wajdeczko]

In next patch we will also try to disable GuC submission during reset
path, where where we don't wait for idle_work to complete.
Remove GEM_BUG_ON to allow new scenario. While here fix order
to match symmetry with enable function.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_guc_submission.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
index 9712123..0facfd2 100644
--- a/drivers/gpu/drm/i915/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/intel_guc_submission.c
@@ -1261,13 +1261,9 @@ void intel_guc_submission_disable(struct intel_guc *guc)
 {
 	struct drm_i915_private *dev_priv = guc_to_i915(guc);
 
-	GEM_BUG_ON(dev_priv->gt.awake); /* GT should be parked first */
-
+	intel_engines_reset_default_submission(dev_priv);
 	guc_interrupts_release(dev_priv);
 	guc_clients_doorbell_fini(guc);
-
-	/* Revert back to manual ELSP submission */
-	intel_engines_reset_default_submission(dev_priv);
 }
 
 #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
-- 
1.9.1

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

[PATCH v6 07/12] drm/i915/guc: Restore symmetric doorbell cleanup

[Michal Wajdeczko]

In commit 9192d4fb811e ("drm/i915/guc: Extract doorbell creation
from client allocation") we introduced asymmetry in doorbell cleanup
to avoid warnings due to failed communication with already reset GuC.
As we improved our reset/unload paths, we can restore symmetry in
doorbell cleanup, as GuC should be still active by now.

Suggested-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Michal Winiarski <michal.winiarski@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_submission.c | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
index 0facfd2..3adac5b 100644
--- a/drivers/gpu/drm/i915/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/intel_guc_submission.c
@@ -848,18 +848,9 @@ static int guc_clients_doorbell_init(struct intel_guc *guc)
 
 static void guc_clients_doorbell_fini(struct intel_guc *guc)
 {
-	/*
-	 * By the time we're here, GuC has already been reset.
-	 * Instead of trying (in vain) to communicate with it, let's just
-	 * cleanup the doorbell HW and our internal state.
-	 */
-	if (guc->preempt_client) {
-		__destroy_doorbell(guc->preempt_client);
-		__update_doorbell_desc(guc->preempt_client,
-				       GUC_DOORBELL_INVALID);
-	}
-	__destroy_doorbell(guc->execbuf_client);
-	__update_doorbell_desc(guc->execbuf_client, GUC_DOORBELL_INVALID);
+	if (guc->preempt_client)
+		destroy_doorbell(guc->preempt_client);
+	destroy_doorbell(guc->execbuf_client);
 }
 
 /**
-- 
1.9.1

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

[PATCH v6 08/12] drm/i915/uc: Fully sanitize uC within intel_uc_fini_hw

[Michal Wajdeczko]

As we always call intel_uc_sanitize after every call to
intel_uc_fini_hw we may drop redundant call and sanitize
uC from the fini_hw function.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem.c | 2 --
 drivers/gpu/drm/i915/intel_uc.c | 9 +++------
 drivers/gpu/drm/i915/intel_uc.h | 1 -
 3 files changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index f5ef02d..ea00f3e 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2998,7 +2998,6 @@ int i915_gem_reset_prepare(struct drm_i915_private *dev_priv)
 	}
 
 	i915_gem_revoke_fences(dev_priv);
-	intel_uc_sanitize(dev_priv);
 
 	return err;
 }
@@ -4987,7 +4986,6 @@ int i915_gem_suspend(struct drm_i915_private *dev_priv)
 	 * machine in an unusable condition.
 	 */
 	i915_gem_fini_hw(dev_priv);
-	intel_uc_sanitize(dev_priv);
 	i915_gem_sanitize(dev_priv);
 
 	intel_runtime_pm_put(dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 1cffaf7..0439966 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -322,18 +322,13 @@ void intel_uc_fini(struct drm_i915_private *dev_priv)
 	intel_guc_fini(guc);
 }
 
-void intel_uc_sanitize(struct drm_i915_private *i915)
+static void __uc_sanitize(struct drm_i915_private *i915)
 {
 	struct intel_guc *guc = &i915->guc;
 	struct intel_huc *huc = &i915->huc;
 
-	if (!USES_GUC(i915))
-		return;
-
 	GEM_BUG_ON(!HAS_GUC(i915));
 
-	guc_disable_communication(guc);
-
 	intel_huc_sanitize(huc);
 	intel_guc_sanitize(guc);
 
@@ -445,6 +440,8 @@ void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
 		intel_guc_submission_disable(guc);
 
 	guc_disable_communication(guc);
+
+	__uc_sanitize(dev_priv);
 }
 
 int intel_uc_suspend(struct drm_i915_private *i915)
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 25d73ad..64aaf93 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -33,7 +33,6 @@
 void intel_uc_init_mmio(struct drm_i915_private *dev_priv);
 int intel_uc_init_misc(struct drm_i915_private *dev_priv);
 void intel_uc_fini_misc(struct drm_i915_private *dev_priv);
-void intel_uc_sanitize(struct drm_i915_private *dev_priv);
 int intel_uc_init_hw(struct drm_i915_private *dev_priv);
 void intel_uc_fini_hw(struct drm_i915_private *dev_priv);
 int intel_uc_init(struct drm_i915_private *dev_priv);
-- 
1.9.1

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

[PATCH v6 09/12] drm/i915/uc: Use correct error code for GuC initialization failure

[Michal Wajdeczko]

Since commit 6ca9a2beb54a ("drm/i915: Unwind i915_gem_init() failure")
we believed that we correctly handle all errors encountered during
GuC initialization, including special one that indicates request to
run driver with disabled GPU submission (-EIO).

Unfortunately since commit 121981fafe69 ("drm/i915/guc: Combine
enable_guc_loading|submission modparams") we stopped using that
error code to avoid unwanted fallback to execlist submission mode.

In result any GuC initialization failure was treated as non-recoverable
error leading to driver load abort, so we could not even read related
GuC error log to investigate cause of the problem.

Fix that by always returning -EIO on uC hardware related failure.

v2: don't allow -EIO from uc_init
    don't call uc_fini[_misc] on -EIO
    mark guc fw as failed on hw init failure
    prepare uc_fini_hw to run after earlier -EIO

v3: update comments (Sagar)
    use sanitize functions on failure in init_hw (Michal)
    and also sanitize guc/huc fw in fini_hw (Michal)

v4: rebase
v5: rebase

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michal Winiarski <michal.winiarski@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c    | 17 ++++++++++-------
 drivers/gpu/drm/i915/intel_guc.h   |  5 +++++
 drivers/gpu/drm/i915/intel_uc.c    | 15 +++++++++++----
 drivers/gpu/drm/i915/intel_uc_fw.h |  5 +++++
 4 files changed, 31 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index ea00f3e..6ab5f4c 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5369,8 +5369,10 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
 	intel_init_gt_powersave(dev_priv);
 
 	ret = intel_uc_init(dev_priv);
-	if (ret)
+	if (ret) {
+		GEM_BUG_ON(ret == -EIO);
 		goto err_pm;
+	}
 
 	ret = i915_gem_init_hw(dev_priv);
 	if (ret)
@@ -5417,7 +5419,8 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
 	i915_gem_contexts_lost(dev_priv);
 	i915_gem_fini_hw(dev_priv);
 err_uc_init:
-	intel_uc_fini(dev_priv);
+	if (ret != -EIO)
+		intel_uc_fini(dev_priv);
 err_pm:
 	if (ret != -EIO) {
 		intel_cleanup_gt_powersave(dev_priv);
@@ -5431,15 +5434,15 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
 	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
 	mutex_unlock(&dev_priv->drm.struct_mutex);
 
-	intel_uc_fini_misc(dev_priv);
-
-	if (ret != -EIO)
+	if (ret != -EIO) {
+		intel_uc_fini_misc(dev_priv);
 		i915_gem_cleanup_userptr(dev_priv);
+	}
 
 	if (ret == -EIO) {
 		/*
-		 * Allow engine initialisation to fail by marking the GPU as
-		 * wedged. But we only want to do this where the GPU is angry,
+		 * Allow engines or uC initialization to fail by marking the GPU
+		 * as wedged. But we only want to do this when the GPU is angry,
 		 * for all other failure, such as an allocation failure, bail.
 		 */
 		if (!i915_terminally_wedged(&dev_priv->gpu_error)) {
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index f1265e1..c587068 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -176,6 +176,11 @@ static inline int intel_guc_sanitize(struct intel_guc *guc)
 	return 0;
 }
 
+static inline bool intel_guc_is_loaded(struct intel_guc *guc)
+{
+	return intel_uc_fw_is_loaded(&guc->fw);
+}
+
 static inline void intel_guc_enable_msg(struct intel_guc *guc, u32 mask)
 {
 	spin_lock_irq(&guc->irq_lock);
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 0439966..7862731 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -332,6 +332,8 @@ static void __uc_sanitize(struct drm_i915_private *i915)
 	intel_huc_sanitize(huc);
 	intel_guc_sanitize(guc);
 
+	GEM_BUG_ON(intel_guc_is_loaded(guc));
+
 	__intel_uc_reset_hw(i915);
 }
 
@@ -420,11 +422,13 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 	 * Note that there is no fallback as either user explicitly asked for
 	 * the GuC or driver default option was to run with the GuC enabled.
 	 */
-	if (GEM_WARN_ON(ret == -EIO))
-		ret = -EINVAL;
-
 	dev_err(dev_priv->drm.dev, "GuC initialization failed %d\n", ret);
-	return ret;
+
+	/* Sanitize GuC/HuC to avoid clean-up on wedged */
+	__uc_sanitize(dev_priv);
+
+	/* We want to disable GPU submission but keep KMS alive */
+	return -EIO;
 }
 
 void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
@@ -436,6 +440,9 @@ void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
 
 	GEM_BUG_ON(!HAS_GUC(dev_priv));
 
+	if (!intel_guc_is_loaded(guc))
+		return;
+
 	if (USES_GUC_SUBMISSION(dev_priv))
 		intel_guc_submission_disable(guc);
 
diff --git a/drivers/gpu/drm/i915/intel_uc_fw.h b/drivers/gpu/drm/i915/intel_uc_fw.h
index dc33b12..77ad2aa 100644
--- a/drivers/gpu/drm/i915/intel_uc_fw.h
+++ b/drivers/gpu/drm/i915/intel_uc_fw.h
@@ -121,6 +121,11 @@ static inline void intel_uc_fw_sanitize(struct intel_uc_fw *uc_fw)
 		uc_fw->load_status = INTEL_UC_FIRMWARE_PENDING;
 }
 
+static inline bool intel_uc_fw_is_loaded(struct intel_uc_fw *uc_fw)
+{
+	return uc_fw->load_status == INTEL_UC_FIRMWARE_SUCCESS;
+}
+
 /**
  * intel_uc_fw_get_upload_size() - Get size of firmware needed to be uploaded.
  * @uc_fw: uC firmware.
-- 
1.9.1

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

[PATCH v6 10/12] drm/i915/uc: Use helper functions to detect fw load status

[Michal Wajdeczko]

We don't have to check load status values.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/intel_huc.c | 2 +-
 drivers/gpu/drm/i915/intel_uc.c  | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c
index 2912852..975ae61 100644
--- a/drivers/gpu/drm/i915/intel_huc.c
+++ b/drivers/gpu/drm/i915/intel_huc.c
@@ -51,7 +51,7 @@ int intel_huc_auth(struct intel_huc *huc)
 	u32 status;
 	int ret;
 
-	if (huc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
+	if (!intel_uc_fw_is_loaded(&huc->fw))
 		return -ENOEXEC;
 
 	vma = i915_gem_object_ggtt_pin(huc->fw.obj, NULL, 0, 0,
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 7862731..ed39273 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -459,7 +459,7 @@ int intel_uc_suspend(struct drm_i915_private *i915)
 	if (!USES_GUC(i915))
 		return 0;
 
-	if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
+	if (!intel_guc_is_loaded(guc))
 		return 0;
 
 	err = intel_guc_suspend(guc);
@@ -481,7 +481,7 @@ int intel_uc_resume(struct drm_i915_private *i915)
 	if (!USES_GUC(i915))
 		return 0;
 
-	if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
+	if (!intel_guc_is_loaded(guc))
 		return 0;
 
 	gen9_enable_guc_interrupts(i915);
-- 
1.9.1

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

[PATCH v6 11/12] drm/i915/uc: Trivial s/dev_priv/i915 in intel_uc.c

[Michal Wajdeczko]

Some functions already use i915 name instead of dev_priv.
Let's rename this param in all remaining functions, except
those that still use legacy macros.

v2: don't forget about function descriptions (Sagar)
v3: rebased

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/intel_uc.c | 133 ++++++++++++++++++++--------------------
 1 file changed, 66 insertions(+), 67 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index ed39273..ccf75aa 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -50,10 +50,10 @@ static int __intel_uc_reset_hw(struct drm_i915_private *dev_priv)
 	return ret;
 }
 
-static int __get_platform_enable_guc(struct drm_i915_private *dev_priv)
+static int __get_platform_enable_guc(struct drm_i915_private *i915)
 {
-	struct intel_uc_fw *guc_fw = &dev_priv->guc.fw;
-	struct intel_uc_fw *huc_fw = &dev_priv->huc.fw;
+	struct intel_uc_fw *guc_fw = &i915->guc.fw;
+	struct intel_uc_fw *huc_fw = &i915->huc.fw;
 	int enable_guc = 0;
 
 	/* Default is to enable GuC/HuC if we know their firmwares */
@@ -67,11 +67,11 @@ static int __get_platform_enable_guc(struct drm_i915_private *dev_priv)
 	return enable_guc;
 }
 
-static int __get_default_guc_log_level(struct drm_i915_private *dev_priv)
+static int __get_default_guc_log_level(struct drm_i915_private *i915)
 {
 	int guc_log_level;
 
-	if (!HAS_GUC(dev_priv) || !intel_uc_is_using_guc())
+	if (!HAS_GUC(i915) || !intel_uc_is_using_guc())
 		guc_log_level = GUC_LOG_LEVEL_DISABLED;
 	else if (IS_ENABLED(CONFIG_DRM_I915_DEBUG) ||
 		 IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM))
@@ -86,7 +86,7 @@ static int __get_default_guc_log_level(struct drm_i915_private *dev_priv)
 
 /**
  * sanitize_options_early - sanitize uC related modparam options
- * @dev_priv: device private
+ * @i915: device private
  *
  * In case of "enable_guc" option this function will attempt to modify
  * it only if it was initially set to "auto(-1)". Default value for this
@@ -101,14 +101,14 @@ static int __get_default_guc_log_level(struct drm_i915_private *dev_priv)
  * unless GuC is enabled on given platform and the driver is compiled with
  * debug config when this modparam will default to "enable(1..4)".
  */
-static void sanitize_options_early(struct drm_i915_private *dev_priv)
+static void sanitize_options_early(struct drm_i915_private *i915)
 {
-	struct intel_uc_fw *guc_fw = &dev_priv->guc.fw;
-	struct intel_uc_fw *huc_fw = &dev_priv->huc.fw;
+	struct intel_uc_fw *guc_fw = &i915->guc.fw;
+	struct intel_uc_fw *huc_fw = &i915->huc.fw;
 
 	/* A negative value means "use platform default" */
 	if (i915_modparams.enable_guc < 0)
-		i915_modparams.enable_guc = __get_platform_enable_guc(dev_priv);
+		i915_modparams.enable_guc = __get_platform_enable_guc(i915);
 
 	DRM_DEBUG_DRIVER("enable_guc=%d (submission:%s huc:%s)\n",
 			 i915_modparams.enable_guc,
@@ -119,28 +119,28 @@ static void sanitize_options_early(struct drm_i915_private *dev_priv)
 	if (intel_uc_is_using_guc() && !intel_uc_fw_is_selected(guc_fw)) {
 		DRM_WARN("Incompatible option detected: %s=%d, %s!\n",
 			 "enable_guc", i915_modparams.enable_guc,
-			 !HAS_GUC(dev_priv) ? "no GuC hardware" :
-					      "no GuC firmware");
+			 !HAS_GUC(i915) ? "no GuC hardware" :
+					  "no GuC firmware");
 	}
 
 	/* Verify HuC firmware availability */
 	if (intel_uc_is_using_huc() && !intel_uc_fw_is_selected(huc_fw)) {
 		DRM_WARN("Incompatible option detected: %s=%d, %s!\n",
 			 "enable_guc", i915_modparams.enable_guc,
-			 !HAS_HUC(dev_priv) ? "no HuC hardware" :
-					      "no HuC firmware");
+			 !HAS_HUC(i915) ? "no HuC hardware" :
+					  "no HuC firmware");
 	}
 
 	/* A negative value means "use platform/config default" */
 	if (i915_modparams.guc_log_level < 0)
 		i915_modparams.guc_log_level =
-			__get_default_guc_log_level(dev_priv);
+			__get_default_guc_log_level(i915);
 
 	if (i915_modparams.guc_log_level > 0 && !intel_uc_is_using_guc()) {
 		DRM_WARN("Incompatible option detected: %s=%d, %s!\n",
 			 "guc_log_level", i915_modparams.guc_log_level,
-			 !HAS_GUC(dev_priv) ? "no GuC hardware" :
-					      "GuC not enabled");
+			 !HAS_GUC(i915) ? "no GuC hardware" :
+					  "GuC not enabled");
 		i915_modparams.guc_log_level = 0;
 	}
 
@@ -195,15 +195,14 @@ void intel_uc_cleanup_early(struct drm_i915_private *i915)
 
 /**
  * intel_uc_init_mmio - setup uC MMIO access
- *
- * @dev_priv: device private
+ * @i915: device private
  *
  * Setup minimal state necessary for MMIO accesses later in the
  * initialization sequence.
  */
-void intel_uc_init_mmio(struct drm_i915_private *dev_priv)
+void intel_uc_init_mmio(struct drm_i915_private *i915)
 {
-	intel_guc_init_send_regs(&dev_priv->guc);
+	intel_guc_init_send_regs(&i915->guc);
 }
 
 static void guc_capture_load_err_log(struct intel_guc *guc)
@@ -225,11 +224,11 @@ static void guc_free_load_err_log(struct intel_guc *guc)
 
 static int guc_enable_communication(struct intel_guc *guc)
 {
-	struct drm_i915_private *dev_priv = guc_to_i915(guc);
+	struct drm_i915_private *i915 = guc_to_i915(guc);
 
-	gen9_enable_guc_interrupts(dev_priv);
+	gen9_enable_guc_interrupts(i915);
 
-	if (HAS_GUC_CT(dev_priv))
+	if (HAS_GUC_CT(i915))
 		return intel_guc_ct_enable(&guc->ct);
 
 	guc->send = intel_guc_send_mmio;
@@ -239,23 +238,23 @@ static int guc_enable_communication(struct intel_guc *guc)
 
 static void guc_disable_communication(struct intel_guc *guc)
 {
-	struct drm_i915_private *dev_priv = guc_to_i915(guc);
+	struct drm_i915_private *i915 = guc_to_i915(guc);
 
-	if (HAS_GUC_CT(dev_priv))
+	if (HAS_GUC_CT(i915))
 		intel_guc_ct_disable(&guc->ct);
 
-	gen9_disable_guc_interrupts(dev_priv);
+	gen9_disable_guc_interrupts(i915);
 
 	guc->send = intel_guc_send_nop;
 	guc->handler = intel_guc_to_host_event_handler_nop;
 }
 
-int intel_uc_init_misc(struct drm_i915_private *dev_priv)
+int intel_uc_init_misc(struct drm_i915_private *i915)
 {
-	struct intel_guc *guc = &dev_priv->guc;
+	struct intel_guc *guc = &i915->guc;
 	int ret;
 
-	if (!USES_GUC(dev_priv))
+	if (!USES_GUC(i915))
 		return 0;
 
 	intel_guc_init_ggtt_pin_bias(guc);
@@ -267,32 +266,32 @@ int intel_uc_init_misc(struct drm_i915_private *dev_priv)
 	return 0;
 }
 
-void intel_uc_fini_misc(struct drm_i915_private *dev_priv)
+void intel_uc_fini_misc(struct drm_i915_private *i915)
 {
-	struct intel_guc *guc = &dev_priv->guc;
+	struct intel_guc *guc = &i915->guc;
 
-	if (!USES_GUC(dev_priv))
+	if (!USES_GUC(i915))
 		return;
 
 	intel_guc_fini_wq(guc);
 }
 
-int intel_uc_init(struct drm_i915_private *dev_priv)
+int intel_uc_init(struct drm_i915_private *i915)
 {
-	struct intel_guc *guc = &dev_priv->guc;
+	struct intel_guc *guc = &i915->guc;
 	int ret;
 
-	if (!USES_GUC(dev_priv))
+	if (!USES_GUC(i915))
 		return 0;
 
-	if (!HAS_GUC(dev_priv))
+	if (!HAS_GUC(i915))
 		return -ENODEV;
 
 	ret = intel_guc_init(guc);
 	if (ret)
 		return ret;
 
-	if (USES_GUC_SUBMISSION(dev_priv)) {
+	if (USES_GUC_SUBMISSION(i915)) {
 		/*
 		 * This is stuff we need to have available at fw load time
 		 * if we are planning to enable submission later
@@ -307,16 +306,16 @@ int intel_uc_init(struct drm_i915_private *dev_priv)
 	return 0;
 }
 
-void intel_uc_fini(struct drm_i915_private *dev_priv)
+void intel_uc_fini(struct drm_i915_private *i915)
 {
-	struct intel_guc *guc = &dev_priv->guc;
+	struct intel_guc *guc = &i915->guc;
 
-	if (!USES_GUC(dev_priv))
+	if (!USES_GUC(i915))
 		return;
 
-	GEM_BUG_ON(!HAS_GUC(dev_priv));
+	GEM_BUG_ON(!HAS_GUC(i915));
 
-	if (USES_GUC_SUBMISSION(dev_priv))
+	if (USES_GUC_SUBMISSION(i915))
 		intel_guc_submission_fini(guc);
 
 	intel_guc_fini(guc);
@@ -337,22 +336,22 @@ static void __uc_sanitize(struct drm_i915_private *i915)
 	__intel_uc_reset_hw(i915);
 }
 
-int intel_uc_init_hw(struct drm_i915_private *dev_priv)
+int intel_uc_init_hw(struct drm_i915_private *i915)
 {
-	struct intel_guc *guc = &dev_priv->guc;
-	struct intel_huc *huc = &dev_priv->huc;
+	struct intel_guc *guc = &i915->guc;
+	struct intel_huc *huc = &i915->huc;
 	int ret, attempts;
 
-	if (!USES_GUC(dev_priv))
+	if (!USES_GUC(i915))
 		return 0;
 
-	GEM_BUG_ON(!HAS_GUC(dev_priv));
+	GEM_BUG_ON(!HAS_GUC(i915));
 
-	gen9_reset_guc_interrupts(dev_priv);
+	gen9_reset_guc_interrupts(i915);
 
 	/* WaEnableuKernelHeaderValidFix:skl */
 	/* WaEnableGuCBootHashCheckNotSet:skl,bxt,kbl */
-	if (IS_GEN9(dev_priv))
+	if (IS_GEN9(i915))
 		attempts = 3;
 	else
 		attempts = 1;
@@ -362,11 +361,11 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 		 * Always reset the GuC just before (re)loading, so
 		 * that the state and timing are fairly predictable
 		 */
-		ret = __intel_uc_reset_hw(dev_priv);
+		ret = __intel_uc_reset_hw(i915);
 		if (ret)
 			goto err_out;
 
-		if (USES_HUC(dev_priv)) {
+		if (USES_HUC(i915)) {
 			ret = intel_huc_fw_upload(huc);
 			if (ret)
 				goto err_out;
@@ -389,24 +388,24 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 	if (ret)
 		goto err_log_capture;
 
-	if (USES_HUC(dev_priv)) {
+	if (USES_HUC(i915)) {
 		ret = intel_huc_auth(huc);
 		if (ret)
 			goto err_communication;
 	}
 
-	if (USES_GUC_SUBMISSION(dev_priv)) {
+	if (USES_GUC_SUBMISSION(i915)) {
 		ret = intel_guc_submission_enable(guc);
 		if (ret)
 			goto err_communication;
 	}
 
-	dev_info(dev_priv->drm.dev, "GuC firmware version %u.%u\n",
+	dev_info(i915->drm.dev, "GuC firmware version %u.%u\n",
 		 guc->fw.major_ver_found, guc->fw.minor_ver_found);
-	dev_info(dev_priv->drm.dev, "GuC submission %s\n",
-		 enableddisabled(USES_GUC_SUBMISSION(dev_priv)));
-	dev_info(dev_priv->drm.dev, "HuC %s\n",
-		 enableddisabled(USES_HUC(dev_priv)));
+	dev_info(i915->drm.dev, "GuC submission %s\n",
+		 enableddisabled(USES_GUC_SUBMISSION(i915)));
+	dev_info(i915->drm.dev, "HuC %s\n",
+		 enableddisabled(USES_HUC(i915)));
 
 	return 0;
 
@@ -422,33 +421,33 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 	 * Note that there is no fallback as either user explicitly asked for
 	 * the GuC or driver default option was to run with the GuC enabled.
 	 */
-	dev_err(dev_priv->drm.dev, "GuC initialization failed %d\n", ret);
+	dev_err(i915->drm.dev, "GuC initialization failed %d\n", ret);
 
 	/* Sanitize GuC/HuC to avoid clean-up on wedged */
-	__uc_sanitize(dev_priv);
+	__uc_sanitize(i915);
 
 	/* We want to disable GPU submission but keep KMS alive */
 	return -EIO;
 }
 
-void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
+void intel_uc_fini_hw(struct drm_i915_private *i915)
 {
-	struct intel_guc *guc = &dev_priv->guc;
+	struct intel_guc *guc = &i915->guc;
 
-	if (!USES_GUC(dev_priv))
+	if (!USES_GUC(i915))
 		return;
 
-	GEM_BUG_ON(!HAS_GUC(dev_priv));
+	GEM_BUG_ON(!HAS_GUC(i915));
 
 	if (!intel_guc_is_loaded(guc))
 		return;
 
-	if (USES_GUC_SUBMISSION(dev_priv))
+	if (USES_GUC_SUBMISSION(i915))
 		intel_guc_submission_disable(guc);
 
 	guc_disable_communication(guc);
 
-	__uc_sanitize(dev_priv);
+	__uc_sanitize(i915);
 }
 
 int intel_uc_suspend(struct drm_i915_private *i915)
-- 
1.9.1

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

[PATCH v6 12/12] HAX: Enable GuC for CI

[Michal Wajdeczko]

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
---
 drivers/gpu/drm/i915/i915_params.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index c963603..53037b5 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -47,7 +47,7 @@
 	param(int, disable_power_well, -1) \
 	param(int, enable_ips, 1) \
 	param(int, invert_brightness, 0) \
-	param(int, enable_guc, 0) \
+	param(int, enable_guc, -1) \
 	param(int, guc_log_level, -1) \
 	param(char *, guc_firmware_path, NULL) \
 	param(char *, huc_firmware_path, NULL) \
-- 
1.9.1

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

✓ Fi.CI.BAT: success for series starting with [v6,01/12] drm/i915: Correctly handle error path in i915_gem_init_hw

[Patchwork]

== Series Details ==

Series: series starting with [v6,01/12] drm/i915: Correctly handle error path in i915_gem_init_hw
URL   : https://patchwork.freedesktop.org/series/41159/
State : success

== Summary ==

Series 41159v1 series starting with [v6,01/12] drm/i915: Correctly handle error path in i915_gem_init_hw
https://patchwork.freedesktop.org/api/1.0/series/41159/revisions/1/mbox/

---- Known issues:

Test gem_exec_suspend:
        Subgroup basic-s3:
                dmesg-warn -> PASS       (fi-glk-j4005) fdo#103359

fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359

fi-bdw-5557u     total:285  pass:264  dwarn:0   dfail:0   fail:0   skip:21  time:435s
fi-bdw-gvtdvm    total:285  pass:261  dwarn:0   dfail:0   fail:0   skip:24  time:443s
fi-blb-e6850     total:285  pass:220  dwarn:1   dfail:0   fail:0   skip:64  time:382s
fi-bsw-n3050     total:285  pass:239  dwarn:0   dfail:0   fail:0   skip:46  time:534s
fi-bwr-2160      total:285  pass:180  dwarn:0   dfail:0   fail:0   skip:105 time:297s
fi-bxt-dsi       total:285  pass:255  dwarn:0   dfail:0   fail:0   skip:30  time:516s
fi-bxt-j4205     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:521s
fi-byt-j1900     total:285  pass:250  dwarn:0   dfail:0   fail:0   skip:35  time:527s
fi-byt-n2820     total:285  pass:246  dwarn:0   dfail:0   fail:0   skip:39  time:507s
fi-cfl-8700k     total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:410s
fi-cfl-s3        total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:561s
fi-cfl-u         total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:514s
fi-cnl-y3        total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:584s
fi-elk-e7500     total:285  pass:226  dwarn:0   dfail:0   fail:0   skip:59  time:424s
fi-gdg-551       total:285  pass:176  dwarn:0   dfail:0   fail:1   skip:108 time:316s
fi-glk-1         total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:535s
fi-glk-j4005     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:489s
fi-hsw-4770      total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:402s
fi-ilk-650       total:285  pass:225  dwarn:0   dfail:0   fail:0   skip:60  time:419s
fi-ivb-3520m     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:465s
fi-ivb-3770      total:285  pass:252  dwarn:0   dfail:0   fail:0   skip:33  time:436s
fi-kbl-7500u     total:285  pass:260  dwarn:1   dfail:0   fail:0   skip:24  time:475s
fi-kbl-7567u     total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:457s
fi-kbl-r         total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:508s
fi-skl-6260u     total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:438s
fi-skl-6600u     total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:535s
fi-skl-6700k2    total:285  pass:261  dwarn:0   dfail:0   fail:0   skip:24  time:506s
fi-skl-6770hq    total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:503s
fi-skl-guc       total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:430s
fi-skl-gvtdvm    total:285  pass:262  dwarn:0   dfail:0   fail:0   skip:23  time:446s
fi-snb-2520m     total:285  pass:245  dwarn:0   dfail:0   fail:0   skip:40  time:574s
fi-snb-2600      total:285  pass:245  dwarn:0   dfail:0   fail:0   skip:40  time:401s
Blacklisted hosts:
fi-cnl-psr       total:285  pass:256  dwarn:3   dfail:0   fail:0   skip:26  time:518s

e29a10513429cca404e9847a399efbdbb4bdd4bf drm-tip: 2018y-04m-04d-20h-47m-24s UTC integration manifest
935985da86a5 HAX: Enable GuC for CI
84d32acff83d drm/i915/uc: Trivial s/dev_priv/i915 in intel_uc.c
ea3137359cbe drm/i915/uc: Use helper functions to detect fw load status
2811630759e1 drm/i915/uc: Use correct error code for GuC initialization failure
46d163d34342 drm/i915/uc: Fully sanitize uC within intel_uc_fini_hw
ffe83d5f08e5 drm/i915/guc: Restore symmetric doorbell cleanup
d78cbce3429a drm/i915/guc: Ignore dev_priv->gt.awake while disabling submission
02f809bd0ab2 drm/i915: Add i915_gem_fini_hw to i915_reset
254ab376bd38 drm/i915: Add i915_gem_fini_hw to i915_gem_suspend
7c48b9b26215 drm/i915: Introduce i915_gem_fini_hw for symmetry with i915_gem_init_hw
3bcf133ff8ff drm/i915: Move i915_gem_fini to i915_gem.c
34610dfa027f drm/i915: Correctly handle error path in i915_gem_init_hw

== Logs ==

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

✗ Fi.CI.IGT: failure for series starting with [v6,01/12] drm/i915: Correctly handle error path in i915_gem_init_hw

[Patchwork]

== Series Details ==

Series: series starting with [v6,01/12] drm/i915: Correctly handle error path in i915_gem_init_hw
URL   : https://patchwork.freedesktop.org/series/41159/
State : failure

== Summary ==

---- Possible new issues:

Test gem_eio:
        Subgroup execbuf:
                pass       -> INCOMPLETE (shard-apl)
        Subgroup in-flight-external:
                pass       -> INCOMPLETE (shard-apl)
        Subgroup throttle:
                pass       -> INCOMPLETE (shard-apl)
Test perf:
        Subgroup gen8-unprivileged-single-ctx-counters:
                pass       -> FAIL       (shard-apl)

---- Known issues:

Test drv_missed_irq:
                pass       -> SKIP       (shard-apl) fdo#103199
Test gem_eio:
        Subgroup in-flight-suspend:
                pass       -> INCOMPLETE (shard-apl) fdo#103375
        Subgroup suspend:
                pass       -> INCOMPLETE (shard-apl) fdo#103927 +1
Test kms_flip:
        Subgroup 2x-flip-vs-expired-vblank:
                fail       -> PASS       (shard-hsw) fdo#102887 +1
        Subgroup plain-flip-ts-check-interruptible:
                fail       -> PASS       (shard-hsw) fdo#100368 +1
Test kms_sysfs_edid_timing:
                warn       -> PASS       (shard-apl) fdo#100047

fdo#103199 https://bugs.freedesktop.org/show_bug.cgi?id=103199
fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375
fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047

shard-apl        total:2701 pass:1404 dwarn:1   dfail:0   fail:6   skip:1274 time:6843s
shard-hsw        total:3499 pass:1786 dwarn:1   dfail:0   fail:1   skip:1710 time:11567s
shard-snb        total:3499 pass:1378 dwarn:1   dfail:0   fail:2   skip:2118 time:7127s
Blacklisted hosts:
shard-kbl        total:2429 pass:1334 dwarn:4   dfail:0   fail:6   skip:1065 time:4018s

== Logs ==

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

Re: ✗ Fi.CI.IGT: failure for series starting with [v6,01/12] drm/i915: Correctly handle error path in i915_gem_init_hw

[Sagar Arun Kamble]

On 4/5/2018 6:03 AM, Patchwork wrote:
> == Series Details ==
>
> Series: series starting with [v6,01/12] drm/i915: Correctly handle error path in i915_gem_init_hw
> URL   : https://patchwork.freedesktop.org/series/41159/
> State : failure
>
> == Summary ==
>
> ---- Possible new issues:
>
> Test gem_eio:
>          Subgroup execbuf:
>                  pass       -> INCOMPLETE (shard-apl)
I am seeing this failure w/o this patch series too.
Working fix  that I have tried is to unpark from reset path explicitly 
if GT is awake:

diff --git a/drivers/gpu/drm/i915/i915_drv.c 
b/drivers/gpu/drm/i915/i915_drv.c
index d354627..2fc9d0e 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1960,6 +1960,9 @@ void i915_reset(struct drm_i915_private *i915)
                 goto error;
         }

+       if (i915->gt.awake)
+               intel_engines_unpark(i915);
+
         i915_queue_hangcheck(i915);

  finish:

This way we will balance the irq refcounts.
>          Subgroup in-flight-external:
>                  pass       -> INCOMPLETE (shard-apl)
>          Subgroup throttle:
>                  pass       -> INCOMPLETE (shard-apl)
> Test perf:
>          Subgroup gen8-unprivileged-single-ctx-counters:
>                  pass       -> FAIL       (shard-apl)
>
> ---- Known issues:
>
> Test drv_missed_irq:
>                  pass       -> SKIP       (shard-apl) fdo#103199
> Test gem_eio:
>          Subgroup in-flight-suspend:
>                  pass       -> INCOMPLETE (shard-apl) fdo#103375
>          Subgroup suspend:
>                  pass       -> INCOMPLETE (shard-apl) fdo#103927 +1
> Test kms_flip:
>          Subgroup 2x-flip-vs-expired-vblank:
>                  fail       -> PASS       (shard-hsw) fdo#102887 +1
>          Subgroup plain-flip-ts-check-interruptible:
>                  fail       -> PASS       (shard-hsw) fdo#100368 +1
> Test kms_sysfs_edid_timing:
>                  warn       -> PASS       (shard-apl) fdo#100047
>
> fdo#103199 https://bugs.freedesktop.org/show_bug.cgi?id=103199
> fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375
> fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
> fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
> fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
> fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047
>
> shard-apl        total:2701 pass:1404 dwarn:1   dfail:0   fail:6   skip:1274 time:6843s
> shard-hsw        total:3499 pass:1786 dwarn:1   dfail:0   fail:1   skip:1710 time:11567s
> shard-snb        total:3499 pass:1378 dwarn:1   dfail:0   fail:2   skip:2118 time:7127s
> Blacklisted hosts:
> shard-kbl        total:2429 pass:1334 dwarn:4   dfail:0   fail:6   skip:1065 time:4018s
>
> == Logs ==
>
> For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8581/shards.html
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Thanks,
Sagar

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

Re: ✗ Fi.CI.IGT: failure for series starting with [v6,01/12] drm/i915: Correctly handle error path in i915_gem_init_hw

[Chris Wilson]

Quoting Sagar Arun Kamble (2018-04-05 11:02:33)
> 
> 
> On 4/5/2018 6:03 AM, Patchwork wrote:
> > == Series Details ==
> >
> > Series: series starting with [v6,01/12] drm/i915: Correctly handle error path in i915_gem_init_hw
> > URL   : https://patchwork.freedesktop.org/series/41159/
> > State : failure
> >
> > == Summary ==
> >
> > ---- Possible new issues:
> >
> > Test gem_eio:
> >          Subgroup execbuf:
> >                  pass       -> INCOMPLETE (shard-apl)
> I am seeing this failure w/o this patch series too.
> Working fix  that I have tried is to unpark from reset path explicitly 
> if GT is awake:
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c 
> b/drivers/gpu/drm/i915/i915_drv.c
> index d354627..2fc9d0e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1960,6 +1960,9 @@ void i915_reset(struct drm_i915_private *i915)
>                  goto error;
>          }
> 
> +       if (i915->gt.awake)
> +               intel_engines_unpark(i915);
> +
>          i915_queue_hangcheck(i915);
> 
>   finish:
> 
> This way we will balance the irq refcounts.

Only for them to be unbalanced on resume... Just we don't currently warn
about that.

But that's the kind of plan I have, before we call
reset_default_submission in unwedge, to bludgeon through the full idle
sequence. Just means playing about with the idle worker to extract what
we need from under the mutex, probably calling it i915_gem_park in the
process.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx