linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] Initial support for the Pixel 3
@ 2022-07-18 21:30 Caleb Connolly
  2022-07-18 21:30 ` [PATCH 1/4] Documentation: dt-bindings: arm: qcom: add google,blueline Caleb Connolly
                   ` (3 more replies)
  0 siblings, 4 replies; 18+ messages in thread
From: Caleb Connolly @ 2022-07-18 21:30 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Rob Herring,
	Krzysztof Kozlowski, Thierry Reding, Sam Ravnborg, David Airlie,
	Daniel Vetter, Sumit Semwal, Caleb Connolly, linux-arm-msm,
	devicetree, linux-kernel, dri-devel, phone-devel,
	~postmarketos/upstreaming, Amit Pundir

This series adds an initial DTS and display panel driver
for the Pixel 3. The Pixel 3 display uses DSC (Display
Stream Compression) which has been supported in mainline
for some time now.

Functionality includes:
 - Display, GPU, venus video transcoder
 - Modem/WiFi/Bluetooth - ModemManager seems to fail

The touchscreen uses some HEFTY downstream driver, hopefully
we'll come up with an upstreamable solution for it soon and
make this a bit more usable.

Amit Pundir (1):
  arm64: dts: qcom: add sdm845-google-blueline (Pixel 3)

Caleb Connolly (1):
  Documentation: dt-bindings: arm: qcom: add google,blueline

Sumit Semwal (2):
  dt-bindings: panel: Add LG SW43408 MIPI-DSI panel
  drm: panel: Add lg sw43408 panel driver

 .../devicetree/bindings/arm/qcom.yaml         |   1 +
 .../bindings/display/panel/lg,43408.yaml      |  41 ++
 .../display/panel/panel-simple-dsi.yaml       |   2 +
 MAINTAINERS                                   |   8 +
 arch/arm64/boot/dts/qcom/Makefile             |   1 +
 .../boot/dts/qcom/sdm845-google-blueline.dts  | 652 ++++++++++++++++++
 drivers/gpu/drm/panel/Kconfig                 |  11 +
 drivers/gpu/drm/panel/Makefile                |   1 +
 drivers/gpu/drm/panel/panel-lg-sw43408.c      | 586 ++++++++++++++++
 9 files changed, 1303 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/lg,43408.yaml
 create mode 100644 arch/arm64/boot/dts/qcom/sdm845-google-blueline.dts
 create mode 100644 drivers/gpu/drm/panel/panel-lg-sw43408.c

-- 
2.36.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/4] Documentation: dt-bindings: arm: qcom: add google,blueline
  2022-07-18 21:30 [PATCH 0/4] Initial support for the Pixel 3 Caleb Connolly
@ 2022-07-18 21:30 ` Caleb Connolly
  2022-07-19  8:36   ` Krzysztof Kozlowski
  2022-07-18 21:30 ` [PATCH 2/4] arm64: dts: qcom: add sdm845-google-blueline (Pixel 3) Caleb Connolly
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 18+ messages in thread
From: Caleb Connolly @ 2022-07-18 21:30 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Rob Herring,
	Krzysztof Kozlowski, Thierry Reding, Sam Ravnborg, David Airlie,
	Daniel Vetter, Sumit Semwal, Caleb Connolly, linux-arm-msm,
	devicetree, linux-kernel, dri-devel, phone-devel,
	~postmarketos/upstreaming

Document the bindings for the Pixel 3

Based on https://lore.kernel.org/all/20220521164550.91115-7-krzysztof.kozlowski@linaro.org/

Signed-off-by: Caleb Connolly <caleb@connolly.tech>
---
 Documentation/devicetree/bindings/arm/qcom.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
index b727467e86c6..b3e1004673c7 100644
--- a/Documentation/devicetree/bindings/arm/qcom.yaml
+++ b/Documentation/devicetree/bindings/arm/qcom.yaml
@@ -320,6 +320,7 @@ properties:
 
       - items:
           - enum:
+              - google,blueline
               - lenovo,yoga-c630
               - oneplus,enchilada
               - oneplus,fajita
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/4] arm64: dts: qcom: add sdm845-google-blueline (Pixel 3)
  2022-07-18 21:30 [PATCH 0/4] Initial support for the Pixel 3 Caleb Connolly
  2022-07-18 21:30 ` [PATCH 1/4] Documentation: dt-bindings: arm: qcom: add google,blueline Caleb Connolly
@ 2022-07-18 21:30 ` Caleb Connolly
  2022-07-18 22:13   ` Dmitry Baryshkov
  2022-07-19  8:45   ` Krzysztof Kozlowski
  2022-07-18 21:30 ` [PATCH 3/4] dt-bindings: panel: Add LG SW43408 MIPI-DSI panel Caleb Connolly
  2022-07-18 21:30 ` [PATCH 4/4] drm: panel: Add lg sw43408 panel driver Caleb Connolly
  3 siblings, 2 replies; 18+ messages in thread
From: Caleb Connolly @ 2022-07-18 21:30 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Rob Herring,
	Krzysztof Kozlowski, Thierry Reding, Sam Ravnborg, David Airlie,
	Daniel Vetter, Sumit Semwal, Caleb Connolly, linux-arm-msm,
	devicetree, linux-kernel, dri-devel, phone-devel,
	~postmarketos/upstreaming
  Cc: Amit Pundir, Vinod Koul

From: Amit Pundir <amit.pundir@linaro.org>

This adds an initial dts for the Blueline (Pixel 3). Supported
functionality includes display, Debug UART, UFS, USB-C (peripheral), WiFi,
Bluetooth and modem.

Bootloader compatible board and msm IDs are needed for the kernel to boot
with Pixel3 bootloader, so those are added.

GPIOs 0 through 3 and 81 through 84 are configured to not be accessible
from the application CPUs, so we mark them as reserved to allow the Pixel 3
to boot.

The reserved-memory locations where obtained from downstream using
kernel logs:
https://gist.github.com/calebccff/090d10bfac3cb9e9bd98dda30b054c96

The rmtfs region is allocated with UIO, making it technically "dynamic".
It's address and size can be read from sysfs:

blueline:/ # cat /sys/class/uio/uio0/name
rmtfs
at /sys/class/uio/uio0/maps/map0/addr
0x00000000f2701000
blueline:/ # cat /sys/class/uio/uio0/maps/map0/size
0x0000000000200000

Like the OnePlus 6, it needs 1kB reserved on either side of the rmtfs
memory to workaround some XPU bug which would otherwise cause erroneous
XPU violations when accessing the rmtfs_mem region.

For wifi, the pixel 3 reports a board-id of 0xFF, and downstream
only includes a single bdwlan file. The qcom,ath10k-calibration-variant
property is set to ensure that the correct calibration data is used.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
[AmitP: Cherry-picked and refactored from Bjorn's db845c dts
        ("arm64: dts: qcom: Add Dragonboard 845c") https://lkml.org/lkml/2019/6/6/7]
Signed-off-by: Amit Pundir <amit.pundir@linaro.org>
[sumits: merged commits to add board and msm ids, gpio range reservation,
  ufs device-reset gpio and adaptation to v5.5+ changes]
Signed-off-by: Sumit Semwal <sumit.semwal@linaro.org>
[vinod: Add display nodes]
Signed-off-by: Vinod Koul <vkoul@kernel.org>
[caleb: remove db845c bits, cleanup, add reserved-memory for modem/wifi]
Signed-off-by: Caleb Connolly <caleb@connolly.tech>
---
 arch/arm64/boot/dts/qcom/Makefile             |   1 +
 .../boot/dts/qcom/sdm845-google-blueline.dts  | 652 ++++++++++++++++++
 2 files changed, 653 insertions(+)
 create mode 100644 arch/arm64/boot/dts/qcom/sdm845-google-blueline.dts

diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
index 2f8aec2cc6db..c151e17e6eb7 100644
--- a/arch/arm64/boot/dts/qcom/Makefile
+++ b/arch/arm64/boot/dts/qcom/Makefile
@@ -100,6 +100,7 @@ dtb-$(CONFIG_ARCH_QCOM)	+= sdm845-cheza-r1.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sdm845-cheza-r2.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sdm845-cheza-r3.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sdm845-db845c.dtb
+dtb-$(CONFIG_ARCH_QCOM)	+= sdm845-google-blueline.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sdm845-mtp.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sdm845-oneplus-enchilada.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= sdm845-oneplus-fajita.dtb
diff --git a/arch/arm64/boot/dts/qcom/sdm845-google-blueline.dts b/arch/arm64/boot/dts/qcom/sdm845-google-blueline.dts
new file mode 100644
index 000000000000..dec979ad9209
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/sdm845-google-blueline.dts
@@ -0,0 +1,652 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/dts-v1/;
+
+#include <dt-bindings/dma/qcom-gpi.h>
+#include <dt-bindings/input/linux-event-codes.h>
+#include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/regulator/qcom,rpmh-regulator.h>
+
+#include "sdm845.dtsi"
+#include "pm8998.dtsi"
+#include "pmi8998.dtsi"
+
+/delete-node/ &mpss_region;
+/delete-node/ &venus_mem;
+/delete-node/ &cdsp_mem;
+/delete-node/ &mba_region;
+/delete-node/ &slpi_mem;
+/delete-node/ &spss_mem;
+/delete-node/ &rmtfs_mem;
+
+/ {
+	model = "Google Pixel 3";
+	compatible = "google,blueline", "qcom,sdm845";
+	qcom,board-id = <0x00021505 0>;
+	qcom,msm-id = <321 0x20001>;
+
+	aliases {
+		serial0 = &uart9;
+		serial1 = &uart6;
+	};
+
+	chosen {
+		stdout-path = "serial0:115200n8";
+	};
+
+	volume-keys {
+		compatible = "gpio-keys";
+		label = "Volume keys";
+		autorepeat;
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&volume_up_gpio>;
+
+		vol-up {
+			label = "Volume Up";
+			linux,code = <KEY_VOLUMEUP>;
+			gpios = <&pm8998_gpio 6 GPIO_ACTIVE_LOW>;
+			debounce-interval = <15>;
+		};
+	};
+
+	reserved-memory {
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		mpss_region: memory@8e000000 {
+			reg = <0 0x8e000000 0 0x9800000>;
+			no-map;
+		};
+
+		venus_mem: venus@97800000 {
+			reg = <0 0x97800000 0 0x500000>;
+			no-map;
+		};
+
+		cdsp_mem: cdsp-mem@97D00000 {
+			reg = <0 0x97D00000 0 0x800000>;
+			no-map;
+		};
+
+		mba_region: mba@98500000 {
+			reg = <0 0x98500000 0 0x200000>;
+			no-map;
+		};
+
+		slpi_mem: slpi@98700000 {
+			reg = <0 0x98700000 0 0x1400000>;
+			no-map;
+		};
+
+		spss_mem: spss@99B00000 {
+			reg = <0 0x99B00000 0 0x100000>;
+			no-map;
+		};
+
+		/* rmtfs lower guard */
+		memory@f2700000 {
+			reg = <0 0xf2700000 0 0x1000>;
+			no-map;
+		};
+
+		rmtfs_mem: memory@f2701000 {
+			compatible = "qcom,rmtfs-mem";
+			reg = <0 0xf2701000 0 0x200000>;
+			no-map;
+
+			qcom,client-id = <1>;
+			qcom,vmid = <15>;
+		};
+
+		/* rmtfs upper guard */
+		memory@f2901000 {
+			reg = <0 0xf2901000 0 0x1000>;
+			no-map;
+		};
+	};
+
+	vph_pwr: vph-pwr-regulator {
+		compatible = "regulator-fixed";
+		regulator-name = "vph_pwr";
+		regulator-min-microvolt = <3700000>;
+		regulator-max-microvolt = <3700000>;
+	};
+
+	vreg_s4a_1p8: vreg-s4a-1p8 {
+		compatible = "regulator-fixed";
+		regulator-name = "vreg_s4a_1p8";
+
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <1800000>;
+		regulator-always-on;
+		regulator-boot-on;
+
+		vin-supply = <&vph_pwr>;
+	};
+};
+
+&adsp_pas {
+	status = "okay";
+
+	firmware-name = "qcom/sdm845/pixel3/adsp.mbn";
+};
+
+&apps_rsc {
+	pm8998-rpmh-regulators {
+		compatible = "qcom,pm8998-rpmh-regulators";
+		qcom,pmic-id = "a";
+		vdd-s1-supply = <&vph_pwr>;
+		vdd-s2-supply = <&vph_pwr>;
+		vdd-s3-supply = <&vph_pwr>;
+		vdd-s4-supply = <&vph_pwr>;
+		vdd-s5-supply = <&vph_pwr>;
+		vdd-s6-supply = <&vph_pwr>;
+		vdd-s7-supply = <&vph_pwr>;
+		vdd-s8-supply = <&vph_pwr>;
+		vdd-s9-supply = <&vph_pwr>;
+		vdd-s10-supply = <&vph_pwr>;
+		vdd-s11-supply = <&vph_pwr>;
+		vdd-s12-supply = <&vph_pwr>;
+		vdd-s13-supply = <&vph_pwr>;
+		vdd-l1-l27-supply = <&vreg_s7a_1p025>;
+		vdd-l2-l8-l17-supply = <&vreg_s3a_1p35>;
+		vdd-l3-l11-supply = <&vreg_s7a_1p025>;
+		vdd-l4-l5-supply = <&vreg_s7a_1p025>;
+		vdd-l6-supply = <&vph_pwr>;
+		vdd-l7-l12-l14-l15-supply = <&vreg_s5a_2p04>;
+		vdd-l9-supply = <&vreg_bob>;
+		vdd-l10-l23-l25-supply = <&vreg_bob>;
+		vdd-l13-l19-l21-supply = <&vreg_bob>;
+		vdd-l16-l28-supply = <&vreg_bob>;
+		vdd-l18-l22-supply = <&vreg_bob>;
+		vdd-l20-l24-supply = <&vreg_bob>;
+		vdd-l26-supply = <&vreg_s3a_1p35>;
+		vin-lvs-1-2-supply = <&vreg_s4a_1p8>;
+
+		vreg_s3a_1p35: smps3 {
+			regulator-min-microvolt = <1352000>;
+			regulator-max-microvolt = <1352000>;
+		};
+
+		vreg_s5a_2p04: smps5 {
+			regulator-min-microvolt = <1904000>;
+			regulator-max-microvolt = <2040000>;
+		};
+
+		vreg_s7a_1p025: smps7 {
+			regulator-min-microvolt = <900000>;
+			regulator-max-microvolt = <1028000>;
+		};
+
+		vdda_mipi_dsi0_pll:
+		vreg_l1a_0p875: ldo1 {
+			regulator-min-microvolt = <880000>;
+			regulator-max-microvolt = <880000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+			regulator-boot-on;
+		};
+
+		vreg_l5a_0p8: ldo5 {
+			regulator-min-microvolt = <800000>;
+			regulator-max-microvolt = <800000>;
+			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_l7a_1p8: ldo7 {
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l13a_2p95: ldo13 {
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <2960000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l14a_1p88: ldo14 {
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+			regulator-boot-on;
+			/*
+			 * We can't properly bring the panel back if it gets turned off
+			 * so keep it's regulators always on for now.
+			 */
+			regulator-always-on;
+		};
+
+		vreg_l17a_1p3: ldo17 {
+			regulator-min-microvolt = <1304000>;
+			regulator-max-microvolt = <1304000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l19a_3p3: ldo19 {
+			regulator-min-microvolt = <3300000>;
+			regulator-max-microvolt = <3312000>;
+			qcom,init-voltage = <3300000>;
+			qcom,initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+			/*
+			 * The touchscreen needs this to be 3.3v, which is apparently
+			 * quite close to the hardware limit for this LDO (3.312v)
+			 * It must be kept in high power mode to prevent TS brownouts
+			 */
+			regulator-allowed-modes = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l20a_2p95: ldo20 {
+			regulator-min-microvolt = <2960000>;
+			regulator-max-microvolt = <2968000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l21a_2p95: ldo21 {
+			regulator-min-microvolt = <2960000>;
+			regulator-max-microvolt = <2968000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l24a_3p075: ldo24 {
+			regulator-min-microvolt = <3088000>;
+			regulator-max-microvolt = <3088000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vreg_l25a_3p3: ldo25 {
+			regulator-min-microvolt = <3300000>;
+			regulator-max-microvolt = <3312000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+		};
+
+		vdda_mipi_dsi0_1p2:
+		vreg_l26a_1p2: ldo26 {
+			regulator-min-microvolt = <1200000>;
+			regulator-max-microvolt = <1200000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+			regulator-boot-on;
+		};
+
+		vreg_l28a_3p0: ldo28 {
+			regulator-min-microvolt = <2856000>;
+			regulator-max-microvolt = <3008000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+			regulator-boot-on;
+			/*
+			 * We can't properly bring the panel back if it gets turned off
+			 * so keep it's regulators always on for now.
+			 */
+			regulator-always-on;
+		};
+	};
+
+	pmi8998-rpmh-regulators {
+		compatible = "qcom,pmi8998-rpmh-regulators";
+		qcom,pmic-id = "b";
+
+		vdd-bob-supply = <&vph_pwr>;
+
+		vreg_bob: bob {
+			regulator-min-microvolt = <3312000>;
+			regulator-max-microvolt = <3600000>;
+			regulator-initial-mode = <RPMH_REGULATOR_MODE_AUTO>;
+			regulator-allow-bypass;
+		};
+	};
+
+	pm8005-rpmh-regulators {
+		compatible = "qcom,pm8005-rpmh-regulators";
+		qcom,pmic-id = "c";
+
+		vdd-s1-supply = <&vph_pwr>;
+		vdd-s2-supply = <&vph_pwr>;
+		vdd-s3-supply = <&vph_pwr>;
+		vdd-s4-supply = <&vph_pwr>;
+
+		vreg_s3c_0p6: smps3 {
+			regulator-min-microvolt = <600000>;
+			regulator-max-microvolt = <600000>;
+		};
+	};
+};
+
+&cdsp_pas {
+	status = "okay";
+	firmware-name = "qcom/sdm845/pixel3/cdsp.mbn";
+};
+
+&dsi0 {
+	status = "okay";
+	vdda-supply = <&vdda_mipi_dsi0_1p2>;
+
+	ports {
+		port@1 {
+			endpoint {
+				remote-endpoint = <&lg_sw43408_in_0>;
+				data-lanes = <0 1 2 3>;
+			};
+		};
+	};
+
+	panel@0 {
+		compatible = "lg,sw43408";
+		reg = <0>;
+
+		vddi-supply = <&vreg_l14a_1p88>;
+		vpnl-supply = <&vreg_l28a_3p0>;
+
+		reset-gpios = <&tlmm 6 GPIO_ACTIVE_LOW>;
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&panel_reset_pins &panel_te_pin &panel_pmgpio_pins>;
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port@0 {
+				reg = <0>;
+				lg_sw43408_in_0: endpoint {
+					remote-endpoint = <&dsi0_out>;
+				};
+			};
+		};
+	};
+};
+
+&dsi0_out {
+	remote-endpoint = <&lg_sw43408_in_0>;
+	data-lanes = <0 1 2 3>;
+};
+
+&dsi0_phy {
+	status = "okay";
+	vdds-supply = <&vdda_mipi_dsi0_pll>;
+};
+
+&gcc {
+	protected-clocks = <GCC_QSPI_CORE_CLK>,
+			   <GCC_QSPI_CORE_CLK_SRC>,
+			   <GCC_QSPI_CNOC_PERIPH_AHB_CLK>;
+};
+
+&gmu {
+	status = "okay";
+};
+
+&gpi_dma0 {
+	status = "okay";
+};
+
+&gpu {
+	status = "okay";
+
+	zap-shader {
+		memory-region = <&gpu_mem>;
+		firmware-name = "qcom/sdm845/pixel3/a630_zap.mbn";
+	};
+};
+
+&ipa {
+	status = "okay";
+
+	firmware-name = "qcom/sdm845/pixel3/ipa_fws.mbn";
+};
+
+&mss_pil {
+	status = "okay";
+	firmware-name = "qcom/sdm845/pixel3/mba.mbn", "qcom/sdm845/pixel3/modem.mbn";
+};
+
+&mdss {
+	status = "okay";
+};
+
+&mdss_mdp {
+	status = "okay";
+};
+
+&pm8998_gpio {
+	volume_up_gpio: vol-up-active {
+		pins = "gpio6";
+		function = "normal";
+		input-enable;
+		bias-pull-up;
+		qcom,drive-strength = <0>;
+	};
+
+	panel_pmgpio_pins: panel-pmgpio-active {
+		pins = "gpio2", "gpio5";
+		function = "normal";
+		input-enable;
+		bias-disable;
+		power-source = <0>;
+	};
+};
+
+&pm8998_pon {
+	resin {
+		compatible = "qcom,pm8941-resin";
+		interrupts = <0x0 0x8 1 IRQ_TYPE_EDGE_BOTH>;
+		debounce = <15625>;
+		bias-pull-up;
+		linux,code = <KEY_VOLUMEDOWN>;
+	};
+};
+
+&qupv3_id_0 {
+	status = "okay";
+};
+
+&qupv3_id_1 {
+	status = "okay";
+};
+
+&qup_uart6_default {
+	pinmux {
+		pins = "gpio45", "gpio46", "gpio47", "gpio48";
+		function = "qup6";
+	};
+
+	cts {
+		pins = "gpio45";
+		bias-disable;
+	};
+
+	rts-tx {
+		pins = "gpio46", "gpio47";
+		drive-strength = <2>;
+		bias-disable;
+	};
+
+	rx {
+		pins = "gpio48";
+		bias-pull-up;
+	};
+};
+
+&qup_uart9_default {
+	pinconf-tx {
+		pins = "gpio4";
+		drive-strength = <2>;
+		bias-disable;
+	};
+
+	pinconf-rx {
+		pins = "gpio5";
+		drive-strength = <2>;
+		bias-pull-up;
+	};
+};
+
+&tlmm {
+	gpio-reserved-ranges = <0 4>, <81 4>;
+
+	panel_te_pin: panel-te {
+		mux {
+			pins = "gpio12";
+			function = "mdp_vsync";
+			drive-strength = <2>;
+			bias-disable;
+			input-enable;
+		};
+	};
+
+	panel_reset_pins: panel-active {
+		mux {
+			pins = "gpio6", "gpio52";
+			function = "gpio";
+			drive-strength = <8>;
+			bias-disable = <0>;
+		};
+	};
+
+	panel_suspend: panel-suspend {
+		mux {
+			pins = "gpio6", "gpio52";
+			function = "gpio";
+			drive-strength = <2>;
+			bias-pull-down;
+		};
+	};
+
+	touchscreen_reset: ts-reset {
+		mux {
+			pins = "gpio99";
+			function = "gpio";
+			drive-strength = <8>;
+			bias-pull-up;
+			//output-high;
+		};
+	};
+
+	touchscreen_pins: ts-pins {
+		mux {
+			pins = "gpio125";
+			function = "gpio";
+			drive-strength = <2>;
+			bias-disable;
+		};
+	};
+
+	touchscreen_i2c_pins: qup-i2c2-gpio {
+		pins = "gpio27", "gpio28";
+		function = "gpio";
+
+		drive-strength = <2>;
+		bias-disable;
+	};
+};
+
+&uart6 {
+	status = "okay";
+
+	bluetooth {
+		compatible = "qcom,wcn3990-bt";
+
+		vddio-supply = <&vreg_s4a_1p8>;
+		vddxo-supply = <&vreg_l7a_1p8>;
+		vddrf-supply = <&vreg_l17a_1p3>;
+		vddch0-supply = <&vreg_l25a_3p3>;
+		max-speed = <3200000>;
+	};
+};
+
+&uart9 {
+	status = "okay";
+};
+
+&usb_1 {
+	status = "okay";
+};
+
+&usb_1_dwc3 {
+	dr_mode = "peripheral";
+};
+
+&usb_1_hsphy {
+	status = "okay";
+
+	vdd-supply = <&vreg_l1a_0p875>;
+	vdda-pll-supply = <&vreg_l12a_1p8>;
+	vdda-phy-dpdm-supply = <&vreg_l24a_3p075>;
+
+	qcom,imp-res-offset-value = <8>;
+	qcom,hstx-trim-value = <QUSB2_V2_HSTX_TRIM_21_6_MA>;
+	qcom,preemphasis-level = <QUSB2_V2_PREEMPHASIS_5_PERCENT>;
+	qcom,preemphasis-width = <QUSB2_V2_PREEMPHASIS_WIDTH_HALF_BIT>;
+};
+
+&usb_1_qmpphy {
+	status = "okay";
+
+	vdda-phy-supply = <&vreg_l26a_1p2>;
+	vdda-pll-supply = <&vreg_l1a_0p875>;
+};
+
+&usb_2 {
+	status = "okay";
+};
+
+&usb_2_dwc3 {
+	dr_mode = "host";
+};
+
+&usb_2_hsphy {
+	status = "okay";
+
+	vdd-supply = <&vreg_l1a_0p875>;
+	vdda-pll-supply = <&vreg_l12a_1p8>;
+	vdda-phy-dpdm-supply = <&vreg_l24a_3p075>;
+
+	qcom,imp-res-offset-value = <8>;
+	qcom,hstx-trim-value = <QUSB2_V2_HSTX_TRIM_22_8_MA>;
+};
+
+&usb_2_qmpphy {
+	status = "okay";
+
+	vdda-phy-supply = <&vreg_l26a_1p2>;
+	vdda-pll-supply = <&vreg_l1a_0p875>;
+};
+
+&ufs_mem_hc {
+	status = "okay";
+
+	reset-gpios = <&tlmm 150 GPIO_ACTIVE_LOW>;
+
+	vcc-supply = <&vreg_l20a_2p95>;
+	vcc-max-microamp = <800000>;
+};
+
+&ufs_mem_phy {
+	status = "okay";
+
+	vdda-phy-supply = <&vreg_l1a_0p875>;
+	vdda-pll-supply = <&vreg_l26a_1p2>;
+};
+
+&venus {
+	status = "okay";
+	firmware-name = "qcom/sdm845/oneplus6/venus.mbn";
+};
+
+&wifi {
+	status = "okay";
+
+	vdd-0.8-cx-mx-supply = <&vreg_l5a_0p8>;
+	vdd-1.8-xo-supply = <&vreg_l7a_1p8>;
+	vdd-1.3-rfa-supply = <&vreg_l17a_1p3>;
+	vdd-3.3-ch0-supply = <&vreg_l25a_3p3>;
+
+	qcom,snoc-host-cap-8bit-quirk;
+	qcom,ath10k-calibration-variant = "google_blueline";
+};
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 3/4] dt-bindings: panel: Add LG SW43408 MIPI-DSI panel
  2022-07-18 21:30 [PATCH 0/4] Initial support for the Pixel 3 Caleb Connolly
  2022-07-18 21:30 ` [PATCH 1/4] Documentation: dt-bindings: arm: qcom: add google,blueline Caleb Connolly
  2022-07-18 21:30 ` [PATCH 2/4] arm64: dts: qcom: add sdm845-google-blueline (Pixel 3) Caleb Connolly
@ 2022-07-18 21:30 ` Caleb Connolly
  2022-07-19  6:10   ` Sam Ravnborg
  2022-07-19 14:11   ` Rob Herring
  2022-07-18 21:30 ` [PATCH 4/4] drm: panel: Add lg sw43408 panel driver Caleb Connolly
  3 siblings, 2 replies; 18+ messages in thread
From: Caleb Connolly @ 2022-07-18 21:30 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Rob Herring,
	Krzysztof Kozlowski, Thierry Reding, Sam Ravnborg, David Airlie,
	Daniel Vetter, Sumit Semwal, Caleb Connolly, linux-arm-msm,
	devicetree, linux-kernel, dri-devel, phone-devel,
	~postmarketos/upstreaming
  Cc: Vinod Koul

From: Sumit Semwal <sumit.semwal@linaro.org>

LG SW43408 is 1080x2160, 4-lane MIPI-DSI panel.

Signed-off-by: Vinod Koul <vkoul@kernel.org>
Signed-off-by: Sumit Semwal <sumit.semwal@linaro.org>
[caleb: convert to yaml]
Signed-off-by: Caleb Connolly <caleb@connolly.tech>
---
 .../bindings/display/panel/lg,43408.yaml      | 41 +++++++++++++++++++
 .../display/panel/panel-simple-dsi.yaml       |  2 +
 2 files changed, 43 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/lg,43408.yaml

diff --git a/Documentation/devicetree/bindings/display/panel/lg,43408.yaml b/Documentation/devicetree/bindings/display/panel/lg,43408.yaml
new file mode 100644
index 000000000000..0529a3aa2692
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/lg,43408.yaml
@@ -0,0 +1,41 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/panel/panel-lvds.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: LG SW43408 1080x2160 DSI panel
+
+maintainers:
+  - Caleb Connolly <caleb@connolly.tech>
+
+description: |
+  This panel is used on the Pixel 3, it is a 60hz OLED panel which
+  required DSC (Display Stream Compression) and has rounded corners.
+
+allOf:
+  - $ref: panel-common.yaml#
+
+properties:
+  compatible:
+    items:
+      - const: lg,sw43408
+
+  vddi-supply: true
+  vpnl-supply: true
+  reset-gpios: true
+
+  backlight: false
+  power-supply: false
+
+additionalProperties: false
+
+required:
+  - compatible
+  - data-mapping
+  - width-mm
+  - height-mm
+  - panel-timing
+  - port
+
+...
diff --git a/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml b/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml
index 2c00813f5d20..4498078cb1ee 100644
--- a/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml
+++ b/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml
@@ -45,6 +45,8 @@ properties:
       - lg,acx467akm-7
         # LG Corporation 7" WXGA TFT LCD panel
       - lg,ld070wx3-sl01
+        # LG Corporation sw43408 1080x2160 OLED
+      - lg,sw43408
         # One Stop Displays OSD101T2587-53TS 10.1" 1920x1200 panel
       - osddisplays,osd101t2587-53ts
         # Panasonic 10" WUXGA TFT LCD panel
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 4/4] drm: panel: Add lg sw43408 panel driver
  2022-07-18 21:30 [PATCH 0/4] Initial support for the Pixel 3 Caleb Connolly
                   ` (2 preceding siblings ...)
  2022-07-18 21:30 ` [PATCH 3/4] dt-bindings: panel: Add LG SW43408 MIPI-DSI panel Caleb Connolly
@ 2022-07-18 21:30 ` Caleb Connolly
  2022-07-18 23:06   ` Dmitry Baryshkov
                     ` (2 more replies)
  3 siblings, 3 replies; 18+ messages in thread
From: Caleb Connolly @ 2022-07-18 21:30 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Rob Herring,
	Krzysztof Kozlowski, Thierry Reding, Sam Ravnborg, David Airlie,
	Daniel Vetter, Sumit Semwal, Caleb Connolly, linux-arm-msm,
	devicetree, linux-kernel, dri-devel, phone-devel,
	~postmarketos/upstreaming
  Cc: Vinod Koul

From: Sumit Semwal <sumit.semwal@linaro.org>

LG SW43408 is 1080x2160, 4-lane MIPI-DSI panel, used in some Pixel3
phones.

Whatever init sequence we have for this panel isn't capable of
initialising it completely, toggling the reset gpio ever causes the
panel to die. Until this is resolved we avoid resetting the panel. The
disable/unprepare functions only put the panel to sleep mode and
disable the backlight.

Signed-off-by: Sumit Semwal <sumit.semwal@linaro.org>
[vinod: Add DSC support]
Signed-off-by: Vinod Koul <vkoul@kernel.org>
[caleb: cleanup and support turning off the panel]
Signed-off-by: Caleb Connolly <caleb@connolly.tech>
---
 MAINTAINERS                              |   8 +
 drivers/gpu/drm/panel/Kconfig            |  11 +
 drivers/gpu/drm/panel/Makefile           |   1 +
 drivers/gpu/drm/panel/panel-lg-sw43408.c | 586 +++++++++++++++++++++++
 4 files changed, 606 insertions(+)
 create mode 100644 drivers/gpu/drm/panel/panel-lg-sw43408.c

diff --git a/MAINTAINERS b/MAINTAINERS
index f679152bdbad..8a2b954ad140 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6376,6 +6376,14 @@ S:	Orphan / Obsolete
 F:	drivers/gpu/drm/i810/
 F:	include/uapi/drm/i810_drm.h
 
+DRM DRIVER FOR LG SW43408 PANELS
+M:	Sumit Semwal <sumit.semwal@linaro.org>
+M:	Caleb Connolly <caleb@connolly.tech>
+S:	Maintained
+T:	git git://anongit.freedesktop.org/drm/drm-misc
+F:	Documentation/devicetree/bindings/display/panel/lg,sw43408-panel.txt
+F:	drivers/gpu/drm/panel/panel-lg-sw43408.c
+
 DRM DRIVER FOR LVDS PANELS
 M:	Laurent Pinchart <laurent.pinchart@ideasonboard.com>
 L:	dri-devel@lists.freedesktop.org
diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 38799effd00a..706b112794b9 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -256,6 +256,17 @@ config DRM_PANEL_LEADTEK_LTK500HD1829
 	  24 bit RGB per pixel. It provides a MIPI DSI interface to
 	  the host and has a built-in LED backlight.
 
+config DRM_PANEL_LG_SW43408
+	tristate "LG SW43408 panel"
+	depends on OF
+	depends on DRM_MIPI_DSI
+	depends on BACKLIGHT_CLASS_DEVICE
+	help
+	  Say Y here if you want to enable support for LG sw43408 panel.
+	  The panel has a 1080x2160 resolution and uses
+	  24 bit RGB per pixel. It provides a MIPI DSI interface to
+	  the host and has a built-in LED backlight.
+
 config DRM_PANEL_SAMSUNG_LD9040
 	tristate "Samsung LD9040 RGB/SPI panel"
 	depends on OF && SPI
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index 42a7ab54234b..ba26a69b74e7 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -25,6 +25,7 @@ obj-$(CONFIG_DRM_PANEL_LEADTEK_LTK050H3146W) += panel-leadtek-ltk050h3146w.o
 obj-$(CONFIG_DRM_PANEL_LEADTEK_LTK500HD1829) += panel-leadtek-ltk500hd1829.o
 obj-$(CONFIG_DRM_PANEL_LG_LB035Q02) += panel-lg-lb035q02.o
 obj-$(CONFIG_DRM_PANEL_LG_LG4573) += panel-lg-lg4573.o
+obj-$(CONFIG_DRM_PANEL_LG_SW43408) += panel-lg-sw43408.o
 obj-$(CONFIG_DRM_PANEL_NEC_NL8048HL11) += panel-nec-nl8048hl11.o
 obj-$(CONFIG_DRM_PANEL_NEWVISION_NV3052C) += panel-newvision-nv3052c.o
 obj-$(CONFIG_DRM_PANEL_NOVATEK_NT35510) += panel-novatek-nt35510.o
diff --git a/drivers/gpu/drm/panel/panel-lg-sw43408.c b/drivers/gpu/drm/panel/panel-lg-sw43408.c
new file mode 100644
index 000000000000..c7b8ec7b970d
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-lg-sw43408.c
@@ -0,0 +1,586 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2019 Linaro Ltd
+ * Author: Sumit Semwal <sumit.semwal@linaro.org>
+ */
+
+#include <linux/backlight.h>
+#include <linux/delay.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+
+#include <linux/gpio/consumer.h>
+#include <linux/regulator/consumer.h>
+
+#include <drm/drm_device.h>
+#include <drm/drm_mipi_dsi.h>
+#include <drm/drm_modes.h>
+#include <drm/drm_panel.h>
+#include <drm/drm_print.h>
+#include <drm/display/drm_dsc.h>
+#include <drm/display/drm_dsc_helper.h>
+
+#include <video/mipi_display.h>
+
+struct panel_cmd {
+	size_t len;
+	const char *data;
+};
+
+#define _INIT_CMD(...)                                                   \
+	{                                                                \
+		.len = sizeof((char[]){ __VA_ARGS__ }), .data = (char[]) \
+		{                                                        \
+			__VA_ARGS__                                      \
+		}                                                        \
+	}
+
+static const char *const regulator_names[] = {
+	"vddi",
+	"vpnl",
+};
+
+static const unsigned long regulator_enable_loads[] = {
+	62000,
+	857000,
+};
+
+static const unsigned long regulator_disable_loads[] = {
+	80,
+	0,
+};
+
+struct sw43408_panel {
+	struct drm_panel base;
+	struct mipi_dsi_device *link;
+
+	const struct drm_display_mode *mode;
+	struct backlight_device *backlight;
+
+	struct regulator_bulk_data supplies[ARRAY_SIZE(regulator_names)];
+
+	struct gpio_desc *reset_gpio;
+
+	bool prepared;
+	bool enabled;
+};
+
+static const struct panel_cmd lg_sw43408_on_cmds_1[] = {
+	_INIT_CMD(0x00, 0x53, 0x0C, 0x30),
+	_INIT_CMD(0x00, 0x55, 0x00, 0x70, 0xDF, 0x00, 0x70, 0xDF),
+	_INIT_CMD(0x00, 0xF7, 0x01, 0x49, 0x0C),
+
+	{},
+};
+
+static const struct panel_cmd lg_sw43408_on_cmds_2[] = {
+	_INIT_CMD(0x00, 0xB0, 0xAC),
+	_INIT_CMD(0x00, 0xCD, 0x00, 0x00, 0x00, 0x19, 0x19, 0x19, 0x19, 0x19,
+		  0x19, 0x19, 0x19, 0x19, 0x19, 0x19, 0x19, 0x19, 0x16, 0x16),
+	_INIT_CMD(0x00, 0xCB, 0x80, 0x5C, 0x07, 0x03, 0x28),
+	_INIT_CMD(0x00, 0xC0, 0x02, 0x02, 0x0F),
+	_INIT_CMD(0x00, 0xE5, 0x00, 0x3A, 0x00, 0x3A, 0x00, 0x0E, 0x10),
+	_INIT_CMD(0x00, 0xB5, 0x75, 0x60, 0x2D, 0x5D, 0x80, 0x00, 0x0A, 0x0B,
+		  0x00, 0x05, 0x0B, 0x00, 0x80, 0x0D, 0x0E, 0x40, 0x00, 0x0C,
+		  0x00, 0x16, 0x00, 0xB8, 0x00, 0x80, 0x0D, 0x0E, 0x40, 0x00,
+		  0x0C, 0x00, 0x16, 0x00, 0xB8, 0x00, 0x81, 0x00, 0x03, 0x03,
+		  0x03, 0x01, 0x01),
+	_INIT_CMD(0x00, 0x55, 0x04, 0x61, 0xDB, 0x04, 0x70, 0xDB),
+	_INIT_CMD(0x00, 0xB0, 0xCA),
+
+	{},
+};
+
+static inline struct sw43408_panel *to_panel_info(struct drm_panel *panel)
+{
+	return container_of(panel, struct sw43408_panel, base);
+}
+
+/*
+ * Currently unable to bring up the panel after resetting, must be missing
+ * some init commands somewhere.
+ */
+static __always_unused int panel_reset(struct sw43408_panel *ctx)
+{
+	int ret = 0, i;
+
+	for (i = 0; i < ARRAY_SIZE(ctx->supplies); i++) {
+		ret = regulator_set_load(ctx->supplies[i].consumer,
+					 regulator_enable_loads[i]);
+		if (ret)
+			return ret;
+	}
+
+	ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
+	if (ret < 0)
+		return ret;
+
+	for (i = 0; i < ARRAY_SIZE(ctx->supplies); i++) {
+		ret = regulator_set_load(ctx->supplies[i].consumer,
+					 regulator_disable_loads[i]);
+		if (ret) {
+			DRM_DEV_ERROR(ctx->base.dev,
+				      "regulator_set_load failed %d\n", ret);
+			return ret;
+		}
+	}
+
+	ret = regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
+	if (ret < 0)
+		return ret;
+
+	gpiod_set_value(ctx->reset_gpio, 0);
+	usleep_range(9000, 10000);
+	gpiod_set_value(ctx->reset_gpio, 1);
+	usleep_range(1000, 2000);
+	gpiod_set_value(ctx->reset_gpio, 0);
+	usleep_range(9000, 10000);
+
+	return 0;
+}
+
+static int send_mipi_cmds(struct drm_panel *panel, const struct panel_cmd *cmds)
+{
+	struct sw43408_panel *ctx = to_panel_info(panel);
+	unsigned int i = 0;
+	int err;
+
+	if (!cmds)
+		return -EFAULT;
+
+	for (i = 0; cmds[i].len != 0; i++) {
+		const struct panel_cmd *cmd = &cmds[i];
+
+		if (cmd->len == 2)
+			err = mipi_dsi_dcs_write(ctx->link, cmd->data[1], NULL,
+						 0);
+		else
+			err = mipi_dsi_dcs_write(ctx->link, cmd->data[1],
+						 cmd->data + 2, cmd->len - 2);
+
+		if (err < 0)
+			return err;
+
+		usleep_range((cmd->data[0]) * 1000, (1 + cmd->data[0]) * 1000);
+	}
+
+	return 0;
+}
+
+static int lg_panel_disable(struct drm_panel *panel)
+{
+	struct sw43408_panel *ctx = to_panel_info(panel);
+
+	backlight_disable(ctx->backlight);
+	ctx->enabled = false;
+
+	return 0;
+}
+
+/*
+ * We can't currently re-initialise the panel properly after powering off.
+ * This function will be used when this is resolved.
+ */
+static __always_unused int lg_panel_power_off(struct drm_panel *panel)
+{
+	struct sw43408_panel *ctx = to_panel_info(panel);
+	int i, ret = 0;
+
+	gpiod_set_value(ctx->reset_gpio, 1);
+
+	for (i = 0; i < ARRAY_SIZE(ctx->supplies); i++) {
+		ret = regulator_set_load(ctx->supplies[i].consumer,
+					 regulator_disable_loads[i]);
+		if (ret) {
+			DRM_DEV_ERROR(panel->dev,
+				      "regulator_set_load failed %d\n", ret);
+			return ret;
+		}
+	}
+
+	ret = regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
+	if (ret) {
+		DRM_DEV_ERROR(panel->dev, "regulator_bulk_disable failed %d\n",
+			      ret);
+	}
+	return ret;
+}
+
+static int lg_panel_unprepare(struct drm_panel *panel)
+{
+	struct sw43408_panel *ctx = to_panel_info(panel);
+	int ret, i;
+
+	if (!ctx->prepared)
+		return 0;
+
+	ret = mipi_dsi_dcs_set_display_off(ctx->link);
+	if (ret < 0) {
+		DRM_DEV_ERROR(panel->dev,
+			      "set_display_off cmd failed ret = %d\n", ret);
+	}
+
+	msleep(120);
+
+	ret = mipi_dsi_dcs_enter_sleep_mode(ctx->link);
+	if (ret < 0) {
+		DRM_DEV_ERROR(panel->dev, "enter_sleep cmd failed ret = %d\n",
+			      ret);
+	}
+
+	/* Would call panel_power_off() */
+
+	for (i = 0; i < ARRAY_SIZE(ctx->supplies); i++) {
+		ret = regulator_set_load(ctx->supplies[i].consumer,
+					 regulator_disable_loads[i]);
+		if (ret) {
+			DRM_DEV_ERROR(panel->dev,
+				      "regulator_set_load failed %d\n", ret);
+			return ret;
+		}
+	}
+
+	ctx->prepared = false;
+
+	return ret;
+}
+
+static int lg_panel_prepare(struct drm_panel *panel)
+{
+	struct sw43408_panel *ctx = to_panel_info(panel);
+	int err, i;
+
+	if (ctx->prepared)
+		return 0;
+
+	/* Would call panel_reset() */
+
+	for (i = 0; i < ARRAY_SIZE(ctx->supplies); i++) {
+		err = regulator_set_load(ctx->supplies[i].consumer,
+					 regulator_enable_loads[i]);
+		if (err)
+			return err;
+	}
+
+	err = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
+	if (err < 0)
+		return err;
+
+	usleep_range(9000, 10000);
+
+	err = mipi_dsi_dcs_write(ctx->link, MIPI_DCS_SET_GAMMA_CURVE,
+				 (u8[]){ 0x02 }, 1);
+	if (err < 0) {
+		DRM_DEV_ERROR(panel->dev, "failed to set gamma curve: %d\n",
+			      err);
+		goto poweroff;
+	}
+
+	err = mipi_dsi_dcs_set_tear_on(ctx->link,
+				       MIPI_DSI_DCS_TEAR_MODE_VBLANK);
+	if (err < 0) {
+		DRM_DEV_ERROR(panel->dev, "failed to set tear on: %d\n", err);
+		goto poweroff;
+	}
+
+	err = send_mipi_cmds(panel, &lg_sw43408_on_cmds_1[0]);
+
+	if (err < 0) {
+		DRM_DEV_ERROR(panel->dev,
+			      "failed to send DCS Init 1st Code: %d\n", err);
+		goto poweroff;
+	}
+
+	err = mipi_dsi_dcs_exit_sleep_mode(ctx->link);
+	if (err < 0) {
+		DRM_DEV_ERROR(panel->dev, "failed to exit sleep mode: %d\n",
+			      err);
+		goto poweroff;
+	}
+
+	msleep(135);
+
+	err = mipi_dsi_dcs_write(ctx->link, MIPI_DSI_COMPRESSION_MODE,
+				 (u8[]){ 0x11 }, 0);
+	if (err < 0) {
+		DRM_DEV_ERROR(panel->dev,
+			      "failed to set compression mode: %d\n", err);
+		goto poweroff;
+	}
+
+	err = send_mipi_cmds(panel, &lg_sw43408_on_cmds_2[0]);
+
+	if (err < 0) {
+		DRM_DEV_ERROR(panel->dev,
+			      "failed to send DCS Init 2nd Code: %d\n", err);
+		goto poweroff;
+	}
+
+	err = mipi_dsi_dcs_set_display_on(ctx->link);
+	if (err < 0) {
+		DRM_DEV_ERROR(panel->dev, "failed to Set Display ON: %d\n",
+			      err);
+		goto poweroff;
+	}
+
+	msleep(120);
+
+	ctx->prepared = true;
+
+	return 0;
+
+poweroff:
+	gpiod_set_value(ctx->reset_gpio, 1);
+	regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
+	return err;
+}
+
+static int lg_panel_enable(struct drm_panel *panel)
+{
+	struct sw43408_panel *ctx = to_panel_info(panel);
+	struct drm_dsc_picture_parameter_set pps;
+	int ret;
+
+	if (ctx->enabled)
+		return 0;
+
+	ret = backlight_enable(ctx->backlight);
+	if (ret) {
+		DRM_DEV_ERROR(panel->dev, "Failed to enable backlight %d\n",
+			      ret);
+		return ret;
+	}
+
+	if (!panel->dsc) {
+		DRM_DEV_ERROR(panel->dev, "Can't find DSC\n");
+		return -ENODEV;
+	}
+
+	drm_dsc_pps_payload_pack(&pps, panel->dsc);
+
+	ctx->enabled = true;
+
+	return 0;
+}
+
+static int lg_panel_get_modes(struct drm_panel *panel,
+			      struct drm_connector *connector)
+{
+	struct sw43408_panel *ctx = to_panel_info(panel);
+	const struct drm_display_mode *m = ctx->mode;
+	struct drm_display_mode *mode;
+
+	mode = drm_mode_duplicate(connector->dev, m);
+	if (!mode) {
+		DRM_DEV_ERROR(panel->dev, "failed to add mode %ux%u\n",
+			      m->hdisplay, m->vdisplay);
+		return -ENOMEM;
+	}
+
+	connector->display_info.width_mm = m->width_mm;
+	connector->display_info.height_mm = m->height_mm;
+
+	drm_mode_set_name(mode);
+	drm_mode_probed_add(connector, mode);
+
+	return 1;
+}
+
+static int lg_panel_backlight_update_status(struct backlight_device *bl)
+{
+	struct mipi_dsi_device *dsi = bl_get_data(bl);
+	int ret = 0;
+	uint16_t brightness;
+
+	brightness = (uint16_t)backlight_get_brightness(bl);
+	/* Brightness is sent in big-endian */
+	brightness = cpu_to_be16(brightness);
+
+	ret = mipi_dsi_dcs_set_display_brightness(dsi, brightness);
+	return ret;
+}
+
+static int lg_panel_backlight_get_brightness(struct backlight_device *bl)
+{
+	struct mipi_dsi_device *dsi = bl_get_data(bl);
+	int ret = 0;
+	u16 brightness = 0;
+
+	ret = mipi_dsi_dcs_get_display_brightness(dsi, &brightness);
+	if (ret < 0)
+		return ret;
+
+	return brightness & 0xff;
+}
+
+const struct backlight_ops lg_panel_backlight_ops = {
+	.update_status = lg_panel_backlight_update_status,
+	.get_brightness = lg_panel_backlight_get_brightness,
+};
+
+static int lg_panel_backlight_init(struct sw43408_panel *ctx)
+{
+	struct device *dev = &ctx->link->dev;
+	const struct backlight_properties props = {
+		.type = BACKLIGHT_PLATFORM,
+		.brightness = 255,
+		.max_brightness = 255,
+	};
+
+	ctx->backlight = devm_backlight_device_register(dev, dev_name(dev), dev,
+							ctx->link,
+							&lg_panel_backlight_ops,
+							&props);
+
+	if (IS_ERR(ctx->backlight))
+		return dev_err_probe(dev, PTR_ERR(ctx->backlight),
+				     "Failed to create backlight\n");
+
+	return 0;
+}
+
+static const struct drm_panel_funcs panel_funcs = {
+	.disable = lg_panel_disable,
+	.unprepare = lg_panel_unprepare,
+	.prepare = lg_panel_prepare,
+	.enable = lg_panel_enable,
+	.get_modes = lg_panel_get_modes,
+};
+
+static const struct drm_display_mode sw43408_default_mode = {
+	.clock = 152340,
+
+	.hdisplay = 1080,
+	.hsync_start = 1080 + 20,
+	.hsync_end = 1080 + 20 + 32,
+	.htotal = 1080 + 20 + 32 + 20,
+
+	.vdisplay = 2160,
+	.vsync_start = 2160 + 20,
+	.vsync_end = 2160 + 20 + 4,
+	.vtotal = 2160 + 20 + 4 + 20,
+
+	.width_mm = 62,
+	.height_mm = 124,
+
+	.type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED,
+};
+
+static const struct of_device_id panel_of_match[] = {
+	{ .compatible = "lg,sw43408", .data = &sw43408_default_mode },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, panel_of_match);
+
+static int panel_add(struct sw43408_panel *ctx)
+{
+	struct device *dev = &ctx->link->dev;
+	int i, ret;
+
+	for (i = 0; i < ARRAY_SIZE(ctx->supplies); i++)
+		ctx->supplies[i].supply = regulator_names[i];
+
+	ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(ctx->supplies),
+				      ctx->supplies);
+	if (ret < 0)
+		return ret;
+
+	ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
+	if (IS_ERR(ctx->reset_gpio)) {
+		DRM_DEV_ERROR(dev, "cannot get reset gpio %ld\n",
+			      PTR_ERR(ctx->reset_gpio));
+		return PTR_ERR(ctx->reset_gpio);
+	}
+
+	ret = lg_panel_backlight_init(ctx);
+	if (ret < 0)
+		return ret;
+
+	drm_panel_init(&ctx->base, dev, &panel_funcs, DRM_MODE_CONNECTOR_DSI);
+
+	drm_panel_add(&ctx->base);
+	return ret;
+}
+
+static int panel_probe(struct mipi_dsi_device *dsi)
+{
+	struct sw43408_panel *ctx;
+	struct drm_dsc_config *dsc;
+	int err;
+
+	ctx = devm_kzalloc(&dsi->dev, sizeof(*ctx), GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+
+	ctx->mode = of_device_get_match_data(&dsi->dev);
+	dsi->mode_flags = MIPI_DSI_MODE_LPM;
+	dsi->format = MIPI_DSI_FMT_RGB888;
+	dsi->lanes = 4;
+
+	ctx->link = dsi;
+	mipi_dsi_set_drvdata(dsi, ctx);
+
+	err = panel_add(ctx);
+	if (err < 0)
+		return err;
+
+	/* The panel is DSC panel only, set the dsc params */
+	dsc = devm_kzalloc(&dsi->dev, sizeof(*dsc), GFP_KERNEL);
+	if (!dsc)
+		return -ENOMEM;
+
+	dsc->dsc_version_major = 0x1;
+	dsc->dsc_version_minor = 0x1;
+
+	dsc->slice_height = 16;
+	dsc->slice_width = 540;
+	dsc->slice_count = 1;
+	dsc->bits_per_component = 8;
+	dsc->bits_per_pixel = 8;
+	dsc->block_pred_enable = true;
+
+	ctx->base.dsc = dsc;
+
+	return mipi_dsi_attach(dsi);
+}
+
+static int panel_remove(struct mipi_dsi_device *dsi)
+{
+	struct sw43408_panel *ctx = mipi_dsi_get_drvdata(dsi);
+	int err;
+
+	err = lg_panel_unprepare(&ctx->base);
+	if (err < 0)
+		DRM_DEV_ERROR(&dsi->dev, "failed to unprepare panel: %d\n",
+			      err);
+
+	err = lg_panel_disable(&ctx->base);
+	if (err < 0)
+		DRM_DEV_ERROR(&dsi->dev, "failed to disable panel: %d\n", err);
+
+	err = mipi_dsi_detach(dsi);
+	if (err < 0)
+		DRM_DEV_ERROR(&dsi->dev, "failed to detach from DSI host: %d\n",
+			      err);
+
+	if (ctx->base.dev)
+		drm_panel_remove(&ctx->base);
+
+	return 0;
+}
+
+static struct mipi_dsi_driver panel_driver = {
+	.driver = {
+		.name = "panel-lg-sw43408",
+		.of_match_table = panel_of_match,
+	},
+	.probe = panel_probe,
+	.remove = panel_remove,
+};
+module_mipi_dsi_driver(panel_driver);
+
+MODULE_AUTHOR("Sumit Semwal <sumit.semwal@linaro.org>");
+MODULE_DESCRIPTION("LG SW436408 MIPI-DSI LED panel");
+MODULE_LICENSE("GPL");
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/4] arm64: dts: qcom: add sdm845-google-blueline (Pixel 3)
  2022-07-18 21:30 ` [PATCH 2/4] arm64: dts: qcom: add sdm845-google-blueline (Pixel 3) Caleb Connolly
@ 2022-07-18 22:13   ` Dmitry Baryshkov
  2022-07-18 22:54     ` Caleb Connolly
                       ` (2 more replies)
  2022-07-19  8:45   ` Krzysztof Kozlowski
  1 sibling, 3 replies; 18+ messages in thread
From: Dmitry Baryshkov @ 2022-07-18 22:13 UTC (permalink / raw)
  To: Caleb Connolly, Andy Gross, Bjorn Andersson, Konrad Dybcio,
	Rob Herring, Krzysztof Kozlowski, Thierry Reding, Sam Ravnborg,
	David Airlie, Daniel Vetter, Sumit Semwal, linux-arm-msm,
	devicetree, linux-kernel, dri-devel, phone-devel,
	~postmarketos/upstreaming, Kalle Valo
  Cc: Amit Pundir, Vinod Koul

On 19/07/2022 00:30, Caleb Connolly wrote:
> From: Amit Pundir <amit.pundir@linaro.org>
> 
> This adds an initial dts for the Blueline (Pixel 3). Supported
> functionality includes display, Debug UART, UFS, USB-C (peripheral), WiFi,
> Bluetooth and modem.
> 
> Bootloader compatible board and msm IDs are needed for the kernel to boot
> with Pixel3 bootloader, so those are added.
> 
> GPIOs 0 through 3 and 81 through 84 are configured to not be accessible
> from the application CPUs, so we mark them as reserved to allow the Pixel 3
> to boot.
> 
> The reserved-memory locations where obtained from downstream using
> kernel logs:
> https://gist.github.com/calebccff/090d10bfac3cb9e9bd98dda30b054c96
> 
> The rmtfs region is allocated with UIO, making it technically "dynamic".
> It's address and size can be read from sysfs:
> 
> blueline:/ # cat /sys/class/uio/uio0/name
> rmtfs
> at /sys/class/uio/uio0/maps/map0/addr
> 0x00000000f2701000
> blueline:/ # cat /sys/class/uio/uio0/maps/map0/size
> 0x0000000000200000
> 
> Like the OnePlus 6, it needs 1kB reserved on either side of the rmtfs
> memory to workaround some XPU bug which would otherwise cause erroneous
> XPU violations when accessing the rmtfs_mem region.
> 
> For wifi, the pixel 3 reports a board-id of 0xFF, and downstream
> only includes a single bdwlan file. The qcom,ath10k-calibration-variant
> property is set to ensure that the correct calibration data is used.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> [AmitP: Cherry-picked and refactored from Bjorn's db845c dts
>          ("arm64: dts: qcom: Add Dragonboard 845c") https://lkml.org/lkml/2019/6/6/7]
> Signed-off-by: Amit Pundir <amit.pundir@linaro.org>
> [sumits: merged commits to add board and msm ids, gpio range reservation,
>    ufs device-reset gpio and adaptation to v5.5+ changes]
> Signed-off-by: Sumit Semwal <sumit.semwal@linaro.org>
> [vinod: Add display nodes]
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
> [caleb: remove db845c bits, cleanup, add reserved-memory for modem/wifi]
> Signed-off-by: Caleb Connolly <caleb@connolly.tech>

Thanks for your patch, few minor items to improve.

> ---
>   arch/arm64/boot/dts/qcom/Makefile             |   1 +
>   .../boot/dts/qcom/sdm845-google-blueline.dts  | 652 ++++++++++++++++++
>   2 files changed, 653 insertions(+)
>   create mode 100644 arch/arm64/boot/dts/qcom/sdm845-google-blueline.dts
> 
> diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
> index 2f8aec2cc6db..c151e17e6eb7 100644
> --- a/arch/arm64/boot/dts/qcom/Makefile
> +++ b/arch/arm64/boot/dts/qcom/Makefile
> @@ -100,6 +100,7 @@ dtb-$(CONFIG_ARCH_QCOM)	+= sdm845-cheza-r1.dtb
>   dtb-$(CONFIG_ARCH_QCOM)	+= sdm845-cheza-r2.dtb
>   dtb-$(CONFIG_ARCH_QCOM)	+= sdm845-cheza-r3.dtb
>   dtb-$(CONFIG_ARCH_QCOM)	+= sdm845-db845c.dtb
> +dtb-$(CONFIG_ARCH_QCOM)	+= sdm845-google-blueline.dtb
>   dtb-$(CONFIG_ARCH_QCOM)	+= sdm845-mtp.dtb
>   dtb-$(CONFIG_ARCH_QCOM)	+= sdm845-oneplus-enchilada.dtb
>   dtb-$(CONFIG_ARCH_QCOM)	+= sdm845-oneplus-fajita.dtb
> diff --git a/arch/arm64/boot/dts/qcom/sdm845-google-blueline.dts b/arch/arm64/boot/dts/qcom/sdm845-google-blueline.dts
> new file mode 100644
> index 000000000000..dec979ad9209
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sdm845-google-blueline.dts
> @@ -0,0 +1,652 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/dts-v1/;
> +
> +#include <dt-bindings/dma/qcom-gpi.h>
> +#include <dt-bindings/input/linux-event-codes.h>
> +#include <dt-bindings/interrupt-controller/irq.h>
> +#include <dt-bindings/regulator/qcom,rpmh-regulator.h>
> +
> +#include "sdm845.dtsi"
> +#include "pm8998.dtsi"
> +#include "pmi8998.dtsi"
> +
> +/delete-node/ &mpss_region;
> +/delete-node/ &venus_mem;
> +/delete-node/ &cdsp_mem;
> +/delete-node/ &mba_region;
> +/delete-node/ &slpi_mem;
> +/delete-node/ &spss_mem;
> +/delete-node/ &rmtfs_mem;
> +
> +/ {
> +	model = "Google Pixel 3";
> +	compatible = "google,blueline", "qcom,sdm845";
> +	qcom,board-id = <0x00021505 0>;
> +	qcom,msm-id = <321 0x20001>;
> +
> +	aliases {
> +		serial0 = &uart9;
> +		serial1 = &uart6;
> +	};
> +
> +	chosen {
> +		stdout-path = "serial0:115200n8";
> +	};
> +
> +	volume-keys {
> +		compatible = "gpio-keys";
> +		label = "Volume keys";
> +		autorepeat;
> +
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&volume_up_gpio>;
> +
> +		vol-up {
> +			label = "Volume Up";
> +			linux,code = <KEY_VOLUMEUP>;
> +			gpios = <&pm8998_gpio 6 GPIO_ACTIVE_LOW>;
> +			debounce-interval = <15>;
> +		};
> +	};
> +
> +	reserved-memory {
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		ranges;

These properties are already part of the sdm845.dtsi, so no need to have 
them here.

> +
> +		mpss_region: memory@8e000000 {
> +			reg = <0 0x8e000000 0 0x9800000>;
> +			no-map;
> +		};
> +
> +		venus_mem: venus@97800000 {
> +			reg = <0 0x97800000 0 0x500000>;
> +			no-map;
> +		};
> +
> +		cdsp_mem: cdsp-mem@97D00000 {
> +			reg = <0 0x97D00000 0 0x800000>;
> +			no-map;
> +		};
> +
> +		mba_region: mba@98500000 {
> +			reg = <0 0x98500000 0 0x200000>;
> +			no-map;
> +		};
> +
> +		slpi_mem: slpi@98700000 {
> +			reg = <0 0x98700000 0 0x1400000>;
> +			no-map;
> +		};
> +
> +		spss_mem: spss@99B00000 {
> +			reg = <0 0x99B00000 0 0x100000>;
> +			no-map;
> +		};
> +
> +		/* rmtfs lower guard */
> +		memory@f2700000 {
> +			reg = <0 0xf2700000 0 0x1000>;
> +			no-map;
> +		};
> +
> +		rmtfs_mem: memory@f2701000 {
> +			compatible = "qcom,rmtfs-mem";
> +			reg = <0 0xf2701000 0 0x200000>;
> +			no-map;
> +
> +			qcom,client-id = <1>;
> +			qcom,vmid = <15>;
> +		};
> +
> +		/* rmtfs upper guard */
> +		memory@f2901000 {
> +			reg = <0 0xf2901000 0 0x1000>;
> +			no-map;
> +		};
> +	};
> +
> +	vph_pwr: vph-pwr-regulator {
> +		compatible = "regulator-fixed";
> +		regulator-name = "vph_pwr";
> +		regulator-min-microvolt = <3700000>;
> +		regulator-max-microvolt = <3700000>;
> +	};
> +
> +	vreg_s4a_1p8: vreg-s4a-1p8 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "vreg_s4a_1p8";
> +
> +		regulator-min-microvolt = <1800000>;
> +		regulator-max-microvolt = <1800000>;
> +		regulator-always-on;
> +		regulator-boot-on;
> +
> +		vin-supply = <&vph_pwr>;
> +	};
> +};
> +
> +&adsp_pas {
> +	status = "okay";
> +
> +	firmware-name = "qcom/sdm845/pixel3/adsp.mbn";

What about using "qcom/sdm845/blueline/adsp.mbn" instead?

Bjorn, Amit?

> +};
> +
> +&apps_rsc {
> +	pm8998-rpmh-regulators {
> +		compatible = "qcom,pm8998-rpmh-regulators";
> +		qcom,pmic-id = "a";
> +		vdd-s1-supply = <&vph_pwr>;
> +		vdd-s2-supply = <&vph_pwr>;
> +		vdd-s3-supply = <&vph_pwr>;
> +		vdd-s4-supply = <&vph_pwr>;
> +		vdd-s5-supply = <&vph_pwr>;
> +		vdd-s6-supply = <&vph_pwr>;
> +		vdd-s7-supply = <&vph_pwr>;
> +		vdd-s8-supply = <&vph_pwr>;
> +		vdd-s9-supply = <&vph_pwr>;
> +		vdd-s10-supply = <&vph_pwr>;
> +		vdd-s11-supply = <&vph_pwr>;
> +		vdd-s12-supply = <&vph_pwr>;
> +		vdd-s13-supply = <&vph_pwr>;
> +		vdd-l1-l27-supply = <&vreg_s7a_1p025>;
> +		vdd-l2-l8-l17-supply = <&vreg_s3a_1p35>;
> +		vdd-l3-l11-supply = <&vreg_s7a_1p025>;
> +		vdd-l4-l5-supply = <&vreg_s7a_1p025>;
> +		vdd-l6-supply = <&vph_pwr>;
> +		vdd-l7-l12-l14-l15-supply = <&vreg_s5a_2p04>;
> +		vdd-l9-supply = <&vreg_bob>;
> +		vdd-l10-l23-l25-supply = <&vreg_bob>;
> +		vdd-l13-l19-l21-supply = <&vreg_bob>;
> +		vdd-l16-l28-supply = <&vreg_bob>;
> +		vdd-l18-l22-supply = <&vreg_bob>;
> +		vdd-l20-l24-supply = <&vreg_bob>;
> +		vdd-l26-supply = <&vreg_s3a_1p35>;
> +		vin-lvs-1-2-supply = <&vreg_s4a_1p8>;
> +
> +		vreg_s3a_1p35: smps3 {
> +			regulator-min-microvolt = <1352000>;
> +			regulator-max-microvolt = <1352000>;
> +		};
> +
> +		vreg_s5a_2p04: smps5 {
> +			regulator-min-microvolt = <1904000>;
> +			regulator-max-microvolt = <2040000>;
> +		};
> +
> +		vreg_s7a_1p025: smps7 {
> +			regulator-min-microvolt = <900000>;
> +			regulator-max-microvolt = <1028000>;
> +		};
> +
> +		vdda_mipi_dsi0_pll:

Do we need this alias?

> +		vreg_l1a_0p875: ldo1 {
> +			regulator-min-microvolt = <880000>;
> +			regulator-max-microvolt = <880000>;
> +			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> +			regulator-boot-on;
> +		};
> +
> +		vreg_l5a_0p8: ldo5 {
> +			regulator-min-microvolt = <800000>;
> +			regulator-max-microvolt = <800000>;
> +			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_l7a_1p8: ldo7 {
> +			regulator-min-microvolt = <1800000>;
> +			regulator-max-microvolt = <1800000>;
> +			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> +		};
> +
> +		vreg_l13a_2p95: ldo13 {
> +			regulator-min-microvolt = <1800000>;
> +			regulator-max-microvolt = <2960000>;
> +			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> +		};
> +
> +		vreg_l14a_1p88: ldo14 {
> +			regulator-min-microvolt = <1800000>;
> +			regulator-max-microvolt = <1800000>;
> +			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> +			regulator-boot-on;
> +			/*
> +			 * We can't properly bring the panel back if it gets turned off
> +			 * so keep it's regulators always on for now.
> +			 */

Any idea, what is the issue here? Do you have the datasheet for the panel?

> +			regulator-always-on;
> +		};
> +
> +		vreg_l17a_1p3: ldo17 {
> +			regulator-min-microvolt = <1304000>;
> +			regulator-max-microvolt = <1304000>;
> +			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> +		};
> +
> +		vreg_l19a_3p3: ldo19 {
> +			regulator-min-microvolt = <3300000>;
> +			regulator-max-microvolt = <3312000>;
> +			qcom,init-voltage = <3300000>;
> +			qcom,initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> +			/*
> +			 * The touchscreen needs this to be 3.3v, which is apparently
> +			 * quite close to the hardware limit for this LDO (3.312v)
> +			 * It must be kept in high power mode to prevent TS brownouts
> +			 */
> +			regulator-allowed-modes = <RPMH_REGULATOR_MODE_HPM>;
> +		};
> +
> +		vreg_l20a_2p95: ldo20 {
> +			regulator-min-microvolt = <2960000>;
> +			regulator-max-microvolt = <2968000>;
> +			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> +		};
> +
> +		vreg_l21a_2p95: ldo21 {
> +			regulator-min-microvolt = <2960000>;
> +			regulator-max-microvolt = <2968000>;
> +			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> +		};
> +
> +		vreg_l24a_3p075: ldo24 {
> +			regulator-min-microvolt = <3088000>;
> +			regulator-max-microvolt = <3088000>;
> +			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> +		};
> +
> +		vreg_l25a_3p3: ldo25 {
> +			regulator-min-microvolt = <3300000>;
> +			regulator-max-microvolt = <3312000>;
> +			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> +		};
> +
> +		vdda_mipi_dsi0_1p2:

Do we need this alias?

> +		vreg_l26a_1p2: ldo26 {
> +			regulator-min-microvolt = <1200000>;
> +			regulator-max-microvolt = <1200000>;
> +			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> +			regulator-boot-on;
> +		};
> +
> +		vreg_l28a_3p0: ldo28 {
> +			regulator-min-microvolt = <2856000>;
> +			regulator-max-microvolt = <3008000>;
> +			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> +			regulator-boot-on;
> +			/*
> +			 * We can't properly bring the panel back if it gets turned off
> +			 * so keep it's regulators always on for now.
> +			 */
> +			regulator-always-on;
> +		};
> +	};
> +
> +	pmi8998-rpmh-regulators {
> +		compatible = "qcom,pmi8998-rpmh-regulators";
> +		qcom,pmic-id = "b";
> +
> +		vdd-bob-supply = <&vph_pwr>;
> +
> +		vreg_bob: bob {
> +			regulator-min-microvolt = <3312000>;
> +			regulator-max-microvolt = <3600000>;
> +			regulator-initial-mode = <RPMH_REGULATOR_MODE_AUTO>;
> +			regulator-allow-bypass;
> +		};
> +	};
> +
> +	pm8005-rpmh-regulators {
> +		compatible = "qcom,pm8005-rpmh-regulators";
> +		qcom,pmic-id = "c";
> +
> +		vdd-s1-supply = <&vph_pwr>;
> +		vdd-s2-supply = <&vph_pwr>;
> +		vdd-s3-supply = <&vph_pwr>;
> +		vdd-s4-supply = <&vph_pwr>;
> +
> +		vreg_s3c_0p6: smps3 {
> +			regulator-min-microvolt = <600000>;
> +			regulator-max-microvolt = <600000>;
> +		};
> +	};
> +};
> +
> +&cdsp_pas {
> +	status = "okay";
> +	firmware-name = "qcom/sdm845/pixel3/cdsp.mbn";
> +};
> +
> +&dsi0 {
> +	status = "okay";
> +	vdda-supply = <&vdda_mipi_dsi0_1p2>;
> +
> +	ports {
> +		port@1 {
> +			endpoint {
> +				remote-endpoint = <&lg_sw43408_in_0>;
> +				data-lanes = <0 1 2 3>;
> +			};
> +		};
> +	};
> +
> +	panel@0 {
> +		compatible = "lg,sw43408";
> +		reg = <0>;
> +
> +		vddi-supply = <&vreg_l14a_1p88>;
> +		vpnl-supply = <&vreg_l28a_3p0>;
> +
> +		reset-gpios = <&tlmm 6 GPIO_ACTIVE_LOW>;
> +
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&panel_reset_pins &panel_te_pin &panel_pmgpio_pins>;
> +
> +		ports {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			port@0 {
> +				reg = <0>;
> +				lg_sw43408_in_0: endpoint {
> +					remote-endpoint = <&dsi0_out>;
> +				};
> +			};
> +		};
> +	};
> +};
> +
> +&dsi0_out {
> +	remote-endpoint = <&lg_sw43408_in_0>;
> +	data-lanes = <0 1 2 3>;
> +};
> +
> +&dsi0_phy {
> +	status = "okay";
> +	vdds-supply = <&vdda_mipi_dsi0_pll>;
> +};
> +
> +&gcc {
> +	protected-clocks = <GCC_QSPI_CORE_CLK>,
> +			   <GCC_QSPI_CORE_CLK_SRC>,
> +			   <GCC_QSPI_CNOC_PERIPH_AHB_CLK>;
> +};
> +
> +&gmu {
> +	status = "okay";
> +};
> +
> +&gpi_dma0 {
> +	status = "okay";
> +};
> +
> +&gpu {
> +	status = "okay";
> +
> +	zap-shader {
> +		memory-region = <&gpu_mem>;
> +		firmware-name = "qcom/sdm845/pixel3/a630_zap.mbn";
> +	};
> +};
> +
> +&ipa {
> +	status = "okay";
> +
> +	firmware-name = "qcom/sdm845/pixel3/ipa_fws.mbn";
> +};
> +
> +&mss_pil {
> +	status = "okay";
> +	firmware-name = "qcom/sdm845/pixel3/mba.mbn", "qcom/sdm845/pixel3/modem.mbn";
> +};
> +
> +&mdss {
> +	status = "okay";
> +};
> +
> +&mdss_mdp {
> +	status = "okay";
> +};

Not necessary, it is a default state since the commit 4a5622c1d975 
("arm64: dts: qcom: sdm845: Don't disable MDP explicitly")

> +
> +&pm8998_gpio {
> +	volume_up_gpio: vol-up-active {
> +		pins = "gpio6";
> +		function = "normal";
> +		input-enable;
> +		bias-pull-up;
> +		qcom,drive-strength = <0>;
> +	};
> +
> +	panel_pmgpio_pins: panel-pmgpio-active {
> +		pins = "gpio2", "gpio5";
> +		function = "normal";
> +		input-enable;
> +		bias-disable;
> +		power-source = <0>;
> +	};
> +};
> +
> +&pm8998_pon {
> +	resin {
> +		compatible = "qcom,pm8941-resin";
> +		interrupts = <0x0 0x8 1 IRQ_TYPE_EDGE_BOTH>;
> +		debounce = <15625>;
> +		bias-pull-up;
> +		linux,code = <KEY_VOLUMEDOWN>;
> +	};

Please move the (disabled, labelled) resin device to pm8998.dtsi and 
just add status = "okay" here.

> +};
> +
> +&qupv3_id_0 {
> +	status = "okay";
> +};
> +
> +&qupv3_id_1 {
> +	status = "okay";
> +};
> +
> +&qup_uart6_default {
> +	pinmux {
> +		pins = "gpio45", "gpio46", "gpio47", "gpio48";
> +		function = "qup6";
> +	};
> +
> +	cts {
> +		pins = "gpio45";
> +		bias-disable;
> +	};
> +
> +	rts-tx {
> +		pins = "gpio46", "gpio47";
> +		drive-strength = <2>;
> +		bias-disable;
> +	};
> +
> +	rx {
> +		pins = "gpio48";
> +		bias-pull-up;
> +	};
> +};
> +
> +&qup_uart9_default {
> +	pinconf-tx {
> +		pins = "gpio4";
> +		drive-strength = <2>;
> +		bias-disable;
> +	};
> +
> +	pinconf-rx {
> +		pins = "gpio5";
> +		drive-strength = <2>;
> +		bias-pull-up;
> +	};
> +};
> +
> +&tlmm {
> +	gpio-reserved-ranges = <0 4>, <81 4>;
> +
> +	panel_te_pin: panel-te {
> +		mux {
> +			pins = "gpio12";
> +			function = "mdp_vsync";
> +			drive-strength = <2>;
> +			bias-disable;
> +			input-enable;
> +		};
> +	};
> +
> +	panel_reset_pins: panel-active {
> +		mux {
> +			pins = "gpio6", "gpio52";
> +			function = "gpio";
> +			drive-strength = <8>;
> +			bias-disable = <0>;
> +		};
> +	};
> +
> +	panel_suspend: panel-suspend {
> +		mux {
> +			pins = "gpio6", "gpio52";
> +			function = "gpio";
> +			drive-strength = <2>;
> +			bias-pull-down;
> +		};
> +	};
> +
> +	touchscreen_reset: ts-reset {
> +		mux {
> +			pins = "gpio99";
> +			function = "gpio";
> +			drive-strength = <8>;
> +			bias-pull-up;
> +			//output-high;

debug, can be removed?

> +		};
> +	};
> +
> +	touchscreen_pins: ts-pins {
> +		mux {
> +			pins = "gpio125";
> +			function = "gpio";
> +			drive-strength = <2>;
> +			bias-disable;
> +		};
> +	};
> +
> +	touchscreen_i2c_pins: qup-i2c2-gpio {
> +		pins = "gpio27", "gpio28";
> +		function = "gpio";
> +
> +		drive-strength = <2>;
> +		bias-disable;
> +	};
> +};
> +
> +&uart6 {
> +	status = "okay";
> +
> +	bluetooth {
> +		compatible = "qcom,wcn3990-bt";
> +
> +		vddio-supply = <&vreg_s4a_1p8>;
> +		vddxo-supply = <&vreg_l7a_1p8>;
> +		vddrf-supply = <&vreg_l17a_1p3>;
> +		vddch0-supply = <&vreg_l25a_3p3>;
> +		max-speed = <3200000>;
> +	};
> +};
> +
> +&uart9 {
> +	status = "okay";
> +};
> +
> +&usb_1 {
> +	status = "okay";
> +};
> +
> +&usb_1_dwc3 {
> +	dr_mode = "peripheral";
> +};
> +
> +&usb_1_hsphy {
> +	status = "okay";
> +
> +	vdd-supply = <&vreg_l1a_0p875>;
> +	vdda-pll-supply = <&vreg_l12a_1p8>;
> +	vdda-phy-dpdm-supply = <&vreg_l24a_3p075>;
> +
> +	qcom,imp-res-offset-value = <8>;
> +	qcom,hstx-trim-value = <QUSB2_V2_HSTX_TRIM_21_6_MA>;
> +	qcom,preemphasis-level = <QUSB2_V2_PREEMPHASIS_5_PERCENT>;
> +	qcom,preemphasis-width = <QUSB2_V2_PREEMPHASIS_WIDTH_HALF_BIT>;
> +};
> +
> +&usb_1_qmpphy {
> +	status = "okay";
> +
> +	vdda-phy-supply = <&vreg_l26a_1p2>;
> +	vdda-pll-supply = <&vreg_l1a_0p875>;
> +};
> +
> +&usb_2 {
> +	status = "okay";
> +};
> +
> +&usb_2_dwc3 {
> +	dr_mode = "host";
> +};
> +
> +&usb_2_hsphy {
> +	status = "okay";
> +
> +	vdd-supply = <&vreg_l1a_0p875>;
> +	vdda-pll-supply = <&vreg_l12a_1p8>;
> +	vdda-phy-dpdm-supply = <&vreg_l24a_3p075>;
> +
> +	qcom,imp-res-offset-value = <8>;
> +	qcom,hstx-trim-value = <QUSB2_V2_HSTX_TRIM_22_8_MA>;
> +};
> +
> +&usb_2_qmpphy {
> +	status = "okay";
> +
> +	vdda-phy-supply = <&vreg_l26a_1p2>;
> +	vdda-pll-supply = <&vreg_l1a_0p875>;
> +};
> +
> +&ufs_mem_hc {
> +	status = "okay";
> +
> +	reset-gpios = <&tlmm 150 GPIO_ACTIVE_LOW>;
> +
> +	vcc-supply = <&vreg_l20a_2p95>;
> +	vcc-max-microamp = <800000>;
> +};
> +
> +&ufs_mem_phy {
> +	status = "okay";
> +
> +	vdda-phy-supply = <&vreg_l1a_0p875>;
> +	vdda-pll-supply = <&vreg_l26a_1p2>;
> +};
> +
> +&venus {
> +	status = "okay";
> +	firmware-name = "qcom/sdm845/oneplus6/venus.mbn";

Why are you using the oneplus6 here?

> +};
> +
> +&wifi {
> +	status = "okay";
> +
> +	vdd-0.8-cx-mx-supply = <&vreg_l5a_0p8>;
> +	vdd-1.8-xo-supply = <&vreg_l7a_1p8>;
> +	vdd-1.3-rfa-supply = <&vreg_l17a_1p3>;
> +	vdd-3.3-ch0-supply = <&vreg_l25a_3p3>;
> +
> +	qcom,snoc-host-cap-8bit-quirk;
> +	qcom,ath10k-calibration-variant = "google_blueline";

Ideally Kalle Valo should bless this string, added him to the Cc list. 
Could you please submit the board file to the ath10k (see [1] for the 
description and [2] for an example).


[1] https://wireless.wiki.kernel.org/en/users/drivers/ath10k/boardfiles
[2] 
https://lore.kernel.org/ath10k/CAA8EJpphUrxr5gtW0=-tZh-DrKXmHkfFxWMvYRpTUGuCesGCbw@mail.gmail.com/T/#u

> +};


-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/4] arm64: dts: qcom: add sdm845-google-blueline (Pixel 3)
  2022-07-18 22:13   ` Dmitry Baryshkov
@ 2022-07-18 22:54     ` Caleb Connolly
  2022-07-18 23:15       ` Dmitry Baryshkov
  2022-07-27  8:47     ` Kalle Valo
  2022-07-27 10:55     ` Amit Pundir
  2 siblings, 1 reply; 18+ messages in thread
From: Caleb Connolly @ 2022-07-18 22:54 UTC (permalink / raw)
  To: Dmitry Baryshkov, Andy Gross, Bjorn Andersson, Konrad Dybcio,
	Rob Herring, Krzysztof Kozlowski, Thierry Reding, Sam Ravnborg,
	David Airlie, Daniel Vetter, Sumit Semwal, linux-arm-msm,
	devicetree, linux-kernel, dri-devel, phone-devel,
	~postmarketos/upstreaming, Kalle Valo
  Cc: Amit Pundir, Vinod Koul



On 18/07/2022 23:13, Dmitry Baryshkov wrote:
> On 19/07/2022 00:30, Caleb Connolly wrote:
>> From: Amit Pundir <amit.pundir@linaro.org>
>>
>> This adds an initial dts for the Blueline (Pixel 3). Supported
>> functionality includes display, Debug UART, UFS, USB-C (peripheral), WiFi,
>> Bluetooth and modem.
>>
>> Bootloader compatible board and msm IDs are needed for the kernel to boot
>> with Pixel3 bootloader, so those are added.
>>
>> GPIOs 0 through 3 and 81 through 84 are configured to not be accessible
>> from the application CPUs, so we mark them as reserved to allow the Pixel 3
>> to boot.
>>
>> The reserved-memory locations where obtained from downstream using
>> kernel logs:
>> https://gist.github.com/calebccff/090d10bfac3cb9e9bd98dda30b054c96
>>
>> The rmtfs region is allocated with UIO, making it technically "dynamic".
>> It's address and size can be read from sysfs:
>>
>> blueline:/ # cat /sys/class/uio/uio0/name
>> rmtfs
>> at /sys/class/uio/uio0/maps/map0/addr
>> 0x00000000f2701000
>> blueline:/ # cat /sys/class/uio/uio0/maps/map0/size
>> 0x0000000000200000
>>
>> Like the OnePlus 6, it needs 1kB reserved on either side of the rmtfs
>> memory to workaround some XPU bug which would otherwise cause erroneous
>> XPU violations when accessing the rmtfs_mem region.
>>
>> For wifi, the pixel 3 reports a board-id of 0xFF, and downstream
>> only includes a single bdwlan file. The qcom,ath10k-calibration-variant
>> property is set to ensure that the correct calibration data is used.
>>
>> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
>> [AmitP: Cherry-picked and refactored from Bjorn's db845c dts
>>           ("arm64: dts: qcom: Add Dragonboard 845c") https://lkml.org/lkml/2019/6/6/7]
>> Signed-off-by: Amit Pundir <amit.pundir@linaro.org>
>> [sumits: merged commits to add board and msm ids, gpio range reservation,
>>     ufs device-reset gpio and adaptation to v5.5+ changes]
>> Signed-off-by: Sumit Semwal <sumit.semwal@linaro.org>
>> [vinod: Add display nodes]
>> Signed-off-by: Vinod Koul <vkoul@kernel.org>
>> [caleb: remove db845c bits, cleanup, add reserved-memory for modem/wifi]
>> Signed-off-by: Caleb Connolly <caleb@connolly.tech>
>
> Thanks for your patch, few minor items to improve.
>
>> ---
>>    arch/arm64/boot/dts/qcom/Makefile             |   1 +
>>    .../boot/dts/qcom/sdm845-google-blueline.dts  | 652 ++++++++++++++++++
>>    2 files changed, 653 insertions(+)
>>    create mode 100644 arch/arm64/boot/dts/qcom/sdm845-google-blueline.dts
>>
>> diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
>> index 2f8aec2cc6db..c151e17e6eb7 100644
>> --- a/arch/arm64/boot/dts/qcom/Makefile
>> +++ b/arch/arm64/boot/dts/qcom/Makefile
>> @@ -100,6 +100,7 @@ dtb-$(CONFIG_ARCH_QCOM)	+= sdm845-cheza-r1.dtb
>>    dtb-$(CONFIG_ARCH_QCOM)	+= sdm845-cheza-r2.dtb
>>    dtb-$(CONFIG_ARCH_QCOM)	+= sdm845-cheza-r3.dtb
>>    dtb-$(CONFIG_ARCH_QCOM)	+= sdm845-db845c.dtb
>> +dtb-$(CONFIG_ARCH_QCOM)	+= sdm845-google-blueline.dtb
>>    dtb-$(CONFIG_ARCH_QCOM)	+= sdm845-mtp.dtb
>>    dtb-$(CONFIG_ARCH_QCOM)	+= sdm845-oneplus-enchilada.dtb
>>    dtb-$(CONFIG_ARCH_QCOM)	+= sdm845-oneplus-fajita.dtb
>> diff --git a/arch/arm64/boot/dts/qcom/sdm845-google-blueline.dts b/arch/arm64/boot/dts/qcom/sdm845-google-blueline.dts
>> new file mode 100644
>> index 000000000000..dec979ad9209
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/qcom/sdm845-google-blueline.dts
>> @@ -0,0 +1,652 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +/dts-v1/;
>> +
>> +#include <dt-bindings/dma/qcom-gpi.h>
>> +#include <dt-bindings/input/linux-event-codes.h>
>> +#include <dt-bindings/interrupt-controller/irq.h>
>> +#include <dt-bindings/regulator/qcom,rpmh-regulator.h>
>> +
>> +#include "sdm845.dtsi"
>> +#include "pm8998.dtsi"
>> +#include "pmi8998.dtsi"
>> +
>> +/delete-node/ &mpss_region;
>> +/delete-node/ &venus_mem;
>> +/delete-node/ &cdsp_mem;
>> +/delete-node/ &mba_region;
>> +/delete-node/ &slpi_mem;
>> +/delete-node/ &spss_mem;
>> +/delete-node/ &rmtfs_mem;
>> +
>> +/ {
>> +	model = "Google Pixel 3";
>> +	compatible = "google,blueline", "qcom,sdm845";
>> +	qcom,board-id = <0x00021505 0>;
>> +	qcom,msm-id = <321 0x20001>;
>> +
>> +	aliases {
>> +		serial0 = &uart9;
>> +		serial1 = &uart6;
>> +	};
>> +
>> +	chosen {
>> +		stdout-path = "serial0:115200n8";
>> +	};
>> +
>> +	volume-keys {
>> +		compatible = "gpio-keys";
>> +		label = "Volume keys";
>> +		autorepeat;
>> +
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&volume_up_gpio>;
>> +
>> +		vol-up {
>> +			label = "Volume Up";
>> +			linux,code = <KEY_VOLUMEUP>;
>> +			gpios = <&pm8998_gpio 6 GPIO_ACTIVE_LOW>;
>> +			debounce-interval = <15>;
>> +		};
>> +	};
>> +
>> +	reserved-memory {
>> +		#address-cells = <2>;
>> +		#size-cells = <2>;
>> +		ranges;
>
> These properties are already part of the sdm845.dtsi, so no need to have
> them here.
>
>> +
>> +		mpss_region: memory@8e000000 {
>> +			reg = <0 0x8e000000 0 0x9800000>;
>> +			no-map;
>> +		};
>> +
>> +		venus_mem: venus@97800000 {
>> +			reg = <0 0x97800000 0 0x500000>;
>> +			no-map;
>> +		};
>> +
>> +		cdsp_mem: cdsp-mem@97D00000 {
>> +			reg = <0 0x97D00000 0 0x800000>;
>> +			no-map;
>> +		};
>> +
>> +		mba_region: mba@98500000 {
>> +			reg = <0 0x98500000 0 0x200000>;
>> +			no-map;
>> +		};
>> +
>> +		slpi_mem: slpi@98700000 {
>> +			reg = <0 0x98700000 0 0x1400000>;
>> +			no-map;
>> +		};
>> +
>> +		spss_mem: spss@99B00000 {
>> +			reg = <0 0x99B00000 0 0x100000>;
>> +			no-map;
>> +		};
>> +
>> +		/* rmtfs lower guard */
>> +		memory@f2700000 {
>> +			reg = <0 0xf2700000 0 0x1000>;
>> +			no-map;
>> +		};
>> +
>> +		rmtfs_mem: memory@f2701000 {
>> +			compatible = "qcom,rmtfs-mem";
>> +			reg = <0 0xf2701000 0 0x200000>;
>> +			no-map;
>> +
>> +			qcom,client-id = <1>;
>> +			qcom,vmid = <15>;
>> +		};
>> +
>> +		/* rmtfs upper guard */
>> +		memory@f2901000 {
>> +			reg = <0 0xf2901000 0 0x1000>;
>> +			no-map;
>> +		};
>> +	};
>> +
>> +	vph_pwr: vph-pwr-regulator {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "vph_pwr";
>> +		regulator-min-microvolt = <3700000>;
>> +		regulator-max-microvolt = <3700000>;
>> +	};
>> +
>> +	vreg_s4a_1p8: vreg-s4a-1p8 {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "vreg_s4a_1p8";
>> +
>> +		regulator-min-microvolt = <1800000>;
>> +		regulator-max-microvolt = <1800000>;
>> +		regulator-always-on;
>> +		regulator-boot-on;
>> +
>> +		vin-supply = <&vph_pwr>;
>> +	};
>> +};
>> +
>> +&adsp_pas {
>> +	status = "okay";
>> +
>> +	firmware-name = "qcom/sdm845/pixel3/adsp.mbn";
>
> What about using "qcom/sdm845/blueline/adsp.mbn" instead?
>
> Bjorn, Amit?
I haven't verified it, but I'd assume that the firmware for bits like
the remoteprocs are the same between the pixel 3 and 3 XL, hence using a
common name (same as the approach taken on the OnePlus 6/6T)
>
>> +};
>> +
>> +&apps_rsc {
>> +	pm8998-rpmh-regulators {
>> +		compatible = "qcom,pm8998-rpmh-regulators";
>> +		qcom,pmic-id = "a";
>> +		vdd-s1-supply = <&vph_pwr>;
>> +		vdd-s2-supply = <&vph_pwr>;
>> +		vdd-s3-supply = <&vph_pwr>;
>> +		vdd-s4-supply = <&vph_pwr>;
>> +		vdd-s5-supply = <&vph_pwr>;
>> +		vdd-s6-supply = <&vph_pwr>;
>> +		vdd-s7-supply = <&vph_pwr>;
>> +		vdd-s8-supply = <&vph_pwr>;
>> +		vdd-s9-supply = <&vph_pwr>;
>> +		vdd-s10-supply = <&vph_pwr>;
>> +		vdd-s11-supply = <&vph_pwr>;
>> +		vdd-s12-supply = <&vph_pwr>;
>> +		vdd-s13-supply = <&vph_pwr>;
>> +		vdd-l1-l27-supply = <&vreg_s7a_1p025>;
>> +		vdd-l2-l8-l17-supply = <&vreg_s3a_1p35>;
>> +		vdd-l3-l11-supply = <&vreg_s7a_1p025>;
>> +		vdd-l4-l5-supply = <&vreg_s7a_1p025>;
>> +		vdd-l6-supply = <&vph_pwr>;
>> +		vdd-l7-l12-l14-l15-supply = <&vreg_s5a_2p04>;
>> +		vdd-l9-supply = <&vreg_bob>;
>> +		vdd-l10-l23-l25-supply = <&vreg_bob>;
>> +		vdd-l13-l19-l21-supply = <&vreg_bob>;
>> +		vdd-l16-l28-supply = <&vreg_bob>;
>> +		vdd-l18-l22-supply = <&vreg_bob>;
>> +		vdd-l20-l24-supply = <&vreg_bob>;
>> +		vdd-l26-supply = <&vreg_s3a_1p35>;
>> +		vin-lvs-1-2-supply = <&vreg_s4a_1p8>;
>> +
>> +		vreg_s3a_1p35: smps3 {
>> +			regulator-min-microvolt = <1352000>;
>> +			regulator-max-microvolt = <1352000>;
>> +		};
>> +
>> +		vreg_s5a_2p04: smps5 {
>> +			regulator-min-microvolt = <1904000>;
>> +			regulator-max-microvolt = <2040000>;
>> +		};
>> +
>> +		vreg_s7a_1p025: smps7 {
>> +			regulator-min-microvolt = <900000>;
>> +			regulator-max-microvolt = <1028000>;
>> +		};
>> +
>> +		vdda_mipi_dsi0_pll:
>
> Do we need this alias?
No, will drop it.
>
>> +		vreg_l1a_0p875: ldo1 {
>> +			regulator-min-microvolt = <880000>;
>> +			regulator-max-microvolt = <880000>;
>> +			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
>> +			regulator-boot-on;
>> +		};
>> +
>> +		vreg_l5a_0p8: ldo5 {
>> +			regulator-min-microvolt = <800000>;
>> +			regulator-max-microvolt = <800000>;
>> +			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_l7a_1p8: ldo7 {
>> +			regulator-min-microvolt = <1800000>;
>> +			regulator-max-microvolt = <1800000>;
>> +			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
>> +		};
>> +
>> +		vreg_l13a_2p95: ldo13 {
>> +			regulator-min-microvolt = <1800000>;
>> +			regulator-max-microvolt = <2960000>;
>> +			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
>> +		};
>> +
>> +		vreg_l14a_1p88: ldo14 {
>> +			regulator-min-microvolt = <1800000>;
>> +			regulator-max-microvolt = <1800000>;
>> +			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
>> +			regulator-boot-on;
>> +			/*
>> +			 * We can't properly bring the panel back if it gets turned off
>> +			 * so keep it's regulators always on for now.
>> +			 */
>
> Any idea, what is the issue here? Do you have the datasheet for the panel?
As Marijn just mentioned on IRC, the driver right now only calls
drm_dsc_pps_payload_pack() but doesn't actually send it. I haven't
investigated this myself but I'll look into it.

Unfortunately no datasheet afaik, they tend to be pretty elusive. The
things I would do for a phone panel datasheet...
>
>> +			regulator-always-on;
>> +		};
>> +
>> +		vreg_l17a_1p3: ldo17 {
>> +			regulator-min-microvolt = <1304000>;
>> +			regulator-max-microvolt = <1304000>;
>> +			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
>> +		};
>> +
>> +		vreg_l19a_3p3: ldo19 {
>> +			regulator-min-microvolt = <3300000>;
>> +			regulator-max-microvolt = <3312000>;
>> +			qcom,init-voltage = <3300000>;
>> +			qcom,initial-mode = <RPMH_REGULATOR_MODE_HPM>;
>> +			/*
>> +			 * The touchscreen needs this to be 3.3v, which is apparently
>> +			 * quite close to the hardware limit for this LDO (3.312v)
>> +			 * It must be kept in high power mode to prevent TS brownouts
>> +			 */
>> +			regulator-allowed-modes = <RPMH_REGULATOR_MODE_HPM>;
>> +		};
>> +
>> +		vreg_l20a_2p95: ldo20 {
>> +			regulator-min-microvolt = <2960000>;
>> +			regulator-max-microvolt = <2968000>;
>> +			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
>> +		};
>> +
>> +		vreg_l21a_2p95: ldo21 {
>> +			regulator-min-microvolt = <2960000>;
>> +			regulator-max-microvolt = <2968000>;
>> +			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
>> +		};
>> +
>> +		vreg_l24a_3p075: ldo24 {
>> +			regulator-min-microvolt = <3088000>;
>> +			regulator-max-microvolt = <3088000>;
>> +			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
>> +		};
>> +
>> +		vreg_l25a_3p3: ldo25 {
>> +			regulator-min-microvolt = <3300000>;
>> +			regulator-max-microvolt = <3312000>;
>> +			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
>> +		};
>> +
>> +		vdda_mipi_dsi0_1p2:
>
> Do we need this alias?
Ack
>
>> +		vreg_l26a_1p2: ldo26 {
>> +			regulator-min-microvolt = <1200000>;
>> +			regulator-max-microvolt = <1200000>;
>> +			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
>> +			regulator-boot-on;
>> +		};
>> +
>> +		vreg_l28a_3p0: ldo28 {
>> +			regulator-min-microvolt = <2856000>;
>> +			regulator-max-microvolt = <3008000>;
>> +			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
>> +			regulator-boot-on;
>> +			/*
>> +			 * We can't properly bring the panel back if it gets turned off
>> +			 * so keep it's regulators always on for now.
>> +			 */
>> +			regulator-always-on;
>> +		};
>> +	};
>> +
>> +	pmi8998-rpmh-regulators {
>> +		compatible = "qcom,pmi8998-rpmh-regulators";
>> +		qcom,pmic-id = "b";
>> +
>> +		vdd-bob-supply = <&vph_pwr>;
>> +
>> +		vreg_bob: bob {
>> +			regulator-min-microvolt = <3312000>;
>> +			regulator-max-microvolt = <3600000>;
>> +			regulator-initial-mode = <RPMH_REGULATOR_MODE_AUTO>;
>> +			regulator-allow-bypass;
>> +		};
>> +	};
>> +
>> +	pm8005-rpmh-regulators {
>> +		compatible = "qcom,pm8005-rpmh-regulators";
>> +		qcom,pmic-id = "c";
>> +
>> +		vdd-s1-supply = <&vph_pwr>;
>> +		vdd-s2-supply = <&vph_pwr>;
>> +		vdd-s3-supply = <&vph_pwr>;
>> +		vdd-s4-supply = <&vph_pwr>;
>> +
>> +		vreg_s3c_0p6: smps3 {
>> +			regulator-min-microvolt = <600000>;
>> +			regulator-max-microvolt = <600000>;
>> +		};
>> +	};
>> +};
>> +
>> +&cdsp_pas {
>> +	status = "okay";
>> +	firmware-name = "qcom/sdm845/pixel3/cdsp.mbn";
>> +};
>> +
>> +&dsi0 {
>> +	status = "okay";
>> +	vdda-supply = <&vdda_mipi_dsi0_1p2>;
>> +
>> +	ports {
>> +		port@1 {
>> +			endpoint {
>> +				remote-endpoint = <&lg_sw43408_in_0>;
>> +				data-lanes = <0 1 2 3>;
>> +			};
>> +		};
>> +	};
>> +
>> +	panel@0 {
>> +		compatible = "lg,sw43408";
>> +		reg = <0>;
>> +
>> +		vddi-supply = <&vreg_l14a_1p88>;
>> +		vpnl-supply = <&vreg_l28a_3p0>;
>> +
>> +		reset-gpios = <&tlmm 6 GPIO_ACTIVE_LOW>;
>> +
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&panel_reset_pins &panel_te_pin &panel_pmgpio_pins>;
>> +
>> +		ports {
>> +			#address-cells = <1>;
>> +			#size-cells = <0>;
>> +
>> +			port@0 {
>> +				reg = <0>;
>> +				lg_sw43408_in_0: endpoint {
>> +					remote-endpoint = <&dsi0_out>;
>> +				};
>> +			};
>> +		};
>> +	};
>> +};
>> +
>> +&dsi0_out {
>> +	remote-endpoint = <&lg_sw43408_in_0>;
>> +	data-lanes = <0 1 2 3>;
>> +};
>> +
>> +&dsi0_phy {
>> +	status = "okay";
>> +	vdds-supply = <&vdda_mipi_dsi0_pll>;
>> +};
>> +
>> +&gcc {
>> +	protected-clocks = <GCC_QSPI_CORE_CLK>,
>> +			   <GCC_QSPI_CORE_CLK_SRC>,
>> +			   <GCC_QSPI_CNOC_PERIPH_AHB_CLK>;
>> +};
>> +
>> +&gmu {
>> +	status = "okay";
>> +};
>> +
>> +&gpi_dma0 {
>> +	status = "okay";
>> +};
>> +
>> +&gpu {
>> +	status = "okay";
>> +
>> +	zap-shader {
>> +		memory-region = <&gpu_mem>;
>> +		firmware-name = "qcom/sdm845/pixel3/a630_zap.mbn";
>> +	};
>> +};
>> +
>> +&ipa {
>> +	status = "okay";
>> +
>> +	firmware-name = "qcom/sdm845/pixel3/ipa_fws.mbn";
>> +};
>> +
>> +&mss_pil {
>> +	status = "okay";
>> +	firmware-name = "qcom/sdm845/pixel3/mba.mbn", "qcom/sdm845/pixel3/modem.mbn";
>> +};
>> +
>> +&mdss {
>> +	status = "okay";
>> +};
>> +
>> +&mdss_mdp {
>> +	status = "okay";
>> +};
>
> Not necessary, it is a default state since the commit 4a5622c1d975
> ("arm64: dts: qcom: sdm845: Don't disable MDP explicitly")
thanks, will drop
>
>> +
>> +&pm8998_gpio {
>> +	volume_up_gpio: vol-up-active {
>> +		pins = "gpio6";
>> +		function = "normal";
>> +		input-enable;
>> +		bias-pull-up;
>> +		qcom,drive-strength = <0>;
>> +	};
>> +
>> +	panel_pmgpio_pins: panel-pmgpio-active {
>> +		pins = "gpio2", "gpio5";
>> +		function = "normal";
>> +		input-enable;
>> +		bias-disable;
>> +		power-source = <0>;
>> +	};
>> +};
>> +
>> +&pm8998_pon {
>> +	resin {
>> +		compatible = "qcom,pm8941-resin";
>> +		interrupts = <0x0 0x8 1 IRQ_TYPE_EDGE_BOTH>;
>> +		debounce = <15625>;
>> +		bias-pull-up;
>> +		linux,code = <KEY_VOLUMEDOWN>;
>> +	};
>
> Please move the (disabled, labelled) resin device to pm8998.dtsi and
> just add status = "okay" here.
OK, will do.
>
>> +};
>> +
>> +&qupv3_id_0 {
>> +	status = "okay";
>> +};
>> +
>> +&qupv3_id_1 {
>> +	status = "okay";
>> +};
>> +
>> +&qup_uart6_default {
>> +	pinmux {
>> +		pins = "gpio45", "gpio46", "gpio47", "gpio48";
>> +		function = "qup6";
>> +	};
>> +
>> +	cts {
>> +		pins = "gpio45";
>> +		bias-disable;
>> +	};
>> +
>> +	rts-tx {
>> +		pins = "gpio46", "gpio47";
>> +		drive-strength = <2>;
>> +		bias-disable;
>> +	};
>> +
>> +	rx {
>> +		pins = "gpio48";
>> +		bias-pull-up;
>> +	};
>> +};
>> +
>> +&qup_uart9_default {
>> +	pinconf-tx {
>> +		pins = "gpio4";
>> +		drive-strength = <2>;
>> +		bias-disable;
>> +	};
>> +
>> +	pinconf-rx {
>> +		pins = "gpio5";
>> +		drive-strength = <2>;
>> +		bias-pull-up;
>> +	};
>> +};
>> +
>> +&tlmm {
>> +	gpio-reserved-ranges = <0 4>, <81 4>;
>> +
>> +	panel_te_pin: panel-te {
>> +		mux {
>> +			pins = "gpio12";
>> +			function = "mdp_vsync";
>> +			drive-strength = <2>;
>> +			bias-disable;
>> +			input-enable;
>> +		};
>> +	};
>> +
>> +	panel_reset_pins: panel-active {
>> +		mux {
>> +			pins = "gpio6", "gpio52";
>> +			function = "gpio";
>> +			drive-strength = <8>;
>> +			bias-disable = <0>;
>> +		};
>> +	};
>> +
>> +	panel_suspend: panel-suspend {
>> +		mux {
>> +			pins = "gpio6", "gpio52";
>> +			function = "gpio";
>> +			drive-strength = <2>;
>> +			bias-pull-down;
>> +		};
>> +	};
>> +
>> +	touchscreen_reset: ts-reset {
>> +		mux {
>> +			pins = "gpio99";
>> +			function = "gpio";
>> +			drive-strength = <8>;
>> +			bias-pull-up;
>> +			//output-high;
>
> debug, can be removed?
>
>> +		};
>> +	};
>> +
>> +	touchscreen_pins: ts-pins {
>> +		mux {
>> +			pins = "gpio125";
>> +			function = "gpio";
>> +			drive-strength = <2>;
>> +			bias-disable;
>> +		};
>> +	};
>> +
>> +	touchscreen_i2c_pins: qup-i2c2-gpio {
>> +		pins = "gpio27", "gpio28";
>> +		function = "gpio";
>> +
>> +		drive-strength = <2>;
>> +		bias-disable;
>> +	};
>> +};
>> +
>> +&uart6 {
>> +	status = "okay";
>> +
>> +	bluetooth {
>> +		compatible = "qcom,wcn3990-bt";
>> +
>> +		vddio-supply = <&vreg_s4a_1p8>;
>> +		vddxo-supply = <&vreg_l7a_1p8>;
>> +		vddrf-supply = <&vreg_l17a_1p3>;
>> +		vddch0-supply = <&vreg_l25a_3p3>;
>> +		max-speed = <3200000>;
>> +	};
>> +};
>> +
>> +&uart9 {
>> +	status = "okay";
>> +};
>> +
>> +&usb_1 {
>> +	status = "okay";
>> +};
>> +
>> +&usb_1_dwc3 {
>> +	dr_mode = "peripheral";
>> +};
>> +
>> +&usb_1_hsphy {
>> +	status = "okay";
>> +
>> +	vdd-supply = <&vreg_l1a_0p875>;
>> +	vdda-pll-supply = <&vreg_l12a_1p8>;
>> +	vdda-phy-dpdm-supply = <&vreg_l24a_3p075>;
>> +
>> +	qcom,imp-res-offset-value = <8>;
>> +	qcom,hstx-trim-value = <QUSB2_V2_HSTX_TRIM_21_6_MA>;
>> +	qcom,preemphasis-level = <QUSB2_V2_PREEMPHASIS_5_PERCENT>;
>> +	qcom,preemphasis-width = <QUSB2_V2_PREEMPHASIS_WIDTH_HALF_BIT>;
>> +};
>> +
>> +&usb_1_qmpphy {
>> +	status = "okay";
>> +
>> +	vdda-phy-supply = <&vreg_l26a_1p2>;
>> +	vdda-pll-supply = <&vreg_l1a_0p875>;
>> +};
>> +
>> +&usb_2 {
>> +	status = "okay";
>> +};
>> +
>> +&usb_2_dwc3 {
>> +	dr_mode = "host";
>> +};
>> +
>> +&usb_2_hsphy {
>> +	status = "okay";
>> +
>> +	vdd-supply = <&vreg_l1a_0p875>;
>> +	vdda-pll-supply = <&vreg_l12a_1p8>;
>> +	vdda-phy-dpdm-supply = <&vreg_l24a_3p075>;
>> +
>> +	qcom,imp-res-offset-value = <8>;
>> +	qcom,hstx-trim-value = <QUSB2_V2_HSTX_TRIM_22_8_MA>;
>> +};
>> +
>> +&usb_2_qmpphy {
>> +	status = "okay";
>> +
>> +	vdda-phy-supply = <&vreg_l26a_1p2>;
>> +	vdda-pll-supply = <&vreg_l1a_0p875>;
>> +};
>> +
>> +&ufs_mem_hc {
>> +	status = "okay";
>> +
>> +	reset-gpios = <&tlmm 150 GPIO_ACTIVE_LOW>;
>> +
>> +	vcc-supply = <&vreg_l20a_2p95>;
>> +	vcc-max-microamp = <800000>;
>> +};
>> +
>> +&ufs_mem_phy {
>> +	status = "okay";
>> +
>> +	vdda-phy-supply = <&vreg_l1a_0p875>;
>> +	vdda-pll-supply = <&vreg_l26a_1p2>;
>> +};
>> +
>> +&venus {
>> +	status = "okay";
>> +	firmware-name = "qcom/sdm845/oneplus6/venus.mbn";
>
> Why are you using the oneplus6 here?
Oops! Will fix that.
>
>> +};
>> +
>> +&wifi {
>> +	status = "okay";
>> +
>> +	vdd-0.8-cx-mx-supply = <&vreg_l5a_0p8>;
>> +	vdd-1.8-xo-supply = <&vreg_l7a_1p8>;
>> +	vdd-1.3-rfa-supply = <&vreg_l17a_1p3>;
>> +	vdd-3.3-ch0-supply = <&vreg_l25a_3p3>;
>> +
>> +	qcom,snoc-host-cap-8bit-quirk;
>> +	qcom,ath10k-calibration-variant = "google_blueline";
>
> Ideally Kalle Valo should bless this string, added him to the Cc list.
> Could you please submit the board file to the ath10k (see [1] for the
> description and [2] for an example).
Sure will do.
>
>
> [1] https://wireless.wiki.kernel.org/en/users/drivers/ath10k/boardfiles
> [2]
> https://lore.kernel.org/ath10k/CAA8EJpphUrxr5gtW0=-tZh-DrKXmHkfFxWMvYRpTUGuCesGCbw@mail.gmail.com/T/#u
>
>> +};
>
Thanks a lot for the review!

Kind Regards,
Caleb
>
> --
> With best wishes
> Dmitry


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 4/4] drm: panel: Add lg sw43408 panel driver
  2022-07-18 21:30 ` [PATCH 4/4] drm: panel: Add lg sw43408 panel driver Caleb Connolly
@ 2022-07-18 23:06   ` Dmitry Baryshkov
  2022-07-19  7:48   ` Sam Ravnborg
  2022-10-01 21:48   ` Marijn Suijten
  2 siblings, 0 replies; 18+ messages in thread
From: Dmitry Baryshkov @ 2022-07-18 23:06 UTC (permalink / raw)
  To: Caleb Connolly, Andy Gross, Bjorn Andersson, Konrad Dybcio,
	Rob Herring, Krzysztof Kozlowski, Thierry Reding, Sam Ravnborg,
	David Airlie, Daniel Vetter, Sumit Semwal, linux-arm-msm,
	devicetree, linux-kernel, dri-devel, phone-devel,
	~postmarketos/upstreaming
  Cc: Vinod Koul

On 19/07/2022 00:30, Caleb Connolly wrote:
> From: Sumit Semwal <sumit.semwal@linaro.org>
> 
> LG SW43408 is 1080x2160, 4-lane MIPI-DSI panel, used in some Pixel3
> phones.
> 
> Whatever init sequence we have for this panel isn't capable of
> initialising it completely, toggling the reset gpio ever causes the
> panel to die. Until this is resolved we avoid resetting the panel. The
> disable/unprepare functions only put the panel to sleep mode and
> disable the backlight.
> 
> Signed-off-by: Sumit Semwal <sumit.semwal@linaro.org>
> [vinod: Add DSC support]
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
> [caleb: cleanup and support turning off the panel]
> Signed-off-by: Caleb Connolly <caleb@connolly.tech>
> ---
>   MAINTAINERS                              |   8 +
>   drivers/gpu/drm/panel/Kconfig            |  11 +
>   drivers/gpu/drm/panel/Makefile           |   1 +
>   drivers/gpu/drm/panel/panel-lg-sw43408.c | 586 +++++++++++++++++++++++
>   4 files changed, 606 insertions(+)
>   create mode 100644 drivers/gpu/drm/panel/panel-lg-sw43408.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f679152bdbad..8a2b954ad140 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6376,6 +6376,14 @@ S:	Orphan / Obsolete
>   F:	drivers/gpu/drm/i810/
>   F:	include/uapi/drm/i810_drm.h
>   
> +DRM DRIVER FOR LG SW43408 PANELS
> +M:	Sumit Semwal <sumit.semwal@linaro.org>
> +M:	Caleb Connolly <caleb@connolly.tech>
> +S:	Maintained
> +T:	git git://anongit.freedesktop.org/drm/drm-misc
> +F:	Documentation/devicetree/bindings/display/panel/lg,sw43408-panel.txt
> +F:	drivers/gpu/drm/panel/panel-lg-sw43408.c
> +
>   DRM DRIVER FOR LVDS PANELS
>   M:	Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>   L:	dri-devel@lists.freedesktop.org
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index 38799effd00a..706b112794b9 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -256,6 +256,17 @@ config DRM_PANEL_LEADTEK_LTK500HD1829
>   	  24 bit RGB per pixel. It provides a MIPI DSI interface to
>   	  the host and has a built-in LED backlight.
>   
> +config DRM_PANEL_LG_SW43408
> +	tristate "LG SW43408 panel"
> +	depends on OF
> +	depends on DRM_MIPI_DSI
> +	depends on BACKLIGHT_CLASS_DEVICE
> +	help
> +	  Say Y here if you want to enable support for LG sw43408 panel.
> +	  The panel has a 1080x2160 resolution and uses
> +	  24 bit RGB per pixel. It provides a MIPI DSI interface to
> +	  the host and has a built-in LED backlight.
> +
>   config DRM_PANEL_SAMSUNG_LD9040
>   	tristate "Samsung LD9040 RGB/SPI panel"
>   	depends on OF && SPI
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index 42a7ab54234b..ba26a69b74e7 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -25,6 +25,7 @@ obj-$(CONFIG_DRM_PANEL_LEADTEK_LTK050H3146W) += panel-leadtek-ltk050h3146w.o
>   obj-$(CONFIG_DRM_PANEL_LEADTEK_LTK500HD1829) += panel-leadtek-ltk500hd1829.o
>   obj-$(CONFIG_DRM_PANEL_LG_LB035Q02) += panel-lg-lb035q02.o
>   obj-$(CONFIG_DRM_PANEL_LG_LG4573) += panel-lg-lg4573.o
> +obj-$(CONFIG_DRM_PANEL_LG_SW43408) += panel-lg-sw43408.o
>   obj-$(CONFIG_DRM_PANEL_NEC_NL8048HL11) += panel-nec-nl8048hl11.o
>   obj-$(CONFIG_DRM_PANEL_NEWVISION_NV3052C) += panel-newvision-nv3052c.o
>   obj-$(CONFIG_DRM_PANEL_NOVATEK_NT35510) += panel-novatek-nt35510.o
> diff --git a/drivers/gpu/drm/panel/panel-lg-sw43408.c b/drivers/gpu/drm/panel/panel-lg-sw43408.c
> new file mode 100644
> index 000000000000..c7b8ec7b970d
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-lg-sw43408.c
> @@ -0,0 +1,586 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2019 Linaro Ltd
> + * Author: Sumit Semwal <sumit.semwal@linaro.org>
> + */
> +
> +#include <linux/backlight.h>
> +#include <linux/delay.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +
> +#include <linux/gpio/consumer.h>
> +#include <linux/regulator/consumer.h>
> +
> +#include <drm/drm_device.h>
> +#include <drm/drm_mipi_dsi.h>
> +#include <drm/drm_modes.h>
> +#include <drm/drm_panel.h>
> +#include <drm/drm_print.h>
> +#include <drm/display/drm_dsc.h>
> +#include <drm/display/drm_dsc_helper.h>
> +
> +#include <video/mipi_display.h>
> +
> +struct panel_cmd {
> +	size_t len;
> +	const char *data;
> +};
> +
> +#define _INIT_CMD(...)                                                   \
> +	{                                                                \
> +		.len = sizeof((char[]){ __VA_ARGS__ }), .data = (char[]) \
> +		{                                                        \
> +			__VA_ARGS__                                      \
> +		}                                                        \
> +	}
> +
> +static const char *const regulator_names[] = {
> +	"vddi",
> +	"vpnl",
> +};
> +
> +static const unsigned long regulator_enable_loads[] = {
> +	62000,
> +	857000,
> +};
> +
> +static const unsigned long regulator_disable_loads[] = {
> +	80,
> +	0,
> +};
> +
> +struct sw43408_panel {
> +	struct drm_panel base;
> +	struct mipi_dsi_device *link;
> +
> +	const struct drm_display_mode *mode;
> +	struct backlight_device *backlight;
> +
> +	struct regulator_bulk_data supplies[ARRAY_SIZE(regulator_names)];
> +
> +	struct gpio_desc *reset_gpio;
> +
> +	bool prepared;
> +	bool enabled;
> +};
> +
> +static const struct panel_cmd lg_sw43408_on_cmds_1[] = {
> +	_INIT_CMD(0x00, 0x53, 0x0C, 0x30),

Please use the lower case for hex numbers.

> +	_INIT_CMD(0x00, 0x55, 0x00, 0x70, 0xDF, 0x00, 0x70, 0xDF),
> +	_INIT_CMD(0x00, 0xF7, 0x01, 0x49, 0x0C),
> +
> +	{},
> +};
> +
> +static const struct panel_cmd lg_sw43408_on_cmds_2[] = {
> +	_INIT_CMD(0x00, 0xB0, 0xAC),
> +	_INIT_CMD(0x00, 0xCD, 0x00, 0x00, 0x00, 0x19, 0x19, 0x19, 0x19, 0x19,
> +		  0x19, 0x19, 0x19, 0x19, 0x19, 0x19, 0x19, 0x19, 0x16, 0x16),
> +	_INIT_CMD(0x00, 0xCB, 0x80, 0x5C, 0x07, 0x03, 0x28),
> +	_INIT_CMD(0x00, 0xC0, 0x02, 0x02, 0x0F),
> +	_INIT_CMD(0x00, 0xE5, 0x00, 0x3A, 0x00, 0x3A, 0x00, 0x0E, 0x10),
> +	_INIT_CMD(0x00, 0xB5, 0x75, 0x60, 0x2D, 0x5D, 0x80, 0x00, 0x0A, 0x0B,
> +		  0x00, 0x05, 0x0B, 0x00, 0x80, 0x0D, 0x0E, 0x40, 0x00, 0x0C,
> +		  0x00, 0x16, 0x00, 0xB8, 0x00, 0x80, 0x0D, 0x0E, 0x40, 0x00,
> +		  0x0C, 0x00, 0x16, 0x00, 0xB8, 0x00, 0x81, 0x00, 0x03, 0x03,
> +		  0x03, 0x01, 0x01),
> +	_INIT_CMD(0x00, 0x55, 0x04, 0x61, 0xDB, 0x04, 0x70, 0xDB),
> +	_INIT_CMD(0x00, 0xB0, 0xCA),

The dtbo from the latest firware uses a bit different sequence here:

0xb0, 0xac
0xe5, 0x00....
0xb5, 0x75.....
msleep(85);
0xcd, 0x00.....
0xcb, 0x80...
0x55, 0x04...
0xb0, 0xca

I'm not sure, but granted that you have issues with panel reinit, maybe 
that would make any difference.

> +
> +	{},
> +};
> +
> +static inline struct sw43408_panel *to_panel_info(struct drm_panel *panel)
> +{
> +	return container_of(panel, struct sw43408_panel, base);
> +}
> +
> +/*
> + * Currently unable to bring up the panel after resetting, must be missing
> + * some init commands somewhere.
> + */
> +static __always_unused int panel_reset(struct sw43408_panel *ctx)
> +{
> +	int ret = 0, i;
> +
> +	for (i = 0; i < ARRAY_SIZE(ctx->supplies); i++) {
> +		ret = regulator_set_load(ctx->supplies[i].consumer,
> +					 regulator_enable_loads[i]);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> +	if (ret < 0)
> +		return ret;
> +
> +	for (i = 0; i < ARRAY_SIZE(ctx->supplies); i++) {
> +		ret = regulator_set_load(ctx->supplies[i].consumer,
> +					 regulator_disable_loads[i]);
> +		if (ret) {
> +			DRM_DEV_ERROR(ctx->base.dev,
> +				      "regulator_set_load failed %d\n", ret);
> +			return ret;
> +		}
> +	}

If I remember correctly, there is no need to set loads before disabling 
the regulator.

> +
> +	ret = regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> +	if (ret < 0)
> +		return ret;

So, you bump & disable regulators before playing with the reset GPIO. 
Does the panel honour reset GPIO while being powered down?

> +
> +	gpiod_set_value(ctx->reset_gpio, 0);
> +	usleep_range(9000, 10000);
> +	gpiod_set_value(ctx->reset_gpio, 1);
> +	usleep_range(1000, 2000);
> +	gpiod_set_value(ctx->reset_gpio, 0);
> +	usleep_range(9000, 10000);
> +
> +	return 0;
> +}
> +
> +static int send_mipi_cmds(struct drm_panel *panel, const struct panel_cmd *cmds)
> +{
> +	struct sw43408_panel *ctx = to_panel_info(panel);
> +	unsigned int i = 0;
> +	int err;
> +
> +	if (!cmds)
> +		return -EFAULT;
> +
> +	for (i = 0; cmds[i].len != 0; i++) {
> +		const struct panel_cmd *cmd = &cmds[i];
> +
> +		if (cmd->len == 2)
> +			err = mipi_dsi_dcs_write(ctx->link, cmd->data[1], NULL,
> +						 0);
> +		else
> +			err = mipi_dsi_dcs_write(ctx->link, cmd->data[1],
> +						 cmd->data + 2, cmd->len - 2);
> +
> +		if (err < 0)
> +			return err;
> +
> +		usleep_range((cmd->data[0]) * 1000, (1 + cmd->data[0]) * 1000);
> +	}
> +
> +	return 0;
> +}
> +
> +static int lg_panel_disable(struct drm_panel *panel)
> +{
> +	struct sw43408_panel *ctx = to_panel_info(panel);
> +
> +	backlight_disable(ctx->backlight);
> +	ctx->enabled = false;
> +
> +	return 0;
> +}
> +
> +/*
> + * We can't currently re-initialise the panel properly after powering off.
> + * This function will be used when this is resolved.
> + */
> +static __always_unused int lg_panel_power_off(struct drm_panel *panel)
> +{
> +	struct sw43408_panel *ctx = to_panel_info(panel);
> +	int i, ret = 0;
> +
> +	gpiod_set_value(ctx->reset_gpio, 1);
> +
> +	for (i = 0; i < ARRAY_SIZE(ctx->supplies); i++) {
> +		ret = regulator_set_load(ctx->supplies[i].consumer,
> +					 regulator_disable_loads[i]);
> +		if (ret) {
> +			DRM_DEV_ERROR(panel->dev,
> +				      "regulator_set_load failed %d\n", ret);
> +			return ret;
> +		}
> +	}
> +
> +	ret = regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> +	if (ret) {
> +		DRM_DEV_ERROR(panel->dev, "regulator_bulk_disable failed %d\n",
> +			      ret);
> +	}
> +	return ret;
> +}
> +
> +static int lg_panel_unprepare(struct drm_panel *panel)
> +{
> +	struct sw43408_panel *ctx = to_panel_info(panel);
> +	int ret, i;
> +
> +	if (!ctx->prepared)
> +		return 0;
> +
> +	ret = mipi_dsi_dcs_set_display_off(ctx->link);
> +	if (ret < 0) {
> +		DRM_DEV_ERROR(panel->dev,
> +			      "set_display_off cmd failed ret = %d\n", ret);
> +	}
> +
> +	msleep(120);
> +
> +	ret = mipi_dsi_dcs_enter_sleep_mode(ctx->link);
> +	if (ret < 0) {
> +		DRM_DEV_ERROR(panel->dev, "enter_sleep cmd failed ret = %d\n",
> +			      ret);
> +	}
> +
> +	/* Would call panel_power_off() */
> +
> +	for (i = 0; i < ARRAY_SIZE(ctx->supplies); i++) {
> +		ret = regulator_set_load(ctx->supplies[i].consumer,
> +					 regulator_disable_loads[i]);
> +		if (ret) {
> +			DRM_DEV_ERROR(panel->dev,
> +				      "regulator_set_load failed %d\n", ret);
> +			return ret;
> +		}
> +	}

Do you need to disable regulators instead?
Also would you need to pull the reset gpio?

> +
> +	ctx->prepared = false;
> +
> +	return ret;
> +}
> +
> +static int lg_panel_prepare(struct drm_panel *panel)
> +{
> +	struct sw43408_panel *ctx = to_panel_info(panel);
> +	int err, i;
> +
> +	if (ctx->prepared)
> +		return 0;
> +
> +	/* Would call panel_reset() */
> +
> +	for (i = 0; i < ARRAY_SIZE(ctx->supplies); i++) {
> +		err = regulator_set_load(ctx->supplies[i].consumer,
> +					 regulator_enable_loads[i]);
> +		if (err)
> +			return err;
> +	}
> +
> +	err = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> +	if (err < 0)
> +		return err;
> +
> +	usleep_range(9000, 10000);

Reset the panel here?

> +
> +	err = mipi_dsi_dcs_write(ctx->link, MIPI_DCS_SET_GAMMA_CURVE,
> +				 (u8[]){ 0x02 }, 1);
> +	if (err < 0) {
> +		DRM_DEV_ERROR(panel->dev, "failed to set gamma curve: %d\n",
> +			      err);
> +		goto poweroff;
> +	}
> +
> +	err = mipi_dsi_dcs_set_tear_on(ctx->link,
> +				       MIPI_DSI_DCS_TEAR_MODE_VBLANK);
> +	if (err < 0) {
> +		DRM_DEV_ERROR(panel->dev, "failed to set tear on: %d\n", err);
> +		goto poweroff;
> +	}
> +
> +	err = send_mipi_cmds(panel, &lg_sw43408_on_cmds_1[0]);
> +
> +	if (err < 0) {
> +		DRM_DEV_ERROR(panel->dev,
> +			      "failed to send DCS Init 1st Code: %d\n", err);
> +		goto poweroff;
> +	}
> +
> +	err = mipi_dsi_dcs_exit_sleep_mode(ctx->link);
> +	if (err < 0) {
> +		DRM_DEV_ERROR(panel->dev, "failed to exit sleep mode: %d\n",
> +			      err);
> +		goto poweroff;
> +	}
> +
> +	msleep(135);
> +
> +	err = mipi_dsi_dcs_write(ctx->link, MIPI_DSI_COMPRESSION_MODE,
> +				 (u8[]){ 0x11 }, 0);

Please adjust mipi_dsi_compression_mode() instead. Do you really need 
0x11 here? Does 0x1 work? You might want to adjust the mentioned function.

> +	if (err < 0) {
> +		DRM_DEV_ERROR(panel->dev,
> +			      "failed to set compression mode: %d\n", err);
> +		goto poweroff;
> +	}
> +
> +	err = send_mipi_cmds(panel, &lg_sw43408_on_cmds_2[0]);
> +
> +	if (err < 0) {
> +		DRM_DEV_ERROR(panel->dev,
> +			      "failed to send DCS Init 2nd Code: %d\n", err);
> +		goto poweroff;
> +	}
> +
> +	err = mipi_dsi_dcs_set_display_on(ctx->link);
> +	if (err < 0) {
> +		DRM_DEV_ERROR(panel->dev, "failed to Set Display ON: %d\n",
> +			      err);
> +		goto poweroff;
> +	}
> +
> +	msleep(120);
> +
> +	ctx->prepared = true;
> +
> +	return 0;
> +
> +poweroff:
> +	gpiod_set_value(ctx->reset_gpio, 1);
> +	regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> +	return err;
> +}
> +
> +static int lg_panel_enable(struct drm_panel *panel)
> +{
> +	struct sw43408_panel *ctx = to_panel_info(panel);
> +	struct drm_dsc_picture_parameter_set pps;
> +	int ret;
> +
> +	if (ctx->enabled)
> +		return 0;
> +
> +	ret = backlight_enable(ctx->backlight);
> +	if (ret) {
> +		DRM_DEV_ERROR(panel->dev, "Failed to enable backlight %d\n",
> +			      ret);
> +		return ret;
> +	}

Do you need to enable the backlight beforehand (well, before sending the 
pps?) drm_panel_enable() would enable the backlight for you.

> +
> +	if (!panel->dsc) {
> +		DRM_DEV_ERROR(panel->dev, "Can't find DSC\n");
> +		return -ENODEV;
> +	}
> +
> +	drm_dsc_pps_payload_pack(&pps, panel->dsc);

Do you need to call mipi_dsi_picture_parameter_set() here? Otherwise 
genrated pps is not used at all.

> +
> +	ctx->enabled = true;
> +
> +	return 0;
> +}
> +
> +static int lg_panel_get_modes(struct drm_panel *panel,
> +			      struct drm_connector *connector)
> +{
> +	struct sw43408_panel *ctx = to_panel_info(panel);
> +	const struct drm_display_mode *m = ctx->mode;
> +	struct drm_display_mode *mode;
> +
> +	mode = drm_mode_duplicate(connector->dev, m);
> +	if (!mode) {
> +		DRM_DEV_ERROR(panel->dev, "failed to add mode %ux%u\n",
> +			      m->hdisplay, m->vdisplay);
> +		return -ENOMEM;
> +	}
> +
> +	connector->display_info.width_mm = m->width_mm;
> +	connector->display_info.height_mm = m->height_mm;
> +
> +	drm_mode_set_name(mode);
> +	drm_mode_probed_add(connector, mode);
> +
> +	return 1;
> +}
> +
> +static int lg_panel_backlight_update_status(struct backlight_device *bl)
> +{
> +	struct mipi_dsi_device *dsi = bl_get_data(bl);
> +	int ret = 0;
> +	uint16_t brightness;

u16

> +
> +	brightness = (uint16_t)backlight_get_brightness(bl);

Probably no need to cast here.

> +	/* Brightness is sent in big-endian */
> +	brightness = cpu_to_be16(brightness);
> +
> +	ret = mipi_dsi_dcs_set_display_brightness(dsi, brightness);

lmdpdg.py suggest that this (and get_display_brightnees()) should be 
wrapped into disabling and reenabling MIPI_DSI_MODE_LPM in dsi->mode_flags.

> +	return ret;
> +}
> +
> +static int lg_panel_backlight_get_brightness(struct backlight_device *bl)
> +{
> +	struct mipi_dsi_device *dsi = bl_get_data(bl);
> +	int ret = 0;
> +	u16 brightness = 0;
> +
> +	ret = mipi_dsi_dcs_get_display_brightness(dsi, &brightness);
> +	if (ret < 0)
> +		return ret;
> +
> +	return brightness & 0xff;

So the brighness is sent as be16, but doesn't require handling on 
get_brightness?

> +}
> +
> +const struct backlight_ops lg_panel_backlight_ops = {
> +	.update_status = lg_panel_backlight_update_status,
> +	.get_brightness = lg_panel_backlight_get_brightness,
> +};
> +
> +static int lg_panel_backlight_init(struct sw43408_panel *ctx)
> +{
> +	struct device *dev = &ctx->link->dev;
> +	const struct backlight_properties props = {
> +		.type = BACKLIGHT_PLATFORM,
> +		.brightness = 255,
> +		.max_brightness = 255,

900, according to the dtsi

> +	};
> +
> +	ctx->backlight = devm_backlight_device_register(dev, dev_name(dev), dev,
> +							ctx->link,
> +							&lg_panel_backlight_ops,
> +							&props);
> +
> +	if (IS_ERR(ctx->backlight))
> +		return dev_err_probe(dev, PTR_ERR(ctx->backlight),
> +				     "Failed to create backlight\n");
> +
> +	return 0;
> +}
> +
> +static const struct drm_panel_funcs panel_funcs = {
> +	.disable = lg_panel_disable,
> +	.unprepare = lg_panel_unprepare,
> +	.prepare = lg_panel_prepare,
> +	.enable = lg_panel_enable,
> +	.get_modes = lg_panel_get_modes,
> +};
> +
> +static const struct drm_display_mode sw43408_default_mode = {
> +	.clock = 152340,
> +
> +	.hdisplay = 1080,
> +	.hsync_start = 1080 + 20,
> +	.hsync_end = 1080 + 20 + 32,
> +	.htotal = 1080 + 20 + 32 + 20,
> +
> +	.vdisplay = 2160,
> +	.vsync_start = 2160 + 20,
> +	.vsync_end = 2160 + 20 + 4,
> +	.vtotal = 2160 + 20 + 4 + 20,
> +
> +	.width_mm = 62,
> +	.height_mm = 124,
> +
> +	.type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED,
> +};
> +
> +static const struct of_device_id panel_of_match[] = {
> +	{ .compatible = "lg,sw43408", .data = &sw43408_default_mode },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, panel_of_match);
> +
> +static int panel_add(struct sw43408_panel *ctx)
> +{
> +	struct device *dev = &ctx->link->dev;
> +	int i, ret;
> +
> +	for (i = 0; i < ARRAY_SIZE(ctx->supplies); i++)
> +		ctx->supplies[i].supply = regulator_names[i];
> +
> +	ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(ctx->supplies),
> +				      ctx->supplies);
> +	if (ret < 0)
> +		return ret;
> +
> +	ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> +	if (IS_ERR(ctx->reset_gpio)) {
> +		DRM_DEV_ERROR(dev, "cannot get reset gpio %ld\n",
> +			      PTR_ERR(ctx->reset_gpio));
> +		return PTR_ERR(ctx->reset_gpio);
> +	}
> +
> +	ret = lg_panel_backlight_init(ctx);
> +	if (ret < 0)
> +		return ret;
> +
> +	drm_panel_init(&ctx->base, dev, &panel_funcs, DRM_MODE_CONNECTOR_DSI);
> +
> +	drm_panel_add(&ctx->base);
> +	return ret;
> +}
> +
> +static int panel_probe(struct mipi_dsi_device *dsi)
> +{
> +	struct sw43408_panel *ctx;
> +	struct drm_dsc_config *dsc;
> +	int err;
> +
> +	ctx = devm_kzalloc(&dsi->dev, sizeof(*ctx), GFP_KERNEL);
> +	if (!ctx)
> +		return -ENOMEM;
> +
> +	ctx->mode = of_device_get_match_data(&dsi->dev);
> +	dsi->mode_flags = MIPI_DSI_MODE_LPM;
> +	dsi->format = MIPI_DSI_FMT_RGB888;
> +	dsi->lanes = 4;
> +
> +	ctx->link = dsi;
> +	mipi_dsi_set_drvdata(dsi, ctx);
> +
> +	err = panel_add(ctx);
> +	if (err < 0)
> +		return err;
> +
> +	/* The panel is DSC panel only, set the dsc params */
> +	dsc = devm_kzalloc(&dsi->dev, sizeof(*dsc), GFP_KERNEL);
> +	if (!dsc)
> +		return -ENOMEM;
> +
> +	dsc->dsc_version_major = 0x1;
> +	dsc->dsc_version_minor = 0x1;
> +
> +	dsc->slice_height = 16;
> +	dsc->slice_width = 540;
> +	dsc->slice_count = 1;
> +	dsc->bits_per_component = 8;
> +	dsc->bits_per_pixel = 8;
> +	dsc->block_pred_enable = true;
> +
> +	ctx->base.dsc = dsc;

I was really hoping to move DSC PPS data to struct mipi_dsi_device, 
before DSC panel drivers start to pop up.

See 
https://lore.kernel.org/linux-arm-msm/20220711094320.368062-2-dmitry.baryshkov@linaro.org/

> +
> +	return mipi_dsi_attach(dsi);
> +}
> +
> +static int panel_remove(struct mipi_dsi_device *dsi)
> +{
> +	struct sw43408_panel *ctx = mipi_dsi_get_drvdata(dsi);
> +	int err;
> +
> +	err = lg_panel_unprepare(&ctx->base);
> +	if (err < 0)
> +		DRM_DEV_ERROR(&dsi->dev, "failed to unprepare panel: %d\n",
> +			      err);
> +
> +	err = lg_panel_disable(&ctx->base);
> +	if (err < 0)
> +		DRM_DEV_ERROR(&dsi->dev, "failed to disable panel: %d\n", err);
> +
> +	err = mipi_dsi_detach(dsi);
> +	if (err < 0)
> +		DRM_DEV_ERROR(&dsi->dev, "failed to detach from DSI host: %d\n",
> +			      err);
> +
> +	if (ctx->base.dev)
> +		drm_panel_remove(&ctx->base);
> +
> +	return 0;
> +}
> +
> +static struct mipi_dsi_driver panel_driver = {
> +	.driver = {
> +		.name = "panel-lg-sw43408",
> +		.of_match_table = panel_of_match,
> +	},
> +	.probe = panel_probe,
> +	.remove = panel_remove,
> +};
> +module_mipi_dsi_driver(panel_driver);
> +
> +MODULE_AUTHOR("Sumit Semwal <sumit.semwal@linaro.org>");
> +MODULE_DESCRIPTION("LG SW436408 MIPI-DSI LED panel");
> +MODULE_LICENSE("GPL");


-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/4] arm64: dts: qcom: add sdm845-google-blueline (Pixel 3)
  2022-07-18 22:54     ` Caleb Connolly
@ 2022-07-18 23:15       ` Dmitry Baryshkov
  0 siblings, 0 replies; 18+ messages in thread
From: Dmitry Baryshkov @ 2022-07-18 23:15 UTC (permalink / raw)
  To: Caleb Connolly, Andy Gross, Bjorn Andersson, Konrad Dybcio,
	Rob Herring, Krzysztof Kozlowski, Thierry Reding, Sam Ravnborg,
	David Airlie, Daniel Vetter, Sumit Semwal, linux-arm-msm,
	devicetree, linux-kernel, dri-devel, phone-devel,
	~postmarketos/upstreaming, Kalle Valo
  Cc: Amit Pundir, Vinod Koul

On 19/07/2022 01:54, Caleb Connolly wrote:
> 
> 
> On 18/07/2022 23:13, Dmitry Baryshkov wrote:
>> Any idea, what is the issue here? Do you have the datasheet for the panel?
> As Marijn just mentioned on IRC, the driver right now only calls
> drm_dsc_pps_payload_pack() but doesn't actually send it. I haven't
> investigated this myself but I'll look into it.
> 
> Unfortunately no datasheet afaik, they tend to be pretty elusive. The
> things I would do for a phone panel datasheet...

as a side note: I checked the dtbo.img. There are three different 
versions of the panel programming sequences in the included dtb files. 
So you might want to check for the difference with 
linux-mdss-dsi-panel-driver-generator.

