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

[GStreamer (GNOME Bugzilla)]

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

--- Comment #9 from Benjamin Gaignard <benjamin.gaignard at gmail.com> ---
Review of attachment 333309:
 --> (https://bugzilla.gnome.org/review?bug=768794&attachment=333309)

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.

::: 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.

@@ +95,3 @@
+
+  return quark;
+}

why do you need quark ?

@@ +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 ?
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.

@@ +121,3 @@
+  allocation_data.align = params->align;
+  allocation_data.heap_id_mask = ION_HEAP_TYPE_DMA_MASK;
+  allocation_data.flags = 0;

you can pass params->flags here

@@ +226,3 @@
+  allocator->mem_type = GST_ALLOCATOR_ION;
+  allocator->mem_map = NULL;
+  allocator->mem_unmap = NULL;

why overwrite mem_map and mem_unmap ?
gst_fd_mem_map and gst_fd_mem_unmap default functions are fine here

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