[PATCH v5 0/5] Add support for video hardware codec of STMicroelectronics STM32 SoC series

[PATCH v5 0/5] Add support for video hardware codec of STMicroelectronics STM32 SoC series

[Hugues Fruchet]

This patchset introduces support for VDEC video hardware decoder
and VENC video hardware encoder of STMicroelectronics STM32MP25
SoC series.

This initial support implements H264 decoding, VP8 decoding and
JPEG encoding.

This has been tested on STM32MP257F-EV1 evaluation board.

===========
= history =
===========
version 5:
   - Precise that video decoding as been successfully tested up to full HD
   - Add Nicolas Dufresne reviewed-by

version 4:
   - Fix comments from Nicolas about dropping encoder raw steps

version 3:
   - Fix remarks from Krzysztof Kozlowski:
    - drop "items", we keep simple enum in such case
    - drop second example - it is the same as the first
   - Drop unused node labels as suggested by Conor Dooley
   - Revisit min/max resolutions as suggested by Nicolas Dufresne

version 2:
   - Fix remarks from Krzysztof Kozlowski on v1:
    - single video-codec binding for both VDEC/VENC
    - get rid of "-names"
    - use of generic node name "video-codec"

version 1:
  - Initial submission

Hugues Fruchet (5):
  dt-bindings: media: Document STM32MP25 VDEC & VENC video codecs
  media: hantro: add support for STM32MP25 VDEC
  media: hantro: add support for STM32MP25 VENC
  arm64: dts: st: add video decoder support to stm32mp255
  arm64: dts: st: add video encoder support to stm32mp255

 .../media/st,stm32mp25-video-codec.yaml       |  50 ++++++++
 arch/arm64/boot/dts/st/stm32mp251.dtsi        |  12 ++
 arch/arm64/boot/dts/st/stm32mp255.dtsi        |  17 +++
 drivers/media/platform/verisilicon/Kconfig    |  14 ++-
 drivers/media/platform/verisilicon/Makefile   |   4 +
 .../media/platform/verisilicon/hantro_drv.c   |   4 +
 .../media/platform/verisilicon/hantro_hw.h    |   2 +
 .../platform/verisilicon/stm32mp25_vdec_hw.c  |  92 ++++++++++++++
 .../platform/verisilicon/stm32mp25_venc_hw.c  | 115 ++++++++++++++++++
 9 files changed, 307 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/media/st,stm32mp25-video-codec.yaml
 create mode 100644 drivers/media/platform/verisilicon/stm32mp25_vdec_hw.c
 create mode 100644 drivers/media/platform/verisilicon/stm32mp25_venc_hw.c

-- 
2.25.1

[PATCH v5 1/5] dt-bindings: media: Document STM32MP25 VDEC & VENC video codecs

[Hugues Fruchet]

Add STM32MP25 VDEC video decoder & VENC video encoder bindings.

Signed-off-by: Hugues Fruchet <hugues.fruchet@foss.st.com>
---
 .../media/st,stm32mp25-video-codec.yaml       | 50 +++++++++++++++++++
 1 file changed, 50 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/st,stm32mp25-video-codec.yaml

diff --git a/Documentation/devicetree/bindings/media/st,stm32mp25-video-codec.yaml b/Documentation/devicetree/bindings/media/st,stm32mp25-video-codec.yaml
new file mode 100644
index 000000000000..e167e3b1bec3
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/st,stm32mp25-video-codec.yaml
@@ -0,0 +1,50 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/st,stm32mp25-video-codec.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: STMicroelectronics STM32MP25 VDEC video decoder & VENC video encoder
+
+maintainers:
+  - Hugues Fruchet <hugues.fruchet@foss.st.com>
+
+description:
+  The STMicroelectronics STM32MP25 SOCs embeds a VDEC video hardware
+  decoder peripheral based on Verisilicon VC8000NanoD IP (former Hantro G1)
+  and a VENC video hardware encoder peripheral based on Verisilicon
+  VC8000NanoE IP (former Hantro H1).
+
+properties:
+  compatible:
+    enum:
+      - st,stm32mp25-vdec
+      - st,stm32mp25-venc
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    video-codec@580d0000 {
+        compatible = "st,stm32mp25-vdec";
+        reg = <0x580d0000 0x3c8>;
+        interrupts = <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>;
+        clocks = <&ck_icn_p_vdec>;
+    };
-- 
2.25.1

[PATCH v5 2/5] media: hantro: add support for STM32MP25 VDEC

[Hugues Fruchet]

Add support for STM32MP25 VDEC video hardware decoder.
Support of H264/VP8 decoding.
No post-processor support.
VDEC has its own reset/clock/irq.

Sucessfully tested up to full HD.

Signed-off-by: Hugues Fruchet <hugues.fruchet@foss.st.com>
Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
---
 drivers/media/platform/verisilicon/Kconfig    | 14 ++-
 drivers/media/platform/verisilicon/Makefile   |  3 +
 .../media/platform/verisilicon/hantro_drv.c   |  3 +
 .../media/platform/verisilicon/hantro_hw.h    |  1 +
 .../platform/verisilicon/stm32mp25_vdec_hw.c  | 92 +++++++++++++++++++
 5 files changed, 110 insertions(+), 3 deletions(-)
 create mode 100644 drivers/media/platform/verisilicon/stm32mp25_vdec_hw.c

diff --git a/drivers/media/platform/verisilicon/Kconfig b/drivers/media/platform/verisilicon/Kconfig
index e65b836b9d78..7642ff9cf96c 100644
--- a/drivers/media/platform/verisilicon/Kconfig
+++ b/drivers/media/platform/verisilicon/Kconfig
@@ -4,7 +4,7 @@ comment "Verisilicon media platform drivers"
 
 config VIDEO_HANTRO
 	tristate "Hantro VPU driver"
-	depends on ARCH_MXC || ARCH_ROCKCHIP || ARCH_AT91 || ARCH_SUNXI || COMPILE_TEST
+	depends on ARCH_MXC || ARCH_ROCKCHIP || ARCH_AT91 || ARCH_SUNXI || ARCH_STM32 || COMPILE_TEST
 	depends on V4L_MEM2MEM_DRIVERS
 	depends on VIDEO_DEV
 	select MEDIA_CONTROLLER
@@ -16,8 +16,8 @@ config VIDEO_HANTRO
 	select V4L2_VP9
 	help
 	  Support for the Hantro IP based Video Processing Units present on
-	  Rockchip and NXP i.MX8M SoCs, which accelerate video and image
-	  encoding and decoding.
+	  Rockchip, NXP i.MX8M and STM32MP25 SoCs, which accelerate video
+	  and image encoding and decoding.
 	  To compile this driver as a module, choose M here: the module
 	  will be called hantro-vpu.
 
@@ -52,3 +52,11 @@ config VIDEO_HANTRO_SUNXI
 	default y
 	help
 	  Enable support for H6 SoC.
+
+config VIDEO_HANTRO_STM32MP25
+	bool "Hantro STM32MP25 support"
+	depends on VIDEO_HANTRO
+	depends on ARCH_STM32 || COMPILE_TEST
+	default y
+	help
+	  Enable support for STM32MP25 SoCs.
diff --git a/drivers/media/platform/verisilicon/Makefile b/drivers/media/platform/verisilicon/Makefile
index 6ad2ef885920..5854e0f0dd32 100644
--- a/drivers/media/platform/verisilicon/Makefile
+++ b/drivers/media/platform/verisilicon/Makefile
@@ -39,3 +39,6 @@ hantro-vpu-$(CONFIG_VIDEO_HANTRO_ROCKCHIP) += \
 
 hantro-vpu-$(CONFIG_VIDEO_HANTRO_SUNXI) += \
 		sunxi_vpu_hw.o
+
+hantro-vpu-$(CONFIG_VIDEO_HANTRO_STM32MP25) += \
+		stm32mp25_vdec_hw.o
diff --git a/drivers/media/platform/verisilicon/hantro_drv.c b/drivers/media/platform/verisilicon/hantro_drv.c
index a9fa05ac56a9..2db27c333924 100644
--- a/drivers/media/platform/verisilicon/hantro_drv.c
+++ b/drivers/media/platform/verisilicon/hantro_drv.c
@@ -733,6 +733,9 @@ static const struct of_device_id of_hantro_match[] = {
 #endif
 #ifdef CONFIG_VIDEO_HANTRO_SUNXI
 	{ .compatible = "allwinner,sun50i-h6-vpu-g2", .data = &sunxi_vpu_variant, },
+#endif
+#ifdef CONFIG_VIDEO_HANTRO_STM32MP25
+	{ .compatible = "st,stm32mp25-vdec", .data = &stm32mp25_vdec_variant, },
 #endif
 	{ /* sentinel */ }
 };
diff --git a/drivers/media/platform/verisilicon/hantro_hw.h b/drivers/media/platform/verisilicon/hantro_hw.h
index 7f33f7b07ce4..b7eccc1a96fc 100644
--- a/drivers/media/platform/verisilicon/hantro_hw.h
+++ b/drivers/media/platform/verisilicon/hantro_hw.h
@@ -406,6 +406,7 @@ extern const struct hantro_variant rk3568_vpu_variant;
 extern const struct hantro_variant rk3588_vpu981_variant;
 extern const struct hantro_variant sama5d4_vdec_variant;
 extern const struct hantro_variant sunxi_vpu_variant;
+extern const struct hantro_variant stm32mp25_vdec_variant;
 
 extern const struct hantro_postproc_ops hantro_g1_postproc_ops;
 extern const struct hantro_postproc_ops hantro_g2_postproc_ops;
diff --git a/drivers/media/platform/verisilicon/stm32mp25_vdec_hw.c b/drivers/media/platform/verisilicon/stm32mp25_vdec_hw.c
new file mode 100644
index 000000000000..aa8b0f751390
--- /dev/null
+++ b/drivers/media/platform/verisilicon/stm32mp25_vdec_hw.c
@@ -0,0 +1,92 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * STM32MP25 VDEC video decoder driver
+ *
+ * Copyright (C) STMicroelectronics SA 2022
+ * Authors: Hugues Fruchet <hugues.fruchet@foss.st.com>
+ *          for STMicroelectronics.
+ *
+ */
+
+#include "hantro.h"
+
+/*
+ * Supported formats.
+ */
+
+static const struct hantro_fmt stm32mp25_vdec_fmts[] = {
+	{
+		.fourcc = V4L2_PIX_FMT_NV12,
+		.codec_mode = HANTRO_MODE_NONE,
+		.frmsize = {
+			.min_width = FMT_MIN_WIDTH,
+			.max_width = FMT_FHD_WIDTH,
+			.step_width = MB_DIM,
+			.min_height = FMT_MIN_HEIGHT,
+			.max_height = FMT_FHD_HEIGHT,
+			.step_height = MB_DIM,
+		},
+	},
+	{
+		.fourcc = V4L2_PIX_FMT_VP8_FRAME,
+		.codec_mode = HANTRO_MODE_VP8_DEC,
+		.max_depth = 2,
+		.frmsize = {
+			.min_width = FMT_MIN_WIDTH,
+			.max_width = FMT_FHD_WIDTH,
+			.step_width = MB_DIM,
+			.min_height = FMT_MIN_HEIGHT,
+			.max_height = FMT_FHD_HEIGHT,
+			.step_height = MB_DIM,
+		},
+	},
+	{
+		.fourcc = V4L2_PIX_FMT_H264_SLICE,
+		.codec_mode = HANTRO_MODE_H264_DEC,
+		.max_depth = 2,
+		.frmsize = {
+			.min_width = FMT_MIN_WIDTH,
+			.max_width = FMT_FHD_WIDTH,
+			.step_width = MB_DIM,
+			.min_height = FMT_MIN_HEIGHT,
+			.max_height = FMT_FHD_HEIGHT,
+			.step_height = MB_DIM,
+		},
+	},
+};
+
+/*
+ * Supported codec ops.
+ */
+
+static const struct hantro_codec_ops stm32mp25_vdec_codec_ops[] = {
+	[HANTRO_MODE_VP8_DEC] = {
+		.run = hantro_g1_vp8_dec_run,
+		.reset = hantro_g1_reset,
+		.init = hantro_vp8_dec_init,
+		.exit = hantro_vp8_dec_exit,
+	},
+	[HANTRO_MODE_H264_DEC] = {
+		.run = hantro_g1_h264_dec_run,
+		.reset = hantro_g1_reset,
+		.init = hantro_h264_dec_init,
+		.exit = hantro_h264_dec_exit,
+	},
+};
+
+static const struct hantro_irq stm32mp25_irqs[] = {
+	{ "vdec", hantro_g1_irq },
+};
+
+static const char * const stm32mp25_clk_names[] = { "vdec-clk" };
+
+const struct hantro_variant stm32mp25_vdec_variant = {
+	.dec_fmts = stm32mp25_vdec_fmts,
+	.num_dec_fmts = ARRAY_SIZE(stm32mp25_vdec_fmts),
+	.codec = HANTRO_VP8_DECODER | HANTRO_H264_DECODER,
+	.codec_ops = stm32mp25_vdec_codec_ops,
+	.irqs = stm32mp25_irqs,
+	.num_irqs = ARRAY_SIZE(stm32mp25_irqs),
+	.clk_names = stm32mp25_clk_names,
+	.num_clocks = ARRAY_SIZE(stm32mp25_clk_names),
+};
-- 
2.25.1

[PATCH v5 3/5] media: hantro: add support for STM32MP25 VENC

[Hugues Fruchet]

Add support for STM32MP25 VENC video hardware encoder.
Support of JPEG encoding.
VENC has its own reset/clock/irq.

Signed-off-by: Hugues Fruchet <hugues.fruchet@foss.st.com>
Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
---
 drivers/media/platform/verisilicon/Makefile   |   3 +-
 .../media/platform/verisilicon/hantro_drv.c   |   1 +
 .../media/platform/verisilicon/hantro_hw.h    |   1 +
 .../platform/verisilicon/stm32mp25_venc_hw.c  | 115 ++++++++++++++++++
 4 files changed, 119 insertions(+), 1 deletion(-)
 create mode 100644 drivers/media/platform/verisilicon/stm32mp25_venc_hw.c

diff --git a/drivers/media/platform/verisilicon/Makefile b/drivers/media/platform/verisilicon/Makefile
index 5854e0f0dd32..3bf43fdbedc1 100644
--- a/drivers/media/platform/verisilicon/Makefile
+++ b/drivers/media/platform/verisilicon/Makefile
@@ -41,4 +41,5 @@ hantro-vpu-$(CONFIG_VIDEO_HANTRO_SUNXI) += \
 		sunxi_vpu_hw.o
 
 hantro-vpu-$(CONFIG_VIDEO_HANTRO_STM32MP25) += \
-		stm32mp25_vdec_hw.o
+		stm32mp25_vdec_hw.o \
+		stm32mp25_venc_hw.o
diff --git a/drivers/media/platform/verisilicon/hantro_drv.c b/drivers/media/platform/verisilicon/hantro_drv.c
index 2db27c333924..4d97a8ac03de 100644
--- a/drivers/media/platform/verisilicon/hantro_drv.c
+++ b/drivers/media/platform/verisilicon/hantro_drv.c
@@ -736,6 +736,7 @@ static const struct of_device_id of_hantro_match[] = {
 #endif
 #ifdef CONFIG_VIDEO_HANTRO_STM32MP25
 	{ .compatible = "st,stm32mp25-vdec", .data = &stm32mp25_vdec_variant, },
+	{ .compatible = "st,stm32mp25-venc", .data = &stm32mp25_venc_variant, },
 #endif
 	{ /* sentinel */ }
 };
diff --git a/drivers/media/platform/verisilicon/hantro_hw.h b/drivers/media/platform/verisilicon/hantro_hw.h
index b7eccc1a96fc..70c72e9d11d5 100644
--- a/drivers/media/platform/verisilicon/hantro_hw.h
+++ b/drivers/media/platform/verisilicon/hantro_hw.h
@@ -407,6 +407,7 @@ extern const struct hantro_variant rk3588_vpu981_variant;
 extern const struct hantro_variant sama5d4_vdec_variant;
 extern const struct hantro_variant sunxi_vpu_variant;
 extern const struct hantro_variant stm32mp25_vdec_variant;
+extern const struct hantro_variant stm32mp25_venc_variant;
 
 extern const struct hantro_postproc_ops hantro_g1_postproc_ops;
 extern const struct hantro_postproc_ops hantro_g2_postproc_ops;
diff --git a/drivers/media/platform/verisilicon/stm32mp25_venc_hw.c b/drivers/media/platform/verisilicon/stm32mp25_venc_hw.c
new file mode 100644
index 000000000000..0ff0f073b922
--- /dev/null
+++ b/drivers/media/platform/verisilicon/stm32mp25_venc_hw.c
@@ -0,0 +1,115 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * STM32MP25 VENC video encoder driver
+ *
+ * Copyright (C) STMicroelectronics SA 2022
+ * Authors: Hugues Fruchet <hugues.fruchet@foss.st.com>
+ *          for STMicroelectronics.
+ *
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/reset.h>
+
+#include "hantro.h"
+#include "hantro_jpeg.h"
+#include "hantro_h1_regs.h"
+
+/*
+ * Supported formats.
+ */
+
+static const struct hantro_fmt stm32mp25_venc_fmts[] = {
+	{
+		.fourcc = V4L2_PIX_FMT_YUV420M,
+		.codec_mode = HANTRO_MODE_NONE,
+		.enc_fmt = ROCKCHIP_VPU_ENC_FMT_YUV420P,
+	},
+	{
+		.fourcc = V4L2_PIX_FMT_NV12M,
+		.codec_mode = HANTRO_MODE_NONE,
+		.enc_fmt = ROCKCHIP_VPU_ENC_FMT_YUV420SP,
+	},
+	{
+		.fourcc = V4L2_PIX_FMT_YUYV,
+		.codec_mode = HANTRO_MODE_NONE,
+		.enc_fmt = ROCKCHIP_VPU_ENC_FMT_YUYV422,
+	},
+	{
+		.fourcc = V4L2_PIX_FMT_UYVY,
+		.codec_mode = HANTRO_MODE_NONE,
+		.enc_fmt = ROCKCHIP_VPU_ENC_FMT_UYVY422,
+	},
+	{
+		.fourcc = V4L2_PIX_FMT_JPEG,
+		.codec_mode = HANTRO_MODE_JPEG_ENC,
+		.max_depth = 2,
+		.header_size = JPEG_HEADER_SIZE,
+		.frmsize = {
+			.min_width = 96,
+			.max_width = FMT_4K_WIDTH,
+			.step_width = MB_DIM,
+			.min_height = 96,
+			.max_height = FMT_4K_HEIGHT,
+			.step_height = MB_DIM,
+		},
+	},
+};
+
+static irqreturn_t stm32mp25_venc_irq(int irq, void *dev_id)
+{
+	struct hantro_dev *vpu = dev_id;
+	enum vb2_buffer_state state;
+	u32 status;
+
+	status = vepu_read(vpu, H1_REG_INTERRUPT);
+	state = (status & H1_REG_INTERRUPT_FRAME_RDY) ?
+		VB2_BUF_STATE_DONE : VB2_BUF_STATE_ERROR;
+
+	vepu_write(vpu, H1_REG_INTERRUPT_BIT, H1_REG_INTERRUPT);
+
+	hantro_irq_done(vpu, state);
+
+	return IRQ_HANDLED;
+}
+
+static void stm32mp25_venc_reset(struct hantro_ctx *ctx)
+{
+}
+
+/*
+ * Supported codec ops.
+ */
+
+static const struct hantro_codec_ops stm32mp25_venc_codec_ops[] = {
+	[HANTRO_MODE_JPEG_ENC] = {
+		.run = hantro_h1_jpeg_enc_run,
+		.reset = stm32mp25_venc_reset,
+		.done = hantro_h1_jpeg_enc_done,
+	},
+};
+
+/*
+ * Variants.
+ */
+
+static const struct hantro_irq stm32mp25_venc_irqs[] = {
+	{ "venc", stm32mp25_venc_irq },
+};
+
+static const char * const stm32mp25_venc_clk_names[] = {
+	"venc-clk"
+};
+
+const struct hantro_variant stm32mp25_venc_variant = {
+	.enc_fmts = stm32mp25_venc_fmts,
+	.num_enc_fmts = ARRAY_SIZE(stm32mp25_venc_fmts),
+	.codec = HANTRO_JPEG_ENCODER,
+	.codec_ops = stm32mp25_venc_codec_ops,
+	.irqs = stm32mp25_venc_irqs,
+	.num_irqs = ARRAY_SIZE(stm32mp25_venc_irqs),
+	.clk_names = stm32mp25_venc_clk_names,
+	.num_clocks = ARRAY_SIZE(stm32mp25_venc_clk_names)
+};
+
-- 
2.25.1

[PATCH v5 4/5] arm64: dts: st: add video decoder support to stm32mp255

[Hugues Fruchet]

Add VDEC hardware video decoder support to STM32MP255.

Signed-off-by: Hugues Fruchet <hugues.fruchet@foss.st.com>
---
 arch/arm64/boot/dts/st/stm32mp251.dtsi |  6 ++++++
 arch/arm64/boot/dts/st/stm32mp255.dtsi | 10 ++++++++++
 2 files changed, 16 insertions(+)

diff --git a/arch/arm64/boot/dts/st/stm32mp251.dtsi b/arch/arm64/boot/dts/st/stm32mp251.dtsi
index 96859d098ef8..8fc7e9199499 100644
--- a/arch/arm64/boot/dts/st/stm32mp251.dtsi
+++ b/arch/arm64/boot/dts/st/stm32mp251.dtsi
@@ -52,6 +52,12 @@ ck_icn_ls_mcu: ck-icn-ls-mcu {
 			compatible = "fixed-clock";
 			clock-frequency = <200000000>;
 		};
+
+		ck_icn_p_vdec: ck-icn-p-vdec {
+			#clock-cells = <0>;
+			compatible = "fixed-clock";
+			clock-frequency = <200000000>;
+		};
 	};
 
 	firmware {
diff --git a/arch/arm64/boot/dts/st/stm32mp255.dtsi b/arch/arm64/boot/dts/st/stm32mp255.dtsi
index e6fa596211f5..aea5096dac3c 100644
--- a/arch/arm64/boot/dts/st/stm32mp255.dtsi
+++ b/arch/arm64/boot/dts/st/stm32mp255.dtsi
@@ -6,4 +6,14 @@
 #include "stm32mp253.dtsi"
 
 / {
+	soc@0 {
+		rifsc: rifsc-bus@42080000 {
+			vdec: vdec@480d0000 {
+				compatible = "st,stm32mp25-vdec";
+				reg = <0x480d0000 0x3c8>;
+				interrupts = <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&ck_icn_p_vdec>;
+			};
+		};
+	};
 };
-- 
2.25.1

[PATCH v5 5/5] arm64: dts: st: add video encoder support to stm32mp255

[Hugues Fruchet]

Add VENC hardware video encoder support to STM32MP255.

Signed-off-by: Hugues Fruchet <hugues.fruchet@foss.st.com>
---
 arch/arm64/boot/dts/st/stm32mp251.dtsi | 6 ++++++
 arch/arm64/boot/dts/st/stm32mp255.dtsi | 7 +++++++
 2 files changed, 13 insertions(+)

diff --git a/arch/arm64/boot/dts/st/stm32mp251.dtsi b/arch/arm64/boot/dts/st/stm32mp251.dtsi
index 8fc7e9199499..5dd4f3580a60 100644
--- a/arch/arm64/boot/dts/st/stm32mp251.dtsi
+++ b/arch/arm64/boot/dts/st/stm32mp251.dtsi
@@ -58,6 +58,12 @@ ck_icn_p_vdec: ck-icn-p-vdec {
 			compatible = "fixed-clock";
 			clock-frequency = <200000000>;
 		};
+
+		ck_icn_p_venc: ck-icn-p-venc {
+			#clock-cells = <0>;
+			compatible = "fixed-clock";
+			clock-frequency = <200000000>;
+		};
 	};
 
 	firmware {
diff --git a/arch/arm64/boot/dts/st/stm32mp255.dtsi b/arch/arm64/boot/dts/st/stm32mp255.dtsi
index aea5096dac3c..17f197c5b22b 100644
--- a/arch/arm64/boot/dts/st/stm32mp255.dtsi
+++ b/arch/arm64/boot/dts/st/stm32mp255.dtsi
@@ -14,6 +14,13 @@ vdec: vdec@480d0000 {
 				interrupts = <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&ck_icn_p_vdec>;
 			};
+
+			venc: venc@480e0000 {
+				compatible = "st,stm32mp25-venc";
+				reg = <0x480e0000 0x800>;
+				interrupts = <GIC_SPI 167 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&ck_icn_ls_mcu>;
+			};
 		};
 	};
 };
-- 
2.25.1

Re: [PATCH v5 1/5] dt-bindings: media: Document STM32MP25 VDEC & VENC video codecs

[Krzysztof Kozlowski]

On 21/12/2023 09:47, Hugues Fruchet wrote:
> Add STM32MP25 VDEC video decoder & VENC video encoder bindings.
> 
> Signed-off-by: Hugues Fruchet <hugues.fruchet@foss.st.com>
> ---

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH v5 2/5] media: hantro: add support for STM32MP25 VDEC

[Sebastian Fricke]

Hey Hugues,

On 21.12.2023 09:47, Hugues Fruchet wrote:
>Add support for STM32MP25 VDEC video hardware decoder.
>Support of H264/VP8 decoding.
>No post-processor support.
>VDEC has its own reset/clock/irq.
>
>Sucessfully tested up to full HD.

s/Sucessfully/Successfully/

if this turns out to be the only problem, then I can fix it up for the
PR.

Greetings,
Sebastian

>
>Signed-off-by: Hugues Fruchet <hugues.fruchet@foss.st.com>
>Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
>---
> drivers/media/platform/verisilicon/Kconfig    | 14 ++-
> drivers/media/platform/verisilicon/Makefile   |  3 +
> .../media/platform/verisilicon/hantro_drv.c   |  3 +
> .../media/platform/verisilicon/hantro_hw.h    |  1 +
> .../platform/verisilicon/stm32mp25_vdec_hw.c  | 92 +++++++++++++++++++
> 5 files changed, 110 insertions(+), 3 deletions(-)
> create mode 100644 drivers/media/platform/verisilicon/stm32mp25_vdec_hw.c
>
>diff --git a/drivers/media/platform/verisilicon/Kconfig b/drivers/media/platform/verisilicon/Kconfig
>index e65b836b9d78..7642ff9cf96c 100644
>--- a/drivers/media/platform/verisilicon/Kconfig
>+++ b/drivers/media/platform/verisilicon/Kconfig
>@@ -4,7 +4,7 @@ comment "Verisilicon media platform drivers"
>
> config VIDEO_HANTRO
> 	tristate "Hantro VPU driver"
>-	depends on ARCH_MXC || ARCH_ROCKCHIP || ARCH_AT91 || ARCH_SUNXI || COMPILE_TEST
>+	depends on ARCH_MXC || ARCH_ROCKCHIP || ARCH_AT91 || ARCH_SUNXI || ARCH_STM32 || COMPILE_TEST
> 	depends on V4L_MEM2MEM_DRIVERS
> 	depends on VIDEO_DEV
> 	select MEDIA_CONTROLLER
>@@ -16,8 +16,8 @@ config VIDEO_HANTRO
> 	select V4L2_VP9
> 	help
> 	  Support for the Hantro IP based Video Processing Units present on
>-	  Rockchip and NXP i.MX8M SoCs, which accelerate video and image
>-	  encoding and decoding.
>+	  Rockchip, NXP i.MX8M and STM32MP25 SoCs, which accelerate video
>+	  and image encoding and decoding.
> 	  To compile this driver as a module, choose M here: the module
> 	  will be called hantro-vpu.
>
>@@ -52,3 +52,11 @@ config VIDEO_HANTRO_SUNXI
> 	default y
> 	help
> 	  Enable support for H6 SoC.
>+
>+config VIDEO_HANTRO_STM32MP25
>+	bool "Hantro STM32MP25 support"
>+	depends on VIDEO_HANTRO
>+	depends on ARCH_STM32 || COMPILE_TEST
>+	default y
>+	help
>+	  Enable support for STM32MP25 SoCs.
>diff --git a/drivers/media/platform/verisilicon/Makefile b/drivers/media/platform/verisilicon/Makefile
>index 6ad2ef885920..5854e0f0dd32 100644
>--- a/drivers/media/platform/verisilicon/Makefile
>+++ b/drivers/media/platform/verisilicon/Makefile
>@@ -39,3 +39,6 @@ hantro-vpu-$(CONFIG_VIDEO_HANTRO_ROCKCHIP) += \
>
> hantro-vpu-$(CONFIG_VIDEO_HANTRO_SUNXI) += \
> 		sunxi_vpu_hw.o
>+
>+hantro-vpu-$(CONFIG_VIDEO_HANTRO_STM32MP25) += \
>+		stm32mp25_vdec_hw.o
>diff --git a/drivers/media/platform/verisilicon/hantro_drv.c b/drivers/media/platform/verisilicon/hantro_drv.c
>index a9fa05ac56a9..2db27c333924 100644
>--- a/drivers/media/platform/verisilicon/hantro_drv.c
>+++ b/drivers/media/platform/verisilicon/hantro_drv.c
>@@ -733,6 +733,9 @@ static const struct of_device_id of_hantro_match[] = {
> #endif
> #ifdef CONFIG_VIDEO_HANTRO_SUNXI
> 	{ .compatible = "allwinner,sun50i-h6-vpu-g2", .data = &sunxi_vpu_variant, },
>+#endif
>+#ifdef CONFIG_VIDEO_HANTRO_STM32MP25
>+	{ .compatible = "st,stm32mp25-vdec", .data = &stm32mp25_vdec_variant, },
> #endif
> 	{ /* sentinel */ }
> };
>diff --git a/drivers/media/platform/verisilicon/hantro_hw.h b/drivers/media/platform/verisilicon/hantro_hw.h
>index 7f33f7b07ce4..b7eccc1a96fc 100644
>--- a/drivers/media/platform/verisilicon/hantro_hw.h
>+++ b/drivers/media/platform/verisilicon/hantro_hw.h
>@@ -406,6 +406,7 @@ extern const struct hantro_variant rk3568_vpu_variant;
> extern const struct hantro_variant rk3588_vpu981_variant;
> extern const struct hantro_variant sama5d4_vdec_variant;
> extern const struct hantro_variant sunxi_vpu_variant;
>+extern const struct hantro_variant stm32mp25_vdec_variant;
>
> extern const struct hantro_postproc_ops hantro_g1_postproc_ops;
> extern const struct hantro_postproc_ops hantro_g2_postproc_ops;
>diff --git a/drivers/media/platform/verisilicon/stm32mp25_vdec_hw.c b/drivers/media/platform/verisilicon/stm32mp25_vdec_hw.c
>new file mode 100644
>index 000000000000..aa8b0f751390
>--- /dev/null
>+++ b/drivers/media/platform/verisilicon/stm32mp25_vdec_hw.c
>@@ -0,0 +1,92 @@
>+// SPDX-License-Identifier: GPL-2.0
>+/*
>+ * STM32MP25 VDEC video decoder driver
>+ *
>+ * Copyright (C) STMicroelectronics SA 2022
>+ * Authors: Hugues Fruchet <hugues.fruchet@foss.st.com>
>+ *          for STMicroelectronics.
>+ *
>+ */
>+
>+#include "hantro.h"
>+
>+/*
>+ * Supported formats.
>+ */
>+
>+static const struct hantro_fmt stm32mp25_vdec_fmts[] = {
>+	{
>+		.fourcc = V4L2_PIX_FMT_NV12,
>+		.codec_mode = HANTRO_MODE_NONE,
>+		.frmsize = {
>+			.min_width = FMT_MIN_WIDTH,
>+			.max_width = FMT_FHD_WIDTH,
>+			.step_width = MB_DIM,
>+			.min_height = FMT_MIN_HEIGHT,
>+			.max_height = FMT_FHD_HEIGHT,
>+			.step_height = MB_DIM,
>+		},
>+	},
>+	{
>+		.fourcc = V4L2_PIX_FMT_VP8_FRAME,
>+		.codec_mode = HANTRO_MODE_VP8_DEC,
>+		.max_depth = 2,
>+		.frmsize = {
>+			.min_width = FMT_MIN_WIDTH,
>+			.max_width = FMT_FHD_WIDTH,
>+			.step_width = MB_DIM,
>+			.min_height = FMT_MIN_HEIGHT,
>+			.max_height = FMT_FHD_HEIGHT,
>+			.step_height = MB_DIM,
>+		},
>+	},
>+	{
>+		.fourcc = V4L2_PIX_FMT_H264_SLICE,
>+		.codec_mode = HANTRO_MODE_H264_DEC,
>+		.max_depth = 2,
>+		.frmsize = {
>+			.min_width = FMT_MIN_WIDTH,
>+			.max_width = FMT_FHD_WIDTH,
>+			.step_width = MB_DIM,
>+			.min_height = FMT_MIN_HEIGHT,
>+			.max_height = FMT_FHD_HEIGHT,
>+			.step_height = MB_DIM,
>+		},
>+	},
>+};
>+
>+/*
>+ * Supported codec ops.
>+ */
>+
>+static const struct hantro_codec_ops stm32mp25_vdec_codec_ops[] = {
>+	[HANTRO_MODE_VP8_DEC] = {
>+		.run = hantro_g1_vp8_dec_run,
>+		.reset = hantro_g1_reset,
>+		.init = hantro_vp8_dec_init,
>+		.exit = hantro_vp8_dec_exit,
>+	},
>+	[HANTRO_MODE_H264_DEC] = {
>+		.run = hantro_g1_h264_dec_run,
>+		.reset = hantro_g1_reset,
>+		.init = hantro_h264_dec_init,
>+		.exit = hantro_h264_dec_exit,
>+	},
>+};
>+
>+static const struct hantro_irq stm32mp25_irqs[] = {
>+	{ "vdec", hantro_g1_irq },
>+};
>+
>+static const char * const stm32mp25_clk_names[] = { "vdec-clk" };
>+
>+const struct hantro_variant stm32mp25_vdec_variant = {
>+	.dec_fmts = stm32mp25_vdec_fmts,
>+	.num_dec_fmts = ARRAY_SIZE(stm32mp25_vdec_fmts),
>+	.codec = HANTRO_VP8_DECODER | HANTRO_H264_DECODER,
>+	.codec_ops = stm32mp25_vdec_codec_ops,
>+	.irqs = stm32mp25_irqs,
>+	.num_irqs = ARRAY_SIZE(stm32mp25_irqs),
>+	.clk_names = stm32mp25_clk_names,
>+	.num_clocks = ARRAY_SIZE(stm32mp25_clk_names),
>+};
>-- 
>2.25.1
>
>

Re: [PATCH v5 3/5] media: hantro: add support for STM32MP25 VENC

[Sebastian Fricke]

Hey Hugues,

one small comment below, if it should turn out to be the only problem I
can fix it myself for the pull request.

On 21.12.2023 09:47, Hugues Fruchet wrote:
>Add support for STM32MP25 VENC video hardware encoder.
>Support of JPEG encoding.
>VENC has its own reset/clock/irq.
>
>Signed-off-by: Hugues Fruchet <hugues.fruchet@foss.st.com>
>Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
>---
> drivers/media/platform/verisilicon/Makefile   |   3 +-
> .../media/platform/verisilicon/hantro_drv.c   |   1 +
> .../media/platform/verisilicon/hantro_hw.h    |   1 +
> .../platform/verisilicon/stm32mp25_venc_hw.c  | 115 ++++++++++++++++++
> 4 files changed, 119 insertions(+), 1 deletion(-)
> create mode 100644 drivers/media/platform/verisilicon/stm32mp25_venc_hw.c
>
>diff --git a/drivers/media/platform/verisilicon/Makefile b/drivers/media/platform/verisilicon/Makefile
>index 5854e0f0dd32..3bf43fdbedc1 100644
>--- a/drivers/media/platform/verisilicon/Makefile
>+++ b/drivers/media/platform/verisilicon/Makefile
>@@ -41,4 +41,5 @@ hantro-vpu-$(CONFIG_VIDEO_HANTRO_SUNXI) += \
> 		sunxi_vpu_hw.o
>
> hantro-vpu-$(CONFIG_VIDEO_HANTRO_STM32MP25) += \
>-		stm32mp25_vdec_hw.o
>+		stm32mp25_vdec_hw.o \
>+		stm32mp25_venc_hw.o
>diff --git a/drivers/media/platform/verisilicon/hantro_drv.c b/drivers/media/platform/verisilicon/hantro_drv.c
>index 2db27c333924..4d97a8ac03de 100644
>--- a/drivers/media/platform/verisilicon/hantro_drv.c
>+++ b/drivers/media/platform/verisilicon/hantro_drv.c
>@@ -736,6 +736,7 @@ static const struct of_device_id of_hantro_match[] = {
> #endif
> #ifdef CONFIG_VIDEO_HANTRO_STM32MP25
> 	{ .compatible = "st,stm32mp25-vdec", .data = &stm32mp25_vdec_variant, },
>+	{ .compatible = "st,stm32mp25-venc", .data = &stm32mp25_venc_variant, },
> #endif
> 	{ /* sentinel */ }
> };
>diff --git a/drivers/media/platform/verisilicon/hantro_hw.h b/drivers/media/platform/verisilicon/hantro_hw.h
>index b7eccc1a96fc..70c72e9d11d5 100644
>--- a/drivers/media/platform/verisilicon/hantro_hw.h
>+++ b/drivers/media/platform/verisilicon/hantro_hw.h
>@@ -407,6 +407,7 @@ extern const struct hantro_variant rk3588_vpu981_variant;
> extern const struct hantro_variant sama5d4_vdec_variant;
> extern const struct hantro_variant sunxi_vpu_variant;
> extern const struct hantro_variant stm32mp25_vdec_variant;
>+extern const struct hantro_variant stm32mp25_venc_variant;
>
> extern const struct hantro_postproc_ops hantro_g1_postproc_ops;
> extern const struct hantro_postproc_ops hantro_g2_postproc_ops;
>diff --git a/drivers/media/platform/verisilicon/stm32mp25_venc_hw.c b/drivers/media/platform/verisilicon/stm32mp25_venc_hw.c
>new file mode 100644
>index 000000000000..0ff0f073b922
>--- /dev/null
>+++ b/drivers/media/platform/verisilicon/stm32mp25_venc_hw.c
>@@ -0,0 +1,115 @@
>+// SPDX-License-Identifier: GPL-2.0
>+/*
>+ * STM32MP25 VENC video encoder driver
>+ *
>+ * Copyright (C) STMicroelectronics SA 2022
>+ * Authors: Hugues Fruchet <hugues.fruchet@foss.st.com>
>+ *          for STMicroelectronics.
>+ *
>+ */
>+
>+#include <linux/clk.h>
>+#include <linux/delay.h>
>+#include <linux/reset.h>
>+
>+#include "hantro.h"
>+#include "hantro_jpeg.h"
>+#include "hantro_h1_regs.h"
>+
>+/*
>+ * Supported formats.
>+ */
>+
>+static const struct hantro_fmt stm32mp25_venc_fmts[] = {
>+	{
>+		.fourcc = V4L2_PIX_FMT_YUV420M,
>+		.codec_mode = HANTRO_MODE_NONE,
>+		.enc_fmt = ROCKCHIP_VPU_ENC_FMT_YUV420P,
>+	},
>+	{
>+		.fourcc = V4L2_PIX_FMT_NV12M,
>+		.codec_mode = HANTRO_MODE_NONE,
>+		.enc_fmt = ROCKCHIP_VPU_ENC_FMT_YUV420SP,
>+	},
>+	{
>+		.fourcc = V4L2_PIX_FMT_YUYV,
>+		.codec_mode = HANTRO_MODE_NONE,
>+		.enc_fmt = ROCKCHIP_VPU_ENC_FMT_YUYV422,
>+	},
>+	{
>+		.fourcc = V4L2_PIX_FMT_UYVY,
>+		.codec_mode = HANTRO_MODE_NONE,
>+		.enc_fmt = ROCKCHIP_VPU_ENC_FMT_UYVY422,
>+	},
>+	{
>+		.fourcc = V4L2_PIX_FMT_JPEG,
>+		.codec_mode = HANTRO_MODE_JPEG_ENC,
>+		.max_depth = 2,
>+		.header_size = JPEG_HEADER_SIZE,
>+		.frmsize = {
>+			.min_width = 96,
>+			.max_width = FMT_4K_WIDTH,
>+			.step_width = MB_DIM,
>+			.min_height = 96,
>+			.max_height = FMT_4K_HEIGHT,
>+			.step_height = MB_DIM,
>+		},
>+	},
>+};
>+
>+static irqreturn_t stm32mp25_venc_irq(int irq, void *dev_id)
>+{
>+	struct hantro_dev *vpu = dev_id;
>+	enum vb2_buffer_state state;
>+	u32 status;
>+
>+	status = vepu_read(vpu, H1_REG_INTERRUPT);
>+	state = (status & H1_REG_INTERRUPT_FRAME_RDY) ?
>+		VB2_BUF_STATE_DONE : VB2_BUF_STATE_ERROR;
>+
>+	vepu_write(vpu, H1_REG_INTERRUPT_BIT, H1_REG_INTERRUPT);
>+
>+	hantro_irq_done(vpu, state);
>+
>+	return IRQ_HANDLED;
>+}
>+
>+static void stm32mp25_venc_reset(struct hantro_ctx *ctx)
>+{
>+}
>+
>+/*
>+ * Supported codec ops.
>+ */
>+
>+static const struct hantro_codec_ops stm32mp25_venc_codec_ops[] = {
>+	[HANTRO_MODE_JPEG_ENC] = {
>+		.run = hantro_h1_jpeg_enc_run,
>+		.reset = stm32mp25_venc_reset,
>+		.done = hantro_h1_jpeg_enc_done,
>+	},
>+};
>+
>+/*
>+ * Variants.
>+ */
>+
>+static const struct hantro_irq stm32mp25_venc_irqs[] = {
>+	{ "venc", stm32mp25_venc_irq },
>+};
>+
>+static const char * const stm32mp25_venc_clk_names[] = {
>+	"venc-clk"
>+};
>+
>+const struct hantro_variant stm32mp25_venc_variant = {
>+	.enc_fmts = stm32mp25_venc_fmts,
>+	.num_enc_fmts = ARRAY_SIZE(stm32mp25_venc_fmts),
>+	.codec = HANTRO_JPEG_ENCODER,
>+	.codec_ops = stm32mp25_venc_codec_ops,
>+	.irqs = stm32mp25_venc_irqs,
>+	.num_irqs = ARRAY_SIZE(stm32mp25_venc_irqs),
>+	.clk_names = stm32mp25_venc_clk_names,
>+	.num_clocks = ARRAY_SIZE(stm32mp25_venc_clk_names)
>+};
>+

There is an superfluous new line here.

Greetings,
Sebastian

>-- 
>2.25.1
>
>

Re: [PATCH v5 0/5] Add support for video hardware codec of STMicroelectronics STM32 SoC series

[Alex Bee]

Hi Hugues, Hi Nicolas,

is there any specific reason I'm not understanding / seeing why this is 
added in two seperate vdec* / venc* files and not a single vpu* file? Is 
it only for the seperate clocks (-names) / irqs (-names) / callbacks? 
Those are defined per variant and perfectly fit in a single file holding 
one vdec and one venc variant.

Alex

Am 21.12.23 um 09:47 schrieb Hugues Fruchet:
> This patchset introduces support for VDEC video hardware decoder
> and VENC video hardware encoder of STMicroelectronics STM32MP25
> SoC series.
> 
> This initial support implements H264 decoding, VP8 decoding and
> JPEG encoding.
> 
> This has been tested on STM32MP257F-EV1 evaluation board.
> 
> ===========
> = history =
> ===========
> version 5:
>     - Precise that video decoding as been successfully tested up to full HD
>     - Add Nicolas Dufresne reviewed-by
> 
> version 4:
>     - Fix comments from Nicolas about dropping encoder raw steps
> 
> version 3:
>     - Fix remarks from Krzysztof Kozlowski:
>      - drop "items", we keep simple enum in such case
>      - drop second example - it is the same as the first
>     - Drop unused node labels as suggested by Conor Dooley
>     - Revisit min/max resolutions as suggested by Nicolas Dufresne
> 
> version 2:
>     - Fix remarks from Krzysztof Kozlowski on v1:
>      - single video-codec binding for both VDEC/VENC
>      - get rid of "-names"
>      - use of generic node name "video-codec"
> 
> version 1:
>    - Initial submission
> 
> Hugues Fruchet (5):
>    dt-bindings: media: Document STM32MP25 VDEC & VENC video codecs
>    media: hantro: add support for STM32MP25 VDEC
>    media: hantro: add support for STM32MP25 VENC
>    arm64: dts: st: add video decoder support to stm32mp255
>    arm64: dts: st: add video encoder support to stm32mp255
> 
>   .../media/st,stm32mp25-video-codec.yaml       |  50 ++++++++
>   arch/arm64/boot/dts/st/stm32mp251.dtsi        |  12 ++
>   arch/arm64/boot/dts/st/stm32mp255.dtsi        |  17 +++
>   drivers/media/platform/verisilicon/Kconfig    |  14 ++-
>   drivers/media/platform/verisilicon/Makefile   |   4 +
>   .../media/platform/verisilicon/hantro_drv.c   |   4 +
>   .../media/platform/verisilicon/hantro_hw.h    |   2 +
>   .../platform/verisilicon/stm32mp25_vdec_hw.c  |  92 ++++++++++++++
>   .../platform/verisilicon/stm32mp25_venc_hw.c  | 115 ++++++++++++++++++
>   9 files changed, 307 insertions(+), 3 deletions(-)
>   create mode 100644 Documentation/devicetree/bindings/media/st,stm32mp25-video-codec.yaml
>   create mode 100644 drivers/media/platform/verisilicon/stm32mp25_vdec_hw.c
>   create mode 100644 drivers/media/platform/verisilicon/stm32mp25_venc_hw.c
> 

Re: [PATCH v5 3/5] media: hantro: add support for STM32MP25 VENC

[Hugues FRUCHET]

Hi Sebastian,

Yes, feel free to fix the two comments !

BR,
Hugues.

On 12/21/23 12:18, Sebastian Fricke wrote:
> Hey Hugues,
> 
> one small comment below, if it should turn out to be the only problem I
> can fix it myself for the pull request.
> 
> On 21.12.2023 09:47, Hugues Fruchet wrote:
>> Add support for STM32MP25 VENC video hardware encoder.
>> Support of JPEG encoding.
>> VENC has its own reset/clock/irq.
>>
>> Signed-off-by: Hugues Fruchet <hugues.fruchet@foss.st.com>
>> Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
>> ---
>> drivers/media/platform/verisilicon/Makefile   |   3 +-
>> .../media/platform/verisilicon/hantro_drv.c   |   1 +
>> .../media/platform/verisilicon/hantro_hw.h    |   1 +
>> .../platform/verisilicon/stm32mp25_venc_hw.c  | 115 ++++++++++++++++++
>> 4 files changed, 119 insertions(+), 1 deletion(-)
>> create mode 100644 drivers/media/platform/verisilicon/stm32mp25_venc_hw.c
>>
>> diff --git a/drivers/media/platform/verisilicon/Makefile 
>> b/drivers/media/platform/verisilicon/Makefile
>> index 5854e0f0dd32..3bf43fdbedc1 100644
>> --- a/drivers/media/platform/verisilicon/Makefile
>> +++ b/drivers/media/platform/verisilicon/Makefile
>> @@ -41,4 +41,5 @@ hantro-vpu-$(CONFIG_VIDEO_HANTRO_SUNXI) += \
>>         sunxi_vpu_hw.o
>>
>> hantro-vpu-$(CONFIG_VIDEO_HANTRO_STM32MP25) += \
>> -        stm32mp25_vdec_hw.o
>> +        stm32mp25_vdec_hw.o \
>> +        stm32mp25_venc_hw.o
>> diff --git a/drivers/media/platform/verisilicon/hantro_drv.c 
>> b/drivers/media/platform/verisilicon/hantro_drv.c
>> index 2db27c333924..4d97a8ac03de 100644
>> --- a/drivers/media/platform/verisilicon/hantro_drv.c
>> +++ b/drivers/media/platform/verisilicon/hantro_drv.c
>> @@ -736,6 +736,7 @@ static const struct of_device_id of_hantro_match[] 
>> = {
>> #endif
>> #ifdef CONFIG_VIDEO_HANTRO_STM32MP25
>>     { .compatible = "st,stm32mp25-vdec", .data = 
>> &stm32mp25_vdec_variant, },
>> +    { .compatible = "st,stm32mp25-venc", .data = 
>> &stm32mp25_venc_variant, },
>> #endif
>>     { /* sentinel */ }
>> };
>> diff --git a/drivers/media/platform/verisilicon/hantro_hw.h 
>> b/drivers/media/platform/verisilicon/hantro_hw.h
>> index b7eccc1a96fc..70c72e9d11d5 100644
>> --- a/drivers/media/platform/verisilicon/hantro_hw.h
>> +++ b/drivers/media/platform/verisilicon/hantro_hw.h
>> @@ -407,6 +407,7 @@ extern const struct hantro_variant 
>> rk3588_vpu981_variant;
>> extern const struct hantro_variant sama5d4_vdec_variant;
>> extern const struct hantro_variant sunxi_vpu_variant;
>> extern const struct hantro_variant stm32mp25_vdec_variant;
>> +extern const struct hantro_variant stm32mp25_venc_variant;
>>
>> extern const struct hantro_postproc_ops hantro_g1_postproc_ops;
>> extern const struct hantro_postproc_ops hantro_g2_postproc_ops;
>> diff --git a/drivers/media/platform/verisilicon/stm32mp25_venc_hw.c 
>> b/drivers/media/platform/verisilicon/stm32mp25_venc_hw.c
>> new file mode 100644
>> index 000000000000..0ff0f073b922
>> --- /dev/null
>> +++ b/drivers/media/platform/verisilicon/stm32mp25_venc_hw.c
>> @@ -0,0 +1,115 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * STM32MP25 VENC video encoder driver
>> + *
>> + * Copyright (C) STMicroelectronics SA 2022
>> + * Authors: Hugues Fruchet <hugues.fruchet@foss.st.com>
>> + *          for STMicroelectronics.
>> + *
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/delay.h>
>> +#include <linux/reset.h>
>> +
>> +#include "hantro.h"
>> +#include "hantro_jpeg.h"
>> +#include "hantro_h1_regs.h"
>> +
>> +/*
>> + * Supported formats.
>> + */
>> +
>> +static const struct hantro_fmt stm32mp25_venc_fmts[] = {
>> +    {
>> +        .fourcc = V4L2_PIX_FMT_YUV420M,
>> +        .codec_mode = HANTRO_MODE_NONE,
>> +        .enc_fmt = ROCKCHIP_VPU_ENC_FMT_YUV420P,
>> +    },
>> +    {
>> +        .fourcc = V4L2_PIX_FMT_NV12M,
>> +        .codec_mode = HANTRO_MODE_NONE,
>> +        .enc_fmt = ROCKCHIP_VPU_ENC_FMT_YUV420SP,
>> +    },
>> +    {
>> +        .fourcc = V4L2_PIX_FMT_YUYV,
>> +        .codec_mode = HANTRO_MODE_NONE,
>> +        .enc_fmt = ROCKCHIP_VPU_ENC_FMT_YUYV422,
>> +    },
>> +    {
>> +        .fourcc = V4L2_PIX_FMT_UYVY,
>> +        .codec_mode = HANTRO_MODE_NONE,
>> +        .enc_fmt = ROCKCHIP_VPU_ENC_FMT_UYVY422,
>> +    },
>> +    {
>> +        .fourcc = V4L2_PIX_FMT_JPEG,
>> +        .codec_mode = HANTRO_MODE_JPEG_ENC,
>> +        .max_depth = 2,
>> +        .header_size = JPEG_HEADER_SIZE,
>> +        .frmsize = {
>> +            .min_width = 96,
>> +            .max_width = FMT_4K_WIDTH,
>> +            .step_width = MB_DIM,
>> +            .min_height = 96,
>> +            .max_height = FMT_4K_HEIGHT,
>> +            .step_height = MB_DIM,
>> +        },
>> +    },
>> +};
>> +
>> +static irqreturn_t stm32mp25_venc_irq(int irq, void *dev_id)
>> +{
>> +    struct hantro_dev *vpu = dev_id;
>> +    enum vb2_buffer_state state;
>> +    u32 status;
>> +
>> +    status = vepu_read(vpu, H1_REG_INTERRUPT);
>> +    state = (status & H1_REG_INTERRUPT_FRAME_RDY) ?
>> +        VB2_BUF_STATE_DONE : VB2_BUF_STATE_ERROR;
>> +
>> +    vepu_write(vpu, H1_REG_INTERRUPT_BIT, H1_REG_INTERRUPT);
>> +
>> +    hantro_irq_done(vpu, state);
>> +
>> +    return IRQ_HANDLED;
>> +}
>> +
>> +static void stm32mp25_venc_reset(struct hantro_ctx *ctx)
>> +{
>> +}
>> +
>> +/*
>> + * Supported codec ops.
>> + */
>> +
>> +static const struct hantro_codec_ops stm32mp25_venc_codec_ops[] = {
>> +    [HANTRO_MODE_JPEG_ENC] = {
>> +        .run = hantro_h1_jpeg_enc_run,
>> +        .reset = stm32mp25_venc_reset,
>> +        .done = hantro_h1_jpeg_enc_done,
>> +    },
>> +};
>> +
>> +/*
>> + * Variants.
>> + */
>> +
>> +static const struct hantro_irq stm32mp25_venc_irqs[] = {
>> +    { "venc", stm32mp25_venc_irq },
>> +};
>> +
>> +static const char * const stm32mp25_venc_clk_names[] = {
>> +    "venc-clk"
>> +};
>> +
>> +const struct hantro_variant stm32mp25_venc_variant = {
>> +    .enc_fmts = stm32mp25_venc_fmts,
>> +    .num_enc_fmts = ARRAY_SIZE(stm32mp25_venc_fmts),
>> +    .codec = HANTRO_JPEG_ENCODER,
>> +    .codec_ops = stm32mp25_venc_codec_ops,
>> +    .irqs = stm32mp25_venc_irqs,
>> +    .num_irqs = ARRAY_SIZE(stm32mp25_venc_irqs),
>> +    .clk_names = stm32mp25_venc_clk_names,
>> +    .num_clocks = ARRAY_SIZE(stm32mp25_venc_clk_names)
>> +};
>> +
> 
> There is an superfluous new line here.
> 
> Greetings,
> Sebastian
> 
>> -- 
>> 2.25.1
>>
>>

Re: [PATCH v5 0/5] Add support for video hardware codec of STMicroelectronics STM32 SoC series

[Hugues FRUCHET]

Hi Alex,

This is because VDEC and VENC are two separated IPs with their own 
hardware resources and no links between both.
On future SoCs, VDEC can ship on its own, same for VENC.

Hoping that this clarify.

Best regards,
Hugues.

On 12/21/23 13:40, Alex Bee wrote:
> Hi Hugues, Hi Nicolas,
> 
> is there any specific reason I'm not understanding / seeing why this is 
> added in two seperate vdec* / venc* files and not a single vpu* file? Is 
> it only for the seperate clocks (-names) / irqs (-names) / callbacks? 
> Those are defined per variant and perfectly fit in a single file holding 
> one vdec and one venc variant.
> 
> Alex
> 
> Am 21.12.23 um 09:47 schrieb Hugues Fruchet:
>> This patchset introduces support for VDEC video hardware decoder
>> and VENC video hardware encoder of STMicroelectronics STM32MP25
>> SoC series.
>>
>> This initial support implements H264 decoding, VP8 decoding and
>> JPEG encoding.
>>
>> This has been tested on STM32MP257F-EV1 evaluation board.
>>
>> ===========
>> = history =
>> ===========
>> version 5:
>>     - Precise that video decoding as been successfully tested up to 
>> full HD
>>     - Add Nicolas Dufresne reviewed-by
>>
>> version 4:
>>     - Fix comments from Nicolas about dropping encoder raw steps
>>
>> version 3:
>>     - Fix remarks from Krzysztof Kozlowski:
>>      - drop "items", we keep simple enum in such case
>>      - drop second example - it is the same as the first
>>     - Drop unused node labels as suggested by Conor Dooley
>>     - Revisit min/max resolutions as suggested by Nicolas Dufresne
>>
>> version 2:
>>     - Fix remarks from Krzysztof Kozlowski on v1:
>>      - single video-codec binding for both VDEC/VENC
>>      - get rid of "-names"
>>      - use of generic node name "video-codec"
>>
>> version 1:
>>    - Initial submission
>>
>> Hugues Fruchet (5):
>>    dt-bindings: media: Document STM32MP25 VDEC & VENC video codecs
>>    media: hantro: add support for STM32MP25 VDEC
>>    media: hantro: add support for STM32MP25 VENC
>>    arm64: dts: st: add video decoder support to stm32mp255
>>    arm64: dts: st: add video encoder support to stm32mp255
>>
>>   .../media/st,stm32mp25-video-codec.yaml       |  50 ++++++++
>>   arch/arm64/boot/dts/st/stm32mp251.dtsi        |  12 ++
>>   arch/arm64/boot/dts/st/stm32mp255.dtsi        |  17 +++
>>   drivers/media/platform/verisilicon/Kconfig    |  14 ++-
>>   drivers/media/platform/verisilicon/Makefile   |   4 +
>>   .../media/platform/verisilicon/hantro_drv.c   |   4 +
>>   .../media/platform/verisilicon/hantro_hw.h    |   2 +
>>   .../platform/verisilicon/stm32mp25_vdec_hw.c  |  92 ++++++++++++++
>>   .../platform/verisilicon/stm32mp25_venc_hw.c  | 115 ++++++++++++++++++
>>   9 files changed, 307 insertions(+), 3 deletions(-)
>>   create mode 100644 
>> Documentation/devicetree/bindings/media/st,stm32mp25-video-codec.yaml
>>   create mode 100644 
>> drivers/media/platform/verisilicon/stm32mp25_vdec_hw.c
>>   create mode 100644 
>> drivers/media/platform/verisilicon/stm32mp25_venc_hw.c
>>
> 

Re: [PATCH v5 0/5] Add support for video hardware codec of STMicroelectronics STM32 SoC series

[Alex Bee]

Hi Hugues,

Am 21.12.23 um 14:08 schrieb Hugues FRUCHET:
> Hi Alex,
>
> This is because VDEC and VENC are two separated IPs with their own 
> hardware resources and no links between both.
> On future SoCs, VDEC can ship on its own, same for VENC.
>
I think that's what the driver is/was designed for :)

I don't  think there _has_ to be a link between variants in the same file.
For Rockchip we only had the issue that there _is_ a link (shared
resources) between encoder and decoder and they had (for that reason) to be
defined has a _single_ variant. And there is no reason you can ship decoder
and encoder seperated when you have two variants (with different
compatibles).
For Rockchip and iMX those files are even containing variants for completly
different generations / different SoCs. I had to cleanup this mess for
Rockchip once - and it was no fun :) Anyways: It's up to the maintainers I
guess - I just wanted to ask if I missunderstand something here.

Greetings,

Alex

> Hoping that this clarify.
>
> Best regards,
> Hugues.
>
> On 12/21/23 13:40, Alex Bee wrote:
>> Hi Hugues, Hi Nicolas,
>>
>> is there any specific reason I'm not understanding / seeing why this 
>> is added in two seperate vdec* / venc* files and not a single vpu* 
>> file? Is it only for the seperate clocks (-names) / irqs (-names) / 
>> callbacks? Those are defined per variant and perfectly fit in a 
>> single file holding one vdec and one venc variant.
>>
>> Alex
>>
>> Am 21.12.23 um 09:47 schrieb Hugues Fruchet:
>>> This patchset introduces support for VDEC video hardware decoder
>>> and VENC video hardware encoder of STMicroelectronics STM32MP25
>>> SoC series.
>>>
>>> This initial support implements H264 decoding, VP8 decoding and
>>> JPEG encoding.
>>>
>>> This has been tested on STM32MP257F-EV1 evaluation board.
>>>
>>> ===========
>>> = history =
>>> ===========
>>> version 5:
>>>     - Precise that video decoding as been successfully tested up to 
>>> full HD
>>>     - Add Nicolas Dufresne reviewed-by
>>>
>>> version 4:
>>>     - Fix comments from Nicolas about dropping encoder raw steps
>>>
>>> version 3:
>>>     - Fix remarks from Krzysztof Kozlowski:
>>>      - drop "items", we keep simple enum in such case
>>>      - drop second example - it is the same as the first
>>>     - Drop unused node labels as suggested by Conor Dooley
>>>     - Revisit min/max resolutions as suggested by Nicolas Dufresne
>>>
>>> version 2:
>>>     - Fix remarks from Krzysztof Kozlowski on v1:
>>>      - single video-codec binding for both VDEC/VENC
>>>      - get rid of "-names"
>>>      - use of generic node name "video-codec"
>>>
>>> version 1:
>>>    - Initial submission
>>>
>>> Hugues Fruchet (5):
>>>    dt-bindings: media: Document STM32MP25 VDEC & VENC video codecs
>>>    media: hantro: add support for STM32MP25 VDEC
>>>    media: hantro: add support for STM32MP25 VENC
>>>    arm64: dts: st: add video decoder support to stm32mp255
>>>    arm64: dts: st: add video encoder support to stm32mp255
>>>
>>>   .../media/st,stm32mp25-video-codec.yaml       |  50 ++++++++
>>>   arch/arm64/boot/dts/st/stm32mp251.dtsi        |  12 ++
>>>   arch/arm64/boot/dts/st/stm32mp255.dtsi        |  17 +++
>>>   drivers/media/platform/verisilicon/Kconfig    |  14 ++-
>>>   drivers/media/platform/verisilicon/Makefile   |   4 +
>>>   .../media/platform/verisilicon/hantro_drv.c   |   4 +
>>>   .../media/platform/verisilicon/hantro_hw.h    |   2 +
>>>   .../platform/verisilicon/stm32mp25_vdec_hw.c  |  92 ++++++++++++++
>>>   .../platform/verisilicon/stm32mp25_venc_hw.c  | 115 
>>> ++++++++++++++++++
>>>   9 files changed, 307 insertions(+), 3 deletions(-)
>>>   create mode 100644 
>>> Documentation/devicetree/bindings/media/st,stm32mp25-video-codec.yaml
>>>   create mode 100644 
>>> drivers/media/platform/verisilicon/stm32mp25_vdec_hw.c
>>>   create mode 100644 
>>> drivers/media/platform/verisilicon/stm32mp25_venc_hw.c
>>>
>>

Re: [PATCH v5 0/5] Add support for video hardware codec of STMicroelectronics STM32 SoC series

[Adam Ford]

On Thu, Dec 21, 2023 at 7:31 AM Alex Bee <knaerzche@gmail.com> wrote:
>
> Hi Hugues,
>
> Am 21.12.23 um 14:08 schrieb Hugues FRUCHET:
> > Hi Alex,
> >
> > This is because VDEC and VENC are two separated IPs with their own
> > hardware resources and no links between both.
> > On future SoCs, VDEC can ship on its own, same for VENC.
> >
> I think that's what the driver is/was designed for :)
>
> I don't  think there _has_ to be a link between variants in the same file.
> For Rockchip we only had the issue that there _is_ a link (shared
> resources) between encoder and decoder and they had (for that reason) to be
> defined has a _single_ variant. And there is no reason you can ship decoder
> and encoder seperated when you have two variants (with different
> compatibles).
> For Rockchip and iMX those files are even containing variants for completly
> different generations / different SoCs. I had to cleanup this mess for

The i.MX8M Mini and Plus have different power domains for encoder and
decoders as well as different clocks.  Keeping them separate would
almost be necessary.

adam

> Rockchip once - and it was no fun :) Anyways: It's up to the maintainers I
> guess - I just wanted to ask if I missunderstand something here.
>
> Greetings,
>
> Alex
>
> > Hoping that this clarify.
> >
> > Best regards,
> > Hugues.
> >
> > On 12/21/23 13:40, Alex Bee wrote:
> >> Hi Hugues, Hi Nicolas,
> >>
> >> is there any specific reason I'm not understanding / seeing why this
> >> is added in two seperate vdec* / venc* files and not a single vpu*
> >> file? Is it only for the seperate clocks (-names) / irqs (-names) /
> >> callbacks? Those are defined per variant and perfectly fit in a
> >> single file holding one vdec and one venc variant.
> >>
> >> Alex
> >>
> >> Am 21.12.23 um 09:47 schrieb Hugues Fruchet:
> >>> This patchset introduces support for VDEC video hardware decoder
> >>> and VENC video hardware encoder of STMicroelectronics STM32MP25
> >>> SoC series.
> >>>
> >>> This initial support implements H264 decoding, VP8 decoding and
> >>> JPEG encoding.
> >>>
> >>> This has been tested on STM32MP257F-EV1 evaluation board.
> >>>
> >>> ===========
> >>> = history =
> >>> ===========
> >>> version 5:
> >>>     - Precise that video decoding as been successfully tested up to
> >>> full HD
> >>>     - Add Nicolas Dufresne reviewed-by
> >>>
> >>> version 4:
> >>>     - Fix comments from Nicolas about dropping encoder raw steps
> >>>
> >>> version 3:
> >>>     - Fix remarks from Krzysztof Kozlowski:
> >>>      - drop "items", we keep simple enum in such case
> >>>      - drop second example - it is the same as the first
> >>>     - Drop unused node labels as suggested by Conor Dooley
> >>>     - Revisit min/max resolutions as suggested by Nicolas Dufresne
> >>>
> >>> version 2:
> >>>     - Fix remarks from Krzysztof Kozlowski on v1:
> >>>      - single video-codec binding for both VDEC/VENC
> >>>      - get rid of "-names"
> >>>      - use of generic node name "video-codec"
> >>>
> >>> version 1:
> >>>    - Initial submission
> >>>
> >>> Hugues Fruchet (5):
> >>>    dt-bindings: media: Document STM32MP25 VDEC & VENC video codecs
> >>>    media: hantro: add support for STM32MP25 VDEC
> >>>    media: hantro: add support for STM32MP25 VENC
> >>>    arm64: dts: st: add video decoder support to stm32mp255
> >>>    arm64: dts: st: add video encoder support to stm32mp255
> >>>
> >>>   .../media/st,stm32mp25-video-codec.yaml       |  50 ++++++++
> >>>   arch/arm64/boot/dts/st/stm32mp251.dtsi        |  12 ++
> >>>   arch/arm64/boot/dts/st/stm32mp255.dtsi        |  17 +++
> >>>   drivers/media/platform/verisilicon/Kconfig    |  14 ++-
> >>>   drivers/media/platform/verisilicon/Makefile   |   4 +
> >>>   .../media/platform/verisilicon/hantro_drv.c   |   4 +
> >>>   .../media/platform/verisilicon/hantro_hw.h    |   2 +
> >>>   .../platform/verisilicon/stm32mp25_vdec_hw.c  |  92 ++++++++++++++
> >>>   .../platform/verisilicon/stm32mp25_venc_hw.c  | 115
> >>> ++++++++++++++++++
> >>>   9 files changed, 307 insertions(+), 3 deletions(-)
> >>>   create mode 100644
> >>> Documentation/devicetree/bindings/media/st,stm32mp25-video-codec.yaml
> >>>   create mode 100644
> >>> drivers/media/platform/verisilicon/stm32mp25_vdec_hw.c
> >>>   create mode 100644
> >>> drivers/media/platform/verisilicon/stm32mp25_venc_hw.c
> >>>
> >>

Re: [PATCH v5 0/5] Add support for video hardware codec of STMicroelectronics STM32 SoC series

[Alex Bee]

Am 21.12.23 um 14:38 schrieb Adam Ford:
> On Thu, Dec 21, 2023 at 7:31 AM Alex Bee <knaerzche@gmail.com> wrote:
>> Hi Hugues,
>>
>> Am 21.12.23 um 14:08 schrieb Hugues FRUCHET:
>>> Hi Alex,
>>>
>>> This is because VDEC and VENC are two separated IPs with their own
>>> hardware resources and no links between both.
>>> On future SoCs, VDEC can ship on its own, same for VENC.
>>>
>> I think that's what the driver is/was designed for :)
>>
>> I don't  think there _has_ to be a link between variants in the same file.
>> For Rockchip we only had the issue that there _is_ a link (shared
>> resources) between encoder and decoder and they had (for that reason) to be
>> defined has a _single_ variant. And there is no reason you can ship decoder
>> and encoder seperated when you have two variants (with different
>> compatibles).
>> For Rockchip and iMX those files are even containing variants for completly
>> different generations / different SoCs. I had to cleanup this mess for
> The i.MX8M Mini and Plus have different power domains for encoder and
> decoders as well as different clocks.  Keeping them separate would
> almost be necessary.
I guess there is missunderstanding: I didn't say the two STM variants
should be merged in one variant, but the two variants should be within the
same _file_, like the other platforms are doing :)
> adam
>
>> Rockchip once - and it was no fun :) Anyways: It's up to the maintainers I
>> guess - I just wanted to ask if I missunderstand something here.
>>
>> Greetings,
>>
>> Alex
>>
>>> Hoping that this clarify.
>>>
>>> Best regards,
>>> Hugues.
>>>
>>> On 12/21/23 13:40, Alex Bee wrote:
>>>> Hi Hugues, Hi Nicolas,
>>>>
>>>> is there any specific reason I'm not understanding / seeing why this
>>>> is added in two seperate vdec* / venc* files and not a single vpu*
>>>> file? Is it only for the seperate clocks (-names) / irqs (-names) /
>>>> callbacks? Those are defined per variant and perfectly fit in a
>>>> single file holding one vdec and one venc variant.
>>>>
>>>> Alex
>>>>
>>>> Am 21.12.23 um 09:47 schrieb Hugues Fruchet:
>>>>> This patchset introduces support for VDEC video hardware decoder
>>>>> and VENC video hardware encoder of STMicroelectronics STM32MP25
>>>>> SoC series.
>>>>>
>>>>> This initial support implements H264 decoding, VP8 decoding and
>>>>> JPEG encoding.
>>>>>
>>>>> This has been tested on STM32MP257F-EV1 evaluation board.
>>>>>
>>>>> ===========
>>>>> = history =
>>>>> ===========
>>>>> version 5:
>>>>>      - Precise that video decoding as been successfully tested up to
>>>>> full HD
>>>>>      - Add Nicolas Dufresne reviewed-by
>>>>>
>>>>> version 4:
>>>>>      - Fix comments from Nicolas about dropping encoder raw steps
>>>>>
>>>>> version 3:
>>>>>      - Fix remarks from Krzysztof Kozlowski:
>>>>>       - drop "items", we keep simple enum in such case
>>>>>       - drop second example - it is the same as the first
>>>>>      - Drop unused node labels as suggested by Conor Dooley
>>>>>      - Revisit min/max resolutions as suggested by Nicolas Dufresne
>>>>>
>>>>> version 2:
>>>>>      - Fix remarks from Krzysztof Kozlowski on v1:
>>>>>       - single video-codec binding for both VDEC/VENC
>>>>>       - get rid of "-names"
>>>>>       - use of generic node name "video-codec"
>>>>>
>>>>> version 1:
>>>>>     - Initial submission
>>>>>
>>>>> Hugues Fruchet (5):
>>>>>     dt-bindings: media: Document STM32MP25 VDEC & VENC video codecs
>>>>>     media: hantro: add support for STM32MP25 VDEC
>>>>>     media: hantro: add support for STM32MP25 VENC
>>>>>     arm64: dts: st: add video decoder support to stm32mp255
>>>>>     arm64: dts: st: add video encoder support to stm32mp255
>>>>>
>>>>>    .../media/st,stm32mp25-video-codec.yaml       |  50 ++++++++
>>>>>    arch/arm64/boot/dts/st/stm32mp251.dtsi        |  12 ++
>>>>>    arch/arm64/boot/dts/st/stm32mp255.dtsi        |  17 +++
>>>>>    drivers/media/platform/verisilicon/Kconfig    |  14 ++-
>>>>>    drivers/media/platform/verisilicon/Makefile   |   4 +
>>>>>    .../media/platform/verisilicon/hantro_drv.c   |   4 +
>>>>>    .../media/platform/verisilicon/hantro_hw.h    |   2 +
>>>>>    .../platform/verisilicon/stm32mp25_vdec_hw.c  |  92 ++++++++++++++
>>>>>    .../platform/verisilicon/stm32mp25_venc_hw.c  | 115
>>>>> ++++++++++++++++++
>>>>>    9 files changed, 307 insertions(+), 3 deletions(-)
>>>>>    create mode 100644
>>>>> Documentation/devicetree/bindings/media/st,stm32mp25-video-codec.yaml
>>>>>    create mode 100644
>>>>> drivers/media/platform/verisilicon/stm32mp25_vdec_hw.c
>>>>>    create mode 100644
>>>>> drivers/media/platform/verisilicon/stm32mp25_venc_hw.c
>>>>>

Re: [PATCH v5 0/5] Add support for video hardware codec of STMicroelectronics STM32 SoC series

[Hugues FRUCHET]

Hi Alex,

On 12/21/23 14:46, Alex Bee wrote:
> 
> Am 21.12.23 um 14:38 schrieb Adam Ford:
>> On Thu, Dec 21, 2023 at 7:31 AM Alex Bee <knaerzche@gmail.com> wrote:
>>> Hi Hugues,
>>>
>>> Am 21.12.23 um 14:08 schrieb Hugues FRUCHET:
>>>> Hi Alex,
>>>>
>>>> This is because VDEC and VENC are two separated IPs with their own
>>>> hardware resources and no links between both.
>>>> On future SoCs, VDEC can ship on its own, same for VENC.
>>>>
>>> I think that's what the driver is/was designed for :)
>>>
>>> I don't  think there _has_ to be a link between variants in the same 
>>> file.
>>> For Rockchip we only had the issue that there _is_ a link (shared
>>> resources) between encoder and decoder and they had (for that reason) 
>>> to be
>>> defined has a _single_ variant. And there is no reason you can ship 
>>> decoder
>>> and encoder seperated when you have two variants (with different
>>> compatibles).
>>> For Rockchip and iMX those files are even containing variants for 
>>> completly
>>> different generations / different SoCs. I had to cleanup this mess for
>> The i.MX8M Mini and Plus have different power domains for encoder and
>> decoders as well as different clocks.  Keeping them separate would
>> almost be necessary.
> I guess there is missunderstanding: I didn't say the two STM variants
> should be merged in one variant, but the two variants should be within the
> same _file_, like the other platforms are doing :)

I have two separated hardware: VDEC and VENC, not a single block like 
"VPU" for example. So what name should have this file ?
Other platforms had a common file because there was a common block 
embedding both decoder and encoder, sometimes with links/dependencies 
between both.
SAMA5D4 has only a decoder, only a single file called "_vdec_hw.c"...
so it is quite logical for me to have one file per independent IP.

>> adam
>>
>>> Rockchip once - and it was no fun :) Anyways: It's up to the 
>>> maintainers I
>>> guess - I just wanted to ask if I missunderstand something here.
>>>
>>> Greetings,
>>>
>>> Alex
>>>
>>>> Hoping that this clarify.
>>>>
>>>> Best regards,
>>>> Hugues.
>>>>
>>>> On 12/21/23 13:40, Alex Bee wrote:
>>>>> Hi Hugues, Hi Nicolas,
>>>>>
>>>>> is there any specific reason I'm not understanding / seeing why this
>>>>> is added in two seperate vdec* / venc* files and not a single vpu*
>>>>> file? Is it only for the seperate clocks (-names) / irqs (-names) /
>>>>> callbacks? Those are defined per variant and perfectly fit in a
>>>>> single file holding one vdec and one venc variant.
>>>>>
>>>>> Alex
>>>>>
>>>>> Am 21.12.23 um 09:47 schrieb Hugues Fruchet:
>>>>>> This patchset introduces support for VDEC video hardware decoder
>>>>>> and VENC video hardware encoder of STMicroelectronics STM32MP25
>>>>>> SoC series.
>>>>>>
>>>>>> This initial support implements H264 decoding, VP8 decoding and
>>>>>> JPEG encoding.
>>>>>>
>>>>>> This has been tested on STM32MP257F-EV1 evaluation board.
>>>>>>
>>>>>> ===========
>>>>>> = history =
>>>>>> ===========
>>>>>> version 5:
>>>>>>      - Precise that video decoding as been successfully tested up to
>>>>>> full HD
>>>>>>      - Add Nicolas Dufresne reviewed-by
>>>>>>
>>>>>> version 4:
>>>>>>      - Fix comments from Nicolas about dropping encoder raw steps
>>>>>>
>>>>>> version 3:
>>>>>>      - Fix remarks from Krzysztof Kozlowski:
>>>>>>       - drop "items", we keep simple enum in such case
>>>>>>       - drop second example - it is the same as the first
>>>>>>      - Drop unused node labels as suggested by Conor Dooley
>>>>>>      - Revisit min/max resolutions as suggested by Nicolas Dufresne
>>>>>>
>>>>>> version 2:
>>>>>>      - Fix remarks from Krzysztof Kozlowski on v1:
>>>>>>       - single video-codec binding for both VDEC/VENC
>>>>>>       - get rid of "-names"
>>>>>>       - use of generic node name "video-codec"
>>>>>>
>>>>>> version 1:
>>>>>>     - Initial submission
>>>>>>
>>>>>> Hugues Fruchet (5):
>>>>>>     dt-bindings: media: Document STM32MP25 VDEC & VENC video codecs
>>>>>>     media: hantro: add support for STM32MP25 VDEC
>>>>>>     media: hantro: add support for STM32MP25 VENC
>>>>>>     arm64: dts: st: add video decoder support to stm32mp255
>>>>>>     arm64: dts: st: add video encoder support to stm32mp255
>>>>>>
>>>>>>    .../media/st,stm32mp25-video-codec.yaml       |  50 ++++++++
>>>>>>    arch/arm64/boot/dts/st/stm32mp251.dtsi        |  12 ++
>>>>>>    arch/arm64/boot/dts/st/stm32mp255.dtsi        |  17 +++
>>>>>>    drivers/media/platform/verisilicon/Kconfig    |  14 ++-
>>>>>>    drivers/media/platform/verisilicon/Makefile   |   4 +
>>>>>>    .../media/platform/verisilicon/hantro_drv.c   |   4 +
>>>>>>    .../media/platform/verisilicon/hantro_hw.h    |   2 +
>>>>>>    .../platform/verisilicon/stm32mp25_vdec_hw.c  |  92 ++++++++++++++
>>>>>>    .../platform/verisilicon/stm32mp25_venc_hw.c  | 115
>>>>>> ++++++++++++++++++
>>>>>>    9 files changed, 307 insertions(+), 3 deletions(-)
>>>>>>    create mode 100644
>>>>>> Documentation/devicetree/bindings/media/st,stm32mp25-video-codec.yaml
>>>>>>    create mode 100644
>>>>>> drivers/media/platform/verisilicon/stm32mp25_vdec_hw.c
>>>>>>    create mode 100644
>>>>>> drivers/media/platform/verisilicon/stm32mp25_venc_hw.c
>>>>>>

Best regards,
Hugues.

Re: [PATCH v5 0/5] Add support for video hardware codec of STMicroelectronics STM32 SoC series

[Alex Bee]

Hi Hugues,

Am 21.12.23 um 14:55 schrieb Hugues FRUCHET:
> Hi Alex,
>
> On 12/21/23 14:46, Alex Bee wrote:
>>
>> Am 21.12.23 um 14:38 schrieb Adam Ford:
>>> On Thu, Dec 21, 2023 at 7:31 AM Alex Bee <knaerzche@gmail.com> wrote:
>>>> Hi Hugues,
>>>>
>>>> Am 21.12.23 um 14:08 schrieb Hugues FRUCHET:
>>>>> Hi Alex,
>>>>>
>>>>> This is because VDEC and VENC are two separated IPs with their own
>>>>> hardware resources and no links between both.
>>>>> On future SoCs, VDEC can ship on its own, same for VENC.
>>>>>
>>>> I think that's what the driver is/was designed for :)
>>>>
>>>> I don't  think there _has_ to be a link between variants in the 
>>>> same file.
>>>> For Rockchip we only had the issue that there _is_ a link (shared
>>>> resources) between encoder and decoder and they had (for that 
>>>> reason) to be
>>>> defined has a _single_ variant. And there is no reason you can ship 
>>>> decoder
>>>> and encoder seperated when you have two variants (with different
>>>> compatibles).
>>>> For Rockchip and iMX those files are even containing variants for 
>>>> completly
>>>> different generations / different SoCs. I had to cleanup this mess for
>>> The i.MX8M Mini and Plus have different power domains for encoder and
>>> decoders as well as different clocks.  Keeping them separate would
>>> almost be necessary.
>> I guess there is missunderstanding: I didn't say the two STM variants
>> should be merged in one variant, but the two variants should be 
>> within the
>> same _file_, like the other platforms are doing :)
>
> I have two separated hardware: VDEC and VENC, not a single block like 
> "VPU" for example. So what name should have this file ?
> Other platforms had a common file because there was a common block 
> embedding both decoder and encoder, sometimes with links/dependencies 
> between both.
> SAMA5D4 has only a decoder, only a single file called "_vdec_hw.c"...
> so it is quite logical for me to have one file per independent IP.
>
Maybe - but that's not way the driver is currently organzied.
rockchip_vpu_hw.c also holds variants which have only have a decoder and
also some which only have a encoder. So "vpu" is quite neutral, I guess. It
doesn't say anything if it belongs to encoder or decoder.
When I was adding the RK3066 variant a I was even asked to add a explicit
comment, why this integration can't be splitted in encoder and decoder
variant.

We were having a a lot of these split-ups in the early days of hantro
driver and it was no fun to clean them up.

Alex

>>> adam
>>>
>>>> Rockchip once - and it was no fun :) Anyways: It's up to the 
>>>> maintainers I
>>>> guess - I just wanted to ask if I missunderstand something here.
>>>>
>>>> Greetings,
>>>>
>>>> Alex
>>>>
>>>>> Hoping that this clarify.
>>>>>
>>>>> Best regards,
>>>>> Hugues.
>>>>>
>>>>> On 12/21/23 13:40, Alex Bee wrote:
>>>>>> Hi Hugues, Hi Nicolas,
>>>>>>
>>>>>> is there any specific reason I'm not understanding / seeing why this
>>>>>> is added in two seperate vdec* / venc* files and not a single vpu*
>>>>>> file? Is it only for the seperate clocks (-names) / irqs (-names) /
>>>>>> callbacks? Those are defined per variant and perfectly fit in a
>>>>>> single file holding one vdec and one venc variant.
>>>>>>
>>>>>> Alex
>>>>>>
>>>>>> Am 21.12.23 um 09:47 schrieb Hugues Fruchet:
>>>>>>> This patchset introduces support for VDEC video hardware decoder
>>>>>>> and VENC video hardware encoder of STMicroelectronics STM32MP25
>>>>>>> SoC series.
>>>>>>>
>>>>>>> This initial support implements H264 decoding, VP8 decoding and
>>>>>>> JPEG encoding.
>>>>>>>
>>>>>>> This has been tested on STM32MP257F-EV1 evaluation board.
>>>>>>>
>>>>>>> ===========
>>>>>>> = history =
>>>>>>> ===========
>>>>>>> version 5:
>>>>>>>      - Precise that video decoding as been successfully tested 
>>>>>>> up to
>>>>>>> full HD
>>>>>>>      - Add Nicolas Dufresne reviewed-by
>>>>>>>
>>>>>>> version 4:
>>>>>>>      - Fix comments from Nicolas about dropping encoder raw steps
>>>>>>>
>>>>>>> version 3:
>>>>>>>      - Fix remarks from Krzysztof Kozlowski:
>>>>>>>       - drop "items", we keep simple enum in such case
>>>>>>>       - drop second example - it is the same as the first
>>>>>>>      - Drop unused node labels as suggested by Conor Dooley
>>>>>>>      - Revisit min/max resolutions as suggested by Nicolas Dufresne
>>>>>>>
>>>>>>> version 2:
>>>>>>>      - Fix remarks from Krzysztof Kozlowski on v1:
>>>>>>>       - single video-codec binding for both VDEC/VENC
>>>>>>>       - get rid of "-names"
>>>>>>>       - use of generic node name "video-codec"
>>>>>>>
>>>>>>> version 1:
>>>>>>>     - Initial submission
>>>>>>>
>>>>>>> Hugues Fruchet (5):
>>>>>>>     dt-bindings: media: Document STM32MP25 VDEC & VENC video codecs
>>>>>>>     media: hantro: add support for STM32MP25 VDEC
>>>>>>>     media: hantro: add support for STM32MP25 VENC
>>>>>>>     arm64: dts: st: add video decoder support to stm32mp255
>>>>>>>     arm64: dts: st: add video encoder support to stm32mp255
>>>>>>>
>>>>>>>    .../media/st,stm32mp25-video-codec.yaml       |  50 ++++++++
>>>>>>>    arch/arm64/boot/dts/st/stm32mp251.dtsi        |  12 ++
>>>>>>>    arch/arm64/boot/dts/st/stm32mp255.dtsi        |  17 +++
>>>>>>>    drivers/media/platform/verisilicon/Kconfig    |  14 ++-
>>>>>>>    drivers/media/platform/verisilicon/Makefile   |   4 +
>>>>>>>    .../media/platform/verisilicon/hantro_drv.c   |   4 +
>>>>>>>    .../media/platform/verisilicon/hantro_hw.h    |   2 +
>>>>>>>    .../platform/verisilicon/stm32mp25_vdec_hw.c  |  92 
>>>>>>> ++++++++++++++
>>>>>>>    .../platform/verisilicon/stm32mp25_venc_hw.c  | 115
>>>>>>> ++++++++++++++++++
>>>>>>>    9 files changed, 307 insertions(+), 3 deletions(-)
>>>>>>>    create mode 100644
>>>>>>> Documentation/devicetree/bindings/media/st,stm32mp25-video-codec.yaml 
>>>>>>>
>>>>>>>    create mode 100644
>>>>>>> drivers/media/platform/verisilicon/stm32mp25_vdec_hw.c
>>>>>>>    create mode 100644
>>>>>>> drivers/media/platform/verisilicon/stm32mp25_venc_hw.c
>>>>>>>
>
> Best regards,
> Hugues.

Re: [PATCH v5 0/5] Add support for video hardware codec of STMicroelectronics STM32 SoC series

[Hugues FRUCHET]

Hi Alex,

v6 sent with all the variants in a single file.

Best regards,
Hugues.

On 12/21/23 14:31, Alex Bee wrote:
> Hi Hugues,
> 
> Am 21.12.23 um 14:08 schrieb Hugues FRUCHET:
>> Hi Alex,
>>
>> This is because VDEC and VENC are two separated IPs with their own 
>> hardware resources and no links between both.
>> On future SoCs, VDEC can ship on its own, same for VENC.
>>
> I think that's what the driver is/was designed for :)
> 
> I don't  think there _has_ to be a link between variants in the same file.
> For Rockchip we only had the issue that there _is_ a link (shared
> resources) between encoder and decoder and they had (for that reason) to be
> defined has a _single_ variant. And there is no reason you can ship decoder
> and encoder seperated when you have two variants (with different
> compatibles).
> For Rockchip and iMX those files are even containing variants for completly
> different generations / different SoCs. I had to cleanup this mess for
> Rockchip once - and it was no fun :) Anyways: It's up to the maintainers I
> guess - I just wanted to ask if I missunderstand something here.
> 
> Greetings,
> 
> Alex
> 
>> Hoping that this clarify.
>>
>> Best regards,
>> Hugues.
>>
>> On 12/21/23 13:40, Alex Bee wrote:
>>> Hi Hugues, Hi Nicolas,
>>>
>>> is there any specific reason I'm not understanding / seeing why this 
>>> is added in two seperate vdec* / venc* files and not a single vpu* 
>>> file? Is it only for the seperate clocks (-names) / irqs (-names) / 
>>> callbacks? Those are defined per variant and perfectly fit in a 
>>> single file holding one vdec and one venc variant.
>>>
>>> Alex
>>>
>>> Am 21.12.23 um 09:47 schrieb Hugues Fruchet:
>>>> This patchset introduces support for VDEC video hardware decoder
>>>> and VENC video hardware encoder of STMicroelectronics STM32MP25
>>>> SoC series.
>>>>
>>>> This initial support implements H264 decoding, VP8 decoding and
>>>> JPEG encoding.
>>>>
>>>> This has been tested on STM32MP257F-EV1 evaluation board.
>>>>
>>>> ===========
>>>> = history =
>>>> ===========
>>>> version 5:
>>>>     - Precise that video decoding as been successfully tested up to 
>>>> full HD
>>>>     - Add Nicolas Dufresne reviewed-by
>>>>
>>>> version 4:
>>>>     - Fix comments from Nicolas about dropping encoder raw steps
>>>>
>>>> version 3:
>>>>     - Fix remarks from Krzysztof Kozlowski:
>>>>      - drop "items", we keep simple enum in such case
>>>>      - drop second example - it is the same as the first
>>>>     - Drop unused node labels as suggested by Conor Dooley
>>>>     - Revisit min/max resolutions as suggested by Nicolas Dufresne
>>>>
>>>> version 2:
>>>>     - Fix remarks from Krzysztof Kozlowski on v1:
>>>>      - single video-codec binding for both VDEC/VENC
>>>>      - get rid of "-names"
>>>>      - use of generic node name "video-codec"
>>>>
>>>> version 1:
>>>>    - Initial submission
>>>>
>>>> Hugues Fruchet (5):
>>>>    dt-bindings: media: Document STM32MP25 VDEC & VENC video codecs
>>>>    media: hantro: add support for STM32MP25 VDEC
>>>>    media: hantro: add support for STM32MP25 VENC
>>>>    arm64: dts: st: add video decoder support to stm32mp255
>>>>    arm64: dts: st: add video encoder support to stm32mp255
>>>>
>>>>   .../media/st,stm32mp25-video-codec.yaml       |  50 ++++++++
>>>>   arch/arm64/boot/dts/st/stm32mp251.dtsi        |  12 ++
>>>>   arch/arm64/boot/dts/st/stm32mp255.dtsi        |  17 +++
>>>>   drivers/media/platform/verisilicon/Kconfig    |  14 ++-
>>>>   drivers/media/platform/verisilicon/Makefile   |   4 +
>>>>   .../media/platform/verisilicon/hantro_drv.c   |   4 +
>>>>   .../media/platform/verisilicon/hantro_hw.h    |   2 +
>>>>   .../platform/verisilicon/stm32mp25_vdec_hw.c  |  92 ++++++++++++++
>>>>   .../platform/verisilicon/stm32mp25_venc_hw.c  | 115 
>>>> ++++++++++++++++++
>>>>   9 files changed, 307 insertions(+), 3 deletions(-)
>>>>   create mode 100644 
>>>> Documentation/devicetree/bindings/media/st,stm32mp25-video-codec.yaml
>>>>   create mode 100644 
>>>> drivers/media/platform/verisilicon/stm32mp25_vdec_hw.c
>>>>   create mode 100644 
>>>> drivers/media/platform/verisilicon/stm32mp25_venc_hw.c
>>>>
>>>