-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/4] dt-bindings: panel: Add LG SW43408 MIPI-DSI panel
  2022-07-18 21:30 ` [PATCH 3/4] dt-bindings: panel: Add LG SW43408 MIPI-DSI panel Caleb Connolly
@ 2022-07-19  6:10   ` Sam Ravnborg
  2022-07-19  7:57     ` Sam Ravnborg
  2022-07-19 14:11   ` Rob Herring
  1 sibling, 1 reply; 18+ messages in thread
From: Sam Ravnborg @ 2022-07-19  6:10 UTC (permalink / raw)
  To: Caleb Connolly
  Cc: Andy Gross, Bjorn Andersson, Konrad Dybcio, Rob Herring,
	Krzysztof Kozlowski, Thierry Reding, David Airlie, Daniel Vetter,
	Sumit Semwal, linux-arm-msm, devicetree, linux-kernel, dri-devel,
	phone-devel, ~postmarketos/upstreaming, Vinod Koul

Hi Caleb,

On Mon, Jul 18, 2022 at 10:30:50PM +0100, Caleb Connolly wrote:
> From: Sumit Semwal <sumit.semwal@linaro.org>
> 
> LG SW43408 is 1080x2160, 4-lane MIPI-DSI panel.
A few things to improve to this binding.

	Sam
> 
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
> Signed-off-by: Sumit Semwal <sumit.semwal@linaro.org>
> [caleb: convert to yaml]
> Signed-off-by: Caleb Connolly <caleb@connolly.tech>
> ---
>  .../bindings/display/panel/lg,43408.yaml      | 41 +++++++++++++++++++
>  .../display/panel/panel-simple-dsi.yaml       |  2 +
>  2 files changed, 43 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/panel/lg,43408.yaml
> 
> diff --git a/Documentation/devicetree/bindings/display/panel/lg,43408.yaml b/Documentation/devicetree/bindings/display/panel/lg,43408.yaml
> new file mode 100644
> index 000000000000..0529a3aa2692
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/lg,43408.yaml
> @@ -0,0 +1,41 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/panel/panel-lvds.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: LG SW43408 1080x2160 DSI panel
> +
> +maintainers:
> +  - Caleb Connolly <caleb@connolly.tech>
> +
> +description: |
> +  This panel is used on the Pixel 3, it is a 60hz OLED panel which
> +  required DSC (Display Stream Compression) and has rounded corners.
> +
> +allOf:
> +  - $ref: panel-common.yaml#
> +
> +properties:
> +  compatible:
> +    items:
> +      - const: lg,sw43408
> +
> +  vddi-supply: true
> +  vpnl-supply: true
> +  reset-gpios: true
> +
> +  backlight: false
> +  power-supply: false
No need to say anything is false, this is covered by the statement below.
Also, the driver uses backlight, so it should be true?
> +
> +additionalProperties: false
> +
> +required:
> +  - compatible
> +  - data-mapping
> +  - width-mm
> +  - height-mm
> +  - panel-timing
> +  - port
> +
> +...
> diff --git a/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml b/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml
> index 2c00813f5d20..4498078cb1ee 100644
> --- a/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml
> +++ b/Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml
> @@ -45,6 +45,8 @@ properties:
>        - lg,acx467akm-7
>          # LG Corporation 7" WXGA TFT LCD panel
>        - lg,ld070wx3-sl01
> +        # LG Corporation sw43408 1080x2160 OLED
> +      - lg,sw43408
The panel uses three power-supplies, so it is not a "panel-simple"
binding. And we cannot have the same compatible twice, so this must be
dropped.

	Sam

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 4/4] drm: panel: Add lg sw43408 panel driver
  2022-07-18 21:30 ` [PATCH 4/4] drm: panel: Add lg sw43408 panel driver Caleb Connolly
  2022-07-18 23:06   ` Dmitry Baryshkov
@ 2022-07-19  7:48   ` Sam Ravnborg
  2022-10-01 21:48   ` Marijn Suijten
  2 siblings, 0 replies; 18+ messages in thread
From: Sam Ravnborg @ 2022-07-19  7:48 UTC (permalink / raw)
  To: Caleb Connolly
  Cc: Andy Gross, Bjorn Andersson, Konrad Dybcio, Rob Herring,
	Krzysztof Kozlowski, Thierry Reding, David Airlie, Daniel Vetter,
	Sumit Semwal, linux-arm-msm, devicetree, linux-kernel, dri-devel,
	phone-devel, ~postmarketos/upstreaming, Vinod Koul

Hi Caleb,

On Mon, Jul 18, 2022 at 10:30:51PM +0100, Caleb Connolly wrote:
> From: Sumit Semwal <sumit.semwal@linaro.org>
> 
> LG SW43408 is 1080x2160, 4-lane MIPI-DSI panel, used in some Pixel3
> phones.

Thanks for submitting this.

When reading the code it is obvious that this was based on an older
panel and there is a few things to improve to get it on on same level as
the other panel drivers today.

I will comment in the following.

	Sam

> 
> Whatever init sequence we have for this panel isn't capable of
> initialising it completely, toggling the reset gpio ever causes the
> panel to die. Until this is resolved we avoid resetting the panel. The
> disable/unprepare functions only put the panel to sleep mode and
> disable the backlight.
> 
> Signed-off-by: Sumit Semwal <sumit.semwal@linaro.org>
> [vinod: Add DSC support]
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
> [caleb: cleanup and support turning off the panel]
> Signed-off-by: Caleb Connolly <caleb@connolly.tech>
> ---
>  MAINTAINERS                              |   8 +
>  drivers/gpu/drm/panel/Kconfig            |  11 +
>  drivers/gpu/drm/panel/Makefile           |   1 +
>  drivers/gpu/drm/panel/panel-lg-sw43408.c | 586 +++++++++++++++++++++++
>  4 files changed, 606 insertions(+)
>  create mode 100644 drivers/gpu/drm/panel/panel-lg-sw43408.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f679152bdbad..8a2b954ad140 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6376,6 +6376,14 @@ S:	Orphan / Obsolete
>  F:	drivers/gpu/drm/i810/
>  F:	include/uapi/drm/i810_drm.h
>  
> +DRM DRIVER FOR LG SW43408 PANELS
> +M:	Sumit Semwal <sumit.semwal@linaro.org>
> +M:	Caleb Connolly <caleb@connolly.tech>
> +S:	Maintained
m +T:	git git://anongit.freedesktop.org/drm/drm-misc
> +F:	Documentation/devicetree/bindings/display/panel/lg,sw43408-panel.txt
> +F:	drivers/gpu/drm/panel/panel-lg-sw43408.c
> +
>  DRM DRIVER FOR LVDS PANELS
>  M:	Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>  L:	dri-devel@lists.freedesktop.org
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index 38799effd00a..706b112794b9 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -256,6 +256,17 @@ config DRM_PANEL_LEADTEK_LTK500HD1829
>  	  24 bit RGB per pixel. It provides a MIPI DSI interface to
>  	  the host and has a built-in LED backlight.
>  
> +config DRM_PANEL_LG_SW43408
> +	tristate "LG SW43408 panel"
> +	depends on OF
> +	depends on DRM_MIPI_DSI
> +	depends on BACKLIGHT_CLASS_DEVICE
> +	help
> +	  Say Y here if you want to enable support for LG sw43408 panel.
> +	  The panel has a 1080x2160 resolution and uses
> +	  24 bit RGB per pixel. It provides a MIPI DSI interface to
> +	  the host and has a built-in LED backlight.
> +

Hrmpf, the DRM_PANEL_SAMSUNG_LD9040 config entry is not placed in
alphabetic order. Can you move it or put you config optiosn with the
other LG config options?

>  config DRM_PANEL_SAMSUNG_LD9040
>  	tristate "Samsung LD9040 RGB/SPI panel"
>  	depends on OF && SPI
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index 42a7ab54234b..ba26a69b74e7 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -25,6 +25,7 @@ obj-$(CONFIG_DRM_PANEL_LEADTEK_LTK050H3146W) += panel-leadtek-ltk050h3146w.o
>  obj-$(CONFIG_DRM_PANEL_LEADTEK_LTK500HD1829) += panel-leadtek-ltk500hd1829.o
>  obj-$(CONFIG_DRM_PANEL_LG_LB035Q02) += panel-lg-lb035q02.o
>  obj-$(CONFIG_DRM_PANEL_LG_LG4573) += panel-lg-lg4573.o
> +obj-$(CONFIG_DRM_PANEL_LG_SW43408) += panel-lg-sw43408.o
>  obj-$(CONFIG_DRM_PANEL_NEC_NL8048HL11) += panel-nec-nl8048hl11.o
>  obj-$(CONFIG_DRM_PANEL_NEWVISION_NV3052C) += panel-newvision-nv3052c.o
>  obj-$(CONFIG_DRM_PANEL_NOVATEK_NT35510) += panel-novatek-nt35510.o
> diff --git a/drivers/gpu/drm/panel/panel-lg-sw43408.c b/drivers/gpu/drm/panel/panel-lg-sw43408.c
> new file mode 100644
> index 000000000000..c7b8ec7b970d
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-lg-sw43408.c
> @@ -0,0 +1,586 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2019 Linaro Ltd
Update to 2022?

> + * Author: Sumit Semwal <sumit.semwal@linaro.org>
> + */
> +
> +#include <linux/backlight.h>
> +#include <linux/delay.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +
> +#include <linux/gpio/consumer.h>
> +#include <linux/regulator/consumer.h>
> +
> +#include <drm/drm_device.h>
> +#include <drm/drm_mipi_dsi.h>
> +#include <drm/drm_modes.h>
> +#include <drm/drm_panel.h>
> +#include <drm/drm_print.h>
> +#include <drm/display/drm_dsc.h>
> +#include <drm/display/drm_dsc_helper.h>
> +
> +#include <video/mipi_display.h>
> +
> +struct panel_cmd {
> +	size_t len;
> +	const char *data;
> +};
> +
> +#define _INIT_CMD(...)                                                   \
> +	{                                                                \
> +		.len = sizeof((char[]){ __VA_ARGS__ }), .data = (char[]) \
> +		{                                                        \
> +			__VA_ARGS__                                      \
> +		}                                                        \
> +	}
Other panel drivers do:
#define _INIT_DCS_CMD(...) { \
        .type = INIT_DCS_CMD, \
        .len = sizeof((char[]){__VA_ARGS__}), \
        .data = (char[]){__VA_ARGS__} }

See for example panel-boe-tv101wum-nl6.c
This makes the init sequences a tad more readable.


