Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org 9BDC960312
Date:   Wed, 13 Mar 2019 13:11:28 -0700 (PDT)
From:   Liam Mark <lmark@codeaurora.org>
To:     John Stultz <john.stultz@linaro.org>
cc:     lkml <linux-kernel@vger.kernel.org>,
        Laura Abbott <labbott@redhat.com>,
        Benjamin Gaignard <benjamin.gaignard@linaro.org>,
        Greg KH <gregkh@linuxfoundation.org>,
        Sumit Semwal <sumit.semwal@linaro.org>,
        Brian Starkey <Brian.Starkey@arm.com>,
        "Andrew F . Davis" <afd@ti.com>, Chenbo Feng <fengc@google.com>,
        Alistair Strachan <astrachan@google.com>,
        dri-devel <dri-devel@lists.freedesktop.org>
Subject: Re: [RFC][PATCH 0/5 v2] DMA-BUF Heaps (destaging ION)
In-Reply-To: <1551819273-640-1-git-send-email-john.stultz@linaro.org>
Message-ID: <alpine.DEB.2.10.1903131309260.5023@lmark-linux.qualcomm.com>
References: <1551819273-640-1-git-send-email-john.stultz@linaro.org>
User-Agent: Alpine 2.10 (DEB 1266 2009-07-14)
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Tue, 5 Mar 2019, John Stultz wrote:

> Here is a initial RFC of the dma-buf heaps patchset Andrew and I
> have been working on which tries to destage a fair chunk of ION
> functionality.
> 
> The patchset implements per-heap devices which can be opened
> directly and then an ioctl is used to allocate a dmabuf from the
> heap.
> 
> The interface is similar, but much simpler then IONs, only
> providing an ALLOC ioctl.
> 
> Also, I've provided simple system and cma heaps. The system
> heap in particular is missing the page-pool optimizations ION
> had, but works well enough to validate the interface.
> 
> I've booted and tested these patches with AOSP on the HiKey960
> using the kernel tree here:
>   https://git.linaro.org/people/john.stultz/android-dev.git/log/?h=dev/dma-buf-heap
> 
> And the userspace changes here:
>   https://android-review.googlesource.com/c/device/linaro/hikey/+/909436
> 
> 
> Compared to ION, this patchset is missing the system-contig,
> carveout and chunk heaps, as I don't have a device that uses
> those, so I'm unable to do much useful validation there.
> Additionally we have no upstream users of chunk or carveout,
> and the system-contig has been deprecated in the common/andoid-*
> kernels, so this should be ok.
> 
> I've also removed the stats accounting for now, since it should
> be implemented by the heaps themselves.
> 
> Eventual TODOS:
> * Reimplement page-pool for system heap (working on this)
> * Add stats accounting to system/cma heaps
> * Make the kselftest actually useful
> * Add other heaps folks see as useful (would love to get
>   some help from actual carveout/chunk users)!

We use a modified carveout heap for certain secure use cases.
Although there would probably be some benefit in discssing how the dma-buf 
heap framework may want to support 
secure heaps in the future it is a large topic which I assume you don't 
want to tackle now.

We don't have any non-secure carveout heap use cases but the client use 
case I have seen usually revolve around
wanting large allocations to succeed very quickly.
For example I have seen camera use cases which do very large allocations 
on camera bootup from the carveout heap, these allocations would come from 
the carveout heap and fallback to the system heap when the carveout heap 
was full.
Actual non-secure carveout heap can perhaps provide more detail.

> 
> That said, the main user-interface is shaping up and I wanted
> to get some input on the device model (particularly from GreKH)
> and any other API/ABI specific input. 
> 
> thanks
> -john

Thanks John and Andrew, it's great to see this effort to get this 
functionality out of staging.

Since we are making some fundamental changes to how ION worked and since 
Android is likely also be the largest user of the dma-buf heaps framework 
I think it would be good
to have a path to resolve the issues which are currently preventing 
commercial Android releases from moving to the upstream version of ION.

I can understand if you don't necessarily want to put all/any of these 
changes into the dma-buf heaps framework as part of this series, but my 
hope is we can get 
the upstream community and the Android framework team to agree on what 
upstreamable changes to dma-buf heaps framework, and/or the Android 
framework, would be required in order for Android to move to the upstream 
dma-buf heaps framework for commercial devices.

I don't mean to make this specific to Android, but my assumption is that 
many of the ION/dma-buf heaps issues which affect Android would likely 
affect other new large users of the dma-buf heaps framework, so if we 
resolve it for Android we would be helping these future users as well.
And I do understand that some the issues facing Android may need to be 
resolved by making changes to Android framework.

I think it would be helpful to try and get as much of this agreed upon as 
possible before the dma-buf heaps framework moves out of staging.

As part of my review I will highlight some of the issues which would 
affect Android. 
In my comments I will apply them to the system heap since that is what 
Android currently uses for a lot of its use cases. 
I realize that this new framework provides more flexibility to heaps, so 
perhaps some of these issues can be solved by creating a new type of 
system heap which Android can use, but even if the solution involves 
creating a new system heap I would like to make sure that this "new" 
system heap is upstreamable.

Liam

Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project