From: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> To: intel-gfx@lists.freedesktop.org Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>, Alan Previn <alan.previn.teres.alexis@intel.com>, dri-devel@lists.freedesktop.org Subject: [PATCH 2/2] drm/i915/gsc: Fix race between HW init and GSC FW load Date: Thu, 23 Feb 2023 09:21:20 -0800 [thread overview] Message-ID: <20230223172120.3304293-3-daniele.ceraolospurio@intel.com> (raw) In-Reply-To: <20230223172120.3304293-1-daniele.ceraolospurio@intel.com> 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> --- drivers/gpu/drm/i915/gt/uc/intel_gsc_uc.c | 19 +++++++++++++++++++ drivers/gpu/drm/i915/gt/uc/intel_gsc_uc.h | 1 + drivers/gpu/drm/i915/gt/uc/intel_uc.c | 5 +++-- 3 files changed, 23 insertions(+), 2 deletions(-) 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 92e1571fdc46..2d5b70b3384c 100644 --- 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 c8b025343ea6..5f50fa1ff8b9 100644 --- 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 5fa5c0999212..1b7ecd384a79 100644 --- 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; } -- 2.37.3
WARNING: multiple messages have this Message-ID (diff)
From: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> To: intel-gfx@lists.freedesktop.org Cc: Alan Previn <alan.previn.teres.alexis@intel.com>, dri-devel@lists.freedesktop.org Subject: [Intel-gfx] [PATCH 2/2] drm/i915/gsc: Fix race between HW init and GSC FW load Date: Thu, 23 Feb 2023 09:21:20 -0800 [thread overview] Message-ID: <20230223172120.3304293-3-daniele.ceraolospurio@intel.com> (raw) In-Reply-To: <20230223172120.3304293-1-daniele.ceraolospurio@intel.com> 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> --- drivers/gpu/drm/i915/gt/uc/intel_gsc_uc.c | 19 +++++++++++++++++++ drivers/gpu/drm/i915/gt/uc/intel_gsc_uc.h | 1 + drivers/gpu/drm/i915/gt/uc/intel_uc.c | 5 +++-- 3 files changed, 23 insertions(+), 2 deletions(-) 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 92e1571fdc46..2d5b70b3384c 100644 --- 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 c8b025343ea6..5f50fa1ff8b9 100644 --- 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 5fa5c0999212..1b7ecd384a79 100644 --- 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; } -- 2.37.3
next prev parent reply other threads:[~2023-02-23 17:14 UTC|newest] Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-02-23 17:21 [PATCH 0/2] Fix a couple of issues with the GSC worker timing Daniele Ceraolo Spurio 2023-02-23 17:21 ` [Intel-gfx] " Daniele Ceraolo Spurio 2023-02-23 17:21 ` [PATCH 1/2] drm/i915/gsc: flush the GSC worker before wedging on unload Daniele Ceraolo Spurio 2023-02-23 17:21 ` [Intel-gfx] " Daniele Ceraolo Spurio 2023-03-07 22:55 ` Teres Alexis, Alan Previn 2023-03-07 22:55 ` [Intel-gfx] " Teres Alexis, Alan Previn 2023-02-23 17:21 ` Daniele Ceraolo Spurio [this message] 2023-02-23 17:21 ` [Intel-gfx] [PATCH 2/2] drm/i915/gsc: Fix race between HW init and GSC FW load Daniele Ceraolo Spurio 2023-03-07 21:50 ` Teres Alexis, Alan Previn 2023-03-07 21:50 ` [Intel-gfx] " Teres Alexis, Alan Previn 2023-02-23 19:32 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for Fix a couple of issues with the GSC worker timing Patchwork 2023-02-23 19:56 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork 2023-02-24 2:40 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork 2023-03-08 1:11 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for Fix a couple of issues with the GSC worker timing (rev2) Patchwork 2023-03-08 1:34 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork 2023-03-09 17:32 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork 2023-03-10 0:49 ` Ceraolo Spurio, Daniele
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20230223172120.3304293-3-daniele.ceraolospurio@intel.com \ --to=daniele.ceraolospurio@intel.com \ --cc=alan.previn.teres.alexis@intel.com \ --cc=dri-devel@lists.freedesktop.org \ --cc=intel-gfx@lists.freedesktop.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.