Date: Thu, 04 Apr 2024 13:41:15 -0400
From: Nicolas Dufresne <nicolas@...fresne.ca>
To: Emmanuel Gil Peyrot <linkmauve@...kmauve.fr>, 
	linux-kernel@...r.kernel.org
Cc: Ezequiel Garcia <ezequiel@...guardiasur.com.ar>, Philipp Zabel
 <p.zabel@...gutronix.de>, Mauro Carvalho Chehab <mchehab@...nel.org>, Rob
 Herring <robh@...nel.org>, Krzysztof Kozlowski
 <krzysztof.kozlowski+dt@...aro.org>, Conor Dooley <conor+dt@...nel.org>,
 Heiko Stuebner <heiko@...ech.de>, Joerg Roedel <joro@...tes.org>, Will
 Deacon <will@...nel.org>, Robin Murphy <robin.murphy@....com>, Sebastian
 Reichel <sebastian.reichel@...labora.com>,  Cristian Ciocaltea
 <cristian.ciocaltea@...labora.com>, Dragan Simic <dsimic@...jaro.org>,
 Shreeya Patel <shreeya.patel@...labora.com>, Chris Morgan
 <macromorgan@...mail.com>, Andy Yan <andy.yan@...k-chips.com>, Nicolas
 Frattaroli <frattaroli.nicolas@...il.com>,  linux-media@...r.kernel.org,
 linux-rockchip@...ts.infradead.org,  devicetree@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org,  iommu@...ts.linux.dev
Subject: Re: [PATCH v2 0/2] Enable JPEG encoding on rk3588

Hi,

Le mercredi 27 mars 2024 à 14:41 +0100, Emmanuel Gil Peyrot a écrit :
> Only the JPEG encoder is available for now, although there are patches
> for the undocumented VP8 encoder floating around[0].

[0] seems like a broken link. The VP8 encoder RFC is for RK3399 (and Hantro H1
posted by ST more recently). The TRM says "VEPU121(JPEG encoder only)", which
suggest that the H.264 and VP8 encoders usually found on the VEPU121 are
removed. As Rockchip have remove the synthesize register while modifying the H1
IP, it is difficult to verify. Confusingly the H.264 specific registers are
documented in the TRM around VEPU121.

> 
> This has been tested on a rock-5b, resulting in four /dev/video*
> encoders.  The userspace program I’ve been using to test them is
> Onix[1], using the jpeg-encoder example, it will pick one of these four
> at random (but displays the one it picked):
> % ffmpeg -i <input image> -pix_fmt yuvj420p temp.yuv
> % jpeg-encoder temp.yuv <width> <height> NV12 <quality> output.jpeg

I don't like that we exposing each identical cores a separate video nodes. I
think we should aim for 1 device, and then multi-plex and schedule de cores from
inside the Linux kernel.

Not doing this now means we'll never have an optimal hardware usage
distribution. Just consider two userspace software wanting to do jpeg encoding.
If they both take a guess, they may endup using a single core. Where with proper
scheduling in V4L2, the kernel will be able to properly distribute the load I
insist on this, since if we merge you changes it becomes an ABI and we can't
change it anymore.

I understand that this impose a rework of the mem2mem framework so that we can
run multiple jobs, but this will be needed anyway on RK3588, since the rkvdec2,
which we don't have a driver yet is also multi-core, but you need to use 2 cores
when the resolution is close to 8K.

Nicolas

> 
> [0] https://patchwork.kernel.org/project/linux-rockchip/list/?series=789885
> [1] https://crates.io/crates/onix
> 
> Changes since v1:
> - Dropped patches 1 and 4.
> - Use the proper compatible form, since this device should be fully
>   compatible with the VEPU of rk356x.
> - Describe where the VEPU121 name comes from, and list other encoders
>   and decoders present in this SoC.
> - Properly test the device tree changes, I previously couldn’t since I
>   was using a too recent version of python-jsonschema…
> 
> Emmanuel Gil Peyrot (2):
>   media: dt-binding: media: Document rk3588’s VEPU121
>   arm64: dts: rockchip: Add VEPU121 to rk3588
> 
>  .../bindings/media/rockchip,rk3568-vepu.yaml  |  8 +-
>  arch/arm64/boot/dts/rockchip/rk3588s.dtsi     | 80 +++++++++++++++++++
>  2 files changed, 86 insertions(+), 2 deletions(-)
> 

Powered by blists - more mailing lists