Re: [PATCH v3] arm64: dts: qcom: sdm845: Add dpu to sdm845 dts file

Hi,

On Wed, Nov 28, 2018 at 3:44 PM Jordan Crouse [off-list ref] wrote:

On Thu, Nov 01, 2018 at 07:25:23PM -0700, Jeykumar Sankaran wrote:

quoted

DPU is short for the Display Processing Unit. It is the display
controller on Qualcomm SDM845 chips.

This change adds MDSS and DSI nodes to enable display on the
target device.

Changes in v2:
       - Beefed up commit message
       - Use SoC specific compatibles for mdss and dpu (Rob H)
       - Use assigned-clocks to set initial clock frequency(Rob H)
Changes in v3:
       - added IOMMU node
       - Fix device naming (remove _phys)
       - Use correct IRQ_TYPE in interrupt specifiers

Signed-off-by: Jeykumar Sankaran <redacted>
Signed-off-by: Sean Paul <redacted>
---
 arch/arm64/boot/dts/qcom/sdm845.dtsi | 191 +++++++++++++++++++++++++++++++++++
 1 file changed, 191 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index 0c9a2aa..dd612ac 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi

@@ -978,6 +978,197 @@
                      #thermal-sensor-cells = <1>;
              };

+             mdss: mdss@ae00000 {
+                     compatible = "qcom,sdm845-mdss";
+                     reg = <0xae00000 0x1000>;
+                     reg-names = "mdss";
+
+                     power-domains = <&dispcc 0>;

Would the powers-that-be in the power domain prefer this to have a symbolic
value? I see MDSS_GDSC in the bindings header.

You're one patch version behind.  Can you look at:

https://patchwork.kernel.org/patch/10666253/ AKA
https://lkml.kernel.org/r/1541197576-19730-2-git-send-email-jsanka@codeaurora.org

...that addresses the _clk issues and I commented about the #define too.

quoted

+                             phys = <&dsi0_phy>;
+                             phy-names = "dsi-phy";

This too, I reckon.

I didn't notice this one, so it's still "dsi-phy" in v4.

quoted

+                     dsi0_phy: dsi-phy@ae94400 {
+                             compatible = "qcom,dsi-phy-10nm";
+                             reg = <0xae94400 0x200>,
+                                   <0xae94a00 0x1e0>,
+                                   <0xae94600 0x280>;

I think it would be better if these were in numerical order.

Still broken in v4.


Also Bjorn's thought about adding a 'status = "disabled"' also makes sense.

So I guess it's time for a v5 Jeykumar.


-Doug
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno