Re: [PATCH 2/3] i915/guc: Ensure busyness counter increases monotonically

[John Harrison <john.c.harrison@intel.com>]

On 11/18/2024 15:22, Umesh Nerlige Ramappa wrote:
> Active busyness of an engine is calculated using gt timestamp and the
> context switch in time. While capturing the gt timestamp, it's possible
> that the context switches out. This race could result in an active
> busyness value that is greater than the actual context runtime value by a
> small amount. This leads to a negative delta and throws off busyness
> calculations for the user.
>
> If a subsequent count is smaller than the previous one, just return the
> previous one, since we expect the busyness to catch up.
>
> Fixes: 77cdd054dd2c ("drm/i915/pmu: Connect engine busyness stats from GuC to pmu")
> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
Reviewed-by: John Harrison <John.C.Harrison@Intel.com>

> ---
>   drivers/gpu/drm/i915/gt/intel_engine_types.h      | 5 +++++
>   drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 5 ++++-
>   2 files changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> index ba55c059063d..fe1f85e5dda3 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> @@ -343,6 +343,11 @@ struct intel_engine_guc_stats {
>   	 * @start_gt_clk: GT clock time of last idle to active transition.
>   	 */
>   	u64 start_gt_clk;
> +
> +	/**
> +	 * @total: The last value of total returned
> +	 */
> +	u64 total;
>   };
>   
>   union intel_engine_tlb_inv_reg {
> 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 c71aedcbce43..56be9f385270 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> @@ -1378,9 +1378,12 @@ static ktime_t guc_engine_busyness(struct intel_engine_cs *engine, ktime_t *now)
>   		total += intel_gt_clock_interval_to_ns(gt, clk);
>   	}
>   
> +	if (total > stats->total)
> +		stats->total = total;
> +
>   	spin_unlock_irqrestore(&guc->timestamp.lock, flags);
>   
> -	return ns_to_ktime(total);
> +	return ns_to_ktime(stats->total);
>   }
>   
>   static void guc_enable_busyness_worker(struct intel_guc *guc)