diff mbox series

[v4,01/17] media: dt-binding: mtk-vcodec: Separating mtk-vcodec encode node.

Message ID 1590826218-23653-2-git-send-email-yong.wu@mediatek.com (mailing list archive)
State New, archived
Headers show
Series Clean up "mediatek,larb" after adding device_link | expand

Commit Message

Yong Wu (吴勇) May 30, 2020, 8:10 a.m. UTC
From: Maoguang Meng <maoguang.meng@mediatek.com>

Update binding document since the avc and vp8 hardware encoder in
mt8173 are now separated. Separate "mediatek,mt8173-vcodec-enc" to
"mediatek,mt8173-vcodec-vp8-enc" and "mediatek,mt8173-vcodec-avc-enc".

This is a preparing patch for smi cleaning up "mediatek,larb".

Signed-off-by: Maoguang Meng <maoguang.meng@mediatek.com>
Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
Signed-off-by: Irui Wang <irui.wang@mediatek.com>
Signed-off-by: Yong Wu <yong.wu@mediatek.com>
---
 .../devicetree/bindings/media/mediatek-vcodec.txt  | 58 ++++++++++++----------
 1 file changed, 31 insertions(+), 27 deletions(-)

Comments

Rob Herring (Arm) June 9, 2020, 9:21 p.m. UTC | #1
On Sat, May 30, 2020 at 04:10:02PM +0800, Yong Wu wrote:
> From: Maoguang Meng <maoguang.meng@mediatek.com>
> 
> Update binding document since the avc and vp8 hardware encoder in
> mt8173 are now separated. Separate "mediatek,mt8173-vcodec-enc" to
> "mediatek,mt8173-vcodec-vp8-enc" and "mediatek,mt8173-vcodec-avc-enc".

The h/w suddenly split in 2? You are breaking compatibility. Up to the 
Mediatek maintainers to decide if that's okay, but you need to state you 
are breaking compatibility (here and in the driver) and why that is 
okay.

> 
> This is a preparing patch for smi cleaning up "mediatek,larb".
> 
> Signed-off-by: Maoguang Meng <maoguang.meng@mediatek.com>
> Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> Signed-off-by: Irui Wang <irui.wang@mediatek.com>
> Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> ---
>  .../devicetree/bindings/media/mediatek-vcodec.txt  | 58 ++++++++++++----------
>  1 file changed, 31 insertions(+), 27 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/mediatek-vcodec.txt b/Documentation/devicetree/bindings/media/mediatek-vcodec.txt
> index 8093335..1023740 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-vp8-enc" for mt8173 vp8 encoder.
> +  "mediatek,mt8173-vcodec-avc-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,11 @@ 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:
> +   avc venc must contain "venc_sel";
> +   vp8 venc must contain "venc_lt_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 +83,10 @@ vcodec_dec: vcodec@16000000 {
>      assigned-clock-rates = <0>, <0>, <0>, <1482000000>, <800000000>;
>    };
>  
> -  vcodec_enc: 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>;
> +vcodec_enc: vcodec@18002000 {
> +    compatible = "mediatek,mt8173-vcodec-avc-enc";
> +    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 +97,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_lt: vcodec@19002000 {
> +    compatible = "mediatek,mt8173-vcodec-vp8-enc";
> +    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 +119,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>;
>    };
> -- 
> 1.9.1
Alexandre Courbot June 10, 2020, 6:46 a.m. UTC | #2
On Wed, Jun 10, 2020 at 6:21 AM Rob Herring <robh@kernel.org> wrote:
>
> On Sat, May 30, 2020 at 04:10:02PM +0800, Yong Wu wrote:
> > From: Maoguang Meng <maoguang.meng@mediatek.com>
> >
> > Update binding document since the avc and vp8 hardware encoder in
> > mt8173 are now separated. Separate "mediatek,mt8173-vcodec-enc" to
> > "mediatek,mt8173-vcodec-vp8-enc" and "mediatek,mt8173-vcodec-avc-enc".
>
> The h/w suddenly split in 2? You are breaking compatibility. Up to the
> Mediatek maintainers to decide if that's okay, but you need to state you
> are breaking compatibility (here and in the driver) and why that is
> okay.

In my understanding there is no real hardware using the old bindings
at the moment, and the split is indeed a reflection of the actual
hardware layout. Tiffany, can you give your acked-by if this change is
ok with you?

>
> >
> > This is a preparing patch for smi cleaning up "mediatek,larb".
> >
> > Signed-off-by: Maoguang Meng <maoguang.meng@mediatek.com>
> > Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> > Signed-off-by: Irui Wang <irui.wang@mediatek.com>
> > Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> > ---
> >  .../devicetree/bindings/media/mediatek-vcodec.txt  | 58 ++++++++++++----------
> >  1 file changed, 31 insertions(+), 27 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/media/mediatek-vcodec.txt b/Documentation/devicetree/bindings/media/mediatek-vcodec.txt
> > index 8093335..1023740 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-vp8-enc" for mt8173 vp8 encoder.
> > +  "mediatek,mt8173-vcodec-avc-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,11 @@ 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:
> > +   avc venc must contain "venc_sel";
> > +   vp8 venc must contain "venc_lt_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 +83,10 @@ vcodec_dec: vcodec@16000000 {
> >      assigned-clock-rates = <0>, <0>, <0>, <1482000000>, <800000000>;
> >    };
> >
> > -  vcodec_enc: 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>;
> > +vcodec_enc: vcodec@18002000 {
> > +    compatible = "mediatek,mt8173-vcodec-avc-enc";
> > +    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 +97,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_lt: vcodec@19002000 {
> > +    compatible = "mediatek,mt8173-vcodec-vp8-enc";
> > +    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 +119,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>;
> >    };
> > --
> > 1.9.1
tiffany.lin June 10, 2020, 7:38 a.m. UTC | #3
On Wed, 2020-06-10 at 15:46 +0900, Alexandre Courbot wrote:
> On Wed, Jun 10, 2020 at 6:21 AM Rob Herring <robh@kernel.org> wrote:
> >
> > On Sat, May 30, 2020 at 04:10:02PM +0800, Yong Wu wrote:
> > > From: Maoguang Meng <maoguang.meng@mediatek.com>
> > >
> > > Update binding document since the avc and vp8 hardware encoder in
> > > mt8173 are now separated. Separate "mediatek,mt8173-vcodec-enc" to
> > > "mediatek,mt8173-vcodec-vp8-enc" and "mediatek,mt8173-vcodec-avc-enc".
> >
> > The h/w suddenly split in 2? You are breaking compatibility. Up to the
> > Mediatek maintainers to decide if that's okay, but you need to state you
> > are breaking compatibility (here and in the driver) and why that is
> > okay.
> 
> In my understanding there is no real hardware using the old bindings
> at the moment, and the split is indeed a reflection of the actual
> hardware layout. Tiffany, can you give your acked-by if this change is
> ok with you?
> 

In my opinion, there is no need to change mt8173 dts for driver to
support mt8183.
I saw another patch that already make change to have encoder driver
support both mt8173 and mt8183.
But they done a lot to prove h264 and vp8 encoder could work
independently and parallel.
In this case, I am ok with it because dts should be a reflection of the
actual hardware.



> >
> > >
> > > This is a preparing patch for smi cleaning up "mediatek,larb".
> > >
> > > Signed-off-by: Maoguang Meng <maoguang.meng@mediatek.com>
> > > Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> > > Signed-off-by: Irui Wang <irui.wang@mediatek.com>
> > > Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> > > ---
> > >  .../devicetree/bindings/media/mediatek-vcodec.txt  | 58 ++++++++++++----------
> > >  1 file changed, 31 insertions(+), 27 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/media/mediatek-vcodec.txt b/Documentation/devicetree/bindings/media/mediatek-vcodec.txt
> > > index 8093335..1023740 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-vp8-enc" for mt8173 vp8 encoder.
> > > +  "mediatek,mt8173-vcodec-avc-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,11 @@ 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:
> > > +   avc venc must contain "venc_sel";
> > > +   vp8 venc must contain "venc_lt_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 +83,10 @@ vcodec_dec: vcodec@16000000 {
> > >      assigned-clock-rates = <0>, <0>, <0>, <1482000000>, <800000000>;
> > >    };
> > >
> > > -  vcodec_enc: 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>;
> > > +vcodec_enc: vcodec@18002000 {
> > > +    compatible = "mediatek,mt8173-vcodec-avc-enc";
> > > +    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 +97,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_lt: vcodec@19002000 {
> > > +    compatible = "mediatek,mt8173-vcodec-vp8-enc";
> > > +    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 +119,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>;
> > >    };
> > > --
> > > 1.9.1
tiffany.lin June 17, 2020, 5:53 a.m. UTC | #4
On Wed, 2020-06-10 at 15:38 +0800, Tiffany Lin wrote:
> On Wed, 2020-06-10 at 15:46 +0900, Alexandre Courbot wrote:
> > On Wed, Jun 10, 2020 at 6:21 AM Rob Herring <robh@kernel.org> wrote:
> > >
> > > On Sat, May 30, 2020 at 04:10:02PM +0800, Yong Wu wrote:
> > > > From: Maoguang Meng <maoguang.meng@mediatek.com>
> > > >
> > > > Update binding document since the avc and vp8 hardware encoder in
> > > > mt8173 are now separated. Separate "mediatek,mt8173-vcodec-enc" to
> > > > "mediatek,mt8173-vcodec-vp8-enc" and "mediatek,mt8173-vcodec-avc-enc".
> > >
> > > The h/w suddenly split in 2? You are breaking compatibility. Up to the
> > > Mediatek maintainers to decide if that's okay, but you need to state you
> > > are breaking compatibility (here and in the driver) and why that is
> > > okay.
> > 
> > In my understanding there is no real hardware using the old bindings
> > at the moment, and the split is indeed a reflection of the actual
> > hardware layout. Tiffany, can you give your acked-by if this change is
> > ok with you?
> > 
> 
> In my opinion, there is no need to change mt8173 dts for driver to
> support mt8183.
> I saw another patch that already make change to have encoder driver
> support both mt8173 and mt8183.
> But they done a lot to prove h264 and vp8 encoder could work
> independently and parallel.
> In this case, I am ok with it because dts should be a reflection of the
> actual hardware.
> 
> 
> 
> > >
> > > >
> > > > This is a preparing patch for smi cleaning up "mediatek,larb".
> > > >
Acked-by: Tiffany Lin <tiffany.lin@mediatek.com>


> > > > Signed-off-by: Maoguang Meng <maoguang.meng@mediatek.com>
> > > > Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> > > > Signed-off-by: Irui Wang <irui.wang@mediatek.com>
> > > > Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> > > > ---
> > > >  .../devicetree/bindings/media/mediatek-vcodec.txt  | 58 ++++++++++++----------
> > > >  1 file changed, 31 insertions(+), 27 deletions(-)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/media/mediatek-vcodec.txt b/Documentation/devicetree/bindings/media/mediatek-vcodec.txt
> > > > index 8093335..1023740 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-vp8-enc" for mt8173 vp8 encoder.
> > > > +  "mediatek,mt8173-vcodec-avc-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,11 @@ 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:
> > > > +   avc venc must contain "venc_sel";
> > > > +   vp8 venc must contain "venc_lt_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 +83,10 @@ vcodec_dec: vcodec@16000000 {
> > > >      assigned-clock-rates = <0>, <0>, <0>, <1482000000>, <800000000>;
> > > >    };
> > > >
> > > > -  vcodec_enc: 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>;
> > > > +vcodec_enc: vcodec@18002000 {
> > > > +    compatible = "mediatek,mt8173-vcodec-avc-enc";
> > > > +    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 +97,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_lt: vcodec@19002000 {
> > > > +    compatible = "mediatek,mt8173-vcodec-vp8-enc";
> > > > +    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 +119,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>;
> > > >    };
> > > > --
> > > > 1.9.1
>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/media/mediatek-vcodec.txt b/Documentation/devicetree/bindings/media/mediatek-vcodec.txt
index 8093335..1023740 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-vp8-enc" for mt8173 vp8 encoder.
+  "mediatek,mt8173-vcodec-avc-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,11 @@  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:
+   avc venc must contain "venc_sel";
+   vp8 venc must contain "venc_lt_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 +83,10 @@  vcodec_dec: vcodec@16000000 {
     assigned-clock-rates = <0>, <0>, <0>, <1482000000>, <800000000>;
   };
 
-  vcodec_enc: 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>;
+vcodec_enc: vcodec@18002000 {
+    compatible = "mediatek,mt8173-vcodec-avc-enc";
+    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 +97,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_lt: vcodec@19002000 {
+    compatible = "mediatek,mt8173-vcodec-vp8-enc";
+    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 +119,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>;
   };