diff mbox series

[v8,3/5] Documentation/gpu: Clarify drm memory stats definition

Message ID 20241116044452.5925-4-Yunxiang.Li@amd.com (mailing list archive)
State New
Headers show
Series [v8,1/5] drm: add drm_memory_stats_is_zero | expand

Commit Message

Yunxiang Li Nov. 16, 2024, 4:44 a.m. UTC
Define how to handle buffers with multiple possible placement so we
don't get incompatible implementations. Callout the resident requirement
for drm-purgeable- explicitly. Remove the requirement for there to be
only drm-memory- or only drm-resident-, it's not what's implemented and
having both is better for back-compat. Also re-order the paragraphs to
flow better.

Signed-off-by: Yunxiang Li <Yunxiang.Li@amd.com>
CC: dri-devel@lists.freedesktop.org
---
 Documentation/gpu/drm-usage-stats.rst | 36 ++++++++++++---------------
 1 file changed, 16 insertions(+), 20 deletions(-)

Comments

Christian König Nov. 18, 2024, 2:03 p.m. UTC | #1
Am 16.11.24 um 05:44 schrieb Yunxiang Li:
> Define how to handle buffers with multiple possible placement so we
> don't get incompatible implementations. Callout the resident requirement
> for drm-purgeable- explicitly. Remove the requirement for there to be
> only drm-memory- or only drm-resident-, it's not what's implemented and
> having both is better for back-compat. Also re-order the paragraphs to
> flow better.
>
> Signed-off-by: Yunxiang Li <Yunxiang.Li@amd.com>
> CC: dri-devel@lists.freedesktop.org
> ---
>   Documentation/gpu/drm-usage-stats.rst | 36 ++++++++++++---------------
>   1 file changed, 16 insertions(+), 20 deletions(-)
>
> diff --git a/Documentation/gpu/drm-usage-stats.rst b/Documentation/gpu/drm-usage-stats.rst
> index ff964c707754a..973663f91a292 100644
> --- a/Documentation/gpu/drm-usage-stats.rst
> +++ b/Documentation/gpu/drm-usage-stats.rst
> @@ -140,13 +140,9 @@ both.
>   Memory
>   ^^^^^^
>   
> -- 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 region name "memory" is reserved to refer to normal system 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 used as the "<region>"
> +string. The region name "memory" is reserved to refer to normal system memory.

That looks like you squashed the "The region name..." sentence at the 
end. Is that really helpful and intended?

>   
>   Value shall reflect the amount of storage currently consumed by the buffer
>   objects belong to this client, in the respective memory region.
> @@ -154,31 +150,27 @@ 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.
> +- drm-total-<region>: <uint> [KiB|MiB]
> +
> +The total size of all created buffers including shared and private memory. The

Maybe write "requested" instead of "created" since without a backing 
store it is questionable if the BO is really "created".

Apart from those two nit picks it looks good to me,
Christian.

> +backing store for the buffers does not have to be currently instantiated to
> +count under this category. To avoid double counting, if a buffer falls under
> +multiple regions, the implementation should pick only one of the regions, and do
> +so in a consistent manner.
>   
>   - drm-shared-<region>: <uint> [KiB|MiB]
>   
>   The total size of buffers that are shared with another file (e.g., have more
> -than a single handle).
> -
> -- drm-total-<region>: <uint> [KiB|MiB]
> -
> -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.
> +than a single handle). Same caveat as drm-total- applies.
>   
>   - drm-resident-<region>: <uint> [KiB|MiB]
>   
>   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.
> +The total size of buffers that are resident and 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
> @@ -192,6 +184,10 @@ 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.
>   
> +- drm-memory-<region>: <uint> [KiB|MiB]
> +
> +This key is deprecated and is an alias for drm-resident-<region> if present.
> +
>   Implementation Details
>   ======================
>
Tvrtko Ursulin Nov. 18, 2024, 2:38 p.m. UTC | #2
On 16/11/2024 04:44, Yunxiang Li wrote:
> Define how to handle buffers with multiple possible placement so we
> don't get incompatible implementations. Callout the resident requirement
> for drm-purgeable- explicitly. Remove the requirement for there to be
> only drm-memory- or only drm-resident-, it's not what's implemented and
> having both is better for back-compat. Also re-order the paragraphs to
> flow better.
> 
> Signed-off-by: Yunxiang Li <Yunxiang.Li@amd.com>
> CC: dri-devel@lists.freedesktop.org
> ---
>   Documentation/gpu/drm-usage-stats.rst | 36 ++++++++++++---------------
>   1 file changed, 16 insertions(+), 20 deletions(-)
> 
> diff --git a/Documentation/gpu/drm-usage-stats.rst b/Documentation/gpu/drm-usage-stats.rst
> index ff964c707754a..973663f91a292 100644
> --- a/Documentation/gpu/drm-usage-stats.rst
> +++ b/Documentation/gpu/drm-usage-stats.rst
> @@ -140,13 +140,9 @@ both.
>   Memory
>   ^^^^^^
>   
> -- 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 region name "memory" is reserved to refer to normal system 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 used as the "<region>"
> +string. 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.
> @@ -154,31 +150,27 @@ 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.

