Re: [PATCH v5 5/5] arm64: dts: qcom: qcm2290: Add venus video node

[Jorge Ramirez <jorge.ramirez@oss.qualcomm.com>]

On 27/06/25 20:28:29, Vikash Garodia wrote:
> 
> On 6/27/2025 6:03 PM, Jorge Ramirez wrote:
> > On 27/06/25 17:40:19, Vikash Garodia wrote:
> >>
> >> On 6/26/2025 7:29 PM, Jorge Ramirez-Ortiz wrote:
> >>> Add DT entries for the qcm2290 venus encoder/decoder.
> >>>
> >>> Co-developed-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> >>> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> >>> Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
> >>> ---
> >>>  arch/arm64/boot/dts/qcom/qcm2290.dtsi | 57 +++++++++++++++++++++++++++
> >>>  1 file changed, 57 insertions(+)
> >>>
> >>> diff --git a/arch/arm64/boot/dts/qcom/qcm2290.dtsi b/arch/arm64/boot/dts/qcom/qcm2290.dtsi
> >>> index f49ac1c1f8a3..5326c91a0ff0 100644
> >>> --- a/arch/arm64/boot/dts/qcom/qcm2290.dtsi
> >>> +++ b/arch/arm64/boot/dts/qcom/qcm2290.dtsi
> >>> @@ -1628,6 +1628,63 @@ adreno_smmu: iommu@59a0000 {
> >>>  			#iommu-cells = <2>;
> >>>  		};
> >>>  
> >>> +		venus: video-codec@5a00000 {
> >>> +			compatible = "qcom,qcm2290-venus";
> >>> +			reg = <0 0x5a00000 0 0xf0000>;
> >>> +			interrupts = <GIC_SPI 225 IRQ_TYPE_LEVEL_HIGH>;
> >>> +
> >>> +			power-domains = <&gcc GCC_VENUS_GDSC>,
> >>> +					<&gcc GCC_VCODEC0_GDSC>,
> >>> +					<&rpmpd QCM2290_VDDCX>;
> >>> +			power-domain-names = "venus",
> >>> +					     "vcodec0",
> >>> +					     "cx";
> >>> +			operating-points-v2 = <&venus_opp_table>;
> >>> +
> >>> +			clocks = <&gcc GCC_VIDEO_VENUS_CTL_CLK>,
> >>> +				 <&gcc GCC_VIDEO_AHB_CLK>,
> >>> +				 <&gcc GCC_VENUS_CTL_AXI_CLK>,
> >>> +				 <&gcc GCC_VIDEO_THROTTLE_CORE_CLK>,
> >>> +				 <&gcc GCC_VIDEO_VCODEC0_SYS_CLK>,
> >>> +				 <&gcc GCC_VCODEC0_AXI_CLK>;
> >>> +			clock-names = "core",
> >>> +				      "iface",
> >>> +				      "bus",
> >>> +				      "throttle",
> >>> +				      "vcodec0_core",
> >>> +				      "vcodec0_bus";
> >>> +
> >>> +			memory-region = <&pil_video_mem>;
> >>> +			iommus = <&apps_smmu 0x860 0x0>,
> >>> +				 <&apps_smmu 0x880 0x0>,
> >>> +				 <&apps_smmu 0x861 0x04>,
> >>> +				 <&apps_smmu 0x863 0x0>,
> >>> +				 <&apps_smmu 0x804 0xe0>;
> >> keep only the non secure ones.
> > 
> > ok
> > 
> >>> +
> >>> +			interconnects = <&mmnrt_virt MASTER_VIDEO_P0 RPM_ALWAYS_TAG
> >>> +					 &bimc SLAVE_EBI1 RPM_ALWAYS_TAG>,
> >>> +					<&bimc MASTER_APPSS_PROC RPM_ACTIVE_TAG
> >>> +					 &config_noc SLAVE_VENUS_CFG RPM_ACTIVE_TAG>;
> >>> +			interconnect-names = "video-mem",
> >>> +					     "cpu-cfg";
> >>> +
> >>> +			status = "okay";
> >>> +
> >>> +			venus_opp_table: opp-table {
> >>> +				compatible = "operating-points-v2";
> >>> +
> >>> +				opp-133000000 {
> >>> +					opp-hz = /bits/ 64 <133000000>;
> >>> +					required-opps = <&rpmpd_opp_low_svs>;
> >>> +				};
> >> Fix the corner freq value
> > 
> > can you add some reference please?
> > 
> > I took this data from an internal document - not sure why the downstream
> > driver supports different values or where those were taken from (AFAIK
> > they are not supported)
> Most likely you have referred incorrect downstream file. Refer scuba-vidc.dtsi.

I took them from actual documents (which might or might not be obsolete,
hard to say but they were the latest version and as such, they
contradict the downstream dtsi).

So I'd rather not use downstream - could you point me to the reference
you used please - I wonder if the fix is required downstream instead of here? 

> Again, good reference for such cases would IP catalogues and if not, gcc driver
> in this case which have structures defining different corners for
> video.

The PM document for this chip only confirms two values - the other 4 ones
claim they are not supported on 50_LT

but we can discuss offline.

> > 
> > 
> >>
> >> Regards,
> >> Vikash
> >>> +
> >>> +				opp-240000000 {
> >>> +					opp-hz = /bits/ 64 <240000000>;
> >>> +					required-opps = <&rpmpd_opp_svs>;
> >>> +				};
> >>> +			};
> >>> +		};
> >>> +
> >>>  		mdss: display-subsystem@5e00000 {
> >>>  			compatible = "qcom,qcm2290-mdss";
> >>>  			reg = <0x0 0x05e00000 0x0 0x1000>;