Re: [PATCH 1/9] dt-bindings: media: mtk-vcodec: Add binding for MT8195 two venc cores

[Irui Wang (王瑞)]

On Wed, 2021-08-25 at 00:49 -0300, Ezequiel Garcia wrote:
> On Tue, 24 Aug 2021 at 23:04, Irui Wang (王瑞) <Irui.Wang@xxxxxxxxxxxx>
> wrote:
> > 
> > Hi,Ezequiel,
> > 
> > Thanks for your reviewing.
> > 
> > On Tue, 2021-08-24 at 08:02 -0300, Ezequiel Garcia wrote:
> > > Hi Irui,
> > > 
> > > On Mon, 16 Aug 2021 at 08:00, Irui Wang <irui.wang@xxxxxxxxxxxx>
> > > wrote:
> > > > 
> > > > Enable MT8195 two H.264 venc cores, updates vcodec binding
> > > > document.
> > > > 
> > > > Signed-off-by: Irui Wang <irui.wang@xxxxxxxxxxxx>
> > > > ---
> > > >  Documentation/devicetree/bindings/media/mediatek-vcodec.txt |
> > > > 2 ++
> > > >  1 file changed, 2 insertions(+)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/media/mediatek-
> > > > vcodec.txt b/Documentation/devicetree/bindings/media/mediatek-
> > > > vcodec.txt
> > > > index de961699ba0a..eb2e24c32426 100644
> > > > --- a/Documentation/devicetree/bindings/media/mediatek-
> > > > vcodec.txt
> > > > +++ b/Documentation/devicetree/bindings/media/mediatek-
> > > > vcodec.txt
> > > > @@ -11,6 +11,8 @@ Required properties:
> > > >    "mediatek,mt8173-vcodec-dec" for MT8173 decoder.
> > > >    "mediatek,mt8192-vcodec-enc" for MT8192 encoder.
> > > >    "mediatek,mt8195-vcodec-enc" for MT8195 encoder.
> > > > +  "mediatek,mtk-venc-core0" for MT8195 avc core0 device.
> > > > +  "mediatek,mtk-venc-core1" for MT8195 avc core1 device.
> > > 
> > > What is the difference between core0 and core1?
> > > 
> > > Thanks,
> > > Ezequiel
> > 
> > Both core0 and core1 are H264 encoder hardware, they have their own
> > hardware register base, used power-domains/clocks/irqs. We can use
> > any
> > of them for H.264 encoding, but the two cores can work together for
> > higher performance, it's called "frame racing", a hardware encoding
> > mode, control flow just like in the commit messages:
> > 
> > core0 frame#0.frame#2.frame#4...
> > core1    frame#1.frame#3.frame#5...
> > 
> 
> If they are two encoder cores, why do you need different compatible
> strings?
> 
> It would be interesting to see a device tree which shows how this
> should
> be used in the real world, but from the looks of it, it seems you
> don't
> need a separate compatible.
> 
We want to use the two cores through one device node, core0 and core1
bind to device "mediatek,mt8195-vcodec-enc", it's feasible to make two
cores work by just opening one device, the device tree looks like:

venc {
    compatible = "mediatek,mt8195-vcodec-enc"
}
venc_core0 {
    compatible = "mediatek,mtk-venc-core0"
    interrupts = xxxx
    clocks = xxxx
    power-domains = xxxx
    ...
}

venc_core1 {
    compatible = "mediatek,mtk-venc-core1"
    interrupts = xxxx
    clocks = xxxx
    power-domains = xxxx
    ...
}

as you can see, node "venc" just has compatible string, it's just used
for register "/dev/videoX" device node in probe process. The encoder
real hardware device are node "venc_core0" and "venc_core1", bind them
to "venc" in probe process.

> It seems this series is somewhat related to Yunfei's "[PATCH v5,
> 00/15] Using component framework to support multi hardware decode",
> but I don't see a device tree patch either in that series.
> 
> Given this is a complex architecture, I don't know if it
> makes sense to discuss decoder and encoder independently.
> 
> If you guys unify the two series, and add the device tree patches for
> it,
> or at least for the most complex cases, maybe that will surface the
> architecture more clearly and come up with an easier solution that
> doesn't involve
> an async framework to pull in the parts together.
> 
> Thanks,
> Ezequiel

Yes, this series have a similar purpose with Yunfei's patch, he has a
requirement for dual cores decoder, mine are dual cores encoder, about
the usage of framework component, it can be discussed together.

Thanks