Re: [PATCH v3] arm64: dts: qcom: sdm845-xiaomi-beryllium: Add DSI and panel bits

From: AngeloGioacchino Del Regno
Date: Fri Feb 12 2021 - 12:44:29 EST


Il 12/02/21 10:24, Amit Pundir ha scritto:
Hi,

On Thu, 11 Feb 2021 at 00:25, AngeloGioacchino Del Regno
<angelogioacchino.delregno@xxxxxxxxxxxxxx> wrote:

Il 10/02/21 09:18, Amit Pundir ha scritto:
From: Sumit Semwal <sumit.semwal@xxxxxxxxxx>

Enabling the Display panel for beryllium requires DSI
labibb regulators and panel dts nodes to be added.
It is also required to keep some of the regulators as
always-on.

Signed-off-by: Sumit Semwal <sumit.semwal@xxxxxxxxxx>
Signed-off-by: Amit Pundir <amit.pundir@xxxxxxxxxx>
---

Hello!
Your patch looks good, however, I have a few concerns...

v3: Addressed Konrad's concerns. Configured labibb regulators
explicitly based on downstream microvolt values. Display
comes up fine with default discharge-resistor-kohms and
soft-start-us properties, so didn't touch them.
Smoke tested on next-20210209.
v2: Rebased to mainline (v5.11-rc6) and fixed build warnings.

.../boot/dts/qcom/sdm845-xiaomi-beryllium.dts | 64 ++++++++++++++++++++++
1 file changed, 64 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sdm845-xiaomi-beryllium.dts b/arch/arm64/boot/dts/qcom/sdm845-xiaomi-beryllium.dts
index 86cbae63eaf7..5ac049a247e1 100644
--- a/arch/arm64/boot/dts/qcom/sdm845-xiaomi-beryllium.dts
+++ b/arch/arm64/boot/dts/qcom/sdm845-xiaomi-beryllium.dts
@@ -157,6 +157,14 @@
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
};

+ vreg_l14a_1p8: ldo14 {
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ regulator-boot-on;
+ regulator-always-on;
+ };
+
vreg_l17a_1p3: ldo17 {
regulator-min-microvolt = <1304000>;
regulator-max-microvolt = <1304000>;
@@ -191,6 +199,7 @@
regulator-min-microvolt = <1200000>;
regulator-max-microvolt = <1200000>;
regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ regulator-boot-on;
};
};
};
@@ -200,6 +209,43 @@
firmware-name = "qcom/sdm845/cdsp.mdt";
};

+&dsi0 {
+ status = "okay";
+ vdda-supply = <&vreg_l26a_1p2>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ panel@0 {
+ compatible = "tianma,fhd-video";
+ reg = <0>;
+ vddi0-supply = <&vreg_l14a_1p8>;
+ vddpos-supply = <&lab>;
+ vddneg-supply = <&ibb>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ reset-gpios = <&tlmm 6 GPIO_ACTIVE_LOW>;
+
+ port {
+ tianma_nt36672a_in_0: endpoint {
+ remote-endpoint = <&dsi0_out>;
+ };
+ };
+ };
+};
+
+&dsi0_out {
+ remote-endpoint = <&tianma_nt36672a_in_0>;
+ data-lanes = <0 1 2 3>;
+};
+
+&dsi0_phy {
+ status = "okay";
+ vdds-supply = <&vreg_l1a_0p875>;
+};
+
&gcc {
protected-clocks = <GCC_QSPI_CORE_CLK>,
<GCC_QSPI_CORE_CLK_SRC>,
@@ -215,6 +261,24 @@
};
};

+&ibb {
+ regulator-min-microvolt = <4600000>;
+ regulator-max-microvolt = <6000000>;
+};
+

I think you want to also configure overvoltage and overcurrent
protection values for both LAB and IBB, as these regulators may be a bit
dangerous if used without.

Can you point me to the relevant DT properties please. I didn't find
any DT properties which set the over voltage/current protection
properties explicitly in upstream as well as in downstream kernel.
Plus I also do not see "regulator-min/max-microamp" values set
downstream, otherwise I'd have used that as well atleast.


The regulator-{min/max}-microamp is the equivalent of the downstream
"qcom,qpnp-lab-limit-maximum-current" and "qcom,qpnp-ibb-limit-maximum-current.

I am sorry if we haven't sent our patch series that will introduce the
Sony MSM8998 Yoshino and SDM630/636 Nile and Ganges platforms, which
are actually using the driver that I've sent fully, as these would have
been a nice reference for you.

In any case, I can point you at our public repositories and their
downstream equivalents...

Here you find the downstream configuration for LAB/IBB:
https://github.com/sonyxperiadev/kernel/blob/aosp/LA.UM.7.1.r1/arch/arm64/boot/dts/qcom/msm8998-yoshino-common.dtsi#L3158

...And here you find the SoMainline upstream stuff for the same:
https://github.com/SoMainline/linux/blob/SoMainline/v5.11-rc4/arch/arm64/boot/dts/qcom/msm8998-sony-xperia-yoshino.dtsi#L332

https://github.com/SoMainline/linux/blob/SoMainline/v5.11-rc4/arch/arm64/boot/dts/qcom/msm8998-sony-xperia-yoshino-maple.dts#L50

I hope that clears out your doubts and difficulties about implementing
the protections on labibb.

Yours,
-- Angelo

Regards,
Amit Pundir

Besides that, even if it wouldn't be that dangerous, since the
protection features are present, it would be nice to configure them
properly as in the rare event that something bad happens, you would be
able to save the hardware (or at least have a chance to!).

+&lab {
+ regulator-min-microvolt = <4600000>;
+ regulator-max-microvolt = <6000000>;
+};
+

Same here.

Yours,
-- Angelo

+&mdss {
+ status = "okay";
+};
+
+&mdss_mdp {
+ status = "okay";
+};
+
&mss_pil {
status = "okay";
firmware-name = "qcom/sdm845/mba.mbn", "qcom/sdm845/modem.mdt";