On Thu, Apr 04, 2024 at 01:41:15PM -0400, Nicolas Dufresne wrote: > Hi, 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. Ah, the link became, and was indeed ST’s series: https://patchwork.kernel.org/project/linux-rockchip/list/?series=789885&archive=both But the TRM part 1 says the VEPU121 supports H.264 encoding (page 367), and it’s likely they didn’t remove just VP8 support since the codec features are pretty close to H.264’s. > > > > > 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. I agree, but this should be handled in the driver not in the device tree, and it can be done later. > > 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. Will it really become ABI just like that? Userspace should always discover the video nodes and their capabilities and not hardcode e.g. a specific /dev/videoN file for a specific codec. I would argue that this series would let userspace do JPEG encoding right away, even if in a less optimal way than if the driver would round-robin them through a single video node, but that can always be added in a future version. > > 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. I think the mediatek JPEG driver already supports that, would it be ok to do it the same way? > > 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(-) > > > -- Link Mauve