On 02.01.2025 21:59, Tvrtko Ursulin wrote: > >On 18/12/2024 18:18, Adrián Martínez Larumbe wrote: >> From: Adrián Larumbe <adrian.larumbe@xxxxxxxxxxxxx> >> >> A previous commit enabled display of driver-internal kernel BO sizes >> through the device file's fdinfo interface. >> >> Expand the description of the relevant driver-specific key:value pairs >> with the definitions of the new drm-*-internal ones. >> >> Signed-off-by: Adrián Larumbe <adrian.larumbe@xxxxxxxxxxxxx> >> Reviewed-by: Mihail Atanassov <mihail.atanassov@xxxxxxx> >> --- >> Documentation/gpu/panthor.rst | 14 ++++++++++++++ >> 1 file changed, 14 insertions(+) >> >> diff --git a/Documentation/gpu/panthor.rst b/Documentation/gpu/panthor.rst >> index 3f8979fa2b86..23aa3d67c9d2 100644 >> --- a/Documentation/gpu/panthor.rst >> +++ b/Documentation/gpu/panthor.rst >> @@ -26,6 +26,10 @@ the currently possible format options: >> drm-cycles-panthor: 94439687187 >> drm-maxfreq-panthor: 1000000000 Hz >> drm-curfreq-panthor: 1000000000 Hz >> + drm-total-internal: 10396 KiB >> + drm-shared-internal: 0 >> + drm-active-internal: 10396 KiB >> + drm-resident-internal: 10396 KiB >> drm-total-memory: 16480 KiB >> drm-shared-memory: 0 >> drm-active-memory: 16200 KiB >> @@ -44,3 +48,13 @@ driver by writing into the appropriate sysfs node:: >> Where `N` is a bit mask where cycle and timestamp sampling are respectively >> enabled by the first and second bits. >> + >> +Possible `drm-*-internal` keys are: `total`, `active`, `resident` and `shared`. >> +These values convey the sizes of the internal driver-owned shmem BO's that >> +aren't exposed to user-space through a DRM handle, like queue ring buffers, >> +sync object arrays and heap chunks. Because they are all allocated and pinned >> +at creation time, `drm-resident-internal` and `drm-total-internal` should always >> +be equal. `drm-active-internal` shows the size of kernel BO's associated with >> +VM's and groups currently being scheduled for execution by the GPU. >> +`drm-shared-internal` is unused at present, but in the future it might stand for >> +the size of executable FW regions, since they do not belong to an open file context. > >The description is way too specific, too tied to some of the implementations. These are panthor-specific key:value pairs. I was in the belief that drivers could define their own when it suits their interest beyond the DRM-wide ones defined in the drm-fdinfo spec. >I also don't remember that you ever explained why totting up the internal >objects into existing regions isn't good enough. I keep asking, you keep not >explaining. Or I missed your emails somehow. It's not that it's not good enough, but rather that it cannot be done in the current state of affairs. drm_show_memory_stats() defines its own drm_memory_stats struct as an automatic variable so we don't have access to it from anywhere else in the driver. In a previous revision of the patch series I had come up with a workaround that would let drivers pass a function pointer to drm_show_memory_stats() which would gather those numbers in a driver-specific way, but it didn't seem to get any traction. >And you keep not copying me on the thread. Copying people who expressed >interest, gave past feedback, etc should be the norm. I did not CC you on this series because these are all panthor-specific changes which do not touch on any DRM fdinfo-wide code, and also because I didn't think that driver-specific key:value pairs needed the approval of the drm-fdinfo core maintainers. >Until we can clarify the above points I don't think this can go in. > >Regards, > >Tvrtko Adrian Larumbe