On 11/12/2018 10:29 AM, Philipp Zabel wrote: > Hi Tomasz, > > On Sun, 2018-11-11 at 12:43 +0900, Tomasz Figa wrote: >> On Sat, Nov 10, 2018 at 6:06 AM Nicolas Dufresne <nicolas@xxxxxxxxxxxx> wrote: >>> >>> Le jeudi 08 novembre 2018 à 16:45 +0900, Tomasz Figa a écrit : >>>>> In this patch we should consider a way to tell userspace that this has >>>>> been opt in, otherwise existing userspace will have to remain using >>>>> sub-optimal copy based reclaiming in order to ensure that renegotiation >>>>> can work on older kernel tool. At worst someone could probably do trial >>>>> and error (reqbufs(1)/mmap/reqbufs(0)) but on CMA with large buffers >>>>> this introduces extra startup time. >>>> >>>> Would such REQBUFS dance be really needed? Couldn't one simply try >>>> reqbufs(0) when it's really needed and if it fails then do the copy, >>>> otherwise just proceed normally? >>> >>> In simple program, maybe, in modularized code, where the consumer of >>> these buffer (the one that is forced to make a copy) does not know the >>> origin of the DMABuf, it's a bit complicated. >>> >>> In GStreamer as an example, the producer is a plugin called >>> libgstvideo4linux2.so, while the common consumer would be libgstkms.so. >>> They don't know each other. The pipeline would be described as: >>> >>> v4l2src ! kmssink >>> >>> GStreamer does not have an explicit reclaiming mechanism. No one knew >>> about V4L2 restrictions when this was designed, DMABuf didn't exist and >>> GStreamer didn't have OMX support. >>> >>> What we ended up crafting, as a plaster, is that when upstream element >>> (v4l2src) query a new allocation from downstream (kmssink), we always >>> copy and return any ancient buffers by copying. kmssink holds on a >>> buffer because we can't remove the scannout buffer on the display. This >>> is slow and inefficient, and also totally unneeded if the dmabuf >>> originate from other kernel subsystems (like DRM). >>> >>> So what I'd like to be able to do, to support this in a more optimal >>> and generic way, is to mark the buffers that needs reclaiming before >>> letting them go. But for that, I would need a flag somewhere to tell me >>> this kernel allow this. >> >> Okay, got it. Thanks for explaining it. >> >>> >>> You got the context, maybe the conclusion is that I should simply do >>> kernel version check, though I'm sure a lot of people will backport >>> this, which means that check won't work so well. >>> >>> Let me know, I understand adding more API is not fun, but as nothing is >>> ever versionned in the linux-media world, it's really hard to detect >>> and use new behaviour while supporting what everyone currently run on >>> their systems. >>> >>> I would probably try and find a way to implement your suggestion, and >>> then introduce a flag in the query itself, but I would need to think >>> about it a little more. It's not as simple as it look like >>> unfortunately. >> >> It sounds like a good fit for a new capability in v4l2_requestbuffers >> and v4l2_create_buffers structs [1]. Perhaps something like >> V4L2_BUF_CAP_SUPPORTS_FREE_AFTER_EXPORT? Hans, what do you think? > > Maybe V4L2_BUF_CAP_SUPPORTS_ORPHANS? With this patch, while the buffers > are in use, reqbufs(0) doesn't free them, they are orphaned. Also, this > patch allows reqbufs(0) not only after export, but also while mmapped. Signaling this through a BUF_CAP makes sense. It's really what it is there for. If someone can make an updated patch of https://lore.kernel.org/patchwork/patch/607853/, then it makes sense to merge it. How about: 'SUPPORTS_ORPHANED_BUFS'? Regards, Hans > > regards > Philipp >
