Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Hans Verkuil <hverkuil+cisco@kernel.org>
To: Detlev Casanova <detlev.casanova@collabora.com>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Nicolas Dufresne <nicolas.dufresne@collabora.com>,
	Benjamin Gaignard <benjamin.gaignard@collabora.com>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>,
	Heiko Stuebner <heiko@sntech.de>
Cc: linux-media@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-rockchip@lists.infradead.org, kernel@collabora.com,
	linux-arm-kernel@lists.infradead.org,
	Christopher Healy <healych@amazon.com>
Subject: Re: [PATCH v2 2/5] docs: media: add documentation for media client usage stats
Date: Fri, 19 Jun 2026 14:58:33 +0200	[thread overview]
Message-ID: <755cf7c1-6bcc-45d1-afea-192d393256af@kernel.org> (raw)
In-Reply-To: <20260617-v4l2-add-fdinfo-v2-2-d298e98ce06a@collabora.com>

Hi Detlev,

Interesting, I had never heard of fdinfo, so if nothing else, I learned something new!

On 17/06/2026 20:10, Detlev Casanova wrote:
> From: Christopher Healy <healych@amazon.com>
> 
> Document the media fdinfo interface for per-file-descriptor usage
> statistics exposed by stateless V4L2 codec drivers via
> /proc/<pid>/fdinfo/<fd>.
> 
> This interface is designed for stateless (request API based) codec
> devices where the kernel driver has per-job visibility into hardware
> execution. Stateful codecs cannot support all of this because their
> firmware manages job scheduling opaquely.
> 
> The specification defines media- prefixed keys for engine utilization
> time, and operating frequency, following the same conventions as the DRM
> fdinfo mechanism documented in drm-usage-stats.rst.
> 
> More fields can be added later.
> 
> Signed-off-by: Christopher Healy <healych@amazon.com>
> Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
> ---
>  .../userspace-api/media/drivers/index.rst          |  1 +
>  .../media/drivers/media-usage-stats.rst            | 85 ++++++++++++++++++++++
>  2 files changed, 86 insertions(+)
> 
> diff --git a/Documentation/userspace-api/media/drivers/index.rst b/Documentation/userspace-api/media/drivers/index.rst
> index 02967c9b18d6..61879738836c 100644
> --- a/Documentation/userspace-api/media/drivers/index.rst
> +++ b/Documentation/userspace-api/media/drivers/index.rst
> @@ -34,6 +34,7 @@ For more details see the file COPYING in the source distribution of Linux.
>  	imx-uapi
>  	mali-c55
>  	max2175
> +	media-usage-stats
>  	npcm-video
>  	omap3isp-uapi
>  	thp7312
> diff --git a/Documentation/userspace-api/media/drivers/media-usage-stats.rst b/Documentation/userspace-api/media/drivers/media-usage-stats.rst
> new file mode 100644
> index 000000000000..d3dc07002f62
> --- /dev/null
> +++ b/Documentation/userspace-api/media/drivers/media-usage-stats.rst
> @@ -0,0 +1,85 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +.. _media-usage-stats:
> +
> +==========================
> +Media client usage stats

stats -> statistics

But are these really statistics? Isn't it just the current status?
In many ways this feature looks to me similar to what VIDIOC_LOG_STATUS does,
except in a nicer format. When VIDIOC_LOG_STATUS was first added, fdinfo
didn't exist yet.

And I wouldn't call it 'Media client': it's specific to stateless V4L2
codec drivers.

> +==========================
> +
> +Stateless V4L2 codec drivers can optionally expose per-file-descriptor usage
> +statistics via ``/proc/<pid>/fdinfo/<fd>``. This is analogous to the DRM fdinfo
> +mechanism documented in :ref:`drm-client-usage-stats`, but uses the ``media-``
> +key prefix for V4L2 media devices.
> +
> +This interface is specific to stateless (request API based) codec devices,
> +including both decoders and encoders. With stateless codecs, the kernel driver
> +explicitly submits each frame to the hardware and receives a completion
> +interrupt, providing a clean per-job boundary that can be attributed to the
> +submitting file descriptor.
> +
> +Stateful codec devices cannot support this interface because their firmware
> +manages job scheduling internally. The kernel driver submits bitstream data
> +but has no visibility into per-frame hardware execution timing.
> +
> +Implementation
> +==============
> +
> +The V4L2 core provides the plumbing: drivers implement the ``show_fdinfo``
> +callback in ``struct v4l2_file_operations``, and the core wires it into the
> +kernel ``struct file_operations`` so that ``/proc/<pid>/fdinfo/<fd>`` output
> +includes the driver-provided keys.
> +
> +File format specification
> +=========================
> +
> +- File shall contain one key value pair per one line of text.
> +- Colon character (``:``) must be used to delimit keys and values.
> +- All standardised keys shall be prefixed with ``media-``.
> +- Driver-specific keys shall be prefixed with ``driver_name-``.
> +
> +Mandatory keys
> +--------------
> +
> +- media-driver: <valstr>

