* [PATCH 2/2] drm/amdgpu: fix fence calculation
2021-05-06 22:37 David M Nieto
@ 2021-05-06 22:37 ` David M Nieto
2021-05-07 7:18 ` Christian König
0 siblings, 1 reply; 13+ messages in thread
From: David M Nieto @ 2021-05-06 22:37 UTC (permalink / raw)
To: amd-gfx; +Cc: David M Nieto
The proper metric for fence utilization over several
contexts is an harmonic mean, but such calculation is
prohibitive in kernel space, so the code approximates it.
Because the approximation diverges when one context has a
very small ratio compared with the other context, this change
filter out ratios smaller that 0.01%
Signed-off-by: David M Nieto <david.nieto@amd.com>
Change-Id: I5b6e0ce5f489a5f55855d35354a6a3653e9d613b
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
index 9036c93b4a0c..a26496735080 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
@@ -689,6 +689,8 @@ void amdgpu_ctx_fence_time(struct amdgpu_ctx *ctx, struct amdgpu_ctx_entity *cen
}
}
+#define FENCE_USAGE_MIN_RATIO(max, total) (max > 16384ULL*total)
+
ktime_t amdgpu_ctx_mgr_fence_usage(struct amdgpu_ctx_mgr *mgr, uint32_t hwip,
uint32_t idx, uint64_t *elapsed)
{
@@ -697,17 +699,29 @@ ktime_t amdgpu_ctx_mgr_fence_usage(struct amdgpu_ctx_mgr *mgr, uint32_t hwip,
uint32_t id;
struct amdgpu_ctx_entity *centity;
ktime_t total = 0, max = 0;
+ ktime_t ttotal = 0, tmax = 0;
+
if (idx >= AMDGPU_MAX_ENTITY_NUM)
return 0;
idp = &mgr->ctx_handles;
mutex_lock(&mgr->lock);
idr_for_each_entry(idp, ctx, id) {
+ ttotal = tmax = ktime_set(0, 0);
if (!ctx->entities[hwip][idx])
continue;
centity = ctx->entities[hwip][idx];
- amdgpu_ctx_fence_time(ctx, centity, &total, &max);
+ amdgpu_ctx_fence_time(ctx, centity, &ttotal, &tmax);
+
+ /* Harmonic mean approximation diverges for very small
+ * values. If ratio < 0.01% ignore
+ */
+ if (FENCE_USAGE_MIN_RATIO(tmax, ttotal))
+ continue;
+
+ total = ktime_add(total, ttotal);
+ max = ktime_after(tmax, max) ? tmax : max;
}
mutex_unlock(&mgr->lock);
@@ -716,3 +730,5 @@ ktime_t amdgpu_ctx_mgr_fence_usage(struct amdgpu_ctx_mgr *mgr, uint32_t hwip,
return total;
}
+
+#undef FENCE_USAGE_MIN_RATIO
--
2.17.1
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] drm/amdgpu: fix fence calculation
2021-05-06 22:37 ` [PATCH 2/2] drm/amdgpu: fix fence calculation David M Nieto
@ 2021-05-07 7:18 ` Christian König
2021-05-10 20:29 ` [PATCH 1/2] drm/amdgpu: free resources on fence usage query David M Nieto
0 siblings, 1 reply; 13+ messages in thread
From: Christian König @ 2021-05-07 7:18 UTC (permalink / raw)
To: David M Nieto, amd-gfx
Am 07.05.21 um 00:37 schrieb David M Nieto:
> The proper metric for fence utilization over several
> contexts is an harmonic mean, but such calculation is
> prohibitive in kernel space, so the code approximates it.
>
> Because the approximation diverges when one context has a
> very small ratio compared with the other context, this change
> filter out ratios smaller that 0.01%
>
> Signed-off-by: David M Nieto <david.nieto@amd.com>
> Change-Id: I5b6e0ce5f489a5f55855d35354a6a3653e9d613b
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 18 +++++++++++++++++-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> index 9036c93b4a0c..a26496735080 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> @@ -689,6 +689,8 @@ void amdgpu_ctx_fence_time(struct amdgpu_ctx *ctx, struct amdgpu_ctx_entity *cen
> }
> }
>
> +#define FENCE_USAGE_MIN_RATIO(max, total) (max > 16384ULL*total)
An AMDGPU_CTX_ prefix looks appropriate here and defines should be at
the beginning of the file.
> +
> ktime_t amdgpu_ctx_mgr_fence_usage(struct amdgpu_ctx_mgr *mgr, uint32_t hwip,
> uint32_t idx, uint64_t *elapsed)
> {
> @@ -697,17 +699,29 @@ ktime_t amdgpu_ctx_mgr_fence_usage(struct amdgpu_ctx_mgr *mgr, uint32_t hwip,
> uint32_t id;
> struct amdgpu_ctx_entity *centity;
> ktime_t total = 0, max = 0;
> + ktime_t ttotal = 0, tmax = 0;
> +
>
> if (idx >= AMDGPU_MAX_ENTITY_NUM)
> return 0;
> idp = &mgr->ctx_handles;
> mutex_lock(&mgr->lock);
> idr_for_each_entry(idp, ctx, id) {
> + ttotal = tmax = ktime_set(0, 0);
Rather define the variable in the loop in the first place.
> if (!ctx->entities[hwip][idx])
> continue;
>
> centity = ctx->entities[hwip][idx];
> - amdgpu_ctx_fence_time(ctx, centity, &total, &max);
> + amdgpu_ctx_fence_time(ctx, centity, &ttotal, &tmax);
> +
> + /* Harmonic mean approximation diverges for very small
> + * values. If ratio < 0.01% ignore
> + */
> + if (FENCE_USAGE_MIN_RATIO(tmax, ttotal))
> + continue;
> +
> + total = ktime_add(total, ttotal);
> + max = ktime_after(tmax, max) ? tmax : max;
> }
>
> mutex_unlock(&mgr->lock);
> @@ -716,3 +730,5 @@ ktime_t amdgpu_ctx_mgr_fence_usage(struct amdgpu_ctx_mgr *mgr, uint32_t hwip,
>
> return total;
> }
> +
> +#undef FENCE_USAGE_MIN_RATIO
Please don't undef macros if not necessary.
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/2] drm/amdgpu: fix fence calculation
2021-05-10 20:29 ` [PATCH 1/2] drm/amdgpu: free resources on fence usage query David M Nieto
@ 2021-05-10 20:29 ` David M Nieto
2021-05-11 7:53 ` Christian König
0 siblings, 1 reply; 13+ messages in thread
From: David M Nieto @ 2021-05-10 20:29 UTC (permalink / raw)
To: amd-gfx; +Cc: David M Nieto
The proper metric for fence utilization over several
contexts is an harmonic mean, but such calculation is
prohibitive in kernel space, so the code approximates it.
Because the approximation diverges when one context has a
very small ratio compared with the other context, this change
filter out ratios smaller that 0.01%
Signed-off-by: David M Nieto <david.nieto@amd.com>
Change-Id: I5b6e0ce5f489a5f55855d35354a6a3653e9d613b
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 13 ++++++++++++-
drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h | 1 +
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
index 9036c93b4a0c..89ee464b9424 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
@@ -698,16 +698,27 @@ ktime_t amdgpu_ctx_mgr_fence_usage(struct amdgpu_ctx_mgr *mgr, uint32_t hwip,
struct amdgpu_ctx_entity *centity;
ktime_t total = 0, max = 0;
+
if (idx >= AMDGPU_MAX_ENTITY_NUM)
return 0;
idp = &mgr->ctx_handles;
mutex_lock(&mgr->lock);
idr_for_each_entry(idp, ctx, id) {
+ ktime_t ttotal = tmax = ktime_set(0, 0);
if (!ctx->entities[hwip][idx])
continue;
centity = ctx->entities[hwip][idx];
- amdgpu_ctx_fence_time(ctx, centity, &total, &max);
+ amdgpu_ctx_fence_time(ctx, centity, &ttotal, &tmax);
+
+ /* Harmonic mean approximation diverges for very small
+ * values. If ratio < 0.01% ignore
+ */
+ if (AMDGPU_CTX_FENCE_USAGE_MIN_RATIO(tmax, ttotal))
+ continue;
+
+ total = ktime_add(total, ttotal);
+ max = ktime_after(tmax, max) ? tmax : max;
}
mutex_unlock(&mgr->lock);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
index 10dcf59a5c6b..3541dfb059ec 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
@@ -30,6 +30,7 @@ struct drm_file;
struct amdgpu_fpriv;
#define AMDGPU_MAX_ENTITY_NUM 4
+#define AMDGPU_CTX_FENCE_USAGE_MIN_RATIO(max, total) (max > 16384ULL*total)
struct amdgpu_ctx_entity {
uint64_t sequence;
--
2.17.1
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] drm/amdgpu: fix fence calculation
2021-05-10 20:29 ` [PATCH 2/2] drm/amdgpu: fix fence calculation David M Nieto
@ 2021-05-11 7:53 ` Christian König
2021-05-11 18:14 ` Nieto, David M
0 siblings, 1 reply; 13+ messages in thread
From: Christian König @ 2021-05-11 7:53 UTC (permalink / raw)
To: David M Nieto, amd-gfx
Am 10.05.21 um 22:29 schrieb David M Nieto:
> The proper metric for fence utilization over several
> contexts is an harmonic mean, but such calculation is
> prohibitive in kernel space, so the code approximates it.
>
> Because the approximation diverges when one context has a
> very small ratio compared with the other context, this change
> filter out ratios smaller that 0.01%
>
> Signed-off-by: David M Nieto <david.nieto@amd.com>
> Change-Id: I5b6e0ce5f489a5f55855d35354a6a3653e9d613b
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 13 ++++++++++++-
> drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h | 1 +
> 2 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> index 9036c93b4a0c..89ee464b9424 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> @@ -698,16 +698,27 @@ ktime_t amdgpu_ctx_mgr_fence_usage(struct amdgpu_ctx_mgr *mgr, uint32_t hwip,
> struct amdgpu_ctx_entity *centity;
> ktime_t total = 0, max = 0;
>
> +
Unrelated white space change.
> if (idx >= AMDGPU_MAX_ENTITY_NUM)
> return 0;
> idp = &mgr->ctx_handles;
> mutex_lock(&mgr->lock);
> idr_for_each_entry(idp, ctx, id) {
> + ktime_t ttotal = tmax = ktime_set(0, 0);
There should be a blank line between decleration and code and please
don't initialize local variables if it isn't necessary.
Christian.
> if (!ctx->entities[hwip][idx])
> continue;
>
> centity = ctx->entities[hwip][idx];
> - amdgpu_ctx_fence_time(ctx, centity, &total, &max);
> + amdgpu_ctx_fence_time(ctx, centity, &ttotal, &tmax);
> +
> + /* Harmonic mean approximation diverges for very small
> + * values. If ratio < 0.01% ignore
> + */
> + if (AMDGPU_CTX_FENCE_USAGE_MIN_RATIO(tmax, ttotal))
> + continue;
> +
> + total = ktime_add(total, ttotal);
> + max = ktime_after(tmax, max) ? tmax : max;
> }
>
> mutex_unlock(&mgr->lock);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
> index 10dcf59a5c6b..3541dfb059ec 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
> @@ -30,6 +30,7 @@ struct drm_file;
> struct amdgpu_fpriv;
>
> #define AMDGPU_MAX_ENTITY_NUM 4
> +#define AMDGPU_CTX_FENCE_USAGE_MIN_RATIO(max, total) (max > 16384ULL*total)
>
> struct amdgpu_ctx_entity {
> uint64_t sequence;
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] drm/amdgpu: fix fence calculation
2021-05-11 7:53 ` Christian König
@ 2021-05-11 18:14 ` Nieto, David M
2021-05-12 6:55 ` Christian König
0 siblings, 1 reply; 13+ messages in thread
From: Nieto, David M @ 2021-05-11 18:14 UTC (permalink / raw)
To: Christian König, amd-gfx@lists.freedesktop.org
[-- Attachment #1.1: Type: text/plain, Size: 3222 bytes --]
[AMD Official Use Only - Internal Distribution Only]
The local variables need to be initialized to zero, since amdgpu_ctx_fence_time accumulates and does not initialize
David
________________________________
From: Christian König <ckoenig.leichtzumerken@gmail.com>
Sent: Tuesday, May 11, 2021 12:53 AM
To: Nieto, David M <David.Nieto@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 2/2] drm/amdgpu: fix fence calculation
Am 10.05.21 um 22:29 schrieb David M Nieto:
> The proper metric for fence utilization over several
> contexts is an harmonic mean, but such calculation is
> prohibitive in kernel space, so the code approximates it.
>
> Because the approximation diverges when one context has a
> very small ratio compared with the other context, this change
> filter out ratios smaller that 0.01%
>
> Signed-off-by: David M Nieto <david.nieto@amd.com>
> Change-Id: I5b6e0ce5f489a5f55855d35354a6a3653e9d613b
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 13 ++++++++++++-
> drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h | 1 +
> 2 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> index 9036c93b4a0c..89ee464b9424 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> @@ -698,16 +698,27 @@ ktime_t amdgpu_ctx_mgr_fence_usage(struct amdgpu_ctx_mgr *mgr, uint32_t hwip,
> struct amdgpu_ctx_entity *centity;
> ktime_t total = 0, max = 0;
>
> +
Unrelated white space change.
> if (idx >= AMDGPU_MAX_ENTITY_NUM)
> return 0;
> idp = &mgr->ctx_handles;
> mutex_lock(&mgr->lock);
> idr_for_each_entry(idp, ctx, id) {
> + ktime_t ttotal = tmax = ktime_set(0, 0);
There should be a blank line between decleration and code and please
don't initialize local variables if it isn't necessary.
Christian.
> if (!ctx->entities[hwip][idx])
> continue;
>
> centity = ctx->entities[hwip][idx];
> - amdgpu_ctx_fence_time(ctx, centity, &total, &max);
> + amdgpu_ctx_fence_time(ctx, centity, &ttotal, &tmax);
> +
> + /* Harmonic mean approximation diverges for very small
> + * values. If ratio < 0.01% ignore
> + */
> + if (AMDGPU_CTX_FENCE_USAGE_MIN_RATIO(tmax, ttotal))
> + continue;
> +
> + total = ktime_add(total, ttotal);
> + max = ktime_after(tmax, max) ? tmax : max;
> }
>
> mutex_unlock(&mgr->lock);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
> index 10dcf59a5c6b..3541dfb059ec 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
> @@ -30,6 +30,7 @@ struct drm_file;
> struct amdgpu_fpriv;
>
> #define AMDGPU_MAX_ENTITY_NUM 4
> +#define AMDGPU_CTX_FENCE_USAGE_MIN_RATIO(max, total) (max > 16384ULL*total)
>
> struct amdgpu_ctx_entity {
> uint64_t sequence;
[-- Attachment #1.2: Type: text/html, Size: 7368 bytes --]
[-- Attachment #2: Type: text/plain, Size: 154 bytes --]
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/2] drm/amdgpu: free resources on fence usage query
[not found] <579fa92-ad25-323a-0c41-ac07ac47fa42@gmail.com>
@ 2021-05-11 18:27 ` David M Nieto
2021-05-11 18:27 ` [PATCH 2/2] drm/amdgpu: fix fence calculation David M Nieto
0 siblings, 1 reply; 13+ messages in thread
From: David M Nieto @ 2021-05-11 18:27 UTC (permalink / raw)
To: amd-gfx; +Cc: David M Nieto
Free the resources if the fence needs to be ignored
during the ratio calculation
Signed-off-by: David M Nieto <david.nieto@amd.com>
Change-Id: Ibfc55a94c53d4b3a1dba8fff4c53fd893195bb96
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
index 01fe60fedcbe..9036c93b4a0c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
@@ -669,11 +669,15 @@ void amdgpu_ctx_fence_time(struct amdgpu_ctx *ctx, struct amdgpu_ctx_entity *cen
if (!fence)
continue;
s_fence = to_drm_sched_fence(fence);
- if (!dma_fence_is_signaled(&s_fence->scheduled))
+ if (!dma_fence_is_signaled(&s_fence->scheduled)) {
+ dma_fence_put(fence);
continue;
+ }
t1 = s_fence->scheduled.timestamp;
- if (t1 >= now)
+ if (!ktime_before(t1, now)) {
+ dma_fence_put(fence);
continue;
+ }
if (dma_fence_is_signaled(&s_fence->finished) &&
s_fence->finished.timestamp < now)
*total += ktime_sub(s_fence->finished.timestamp, t1);
--
2.17.1
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/2] drm/amdgpu: fix fence calculation
2021-05-11 18:27 ` [PATCH 1/2] drm/amdgpu: free resources on fence usage query David M Nieto
@ 2021-05-11 18:27 ` David M Nieto
0 siblings, 0 replies; 13+ messages in thread
From: David M Nieto @ 2021-05-11 18:27 UTC (permalink / raw)
To: amd-gfx; +Cc: David M Nieto
The proper metric for fence utilization over several
contexts is an harmonic mean, but such calculation is
prohibitive in kernel space, so the code approximates it.
Because the approximation diverges when one context has a
very small ratio compared with the other context, this change
filter out ratios smaller that 0.01%
Signed-off-by: David M Nieto <david.nieto@amd.com>
Change-Id: I5b6e0ce5f489a5f55855d35354a6a3653e9d613b
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 13 ++++++++++++-
drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h | 1 +
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
index 9036c93b4a0c..78579ad03e93 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
@@ -703,11 +703,22 @@ ktime_t amdgpu_ctx_mgr_fence_usage(struct amdgpu_ctx_mgr *mgr, uint32_t hwip,
idp = &mgr->ctx_handles;
mutex_lock(&mgr->lock);
idr_for_each_entry(idp, ctx, id) {
+ ktime_t ttotal = 0, tmax = 0;
+
if (!ctx->entities[hwip][idx])
continue;
centity = ctx->entities[hwip][idx];
- amdgpu_ctx_fence_time(ctx, centity, &total, &max);
+ amdgpu_ctx_fence_time(ctx, centity, &ttotal, &tmax);
+
+ /* Harmonic mean approximation diverges for very small
+ * values. If ratio < 0.01% ignore
+ */
+ if (AMDGPU_CTX_FENCE_USAGE_MIN_RATIO(tmax, ttotal))
+ continue;
+
+ total = ktime_add(total, ttotal);
+ max = ktime_after(tmax, max) ? tmax : max;
}
mutex_unlock(&mgr->lock);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
index 10dcf59a5c6b..3541dfb059ec 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
@@ -30,6 +30,7 @@ struct drm_file;
struct amdgpu_fpriv;
#define AMDGPU_MAX_ENTITY_NUM 4
+#define AMDGPU_CTX_FENCE_USAGE_MIN_RATIO(max, total) (max > 16384ULL*total)
struct amdgpu_ctx_entity {
uint64_t sequence;
--
2.17.1
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] drm/amdgpu: fix fence calculation
2021-05-11 18:14 ` Nieto, David M
@ 2021-05-12 6:55 ` Christian König
2021-05-12 6:56 ` Christian König
0 siblings, 1 reply; 13+ messages in thread
From: Christian König @ 2021-05-12 6:55 UTC (permalink / raw)
To: Nieto, David M, amd-gfx@lists.freedesktop.org
[-- Attachment #1.1: Type: text/plain, Size: 3564 bytes --]
In this case amdgpu_ctx_fence_time should probably be changed to
initialize the variable itself.
That is really bad coding style otherwise.
Christian.
Am 11.05.21 um 20:14 schrieb Nieto, David M:
>
> [AMD Official Use Only - Internal Distribution Only]
>
>
> The local variables need to be initialized to zero, since
> amdgpu_ctx_fence_time accumulates and does not initialize
>
> David
> ------------------------------------------------------------------------
> *From:* Christian König <ckoenig.leichtzumerken@gmail.com>
> *Sent:* Tuesday, May 11, 2021 12:53 AM
> *To:* Nieto, David M <David.Nieto@amd.com>;
> amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
> *Subject:* Re: [PATCH 2/2] drm/amdgpu: fix fence calculation
> Am 10.05.21 um 22:29 schrieb David M Nieto:
> > The proper metric for fence utilization over several
> > contexts is an harmonic mean, but such calculation is
> > prohibitive in kernel space, so the code approximates it.
> >
> > Because the approximation diverges when one context has a
> > very small ratio compared with the other context, this change
> > filter out ratios smaller that 0.01%
> >
> > Signed-off-by: David M Nieto <david.nieto@amd.com>
> > Change-Id: I5b6e0ce5f489a5f55855d35354a6a3653e9d613b
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 13 ++++++++++++-
> > drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h | 1 +
> > 2 files changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> > index 9036c93b4a0c..89ee464b9424 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> > @@ -698,16 +698,27 @@ ktime_t amdgpu_ctx_mgr_fence_usage(struct
> amdgpu_ctx_mgr *mgr, uint32_t hwip,
> > struct amdgpu_ctx_entity *centity;
> > ktime_t total = 0, max = 0;
> >
> > +
>
> Unrelated white space change.
>
> > if (idx >= AMDGPU_MAX_ENTITY_NUM)
> > return 0;
> > idp = &mgr->ctx_handles;
> > mutex_lock(&mgr->lock);
> > idr_for_each_entry(idp, ctx, id) {
> > + ktime_t ttotal = tmax = ktime_set(0, 0);
>
> There should be a blank line between decleration and code and please
> don't initialize local variables if it isn't necessary.
>
> Christian.
>
> > if (!ctx->entities[hwip][idx])
> > continue;
> >
> > centity = ctx->entities[hwip][idx];
> > - amdgpu_ctx_fence_time(ctx, centity, &total, &max);
> > + amdgpu_ctx_fence_time(ctx, centity, &ttotal, &tmax);
> > +
> > + /* Harmonic mean approximation diverges for very small
> > + * values. If ratio < 0.01% ignore
> > + */
> > + if (AMDGPU_CTX_FENCE_USAGE_MIN_RATIO(tmax, ttotal))
> > + continue;
> > +
> > + total = ktime_add(total, ttotal);
> > + max = ktime_after(tmax, max) ? tmax : max;
> > }
> >
> > mutex_unlock(&mgr->lock);
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
> > index 10dcf59a5c6b..3541dfb059ec 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
> > @@ -30,6 +30,7 @@ struct drm_file;
> > struct amdgpu_fpriv;
> >
> > #define AMDGPU_MAX_ENTITY_NUM 4
> > +#define AMDGPU_CTX_FENCE_USAGE_MIN_RATIO(max, total) (max >
> 16384ULL*total)
> >
> > struct amdgpu_ctx_entity {
> > uint64_t sequence;
>
[-- Attachment #1.2: Type: text/html, Size: 9097 bytes --]
[-- Attachment #2: Type: text/plain, Size: 154 bytes --]
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] drm/amdgpu: fix fence calculation
2021-05-12 6:55 ` Christian König
@ 2021-05-12 6:56 ` Christian König
2021-05-12 19:40 ` Nieto, David M
2021-05-12 19:45 ` [PATCH 1/2] drm/amdgpu: free resources on fence usage query David M Nieto
0 siblings, 2 replies; 13+ messages in thread
From: Christian König @ 2021-05-12 6:56 UTC (permalink / raw)
To: Nieto, David M, amd-gfx@lists.freedesktop.org
[-- Attachment #1.1: Type: text/plain, Size: 3786 bytes --]
And BTW amdgpu_ctx_fence_time() should probably be static.
Christian.
Am 12.05.21 um 08:55 schrieb Christian König:
> In this case amdgpu_ctx_fence_time should probably be changed to
> initialize the variable itself.
>
> That is really bad coding style otherwise.
>
> Christian.
>
> Am 11.05.21 um 20:14 schrieb Nieto, David M:
>>
>> [AMD Official Use Only - Internal Distribution Only]
>>
>>
>> The local variables need to be initialized to zero, since
>> amdgpu_ctx_fence_time accumulates and does not initialize
>>
>> David
>> ------------------------------------------------------------------------
>> *From:* Christian König <ckoenig.leichtzumerken@gmail.com>
>> *Sent:* Tuesday, May 11, 2021 12:53 AM
>> *To:* Nieto, David M <David.Nieto@amd.com>;
>> amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
>> *Subject:* Re: [PATCH 2/2] drm/amdgpu: fix fence calculation
>> Am 10.05.21 um 22:29 schrieb David M Nieto:
>> > The proper metric for fence utilization over several
>> > contexts is an harmonic mean, but such calculation is
>> > prohibitive in kernel space, so the code approximates it.
>> >
>> > Because the approximation diverges when one context has a
>> > very small ratio compared with the other context, this change
>> > filter out ratios smaller that 0.01%
>> >
>> > Signed-off-by: David M Nieto <david.nieto@amd.com>
>> > Change-Id: I5b6e0ce5f489a5f55855d35354a6a3653e9d613b
>> > ---
>> > drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 13 ++++++++++++-
>> > drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h | 1 +
>> > 2 files changed, 13 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>> > index 9036c93b4a0c..89ee464b9424 100644
>> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>> > @@ -698,16 +698,27 @@ ktime_t amdgpu_ctx_mgr_fence_usage(struct
>> amdgpu_ctx_mgr *mgr, uint32_t hwip,
>> > struct amdgpu_ctx_entity *centity;
>> > ktime_t total = 0, max = 0;
>> >
>> > +
>>
>> Unrelated white space change.
>>
>> > if (idx >= AMDGPU_MAX_ENTITY_NUM)
>> > return 0;
>> > idp = &mgr->ctx_handles;
>> > mutex_lock(&mgr->lock);
>> > idr_for_each_entry(idp, ctx, id) {
>> > + ktime_t ttotal = tmax = ktime_set(0, 0);
>>
>> There should be a blank line between decleration and code and please
>> don't initialize local variables if it isn't necessary.
>>
>> Christian.
>>
>> > if (!ctx->entities[hwip][idx])
>> > continue;
>> >
>> > centity = ctx->entities[hwip][idx];
>> > - amdgpu_ctx_fence_time(ctx, centity, &total, &max);
>> > + amdgpu_ctx_fence_time(ctx, centity, &ttotal, &tmax);
>> > +
>> > + /* Harmonic mean approximation diverges for very small
>> > + * values. If ratio < 0.01% ignore
>> > + */
>> > + if (AMDGPU_CTX_FENCE_USAGE_MIN_RATIO(tmax, ttotal))
>> > + continue;
>> > +
>> > + total = ktime_add(total, ttotal);
>> > + max = ktime_after(tmax, max) ? tmax : max;
>> > }
>> >
>> > mutex_unlock(&mgr->lock);
>> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
>> > index 10dcf59a5c6b..3541dfb059ec 100644
>> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
>> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
>> > @@ -30,6 +30,7 @@ struct drm_file;
>> > struct amdgpu_fpriv;
>> >
>> > #define AMDGPU_MAX_ENTITY_NUM 4
>> > +#define AMDGPU_CTX_FENCE_USAGE_MIN_RATIO(max, total) (max >
>> 16384ULL*total)
>> >
>> > struct amdgpu_ctx_entity {
>> > uint64_t sequence;
>>
>
[-- Attachment #1.2: Type: text/html, Size: 10545 bytes --]
[-- Attachment #2: Type: text/plain, Size: 154 bytes --]
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] drm/amdgpu: fix fence calculation
2021-05-12 6:56 ` Christian König
@ 2021-05-12 19:40 ` Nieto, David M
2021-05-12 19:45 ` [PATCH 1/2] drm/amdgpu: free resources on fence usage query David M Nieto
1 sibling, 0 replies; 13+ messages in thread
From: Nieto, David M @ 2021-05-12 19:40 UTC (permalink / raw)
To: Christian König, amd-gfx@lists.freedesktop.org
[-- Attachment #1.1: Type: text/plain, Size: 4143 bytes --]
[AMD Official Use Only - Internal Distribution Only]
yep, you are right...
I'll make those changes.
David
________________________________
From: Christian König <ckoenig.leichtzumerken@gmail.com>
Sent: Tuesday, May 11, 2021 11:56 PM
To: Nieto, David M <David.Nieto@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 2/2] drm/amdgpu: fix fence calculation
And BTW amdgpu_ctx_fence_time() should probably be static.
Christian.
Am 12.05.21 um 08:55 schrieb Christian König:
In this case amdgpu_ctx_fence_time should probably be changed to initialize the variable itself.
That is really bad coding style otherwise.
Christian.
Am 11.05.21 um 20:14 schrieb Nieto, David M:
[AMD Official Use Only - Internal Distribution Only]
The local variables need to be initialized to zero, since amdgpu_ctx_fence_time accumulates and does not initialize
David
________________________________
From: Christian König <ckoenig.leichtzumerken@gmail.com><mailto:ckoenig.leichtzumerken@gmail.com>
Sent: Tuesday, May 11, 2021 12:53 AM
To: Nieto, David M <David.Nieto@amd.com><mailto:David.Nieto@amd.com>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> <amd-gfx@lists.freedesktop.org><mailto:amd-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 2/2] drm/amdgpu: fix fence calculation
Am 10.05.21 um 22:29 schrieb David M Nieto:
> The proper metric for fence utilization over several
> contexts is an harmonic mean, but such calculation is
> prohibitive in kernel space, so the code approximates it.
>
> Because the approximation diverges when one context has a
> very small ratio compared with the other context, this change
> filter out ratios smaller that 0.01%
>
> Signed-off-by: David M Nieto <david.nieto@amd.com><mailto:david.nieto@amd.com>
> Change-Id: I5b6e0ce5f489a5f55855d35354a6a3653e9d613b
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 13 ++++++++++++-
> drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h | 1 +
> 2 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> index 9036c93b4a0c..89ee464b9424 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> @@ -698,16 +698,27 @@ ktime_t amdgpu_ctx_mgr_fence_usage(struct amdgpu_ctx_mgr *mgr, uint32_t hwip,
> struct amdgpu_ctx_entity *centity;
> ktime_t total = 0, max = 0;
>
> +
Unrelated white space change.
> if (idx >= AMDGPU_MAX_ENTITY_NUM)
> return 0;
> idp = &mgr->ctx_handles;
> mutex_lock(&mgr->lock);
> idr_for_each_entry(idp, ctx, id) {
> + ktime_t ttotal = tmax = ktime_set(0, 0);
There should be a blank line between decleration and code and please
don't initialize local variables if it isn't necessary.
Christian.
> if (!ctx->entities[hwip][idx])
> continue;
>
> centity = ctx->entities[hwip][idx];
> - amdgpu_ctx_fence_time(ctx, centity, &total, &max);
> + amdgpu_ctx_fence_time(ctx, centity, &ttotal, &tmax);
> +
> + /* Harmonic mean approximation diverges for very small
> + * values. If ratio < 0.01% ignore
> + */
> + if (AMDGPU_CTX_FENCE_USAGE_MIN_RATIO(tmax, ttotal))
> + continue;
> +
> + total = ktime_add(total, ttotal);
> + max = ktime_after(tmax, max) ? tmax : max;
> }
>
> mutex_unlock(&mgr->lock);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
> index 10dcf59a5c6b..3541dfb059ec 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
> @@ -30,6 +30,7 @@ struct drm_file;
> struct amdgpu_fpriv;
>
> #define AMDGPU_MAX_ENTITY_NUM 4
> +#define AMDGPU_CTX_FENCE_USAGE_MIN_RATIO(max, total) (max > 16384ULL*total)
>
> struct amdgpu_ctx_entity {
> uint64_t sequence;
[-- Attachment #1.2: Type: text/html, Size: 9193 bytes --]
[-- Attachment #2: Type: text/plain, Size: 154 bytes --]
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/2] drm/amdgpu: fix fence calculation
2021-05-12 19:45 ` [PATCH 1/2] drm/amdgpu: free resources on fence usage query David M Nieto
@ 2021-05-12 19:45 ` David M Nieto
2021-05-13 8:11 ` Christian König
0 siblings, 1 reply; 13+ messages in thread
From: David M Nieto @ 2021-05-12 19:45 UTC (permalink / raw)
To: amd-gfx; +Cc: David M Nieto
The proper metric for fence utilization over several
contexts is an harmonic mean, but such calculation is
prohibitive in kernel space, so the code approximates it.
Because the approximation diverges when one context has a
very small ratio compared with the other context, this change
filter out ratios smaller that 0.01%
Signed-off-by: David M Nieto <david.nieto@amd.com>
Change-Id: I5b6e0ce5f489a5f55855d35354a6a3653e9d613b
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 17 +++++++++++++++--
drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h | 1 +
2 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
index 9036c93b4a0c..b919615e6644 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
@@ -652,12 +652,14 @@ void amdgpu_ctx_mgr_fini(struct amdgpu_ctx_mgr *mgr)
mutex_destroy(&mgr->lock);
}
-void amdgpu_ctx_fence_time(struct amdgpu_ctx *ctx, struct amdgpu_ctx_entity *centity,
+static void amdgpu_ctx_fence_time(struct amdgpu_ctx *ctx, struct amdgpu_ctx_entity *centity,
ktime_t *total, ktime_t *max)
{
ktime_t now, t1;
uint32_t i;
+ *total = *max = 0;
+
now = ktime_get();
for (i = 0; i < amdgpu_sched_jobs; i++) {
struct dma_fence *fence;
@@ -703,11 +705,22 @@ ktime_t amdgpu_ctx_mgr_fence_usage(struct amdgpu_ctx_mgr *mgr, uint32_t hwip,
idp = &mgr->ctx_handles;
mutex_lock(&mgr->lock);
idr_for_each_entry(idp, ctx, id) {
+ ktime_t ttotal, tmax;
+
if (!ctx->entities[hwip][idx])
continue;
centity = ctx->entities[hwip][idx];
- amdgpu_ctx_fence_time(ctx, centity, &total, &max);
+ amdgpu_ctx_fence_time(ctx, centity, &ttotal, &tmax);
+
+ /* Harmonic mean approximation diverges for very small
+ * values. If ratio < 0.01% ignore
+ */
+ if (AMDGPU_CTX_FENCE_USAGE_MIN_RATIO(tmax, ttotal))
+ continue;
+
+ total = ktime_add(total, ttotal);
+ max = ktime_after(tmax, max) ? tmax : max;
}
mutex_unlock(&mgr->lock);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
index 10dcf59a5c6b..3541dfb059ec 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
@@ -30,6 +30,7 @@ struct drm_file;
struct amdgpu_fpriv;
#define AMDGPU_MAX_ENTITY_NUM 4
+#define AMDGPU_CTX_FENCE_USAGE_MIN_RATIO(max, total) (max > 16384ULL*total)
struct amdgpu_ctx_entity {
uint64_t sequence;
--
2.17.1
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] drm/amdgpu: fix fence calculation
2021-05-12 19:45 ` [PATCH 2/2] drm/amdgpu: fix fence calculation David M Nieto
@ 2021-05-13 8:11 ` Christian König
2021-05-13 17:42 ` Nieto, David M
0 siblings, 1 reply; 13+ messages in thread
From: Christian König @ 2021-05-13 8:11 UTC (permalink / raw)
To: David M Nieto, amd-gfx
Am 12.05.21 um 21:45 schrieb David M Nieto:
> The proper metric for fence utilization over several
> contexts is an harmonic mean, but such calculation is
> prohibitive in kernel space, so the code approximates it.
>
> Because the approximation diverges when one context has a
> very small ratio compared with the other context, this change
> filter out ratios smaller that 0.01%
>
> Signed-off-by: David M Nieto <david.nieto@amd.com>
> Change-Id: I5b6e0ce5f489a5f55855d35354a6a3653e9d613b
Looks good to me now, but the automated tools complain about "DOS line
endings" plus a couple of other nit picks and won't let me push it :)
Please use the checkpatch.pl script found in the Linux kernel to fix
those errors and resend.
Thanks,
Christian.
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 17 +++++++++++++++--
> drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h | 1 +
> 2 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> index 9036c93b4a0c..b919615e6644 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> @@ -652,12 +652,14 @@ void amdgpu_ctx_mgr_fini(struct amdgpu_ctx_mgr *mgr)
> mutex_destroy(&mgr->lock);
> }
>
> -void amdgpu_ctx_fence_time(struct amdgpu_ctx *ctx, struct amdgpu_ctx_entity *centity,
> +static void amdgpu_ctx_fence_time(struct amdgpu_ctx *ctx, struct amdgpu_ctx_entity *centity,
> ktime_t *total, ktime_t *max)
> {
> ktime_t now, t1;
> uint32_t i;
>
> + *total = *max = 0;
> +
> now = ktime_get();
> for (i = 0; i < amdgpu_sched_jobs; i++) {
> struct dma_fence *fence;
> @@ -703,11 +705,22 @@ ktime_t amdgpu_ctx_mgr_fence_usage(struct amdgpu_ctx_mgr *mgr, uint32_t hwip,
> idp = &mgr->ctx_handles;
> mutex_lock(&mgr->lock);
> idr_for_each_entry(idp, ctx, id) {
> + ktime_t ttotal, tmax;
> +
> if (!ctx->entities[hwip][idx])
> continue;
>
> centity = ctx->entities[hwip][idx];
> - amdgpu_ctx_fence_time(ctx, centity, &total, &max);
> + amdgpu_ctx_fence_time(ctx, centity, &ttotal, &tmax);
> +
> + /* Harmonic mean approximation diverges for very small
> + * values. If ratio < 0.01% ignore
> + */
> + if (AMDGPU_CTX_FENCE_USAGE_MIN_RATIO(tmax, ttotal))
> + continue;
> +
> + total = ktime_add(total, ttotal);
> + max = ktime_after(tmax, max) ? tmax : max;
> }
>
> mutex_unlock(&mgr->lock);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
> index 10dcf59a5c6b..3541dfb059ec 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
> @@ -30,6 +30,7 @@ struct drm_file;
> struct amdgpu_fpriv;
>
> #define AMDGPU_MAX_ENTITY_NUM 4
> +#define AMDGPU_CTX_FENCE_USAGE_MIN_RATIO(max, total) (max > 16384ULL*total)
>
> struct amdgpu_ctx_entity {
> uint64_t sequence;
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] drm/amdgpu: fix fence calculation
2021-05-13 8:11 ` Christian König
@ 2021-05-13 17:42 ` Nieto, David M
0 siblings, 0 replies; 13+ messages in thread
From: Nieto, David M @ 2021-05-13 17:42 UTC (permalink / raw)
To: Christian König, amd-gfx@lists.freedesktop.org
[-- Attachment #1.1: Type: text/plain, Size: 3715 bytes --]
[AMD Official Use Only - Internal Distribution Only]
DOS line endings? That is weird....
I corrected the issues that showed in checkpatch.pl (the > 80 lines)
I'll re-upload
________________________________
From: Christian König <ckoenig.leichtzumerken@gmail.com>
Sent: Thursday, May 13, 2021 1:11 AM
To: Nieto, David M <David.Nieto@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 2/2] drm/amdgpu: fix fence calculation
Am 12.05.21 um 21:45 schrieb David M Nieto:
> The proper metric for fence utilization over several
> contexts is an harmonic mean, but such calculation is
> prohibitive in kernel space, so the code approximates it.
>
> Because the approximation diverges when one context has a
> very small ratio compared with the other context, this change
> filter out ratios smaller that 0.01%
>
> Signed-off-by: David M Nieto <david.nieto@amd.com>
> Change-Id: I5b6e0ce5f489a5f55855d35354a6a3653e9d613b
Looks good to me now, but the automated tools complain about "DOS line
endings" plus a couple of other nit picks and won't let me push it :)
Please use the checkpatch.pl script found in the Linux kernel to fix
those errors and resend.
Thanks,
Christian.
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 17 +++++++++++++++--
> drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h | 1 +
> 2 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> index 9036c93b4a0c..b919615e6644 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> @@ -652,12 +652,14 @@ void amdgpu_ctx_mgr_fini(struct amdgpu_ctx_mgr *mgr)
> mutex_destroy(&mgr->lock);
> }
>
> -void amdgpu_ctx_fence_time(struct amdgpu_ctx *ctx, struct amdgpu_ctx_entity *centity,
> +static void amdgpu_ctx_fence_time(struct amdgpu_ctx *ctx, struct amdgpu_ctx_entity *centity,
> ktime_t *total, ktime_t *max)
> {
> ktime_t now, t1;
> uint32_t i;
>
> + *total = *max = 0;
> +
> now = ktime_get();
> for (i = 0; i < amdgpu_sched_jobs; i++) {
> struct dma_fence *fence;
> @@ -703,11 +705,22 @@ ktime_t amdgpu_ctx_mgr_fence_usage(struct amdgpu_ctx_mgr *mgr, uint32_t hwip,
> idp = &mgr->ctx_handles;
> mutex_lock(&mgr->lock);
> idr_for_each_entry(idp, ctx, id) {
> + ktime_t ttotal, tmax;
> +
> if (!ctx->entities[hwip][idx])
> continue;
>
> centity = ctx->entities[hwip][idx];
> - amdgpu_ctx_fence_time(ctx, centity, &total, &max);
> + amdgpu_ctx_fence_time(ctx, centity, &ttotal, &tmax);
> +
> + /* Harmonic mean approximation diverges for very small
> + * values. If ratio < 0.01% ignore
> + */
> + if (AMDGPU_CTX_FENCE_USAGE_MIN_RATIO(tmax, ttotal))
> + continue;
> +
> + total = ktime_add(total, ttotal);
> + max = ktime_after(tmax, max) ? tmax : max;
> }
>
> mutex_unlock(&mgr->lock);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
> index 10dcf59a5c6b..3541dfb059ec 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
> @@ -30,6 +30,7 @@ struct drm_file;
> struct amdgpu_fpriv;
>
> #define AMDGPU_MAX_ENTITY_NUM 4
> +#define AMDGPU_CTX_FENCE_USAGE_MIN_RATIO(max, total) (max > 16384ULL*total)
>
> struct amdgpu_ctx_entity {
> uint64_t sequence;
[-- Attachment #1.2: Type: text/html, Size: 7522 bytes --]
[-- Attachment #2: Type: text/plain, Size: 154 bytes --]
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2021-05-13 17:42 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <579fa92-ad25-323a-0c41-ac07ac47fa42@gmail.com>
2021-05-11 18:27 ` [PATCH 1/2] drm/amdgpu: free resources on fence usage query David M Nieto
2021-05-11 18:27 ` [PATCH 2/2] drm/amdgpu: fix fence calculation David M Nieto
2021-05-06 22:37 David M Nieto
2021-05-06 22:37 ` [PATCH 2/2] drm/amdgpu: fix fence calculation David M Nieto
2021-05-07 7:18 ` Christian König
2021-05-10 20:29 ` [PATCH 1/2] drm/amdgpu: free resources on fence usage query David M Nieto
2021-05-10 20:29 ` [PATCH 2/2] drm/amdgpu: fix fence calculation David M Nieto
2021-05-11 7:53 ` Christian König
2021-05-11 18:14 ` Nieto, David M
2021-05-12 6:55 ` Christian König
2021-05-12 6:56 ` Christian König
2021-05-12 19:40 ` Nieto, David M
2021-05-12 19:45 ` [PATCH 1/2] drm/amdgpu: free resources on fence usage query David M Nieto
2021-05-12 19:45 ` [PATCH 2/2] drm/amdgpu: fix fence calculation David M Nieto
2021-05-13 8:11 ` Christian König
2021-05-13 17:42 ` Nieto, David M
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox