linux-kernel - Re: PROBLEM: Asus C201 video mode problems on HDMI hotplug (regression)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 4 Dec 2017 22:22:28 -0500
From:   Nick Bowler <nbowler@...conx.ca>
To:     Laurent Pinchart <laurent.pinchart@...asonboard.com>
Cc:     Archit Taneja <architt@...eaurora.org>,
        linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org,
        Laurent Pinchart <laurent.pinchart+renesas@...asonboard.com>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Dave Airlie <airlied@...hat.com>,
        Andrzej Hajda <a.hajda@...sung.com>,
        Jose Abreu <Jose.Abreu@...opsys.com>
Subject: Re: PROBLEM: Asus C201 video mode problems on HDMI hotplug
 (regression)

On 2017-12-04 21:34 +0200, Laurent Pinchart wrote:
> On Monday, 4 December 2017 21:30:01 EET Nick Bowler wrote:
> > On 2017-12-04 21:06 +0200, Laurent Pinchart wrote:
> > > As you reported that the PLL lock failure message is not printed, the
> > > failure can only come from either the extra delay introduced by the
> > > above loop, or from reading the HDMI_PHY_STAT0 register.
> > > 
> > > How many iterations of the for loop execute before the condition
> > > becomes true?
> > 
> > Judging from the log posted elsethread (where I added extra printouts),
> > it seems to consistently become true on the second iteration.
> > 
> > I will try to rule out read side effects by replacing the polling loop
> > with an unconditional delay.
> 
> You're reading my mind :-)

I did this test by applying the following patch on 4.15-rc1, and the
problem remains.  So it appears the delay is responsible somehow.

Cheers,
  Nick

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index bf14214fa464..4aec4d5c130e 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -1101,8 +1101,6 @@ static void dw_hdmi_phy_power_off(struct dw_hdmi *hdmi)
 static int dw_hdmi_phy_power_on(struct dw_hdmi *hdmi)
 {
        const struct dw_hdmi_phy_data *phy = hdmi->phy.data;
-       unsigned int i;
-       u8 val;

        if (phy->gen == 1) {
                dw_hdmi_phy_enable_powerdown(hdmi, false);
@@ -1116,21 +1114,7 @@ static int dw_hdmi_phy_power_on(struct dw_hdmi *hdmi)
        dw_hdmi_phy_gen2_txpwron(hdmi, 1);
        dw_hdmi_phy_gen2_pddq(hdmi, 0);

-       /* Wait for PHY PLL lock */
-       for (i = 0; i < 5; ++i) {
-               val = hdmi_readb(hdmi, HDMI_PHY_STAT0) & HDMI_PHY_TX_PHY_LOCK;
-               if (val)
-                       break;
-
-               usleep_range(1000, 2000);
-       }
-
-       if (!val) {
-               dev_err(hdmi->dev, "PHY PLL failed to lock\n");
-               return -ETIMEDOUT;
-       }
-
-       dev_dbg(hdmi->dev, "PHY PLL locked %u iterations\n", i);
+       usleep_range(1000, 2000);
        return 0;
 }