Re: [Intel-gfx] [Intel-gfx 1/1] drm/i915/guc: Don't update engine busyness stats too frequently

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 11/06/2022 18:27, Alan Previn wrote:
> Using igt's gem-create and with additional patches to track object
> creation time, it was measured that guc_update_engine_gt_clks was
> getting called over 188 thousand times in the span of 15 seconds
> (running the test three times).
> 
> Get a jiffies sample on every trigger and ensure we skip sampling
> if we are being called too soon. Use half of the ping_delay as a
> safe threshold.
> 
> NOTE: with this change, the number of calls went down to just 14
> over the same span of time (matching the original intent of running
> about once every 24 seconds, at 19.2Mhz GT freq, per engine).

+ Umesh

What is the effect on accuracy? AFAIR up to date clock was needed for 
correct accounting of running contexts.

Regards,

Tvrtko

> 
> Signed-off-by: Alan Previn <alan.previn.teres.alexis@intel.com>
> ---
>   drivers/gpu/drm/i915/gt/intel_engine_types.h      | 10 ++++++++++
>   drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c |  9 +++++++++
>   2 files changed, 19 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> index 2286f96f5f87..63f4ecdf1606 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> @@ -323,6 +323,16 @@ struct intel_engine_guc_stats {
>   	 * @start_gt_clk: GT clock time of last idle to active transition.
>   	 */
>   	u64 start_gt_clk;
> +
> +	/**
> +	 * @last_jiffies: Jiffies at last actual stats collection time
> +	 *
> +	 * We use this timestamp to ensure we don't oversample the
> +	 * stats because runtime power management events can trigger
> +	 * stats collection at much higher rates than required.
> +	 */
> +	u64 last_jiffies;
> +
>   };
>   
>   struct intel_engine_cs {
> 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 5a1dfacf24ea..8f8bf6e40ccb 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> @@ -1167,6 +1167,15 @@ static void guc_update_engine_gt_clks(struct intel_engine_cs *engine)
>   	struct intel_engine_guc_stats *stats = &engine->stats.guc;
>   	struct intel_guc *guc = &engine->gt->uc.guc;
>   	u32 last_switch, ctx_id, total;
> +	u64 newjiffs;
> +
> +	/* Don't worry about jiffies wrap-around, a rare additional sample won't have any impact */
> +	newjiffs = get_jiffies_64();
> +	if (stats->last_jiffies && (newjiffs - stats->last_jiffies <
> +	   (guc->timestamp.ping_delay << 1)))
> +		return;
> +
> +	stats->last_jiffies = newjiffs;
>   
>   	lockdep_assert_held(&guc->timestamp.lock);
>