On Mon, Jul 08, 2024 at 09:14:14AM GMT, Christian König wrote: > Am 05.07.24 um 17:35 schrieb Daniel Vetter: > > Just figured I'll jump in on one detail here. > > > > On Fri, Jul 05, 2024 at 04:31:34PM +0200, Thierry Reding wrote: > > > On Thu, Jul 04, 2024 at 02:24:49PM GMT, Maxime Ripard wrote: > > > > On Fri, Jun 28, 2024 at 04:42:35PM GMT, Thierry Reding wrote: > > > > > On Fri, Jun 28, 2024 at 03:08:46PM GMT, Maxime Ripard wrote: > > > > > > Hi, > > > > > > > > > > > > On Fri, Jun 28, 2024 at 01:29:17PM GMT, Thierry Reding wrote: > > > > > > > On Tue, May 21, 2024 at 02:06:19PM GMT, Daniel Vetter wrote: > > > > > > > > On Thu, May 16, 2024 at 09:51:35AM -0700, John Stultz wrote: > > > > > > > > > On Thu, May 16, 2024 at 3:56 AM Daniel Vetter <daniel@xxxxxxxx> wrote: > > > > > > > > > > On Wed, May 15, 2024 at 11:42:58AM -0700, John Stultz wrote: > > > > > > > > > > > But it makes me a little nervous to add a new generic allocation flag > > > > > > > > > > > for a feature most hardware doesn't support (yet, at least). So it's > > > > > > > > > > > hard to weigh how common the actual usage will be across all the > > > > > > > > > > > heaps. > > > > > > > > > > > > > > > > > > > > > > I apologize as my worry is mostly born out of seeing vendors really > > > > > > > > > > > push opaque feature flags in their old ion heaps, so in providing a > > > > > > > > > > > flags argument, it was mostly intended as an escape hatch for > > > > > > > > > > > obviously common attributes. So having the first be something that > > > > > > > > > > > seems reasonable, but isn't actually that common makes me fret some. > > > > > > > > > > > > > > > > > > > > > > So again, not an objection, just something for folks to stew on to > > > > > > > > > > > make sure this is really the right approach. > > > > > > > > > > Another good reason to go with full heap names instead of opaque flags on > > > > > > > > > > existing heaps is that with the former we can use symlinks in sysfs to > > > > > > > > > > specify heaps, with the latter we need a new idea. We haven't yet gotten > > > > > > > > > > around to implement this anywhere, but it's been in the dma-buf/heap todo > > > > > > > > > > since forever, and I like it as a design approach. So would be a good idea > > > > > > > > > > to not toss it. With that display would have symlinks to cma-ecc and cma, > > > > > > > > > > and rendering maybe cma-ecc, shmem, cma heaps (in priority order) for a > > > > > > > > > > SoC where the display needs contig memory for scanout. > > > > > > > > > So indeed that is a good point to keep in mind, but I also think it > > > > > > > > > might re-inforce the choice of having ECC as a flag here. > > > > > > > > > > > > > > > > > > Since my understanding of the sysfs symlinks to heaps idea is about > > > > > > > > > being able to figure out a common heap from a collection of devices, > > > > > > > > > it's really about the ability for the driver to access the type of > > > > > > > > > memory. If ECC is just an attribute of the type of memory (as in this > > > > > > > > > patch series), it being on or off won't necessarily affect > > > > > > > > > compatibility of the buffer with the device. Similarly "uncached" > > > > > > > > > seems more of an attribute of memory type and not a type itself. > > > > > > > > > Hardware that can access non-contiguous "system" buffers can access > > > > > > > > > uncached system buffers. > > > > > > > > Yeah, but in graphics there's a wide band where "shit performance" is > > > > > > > > defacto "not useable (as intended at least)". > > > > > > > > > > > > > > > > So if we limit the symlink idea to just making sure zero-copy access is > > > > > > > > possible, then we might not actually solve the real world problem we need > > > > > > > > to solve. And so the symlinks become somewhat useless, and we need to > > > > > > > > somewhere encode which flags you need to use with each symlink. > > > > > > > > > > > > > > > > But I also see the argument that there's a bit a combinatorial explosion > > > > > > > > possible. So I guess the question is where we want to handle it ... > > > > > > > Sorry for jumping into this discussion so late. But are we really > > > > > > > concerned about this combinatorial explosion in practice? It may be > > > > > > > theoretically possible to create any combination of these, but do we > > > > > > > expect more than a couple of heaps to exist in any given system? > > > > > > I don't worry too much about the number of heaps available in a given > > > > > > system, it would indeed be fairly low. > > > > > > > > > > > > My concern is about the semantics combinatorial explosion. So far, the > > > > > > name has carried what semantics we were supposed to get from the buffer > > > > > > we allocate from that heap. > > > > > > > > > > > > The more variations and concepts we'll have, the more heap names we'll > > > > > > need, and with confusing names since we wouldn't be able to change the > > > > > > names of the heaps we already have. > > > > > What I was trying to say is that none of this matters if we make these > > > > > names opaque. If these names are contextual for the given system it > > > > > doesn't matter what the exact capabilities are. It only matters that > > > > > their purpose is known and that's what applications will be interested > > > > > in. > > > > If the names are opaque, and we don't publish what the exact > > > > capabilities are, how can an application figure out which heap to use in > > > > the first place? > > > This would need to be based on conventions. The idea is to standardize > > > on a set of names for specific, well-known use-cases. > > > > > > > > > > Would it perhaps make more sense to let a platform override the heap > > > > > > > name to make it more easily identifiable? Maybe this is a naive > > > > > > > assumption, but aren't userspace applications and drivers not primarily > > > > > > > interested in the "type" of heap rather than whatever specific flags > > > > > > > have been set for it? > > > > > > I guess it depends on what you call the type of a heap. Where we > > > > > > allocate the memory from, sure, an application won't care about that. > > > > > > How the buffer behaves on the other end is definitely something > > > > > > applications are going to be interested in though. > > > > > Most of these heaps will be very specific, I would assume. > > > > We don't have any specific heap upstream at the moment, only generic > > > > ones. > > > But we're trying to add more specific ones, right? > > > > > > > > For example a heap that is meant to be protected for protected video > > > > > decoding is both going to be created in such a way as to allow that > > > > > use-case (i.e. it doesn't make sense for it to be uncached, for > > > > > example) and it's also not going to be useful for any other use-case > > > > > (i.e. there's no reason to use that heap for GPU jobs or networking, > > > > > or whatever). > > > > Right. But also, libcamera has started to use dma-heaps to allocate > > > > dma-capable buffers and do software processing on it before sending it > > > > to some hardware controller. > > > > > > > > Caches are critical here, and getting a non-cacheable buffer would be > > > > a clear regression. > > > I understand that. My point is that maybe we shouldn't try to design a > > > complex mechanism that allows full discoverability of everything that a > > > heap supports or is capable of. Instead if the camera has specific > > > requirements, it could look for a heap named "camera". Or if it can > > > share a heap with other multimedia devices, maybe call the heap > > > "multimedia". > > > > > > The idea is that heaps for these use-cases are quite specific, so you > > > would likely not find an arbitrary number of processes try to use the > > > same heap. > > Yeah the idea to sort this out was to have symlinks in sysfs from the > > device to each heap. We could then have priorities for each such link, so > > that applications can pick the "best" heap that will work with all > > devices. Or also special links for special use-cases, like for a > > display+render drm device you might want to have separate links for the > > display and the render-only use-case. > > > > I think trying to encode this all into the name of a heap without linking > > it to the device is not going to work well in general. > > > > We still have that entire "make sysfs symlinks work for dma-buf heaps" on > > our todos, and that idea is almost as old as dma-buf itself :-/ > > I still have the draft patches for that lying around on my harddisk > somewhere with zero time to look into it. > > If anybody wants to pick it up feel free to ping me, but be aware that you > need to write more documentation than code. I'm interested, so if you can dig those out that'd be a great reference. Thanks, Thierry
Attachment:
signature.asc
Description: PGP signature