linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/3] dma-buf: heaps: Use constant name for CMA heap
@ 2025-05-22 19:14 Jared Kangas
  2025-05-22 19:14 ` [PATCH v3 1/3] Documentation: dma-buf: heaps: Fix code markup Jared Kangas
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Jared Kangas @ 2025-05-22 19:14 UTC (permalink / raw)
  To: sumit.semwal, benjamin.gaignard, Brian.Starkey, jstultz,
	tjmercier, christian.koenig
  Cc: mripard, linux-media, dri-devel, linaro-mm-sig, linux-kernel,
	Jared Kangas

Hi all,

This patch series is based on a previous discussion around CMA heap
naming. [1] The heap's name depends on the device name, which is
generally "reserved", "linux,cma", or "default-pool", but could be any
arbitrary name given to the default CMA area in the devicetree. For a
consistent userspace interface, the series introduces a constant name
for the CMA heap, and for backwards compatibility, an additional Kconfig
that controls the creation of a legacy-named heap with the same CMA
backing.

The ideas to handle backwards compatibility in [1] are to either use a
symlink or add a heap node with a duplicate minor. However, I assume
that we don't want to create symlinks in /dev from module initcalls, and
attempting to duplicate minors would cause device_create() to fail.
Because of these drawbacks, after brainstorming with Maxime Ripard, I
went with creating a new node in devtmpfs with its own minor. This
admittedly makes it a little unclear that the old and new nodes are
backed by the same heap when both are present. The only approach that I
think would provide total clarity on this in userspace is symlinking,
which seemed like a fairly involved solution for devtmpfs, but if I'm
wrong on this, please let me know.

Changelog:

v3:
  - Extract documentation markup fix to separate patch.
  - Adjust DEFAULT_CMA_NAME per discussion in [2].
  - Warn if the legacy heap name and the default heap name are the same.
  - Fix DMABUF_HEAPS_CMA_LEGACY prompt.
  - Touch up commit log wording.

v2:
  - Use tabs instead of spaces for large vertical alignment.

[1]: https://lore.kernel.org/all/f6412229-4606-41ad-8c05-7bbba2eb6e08@ti.com/
[2]: https://lore.kernel.org/all/CANDhNCroe6ZBtN_o=c71kzFFaWK-fF5rCdnr9P5h1sgPOWSGSw@mail.gmail.com/

Jared Kangas (3):
  Documentation: dma-buf: heaps: Fix code markup
  dma-buf: heaps: Parameterize heap name in __add_cma_heap()
  dma-buf: heaps: Give default CMA heap a fixed name

 Documentation/userspace-api/dma-buf-heaps.rst | 11 +++---
 drivers/dma-buf/heaps/Kconfig                 | 10 ++++++
 drivers/dma-buf/heaps/cma_heap.c              | 36 +++++++++++++++----
 3 files changed, 46 insertions(+), 11 deletions(-)

-- 
2.49.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v3 1/3] Documentation: dma-buf: heaps: Fix code markup
  2025-05-22 19:14 [PATCH v3 0/3] dma-buf: heaps: Use constant name for CMA heap Jared Kangas
@ 2025-05-22 19:14 ` Jared Kangas
  2025-05-27 12:59   ` Maxime Ripard
  2025-05-22 19:14 ` [PATCH v3 2/3] dma-buf: heaps: Parameterize heap name in __add_cma_heap() Jared Kangas
  2025-05-22 19:14 ` [PATCH v3 3/3] dma-buf: heaps: Give default CMA heap a fixed name Jared Kangas
  2 siblings, 1 reply; 7+ messages in thread
From: Jared Kangas @ 2025-05-22 19:14 UTC (permalink / raw)
  To: sumit.semwal, benjamin.gaignard, Brian.Starkey, jstultz,
	tjmercier, christian.koenig
  Cc: mripard, linux-media, dri-devel, linaro-mm-sig, linux-kernel,
	Jared Kangas

Code snippets should be wrapped in double backticks to follow
reStructuredText semantics; the use of single backticks uses the
:title-reference: role by default, which isn't quite what we want.
Add double backticks to code snippets to fix this.

Signed-off-by: Jared Kangas <jkangas@redhat.com>
---
 Documentation/userspace-api/dma-buf-heaps.rst | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/userspace-api/dma-buf-heaps.rst b/Documentation/userspace-api/dma-buf-heaps.rst
index 535f49047ce64..23bd0bd7b0654 100644
--- a/Documentation/userspace-api/dma-buf-heaps.rst
+++ b/Documentation/userspace-api/dma-buf-heaps.rst
@@ -19,7 +19,7 @@ following heaps:
  - The ``cma`` heap allocates physically contiguous, cacheable,
    buffers. Only present if a CMA region is present. Such a region is
    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
+   ``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``.
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v3 2/3] dma-buf: heaps: Parameterize heap name in __add_cma_heap()
  2025-05-22 19:14 [PATCH v3 0/3] dma-buf: heaps: Use constant name for CMA heap Jared Kangas
  2025-05-22 19:14 ` [PATCH v3 1/3] Documentation: dma-buf: heaps: Fix code markup Jared Kangas
