From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurent Pinchart 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> References: <2033574.q3E8txTlp8@avalon> <20171205032228.m62nquqgrnswfbxl@aura.draconx.ca> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from galahad.ideasonboard.com (galahad.ideasonboard.com [185.26.127.97]) by gabe.freedesktop.org (Postfix) with ESMTPS id 6CEA76E486 for ; Tue, 5 Dec 2017 09:41:08 +0000 (UTC) In-Reply-To: <20171205032228.m62nquqgrnswfbxl@aura.draconx.ca> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Nick Bowler Cc: Jose Abreu , Laurent Pinchart , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, Dave Airlie , Linus Torvalds List-Id: dri-devel@lists.freedesktop.org SGkgTmljaywKCk9uIFR1ZXNkYXksIDUgRGVjZW1iZXIgMjAxNyAwNToyMjoyOCBFRVQgTmljayBC b3dsZXIgd3JvdGU6Cj4gT24gMjAxNy0xMi0wNCAyMTozNCArMDIwMCwgTGF1cmVudCBQaW5jaGFy dCB3cm90ZToKPiA+IE9uIE1vbmRheSwgNCBEZWNlbWJlciAyMDE3IDIxOjMwOjAxIEVFVCBOaWNr IEJvd2xlciB3cm90ZToKPiA+PiBPbiAyMDE3LTEyLTA0IDIxOjA2ICswMjAwLCBMYXVyZW50IFBp bmNoYXJ0IHdyb3RlOgo+ID4+PiBBcyB5b3UgcmVwb3J0ZWQgdGhhdCB0aGUgUExMIGxvY2sgZmFp bHVyZSBtZXNzYWdlIGlzIG5vdCBwcmludGVkLCB0aGUKPiA+Pj4gZmFpbHVyZSBjYW4gb25seSBj b21lIGZyb20gZWl0aGVyIHRoZSBleHRyYSBkZWxheSBpbnRyb2R1Y2VkIGJ5IHRoZQo+ID4+PiBh Ym92ZSBsb29wLCBvciBmcm9tIHJlYWRpbmcgdGhlIEhETUlfUEhZX1NUQVQwIHJlZ2lzdGVyLgo+ ID4+PiAKPiA+Pj4gSG93IG1hbnkgaXRlcmF0aW9ucyBvZiB0aGUgZm9yIGxvb3AgZXhlY3V0ZSBi ZWZvcmUgdGhlIGNvbmRpdGlvbgo+ID4+PiBiZWNvbWVzIHRydWU/Cj4gPj4gCj4gPj4gSnVkZ2lu ZyBmcm9tIHRoZSBsb2cgcG9zdGVkIGVsc2V0aHJlYWQgKHdoZXJlIEkgYWRkZWQgZXh0cmEgcHJp bnRvdXRzKSwKPiA+PiBpdCBzZWVtcyB0byBjb25zaXN0ZW50bHkgYmVjb21lIHRydWUgb24gdGhl IHNlY29uZCBpdGVyYXRpb24uCj4gPj4gCj4gPj4gSSB3aWxsIHRyeSB0byBydWxlIG91dCByZWFk IHNpZGUgZWZmZWN0cyBieSByZXBsYWNpbmcgdGhlIHBvbGxpbmcgbG9vcAo+ID4+IHdpdGggYW4g dW5jb25kaXRpb25hbCBkZWxheS4KPiA+IAo+ID4gWW91J3JlIHJlYWRpbmcgbXkgbWluZCA6LSkK PiAKPiBJIGRpZCB0aGlzIHRlc3QgYnkgYXBwbHlpbmcgdGhlIGZvbGxvd2luZyBwYXRjaCBvbiA0 LjE1LXJjMSwgYW5kIHRoZQo+IHByb2JsZW0gcmVtYWlucy4gIFNvIGl0IGFwcGVhcnMgdGhlIGRl bGF5IGlzIHJlc3BvbnNpYmxlIHNvbWVob3cuCgpUaGF0J3MgaW50ZXJlc3RpbmcuIEkgdGhpbmsg aXQgYWxzbyBtZWFucyB3ZSByZWFsbHkgbmVlZCB0byBmaW5kIHRoZSByb290IApjYXVzZSwgYXMg b3RoZXJ3aXNlIHlvdXIgc3lzdGVtIHdvdWxkIGJlIHN1c2NlcHRpYmxlIHRvIHJhbmRvbSBtYWxm dW5jdGlvbiBpZiAKdGhlIHNjaGVkdWxlciBlbmRzIHVwIGludGVycnVwdGluZyB0aGUgcG93ZXIg b24gc2VxdWVuY2UuIFRoYXQgbWlnaHQgbm90IApoYXBwZW4gZnJlcXVlbnRseSwgYnV0IHdvdWxk IGJlIG11Y2ggaGFyZGVyIHRvIGRlYnVnLgoKPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9ncHUvZHJt L2JyaWRnZS9zeW5vcHN5cy9kdy1oZG1pLmMKPiBiL2RyaXZlcnMvZ3B1L2RybS9icmlkZ2Uvc3lu b3BzeXMvZHctaGRtaS5jIGluZGV4Cj4gYmYxNDIxNGZhNDY0Li40YWVjNGQ1YzEzMGUgMTAwNjQ0 Cj4gLS0tIGEvZHJpdmVycy9ncHUvZHJtL2JyaWRnZS9zeW5vcHN5cy9kdy1oZG1pLmMKPiArKysg Yi9kcml2ZXJzL2dwdS9kcm0vYnJpZGdlL3N5bm9wc3lzL2R3LWhkbWkuYwo+IEBAIC0xMTAxLDgg KzExMDEsNiBAQCBzdGF0aWMgdm9pZCBkd19oZG1pX3BoeV9wb3dlcl9vZmYoc3RydWN0IGR3X2hk bWkKPiAqaGRtaSkgc3RhdGljIGludCBkd19oZG1pX3BoeV9wb3dlcl9vbihzdHJ1Y3QgZHdfaGRt aSAqaGRtaSkKPiAgewo+ICAgICAgICAgY29uc3Qgc3RydWN0IGR3X2hkbWlfcGh5X2RhdGEgKnBo eSA9IGhkbWktPnBoeS5kYXRhOwo+IC0gICAgICAgdW5zaWduZWQgaW50IGk7Cj4gLSAgICAgICB1 OCB2YWw7Cj4gCj4gICAgICAgICBpZiAocGh5LT5nZW4gPT0gMSkgewo+ICAgICAgICAgICAgICAg ICBkd19oZG1pX3BoeV9lbmFibGVfcG93ZXJkb3duKGhkbWksIGZhbHNlKTsKPiBAQCAtMTExNiwy MSArMTExNCw3IEBAIHN0YXRpYyBpbnQgZHdfaGRtaV9waHlfcG93ZXJfb24oc3RydWN0IGR3X2hk bWkgKmhkbWkpCj4gZHdfaGRtaV9waHlfZ2VuMl90eHB3cm9uKGhkbWksIDEpOwo+ICAgICAgICAg ZHdfaGRtaV9waHlfZ2VuMl9wZGRxKGhkbWksIDApOwo+IAo+IC0gICAgICAgLyogV2FpdCBmb3Ig UEhZIFBMTCBsb2NrICovCj4gLSAgICAgICBmb3IgKGkgPSAwOyBpIDwgNTsgKytpKSB7Cj4gLSAg ICAgICAgICAgICAgIHZhbCA9IGhkbWlfcmVhZGIoaGRtaSwgSERNSV9QSFlfU1RBVDApICYKPiBI RE1JX1BIWV9UWF9QSFlfTE9DSzsgLSAgICAgICAgICAgICAgIGlmICh2YWwpCj4gLSAgICAgICAg ICAgICAgICAgICAgICAgYnJlYWs7Cj4gLQo+IC0gICAgICAgICAgICAgICB1c2xlZXBfcmFuZ2Uo MTAwMCwgMjAwMCk7Cj4gLSAgICAgICB9Cj4gLQo+IC0gICAgICAgaWYgKCF2YWwpIHsKPiAtICAg ICAgICAgICAgICAgZGV2X2VycihoZG1pLT5kZXYsICJQSFkgUExMIGZhaWxlZCB0byBsb2NrXG4i KTsKPiAtICAgICAgICAgICAgICAgcmV0dXJuIC1FVElNRURPVVQ7Cj4gLSAgICAgICB9Cj4gLQo+ IC0gICAgICAgZGV2X2RiZyhoZG1pLT5kZXYsICJQSFkgUExMIGxvY2tlZCAldSBpdGVyYXRpb25z XG4iLCBpKTsKPiArICAgICAgIHVzbGVlcF9yYW5nZSgxMDAwLCAyMDAwKTsKPiAgICAgICAgIHJl dHVybiAwOwo+ICB9CgotLSAKUmVnYXJkcywKCkxhdXJlbnQgUGluY2hhcnQKCl9fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmRyaS1kZXZlbCBtYWlsaW5nIGxp c3QKZHJpLWRldmVsQGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpodHRwczovL2xpc3RzLmZyZWVkZXNr dG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2RyaS1kZXZlbAo= From mboxrd@z Thu Jan 1 00:00:00 1970 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 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