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

[Jonas Karlman <jonas@kwiboo.se>]

Hi Nicolas,

On 8/11/2025 9:12 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@gmail.com>
>> Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
>> Signed-off-by: Sebastian Fricke <sebastian.fricke@collabora.com>
>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>> ---
>> 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
>>
>> diff --git a/drivers/media/platform/rockchip/rkvdec/Makefile
>> b/drivers/media/platform/rockchip/rkvdec/Makefile
>> index cb86b429cfaa..a77122641d14 100644
>> --- a/drivers/media/platform/rockchip/rkvdec/Makefile
>> +++ b/drivers/media/platform/rockchip/rkvdec/Makefile
>> @@ -1,3 +1,3 @@
>>  obj-$(CONFIG_VIDEO_ROCKCHIP_VDEC) += rockchip-vdec.o
>>  
>> -rockchip-vdec-y += rkvdec.o rkvdec-h264.o rkvdec-vp9.o
>> +rockchip-vdec-y += rkvdec.o rkvdec-h264.o rkvdec-hevc.o rkvdec-vp9.o
>> diff --git a/drivers/media/platform/rockchip/rkvdec/rkvdec-hevc-data.c
>> b/drivers/media/platform/rockchip/rkvdec/rkvdec-hevc-data.c
>> new file mode 100644
>> index 000000000000..eac4ea604949
>> --- /dev/null
>> +++ b/drivers/media/platform/rockchip/rkvdec/rkvdec-hevc-data.c
>> @@ -0,0 +1,1848 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Rockchip Video Decoder driver
>> + *
>> + * Copyright (C) 2023 Collabora, Ltd.
>> + *	Sebastian Fricke <sebastian.fricke@collabora.com>
>> + */
>> +
>> +#include <linux/types.h>
>> +
>> +#define RKV_CABAC_TABLE_SIZE		27456
>> +
>> +/*
>> + * This file is #include from rkvdec-hevc.c and not compiled.
>> + */
>> +static const u8 rkvdec_hevc_cabac_table[RKV_CABAC_TABLE_SIZE] = {
>> +	0x07, 0x0f, 0x48, 0x58, 0x58, 0x40, 0x40, 0x40, 0x40, 0x40, 0x0f,
>> 0x40, 0x40, 0x40, 0x0f,
> 
> Nit, in H.264 case, we managed to fill the CABAC based on the spect with macros,
> didn't we figure-out this one ? I didn't check in Detlev tree, I'm just asking
> here.

As hinted at in the cover letter: in the initial implementation used for
LibreELEC I just shamelessly copied the cabac data 1:1 from the Rockchip
mpp library, for this series it was replaced with the cabac table from
Sebastian Fricke prior series to add a HEVC backend [1]. Sebastian
mentioned following regarding the cabac table:

"""
Notable design decisions:
- The giant static array of cabac values is moved to a separate c file,
I did so because a separate .h file would be incorrect as it doesn't
expose anything of any value for any other file than the rkvdec-hevc.c
file. Other options were:
  - Calculating the values instead of storing the results (No clear pattern
    found for the calculation using the static array and the formulas from the
    specification)
  - Supply them via firmware (Adding firmware makes the whole software
    way more complicated and the usage of the driver less obvious)
"""

I have not explored any other way to handle the cabac table based on
these design decisions.

[1] https://lore.kernel.org/linux-media/20230101-patch-series-v2-6-2-rc1-v2-0-fa1897efac14@collabora.com/

Regards,
Jonas

> 
> Nicolas

[snip]