From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Return-Path: From: Laurent Pinchart To: dri-devel@lists.freedesktop.org Cc: Jose Abreu , Fabio Estevam , Laurent Pinchart , Russell King - ARM Linux , Kieran Bingham , linux-renesas-soc@vger.kernel.org, Ulrich Hecht , Andy Yan , Vladimir Zapolskiy Subject: Re: [PATCH v2 17/29] drm: bridge: dw-hdmi: Refactor PHY power handling Date: Wed, 01 Mar 2017 13:09:44 +0200 Message-ID: <7271033.6cEIpZMQXV@avalon> In-Reply-To: References: <20161220013400.28317-1-laurent.pinchart+renesas@ideasonboard.com> <5812310.k87Vx0mP1E@avalon> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" List-ID: Hi Jose, On Thursday 05 Jan 2017 15:06:49 Jose Abreu wrote: > On 05-01-2017 12:29, Laurent Pinchart wrote: > > On Tuesday 20 Dec 2016 12:17:23 Jose Abreu wrote: > >> On 20-12-2016 11:45, Russell King - ARM Linux wrote: > >>> On Tue, Dec 20, 2016 at 03:33:48AM +0200, Laurent Pinchart wrote: > >>>> Instead of spreading version-dependent PHY power handling code around, > >>>> group it in two functions to power the PHY on and off and use them > >>>> through the driver. > >>>> > >>>> Powering off the PHY at the beginning of the setup phase is currently > >>>> split in two locations for first and second generation PHYs, group all > >>>> the operations in the dw_hdmi_phy_init() function. > >>> > >>> This changes the behaviour of the driver. > >>> > >>>> +static void dw_hdmi_phy_power_off(struct dw_hdmi *hdmi) > >>>> +{ > >>>> + if (hdmi->phy->gen == 1) { > >>>> + dw_hdmi_phy_enable_tmds(hdmi, 0); > >>>> + dw_hdmi_phy_enable_powerdown(hdmi, true); > >>>> + } else { > >>>> + dw_hdmi_phy_gen2_txpwron(hdmi, 0); > >>>> + dw_hdmi_phy_gen2_pddq(hdmi, 1); > >>>> + } > >>>> +} > >>>> @@ -1290,9 +1302,7 @@ static void dw_hdmi_phy_disable(struct dw_hdmi > >>>> *hdmi) > >>>> if (!hdmi->phy_enabled) > >>>> return; > >>>> > >>>> - dw_hdmi_phy_enable_tmds(hdmi, 0); > >>>> - dw_hdmi_phy_enable_powerdown(hdmi, true); > >>>> - > >>>> + dw_hdmi_phy_power_off(hdmi); > >>> > >>> This makes dw_hdmi_phy_disable() power down a gen2 phy. > >>> > >>> The iMX6 has a DW_HDMI_PHY_DWC_HDMI_3D_TX_PHY phy, which you list as a > >>> gen2 phy. I've been carrying this change for a while, which I've had > >>> to revert (and finally expunge), as it causes problems on iMX6: > >>> > >>> @@ -1112,6 +1112,14 @@ static void dw_hdmi_phy_disable(struct dw_hdmi > >>> *hdmi) > >>> if (!hdmi->phy_enabled) > >>> return; > >>> > >>> + /* Actually set the phy into low power mode */ > >>> + dw_hdmi_phy_gen2_txpwron(hdmi, 0); > >>> + > >>> + /* FIXME: We should wait for TX_READY to be deasserted */ > >>> + > >>> + dw_hdmi_phy_gen2_pddq(hdmi, 1); > >>> + > >>> + /* This appears to have no effect on iMX6 */ > >>> dw_hdmi_phy_enable_tmds(hdmi, 0); > >>> dw_hdmi_phy_enable_powerdown(hdmi, true); > >>> > >>> So, I think your change here will cause problems for iMX6. > >>> > >>> From what I remember, it seems that iMX6 has issues with RXSENSE/HPD > >>> bouncing when the PHY is powered down. I can't promise when I'll be > >>> able to check for that again. > >> > >> Indeed TX_READY must be low before asserting pddq. > > > > The TX_READY signal is documented in the i.MX6 datasheet as being a PHY > > output signal, but there seems to be no HDMI TX register from which its > > state can be read. Do I need to poll the HDMI_PHY_PTRPT_ENBL register > > through I2C ? How long is the PHY expected to take to set TX_READY to 0 ? > > TX_READY can be read from register 0x1A of phy, BIT(2) (through > I2C). Not sure if same offset for all phys though. I have been told that another option is to poll the TX_PHY_LOCK bit in the phy_stat0 register. That would be a simpler solution that doesn't require I2C access. Could you confirm (or disconfirm) this ? > >> HPD and RXSENSE should work in power down mode by enabling enhpdrxsense > >> bit in phy_conf0 BUT to enter power down you must disable TX_PWRON, > >> enhpdrxsense and enable PDDQ and PHY_RESET. Only after doing this (and > >> consequently entering power down mode) you can enable again enhpdrxsense. > > > > Thanks, I'll give it a try. -- Regards, Laurent Pinchart From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurent Pinchart Subject: Re: [PATCH v2 17/29] drm: bridge: dw-hdmi: Refactor PHY power handling Date: Wed, 01 Mar 2017 13:09:44 +0200 Message-ID: <7271033.6cEIpZMQXV@avalon> References: <20161220013400.28317-1-laurent.pinchart+renesas@ideasonboard.com> <5812310.k87Vx0mP1E@avalon> 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 C42F96E8EE for ; Wed, 1 Mar 2017 11:09:11 +0000 (UTC) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: dri-devel@lists.freedesktop.org Cc: Jose Abreu , Fabio Estevam , Laurent Pinchart , Russell King - ARM Linux , Kieran Bingham , linux-renesas-soc@vger.kernel.org, Ulrich Hecht , Andy Yan , Vladimir Zapolskiy List-Id: dri-devel@lists.freedesktop.org SGkgSm9zZSwKCk9uIFRodXJzZGF5IDA1IEphbiAyMDE3IDE1OjA2OjQ5IEpvc2UgQWJyZXUgd3Jv dGU6Cj4gT24gMDUtMDEtMjAxNyAxMjoyOSwgTGF1cmVudCBQaW5jaGFydCB3cm90ZToKPiA+IE9u IFR1ZXNkYXkgMjAgRGVjIDIwMTYgMTI6MTc6MjMgSm9zZSBBYnJldSB3cm90ZToKPiA+PiBPbiAy MC0xMi0yMDE2IDExOjQ1LCBSdXNzZWxsIEtpbmcgLSBBUk0gTGludXggd3JvdGU6Cj4gPj4+IE9u IFR1ZSwgRGVjIDIwLCAyMDE2IGF0IDAzOjMzOjQ4QU0gKzAyMDAsIExhdXJlbnQgUGluY2hhcnQg d3JvdGU6Cj4gPj4+PiBJbnN0ZWFkIG9mIHNwcmVhZGluZyB2ZXJzaW9uLWRlcGVuZGVudCBQSFkg cG93ZXIgaGFuZGxpbmcgY29kZSBhcm91bmQsCj4gPj4+PiBncm91cCBpdCBpbiB0d28gZnVuY3Rp b25zIHRvIHBvd2VyIHRoZSBQSFkgb24gYW5kIG9mZiBhbmQgdXNlIHRoZW0KPiA+Pj4+IHRocm91 Z2ggdGhlIGRyaXZlci4KPiA+Pj4+IAo+ID4+Pj4gUG93ZXJpbmcgb2ZmIHRoZSBQSFkgYXQgdGhl IGJlZ2lubmluZyBvZiB0aGUgc2V0dXAgcGhhc2UgaXMgY3VycmVudGx5Cj4gPj4+PiBzcGxpdCBp biB0d28gbG9jYXRpb25zIGZvciBmaXJzdCBhbmQgc2Vjb25kIGdlbmVyYXRpb24gUEhZcywgZ3Jv dXAgYWxsCj4gPj4+PiB0aGUgb3BlcmF0aW9ucyBpbiB0aGUgZHdfaGRtaV9waHlfaW5pdCgpIGZ1 bmN0aW9uLgo+ID4+PiAKPiA+Pj4gVGhpcyBjaGFuZ2VzIHRoZSBiZWhhdmlvdXIgb2YgdGhlIGRy aXZlci4KPiA+Pj4gCj4gPj4+PiArc3RhdGljIHZvaWQgZHdfaGRtaV9waHlfcG93ZXJfb2ZmKHN0 cnVjdCBkd19oZG1pICpoZG1pKQo+ID4+Pj4gK3sKPiA+Pj4+ICsJaWYgKGhkbWktPnBoeS0+Z2Vu ID09IDEpIHsKPiA+Pj4+ICsJCWR3X2hkbWlfcGh5X2VuYWJsZV90bWRzKGhkbWksIDApOwo+ID4+ Pj4gKwkJZHdfaGRtaV9waHlfZW5hYmxlX3Bvd2VyZG93bihoZG1pLCB0cnVlKTsKPiA+Pj4+ICsJ fSBlbHNlIHsKPiA+Pj4+ICsJCWR3X2hkbWlfcGh5X2dlbjJfdHhwd3JvbihoZG1pLCAwKTsKPiA+ Pj4+ICsJCWR3X2hkbWlfcGh5X2dlbjJfcGRkcShoZG1pLCAxKTsKPiA+Pj4+ICsJfQo+ID4+Pj4g K30KPiA+Pj4+IEBAIC0xMjkwLDkgKzEzMDIsNyBAQCBzdGF0aWMgdm9pZCBkd19oZG1pX3BoeV9k aXNhYmxlKHN0cnVjdCBkd19oZG1pCj4gPj4+PiAqaGRtaSkKPiA+Pj4+ICAJaWYgKCFoZG1pLT5w aHlfZW5hYmxlZCkKPiA+Pj4+ICAJCXJldHVybjsKPiA+Pj4+IAo+ID4+Pj4gLQlkd19oZG1pX3Bo eV9lbmFibGVfdG1kcyhoZG1pLCAwKTsKPiA+Pj4+IC0JZHdfaGRtaV9waHlfZW5hYmxlX3Bvd2Vy ZG93bihoZG1pLCB0cnVlKTsKPiA+Pj4+IC0KPiA+Pj4+ICsJZHdfaGRtaV9waHlfcG93ZXJfb2Zm KGhkbWkpOwo+ID4+PiAKPiA+Pj4gVGhpcyBtYWtlcyBkd19oZG1pX3BoeV9kaXNhYmxlKCkgcG93 ZXIgZG93biBhIGdlbjIgcGh5Lgo+ID4+PiAKPiA+Pj4gVGhlIGlNWDYgaGFzIGEgRFdfSERNSV9Q SFlfRFdDX0hETUlfM0RfVFhfUEhZIHBoeSwgd2hpY2ggeW91IGxpc3QgYXMgYQo+ID4+PiBnZW4y IHBoeS4gIEkndmUgYmVlbiBjYXJyeWluZyB0aGlzIGNoYW5nZSBmb3IgYSB3aGlsZSwgd2hpY2gg SSd2ZSBoYWQKPiA+Pj4gdG8gcmV2ZXJ0IChhbmQgZmluYWxseSBleHB1bmdlKSwgYXMgaXQgY2F1 c2VzIHByb2JsZW1zIG9uIGlNWDY6Cj4gPj4+IAo+ID4+PiBAQCAtMTExMiw2ICsxMTEyLDE0IEBA IHN0YXRpYyB2b2lkIGR3X2hkbWlfcGh5X2Rpc2FibGUoc3RydWN0IGR3X2hkbWkKPiA+Pj4gKmhk bWkpCj4gPj4+ICAgICAgICAgaWYgKCFoZG1pLT5waHlfZW5hYmxlZCkKPiA+Pj4gICAgICAgICAg ICAgICAgIHJldHVybjsKPiA+Pj4gCj4gPj4+ICsgICAgICAgLyogQWN0dWFsbHkgc2V0IHRoZSBw aHkgaW50byBsb3cgcG93ZXIgbW9kZSAqLwo+ID4+PiArICAgICAgIGR3X2hkbWlfcGh5X2dlbjJf dHhwd3JvbihoZG1pLCAwKTsKPiA+Pj4gKwo+ID4+PiArICAgICAgIC8qIEZJWE1FOiBXZSBzaG91 bGQgd2FpdCBmb3IgVFhfUkVBRFkgdG8gYmUgZGVhc3NlcnRlZCAqLwo+ID4+PiArCj4gPj4+ICsg ICAgICAgZHdfaGRtaV9waHlfZ2VuMl9wZGRxKGhkbWksIDEpOwo+ID4+PiArCj4gPj4+ICsgICAg ICAgLyogVGhpcyBhcHBlYXJzIHRvIGhhdmUgbm8gZWZmZWN0IG9uIGlNWDYgKi8KPiA+Pj4gICAg ICAgICBkd19oZG1pX3BoeV9lbmFibGVfdG1kcyhoZG1pLCAwKTsKPiA+Pj4gICAgICAgICBkd19o ZG1pX3BoeV9lbmFibGVfcG93ZXJkb3duKGhkbWksIHRydWUpOwo+ID4+PiAKPiA+Pj4gU28sIEkg dGhpbmsgeW91ciBjaGFuZ2UgaGVyZSB3aWxsIGNhdXNlIHByb2JsZW1zIGZvciBpTVg2Lgo+ID4+ PiAKPiA+Pj4gRnJvbSB3aGF0IEkgcmVtZW1iZXIsIGl0IHNlZW1zIHRoYXQgaU1YNiBoYXMgaXNz dWVzIHdpdGggUlhTRU5TRS9IUEQKPiA+Pj4gYm91bmNpbmcgd2hlbiB0aGUgUEhZIGlzIHBvd2Vy ZWQgZG93bi4gIEkgY2FuJ3QgcHJvbWlzZSB3aGVuIEknbGwgYmUKPiA+Pj4gYWJsZSB0byBjaGVj ayBmb3IgdGhhdCBhZ2Fpbi4KPiA+PiAKPiA+PiBJbmRlZWQgVFhfUkVBRFkgbXVzdCBiZSBsb3cg YmVmb3JlIGFzc2VydGluZyBwZGRxLgo+ID4gCj4gPiBUaGUgVFhfUkVBRFkgc2lnbmFsIGlzIGRv Y3VtZW50ZWQgaW4gdGhlIGkuTVg2IGRhdGFzaGVldCBhcyBiZWluZyBhIFBIWQo+ID4gb3V0cHV0 IHNpZ25hbCwgYnV0IHRoZXJlIHNlZW1zIHRvIGJlIG5vIEhETUkgVFggcmVnaXN0ZXIgZnJvbSB3 aGljaCBpdHMKPiA+IHN0YXRlIGNhbiBiZSByZWFkLiBEbyBJIG5lZWQgdG8gcG9sbCB0aGUgSERN SV9QSFlfUFRSUFRfRU5CTCByZWdpc3Rlcgo+ID4gdGhyb3VnaCBJMkMgPyBIb3cgbG9uZyBpcyB0 aGUgUEhZIGV4cGVjdGVkIHRvIHRha2UgdG8gc2V0IFRYX1JFQURZIHRvIDAgPwo+IAo+IFRYX1JF QURZIGNhbiBiZSByZWFkIGZyb20gcmVnaXN0ZXIgMHgxQSBvZiBwaHksIEJJVCgyKSAodGhyb3Vn aAo+IEkyQykuIE5vdCBzdXJlIGlmIHNhbWUgb2Zmc2V0IGZvciBhbGwgcGh5cyB0aG91Z2guCgpJ IGhhdmUgYmVlbiB0b2xkIHRoYXQgYW5vdGhlciBvcHRpb24gaXMgdG8gcG9sbCB0aGUgVFhfUEhZ X0xPQ0sgYml0IGluIHRoZSAKcGh5X3N0YXQwIHJlZ2lzdGVyLiBUaGF0IHdvdWxkIGJlIGEgc2lt cGxlciBzb2x1dGlvbiB0aGF0IGRvZXNuJ3QgcmVxdWlyZSBJMkMgCmFjY2Vzcy4gQ291bGQgeW91 IGNvbmZpcm0gKG9yIGRpc2NvbmZpcm0pIHRoaXMgPwoKPiA+PiBIUEQgYW5kIFJYU0VOU0Ugc2hv dWxkIHdvcmsgaW4gcG93ZXIgZG93biBtb2RlIGJ5IGVuYWJsaW5nIGVuaHBkcnhzZW5zZQo+ID4+ IGJpdCBpbiBwaHlfY29uZjAgQlVUIHRvIGVudGVyIHBvd2VyIGRvd24geW91IG11c3QgZGlzYWJs ZSBUWF9QV1JPTiwKPiA+PiBlbmhwZHJ4c2Vuc2UgYW5kIGVuYWJsZSBQRERRIGFuZCBQSFlfUkVT RVQuIE9ubHkgYWZ0ZXIgZG9pbmcgdGhpcyAoYW5kCj4gPj4gY29uc2VxdWVudGx5IGVudGVyaW5n IHBvd2VyIGRvd24gbW9kZSkgeW91IGNhbiBlbmFibGUgYWdhaW4gZW5ocGRyeHNlbnNlLgo+ID4g Cj4gPiBUaGFua3MsIEknbGwgZ2l2ZSBpdCBhIHRyeS4KCi0tIApSZWdhcmRzLAoKTGF1cmVudCBQ aW5jaGFydAoKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18K ZHJpLWRldmVsIG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0 dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg==