[Bug 768794] ion: DMA Buf allocator based on ion

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=768794

--- Comment #14 from Nicolas Dufresne (stormer) <nicolas at ndufresne.ca> ---
(In reply to Benjamin Gaignard from comment #9)
> Review of attachment 333309 [details] [review]:
> 
> I really would like to this in gst-plugins-base allocator director.
> Anyway in -bad that good enough for me now.
> Get that will be really helpful since it is a good way to allocate
> contiguous memory.

I agree.

> 
> ::: gst-libs/gst/ion/gstionmemory.c
> @@ +38,3 @@
> +#define PAGE_ALIGN(x) (((x) + 4095) & ~4095)
> +
> +G_DEFINE_TYPE (GstIONAllocator, gst_ion_allocator, GST_TYPE_ALLOCATOR)
> 
> Since ION driver use dmabuf I think it should inherit of
> GST_TYPE_DMABUF_ALLOCATOR
> so a function like gst_is_dmabuf_memory() will return true.

That was non obvious, I have attached a patch that make it sub-classable. I
wanted to do this in 1.11 anyway.

> 
> @@ +120,3 @@
> +  allocation_data.len = ion_size;
> +  allocation_data.align = params->align;
> +  allocation_data.heap_id_mask = ION_HEAP_TYPE_DMA_MASK;
> 
> ION_HEAP_TYPE_DMA_MASK is fine for me because I want to allocate (by
> default) contiguous memory.
> Maybe we should have an allocator for each heap type ?

Seems like a big amount of work for just a flag. We could register multiple
allocator instance for each type (without creating more GType). For that, we
just need that to be configurable (like a construct parameter or property). 

> named ionsystem, ionsystem_contig, ioncarveout, iondma.
> You could extend gst_ion_alloc_alloc with one parameter for the type and
> only create alloc function for each allocator.

No need to introduce a custom allocation. You can (and should) extend
GstMemoryFlags.

IMPORTANT note, if you subclass, don't forget to clear
GST_ALLOCATOR_FLAG_CUSTOM_ALLOC in your _init() function.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.