| 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
| ||
|
Message-Id: <DCXPTZRNNTDY.1773L2181HWF@cknow.org>
Date: Sat, 20 Sep 2025 17:00:07 +0200
From: "Diederik de Haas" <didi.debian@...ow.org>
To: "Detlev Casanova" <detlev.casanova@...labora.com>,
<linux-kernel@...r.kernel.org>
Cc: "Mauro Carvalho Chehab" <mchehab@...nel.org>, "Heiko Stuebner"
<heiko@...ech.de>, <linux-media@...r.kernel.org>,
<linux-rockchip@...ts.infradead.org>,
<linux-arm-kernel@...ts.infradead.org>, <kernel@...labora.com>
Subject: Re: [PATCH v2 09/12] media: rkvdec: Add H264 support for the
VDPU381 variant
Hi Detlev,
Thanks a LOT for this patch set!
As already reported the results were quite impressive :-D
To figure out how to make VDPU346 for rk3568 work, I had a close look at
the TRM and compared that to rk3588's TRM and compared it to your code.
I think I may have found a few potential issue, but I may also not
understand things correctly.
On Fri Aug 8, 2025 at 10:03 PM CEST, Detlev Casanova wrote:
> This decoder variant is found in Rockchip RK3588 SoC family.
>
> Like for rkvdec on rk3399, it supports the NV12, NV15, NV16 and NV20
> output formats and level up to 5.1.
>
> The maximum width and height have been significantly increased
> supporting up to 65520 pixels for both.
>
> Signed-off-by: Detlev Casanova <detlev.casanova@...labora.com>
> ---
> .../media/platform/rockchip/rkvdec/Makefile | 1 +
> .../rockchip/rkvdec/rkvdec-h264-common.h | 2 +
> .../platform/rockchip/rkvdec/rkvdec-h264.c | 36 --
> .../rockchip/rkvdec/rkvdec-vdpu381-h264.c | 469 ++++++++++++++++++
> .../rockchip/rkvdec/rkvdec-vdpu381-regs.h | 427 ++++++++++++++++
> .../media/platform/rockchip/rkvdec/rkvdec.c | 164 +++++-
> .../media/platform/rockchip/rkvdec/rkvdec.h | 6 +
> 7 files changed, 1067 insertions(+), 38 deletions(-)
> create mode 100644 drivers/media/platform/rockchip/rkvdec/rkvdec-vdpu381-h264.c
> create mode 100644 drivers/media/platform/rockchip/rkvdec/rkvdec-vdpu381-regs.h
>
> ...
>
> diff --git a/drivers/media/platform/rockchip/rkvdec/rkvdec-vdpu381-regs.h b/drivers/media/platform/rockchip/rkvdec/rkvdec-vdpu381-regs.h
> new file mode 100644
> index 0000000000000..11b545e9ee7ea
> --- /dev/null
> +++ b/drivers/media/platform/rockchip/rkvdec/rkvdec-vdpu381-regs.h
> @@ -0,0 +1,427 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Rockchip VDPU381 Video Decoder driver registers description
> + *
> + * Copyright (C) 2024 Collabora, Ltd.
> + * Detlev Casanova <detlev.casanova@...labora.com>
> + */
> +
> +#include <linux/types.h>
> +
> +#ifndef _RKVDEC_REGS_H_
> +#define _RKVDEC_REGS_H_
> +
> +#define OFFSET_COMMON_REGS (8 * sizeof(u32))
> +#define OFFSET_CODEC_PARAMS_REGS (64 * sizeof(u32))
> +#define OFFSET_COMMON_ADDR_REGS (128 * sizeof(u32))
> +#define OFFSET_CODEC_ADDR_REGS (160 * sizeof(u32))
> +#define OFFSET_POC_HIGHBIT_REGS (200 * sizeof(u32))
> +
> +#define VDPU381_MODE_HEVC 0
> +#define VDPU381_MODE_H264 1
> +#define VDPU381_MODE_VP9 2
> +#define VDPU381_MODE_AVS2 3
> +
> +#define MAX_SLICE_NUMBER 0x3fff
> +
> +#define RKVDEC_1080P_PIXELS (1920 * 1080)
> +#define RKVDEC_4K_PIXELS (4096 * 2304)
> +#define RKVDEC_8K_PIXELS (7680 * 4320)
In the RK3588 TRM Part 1 paragraph 5.4.3 I can find the values for 4K
and 8K, but the 1080P resolution seems to be 1920x1088 (not 1080).
> +#define RKVDEC_TIMEOUT_1080p (0xefffff)
> +#define RKVDEC_TIMEOUT_4K (0x2cfffff)
> +#define RKVDEC_TIMEOUT_8K (0x4ffffff)
> +#define RKVDEC_TIMEOUT_MAX (0xffffffff)
> +
> +#define VDPU381_REG_DEC_E 0x028
> +#define VDPU381_DEC_E_BIT 1
> +
> +#define VDPU381_REG_IMPORTANT_EN 0x02c
> +#define VDPU381_DEC_IRQ_DISABLE BIT(4)
> +
> +#define VDPU381_REG_STA_INT 0x380
> +#define VDPU381_STA_INT_DEC_RDY_STA BIT(2)
> +#define VDPU381_STA_INT_ERROR BIT(4)
> +#define VDPU381_STA_INT_TIMEOUT BIT(5)
> +#define VDPU381_STA_INT_SOFTRESET_RDY BIT(9)
> +
> +/* base: OFFSET_COMMON_REGS */
>
> ...
>
> diff --git a/drivers/media/platform/rockchip/rkvdec/rkvdec.c b/drivers/media/platform/rockchip/rkvdec/rkvdec.c
> index 0ccf1ba81958a..1b55fe4ff2baf 100644
> --- a/drivers/media/platform/rockchip/rkvdec/rkvdec.c
> +++ b/drivers/media/platform/rockchip/rkvdec/rkvdec.c
> @@ -28,6 +28,7 @@
>
> #include "rkvdec.h"
> #include "rkvdec-regs.h"
> +#include "rkvdec-vdpu381-regs.h"
> #include "rkvdec-rcb.h"
>
> static bool rkvdec_image_fmt_match(enum rkvdec_image_fmt fmt1,
> @@ -84,11 +85,50 @@ static bool rkvdec_is_valid_fmt(struct rkvdec_ctx *ctx, u32 fourcc,
> return false;
> }
>
> +#define VDPU38X_STRIDE_ALIGN 16
> +
> +/**
> + * The default v4l2_fill_pixfmt_mp() function doesn't allow for specific alignment values.
> + * As the VDPU381 and VDPU383 need lines to be aligned on 16, use our own implementation here.
> + */
> +static int vdpu38x_fill_pixfmt_mp(struct v4l2_pix_format_mplane *pix_mp, u32 pixelformat,
> + u32 width, u32 height)
> +{
> + const struct v4l2_format_info *info = v4l2_format_info(pix_mp->pixelformat);
> + struct v4l2_plane_pix_format *plane = &pix_mp->plane_fmt[0];
> +
> + if (!info)
> + return -EINVAL;
> +
> + pix_mp->num_planes = 1;
> +
> + memset(plane, 0, sizeof(*plane));
> +
> + plane->bytesperline = pix_mp->width * info->bpp[0] / info->bpp_div[0];
> + plane->bytesperline = ALIGN(plane->bytesperline, VDPU38X_STRIDE_ALIGN);
> +
> + for (int i = 0; i < info->comp_planes; i++) {
> + unsigned int vdiv = i ? info->vdiv : 1;
> + unsigned int hdiv = i ? info->hdiv : 1;
> + unsigned int stride = DIV_ROUND_UP(pix_mp->width, hdiv)
> + * info->bpp[i] / info->bpp_div[i];
> + unsigned int height = DIV_ROUND_UP(pix_mp->height, vdiv);
> +
> + plane->sizeimage += ALIGN(stride, VDPU38X_STRIDE_ALIGN) * height;
> + }
> +
> + return 0;
> +}
> +
> static void rkvdec_fill_decoded_pixfmt(struct rkvdec_ctx *ctx,
> struct v4l2_pix_format_mplane *pix_mp)
> {
> - v4l2_fill_pixfmt_mp(pix_mp, pix_mp->pixelformat,
> - pix_mp->width, pix_mp->height);
> + struct rkvdec_config *cfg = ctx->dev->config;
> +
> + cfg->fill_pixfmt_mp(pix_mp, pix_mp->pixelformat, pix_mp->width, pix_mp->height);
> +
> + ctx->colmv_offset = pix_mp->plane_fmt[0].sizeimage;
> +
> pix_mp->plane_fmt[0].sizeimage += 128 *
> DIV_ROUND_UP(pix_mp->width, 16) *
> DIV_ROUND_UP(pix_mp->height, 16);
> @@ -287,6 +327,25 @@ static const struct rkvdec_coded_fmt_desc rkvdec_coded_fmts[] = {
> }
> };
>
> +static const struct rkvdec_coded_fmt_desc vdpu381_coded_fmts[] = {
> + {
> + .fourcc = V4L2_PIX_FMT_H264_SLICE,
> + .frmsize = {
> + .min_width = 64,
> + .max_width = 65520,
> + .step_width = 64,
> + .min_height = 16,
> + .max_height = 65520,
> + .step_height = 16,
> + },
Also in the RK3588 TRM Part 1 paragraph 5.4.3, I see "Supported image size" :
16x16 to 65520x65520; step size 16 pixels
I interpret that that .min_width and .step_width should both be 16.
(.min_height and .step_height are correct at 16; if I read the TRM right)
> + .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,
> + },
> +};
> +
> static const struct rkvdec_coded_fmt_desc *
> rkvdec_find_coded_fmt_desc(struct rkvdec_ctx *ctx, u32 fourcc)
> {
> @@ -1125,6 +1184,35 @@ static irqreturn_t rk3399_irq_handler(struct rkvdec_ctx *ctx)
Cheers,
Diederik
Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)
Powered by blists - more mailing lists