Re: [PATCH 4/4] dma-heap: Devicetree binding for chunk heap

[John Stultz]

On Thu, Dec 10, 2020 at 8:06 AM Minchan Kim <minchan@xxxxxxxxxx> wrote:
> On Thu, Dec 10, 2020 at 12:15:15AM -0800, John Stultz wrote:
> > Well, while I agree that conceptually the dmabuf heaps allow for
> > allocations for multi-device pipelines, and thus are not tied to
> > specific devices. I do think that the memory types exposed are likely
> > to have specific devices/drivers in the pipeline that it matters most
> > to. So I don't see a big issue with the in-kernel driver registering a
> > specific CMA region as a dmabuf heap.
>
> Then, I am worry about that we spread out dma_heap_add_cma to too many
> drivers since kernel doesn't how userspace will use it.
> For example, system 1 could have device A-B-C pipeline so they added
> it A driver. After that, system 2 could have device B-C-D pipeline
> so they add dma_heap_add_cma into B device.

I'm not sure I see this as a major issue? If the drivers add it based
on the dt memory reference, those will be configured to not add
duplicate heaps (and even so the heap driver can also ensure we don't
try to add a heap twice).

> > Yea, an un-upstreamable dummy driver is maybe what it devolves to in
> > the worst case. But I suspect it would be cleaner for a display or ISP
> > driver that benefits most from the heap type to add the reserved
> > memory reference to their DT node, and on init for them to register
> > the region with the dmabuf heap code.
>
> As I mentioned above, it could be a display at this moment but it could
> be different driver next time. If I miss your point, let me know.
>

I guess I just don't see potentially having the registration calls
added to multiple drivers as a big problem.

Ideally, yes, I'd probably rather see a DT node that would allow the
heap driver to register specified regions, but that's been NACKed
multiple times. Given that, having hooks in device drivers to export
the region seems to me like the next best approach, as it avoids DT
ABI ( if ends up its a bad approach, its not something we have to
keep).

The bigger problem right now is not that there are too many places the
registration call would be made from, but that there aren't upstream
drivers which I'm aware of where it would currently make sense to add
specific dma_heap_add_cma() registration hooks to.  We need an
upstream user of Kunihiko Hayashi's patch.

thanks
-john