All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 0/7] GuC Fixes, HuC auth. reorg and v9+ logging change
@ 2017-09-22  4:49 Sagar Arun Kamble
  2017-09-22  4:49 ` [PATCH v6 1/7] drm/i915: Create uc runtime and system suspend/resume helpers Sagar Arun Kamble
                   ` (8 more replies)
  0 siblings, 9 replies; 15+ messages in thread
From: Sagar Arun Kamble @ 2017-09-22  4:49 UTC (permalink / raw)
  To: intel-gfx

Updated patch 3 and patch 5.

Sagar Arun Kamble (7):
  drm/i915: Create uc runtime and system suspend/resume helpers
  drm/i915/guc: Update GuC ggtt.invalidate/interrupts/communication
    across RPM suspend/resume
  drm/i915/guc: Update GuC suspend functionality in intel_uc_suspend
    path
  drm/i915/guc: Disable GuC submission and suspend it prior to i915
    reset
  drm/i915/guc: Fix GuC cleanup in unload path
  drm/i915/guc: Enable default/critical logging in GuC by default from
    GuC v9
  drm/i915: Reorganize HuC authentication

 drivers/gpu/drm/i915/i915_drv.c            |  25 +++++--
 drivers/gpu/drm/i915/i915_gem.c            |   9 ++-
 drivers/gpu/drm/i915/i915_guc_submission.c |  22 +++---
 drivers/gpu/drm/i915/intel_guc_fwif.h      |   4 +-
 drivers/gpu/drm/i915/intel_guc_log.c       |  13 +++-
 drivers/gpu/drm/i915/intel_huc.c           |  22 ++----
 drivers/gpu/drm/i915/intel_uc.c            | 104 +++++++++++++++++++++++++----
 drivers/gpu/drm/i915/intel_uc.h            |  12 +++-
 8 files changed, 163 insertions(+), 48 deletions(-)

-- 
1.9.1

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

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

* [PATCH v6 1/7] drm/i915: Create uc runtime and system suspend/resume helpers
  2017-09-22  4:49 [PATCH v6 0/7] GuC Fixes, HuC auth. reorg and v9+ logging change Sagar Arun Kamble
@ 2017-09-22  4:49 ` Sagar Arun Kamble
  2017-09-22  7:11   ` Michal Wajdeczko
  2017-09-22  4:49 ` [PATCH v6 2/7] drm/i915/guc: Update GuC ggtt.invalidate/interrupts/communication across RPM suspend/resume Sagar Arun Kamble
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 15+ messages in thread
From: Sagar Arun Kamble @ 2017-09-22  4:49 UTC (permalink / raw)
  To: intel-gfx

Prepared generic helpers intel_uc_suspend, intel_uc_resume,
intel_uc_runtime_suspend, intel_uc_runtime_resume. Added
error handling to all the calls for suspend/resume.

v2: Rebase w.r.t removal of GuC code restructuring.

Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 23 ++++++++++++++++++++---
 drivers/gpu/drm/i915/i915_gem.c |  7 ++++++-
 drivers/gpu/drm/i915/intel_uc.c | 20 ++++++++++++++++++++
 drivers/gpu/drm/i915/intel_uc.h |  4 ++++
 4 files changed, 50 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 5c111ea..8635f40 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1691,7 +1691,15 @@ static int i915_drm_resume(struct drm_device *dev)
 	}
 	mutex_unlock(&dev->struct_mutex);
 
-	intel_guc_resume(dev_priv);
+	/*
+	 * NB: Full gem reinitialization is being done above, hence
+	 * intel_uc_resume will be of no use. Currently intel_uc_resume
+	 * is nop. If full reinitialization is removed, will  need to put
+	 * functionality to resume from sleep in intel_uc_resume.
+	 */
+	ret = intel_uc_resume(dev_priv);
+	if (ret)
+		DRM_ERROR("failed to resume uc\n");
 
 	intel_modeset_init_hw(dev);
 
@@ -2493,7 +2501,12 @@ static int intel_runtime_suspend(struct device *kdev)
 	 */
 	i915_gem_runtime_suspend(dev_priv);
 
-	intel_guc_suspend(dev_priv);
+	ret = intel_uc_runtime_suspend(dev_priv);
+	if (ret) {
+		DRM_ERROR("uc runtime suspend failed, disabling it(%d)\n", ret);
+		enable_rpm_wakeref_asserts(dev_priv);
+		return ret;
+	}
 
 	intel_runtime_pm_disable_interrupts(dev_priv);
 
@@ -2578,7 +2591,11 @@ static int intel_runtime_resume(struct device *kdev)
 	if (intel_uncore_unclaimed_mmio(dev_priv))
 		DRM_DEBUG_DRIVER("Unclaimed access during suspend, bios?\n");
 
-	intel_guc_resume(dev_priv);
+	ret = intel_uc_runtime_resume(dev_priv);
+	if (ret) {
+		DRM_ERROR("uc runtime resume failed (%d)\n", ret);
+		return ret;
+	}
 
 	if (IS_GEN9_LP(dev_priv)) {
 		bxt_disable_dc9(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index c4bf348..dd56d45 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4539,7 +4539,11 @@ int i915_gem_suspend(struct drm_i915_private *dev_priv)
 	i915_gem_contexts_lost(dev_priv);
 	mutex_unlock(&dev->struct_mutex);
 
-	intel_guc_suspend(dev_priv);
+	ret = intel_uc_suspend(dev_priv);
+	if (ret) {
+		DRM_ERROR("uc suspend failed (%d)\n", ret);
+		goto out;
+	}
 
 	cancel_delayed_work_sync(&dev_priv->gpu_error.hangcheck_work);
 	cancel_delayed_work_sync(&dev_priv->gt.retire_work);
@@ -4583,6 +4587,7 @@ int i915_gem_suspend(struct drm_i915_private *dev_priv)
 
 err_unlock:
 	mutex_unlock(&dev->struct_mutex);
+out:
 	intel_runtime_pm_put(dev_priv);
 	return ret;
 }
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 0178ba4..8e4d8b0 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -537,3 +537,23 @@ int intel_guc_sample_forcewake(struct intel_guc *guc)
 
 	return intel_guc_send(guc, action, ARRAY_SIZE(action));
 }
+
+int intel_uc_runtime_suspend(struct drm_i915_private *dev_priv)
+{
+	return intel_guc_suspend(dev_priv);
+}
+
+int intel_uc_runtime_resume(struct drm_i915_private *dev_priv)
+{
+	return intel_guc_resume(dev_priv);
+}
+
+int intel_uc_suspend(struct drm_i915_private *dev_priv)
+{
+	return intel_guc_suspend(dev_priv);
+}
+
+int intel_uc_resume(struct drm_i915_private *dev_priv)
+{
+	return 0;
+}
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 7703c9a..3d33a51 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -208,6 +208,10 @@ struct intel_huc {
 void intel_uc_fini_fw(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_runtime_suspend(struct drm_i915_private *dev_priv);
+int intel_uc_runtime_resume(struct drm_i915_private *dev_priv);
+int intel_uc_suspend(struct drm_i915_private *dev_priv);
+int intel_uc_resume(struct drm_i915_private *dev_priv);
 int intel_guc_sample_forcewake(struct intel_guc *guc);
 int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len);
 int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len);
-- 
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] 15+ messages in thread

* [PATCH v6 2/7] drm/i915/guc: Update GuC ggtt.invalidate/interrupts/communication across RPM suspend/resume
  2017-09-22  4:49 [PATCH v6 0/7] GuC Fixes, HuC auth. reorg and v9+ logging change Sagar Arun Kamble
  2017-09-22  4:49 ` [PATCH v6 1/7] drm/i915: Create uc runtime and system suspend/resume helpers Sagar Arun Kamble
@ 2017-09-22  4:49 ` Sagar Arun Kamble
  2017-09-22  7:38   ` Michal Wajdeczko
  2017-09-22  4:49 ` [PATCH v6 3/7] drm/i915/guc: Update GuC suspend functionality in intel_uc_suspend path Sagar Arun Kamble
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 15+ messages in thread
From: Sagar Arun Kamble @ 2017-09-22  4:49 UTC (permalink / raw)
  To: intel-gfx

Apart from configuring interrupts, we need to update the ggtt invalidate
interface and GuC communication on suspend. This functionality can be
reused for other suspend and reset paths.

v2: Rebase w.r.t removal of GuC code restructuring.

v3: Removed GuC specific helpers as tasks other than send H2G for
sleep/resume are to be done from uc generic functions. (Michal)

Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/i915_guc_submission.c |  5 ----
 drivers/gpu/drm/i915/intel_uc.c            | 39 ++++++++++++++++++++++++++++--
 2 files changed, 37 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index e191d56..cd5ef8b 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -1219,8 +1219,6 @@ int intel_guc_suspend(struct drm_i915_private *dev_priv)
 	if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
 		return 0;
 
-	gen9_disable_guc_interrupts(dev_priv);
-
 	ctx = dev_priv->kernel_context;
 
 	data[0] = INTEL_GUC_ACTION_ENTER_S_STATE;
@@ -1245,9 +1243,6 @@ int intel_guc_resume(struct drm_i915_private *dev_priv)
 	if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
 		return 0;
 
-	if (i915.guc_log_level >= 0)
-		gen9_enable_guc_interrupts(dev_priv);
-
 	ctx = dev_priv->kernel_context;
 
 	data[0] = INTEL_GUC_ACTION_EXIT_S_STATE;
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 8e4d8b0..66d2cff 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -540,12 +540,47 @@ int intel_guc_sample_forcewake(struct intel_guc *guc)
 
 int intel_uc_runtime_suspend(struct drm_i915_private *dev_priv)
 {
-	return intel_guc_suspend(dev_priv);
+	int ret;
+
+	if (!i915.enable_guc_loading)
+		return 0;
+
+	ret = intel_guc_suspend(dev_priv);
+	if (ret) {
+		DRM_ERROR("GuC runtime suspend failed (%d)\n", ret);
+		return ret;
+	}
+
+	i915_ggtt_disable_guc(dev_priv);
+	gen9_disable_guc_interrupts(dev_priv);
+	guc_disable_communication(&dev_priv->guc);
+
+	return 0;
 }
 
 int intel_uc_runtime_resume(struct drm_i915_private *dev_priv)
 {
-	return intel_guc_resume(dev_priv);
+	int ret;
+
+	if (!i915.enable_guc_loading)
+		return 0;
+
+	ret = guc_enable_communication(&dev_priv->guc);
+	if (ret) {
+		DRM_ERROR("GuC enable communication failed (%d)\n", ret);
+		return ret;
+	}
+	if (i915.guc_log_level >= 0)
+		gen9_enable_guc_interrupts(dev_priv);
+	i915_ggtt_enable_guc(dev_priv);
+
+	ret = intel_guc_resume(dev_priv);
+	if (ret) {
+		DRM_ERROR("GuC runtime resume failed (%d)\n", ret);
+		return ret;
+	}
+
+	return 0;
 }
 
 int intel_uc_suspend(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

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

* [PATCH v6 3/7] drm/i915/guc: Update GuC suspend functionality in intel_uc_suspend path
  2017-09-22  4:49 [PATCH v6 0/7] GuC Fixes, HuC auth. reorg and v9+ logging change Sagar Arun Kamble
  2017-09-22  4:49 ` [PATCH v6 1/7] drm/i915: Create uc runtime and system suspend/resume helpers Sagar Arun Kamble
  2017-09-22  4:49 ` [PATCH v6 2/7] drm/i915/guc: Update GuC ggtt.invalidate/interrupts/communication across RPM suspend/resume Sagar Arun Kamble
@ 2017-09-22  4:49 ` Sagar Arun Kamble
  2017-09-22  4:49 ` [PATCH v6 4/7] drm/i915/guc: Disable GuC submission and suspend it prior to i915 reset Sagar Arun Kamble
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Sagar Arun Kamble @ 2017-09-22  4:49 UTC (permalink / raw)
  To: intel-gfx

With this patch we disable GuC submission in i915_drm_suspend. This will
destroy the client which will be setup back again. We also reuse the
complete sanitization done via intel_uc_runtime_suspend in this path.
Post drm resume this state is recreated by intel_uc_init_hw hence we need
not have similar reuse for intel_uc_resume.
This also fixes issue where intel_uc_fini_hw was being called after GPU
reset happening in i915_gem_suspend in i915_driver_unload.

v2: Rebase w.r.t removal of GuC code restructuring. Added struct_mutex
protection for i915_guc_submission_disable.

v3: Rebase w.r.t updated GuC suspend function name.

v4: Added lockdep assert in i915_guc_submission_enable/disable. Refined
intel_uc_suspend to remove unnecessary locals and simplify return.
(Michal Winiarski)
Removed comment in guc_client_free about ignoring failure for
destroy_doorbell. (Oscar)

Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Reviewed-by: Michał Winiarski <michal.winiarski@intel.com>
Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/i915_guc_submission.c | 15 ++++++++++++---
 drivers/gpu/drm/i915/intel_uc.c            | 11 +++++++----
 2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index cd5ef8b..09fb156 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -879,9 +879,6 @@ static void guc_client_free(struct i915_guc_client *client)
 	 * Be sure to drop any locks
 	 */
 
-	/* FIXME: in many cases, by the time we get here the GuC has been
-	 * reset, so we cannot destroy the doorbell properly. Ignore the
-	 * error message for now */
 	destroy_doorbell(client);
 	guc_stage_desc_fini(client->guc, client);
 	i915_gem_object_unpin_map(client->vma->obj);
@@ -1148,6 +1145,12 @@ int i915_guc_submission_enable(struct drm_i915_private *dev_priv)
 		     sizeof(struct guc_wq_item) *
 		     I915_NUM_ENGINES > GUC_WQ_SIZE);
 
+	/*
+	 * Assert that drm.struct_mutex is held.
+	 * Needed for GuC client vma binding.
+	 */
+	lockdep_assert_held(&dev_priv->drm.struct_mutex);
+
 	if (!client) {
 		client = guc_client_alloc(dev_priv,
 					  INTEL_INFO(dev_priv)->ring_mask,
@@ -1197,6 +1200,12 @@ void i915_guc_submission_disable(struct drm_i915_private *dev_priv)
 {
 	struct intel_guc *guc = &dev_priv->guc;
 
+	/*
+	 * Assert that drm.struct_mutex is held.
+	 * Needed for GuC client vma unbinding.
+	 */
+	lockdep_assert_held(&dev_priv->drm.struct_mutex);
+
 	guc_interrupts_release(dev_priv);
 
 	/* Revert back to manual ELSP submission */
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 66d2cff..0e971d7 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -446,9 +446,6 @@ void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
 	if (!i915.enable_guc_loading)
 		return;
 
-	if (i915.enable_guc_submission)
-		i915_guc_submission_disable(dev_priv);
-
 	guc_disable_communication(&dev_priv->guc);
 
 	if (i915.enable_guc_submission) {
@@ -585,7 +582,13 @@ int intel_uc_runtime_resume(struct drm_i915_private *dev_priv)
 
 int intel_uc_suspend(struct drm_i915_private *dev_priv)
 {
-	return intel_guc_suspend(dev_priv);
+	if (i915.enable_guc_submission) {
+		mutex_lock(&dev_priv->drm.struct_mutex);
+		i915_guc_submission_disable(dev_priv);
+		mutex_unlock(&dev_priv->drm.struct_mutex);
+	}
+
+	return intel_uc_runtime_suspend(dev_priv);
 }
 
 int intel_uc_resume(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

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

* [PATCH v6 4/7] drm/i915/guc: Disable GuC submission and suspend it prior to i915 reset
  2017-09-22  4:49 [PATCH v6 0/7] GuC Fixes, HuC auth. reorg and v9+ logging change Sagar Arun Kamble
                   ` (2 preceding siblings ...)
  2017-09-22  4:49 ` [PATCH v6 3/7] drm/i915/guc: Update GuC suspend functionality in intel_uc_suspend path Sagar Arun Kamble
@ 2017-09-22  4:49 ` Sagar Arun Kamble
  2017-09-22  4:49 ` [PATCH v6 5/7] drm/i915/guc: Fix GuC cleanup in unload path Sagar Arun Kamble
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Sagar Arun Kamble @ 2017-09-22  4:49 UTC (permalink / raw)
  To: intel-gfx

Before i915 reset, we need to disable GuC submission and suspend GuC
operations as it is recreated during intel_uc_init_hw. We can't reuse the
intel_uc_suspend functionality as reset path already holds struct_mutex.

v2: Rebase w.r.t removal of GuC code restructuring. Updated reset_prepare
function as struct_mutex is not needed.

v3: Fixed typo in commit message. Made return from intel_uc_reset_prepare
simpler. (Michal)

Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 2 ++
 drivers/gpu/drm/i915/intel_uc.c | 8 ++++++++
 drivers/gpu/drm/i915/intel_uc.h | 1 +
 3 files changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index dd56d45..76e1bb2 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2847,6 +2847,8 @@ int i915_gem_reset_prepare(struct drm_i915_private *dev_priv)
 
 	i915_gem_revoke_fences(dev_priv);
 
+	intel_uc_reset_prepare(dev_priv);
+
 	return err;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 0e971d7..7137e38 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -595,3 +595,11 @@ int intel_uc_resume(struct drm_i915_private *dev_priv)
 {
 	return 0;
 }
+
+int intel_uc_reset_prepare(struct drm_i915_private *dev_priv)
+{
+	if (i915.enable_guc_submission)
+		i915_guc_submission_disable(dev_priv);
+
+	return intel_uc_runtime_suspend(dev_priv);
+}
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 3d33a51..7b9ac3e 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -212,6 +212,7 @@ struct intel_huc {
 int intel_uc_runtime_resume(struct drm_i915_private *dev_priv);
 int intel_uc_suspend(struct drm_i915_private *dev_priv);
 int intel_uc_resume(struct drm_i915_private *dev_priv);
+int intel_uc_reset_prepare(struct drm_i915_private *dev_priv);
 int intel_guc_sample_forcewake(struct intel_guc *guc);
 int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len);
 int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len);
-- 
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] 15+ messages in thread

* [PATCH v6 5/7] drm/i915/guc: Fix GuC cleanup in unload path
  2017-09-22  4:49 [PATCH v6 0/7] GuC Fixes, HuC auth. reorg and v9+ logging change Sagar Arun Kamble
                   ` (3 preceding siblings ...)
  2017-09-22  4:49 ` [PATCH v6 4/7] drm/i915/guc: Disable GuC submission and suspend it prior to i915 reset Sagar Arun Kamble
@ 2017-09-22  4:49 ` Sagar Arun Kamble
  2017-09-22 10:31   ` Michal Wajdeczko
  2017-09-22  4:49 ` [PATCH v6 6/7] drm/i915/guc: Enable default/critical logging in GuC by default from GuC v9 Sagar Arun Kamble
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 15+ messages in thread
From: Sagar Arun Kamble @ 2017-09-22  4:49 UTC (permalink / raw)
  To: intel-gfx

We ensure that GuC is completely suspended and client is destroyed
in i915_gem_suspend during i915_driver_unload. So now intel_uc_fini_hw
should just take care of cleanup,
hence s/intel_uc_fini_hw/intel_uc_cleanup. Correspondingly
we also updated as s/i915_guc_submission_fini/i915_guc_submission_cleanup
Other functionality to disable communication, disable interrupts and
update of ggtt.invalidate is taken care by intel_uc_suspend.
With this patch we are also doing guc_free_load_err_log only
if i915.enable_guc_loading is set.

v2: Rebase w.r.t removal of GuC code restructuring.

v3: Removed intel_guc_cleanup. (Michal Wajdeczko)

v4: guc_free_load_err_log() needs to be called without checking
i915.enable_guc_loading as this param is cleared on failure.
(Michal Wajdeczko)

Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c            |  2 +-
 drivers/gpu/drm/i915/i915_guc_submission.c |  2 +-
 drivers/gpu/drm/i915/intel_uc.c            | 14 ++++----------
 drivers/gpu/drm/i915/intel_uc.h            |  4 ++--
 4 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 8635f40..6f36ced 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -602,7 +602,7 @@ static 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);
+	intel_uc_cleanup(dev_priv);
 	i915_gem_cleanup_engines(dev_priv);
 	i915_gem_contexts_fini(dev_priv);
 	i915_gem_cleanup_userptr(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 09fb156..c910d79 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -1047,7 +1047,7 @@ int i915_guc_submission_init(struct drm_i915_private *dev_priv)
 	return ret;
 }
 
-void i915_guc_submission_fini(struct drm_i915_private *dev_priv)
+void i915_guc_submission_cleanup(struct drm_i915_private *dev_priv)
 {
 	struct intel_guc *guc = &dev_priv->guc;
 
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 7137e38..4efe67e 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -418,7 +418,7 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 	guc_capture_load_err_log(guc);
 err_submission:
 	if (i915.enable_guc_submission)
-		i915_guc_submission_fini(dev_priv);
+		i915_guc_submission_cleanup(dev_priv);
 err_guc:
 	i915_ggtt_disable_guc(dev_priv);
 
@@ -439,21 +439,15 @@ 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_cleanup(struct drm_i915_private *dev_priv)
 {
 	guc_free_load_err_log(&dev_priv->guc);
 
 	if (!i915.enable_guc_loading)
 		return;
 
-	guc_disable_communication(&dev_priv->guc);
-
-	if (i915.enable_guc_submission) {
-		gen9_disable_guc_interrupts(dev_priv);
-		i915_guc_submission_fini(dev_priv);
-	}
-
-	i915_ggtt_disable_guc(dev_priv);
+	if (i915.enable_guc_submission)
+		i915_guc_submission_cleanup(dev_priv);
 }
 
 int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len)
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 7b9ac3e..c6038de 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -207,7 +207,7 @@ struct intel_huc {
 void intel_uc_init_fw(struct drm_i915_private *dev_priv);
 void intel_uc_fini_fw(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);
+void intel_uc_cleanup(struct drm_i915_private *dev_priv);
 int intel_uc_runtime_suspend(struct drm_i915_private *dev_priv);
 int intel_uc_runtime_resume(struct drm_i915_private *dev_priv);
 int intel_uc_suspend(struct drm_i915_private *dev_priv);
@@ -238,7 +238,7 @@ static inline void intel_guc_notify(struct intel_guc *guc)
 int i915_guc_submission_init(struct drm_i915_private *dev_priv);
 int i915_guc_submission_enable(struct drm_i915_private *dev_priv);
 void i915_guc_submission_disable(struct drm_i915_private *dev_priv);
-void i915_guc_submission_fini(struct drm_i915_private *dev_priv);
+void i915_guc_submission_cleanup(struct drm_i915_private *dev_priv);
 struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size);
 
 /* intel_guc_log.c */
-- 
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] 15+ messages in thread

* [PATCH v6 6/7] drm/i915/guc: Enable default/critical logging in GuC by default from GuC v9
  2017-09-22  4:49 [PATCH v6 0/7] GuC Fixes, HuC auth. reorg and v9+ logging change Sagar Arun Kamble
                   ` (4 preceding siblings ...)
  2017-09-22  4:49 ` [PATCH v6 5/7] drm/i915/guc: Fix GuC cleanup in unload path Sagar Arun Kamble
@ 2017-09-22  4:49 ` Sagar Arun Kamble
  2017-09-22  4:49 ` [PATCH v6 7/7] drm/i915: Reorganize HuC authentication Sagar Arun Kamble
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Sagar Arun Kamble @ 2017-09-22  4:49 UTC (permalink / raw)
  To: intel-gfx; +Cc: Chheda Harsh J, Fry Gregory P

With GuC v9, new type of Default/critical logging in GuC to enable
capturing minimal important logs in production systems efficiently.
This patch enables this logging in GuC by default always. It should
be noted that streaming support with half-full interrupt mechanism
that is present for normal logging is not present for this type of
logging.

v2: Emulated GuC critical logging through i915.guc_log_level.

v3: Commit message update. Enable default/critical logging in GuC always.
Fixed RPM wake during guc_log_unregister in the unload path.

v4: Moved RPM wake change to separate patch. Removed GUC_DEBUG_RESERVED
and updated name of new bit to be version agnostic. Updated parameter to
struct intel_guc * and name of macro NEEDS_GUC_CRITICAL_LOGGING.
Removed explicit clearing of GUC_CRITICAL_LOGGING_DISABLED from
params[GUC_CTL_DEBUG] as it is unnecessary. (Michal Wajdeczko)

v5: Removed GUC_CRITICAL_LOGGING_DISABLED. Added HAS_GUC check to
GUC_NEEDS_CRITICAL_LOGGING. (Michal Wajdeczko)

v6: More refined version of GUC_NEEDS_CRITICAL_LOGGING. Commit message
update. (Michal Wajdeczko)

Cc: Chheda Harsh J <harsh.j.chheda@intel.com>
Cc: Fry Gregory P <gregory.p.fry@intel.com>
Cc: Spotswood John A <john.a.spotswood@intel.com>
Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Signed-off-by: Jeff McGee <jeff.mcgee@intel.com>
Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_fwif.h |  4 ++--
 drivers/gpu/drm/i915/intel_guc_log.c  | 13 ++++++++++++-
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h b/drivers/gpu/drm/i915/intel_guc_fwif.h
index 7eb6b4f..fed875a 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -127,7 +127,6 @@
 #define   GUC_PROFILE_ENABLED		(1 << 7)
 #define   GUC_WQ_TRACK_ENABLED		(1 << 8)
 #define   GUC_ADS_ENABLED		(1 << 9)
-#define   GUC_DEBUG_RESERVED		(1 << 10)
 #define   GUC_ADS_ADDR_SHIFT		11
 #define   GUC_ADS_ADDR_MASK		0xfffff800
 
@@ -539,7 +538,8 @@ struct guc_log_buffer_state {
 		u32 logging_enabled:1;
 		u32 reserved1:3;
 		u32 verbosity:4;
-		u32 reserved2:24;
+		u32 critical_logging_enabled:1;
+		u32 reserved2:23;
 	};
 	u32 value;
 } __packed;
diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
index 16d3b87..677ec3d 100644
--- a/drivers/gpu/drm/i915/intel_guc_log.c
+++ b/drivers/gpu/drm/i915/intel_guc_log.c
@@ -586,10 +586,18 @@ void intel_guc_log_destroy(struct intel_guc *guc)
 	i915_vma_unpin_and_release(&guc->log.vma);
 }
 
+/*
+ * Critical logging in GuC is to be enabled always from GuC v9+.
+ * (for KBL - v9.39+)
+ */
+#define GUC_NEEDS_CRITICAL_LOGGING(guc)	\
+	(HAS_GUC(guc_to_i915(guc)) && \
+	 (guc->fw.major_ver_found >= 9) && \
+	 (guc->fw.minor_ver_found >= (IS_KABYLAKE(guc_to_i915(guc)) ? 39 : 0)))
+
 int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val)
 {
 	struct intel_guc *guc = &dev_priv->guc;
-
 	union guc_log_control log_param;
 	int ret;
 
@@ -603,6 +611,9 @@ int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val)
 	if (!log_param.logging_enabled && (i915.guc_log_level < 0))
 		return 0;
 
+	if (GUC_NEEDS_CRITICAL_LOGGING(guc))
+		log_param.critical_logging_enabled = 1;
+
 	ret = guc_log_control(guc, log_param.value);
 	if (ret < 0) {
 		DRM_DEBUG_DRIVER("guc_logging_control action failed %d\n", ret);
-- 
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] 15+ messages in thread

* [PATCH v6 7/7] drm/i915: Reorganize HuC authentication
  2017-09-22  4:49 [PATCH v6 0/7] GuC Fixes, HuC auth. reorg and v9+ logging change Sagar Arun Kamble
                   ` (5 preceding siblings ...)
  2017-09-22  4:49 ` [PATCH v6 6/7] drm/i915/guc: Enable default/critical logging in GuC by default from GuC v9 Sagar Arun Kamble
@ 2017-09-22  4:49 ` Sagar Arun Kamble
  2017-09-22 11:22 ` ✓ Fi.CI.BAT: success for GuC Fixes, HuC auth. reorg and v9+ logging change (rev2) Patchwork
  2017-09-22 13:29 ` ✗ Fi.CI.IGT: warning " Patchwork
  8 siblings, 0 replies; 15+ messages in thread
From: Sagar Arun Kamble @ 2017-09-22  4:49 UTC (permalink / raw)
  To: intel-gfx

Prepared intel_auth_huc to separate HuC specific functionality
from GuC send action. Created new header intel_huc.h to group
HuC specific declarations.

v2: Changed argument preparation for AUTHENTICATE_HUC.
s/intel_auth_huc/intel_huc_auth. Deferred creation of intel_huc.h
to later patch.

v3: Rebase as intel_guc.h is removed. Added param description to
intel_huc_auth. (Michal)

v4: Rebase as intel_guc.h is added again. :)

v5: Rebase w.r.t removal of GuC code restructuring.

Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/intel_huc.c | 22 +++++++---------------
 drivers/gpu/drm/i915/intel_uc.c  | 20 +++++++++++++++++++-
 drivers/gpu/drm/i915/intel_uc.h  |  3 ++-
 3 files changed, 28 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c
index 6145fa0..d3da4d3 100644
--- a/drivers/gpu/drm/i915/intel_huc.c
+++ b/drivers/gpu/drm/i915/intel_huc.c
@@ -225,19 +225,16 @@ void intel_huc_init_hw(struct intel_huc *huc)
 }
 
 /**
- * intel_guc_auth_huc() - authenticate ucode
- * @dev_priv: the drm_i915_device
- *
- * Triggers a HuC fw authentication request to the GuC via intel_guc_action_
- * authenticate_huc interface.
+ * intel_huc_auth() - authenticate ucode
+ * @huc: intel_huc structure
  */
-void intel_guc_auth_huc(struct drm_i915_private *dev_priv)
+void intel_huc_auth(struct intel_huc *huc)
 {
+	struct drm_i915_private *dev_priv = huc_to_i915(huc);
 	struct intel_guc *guc = &dev_priv->guc;
-	struct intel_huc *huc = &dev_priv->huc;
 	struct i915_vma *vma;
+	u32 offset;
 	int ret;
-	u32 data[2];
 
 	if (huc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
 		return;
@@ -250,11 +247,8 @@ void intel_guc_auth_huc(struct drm_i915_private *dev_priv)
 		return;
 	}
 
-	/* Specify auth action and where public signature is. */
-	data[0] = INTEL_GUC_ACTION_AUTHENTICATE_HUC;
-	data[1] = guc_ggtt_offset(vma) + huc->fw.rsa_offset;
-
-	ret = intel_guc_send(guc, data, ARRAY_SIZE(data));
+	offset = guc_ggtt_offset(vma) + huc->fw.rsa_offset;
+	ret = intel_guc_auth_huc(guc, offset);
 	if (ret) {
 		DRM_ERROR("HuC: GuC did not ack Auth request %d\n", ret);
 		goto out;
@@ -266,7 +260,6 @@ void intel_guc_auth_huc(struct drm_i915_private *dev_priv)
 				HUC_FW_VERIFIED,
 				HUC_FW_VERIFIED,
 				50);
-
 	if (ret) {
 		DRM_ERROR("HuC: Authentication failed %d\n", ret);
 		goto out;
@@ -275,4 +268,3 @@ void intel_guc_auth_huc(struct drm_i915_private *dev_priv)
 out:
 	i915_vma_unpin(vma);
 }
-
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 4efe67e..68edea4 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -328,6 +328,24 @@ static void guc_disable_communication(struct intel_guc *guc)
 	guc->send = intel_guc_send_nop;
 }
 
+/**
+ * intel_guc_auth_huc() - authenticate ucode
+ * @guc: struct intel_guc*
+ * @offset: rsa offset w.r.t ggtt base of huc vma
+ *
+ * triggers a huc fw authentication request to the guc via intel_guc_send
+ * authenticate_huc interface.
+ */
+int intel_guc_auth_huc(struct intel_guc *guc, u32 rsa_offset)
+{
+	u32 action[] = {
+		INTEL_GUC_ACTION_AUTHENTICATE_HUC,
+		rsa_offset
+	};
+
+	return intel_guc_send(guc, action, ARRAY_SIZE(action));
+}
+
 int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 {
 	struct intel_guc *guc = &dev_priv->guc;
@@ -390,7 +408,7 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 	if (ret)
 		goto err_log_capture;
 
-	intel_guc_auth_huc(dev_priv);
+	intel_huc_auth(&dev_priv->huc);
 	if (i915.enable_guc_submission) {
 		if (i915.guc_log_level >= 0)
 			gen9_enable_guc_interrupts(dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index c6038de..838a364 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -216,6 +216,7 @@ struct intel_huc {
 int intel_guc_sample_forcewake(struct intel_guc *guc);
 int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len);
 int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len);
+int intel_guc_auth_huc(struct intel_guc *guc, u32 rsa_offset);
 
 static inline int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 len)
 {
@@ -259,6 +260,6 @@ static inline u32 guc_ggtt_offset(struct i915_vma *vma)
 /* intel_huc.c */
 void intel_huc_select_fw(struct intel_huc *huc);
 void intel_huc_init_hw(struct intel_huc *huc);
-void intel_guc_auth_huc(struct drm_i915_private *dev_priv);
+void intel_huc_auth(struct intel_huc *huc);
 
 #endif
-- 
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] 15+ messages in thread

* Re: [PATCH v6 1/7] drm/i915: Create uc runtime and system suspend/resume helpers
  2017-09-22  4:49 ` [PATCH v6 1/7] drm/i915: Create uc runtime and system suspend/resume helpers Sagar Arun Kamble
@ 2017-09-22  7:11   ` Michal Wajdeczko
  2017-09-22  7:32     ` Sagar Arun Kamble
  0 siblings, 1 reply; 15+ messages in thread
