From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Return-Path: From: Laurent Pinchart To: Jose Abreu Cc: Laurent Pinchart , dri-devel@lists.freedesktop.org, Andy Yan , Fabio Estevam , Kieran Bingham , Russell King , Ulrich Hecht , Vladimir Zapolskiy , linux-renesas-soc@vger.kernel.org Subject: Re: [PATCH v2 16/29] drm: bridge: dw-hdmi: Detect PHY type at runtime Date: Thu, 05 Jan 2017 13:44:51 +0200 Message-ID: <3716714.KhkP0QV9QI@avalon> In-Reply-To: <5e1041ec-dd9e-4cbf-f2d3-1bd17595adaf@synopsys.com> References: <20161220013400.28317-1-laurent.pinchart+renesas@ideasonboard.com> <6935724.YkYy7FE9s6@avalon> <5e1041ec-dd9e-4cbf-f2d3-1bd17595adaf@synopsys.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" List-ID: Hi Jose, On Thursday 05 Jan 2017 10:33:55 Jose Abreu wrote: > On 05-01-2017 00:15, Laurent Pinchart wrote: > > On Tuesday 20 Dec 2016 15:01:52 Jose Abreu wrote: > >> On 20-12-2016 13:11, Laurent Pinchart wrote: > >>> On Tuesday 20 Dec 2016 11:39:21 Jose Abreu wrote: > >>>> On 20-12-2016 01:33, Laurent Pinchart wrote: > >>>>> Detect the PHY type and use it to handle the PHY type-specific SVSRET > >>>>> signal. > >>>>> > >>>>> Signed-off-by: Laurent Pinchart > >>>>> > >>>>> --- > >>>>> > >>>>> drivers/gpu/drm/bridge/dw-hdmi.c | 68 +++++++++++++++++++++++++++++-- > >>>>> include/drm/bridge/dw_hdmi.h | 10 ++++++ > >>>>> 2 files changed, 75 insertions(+), 3 deletions(-) > >>>>> > >>>>> diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c > >>>>> b/drivers/gpu/drm/bridge/dw-hdmi.c index f4faa14213e5..ef4f2f96ed2c > >>>>> 100644 > >>>>> --- a/drivers/gpu/drm/bridge/dw-hdmi.c > >>>>> +++ b/drivers/gpu/drm/bridge/dw-hdmi.c > >> > >> [snip] > >> > >>> I don't have access to the documentation so I can't comment on that :-) > >>> What does the SVSRET signal control (and what does the name stand for) ? > >> > >> SVSRET stands for SVSRET :) (no idea what it means) Its a low > >> power mode of consumption. > >> > >>> By de-asserting PHY reset, do you mean > >>> > >>> /* PHY reset */ > >>> hdmi_writeb(hdmi, HDMI_MC_PHYRSTZ_DEASSERT, HDMI_MC_PHYRSTZ); > >>> hdmi_writeb(hdmi, HDMI_MC_PHYRSTZ_ASSERT, HDMI_MC_PHYRSTZ); > >>> > >>> ? HDMI_MC_PHYRSTZ_DEASSERT is defined as 0x01 and HDMI_MC_PHYRSTZ_ASSERT > >>> as 0x00, which I believe leads to correct operation on Gen2 PHYs, but is > >>> incorrect on Gen1 PHYs that have an active low reset signal. Could you > >>> confirm that ? The DEASSERT and ASSERT macros should be renamed as > >>> they're obviously incorrect. > >> > >> Correct. Older phys require PHYRSTZ to be deasserted (i.e. low) > >> for a PHY-dependent time. Newer phys require PHYRSTZ to be > >> asserted (i.e. high) for, again, a PHY-dependent time. > > > > Thanks for the confirmation, I'll rename the macros. > > > >> This is the kind of things that made me suggest you to extract > >> all the phy configuration from dw-hdmi. I think that having a > >> bunch of if's because of all the phy's that we need to support > >> does not make much sense. The downside is, of course, having code > >> duplicated. > > > > I agree with you in principle, but I'd rather do that when I'll have > > devices to test the code on. The three devices (soon to be) supported in > > mainline by the dw-hdmi drivers, i.MX6, RK3288 and R-Car Gen3, all use > > Gen2 PHYs, so I can't test the Gen1 code paths meaningfully at the > > moment. > > > >>> I can move the SVSRET assertion before PHY reset and test it on RK3288 > >>> and R-Car H3. > >> > >> Probably wont make much difference unless you have a way to > >> measure how much power the phy is consuming. But I think it is > >> the right thing to do according to docs. > > > > The init sequence is currently > > > > - Power the PHY off (TXPWRON = 0, PDDQ = 1) > > - Reset the PHY (through HDMI_MC_PHYRSTZ and HDMI_MC_HEACPHY_RST) > > - Configure the PHY through I2C > > - Power the PHY on (TXPWRON = 1, PDDQ = 0) > > - Set SVSRET > > - Wait for PHY PLL lock > > What I have here for the most recent phy we tested is this: > - Board specific configuration (should not be needed by you) > - Set MC_PHY_RST high > - Set TXPWRON = 0 > - Set PDDQ = 1 > - Set SVSRET = 0 > - Board specific impendance calibration reset (should not be > needed by you) > - Set SVSRET = 1 > - Set MC_PHY_RST low > - Configure phy through I2C > - Set PDDQ = 0 > - Set TXPWRON = 1, > - Wait for phy pll lock Thank you, I'll test that. > > I've tried moving SVSRET right before the reset and everything still seems > > to work fine, so I'll submit a patch. > > > > Is the rest of the sequence correct ? When should SVSRET be deasserted > > (the driver currently keeps it asserted at all times) ? > > It should not be deasserted. At all ? Even when powering the PHY down ? > > Speaking of reset, I believe support for HEAC PHYs is broken, given that > > the driver asserts the reset signal (through the HDMI_MC_HEACPHY_RST > > register) but never deasserts it. > > Hmm, probably, but not sure. I never tested this in older phys. One more item we'll fix when we'll be able to test it :-) > >> [snip] > >> > >>> The SoC datasheet mentions the SVSRET bit in the HDMI TX registers but > >>> doesn't document it any further. If I don't set SVSRET the HDMI output > >>> stays dead, so I assume I need to set it :-) > >> > >> Hmm, ok. I still haven't figured out which phy you are using so > >> can't comment much further. > > > > I'll then leave has_svsret set to true for DW_HDMI_PHY_DWC_HDMI20_TX_PHY > > as tests show it's needed. -- Regards, Laurent Pinchart From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurent Pinchart Subject: Re: [PATCH v2 16/29] drm: bridge: dw-hdmi: Detect PHY type at runtime Date: Thu, 05 Jan 2017 13:44:51 +0200 Message-ID: <3716714.KhkP0QV9QI@avalon> References: <20161220013400.28317-1-laurent.pinchart+renesas@ideasonboard.com> <6935724.YkYy7FE9s6@avalon> <5e1041ec-dd9e-4cbf-f2d3-1bd17595adaf@synopsys.com> 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 [IPv6:2001:4b98:dc2:45:216:3eff:febb:480d]) by gabe.freedesktop.org (Postfix) with ESMTPS id 7BF946E2D9 for ; Thu, 5 Jan 2017 11:44:47 +0000 (UTC) In-Reply-To: <5e1041ec-dd9e-4cbf-f2d3-1bd17595adaf@synopsys.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Jose Abreu Cc: Fabio Estevam , Laurent Pinchart , Kieran Bingham , dri-devel@lists.freedesktop.org, linux-renesas-soc@vger.kernel.org, Ulrich Hecht , Russell King , Andy Yan , Vladimir Zapolskiy List-Id: dri-devel@lists.freedesktop.org SGkgSm9zZSwKCk9uIFRodXJzZGF5IDA1IEphbiAyMDE3IDEwOjMzOjU1IEpvc2UgQWJyZXUgd3Jv dGU6Cj4gT24gMDUtMDEtMjAxNyAwMDoxNSwgTGF1cmVudCBQaW5jaGFydCB3cm90ZToKPiA+IE9u IFR1ZXNkYXkgMjAgRGVjIDIwMTYgMTU6MDE6NTIgSm9zZSBBYnJldSB3cm90ZToKPiA+PiBPbiAy MC0xMi0yMDE2IDEzOjExLCBMYXVyZW50IFBpbmNoYXJ0IHdyb3RlOgo+ID4+PiBPbiBUdWVzZGF5 IDIwIERlYyAyMDE2IDExOjM5OjIxIEpvc2UgQWJyZXUgd3JvdGU6Cj4gPj4+PiBPbiAyMC0xMi0y MDE2IDAxOjMzLCBMYXVyZW50IFBpbmNoYXJ0IHdyb3RlOgo+ID4+Pj4+IERldGVjdCB0aGUgUEhZ IHR5cGUgYW5kIHVzZSBpdCB0byBoYW5kbGUgdGhlIFBIWSB0eXBlLXNwZWNpZmljIFNWU1JFVAo+ ID4+Pj4+IHNpZ25hbC4KPiA+Pj4+PiAKPiA+Pj4+PiBTaWduZWQtb2ZmLWJ5OiBMYXVyZW50IFBp bmNoYXJ0Cj4gPj4+Pj4gPGxhdXJlbnQucGluY2hhcnQrcmVuZXNhc0BpZGVhc29uYm9hcmQuY29t Pgo+ID4+Pj4+IC0tLQo+ID4+Pj4+IAo+ID4+Pj4+ICBkcml2ZXJzL2dwdS9kcm0vYnJpZGdlL2R3 LWhkbWkuYyB8IDY4ICsrKysrKysrKysrKysrKysrKysrKysrKysrKysrLS0KPiA+Pj4+PiAgaW5j bHVkZS9kcm0vYnJpZGdlL2R3X2hkbWkuaCAgICAgfCAxMCArKysrKysKPiA+Pj4+PiAgMiBmaWxl cyBjaGFuZ2VkLCA3NSBpbnNlcnRpb25zKCspLCAzIGRlbGV0aW9ucygtKQo+ID4+Pj4+IAo+ID4+ Pj4+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2dwdS9kcm0vYnJpZGdlL2R3LWhkbWkuYwo+ID4+Pj4+ IGIvZHJpdmVycy9ncHUvZHJtL2JyaWRnZS9kdy1oZG1pLmMgaW5kZXggZjRmYWExNDIxM2U1Li5l ZjRmMmY5NmVkMmMKPiA+Pj4+PiAxMDA2NDQKPiA+Pj4+PiAtLS0gYS9kcml2ZXJzL2dwdS9kcm0v YnJpZGdlL2R3LWhkbWkuYwo+ID4+Pj4+ICsrKyBiL2RyaXZlcnMvZ3B1L2RybS9icmlkZ2UvZHct aGRtaS5jCj4gPj4gCj4gPj4gW3NuaXBdCj4gPj4gCj4gPj4+IEkgZG9uJ3QgaGF2ZSBhY2Nlc3Mg dG8gdGhlIGRvY3VtZW50YXRpb24gc28gSSBjYW4ndCBjb21tZW50IG9uIHRoYXQgOi0pCj4gPj4+ IFdoYXQgZG9lcyB0aGUgU1ZTUkVUIHNpZ25hbCBjb250cm9sIChhbmQgd2hhdCBkb2VzIHRoZSBu YW1lIHN0YW5kIGZvcikgPwo+ID4+IAo+ID4+IFNWU1JFVCBzdGFuZHMgZm9yIFNWU1JFVCA6KSAo bm8gaWRlYSB3aGF0IGl0IG1lYW5zKSBJdHMgYSBsb3cKPiA+PiBwb3dlciBtb2RlIG9mIGNvbnN1 bXB0aW9uLgo+ID4+IAo+ID4+PiBCeSBkZS1hc3NlcnRpbmcgUEhZIHJlc2V0LCBkbyB5b3UgbWVh bgo+ID4+PiAKPiA+Pj4gICAgICAgICAvKiBQSFkgcmVzZXQgKi8KPiA+Pj4gICAgICAgICBoZG1p X3dyaXRlYihoZG1pLCBIRE1JX01DX1BIWVJTVFpfREVBU1NFUlQsIEhETUlfTUNfUEhZUlNUWik7 Cj4gPj4+ICAgICAgICAgaGRtaV93cml0ZWIoaGRtaSwgSERNSV9NQ19QSFlSU1RaX0FTU0VSVCwg SERNSV9NQ19QSFlSU1RaKTsKPiA+Pj4gCj4gPj4+ID8gSERNSV9NQ19QSFlSU1RaX0RFQVNTRVJU IGlzIGRlZmluZWQgYXMgMHgwMSBhbmQgSERNSV9NQ19QSFlSU1RaX0FTU0VSVAo+ID4+PiBhcyAw eDAwLCB3aGljaCBJIGJlbGlldmUgbGVhZHMgdG8gY29ycmVjdCBvcGVyYXRpb24gb24gR2VuMiBQ SFlzLCBidXQgaXMKPiA+Pj4gaW5jb3JyZWN0IG9uIEdlbjEgUEhZcyB0aGF0IGhhdmUgYW4gYWN0 aXZlIGxvdyByZXNldCBzaWduYWwuIENvdWxkIHlvdQo+ID4+PiBjb25maXJtIHRoYXQgPyBUaGUg REVBU1NFUlQgYW5kIEFTU0VSVCBtYWNyb3Mgc2hvdWxkIGJlIHJlbmFtZWQgYXMKPiA+Pj4gdGhl eSdyZSBvYnZpb3VzbHkgaW5jb3JyZWN0Lgo+ID4+IAo+ID4+IENvcnJlY3QuIE9sZGVyIHBoeXMg cmVxdWlyZSBQSFlSU1RaIHRvIGJlIGRlYXNzZXJ0ZWQgKGkuZS4gbG93KQo+ID4+IGZvciBhIFBI WS1kZXBlbmRlbnQgdGltZS4gTmV3ZXIgcGh5cyByZXF1aXJlIFBIWVJTVFogdG8gYmUKPiA+PiBh c3NlcnRlZCAoaS5lLiBoaWdoKSBmb3IsIGFnYWluLCBhIFBIWS1kZXBlbmRlbnQgdGltZS4KPiA+ IAo+ID4gVGhhbmtzIGZvciB0aGUgY29uZmlybWF0aW9uLCBJJ2xsIHJlbmFtZSB0aGUgbWFjcm9z Lgo+ID4gCj4gPj4gVGhpcyBpcyB0aGUga2luZCBvZiB0aGluZ3MgdGhhdCBtYWRlIG1lIHN1Z2dl c3QgeW91IHRvIGV4dHJhY3QKPiA+PiBhbGwgdGhlIHBoeSBjb25maWd1cmF0aW9uIGZyb20gZHct aGRtaS4gSSB0aGluayB0aGF0IGhhdmluZyBhCj4gPj4gYnVuY2ggb2YgaWYncyBiZWNhdXNlIG9m IGFsbCB0aGUgcGh5J3MgdGhhdCB3ZSBuZWVkIHRvIHN1cHBvcnQKPiA+PiBkb2VzIG5vdCBtYWtl IG11Y2ggc2Vuc2UuIFRoZSBkb3duc2lkZSBpcywgb2YgY291cnNlLCBoYXZpbmcgY29kZQo+ID4+ IGR1cGxpY2F0ZWQuCj4gPiAKPiA+IEkgYWdyZWUgd2l0aCB5b3UgaW4gcHJpbmNpcGxlLCBidXQg SSdkIHJhdGhlciBkbyB0aGF0IHdoZW4gSSdsbCBoYXZlCj4gPiBkZXZpY2VzIHRvIHRlc3QgdGhl IGNvZGUgb24uIFRoZSB0aHJlZSBkZXZpY2VzIChzb29uIHRvIGJlKSBzdXBwb3J0ZWQgaW4KPiA+ IG1haW5saW5lIGJ5IHRoZSBkdy1oZG1pIGRyaXZlcnMsIGkuTVg2LCBSSzMyODggYW5kIFItQ2Fy IEdlbjMsIGFsbCB1c2UKPiA+IEdlbjIgUEhZcywgc28gSSBjYW4ndCB0ZXN0IHRoZSBHZW4xIGNv ZGUgcGF0aHMgbWVhbmluZ2Z1bGx5IGF0IHRoZQo+ID4gbW9tZW50Lgo+ID4gCj4gPj4+IEkgY2Fu IG1vdmUgdGhlIFNWU1JFVCBhc3NlcnRpb24gYmVmb3JlIFBIWSByZXNldCBhbmQgdGVzdCBpdCBv biBSSzMyODgKPiA+Pj4gYW5kIFItQ2FyIEgzLgo+ID4+IAo+ID4+IFByb2JhYmx5IHdvbnQgbWFr ZSBtdWNoIGRpZmZlcmVuY2UgdW5sZXNzIHlvdSBoYXZlIGEgd2F5IHRvCj4gPj4gbWVhc3VyZSBo b3cgbXVjaCBwb3dlciB0aGUgcGh5IGlzIGNvbnN1bWluZy4gQnV0IEkgdGhpbmsgaXQgaXMKPiA+ PiB0aGUgcmlnaHQgdGhpbmcgdG8gZG8gYWNjb3JkaW5nIHRvIGRvY3MuCj4gPiAKPiA+IFRoZSBp bml0IHNlcXVlbmNlIGlzIGN1cnJlbnRseQo+ID4gCj4gPiAtIFBvd2VyIHRoZSBQSFkgb2ZmIChU WFBXUk9OID0gMCwgUEREUSA9IDEpCj4gPiAtIFJlc2V0IHRoZSBQSFkgKHRocm91Z2ggSERNSV9N Q19QSFlSU1RaIGFuZCBIRE1JX01DX0hFQUNQSFlfUlNUKQo+ID4gLSBDb25maWd1cmUgdGhlIFBI WSB0aHJvdWdoIEkyQwo+ID4gLSBQb3dlciB0aGUgUEhZIG9uIChUWFBXUk9OID0gMSwgUEREUSA9 IDApCj4gPiAtIFNldCBTVlNSRVQKPiA+IC0gV2FpdCBmb3IgUEhZIFBMTCBsb2NrCj4gCj4gV2hh dCBJIGhhdmUgaGVyZSBmb3IgdGhlIG1vc3QgcmVjZW50IHBoeSB3ZSB0ZXN0ZWQgaXMgdGhpczoK PiAgICAgLSBCb2FyZCBzcGVjaWZpYyBjb25maWd1cmF0aW9uIChzaG91bGQgbm90IGJlIG5lZWRl ZCBieSB5b3UpCj4gICAgIC0gU2V0IE1DX1BIWV9SU1QgaGlnaAo+ICAgICAtIFNldCBUWFBXUk9O ID0gMAo+ICAgICAtIFNldCBQRERRID0gMQo+ICAgICAtIFNldCBTVlNSRVQgPSAwCj4gICAgIC0g Qm9hcmQgc3BlY2lmaWMgaW1wZW5kYW5jZSBjYWxpYnJhdGlvbiByZXNldCAoc2hvdWxkIG5vdCBi ZQo+IG5lZWRlZCBieSB5b3UpCj4gICAgIC0gU2V0IFNWU1JFVCA9IDEKPiAgICAgLSBTZXQgTUNf UEhZX1JTVCBsb3cKPiAgICAgLSBDb25maWd1cmUgcGh5IHRocm91Z2ggSTJDCj4gICAgIC0gU2V0 IFBERFEgPSAwCj4gICAgIC0gU2V0IFRYUFdST04gPSAxLAo+ICAgICAtIFdhaXQgZm9yIHBoeSBw bGwgbG9jawoKVGhhbmsgeW91LCBJJ2xsIHRlc3QgdGhhdC4KCj4gPiBJJ3ZlIHRyaWVkIG1vdmlu ZyBTVlNSRVQgcmlnaHQgYmVmb3JlIHRoZSByZXNldCBhbmQgZXZlcnl0aGluZyBzdGlsbCBzZWVt cwo+ID4gdG8gd29yayBmaW5lLCBzbyBJJ2xsIHN1Ym1pdCBhIHBhdGNoLgo+ID4gCj4gPiBJcyB0 aGUgcmVzdCBvZiB0aGUgc2VxdWVuY2UgY29ycmVjdCA/IFdoZW4gc2hvdWxkIFNWU1JFVCBiZSBk ZWFzc2VydGVkCj4gPiAodGhlIGRyaXZlciBjdXJyZW50bHkga2VlcHMgaXQgYXNzZXJ0ZWQgYXQg YWxsIHRpbWVzKSA/Cj4gCj4gSXQgc2hvdWxkIG5vdCBiZSBkZWFzc2VydGVkLgoKQXQgYWxsID8g RXZlbiB3aGVuIHBvd2VyaW5nIHRoZSBQSFkgZG93biA/Cgo+ID4gU3BlYWtpbmcgb2YgcmVzZXQs IEkgYmVsaWV2ZSBzdXBwb3J0IGZvciBIRUFDIFBIWXMgaXMgYnJva2VuLCBnaXZlbiB0aGF0Cj4g PiB0aGUgZHJpdmVyIGFzc2VydHMgdGhlIHJlc2V0IHNpZ25hbCAodGhyb3VnaCB0aGUgSERNSV9N Q19IRUFDUEhZX1JTVAo+ID4gcmVnaXN0ZXIpIGJ1dCBuZXZlciBkZWFzc2VydHMgaXQuCj4gCj4g SG1tLCBwcm9iYWJseSwgYnV0IG5vdCBzdXJlLiBJIG5ldmVyIHRlc3RlZCB0aGlzIGluIG9sZGVy IHBoeXMuCgpPbmUgbW9yZSBpdGVtIHdlJ2xsIGZpeCB3aGVuIHdlJ2xsIGJlIGFibGUgdG8gdGVz dCBpdCA6LSkKCj4gPj4gW3NuaXBdCj4gPj4gCj4gPj4+IFRoZSBTb0MgZGF0YXNoZWV0IG1lbnRp b25zIHRoZSBTVlNSRVQgYml0IGluIHRoZSBIRE1JIFRYIHJlZ2lzdGVycyBidXQKPiA+Pj4gZG9l c24ndCBkb2N1bWVudCBpdCBhbnkgZnVydGhlci4gSWYgSSBkb24ndCBzZXQgU1ZTUkVUIHRoZSBI RE1JIG91dHB1dAo+ID4+PiBzdGF5cyBkZWFkLCBzbyBJIGFzc3VtZSBJIG5lZWQgdG8gc2V0IGl0 IDotKQo+ID4+IAo+ID4+IEhtbSwgb2suIEkgc3RpbGwgaGF2ZW4ndCBmaWd1cmVkIG91dCB3aGlj aCBwaHkgeW91IGFyZSB1c2luZyBzbwo+ID4+IGNhbid0IGNvbW1lbnQgbXVjaCBmdXJ0aGVyLgo+ ID4gCj4gPiBJJ2xsIHRoZW4gbGVhdmUgaGFzX3N2c3JldCBzZXQgdG8gdHJ1ZSBmb3IgRFdfSERN SV9QSFlfRFdDX0hETUkyMF9UWF9QSFkKPiA+IGFzIHRlc3RzIHNob3cgaXQncyBuZWVkZWQuCgot LSAKUmVnYXJkcywKCkxhdXJlbnQgUGluY2hhcnQKCl9fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fCmRyaS1kZXZlbCBtYWlsaW5nIGxpc3QKZHJpLWRldmVsQGxp c3RzLmZyZWVkZXNrdG9wLm9yZwpodHRwczovL2xpc3RzLmZyZWVkZXNrdG9wLm9yZy9tYWlsbWFu L2xpc3RpbmZvL2RyaS1kZXZlbAo=