diff mbox series

[1/2] arm64: dts: mediatek: mt8395-nio-12l: Prepare MIPI DSI port

Message ID 20250226-radxa-panel-overlay-v1-1-9e8938dfbead@collabora.com (mailing list archive)
State New, archived
Headers show
Series mt8395-radxa-nio-12l: Add overlay for Radxa 8HD panel | expand

Commit Message

Julien Massot Feb. 26, 2025, 2:35 p.m. UTC
This board can use a MIPI-DSI panel on the DSI0 connector: in
preparation for adding an overlay for the Radxa Display 8HD,
add a pipeline connecting VDOSYS0 components to DSI0.

Also add the backlight, and some pin definitions available
through the DSI0 port.

Signed-off-by: Julien Massot <julien.massot@collabora.com>
---
 .../boot/dts/mediatek/mt8395-radxa-nio-12l.dts     | 60 ++++++++++++++++++++++
 1 file changed, 60 insertions(+)

Comments

AngeloGioacchino Del Regno March 3, 2025, 11:32 a.m. UTC | #1
Il 26/02/25 15:35, Julien Massot ha scritto:
> This board can use a MIPI-DSI panel on the DSI0 connector: in
> preparation for adding an overlay for the Radxa Display 8HD,
> add a pipeline connecting VDOSYS0 components to DSI0.
> 
> Also add the backlight, and some pin definitions available
> through the DSI0 port.
> 
> Signed-off-by: Julien Massot <julien.massot@collabora.com>
> ---
>   .../boot/dts/mediatek/mt8395-radxa-nio-12l.dts     | 60 ++++++++++++++++++++++
>   1 file changed, 60 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt8395-radxa-nio-12l.dts b/arch/arm64/boot/dts/mediatek/mt8395-radxa-nio-12l.dts
> index 7184dc99296c7f5d749c7e6d378722677970b3b7..65c77e43d1cd4913b6741e25130febd746ff753c 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8395-radxa-nio-12l.dts
> +++ b/arch/arm64/boot/dts/mediatek/mt8395-radxa-nio-12l.dts
> @@ -48,6 +48,17 @@ memory@40000000 {
>   		reg = <0 0x40000000 0x1 0x0>;
>   	};
>   
> +	backlight: backlight {
> +		compatible = "pwm-backlight";
> +		brightness-levels = <0 1023>;
> +		default-brightness-level = <576>;
> +		enable-gpios = <&pio 107 GPIO_ACTIVE_HIGH>;
> +		num-interpolated-steps = <1023>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&dsi0_backlight_pins>;
> +		pwms = <&disp_pwm0 0 500000>;

This should be disabled, unless there is a display connected to the DSI connector.

If there's no display, there's no point in enabling any backlight, as that pin
may be reused somehow (with hardware hacks, maybe, yes, but still configuring
the PWM IP and the pin as PWM while unused is at least a waste of energy).

> +	};
> +
>   	wifi_vreg: regulator-wifi-3v3-en {
>   		compatible = "regulator-fixed";
>   		regulator-name = "wifi_3v3_en";
> @@ -499,9 +510,20 @@ &mt6359_vsram_others_ldo_reg {
>   	regulator-max-microvolt = <750000>;
>   };
>   
> +&ovl0_in {
> +	remote-endpoint = <&vdosys0_ep_main>;
> +};
> +

This goes to the overlay that enables the DSI display.

> @@ -912,6 +960,18 @@ &ssusb2 {
>   	status = "okay";
>   };
>   
> +&vdosys0 {
> +	port {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		vdosys0_ep_main: endpoint@0 {
> +			reg = <0>;
> +			remote-endpoint = <&ovl0_in>;
> +		};
> +	};
> +};

If you enable this path, in the event that the DSI display overlay is not added,
the mediatek-drm driver will fail probing: even if a second path is enabled and
that will succeed regardless of the first one failing, I don't see a reason why
whoever doesn't have a DSI display attached should see a miserable failure in the
kernel log :-)

Besides, mediatek-drm failing will also slow down boot for no reason.... etc etc.

Please move the display path setup to the DSI display overlay.

Thanks,
Angelo

> +
>   &xhci0 {
>   	vbus-supply = <&otg_vbus_regulator>;
>   	status = "okay";
>
Julien Massot March 4, 2025, 1:56 p.m. UTC | #2
Hi Angelo,

On Mon, 2025-03-03 at 12:32 +0100, AngeloGioacchino Del Regno wrote:
> Il 26/02/25 15:35, Julien Massot ha scritto:
> > This board can use a MIPI-DSI panel on the DSI0 connector: in
> > preparation for adding an overlay for the Radxa Display 8HD,
> > add a pipeline connecting VDOSYS0 components to DSI0.
> > 
> > Also add the backlight, and some pin definitions available
> > through the DSI0 port.
> > 
> > Signed-off-by: Julien Massot <julien.massot@collabora.com>
> > ---
> >   .../boot/dts/mediatek/mt8395-radxa-nio-12l.dts     | 60 ++++++++++++++++++++++
> >   1 file changed, 60 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8395-radxa-nio-12l.dts
> > b/arch/arm64/boot/dts/mediatek/mt8395-radxa-nio-12l.dts
> > index 7184dc99296c7f5d749c7e6d378722677970b3b7..65c77e43d1cd4913b6741e25130febd746ff753c 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt8395-radxa-nio-12l.dts
> > +++ b/arch/arm64/boot/dts/mediatek/mt8395-radxa-nio-12l.dts
> > @@ -48,6 +48,17 @@ memory@40000000 {
> >   		reg = <0 0x40000000 0x1 0x0>;
> >   	};
> >   
> > +	backlight: backlight {
> > +		compatible = "pwm-backlight";
> > +		brightness-levels = <0 1023>;
> > +		default-brightness-level = <576>;
> > +		enable-gpios = <&pio 107 GPIO_ACTIVE_HIGH>;
> > +		num-interpolated-steps = <1023>;
> > +		pinctrl-names = "default";
> > +		pinctrl-0 = <&dsi0_backlight_pins>;
> > +		pwms = <&disp_pwm0 0 500000>;
> 
> This should be disabled, unless there is a display connected to the DSI connector.
> 
> If there's no display, there's no point in enabling any backlight, as that pin
> may be reused somehow (with hardware hacks, maybe, yes, but still configuring
> the PWM IP and the pin as PWM while unused is at least a waste of energy).
> 
Yes, I will keep it disabled in the board dts.

> > +	};
> > +
> >   	wifi_vreg: regulator-wifi-3v3-en {
> >   		compatible = "regulator-fixed";
> >   		regulator-name = "wifi_3v3_en";
> > @@ -499,9 +510,20 @@ &mt6359_vsram_others_ldo_reg {
> >   	regulator-max-microvolt = <750000>;
> >   };
> >   
> > +&ovl0_in {
> > +	remote-endpoint = <&vdosys0_ep_main>;
> > +};
> > +
> 
> This goes to the overlay that enables the DSI display.
Ok

> 
> > @@ -912,6 +960,18 @@ &ssusb2 {
> >   	status = "okay";
> >   };
> >   
> > +&vdosys0 {
> > +	port {
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +
> > +		vdosys0_ep_main: endpoint@0 {
> > +			reg = <0>;
> > +			remote-endpoint = <&ovl0_in>;
> > +		};
> > +	};
> > +};
> 
> If you enable this path, in the event that the DSI display overlay is not added,
> the mediatek-drm driver will fail probing: even if a second path is enabled and
> that will succeed regardless of the first one failing, I don't see a reason why
> whoever doesn't have a DSI display attached should see a miserable failure in the
> kernel log :-)
> 
> Besides, mediatek-drm failing will also slow down boot for no reason.... etc etc.
> 
> Please move the display path setup to the DSI display overlay.
Sure will move the display path setup to the display overlay.

Thank you,
Julien
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/mediatek/mt8395-radxa-nio-12l.dts b/arch/arm64/boot/dts/mediatek/mt8395-radxa-nio-12l.dts
index 7184dc99296c7f5d749c7e6d378722677970b3b7..65c77e43d1cd4913b6741e25130febd746ff753c 100644
--- a/arch/arm64/boot/dts/mediatek/mt8395-radxa-nio-12l.dts
+++ b/arch/arm64/boot/dts/mediatek/mt8395-radxa-nio-12l.dts
@@ -48,6 +48,17 @@  memory@40000000 {
 		reg = <0 0x40000000 0x1 0x0>;
 	};
 
+	backlight: backlight {
+		compatible = "pwm-backlight";
+		brightness-levels = <0 1023>;
+		default-brightness-level = <576>;
+		enable-gpios = <&pio 107 GPIO_ACTIVE_HIGH>;
+		num-interpolated-steps = <1023>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&dsi0_backlight_pins>;
+		pwms = <&disp_pwm0 0 500000>;
+	};
+
 	wifi_vreg: regulator-wifi-3v3-en {
 		compatible = "regulator-fixed";
 		regulator-name = "wifi_3v3_en";
@@ -499,9 +510,20 @@  &mt6359_vsram_others_ldo_reg {
 	regulator-max-microvolt = <750000>;
 };
 
+&ovl0_in {
+	remote-endpoint = <&vdosys0_ep_main>;
+};
+
 &pio {
 	mediatek,rsel-resistance-in-si-unit;
 
+	dsi0_backlight_pins: dsi0-backlight-pins {
+		pins-backlight-en {
+			pinmux = <PINMUX_GPIO107__FUNC_GPIO107>;
+			output-high;
+		};
+	};
+
 	eth_default_pins: eth-default-pins {
 		pins-cc {
 			pinmux = <PINMUX_GPIO85__FUNC_GBE_TXC>,
@@ -699,6 +721,13 @@  pins-irq {
 		};
 	};
 
+	panel_default_pins: panel-pins {
+		pins-rst {
+			pinmux = <PINMUX_GPIO108__FUNC_GPIO108>;
+			bias-pull-up;
+		};
+	};
+
 	pcie0_default_pins: pcie0-default-pins {
 		pins-bus {
 			pinmux = <PINMUX_GPIO19__FUNC_WAKEN>,
@@ -717,6 +746,12 @@  pins-bus {
 		};
 	};
 
+	pwm0_default_pins: pwm0-pins {
+		pins-disp-pwm {
+			pinmux = <PINMUX_GPIO97__FUNC_DISP_PWM0>;
+		};
+	};
+
 	spi1_pins: spi1-default-pins {
 		pins-bus {
 			pinmux = <PINMUX_GPIO136__FUNC_SPIM1_CSB>,
@@ -737,6 +772,19 @@  pins-bus {
 		};
 	};
 
+	touch_pins: touch-pins {
+		pins-touch-int {
+			pinmux = <PINMUX_GPIO132__FUNC_GPIO132>;
+			input-enable;
+			bias-disable;
+		};
+
+		pins-touch-rst {
+			pinmux = <PINMUX_GPIO133__FUNC_GPIO133>;
+			output-high;
+		};
+	};
+
 	uart0_pins: uart0-pins {
 		pins-bus {
 			pinmux = <PINMUX_GPIO98__FUNC_UTXD0>,
@@ -912,6 +960,18 @@  &ssusb2 {
 	status = "okay";
 };
 
+&vdosys0 {
+	port {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		vdosys0_ep_main: endpoint@0 {
+			reg = <0>;
+			remote-endpoint = <&ovl0_in>;
+		};
+	};
+};
+
 &xhci0 {
 	vbus-supply = <&otg_vbus_regulator>;
 	status = "okay";