Re: [PATCH v11 2/4] arm64: dts: qcom: x1e80100: add video node

[Yijie Yang <yijie.yang@oss.qualcomm.com>]

On 2025-09-12 16:45, Stephan Gerhold wrote:
> On Wed, Sep 10, 2025 at 05:02:10PM +0800, YijieYang wrote:
>> From: Wangao Wang <quic_wangaow@quicinc.com>
>>
>> Add the IRIS video-codec node on X1E80100 platform to support video
>> functionality.
>>
>> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>> Signed-off-by: Wangao Wang <quic_wangaow@quicinc.com>
>> Signed-off-by: Yijie Yang <yijie.yang@oss.qualcomm.com>
>> ---
>>   arch/arm64/boot/dts/qcom/x1e80100.dtsi | 82 ++++++++++++++++++++++++++++++++++
>>   1 file changed, 82 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
>> index 737c5dbd1c80..4a450738b695 100644
>> --- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
>> @@ -5186,6 +5186,88 @@ usb_1_ss1_dwc3_ss: endpoint {
>>   			};
>>   		};
>>   
>> +		iris: video-codec@aa00000 {
>> +			compatible = "qcom,x1e80100-iris", "qcom,sm8550-iris";
>> +
>> +			reg = <0x0 0x0aa00000 0x0 0xf0000>;
>> +			interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
>> +
>> +			power-domains = <&videocc VIDEO_CC_MVS0C_GDSC>,
>> +					<&videocc VIDEO_CC_MVS0_GDSC>,
>> +					<&rpmhpd RPMHPD_MXC>,
>> +					<&rpmhpd RPMHPD_MMCX>;
>> +			power-domain-names = "venus",
>> +					     "vcodec0",
>> +					     "mxc",
>> +					     "mmcx";
>> +			operating-points-v2 = <&iris_opp_table>;
>> +
>> +			clocks = <&gcc GCC_VIDEO_AXI0_CLK>,
>> +				 <&videocc VIDEO_CC_MVS0C_CLK>,
>> +				 <&videocc VIDEO_CC_MVS0_CLK>;
>> +			clock-names = "iface",
>> +				      "core",
>> +				      "vcodec0_core";
>> +
>> +			interconnects = <&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ACTIVE_ONLY
>> +					 &config_noc SLAVE_VENUS_CFG QCOM_ICC_TAG_ACTIVE_ONLY>,
>> +					<&mmss_noc MASTER_VIDEO QCOM_ICC_TAG_ALWAYS
>> +					 &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>;
>> +			interconnect-names = "cpu-cfg",
>> +					     "video-mem";
>> +
>> +			memory-region = <&video_mem>;
>> +
>> +			resets = <&gcc GCC_VIDEO_AXI0_CLK_ARES>;
>> +			reset-names = "bus";
>> +
>> +			iommus = <&apps_smmu 0x1940 0x0>,
>> +				 <&apps_smmu 0x1947 0x0>;
>> +			dma-coherent;
>> +
>> +			status = "disabled";
>> +
>> +			iris_opp_table: opp-table {
>> +				compatible = "operating-points-v2";
>> +
>> +				opp-192000000 {
>> +					opp-hz = /bits/ 64 <192000000>;
>> +					required-opps = <&rpmhpd_opp_low_svs_d1>,
>> +							<&rpmhpd_opp_low_svs_d1>;
>> +				};
>> +
>> +				opp-240000000 {
>> +					opp-hz = /bits/ 64 <240000000>;
>> +					required-opps = <&rpmhpd_opp_low_svs>,
>> +							<&rpmhpd_opp_low_svs>;
> 
> You need &rpmhpd_opp_svs here for one of the OPPs, because this
> describes not just the requirements for the derived clocks but also the
> requirements for the PLL itself. sm8550.dtsi has the same.
> 
> I didn't realize that you sent a DT patch for qcom,x1e80100-iris, so
> I sent my own patch yesterday [1] that was just waiting for the
> dt-bindings to land in linux-next.
> 
> Have you talked to your colleagues in the video team before submitting
> this patch? I'm pretty sure they could have pointed that out during
> internal review. They also have access to my patch (which has been
> shared in a public branch for over a year now) and knew I was going to
> send it as soon as the binding lands in linux-next. I just wish we could
> have coordinated this better to avoid the duplicate work. :/
> 
> I suggest that you add a dependency on my patch series or postpone
> enabling IRIS support for a follow up patch, it's better to have it
> separate from a new board addition.
> 
> Thanks,
> Stephan
> 
> [1]: https://lore.kernel.org/linux-arm-msm/20250911-x1e-iris-dt-v1-1-63caf0fd202c@linaro.org/

You're right. I checked with my colleagues, and I’ll remove it from my 
patch series and list yours as a dependency.

-- 
Best Regards,
Yijie