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

[AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>]

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";
>