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