Re: [PATCH 0/3] Enable JPEG Encoder on RK3566/RK3568

[Nicolas Frattaroli <frattaroli.nicolas@gmail.com>]

On Freitag, 29. April 2022 16:46:01 CEST Nicolas Dufresne wrote:
> Le jeudi 28 avril 2022 à 00:44 +0200, Nicolas Frattaroli a écrit :
> > Hello,
> > 
> > the following series adds support for and enables the hardware JPEG
> > encoder on the RK3566 and RK3568 line of SoCs by Rockchip.
> > 
> > The JPEG encoder is its own little Hantro instance with seemingly just
> > the encode functionality.
> 
> I'm a little suspicious about this statement. I believe the Hantro combo is
> identical to RK3399 and that you are confusing with Rockchip JPEG encoder here.
> Here's the source of my suspicion:
> 
> https://github.com/JeffyCN/rockchip_mirrors/blob/mpp/osal/mpp_soc.cpp#L637
> 
> As this get burnt into DT, we really need to get this right. Perhaps we need to
> run the reference software to verify ? Ping me if you need help with that.

I believe there's three separate questions here:

A) is this a Hantro instance with more than just JPEG encoding?
B) is this the same as the other[1] Hantro instance, just with a bigger
   offset for the encoder?
C) if A is true and B is false, does this also have decoders?

I think I can only answer B). I do not think fdee0000 is the same hardware
instance as fdea0400. They are in different power domains (yes, this one
being in the RGA PD is correct according to downstream code, TRM and my
own testing). They also have their own MMU each.

Your findings point towards A) being true. I'd love to be able to just
throw a mainline VP8 or H.264 driver at it to check, but that sadly does
not exist yet. Upon checking the TRM again, it seems it documents
registers for H.264 as well.

However I can do this to answer C), and I will once I get the time.
It would be somewhat surprising if there was a hidden third H.264
decoder on this chip.

I'll also CC Kever Yang from rockchip, maybe he can shine some light on
this.

Regards,
Nicolas Frattaroli

[1]: https://patchwork.kernel.org/project/linux-rockchip/patch/20220214212955.1178947-2-piotr.oniszczuk@gmail.com/

> 
> > 
> > The first patch modifies the bindings with a new compatible, and adds
> > the ability to just have a vepu interrupt without a vdpu interrupt.
> > 
> > The second patch makes the actual driver changes to support this variant.
> > 
> > The third and final patch makes the necessary device tree changes for
> > the rk356x device tree file to add both the node for the encoder and
> > its MMU.
> > 
> > The series has been tested on a PINE64 Quartz64 Model A with an RK3566
> > SoC using GStreamer.
> > 
> > Regards,
> > Nicolas Frattaroli
> > 
> > Nicolas Frattaroli (3):
> >   dt-bindings: media: rockchip-vpu: Add RK3568 JPEG compatible
> >   media: hantro: Add support for RK356x JPEG encoder
> >   arm64: dts: rockchip: Add JPEG encoder node to rk356x
> > 
> >  .../bindings/media/rockchip-vpu.yaml          |  2 +
> >  arch/arm64/boot/dts/rockchip/rk356x.dtsi      | 21 +++++++
> >  drivers/staging/media/hantro/hantro_drv.c     |  1 +
> >  drivers/staging/media/hantro/hantro_hw.h      |  1 +
> >  .../staging/media/hantro/rockchip_vpu_hw.c    | 62 +++++++++++++++++++
> >  5 files changed, 87 insertions(+)
> > 
> 
>