drm/i915/gsc: Fix race between HW init and GSC FW load

The GSC FW load is a slow process (up to 250 ms), so we defer it to a
dedicated worker to avoid stalling the init flow for that long. However,
we currently start this worker before the HW init is complete, so there
is a chance that the GSC loading code submits to the HW before the
engine initialization has completed. We can easily fix this by starting
the thread later in the gt_resume flow.
From this later spot, the GSC code can still race with the default
submission code; we functionally don't care who wins the race (the GSC
load doesn't need any state), but since the whole point of the separate
worker is to make the main thread faster, we prefer the default
submission code to run first. Therefore, make an exception for driver
probe and only and start the gsc load from uc_init_late.

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Alan Previn <alan.previn.teres.alexis@intel.com>
Reviewed-by: Alan Previn <alan.previn.teres.alexis@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20230223172120.3304293-3-daniele.ceraolospurio@intel.com

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 92e1571fdc4647ff45c719c6c2ce62eb8be2edd4..2d5b70b3384cbc4b5b8b58d01d671a5e601a2689 100644 (file)
--- a/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc.c
@@ -124,6 +124,25 @@ void intel_gsc_uc_flush_work(struct intel_gsc_uc *gsc)
        flush_work(&gsc->work);
 }
 
+void intel_gsc_uc_resume(struct intel_gsc_uc *gsc)
+{
+       if (!intel_uc_fw_is_loadable(&gsc->fw))
+               return;
+
+       /*
+        * we only want to start the GSC worker from here in the actual resume
+        * flow and not during driver load. This is because GSC load is slow and
+        * therefore we want to make sure that the default state init completes
+        * first to not slow down the init thread. A separate call to
+        * intel_gsc_uc_load_start will ensure that the GSC is loaded during
+        * driver load.
+        */
+       if (!gsc_uc_to_gt(gsc)->engine[GSC0]->default_state)
+               return;
+
+       intel_gsc_uc_load_start(gsc);
+}
+
 void intel_gsc_uc_load_start(struct intel_gsc_uc *gsc)
 {
        if (!intel_uc_fw_is_loadable(&gsc->fw))

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc.h b/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc.h
index c8b025343ea61abb55458befcc936b0f36305c75..5f50fa1ff8b939b02c8e7b93ff76f4f5e7c837ad 100644 (file)
--- a/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc.h
@@ -26,6 +26,7 @@ void intel_gsc_uc_init_early(struct intel_gsc_uc *gsc);
 int intel_gsc_uc_init(struct intel_gsc_uc *gsc);
 void intel_gsc_uc_fini(struct intel_gsc_uc *gsc);
 void intel_gsc_uc_suspend(struct intel_gsc_uc *gsc);
+void intel_gsc_uc_resume(struct intel_gsc_uc *gsc);
 void intel_gsc_uc_flush_work(struct intel_gsc_uc *gsc);
 void intel_gsc_uc_load_start(struct intel_gsc_uc *gsc);
 

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index 5fa5c09992121841777d6e2547147310a43f79ef..1b7ecd384a796a843628706bc3c9d58018ca2eac 100644 (file)
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -139,6 +139,7 @@ void intel_uc_init_early(struct intel_uc *uc)
 void intel_uc_init_late(struct intel_uc *uc)
 {
        intel_guc_init_late(&uc->guc);
+       intel_gsc_uc_load_start(&uc->gsc);
 }
 
 void intel_uc_driver_late_release(struct intel_uc *uc)
@@ -543,8 +544,6 @@ static int __uc_init_hw(struct intel_uc *uc)
                intel_rps_lower_unslice(&uc_to_gt(uc)->rps);
        }
 
-       intel_gsc_uc_load_start(&uc->gsc);
-
        guc_info(guc, "submission %s\n", str_enabled_disabled(intel_uc_uses_guc_submission(uc)));
        guc_info(guc, "SLPC %s\n", str_enabled_disabled(intel_uc_uses_guc_slpc(uc)));
 
@@ -714,6 +713,8 @@ static int __uc_resume(struct intel_uc *uc, bool enable_communication)
                return err;
        }
 
+       intel_gsc_uc_resume(&uc->gsc);
+
        return 0;
 }