media: rockchip: Add the rkvdec driver

Hello,

This is v4 of Boris' rkvdec driver.

I've addressed the comments received during v3, all of them quite minor. Please check each patch changelog for more details.

Thanks,
Ezequiel

Boris Brezillon (5):
media: v4l2-core: Add helpers to build the H264 P/B0/B1 reflists
media: hantro: h264: Use the generic H264 reflist builder
media: dt-bindings: rockchip: Document RK3399 Video Decoder bindings
media: rkvdec: Add the rkvdec driver
arm64: dts: rockchip: rk3399: Define the rockchip Video Decoder node

Jonas Karlman (1):
media: uapi: h264: Add DPB entry field reference flags

 .../bindings/media/rockchip,vdec.yaml         |   71 +
 .../media/uapi/v4l/ext-ctrls-codec.rst        |   16 +
 arch/arm64/boot/dts/rockchip/rk3399.dtsi      |   14 +-
 drivers/media/v4l2-core/Kconfig               |    4 +
 drivers/media/v4l2-core/Makefile              |    1 +
 drivers/media/v4l2-core/v4l2-h264.c           |  258 ++++
 drivers/staging/media/Kconfig                 |    2 +
 drivers/staging/media/Makefile                |    1 +
 drivers/staging/media/hantro/Kconfig          |    1 +
 drivers/staging/media/hantro/hantro_h264.c    |  237 +---
 drivers/staging/media/rkvdec/Kconfig          |   15 +
 drivers/staging/media/rkvdec/Makefile         |    3 +
 drivers/staging/media/rkvdec/rkvdec-h264.c    | 1154 +++++++++++++++++
 drivers/staging/media/rkvdec/rkvdec-regs.h    |  239 ++++
 drivers/staging/media/rkvdec/rkvdec.c         | 1134 ++++++++++++++++
 drivers/staging/media/rkvdec/rkvdec.h         |  124 ++
 include/media/h264-ctrls.h                    |    2 +
 include/media/v4l2-h264.h                     |   86 ++
 18 files changed, 3132 insertions(+), 230 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/media/rockchip,vdec.yaml
 create mode 100644 drivers/media/v4l2-core/v4l2-h264.c
 create mode 100644 drivers/staging/media/rkvdec/Kconfig
 create mode 100644 drivers/staging/media/rkvdec/Makefile
 create mode 100644 drivers/staging/media/rkvdec/rkvdec-h264.c
 create mode 100644 drivers/staging/media/rkvdec/rkvdec-regs.h
 create mode 100644 drivers/staging/media/rkvdec/rkvdec.c
 create mode 100644 drivers/staging/media/rkvdec/rkvdec.h
 create mode 100644 include/media/v4l2-h264.h

2.25.0

From: Jonas Karlman <[email protected]>

Using the field information attached to v4l2 buffers is not enough to determine the type of field referenced by a DPB entry: the decoded frame might contain the full picture (both top and bottom fields) but the reference only point to one of them.
Let's add new V4L2_H264_DPB_ENTRY_FLAG_ flags to express that.

---
v4:
* None
v3:
* This patch was previously part of https://lkml.org/lkml/2019/10/28/956
* Kept the FIELD and BOTTOM_FIELD flags
* Updated the doc with more details

---
 Documentation/media/uapi/v4l/ext-ctrls-codec.rst | 16 ++++++++++++++++
 include/media/h264-ctrls.h                       |  2 ++
 2 files changed, 18 insertions(+)

index 28313c0f4e7c..d4fc5f25aa14 100644
@@ -2028,6 +2028,22 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type -
     * - ``V4L2_H264_DPB_ENTRY_FLAG_LONG_TERM``
       - 0x00000004
       - The DPB entry is a long term reference frame
+    * - ``V4L2_H264_DPB_ENTRY_FLAG_FIELD``
+      - 0x00000008
+      - The DPB entry is a field reference, which means only one of the field
+        will be used when decoding the new frame/field. When not set the DPB
+        entry is a frame reference (both fields will be used). Note that this
+        flag does not say anything about the number of fields contained in the
+        reference frame, it just describes the one used to decode the new
+        field/frame
+    * - ``V4L2_H264_DPB_ENTRY_FLAG_BOTTOM_FIELD``
+      - 0x00000010
+      - The DPB entry is a bottom field reference (only the bottom field of the
+        reference frame is needed to decode the new frame/field). Only valid if
+        V4L2_H264_DPB_ENTRY_FLAG_FIELD is set. When
+        V4L2_H264_DPB_ENTRY_FLAG_FIELD is set but
+        V4L2_H264_DPB_ENTRY_FLAG_BOTTOM_FIELD is not, that means the
+        DPB entry is a top field reference
 
 ``V4L2_CID_MPEG_VIDEO_H264_DECODE_MODE (enum)``
     Specifies the decoding mode to use. Currently exposes slice-based and

index e877bf1d537c..1c6ff7d63bca 100644
@@ -185,6 +185,8 @@ struct v4l2_ctrl_h264_slice_params {
 #define V4L2_H264_DPB_ENTRY_FLAG_VALID		0x01
 #define V4L2_H264_DPB_ENTRY_FLAG_ACTIVE		0x02
 #define V4L2_H264_DPB_ENTRY_FLAG_LONG_TERM	0x04
+#define V4L2_H264_DPB_ENTRY_FLAG_FIELD		0x08
+#define V4L2_H264_DPB_ENTRY_FLAG_BOTTOM_FIELD	0x10
 
 struct v4l2_h264_dpb_entry {
 	__u64 reference_ts;

2.25.0

From: Boris Brezillon <boris.brezillon-ZGY8ohtN/[email protected]>

Building those list is a standard procedure described in section '8.2.4 Decoding process for reference picture lists construction' of the H264 specification.

We already have 2 drivers needing the same logic (hantro and rkvdec) and I suspect we will soon have more.

Let's provide generic helpers to create those lists.

---
v4:
* None
v3:
* New patch

---
 drivers/media/v4l2-core/Kconfig     |   4 +
 drivers/media/v4l2-core/Makefile    |   1 +
 drivers/media/v4l2-core/v4l2-h264.c | 258 ++++++++++++++++++++++++++++
 include/media/v4l2-h264.h           |  86 ++++++++++
 4 files changed, 349 insertions(+)
 create mode 100644 drivers/media/v4l2-core/v4l2-h264.c
 create mode 100644 include/media/v4l2-h264.h

index 39e3fb30ba0b..8a4ccfbca8cf 100644
@@ -45,6 +45,10 @@ config VIDEO_PCI_SKELETON
 config VIDEO_TUNER
 	tristate
 
+# Used by drivers that need v4l2-h264.ko
+config V4L2_H264
+	tristate
+
 # Used by drivers that need v4l2-mem2mem.ko
 config V4L2_MEM2MEM_DEV
 	tristate

index 786bd1ec4d1b..c5c53e0941ad 100644
@@ -21,6 +21,7 @@ obj-$(CONFIG_VIDEO_V4L2) += v4l2-dv-timings.o
 obj-$(CONFIG_VIDEO_TUNER) += tuner.o
 
 obj-$(CONFIG_V4L2_MEM2MEM_DEV) += v4l2-mem2mem.o
+obj-$(CONFIG_V4L2_H264) += v4l2-h264.o
 
 obj-$(CONFIG_V4L2_FLASH_LED_CLASS) += v4l2-flash-led-class.o
 

2.25.0

From: Boris Brezillon <[email protected]>

Now that the core provides generic reflist builders, we can use them instead of implementing our own.

--
v4:
* Select V4L2_H264 as pointed out by Hugh.
v3:
* New patch

---
 drivers/staging/media/hantro/Kconfig       |   1 +
 drivers/staging/media/hantro/hantro_h264.c | 237 +--------------------
 2 files changed, 9 insertions(+), 229 deletions(-)

index de77fe6554e7..33482910dee2 100644
@@ -7,6 +7,7 @@ config VIDEO_HANTRO
 	select VIDEOBUF2_DMA_CONTIG
 	select VIDEOBUF2_VMALLOC
 	select V4L2_MEM2MEM_DEV
+	select V4L2_H264
 	help
 	  Support for the Hantro IP based Video Processing Unit present on
 	  Rockchip SoC, which accelerates video and image encoding and

2.25.0

From: Boris Brezillon <boris.brezillon-ZGY8ohtN/[email protected]>

Document the Rockchip RK3399 Video Decoder bindings.

---
v4:
* Change clock names as suggested by Laurent.
v3:
* Fix dtbs_check failures

---
 .../bindings/media/rockchip,vdec.yaml         | 71 +++++++++++++++++++
 1 file changed, 71 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/rockchip,vdec.yaml

new file mode 100644
index 000000000000..eba7ead72f48
@@ -0,0 +1,71 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/rockchip,vdec.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Rockchip Video Decoder (VDec) Device Tree Bindings
+
+maintainers:
+  - Heiko Stuebner <[email protected]>
+
+description: |-
+  The Rockchip rk3399 has a stateless Video Decoder that can decodes H.264,
+  HEVC an VP9 streams.
+
+properties:
+  compatible:
+    const: rockchip,rk3399-vdec
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: The Video Decoder AXI interface clock
+      - description: The Video Decoder AHB interface clock
+      - description: The Video Decoded CABAC clock
+      - description: The Video Decoder core clock
+
+  clock-names:
+    items:
+      - const: axi
+      - const: ahb
+      - const: cabac
+      - const: core
+
+  power-domains:
+    maxItems: 1
+
+  iommus:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+  - power-domains
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    vdec: video-codec@ff660000 {
+        compatible = "rockchip,rk3399-vdec";
+        reg = <0x0 0xff660000 0x0 0x400>;
+        interrupts = <GIC_SPI 116 IRQ_TYPE_LEVEL_HIGH 0>;
+        clocks = <&cru ACLK_VDU>, <&cru HCLK_VDU>,
+                 <&cru SCLK_VDU_CA>, <&cru SCLK_VDU_CORE>;
+        clock-names = "aclk", "iface", "cabac", "core";
+        power-domains = <&power RK3399_PD_VDU>;
+        iommus = <&vdec_mmu>;
+    };
+
+...

2.25.0

From: Boris Brezillon <[email protected]>

The rockchip vdec block is a stateless decoder that's able to decode H264, HEVC and VP9 content. This commit adds the core infrastructure and the H264 backend. Support for VP9 and HEVS will be added later on.

---
v4:
* Make poc tables const, as suggested by Hans
* Move buffer extra space calculation TRY_FMT, as done on Hantro.
* Get rid of vb2_request_get_buf and open-code
following Cedrus. We can move it to a helper as a follow-up patch.
* Correct clocks.
* Add a minor comment about the order to xxx_buf_done. v3:
* Move the driver to drivers/staging/media/rkvdec/
* Fix copy&paste error in the Kconfig desc
* Use REC709 color space when resetting the capture format
* Prepare things to support VP9
* Move H264 priv-table field definition out of rkvdec-regs.h
* Clarify the provenance of the CABAC table
* Ease RPS,PPS_FIELD() definition/manipulation
* Take DPB field flags into account
* Use the generic H264 helpers to build the reflists

---
 drivers/staging/media/Kconfig              |    2 +
 drivers/staging/media/Makefile             |    1 +
 drivers/staging/media/rkvdec/Kconfig       |   15 +
 drivers/staging/media/rkvdec/Makefile      |    3 +
 drivers/staging/media/rkvdec/rkvdec-h264.c | 1154 ++++++++++++++++++++
 drivers/staging/media/rkvdec/rkvdec-regs.h |  239 ++++
 drivers/staging/media/rkvdec/rkvdec.c      | 1134 +++++++++++++++++++
 drivers/staging/media/rkvdec/rkvdec.h      |  124 +++
 8 files changed, 2672 insertions(+)
 create mode 100644 drivers/staging/media/rkvdec/Kconfig
 create mode 100644 drivers/staging/media/rkvdec/Makefile
 create mode 100644 drivers/staging/media/rkvdec/rkvdec-h264.c
 create mode 100644 drivers/staging/media/rkvdec/rkvdec-regs.h
 create mode 100644 drivers/staging/media/rkvdec/rkvdec.c
 create mode 100644 drivers/staging/media/rkvdec/rkvdec.h

index c394abffea86..bf70de84f5cb 100644
@@ -30,6 +30,8 @@ source "drivers/staging/media/meson/vdec/Kconfig"
 
 source "drivers/staging/media/omap4iss/Kconfig"
 
+source "drivers/staging/media/rkvdec/Kconfig"
+
 source "drivers/staging/media/sunxi/Kconfig"
 
 source "drivers/staging/media/tegra-vde/Kconfig"

index ea9fce8014bb..7169dffe3fca 100644
@@ -3,6 +3,7 @@ obj-$(CONFIG_VIDEO_ALLEGRO_DVT)	+= allegro-dvt/
 obj-$(CONFIG_VIDEO_IMX_MEDIA)	+= imx/
 obj-$(CONFIG_VIDEO_MESON_VDEC)	+= meson/vdec/
 obj-$(CONFIG_VIDEO_OMAP4)	+= omap4iss/
+obj-$(CONFIG_VIDEO_ROCKCHIP_VDEC)	+= rkvdec/
 obj-$(CONFIG_VIDEO_SUNXI)	+= sunxi/
 obj-$(CONFIG_TEGRA_VDE)		+= tegra-vde/
 obj-$(CONFIG_VIDEO_HANTRO)	+= hantro/

new file mode 100644
index 000000000000..a22756deded7
@@ -0,0 +1,15 @@
+# SPDX-License-Identifier: GPL-2.0
+config VIDEO_ROCKCHIP_VDEC
+	tristate "Rockchip Video Decoder driver"
+	depends on ARCH_ROCKCHIP || COMPILE_TEST
+	depends on VIDEO_DEV && VIDEO_V4L2 && MEDIA_CONTROLLER
+	depends on MEDIA_CONTROLLER_REQUEST_API
+	select VIDEOBUF2_DMA_CONTIG
+	select VIDEOBUF2_VMALLOC
+	select V4L2_MEM2MEM_DEV
+	select V4L2_H264
+	help
+	  Support for the Rockchip Video Decoder IP present on Rockchip SoCs,
+	  which accelerates video decoding.
+	  To compile this driver as a module, choose M here: the module
+	  will be called rockchip-vdec.

new file mode 100644
index 000000000000..c08fed0a39f9
@@ -0,0 +1,3 @@
+obj-$(CONFIG_VIDEO_ROCKCHIP_VDEC) += rockchip-vdec.o
+
+rockchip-vdec-y += rkvdec.o rkvdec-h264.o

2.25.0

Well, we know what that means in practice :-).

Well, not necesarily. I think we can expect a helper soon, but it doesn't have to be part of this series :-)

And perhaps we can find another way to link a media request to a vb2 queue, so we don't have to iterate over objects like we are doing now?

Right.

Oh, I totally missed this.

Thanks for reviewing!
Ezequiel

^HEVC

I'd be happy to be proven wrong ;-).

From: Boris Brezillon <boris.brezillon-ZGY8ohtN/[email protected]>

RK3399 has a Video decoder, define the node in the dtsi. We also add the missing power-domain in mmu node and enable the block.

---
v4:
* Correct clock names.
v3:
* None.

---
 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

index e62ea0e2b657..ca2433ffcf55 100644
@@ -1277,8 +1277,20 @@ vdec_mmu: iommu@ff660480 {
 		interrupt-names = "vdec_mmu";
 		clocks = <&cru ACLK_VDU>, <&cru HCLK_VDU>;
 		clock-names = "aclk", "iface";
+		power-domains = <&power RK3399_PD_VDU>;
 		#iommu-cells = <0>;
-		status = "disabled";
+	};
+
+	vdec: video-codec@ff660000 {
+		compatible = "rockchip,rk3399-vdec";
+		reg = <0x0 0xff660000 0x0 0x400>;
+		interrupts = <GIC_SPI 116 IRQ_TYPE_LEVEL_HIGH 0>;
+		interrupt-names = "vdpu";
+		clocks = <&cru ACLK_VDU>, <&cru HCLK_VDU>,
+			 <&cru SCLK_VDU_CA>, <&cru SCLK_VDU_CORE>;
+		clock-names = "axi", "ahb", "cabac", "core";
+		power-domains = <&power RK3399_PD_VDU>;
+		iommus = <&vdec_mmu>;
 	};
 
 	iep_mmu: iommu@ff670800 {

2.25.0