Re: [PATCH v2 2/3] media: rkvdec: Add support for the VDPU346 variant

Le vendredi 26 décembre 2025 à 16:25 +0400, Christian Hewitt a écrit :

quoted

On 26 Dec 2025, at 4:00 pm, Diederik de Haas [off-list ref] wrote:

Hi Christian,

On Fri Dec 26, 2025 at 12:31 PM CET, Christian Hewitt wrote:

quoted

VDPU346 is similar to VDPU381 but with a single core and limited
to 4K60 media. It is also limited to H264 L5.1 and omits AV1 and
AVS2 capabilities. VDPU346 is used with RK3566 and RK3568.

Signed-off-by: Christian Hewitt <redacted>
Reviewed-by: Nicolas Dufresne <redacted>
---
.../media/platform/rockchip/rkvdec/rkvdec.c   | 103 ++++++++++++++++++
1 file changed, 103 insertions(+)

diff --git a/drivers/media/platform/rockchip/rkvdec/rkvdec.c b/drivers/media/platform/rockchip/rkvdec/rkvdec.c
index e547057dc75f..6b39e99d8a8b 100644
--- a/drivers/media/platform/rockchip/rkvdec/rkvdec.c
+++ b/drivers/media/platform/rockchip/rkvdec/rkvdec.c

@@ -236,6 +236,62 @@ static const struct rkvdec_ctrls rkvdec_hevc_ctrls = {

.num_ctrls = ARRAY_SIZE(rkvdec_hevc_ctrl_descs),
};