@ 2025-05-22 19:14 ` Jared Kangas
  2025-05-27 13:00   ` Maxime Ripard
  2025-05-22 19:14 ` [PATCH v3 3/3] dma-buf: heaps: Give default CMA heap a fixed name Jared Kangas
  2 siblings, 1 reply; 7+ messages in thread
From: Jared Kangas @ 2025-05-22 19:14 UTC (permalink / raw)
  To: sumit.semwal, benjamin.gaignard, Brian.Starkey, jstultz,
	tjmercier, christian.koenig
  Cc: mripard, linux-media, dri-devel, linaro-mm-sig, linux-kernel,
	Jared Kangas

Prepare for the introduction of a fixed-name CMA heap by replacing the
unused void pointer parameter in __add_cma_heap() with the heap name.

Signed-off-by: Jared Kangas <jkangas@redhat.com>
---
 drivers/dma-buf/heaps/cma_heap.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma-buf/heaps/cma_heap.c
index 9512d050563a9..e998d8ccd1dc6 100644
--- a/drivers/dma-buf/heaps/cma_heap.c
+++ b/drivers/dma-buf/heaps/cma_heap.c
@@ -366,17 +366,17 @@ static const struct dma_heap_ops cma_heap_ops = {
 	.allocate = cma_heap_allocate,
 };
 
-static int __init __add_cma_heap(struct cma *cma, void *data)
+static int __init __add_cma_heap(struct cma *cma, const char *name)
 {
-	struct cma_heap *cma_heap;
 	struct dma_heap_export_info exp_info;
+	struct cma_heap *cma_heap;
 
 	cma_heap = kzalloc(sizeof(*cma_heap), GFP_KERNEL);
 	if (!cma_heap)
 		return -ENOMEM;
 	cma_heap->cma = cma;
 
-	exp_info.name = cma_get_name(cma);
+	exp_info.name = name;
 	exp_info.ops = &cma_heap_ops;
 	exp_info.priv = cma_heap;
 
@@ -394,12 +394,16 @@ static int __init __add_cma_heap(struct cma *cma, void *data)
 static int __init add_default_cma_heap(void)
 {
 	struct cma *default_cma = dev_get_cma_area(NULL);
-	int ret = 0;
+	int ret;
 
-	if (default_cma)
-		ret = __add_cma_heap(default_cma, NULL);
+	if (!default_cma)
+		return 0;
 
-	return ret;
+	ret = __add_cma_heap(default_cma, cma_get_name(default_cma));
+	if (ret)
+		return ret;
+
+	return 0;
 }
 module_init(add_default_cma_heap);
 MODULE_DESCRIPTION("DMA-BUF CMA Heap");
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v3 3/3] dma-buf: heaps: Give default CMA heap a fixed name
  2025-05-22 19:14 [PATCH v3 0/3] dma-buf: heaps: Use constant name for CMA heap Jared Kangas
  2025-05-22 19:14 ` [PATCH v3 1/3] Documentation: dma-buf: heaps: Fix code markup Jared Kangas
  2025-05-22 19:14 ` [PATCH v3 2/3] dma-buf: heaps: Parameterize heap name in __add_cma_heap() Jared Kangas
@ 2025-05-22 19:14 ` Jared Kangas
  2025-05-27 13:05   ` Maxime Ripard
  2 siblings, 1 reply; 7+ messages in thread
From: Jared Kangas @ 2025-05-22 19:14 UTC (permalink / raw)
  To: sumit.semwal, benjamin.gaignard, Brian.Starkey, jstultz,
	tjmercier, christian.koenig
  Cc: mripard, linux-media, dri-devel, linaro-mm-sig, linux-kernel,
	Jared Kangas

The CMA heap's name in devtmpfs can vary depending on how the heap is
defined. Its name defaults to "reserved", but if a CMA area is defined
in the devicetree, the heap takes on the devicetree node's name, such as
"default-pool" or "linux,cma". To simplify naming, unconditionally name
it "default_cma_region", but keep a legacy node in place backed by the
same underlying allocator for backwards compatibility.

Signed-off-by: Jared Kangas <jkangas@redhat.com>
---
 Documentation/userspace-api/dma-buf-heaps.rst |  7 +++++--
 drivers/dma-buf/heaps/Kconfig                 | 10 ++++++++++
 drivers/dma-buf/heaps/cma_heap.c              | 20 ++++++++++++++++++-
 3 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/Documentation/userspace-api/dma-buf-heaps.rst b/Documentation/userspace-api/dma-buf-heaps.rst
index 23bd0bd7b0654..1dfe5e7acd5a3 100644
--- a/Documentation/userspace-api/dma-buf-heaps.rst
+++ b/Documentation/userspace-api/dma-buf-heaps.rst
@@ -21,5 +21,8 @@ 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``.
+   ``CMA_SIZE_PERCENTAGE`` Kconfig options. The heap's name in devtmpfs is
+   ``default_cma_region``. For backwards compatibility, when the
+   ``DMABUF_HEAPS_CMA_LEGACY`` Kconfig option is set, a duplicate node is
+   created following legacy naming conventions; the legacy name might be
+   ``reserved``, ``linux,cma``, or ``default-pool``.
diff --git a/drivers/dma-buf/heaps/Kconfig b/drivers/dma-buf/heaps/Kconfig
index a5eef06c42264..bb369b38b001a 100644
--- a/drivers/dma-buf/heaps/Kconfig
+++ b/drivers/dma-buf/heaps/Kconfig
@@ -12,3 +12,13 @@ config DMABUF_HEAPS_CMA
 	  Choose this option to enable dma-buf CMA heap. This heap is backed
 	  by the Contiguous Memory Allocator (CMA). If your system has these
 	  regions, you should say Y here.
+
+config DMABUF_HEAPS_CMA_LEGACY
+	bool "Legacy DMA-BUF CMA Heap"
+	default y
+	depends on DMABUF_HEAPS_CMA
+	help
+	  Add a duplicate CMA-backed dma-buf heap with legacy naming derived
+	  from the CMA area's devicetree node, or "reserved" if the area is not
+	  defined in the devicetree. This uses the same underlying allocator as
+	  CONFIG_DMABUF_HEAPS_CMA.
diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma-buf/heaps/cma_heap.c
index e998d8ccd1dc6..dfeccafc6ae3c 100644
--- a/drivers/dma-buf/heaps/cma_heap.c
+++ b/drivers/dma-buf/heaps/cma_heap.c
@@ -9,6 +9,9 @@
  * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/
  *	Andrew F. Davis <afd@ti.com>
  */
+
+#define pr_fmt(fmt) "cma_heap: " fmt
+
 #include <linux/cma.h>
 #include <linux/dma-buf.h>
 #include <linux/dma-heap.h>
@@ -22,6 +25,7 @@
 #include <linux/slab.h>
 #include <linux/vmalloc.h>
 
+#define DEFAULT_CMA_NAME "default_cma_region"
 
 struct cma_heap {
 	struct dma_heap *heap;
@@ -394,15 +398,29 @@ static int __init __add_cma_heap(struct cma *cma, const char *name)
 static int __init add_default_cma_heap(void)
 {
 	struct cma *default_cma = dev_get_cma_area(NULL);
+	const char *legacy_cma_name;
 	int ret;
 
 	if (!default_cma)
 		return 0;
 
-	ret = __add_cma_heap(default_cma, cma_get_name(default_cma));
+	ret = __add_cma_heap(default_cma, DEFAULT_CMA_NAME);
 	if (ret)
 		return ret;
 
+	if (IS_ENABLED(CONFIG_DMABUF_HEAPS_CMA_LEGACY)) {
+		legacy_cma_name = cma_get_name(default_cma);
+		if (!strcmp(legacy_cma_name, DEFAULT_CMA_NAME)) {
+			pr_warn("legacy name and default name are the same, skipping legacy heap\n");
+			return 0;
+		}
+
+		ret = __add_cma_heap(default_cma, legacy_cma_name);
+		if (ret)
+			pr_warn("failed to add legacy heap: %pe\n",
+				ERR_PTR(-ret));
+	}
+
 	return 0;
 }
 module_init(add_default_cma_heap);
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 1/3] Documentation: dma-buf: heaps: Fix code markup
  2025-05-22 19:14 ` [PATCH v3 1/3] Documentation: dma-buf: heaps: Fix code markup Jared Kangas
@ 2025-05-27 12:59   ` Maxime Ripard
  0 siblings, 0 replies; 7+ messages in thread
From: Maxime Ripard @ 2025-05-27 12:59 UTC (permalink / raw)
  To: Jared Kangas
  Cc: Brian.Starkey, benjamin.gaignard, christian.koenig, dri-devel,
	jstultz, linaro-mm-sig, linux-kernel, linux-media, mripard,
	sumit.semwal, tjmercier, Maxime Ripard

On Thu, 22 May 2025 12:14:16 -0700, Jared Kangas wrote:
> Code snippets should be wrapped in double backticks to follow
> reStructuredText semantics; the use of single backticks uses the
> :title-reference: role by default, which isn't quite what we want.
> Add double backticks to code snippets to fix this.
> 
> 
> [ ... ]

Reviewed-by: Maxime Ripard <mripard@kernel.org>

Thanks!
Maxime

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 2/3] dma-buf: heaps: Parameterize heap name in __add_cma_heap()
  2025-05-22 19:14 ` [PATCH v3 2/3] dma-buf: heaps: Parameterize heap name in __add_cma_heap() Jared Kangas
@ 2025-05-27 13:00   ` Maxime Ripard
  0 siblings, 0 replies; 7+ messages in thread
From: Maxime Ripard @ 2025-05-27 13:00 UTC (permalink / raw)
  To: Jared Kangas
  Cc: Brian.Starkey, benjamin.gaignard, christian.koenig, dri-devel,
	jstultz, linaro-mm-sig, linux-kernel, linux-media, mripard,
	sumit.semwal, tjmercier, Maxime Ripard

On Thu, 22 May 2025 12:14:17 -0700, Jared Kangas wrote:
> Prepare for the introduction of a fixed-name CMA heap by replacing the
> unused void pointer parameter in __add_cma_heap() with the heap name.
> 
> Signed-off-by: Jared Kangas <jkangas@redhat.com>

Reviewed-by: Maxime Ripard <mripard@kernel.org>

Thanks!
Maxime

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 3/3] dma-buf: heaps: Give default CMA heap a fixed name
  2025-05-22 19:14 ` [PATCH v3 3/3] dma-buf: heaps: Give default CMA heap a fixed name Jared Kangas
