Re: [PATCH v6, 00/15] Using component framework to support multi hardware decode

[yunfei.dong@xxxxxxxxxxxx]

Hi Ezequiel,

Could you please help to give some feedback when you are free for iommu
limitation?

According to google's suggestion, it's better not to use v4l2 async
also.
If there are no further comments, I don't have other choice for this
architecture.

Thanks,
Yunfei Dong
On Tue, 2021-09-14 at 20:16 +0800, yunfei.dong@xxxxxxxxxxxx wrote:
> Hi Ezequiel,
> 
> On Fri, 2021-09-03 at 11:08 +0800, yunfei.dong@xxxxxxxxxxxx wrote:
> > Hi Ezequiel,
> > 
> > Thanks for your suggestion.
> > On Thu, 2021-09-02 at 13:30 -0300, Ezequiel Garcia wrote:
> > > On Wed, 1 Sept 2021 at 05:32, Yunfei Dong <
> > > yunfei.dong@xxxxxxxxxxxx
> > > > 
> > > 
> > > 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.
> > > > 
> > > > This series has been tested with both MT8183 and MT8173.
> > > > Decoding
> > > > was working
> > > > for both chips.
> > > > 
> > > > Patches 1~3 rewrite get register bases and power on/off
> > > > interface.
> > > > 
> > > > Patch 4 add component framework to support multi hardware.
> > > > 
> > > > Patch 5 separate video encoder and decoder document
> > > > 
> > > > Patches 6-15 add interfaces to support core hardware.
> > > > ----
> > > > This patch dependents on : "media: mtk-vcodec: support for
> > > > MT8183
> > > > decoder"[1] and
> > > > "Mediatek MT8192 clock support"[2].
> > > > 
> > > > 1: Multi hardware decode is based on stateless decoder, MT8183
> > > > is
> > > > the first time
> > > > to add stateless decoder. Otherwise it will cause conflict.
> > > > This
> > > > patch will be
> > > > accepted in 5.15[1].
> > > > 
> > > > 2: The definition of decoder clocks are in mt8192-clk.h, this
> > > > patch
> > > > already in clk tree[2].
> > > > 
> > > > [1]
> > > > 
> 
> https://patchwork.linuxtv.org/project/linux-media/list/?series=5826
> > > > [2]
> > > > 
> 
> 
https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git/commit/?h=clk-next&id=f35f1a23e0e12e3173e9e9dedbc150d139027189
> > > > ----
> > > > Changes compared with v5:
> > > > -Add decoder hardware block diagram for patch 13/15
> > > > 
> > > 
> > > 
> > > The discussion on v5 was still on-going, so sending this v6
> > > is not helpful. The context for v5's discussion is now harder to
> > > find.
> > > 
> > > Please avoid sending a new version without properly
> > > discussing all the feedback, and without reaching consensus.
> > > This is very important, please keep it in mind.
> > > 
> > 
> > Thanks for your remind, I will keep this patch until get the
> > solution.
> > 
> > > Specifically, the feedback on v5 was NAK, with the request to
> > > avoid
> > > using any async framework, and instead try to find a simpler
> > > solution.
> > > 
> > > For instance, you can model things with a bus-like pattern,
> > > which ties all the devices together, under a parent node.
> > > This pattern is common in the kernel, the parent
> > > node can use of_platform_populate or similar
> > > (git grep of_platform_populate, you will see plenty of examples).
> > > 
> > > You will still have to do some work to have the proper
> > > regs resources, but this is doable. Each child is a device,
> > > so it can have its own resources (clocks, interrupts, iommus).
> > > 
> > > You don't need any async framework.
> > > 
> 
> Thanks for your suggestion very much, and there are several actions
> need to check.
> 
> 1: The iommu register like this:
> ret = bus_set_iommu(&platform_bus_type,
> &mtk_iommu_ops); 
> It expect the consumer is a standard platform device.
> otherwise it
> could not enter to the iommu of_xlate.)
> 
> So if putting the iommus property in the child node, all the child
> device need to registered as platform device.
> 
> 2: For the interrupt in each child node, but the logical processing
> in
> parent part. Child and parent need to send message for each other. In
> order to control clk/power/irq for multi instance, need send message
> to
> child to separate different hardware; child also need send message to
> parent when get interrupt.
> 
> 3: About Chen-Yu's mail, do you have any advice?
> 
> Do you have any suggestion about these two scenarios?
> I'm very happy to get your reply.
> 
> Thanks
> Yunfei Dong
> 
> > >     vcodec_dec: vcodec_dec@16000000 {
> > >         compatible = "mediatek,mt8192-vcodec-dec";
> > >         reg = <something>;
> > >         mediatek,scp = <&scp>;
> > >         iommus = <&iommu0 M4U_PORT_L4_VDEC_MC_EXT>;
> > >         dma-ranges = <0x1 0x0 0x0 0x40000000 0x0 0xfff00000>;
> > > 
> > >         vcodec_lat@0x10000 {
> > >             compatible = "mediatek,mtk-vcodec-lat";
> > >             reg = <0x10000 0x800>;      /* VDEC_MISC */
> > >             interrupts = <GIC_SPI 426 IRQ_TYPE_LEVEL_HIGH 0>;
> > >             // etc
> > >         };
> > > 
> > >         vcodec_core@0x25000 {
> > >            compatible = "mediatek,mtk-vcodec-core";
> > >            reg = <0x25000 0x1000>;      /* VDEC_CORE_MISC */
> > >            interrupts = <GIC_SPI 425 IRQ_TYPE_LEVEL_HIGH 0>;
> > >            // etc
> > >         };
> > >     };
> > > 
> > > Thanks,
> > > Ezequiel