Now that the DMA BUF heaps core code has been queued, I wanted
to send out some of the pending changes that I've been working
on.
For use with Android and their GKI effort, it is desired that
DMA BUF heaps are able to be loaded as modules. This is required
for migrating vendors off of ION which was also recently changed
to support modules.
So this patch series simply provides the necessary exported
symbols and allows the system and CMA drivers to be built
as modules.
Due to the fact that dmabuf's allocated from a heap may
be in use for quite some time, there isn't a way to safely
unload the driver once it has been loaded. Thus these
drivers do no implement module_exit() functions and will
show up in lsmod as "[permanent]"
Feedback and thoughts on this would be greatly appreciated!
thanks
-john
Cc: Laura Abbott <[email protected]>
Cc: Benjamin Gaignard <[email protected]>
Cc: Sumit Semwal <[email protected]>
Cc: Liam Mark <[email protected]>
Cc: Pratik Patel <[email protected]>
Cc: Brian Starkey <[email protected]>
Cc: Andrew F. Davis <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Yue Hu <[email protected]>
Cc: Mike Rapoport <[email protected]>
Cc: Chenbo Feng <[email protected]>
Cc: Alistair Strachan <[email protected]>
Cc: Sandeep Patil <[email protected]>
Cc: Hridya Valsaraju <[email protected]>
Cc: [email protected]
John Stultz (1):
dma-buf: heaps: Allow system & cma heaps to be configured as a modules
Sandeep Patil (1):
mm: cma: Export cma symbols for cma heap as a module
drivers/dma-buf/dma-heap.c | 2 ++
drivers/dma-buf/heaps/Kconfig | 4 ++--
drivers/dma-buf/heaps/heap-helpers.c | 2 ++
kernel/dma/contiguous.c | 1 +
mm/cma.c | 5 +++++
5 files changed, 12 insertions(+), 2 deletions(-)
--
2.17.1
Allow loading system and cma heap as a module instead of just as
a statically built in heap.
Since there isn't a good mechanism for dmabuf lifetime tracking
it isn't safe to allow the heap drivers to be unloaded, so these
drivers do not implement any module unloading functionality and
will show up in lsmod as "[permanent]".
This patch also exports key functions from dmabuf heaps core and
the heap helper functions so they can be accessed by the module.
Cc: Laura Abbott <[email protected]>
Cc: Benjamin Gaignard <[email protected]>
Cc: Sumit Semwal <[email protected]>
Cc: Liam Mark <[email protected]>
Cc: Pratik Patel <[email protected]>
Cc: Brian Starkey <[email protected]>
Cc: Andrew F. Davis <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Yue Hu <[email protected]>
Cc: Mike Rapoport <[email protected]>
Cc: Chenbo Feng <[email protected]>
Cc: Alistair Strachan <[email protected]>
Cc: Sandeep Patil <[email protected]>
Cc: Hridya Valsaraju <[email protected]>
Cc: [email protected]
Signed-off-by: John Stultz <[email protected]>
---
drivers/dma-buf/dma-heap.c | 2 ++
drivers/dma-buf/heaps/Kconfig | 4 ++--
drivers/dma-buf/heaps/heap-helpers.c | 2 ++
3 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
index 9a41b73e54b4..2c4ac71a715b 100644
--- a/drivers/dma-buf/dma-heap.c
+++ b/drivers/dma-buf/dma-heap.c
@@ -161,6 +161,7 @@ void *dma_heap_get_drvdata(struct dma_heap *heap)
{
return heap->priv;
}
+EXPORT_SYMBOL_GPL(dma_heap_get_drvdata);
struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info)
{
@@ -243,6 +244,7 @@ struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info)
kfree(heap);
return err_ret;
}
+EXPORT_SYMBOL_GPL(dma_heap_add);
static char *dma_heap_devnode(struct device *dev, umode_t *mode)
{
diff --git a/drivers/dma-buf/heaps/Kconfig b/drivers/dma-buf/heaps/Kconfig
index a5eef06c4226..e273fb18feca 100644
--- a/drivers/dma-buf/heaps/Kconfig
+++ b/drivers/dma-buf/heaps/Kconfig
@@ -1,12 +1,12 @@
config DMABUF_HEAPS_SYSTEM
- bool "DMA-BUF System Heap"
+ tristate "DMA-BUF System Heap"
depends on DMABUF_HEAPS
help
Choose this option to enable the system dmabuf heap. The system heap
is backed by pages from the buddy allocator. If in doubt, say Y.
config DMABUF_HEAPS_CMA
- bool "DMA-BUF CMA Heap"
+ tristate "DMA-BUF CMA Heap"
depends on DMABUF_HEAPS && DMA_CMA
help
Choose this option to enable dma-buf CMA heap. This heap is backed
diff --git a/drivers/dma-buf/heaps/heap-helpers.c b/drivers/dma-buf/heaps/heap-helpers.c
index 750bef4e902d..fb9835126893 100644
--- a/drivers/dma-buf/heaps/heap-helpers.c
+++ b/drivers/dma-buf/heaps/heap-helpers.c
@@ -24,6 +24,7 @@ void init_heap_helper_buffer(struct heap_helper_buffer *buffer,
INIT_LIST_HEAD(&buffer->attachments);
buffer->free = free;
}
+EXPORT_SYMBOL_GPL(init_heap_helper_buffer);
struct dma_buf *heap_helper_export_dmabuf(struct heap_helper_buffer *buffer,
int fd_flags)
@@ -37,6 +38,7 @@ struct dma_buf *heap_helper_export_dmabuf(struct heap_helper_buffer *buffer,
return dma_buf_export(&exp_info);
}
+EXPORT_SYMBOL_GPL(heap_helper_export_dmabuf);
static void *dma_heap_map_kernel(struct heap_helper_buffer *buffer)
{
--
2.17.1
From: Sandeep Patil <[email protected]>
Export cma_get_name, cma_alloc, cma_release, cma_for_each_area
and dma_contiguous_default_area so that we can use these from
the dmabuf cma heap when it is built as module.
Cc: Laura Abbott <[email protected]>
Cc: Benjamin Gaignard <[email protected]>
Cc: Sumit Semwal <[email protected]>
Cc: Liam Mark <[email protected]>
Cc: Pratik Patel <[email protected]>
Cc: Brian Starkey <[email protected]>
Cc: Andrew F. Davis <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Yue Hu <[email protected]>
Cc: Mike Rapoport <[email protected]>
Cc: Chenbo Feng <[email protected]>
Cc: Alistair Strachan <[email protected]>
Cc: Sandeep Patil <[email protected]>
Cc: Hridya Valsaraju <[email protected]>
Cc: [email protected]
Signed-off-by: Sandeep Patil <[email protected]>
[jstultz: Rewrote commit message, added
dma_contiguous_default_area to the set of exported symbols]
Signed-off-by: John Stultz <[email protected]>
---
kernel/dma/contiguous.c | 1 +
mm/cma.c | 5 +++++
2 files changed, 6 insertions(+)
diff --git a/kernel/dma/contiguous.c b/kernel/dma/contiguous.c
index 69cfb4345388..ff6cba63ea6f 100644
--- a/kernel/dma/contiguous.c
+++ b/kernel/dma/contiguous.c
@@ -31,6 +31,7 @@
#endif
struct cma *dma_contiguous_default_area;
+EXPORT_SYMBOL(dma_contiguous_default_area);
/*
* Default global CMA area size can be defined in kernel's .config.
diff --git a/mm/cma.c b/mm/cma.c
index 7fe0b8356775..db4642e58058 100644
--- a/mm/cma.c
+++ b/mm/cma.c
@@ -24,6 +24,7 @@
#include <linux/memblock.h>
#include <linux/err.h>
#include <linux/mm.h>
+#include <linux/module.h>
#include <linux/mutex.h>
#include <linux/sizes.h>
#include <linux/slab.h>
@@ -54,6 +55,7 @@ const char *cma_get_name(const struct cma *cma)
{
return cma->name ? cma->name : "(undefined)";
}
+EXPORT_SYMBOL_GPL(cma_get_name);
static unsigned long cma_bitmap_aligned_mask(const struct cma *cma,
unsigned int align_order)
@@ -500,6 +502,7 @@ struct page *cma_alloc(struct cma *cma, size_t count, unsigned int align,
pr_debug("%s(): returned %p\n", __func__, page);
return page;
}
+EXPORT_SYMBOL_GPL(cma_alloc);
/**
* cma_release() - release allocated pages
@@ -533,6 +536,7 @@ bool cma_release(struct cma *cma, const struct page *pages, unsigned int count)
return true;
}
+EXPORT_SYMBOL_GPL(cma_release);
int cma_for_each_area(int (*it)(struct cma *cma, void *data), void *data)
{
@@ -547,3 +551,4 @@ int cma_for_each_area(int (*it)(struct cma *cma, void *data), void *data)
return 0;
}
+EXPORT_SYMBOL_GPL(cma_for_each_area);
--
2.17.1
On Fri, Oct 25, 2019 at 11:48:33PM +0000, John Stultz wrote:
> struct cma *dma_contiguous_default_area;
> +EXPORT_SYMBOL(dma_contiguous_default_area);
Please CC the dma maintainer. And no, you have no business using this.
Even if you did, internals like this should always be EXPORT_SYMBOL_GPL.
On Fri, Oct 25, 2019 at 11:48:33PM +0000, John Stultz wrote:
> From: Sandeep Patil <[email protected]>
>
> Export cma_get_name, cma_alloc, cma_release, cma_for_each_area
> and dma_contiguous_default_area so that we can use these from
> the dmabuf cma heap when it is built as module.
>
> Cc: Laura Abbott <[email protected]>
> Cc: Benjamin Gaignard <[email protected]>
> Cc: Sumit Semwal <[email protected]>
> Cc: Liam Mark <[email protected]>
> Cc: Pratik Patel <[email protected]>
> Cc: Brian Starkey <[email protected]>
> Cc: Andrew F. Davis <[email protected]>
> Cc: Andrew Morton <[email protected]>
> Cc: Yue Hu <[email protected]>
> Cc: Mike Rapoport <[email protected]>
> Cc: Chenbo Feng <[email protected]>
> Cc: Alistair Strachan <[email protected]>
> Cc: Sandeep Patil <[email protected]>
> Cc: Hridya Valsaraju <[email protected]>
> Cc: [email protected]
> Signed-off-by: Sandeep Patil <[email protected]>
> [jstultz: Rewrote commit message, added
> dma_contiguous_default_area to the set of exported symbols]
> Signed-off-by: John Stultz <[email protected]>
> ---
> kernel/dma/contiguous.c | 1 +
> mm/cma.c | 5 +++++
> 2 files changed, 6 insertions(+)
>
> diff --git a/kernel/dma/contiguous.c b/kernel/dma/contiguous.c
> index 69cfb4345388..ff6cba63ea6f 100644
> --- a/kernel/dma/contiguous.c
> +++ b/kernel/dma/contiguous.c
> @@ -31,6 +31,7 @@
> #endif
>
> struct cma *dma_contiguous_default_area;
> +EXPORT_SYMBOL(dma_contiguous_default_area);
I didn't need to do this for the (out-of-tree) ion cma heap [1].
Any reason why you had to?
Other than that, thanks for doing this John.
Acked-by: Sandeep Patil <[email protected]>
- ssp
1] https://android-review.googlesource.com/c/kernel/common/+/1121591
On Mon, Oct 28, 2019 at 12:12 PM <[email protected]> wrote:
> On Fri, Oct 25, 2019 at 11:48:33PM +0000, John Stultz wrote:
> > --- a/kernel/dma/contiguous.c
> > +++ b/kernel/dma/contiguous.c
> > @@ -31,6 +31,7 @@
> > #endif
> >
> > struct cma *dma_contiguous_default_area;
> > +EXPORT_SYMBOL(dma_contiguous_default_area);
>
> I didn't need to do this for the (out-of-tree) ion cma heap [1].
> Any reason why you had to?
Its likely due to the changes I made in the separate
non-default-CMA-region patch set. Earlier I had gotten away with just
your change, but in testing before I sent this series, I hit the build
error and quickly added the export before sending.
I'll revise and respin this. Mostly just wanting to get initial
feedback on the dmabuf-heaps-as-modules bit, so I didn't take as much
care as I should have with the exports here.
thanks
-john
On Mon, Oct 28, 2019 at 1:03 PM John Stultz <[email protected]> wrote:
>
> On Mon, Oct 28, 2019 at 12:12 PM <[email protected]> wrote:
> > On Fri, Oct 25, 2019 at 11:48:33PM +0000, John Stultz wrote:
> > > --- a/kernel/dma/contiguous.c
> > > +++ b/kernel/dma/contiguous.c
> > > @@ -31,6 +31,7 @@
> > > #endif
> > >
> > > struct cma *dma_contiguous_default_area;
> > > +EXPORT_SYMBOL(dma_contiguous_default_area);
> >
> > I didn't need to do this for the (out-of-tree) ion cma heap [1].
> > Any reason why you had to?
>
> Its likely due to the changes I made in the separate
> non-default-CMA-region patch set. Earlier I had gotten away with just
> your change, but in testing before I sent this series, I hit the build
> error and quickly added the export before sending.
And just to clarify this point, I was mistaken and it wasn't the
non-default-CMA-region patch set, but the fact that I'm using
dev_get_cma_area(NULL) to get the default CMA area to register, rather
then trying to register every CMA area via cma_for_each_area() as is
done in ION. I actually could probably drop the cma_for_each_area
export for the dmabuf heaps usage.
thanks
-john
On Mon, Oct 28, 2019 at 12:46 AM Christoph Hellwig <[email protected]> wrote:
>
> On Fri, Oct 25, 2019 at 11:48:33PM +0000, John Stultz wrote:
> > struct cma *dma_contiguous_default_area;
> > +EXPORT_SYMBOL(dma_contiguous_default_area);
>
> Please CC the dma maintainer. And no, you have no business using this.
Sure thing. And I'll look again to see why I was needing to pull that
one in to get it to build.
> Even if you did, internals like this should always be EXPORT_SYMBOL_GPL.
Certainly! My mistake here!
Thanks for the feedback!
-john
On Mon, Oct 28, 2019 at 11:39 AM John Stultz <[email protected]> wrote:
> On Mon, Oct 28, 2019 at 12:46 AM Christoph Hellwig <[email protected]> wrote:
> >
> > On Fri, Oct 25, 2019 at 11:48:33PM +0000, John Stultz wrote:
> > > struct cma *dma_contiguous_default_area;
> > > +EXPORT_SYMBOL(dma_contiguous_default_area);
> >
> > Please CC the dma maintainer. And no, you have no business using this.
>
> Sure thing. And I'll look again to see why I was needing to pull that
> one in to get it to build.
Ah. So looking a bit closer, I'm needing this due to my using
dev_get_cma_area() to get the default cma area for the dmabuf
cma_heap.
Do you have a suggestion for how to get a reference to the default CMA
area without exporting dma_contiguous_default_area? Would it be
preferred to move dev_get_cma_area() into the .c file and
EXPORT_SYMBOL_GPL that function?
thanks
-john
Hi John,
On Fri, Oct 25, 2019 at 11:48:34PM +0000, John Stultz wrote:
> Allow loading system and cma heap as a module instead of just as
> a statically built in heap.
>
> Since there isn't a good mechanism for dmabuf lifetime tracking
> it isn't safe to allow the heap drivers to be unloaded, so these
> drivers do not implement any module unloading functionality and
> will show up in lsmod as "[permanent]".
Cool, that alleviates my concerns :-)
>
> This patch also exports key functions from dmabuf heaps core and
> the heap helper functions so they can be accessed by the module.
>
> Cc: Laura Abbott <[email protected]>
> Cc: Benjamin Gaignard <[email protected]>
> Cc: Sumit Semwal <[email protected]>
> Cc: Liam Mark <[email protected]>
> Cc: Pratik Patel <[email protected]>
> Cc: Brian Starkey <[email protected]>
> Cc: Andrew F. Davis <[email protected]>
> Cc: Andrew Morton <[email protected]>
> Cc: Yue Hu <[email protected]>
> Cc: Mike Rapoport <[email protected]>
> Cc: Chenbo Feng <[email protected]>
> Cc: Alistair Strachan <[email protected]>
> Cc: Sandeep Patil <[email protected]>
> Cc: Hridya Valsaraju <[email protected]>
> Cc: [email protected]
> Signed-off-by: John Stultz <[email protected]>
> ---
> drivers/dma-buf/dma-heap.c | 2 ++
> drivers/dma-buf/heaps/Kconfig | 4 ++--
> drivers/dma-buf/heaps/heap-helpers.c | 2 ++
> 3 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
> index 9a41b73e54b4..2c4ac71a715b 100644
> --- a/drivers/dma-buf/dma-heap.c
> +++ b/drivers/dma-buf/dma-heap.c
> @@ -161,6 +161,7 @@ void *dma_heap_get_drvdata(struct dma_heap *heap)
> {
> return heap->priv;
> }
> +EXPORT_SYMBOL_GPL(dma_heap_get_drvdata);
>
> struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info)
> {
> @@ -243,6 +244,7 @@ struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info)
> kfree(heap);
> return err_ret;
> }
> +EXPORT_SYMBOL_GPL(dma_heap_add);
Maybe overly picky - but adding the note about "no safe way to remove,
so there's no dma_heap_remove" to a comment on this function may be
easier to notice than in the git log alone.
Cheers,
-Brian
>
> static char *dma_heap_devnode(struct device *dev, umode_t *mode)
> {
> diff --git a/drivers/dma-buf/heaps/Kconfig b/drivers/dma-buf/heaps/Kconfig
> index a5eef06c4226..e273fb18feca 100644
> --- a/drivers/dma-buf/heaps/Kconfig
> +++ b/drivers/dma-buf/heaps/Kconfig
> @@ -1,12 +1,12 @@
> config DMABUF_HEAPS_SYSTEM
> - bool "DMA-BUF System Heap"
> + tristate "DMA-BUF System Heap"
> depends on DMABUF_HEAPS
> help
> Choose this option to enable the system dmabuf heap. The system heap
> is backed by pages from the buddy allocator. If in doubt, say Y.
>
> config DMABUF_HEAPS_CMA
> - bool "DMA-BUF CMA Heap"
> + tristate "DMA-BUF CMA Heap"
> depends on DMABUF_HEAPS && DMA_CMA
> help
> Choose this option to enable dma-buf CMA heap. This heap is backed
> diff --git a/drivers/dma-buf/heaps/heap-helpers.c b/drivers/dma-buf/heaps/heap-helpers.c
> index 750bef4e902d..fb9835126893 100644
> --- a/drivers/dma-buf/heaps/heap-helpers.c
> +++ b/drivers/dma-buf/heaps/heap-helpers.c
> @@ -24,6 +24,7 @@ void init_heap_helper_buffer(struct heap_helper_buffer *buffer,
> INIT_LIST_HEAD(&buffer->attachments);
> buffer->free = free;
> }
> +EXPORT_SYMBOL_GPL(init_heap_helper_buffer);
>
> struct dma_buf *heap_helper_export_dmabuf(struct heap_helper_buffer *buffer,
> int fd_flags)
> @@ -37,6 +38,7 @@ struct dma_buf *heap_helper_export_dmabuf(struct heap_helper_buffer *buffer,
>
> return dma_buf_export(&exp_info);
> }
> +EXPORT_SYMBOL_GPL(heap_helper_export_dmabuf);
>
> static void *dma_heap_map_kernel(struct heap_helper_buffer *buffer)
> {
> --
> 2.17.1
>
On Fri, Oct 25, 2019 at 11:48:32PM +0000, John Stultz wrote:
> Now that the DMA BUF heaps core code has been queued, I wanted
> to send out some of the pending changes that I've been working
> on.
>
> For use with Android and their GKI effort, it is desired that
> DMA BUF heaps are able to be loaded as modules. This is required
> for migrating vendors off of ION which was also recently changed
> to support modules.
>
> So this patch series simply provides the necessary exported
> symbols and allows the system and CMA drivers to be built
> as modules.
>
> Due to the fact that dmabuf's allocated from a heap may
> be in use for quite some time, there isn't a way to safely
> unload the driver once it has been loaded. Thus these
> drivers do no implement module_exit() functions and will
> show up in lsmod as "[permanent]"
>
> Feedback and thoughts on this would be greatly appreciated!
Do we actually want this?
I figured if we just state that vendors should set up all the right
dma-buf heaps in dt, is that not enough?
Exporting symbols for no real in-tree users feels fishy.
-Daniel
>
> thanks
> -john
>
> Cc: Laura Abbott <[email protected]>
> Cc: Benjamin Gaignard <[email protected]>
> Cc: Sumit Semwal <[email protected]>
> Cc: Liam Mark <[email protected]>
> Cc: Pratik Patel <[email protected]>
> Cc: Brian Starkey <[email protected]>
> Cc: Andrew F. Davis <[email protected]>
> Cc: Andrew Morton <[email protected]>
> Cc: Yue Hu <[email protected]>
> Cc: Mike Rapoport <[email protected]>
> Cc: Chenbo Feng <[email protected]>
> Cc: Alistair Strachan <[email protected]>
> Cc: Sandeep Patil <[email protected]>
> Cc: Hridya Valsaraju <[email protected]>
> Cc: [email protected]
>
> John Stultz (1):
> dma-buf: heaps: Allow system & cma heaps to be configured as a modules
>
> Sandeep Patil (1):
> mm: cma: Export cma symbols for cma heap as a module
>
> drivers/dma-buf/dma-heap.c | 2 ++
> drivers/dma-buf/heaps/Kconfig | 4 ++--
> drivers/dma-buf/heaps/heap-helpers.c | 2 ++
> kernel/dma/contiguous.c | 1 +
> mm/cma.c | 5 +++++
> 5 files changed, 12 insertions(+), 2 deletions(-)
>
> --
> 2.17.1
>
> _______________________________________________
> dri-devel mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
On Fri, Oct 25, 2019 at 11:48:34PM +0000, John Stultz wrote:
> Allow loading system and cma heap as a module instead of just as
> a statically built in heap.
>
> Since there isn't a good mechanism for dmabuf lifetime tracking
> it isn't safe to allow the heap drivers to be unloaded, so these
> drivers do not implement any module unloading functionality and
> will show up in lsmod as "[permanent]".
dma-buf itself has all the try_module_get we'll need ... why is this not
possible?
-Daniel
>
> This patch also exports key functions from dmabuf heaps core and
> the heap helper functions so they can be accessed by the module.
>
> Cc: Laura Abbott <[email protected]>
> Cc: Benjamin Gaignard <[email protected]>
> Cc: Sumit Semwal <[email protected]>
> Cc: Liam Mark <[email protected]>
> Cc: Pratik Patel <[email protected]>
> Cc: Brian Starkey <[email protected]>
> Cc: Andrew F. Davis <[email protected]>
> Cc: Andrew Morton <[email protected]>
> Cc: Yue Hu <[email protected]>
> Cc: Mike Rapoport <[email protected]>
> Cc: Chenbo Feng <[email protected]>
> Cc: Alistair Strachan <[email protected]>
> Cc: Sandeep Patil <[email protected]>
> Cc: Hridya Valsaraju <[email protected]>
> Cc: [email protected]
> Signed-off-by: John Stultz <[email protected]>
> ---
> drivers/dma-buf/dma-heap.c | 2 ++
> drivers/dma-buf/heaps/Kconfig | 4 ++--
> drivers/dma-buf/heaps/heap-helpers.c | 2 ++
> 3 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
> index 9a41b73e54b4..2c4ac71a715b 100644
> --- a/drivers/dma-buf/dma-heap.c
> +++ b/drivers/dma-buf/dma-heap.c
> @@ -161,6 +161,7 @@ void *dma_heap_get_drvdata(struct dma_heap *heap)
> {
> return heap->priv;
> }
> +EXPORT_SYMBOL_GPL(dma_heap_get_drvdata);
>
> struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info)
> {
> @@ -243,6 +244,7 @@ struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info)
> kfree(heap);
> return err_ret;
> }
> +EXPORT_SYMBOL_GPL(dma_heap_add);
>
> static char *dma_heap_devnode(struct device *dev, umode_t *mode)
> {
> diff --git a/drivers/dma-buf/heaps/Kconfig b/drivers/dma-buf/heaps/Kconfig
> index a5eef06c4226..e273fb18feca 100644
> --- a/drivers/dma-buf/heaps/Kconfig
> +++ b/drivers/dma-buf/heaps/Kconfig
> @@ -1,12 +1,12 @@
> config DMABUF_HEAPS_SYSTEM
> - bool "DMA-BUF System Heap"
> + tristate "DMA-BUF System Heap"
> depends on DMABUF_HEAPS
> help
> Choose this option to enable the system dmabuf heap. The system heap
> is backed by pages from the buddy allocator. If in doubt, say Y.
>
> config DMABUF_HEAPS_CMA
> - bool "DMA-BUF CMA Heap"
> + tristate "DMA-BUF CMA Heap"
> depends on DMABUF_HEAPS && DMA_CMA
> help
> Choose this option to enable dma-buf CMA heap. This heap is backed
> diff --git a/drivers/dma-buf/heaps/heap-helpers.c b/drivers/dma-buf/heaps/heap-helpers.c
> index 750bef4e902d..fb9835126893 100644
> --- a/drivers/dma-buf/heaps/heap-helpers.c
> +++ b/drivers/dma-buf/heaps/heap-helpers.c
> @@ -24,6 +24,7 @@ void init_heap_helper_buffer(struct heap_helper_buffer *buffer,
> INIT_LIST_HEAD(&buffer->attachments);
> buffer->free = free;
> }
> +EXPORT_SYMBOL_GPL(init_heap_helper_buffer);
>
> struct dma_buf *heap_helper_export_dmabuf(struct heap_helper_buffer *buffer,
> int fd_flags)
> @@ -37,6 +38,7 @@ struct dma_buf *heap_helper_export_dmabuf(struct heap_helper_buffer *buffer,
>
> return dma_buf_export(&exp_info);
> }
> +EXPORT_SYMBOL_GPL(heap_helper_export_dmabuf);
>
> static void *dma_heap_map_kernel(struct heap_helper_buffer *buffer)
> {
> --
> 2.17.1
>
> _______________________________________________
> dri-devel mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
On Mon, Nov 4, 2019 at 1:58 AM Daniel Vetter <[email protected]> wrote:
> On Fri, Oct 25, 2019 at 11:48:32PM +0000, John Stultz wrote:
> > Now that the DMA BUF heaps core code has been queued, I wanted
> > to send out some of the pending changes that I've been working
> > on.
> >
> > For use with Android and their GKI effort, it is desired that
> > DMA BUF heaps are able to be loaded as modules. This is required
> > for migrating vendors off of ION which was also recently changed
> > to support modules.
> >
> > So this patch series simply provides the necessary exported
> > symbols and allows the system and CMA drivers to be built
> > as modules.
> >
> > Due to the fact that dmabuf's allocated from a heap may
> > be in use for quite some time, there isn't a way to safely
> > unload the driver once it has been loaded. Thus these
> > drivers do no implement module_exit() functions and will
> > show up in lsmod as "[permanent]"
> >
> > Feedback and thoughts on this would be greatly appreciated!
>
> Do we actually want this?
I guess that always depends on the definition of "we" :)
> I figured if we just state that vendors should set up all the right
> dma-buf heaps in dt, is that not enough?
So even if the heaps are configured via DT (which at the moment there
is no such binding, so that's not really a valid method yet), there's
still the question of if the heap is necessary/makes sense on the
device. And the DT would only control the availability of the heap
interface, not if the heap driver is loaded or not.
On the HiKey/HiKey960 boards, we have to allocate contiguous buffers
for the display framebuffer. So gralloc uses ION to allocate from the
CMA heap. However on the db845c, it has no such restrictions, so the
CMA heap isn't necessary.
With Android's GKI effort, there needs to be one kernel that works on
all the devices, and they are using modules to try to minimize the
amount of memory spent on functionality that isn't universally needed.
So on devices that don't need the CMA heap, they'd probably prefer not
to load the CMA dmabuf heap driver, so it would be best if it could be
built as a module. If we want to build the CMA heap as a module, the
symbols it uses need to be exported.
> Exporting symbols for no real in-tree users feels fishy.
I'm submitting an in-tree user here. So I'm not sure what you mean? I
suspect you're thinking there is some hidden/nefarious plan here, but
really there isn't.
thanks
-john
On Mon, Nov 4, 2019 at 2:24 AM Daniel Vetter <[email protected]> wrote:
> On Fri, Oct 25, 2019 at 11:48:34PM +0000, John Stultz wrote:
> > Allow loading system and cma heap as a module instead of just as
> > a statically built in heap.
> >
> > Since there isn't a good mechanism for dmabuf lifetime tracking
> > it isn't safe to allow the heap drivers to be unloaded, so these
> > drivers do not implement any module unloading functionality and
> > will show up in lsmod as "[permanent]".
>
> dma-buf itself has all the try_module_get we'll need ... why is this not
> possible?
Let me look into that. Thanks for the pointer.
thanks
-john
On Mon, Nov 04, 2019 at 10:57:44AM -0800, John Stultz wrote:
> On Mon, Nov 4, 2019 at 1:58 AM Daniel Vetter <[email protected]> wrote:
> > On Fri, Oct 25, 2019 at 11:48:32PM +0000, John Stultz wrote:
> > > Now that the DMA BUF heaps core code has been queued, I wanted
> > > to send out some of the pending changes that I've been working
> > > on.
> > >
> > > For use with Android and their GKI effort, it is desired that
> > > DMA BUF heaps are able to be loaded as modules. This is required
> > > for migrating vendors off of ION which was also recently changed
> > > to support modules.
> > >
> > > So this patch series simply provides the necessary exported
> > > symbols and allows the system and CMA drivers to be built
> > > as modules.
> > >
> > > Due to the fact that dmabuf's allocated from a heap may
> > > be in use for quite some time, there isn't a way to safely
> > > unload the driver once it has been loaded. Thus these
> > > drivers do no implement module_exit() functions and will
> > > show up in lsmod as "[permanent]"
> > >
> > > Feedback and thoughts on this would be greatly appreciated!
> >
> > Do we actually want this?
>
> I guess that always depends on the definition of "we" :)
>
> > I figured if we just state that vendors should set up all the right
> > dma-buf heaps in dt, is that not enough?
>
> So even if the heaps are configured via DT (which at the moment there
> is no such binding, so that's not really a valid method yet), there's
> still the question of if the heap is necessary/makes sense on the
> device. And the DT would only control the availability of the heap
> interface, not if the heap driver is loaded or not.
Hm I thought the cma regions are configured in DT? How does that work if
it's not using DT?
> On the HiKey/HiKey960 boards, we have to allocate contiguous buffers
> for the display framebuffer. So gralloc uses ION to allocate from the
> CMA heap. However on the db845c, it has no such restrictions, so the
> CMA heap isn't necessary.
Why do you have a CMA region for the 2nd board if you don't need it?
_That_ sounds like some serious memory waster, not a few lines of code
loaded for nothing :-)
> With Android's GKI effort, there needs to be one kernel that works on
> all the devices, and they are using modules to try to minimize the
> amount of memory spent on functionality that isn't universally needed.
> So on devices that don't need the CMA heap, they'd probably prefer not
> to load the CMA dmabuf heap driver, so it would be best if it could be
> built as a module. If we want to build the CMA heap as a module, the
> symbols it uses need to be exported.
Yeah, I guess I'm disagreeing on whether dma-buf heaps are core or not.
> > Exporting symbols for no real in-tree users feels fishy.
>
> I'm submitting an in-tree user here. So I'm not sure what you mean? I
> suspect you're thinking there is some hidden/nefarious plan here, but
> really there isn't.
I was working under the assumption that you're only exporting the symbols
for other heaps, and keep the current ones in-tree. Are there even any
out-of-tree dma-buf heaps still? out-of-tree and legit different use-case
I mean ofc, not just out-of-tree because inertia :-)
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
On 11/5/19 4:42 AM, Daniel Vetter wrote:
> On Mon, Nov 04, 2019 at 10:57:44AM -0800, John Stultz wrote:
>> On Mon, Nov 4, 2019 at 1:58 AM Daniel Vetter <[email protected]> wrote:
>>> On Fri, Oct 25, 2019 at 11:48:32PM +0000, John Stultz wrote:
>>>> Now that the DMA BUF heaps core code has been queued, I wanted
>>>> to send out some of the pending changes that I've been working
>>>> on.
>>>>
>>>> For use with Android and their GKI effort, it is desired that
>>>> DMA BUF heaps are able to be loaded as modules. This is required
>>>> for migrating vendors off of ION which was also recently changed
>>>> to support modules.
>>>>
>>>> So this patch series simply provides the necessary exported
>>>> symbols and allows the system and CMA drivers to be built
>>>> as modules.
>>>>
>>>> Due to the fact that dmabuf's allocated from a heap may
>>>> be in use for quite some time, there isn't a way to safely
>>>> unload the driver once it has been loaded. Thus these
>>>> drivers do no implement module_exit() functions and will
>>>> show up in lsmod as "[permanent]"
>>>>
>>>> Feedback and thoughts on this would be greatly appreciated!
>>>
>>> Do we actually want this?
>>
>> I guess that always depends on the definition of "we" :)
>>
>>> I figured if we just state that vendors should set up all the right
>>> dma-buf heaps in dt, is that not enough?
>>
>> So even if the heaps are configured via DT (which at the moment there
>> is no such binding, so that's not really a valid method yet), there's
>> still the question of if the heap is necessary/makes sense on the
>> device. And the DT would only control the availability of the heap
>> interface, not if the heap driver is loaded or not.
>
> Hm I thought the cma regions are configured in DT? How does that work if
> it's not using DT?
>
>> On the HiKey/HiKey960 boards, we have to allocate contiguous buffers
>> for the display framebuffer. So gralloc uses ION to allocate from the
>> CMA heap. However on the db845c, it has no such restrictions, so the
>> CMA heap isn't necessary.
>
> Why do you have a CMA region for the 2nd board if you don't need it?
> _That_ sounds like some serious memory waster, not a few lines of code
> loaded for nothing :-)
>
>> With Android's GKI effort, there needs to be one kernel that works on
>> all the devices, and they are using modules to try to minimize the
>> amount of memory spent on functionality that isn't universally needed.
>> So on devices that don't need the CMA heap, they'd probably prefer not
>> to load the CMA dmabuf heap driver, so it would be best if it could be
>> built as a module. If we want to build the CMA heap as a module, the
>> symbols it uses need to be exported.
>
> Yeah, I guess I'm disagreeing on whether dma-buf heaps are core or not.
>
>>> Exporting symbols for no real in-tree users feels fishy.
>>
>> I'm submitting an in-tree user here. So I'm not sure what you mean? I
>> suspect you're thinking there is some hidden/nefarious plan here, but
>> really there isn't.
>
> I was working under the assumption that you're only exporting the symbols
> for other heaps, and keep the current ones in-tree. Are there even any
> out-of-tree dma-buf heaps still? out-of-tree and legit different use-case
> I mean ofc, not just out-of-tree because inertia :-)
Not sure what you mean here, hopefully all the heaps can be "in-tree"
some day.
https://patchwork.kernel.org/patch/10863957/
Plus some non-caching heaps and one that forces early allocation of our
PAT (gart like) IP.
All this stuff is going into our evil vendor tree next cycle (if not
upstream by then :)), if we want some of these "specialty" heaps to go
into generic kernel builds at some point they will need to be modules if
the core is.
Although I am still thinking Heaps should be always built in + system +
CMA heaps, then the rando heaps could be modules if needed.
Andrew
> -Daniel
>
On Tue, Nov 5, 2019 at 2:30 PM Andrew F. Davis <[email protected]> wrote:
>
> On 11/5/19 4:42 AM, Daniel Vetter wrote:
> > On Mon, Nov 04, 2019 at 10:57:44AM -0800, John Stultz wrote:
> >> On Mon, Nov 4, 2019 at 1:58 AM Daniel Vetter <[email protected]> wrote:
> >>> On Fri, Oct 25, 2019 at 11:48:32PM +0000, John Stultz wrote:
> >>>> Now that the DMA BUF heaps core code has been queued, I wanted
> >>>> to send out some of the pending changes that I've been working
> >>>> on.
> >>>>
> >>>> For use with Android and their GKI effort, it is desired that
> >>>> DMA BUF heaps are able to be loaded as modules. This is required
> >>>> for migrating vendors off of ION which was also recently changed
> >>>> to support modules.
> >>>>
> >>>> So this patch series simply provides the necessary exported
> >>>> symbols and allows the system and CMA drivers to be built
> >>>> as modules.
> >>>>
> >>>> Due to the fact that dmabuf's allocated from a heap may
> >>>> be in use for quite some time, there isn't a way to safely
> >>>> unload the driver once it has been loaded. Thus these
> >>>> drivers do no implement module_exit() functions and will
> >>>> show up in lsmod as "[permanent]"
> >>>>
> >>>> Feedback and thoughts on this would be greatly appreciated!
> >>>
> >>> Do we actually want this?
> >>
> >> I guess that always depends on the definition of "we" :)
> >>
> >>> I figured if we just state that vendors should set up all the right
> >>> dma-buf heaps in dt, is that not enough?
> >>
> >> So even if the heaps are configured via DT (which at the moment there
> >> is no such binding, so that's not really a valid method yet), there's
> >> still the question of if the heap is necessary/makes sense on the
> >> device. And the DT would only control the availability of the heap
> >> interface, not if the heap driver is loaded or not.
> >
> > Hm I thought the cma regions are configured in DT? How does that work if
> > it's not using DT?
> >
> >> On the HiKey/HiKey960 boards, we have to allocate contiguous buffers
> >> for the display framebuffer. So gralloc uses ION to allocate from the
> >> CMA heap. However on the db845c, it has no such restrictions, so the
> >> CMA heap isn't necessary.
> >
> > Why do you have a CMA region for the 2nd board if you don't need it?
> > _That_ sounds like some serious memory waster, not a few lines of code
> > loaded for nothing :-)
> >
> >> With Android's GKI effort, there needs to be one kernel that works on
> >> all the devices, and they are using modules to try to minimize the
> >> amount of memory spent on functionality that isn't universally needed.
> >> So on devices that don't need the CMA heap, they'd probably prefer not
> >> to load the CMA dmabuf heap driver, so it would be best if it could be
> >> built as a module. If we want to build the CMA heap as a module, the
> >> symbols it uses need to be exported.
> >
> > Yeah, I guess I'm disagreeing on whether dma-buf heaps are core or not.
> >
> >>> Exporting symbols for no real in-tree users feels fishy.
> >>
> >> I'm submitting an in-tree user here. So I'm not sure what you mean? I
> >> suspect you're thinking there is some hidden/nefarious plan here, but
> >> really there isn't.
> >
> > I was working under the assumption that you're only exporting the symbols
> > for other heaps, and keep the current ones in-tree. Are there even any
> > out-of-tree dma-buf heaps still? out-of-tree and legit different use-case
> > I mean ofc, not just out-of-tree because inertia :-)
>
> Not sure what you mean here, hopefully all the heaps can be "in-tree"
> some day.
>
> https://patchwork.kernel.org/patch/10863957/
No idea this is good or bad, where's the userspace for it?
> Plus some non-caching heaps and one that forces early allocation of our
> PAT (gart like) IP.
Hm, so essentially we'd need to move _all_ drm allocators into dma-buf
heaps, for all drivers? Can't just do this for TI only ...
> All this stuff is going into our evil vendor tree next cycle (if not
> upstream by then :)), if we want some of these "specialty" heaps to go
> into generic kernel builds at some point they will need to be modules if
> the core is.
>
> Although I am still thinking Heaps should be always built in + system +
> CMA heaps, then the rando heaps could be modules if needed.
Yeah that's what I'd expected to happen. Speciality heaps for when you
have the relevant something unusual (and I'm not sure whether upstream
does want something unusual really, the above examples from you sound
a bit strange).
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
On Tue, Nov 5, 2019 at 1:43 AM Daniel Vetter <[email protected]> wrote:
>
> On Mon, Nov 04, 2019 at 10:57:44AM -0800, John Stultz wrote:
> > On Mon, Nov 4, 2019 at 1:58 AM Daniel Vetter <[email protected]> wrote:
> > > On Fri, Oct 25, 2019 at 11:48:32PM +0000, John Stultz wrote:
> > So even if the heaps are configured via DT (which at the moment there
> > is no such binding, so that's not really a valid method yet), there's
> > still the question of if the heap is necessary/makes sense on the
> > device. And the DT would only control the availability of the heap
> > interface, not if the heap driver is loaded or not.
>
> Hm I thought the cma regions are configured in DT? How does that work if
> it's not using DT?
So yea, CMA regions are either configured by DT or setup at build time
(I think there's a cmdline option to set it up as well).
But the CMA regions and the dmabuf cma heap driver are separate
things. The latter uses the former.
> > On the HiKey/HiKey960 boards, we have to allocate contiguous buffers
> > for the display framebuffer. So gralloc uses ION to allocate from the
> > CMA heap. However on the db845c, it has no such restrictions, so the
> > CMA heap isn't necessary.
>
> Why do you have a CMA region for the 2nd board if you don't need it?
> _That_ sounds like some serious memory waster, not a few lines of code
> loaded for nothing :-)
??? That's not what I said above. If the db845c doesn't need CMA it
won't have a CMA region.
The issue at hand is that we may want to avoid loading the dmabuf CMA
heap driver on a board that doesn't use CMA.
> > With Android's GKI effort, there needs to be one kernel that works on
> > all the devices, and they are using modules to try to minimize the
> > amount of memory spent on functionality that isn't universally needed.
> > So on devices that don't need the CMA heap, they'd probably prefer not
> > to load the CMA dmabuf heap driver, so it would be best if it could be
> > built as a module. If we want to build the CMA heap as a module, the
> > symbols it uses need to be exported.
>
> Yeah, I guess I'm disagreeing on whether dma-buf heaps are core or not.
That's fine to dispute. I'm not really in a place to assert one way or
not, but the Android folks have made their ION system and CMA heaps
loadable via a module, so it would seem like having the dmabuf system
and CMA heaps be modular would be useful to properly replace that
usage.
For instance, the system heap as a module probably doesn't make much
sense, as most boards that want to use the dmabuf heaps interface are
likely to use that as well. CMA is more optional as not all boards
will use that one, so it might make sense to avoid loading it.
Sandeep: Can you chime in here as to how critical having the system
and cma heaps be modules are?
> > > Exporting symbols for no real in-tree users feels fishy.
> >
> > I'm submitting an in-tree user here. So I'm not sure what you mean? I
> > suspect you're thinking there is some hidden/nefarious plan here, but
> > really there isn't.
>
> I was working under the assumption that you're only exporting the symbols
> for other heaps, and keep the current ones in-tree.
No. I'm trying to allow the (hopefully-soon-to-be-in-tree) system and
cma heap drivers to be loaded from a module.
If other heaps need exports, they can submit their heaps upstream and
argue for the exported symbols themselves.
> Are there even any
> out-of-tree dma-buf heaps still? out-of-tree and legit different use-case
> I mean ofc, not just out-of-tree because inertia :-)
So as Andrew mentioned, he has some dmabuf heaps he's working on at
TI. From what I've heard the qualcomm folks like the dmabuf heaps
approach, but I'm not sure if they've taken a pass at converting their
vendor ION heaps to it yet.
The main reason I'm only submitting system and CMA is because those
are the only two I personally have a user for (HiKey/HiKey960 boards).
It's my hope that once we deprecate ION in Android, vendors will
migrate and we'll be able to push them to upstream their heaps as
well.
thanks
-john
On Tue, Nov 5, 2019 at 6:41 PM John Stultz <[email protected]> wrote:
>
> On Tue, Nov 5, 2019 at 1:43 AM Daniel Vetter <[email protected]> wrote:
> >
> > On Mon, Nov 04, 2019 at 10:57:44AM -0800, John Stultz wrote:
> > > On Mon, Nov 4, 2019 at 1:58 AM Daniel Vetter <[email protected]> wrote:
> > > > On Fri, Oct 25, 2019 at 11:48:32PM +0000, John Stultz wrote:
> > > So even if the heaps are configured via DT (which at the moment there
> > > is no such binding, so that's not really a valid method yet), there's
> > > still the question of if the heap is necessary/makes sense on the
> > > device. And the DT would only control the availability of the heap
> > > interface, not if the heap driver is loaded or not.
> >
> > Hm I thought the cma regions are configured in DT? How does that work if
> > it's not using DT?
>
> So yea, CMA regions are either configured by DT or setup at build time
> (I think there's a cmdline option to set it up as well).
>
> But the CMA regions and the dmabuf cma heap driver are separate
> things. The latter uses the former.
Huh, I assumed the plan is that whenever there's a cma region, we want
to instantiate a dma-buf heap for it? Why/when would we not want to do
that?
> > > On the HiKey/HiKey960 boards, we have to allocate contiguous buffers
> > > for the display framebuffer. So gralloc uses ION to allocate from the
> > > CMA heap. However on the db845c, it has no such restrictions, so the
> > > CMA heap isn't necessary.
> >
> > Why do you have a CMA region for the 2nd board if you don't need it?
> > _That_ sounds like some serious memory waster, not a few lines of code
> > loaded for nothing :-)
>
> ??? That's not what I said above. If the db845c doesn't need CMA it
> won't have a CMA region.
>
> The issue at hand is that we may want to avoid loading the dmabuf CMA
> heap driver on a board that doesn't use CMA.
So the CMA core code is also a module, or how does that work? Not
loading the cma dma-buf heap, but keeping all the other cma code
around feels slightly silly. Do we have numbers that justify this
silliness?
> > > With Android's GKI effort, there needs to be one kernel that works on
> > > all the devices, and they are using modules to try to minimize the
> > > amount of memory spent on functionality that isn't universally needed.
> > > So on devices that don't need the CMA heap, they'd probably prefer not
> > > to load the CMA dmabuf heap driver, so it would be best if it could be
> > > built as a module. If we want to build the CMA heap as a module, the
> > > symbols it uses need to be exported.
> >
> > Yeah, I guess I'm disagreeing on whether dma-buf heaps are core or not.
>
> That's fine to dispute. I'm not really in a place to assert one way or
> not, but the Android folks have made their ION system and CMA heaps
> loadable via a module, so it would seem like having the dmabuf system
> and CMA heaps be modular would be useful to properly replace that
> usage.
>
> For instance, the system heap as a module probably doesn't make much
> sense, as most boards that want to use the dmabuf heaps interface are
> likely to use that as well. CMA is more optional as not all boards
> will use that one, so it might make sense to avoid loading it.
>
> Sandeep: Can you chime in here as to how critical having the system
> and cma heaps be modules are?
>
>
> > > > Exporting symbols for no real in-tree users feels fishy.
> > >
> > > I'm submitting an in-tree user here. So I'm not sure what you mean? I
> > > suspect you're thinking there is some hidden/nefarious plan here, but
> > > really there isn't.
> >
> > I was working under the assumption that you're only exporting the symbols
> > for other heaps, and keep the current ones in-tree.
>
> No. I'm trying to allow the (hopefully-soon-to-be-in-tree) system and
> cma heap drivers to be loaded from a module.
> If other heaps need exports, they can submit their heaps upstream and
> argue for the exported symbols themselves.
>
> > Are there even any
> > out-of-tree dma-buf heaps still? out-of-tree and legit different use-case
> > I mean ofc, not just out-of-tree because inertia :-)
>
> So as Andrew mentioned, he has some dmabuf heaps he's working on at
> TI. From what I've heard the qualcomm folks like the dmabuf heaps
> approach, but I'm not sure if they've taken a pass at converting their
> vendor ION heaps to it yet.
>
> The main reason I'm only submitting system and CMA is because those
> are the only two I personally have a user for (HiKey/HiKey960 boards).
> It's my hope that once we deprecate ION in Android, vendors will
> migrate and we'll be able to push them to upstream their heaps as
> well.
I think for upstream I'd want to see those other heaps first. If
they're mostly driver allocators exposed as heaps, then I think we
need something different than heap modules, maybe allow dma-buf to
allocate from drivers instead. But afaiui all such driver allocators
we have in upstream are cma regions only right now.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
On Tue, Nov 5, 2019 at 11:19 AM Daniel Vetter <[email protected]> wrote:
> On Tue, Nov 5, 2019 at 6:41 PM John Stultz <[email protected]> wrote:
> > On Tue, Nov 5, 2019 at 1:43 AM Daniel Vetter <[email protected]> wrote:
> > >
> > > On Mon, Nov 04, 2019 at 10:57:44AM -0800, John Stultz wrote:
> > > > On Mon, Nov 4, 2019 at 1:58 AM Daniel Vetter <[email protected]> wrote:
> > > > > On Fri, Oct 25, 2019 at 11:48:32PM +0000, John Stultz wrote:
> > > > So even if the heaps are configured via DT (which at the moment there
> > > > is no such binding, so that's not really a valid method yet), there's
> > > > still the question of if the heap is necessary/makes sense on the
> > > > device. And the DT would only control the availability of the heap
> > > > interface, not if the heap driver is loaded or not.
> > >
> > > Hm I thought the cma regions are configured in DT? How does that work if
> > > it's not using DT?
> >
> > So yea, CMA regions are either configured by DT or setup at build time
> > (I think there's a cmdline option to set it up as well).
> >
> > But the CMA regions and the dmabuf cma heap driver are separate
> > things. The latter uses the former.
>
> Huh, I assumed the plan is that whenever there's a cma region, we want
> to instantiate a dma-buf heap for it? Why/when would we not want to do
> that?
Not quite. Andrew noted that we may not want to expose all CMA regions
via dmabuf heaps, so right now we only expose the default region. I
have follow on patches that I sent out earlier (which requires a
to-be-finalized DT binding) which allows us to specify which other CMA
regions to expose.
> > > > On the HiKey/HiKey960 boards, we have to allocate contiguous buffers
> > > > for the display framebuffer. So gralloc uses ION to allocate from the
> > > > CMA heap. However on the db845c, it has no such restrictions, so the
> > > > CMA heap isn't necessary.
> > >
> > > Why do you have a CMA region for the 2nd board if you don't need it?
> > > _That_ sounds like some serious memory waster, not a few lines of code
> > > loaded for nothing :-)
> >
> > ??? That's not what I said above. If the db845c doesn't need CMA it
> > won't have a CMA region.
> >
> > The issue at hand is that we may want to avoid loading the dmabuf CMA
> > heap driver on a board that doesn't use CMA.
>
> So the CMA core code is also a module, or how does that work? Not
No. CMA core isn't available as a module.
> loading the cma dma-buf heap, but keeping all the other cma code
> around feels slightly silly. Do we have numbers that justify this
> silliness?
I agree that is maybe not the most critical item on the list, but its
one of many that do add up over time.
Again, I'll defer to Sandeep or other folks on the Google side to help
with the importance here. Mostly I'm trying to ensure there is
functional parity to ION so we don't give folks any reason they could
object to deprecating it.
> > The main reason I'm only submitting system and CMA is because those
> > are the only two I personally have a user for (HiKey/HiKey960 boards).
> > It's my hope that once we deprecate ION in Android, vendors will
> > migrate and we'll be able to push them to upstream their heaps as
> > well.
>
> I think for upstream I'd want to see those other heaps first. If
> they're mostly driver allocators exposed as heaps, then I think we
> need something different than heap modules, maybe allow dma-buf to
> allocate from drivers instead. But afaiui all such driver allocators
> we have in upstream are cma regions only right now.
I'm sorry, I'm not sure I understand what you mean here (I'm not sure
what action I should be taking). Could you clarify this point?
thanks
-john
On Tue, Nov 5, 2019 at 8:48 PM John Stultz <[email protected]> wrote:
>
> On Tue, Nov 5, 2019 at 11:19 AM Daniel Vetter <[email protected]> wrote:
> > On Tue, Nov 5, 2019 at 6:41 PM John Stultz <[email protected]> wrote:
> > > On Tue, Nov 5, 2019 at 1:43 AM Daniel Vetter <[email protected]> wrote:
> > > >
> > > > On Mon, Nov 04, 2019 at 10:57:44AM -0800, John Stultz wrote:
> > > > > On Mon, Nov 4, 2019 at 1:58 AM Daniel Vetter <[email protected]> wrote:
> > > > > > On Fri, Oct 25, 2019 at 11:48:32PM +0000, John Stultz wrote:
> > > > > So even if the heaps are configured via DT (which at the moment there
> > > > > is no such binding, so that's not really a valid method yet), there's
> > > > > still the question of if the heap is necessary/makes sense on the
> > > > > device. And the DT would only control the availability of the heap
> > > > > interface, not if the heap driver is loaded or not.
> > > >
> > > > Hm I thought the cma regions are configured in DT? How does that work if
> > > > it's not using DT?
> > >
> > > So yea, CMA regions are either configured by DT or setup at build time
> > > (I think there's a cmdline option to set it up as well).
> > >
> > > But the CMA regions and the dmabuf cma heap driver are separate
> > > things. The latter uses the former.
> >
> > Huh, I assumed the plan is that whenever there's a cma region, we want
> > to instantiate a dma-buf heap for it? Why/when would we not want to do
> > that?
>
> Not quite. Andrew noted that we may not want to expose all CMA regions
> via dmabuf heaps, so right now we only expose the default region. I
> have follow on patches that I sent out earlier (which requires a
> to-be-finalized DT binding) which allows us to specify which other CMA
> regions to expose.
Why do we not want to expose them all? I figured if there's a cma
heap, then a device you have needs it, and if that's the case you
might want to allocate for that device from the heap? Maybe link to
the discussion?
> > > > > On the HiKey/HiKey960 boards, we have to allocate contiguous buffers
> > > > > for the display framebuffer. So gralloc uses ION to allocate from the
> > > > > CMA heap. However on the db845c, it has no such restrictions, so the
> > > > > CMA heap isn't necessary.
> > > >
> > > > Why do you have a CMA region for the 2nd board if you don't need it?
> > > > _That_ sounds like some serious memory waster, not a few lines of code
> > > > loaded for nothing :-)
> > >
> > > ??? That's not what I said above. If the db845c doesn't need CMA it
> > > won't have a CMA region.
> > >
> > > The issue at hand is that we may want to avoid loading the dmabuf CMA
> > > heap driver on a board that doesn't use CMA.
> >
> > So the CMA core code is also a module, or how does that work? Not
>
> No. CMA core isn't available as a module.
>
> > loading the cma dma-buf heap, but keeping all the other cma code
> > around feels slightly silly. Do we have numbers that justify this
> > silliness?
>
> I agree that is maybe not the most critical item on the list, but its
> one of many that do add up over time.
>
> Again, I'll defer to Sandeep or other folks on the Google side to help
> with the importance here. Mostly I'm trying to ensure there is
> functional parity to ION so we don't give folks any reason they could
> object to deprecating it.
>
> > > The main reason I'm only submitting system and CMA is because those
> > > are the only two I personally have a user for (HiKey/HiKey960 boards).
> > > It's my hope that once we deprecate ION in Android, vendors will
> > > migrate and we'll be able to push them to upstream their heaps as
> > > well.
> >
> > I think for upstream I'd want to see those other heaps first. If
> > they're mostly driver allocators exposed as heaps, then I think we
> > need something different than heap modules, maybe allow dma-buf to
> > allocate from drivers instead. But afaiui all such driver allocators
> > we have in upstream are cma regions only right now.
>
> I'm sorry, I'm not sure I understand what you mean here (I'm not sure
> what action I should be taking). Could you clarify this point?
I'm not sold on the use-case for this, but maybe if I see the actual
use-cases I might be swayed. A very basic/minimal "register a new
dma-buf heap" function should be all that's really needed for android,
so maybe we can start with that?
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
On Tue, Nov 05, 2019 at 09:41:44AM -0800, John Stultz wrote:
> On Tue, Nov 5, 2019 at 1:43 AM Daniel Vetter <[email protected]> wrote:
> >
> > On Mon, Nov 04, 2019 at 10:57:44AM -0800, John Stultz wrote:
> > > On Mon, Nov 4, 2019 at 1:58 AM Daniel Vetter <[email protected]> wrote:
> > > > On Fri, Oct 25, 2019 at 11:48:32PM +0000, John Stultz wrote:
> > > So even if the heaps are configured via DT (which at the moment there
> > > is no such binding, so that's not really a valid method yet), there's
> > > still the question of if the heap is necessary/makes sense on the
> > > device. And the DT would only control the availability of the heap
> > > interface, not if the heap driver is loaded or not.
> >
> > Hm I thought the cma regions are configured in DT? How does that work if
> > it's not using DT?
>
> So yea, CMA regions are either configured by DT or setup at build time
> (I think there's a cmdline option to set it up as well).
>
> But the CMA regions and the dmabuf cma heap driver are separate
> things. The latter uses the former.
>
> > > On the HiKey/HiKey960 boards, we have to allocate contiguous buffers
> > > for the display framebuffer. So gralloc uses ION to allocate from the
> > > CMA heap. However on the db845c, it has no such restrictions, so the
> > > CMA heap isn't necessary.
> >
> > Why do you have a CMA region for the 2nd board if you don't need it?
> > _That_ sounds like some serious memory waster, not a few lines of code
> > loaded for nothing :-)
>
> ??? That's not what I said above. If the db845c doesn't need CMA it
> won't have a CMA region.
>
> The issue at hand is that we may want to avoid loading the dmabuf CMA
> heap driver on a board that doesn't use CMA.
>
>
> > > With Android's GKI effort, there needs to be one kernel that works on
> > > all the devices, and they are using modules to try to minimize the
> > > amount of memory spent on functionality that isn't universally needed.
> > > So on devices that don't need the CMA heap, they'd probably prefer not
> > > to load the CMA dmabuf heap driver, so it would be best if it could be
> > > built as a module. If we want to build the CMA heap as a module, the
> > > symbols it uses need to be exported.
> >
> > Yeah, I guess I'm disagreeing on whether dma-buf heaps are core or not.
>
> That's fine to dispute. I'm not really in a place to assert one way or
> not, but the Android folks have made their ION system and CMA heaps
> loadable via a module, so it would seem like having the dmabuf system
> and CMA heaps be modular would be useful to properly replace that
> usage.
>
> For instance, the system heap as a module probably doesn't make much
> sense, as most boards that want to use the dmabuf heaps interface are
> likely to use that as well. CMA is more optional as not all boards
> will use that one, so it might make sense to avoid loading it.
>
> Sandeep: Can you chime in here as to how critical having the system
> and cma heaps be modules are?
With ion, we are making sure there are *standard* heaps that Android should
be able to rely on to exist in all kernels [1]. That list is based on what
default heaps ion had out-of-tree.
As of today, even from those that ion had, Android vendor independent code only
relies on 'system heap' and 'cma/dma heaps' so, can safely ignore the carveout
and other ion heaps.
system heap is really the one that is realistically 'hardware independent',
so that can be in kernel. The cma heaps and their existence is optional, so
it will be nice to be able to load them as modules.
<snip>
- ssp
1. https://android.googlesource.com/platform/system/core/+/refs/heads/master/libion/kernel-headers/linux/ion_4.19.h#28
On Tue, Nov 05, 2019 at 11:47:53AM -0800, John Stultz wrote:
> On Tue, Nov 5, 2019 at 11:19 AM Daniel Vetter <[email protected]> wrote:
> > On Tue, Nov 5, 2019 at 6:41 PM John Stultz <[email protected]> wrote:
> > > On Tue, Nov 5, 2019 at 1:43 AM Daniel Vetter <[email protected]> wrote:
> > > >
> > > > On Mon, Nov 04, 2019 at 10:57:44AM -0800, John Stultz wrote:
> > > > > On Mon, Nov 4, 2019 at 1:58 AM Daniel Vetter <[email protected]> wrote:
> > > > > > On Fri, Oct 25, 2019 at 11:48:32PM +0000, John Stultz wrote:
> > > > > So even if the heaps are configured via DT (which at the moment there
> > > > > is no such binding, so that's not really a valid method yet), there's
> > > > > still the question of if the heap is necessary/makes sense on the
> > > > > device. And the DT would only control the availability of the heap
> > > > > interface, not if the heap driver is loaded or not.
> > > >
> > > > Hm I thought the cma regions are configured in DT? How does that work if
> > > > it's not using DT?
> > >
> > > So yea, CMA regions are either configured by DT or setup at build time
> > > (I think there's a cmdline option to set it up as well).
> > >
> > > But the CMA regions and the dmabuf cma heap driver are separate
> > > things. The latter uses the former.
> >
> > Huh, I assumed the plan is that whenever there's a cma region, we want
> > to instantiate a dma-buf heap for it? Why/when would we not want to do
> > that?
>
> Not quite. Andrew noted that we may not want to expose all CMA regions
> via dmabuf heaps, so right now we only expose the default region. I
> have follow on patches that I sent out earlier (which requires a
> to-be-finalized DT binding) which allows us to specify which other CMA
> regions to expose.
>
> > > > > On the HiKey/HiKey960 boards, we have to allocate contiguous buffers
> > > > > for the display framebuffer. So gralloc uses ION to allocate from the
> > > > > CMA heap. However on the db845c, it has no such restrictions, so the
> > > > > CMA heap isn't necessary.
> > > >
> > > > Why do you have a CMA region for the 2nd board if you don't need it?
> > > > _That_ sounds like some serious memory waster, not a few lines of code
> > > > loaded for nothing :-)
> > >
> > > ??? That's not what I said above. If the db845c doesn't need CMA it
> > > won't have a CMA region.
> > >
> > > The issue at hand is that we may want to avoid loading the dmabuf CMA
> > > heap driver on a board that doesn't use CMA.
> >
> > So the CMA core code is also a module, or how does that work? Not
>
> No. CMA core isn't available as a module.
>
> > loading the cma dma-buf heap, but keeping all the other cma code
> > around feels slightly silly. Do we have numbers that justify this
> > silliness?
>
> I agree that is maybe not the most critical item on the list, but its
> one of many that do add up over time.
>
> Again, I'll defer to Sandeep or other folks on the Google side to help
> with the importance here. Mostly I'm trying to ensure there is
> functional parity to ION so we don't give folks any reason they could
> object to deprecating it.
Parity with ION will definitely be nice. For now, however, even if we achieve
that parity with UAPI and think about the cma-heap-as-module bit later, I
guess that's ok.
The real problem is the need for these heaps to be a module in the first
place. I'd much rather have an upstream user to show the need for cache
maintenance operations that have been talked about so many times, so we can
make them happen for dma-buf-heaps in upstream. None of this has to be
a module if that happens :(.
The reason for the "modularization" for ion heaps is also the CMOs for
Android use cases. Unfortunately we haven't had any luck with proving the
need for. John, CMIIW.
- ssp