From e70d65d1fad18ce3f2348f369c85c2faed9d3c82 Mon Sep 17 00:00:00 2001 From: Kevin Park Date: Wed, 29 Nov 2023 13:57:00 +0000 Subject: [Official] MIDCET-4922, GPUCORE-40482 Emitting unmerged gpu_metrics tracepoints There might be multiple GPU activities in a work period (500ms). Current gpu_metrics implementation merged all GPU activities for the given work period and emit single tracepoint rather than emitting per-activity multiple tracepoints. This loses the granularity of each activity even if the total active time of a context (application) for a work period is correct. For better granularity, we instead emit one tracepoint per GPU activity. Since we don't have to merge activities in a work period, some members of struct kbase_gpu_metrics_ctx are no longer needed. |------------------- Work-period 1 -----------------------| S1---E1 S2----E2 S3------------E3 For work-period 1, previously 1 tracepoint was emitted and now 3 will be emitted for better granularity of GPU activity. |------------------- Work-period 2 -----------------------| S1---E1 S2--------E2 S3------------------E3 For work-period 2 (overlapped case), they will be treated as 1 activity. Hence 1 tracepoint will be emitted as previously. 'multiple null jobs' will be skipped as the kbase change will emit tracepoint per each activity. Bug: 301904509 Test: Perfetto trace inspection (Oriole, Felix, Husky) Provenance: https://code.ipdelivery.arm.com/c/GPU/mali-ddk/+/6314 Change-Id: I21b8ebe7af3b429dcd9fef20dabf97295a2a5ab7 Signed-off-by: Mattias Simonsson --- mali_kbase/mali_kbase_defs.h | 22 ++----- mali_kbase/mali_kbase_gpu_metrics.c | 113 +++++++++--------------------------- mali_kbase/mali_kbase_gpu_metrics.h | 12 ++-- 3 files changed, 38 insertions(+), 109 deletions(-) (limited to 'mali_kbase') diff --git a/mali_kbase/mali_kbase_defs.h b/mali_kbase/mali_kbase_defs.h index b6e9147..d7adc7d 100644 --- a/mali_kbase/mali_kbase_defs.h +++ b/mali_kbase/mali_kbase_defs.h @@ -198,16 +198,11 @@ struct kbase_gpu_metrics { * * @link: Links the object in kbase_device::gpu_metrics::active_list * or kbase_device::gpu_metrics::inactive_list. - * @first_active_start_time: Records the time at which the application first became + * @active_start_time: Records the time at which the application first became * active in the current work period. - * @last_active_start_time: Records the time at which the application last became - * active in the current work period. - * @last_active_end_time: Records the time at which the application last became - * inactive in the current work period. - * @total_active: Tracks the time for which application has been active - * in the current work period. - * @prev_wp_active_end_time: Records the time at which the application last became - * inactive in the previous work period. + * @active_end_time: Records the time at which the application last became + * inactive in the current work period, or the time of the end of + * previous work period if the application remained active. * @aid: Unique identifier for an application. * @kctx_count: Counter to keep a track of the number of Kbase contexts * created for an application. There may be multiple Kbase @@ -215,19 +210,14 @@ struct kbase_gpu_metrics { * metrics context. * @active_cnt: Counter that is updated every time the GPU activity starts * and ends in the current work period for an application. - * @flags: Flags to track the state of GPU metrics context. */ struct kbase_gpu_metrics_ctx { struct list_head link; - u64 first_active_start_time; - u64 last_active_start_time; - u64 last_active_end_time; - u64 total_active; - u64 prev_wp_active_end_time; + u64 active_start_time; + u64 active_end_time; unsigned int aid; unsigned int kctx_count; u8 active_cnt; - u8 flags; }; #endif diff --git a/mali_kbase/mali_kbase_gpu_metrics.c b/mali_kbase/mali_kbase_gpu_metrics.c index d404c69..e5dcde9 100644 --- a/mali_kbase/mali_kbase_gpu_metrics.c +++ b/mali_kbase/mali_kbase_gpu_metrics.c @@ -29,46 +29,12 @@ #include #include -/** - * enum gpu_metrics_ctx_flags - Flags for the GPU metrics context - * - * @ACTIVE_INTERVAL_IN_WP: Flag set when the application first becomes active in - * the current work period. - * - * @INSIDE_ACTIVE_LIST: Flag to track if object is in kbase_device::gpu_metrics::active_list - * - * All members need to be separate bits. This enum is intended for use in a - * bitmask where multiple values get OR-ed together. - */ -enum gpu_metrics_ctx_flags { - ACTIVE_INTERVAL_IN_WP = 1 << 0, - INSIDE_ACTIVE_LIST = 1 << 1, -}; - static unsigned long gpu_metrics_tp_emit_interval_ns = DEFAULT_GPU_METRICS_TP_EMIT_INTERVAL_NS; module_param(gpu_metrics_tp_emit_interval_ns, ulong, 0444); MODULE_PARM_DESC(gpu_metrics_tp_emit_interval_ns, "Time interval in nano seconds at which GPU metrics tracepoints are emitted"); -static inline bool gpu_metrics_ctx_flag(struct kbase_gpu_metrics_ctx *gpu_metrics_ctx, - enum gpu_metrics_ctx_flags flag) -{ - return (gpu_metrics_ctx->flags & flag); -} - -static inline void gpu_metrics_ctx_flag_set(struct kbase_gpu_metrics_ctx *gpu_metrics_ctx, - enum gpu_metrics_ctx_flags flag) -{ - gpu_metrics_ctx->flags |= flag; -} - -static inline void gpu_metrics_ctx_flag_clear(struct kbase_gpu_metrics_ctx *gpu_metrics_ctx, - enum gpu_metrics_ctx_flags flag) -{ - gpu_metrics_ctx->flags &= ~flag; -} - static inline void validate_tracepoint_data(struct kbase_gpu_metrics_ctx *gpu_metrics_ctx, u64 start_time, u64 end_time, u64 total_active) { @@ -82,43 +48,30 @@ static inline void validate_tracepoint_data(struct kbase_gpu_metrics_ctx *gpu_me WARN(total_active > (end_time - start_time), "total_active %llu > end_time %llu - start_time %llu for aid %u active_cnt %u", total_active, end_time, start_time, gpu_metrics_ctx->aid, gpu_metrics_ctx->active_cnt); - - WARN(gpu_metrics_ctx->prev_wp_active_end_time > start_time, - "prev_wp_active_end_time %llu > start_time %llu for aid %u active_cnt %u", - gpu_metrics_ctx->prev_wp_active_end_time, start_time, gpu_metrics_ctx->aid, - gpu_metrics_ctx->active_cnt); #endif } static void emit_tracepoint_for_active_gpu_metrics_ctx( struct kbase_device *kbdev, struct kbase_gpu_metrics_ctx *gpu_metrics_ctx, u64 current_time) { - const u64 start_time = gpu_metrics_ctx->first_active_start_time; - u64 total_active = gpu_metrics_ctx->total_active; - u64 end_time; + const u64 start_time = gpu_metrics_ctx->active_start_time; + u64 total_active, end_time = current_time; /* Check if the GPU activity is currently ongoing */ if (gpu_metrics_ctx->active_cnt) { /* The following check is to handle the race on CSF GPUs that can happen between * the draining of trace buffer and FW emitting the ACT=1 event . */ - if (unlikely(current_time == gpu_metrics_ctx->last_active_start_time)) - current_time++; - end_time = current_time; - total_active += end_time - gpu_metrics_ctx->last_active_start_time; - - gpu_metrics_ctx->first_active_start_time = current_time; - gpu_metrics_ctx->last_active_start_time = current_time; - } else { - end_time = gpu_metrics_ctx->last_active_end_time; - gpu_metrics_ctx_flag_clear(gpu_metrics_ctx, ACTIVE_INTERVAL_IN_WP); + if (unlikely(end_time == start_time)) + end_time++; + gpu_metrics_ctx->active_start_time = end_time; } + total_active = end_time - start_time; trace_gpu_work_period(kbdev->id, gpu_metrics_ctx->aid, start_time, end_time, total_active); validate_tracepoint_data(gpu_metrics_ctx, start_time, end_time, total_active); - gpu_metrics_ctx->prev_wp_active_end_time = end_time; - gpu_metrics_ctx->total_active = 0; + gpu_metrics_ctx->active_end_time = end_time; } void kbase_gpu_metrics_ctx_put(struct kbase_device *kbdev, @@ -131,7 +84,8 @@ void kbase_gpu_metrics_ctx_put(struct kbase_device *kbdev, if (gpu_metrics_ctx->kctx_count) return; - if (gpu_metrics_ctx_flag(gpu_metrics_ctx, ACTIVE_INTERVAL_IN_WP)) + /* Generate a tracepoint if there's still activity */ + if (gpu_metrics_ctx->active_cnt) emit_tracepoint_for_active_gpu_metrics_ctx(kbdev, gpu_metrics_ctx, ktime_get_raw_ns()); @@ -166,12 +120,11 @@ struct kbase_gpu_metrics_ctx *kbase_gpu_metrics_ctx_get(struct kbase_device *kbd void kbase_gpu_metrics_ctx_init(struct kbase_device *kbdev, struct kbase_gpu_metrics_ctx *gpu_metrics_ctx, unsigned int aid) { + gpu_metrics_ctx->active_start_time = 0; + gpu_metrics_ctx->active_end_time = 0; gpu_metrics_ctx->aid = aid; - gpu_metrics_ctx->total_active = 0; gpu_metrics_ctx->kctx_count = 1; gpu_metrics_ctx->active_cnt = 0; - gpu_metrics_ctx->prev_wp_active_end_time = 0; - gpu_metrics_ctx->flags = 0; list_add_tail(&gpu_metrics_ctx->link, &kbdev->gpu_metrics.inactive_list); } @@ -180,17 +133,9 @@ void kbase_gpu_metrics_ctx_start_activity(struct kbase_context *kctx, u64 timest struct kbase_gpu_metrics_ctx *gpu_metrics_ctx = kctx->gpu_metrics_ctx; gpu_metrics_ctx->active_cnt++; - if (gpu_metrics_ctx->active_cnt == 1) - gpu_metrics_ctx->last_active_start_time = timestamp_ns; - - if (!gpu_metrics_ctx_flag(gpu_metrics_ctx, ACTIVE_INTERVAL_IN_WP)) { - gpu_metrics_ctx->first_active_start_time = timestamp_ns; - gpu_metrics_ctx_flag_set(gpu_metrics_ctx, ACTIVE_INTERVAL_IN_WP); - } - - if (!gpu_metrics_ctx_flag(gpu_metrics_ctx, INSIDE_ACTIVE_LIST)) { + if (gpu_metrics_ctx->active_cnt == 1) { + gpu_metrics_ctx->active_start_time = timestamp_ns; list_move_tail(&gpu_metrics_ctx->link, &kctx->kbdev->gpu_metrics.active_list); - gpu_metrics_ctx_flag_set(gpu_metrics_ctx, INSIDE_ACTIVE_LIST); } } @@ -201,22 +146,22 @@ void kbase_gpu_metrics_ctx_end_activity(struct kbase_context *kctx, u64 timestam if (WARN_ON_ONCE(!gpu_metrics_ctx->active_cnt)) return; + /* Do not emit tracepoint if GPU activity still continues. */ if (--gpu_metrics_ctx->active_cnt) return; - if (likely(timestamp_ns > gpu_metrics_ctx->last_active_start_time)) { - gpu_metrics_ctx->last_active_end_time = timestamp_ns; - gpu_metrics_ctx->total_active += - timestamp_ns - gpu_metrics_ctx->last_active_start_time; + if (likely(timestamp_ns > gpu_metrics_ctx->active_start_time)) { + emit_tracepoint_for_active_gpu_metrics_ctx(kctx->kbdev, gpu_metrics_ctx, + timestamp_ns); return; } /* Due to conversion from system timestamp to CPU timestamp (which involves rounding) * the value for start and end timestamp could come as same on CSF GPUs. */ - if (timestamp_ns == gpu_metrics_ctx->last_active_start_time) { - gpu_metrics_ctx->last_active_end_time = timestamp_ns + 1; - gpu_metrics_ctx->total_active += 1; + if (timestamp_ns == gpu_metrics_ctx->active_start_time) { + emit_tracepoint_for_active_gpu_metrics_ctx(kctx->kbdev, gpu_metrics_ctx, + timestamp_ns + 1); return; } @@ -224,12 +169,9 @@ void kbase_gpu_metrics_ctx_end_activity(struct kbase_context *kctx, u64 timestam * visible to the Kbase even though the system timestamp value sampled by FW was less than * the system timestamp value sampled by Kbase just before the draining of trace buffer. */ - if (gpu_metrics_ctx->last_active_start_time == gpu_metrics_ctx->first_active_start_time && - gpu_metrics_ctx->prev_wp_active_end_time == gpu_metrics_ctx->first_active_start_time) { - WARN_ON_ONCE(gpu_metrics_ctx->total_active); - gpu_metrics_ctx->last_active_end_time = - gpu_metrics_ctx->prev_wp_active_end_time + 1; - gpu_metrics_ctx->total_active = 1; + if (gpu_metrics_ctx->active_end_time == gpu_metrics_ctx->active_start_time) { + emit_tracepoint_for_active_gpu_metrics_ctx(kctx->kbdev, gpu_metrics_ctx, + gpu_metrics_ctx->active_end_time + 1); return; } @@ -242,15 +184,12 @@ void kbase_gpu_metrics_emit_tracepoint(struct kbase_device *kbdev, u64 ts) struct kbase_gpu_metrics_ctx *gpu_metrics_ctx, *tmp; list_for_each_entry_safe(gpu_metrics_ctx, tmp, &gpu_metrics->active_list, link) { - if (!gpu_metrics_ctx_flag(gpu_metrics_ctx, ACTIVE_INTERVAL_IN_WP)) { - WARN_ON(!gpu_metrics_ctx_flag(gpu_metrics_ctx, INSIDE_ACTIVE_LIST)); - WARN_ON(gpu_metrics_ctx->active_cnt); - list_move_tail(&gpu_metrics_ctx->link, &gpu_metrics->inactive_list); - gpu_metrics_ctx_flag_clear(gpu_metrics_ctx, INSIDE_ACTIVE_LIST); + if (gpu_metrics_ctx->active_cnt) { + emit_tracepoint_for_active_gpu_metrics_ctx(kbdev, gpu_metrics_ctx, ts); continue; } - emit_tracepoint_for_active_gpu_metrics_ctx(kbdev, gpu_metrics_ctx, ts); + list_move_tail(&gpu_metrics_ctx->link, &gpu_metrics->inactive_list); } } diff --git a/mali_kbase/mali_kbase_gpu_metrics.h b/mali_kbase/mali_kbase_gpu_metrics.h index c445dff..658cf1c 100644 --- a/mali_kbase/mali_kbase_gpu_metrics.h +++ b/mali_kbase/mali_kbase_gpu_metrics.h @@ -106,7 +106,7 @@ void kbase_gpu_metrics_ctx_init(struct kbase_device *kbdev, * @kctx: Pointer to the Kbase context contributing data to the GPU metrics context. * @timestamp_ns: CPU timestamp at which the GPU activity started. * - * The provided timestamp would be later used as the "start_time_ns" for the + * The provided timestamp is used as the "start_time_ns" for the * power/gpu_work_period tracepoint if this is the first GPU activity for the GPU * metrics context in the current work period. * @@ -122,9 +122,9 @@ void kbase_gpu_metrics_ctx_start_activity(struct kbase_context *kctx, u64 timest * @kctx: Pointer to the Kbase context contributing data to the GPU metrics context. * @timestamp_ns: CPU timestamp at which the GPU activity ended. * - * The provided timestamp would be later used as the "end_time_ns" for the - * power/gpu_work_period tracepoint if this is the last GPU activity for the GPU - * metrics context in the current work period. + * The provided timestamp is used as the "end_time_ns" for the power/gpu_work_period + * tracepoint if this is the last GPU activity for the GPU metrics context + * in the current work period. * * Note: The caller must appropriately serialize the call to this function with the * call to other GPU metrics functions declared in this file. @@ -138,8 +138,8 @@ void kbase_gpu_metrics_ctx_end_activity(struct kbase_context *kctx, u64 timestam * @kbdev: Pointer to the GPU device. * @ts: Timestamp at which the tracepoint is being emitted. * - * This function would loop through all the active GPU metrics contexts and emit a - * power/gpu_work_period tracepoint for them. + * This function would loop through all GPU metrics contexts in the active list and + * emit a power/gpu_work_period tracepoint if the GPU work in the context still active. * The GPU metrics context that is found to be inactive since the last tracepoint * was emitted would be moved to the inactive list. * The current work period would be considered as over and a new work period would -- cgit v1.2.3