From: Michal Wajdeczko @ 2017-09-22  7:11 UTC (permalink / raw)
  To: intel-gfx, Sagar Arun Kamble

On Fri, 22 Sep 2017 06:49:12 +0200, Sagar Arun Kamble  
<sagar.a.kamble@intel.com> wrote:

> Prepared generic helpers intel_uc_suspend, intel_uc_resume,
> intel_uc_runtime_suspend, intel_uc_runtime_resume. Added
> error handling to all the calls for suspend/resume.
>
> v2: Rebase w.r.t removal of GuC code restructuring.
>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Michał Winiarski <michal.winiarski@intel.com>
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c | 23 ++++++++++++++++++++---
>  drivers/gpu/drm/i915/i915_gem.c |  7 ++++++-
>  drivers/gpu/drm/i915/intel_uc.c | 20 ++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_uc.h |  4 ++++
>  4 files changed, 50 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c  
> b/drivers/gpu/drm/i915/i915_drv.c
> index 5c111ea..8635f40 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1691,7 +1691,15 @@ static int i915_drm_resume(struct drm_device *dev)
>  	}
>  	mutex_unlock(&dev->struct_mutex);
> -	intel_guc_resume(dev_priv);
> +	/*
> +	 * NB: Full gem reinitialization is being done above, hence
> +	 * intel_uc_resume will be of no use. Currently intel_uc_resume
> +	 * is nop. If full reinitialization is removed, will  need to put
> +	 * functionality to resume from sleep in intel_uc_resume.
> +	 */
> +	ret = intel_uc_resume(dev_priv);
> +	if (ret)
> +		DRM_ERROR("failed to resume uc\n");
> 	intel_modeset_init_hw(dev);
> @@ -2493,7 +2501,12 @@ static int intel_runtime_suspend(struct device  
> *kdev)
>  	 */
>  	i915_gem_runtime_suspend(dev_priv);
> -	intel_guc_suspend(dev_priv);
> +	ret = intel_uc_runtime_suspend(dev_priv);
> +	if (ret) {
> +		DRM_ERROR("uc runtime suspend failed, disabling it(%d)\n", ret);
> +		enable_rpm_wakeref_asserts(dev_priv);
> +		return ret;
> +	}
> 	intel_runtime_pm_disable_interrupts(dev_priv);
> @@ -2578,7 +2591,11 @@ static int intel_runtime_resume(struct device  
> *kdev)
>  	if (intel_uncore_unclaimed_mmio(dev_priv))
>  		DRM_DEBUG_DRIVER("Unclaimed access during suspend, bios?\n");
> -	intel_guc_resume(dev_priv);
> +	ret = intel_uc_runtime_resume(dev_priv);
> +	if (ret) {
> +		DRM_ERROR("uc runtime resume failed (%d)\n", ret);
> +		return ret;
> +	}
> 	if (IS_GEN9_LP(dev_priv)) {
>  		bxt_disable_dc9(dev_priv);
> diff --git a/drivers/gpu/drm/i915/i915_gem.c  
> b/drivers/gpu/drm/i915/i915_gem.c
> index c4bf348..dd56d45 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4539,7 +4539,11 @@ int i915_gem_suspend(struct drm_i915_private  
> *dev_priv)
>  	i915_gem_contexts_lost(dev_priv);
>  	mutex_unlock(&dev->struct_mutex);
> -	intel_guc_suspend(dev_priv);
> +	ret = intel_uc_suspend(dev_priv);

Is it ok that we suspend uc in 'gem_suspend' but resume in 'drm_resume' ?

Michal

> +	if (ret) {
> +		DRM_ERROR("uc suspend failed (%d)\n", ret);
> +		goto out;
> +	}
> 	cancel_delayed_work_sync(&dev_priv->gpu_error.hangcheck_work);
>  	cancel_delayed_work_sync(&dev_priv->gt.retire_work);
> @@ -4583,6 +4587,7 @@ int i915_gem_suspend(struct drm_i915_private  
> *dev_priv)
> err_unlock:
>  	mutex_unlock(&dev->struct_mutex);
> +out:
>  	intel_runtime_pm_put(dev_priv);
>  	return ret;
>  }
> diff --git a/drivers/gpu/drm/i915/intel_uc.c  
> b/drivers/gpu/drm/i915/intel_uc.c
> index 0178ba4..8e4d8b0 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -537,3 +537,23 @@ int intel_guc_sample_forcewake(struct intel_guc  
> *guc)
> 	return intel_guc_send(guc, action, ARRAY_SIZE(action));
>  }
> +
> +int intel_uc_runtime_suspend(struct drm_i915_private *dev_priv)
> +{
> +	return intel_guc_suspend(dev_priv);
> +}
> +
> +int intel_uc_runtime_resume(struct drm_i915_private *dev_priv)
> +{
> +	return intel_guc_resume(dev_priv);
> +}
> +
> +int intel_uc_suspend(struct drm_i915_private *dev_priv)
> +{
> +	return intel_guc_suspend(dev_priv);
> +}
> +
> +int intel_uc_resume(struct drm_i915_private *dev_priv)
> +{
> +	return 0;
> +}
> diff --git a/drivers/gpu/drm/i915/intel_uc.h  
> b/drivers/gpu/drm/i915/intel_uc.h
> index 7703c9a..3d33a51 100644
> --- a/drivers/gpu/drm/i915/intel_uc.h
> +++ b/drivers/gpu/drm/i915/intel_uc.h
> @@ -208,6 +208,10 @@ struct intel_huc {
>  void intel_uc_fini_fw(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_runtime_suspend(struct drm_i915_private *dev_priv);
> +int intel_uc_runtime_resume(struct drm_i915_private *dev_priv);
> +int intel_uc_suspend(struct drm_i915_private *dev_priv);
> +int intel_uc_resume(struct drm_i915_private *dev_priv);
>  int intel_guc_sample_forcewake(struct intel_guc *guc);
>  int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32  
> len);
>  int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32  
> len);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v6 1/7] drm/i915: Create uc runtime and system suspend/resume helpers
  2017-09-22  7:11   ` Michal Wajdeczko
@ 2017-09-22  7:32     ` Sagar Arun Kamble
  0 siblings, 0 replies; 15+ messages in thread
From: Sagar Arun Kamble @ 2017-09-22  7:32 UTC (permalink / raw)
  To: Michal Wajdeczko, intel-gfx



