| 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: <3cf31d3b89a66b1bec57486c54c3df31393335e5.camel@collabora.com>
Date: Mon, 11 Aug 2025 17:25:17 -0400
From: Nicolas Dufresne <nicolas.dufresne@...labora.com>
To: Jonas Karlman <jonas@...boo.se>, Ezequiel Garcia
<ezequiel@...guardiasur.com.ar>, Detlev Casanova
<detlev.casanova@...labora.com>, Mauro Carvalho Chehab
<mchehab@...nel.org>, Heiko Stuebner <heiko@...ech.de>
Cc: Alex Bee <knaerzche@...il.com>, Sebastian Fricke
<sebastian.fricke@...labora.com>, 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 5/7] media: rkvdec: Disable QoS for HEVC and VP9 on
RK3328
Le dimanche 10 août 2025 à 21:24 +0000, Jonas Karlman a écrit :
> From: Alex Bee <knaerzche@...il.com>
>
> The RK3328 VDEC has a HW quirk that require QoS to be disabled when HEVC
> or VP9 is decoded, otherwise the decoded picture may become corrupted.
>
> Add a RK3328 variant with a quirk flag to disable QoS when before
> decoding is started.
>
> Signed-off-by: Alex Bee <knaerzche@...il.com>
> Signed-off-by: Jonas Karlman <jonas@...boo.se>
> ---
> Changes in v2:
> - No change
> ---
> drivers/media/platform/rockchip/rkvdec/rkvdec-hevc.c | 9 +++++++++
> drivers/media/platform/rockchip/rkvdec/rkvdec-regs.h | 2 ++
> drivers/media/platform/rockchip/rkvdec/rkvdec-vp9.c | 10 ++++++++++
> drivers/media/platform/rockchip/rkvdec/rkvdec.c | 12 ++++++++++++
> drivers/media/platform/rockchip/rkvdec/rkvdec.h | 4 ++++
> 5 files changed, 37 insertions(+)
>
> diff --git a/drivers/media/platform/rockchip/rkvdec/rkvdec-hevc.c
> b/drivers/media/platform/rockchip/rkvdec/rkvdec-hevc.c
> index 1994ea24f0be..f8bb8c4264f7 100644
> --- a/drivers/media/platform/rockchip/rkvdec/rkvdec-hevc.c
> +++ b/drivers/media/platform/rockchip/rkvdec/rkvdec-hevc.c
> @@ -789,6 +789,15 @@ static int rkvdec_hevc_run(struct rkvdec_ctx *ctx)
> writel(1, rkvdec->regs + RKVDEC_REG_PREF_LUMA_CACHE_COMMAND);
> writel(1, rkvdec->regs + RKVDEC_REG_PREF_CHR_CACHE_COMMAND);
>
> + if (rkvdec->quirks & RKVDEC_QUIRK_DISABLE_QOS) {
> + u32 reg;
> +
> + reg = readl(rkvdec->regs + RKVDEC_REG_QOS_CTRL);
> + reg |= 0xFFFF;
> + reg &= ~BIT(12);
I wonder if there is a better way to express that, if not, a comment for future
readers would be nice. If read it will, we keep the upper 16bit, and replaced
the lower bits with 0xEFFF (all bits set except 12) ? I'd rather not spend time
thinking if I walk by this code again.
> + writel(reg, rkvdec->regs + RKVDEC_REG_QOS_CTRL);
> + }
> +
> /* Start decoding! */
> reg = (run.pps->flags & V4L2_HEVC_PPS_FLAG_TILES_ENABLED) ?
> 0 : RKVDEC_WR_DDR_ALIGN_EN;
> diff --git a/drivers/media/platform/rockchip/rkvdec/rkvdec-regs.h
> b/drivers/media/platform/rockchip/rkvdec/rkvdec-regs.h
> index 540c8bdf24e4..c627b6b6f53a 100644
> --- a/drivers/media/platform/rockchip/rkvdec/rkvdec-regs.h
> +++ b/drivers/media/platform/rockchip/rkvdec/rkvdec-regs.h
> @@ -219,6 +219,8 @@
> #define RKVDEC_REG_H264_ERR_E 0x134
> #define RKVDEC_H264_ERR_EN_HIGHBITS(x) ((x) & 0x3fffffff)
>
> +#define RKVDEC_REG_QOS_CTRL 0x18C
> +
> #define RKVDEC_REG_PREF_LUMA_CACHE_COMMAND 0x410
> #define RKVDEC_REG_PREF_CHR_CACHE_COMMAND 0x450
>
> diff --git a/drivers/media/platform/rockchip/rkvdec/rkvdec-vp9.c
> b/drivers/media/platform/rockchip/rkvdec/rkvdec-vp9.c
> index 0e7e16f20eeb..cadb9d592308 100644
> --- a/drivers/media/platform/rockchip/rkvdec/rkvdec-vp9.c
> +++ b/drivers/media/platform/rockchip/rkvdec/rkvdec-vp9.c
> @@ -824,6 +824,16 @@ static int rkvdec_vp9_run(struct rkvdec_ctx *ctx)
> writel(1, rkvdec->regs + RKVDEC_REG_PREF_CHR_CACHE_COMMAND);
>
> writel(0xe, rkvdec->regs + RKVDEC_REG_STRMD_ERR_EN);
> +
> + if (rkvdec->quirks & RKVDEC_QUIRK_DISABLE_QOS) {
> + u32 reg;
> +
> + reg = readl(rkvdec->regs + RKVDEC_REG_QOS_CTRL);
> + reg |= 0xFFFF;
> + reg &= ~BIT(12);
> + writel(reg, rkvdec->regs + RKVDEC_REG_QOS_CTRL);
Can we deduplicate that ?
> + }
> +
> /* Start decoding! */
> writel(RKVDEC_INTERRUPT_DEC_E | RKVDEC_CONFIG_DEC_CLK_GATE_E |
> RKVDEC_TIMEOUT_E | RKVDEC_BUF_EMPTY_E,
> diff --git a/drivers/media/platform/rockchip/rkvdec/rkvdec.c
> b/drivers/media/platform/rockchip/rkvdec/rkvdec.c
> index c20e046205fe..d61d4c419992 100644
> --- a/drivers/media/platform/rockchip/rkvdec/rkvdec.c
> +++ b/drivers/media/platform/rockchip/rkvdec/rkvdec.c
> @@ -1226,6 +1226,13 @@ static const struct rkvdec_variant
> rk3288_rkvdec_variant = {
> .capabilities = RKVDEC_CAPABILITY_HEVC,
> };
>
> +static const struct rkvdec_variant rk3328_rkvdec_variant = {
> + .capabilities = RKVDEC_CAPABILITY_HEVC |
> + RKVDEC_CAPABILITY_H264 |
> + RKVDEC_CAPABILITY_VP9,
> + .quirks = RKVDEC_QUIRK_DISABLE_QOS,
> +};
> +
> static const struct rkvdec_variant rk3399_rkvdec_variant = {
> .capabilities = RKVDEC_CAPABILITY_HEVC |
> RKVDEC_CAPABILITY_H264 |
> @@ -1237,6 +1244,10 @@ static const struct of_device_id of_rkvdec_match[] = {
> .compatible = "rockchip,rk3288-vdec",
> .data = &rk3288_rkvdec_variant,
> },
> + {
> + .compatible = "rockchip,rk3328-vdec",
> + .data = &rk3328_rkvdec_variant,
> + },
> {
> .compatible = "rockchip,rk3399-vdec",
> .data = &rk3399_rkvdec_variant,
> @@ -1267,6 +1278,7 @@ static int rkvdec_probe(struct platform_device *pdev)
> platform_set_drvdata(pdev, rkvdec);
> rkvdec->dev = &pdev->dev;
> rkvdec->capabilities = variant->capabilities;
> + rkvdec->quirks = variant->quirks;
> mutex_init(&rkvdec->vdev_lock);
> INIT_DELAYED_WORK(&rkvdec->watchdog_work, rkvdec_watchdog_func);
>
> diff --git a/drivers/media/platform/rockchip/rkvdec/rkvdec.h
> b/drivers/media/platform/rockchip/rkvdec/rkvdec.h
> index 8e1f8548eae4..e633a879e9bf 100644
> --- a/drivers/media/platform/rockchip/rkvdec/rkvdec.h
> +++ b/drivers/media/platform/rockchip/rkvdec/rkvdec.h
> @@ -26,6 +26,8 @@
> #define RKVDEC_CAPABILITY_H264 BIT(1)
> #define RKVDEC_CAPABILITY_VP9 BIT(2)
>
> +#define RKVDEC_QUIRK_DISABLE_QOS BIT(0)
Can you go back in the series, get H264 into bit 0, VP9 into bit 1, and set
quirks from bit 16 ? Just worried the whole finding can becomes a mess in many
years from now.
> +
> struct rkvdec_ctx;
>
> struct rkvdec_ctrl_desc {
> @@ -69,6 +71,7 @@ vb2_to_rkvdec_decoded_buf(struct vb2_buffer *buf)
>
> struct rkvdec_variant {
> unsigned int capabilities;
> + unsigned int quirks;
> };
>
> struct rkvdec_coded_fmt_ops {
> @@ -121,6 +124,7 @@ struct rkvdec_dev {
> struct delayed_work watchdog_work;
> struct iommu_domain *empty_domain;
> unsigned int capabilities;
> + unsigned int quirks;
> };
>
> struct rkvdec_ctx {
Download attachment "signature.asc" of type "application/pgp-signature" (196 bytes)
Powered by blists - more mailing lists