Hi,

On Fri, May 01, 2020 at 07:39:46AM +0000, John Stultz wrote:

This patch adds a linux,cma-heap property for CMA reserved memory regions, which will be used to allow the region to be exposed via the DMA-BUF Heaps interface

Cc: Rob Herring robh+dt@kernel.org Cc: Sumit Semwal sumit.semwal@linaro.org Cc: "Andrew F. Davis" afd@ti.com Cc: Benjamin Gaignard benjamin.gaignard@linaro.org Cc: Liam Mark lmark@codeaurora.org Cc: Pratik Patel pratikp@codeaurora.org Cc: Laura Abbott labbott@redhat.com Cc: Brian Starkey Brian.Starkey@arm.com Cc: Chenbo Feng fengc@google.com Cc: Alistair Strachan astrachan@google.com Cc: Sandeep Patil sspatil@google.com Cc: Hridya Valsaraju hridya@google.com Cc: Christoph Hellwig hch@lst.de Cc: Marek Szyprowski m.szyprowski@samsung.com Cc: Robin Murphy robin.murphy@arm.com Cc: Andrew Morton akpm@linux-foundation.org Cc: devicetree@vger.kernel.org Cc: dri-devel@lists.freedesktop.org Cc: linux-mm@kvack.org Signed-off-by: John Stultz john.stultz@linaro.org

---

.../devicetree/bindings/reserved-memory/reserved-memory.txt | 3 +++ 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt index bac4afa3b197..e97b6a4c3bc0 100644 --- a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt +++ b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt @@ -68,6 +68,9 @@ Linux implementation note:

• If a "linux,cma-default" property is present, then Linux will use the region for the default pool of the contiguous memory allocator.

+- If a "linux,cma-heap" property is present, then Linux will expose the

• the CMA region via the DMA-BUF Heaps interface.

Would it be useful or even possible to give some indication of what the heap will end up being called? I'm afraid I don't remember what if any conclusions came out of previous discussions on UAPI for heap enumeration.

I suppose CMA names haven't been relevant to userspace before, but they perhaps would be with this change.

Alternatively, leaving it effectively undefined doesn't tie us down, and something like links in sysfs can be added as a richer API in the future.

Cheers, -Brian

• If a "linux,dma-default" property is present, then Linux will use the region for the default pool of the consistent DMA allocator.

-- 2.17.1

On Fri, May 01, 2020 at 11:40:16AM -0700, John Stultz wrote:

On Fri, May 1, 2020 at 3:42 AM Brian Starkey brian.starkey@arm.com wrote:

...

Hi,

On Fri, May 01, 2020 at 07:39:46AM +0000, John Stultz wrote:

...

This patch adds a linux,cma-heap property for CMA reserved memory regions, which will be used to allow the region to be exposed via the DMA-BUF Heaps interface

Cc: Rob Herring robh+dt@kernel.org Cc: Sumit Semwal sumit.semwal@linaro.org Cc: "Andrew F. Davis" afd@ti.com Cc: Benjamin Gaignard benjamin.gaignard@linaro.org Cc: Liam Mark lmark@codeaurora.org Cc: Pratik Patel pratikp@codeaurora.org Cc: Laura Abbott labbott@redhat.com Cc: Brian Starkey Brian.Starkey@arm.com Cc: Chenbo Feng fengc@google.com Cc: Alistair Strachan astrachan@google.com Cc: Sandeep Patil sspatil@google.com Cc: Hridya Valsaraju hridya@google.com Cc: Christoph Hellwig hch@lst.de Cc: Marek Szyprowski m.szyprowski@samsung.com Cc: Robin Murphy robin.murphy@arm.com Cc: Andrew Morton akpm@linux-foundation.org Cc: devicetree@vger.kernel.org Cc: dri-devel@lists.freedesktop.org Cc: linux-mm@kvack.org Signed-off-by: John Stultz john.stultz@linaro.org

---

.../devicetree/bindings/reserved-memory/reserved-memory.txt | 3 +++ 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt index bac4afa3b197..e97b6a4c3bc0 100644 --- a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt +++ b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt @@ -68,6 +68,9 @@ Linux implementation note:

• If a "linux,cma-default" property is present, then Linux will use the region for the default pool of the contiguous memory allocator.

+- If a "linux,cma-heap" property is present, then Linux will expose the

• the CMA region via the DMA-BUF Heaps interface.

Would it be useful or even possible to give some indication of what the heap will end up being called? I'm afraid I don't remember what if any conclusions came out of previous discussions on UAPI for heap enumeration.

So the name we expose is the CMA name itself. So with dt it will be the name of the reserved memory node that the flag property is added to.

Yeah I'm just wondering if that's "stable" so we can say "the heap will use the node name", or if saying that would cause us a headache in the future.

...

I suppose CMA names haven't been relevant to userspace before, but they perhaps would be with this change.

Alternatively, leaving it effectively undefined doesn't tie us down, and something like links in sysfs can be added as a richer API in the future.

Hrm. Mind expanding on what you're thinking here?

Super hand-wavy, something like:

/sys/devices/blah/display@2f000000/cma_region is a symlink to /sys/class/dma_heaps/heap_display

I think danvet had some thoughts in this vein.

Cheers, -Brian

thanks -john

On Fri, May 01, 2020 at 07:39:47AM +0000, John Stultz wrote:

This patch adds a dma_heap flag on the cma structure, along with accessors to set and read the flag.

This is then used to process and store the "linux,cma-heap" property documented in the previous patch.

Cc: Rob Herring robh+dt@kernel.org Cc: Sumit Semwal sumit.semwal@linaro.org Cc: "Andrew F. Davis" afd@ti.com Cc: Benjamin Gaignard benjamin.gaignard@linaro.org Cc: Liam Mark lmark@codeaurora.org Cc: Pratik Patel pratikp@codeaurora.org Cc: Laura Abbott labbott@redhat.com Cc: Brian Starkey Brian.Starkey@arm.com Cc: Chenbo Feng fengc@google.com Cc: Alistair Strachan astrachan@google.com Cc: Sandeep Patil sspatil@google.com Cc: Hridya Valsaraju hridya@google.com Cc: Christoph Hellwig hch@lst.de Cc: Marek Szyprowski m.szyprowski@samsung.com Cc: Robin Murphy robin.murphy@arm.com Cc: Andrew Morton akpm@linux-foundation.org Cc: devicetree@vger.kernel.org Cc: dri-devel@lists.freedesktop.org Cc: linux-mm@kvack.org Signed-off-by: John Stultz john.stultz@linaro.org

---

include/linux/cma.h | 3 +++ kernel/dma/contiguous.c | 3 +++ mm/cma.c | 11 +++++++++++ mm/cma.h | 1 + 4 files changed, 18 insertions(+)

diff --git a/include/linux/cma.h b/include/linux/cma.h index 6ff79fefd01f..d8b8e6ce221c 100644 --- a/include/linux/cma.h +++ b/include/linux/cma.h @@ -25,6 +25,9 @@ extern phys_addr_t cma_get_base(const struct cma *cma); extern unsigned long cma_get_size(const struct cma *cma); extern const char *cma_get_name(const struct cma *cma);

+extern void __init cma_enable_dma_heap(struct cma *cma, bool enabled); +extern bool cma_dma_heap_enabled(struct cma *cma);

