On Sat, 2021-03-06 at 13:46 -0700, Rob Herring wrote: > On Thu, Feb 25, 2021 at 03:36:01PM +0800, Irui Wang wrote: > > Updates binding document since the avc and vp8 hardware encoder in > > MT8173 are now separated. Separate "mediatek,mt8173-vcodec-enc" to > > "mediatek,mt8173-vcodec-enc-vp8" and "mediatek,mt8173-vcodec-enc". > > This is not a compatible change. Please explain that here and why that's > okay (if it is). > Dear Rob, As the following commit message, this is a patch for smi cleaning up "mediatek,larb", or put the following message in the commit message: this patch separates the two devices, it's a preparing patch for adding device_link between the larbs and venc-device. It's mainly for fixing the problem: https://lkml.org/lkml/2019/9/3/316 Is this OK ? > > > > This is a preparing patch for smi cleaning up "mediatek,larb". > > > > Acked-by: Tiffany Lin <tiffany.lin@xxxxxxxxxxxx> > > Signed-off-by: Hsin-Yi Wang <hsinyi@xxxxxxxxxxxx> > > Signed-off-by: Maoguang Meng <maoguang.meng@xxxxxxxxxxxx> > > Signed-off-by: Irui Wang <irui.wang@xxxxxxxxxxxx> > > --- > > Change since v1: > > - rename compatible and device node > > --- > > .../bindings/media/mediatek-vcodec.txt | 55 ++++++++++--------- > > 1 file changed, 29 insertions(+), 26 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/media/mediatek-vcodec.txt b/Documentation/devicetree/bindings/media/mediatek-vcodec.txt > > index 8217424fd4bd..03209cbd7540 100644 > > --- a/Documentation/devicetree/bindings/media/mediatek-vcodec.txt > > +++ b/Documentation/devicetree/bindings/media/mediatek-vcodec.txt > > @@ -4,7 +4,9 @@ Mediatek Video Codec is the video codec hw present in Mediatek SoCs which > > supports high resolution encoding and decoding functionalities. > > > > Required properties: > > -- compatible : "mediatek,mt8173-vcodec-enc" for MT8173 encoder > > +- compatible : must be one of the following string: > > + "mediatek,mt8173-vcodec-enc-vp8" for mt8173 vp8 encoder. > > + "mediatek,mt8173-vcodec-enc" for mt8173 avc encoder. > > "mediatek,mt8183-vcodec-enc" for MT8183 encoder. > > "mediatek,mt8173-vcodec-dec" for MT8173 decoder. > > - reg : Physical base address of the video codec registers and length of > > @@ -13,10 +15,10 @@ Required properties: > > - mediatek,larb : must contain the local arbiters in the current Socs. > > - clocks : list of clock specifiers, corresponding to entries in > > the clock-names property. > > -- clock-names: encoder must contain "venc_sel_src", "venc_sel",, > > - "venc_lt_sel_src", "venc_lt_sel", decoder must contain "vcodecpll", > > - "univpll_d2", "clk_cci400_sel", "vdec_sel", "vdecpll", "vencpll", > > - "venc_lt_sel", "vdec_bus_clk_src". > > +- clock-names: > > + encoder must contain "venc_sel"; > > What happened to the other clocks? > > Seems like you are dropping what are parent clocks? That seems unrelated > to the VP8 split? If so, that's a separate change. Dear Rob, "venc_sel" is for avc encoder clock, "venc_sel_src" is its parent, we remove the parent because we set "assigned-clocks" and "assigned-clock-parents" same as venc_lt_sel" for vp8 encoder, we split the device node, so the clocks need split as well. or can we say: - clock-names: avc venc must contain "venc_sel"; vp8 venc must contain "venc_lt_sel"; Dear Alexandre, could you help to give some more suggestions about this? Thanks Best Regards > > > + decoder must contain "vcodecpll", "univpll_d2", "clk_cci400_sel", > > + "vdec_sel", "vdecpll", "vencpll", "venc_lt_sel", "vdec_bus_clk_src". > > - iommus : should point to the respective IOMMU block with master port as > > argument, see Documentation/devicetree/bindings/iommu/mediatek,iommu.txt > > for details. > > @@ -80,14 +82,10 @@ vcodec_dec: vcodec@16000000 { > > assigned-clock-rates = <0>, <0>, <0>, <1482000000>, <800000000>; > > }; > > > > - vcodec_enc: vcodec@18002000 { > > +vcodec_enc_avc: vcodec@18002000 { > > compatible = "mediatek,mt8173-vcodec-enc"; > > - reg = <0 0x18002000 0 0x1000>, /*VENC_SYS*/ > > - <0 0x19002000 0 0x1000>; /*VENC_LT_SYS*/ > > - interrupts = <GIC_SPI 198 IRQ_TYPE_LEVEL_LOW>, > > - <GIC_SPI 202 IRQ_TYPE_LEVEL_LOW>; > > - mediatek,larb = <&larb3>, > > - <&larb5>; > > + reg = <0 0x18002000 0 0x1000>; > > + interrupts = <GIC_SPI 198 IRQ_TYPE_LEVEL_LOW>; > > iommus = <&iommu M4U_PORT_VENC_RCPU>, > > <&iommu M4U_PORT_VENC_REC>, > > <&iommu M4U_PORT_VENC_BSDMA>, > > @@ -98,8 +96,20 @@ vcodec_dec: vcodec@16000000 { > > <&iommu M4U_PORT_VENC_REF_LUMA>, > > <&iommu M4U_PORT_VENC_REF_CHROMA>, > > <&iommu M4U_PORT_VENC_NBM_RDMA>, > > - <&iommu M4U_PORT_VENC_NBM_WDMA>, > > - <&iommu M4U_PORT_VENC_RCPU_SET2>, > > + <&iommu M4U_PORT_VENC_NBM_WDMA>; > > + mediatek,larb = <&larb3>; > > + mediatek,vpu = <&vpu>; > > + clocks = <&topckgen CLK_TOP_VENC_SEL>; > > + clock-names = "venc_sel"; > > + assigned-clocks = <&topckgen CLK_TOP_VENC_SEL>; > > + assigned-clock-parents = <&topckgen CLK_TOP_VCODECPLL>; > > + }; > > + > > +vcodec_enc_vp8: vcodec@19002000 { > > + compatible = "mediatek,mt8173-vcodec-enc-vp8"; > > + reg = <0 0x19002000 0 0x1000>; /* VENC_LT_SYS */ > > + interrupts = <GIC_SPI 202 IRQ_TYPE_LEVEL_LOW>; > > + iommus = <&iommu M4U_PORT_VENC_RCPU_SET2>, > > <&iommu M4U_PORT_VENC_REC_FRM_SET2>, > > <&iommu M4U_PORT_VENC_BSDMA_SET2>, > > <&iommu M4U_PORT_VENC_SV_COMA_SET2>, > > @@ -108,17 +118,10 @@ vcodec_dec: vcodec@16000000 { > > <&iommu M4U_PORT_VENC_CUR_CHROMA_SET2>, > > <&iommu M4U_PORT_VENC_REF_LUMA_SET2>, > > <&iommu M4U_PORT_VENC_REC_CHROMA_SET2>; > > + mediatek,larb = <&larb5>; > > mediatek,vpu = <&vpu>; > > - clocks = <&topckgen CLK_TOP_VENCPLL_D2>, > > - <&topckgen CLK_TOP_VENC_SEL>, > > - <&topckgen CLK_TOP_UNIVPLL1_D2>, > > - <&topckgen CLK_TOP_VENC_LT_SEL>; > > - clock-names = "venc_sel_src", > > - "venc_sel", > > - "venc_lt_sel_src", > > - "venc_lt_sel"; > > - assigned-clocks = <&topckgen CLK_TOP_VENC_SEL>, > > - <&topckgen CLK_TOP_VENC_LT_SEL>; > > - assigned-clock-parents = <&topckgen CLK_TOP_VENCPLL_D2>, > > - <&topckgen CLK_TOP_UNIVPLL1_D2>; > > + clocks = <&topckgen CLK_TOP_VENC_LT_SEL>; > > + clock-names = "venc_lt_sel"; > > + assigned-clocks = <&topckgen CLK_TOP_VENC_LT_SEL>; > > + assigned-clock-parents = <&topckgen CLK_TOP_VCODECPLL_370P5>; > > }; > > -- > > 2.25.1 > >