On 9/22/2017 12:41 PM, Michal Wajdeczko wrote:
> On Fri, 22 Sep 2017 06:49:12 +0200, Sagar Arun Kamble 
> <sagar.a.kamble@intel.com> wrote:
>
>> Prepared generic helpers intel_uc_suspend, intel_uc_resume,
>> intel_uc_runtime_suspend, intel_uc_runtime_resume. Added
>> error handling to all the calls for suspend/resume.
>>
>> v2: Rebase w.r.t removal of GuC code restructuring.
>>
>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Michał Winiarski <michal.winiarski@intel.com>
>> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_drv.c | 23 ++++++++++++++++++++---
>>  drivers/gpu/drm/i915/i915_gem.c |  7 ++++++-
>>  drivers/gpu/drm/i915/intel_uc.c | 20 ++++++++++++++++++++
>>  drivers/gpu/drm/i915/intel_uc.h |  4 ++++
>>  4 files changed, 50 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.c 
>> b/drivers/gpu/drm/i915/i915_drv.c
>> index 5c111ea..8635f40 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.c
>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>> @@ -1691,7 +1691,15 @@ static int i915_drm_resume(struct drm_device 
>> *dev)
>>      }
>>      mutex_unlock(&dev->struct_mutex);
>> -    intel_guc_resume(dev_priv);
>> +    /*
>> +     * NB: Full gem reinitialization is being done above, hence
>> +     * intel_uc_resume will be of no use. Currently intel_uc_resume
>> +     * is nop. If full reinitialization is removed, will  need to put
>> +     * functionality to resume from sleep in intel_uc_resume.
>> +     */
>> +    ret = intel_uc_resume(dev_priv);
>> +    if (ret)
>> +        DRM_ERROR("failed to resume uc\n");
>>     intel_modeset_init_hw(dev);
>> @@ -2493,7 +2501,12 @@ static int intel_runtime_suspend(struct device 
>> *kdev)
>>       */
>>      i915_gem_runtime_suspend(dev_priv);
>> -    intel_guc_suspend(dev_priv);
>> +    ret = intel_uc_runtime_suspend(dev_priv);
>> +    if (ret) {
>> +        DRM_ERROR("uc runtime suspend failed, disabling it(%d)\n", 
>> ret);
>> +        enable_rpm_wakeref_asserts(dev_priv);
>> +        return ret;
>> +    }
>>     intel_runtime_pm_disable_interrupts(dev_priv);
>> @@ -2578,7 +2591,11 @@ static int intel_runtime_resume(struct device 
>> *kdev)
>>      if (intel_uncore_unclaimed_mmio(dev_priv))
>>          DRM_DEBUG_DRIVER("Unclaimed access during suspend, bios?\n");
>> -    intel_guc_resume(dev_priv);
>> +    ret = intel_uc_runtime_resume(dev_priv);
>> +    if (ret) {
>> +        DRM_ERROR("uc runtime resume failed (%d)\n", ret);
>> +        return ret;
>> +    }
>>     if (IS_GEN9_LP(dev_priv)) {
>>          bxt_disable_dc9(dev_priv);
>> diff --git a/drivers/gpu/drm/i915/i915_gem.c 
>> b/drivers/gpu/drm/i915/i915_gem.c
>> index c4bf348..dd56d45 100644
>> --- a/drivers/gpu/drm/i915/i915_gem.c
>> +++ b/drivers/gpu/drm/i915/i915_gem.c
>> @@ -4539,7 +4539,11 @@ int i915_gem_suspend(struct drm_i915_private 
>> *dev_priv)
>>      i915_gem_contexts_lost(dev_priv);
>>      mutex_unlock(&dev->struct_mutex);
>> -    intel_guc_suspend(dev_priv);
>> +    ret = intel_uc_suspend(dev_priv);
>
> Is it ok that we suspend uc in 'gem_suspend' but resume in 'drm_resume' ?
>
> Michal
Currently it is fine as we are doing full reinit. But in future, it is 
better to have it in i915_gem_resume.
Will move it there. Thanks.
>
>> +    if (ret) {
>> +        DRM_ERROR("uc suspend failed (%d)\n", ret);
>> +        goto out;
>> +    }
>>     cancel_delayed_work_sync(&dev_priv->gpu_error.hangcheck_work);
>>      cancel_delayed_work_sync(&dev_priv->gt.retire_work);
>> @@ -4583,6 +4587,7 @@ int i915_gem_suspend(struct drm_i915_private 
>> *dev_priv)
>> err_unlock:
>>      mutex_unlock(&dev->struct_mutex);
>> +out:
>>      intel_runtime_pm_put(dev_priv);
>>      return ret;
>>  }
>> diff --git a/drivers/gpu/drm/i915/intel_uc.c 
>> b/drivers/gpu/drm/i915/intel_uc.c
>> index 0178ba4..8e4d8b0 100644
>> --- a/drivers/gpu/drm/i915/intel_uc.c
>> +++ b/drivers/gpu/drm/i915/intel_uc.c
>> @@ -537,3 +537,23 @@ int intel_guc_sample_forcewake(struct intel_guc 
>> *guc)
>>     return intel_guc_send(guc, action, ARRAY_SIZE(action));
>>  }
>> +
>> +int intel_uc_runtime_suspend(struct drm_i915_private *dev_priv)
>> +{
>> +    return intel_guc_suspend(dev_priv);
>> +}
>> +
>> +int intel_uc_runtime_resume(struct drm_i915_private *dev_priv)
>> +{
>> +    return intel_guc_resume(dev_priv);
>> +}
>> +
>> +int intel_uc_suspend(struct drm_i915_private *dev_priv)
>> +{
>> +    return intel_guc_suspend(dev_priv);
>> +}
>> +
>> +int intel_uc_resume(struct drm_i915_private *dev_priv)
>> +{
>> +    return 0;
>> +}
>> diff --git a/drivers/gpu/drm/i915/intel_uc.h 
>> b/drivers/gpu/drm/i915/intel_uc.h
>> index 7703c9a..3d33a51 100644
>> --- a/drivers/gpu/drm/i915/intel_uc.h
>> +++ b/drivers/gpu/drm/i915/intel_uc.h
>> @@ -208,6 +208,10 @@ struct intel_huc {
>>  void intel_uc_fini_fw(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_runtime_suspend(struct drm_i915_private *dev_priv);
>> +int intel_uc_runtime_resume(struct drm_i915_private *dev_priv);
>> +int intel_uc_suspend(struct drm_i915_private *dev_priv);
>> +int intel_uc_resume(struct drm_i915_private *dev_priv);
>>  int intel_guc_sample_forcewake(struct intel_guc *guc);
>>  int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 
>> len);
>>  int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, 
>> u32 len);

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

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

* Re: [PATCH v6 2/7] drm/i915/guc: Update GuC ggtt.invalidate/interrupts/communication across RPM suspend/resume
  2017-09-22  4:49 ` [PATCH v6 2/7] drm/i915/guc: Update GuC ggtt.invalidate/interrupts/communication across RPM suspend/resume Sagar Arun Kamble
@ 2017-09-22  7:38   ` Michal Wajdeczko
  2017-09-22  7:59     ` Sagar Arun Kamble
  0 siblings, 1 reply; 15+ messages in thread
From: Michal Wajdeczko @ 2017-09-22  7:38 UTC (permalink / raw)
  To: intel-gfx, Sagar Arun Kamble

On Fri, 22 Sep 2017 06:49:13 +0200, Sagar Arun Kamble  
<sagar.a.kamble@intel.com> wrote:

> Apart from configuring interrupts, we need to update the ggtt invalidate
> interface and GuC communication on suspend. This functionality can be
> reused for other suspend and reset paths.
>
> v2: Rebase w.r.t removal of GuC code restructuring.
>
> v3: Removed GuC specific helpers as tasks other than send H2G for
> sleep/resume are to be done from uc generic functions. (Michal)
>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Michał Winiarski <michal.winiarski@intel.com>
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_guc_submission.c |  5 ----
>  drivers/gpu/drm/i915/intel_uc.c            | 39  
> ++++++++++++++++++++++++++++--
>  2 files changed, 37 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c  
> b/drivers/gpu/drm/i915/i915_guc_submission.c
> index e191d56..cd5ef8b 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -1219,8 +1219,6 @@ int intel_guc_suspend(struct drm_i915_private  
> *dev_priv)
>  	if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
>  		return 0;
> -	gen9_disable_guc_interrupts(dev_priv);
> -
>  	ctx = dev_priv->kernel_context;
> 	data[0] = INTEL_GUC_ACTION_ENTER_S_STATE;
> @@ -1245,9 +1243,6 @@ int intel_guc_resume(struct drm_i915_private  
> *dev_priv)
>  	if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
>  		return 0;
> -	if (i915.guc_log_level >= 0)
> -		gen9_enable_guc_interrupts(dev_priv);
> -
>  	ctx = dev_priv->kernel_context;
> 	data[0] = INTEL_GUC_ACTION_EXIT_S_STATE;
> diff --git a/drivers/gpu/drm/i915/intel_uc.c  
> b/drivers/gpu/drm/i915/intel_uc.c
> index 8e4d8b0..66d2cff 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -540,12 +540,47 @@ int intel_guc_sample_forcewake(struct intel_guc  
> *guc)
> int intel_uc_runtime_suspend(struct drm_i915_private *dev_priv)
>  {
> -	return intel_guc_suspend(dev_priv);
> +	int ret;
> +
> +	if (!i915.enable_guc_loading)
> +		return 0;
> +
> +	ret = intel_guc_suspend(dev_priv);
> +	if (ret) {
> +		DRM_ERROR("GuC runtime suspend failed (%d)\n", ret);
> +		return ret;
> +	}
> +
> +	i915_ggtt_disable_guc(dev_priv);
> +	gen9_disable_guc_interrupts(dev_priv);
> +	guc_disable_communication(&dev_priv->guc);
> +
> +	return 0;
>  }
> int intel_uc_runtime_resume(struct drm_i915_private *dev_priv)
>  {
> -	return intel_guc_resume(dev_priv);
> +	int ret;
> +
> +	if (!i915.enable_guc_loading)
> +		return 0;
> +
> +	ret = guc_enable_communication(&dev_priv->guc);
> +	if (ret) {
> +		DRM_ERROR("GuC enable communication failed (%d)\n", ret);

Not sure if we need extra message here, as Guc CT has its own error
messages

> +		return ret;
> +	}
> +	if (i915.guc_log_level >= 0)
> +		gen9_enable_guc_interrupts(dev_priv);
> +	i915_ggtt_enable_guc(dev_priv);
> +
> +	ret = intel_guc_resume(dev_priv);
> +	if (ret) {
> +		DRM_ERROR("GuC runtime resume failed (%d)\n", ret);
> +		return ret;
> +	}
> +
> +	return 0;

Maybe better would be to have single error message:

out:
	if (ret)
		DRM_ERROR("Failed to resume uC runtime (%d)\n", ret);
	return ret;

>  }
> int intel_uc_suspend(struct drm_i915_private *dev_priv)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v6 2/7] drm/i915/guc: Update GuC ggtt.invalidate/interrupts/communication across RPM suspend/resume
  2017-09-22  7:38   ` Michal Wajdeczko
@ 2017-09-22  7:59     ` Sagar Arun Kamble
  0 siblings, 0 replies; 15+ messages in thread
From: Sagar Arun Kamble @ 2017-09-22  7:59 UTC (permalink / raw)
  To: Michal Wajdeczko, intel-gfx



On 9/22/2017 1:08 PM, Michal Wajdeczko wrote:
> On Fri, 22 Sep 2017 06:49:13 +0200, Sagar Arun Kamble 
> <sagar.a.kamble@intel.com> wrote:
>
>> Apart from configuring interrupts, we need to update the ggtt invalidate
>> interface and GuC communication on suspend. This functionality can be
>> reused for other suspend and reset paths.
>>
>> v2: Rebase w.r.t removal of GuC code restructuring.
>>
>> v3: Removed GuC specific helpers as tasks other than send H2G for
>> sleep/resume are to be done from uc generic functions. (Michal)
>>
>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Michał Winiarski <michal.winiarski@intel.com>
>> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_guc_submission.c |  5 ----
>>  drivers/gpu/drm/i915/intel_uc.c            | 39 
>> ++++++++++++++++++++++++++++--
>>  2 files changed, 37 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c 
>> b/drivers/gpu/drm/i915/i915_guc_submission.c
>> index e191d56..cd5ef8b 100644
>> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
>> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
>> @@ -1219,8 +1219,6 @@ int intel_guc_suspend(struct drm_i915_private 
>> *dev_priv)
>>      if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
>>          return 0;
>> -    gen9_disable_guc_interrupts(dev_priv);
>> -
>>      ctx = dev_priv->kernel_context;
>>     data[0] = INTEL_GUC_ACTION_ENTER_S_STATE;
>> @@ -1245,9 +1243,6 @@ int intel_guc_resume(struct drm_i915_private 
>> *dev_priv)
>>      if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
>>          return 0;
>> -    if (i915.guc_log_level >= 0)
>> -        gen9_enable_guc_interrupts(dev_priv);
>> -
>>      ctx = dev_priv->kernel_context;
>>     data[0] = INTEL_GUC_ACTION_EXIT_S_STATE;
>> diff --git a/drivers/gpu/drm/i915/intel_uc.c 
>> b/drivers/gpu/drm/i915/intel_uc.c
>> index 8e4d8b0..66d2cff 100644
>> --- a/drivers/gpu/drm/i915/intel_uc.c
>> +++ b/drivers/gpu/drm/i915/intel_uc.c
>> @@ -540,12 +540,47 @@ int intel_guc_sample_forcewake(struct intel_guc 
>> *guc)
>> int intel_uc_runtime_suspend(struct drm_i915_private *dev_priv)
>>  {
>> -    return intel_guc_suspend(dev_priv);
>> +    int ret;
>> +
>> +    if (!i915.enable_guc_loading)
>> +        return 0;
>> +
>> +    ret = intel_guc_suspend(dev_priv);
>> +    if (ret) {
>> +        DRM_ERROR("GuC runtime suspend failed (%d)\n", ret);
>> +        return ret;
>> +    }
>> +
>> +    i915_ggtt_disable_guc(dev_priv);
>> +    gen9_disable_guc_interrupts(dev_priv);
>> +    guc_disable_communication(&dev_priv->guc);
>> +
>> +    return 0;
>>  }
>> int intel_uc_runtime_resume(struct drm_i915_private *dev_priv)
>>  {
>> -    return intel_guc_resume(dev_priv);
>> +    int ret;
>> +
>> +    if (!i915.enable_guc_loading)
>> +        return 0;
>> +
>> +    ret = guc_enable_communication(&dev_priv->guc);
>> +    if (ret) {
>> +        DRM_ERROR("GuC enable communication failed (%d)\n", ret);
>
> Not sure if we need extra message here, as Guc CT has its own error
> messages
Ok. Will remove these.
>
>> +        return ret;
>> +    }
>> +    if (i915.guc_log_level >= 0)
>> +        gen9_enable_guc_interrupts(dev_priv);
>> +    i915_ggtt_enable_guc(dev_priv);
>> +
>> +    ret = intel_guc_resume(dev_priv);
>> +    if (ret) {
>> +        DRM_ERROR("GuC runtime resume failed (%d)\n", ret);
>> +        return ret;
>> +    }
>> +
>> +    return 0;
>
> Maybe better would be to have single error message:
>
> out:
>     if (ret)
>         DRM_ERROR("Failed to resume uC runtime (%d)\n", ret);
>     return ret;
Will update like this. Thanks.
>
>>  }
>> int intel_uc_suspend(struct drm_i915_private *dev_priv)

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

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

* Re: [PATCH v6 5/7] drm/i915/guc: Fix GuC cleanup in unload path
  2017-09-22  4:49 ` [PATCH v6 5/7] drm/i915/guc: Fix GuC cleanup in unload path Sagar Arun Kamble
@ 2017-09-22 10:31   ` Michal Wajdeczko
  0 siblings, 0 replies; 15+ messages in thread
From: Michal Wajdeczko @ 2017-09-22 10:31 UTC (permalink / raw)
  To: intel-gfx, Sagar Arun Kamble

On Fri, 22 Sep 2017 06:49:16 +0200, Sagar Arun Kamble  
<sagar.a.kamble@intel.com> wrote:

> We ensure that GuC is completely suspended and client is destroyed
> in i915_gem_suspend during i915_driver_unload. So now intel_uc_fini_hw
> should just take care of cleanup,
> hence s/intel_uc_fini_hw/intel_uc_cleanup. Correspondingly
> we also updated as s/i915_guc_submission_fini/i915_guc_submission_cleanup
> Other functionality to disable communication, disable interrupts and
> update of ggtt.invalidate is taken care by intel_uc_suspend.
> With this patch we are also doing guc_free_load_err_log only
> if i915.enable_guc_loading is set.

I'm not sure that commit message must stay immutable during whole
review process, as now contains misleading statement. With that fixed:

Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>

>
> v2: Rebase w.r.t removal of GuC code restructuring.
>
> v3: Removed intel_guc_cleanup. (Michal Wajdeczko)
>
> v4: guc_free_load_err_log() needs to be called without checking
> i915.enable_guc_loading as this param is cleared on failure.
> (Michal Wajdeczko)
>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Michał Winiarski <michal.winiarski@intel.com>
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c            |  2 +-
>  drivers/gpu/drm/i915/i915_guc_submission.c |  2 +-
>  drivers/gpu/drm/i915/intel_uc.c            | 14 ++++----------
>  drivers/gpu/drm/i915/intel_uc.h            |  4 ++--
>  4 files changed, 8 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c  
> b/drivers/gpu/drm/i915/i915_drv.c
> index 8635f40..6f36ced 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -602,7 +602,7 @@ static 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);
> +	intel_uc_cleanup(dev_priv);
>  	i915_gem_cleanup_engines(dev_priv);
>  	i915_gem_contexts_fini(dev_priv);
>  	i915_gem_cleanup_userptr(dev_priv);
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c  
> b/drivers/gpu/drm/i915/i915_guc_submission.c
> index 09fb156..c910d79 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -1047,7 +1047,7 @@ int i915_guc_submission_init(struct  
> drm_i915_private *dev_priv)
>  	return ret;
>  }
> -void i915_guc_submission_fini(struct drm_i915_private *dev_priv)
> +void i915_guc_submission_cleanup(struct drm_i915_private *dev_priv)
>  {
>  	struct intel_guc *guc = &dev_priv->guc;
> diff --git a/drivers/gpu/drm/i915/intel_uc.c  
> b/drivers/gpu/drm/i915/intel_uc.c
> index 7137e38..4efe67e 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -418,7 +418,7 @@ int intel_uc_init_hw(struct drm_i915_private  
> *dev_priv)
>  	guc_capture_load_err_log(guc);
>  err_submission:
>  	if (i915.enable_guc_submission)
> -		i915_guc_submission_fini(dev_priv);
> +		i915_guc_submission_cleanup(dev_priv);
>  err_guc:
>  	i915_ggtt_disable_guc(dev_priv);
> @@ -439,21 +439,15 @@ 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_cleanup(struct drm_i915_private *dev_priv)
>  {
>  	guc_free_load_err_log(&dev_priv->guc);
> 	if (!i915.enable_guc_loading)
>  		return;
> -	guc_disable_communication(&dev_priv->guc);
> -
> -	if (i915.enable_guc_submission) {
> -		gen9_disable_guc_interrupts(dev_priv);
> -		i915_guc_submission_fini(dev_priv);
> -	}
> -
> -	i915_ggtt_disable_guc(dev_priv);
> +	if (i915.enable_guc_submission)
> +		i915_guc_submission_cleanup(dev_priv);
>  }
> int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len)
> diff --git a/drivers/gpu/drm/i915/intel_uc.h  
> b/drivers/gpu/drm/i915/intel_uc.h
> index 7b9ac3e..c6038de 100644
> --- a/drivers/gpu/drm/i915/intel_uc.h
> +++ b/drivers/gpu/drm/i915/intel_uc.h
> @@ -207,7 +207,7 @@ struct intel_huc {
>  void intel_uc_init_fw(struct drm_i915_private *dev_priv);
>  void intel_uc_fini_fw(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);
> +void intel_uc_cleanup(struct drm_i915_private *dev_priv);
>  int intel_uc_runtime_suspend(struct drm_i915_private *dev_priv);
>  int intel_uc_runtime_resume(struct drm_i915_private *dev_priv);
>  int intel_uc_suspend(struct drm_i915_private *dev_priv);
> @@ -238,7 +238,7 @@ static inline void intel_guc_notify(struct intel_guc  
> *guc)
>  int i915_guc_submission_init(struct drm_i915_private *dev_priv);
>  int i915_guc_submission_enable(struct drm_i915_private *dev_priv);
>  void i915_guc_submission_disable(struct drm_i915_private *dev_priv);
> -void i915_guc_submission_fini(struct drm_i915_private *dev_priv);
> +void i915_guc_submission_cleanup(struct drm_i915_private *dev_priv);
>  struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32  
> size);
> /* intel_guc_log.c */
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* ✓ Fi.CI.BAT: success for GuC Fixes, HuC auth. reorg and v9+ logging change (rev2)
  2017-09-22  4:49 [PATCH v6 0/7] GuC Fixes, HuC auth. reorg and v9+ logging change Sagar Arun Kamble
                   ` (6 preceding siblings ...)
  2017-09-22  4:49 ` [PATCH v6 7/7] drm/i915: Reorganize HuC authentication Sagar Arun Kamble
@ 2017-09-22 11:22 ` Patchwork
  2017-09-22 13:29 ` ✗ Fi.CI.IGT: warning " Patchwork
  8 siblings, 0 replies; 15+ messages in thread
From: Patchwork @ 2017-09-22 11:22 UTC (permalink / raw)
  To: Sagar Arun Kamble; +Cc: intel-gfx

== Series Details ==

Series: GuC Fixes, HuC auth. reorg and v9+ logging change (rev2)
URL   : https://patchwork.freedesktop.org/series/30715/
State : success

== Summary ==

Series 30715v2 GuC Fixes, HuC auth. reorg and v9+ logging change
https://patchwork.freedesktop.org/api/1.0/series/30715/revisions/2/mbox/

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:443s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:466s
fi-blb-e6850     total:289  pass:224  dwarn:1   dfail:0   fail:0   skip:64  time:418s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:517s
fi-bwr-2160      total:289  pass:184  dwarn:0   dfail:0   fail:0   skip:105 time:277s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:528s
fi-byt-j1900     total:289  pass:254  dwarn:1   dfail:0   fail:0   skip:34  time:500s
fi-byt-n2820     total:289  pass:250  dwarn:1   dfail:0   fail:0   skip:38  time:493s
fi-cfl-s         total:289  pass:223  dwarn:34  dfail:0   fail:0   skip:32  time:537s
fi-elk-e7500     total:289  pass:230  dwarn:0   dfail:0   fail:0   skip:59  time:421s
fi-glk-1         total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:569s
fi-hsw-4770      total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:428s
fi-hsw-4770r     total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:407s
fi-ilk-650       total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:432s
fi-ivb-3520m     total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:492s
fi-ivb-3770      total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:468s
fi-kbl-7500u     total:289  pass:264  dwarn:1   dfail:0   fail:0   skip:24  time:474s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:575s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:592s
fi-pnv-d510      total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:544s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:452s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:753s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:484s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:475s
fi-snb-2520m     total:289  pass:251  dwarn:0   dfail:0   fail:0   skip:38  time:571s
fi-snb-2600      total:289  pass:249  dwarn:0   dfail:0   fail:1   skip:39  time:423s

b32248fc0bd87b45bc068f234f061324a29fa809 drm-tip: 2017y-09m-21d-22h-36m-39s UTC integration manifest
2c043a4a88ed drm/i915: Reorganize HuC authentication
edd7fc3ac1e6 drm/i915/guc: Enable default/critical logging in GuC by default from GuC v9
71fda6216548 drm/i915/guc: Fix GuC cleanup in unload path
2e70c256b457 drm/i915/guc: Disable GuC submission and suspend it prior to i915 reset
4d77d940d6ae drm/i915/guc: Update GuC suspend functionality in intel_uc_suspend path
f74a4c62c445 drm/i915/guc: Update GuC ggtt.invalidate/interrupts/communication across RPM suspend/resume
35fbb52470da drm/i915: Create uc runtime and system suspend/resume helpers

== Logs ==

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

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

* ✗ Fi.CI.IGT: warning for GuC Fixes, HuC auth. reorg and v9+ logging change (rev2)
  2017-09-22  4:49 [PATCH v6 0/7] GuC Fixes, HuC auth. reorg and v9+ logging change Sagar Arun Kamble
                   ` (7 preceding siblings ...)
  2017-09-22 11:22 ` ✓ Fi.CI.BAT: success for GuC Fixes, HuC auth. reorg and v9+ logging change (rev2) Patchwork
@ 2017-09-22 13:29 ` Patchwork
  8 siblings, 0 replies; 15+ messages in thread
From: Patchwork @ 2017-09-22 13:29 UTC (permalink / raw)
  To: Kamble, Sagar A; +Cc: intel-gfx

== Series Details ==

Series: GuC Fixes, HuC auth. reorg and v9+ logging change (rev2)
URL   : https://patchwork.freedesktop.org/series/30715/
State : warning

== Summary ==

Test perf:
        Subgroup blocking:
                fail       -> PASS       (shard-hsw) fdo#102252
Test kms_flip:
        Subgroup wf_vblank-vs-modeset-interruptible:
                pass       -> DMESG-WARN (shard-hsw)

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

shard-hsw        total:2429 pass:1328 dwarn:7   dfail:0   fail:11  skip:1083 time:9726s

== Logs ==

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

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

end of thread, other threads:[~2017-09-22 13:29 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-22  4:49 [PATCH v6 0/7] GuC Fixes, HuC auth. reorg and v9+ logging change Sagar Arun Kamble
2017-09-22  4:49 ` [PATCH v6 1/7] drm/i915: Create uc runtime and system suspend/resume helpers Sagar Arun Kamble
2017-09-22  7:11   ` Michal Wajdeczko
2017-09-22  7:32     ` Sagar Arun Kamble
2017-09-22  4:49 ` [PATCH v6 2/7] drm/i915/guc: Update GuC ggtt.invalidate/interrupts/communication across RPM suspend/resume Sagar Arun Kamble
2017-09-22  7:38   ` Michal Wajdeczko
2017-09-22  7:59     ` Sagar Arun Kamble
2017-09-22  4:49 ` [PATCH v6 3/7] drm/i915/guc: Update GuC suspend functionality in intel_uc_suspend path Sagar Arun Kamble
2017-09-22  4:49 ` [PATCH v6 4/7] drm/i915/guc: Disable GuC submission and suspend it prior to i915 reset Sagar Arun Kamble
2017-09-22  4:49 ` [PATCH v6 5/7] drm/i915/guc: Fix GuC cleanup in unload path Sagar Arun Kamble
2017-09-22 10:31   ` Michal Wajdeczko
2017-09-22  4:49 ` [PATCH v6 6/7] drm/i915/guc: Enable default/critical logging in GuC by default from GuC v9 Sagar Arun Kamble
2017-09-22  4:49 ` [PATCH v6 7/7] drm/i915: Reorganize HuC authentication Sagar Arun Kamble
2017-09-22 11:22 ` ✓ Fi.CI.BAT: success for GuC Fixes, HuC auth. reorg and v9+ logging change (rev2) Patchwork
2017-09-22 13:29 ` ✗ Fi.CI.IGT: warning " Patchwork

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.