All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 1/8] drm/i915: Correctly handle error path in i915_gem_init_hw
@ 2018-03-27 20:35 Michal Wajdeczko
  2018-03-27 20:35 ` [PATCH v5 2/8] drm/i915/uc: Disable GuC submission during sanitize Michal Wajdeczko
                   ` (8 more replies)
  0 siblings, 9 replies; 16+ messages in thread
From: Michal Wajdeczko @ 2018-03-27 20:35 UTC (permalink / raw)
  To: intel-gfx

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

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

* [PATCH v5 2/8] drm/i915/uc: Disable GuC submission during sanitize
  2018-03-27 20:35 [PATCH v5 1/8] drm/i915: Correctly handle error path in i915_gem_init_hw Michal Wajdeczko
@ 2018-03-27 20:35 ` Michal Wajdeczko
  2018-03-28  9:06   ` Sagar Arun Kamble
  2018-03-27 20:35 ` [PATCH v5 3/8] drm/i915/guc: Restore symmetric doorbell cleanup Michal Wajdeczko
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 16+ messages in thread
From: Michal Wajdeczko @ 2018-03-27 20:35 UTC (permalink / raw)
  To: intel-gfx

We should not leave GuC submission enabled after sanitize,
as we are going to reset all GuC/HuC hardware.

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_uc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 4aad844..ec90c81 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -330,6 +330,9 @@ void intel_uc_sanitize(struct drm_i915_private *i915)
 
 	GEM_BUG_ON(!HAS_GUC(i915));
 
+	if (USES_GUC_SUBMISSION(dev_priv))
+		intel_guc_submission_disable(guc);
+
 	guc_disable_communication(guc);
 
 	intel_huc_sanitize(huc);
-- 
1.9.1

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

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

* [PATCH v5 3/8] drm/i915/guc: Restore symmetric doorbell cleanup
  2018-03-27 20:35 [PATCH v5 1/8] drm/i915: Correctly handle error path in i915_gem_init_hw Michal Wajdeczko
  2018-03-27 20:35 ` [PATCH v5 2/8] drm/i915/uc: Disable GuC submission during sanitize Michal Wajdeczko
@ 2018-03-27 20:35 ` Michal Wajdeczko
  2018-03-28  9:20   ` Sagar Arun Kamble
  2018-03-27 20:35 ` [PATCH v5 4/8] drm/i915/uc: Fully sanitize uC in uc_fini_hw Michal Wajdeczko
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 16+ messages in thread
From: Michal Wajdeczko @ 2018-03-27 20:35 UTC (permalink / raw)
  To: intel-gfx

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>
---
 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 207cda0..26985d8 100644
--- a/drivers/gpu/drm/i915/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/intel_guc_submission.c
@@ -835,18 +835,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

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

* [PATCH v5 4/8] drm/i915/uc: Fully sanitize uC in uc_fini_hw
  2018-03-27 20:35 [PATCH v5 1/8] drm/i915: Correctly handle error path in i915_gem_init_hw Michal Wajdeczko
  2018-03-27 20:35 ` [PATCH v5 2/8] drm/i915/uc: Disable GuC submission during sanitize Michal Wajdeczko
  2018-03-27 20:35 ` [PATCH v5 3/8] drm/i915/guc: Restore symmetric doorbell cleanup Michal Wajdeczko
@ 2018-03-27 20:35 ` Michal Wajdeczko
  2018-03-28  9:23   ` Sagar Arun Kamble
  2018-03-27 20:35 ` [PATCH v5 5/8] drm/i915/uc: Use correct error code for GuC initialization failure Michal Wajdeczko
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 16+ messages in thread
From: Michal Wajdeczko @ 2018-03-27 20:35 UTC (permalink / raw)
  To: intel-gfx

Today uc_fini_hw is subset of uc_sanitize, but remaining
code in sanitize function is also desired for uc_fini_hw.
Instead of duplicating the code, just call uc_sanitize,
but leave as separate function to maintain symmetry with
uc_init_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>
---
 drivers/gpu/drm/i915/intel_uc.c | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index ec90c81..46c4dc4 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -433,19 +433,9 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 	return ret;
 }
 
-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;
-
-	if (!USES_GUC(dev_priv))
-		return;
-
-	GEM_BUG_ON(!HAS_GUC(dev_priv));
-
-	if (USES_GUC_SUBMISSION(dev_priv))
-		intel_guc_submission_disable(guc);
-
-	guc_disable_communication(guc);
+	intel_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

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

* [PATCH v5 5/8] drm/i915/uc: Use correct error code for GuC initialization failure
  2018-03-27 20:35 [PATCH v5 1/8] drm/i915: Correctly handle error path in i915_gem_init_hw Michal Wajdeczko
                   ` (2 preceding siblings ...)
  2018-03-27 20:35 ` [PATCH v5 4/8] drm/i915/uc: Fully sanitize uC in uc_fini_hw Michal Wajdeczko
@ 2018-03-27 20:35 ` Michal Wajdeczko
  2018-03-28  9:31   ` Sagar Arun Kamble
  2018-03-27 20:35 ` [PATCH v5 6/8] drm/i915/uc: Use helper functions to detect fw load status Michal Wajdeczko
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 16+ messages in thread
From: Michal Wajdeczko @ 2018-03-27 20:35 UTC (permalink / raw)
  To: intel-gfx

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

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>
---
 drivers/gpu/drm/i915/i915_gem.c    | 17 ++++++++++-------
 drivers/gpu/drm/i915/intel_guc.h   |  5 +++++
 drivers/gpu/drm/i915/intel_uc.c    | 17 +++++++++++++----
 drivers/gpu/drm/i915/intel_uc_fw.h |  5 +++++
 4 files changed, 33 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 65ba104..82000bd 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5361,8 +5361,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)
@@ -5409,7 +5411,8 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
 	i915_gem_contexts_lost(dev_priv);
 	intel_uc_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);
@@ -5423,15 +5426,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 13f3d1d..7325b0e 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -155,6 +155,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 46c4dc4..2d8f5d9 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -426,15 +426,24 @@ 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 */
+	intel_huc_sanitize(huc);
+	intel_guc_sanitize(guc);
+	GEM_BUG_ON(intel_guc_is_loaded(guc));
+
+	/* We want to disable GPU submission but keep KMS alive */
+	return -EIO;
 }
 
 void intel_uc_fini_hw(struct drm_i915_private *i915)
 {
+	struct intel_guc *guc = &i915->guc;
+
+	if (!intel_guc_is_loaded(guc))
+		return;
+
 	intel_uc_sanitize(i915);
 }
 
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

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

* [PATCH v5 6/8] drm/i915/uc: Use helper functions to detect fw load status
  2018-03-27 20:35 [PATCH v5 1/8] drm/i915: Correctly handle error path in i915_gem_init_hw Michal Wajdeczko
                   ` (3 preceding siblings ...)
  2018-03-27 20:35 ` [PATCH v5 5/8] drm/i915/uc: Use correct error code for GuC initialization failure Michal Wajdeczko
@ 2018-03-27 20:35 ` Michal Wajdeczko
  2018-03-27 20:35 ` [PATCH v5 7/8] drm/i915/uc: Trivial s/dev_priv/i915 in intel_uc.c Michal Wajdeczko
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Michal Wajdeczko @ 2018-03-27 20:35 UTC (permalink / raw)
  To: intel-gfx

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 2d8f5d9..14664fe 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -455,7 +455,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);
@@ -477,7 +477,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;
 
 	if (i915_modparams.guc_log_level)
-- 
1.9.1

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

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

* [PATCH v5 7/8] drm/i915/uc: Trivial s/dev_priv/i915 in intel_uc.c
  2018-03-27 20:35 [PATCH v5 1/8] drm/i915: Correctly handle error path in i915_gem_init_hw Michal Wajdeczko
                   ` (4 preceding siblings ...)
  2018-03-27 20:35 ` [PATCH v5 6/8] drm/i915/uc: Use helper functions to detect fw load status Michal Wajdeczko
@ 2018-03-27 20:35 ` Michal Wajdeczko
  2018-03-27 20:35 ` [PATCH v5 8/8] HAX: Enable GuC for CI Michal Wajdeczko
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 16+ messages in thread
From: Michal Wajdeczko @ 2018-03-27 20:35 UTC (permalink / raw)
  To: intel-gfx

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 | 87 ++++++++++++++++++++---------------------
 1 file changed, 43 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 14664fe..fec5514 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;
@@ -238,22 +237,22 @@ 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;
 }
 
-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);
@@ -265,32 +264,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
@@ -305,16 +304,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);
@@ -330,7 +329,7 @@ void intel_uc_sanitize(struct drm_i915_private *i915)
 
 	GEM_BUG_ON(!HAS_GUC(i915));
 
-	if (USES_GUC_SUBMISSION(dev_priv))
+	if (USES_GUC_SUBMISSION(i915))
 		intel_guc_submission_disable(guc);
 
 	guc_disable_communication(guc);
-- 
1.9.1

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

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

* [PATCH v5 8/8] HAX: Enable GuC for CI
  2018-03-27 20:35 [PATCH v5 1/8] drm/i915: Correctly handle error path in i915_gem_init_hw Michal Wajdeczko
                   ` (5 preceding siblings ...)
  2018-03-27 20:35 ` [PATCH v5 7/8] drm/i915/uc: Trivial s/dev_priv/i915 in intel_uc.c Michal Wajdeczko
@ 2018-03-27 20:35 ` Michal Wajdeczko
  2018-03-28 15:51   ` Sagar Arun Kamble
  2018-03-27 21:10 ` ✓ Fi.CI.BAT: success for series starting with [v5,1/8] drm/i915: Correctly handle error path in i915_gem_init_hw Patchwork
  2018-03-28  5:21 ` ✗ Fi.CI.IGT: failure " Patchwork
  8 siblings, 1 reply; 16+ messages in thread
From: Michal Wajdeczko @ 2018-03-27 20:35 UTC (permalink / raw)
  To: intel-gfx

v2: except running with HYPERVISOR

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
---
 drivers/gpu/drm/i915/i915_params.h | 2 +-
 drivers/gpu/drm/i915/intel_uc.c    | 2 ++
 2 files changed, 3 insertions(+), 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) \
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index fec5514..bbb2c36 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -63,6 +63,8 @@ static int __get_platform_enable_guc(struct drm_i915_private *i915)
 		enable_guc |= ENABLE_GUC_LOAD_HUC;
 
 	/* Any platform specific fine-tuning can be done here */
+	if (boot_cpu_has(X86_FEATURE_HYPERVISOR))
+		enable_guc = 0;
 
 	return enable_guc;
 }
-- 
1.9.1

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

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

* ✓ Fi.CI.BAT: success for series starting with [v5,1/8] drm/i915: Correctly handle error path in i915_gem_init_hw
  2018-03-27 20:35 [PATCH v5 1/8] drm/i915: Correctly handle error path in i915_gem_init_hw Michal Wajdeczko
                   ` (6 preceding siblings ...)
  2018-03-27 20:35 ` [PATCH v5 8/8] HAX: Enable GuC for CI Michal Wajdeczko
@ 2018-03-27 21:10 ` Patchwork
  2018-03-28  5:21 ` ✗ Fi.CI.IGT: failure " Patchwork
  8 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2018-03-27 21:10 UTC (permalink / raw)
  To: Michal Wajdeczko; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v5,1/8] drm/i915: Correctly handle error path in i915_gem_init_hw
URL   : https://patchwork.freedesktop.org/series/40759/
State : success

== Summary ==

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

---- Known issues:

Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                pass       -> INCOMPLETE (fi-snb-2520m) fdo#103713

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

fi-bdw-5557u     total:285  pass:264  dwarn:0   dfail:0   fail:0   skip:21  time:434s
fi-bdw-gvtdvm    total:285  pass:261  dwarn:0   dfail:0   fail:0   skip:24  time:446s
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:538s
fi-bwr-2160      total:285  pass:180  dwarn:0   dfail:0   fail:0   skip:105 time:298s
fi-bxt-j4205     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:517s
fi-byt-j1900     total:285  pass:250  dwarn:0   dfail:0   fail:0   skip:35  time:523s
fi-byt-n2820     total:285  pass:246  dwarn:0   dfail:0   fail:0   skip:39  time:512s
fi-cfl-8700k     total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:415s
fi-cfl-s3        total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:568s
fi-cfl-u         total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:517s
fi-cnl-y3        total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:578s
fi-elk-e7500     total:285  pass:225  dwarn:1   dfail:0   fail:0   skip:59  time:430s
fi-gdg-551       total:285  pass:176  dwarn:0   dfail:0   fail:1   skip:108 time:325s
fi-glk-1         total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:536s
fi-hsw-4770      total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:408s
fi-ilk-650       total:285  pass:225  dwarn:0   dfail:0   fail:0   skip:60  time:420s
fi-ivb-3520m     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:476s
fi-ivb-3770      total:285  pass:252  dwarn:0   dfail:0   fail:0   skip:33  time:432s
fi-kbl-7500u     total:285  pass:260  dwarn:1   dfail:0   fail:0   skip:24  time:477s
fi-kbl-7567u     total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:468s
fi-kbl-r         total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:513s
fi-pnv-d510      total:285  pass:219  dwarn:1   dfail:0   fail:0   skip:65  time:666s
fi-skl-6260u     total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:444s
fi-skl-6600u     total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:544s
fi-skl-6700k2    total:285  pass:261  dwarn:0   dfail:0   fail:0   skip:24  time:504s
fi-skl-6770hq    total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:498s
fi-skl-guc       total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:432s
fi-skl-gvtdvm    total:285  pass:262  dwarn:0   dfail:0   fail:0   skip:23  time:447s
fi-snb-2520m     total:242  pass:208  dwarn:0   dfail:0   fail:0   skip:33 
Blacklisted hosts:
fi-cnl-psr       total:285  pass:256  dwarn:3   dfail:0   fail:0   skip:26  time:531s
fi-glk-j4005     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:489s

0539b52e05cd0abe697d45f2a2373ec42af7ebcb drm-tip: 2018y-03m-27d-18h-45m-40s UTC integration manifest
144a5f9809e1 HAX: Enable GuC for CI
d5e88b21ece7 drm/i915/uc: Trivial s/dev_priv/i915 in intel_uc.c
a8b655ca5aa7 drm/i915/uc: Use helper functions to detect fw load status
3217ab178f2a drm/i915/uc: Use correct error code for GuC initialization failure
6eb5c9f7705a drm/i915/uc: Fully sanitize uC in uc_fini_hw
24f107d1ea1f drm/i915/guc: Restore symmetric doorbell cleanup
25a5033a07f3 drm/i915/uc: Disable GuC submission during sanitize
1dd9d26952a0 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_8506/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* ✗ Fi.CI.IGT: failure for series starting with [v5,1/8] drm/i915: Correctly handle error path in i915_gem_init_hw
  2018-03-27 20:35 [PATCH v5 1/8] drm/i915: Correctly handle error path in i915_gem_init_hw Michal Wajdeczko
                   ` (7 preceding siblings ...)
  2018-03-27 21:10 ` ✓ Fi.CI.BAT: success for series starting with [v5,1/8] drm/i915: Correctly handle error path in i915_gem_init_hw Patchwork
@ 2018-03-28  5:21 ` Patchwork
  2018-03-28  9:38   ` Chris Wilson
  8 siblings, 1 reply; 16+ messages in thread
From: Patchwork @ 2018-03-28  5:21 UTC (permalink / raw)
  To: Michal Wajdeczko; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v5,1/8] drm/i915: Correctly handle error path in i915_gem_init_hw
URL   : https://patchwork.freedesktop.org/series/40759/
State : failure

== Summary ==

---- Possible new issues:

Test drm_mm:
        Subgroup sanitycheck:
                pass       -> INCOMPLETE (shard-apl)
Test drv_hangman:
        Subgroup error-state-basic:
                pass       -> INCOMPLETE (shard-apl)
        Subgroup error-state-capture-blt:
                pass       -> INCOMPLETE (shard-apl)
        Subgroup hangcheck-unterminated:
                pass       -> INCOMPLETE (shard-apl)
Test drv_selftest:
        Subgroup live_guc:
                pass       -> SKIP       (shard-apl)
        Subgroup live_hangcheck:
                pass       -> DMESG-FAIL (shard-apl)
Test gem_eio:
        Subgroup execbuf:
                pass       -> INCOMPLETE (shard-apl)
        Subgroup in-flight-external:
                pass       -> INCOMPLETE (shard-apl)
Test gem_mocs_settings:
        Subgroup mocs-reset-vebox:
                pass       -> INCOMPLETE (shard-apl)
Test gem_ringfill:
        Subgroup basic-default-hang:
                pass       -> INCOMPLETE (shard-apl)
Test kms_cursor_crc:
        Subgroup cursor-128x128-suspend:
                dmesg-warn -> PASS       (shard-snb)
Test kms_flip:
        Subgroup flip-vs-panning-vs-hang-interruptible:
                pass       -> INCOMPLETE (shard-apl)
Test kms_frontbuffer_tracking:
        Subgroup fbc-1p-offscren-pri-shrfb-draw-mmap-cpu:
                pass       -> SKIP       (shard-snb)
Test kms_vblank:
        Subgroup pipe-a-query-idle-hang:
                pass       -> INCOMPLETE (shard-apl)
        Subgroup pipe-a-wait-forked-busy:
                pass       -> SKIP       (shard-snb)
        Subgroup pipe-c-query-busy-hang:
                pass       -> INCOMPLETE (shard-apl)
        Subgroup pipe-c-query-forked-hang:
                pass       -> INCOMPLETE (shard-apl)
Test perf:
        Subgroup gen8-unprivileged-single-ctx-counters:
                pass       -> FAIL       (shard-apl)
Test pm_rpm:
        Subgroup debugfs-read:
                pass       -> DMESG-WARN (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
Test kms_flip:
        Subgroup 2x-dpms-vs-vblank-race-interruptible:
                fail       -> PASS       (shard-hsw) fdo#103060
        Subgroup 2x-flip-vs-expired-vblank-interruptible:
                fail       -> PASS       (shard-hsw) fdo#102887 +1
        Subgroup 2x-plain-flip-ts-check:
                pass       -> FAIL       (shard-hsw) fdo#100368
Test kms_rotation_crc:
        Subgroup sprite-rotation-180:
                pass       -> FAIL       (shard-snb) fdo#103925
Test perf:
        Subgroup polling:
                fail       -> PASS       (shard-hsw) fdo#102252

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#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925
fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252

shard-apl        total:2115 pass:1059 dwarn:2   dfail:1   fail:5   skip:1022 time:3044s
shard-hsw        total:3495 pass:1782 dwarn:1   dfail:0   fail:2   skip:1709 time:11596s
shard-snb        total:3495 pass:1371 dwarn:1   dfail:0   fail:4   skip:2119 time:6949s
Blacklisted hosts:
shard-kbl        total:2098 pass:1095 dwarn:3   dfail:1   fail:3   skip:971 time:2232s

== Logs ==

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

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

* Re: [PATCH v5 2/8] drm/i915/uc: Disable GuC submission during sanitize
  2018-03-27 20:35 ` [PATCH v5 2/8] drm/i915/uc: Disable GuC submission during sanitize Michal Wajdeczko
@ 2018-03-28  9:06   ` Sagar Arun Kamble
  0 siblings, 0 replies; 16+ messages in thread
From: Sagar Arun Kamble @ 2018-03-28  9:06 UTC (permalink / raw)
  To: Michal Wajdeczko, intel-gfx



On 3/28/2018 2:05 AM, Michal Wajdeczko wrote:
> We should not leave GuC submission enabled after sanitize,
> as we are going to reset all GuC/HuC hardware.
>
> 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_uc.c | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
> index 4aad844..ec90c81 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -330,6 +330,9 @@ void intel_uc_sanitize(struct drm_i915_private *i915)
>   
>   	GEM_BUG_ON(!HAS_GUC(i915));
>   
> +	if (USES_GUC_SUBMISSION(dev_priv))
> +		intel_guc_submission_disable(guc);
> +
>   	guc_disable_communication(guc);
>   
>   	intel_huc_sanitize(huc);

-- 
Thanks,
Sagar

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

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

* Re: [PATCH v5 3/8] drm/i915/guc: Restore symmetric doorbell cleanup
  2018-03-27 20:35 ` [PATCH v5 3/8] drm/i915/guc: Restore symmetric doorbell cleanup Michal Wajdeczko
@ 2018-03-28  9:20   ` Sagar Arun Kamble
  0 siblings, 0 replies; 16+ messages in thread
From: Sagar Arun Kamble @ 2018-03-28  9:20 UTC (permalink / raw)
  To: Michal Wajdeczko, intel-gfx



On 3/28/2018 2:05 AM, Michal Wajdeczko wrote:
> 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>
This looks good.
Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>

We should extend this functionality further to consider cases when GuC 
doorbell deactivation flow is to be
invoked (when GuC is in sane state) and not to be invoked (when GuC is 
hung) as was prepared in patchset
https://patchwork.freedesktop.org/series/33209/ but that can be done 
after this series if we agree on need for such handling.

Thanks,
Sagar
> ---
>   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 207cda0..26985d8 100644
> --- a/drivers/gpu/drm/i915/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/intel_guc_submission.c
> @@ -835,18 +835,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);
>   }
>   
>   /**

-- 
Thanks,
Sagar

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

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

* Re: [PATCH v5 4/8] drm/i915/uc: Fully sanitize uC in uc_fini_hw
  2018-03-27 20:35 ` [PATCH v5 4/8] drm/i915/uc: Fully sanitize uC in uc_fini_hw Michal Wajdeczko
