[PATCH i-g-t] lib/igt_drm_fdinfo: Handle amdgpu memory stats

[Lucas De Marchi]

On Fri, May 03, 2024 at 02:53:20PM GMT, Tvrtko Ursulin wrote:
>
>On 03/05/2024 14:32, Lucas De Marchi wrote:
>>On Fri, May 03, 2024 at 01:37:31PM GMT, Tvrtko Ursulin wrote:
>>>From: Tvrtko Ursulin <tvrtko.ursulin at igalia.com>
>>>
>>>Code so far only handles the clients using the common DRM helper.
>>>
>>>Handle the amdgpu driver which uses a slightly different set of 
>>>keys. More
>>>specifically, outputs drm-memory-<region> instead of drm-total-<region>.
>>>
>>>With this added gputop starts showing total memory usage for amdgpu.
>>>
>>>Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at igalia.com>
>>>Cc: Alex Deucher <alexander.deucher at amd.com>
>>>Cc: Christian König <christian.keonig at amd.com>
>>>Cc: Rob Clark <robdclark at chromium.org>
>>>Cc: Lucas De Marchi <lucas.demarchi at intel.com>
>>>---
>>>lib/igt_drm_fdinfo.c | 4 ++++
>>>1 file changed, 4 insertions(+)
>>>
>>>diff --git a/lib/igt_drm_fdinfo.c b/lib/igt_drm_fdinfo.c
>>>index cab677df29a9..89992f1477ec 100644
>>>--- a/lib/igt_drm_fdinfo.c
>>>+++ b/lib/igt_drm_fdinfo.c
>>>@@ -262,6 +262,10 @@ __igt_parse_drm_fdinfo(int dir, const char 
>>>*fd, struct drm_client_fdinfo *info,
>>>            idx = parse_region(l, info, strlen("drm-total-"),
>>>                       region_map, region_entries, &val);
>>>            UPDATE_REGION(idx, total, val);
>>>+        } else if (!strncmp(l, "drm-memory-", 11)) {
>>>+            idx = parse_region(l, info, strlen("drm-memory-"),
>>>+                       region_map, region_entries, &val);
>>>+            UPDATE_REGION(idx, total, val);
>>
>>are they the same thing? Should we clarify the doc that a driver can't
>>use both drm-memory- and drm-total-?
>>
>>     - drm-memory-<region>: <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.  The name 
>>"memory" is reserved to refer to normal system memory.
>>     Value shall reflect the amount of storage currently consumed by 
>>the buffer     objects 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.
>>     - drm-total-<region>: <uint> [KiB|MiB]
>>
>>     The total size of buffers that including shared and private memory.
>>
>>If they are the same and the total field can just be re-used,
>
>Yep exactly, they are the same in practice so we are lucky that we are 
>able to fix it still. See: https://lore.kernel.org/amd-gfx/CADnq5_NAO_Ao0EJTO=MJxvR-KJkF1WCwKGV-9ami7qQdzf029w@mail.gmail.com/T/#t 
>(I forgot to CC dri-devel when posting the patch.)
>
>>
>>
>>     Reviewed-by: Lucas De Marchi <lucas.demarchi at intel.com>
>
>Thanks! Are you okay if I put this in first since it collides with 
>your fdinfo rework?

yes, should be easy to update whatever lands first.

Lucas De Marchi

>
>Although I will wait for the drm-usage-stats.rst patch to get approved 
>first.
>
>Regards,
>
>Tvrtko
>
>>Lucas De Marchi
>>
>>>        } else if (!strncmp(l, "drm-shared-", 11)) {
>>>            idx = parse_region(l, info, strlen("drm-shared-"),
>>>                       region_map, region_entries, &val);
>>>-- 
>>>2.44.0
>>>