On 5/19/2025 6:31 AM, Dmitry Baryshkov wrote:
> On Tue, May 13, 2025 at 03:56:10PM +0530, Ayushi Makhija wrote:
>> From: Ayushi Makhija <quic_amakh...@quicinc.com>
>>
>> Add device tree nodes for the DSI0 and DSI1 controllers
>> with their corresponding PHYs found on Qualcomm SA8775P SoC.
>>
>> Signed-off-by: Ayushi Makhija <quic_amakh...@quicinc.com>
>> Reviewed-by: Dmitry Baryshkov <lu...@kernel.org>
>> Reviewed-by: Konrad Dybcio <konrad.dyb...@oss.qualcomm.com>
>> ---
>>  arch/arm64/boot/dts/qcom/sa8775p.dtsi | 186 +++++++++++++++++++++++++-
>>  1 file changed, 185 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sa8775p.dtsi 
>> b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
>> index 5bd0c03476b1..f8777f17d24a 100644
>> --- a/arch/arm64/boot/dts/qcom/sa8775p.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
>> @@ -6,6 +6,7 @@
>>  
>>  #include <dt-bindings/interconnect/qcom,icc.h>
>>  #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +#include <dt-bindings/clock/qcom,dsi-phy-28nm.h>
>>  #include <dt-bindings/clock/qcom,rpmh.h>
>>  #include <dt-bindings/clock/qcom,sa8775p-dispcc.h>
>>  #include <dt-bindings/clock/qcom,sa8775p-gcc.h>
>> @@ -4034,6 +4035,22 @@ dpu_intf4_out: endpoint {
>>                                                      remote-endpoint = 
>> <&mdss0_dp1_in>;
>>                                              };
>>                                      };
>> +
>> +                                    port@2 {
>> +                                            reg = <2>;
>> +
>> +                                            dpu_intf1_out: endpoint {
>> +                                                    remote-endpoint = 
>> <&mdss0_dsi0_in>;
>> +                                            };
>> +                                    };
>> +
>> +                                    port@3 {
>> +                                            reg = <3>;
>> +
>> +                                            dpu_intf2_out: endpoint {
>> +                                                    remote-endpoint = 
>> <&mdss0_dsi1_in>;
>> +                                            };
>> +                                    };
>>                              };
>>  
>>                              mdss0_mdp_opp_table: opp-table {
>> @@ -4061,6 +4078,170 @@ opp-650000000 {
>>                              };
>>                      };
>>  
>> +                    mdss0_dsi0: dsi@ae94000 {
>> +                            compatible = "qcom,sa8775p-dsi-ctrl", 
>> "qcom,mdss-dsi-ctrl";
>> +                            reg = <0x0 0x0ae94000 0x0 0x400>;
>> +                            reg-names = "dsi_ctrl";
>> +
>> +                            interrupt-parent = <&mdss0>;
>> +                            interrupts = <4>;
>> +
>> +                            clocks = <&dispcc0 MDSS_DISP_CC_MDSS_BYTE0_CLK>,
>> +                                     <&dispcc0 
>> MDSS_DISP_CC_MDSS_BYTE0_INTF_CLK>,
>> +                                     <&dispcc0 MDSS_DISP_CC_MDSS_PCLK0_CLK>,
>> +                                     <&dispcc0 MDSS_DISP_CC_MDSS_ESC0_CLK>,
>> +                                     <&dispcc0 MDSS_DISP_CC_MDSS_AHB_CLK>,
>> +                                     <&gcc GCC_DISP_HF_AXI_CLK>;
>> +                            clock-names = "byte",
>> +                                          "byte_intf",
>> +                                          "pixel",
>> +                                          "core",
>> +                                          "iface",
>> +                                          "bus";
>> +                            assigned-clocks = <&dispcc0 
>> MDSS_DISP_CC_MDSS_BYTE0_CLK_SRC>,
>> +                                              <&dispcc0 
>> MDSS_DISP_CC_MDSS_PCLK0_CLK_SRC>;
>> +                            assigned-clock-parents = <&mdss0_dsi0_phy 
>> DSI_BYTE_PLL_CLK>,
>> +                                                     <&mdss0_dsi0_phy 
>> DSI_PIXEL_PLL_CLK>;
>> +                            phys = <&mdss0_dsi0_phy>;
>> +
>> +                            operating-points-v2 = <&dsi0_opp_table>;
>> +                            power-domains = <&rpmhpd SA8775P_MMCX>;
>> +
>> +                            #address-cells = <1>;
>> +                            #size-cells = <0>;
>> +
>> +                            status = "disabled";
>> +
>> +                            ports {
>> +                                    #address-cells = <1>;
>> +                                    #size-cells = <0>;
>> +
>> +                                    port@0 {
>> +                                            reg = <0>;
>> +
>> +                                            mdss0_dsi0_in: endpoint {
>> +                                                    remote-endpoint = 
>> <&dpu_intf1_out>;
>> +                                            };
>> +                                    };
>> +
>> +                                    port@1 {
>> +                                            reg = <1>;
>> +
>> +                                            mdss0_dsi0_out: endpoint{ };
>> +                                    };
>> +                            };
>> +
>> +                            dsi0_opp_table: opp-table {
> 
> mdss_dsi_opp_table: opp-table {}
> 
>> +                                    compatible = "operating-points-v2";
>> +
>> +                                    opp-358000000 {
> 
> Is there only one entry? Usually there are several.
> 

Hi Dmitry,

Thanks, for the review.

In the IP catalog Clock documentation of SA8775P, the same DSI clock frequency 
(358Mhz)
is mentioned for all the voltage corners (svs_l1, nom, turbo and turbo_l1).
That's why I kept the single entry opp-358000000 for 358Mhz and selected lowest 
voltage corner svs_l1.
 
I will address rest of the comments in next version of series.

Thanks,
Ayushi

>> +                                            opp-hz = /bits/ 64 <358000000>;
>> +                                            required-opps = 
>> <&rpmhpd_opp_svs_l1>;
>> +                                    };
>> +                            };
>> +                    };
>> +
>> +                    mdss0_dsi0_phy: phy@ae94400 {
>> +                            compatible = "qcom,sa8775p-dsi-phy-5nm";
>> +                            reg = <0x0 0x0ae94400 0x0 0x200>,
>> +                                  <0x0 0x0ae94600 0x0 0x280>,
>> +                                  <0x0 0x0ae94900 0x0 0x27c>;
>> +                            reg-names = "dsi_phy",
>> +                                        "dsi_phy_lane",
>> +                                        "dsi_pll";
>> +
>> +                            #clock-cells = <1>;
>> +                            #phy-cells = <0>;
>> +
>> +                            clocks = <&dispcc0 MDSS_DISP_CC_MDSS_AHB_CLK>,
>> +                                     <&rpmhcc RPMH_CXO_CLK>;
>> +                            clock-names = "iface", "ref";
>> +
>> +                            status = "disabled";
>> +                    };
>> +
>> +                    mdss0_dsi1: dsi@ae96000 {
>> +                            compatible = "qcom,sa8775p-dsi-ctrl", 
>> "qcom,mdss-dsi-ctrl";
>> +                            reg = <0x0 0x0ae96000 0x0 0x400>;
>> +                            reg-names = "dsi_ctrl";
>> +
>> +                            interrupt-parent = <&mdss0>;
>> +                            interrupts = <5>;
>> +
>> +                            clocks = <&dispcc0 MDSS_DISP_CC_MDSS_BYTE1_CLK>,
>> +                                     <&dispcc0 
>> MDSS_DISP_CC_MDSS_BYTE1_INTF_CLK>,
>> +                                     <&dispcc0 MDSS_DISP_CC_MDSS_PCLK1_CLK>,
>> +                                     <&dispcc0 MDSS_DISP_CC_MDSS_ESC1_CLK>,
>> +                                     <&dispcc0 MDSS_DISP_CC_MDSS_AHB_CLK>,
>> +                                     <&gcc GCC_DISP_HF_AXI_CLK>;
>> +                            clock-names = "byte",
>> +                                          "byte_intf",
>> +                                          "pixel",
>> +                                          "core",
>> +                                          "iface",
>> +                                          "bus";
>> +                            assigned-clocks = <&dispcc0 
>> MDSS_DISP_CC_MDSS_BYTE1_CLK_SRC>,
>> +                                              <&dispcc0 
>> MDSS_DISP_CC_MDSS_PCLK1_CLK_SRC>;
>> +                            assigned-clock-parents = <&mdss0_dsi1_phy 
>> DSI_BYTE_PLL_CLK>,
>> +                                                     <&mdss0_dsi1_phy 
>> DSI_PIXEL_PLL_CLK>;
>> +                            phys = <&mdss0_dsi1_phy>;
>> +
>> +                            operating-points-v2 = <&dsi1_opp_table>;
>> +                            power-domains = <&rpmhpd SA8775P_MMCX>;
>> +
>> +                            #address-cells = <1>;
>> +                            #size-cells = <0>;
>> +
>> +                            status = "disabled";
>> +
>> +                            ports {
>> +                                    #address-cells = <1>;
>> +                                    #size-cells = <0>;
>> +
>> +                                    port@0 {
>> +                                            reg = <0>;
>> +
>> +                                            mdss0_dsi1_in: endpoint {
>> +                                                    remote-endpoint = 
>> <&dpu_intf2_out>;
>> +                                            };
>> +                                    };
>> +
>> +                                    port@1 {
>> +                                            reg = <1>;
>> +
>> +                                            mdss0_dsi1_out: endpoint { };
>> +                                    };
>> +                            };
>> +
>> +                            dsi1_opp_table: opp-table {
> 
> You don't need a second DSI OPP table.
> 
>> +                                    compatible = "operating-points-v2";
>> +
>> +                                    opp-358000000 {
>> +                                            opp-hz = /bits/ 64 <358000000>;
>> +                                            required-opps = 
>> <&rpmhpd_opp_svs_l1>;
>> +                                    };
>> +                            };
>> +                    };
>> +
>> +                    mdss0_dsi1_phy: phy@ae96400 {
>> +                            compatible = "qcom,sa8775p-dsi-phy-5nm";
>> +                            reg = <0x0 0x0ae96400 0x0 0x200>,
>> +                                  <0x0 0x0ae96600 0x0 0x280>,
>> +                                  <0x0 0x0ae96900 0x0 0x27c>;
>> +                            reg-names = "dsi_phy",
>> +                                        "dsi_phy_lane",
>> +                                        "dsi_pll";
>> +
>> +                            #clock-cells = <1>;
>> +                            #phy-cells = <0>;
>> +
>> +                            clocks = <&dispcc0 MDSS_DISP_CC_MDSS_AHB_CLK>,
>> +                                     <&rpmhcc RPMH_CXO_CLK>;
>> +                            clock-names = "iface", "ref";
>> +
>> +                            status = "disabled";
>> +                    };
>> +
>>                      mdss0_dp0_phy: phy@aec2a00 {
>>                              compatible = "qcom,sa8775p-edp-phy";
>>  
>> @@ -4267,7 +4448,10 @@ dispcc0: clock-controller@af00000 {
>>                               <&sleep_clk>,
>>                               <&mdss0_dp0_phy 0>, <&mdss0_dp0_phy 1>,
>>                               <&mdss0_dp1_phy 0>, <&mdss0_dp1_phy 1>,
>> -                             <0>, <0>, <0>, <0>;
>> +                             <&mdss0_dsi0_phy DSI_BYTE_PLL_CLK>,
>> +                             <&mdss0_dsi0_phy DSI_PIXEL_PLL_CLK>,
>> +                             <&mdss0_dsi1_phy DSI_BYTE_PLL_CLK>,
>> +                             <&mdss0_dsi1_phy DSI_PIXEL_PLL_CLK>;
>>                      power-domains = <&rpmhpd SA8775P_MMCX>;
>>                      #clock-cells = <1>;
>>                      #reset-cells = <1>;
>> -- 
>> 2.34.1
>>
> 

Reply via email to