Message-ID: <aba6defc-817f-477c-8569-c5e01ca734b6@ti.com>
Date: Thu, 10 Jul 2025 09:34:12 -0500
From: Andrew Davis <afd@...com>
To: Maxime Ripard <mripard@...nel.org>
CC: Sumit Semwal <sumit.semwal@...aro.org>,
        Benjamin Gaignard
	<benjamin.gaignard@...labora.com>,
        Brian Starkey <Brian.Starkey@....com>,
        John Stultz <jstultz@...gle.com>,
        "T.J. Mercier" <tjmercier@...gle.com>,
        Jonathan Corbet <corbet@....net>, <linux-media@...r.kernel.org>,
        <dri-devel@...ts.freedesktop.org>, <linaro-mm-sig@...ts.linaro.org>,
        <linux-doc@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        Jared Kangas
	<jkangas@...hat.com>,
        Mattijs Korpershoek <mkorpershoek@...nel.org>,
        Bagas
 Sanjaya <bagasdotme@...il.com>
Subject: Re: [PATCH v2] Documentation: dma-buf: heaps: Add naming guidelines

On 7/10/25 2:06 AM, Maxime Ripard wrote:
> On Wed, Jul 09, 2025 at 12:39:15PM -0500, Andrew Davis wrote:
>> On 6/16/25 10:21 AM, Maxime Ripard wrote:
>>> We've discussed a number of times of how some heap names are bad, but
>>> not really what makes a good heap name.
>>>
>>> Let's document what we expect the heap names to look like.
>>>
>>> Reviewed-by: Bagas Sanjaya <bagasdotme@...il.com>
>>> Signed-off-by: Maxime Ripard <mripard@...nel.org>
>>> ---
>>> Changes in v2:
>>> - Added justifications for each requirement / suggestions
>>> - Added a mention and example of buffer attributes
>>> - Link to v1: https://lore.kernel.org/r/20250520-dma-buf-heap-names-doc-v1-1-ab31f74809ee@kernel.org
>>> ---
>>>    Documentation/userspace-api/dma-buf-heaps.rst | 38 +++++++++++++++++++++++++++
>>>    1 file changed, 38 insertions(+)
>>>
>>> diff --git a/Documentation/userspace-api/dma-buf-heaps.rst b/Documentation/userspace-api/dma-buf-heaps.rst
>>> index 535f49047ce6450796bf4380c989e109355efc05..835ad1c3a65bc07b6f41d387d85c57162909e859 100644
>>> --- a/Documentation/userspace-api/dma-buf-heaps.rst
>>> +++ b/Documentation/userspace-api/dma-buf-heaps.rst
>>> @@ -21,5 +21,43 @@ following heaps:
>>>       usually created either through the kernel commandline through the
>>>       `cma` parameter, a memory region Device-Tree node with the
>>>       `linux,cma-default` property set, or through the `CMA_SIZE_MBYTES` or
>>>       `CMA_SIZE_PERCENTAGE` Kconfig options. Depending on the platform, it
>>>       might be called ``reserved``, ``linux,cma``, or ``default-pool``.
>>> +
>>> +Naming Convention
>>> +=================
>>> +
>>> +``dma-buf`` heaps name should meet a number of constraints:
>>> +
>>> +- That name must be stable, and must not change from one version to the
>>> +  other. Userspace identifies heaps by their name, so if the names ever
>>> +  changes, we would be likely to introduce regressions.
>>> +
>>> +- That name must describe the memory region the heap will allocate from,
>>> +  and must uniquely identify it in a given platform. Since userspace
>>> +  applications use the heap name as the discriminant, it must be able to
>>> +  tell which heap it wants to use reliably if there's multiple heaps.
>>> +
>>> +- That name must not mention implementation details, such as the
>>> +  allocator. The heap driver will change over time, and implementation
>>> +  details when it was introduced might not be relevant in the future.
>>> +
>>> +- The name should describe properties of the buffers that would be
>>> +  allocated. Doing so will make heap identification easier for
>>> +  userspace. Such properties are:
>>> +
>>> +  - ``cacheable`` / ``uncacheable`` for buffers with CPU caches enabled
>>> +    or disabled;
>>> +
>>
>> We should avoid exposing cacheability to userspace. What users care about
>> is if writes are readable by the other side (and vice versa) without SYNC
>> operations in-between. This property is "coherency". Being non-cached
>> is just one way to achieve coherency on some systems. For many systems
>> even cached buffers are still coherent and manually specifying "non-cached"
>> causes unneeded performance issues.
> 
> I disagree. If you want to do any kind of software rendering, the
> buffers being cached is absolutely critical to having decent
> performance.
> 

I think we are saying the same thing, the default should be cached.
If the user doesn't have an option for specifying "non-cached" then
they will always get the better performing cached buffers.

>> DMA-BUFs are default assumed to be non-coherent and sync operations should
>> be always be performed (if the buffer is actually coherent these operations
>> are turned into NOPs and no harm done). If sync operations cannot be done
>> (for instance small multi-writer ring-buffers), then the property can
>> be simply:
>>
>> - ``coherent`` for buffers which do not require sync operations
> 
> That would be a change in the uAPI which, so far, requires sync
> operations to be performed. I'm not necessarily agaisnt it, but handling
> coherency in general is not what this patch is about.

Agree, so then let's just drop the line about cacheable/coherent from
this doc, we can deal with those properties and document them when/if
they are ever needed.

Andrew

> 
> Maxime

Powered by blists - more mailing lists