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. Hans, would you be happy with taking the kernel's KERNEL_VERSION macro and using it to encode the whole version, eg diff --git a/utils/v4l2-compliance/v4l2-compliance.cpp b/utils/v4l2-compliance/v4l2-compliance.cpp index d0441651..c372a15c 100644 --- a/utils/v4l2-compliance/v4l2-compliance.cpp +++ b/utils/v4l2-compliance/v4l2-compliance.cpp @@ -1497,10 +1497,9 @@ int main(int argc, char **argv) printf(", %zd bits, %zd-bit time_t\n", sizeof(void *) * 8, sizeof(time_t) * 8); uname(&uts); sscanf(uts.release, "%d.%d.%d", &v1, &v2, &v3); - if (v1 == 2 && v2 == 6) - kernel_version = v3; + kernel_version = KERNEL_VERSION(v1, v2, v3); if (kernel_version) - printf("Running on 2.6.%d\n", kernel_version); + printf("Running on %d.%d.%d\n", v1, v2, v3); printf("\n"); if (!env_media_apps_color || !strcmp(env_media_apps_color, "auto")) diff --git a/utils/v4l2-compliance/v4l2-compliance.h b/utils/v4l2-compliance/v4l2-compliance.h index ae10bdf9..657f5a2a 100644 --- a/utils/v4l2-compliance/v4l2-compliance.h +++ b/utils/v4l2-compliance/v4l2-compliance.h @@ -61,6 +61,8 @@ extern int kernel_version; extern int media_fd; extern unsigned warnings; +#define KERNEL_VERSION(a,b,c) (((a) << 16) + ((b) << 8) + (c)) + enum poll_mode { POLL_MODE_NONE, POLL_MODE_SELECT, diff --git a/utils/v4l2-compliance/v4l2-test-buffers.cpp b/utils/v4l2-compliance/v4l2-test-buffers.cpp index b0b878c1..7e6fb30e 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 (kernel_version >= KERNEL_VERSION(5, 9, 0)) { fail_on_test(flags & V4L2_BUF_FLAG_NO_CACHE_INVALIDATE); fail_on_test(flags & V4L2_BUF_FLAG_NO_CACHE_CLEAN); } 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. Cheers Dave
