On Thu, Dec 10, 2020 at 5:10 AM Daniel Vetter <daniel@xxxxxxxx> wrote: > On Thu, Dec 10, 2020 at 1:06 PM Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote: > > On Thu, Dec 10, 2020 at 12:26:01PM +0100, Daniel Vetter wrote: > > > On Thu, Dec 10, 2020 at 11:55 AM Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote: > > > > On Thu, Dec 10, 2020 at 11:27:27AM +0100, Daniel Vetter wrote: > > > > > This only shows shared memory, so it does smell a lot like $specific_issue > > > > > and we're designing a narrow solution for that and then have to carry it > > > > > forever. > > > > > > > > I think the "issue" is that this was a feature from ion that people > > > > "missed" in the dmabuf move. Taking away the ability to see what kind > > > > of allocations were being made didn't make a lot of debugging tools > > > > happy :( > > > > > > If this is just for dma-heaps then why don't we add the stuff back > > > over there? It reinforces more that the android gpu stack and the > > > non-android gpu stack on linux are fairly different in fundamental > > > ways, but that's not really new. > > > > Back "over where"? > > > > dma-bufs are not only used for the graphics stack on android from what I > > can tell, so this shouldn't be a gpu-specific issue. > > dma-buf heaps exist because android, mostly because google mandates > it. So, I don't think that's fair. dma-buf heaps and ION before exist because it solves a problem they have for allocating shared buffers for multiple complicated multi-device pipelines where the various devices have constraints. It's not strictly required[1], as your next point makes clear (along with ChromeOS's Android not using it). > There's not a whole lot (meaning zero) of actually open gpu stacks > around that run on android and use dma-buf heaps like approved google > systems, largely because the gralloc implementation in mesa just > doesnt. So yes, db845c currently uses the gbm_gralloc, which doesn't use dmabuf heaps or ION. That said, the resulting system still uses quite a number of dmabufs, as Hridya's patch shows: ==> /sys/kernel/dmabuf/28435/exporter_name <== drm ==> /sys/kernel/dmabuf/28435/dev_map_info <== ==> /sys/kernel/dmabuf/28435/size <== 16384 ==> /sys/kernel/dmabuf/28161/exporter_name <== drm ==> /sys/kernel/dmabuf/28161/dev_map_info <== ==> /sys/kernel/dmabuf/28161/size <== 524288 ==> /sys/kernel/dmabuf/30924/exporter_name <== drm ==> /sys/kernel/dmabuf/30924/dev_map_info <== ==> /sys/kernel/dmabuf/30924/size <== 8192 ==> /sys/kernel/dmabuf/26880/exporter_name <== drm ==> /sys/kernel/dmabuf/26880/dev_map_info <== ==> /sys/kernel/dmabuf/26880/size <== 262144 ... So even when devices are not using dma-buf heaps (which I get, you have an axe to grind with :), having some way to collect useful stats for dmabufs in use can be valuable. (Also one might note, the db845c also doesn't have many constrained devices, and we've not yet enabled hw codec support or camera pipelines, so it avoids much of the complexity that ION/dma-buf heaps was created to solve) > So if android needs some quick debug output in sysfs, we can just add > that in dma-buf heaps, for android only, problem solved. And much less > annoying review to make sure it actually fits into the wider ecosystem > because as-is (and I'm not seeing that chance anytime soon), dma-buf > heaps is for android only. dma-buf at large isn't, so merging a debug > output sysfs api that's just for android but misses a ton of the more > generic features and semantics of dma-buf is not great. The intent behind this patch is *not* to create more Android-specific logic, but to provide useful information generically. Indeed, Android does use dmabufs heavily for passing buffers around, and your point that not all systems handle graphics buffers that way is valid, and it's important we don't bake any Android-isms into the interface. But being able to collect data about the active dmabufs in a system is useful, regardless of how regardless of how the dma-buf was allocated. So I'd much rather see your feedback on how we expose other aspects of dmabufs (dma_resv, exporter devices, attachment links) integrated, rather then trying to ghettoize it as android-only and limit it to the dmabuf heaps, where I don't think it makes as much sense to add. thanks -john [1] Out of the box, the codec2 code added a few years back does directly call to ION (and now dmabuf heaps) for system buffers, but it can be configured differently as it's used in ChromeOS's Android too.
