On Wed, Jan 19, 2022 at 7:09 AM Daniel Vetter <daniel@xxxxxxxx> wrote: > > On Thu, Jan 06, 2022 at 04:55:35PM +0000, Tvrtko Ursulin wrote: > > From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> > > > > 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@xxxxxxxxx> > > Cc: David M Nieto <David.Nieto@xxxxxxx> > > Cc: Christian König <christian.koenig@xxxxxxx> > > Cc: Daniel Vetter <daniel@xxxxxxxx> > > Cc: Daniel Stone <daniel@xxxxxxxxxxxxx> > > Cc: Chris Healy <cphealy@xxxxxxxxx> > > Acked-by: Christian König <christian.koenig@xxxxxxx> > > 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