extern int __init cma_declare_contiguous_nid(phys_addr_t base, phys_addr_t size, phys_addr_t limit, phys_addr_t alignment, unsigned int order_per_bit, diff --git a/kernel/dma/contiguous.c b/kernel/dma/contiguous.c index 8bc6f2d670f9..f667fd51daa2 100644 --- a/kernel/dma/contiguous.c +++ b/kernel/dma/contiguous.c @@ -303,6 +303,7 @@ static int __init rmem_cma_setup(struct reserved_mem *rmem) phys_addr_t mask = align - 1; unsigned long node = rmem->fdt_node; bool default_cma = of_get_flat_dt_prop(node, "linux,cma-default", NULL);

• bool heap_exported = of_get_flat_dt_prop(node, "linux,cma-heap", NULL); struct cma *cma; int err;

@@ -332,6 +333,8 @@ static int __init rmem_cma_setup(struct reserved_mem *rmem) if (default_cma) dma_contiguous_set_default(cma);

• cma_enable_dma_heap(cma, heap_exported);
• rmem->ops = &rmem_cma_ops; rmem->priv = cma;

diff --git a/mm/cma.c b/mm/cma.c index 0463ad2ce06b..ec671bd8f66e 100644 --- a/mm/cma.c +++ b/mm/cma.c @@ -55,6 +55,16 @@ const char *cma_get_name(const struct cma *cma) return cma->name ? cma->name : "(undefined)"; }

+void __init cma_enable_dma_heap(struct cma *cma, bool enabled) +{

• cma->dma_heap = enabled;

+}

+bool cma_dma_heap_enabled(struct cma *cma) +{

• return !!cma->dma_heap;

Stylistic thing, but I don't think the !! is really necessary. It's already a bool anyway.

+}

static unsigned long cma_bitmap_aligned_mask(const struct cma *cma, unsigned int align_order) { @@ -157,6 +167,7 @@ static int __init cma_init_reserved_areas(void) } core_initcall(cma_init_reserved_areas);

nit: spurious newline

Cheers, -Brian

/**

• cma_init_reserved_mem() - create custom contiguous area from reserved memory
• @base: Base address of the reserved area

diff --git a/mm/cma.h b/mm/cma.h index 33c0b517733c..6fe2242c724f 100644 --- a/mm/cma.h +++ b/mm/cma.h @@ -13,6 +13,7 @@ struct cma { spinlock_t mem_head_lock; #endif const char *name;

• bool dma_heap;

};

extern struct cma cma_areas[MAX_CMA_AREAS];

2.17.1

On 2020-05-01 11:21 am, Brian Starkey wrote:

Hi John,

On Fri, May 01, 2020 at 07:39:48AM +0000, John Stultz wrote:

...

This patch reworks the cma_heap initialization so that we expose both the default CMA region and any CMA regions tagged with "linux,cma-heap" in the device-tree.

Cc: Rob Herring robh+dt@kernel.org Cc: Sumit Semwal sumit.semwal@linaro.org Cc: "Andrew F. Davis" afd@ti.com Cc: Benjamin Gaignard benjamin.gaignard@linaro.org Cc: Liam Mark lmark@codeaurora.org Cc: Pratik Patel pratikp@codeaurora.org Cc: Laura Abbott labbott@redhat.com Cc: Brian Starkey Brian.Starkey@arm.com Cc: Chenbo Feng fengc@google.com Cc: Alistair Strachan astrachan@google.com Cc: Sandeep Patil sspatil@google.com Cc: Hridya Valsaraju hridya@google.com Cc: Christoph Hellwig hch@lst.de Cc: Marek Szyprowski m.szyprowski@samsung.com Cc: Robin Murphy robin.murphy@arm.com Cc: Andrew Morton akpm@linux-foundation.org Cc: devicetree@vger.kernel.org Cc: dri-devel@lists.freedesktop.org Cc: linux-mm@kvack.org Signed-off-by: John Stultz john.stultz@linaro.org

---

drivers/dma-buf/heaps/cma_heap.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma-buf/heaps/cma_heap.c index 626cf7fd033a..dd154e2db101 100644 --- a/drivers/dma-buf/heaps/cma_heap.c +++ b/drivers/dma-buf/heaps/cma_heap.c @@ -141,6 +141,11 @@ static int __add_cma_heap(struct cma *cma, void *data) { struct cma_heap *cma_heap; struct dma_heap_export_info exp_info;

• struct cma *default_cma = dev_get_cma_area(NULL);
• /* We only add the default heap and explicitly tagged heaps */
• if (cma != default_cma && !cma_dma_heap_enabled(cma))

• return 0;
  

Thinking about the pl111 thread[1], I'm wondering if we should also let drivers call this directly to expose their CMA pools, even if they aren't tagged for dma-heaps in DT. But perhaps that's too close to policy.

That sounds much like what my first thoughts were - apologies if I'm wildly off-base here, but as far as I understand:

- Device drivers know whether they have their own "memory-region" or not. - Device drivers already have to do *something* to participate in dma-buf. - Device drivers know best how they make use of both the above. - Therefore couldn't it be left to drivers to choose whether to register their CMA regions as heaps, without having to mess with DT at all?

Robin.

Cheers, -Brian

[1] https://lists.freedesktop.org/archives/dri-devel/2020-April/264358.html

...

cma_heap = kzalloc(sizeof(*cma_heap), GFP_KERNEL); if (!cma_heap) @@ -162,16 +167,11 @@ static int __add_cma_heap(struct cma *cma, void *data) return 0; }

-static int add_default_cma_heap(void) +static int cma_heaps_init(void) {

• struct cma *default_cma = dev_get_cma_area(NULL);
• int ret = 0;
• if (default_cma)

• ret = __add_cma_heap(default_cma, NULL);
  

• return ret;

• cma_for_each_area(__add_cma_heap, NULL);
• return 0; }

-module_init(add_default_cma_heap); +module_init(cma_heaps_init); MODULE_DESCRIPTION("DMA-BUF CMA Heap"); MODULE_LICENSE("GPL v2"); -- 2.17.1

On Fri, May 01, 2020 at 12:01:40PM -0700, John Stultz wrote:

On Fri, May 1, 2020 at 4:08 AM Robin Murphy robin.murphy@arm.com wrote:

...

On 2020-05-01 11:21 am, Brian Starkey wrote:

...

Hi John,

On Fri, May 01, 2020 at 07:39:48AM +0000, John Stultz wrote:

...

This patch reworks the cma_heap initialization so that we expose both the default CMA region and any CMA regions tagged with "linux,cma-heap" in the device-tree.

Cc: Rob Herring robh+dt@kernel.org Cc: Sumit Semwal sumit.semwal@linaro.org Cc: "Andrew F. Davis" afd@ti.com Cc: Benjamin Gaignard benjamin.gaignard@linaro.org Cc: Liam Mark lmark@codeaurora.org Cc: Pratik Patel pratikp@codeaurora.org Cc: Laura Abbott labbott@redhat.com Cc: Brian Starkey Brian.Starkey@arm.com Cc: Chenbo Feng fengc@google.com Cc: Alistair Strachan astrachan@google.com Cc: Sandeep Patil sspatil@google.com Cc: Hridya Valsaraju hridya@google.com Cc: Christoph Hellwig hch@lst.de Cc: Marek Szyprowski m.szyprowski@samsung.com Cc: Robin Murphy robin.murphy@arm.com Cc: Andrew Morton akpm@linux-foundation.org Cc: devicetree@vger.kernel.org Cc: dri-devel@lists.freedesktop.org Cc: linux-mm@kvack.org Signed-off-by: John Stultz john.stultz@linaro.org

---

