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

Jorge Ramirez-Ortiz posted 5 patches 3 months, 2 weeks ago
There is a newer version of this series
[PATCH v5 5/5] arm64: dts: qcom: qcm2290: Add venus video node
Posted by Jorge Ramirez-Ortiz 3 months, 2 weeks ago
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>;
+
+			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>;
+				};
+
+				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>;
-- 
2.34.1
Re: [PATCH v5 5/5] arm64: dts: qcom: qcm2290: Add venus video node
Posted by Vikash Garodia 3 months, 1 week ago
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.
> +
> +			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

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>;
Re: [PATCH v5 5/5] arm64: dts: qcom: qcm2290: Add venus video node
Posted by Jorge Ramirez 3 months, 1 week ago
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)


> 
> 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>;
Re: [PATCH v5 5/5] arm64: dts: qcom: qcm2290: Add venus video node
Posted by Vikash Garodia 3 months, 1 week ago
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.
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.
> 
> 
>>
>> 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>;
Re: [PATCH v5 5/5] arm64: dts: qcom: qcm2290: Add venus video node
Posted by Jorge Ramirez 3 months, 1 week ago
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>;
Re: [PATCH v5 5/5] arm64: dts: qcom: qcm2290: Add venus video node
Posted by Vikash Garodia 3 months, 1 week ago
On 6/27/2025 8:38 PM, Jorge Ramirez wrote:
> 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?

You can look for this file gcc-scuba.c and refer gcc_video_venus_clk_src which
is the src for different venus clocks.

> 
>> 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>;
Re: [PATCH v5 5/5] arm64: dts: qcom: qcm2290: Add venus video node
Posted by Jorge Ramirez 3 months ago
On 27/06/25 20:42:45, Vikash Garodia wrote:
> 
> On 6/27/2025 8:38 PM, Jorge Ramirez wrote:
> > 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?
> 
> You can look for this file gcc-scuba.c and refer gcc_video_venus_clk_src which
> is the src for different venus clocks.

sure, but the question remains, how do I know these are correct when the
documentation I have claims the opposite?

AFAIK downstream could be wrong, no?

> 
> > 
> >> 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>;
Re: [PATCH v5 5/5] arm64: dts: qcom: qcm2290: Add venus video node
Posted by Konrad Dybcio 3 months, 1 week ago
On 6/27/25 5:12 PM, Vikash Garodia wrote:
> 
> On 6/27/2025 8:38 PM, Jorge Ramirez wrote:
>> 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?
> 
> You can look for this file gcc-scuba.c and refer gcc_video_venus_clk_src which
> is the src for different venus clocks.

This is not a good source in general, GCC often has more rates defined
than the consumer is supposed to finally run at (because they were deemed
power-inefficient or similar, you can pretty much set any rate you want
on the clocks fwiw)

Konrad
Re: [PATCH v5 5/5] arm64: dts: qcom: qcm2290: Add venus video node
Posted by Vikash Garodia 3 months, 1 week ago
On 6/27/2025 8:50 PM, Konrad Dybcio wrote:
> On 6/27/25 5:12 PM, Vikash Garodia wrote:
>>
>> On 6/27/2025 8:38 PM, Jorge Ramirez wrote:
>>> 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?
>>
>> You can look for this file gcc-scuba.c and refer gcc_video_venus_clk_src which
>> is the src for different venus clocks.
> 
> This is not a good source in general, GCC often has more rates defined
> than the consumer is supposed to finally run at (because they were deemed
> power-inefficient or similar, you can pretty much set any rate you want
> on the clocks fwiw)
Count wise, i agree. Value-wise, afaik, corners should match OR are you saying
client scaling request for 133.0 MHz is ok when src is generating 133.33 MHz ?

> 
> Konrad
Re: [PATCH v5 5/5] arm64: dts: qcom: qcm2290: Add venus video node
Posted by Konrad Dybcio 3 months, 1 week ago
On 6/27/25 5:23 PM, Vikash Garodia wrote:
> 
> On 6/27/2025 8:50 PM, Konrad Dybcio wrote:
>> On 6/27/25 5:12 PM, Vikash Garodia wrote:
>>>
>>> On 6/27/2025 8:38 PM, Jorge Ramirez wrote:
>>>> 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?
>>>
>>> You can look for this file gcc-scuba.c and refer gcc_video_venus_clk_src which
>>> is the src for different venus clocks.
>>
>> This is not a good source in general, GCC often has more rates defined
>> than the consumer is supposed to finally run at (because they were deemed
>> power-inefficient or similar, you can pretty much set any rate you want
>> on the clocks fwiw)
> Count wise, i agree. Value-wise, afaik, corners should match OR are you saying
> client scaling request for 133.0 MHz is ok when src is generating 133.33 MHz ?

I *think* we're running a closest-match in there.. but I'd love to
have the clock and consumer drivers agree on the rate exactly
(which in this case would be 133333333 - and the clock plan in
our docs agrees)

Konrad
Re: [PATCH v5 5/5] arm64: dts: qcom: qcm2290: Add venus video node
Posted by Jorge Ramirez 3 months ago
On 27/06/25 17:27:04, Konrad Dybcio wrote:
> On 6/27/25 5:23 PM, Vikash Garodia wrote:
> > 
> > On 6/27/2025 8:50 PM, Konrad Dybcio wrote:
> >> On 6/27/25 5:12 PM, Vikash Garodia wrote:
> >>>
> >>> On 6/27/2025 8:38 PM, Jorge Ramirez wrote:
> >>>> 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?
> >>>
> >>> You can look for this file gcc-scuba.c and refer gcc_video_venus_clk_src which
> >>> is the src for different venus clocks.
> >>
> >> This is not a good source in general, GCC often has more rates defined
> >> than the consumer is supposed to finally run at (because they were deemed
> >> power-inefficient or similar, you can pretty much set any rate you want
> >> on the clocks fwiw)
> > Count wise, i agree. Value-wise, afaik, corners should match OR are you saying
> > client scaling request for 133.0 MHz is ok when src is generating 133.33 MHz ?
> 
> I *think* we're running a closest-match in there.. but I'd love to
> have the clock and consumer drivers agree on the rate exactly
> (which in this case would be 133333333 - and the clock plan in
> our docs agrees)
>

ok, will use this instead

> Konrad
Re: [PATCH v5 5/5] arm64: dts: qcom: qcm2290: Add venus video node
Posted by Krzysztof Kozlowski 3 months, 2 weeks ago
On 26/06/2025 15:59, Jorge Ramirez-Ortiz wrote:
> +
> +			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";

Drop, unless you override existing node, but then this should follow
standard override-label/phandle syntax.

Best regards,
Krzysztof
Re: [PATCH v5 5/5] arm64: dts: qcom: qcm2290: Add venus video node
Posted by Jorge Ramirez 3 months, 2 weeks ago
On 26/06/25 16:05:00, Krzysztof Kozlowski wrote:
> On 26/06/2025 15:59, Jorge Ramirez-Ortiz wrote:
> > +
> > +			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";
> 
> Drop, unless you override existing node, but then this should follow
> standard override-label/phandle syntax.
>

yep
Re: [PATCH v5 5/5] arm64: dts: qcom: qcm2290: Add venus video node
Posted by Konrad Dybcio 3 months, 1 week ago
On 6/26/25 4:25 PM, Jorge Ramirez wrote:
> On 26/06/25 16:05:00, Krzysztof Kozlowski wrote:
>> On 26/06/2025 15:59, Jorge Ramirez-Ortiz wrote:
>>> +
>>> +			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";
>>
>> Drop, unless you override existing node, but then this should follow
>> standard override-label/phandle syntax.
>>
> 
> yep

With that taken care of:

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad