From: hverkuil@xs4all.nl (Hans Verkuil)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v3 0/9] Add MT8173 Video Decoder Driver
Date: Fri, 8 Jul 2016 13:44:41 +0200 [thread overview]
Message-ID: <e2952cc2-3abd-894e-9481-b91a45cf7891@xs4all.nl> (raw)
In-Reply-To: <1467886612.21382.18.camel@mtksdaap41>
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 at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
next prev parent reply other threads:[~2016-07-08 11:44 UTC|newest]
Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-05-30 12:29 [PATCH v3 0/9] Add MT8173 Video Decoder Driver Tiffany Lin
2016-05-30 12:29 ` [PATCH v3 1/9] VPU: mediatek: Add decode support Tiffany Lin
2016-05-30 12:29 ` [PATCH v3 2/9] v4l: add Mediatek compressed video block format Tiffany Lin
2016-05-30 12:29 ` [PATCH v3 3/9] DocBook/v4l: Add compressed video formats used on MT8173 codec driver Tiffany Lin
2016-05-30 12:29 ` [PATCH v3 4/9] dt-bindings: Add a binding for Mediatek Video Decoder Tiffany Lin
2016-05-30 12:29 ` [PATCH v3 5/9] vcodec: mediatek: Add Mediatek V4L2 Video Decoder Driver Tiffany Lin
2016-05-30 12:29 ` [PATCH v3 6/9] vcodec: mediatek: Add Mediatek H264 " Tiffany Lin
2016-05-30 12:29 ` [PATCH v3 7/9] vcodec: mediatek: Add Mediatek VP8 " Tiffany Lin
2016-05-30 12:29 ` [PATCH v3 8/9] vcodec: mediatek: Add Mediatek VP9 " Tiffany Lin
2016-05-30 12:29 ` [PATCH v3 9/9] arm64: dts: mediatek: Add Video Decoder for MT8173 Tiffany Lin
2016-07-08 11:06 ` [PATCH v3 5/9] vcodec: mediatek: Add Mediatek V4L2 Video Decoder Driver Hans Verkuil
2016-08-01 9:38 ` Tiffany Lin
2016-08-01 10:39 ` Hans Verkuil
2016-08-01 11:28 ` Tiffany Lin
2016-07-08 10:23 ` [PATCH v3 3/9] DocBook/v4l: Add compressed video formats used on MT8173 codec driver Hans Verkuil
2016-07-11 2:56 ` tiffany lin
2016-07-12 8:16 ` Wu-Cheng Li (李務誠)
2016-07-12 19:08 ` Nicolas Dufresne
2016-07-12 19:14 ` Nicolas Dufresne
2016-07-12 19:31 ` Ian Arkver
2016-07-13 2:00 ` tiffany lin
2016-07-13 13:55 ` Nicolas Dufresne
2016-07-14 7:30 ` tiffany lin
2016-07-13 2:22 ` Wu-Cheng Li (李務誠)
2016-07-13 1:50 ` tiffany lin
2016-07-13 2:03 ` tiffany lin
2016-06-07 14:22 ` [PATCH v3 0/9] Add MT8173 Video Decoder Driver Mauro Carvalho Chehab
2016-06-07 22:13 ` Hans Verkuil
2016-06-14 8:44 ` Wu-Cheng Li (李務誠)
2016-06-14 11:08 ` tiffany lin
2016-06-16 10:54 ` Mauro Carvalho Chehab
2016-07-01 10:11 ` Hans Verkuil
2016-07-01 10:47 ` Wu-Cheng Li (李務誠)
2016-07-01 11:53 ` andrew-ct chen
2016-07-01 12:00 ` Hans Verkuil
2016-07-04 11:09 ` andrew-ct chen
2016-07-06 13:19 ` Hans Verkuil
2016-07-07 10:16 ` tiffany lin
2016-07-07 11:13 ` Hans Verkuil
2016-07-08 11:44 ` Hans Verkuil [this message]
2016-07-11 3:58 ` tiffany lin
2016-07-21 9:12 ` tiffany lin
2016-08-15 8:30 ` Hans Verkuil
2016-08-15 9:03 ` Tiffany Lin
2016-08-15 9:07 ` Hans Verkuil
2016-08-15 9:10 ` Tiffany Lin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e2952cc2-3abd-894e-9481-b91a45cf7891@xs4all.nl \
--to=hverkuil@xs4all.nl \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).