drivers/dma-buf/heaps/cma_heap.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma-buf/heaps/cma_heap.c index 626cf7fd033a..dd154e2db101 100644 --- a/drivers/dma-buf/heaps/cma_heap.c +++ b/drivers/dma-buf/heaps/cma_heap.c @@ -141,6 +141,11 @@ static int __add_cma_heap(struct cma *cma, void *data) { struct cma_heap *cma_heap; struct dma_heap_export_info exp_info;

• struct cma *default_cma = dev_get_cma_area(NULL);
• /* We only add the default heap and explicitly tagged heaps */
• if (cma != default_cma && !cma_dma_heap_enabled(cma))

•        return 0;
  

Thinking about the pl111 thread[1], I'm wondering if we should also let drivers call this directly to expose their CMA pools, even if they aren't tagged for dma-heaps in DT. But perhaps that's too close to policy.

That sounds much like what my first thoughts were - apologies if I'm wildly off-base here, but as far as I understand:

• Device drivers know whether they have their own "memory-region" or not.
• Device drivers already have to do *something* to participate in dma-buf.
• Device drivers know best how they make use of both the above.
• Therefore couldn't it be left to drivers to choose whether to register

their CMA regions as heaps, without having to mess with DT at all?

I guess I'm not opposed to this. But I guess I'd like to see some more details? You're thinking the pl111 driver would add the "memory-region" node itself?

Assuming that's the case, my only worry is what if that memory-region node isn't a CMA area, but instead something like a carveout? Does the driver need to parse enough of the dt to figure out where to register the region as a heap?

My thinking was more like there would already be a reserved-memory node in DT for the chunk of memory, appropriately tagged so that it gets added as a CMA region.

The device's node would have "memory-region=<&blah>;" and would use of_reserved_mem_device_init() to link up dev->cma_area to the corresponding cma region.

So far, that's all in-place already. The bit that's missing is exposing that dev->cma_area to userspace as a dma_heap - so we could just have "int cma_heap_add(struct cma *cma)" or "int cma_heap_dev_add(struct device *dev)" or something exported for drivers to expose their device-assigned cma region if they wanted to.

I don't think this runs into the lifetime problems of generalised heaps-as-modules either, because the CMA region will never go away even if the driver does.

Alongside that, I do think the completely DT-driven approach can be useful too - because there may be regions which aren't associated with any specific device driver, that we want exported as heaps.

Hope that makes sense, -Brian

thanks -john

Hi Rob,

On Tue, May 12, 2020 at 11:37:14AM -0500, Rob Herring wrote:

On Mon, May 04, 2020 at 10:06:28AM +0100, Brian Starkey wrote:

...

On Fri, May 01, 2020 at 12:01:40PM -0700, John Stultz wrote:

...

On Fri, May 1, 2020 at 4:08 AM Robin Murphy robin.murphy@arm.com wrote:

...

On 2020-05-01 11:21 am, Brian Starkey wrote:

...

Hi John,

On Fri, May 01, 2020 at 07:39:48AM +0000, John Stultz wrote:

...

This patch reworks the cma_heap initialization so that we expose both the default CMA region and any CMA regions tagged with "linux,cma-heap" in the device-tree.

Cc: Rob Herring robh+dt@kernel.org Cc: Sumit Semwal sumit.semwal@linaro.org Cc: "Andrew F. Davis" afd@ti.com Cc: Benjamin Gaignard benjamin.gaignard@linaro.org Cc: Liam Mark lmark@codeaurora.org Cc: Pratik Patel pratikp@codeaurora.org Cc: Laura Abbott labbott@redhat.com Cc: Brian Starkey Brian.Starkey@arm.com Cc: Chenbo Feng fengc@google.com Cc: Alistair Strachan astrachan@google.com Cc: Sandeep Patil sspatil@google.com Cc: Hridya Valsaraju hridya@google.com Cc: Christoph Hellwig hch@lst.de Cc: Marek Szyprowski m.szyprowski@samsung.com Cc: Robin Murphy robin.murphy@arm.com Cc: Andrew Morton akpm@linux-foundation.org Cc: devicetree@vger.kernel.org Cc: dri-devel@lists.freedesktop.org Cc: linux-mm@kvack.org Signed-off-by: John Stultz john.stultz@linaro.org

---

drivers/dma-buf/heaps/cma_heap.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma-buf/heaps/cma_heap.c index 626cf7fd033a..dd154e2db101 100644 --- a/drivers/dma-buf/heaps/cma_heap.c +++ b/drivers/dma-buf/heaps/cma_heap.c @@ -141,6 +141,11 @@ static int __add_cma_heap(struct cma *cma, void *data) { struct cma_heap *cma_heap; struct dma_heap_export_info exp_info;

• struct cma *default_cma = dev_get_cma_area(NULL);
• /* We only add the default heap and explicitly tagged heaps */
• if (cma != default_cma && !cma_dma_heap_enabled(cma))

•        return 0;
  

Thinking about the pl111 thread[1], I'm wondering if we should also let drivers call this directly to expose their CMA pools, even if they aren't tagged for dma-heaps in DT. But perhaps that's too close to policy.

That sounds much like what my first thoughts were - apologies if I'm wildly off-base here, but as far as I understand:

• Device drivers know whether they have their own "memory-region" or not.
• Device drivers already have to do *something* to participate in dma-buf.
• Device drivers know best how they make use of both the above.
• Therefore couldn't it be left to drivers to choose whether to register

their CMA regions as heaps, without having to mess with DT at all?

+1, but I'm biased toward any solution not using DT. :)

...

...

I guess I'm not opposed to this. But I guess I'd like to see some more details? You're thinking the pl111 driver would add the "memory-region" node itself?

Assuming that's the case, my only worry is what if that memory-region node isn't a CMA area, but instead something like a carveout? Does the driver need to parse enough of the dt to figure out where to register the region as a heap?

My thinking was more like there would already be a reserved-memory node in DT for the chunk of memory, appropriately tagged so that it gets added as a CMA region.

The device's node would have "memory-region=<&blah>;" and would use of_reserved_mem_device_init() to link up dev->cma_area to the corresponding cma region.

So far, that's all in-place already. The bit that's missing is exposing that dev->cma_area to userspace as a dma_heap - so we could just have "int cma_heap_add(struct cma *cma)" or "int cma_heap_dev_add(struct device *dev)" or something exported for drivers to expose their device-assigned cma region if they wanted to.

I don't think this runs into the lifetime problems of generalised heaps-as-modules either, because the CMA region will never go away even if the driver does.

Alongside that, I do think the completely DT-driven approach can be useful too - because there may be regions which aren't associated with any specific device driver, that we want exported as heaps.

And they are associated with the hardware description rather than the userspace environment?

I'm not sure how to answer that. We already have CMA regions being created from device-tree, so we're only talking about explicitly exposing those to userspace.

Are you thinking that userspace should be deciding whether they get exposed or not? I don't know how userspace would discover them in order to make that decision.

Thanks, -Brian

Rob

On Thu, May 14, 2020 at 09:52:35AM -0500, Rob Herring wrote:

On Wed, May 13, 2020 at 5:44 AM Brian Starkey brian.starkey@arm.com wrote:

...

Hi Rob,

On Tue, May 12, 2020 at 11:37:14AM -0500, Rob Herring wrote:

...

On Mon, May 04, 2020 at 10:06:28AM +0100, Brian Starkey wrote:

...

On Fri, May 01, 2020 at 12:01:40PM -0700, John Stultz wrote:

...

On Fri, May 1, 2020 at 4:08 AM Robin Murphy robin.murphy@arm.com wrote:

...

On 2020-05-01 11:21 am, Brian Starkey wrote: > Hi John, > > On Fri, May 01, 2020 at 07:39:48AM +0000, John Stultz wrote: >> This patch reworks the cma_heap initialization so that >> we expose both the default CMA region and any CMA regions >> tagged with "linux,cma-heap" in the device-tree. >> >> Cc: Rob Herring robh+dt@kernel.org >> Cc: Sumit Semwal sumit.semwal@linaro.org >> Cc: "Andrew F. Davis" afd@ti.com >> Cc: Benjamin Gaignard benjamin.gaignard@linaro.org >> Cc: Liam Mark lmark@codeaurora.org >> Cc: Pratik Patel pratikp@codeaurora.org >> Cc: Laura Abbott labbott@redhat.com >> Cc: Brian Starkey Brian.Starkey@arm.com >> Cc: Chenbo Feng fengc@google.com >> Cc: Alistair Strachan astrachan@google.com >> Cc: Sandeep Patil sspatil@google.com >> Cc: Hridya Valsaraju hridya@google.com >> Cc: Christoph Hellwig hch@lst.de >> Cc: Marek Szyprowski m.szyprowski@samsung.com >> Cc: Robin Murphy robin.murphy@arm.com >> Cc: Andrew Morton akpm@linux-foundation.org >> Cc: devicetree@vger.kernel.org >> Cc: dri-devel@lists.freedesktop.org >> Cc: linux-mm@kvack.org >> Signed-off-by: John Stultz john.stultz@linaro.org >> --- >> drivers/dma-buf/heaps/cma_heap.c | 18 +++++++++--------- >> 1 file changed, 9 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma-buf/heaps/cma_heap.c >> index 626cf7fd033a..dd154e2db101 100644 >> --- a/drivers/dma-buf/heaps/cma_heap.c >> +++ b/drivers/dma-buf/heaps/cma_heap.c >> @@ -141,6 +141,11 @@ static int __add_cma_heap(struct cma *cma, void *data) >> { >> struct cma_heap *cma_heap; >> struct dma_heap_export_info exp_info; >> + struct cma *default_cma = dev_get_cma_area(NULL); >> + >> + /* We only add the default heap and explicitly tagged heaps */ >> + if (cma != default_cma && !cma_dma_heap_enabled(cma)) >> + return 0; > > Thinking about the pl111 thread[1], I'm wondering if we should also > let drivers call this directly to expose their CMA pools, even if they > aren't tagged for dma-heaps in DT. But perhaps that's too close to > policy.

That sounds much like what my first thoughts were - apologies if I'm wildly off-base here, but as far as I understand:

• Device drivers know whether they have their own "memory-region" or not.
• Device drivers already have to do *something* to participate in dma-buf.
• Device drivers know best how they make use of both the above.
• Therefore couldn't it be left to drivers to choose whether to register

their CMA regions as heaps, without having to mess with DT at all?

+1, but I'm biased toward any solution not using DT. :)

...

...

I guess I'm not opposed to this. But I guess I'd like to see some more details? You're thinking the pl111 driver would add the "memory-region" node itself?

Assuming that's the case, my only worry is what if that memory-region node isn't a CMA area, but instead something like a carveout? Does the driver need to parse enough of the dt to figure out where to register the region as a heap?

My thinking was more like there would already be a reserved-memory node in DT for the chunk of memory, appropriately tagged so that it gets added as a CMA region.

The device's node would have "memory-region=<&blah>;" and would use of_reserved_mem_device_init() to link up dev->cma_area to the corresponding cma region.

So far, that's all in-place already. The bit that's missing is exposing that dev->cma_area to userspace as a dma_heap - so we could just have "int cma_heap_add(struct cma *cma)" or "int cma_heap_dev_add(struct device *dev)" or something exported for drivers to expose their device-assigned cma region if they wanted to.

I don't think this runs into the lifetime problems of generalised heaps-as-modules either, because the CMA region will never go away even if the driver does.

Alongside that, I do think the completely DT-driven approach can be useful too - because there may be regions which aren't associated with any specific device driver, that we want exported as heaps.

And they are associated with the hardware description rather than the userspace environment?

I'm not sure how to answer that. We already have CMA regions being created from device-tree, so we're only talking about explicitly exposing those to userspace.

It's easier to argue that how much CMA memory a system/device needs is h/w description as that's more a function of devices and frame sizes. But exposing to userspace or not is an OS architecture decision. It's not really any different than a kernel vs. userspace driver question. What's exposed by UIO or spi-dev is purely a kernel thing.

...

Are you thinking that userspace should be deciding whether they get exposed or not? I don't know how userspace would discover them in order to make that decision.

Or perhaps the kernel should be deciding. Expose to userspace what the kernel doesn't need or drivers decide?

If not via device-tree how would the kernel make that decision? For regions which get "claimed" by a device/driver, obviously that driver can make the decision, and I totally think we should provide a way for them to expose it.

But for something like a shared pool amongst the media subsystem, there may not be any specific single device/driver - and creating a dummy driver with the sole purpose of exposing the heap doesn't seem like an improvement over a simple "linux,cma-heap".

Cheers, -Brian

It's hard to argue against 1 new property. It's death by a 1000 cuts though.

Rob