[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAD=FV=U5DPxR54YM38w4_QVp24VorQ2eYDGq2GXScAs9APTygA@mail.gmail.com>
Date: Tue, 4 Feb 2020 11:13:54 -0800
From: Doug Anderson <dianders@...omium.org>
To: Harigovindan P <harigovi@...eaurora.org>
Cc: dri-devel <dri-devel@...ts.freedesktop.org>,
linux-arm-msm <linux-arm-msm@...r.kernel.org>,
freedreno <freedreno@...ts.freedesktop.org>,
"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
<devicetree@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org>,
Rob Clark <robdclark@...il.com>,
Sean Paul <seanpaul@...omium.org>,
"Kristian H. Kristensen" <hoegsberg@...omium.org>,
Kalyan Thota <kalyan_t@...eaurora.org>, nganji@...eaurora.org,
Taniya Das <tdas@...eaurora.org>
Subject: Re: [v5] arm64: dts: sc7180: add display dt nodes
Hi,
On Tue, Feb 4, 2020 at 6:15 AM Harigovindan P <harigovi@...eaurora.org> wrote:
>
> Add display, DSI hardware DT nodes for sc7180.
>
> Co-developed-by: Kalyan Thota <kalyan_t@...eaurora.org>
> Signed-off-by: Kalyan Thota <kalyan_t@...eaurora.org>
> Signed-off-by: Harigovindan P <harigovi@...eaurora.org>
> ---
>
> Changes in v1:
> - Added display DT nodes for sc7180
> Changes in v2:
> - Renamed node names
> - Corrected code alignments
> - Removed extra new line
> - Added DISP AHB clock for register access
> under display_subsystem node for global settings
> Changes in v3:
> - Modified node names
> - Modified hard coded values
> - Removed mdss reg entry
> Changes in v4:
> - Reverting mdp node name
> - Setting status to disabled in main SOC dtsi file
> - Replacing _ to - for node names
> - Adding clock dependency patch link
> - Splitting idp dt file to a separate patch
> Changes in v5:
> - Renaming "gcc_bus" to "bus" as per bindings (Doug Anderson)
> - Making status as disabled for mdss and mdss_mdp by default (Doug Anderson)
> - Removing "disp_cc" register space (Doug Anderson)
> - Renaming "dsi_controller" to "dsi" as per bindings (Doug Anderson)
> - Providing "ref" clk for dsi_phy (Doug Anderson)
> - Sorting mdss node before dispcc (Doug Anderson)
>
> This patch has dependency on the below series
> https://lkml.org/lkml/2019/12/27/73
You should have probably pointed to [1] which is a much newer version.
> arch/arm64/boot/dts/qcom/sc7180.dtsi | 136 ++++++++++++++++++++++++++++++++++-
> 1 file changed, 134 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> index bd2584d..3ac1b87 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> @@ -1173,13 +1173,145 @@
> #power-domain-cells = <1>;
> };
>
> + mdss: mdss@...0000 {
> + compatible = "qcom,sc7180-mdss";
> + reg = <0 0x0ae00000 0 0x1000>;
> + reg-names = "mdss";
> +
> + power-domains = <&dispcc MDSS_GDSC>;
> +
> + clocks = <&gcc GCC_DISP_AHB_CLK>,
> + <&gcc GCC_DISP_HF_AXI_CLK>,
> + <&dispcc DISP_CC_MDSS_AHB_CLK>,
> + <&dispcc DISP_CC_MDSS_MDP_CLK>;
> + clock-names = "iface", "bus", "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>;
> +
> + iommus = <&apps_smmu 0x800 0x2>;
> +
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> +
> + status = "disabled";
> +
> + mdp: mdp@...1000 {
> + compatible = "qcom,sc7180-dpu";
> + reg = <0 0x0ae01000 0 0x8f000>,
> + <0 0x0aeb0000 0 0x2008>;
> + reg-names = "mdp", "vbif";
> +
> + clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>,
> + <&dispcc DISP_CC_MDSS_ROT_CLK>,
> + <&dispcc DISP_CC_MDSS_MDP_LUT_CLK>,
> + <&dispcc DISP_CC_MDSS_MDP_CLK>,
> + <&dispcc DISP_CC_MDSS_VSYNC_CLK>;
> + clock-names = "iface", "rot", "lut", "core",
> + "vsync";
> + assigned-clocks = <&dispcc DISP_CC_MDSS_MDP_CLK>,
> + <&dispcc DISP_CC_MDSS_VSYNC_CLK>;
> + assigned-clock-rates = <300000000>,
> + <19200000>;
> +
> + interrupt-parent = <&mdss>;
> + interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
> +
> + status = "disabled";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> + dpu_intf1_out: endpoint {
> + remote-endpoint = <&dsi0_in>;
> + };
> + };
> + };
> + };
> +
> + dsi0: dsi@...4000 {
> + compatible = "qcom,mdss-dsi-ctrl";
> + reg = <0 0x0ae94000 0 0x400>;
> + reg-names = "dsi_ctrl";
> +
> + interrupt-parent = <&mdss>;
> + interrupts = <4 IRQ_TYPE_LEVEL_HIGH>;
> +
> + clocks = <&dispcc DISP_CC_MDSS_BYTE0_CLK>,
> + <&dispcc DISP_CC_MDSS_BYTE0_INTF_CLK>,
> + <&dispcc DISP_CC_MDSS_PCLK0_CLK>,
> + <&dispcc DISP_CC_MDSS_ESC0_CLK>,
> + <&dispcc DISP_CC_MDSS_AHB_CLK>,
> + <&gcc GCC_DISP_HF_AXI_CLK>;
> + clock-names = "byte",
> + "byte_intf",
> + "pixel",
> + "core",
> + "iface",
> + "bus";
> +
> + phys = <&dsi_phy>;
> + phy-names = "dsi";
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + status = "disabled";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> + dsi0_in: endpoint {
> + remote-endpoint = <&dpu_intf1_out>;
> + };
> + };
> +
> + port@1 {
> + reg = <1>;
> + dsi0_out: endpoint {
> + };
> + };
> + };
> + };
> +
> + dsi_phy: dsi-phy@...4400 {
> + compatible = "qcom,dsi-phy-10nm";
> + reg = <0 0x0ae94400 0 0x200>,
> + <0 0x0ae94600 0 0x280>,
> + <0 0x0ae94a00 0 0x1e0>;
> + reg-names = "dsi_phy",
> + "dsi_phy_lane",
> + "dsi_pll";
> +
> + #clock-cells = <1>;
> + #phy-cells = <0>;
> +
> + clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>,
> + <&rpmhcc RPMH_CXO_CLK>;
> + clock-names = "iface", "ref";
> +
> + status = "disabled";
> + };
> + };
> +
> dispcc: clock-controller@...0000 {
> compatible = "qcom,sc7180-dispcc";
> reg = <0 0x0af00000 0 0x200000>;
> clocks = <&rpmhcc RPMH_CXO_CLK>,
> <&gcc GCC_DISP_GPLL0_CLK_SRC>,
> - <0>,
> - <1>,
> + <&dsi_phy 0>,
> + <&dsi_phy 1>,
> <0>,
> <0>;
> clock-names = "xo", "gpll0",
Thanks for adding this bit in v5. What you end up with is good, but
I'm slightly confused by your baseline and that makes it hard for git
to automatically apply your patch. Specifically:
* I don't think I ever sent out a patch where "<1>" was a
bogus/placeholder phandle. Where did you get that from?
* On the newest version of my patch [1] the clock names were
"bi_tcxo", "gcc_disp_gpll0_clk_src", etc. Not "xo", "gpll0", ....
Presumably you're applying atop an older version?
NOTE: it's not actually that hard to resolve this manually, so unless
Bjorn / Andy requests it you probably don't need a v6. How I applied
it if it's helpful [2].
I see that you're working to fix the bindings [3]. Seems like that
still needs to be spun a bit more, but I think in general I'm
convinced that what you're got in the dts is OK for sc7180 while that
spins. It's not like the bindings were in amazing shape to start
with. Thus:
Reviewed-by: Douglas Anderson <dianders@...omium.org>
Tested-by: Douglas Anderson <dianders@...omium.org>
[1] https://lore.kernel.org/r/20200203103049.v4.15.I1a4b93fb005791e29a9dcf288fc8bd459a555a59@changeid
[2] https://crrev.com/c/2020394/4
[3] https://lore.kernel.org/r/1580825737-27189-1-git-send-email-harigovi@codeaurora.org
-Doug
Powered by blists - more mailing lists