2023-04-07 15:22:42

by Nikita Travkin

[permalink] [raw]
Subject: [PATCH v5 0/4] Add Acer Aspire 1

This series introduces Acer Aspire 1 - A WoA laptop with sc7180.

The dts adds mostly complite support for the hardware and the device,
with minor patches on top, can be used as a normal laptop daily.

Notable features absent from this patch:
- Sound
While the dedicated sound components are defined, since the
ADSP must be used, sound requires additions of that remoteproc
as well some extra "glue" to connect the i2s outputs to it.
I was able to hack together some sound based on sm8250 stuff
but it needs more work.
- Embedded Controller
The laptop has a dedicated EC that controls, notably,
battery/charger and notifies the device about the USB-C DisplayPort
HPD events. As per this patch, there is no battery status
indication and external display support. Also, due to the EC
defaults, the fn key is disabled. I have an experimental driver that
implements all of that, which needs more work and will be submitted
at a later date.
- PSCI OSI Mode
Firmware on this laptop does not support the PC mode, as is usual
for Qualcomm. This change would require adding OSI related
power-domains to the SoC dtsi and is omitted in expectation that
this can be handled when (if?) CrOS team handles their tf-a, like
they did with sc7280.

Changed in v3:
- Disable lpass clocks by default (Konrad)
- Drop status=disabled for mdp in the common soc dtsi (Konrad)

Changed in v4:
- Resend with picked up tags, no other change.

Changed in v5:
- Minor style issues fixed. (Konrad)

Nikita Travkin (4):
arm64: dts: qcom: sc7180: Don't enable lpass clocks by default
arm64: dts: qcom: sc7180: Drop redundant disable in mdp
dt-bindings: arm: qcom: Add Acer Aspire 1
arm64: dts: qcom: Add Acer Aspire 1

.../devicetree/bindings/arm/qcom.yaml | 4 +-
arch/arm64/boot/dts/qcom/Makefile | 1 +
.../boot/dts/qcom/sc7180-acer-aspire1.dts | 854 ++++++++++++++++++
arch/arm64/boot/dts/qcom/sc7180-idp.dts | 4 -
arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi | 6 +-
arch/arm64/boot/dts/qcom/sc7180.dtsi | 6 +-
6 files changed, 866 insertions(+), 9 deletions(-)
create mode 100644 arch/arm64/boot/dts/qcom/sc7180-acer-aspire1.dts

--
2.39.2


2023-04-07 15:23:11

by Nikita Travkin

[permalink] [raw]
Subject: [PATCH v5 1/4] arm64: dts: qcom: sc7180: Don't enable lpass clocks by default

lpass clocks are usually blocked from HLOS by the firmware and
instead are managed by the ADSP. Mark them as reserved and explicitly
enable in the CrOS boards that have special, cooperative firmware.

Signed-off-by: Nikita Travkin <[email protected]>
Reviewed-by: Konrad Dybcio <[email protected]>
---
v5: minor style changes (Konrad)
---
arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi | 8 ++++++++
arch/arm64/boot/dts/qcom/sc7180.dtsi | 4 ++++
2 files changed, 12 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
index 423630c4d02c..7e57899ef2c6 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
@@ -785,6 +785,10 @@ alc5682: codec@1a {
};
};

+&lpasscc {
+ status = "okay";
+};
+
&lpass_cpu {
status = "okay";

@@ -810,6 +814,10 @@ dai-link@5 {
};
};

+&lpass_hm {
+ status = "okay";
+};
+
&mdp {
status = "okay";
};
diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
index 3c799b564b64..6f40301faa1c 100644
--- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
@@ -3621,6 +3621,8 @@ lpasscc: clock-controller@62d00000 {
power-domains = <&lpass_hm LPASS_CORE_HM_GDSCR>;
#clock-cells = <1>;
#power-domain-cells = <1>;
+
+ status = "reserved"; /* Controlled by ADSP */
};

lpass_cpu: lpass@62d87000 {
@@ -3669,6 +3671,8 @@ lpass_hm: clock-controller@63000000 {

#clock-cells = <1>;
#power-domain-cells = <1>;
+
+ status = "reserved"; /* Controlled by ADSP */
};
};

--
2.39.2

2023-04-07 15:24:36

by Nikita Travkin

[permalink] [raw]
Subject: [PATCH v5 2/4] arm64: dts: qcom: sc7180: Drop redundant disable in mdp

mdss is useless without a display controller which makes explicitly
enabling mdp redundant. Have it enabled by default to drop the extra
node for all users.

Signed-off-by: Nikita Travkin <[email protected]>
Reviewed-by: Konrad Dybcio <[email protected]>
---
arch/arm64/boot/dts/qcom/sc7180-idp.dts | 4 ----
arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi | 4 ----
arch/arm64/boot/dts/qcom/sc7180.dtsi | 2 --
3 files changed, 10 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sc7180-idp.dts b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
index fcabbc6a897f..292aed241839 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-idp.dts
+++ b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
@@ -334,10 +334,6 @@ &dsi_phy {
vdds-supply = <&vreg_l4a_0p8>;
};

-&mdp {
- status = "okay";
-};
-
&mdss {
status = "okay";
};
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
index 7e57899ef2c6..1d2867cc0526 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
@@ -818,10 +818,6 @@ &lpass_hm {
status = "okay";
};

-&mdp {
- status = "okay";
-};
-
&mdss {
status = "okay";
};
diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
index 6f40301faa1c..13e4a5045fdc 100644
--- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
@@ -2983,8 +2983,6 @@ mdp: display-controller@ae01000 {
interrupt-parent = <&mdss>;
interrupts = <0>;

- status = "disabled";
-
ports {
#address-cells = <1>;
#size-cells = <0>;
--
2.39.2

2023-04-07 15:24:38

by Nikita Travkin

[permalink] [raw]
Subject: [PATCH v5 3/4] dt-bindings: arm: qcom: Add Acer Aspire 1

Acer Aspire 1 is a laptop based on sc7180. Document it's compatible.

Signed-off-by: Nikita Travkin <[email protected]>
Reviewed-by: Krzysztof Kozlowski <[email protected]>
---
Changes in v2:
- Merge with IDP (Krzysztof)
---
Documentation/devicetree/bindings/arm/qcom.yaml | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
index f8d29b65f28b..db97a61e8ccb 100644
--- a/Documentation/devicetree/bindings/arm/qcom.yaml
+++ b/Documentation/devicetree/bindings/arm/qcom.yaml
@@ -367,9 +367,9 @@ properties:
- qcom,qru1000-idp
- const: qcom,qru1000

- - description: Qualcomm Technologies, Inc. SC7180 IDP
- items:
+ - items:
- enum:
+ - acer,aspire1
- qcom,sc7180-idp
- const: qcom,sc7180

--
2.39.2

2023-04-07 15:25:56

by Nikita Travkin

[permalink] [raw]
Subject: [PATCH v5 4/4] arm64: dts: qcom: Add Acer Aspire 1

Acer Aspire 1 is a WoA laptop based on Snapdragon 7c gen1 platform.

The laptop design is similar to trogdor in the choice of primary
components but the specifics on usage of those differ slightly.

Add the devicetree for the laptop with support for most of the
hardware present.

Signed-off-by: Nikita Travkin <[email protected]>
Reviewed-by: Konrad Dybcio <[email protected]>
---
Changed in v2:
- Various styling, ordering and node naming issues fixed. (Krzysztof)

Changed in v3:
- Kepp camcc on, wakeup on touchpad, minor style issues. (Konrad)

Changed in v5:
- Minor style issues (Konrad)
---
arch/arm64/boot/dts/qcom/Makefile | 1 +
.../boot/dts/qcom/sc7180-acer-aspire1.dts | 854 ++++++++++++++++++
2 files changed, 855 insertions(+)
create mode 100644 arch/arm64/boot/dts/qcom/sc7180-acer-aspire1.dts

diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
index f6ff4024a60e..f0d92c47bc7f 100644
--- a/arch/arm64/boot/dts/qcom/Makefile
+++ b/arch/arm64/boot/dts/qcom/Makefile
@@ -78,6 +78,7 @@ dtb-$(CONFIG_ARCH_QCOM) += sa8155p-adp.dtb
dtb-$(CONFIG_ARCH_QCOM) += sa8295p-adp.dtb
dtb-$(CONFIG_ARCH_QCOM) += sa8540p-ride.dtb
dtb-$(CONFIG_ARCH_QCOM) += sa8775p-ride.dtb
+dtb-$(CONFIG_ARCH_QCOM) += sc7180-acer-aspire1.dtb
dtb-$(CONFIG_ARCH_QCOM) += sc7180-idp.dtb
dtb-$(CONFIG_ARCH_QCOM) += sc7180-trogdor-coachz-r1.dtb
dtb-$(CONFIG_ARCH_QCOM) += sc7180-trogdor-coachz-r1-lte.dtb
diff --git a/arch/arm64/boot/dts/qcom/sc7180-acer-aspire1.dts b/arch/arm64/boot/dts/qcom/sc7180-acer-aspire1.dts
new file mode 100644
index 000000000000..d0ec759a44ce
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/sc7180-acer-aspire1.dts
@@ -0,0 +1,854 @@
+// SPDX-License-Identifier: BSD-3-Clause
+
+/dts-v1/;
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/regulator/qcom,rpmh-regulator.h>
+
+#include "sc7180.dtsi"
+
+#include "pm6150.dtsi"
+#include "pm6150l.dtsi"
+
+/delete-node/ &tz_mem;
+/delete-node/ &ipa_fw_mem;
+
+/ {
+ model = "Acer Aspire 1";
+ compatible = "acer,aspire1", "qcom,sc7180";
+ chassis-type = "laptop";
+
+ aliases {
+ bluetooth0 = &bluetooth;
+ hsuart0 = &uart3;
+ serial0 = &uart8;
+ wifi0 = &wifi;
+ };
+
+ chosen {
+ stdout-path = "serial0:115200n8";
+ };
+
+ reserved-memory {
+ zap_mem: zap-shader@80840000 {
+ reg = <0x0 0x80840000 0 0x2000>;
+ no-map;
+ };
+
+ venus_mem: venus@85b00000 {
+ reg = <0x0 0x85b00000 0 0x500000>;
+ no-map;
+ };
+
+ mpss_mem: mpss@86000000 {
+ reg = <0x0 0x86000000 0x0 0x2000000>;
+ no-map;
+ };
+
+ adsp_mem: adsp@8e400000 {
+ reg = <0x0 0x8e400000 0x0 0x2800000>;
+ no-map;
+ };
+
+ wlan_mem: wlan@93900000 {
+ reg = <0x0 0x93900000 0x0 0x200000>;
+ no-map;
+ };
+ };
+
+ max98357a: audio-codec {
+ compatible = "maxim,max98357a";
+ sdmode-gpios = <&tlmm 23 GPIO_ACTIVE_HIGH>;
+
+ pinctrl-0 = <&amp_sd_mode_default>;
+ pinctrl-names = "default";
+
+ #sound-dai-cells = <0>;
+ };
+
+ backlight: backlight {
+ compatible = "pwm-backlight";
+ pwms = <&sn65dsi86_bridge 1000000>;
+ enable-gpios = <&tlmm 10 GPIO_ACTIVE_HIGH>;
+
+ pinctrl-0 = <&soc_bkoff_default>;
+ pinctrl-names = "default";
+ };
+
+ reg_brij_1p2: bridge-1p2-regulator {
+ compatible = "regulator-fixed";
+ regulator-name = "brij_1p2";
+ regulator-min-microvolt = <1200000>;
+ regulator-max-microvolt = <1200000>;
+
+ gpio = <&tlmm 19 GPIO_ACTIVE_HIGH>;
+ enable-active-high;
+
+ pinctrl-0 = <&reg_edp_1p2_en_default>;
+ pinctrl-names = "default";
+ };
+
+ reg_brij_1p8: bridge-regulator {
+ compatible = "regulator-fixed";
+ regulator-name = "brij_1p8";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+
+ vin-supply = <&vreg_l8c_1p8>;
+
+ gpio = <&tlmm 20 GPIO_ACTIVE_HIGH>;
+ enable-active-high;
+
+ pinctrl-0 = <&reg_edp_1p8_en_default>;
+ pinctrl-names = "default";
+ };
+
+ reg_codec_3p3: codec-regulator {
+ compatible = "regulator-fixed";
+ regulator-name = "codec_3p3";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+
+ gpio = <&tlmm 83 GPIO_ACTIVE_HIGH>;
+ enable-active-high;
+
+ pinctrl-0 = <&reg_audio_en_default>;
+ pinctrl-names = "default";
+ };
+
+ reg_lcm_3p3: panel-regulator {
+ compatible = "regulator-fixed";
+ regulator-name = "lcm_3p3";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+
+ /*
+ * HACK: Display fails with
+ *
+ * *ERROR* Unexpected max rate (0x0); assuming 5.4 GHz
+ * *ERROR* Link training failed, link is off (-5)
+ *
+ * if the power to the panel was ever cut
+ */
+ regulator-always-on;
+
+ gpio = <&tlmm 26 GPIO_ACTIVE_HIGH>;
+ enable-active-high;
+
+ pinctrl-0 = <&reg_lcm_en_default>;
+ pinctrl-names = "default";
+ };
+
+ reg_tp_3p3: touchpad-regulator {
+ compatible = "regulator-fixed";
+ regulator-name = "tp_3p3";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+
+ gpio = <&tlmm 25 GPIO_ACTIVE_HIGH>;
+ enable-active-high;
+
+ pinctrl-0 = <&reg_tp_en_default>;
+ pinctrl-names = "default";
+ };
+};
+
+&dsi0 {
+ vdda-supply = <&vreg_l3c_1p2>;
+ status = "okay";
+};
+
+&dsi0_out {
+ remote-endpoint = <&sn65dsi86_in>;
+ data-lanes = <0 1 2 3>;
+};
+
+&dsi_phy {
+ vdds-supply = <&vreg_l4a_0p8>;
+ status = "okay";
+};
+
+&i2c2 {
+ clock-frequency = <400000>;
+ status = "okay";
+
+ /* embedded-controller@76 */
+};
+
+&i2c4 {
+ clock-frequency = <400000>;
+ status = "okay";
+
+ /*
+ * NOTE: DSDT defines two possible touchpads, other one is
+ *
+ * reg = <0x15>;
+ * hid-descr-addr = <0x1>;
+ */
+
+ touchpad@2c {
+ compatible = "hid-over-i2c";
+ reg = <0x2c>;
+ hid-descr-addr = <0x20>;
+
+ vdd-supply = <&reg_tp_3p3>;
+
+ interrupts-extended = <&tlmm 94 IRQ_TYPE_LEVEL_LOW>;
+
+ pinctrl-0 = <&hid_touchpad_default>;
+ pinctrl-names = "default";
+
+ wakeup-source;
+ };
+
+ keyboard@3a {
+ compatible = "hid-over-i2c";
+ reg = <0x3a>;
+ hid-descr-addr = <0x1>;
+
+ interrupts-extended = <&tlmm 33 IRQ_TYPE_LEVEL_LOW>;
+
+ pinctrl-0 = <&hid_keyboard_default>;
+ pinctrl-names = "default";
+
+ wakeup-source;
+ };
+};
+
+&i2c9 {
+ clock-frequency = <400000>;
+ status = "okay";
+
+ alc5682: codec@1a {
+ compatible = "realtek,rt5682i";
+ reg = <0x1a>;
+
+ #sound-dai-cells = <1>;
+
+ interrupt-parent = <&tlmm>;
+ interrupts = <28 IRQ_TYPE_EDGE_BOTH>;
+
+ pinctrl-0 = <&codec_irq_default>;
+ pinctrl-names = "default";
+
+ AVDD-supply = <&vreg_l15a_1p8>;
+ MICVDD-supply = <&reg_codec_3p3>;
+ VBAT-supply = <&reg_codec_3p3>;
+
+ realtek,dmic1-data-pin = <1>;
+ realtek,dmic1-clk-pin = <1>;
+ realtek,jd-src = <1>;
+ };
+};
+
+&i2c10 {
+ clock-frequency = <400000>;
+ status = "okay";
+
+ sn65dsi86_bridge: bridge@2c {
+ compatible = "ti,sn65dsi86";
+ reg = <0x2c>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ #pwm-cells = <1>;
+
+ interrupt-parent = <&tlmm>;
+ interrupts = <11 IRQ_TYPE_LEVEL_HIGH>;
+
+ enable-gpios = <&tlmm 51 GPIO_ACTIVE_HIGH>;
+ suspend-gpios = <&tlmm 22 GPIO_ACTIVE_LOW>;
+
+ pinctrl-0 = <&bridge_en_default>,
+ <&edp_bridge_irq_default>,
+ <&bridge_suspend_default>;
+ pinctrl-names = "default";
+
+ vpll-supply = <&reg_brij_1p8>;
+ vccio-supply = <&reg_brij_1p8>;
+ vcca-supply = <&reg_brij_1p2>;
+ vcc-supply = <&reg_brij_1p2>;
+
+ clocks = <&rpmhcc RPMH_LN_BB_CLK3>;
+ clock-names = "refclk";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+
+ sn65dsi86_in: endpoint {
+ remote-endpoint = <&dsi0_out>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+
+ sn65dsi86_out: endpoint {
+ data-lanes = <0 1>;
+ remote-endpoint = <&panel_in_edp>;
+ };
+ };
+ };
+
+ aux-bus {
+ panel: panel {
+ compatible = "edp-panel";
+ power-supply = <&reg_lcm_3p3>;
+ backlight = <&backlight>;
+
+ port {
+ panel_in_edp: endpoint {
+ remote-endpoint = <&sn65dsi86_out>;
+ };
+ };
+ };
+ };
+ };
+};
+
+&gpu {
+ status = "okay";
+
+ zap-shader {
+ memory-region = <&zap_mem>;
+ firmware-name = "qcom/sc7180/acer/aspire1/qcdxkmsuc7180.mbn";
+ };
+};
+
+&mdss {
+ status = "okay";
+};
+
+&pm6150_adc {
+ thermistor@4e {
+ reg = <ADC5_AMUX_THM2_100K_PU>;
+ qcom,ratiometric;
+ qcom,hw-settle-time = <200>;
+ };
+
+ charger-thermistor@4f {
+ reg = <ADC5_AMUX_THM3_100K_PU>;
+ qcom,ratiometric;
+ qcom,hw-settle-time = <200>;
+ };
+};
+
+&pm6150_adc_tm {
+ status = "okay";
+
+ charger-thermistor@0 {
+ reg = <0>;
+ io-channels = <&pm6150_adc ADC5_AMUX_THM3_100K_PU>;
+ qcom,ratiometric;
+ qcom,hw-settle-time-us = <200>;
+ };
+
+ thermistor@1 {
+ reg = <1>;
+ io-channels = <&pm6150_adc ADC5_AMUX_THM2_100K_PU>;
+ qcom,ratiometric;
+ qcom,hw-settle-time-us = <200>;
+ };
+};
+
+&pm6150_pon {
+ status = "disabled";
+};
+
+&qupv3_id_0 {
+ status = "okay";
+};
+
+&qupv3_id_1 {
+ status = "okay";
+};
+
+&remoteproc_mpss {
+ firmware-name = "qcom/sc7180/acer/aspire1/qcmpss7180_nm.mbn";
+ status = "okay";
+};
+
+&sdhc_1 {
+ pinctrl-0 = <&sdc1_default>;
+ pinctrl-1 = <&sdc1_sleep>;
+ pinctrl-names = "default", "sleep";
+ vmmc-supply = <&vreg_l19a_2p9>;
+ vqmmc-supply = <&vreg_l12a_1p8>;
+
+ status = "okay";
+};
+
+&uart3 {
+ /delete-property/interrupts;
+ interrupts-extended = <&intc GIC_SPI 604 IRQ_TYPE_LEVEL_HIGH>,
+ <&tlmm 41 IRQ_TYPE_EDGE_FALLING>;
+
+ pinctrl-1 = <&qup_uart3_sleep>;
+ pinctrl-names = "default", "sleep";
+
+ status = "okay";
+
+ bluetooth: bluetooth {
+ compatible = "qcom,wcn3991-bt";
+ vddio-supply = <&vreg_l10a_1p8>;
+ vddxo-supply = <&vreg_l1c_1p8>;
+ vddrf-supply = <&vreg_l2c_1p3>;
+ vddch0-supply = <&vreg_l10c_3p3>;
+ max-speed = <3200000>;
+ };
+};
+
+&uart8 {
+ status = "okay";
+};
+
+&usb_1 {
+ status = "okay";
+};
+
+&usb_1_dwc3 {
+ dr_mode = "host";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ usb_hub_2_x: hub@1 {
+ compatible = "usbbda,5411";
+ reg = <1>;
+ peer-hub = <&usb_hub_3_x>;
+ };
+
+ usb_hub_3_x: hub@2 {
+ compatible = "usbbda,411";
+ reg = <2>;
+ peer-hub = <&usb_hub_2_x>;
+ };
+};
+
+&usb_1_hsphy {
+ vdd-supply = <&vreg_l4a_0p8>;
+ vdda-pll-supply = <&vreg_l11a_1p8>;
+ vdda-phy-dpdm-supply = <&vreg_l17a_3p0>;
+ qcom,imp-res-offset-value = <8>;
+ qcom,preemphasis-level = <QUSB2_V2_PREEMPHASIS_15_PERCENT>;
+ qcom,preemphasis-width = <QUSB2_V2_PREEMPHASIS_WIDTH_HALF_BIT>;
+ qcom,bias-ctrl-value = <0x22>;
+ qcom,charge-ctrl-value = <3>;
+ qcom,hsdisc-trim-value = <0>;
+
+ status = "okay";
+};
+
+&usb_1_qmpphy {
+ vdda-phy-supply = <&vreg_l3c_1p2>;
+ vdda-pll-supply = <&vreg_l4a_0p8>;
+
+ status = "okay";
+};
+
+&venus {
+ firmware-name = "qcom/sc7180/acer/aspire1/qcvss7180.mbn";
+};
+
+&wifi {
+ vdd-0.8-cx-mx-supply = <&vreg_l9a_0p6>;
+ vdd-1.8-xo-supply = <&vreg_l1c_1p8>;
+ vdd-1.3-rfa-supply = <&vreg_l2c_1p3>;
+ vdd-3.3-ch0-supply = <&vreg_l10c_3p3>;
+ vdd-3.3-ch1-supply = <&vreg_l11c_3p3>;
+
+ status = "okay";
+};
+
+&apps_rsc {
+ regulators-0 {
+ compatible = "qcom,pm6150-rpmh-regulators";
+ qcom,pmic-id = "a";
+
+ vreg_s1a_1p1: smps1 {
+ regulator-min-microvolt = <1128000>;
+ regulator-max-microvolt = <1128000>;
+ };
+
+ vreg_l4a_0p8: ldo4 {
+ regulator-min-microvolt = <824000>;
+ regulator-max-microvolt = <928000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ vreg_l9a_0p6: ldo9 {
+ regulator-min-microvolt = <488000>;
+ regulator-max-microvolt = <800000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ vreg_l10a_1p8: ldo10 {
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ regulator-always-on;
+ regulator-boot-on;
+ };
+
+ vreg_l11a_1p8: ldo11 {
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ vreg_l12a_1p8: ldo12 {
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ vreg_l13a_1p8: ldo13 {
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ 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>;
+ };
+
+ vreg_l15a_1p8: ldo15 {
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ vreg_l16a_2p7: ldo16 {
+ regulator-min-microvolt = <2496000>;
+ regulator-max-microvolt = <3304000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ vreg_l17a_3p0: ldo17 {
+ regulator-min-microvolt = <2920000>;
+ regulator-max-microvolt = <3232000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ vreg_l18a_2p8: ldo18 {
+ regulator-min-microvolt = <2496000>;
+ regulator-max-microvolt = <3304000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ vreg_l19a_2p9: ldo19 {
+ regulator-min-microvolt = <2960000>;
+ regulator-max-microvolt = <2960000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+ };
+
+ regulators-1 {
+ compatible = "qcom,pm6150l-rpmh-regulators";
+ qcom,pmic-id = "c";
+
+ vreg_s8c_1p3: smps8 {
+ regulator-min-microvolt = <1120000>;
+ regulator-max-microvolt = <1408000>;
+ };
+
+ vreg_l1c_1p8: ldo1 {
+ regulator-min-microvolt = <1616000>;
+ regulator-max-microvolt = <1984000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ vreg_l2c_1p3: ldo2 {
+ regulator-min-microvolt = <1168000>;
+ regulator-max-microvolt = <1304000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ vreg_l3c_1p2: ldo3 {
+ regulator-min-microvolt = <1144000>;
+ regulator-max-microvolt = <1304000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ vreg_l4c_1p8: ldo4 {
+ regulator-min-microvolt = <1648000>;
+ regulator-max-microvolt = <3304000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_LPM>;
+ };
+
+ vreg_l5c_1p8: ldo5 {
+ regulator-min-microvolt = <1648000>;
+ regulator-max-microvolt = <3304000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_LPM>;
+ };
+
+ vreg_l6c_2p9: ldo6 {
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <2950000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ vreg_l7c_3p0: ldo7 {
+ regulator-min-microvolt = <3000000>;
+ regulator-max-microvolt = <3312000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_LPM>;
+ };
+
+ vreg_l8c_1p8: ldo8 {
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ vreg_l9c_2p9: ldo9 {
+ regulator-min-microvolt = <2952000>;
+ regulator-max-microvolt = <2952000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ vreg_l10c_3p3: ldo10 {
+ regulator-min-microvolt = <3000000>;
+ regulator-max-microvolt = <3400000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ vreg_l11c_3p3: ldo11 {
+ regulator-min-microvolt = <3000000>;
+ regulator-max-microvolt = <3400000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ vreg_bob: bob {
+ regulator-min-microvolt = <3008000>;
+ regulator-max-microvolt = <3960000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_AUTO>;
+ };
+ };
+};
+
+&qup_i2c2_default {
+ drive-strength = <2>;
+
+ /* Has external pullup */
+ bias-disable;
+};
+
+&qup_i2c4_default {
+ drive-strength = <2>;
+
+ /* Has external pullup */
+ bias-disable;
+};
+
+&qup_i2c9_default {
+ drive-strength = <2>;
+
+ /* Has external pullup */
+ bias-disable;
+};
+
+&qup_i2c10_default {
+ drive-strength = <2>;
+
+ /* Has external pullup */
+ bias-disable;
+};
+
+&tlmm {
+ /*
+ * The TZ seem to protect those because some boards can have
+ * fingerprint sensor connected to this range. Not connected
+ * on this board
+ */
+ gpio-reserved-ranges = <58 5>;
+
+ amp_sd_mode_default: amp-sd-mode-deault-state {
+ pins = "gpio23";
+ function = "gpio";
+ drive-strength = <16>;
+ bias-disable;
+ };
+
+ bridge_en_default: bridge-en-default-state {
+ pins = "gpio51";
+ function = "gpio";
+ drive-strength = <16>;
+ bias-disable;
+ };
+
+ bridge_suspend_default: bridge-suspend-default-state {
+ pins = "gpio22";
+ function = "gpio";
+ drive-strength = <16>;
+ bias-pull-up;
+ };
+
+ codec_irq_default: codec-irq-deault-state {
+ pins = "gpio28";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-disable;
+ };
+
+ edp_bridge_irq_default: edp-bridge-irq-default-state {
+ pins = "gpio11";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-pull-down;
+ };
+
+ hid_keyboard_default: hid-keyboard-default-state {
+ pins = "gpio33";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-disable;
+ };
+
+ hid_touchpad_default: hid-touchpad-default-state {
+ pins = "gpio94";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-disable;
+ };
+
+ qup_uart3_sleep: qup-uart3-sleep-state {
+ cts-pins {
+ /*
+ * Configure a pull-down on CTS to match the pull of
+ * the Bluetooth module.
+ */
+ pins = "gpio38";
+ function = "gpio";
+ bias-pull-down;
+ };
+
+ rts-pins {
+ /*
+ * Configure pull-down on RTS. As RTS is active low
+ * signal, pull it low to indicate the BT SoC that it
+ * can wakeup the system anytime from suspend state by
+ * pulling RX low (by sending wakeup bytes).
+ */
+ pins = "gpio39";
+ function = "gpio";
+ bias-pull-down;
+ };
+
+ tx-pins {
+ /*
+ * Configure pull-up on TX when it isn't actively driven
+ * to prevent BT SoC from receiving garbage during sleep.
+ */
+ pins = "gpio40";
+ function = "gpio";
+ bias-pull-up;
+ };
+
+ rx-pins {
+ /*
+ * Configure a pull-up on RX. This is needed to avoid
+ * garbage data when the TX pin of the Bluetooth module
+ * is floating which may cause spurious wakeups.
+ */
+ pins = "gpio41";
+ function = "gpio";
+ bias-pull-up;
+ };
+ };
+
+ reg_edp_1p2_en_default: reg-edp-1p2-en-deault-state {
+ pins = "gpio19";
+ function = "gpio";
+ drive-strength = <16>;
+ bias-disable;
+ };
+
+ reg_edp_1p8_en_default: reg-edp-1p8-en-deault-state {
+ pins = "gpio20";
+ function = "gpio";
+ drive-strength = <16>;
+ bias-disable;
+ };
+
+ reg_lcm_en_default: reg-lcm-en-deault-state {
+ pins = "gpio26";
+ function = "gpio";
+ drive-strength = <16>;
+ bias-disable;
+ };
+
+ reg_audio_en_default: reg-audio-en-deault-state {
+ pins = "gpio83";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-disable;
+ };
+
+ reg_tp_en_default: reg-tp-en-deault-state {
+ pins = "gpio25";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-disable;
+ };
+
+ soc_bkoff_default: soc-bkoff-deault-state {
+ pins = "gpio10";
+ function = "gpio";
+ drive-strength = <16>;
+ bias-disable;
+ };
+
+ sdc1_default: sdc1-default-state {
+ clk-pins {
+ pins = "sdc1_clk";
+ drive-strength = <16>;
+ bias-disable;
+ };
+
+ cmd-pins {
+ pins = "sdc1_cmd";
+ drive-strength = <16>;
+ bias-pull-up;
+ };
+
+ data-pins {
+ pins = "sdc1_data";
+ drive-strength = <16>;
+ bias-pull-up;
+ };
+
+ rclk-pins {
+ pins = "sdc1_rclk";
+ bias-pull-down;
+ };
+ };
+
+ sdc1_sleep: sdc1-sleep-state {
+ clk-pins {
+ pins = "sdc1_clk";
+ drive-strength = <2>;
+ bias-disable;
+ };
+
+ cmd-pins {
+ pins = "sdc1_cmd";
+ drive-strength = <2>;
+ bias-pull-up;
+ };
+
+ data-pins {
+ pins = "sdc1_data";
+ drive-strength = <2>;
+ bias-pull-up;
+ };
+
+ rclk-pins {
+ pins = "sdc1_rclk";
+ bias-pull-down;
+ };
+ };
+};
--
2.39.2

2023-04-07 16:25:28

by Doug Anderson

[permalink] [raw]
Subject: Re: [PATCH v5 1/4] arm64: dts: qcom: sc7180: Don't enable lpass clocks by default

Hi,

On Fri, Apr 7, 2023 at 8:14 AM Nikita Travkin <[email protected]> wrote:
>
> lpass clocks are usually blocked from HLOS by the firmware and
> instead are managed by the ADSP. Mark them as reserved and explicitly
> enable in the CrOS boards that have special, cooperative firmware.
>
> Signed-off-by: Nikita Travkin <[email protected]>
> Reviewed-by: Konrad Dybcio <[email protected]>
> ---
> v5: minor style changes (Konrad)
> ---
> arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi | 8 ++++++++
> arch/arm64/boot/dts/qcom/sc7180.dtsi | 4 ++++
> 2 files changed, 12 insertions(+)

I probably would have put a note in the commit message about why you
chose not to enable these for IDP (AKA summarize the results of the
conversation [1] in your v3 post). IMO not worth spinning a v6 just
for this, though.

Reviewed-by: Douglas Anderson <[email protected]>

[1] https://lore.kernel.org/r/[email protected]

2023-04-07 16:25:55

by Doug Anderson

[permalink] [raw]
Subject: Re: [PATCH v5 2/4] arm64: dts: qcom: sc7180: Drop redundant disable in mdp

Hi,

On Fri, Apr 7, 2023 at 8:14 AM Nikita Travkin <[email protected]> wrote:
>
> mdss is useless without a display controller which makes explicitly
> enabling mdp redundant. Have it enabled by default to drop the extra
> node for all users.
>
> Signed-off-by: Nikita Travkin <[email protected]>
> Reviewed-by: Konrad Dybcio <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/sc7180-idp.dts | 4 ----
> arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi | 4 ----
> arch/arm64/boot/dts/qcom/sc7180.dtsi | 2 --
> 3 files changed, 10 deletions(-)

Makes sense to me. If you were feeling particularly proactive, you
could also fix the same issue on sc7280.dtsi

Reviewed-by: Douglas Anderson <[email protected]>

2023-04-07 16:26:09

by Doug Anderson

[permalink] [raw]
Subject: Re: [PATCH v5 4/4] arm64: dts: qcom: Add Acer Aspire 1

Hi,

I didn't do too thorough of a review, but I noticed your comment about
the panel power and took a look...

On Fri, Apr 7, 2023 at 8:14 AM Nikita Travkin <[email protected]> wrote:
>
> + reg_lcm_3p3: panel-regulator {
> + compatible = "regulator-fixed";
> + regulator-name = "lcm_3p3";
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> +
> + /*
> + * HACK: Display fails with
> + *
> + * *ERROR* Unexpected max rate (0x0); assuming 5.4 GHz
> + * *ERROR* Link training failed, link is off (-5)
> + *
> + * if the power to the panel was ever cut
> + */
> + regulator-always-on;

I'm curious if `off-on-delay-us = <500000>;` would help you avoid the
hack. The eDP driver should already enforce stuff like this but I
think in some esoteric -EPROBE_DEFER cases it can end up violating
things. Any chance that's what you hit?

Oh, or maybe it's HPD. See below. Even if it's HPD, having an
'off-on-delay-us' specified here isn't a bad idea.

> +&i2c10 {
> + clock-frequency = <400000>;
> + status = "okay";
> +
> + sn65dsi86_bridge: bridge@2c {
> + compatible = "ti,sn65dsi86";
> + reg = <0x2c>;
> + gpio-controller;
> + #gpio-cells = <2>;
> + #pwm-cells = <1>;
> +
> + interrupt-parent = <&tlmm>;
> + interrupts = <11 IRQ_TYPE_LEVEL_HIGH>;
> +
> + enable-gpios = <&tlmm 51 GPIO_ACTIVE_HIGH>;
> + suspend-gpios = <&tlmm 22 GPIO_ACTIVE_LOW>;
> +
> + pinctrl-0 = <&bridge_en_default>,
> + <&edp_bridge_irq_default>,
> + <&bridge_suspend_default>;
> + pinctrl-names = "default";
> +
> + vpll-supply = <&reg_brij_1p8>;
> + vccio-supply = <&reg_brij_1p8>;
> + vcca-supply = <&reg_brij_1p2>;
> + vcc-supply = <&reg_brij_1p2>;
> +
> + clocks = <&rpmhcc RPMH_LN_BB_CLK3>;
> + clock-names = "refclk";

You want "no-hpd;" here somewhere. See below.


> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> +
> + sn65dsi86_in: endpoint {
> + remote-endpoint = <&dsi0_out>;
> + };
> + };
> +
> + port@1 {
> + reg = <1>;
> +
> + sn65dsi86_out: endpoint {
> + data-lanes = <0 1>;
> + remote-endpoint = <&panel_in_edp>;
> + };
> + };
> + };
> +
> + aux-bus {
> + panel: panel {
> + compatible = "edp-panel";
> + power-supply = <&reg_lcm_3p3>;
> + backlight = <&backlight>;

I think you want:

no-hpd;
hpd-absent-delay-ms = <200>;

...and yes, you end up with "no-hpd" in both the panel node and the
ti-sn65dsi86 node. See sdm845-cheza.

HPD might very well be hooked up on your board, but the current Linux
ti-sn65dsi86 driver does not look at its own HPD line because it's
actually slower than just pretending that HPD isn't there. On trogdor
boards we ended up routing HPD to a GPIO.

I guess your other option would be to implement HPD support in
ti-sn65dsi86. That would probably be an overall slower boot for you,
but is technically more correct. In the past people have posted up
patches to get ti-sn65dsi86 working as a full DP port and they added
HPD support for that, but none of those patch series ever go to the
point of landing...

2023-04-07 16:26:42

by Doug Anderson

[permalink] [raw]
Subject: Re: [PATCH v5 3/4] dt-bindings: arm: qcom: Add Acer Aspire 1

Hi,

On Fri, Apr 7, 2023 at 8:14 AM Nikita Travkin <[email protected]> wrote:
>
> Acer Aspire 1 is a laptop based on sc7180. Document it's compatible.
>
> Signed-off-by: Nikita Travkin <[email protected]>
> Reviewed-by: Krzysztof Kozlowski <[email protected]>
> ---
> Changes in v2:
> - Merge with IDP (Krzysztof)
> ---
> Documentation/devicetree/bindings/arm/qcom.yaml | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
> index f8d29b65f28b..db97a61e8ccb 100644
> --- a/Documentation/devicetree/bindings/arm/qcom.yaml
> +++ b/Documentation/devicetree/bindings/arm/qcom.yaml
> @@ -367,9 +367,9 @@ properties:
> - qcom,qru1000-idp
> - const: qcom,qru1000
>
> - - description: Qualcomm Technologies, Inc. SC7180 IDP
> - items:
> + - items:
> - enum:
> + - acer,aspire1
> - qcom,sc7180-idp
> - const: qcom,sc7180

If Krzysztof is happy then I have no real objections here. That being
said, I personally would have updated the description to be more
generic and not say "IDP" anymore. Something like "Non-Chromebook
sc7180 boards".


-Doug

2023-04-07 16:52:49

by Nikita Travkin

[permalink] [raw]
Subject: Re: [PATCH v5 4/4] arm64: dts: qcom: Add Acer Aspire 1

Doug Anderson писал(а) 07.04.2023 21:23:
> Hi,
>
> I didn't do too thorough of a review, but I noticed your comment about
> the panel power and took a look...
>
> On Fri, Apr 7, 2023 at 8:14 AM Nikita Travkin <[email protected]> wrote:
>>
>> + reg_lcm_3p3: panel-regulator {
>> + compatible = "regulator-fixed";
>> + regulator-name = "lcm_3p3";
>> + regulator-min-microvolt = <3300000>;
>> + regulator-max-microvolt = <3300000>;
>> +
>> + /*
>> + * HACK: Display fails with
>> + *
>> + * *ERROR* Unexpected max rate (0x0); assuming 5.4 GHz
>> + * *ERROR* Link training failed, link is off (-5)
>> + *
>> + * if the power to the panel was ever cut
>> + */
>> + regulator-always-on;
>
> I'm curious if `off-on-delay-us = <500000>;` would help you avoid the
> hack. The eDP driver should already enforce stuff like this but I
> think in some esoteric -EPROBE_DEFER cases it can end up violating
> things. Any chance that's what you hit?
>
> Oh, or maybe it's HPD. See below. Even if it's HPD, having an
> 'off-on-delay-us' specified here isn't a bad idea.
>
>> +&i2c10 {
>> + clock-frequency = <400000>;
>> + status = "okay";
>> +
>> + sn65dsi86_bridge: bridge@2c {
>> + compatible = "ti,sn65dsi86";
>> + reg = <0x2c>;
>> + gpio-controller;
>> + #gpio-cells = <2>;
>> + #pwm-cells = <1>;
>> +
>> + interrupt-parent = <&tlmm>;
>> + interrupts = <11 IRQ_TYPE_LEVEL_HIGH>;
>> +
>> + enable-gpios = <&tlmm 51 GPIO_ACTIVE_HIGH>;
>> + suspend-gpios = <&tlmm 22 GPIO_ACTIVE_LOW>;
>> +
>> + pinctrl-0 = <&bridge_en_default>,
>> + <&edp_bridge_irq_default>,
>> + <&bridge_suspend_default>;
>> + pinctrl-names = "default";
>> +
>> + vpll-supply = <&reg_brij_1p8>;
>> + vccio-supply = <&reg_brij_1p8>;
>> + vcca-supply = <&reg_brij_1p2>;
>> + vcc-supply = <&reg_brij_1p2>;
>> +
>> + clocks = <&rpmhcc RPMH_LN_BB_CLK3>;
>> + clock-names = "refclk";
>
> You want "no-hpd;" here somewhere. See below.
>
>
>> +
>> + ports {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + port@0 {
>> + reg = <0>;
>> +
>> + sn65dsi86_in: endpoint {
>> + remote-endpoint = <&dsi0_out>;
>> + };
>> + };
>> +
>> + port@1 {
>> + reg = <1>;
>> +
>> + sn65dsi86_out: endpoint {
>> + data-lanes = <0 1>;
>> + remote-endpoint = <&panel_in_edp>;
>> + };
>> + };
>> + };
>> +
>> + aux-bus {
>> + panel: panel {
>> + compatible = "edp-panel";
>> + power-supply = <&reg_lcm_3p3>;
>> + backlight = <&backlight>;
>
> I think you want:
>
> no-hpd;
> hpd-absent-delay-ms = <200>;
>
> ...and yes, you end up with "no-hpd" in both the panel node and the
> ti-sn65dsi86 node. See sdm845-cheza.
>
> HPD might very well be hooked up on your board, but the current Linux
> ti-sn65dsi86 driver does not look at its own HPD line because it's
> actually slower than just pretending that HPD isn't there. On trogdor
> boards we ended up routing HPD to a GPIO.
>

Oh, this makes so much sense then! The line is hooked up on
the board indeed and I remember being confused why trogdor boards
don't use it.

I will try to add the suggestions (annotating the reason)
and verify that it works, would prefer the panel power to be
gated when possible. I hope this would also fix the initial
EDID reading issues I occasionally have and carry a hack for
as of now...

Thank you a lot for this insight!

Nikita

> I guess your other option would be to implement HPD support in
> ti-sn65dsi86. That would probably be an overall slower boot for you,
> but is technically more correct. In the past people have posted up
> patches to get ti-sn65dsi86 working as a full DP port and they added
> HPD support for that, but none of those patch series ever go to the
> point of landing...

2023-04-07 17:13:11

by Doug Anderson

[permalink] [raw]
Subject: Re: [PATCH v5 4/4] arm64: dts: qcom: Add Acer Aspire 1

Hi,

On Fri, Apr 7, 2023 at 9:46 AM Nikita Travkin <[email protected]> wrote:
>
> > HPD might very well be hooked up on your board, but the current Linux
> > ti-sn65dsi86 driver does not look at its own HPD line because it's
> > actually slower than just pretending that HPD isn't there. On trogdor
> > boards we ended up routing HPD to a GPIO.
> >
>
> Oh, this makes so much sense then! The line is hooked up on
> the board indeed and I remember being confused why trogdor boards
> don't use it.
>
> I will try to add the suggestions (annotating the reason)
> and verify that it works, would prefer the panel power to be
> gated when possible. I hope this would also fix the initial
> EDID reading issues I occasionally have and carry a hack for
> as of now...
>
> Thank you a lot for this insight!
>
> Nikita
>
> > I guess your other option would be to implement HPD support in
> > ti-sn65dsi86. That would probably be an overall slower boot for you,
> > but is technically more correct. In the past people have posted up
> > patches to get ti-sn65dsi86 working as a full DP port and they added
> > HPD support for that, but none of those patch series ever go to the
> > point of landing...

Yeah, see the big comment in ti_sn65dsi86_enable_comms().

Actually, looking at how the code has evolved in the meantime, you
could probably get away with:

1. Making sure you have an "hpd-absent-delay-ms" in the device tree
for the panel.

2. Implement "wait_hpd_asserted" in ti-sn65dsi86 to simply be a msleep
with the passed in delay.

Then I think you don't need "no-hpd" anywhere which keeps the device
tree pretty.


-Doug

2023-04-07 19:33:55

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v5 3/4] dt-bindings: arm: qcom: Add Acer Aspire 1

On 07/04/2023 18:23, Doug Anderson wrote:
> Hi,
>
> On Fri, Apr 7, 2023 at 8:14 AM Nikita Travkin <[email protected]> wrote:
>>
>> Acer Aspire 1 is a laptop based on sc7180. Document it's compatible.
>>
>> Signed-off-by: Nikita Travkin <[email protected]>
>> Reviewed-by: Krzysztof Kozlowski <[email protected]>
>> ---
>> Changes in v2:
>> - Merge with IDP (Krzysztof)
>> ---
>> Documentation/devicetree/bindings/arm/qcom.yaml | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
>> index f8d29b65f28b..db97a61e8ccb 100644
>> --- a/Documentation/devicetree/bindings/arm/qcom.yaml
>> +++ b/Documentation/devicetree/bindings/arm/qcom.yaml
>> @@ -367,9 +367,9 @@ properties:
>> - qcom,qru1000-idp
>> - const: qcom,qru1000
>>
>> - - description: Qualcomm Technologies, Inc. SC7180 IDP
>> - items:
>> + - items:
>> - enum:
>> + - acer,aspire1
>> - qcom,sc7180-idp
>> - const: qcom,sc7180
>
> If Krzysztof is happy then I have no real objections here. That being
> said, I personally would have updated the description to be more
> generic and not say "IDP" anymore. Something like "Non-Chromebook
> sc7180 boards".

I am fine with both - dropping the description or changing it to
something meaningful.

Best regards,
Krzysztof

2023-04-08 07:54:40

by Nikita Travkin

[permalink] [raw]
Subject: Re: [PATCH v5 4/4] arm64: dts: qcom: Add Acer Aspire 1

Doug Anderson писал(а) 07.04.2023 22:06:
> Hi,
>
> On Fri, Apr 7, 2023 at 9:46 AM Nikita Travkin <[email protected]> wrote:
>>
>> > HPD might very well be hooked up on your board, but the current Linux
>> > ti-sn65dsi86 driver does not look at its own HPD line because it's
>> > actually slower than just pretending that HPD isn't there. On trogdor
>> > boards we ended up routing HPD to a GPIO.
>> >
>>
>> Oh, this makes so much sense then! The line is hooked up on
>> the board indeed and I remember being confused why trogdor boards
>> don't use it.
>>
>> I will try to add the suggestions (annotating the reason)
>> and verify that it works, would prefer the panel power to be
>> gated when possible. I hope this would also fix the initial
>> EDID reading issues I occasionally have and carry a hack for
>> as of now...
>>
>> Thank you a lot for this insight!
>>
>> Nikita
>>
>> > I guess your other option would be to implement HPD support in
>> > ti-sn65dsi86. That would probably be an overall slower boot for you,
>> > but is technically more correct. In the past people have posted up
>> > patches to get ti-sn65dsi86 working as a full DP port and they added
>> > HPD support for that, but none of those patch series ever go to the
>> > point of landing...
>
> Yeah, see the big comment in ti_sn65dsi86_enable_comms().
>
> Actually, looking at how the code has evolved in the meantime, you
> could probably get away with:
>
> 1. Making sure you have an "hpd-absent-delay-ms" in the device tree
> for the panel.
>
> 2. Implement "wait_hpd_asserted" in ti-sn65dsi86 to simply be a msleep
> with the passed in delay.
>
> Then I think you don't need "no-hpd" anywhere which keeps the device
> tree pretty.
>

Yes, implementing a dummy wait_hpd_asserted seems to indeed fix my
issues here. (Can actually see the panel dying for 200ms on boot
due to the violent takeover if the backlight driver was not built
in, so had to change my kernel config a little)

Will have the "hpd-absent-delay-ms" in v6 and submit a driver fix
alongside it.

Thanks!
Nikita

>
> -Doug