On 08/07/2020 11:30, Dave Stevenson wrote: > Hi Sergey & Hans > > On Wed, 8 Jul 2020 at 09:22, Hans Verkuil <hverkuil@xxxxxxxxx> wrote: >> >> On 08/07/2020 05:59, Sergey Senozhatsky wrote: >>> On (20/07/07 15:41), Dave Stevenson wrote: >>>>> v4l2-compliance tests are in sync with our master and it expects that >>>>> that kernel is used. >>>> >>>> Thanks, I'd never noted that restriction. All previous times I'd used >>>> v4l2-compliance against any kernel it had performed as expected. This >>>> is the first change that causes a major failure of tests due to an >>>> older kernel. >>> >>> It depends on Linux UAPI headers, so the restriction is sort of mandated, >>> but probably not widely recognized the by the distributions, looking at >>> arch, for instance: >>> https://git.archlinux.org/svntogit/packages.git/tree/trunk/PKGBUILD?h=packages/v4l-utils > > Except that v4l-utils has a private copy of all the headers in > https://git.linuxtv.org/v4l-utils.git/tree/include/linux, so it builds > independently of the distribution's kernel headers. > >>> Adding a Linux version check code can be a bit intrusive. There has been >>> a whole bunch of changes all over the place, for instance: >>> >>> --- >>> >>> +++ b/utils/v4l2-compliance/v4l2-test-buffers.cpp >>> @@ -381,8 +381,6 @@ int buffer::check(unsigned type, unsigned memory, unsigned index, >>> if (g_flags() & V4L2_BUF_FLAG_BFRAME) >>> frame_types++; >>> fail_on_test(frame_types > 1); >>> - fail_on_test(g_flags() & (V4L2_BUF_FLAG_NO_CACHE_INVALIDATE | >>> - V4L2_BUF_FLAG_NO_CACHE_CLEAN)); >>> --- >>> >>> So running newer v4l-compliance against the older kernel or older >>> v4l-compliance against the newer kernel may trigger various failures. >> >> It shouldn't. It's (I believe) just the check that those two flags >> are cleared if cache hints are not supported that should be under a >> version check. > > Indeed, the only change I need to make for the tests to work is to > disable the two lines in the else clause: > > --- a/utils/v4l2-compliance/v4l2-test-buffers.cpp > +++ b/utils/v4l2-compliance/v4l2-test-buffers.cpp > @@ -1272,7 +1272,7 @@ static int setupMmap(struct node *node, cv4l_queue &q) > if (cache_hints) { > fail_on_test(!(flags & > V4L2_BUF_FLAG_NO_CACHE_INVALIDATE)); > fail_on_test(!(flags & V4L2_BUF_FLAG_NO_CACHE_CLEAN)); > - } else { > + } else if (0) { > fail_on_test(flags & V4L2_BUF_FLAG_NO_CACHE_INVALIDATE); > fail_on_test(flags & V4L2_BUF_FLAG_NO_CACHE_CLEAN); > } > > I started looking at kernel_version, but currently it only gets set > for 2.6.x kernels, and is set to the x. Try this patch: Signed-off-by: Hans Verkuil <hverkuil-cisco@xxxxxxxxx> --- diff --git a/utils/v4l2-compliance/v4l2-compliance.cpp b/utils/v4l2-compliance/v4l2-compliance.cpp index d0441651..e32b57e3 100644 --- a/utils/v4l2-compliance/v4l2-compliance.cpp +++ b/utils/v4l2-compliance/v4l2-compliance.cpp @@ -637,6 +637,8 @@ static int testCap(struct node *node) warn("media bus_info '%s' differs from V4L2 bus_info '%s'\n", node->media_bus_info.c_str(), vcap.bus_info); fail_on_test((vcap.version >> 16) < 3); + if (vcap.version >= 0x050900) // Present from 5.9.0 onwards + node->might_support_cache_hints = true; fail_on_test(check_0(vcap.reserved, sizeof(vcap.reserved))); caps = vcap.capabilities; dcaps = vcap.device_caps; diff --git a/utils/v4l2-compliance/v4l2-compliance.h b/utils/v4l2-compliance/v4l2-compliance.h index ae10bdf9..38a4ded3 100644 --- a/utils/v4l2-compliance/v4l2-compliance.h +++ b/utils/v4l2-compliance/v4l2-compliance.h @@ -146,6 +146,8 @@ struct base_node { __u32 valid_buftype; __u32 valid_memorytype; bool supports_orphaned_bufs; + // support for this was introduced in 5.9 + bool might_support_cache_hints; }; struct node : public base_node, public cv4l_fd { diff --git a/utils/v4l2-compliance/v4l2-test-buffers.cpp b/utils/v4l2-compliance/v4l2-test-buffers.cpp index b0b878c1..cdfbbd34 100644 --- a/utils/v4l2-compliance/v4l2-test-buffers.cpp +++ b/utils/v4l2-compliance/v4l2-test-buffers.cpp @@ -1272,7 +1272,7 @@ static int setupMmap(struct node *node, cv4l_queue &q) if (cache_hints) { fail_on_test(!(flags & V4L2_BUF_FLAG_NO_CACHE_INVALIDATE)); fail_on_test(!(flags & V4L2_BUF_FLAG_NO_CACHE_CLEAN)); - } else { + } else if (node->might_support_cache_hints) { fail_on_test(flags & V4L2_BUF_FLAG_NO_CACHE_INVALIDATE); fail_on_test(flags & V4L2_BUF_FLAG_NO_CACHE_CLEAN); } This will work as well for people who use the media_build system to backport the media subsystem from our tree to an older kernel. > > Hans, would you be happy with taking the kernel's KERNEL_VERSION macro > and using it to encode the whole version, eg Not worth the effort, but if we need more of such checks, then I might change my mind :-) <snip> > diff --git a/utils/v4l2-compliance/v4l2-test-controls.cpp > b/utils/v4l2-compliance/v4l2-test-controls.cpp > index d81dddb2..dde661ed 100644 > --- a/utils/v4l2-compliance/v4l2-test-controls.cpp > +++ b/utils/v4l2-compliance/v4l2-test-controls.cpp > @@ -411,7 +411,8 @@ int testSimpleControls(struct node *node) > ctrl.value = 0; > // This call will crash on kernels <= 2.6.37 > for control classes due to > // a bug in v4l2-ctrls.c. So skip this on those kernels. > - if (kernel_version < 38 && qctrl.type == > V4L2_CTRL_TYPE_CTRL_CLASS) > + if (kernel_version < KERNEL_VERSION(2, 6, 38) && > + qctrl.type == V4L2_CTRL_TYPE_CTRL_CLASS) > ret = EACCES; > else > ret = doioctl(node, VIDIOC_S_CTRL, &ctrl); > > Actually isn't the current test of kernel_version in > v4l2-test-controls.cpp wrong? > If running on a 3.x, 4.x, or 5.x kernel then kernel_version will be > left at 0. 0 < 38, so we fall into the ret = EACESS; clause and skip > the test. > Is that what was desired? Presumably the bug in v4l2-ctls.c referenced > is fixed in all >= 3.x kernels. I obviously don't want to alter the > behaviour in an incorrect manner here. Yup, I'm about to commit a patch that removes kernel_version and kills this workaround in testSimpleControls(). I really don't care about such old kernels. Thanks for reporting this, good catch! Regards, Hans > > Cheers > Dave >