IMO the second sentence should stay in principle (I mean at the new 
location, where you moved it). Intent is to avoid new implementations 
adding both keys. The fact amdgpu has both is not relevant for that 
purpose. We don't want someone just reading it is an alias and having to 
have any doubt whether they need to output both or not.

> +- drm-total-<region>: <uint> [KiB|MiB]
> +
> +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
> +count under this category. To avoid double counting, if a buffer falls under
> +multiple regions, the implementation should pick only one of the regions, and do
> +so in a consistent manner.

Addition feels fine to me in principle. I would only suggest rewording 
it a bit to avoid ambiguity about what it means to "fall under". Perhaps 
like this:

To avoid double counting when buffers can be instantiated in one of the 
multiple allowed memory regions, the implementation should account the 
total against only one of the regions, and should pick this region in a 
consistent manner.

>   
>   - drm-shared-<region>: <uint> [KiB|MiB]
>   
>   The total size of buffers that are shared with another file (e.g., have more
> -than a single handle).
> -
> -- drm-total-<region>: <uint> [KiB|MiB]
> -
> -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.
> +than a single handle). Same caveat as drm-total- applies.

I suggest to explicitly point out the caveat is the multiple region one.

>   
>   - drm-resident-<region>: <uint> [KiB|MiB]
>   
>   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.

I think it does not harm to keep this note at both keys. Or at least 
make one reference the other for this point specifically.

> -
>   - drm-purgeable-<region>: <uint> [KiB|MiB]
>   
> -The total size of buffers that are purgeable.
> +The total size of buffers that are resident and purgeable.

Is it not redundant? How could something not resident be purgeable in 
the first place?
>   For example drivers which implement a form of 'madvise' like functionality can
>   here count buffers which have instantiated backing store, but have been marked
> @@ -192,6 +184,10 @@ 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.
>   
> +- drm-memory-<region>: <uint> [KiB|MiB]
> +
> +This key is deprecated and is an alias for drm-resident-<region> if present.
> +
>   Implementation Details
>   ======================
>   

Regards,

Tvrtko
Tvrtko Ursulin Nov. 18, 2024, 2:47 p.m. UTC | #3
On 18/11/2024 14:03, Christian König wrote:
> Am 16.11.24 um 05:44 schrieb Yunxiang Li:
>> Define how to handle buffers with multiple possible placement so we
>> don't get incompatible implementations. Callout the resident requirement
>> for drm-purgeable- explicitly. Remove the requirement for there to be
>> only drm-memory- or only drm-resident-, it's not what's implemented and
>> having both is better for back-compat. Also re-order the paragraphs to
>> flow better.
>>
>> Signed-off-by: Yunxiang Li <Yunxiang.Li@amd.com>
>> CC: dri-devel@lists.freedesktop.org
>> ---
>>   Documentation/gpu/drm-usage-stats.rst | 36 ++++++++++++---------------
>>   1 file changed, 16 insertions(+), 20 deletions(-)
>>
>> diff --git a/Documentation/gpu/drm-usage-stats.rst 
>> b/Documentation/gpu/drm-usage-stats.rst
>> index ff964c707754a..973663f91a292 100644
>> --- a/Documentation/gpu/drm-usage-stats.rst
>> +++ b/Documentation/gpu/drm-usage-stats.rst
>> @@ -140,13 +140,9 @@ both.
>>   Memory
>>   ^^^^^^
>> -- 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 region name "memory" is reserved to refer to normal system 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 used as the 
>> "<region>"
>> +string. The region name "memory" is reserved to refer to normal 
>> system memory.
> 
> That looks like you squashed the "The region name..." sentence at the 
> end. Is that really helpful and intended?
> 
>>   Value shall reflect the amount of storage currently consumed by the 
>> buffer
>>   objects belong to this client, in the respective memory region.
>> @@ -154,31 +150,27 @@ 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.
>> +- drm-total-<region>: <uint> [KiB|MiB]
>> +
>> +The total size of all created buffers including shared and private 
>> memory. The
> 
> Maybe write "requested" instead of "created" since without a backing 
> store it is questionable if the BO is really "created".

