Re: [PATCH 4/4] arm64: dts: rk3399-pinephone-pro: Add internal display support

[Maya Matuszczyk]

Nice to see Pinephone Pro getting worked on.

czw., 22 gru 2022 o 23:39 Javier Martinez Canillas
<javierm@xxxxxxxxxx> napisał(a):
>
> From: Ondrej Jirman <megi@xxxxxx>
>
> The phone's display is using Hannstar LCD panel, and Goodix based
> touchscreen. Support it.
>
> Signed-off-by: Ondrej Jirman <megi@xxxxxx>
> Co-developed-by: Martijn Braam <martijn@xxxxxxxxx>
> Signed-off-by: Martijn Braam <martijn@xxxxxxxxx>
> Co-developed-by: Kamil Trzciński <ayufan@xxxxxxxxx>
> Signed-off-by: Kamil Trzciński <ayufan@xxxxxxxxx>
> Signed-off-by: Javier Martinez Canillas <javierm@xxxxxxxxxx>
> ---
>
>  .../dts/rockchip/rk3399-pinephone-pro.dts     | 124 ++++++++++++++++++
>  1 file changed, 124 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts b/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
> index 0e4442b59a55..a0439a60395e 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
> @@ -29,6 +29,12 @@ chosen {
>                 stdout-path = "serial2:1500000n8";
>         };
>
> +       backlight: backlight {
> +               compatible = "pwm-backlight";
> +               pwms = <&pwm0 0 1000000 0>;
> +               pwm-delay-us = <10000>;
> +       };
> +
>         gpio-keys {
>                 compatible = "gpio-keys";
>                 pinctrl-names = "default";
> @@ -81,6 +87,32 @@ vcc1v8_codec: vcc1v8-codec-regulator {
>                 regulator-max-microvolt = <1800000>;
>                 vin-supply = <&vcc3v3_sys>;
>         };
> +
> +       /* MIPI DSI panel 1.8v supply */
> +       vcc1v8_lcd: vcc1v8-lcd {
Node names should be generic, for example "vcc1v8-lcd-regulator".

> +               compatible = "regulator-fixed";
> +               enable-active-high;
Is this really needed?
You can set the polarity in "gpios" property.

> +               regulator-name = "vcc1v8_lcd";
> +               regulator-min-microvolt = <1800000>;
> +               regulator-max-microvolt = <1800000>;
> +               vin-supply = <&vcc3v3_sys>;
> +               gpio = <&gpio3 RK_PA5 GPIO_ACTIVE_HIGH>;
Is this a typo? Documentation says "gpios"

> +               pinctrl-names = "default";
> +               pinctrl-0 = <&display_pwren1>;
> +       };
> +
> +       /* MIPI DSI panel 2.8v supply */
> +       vcc2v8_lcd: vcc2v8-lcd {
> +               compatible = "regulator-fixed";
> +               enable-active-high;
Ditto

> +               regulator-name = "vcc2v8_lcd";
> +               regulator-min-microvolt = <2800000>;
> +               regulator-max-microvolt = <2800000>;
> +               vin-supply = <&vcc3v3_sys>;
> +               gpio = <&gpio3 RK_PA1 GPIO_ACTIVE_HIGH>;
Same as before

> +               pinctrl-names = "default";
> +               pinctrl-0 = <&display_pwren>;
> +       };
>  };
>
>  &cpu_l0 {
> @@ -111,6 +143,11 @@ &emmc_phy {
>         status = "okay";
>  };
>
> +&gpu {
> +       mali-supply = <&vdd_gpu>;
> +       status = "okay";
> +};
> +
>  &i2c0 {
>         clock-frequency = <400000>;
>         i2c-scl-rising-time-ns = <168>;
> @@ -193,6 +230,9 @@ vcc3v0_touch: LDO_REG2 {
>                                 regulator-name = "vcc3v0_touch";
>                                 regulator-min-microvolt = <3000000>;
>                                 regulator-max-microvolt = <3000000>;
> +                               regulator-state-mem {
> +                                       regulator-off-in-suspend;
> +                               };
>                         };
>
>                         vcca1v8_codec: LDO_REG3 {
> @@ -326,6 +366,26 @@ opp07 {
>         };
>  };
>
> +&i2c3 {
> +       i2c-scl-rising-time-ns = <450>;
> +       i2c-scl-falling-time-ns = <15>;
> +       status = "okay";
> +
> +       touchscreen@14 {
> +               compatible = "goodix,gt917s";
> +               reg = <0x14>;
> +               interrupt-parent = <&gpio3>;
> +               interrupts = <RK_PB5 IRQ_TYPE_EDGE_RISING>;
> +               irq-gpios = <&gpio3 RK_PB5 GPIO_ACTIVE_HIGH>;
> +               reset-gpios = <&gpio3 RK_PB4 GPIO_ACTIVE_HIGH>;
> +               AVDD28-supply = <&vcc3v0_touch>;
> +               VDDIO-supply = <&vcc3v0_touch>;
> +               touchscreen-size-x = <720>;
> +               touchscreen-size-y = <1440>;
> +               poweroff-in-suspend;
Are you really sure this property exists in touchscreen driver's dt bindings?

> +       };
> +};
> +
>  &io_domains {
>         bt656-supply = <&vcc1v8_dvp>;
>         audio-supply = <&vcca1v8_codec>;
> @@ -334,6 +394,40 @@ &io_domains {
>         status = "okay";
>  };
>
> +&mipi_dsi {
> +       status = "okay";
> +       clock-master;
> +
> +       ports {
> +               mipi_out: port@1 {
> +                       #address-cells = <0>;
> +                       #size-cells = <0>;
> +                       reg = <1>;
> +
> +                       mipi_out_panel: endpoint {
> +                               remote-endpoint = <&mipi_in_panel>;
> +                       };
> +               };
> +       };
> +
> +       panel@0 {
> +               compatible = "hannstar,hsd060bhw4";
> +               reg = <0>;
> +               backlight = <&backlight>;
> +               reset-gpios = <&gpio4 RK_PD1 GPIO_ACTIVE_LOW>;
> +               vcc-supply = <&vcc2v8_lcd>; // 2v8
What is the purpose of that comment?

> +               iovcc-supply = <&vcc1v8_lcd>; // 1v8
> +               pinctrl-names = "default";
> +               pinctrl-0 = <&display_rst_l>;
> +
> +               port {
> +                       mipi_in_panel: endpoint {
> +                               remote-endpoint = <&mipi_out_panel>;
> +                       };
> +               };
> +       };
> +};
> +
>  &pmu_io_domains {
>         pmu1830-supply = <&vcc_1v8>;
>         status = "okay";
> @@ -360,6 +454,20 @@ vsel2_pin: vsel2-pin {
>                 };
>         };
>
> +       dsi {
> +               display_rst_l: display-rst-l {
> +                       rockchip,pins = <4 RK_PD1 RK_FUNC_GPIO &pcfg_pull_down>;
> +               };
> +
> +               display_pwren: display-pwren {
> +                       rockchip,pins = <3 RK_PA1 RK_FUNC_GPIO &pcfg_pull_down>;
> +               };
> +
> +               display_pwren1: display-pwren1 {
> +                       rockchip,pins = <3 RK_PA5 RK_FUNC_GPIO &pcfg_pull_down>;
> +               };
> +       };
> +
>         sound {
>                 vcc1v8_codec_en: vcc1v8-codec-en {
>                         rockchip,pins = <3 RK_PA4 RK_FUNC_GPIO &pcfg_pull_down>;
> @@ -367,6 +475,10 @@ vcc1v8_codec_en: vcc1v8-codec-en {
>         };
>  };
>
> +&pwm0 {
> +       status = "okay";
> +};
> +
>  &sdmmc {
>         bus-width = <4>;
>         cap-sd-highspeed;
> @@ -396,3 +508,15 @@ &tsadc {
>  &uart2 {
>         status = "okay";
>  };
> +
> +&vopb {
> +       status = "okay";
> +       assigned-clocks = <&cru DCLK_VOP0_DIV>, <&cru DCLK_VOP0>,
> +                         <&cru ACLK_VOP0>, <&cru HCLK_VOP0>;
> +       assigned-clock-rates = <0>, <0>, <400000000>, <100000000>;
> +       assigned-clock-parents = <&cru PLL_CPLL>, <&cru DCLK_VOP0_FRAC>;
> +};
> +
> +&vopb_mmu {
> +       status = "okay";
> +};
> --
> 2.38.1
>
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@xxxxxxxxxxxxxxxxxxx
> http://lists.infradead.org/mailman/listinfo/linux-rockchip

Best Regards,
Maya Matuszczyk