I'd pick 'v4l2-driver'. Since 'media-driver' is too generic for this
since that encompasses also DVB/CEC/RC drivers.

> +
> +  String shall contain the name of the media driver.

'V4L2 stateless codec driver'

> +
> +- media-type: <valstr>

Poor name.

> +
> +  String shall identify the type of media engine exposed through this file
> +  descriptor. Standard values are ``decoder`` and ``encoder``.

I think I would use 'stateless-decoder' and 'stateless-encoder'. It's more
specific than de/encoder since that can be stateful as well.

So I am missing the big picture here: right now this patch adds support for
this for stateless codecs, but what happens in the future if this is also
added for regular video capture devices, ISPs, etc.?

The naming here matters, it has to have some sort of scheme so it can be
extended to other types of drivers. So a 'media' prefix is too generic,
and it also looks like it refers to the /dev/mediaX device.

> +
> +Utilization keys
> +----------------
> +
> +- media-engine-usage: <uint> ns

'Media Engine': very vague.

> +
> +  Time in nanoseconds that the hardware engine spent busy processing work

Cumulative since creating the file handle? Or since the last read?

> +  belonging to this file descriptor. The engine being measured is identified
> +  by the ``media-type`` key.
> +
> +  Values are not required to be constantly monotonic if it makes the driver
> +  implementation easier, but are required to catch up with the previously
> +  reported larger value within a reasonable period.

Does this make sense for codecs?

I can tell that this is heavily influenced by the drm documentation, but that
does not necessarily translate to V4L2.

> +
> +Frequency keys
> +--------------
> +
> +- media-maxfreq: <uint> Hz
> +
> +  Maximum operating frequency of the main engine clock.
> +
> +- media-curfreq: <uint> Hz
> +
> +  Current operating frequency of the main engine clock.

'Main engine clock'?

> +
> +Example output
> +==============
> +
> +::
> +
> +  media-driver:           hantro-vpu
> +  media-type:             decoder
> +  media-engine-usage:     123456789 ns
> +  media-maxfreq:          600000000 Hz
> +  media-curfreq:          600000000 Hz
> 

Regards,

	Hans


  reply	other threads:[~2026-06-19 12:58 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-17 18:10 [PATCH v2 0/5] media: Add fdinfo support for v4l2 drivers Detlev Casanova
2026-06-17 18:10 ` [PATCH v2 1/5] media: v4l2: Add callback for show_fdinfo Detlev Casanova
2026-06-17 18:10 ` [PATCH v2 2/5] docs: media: add documentation for media client usage stats Detlev Casanova
2026-06-19 12:58   ` Hans Verkuil [this message]
2026-06-19 14:04     ` Nicolas Dufresne
2026-06-19 13:06   ` Mauro Carvalho Chehab
2026-06-17 18:10 ` [PATCH v2 3/5] media: v4l2-core: Add v4l2-stats interface Detlev Casanova
2026-06-19 13:05   ` Hans Verkuil
2026-06-17 18:10 ` [PATCH v2 4/5] media: hantro: add per-context fdinfo usage stats Detlev Casanova
2026-06-17 18:11 ` [PATCH v2 5/5] media: rkvdec: Add " Detlev Casanova

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=755cf7c1-6bcc-45d1-afea-192d393256af@kernel.org \
    --to=hverkuil+cisco@kernel.org \
    --cc=benjamin.gaignard@collabora.com \
    --cc=detlev.casanova@collabora.com \
    --cc=ezequiel@vanguardiasur.com.ar \
    --cc=healych@amazon.com \
    --cc=heiko@sntech.de \
    --cc=kernel@collabora.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=mchehab@kernel.org \
    --cc=nicolas.dufresne@collabora.com \
    --cc=p.zabel@pengutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox