Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230407151423.59993-1-nikita@trvn.ru> <20230407151423.59993-5-nikita@trvn.ru>
In-Reply-To: <20230407151423.59993-5-nikita@trvn.ru>
From:   Doug Anderson <dianders@chromium.org>
Date:   Fri, 7 Apr 2023 09:23:31 -0700
Message-ID: <CAD=FV=UCJoz1E4wErJawQjpBRiXw0C0-J4TTWO1+uRiDsdzSUg@mail.gmail.com>
Subject: Re: [PATCH v5 4/4] arm64: dts: qcom: Add Acer Aspire 1
To:     Nikita Travkin <nikita@trvn.ru>
Cc:     agross@kernel.org, andersson@kernel.org, robh+dt@kernel.org,
        krzysztof.kozlowski+dt@linaro.org, konrad.dybcio@linaro.org,
        quic_srivasam@quicinc.com, judyhsiao@chromium.org,
        mka@chromium.org, cros-qcom-dts-watchers@chromium.org,
        linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org, ~postmarketos/upstreaming@lists.sr.ht
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi,

I didn't do too thorough of a review, but I noticed your comment about
the panel power and took a look...

On Fri, Apr 7, 2023 at 8:14=E2=80=AFAM Nikita Travkin <nikita@trvn.ru> wrot=
e:
>
> +       reg_lcm_3p3: panel-regulator {
> +               compatible =3D "regulator-fixed";
> +               regulator-name =3D "lcm_3p3";
> +               regulator-min-microvolt =3D <3300000>;
> +               regulator-max-microvolt =3D <3300000>;
> +
> +               /*
> +                * HACK: Display fails with
> +                *
> +                * *ERROR* Unexpected max rate (0x0); assuming 5.4 GHz
> +                * *ERROR* Link training failed, link is off (-5)
> +                *
> +                * if the power to the panel was ever cut
> +                */
> +               regulator-always-on;

I'm curious if `off-on-delay-us =3D <500000>;` would help you avoid the
hack. The eDP driver should already enforce stuff like this but I
think in some esoteric -EPROBE_DEFER cases it can end up violating
things. Any chance that's what you hit?

Oh, or maybe it's HPD. See below. Even if it's HPD, having an
'off-on-delay-us' specified here isn't a bad idea.

> +&i2c10 {
> +       clock-frequency =3D <400000>;
> +       status =3D "okay";
> +
> +       sn65dsi86_bridge: bridge@2c {
> +               compatible =3D "ti,sn65dsi86";
> +               reg =3D <0x2c>;
> +               gpio-controller;
> +               #gpio-cells =3D <2>;
> +               #pwm-cells =3D <1>;
> +
> +               interrupt-parent =3D <&tlmm>;
> +               interrupts =3D <11 IRQ_TYPE_LEVEL_HIGH>;
> +
> +               enable-gpios =3D <&tlmm 51 GPIO_ACTIVE_HIGH>;
> +               suspend-gpios =3D <&tlmm 22 GPIO_ACTIVE_LOW>;
> +
> +               pinctrl-0 =3D <&bridge_en_default>,
> +                           <&edp_bridge_irq_default>,
> +                           <&bridge_suspend_default>;
> +               pinctrl-names =3D "default";
> +
> +               vpll-supply =3D <&reg_brij_1p8>;
> +               vccio-supply =3D <&reg_brij_1p8>;
> +               vcca-supply =3D <&reg_brij_1p2>;
> +               vcc-supply =3D <&reg_brij_1p2>;
> +
> +               clocks =3D <&rpmhcc RPMH_LN_BB_CLK3>;
> +               clock-names =3D "refclk";

You want "no-hpd;" here somewhere. See below.


> +
> +               ports {
> +                       #address-cells =3D <1>;
> +                       #size-cells =3D <0>;
> +
> +                       port@0 {
> +                               reg =3D <0>;
> +
> +                               sn65dsi86_in: endpoint {
> +                                       remote-endpoint =3D <&dsi0_out>;
> +                               };
> +                       };
> +
> +                       port@1 {
> +                               reg =3D <1>;
> +
> +                               sn65dsi86_out: endpoint {
> +                                       data-lanes =3D <0 1>;
> +                                       remote-endpoint =3D <&panel_in_ed=
p>;
> +                               };
> +                       };
> +               };
> +
> +               aux-bus {
> +                       panel: panel {
> +                               compatible =3D "edp-panel";
> +                               power-supply =3D <&reg_lcm_3p3>;
> +                               backlight =3D <&backlight>;

I think you want:

no-hpd;
hpd-absent-delay-ms =3D <200>;

...and yes, you end up with "no-hpd" in both the panel node and the
ti-sn65dsi86 node. See sdm845-cheza.

HPD might very well be hooked up on your board, but the current Linux
ti-sn65dsi86 driver does not look at its own HPD line because it's
actually slower than just pretending that HPD isn't there. On trogdor
boards we ended up routing HPD to a GPIO.

I guess your other option would be to implement HPD support in
ti-sn65dsi86. That would probably be an overall slower boot for you,
but is technically more correct. In the past people have posted up
patches to get ti-sn65dsi86 working as a full DP port and they added
HPD support for that, but none of those patch series ever go to the
point of landing...