On Mon 22 Nov 05:26 CST 2021, Sankeerth Billakanti wrote:
> From: Krishna Manikandan <quic_mkri...@quicinc.com>
>
> Add mdss and mdp DT nodes for sc7280.
>
> Signed-off-by: Krishna Manikandan <quic_mkri...@quicinc.com>
> Reported-by: kernel test robot <l...@intel.com>
> Reviewed-by: Stephen Boyd <swb...@chromium.org>
> Reported-by: kernel test robot <l...@intel.com>
> Signed-off-by: Sankeerth Billakanti <quic_sbill...@quicinc.com>
> ---
>
> Changes in v4:
> None
>
> Changes in v3:
> None
>
> Changes in v2:
> - Rename display dt nodes (Stephen Boyd)
> - Add clock names one per line for readability (Stephen Boyd)
>
>
> arch/arm64/boot/dts/qcom/sc7280.dtsi | 90
> ++++++++++++++++++++++++++++++++++++
> 1 file changed, 90 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> index 365a2e0..a4536b6 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> @@ -2704,6 +2704,96 @@
> #power-domain-cells = <1>;
> };
>
> + mdss: display-subsystem@ae00000 {
> + compatible = "qcom,sc7280-mdss";
> + reg = <0 0x0ae00000 0 0x1000>;
> + reg-names = "mdss";
> +
> + power-domains = <&dispcc DISP_CC_MDSS_CORE_GDSC>;
> +
> + clocks = <&gcc GCC_DISP_AHB_CLK>,
> + <&dispcc DISP_CC_MDSS_AHB_CLK>,
> + <&dispcc DISP_CC_MDSS_MDP_CLK>;
> + clock-names = "iface",
> + "ahb",
> + "core";
> +
> + assigned-clocks = <&dispcc DISP_CC_MDSS_MDP_CLK>;
> + assigned-clock-rates = <300000000>;
> +
> + interrupts = <GIC_SPI 83 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-controller;
> + #interrupt-cells = <1>;
> +
> + interconnects = <&mmss_noc MASTER_MDP0 0 &mc_virt
> SLAVE_EBI1 0>;
> + interconnect-names = "mdp0-mem";
> +
> + iommus = <&apps_smmu 0x900 0x402>;
> +
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> +
> + status = "disabled";
> +
> + mdp: display-controller@ae01000 {
I believe the only reason to give this a label is so that you can enable
it in the dts. But I don't see the point of having it status disabled,
given that it should always follow the mdss node's status.
> + compatible = "qcom,sc7280-dpu";
> + reg = <0 0x0ae01000 0 0x8f030>,
> + <0 0x0aeb0000 0 0x2008>;
> + reg-names = "mdp", "vbif";
> +
> + clocks = <&gcc GCC_DISP_HF_AXI_CLK>,
> + <&gcc GCC_DISP_SF_AXI_CLK>,
> + <&dispcc DISP_CC_MDSS_AHB_CLK>,
> + <&dispcc DISP_CC_MDSS_MDP_LUT_CLK>,
> + <&dispcc DISP_CC_MDSS_MDP_CLK>,
> + <&dispcc DISP_CC_MDSS_VSYNC_CLK>;
> + clock-names = "bus",
> + "nrt_bus",
> + "iface",
> + "lut",
> + "core",
> + "vsync";
> + assigned-clocks = <&dispcc
> DISP_CC_MDSS_MDP_CLK>,
> + <&dispcc
> DISP_CC_MDSS_VSYNC_CLK>,
> + <&dispcc DISP_CC_MDSS_AHB_CLK>;
> + assigned-clock-rates = <300000000>,
> + <19200000>,
> + <19200000>;
> + operating-points-v2 = <&mdp_opp_table>;
> + power-domains = <&rpmhpd SC7280_CX>;
> +
> + interrupt-parent = <&mdss>;
> + interrupts = <0>;
> +
> + status = "disabled";
So my suggestion is to drop this and drop the label.
If not, please change the label of this node to mdss_mdp, for sorting
purposes.
Thanks,
Bjorn
> +
> + mdp_opp_table: opp-table {
> + compatible = "operating-points-v2";
> +
> + opp-200000000 {
> + opp-hz = /bits/ 64 <200000000>;
> + required-opps =
> <&rpmhpd_opp_low_svs>;
> + };
> +
> + opp-300000000 {
> + opp-hz = /bits/ 64 <300000000>;
> + required-opps =
> <&rpmhpd_opp_svs>;
> + };
> +
> + opp-380000000 {
> + opp-hz = /bits/ 64 <380000000>;
> + required-opps =
> <&rpmhpd_opp_svs_l1>;
> + };
> +
> + opp-506666667 {
> + opp-hz = /bits/ 64 <506666667>;
> + required-opps =
> <&rpmhpd_opp_nom>;
> + };
> + };
> + };
> + };
> +
> pdc: interrupt-controller@b220000 {
> compatible = "qcom,sc7280-pdc", "qcom,pdc";
> reg = <0 0x0b220000 0 0x30000>;
> --
> 2.7.4
>
