[PATCH] dma-buf: heaps: Introduce dma_heap_add_cma() for non-default CMA heap
Current dma-buf heaps can handle only default CMA. This introduces dma_heap_add_cma() function to attach CMA heaps that belongs to a device. At first, the driver calls of_reserved_mem_device_init() to set memory-region property associated with reserved-memory defined as CMA to the device. And when the driver calls this dma_heap_add_cma(), the CMA will be added to dma-buf heaps. For example, prepare CMA node named "linux,cma@1000" and specify the node for memory-region property. After the above calls in the driver, a device file "/dev/dma_heap/linux,cma@1000" associated with the CMA become available as dma-buf heaps. Signed-off-by: Kunihiko Hayashi --- drivers/dma-buf/heaps/cma_heap.c | 12 include/linux/dma-heap.h | 9 + 2 files changed, 21 insertions(+) diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma-buf/heaps/cma_heap.c index 626cf7f..5d2442e 100644 --- a/drivers/dma-buf/heaps/cma_heap.c +++ b/drivers/dma-buf/heaps/cma_heap.c @@ -162,6 +162,18 @@ static int __add_cma_heap(struct cma *cma, void *data) return 0; } +/* add device CMA heap to dmabuf heaps */ +int dma_heap_add_cma(struct device *dev) +{ + struct cma *cma = dev_get_cma_area(dev); + + if (!cma) + return -ENOMEM; + + return __add_cma_heap(cma, NULL); +} +EXPORT_SYMBOL_GPL(dma_heap_add_cma); + static int add_default_cma_heap(void) { struct cma *default_cma = dev_get_cma_area(NULL); diff --git a/include/linux/dma-heap.h b/include/linux/dma-heap.h index 454e354..16bec7d 100644 --- a/include/linux/dma-heap.h +++ b/include/linux/dma-heap.h @@ -56,4 +56,13 @@ void *dma_heap_get_drvdata(struct dma_heap *heap); */ struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info); +#ifdef CONFIG_DMABUF_HEAPS_CMA +/** + * dma_heap_add_cma - adds a device CMA heap to dmabuf heaps + * @dev: device with a CMA heap to register + */ +int dma_heap_add_cma(struct device *dev); + +#endif /* CONFIG_DMABUF_HEAPS_CMA */ + #endif /* _DMA_HEAPS_H */ -- 2.7.4
Re: [PATCH] dma-buf: heaps: Introduce dma_heap_add_cma() for non-default CMA heap
Hi John, Thank you for your comment. On 2020/07/29 4:17, John Stultz wrote: On Thu, Jul 16, 2020 at 6:10 PM Kunihiko Hayashi wrote: Current dma-buf heaps can handle only default CMA. This introduces dma_heap_add_cma() function to attach CMA heaps that belongs to a device. At first, the driver calls of_reserved_mem_device_init() to set memory-region property associated with reserved-memory defined as CMA to the device. And when the driver calls this dma_heap_add_cma(), the CMA will be added to dma-buf heaps. For example, prepare CMA node named "linux,cma@1000" and specify the node for memory-region property. After the above calls in the driver, a device file "/dev/dma_heap/linux,cma@1000" associated with the CMA become available as dma-buf heaps. Signed-off-by: Kunihiko Hayashi --- drivers/dma-buf/heaps/cma_heap.c | 12 include/linux/dma-heap.h | 9 + 2 files changed, 21 insertions(+) Hey! Sorry for the slow response on this! I just realized I never replied! I waited but no problem. diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma-buf/heaps/cma_heap.c index 626cf7f..5d2442e 100644 --- a/drivers/dma-buf/heaps/cma_heap.c +++ b/drivers/dma-buf/heaps/cma_heap.c [snip] -- 2.7.4 Looks sane to me. Being able to expose different multiple CMA heaps is needed, and I agree this way (as opposed to my earlier dts appraoch) is probably the best approach. The only bit I'm so-so on is adding the CMA heap specific call in the dma-heap.h, but at the same time I can't justify adding a whole new header for a single function. I'm also a little worried about whether "CMA specific" call is in the dma-heap.h, however I can't find another solution. Do you have a upstream driver that you plan to make use this new call? Unfortunately I don't have an upstream driver using this call. This call is called from dma-buf heaps "importer" or "customer", and I only made an example (do nothing) importer driver to test the call. We want to have in-tree users of code added. I think this is a generic way to use non-default CMA heaps, however, we need in-tree "importer" drivers to want to use non-default CMA heaps. I don't find it from now. But if so, feel free to add my: Acked-by: John Stultz To this patch when you submit the driver changes. Thank you, --- Best Regards Kunihiko Hayashi
Re: [PATCH] dma-buf: heaps: Introduce dma_heap_add_cma() for non-default CMA heap
On Fri, Jul 31, 2020 at 2:32 AM Kunihiko Hayashi wrote: > On 2020/07/29 4:17, John Stultz wrote: > > Do you have a upstream driver that you plan to make use this new call? > > Unfortunately I don't have an upstream driver using this call. > > This call is called from dma-buf heaps "importer" or "customer", > and I only made an example (do nothing) importer driver > to test the call. > > > We want to have in-tree users of code added. > > I think this is a generic way to use non-default CMA heaps, however, > we need in-tree "importer" drivers to want to use non-default CMA heaps. > I don't find it from now. > Yea, I and again, I do agree this is functionality that will be needed. But we'll need to wait for a user (camera driver, etc which would utilize the reserved cma region) before we can merge it upstream. :( Do let me know if you have an out of tree driver that would make use of it, and we can see what can be done to help upstream things. thanks -john
Re: [PATCH] dma-buf: heaps: Introduce dma_heap_add_cma() for non-default CMA heap
Hi John, On 2020/08/22 3:32, John Stultz wrote: On Fri, Aug 21, 2020 at 2:14 AM Kunihiko Hayashi wrote: On 2020/08/01 4:38, John Stultz wrote: On Fri, Jul 31, 2020 at 2:32 AM Kunihiko Hayashi wrote: On 2020/07/29 4:17, John Stultz wrote: Do you have a upstream driver that you plan to make use this new call? Unfortunately I don't have an upstream driver using this call. This call is called from dma-buf heaps "importer" or "customer", and I only made an example (do nothing) importer driver to test the call. We want to have in-tree users of code added. I think this is a generic way to use non-default CMA heaps, however, we need in-tree "importer" drivers to want to use non-default CMA heaps. I don't find it from now. Yea, I and again, I do agree this is functionality that will be needed. But we'll need to wait for a user (camera driver, etc which would utilize the reserved cma region) before we can merge it upstream. :( Do let me know if you have an out of tree driver that would make use of it, and we can see what can be done to help upstream things. Sorry for late. Before I prepare or find a user driver as "importer", I think something is different in this patch. This patch makes it possible to treat non-default CMA connected to "importer" device with memory-region as dma-buf heaps. However, the allocated memory from this dma-buf heaps can be used for "any" devices, and the "importer" can treat memories from other dma-buf heaps. So, the "importer" and the non-default CMA aren't directly related, and I think an "exporter" for the non-default CMA should be enabled. In paticular, the kernel initializer (as an "exporter") calls dma_heap_add_cma() for all CMAs defined in Devicetree, and the device files associated with each CMA appear under "/dev/dma_heap/". For example: /dev/dma_heap/linux,cma@1000 /dev/dma_heap/linux,cma@1100 /dev/dma_heap/linux,cma@1200 ... All of these device files can be fairly allocated to any "importer" device. Actually I think that the kernel should executes dma_heap_add_cma() for ALL defined reserved-memory nodes. If this idea hasn't been discussed yet and this is reasonable, I'll prepare RFC patches. So yes! An earlier version of the CMA heap I submitted did add all CMA regions as accessible heaps as you propose here. Sorry I've missed your submitted patch and previous discussions. However, the concern was that in some cases, those regions are device specific reserved memory that the driver is probably expecting to have exclusive access. To allow (sufficiently privileged, or misconfigured) userland to be able to allocate out of that seemed like it might cause trouble, and instead we should have CMA regions explicitly exported. Ah, I see. Surely if "device-specific" memory is specified as CMA in the devicetree, it's difficult to avoid any trouble to access it. There was some proposal to add a dt property to the reserved memory section (similar to linux,cma-default) and use that to do the exporting, but other discussions seemed to prefer having drivers export it explicitly in a fashion very similar to what your earlier patch does. The only trouble is we just need an upstream driver to add such a call in the series before we can merge it. Okay, I understand. I agree that it seems safe for upstream driver to handle memory that is only used explicitly. However, I've not found any drivers to use heaps. I'll hold this patch until such a driver or other ideas are upstreamed. Thank you, --- Best Regards Kunihiko Hayashi
Re: [PATCH] dma-buf: heaps: Introduce dma_heap_add_cma() for non-default CMA heap
On Thu, Jul 16, 2020 at 6:10 PM Kunihiko Hayashi wrote: > > Current dma-buf heaps can handle only default CMA. This introduces > dma_heap_add_cma() function to attach CMA heaps that belongs to a device. > > At first, the driver calls of_reserved_mem_device_init() to set > memory-region property associated with reserved-memory defined as CMA > to the device. And when the driver calls this dma_heap_add_cma(), > the CMA will be added to dma-buf heaps. > > For example, prepare CMA node named "linux,cma@1000" and > specify the node for memory-region property. After the above calls > in the driver, a device file "/dev/dma_heap/linux,cma@1000" > associated with the CMA become available as dma-buf heaps. > > Signed-off-by: Kunihiko Hayashi > --- > drivers/dma-buf/heaps/cma_heap.c | 12 > include/linux/dma-heap.h | 9 + > 2 files changed, 21 insertions(+) Hey! Sorry for the slow response on this! I just realized I never replied! > diff --git a/drivers/dma-buf/heaps/cma_heap.c > b/drivers/dma-buf/heaps/cma_heap.c > index 626cf7f..5d2442e 100644 > --- a/drivers/dma-buf/heaps/cma_heap.c > +++ b/drivers/dma-buf/heaps/cma_heap.c > @@ -162,6 +162,18 @@ static int __add_cma_heap(struct cma *cma, void *data) > return 0; > } > > +/* add device CMA heap to dmabuf heaps */ > +int dma_heap_add_cma(struct device *dev) > +{ > + struct cma *cma = dev_get_cma_area(dev); > + > + if (!cma) > + return -ENOMEM; > + > + return __add_cma_heap(cma, NULL); > +} > +EXPORT_SYMBOL_GPL(dma_heap_add_cma); > + > static int add_default_cma_heap(void) > { > struct cma *default_cma = dev_get_cma_area(NULL); > diff --git a/include/linux/dma-heap.h b/include/linux/dma-heap.h > index 454e354..16bec7d 100644 > --- a/include/linux/dma-heap.h > +++ b/include/linux/dma-heap.h > @@ -56,4 +56,13 @@ void *dma_heap_get_drvdata(struct dma_heap *heap); > */ > struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info); > > +#ifdef CONFIG_DMABUF_HEAPS_CMA > +/** > + * dma_heap_add_cma - adds a device CMA heap to dmabuf heaps > + * @dev: device with a CMA heap to register > + */ > +int dma_heap_add_cma(struct device *dev); > + > +#endif /* CONFIG_DMABUF_HEAPS_CMA */ > + > #endif /* _DMA_HEAPS_H */ > -- > 2.7.4 Looks sane to me. Being able to expose different multiple CMA heaps is needed, and I agree this way (as opposed to my earlier dts appraoch) is probably the best approach. The only bit I'm so-so on is adding the CMA heap specific call in the dma-heap.h, but at the same time I can't justify adding a whole new header for a single function. Do you have a upstream driver that you plan to make use this new call? We want to have in-tree users of code added. But if so, feel free to add my: Acked-by: John Stultz To this patch when you submit the driver changes. thanks -john
Re: [PATCH] dma-buf: heaps: Introduce dma_heap_add_cma() for non-default CMA heap
On 2020/08/01 4:38, John Stultz wrote: On Fri, Jul 31, 2020 at 2:32 AM Kunihiko Hayashi wrote: On 2020/07/29 4:17, John Stultz wrote: Do you have a upstream driver that you plan to make use this new call? Unfortunately I don't have an upstream driver using this call. This call is called from dma-buf heaps "importer" or "customer", and I only made an example (do nothing) importer driver to test the call. We want to have in-tree users of code added. I think this is a generic way to use non-default CMA heaps, however, we need in-tree "importer" drivers to want to use non-default CMA heaps. I don't find it from now. Yea, I and again, I do agree this is functionality that will be needed. But we'll need to wait for a user (camera driver, etc which would utilize the reserved cma region) before we can merge it upstream. :( Do let me know if you have an out of tree driver that would make use of it, and we can see what can be done to help upstream things. Sorry for late. Before I prepare or find a user driver as "importer", I think something is different in this patch. This patch makes it possible to treat non-default CMA connected to "importer" device with memory-region as dma-buf heaps. However, the allocated memory from this dma-buf heaps can be used for "any" devices, and the "importer" can treat memories from other dma-buf heaps. So, the "importer" and the non-default CMA aren't directly related, and I think an "exporter" for the non-default CMA should be enabled. In paticular, the kernel initializer (as an "exporter") calls dma_heap_add_cma() for all CMAs defined in Devicetree, and the device files associated with each CMA appear under "/dev/dma_heap/". For example: /dev/dma_heap/linux,cma@1000 /dev/dma_heap/linux,cma@1100 /dev/dma_heap/linux,cma@1200 ... All of these device files can be fairly allocated to any "importer" device. Actually I think that the kernel should executes dma_heap_add_cma() for ALL defined reserved-memory nodes. If this idea hasn't been discussed yet and this is reasonable, I'll prepare RFC patches. Thank you, --- Best Regards Kunihiko Hayashi
Re: [PATCH] dma-buf: heaps: Introduce dma_heap_add_cma() for non-default CMA heap
On Fri, Aug 21, 2020 at 2:14 AM Kunihiko Hayashi wrote: > > On 2020/08/01 4:38, John Stultz wrote: > > On Fri, Jul 31, 2020 at 2:32 AM Kunihiko Hayashi > > wrote: > >> On 2020/07/29 4:17, John Stultz wrote: > >>> Do you have a upstream driver that you plan to make use this new call? > >> > >> Unfortunately I don't have an upstream driver using this call. > >> > >> This call is called from dma-buf heaps "importer" or "customer", > >> and I only made an example (do nothing) importer driver > >> to test the call. > >> > >>> We want to have in-tree users of code added. > >> > >> I think this is a generic way to use non-default CMA heaps, however, > >> we need in-tree "importer" drivers to want to use non-default CMA heaps. > >> I don't find it from now. > >> > > > > Yea, I and again, I do agree this is functionality that will be > > needed. But we'll need to wait for a user (camera driver, etc which > > would utilize the reserved cma region) before we can merge it > > upstream. :( Do let me know if you have an out of tree driver that > > would make use of it, and we can see what can be done to help upstream > > things. > > Sorry for late. > Before I prepare or find a user driver as "importer", > I think something is different in this patch. > > This patch makes it possible to treat non-default CMA connected to > "importer" device with memory-region as dma-buf heaps. > > However, the allocated memory from this dma-buf heaps can be used > for "any" devices, and the "importer" can treat memories from other > dma-buf heaps. > > So, the "importer" and the non-default CMA aren't directly related, > and I think an "exporter" for the non-default CMA should be enabled. > > In paticular, the kernel initializer (as an "exporter") calls > dma_heap_add_cma() for all CMAs defined in Devicetree, and > the device files associated with each CMA appear under "/dev/dma_heap/". > For example: > > /dev/dma_heap/linux,cma@1000 > /dev/dma_heap/linux,cma@1100 > /dev/dma_heap/linux,cma@1200 > ... > > All of these device files can be fairly allocated to any "importer" device. > > Actually I think that the kernel should executes dma_heap_add_cma() > for ALL defined reserved-memory nodes. > > If this idea hasn't been discussed yet and this is reasonable, > I'll prepare RFC patches. So yes! An earlier version of the CMA heap I submitted did add all CMA regions as accessible heaps as you propose here. However, the concern was that in some cases, those regions are device specific reserved memory that the driver is probably expecting to have exclusive access. To allow (sufficiently privileged, or misconfigured) userland to be able to allocate out of that seemed like it might cause trouble, and instead we should have CMA regions explicitly exported. There was some proposal to add a dt property to the reserved memory section (similar to linux,cma-default) and use that to do the exporting, but other discussions seemed to prefer having drivers export it explicitly in a fashion very similar to what your earlier patch does. The only trouble is we just need an upstream driver to add such a call in the series before we can merge it. thanks -john