linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] arm64: dts: mediatek: asurada: Enable internal display
@ 2022-09-08 17:11 Nícolas F. R. A. Prado
  2022-09-08 17:11 ` [PATCH 1/3] arm64: dts: mediatek: asurada: Add display regulators Nícolas F. R. A. Prado
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Nícolas F. R. A. Prado @ 2022-09-08 17:11 UTC (permalink / raw)
  To: Matthias Brugger
  Cc: Chen-Yu Tsai, AngeloGioacchino Del Regno, kernel,
	Nícolas F. R. A. Prado, Krzysztof Kozlowski, Rob Herring,
	devicetree, linux-arm-kernel, linux-kernel, linux-mediatek


This series adds and enables all components required to have a working
internal display on the Asurada platform.


Nícolas F. R. A. Prado (3):
  arm64: dts: mediatek: asurada: Add display regulators
  arm64: dts: mediatek: asurada: Add display backlight
  arm64: dts: mediatek: asurada: Enable internal display

 .../boot/dts/mediatek/mt8192-asurada.dtsi     | 215 ++++++++++++++++++
 1 file changed, 215 insertions(+)

-- 
2.37.3


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH 1/3] arm64: dts: mediatek: asurada: Add display regulators
  2022-09-08 17:11 [PATCH 0/3] arm64: dts: mediatek: asurada: Enable internal display Nícolas F. R. A. Prado
@ 2022-09-08 17:11 ` Nícolas F. R. A. Prado
  2022-09-09  7:46   ` AngeloGioacchino Del Regno
  2022-09-21 14:20   ` Chen-Yu Tsai
  2022-09-08 17:11 ` [PATCH 2/3] arm64: dts: mediatek: asurada: Add display backlight Nícolas F. R. A. Prado
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 11+ messages in thread
From: Nícolas F. R. A. Prado @ 2022-09-08 17:11 UTC (permalink / raw)
  To: Matthias Brugger
  Cc: Chen-Yu Tsai, AngeloGioacchino Del Regno, kernel,
	Nícolas F. R. A. Prado, Krzysztof Kozlowski, Rob Herring,
	devicetree, linux-arm-kernel, linux-kernel, linux-mediatek

Add the regulators present on the Asurada platform that are used to
power the internal and external displays.

Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>

---

 .../boot/dts/mediatek/mt8192-asurada.dtsi     | 114 ++++++++++++++++++
 1 file changed, 114 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi b/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi
index 4b314435f8fd..1d99e470ea1a 100644
--- a/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi
@@ -23,6 +23,42 @@ memory@40000000 {
 		reg = <0 0x40000000 0 0x80000000>;
 	};
 
+	pp1000_dpbrdg: regulator-1v0-dpbrdg {
+		compatible = "regulator-fixed";
+		regulator-name = "pp1000_dpbrdg";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pp1000_dpbrdg_en_pins>;
+		regulator-min-microvolt = <1000000>;
+		regulator-max-microvolt = <1000000>;
+		enable-active-high;
+		regulator-boot-on;
+		gpio = <&pio 19 GPIO_ACTIVE_HIGH>;
+	};
+
+	pp1000_mipibrdg: regulator-1v0-mipibrdg {
+		compatible = "regulator-fixed";
+		regulator-name = "pp1000_mipibrdg";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pp1000_mipibrdg_en_pins>;
+		regulator-min-microvolt = <1000000>;
+		regulator-max-microvolt = <1000000>;
+		enable-active-high;
+		regulator-boot-on;
+		gpio = <&pio 129 GPIO_ACTIVE_HIGH>;
+	};
+
+	pp1800_dpbrdg: regulator-1v8-dpbrdg {
+		compatible = "regulator-fixed";
+		regulator-name = "pp1800_dpbrdg";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pp1800_dpbrdg_en_pins>;
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <1800000>;
+		enable-active-high;
+		regulator-boot-on;
+		gpio = <&pio 126 GPIO_ACTIVE_HIGH>;
+	};
+
 	/* system wide LDO 1.8V power rail */
 	pp1800_ldo_g: regulator-1v8-g {
 		compatible = "regulator-fixed";
@@ -34,6 +70,30 @@ pp1800_ldo_g: regulator-1v8-g {
 		vin-supply = <&pp3300_g>;
 	};
 
+	pp1800_mipibrdg: regulator-1v8-mipibrdg {
+		compatible = "regulator-fixed";
+		regulator-name = "pp1800_mipibrdg";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pp1800_mipibrdg_en_pins>;
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <1800000>;
+		enable-active-high;
+		regulator-boot-on;
+		gpio = <&pio 128 GPIO_ACTIVE_HIGH>;
+	};
+
+	pp3300_dpbrdg: regulator-3v3-dpbrdg {
+		compatible = "regulator-fixed";
+		regulator-name = "pp3300_dpbrdg";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pp3300_dpbrdg_en_pins>;
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		enable-active-high;
+		regulator-boot-on;
+		gpio = <&pio 26 GPIO_ACTIVE_HIGH>;
+	};
+
 	/* system wide switching 3.3V power rail */
 	pp3300_g: regulator-3v3-g {
 		compatible = "regulator-fixed";
@@ -56,6 +116,18 @@ pp3300_ldo_z: regulator-3v3-z {
 		vin-supply = <&ppvar_sys>;
 	};
 
+	pp3300_mipibrdg: regulator-3v3-mipibrdg {
+		compatible = "regulator-fixed";
+		regulator-name = "pp3300_mipibrdg";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pp3300_mipibrdg_en_pins>;
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		enable-active-high;
+		regulator-boot-on;
+		gpio = <&pio 127 GPIO_ACTIVE_HIGH>;
+	};
+
 	/* separately switched 3.3V power rail */
 	pp3300_u: regulator-3v3-u {
 		compatible = "regulator-fixed";
@@ -719,6 +791,48 @@ pins-wifi-kill {
 		};
 	};
 
+	pp1000_dpbrdg_en_pins: pp1000-dpbrdg-en-pins {
+		pins-en {
+			pinmux = <PINMUX_GPIO19__FUNC_GPIO19>;
+			output-low;
+		};
+	};
+
+	pp1000_mipibrdg_en_pins: pp1000-mipibrdg-en-pins {
+		pins-en {
+			pinmux = <PINMUX_GPIO129__FUNC_GPIO129>;
+			output-low;
+		};
+	};
+
+	pp1800_dpbrdg_en_pins: pp1800-dpbrdg-en-pins {
+		pins-en {
+			pinmux = <PINMUX_GPIO126__FUNC_GPIO126>;
+			output-low;
+		};
+	};
+
+	pp1800_mipibrdg_en_pins: pp1800-mipibrd-en-pins {
+		pins-en {
+			pinmux = <PINMUX_GPIO128__FUNC_GPIO128>;
+			output-low;
+		};
+	};
+
+	pp3300_dpbrdg_en_pins: pp3300-dpbrdg-en-pins {
+		pins-en {
+			pinmux = <PINMUX_GPIO26__FUNC_GPIO26>;
+			output-low;
+		};
+	};
+
+	pp3300_mipibrdg_en_pins: pp3300-mipibrdg-en-pins {
+		pins-en {
+			pinmux = <PINMUX_GPIO127__FUNC_GPIO127>;
+			output-low;
+		};
+	};
+
 	pp3300_wlan_pins: pp3300-wlan-pins {
 		pins-pcie-en-pp3300-wlan {
 			pinmux = <PINMUX_GPIO143__FUNC_GPIO143>;
-- 
2.37.3


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH 2/3] arm64: dts: mediatek: asurada: Add display backlight
  2022-09-08 17:11 [PATCH 0/3] arm64: dts: mediatek: asurada: Enable internal display Nícolas F. R. A. Prado
  2022-09-08 17:11 ` [PATCH 1/3] arm64: dts: mediatek: asurada: Add display regulators Nícolas F. R. A. Prado
