On Tue, Oct 26, 2021 at 05:48:20PM -0700, Umesh Nerlige Ramappa wrote:
In preparation for GuC pmu stats, add a name to the execlists stats structure so that it can be differentiated from the GuC stats.
Signed-off-by: Umesh Nerlige Ramappa umesh.nerlige.ramappa@intel.com
drivers/gpu/drm/i915/gt/intel_engine_cs.c | 14 +++--- drivers/gpu/drm/i915/gt/intel_engine_stats.h | 33 +++++++------ drivers/gpu/drm/i915/gt/intel_engine_types.h | 52 +++++++++++--------- 3 files changed, 53 insertions(+), 46 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c index ff6753ccb129..2de396e34d83 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c @@ -363,7 +363,7 @@ static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id, DRIVER_CAPS(i915)->has_logical_contexts = true;
ewma__engine_latency_init(&engine->latency);
- seqcount_init(&engine->stats.lock);
seqcount_init(&engine->stats.execlists.lock);
ATOMIC_INIT_NOTIFIER_HEAD(&engine->context_status_notifier);
@@ -1918,15 +1918,16 @@ void intel_engine_dump(struct intel_engine_cs *engine, static ktime_t __intel_engine_get_busy_time(struct intel_engine_cs *engine, ktime_t *now) {
- ktime_t total = engine->stats.total;
struct intel_engine_execlists_stats *stats = &engine->stats.execlists;
ktime_t total = stats->total;
/*
- If the engine is executing something at the moment
- add it to the total.
*/ *now = ktime_get();
- if (READ_ONCE(engine->stats.active))
total = ktime_add(total, ktime_sub(*now, engine->stats.start));
if (READ_ONCE(stats->active))
total = ktime_add(total, ktime_sub(*now, stats->start));return total;
} @@ -1940,13 +1941,14 @@ static ktime_t __intel_engine_get_busy_time(struct intel_engine_cs *engine, */ ktime_t intel_engine_get_busy_time(struct intel_engine_cs *engine, ktime_t *now) {
struct intel_engine_execlists_stats *stats = &engine->stats.execlists; unsigned int seq; ktime_t total;
do {
seq = read_seqcount_begin(&engine->stats.lock);
seq = read_seqcount_begin(&stats->lock);
- } while (read_seqcount_retry(&engine->stats.lock, seq));
} while (read_seqcount_retry(&stats->lock, seq));
return total;
} diff --git a/drivers/gpu/drm/i915/gt/intel_engine_stats.h b/drivers/gpu/drm/i915/gt/intel_engine_stats.h index 24fbdd94351a..8e762d683e50 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_stats.h +++ b/drivers/gpu/drm/i915/gt/intel_engine_stats.h @@ -15,45 +15,46 @@
static inline void intel_engine_context_in(struct intel_engine_cs *engine) {
- struct intel_engine_execlists_stats *stats = &engine->stats.execlists; unsigned long flags;
- if (engine->stats.active) {
engine->stats.active++;
if (stats->active) {
stats->active++;return; }
/* The writer is serialised; but the pmu reader may be from hardirq */ local_irq_save(flags);
- write_seqcount_begin(&engine->stats.lock);
- write_seqcount_begin(&stats->lock);
- engine->stats.start = ktime_get();
- engine->stats.active++;
- stats->start = ktime_get();
- stats->active++;
- write_seqcount_end(&engine->stats.lock);
- write_seqcount_end(&stats->lock); local_irq_restore(flags);
- GEM_BUG_ON(!engine->stats.active);
- GEM_BUG_ON(!stats->active);
}
static inline void intel_engine_context_out(struct intel_engine_cs *engine) {
- struct intel_engine_execlists_stats *stats = &engine->stats.execlists; unsigned long flags;
- GEM_BUG_ON(!engine->stats.active);
- if (engine->stats.active > 1) {
engine->stats.active--;
GEM_BUG_ON(!stats->active);
if (stats->active > 1) {
stats->active--;return; }
local_irq_save(flags);
- write_seqcount_begin(&engine->stats.lock);
- write_seqcount_begin(&stats->lock);
- engine->stats.active--;
- engine->stats.total =
ktime_add(engine->stats.total,ktime_sub(ktime_get(), engine->stats.start));
- stats->active--;
- stats->total = ktime_add(stats->total,
ktime_sub(ktime_get(), stats->start));
- write_seqcount_end(&engine->stats.lock);
- write_seqcount_end(&stats->lock); local_irq_restore(flags);
}
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h index e0f773585c29..24fa7fb0e7de 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h @@ -257,6 +257,33 @@ struct intel_engine_execlists {
#define INTEL_ENGINE_CS_MAX_NAME 8
+struct intel_engine_execlists_stats {
- /**
* @active: Number of contexts currently scheduled in.*/- unsigned int active;
- /**
* @lock: Lock protecting the below fields.*/- seqcount_t lock;
- /**
* @total: Total time this engine was busy.** Accumulated time not counting the most recent block in cases where* engine is currently busy (active > 0).*/- ktime_t total;
- /**
* @start: Timestamp of the last idle to active transition.** Idle is defined as active == 0, active is active > 0.*/- ktime_t start;
+};
struct intel_engine_cs { struct drm_i915_private *i915; struct intel_gt *gt; @@ -488,30 +515,7 @@ struct intel_engine_cs { u32 (*get_cmd_length_mask)(u32 cmd_header);
struct {
/*** @active: Number of contexts currently scheduled in.*/unsigned int active;/*** @lock: Lock protecting the below fields.*/seqcount_t lock;/*** @total: Total time this engine was busy.** Accumulated time not counting the most recent block in cases* where engine is currently busy (active > 0).*/ktime_t total;/*** @start: Timestamp of the last idle to active transition.** Idle is defined as active == 0, active is active > 0.*/ktime_t start;
struct intel_engine_execlists_stats execlists;
I'd probably just define this sub-structure inline rather defining it above. e.g.
struct { struct { fields... } execlists; } stats;
Then the follow up patch:
union { struct { fields... } execlists; struct { fields... } guc; } stats;
Or if there is common fields:
struct { common fields... struct { fields... } execlists; struct { fields... } guc; } stats;
That being said, if you prefer to leave it as is, that's fine too.
With that: Reviewed-by: Matthew Brost matthew.brost@intel.com
/** * @rps: Utilisation at last RPS sampling.-- 2.20.1