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

[Jonas Karlman]

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@xxxxxxxxx>
>> Signed-off-by: Nicolas Dufresne <nicolas.dufresne@xxxxxxxxxxxxx>
>> Signed-off-by: Sebastian Fricke <sebastian.fricke@xxxxxxxxxxxxx>
>> Signed-off-by: Jonas Karlman <jonas@xxxxxxxxx>
> 
> 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@xxxxxxxxxxxxx>
> Reviewed-by: Nicolas Dufresne <nicolas.dufresne@xxxxxxxxxxxxx>
> 
> 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]