> +
> +static const char *const regulator_names[] = {
> +	"vddi",
> +	"vpnl",
> +};
> +
> +static const unsigned long regulator_enable_loads[] = {
> +	62000,
> +	857000,
> +};
> +
> +static const unsigned long regulator_disable_loads[] = {
> +	80,
> +	0,
> +};
> +
> +struct sw43408_panel {
> +	struct drm_panel base;
> +	struct mipi_dsi_device *link;
This is often named "dsi"

> +
> +	const struct drm_display_mode *mode;
> +	struct backlight_device *backlight;
Unless there are specific needs, use the drm_panel provided backlight.

> +
> +	struct regulator_bulk_data supplies[ARRAY_SIZE(regulator_names)];
> +
> +	struct gpio_desc *reset_gpio;
> +
> +	bool prepared;
> +	bool enabled;
> +};
> +
> +static const struct panel_cmd lg_sw43408_on_cmds_1[] = {
> +	_INIT_CMD(0x00, 0x53, 0x0C, 0x30),
> +	_INIT_CMD(0x00, 0x55, 0x00, 0x70, 0xDF, 0x00, 0x70, 0xDF),
> +	_INIT_CMD(0x00, 0xF7, 0x01, 0x49, 0x0C),
> +
> +	{},
> +};
> +
> +static const struct panel_cmd lg_sw43408_on_cmds_2[] = {
> +	_INIT_CMD(0x00, 0xB0, 0xAC),
> +	_INIT_CMD(0x00, 0xCD, 0x00, 0x00, 0x00, 0x19, 0x19, 0x19, 0x19, 0x19,
> +		  0x19, 0x19, 0x19, 0x19, 0x19, 0x19, 0x19, 0x19, 0x16, 0x16),
> +	_INIT_CMD(0x00, 0xCB, 0x80, 0x5C, 0x07, 0x03, 0x28),
> +	_INIT_CMD(0x00, 0xC0, 0x02, 0x02, 0x0F),
> +	_INIT_CMD(0x00, 0xE5, 0x00, 0x3A, 0x00, 0x3A, 0x00, 0x0E, 0x10),
> +	_INIT_CMD(0x00, 0xB5, 0x75, 0x60, 0x2D, 0x5D, 0x80, 0x00, 0x0A, 0x0B,
> +		  0x00, 0x05, 0x0B, 0x00, 0x80, 0x0D, 0x0E, 0x40, 0x00, 0x0C,
> +		  0x00, 0x16, 0x00, 0xB8, 0x00, 0x80, 0x0D, 0x0E, 0x40, 0x00,
> +		  0x0C, 0x00, 0x16, 0x00, 0xB8, 0x00, 0x81, 0x00, 0x03, 0x03,
> +		  0x03, 0x01, 0x01),
> +	_INIT_CMD(0x00, 0x55, 0x04, 0x61, 0xDB, 0x04, 0x70, 0xDB),
> +	_INIT_CMD(0x00, 0xB0, 0xCA),
> +
> +	{},
> +};
> +
> +static inline struct sw43408_panel *to_panel_info(struct drm_panel *panel)
> +{
> +	return container_of(panel, struct sw43408_panel, base);
> +}
> +
> +/*
> + * Currently unable to bring up the panel after resetting, must be missing
> + * some init commands somewhere.
> + */
> +static __always_unused int panel_reset(struct sw43408_panel *ctx)
> +{
> +	int ret = 0, i;
> +
> +	for (i = 0; i < ARRAY_SIZE(ctx->supplies); i++) {
> +		ret = regulator_set_load(ctx->supplies[i].consumer,
> +					 regulator_enable_loads[i]);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> +	if (ret < 0)
> +		return ret;
> +
> +	for (i = 0; i < ARRAY_SIZE(ctx->supplies); i++) {
> +		ret = regulator_set_load(ctx->supplies[i].consumer,
> +					 regulator_disable_loads[i]);
> +		if (ret) {
> +			DRM_DEV_ERROR(ctx->base.dev,
> +				      "regulator_set_load failed %d\n", ret);
panels do not use DRM_DEV_ERROR() these days. Use dev_err() or pr_err().

> +			return ret;
> +		}
> +	}
> +
> +	ret = regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> +	if (ret < 0)
> +		return ret;
> +
> +	gpiod_set_value(ctx->reset_gpio, 0);
> +	usleep_range(9000, 10000);
> +	gpiod_set_value(ctx->reset_gpio, 1);
> +	usleep_range(1000, 2000);
> +	gpiod_set_value(ctx->reset_gpio, 0);
> +	usleep_range(9000, 10000);
> +
> +	return 0;
> +}
> +
> +static int send_mipi_cmds(struct drm_panel *panel, const struct panel_cmd *cmds)
> +{
> +	struct sw43408_panel *ctx = to_panel_info(panel);
> +	unsigned int i = 0;
> +	int err;
> +
> +	if (!cmds)
> +		return -EFAULT;
> +
> +	for (i = 0; cmds[i].len != 0; i++) {
> +		const struct panel_cmd *cmd = &cmds[i];
> +
> +		if (cmd->len == 2)
> +			err = mipi_dsi_dcs_write(ctx->link, cmd->data[1], NULL,
> +						 0);
Do not wrap lines like this, up to 100 chars is ok.

There is no use of len == 2 - as there is none with this length. Maybe
error out there is one as this is likely a bug?

> +		else
> +			err = mipi_dsi_dcs_write(ctx->link, cmd->data[1],
> +						 cmd->data + 2, cmd->len - 2);
> +
Consider to use mipi_dsi_dcs_write_buffer

> +		if (err < 0)
> +			return err;
> +
> +		usleep_range((cmd->data[0]) * 1000, (1 + cmd->data[0]) * 1000);
So there is always a usleep_range(0, 1000) after each write - is this
really required?
Also, it is non-obvious that the first byte in the arry is a delay
parameter, this can be done better. See panel-boe-tv101wum-nl6.c as one
example - there may be betters ways to do it.

> +	}
> +
> +	return 0;
> +}
> +
> +static int lg_panel_disable(struct drm_panel *panel)
> +{
> +	struct sw43408_panel *ctx = to_panel_info(panel);
> +
> +	backlight_disable(ctx->backlight);
> +	ctx->enabled = false;
> +
> +	return 0;
> +}
Let drm_panel handle backlight, then you can drop the disable method.

> +
> +/*
> + * We can't currently re-initialise the panel properly after powering off.
> + * This function will be used when this is resolved.
> + */
> +static __always_unused int lg_panel_power_off(struct drm_panel *panel)
> +{
> +	struct sw43408_panel *ctx = to_panel_info(panel);
> +	int i, ret = 0;
> +
> +	gpiod_set_value(ctx->reset_gpio, 1);
> +
> +	for (i = 0; i < ARRAY_SIZE(ctx->supplies); i++) {
> +		ret = regulator_set_load(ctx->supplies[i].consumer,
> +					 regulator_disable_loads[i]);
> +		if (ret) {
> +			DRM_DEV_ERROR(panel->dev,
> +				      "regulator_set_load failed %d\n", ret);
> +			return ret;
> +		}
> +	}
> +
> +	ret = regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> +	if (ret) {
> +		DRM_DEV_ERROR(panel->dev, "regulator_bulk_disable failed %d\n",
> +			      ret);
> +	}
> +	return ret;
> +}
> +
> +static int lg_panel_unprepare(struct drm_panel *panel)
> +{
> +	struct sw43408_panel *ctx = to_panel_info(panel);
> +	int ret, i;
> +
> +	if (!ctx->prepared)
> +		return 0;
> +
> +	ret = mipi_dsi_dcs_set_display_off(ctx->link);
> +	if (ret < 0) {
> +		DRM_DEV_ERROR(panel->dev,
> +			      "set_display_off cmd failed ret = %d\n", ret);
> +	}
> +
> +	msleep(120);
> +
> +	ret = mipi_dsi_dcs_enter_sleep_mode(ctx->link);
> +	if (ret < 0) {
> +		DRM_DEV_ERROR(panel->dev, "enter_sleep cmd failed ret = %d\n",
> +			      ret);
> +	}
> +
> +	/* Would call panel_power_off() */
And please do so.
If the reset code is not OK then comment it out in power_off(),
but do not duplicate all the regulator handling - it is very confusing
to read.
Same with the other commented out functions.

I did not look at the unprepare code - this will wait until next
iteration of the driver where the power supply stuff is cleaned up.

> +
> +	for (i = 0; i < ARRAY_SIZE(ctx->supplies); i++) {
> +		ret = regulator_set_load(ctx->supplies[i].consumer,
> +					 regulator_disable_loads[i]);
> +		if (ret) {
> +			DRM_DEV_ERROR(panel->dev,
> +				      "regulator_set_load failed %d\n", ret);
> +			return ret;
> +		}
> +	}
> +
> +	ctx->prepared = false;
> +
> +	return ret;
> +}
> +
> +static int lg_panel_prepare(struct drm_panel *panel)
> +{
> +	struct sw43408_panel *ctx = to_panel_info(panel);
> +	int err, i;
> +
> +	if (ctx->prepared)
> +		return 0;
> +
> +	/* Would call panel_reset() */
> +
> +	for (i = 0; i < ARRAY_SIZE(ctx->supplies); i++) {
> +		err = regulator_set_load(ctx->supplies[i].consumer,
> +					 regulator_enable_loads[i]);
> +		if (err)
> +			return err;
> +	}
> +
> +	err = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> +	if (err < 0)
> +		return err;
> +
> +	usleep_range(9000, 10000);
> +
> +	err = mipi_dsi_dcs_write(ctx->link, MIPI_DCS_SET_GAMMA_CURVE,
> +				 (u8[]){ 0x02 }, 1);
> +	if (err < 0) {
> +		DRM_DEV_ERROR(panel->dev, "failed to set gamma curve: %d\n",
> +			      err);
> +		goto poweroff;
> +	}
> +
> +	err = mipi_dsi_dcs_set_tear_on(ctx->link,
> +				       MIPI_DSI_DCS_TEAR_MODE_VBLANK);
> +	if (err < 0) {
> +		DRM_DEV_ERROR(panel->dev, "failed to set tear on: %d\n", err);
> +		goto poweroff;
> +	}
> +
> +	err = send_mipi_cmds(panel, &lg_sw43408_on_cmds_1[0]);
> +
> +	if (err < 0) {
> +		DRM_DEV_ERROR(panel->dev,
> +			      "failed to send DCS Init 1st Code: %d\n", err);
> +		goto poweroff;
> +	}
> +
> +	err = mipi_dsi_dcs_exit_sleep_mode(ctx->link);
> +	if (err < 0) {
> +		DRM_DEV_ERROR(panel->dev, "failed to exit sleep mode: %d\n",
> +			      err);
> +		goto poweroff;
> +	}
> +
> +	msleep(135);
> +
> +	err = mipi_dsi_dcs_write(ctx->link, MIPI_DSI_COMPRESSION_MODE,
> +				 (u8[]){ 0x11 }, 0);
> +	if (err < 0) {
> +		DRM_DEV_ERROR(panel->dev,
> +			      "failed to set compression mode: %d\n", err);
> +		goto poweroff;
> +	}
> +
> +	err = send_mipi_cmds(panel, &lg_sw43408_on_cmds_2[0]);
> +
> +	if (err < 0) {
> +		DRM_DEV_ERROR(panel->dev,
> +			      "failed to send DCS Init 2nd Code: %d\n", err);
> +		goto poweroff;
> +	}
> +
> +	err = mipi_dsi_dcs_set_display_on(ctx->link);
> +	if (err < 0) {
> +		DRM_DEV_ERROR(panel->dev, "failed to Set Display ON: %d\n",
> +			      err);
> +		goto poweroff;
> +	}
> +
> +	msleep(120);
> +
> +	ctx->prepared = true;
> +
> +	return 0;
> +
> +poweroff:
> +	gpiod_set_value(ctx->reset_gpio, 1);
> +	regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> +	return err;
> +}
> +
> +static int lg_panel_enable(struct drm_panel *panel)
> +{
> +	struct sw43408_panel *ctx = to_panel_info(panel);
> +	struct drm_dsc_picture_parameter_set pps;
> +	int ret;
> +
> +	if (ctx->enabled)
> +		return 0;
> +
> +	ret = backlight_enable(ctx->backlight);
> +	if (ret) {
> +		DRM_DEV_ERROR(panel->dev, "Failed to enable backlight %d\n",
> +			      ret);
> +		return ret;
> +	}
> +
> +	if (!panel->dsc) {
> +		DRM_DEV_ERROR(panel->dev, "Can't find DSC\n");
> +		return -ENODEV;
> +	}
> +
> +	drm_dsc_pps_payload_pack(&pps, panel->dsc);
There is nothing in disable about pps_payload - so they are not
symmetrical. Check if this is OK.

> +
> +	ctx->enabled = true;
> +
> +	return 0;
> +}
> +
> +static int lg_panel_get_modes(struct drm_panel *panel,
> +			      struct drm_connector *connector)
> +{
> +	struct sw43408_panel *ctx = to_panel_info(panel);
> +	const struct drm_display_mode *m = ctx->mode;
> +	struct drm_display_mode *mode;
> +
> +	mode = drm_mode_duplicate(connector->dev, m);
> +	if (!mode) {
> +		DRM_DEV_ERROR(panel->dev, "failed to add mode %ux%u\n",
> +			      m->hdisplay, m->vdisplay);
> +		return -ENOMEM;
> +	}
> +
> +	connector->display_info.width_mm = m->width_mm;
> +	connector->display_info.height_mm = m->height_mm;
> +
> +	drm_mode_set_name(mode);
> +	drm_mode_probed_add(connector, mode);
> +
> +	return 1;
> +}
> +
> +static int lg_panel_backlight_update_status(struct backlight_device *bl)
> +{
> +	struct mipi_dsi_device *dsi = bl_get_data(bl);
> +	int ret = 0;
> +	uint16_t brightness;
> +
> +	brightness = (uint16_t)backlight_get_brightness(bl);
> +	/* Brightness is sent in big-endian */
> +	brightness = cpu_to_be16(brightness);
This cpu_to_be16() looks wrong - the mipi_dsi_dcs_set_display_brightness() takes care.


> +
> +	ret = mipi_dsi_dcs_set_display_brightness(dsi, brightness);
> +	return ret;
> +}
> +
> +static int lg_panel_backlight_get_brightness(struct backlight_device *bl)
> +{
> +	struct mipi_dsi_device *dsi = bl_get_data(bl);
> +	int ret = 0;
> +	u16 brightness = 0;
> +
> +	ret = mipi_dsi_dcs_get_display_brightness(dsi, &brightness);
> +	if (ret < 0)
> +		return ret;
> +
> +	return brightness & 0xff;
The truncation to 8 bit is not needed unless the HW is buggy.
And since the brightness value is not adjusted, there is no need to add
the get_brightness() method, it will just return what was written
earlier anyway. Most panels get away without it.

> +}
> +
> +const struct backlight_ops lg_panel_backlight_ops = {
> +	.update_status = lg_panel_backlight_update_status,
> +	.get_brightness = lg_panel_backlight_get_brightness,
> +};
> +
> +static int lg_panel_backlight_init(struct sw43408_panel *ctx)
> +{
> +	struct device *dev = &ctx->link->dev;
> +	const struct backlight_properties props = {
> +		.type = BACKLIGHT_PLATFORM,
This is not a PLATFORM backlight, RAW it the right choice here.

> +		.brightness = 255,
> +		.max_brightness = 255,
Is 255 the correct max here? Check the HW specs.
> +	};
> +
> +	ctx->backlight = devm_backlight_device_register(dev, dev_name(dev), dev,
> +							ctx->link,
> +							&lg_panel_backlight_ops,
> +							&props);
Use panel->backlight here.

> +
> +	if (IS_ERR(ctx->backlight))
> +		return dev_err_probe(dev, PTR_ERR(ctx->backlight),
> +				     "Failed to create backlight\n");
> +
> +	return 0;
> +}
> +
> +static const struct drm_panel_funcs panel_funcs = {
> +	.disable = lg_panel_disable,
> +	.unprepare = lg_panel_unprepare,
> +	.prepare = lg_panel_prepare,
> +	.enable = lg_panel_enable,
> +	.get_modes = lg_panel_get_modes,
> +};
> +
> +static const struct drm_display_mode sw43408_default_mode = {
> +	.clock = 152340,
> +
> +	.hdisplay = 1080,
> +	.hsync_start = 1080 + 20,
> +	.hsync_end = 1080 + 20 + 32,
> +	.htotal = 1080 + 20 + 32 + 20,
> +
> +	.vdisplay = 2160,
> +	.vsync_start = 2160 + 20,
> +	.vsync_end = 2160 + 20 + 4,
> +	.vtotal = 2160 + 20 + 4 + 20,
> +
> +	.width_mm = 62,
> +	.height_mm = 124,
> +
> +	.type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED,
> +};
> +
> +static const struct of_device_id panel_of_match[] = {
> +	{ .compatible = "lg,sw43408", .data = &sw43408_default_mode },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, panel_of_match);
> +
> +static int panel_add(struct sw43408_panel *ctx)
> +{
> +	struct device *dev = &ctx->link->dev;
> +	int i, ret;
> +
> +	for (i = 0; i < ARRAY_SIZE(ctx->supplies); i++)
> +		ctx->supplies[i].supply = regulator_names[i];
> +
> +	ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(ctx->supplies),
> +				      ctx->supplies);
> +	if (ret < 0)
> +		return ret;
> +
> +	ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> +	if (IS_ERR(ctx->reset_gpio)) {
> +		DRM_DEV_ERROR(dev, "cannot get reset gpio %ld\n",
> +			      PTR_ERR(ctx->reset_gpio));
> +		return PTR_ERR(ctx->reset_gpio);
> +	}
Use dev_err_probe() here.

> +
> +	ret = lg_panel_backlight_init(ctx);
> +	if (ret < 0)
> +		return ret;
> +
> +	drm_panel_init(&ctx->base, dev, &panel_funcs, DRM_MODE_CONNECTOR_DSI);
Init backlight after panel init.

> +
> +	drm_panel_add(&ctx->base);
> +	return ret;
> +}
> +
> +static int panel_probe(struct mipi_dsi_device *dsi)
> +{
> +	struct sw43408_panel *ctx;
> +	struct drm_dsc_config *dsc;
> +	int err;
> +
> +	ctx = devm_kzalloc(&dsi->dev, sizeof(*ctx), GFP_KERNEL);
> +	if (!ctx)
> +		return -ENOMEM;
> +
> +	ctx->mode = of_device_get_match_data(&dsi->dev);
> +	dsi->mode_flags = MIPI_DSI_MODE_LPM;
> +	dsi->format = MIPI_DSI_FMT_RGB888;
> +	dsi->lanes = 4;
> +
> +	ctx->link = dsi;
> +	mipi_dsi_set_drvdata(dsi, ctx);
> +
> +	err = panel_add(ctx);
> +	if (err < 0)
> +		return err;
> +
> +	/* The panel is DSC panel only, set the dsc params */
> +	dsc = devm_kzalloc(&dsi->dev, sizeof(*dsc), GFP_KERNEL);
> +	if (!dsc)
> +		return -ENOMEM;
The drm_dsc_config could be embedded in sw43408_panel, so there was not
two allocations to track but with devm_kzalloc this is not a big win.

> +
> +	dsc->dsc_version_major = 0x1;
> +	dsc->dsc_version_minor = 0x1;
> +
> +	dsc->slice_height = 16;
> +	dsc->slice_width = 540;
> +	dsc->slice_count = 1;
> +	dsc->bits_per_component = 8;
> +	dsc->bits_per_pixel = 8;
> +	dsc->block_pred_enable = true;
> +
> +	ctx->base.dsc = dsc;
> +
> +	return mipi_dsi_attach(dsi);
> +}
> +
> +static int panel_remove(struct mipi_dsi_device *dsi)
> +{
> +	struct sw43408_panel *ctx = mipi_dsi_get_drvdata(dsi);
> +	int err;
> +
> +	err = lg_panel_unprepare(&ctx->base);
> +	if (err < 0)
> +		DRM_DEV_ERROR(&dsi->dev, "failed to unprepare panel: %d\n",
> +			      err);
Use the drm_panel() functions here, do not call your own.
See for example: y030xx067a_remove()

> +
> +	err = lg_panel_disable(&ctx->base);
> +	if (err < 0)
> +		DRM_DEV_ERROR(&dsi->dev, "failed to disable panel: %d\n", err);
> +
> +	err = mipi_dsi_detach(dsi);
> +	if (err < 0)
> +		DRM_DEV_ERROR(&dsi->dev, "failed to detach from DSI host: %d\n",
> +			      err);
mipi_dsi_detach() is needed here.

> +
> +	if (ctx->base.dev)
No need to check, the pointer must be valid.
> +		drm_panel_remove(&ctx->base);
> +
> +	return 0;
> +}
> +
> +static struct mipi_dsi_driver panel_driver = {
> +	.driver = {
> +		.name = "panel-lg-sw43408",
> +		.of_match_table = panel_of_match,
> +	},
> +	.probe = panel_probe,
> +	.remove = panel_remove,
> +};
> +module_mipi_dsi_driver(panel_driver);
> +
> +MODULE_AUTHOR("Sumit Semwal <sumit.semwal@linaro.org>");
> +MODULE_DESCRIPTION("LG SW436408 MIPI-DSI LED panel");
> +MODULE_LICENSE("GPL");
> -- 
> 2.36.1

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/4] dt-bindings: panel: Add LG SW43408 MIPI-DSI panel
  2022-07-19  6:10   ` Sam Ravnborg
@ 2022-07-19  7:57     ` Sam Ravnborg
  0 siblings, 0 replies; 18+ messages in thread
From: Sam Ravnborg @ 2022-07-19  7:57 UTC (permalink / raw)
  To: Caleb Connolly
  Cc: devicetree, David Airlie, linux-arm-msm, Konrad Dybcio,
	Andy Gross, Rob Herring, Bjorn Andersson, Vinod Koul,
	Thierry Reding, dri-devel, Krzysztof Kozlowski, phone-devel,
	Sumit Semwal, linux-kernel, ~postmarketos/upstreaming

Hi Caleb,

On Tue, Jul 19, 2022 at 08:10:18AM +0200, Sam Ravnborg wrote:
> Hi Caleb,
> 
> On Mon, Jul 18, 2022 at 10:30:50PM +0100, Caleb Connolly wrote:
> > From: Sumit Semwal <sumit.semwal@linaro.org>
> > 
> > LG SW43408 is 1080x2160, 4-lane MIPI-DSI panel.
> A few things to improve to this binding.
> 
> 	Sam
> > 
> > Signed-off-by: Vinod Koul <vkoul@kernel.org>
> > Signed-off-by: Sumit Semwal <sumit.semwal@linaro.org>
> > [caleb: convert to yaml]
> > Signed-off-by: Caleb Connolly <caleb@connolly.tech>
> > ---
> >  .../bindings/display/panel/lg,43408.yaml      | 41 +++++++++++++++++++
> >  .../display/panel/panel-simple-dsi.yaml       |  2 +
> >  2 files changed, 43 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/display/panel/lg,43408.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/display/panel/lg,43408.yaml b/Documentation/devicetree/bindings/display/panel/lg,43408.yaml
> > new file mode 100644
> > index 000000000000..0529a3aa2692
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/panel/lg,43408.yaml
> > @@ -0,0 +1,41 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/display/panel/panel-lvds.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: LG SW43408 1080x2160 DSI panel
> > +
> > +maintainers:
> > +  - Caleb Connolly <caleb@connolly.tech>
> > +
> > +description: |
> > +  This panel is used on the Pixel 3, it is a 60hz OLED panel which
> > +  required DSC (Display Stream Compression) and has rounded corners.
> > +
> > +allOf:
> > +  - $ref: panel-common.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    items:
> > +      - const: lg,sw43408
> > +
> > +  vddi-supply: true
> > +  vpnl-supply: true
> > +  reset-gpios: true
> > +
> > +  backlight: false
> > +  power-supply: false
> No need to say anything is false, this is covered by the statement below.
> Also, the driver uses backlight, so it should be true?
The driver do not use backlight from the DT so disregard the last
comment.

	Sam

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/4] Documentation: dt-bindings: arm: qcom: add google,blueline
  2022-07-18 21:30 ` [PATCH 1/4] Documentation: dt-bindings: arm: qcom: add google,blueline Caleb Connolly
@ 2022-07-19  8:36   ` Krzysztof Kozlowski
  0 siblings, 0 replies; 18+ messages in thread
From: Krzysztof Kozlowski @ 2022-07-19  8:36 UTC (permalink / raw)
  To: Caleb Connolly, Andy Gross, Bjorn Andersson, Konrad Dybcio,
	Rob Herring, Krzysztof Kozlowski, Thierry Reding, Sam Ravnborg,
	David Airlie, Daniel Vetter, Sumit Semwal, linux-arm-msm,
	devicetree, linux-kernel, dri-devel, phone-devel,
	~postmarketos/upstreaming

On 18/07/2022 23:30, Caleb Connolly wrote:
> Document the bindings for the Pixel 3
> 
> Based on https://lore.kernel.org/all/20220521164550.91115-7-krzysztof.kozlowski@linaro.org/

Thanks for mention dependency. However this should not go to the final
commit, thus please put such references after '---' marker.

With that change:
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

> 
> Signed-off-by: Caleb Connolly <caleb@connolly.tech>
> ---

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/4] arm64: dts: qcom: add sdm845-google-blueline (Pixel 3)
  2022-07-18 21:30 ` [PATCH 2/4] arm64: dts: qcom: add sdm845-google-blueline (Pixel 3) Caleb Connolly
  2022-07-18 22:13   ` Dmitry Baryshkov
@ 2022-07-19  8:45   ` Krzysztof Kozlowski
  1 sibling, 0 replies; 18+ messages in thread
From: Krzysztof Kozlowski @ 2022-07-19  8:45 UTC (permalink / raw)
  To: Caleb Connolly, Andy Gross, Bjorn Andersson, Konrad Dybcio,
	Rob Herring, Krzysztof Kozlowski, Thierry Reding, Sam Ravnborg,
	David Airlie, Daniel Vetter, Sumit Semwal, linux-arm-msm,
	devicetree, linux-kernel, dri-devel, phone-devel,
	~postmarketos/upstreaming
  Cc: Amit Pundir, Vinod Koul

On 18/07/2022 23:30, Caleb Connolly wrote:
> From: Amit Pundir <amit.pundir@linaro.org>
> 
> This adds an initial dts for the Blueline (Pixel 3). Supported
> functionality includes display, Debug UART, UFS, USB-C (peripheral), WiFi,
> Bluetooth and modem.
> 

Thank you for your patch. There is something to discuss/improve.

(...)

> +	volume-keys {
> +		compatible = "gpio-keys";
> +		label = "Volume keys";
> +		autorepeat;
> +
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&volume_up_gpio>;
> +
> +		vol-up {

key-vol-up
(DT schema requires it now)

> +			label = "Volume Up";
> +			linux,code = <KEY_VOLUMEUP>;
> +			gpios = <&pm8998_gpio 6 GPIO_ACTIVE_LOW>;
> +			debounce-interval = <15>;
> +		};
> +	};
> +
> +	reserved-memory {
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		ranges;
> +
> +		mpss_region: memory@8e000000 {
> +			reg = <0 0x8e000000 0 0x9800000>;
> +			no-map;
> +		};
> +
> +		venus_mem: venus@97800000 {
> +			reg = <0 0x97800000 0 0x500000>;
> +			no-map;
> +		};
> +
> +		cdsp_mem: cdsp-mem@97D00000 {
> +			reg = <0 0x97D00000 0 0x800000>;
> +			no-map;
> +		};
> +
> +		mba_region: mba@98500000 {
> +			reg = <0 0x98500000 0 0x200000>;
> +			no-map;
> +		};
> +
> +		slpi_mem: slpi@98700000 {
> +			reg = <0 0x98700000 0 0x1400000>;
> +			no-map;
> +		};
> +
> +		spss_mem: spss@99B00000 {
> +			reg = <0 0x99B00000 0 0x100000>;
> +			no-map;
> +		};
> +
> +		/* rmtfs lower guard */
> +		memory@f2700000 {
> +			reg = <0 0xf2700000 0 0x1000>;
> +			no-map;
> +		};
> +
> +		rmtfs_mem: memory@f2701000 {
> +			compatible = "qcom,rmtfs-mem";
> +			reg = <0 0xf2701000 0 0x200000>;
> +			no-map;
> +
> +			qcom,client-id = <1>;
> +			qcom,vmid = <15>;
> +		};
> +
> +		/* rmtfs upper guard */
> +		memory@f2901000 {
> +			reg = <0 0xf2901000 0 0x1000>;
> +			no-map;
> +		};
> +	};
> +
> +	vph_pwr: vph-pwr-regulator {
> +		compatible = "regulator-fixed";
> +		regulator-name = "vph_pwr";
> +		regulator-min-microvolt = <3700000>;
> +		regulator-max-microvolt = <3700000>;
> +	};
> +
> +	vreg_s4a_1p8: vreg-s4a-1p8 {

Please use consistent naming, so if previous was "xxx-regulator", keep
similar pattern here.

> +		compatible = "regulator-fixed";
> +		regulator-name = "vreg_s4a_1p8";
> +
> +		regulator-min-microvolt = <1800000>;
> +		regulator-max-microvolt = <1800000>;
> +		regulator-always-on;
> +		regulator-boot-on;
> +
> +		vin-supply = <&vph_pwr>;
> +	};
> +};
> +
> +&adsp_pas {

(...)

> +
> +&pm8998_gpio {
> +	volume_up_gpio: vol-up-active {

The bindings require node name to finish with "-state"

> +		pins = "gpio6";
> +		function = "normal";
> +		input-enable;
> +		bias-pull-up;
> +		qcom,drive-strength = <0>;
> +	};
> +
> +	panel_pmgpio_pins: panel-pmgpio-active {

Ditto.

> +		pins = "gpio2", "gpio5";
> +		function = "normal";
> +		input-enable;
> +		bias-disable;
> +		power-source = <0>;
> +	};
> +};
> +
> +&pm8998_pon {
> +	resin {
> +		compatible = "qcom,pm8941-resin";
> +		interrupts = <0x0 0x8 1 IRQ_TYPE_EDGE_BOTH>;
> +		debounce = <15625>;
> +		bias-pull-up;
> +		linux,code = <KEY_VOLUMEDOWN>;
> +	};
> +};
> +
Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/4] dt-bindings: panel: Add LG SW43408 MIPI-DSI panel
  2022-07-18 21:30 ` [PATCH 3/4] dt-bindings: panel: Add LG SW43408 MIPI-DSI panel Caleb Connolly
  2022-07-19  6:10   ` Sam Ravnborg
@ 2022-07-19 14:11   ` Rob Herring
  1 sibling, 0 replies; 18+ messages in thread
From: Rob Herring @ 2022-07-19 14:11 UTC (permalink / raw)
  To: Caleb Connolly
  Cc: Konrad Dybcio, Vinod Koul, David Airlie, Krzysztof Kozlowski,
	devicetree, dri-devel, Sam Ravnborg, Sumit Semwal, linux-arm-msm,
	linux-kernel, phone-devel, Andy Gross, ~postmarketos/upstreaming,
	Bjorn Andersson, Daniel Vetter, Rob Herring, Thierry Reding

On Mon, 18 Jul 2022 22:30:50 +0100, Caleb Connolly wrote:
> From: Sumit Semwal <sumit.semwal@linaro.org>
> 
> LG SW43408 is 1080x2160, 4-lane MIPI-DSI panel.
> 
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
> Signed-off-by: Sumit Semwal <sumit.semwal@linaro.org>
> [caleb: convert to yaml]
> Signed-off-by: Caleb Connolly <caleb@connolly.tech>
> ---
>  .../bindings/display/panel/lg,43408.yaml      | 41 +++++++++++++++++++
>  .../display/panel/panel-simple-dsi.yaml       |  2 +
>  2 files changed, 43 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/panel/lg,43408.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
./Documentation/devicetree/bindings/display/panel/lg,43408.yaml: $id: relative path/filename doesn't match actual path or filename
	expected: http://devicetree.org/schemas/display/panel/lg,43408.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/panel/lg,43408.yaml: duplicate '$id' value 'http://devicetree.org/schemas/display/panel/panel-lvds.yaml#'

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/4] arm64: dts: qcom: add sdm845-google-blueline (Pixel 3)
  2022-07-18 22:13   ` Dmitry Baryshkov
  2022-07-18 22:54     ` Caleb Connolly
@ 2022-07-27  8:47     ` Kalle Valo
  2022-07-27 10:55     ` Amit Pundir
  2 siblings, 0 replies; 18+ messages in thread
From: Kalle Valo @ 2022-07-27  8:47 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Caleb Connolly, Andy Gross, Bjorn Andersson, Konrad Dybcio,
	Rob Herring, Krzysztof Kozlowski, Thierry Reding, Sam Ravnborg,
	David Airlie, Daniel Vetter, Sumit Semwal, linux-arm-msm,
	devicetree, linux-kernel, dri-devel, phone-devel,
	~postmarketos/upstreaming, Amit Pundir, Vinod Koul, ath10k

+ ath10k list

Dmitry Baryshkov <dmitry.baryshkov@linaro.org> writes:

> On 19/07/2022 00:30, Caleb Connolly wrote:
>
>> From: Amit Pundir <amit.pundir@linaro.org>
>>
>> This adds an initial dts for the Blueline (Pixel 3). Supported
>> functionality includes display, Debug UART, UFS, USB-C (peripheral), WiFi,
>> Bluetooth and modem.
>>
>> Bootloader compatible board and msm IDs are needed for the kernel to boot
>> with Pixel3 bootloader, so those are added.
>>
>> GPIOs 0 through 3 and 81 through 84 are configured to not be accessible
>> from the application CPUs, so we mark them as reserved to allow the Pixel 3
>> to boot.
>>
>> The reserved-memory locations where obtained from downstream using
>> kernel logs:
>> https://gist.github.com/calebccff/090d10bfac3cb9e9bd98dda30b054c96
>>
>> The rmtfs region is allocated with UIO, making it technically "dynamic".
>> It's address and size can be read from sysfs:
>>
>> blueline:/ # cat /sys/class/uio/uio0/name
>> rmtfs
>> at /sys/class/uio/uio0/maps/map0/addr
>> 0x00000000f2701000
>> blueline:/ # cat /sys/class/uio/uio0/maps/map0/size
>> 0x0000000000200000
>>
>> Like the OnePlus 6, it needs 1kB reserved on either side of the rmtfs
>> memory to workaround some XPU bug which would otherwise cause erroneous
>> XPU violations when accessing the rmtfs_mem region.
>>
>> For wifi, the pixel 3 reports a board-id of 0xFF, and downstream
>> only includes a single bdwlan file. The qcom,ath10k-calibration-variant
>> property is set to ensure that the correct calibration data is used.
>>
>> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
>> [AmitP: Cherry-picked and refactored from Bjorn's db845c dts
>>          ("arm64: dts: qcom: Add Dragonboard 845c") https://lkml.org/lkml/2019/6/6/7]
>> Signed-off-by: Amit Pundir <amit.pundir@linaro.org>
>> [sumits: merged commits to add board and msm ids, gpio range reservation,
>>    ufs device-reset gpio and adaptation to v5.5+ changes]
>> Signed-off-by: Sumit Semwal <sumit.semwal@linaro.org>
>> [vinod: Add display nodes]
>> Signed-off-by: Vinod Koul <vkoul@kernel.org>
>> [caleb: remove db845c bits, cleanup, add reserved-memory for modem/wifi]
>> Signed-off-by: Caleb Connolly <caleb@connolly.tech>
>
> Thanks for your patch, few minor items to improve.

[...]

>> +&wifi {
>> +	status = "okay";
>> +
>> +	vdd-0.8-cx-mx-supply = <&vreg_l5a_0p8>;
>> +	vdd-1.8-xo-supply = <&vreg_l7a_1p8>;
>> +	vdd-1.3-rfa-supply = <&vreg_l17a_1p3>;
>> +	vdd-3.3-ch0-supply = <&vreg_l25a_3p3>;
>> +
>> +	qcom,snoc-host-cap-8bit-quirk;
>> +	qcom,ath10k-calibration-variant = "google_blueline";
>
> Ideally Kalle Valo should bless this string, added him to the Cc list.
> Could you please submit the board file to the ath10k (see [1] for the
> description and [2] for an example).

Thanks for CC. I prefer "google-blueline" but that's really a cosmetic
issue.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/4] arm64: dts: qcom: add sdm845-google-blueline (Pixel 3)
  2022-07-18 22:13   ` Dmitry Baryshkov
  2022-07-18 22:54     ` Caleb Connolly
  2022-07-27  8:47     ` Kalle Valo
@ 2022-07-27 10:55     ` Amit Pundir
  2 siblings, 0 replies; 18+ messages in thread
From: Amit Pundir @ 2022-07-27 10:55 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Caleb Connolly, Andy Gross, Bjorn Andersson, Konrad Dybcio,
	Rob Herring, Krzysztof Kozlowski, Thierry Reding, Sam Ravnborg,
	David Airlie, Daniel Vetter, Sumit Semwal, linux-arm-msm,
	devicetree, linux-kernel, dri-devel, phone-devel,
	~postmarketos/upstreaming, Kalle Valo, Vinod Koul

n Tue, 19 Jul 2022 at 03:43, Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On 19/07/2022 00:30, Caleb Connolly wrote:
> > From: Amit Pundir <amit.pundir@linaro.org>
> >
> > This adds an initial dts for the Blueline (Pixel 3). Supported
> > functionality includes display, Debug UART, UFS, USB-C (peripheral), WiFi,
> > Bluetooth and modem.
> >
> > Bootloader compatible board and msm IDs are needed for the kernel to boot
> > with Pixel3 bootloader, so those are added.
> >
> > GPIOs 0 through 3 and 81 through 84 are configured to not be accessible
> > from the application CPUs, so we mark them as reserved to allow the Pixel 3
> > to boot.
> >
> > The reserved-memory locations where obtained from downstream using
> > kernel logs:
> > https://gist.github.com/calebccff/090d10bfac3cb9e9bd98dda30b054c96
> >
> > The rmtfs region is allocated with UIO, making it technically "dynamic".
> > It's address and size can be read from sysfs:
> >
> > blueline:/ # cat /sys/class/uio/uio0/name
> > rmtfs
> > at /sys/class/uio/uio0/maps/map0/addr
> > 0x00000000f2701000
> > blueline:/ # cat /sys/class/uio/uio0/maps/map0/size
> > 0x0000000000200000
> >
> > Like the OnePlus 6, it needs 1kB reserved on either side of the rmtfs
> > memory to workaround some XPU bug which would otherwise cause erroneous
> > XPU violations when accessing the rmtfs_mem region.
> >
> > For wifi, the pixel 3 reports a board-id of 0xFF, and downstream
> > only includes a single bdwlan file. The qcom,ath10k-calibration-variant
> > property is set to ensure that the correct calibration data is used.
> >
> > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > [AmitP: Cherry-picked and refactored from Bjorn's db845c dts
> >          ("arm64: dts: qcom: Add Dragonboard 845c") https://lkml.org/lkml/2019/6/6/7]
> > Signed-off-by: Amit Pundir <amit.pundir@linaro.org>
> > [sumits: merged commits to add board and msm ids, gpio range reservation,
> >    ufs device-reset gpio and adaptation to v5.5+ changes]
> > Signed-off-by: Sumit Semwal <sumit.semwal@linaro.org>
> > [vinod: Add display nodes]
> > Signed-off-by: Vinod Koul <vkoul@kernel.org>
> > [caleb: remove db845c bits, cleanup, add reserved-memory for modem/wifi]
> > Signed-off-by: Caleb Connolly <caleb@connolly.tech>
>
> Thanks for your patch, few minor items to improve.
>
> > ---
> >   arch/arm64/boot/dts/qcom/Makefile             |   1 +
> >   .../boot/dts/qcom/sdm845-google-blueline.dts  | 652 ++++++++++++++++++
> >   2 files changed, 653 insertions(+)
> >   create mode 100644 arch/arm64/boot/dts/qcom/sdm845-google-blueline.dts
> >
> > diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
> > index 2f8aec2cc6db..c151e17e6eb7 100644
> > --- a/arch/arm64/boot/dts/qcom/Makefile
> > +++ b/arch/arm64/boot/dts/qcom/Makefile
> > @@ -100,6 +100,7 @@ dtb-$(CONFIG_ARCH_QCOM)   += sdm845-cheza-r1.dtb
> >   dtb-$(CONFIG_ARCH_QCOM)     += sdm845-cheza-r2.dtb
> >   dtb-$(CONFIG_ARCH_QCOM)     += sdm845-cheza-r3.dtb
> >   dtb-$(CONFIG_ARCH_QCOM)     += sdm845-db845c.dtb
> > +dtb-$(CONFIG_ARCH_QCOM)      += sdm845-google-blueline.dtb
> >   dtb-$(CONFIG_ARCH_QCOM)     += sdm845-mtp.dtb
> >   dtb-$(CONFIG_ARCH_QCOM)     += sdm845-oneplus-enchilada.dtb
> >   dtb-$(CONFIG_ARCH_QCOM)     += sdm845-oneplus-fajita.dtb
> > diff --git a/arch/arm64/boot/dts/qcom/sdm845-google-blueline.dts b/arch/arm64/boot/dts/qcom/sdm845-google-blueline.dts
> > new file mode 100644
> > index 000000000000..dec979ad9209
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/qcom/sdm845-google-blueline.dts
> > @@ -0,0 +1,652 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +/dts-v1/;
> > +
> > +#include <dt-bindings/dma/qcom-gpi.h>
> > +#include <dt-bindings/input/linux-event-codes.h>
> > +#include <dt-bindings/interrupt-controller/irq.h>
> > +#include <dt-bindings/regulator/qcom,rpmh-regulator.h>
> > +
> > +#include "sdm845.dtsi"
> > +#include "pm8998.dtsi"
> > +#include "pmi8998.dtsi"
> > +
> > +/delete-node/ &mpss_region;
> > +/delete-node/ &venus_mem;
> > +/delete-node/ &cdsp_mem;
> > +/delete-node/ &mba_region;
> > +/delete-node/ &slpi_mem;
> > +/delete-node/ &spss_mem;
> > +/delete-node/ &rmtfs_mem;
> > +
> > +/ {
> > +     model = "Google Pixel 3";
> > +     compatible = "google,blueline", "qcom,sdm845";
> > +     qcom,board-id = <0x00021505 0>;
> > +     qcom,msm-id = <321 0x20001>;
> > +
> > +     aliases {
> > +             serial0 = &uart9;
> > +             serial1 = &uart6;
> > +     };
> > +
> > +     chosen {
> > +             stdout-path = "serial0:115200n8";
> > +     };
> > +
> > +     volume-keys {
> > +             compatible = "gpio-keys";
> > +             label = "Volume keys";
> > +             autorepeat;
> > +
> > +             pinctrl-names = "default";
> > +             pinctrl-0 = <&volume_up_gpio>;
> > +
> > +             vol-up {
> > +                     label = "Volume Up";
> > +                     linux,code = <KEY_VOLUMEUP>;
> > +                     gpios = <&pm8998_gpio 6 GPIO_ACTIVE_LOW>;
> > +                     debounce-interval = <15>;
> > +             };
> > +     };
> > +
> > +     reserved-memory {
> > +             #address-cells = <2>;
> > +             #size-cells = <2>;
> > +             ranges;
>
> These properties are already part of the sdm845.dtsi, so no need to have
> them here.
>
> > +
> > +             mpss_region: memory@8e000000 {
> > +                     reg = <0 0x8e000000 0 0x9800000>;
> > +                     no-map;
> > +             };
> > +
> > +             venus_mem: venus@97800000 {
> > +                     reg = <0 0x97800000 0 0x500000>;
> > +                     no-map;
> > +             };
> > +
> > +             cdsp_mem: cdsp-mem@97D00000 {
> > +                     reg = <0 0x97D00000 0 0x800000>;
> > +                     no-map;
> > +             };
> > +
> > +             mba_region: mba@98500000 {
> > +                     reg = <0 0x98500000 0 0x200000>;
> > +                     no-map;
> > +             };
> > +
> > +             slpi_mem: slpi@98700000 {
> > +                     reg = <0 0x98700000 0 0x1400000>;
> > +                     no-map;
> > +             };
> > +
> > +             spss_mem: spss@99B00000 {
> > +                     reg = <0 0x99B00000 0 0x100000>;
> > +                     no-map;
> > +             };
> > +
> > +             /* rmtfs lower guard */
> > +             memory@f2700000 {
> > +                     reg = <0 0xf2700000 0 0x1000>;
> > +                     no-map;
> > +             };
> > +
> > +             rmtfs_mem: memory@f2701000 {
> > +                     compatible = "qcom,rmtfs-mem";
> > +                     reg = <0 0xf2701000 0 0x200000>;
> > +                     no-map;
> > +
> > +                     qcom,client-id = <1>;
> > +                     qcom,vmid = <15>;
> > +             };
> > +
> > +             /* rmtfs upper guard */
> > +             memory@f2901000 {
> > +                     reg = <0 0xf2901000 0 0x1000>;
> > +                     no-map;
> > +             };
> > +     };
> > +
> > +     vph_pwr: vph-pwr-regulator {
> > +             compatible = "regulator-fixed";
> > +             regulator-name = "vph_pwr";
> > +             regulator-min-microvolt = <3700000>;
> > +             regulator-max-microvolt = <3700000>;
> > +     };
> > +
> > +     vreg_s4a_1p8: vreg-s4a-1p8 {
> > +             compatible = "regulator-fixed";
> > +             regulator-name = "vreg_s4a_1p8";
> > +
> > +             regulator-min-microvolt = <1800000>;
> > +             regulator-max-microvolt = <1800000>;
> > +             regulator-always-on;
> > +             regulator-boot-on;
> > +
> > +             vin-supply = <&vph_pwr>;
> > +     };
> > +};
> > +
> > +&adsp_pas {
> > +     status = "okay";
> > +
> > +     firmware-name = "qcom/sdm845/pixel3/adsp.mbn";
>
> What about using "qcom/sdm845/blueline/adsp.mbn" instead?
>
> Bjorn, Amit?

qcom/sdm845/blueline/adsp.mbn seems more apt.

>
> > +};
> > +
> > +&apps_rsc {
> > +     pm8998-rpmh-regulators {
> > +             compatible = "qcom,pm8998-rpmh-regulators";
> > +             qcom,pmic-id = "a";
> > +             vdd-s1-supply = <&vph_pwr>;
> > +             vdd-s2-supply = <&vph_pwr>;
> > +             vdd-s3-supply = <&vph_pwr>;
> > +             vdd-s4-supply = <&vph_pwr>;
> > +             vdd-s5-supply = <&vph_pwr>;
> > +             vdd-s6-supply = <&vph_pwr>;
> > +             vdd-s7-supply = <&vph_pwr>;
> > +             vdd-s8-supply = <&vph_pwr>;
> > +             vdd-s9-supply = <&vph_pwr>;
> > +             vdd-s10-supply = <&vph_pwr>;
> > +             vdd-s11-supply = <&vph_pwr>;
> > +             vdd-s12-supply = <&vph_pwr>;
> > +             vdd-s13-supply = <&vph_pwr>;
> > +             vdd-l1-l27-supply = <&vreg_s7a_1p025>;
> > +             vdd-l2-l8-l17-supply = <&vreg_s3a_1p35>;
> > +             vdd-l3-l11-supply = <&vreg_s7a_1p025>;
> > +             vdd-l4-l5-supply = <&vreg_s7a_1p025>;
> > +             vdd-l6-supply = <&vph_pwr>;
> > +             vdd-l7-l12-l14-l15-supply = <&vreg_s5a_2p04>;
> > +             vdd-l9-supply = <&vreg_bob>;
> > +             vdd-l10-l23-l25-supply = <&vreg_bob>;
> > +             vdd-l13-l19-l21-supply = <&vreg_bob>;
> > +             vdd-l16-l28-supply = <&vreg_bob>;
> > +             vdd-l18-l22-supply = <&vreg_bob>;
> > +             vdd-l20-l24-supply = <&vreg_bob>;
> > +             vdd-l26-supply = <&vreg_s3a_1p35>;
> > +             vin-lvs-1-2-supply = <&vreg_s4a_1p8>;
> > +
> > +             vreg_s3a_1p35: smps3 {
> > +                     regulator-min-microvolt = <1352000>;
> > +                     regulator-max-microvolt = <1352000>;
> > +             };
> > +
> > +             vreg_s5a_2p04: smps5 {
> > +                     regulator-min-microvolt = <1904000>;
> > +                     regulator-max-microvolt = <2040000>;
> > +             };
> > +
> > +             vreg_s7a_1p025: smps7 {
> > +                     regulator-min-microvolt = <900000>;
> > +                     regulator-max-microvolt = <1028000>;
> > +             };
> > +
> > +             vdda_mipi_dsi0_pll:
>
> Do we need this alias?
>
> > +             vreg_l1a_0p875: ldo1 {
> > +                     regulator-min-microvolt = <880000>;
> > +                     regulator-max-microvolt = <880000>;
> > +                     regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> > +                     regulator-boot-on;
> > +             };
> > +
> > +             vreg_l5a_0p8: ldo5 {
> > +                     regulator-min-microvolt = <800000>;
> > +                     regulator-max-microvolt = <800000>;
> > +                     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_l7a_1p8: ldo7 {
> > +                     regulator-min-microvolt = <1800000>;
> > +                     regulator-max-microvolt = <1800000>;
> > +                     regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> > +             };
> > +
> > +             vreg_l13a_2p95: ldo13 {
> > +                     regulator-min-microvolt = <1800000>;
> > +                     regulator-max-microvolt = <2960000>;
> > +                     regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> > +             };
> > +
> > +             vreg_l14a_1p88: ldo14 {
> > +                     regulator-min-microvolt = <1800000>;
> > +                     regulator-max-microvolt = <1800000>;
> > +                     regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> > +                     regulator-boot-on;
> > +                     /*
> > +                      * We can't properly bring the panel back if it gets turned off
> > +                      * so keep it's regulators always on for now.
> > +                      */
>
> Any idea, what is the issue here? Do you have the datasheet for the panel?
>
> > +                     regulator-always-on;
> > +             };
> > +
> > +             vreg_l17a_1p3: ldo17 {
> > +                     regulator-min-microvolt = <1304000>;
> > +                     regulator-max-microvolt = <1304000>;
> > +                     regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> > +             };
> > +
> > +             vreg_l19a_3p3: ldo19 {
> > +                     regulator-min-microvolt = <3300000>;
> > +                     regulator-max-microvolt = <3312000>;
> > +                     qcom,init-voltage = <3300000>;
> > +                     qcom,initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> > +                     /*
> > +                      * The touchscreen needs this to be 3.3v, which is apparently
> > +                      * quite close to the hardware limit for this LDO (3.312v)
> > +                      * It must be kept in high power mode to prevent TS brownouts
> > +                      */
> > +                     regulator-allowed-modes = <RPMH_REGULATOR_MODE_HPM>;
> > +             };
> > +
> > +             vreg_l20a_2p95: ldo20 {
> > +                     regulator-min-microvolt = <2960000>;
> > +                     regulator-max-microvolt = <2968000>;
> > +                     regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> > +             };
> > +
> > +             vreg_l21a_2p95: ldo21 {
> > +                     regulator-min-microvolt = <2960000>;
> > +                     regulator-max-microvolt = <2968000>;
> > +                     regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> > +             };
> > +
> > +             vreg_l24a_3p075: ldo24 {
> > +                     regulator-min-microvolt = <3088000>;
> > +                     regulator-max-microvolt = <3088000>;
> > +                     regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> > +             };
> > +
> > +             vreg_l25a_3p3: ldo25 {
> > +                     regulator-min-microvolt = <3300000>;
> > +                     regulator-max-microvolt = <3312000>;
> > +                     regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> > +             };
> > +
> > +             vdda_mipi_dsi0_1p2:
>
> Do we need this alias?
>
> > +             vreg_l26a_1p2: ldo26 {
> > +                     regulator-min-microvolt = <1200000>;
> > +                     regulator-max-microvolt = <1200000>;
> > +                     regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> > +                     regulator-boot-on;
> > +             };
> > +
> > +             vreg_l28a_3p0: ldo28 {
> > +                     regulator-min-microvolt = <2856000>;
> > +                     regulator-max-microvolt = <3008000>;
> > +                     regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> > +                     regulator-boot-on;
> > +                     /*
> > +                      * We can't properly bring the panel back if it gets turned off
> > +                      * so keep it's regulators always on for now.
> > +                      */
> > +                     regulator-always-on;
> > +             };
> > +     };
> > +
> > +     pmi8998-rpmh-regulators {
> > +             compatible = "qcom,pmi8998-rpmh-regulators";
> > +             qcom,pmic-id = "b";
> > +
> > +             vdd-bob-supply = <&vph_pwr>;
> > +
> > +             vreg_bob: bob {
> > +                     regulator-min-microvolt = <3312000>;
> > +                     regulator-max-microvolt = <3600000>;
> > +                     regulator-initial-mode = <RPMH_REGULATOR_MODE_AUTO>;
> > +                     regulator-allow-bypass;
> > +             };
> > +     };
> > +
> > +     pm8005-rpmh-regulators {
> > +             compatible = "qcom,pm8005-rpmh-regulators";
> > +             qcom,pmic-id = "c";
> > +
> > +             vdd-s1-supply = <&vph_pwr>;
> > +             vdd-s2-supply = <&vph_pwr>;
> > +             vdd-s3-supply = <&vph_pwr>;
> > +             vdd-s4-supply = <&vph_pwr>;
> > +
> > +             vreg_s3c_0p6: smps3 {
> > +                     regulator-min-microvolt = <600000>;
> > +                     regulator-max-microvolt = <600000>;
> > +             };
> > +     };
> > +};
> > +
> > +&cdsp_pas {
> > +     status = "okay";
> > +     firmware-name = "qcom/sdm845/pixel3/cdsp.mbn";
> > +};
> > +
> > +&dsi0 {
> > +     status = "okay";
> > +     vdda-supply = <&vdda_mipi_dsi0_1p2>;
> > +
> > +     ports {
> > +             port@1 {
> > +                     endpoint {
> > +                             remote-endpoint = <&lg_sw43408_in_0>;
> > +                             data-lanes = <0 1 2 3>;
> > +                     };
> > +             };
> > +     };
> > +
> > +     panel@0 {
> > +             compatible = "lg,sw43408";
> > +             reg = <0>;
> > +
> > +             vddi-supply = <&vreg_l14a_1p88>;
> > +             vpnl-supply = <&vreg_l28a_3p0>;
> > +
> > +             reset-gpios = <&tlmm 6 GPIO_ACTIVE_LOW>;
> > +
> > +             pinctrl-names = "default";
> > +             pinctrl-0 = <&panel_reset_pins &panel_te_pin &panel_pmgpio_pins>;
> > +
> > +             ports {
> > +                     #address-cells = <1>;
> > +                     #size-cells = <0>;
> > +
> > +                     port@0 {
> > +                             reg = <0>;
> > +                             lg_sw43408_in_0: endpoint {
> > +                                     remote-endpoint = <&dsi0_out>;
> > +                             };
> > +                     };
> > +             };
> > +     };
> > +};
> > +
> > +&dsi0_out {
> > +     remote-endpoint = <&lg_sw43408_in_0>;
> > +     data-lanes = <0 1 2 3>;
> > +};
> > +
> > +&dsi0_phy {
> > +     status = "okay";
> > +     vdds-supply = <&vdda_mipi_dsi0_pll>;
> > +};
> > +
> > +&gcc {
> > +     protected-clocks = <GCC_QSPI_CORE_CLK>,
> > +                        <GCC_QSPI_CORE_CLK_SRC>,
> > +                        <GCC_QSPI_CNOC_PERIPH_AHB_CLK>;
> > +};
> > +
> > +&gmu {
> > +     status = "okay";
> > +};
> > +
> > +&gpi_dma0 {
> > +     status = "okay";
> > +};
> > +
> > +&gpu {
> > +     status = "okay";
> > +
> > +     zap-shader {
> > +             memory-region = <&gpu_mem>;
> > +             firmware-name = "qcom/sdm845/pixel3/a630_zap.mbn";
> > +     };
> > +};
> > +
> > +&ipa {
> > +     status = "okay";
> > +
> > +     firmware-name = "qcom/sdm845/pixel3/ipa_fws.mbn";
> > +};
> > +
> > +&mss_pil {
> > +     status = "okay";
> > +     firmware-name = "qcom/sdm845/pixel3/mba.mbn", "qcom/sdm845/pixel3/modem.mbn";
> > +};
> > +
> > +&mdss {
> > +     status = "okay";
> > +};
> > +
> > +&mdss_mdp {
> > +     status = "okay";
> > +};
>
> Not necessary, it is a default state since the commit 4a5622c1d975
> ("arm64: dts: qcom: sdm845: Don't disable MDP explicitly")
>
> > +
> > +&pm8998_gpio {
> > +     volume_up_gpio: vol-up-active {
> > +             pins = "gpio6";
> > +             function = "normal";
> > +             input-enable;
> > +             bias-pull-up;
> > +             qcom,drive-strength = <0>;
> > +     };
> > +
> > +     panel_pmgpio_pins: panel-pmgpio-active {
> > +             pins = "gpio2", "gpio5";
> > +             function = "normal";
> > +             input-enable;
> > +             bias-disable;
> > +             power-source = <0>;
> > +     };
> > +};
> > +
> > +&pm8998_pon {
> > +     resin {
> > +             compatible = "qcom,pm8941-resin";
> > +             interrupts = <0x0 0x8 1 IRQ_TYPE_EDGE_BOTH>;
> > +             debounce = <15625>;
> > +             bias-pull-up;
> > +             linux,code = <KEY_VOLUMEDOWN>;
> > +     };
>
> Please move the (disabled, labelled) resin device to pm8998.dtsi and
> just add status = "okay" here.
>
> > +};
> > +
> > +&qupv3_id_0 {
> > +     status = "okay";
> > +};
> > +
> > +&qupv3_id_1 {
> > +     status = "okay";
> > +};
> > +
> > +&qup_uart6_default {
> > +     pinmux {
> > +             pins = "gpio45", "gpio46", "gpio47", "gpio48";
> > +             function = "qup6";
> > +     };
> > +
> > +     cts {
> > +             pins = "gpio45";
> > +             bias-disable;
> > +     };
> > +
> > +     rts-tx {
> > +             pins = "gpio46", "gpio47";
> > +             drive-strength = <2>;
> > +             bias-disable;
> > +     };
> > +
> > +     rx {
> > +             pins = "gpio48";
> > +             bias-pull-up;
> > +     };
> > +};
> > +
> > +&qup_uart9_default {
> > +     pinconf-tx {
> > +             pins = "gpio4";
> > +             drive-strength = <2>;
> > +             bias-disable;
> > +     };
> > +
> > +     pinconf-rx {
> > +             pins = "gpio5";
> > +             drive-strength = <2>;
> > +             bias-pull-up;
> > +     };
> > +};
> > +
> > +&tlmm {
> > +     gpio-reserved-ranges = <0 4>, <81 4>;
> > +
> > +     panel_te_pin: panel-te {
> > +             mux {
> > +                     pins = "gpio12";
> > +                     function = "mdp_vsync";
> > +                     drive-strength = <2>;
> > +                     bias-disable;
> > +                     input-enable;
> > +             };
> > +     };
> > +
> > +     panel_reset_pins: panel-active {
> > +             mux {
> > +                     pins = "gpio6", "gpio52";
> > +                     function = "gpio";
> > +                     drive-strength = <8>;
> > +                     bias-disable = <0>;
> > +             };
> > +     };
> > +
> > +     panel_suspend: panel-suspend {
> > +             mux {
> > +                     pins = "gpio6", "gpio52";
> > +                     function = "gpio";
> > +                     drive-strength = <2>;
> > +                     bias-pull-down;
> > +             };
> > +     };
> > +
> > +     touchscreen_reset: ts-reset {
> > +             mux {
> > +                     pins = "gpio99";
> > +                     function = "gpio";
> > +                     drive-strength = <8>;
> > +                     bias-pull-up;
> > +                     //output-high;
>
> debug, can be removed?
>
> > +             };
> > +     };
> > +
> > +     touchscreen_pins: ts-pins {
> > +             mux {
> > +                     pins = "gpio125";
> > +                     function = "gpio";
> > +                     drive-strength = <2>;
> > +                     bias-disable;
> > +             };
> > +     };
> > +
> > +     touchscreen_i2c_pins: qup-i2c2-gpio {
> > +             pins = "gpio27", "gpio28";
> > +             function = "gpio";
> > +
> > +             drive-strength = <2>;
> > +             bias-disable;
> > +     };
> > +};
> > +
> > +&uart6 {
> > +     status = "okay";
> > +
> > +     bluetooth {
> > +             compatible = "qcom,wcn3990-bt";
> > +
> > +             vddio-supply = <&vreg_s4a_1p8>;
> > +             vddxo-supply = <&vreg_l7a_1p8>;
> > +             vddrf-supply = <&vreg_l17a_1p3>;
> > +             vddch0-supply = <&vreg_l25a_3p3>;
> > +             max-speed = <3200000>;
> > +     };
> > +};
> > +
> > +&uart9 {
> > +     status = "okay";
> > +};
> > +
> > +&usb_1 {
> > +     status = "okay";
> > +};
> > +
> > +&usb_1_dwc3 {
> > +     dr_mode = "peripheral";
> > +};
> > +
> > +&usb_1_hsphy {
> > +     status = "okay";
> > +
> > +     vdd-supply = <&vreg_l1a_0p875>;
> > +     vdda-pll-supply = <&vreg_l12a_1p8>;
> > +     vdda-phy-dpdm-supply = <&vreg_l24a_3p075>;
> > +
> > +     qcom,imp-res-offset-value = <8>;
> > +     qcom,hstx-trim-value = <QUSB2_V2_HSTX_TRIM_21_6_MA>;
> > +     qcom,preemphasis-level = <QUSB2_V2_PREEMPHASIS_5_PERCENT>;
> > +     qcom,preemphasis-width = <QUSB2_V2_PREEMPHASIS_WIDTH_HALF_BIT>;
> > +};
> > +
> > +&usb_1_qmpphy {
> > +     status = "okay";
> > +
> > +     vdda-phy-supply = <&vreg_l26a_1p2>;
> > +     vdda-pll-supply = <&vreg_l1a_0p875>;
> > +};
> > +
> > +&usb_2 {
> > +     status = "okay";
> > +};
> > +
> > +&usb_2_dwc3 {
> > +     dr_mode = "host";
> > +};
> > +
> > +&usb_2_hsphy {
> > +     status = "okay";
> > +
> > +     vdd-supply = <&vreg_l1a_0p875>;
> > +     vdda-pll-supply = <&vreg_l12a_1p8>;
> > +     vdda-phy-dpdm-supply = <&vreg_l24a_3p075>;
> > +
> > +     qcom,imp-res-offset-value = <8>;
> > +     qcom,hstx-trim-value = <QUSB2_V2_HSTX_TRIM_22_8_MA>;
> > +};
> > +
> > +&usb_2_qmpphy {
> > +     status = "okay";
> > +
> > +     vdda-phy-supply = <&vreg_l26a_1p2>;
> > +     vdda-pll-supply = <&vreg_l1a_0p875>;
> > +};
> > +
> > +&ufs_mem_hc {
> > +     status = "okay";
> > +
> > +     reset-gpios = <&tlmm 150 GPIO_ACTIVE_LOW>;
> > +
> > +     vcc-supply = <&vreg_l20a_2p95>;
> > +     vcc-max-microamp = <800000>;
> > +};
> > +
> > +&ufs_mem_phy {
> > +     status = "okay";
> > +
> > +     vdda-phy-supply = <&vreg_l1a_0p875>;
> > +     vdda-pll-supply = <&vreg_l26a_1p2>;
> > +};
> > +
> > +&venus {
> > +     status = "okay";
> > +     firmware-name = "qcom/sdm845/oneplus6/venus.mbn";
>
> Why are you using the oneplus6 here?
>
> > +};
> > +
> > +&wifi {
> > +     status = "okay";
> > +
> > +     vdd-0.8-cx-mx-supply = <&vreg_l5a_0p8>;
> > +     vdd-1.8-xo-supply = <&vreg_l7a_1p8>;
> > +     vdd-1.3-rfa-supply = <&vreg_l17a_1p3>;
> > +     vdd-3.3-ch0-supply = <&vreg_l25a_3p3>;
> > +
> > +     qcom,snoc-host-cap-8bit-quirk;
> > +     qcom,ath10k-calibration-variant = "google_blueline";
>
> Ideally Kalle Valo should bless this string, added him to the Cc list.
> Could you please submit the board file to the ath10k (see [1] for the
> description and [2] for an example).
>
>
> [1] https://wireless.wiki.kernel.org/en/users/drivers/ath10k/boardfiles
> [2]
> https://lore.kernel.org/ath10k/CAA8EJpphUrxr5gtW0=-tZh-DrKXmHkfFxWMvYRpTUGuCesGCbw@mail.gmail.com/T/#u
>
> > +};
>
>
> --
> With best wishes
> Dmitry

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 4/4] drm: panel: Add lg sw43408 panel driver
  2022-07-18 21:30 ` [PATCH 4/4] drm: panel: Add lg sw43408 panel driver Caleb Connolly
  2022-07-18 23:06   ` Dmitry Baryshkov
  2022-07-19  7:48   ` Sam Ravnborg
@ 2022-10-01 21:48   ` Marijn Suijten
  2 siblings, 0 replies; 18+ messages in thread
From: Marijn Suijten @ 2022-10-01 21:48 UTC (permalink / raw)
  To: Caleb Connolly
  Cc: Andy Gross, Bjorn Andersson, Konrad Dybcio, Rob Herring,
	Krzysztof Kozlowski, Thierry Reding, Sam Ravnborg, David Airlie,
	Daniel Vetter, Sumit Semwal, linux-arm-msm, devicetree,
	linux-kernel, dri-devel, phone-devel, ~postmarketos/upstreaming,
	Vinod Koul

On 2022-07-18 22:30:51, Caleb Connolly wrote:
> From: Sumit Semwal <sumit.semwal@linaro.org>
> 
> LG SW43408 is 1080x2160, 4-lane MIPI-DSI panel, used in some Pixel3
> phones.
> 
> Whatever init sequence we have for this panel isn't capable of
> initialising it completely, toggling the reset gpio ever causes the
> panel to die. Until this is resolved we avoid resetting the panel. The
> disable/unprepare functions only put the panel to sleep mode and
> disable the backlight.
> 
> Signed-off-by: Sumit Semwal <sumit.semwal@linaro.org>
> [vinod: Add DSC support]
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
> [caleb: cleanup and support turning off the panel]
> Signed-off-by: Caleb Connolly <caleb@connolly.tech>
> ---
>  MAINTAINERS                              |   8 +
>  drivers/gpu/drm/panel/Kconfig            |  11 +
>  drivers/gpu/drm/panel/Makefile           |   1 +
>  drivers/gpu/drm/panel/panel-lg-sw43408.c | 586 +++++++++++++++++++++++
>  4 files changed, 606 insertions(+)
>  create mode 100644 drivers/gpu/drm/panel/panel-lg-sw43408.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f679152bdbad..8a2b954ad140 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6376,6 +6376,14 @@ S:	Orphan / Obsolete
>  F:	drivers/gpu/drm/i810/
>  F:	include/uapi/drm/i810_drm.h
>  
> +DRM DRIVER FOR LG SW43408 PANELS
> +M:	Sumit Semwal <sumit.semwal@linaro.org>
> +M:	Caleb Connolly <caleb@connolly.tech>
> +S:	Maintained
> +T:	git git://anongit.freedesktop.org/drm/drm-misc
> +F:	Documentation/devicetree/bindings/display/panel/lg,sw43408-panel.txt
> +F:	drivers/gpu/drm/panel/panel-lg-sw43408.c
> +
>  DRM DRIVER FOR LVDS PANELS
>  M:	Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>  L:	dri-devel@lists.freedesktop.org
> diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> index 38799effd00a..706b112794b9 100644
> --- a/drivers/gpu/drm/panel/Kconfig
> +++ b/drivers/gpu/drm/panel/Kconfig
> @@ -256,6 +256,17 @@ config DRM_PANEL_LEADTEK_LTK500HD1829
>  	  24 bit RGB per pixel. It provides a MIPI DSI interface to
>  	  the host and has a built-in LED backlight.
>  
> +config DRM_PANEL_LG_SW43408
> +	tristate "LG SW43408 panel"
> +	depends on OF
> +	depends on DRM_MIPI_DSI
> +	depends on BACKLIGHT_CLASS_DEVICE
> +	help
> +	  Say Y here if you want to enable support for LG sw43408 panel.
> +	  The panel has a 1080x2160 resolution and uses
> +	  24 bit RGB per pixel. It provides a MIPI DSI interface to
> +	  the host and has a built-in LED backlight.
> +
>  config DRM_PANEL_SAMSUNG_LD9040
>  	tristate "Samsung LD9040 RGB/SPI panel"
>  	depends on OF && SPI
> diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> index 42a7ab54234b..ba26a69b74e7 100644
> --- a/drivers/gpu/drm/panel/Makefile
> +++ b/drivers/gpu/drm/panel/Makefile
> @@ -25,6 +25,7 @@ obj-$(CONFIG_DRM_PANEL_LEADTEK_LTK050H3146W) += panel-leadtek-ltk050h3146w.o
>  obj-$(CONFIG_DRM_PANEL_LEADTEK_LTK500HD1829) += panel-leadtek-ltk500hd1829.o
>  obj-$(CONFIG_DRM_PANEL_LG_LB035Q02) += panel-lg-lb035q02.o
>  obj-$(CONFIG_DRM_PANEL_LG_LG4573) += panel-lg-lg4573.o
> +obj-$(CONFIG_DRM_PANEL_LG_SW43408) += panel-lg-sw43408.o
>  obj-$(CONFIG_DRM_PANEL_NEC_NL8048HL11) += panel-nec-nl8048hl11.o
>  obj-$(CONFIG_DRM_PANEL_NEWVISION_NV3052C) += panel-newvision-nv3052c.o
>  obj-$(CONFIG_DRM_PANEL_NOVATEK_NT35510) += panel-novatek-nt35510.o
> diff --git a/drivers/gpu/drm/panel/panel-lg-sw43408.c b/drivers/gpu/drm/panel/panel-lg-sw43408.c
> new file mode 100644
> index 000000000000..c7b8ec7b970d
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-lg-sw43408.c
> @@ -0,0 +1,586 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2019 Linaro Ltd
> + * Author: Sumit Semwal <sumit.semwal@linaro.org>
> + */
> +
> +#include <linux/backlight.h>
> +#include <linux/delay.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +
> +#include <linux/gpio/consumer.h>
> +#include <linux/regulator/consumer.h>
> +
> +#include <drm/drm_device.h>
> +#include <drm/drm_mipi_dsi.h>
> +#include <drm/drm_modes.h>
> +#include <drm/drm_panel.h>
> +#include <drm/drm_print.h>
> +#include <drm/display/drm_dsc.h>
> +#include <drm/display/drm_dsc_helper.h>
> +
> +#include <video/mipi_display.h>
> +
> +struct panel_cmd {
> +	size_t len;
> +	const char *data;
> +};
> +
> +#define _INIT_CMD(...)                                                   \
> +	{                                                                \
> +		.len = sizeof((char[]){ __VA_ARGS__ }), .data = (char[]) \
> +		{                                                        \
> +			__VA_ARGS__                                      \
> +		}                                                        \
> +	}
> +
> +static const char *const regulator_names[] = {
> +	"vddi",
> +	"vpnl",
> +};
> +
> +static const unsigned long regulator_enable_loads[] = {
> +	62000,
> +	857000,
> +};
> +
> +static const unsigned long regulator_disable_loads[] = {
> +	80,
> +	0,
> +};
> +
> +struct sw43408_panel {
> +	struct drm_panel base;
> +	struct mipi_dsi_device *link;
> +
> +	const struct drm_display_mode *mode;
> +	struct backlight_device *backlight;
> +
> +	struct regulator_bulk_data supplies[ARRAY_SIZE(regulator_names)];
> +
> +	struct gpio_desc *reset_gpio;
> +
> +	bool prepared;
> +	bool enabled;
> +};
> +
> +static const struct panel_cmd lg_sw43408_on_cmds_1[] = {
> +	_INIT_CMD(0x00, 0x53, 0x0C, 0x30),
> +	_INIT_CMD(0x00, 0x55, 0x00, 0x70, 0xDF, 0x00, 0x70, 0xDF),
> +	_INIT_CMD(0x00, 0xF7, 0x01, 0x49, 0x0C),
> +
> +	{},
> +};
> +
> +static const struct panel_cmd lg_sw43408_on_cmds_2[] = {
> +	_INIT_CMD(0x00, 0xB0, 0xAC),
> +	_INIT_CMD(0x00, 0xCD, 0x00, 0x00, 0x00, 0x19, 0x19, 0x19, 0x19, 0x19,
> +		  0x19, 0x19, 0x19, 0x19, 0x19, 0x19, 0x19, 0x19, 0x16, 0x16),
> +	_INIT_CMD(0x00, 0xCB, 0x80, 0x5C, 0x07, 0x03, 0x28),
> +	_INIT_CMD(0x00, 0xC0, 0x02, 0x02, 0x0F),
> +	_INIT_CMD(0x00, 0xE5, 0x00, 0x3A, 0x00, 0x3A, 0x00, 0x0E, 0x10),
> +	_INIT_CMD(0x00, 0xB5, 0x75, 0x60, 0x2D, 0x5D, 0x80, 0x00, 0x0A, 0x0B,
> +		  0x00, 0x05, 0x0B, 0x00, 0x80, 0x0D, 0x0E, 0x40, 0x00, 0x0C,
> +		  0x00, 0x16, 0x00, 0xB8, 0x00, 0x80, 0x0D, 0x0E, 0x40, 0x00,
> +		  0x0C, 0x00, 0x16, 0x00, 0xB8, 0x00, 0x81, 0x00, 0x03, 0x03,
> +		  0x03, 0x01, 0x01),
> +	_INIT_CMD(0x00, 0x55, 0x04, 0x61, 0xDB, 0x04, 0x70, 0xDB),
> +	_INIT_CMD(0x00, 0xB0, 0xCA),
> +
> +	{},
> +};
> +
> +static inline struct sw43408_panel *to_panel_info(struct drm_panel *panel)
> +{
> +	return container_of(panel, struct sw43408_panel, base);
> +}
> +
> +/*
> + * Currently unable to bring up the panel after resetting, must be missing
> + * some init commands somewhere.
> + */
> +static __always_unused int panel_reset(struct sw43408_panel *ctx)
> +{
> +	int ret = 0, i;
> +
> +	for (i = 0; i < ARRAY_SIZE(ctx->supplies); i++) {
> +		ret = regulator_set_load(ctx->supplies[i].consumer,
> +					 regulator_enable_loads[i]);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> +	if (ret < 0)
> +		return ret;
> +
> +	for (i = 0; i < ARRAY_SIZE(ctx->supplies); i++) {
> +		ret = regulator_set_load(ctx->supplies[i].consumer,
> +					 regulator_disable_loads[i]);
> +		if (ret) {
> +			DRM_DEV_ERROR(ctx->base.dev,
> +				      "regulator_set_load failed %d\n", ret);
> +			return ret;
> +		}
> +	}
> +
> +	ret = regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> +	if (ret < 0)
> +		return ret;
> +
> +	gpiod_set_value(ctx->reset_gpio, 0);
> +	usleep_range(9000, 10000);
> +	gpiod_set_value(ctx->reset_gpio, 1);
> +	usleep_range(1000, 2000);
> +	gpiod_set_value(ctx->reset_gpio, 0);
> +	usleep_range(9000, 10000);
> +
> +	return 0;
> +}
> +
> +static int send_mipi_cmds(struct drm_panel *panel, const struct panel_cmd *cmds)
> +{
> +	struct sw43408_panel *ctx = to_panel_info(panel);
> +	unsigned int i = 0;
> +	int err;
> +
> +	if (!cmds)
> +		return -EFAULT;
> +
> +	for (i = 0; cmds[i].len != 0; i++) {
> +		const struct panel_cmd *cmd = &cmds[i];
> +
> +		if (cmd->len == 2)
> +			err = mipi_dsi_dcs_write(ctx->link, cmd->data[1], NULL,
> +						 0);
> +		else
> +			err = mipi_dsi_dcs_write(ctx->link, cmd->data[1],
> +						 cmd->data + 2, cmd->len - 2);
> +
> +		if (err < 0)
> +			return err;
> +
> +		usleep_range((cmd->data[0]) * 1000, (1 + cmd->data[0]) * 1000);
> +	}
> +
> +	return 0;
> +}
> +
> +static int lg_panel_disable(struct drm_panel *panel)
> +{
> +	struct sw43408_panel *ctx = to_panel_info(panel);
> +
> +	backlight_disable(ctx->backlight);
> +	ctx->enabled = false;
> +
> +	return 0;
> +}
> +
> +/*
> + * We can't currently re-initialise the panel properly after powering off.
> + * This function will be used when this is resolved.
> + */
> +static __always_unused int lg_panel_power_off(struct drm_panel *panel)
> +{
> +	struct sw43408_panel *ctx = to_panel_info(panel);
> +	int i, ret = 0;
> +
> +	gpiod_set_value(ctx->reset_gpio, 1);
> +
> +	for (i = 0; i < ARRAY_SIZE(ctx->supplies); i++) {
> +		ret = regulator_set_load(ctx->supplies[i].consumer,
> +					 regulator_disable_loads[i]);
> +		if (ret) {
> +			DRM_DEV_ERROR(panel->dev,
> +				      "regulator_set_load failed %d\n", ret);
> +			return ret;
> +		}
> +	}
> +
> +	ret = regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> +	if (ret) {
> +		DRM_DEV_ERROR(panel->dev, "regulator_bulk_disable failed %d\n",
> +			      ret);
> +	}
> +	return ret;
> +}
> +
> +static int lg_panel_unprepare(struct drm_panel *panel)
> +{
> +	struct sw43408_panel *ctx = to_panel_info(panel);
> +	int ret, i;
> +
> +	if (!ctx->prepared)
> +		return 0;
> +
> +	ret = mipi_dsi_dcs_set_display_off(ctx->link);
> +	if (ret < 0) {
> +		DRM_DEV_ERROR(panel->dev,
> +			      "set_display_off cmd failed ret = %d\n", ret);
> +	}
> +
> +	msleep(120);
> +
> +	ret = mipi_dsi_dcs_enter_sleep_mode(ctx->link);
> +	if (ret < 0) {
> +		DRM_DEV_ERROR(panel->dev, "enter_sleep cmd failed ret = %d\n",
> +			      ret);
> +	}
> +
> +	/* Would call panel_power_off() */
> +
> +	for (i = 0; i < ARRAY_SIZE(ctx->supplies); i++) {
> +		ret = regulator_set_load(ctx->supplies[i].consumer,
> +					 regulator_disable_loads[i]);
> +		if (ret) {
> +			DRM_DEV_ERROR(panel->dev,
> +				      "regulator_set_load failed %d\n", ret);
> +			return ret;
> +		}
> +	}
> +
> +	ctx->prepared = false;
> +
> +	return ret;
> +}
> +
> +static int lg_panel_prepare(struct drm_panel *panel)
> +{
> +	struct sw43408_panel *ctx = to_panel_info(panel);
> +	int err, i;
> +
> +	if (ctx->prepared)
> +		return 0;
> +
> +	/* Would call panel_reset() */
> +
> +	for (i = 0; i < ARRAY_SIZE(ctx->supplies); i++) {
> +		err = regulator_set_load(ctx->supplies[i].consumer,
> +					 regulator_enable_loads[i]);
> +		if (err)
> +			return err;
> +	}
> +
> +	err = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> +	if (err < 0)
> +		return err;
> +
> +	usleep_range(9000, 10000);
> +
> +	err = mipi_dsi_dcs_write(ctx->link, MIPI_DCS_SET_GAMMA_CURVE,
> +				 (u8[]){ 0x02 }, 1);
> +	if (err < 0) {
> +		DRM_DEV_ERROR(panel->dev, "failed to set gamma curve: %d\n",
> +			      err);
> +		goto poweroff;
> +	}
> +
> +	err = mipi_dsi_dcs_set_tear_on(ctx->link,
> +				       MIPI_DSI_DCS_TEAR_MODE_VBLANK);
> +	if (err < 0) {
> +		DRM_DEV_ERROR(panel->dev, "failed to set tear on: %d\n", err);
> +		goto poweroff;
> +	}
> +
> +	err = send_mipi_cmds(panel, &lg_sw43408_on_cmds_1[0]);
> +
> +	if (err < 0) {
> +		DRM_DEV_ERROR(panel->dev,
> +			      "failed to send DCS Init 1st Code: %d\n", err);
> +		goto poweroff;
> +	}
> +
> +	err = mipi_dsi_dcs_exit_sleep_mode(ctx->link);
> +	if (err < 0) {
> +		DRM_DEV_ERROR(panel->dev, "failed to exit sleep mode: %d\n",
> +			      err);
> +		goto poweroff;
> +	}
> +
> +	msleep(135);
> +
> +	err = mipi_dsi_dcs_write(ctx->link, MIPI_DSI_COMPRESSION_MODE,
> +				 (u8[]){ 0x11 }, 0);
> +	if (err < 0) {
> +		DRM_DEV_ERROR(panel->dev,
> +			      "failed to set compression mode: %d\n", err);
> +		goto poweroff;
> +	}
> +
> +	err = send_mipi_cmds(panel, &lg_sw43408_on_cmds_2[0]);
> +
> +	if (err < 0) {
> +		DRM_DEV_ERROR(panel->dev,
> +			      "failed to send DCS Init 2nd Code: %d\n", err);
> +		goto poweroff;
> +	}
> +
> +	err = mipi_dsi_dcs_set_display_on(ctx->link);
> +	if (err < 0) {
> +		DRM_DEV_ERROR(panel->dev, "failed to Set Display ON: %d\n",
> +			      err);
> +		goto poweroff;
> +	}
> +
> +	msleep(120);
> +
> +	ctx->prepared = true;
> +
> +	return 0;
> +
> +poweroff:
> +	gpiod_set_value(ctx->reset_gpio, 1);
> +	regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> +	return err;
> +}
> +
> +static int lg_panel_enable(struct drm_panel *panel)
> +{
> +	struct sw43408_panel *ctx = to_panel_info(panel);
> +	struct drm_dsc_picture_parameter_set pps;
> +	int ret;
> +
> +	if (ctx->enabled)
> +		return 0;
> +
> +	ret = backlight_enable(ctx->backlight);
> +	if (ret) {
> +		DRM_DEV_ERROR(panel->dev, "Failed to enable backlight %d\n",
> +			      ret);
> +		return ret;
> +	}
> +
> +	if (!panel->dsc) {
> +		DRM_DEV_ERROR(panel->dev, "Can't find DSC\n");
> +		return -ENODEV;
> +	}
> +
> +	drm_dsc_pps_payload_pack(&pps, panel->dsc);
> +
> +	ctx->enabled = true;
> +
> +	return 0;
> +}
> +
> +static int lg_panel_get_modes(struct drm_panel *panel,
> +			      struct drm_connector *connector)
> +{
> +	struct sw43408_panel *ctx = to_panel_info(panel);
> +	const struct drm_display_mode *m = ctx->mode;
> +	struct drm_display_mode *mode;
> +
> +	mode = drm_mode_duplicate(connector->dev, m);
> +	if (!mode) {
> +		DRM_DEV_ERROR(panel->dev, "failed to add mode %ux%u\n",
> +			      m->hdisplay, m->vdisplay);
> +		return -ENOMEM;
> +	}
> +
> +	connector->display_info.width_mm = m->width_mm;
> +	connector->display_info.height_mm = m->height_mm;
> +
> +	drm_mode_set_name(mode);
> +	drm_mode_probed_add(connector, mode);
> +
> +	return 1;
> +}
> +
> +static int lg_panel_backlight_update_status(struct backlight_device *bl)
> +{
> +	struct mipi_dsi_device *dsi = bl_get_data(bl);
> +	int ret = 0;
> +	uint16_t brightness;
> +
> +	brightness = (uint16_t)backlight_get_brightness(bl);
> +	/* Brightness is sent in big-endian */
> +	brightness = cpu_to_be16(brightness);
> +
> +	ret = mipi_dsi_dcs_set_display_brightness(dsi, brightness);
> +	return ret;
> +}
> +
> +static int lg_panel_backlight_get_brightness(struct backlight_device *bl)
> +{
> +	struct mipi_dsi_device *dsi = bl_get_data(bl);
> +	int ret = 0;
> +	u16 brightness = 0;
> +
> +	ret = mipi_dsi_dcs_get_display_brightness(dsi, &brightness);
> +	if (ret < 0)
> +		return ret;
> +
> +	return brightness & 0xff;
> +}
> +
> +const struct backlight_ops lg_panel_backlight_ops = {
> +	.update_status = lg_panel_backlight_update_status,
> +	.get_brightness = lg_panel_backlight_get_brightness,
> +};
> +
> +static int lg_panel_backlight_init(struct sw43408_panel *ctx)
> +{
> +	struct device *dev = &ctx->link->dev;
> +	const struct backlight_properties props = {
> +		.type = BACKLIGHT_PLATFORM,
> +		.brightness = 255,
> +		.max_brightness = 255,
> +	};
> +
> +	ctx->backlight = devm_backlight_device_register(dev, dev_name(dev), dev,
> +							ctx->link,
> +							&lg_panel_backlight_ops,
> +							&props);
> +
> +	if (IS_ERR(ctx->backlight))
> +		return dev_err_probe(dev, PTR_ERR(ctx->backlight),
> +				     "Failed to create backlight\n");
> +
> +	return 0;
> +}
> +
> +static const struct drm_panel_funcs panel_funcs = {
> +	.disable = lg_panel_disable,
> +	.unprepare = lg_panel_unprepare,
> +	.prepare = lg_panel_prepare,
> +	.enable = lg_panel_enable,
> +	.get_modes = lg_panel_get_modes,
> +};
> +
> +static const struct drm_display_mode sw43408_default_mode = {
> +	.clock = 152340,
> +
> +	.hdisplay = 1080,
> +	.hsync_start = 1080 + 20,
> +	.hsync_end = 1080 + 20 + 32,
> +	.htotal = 1080 + 20 + 32 + 20,
> +
> +	.vdisplay = 2160,
> +	.vsync_start = 2160 + 20,
> +	.vsync_end = 2160 + 20 + 4,
> +	.vtotal = 2160 + 20 + 4 + 20,
> +
> +	.width_mm = 62,
> +	.height_mm = 124,
> +
> +	.type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED,
> +};
> +
> +static const struct of_device_id panel_of_match[] = {
> +	{ .compatible = "lg,sw43408", .data = &sw43408_default_mode },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, panel_of_match);
> +
> +static int panel_add(struct sw43408_panel *ctx)
> +{
> +	struct device *dev = &ctx->link->dev;
> +	int i, ret;
> +
> +	for (i = 0; i < ARRAY_SIZE(ctx->supplies); i++)
> +		ctx->supplies[i].supply = regulator_names[i];
> +
> +	ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(ctx->supplies),
> +				      ctx->supplies);
> +	if (ret < 0)
> +		return ret;
> +
> +	ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> +	if (IS_ERR(ctx->reset_gpio)) {
> +		DRM_DEV_ERROR(dev, "cannot get reset gpio %ld\n",
> +			      PTR_ERR(ctx->reset_gpio));
> +		return PTR_ERR(ctx->reset_gpio);
> +	}
> +
> +	ret = lg_panel_backlight_init(ctx);
> +	if (ret < 0)
> +		return ret;
> +
> +	drm_panel_init(&ctx->base, dev, &panel_funcs, DRM_MODE_CONNECTOR_DSI);
> +
> +	drm_panel_add(&ctx->base);
> +	return ret;
> +}
> +
> +static int panel_probe(struct mipi_dsi_device *dsi)
> +{
> +	struct sw43408_panel *ctx;
> +	struct drm_dsc_config *dsc;
> +	int err;
> +
> +	ctx = devm_kzalloc(&dsi->dev, sizeof(*ctx), GFP_KERNEL);
> +	if (!ctx)
> +		return -ENOMEM;
> +
> +	ctx->mode = of_device_get_match_data(&dsi->dev);
> +	dsi->mode_flags = MIPI_DSI_MODE_LPM;
> +	dsi->format = MIPI_DSI_FMT_RGB888;
> +	dsi->lanes = 4;
> +
> +	ctx->link = dsi;
> +	mipi_dsi_set_drvdata(dsi, ctx);
> +
> +	err = panel_add(ctx);
> +	if (err < 0)
> +		return err;
> +
> +	/* The panel is DSC panel only, set the dsc params */
> +	dsc = devm_kzalloc(&dsi->dev, sizeof(*dsc), GFP_KERNEL);
> +	if (!dsc)
> +		return -ENOMEM;
> +
> +	dsc->dsc_version_major = 0x1;
> +	dsc->dsc_version_minor = 0x1;
> +
> +	dsc->slice_height = 16;
> +	dsc->slice_width = 540;
> +	dsc->slice_count = 1;
> +	dsc->bits_per_component = 8;
> +	dsc->bits_per_pixel = 8;

Note that this field holds 4 fractional bits, this should be 8 << 4.

That's what drm_dsc_pps_payload_pack() expects and returns, but not what
the DSC implementation in DSI nor DPU1 expects until this lands:

https://lore.kernel.org/linux-arm-msm/20221001190807.358691-1-marijn.suijten@somainline.org/

- Marijn

> +	dsc->block_pred_enable = true;
> +
> +	ctx->base.dsc = dsc;
> +
> +	return mipi_dsi_attach(dsi);
> +}
> +
> +static int panel_remove(struct mipi_dsi_device *dsi)
> +{
> +	struct sw43408_panel *ctx = mipi_dsi_get_drvdata(dsi);
> +	int err;
> +
> +	err = lg_panel_unprepare(&ctx->base);
> +	if (err < 0)
> +		DRM_DEV_ERROR(&dsi->dev, "failed to unprepare panel: %d\n",
> +			      err);
> +
> +	err = lg_panel_disable(&ctx->base);
> +	if (err < 0)
> +		DRM_DEV_ERROR(&dsi->dev, "failed to disable panel: %d\n", err);
> +
> +	err = mipi_dsi_detach(dsi);
> +	if (err < 0)
> +		DRM_DEV_ERROR(&dsi->dev, "failed to detach from DSI host: %d\n",
> +			      err);
> +
> +	if (ctx->base.dev)
> +		drm_panel_remove(&ctx->base);
> +
> +	return 0;
> +}
> +
> +static struct mipi_dsi_driver panel_driver = {
> +	.driver = {
> +		.name = "panel-lg-sw43408",
> +		.of_match_table = panel_of_match,
> +	},
> +	.probe = panel_probe,
> +	.remove = panel_remove,
> +};
> +module_mipi_dsi_driver(panel_driver);
> +
> +MODULE_AUTHOR("Sumit Semwal <sumit.semwal@linaro.org>");
> +MODULE_DESCRIPTION("LG SW436408 MIPI-DSI LED panel");
> +MODULE_LICENSE("GPL");
> -- 
> 2.36.1
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2022-10-01 21:48 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-18 21:30 [PATCH 0/4] Initial support for the Pixel 3 Caleb Connolly
2022-07-18 21:30 ` [PATCH 1/4] Documentation: dt-bindings: arm: qcom: add google,blueline Caleb Connolly
2022-07-19  8:36   ` Krzysztof Kozlowski
2022-07-18 21:30 ` [PATCH 2/4] arm64: dts: qcom: add sdm845-google-blueline (Pixel 3) Caleb Connolly
2022-07-18 22:13   ` Dmitry Baryshkov
2022-07-18 22:54     ` Caleb Connolly
2022-07-18 23:15       ` Dmitry Baryshkov
2022-07-27  8:47     ` Kalle Valo
2022-07-27 10:55     ` Amit Pundir
2022-07-19  8:45   ` Krzysztof Kozlowski
2022-07-18 21:30 ` [PATCH 3/4] dt-bindings: panel: Add LG SW43408 MIPI-DSI panel Caleb Connolly
2022-07-19  6:10   ` Sam Ravnborg
2022-07-19  7:57     ` Sam Ravnborg
2022-07-19 14:11   ` Rob Herring
2022-07-18 21:30 ` [PATCH 4/4] drm: panel: Add lg sw43408 panel driver Caleb Connolly
2022-07-18 23:06   ` Dmitry Baryshkov
2022-07-19  7:48   ` Sam Ravnborg
2022-10-01 21:48   ` Marijn Suijten

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).