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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <71EA9FB8-83DB-4785-86C1-2E6BA9C739D9@gmail.com>
Date: Fri, 26 Dec 2025 16:25:09 +0400
From: Christian Hewitt <christianshewitt@...il.com>
To: Diederik de Haas <diederik@...ow-tech.com>
Cc: Detlev Casanova <detlev.casanova@...labora.com>,
 Nicolas Dufresne <nicolas.collabora@...labora.com>,
 Olivier Crête <olivier.crete@...labora.com>,
 Ezequiel Garcia <ezequiel@...guardiasur.com.ar>,
 Mauro Carvalho Chehab <mchehab@...nel.org>,
 Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>,
 Heiko Stuebner <heiko@...ech.de>,
 Dmitry Osipenko <dmitry.osipenko@...labora.com>,
 Thomas Gleixner <tglx@...utronix.de>,
 Dragan Simic <dsimic@...jaro.org>,
 Chukun Pan <amadeus@....edu.cn>,
 linux-media@...r.kernel.org,
 linux-rockchip@...ts.infradead.org,
 devicetree@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/3] media: rkvdec: Add support for the VDPU346 variant

> On 26 Dec 2025, at 4:00 pm, Diederik de Haas <diederik@...ow-tech.com> wrote:
> 
> Hi Christian,
> 
> On Fri Dec 26, 2025 at 12:31 PM CET, Christian Hewitt wrote:
>> 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 <christianshewitt@...il.com>
>> Reviewed-by: Nicolas Dufresne <nicolas.collabora@...labora.com>
>> ---
>> .../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 ...
> 
>> + .step_width = 64,
>> + .min_height = 64,
>> + .max_height = 65472,
> 
> ... and this 2304.
> 
>> + .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 ...
> 
>> + .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.

> + .step_height = 16,
>> + },
>> + .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).

Christian


> Cheers,
>  Diederik
> 
>> +};
>> +
>> 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,