Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753208AbdLEJlL (ORCPT ); Tue, 5 Dec 2017 04:41:11 -0500 Received: from galahad.ideasonboard.com ([185.26.127.97]:47522 "EHLO galahad.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752482AbdLEJlI (ORCPT ); Tue, 5 Dec 2017 04:41:08 -0500 From: Laurent Pinchart To: Nick Bowler Cc: Archit Taneja , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, Laurent Pinchart , Linus Torvalds , Dave Airlie , Andrzej Hajda , Jose Abreu Subject: Re: PROBLEM: Asus C201 video mode problems on HDMI hotplug (regression) Date: Tue, 05 Dec 2017 11:41:22 +0200 Message-ID: <189080660.yi4IrDcgPy@avalon> Organization: Ideas on Board Oy In-Reply-To: <20171205032228.m62nquqgrnswfbxl@aura.draconx.ca> References: <2033574.q3E8txTlp8@avalon> <20171205032228.m62nquqgrnswfbxl@aura.draconx.ca> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2610 Lines: 70 Hi Nick, On Tuesday, 5 December 2017 05:22:28 EET Nick Bowler wrote: > 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. That's interesting. I think it also means we really need to find the root cause, as otherwise your system would be susceptible to random malfunction if the scheduler ends up interrupting the power on sequence. That might not happen frequently, but would be much harder to debug. > 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; > } -- Regards, Laurent Pinchart