+static const struct rkvdec_ctrl_desc vdpu346_hevc_ctrl_descs[] = {
+ {
+ .cfg.id = V4L2_CID_STATELESS_HEVC_DECODE_PARAMS,
+ },
+ {
+ .cfg.id = V4L2_CID_STATELESS_HEVC_SPS,
+ .cfg.ops = &rkvdec_ctrl_ops,
+ },
+ {
+ .cfg.id = V4L2_CID_STATELESS_HEVC_PPS,
+ },
+ {
+ .cfg.id = V4L2_CID_STATELESS_HEVC_SCALING_MATRIX,
+ },
+ {
+ .cfg.id = V4L2_CID_STATELESS_HEVC_DECODE_MODE,
+ .cfg.min = V4L2_STATELESS_HEVC_DECODE_MODE_FRAME_BASED,
+ .cfg.max = V4L2_STATELESS_HEVC_DECODE_MODE_FRAME_BASED,
+ .cfg.def = V4L2_STATELESS_HEVC_DECODE_MODE_FRAME_BASED,
+ },
+ {
+ .cfg.id = V4L2_CID_STATELESS_HEVC_START_CODE,
+ .cfg.min = V4L2_STATELESS_HEVC_START_CODE_ANNEX_B,
+ .cfg.def = V4L2_STATELESS_HEVC_START_CODE_ANNEX_B,
+ .cfg.max = V4L2_STATELESS_HEVC_START_CODE_ANNEX_B,
+ },
+ {
+ .cfg.id = V4L2_CID_MPEG_VIDEO_HEVC_PROFILE,
+ .cfg.min = V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN,
+ .cfg.max = V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN_10,
+ .cfg.menu_skip_mask =
+ BIT(V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN_STILL_PICTURE),
+ .cfg.def = V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN,
+ },
+ {
+ .cfg.id = V4L2_CID_MPEG_VIDEO_HEVC_LEVEL,
+ .cfg.min = V4L2_MPEG_VIDEO_HEVC_LEVEL_1,
+ .cfg.max = V4L2_MPEG_VIDEO_HEVC_LEVEL_5_1,
+ },
+ {
+ .cfg.id = V4L2_CID_STATELESS_HEVC_EXT_SPS_ST_RPS,
+ .cfg.ops = &rkvdec_ctrl_ops,
+ .cfg.dims = { 65 },
+ },
+ {
+ .cfg.id = V4L2_CID_STATELESS_HEVC_EXT_SPS_LT_RPS,
+ .cfg.ops = &rkvdec_ctrl_ops,
+ .cfg.dims = { 65 },
+ },
+};
+
+static const struct rkvdec_ctrls vdpu346_hevc_ctrls = {
+ .ctrls = vdpu346_hevc_ctrl_descs,
+ .num_ctrls = ARRAY_SIZE(vdpu346_hevc_ctrl_descs),
+};
+
static const struct rkvdec_ctrl_desc vdpu38x_hevc_ctrl_descs[] = {
{
.cfg.id = V4L2_CID_STATELESS_HEVC_DECODE_PARAMS,

@@ -463,6 +519,41 @@ static const struct rkvdec_coded_fmt_desc rk3288_coded_fmts[] = {

}
};

+static const struct rkvdec_coded_fmt_desc vdpu346_coded_fmts[] = {
+ {
+ .fourcc = V4L2_PIX_FMT_HEVC_SLICE,
+ .frmsize = {
+ .min_width = 64,
+ .max_width = 65472,

This should be 4096 according to page 469 of RK3568 TRM Part 2 ...

quoted

+ .step_width = 64,
+ .min_height = 64,
+ .max_height = 65472,

... and this 2304.

quoted

+ .step_height = 16,
+ },
+ .ctrls = &vdpu346_hevc_ctrls,
+ .ops = &rkvdec_vdpu381_hevc_fmt_ops,
+ .num_decoded_fmts = ARRAY_SIZE(rkvdec_hevc_decoded_fmts),
+ .decoded_fmts = rkvdec_hevc_decoded_fmts,
+ .subsystem_flags = VB2_V4L2_FL_SUPPORTS_M2M_HOLD_CAPTURE_BUF,
+ },
+ {
+ .fourcc = V4L2_PIX_FMT_H264_SLICE,
+ .frmsize = {
+ .min_width = 64,
+ .max_width =  65520,

This too should be 4096 according to page 469 of RK3568 TRM Part 2 ...

quoted

+ .step_width = 64,
+ .min_height = 64,
+ .max_height =  65520,

... and this 2304.

I guess this also explains the 'green images' Nicolas noticed.

Quite probably. I’ve picked the above changes into my working tree
(for those following it) and will send a v3 series in response to
the next revision of Detlev’s patches.

quoted

+ .step_height = 16,

quoted

+ },
+ .ctrls = &rkvdec_h264_ctrls,
+ .ops = &rkvdec_vdpu381_h264_fmt_ops,
+ .num_decoded_fmts = ARRAY_SIZE(rkvdec_h264_decoded_fmts),
+ .decoded_fmts = rkvdec_h264_decoded_fmts,
+ .subsystem_flags = VB2_V4L2_FL_SUPPORTS_M2M_HOLD_CAPTURE_BUF,
+ },

I see you've reversed the order of the blocks so that HEVC now comes
before the H264 block. While that makes it consistent with what Detlev
has in their v7 and with the existing code in the driver ... I actually
prefer having H264 before HEVC as the alphabetical sorting order is
H264 before HEVC.
In the existing code the VP9 'stuff' is listed below H264 and HEVC.

But then Detlev should do that too in their patch set ... and 'ideally'
the order of the existing code be updated to be alphabetically too.

OTOH, a consistent order works for me too.

I believe the reorder was requested by Nic (offline from the list) so
there’s probably a reason behind it. I’ll keep things aligned to the
order in Detlev’s series (whatever that is).

I'm taking note of this feedback, and instead of multiplying the revisions, I
will probably fix it on top once we have the series ready.

regards,
Nicolas

Christian

quoted

Cheers,
 Diederik

quoted

+};
+
static const struct rkvdec_coded_fmt_desc vdpu381_coded_fmts[] = {
{
.fourcc = V4L2_PIX_FMT_HEVC_SLICE,

@@ -1643,6 +1734,14 @@ static const struct rkvdec_variant_ops vdpu381_variant_ops = {

.flatten_matrices = transpose_and_flatten_matrices,
};

+static const struct rkvdec_variant vdpu346_variant = {
+ .coded_fmts = vdpu346_coded_fmts,
+ .num_coded_fmts = ARRAY_SIZE(vdpu346_coded_fmts),
+ .rcb_sizes = vdpu381_rcb_sizes,
+ .num_rcb_sizes = ARRAY_SIZE(vdpu381_rcb_sizes),
+ .ops = &vdpu381_variant_ops,
+};
+
static const struct rkvdec_variant vdpu381_variant = {
.coded_fmts = vdpu381_coded_fmts,
.num_coded_fmts = ARRAY_SIZE(vdpu381_coded_fmts),

@@ -1691,6 +1790,10 @@ static const struct of_device_id of_rkvdec_match[] = {

.compatible = "rockchip,rk3399-vdec",
.data = &rk3399_rkvdec_variant,
},
+ {
+ .compatible = "rockchip,rk3568-vdec",
+ .data = &vdpu346_variant,
+ },
{
.compatible = "rockchip,rk3588-vdec",
.data = &vdpu381_variant,