[Intel-gfx] [PATCH 6/7] drm: Document fdinfo format specification

[Rob Clark]

On Wed, Jan 19, 2022 at 7:09 AM Daniel Vetter <daniel at ffwll.ch> wrote:
>
> On Thu, Jan 06, 2022 at 04:55:35PM +0000, Tvrtko Ursulin wrote:
> > From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> >
> > Proposal to standardise the fdinfo text format as optionally output by DRM
> > drivers.
> >
> > Idea is that a simple but, well defined, spec will enable generic
> > userspace tools to be written while at the same time avoiding a more heavy
> > handed approach of adding a mid-layer to DRM.
> >
> > i915 implements a subset of the spec, everything apart from the memory
> > stats currently, and a matching intel_gpu_top tool exists.
> >
> > Open is to see if AMD can migrate to using the proposed GPU utilisation
> > key-value pairs, or if they are not workable to see whether to go
> > vendor specific, or if a standardised  alternative can be found which is
> > workable for both drivers.
> >
> > Same for the memory utilisation key-value pairs proposal.
> >
> > v2:
> >  * Update for removal of name and pid.
> >
> > v3:
> >  * 'Drm-driver' tag will be obtained from struct drm_driver.name. (Daniel)
> >
> > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> > Cc: David M Nieto <David.Nieto at amd.com>
> > Cc: Christian König <christian.koenig at amd.com>
> > Cc: Daniel Vetter <daniel at ffwll.ch>
> > Cc: Daniel Stone <daniel at fooishbar.org>
> > Cc: Chris Healy <cphealy at gmail.com>
> > Acked-by: Christian König <christian.koenig at amd.com>
>
> I'm assuming this ack here and later on is a "amdgpu plans to use this
> too" kind of affair. Especially also in the lights of eventually using
> matching semantics for cgroups and everything else tied to gpu execution
> resource management.
>
> If not I'm mildly worried that we're creating fake-standard stuff here
> which cannot actually be used by anything resembling driver-agnostic
> userspace.

I think I could implement something like this for drm/msm.  I am a bit
uncertain about the memory stats (ie. how are we intended to account
for imported/exported/shared bo's)?  But we already track cycles+time
per submit for devfreq, it would be pretty easy to add per drm_file
counters to accumulate the per-submit results.  We could even track
per-context (submitqueue) for processes that have more than a single
context, although not sure if that is useful.

And I think there is probably some room for shared helper to print
parts other than the per-engine stats (and maybe memory stats,
although even that could be a shared implementation for some
drivers).. with a driver callback for the non-generic parts, ie.
something like:

   drm_driver::show_client_stats(struct drm_file *, struct drm_printer *)

but that can come later.

If there is a tool somewhere that displays this info, that would be
useful for testing my implementation.

BR,
-R

> -Daniel
>
> > ---
> >  Documentation/gpu/drm-usage-stats.rst | 97 +++++++++++++++++++++++++++
> >  Documentation/gpu/index.rst           |  1 +
> >  2 files changed, 98 insertions(+)
> >  create mode 100644 Documentation/gpu/drm-usage-stats.rst
> >
> > diff --git a/Documentation/gpu/drm-usage-stats.rst b/Documentation/gpu/drm-usage-stats.rst
> > new file mode 100644
> > index 000000000000..c669026be244
> > --- /dev/null
> > +++ b/Documentation/gpu/drm-usage-stats.rst
> > @@ -0,0 +1,97 @@
> > +.. _drm-client-usage-stats:
> > +
> > +======================
> > +DRM client usage stats
> > +======================
> > +
> > +DRM drivers can choose to export partly standardised text output via the
> > +`fops->show_fdinfo()` as part of the driver specific file operations registered
> > +in the `struct drm_driver` object registered with the DRM core.
> > +
> > +One purpose of this output is to enable writing as generic as practicaly
> > +feasible `top(1)` like userspace monitoring tools.
> > +
> > +Given the differences between various DRM drivers the specification of the
> > +output is split between common and driver specific parts. Having said that,
> > +wherever possible effort should still be made to standardise as much as
> > +possible.
> > +
> > +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 keys shall be prefixed with `drm-`.
> > +- Whitespace between the delimiter and first non-whitespace character shall be
> > +  ignored when parsing.
> > +- Neither keys or values are allowed to contain whitespace characters.
> > +- Numerical key value pairs can end with optional unit string.
> > +- Data type of the value is fixed as defined in the specification.
> > +
> > +Key types
> > +---------
> > +
> > +1. Mandatory, fully standardised.
> > +2. Optional, fully standardised.
> > +3. Driver specific.
> > +
> > +Data types
> > +----------
> > +
> > +- <uint> - Unsigned integer without defining the maximum value.
> > +- <str> - String excluding any above defined reserved characters or whitespace.
> > +
> > +Mandatory fully standardised keys
> > +---------------------------------
> > +
> > +- drm-driver: <str>
> > +
> > +String shall contain the name this driver registered as via the respective
> > +`struct drm_driver` data structure.
> > +
> > +Optional fully standardised keys
> > +--------------------------------
> > +
> > +- drm-pdev: <aaaa:bb.cc.d>
> > +
> > +For PCI devices this should contain the PCI slot address of the device in
> > +question.
> > +
> > +- drm-client-id: <uint>
> > +
> > +Unique value relating to the open DRM file descriptor used to distinguish
> > +duplicated and shared file descriptors. Conceptually the value should map 1:1
> > +to the in kernel representation of `struct drm_file` instances.
> > +
> > +Uniqueness of the value shall be either globally unique, or unique within the
> > +scope of each device, in which case `drm-pdev` shall be present as well.
> > +
> > +Userspace should make sure to not double account any usage statistics by using
> > +the above described criteria in order to associate data to individual clients.
> > +
> > +- drm-engine-<str>: <uint> ns
> > +
> > +GPUs usually contain multiple execution engines. Each shall be given a stable
> > +and unique name (str), with possible values documented in the driver specific
> > +documentation.
> > +
> > +Value shall be in specified time units which the respective GPU engine spent
> > +busy executing workloads belonging to this client.
> > +
> > +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. Upon observing a value lower than what
> > +was previously read, userspace is expected to stay with that larger previous
> > +value until a monotonic update is seen.
> > +
> > +- drm-memory-<str>: <uint> [KiB|MiB]
> > +
> > +Each possible memory type which can be used to store buffer objects by the
> > +GPU in question shall be given a stable and unique name to be returned as the
> > +string here.
> > +
> > +Value shall reflect the amount of storage currently consumed by the buffer
> > +object belong to this client, in the respective memory region.
> > +
> > +Default unit shall be bytes with optional unit specifiers of 'KiB' or 'MiB'
> > +indicating kibi- or mebi-bytes.
> > diff --git a/Documentation/gpu/index.rst b/Documentation/gpu/index.rst
> > index b9c1214d8f23..b99dede9a5b1 100644
> > --- a/Documentation/gpu/index.rst
> > +++ b/Documentation/gpu/index.rst
> > @@ -10,6 +10,7 @@ Linux GPU Driver Developer's Guide
> >     drm-kms
> >     drm-kms-helpers
> >     drm-uapi
> > +   drm-usage-stats
> >     driver-uapi
> >     drm-client
> >     drivers
> > --
> > 2.32.0
> >
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch