Re: [PATCH v4 2/2] Documentation/gpu: Add fdinfo meanings of drm-*-internal memory tags

[Mihail Atanassov <mihail.atanassov@arm.com>]

On 11/12/2024 17:02, Adrián Larumbe wrote:
> Hi Mihail,
> 
> On 11.12.2024 16:40, Mihail Atanassov wrote:
>> Hi Adrián,
>>
>> On 11/12/2024 16:34, Adrián Larumbe wrote:
>>> 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@collabora.com>
>>> ---
>>>    Documentation/gpu/panthor.rst | 14 ++++++++++++++
>>>    1 file changed, 14 insertions(+)
>>>
>>> diff --git a/Documentation/gpu/panthor.rst b/Documentation/gpu/panthor.rst
>>> index 3f8979fa2b86..c6d8236e3665 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
>>
>> You give an example of `drm-shared-internal`...
>>
>>> +     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` key names are: `total`, `active` and `resident`.
>>
>> ... but don't list it as a valid key name here.
> 
> I do mention slightly further below that that key:value pair is at the time being unused,
> but I've thought of a possible interpretation that could be part of another commit.

Understood, it just looks weird reading the paragraph after the context 
above. Seeing as `drm_print_memory_stats` will always emit it, it stands 
to reason it's a valid key name, just with no assigned meaning to it 
(yet). I'm being nit-picky :). Feel free to add my R-b with or without 
the extra key in the list.

> 
>>> +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-memory` is unused at present, but in the future it might stand for
>>> +the size of the Firmware regions, since they do not belong to an open file context.
>>
>> -- 
>> Mihail Atanassov <mihail.atanassov@arm.com>
> 
> Adrian Larumbe

-- 
Mihail Atanassov <mihail.atanassov@arm.com>