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]

Message-ID: <CADyTPExCTRcWACfL1XAUdy9L-HR2fQwvDrYvUPzdfPmc6L39Hw@mail.gmail.com>
Date:   Sun, 5 Nov 2017 11:41:06 -0500
From:   Nick Bowler <nbowler@...conx.ca>
To:     linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org
Cc:     Laurent Pinchart <laurent.pinchart+renesas@...asonboard.com>
Subject: Re: PROBLEM: Asus C201 video mode problems on HDMI hotplug (regression)

Hi,

I completed bisecting this issue.  See below.

On 2017-11-02, Nick Bowler <nbowler@...conx.ca> wrote:
> ~50% of the time after a hotplug, there is a vertical pink bar on the
> left of the display area and audio is not working at all.  According to
> the sink device the display size is 1282x720 which seems pretty wrong
> (normal and working situation is 1280x720).
>
> I posted photos of non-working versus working states here:
>
>   https://imgur.com/a/qhAZG
>
> Unplugging and plugging the cable again will correct the issue (it seems
> to, for the most part, alternate between working and not-working states,
> although not always).  It always works on power up with the cable initially
> connected.
>
> This is a regression from 4.11, where hotplug works perfectly every time.

Bisection implicates the following commit:

181e0ef092a4952aa523c5b9cb21394cf43bcd46 is the first bad commit
commit 181e0ef092a4952aa523c5b9cb21394cf43bcd46
Author: Laurent Pinchart <laurent.pinchart+renesas@...asonboard.com>
Date:   Mon Mar 6 01:35:57 2017 +0200

    drm: bridge: dw-hdmi: Fix the PHY power up sequence

    When powering the PHY up we need to wait for the PLL to lock. This is
    done by polling the TX_PHY_LOCK bit in the HDMI_PHY_STAT0 register
    (interrupt-based wait could be implemented as well but is likely
    overkill). The bit is asserted when the PLL locks, but the current code
    incorrectly waits for the bit to be deasserted. Fix it, and while at it,
    replace the udelay() with a sleep as the code never runs in
    non-sleepable context.

    To be consistent with the power down implementation move the poll loop
    to the power off function.

    Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@...asonboard.com>
    Tested-by: Neil Armstrong <narmstrong@...libre.com>
    Reviewed-by: Jose Abreu <joabreu@...opsys.com>
    Signed-off-by: Archit Taneja <architt@...eaurora.org>
    Link: http://patchwork.freedesktop.org/patch/msgid/20170305233557.11945-1-laurent.pinchart+renesas@ideasonboard.com

:040000 040000 0defad9d1a61c0355f49c679b18eebae2c4b9495
5d260e6db25d6abc1211d61ec3405be99e693a23 M	drivers

This commit does not revert cleanly, but on top of latest master (which has
the problem) I manually changed the relevant code back to its original state
and the problem is fixed, like this:

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index bf14214fa464..6618aac95a51 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -1100,37 +1100,34 @@ 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;
+	u8 val, msec;

-	if (phy->gen == 1) {
-		dw_hdmi_phy_enable_powerdown(hdmi, false);
+	dw_hdmi_phy_enable_powerdown(hdmi, false);

-		/* Toggle TMDS enable. */
-		dw_hdmi_phy_enable_tmds(hdmi, 0);
-		dw_hdmi_phy_enable_tmds(hdmi, 1);
-		return 0;
-	}
+	/* toggle TMDS enable */
+	dw_hdmi_phy_enable_tmds(hdmi, 0);
+	dw_hdmi_phy_enable_tmds(hdmi, 1);

+	/* gen2 tx power on */
 	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) {
+	msec = 5;
+	do {
 		val = hdmi_readb(hdmi, HDMI_PHY_STAT0) & HDMI_PHY_TX_PHY_LOCK;
-		if (val)
+		if (!val)
 			break;

-		usleep_range(1000, 2000);
-	}
+		if (msec == 0) {
+			dev_err(hdmi->dev, "PHY PLL not locked\n");
+			return -ETIMEDOUT;
+		}

-	if (!val) {
-		dev_err(hdmi->dev, "PHY PLL failed to lock\n");
-		return -ETIMEDOUT;
-	}
+		udelay(1000);
+		msec--;
+	} while (1);

-	dev_dbg(hdmi->dev, "PHY PLL locked %u iterations\n", i);
 	return 0;
 }

Cheers,
  Nick