Re: [PATCH v4 0/3] Add ZynqMP VCU/Allegro DVT H.264 encoder driver

[Michael Tretter <m.tretter@pengutronix.de>]

On Fri, 01 Mar 2019 16:27:15 +0100, Michael Tretter wrote:
> This is v4 of the series to add support for the Allegro DVT H.264 encoder
> found in the EV family of the Xilinx ZynqMP platform.

Ping.

Michael

> 
> The most prominent change is the added documentation in the nal_h264.h header.
> The structs for the SPS and PPS NAL units and the function prototypes to
> convert between RBSP and the C structs are now accompanied by kernel-doc.
> 
> Furthermore, I went through all TODOs and FIXMEs in the driver. This resulted
> in a cleaner handling of messages that are exchanged with the encoder
> firmware, better documentation of the limits that are imposed by the encoder
> firmware on the driver, fixed handling of failures during channel creation,
> and support for 4k video.
> 
> v4l2-compliance also seems to be happy with the new version:
> 
> v4l2-compliance SHA: 410942e345b889d09456f5f862ee6cd415d8ae59, 64 bits
> 
> Compliance test for allegro device /dev/video4:
> 
> Driver Info:
>         Driver name      : allegro
>         Card type        : Allegro DVT Video Encoder
>         Bus info         : platform:a0009000.video-codec
>         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
>         Detected Stateful Encoder
> 
> 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)
>         test blocking wait: OK
>         Video Capture: Captured 10 buffers                
>         test MMAP (no poll): OK
>         Video Capture: Captured 10 buffers                
>         test MMAP (select): OK
>         Video Capture: Captured 10 buffers                
>         test MMAP (epoll): OK
>         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: 51, Succeeded: 51, Failed: 0, Warnings: 0
> 
> Apart from that, there are a few cleanups to resolve checkpatch or compiler
> warnings. A more detailed changelog is attached to each patch.
> 
> Michael
> 
> v3 -> v4:
> - fix checkpatch and compiler warnings
> - use v4l2_m2m_buf_copy_metadata to copy buffer metadata
> - resolve FIXME regarding channel creation and streamon
> - resolve various TODOs
> - add mailbox format to firmware info
> - add suballocator_size to firmware info
> - use struct_size to allocate mcu_msg_push_buffers_internal
> - handle *_response messages in a union
> - cleanup mcu_send_msg functions
> - increase maximum video resolution to 4k
> - handle errors when creating a channel
> - do not update ctrls after channel is created
> - add documentation for nal_h264.h
> 
> v2 -> v3:
> - add clocks to devicetree bindings
> - fix devicetree binding according to review comments on v2
> - add missing v4l2 callbacks
> - drop unnecessary v4l2 callbacks
> - drop debug module parameter poison_capture_buffers
> - check firmware size before loading firmware
> - rework error handling
> 
> v1 -> v2:
> - 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     |   43 +
>  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  | 2835 +++++++++++++++++
>  drivers/staging/media/allegro-dvt/nal-h264.c  | 1278 ++++++++
>  drivers/staging/media/allegro-dvt/nal-h264.h  |  330 ++
>  9 files changed, 4517 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
>