| Message ID | 20210225073603.5881-1-irui.wang@mediatek.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,1/3] dt-bindings: media: mtk-vcodec: Separating mtk vcodec encoder node | expand |
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). > > This is a preparing patch for smi cleaning up "mediatek,larb". > > Acked-by: Tiffany Lin <tiffany.lin@mediatek.com> > Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org> > Signed-off-by: Maoguang Meng <maoguang.meng@mediatek.com> > Signed-off-by: Irui Wang <irui.wang@mediatek.com> > --- > 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. > + 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 >
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@mediatek.com> > > Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org> > > Signed-off-by: Maoguang Meng <maoguang.meng@mediatek.com> > > Signed-off-by: Irui Wang <irui.wang@mediatek.com> > > --- > > 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 > >
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"; + 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>; };