Message-Id: <B9A578FB-BF95-47E1-B202-B59897C2C898@gmail.com>
Date: Mon, 27 Oct 2025 16:57:45 +0400
From: Christian Hewitt <christianshewitt@...il.com>
To: zhentao.guo@...ogic.com
Cc: Mauro Carvalho Chehab <mchehab@...nel.org>,
 Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>,
 Neil Armstrong <neil.armstrong@...aro.org>,
 Kevin Hilman <khilman@...libre.com>,
 Jerome Brunet <jbrunet@...libre.com>,
 Martin Blumenstingl <martin.blumenstingl@...glemail.com>,
 linux-media@...r.kernel.org,
 devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org,
 linux-amlogic@...ts.infradead.org
Subject: Re: [PATCH 0/3] Add Amlogic stateless H.264 video decoder for S4

> On 27 Oct 2025, at 9:42 am, Zhentao Guo via B4 Relay <devnull+zhentao.guo.amlogic.com@...nel.org> wrote:
> 
> Introduce initial driver support for Amlogic's new video acceleration
> hardware architecture, designed for video stream decoding.
> 
> Compared to the current Amlogic video decoder hardware architecture,
> this new implementation eliminates the Esparser hardware component,
> enabling direct vb2 buffer input. The driver is designed to support
> the V4L2 M2M stateless decoder API. The initial phase includes support
> for H.264 decoding on Amlogic S805X2 platform.

It would be nice if you can outline the additional hardware the driver
plans to support so reviewers have perspective on where we are now, and
what more be coming? - the community would also like to understand if
this driver will supersede the stalled driver attempt in staging.

NB: I can only find two S805X2 devices to purchase for testing. The
first is a Mecool KD5 HDMI stick, but this has Widevine L1 certs so
will be unusable due to verified boot. The second is a no-name TV box
on Aliexpress where the advert claims S805X2 inside but also shows a
prominent Rockchip RK3228 logo :) .. please advise on some accessible
hardware that can be used for validating the driver.

> The driver is capable of:
> - Supporting stateless H.264 decoding up to a resolution 1920x1088(on the S805X2 platform).

It would be good to provide guidance to reviewers about other codecs
the driver plans to support and how (architecturally) the driver will
be expanded. I’m aware due to off-list discussions, but others are
not and will want to know.

> - Supporting I/P/B frame handling.
> - Supporting vb2 mmap and dma-buf modes.
> - Supporting frame-based decode mode. (Note that some H.264 bitstreams require
>  DPB reordering to generate reference lists, the stateless decoder driver
>  cannot access reordered reference lists in this mode, requiring the driver
>  to perform reference list reordering itself)
> - Supporting NV12/NV21 output.
> - Supporting Annex B start codes.
> 
> This driver is tested with Gstreamer.
> Example:
> gst-launch-1.0 filesrc location=/tmp/video_640x360_mp4_hevc_450kbps_no_b.mp4 !
> parsebin ! v4l2slh264dec ! filesink location=/tmp/output.yuv

Is this upstream gstreamer? if yes which version? If no, where can
we obtain sources for testing. Has ffmpeg been tested? - the same
questions apply.

> The driver passes v4l2 compliance test, test reports :
> v4l2-compliance 1.30.1, 64 bits, 64-bit time_t
> 
> Compliance test for aml-vdec-drv device /dev/video0:
> 
> Driver Info:
>        Driver name      : aml-vdec-drv
>        Card type        : platform:aml-vdec-drv
>        Bus info         : platform:fe320000.video-codec
>        Driver version   : 6.16.0

It would be preferable to test the next iteration of patches on
the latest kernel release, e.g. 6.18-rc, or recent media tree or
perhaps drm-tip source.

>        Capabilities     : 0x84204000
>                Video Memory-to-Memory Multiplanar
>                Streaming
>                Extended Pix Format
>                Device Capabilities
>        Device Caps      : 0x04204000
>                Video Memory-to-Memory Multiplanar
>                Streaming
>                Extended Pix Format
>        Detected Stateless Decoder
> Media Driver Info:
>        Driver name      : aml-vdec-drv
>        Model            : aml-vdec-drv
>        Serial           :
>        Bus info         : platform:fe320000.video-codec
>        Media version    : 6.16.0
>        Hardware revision: 0x00000000 (0)
>        Driver version   : 6.16.0
> Interface Info:
>        ID               : 0x0300000c
>        Type             : V4L Video
> Entity Info:
>        ID               : 0x00000001 (1)
>        Name             : aml_dev_drv-source
>        Function         : V4L2 I/O
>        Pad 0x01000002   : 0: Source
>          Link 0x02000008: to remote pad 0x1000004 of entity 'aml_dev_drv-proc' (Video Decoder): Data, Enabled, Immutable
> 
> Required ioctls:
>        test MC information (see 'Media Driver Info' above): OK
>        test VIDIOC_QUERYCAP: OK
>        test invalid ioctls: OK
> 
> Allow for multiple opens:
>        test second /dev/video0 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 (Not Supported)
>        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: 6 Private Controls: 0
>        Standard Compound Controls: 4 Private Compound 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 (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
> 
> Buffer ioctls:
>        test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
>        test CREATE_BUFS maximum buffers: OK
>        test VIDIOC_REMOVE_BUFS: OK
>        test VIDIOC_EXPBUF: OK
>        test Requests: OK
>        test blocking wait: OK
> 
> Total for aml-vdec-drv device /dev/video0: 49, Succeeded: 49, Failed: 0, Warnings: 0
> 
> Some Fluster test cases are still failing. We will publish the final results
> once all these Fluster test failures have been resolved.

IMHO good drivers that merge quickly have more merit than perfect
drivers that take forever. No driver passes all fluster tests. If
you share the results, people can advise whether the failures are
normal, something that must be improved, or something that can be
improved with time.

> Signed-off-by: Zhentao Guo <zhentao.guo@...ogic.com>
> ---
> Zhentao Guo (3):
>      dt-bindings: vdec: Add binding document of Amlogic decoder accelerator
>      dts: decoder: Support V4L2 stateless decoder dt node for S4
>      decoder: Add V4L2 stateless H.264 decoder driver

None of the patch subject lines are following the naming formats
used for the subsystems. Patch order is also important. Bindings
go first, device-tree changes to expose support normally go last.
Expect maintainer complaints about not reading kernel etiquette
rules before submitting. 

> .../bindings/media/amlogic,vcodec-dec.yaml         |   96 ++
> MAINTAINERS                                        |    7 +
> arch/arm64/boot/dts/amlogic/meson-s4.dtsi          |   24 +
> drivers/media/platform/amlogic/Kconfig             |    2 +
> drivers/media/platform/amlogic/Makefile            |    1 +
> drivers/media/platform/amlogic/vdec/Kconfig        |   15 +
> drivers/media/platform/amlogic/vdec/Makefile       |    4 +
> drivers/media/platform/amlogic/vdec/aml_vdec.c     |  759 +++++++++
> drivers/media/platform/amlogic/vdec/aml_vdec.h     |   31 +
> .../platform/amlogic/vdec/aml_vdec_canvas_utils.c  |  154 ++
> .../platform/amlogic/vdec/aml_vdec_canvas_utils.h  |   22 +
> drivers/media/platform/amlogic/vdec/aml_vdec_drv.c |  263 +++
> drivers/media/platform/amlogic/vdec/aml_vdec_drv.h |  194 +++
> drivers/media/platform/amlogic/vdec/aml_vdec_hw.c  |  652 +++++++
> drivers/media/platform/amlogic/vdec/aml_vdec_hw.h  |  182 ++
> .../platform/amlogic/vdec/aml_vdec_platform.c      |   37 +
> .../platform/amlogic/vdec/aml_vdec_platform.h      |   62 +
> drivers/media/platform/amlogic/vdec/h264.c         | 1790 ++++++++++++++++++++
> drivers/media/platform/amlogic/vdec/h264.h         |  300 ++++
> drivers/media/platform/amlogic/vdec/reg_defines.h  |  175 ++
> 20 files changed, 4770 insertions(+)

The driver requires closed-source firmware blobs (sadly) for each
codec and hardware target. Will that change in future iterations?
If not, the firmware blobs are not provided here so it would be
useful if you could provide a public source for them. There are
known FOSS tools for extracting files from the Android ucode blob
but testing presumably needs a specific blob version?

Most maintainers also prefer a complex driver to be submitted in a
series of patches that break the driver up into logical and easier
to-review pieces. Large uber-patches are harder to digest and are
likely to take much longer to review, and more effort for everyone
to track feedback on over time.

*** 

NB: The fact this driver is being submitted, is still a positive
and welcomed step towards better upstream Amlogic support and it’s
great to see! I would encourage you and colleagues to establish a
public working space to share code and tools so the community can
directly engage and support driver testing and development.

Christian

Powered by blists - more mailing lists