[PATCH v1] drm/i915/gsc: Fix intel_gsc_uc_fw_proxy_init_done with directed wakerefs

Alan Previn alan.previn.teres.alexis at intel.com
Thu Jun 15 21:19:40 UTC 2023


intel_gsc_uc_fw_proxy_init_done is used by a few code paths
and usages. However, certain paths need a wakeref while others
can't take a wakeref such as from the runtime_pm_resume callstack.

Add a param into this helper to allow callers to direct whether
to take the wakeref or not. This resolves the following bug:

   INFO: task sh:2607 blocked for more than 61 seconds.
   Not tainted 6.3.0-pxp-gsc-final-jun14+ #297
   "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
   task:sh              state:D stack:13016 pid:2607  ppid:2602   flags:0x00004000
   Call Trace:
      <TASK>
      __schedule+0x47b/0xe10
      schedule+0x58/0xd0
      rpm_resume+0x1cc/0x800
      ? __pfx_autoremove_wake_function+0x10/0x10
      __pm_runtime_resume+0x42/0x80
      __intel_runtime_pm_get+0x19/0x80 [i915]
      gsc_uc_get_fw_status+0x10/0x50 [i915]
      intel_gsc_uc_fw_init_done+0x9/0x20 [i915]
      intel_gsc_uc_load_start+0x5b/0x130 [i915]
      __uc_resume+0xa5/0x280 [i915]
      intel_runtime_resume+0xd4/0x250 [i915]
      ? __pfx_pci_pm_runtime_resume+0x10/0x10
   __rpm_callback+0x3c/0x160

Fixes: 8c33c3755b75 ("drm/i915/gsc: take a wakeref for the proxy-init-completion check")
Signed-off-by: Alan Previn <alan.previn.teres.alexis at intel.com>
---
 drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.c  | 17 +++++++++++------
 drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.h  |  2 +-
 drivers/gpu/drm/i915/gt/uc/intel_gsc_uc.c  |  2 +-
 drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.c |  2 +-
 4 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.c
index 856de9af1e3a..ab1a456f833d 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.c
@@ -22,27 +22,32 @@ static bool gsc_is_in_reset(struct intel_uncore *uncore)
 			HECI1_FWSTS1_CURRENT_STATE_RESET;
 }
 
-static u32 gsc_uc_get_fw_status(struct intel_uncore *uncore)
+static u32 gsc_uc_get_fw_status(struct intel_uncore *uncore, bool needs_wakeref)
 {
 	intel_wakeref_t wakeref;
 	u32 fw_status = 0;
 
-	with_intel_runtime_pm(uncore->rpm, wakeref)
-		fw_status = intel_uncore_read(uncore, HECI_FWSTS(MTL_GSC_HECI1_BASE, 1));
+	if (needs_wakeref)
+		wakeref = intel_runtime_pm_get(uncore->rpm);
 
+	fw_status = intel_uncore_read(uncore, HECI_FWSTS(MTL_GSC_HECI1_BASE, 1));
+
+	if (needs_wakeref)
+		intel_runtime_pm_put(uncore->rpm, wakeref);
 	return fw_status;
 }
 
-bool intel_gsc_uc_fw_proxy_init_done(struct intel_gsc_uc *gsc)
+bool intel_gsc_uc_fw_proxy_init_done(struct intel_gsc_uc *gsc, bool needs_wakeref)
 {
 	return REG_FIELD_GET(HECI1_FWSTS1_CURRENT_STATE,
-			     gsc_uc_get_fw_status(gsc_uc_to_gt(gsc)->uncore)) ==
+			     gsc_uc_get_fw_status(gsc_uc_to_gt(gsc)->uncore,
+						  needs_wakeref)) ==
 	       HECI1_FWSTS1_PROXY_STATE_NORMAL;
 }
 
 bool intel_gsc_uc_fw_init_done(struct intel_gsc_uc *gsc)
 {
-	return gsc_uc_get_fw_status(gsc_uc_to_gt(gsc)->uncore) &
+	return gsc_uc_get_fw_status(gsc_uc_to_gt(gsc)->uncore, false) &
 	       HECI1_FWSTS1_INIT_COMPLETE;
 }
 
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.h b/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.h
index 8d7b9e4f1ffc..ad2167ce9137 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.h
@@ -15,6 +15,6 @@ struct intel_uncore;
 int intel_gsc_fw_get_binary_info(struct intel_uc_fw *gsc_fw, const void *data, size_t size);
 int intel_gsc_uc_fw_upload(struct intel_gsc_uc *gsc);
 bool intel_gsc_uc_fw_init_done(struct intel_gsc_uc *gsc);
-bool intel_gsc_uc_fw_proxy_init_done(struct intel_gsc_uc *gsc);
+bool intel_gsc_uc_fw_proxy_init_done(struct intel_gsc_uc *gsc, bool needs_wakeref);
 
 #endif
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc.c
index 85d90f0a15e3..75a3a0790ef3 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc.c
@@ -72,7 +72,7 @@ static void gsc_work(struct work_struct *work)
 			 * complete the request handling cleanly, so we need to check the
 			 * status register to check if the proxy init was actually successful
 			 */
-			if (intel_gsc_uc_fw_proxy_init_done(gsc)) {
+			if (intel_gsc_uc_fw_proxy_init_done(gsc, false)) {
 				drm_dbg(&gt->i915->drm, "GSC Proxy initialized\n");
 				intel_uc_fw_change_status(&gsc->fw, INTEL_UC_FIRMWARE_RUNNING);
 			} else {
diff --git a/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.c b/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.c
index f13890ec7db1..c7df47364013 100644
--- a/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.c
+++ b/drivers/gpu/drm/i915/pxp/intel_pxp_gsccs.c
@@ -197,7 +197,7 @@ bool intel_pxp_gsccs_is_ready_for_sessions(struct intel_pxp *pxp)
 	 * are out of order) will suffice.
 	 */
 	if (intel_huc_is_authenticated(&pxp->ctrl_gt->uc.huc, INTEL_HUC_AUTH_BY_GSC) &&
-	    intel_gsc_uc_fw_proxy_init_done(&pxp->ctrl_gt->uc.gsc))
+	    intel_gsc_uc_fw_proxy_init_done(&pxp->ctrl_gt->uc.gsc, true))
 		return true;
 
 	return false;

base-commit: 134d180cacae82fadbc5ee32f86014cc290f5e0c
-- 
2.39.0



More information about the dri-devel mailing list