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

[Nick Bowler]

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@xxxxxxxxxx> 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@xxxxxxxxxxxxxxxx>
> 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@xxxxxxxxxxxxxxxx>
>     Tested-by: Neil Armstrong <narmstrong@xxxxxxxxxxxx>
>     Reviewed-by: Jose Abreu <joabreu@xxxxxxxxxxxx>
>     Signed-off-by: Archit Taneja <architt@xxxxxxxxxxxxxx>
>     Link: http://patchwork.freedesktop.org/patch/msgid/20170305233557.11945-1-laurent.pinchart+renesas@xxxxxxxxxxxxxxxx
> 
> :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