@ 2018-03-28  9:23   ` Sagar Arun Kamble
  0 siblings, 0 replies; 16+ messages in thread
From: Sagar Arun Kamble @ 2018-03-28  9:23 UTC (permalink / raw)
  To: Michal Wajdeczko, intel-gfx



On 3/28/2018 2:05 AM, Michal Wajdeczko wrote:
> Today uc_fini_hw is subset of uc_sanitize, but remaining
> code in sanitize function is also desired for uc_fini_hw.
> Instead of duplicating the code, just call uc_sanitize,
> but leave as separate function to maintain symmetry with
> uc_init_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/intel_uc.c | 14 ++------------
>   1 file changed, 2 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
> index ec90c81..46c4dc4 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -433,19 +433,9 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
>   	return ret;
>   }
>   
> -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;
> -
> -	if (!USES_GUC(dev_priv))
> -		return;
> -
> -	GEM_BUG_ON(!HAS_GUC(dev_priv));
> -
> -	if (USES_GUC_SUBMISSION(dev_priv))
> -		intel_guc_submission_disable(guc);
> -
> -	guc_disable_communication(guc);
> +	intel_uc_sanitize(i915);
>   }
>   
>   int intel_uc_suspend(struct drm_i915_private *i915)

-- 
Thanks,
Sagar

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

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

* Re: [PATCH v5 5/8] drm/i915/uc: Use correct error code for GuC initialization failure
  2018-03-27 20:35 ` [PATCH v5 5/8] drm/i915/uc: Use correct error code for GuC initialization failure Michal Wajdeczko
@ 2018-03-28  9:31   ` Sagar Arun Kamble
  0 siblings, 0 replies; 16+ messages in thread
From: Sagar Arun Kamble @ 2018-03-28  9:31 UTC (permalink / raw)
  To: Michal Wajdeczko, intel-gfx



On 3/28/2018 2:05 AM, Michal Wajdeczko wrote:
> 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
>
> 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>
<snip>
>   void intel_uc_fini_hw(struct drm_i915_private *i915)
>   {
> +	struct intel_guc *guc = &i915->guc;
> +
> +	if (!intel_guc_is_loaded(guc))
> +		return;
I feel above guc_is_loaded check is more applicable in uc_sanitize. So 
callers won't have to bother if GuC is loaded or not.
w/ or w/o that change patch looks good though.
Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> +
>   	intel_uc_sanitize(i915);
>   }
>   
> 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.

-- 
Thanks,
Sagar

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

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

* Re: ✗ Fi.CI.IGT: failure for series starting with [v5,1/8] drm/i915: Correctly handle error path in i915_gem_init_hw
  2018-03-28  5:21 ` ✗ Fi.CI.IGT: failure " Patchwork
@ 2018-03-28  9:38   ` Chris Wilson
  0 siblings, 0 replies; 16+ messages in thread
From: Chris Wilson @ 2018-03-28  9:38 UTC (permalink / raw)
  To: Patchwork, Michal Wajdeczko; +Cc: intel-gfx

Quoting Patchwork (2018-03-28 06:21:58)
> == Series Details ==
> 
> Series: series starting with [v5,1/8] drm/i915: Correctly handle error path in i915_gem_init_hw
> URL   : https://patchwork.freedesktop.org/series/40759/
> State : failure
> 
> == Summary ==
> 
> ---- Possible new issues:
> 
> Test drm_mm:
>         Subgroup sanitycheck:
>                 pass       -> INCOMPLETE (shard-apl)

<0>[  395.621771] drv_self-5796    0.... 395583503us : intel_guc_submission_disable: intel_guc_submission_disable:1242 GEM_BUG_ON(dev_priv->gt.awake)

> Test drv_hangman:
>         Subgroup error-state-basic:
>                 pass       -> INCOMPLETE (shard-apl)
>         Subgroup error-state-capture-blt:
>                 pass       -> INCOMPLETE (shard-apl)
>         Subgroup hangcheck-unterminated:
>                 pass       -> INCOMPLETE (shard-apl)

> Test gem_eio:
>         Subgroup execbuf:
>                 pass       -> INCOMPLETE (shard-apl)
>         Subgroup in-flight-external:
>                 pass       -> INCOMPLETE (shard-apl)

Wedged the machine (which then hit the same bug as gem_eio where we not
calling reset_default_submission from a safe point).

> Test gem_mocs_settings:
>         Subgroup mocs-reset-vebox:
>                 pass       -> INCOMPLETE (shard-apl)
> Test gem_ringfill:
>         Subgroup basic-default-hang:
>                 pass       -> INCOMPLETE (shard-apl)
> Test kms_flip:
>         Subgroup flip-vs-panning-vs-hang-interruptible:
>                 pass       -> INCOMPLETE (shard-apl)
> Test kms_frontbuffer_tracking:
>         Subgroup fbc-1p-offscren-pri-shrfb-draw-mmap-cpu:
>                 pass       -> SKIP       (shard-snb)
> Test kms_vblank:
>         Subgroup pipe-a-query-idle-hang:
>                 pass       -> INCOMPLETE (shard-apl)
>         Subgroup pipe-a-wait-forked-busy:
>                 pass       -> SKIP       (shard-snb)
>         Subgroup pipe-c-query-busy-hang:
>                 pass       -> INCOMPLETE (shard-apl)
>         Subgroup pipe-c-query-forked-hang:
>                 pass       -> INCOMPLETE (shard-apl)

More
<0>[  395.621771] drv_self-5796    0.... 395583503us : intel_guc_submission_disable: intel_guc_submission_disable:1242 GEM_BUG_ON(dev_priv->gt.awake)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v5 8/8] HAX: Enable GuC for CI
  2018-03-27 20:35 ` [PATCH v5 8/8] HAX: Enable GuC for CI Michal Wajdeczko
