Message-ID: <CAAEAJfCdvj4drV+YSbvyR30b0+50RpJcOYdEoNODTdrAoL6+vA@mail.gmail.com>
Date:   Sun, 22 Aug 2021 11:32:32 -0300
From:   Ezequiel Garcia <ezequiel@...guardiasur.com.ar>
To:     "yunfei.dong@...iatek.com" <yunfei.dong@...iatek.com>
Cc:     Daniel Vetter <daniel@...ll.ch>,
        dri-devel <dri-devel@...ts.freedesktop.org>,
        Alexandre Courbot <acourbot@...omium.org>,
        Hans Verkuil <hverkuil-cisco@...all.nl>,
        Tzung-Bi Shih <tzungbi@...omium.org>,
        Tiffany Lin <tiffany.lin@...iatek.com>,
        Andrew-CT Chen <andrew-ct.chen@...iatek.com>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Matthias Brugger <matthias.bgg@...il.com>,
        Tomasz Figa <tfiga@...gle.com>,
        Hsin-Yi Wang <hsinyi@...omium.org>,
        Fritz Koenig <frkoenig@...omium.org>,
        Irui Wang <irui.wang@...iatek.com>,
        linux-media <linux-media@...r.kernel.org>,
        devicetree <devicetree@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
        srv_heupstream <srv_heupstream@...iatek.com>,
        "moderated list:ARM/Mediatek SoC support" 
        <linux-mediatek@...ts.infradead.org>,
        Project_Global_Chrome_Upstream_Group@...iatek.com,
        George Sun <george.sun@...iatek.com>
Subject: Re: [PATCH v5, 00/15] Using component framework to support multi
 hardware decode

On Fri, 20 Aug 2021 at 04:59, yunfei.dong@...iatek.com
<yunfei.dong@...iatek.com> wrote:
>
> Hi Ezequiel,
>
> Thanks for your detail feedback.
>
> On Thu, 2021-08-19 at 11:10 -0300, Ezequiel Garcia wrote:
> > On Thu, 19 Aug 2021 at 04:13, yunfei.dong@...iatek.com
> > <yunfei.dong@...iatek.com> wrote:
> > >
> > > Hi Ezequiel,
> > >
> > > Thanks for your suggestion.
> > >
> > > On Wed, 2021-08-18 at 11:11 -0300, Ezequiel Garcia wrote:
> > > > +danvet
> > > >
> > > > Hi,
> > > >
> > > > On Tue, 10 Aug 2021 at 23:58, Yunfei Dong <yunfei.dong@...iatek.com>
> > > > wrote:
> > > > >
> > > > > This series adds support for multi hardware decode into mtk-vcodec,
> > > > > by first
> > > > > adding component framework to manage each hardware information:
> > > > > interrupt,
> > > > > clock, register bases and power. Secondly add core thread to deal
> > > > > with core
> > > > > hardware message, at the same time, add msg queue for different
> > > > > hardware
> > > > > share messages. Lastly, the architecture of different specs are not
> > > > > the same,
> > > > > using specs type to separate them.
> > > > >
> > > >
> > > > I don't think it's a good idea to introduce the component API in the
> > > > media subsystem. It doesn't seem to be maintained, IRC there's not
> > > > even
> > > > a maintainer for it, and it has some issues that were never
> > > > addressed.
> > > >
> > > > It would be really important to avoid it. Is it really needed in the
> > > > first place?
> > > >
> > > > Thanks,
> > > > Ezequiel
> > >
> > > For there are many hardware need to use, mt8192 is three and mt8195 is
> > > five. Maybe need more to be used in the feature.
> > >
> > > Each hardware has independent clk/power/iommu port/irq.
> > > Use component interface in prob to get each component's information.
> > > Just enable the hardware when need to use it, very convenient and
> > > simple.
> > >
> > > I found that there are many modules use component to manage hardware
> > > information, such as iommu and drm etc.
> > >
> >
> > Many drivers support multiple hardware variants, where each variant
> > has a different number of clocks or interrupts, see for instance
> > struct hantro_variant which allows to expose different codec cores,
> > some having both decoder/encoder, and some having just a decoder.
> >
> > The component API is mostly used by DRM to aggregate independent
> > subdevices (called components) into an aggregated driver.
> >
> > For instance, a DRM driver needs to glue together the HDMI, MIPI,
> > and plany controller, or any other hardware arrangement where
> > devices can be described independently.
> >
> The usage scenario is very similar with drm and iommu, So decide to use
> component framework.
> Decode has three/five or more hardwares, these hardware are independent.
> For mt8183 just need core hardware to decode, but mt8192 has lat,soc and
> core hardware to decode. When lat need to use, just enable lat hardware,
> core is the same.And mt8195 will has two cores, each core can work well
> independent.
>
> For each component device just used to open their power/clk/iommu
> port/irq when master need to enable it. The main logic is in master
> device.
>
> > The component API may look simple but has some issues, it's not easy
> > to debug, and can cause troubles if not used as expected [1].
> > It's worth making sure you actually need a framework
> > to glue different devices together.
> >
> Each hardware has its index, master can get hardware information
> according these index, looks not complex. What do you mean about not
> easy to debug?
>
> > > Do you have any other suggestion for this architecture?
> > >
> >
> > Looking at the different patchsets that are posted, it's not clear
> > to me what exactly are the different architectures that you intend
> > to support, can you some documentation which clarifies that?
> >
> Have five hardwares lat,soc,core0,core1 and main. Lat thread can use lat
> soc and main, core thread can use soc,lat, core0 and core1. Core thread
> can be used or not for different project.

Can you explain what are these lat,soc and core threads for?

> Also Need to use these
> hardware dynamic at the same time. So I use component framework, just
> need to know the used  hardware index according to different
> project.Need not to do complex logic to manage these hardwares.
>

I am not thrilled to see the component framework introduced to the
media subsystem. Like I said, it has no clear maintainer, and it's not
easy to use.

The media subsystem has some support which AFAIK does the same thing,
see v4l2-async, which is maintained by media people.

Please push a branch based on media/master containing these changes.
I see there are other patch series for this device, but it's hard to track
which goes first, etc.

Thanks,
Ezequiel

Powered by blists - more mailing lists