Re: [Intel-gfx] [RFC PATCH 15/97] drm/i915/guc: Relax CTB response timeout
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
>>
>> 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
>> Signed-off-by: Matthew Brost
>> ---
>> 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
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC PATCH 15/97] drm/i915/guc: Relax CTB response timeout
On Thu, May 06, 2021 at 12:13:29PM -0700, Matthew Brost wrote:
> From: Michal Wajdeczko
>
> 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
> Signed-off-by: Matthew Brost
> ---
> 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
[Intel-gfx] [RFC PATCH 15/97] drm/i915/guc: Relax CTB response timeout
From: Michal Wajdeczko
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.
Signed-off-by: Michal Wajdeczko
Signed-off-by: Matthew Brost
---
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
