| 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
| ||
|
Message-ID: <1468209529.3725.25.camel@mtksdaap41> Date: Mon, 11 Jul 2016 11:58:49 +0800 From: tiffany lin <tiffany.lin@...iatek.com> To: Hans Verkuil <hverkuil@...all.nl> 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 Hi Hans, On Fri, 2016-07-08 at 13:44 +0200, Hans Verkuil wrote: > 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. > Got it. We will return 0 as the value instead EAGAIN. > > > >>> 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. > Got it. I will rerun using latest v4l2-compliance test check this. > >>> 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. > Got it. I will rerun using master branch to check this. best regards, Tiffany > 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