Em Tue, 27 May 2025 22:21:42 +0900 Alexandre Courbot <gnu...@gmail.com> escreveu:
> On Tue, May 27, 2025 at 6:13 PM Mauro Carvalho Chehab > <mchehab+hua...@kernel.org> wrote: > > > > Em Tue, 27 May 2025 15:14:50 +0900 > > "Alexandre Courbot" <gnu...@gmail.com> escreveu: > > > > > Hi Mauro, > > > > > > On Mon May 26, 2025 at 9:13 PM JST, Mauro Carvalho Chehab wrote: > > > > Hi Michael, > > > > > > > > Em Sat, 12 Apr 2025 13:08:01 +0900 > > > > Alexandre Courbot <gnu...@gmail.com> escreveu: > > > > > > > >> Add the first version of the virtio-media driver. > > > >> > > > >> This driver acts roughly as a V4L2 relay between user-space and the > > > >> virtio virtual device on the host, so it is relatively simple, yet > > > >> unconventional. It doesn't use VB2 or other frameworks typically used > > > >> in > > > >> a V4L2 driver, and most of its complexity resides in correctly and > > > >> efficiently building the virtio descriptor chain to pass to the host, > > > >> avoiding copies whenever possible. This is done by > > > >> scatterlist_builder.[ch]. > > > >> > > > >> virtio_media_ioctls.c proxies each supported ioctl to the host, using > > > >> code generated through macros for ioctls that can be forwarded > > > >> directly, > > > >> which is most of them. > > > >> > > > >> virtio_media_driver.c provides the expected driver hooks, and support > > > >> for mmapping and polling. > > > >> > > > >> This version supports MMAP buffers, while USERPTR buffers can also be > > > >> enabled through a driver option. DMABUF support is still pending. > > > > > > > > It sounds that you applied this one at the virtio tree, but it hasn't > > > > being reviewed or acked by media maintainers. > > > > > > > > Please drop it. > > > > > > > > Alexandre, > > > > > > > > Please send media patches to media maintainers, c/c other subsystem > > > > maintainers, as otherwise they might end being merged without a > > > > proper review. > > > > > > Sorry about that, I put everyone in "To:" without giving it a second > > > thought. > > > > > > > > > > > In this particular case, we need to double-check if this won't cause > > > > any issues, in special with regards to media locks and mutexes. > > > > > > Agreed, I am not 100% confident about that part myself. > > > > > > > > > > > I'll try to look on it after this merge window, as it is too late > > > > for it to be applied during this one. > > > > > > Appreciate that - given the high traffic on the list I was worried that > > > this patch would eventually be overlooked. Not making it for this merge > > > window should not be a problem, so please take the time you need. > > > > Provided that your patch was caught by patchwork, it won't be lost: > > > > https://patchwork.linuxtv.org/project/linux-media/patch/20250412-virtio-media-v3-1-97dc94c18...@gmail.com/ > > > > Please notice that our CI got a number of checkpatch issues there. > > Please check and fix the non-false-positive ones. > > Will do. The macro-related ones are false-positives AFAICT. Some of > the "lines should not end with a '('" are actually the result of > applying clang-format with the kernel-provided style... I don't know any lint tool that honors kernel style. The best one is checkpatch with the auto-correcting parameter in pedantic mode, but still one needs to manually review its output. > > > > > Btw, I was looking at: > > > > https://github.com/chromeos/virtio-media > > > > (I'm assuming that this is the QEMU counterpart, right?) > > crosvm actually, but QEMU support is also being worked on. Do you have already QEMU patches? The best is to have the Kernel driver submitted altogether with QEMU, as Kernel developers need it to do the tests. In my case, I never use crosvm, and I don't have any Chromebook anymore. > > And I noticed something weird there: > > > > "Unsupported ioctls > > > > A few ioctls are replaced by other, more suitable mechanisms. If > > being requested these ioctls, the device must return the same response as > > it would for an unknown ioctl, i.e. ENOTTY. > > > > VIDIOC_QUERYCAP is replaced by reading the configuration area. > > VIDIOC_DQBUF is replaced by a dedicated event. > > VIDIOC_DQEVENT is replaced by a dedicated event." > > > > While this could be ok for cromeOS, this will be broken for guests with > > Linux, as all Linux applications rely on VIDIOC_QUERYCAP and VIDIOC_DQBUF > > to work. Please implement support for it, as otherwise we won't even be > > able to test the driver with the v4l2-compliance tool (*). > > The phrasing was a bit confusing. The guest driver does support these > ioctls, and passes v4l2-compliance. So there is no problem here. Please add v4l2-compliance output on the next patch series. > Where these ioctls are not supported is between the guest and the > host, i.e. as ioctls encapsulated into a virtio request. For QUERYCAP, > that's because the virtio configuration area already provides the same > information. For DQBUF and DQEVENT, that's because we want to serve > these asynchronously for performance reasons (hence the dedicated > events). > > But these differences don't affect guest user applications which will > be able to perform these ioctls exactly as they expect. OK. Better to let it clear then at the documentation. > > > > (*) Passing at v4l2-compliance is a requirement for any media driver > > to be merged. > > > > With regards to testing, what's the expected testing scenario? > > My guess is that, as a virtio device, a possible test scenario would be > > to have the UVC camera from the host OS mapped using virtio-camera into > > the guest OS, allowing a V4L2 application running at the guest to map the > > camera from the host, right? > > That's one of the scenarios, yes. Ok, this is the easiest test scenario for media developers. > Another one is to expose an accelerated decoder on the host to the guest. > One can also create > "fake" devices backed by software on the host for testing purposes. That sounds interesting. It probably makes sense to have some test scenario using such fake devices plus v4l2-compliance test to check for regressions running on some CI engine. > That's actually the benefit of using V4L2 as a guest/host protocol: > the same guest and the same software stack on the host can be used to > virtualize multiple kinds of media devices, removing the need to have > e.g. virtio-camera and virtio-codec. This removes a ton of work (and > code). Makes sense. Regards, Mauro