@ 2018-03-28 15:51   ` Sagar Arun Kamble
  0 siblings, 0 replies; 16+ messages in thread
From: Sagar Arun Kamble @ 2018-03-28 15:51 UTC (permalink / raw)
  To: Michal Wajdeczko, intel-gfx



On 3/28/2018 2:05 AM, Michal Wajdeczko wrote:
> v2: except running with HYPERVISOR
>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_params.h | 2 +-
>   drivers/gpu/drm/i915/intel_uc.c    | 2 ++
>   2 files changed, 3 insertions(+), 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) \
> diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
> index fec5514..bbb2c36 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -63,6 +63,8 @@ static int __get_platform_enable_guc(struct drm_i915_private *i915)
>   		enable_guc |= ENABLE_GUC_LOAD_HUC;
>   
>   	/* Any platform specific fine-tuning can be done here */
> +	if (boot_cpu_has(X86_FEATURE_HYPERVISOR))
> +		enable_guc = 0;
>   
This is not needed now that GuC logging is fixed.
>   	return enable_guc;
>   }

-- 
Thanks,
Sagar

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

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

end of thread, other threads:[~2018-03-28 15:51 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-27 20:35 [PATCH v5 1/8] drm/i915: Correctly handle error path in i915_gem_init_hw Michal Wajdeczko
2018-03-27 20:35 ` [PATCH v5 2/8] drm/i915/uc: Disable GuC submission during sanitize Michal Wajdeczko
2018-03-28  9:06   ` Sagar Arun Kamble
2018-03-27 20:35 ` [PATCH v5 3/8] drm/i915/guc: Restore symmetric doorbell cleanup Michal Wajdeczko
2018-03-28  9:20   ` Sagar Arun Kamble
2018-03-27 20:35 ` [PATCH v5 4/8] drm/i915/uc: Fully sanitize uC in uc_fini_hw Michal Wajdeczko
2018-03-28  9:23   ` Sagar Arun Kamble
2018-03-27 20:35 ` [PATCH v5 5/8] drm/i915/uc: Use correct error code for GuC initialization failure Michal Wajdeczko
2018-03-28  9:31   ` Sagar Arun Kamble
2018-03-27 20:35 ` [PATCH v5 6/8] drm/i915/uc: Use helper functions to detect fw load status Michal Wajdeczko
2018-03-27 20:35 ` [PATCH v5 7/8] drm/i915/uc: Trivial s/dev_priv/i915 in intel_uc.c Michal Wajdeczko
2018-03-27 20:35 ` [PATCH v5 8/8] HAX: Enable GuC for CI Michal Wajdeczko
2018-03-28 15:51   ` Sagar Arun Kamble
2018-03-27 21:10 ` ✓ Fi.CI.BAT: success for series starting with [v5,1/8] drm/i915: Correctly handle error path in i915_gem_init_hw Patchwork
2018-03-28  5:21 ` ✗ Fi.CI.IGT: failure " Patchwork
2018-03-28  9:38   ` Chris Wilson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.