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@xxxxxxxxxxxxx>
> > ---
> > .../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
