Re: [PATCH v2 3/3] arm64: dts: qcom: sm8550-qrd: add display and panel

[Dmitry Baryshkov]

On Tue, 16 May 2023 at 18:46, Krzysztof Kozlowski
<krzysztof.kozlowski@xxxxxxxxxx> wrote:
>
> Enable Display Subsystem with Visionox VTDR6130 Panel (same as on
> MTP8550).
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>
>
> ---
>
> Context in the patch depends on:
> 1. https://lore.kernel.org/linux-arm-msm/20230516133011.108093-1-krzysztof.kozlowski@xxxxxxxxxx/T/#t
> 2. https://lore.kernel.org/linux-arm-msm/20230512160452.206585-1-krzysztof.kozlowski@xxxxxxxxxx/
>
> Changes in v2:
> 1. dispcc is enabled in DTSI.
> 2. Re-order pinctrl and regulators.
> 3. Drop mdp.

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>

A small question below.

> ---
>  arch/arm64/boot/dts/qcom/sm8550-qrd.dts | 68 +++++++++++++++++++++++++
>  1 file changed, 68 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sm8550-qrd.dts b/arch/arm64/boot/dts/qcom/sm8550-qrd.dts
> index 30b36a149125..ade6ba53ae6b 100644
> --- a/arch/arm64/boot/dts/qcom/sm8550-qrd.dts
> +++ b/arch/arm64/boot/dts/qcom/sm8550-qrd.dts
> @@ -431,6 +431,46 @@ &gcc {
>                  <&usb_dp_qmpphy QMP_USB43DP_USB3_PIPE_CLK>;
>  };
>
> +&mdss {
> +       status = "okay";
> +};
> +
> +&mdss_dsi0 {
> +       vdda-supply = <&vreg_l3e_1p2>;
> +       status = "okay";
> +
> +       panel@0 {
> +               compatible = "visionox,vtdr6130";
> +               reg = <0>;
> +
> +               pinctrl-0 = <&sde_dsi_active>, <&sde_te_active>;
> +               pinctrl-1 = <&sde_dsi_suspend>, <&sde_te_suspend>;
> +               pinctrl-names = "default", "sleep";
> +
> +               vci-supply = <&vreg_l13b_3p0>;
> +               vdd-supply = <&vreg_l11b_1p2>;
> +               vddio-supply = <&vreg_l12b_1p8>;
> +
> +               reset-gpios = <&tlmm 133 GPIO_ACTIVE_LOW>;
> +
> +               port {
> +                       panel0_in: endpoint {
> +                               remote-endpoint = <&mdss_dsi0_out>;
> +                       };
> +               };
> +       };
> +};
> +
> +&mdss_dsi0_out {
> +       remote-endpoint = <&panel0_in>;
> +       data-lanes = <0 1 2 3>;
> +};
> +
> +&mdss_dsi0_phy {
> +       vdds-supply = <&vreg_l1e_0p88>;
> +       status = "okay";
> +};
> +
>  &pcie_1_phy_aux_clk {
>         status = "disabled";
>  };
> @@ -532,6 +572,34 @@ wcd_tx: codec@0,3 {
>  &tlmm {
>         gpio-reserved-ranges = <32 8>;
>
> +       sde_dsi_active: sde-dsi-active-state {
> +               pins = "gpio133";
> +               function = "gpio";
> +               drive-strength = <8>;
> +               bias-disable;
> +       };
> +
> +       sde_dsi_suspend: sde-dsi-suspend-state {
> +               pins = "gpio133";
> +               function = "gpio";
> +               drive-strength = <2>;
> +               bias-pull-down;
> +       };
> +
> +       sde_te_active: sde-te-active-state {
> +               pins = "gpio86";
> +               function = "mdp_vsync";
> +               drive-strength = <2>;
> +               bias-pull-down;
> +       };
> +
> +       sde_te_suspend: sde-te-suspend-state {
> +               pins = "gpio86";
> +               function = "mdp_vsync";
> +               drive-strength = <2>;
> +               bias-pull-down;
> +       };

Do we really need the suspend states for reset and TE pins? Especially
for the TE pin, where the only difference is the drive strength.

> +
>         wcd_default: wcd-reset-n-active-state {
>                 pins = "gpio108";
>                 function = "gpio";
> --
> 2.34.1
>


-- 
With best wishes
Dmitry