[PATCH v12 0/6] Add Amlogic stateless H.264 video decoder for S4

[PATCH v12 0/6] Add Amlogic stateless H.264 video decoder for S4

[Zhentao Guo via B4 Relay]

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.

The driver needs to work alongside with a signed firmware. The loading process of
the signed fw is as follow.
Stage1: Decypt and decompose the full firmware package when the driver is probed.
  +---------------------+                        +---------------------+
  |  Decoder Driver     |                        |  TEE Shared Memory  |
  |  (Kernel Space)     |                        |                     |
  |  +---------------+  |                        |  +---------------+  |
  |  | video_ucode   |  |                        |  | firmware      |  |
  |  |    .bin       |  |  Copy payload to SHM   |  |   payload     |  |
  |  | (from fs)     |  | ---------------------> |  | (Secure RAM)  |  |
  |  +---------------+  |                        |  +---------------+  |
  +---------------------+                        +----------+----------+
                                                            |
                                                            | PTA Invocation
                                                            v
                                       +-------------------------------+
                                       |             BL32              |
                                       |  +-------------------------+  |
                                       |  | Decrypt Firmware        |  |
                                       |  +-----------+-------------+  |
                                       |              |                |
                                       |              v                |
                                       |  +-------------------------+  |
                                       |  | Decompose the full      |  |
				       |  | firmware pacakge        |  |
                                       |  +-----------+-------------+  |
                                       |              |                |
                                       |              v                |
                                       |  +-------------------------+  |
                                       |  | Store decomposed .bin   |  |
                                       |  | in Secure Memory        |  |
                                       |  +-------------------------+  |
                                       +-------------------------------+

Stage2: When a decode job is scheduled, load decrypted fw via secure monitor.

  +---------------------+
  |  V4L2 M2M Framework |
  |  +---------------+  |
  |  |  device_run   |  |
  |  +------+--------+  |
  +---------+-----------+
            |
            v
  +---------------------+                  +---------------------+
  |  Decoder Driver     |                  |  Secure Monitor     |
  |  (Kernel Space)     |                  |      (bl32)         |
  |  +---------------+  |     SMC Call     |  +---------------+  |
  |  | Select Codec  |  | ---------------> |  | Select & Load |  |
  |  | Specific FW   |  |                  |  | firmware.bin  |  |
  |  +---------------+  |                  |  | to AMRISC     |  |
  +---------------------+                  |  +-------+-------+  |
                                           +----------+----------+
                                                      |
                                                      v
                                           +---------------------+
                                           |     AMRISC Core     |
                                           |  +---------------+  |
                                           |  | Running fw on |  |
                                           |  | AMRISC        |  |
                                           |  +---------------+  |
                                           +---------------------+

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

Retry the compliance test based on kernel 7.1.0:
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 : 7.1.0
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 : 7.1.0
Hardware revision: 0x00000000 (0)
Driver version : 7.1.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
Fluster test result of JVT-AVC_V1.
Result:
Ran 77/135 tests successfully

- 52 test vectors failed due to interlaced or mbaff clips: The Amlogic stateless
  decoder driver only support bitstreams with frame_mbs_only_flags == 1.
  Test Vectors:
        cabac_mot_fld0_full
        cabac_mot_mbaff0_full
        cabac_mot_picaff0_full
        CABREF3_Sand_D
        CAFI1_SVA_C
        CAMA1_Sony_C
        CAMA1_TOSHIBA_B
        cama1_vtc_c
        cama2_vtc_b
        CAMA3_Sand_E
        cama3_vtc_b
        CAMACI3_Sony_C
        CAMANL1_TOSHIBA_B
        CAMANL2_TOSHIBA_B
        CAMANL3_Sand_E
        CAMASL3_Sony_B
        CAMP_MOT_MBAFF_L30
        CAMP_MOT_MBAFF_L31
        CANLMA2_Sony_C
        CANLMA3_Sony_C
        CAPA1_TOSHIBA_B
        CAPAMA3_Sand_F
        cavlc_mot_fld0_full_B
        cavlc_mot_mbaff0_full_B
        cavlc_mot_picaff0_full_B
        CVCANLMA2_Sony_C
        CVFI1_Sony_D
        CVFI1_SVA_C
        CVFI2_Sony_H
        CVFI2_SVA_C
        CVMA1_Sony_D
        CVMA1_TOSHIBA_B
        CVMANL1_TOSHIBA_B
        CVMANL2_TOSHIBA_B
        CVMAPAQP3_Sony_E
        CVMAQP2_Sony_G
        CVMAQP3_Sony_D
        CVMP_MOT_FLD_L30_B
        CVNLFI1_Sony_C
        CVNLFI2_Sony_H
        CVPA1_TOSHIBA_B
        FI1_Sony_E
        MR6_BT_B
        MR7_BT_B
        MR8_BT_B
        MR9_BT_B
        Sharp_MP_Field_1_B
        Sharp_MP_Field_2_B
        Sharp_MP_Field_3_B
        Sharp_MP_PAFF_1r2
        Sharp_MP_PAFF_2r
        CVMP_MOT_FRM_L31_B
- 3 test vectors failed due to unsupported bitstream.
  num_slice_group_minus1 greater than zero is not supported by the
  hardware.
  Test Vectors:
        FM1_BT_B
        FM1_FT_E
        FM2_SVA_C
- 2 test vectors failed because SP_SLICE type is not supported by the
  hardware.
  Test Vectors:
        SP1_BT_A
        sp2_bt_b

One remain failure is CVFC1_Sony_C, which contains crop information. The md5sum of every decoded YUV indicates that original output from the decoder was correct. The YUV was cropped by gstreamer. The correct cropping method for this bitstream should be to crop 30*2 rows of pixels from both the top and bottom of the image, and 13*2 columns of pixels from both the left and right sides.However, gstreamer cropped 13*4 columns of pixels from the right side and 30*4 rows of pixels from the bottom. We are trying to find out the cause of this. Other failuers mentioned in V1 and V2 were resolved.

Changes in v12:
- Add additional check to avoid the resolution happened in middle of a bitstream without streamon/off, due to the non-standard habbit of the userspace.
- Add a separate initialization claim, init_ctx, to protect the hardware initialization process.
- Add rejection to the formats whose type does not match the queue in all four s_fmt/try_fmt paths.
- Link to v11: https://lore.kernel.org/r/20261012-b4-s4-vdec-upstream-v11-0-25939f85ada5@amlogic.com

Changes in v11:
- Check clock/power-on failures before operating the registers. Use pm_runtime_resume_and_get() instead of pm_runtime_get_sync() so the usage counter stays balanced on the error path.
- Fix missing validation of untrusted bitstream input in the H.264 decoder.
- Fix instance/resource lifetime issues: do not re-init a live decoder instance when only output or capture queue restarts.
- Link to v10: https://lore.kernel.org/r/20260924-b4-s4-vdec-upstream-v10-0-1289303a58a4@amlogic.com

Changes in v10:
- Rename the clock/power/interrupt names in dt-binding and dts to match the name of decoder sub-core (dos, vdec, hvdec).
- Fix the h264 high profile bitstreams decoding error issue.
- Use the allocation bitmaps instead of poc != INVALID_POC as the slot occupancy test, since INVALID_POC (0xffffffff) aliases with the valid POC value -1.
- Link to v9: https://lore.kernel.org/r/20260914-b4-s4-vdec-upstream-v9-0-72fdb0d71b02@amlogic.com

Changes in v9:
- Rename the clock/power/interrupt names in dt-binding and dts. Add the descriptions to the clock items in dt-binding.
- Optimized the handling process for reference frame loss. The address of a lost reference frame will not be configured to the hardware, to prevent the hardware from accessing an illegal address.
- Optimized the error handling paths in the driver based on v8. 
- Link to v8: https://lore.kernel.org/r/20260831-b4-s4-vdec-upstream-v8-0-830195b9bd91@amlogic.com

Changes in v8:
- Rename the clock/power names in dt-binding and dts. Add the descriptions to the canvas and secure-monitor node in dt-binding.
- Improve code accroding to the feedback of sashiko bot, including:
    1.Imporve the resource cleanup and error handling paths in the driver.
    2.Mitigated race conditions between the decode path, interrupt handler, and resource teardown.
    3.Validate codec-stream parameters passed to the driver to prevent the potential risk.
- Link to v7: https://lore.kernel.org/r/20260812-b4-s4-vdec-upstream-v7-0-3fae085861bd@amlogic.com

Changes in v7:
- Addressed Krzysztof's review comments: fixed DTS coding style, added board info requiring the config to patch 6/6's commit message.
- Improvements made per sashiko bot review, including:
    1.Fixed memory out-of-bounds access / buffer overflow issues.
    2.Fixed concurrency and race conditions related issues.
    3.Improved the error handling paths to avoid the risk of resource leakage.
    4.Fixed the improper V4L2 framework usage.
- Link to v6: https://lore.kernel.org/r/20260724-b4-s4-vdec-upstream-v6-0-6072a7ee7428@amlogic.com

Changes in v6:
- Rename the HEVC core and the associated power, clock, and interrupt names. 'HEVC' was a development codename of the decoder IP which accelerates h265 vp9 av1 decoding, but it coincidentally matches the acronym for High Efficiency Video Coding. To avoid naming ambiguity, HEVC core is renamed to HVDEC core(H stands for HEVC).
- Get rid of the unused function and interface.
- Link to v5: https://lore.kernel.org/r/20260526-b4-s4-vdec-upstream-v5-0-33bc817f93f4@amlogic.com

Changes in v5:
- Rename the compatible and the clock item accroding to Krzysztof's feedback.
- Use tee & meson_sm helpers to decrypt load the signed decoder firmware. Add the meson_sm describsion and reference to dt-binding and dts.
- Link to v4: https://lore.kernel.org/r/20260213-b4-s4-vdec-upstream-v4-0-c7112d00d662@amlogic.com

Changes in v4:
- Use %pad to print dma_addr_t type instead of using %llx.
- Add initial values to some local variables.
- Link to v3: https://lore.kernel.org/r/20260121-b4-s4-vdec-upstream-v3-0-4496aec3d79e@amlogic.com

Changes in v3:
- Fixed the DT check error:
  arch/arm64/boot/dts/amlogic/meson-s4-s805x2-aq222.dtb: video-codec@fe320000 (amlogic,s4-vcodec-dec): 'amlogic,canvas' does not match any of the regexes: '^pinctrl-[0-9]+$' 
  from schema $id: http://devicetree.org/schemas/media/amlogic,vcodec-dec.yaml
- Added DOS reset lines to dtsi and dt-binding.
- Fixed the issue where some B-frames were not decoded correctly(The fluster failures mentioned in patch V1 and V2 were mostly caused by this).
- Fixed the issue where canvas_index leaks occurred during the decoding of some bitstreams.
- Rework the src/dst format storage. Use v4l2_pix_format_mplane to store formats that related to bitstreams into the context. Add the reset format function to reset all the formats to default value. 
- Store decoding parameters related to chip platforms, such as maximum width/height and alignment requirement, organized by chip platform.
- Link to v2: https://lore.kernel.org/r/20251124-b4-s4-vdec-upstream-v2-0-bdbbce3f11a6@amlogic.com

Changes in v2:
- Fixed incorrect generation of the reference lists for some B-frames.
- Rename or get rid of some properties in DTS and dt-binding.
- Remove some useless code or helper functions, (eg. clk helper functions, reg I/O macros, and some superfluous print messages) replace these functions with existing ones.
- Replace all the printk messages with dev_err/dev_info/dev_dbg
- Use the helper functions from the existing meson-canvas driver.
- Use clk_bulk_data to map clocks from DTS.
- Retry the V4L2 Compliance test on 6.18-rc6, fix a newly introduced bug.
- Link to v1: https://lore.kernel.org/r/20251027-b4-s4-vdec-upstream-v1-0-620401813b5d@amlogic.com

Signed-off-by: Zhentao Guo <zhentao.guo@amlogic.com>
---
Zhentao Guo (6):
      firmware: meson: sm: video firmware loading via secure monitor
      firmware: meson: sm: Add video firmware loading SMC call
      media: dt-bindings: Add Amlogic V4L2 video decoder
      decoder: Add V4L2 stateless H.264 decoder driver
      arm64: dts: amlogic: Add video decoder driver support for S4 SOCs
      arm64: defconfig: Enable CONFIG_VIDEO_AMLOGIC_VDEC

 .../devicetree/bindings/media/amlogic,s4-vdec.yaml |  114 +
 MAINTAINERS                                        |    7 +
 arch/arm64/boot/dts/amlogic/meson-s4.dtsi          |   33 +
 arch/arm64/configs/defconfig                       |    1 +
 drivers/firmware/meson/meson_sm.c                  |    1 +
 drivers/media/platform/amlogic/Kconfig             |    1 +
 drivers/media/platform/amlogic/Makefile            |    1 +
 drivers/media/platform/amlogic/vdec/Kconfig        |   18 +
 drivers/media/platform/amlogic/vdec/Makefile       |    4 +
 drivers/media/platform/amlogic/vdec/TODO           |    7 +
 drivers/media/platform/amlogic/vdec/aml_vdec.c     |  846 ++++++++
 drivers/media/platform/amlogic/vdec/aml_vdec.h     |   33 +
 drivers/media/platform/amlogic/vdec/aml_vdec_drv.c |  260 +++
 drivers/media/platform/amlogic/vdec/aml_vdec_drv.h |  168 ++
 drivers/media/platform/amlogic/vdec/aml_vdec_hw.c  |  570 +++++
 drivers/media/platform/amlogic/vdec/aml_vdec_hw.h  |  161 ++
 .../platform/amlogic/vdec/aml_vdec_platform.c      |   81 +
 .../platform/amlogic/vdec/aml_vdec_platform.h      |   46 +
 .../media/platform/amlogic/vdec/aml_vdec_tee_fw.c  |  249 +++
 .../media/platform/amlogic/vdec/aml_vdec_tee_fw.h  |   27 +
 drivers/media/platform/amlogic/vdec/h264.c         | 2209 ++++++++++++++++++++
 drivers/media/platform/amlogic/vdec/h264.h         |  299 +++
 drivers/media/platform/amlogic/vdec/reg_defines.h  |  177 ++
 include/linux/firmware/meson/meson_sm.h            |    1 +
 24 files changed, 5314 insertions(+)
---
base-commit: 2efb7a11f1aa4d2dc285fe2680cca22cb9f72f64
change-id: 20251027-b4-s4-vdec-upstream-0603c1a4c84a

Best regards,
-- 
Zhentao Guo <zhentao.guo@amlogic.com>

[PATCH v12 1/6] firmware: meson: sm: video firmware loading via secure monitor

[Zhentao Guo via B4 Relay]

From: Zhentao Guo <zhentao.guo@amlogic.com>

Add SM_LOAD_VIDEO_FW to the secure monitor command enum
to allow decoder drivers to load firmware through the meson_sm
interface.

Signed-off-by: Zhentao Guo <zhentao.guo@amlogic.com>
---
 include/linux/firmware/meson/meson_sm.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/firmware/meson/meson_sm.h b/include/linux/firmware/meson/meson_sm.h
index 3ebc2bd9a976..3b90b49d0552 100644
--- a/include/linux/firmware/meson/meson_sm.h
+++ b/include/linux/firmware/meson/meson_sm.h
@@ -15,6 +15,7 @@ enum {
 	SM_THERMAL_CALIB_READ,
 	SM_A1_PWRC_SET,
 	SM_A1_PWRC_GET,
+	SM_LOAD_VIDEO_FW,
 };
 
 struct meson_sm_firmware;

-- 
2.42.0

[PATCH v12 2/6] firmware: meson: sm: Add video firmware loading SMC call

[Zhentao Guo via B4 Relay]

From: Zhentao Guo <zhentao.guo@amlogic.com>

Add SM_LOAD_VIDEO_FW at SMC ID 0xb200000f in the command
table to load video firmware.

Signed-off-by: Zhentao Guo <zhentao.guo@amlogic.com>
---
 drivers/firmware/meson/meson_sm.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/firmware/meson/meson_sm.c b/drivers/firmware/meson/meson_sm.c
index ab9751a59b55..887caa5c625d 100644
--- a/drivers/firmware/meson/meson_sm.c
+++ b/drivers/firmware/meson/meson_sm.c
@@ -48,6 +48,7 @@ static const struct meson_sm_chip gxbb_chip = {
 		CMD(SM_THERMAL_CALIB_READ,	0x82000047),
 		CMD(SM_A1_PWRC_SET,		0x82000093),
 		CMD(SM_A1_PWRC_GET,		0x82000095),
+		CMD(SM_LOAD_VIDEO_FW,		0xb200000f),
 		{ /* sentinel */ },
 	},
 };

-- 
2.42.0

[PATCH v12 3/6] media: dt-bindings: Add Amlogic V4L2 video decoder

[Zhentao Guo via B4 Relay]

From: Zhentao Guo <zhentao.guo@amlogic.com>

Add bindings for Amlogic hardware decoder accelerator.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Signed-off-by: Zhentao Guo <zhentao.guo@amlogic.com>
---
 .../devicetree/bindings/media/amlogic,s4-vdec.yaml | 114 +++++++++++++++++++++
 1 file changed, 114 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/amlogic,s4-vdec.yaml b/Documentation/devicetree/bindings/media/amlogic,s4-vdec.yaml
new file mode 100644
index 000000000000..75b9ba3cb08d
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/amlogic,s4-vdec.yaml
@@ -0,0 +1,114 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2025 Amlogic, Inc. All rights reserved
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/amlogic,s4-vdec.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Amlogic Video Decode Accelerator
+
+maintainers:
+  - Zhentao Guo <zhentao.guo@amlogic.com>
+
+description:
+  The Video Decoder Accelerator present on Amlogic SOCs.
+  It supports stateless h264 decoding.
+
+properties:
+  compatible:
+    const: amlogic,s4-vdec
+
+  reg:
+    maxItems: 2
+
+  reg-names:
+    items:
+      - const: dos
+      - const: dmc
+
+  interrupts:
+    maxItems: 2
+
+  interrupt-names:
+    items:
+      - const: hvdec
+      - const: vdec
+
+  clocks:
+    items:
+      - description: Clock for dos core
+      - description: Clock for vdec core
+      - description: Clock for hvdec core
+
+  clock-names:
+    items:
+      - const: dos
+      - const: vdec
+      - const: hvdec
+
+  power-domains:
+    maxItems: 2
+
+  power-domain-names:
+    items:
+      - const: vdec
+      - const: hvdec
+
+  resets:
+    maxItems: 1
+
+  amlogic,canvas:
+    description: Canvas maintains an index-to-physical-address
+     mapping table that allows the 16-bit AMRISC decoder core
+     to access 32-bit or 64-bit physical address.
+    $ref: /schemas/types.yaml#/definitions/phandle
+
+  amlogic,secure-monitor:
+    description: Used to decrypt and load the signed decoder
+     firmware through ARM SMC (Secure Monitor Call).
+    $ref: /schemas/types.yaml#/definitions/phandle
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - interrupts
+  - clocks
+  - clock-names
+  - power-domains
+  - power-domain-names
+  - resets
+  - amlogic,canvas
+  - amlogic,secure-monitor
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/clock/amlogic,s4-pll-clkc.h>
+    #include <dt-bindings/clock/amlogic,s4-peripherals-clkc.h>
+    #include <dt-bindings/power/meson-s4-power.h>
+    #include <dt-bindings/reset/amlogic,meson-s4-reset.h>
+    video-codec@fe320000 {
+      compatible = "amlogic,s4-vdec";
+      reg = <0xfe320000 0x10000>,
+            <0xfe036000 0x20>;
+      reg-names = "dos",
+                  "dmc";
+      interrupts = <GIC_SPI 91 IRQ_TYPE_EDGE_RISING>,
+                   <GIC_SPI 92 IRQ_TYPE_EDGE_RISING>;
+      clocks = <&clkc_periphs CLKID_DOS>,
+               <&clkc_periphs CLKID_VDEC_SEL>,
+               <&clkc_periphs CLKID_HEVCF_SEL>;
+      clock-names = "dos",
+                    "vdec",
+                    "hvdec";
+      power-domains = <&pwrc PWRC_S4_DOS_VDEC_ID>,
+                      <&pwrc PWRC_S4_DOS_HEVC_ID>;
+      power-domain-names = "vdec",
+                           "hvdec";
+      resets = <&reset RESET_DOS>;
+      amlogic,canvas = <&canvas>;
+      amlogic,secure-monitor = <&sm>;
+    };

-- 
2.42.0

[PATCH v12 5/6] arm64: dts: amlogic: Add video decoder driver support for S4 SOCs

[Zhentao Guo via B4 Relay]

From: Zhentao Guo <zhentao.guo@amlogic.com>

Add vdec node to enable Amlogic V4L2 stateless video decoder
support. Since the video fw needs to be loaded via TEE helpers,
enable the optee node in this patch.

Signed-off-by: Zhentao Guo <zhentao.guo@amlogic.com>
---
 arch/arm64/boot/dts/amlogic/meson-s4.dtsi | 33 +++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-s4.dtsi b/arch/arm64/boot/dts/amlogic/meson-s4.dtsi
index 2a6fbd530836..7a1dcb3951c8 100644
--- a/arch/arm64/boot/dts/amlogic/meson-s4.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-s4.dtsi
@@ -80,6 +80,11 @@ pwrc: power-controller {
 				#power-domain-cells = <1>;
 			};
 		};
+
+		optee {
+			compatible = "linaro,optee-tz";
+			method = "smc";
+		};
 	};
 
 	saradc: adc@fe026000 {
@@ -907,5 +912,33 @@ emmc: mmc@fe08c000 {
 			assigned-clocks = <&clkc_periphs CLKID_SD_EMMC_C>;
 			assigned-clock-rates = <24000000>;
 		};
+
+		canvas: video-lut@fe036048 {
+			compatible = "amlogic,canvas";
+			reg = <0x0 0xfe036048 0x0 0x14>;
+		};
+
+		video-codec@fe320000 {
+			compatible = "amlogic,s4-vdec";
+			reg = <0x0 0xfe320000 0x0 0x10000>,
+			      <0x0 0xfe036000 0x0 0x20>;
+			reg-names = "dos",
+				    "dmc";
+			interrupts = <GIC_SPI 91 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 92 IRQ_TYPE_EDGE_RISING>;
+			clocks = <&clkc_periphs CLKID_DOS>,
+				 <&clkc_periphs CLKID_VDEC_SEL>,
+				 <&clkc_periphs CLKID_HEVCF_SEL>;
+			clock-names = "dos",
+				      "vdec",
+				      "hvdec";
+			power-domains = <&pwrc PWRC_S4_DOS_VDEC_ID>,
+					<&pwrc PWRC_S4_DOS_HEVC_ID>;
+			power-domain-names = "vdec",
+					     "hvdec";
+			resets = <&reset RESET_DOS>;
+			amlogic,canvas = <&canvas>;
+			amlogic,secure-monitor = <&sm>;
+		};
 	};
 };

-- 
2.42.0

[PATCH v12 6/6] arm64: defconfig: Enable CONFIG_VIDEO_AMLOGIC_VDEC

[Zhentao Guo via B4 Relay]

From: Zhentao Guo <zhentao.guo@amlogic.com>

Enable the Amlogic V4L2 stateless video decoder driver as a module
in the arm64 defconfig to support stateless video decoder on Amlogic
SoCs. The driver is used by Amlogic S4 platform, such as S805X2
board.

Signed-off-by: Zhentao Guo <zhentao.guo@amlogic.com>
---
 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 7b4985b29b21..f00c8a418a46 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -929,6 +929,7 @@ CONFIG_USB_VIDEO_CLASS=m
 CONFIG_V4L_PLATFORM_DRIVERS=y
 CONFIG_SDR_PLATFORM_DRIVERS=y
 CONFIG_V4L_MEM2MEM_DRIVERS=y
+CONFIG_VIDEO_AMLOGIC_VDEC=m
 CONFIG_VIDEO_AMPHION_VPU=m
 CONFIG_VIDEO_CADENCE_CSI2RX=m
 CONFIG_VIDEO_WAVE_VPU=m

-- 
2.42.0

Re: [PATCH v12 3/6] media: dt-bindings: Add Amlogic V4L2 video decoder

[Krzysztof Kozlowski]

On 22/10/2026 07:27, Zhentao Guo via B4 Relay wrote:
> From: Zhentao Guo <zhentao.guo@amlogic.com>
> 
> Add bindings for Amlogic hardware decoder accelerator.
> 
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
> Signed-off-by: Zhentao Guo <zhentao.guo@amlogic.com>
> ---
>  .../devicetree/bindings/media/amlogic,s4-vdec.yaml | 114 +++++++++++++++++++++
>  1 file changed, 114 insertions(+)

I do not understand why, but people have comments here which they do not
express by reviewing/commenting, but sending some follow up patches.

This needs fixes then.

Best regards,
Krzysztof

Re: [PATCH v12 4/6] decoder: Add V4L2 stateless H.264 decoder driver

[Krzysztof Kozlowski]

On 22/10/2026 07:27, Zhentao Guo via B4 Relay wrote:
> From: Zhentao Guo <zhentao.guo@amlogic.com>
> 
> Add initial support for V4L2 stateless video decoder
> driver on Amlogic S4(S805X2) platform. In phase 1,
> it supports 8bit H.264 bitstreams decoding. Currently
> only progressive streams are supported.

Please wrap commit message according to Linux coding style / submission
process (neither too early nor over the limit):
https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597

You already received this comment, no?

> 
> Signed-off-by: Zhentao Guo <zhentao.guo@amlogic.com>

Anyway, several issues were reported here in separate threading.

Best regards,
Krzysztof

Re: [PATCH v12 4/6] decoder: Add V4L2 stateless H.264 decoder driver

[Zhentao Guo]

Hi Krzysztof,

> On 22/10/2026 07:27, Zhentao Guo via B4 Relay wrote:
>> From: Zhentao Guo <zhentao.guo@amlogic.com>
>>
>> Add initial support for V4L2 stateless video decoder
>> driver on Amlogic S4(S805X2) platform. In phase 1,
>> it supports 8bit H.264 bitstreams decoding. Currently
>> only progressive streams are supported.
> Please wrap commit message according to Linux coding style / submission
> process (neither too early nor over the limit):
> https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597
>
> You already received this comment, no?
Thanks for pointing out again, I'll check carefully here as well as in 
other places.
>> Signed-off-by: Zhentao Guo <zhentao.guo@amlogic.com>
> Anyway, several issues were reported here in separate threading.
>
> Best regards,
> Krzysztof

Best regards,

Zhentao

Re: [PATCH v12 3/6] media: dt-bindings: Add Amlogic V4L2 video decoder

[Zhentao Guo]

Hi Krzysztof，

> On 22/10/2026 07:27, Zhentao Guo via B4 Relay wrote:
>> From: Zhentao Guo <zhentao.guo@amlogic.com>
>>
>> Add bindings for Amlogic hardware decoder accelerator.
>>
>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
>> Signed-off-by: Zhentao Guo <zhentao.guo@amlogic.com>
>> ---
>>   .../devicetree/bindings/media/amlogic,s4-vdec.yaml | 114 +++++++++++++++++++++
>>   1 file changed, 114 insertions(+)
> I do not understand why, but people have comments here which they do not
> express by reviewing/commenting, but sending some follow up patches.
>
> This needs fixes then.
You mean the patchset from Christian? Indeed, Christian did not give me 
review comments before posting his modification patches, nor did he add 
the other codec formats based on my latest patchset. I have already 
reminded him of this.
>
> Best regards,
> Krzysztof

BRs

Zhentao

Re: [PATCH v12 0/6] Add Amlogic stateless H.264 video decoder for S4

[Wojciech Biegański]

Hi,
I tested v12 series on AQ222 board with v7.2-rc1. Firmware version:
'v0.4.ff.I0c83495.75444b3' build 'Thu Apr 16 18:18:55 2026'.

RAW NV12 frames at the decoder output were visually OK across typical
resolutions and H.264 profiles (main, high, baseline) with and without
B-frames in the stream, also with I-frames only streams. I used Gstreamer's
(ver. 1.26) 'v4l2slh264dec' element on Trixie with both mmap and DMA-bufs
buffering (on the capture queue only). Input bitstreams where
FFmpeg-generated (testsrc2 + libx264) or publicly available test videos.

Decoder (and the output) remained stable with simulated (non key) frame
drop at max resolution (1920x1088, noise generated by FFmpeg's bitstream
filter).

The performance at max resolution, tested with 'filesrc ! parsebin !
v4l2slh264dec ! fakevideosink' pipeline was ~85 fps (fixed number of
outputted frames divided by the total execution time).

Tested-by: Wojciech Biegański <wbieganski@baylibre.com>

Re: [PATCH v12 0/6] Add Amlogic stateless H.264 video decoder for S4

[Nicolas Dufresne]

Hi,


---- Le ven., 30 oct. 2026 10:24:06 -0400 wbieganski@baylibre.com a écrit ----


> Hi,
> I tested v12 series on AQ222 board with v7.2-rc1. Firmware version:
> 'v0.4.ff.I0c83495.75444b3' build 'Thu Apr 16 18:18:55 2026'.


That reminded me, is that firmware in linux-firmware?


Nicolas

>
> RAW NV12 frames at the decoder output were visually OK across typical
> resolutions and H.264 profiles (main, high, baseline) with and without
> B-frames in the stream, also with I-frames only streams. I used Gstreamer's
> (ver. 1.26) 'v4l2slh264dec' element on Trixie with both mmap and DMA-bufs
> buffering (on the capture queue only). Input bitstreams where
> FFmpeg-generated (testsrc2 + libx264) or publicly available test videos.
>
> Decoder (and the output) remained stable with simulated (non key) frame
> drop at max resolution (1920x1088, noise generated by FFmpeg's bitstream
> filter).
>
> The performance at max resolution, tested with 'filesrc ! parsebin !
> v4l2slh264dec ! fakevideosink' pipeline was ~85 fps (fixed number of
> outputted frames divided by the total execution time).
>
> Tested-by: Wojciech Biegański
>

Re: [PATCH v12 0/6] Add Amlogic stateless H.264 video decoder for S4

[Zhentao Guo]

Hi Nicolas,

>
> Hi,
>
>
> ---- Le ven., 30 oct. 2026 10:24:06 -0400 wbieganski@baylibre.com a écrit ----
>
>
>> Hi,
>> I tested v12 series on AQ222 board with v7.2-rc1. Firmware version:
>> 'v0.4.ff.I0c83495.75444b3' build 'Thu Apr 16 18:18:55 2026'.
>
> That reminded me, is that firmware in linux-firmware?
This firmware is currently not in the repo of linux firmware. Should we 
merge the driver first, and then submit the blob to the firmware repo?
>
>
> Nicolas

BRs

Zhentao

>> RAW NV12 frames at the decoder output were visually OK across typical
>> resolutions and H.264 profiles (main, high, baseline) with and without
>> B-frames in the stream, also with I-frames only streams. I used Gstreamer's
>> (ver. 1.26) 'v4l2slh264dec' element on Trixie with both mmap and DMA-bufs
>> buffering (on the capture queue only). Input bitstreams where
>> FFmpeg-generated (testsrc2 + libx264) or publicly available test videos.
>>
>> Decoder (and the output) remained stable with simulated (non key) frame
>> drop at max resolution (1920x1088, noise generated by FFmpeg's bitstream
>> filter).
>>
>> The performance at max resolution, tested with 'filesrc ! parsebin !
>> v4l2slh264dec ! fakevideosink' pipeline was ~85 fps (fixed number of
>> outputted frames divided by the total execution time).
>>
>> Tested-by: Wojciech Biegański
>>

Re: [PATCH v12 0/6] Add Amlogic stateless H.264 video decoder for S4

[Nicolas Dufresne]

Hi Zhentao,

---- Le sam., 31 oct. 2026 00:51:29 -0400 zhentao.guo@amlogic.com a écrit ----

> > That reminded me, is that firmware in linux-firmware?
> This firmware is currently not in the repo of linux firmware. Should we
> merge the driver first, and then submit the blob to the firmware repo?



Thus far, we have always required the firmwares to land first, unless a strong justification or valid alternative was proposed.


Nicolas

Re: [PATCH v12 1/6] firmware: meson: sm: video firmware loading via secure monitor

[Nicolas Dufresne]

[-- Attachment #1: Type: text/plain, Size: 1180 bytes --]

Hi,

Le jeudi 22 octobre 2026 à 13:26 +0800, Zhentao Guo via B4 Relay a écrit :
> From: Zhentao Guo <zhentao.guo@amlogic.com>
> 
> Add SM_LOAD_VIDEO_FW to the secure monitor command enum
> to allow decoder drivers to load firmware through the meson_sm
> interface.
> 
> Signed-off-by: Zhentao Guo <zhentao.guo@amlogic.com>

Any feedback from firmware/meson maintainers already ? Wouldn't this be better
as its own patchset and documented as dependency ? Both ways work, but
understand that I cannot take your patches until these are in Linus tree (won't
build). This comments applies to patch 1/2, please coordinate and let us know
what is the plan.

Nicolas

> ---
>  include/linux/firmware/meson/meson_sm.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/linux/firmware/meson/meson_sm.h b/include/linux/firmware/meson/meson_sm.h
> index 3ebc2bd9a976..3b90b49d0552 100644
> --- a/include/linux/firmware/meson/meson_sm.h
> +++ b/include/linux/firmware/meson/meson_sm.h
> @@ -15,6 +15,7 @@ enum {
>  	SM_THERMAL_CALIB_READ,
>  	SM_A1_PWRC_SET,
>  	SM_A1_PWRC_GET,
> +	SM_LOAD_VIDEO_FW,
>  };
>  
>  struct meson_sm_firmware;

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v12 0/6] Add Amlogic stateless H.264 video decoder for S4

[Nicolas Dufresne]

[-- Attachment #1: Type: text/plain, Size: 20280 bytes --]

Hi,

Le jeudi 22 octobre 2026 à 13:26 +0800, Zhentao Guo via B4 Relay a écrit :
> 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.
> 
> The driver needs to work alongside with a signed firmware. The loading process of
> the signed fw is as follow.
> Stage1: Decypt and decompose the full firmware package when the driver is probed.
>   +---------------------+                        +---------------------+
>   |  Decoder Driver     |                        |  TEE Shared Memory  |
>   |  (Kernel Space)     |                        |                     |
>   |  +---------------+  |                        |  +---------------+  |
>   |  | video_ucode   |  |                        |  | firmware      |  |
>   |  |    .bin       |  |  Copy payload to SHM   |  |   payload     |  |
>   |  | (from fs)     |  | ---------------------> |  | (Secure RAM)  |  |
>   |  +---------------+  |                        |  +---------------+  |
>   +---------------------+                        +----------+----------+
>                                                             |
>                                                             | PTA Invocation
>                                                             v
>                                        +-------------------------------+
>                                        |             BL32              |
>                                        |  +-------------------------+  |
>                                        |  | Decrypt Firmware        |  |
>                                        |  +-----------+-------------+  |
>                                        |              |                |
>                                        |              v                |
>                                        |  +-------------------------+  |
>                                        |  | Decompose the full      |  |
> 				       |  | firmware pacakge        |  |
>                                        |  +-----------+-------------+  |
>                                        |              |                |
>                                        |              v                |
>                                        |  +-------------------------+  |
>                                        |  | Store decomposed .bin   |  |
>                                        |  | in Secure Memory        |  |
>                                        |  +-------------------------+  |
>                                        +-------------------------------+
> 
> Stage2: When a decode job is scheduled, load decrypted fw via secure monitor.
> 
>   +---------------------+
>   |  V4L2 M2M Framework |
>   |  +---------------+  |
>   |  |  device_run   |  |
>   |  +------+--------+  |
>   +---------+-----------+
>             |
>             v
>   +---------------------+                  +---------------------+
>   |  Decoder Driver     |                  |  Secure Monitor     |
>   |  (Kernel Space)     |                  |      (bl32)         |
>   |  +---------------+  |     SMC Call     |  +---------------+  |
>   |  | Select Codec  |  | ---------------> |  | Select & Load |  |
>   |  | Specific FW   |  |                  |  | firmware.bin  |  |
>   |  +---------------+  |                  |  | to AMRISC     |  |
>   +---------------------+                  |  +-------+-------+  |
>                                            +----------+----------+
>                                                       |
>                                                       v
>                                            +---------------------+
>                                            |     AMRISC Core     |
>                                            |  +---------------+  |
>                                            |  | Running fw on |  |
>                                            |  | AMRISC        |  |
>                                            |  +---------------+  |
>                                            +---------------------+
> 
> The driver is capable of:
> - Supporting stateless H.264 decoding up to a resolution 1920x1088(on the S805X2 platform).
> - 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
> 
> Retry the compliance test based on kernel 7.1.0:
> 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 : 7.1.0
> 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 : 7.1.0
> Hardware revision: 0x00000000 (0)
> Driver version : 7.1.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
> Fluster test result of JVT-AVC_V1.
> Result:
> Ran 77/135 tests successfully

After looking in my past data, 90/135 would be expected without deinterlacer. I
already find bugs in the review, comment on that patches.

> 
> - 52 test vectors failed due to interlaced or mbaff clips: The Amlogic stateless
>   decoder driver only support bitstreams with frame_mbs_only_flags == 1.
>   Test Vectors:
>         cabac_mot_fld0_full
>         cabac_mot_mbaff0_full
>         cabac_mot_picaff0_full
>         CABREF3_Sand_D
>         CAFI1_SVA_C
>         CAMA1_Sony_C

I don't know them all, but at least this one, and CVMA1_Sony_D do not contain
field coded macroblocks, and should decode fine without the interlaced reference
list. We've seen a lot of artifically limited decoder like yours output 90/135.

>         CAMA1_TOSHIBA_B
>         cama1_vtc_c
>         cama2_vtc_b
>         CAMA3_Sand_E
>         cama3_vtc_b
>         CAMACI3_Sony_C
>         CAMANL1_TOSHIBA_B
>         CAMANL2_TOSHIBA_B
>         CAMANL3_Sand_E
>         CAMASL3_Sony_B
>         CAMP_MOT_MBAFF_L30
>         CAMP_MOT_MBAFF_L31
>         CANLMA2_Sony_C
>         CANLMA3_Sony_C
>         CAPA1_TOSHIBA_B
>         CAPAMA3_Sand_F
>         cavlc_mot_fld0_full_B
>         cavlc_mot_mbaff0_full_B
>         cavlc_mot_picaff0_full_B
>         CVCANLMA2_Sony_C
>         CVFI1_Sony_D
>         CVFI1_SVA_C
>         CVFI2_Sony_H
>         CVFI2_SVA_C
>         CVMA1_Sony_D
>         CVMA1_TOSHIBA_B
>         CVMANL1_TOSHIBA_B
>         CVMANL2_TOSHIBA_B
>         CVMAPAQP3_Sony_E
>         CVMAQP2_Sony_G
>         CVMAQP3_Sony_D
>         CVMP_MOT_FLD_L30_B
>         CVNLFI1_Sony_C
>         CVNLFI2_Sony_H
>         CVPA1_TOSHIBA_B
>         FI1_Sony_E
>         MR6_BT_B
>         MR7_BT_B
>         MR8_BT_B
>         MR9_BT_B
>         Sharp_MP_Field_1_B
>         Sharp_MP_Field_2_B
>         Sharp_MP_Field_3_B
>         Sharp_MP_PAFF_1r2
>         Sharp_MP_PAFF_2r
>         CVMP_MOT_FRM_L31_B
> - 3 test vectors failed due to unsupported bitstream.
>   num_slice_group_minus1 greater than zero is not supported by the
>   hardware.
>   Test Vectors:
>         FM1_BT_B
>         FM1_FT_E
>         FM2_SVA_C
> - 2 test vectors failed because SP_SLICE type is not supported by the
>   hardware.
>   Test Vectors:
>         SP1_BT_A
>         sp2_bt_b
> 
> One remain failure is CVFC1_Sony_C, which contains crop information. The md5sum of every decoded YUV indicates that original output from the decoder was correct. The YUV was cropped by gstreamer. The correct cropping method for this bitstream should be to crop 30*2 rows of pixels from both the top and bottom of the image, and 13*2 columns of pixels from both the left and right sides.However, gstreamer cropped 13*4 columns of pixels from the right side and 30*4 rows of pixels from the bottom. We are trying to find out the cause of this. Other failuers mentioned in V1 and V2 were resolved.
> 

Would be nice to report this bug.

> Changes in v12:
> - Add additional check to avoid the resolution happened in middle of a bitstream without streamon/off, due to the non-standard habbit of the userspace.
> - Add a separate initialization claim, init_ctx, to protect the hardware initialization process.
> - Add rejection to the formats whose type does not match the queue in all four s_fmt/try_fmt paths.
> - Link to v11: https://lore.kernel.org/r/20261012-b4-s4-vdec-upstream-v11-0-25939f85ada5@amlogic.com
> 
> Changes in v11:
> - Check clock/power-on failures before operating the registers. Use pm_runtime_resume_and_get() instead of pm_runtime_get_sync() so the usage counter stays balanced on the error path.
> - Fix missing validation of untrusted bitstream input in the H.264 decoder.
> - Fix instance/resource lifetime issues: do not re-init a live decoder instance when only output or capture queue restarts.
> - Link to v10: https://lore.kernel.org/r/20260924-b4-s4-vdec-upstream-v10-0-1289303a58a4@amlogic.com
> 
> Changes in v10:
> - Rename the clock/power/interrupt names in dt-binding and dts to match the name of decoder sub-core (dos, vdec, hvdec).
> - Fix the h264 high profile bitstreams decoding error issue.
> - Use the allocation bitmaps instead of poc != INVALID_POC as the slot occupancy test, since INVALID_POC (0xffffffff) aliases with the valid POC value -1.
> - Link to v9: https://lore.kernel.org/r/20260914-b4-s4-vdec-upstream-v9-0-72fdb0d71b02@amlogic.com
> 
> Changes in v9:
> - Rename the clock/power/interrupt names in dt-binding and dts. Add the descriptions to the clock items in dt-binding.
> - Optimized the handling process for reference frame loss. The address of a lost reference frame will not be configured to the hardware, to prevent the hardware from accessing an illegal address.
> - Optimized the error handling paths in the driver based on v8. 
> - Link to v8: https://lore.kernel.org/r/20260831-b4-s4-vdec-upstream-v8-0-830195b9bd91@amlogic.com
> 
> Changes in v8:
> - Rename the clock/power names in dt-binding and dts. Add the descriptions to the canvas and secure-monitor node in dt-binding.
> - Improve code accroding to the feedback of sashiko bot, including:
>     1.Imporve the resource cleanup and error handling paths in the driver.
>     2.Mitigated race conditions between the decode path, interrupt handler, and resource teardown.
>     3.Validate codec-stream parameters passed to the driver to prevent the potential risk.
> - Link to v7: https://lore.kernel.org/r/20260812-b4-s4-vdec-upstream-v7-0-3fae085861bd@amlogic.com
> 
> Changes in v7:
> - Addressed Krzysztof's review comments: fixed DTS coding style, added board info requiring the config to patch 6/6's commit message.
> - Improvements made per sashiko bot review, including:
>     1.Fixed memory out-of-bounds access / buffer overflow issues.
>     2.Fixed concurrency and race conditions related issues.
>     3.Improved the error handling paths to avoid the risk of resource leakage.
>     4.Fixed the improper V4L2 framework usage.
> - Link to v6: https://lore.kernel.org/r/20260724-b4-s4-vdec-upstream-v6-0-6072a7ee7428@amlogic.com
> 
> Changes in v6:
> - Rename the HEVC core and the associated power, clock, and interrupt names. 'HEVC' was a development codename of the decoder IP which accelerates h265 vp9 av1 decoding, but it coincidentally matches the acronym for High Efficiency Video Coding. To avoid naming ambiguity, HEVC core is renamed to HVDEC core(H stands for HEVC).
> - Get rid of the unused function and interface.
> - Link to v5: https://lore.kernel.org/r/20260526-b4-s4-vdec-upstream-v5-0-33bc817f93f4@amlogic.com
> 
> Changes in v5:
> - Rename the compatible and the clock item accroding to Krzysztof's feedback.
> - Use tee & meson_sm helpers to decrypt load the signed decoder firmware. Add the meson_sm describsion and reference to dt-binding and dts.
> - Link to v4: https://lore.kernel.org/r/20260213-b4-s4-vdec-upstream-v4-0-c7112d00d662@amlogic.com
> 
> Changes in v4:
> - Use %pad to print dma_addr_t type instead of using %llx.
> - Add initial values to some local variables.
> - Link to v3: https://lore.kernel.org/r/20260121-b4-s4-vdec-upstream-v3-0-4496aec3d79e@amlogic.com
> 
> Changes in v3:
> - Fixed the DT check error:
>   arch/arm64/boot/dts/amlogic/meson-s4-s805x2-aq222.dtb: video-codec@fe320000 (amlogic,s4-vcodec-dec): 'amlogic,canvas' does not match any of the regexes: '^pinctrl-[0-9]+$' 
>   from schema $id: http://devicetree.org/schemas/media/amlogic,vcodec-dec.yaml
> - Added DOS reset lines to dtsi and dt-binding.
> - Fixed the issue where some B-frames were not decoded correctly(The fluster failures mentioned in patch V1 and V2 were mostly caused by this).
> - Fixed the issue where canvas_index leaks occurred during the decoding of some bitstreams.
> - Rework the src/dst format storage. Use v4l2_pix_format_mplane to store formats that related to bitstreams into the context. Add the reset format function to reset all the formats to default value. 
> - Store decoding parameters related to chip platforms, such as maximum width/height and alignment requirement, organized by chip platform.
> - Link to v2: https://lore.kernel.org/r/20251124-b4-s4-vdec-upstream-v2-0-bdbbce3f11a6@amlogic.com
> 
> Changes in v2:
> - Fixed incorrect generation of the reference lists for some B-frames.
> - Rename or get rid of some properties in DTS and dt-binding.
> - Remove some useless code or helper functions, (eg. clk helper functions, reg I/O macros, and some superfluous print messages) replace these functions with existing ones.
> - Replace all the printk messages with dev_err/dev_info/dev_dbg
> - Use the helper functions from the existing meson-canvas driver.
> - Use clk_bulk_data to map clocks from DTS.
> - Retry the V4L2 Compliance test on 6.18-rc6, fix a newly introduced bug.
> - Link to v1: https://lore.kernel.org/r/20251027-b4-s4-vdec-upstream-v1-0-620401813b5d@amlogic.com
> 
> Signed-off-by: Zhentao Guo <zhentao.guo@amlogic.com>
> ---
> Zhentao Guo (6):
>       firmware: meson: sm: video firmware loading via secure monitor
>       firmware: meson: sm: Add video firmware loading SMC call
>       media: dt-bindings: Add Amlogic V4L2 video decoder
>       decoder: Add V4L2 stateless H.264 decoder driver
>       arm64: dts: amlogic: Add video decoder driver support for S4 SOCs
>       arm64: defconfig: Enable CONFIG_VIDEO_AMLOGIC_VDEC
> 
>  .../devicetree/bindings/media/amlogic,s4-vdec.yaml |  114 +
>  MAINTAINERS                                        |    7 +
>  arch/arm64/boot/dts/amlogic/meson-s4.dtsi          |   33 +
>  arch/arm64/configs/defconfig                       |    1 +
>  drivers/firmware/meson/meson_sm.c                  |    1 +
>  drivers/media/platform/amlogic/Kconfig             |    1 +
>  drivers/media/platform/amlogic/Makefile            |    1 +
>  drivers/media/platform/amlogic/vdec/Kconfig        |   18 +
>  drivers/media/platform/amlogic/vdec/Makefile       |    4 +
>  drivers/media/platform/amlogic/vdec/TODO           |    7 +
>  drivers/media/platform/amlogic/vdec/aml_vdec.c     |  846 ++++++++
>  drivers/media/platform/amlogic/vdec/aml_vdec.h     |   33 +
>  drivers/media/platform/amlogic/vdec/aml_vdec_drv.c |  260 +++
>  drivers/media/platform/amlogic/vdec/aml_vdec_drv.h |  168 ++
>  drivers/media/platform/amlogic/vdec/aml_vdec_hw.c  |  570 +++++
>  drivers/media/platform/amlogic/vdec/aml_vdec_hw.h  |  161 ++
>  .../platform/amlogic/vdec/aml_vdec_platform.c      |   81 +
>  .../platform/amlogic/vdec/aml_vdec_platform.h      |   46 +
>  .../media/platform/amlogic/vdec/aml_vdec_tee_fw.c  |  249 +++
>  .../media/platform/amlogic/vdec/aml_vdec_tee_fw.h  |   27 +
>  drivers/media/platform/amlogic/vdec/h264.c         | 2209 ++++++++++++++++++++
>  drivers/media/platform/amlogic/vdec/h264.h         |  299 +++
>  drivers/media/platform/amlogic/vdec/reg_defines.h  |  177 ++
>  include/linux/firmware/meson/meson_sm.h            |    1 +
>  24 files changed, 5314 insertions(+)
> ---
> base-commit: 2efb7a11f1aa4d2dc285fe2680cca22cb9f72f64
> change-id: 20251027-b4-s4-vdec-upstream-0603c1a4c84a
> 
> Best regards,

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v12 4/6] decoder: Add V4L2 stateless H.264 decoder driver

[Nicolas Dufresne]

[-- Attachment #1: Type: text/plain, Size: 49144 bytes --]

Hi,

Le jeudi 22 octobre 2026 à 13:27 +0800, Zhentao Guo via B4 Relay a écrit :
> From: Zhentao Guo <zhentao.guo@amlogic.com>
> 
> Add initial support for V4L2 stateless video decoder
> driver on Amlogic S4(S805X2) platform. In phase 1,
> it supports 8bit H.264 bitstreams decoding. Currently
> only progressive streams are supported.
> 
> Signed-off-by: Zhentao Guo <zhentao.guo@amlogic.com>
> ---
>  MAINTAINERS                                        |    7 +
>  drivers/media/platform/amlogic/Kconfig             |    1 +
>  drivers/media/platform/amlogic/Makefile            |    1 +
>  drivers/media/platform/amlogic/vdec/Kconfig        |   18 +
>  drivers/media/platform/amlogic/vdec/Makefile       |    4 +
>  drivers/media/platform/amlogic/vdec/TODO           |    7 +
>  drivers/media/platform/amlogic/vdec/aml_vdec.c     |  846 ++++++++
>  drivers/media/platform/amlogic/vdec/aml_vdec.h     |   33 +
>  drivers/media/platform/amlogic/vdec/aml_vdec_drv.c |  260 +++
>  drivers/media/platform/amlogic/vdec/aml_vdec_drv.h |  168 ++
>  drivers/media/platform/amlogic/vdec/aml_vdec_hw.c  |  570 +++++
>  drivers/media/platform/amlogic/vdec/aml_vdec_hw.h  |  161 ++
>  .../platform/amlogic/vdec/aml_vdec_platform.c      |   81 +
>  .../platform/amlogic/vdec/aml_vdec_platform.h      |   46 +
>  .../media/platform/amlogic/vdec/aml_vdec_tee_fw.c  |  249 +++
>  .../media/platform/amlogic/vdec/aml_vdec_tee_fw.h  |   27 +
>  drivers/media/platform/amlogic/vdec/h264.c         | 2209 ++++++++++++++++++++
>  drivers/media/platform/amlogic/vdec/h264.h         |  299 +++
>  drivers/media/platform/amlogic/vdec/reg_defines.h  |  177 ++
>  19 files changed, 5164 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 149bc44003bd..58df555ae864 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1458,6 +1458,13 @@ S:	Maintained
>  F:	Documentation/devicetree/bindings/spi/amlogic,a4-spisg.yaml
>  F:	drivers/spi/spi-amlogic-spisg.c
>  
> +AMLOGIC VDEC DRIVER
> +M:	Zhentao Guo <zhentao.guo@amlogic.com>
> +L:	linux-media@vger.kernel.org
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/media/amlogic,s4-vdec.yaml
> +F:	drivers/media/platform/amlogic/vdec/
> +
>  AMPHENOL CHIPCAP 2 DRIVER
>  M:	Javier Carrasco <javier.carrasco.cruz@gmail.com>
>  L:	linux-hwmon@vger.kernel.org
> diff --git a/drivers/media/platform/amlogic/Kconfig b/drivers/media/platform/amlogic/Kconfig
> index 458acf3d5fa8..7c541ac0d0c3 100644
> --- a/drivers/media/platform/amlogic/Kconfig
> +++ b/drivers/media/platform/amlogic/Kconfig
> @@ -4,3 +4,4 @@ comment "Amlogic media platform drivers"
>  
>  source "drivers/media/platform/amlogic/c3/Kconfig"
>  source "drivers/media/platform/amlogic/meson-ge2d/Kconfig"
> +source "drivers/media/platform/amlogic/vdec/Kconfig"
> diff --git a/drivers/media/platform/amlogic/Makefile b/drivers/media/platform/amlogic/Makefile
> index c744afcd1b9e..7409de674c0b 100644
> --- a/drivers/media/platform/amlogic/Makefile
> +++ b/drivers/media/platform/amlogic/Makefile
> @@ -2,3 +2,4 @@
>  
>  obj-y += c3/
>  obj-y += meson-ge2d/
> +obj-y += vdec/
> diff --git a/drivers/media/platform/amlogic/vdec/Kconfig b/drivers/media/platform/amlogic/vdec/Kconfig
> new file mode 100644
> index 000000000000..2c5dd8452962
> --- /dev/null
> +++ b/drivers/media/platform/amlogic/vdec/Kconfig
> @@ -0,0 +1,18 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR MIT)
> +
> +config VIDEO_AMLOGIC_VDEC
> +    tristate "Amlogic Video Decoder Driver"
> +    depends on ARCH_MESON || COMPILE_TEST
> +    depends on VIDEO_DEV
> +    depends on V4L_MEM2MEM_DRIVERS
> +    depends on TEE
> +    depends on MESON_SM
> +    select VIDEOBUF2_DMA_CONTIG
> +    select V4L2_H264
> +    select V4L2_MEM2MEM_DEV
> +    select MESON_CANVAS
> +    help
> +      This is a v4l2 driver for Amlogic video decoder driver.
> +      This driver is designed to support V4L2 M2M STATELESS
> +      interface.
> +      To compile this driver as a module choose m here.
> diff --git a/drivers/media/platform/amlogic/vdec/Makefile b/drivers/media/platform/amlogic/vdec/Makefile
> new file mode 100644
> index 000000000000..f752716cbf9e
> --- /dev/null
> +++ b/drivers/media/platform/amlogic/vdec/Makefile
> @@ -0,0 +1,4 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +aml-vdec-drv-objs := aml_vdec.o aml_vdec_drv.o aml_vdec_hw.o aml_vdec_platform.o h264.o aml_vdec_tee_fw.o\
> +
> +obj-$(CONFIG_VIDEO_AMLOGIC_VDEC) += aml-vdec-drv.o
> diff --git a/drivers/media/platform/amlogic/vdec/TODO b/drivers/media/platform/amlogic/vdec/TODO
> new file mode 100644
> index 000000000000..54c60145770e
> --- /dev/null
> +++ b/drivers/media/platform/amlogic/vdec/TODO
> @@ -0,0 +1,7 @@
> +TODO list for Amlogic V4L2 stateless decoder driver:
> +
> +1. Support decoding for HEVC, VP9, AV1, and MPEG-2.
> +2. Support more SoCs, including the new T7/S7 series and legacy SoCs (e.g., GXL, SM1, G12B).
> +3. Support 10-bit decoding and P010 output.
> +   Note: P010 output requires hardware support.
> +4. Support interlaced stream decoding for H.264, HEVC, and MPEG-2.
> diff --git a/drivers/media/platform/amlogic/vdec/aml_vdec.c b/drivers/media/platform/amlogic/vdec/aml_vdec.c
> new file mode 100644
> index 000000000000..e65ebb2ce311
> --- /dev/null
> +++ b/drivers/media/platform/amlogic/vdec/aml_vdec.c
> @@ -0,0 +1,846 @@
> +// SPDX-License-Identifier: (GPL-2.0-only OR MIT)
> +/*
> + * Copyright (C) 2025 Amlogic, Inc. All rights reserved
> + */
> +
> +#include <media/v4l2-mem2mem.h>
> +#include <media/v4l2-ctrls.h>
> +#include <media/v4l2-event.h>
> +#include <media/videobuf2-v4l2.h>
> +#include <media/videobuf2-dma-contig.h>
> +
> +#include "aml_vdec.h"
> +#include "aml_vdec_hw.h"
> +#include "aml_vdec_platform.h"
> +#include "aml_vdec_tee_fw.h"
> +
> +#define VCODEC_DRV_NAME "aml-vdec-drv"
> +
> +static const struct aml_vdec_v4l2_ctrl controls[] = {
> +	{
> +		.codec_type = CODEC_TYPE_H264,
> +		.cfg = {
> +			.id = V4L2_CID_STATELESS_H264_DECODE_PARAMS,
> +		},
> +	}, {
> +		.codec_type = CODEC_TYPE_H264,
> +		.cfg = {
> +			.id = V4L2_CID_STATELESS_H264_SPS,
> +		},
> +	}, {
> +		.codec_type = CODEC_TYPE_H264,
> +		.cfg = {
> +			.id = V4L2_CID_STATELESS_H264_PPS,
> +		},
> +	}, {
> +		.codec_type = CODEC_TYPE_H264,
> +		.cfg = {
> +			.id = V4L2_CID_STATELESS_H264_SCALING_MATRIX,
> +		},
> +	}, {
> +		.codec_type = CODEC_TYPE_H264,
> +		.cfg = {
> +			.id = V4L2_CID_STATELESS_H264_DECODE_MODE,
> +			.min = V4L2_STATELESS_H264_DECODE_MODE_FRAME_BASED,
> +			.def = V4L2_STATELESS_H264_DECODE_MODE_FRAME_BASED,
> +			.max = V4L2_STATELESS_H264_DECODE_MODE_FRAME_BASED,
> +		},
> +	}, {
> +		.codec_type = CODEC_TYPE_H264,
> +		.cfg = {
> +			.id = V4L2_CID_MPEG_VIDEO_H264_LEVEL,
> +		},
> +	}, {
> +		.codec_type = CODEC_TYPE_H264,
> +		.cfg = {
> +			.id = V4L2_CID_STATELESS_H264_START_CODE,
> +			.min = V4L2_STATELESS_H264_START_CODE_ANNEX_B,
> +			.def = V4L2_STATELESS_H264_START_CODE_ANNEX_B,
> +			.max = V4L2_STATELESS_H264_START_CODE_ANNEX_B,
> +		},
> +	}, {
> +		.codec_type = CODEC_TYPE_H264,
> +		.cfg = {
> +			.id = V4L2_CID_MPEG_VIDEO_H264_PROFILE,
> +			.min = V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE,
> +			.max = V4L2_MPEG_VIDEO_H264_PROFILE_HIGH,
> +			.def = V4L2_MPEG_VIDEO_H264_PROFILE_MAIN,
> +		},
> +	}
> +};
> +
> +static const struct aml_dec_type dec_type_name[] = {
> +	{
> +		.codec_type = CODEC_TYPE_H264,
> +		.name = "H264",
> +	},
> +};
> +
> +static const char *dec_type_to_name(unsigned int type)
> +{
> +	int i;
> +	int size = ARRAY_SIZE(dec_type_name);
> +
> +	for (i = 0; i < size; i++) {
> +		if (dec_type_name[i].codec_type == type)
> +			return dec_type_name[i].name;
> +	}
> +
> +	return "ERR";
> +}
> +
> +int aml_vdec_ctrls_setup(struct aml_vdec_ctx *ctx)
> +{
> +	int i;
> +	int ret;
> +	int ctrls_size = ARRAY_SIZE(controls);
> +
> +	v4l2_ctrl_handler_init(&ctx->ctrl_handler, ctrls_size);
> +	for (i = 0; i < ctrls_size; i++) {
> +		v4l2_ctrl_new_custom(&ctx->ctrl_handler, &controls[i].cfg, NULL);
> +		if (ctx->ctrl_handler.error) {
> +			dev_info(&ctx->dev->plat_dev->dev, "add ctrl for (%d) failed%d\n",
> +				 controls[i].cfg.id, ctx->ctrl_handler.error);
> +			v4l2_ctrl_handler_free(&ctx->ctrl_handler);
> +			return ctx->ctrl_handler.error;
> +		}
> +	}
> +	ctx->fh.ctrl_handler = &ctx->ctrl_handler;
> +
> +	ret = v4l2_ctrl_handler_setup(&ctx->ctrl_handler);
> +	if (ret)
> +		v4l2_ctrl_handler_free(&ctx->ctrl_handler);
> +
> +	return ret;
> +}
> +
> +static void m2mops_vdec_device_run(void *m2m_priv)

