* [PATCH v6 1/2] dma/contiguous: Add helper to test reserved memory type
2025-07-09 12:44 [PATCH v6 0/2] dma-buf: heaps: Create a CMA heap for each CMA reserved region Maxime Ripard
@ 2025-07-09 12:44 ` Maxime Ripard
2025-07-09 15:55 ` Andrew Davis
2025-07-09 12:44 ` [PATCH v6 2/2] dma-buf: heaps: cma: Create CMA heap for each CMA reserved region Maxime Ripard
2025-07-09 13:10 ` [PATCH v6 0/2] dma-buf: heaps: Create a " Nicolas Dufresne
2 siblings, 1 reply; 13+ messages in thread
From: Maxime Ripard @ 2025-07-09 12:44 UTC (permalink / raw)
To: Rob Herring, Saravana Kannan, Sumit Semwal, Benjamin Gaignard,
Brian Starkey, John Stultz, T.J. Mercier, Christian König,
Krzysztof Kozlowski, Conor Dooley, Marek Szyprowski, Robin Murphy
Cc: Andrew Davis, Jared Kangas, Mattijs Korpershoek, devicetree,
linux-kernel, linux-media, dri-devel, linaro-mm-sig, iommu,
Maxime Ripard
A given reserved-memory region can be of multiple types.
We have currently four types defined in the tree: contiguous, backed by
CMA, coherent and swiotlb, backed by their respective allocators, and a
platform-specific one for tegra.
However, some users, like dma-buf heaps, might be interested in the
exact type of a reserved memory region they are getting. It would thus
be useful to have helpers to test if a given region is of a given type.
Since we only care about CMA for now though, let's create one for CMA
only.
Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
include/linux/dma-map-ops.h | 13 +++++++++++++
kernel/dma/contiguous.c | 7 +++++++
2 files changed, 20 insertions(+)
diff --git a/include/linux/dma-map-ops.h b/include/linux/dma-map-ops.h
index f48e5fb88bd5dd346094bbf2ce1b79e5f5bfe1a6..ea646acb6367bd062619b337013db221749f85ab 100644
--- a/include/linux/dma-map-ops.h
+++ b/include/linux/dma-map-ops.h
@@ -153,10 +153,23 @@ static inline void dma_free_contiguous(struct device *dev, struct page *page,
{
__free_pages(page, get_order(size));
}
#endif /* CONFIG_DMA_CMA*/
+#if defined(CONFIG_DMA_CMA) && defined(CONFIG_OF_RESERVED_MEM)
+struct reserved_mem;
+
+bool of_reserved_mem_is_contiguous(const struct reserved_mem *rmem);
+#else
+struct reserved_mem;
+
+static inline bool of_reserved_mem_is_contiguous(const struct reserved_mem *rmem)
+{
+ return false;
+}
+#endif
+
#ifdef CONFIG_DMA_DECLARE_COHERENT
int dma_declare_coherent_memory(struct device *dev, phys_addr_t phys_addr,
dma_addr_t device_addr, size_t size);
void dma_release_coherent_memory(struct device *dev);
int dma_alloc_from_dev_coherent(struct device *dev, ssize_t size,
diff --git a/kernel/dma/contiguous.c b/kernel/dma/contiguous.c
index 8df0dfaaca18eeb0a20145512ba64425d2e7601e..ace4982e928e404315cf38551e1596f7ed445156 100644
--- a/kernel/dma/contiguous.c
+++ b/kernel/dma/contiguous.c
@@ -493,6 +493,13 @@ static int __init rmem_cma_setup(struct reserved_mem *rmem)
&rmem->base, (unsigned long)rmem->size / SZ_1M);
return 0;
}
RESERVEDMEM_OF_DECLARE(cma, "shared-dma-pool", rmem_cma_setup);
+
+bool of_reserved_mem_is_contiguous(const struct reserved_mem *rmem)
+{
+ return rmem->ops == &rmem_cma_ops;
+}
+EXPORT_SYMBOL_GPL(of_reserved_mem_is_contiguous);
+
#endif
--
2.50.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v6 1/2] dma/contiguous: Add helper to test reserved memory type
2025-07-09 12:44 ` [PATCH v6 1/2] dma/contiguous: Add helper to test reserved memory type Maxime Ripard
@ 2025-07-09 15:55 ` Andrew Davis
2025-07-10 7:36 ` Maxime Ripard
0 siblings, 1 reply; 13+ messages in thread
From: Andrew Davis @ 2025-07-09 15:55 UTC (permalink / raw)
To: Maxime Ripard, Rob Herring, Saravana Kannan, Sumit Semwal,
Benjamin Gaignard, Brian Starkey, John Stultz, T.J. Mercier,
Christian König, Krzysztof Kozlowski, Conor Dooley,
Marek Szyprowski, Robin Murphy
Cc: Jared Kangas, Mattijs Korpershoek, devicetree, linux-kernel,
linux-media, dri-devel, linaro-mm-sig, iommu
On 7/9/25 7:44 AM, Maxime Ripard wrote:
> A given reserved-memory region can be of multiple types.
>
> We have currently four types defined in the tree: contiguous, backed by
> CMA, coherent and swiotlb, backed by their respective allocators, and a
> platform-specific one for tegra.
>
> However, some users, like dma-buf heaps, might be interested in the
> exact type of a reserved memory region they are getting. It would thus
> be useful to have helpers to test if a given region is of a given type.
>
> Since we only care about CMA for now though, let's create one for CMA
> only.
>
> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> ---
> include/linux/dma-map-ops.h | 13 +++++++++++++
> kernel/dma/contiguous.c | 7 +++++++
> 2 files changed, 20 insertions(+)
>
> diff --git a/include/linux/dma-map-ops.h b/include/linux/dma-map-ops.h
> index f48e5fb88bd5dd346094bbf2ce1b79e5f5bfe1a6..ea646acb6367bd062619b337013db221749f85ab 100644
> --- a/include/linux/dma-map-ops.h
> +++ b/include/linux/dma-map-ops.h
> @@ -153,10 +153,23 @@ static inline void dma_free_contiguous(struct device *dev, struct page *page,
> {
> __free_pages(page, get_order(size));
> }
> #endif /* CONFIG_DMA_CMA*/
>
> +#if defined(CONFIG_DMA_CMA) && defined(CONFIG_OF_RESERVED_MEM)
> +struct reserved_mem;
> +
> +bool of_reserved_mem_is_contiguous(const struct reserved_mem *rmem);
> +#else
> +struct reserved_mem;
> +
> +static inline bool of_reserved_mem_is_contiguous(const struct reserved_mem *rmem)
> +{
> + return false;
> +}
> +#endif
> +
Should this all go in linux/of_reserved_mem.h?
> #ifdef CONFIG_DMA_DECLARE_COHERENT
> int dma_declare_coherent_memory(struct device *dev, phys_addr_t phys_addr,
> dma_addr_t device_addr, size_t size);
> void dma_release_coherent_memory(struct device *dev);
> int dma_alloc_from_dev_coherent(struct device *dev, ssize_t size,
> diff --git a/kernel/dma/contiguous.c b/kernel/dma/contiguous.c
> index 8df0dfaaca18eeb0a20145512ba64425d2e7601e..ace4982e928e404315cf38551e1596f7ed445156 100644
> --- a/kernel/dma/contiguous.c
> +++ b/kernel/dma/contiguous.c
> @@ -493,6 +493,13 @@ static int __init rmem_cma_setup(struct reserved_mem *rmem)
> &rmem->base, (unsigned long)rmem->size / SZ_1M);
>
> return 0;
> }
> RESERVEDMEM_OF_DECLARE(cma, "shared-dma-pool", rmem_cma_setup);
> +
> +bool of_reserved_mem_is_contiguous(const struct reserved_mem *rmem)
Needing to check where the reserved mem comes from seems wrong, it hints
that the reserved mem region drivers, like this one, are not in full control
of their regions. Instead of looping over all the regions in DT in the next
patch and searching for the owner, how about the owner (this driver) call
into __add_cma_heap() if it chooses to expose the region in that way.
(I know RESERVEDMEM_OF_DECLARE callbacks are done very early and the CMA-Heap
driver might not be able to deal with adding heaps at this point, so maybe
keeping a table the heaps driver can later iterate over would also work).
Andrew
> +{
> + return rmem->ops == &rmem_cma_ops;
> +}
> +EXPORT_SYMBOL_GPL(of_reserved_mem_is_contiguous);
> +
> #endif
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v6 1/2] dma/contiguous: Add helper to test reserved memory type
2025-07-09 15:55 ` Andrew Davis
@ 2025-07-10 7:36 ` Maxime Ripard
0 siblings, 0 replies; 13+ messages in thread
From: Maxime Ripard @ 2025-07-10 7:36 UTC (permalink / raw)
To: Andrew Davis
Cc: Rob Herring, Saravana Kannan, Sumit Semwal, Benjamin Gaignard,
Brian Starkey, John Stultz, T.J. Mercier, Christian König,
Krzysztof Kozlowski, Conor Dooley, Marek Szyprowski, Robin Murphy,
Jared Kangas, Mattijs Korpershoek, devicetree, linux-kernel,
linux-media, dri-devel, linaro-mm-sig, iommu
[-- Attachment #1: Type: text/plain, Size: 3455 bytes --]
Hi Andrew,
On Wed, Jul 09, 2025 at 10:55:40AM -0500, Andrew Davis wrote:
> On 7/9/25 7:44 AM, Maxime Ripard wrote:
> > A given reserved-memory region can be of multiple types.
> >
> > We have currently four types defined in the tree: contiguous, backed by
> > CMA, coherent and swiotlb, backed by their respective allocators, and a
> > platform-specific one for tegra.
> >
> > However, some users, like dma-buf heaps, might be interested in the
> > exact type of a reserved memory region they are getting. It would thus
> > be useful to have helpers to test if a given region is of a given type.
> >
> > Since we only care about CMA for now though, let's create one for CMA
> > only.
> >
> > Signed-off-by: Maxime Ripard <mripard@kernel.org>
> > ---
> > include/linux/dma-map-ops.h | 13 +++++++++++++
> > kernel/dma/contiguous.c | 7 +++++++
> > 2 files changed, 20 insertions(+)
> >
> > diff --git a/include/linux/dma-map-ops.h b/include/linux/dma-map-ops.h
> > index f48e5fb88bd5dd346094bbf2ce1b79e5f5bfe1a6..ea646acb6367bd062619b337013db221749f85ab 100644
> > --- a/include/linux/dma-map-ops.h
> > +++ b/include/linux/dma-map-ops.h
> > @@ -153,10 +153,23 @@ static inline void dma_free_contiguous(struct device *dev, struct page *page,
> > {
> > __free_pages(page, get_order(size));
> > }
> > #endif /* CONFIG_DMA_CMA*/
> > +#if defined(CONFIG_DMA_CMA) && defined(CONFIG_OF_RESERVED_MEM)
> > +struct reserved_mem;
> > +
> > +bool of_reserved_mem_is_contiguous(const struct reserved_mem *rmem);
> > +#else
> > +struct reserved_mem;
> > +
> > +static inline bool of_reserved_mem_is_contiguous(const struct reserved_mem *rmem)
> > +{
> > + return false;
> > +}
> > +#endif
> > +
>
> Should this all go in linux/of_reserved_mem.h?
>
> > #ifdef CONFIG_DMA_DECLARE_COHERENT
> > int dma_declare_coherent_memory(struct device *dev, phys_addr_t phys_addr,
> > dma_addr_t device_addr, size_t size);
> > void dma_release_coherent_memory(struct device *dev);
> > int dma_alloc_from_dev_coherent(struct device *dev, ssize_t size,
> > diff --git a/kernel/dma/contiguous.c b/kernel/dma/contiguous.c
> > index 8df0dfaaca18eeb0a20145512ba64425d2e7601e..ace4982e928e404315cf38551e1596f7ed445156 100644
> > --- a/kernel/dma/contiguous.c
> > +++ b/kernel/dma/contiguous.c
> > @@ -493,6 +493,13 @@ static int __init rmem_cma_setup(struct reserved_mem *rmem)
> > &rmem->base, (unsigned long)rmem->size / SZ_1M);
> > return 0;
> > }
> > RESERVEDMEM_OF_DECLARE(cma, "shared-dma-pool", rmem_cma_setup);
> > +
> > +bool of_reserved_mem_is_contiguous(const struct reserved_mem *rmem)
>
> Needing to check where the reserved mem comes from seems wrong, it hints
> that the reserved mem region drivers, like this one, are not in full control
> of their regions. Instead of looping over all the regions in DT in the next
> patch and searching for the owner, how about the owner (this driver) call
> into __add_cma_heap() if it chooses to expose the region in that way.
>
> (I know RESERVEDMEM_OF_DECLARE callbacks are done very early and the CMA-Heap
> driver might not be able to deal with adding heaps at this point, so maybe
> keeping a table the heaps driver can later iterate over would also work).
It's something I considered but wasn't too sure about, so I went the
less intrusive way.
I'll work on that for the next version, thanks!
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v6 2/2] dma-buf: heaps: cma: Create CMA heap for each CMA reserved region
2025-07-09 12:44 [PATCH v6 0/2] dma-buf: heaps: Create a CMA heap for each CMA reserved region Maxime Ripard
2025-07-09 12:44 ` [PATCH v6 1/2] dma/contiguous: Add helper to test reserved memory type Maxime Ripard
@ 2025-07-09 12:44 ` Maxime Ripard
2025-07-09 16:14 ` Andrew Davis
2025-07-09 13:10 ` [PATCH v6 0/2] dma-buf: heaps: Create a " Nicolas Dufresne
2 siblings, 1 reply; 13+ messages in thread
From: Maxime Ripard @ 2025-07-09 12:44 UTC (permalink / raw)
To: Rob Herring, Saravana Kannan, Sumit Semwal, Benjamin Gaignard,
Brian Starkey, John Stultz, T.J. Mercier, Christian König,
Krzysztof Kozlowski, Conor Dooley, Marek Szyprowski, Robin Murphy
Cc: Andrew Davis, Jared Kangas, Mattijs Korpershoek, devicetree,
linux-kernel, linux-media, dri-devel, linaro-mm-sig, iommu,
Maxime Ripard
Aside from the main CMA region, it can be useful to allow userspace to
allocate from the other CMA reserved regions.
Indeed, those regions can have specific properties that can be useful to
a specific us-case.
For example, one of them platform I've been with has ECC enabled on the
entire memory but for a specific region. Using that region to allocate
framebuffers can be particular beneficial because enabling the ECC has a
performance and memory footprint cost.
Thus, exposing these regions as heaps user-space can allocate from and
import wherever needed allows to cover that use-case.
For now, only shared-dma-pools regions with the reusable property (ie,
backed by CMA) are supported, but eventually we'll want to support other
DMA pools types.
Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
drivers/dma-buf/heaps/cma_heap.c | 52 +++++++++++++++++++++++++++++++++++++++-
1 file changed, 51 insertions(+), 1 deletion(-)
diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma-buf/heaps/cma_heap.c
index 0df007111975447d555714d61ead9699287fd65a..31a18683ee25788a800f3f878fd958718a930ff7 100644
--- a/drivers/dma-buf/heaps/cma_heap.c
+++ b/drivers/dma-buf/heaps/cma_heap.c
@@ -19,10 +19,12 @@
#include <linux/err.h>
#include <linux/highmem.h>
#include <linux/io.h>
#include <linux/mm.h>
#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_reserved_mem.h>
#include <linux/scatterlist.h>
#include <linux/slab.h>
#include <linux/vmalloc.h>
#define DEFAULT_CMA_NAME "default_cma_region"
@@ -421,7 +423,55 @@ static int __init add_default_cma_heap(void)
ERR_PTR(ret));
}
return 0;
}
-module_init(add_default_cma_heap);
+
+static int __init add_cma_heaps(void)
+{
+ struct device_node *rmem_node;
+ struct device_node *node;
+ int ret;
+
+ ret = add_default_cma_heap();
+ if (ret)
+ return ret;
+
+ rmem_node = of_find_node_by_path("/reserved-memory");
+ if (!rmem_node)
+ goto out;
+
+ for_each_child_of_node(rmem_node, node) {
+ struct reserved_mem *rmem;
+ struct cma *cma;
+
+ rmem = of_reserved_mem_lookup(node);
+ if (!rmem) {
+ ret = -EINVAL;
+ goto err_put_node;
+ }
+
+ if (!of_reserved_mem_is_contiguous(rmem))
+ continue;
+
+ cma = rmem->priv;
+ if (!cma) {
+ ret = -EINVAL;
+ goto err_put_node;
+ }
+
+ ret = __add_cma_heap(cma, of_node_full_name(node));
+ if (ret)
+ goto err_put_node;
+ }
+
+out:
+ of_node_put(rmem_node);
+ return 0;
+
+err_put_node:
+ of_node_put(rmem_node);
+ return ret;
+}
+
+module_init(add_cma_heaps);
MODULE_DESCRIPTION("DMA-BUF CMA Heap");
--
2.50.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v6 2/2] dma-buf: heaps: cma: Create CMA heap for each CMA reserved region
2025-07-09 12:44 ` [PATCH v6 2/2] dma-buf: heaps: cma: Create CMA heap for each CMA reserved region Maxime Ripard
@ 2025-07-09 16:14 ` Andrew Davis
2025-07-10 7:44 ` Maxime Ripard
0 siblings, 1 reply; 13+ messages in thread
From: Andrew Davis @ 2025-07-09 16:14 UTC (permalink / raw)
To: Maxime Ripard, Rob Herring, Saravana Kannan, Sumit Semwal,
Benjamin Gaignard, Brian Starkey, John Stultz, T.J. Mercier,
Christian König, Krzysztof Kozlowski, Conor Dooley,
Marek Szyprowski, Robin Murphy
Cc: Jared Kangas, Mattijs Korpershoek, devicetree, linux-kernel,
linux-media, dri-devel, linaro-mm-sig, iommu
On 7/9/25 7:44 AM, Maxime Ripard wrote:
> Aside from the main CMA region, it can be useful to allow userspace to
> allocate from the other CMA reserved regions.
>
> Indeed, those regions can have specific properties that can be useful to
> a specific us-case.
>
> For example, one of them platform I've been with has ECC enabled on the
> entire memory but for a specific region. Using that region to allocate
> framebuffers can be particular beneficial because enabling the ECC has a
> performance and memory footprint cost.
>
> Thus, exposing these regions as heaps user-space can allocate from and
> import wherever needed allows to cover that use-case.
>
> For now, only shared-dma-pools regions with the reusable property (ie,
> backed by CMA) are supported, but eventually we'll want to support other
> DMA pools types.
>
> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> ---
> drivers/dma-buf/heaps/cma_heap.c | 52 +++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 51 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma-buf/heaps/cma_heap.c
> index 0df007111975447d555714d61ead9699287fd65a..31a18683ee25788a800f3f878fd958718a930ff7 100644
> --- a/drivers/dma-buf/heaps/cma_heap.c
> +++ b/drivers/dma-buf/heaps/cma_heap.c
> @@ -19,10 +19,12 @@
> #include <linux/err.h>
> #include <linux/highmem.h>
> #include <linux/io.h>
> #include <linux/mm.h>
> #include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_reserved_mem.h>
> #include <linux/scatterlist.h>
> #include <linux/slab.h>
> #include <linux/vmalloc.h>
>
> #define DEFAULT_CMA_NAME "default_cma_region"
> @@ -421,7 +423,55 @@ static int __init add_default_cma_heap(void)
> ERR_PTR(ret));
> }
>
> return 0;
> }
> -module_init(add_default_cma_heap);
> +
> +static int __init add_cma_heaps(void)
> +{
> + struct device_node *rmem_node;
> + struct device_node *node;
> + int ret;
> +
> + ret = add_default_cma_heap();
Will this double add the default CMA region if it was declared
using DT (reserved-memory) when all those nodes are again scanned
through below? Might need a check in that loop for linux,cma-default.
> + if (ret)
> + return ret;
> +
> + rmem_node = of_find_node_by_path("/reserved-memory");
> + if (!rmem_node)
> + goto out;
Can just return here, "out" path doesn't need to put a NULL node.
Andrew
> +
> + for_each_child_of_node(rmem_node, node) {
> + struct reserved_mem *rmem;
> + struct cma *cma;
> +
> + rmem = of_reserved_mem_lookup(node);
> + if (!rmem) {
> + ret = -EINVAL;
> + goto err_put_node;
> + }
> +
> + if (!of_reserved_mem_is_contiguous(rmem))
> + continue;
> +
> + cma = rmem->priv;
> + if (!cma) {
> + ret = -EINVAL;
> + goto err_put_node;
> + }
> +
> + ret = __add_cma_heap(cma, of_node_full_name(node));
> + if (ret)
> + goto err_put_node;
> + }
> +
> +out:
> + of_node_put(rmem_node);
> + return 0;
> +
> +err_put_node:
> + of_node_put(rmem_node);
> + return ret;
> +}
> +
> +module_init(add_cma_heaps);
> MODULE_DESCRIPTION("DMA-BUF CMA Heap");
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v6 2/2] dma-buf: heaps: cma: Create CMA heap for each CMA reserved region
2025-07-09 16:14 ` Andrew Davis
@ 2025-07-10 7:44 ` Maxime Ripard
2025-07-10 14:46 ` Andrew Davis
0 siblings, 1 reply; 13+ messages in thread
From: Maxime Ripard @ 2025-07-10 7:44 UTC (permalink / raw)
To: Andrew Davis
Cc: Rob Herring, Saravana Kannan, Sumit Semwal, Benjamin Gaignard,
Brian Starkey, John Stultz, T.J. Mercier, Christian König,
Krzysztof Kozlowski, Conor Dooley, Marek Szyprowski, Robin Murphy,
Jared Kangas, Mattijs Korpershoek, devicetree, linux-kernel,
linux-media, dri-devel, linaro-mm-sig, iommu
[-- Attachment #1: Type: text/plain, Size: 3011 bytes --]
On Wed, Jul 09, 2025 at 11:14:37AM -0500, Andrew Davis wrote:
> On 7/9/25 7:44 AM, Maxime Ripard wrote:
> > Aside from the main CMA region, it can be useful to allow userspace to
> > allocate from the other CMA reserved regions.
> >
> > Indeed, those regions can have specific properties that can be useful to
> > a specific us-case.
> >
> > For example, one of them platform I've been with has ECC enabled on the
> > entire memory but for a specific region. Using that region to allocate
> > framebuffers can be particular beneficial because enabling the ECC has a
> > performance and memory footprint cost.
> >
> > Thus, exposing these regions as heaps user-space can allocate from and
> > import wherever needed allows to cover that use-case.
> >
> > For now, only shared-dma-pools regions with the reusable property (ie,
> > backed by CMA) are supported, but eventually we'll want to support other
> > DMA pools types.
> >
> > Signed-off-by: Maxime Ripard <mripard@kernel.org>
> > ---
> > drivers/dma-buf/heaps/cma_heap.c | 52 +++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 51 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma-buf/heaps/cma_heap.c
> > index 0df007111975447d555714d61ead9699287fd65a..31a18683ee25788a800f3f878fd958718a930ff7 100644
> > --- a/drivers/dma-buf/heaps/cma_heap.c
> > +++ b/drivers/dma-buf/heaps/cma_heap.c
> > @@ -19,10 +19,12 @@
> > #include <linux/err.h>
> > #include <linux/highmem.h>
> > #include <linux/io.h>
> > #include <linux/mm.h>
> > #include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/of_reserved_mem.h>
> > #include <linux/scatterlist.h>
> > #include <linux/slab.h>
> > #include <linux/vmalloc.h>
> > #define DEFAULT_CMA_NAME "default_cma_region"
> > @@ -421,7 +423,55 @@ static int __init add_default_cma_heap(void)
> > ERR_PTR(ret));
> > }
> > return 0;
> > }
> > -module_init(add_default_cma_heap);
> > +
> > +static int __init add_cma_heaps(void)
> > +{
> > + struct device_node *rmem_node;
> > + struct device_node *node;
> > + int ret;
> > +
> > + ret = add_default_cma_heap();
>
> Will this double add the default CMA region if it was declared
> using DT (reserved-memory) when all those nodes are again scanned
> through below? Might need a check in that loop for linux,cma-default.
Yeah, but we probably should anyway. Otherwise, if linux,cma-default
ever change on a platform, we would get heaps appearing/disappearing as
we reboot, which doesn't sound great from a regression perspective.
Both would allocate from the same pool though, so we don't risk stepping
into each others toes. Or am I missing something?
> > + if (ret)
> > + return ret;
> > +
> > + rmem_node = of_find_node_by_path("/reserved-memory");
> > + if (!rmem_node)
> > + goto out;
>
> Can just return here, "out" path doesn't need to put a NULL node.
Oh, right. Thanks!
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v6 2/2] dma-buf: heaps: cma: Create CMA heap for each CMA reserved region
2025-07-10 7:44 ` Maxime Ripard
@ 2025-07-10 14:46 ` Andrew Davis
2025-07-10 15:11 ` Maxime Ripard
0 siblings, 1 reply; 13+ messages in thread
From: Andrew Davis @ 2025-07-10 14:46 UTC (permalink / raw)
To: Maxime Ripard
Cc: Rob Herring, Saravana Kannan, Sumit Semwal, Benjamin Gaignard,
Brian Starkey, John Stultz, T.J. Mercier, Christian König,
Krzysztof Kozlowski, Conor Dooley, Marek Szyprowski, Robin Murphy,
Jared Kangas, Mattijs Korpershoek, devicetree, linux-kernel,
linux-media, dri-devel, linaro-mm-sig, iommu
On 7/10/25 2:44 AM, Maxime Ripard wrote:
> On Wed, Jul 09, 2025 at 11:14:37AM -0500, Andrew Davis wrote:
>> On 7/9/25 7:44 AM, Maxime Ripard wrote:
>>> Aside from the main CMA region, it can be useful to allow userspace to
>>> allocate from the other CMA reserved regions.
>>>
>>> Indeed, those regions can have specific properties that can be useful to
>>> a specific us-case.
>>>
>>> For example, one of them platform I've been with has ECC enabled on the
>>> entire memory but for a specific region. Using that region to allocate
>>> framebuffers can be particular beneficial because enabling the ECC has a
>>> performance and memory footprint cost.
>>>
>>> Thus, exposing these regions as heaps user-space can allocate from and
>>> import wherever needed allows to cover that use-case.
>>>
>>> For now, only shared-dma-pools regions with the reusable property (ie,
>>> backed by CMA) are supported, but eventually we'll want to support other
>>> DMA pools types.
>>>
>>> Signed-off-by: Maxime Ripard <mripard@kernel.org>
>>> ---
>>> drivers/dma-buf/heaps/cma_heap.c | 52 +++++++++++++++++++++++++++++++++++++++-
>>> 1 file changed, 51 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma-buf/heaps/cma_heap.c
>>> index 0df007111975447d555714d61ead9699287fd65a..31a18683ee25788a800f3f878fd958718a930ff7 100644
>>> --- a/drivers/dma-buf/heaps/cma_heap.c
>>> +++ b/drivers/dma-buf/heaps/cma_heap.c
>>> @@ -19,10 +19,12 @@
>>> #include <linux/err.h>
>>> #include <linux/highmem.h>
>>> #include <linux/io.h>
>>> #include <linux/mm.h>
>>> #include <linux/module.h>
>>> +#include <linux/of.h>
>>> +#include <linux/of_reserved_mem.h>
>>> #include <linux/scatterlist.h>
>>> #include <linux/slab.h>
>>> #include <linux/vmalloc.h>
>>> #define DEFAULT_CMA_NAME "default_cma_region"
>>> @@ -421,7 +423,55 @@ static int __init add_default_cma_heap(void)
>>> ERR_PTR(ret));
>>> }
>>> return 0;
>>> }
>>> -module_init(add_default_cma_heap);
>>> +
>>> +static int __init add_cma_heaps(void)
>>> +{
>>> + struct device_node *rmem_node;
>>> + struct device_node *node;
>>> + int ret;
>>> +
>>> + ret = add_default_cma_heap();
>>
>> Will this double add the default CMA region if it was declared
>> using DT (reserved-memory) when all those nodes are again scanned
>> through below? Might need a check in that loop for linux,cma-default.
>
> Yeah, but we probably should anyway. Otherwise, if linux,cma-default
> ever change on a platform, we would get heaps appearing/disappearing as
> we reboot, which doesn't sound great from a regression perspective.
>
> Both would allocate from the same pool though, so we don't risk stepping
> into each others toes. Or am I missing something?
>
You are not missing anything, having both wouldn't cause anything to break,
but would cause heaps to appear/disappear based on how the CMA region was
defined (DT vs kernel cmd line) which we should avoid.
Andrew
>>> + if (ret)
>>> + return ret;
>>> +
>>> + rmem_node = of_find_node_by_path("/reserved-memory");
>>> + if (!rmem_node)
>>> + goto out;
>>
>> Can just return here, "out" path doesn't need to put a NULL node.
>
> Oh, right. Thanks!
> Maxime
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v6 2/2] dma-buf: heaps: cma: Create CMA heap for each CMA reserved region
2025-07-10 14:46 ` Andrew Davis
@ 2025-07-10 15:11 ` Maxime Ripard
0 siblings, 0 replies; 13+ messages in thread
From: Maxime Ripard @ 2025-07-10 15:11 UTC (permalink / raw)
To: Andrew Davis
Cc: Rob Herring, Saravana Kannan, Sumit Semwal, Benjamin Gaignard,
Brian Starkey, John Stultz, T.J. Mercier, Christian König,
Krzysztof Kozlowski, Conor Dooley, Marek Szyprowski, Robin Murphy,
Jared Kangas, Mattijs Korpershoek, devicetree, linux-kernel,
linux-media, dri-devel, linaro-mm-sig, iommu
[-- Attachment #1: Type: text/plain, Size: 3822 bytes --]
On Thu, Jul 10, 2025 at 09:46:56AM -0500, Andrew Davis wrote:
>
>
> On 7/10/25 2:44 AM, Maxime Ripard wrote:
> > On Wed, Jul 09, 2025 at 11:14:37AM -0500, Andrew Davis wrote:
> > > On 7/9/25 7:44 AM, Maxime Ripard wrote:
> > > > Aside from the main CMA region, it can be useful to allow userspace to
> > > > allocate from the other CMA reserved regions.
> > > >
> > > > Indeed, those regions can have specific properties that can be useful to
> > > > a specific us-case.
> > > >
> > > > For example, one of them platform I've been with has ECC enabled on the
> > > > entire memory but for a specific region. Using that region to allocate
> > > > framebuffers can be particular beneficial because enabling the ECC has a
> > > > performance and memory footprint cost.
> > > >
> > > > Thus, exposing these regions as heaps user-space can allocate from and
> > > > import wherever needed allows to cover that use-case.
> > > >
> > > > For now, only shared-dma-pools regions with the reusable property (ie,
> > > > backed by CMA) are supported, but eventually we'll want to support other
> > > > DMA pools types.
> > > >
> > > > Signed-off-by: Maxime Ripard <mripard@kernel.org>
> > > > ---
> > > > drivers/dma-buf/heaps/cma_heap.c | 52 +++++++++++++++++++++++++++++++++++++++-
> > > > 1 file changed, 51 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma-buf/heaps/cma_heap.c
> > > > index 0df007111975447d555714d61ead9699287fd65a..31a18683ee25788a800f3f878fd958718a930ff7 100644
> > > > --- a/drivers/dma-buf/heaps/cma_heap.c
> > > > +++ b/drivers/dma-buf/heaps/cma_heap.c
> > > > @@ -19,10 +19,12 @@
> > > > #include <linux/err.h>
> > > > #include <linux/highmem.h>
> > > > #include <linux/io.h>
> > > > #include <linux/mm.h>
> > > > #include <linux/module.h>
> > > > +#include <linux/of.h>
> > > > +#include <linux/of_reserved_mem.h>
> > > > #include <linux/scatterlist.h>
> > > > #include <linux/slab.h>
> > > > #include <linux/vmalloc.h>
> > > > #define DEFAULT_CMA_NAME "default_cma_region"
> > > > @@ -421,7 +423,55 @@ static int __init add_default_cma_heap(void)
> > > > ERR_PTR(ret));
> > > > }
> > > > return 0;
> > > > }
> > > > -module_init(add_default_cma_heap);
> > > > +
> > > > +static int __init add_cma_heaps(void)
> > > > +{
> > > > + struct device_node *rmem_node;
> > > > + struct device_node *node;
> > > > + int ret;
> > > > +
> > > > + ret = add_default_cma_heap();
> > >
> > > Will this double add the default CMA region if it was declared
> > > using DT (reserved-memory) when all those nodes are again scanned
> > > through below? Might need a check in that loop for linux,cma-default.
> >
> > Yeah, but we probably should anyway. Otherwise, if linux,cma-default
> > ever change on a platform, we would get heaps appearing/disappearing as
> > we reboot, which doesn't sound great from a regression perspective.
> >
> > Both would allocate from the same pool though, so we don't risk stepping
> > into each others toes. Or am I missing something?
>
> You are not missing anything, having both wouldn't cause anything to break,
> but would cause heaps to appear/disappear based on how the CMA region was
> defined (DT vs kernel cmd line) which we should avoid.
I'm sorry I guess I don't follow you then. It looked like you were
arguing for avoiding double registration (under the CMA default name,
and the region specific name).
I'm arguing that we should always double register to avoid
linux,cma-default having any effect on the userspace.
In the latter, even if linux,cma-default isn't set at all, then the
default CMA heap would still be registered from the command line CMA
region.
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v6 0/2] dma-buf: heaps: Create a CMA heap for each CMA reserved region
2025-07-09 12:44 [PATCH v6 0/2] dma-buf: heaps: Create a CMA heap for each CMA reserved region Maxime Ripard
2025-07-09 12:44 ` [PATCH v6 1/2] dma/contiguous: Add helper to test reserved memory type Maxime Ripard
2025-07-09 12:44 ` [PATCH v6 2/2] dma-buf: heaps: cma: Create CMA heap for each CMA reserved region Maxime Ripard
@ 2025-07-09 13:10 ` Nicolas Dufresne
2025-07-09 13:38 ` Maxime Ripard
2 siblings, 1 reply; 13+ messages in thread
From: Nicolas Dufresne @ 2025-07-09 13:10 UTC (permalink / raw)
To: Maxime Ripard, Rob Herring, Saravana Kannan, Sumit Semwal,
Benjamin Gaignard, Brian Starkey, John Stultz, T.J. Mercier,
Christian König, Krzysztof Kozlowski, Conor Dooley,
Marek Szyprowski, Robin Murphy
Cc: Andrew Davis, Jared Kangas, Mattijs Korpershoek, devicetree,
linux-kernel, linux-media, dri-devel, linaro-mm-sig, iommu
[-- Attachment #1: Type: text/plain, Size: 5148 bytes --]
Hi Maxime,
Le mercredi 09 juillet 2025 à 14:44 +0200, Maxime Ripard a écrit :
> Hi,
>
> Here's another attempt at supporting user-space allocations from a
> specific carved-out reserved memory region.
>
> The initial problem we were discussing was that I'm currently working on
> a platform which has a memory layout with ECC enabled. However, enabling
> the ECC has a number of drawbacks on that platform: lower performance,
> increased memory usage, etc. So for things like framebuffers, the
> trade-off isn't great and thus there's a memory region with ECC disabled
> to allocate from for such use cases.
>
> After a suggestion from John, I chose to first start using heap
> allocations flags to allow for userspace to ask for a particular ECC
> setup. This is then backed by a new heap type that runs from reserved
> memory chunks flagged as such, and the existing DT properties to specify
> the ECC properties.
>
> After further discussion, it was considered that flags were not the
> right solution, and relying on the names of the heaps would be enough to
> let userspace know the kind of buffer it deals with.
>
> Thus, even though the uAPI part of it had been dropped in this second
> version, we still needed a driver to create heaps out of carved-out memory
> regions. In addition to the original usecase, a similar driver can be
> found in BSPs from most vendors, so I believe it would be a useful
> addition to the kernel.
>
> Some extra discussion with Rob Herring [1] came to the conclusion that
> some specific compatible for this is not great either, and as such an
> new driver probably isn't called for either.
>
> Some other discussions we had with John [2] also dropped some hints that
> multiple CMA heaps might be a good idea, and some vendors seem to do
> that too.
>
> So here's another attempt that doesn't affect the device tree at all and
> will just create a heap for every CMA reserved memory region.
Does that means that if we carve-out memory for a co-processor operating system,
that memory region is now available to userspace to allocate from ? Or is there
a nuance to that ?
For other carveout, such as RK3588 HDMI receiver, that is clearly a win, giving
user the ability to allocate using externally supplied constraints rather then
having to convince the v4l2 driver to match these. While keeping the safety that
this carveout will yield valid addresses for the IP.
Will there be a generic way to find out which driver/device this carveout
belongs to ? In V4L2, only complex cameras have userspace drivers, everything
else is generic code.
Nicolas
>
> It also falls nicely into the current plan we have to support cgroups in
> DRM/KMS and v4l2, which is an additional benefit.
>
> Let me know what you think,
> Maxime
>
> 1: https://lore.kernel.org/all/20250707-cobalt-dingo-of-serenity-dbf92c@houat/
> 2:
> https://lore.kernel.org/all/CANDhNCroe6ZBtN_o=c71kzFFaWK-fF5rCdnr9P5h1sgPOWSGSw@mail.gmail.com/
>
> Let me know what you think,
> Maxime
>
> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> ---
> Changes in v6:
> - Drop the new driver and allocate a CMA heap for each region now
> - Dropped the binding
> - Rebased on 6.16-rc5
> - Link to v5:
> https://lore.kernel.org/r/20250617-dma-buf-ecc-heap-v5-0-0abdc5863a4f@kernel.org
>
> Changes in v5:
> - Rebased on 6.16-rc2
> - Switch from property to dedicated binding
> - Link to v4:
> https://lore.kernel.org/r/20250520-dma-buf-ecc-heap-v4-1-bd2e1f1bb42c@kernel.org
>
> Changes in v4:
> - Rebased on 6.15-rc7
> - Map buffers only when map is actually called, not at allocation time
> - Deal with restricted-dma-pool and shared-dma-pool
> - Reword Kconfig options
> - Properly report dma_map_sgtable failures
> - Link to v3:
> https://lore.kernel.org/r/20250407-dma-buf-ecc-heap-v3-0-97cdd36a5f29@kernel.org
>
> Changes in v3:
> - Reworked global variable patch
> - Link to v2:
> https://lore.kernel.org/r/20250401-dma-buf-ecc-heap-v2-0-043fd006a1af@kernel.org
>
> Changes in v2:
> - Add vmap/vunmap operations
> - Drop ECC flags uapi
> - Rebase on top of 6.14
> - Link to v1:
> https://lore.kernel.org/r/20240515-dma-buf-ecc-heap-v1-0-54cbbd049511@kernel.org
>
> ---
> Maxime Ripard (2):
> dma/contiguous: Add helper to test reserved memory type
> dma-buf: heaps: cma: Create CMA heap for each CMA reserved region
>
> drivers/dma-buf/heaps/cma_heap.c | 52
> +++++++++++++++++++++++++++++++++++++++-
> include/linux/dma-map-ops.h | 13 ++++++++++
> kernel/dma/contiguous.c | 7 ++++++
> 3 files changed, 71 insertions(+), 1 deletion(-)
> ---
> base-commit: 47633099a672fc7bfe604ef454e4f116e2c954b1
> change-id: 20240515-dma-buf-ecc-heap-28a311d2c94e
> prerequisite-message-id: <20250610131231.1724627-1-jkangas@redhat.com>
> prerequisite-patch-id: bc44be5968feb187f2bc1b8074af7209462b18e7
> prerequisite-patch-id: f02a91b723e5ec01fbfedf3c3905218b43d432da
> prerequisite-patch-id: e944d0a3e22f2cdf4d3b3906e5603af934696deb
>
> Best regards,
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v6 0/2] dma-buf: heaps: Create a CMA heap for each CMA reserved region
2025-07-09 13:10 ` [PATCH v6 0/2] dma-buf: heaps: Create a " Nicolas Dufresne
@ 2025-07-09 13:38 ` Maxime Ripard
2025-07-10 15:21 ` Nicolas Dufresne
0 siblings, 1 reply; 13+ messages in thread
From: Maxime Ripard @ 2025-07-09 13:38 UTC (permalink / raw)
To: Nicolas Dufresne
Cc: Rob Herring, Saravana Kannan, Sumit Semwal, Benjamin Gaignard,
Brian Starkey, John Stultz, T.J. Mercier, Christian König,
Krzysztof Kozlowski, Conor Dooley, Marek Szyprowski, Robin Murphy,
Andrew Davis, Jared Kangas, Mattijs Korpershoek, devicetree,
linux-kernel, linux-media, dri-devel, linaro-mm-sig, iommu
[-- Attachment #1: Type: text/plain, Size: 3893 bytes --]
On Wed, Jul 09, 2025 at 09:10:02AM -0400, Nicolas Dufresne wrote:
> Hi Maxime,
>
> Le mercredi 09 juillet 2025 à 14:44 +0200, Maxime Ripard a écrit :
> > Hi,
> >
> > Here's another attempt at supporting user-space allocations from a
> > specific carved-out reserved memory region.
> >
> > The initial problem we were discussing was that I'm currently working on
> > a platform which has a memory layout with ECC enabled. However, enabling
> > the ECC has a number of drawbacks on that platform: lower performance,
> > increased memory usage, etc. So for things like framebuffers, the
> > trade-off isn't great and thus there's a memory region with ECC disabled
> > to allocate from for such use cases.
> >
> > After a suggestion from John, I chose to first start using heap
> > allocations flags to allow for userspace to ask for a particular ECC
> > setup. This is then backed by a new heap type that runs from reserved
> > memory chunks flagged as such, and the existing DT properties to specify
> > the ECC properties.
> >
> > After further discussion, it was considered that flags were not the
> > right solution, and relying on the names of the heaps would be enough to
> > let userspace know the kind of buffer it deals with.
> >
> > Thus, even though the uAPI part of it had been dropped in this second
> > version, we still needed a driver to create heaps out of carved-out memory
> > regions. In addition to the original usecase, a similar driver can be
> > found in BSPs from most vendors, so I believe it would be a useful
> > addition to the kernel.
> >
> > Some extra discussion with Rob Herring [1] came to the conclusion that
> > some specific compatible for this is not great either, and as such an
> > new driver probably isn't called for either.
> >
> > Some other discussions we had with John [2] also dropped some hints that
> > multiple CMA heaps might be a good idea, and some vendors seem to do
> > that too.
> >
> > So here's another attempt that doesn't affect the device tree at all and
> > will just create a heap for every CMA reserved memory region.
>
> Does that means that if we carve-out memory for a co-processor operating system,
> that memory region is now available to userspace to allocate from ? Or is there
> a nuance to that ?
There is a nuance to that :)
You need to have the "reusable" property set which is documented as:
The operating system can use the memory in this region with the
limitation that the device driver(s) owning the region need to be
able to reclaim it back. Typically that means that the operating
system can use that region to store volatile or cached data that
can be otherwise regenerated or migrated elsewhere.
https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/reserved-memory/reserved-memory.yaml#L87
If it's not set, it's not exposed, and I'd expect a coprocessor memory
region wouldn't be flagged as such.
> For other carveout, such as RK3588 HDMI receiver, that is clearly a win, giving
> user the ability to allocate using externally supplied constraints rather then
> having to convince the v4l2 driver to match these. While keeping the safety that
> this carveout will yield valid addresses for the IP.
>
> Will there be a generic way to find out which driver/device this carveout
> belongs to ? In V4L2, only complex cameras have userspace drivers, everything
> else is generic code.
I believe it's a separate discussion, but the current stance is that the
heap name is enough to identify in a platform-specific way where you
allocate from. I've worked on documenting what a good name is so
userspace can pick it up more easily here:
https://lore.kernel.org/r/20250616-dma-buf-heap-names-doc-v2-1-8ae43174cdbf@kernel.org
But it's not really what you expected
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v6 0/2] dma-buf: heaps: Create a CMA heap for each CMA reserved region
2025-07-09 13:38 ` Maxime Ripard
@ 2025-07-10 15:21 ` Nicolas Dufresne
2025-07-11 7:03 ` Maxime Ripard
0 siblings, 1 reply; 13+ messages in thread
From: Nicolas Dufresne @ 2025-07-10 15:21 UTC (permalink / raw)
To: Maxime Ripard
Cc: Rob Herring, Saravana Kannan, Sumit Semwal, Benjamin Gaignard,
Brian Starkey, John Stultz, T.J. Mercier, Christian König,
Krzysztof Kozlowski, Conor Dooley, Marek Szyprowski, Robin Murphy,
Andrew Davis, Jared Kangas, Mattijs Korpershoek, devicetree,
linux-kernel, linux-media, dri-devel, linaro-mm-sig, iommu
[-- Attachment #1: Type: text/plain, Size: 2918 bytes --]
Hi,
Le mercredi 09 juillet 2025 à 15:38 +0200, Maxime Ripard a écrit :
> > Will there be a generic way to find out which driver/device this carveout
> > belongs to ? In V4L2, only complex cameras have userspace drivers,
> > everything
> > else is generic code.
>
> I believe it's a separate discussion, but the current stance is that the
> heap name is enough to identify in a platform-specific way where you
> allocate from. I've worked on documenting what a good name is so
> userspace can pick it up more easily here:
>
> https://lore.kernel.org/r/20250616-dma-buf-heap-names-doc-v2-1-8ae43174cdbf@kernel.org
>
> But it's not really what you expected
From a dma-heap API, the naming rules seems necessary, but suggesting generic
code to use "grep" style of search to match a heap is extremely fragile. The
documentation you propose is (intentionally?) vague. For me, the naming is more
like giving proper names to your function calls do devs can make sense out of
it.
Stepping back a little, we already opened the door for in-driver use of heaps.
So perhaps the way forward is to have V4L2 drivers utilize heaps from inside the
kernel. Once driver are fully ported, additional APIs could be added so that
userspace can read which heap(s) is going to be used for the active
configuration, and which other heaps are known usable (enumerate them). There is
no need to add properties in that context, since these will derives from the
driver configuration you picked. If you told you driver you doing secure memory
playback, the driver will filter-out what can't be used.
Examples out there often express simplified view of the problem. Your ECC video
playback case is a good one. Let's say you have performance issue in both
decoder and display due to ECC. You may think that you just allocate from a non-
ECC heap, import these into the decoder, and once filled, import these into the
display driver and you won.
But in reality, your display buffer might not be the reference buffers, and most
of the memory bandwidth in a modern decoder goes into reading reference frames
and the attached metadata (the later which may or may not be in the same
allocation block).
Even once the reference frames get exposed to userspace (which is a long term
goal), there will still be couple of buffers that just simply don't fit and must
be kept hidden inside the driver.
My general conclusion is that once these heap exists, and that we guarantee
platform specific unique names, we should probably build on top. Both userspace
and driver become consumers of the heap. And for the case where the platform-
specific knowledge lives inside the kernel, then heaps are selected by the
kernel. Also, very little per-driver duplication will be needed, since 90% of
the V4L2 driver share the allocator implementation.
Does that makes any sense to anyone ?
Nicolas
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v6 0/2] dma-buf: heaps: Create a CMA heap for each CMA reserved region
2025-07-10 15:21 ` Nicolas Dufresne
@ 2025-07-11 7:03 ` Maxime Ripard
0 siblings, 0 replies; 13+ messages in thread
From: Maxime Ripard @ 2025-07-11 7:03 UTC (permalink / raw)
To: Nicolas Dufresne
Cc: Rob Herring, Saravana Kannan, Sumit Semwal, Benjamin Gaignard,
Brian Starkey, John Stultz, T.J. Mercier, Christian König,
Krzysztof Kozlowski, Conor Dooley, Marek Szyprowski, Robin Murphy,
Andrew Davis, Jared Kangas, Mattijs Korpershoek, devicetree,
linux-kernel, linux-media, dri-devel, linaro-mm-sig, iommu
[-- Attachment #1: Type: text/plain, Size: 4010 bytes --]
On Thu, Jul 10, 2025 at 11:21:02AM -0400, Nicolas Dufresne wrote:
> Hi,
>
> Le mercredi 09 juillet 2025 à 15:38 +0200, Maxime Ripard a écrit :
> > > Will there be a generic way to find out which driver/device this carveout
> > > belongs to ? In V4L2, only complex cameras have userspace drivers,
> > > everything
> > > else is generic code.
> >
> > I believe it's a separate discussion, but the current stance is that the
> > heap name is enough to identify in a platform-specific way where you
> > allocate from. I've worked on documenting what a good name is so
> > userspace can pick it up more easily here:
> >
> > https://lore.kernel.org/r/20250616-dma-buf-heap-names-doc-v2-1-8ae43174cdbf@kernel.org
> >
> > But it's not really what you expected
>
> From a dma-heap API, the naming rules seems necessary, but suggesting generic
> code to use "grep" style of search to match a heap is extremely fragile. The
> documentation you propose is (intentionally?) vague. For me, the naming is more
> like giving proper names to your function calls do devs can make sense out of
> it.
I agree, and made a proposal to implement some kind of heap capabilities
discovery ioctl. The main concern at the time was that Android tried
that with ION and it lead to a proliferation of poorly defined flags,
and that names were enough to do so.
I still think that at some point we will need this, but I also don't
have a good idea to address these concerns.
> Stepping back a little, we already opened the door for in-driver use of heaps.
> So perhaps the way forward is to have V4L2 drivers utilize heaps from inside the
> kernel. Once driver are fully ported, additional APIs could be added so that
> userspace can read which heap(s) is going to be used for the active
> configuration, and which other heaps are known usable (enumerate them). There is
> no need to add properties in that context, since these will derives from the
> driver configuration you picked. If you told you driver you doing secure memory
> playback, the driver will filter-out what can't be used.
>
> Examples out there often express simplified view of the problem. Your ECC video
> playback case is a good one. Let's say you have performance issue in both
> decoder and display due to ECC. You may think that you just allocate from a non-
> ECC heap, import these into the decoder, and once filled, import these into the
> display driver and you won.
>
> But in reality, your display buffer might not be the reference buffers, and most
> of the memory bandwidth in a modern decoder goes into reading reference frames
> and the attached metadata (the later which may or may not be in the same
> allocation block).
>
> Even once the reference frames get exposed to userspace (which is a long term
> goal), there will still be couple of buffers that just simply don't fit and must
> be kept hidden inside the driver.
>
> My general conclusion is that once these heap exists, and that we guarantee
> platform specific unique names, we should probably build on top. Both userspace
> and driver become consumers of the heap. And for the case where the platform-
> specific knowledge lives inside the kernel, then heaps are selected by the
> kernel. Also, very little per-driver duplication will be needed, since 90% of
> the V4L2 driver share the allocator implementation.
>
> Does that makes any sense to anyone ?
It does, and it's roughly what we have in mind for the cgroups support
in KMS and v4l2. The main issue with it is that knowing if you allocate
from a dedicated pool (which would use the dmem cgroup controller) or
the main memory pool (which would use memcg) wasn't deterministic and
thus you couldn't properly account.
The solution we have in mind right now is indeed to switch everyone to
using heaps, and then exposing which cgroup that heap allocates from.
Your proposal here has a few extra steps, but the main idea is there
still.
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread