| Message ID | 20241213-add-venus-for-qcs615-v4-3-7e2c9a72d309@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | media: venus: enable venus on qcs615 | expand |
On 13/12/2024 09:56, Renjiang Han wrote: > + video-decoder { > + compatible = "venus-decoder"; > + }; > + > + video-encoder { > + compatible = "venus-encoder"; > + }; To be honest, I'd prefer not to continue on doing this. Adding a compat string to existing yaml might work-around the issue but, it doesn't really _account_ for the issue. I've posted a series to fix the problem 20241209-media-staging-24-11-25-rb3-hw-compat-string-v5-0-ef7e5f85f302@linaro.org Either that or deeper rewrite of venus to remove the above dependencies should go ahead. I'm not in favour of willfully continuing to do the wrong thing, especially when per above, there's a working solution for it. --- bod
On Fri, Dec 13, 2024 at 03:26:48PM +0530, Renjiang Han wrote: Subject should be prefixed per the file being changed, i.e: "arm64: dts: qcom: qcs615: Add Venus" > Add venus node into devicetree for the qcs615 video and fallback > qcs615 to sc7180 due to the same video core. > > Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com> > --- > arch/arm64/boot/dts/qcom/qcs615.dtsi | 86 ++++++++++++++++++++++++++++++++++++ > 1 file changed, 86 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/qcs615.dtsi b/arch/arm64/boot/dts/qcom/qcs615.dtsi > index 37a189e0834d2f4b75ed9deb6fff73da163cb3a3..c08da80c7fd8fa8c69aff04b14784b821ce3ea13 100644 > --- a/arch/arm64/boot/dts/qcom/qcs615.dtsi > +++ b/arch/arm64/boot/dts/qcom/qcs615.dtsi > @@ -427,6 +427,11 @@ smem_region: smem@86000000 { > no-map; > hwlocks = <&tcsr_mutex 3>; > }; > + > + pil_video_mem: pil-video@93400000 { > + reg = <0x0 0x93400000 0x0 0x500000>; > + no-map; > + }; > }; > > soc: soc@0 { > @@ -2806,6 +2811,87 @@ gem_noc: interconnect@9680000 { > qcom,bcm-voters = <&apps_bcm_voter>; > }; > > + venus: video-codec@aa00000 { > + compatible = "qcom,qcs615-venus", "qcom,sc7180-venus"; > + reg = <0x0 0x0aa00000 0x0 0x100000>; > + interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>; > + > + clocks = <&videocc VIDEO_CC_VENUS_CTL_CORE_CLK>, > + <&videocc VIDEO_CC_VENUS_AHB_CLK>, > + <&videocc VIDEO_CC_VENUS_CTL_AXI_CLK>, > + <&videocc VIDEO_CC_VCODEC0_CORE_CLK>, > + <&videocc VIDEO_CC_VCODEC0_AXI_CLK>; > + clock-names = "core", > + "iface", > + "bus", > + "vcodec0_core", > + "vcodec0_bus"; > + > + power-domains = <&videocc VENUS_GDSC>, > + <&videocc VCODEC0_GDSC>, > + <&rpmhpd RPMHPD_CX>; > + power-domain-names = "venus", > + "vcodec0", > + "cx"; > + > + operating-points-v2 = <&venus_opp_table>; > + > + interconnects = <&mmss_noc MASTER_VIDEO_P0 QCOM_ICC_TAG_ALWAYS > + &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>, > + <&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ALWAYS > + &config_noc SLAVE_VENUS_CFG QCOM_ICC_TAG_ALWAYS>; > + interconnect-names = "video-mem", > + "cpu-cfg"; > + > + iommus = <&apps_smmu 0xe40 0x20>; > + > + memory-region = <&pil_video_mem>; > + > + status = "disabled"; > + > + video-decoder { > + compatible = "venus-decoder"; > + }; > + > + video-encoder { > + compatible = "venus-encoder"; > + }; > + > + venus_opp_table: opp-table { 'o' < 'v', so this should come above video-decoder. Regards, Bjorn > + compatible = "operating-points-v2"; > + > + opp-133330000 { > + opp-hz = /bits/ 64 <133330000>; > + required-opps = <&rpmhpd_opp_low_svs>; > + }; > + > + opp-240000000 { > + opp-hz = /bits/ 64 <240000000>; > + required-opps = <&rpmhpd_opp_svs>; > + }; > + > + opp-300000000 { > + opp-hz = /bits/ 64 <300000000>; > + required-opps = <&rpmhpd_opp_svs_l1>; > + }; > + > + opp-380000000 { > + opp-hz = /bits/ 64 <380000000>; > + required-opps = <&rpmhpd_opp_nom>; > + }; > + > + opp-410000000 { > + opp-hz = /bits/ 64 <410000000>; > + required-opps = <&rpmhpd_opp_turbo>; > + }; > + > + opp-460000000 { > + opp-hz = /bits/ 64 <460000000>; > + required-opps = <&rpmhpd_opp_turbo_l1>; > + }; > + }; > + }; > + > videocc: clock-controller@ab00000 { > compatible = "qcom,qcs615-videocc"; > reg = <0 0xab00000 0 0x10000>; > > -- > 2.34.1 >
On 12/13/2024 7:31 PM, Bryan O'Donoghue wrote: > On 13/12/2024 09:56, Renjiang Han wrote: >> + video-decoder { >> + compatible = "venus-decoder"; >> + }; >> + >> + video-encoder { >> + compatible = "venus-encoder"; >> + }; > > To be honest, I'd prefer not to continue on doing this. > > Adding a compat string to existing yaml might work-around the issue > but, it doesn't really _account_ for the issue. > > I've posted a series to fix the problem > > 20241209-media-staging-24-11-25-rb3-hw-compat-string-v5-0-ef7e5f85f302@linaro.org > > > Either that or deeper rewrite of venus to remove the above > dependencies should go ahead. > > I'm not in favour of willfully continuing to do the wrong thing, > especially when per above, there's a working solution for it. > > --- > bod Thanks for your review. Your change is a good way. But your change is a big change, involving many platforms, I am not sure if other guys have comments. Currently, my change only refers to SC7180 and falls back QCS615 to SC7180. Now I have verified my patches on SC7180 and QCS615. I think if your change has passed the review, then we only need to remove the video-decoder and video-encoder nodes in this device tree.
diff --git a/arch/arm64/boot/dts/qcom/qcs615.dtsi b/arch/arm64/boot/dts/qcom/qcs615.dtsi index 37a189e0834d2f4b75ed9deb6fff73da163cb3a3..c08da80c7fd8fa8c69aff04b14784b821ce3ea13 100644 --- a/arch/arm64/boot/dts/qcom/qcs615.dtsi +++ b/arch/arm64/boot/dts/qcom/qcs615.dtsi @@ -427,6 +427,11 @@ smem_region: smem@86000000 { no-map; hwlocks = <&tcsr_mutex 3>; }; + + pil_video_mem: pil-video@93400000 { + reg = <0x0 0x93400000 0x0 0x500000>; + no-map; + }; }; soc: soc@0 { @@ -2806,6 +2811,87 @@ gem_noc: interconnect@9680000 { qcom,bcm-voters = <&apps_bcm_voter>; }; + venus: video-codec@aa00000 { + compatible = "qcom,qcs615-venus", "qcom,sc7180-venus"; + reg = <0x0 0x0aa00000 0x0 0x100000>; + interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>; + + clocks = <&videocc VIDEO_CC_VENUS_CTL_CORE_CLK>, + <&videocc VIDEO_CC_VENUS_AHB_CLK>, + <&videocc VIDEO_CC_VENUS_CTL_AXI_CLK>, + <&videocc VIDEO_CC_VCODEC0_CORE_CLK>, + <&videocc VIDEO_CC_VCODEC0_AXI_CLK>; + clock-names = "core", + "iface", + "bus", + "vcodec0_core", + "vcodec0_bus"; + + power-domains = <&videocc VENUS_GDSC>, + <&videocc VCODEC0_GDSC>, + <&rpmhpd RPMHPD_CX>; + power-domain-names = "venus", + "vcodec0", + "cx"; + + operating-points-v2 = <&venus_opp_table>; + + interconnects = <&mmss_noc MASTER_VIDEO_P0 QCOM_ICC_TAG_ALWAYS + &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>, + <&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ALWAYS + &config_noc SLAVE_VENUS_CFG QCOM_ICC_TAG_ALWAYS>; + interconnect-names = "video-mem", + "cpu-cfg"; + + iommus = <&apps_smmu 0xe40 0x20>; + + memory-region = <&pil_video_mem>; + + status = "disabled"; + + video-decoder { + compatible = "venus-decoder"; + }; + + video-encoder { + compatible = "venus-encoder"; + }; + + venus_opp_table: opp-table { + compatible = "operating-points-v2"; + + opp-133330000 { + opp-hz = /bits/ 64 <133330000>; + required-opps = <&rpmhpd_opp_low_svs>; + }; + + opp-240000000 { + opp-hz = /bits/ 64 <240000000>; + required-opps = <&rpmhpd_opp_svs>; + }; + + opp-300000000 { + opp-hz = /bits/ 64 <300000000>; + required-opps = <&rpmhpd_opp_svs_l1>; + }; + + opp-380000000 { + opp-hz = /bits/ 64 <380000000>; + required-opps = <&rpmhpd_opp_nom>; + }; + + opp-410000000 { + opp-hz = /bits/ 64 <410000000>; + required-opps = <&rpmhpd_opp_turbo>; + }; + + opp-460000000 { + opp-hz = /bits/ 64 <460000000>; + required-opps = <&rpmhpd_opp_turbo_l1>; + }; + }; + }; + videocc: clock-controller@ab00000 { compatible = "qcom,qcs615-videocc"; reg = <0 0xab00000 0 0x10000>;
Add venus node into devicetree for the qcs615 video and fallback qcs615 to sc7180 due to the same video core. Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com> --- arch/arm64/boot/dts/qcom/qcs615.dtsi | 86 ++++++++++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+)