| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 8 Jul 2016 13:44:41 +0200 From: Hans Verkuil <hverkuil@...all.nl> To: tiffany lin <tiffany.lin@...iatek.com> Cc: Hans Verkuil <hans.verkuil@...co.com>, daniel.thompson@...aro.org, Rob Herring <robh+dt@...nel.org>, Mauro Carvalho Chehab <mchehab@....samsung.com>, Matthias Brugger <matthias.bgg@...il.com>, Daniel Kurtz <djkurtz@...omium.org>, Pawel Osciak <posciak@...omium.org>, Eddie Huang <eddie.huang@...iatek.com>, Yingjoe Chen <yingjoe.chen@...iatek.com>, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, linux-media@...r.kernel.org, linux-mediatek@...ts.infradead.org, PoChun.Lin@...iatek.com, Kamil Debski <k.debski@...sung.com>, Marek Szyprowski <m.szyprowski@...sung.com> Subject: Re: [PATCH v3 0/9] Add MT8173 Video Decoder Driver On 07/07/2016 12:16 PM, tiffany lin wrote: > Hi Hans, > > > On Wed, 2016-07-06 at 15:19 +0200, Hans Verkuil wrote: >> Hi Tiffany, >> >> I plan to review this patch series on Friday, but one obvious question is >> what the reason for these failures is: >> >>> Input/Output configuration ioctls: >>> test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported) >>> test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported) >>> test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported) >>> test VIDIOC_G/S_EDID: OK (Not Supported) >>> >>> Control ioctls: >>> test VIDIOC_QUERYCTRL/MENU: OK >>> fail: ../../../v4l-utils-1.6.0/utils/v4l2-compliance/v4l2-test-controls.cpp(357): g_ctrl returned an error (11) >>> test VIDIOC_G/S_CTRL: FAIL >>> fail: ../../../v4l-utils-1.6.0/utils/v4l2-compliance/v4l2-test-controls.cpp(579): g_ext_ctrls returned an error (11) >>> test VIDIOC_G/S/TRY_EXT_CTRLS: FAIL > These fails are because VIDIOC_G_CTRL and VIDIOC_G_EXT_CTRLS return > V4L2_CID_MIN_BUFFERS_FOR_CAPTURE only when dirver in MTK_STATE_HEADER > state, or it will return EAGAIN. > This could help user space get correct value, not default value that may > changed base on media content. OK, I really don't like this. I also looked what the s5p-mfc-dec driver does (the only other driver currently implementing this), and that returns -EINVAL. My proposal would be to change this. If this information isn't known yet, why not just return 0 as the value? The doc would have to be updated and (preferably) also the s5p-mfc-dec driver. I've added Samsung devs to the Cc list, let me know what you think. > >>> fail: ../../../v4l-utils-1.6.0/utils/v4l2-compliance/v4l2-test-controls.cpp(721): subscribe event for control 'User Controls' failed >>> test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: FAIL > Driver do not support subscribe event for control 'User Controls' for > now. > Do we need to support this? I don't see why this would fail. It's OK to subscribe to such controls, although you'll never get an event. >>> test VIDIOC_G/S_JPEGCOMP: OK (Not Supported) >>> Standard Controls: 2 Private Controls: 0 >>> >>> Format ioctls: >>> test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK >>> test VIDIOC_G/S_PARM: OK (Not Supported) >>> test VIDIOC_G_FBUF: OK (Not Supported) >>> fail: ../../../v4l-utils-1.6.0/utils/v4l2-compliance/v4l2-test-formats.cpp(405): expected EINVAL, but got 11 when getting format for buftype 9 >>> test VIDIOC_G_FMT: FAIL > This is because vidioc_vdec_g_fmt only succeed when context is in > MTK_STATE_HEADER state, or user space cannot get correct format data > using this function. Comparing this to s5p-mfc-dec I see that -EINVAL is returned in that case. I am not opposed to using EAGAIN in s5p-mfc-dec as well. Marek, Kamil, what is your opinion? > >>> test VIDIOC_TRY_FMT: OK (Not Supported) >>> test VIDIOC_S_FMT: OK (Not Supported) >>> test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported) >>> >>> Codec ioctls: >>> test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported) >>> test VIDIOC_G_ENC_INDEX: OK (Not Supported) >>> test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported) >>> >>> Buffer ioctls: >>> test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK >>> fail: ../../../v4l-utils-1.6.0/utils/v4l2-compliance/v4l2-test-buffers.cpp(500): q.has_expbuf(node) > Our OUTPUT and CAPTURE queue support both VB2_DMABUF and VB2_MMAP, user > space can select which to use in runtime. > So our driver default support v4l2_m2m_ioctl_expbuf functionality. > In v4l2-compliance test, it will check v4l2_m2m_ioctl_expbuf only valid > when node->valid_memorytype is V4L2_MEMORY_MMAP. > So when go through node->valid_memorytype is V4L2_MEMORY_DMABUF, it > fail. valid_memorytype should have both MMAP and DMABUF flags. But v4l-utils-1.6.0 is way too old to be certain it isn't some v4l2-compliance bug that has since been fixed. Regards, Hans > > > best regards, > Tiffany > > > >>> test VIDIOC_EXPBUF: FAIL >>> >>> >>> Total: 38, Succeeded: 33, Failed: 5, Warnings: 0 >> >> If it is due to a bug in v4l2-compliance, then let me know and I'll fix it. If not, >> then it should be fixed in the driver. >> >> Frankly, it was the presence of these failures that made me think this patch series >> wasn't final. Before a v4l2 driver can be accepted in the kernel, v4l2-compliance must pass. >> >> Regards, >> >> Hans > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-media" in > the body of a message to majordomo@...r.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
Powered by blists - more mailing lists