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

Nick Bowler
2017-11-16T06:28:29Z

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@draconx.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@ideasonboard.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@ideasonboard.com>
> Tested-by: Neil Armstrong <narmstrong@baylibre.com>
> Reviewed-by: Jose Abreu <joabreu@synopsys.com>
> Signed-off-by: Archit Taneja <architt@codeaurora.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