Re: [PATCH] drm/i915/gsc: Fix the Driver-FLR completion

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

 





On 2/23/2023 2:04 PM, Alan Previn wrote:
The Driver-FLR flow may inadvertently exit early before the full
completion of the re-init of the internal HW state if we only poll
GU_DEBUG Bit31 (polling for it to toggle from 0 -> 1). Instead
we need a two-step completion wait-for-completion flow that also
involves GU_CNTL. See the patch and new code comments for detail.
This is new direction from HW architecture folks.

    v2: - Add error message for the teardown timeout (Anshuman)
        - Don't duplicate code in comments (Jani)
    v3: - Add get/put runtime-pm for this function. Though
          not functionally required during unload, its so the uncore
	 doesn't complain.

I think my comment applied to an older version of this code. The FLR is done after we've already disabled rpm (from driver_unregister), so we shouldn't need to take a ref here. If this ever changes, then the rpm get/put should be added to the caller (uncore_fini_mmio) because there are other register accesses performed by that function and we should cover them all. Given this, IMO we should revert back to the v2 version of this patch, so I'm going to review that one.

Daniele


Signed-off-by: Alan Previn <alan.previn.teres.alexis@xxxxxxxxx>
Fixes: 5a44fcd73498 ("drm/i915/gsc: Do a driver-FLR on unload if GSC was loaded")
---
  drivers/gpu/drm/i915/intel_uncore.c | 23 ++++++++++++++++++++---
  1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index f018da7ebaac..9832b8ac8b1a 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -2724,10 +2724,13 @@ static void driver_initiated_flr(struct intel_uncore *uncore)
  {
  	struct drm_i915_private *i915 = uncore->i915;
  	const unsigned int flr_timeout_ms = 3000; /* specs recommend a 3s wait */
+	intel_wakeref_t wakeref;
  	int ret;
drm_dbg(&i915->drm, "Triggering Driver-FLR\n"); + wakeref = intel_runtime_pm_get(&i915->runtime_pm);
+
  	/*
  	 * Make sure any pending FLR requests have cleared by waiting for the
  	 * FLR trigger bit to go to zero. Also clear GU_DEBUG's DRIVERFLR_STATUS
@@ -2742,22 +2745,36 @@ static void driver_initiated_flr(struct intel_uncore *uncore)
  		drm_err(&i915->drm,
  			"Failed to wait for Driver-FLR bit to clear! %d\n",
  			ret);
-		return;
+		goto out;
  	}
  	intel_uncore_write_fw(uncore, GU_DEBUG, DRIVERFLR_STATUS);
/* Trigger the actual Driver-FLR */
  	intel_uncore_rmw_fw(uncore, GU_CNTL, 0, DRIVERFLR);
+ /* Wait for hardware teardown to complete */
+	ret = intel_wait_for_register_fw(uncore, GU_CNTL,
+					 DRIVERFLR_STATUS, 0,
+					 flr_timeout_ms);
+	if (ret) {
+		drm_err(&i915->drm, "Driver-FLR-teardown wait completion failed! %d\n", ret);
+		goto out;
+	}
+
+	/* Wait for hardware/firmware re-init to complete */
  	ret = intel_wait_for_register_fw(uncore, GU_DEBUG,
  					 DRIVERFLR_STATUS, DRIVERFLR_STATUS,
  					 flr_timeout_ms);
  	if (ret) {
-		drm_err(&i915->drm, "wait for Driver-FLR completion failed! %d\n", ret);
-		return;
+		drm_err(&i915->drm, "Driver-FLR-reinit wait completion failed! %d\n", ret);
+		goto out;
  	}
+ /* Clear sticky completion status */
  	intel_uncore_write_fw(uncore, GU_DEBUG, DRIVERFLR_STATUS);
+
+out:
+	intel_runtime_pm_put(&i915->runtime_pm, wakeref);
  }
/* Called via drm-managed action */




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

  Powered by Linux