Re: [PATCH v5 4/4] arm64: dts: qcom: Add Acer Aspire 1

[Nikita Travkin <nikita@trvn.ru>]

Doug Anderson писал(а) 07.04.2023 21:23:
> 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 AM Nikita Travkin <nikita@trvn.ru> wrote:
>>
>> +       reg_lcm_3p3: panel-regulator {
>> +               compatible = "regulator-fixed";
>> +               regulator-name = "lcm_3p3";
>> +               regulator-min-microvolt = <3300000>;
>> +               regulator-max-microvolt = <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 = <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 = <400000>;
>> +       status = "okay";
>> +
>> +       sn65dsi86_bridge: bridge@2c {
>> +               compatible = "ti,sn65dsi86";
>> +               reg = <0x2c>;
>> +               gpio-controller;
>> +               #gpio-cells = <2>;
>> +               #pwm-cells = <1>;
>> +
>> +               interrupt-parent = <&tlmm>;
>> +               interrupts = <11 IRQ_TYPE_LEVEL_HIGH>;
>> +
>> +               enable-gpios = <&tlmm 51 GPIO_ACTIVE_HIGH>;
>> +               suspend-gpios = <&tlmm 22 GPIO_ACTIVE_LOW>;
>> +
>> +               pinctrl-0 = <&bridge_en_default>,
>> +                           <&edp_bridge_irq_default>,
>> +                           <&bridge_suspend_default>;
>> +               pinctrl-names = "default";
>> +
>> +               vpll-supply = <&reg_brij_1p8>;
>> +               vccio-supply = <&reg_brij_1p8>;
>> +               vcca-supply = <&reg_brij_1p2>;
>> +               vcc-supply = <&reg_brij_1p2>;
>> +
>> +               clocks = <&rpmhcc RPMH_LN_BB_CLK3>;
>> +               clock-names = "refclk";
> 
> You want "no-hpd;" here somewhere. See below.
> 
> 
>> +
>> +               ports {
>> +                       #address-cells = <1>;
>> +                       #size-cells = <0>;
>> +
>> +                       port@0 {
>> +                               reg = <0>;
>> +
>> +                               sn65dsi86_in: endpoint {
>> +                                       remote-endpoint = <&dsi0_out>;
>> +                               };
>> +                       };
>> +
>> +                       port@1 {
>> +                               reg = <1>;
>> +
>> +                               sn65dsi86_out: endpoint {
>> +                                       data-lanes = <0 1>;
>> +                                       remote-endpoint = <&panel_in_edp>;
>> +                               };
>> +                       };
>> +               };
>> +
>> +               aux-bus {
>> +                       panel: panel {
>> +                               compatible = "edp-panel";
>> +                               power-supply = <&reg_lcm_3p3>;
>> +                               backlight = <&backlight>;
> 
> I think you want:
> 
> no-hpd;
> hpd-absent-delay-ms = <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.
> 

Oh, this makes so much sense then! The line is hooked up on
the board indeed and I remember being confused why trogdor boards
don't use it.

I will try to add the suggestions (annotating the reason)
and verify that it works, would prefer the panel power to be
gated when possible. I hope this would also fix the initial
EDID reading issues I occasionally have and carry a hack for
as of now...

Thank you a lot for this insight!

Nikita

> 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...