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? [1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/include/uapi/linux/videodev2.h?h=next-20181109#n866 Best regards, Tomasz
