| Message ID | 20190118133716.29288-1-m.tretter@pengutronix.de (mailing list archive) |
|---|---|
| Headers | show |
| Series | Add ZynqMP VCU/Allegro DVT H.264 encoder driver | expand |
Hi Michael, On 1/18/19 2:37 PM, Michael Tretter wrote: > This is v2 of the series to add support for the Allegro DVT H.264 encoder > found in the EV family of the Xilinx ZynqMP platform. > > See v1 [0] of the patch series for a description of the hardware. > > I fixed the handling of frames with various sizes and driver is now able to > encode H.264 video in the baseline profile up to 1920x1080 pixels. I also > addressed the issues reported by the kbuild robot for the previous series, > implemented a few extended controls and changed the interface to the mcu to > follow the register documentation rather than the downstream driver > implementation. > > I would especially appreciate feedback to the device tree bindings and the > overall architecture of the driver. I'll try to review this next week. Ping me if you didn't see a review by the end of next week. BTW, can you post the output of 'v4l2-compliance -s'? (make sure you use the very latest version of v4l2-compliance!) Regards, Hans > > The driver still only works with the vcu-firmware release 2018.2. I am not yet > sure how to address the different firmware versions, because in addition to > the mailbox sizes, there are also changes within the messages themselves. > > I also did not address the integration with the xlnx-vcu driver, yet. > > Michael > > [0] https://lore.kernel.org/linux-media/20190109113037.28430-1-m.tretter@pengutronix.de/ > > Changes since v1: > - clean up debug log levels > - fix unused variable in allegro_mbox_init > - fix uninitialized variable in allegro_mbox_write > - fix global module parameters > - fix Kconfig dependencies > - return h264 as default codec for mcu > - implement device reset as documented > - document why irq does not wait for clear > - rename ENCODE_ONE_FRM to ENCODE_FRAME > - allow error codes for mcu_channel_id > - move control handler to channel > - add fw version check > - add support for colorspaces > - enable configuration of H.264 levels > - enable configuration of frame size > - enable configuration of bit rate and CPB size > - enable configuration of GOP size > - rework response handling > - fix missing error handling in allegro_h264_write_sps > > Michael Tretter (3): > media: dt-bindings: media: document allegro-dvt bindings > [media] allegro: add Allegro DVT video IP core driver > [media] allegro: add SPS/PPS nal unit writer > > .../devicetree/bindings/media/allegro.txt | 35 + > MAINTAINERS | 6 + > drivers/staging/media/Kconfig | 2 + > drivers/staging/media/Makefile | 1 + > drivers/staging/media/allegro-dvt/Kconfig | 16 + > drivers/staging/media/allegro-dvt/Makefile | 6 + > .../staging/media/allegro-dvt/allegro-core.c | 2828 +++++++++++++++++ > drivers/staging/media/allegro-dvt/nal-h264.c | 1278 ++++++++ > drivers/staging/media/allegro-dvt/nal-h264.h | 188 ++ > 9 files changed, 4360 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/allegro.txt > create mode 100644 drivers/staging/media/allegro-dvt/Kconfig > create mode 100644 drivers/staging/media/allegro-dvt/Makefile > create mode 100644 drivers/staging/media/allegro-dvt/allegro-core.c > create mode 100644 drivers/staging/media/allegro-dvt/nal-h264.c > create mode 100644 drivers/staging/media/allegro-dvt/nal-h264.h >
Hi Hans, On Fri, 18 Jan 2019 15:11:32 +0100, Hans Verkuil wrote: > Hi Michael, > > On 1/18/19 2:37 PM, Michael Tretter wrote: > > This is v2 of the series to add support for the Allegro DVT H.264 encoder > > found in the EV family of the Xilinx ZynqMP platform. > > > > See v1 [0] of the patch series for a description of the hardware. > > > > I fixed the handling of frames with various sizes and driver is now able to > > encode H.264 video in the baseline profile up to 1920x1080 pixels. I also > > addressed the issues reported by the kbuild robot for the previous series, > > implemented a few extended controls and changed the interface to the mcu to > > follow the register documentation rather than the downstream driver > > implementation. > > > > I would especially appreciate feedback to the device tree bindings and the > > overall architecture of the driver. > > I'll try to review this next week. Ping me if you didn't see a review by the > end of next week. > > BTW, can you post the output of 'v4l2-compliance -s'? (make sure you use the > very latest version of v4l2-compliance!) Thanks, here is the output of v4l2-compliance. I haven't used v4l2-compliance with the -s switch yet and will look into the reported failures for the streaming ioctls. v4l2-compliance SHA: e07d1b90190f2b98fe4f5be20406b49ecbe5b3e7, 64 bits Compliance test for allegro device /dev/video4: Driver Info: Driver name : allegro Card type : Allegro DVT Video Encoder Bus info : platform:a0009000.al5e Driver version : 5.0.0 Capabilities : 0x84208000 Video Memory-to-Memory Streaming Extended Pix Format Device Capabilities Device Caps : 0x04208000 Video Memory-to-Memory Streaming Extended Pix Format Required ioctls: test VIDIOC_QUERYCAP: OK Allow for multiple opens: test second /dev/video4 open: OK test VIDIOC_QUERYCAP: OK test VIDIOC_G/S_PRIORITY: OK test for unlimited opens: OK Debug ioctls: test VIDIOC_DBG_G/S_REGISTER: OK test VIDIOC_LOG_STATUS: OK (Not Supported) Input ioctls: test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported) test VIDIOC_ENUMAUDIO: OK (Not Supported) test VIDIOC_G/S/ENUMINPUT: OK (Not Supported) test VIDIOC_G/S_AUDIO: OK (Not Supported) Inputs: 0 Audio Inputs: 0 Tuners: 0 Output ioctls: test VIDIOC_G/S_MODULATOR: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_ENUMAUDOUT: OK (Not Supported) test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported) test VIDIOC_G/S_AUDOUT: OK (Not Supported) Outputs: 0 Audio Outputs: 0 Modulators: 0 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_QUERY_EXT_CTRL/QUERYMENU: OK test VIDIOC_QUERYCTRL: OK test VIDIOC_G/S_CTRL: OK test VIDIOC_G/S/TRY_EXT_CTRLS: OK test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK test VIDIOC_G/S_JPEGCOMP: OK (Not Supported) Standard Controls: 8 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) test VIDIOC_G_FMT: OK test VIDIOC_TRY_FMT: OK test VIDIOC_S_FMT: OK test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported) test Cropping: OK (Not Supported) test Composing: OK (Not Supported) test Scaling: OK Codec ioctls: test VIDIOC_(TRY_)ENCODER_CMD: OK 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 test VIDIOC_EXPBUF: OK test Requests: OK (Not Supported) Test input 0: Streaming ioctls: test read/write: OK (Not Supported) fail: v4l2-test-buffers.cpp(1920): pid != pid_streamoff fail: v4l2-test-buffers.cpp(1953): testBlockingDQBuf(node, q) test blocking wait: FAIL fail: v4l2-test-buffers.cpp(253): g_field() == V4L2_FIELD_ANY fail: v4l2-test-buffers.cpp(1157): buf.qbuf(node) test MMAP (no poll): FAIL fail: v4l2-test-buffers.cpp(253): g_field() == V4L2_FIELD_ANY fail: v4l2-test-buffers.cpp(1157): buf.qbuf(node) test MMAP (select): FAIL test USERPTR (no poll): OK (Not Supported) test USERPTR (select): OK (Not Supported) test DMABUF: Cannot test, specify --expbuf-device Total for allegro device /dev/video4: 50, Succeeded: 47, Failed: 3, Warnings: 0 Michael > > Regards, > > Hans > > > > > The driver still only works with the vcu-firmware release 2018.2. I am not yet > > sure how to address the different firmware versions, because in addition to > > the mailbox sizes, there are also changes within the messages themselves. > > > > I also did not address the integration with the xlnx-vcu driver, yet. > > > > Michael > > > > [0] https://lore.kernel.org/linux-media/20190109113037.28430-1-m.tretter@pengutronix.de/ > > > > Changes since v1: > > - clean up debug log levels > > - fix unused variable in allegro_mbox_init > > - fix uninitialized variable in allegro_mbox_write > > - fix global module parameters > > - fix Kconfig dependencies > > - return h264 as default codec for mcu > > - implement device reset as documented > > - document why irq does not wait for clear > > - rename ENCODE_ONE_FRM to ENCODE_FRAME > > - allow error codes for mcu_channel_id > > - move control handler to channel > > - add fw version check > > - add support for colorspaces > > - enable configuration of H.264 levels > > - enable configuration of frame size > > - enable configuration of bit rate and CPB size > > - enable configuration of GOP size > > - rework response handling > > - fix missing error handling in allegro_h264_write_sps > > > > Michael Tretter (3): > > media: dt-bindings: media: document allegro-dvt bindings > > [media] allegro: add Allegro DVT video IP core driver > > [media] allegro: add SPS/PPS nal unit writer > > > > .../devicetree/bindings/media/allegro.txt | 35 + > > MAINTAINERS | 6 + > > drivers/staging/media/Kconfig | 2 + > > drivers/staging/media/Makefile | 1 + > > drivers/staging/media/allegro-dvt/Kconfig | 16 + > > drivers/staging/media/allegro-dvt/Makefile | 6 + > > .../staging/media/allegro-dvt/allegro-core.c | 2828 +++++++++++++++++ > > drivers/staging/media/allegro-dvt/nal-h264.c | 1278 ++++++++ > > drivers/staging/media/allegro-dvt/nal-h264.h | 188 ++ > > 9 files changed, 4360 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/media/allegro.txt > > create mode 100644 drivers/staging/media/allegro-dvt/Kconfig > > create mode 100644 drivers/staging/media/allegro-dvt/Makefile > > create mode 100644 drivers/staging/media/allegro-dvt/allegro-core.c > > create mode 100644 drivers/staging/media/allegro-dvt/nal-h264.c > > create mode 100644 drivers/staging/media/allegro-dvt/nal-h264.h > > > >
On 01/21/19 11:42, Michael Tretter wrote: > Hi Hans, > > On Fri, 18 Jan 2019 15:11:32 +0100, Hans Verkuil wrote: >> Hi Michael, >> >> On 1/18/19 2:37 PM, Michael Tretter wrote: >>> This is v2 of the series to add support for the Allegro DVT H.264 encoder >>> found in the EV family of the Xilinx ZynqMP platform. >>> >>> See v1 [0] of the patch series for a description of the hardware. >>> >>> I fixed the handling of frames with various sizes and driver is now able to >>> encode H.264 video in the baseline profile up to 1920x1080 pixels. I also >>> addressed the issues reported by the kbuild robot for the previous series, >>> implemented a few extended controls and changed the interface to the mcu to >>> follow the register documentation rather than the downstream driver >>> implementation. >>> >>> I would especially appreciate feedback to the device tree bindings and the >>> overall architecture of the driver. >> >> I'll try to review this next week. Ping me if you didn't see a review by the >> end of next week. >> >> BTW, can you post the output of 'v4l2-compliance -s'? (make sure you use the >> very latest version of v4l2-compliance!) > > Thanks, here is the output of v4l2-compliance. I haven't used > v4l2-compliance with the -s switch yet and will look into the reported > failures for the streaming ioctls. > > v4l2-compliance SHA: e07d1b90190f2b98fe4f5be20406b49ecbe5b3e7, 64 bits > > Compliance test for allegro device /dev/video4: > > Driver Info: > Driver name : allegro > Card type : Allegro DVT Video Encoder > Bus info : platform:a0009000.al5e > Driver version : 5.0.0 > Capabilities : 0x84208000 > Video Memory-to-Memory > Streaming > Extended Pix Format > Device Capabilities > Device Caps : 0x04208000 > Video Memory-to-Memory > Streaming > Extended Pix Format > > Required ioctls: > test VIDIOC_QUERYCAP: OK > > Allow for multiple opens: > test second /dev/video4 open: OK > test VIDIOC_QUERYCAP: OK > test VIDIOC_G/S_PRIORITY: OK > test for unlimited opens: OK > > Debug ioctls: > test VIDIOC_DBG_G/S_REGISTER: OK > test VIDIOC_LOG_STATUS: OK (Not Supported) > > Input ioctls: > test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported) > test VIDIOC_G/S_FREQUENCY: OK (Not Supported) > test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported) > test VIDIOC_ENUMAUDIO: OK (Not Supported) > test VIDIOC_G/S/ENUMINPUT: OK (Not Supported) > test VIDIOC_G/S_AUDIO: OK (Not Supported) > Inputs: 0 Audio Inputs: 0 Tuners: 0 > > Output ioctls: > test VIDIOC_G/S_MODULATOR: OK (Not Supported) > test VIDIOC_G/S_FREQUENCY: OK (Not Supported) > test VIDIOC_ENUMAUDOUT: OK (Not Supported) > test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported) > test VIDIOC_G/S_AUDOUT: OK (Not Supported) > Outputs: 0 Audio Outputs: 0 Modulators: 0 > > 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_QUERY_EXT_CTRL/QUERYMENU: OK > test VIDIOC_QUERYCTRL: OK > test VIDIOC_G/S_CTRL: OK > test VIDIOC_G/S/TRY_EXT_CTRLS: OK > test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK > test VIDIOC_G/S_JPEGCOMP: OK (Not Supported) > Standard Controls: 8 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) > test VIDIOC_G_FMT: OK > test VIDIOC_TRY_FMT: OK > test VIDIOC_S_FMT: OK > test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported) > test Cropping: OK (Not Supported) > test Composing: OK (Not Supported) > test Scaling: OK > > Codec ioctls: > test VIDIOC_(TRY_)ENCODER_CMD: OK > 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 > test VIDIOC_EXPBUF: OK > test Requests: OK (Not Supported) > > Test input 0: > > Streaming ioctls: > test read/write: OK (Not Supported) > fail: v4l2-test-buffers.cpp(1920): pid != pid_streamoff > fail: v4l2-test-buffers.cpp(1953): testBlockingDQBuf(node, q) > test blocking wait: FAIL > fail: v4l2-test-buffers.cpp(253): g_field() == V4L2_FIELD_ANY > fail: v4l2-test-buffers.cpp(1157): buf.qbuf(node) You are missing a buf_prepare callback that checks the field value for the output buffer and replaces it with FIELD_NONE if it is FIELD_ANY. See e.g. vim2m_buf_prepare in drivers/media/platform/vim2m.c. Regards, Hans > test MMAP (no poll): FAIL > fail: v4l2-test-buffers.cpp(253): g_field() == V4L2_FIELD_ANY > fail: v4l2-test-buffers.cpp(1157): buf.qbuf(node) > test MMAP (select): FAIL > test USERPTR (no poll): OK (Not Supported) > test USERPTR (select): OK (Not Supported) > test DMABUF: Cannot test, specify --expbuf-device > > Total for allegro device /dev/video4: 50, Succeeded: 47, Failed: 3, Warnings: 0 > > Michael > >> >> Regards, >> >> Hans >> >>> >>> The driver still only works with the vcu-firmware release 2018.2. I am not yet >>> sure how to address the different firmware versions, because in addition to >>> the mailbox sizes, there are also changes within the messages themselves. >>> >>> I also did not address the integration with the xlnx-vcu driver, yet. >>> >>> Michael >>> >>> [0] https://lore.kernel.org/linux-media/20190109113037.28430-1-m.tretter@pengutronix.de/ >>> >>> Changes since v1: >>> - clean up debug log levels >>> - fix unused variable in allegro_mbox_init >>> - fix uninitialized variable in allegro_mbox_write >>> - fix global module parameters >>> - fix Kconfig dependencies >>> - return h264 as default codec for mcu >>> - implement device reset as documented >>> - document why irq does not wait for clear >>> - rename ENCODE_ONE_FRM to ENCODE_FRAME >>> - allow error codes for mcu_channel_id >>> - move control handler to channel >>> - add fw version check >>> - add support for colorspaces >>> - enable configuration of H.264 levels >>> - enable configuration of frame size >>> - enable configuration of bit rate and CPB size >>> - enable configuration of GOP size >>> - rework response handling >>> - fix missing error handling in allegro_h264_write_sps >>> >>> Michael Tretter (3): >>> media: dt-bindings: media: document allegro-dvt bindings >>> [media] allegro: add Allegro DVT video IP core driver >>> [media] allegro: add SPS/PPS nal unit writer >>> >>> .../devicetree/bindings/media/allegro.txt | 35 + >>> MAINTAINERS | 6 + >>> drivers/staging/media/Kconfig | 2 + >>> drivers/staging/media/Makefile | 1 + >>> drivers/staging/media/allegro-dvt/Kconfig | 16 + >>> drivers/staging/media/allegro-dvt/Makefile | 6 + >>> .../staging/media/allegro-dvt/allegro-core.c | 2828 +++++++++++++++++ >>> drivers/staging/media/allegro-dvt/nal-h264.c | 1278 ++++++++ >>> drivers/staging/media/allegro-dvt/nal-h264.h | 188 ++ >>> 9 files changed, 4360 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/media/allegro.txt >>> create mode 100644 drivers/staging/media/allegro-dvt/Kconfig >>> create mode 100644 drivers/staging/media/allegro-dvt/Makefile >>> create mode 100644 drivers/staging/media/allegro-dvt/allegro-core.c >>> create mode 100644 drivers/staging/media/allegro-dvt/nal-h264.c >>> create mode 100644 drivers/staging/media/allegro-dvt/nal-h264.h >>> >> >>