Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Nicolas Dufresne <nicolas.dufresne@collabora.com>
To: Hans Verkuil <hverkuil+cisco@kernel.org>,
	Detlev Casanova	 <detlev.casanova@collabora.com>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	 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 10:04:42 -0400	[thread overview]
Message-ID: <0449f2bf75685e17034ada5ae3961518ceb04345.camel@collabora.com> (raw)
In-Reply-To: <755cf7c1-6bcc-45d1-afea-192d393256af@kernel.org>

[-- Attachment #1: Type: text/plain, Size: 9829 bytes --]

Hi,

Le vendredi 19 juin 2026 à 14:58 +0200, Hans Verkuil a écrit :
> 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.

While it is currently implemented for sateless m2m codec drivers as example
(because we can't implement this everywhere at once really, its not practical),
there is no reason why we cannot track to a process fdinfo memory usage
associated with other video devices. In fact, I would pretty much want to see
capture devices showing up in v4l2top, as these are using a lot of memory too,
specially the new camera pipelines. And this is also perfectly suitable for
stateful codec, converters, everything.

So please, let's agree this is not "stateless V4L2 codec drivers" specific.

About VIDIOC_LOG_STATUS, this is effectively broken for m2m, only the owning
session process could call that. Its also pretty bad for tools to have to
monitor the dmesg and filter it up. fdinfo on the other end, does not require
opening the device and permissions are very clear, and bound to user process.

> 
> > +==========================
> > +
> > +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'

With that said, we can refine, but not in that direction.

> 
> > +
> > +- 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.?

We should work further on the type system, then we'd simply have to extend it.
Different type of driver will provide different key/value pair. We should well
document which key are mandatory for the type (or for all type, like giving a
name for general identification purpose).

Of course, the current proposal is very torward processing cores, hence the one
to one match with what GPUs present in fdinfo today in mainline, such are
frequency, core utilization time, etc.

Though, DMA devices such as capture may expose more of a bandwidth kind of
information. I really don't want to make up too may cases, so owners of these
don't feel like this is mandated though. As we develop software using our
drivers, we should be able to figure-out what kind of things are important to
monitor. For CODECs, we have two majors things. a) core utilization (which can
be with a timer like this one, but ideally using cycle counters, or firmware
provided data). b) memory usage.

> 
> 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.

I would guess this is bound to usage of cycle counter, which we definitely have
in some codec HW exposing. As the frequency fluctuate, I suppose there is ways
you can get your calculation a bit off, as this is being sampled. Basically, the
frequence change is not strictly tracked by our drivers, which is ok. But I also
don't seen why we wouldn't keep the counters monotonic, its really easy to do.

> 
> > +
> > +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'?

The word "engine" seems indeed alien in this subsystem. It is referring to a
system where you open one devices, so you have one fdinfo, but multiple cores.
Each cores may be running at it owns frequency. But I think this is a little too
vague to my taste.

Hope this feedback does not cross to many wires, but tagging this as specific to
stateless codec drivers did spark a little in my head. And we are really in need
for a way to monitor our drivers.

Nicolas

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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

  reply	other threads:[~2026-06-19 14:05 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
2026-06-19 14:04     ` Nicolas Dufresne [this message]
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=0449f2bf75685e17034ada5ae3961518ceb04345.camel@collabora.com \
    --to=nicolas.dufresne@collabora.com \
    --cc=benjamin.gaignard@collabora.com \
    --cc=detlev.casanova@collabora.com \
    --cc=ezequiel@vanguardiasur.com.ar \
    --cc=healych@amazon.com \
    --cc=heiko@sntech.de \
    --cc=hverkuil+cisco@kernel.org \
    --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=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