| Message ID | 20210506191451.77768-16-matthew.brost@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Basic GuC submission support in the i915 | expand |
On Thu, May 06, 2021 at 12:13:29PM -0700, Matthew Brost wrote: > From: Michal Wajdeczko <michal.wajdeczko@intel.com> > > In upcoming patch we will allow more CTB requests to be sent in > parallel to the GuC for procesing, so we shouldn't assume any more > that GuC will always reply without 10ms. > > Use bigger value from CONFIG_DRM_I915_HEARTBEAT_INTERVAL instead. > I think this should be its own config option or we combine it with a config option suggested in patch 37. What do you think Michal? If you agree I can fix this up in the post of these patches. Matt > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> > Signed-off-by: Matthew Brost <matthew.brost@intel.com> > --- > drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c > index c87a0a8bef26..a4b2e7fe318b 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c > @@ -436,17 +436,23 @@ static int ct_write(struct intel_guc_ct *ct, > */ > static int wait_for_ct_request_update(struct ct_request *req, u32 *status) > { > + long timeout; > int err; > > /* > * Fast commands should complete in less than 10us, so sample quickly > * up to that length of time, then switch to a slower sleep-wait loop. > * No GuC command should ever take longer than 10ms. > + * > + * However, there might be other CT requests in flight before this one, > + * so use @CONFIG_DRM_I915_HEARTBEAT_INTERVAL as backup timeout value. > */ > + timeout = max(10, CONFIG_DRM_I915_HEARTBEAT_INTERVAL); > + > #define done INTEL_GUC_MSG_IS_RESPONSE(READ_ONCE(req->status)) > err = wait_for_us(done, 10); > if (err) > - err = wait_for(done, 10); > + err = wait_for(done, timeout); > #undef done > > if (unlikely(err)) > -- > 2.28.0 >
On 25.05.2021 20:08, Matthew Brost wrote: > On Thu, May 06, 2021 at 12:13:29PM -0700, Matthew Brost wrote: >> From: Michal Wajdeczko <michal.wajdeczko@intel.com> >> >> In upcoming patch we will allow more CTB requests to be sent in >> parallel to the GuC for procesing, so we shouldn't assume any more >> that GuC will always reply without 10ms. >> >> Use bigger value from CONFIG_DRM_I915_HEARTBEAT_INTERVAL instead. >> > > I think this should be its own config option or we combine it with a > config option suggested in patch 37. > > What do you think Michal? If you agree I can fix this up in the post of > these patches. + Tvrtko yep, use of dedicated GuC CONFIG is what we also agree internally, existing HEARTBEAT_INTERVAL was just fastest way to bypass 10ms so, yes, please go ahead and do it right > > Matt > >> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> >> Signed-off-by: Matthew Brost <matthew.brost@intel.com> >> --- >> drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 8 +++++++- >> 1 file changed, 7 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c >> index c87a0a8bef26..a4b2e7fe318b 100644 >> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c >> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c >> @@ -436,17 +436,23 @@ static int ct_write(struct intel_guc_ct *ct, >> */ >> static int wait_for_ct_request_update(struct ct_request *req, u32 *status) >> { >> + long timeout; >> int err; >> >> /* >> * Fast commands should complete in less than 10us, so sample quickly >> * up to that length of time, then switch to a slower sleep-wait loop. >> * No GuC command should ever take longer than 10ms. >> + * >> + * However, there might be other CT requests in flight before this one, >> + * so use @CONFIG_DRM_I915_HEARTBEAT_INTERVAL as backup timeout value. >> */ >> + timeout = max(10, CONFIG_DRM_I915_HEARTBEAT_INTERVAL); >> + >> #define done INTEL_GUC_MSG_IS_RESPONSE(READ_ONCE(req->status)) >> err = wait_for_us(done, 10); >> if (err) >> - err = wait_for(done, 10); >> + err = wait_for(done, timeout); >> #undef done >> >> if (unlikely(err)) >> -- >> 2.28.0 >> > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx >
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c index c87a0a8bef26..a4b2e7fe318b 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c @@ -436,17 +436,23 @@ static int ct_write(struct intel_guc_ct *ct, */ static int wait_for_ct_request_update(struct ct_request *req, u32 *status) { + long timeout; int err; /* * Fast commands should complete in less than 10us, so sample quickly * up to that length of time, then switch to a slower sleep-wait loop. * No GuC command should ever take longer than 10ms. + * + * However, there might be other CT requests in flight before this one, + * so use @CONFIG_DRM_I915_HEARTBEAT_INTERVAL as backup timeout value. */ + timeout = max(10, CONFIG_DRM_I915_HEARTBEAT_INTERVAL); + #define done INTEL_GUC_MSG_IS_RESPONSE(READ_ONCE(req->status)) err = wait_for_us(done, 10); if (err) - err = wait_for(done, 10); + err = wait_for(done, timeout); #undef done if (unlikely(err))