Re: [Intel-gfx] [PATCH v2 2/3] drm/i915/uc: Sanitize uC together with GEM

2018-03-09 Thread Daniele Ceraolo Spurio 



On 09/03/18 08:01, Michal Wajdeczko wrote:

Instead of dancing around uC on reset/suspend/resume scenarios,
explicitly sanitize uC when we sanitize GEM to force uC reload
and start from known beginning.

v2: don't forget about reset path (Daniele)
 sanitize uc before gem initiated full reset (Daniele)

Signed-off-by: Michal Wajdeczko 
Cc: Daniele Ceraolo Spurio 
Cc: Sagar Arun Kamble 
Cc: Chris Wilson 
Cc: Michel Thierry 


Reviewed-by: Daniele Ceraolo Spurio 

a small comment below





diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index a45171c..abd1f7b 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -327,6 +327,24 @@ void intel_uc_fini(struct drm_i915_private *dev_priv)
intel_guc_fini(guc);
  }
  
+void intel_uc_sanitize(struct drm_i915_private *i915)

+{
+   struct intel_guc *guc = >guc;
+   struct intel_huc *huc = >huc;
+
+   if (!USES_GUC(i915))
+   return;
+
+   GEM_BUG_ON(!HAS_GUC(i915));
+
+   guc_disable_communication(guc);


With this here, can we now drop the guc_disable_communication in 
intel_uc_init_hw?


Thanks,
Daniele


+
+   intel_huc_sanitize(huc);
+   intel_guc_sanitize(guc);
+
+   __intel_uc_reset_hw(i915);
+}
+
  int intel_uc_init_hw(struct drm_i915_private *dev_priv)
  {
struct intel_guc *guc = _priv->guc;
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index dce4813..937e611 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -34,6 +34,7 @@
  void intel_uc_fini_fw(struct drm_i915_private *dev_priv);
  int intel_uc_init_misc(struct drm_i915_private *dev_priv);
  void intel_uc_fini_misc(struct drm_i915_private *dev_priv);
+void intel_uc_sanitize(struct drm_i915_private *dev_priv);
  int intel_uc_init_hw(struct drm_i915_private *dev_priv);
  void intel_uc_fini_hw(struct drm_i915_private *dev_priv);
  int intel_uc_init(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_uc_fw.h 
b/drivers/gpu/drm/i915/intel_uc_fw.h
index d5fd460..2601521 100644
--- a/drivers/gpu/drm/i915/intel_uc_fw.h
+++ b/drivers/gpu/drm/i915/intel_uc_fw.h
@@ -115,6 +115,12 @@ static inline bool intel_uc_fw_is_selected(struct 
intel_uc_fw *uc_fw)
return uc_fw->path != NULL;
  }
  
+static inline void intel_uc_fw_sanitize(struct intel_uc_fw *uc_fw)

+{
+   if (uc_fw->load_status == INTEL_UC_FIRMWARE_SUCCESS)
+   uc_fw->load_status = INTEL_UC_FIRMWARE_PENDING;
+}
+
  void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
   struct intel_uc_fw *uc_fw);
  int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,


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

[Intel-gfx] [PATCH v2 2/3] drm/i915/uc: Sanitize uC together with GEM

2018-03-09 Thread Michal Wajdeczko 
Instead of dancing around uC on reset/suspend/resume scenarios,
explicitly sanitize uC when we sanitize GEM to force uC reload
and start from known beginning.

v2: don't forget about reset path (Daniele)
sanitize uc before gem initiated full reset (Daniele)

Signed-off-by: Michal Wajdeczko 
Cc: Daniele Ceraolo Spurio 
Cc: Sagar Arun Kamble 
Cc: Chris Wilson 
Cc: Michel Thierry 
---
 drivers/gpu/drm/i915/i915_gem.c|  2 ++
 drivers/gpu/drm/i915/intel_guc.h   |  6 ++
 drivers/gpu/drm/i915/intel_huc.h   |  6 ++
 drivers/gpu/drm/i915/intel_uc.c| 18 ++
 drivers/gpu/drm/i915/intel_uc.h|  1 +
 drivers/gpu/drm/i915/intel_uc_fw.h |  6 ++
 6 files changed, 39 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index e58b741..05b0724 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2998,6 +2998,7 @@ int i915_gem_reset_prepare(struct drm_i915_private 
*dev_priv)
}
 
i915_gem_revoke_fences(dev_priv);
+   intel_uc_sanitize(dev_priv);
 
return err;
 }
@@ -4978,6 +4979,7 @@ int i915_gem_suspend(struct drm_i915_private *dev_priv)
 * machines is a good idea, we don't - just in case it leaves the
 * machine in an unusable condition.
 */
+   intel_uc_sanitize(dev_priv);
i915_gem_sanitize(dev_priv);
 
intel_runtime_pm_put(dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index b9424ac..ec8569f 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -132,4 +132,10 @@ static inline u32 guc_ggtt_offset(struct i915_vma *vma)
 struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size);
 u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv);
 
+static inline int intel_guc_sanitize(struct intel_guc *guc)
+{
+   intel_uc_fw_sanitize(>fw);
+   return 0;
+}
+
 #endif
diff --git a/drivers/gpu/drm/i915/intel_huc.h b/drivers/gpu/drm/i915/intel_huc.h
index 5d6e804..b185850 100644
--- a/drivers/gpu/drm/i915/intel_huc.h
+++ b/drivers/gpu/drm/i915/intel_huc.h
@@ -38,4 +38,10 @@ struct intel_huc {
 void intel_huc_init_early(struct intel_huc *huc);
 int intel_huc_auth(struct intel_huc *huc);
 
+static inline int intel_huc_sanitize(struct intel_huc *huc)
+{
+   intel_uc_fw_sanitize(>fw);
+   return 0;
+}
+
 #endif
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index a45171c..abd1f7b 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -327,6 +327,24 @@ void intel_uc_fini(struct drm_i915_private *dev_priv)
intel_guc_fini(guc);
 }
 
+void intel_uc_sanitize(struct drm_i915_private *i915)
+{
+   struct intel_guc *guc = >guc;
+   struct intel_huc *huc = >huc;
+
+   if (!USES_GUC(i915))
+   return;
+
+   GEM_BUG_ON(!HAS_GUC(i915));
+
+   guc_disable_communication(guc);
+
+   intel_huc_sanitize(huc);
+   intel_guc_sanitize(guc);
+
+   __intel_uc_reset_hw(i915);
+}
+
 int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 {
struct intel_guc *guc = _priv->guc;
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index dce4813..937e611 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -34,6 +34,7 @@
 void intel_uc_fini_fw(struct drm_i915_private *dev_priv);
 int intel_uc_init_misc(struct drm_i915_private *dev_priv);
 void intel_uc_fini_misc(struct drm_i915_private *dev_priv);
+void intel_uc_sanitize(struct drm_i915_private *dev_priv);
 int intel_uc_init_hw(struct drm_i915_private *dev_priv);
 void intel_uc_fini_hw(struct drm_i915_private *dev_priv);
 int intel_uc_init(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_uc_fw.h 
b/drivers/gpu/drm/i915/intel_uc_fw.h
index d5fd460..2601521 100644
--- a/drivers/gpu/drm/i915/intel_uc_fw.h
+++ b/drivers/gpu/drm/i915/intel_uc_fw.h
@@ -115,6 +115,12 @@ static inline bool intel_uc_fw_is_selected(struct 
intel_uc_fw *uc_fw)
return uc_fw->path != NULL;
 }
 
+static inline void intel_uc_fw_sanitize(struct intel_uc_fw *uc_fw)
+{
+   if (uc_fw->load_status == INTEL_UC_FIRMWARE_SUCCESS)
+   uc_fw->load_status = INTEL_UC_FIRMWARE_PENDING;
+}
+
 void intel_uc_fw_fetch(struct drm_i915_private *dev_priv,
   struct intel_uc_fw *uc_fw);
 int intel_uc_fw_upload(struct intel_uc_fw *uc_fw,
-- 
1.9.1

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