Re: [PATCH v4 1/3] drm/panthor: Add tracepoint for cache flushing

[Liviu Dudau <liviu.dudau@arm.com>]

On Wed, Aug 12, 2026 at 04:07:18PM +0200, Nicolas Frattaroli wrote:
> Add a new event tracepoint: gpu_cache_flush to be emitted after a GPU
> cache flush completes, with duration and return status arguments.
> 
> This allows debugging the duration a flush takes irrespective of initial
> function entry lock contention, and communicates information about
> whether the flush timed out or errored out in other ways, and which
> caches were flushed.
> 
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>

Best regards,
Liviu

> ---
>  drivers/gpu/drm/panthor/panthor_gpu.c   | 26 +++++++++++++++++++++-
>  drivers/gpu/drm/panthor/panthor_trace.h | 38 +++++++++++++++++++++++++++++++++
>  2 files changed, 63 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/panthor/panthor_gpu.c b/drivers/gpu/drm/panthor/panthor_gpu.c
> index c013d6bf9a59..7088371c6d64 100644
> --- a/drivers/gpu/drm/panthor/panthor_gpu.c
> +++ b/drivers/gpu/drm/panthor/panthor_gpu.c
> @@ -317,6 +317,21 @@ int panthor_gpu_l2_power_on(struct panthor_device *ptdev)
>  	return panthor_gpu_power_on(ptdev, L2, 1, 20000);
>  }
>  
> +static inline void panthor_gpu_emit_flush_caches_tp(struct panthor_device *ptdev,
> +						    u64 start, u32 l2, u32 lsc,
> +						    u32 other, int ret)
> +{
> +	u32 duration;
> +
> +	if (!tracepoint_enabled(gpu_cache_flush) || !start)
> +		return;
> +
> +	if (check_sub_overflow(ktime_get_ns(), start, &duration))
> +		duration = U32_MAX;
> +
> +	trace_gpu_cache_flush(ptdev->base.dev, l2, lsc, other, duration, ret);
> +}
> +
>  /**
>   * panthor_gpu_flush_caches() - Flush caches
>   * @ptdev: Device.
> @@ -331,12 +346,17 @@ int panthor_gpu_flush_caches(struct panthor_device *ptdev,
>  {
>  	struct panthor_gpu *gpu = ptdev->gpu;
>  	unsigned long flags;
> +	u64 start = 0;
>  	int ret = 0;
>  
>  	/* Serialize cache flush operations. */
>  	guard(mutex)(&ptdev->gpu->cache_flush_lock);
>  
>  	spin_lock_irqsave(&ptdev->gpu->reqs_lock, flags);
> +
> +	if (tracepoint_enabled(gpu_cache_flush))
> +		start = ktime_get_ns();
> +
>  	if (!(ptdev->gpu->pending_reqs & GPU_IRQ_CLEAN_CACHES_COMPLETED)) {
>  		ptdev->gpu->pending_reqs |= GPU_IRQ_CLEAN_CACHES_COMPLETED;
>  		gpu_write(gpu->iomem, GPU_CMD, GPU_FLUSH_CACHES(l2, lsc, other));
> @@ -345,8 +365,10 @@ int panthor_gpu_flush_caches(struct panthor_device *ptdev,
>  	}
>  	spin_unlock_irqrestore(&ptdev->gpu->reqs_lock, flags);
>  
> -	if (ret)
> +	if (ret) {
> +		panthor_gpu_emit_flush_caches_tp(ptdev, start, l2, lsc, other, ret);
>  		return ret;
> +	}
>  
>  	if (!wait_event_timeout(ptdev->gpu->reqs_acked,
>  				!(ptdev->gpu->pending_reqs & GPU_IRQ_CLEAN_CACHES_COMPLETED),
> @@ -360,6 +382,8 @@ int panthor_gpu_flush_caches(struct panthor_device *ptdev,
>  		spin_unlock_irqrestore(&ptdev->gpu->reqs_lock, flags);
>  	}
>  
> +	panthor_gpu_emit_flush_caches_tp(ptdev, start, l2, lsc, other, ret);
> +
>  	if (ret) {
>  		panthor_device_schedule_reset(ptdev);
>  		drm_err(&ptdev->base, "Flush caches timeout");
> diff --git a/drivers/gpu/drm/panthor/panthor_trace.h b/drivers/gpu/drm/panthor/panthor_trace.h
> index 6ffeb4fe6599..bd8652549ab4 100644
> --- a/drivers/gpu/drm/panthor/panthor_trace.h
> +++ b/drivers/gpu/drm/panthor/panthor_trace.h
> @@ -76,6 +76,44 @@ TRACE_EVENT(gpu_job_irq,
>  		  __entry->events, __entry->duration_ns)
>  );
>  
> +/**
> + * gpu_cache_flush - emitted after cache flush completes
> + * @dev: pointer to the &struct device, for printing the device name
> + * @l2: "l2" flush flags
> + * @lsc: "lsc" flush flags
> + * @other: "other" flush flags
> + * @duration_ns: how long the cache flush operation took, in nanoseconds
> + * @ret: return status, 0 == success, negative errno on error
> + *
> + * Begins measuring after any initial lock contention around the locks needed
> + * for flushing caches, but before the actual cache flush is requested. Stops
> + * measuring and is emitted after flush operation is over.
> + */
> +TRACE_EVENT(gpu_cache_flush,
> +	    TP_PROTO(const struct device *dev, u32 l2, u32 lsc, u32 other,
> +		     u32 duration_ns, int ret),
> +	    TP_ARGS(dev, l2, lsc, other, duration_ns, ret),
> +	    TP_STRUCT__entry(
> +		    __string(dev_name, dev_name(dev))
> +		    __field(u32, l2)
> +		    __field(u32, lsc)
> +		    __field(u32, other)
> +		    __field(u32, duration_ns)
> +		    __field(int, ret)
> +	    ),
> +	    TP_fast_assign(
> +		    __assign_str(dev_name);
> +		    __entry->l2          = l2;
> +		    __entry->lsc         = lsc;
> +		    __entry->other       = other;
> +		    __entry->duration_ns = duration_ns;
> +		    __entry->ret         = ret;
> +	    ),
> +	    TP_printk("%s: l2=0x%x lsc=0x%x other=0x%x duration_ns=%u ret=%d",
> +		      __get_str(dev_name), __entry->l2, __entry->lsc,
> +		      __entry->other, __entry->duration_ns, __entry->ret)
> +);
> +
>  #endif /* __PANTHOR_TRACE_H__ */
>  
>  #undef TRACE_INCLUDE_PATH
> 
> -- 
> 2.55.0
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