| Message ID | 20220926105047.19419-1-allen-kh.cheng@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] arm64: dts: mt8192: Add vcodec lat and core nodes | expand |
Il 26/09/22 12:50, Allen-KH Cheng ha scritto: > Add vcodec lat and core nodes for mt8192 SoC. > > Signed-off-by: Allen-KH Cheng <allen-kh.cheng@mediatek.com> > Tested-by: Chen-Yu Tsai <wenst@chromium.org> > --- > Change in v1: > * Replace node names with video-codec > [Allen-KH Cheng <allen-kh.cheng@mediatek.com>] > --- > --- > arch/arm64/boot/dts/mediatek/mt8192.dtsi | 60 ++++++++++++++++++++++++ > 1 file changed, 60 insertions(+) > > diff --git a/arch/arm64/boot/dts/mediatek/mt8192.dtsi b/arch/arm64/boot/dts/mediatek/mt8192.dtsi > index 6b20376191a7..214dfc6b0ed1 100644 > --- a/arch/arm64/boot/dts/mediatek/mt8192.dtsi > +++ b/arch/arm64/boot/dts/mediatek/mt8192.dtsi > @@ -1449,6 +1449,66 @@ > power-domains = <&spm MT8192_POWER_DOMAIN_ISP2>; > }; > > + vcodec_dec: video-codec@16000000 { > + compatible = "mediatek,mt8192-vcodec-dec"; > + reg = <0 0x16000000 0 0x1000>; > + mediatek,scp = <&scp>; > + iommus = <&iommu0 M4U_PORT_L4_VDEC_MC_EXT>; > + dma-ranges = <0x1 0x0 0x0 0x40000000 0x0 0xfff00000>; > + #address-cells = <2>; > + #size-cells = <2>; > + ranges = <0 0 0 0x16000000 0 0x26000>; > + > + vcodec_lat: video-codec@10000 { Allen, this won't work :-( Check Documentation/devicetree/bindings/media/mediatek,vcodec-subdev-decoder.yaml ...the schema requires vcodec-lat@[0-9a-f] and vcodec-core@[0-9a-f]. If you want to call them all video-codec@addr, you have to also fix the schema. P.S.: Did you try to run `make dtbs_check`? Regards, Angelo
Hi Angelo, On Mon, 2022-09-26 at 17:29 +0200, AngeloGioacchino Del Regno wrote: > Il 26/09/22 12:50, Allen-KH Cheng ha scritto: > > Add vcodec lat and core nodes for mt8192 SoC. > > > > Signed-off-by: Allen-KH Cheng <allen-kh.cheng@mediatek.com> > > Tested-by: Chen-Yu Tsai <wenst@chromium.org> > > --- > > Change in v1: > > * Replace node names with video-codec > > [Allen-KH Cheng <allen-kh.cheng@mediatek.com>] > > --- > > --- > > arch/arm64/boot/dts/mediatek/mt8192.dtsi | 60 > > ++++++++++++++++++++++++ > > 1 file changed, 60 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/mediatek/mt8192.dtsi > > b/arch/arm64/boot/dts/mediatek/mt8192.dtsi > > index 6b20376191a7..214dfc6b0ed1 100644 > > --- a/arch/arm64/boot/dts/mediatek/mt8192.dtsi > > +++ b/arch/arm64/boot/dts/mediatek/mt8192.dtsi > > @@ -1449,6 +1449,66 @@ > > power-domains = <&spm > > MT8192_POWER_DOMAIN_ISP2>; > > }; > > > > + vcodec_dec: video-codec@16000000 { > > + compatible = "mediatek,mt8192-vcodec-dec"; > > + reg = <0 0x16000000 0 0x1000>; > > + mediatek,scp = <&scp>; > > + iommus = <&iommu0 M4U_PORT_L4_VDEC_MC_EXT>; > > + dma-ranges = <0x1 0x0 0x0 0x40000000 0x0 > > 0xfff00000>; > > + #address-cells = <2>; > > + #size-cells = <2>; > > + ranges = <0 0 0 0x16000000 0 0x26000>; > > + > > + vcodec_lat: video-codec@10000 { > > Allen, this won't work :-( > Check Documentation/devicetree/bindings/media/mediatek,vcodec-subdev- > decoder.yaml > ...the schema requires vcodec-lat@[0-9a-f] and vcodec-core@[0-9a-f]. > > If you want to call them all video-codec@addr, you have to also fix > the schema. > > P.S.: Did you try to run `make dtbs_check`? > > Regards, > Angelo > Sorry, my bad. I alsways run `make dtbs_check` to confirm dtb with bindings. I just think we didn't limit node names in mtk-vodec bindings. I will pay attention next time. Since currently the vcodec lat and core nodes are absent from the mtk dts, do you think the child node name should be changed to something more general (ex: video-codec) in mediatek,vcodec-subdev-decoder bindings? Thanks, Allen
Il 27/09/22 04:19, Allen-KH Cheng (程冠勳) ha scritto: > Hi Angelo, > > On Mon, 2022-09-26 at 17:29 +0200, AngeloGioacchino Del Regno wrote: >> Il 26/09/22 12:50, Allen-KH Cheng ha scritto: >>> Add vcodec lat and core nodes for mt8192 SoC. >>> >>> Signed-off-by: Allen-KH Cheng <allen-kh.cheng@mediatek.com> >>> Tested-by: Chen-Yu Tsai <wenst@chromium.org> >>> --- >>> Change in v1: >>> * Replace node names with video-codec >>> [Allen-KH Cheng <allen-kh.cheng@mediatek.com>] >>> --- >>> --- >>> arch/arm64/boot/dts/mediatek/mt8192.dtsi | 60 >>> ++++++++++++++++++++++++ >>> 1 file changed, 60 insertions(+) >>> >>> diff --git a/arch/arm64/boot/dts/mediatek/mt8192.dtsi >>> b/arch/arm64/boot/dts/mediatek/mt8192.dtsi >>> index 6b20376191a7..214dfc6b0ed1 100644 >>> --- a/arch/arm64/boot/dts/mediatek/mt8192.dtsi >>> +++ b/arch/arm64/boot/dts/mediatek/mt8192.dtsi >>> @@ -1449,6 +1449,66 @@ >>> power-domains = <&spm >>> MT8192_POWER_DOMAIN_ISP2>; >>> }; >>> >>> + vcodec_dec: video-codec@16000000 { >>> + compatible = "mediatek,mt8192-vcodec-dec"; >>> + reg = <0 0x16000000 0 0x1000>; >>> + mediatek,scp = <&scp>; >>> + iommus = <&iommu0 M4U_PORT_L4_VDEC_MC_EXT>; >>> + dma-ranges = <0x1 0x0 0x0 0x40000000 0x0 >>> 0xfff00000>; >>> + #address-cells = <2>; >>> + #size-cells = <2>; >>> + ranges = <0 0 0 0x16000000 0 0x26000>; >>> + >>> + vcodec_lat: video-codec@10000 { >> >> Allen, this won't work :-( >> Check Documentation/devicetree/bindings/media/mediatek,vcodec-subdev- >> decoder.yaml >> ...the schema requires vcodec-lat@[0-9a-f] and vcodec-core@[0-9a-f]. >> >> If you want to call them all video-codec@addr, you have to also fix >> the schema. >> >> P.S.: Did you try to run `make dtbs_check`? >> >> Regards, >> Angelo >> > > Sorry, my bad. I alsways run `make dtbs_check` to confirm dtb with > bindings. I just think we didn't limit node names in mtk-vodec > bindings. I will pay attention next time. > > > Since currently the vcodec lat and core nodes are absent from the mtk > dts, do you think the child node name should be changed to something > more general (ex: video-codec) in mediatek,vcodec-subdev-decoder > bindings? The video codec is mt8192-vcodec-dec, while the other nodes are describing the VPU instances (and/or vpu cores)... I'm not sure. Krzysztof, please, can you give your opinion on that? Regards, Angelo > > Thanks, > Allen > > > > >
On 27/09/2022 12:17, AngeloGioacchino Del Regno wrote: >>> >> >> Sorry, my bad. I alsways run `make dtbs_check` to confirm dtb with >> bindings. I just think we didn't limit node names in mtk-vodec >> bindings. I will pay attention next time. >> >> >> Since currently the vcodec lat and core nodes are absent from the mtk >> dts, do you think the child node name should be changed to something >> more general (ex: video-codec) in mediatek,vcodec-subdev-decoder >> bindings? > > The video codec is mt8192-vcodec-dec, while the other nodes are describing > the VPU instances (and/or vpu cores)... I'm not sure. > > Krzysztof, please, can you give your opinion on that? > What's the difference between them? I understand parent device is entire block of consisting of multiple processing units? If so, video-codec actually could fit in both places. But feel free to call it a bit different (video-codec-core, video-codec-lat, processing-unit, even something less generic). Sometimes it's tricky to find nice name, so I wouldn't worry too much in that case. Just not "mt8192-vcodec" :) Best regards, Krzysztof
Il 28/09/22 09:04, Krzysztof Kozlowski ha scritto: > On 27/09/2022 12:17, AngeloGioacchino Del Regno wrote: >>>> >>> >>> Sorry, my bad. I alsways run `make dtbs_check` to confirm dtb with >>> bindings. I just think we didn't limit node names in mtk-vodec >>> bindings. I will pay attention next time. >>> >>> >>> Since currently the vcodec lat and core nodes are absent from the mtk >>> dts, do you think the child node name should be changed to something >>> more general (ex: video-codec) in mediatek,vcodec-subdev-decoder >>> bindings? >> >> The video codec is mt8192-vcodec-dec, while the other nodes are describing >> the VPU instances (and/or vpu cores)... I'm not sure. >> >> Krzysztof, please, can you give your opinion on that? >> > > What's the difference between them? I understand parent device is entire > block of consisting of multiple processing units? If so, video-codec > actually could fit in both places. But feel free to call it a bit > different (video-codec-core, video-codec-lat, processing-unit, even > something less generic). Sometimes it's tricky to find nice name, so I > wouldn't worry too much in that case. Just not "mt8192-vcodec" :) > The parent device is the entire block consisting of multiple processing units and has "global" control registers; children are LAT(s) and processing cores. From my understanding, the processing cores are physical cores of one big VPU and, depending on the actual (current gen) SoC, the VPU may have one or two cores. Right now, the bindings want vcodec-latX@addr, vcodec-coreX@addr (where X is a number, like vcodec-core0, vcodec-core1) but, in my opinion, changing that to video-codec-lat@addr and video-codec-core@addr would be more descriptive. ...Or should we simply leave the bindings as they are and just go with the abbreviated "vcodec-(hwtype)" names? Regards, Angelo
On 28/09/2022 09:58, AngeloGioacchino Del Regno wrote: > Il 28/09/22 09:04, Krzysztof Kozlowski ha scritto: >> On 27/09/2022 12:17, AngeloGioacchino Del Regno wrote: >>>>> >>>> >>>> Sorry, my bad. I alsways run `make dtbs_check` to confirm dtb with >>>> bindings. I just think we didn't limit node names in mtk-vodec >>>> bindings. I will pay attention next time. >>>> >>>> >>>> Since currently the vcodec lat and core nodes are absent from the mtk >>>> dts, do you think the child node name should be changed to something >>>> more general (ex: video-codec) in mediatek,vcodec-subdev-decoder >>>> bindings? >>> >>> The video codec is mt8192-vcodec-dec, while the other nodes are describing >>> the VPU instances (and/or vpu cores)... I'm not sure. >>> >>> Krzysztof, please, can you give your opinion on that? >>> >> >> What's the difference between them? I understand parent device is entire >> block of consisting of multiple processing units? If so, video-codec >> actually could fit in both places. But feel free to call it a bit >> different (video-codec-core, video-codec-lat, processing-unit, even >> something less generic). Sometimes it's tricky to find nice name, so I >> wouldn't worry too much in that case. Just not "mt8192-vcodec" :) >> > > The parent device is the entire block consisting of multiple processing units > and has "global" control registers; children are LAT(s) and processing cores. > > From my understanding, the processing cores are physical cores of one big VPU > and, depending on the actual (current gen) SoC, the VPU may have one or two > cores. > > Right now, the bindings want vcodec-latX@addr, vcodec-coreX@addr (where X is > a number, like vcodec-core0, vcodec-core1) but, in my opinion, changing that > to video-codec-lat@addr and video-codec-core@addr would be more descriptive. > > ...Or should we simply leave the bindings as they are and just go with the > abbreviated "vcodec-(hwtype)" names? video-codec-lat sounds better, but I am not sure if it is worth the churn, so I am fine with both. Best regards, Krzysztof
Il 28/09/22 19:26, Krzysztof Kozlowski ha scritto: > On 28/09/2022 09:58, AngeloGioacchino Del Regno wrote: >> Il 28/09/22 09:04, Krzysztof Kozlowski ha scritto: >>> On 27/09/2022 12:17, AngeloGioacchino Del Regno wrote: >>>>>> >>>>> >>>>> Sorry, my bad. I alsways run `make dtbs_check` to confirm dtb with >>>>> bindings. I just think we didn't limit node names in mtk-vodec >>>>> bindings. I will pay attention next time. >>>>> >>>>> >>>>> Since currently the vcodec lat and core nodes are absent from the mtk >>>>> dts, do you think the child node name should be changed to something >>>>> more general (ex: video-codec) in mediatek,vcodec-subdev-decoder >>>>> bindings? >>>> >>>> The video codec is mt8192-vcodec-dec, while the other nodes are describing >>>> the VPU instances (and/or vpu cores)... I'm not sure. >>>> >>>> Krzysztof, please, can you give your opinion on that? >>>> >>> >>> What's the difference between them? I understand parent device is entire >>> block of consisting of multiple processing units? If so, video-codec >>> actually could fit in both places. But feel free to call it a bit >>> different (video-codec-core, video-codec-lat, processing-unit, even >>> something less generic). Sometimes it's tricky to find nice name, so I >>> wouldn't worry too much in that case. Just not "mt8192-vcodec" :) >>> >> >> The parent device is the entire block consisting of multiple processing units >> and has "global" control registers; children are LAT(s) and processing cores. >> >> From my understanding, the processing cores are physical cores of one big VPU >> and, depending on the actual (current gen) SoC, the VPU may have one or two >> cores. >> >> Right now, the bindings want vcodec-latX@addr, vcodec-coreX@addr (where X is >> a number, like vcodec-core0, vcodec-core1) but, in my opinion, changing that >> to video-codec-lat@addr and video-codec-core@addr would be more descriptive. >> >> ...Or should we simply leave the bindings as they are and just go with the >> abbreviated "vcodec-(hwtype)" names? > > video-codec-lat sounds better, but I am not sure if it is worth the > churn, so I am fine with both. > Thank you Krzysztof! Allen, can you please change the binding to use "video-codec-lat" and "video-codec-core" (so change "vcodec" to "video-codec") and then use these names for this commit? Doing the change right now won't break the ABI, as there's no devicetree using that binding yet (with this commit adding the first user), so you can safely go for the rename without any complication. Regards, Angelo
On Thu, 2022-09-29 at 10:30 +0200, AngeloGioacchino Del Regno wrote: > Il 28/09/22 19:26, Krzysztof Kozlowski ha scritto: > > On 28/09/2022 09:58, AngeloGioacchino Del Regno wrote: > > > Il 28/09/22 09:04, Krzysztof Kozlowski ha scritto: > > > > On 27/09/2022 12:17, AngeloGioacchino Del Regno wrote: > > > > > > > > > > > > > > > > > > > Sorry, my bad. I alsways run `make dtbs_check` to confirm > > > > > > dtb with > > > > > > bindings. I just think we didn't limit node names in mtk- > > > > > > vodec > > > > > > bindings. I will pay attention next time. > > > > > > > > > > > > > > > > > > Since currently the vcodec lat and core nodes are absent > > > > > > from the mtk > > > > > > dts, do you think the child node name should be changed to > > > > > > something > > > > > > more general (ex: video-codec) in mediatek,vcodec-subdev- > > > > > > decoder > > > > > > bindings? > > > > > > > > > > The video codec is mt8192-vcodec-dec, while the other nodes > > > > > are describing > > > > > the VPU instances (and/or vpu cores)... I'm not sure. > > > > > > > > > > Krzysztof, please, can you give your opinion on that? > > > > > > > > > > > > > What's the difference between them? I understand parent device > > > > is entire > > > > block of consisting of multiple processing units? If so, video- > > > > codec > > > > actually could fit in both places. But feel free to call it a > > > > bit > > > > different (video-codec-core, video-codec-lat, processing-unit, > > > > even > > > > something less generic). Sometimes it's tricky to find nice > > > > name, so I > > > > wouldn't worry too much in that case. Just not "mt8192-vcodec" > > > > :) > > > > > > > > > > The parent device is the entire block consisting of multiple > > > processing units > > > and has "global" control registers; children are LAT(s) and > > > processing cores. > > > > > > From my understanding, the processing cores are physical cores > > > of one big VPU > > > and, depending on the actual (current gen) SoC, the VPU may have > > > one or two > > > cores. > > > > > > Right now, the bindings want vcodec-latX@addr, vcodec-coreX@addr > > > (where X is > > > a number, like vcodec-core0, vcodec-core1) but, in my opinion, > > > changing that > > > to video-codec-lat@addr and video-codec-core@addr would be more > > > descriptive. > > > > > > ...Or should we simply leave the bindings as they are and just go > > > with the > > > abbreviated "vcodec-(hwtype)" names? > > > > video-codec-lat sounds better, but I am not sure if it is worth the > > churn, so I am fine with both. > > > > Thank you Krzysztof! > > Allen, can you please change the binding to use "video-codec-lat" and > "video-codec-core" (so change "vcodec" to "video-codec") and then use > these names for this commit? > > Doing the change right now won't break the ABI, as there's no > devicetree > using that binding yet (with this commit adding the first user), so > you > can safely go for the rename without any complication. > > Regards, > Angelo > > > Hi Angelo, No problem, I will update in the following version. Thanks, Allen
diff --git a/arch/arm64/boot/dts/mediatek/mt8192.dtsi b/arch/arm64/boot/dts/mediatek/mt8192.dtsi index 6b20376191a7..214dfc6b0ed1 100644 --- a/arch/arm64/boot/dts/mediatek/mt8192.dtsi +++ b/arch/arm64/boot/dts/mediatek/mt8192.dtsi @@ -1449,6 +1449,66 @@ power-domains = <&spm MT8192_POWER_DOMAIN_ISP2>; }; + vcodec_dec: video-codec@16000000 { + compatible = "mediatek,mt8192-vcodec-dec"; + reg = <0 0x16000000 0 0x1000>; + mediatek,scp = <&scp>; + iommus = <&iommu0 M4U_PORT_L4_VDEC_MC_EXT>; + dma-ranges = <0x1 0x0 0x0 0x40000000 0x0 0xfff00000>; + #address-cells = <2>; + #size-cells = <2>; + ranges = <0 0 0 0x16000000 0 0x26000>; + + vcodec_lat: video-codec@10000 { + compatible = "mediatek,mtk-vcodec-lat"; + reg = <0x0 0x10000 0 0x800>; + interrupts = <GIC_SPI 426 IRQ_TYPE_LEVEL_HIGH 0>; + iommus = <&iommu0 M4U_PORT_L5_VDEC_LAT0_VLD_EXT>, + <&iommu0 M4U_PORT_L5_VDEC_LAT0_VLD2_EXT>, + <&iommu0 M4U_PORT_L5_VDEC_LAT0_AVC_MV_EXT>, + <&iommu0 M4U_PORT_L5_VDEC_LAT0_PRED_RD_EXT>, + <&iommu0 M4U_PORT_L5_VDEC_LAT0_TILE_EXT>, + <&iommu0 M4U_PORT_L5_VDEC_LAT0_WDMA_EXT>, + <&iommu0 M4U_PORT_L5_VDEC_LAT0_RG_CTRL_DMA_EXT>, + <&iommu0 M4U_PORT_L5_VDEC_UFO_ENC_EXT>; + clocks = <&topckgen CLK_TOP_VDEC_SEL>, + <&vdecsys_soc CLK_VDEC_SOC_VDEC>, + <&vdecsys_soc CLK_VDEC_SOC_LAT>, + <&vdecsys_soc CLK_VDEC_SOC_LARB1>, + <&topckgen CLK_TOP_MAINPLL_D4>; + clock-names = "sel", "soc-vdec", "soc-lat", "vdec", "top"; + assigned-clocks = <&topckgen CLK_TOP_VDEC_SEL>; + assigned-clock-parents = <&topckgen CLK_TOP_MAINPLL_D4>; + power-domains = <&spm MT8192_POWER_DOMAIN_VDEC>; + }; + + vcodec_core: video-codec@25000 { + compatible = "mediatek,mtk-vcodec-core"; + reg = <0 0x25000 0 0x1000>; + interrupts = <GIC_SPI 425 IRQ_TYPE_LEVEL_HIGH 0>; + iommus = <&iommu0 M4U_PORT_L4_VDEC_MC_EXT>, + <&iommu0 M4U_PORT_L4_VDEC_UFO_EXT>, + <&iommu0 M4U_PORT_L4_VDEC_PP_EXT>, + <&iommu0 M4U_PORT_L4_VDEC_PRED_RD_EXT>, + <&iommu0 M4U_PORT_L4_VDEC_PRED_WR_EXT>, + <&iommu0 M4U_PORT_L4_VDEC_PPWRAP_EXT>, + <&iommu0 M4U_PORT_L4_VDEC_TILE_EXT>, + <&iommu0 M4U_PORT_L4_VDEC_VLD_EXT>, + <&iommu0 M4U_PORT_L4_VDEC_VLD2_EXT>, + <&iommu0 M4U_PORT_L4_VDEC_AVC_MV_EXT>, + <&iommu0 M4U_PORT_L4_VDEC_RG_CTRL_DMA_EXT>; + clocks = <&topckgen CLK_TOP_VDEC_SEL>, + <&vdecsys CLK_VDEC_VDEC>, + <&vdecsys CLK_VDEC_LAT>, + <&vdecsys CLK_VDEC_LARB1>, + <&topckgen CLK_TOP_MAINPLL_D4>; + clock-names = "sel", "soc-vdec", "soc-lat", "vdec", "top"; + assigned-clocks = <&topckgen CLK_TOP_VDEC_SEL>; + assigned-clock-parents = <&topckgen CLK_TOP_MAINPLL_D4>; + power-domains = <&spm MT8192_POWER_DOMAIN_VDEC2>; + }; + }; + larb5: larb@1600d000 { compatible = "mediatek,mt8192-smi-larb"; reg = <0 0x1600d000 0 0x1000>;