Hmm is the term "requested" in colloquial use either by end users or 
user space developers in the context of buffer objects?

If we think the sentence needs to be improved upon, maybe something 
like: "The total size of all buffers, either created by user space APIs, 
or internally by the driver, including both shared and private buffers."

?

Regards,

Tvrtko

> 
> Apart from those two nit picks it looks good to me,
> Christian.
> 
>> +backing store for the buffers does not have to be currently 
>> instantiated to
>> +count under this category. To avoid double counting, if a buffer 
>> falls under
>> +multiple regions, the implementation should pick only one of the 
>> regions, and do
>> +so in a consistent manner.
>>   - drm-shared-<region>: <uint> [KiB|MiB]
>>   The total size of buffers that are shared with another file (e.g., 
>> have more
>> -than a single handle).
>> -
>> -- drm-total-<region>: <uint> [KiB|MiB]
>> -
>> -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.
>> +than a single handle). Same caveat as drm-total- applies.
>>   - drm-resident-<region>: <uint> [KiB|MiB]
>>   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.
>> +The total size of buffers that are resident and 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
>> @@ -192,6 +184,10 @@ 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.
>> +- drm-memory-<region>: <uint> [KiB|MiB]
>> +
>> +This key is deprecated and is an alias for drm-resident-<region> if 
>> present.
>> +
>>   Implementation Details
>>   ======================
>
Yunxiang Li Nov. 18, 2024, 2:56 p.m. UTC | #4
[Public]

> From: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
> Sent: Monday, November 18, 2024 9:38
> On 16/11/2024 04:44, Yunxiang Li wrote:
> > Define how to handle buffers with multiple possible placement so we
> > don't get incompatible implementations. Callout the resident
> > requirement for drm-purgeable- explicitly. Remove the requirement for
> > there to be only drm-memory- or only drm-resident-, it's not what's
> > implemented and having both is better for back-compat. Also re-order
> > the paragraphs to flow better.
> >
> > Signed-off-by: Yunxiang Li <Yunxiang.Li@amd.com>
> > CC: dri-devel@lists.freedesktop.org
> > ---
> >   Documentation/gpu/drm-usage-stats.rst | 36 ++++++++++++---------------
> >   1 file changed, 16 insertions(+), 20 deletions(-)
> >
> > diff --git a/Documentation/gpu/drm-usage-stats.rst
> > b/Documentation/gpu/drm-usage-stats.rst
> > index ff964c707754a..973663f91a292 100644
> > --- a/Documentation/gpu/drm-usage-stats.rst
> > +++ b/Documentation/gpu/drm-usage-stats.rst
> > @@ -140,13 +140,9 @@ both.
> >   Memory
> >   ^^^^^^
> >
> > -- 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 region name "memory" is reserved to refer to normal system 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 used as
> the "<region>"
> > +string. 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.
> > @@ -154,31 +150,27 @@ 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.
>
> IMO the second sentence should stay in principle (I mean at the new location,
> where you moved it). Intent is to avoid new implementations adding both keys. The
> fact amdgpu has both is not relevant for that purpose. We don't want someone just
> reading it is an alias and having to have any doubt whether they need to output both
> or not.

I see, yeah I will mention in the drm-memory- part that that tag is legacy amdgpu only behavior.

> > +- drm-total-<region>: <uint> [KiB|MiB]
> > +
> > +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 count under this category. To avoid double
> > +counting, if a buffer falls under multiple regions, the
> > +implementation should pick only one of the regions, and do so in a consistent
> manner.
>
> Addition feels fine to me in principle. I would only suggest rewording it a bit to avoid
> ambiguity about what it means to "fall under". Perhaps like this:
>
> To avoid double counting when buffers can be instantiated in one of the multiple
> allowed memory regions, the implementation should account the total against only
> one of the regions, and should pick this region in a consistent manner.

Ack

> >
> >   - drm-shared-<region>: <uint> [KiB|MiB]
> >
> >   The total size of buffers that are shared with another file (e.g.,
> > have more -than a single handle).
> > -
> > -- drm-total-<region>: <uint> [KiB|MiB]
> > -
> > -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.
> > +than a single handle). Same caveat as drm-total- applies.
>
> I suggest to explicitly point out the caveat is the multiple region one.

and Ack

> >
> >   - drm-resident-<region>: <uint> [KiB|MiB]
> >
> >   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.
>
> I think it does not harm to keep this note at both keys. Or at least make one
> reference the other for this point specifically.

Might be easier to just have drm-memory- as a foot note here, instead of its own section

> > -
> >   - drm-purgeable-<region>: <uint> [KiB|MiB]
> >
> > -The total size of buffers that are purgeable.
> > +The total size of buffers that are resident and purgeable.
>
> Is it not redundant? How could something not resident be purgeable in the first
> place?

There is the possible confusion between buffers having a purgeable bit and buffers in a state that is purgeable, I feel like it's worth an explicit callout since there's also code comments about this difference.

> >   For example drivers which implement a form of 'madvise' like functionality can
> >   here count buffers which have instantiated backing store, but have
> > been marked @@ -192,6 +184,10 @@ 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.
> >
> > +- drm-memory-<region>: <uint> [KiB|MiB]
> > +
> > +This key is deprecated and is an alias for drm-resident-<region> if present.
> > +
> >   Implementation Details
> >   ======================
> >
>
> Regards,
>
> Tvrtko
Tvrtko Ursulin Nov. 18, 2024, 3:48 p.m. UTC | #5
On 18/11/2024 14:56, Li, Yunxiang (Teddy) wrote:
> [Public]
> 
>> From: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
>> Sent: Monday, November 18, 2024 9:38
>> On 16/11/2024 04:44, Yunxiang Li wrote:
>>> Define how to handle buffers with multiple possible placement so we
>>> don't get incompatible implementations. Callout the resident
>>> requirement for drm-purgeable- explicitly. Remove the requirement for
>>> there to be only drm-memory- or only drm-resident-, it's not what's
>>> implemented and having both is better for back-compat. Also re-order
>>> the paragraphs to flow better.
>>>
>>> Signed-off-by: Yunxiang Li <Yunxiang.Li@amd.com>
>>> CC: dri-devel@lists.freedesktop.org
>>> ---
>>>    Documentation/gpu/drm-usage-stats.rst | 36 ++++++++++++---------------
>>>    1 file changed, 16 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/Documentation/gpu/drm-usage-stats.rst
>>> b/Documentation/gpu/drm-usage-stats.rst
>>> index ff964c707754a..973663f91a292 100644
>>> --- a/Documentation/gpu/drm-usage-stats.rst
>>> +++ b/Documentation/gpu/drm-usage-stats.rst
>>> @@ -140,13 +140,9 @@ both.
>>>    Memory
>>>    ^^^^^^
>>>
>>> -- 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 region name "memory" is reserved to refer to normal system 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 used as
>> the "<region>"
>>> +string. 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.
>>> @@ -154,31 +150,27 @@ 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.
>>
>> IMO the second sentence should stay in principle (I mean at the new location,
>> where you moved it). Intent is to avoid new implementations adding both keys. The
>> fact amdgpu has both is not relevant for that purpose. We don't want someone just
>> reading it is an alias and having to have any doubt whether they need to output both
>> or not.
> 
> I see, yeah I will mention in the drm-memory- part that that tag is legacy amdgpu only behavior.
> 
>>> +- drm-total-<region>: <uint> [KiB|MiB]
>>> +
>>> +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 count under this category. To avoid double
>>> +counting, if a buffer falls under multiple regions, the
>>> +implementation should pick only one of the regions, and do so in a consistent
>> manner.
>>
>> Addition feels fine to me in principle. I would only suggest rewording it a bit to avoid
>> ambiguity about what it means to "fall under". Perhaps like this:
>>
>> To avoid double counting when buffers can be instantiated in one of the multiple
>> allowed memory regions, the implementation should account the total against only
>> one of the regions, and should pick this region in a consistent manner.
> 
> Ack
> 
>>>
>>>    - drm-shared-<region>: <uint> [KiB|MiB]
>>>
>>>    The total size of buffers that are shared with another file (e.g.,
>>> have more -than a single handle).
>>> -
>>> -- drm-total-<region>: <uint> [KiB|MiB]
>>> -
>>> -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.
>>> +than a single handle). Same caveat as drm-total- applies.
>>
>> I suggest to explicitly point out the caveat is the multiple region one.
> 
> and Ack
> 
>>>
>>>    - drm-resident-<region>: <uint> [KiB|MiB]
>>>
>>>    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.
>>
>> I think it does not harm to keep this note at both keys. Or at least make one
>> reference the other for this point specifically.
> 
> Might be easier to just have drm-memory- as a foot note here, instead of its own section

