Re: [PATCH v2 09/12] media: rkvdec: Add H264 support for the VDPU381 variant

["Diederik de Haas" <didi.debian@cknow.org>]

[-- Attachment #1: Type: text/plain, Size: 3919 bytes --]

Hi Jonas,

On Sat Sep 20, 2025 at 6:15 PM CEST, Jonas Karlman wrote:
> On 9/20/2025 5:00 PM, Diederik de Haas wrote:
>> 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@collabora.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.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
>>>
>>> ...
>>>
>>> @@ -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)
>
> The frmsize used internally for rkvdec is only meant to restrict/align
> the frame buffer use while decoding, It does not reflect sizes the HW
> can decode.
>
> frmsize should typically be min 64x64 with step_height 16 and step_width
> 64 to ensure stride is 16 byte aligned for both 8-bit and 10-bit video.
>
> Only max_width and max_height is used from here to signal userspace what
> max res is supported, together with min/max res and step of 1.

Interesting, thanks for the explanation :-)

So if I understand correctly, only .min_height should be changed to '64'
here? And it should also be 64+64+64+16 for HEVC (patch 11)?
(and also for VP9 and AVS2?)

> Regards,
> Jonas

Cheers,
  Diederik

>> 
>>> +		.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)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]