Namespaces are inconsistant since the start of this patch. 

> +{
> +	struct aml_vdec_ctx *ctx = (struct aml_vdec_ctx *)m2m_priv;
> +	struct aml_vdec_dev *dev = ctx->dev;
> +	struct vb2_v4l2_buffer *src_buf, *dst_buf;
> +	struct media_request *src_req;
> +	int ret = 0;
> +
> +	src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
> +	dst_buf = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
> +	dev_dbg(&dev->plat_dev->dev, "device run : src buf : %d dst buf %d\n",
> +		src_buf->vb2_buf.index, dst_buf->vb2_buf.index);
> +	if (WARN_ON_ONCE(!ctx->codec_ops->run))

It looks like a BUG_ON to me, any reason they may happen and would require fail-
safety ?

> +		goto err_cancel_job;
> +
> +	src_req = src_buf->vb2_buf.req_obj.req;
> +	if (src_req)
> +		v4l2_ctrl_request_setup(src_req, &ctx->ctrl_handler);
> +
> +	/*
> +	 * Claim the hardware initialization window: curr_ctx has a
> +	 * strictly per-frame lifecycle (set by the codec run callback,
> +	 * cleared when the frame completes) and cannot cover the
> +	 * bring-up below. The claim tells a concurrent aml_h264_exit()
> +	 * from another instance to leave the hardware alone; it is
> +	 * handed over to curr_ctx when the frame starts.
> +	 */
> +	mutex_lock(&dev->dec_hw->ctx_mutex);
> +	dev->dec_hw->init_ctx = ctx->codec_priv;
> +	mutex_unlock(&dev->dec_hw->ctx_mutex);

Sounds like "the claim" refer to some sashiko report ? Might be nice to
reference it, but also, I don't understand a thing here. How is init_ctx pointer
safer because you copied codec_priv under a mutex? Normally, to fix such a
lifetime issue, we'd opt for kref.

> +
> +	dos_enable(dev->dec_hw);
> +	/* incase of bus hang in stop_streaming */
> +	ctx->dos_clk_en = 1;

The code in aml_h264_exit() uses mutex, which hint that it is maybe running in a
concurrent thread and so dos_enable() + setting dos_clk_en should be serialized
under the ctx_mutex. The race might not exist in practice, but the locking seems
to be an after thought, make it safe by design, you always pay the cost.

> +	ctx->curr_frm_err_flag = 0;
> +
> +	if (ctx->curr_dec_type == CODEC_TYPE_H264)
> +		aml_vdec_reset_core(dev->dec_hw);
> +
> +	if (load_firmware(dev->dec_hw, ctx->curr_dec_type) < 0)
> +		goto err_load_fw;
> +
> +	ret = ctx->codec_ops->run(ctx);

We encourage not doing this synchronously, as this introduce an extra context
switch from the threaded IRQ to here. Only handle synchronous failures (like bad
parameters) in the lower part, finish the job in the threaded IRQ. Its not
invalid, just not optimal.

> +
> +	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf);
> +	if (src_req)
> +		v4l2_ctrl_request_complete(src_req, &ctx->ctrl_handler);
> +	if (ret < 0 || ctx->curr_frm_err_flag)
> +		goto err_cancel_job;
> +	v4l2_m2m_buf_done_and_job_finish(dev->m2m_dev_dec, ctx->m2m_ctx,
> +					 VB2_BUF_STATE_DONE);
> +	return;
> +
> +err_load_fw:
> +	mutex_lock(&dev->dec_hw->ctx_mutex);
> +	dev->dec_hw->init_ctx = NULL;
> +	mutex_unlock(&dev->dec_hw->ctx_mutex);
> +	if (src_req)
> +		v4l2_ctrl_request_complete(src_req, &ctx->ctrl_handler);
> +err_cancel_job:
> +	v4l2_m2m_buf_done_and_job_finish(dev->m2m_dev_dec, ctx->m2m_ctx,
> +					 VB2_BUF_STATE_ERROR);

There is clear cases, like err_load_fw where nothing has been written in the
CAPTURE buffer. And ret < 0 might be another one ? In these cases, which are
unlike missing slices or missing references, set the payload size to 0. This
will tell userspace this is a frame to be discarded. I'm working on GStreamer
fix for failed frame discard, and corrupted frame marking, other implementation
such as chromium have it.

> +}
> +
> +const struct v4l2_m2m_ops aml_vdec_m2m_ops = {
> +	.device_run = m2mops_vdec_device_run,
> +};
> +
> +static int vidioc_vdec_querycap(struct file *file, void *priv,
> +				struct v4l2_capability *cap)
> +{
> +	strscpy(cap->driver, VCODEC_DRV_NAME, sizeof(cap->driver));
> +	strscpy(cap->card, "platform:" VCODEC_DRV_NAME, sizeof(cap->card));
> +
> +	return 0;
> +}
> +
> +static int vidioc_vdec_enum_fmt(struct aml_vdec_ctx *ctx,
> +				struct v4l2_fmtdesc *f, bool is_output)
> +{
> +	struct aml_vdec_dev *dev = ctx->dev;
> +	const struct aml_video_fmt *fmt;
> +	int i = 0, j = 0;
> +
> +	for (; i < dev->pvdec_data->num_fmts; i++) {
> +		fmt = &dev->pvdec_data->dec_fmt[i];
> +		if (is_output && fmt->type != AML_FMT_DEC)
> +			continue;
> +		if (!is_output && fmt->type != AML_FMT_FRAME)
> +			continue;
> +
> +		if (j == f->index) {
> +			f->pixelformat = fmt->fourcc;
> +			strscpy(f->description, fmt->name,
> +				sizeof(f->description));
> +			return 0;
> +		}
> +		++j;
> +	}
> +	return -EINVAL;
> +}
> +
> +static const struct aml_video_fmt *aml_vdec_get_video_fmt(struct aml_vdec_dev
> +							  *dev, u32 format)
> +{
> +	const struct aml_video_fmt *fmt;
> +	unsigned int k;
> +
> +	for (k = 0; k < dev->pvdec_data->num_fmts; k++) {
> +		fmt = &dev->pvdec_data->dec_fmt[k];
> +		if (fmt->fourcc == format)
> +			return fmt;
> +	}
> +
> +	return NULL;
> +}
> +
> +static int aml_vdec_init_dec_inst(struct aml_vdec_ctx *ctx)
> +{
> +	struct aml_vdec_dev *dev = ctx->dev;
> +	struct aml_video_fmt *fmt_out = &ctx->dec_fmt[AML_FMT_SRC];
> +	int ret = -1;
> +
> +	ctx->codec_ops = &dev->pvdec_data->codec_ops[fmt_out->codec_type];
> +	/*
> +	 * The codec instance is only released when both queues are
> +	 * streamed off: do not allocate a new one (leaking the previous
> +	 * instance) when the output queue is restarted while the
> +	 * capture queue is still streaming.
> +	 */
> +	if (ctx->codec_ops->init && !ctx->codec_priv) {
> +		ret = ctx->codec_ops->init(ctx);
> +		if (ret < 0)
> +			return ret;
> +	}
> +	ctx->curr_dec_type = fmt_out->codec_type;

That's not much symmetrical. My worry now is that maybe we endup with
curr_dec_type not matching the codec_priv, which will lead to memory corruption.

> +	dev_info(&dev->plat_dev->dev, "%s set curr_dec_type %s\n",
> +		 __func__, dec_type_to_name(ctx->curr_dec_type));
> +
> +	return ret;
> +}
> +
> +static void set_pic_info(struct aml_vdec_ctx *ctx,
> +			 struct v4l2_pix_format_mplane *pix_mp,
> +			 enum v4l2_buf_type type)

This is used once, either fold or move next to its users which is not
vidioc_vdec_enum_framesizes().

> +{
> +	if (V4L2_TYPE_IS_OUTPUT(type)) {
> +		ctx->pic_info.output_pix_fmt = pix_mp->pixelformat;
> +		ctx->pic_info.coded_width = ALIGN(pix_mp->width, 64);
> +		ctx->pic_info.coded_height = ALIGN(pix_mp->height, 64);

The coded size alignment in H.264 is 16 steps, this isn't a coded width/height
you store here, but a padded_width/height.

> +		ctx->pic_info.fb_size[0] =
> +		    ctx->pic_info.coded_width * ctx->pic_info.coded_height;
> +		ctx->pic_info.fb_size[1] = ctx->pic_info.fb_size[0] / 2;

All this is duplication of the dst_fmt. Size should be pixel format dependent,
not hard coded to NV12 (which is not even commented here). Check other driver,
but basically, s_fmt should fait if the capture queue is busy, and reset the
dst_fmt to a valid value otherwise. The calculation of NV12 stride and sizes
should not be open coded but use v4l2_common. This means using
v4l2_apply_frmsize_constraints() and v4l2_fill_pixfmt_mp().

> +	}
> +}
> +
> +static int vidioc_vdec_enum_framesizes(struct file *file, void *priv,
> +				       struct v4l2_frmsizeenum *fsize)
> +{
> +	const struct aml_video_fmt *fmt;
> +	struct aml_vdec_dev *dev = video_drvdata(file);
> +
> +	if (fsize->index != 0)
> +		return -EINVAL;
> +
> +	fmt = aml_vdec_get_video_fmt(dev, fsize->pixel_format);
> +	if (!fmt)
> +		return -EINVAL;
> +
> +	fsize->type = V4L2_FRMSIZE_TYPE_STEPWISE;
> +	fsize->stepwise = fmt->stepwise;
> +
> +	return 0;
> +}
> +
> +static int vdec_try_fmt_mp(struct aml_vdec_ctx *ctx, enum v4l2_buf_type type,
> +			   struct v4l2_pix_format_mplane *pix_mp,
> +			   const struct aml_video_fmt *dec_fmt)
> +{
> +	int i, align;
> +
> +	pix_mp->field = V4L2_FIELD_NONE;
> +	align = dec_fmt->align;
> +
> +	if (V4L2_TYPE_IS_OUTPUT(type)) {
> +		pix_mp->num_planes = dec_fmt->num_planes;
> +		pix_mp->pixelformat = dec_fmt->fourcc;
> +		if (!pix_mp->plane_fmt[0].sizeimage)
> +			pix_mp->plane_fmt[0].sizeimage =
> +			    (pix_mp->height * pix_mp->width * 3) / 2;

Comment that this is bitstream default size.

> +		pix_mp->height = ALIGN(pix_mp->height, align);
> +		pix_mp->width = ALIGN(pix_mp->width, align);

Why do you need this, you call v4l2_apply_frmsize_constraints() right below. If
you configured the stepwise properly, it would just work.

> +	} else {
> +		pix_mp->height = ctx->pix_fmt[AML_FMT_SRC].height;
> +		pix_mp->width = ctx->pix_fmt[AML_FMT_SRC].width;
> +	}
> +
> +	v4l2_apply_frmsize_constraints(&pix_mp->width, &pix_mp->height,
> +				       &dec_fmt->stepwise);
> +	dev_dbg(&ctx->dev->plat_dev->dev,
> +		"%s type %d four_cc %d pix_mp->width %d pix_mp->height %d\n",
> +		__func__, type, dec_fmt->fourcc, pix_mp->width, pix_mp->height);
> +
> +	v4l2_fill_pixfmt_mp(pix_mp, dec_fmt->fourcc, pix_mp->width,
> +			    pix_mp->height);
> +
> +	for (i = 0; i < pix_mp->num_planes; i++)
> +		memset(&pix_mp->plane_fmt[i].reserved[0], 0x0,
> +		       sizeof(pix_mp->plane_fmt[0].reserved));
> +
> +	memset(pix_mp->reserved, 0x0, sizeof(pix_mp->reserved));
> +	pix_mp->flags = 0;

Drop this, should be filled by the core already.

> +
> +	dev_dbg(&ctx->dev->plat_dev->dev,
> +		"%s type %d fmt %d num_plane %d sizeimage0 %d sizeimage1 %d\n",
> +		__func__, type, pix_mp->pixelformat, pix_mp->num_planes,
> +		pix_mp->plane_fmt[0].sizeimage, pix_mp->plane_fmt[1].sizeimage);

This is all traced by the code and application can enable/disable this, please
drop.

> +
> +	return 0;
> +}
> +
> +static int vdec_s_fmt_output(struct aml_vdec_ctx *ctx, struct v4l2_format *f)
> +{
> +	struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp;
> +	struct v4l2_pix_format_mplane *cap_fmt = &ctx->pix_fmt[AML_FMT_DST];
> +	const struct aml_video_fmt *out_fmt;
> +	struct vb2_queue *vq, *peer_vq;
> +	int ret;
> +
> +	vq = v4l2_m2m_get_vq(ctx->m2m_ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE);
> +	if (vb2_is_busy(vq) &&
> +	    pix_mp->pixelformat != ctx->pix_fmt[AML_FMT_SRC].pixelformat)
> +		return -EBUSY;
> +
> +	/*
> +	 * Since s_fmt output will reset the width and height in the capture queue,
> +	 * s_fmt_output with resolution change should not take effect when capture
> +	 * queue is busy.
> +	 */
> +	peer_vq = v4l2_m2m_get_vq(ctx->m2m_ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
> +	if (vb2_is_busy(peer_vq) &&
> +	    (pix_mp->width != cap_fmt->width ||
> +	    pix_mp->height != cap_fmt->height))

As a future precaution, add fmt compare too.

> +		return -EBUSY;
> +
> +	out_fmt = aml_vdec_get_video_fmt(ctx->dev, pix_mp->pixelformat);
> +	if (out_fmt && out_fmt->type != AML_FMT_DEC)
> +		out_fmt = NULL;
> +	if (out_fmt) {
> +		ctx->dec_fmt[AML_FMT_SRC] = *out_fmt;
> +	} else {
> +		out_fmt = &ctx->dec_fmt[AML_FMT_SRC];
> +		dev_dbg(&ctx->dev->plat_dev->dev,
> +			"%s fmt %d not supported, use default\n", __func__,
> +			pix_mp->pixelformat);
> +	}
> +
> +	ret = vdec_try_fmt_mp(ctx, f->type, pix_mp, out_fmt);
> +	set_pic_info(ctx, pix_mp, f->type);

Check other driver, what I'm trying to suggest it to store the src/dst_fmt, and
reset the dst_fmt instead of duplicating things with the pic info.

> +
> +	ctx->pix_fmt[AML_FMT_SRC] = *pix_mp;
> +	cap_fmt->width = pix_mp->width;
> +	cap_fmt->height = pix_mp->height;
> +	v4l2_fill_pixfmt_mp(cap_fmt, ctx->dec_fmt[AML_FMT_DST].fourcc,
> +			    cap_fmt->width, cap_fmt->height);
> +	cap_fmt->colorspace = pix_mp->colorspace;
> +	cap_fmt->quantization = pix_mp->quantization;
> +	cap_fmt->ycbcr_enc = pix_mp->ycbcr_enc;
> +	cap_fmt->xfer_func = pix_mp->xfer_func;
> +
> +	return ret;
> +}
> +
> +static int vdec_s_fmt_capture(struct aml_vdec_ctx *ctx, struct v4l2_format *f)
> +{
> +	struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp;
> +	const struct aml_video_fmt *cap_fmt;
> +	struct vb2_queue *vq;
> +	int ret;
> +
> +	vq = v4l2_m2m_get_vq(ctx->m2m_ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
> +	if (vb2_is_busy(vq))
> +		return -EBUSY;
> +
> +	cap_fmt = aml_vdec_get_video_fmt(ctx->dev, pix_mp->pixelformat);
> +	if (cap_fmt && cap_fmt->type != AML_FMT_FRAME)
> +		cap_fmt = NULL;
> +	if (cap_fmt) {
> +		ctx->dec_fmt[AML_FMT_DST] = *cap_fmt;
> +	} else {
> +		cap_fmt = &ctx->dec_fmt[AML_FMT_DST];
> +		dev_dbg(&ctx->dev->plat_dev->dev,
> +			"%s fmt %d not supported, use default\n", __func__,
> +			pix_mp->pixelformat);
> +	}
> +
> +	ret = vdec_try_fmt_mp(ctx, f->type, pix_mp, cap_fmt);
> +
> +	ctx->pix_fmt[AML_FMT_DST] = *pix_mp;
> +
> +	return ret;
> +}
> +
> +static int vdec_try_fmt_output(struct aml_vdec_ctx *ctx, struct v4l2_format *f)
> +{
> +	struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp;
> +	const struct aml_video_fmt *out_fmt;
> +	int ret;
> +
> +	out_fmt = aml_vdec_get_video_fmt(ctx->dev, pix_mp->pixelformat);
> +	if (out_fmt && out_fmt->type != AML_FMT_DEC)
> +		out_fmt = NULL;
> +	if (!out_fmt) {
> +		out_fmt = &ctx->dec_fmt[AML_FMT_SRC];
> +		dev_dbg(&ctx->dev->plat_dev->dev,
> +			"%s fmt %d not supported, use default\n", __func__,
> +			pix_mp->pixelformat);
> +	}
> +
> +	ret = vdec_try_fmt_mp(ctx, f->type, pix_mp, out_fmt);
> +
> +	return ret;
> +}
> +
> +static int vdec_try_fmt_capture(struct aml_vdec_ctx *ctx, struct v4l2_format *f)
> +{
> +	struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp;
> +	const struct aml_video_fmt *cap_fmt;
> +	int ret;
> +
> +	cap_fmt = aml_vdec_get_video_fmt(ctx->dev, pix_mp->pixelformat);
> +	if (cap_fmt && cap_fmt->type != AML_FMT_FRAME)
> +		cap_fmt = NULL;
> +	if (!cap_fmt) {
> +		cap_fmt = &ctx->dec_fmt[AML_FMT_DST];
> +		dev_dbg(&ctx->dev->plat_dev->dev,
> +			"%s fmt %d not supported, use default\n", __func__,
> +			pix_mp->pixelformat);
> +	}
> +
> +	ret = vdec_try_fmt_mp(ctx, f->type, pix_mp, cap_fmt);
> +
> +	return ret;
> +}
> +
> +static void reset_output_fmts(struct aml_vdec_ctx *ctx)
> +{
> +	struct aml_vdec_dev *dev = ctx->dev;
> +	const struct aml_video_fmt *out_fmt;
> +	struct v4l2_pix_format_mplane fmt;
> +
> +	/* reset default output fmt to V4L2_PIX_FMT_H264_SLICE */
> +	out_fmt = aml_vdec_get_video_fmt(dev, V4L2_PIX_FMT_H264_SLICE);
> +	if (!out_fmt)
> +		return;
> +
> +	ctx->dec_fmt[AML_FMT_SRC] = *out_fmt;
> +
> +	memset(&fmt, 0, sizeof(struct v4l2_pix_format_mplane));
> +
> +	fmt.height = out_fmt->stepwise.min_height;
> +	fmt.width = out_fmt->stepwise.min_width;
> +	/* reset bytesperline to 0 for output fmts */
> +	fmt.plane_fmt[0].bytesperline = 0;
> +	fmt.colorspace = V4L2_COLORSPACE_DEFAULT;
> +	fmt.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
> +	fmt.quantization = V4L2_QUANTIZATION_DEFAULT;
> +	fmt.xfer_func = V4L2_XFER_FUNC_DEFAULT;
> +	vdec_try_fmt_mp(ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, &fmt,
> +			out_fmt);
> +
> +	ctx->pix_fmt[AML_FMT_SRC] = fmt;
> +}
> +
> +static void reset_capture_fmts(struct aml_vdec_ctx *ctx)
> +{
> +	struct aml_vdec_dev *dev = ctx->dev;
> +	const struct aml_video_fmt *cap_fmt;
> +	struct v4l2_pix_format_mplane fmt;
> +
> +	/* reset default output fmt to V4L2_PIX_FMT_NV12 */
> +	cap_fmt = aml_vdec_get_video_fmt(dev, V4L2_PIX_FMT_NV12);
> +	if (!cap_fmt)
> +		return;
> +
> +	ctx->dec_fmt[AML_FMT_DST] = *cap_fmt;
> +
> +	memset(&fmt, 0, sizeof(struct v4l2_pix_format_mplane));
> +
> +	fmt.height = cap_fmt->stepwise.min_height;
> +	fmt.width = cap_fmt->stepwise.min_width;

Don't hardcode this, derive from the coded src_fmt (OUTPUT format). This way you
can properly implement reset on s_fmt(OUTPUT).

> +	fmt.colorspace = V4L2_COLORSPACE_DEFAULT;
> +	fmt.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
> +	fmt.quantization = V4L2_QUANTIZATION_DEFAULT;
> +	fmt.xfer_func = V4L2_XFER_FUNC_DEFAULT;
> +	vdec_try_fmt_mp(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE, &fmt,
> +			cap_fmt);
> +
> +	ctx->pix_fmt[AML_FMT_DST] = fmt;
> +}
> +
> +void aml_vdec_reset_fmts(struct aml_vdec_ctx *ctx)
> +{
> +	ctx->m2m_ctx->q_lock = &ctx->v4l2_intf_lock;
> +	reset_output_fmts(ctx);
> +	reset_capture_fmts(ctx);

Resetting output imply resetting capture, you can embed them.

> +}
> +
> +static int vdec_g_fmt(struct aml_vdec_ctx *ctx, struct v4l2_format *f)
> +{
> +	struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp;
> +
> +	if (V4L2_TYPE_IS_OUTPUT(f->type))
> +		*pix_mp = ctx->pix_fmt[AML_FMT_SRC];
> +	else
> +		*pix_mp = ctx->pix_fmt[AML_FMT_DST];

I personally prefer to not use an array, and have src_fmt and dst_dmt. It will
also make other code path easier to read.

> +
> +	dev_dbg(&ctx->dev->plat_dev->dev,
> +		"%s fmt %d num planes %d\n", __func__, pix_mp->pixelformat,
> +		pix_mp->num_planes);
> +
> +	return 0;
> +}
> +
> +static int vidioc_try_fmt_cap_mplane(struct file *file, void *priv,
> +				     struct v4l2_format *f)
> +{
> +	struct aml_vdec_ctx *ctx = fh_to_dec_ctx(file);
> +
> +	return vdec_try_fmt_capture(ctx, f);
> +}
> +
> +static int vidioc_try_fmt_out_mplane(struct file *file, void *priv,
> +				     struct v4l2_format *f)
> +{
> +	struct aml_vdec_ctx *ctx = fh_to_dec_ctx(file);
> +
> +	return vdec_try_fmt_output(ctx, f);
> +}
> +
> +static int vidioc_vdec_s_fmt_out_mplane(struct file *file, void *priv,
> +					struct v4l2_format *f)
> +{
> +	struct aml_vdec_ctx *ctx = fh_to_dec_ctx(file);
> +
> +	return vdec_s_fmt_output(ctx, f);
> +}
> +
> +static int vidioc_vdec_s_fmt_cap_mplane(struct file *file, void *priv,
> +					struct v4l2_format *f)
> +{
> +	struct aml_vdec_ctx *ctx = fh_to_dec_ctx(file);
> +
> +	return vdec_s_fmt_capture(ctx, f);
> +}
> +
> +static int vidioc_vdec_g_fmt_out_mplane(struct file *file, void *priv,
> +					struct v4l2_format *f)
> +{
> +	struct aml_vdec_ctx *ctx = fh_to_dec_ctx(file);
> +
> +	return vdec_g_fmt(ctx, f);
> +}
> +
> +static int vidioc_vdec_g_fmt_cap_mplane(struct file *file, void *priv,
> +					struct v4l2_format *f)
> +{
> +	struct aml_vdec_ctx *ctx = fh_to_dec_ctx(file);
> +
> +	return vdec_g_fmt(ctx, f);
> +}
> +
> +static int vidioc_vdec_enum_fmt_out_mplane(struct file *file,
> +					   void *priv, struct v4l2_fmtdesc *f)
> +{
> +	struct aml_vdec_ctx *ctx = fh_to_dec_ctx(file);
> +
> +	return vidioc_vdec_enum_fmt(ctx, f, 1);
> +}
> +
> +static int vidioc_vdec_enum_fmt_cap_mplane(struct file *file,
> +					   void *priv, struct v4l2_fmtdesc *f)
> +{
> +	struct aml_vdec_ctx *ctx = fh_to_dec_ctx(file);
> +
> +	return vidioc_vdec_enum_fmt(ctx, f, 0);
> +}
> +
> +const struct v4l2_ioctl_ops aml_vdec_ioctl_ops = {
> +	.vidioc_querycap = vidioc_vdec_querycap,
> +	.vidioc_enum_framesizes = vidioc_vdec_enum_framesizes,
> +
> +	.vidioc_enum_fmt_vid_cap = vidioc_vdec_enum_fmt_cap_mplane,
> +	.vidioc_try_fmt_vid_cap_mplane = vidioc_try_fmt_cap_mplane,
> +	.vidioc_s_fmt_vid_cap_mplane = vidioc_vdec_s_fmt_cap_mplane,
> +	.vidioc_g_fmt_vid_cap_mplane = vidioc_vdec_g_fmt_cap_mplane,
> +
> +	.vidioc_enum_fmt_vid_out = vidioc_vdec_enum_fmt_out_mplane,
> +	.vidioc_try_fmt_vid_out_mplane = vidioc_try_fmt_out_mplane,
> +	.vidioc_s_fmt_vid_out_mplane = vidioc_vdec_s_fmt_out_mplane,
> +	.vidioc_g_fmt_vid_out_mplane = vidioc_vdec_g_fmt_out_mplane,
> +
> +	.vidioc_reqbufs = v4l2_m2m_ioctl_reqbufs,
> +	.vidioc_querybuf = v4l2_m2m_ioctl_querybuf,
> +	.vidioc_qbuf = v4l2_m2m_ioctl_qbuf,
> +	.vidioc_dqbuf = v4l2_m2m_ioctl_dqbuf,
> +	.vidioc_prepare_buf = v4l2_m2m_ioctl_prepare_buf,
> +	.vidioc_create_bufs = v4l2_m2m_ioctl_create_bufs,
> +
> +	.vidioc_expbuf = v4l2_m2m_ioctl_expbuf,
> +
> +	.vidioc_decoder_cmd = v4l2_m2m_ioctl_stateless_decoder_cmd,
> +	.vidioc_try_decoder_cmd = v4l2_m2m_ioctl_stateless_try_decoder_cmd,
> +
> +	.vidioc_subscribe_event = v4l2_ctrl_subscribe_event,
> +	.vidioc_unsubscribe_event = v4l2_event_unsubscribe,
> +
> +	.vidioc_streamon = v4l2_m2m_ioctl_streamon,
> +	.vidioc_streamoff = v4l2_m2m_ioctl_streamoff,
> +};
> +
> +static void aml_vdec_release_instance(struct aml_vdec_ctx *ctx)
> +{
> +	if (ctx->codec_ops && ctx->codec_ops->exit)
> +		ctx->codec_ops->exit(ctx);
> +}
> +
> +static int vb2ops_vdec_queue_setup(struct vb2_queue *vq,
> +				   unsigned int *nbuffers,
> +				   unsigned int *nplanes,
> +				   unsigned int sizes[],
> +				   struct device *alloc_devs[])
> +{
> +	struct aml_vdec_ctx *ctx = vb2_get_drv_priv(vq);
> +	struct v4l2_pix_format_mplane *pix_fmt;
> +	unsigned int i;
> +
> +	if (V4L2_TYPE_IS_OUTPUT(vq->type))
> +		pix_fmt = &ctx->pix_fmt[AML_FMT_SRC];
> +	else
> +		pix_fmt = &ctx->pix_fmt[AML_FMT_DST];
> +
> +	if (*nplanes) {
> +		if (*nplanes != pix_fmt->num_planes)
> +			return -EINVAL;
> +
> +		for (i = 0; i < *nplanes; i++) {
> +			if (sizes[i] < pix_fmt->plane_fmt[i].sizeimage) {
> +				dev_err(&ctx->dev->plat_dev->dev,
> +					"not supported sizeimage\n");
> +				return -EINVAL;
> +			}
> +			alloc_devs[i] = &ctx->dev->plat_dev->dev;
> +		}
> +	} else {
> +		if (vq->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
> +			*nplanes = pix_fmt->num_planes;
> +		else if (vq->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
> +			*nplanes = 1;
> +
> +		for (i = 0; i < *nplanes; i++) {
> +			alloc_devs[i] = &ctx->dev->plat_dev->dev;
> +			sizes[i] = pix_fmt->plane_fmt[i].sizeimage;
> +		}
> +	}
> +
> +	if (*nplanes) {
> +		dev_dbg(&ctx->dev->plat_dev->dev,
> +			"type: %d, plane: %d, buf cnt: %d, size: [Y: %u, C: %u]\n",
> +			vq->type, *nplanes, *nbuffers, sizes[0], sizes[1]);
> +		return 0;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int vb2ops_vdec_buf_prepare(struct vb2_buffer *vb)
> +{
> +	struct aml_vdec_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
> +	struct v4l2_pix_format_mplane *pix_fmt;
> +	unsigned int sizeimage = 0;
> +	int i;
> +
> +	if (V4L2_TYPE_IS_OUTPUT(vb->vb2_queue->type))
> +		pix_fmt = &ctx->pix_fmt[AML_FMT_SRC];
> +	else
> +		pix_fmt = &ctx->pix_fmt[AML_FMT_DST];
> +
> +	for (i = 0; i < pix_fmt->num_planes; i++) {
> +		sizeimage = pix_fmt->plane_fmt[i].sizeimage;
> +		if (vb2_plane_size(vb, i) < sizeimage)
> +			return -EINVAL;
> +
> +		if (V4L2_TYPE_IS_CAPTURE(vb->type)) {
> +			vb2_set_plane_payload(vb, i,
> +					      pix_fmt->plane_fmt[i].sizeimage);
> +			dev_dbg(&ctx->dev->plat_dev->dev,
> +				"%s type: %d set plane: %d, sizeimage: %d\n",
> +				__func__, vb->vb2_queue->type, i,
> +				pix_fmt->plane_fmt[i].sizeimage);
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int vb2_ops_vdec_buf_init(struct vb2_buffer *vb)
> +{
> +	return 0;
> +}
> +
> +static void vb2_ops_vdec_buf_queue(struct vb2_buffer *vb)
> +{
> +	struct aml_vdec_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
> +	struct vb2_v4l2_buffer *vb2_v4l2 = to_vb2_v4l2_buffer(vb);
> +
> +	v4l2_m2m_buf_queue(ctx->fh.m2m_ctx, vb2_v4l2);
> +}
> +
> +static void vb2_ops_vdec_buf_finish(struct vb2_buffer *vb)
> +{
> +}
> +
> +static int vb2ops_vdec_start_streaming(struct vb2_queue *q, unsigned int count)
> +{
> +	struct aml_vdec_ctx *ctx = vb2_get_drv_priv(q);

You seem to be missing:
	v4l2_m2m_update_start_streaming_state(ctx->fh.m2m_ctx, q);

> +
> +	if (V4L2_TYPE_IS_OUTPUT(q->type)) {
> +		struct vb2_v4l2_buffer *src_buf;
> +
> +		if (aml_vdec_init_dec_inst(ctx) < 0) {
> +			while ((src_buf = v4l2_m2m_src_buf_remove(ctx->m2m_ctx)))
> +				v4l2_m2m_buf_done(src_buf, VB2_BUF_STATE_QUEUED);
> +			return -EINVAL;
> +		}
> +		ctx->is_output_streamon = 1;

Are these state really needed ? Looks like vb2_is_streaming() will do the same.

> +	} else {
> +		ctx->is_cap_streamon = 1;
> +	}
> +
> +	return 0;
> +}
> +
> +static void vb2ops_vdec_stop_streaming(struct vb2_queue *q)
> +{
> +	struct aml_vdec_ctx *ctx = vb2_get_drv_priv(q);
> +	struct vb2_v4l2_buffer *src_buf = NULL, *dst_buf = NULL;
> +	
> +	if (V4L2_TYPE_IS_OUTPUT(q->type)) {
> +		while ((src_buf = v4l2_m2m_src_buf_remove(ctx->m2m_ctx)))
> +			v4l2_m2m_buf_done(src_buf, VB2_BUF_STATE_ERROR);

P.s. once your driver is async, you'll also have to call
v4l2_ctrl_request_complete().

> +		ctx->is_output_streamon = 0;

This seems redundant with vb2.

> +	} else {
> +		while ((dst_buf = v4l2_m2m_dst_buf_remove(ctx->m2m_ctx)))
> +			v4l2_m2m_buf_done(dst_buf, VB2_BUF_STATE_ERROR);
> +		ctx->is_cap_streamon = 0;
> +	}
> +
> +	if (!ctx->is_output_streamon && !ctx->is_cap_streamon)
> +		aml_vdec_release_instance(ctx);

You seem to be missing something like this (picked from Hantro as an example):

    20   │       v4l2_m2m_update_stop_streaming_state(ctx->fh.m2m_ctx, q);
    21   │
    22   │       if (V4L2_TYPE_IS_OUTPUT(q->type) &&                    
    23   │           v4l2_m2m_has_stopped(ctx->fh.m2m_ctx))
    24   │       │       v4l2_event_queue_fh(&ctx->fh, &hantro_eos_event);

> +}
> +
> +static int vb2ops_vdec_out_buf_validate(struct vb2_buffer *vb)
> +{
> +	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> +
> +	vbuf->field = V4L2_FIELD_NONE;
> +	return 0;
> +}

Add a fixme comment, or just implement interlace.

> +
> +static void vb2ops_vdec_buf_request_complete(struct vb2_buffer *vb)
> +{
> +	struct aml_vdec_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
> +
> +	v4l2_ctrl_request_complete(vb->req_obj.req, &ctx->ctrl_handler);
> +}
> +
> +static const struct vb2_ops aml_vdec_vb2_ops = {
> +	.queue_setup = vb2ops_vdec_queue_setup,
> +	.start_streaming = vb2ops_vdec_start_streaming,
> +	.stop_streaming = vb2ops_vdec_stop_streaming,
> +
> +	.buf_init = vb2_ops_vdec_buf_init,
> +	.buf_prepare = vb2ops_vdec_buf_prepare,
> +	.buf_out_validate = vb2ops_vdec_out_buf_validate,
> +	.buf_queue = vb2_ops_vdec_buf_queue,
> +	.buf_finish = vb2_ops_vdec_buf_finish,
> +	.buf_request_complete = vb2ops_vdec_buf_request_complete,
> +};
> +
> +int aml_vdec_queue_init(void *priv, struct vb2_queue *src_vq,
> +			struct vb2_queue *dst_vq)
> +{
> +	struct aml_vdec_ctx *ctx = (struct aml_vdec_ctx *)priv;
> +	struct aml_vdec_dev *dev = ctx->dev;
> +	int ret = 0;
> +
> +	src_vq->type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
> +	src_vq->io_modes = VB2_MMAP | VB2_DMABUF;
> +	src_vq->mem_ops = &vb2_dma_contig_memops;
> +	src_vq->drv_priv = ctx;
> +	src_vq->ops = &aml_vdec_vb2_ops;
> +	src_vq->lock = &ctx->v4l2_intf_lock;
> +	src_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
> +	src_vq->supports_requests = true;
> +	src_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> +	ret = vb2_queue_init(src_vq);
> +	if (ret) {
> +		v4l2_info(&dev->v4l2_dev,
> +			  "Failed to initialize videobuf2 queue(output)");
> +		return ret;
> +	}
> +
> +	dst_vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
> +	dst_vq->io_modes = VB2_MMAP | VB2_DMABUF;
> +	dst_vq->drv_priv = ctx;
> +	dst_vq->mem_ops = &vb2_dma_contig_memops;
> +	dst_vq->ops = &aml_vdec_vb2_ops;
> +	dst_vq->lock = &ctx->v4l2_intf_lock;
> +	dst_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
> +	dst_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> +	ret = vb2_queue_init(dst_vq);
> +	if (ret) {
> +		v4l2_info(&dev->v4l2_dev,
> +			  "Failed to initialize videobuf2 queue(capture)");
> +		vb2_queue_release(src_vq);
> +	}
> +
> +	return ret;
> +}
> diff --git a/drivers/media/platform/amlogic/vdec/aml_vdec.h b/drivers/media/platform/amlogic/vdec/aml_vdec.h
> new file mode 100644
> index 000000000000..32f7fa245f7e
> --- /dev/null
> +++ b/drivers/media/platform/amlogic/vdec/aml_vdec.h
> @@ -0,0 +1,33 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR MIT) */
> +/*
> + * Copyright (C) 2025 Amlogic, Inc. All rights reserved
> + */
> +
> +#ifndef _AML_VDEC_H_
> +#define _AML_VDEC_H_
> +
> +#include "aml_vdec_drv.h"
> +
> +/**
> + * struct aml_vdec_v4l2_ctrl - helper type to declare supported ctrls
> + * @codec_type: codec id this control belong to (CODEC_TYPE_H264, etc.)
> + * @cfg: control configuration
> + */
> +struct aml_vdec_v4l2_ctrl {
> +	unsigned int codec_type;
> +	struct v4l2_ctrl_config cfg;
> +};
> +
> +struct aml_dec_type {
> +	unsigned int codec_type;
> +	const char *name;
> +};
> +
> +extern const struct v4l2_m2m_ops aml_vdec_m2m_ops;
> +extern const struct v4l2_ioctl_ops aml_vdec_ioctl_ops;
> +
> +int aml_vdec_ctrls_setup(struct aml_vdec_ctx *ctx);
> +int aml_vdec_queue_init(void *priv, struct vb2_queue *src_vq,
> +			struct vb2_queue *dst_vq);
> +void aml_vdec_reset_fmts(struct aml_vdec_ctx *ctx);
> +#endif
> diff --git a/drivers/media/platform/amlogic/vdec/aml_vdec_drv.c b/drivers/media/platform/amlogic/vdec/aml_vdec_drv.c
> new file mode 100644
> index 000000000000..1870d56659cf
> --- /dev/null
> +++ b/drivers/media/platform/amlogic/vdec/aml_vdec_drv.c
> @@ -0,0 +1,260 @@
> +// SPDX-License-Identifier: (GPL-2.0-only OR MIT)
> +/*
> + * Copyright (C) 2025 Amlogic, Inc. All rights reserved
> + */
> +
> +#include <linux/dma-mapping.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-ioctl.h>
> +
> +#include "aml_vdec.h"
> +#include "aml_vdec_hw.h"
> +#include "aml_vdec_platform.h"
> +
> +#define AML_VDEC_DRV_NAME "aml-vdec-drv"
> +
> +static int fops_vcodec_open(struct file *file)
> +{
> +	struct aml_vdec_dev *dec_dev = video_drvdata(file);
> +	struct aml_vdec_ctx *ctx = NULL;
> +	int ret = 0;
> +
> +	ctx = kzalloc_obj(*ctx, GFP_KERNEL);
> +	if (!ctx)
> +		return -ENOMEM;
> +
> +	mutex_lock(&dec_dev->dev_mutex);

Why?

> +	ctx->dev = dec_dev;
> +	v4l2_fh_init(&ctx->fh, video_devdata(file));

hmm.
	v4l2_fh_init(&ctx->fh, dec_dev);

> +	file->private_data = &ctx->fh;

Why ?

> +	v4l2_fh_add(&ctx->fh, file);
> +	mutex_init(&ctx->v4l2_intf_lock);
> +	init_waitqueue_head(&ctx->queue);

I believe this goes away once device_run() is asynchronous.

> +	ctx->int_cond = 0;
> +
> +	ctx->m2m_ctx = v4l2_m2m_ctx_init(dec_dev->m2m_dev_dec, ctx,
> +					 &aml_vdec_queue_init);
> +	if (IS_ERR(ctx->m2m_ctx)) {
> +		ret = PTR_ERR((__force void *)ctx->m2m_ctx);
> +		v4l2_err(&dec_dev->v4l2_dev, "Failed to v4l2_m2m_ctx_init() (%d)", ret);
> +		goto err_m2m_ctx_init;
> +	}
> +
> +	ctx->fh.m2m_ctx = ctx->m2m_ctx;
> +	ret = aml_vdec_ctrls_setup(ctx);
> +	if (ret) {
> +		v4l2_err(&dec_dev->v4l2_dev, "Failed to init all ctrls (%d)", ret);
> +		goto err_ctrls_setup;
> +	}
> +
> +	aml_vdec_reset_fmts(ctx);
> +	mutex_unlock(&dec_dev->dev_mutex);

Everything in this function is modifying ctx only, dev_dec have to have a
lifetime guarantied by the caller (holding on dev and its private data until the
open call have complete). I don't see a reason, from a driver point of view why
we'd need that mutex here. If there is a real issue here, it would need fixing
elsewhere.

> +
> +	return ret;
> +
> +err_ctrls_setup:
> +	v4l2_m2m_ctx_release(ctx->m2m_ctx);
> +err_m2m_ctx_init:
> +	v4l2_fh_del(&ctx->fh, file);
> +	v4l2_fh_exit(&ctx->fh);
> +	kfree(ctx);
> +	mutex_unlock(&dec_dev->dev_mutex);
> +
> +	return ret;
> +}
> +
> +static int fops_vcodec_release(struct file *file)
> +{
> +	struct aml_vdec_ctx *ctx = fh_to_dec_ctx(file);
> +
> +	v4l2_m2m_ctx_release(ctx->m2m_ctx);
> +	v4l2_ctrl_handler_free(&ctx->ctrl_handler);
> +	v4l2_fh_del(&ctx->fh, file);
> +	v4l2_fh_exit(&ctx->fh);
> +	kfree(ctx);
> +
> +	return 0;
> +}
> +
> +static const struct v4l2_file_operations aml_vdec_fops = {
> +	.owner        = THIS_MODULE,
> +	.open        = fops_vcodec_open,
> +	.release    = fops_vcodec_release,
> +	.poll        = v4l2_m2m_fop_poll,
> +	.unlocked_ioctl    = video_ioctl2,
> +	.mmap        = v4l2_m2m_fop_mmap,
> +};
> +
> +static const struct video_device dec_dev = {
> +	.name = "aml_dev_drv",
> +	.fops = &aml_vdec_fops,
> +	.ioctl_ops = &aml_vdec_ioctl_ops,
> +	.release = video_device_release,
> +	.vfl_dir = VFL_DIR_M2M,
> +	.device_caps = V4L2_CAP_VIDEO_M2M_MPLANE | V4L2_CAP_STREAMING,
> +};
> +
> +static const struct media_device_ops aml_m2m_media_ops = {
> +	.req_validate = vb2_request_validate,
> +	.req_queue = v4l2_m2m_request_queue,
> +};
> +
> +static int aml_vdec_drv_probe(struct platform_device *pdev)
> +{
> +	struct aml_vdec_dev *dev;
> +	struct video_device *vfd_dec;
> +	struct aml_vdec_hw *hw;
> +	int ret = 0;
> +
> +	dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL);
> +	if (!dev)
> +		return -ENOMEM;
> +
> +	dev->plat_dev = pdev;
> +	mutex_init(&dev->dev_mutex);
> +
> +	/*
> +	 * The decoder hardware registers take 32-bit addresses only;
> +	 * make sure DMA allocations (bitstream buffers, frame buffers
> +	 * and firmware workspaces) stay below 4GB.
> +	 */
> +	ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
> +	if (ret)
> +		return dev_err_probe(&pdev->dev, ret,
> +				     "Failed to set 32-bit DMA mask\n");
> +
> +	ret = v4l2_device_register(&pdev->dev, &dev->v4l2_dev);
> +	if (ret)
> +		return dev_err_probe(&pdev->dev, ret, "v4l2_device_register err\n");
> +
> +	vfd_dec = video_device_alloc();
> +	if (!vfd_dec) {
> +		v4l2_err(&dev->v4l2_dev, "Failed to allocate video device\n");
> +		ret = -ENOMEM;
> +		goto err_device_alloc;
> +	}
> +	*vfd_dec = dec_dev;
> +	vfd_dec->v4l2_dev = &dev->v4l2_dev;
> +	vfd_dec->lock = &dev->dev_mutex;
> +	video_set_drvdata(vfd_dec, dev);
> +	dev->vfd = vfd_dec;
> +	platform_set_drvdata(pdev, dev);
> +
> +	hw = devm_kzalloc(&pdev->dev, sizeof(*hw), GFP_KERNEL);
> +	if (!hw) {
> +		ret = -ENOMEM;
> +		goto err_dec_mem_init;
> +	}
> +	dev->dec_hw = hw;
> +
> +	dev->pvdec_data = of_device_get_match_data(&pdev->dev);
> +	if (!dev->pvdec_data) {
> +		dev_err(&pdev->dev, "no match data for this device\n");
> +		ret = -ENODEV;
> +		goto err_hw_init;
> +	}
> +
> +	ret = dev->pvdec_data->req_hw_resource(dev);
> +	if (ret < 0)
> +		goto err_hw_init;
> +
> +	dev->m2m_dev_dec = v4l2_m2m_init(&aml_vdec_m2m_ops);
> +	if (IS_ERR(dev->m2m_dev_dec)) {
> +		v4l2_err(&dev->v4l2_dev, "Failed to init mem2mem dec device\n");
> +		ret = PTR_ERR((__force void *)dev->m2m_dev_dec);
> +		goto err_m2m_dev_init;
> +	}
> +
> +	ret = video_register_device(vfd_dec, VFL_TYPE_VIDEO, -1);
> +	if (ret) {
> +		v4l2_err(&dev->v4l2_dev, "Failed to register video device");
> +		goto err_vid_dev_register;
> +	}
> +
> +	dev->mdev.dev = &pdev->dev;
> +	strscpy(dev->mdev.model, AML_VDEC_DRV_NAME, sizeof(dev->mdev.model));
> +	media_device_init(&dev->mdev);
> +	dev->mdev.ops = &aml_m2m_media_ops;
> +	dev->v4l2_dev.mdev = &dev->mdev;
> +
> +	ret = v4l2_m2m_register_media_controller(dev->m2m_dev_dec, vfd_dec,
> +						 MEDIA_ENT_F_PROC_VIDEO_DECODER);
> +	if (ret) {
> +		v4l2_err(&dev->v4l2_dev, "Failed to init mem2mem media controller\n");
> +		goto error_m2m_mc_register;
> +	}
> +
> +	ret = media_device_register(&dev->mdev);
> +	if (ret) {
> +		v4l2_err(&dev->v4l2_dev, "Failed to register media device");
> +		goto err_media_dev_register;
> +	}
> +	vdec_enable(dev->dec_hw);
> +	return 0;
> +
> +err_media_dev_register:
> +	v4l2_m2m_unregister_media_controller(dev->m2m_dev_dec);
> +error_m2m_mc_register:
> +	media_device_cleanup(&dev->mdev);
> +	video_unregister_device(dev->vfd);
> +	/* video_unregister_device() has released the video_device,
> +	 * set the vfd_dec to NULL to avoid double free in err_dec_mem_init
> +	 */
> +	vfd_dec = NULL;
> +err_vid_dev_register:
> +	v4l2_m2m_release(dev->m2m_dev_dec);
> +err_m2m_dev_init:
> +	dev->pvdec_data->destroy_hw_resource(dev);
> +err_hw_init:
> +	dev->dec_hw = NULL;
> +err_dec_mem_init:
> +	/* In case of double-freeing the video_device from error_m2m_mc_register */
> +	if (vfd_dec)
> +		video_device_release(vfd_dec);
> +err_device_alloc:
> +	v4l2_device_unregister(&dev->v4l2_dev);
> +	return ret;
> +}
> +
> +static void aml_vdec_drv_remove(struct platform_device *pdev)
> +{
> +	struct aml_vdec_dev *dev = platform_get_drvdata(pdev);
> +
> +	/*
> +	 * The IRQ is devm-managed and only released after remove()
> +	 * returns: mask it and wait for any in-flight threaded handler
> +	 * before powering off the VDEC domain, otherwise the handler
> +	 * could access powered-down registers.
> +	 */
> +	disable_irq(dev->dec_hw->dec_irq);
> +	vdec_disable(dev->dec_hw);
> +	media_device_unregister(&dev->mdev);
> +	v4l2_m2m_unregister_media_controller(dev->m2m_dev_dec);
> +	video_unregister_device(dev->vfd);
> +	media_device_cleanup(&dev->mdev);
> +
> +	v4l2_m2m_release(dev->m2m_dev_dec);
> +	dev->pvdec_data->destroy_hw_resource(dev);
> +	v4l2_device_unregister(&dev->v4l2_dev);
> +}
> +
> +static const struct of_device_id aml_vdec_match[] = {
> +	{ .compatible = "amlogic,s4-vdec", .data = &aml_vdec_s4_pdata },
> +	{ }
> +};
> +
> +static struct platform_driver aml_vcodec_dec_driver = {
> +	.probe    = aml_vdec_drv_probe,
> +	.remove   = aml_vdec_drv_remove,
> +	.driver   = {
> +		.name    = AML_VDEC_DRV_NAME,
> +		.of_match_table = aml_vdec_match,
> +	},
> +};

Breaking my review here. Note on the best way to be reviewer friendly is to
split the low level driver helpers in its own patch. This way we can sort out
the HW access and give you an RB on it, and focus on the v4l2 part. It makes
respinning patch set and reviewing revision a lot faster. An example of patchset
for you could have been:

1. Core loadable driver, resource allocation, IRQ registration, PM, etc.
2. Low level common helpers (non codec specifics)
3. V4L2 implementation and codec specific ops interface
4. H.264 implementation

Patches must build, it does not have to fully work until the full set is
applied. But it is very good habit to use the split to cover error handling
testing.

You may notice not getting a lot of human review, you spun a lot of revision,
and each time you endup at the bottom of the maintainer human review queue, as
you patch is seen like in active development. I think its explain how you ended
in v12 so quickly with so much work left to do.

I have a lot of comments coming on the H.264 implementation, including bug
report I have received from individual. I will come back tomorrow with these.
Meanwhile I wanted to highlight this big one, which imply a large amount of
unrorted conformance failure (you forget about JVT-FR-EXT existance it seems).

> +
> +	ctrls->scaling =
> +		(struct v4l2_ctrl_h264_scaling_matrix *)aml_h264_get_ctrl(&ctx->ctrl_handler,
> +			V4L2_CID_STATELESS_H264_SCALING_MATRIX);
> +	if (WARN_ON(!ctrls->scaling))
> +		return -EINVAL;

This is being set but not used. Its not covered by JVT-AVC_V1, but JVT-FR-EXT
do. The following test (420 only) are likely failing for you due to this bug.
Please address in v13.

      brcm_freh3
      brcm_freh4
      brcm_freh5
      brcm_freh6
      brcm_freh8
      brcm_freh9
      brcm_freh10
      brcm_freh11
      freh1_b
      freh2_b
      freh7_b
      freh12_b
      FREXT01_JVC_D
      FREXT02_JVC_C
      FRExt1_Panasonic_D
      FRExt2_Panasonic_C
      FRExt3_Panasonic_E
      FRExt4_Panasonic_B
      FRExt_MMCO4_Sony_B
      HCAFF1_HHI_B
      HCAFR1_HHI_C
      HCAFR2_HHI_A
      HCAFR3_HHI_A
      HCAFR4_HHI_A
      HCAMFF1_HHI_B
      HPCADQ_BRCM_B
      HPCALQ_BRCM_B
      HPCAMAPALQ_BRCM_B
      HPCAMOLQ_BRCM_B
      HPCAQ2LQ_BRCM_B
      HPCVMOLQ_BRCM_B
      HVLCFI0_Sony_B
      HVLCMFF0_Sony_B
      HVLCPFF0_Sony_B

I would also like to mention that I notice the presence of per-slice reference
list calculation. This tells me you might have implemented the wrong decoding
mode. All this driver code is not required if you implement:

- V4L2_CID_STATELESS_H264_SLICE_PARAMS
- V4L2_STATELESS_H264_DECODE_MODE_SLICE_BASED

As an alternative, we have that for HEVC but not yet for H264, is to require
V4L2_CID_STATELESS_H264_SLICE_PARAMS in MODE_FRAME_BASED, though you'll endup
with a bit more friction with existing userspace implementation. For that
reason, I would not recommend, and suggest to follow the cedrus driver.

Nicolas

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]