Not entirely sure but as long as the legacy and one key note is easily 
spotted when reading either of the two it works for me.

> 
>>> -
>>>    - drm-purgeable-<region>: <uint> [KiB|MiB]
>>>
>>> -The total size of buffers that are purgeable.
>>> +The total size of buffers that are resident and purgeable.
>>
>> Is it not redundant? How could something not resident be purgeable in the first
>> place?
> 
> There is the possible confusion between buffers having a purgeable bit and buffers in a state that is purgeable, I feel like it's worth an explicit callout since there's also code comments about this difference.

Hm I don't follow this. If you are talking about some implementation 
details does someone viewing this from the outside cares? Anyway, the 
addition does not harm, just that I don't see the need. Feel free to 
leave it.

Regards,

Tvrtko

> 
>>>    For example drivers which implement a form of 'madvise' like functionality can
>>>    here count buffers which have instantiated backing store, but have
>>> been marked @@ -192,6 +184,10 @@ 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.
>>>
>>> +- drm-memory-<region>: <uint> [KiB|MiB]
>>> +
>>> +This key is deprecated and is an alias for drm-resident-<region> if present.
>>> +
>>>    Implementation Details
>>>    ======================
>>>
>>
>> Regards,
>>
>> Tvrtko
diff mbox series

Patch

diff --git a/Documentation/gpu/drm-usage-stats.rst b/Documentation/gpu/drm-usage-stats.rst
index ff964c707754a..973663f91a292 100644
--- a/Documentation/gpu/drm-usage-stats.rst
+++ b/Documentation/gpu/drm-usage-stats.rst
@@ -140,13 +140,9 @@  both.
 Memory
 ^^^^^^
 
-- 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 region name "memory" is reserved to refer to normal system 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 used as the "<region>"
+string. 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.
@@ -154,31 +150,27 @@  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.
+- drm-total-<region>: <uint> [KiB|MiB]
+
+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
+count under this category. To avoid double counting, if a buffer falls under
+multiple regions, the implementation should pick only one of the regions, and do
+so in a consistent manner.
 
 - drm-shared-<region>: <uint> [KiB|MiB]
 
 The total size of buffers that are shared with another file (e.g., have more
-than a single handle).
-
-- drm-total-<region>: <uint> [KiB|MiB]
-
-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.
+than a single handle). Same caveat as drm-total- applies.
 
 - drm-resident-<region>: <uint> [KiB|MiB]
 
 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.
+The total size of buffers that are resident and 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
@@ -192,6 +184,10 @@  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.
 
+- drm-memory-<region>: <uint> [KiB|MiB]
+
+This key is deprecated and is an alias for drm-resident-<region> if present.
+
 Implementation Details
 ======================