On Wed, Sep 4, 2024 at 4:36 AM Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxx> wrote: > > > On 21/08/2024 21:47, Alex Deucher wrote: > > On Tue, Aug 13, 2024 at 9:57 AM Tvrtko Ursulin <tursulin@xxxxxxxxxx> wrote: > >> > >> From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxx> > >> > >> Currently it is not well defined what is drm-memory- compared to other > >> categories. > >> > >> In practice the only driver which emits these keys is amdgpu and in them > >> exposes the current resident buffer object memory (including shared). > >> > >> To prevent any confusion, document that drm-memory- is deprecated and an > >> alias for drm-resident-memory-. > >> > >> While at it also clarify that the reserved sub-string 'memory' refers to > >> the memory region component, and also clarify the intended semantics of > >> other memory categories. > >> > >> v2: > >> * Also mark drm-memory- as deprecated. > >> * Add some more text describing memory categories. (Alex) > >> > >> v3: > >> * Semantics of the amdgpu drm-memory is actually as drm-resident. > >> > >> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxx> > >> Cc: Alex Deucher <alexander.deucher@xxxxxxx> > >> Cc: Christian König <christian.keonig@xxxxxxx> > >> Cc: Rob Clark <robdclark@xxxxxxxxxxxx> > >> --- > >> Documentation/gpu/drm-usage-stats.rst | 25 ++++++++++++++++++++++--- > >> 1 file changed, 22 insertions(+), 3 deletions(-) > >> > >> diff --git a/Documentation/gpu/drm-usage-stats.rst b/Documentation/gpu/drm-usage-stats.rst > >> index a80f95ca1b2f..ff964c707754 100644 > >> --- a/Documentation/gpu/drm-usage-stats.rst > >> +++ b/Documentation/gpu/drm-usage-stats.rst > >> @@ -144,7 +144,9 @@ Memory > >> > >> 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. > >> +string here. > >> + > >> +The region 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. > >> @@ -152,6 +154,9 @@ 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. > >> > >> +This key is deprecated and is an alias for drm-resident-<region>. Only one of > >> +the two should be present in the output. > >> + > > > > I'm not sure how best to handle this. What should amdgpu do? We have > > customers out in the field using these existing fields and then with > > patch 2, they go away. Arguably we'd want both for backwards > > compatibility. > > Exactly, so it looks you maybe missed that 2/2 is not removing the > amdgpu "legacy" drm-memory-. It keeps outputting it and also duplicating > under drm-resident-. This is mentioned in the commit paragraph: > > """ > Legacy keys have been preserved, with the outlook of only potentially > removing only the drm-memory- when the time gets right. > """ > > Put differently, I don't think 2/2 should break the existing > tools/parsers. Only if they have hardcoded assumptions about the order > of keys perhaps? You're right. I totally missed that part. The series is: Reviewed-by: Alex Deucher <alexander.deucher@xxxxxxx> Happy to take the patches through my tree or patch 2 via mine and patch 1 via drm-misc. Alex > > Regards, > > Tvrtko > > >> - drm-shared-<region>: <uint> [KiB|MiB] > >> > >> The total size of buffers that are shared with another file (e.g., have more > >> @@ -159,20 +164,34 @@ than a single handle). > >> > >> - drm-total-<region>: <uint> [KiB|MiB] > >> > >> -The total size of buffers that including shared and private memory. > >> +The total size of all created buffers including shared and private memory. The > >> +backing store for the buffers does not have to be currently instantiated to be > >> +counted under this category. > >> > >> - drm-resident-<region>: <uint> [KiB|MiB] > >> > >> -The total size of buffers that are resident in the specified region. > >> +The total size of buffers that are resident (have their backing store present or > >> +instantiated) in the specified region. > >> + > >> +This is an alias for drm-memory-<region> and only one of the two should be > >> +present in the output. > >> > >> - drm-purgeable-<region>: <uint> [KiB|MiB] > >> > >> The total size of buffers that are purgeable. > >> > >> +For example drivers which implement a form of 'madvise' like functionality can > >> +here count buffers which have instantiated backing store, but have been marked > >> +with an equivalent of MADV_DONTNEED. > >> + > >> - drm-active-<region>: <uint> [KiB|MiB] > >> > >> The total size of buffers that are active on one or more engines. > >> > >> +One practical example of this can be presence of unsignaled fences in an GEM > >> +buffer reservation object. Therefore the active category is a subset of > >> +resident. > >> + > >> Implementation Details > >> ====================== > >> > >> -- > >> 2.44.0 > >>
