Re: [PATCH v2 6/6] drm/i915/uc: do not free err log on uc_fini

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 3/25/20 10:58 AM, John Harrison wrote:
On 3/11/2020 18:16, Daniele Ceraolo Spurio wrote:
we do call uc_fini if there is an issue while loading the GuC, so we
can't delete in there the logs we need to debug the load failure.
Moving the log free to driver remove ensures the logs stick around ong
enough for us to dump them.
I think this could be worded better and has a couple of typos.


How about:

"We need to keep the GuC error logs around to debug the load failure, so we can't clean them in the error unwind, which includes uc_fini(). Moving the cleanup to driver remove ensures that the logs stick around long enough for us to dump them."

?

Daniele

Otherwise it looks plausible.
Reviewed-by: John Harrison <John.C.Harrison@xxxxxxxxx>

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
Cc: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
---
  drivers/gpu/drm/i915/gt/intel_gt.c    | 3 +--
  drivers/gpu/drm/i915/gt/uc/intel_uc.c | 9 +++++++--
  drivers/gpu/drm/i915/gt/uc/intel_uc.h | 1 +
  3 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
index 3dea8881e915..eda66b0d44bd 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -635,8 +635,7 @@ void intel_gt_driver_remove(struct intel_gt *gt)
  {
      __intel_gt_disable(gt);
-    intel_uc_fini_hw(&gt->uc);
-    intel_uc_fini(&gt->uc);
+    intel_uc_driver_remove(&gt->uc);
      intel_engines_release(gt);
  }
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index a4cbe06e06bd..b11e564ef22e 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -131,6 +131,13 @@ static void __uc_free_load_err_log(struct intel_uc *uc)
          i915_gem_object_put(log);
  }
+void intel_uc_driver_remove(struct intel_uc *uc)
+{
+    intel_uc_fini_hw(uc);
+    intel_uc_fini(uc);
+    __uc_free_load_err_log(uc);
+}
+
  static inline bool guc_communication_enabled(struct intel_guc *guc)
  {
      return intel_guc_ct_enabled(&guc->ct);
@@ -311,8 +318,6 @@ static void __uc_fini(struct intel_uc *uc)
  {
      intel_huc_fini(&uc->huc);
      intel_guc_fini(&uc->guc);
-
-    __uc_free_load_err_log(uc);
  }
  static int __uc_sanitize(struct intel_uc *uc)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.h b/drivers/gpu/drm/i915/gt/uc/intel_uc.h
index 5ae7b50b7dc1..9c954c589edf 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.h
@@ -34,6 +34,7 @@ struct intel_uc {
  void intel_uc_init_early(struct intel_uc *uc);
  void intel_uc_driver_late_release(struct intel_uc *uc);
+void intel_uc_driver_remove(struct intel_uc *uc);
  void intel_uc_init_mmio(struct intel_uc *uc);
  void intel_uc_reset_prepare(struct intel_uc *uc);
  void intel_uc_suspend(struct intel_uc *uc);

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




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux