[RFC PATCH 00/11] VC8000E H.264 V4L2 Stateless Encoder

[RFC PATCH 00/11] VC8000E H.264 V4L2 Stateless Encoder

[Marco Felsch]

Hi all,

this RFC implements the kernel V4L2 stateless encoding counter part for
the Verisilicon VC8000E encoder. The encoder is capable of H.264 and
H.265 encoding and can be found on several SoCs e.g. NXP i.MX8MP.

The RFC is based on Pauls initial attempts [1].

This patchset is still in a *very very* early state since the uAPI handling
still needs to be figured out. It's clearly not for productive use yet!
The only reason of publishing the driver in this early state is to align
with other developers also working on V4L2 stateless H.264 encoding.

That said, paired with the GStreamer userspace [2] the driver is capable
of:
 * H.264 encoding
 * I/P frame handling
 * Arbitrary frame sizes
 * YUV420M input

Note: Be aware that the GStreamer element [2] is in a *very* early state
too, so don't expect to much. There are limitations like: the element
requires to work on its own buffers, so there is no fast-path and
always a copy involved.

@DT folks
The dt-bindings are missing yet.

[1] https://github.com/bootlin/linux/tree/hantro/h264-encoding-v5.11
[2] https://gitlab.freedesktop.org/dude/gstreamer/-/tree/h264-stateless-encoder

Regards,
  Marco

Marco Felsch (7):
  arm64: dts: imx8mp: drop gpcv2 vpu power-domains and clocks
  arm64: dts: imx8mp: add VC8000E encoder node
  arm64: dts: imx8mp: fix VPU_BUS clock setting
  media: hantro: use hantro_decoded_buffer only for dst_vq
  media: verisilicon: add H264 encoder support
  media: verisilicon: split read/write debug
  media: hantro: add support for i.MX8MP VC8000E

Michael Tretter (3):
  media: uapi: add documentation for the V4L2 H.264 stateless encoding
    API
  media: uapi: add nal unit header fields to encode_params
  media: uapi: add more V4L2_H264_ENCODE_FLAGs

Paul Kocialkowski (1):
  media: Introduce Hantro V4L2 H.264 stateless encoding API

-- 
2.39.5

[RFC PATCH 01/11] media: Introduce Hantro V4L2 H.264 stateless encoding API

[Marco Felsch]

From: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 drivers/media/v4l2-core/v4l2-ctrls-core.c | 23 ++++++++++
 drivers/media/v4l2-core/v4l2-ctrls-defs.c | 12 +++++
 include/uapi/linux/v4l2-controls.h        | 55 +++++++++++++++++++++++
 include/uapi/linux/videodev2.h            |  3 ++
 4 files changed, 93 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-ctrls-core.c b/drivers/media/v4l2-core/v4l2-ctrls-core.c
index eeab6a5eb7ba..b52d04ba8fd4 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls-core.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls-core.c
@@ -316,6 +316,15 @@ void v4l2_ctrl_type_op_log(const struct v4l2_ctrl *ctrl)
 	case V4L2_CTRL_TYPE_H264_PRED_WEIGHTS:
 		pr_cont("H264_PRED_WEIGHTS");
 		break;
+	case V4L2_CTRL_TYPE_H264_ENCODE_PARAMS:
+		pr_cont("H264_ENCODE_PARAMS");
+		break;
+	case V4L2_CTRL_TYPE_H264_ENCODE_RC:
+		pr_cont("H264_ENCODE_RC");
+		break;
+	case V4L2_CTRL_TYPE_H264_ENCODE_FEEDBACK:
+		pr_cont("H264_ENCODE_FEEDBACK");
+		break;
 	case V4L2_CTRL_TYPE_FWHT_PARAMS:
 		pr_cont("FWHT_PARAMS");
 		break;
@@ -1031,6 +1040,11 @@ static int std_validate_compound(const struct v4l2_ctrl *ctrl, u32 idx,
 		zero_reserved(*p_h264_dec_params);
 		break;
 
+	case V4L2_CTRL_TYPE_H264_ENCODE_PARAMS:
+	case V4L2_CTRL_TYPE_H264_ENCODE_RC:
+	case V4L2_CTRL_TYPE_H264_ENCODE_FEEDBACK:
+		break;
+
 	case V4L2_CTRL_TYPE_VP8_FRAME:
 		p_vp8_frame = p;
 
@@ -1827,6 +1841,15 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
 	case V4L2_CTRL_TYPE_H264_PRED_WEIGHTS:
 		elem_size = sizeof(struct v4l2_ctrl_h264_pred_weights);
 		break;
+	case V4L2_CTRL_TYPE_H264_ENCODE_PARAMS:
+		elem_size = sizeof(struct v4l2_ctrl_h264_encode_params);
+		break;
+	case V4L2_CTRL_TYPE_H264_ENCODE_RC:
+		elem_size = sizeof(struct v4l2_ctrl_h264_encode_rc);
+		break;
+	case V4L2_CTRL_TYPE_H264_ENCODE_FEEDBACK:
+		elem_size = sizeof(struct v4l2_ctrl_h264_encode_feedback);
+		break;
 	case V4L2_CTRL_TYPE_VP8_FRAME:
 		elem_size = sizeof(struct v4l2_ctrl_vp8_frame);
 		break;
diff --git a/drivers/media/v4l2-core/v4l2-ctrls-defs.c b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
index 1ea52011247a..6cc53842778c 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls-defs.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
@@ -1218,6 +1218,9 @@ const char *v4l2_ctrl_get_name(u32 id)
 	case V4L2_CID_STATELESS_H264_PRED_WEIGHTS:		return "H264 Prediction Weight Table";
 	case V4L2_CID_STATELESS_H264_SLICE_PARAMS:		return "H264 Slice Parameters";
 	case V4L2_CID_STATELESS_H264_DECODE_PARAMS:		return "H264 Decode Parameters";
+	case V4L2_CID_STATELESS_H264_ENCODE_PARAMS:		return "H264 Encode Parameters";
+	case V4L2_CID_STATELESS_H264_ENCODE_RC:			return "H264 Encode Rate-Control";
+	case V4L2_CID_STATELESS_H264_ENCODE_FEEDBACK:		return "H264 Encode Feedback";
 	case V4L2_CID_STATELESS_FWHT_PARAMS:			return "FWHT Stateless Parameters";
 	case V4L2_CID_STATELESS_VP8_FRAME:			return "VP8 Frame Parameters";
 	case V4L2_CID_STATELESS_MPEG2_SEQUENCE:			return "MPEG-2 Sequence Header";
@@ -1555,6 +1558,15 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 	case V4L2_CID_STATELESS_H264_PRED_WEIGHTS:
 		*type = V4L2_CTRL_TYPE_H264_PRED_WEIGHTS;
 		break;
+	case V4L2_CID_STATELESS_H264_ENCODE_PARAMS:
+		*type = V4L2_CTRL_TYPE_H264_ENCODE_PARAMS;
+		break;
+	case V4L2_CID_STATELESS_H264_ENCODE_RC:
+		*type = V4L2_CTRL_TYPE_H264_ENCODE_RC;
+		break;
+	case V4L2_CID_STATELESS_H264_ENCODE_FEEDBACK:
+		*type = V4L2_CTRL_TYPE_H264_ENCODE_FEEDBACK;
+		break;
 	case V4L2_CID_STATELESS_VP8_FRAME:
 		*type = V4L2_CTRL_TYPE_VP8_FRAME;
 		break;
diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
index 974fd254e573..4f6b37513edc 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -1694,6 +1694,61 @@ struct v4l2_ctrl_h264_decode_params {
 	__u32 flags;
 };
 
+#define V4L2_CID_STATELESS_H264_ENCODE_PARAMS	(V4L2_CID_CODEC_STATELESS_BASE + 8)
+
+struct v4l2_ctrl_h264_encode_params {
+	/* Slice parameters */
+
+	__u8 slice_type;
+	__u8 pic_parameter_set_id;
+	__u16 frame_num;
+	__u16 idr_pic_id;
+	__u8 cabac_init_idc;
+	__u8 disable_deblocking_filter_idc;
+	__s8 slice_alpha_c0_offset_div2;
+	__s8 slice_beta_offset_div2;
+
+	__s32 slice_size_mb_rows;
+
+	/* PPS parameters */
+
+	__s8 pic_init_qp_minus26;
+	__s8 chroma_qp_index_offset;
+
+	__u32 flags; /* V4L2_H264_ENCODE_FLAG_ */
+
+	/* Reference */
+
+	__u64 reference_ts;
+};
+
+#define V4L2_H264_ENCODE_FLAG_ENTROPY_CODING_MODE	0x01
+#define V4L2_H264_ENCODE_FLAG_TRANSFORM_8X8_MODE	0x02
+#define V4L2_H264_ENCODE_FLAG_CONSTRAINED_INTRA_PRED	0x04
+
+#define V4L2_CID_STATELESS_H264_ENCODE_RC	(V4L2_CID_CODEC_STATELESS_BASE + 9)
+
+struct v4l2_ctrl_h264_encode_rc {
+	__u32 qp;
+	__u32 qp_min;
+	__u32 qp_max;
+	__s32 mad_qp_delta;
+	__u32 mad_threshold;
+
+	__u32 cp_distance_mbs;
+	__u32 cp_target[10];
+	__s32 cp_target_error[6];
+	__s32 cp_qp_delta[7];
+};
+
+#define V4L2_CID_STATELESS_H264_ENCODE_FEEDBACK	(V4L2_CID_CODEC_STATELESS_BASE + 10)
+
+struct v4l2_ctrl_h264_encode_feedback {
+	__u32 qp_sum;
+	__u32 cp[10];
+	__u32 mad_count;
+	__u32 rlc_count;
+};
 
 /* Stateless FWHT control, used by the vicodec driver */
 
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index e7c4dce39007..9962532d32e3 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -1939,6 +1939,9 @@ enum v4l2_ctrl_type {
 	V4L2_CTRL_TYPE_H264_SLICE_PARAMS    = 0x0203,
 	V4L2_CTRL_TYPE_H264_DECODE_PARAMS   = 0x0204,
 	V4L2_CTRL_TYPE_H264_PRED_WEIGHTS    = 0x0205,
+	V4L2_CTRL_TYPE_H264_ENCODE_PARAMS   = 0x0206,
+	V4L2_CTRL_TYPE_H264_ENCODE_RC       = 0x0207,
+	V4L2_CTRL_TYPE_H264_ENCODE_FEEDBACK = 0x0208,
 
 	V4L2_CTRL_TYPE_FWHT_PARAMS	    = 0x0220,
 
-- 
2.39.5

[RFC PATCH 02/11] media: uapi: add documentation for the V4L2 H.264 stateless encoding API

[Marco Felsch]

From: Michael Tretter <m.tretter@pengutronix.de>

Document the parameters that are passed to the stateless encoding API
since they need to be filled by user space and knowing what the
parameters do helps when filling it.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
 include/uapi/linux/v4l2-controls.h | 46 ++++++++++++++++++++++++++++--
 1 file changed, 44 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
index 4f6b37513edc..670f822ee758 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -1696,6 +1696,32 @@ struct v4l2_ctrl_h264_decode_params {
 
 #define V4L2_CID_STATELESS_H264_ENCODE_PARAMS	(V4L2_CID_CODEC_STATELESS_BASE + 8)
 
+/**
+ * struct v4l2_ctrl_h264_encode_params - H264 encoding parameters
+ *
+ * @slice_type: selects slice type. Set to one of V4L2_H264_SLICE_TYPE_{}
+ * @pic_parameter_set_id: identifies the picture parameter set that is referred to
+ * in the slice header. The value shall be in the range of 0 to 255, inclusive.
+ * @frame_num: an identifier for pictures.
+ * @idr_pic_id: identifies an IDR picture.
+ * @cabac_init_idc: index for determining the initialization table used in the
+ * initialization process for context variables. The value of cabac_init_idc
+ * shall be in the range of 0 to 2, inclusive.
+ * @disable_deblocking_filter_idc: specifies whether the operation of the
+ * deblocking filter shall be disabled across some block edges of the slice and
+ * specifies for which edges the filtering is disabled.
+ * @slice_alpha_c0_offset_div2: offset used in accessing the alpha and tC0
+ * deblocking filter tables for filtering operations controlled by the macroblocks
+ * within the slice.
+ * @slice_beta_offset_div2: offset used in accessing the beta deblocking filter
+ * table for filtering operations controlled by the macroblocks within the slice.
+ * @slice_size_mb_rows: number of macroblock rows in a slice.
+ * @pic_init_qp_minus26: initial value minus 26 of luma qp for each slice.
+ * @chroma_qp_index_offset: offset that shall be added to qp luma for addressing the
+ * table of qp chroma values for the Cb chroma component.
+ * @flags: combination of V4L2_H264_ENCODE_FLAG_{} flags.
+ * @reference_ts: timestamp of the V4L2 buffer to use as reference
+ */
 struct v4l2_ctrl_h264_encode_params {
 	/* Slice parameters */
 
@@ -1710,8 +1736,16 @@ struct v4l2_ctrl_h264_encode_params {
 
 	__s32 slice_size_mb_rows;
 
-	/* PPS parameters */
-
+	/*
+	 * PPS parameters
+	 *
+	 * TODO Duplicating the PPS in the encode_params may not be necessary,
+	 * if the PPS are set via separate control. Otherwise, it may be useful
+	 * to just use struct v4l2_ctrl_h264_pps here.
+	 *
+	 * Needs to be consistent with the values set in the PPS referenced by
+	 * pic_parameter_set_id.
+	 */
 	__s8 pic_init_qp_minus26;
 	__s8 chroma_qp_index_offset;
 
@@ -1728,6 +1762,14 @@ struct v4l2_ctrl_h264_encode_params {
 
 #define V4L2_CID_STATELESS_H264_ENCODE_RC	(V4L2_CID_CODEC_STATELESS_BASE + 9)
 
+/**
+ * struct v4l2_ctrl_h264_encode_rc
+ *
+ * @qp: quantization parameter for the currently encoded slice
+ *
+ * TODO Setting the QP is enough for implementing const QP, but probably the
+ * entire rate control mechanism has to be reworked.
+ */
 struct v4l2_ctrl_h264_encode_rc {
 	__u32 qp;
 	__u32 qp_min;
-- 
2.39.5

[RFC PATCH 03/11] media: uapi: add nal unit header fields to encode_params

[Marco Felsch]

From: Michael Tretter <m.tretter@pengutronix.de>

The VEPU540 and VEPU580 may prepare the NAL unit headers in the coded
params. These values have to be provided by user space and be written
into the hardware registers.

Furthermore, nal_ref_idc indicates if a picture will be used as
reference and is a hint to the driver, if it needs to keep the
reconstructed buffer or not.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
 include/uapi/linux/v4l2-controls.h | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
index 670f822ee758..a8df393c9ead 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -1694,6 +1694,9 @@ struct v4l2_ctrl_h264_decode_params {
 	__u32 flags;
 };
 
+#define V4L2_H264_NAL_CODED_SLICE_NON_IDR_PIC	1
+#define V4L2_H264_NAL_CODED_SLICE_IDR_PIC	5
+
 #define V4L2_CID_STATELESS_H264_ENCODE_PARAMS	(V4L2_CID_CODEC_STATELESS_BASE + 8)
 
 /**
@@ -1719,6 +1722,8 @@ struct v4l2_ctrl_h264_decode_params {
  * @pic_init_qp_minus26: initial value minus 26 of luma qp for each slice.
  * @chroma_qp_index_offset: offset that shall be added to qp luma for addressing the
  * table of qp chroma values for the Cb chroma component.
+ * @nal_ref_idc: nal_ref_idc for the header of the generated NAL unit
+ * @nal_unit_type: one of the V4L2_H264_NAL_CODED_SLICE_{} values
  * @flags: combination of V4L2_H264_ENCODE_FLAG_{} flags.
  * @reference_ts: timestamp of the V4L2 buffer to use as reference
  */
@@ -1751,6 +1756,16 @@ struct v4l2_ctrl_h264_encode_params {
 
 	__u32 flags; /* V4L2_H264_ENCODE_FLAG_ */
 
+	/*
+	 * If nal_ref_idc is 0, the NAL unit won't be used as reference by
+	 * later NAL units. Any other value indicates that the NAL unit may be
+	 * used as reference.
+	 */
+	__u8 nal_ref_idc;
+
+	/* TODO Can we infer the nal_unit_type from the slice_type? */
+	__u8 nal_unit_type;
+
 	/* Reference */
 
 	__u64 reference_ts;
-- 
2.39.5

[RFC PATCH 04/11] media: uapi: add more V4L2_H264_ENCODE_FLAGs

[Marco Felsch]

From: Michael Tretter <m.tretter@pengutronix.de>

The encode flags may be written into the slice header by the hardware
encoder. User space decides if the flags are set and which features are
used, but the driver has to write it into the registers.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
 include/uapi/linux/v4l2-controls.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
index a8df393c9ead..152b6b0cfe78 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -1774,6 +1774,9 @@ struct v4l2_ctrl_h264_encode_params {
 #define V4L2_H264_ENCODE_FLAG_ENTROPY_CODING_MODE	0x01
 #define V4L2_H264_ENCODE_FLAG_TRANSFORM_8X8_MODE	0x02
 #define V4L2_H264_ENCODE_FLAG_CONSTRAINED_INTRA_PRED	0x04
+#define V4L2_H264_ENCODE_FLAG_NUM_REF_IDX_OVERRIDE	0x08
+#define V4L2_H264_ENCODE_FLAG_NO_OUTPUT_OF_PRIOR_PICS	0x10
+#define V4L2_H264_ENCODE_FLAG_LONG_TERM_REFERENCE	0x20
 
 #define V4L2_CID_STATELESS_H264_ENCODE_RC	(V4L2_CID_CODEC_STATELESS_BASE + 9)
 
-- 
2.39.5

[RFC PATCH 05/11] arm64: dts: imx8mp: drop gpcv2 vpu power-domains and clocks

[Marco Felsch]

The GPCv2 G1, G2 and VC8000E power-domain don't need to reference the
VPUMIX power-domain nor their module clocks since the power and reset
handling is done by the VPUMIX blkctrl driver.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 arch/arm64/boot/dts/freescale/imx8mp.dtsi | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
index e0d3b8cba221..cf9b6c487bd5 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
@@ -879,24 +879,17 @@ pgc_mediamix: power-domain@10 {
 
 					pgc_vpu_g1: power-domain@11 {
 						#power-domain-cells = <0>;
-						power-domains = <&pgc_vpumix>;
 						reg = <IMX8MP_POWER_DOMAIN_VPU_G1>;
-						clocks = <&clk IMX8MP_CLK_VPU_G1_ROOT>;
 					};
 
 					pgc_vpu_g2: power-domain@12 {
 						#power-domain-cells = <0>;
-						power-domains = <&pgc_vpumix>;
 						reg = <IMX8MP_POWER_DOMAIN_VPU_G2>;
-						clocks = <&clk IMX8MP_CLK_VPU_G2_ROOT>;
-
 					};
 
 					pgc_vpu_vc8000e: power-domain@13 {
 						#power-domain-cells = <0>;
-						power-domains = <&pgc_vpumix>;
 						reg = <IMX8MP_POWER_DOMAIN_VPU_VC8000E>;
-						clocks = <&clk IMX8MP_CLK_VPU_VC8KE_ROOT>;
 					};
 
 					pgc_hdmimix: power-domain@14 {
-- 
2.39.5

[RFC PATCH 06/11] arm64: dts: imx8mp: add VC8000E encoder node

[Marco Felsch]

Add support for the Versilicon VC8000E multi-codec stateless encoder.
The IP integrated on the i.MX8MP supports H.264 and H.265 encoding.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 arch/arm64/boot/dts/freescale/imx8mp.dtsi | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
index cf9b6c487bd5..97b09b647ec7 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
@@ -2267,6 +2267,17 @@ vpu_g2: video-codec@38310000 {
 			power-domains = <&vpumix_blk_ctrl IMX8MP_VPUBLK_PD_G2>;
 		};
 
+		vpu_vc8000e: video-codec@38320000 {
+			compatible = "nxp,imx8mp-vpu-vc8000e";
+			reg = <0x38320000 0x10000>;
+			interrupts = <GIC_SPI 30 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&clk IMX8MP_CLK_VPU_VC8KE_ROOT>;
+			assigned-clocks = <&clk IMX8MP_CLK_VPU_VC8000E>;
+			assigned-clock-parents = <&clk IMX8MP_SYS_PLL1_800M>;
+			assigned-clock-rates = <400000000>;
+			power-domains = <&vpumix_blk_ctrl IMX8MP_VPUBLK_PD_VC8000E>;
+		};
+
 		vpumix_blk_ctrl: blk-ctrl@38330000 {
 			compatible = "fsl,imx8mp-vpu-blk-ctrl", "syscon";
 			reg = <0x38330000 0x100>;
-- 
2.39.5

[RFC PATCH 07/11] arm64: dts: imx8mp: fix VPU_BUS clock setting

[Marco Felsch]

The VPU_PLL clock must be set before the VPU_BUS clock which is derived
from the VPU_PLL clock else the VPU_BUS clock is 300MHz and not 600MHz.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 arch/arm64/boot/dts/freescale/imx8mp.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
index 97b09b647ec7..7f4bdefb3480 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
@@ -2289,8 +2289,8 @@ vpumix_blk_ctrl: blk-ctrl@38330000 {
 				 <&clk IMX8MP_CLK_VPU_G2_ROOT>,
 				 <&clk IMX8MP_CLK_VPU_VC8KE_ROOT>;
 			clock-names = "g1", "g2", "vc8000e";
-			assigned-clocks = <&clk IMX8MP_CLK_VPU_BUS>, <&clk IMX8MP_VPU_PLL>;
-			assigned-clock-parents = <&clk IMX8MP_VPU_PLL_OUT>;
+			assigned-clocks = <&clk IMX8MP_VPU_PLL>, <&clk IMX8MP_CLK_VPU_BUS>;
+			assigned-clock-parents = <0>, <&clk IMX8MP_VPU_PLL_OUT>;
 			assigned-clock-rates = <600000000>, <600000000>;
 			interconnects = <&noc IMX8MP_ICM_VPU_G1 &noc IMX8MP_ICN_VIDEO>,
 					<&noc IMX8MP_ICM_VPU_G2 &noc IMX8MP_ICN_VIDEO>,
-- 
2.39.5

[RFC PATCH 08/11] media: hantro: use hantro_decoded_buffer only for dst_vq

[Marco Felsch]

The dst_vq buffer size for encoders should not use the size of the
'hantro_decoded_buffer'. Make use of 'v4l2_m2m_buffer' instead till some
encoder requires pre buffer extra data.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 drivers/media/platform/verisilicon/hantro_drv.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/verisilicon/hantro_drv.c b/drivers/media/platform/verisilicon/hantro_drv.c
index 8542238e0fb1..e4850ae4e8b8 100644
--- a/drivers/media/platform/verisilicon/hantro_drv.c
+++ b/drivers/media/platform/verisilicon/hantro_drv.c
@@ -244,7 +244,10 @@ queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq)
 	dst_vq->io_modes = VB2_MMAP | VB2_DMABUF;
 	dst_vq->drv_priv = ctx;
 	dst_vq->ops = &hantro_queue_ops;
-	dst_vq->buf_struct_size = sizeof(struct hantro_decoded_buffer);
+	if (ctx->is_encoder)
+		dst_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
+	else
+		dst_vq->buf_struct_size = sizeof(struct hantro_decoded_buffer);
 	dst_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
 	dst_vq->lock = &ctx->dev->vpu_mutex;
 	dst_vq->dev = ctx->dev->v4l2_dev.dev;
-- 
2.39.5

[RFC PATCH 09/11] media: verisilicon: add H264 encoder support

[Marco Felsch]

Add the required HANTRO_MODE_H264_ENC enum to handle Hantro H264
stateless encoders.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 drivers/media/platform/verisilicon/hantro.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/platform/verisilicon/hantro.h b/drivers/media/platform/verisilicon/hantro.h
index edc217eed293..e464bec47b11 100644
--- a/drivers/media/platform/verisilicon/hantro.h
+++ b/drivers/media/platform/verisilicon/hantro.h
@@ -107,6 +107,7 @@ struct hantro_variant {
  * enum hantro_codec_mode - codec operating mode.
  * @HANTRO_MODE_NONE:  No operating mode. Used for RAW video formats.
  * @HANTRO_MODE_JPEG_ENC: JPEG encoder.
+ * @HANTRO_MODE_H264_ENC: H264 encoder.
  * @HANTRO_MODE_H264_DEC: H264 decoder.
  * @HANTRO_MODE_MPEG2_DEC: MPEG-2 decoder.
  * @HANTRO_MODE_VP8_DEC: VP8 decoder.
@@ -117,6 +118,7 @@ struct hantro_variant {
 enum hantro_codec_mode {
 	HANTRO_MODE_NONE = -1,
 	HANTRO_MODE_JPEG_ENC,
+	HANTRO_MODE_H264_ENC,
 	HANTRO_MODE_H264_DEC,
 	HANTRO_MODE_MPEG2_DEC,
 	HANTRO_MODE_VP8_DEC,
-- 
2.39.5

[RFC PATCH 10/11] media: verisilicon: split read/write debug

[Marco Felsch]

Allow a more fine grain read/write debug control. This is very useful in
case of hantro_reg_write() is used to not interfere the write debug
prints with read debug prints.

While on it, make the reg/swreg value clear for new developers.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 drivers/media/platform/verisilicon/hantro.h | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/media/platform/verisilicon/hantro.h b/drivers/media/platform/verisilicon/hantro.h
index e464bec47b11..7b031df5b0a1 100644
--- a/drivers/media/platform/verisilicon/hantro.h
+++ b/drivers/media/platform/verisilicon/hantro.h
@@ -367,7 +367,8 @@ struct hantro_decoded_buffer {
  * bit 3 - contents of big controls from userspace
  * bit 4 - detail fmt, ctrl, buffer q/dq information
  * bit 5 - detail function enter/leave trace information
- * bit 6 - register write/read information
+ * bit 6 - register write information
+ * bit 7 - register read information
  */
 extern int hantro_debug;
 
@@ -391,13 +392,13 @@ static __always_inline struct hantro_ctx *fh_to_ctx(struct v4l2_fh *fh)
 static __always_inline void vepu_write_relaxed(struct hantro_dev *vpu,
 					       u32 val, u32 reg)
 {
-	vpu_debug(6, "0x%04x = 0x%08x\n", reg / 4, val);
+	vpu_debug(6, "0x%04x (swreg%u) = 0x%08x\n", reg, reg / 4, val);
 	writel_relaxed(val, vpu->enc_base + reg);
 }
 
 static __always_inline void vepu_write(struct hantro_dev *vpu, u32 val, u32 reg)
 {
-	vpu_debug(6, "0x%04x = 0x%08x\n", reg / 4, val);
+	vpu_debug(6, "0x%04x (swreg%u) = 0x%08x\n", reg, reg / 4, val);
 	writel(val, vpu->enc_base + reg);
 }
 
@@ -405,20 +406,20 @@ static __always_inline u32 vepu_read(struct hantro_dev *vpu, u32 reg)
 {
 	u32 val = readl(vpu->enc_base + reg);
 
-	vpu_debug(6, "0x%04x = 0x%08x\n", reg / 4, val);
+	vpu_debug(7, "0x%04x (swreg%u) = 0x%08x\n", reg, reg / 4, val);
 	return val;
 }
 
 static __always_inline void vdpu_write_relaxed(struct hantro_dev *vpu,
 					       u32 val, u32 reg)
 {
-	vpu_debug(6, "0x%04x = 0x%08x\n", reg / 4, val);
+	vpu_debug(6, "0x%04x (swreg%u) = 0x%08x\n", reg, reg / 4, val);
 	writel_relaxed(val, vpu->dec_base + reg);
 }
 
 static __always_inline void vdpu_write(struct hantro_dev *vpu, u32 val, u32 reg)
 {
-	vpu_debug(6, "0x%04x = 0x%08x\n", reg / 4, val);
+	vpu_debug(6, "0x%04x (swreg%u) = 0x%08x\n", reg, reg / 4, val);
 	writel(val, vpu->dec_base + reg);
 }
 
@@ -433,7 +434,7 @@ static __always_inline u32 vdpu_read(struct hantro_dev *vpu, u32 reg)
 {
 	u32 val = readl(vpu->dec_base + reg);
 
-	vpu_debug(6, "0x%04x = 0x%08x\n", reg / 4, val);
+	vpu_debug(7, "0x%04x (swreg%u) = 0x%08x\n", reg, reg / 4, val);
 	return val;
 }
 
-- 
2.39.5

Re: [RFC PATCH 05/11] arm64: dts: imx8mp: drop gpcv2 vpu power-domains and clocks

[Adam Ford]

On Fri, May 2, 2025 at 10:10 AM Marco Felsch <m.felsch@pengutronix.de> wrote:
>
> The GPCv2 G1, G2 and VC8000E power-domain don't need to reference the
> VPUMIX power-domain nor their module clocks since the power and reset
> handling is done by the VPUMIX blkctrl driver.
>
It was my understanding that having this dependency ensures the order
of the bring-up, but maybe I am wrong.  Do you know if the 8MP
suspend-resume works properly?


Should this get a fixes tag?

> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
>  arch/arm64/boot/dts/freescale/imx8mp.dtsi | 7 -------
>  1 file changed, 7 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> index e0d3b8cba221..cf9b6c487bd5 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> @@ -879,24 +879,17 @@ pgc_mediamix: power-domain@10 {
>
>                                         pgc_vpu_g1: power-domain@11 {
>                                                 #power-domain-cells = <0>;
> -                                               power-domains = <&pgc_vpumix>;
>                                                 reg = <IMX8MP_POWER_DOMAIN_VPU_G1>;
> -                                               clocks = <&clk IMX8MP_CLK_VPU_G1_ROOT>;
>                                         };
>
>                                         pgc_vpu_g2: power-domain@12 {
>                                                 #power-domain-cells = <0>;
> -                                               power-domains = <&pgc_vpumix>;
>                                                 reg = <IMX8MP_POWER_DOMAIN_VPU_G2>;
> -                                               clocks = <&clk IMX8MP_CLK_VPU_G2_ROOT>;
> -
>                                         };
>
>                                         pgc_vpu_vc8000e: power-domain@13 {
>                                                 #power-domain-cells = <0>;
> -                                               power-domains = <&pgc_vpumix>;
>                                                 reg = <IMX8MP_POWER_DOMAIN_VPU_VC8000E>;
> -                                               clocks = <&clk IMX8MP_CLK_VPU_VC8KE_ROOT>;
>                                         };
>
>                                         pgc_hdmimix: power-domain@14 {
> --
> 2.39.5
>
>

Re: [RFC PATCH 03/11] media: uapi: add nal unit header fields to encode_params

[Nicolas Dufresne]

Hi,

Le vendredi 02 mai 2025 à 17:05 +0200, Marco Felsch a écrit :
> From: Michael Tretter <m.tretter@pengutronix.de>
> 
> The VEPU540 and VEPU580 may prepare the NAL unit headers in the coded
> params. These values have to be provided by user space and be written
> into the hardware registers.

To be reworked in future version. This commit message refers to RK356x and
RK3688 encoders block from Rockchip. Though, its possible this applies for
Hantro IP too.

Nicolas
 
> Furthermore, nal_ref_idc indicates if a picture will be used as
> reference and is a hint to the driver, if it needs to keep the
> reconstructed buffer or not.
> 
> Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> ---
>  include/uapi/linux/v4l2-controls.h | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> index 670f822ee758..a8df393c9ead 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -1694,6 +1694,9 @@ struct v4l2_ctrl_h264_decode_params {
>  	__u32 flags;
>  };
>  
> +#define V4L2_H264_NAL_CODED_SLICE_NON_IDR_PIC	1
> +#define V4L2_H264_NAL_CODED_SLICE_IDR_PIC	5
> +
>  #define V4L2_CID_STATELESS_H264_ENCODE_PARAMS	(V4L2_CID_CODEC_STATELESS_BASE + 8)
>  
>  /**
> @@ -1719,6 +1722,8 @@ struct v4l2_ctrl_h264_decode_params {
>   * @pic_init_qp_minus26: initial value minus 26 of luma qp for each slice.
>   * @chroma_qp_index_offset: offset that shall be added to qp luma for addressing the
>   * table of qp chroma values for the Cb chroma component.
> + * @nal_ref_idc: nal_ref_idc for the header of the generated NAL unit
> + * @nal_unit_type: one of the V4L2_H264_NAL_CODED_SLICE_{} values
>   * @flags: combination of V4L2_H264_ENCODE_FLAG_{} flags.
>   * @reference_ts: timestamp of the V4L2 buffer to use as reference
>   */
> @@ -1751,6 +1756,16 @@ struct v4l2_ctrl_h264_encode_params {
>  
>  	__u32 flags; /* V4L2_H264_ENCODE_FLAG_ */
>  
> +	/*
> +	 * If nal_ref_idc is 0, the NAL unit won't be used as reference by
> +	 * later NAL units. Any other value indicates that the NAL unit may be
> +	 * used as reference.
> +	 */
> +	__u8 nal_ref_idc;
> +
> +	/* TODO Can we infer the nal_unit_type from the slice_type? */
> +	__u8 nal_unit_type;
> +
>  	/* Reference */
>  
>  	__u64 reference_ts;

Re: [RFC PATCH 07/11] arm64: dts: imx8mp: fix VPU_BUS clock setting

[Adam Ford]

On Fri, May 2, 2025 at 10:10 AM Marco Felsch <m.felsch@pengutronix.de> wrote:
>
> The VPU_PLL clock must be set before the VPU_BUS clock which is derived
> from the VPU_PLL clock else the VPU_BUS clock is 300MHz and not 600MHz.
>
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
>  arch/arm64/boot/dts/freescale/imx8mp.dtsi | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> index 97b09b647ec7..7f4bdefb3480 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> @@ -2289,8 +2289,8 @@ vpumix_blk_ctrl: blk-ctrl@38330000 {
>                                  <&clk IMX8MP_CLK_VPU_G2_ROOT>,
>                                  <&clk IMX8MP_CLK_VPU_VC8KE_ROOT>;
>                         clock-names = "g1", "g2", "vc8000e";
> -                       assigned-clocks = <&clk IMX8MP_CLK_VPU_BUS>, <&clk IMX8MP_VPU_PLL>;
> -                       assigned-clock-parents = <&clk IMX8MP_VPU_PLL_OUT>;
> +                       assigned-clocks = <&clk IMX8MP_VPU_PLL>, <&clk IMX8MP_CLK_VPU_BUS>;
> +                       assigned-clock-parents = <0>, <&clk IMX8MP_VPU_PLL_OUT>;
>                         assigned-clock-rates = <600000000>, <600000000>;

I think there was a move to make the default be overdrive [1]  and [2]
and use a 'nominal' device tree for those who are not in overdrive
mode.  According to the TRM, the VPU_BUS_CLK_ROOT, the nominal is
600MHz and the overdrive is 800MHz.  Based on that, I wonder if the
values here should be 800MHz and if we should add the nominal values
of 600MHz to the imx8m-nominal.dtsi file.

adam

>                         interconnects = <&noc IMX8MP_ICM_VPU_G1 &noc IMX8MP_ICN_VIDEO>,
>                                         <&noc IMX8MP_ICM_VPU_G2 &noc IMX8MP_ICN_VIDEO>,


[1] - https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/arch/arm64/boot/dts/freescale/imx8mp.dtsi?h=next-20250502&id=9f7595b3e5ae0ead20a74a5f2a8f0434b3254ac5

[2] - https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/arch/arm64/boot/dts/freescale/imx8mp-nominal.dtsi?h=next-20250502&id=4b98bf3bff7353d94824c4d874ff2d7f38acc49a

> --
> 2.39.5
>
>

Re: [RFC PATCH 05/11] arm64: dts: imx8mp: drop gpcv2 vpu power-domains and clocks

[Marco Felsch]

On 25-05-02, Adam Ford wrote:
> On Fri, May 2, 2025 at 10:10 AM Marco Felsch <m.felsch@pengutronix.de> wrote:
> >
> > The GPCv2 G1, G2 and VC8000E power-domain don't need to reference the
> > VPUMIX power-domain nor their module clocks since the power and reset
> > handling is done by the VPUMIX blkctrl driver.
> >
> It was my understanding that having this dependency ensures the order
> of the bring-up, but maybe I am wrong.  Do you know if the 8MP

If that is true, the 8MM should be broken.

> suspend-resume works properly?

No I didn't test suspend/resume.

> Should this get a fixes tag?

It's just a cleanup, therefore I didn't add the fixes-tag.

Regards,
  Marco


> > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > ---
> >  arch/arm64/boot/dts/freescale/imx8mp.dtsi | 7 -------
> >  1 file changed, 7 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > index e0d3b8cba221..cf9b6c487bd5 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > @@ -879,24 +879,17 @@ pgc_mediamix: power-domain@10 {
> >
> >                                         pgc_vpu_g1: power-domain@11 {
> >                                                 #power-domain-cells = <0>;
> > -                                               power-domains = <&pgc_vpumix>;
> >                                                 reg = <IMX8MP_POWER_DOMAIN_VPU_G1>;
> > -                                               clocks = <&clk IMX8MP_CLK_VPU_G1_ROOT>;
> >                                         };
> >
> >                                         pgc_vpu_g2: power-domain@12 {
> >                                                 #power-domain-cells = <0>;
> > -                                               power-domains = <&pgc_vpumix>;
> >                                                 reg = <IMX8MP_POWER_DOMAIN_VPU_G2>;
> > -                                               clocks = <&clk IMX8MP_CLK_VPU_G2_ROOT>;
> > -
> >                                         };
> >
> >                                         pgc_vpu_vc8000e: power-domain@13 {
> >                                                 #power-domain-cells = <0>;
> > -                                               power-domains = <&pgc_vpumix>;
> >                                                 reg = <IMX8MP_POWER_DOMAIN_VPU_VC8000E>;
> > -                                               clocks = <&clk IMX8MP_CLK_VPU_VC8KE_ROOT>;
> >                                         };
> >
> >                                         pgc_hdmimix: power-domain@14 {
> > --
> > 2.39.5
> >
> >
> 

Re: [RFC PATCH 07/11] arm64: dts: imx8mp: fix VPU_BUS clock setting

[Marco Felsch]

On 25-05-02, Adam Ford wrote:
> On Fri, May 2, 2025 at 10:10 AM Marco Felsch <m.felsch@pengutronix.de> wrote:
> >
> > The VPU_PLL clock must be set before the VPU_BUS clock which is derived
> > from the VPU_PLL clock else the VPU_BUS clock is 300MHz and not 600MHz.
> >
> > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > ---
> >  arch/arm64/boot/dts/freescale/imx8mp.dtsi | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > index 97b09b647ec7..7f4bdefb3480 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > @@ -2289,8 +2289,8 @@ vpumix_blk_ctrl: blk-ctrl@38330000 {
> >                                  <&clk IMX8MP_CLK_VPU_G2_ROOT>,
> >                                  <&clk IMX8MP_CLK_VPU_VC8KE_ROOT>;
> >                         clock-names = "g1", "g2", "vc8000e";
> > -                       assigned-clocks = <&clk IMX8MP_CLK_VPU_BUS>, <&clk IMX8MP_VPU_PLL>;
> > -                       assigned-clock-parents = <&clk IMX8MP_VPU_PLL_OUT>;
> > +                       assigned-clocks = <&clk IMX8MP_VPU_PLL>, <&clk IMX8MP_CLK_VPU_BUS>;
> > +                       assigned-clock-parents = <0>, <&clk IMX8MP_VPU_PLL_OUT>;
> >                         assigned-clock-rates = <600000000>, <600000000>;
> 
> I think there was a move to make the default be overdrive [1]  and [2]
> and use a 'nominal' device tree for those who are not in overdrive
> mode.  According to the TRM, the VPU_BUS_CLK_ROOT, the nominal is
> 600MHz and the overdrive is 800MHz.  Based on that, I wonder if the
> values here should be 800MHz and if we should add the nominal values
> of 600MHz to the imx8m-nominal.dtsi file.

You're right, Ahamd and Lucas did change this. I will adapt it later on.

Regards,
  Marco

Re: [RFC PATCH 05/11] arm64: dts: imx8mp: drop gpcv2 vpu power-domains and clocks

[Adam Ford]

On Fri, May 2, 2025 at 10:10 AM Marco Felsch <m.felsch@pengutronix.de> wrote:
>
> The GPCv2 G1, G2 and VC8000E power-domain don't need to reference the
> VPUMIX power-domain nor their module clocks since the power and reset
> handling is done by the VPUMIX blkctrl driver.
>
Using fluster, I wanted to test this.  I didn't test fluster before this:

./fluster.py run -d GStreamer-VP8-V4L2SL-Gst1.0
Ran 57/61 tests successfully               in 7.059 secs

./fluster.py run -dGStreamer-H.264-V4L2SL-Gst1.0
Ran 129/135 tests successfully               in 45.741 secs

Both of these outperformed the tests I ran a while ago on the imx8mm
running at 600MHz.  I haven't updated my clocks to run at overdrive
rates yet, but if you want, I could help with that.  Either way, I
think you could push submit patch as a stand-alone or with some clock
updates even before the adding the encoder stuff.

Tested-by: Adam Ford <aford173@gmail.com> #imx8mp-beacon-kit

> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
>  arch/arm64/boot/dts/freescale/imx8mp.dtsi | 7 -------
>  1 file changed, 7 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> index e0d3b8cba221..cf9b6c487bd5 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> @@ -879,24 +879,17 @@ pgc_mediamix: power-domain@10 {
>
>                                         pgc_vpu_g1: power-domain@11 {
>                                                 #power-domain-cells = <0>;
> -                                               power-domains = <&pgc_vpumix>;
>                                                 reg = <IMX8MP_POWER_DOMAIN_VPU_G1>;
> -                                               clocks = <&clk IMX8MP_CLK_VPU_G1_ROOT>;
>                                         };
>
>                                         pgc_vpu_g2: power-domain@12 {
>                                                 #power-domain-cells = <0>;
> -                                               power-domains = <&pgc_vpumix>;
>                                                 reg = <IMX8MP_POWER_DOMAIN_VPU_G2>;
> -                                               clocks = <&clk IMX8MP_CLK_VPU_G2_ROOT>;
> -
>                                         };
>
>                                         pgc_vpu_vc8000e: power-domain@13 {
>                                                 #power-domain-cells = <0>;
> -                                               power-domains = <&pgc_vpumix>;
>                                                 reg = <IMX8MP_POWER_DOMAIN_VPU_VC8000E>;
> -                                               clocks = <&clk IMX8MP_CLK_VPU_VC8KE_ROOT>;
>                                         };
>
>                                         pgc_hdmimix: power-domain@14 {
> --
> 2.39.5
>
>

Re: [RFC PATCH 07/11] arm64: dts: imx8mp: fix VPU_BUS clock setting

[Adam Ford]

On Fri, May 2, 2025 at 11:55 AM Marco Felsch <m.felsch@pengutronix.de> wrote:
>
> On 25-05-02, Adam Ford wrote:
> > On Fri, May 2, 2025 at 10:10 AM Marco Felsch <m.felsch@pengutronix.de> wrote:
> > >
> > > The VPU_PLL clock must be set before the VPU_BUS clock which is derived
> > > from the VPU_PLL clock else the VPU_BUS clock is 300MHz and not 600MHz.

I did verify the current clock rate ends up at 300MHz instead of the
desired 600 or 800MHz, so we should do something.

> > >
> > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > > ---
> > >  arch/arm64/boot/dts/freescale/imx8mp.dtsi | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > > index 97b09b647ec7..7f4bdefb3480 100644
> > > --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > > +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > > @@ -2289,8 +2289,8 @@ vpumix_blk_ctrl: blk-ctrl@38330000 {
> > >                                  <&clk IMX8MP_CLK_VPU_G2_ROOT>,
> > >                                  <&clk IMX8MP_CLK_VPU_VC8KE_ROOT>;
> > >                         clock-names = "g1", "g2", "vc8000e";
> > > -                       assigned-clocks = <&clk IMX8MP_CLK_VPU_BUS>, <&clk IMX8MP_VPU_PLL>;
> > > -                       assigned-clock-parents = <&clk IMX8MP_VPU_PLL_OUT>;
> > > +                       assigned-clocks = <&clk IMX8MP_VPU_PLL>, <&clk IMX8MP_CLK_VPU_BUS>;
> > > +                       assigned-clock-parents = <0>, <&clk IMX8MP_VPU_PLL_OUT>;
> > >                         assigned-clock-rates = <600000000>, <600000000>;
> >
> > I think there was a move to make the default be overdrive [1]  and [2]
> > and use a 'nominal' device tree for those who are not in overdrive
> > mode.  According to the TRM, the VPU_BUS_CLK_ROOT, the nominal is
> > 600MHz and the overdrive is 800MHz.  Based on that, I wonder if the
> > values here should be 800MHz and if we should add the nominal values
> > of 600MHz to the imx8m-nominal.dtsi file.
>
> You're right, Ahamd and Lucas did change this. I will adapt it later on.

I updated my device tree to run in overdrive mode and ran fluster at
the higher rates:
VPU_G1 - 800MHz,
VPU-G2 - 700MHz
VPU-Bus - 800MHz

./fluster.py run -d GStreamer-VP8-V4L2SL-Gst1.0
Ran 57/61 tests successfully               in 5.922 secs
(vs 7.059 secs at nominal speed)

./fluster.py run -dGStreamer-H.264-V4L2SL-Gst1.0
Ran 129/135 tests successfully               in 40.107 secs
(vs 45.741 secs at nominal speed)

If you want, I can submit the clock updates I have for overdrive or
send them to you to save you some time.

adam

>
> Regards,
>   Marco

Re: [RFC PATCH 07/11] arm64: dts: imx8mp: fix VPU_BUS clock setting

[Marco Felsch]

Hi Adam,

thanks for testing.

On 25-05-27, Adam Ford wrote:
> On Fri, May 2, 2025 at 11:55 AM Marco Felsch <m.felsch@pengutronix.de> wrote:
> >
> > On 25-05-02, Adam Ford wrote:
> > > On Fri, May 2, 2025 at 10:10 AM Marco Felsch <m.felsch@pengutronix.de> wrote:
> > > >
> > > > The VPU_PLL clock must be set before the VPU_BUS clock which is derived
> > > > from the VPU_PLL clock else the VPU_BUS clock is 300MHz and not 600MHz.
> 
> I did verify the current clock rate ends up at 300MHz instead of the
> desired 600 or 800MHz, so we should do something.
> 
> > > >
> > > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > > > ---
> > > >  arch/arm64/boot/dts/freescale/imx8mp.dtsi | 4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > > > index 97b09b647ec7..7f4bdefb3480 100644
> > > > --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > > > +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > > > @@ -2289,8 +2289,8 @@ vpumix_blk_ctrl: blk-ctrl@38330000 {
> > > >                                  <&clk IMX8MP_CLK_VPU_G2_ROOT>,
> > > >                                  <&clk IMX8MP_CLK_VPU_VC8KE_ROOT>;
> > > >                         clock-names = "g1", "g2", "vc8000e";
> > > > -                       assigned-clocks = <&clk IMX8MP_CLK_VPU_BUS>, <&clk IMX8MP_VPU_PLL>;
> > > > -                       assigned-clock-parents = <&clk IMX8MP_VPU_PLL_OUT>;
> > > > +                       assigned-clocks = <&clk IMX8MP_VPU_PLL>, <&clk IMX8MP_CLK_VPU_BUS>;
> > > > +                       assigned-clock-parents = <0>, <&clk IMX8MP_VPU_PLL_OUT>;
> > > >                         assigned-clock-rates = <600000000>, <600000000>;
> > >
> > > I think there was a move to make the default be overdrive [1]  and [2]
> > > and use a 'nominal' device tree for those who are not in overdrive
> > > mode.  According to the TRM, the VPU_BUS_CLK_ROOT, the nominal is
> > > 600MHz and the overdrive is 800MHz.  Based on that, I wonder if the
> > > values here should be 800MHz and if we should add the nominal values
> > > of 600MHz to the imx8m-nominal.dtsi file.
> >
> > You're right, Ahamd and Lucas did change this. I will adapt it later on.
> 
> I updated my device tree to run in overdrive mode and ran fluster at
> the higher rates:
> VPU_G1 - 800MHz,
> VPU-G2 - 700MHz
> VPU-Bus - 800MHz
> 
> ./fluster.py run -d GStreamer-VP8-V4L2SL-Gst1.0
> Ran 57/61 tests successfully               in 5.922 secs
> (vs 7.059 secs at nominal speed)
> 
> ./fluster.py run -dGStreamer-H.264-V4L2SL-Gst1.0
> Ran 129/135 tests successfully               in 40.107 secs
> (vs 45.741 secs at nominal speed)
> 
> If you want, I can submit the clock updates I have for overdrive or
> send them to you to save you some time.

Sure, you can pick-up my patches or add new ones. Either way you're
right, the clock fixes can go in separately.

Regards,
  Marco

> 
> adam
> 
> >
> > Regards,
> >   Marco
> 

Re: [RFC PATCH 07/11] arm64: dts: imx8mp: fix VPU_BUS clock setting

[Nicolas Dufresne]

Hi,

Le mardi 27 mai 2025 à 22:05 -0500, Adam Ford a écrit :
> On Fri, May 2, 2025 at 11:55 AM Marco Felsch <m.felsch@pengutronix.de> wrote:
> > 
> > On 25-05-02, Adam Ford wrote:
> > > On Fri, May 2, 2025 at 10:10 AM Marco Felsch <m.felsch@pengutronix.de> wrote:
> > > > 
> > > > The VPU_PLL clock must be set before the VPU_BUS clock which is derived
> > > > from the VPU_PLL clock else the VPU_BUS clock is 300MHz and not 600MHz.
> 
> I did verify the current clock rate ends up at 300MHz instead of the
> desired 600 or 800MHz, so we should do something.
> 

This reminded me of: 

https://patchwork.linuxtv.org/project/linux-media/patch/20250217-b4-hantro-av1-clock-rate-v2-1-e179fad52641@collabora.com/

Which also made me discover that this patch wasn't picked despite being mark accepted. We
will favour DT clock settings from here, since its not really managable otherwise, old board
will stay like this, otherwise we face backward compatibility issues.

Note that G2 and VC8K can be run at higher rate, but to be stable, you need
to also control voltage and proper cooling, not something we want "by default".

Nicolas

> 
> > > > 
> > > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > > > ---
> > > >  arch/arm64/boot/dts/freescale/imx8mp.dtsi | 4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > > > index 97b09b647ec7..7f4bdefb3480 100644
> > > > --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > > > +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > > > @@ -2289,8 +2289,8 @@ vpumix_blk_ctrl: blk-ctrl@38330000 {
> > > >                                  <&clk IMX8MP_CLK_VPU_G2_ROOT>,
> > > >                                  <&clk IMX8MP_CLK_VPU_VC8KE_ROOT>;
> > > >                         clock-names = "g1", "g2", "vc8000e";
> > > > -                       assigned-clocks = <&clk IMX8MP_CLK_VPU_BUS>, <&clk IMX8MP_VPU_PLL>;
> > > > -                       assigned-clock-parents = <&clk IMX8MP_VPU_PLL_OUT>;
> > > > +                       assigned-clocks = <&clk IMX8MP_VPU_PLL>, <&clk IMX8MP_CLK_VPU_BUS>;
> > > > +                       assigned-clock-parents = <0>, <&clk IMX8MP_VPU_PLL_OUT>;
> > > >                         assigned-clock-rates = <600000000>, <600000000>;
> > > 
> > > I think there was a move to make the default be overdrive [1]  and [2]
> > > and use a 'nominal' device tree for those who are not in overdrive
> > > mode.  According to the TRM, the VPU_BUS_CLK_ROOT, the nominal is
> > > 600MHz and the overdrive is 800MHz.  Based on that, I wonder if the
> > > values here should be 800MHz and if we should add the nominal values
> > > of 600MHz to the imx8m-nominal.dtsi file.
> > 
> > You're right, Ahamd and Lucas did change this. I will adapt it later on.
> 
> I updated my device tree to run in overdrive mode and ran fluster at
> the higher rates:
> VPU_G1 - 800MHz,
> VPU-G2 - 700MHz
> VPU-Bus - 800MHz
> 
> ./fluster.py run -d GStreamer-VP8-V4L2SL-Gst1.0
> Ran 57/61 tests successfully               in 5.922 secs
> (vs 7.059 secs at nominal speed)
> 
> ./fluster.py run -dGStreamer-H.264-V4L2SL-Gst1.0
> Ran 129/135 tests successfully               in 40.107 secs
> (vs 45.741 secs at nominal speed)
> 
> If you want, I can submit the clock updates I have for overdrive or
> send them to you to save you some time.
> 
> adam
> 
> > 
> > Regards,
> >   Marco

Re: [RFC PATCH 07/11] arm64: dts: imx8mp: fix VPU_BUS clock setting

[Adam Ford]

On Wed, May 28, 2025 at 9:14 AM Nicolas Dufresne
<nicolas.dufresne@collabora.com> wrote:
>
> Hi,
>
> Le mardi 27 mai 2025 à 22:05 -0500, Adam Ford a écrit :
> > On Fri, May 2, 2025 at 11:55 AM Marco Felsch <m.felsch@pengutronix.de> wrote:
> > >
> > > On 25-05-02, Adam Ford wrote:
> > > > On Fri, May 2, 2025 at 10:10 AM Marco Felsch <m.felsch@pengutronix.de> wrote:
> > > > >
> > > > > The VPU_PLL clock must be set before the VPU_BUS clock which is derived
> > > > > from the VPU_PLL clock else the VPU_BUS clock is 300MHz and not 600MHz.
> >
> > I did verify the current clock rate ends up at 300MHz instead of the
> > desired 600 or 800MHz, so we should do something.
> >
>
> This reminded me of:
>
> https://patchwork.linuxtv.org/project/linux-media/patch/20250217-b4-hantro-av1-clock-rate-v2-1-e179fad52641@collabora.com/
>
> Which also made me discover that this patch wasn't picked despite being mark accepted. We
> will favour DT clock settings from here, since its not really managable otherwise, old board
> will stay like this, otherwise we face backward compatibility issues.
>
> Note that G2 and VC8K can be run at higher rate, but to be stable, you need
> to also control voltage and proper cooling, not something we want "by default".

From what I can tell, imx8m_vpu_hw.c doesn't actually set the clock
rates.  It looks like it just enables and disables them.  I think the
clocks are all set via the device tree.  The 8MP datasheets list the
clock rates based on what they call 'nominal' and 'overdrive' which
determined by some SoC voltages.  600MHz seems reasonable for what NXP
states as Nominal.  I didn't see any NXP downstream voltage or cooling
options.

adam

>
> Nicolas
>
> >
> > > > >
> > > > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > > > > ---
> > > > >  arch/arm64/boot/dts/freescale/imx8mp.dtsi | 4 ++--
> > > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > > > > index 97b09b647ec7..7f4bdefb3480 100644
> > > > > --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > > > > +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > > > > @@ -2289,8 +2289,8 @@ vpumix_blk_ctrl: blk-ctrl@38330000 {
> > > > >                                  <&clk IMX8MP_CLK_VPU_G2_ROOT>,
> > > > >                                  <&clk IMX8MP_CLK_VPU_VC8KE_ROOT>;
> > > > >                         clock-names = "g1", "g2", "vc8000e";
> > > > > -                       assigned-clocks = <&clk IMX8MP_CLK_VPU_BUS>, <&clk IMX8MP_VPU_PLL>;
> > > > > -                       assigned-clock-parents = <&clk IMX8MP_VPU_PLL_OUT>;
> > > > > +                       assigned-clocks = <&clk IMX8MP_VPU_PLL>, <&clk IMX8MP_CLK_VPU_BUS>;
> > > > > +                       assigned-clock-parents = <0>, <&clk IMX8MP_VPU_PLL_OUT>;
> > > > >                         assigned-clock-rates = <600000000>, <600000000>;
> > > >
> > > > I think there was a move to make the default be overdrive [1]  and [2]
> > > > and use a 'nominal' device tree for those who are not in overdrive
> > > > mode.  According to the TRM, the VPU_BUS_CLK_ROOT, the nominal is
> > > > 600MHz and the overdrive is 800MHz.  Based on that, I wonder if the
> > > > values here should be 800MHz and if we should add the nominal values
> > > > of 600MHz to the imx8m-nominal.dtsi file.
> > >
> > > You're right, Ahamd and Lucas did change this. I will adapt it later on.
> >
> > I updated my device tree to run in overdrive mode and ran fluster at
> > the higher rates:
> > VPU_G1 - 800MHz,
> > VPU-G2 - 700MHz
> > VPU-Bus - 800MHz
> >
> > ./fluster.py run -d GStreamer-VP8-V4L2SL-Gst1.0
> > Ran 57/61 tests successfully               in 5.922 secs
> > (vs 7.059 secs at nominal speed)
> >
> > ./fluster.py run -dGStreamer-H.264-V4L2SL-Gst1.0
> > Ran 129/135 tests successfully               in 40.107 secs
> > (vs 45.741 secs at nominal speed)
> >
> > If you want, I can submit the clock updates I have for overdrive or
> > send them to you to save you some time.
> >
> > adam
> >
> > >
> > > Regards,
> > >   Marco

Re: [RFC PATCH 00/11] VC8000E H.264 V4L2 Stateless Encoder

[Nicolas Dufresne]

Hi Marco,


Le vendredi 02 mai 2025 à 17:05 +0200, Marco Felsch a écrit
> 
> [1] https://github.com/bootlin/linux/tree/hantro/h264-encoding-v5.11
> [2] https://gitlab.freedesktop.org/dude/gstreamer/-/tree/h264-stateless-encoder

Can you rebase against the upstream work:

https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/5676

A lot of changes Michael made in your branch are already in the upstream MR
branch. An example, in the upstream version, the src pad (CAPTURE) is already
being set before the sink pad (OUTPUT).

I'd like to open the discussion about sizes, as I'm writing things down.
In your modification, you affirm that the encoder must ignore the size
set on the CAPTURE. At the moment I tend to disagree with this
interpretation and would like some feedback.

There is couple of different sizes we'll have to support:

1. Allocation sizes
2. coded size
3. display size

My believe is that we want to split the size in 1 and 2 since the padding
added to the allocated size should not affect the amount of bits that will
be compressed. We should be able to further pad frames without increasing
the compressed size.

For this, I wanted to mimic the stateless decoders, and define the coded
size, the one that occupy space in the bitstream and found in the sequence
headers to match the CATPURE size.

3. does not exists in stateless decoders, since it has no implication
in the decoding process. This one I'll leave open for now, since its
only needed if we have to generate some headers in the kernel. We have
had a lot of discussion toward that, and if so, I will pull in the
use of S_SELECTION.

regards,
Nicolas