@ 2025-05-27 13:05   ` Maxime Ripard
  0 siblings, 0 replies; 7+ messages in thread
From: Maxime Ripard @ 2025-05-27 13:05 UTC (permalink / raw)
  To: Jared Kangas
  Cc: sumit.semwal, benjamin.gaignard, Brian.Starkey, jstultz,
	tjmercier, christian.koenig, linux-media, dri-devel,
	linaro-mm-sig, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 4428 bytes --]

Hi,

On Thu, May 22, 2025 at 12:14:18PM -0700, Jared Kangas wrote:
> The CMA heap's name in devtmpfs can vary depending on how the heap is
> defined. Its name defaults to "reserved", but if a CMA area is defined
> in the devicetree, the heap takes on the devicetree node's name, such as
> "default-pool" or "linux,cma". To simplify naming, unconditionally name
> it "default_cma_region", but keep a legacy node in place backed by the
> same underlying allocator for backwards compatibility.
> 
> Signed-off-by: Jared Kangas <jkangas@redhat.com>
> ---
>  Documentation/userspace-api/dma-buf-heaps.rst |  7 +++++--
>  drivers/dma-buf/heaps/Kconfig                 | 10 ++++++++++
>  drivers/dma-buf/heaps/cma_heap.c              | 20 ++++++++++++++++++-
>  3 files changed, 34 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/userspace-api/dma-buf-heaps.rst b/Documentation/userspace-api/dma-buf-heaps.rst
> index 23bd0bd7b0654..1dfe5e7acd5a3 100644
> --- a/Documentation/userspace-api/dma-buf-heaps.rst
> +++ b/Documentation/userspace-api/dma-buf-heaps.rst
> @@ -21,5 +21,8 @@ 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``.
> +   ``CMA_SIZE_PERCENTAGE`` Kconfig options. The heap's name in devtmpfs is
> +   ``default_cma_region``. For backwards compatibility, when the
> +   ``DMABUF_HEAPS_CMA_LEGACY`` Kconfig option is set, a duplicate node is
> +   created following legacy naming conventions; the legacy name might be
> +   ``reserved``, ``linux,cma``, or ``default-pool``.
> diff --git a/drivers/dma-buf/heaps/Kconfig b/drivers/dma-buf/heaps/Kconfig
> index a5eef06c42264..bb369b38b001a 100644
> --- a/drivers/dma-buf/heaps/Kconfig
> +++ b/drivers/dma-buf/heaps/Kconfig
> @@ -12,3 +12,13 @@ config DMABUF_HEAPS_CMA
>  	  Choose this option to enable dma-buf CMA heap. This heap is backed
>  	  by the Contiguous Memory Allocator (CMA). If your system has these
>  	  regions, you should say Y here.
> +
> +config DMABUF_HEAPS_CMA_LEGACY
> +	bool "Legacy DMA-BUF CMA Heap"
> +	default y
> +	depends on DMABUF_HEAPS_CMA
> +	help
> +	  Add a duplicate CMA-backed dma-buf heap with legacy naming derived
> +	  from the CMA area's devicetree node, or "reserved" if the area is not
> +	  defined in the devicetree. This uses the same underlying allocator as
> +	  CONFIG_DMABUF_HEAPS_CMA.
> diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma-buf/heaps/cma_heap.c
> index e998d8ccd1dc6..dfeccafc6ae3c 100644
> --- a/drivers/dma-buf/heaps/cma_heap.c
> +++ b/drivers/dma-buf/heaps/cma_heap.c
> @@ -9,6 +9,9 @@
>   * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/
>   *	Andrew F. Davis <afd@ti.com>
>   */
> +
> +#define pr_fmt(fmt) "cma_heap: " fmt
> +
>  #include <linux/cma.h>
>  #include <linux/dma-buf.h>
>  #include <linux/dma-heap.h>
> @@ -22,6 +25,7 @@
>  #include <linux/slab.h>
>  #include <linux/vmalloc.h>
>  
> +#define DEFAULT_CMA_NAME "default_cma_region"
>  
>  struct cma_heap {
>  	struct dma_heap *heap;
> @@ -394,15 +398,29 @@ static int __init __add_cma_heap(struct cma *cma, const char *name)
>  static int __init add_default_cma_heap(void)
>  {
>  	struct cma *default_cma = dev_get_cma_area(NULL);
> +	const char *legacy_cma_name;
>  	int ret;
>  
>  	if (!default_cma)
>  		return 0;
>  
> -	ret = __add_cma_heap(default_cma, cma_get_name(default_cma));
> +	ret = __add_cma_heap(default_cma, DEFAULT_CMA_NAME);
>  	if (ret)
>  		return ret;
>  
> +	if (IS_ENABLED(CONFIG_DMABUF_HEAPS_CMA_LEGACY)) {
> +		legacy_cma_name = cma_get_name(default_cma);
> +		if (!strcmp(legacy_cma_name, DEFAULT_CMA_NAME)) {
> +			pr_warn("legacy name and default name are the same, skipping legacy heap\n");
> +			return 0;
> +		}
> +
> +		ret = __add_cma_heap(default_cma, legacy_cma_name);
> +		if (ret)
> +			pr_warn("failed to add legacy heap: %pe\n",
> +				ERR_PTR(-ret));

Are you sure about the -ret? ret should already be a negative number if it failed?

With that fixed,

Reviewed-by: Maxime Ripard <mripard@kernel.org>

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-05-27 13:05 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-22 19:14 [PATCH v3 0/3] dma-buf: heaps: Use constant name for CMA heap Jared Kangas
2025-05-22 19:14 ` [PATCH v3 1/3] Documentation: dma-buf: heaps: Fix code markup Jared Kangas
2025-05-27 12:59   ` Maxime Ripard
2025-05-22 19:14 ` [PATCH v3 2/3] dma-buf: heaps: Parameterize heap name in __add_cma_heap() Jared Kangas
2025-05-27 13:00   ` Maxime Ripard
2025-05-22 19:14 ` [PATCH v3 3/3] dma-buf: heaps: Give default CMA heap a fixed name Jared Kangas
2025-05-27 13:05   ` Maxime Ripard

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).