@ 2022-09-08 17:11 ` Nícolas F. R. A. Prado
  2022-09-09  7:46   ` AngeloGioacchino Del Regno
  2022-09-08 17:11 ` [PATCH 3/3] arm64: dts: mediatek: asurada: Enable internal display Nícolas F. R. A. Prado
  2022-09-12 10:32 ` [PATCH 0/3] " Chen-Yu Tsai
  3 siblings, 1 reply; 11+ messages in thread
From: Nícolas F. R. A. Prado @ 2022-09-08 17:11 UTC (permalink / raw)
  To: Matthias Brugger
  Cc: Chen-Yu Tsai, AngeloGioacchino Del Regno, kernel,
	Nícolas F. R. A. Prado, Krzysztof Kozlowski, Rob Herring,
	devicetree, linux-arm-kernel, linux-kernel, linux-mediatek

Add the display backlight for the Asurada platform. It relies on the
display PWM controller, so also enable and configure this component.

Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
---

 .../boot/dts/mediatek/mt8192-asurada.dtsi     | 28 +++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi b/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi
index 1d99e470ea1a..33ef55b6dbe1 100644
--- a/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi
@@ -23,6 +23,16 @@ memory@40000000 {
 		reg = <0 0x40000000 0 0x80000000>;
 	};
 
+	backlight_lcd0: backlight-lcd0 {
+		compatible = "pwm-backlight";
+		pwms = <&pwm0 0 500000>;
+		power-supply = <&ppvar_sys>;
+		enable-gpios = <&pio 152 0>;
+		brightness-levels = <0 1023>;
+		num-interpolated-steps = <1023>;
+		default-brightness-level = <576>;
+	};
+
 	pp1000_dpbrdg: regulator-1v0-dpbrdg {
 		compatible = "regulator-fixed";
 		regulator-name = "pp1000_dpbrdg";
@@ -840,6 +850,17 @@ pins-pcie-en-pp3300-wlan {
 		};
 	};
 
