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

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 20/01/2022 16:44, Rob Clark wrote:
> On Wed, Jan 19, 2022 at 7:09 AM Daniel Vetter <daniel@ffwll.ch> wrote:
>>
>> On Thu, Jan 06, 2022 at 04:55:35PM +0000, Tvrtko Ursulin wrote:
>>> From: Tvrtko Ursulin <tvrtko.ursulin@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@intel.com>
>>> Cc: David M Nieto <David.Nieto@amd.com>
>>> Cc: Christian König <christian.koenig@amd.com>
>>> Cc: Daniel Vetter <daniel@ffwll.ch>
>>> Cc: Daniel Stone <daniel@fooishbar.org>
>>> Cc: Chris Healy <cphealy@gmail.com>
>>> Acked-by: Christian König <christian.koenig@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.

Interesting tidbit is that the whole i915 work started from a customer 
request to expose just that (per context) in a form akin to 
getrusage(2). I think this kind of introspection capability is 
interesting but as it is driver specific territory it's only anecdotal 
for what this thread is concerned.

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

I have a patch to Intel specific intel_gpu_top (see 
https://patchwork.freedesktop.org/patch/468491/?series=98555&rev=1). 
I'll have a look to see how much work would it be to extract common bits 
into a library and write a quick agnostic tool using it.

Regards,

Tvrtko