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: <20171116062811.g27zgeejizfxu6oc@aura.draconx.ca>
Date:   Thu, 16 Nov 2017 01:28:11 -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>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Dave Airlie <airlied@...hat.com>,
        Archit Taneja <architt@...eaurora.org>,
        Andrzej Hajda <a.hajda@...sung.com>
Subject: Re: PROBLEM: Asus C201 video mode problems on HDMI hotplug
 (regression)

Hi,

Any ideas on this issue?  Are there any additional tests I can perform
to help debug this?

On 2017-11-05 11:41 -0500, Nick Bowler wrote:
> 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;
>  }
> 

Thanks,
  Nick