+	pwm0_pins: pwm0-default-pins {
+		pins-pwm {
+			pinmux = <PINMUX_GPIO40__FUNC_DISP_PWM>;
+		};
+
+		pins-inhibit {
+			pinmux = <PINMUX_GPIO152__FUNC_GPIO152>;
+			output-high;
+		};
+	};
+
 	scp_pins: scp-pins {
 		pins-vreq-vao {
 			pinmux = <PINMUX_GPIO195__FUNC_SCP_VREQ_VAO>;
@@ -901,6 +922,13 @@ &pmic {
 	interrupts-extended = <&pio 214 IRQ_TYPE_LEVEL_HIGH>;
 };
 
+&pwm0 {
+	status = "okay";
+
+	pinctrl-names = "default";
+	pinctrl-0 = <&pwm0_pins>;
+};
+
 &scp {
 	status = "okay";
 
-- 
2.37.3


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH 3/3] arm64: dts: mediatek: asurada: Enable internal display
  2022-09-08 17:11 [PATCH 0/3] arm64: dts: mediatek: asurada: Enable internal display Nícolas F. R. A. Prado
  2022-09-08 17:11 ` [PATCH 1/3] arm64: dts: mediatek: asurada: Add display regulators Nícolas F. R. A. Prado
  2022-09-08 17:11 ` [PATCH 2/3] arm64: dts: mediatek: asurada: Add display backlight Nícolas F. R. A. Prado
@ 2022-09-08 17:11 ` Nícolas F. R. A. Prado
  2022-09-12 10:32 ` [PATCH 0/3] " Chen-Yu Tsai
  3 siblings, 0 replies; 11+ messages in thread
From: Nícolas F. R. A. Prado @ 2022-09-08 17:11 UTC (permalink / raw)
  To: Matthias Brugger
  Cc: Chen-Yu Tsai, AngeloGioacchino Del Regno, kernel,
	Nícolas F. R. A. Prado, Krzysztof Kozlowski, Rob Herring,
	devicetree, linux-arm-kernel, linux-kernel, linux-mediatek

The asurada platform has an ANX7625 bridge connecting the DSI's output
to the internal eDP panel. Add and enable these devices in order to get
a usable internal display.

Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
---

 .../boot/dts/mediatek/mt8192-asurada.dtsi     | 73 +++++++++++++++++++
 1 file changed, 73 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi b/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi
index 33ef55b6dbe1..fde8548f67ae 100644
--- a/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi
@@ -200,6 +200,14 @@ wifi_restricted_dma_region: wifi@c0000000 {
 	};
 };
 
+&dsi0 {
+	status = "okay";
+};
+
+&dsi_out {
+	remote-endpoint = <&anx7625_in>;
+};
+
 &i2c0 {
 	status = "okay";
 
@@ -248,6 +256,53 @@ &i2c3 {
 	clock-frequency = <400000>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&i2c3_pins>;
+
+	anx_bridge: anx7625@58 {
+		compatible = "analogix,anx7625";
+		reg = <0x58>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&anx7625_pins>;
+		enable-gpios = <&pio 41 GPIO_ACTIVE_HIGH>;
+		reset-gpios = <&pio 42 GPIO_ACTIVE_HIGH>;
+		vdd10-supply = <&pp1000_mipibrdg>;
+		vdd18-supply = <&pp1800_mipibrdg>;
+		vdd33-supply = <&pp3300_mipibrdg>;
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port@0 {
+				reg = <0>;
+
+				anx7625_in: endpoint {
+					remote-endpoint = <&dsi_out>;
+				};
+			};
+
+			port@1 {
+				reg = <1>;
+
+				anx7625_out: endpoint {
+					remote-endpoint = <&panel_in>;
+				};
+			};
+		};
+
+		aux-bus {
+			panel: panel {
+				compatible = "edp-panel";
+				power-supply = <&pp3300_mipibrdg>;
+				backlight = <&backlight_lcd0>;
+
+				port {
+					panel_in: endpoint {
+						remote-endpoint = <&anx7625_out>;
+					};
+				};
+			};
+		};
+	};
 };
 
 &i2c7 {
@@ -258,6 +313,10 @@ &i2c7 {
 	pinctrl-0 = <&i2c7_pins>;
 };
 
+&mipi_tx0 {
+	status = "okay";
+};
+
 &mmc0 {
 	status = "okay";
 
@@ -589,6 +648,20 @@ &pio {
 			  "AUD_DAT_MISO0",
 			  "AUD_DAT_MISO1";
 
+	anx7625_pins: anx7625-default-pins {
+		pins-out {
+			pinmux = <PINMUX_GPIO41__FUNC_GPIO41>,
+				 <PINMUX_GPIO42__FUNC_GPIO42>;
+			output-low;
+		};
+
+		pins-in {
+			pinmux = <PINMUX_GPIO6__FUNC_GPIO6>;
+			input-enable;
+			bias-pull-up;
+		};
+	};
+
 	cr50_int: cr50-irq-default-pins {
 		pins-gsc-ap-int-odl {
 			pinmux = <PINMUX_GPIO171__FUNC_GPIO171>;
-- 
2.37.3


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH 2/3] arm64: dts: mediatek: asurada: Add display backlight
  2022-09-08 17:11 ` [PATCH 2/3] arm64: dts: mediatek: asurada: Add display backlight Nícolas F. R. A. Prado
@ 2022-09-09  7:46   ` AngeloGioacchino Del Regno
  0 siblings, 0 replies; 11+ messages in thread
From: AngeloGioacchino Del Regno @ 2022-09-09  7:46 UTC (permalink / raw)
  To: Nícolas F. R. A. Prado, Matthias Brugger
  Cc: Chen-Yu Tsai, kernel, Krzysztof Kozlowski, Rob Herring,
	devicetree, linux-arm-kernel, linux-kernel, linux-mediatek

Il 08/09/22 19:11, Nícolas F. R. A. Prado ha scritto:
> Add the display backlight for the Asurada platform. It relies on the
> display PWM controller, so also enable and configure this component.
> 
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH 1/3] arm64: dts: mediatek: asurada: Add display regulators
  2022-09-08 17:11 ` [PATCH 1/3] arm64: dts: mediatek: asurada: Add display regulators Nícolas F. R. A. Prado
@ 2022-09-09  7:46   ` AngeloGioacchino Del Regno
  2022-09-21 13:48     ` Nícolas F. R. A. Prado
  2022-09-21 14:20   ` Chen-Yu Tsai
  1 sibling, 1 reply; 11+ messages in thread
From: AngeloGioacchino Del Regno @ 2022-09-09  7:46 UTC (permalink / raw)
  To: Nícolas F. R. A. Prado, Matthias Brugger
  Cc: Chen-Yu Tsai, kernel, Krzysztof Kozlowski, Rob Herring,
	devicetree, linux-arm-kernel, linux-kernel, linux-mediatek

Il 08/09/22 19:11, Nícolas F. R. A. Prado ha scritto:
> Add the regulators present on the Asurada platform that are used to
> power the internal and external displays.
> 
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> 
> ---
> 
>   .../boot/dts/mediatek/mt8192-asurada.dtsi     | 114 ++++++++++++++++++
>   1 file changed, 114 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi b/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi
> index 4b314435f8fd..1d99e470ea1a 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi

..snip..

> @@ -56,6 +116,18 @@ pp3300_ldo_z: regulator-3v3-z {
>   		vin-supply = <&ppvar_sys>;
>   	};
>   

Can you please add a comment here advertising that this regulator
will not only provide power to the MIPI bridge, but *also* to the
display panel itself?

This is to make sure that everyone understands what's going on, and
also that we ourselves don't forget about that.

Probably something like:
/* pp3300_mipibrdg also enables pp3300_panel */

I would then propose to add a "regulator-fixed" that has no GPIO
but with vin-supply as this one.

pp3300_panel: regulator-3v3-panel {
	compatible = "regulator-fixed";
	regulator-name = "pp3300_panel";
	regulator-min-microvolt = <3300000>;
	regulator-max-microvolt = <3300000>;

	vin-supply = <&pp3300_mipibrdg>;
};

I would also test assigning this regulator to the panel node, as this
will make sure to cover future corner cases (think about PM suspend/resume).

P.S.: If you add the pp3300_panel regulator-fixed with that vin-supply,
       maybe the proposed comment would become a bit overkill. Your choice!

Cheers,
Angelo

> +	pp3300_mipibrdg: regulator-3v3-mipibrdg {
> +		compatible = "regulator-fixed";
> +		regulator-name = "pp3300_mipibrdg";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pp3300_mipibrdg_en_pins>;
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		enable-active-high;
> +		regulator-boot-on;
> +		gpio = <&pio 127 GPIO_ACTIVE_HIGH>;
> +	};
> +
>   	/* separately switched 3.3V power rail */
>   	pp3300_u: regulator-3v3-u {
>   		compatible = "regulator-fixed";


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH 0/3] arm64: dts: mediatek: asurada: Enable internal display
  2022-09-08 17:11 [PATCH 0/3] arm64: dts: mediatek: asurada: Enable internal display Nícolas F. R. A. Prado
                   ` (2 preceding siblings ...)
  2022-09-08 17:11 ` [PATCH 3/3] arm64: dts: mediatek: asurada: Enable internal display Nícolas F. R. A. Prado
@ 2022-09-12 10:32 ` Chen-Yu Tsai
  3 siblings, 0 replies; 11+ messages in thread
From: Chen-Yu Tsai @ 2022-09-12 10:32 UTC (permalink / raw)
  To: Nícolas F. R. A. Prado
  Cc: Matthias Brugger, AngeloGioacchino Del Regno, kernel,
	Krzysztof Kozlowski, Rob Herring, devicetree, linux-arm-kernel,
	linux-kernel, linux-mediatek

On Fri, Sep 9, 2022 at 1:11 AM Nícolas F. R. A. Prado
<nfraprado@collabora.com> wrote:
>
>
> This series adds and enables all components required to have a working
> internal display on the Asurada platform.
>
>
> Nícolas F. R. A. Prado (3):
>   arm64: dts: mediatek: asurada: Add display regulators
>   arm64: dts: mediatek: asurada: Add display backlight
>   arm64: dts: mediatek: asurada: Enable internal display

Tested-by: Chen-Yu Tsai <wenst@chromium.org>

Tested _remotely_ on Hayato, with EDID read back correctly.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH 1/3] arm64: dts: mediatek: asurada: Add display regulators
  2022-09-09  7:46   ` AngeloGioacchino Del Regno
@ 2022-09-21 13:48     ` Nícolas F. R. A. Prado
  2022-09-21 13:58       ` AngeloGioacchino Del Regno
  0 siblings, 1 reply; 11+ messages in thread
From: Nícolas F. R. A. Prado @ 2022-09-21 13:48 UTC (permalink / raw)
  To: AngeloGioacchino Del Regno
  Cc: Matthias Brugger, Chen-Yu Tsai, kernel, Krzysztof Kozlowski,
	Rob Herring, devicetree, linux-arm-kernel, linux-kernel,
	linux-mediatek

On Fri, Sep 09, 2022 at 09:46:33AM +0200, AngeloGioacchino Del Regno wrote:
> Il 08/09/22 19:11, Nícolas F. R. A. Prado ha scritto:
> > Add the regulators present on the Asurada platform that are used to
> > power the internal and external displays.
> > 
> > Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> > 
> > ---
> > 
> >   .../boot/dts/mediatek/mt8192-asurada.dtsi     | 114 ++++++++++++++++++
> >   1 file changed, 114 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi b/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi
> > index 4b314435f8fd..1d99e470ea1a 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi
> > +++ b/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi
> 
> ..snip..
> 
> > @@ -56,6 +116,18 @@ pp3300_ldo_z: regulator-3v3-z {
> >   		vin-supply = <&ppvar_sys>;
> >   	};
> 
> Can you please add a comment here advertising that this regulator
> will not only provide power to the MIPI bridge, but *also* to the
> display panel itself?
> 
> This is to make sure that everyone understands what's going on, and
> also that we ourselves don't forget about that.
> 
> Probably something like:
> /* pp3300_mipibrdg also enables pp3300_panel */
> 
> I would then propose to add a "regulator-fixed" that has no GPIO
> but with vin-supply as this one.
> 
> pp3300_panel: regulator-3v3-panel {
> 	compatible = "regulator-fixed";
> 	regulator-name = "pp3300_panel";
> 	regulator-min-microvolt = <3300000>;
> 	regulator-max-microvolt = <3300000>;
> 
> 	vin-supply = <&pp3300_mipibrdg>;
> };
> 
> I would also test assigning this regulator to the panel node, as this
> will make sure to cover future corner cases (think about PM suspend/resume).
> 
> P.S.: If you add the pp3300_panel regulator-fixed with that vin-supply,
>       maybe the proposed comment would become a bit overkill. Your choice!

Hi Angelo,

thanks for the feedback.

I think the current layout makes more sense based on my understanding of the
power routing here: a single power line output by the pp3300_mipibrdg regulator
powers both the ANX chip as well as the panel. So I'm going to keep it the way
it is for now. If there are any other concerns please let me know.

Thanks,
Nícolas

> 
> Cheers,
> Angelo
> 
> > +	pp3300_mipibrdg: regulator-3v3-mipibrdg {
> > +		compatible = "regulator-fixed";
> > +		regulator-name = "pp3300_mipibrdg";
> > +		pinctrl-names = "default";
> > +		pinctrl-0 = <&pp3300_mipibrdg_en_pins>;
> > +		regulator-min-microvolt = <3300000>;
> > +		regulator-max-microvolt = <3300000>;
> > +		enable-active-high;
> > +		regulator-boot-on;
> > +		gpio = <&pio 127 GPIO_ACTIVE_HIGH>;
> > +	};
> > +
> >   	/* separately switched 3.3V power rail */
> >   	pp3300_u: regulator-3v3-u {
> >   		compatible = "regulator-fixed";
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH 1/3] arm64: dts: mediatek: asurada: Add display regulators
  2022-09-21 13:48     ` Nícolas F. R. A. Prado
@ 2022-09-21 13:58       ` AngeloGioacchino Del Regno
  0 siblings, 0 replies; 11+ messages in thread
From: AngeloGioacchino Del Regno @ 2022-09-21 13:58 UTC (permalink / raw)
  To: Nícolas F. R. A. Prado
  Cc: Matthias Brugger, Chen-Yu Tsai, kernel, Krzysztof Kozlowski,
	Rob Herring, devicetree, linux-arm-kernel, linux-kernel,
	linux-mediatek

Il 21/09/22 15:48, Nícolas F. R. A. Prado ha scritto:
> On Fri, Sep 09, 2022 at 09:46:33AM +0200, AngeloGioacchino Del Regno wrote:
>> Il 08/09/22 19:11, Nícolas F. R. A. Prado ha scritto:
>>> Add the regulators present on the Asurada platform that are used to
>>> power the internal and external displays.
>>>
>>> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
>>>
>>> ---
>>>
>>>    .../boot/dts/mediatek/mt8192-asurada.dtsi     | 114 ++++++++++++++++++
>>>    1 file changed, 114 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi b/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi
>>> index 4b314435f8fd..1d99e470ea1a 100644
>>> --- a/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi
>>> +++ b/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi
>>
>> ..snip..
>>
>>> @@ -56,6 +116,18 @@ pp3300_ldo_z: regulator-3v3-z {
>>>    		vin-supply = <&ppvar_sys>;
>>>    	};
>>
>> Can you please add a comment here advertising that this regulator
>> will not only provide power to the MIPI bridge, but *also* to the
>> display panel itself?
>>
>> This is to make sure that everyone understands what's going on, and
>> also that we ourselves don't forget about that.
>>
>> Probably something like:
>> /* pp3300_mipibrdg also enables pp3300_panel */
>>
>> I would then propose to add a "regulator-fixed" that has no GPIO
>> but with vin-supply as this one.
>>
>> pp3300_panel: regulator-3v3-panel {
>> 	compatible = "regulator-fixed";
>> 	regulator-name = "pp3300_panel";
>> 	regulator-min-microvolt = <3300000>;
>> 	regulator-max-microvolt = <3300000>;
>>
>> 	vin-supply = <&pp3300_mipibrdg>;
>> };
>>
>> I would also test assigning this regulator to the panel node, as this
>> will make sure to cover future corner cases (think about PM suspend/resume).
>>
>> P.S.: If you add the pp3300_panel regulator-fixed with that vin-supply,
>>        maybe the proposed comment would become a bit overkill. Your choice!
> 
> Hi Angelo,
> 
> thanks for the feedback.
> 
> I think the current layout makes more sense based on my understanding of the
> power routing here: a single power line output by the pp3300_mipibrdg regulator
> powers both the ANX chip as well as the panel. So I'm going to keep it the way
> it is for now. If there are any other concerns please let me know.
> 

As we discussed, I agree on this decision. Please go on.

Cheers!

> Thanks,
> Nícolas
> 
>>
>> Cheers,
>> Angelo
>>
>>> +	pp3300_mipibrdg: regulator-3v3-mipibrdg {
>>> +		compatible = "regulator-fixed";
>>> +		regulator-name = "pp3300_mipibrdg";
>>> +		pinctrl-names = "default";
>>> +		pinctrl-0 = <&pp3300_mipibrdg_en_pins>;
>>> +		regulator-min-microvolt = <3300000>;
>>> +		regulator-max-microvolt = <3300000>;
>>> +		enable-active-high;
>>> +		regulator-boot-on;
>>> +		gpio = <&pio 127 GPIO_ACTIVE_HIGH>;
>>> +	};
>>> +
>>>    	/* separately switched 3.3V power rail */
>>>    	pp3300_u: regulator-3v3-u {
>>>    		compatible = "regulator-fixed";
>>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH 1/3] arm64: dts: mediatek: asurada: Add display regulators
  2022-09-08 17:11 ` [PATCH 1/3] arm64: dts: mediatek: asurada: Add display regulators Nícolas F. R. A. Prado
  2022-09-09  7:46   ` AngeloGioacchino Del Regno
@ 2022-09-21 14:20   ` Chen-Yu Tsai
  2022-09-27 21:13     ` Nícolas F. R. A. Prado
  1 sibling, 1 reply; 11+ messages in thread
From: Chen-Yu Tsai @ 2022-09-21 14:20 UTC (permalink / raw)
  To: Nícolas F. R. A. Prado
  Cc: Matthias Brugger, AngeloGioacchino Del Regno, kernel,
	Krzysztof Kozlowski, Rob Herring, devicetree, linux-arm-kernel,
	linux-kernel, linux-mediatek

Hi,

On Fri, Sep 9, 2022 at 1:12 AM Nícolas F. R. A. Prado
<nfraprado@collabora.com> wrote:
>
> Add the regulators present on the Asurada platform that are used to
> power the internal and external displays.
>
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
>
> ---
>
>  .../boot/dts/mediatek/mt8192-asurada.dtsi     | 114 ++++++++++++++++++
>  1 file changed, 114 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi b/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi
> index 4b314435f8fd..1d99e470ea1a 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi
> @@ -23,6 +23,42 @@ memory@40000000 {
>                 reg = <0 0x40000000 0 0x80000000>;
>         };
>
> +       pp1000_dpbrdg: regulator-1v0-dpbrdg {
> +               compatible = "regulator-fixed";
> +               regulator-name = "pp1000_dpbrdg";
> +               pinctrl-names = "default";
> +               pinctrl-0 = <&pp1000_dpbrdg_en_pins>;
> +               regulator-min-microvolt = <1000000>;
> +               regulator-max-microvolt = <1000000>;

This is fed by a rail called PP1350_VS2, which is from the MT6359 PMIC.
And this regulator is a proper LDO.

> +               enable-active-high;
> +               regulator-boot-on;
> +               gpio = <&pio 19 GPIO_ACTIVE_HIGH>;
> +       };
> +
> +       pp1000_mipibrdg: regulator-1v0-mipibrdg {
> +               compatible = "regulator-fixed";
> +               regulator-name = "pp1000_mipibrdg";
> +               pinctrl-names = "default";
> +               pinctrl-0 = <&pp1000_mipibrdg_en_pins>;
> +               regulator-min-microvolt = <1000000>;
> +               regulator-max-microvolt = <1000000>;

This is fed by a rail called PP1350_VS2, which is from the MT6359 PMIC.
And this regulator is a proper LDO.

> +               enable-active-high;
> +               regulator-boot-on;
> +               gpio = <&pio 129 GPIO_ACTIVE_HIGH>;
> +       };
> +
> +       pp1800_dpbrdg: regulator-1v8-dpbrdg {
> +               compatible = "regulator-fixed";
> +               regulator-name = "pp1800_dpbrdg";
> +               pinctrl-names = "default";
> +               pinctrl-0 = <&pp1800_dpbrdg_en_pins>;
> +               regulator-min-microvolt = <1800000>;
> +               regulator-max-microvolt = <1800000>;

This regulator is only a power switch. Please drop the min/max properties.
This is fed by a rail called PP1800_VIO18_U, which is from an LDO on the
MT6359 PMIC.

> +               enable-active-high;
> +               regulator-boot-on;
> +               gpio = <&pio 126 GPIO_ACTIVE_HIGH>;
> +       };
> +
>         /* system wide LDO 1.8V power rail */
>         pp1800_ldo_g: regulator-1v8-g {
>                 compatible = "regulator-fixed";
> @@ -34,6 +70,30 @@ pp1800_ldo_g: regulator-1v8-g {
>                 vin-supply = <&pp3300_g>;
>         };
>
> +       pp1800_mipibrdg: regulator-1v8-mipibrdg {
> +               compatible = "regulator-fixed";
> +               regulator-name = "pp1800_mipibrdg";
> +               pinctrl-names = "default";
> +               pinctrl-0 = <&pp1800_mipibrdg_en_pins>;
> +               regulator-min-microvolt = <1800000>;
> +               regulator-max-microvolt = <1800000>;

This regulator is only a power switch. Please drop the min/max properties.
This is fed by a rail called PP1800_VIO18_U, which is from an LDO on the
MT6359 PMIC.

> +               enable-active-high;
> +               regulator-boot-on;
> +               gpio = <&pio 128 GPIO_ACTIVE_HIGH>;
> +       };
> +
> +       pp3300_dpbrdg: regulator-3v3-dpbrdg {
> +               compatible = "regulator-fixed";
> +               regulator-name = "pp3300_dpbrdg";
> +               pinctrl-names = "default";
> +               pinctrl-0 = <&pp3300_dpbrdg_en_pins>;
> +               regulator-min-microvolt = <3300000>;
> +               regulator-max-microvolt = <3300000>;

This regulator is only a power switch. Please drop the min/max properties.
This is fed by a rail called PP3300_G, which is already described below.

> +               enable-active-high;
> +               regulator-boot-on;
> +               gpio = <&pio 26 GPIO_ACTIVE_HIGH>;
> +       };
> +
>         /* system wide switching 3.3V power rail */
>         pp3300_g: regulator-3v3-g {
>                 compatible = "regulator-fixed";
> @@ -56,6 +116,18 @@ pp3300_ldo_z: regulator-3v3-z {
>                 vin-supply = <&ppvar_sys>;
>         };
>
> +       pp3300_mipibrdg: regulator-3v3-mipibrdg {
> +               compatible = "regulator-fixed";
> +               regulator-name = "pp3300_mipibrdg";
> +               pinctrl-names = "default";
> +               pinctrl-0 = <&pp3300_mipibrdg_en_pins>;
> +               regulator-min-microvolt = <3300000>;
> +               regulator-max-microvolt = <3300000>;

This regulator is only a power switch. Please drop the min/max properties.
This is fed by a rail called PP3300_G, which is already described above.

ChenYu

> +               enable-active-high;
> +               regulator-boot-on;
> +               gpio = <&pio 127 GPIO_ACTIVE_HIGH>;
> +       };
> +
>         /* separately switched 3.3V power rail */
>         pp3300_u: regulator-3v3-u {
>                 compatible = "regulator-fixed";
> @@ -719,6 +791,48 @@ pins-wifi-kill {
>                 };
>         };
>
> +       pp1000_dpbrdg_en_pins: pp1000-dpbrdg-en-pins {
> +               pins-en {
> +                       pinmux = <PINMUX_GPIO19__FUNC_GPIO19>;
> +                       output-low;
> +               };
> +       };
> +
> +       pp1000_mipibrdg_en_pins: pp1000-mipibrdg-en-pins {
> +               pins-en {
> +                       pinmux = <PINMUX_GPIO129__FUNC_GPIO129>;
> +                       output-low;
> +               };
> +       };
> +
> +       pp1800_dpbrdg_en_pins: pp1800-dpbrdg-en-pins {
> +               pins-en {
> +                       pinmux = <PINMUX_GPIO126__FUNC_GPIO126>;
> +                       output-low;
> +               };
> +       };
> +
> +       pp1800_mipibrdg_en_pins: pp1800-mipibrd-en-pins {
> +               pins-en {
> +                       pinmux = <PINMUX_GPIO128__FUNC_GPIO128>;
> +                       output-low;
> +               };
> +       };
> +
> +       pp3300_dpbrdg_en_pins: pp3300-dpbrdg-en-pins {
> +               pins-en {
> +                       pinmux = <PINMUX_GPIO26__FUNC_GPIO26>;
> +                       output-low;
> +               };
> +       };
> +
> +       pp3300_mipibrdg_en_pins: pp3300-mipibrdg-en-pins {
> +               pins-en {
> +                       pinmux = <PINMUX_GPIO127__FUNC_GPIO127>;
> +                       output-low;
> +               };
> +       };
> +
>         pp3300_wlan_pins: pp3300-wlan-pins {
>                 pins-pcie-en-pp3300-wlan {
>                         pinmux = <PINMUX_GPIO143__FUNC_GPIO143>;
> --
> 2.37.3
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH 1/3] arm64: dts: mediatek: asurada: Add display regulators
  2022-09-21 14:20   ` Chen-Yu Tsai
@ 2022-09-27 21:13     ` Nícolas F. R. A. Prado
  0 siblings, 0 replies; 11+ messages in thread
From: Nícolas F. R. A. Prado @ 2022-09-27 21:13 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: Matthias Brugger, AngeloGioacchino Del Regno, kernel,
	Krzysztof Kozlowski, Rob Herring, devicetree, linux-arm-kernel,
	linux-kernel, linux-mediatek

Hi Chen-Yu,

thank you for the insights on the power supply hierarchy. Just a couple further
questions below.

On Wed, Sep 21, 2022 at 10:20:43PM +0800, Chen-Yu Tsai wrote:
> Hi,
> 
> On Fri, Sep 9, 2022 at 1:12 AM Nícolas F. R. A. Prado
> <nfraprado@collabora.com> wrote:
> >
> > Add the regulators present on the Asurada platform that are used to
> > power the internal and external displays.
> >
> > Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> >
> > ---
> >
> >  .../boot/dts/mediatek/mt8192-asurada.dtsi     | 114 ++++++++++++++++++
> >  1 file changed, 114 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi b/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi
> > index 4b314435f8fd..1d99e470ea1a 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi
> > +++ b/arch/arm64/boot/dts/mediatek/mt8192-asurada.dtsi
> > @@ -23,6 +23,42 @@ memory@40000000 {
> >                 reg = <0 0x40000000 0 0x80000000>;
> >         };
> >
> > +       pp1000_dpbrdg: regulator-1v0-dpbrdg {
> > +               compatible = "regulator-fixed";
> > +               regulator-name = "pp1000_dpbrdg";
> > +               pinctrl-names = "default";
> > +               pinctrl-0 = <&pp1000_dpbrdg_en_pins>;
> > +               regulator-min-microvolt = <1000000>;
> > +               regulator-max-microvolt = <1000000>;
> 
> This is fed by a rail called PP1350_VS2, which is from the MT6359 PMIC.
> And this regulator is a proper LDO.

So, we should have an additional regulator node here called pp1350_vs2 which
will feed into pp1000_dpbrdg and that is itself fed in from mt6359_vs2_buck_reg
(from mt6359.dtsi). Is that right?

Also, is PP1350_VS2 just a simple switch or an LDO?

> 
[..]
> > +       pp1800_dpbrdg: regulator-1v8-dpbrdg {
> > +               compatible = "regulator-fixed";
> > +               regulator-name = "pp1800_dpbrdg";
> > +               pinctrl-names = "default";
> > +               pinctrl-0 = <&pp1800_dpbrdg_en_pins>;
> > +               regulator-min-microvolt = <1800000>;
> > +               regulator-max-microvolt = <1800000>;
> 
> This regulator is only a power switch. Please drop the min/max properties.
> This is fed by a rail called PP1800_VIO18_U, which is from an LDO on the
> MT6359 PMIC.

Similarly, we should have a pp1800_vio18_u node that is fed in by
mt6359_vio18_ldo_reg, right? And is it a switch or an LDO?

Thanks,
Nícolas

> 
> > +               enable-active-high;
> > +               regulator-boot-on;
> > +               gpio = <&pio 126 GPIO_ACTIVE_HIGH>;
> > +       };
> > +
[..]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

end of thread, other threads:[~2022-09-27 21:15 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-08 17:11 [PATCH 0/3] arm64: dts: mediatek: asurada: Enable internal display Nícolas F. R. A. Prado
2022-09-08 17:11 ` [PATCH 1/3] arm64: dts: mediatek: asurada: Add display regulators Nícolas F. R. A. Prado
2022-09-09  7:46   ` AngeloGioacchino Del Regno
2022-09-21 13:48     ` Nícolas F. R. A. Prado
2022-09-21 13:58       ` AngeloGioacchino Del Regno
2022-09-21 14:20   ` Chen-Yu Tsai
2022-09-27 21:13     ` Nícolas F. R. A. Prado
2022-09-08 17:11 ` [PATCH 2/3] arm64: dts: mediatek: asurada: Add display backlight Nícolas F. R. A. Prado
2022-09-09  7:46   ` AngeloGioacchino Del Regno
2022-09-08 17:11 ` [PATCH 3/3] arm64: dts: mediatek: asurada: Enable internal display Nícolas F. R. A. Prado
2022-09-12 10:32 ` [PATCH 0/3] " Chen-Yu Tsai

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).