Re: [PATCH 3/3] media: hantro: add per-context fdinfo usage stats

[Detlev Casanova <detlev.casanova@collabora.com>]

Hi Nicolas,

On 6/12/26 11:18, Nicolas Dufresne wrote:
> Hi,
>
> Le vendredi 12 juin 2026 à 10:26 -0400, Detlev Casanova a écrit :
>> From: Christopher Healy <healych@amazon.com>
>>
>> Add per-file-descriptor hardware utilization tracking to the Hantro
>> VPU stateless codec driver, exposed via /proc/<pid>/fdinfo/<fd>.
>>
>> Record a ktime timestamp when each job is submitted to hardware in
>> device_run(), and accumulate the elapsed nanoseconds when the job
>> completes in hantro_job_finish(). Report the accumulated time along
>> with the current clock frequency through the new V4L2 show_fdinfo
>> callback.
>>
>> The output uses a media- key prefix with the following keys:
>>    media-driver:           driver name
>>    media-engine-<eng>:     accumulated busy time in nanoseconds
>>    media-maxfreq-<eng>:    maximum engine frequency in Hz
>>    media-curfreq-<eng>:    current engine frequency in Hz
>>
>> Where <eng> is "decoder" or "encoder" depending on the context.
>>
>> This enables userspace monitoring tools to compute per-process decoder
>> and encoder utilization. The current and max frequency keys report the
>> same value today since the driver lacks devfreq support, but will
>> diverge once DVFS is added, allowing userspace to approximate true
>> capacity utilization without any fdinfo code changes. A future series
>> can add hardware cycle counter support (via media-cycles-<eng>) for
>> exact utilization under DVFS, with no changes to the existing uAPI.
>>
>> Signed-off-by: Christopher Healy <healych@amazon.com>
>> Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
>> ---
>>   drivers/media/platform/verisilicon/hantro.h     |  5 +++++
>>   drivers/media/platform/verisilicon/hantro_drv.c | 22 ++++++++++++++++++++++
>>   2 files changed, 27 insertions(+)
>>
>> diff --git a/drivers/media/platform/verisilicon/hantro.h b/drivers/media/platform/verisilicon/hantro.h
>> index 0353de154a1e..10287e5cebac 100644
>> --- a/drivers/media/platform/verisilicon/hantro.h
>> +++ b/drivers/media/platform/verisilicon/hantro.h
>> @@ -16,6 +16,7 @@
>>   #include <linux/videodev2.h>
>>   #include <linux/wait.h>
>>   #include <linux/clk.h>
>> +#include <linux/ktime.h>
>>   #include <linux/reset.h>
>>   
>>   #include <media/v4l2-ctrls.h>
>> @@ -268,6 +269,10 @@ struct hantro_ctx {
>>   	struct hantro_postproc_ctx postproc;
>>   	bool need_postproc;
>>   
>> +	/* Statistics for debugging and performance measurements. */
>> +	ktime_t start_time;
>> +	u64 total_ns;
>> +
>>   	/* Specific for particular codec modes. */
>>   	union {
>>   		struct hantro_h264_dec_hw_ctx h264_dec;
>> diff --git a/drivers/media/platform/verisilicon/hantro_drv.c b/drivers/media/platform/verisilicon/hantro_drv.c
>> index 2e81877f640f..139c2e6a83aa 100644
>> --- a/drivers/media/platform/verisilicon/hantro_drv.c
>> +++ b/drivers/media/platform/verisilicon/hantro_drv.c
>> @@ -90,6 +90,8 @@ static void hantro_job_finish(struct hantro_dev *vpu,
>>   			      struct hantro_ctx *ctx,
>>   			      enum vb2_buffer_state result)
>>   {
>> +	ctx->total_ns += ktime_to_ns(ktime_sub(ktime_get(), ctx->start_time));
>> +
>>   	pm_runtime_put_autosuspend(vpu->dev);
>>   
>>   	clk_bulk_disable(vpu->variant->num_clocks, vpu->clocks);
>> @@ -186,6 +188,8 @@ static void device_run(void *priv)
>>   
>>   	v4l2_m2m_buf_copy_metadata(src, dst);
>>   
>> +	ctx->start_time = ktime_get();
>> +
>>   	if (ctx->codec_ops->run(ctx))
>>   		goto err_cancel_job;
>>   
>> @@ -701,10 +705,28 @@ static int hantro_release(struct file *filp)
>>   	return 0;
>>   }
>>   
>> +static void hantro_show_fdinfo(struct seq_file *m, struct file *f)
>> +{
>> +	struct hantro_ctx *ctx = file_to_ctx(f);
>> +	struct hantro_dev *vpu = ctx->dev;
>> +
>> +	seq_printf(m, "media-driver:\t%s\n", DRIVER_NAME);
>> +	seq_printf(m, "media-engine-%s:\t%llu ns\n",
>> +		   ctx->is_encoder ? "encoder" : "decoder",
>> +		   ctx->total_ns);
>> +	seq_printf(m, "media-maxfreq-%s:\t%lu Hz\n",
>> +		   ctx->is_encoder ? "encoder" : "decoder",
>> +		   clk_get_rate(vpu->clocks[0].clk));
>> +	seq_printf(m, "media-curfreq-%s:\t%lu Hz\n",
>> +		   ctx->is_encoder ? "encoder" : "decoder",
>> +		   clk_get_rate(vpu->clocks[0].clk));
> Similar to what was done in DRM helpers, I would create a common structure,
> maybe in a v4l2-stats library (or some better name). The driver would fill that
> structure directly, or through helpers (such as software timer helper, cycle
> count translation helper), and finally, this show_fd_info() would be a one
> liners, just like DRM do.
>
> This will ensure full consistency in the trace, and avoid possible future
> unresolvable compatibility issue due to past mistakes. If we ever let a bug
> through (of course we won't :-D) all driver will have the same bug, and it will
> be easier to not break userspace.
Yes, that makes a lot of sense actually, And will also help with the 
ftrace HW info if we have stats and debug info centralized in v4l2
> Nicolas
>
>> +}
>> +
>>   static const struct v4l2_file_operations hantro_fops = {
>>   	.owner = THIS_MODULE,
>>   	.open = hantro_open,
>>   	.release = hantro_release,
>> +	.show_fdinfo = hantro_show_fdinfo,
>>   	.poll = v4l2_m2m_fop_poll,
>>   	.unlocked_ioctl = video_ioctl2,
>>   	.mmap = v4l2_m2m_fop_mmap,