linux-kernel - Re: [PATCH v2 1/7] media: rkvdec: Add HEVC backend

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <8394c12a-1e1e-44fc-9bb5-92a464dfe410@kwiboo.se>
Date: Wed, 3 Sep 2025 09:28:41 +0200
From: Jonas Karlman <jonas@...boo.se>
To: Nicolas Dufresne <nicolas.dufresne@...labora.com>,
 Detlev Casanova <detlev.casanova@...labora.com>
Cc: Ezequiel Garcia <ezequiel@...guardiasur.com.ar>,
 Mauro Carvalho Chehab <mchehab@...nel.org>, Heiko Stuebner
 <heiko@...ech.de>, 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 1/7] media: rkvdec: Add HEVC backend

Hi Nicolas,

On 8/29/2025 10:22 PM, Nicolas Dufresne wrote:
> Le dimanche 10 août 2025 à 21:24 +0000, Jonas Karlman a écrit :
>> The Rockchip VDEC supports the HEVC codec with the Main and Main10
>> Profile up to Level 5.1 High tier: 4096x2304@60 fps.
>>
>> Add the backend for HEVC format to the decoder.
>>
>> Signed-off-by: Alex Bee <knaerzche@...il.com>
>> Signed-off-by: Nicolas Dufresne <nicolas.dufresne@...labora.com>
>> Signed-off-by: Sebastian Fricke <sebastian.fricke@...labora.com>
>> Signed-off-by: Jonas Karlman <jonas@...boo.se>
> 
> Re-reading myself, most of my comments were off or really "nitty". So let's move
> forward and spare you the v3. Detlev is happy to rebase and work on top of your
> series, so let's help everyone getting better RK codec support.
> 
> Tested-by: Nicolas Dufresne <nicolas.dufresne@...labora.com>
> Reviewed-by: Nicolas Dufresne <nicolas.dufresne@...labora.com>
> 
> Just to be transparent, during testing, it was notice that some concurrent
> decoding resulted into failures. I tested Detlev port to structure registers,
> and it didn't change anything (so probably not a stalled state, or one that we
> control). This could easily be a HW issue with older chip. Since you have used
> this for years without major issue reported, I happy to move on.

Thanks, I found some minor changes compared to the LibreELEC version
that I am running some new tests on, plan to send out a v3 as soon as
testing completes later today.

In LibreELEC version we enable some error detection,

	// sw_cabac_error_e - cabac error enable
	writel(0xfdfffffd, rkvdec->regs + RKVDEC_REG_STRMD_ERR_EN);
	// slice end error enable = BIT(28)
	// frame end error enable = BIT(29)
	writel(0x30000000, rkvdec->regs + RKVDEC_REG_H264_ERR_E);

and in this series it was fully disabled to closer match H264/VP9:

	writel(0, rkvdec->regs + RKVDEC_REG_STRMD_ERR_EN);
	writel(0, rkvdec->regs + RKVDEC_REG_H264_ERR_E);

There is also an extra memset(0, ...) in rkvdec_hevc_start:

	memset(priv_tbl, 0, sizeof(*priv_tbl));

This should not really be needed and was removed in this series.

Still unclear if any of these will result in a changed behavior. Enable
of cabac/slice end/frame end error could possible activate some more
states when block issue a self-reset, but I am only guessing.

One thing to note for the flaky tests is that when they fail, they
typically just end up with a different consistent checksum. I have not
done any visual inspection of those frames, but will extract each frame
and compare them both bitwise and visually.

Regards,
Jonas

> 
> regards,
> Nicolas
> 
>> ---
>> Changes in v2:
>> - Use new_value in transpose_and_flatten_matrices()
>> - Add NULL check for ctrl->new_elems in rkvdec_hevc_run_preamble()
>> - Set RKVDEC_WR_DDR_ALIGN_EN for RK3328
>> ---
>>  .../media/platform/rockchip/rkvdec/Makefile   |    2 +-
>>  .../rockchip/rkvdec/rkvdec-hevc-data.c        | 1848 +++++++++++++++++
>>  .../platform/rockchip/rkvdec/rkvdec-hevc.c    |  817 ++++++++
>>  .../platform/rockchip/rkvdec/rkvdec-regs.h    |    2 +
>>  .../media/platform/rockchip/rkvdec/rkvdec.c   |   76 +
>>  .../media/platform/rockchip/rkvdec/rkvdec.h   |    1 +
>>  6 files changed, 2745 insertions(+), 1 deletion(-)
>>  create mode 100644 drivers/media/platform/rockchip/rkvdec/rkvdec-hevc-data.c
>>  create mode 100644 drivers/media/platform/rockchip/rkvdec/rkvdec-hevc.c

[snip]