* [Intel-gfx] [PATCH 0/2] fdinfo: Enable some support for GuC based client busyness
@ 2023-04-05 0:14 Umesh Nerlige Ramappa
2023-04-05 0:14 ` [Intel-gfx] [PATCH 1/2] i915/pmu: Add support for total context runtime for GuC back-end Umesh Nerlige Ramappa
` (4 more replies)
0 siblings, 5 replies; 12+ messages in thread
From: Umesh Nerlige Ramappa @ 2023-04-05 0:14 UTC (permalink / raw)
To: intel-gfx
Export context runtime into the fdinfo framework to enable per client
busyness for GuC back-end.
v2: Fix zeroed busyness values
Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
Test-with: 20230405000909.51175-1-umesh.nerlige.ramappa@intel.com
Umesh Nerlige Ramappa (2):
i915/pmu: Add support for total context runtime for GuC back-end
drm/i915/fdinfo: Enable fdinfo for GuC backends
drivers/gpu/drm/i915/gt/intel_context.c | 12 +++++--
drivers/gpu/drm/i915/gt/intel_context.h | 2 +-
drivers/gpu/drm/i915/gt/intel_context_types.h | 5 +++
.../gpu/drm/i915/gt/uc/intel_guc_submission.c | 33 +++++++++++++++++++
drivers/gpu/drm/i915/i915_drm_client.c | 6 +---
5 files changed, 50 insertions(+), 8 deletions(-)
--
2.36.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Intel-gfx] [PATCH 1/2] i915/pmu: Add support for total context runtime for GuC back-end
2023-04-05 0:14 [Intel-gfx] [PATCH 0/2] fdinfo: Enable some support for GuC based client busyness Umesh Nerlige Ramappa
@ 2023-04-05 0:14 ` Umesh Nerlige Ramappa
2023-04-24 17:41 ` Dixit, Ashutosh
2023-04-05 0:14 ` [Intel-gfx] [PATCH 2/2] drm/i915/fdinfo: Enable fdinfo for GuC backends Umesh Nerlige Ramappa
` (3 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Umesh Nerlige Ramappa @ 2023-04-05 0:14 UTC (permalink / raw)
To: intel-gfx
GPU accumulates the context runtime in a 32 bit counter - CTX_TIMESTAMP
in the context image. This value is saved/restored on context switches.
KMD accumulates these values into a 64 bit counter taking care of any
overflows as needed. This count provides the basis for client specific
busyness in the fdinfo interface.
KMD accumulation happens just before the context is unpinned and when
context switches out. This works for execlist back-end since execlist
scheduling has visibility into context switches. With GuC mode, KMD does
not have visibility into context switches and this counter is
accumulated only when context is unpinned. Context is unpinned once the
context scheduling is successfully disabled. Disabling context
scheduling is an asynchronous operation. Also if a context is servicing
frequent requests, scheduling may never be disabled on it.
For GuC mode, since updates to the context runtime may be delayed, add
hooks to update the context runtime in a worker thread as well as when
a user queries for it.
Limitation:
- If a context is never switched out or runs for a long period of time,
the runtime value of CTX_TIMESTAMP may never be updated, so the
counter value may be unreliable. This patch does not support such
cases. Such support must be available from the GuC FW and it is WIP.
This patch is an extract from previous work authored by John/Umesh here -
https://patchwork.freedesktop.org/patch/496441/?series=105085&rev=4
Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
Co-developed-by: John Harrison <John.C.Harrison@Intel.com>
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
---
drivers/gpu/drm/i915/gt/intel_context.c | 12 +++++--
drivers/gpu/drm/i915/gt/intel_context.h | 2 +-
drivers/gpu/drm/i915/gt/intel_context_types.h | 5 +++
.../gpu/drm/i915/gt/uc/intel_guc_submission.c | 33 +++++++++++++++++++
4 files changed, 49 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c
index 2aa63ec521b8..e01f222e9e42 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.c
+++ b/drivers/gpu/drm/i915/gt/intel_context.c
@@ -578,16 +578,24 @@ void intel_context_bind_parent_child(struct intel_context *parent,
child->parallel.parent = parent;
}
-u64 intel_context_get_total_runtime_ns(const struct intel_context *ce)
+u64 intel_context_get_total_runtime_ns(struct intel_context *ce)
{
u64 total, active;
+ if (ce->ops->update_stats)
+ ce->ops->update_stats(ce);
+
total = ce->stats.runtime.total;
if (ce->ops->flags & COPS_RUNTIME_CYCLES)
total *= ce->engine->gt->clock_period_ns;
active = READ_ONCE(ce->stats.active);
- if (active)
+ /*
+ * When COPS_RUNTIME_ACTIVE_TOTAL is set for ce->cops, the backend
+ * already provides the total active time of the context, so skip this
+ * calculation when this flag is set.
+ */
+ if (active && !(ce->ops->flags & COPS_RUNTIME_ACTIVE_TOTAL))
active = intel_context_clock() - active;
return total + active;
diff --git a/drivers/gpu/drm/i915/gt/intel_context.h b/drivers/gpu/drm/i915/gt/intel_context.h
index 0a8d553da3f4..720809523e2d 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.h
+++ b/drivers/gpu/drm/i915/gt/intel_context.h
@@ -368,7 +368,7 @@ intel_context_clear_nopreempt(struct intel_context *ce)
clear_bit(CONTEXT_NOPREEMPT, &ce->flags);
}
-u64 intel_context_get_total_runtime_ns(const struct intel_context *ce);
+u64 intel_context_get_total_runtime_ns(struct intel_context *ce);
u64 intel_context_get_avg_runtime_ns(struct intel_context *ce);
static inline u64 intel_context_clock(void)
diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h b/drivers/gpu/drm/i915/gt/intel_context_types.h
index e36670f2e626..58b0294d359d 100644
--- a/drivers/gpu/drm/i915/gt/intel_context_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_context_types.h
@@ -38,6 +38,9 @@ struct intel_context_ops {
#define COPS_RUNTIME_CYCLES_BIT 1
#define COPS_RUNTIME_CYCLES BIT(COPS_RUNTIME_CYCLES_BIT)
+#define COPS_RUNTIME_ACTIVE_TOTAL_BIT 2
+#define COPS_RUNTIME_ACTIVE_TOTAL BIT(COPS_RUNTIME_ACTIVE_TOTAL_BIT)
+
int (*alloc)(struct intel_context *ce);
void (*revoke)(struct intel_context *ce, struct i915_request *rq,
@@ -58,6 +61,8 @@ struct intel_context_ops {
void (*sched_disable)(struct intel_context *ce);
+ void (*update_stats)(struct intel_context *ce);
+
void (*reset)(struct intel_context *ce);
void (*destroy)(struct kref *kref);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index 88e881b100cf..8048a3e97a68 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -1402,13 +1402,25 @@ static void __update_guc_busyness_stats(struct intel_guc *guc)
spin_unlock_irqrestore(&guc->timestamp.lock, flags);
}
+static void guc_context_update_clks(struct intel_context *ce)
+{
+ struct intel_guc *guc = ce_to_guc(ce);
+ unsigned long flags;
+
+ spin_lock_irqsave(&guc->timestamp.lock, flags);
+ lrc_update_runtime(ce);
+ spin_unlock_irqrestore(&guc->timestamp.lock, flags);
+}
+
static void guc_timestamp_ping(struct work_struct *wrk)
{
struct intel_guc *guc = container_of(wrk, typeof(*guc),
timestamp.work.work);
struct intel_uc *uc = container_of(guc, typeof(*uc), guc);
struct intel_gt *gt = guc_to_gt(guc);
+ struct intel_context *ce;
intel_wakeref_t wakeref;
+ unsigned long index;
int srcu, ret;
/*
@@ -1424,6 +1436,10 @@ static void guc_timestamp_ping(struct work_struct *wrk)
with_intel_runtime_pm(>->i915->runtime_pm, wakeref)
__update_guc_busyness_stats(guc);
+ /* adjust context stats for overflow */
+ xa_for_each(&guc->context_lookup, index, ce)
+ guc_context_update_clks(ce);
+
intel_gt_reset_unlock(gt, srcu);
guc_enable_busyness_worker(guc);
@@ -1505,6 +1521,17 @@ void intel_guc_busyness_unpark(struct intel_gt *gt)
guc_enable_busyness_worker(guc);
}
+static void guc_context_update_stats(struct intel_context *ce)
+{
+ if (!intel_context_pin_if_active(ce)) {
+ WRITE_ONCE(ce->stats.active, 0);
+ return;
+ }
+
+ guc_context_update_clks(ce);
+ intel_context_unpin(ce);
+}
+
static inline bool
submission_disabled(struct intel_guc *guc)
{
@@ -2774,6 +2801,7 @@ static void guc_context_unpin(struct intel_context *ce)
{
struct intel_guc *guc = ce_to_guc(ce);
+ lrc_update_runtime(ce);
unpin_guc_id(guc, ce);
lrc_unpin(ce);
@@ -3455,6 +3483,7 @@ static void remove_from_context(struct i915_request *rq)
}
static const struct intel_context_ops guc_context_ops = {
+ .flags = COPS_RUNTIME_CYCLES | COPS_RUNTIME_ACTIVE_TOTAL,
.alloc = guc_context_alloc,
.close = guc_context_close,
@@ -3473,6 +3502,8 @@ static const struct intel_context_ops guc_context_ops = {
.sched_disable = guc_context_sched_disable,
+ .update_stats = guc_context_update_stats,
+
.reset = lrc_reset,
.destroy = guc_context_destroy,
@@ -3728,6 +3759,7 @@ static int guc_virtual_context_alloc(struct intel_context *ce)
}
static const struct intel_context_ops virtual_guc_context_ops = {
+ .flags = COPS_RUNTIME_CYCLES | COPS_RUNTIME_ACTIVE_TOTAL,
.alloc = guc_virtual_context_alloc,
.close = guc_context_close,
@@ -3745,6 +3777,7 @@ static const struct intel_context_ops virtual_guc_context_ops = {
.exit = guc_virtual_context_exit,
.sched_disable = guc_context_sched_disable,
+ .update_stats = guc_context_update_stats,
.destroy = guc_context_destroy,
--
2.36.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Intel-gfx] [PATCH 2/2] drm/i915/fdinfo: Enable fdinfo for GuC backends
2023-04-05 0:14 [Intel-gfx] [PATCH 0/2] fdinfo: Enable some support for GuC based client busyness Umesh Nerlige Ramappa
2023-04-05 0:14 ` [Intel-gfx] [PATCH 1/2] i915/pmu: Add support for total context runtime for GuC back-end Umesh Nerlige Ramappa
@ 2023-04-05 0:14 ` Umesh Nerlige Ramappa
2023-04-05 1:46 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for fdinfo: Enable some support for GuC based client busyness Patchwork
` (2 subsequent siblings)
4 siblings, 0 replies; 12+ messages in thread
From: Umesh Nerlige Ramappa @ 2023-04-05 0:14 UTC (permalink / raw)
To: intel-gfx
Enable fdinfo for GuC based platforms with the exception that long
running contexts will not provide reliable busyness data unless they
switch out at some reasonable point in time.
Link: https://gitlab.freedesktop.org/drm/intel/issues/8303
Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
---
drivers/gpu/drm/i915/i915_drm_client.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drm_client.c b/drivers/gpu/drm/i915/i915_drm_client.c
index e8fa172ebe5e..d18d0a3ed905 100644
--- a/drivers/gpu/drm/i915/i915_drm_client.c
+++ b/drivers/gpu/drm/i915/i915_drm_client.c
@@ -147,11 +147,7 @@ void i915_drm_client_fdinfo(struct seq_file *m, struct file *f)
PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn));
seq_printf(m, "drm-client-id:\t%u\n", client->id);
- /*
- * Temporarily skip showing client engine information with GuC submission till
- * fetching engine busyness is implemented in the GuC submission backend
- */
- if (GRAPHICS_VER(i915) < 8 || intel_uc_uses_guc_submission(&i915->gt0.uc))
+ if (GRAPHICS_VER(i915) < 8)
return;
for (i = 0; i < ARRAY_SIZE(uabi_class_names); i++)
--
2.36.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Intel-gfx] ✗ Fi.CI.SPARSE: warning for fdinfo: Enable some support for GuC based client busyness
2023-04-05 0:14 [Intel-gfx] [PATCH 0/2] fdinfo: Enable some support for GuC based client busyness Umesh Nerlige Ramappa
2023-04-05 0:14 ` [Intel-gfx] [PATCH 1/2] i915/pmu: Add support for total context runtime for GuC back-end Umesh Nerlige Ramappa
2023-04-05 0:14 ` [Intel-gfx] [PATCH 2/2] drm/i915/fdinfo: Enable fdinfo for GuC backends Umesh Nerlige Ramappa
@ 2023-04-05 1:46 ` Patchwork
2023-04-05 1:59 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2023-04-05 12:40 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
4 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2023-04-05 1:46 UTC (permalink / raw)
To: Umesh Nerlige Ramappa; +Cc: intel-gfx
== Series Details ==
Series: fdinfo: Enable some support for GuC based client busyness
URL : https://patchwork.freedesktop.org/series/116120/
State : warning
== Summary ==
Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Intel-gfx] ✓ Fi.CI.BAT: success for fdinfo: Enable some support for GuC based client busyness
2023-04-05 0:14 [Intel-gfx] [PATCH 0/2] fdinfo: Enable some support for GuC based client busyness Umesh Nerlige Ramappa
` (2 preceding siblings ...)
2023-04-05 1:46 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for fdinfo: Enable some support for GuC based client busyness Patchwork
@ 2023-04-05 1:59 ` Patchwork
2023-04-05 12:40 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
4 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2023-04-05 1:59 UTC (permalink / raw)
To: Umesh Nerlige Ramappa; +Cc: intel-gfx
[-- Attachment #1: Type: text/plain, Size: 7926 bytes --]
== Series Details ==
Series: fdinfo: Enable some support for GuC based client busyness
URL : https://patchwork.freedesktop.org/series/116120/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_12967 -> Patchwork_116120v1
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/index.html
Participating hosts (37 -> 37)
------------------------------
Additional (1): fi-kbl-soraka
Missing (1): fi-snb-2520m
Known issues
------------
Here are the changes found in Patchwork_116120v1 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_huc_copy@huc-copy:
- fi-kbl-soraka: NOTRUN -> [SKIP][1] ([fdo#109271] / [i915#2190])
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/fi-kbl-soraka/igt@gem_huc_copy@huc-copy.html
* igt@gem_lmem_swapping@basic:
- fi-kbl-soraka: NOTRUN -> [SKIP][2] ([fdo#109271] / [i915#4613]) +3 similar issues
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/fi-kbl-soraka/igt@gem_lmem_swapping@basic.html
* igt@i915_pm_rps@basic-api:
- bat-dg2-11: [PASS][3] -> [FAIL][4] ([i915#8308])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12967/bat-dg2-11/igt@i915_pm_rps@basic-api.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/bat-dg2-11/igt@i915_pm_rps@basic-api.html
* igt@i915_selftest@live@gt_heartbeat:
- fi-apl-guc: [PASS][5] -> [DMESG-FAIL][6] ([i915#5334])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12967/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html
* igt@i915_selftest@live@gt_pm:
- fi-kbl-soraka: NOTRUN -> [DMESG-FAIL][7] ([i915#1886])
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/fi-kbl-soraka/igt@i915_selftest@live@gt_pm.html
* igt@i915_selftest@live@migrate:
- bat-dg2-11: [PASS][8] -> [DMESG-WARN][9] ([i915#7699])
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12967/bat-dg2-11/igt@i915_selftest@live@migrate.html
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/bat-dg2-11/igt@i915_selftest@live@migrate.html
* igt@i915_selftest@live@reset:
- bat-rpls-1: [PASS][10] -> [ABORT][11] ([i915#4983])
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12967/bat-rpls-1/igt@i915_selftest@live@reset.html
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/bat-rpls-1/igt@i915_selftest@live@reset.html
* igt@i915_selftest@live@slpc:
- bat-rpls-2: NOTRUN -> [DMESG-FAIL][12] ([i915#6367] / [i915#7913])
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/bat-rpls-2/igt@i915_selftest@live@slpc.html
- bat-dg2-11: [PASS][13] -> [DMESG-WARN][14] ([i915#7996])
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12967/bat-dg2-11/igt@i915_selftest@live@slpc.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/bat-dg2-11/igt@i915_selftest@live@slpc.html
* igt@kms_chamelium_frames@hdmi-crc-fast:
- fi-kbl-soraka: NOTRUN -> [SKIP][15] ([fdo#109271]) +16 similar issues
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/fi-kbl-soraka/igt@kms_chamelium_frames@hdmi-crc-fast.html
* igt@kms_chamelium_hpd@common-hpd-after-suspend:
- bat-rpls-2: NOTRUN -> [SKIP][16] ([i915#7828])
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/bat-rpls-2/igt@kms_chamelium_hpd@common-hpd-after-suspend.html
- fi-bsw-n3050: NOTRUN -> [SKIP][17] ([fdo#109271])
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/fi-bsw-n3050/igt@kms_chamelium_hpd@common-hpd-after-suspend.html
* igt@kms_pipe_crc_basic@read-crc:
- bat-adlp-9: NOTRUN -> [SKIP][18] ([i915#3546]) +1 similar issue
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/bat-adlp-9/igt@kms_pipe_crc_basic@read-crc.html
* igt@kms_pipe_crc_basic@suspend-read-crc:
- bat-rpls-2: NOTRUN -> [SKIP][19] ([i915#1845])
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/bat-rpls-2/igt@kms_pipe_crc_basic@suspend-read-crc.html
* igt@kms_pipe_crc_basic@suspend-read-crc@pipe-a-hdmi-a-1:
- fi-rkl-11600: [PASS][20] -> [FAIL][21] ([fdo#103375])
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12967/fi-rkl-11600/igt@kms_pipe_crc_basic@suspend-read-crc@pipe-a-hdmi-a-1.html
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/fi-rkl-11600/igt@kms_pipe_crc_basic@suspend-read-crc@pipe-a-hdmi-a-1.html
#### Possible fixes ####
* igt@i915_selftest@live@execlists:
- fi-bsw-n3050: [ABORT][22] ([i915#7911] / [i915#7913]) -> [PASS][23]
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12967/fi-bsw-n3050/igt@i915_selftest@live@execlists.html
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/fi-bsw-n3050/igt@i915_selftest@live@execlists.html
* igt@i915_selftest@live@requests:
- bat-rpls-2: [ABORT][24] ([i915#7913] / [i915#7982]) -> [PASS][25]
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12967/bat-rpls-2/igt@i915_selftest@live@requests.html
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/bat-rpls-2/igt@i915_selftest@live@requests.html
* igt@kms_pipe_crc_basic@nonblocking-crc@pipe-d-dp-1:
- bat-dg2-8: [FAIL][26] ([i915#7932]) -> [PASS][27]
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12967/bat-dg2-8/igt@kms_pipe_crc_basic@nonblocking-crc@pipe-d-dp-1.html
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/bat-dg2-8/igt@kms_pipe_crc_basic@nonblocking-crc@pipe-d-dp-1.html
[fdo#103375]: https://bugs.freedesktop.org/show_bug.cgi?id=103375
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845
[i915#1886]: https://gitlab.freedesktop.org/drm/intel/issues/1886
[i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
[i915#3546]: https://gitlab.freedesktop.org/drm/intel/issues/3546
[i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
[i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
[i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334
[i915#6367]: https://gitlab.freedesktop.org/drm/intel/issues/6367
[i915#7699]: https://gitlab.freedesktop.org/drm/intel/issues/7699
[i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
[i915#7911]: https://gitlab.freedesktop.org/drm/intel/issues/7911
[i915#7913]: https://gitlab.freedesktop.org/drm/intel/issues/7913
[i915#7932]: https://gitlab.freedesktop.org/drm/intel/issues/7932
[i915#7982]: https://gitlab.freedesktop.org/drm/intel/issues/7982
[i915#7996]: https://gitlab.freedesktop.org/drm/intel/issues/7996
[i915#8308]: https://gitlab.freedesktop.org/drm/intel/issues/8308
Build changes
-------------
* IGT: IGT_7236 -> IGTPW_8755
* Linux: CI_DRM_12967 -> Patchwork_116120v1
CI-20190529: 20190529
CI_DRM_12967: 28c342e55ec8081498c63f4a38825f055e294a3a @ git://anongit.freedesktop.org/gfx-ci/linux
IGTPW_8755: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8755/index.html
IGT_7236: bac5a4cc31b3212a205219a6cbc45a173d30d04b @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_116120v1: 28c342e55ec8081498c63f4a38825f055e294a3a @ git://anongit.freedesktop.org/gfx-ci/linux
### Linux commits
cf677c9ded4d drm/i915/fdinfo: Enable fdinfo for GuC backends
7b313971c993 i915/pmu: Add support for total context runtime for GuC back-end
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/index.html
[-- Attachment #2: Type: text/html, Size: 9318 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Intel-gfx] ✓ Fi.CI.IGT: success for fdinfo: Enable some support for GuC based client busyness
2023-04-05 0:14 [Intel-gfx] [PATCH 0/2] fdinfo: Enable some support for GuC based client busyness Umesh Nerlige Ramappa
` (3 preceding siblings ...)
2023-04-05 1:59 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
@ 2023-04-05 12:40 ` Patchwork
4 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2023-04-05 12:40 UTC (permalink / raw)
To: Umesh Nerlige Ramappa; +Cc: intel-gfx
[-- Attachment #1: Type: text/plain, Size: 22567 bytes --]
== Series Details ==
Series: fdinfo: Enable some support for GuC based client busyness
URL : https://patchwork.freedesktop.org/series/116120/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_12967_full -> Patchwork_116120v1_full
====================================================
Summary
-------
**SUCCESS**
No regressions found.
Participating hosts (7 -> 7)
------------------------------
No changes in participating hosts
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_116120v1_full:
### IGT changes ###
#### Suppressed ####
The following results come from untrusted machines, tests, or statuses.
They do not affect the overall result.
* igt@drm_fdinfo@isolation@vecs0:
- {shard-dg1}: NOTRUN -> [SKIP][1] +26 similar issues
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/shard-dg1-16/igt@drm_fdinfo@isolation@vecs0.html
* igt@drm_fdinfo@virtual-busy-idle-all:
- {shard-dg1}: [SKIP][2] ([i915#5563]) -> [SKIP][3] +2 similar issues
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12967/shard-dg1-14/igt@drm_fdinfo@virtual-busy-idle-all.html
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/shard-dg1-14/igt@drm_fdinfo@virtual-busy-idle-all.html
Known issues
------------
Here are the changes found in Patchwork_116120v1_full that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_exec_fair@basic-none-solo@rcs0:
- shard-apl: [PASS][4] -> [FAIL][5] ([i915#2842])
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12967/shard-apl2/igt@gem_exec_fair@basic-none-solo@rcs0.html
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/shard-apl4/igt@gem_exec_fair@basic-none-solo@rcs0.html
* igt@gem_huc_copy@huc-copy:
- shard-apl: NOTRUN -> [SKIP][6] ([fdo#109271] / [i915#2190])
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/shard-apl4/igt@gem_huc_copy@huc-copy.html
- shard-glk: NOTRUN -> [SKIP][7] ([fdo#109271] / [i915#2190])
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/shard-glk7/igt@gem_huc_copy@huc-copy.html
* igt@gen9_exec_parse@allowed-single:
- shard-apl: [PASS][8] -> [ABORT][9] ([i915#5566])
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12967/shard-apl2/igt@gen9_exec_parse@allowed-single.html
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/shard-apl3/igt@gen9_exec_parse@allowed-single.html
* igt@i915_pm_rpm@system-suspend-devices:
- shard-snb: NOTRUN -> [SKIP][10] ([fdo#109271]) +22 similar issues
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/shard-snb5/igt@i915_pm_rpm@system-suspend-devices.html
* igt@i915_selftest@mock@sanitycheck:
- shard-snb: [PASS][11] -> [ABORT][12] ([i915#4528])
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12967/shard-snb4/igt@i915_selftest@mock@sanitycheck.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/shard-snb4/igt@i915_selftest@mock@sanitycheck.html
* igt@kms_ccs@pipe-b-random-ccs-data-4_tiled_dg2_rc_ccs:
- shard-apl: NOTRUN -> [SKIP][13] ([fdo#109271]) +12 similar issues
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/shard-apl2/igt@kms_ccs@pipe-b-random-ccs-data-4_tiled_dg2_rc_ccs.html
* igt@kms_cursor_crc@cursor-random-max-size:
- shard-glk: NOTRUN -> [SKIP][14] ([fdo#109271]) +10 similar issues
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/shard-glk9/igt@kms_cursor_crc@cursor-random-max-size.html
* igt@kms_plane_alpha_blend@alpha-basic@pipe-a-dp-1:
- shard-apl: NOTRUN -> [FAIL][15] ([i915#7862]) +1 similar issue
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/shard-apl4/igt@kms_plane_alpha_blend@alpha-basic@pipe-a-dp-1.html
* igt@kms_plane_alpha_blend@alpha-basic@pipe-c-hdmi-a-1:
- shard-glk: NOTRUN -> [FAIL][16] ([i915#7862]) +1 similar issue
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/shard-glk7/igt@kms_plane_alpha_blend@alpha-basic@pipe-c-hdmi-a-1.html
* igt@kms_psr2_sf@cursor-plane-move-continuous-exceed-fully-sf:
- shard-apl: NOTRUN -> [SKIP][17] ([fdo#109271] / [i915#658])
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/shard-apl7/igt@kms_psr2_sf@cursor-plane-move-continuous-exceed-fully-sf.html
- shard-glk: NOTRUN -> [SKIP][18] ([fdo#109271] / [i915#658])
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/shard-glk6/igt@kms_psr2_sf@cursor-plane-move-continuous-exceed-fully-sf.html
* igt@perf@stress-open-close@0-rcs0:
- shard-glk: [PASS][19] -> [ABORT][20] ([i915#5213])
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12967/shard-glk8/igt@perf@stress-open-close@0-rcs0.html
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/shard-glk4/igt@perf@stress-open-close@0-rcs0.html
#### Possible fixes ####
* igt@drm_fdinfo@most-busy-check-all@rcs0:
- {shard-rkl}: [FAIL][21] ([i915#7742]) -> [PASS][22]
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12967/shard-rkl-1/igt@drm_fdinfo@most-busy-check-all@rcs0.html
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/shard-rkl-2/igt@drm_fdinfo@most-busy-check-all@rcs0.html
* igt@gem_exec_fair@basic-deadline:
- shard-apl: [FAIL][23] ([i915#2846]) -> [PASS][24]
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12967/shard-apl1/igt@gem_exec_fair@basic-deadline.html
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/shard-apl4/igt@gem_exec_fair@basic-deadline.html
- shard-glk: [FAIL][25] ([i915#2846]) -> [PASS][26]
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12967/shard-glk1/igt@gem_exec_fair@basic-deadline.html
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/shard-glk4/igt@gem_exec_fair@basic-deadline.html
* igt@gem_exec_fair@basic-none@bcs0:
- {shard-rkl}: [FAIL][27] ([i915#2842]) -> [PASS][28]
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12967/shard-rkl-3/igt@gem_exec_fair@basic-none@bcs0.html
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/shard-rkl-3/igt@gem_exec_fair@basic-none@bcs0.html
* igt@gem_exec_fair@basic-pace-solo@rcs0:
- shard-apl: [FAIL][29] ([i915#2842]) -> [PASS][30]
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12967/shard-apl7/igt@gem_exec_fair@basic-pace-solo@rcs0.html
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/shard-apl4/igt@gem_exec_fair@basic-pace-solo@rcs0.html
* igt@gem_exec_fair@basic-pace@rcs0:
- shard-glk: [FAIL][31] ([i915#2842]) -> [PASS][32]
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12967/shard-glk2/igt@gem_exec_fair@basic-pace@rcs0.html
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/shard-glk2/igt@gem_exec_fair@basic-pace@rcs0.html
* igt@i915_pm_rc6_residency@rc6-idle@bcs0:
- {shard-dg1}: [FAIL][33] ([i915#3591]) -> [PASS][34]
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12967/shard-dg1-17/igt@i915_pm_rc6_residency@rc6-idle@bcs0.html
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/shard-dg1-18/igt@i915_pm_rc6_residency@rc6-idle@bcs0.html
* igt@i915_pm_rpm@modeset-lpsp-stress-no-wait:
- {shard-rkl}: [SKIP][35] ([i915#1397]) -> [PASS][36] +1 similar issue
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12967/shard-rkl-6/igt@i915_pm_rpm@modeset-lpsp-stress-no-wait.html
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/shard-rkl-7/igt@i915_pm_rpm@modeset-lpsp-stress-no-wait.html
* igt@i915_pm_rps@reset:
- shard-snb: [DMESG-FAIL][37] ([i915#8319]) -> [PASS][38]
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12967/shard-snb4/igt@i915_pm_rps@reset.html
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/shard-snb5/igt@i915_pm_rps@reset.html
* igt@i915_selftest@live@gt_heartbeat:
- shard-apl: [DMESG-FAIL][39] ([i915#5334]) -> [PASS][40]
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12967/shard-apl6/igt@i915_selftest@live@gt_heartbeat.html
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/shard-apl7/igt@i915_selftest@live@gt_heartbeat.html
* igt@i915_suspend@forcewake:
- shard-snb: [DMESG-WARN][41] ([i915#5090]) -> [PASS][42]
[41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12967/shard-snb5/igt@i915_suspend@forcewake.html
[42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/shard-snb4/igt@i915_suspend@forcewake.html
* igt@kms_big_fb@x-tiled-max-hw-stride-32bpp-rotate-180-hflip-async-flip:
- {shard-rkl}: [FAIL][43] ([i915#3743]) -> [PASS][44]
[43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12967/shard-rkl-7/igt@kms_big_fb@x-tiled-max-hw-stride-32bpp-rotate-180-hflip-async-flip.html
[44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/shard-rkl-7/igt@kms_big_fb@x-tiled-max-hw-stride-32bpp-rotate-180-hflip-async-flip.html
* igt@kms_big_fb@y-tiled-max-hw-stride-32bpp-rotate-180-async-flip:
- {shard-tglu}: [FAIL][45] ([i915#3743]) -> [PASS][46]
[45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12967/shard-tglu-6/igt@kms_big_fb@y-tiled-max-hw-stride-32bpp-rotate-180-async-flip.html
[46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/shard-tglu-6/igt@kms_big_fb@y-tiled-max-hw-stride-32bpp-rotate-180-async-flip.html
* igt@kms_cursor_crc@cursor-suspend@pipe-d-hdmi-a-1:
- {shard-tglu}: [INCOMPLETE][47] -> [PASS][48]
[47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12967/shard-tglu-6/igt@kms_cursor_crc@cursor-suspend@pipe-d-hdmi-a-1.html
[48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/shard-tglu-7/igt@kms_cursor_crc@cursor-suspend@pipe-d-hdmi-a-1.html
* igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size:
- shard-glk: [FAIL][49] ([i915#2346]) -> [PASS][50]
[49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12967/shard-glk4/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html
[50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/shard-glk1/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html
* igt@kms_cursor_legacy@single-move@pipe-b:
- {shard-rkl}: [INCOMPLETE][51] ([i915#8011]) -> [PASS][52] +1 similar issue
[51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12967/shard-rkl-7/igt@kms_cursor_legacy@single-move@pipe-b.html
[52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/shard-rkl-4/igt@kms_cursor_legacy@single-move@pipe-b.html
* igt@kms_fbcon_fbt@fbc-suspend:
- shard-apl: [FAIL][53] ([i915#4767]) -> [PASS][54]
[53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12967/shard-apl6/igt@kms_fbcon_fbt@fbc-suspend.html
[54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/shard-apl3/igt@kms_fbcon_fbt@fbc-suspend.html
* igt@kms_flip@2x-plain-flip-fb-recreate-interruptible@bc-hdmi-a1-hdmi-a2:
- shard-glk: [FAIL][55] ([i915#2122]) -> [PASS][56]
[55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12967/shard-glk2/igt@kms_flip@2x-plain-flip-fb-recreate-interruptible@bc-hdmi-a1-hdmi-a2.html
[56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/shard-glk5/igt@kms_flip@2x-plain-flip-fb-recreate-interruptible@bc-hdmi-a1-hdmi-a2.html
* igt@kms_flip@flip-vs-suspend@c-dp1:
- shard-apl: [ABORT][57] ([i915#180]) -> [PASS][58]
[57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12967/shard-apl2/igt@kms_flip@flip-vs-suspend@c-dp1.html
[58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/shard-apl7/igt@kms_flip@flip-vs-suspend@c-dp1.html
* igt@perf_pmu@idle@rcs0:
- {shard-rkl}: [FAIL][59] ([i915#4349]) -> [PASS][60]
[59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12967/shard-rkl-6/igt@perf_pmu@idle@rcs0.html
[60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/shard-rkl-1/igt@perf_pmu@idle@rcs0.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
[fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
[fdo#109295]: https://bugs.freedesktop.org/show_bug.cgi?id=109295
[fdo#109300]: https://bugs.freedesktop.org/show_bug.cgi?id=109300
[fdo#109302]: https://bugs.freedesktop.org/show_bug.cgi?id=109302
[fdo#109309]: https://bugs.freedesktop.org/show_bug.cgi?id=109309
[fdo#109314]: https://bugs.freedesktop.org/show_bug.cgi?id=109314
[fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
[fdo#109506]: https://bugs.freedesktop.org/show_bug.cgi?id=109506
[fdo#110189]: https://bugs.freedesktop.org/show_bug.cgi?id=110189
[fdo#110723]: https://bugs.freedesktop.org/show_bug.cgi?id=110723
[fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
[fdo#111614]: https://bugs.freedesktop.org/show_bug.cgi?id=111614
[fdo#111615]: https://bugs.freedesktop.org/show_bug.cgi?id=111615
[fdo#111825]: https://bugs.freedesktop.org/show_bug.cgi?id=111825
[fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
[fdo#112054]: https://bugs.freedesktop.org/show_bug.cgi?id=112054
[fdo#112283]: https://bugs.freedesktop.org/show_bug.cgi?id=112283
[i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
[i915#1397]: https://gitlab.freedesktop.org/drm/intel/issues/1397
[i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
[i915#1825]: https://gitlab.freedesktop.org/drm/intel/issues/1825
[i915#1839]: https://gitlab.freedesktop.org/drm/intel/issues/1839
[i915#1902]: https://gitlab.freedesktop.org/drm/intel/issues/1902
[i915#2122]: https://gitlab.freedesktop.org/drm/intel/issues/2122
[i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
[i915#2346]: https://gitlab.freedesktop.org/drm/intel/issues/2346
[i915#2433]: https://gitlab.freedesktop.org/drm/intel/issues/2433
[i915#2435]: https://gitlab.freedesktop.org/drm/intel/issues/2435
[i915#2437]: https://gitlab.freedesktop.org/drm/intel/issues/2437
[i915#2527]: https://gitlab.freedesktop.org/drm/intel/issues/2527
[i915#2575]: https://gitlab.freedesktop.org/drm/intel/issues/2575
[i915#2587]: https://gitlab.freedesktop.org/drm/intel/issues/2587
[i915#2672]: https://gitlab.freedesktop.org/drm/intel/issues/2672
[i915#2705]: https://gitlab.freedesktop.org/drm/intel/issues/2705
[i915#280]: https://gitlab.freedesktop.org/drm/intel/issues/280
[i915#2842]: https://gitlab.freedesktop.org/drm/intel/issues/2842
[i915#2846]: https://gitlab.freedesktop.org/drm/intel/issues/2846
[i915#3023]: https://gitlab.freedesktop.org/drm/intel/issues/3023
[i915#3063]: https://gitlab.freedesktop.org/drm/intel/issues/3063
[i915#315]: https://gitlab.freedesktop.org/drm/intel/issues/315
[i915#3281]: https://gitlab.freedesktop.org/drm/intel/issues/3281
[i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
[i915#3291]: https://gitlab.freedesktop.org/drm/intel/issues/3291
[i915#3297]: https://gitlab.freedesktop.org/drm/intel/issues/3297
[i915#3299]: https://gitlab.freedesktop.org/drm/intel/issues/3299
[i915#3301]: https://gitlab.freedesktop.org/drm/intel/issues/3301
[i915#3359]: https://gitlab.freedesktop.org/drm/intel/issues/3359
[i915#3458]: https://gitlab.freedesktop.org/drm/intel/issues/3458
[i915#3469]: https://gitlab.freedesktop.org/drm/intel/issues/3469
[i915#3539]: https://gitlab.freedesktop.org/drm/intel/issues/3539
[i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
[i915#3591]: https://gitlab.freedesktop.org/drm/intel/issues/3591
[i915#3638]: https://gitlab.freedesktop.org/drm/intel/issues/3638
[i915#3689]: https://gitlab.freedesktop.org/drm/intel/issues/3689
[i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
[i915#3734]: https://gitlab.freedesktop.org/drm/intel/issues/3734
[i915#3742]: https://gitlab.freedesktop.org/drm/intel/issues/3742
[i915#3743]: https://gitlab.freedesktop.org/drm/intel/issues/3743
[i915#3826]: https://gitlab.freedesktop.org/drm/intel/issues/3826
[i915#3840]: https://gitlab.freedesktop.org/drm/intel/issues/3840
[i915#3886]: https://gitlab.freedesktop.org/drm/intel/issues/3886
[i915#3938]: https://gitlab.freedesktop.org/drm/intel/issues/3938
[i915#3955]: https://gitlab.freedesktop.org/drm/intel/issues/3955
[i915#4070]: https://gitlab.freedesktop.org/drm/intel/issues/4070
[i915#4077]: https://gitlab.freedesktop.org/drm/intel/issues/4077
[i915#4079]: https://gitlab.freedesktop.org/drm/intel/issues/4079
[i915#4083]: https://gitlab.freedesktop.org/drm/intel/issues/4083
[i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
[i915#4212]: https://gitlab.freedesktop.org/drm/intel/issues/4212
[i915#4213]: https://gitlab.freedesktop.org/drm/intel/issues/4213
[i915#4270]: https://gitlab.freedesktop.org/drm/intel/issues/4270
[i915#4349]: https://gitlab.freedesktop.org/drm/intel/issues/4349
[i915#4391]: https://gitlab.freedesktop.org/drm/intel/issues/4391
[i915#4525]: https://gitlab.freedesktop.org/drm/intel/issues/4525
[i915#4528]: https://gitlab.freedesktop.org/drm/intel/issues/4528
[i915#4538]: https://gitlab.freedesktop.org/drm/intel/issues/4538
[i915#4565]: https://gitlab.freedesktop.org/drm/intel/issues/4565
[i915#4579]: https://gitlab.freedesktop.org/drm/intel/issues/4579
[i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
[i915#4767]: https://gitlab.freedesktop.org/drm/intel/issues/4767
[i915#4771]: https://gitlab.freedesktop.org/drm/intel/issues/4771
[i915#4812]: https://gitlab.freedesktop.org/drm/intel/issues/4812
[i915#4816]: https://gitlab.freedesktop.org/drm/intel/issues/4816
[i915#4833]: https://gitlab.freedesktop.org/drm/intel/issues/4833
[i915#4852]: https://gitlab.freedesktop.org/drm/intel/issues/4852
[i915#4854]: https://gitlab.freedesktop.org/drm/intel/issues/4854
[i915#4859]: https://gitlab.freedesktop.org/drm/intel/issues/4859
[i915#4860]: https://gitlab.freedesktop.org/drm/intel/issues/4860
[i915#4880]: https://gitlab.freedesktop.org/drm/intel/issues/4880
[i915#4881]: https://gitlab.freedesktop.org/drm/intel/issues/4881
[i915#4958]: https://gitlab.freedesktop.org/drm/intel/issues/4958
[i915#5090]: https://gitlab.freedesktop.org/drm/intel/issues/5090
[i915#5122]: https://gitlab.freedesktop.org/drm/intel/issues/5122
[i915#5176]: https://gitlab.freedesktop.org/drm/intel/issues/5176
[i915#5213]: https://gitlab.freedesktop.org/drm/intel/issues/5213
[i915#5235]: https://gitlab.freedesktop.org/drm/intel/issues/5235
[i915#5286]: https://gitlab.freedesktop.org/drm/intel/issues/5286
[i915#5288]: https://gitlab.freedesktop.org/drm/intel/issues/5288
[i915#5289]: https://gitlab.freedesktop.org/drm/intel/issues/5289
[i915#5325]: https://gitlab.freedesktop.org/drm/intel/issues/5325
[i915#533]: https://gitlab.freedesktop.org/drm/intel/issues/533
[i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334
[i915#5354]: https://gitlab.freedesktop.org/drm/intel/issues/5354
[i915#5431]: https://gitlab.freedesktop.org/drm/intel/issues/5431
[i915#5439]: https://gitlab.freedesktop.org/drm/intel/issues/5439
[i915#5461]: https://gitlab.freedesktop.org/drm/intel/issues/5461
[i915#5563]: https://gitlab.freedesktop.org/drm/intel/issues/5563
[i915#5566]: https://gitlab.freedesktop.org/drm/intel/issues/5566
[i915#5723]: https://gitlab.freedesktop.org/drm/intel/issues/5723
[i915#6095]: https://gitlab.freedesktop.org/drm/intel/issues/6095
[i915#6245]: https://gitlab.freedesktop.org/drm/intel/issues/6245
[i915#6433]: https://gitlab.freedesktop.org/drm/intel/issues/6433
[i915#6524]: https://gitlab.freedesktop.org/drm/intel/issues/6524
[i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658
[i915#6590]: https://gitlab.freedesktop.org/drm/intel/issues/6590
[i915#6768]: https://gitlab.freedesktop.org/drm/intel/issues/6768
[i915#6944]: https://gitlab.freedesktop.org/drm/intel/issues/6944
[i915#6946]: https://gitlab.freedesktop.org/drm/intel/issues/6946
[i915#6953]: https://gitlab.freedesktop.org/drm/intel/issues/6953
[i915#7116]: https://gitlab.freedesktop.org/drm/intel/issues/7116
[i915#7118]: https://gitlab.freedesktop.org/drm/intel/issues/7118
[i915#7561]: https://gitlab.freedesktop.org/drm/intel/issues/7561
[i915#7697]: https://gitlab.freedesktop.org/drm/intel/issues/7697
[i915#7711]: https://gitlab.freedesktop.org/drm/intel/issues/7711
[i915#7742]: https://gitlab.freedesktop.org/drm/intel/issues/7742
[i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
[i915#7862]: https://gitlab.freedesktop.org/drm/intel/issues/7862
[i915#7975]: https://gitlab.freedesktop.org/drm/intel/issues/7975
[i915#8011]: https://gitlab.freedesktop.org/drm/intel/issues/8011
[i915#8155]: https://gitlab.freedesktop.org/drm/intel/issues/8155
[i915#8253]: https://gitlab.freedesktop.org/drm/intel/issues/8253
[i915#8292]: https://gitlab.freedesktop.org/drm/intel/issues/8292
[i915#8308]: https://gitlab.freedesktop.org/drm/intel/issues/8308
[i915#8311]: https://gitlab.freedesktop.org/drm/intel/issues/8311
[i915#8319]: https://gitlab.freedesktop.org/drm/intel/issues/8319
Build changes
-------------
* IGT: IGT_7236 -> IGTPW_8755
* Linux: CI_DRM_12967 -> Patchwork_116120v1
CI-20190529: 20190529
CI_DRM_12967: 28c342e55ec8081498c63f4a38825f055e294a3a @ git://anongit.freedesktop.org/gfx-ci/linux
IGTPW_8755: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8755/index.html
IGT_7236: bac5a4cc31b3212a205219a6cbc45a173d30d04b @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_116120v1: 28c342e55ec8081498c63f4a38825f055e294a3a @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116120v1/index.html
[-- Attachment #2: Type: text/html, Size: 17248 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Intel-gfx] [PATCH 1/2] i915/pmu: Add support for total context runtime for GuC back-end
2023-04-05 0:14 ` [Intel-gfx] [PATCH 1/2] i915/pmu: Add support for total context runtime for GuC back-end Umesh Nerlige Ramappa
@ 2023-04-24 17:41 ` Dixit, Ashutosh
2023-04-27 0:11 ` Umesh Nerlige Ramappa
0 siblings, 1 reply; 12+ messages in thread
From: Dixit, Ashutosh @ 2023-04-24 17:41 UTC (permalink / raw)
To: Umesh Nerlige Ramappa; +Cc: intel-gfx
On Tue, 04 Apr 2023 17:14:32 -0700, Umesh Nerlige Ramappa wrote:
Hi Umesh,
> GPU accumulates the context runtime in a 32 bit counter - CTX_TIMESTAMP
> in the context image. This value is saved/restored on context switches.
> KMD accumulates these values into a 64 bit counter taking care of any
> overflows as needed. This count provides the basis for client specific
> busyness in the fdinfo interface.
>
> KMD accumulation happens just before the context is unpinned and when
> context switches out. This works for execlist back-end since execlist
> scheduling has visibility into context switches. With GuC mode, KMD does
> not have visibility into context switches and this counter is
> accumulated only when context is unpinned. Context is unpinned once the
> context scheduling is successfully disabled. Disabling context
> scheduling is an asynchronous operation.
This means guc_context_unpin() is called asynchronously, correct? From
guc_context_sched_disable()?
> Also if a context is servicing frequent requests, scheduling may never be
> disabled on it.
>
> For GuC mode, since updates to the context runtime may be delayed, add
> hooks to update the context runtime in a worker thread as well as when
> a user queries for it.
>
> Limitation:
> - If a context is never switched out or runs for a long period of time,
> the runtime value of CTX_TIMESTAMP may never be updated, so the
> counter value may be unreliable. This patch does not support such
> cases. Such support must be available from the GuC FW and it is WIP.
>
> This patch is an extract from previous work authored by John/Umesh here -
> https://patchwork.freedesktop.org/patch/496441/?series=105085&rev=4
>
> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
> Co-developed-by: John Harrison <John.C.Harrison@Intel.com>
> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
> ---
> drivers/gpu/drm/i915/gt/intel_context.c | 12 +++++--
> drivers/gpu/drm/i915/gt/intel_context.h | 2 +-
> drivers/gpu/drm/i915/gt/intel_context_types.h | 5 +++
> .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 33 +++++++++++++++++++
> 4 files changed, 49 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c
> index 2aa63ec521b8..e01f222e9e42 100644
> --- a/drivers/gpu/drm/i915/gt/intel_context.c
> +++ b/drivers/gpu/drm/i915/gt/intel_context.c
> @@ -578,16 +578,24 @@ void intel_context_bind_parent_child(struct intel_context *parent,
> child->parallel.parent = parent;
> }
>
> -u64 intel_context_get_total_runtime_ns(const struct intel_context *ce)
> +u64 intel_context_get_total_runtime_ns(struct intel_context *ce)
> {
> u64 total, active;
>
> + if (ce->ops->update_stats)
> + ce->ops->update_stats(ce);
> +
> total = ce->stats.runtime.total;
> if (ce->ops->flags & COPS_RUNTIME_CYCLES)
> total *= ce->engine->gt->clock_period_ns;
>
> active = READ_ONCE(ce->stats.active);
> - if (active)
> + /*
> + * When COPS_RUNTIME_ACTIVE_TOTAL is set for ce->cops, the backend
> + * already provides the total active time of the context,
Where is this done in the GuC case? I looked but couldn't find it (at least
in this version of the patch, it is there in the old version).
> + * so skip this calculation when this flag is set.
> + */
> + if (active && !(ce->ops->flags & COPS_RUNTIME_ACTIVE_TOTAL))
> active = intel_context_clock() - active;
>
> return total + active;
> diff --git a/drivers/gpu/drm/i915/gt/intel_context.h b/drivers/gpu/drm/i915/gt/intel_context.h
> index 0a8d553da3f4..720809523e2d 100644
> --- a/drivers/gpu/drm/i915/gt/intel_context.h
> +++ b/drivers/gpu/drm/i915/gt/intel_context.h
> @@ -368,7 +368,7 @@ intel_context_clear_nopreempt(struct intel_context *ce)
> clear_bit(CONTEXT_NOPREEMPT, &ce->flags);
> }
>
> -u64 intel_context_get_total_runtime_ns(const struct intel_context *ce);
> +u64 intel_context_get_total_runtime_ns(struct intel_context *ce);
> u64 intel_context_get_avg_runtime_ns(struct intel_context *ce);
>
> static inline u64 intel_context_clock(void)
> diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h b/drivers/gpu/drm/i915/gt/intel_context_types.h
> index e36670f2e626..58b0294d359d 100644
> --- a/drivers/gpu/drm/i915/gt/intel_context_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_context_types.h
> @@ -38,6 +38,9 @@ struct intel_context_ops {
> #define COPS_RUNTIME_CYCLES_BIT 1
> #define COPS_RUNTIME_CYCLES BIT(COPS_RUNTIME_CYCLES_BIT)
>
> +#define COPS_RUNTIME_ACTIVE_TOTAL_BIT 2
> +#define COPS_RUNTIME_ACTIVE_TOTAL BIT(COPS_RUNTIME_ACTIVE_TOTAL_BIT)
> +
> int (*alloc)(struct intel_context *ce);
>
> void (*revoke)(struct intel_context *ce, struct i915_request *rq,
> @@ -58,6 +61,8 @@ struct intel_context_ops {
>
> void (*sched_disable)(struct intel_context *ce);
>
> + void (*update_stats)(struct intel_context *ce);
> +
> void (*reset)(struct intel_context *ce);
> void (*destroy)(struct kref *kref);
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> index 88e881b100cf..8048a3e97a68 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> @@ -1402,13 +1402,25 @@ static void __update_guc_busyness_stats(struct intel_guc *guc)
> spin_unlock_irqrestore(&guc->timestamp.lock, flags);
> }
>
> +static void guc_context_update_clks(struct intel_context *ce)
> +{
> + struct intel_guc *guc = ce_to_guc(ce);
> + unsigned long flags;
> +
> + spin_lock_irqsave(&guc->timestamp.lock, flags);
> + lrc_update_runtime(ce);
> + spin_unlock_irqrestore(&guc->timestamp.lock, flags);
> +}
> +
> static void guc_timestamp_ping(struct work_struct *wrk)
> {
> struct intel_guc *guc = container_of(wrk, typeof(*guc),
> timestamp.work.work);
> struct intel_uc *uc = container_of(guc, typeof(*uc), guc);
> struct intel_gt *gt = guc_to_gt(guc);
> + struct intel_context *ce;
> intel_wakeref_t wakeref;
> + unsigned long index;
> int srcu, ret;
>
> /*
> @@ -1424,6 +1436,10 @@ static void guc_timestamp_ping(struct work_struct *wrk)
> with_intel_runtime_pm(>->i915->runtime_pm, wakeref)
> __update_guc_busyness_stats(guc);
How do we know the context images are pinned at this point which is needed
for the code below? Where is the pinning happening?
> + /* adjust context stats for overflow */
> + xa_for_each(&guc->context_lookup, index, ce)
> + guc_context_update_clks(ce);
Here are we saying that we need to do this because the context can get
switched out (and context image saved) and back in multiple times without
the context getting unpinned? Otherwise guc_context_unpin() would call
lrc_update_runtime() and we wouldn't have to do this here.
> +
> intel_gt_reset_unlock(gt, srcu);
>
> guc_enable_busyness_worker(guc);
> @@ -1505,6 +1521,17 @@ void intel_guc_busyness_unpark(struct intel_gt *gt)
> guc_enable_busyness_worker(guc);
> }
>
> +static void guc_context_update_stats(struct intel_context *ce)
> +{
> + if (!intel_context_pin_if_active(ce)) {
> + WRITE_ONCE(ce->stats.active, 0);
This is related to the question above about where is ce->stats.active being
updated in GuC case. If it is not being updated then we wouldn't have to do
this here, we could just initialize it to 0 once or it might already be
initialized to 0 (if say ce->stats was kzalloc'd).
/> + return;
> + }
> +
> + guc_context_update_clks(ce);
> + intel_context_unpin(ce);
> +}
> +
> static inline bool
> submission_disabled(struct intel_guc *guc)
> {
> @@ -2774,6 +2801,7 @@ static void guc_context_unpin(struct intel_context *ce)
> {
> struct intel_guc *guc = ce_to_guc(ce);
>
> + lrc_update_runtime(ce);
If we call lrc_update_runtime from these 3 code paths (as is done in this
patch), we would need to hold guc->timestamp.lock here. Is that happening
(I don't see it) or we need to call guc_context_update_clks() here? I am
assuming the context image is pinned here so pinning is not an issue.
> unpin_guc_id(guc, ce);
> lrc_unpin(ce);
>
> @@ -3455,6 +3483,7 @@ static void remove_from_context(struct i915_request *rq)
> }
>
> static const struct intel_context_ops guc_context_ops = {
> + .flags = COPS_RUNTIME_CYCLES | COPS_RUNTIME_ACTIVE_TOTAL,
> .alloc = guc_context_alloc,
>
> .close = guc_context_close,
> @@ -3473,6 +3502,8 @@ static const struct intel_context_ops guc_context_ops = {
>
> .sched_disable = guc_context_sched_disable,
>
> + .update_stats = guc_context_update_stats,
> +
> .reset = lrc_reset,
> .destroy = guc_context_destroy,
>
> @@ -3728,6 +3759,7 @@ static int guc_virtual_context_alloc(struct intel_context *ce)
> }
>
> static const struct intel_context_ops virtual_guc_context_ops = {
> + .flags = COPS_RUNTIME_CYCLES | COPS_RUNTIME_ACTIVE_TOTAL,
> .alloc = guc_virtual_context_alloc,
>
> .close = guc_context_close,
> @@ -3745,6 +3777,7 @@ static const struct intel_context_ops virtual_guc_context_ops = {
> .exit = guc_virtual_context_exit,
>
> .sched_disable = guc_context_sched_disable,
> + .update_stats = guc_context_update_stats,
>
> .destroy = guc_context_destroy,
>
> --
> 2.36.1
>
Thanks.
--
Ashutosh
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Intel-gfx] [PATCH 1/2] i915/pmu: Add support for total context runtime for GuC back-end
2023-04-24 17:41 ` Dixit, Ashutosh
@ 2023-04-27 0:11 ` Umesh Nerlige Ramappa
2023-04-27 0:51 ` Dixit, Ashutosh
0 siblings, 1 reply; 12+ messages in thread
From: Umesh Nerlige Ramappa @ 2023-04-27 0:11 UTC (permalink / raw)
To: Dixit, Ashutosh; +Cc: intel-gfx
On Mon, Apr 24, 2023 at 10:41:41AM -0700, Dixit, Ashutosh wrote:
>On Tue, 04 Apr 2023 17:14:32 -0700, Umesh Nerlige Ramappa wrote:
>
>Hi Umesh,
>
>> GPU accumulates the context runtime in a 32 bit counter - CTX_TIMESTAMP
>> in the context image. This value is saved/restored on context switches.
>> KMD accumulates these values into a 64 bit counter taking care of any
>> overflows as needed. This count provides the basis for client specific
>> busyness in the fdinfo interface.
>>
>> KMD accumulation happens just before the context is unpinned and when
>> context switches out. This works for execlist back-end since execlist
>> scheduling has visibility into context switches. With GuC mode, KMD does
>> not have visibility into context switches and this counter is
>> accumulated only when context is unpinned. Context is unpinned once the
>> context scheduling is successfully disabled. Disabling context
>> scheduling is an asynchronous operation.
>
>This means guc_context_unpin() is called asynchronously, correct? From
>guc_context_sched_disable()?
correct
>
>> Also if a context is servicing frequent requests, scheduling may never be
>> disabled on it.
>>
>> For GuC mode, since updates to the context runtime may be delayed, add
>> hooks to update the context runtime in a worker thread as well as when
>> a user queries for it.
>>
>> Limitation:
>> - If a context is never switched out or runs for a long period of time,
>> the runtime value of CTX_TIMESTAMP may never be updated, so the
>> counter value may be unreliable. This patch does not support such
>> cases. Such support must be available from the GuC FW and it is WIP.
>>
>> This patch is an extract from previous work authored by John/Umesh here -
>> https://patchwork.freedesktop.org/patch/496441/?series=105085&rev=4
>>
>> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
>> Co-developed-by: John Harrison <John.C.Harrison@Intel.com>
>> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
>> ---
>> drivers/gpu/drm/i915/gt/intel_context.c | 12 +++++--
>> drivers/gpu/drm/i915/gt/intel_context.h | 2 +-
>> drivers/gpu/drm/i915/gt/intel_context_types.h | 5 +++
>> .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 33 +++++++++++++++++++
>> 4 files changed, 49 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c
>> index 2aa63ec521b8..e01f222e9e42 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_context.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_context.c
>> @@ -578,16 +578,24 @@ void intel_context_bind_parent_child(struct intel_context *parent,
>> child->parallel.parent = parent;
>> }
>>
>> -u64 intel_context_get_total_runtime_ns(const struct intel_context *ce)
>> +u64 intel_context_get_total_runtime_ns(struct intel_context *ce)
>> {
>> u64 total, active;
>>
>> + if (ce->ops->update_stats)
>> + ce->ops->update_stats(ce);
>> +
>> total = ce->stats.runtime.total;
>> if (ce->ops->flags & COPS_RUNTIME_CYCLES)
>> total *= ce->engine->gt->clock_period_ns;
>>
>> active = READ_ONCE(ce->stats.active);
>> - if (active)
>> + /*
>> + * When COPS_RUNTIME_ACTIVE_TOTAL is set for ce->cops, the backend
>> + * already provides the total active time of the context,
>
>Where is this done in the GuC case? I looked but couldn't find it (at least
>in this version of the patch, it is there in the old version).
In this case, the backend is not providing the total active time, I
guess I should drop this logic provided ce->stats.active is 0 for GuC
mode.
>
>> + * so skip this calculation when this flag is set.
>> + */
>> + if (active && !(ce->ops->flags & COPS_RUNTIME_ACTIVE_TOTAL))
>> active = intel_context_clock() - active;
>>
>> return total + active;
>> diff --git a/drivers/gpu/drm/i915/gt/intel_context.h b/drivers/gpu/drm/i915/gt/intel_context.h
>> index 0a8d553da3f4..720809523e2d 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_context.h
>> +++ b/drivers/gpu/drm/i915/gt/intel_context.h
>> @@ -368,7 +368,7 @@ intel_context_clear_nopreempt(struct intel_context *ce)
>> clear_bit(CONTEXT_NOPREEMPT, &ce->flags);
>> }
>>
>> -u64 intel_context_get_total_runtime_ns(const struct intel_context *ce);
>> +u64 intel_context_get_total_runtime_ns(struct intel_context *ce);
>> u64 intel_context_get_avg_runtime_ns(struct intel_context *ce);
>>
>> static inline u64 intel_context_clock(void)
>> diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h b/drivers/gpu/drm/i915/gt/intel_context_types.h
>> index e36670f2e626..58b0294d359d 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_context_types.h
>> +++ b/drivers/gpu/drm/i915/gt/intel_context_types.h
>> @@ -38,6 +38,9 @@ struct intel_context_ops {
>> #define COPS_RUNTIME_CYCLES_BIT 1
>> #define COPS_RUNTIME_CYCLES BIT(COPS_RUNTIME_CYCLES_BIT)
>>
>> +#define COPS_RUNTIME_ACTIVE_TOTAL_BIT 2
>> +#define COPS_RUNTIME_ACTIVE_TOTAL BIT(COPS_RUNTIME_ACTIVE_TOTAL_BIT)
>> +
>> int (*alloc)(struct intel_context *ce);
>>
>> void (*revoke)(struct intel_context *ce, struct i915_request *rq,
>> @@ -58,6 +61,8 @@ struct intel_context_ops {
>>
>> void (*sched_disable)(struct intel_context *ce);
>>
>> + void (*update_stats)(struct intel_context *ce);
>> +
>> void (*reset)(struct intel_context *ce);
>> void (*destroy)(struct kref *kref);
>>
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
>> index 88e881b100cf..8048a3e97a68 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
>> @@ -1402,13 +1402,25 @@ static void __update_guc_busyness_stats(struct intel_guc *guc)
>> spin_unlock_irqrestore(&guc->timestamp.lock, flags);
>> }
>>
>> +static void guc_context_update_clks(struct intel_context *ce)
>> +{
>> + struct intel_guc *guc = ce_to_guc(ce);
>> + unsigned long flags;
>> +
>> + spin_lock_irqsave(&guc->timestamp.lock, flags);
>> + lrc_update_runtime(ce);
>> + spin_unlock_irqrestore(&guc->timestamp.lock, flags);
>> +}
>> +
>> static void guc_timestamp_ping(struct work_struct *wrk)
>> {
>> struct intel_guc *guc = container_of(wrk, typeof(*guc),
>> timestamp.work.work);
>> struct intel_uc *uc = container_of(guc, typeof(*uc), guc);
>> struct intel_gt *gt = guc_to_gt(guc);
>> + struct intel_context *ce;
>> intel_wakeref_t wakeref;
>> + unsigned long index;
>> int srcu, ret;
>>
>> /*
>> @@ -1424,6 +1436,10 @@ static void guc_timestamp_ping(struct work_struct *wrk)
>> with_intel_runtime_pm(>->i915->runtime_pm, wakeref)
>> __update_guc_busyness_stats(guc);
>
>How do we know the context images are pinned at this point which is needed
>for the code below? Where is the pinning happening?
Ah, maybe this should just call - guc_context_update_stats()
>
>> + /* adjust context stats for overflow */
>> + xa_for_each(&guc->context_lookup, index, ce)
>> + guc_context_update_clks(ce);
>
>Here are we saying that we need to do this because the context can get
>switched out (and context image saved) and back in multiple times without
>the context getting unpinned? Otherwise guc_context_unpin() would call
>lrc_update_runtime() and we wouldn't have to do this here.
Mainly for 32 bit overflows. The busyness value obtained from the
context image is a 32 bit value and could wrap around. If we keep
grabbing it periodically and accumulate it in the 64 bit value in ce
stats, we should be good.
>
>> +
>> intel_gt_reset_unlock(gt, srcu);
>>
>> guc_enable_busyness_worker(guc);
>> @@ -1505,6 +1521,17 @@ void intel_guc_busyness_unpark(struct intel_gt *gt)
>> guc_enable_busyness_worker(guc);
>> }
>>
>> +static void guc_context_update_stats(struct intel_context *ce)
>> +{
>> + if (!intel_context_pin_if_active(ce)) {
>> + WRITE_ONCE(ce->stats.active, 0);
>
>This is related to the question above about where is ce->stats.active being
>updated in GuC case. If it is not being updated then we wouldn't have to do
>this here, we could just initialize it to 0 once or it might already be
>initialized to 0 (if say ce->stats was kzalloc'd).
Agree, will drop this.
>
>/> + return;
>> + }
>> +
>> + guc_context_update_clks(ce);
>> + intel_context_unpin(ce);
>> +}
>> +
>> static inline bool
>> submission_disabled(struct intel_guc *guc)
>> {
>> @@ -2774,6 +2801,7 @@ static void guc_context_unpin(struct intel_context *ce)
>> {
>> struct intel_guc *guc = ce_to_guc(ce);
>>
>> + lrc_update_runtime(ce);
>
>If we call lrc_update_runtime from these 3 code paths (as is done in this
>patch), we would need to hold guc->timestamp.lock here. Is that happening
>(I don't see it) or we need to call guc_context_update_clks() here? I am
>assuming the context image is pinned here so pinning is not an issue.
Maybe will just call guc_context_update_clks() here.
thanks,
Umesh
>
>> unpin_guc_id(guc, ce);
>> lrc_unpin(ce);
>>
>> @@ -3455,6 +3483,7 @@ static void remove_from_context(struct i915_request *rq)
>> }
>>
>> static const struct intel_context_ops guc_context_ops = {
>> + .flags = COPS_RUNTIME_CYCLES | COPS_RUNTIME_ACTIVE_TOTAL,
>> .alloc = guc_context_alloc,
>>
>> .close = guc_context_close,
>> @@ -3473,6 +3502,8 @@ static const struct intel_context_ops guc_context_ops = {
>>
>> .sched_disable = guc_context_sched_disable,
>>
>> + .update_stats = guc_context_update_stats,
>> +
>> .reset = lrc_reset,
>> .destroy = guc_context_destroy,
>>
>> @@ -3728,6 +3759,7 @@ static int guc_virtual_context_alloc(struct intel_context *ce)
>> }
>>
>> static const struct intel_context_ops virtual_guc_context_ops = {
>> + .flags = COPS_RUNTIME_CYCLES | COPS_RUNTIME_ACTIVE_TOTAL,
>> .alloc = guc_virtual_context_alloc,
>>
>> .close = guc_context_close,
>> @@ -3745,6 +3777,7 @@ static const struct intel_context_ops virtual_guc_context_ops = {
>> .exit = guc_virtual_context_exit,
>>
>> .sched_disable = guc_context_sched_disable,
>> + .update_stats = guc_context_update_stats,
>>
>> .destroy = guc_context_destroy,
>>
>> --
>> 2.36.1
>>
>
>Thanks.
>--
>Ashutosh
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Intel-gfx] [PATCH 1/2] i915/pmu: Add support for total context runtime for GuC back-end
2023-04-27 0:11 ` Umesh Nerlige Ramappa
@ 2023-04-27 0:51 ` Dixit, Ashutosh
2023-04-27 18:57 ` Umesh Nerlige Ramappa
0 siblings, 1 reply; 12+ messages in thread
From: Dixit, Ashutosh @ 2023-04-27 0:51 UTC (permalink / raw)
To: Umesh Nerlige Ramappa; +Cc: intel-gfx
On Wed, 26 Apr 2023 17:11:27 -0700, Umesh Nerlige Ramappa wrote:
>
Hi Umesh,
> On Mon, Apr 24, 2023 at 10:41:41AM -0700, Dixit, Ashutosh wrote:
> > On Tue, 04 Apr 2023 17:14:32 -0700, Umesh Nerlige Ramappa wrote:
> >
> >> GPU accumulates the context runtime in a 32 bit counter - CTX_TIMESTAMP
> >> in the context image. This value is saved/restored on context switches.
> >> KMD accumulates these values into a 64 bit counter taking care of any
> >> overflows as needed. This count provides the basis for client specific
> >> busyness in the fdinfo interface.
> >>
> >> KMD accumulation happens just before the context is unpinned and when
> >> context switches out. This works for execlist back-end since execlist
> >> scheduling has visibility into context switches. With GuC mode, KMD does
> >> not have visibility into context switches and this counter is
> >> accumulated only when context is unpinned. Context is unpinned once the
> >> context scheduling is successfully disabled. Disabling context
> >> scheduling is an asynchronous operation.
> >
> > This means guc_context_unpin() is called asynchronously, correct? From
> > guc_context_sched_disable()?
>
> correct
> >
> >> Also if a context is servicing frequent requests, scheduling may never be
> >> disabled on it.
> >>
> >> For GuC mode, since updates to the context runtime may be delayed, add
> >> hooks to update the context runtime in a worker thread as well as when
> >> a user queries for it.
> >>
> >> Limitation:
> >> - If a context is never switched out or runs for a long period of time,
> >> the runtime value of CTX_TIMESTAMP may never be updated, so the
> >> counter value may be unreliable. This patch does not support such
> >> cases. Such support must be available from the GuC FW and it is WIP.
> >>
> >> This patch is an extract from previous work authored by John/Umesh here -
> >> https://patchwork.freedesktop.org/patch/496441/?series=105085&rev=4
> >>
> >> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
> >> Co-developed-by: John Harrison <John.C.Harrison@Intel.com>
> >> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
> >> ---
> >> drivers/gpu/drm/i915/gt/intel_context.c | 12 +++++--
> >> drivers/gpu/drm/i915/gt/intel_context.h | 2 +-
> >> drivers/gpu/drm/i915/gt/intel_context_types.h | 5 +++
> >> .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 33 +++++++++++++++++++
> >> 4 files changed, 49 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c
> >> index 2aa63ec521b8..e01f222e9e42 100644
> >> --- a/drivers/gpu/drm/i915/gt/intel_context.c
> >> +++ b/drivers/gpu/drm/i915/gt/intel_context.c
> >> @@ -578,16 +578,24 @@ void intel_context_bind_parent_child(struct intel_context *parent,
> >> child->parallel.parent = parent;
> >> }
> >>
> >> -u64 intel_context_get_total_runtime_ns(const struct intel_context *ce)
> >> +u64 intel_context_get_total_runtime_ns(struct intel_context *ce)
> >> {
> >> u64 total, active;
> >>
> >> + if (ce->ops->update_stats)
> >> + ce->ops->update_stats(ce);
> >> +
> >> total = ce->stats.runtime.total;
> >> if (ce->ops->flags & COPS_RUNTIME_CYCLES)
> >> total *= ce->engine->gt->clock_period_ns;
> >>
> >> active = READ_ONCE(ce->stats.active);
> >> - if (active)
> >> + /*
> >> + * When COPS_RUNTIME_ACTIVE_TOTAL is set for ce->cops, the backend
> >> + * already provides the total active time of the context,
> >
> > Where is this done in the GuC case? I looked but couldn't find it (at least
> > in this version of the patch, it is there in the old version).
>
> In this case, the backend is not providing the total active time, I guess I
> should drop this logic provided ce->stats.active is 0 for GuC mode.
Yes, I think best to skip the active part in this patch since this is a
temporary/inaccurate solution.
>
> >
> >> + * so skip this calculation when this flag is set.
> >> + */
> >> + if (active && !(ce->ops->flags & COPS_RUNTIME_ACTIVE_TOTAL))
> >> active = intel_context_clock() - active;
> >>
> >> return total + active;
> >> diff --git a/drivers/gpu/drm/i915/gt/intel_context.h b/drivers/gpu/drm/i915/gt/intel_context.h
> >> index 0a8d553da3f4..720809523e2d 100644
> >> --- a/drivers/gpu/drm/i915/gt/intel_context.h
> >> +++ b/drivers/gpu/drm/i915/gt/intel_context.h
> >> @@ -368,7 +368,7 @@ intel_context_clear_nopreempt(struct intel_context *ce)
> >> clear_bit(CONTEXT_NOPREEMPT, &ce->flags);
> >> }
> >>
> >> -u64 intel_context_get_total_runtime_ns(const struct intel_context *ce);
> >> +u64 intel_context_get_total_runtime_ns(struct intel_context *ce);
> >> u64 intel_context_get_avg_runtime_ns(struct intel_context *ce);
> >>
> >> static inline u64 intel_context_clock(void)
> >> diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h b/drivers/gpu/drm/i915/gt/intel_context_types.h
> >> index e36670f2e626..58b0294d359d 100644
> >> --- a/drivers/gpu/drm/i915/gt/intel_context_types.h
> >> +++ b/drivers/gpu/drm/i915/gt/intel_context_types.h
> >> @@ -38,6 +38,9 @@ struct intel_context_ops {
> >> #define COPS_RUNTIME_CYCLES_BIT 1
> >> #define COPS_RUNTIME_CYCLES BIT(COPS_RUNTIME_CYCLES_BIT)
> >>
> >> +#define COPS_RUNTIME_ACTIVE_TOTAL_BIT 2
> >> +#define COPS_RUNTIME_ACTIVE_TOTAL BIT(COPS_RUNTIME_ACTIVE_TOTAL_BIT)
> >> +
> >> int (*alloc)(struct intel_context *ce);
> >>
> >> void (*revoke)(struct intel_context *ce, struct i915_request *rq,
> >> @@ -58,6 +61,8 @@ struct intel_context_ops {
> >>
> >> void (*sched_disable)(struct intel_context *ce);
> >>
> >> + void (*update_stats)(struct intel_context *ce);
> >> +
> >> void (*reset)(struct intel_context *ce);
> >> void (*destroy)(struct kref *kref);
> >>
> >> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> >> index 88e881b100cf..8048a3e97a68 100644
> >> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> >> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> >> @@ -1402,13 +1402,25 @@ static void __update_guc_busyness_stats(struct intel_guc *guc)
> >> spin_unlock_irqrestore(&guc->timestamp.lock, flags);
> >> }
> >>
> >> +static void guc_context_update_clks(struct intel_context *ce)
> >> +{
> >> + struct intel_guc *guc = ce_to_guc(ce);
> >> + unsigned long flags;
> >> +
> >> + spin_lock_irqsave(&guc->timestamp.lock, flags);
> >> + lrc_update_runtime(ce);
> >> + spin_unlock_irqrestore(&guc->timestamp.lock, flags);
> >> +}
> >> +
> >> static void guc_timestamp_ping(struct work_struct *wrk)
> >> {
> >> struct intel_guc *guc = container_of(wrk, typeof(*guc),
> >> timestamp.work.work);
> >> struct intel_uc *uc = container_of(guc, typeof(*uc), guc);
> >> struct intel_gt *gt = guc_to_gt(guc);
> >> + struct intel_context *ce;
> >> intel_wakeref_t wakeref;
> >> + unsigned long index;
> >> int srcu, ret;
> >>
> >> /*
> >> @@ -1424,6 +1436,10 @@ static void guc_timestamp_ping(struct work_struct *wrk)
> >> with_intel_runtime_pm(>->i915->runtime_pm, wakeref)
> >> __update_guc_busyness_stats(guc);
> >
> > How do we know the context images are pinned at this point which is needed
> > for the code below? Where is the pinning happening?
>
> Ah, maybe this should just call - guc_context_update_stats()
> >
> >> + /* adjust context stats for overflow */
> >> + xa_for_each(&guc->context_lookup, index, ce)
> >> + guc_context_update_clks(ce);
> >
> > Here are we saying that we need to do this because the context can get
> > switched out (and context image saved) and back in multiple times without
> > the context getting unpinned? Otherwise guc_context_unpin() would call
> > lrc_update_runtime() and we wouldn't have to do this here.
>
> Mainly for 32 bit overflows. The busyness value obtained from the context
> image is a 32 bit value and could wrap around. If we keep grabbing it
> periodically and accumulate it in the 64 bit value in ce stats, we should
> be good.
But it will not help unless the busyness value in context image is being
updated. Which will only happen if the context switches. So that is why I
was saying this makes sense only if context switches are happening but not
context unpins (which would anyway accumulate the values in
lrc_update_runtime).
>
> >
> >> +
> >> intel_gt_reset_unlock(gt, srcu);
> >>
> >> guc_enable_busyness_worker(guc);
> >> @@ -1505,6 +1521,17 @@ void intel_guc_busyness_unpark(struct intel_gt *gt)
> >> guc_enable_busyness_worker(guc);
> >> }
> >>
> >> +static void guc_context_update_stats(struct intel_context *ce)
> >> +{
> >> + if (!intel_context_pin_if_active(ce)) {
> >> + WRITE_ONCE(ce->stats.active, 0);
> >
> > This is related to the question above about where is ce->stats.active being
> > updated in GuC case. If it is not being updated then we wouldn't have to do
> > this here, we could just initialize it to 0 once or it might already be
> > initialized to 0 (if say ce->stats was kzalloc'd).
>
> Agree, will drop this.
>
> >
> > /> + return;
> >> + }
> >> +
> >> + guc_context_update_clks(ce);
> >> + intel_context_unpin(ce);
> >> +}
> >> +
> >> static inline bool
> >> submission_disabled(struct intel_guc *guc)
> >> {
> >> @@ -2774,6 +2801,7 @@ static void guc_context_unpin(struct intel_context *ce)
> >> {
> >> struct intel_guc *guc = ce_to_guc(ce);
> >>
> >> + lrc_update_runtime(ce);
> >
> > If we call lrc_update_runtime from these 3 code paths (as is done in this
> > patch), we would need to hold guc->timestamp.lock here. Is that happening
> > (I don't see it) or we need to call guc_context_update_clks() here? I am
> > assuming the context image is pinned here so pinning is not an issue.
>
> Maybe will just call guc_context_update_clks() here.
If active is not there, it would be best if we call the same function from
all 3 places (unless the lock is already taken somewhere). Basically
something like:
static void guc_context_update_stats(struct intel_context *ce)
{
struct intel_guc *guc = ce_to_guc(ce);
unsigned long flags;
intel_context_pin(ce);
spin_lock_irqsave(&guc->timestamp.lock, flags);
lrc_update_runtime(ce);
spin_unlock_irqrestore(&guc->timestamp.lock, flags);
intel_context_unpin(ce);
}
And call this from everywhere.
Thanks.
--
Ashutosh
> >
> >> unpin_guc_id(guc, ce);
> >> lrc_unpin(ce);
> >>
> >> @@ -3455,6 +3483,7 @@ static void remove_from_context(struct i915_request *rq)
> >> }
> >>
> >> static const struct intel_context_ops guc_context_ops = {
> >> + .flags = COPS_RUNTIME_CYCLES | COPS_RUNTIME_ACTIVE_TOTAL,
> >> .alloc = guc_context_alloc,
> >>
> >> .close = guc_context_close,
> >> @@ -3473,6 +3502,8 @@ static const struct intel_context_ops guc_context_ops = {
> >>
> >> .sched_disable = guc_context_sched_disable,
> >>
> >> + .update_stats = guc_context_update_stats,
> >> +
> >> .reset = lrc_reset,
> >> .destroy = guc_context_destroy,
> >>
> >> @@ -3728,6 +3759,7 @@ static int guc_virtual_context_alloc(struct intel_context *ce)
> >> }
> >>
> >> static const struct intel_context_ops virtual_guc_context_ops = {
> >> + .flags = COPS_RUNTIME_CYCLES | COPS_RUNTIME_ACTIVE_TOTAL,
> >> .alloc = guc_virtual_context_alloc,
> >>
> >> .close = guc_context_close,
> >> @@ -3745,6 +3777,7 @@ static const struct intel_context_ops virtual_guc_context_ops = {
> >> .exit = guc_virtual_context_exit,
> >>
> >> .sched_disable = guc_context_sched_disable,
> >> + .update_stats = guc_context_update_stats,
> >>
> >> .destroy = guc_context_destroy,
> >>
> >> --
> >> 2.36.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Intel-gfx] [PATCH 1/2] i915/pmu: Add support for total context runtime for GuC back-end
2023-04-27 0:51 ` Dixit, Ashutosh
@ 2023-04-27 18:57 ` Umesh Nerlige Ramappa
0 siblings, 0 replies; 12+ messages in thread
From: Umesh Nerlige Ramappa @ 2023-04-27 18:57 UTC (permalink / raw)
To: Dixit, Ashutosh; +Cc: intel-gfx
On Wed, Apr 26, 2023 at 05:51:24PM -0700, Dixit, Ashutosh wrote:
>On Wed, 26 Apr 2023 17:11:27 -0700, Umesh Nerlige Ramappa wrote:
>>
>
>Hi Umesh,
>
>> On Mon, Apr 24, 2023 at 10:41:41AM -0700, Dixit, Ashutosh wrote:
>> > On Tue, 04 Apr 2023 17:14:32 -0700, Umesh Nerlige Ramappa wrote:
>> >
>> >> GPU accumulates the context runtime in a 32 bit counter - CTX_TIMESTAMP
>> >> in the context image. This value is saved/restored on context switches.
>> >> KMD accumulates these values into a 64 bit counter taking care of any
>> >> overflows as needed. This count provides the basis for client specific
>> >> busyness in the fdinfo interface.
>> >>
>> >> KMD accumulation happens just before the context is unpinned and when
>> >> context switches out. This works for execlist back-end since execlist
>> >> scheduling has visibility into context switches. With GuC mode, KMD does
>> >> not have visibility into context switches and this counter is
>> >> accumulated only when context is unpinned. Context is unpinned once the
>> >> context scheduling is successfully disabled. Disabling context
>> >> scheduling is an asynchronous operation.
>> >
>> > This means guc_context_unpin() is called asynchronously, correct? From
>> > guc_context_sched_disable()?
>>
>> correct
>> >
>> >> Also if a context is servicing frequent requests, scheduling may never be
>> >> disabled on it.
>> >>
>> >> For GuC mode, since updates to the context runtime may be delayed, add
>> >> hooks to update the context runtime in a worker thread as well as when
>> >> a user queries for it.
>> >>
>> >> Limitation:
>> >> - If a context is never switched out or runs for a long period of time,
>> >> the runtime value of CTX_TIMESTAMP may never be updated, so the
>> >> counter value may be unreliable. This patch does not support such
>> >> cases. Such support must be available from the GuC FW and it is WIP.
>> >>
>> >> This patch is an extract from previous work authored by John/Umesh here -
>> >> https://patchwork.freedesktop.org/patch/496441/?series=105085&rev=4
>> >>
>> >> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
>> >> Co-developed-by: John Harrison <John.C.Harrison@Intel.com>
>> >> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
>> >> ---
>> >> drivers/gpu/drm/i915/gt/intel_context.c | 12 +++++--
>> >> drivers/gpu/drm/i915/gt/intel_context.h | 2 +-
>> >> drivers/gpu/drm/i915/gt/intel_context_types.h | 5 +++
>> >> .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 33 +++++++++++++++++++
>> >> 4 files changed, 49 insertions(+), 3 deletions(-)
>> >>
>> >> diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c
>> >> index 2aa63ec521b8..e01f222e9e42 100644
>> >> --- a/drivers/gpu/drm/i915/gt/intel_context.c
>> >> +++ b/drivers/gpu/drm/i915/gt/intel_context.c
>> >> @@ -578,16 +578,24 @@ void intel_context_bind_parent_child(struct intel_context *parent,
>> >> child->parallel.parent = parent;
>> >> }
>> >>
>> >> -u64 intel_context_get_total_runtime_ns(const struct intel_context *ce)
>> >> +u64 intel_context_get_total_runtime_ns(struct intel_context *ce)
>> >> {
>> >> u64 total, active;
>> >>
>> >> + if (ce->ops->update_stats)
>> >> + ce->ops->update_stats(ce);
>> >> +
>> >> total = ce->stats.runtime.total;
>> >> if (ce->ops->flags & COPS_RUNTIME_CYCLES)
>> >> total *= ce->engine->gt->clock_period_ns;
>> >>
>> >> active = READ_ONCE(ce->stats.active);
>> >> - if (active)
>> >> + /*
>> >> + * When COPS_RUNTIME_ACTIVE_TOTAL is set for ce->cops, the backend
>> >> + * already provides the total active time of the context,
>> >
>> > Where is this done in the GuC case? I looked but couldn't find it (at least
>> > in this version of the patch, it is there in the old version).
>>
>> In this case, the backend is not providing the total active time, I guess I
>> should drop this logic provided ce->stats.active is 0 for GuC mode.
>
>Yes, I think best to skip the active part in this patch since this is a
>temporary/inaccurate solution.
>
>>
>> >
>> >> + * so skip this calculation when this flag is set.
>> >> + */
>> >> + if (active && !(ce->ops->flags & COPS_RUNTIME_ACTIVE_TOTAL))
>> >> active = intel_context_clock() - active;
>> >>
>> >> return total + active;
>> >> diff --git a/drivers/gpu/drm/i915/gt/intel_context.h b/drivers/gpu/drm/i915/gt/intel_context.h
>> >> index 0a8d553da3f4..720809523e2d 100644
>> >> --- a/drivers/gpu/drm/i915/gt/intel_context.h
>> >> +++ b/drivers/gpu/drm/i915/gt/intel_context.h
>> >> @@ -368,7 +368,7 @@ intel_context_clear_nopreempt(struct intel_context *ce)
>> >> clear_bit(CONTEXT_NOPREEMPT, &ce->flags);
>> >> }
>> >>
>> >> -u64 intel_context_get_total_runtime_ns(const struct intel_context *ce);
>> >> +u64 intel_context_get_total_runtime_ns(struct intel_context *ce);
>> >> u64 intel_context_get_avg_runtime_ns(struct intel_context *ce);
>> >>
>> >> static inline u64 intel_context_clock(void)
>> >> diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h b/drivers/gpu/drm/i915/gt/intel_context_types.h
>> >> index e36670f2e626..58b0294d359d 100644
>> >> --- a/drivers/gpu/drm/i915/gt/intel_context_types.h
>> >> +++ b/drivers/gpu/drm/i915/gt/intel_context_types.h
>> >> @@ -38,6 +38,9 @@ struct intel_context_ops {
>> >> #define COPS_RUNTIME_CYCLES_BIT 1
>> >> #define COPS_RUNTIME_CYCLES BIT(COPS_RUNTIME_CYCLES_BIT)
>> >>
>> >> +#define COPS_RUNTIME_ACTIVE_TOTAL_BIT 2
>> >> +#define COPS_RUNTIME_ACTIVE_TOTAL BIT(COPS_RUNTIME_ACTIVE_TOTAL_BIT)
>> >> +
>> >> int (*alloc)(struct intel_context *ce);
>> >>
>> >> void (*revoke)(struct intel_context *ce, struct i915_request *rq,
>> >> @@ -58,6 +61,8 @@ struct intel_context_ops {
>> >>
>> >> void (*sched_disable)(struct intel_context *ce);
>> >>
>> >> + void (*update_stats)(struct intel_context *ce);
>> >> +
>> >> void (*reset)(struct intel_context *ce);
>> >> void (*destroy)(struct kref *kref);
>> >>
>> >> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
>> >> index 88e881b100cf..8048a3e97a68 100644
>> >> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
>> >> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
>> >> @@ -1402,13 +1402,25 @@ static void __update_guc_busyness_stats(struct intel_guc *guc)
>> >> spin_unlock_irqrestore(&guc->timestamp.lock, flags);
>> >> }
>> >>
>> >> +static void guc_context_update_clks(struct intel_context *ce)
>> >> +{
>> >> + struct intel_guc *guc = ce_to_guc(ce);
>> >> + unsigned long flags;
>> >> +
>> >> + spin_lock_irqsave(&guc->timestamp.lock, flags);
>> >> + lrc_update_runtime(ce);
>> >> + spin_unlock_irqrestore(&guc->timestamp.lock, flags);
>> >> +}
>> >> +
>> >> static void guc_timestamp_ping(struct work_struct *wrk)
>> >> {
>> >> struct intel_guc *guc = container_of(wrk, typeof(*guc),
>> >> timestamp.work.work);
>> >> struct intel_uc *uc = container_of(guc, typeof(*uc), guc);
>> >> struct intel_gt *gt = guc_to_gt(guc);
>> >> + struct intel_context *ce;
>> >> intel_wakeref_t wakeref;
>> >> + unsigned long index;
>> >> int srcu, ret;
>> >>
>> >> /*
>> >> @@ -1424,6 +1436,10 @@ static void guc_timestamp_ping(struct work_struct *wrk)
>> >> with_intel_runtime_pm(>->i915->runtime_pm, wakeref)
>> >> __update_guc_busyness_stats(guc);
>> >
>> > How do we know the context images are pinned at this point which is needed
>> > for the code below? Where is the pinning happening?
>>
>> Ah, maybe this should just call - guc_context_update_stats()
>> >
>> >> + /* adjust context stats for overflow */
>> >> + xa_for_each(&guc->context_lookup, index, ce)
>> >> + guc_context_update_clks(ce);
>> >
>> > Here are we saying that we need to do this because the context can get
>> > switched out (and context image saved) and back in multiple times without
>> > the context getting unpinned? Otherwise guc_context_unpin() would call
>> > lrc_update_runtime() and we wouldn't have to do this here.
>>
>> Mainly for 32 bit overflows. The busyness value obtained from the context
>> image is a 32 bit value and could wrap around. If we keep grabbing it
>> periodically and accumulate it in the 64 bit value in ce stats, we should
>> be good.
>
>But it will not help unless the busyness value in context image is being
>updated. Which will only happen if the context switches. So that is why I
>was saying this makes sense only if context switches are happening but not
>context unpins (which would anyway accumulate the values in
>lrc_update_runtime).
I think that's exactly the case here - the busyness value in the context
image is being updated by the GPU when context switches out, but the
context may be scheduled to run again by GuC without any notification to
the KMD. Like you mention - "context switches are happening but not
context unpins", so I think there is scope for overflow.
>
>
>>
>> >
>> >> +
>> >> intel_gt_reset_unlock(gt, srcu);
>> >>
>> >> guc_enable_busyness_worker(guc);
>> >> @@ -1505,6 +1521,17 @@ void intel_guc_busyness_unpark(struct intel_gt *gt)
>> >> guc_enable_busyness_worker(guc);
>> >> }
>> >>
>> >> +static void guc_context_update_stats(struct intel_context *ce)
>> >> +{
>> >> + if (!intel_context_pin_if_active(ce)) {
>> >> + WRITE_ONCE(ce->stats.active, 0);
>> >
>> > This is related to the question above about where is ce->stats.active being
>> > updated in GuC case. If it is not being updated then we wouldn't have to do
>> > this here, we could just initialize it to 0 once or it might already be
>> > initialized to 0 (if say ce->stats was kzalloc'd).
>>
>> Agree, will drop this.
>>
>> >
>> > /> + return;
>> >> + }
>> >> +
>> >> + guc_context_update_clks(ce);
>> >> + intel_context_unpin(ce);
>> >> +}
>> >> +
>> >> static inline bool
>> >> submission_disabled(struct intel_guc *guc)
>> >> {
>> >> @@ -2774,6 +2801,7 @@ static void guc_context_unpin(struct intel_context *ce)
>> >> {
>> >> struct intel_guc *guc = ce_to_guc(ce);
>> >>
>> >> + lrc_update_runtime(ce);
>> >
>> > If we call lrc_update_runtime from these 3 code paths (as is done in this
>> > patch), we would need to hold guc->timestamp.lock here. Is that happening
>> > (I don't see it) or we need to call guc_context_update_clks() here? I am
>> > assuming the context image is pinned here so pinning is not an issue.
>>
>> Maybe will just call guc_context_update_clks() here.
>
>If active is not there, it would be best if we call the same function from
>all 3 places (unless the lock is already taken somewhere). Basically
>something like:
>
>static void guc_context_update_stats(struct intel_context *ce)
>{
> struct intel_guc *guc = ce_to_guc(ce);
> unsigned long flags;
>
> intel_context_pin(ce);
> spin_lock_irqsave(&guc->timestamp.lock, flags);
> lrc_update_runtime(ce);
> spin_unlock_irqrestore(&guc->timestamp.lock, flags);
> intel_context_unpin(ce);
>}
I think I would just do something like this:
s/guc_context_update_clks/__guc_context_update_stats/
From guc_context_unpin(), I would call __guc_context_update_stats() and
that would take care of locking before updating runtime stats. Also note
that we don't want to call intel_context_pin/intel_context_pin_if_active
from within guc_context_pin as I suspect that might recurse endlessly.
(Ref: __intel_context_do_unpin)
From guc_timestamp_ping(), I would just call guc_context_update_stats().
This would take care of pinning and locking from the other 2 call
locations. Note that this will be intel_context_pin_if_active() which is
sufficient in these 2 cases. If the context was not pinned at these 2
call locations, it just means that the stats were already updated in
guc_context_unpin.
Thanks,
Umesh
>
>And call this from everywhere.
>
>Thanks.
>--
>Ashutosh
>
>
>> >
>> >> unpin_guc_id(guc, ce);
>> >> lrc_unpin(ce);
>> >>
>> >> @@ -3455,6 +3483,7 @@ static void remove_from_context(struct i915_request *rq)
>> >> }
>> >>
>> >> static const struct intel_context_ops guc_context_ops = {
>> >> + .flags = COPS_RUNTIME_CYCLES | COPS_RUNTIME_ACTIVE_TOTAL,
>> >> .alloc = guc_context_alloc,
>> >>
>> >> .close = guc_context_close,
>> >> @@ -3473,6 +3502,8 @@ static const struct intel_context_ops guc_context_ops = {
>> >>
>> >> .sched_disable = guc_context_sched_disable,
>> >>
>> >> + .update_stats = guc_context_update_stats,
>> >> +
>> >> .reset = lrc_reset,
>> >> .destroy = guc_context_destroy,
>> >>
>> >> @@ -3728,6 +3759,7 @@ static int guc_virtual_context_alloc(struct intel_context *ce)
>> >> }
>> >>
>> >> static const struct intel_context_ops virtual_guc_context_ops = {
>> >> + .flags = COPS_RUNTIME_CYCLES | COPS_RUNTIME_ACTIVE_TOTAL,
>> >> .alloc = guc_virtual_context_alloc,
>> >>
>> >> .close = guc_context_close,
>> >> @@ -3745,6 +3777,7 @@ static const struct intel_context_ops virtual_guc_context_ops = {
>> >> .exit = guc_virtual_context_exit,
>> >>
>> >> .sched_disable = guc_context_sched_disable,
>> >> + .update_stats = guc_context_update_stats,
>> >>
>> >> .destroy = guc_context_destroy,
>> >>
>> >> --
>> >> 2.36.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Intel-gfx] [PATCH 1/2] i915/pmu: Add support for total context runtime for GuC back-end
2023-04-27 22:47 [Intel-gfx] [PATCH 0/2] " Umesh Nerlige Ramappa
@ 2023-04-27 22:47 ` Umesh Nerlige Ramappa
2023-04-28 21:55 ` Dixit, Ashutosh
0 siblings, 1 reply; 12+ messages in thread
From: Umesh Nerlige Ramappa @ 2023-04-27 22:47 UTC (permalink / raw)
To: intel-gfx
GPU accumulates the context runtime in a 32 bit counter - CTX_TIMESTAMP
in the context image. This value is saved/restored on context switches.
KMD accumulates these values into a 64 bit counter taking care of any
overflows as needed. This count provides the basis for client specific
busyness in the fdinfo interface.
KMD accumulation happens just before the context is unpinned and when
context switches out. This works for execlist back-end since execlist
scheduling has visibility into context switches. With GuC mode, KMD does
not have visibility into context switches and this counter is
accumulated only when context is unpinned. Context is unpinned once the
context scheduling is successfully disabled. Disabling context
scheduling is an asynchronous operation. Also if a context is servicing
frequent requests, scheduling may never be disabled on it.
For GuC mode, since updates to the context runtime may be delayed, add
hooks to update the context runtime in a worker thread as well as when
a user queries for it.
Limitation:
- If a context is never switched out or runs for a long period of time,
the runtime value of CTX_TIMESTAMP may never be updated, so the
counter value may be unreliable. This patch does not support such
cases. Such support must be available from the GuC FW and it is WIP.
This patch is an extract from previous work authored by John/Umesh here -
https://patchwork.freedesktop.org/patch/496441/?series=105085&rev=4
v2: (Ashutosh)
- Drop COPS_RUNTIME_ACTIVE_TOTAL
- s/guc_context_update_clks/__guc_context_update_stats
- Pin context before accessing in guc_timestamp_ping
- In guc_context_unpin, use spinlock to serialize access to runtime stats
Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
Co-developed-by: John Harrison <John.C.Harrison@Intel.com>
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
---
drivers/gpu/drm/i915/gt/intel_context.c | 5 ++-
drivers/gpu/drm/i915/gt/intel_context.h | 2 +-
drivers/gpu/drm/i915/gt/intel_context_types.h | 2 ++
.../gpu/drm/i915/gt/uc/intel_guc_submission.c | 31 +++++++++++++++++++
4 files changed, 38 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c
index 2aa63ec521b8..a53b26178f0a 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.c
+++ b/drivers/gpu/drm/i915/gt/intel_context.c
@@ -578,10 +578,13 @@ void intel_context_bind_parent_child(struct intel_context *parent,
child->parallel.parent = parent;
}
-u64 intel_context_get_total_runtime_ns(const struct intel_context *ce)
+u64 intel_context_get_total_runtime_ns(struct intel_context *ce)
{
u64 total, active;
+ if (ce->ops->update_stats)
+ ce->ops->update_stats(ce);
+
total = ce->stats.runtime.total;
if (ce->ops->flags & COPS_RUNTIME_CYCLES)
total *= ce->engine->gt->clock_period_ns;
diff --git a/drivers/gpu/drm/i915/gt/intel_context.h b/drivers/gpu/drm/i915/gt/intel_context.h
index 48f888c3da08..43ed92f8dc83 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.h
+++ b/drivers/gpu/drm/i915/gt/intel_context.h
@@ -375,7 +375,7 @@ intel_context_clear_nopreempt(struct intel_context *ce)
clear_bit(CONTEXT_NOPREEMPT, &ce->flags);
}
-u64 intel_context_get_total_runtime_ns(const struct intel_context *ce);
+u64 intel_context_get_total_runtime_ns(struct intel_context *ce);
u64 intel_context_get_avg_runtime_ns(struct intel_context *ce);
static inline u64 intel_context_clock(void)
diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h b/drivers/gpu/drm/i915/gt/intel_context_types.h
index e36670f2e626..aceaac28a33e 100644
--- a/drivers/gpu/drm/i915/gt/intel_context_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_context_types.h
@@ -58,6 +58,8 @@ struct intel_context_ops {
void (*sched_disable)(struct intel_context *ce);
+ void (*update_stats)(struct intel_context *ce);
+
void (*reset)(struct intel_context *ce);
void (*destroy)(struct kref *kref);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index ee3e8352637f..c869ddc73e69 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -1402,13 +1402,34 @@ static void __update_guc_busyness_stats(struct intel_guc *guc)
spin_unlock_irqrestore(&guc->timestamp.lock, flags);
}
+static void __guc_context_update_stats(struct intel_context *ce)
+{
+ struct intel_guc *guc = ce_to_guc(ce);
+ unsigned long flags;
+
+ spin_lock_irqsave(&guc->timestamp.lock, flags);
+ lrc_update_runtime(ce);
+ spin_unlock_irqrestore(&guc->timestamp.lock, flags);
+}
+
+static void guc_context_update_stats(struct intel_context *ce)
+{
+ if (!intel_context_pin_if_active(ce))
+ return;
+
+ __guc_context_update_stats(ce);
+ intel_context_unpin(ce);
+}
+
static void guc_timestamp_ping(struct work_struct *wrk)
{
struct intel_guc *guc = container_of(wrk, typeof(*guc),
timestamp.work.work);
struct intel_uc *uc = container_of(guc, typeof(*uc), guc);
struct intel_gt *gt = guc_to_gt(guc);
+ struct intel_context *ce;
intel_wakeref_t wakeref;
+ unsigned long index;
int srcu, ret;
/*
@@ -1424,6 +1445,10 @@ static void guc_timestamp_ping(struct work_struct *wrk)
with_intel_runtime_pm(>->i915->runtime_pm, wakeref)
__update_guc_busyness_stats(guc);
+ /* adjust context stats for overflow */
+ xa_for_each(&guc->context_lookup, index, ce)
+ guc_context_update_stats(ce);
+
intel_gt_reset_unlock(gt, srcu);
guc_enable_busyness_worker(guc);
@@ -2774,6 +2799,7 @@ static void guc_context_unpin(struct intel_context *ce)
{
struct intel_guc *guc = ce_to_guc(ce);
+ __guc_context_update_stats(ce);
unpin_guc_id(guc, ce);
lrc_unpin(ce);
@@ -3455,6 +3481,7 @@ static void remove_from_context(struct i915_request *rq)
}
static const struct intel_context_ops guc_context_ops = {
+ .flags = COPS_RUNTIME_CYCLES,
.alloc = guc_context_alloc,
.close = guc_context_close,
@@ -3473,6 +3500,8 @@ static const struct intel_context_ops guc_context_ops = {
.sched_disable = guc_context_sched_disable,
+ .update_stats = guc_context_update_stats,
+
.reset = lrc_reset,
.destroy = guc_context_destroy,
@@ -3728,6 +3757,7 @@ static int guc_virtual_context_alloc(struct intel_context *ce)
}
static const struct intel_context_ops virtual_guc_context_ops = {
+ .flags = COPS_RUNTIME_CYCLES,
.alloc = guc_virtual_context_alloc,
.close = guc_context_close,
@@ -3745,6 +3775,7 @@ static const struct intel_context_ops virtual_guc_context_ops = {
.exit = guc_virtual_context_exit,
.sched_disable = guc_context_sched_disable,
+ .update_stats = guc_context_update_stats,
.destroy = guc_context_destroy,
--
2.36.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [Intel-gfx] [PATCH 1/2] i915/pmu: Add support for total context runtime for GuC back-end
2023-04-27 22:47 ` [Intel-gfx] [PATCH 1/2] i915/pmu: Add support for total context runtime for GuC back-end Umesh Nerlige Ramappa
@ 2023-04-28 21:55 ` Dixit, Ashutosh
0 siblings, 0 replies; 12+ messages in thread
From: Dixit, Ashutosh @ 2023-04-28 21:55 UTC (permalink / raw)
To: Umesh Nerlige Ramappa; +Cc: intel-gfx
On Thu, 27 Apr 2023 15:47:04 -0700, Umesh Nerlige Ramappa wrote:
>
> GPU accumulates the context runtime in a 32 bit counter - CTX_TIMESTAMP
> in the context image. This value is saved/restored on context switches.
> KMD accumulates these values into a 64 bit counter taking care of any
> overflows as needed. This count provides the basis for client specific
> busyness in the fdinfo interface.
>
> KMD accumulation happens just before the context is unpinned and when
> context switches out. This works for execlist back-end since execlist
> scheduling has visibility into context switches. With GuC mode, KMD does
> not have visibility into context switches and this counter is
> accumulated only when context is unpinned. Context is unpinned once the
> context scheduling is successfully disabled. Disabling context
> scheduling is an asynchronous operation. Also if a context is servicing
> frequent requests, scheduling may never be disabled on it.
>
> For GuC mode, since updates to the context runtime may be delayed, add
> hooks to update the context runtime in a worker thread as well as when
> a user queries for it.
>
> Limitation:
> - If a context is never switched out or runs for a long period of time,
> the runtime value of CTX_TIMESTAMP may never be updated, so the
> counter value may be unreliable. This patch does not support such
> cases. Such support must be available from the GuC FW and it is WIP.
>
> This patch is an extract from previous work authored by John/Umesh here -
> https://patchwork.freedesktop.org/patch/496441/?series=105085&rev=4
>
> v2: (Ashutosh)
> - Drop COPS_RUNTIME_ACTIVE_TOTAL
> - s/guc_context_update_clks/__guc_context_update_stats
> - Pin context before accessing in guc_timestamp_ping
> - In guc_context_unpin, use spinlock to serialize access to runtime stats
LGTM now:
Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
I have made a few notes for myself below.
>
> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
> Co-developed-by: John Harrison <John.C.Harrison@Intel.com>
> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
> ---
> drivers/gpu/drm/i915/gt/intel_context.c | 5 ++-
> drivers/gpu/drm/i915/gt/intel_context.h | 2 +-
> drivers/gpu/drm/i915/gt/intel_context_types.h | 2 ++
> .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 31 +++++++++++++++++++
> 4 files changed, 38 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c
> index 2aa63ec521b8..a53b26178f0a 100644
> --- a/drivers/gpu/drm/i915/gt/intel_context.c
> +++ b/drivers/gpu/drm/i915/gt/intel_context.c
> @@ -578,10 +578,13 @@ void intel_context_bind_parent_child(struct intel_context *parent,
> child->parallel.parent = parent;
> }
>
> -u64 intel_context_get_total_runtime_ns(const struct intel_context *ce)
> +u64 intel_context_get_total_runtime_ns(struct intel_context *ce)
> {
> u64 total, active;
>
> + if (ce->ops->update_stats)
> + ce->ops->update_stats(ce);
This is indeed called from atomic context due to rcu_read_lock in
show_client_class, so we can't sleep in update_stats (and we are not due to
use of intel_context_pin_if_active).
Because we are already updating stats from guc_timestamp_ping, we could
have skipped updating them again from here without losing too much accuracy
but it's fine.
> +
> total = ce->stats.runtime.total;
> if (ce->ops->flags & COPS_RUNTIME_CYCLES)
> total *= ce->engine->gt->clock_period_ns;
> diff --git a/drivers/gpu/drm/i915/gt/intel_context.h b/drivers/gpu/drm/i915/gt/intel_context.h
> index 48f888c3da08..43ed92f8dc83 100644
> --- a/drivers/gpu/drm/i915/gt/intel_context.h
> +++ b/drivers/gpu/drm/i915/gt/intel_context.h
> @@ -375,7 +375,7 @@ intel_context_clear_nopreempt(struct intel_context *ce)
> clear_bit(CONTEXT_NOPREEMPT, &ce->flags);
> }
>
> -u64 intel_context_get_total_runtime_ns(const struct intel_context *ce);
> +u64 intel_context_get_total_runtime_ns(struct intel_context *ce);
> u64 intel_context_get_avg_runtime_ns(struct intel_context *ce);
>
> static inline u64 intel_context_clock(void)
> diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h b/drivers/gpu/drm/i915/gt/intel_context_types.h
> index e36670f2e626..aceaac28a33e 100644
> --- a/drivers/gpu/drm/i915/gt/intel_context_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_context_types.h
> @@ -58,6 +58,8 @@ struct intel_context_ops {
>
> void (*sched_disable)(struct intel_context *ce);
>
> + void (*update_stats)(struct intel_context *ce);
> +
> void (*reset)(struct intel_context *ce);
> void (*destroy)(struct kref *kref);
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> index ee3e8352637f..c869ddc73e69 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> @@ -1402,13 +1402,34 @@ static void __update_guc_busyness_stats(struct intel_guc *guc)
> spin_unlock_irqrestore(&guc->timestamp.lock, flags);
> }
>
> +static void __guc_context_update_stats(struct intel_context *ce)
> +{
> + struct intel_guc *guc = ce_to_guc(ce);
> + unsigned long flags;
> +
> + spin_lock_irqsave(&guc->timestamp.lock, flags);
> + lrc_update_runtime(ce);
> + spin_unlock_irqrestore(&guc->timestamp.lock, flags);
> +}
> +
> +static void guc_context_update_stats(struct intel_context *ce)
> +{
> + if (!intel_context_pin_if_active(ce))
Correct, all we need here is intel_context_pin_if_active. If the context is
not pinned we updated stats when we unpinned.
> + return;
> +
> + __guc_context_update_stats(ce);
> + intel_context_unpin(ce);
> +}
> +
> static void guc_timestamp_ping(struct work_struct *wrk)
> {
> struct intel_guc *guc = container_of(wrk, typeof(*guc),
> timestamp.work.work);
> struct intel_uc *uc = container_of(guc, typeof(*uc), guc);
> struct intel_gt *gt = guc_to_gt(guc);
> + struct intel_context *ce;
> intel_wakeref_t wakeref;
> + unsigned long index;
> int srcu, ret;
>
> /*
> @@ -1424,6 +1445,10 @@ static void guc_timestamp_ping(struct work_struct *wrk)
> with_intel_runtime_pm(>->i915->runtime_pm, wakeref)
> __update_guc_busyness_stats(guc);
>
> + /* adjust context stats for overflow */
> + xa_for_each(&guc->context_lookup, index, ce)
> + guc_context_update_stats(ce);
This helps prevent overflow if the context switched out (which updated the
runtime in the context image) but was not not unpinned.
> +
> intel_gt_reset_unlock(gt, srcu);
>
> guc_enable_busyness_worker(guc);
> @@ -2774,6 +2799,7 @@ static void guc_context_unpin(struct intel_context *ce)
> {
> struct intel_guc *guc = ce_to_guc(ce);
>
> + __guc_context_update_stats(ce);
Correct, cannot call intel_context_unpin (from guc_context_update_stats)
since this function is called from intel_context_unpin.
> unpin_guc_id(guc, ce);
> lrc_unpin(ce);
>
> @@ -3455,6 +3481,7 @@ static void remove_from_context(struct i915_request *rq)
> }
>
> static const struct intel_context_ops guc_context_ops = {
> + .flags = COPS_RUNTIME_CYCLES,
> .alloc = guc_context_alloc,
>
> .close = guc_context_close,
> @@ -3473,6 +3500,8 @@ static const struct intel_context_ops guc_context_ops = {
>
> .sched_disable = guc_context_sched_disable,
>
> + .update_stats = guc_context_update_stats,
> +
> .reset = lrc_reset,
> .destroy = guc_context_destroy,
>
> @@ -3728,6 +3757,7 @@ static int guc_virtual_context_alloc(struct intel_context *ce)
> }
>
> static const struct intel_context_ops virtual_guc_context_ops = {
> + .flags = COPS_RUNTIME_CYCLES,
> .alloc = guc_virtual_context_alloc,
>
> .close = guc_context_close,
> @@ -3745,6 +3775,7 @@ static const struct intel_context_ops virtual_guc_context_ops = {
> .exit = guc_virtual_context_exit,
>
> .sched_disable = guc_context_sched_disable,
> + .update_stats = guc_context_update_stats,
The approach should also work for virtual engines.
>
> .destroy = guc_context_destroy,
>
> --
> 2.36.1
>
Thanks.
--
Ashutosh
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2023-04-28 21:56 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-04-05 0:14 [Intel-gfx] [PATCH 0/2] fdinfo: Enable some support for GuC based client busyness Umesh Nerlige Ramappa
2023-04-05 0:14 ` [Intel-gfx] [PATCH 1/2] i915/pmu: Add support for total context runtime for GuC back-end Umesh Nerlige Ramappa
2023-04-24 17:41 ` Dixit, Ashutosh
2023-04-27 0:11 ` Umesh Nerlige Ramappa
2023-04-27 0:51 ` Dixit, Ashutosh
2023-04-27 18:57 ` Umesh Nerlige Ramappa
2023-04-05 0:14 ` [Intel-gfx] [PATCH 2/2] drm/i915/fdinfo: Enable fdinfo for GuC backends Umesh Nerlige Ramappa
2023-04-05 1:46 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for fdinfo: Enable some support for GuC based client busyness Patchwork
2023-04-05 1:59 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2023-04-05 12:40 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
-- strict thread matches above, loose matches on Subject: below --
2023-04-27 22:47 [Intel-gfx] [PATCH 0/2] " Umesh Nerlige Ramappa
2023-04-27 22:47 ` [Intel-gfx] [PATCH 1/2] i915/pmu: Add support for total context runtime for GuC back-end Umesh Nerlige Ramappa
2023-04-28 21:55 ` Dixit, Ashutosh
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox